From 51523e88c15ab578640ab0ecf4756212d737aaf6 Mon Sep 17 00:00:00 2001
From: joel <lee.yi.jie.joel@gmail.com>
Date: Tue, 12 Nov 2024 22:34:41 +0800
Subject: [PATCH] fix: add tests

---
 internal/api/user_test.go | 50 ++++++++++++++++++++++++++++++++++++++-
 internal/models/user.go   | 19 +++++++--------
 2 files changed, 58 insertions(+), 11 deletions(-)

diff --git a/internal/api/user_test.go b/internal/api/user_test.go
index 310979d80..ed74a9ee2 100644
--- a/internal/api/user_test.go
+++ b/internal/api/user_test.go
@@ -195,8 +195,8 @@ func (ts *UserTestSuite) TestUserUpdateEmail() {
 			require.NoError(ts.T(), ts.API.db.Destroy(u))
 		})
 	}
-
 }
+
 func (ts *UserTestSuite) TestUserUpdatePhoneAutoconfirmEnabled() {
 	u, err := models.FindUserByEmailAndAudience(ts.API.db, "test@example.com", ts.Config.JWT.Aud)
 	require.NoError(ts.T(), err)
@@ -269,6 +269,54 @@ func (ts *UserTestSuite) TestUserUpdatePhoneAutoconfirmEnabled() {
 
 }
 
+func (ts *UserTestSuite) TestUserAllowPhoneUpdateIfPhoneNotConfirmed() {
+	// Sign up first user with Phone A
+	existingUser, err := models.NewUser("333333333", "", "", ts.Config.JWT.Aud, nil)
+	require.NoError(ts.T(), err)
+	require.NoError(ts.T(), ts.API.db.Create(existingUser))
+
+	var buffer bytes.Buffer
+	// Sign up second user with phone B
+	phoneB := "1234567890"
+
+	require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{
+		"phone":    phoneB,
+		"password": "testpassword",
+	}))
+
+	ts.Config.External.Phone.Enabled = true
+	ts.Config.Sms.Autoconfirm = true
+	req := httptest.NewRequest(http.MethodPost, "/signup", &buffer)
+	req.Header.Set("Content-Type", "application/json")
+	w := httptest.NewRecorder()
+	ts.API.handler.ServeHTTP(w, req)
+	require.Equal(ts.T(), http.StatusOK, w.Code)
+
+	// Try to update Phone A user's phone number to Phone B
+	buffer.Reset()
+	require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{
+		"phone": phoneB,
+	}))
+
+	data := AccessTokenResponse{}
+
+	require.NoError(ts.T(), json.NewDecoder(w.Body).Decode(&data))
+
+	// Make update request
+	req = httptest.NewRequest(http.MethodPut, "/user", &buffer)
+	req.Header.Set("Content-Type", "application/json")
+	req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", data.Token))
+	w = httptest.NewRecorder()
+	ts.API.handler.ServeHTTP(w, req)
+
+	// Should succeed since the original phone number isn't confirmed
+	require.Equal(ts.T(), http.StatusOK, w.Code)
+
+	updatedUser := &models.User{}
+	require.NoError(ts.T(), json.NewDecoder(w.Body).Decode(updatedUser))
+	require.Equal(ts.T(), phoneB, updatedUser.GetPhone())
+}
+
 func (ts *UserTestSuite) TestUserUpdatePassword() {
 	u, err := models.FindUserByEmailAndAudience(ts.API.db, "test@example.com", ts.Config.JWT.Aud)
 	require.NoError(ts.T(), err)
diff --git a/internal/models/user.go b/internal/models/user.go
index 5fb9a9171..9bf577af3 100644
--- a/internal/models/user.go
+++ b/internal/models/user.go
@@ -759,21 +759,20 @@ func IsDuplicatedPhone(tx *storage.Connection, phone, aud string) (bool, error)
 	return true, nil
 }
 
-// TODO: Simplify this, check if it is safe to do this in admin create user too
 // HasPhoneIdentity checks if the phone number already exists in the identities table
 func HasPhoneIdentity(tx *storage.Connection, phone, aud string) (bool, error) {
-	query := `SELECT 1 FROM users
-        JOIN identities ON users.id = identities.user_id
-        WHERE users.phone = ? AND users.aud = ? AND identities.provider = ?
-        LIMIT 1`
-
-	var exists bool
-	q := tx.RawQuery(query, phone, aud, "phone")
-	exists, err := q.Exists(&exists)
+	usersTable := (&pop.Model{Value: User{}}).TableName()
+
+	exists, err := tx.Q().
+		Join(usersTable, fmt.Sprintf("%s.id = identities.user_id", usersTable)).
+		Where("users.phone = ? AND users.aud = ? AND identities.provider = ?",
+			phone, aud, "phone").
+		Limit(1).
+		Exists(Identity{})
+
 	if err != nil {
 		return false, err
 	}
-
 	return exists, nil
 }