fix: create & update phone identity after verify

supabase · Feb 7, 2024 · 80e0262 · 80e0262
1 parent b7f44d8
commit 80e0262
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 33 deletions.
diff --git a/internal/api/user.go b/internal/api/user.go
@@ -7,9 +7,7 @@ import (
 	"net/http"
 	"time"
 
-	"github.com/fatih/structs"
 	"github.com/gofrs/uuid"
-	"github.com/supabase/auth/internal/api/provider"
 	"github.com/supabase/auth/internal/api/sms_provider"
 	"github.com/supabase/auth/internal/models"
 	"github.com/supabase/auth/internal/storage"
@@ -193,7 +191,6 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error {
 			}
 		}
 
-		var identities []models.Identity
 		if params.Email != "" && params.Email != user.GetEmail() {
 			mailer := a.Mailer(ctx)
 			referrer := utilities.GetReferrer(r, config)
@@ -217,29 +214,13 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error {
 		}
 
 		if params.Phone != "" && params.Phone != user.GetPhone() {
-			identity, terr := models.FindIdentityByIdAndProvider(tx, user.ID.String(), "phone")
-			if terr != nil {
-				if !models.IsNotFoundError(terr) {
-					return terr
-				}
-				// updating the user's phone should create a new phone identity since the user doesn't have one
-				identity, terr = a.createNewIdentity(tx, user, "phone", structs.Map(provider.Claims{
-					Subject: user.ID.String(),
-					Phone:   params.Phone,
-				}))
-				if terr != nil {
-					return terr
-				}
-			} else {
-				if terr := identity.UpdateIdentityData(tx, map[string]interface{}{
-					"phone": params.Phone,
+			if config.Sms.Autoconfirm {
+				if _, terr := a.smsVerify(r, ctx, tx, user, &VerifyParams{
+					Type:  phoneChangeVerification,
+					Phone: params.Phone,
 				}); terr != nil {
 					return terr
 				}
-			}
-			identities = append(identities, *identity)
-			if config.Sms.Autoconfirm {
-				return user.UpdatePhone(tx, params.Phone)
 			} else {
 				smsProvider, terr := sms_provider.GetSmsProvider(*config)
 				if terr != nil {
@@ -250,7 +231,6 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error {
 				}
 			}
 		}
-		user.Identities = append(user.Identities, identities...)
 
 		if terr = models.NewAuditLogEntry(r, tx, user, models.UserModifiedAction, "", nil); terr != nil {
 			return internalServerError("Error recording audit log entry").WithInternalError(terr)

diff --git a/internal/api/verify.go b/internal/api/verify.go
@@ -260,7 +260,7 @@ func (a *API) verifyPost(w http.ResponseWriter, r *http.Request, params *VerifyP
 				return nil
 			}
 		case smsVerification, phoneChangeVerification:
-			user, terr = a.smsVerify(r, ctx, tx, user, params.Type)
+			user, terr = a.smsVerify(r, ctx, tx, user, params)
 		default:
 			return unprocessableEntityError("Unsupported verification type")
 		}
@@ -369,28 +369,51 @@ func (a *API) recoverVerify(r *http.Request, ctx context.Context, conn *storage.
 	return user, nil
 }
 
-func (a *API) smsVerify(r *http.Request, ctx context.Context, conn *storage.Connection, user *models.User, otpType string) (*models.User, error) {
+func (a *API) smsVerify(r *http.Request, ctx context.Context, conn *storage.Connection, user *models.User, params *VerifyParams) (*models.User, error) {
 	config := a.config
 
 	err := conn.Transaction(func(tx *storage.Connection) error {
-		var terr error
-		if terr = models.NewAuditLogEntry(r, tx, user, models.UserSignedUpAction, "", nil); terr != nil {
+		if terr := models.NewAuditLogEntry(r, tx, user, models.UserSignedUpAction, "", nil); terr != nil {
 			return terr
 		}
 
-		if terr = triggerEventHooks(ctx, tx, SignupEvent, user, config); terr != nil {
+		if terr := triggerEventHooks(ctx, tx, SignupEvent, user, config); terr != nil {
 			return terr
 		}
 
-		if otpType == smsVerification {
-			if terr = user.ConfirmPhone(tx); terr != nil {
+		if params.Type == smsVerification {
+			if terr := user.ConfirmPhone(tx); terr != nil {
 				return internalServerError("Error confirming user").WithInternalError(terr)
 			}
-		} else if otpType == phoneChangeVerification {
-			if terr = user.ConfirmPhoneChange(tx); terr != nil {
+		} else if params.Type == phoneChangeVerification {
+			if identity, terr := models.FindIdentityByIdAndProvider(tx, user.ID.String(), "phone"); terr != nil {
+				if !models.IsNotFoundError(terr) {
+					return terr
+				}
+				// confirming the phone change should create a new phone identity if the user doesn't have one
+				if _, terr = a.createNewIdentity(tx, user, "phone", structs.Map(provider.Claims{
+					Subject:       user.ID.String(),
+					Phone:         params.Phone,
+					PhoneVerified: true,
+				})); terr != nil {
+					return terr
+				}
+			} else {
+				if terr := identity.UpdateIdentityData(tx, map[string]interface{}{
+					"phone":          params.Phone,
+					"phone_verified": true,
+				}); terr != nil {
+					return terr
+				}
+			}
+			if terr := user.ConfirmPhoneChange(tx); terr != nil {
 				return internalServerError("Error confirming user").WithInternalError(terr)
 			}
 		}
+
+		if terr := tx.Load(user, "Identities"); terr != nil {
+			return internalServerError("Error refetching identities").WithInternalError(terr)
+		}
 		return nil
 	})
 	if err != nil {