From b7f44d817df9f0769d781dc2e9a7cc3bf24900a2 Mon Sep 17 00:00:00 2001 From: Kang Ming Date: Wed, 7 Feb 2024 18:38:15 +0800 Subject: [PATCH 1/3] fix: create & update email identity after verify --- internal/api/user.go | 21 --------------------- internal/api/verify.go | 33 ++++++++++++++++++++++++++++----- 2 files changed, 28 insertions(+), 26 deletions(-) diff --git a/internal/api/user.go b/internal/api/user.go index 02fd099b21..50ea3fa4b7 100644 --- a/internal/api/user.go +++ b/internal/api/user.go @@ -195,27 +195,6 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error { var identities []models.Identity if params.Email != "" && params.Email != user.GetEmail() { - identity, terr := models.FindIdentityByIdAndProvider(tx, user.ID.String(), "email") - if terr != nil { - if !models.IsNotFoundError(terr) { - return terr - } - // updating the user's email should create a new email identity since the user doesn't have one - identity, terr = a.createNewIdentity(tx, user, "email", structs.Map(provider.Claims{ - Subject: user.ID.String(), - Email: params.Email, - })) - if terr != nil { - return terr - } - } else { - if terr := identity.UpdateIdentityData(tx, map[string]interface{}{ - "email": params.Email, - }); terr != nil { - return terr - } - } - identities = append(identities, *identity) mailer := a.Mailer(ctx) referrer := utilities.GetReferrer(r, config) flowType := getFlowFromChallenge(params.CodeChallenge) diff --git a/internal/api/verify.go b/internal/api/verify.go index c338064097..590aea5b5a 100644 --- a/internal/api/verify.go +++ b/internal/api/verify.go @@ -10,7 +10,9 @@ import ( "strings" "time" + "github.com/fatih/structs" "github.com/sethvargo/go-password/password" + "github.com/supabase/auth/internal/api/provider" "github.com/supabase/auth/internal/api/sms_provider" "github.com/supabase/auth/internal/crypto" "github.com/supabase/auth/internal/models" @@ -478,17 +480,38 @@ func (a *API) emailChangeVerify(r *http.Request, ctx context.Context, conn *stor // one email is confirmed at this point if GOTRUE_MAILER_SECURE_EMAIL_CHANGE_ENABLED is enabled err := conn.Transaction(func(tx *storage.Connection) error { - var terr error - - if terr = models.NewAuditLogEntry(r, tx, user, models.UserModifiedAction, "", nil); terr != nil { + if terr := models.NewAuditLogEntry(r, tx, user, models.UserModifiedAction, "", nil); terr != nil { return terr } - if terr = triggerEventHooks(ctx, tx, EmailChangeEvent, user, config); terr != nil { + if terr := triggerEventHooks(ctx, tx, EmailChangeEvent, user, config); terr != nil { return terr } - if terr = user.ConfirmEmailChange(tx, zeroConfirmation); terr != nil { + if identity, terr := models.FindIdentityByIdAndProvider(tx, user.ID.String(), "email"); terr != nil { + if !models.IsNotFoundError(terr) { + return terr + } + // confirming the email change should create a new email identity if the user doesn't have one + if _, terr = a.createNewIdentity(tx, user, "email", structs.Map(provider.Claims{ + Subject: user.ID.String(), + Email: params.Email, + EmailVerified: true, + })); terr != nil { + return terr + } + } else { + if terr := identity.UpdateIdentityData(tx, map[string]interface{}{ + "email": params.Email, + "email_verified": true, + }); terr != nil { + return terr + } + } + if terr := tx.Load(user, "Identities"); terr != nil { + return internalServerError("Error refetching identities").WithInternalError(terr) + } + if terr := user.ConfirmEmailChange(tx, zeroConfirmation); terr != nil { return internalServerError("Error confirm email").WithInternalError(terr) } From 80e02628c18677bf034bfc2325f4254a68fa6ee8 Mon Sep 17 00:00:00 2001 From: Kang Ming Date: Wed, 7 Feb 2024 18:47:32 +0800 Subject: [PATCH 2/3] fix: create & update phone identity after verify --- internal/api/user.go | 28 ++++------------------------ internal/api/verify.go | 41 ++++++++++++++++++++++++++++++++--------- 2 files changed, 36 insertions(+), 33 deletions(-) diff --git a/internal/api/user.go b/internal/api/user.go index 50ea3fa4b7..83d067a264 100644 --- 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 index 590aea5b5a..52d4e6b2ba 100644 --- 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 { From fc2554b1bfe582398643f9fb5df40e0e8bde880f Mon Sep 17 00:00:00 2001 From: Kang Ming Date: Wed, 7 Feb 2024 19:50:23 +0800 Subject: [PATCH 3/3] fix: update audit log events for phone change --- internal/api/verify.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/internal/api/verify.go b/internal/api/verify.go index 52d4e6b2ba..ecacc1e9ac 100644 --- a/internal/api/verify.go +++ b/internal/api/verify.go @@ -373,19 +373,21 @@ func (a *API) smsVerify(r *http.Request, ctx context.Context, conn *storage.Conn config := a.config err := conn.Transaction(func(tx *storage.Connection) error { - if terr := models.NewAuditLogEntry(r, tx, user, models.UserSignedUpAction, "", nil); terr != nil { - return terr - } - if terr := triggerEventHooks(ctx, tx, SignupEvent, user, config); terr != nil { return terr } if params.Type == smsVerification { + if terr := models.NewAuditLogEntry(r, tx, user, models.UserSignedUpAction, "", nil); terr != nil { + return terr + } if terr := user.ConfirmPhone(tx); terr != nil { return internalServerError("Error confirming user").WithInternalError(terr) } } else if params.Type == phoneChangeVerification { + if terr := models.NewAuditLogEntry(r, tx, user, models.UserModifiedAction, "", nil); terr != nil { + return terr + } if identity, terr := models.FindIdentityByIdAndProvider(tx, user.ID.String(), "phone"); terr != nil { if !models.IsNotFoundError(terr) { return terr