fix: check for phone identity on Phone Change

internal/api/user.go

@@ -136,7 +136,7 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error {
 	}
 
 	if params.Phone != "" && user.GetPhone() != params.Phone {
-		if exists, err := models.IsDuplicatedPhone(db, params.Phone, aud); err != nil {
+		if exists, err := models.HasPhoneIdentity(db, params.Phone, aud); err != nil {
 			return internalServerError("Database error checking phone").WithInternalError(err)
 		} else if exists {
 			return unprocessableEntityError(ErrorCodePhoneExists, DuplicatePhoneMsg)

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, "[email protected]", ts.Config.JWT.Aud)
 	require.NoError(ts.T(), err)
@@ -205,6 +205,12 @@ func (ts *UserTestSuite) TestUserUpdatePhoneAutoconfirmEnabled() {
 	require.NoError(ts.T(), err)
 	require.NoError(ts.T(), ts.API.db.Create(existingUser))
 
+	idPhone, err := models.NewIdentity(existingUser, "phone", map[string]interface{}{
+		"sub": "+29382983298",
+	})
+	require.NoError(ts.T(), err)
+	require.NoError(ts.T(), ts.API.db.Create(idPhone))
+
 	cases := []struct {
 		desc         string
 		userData     map[string]string
@@ -263,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, "[email protected]", ts.Config.JWT.Aud)
 	require.NoError(ts.T(), err)

internal/models/user.go

@@ -759,6 +759,23 @@ func IsDuplicatedPhone(tx *storage.Connection, phone, aud string) (bool, error)
 	return true, nil
 }
 
+// HasPhoneIdentity checks if the phone number already exists in the identities table
+func HasPhoneIdentity(tx *storage.Connection, phone, aud string) (bool, error) {
+	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
+}
+
 // Ban a user for a given duration.
 func (u *User) Ban(tx *storage.Connection, duration time.Duration) error {
 	if duration == time.Duration(0) {