From 91e0a6c169ce494387d3ea2eaa3dc0fba75b0f1b Mon Sep 17 00:00:00 2001 From: Kang Ming Date: Fri, 9 Feb 2024 01:00:41 +0800 Subject: [PATCH 1/2] Revert "fix: only create or update the email / phone identity after it's been verified (#1403)" This reverts commit 2d207296ec22dd6c003c89626d255e35441fd52d. --- internal/api/user.go | 49 ++++++++++++++++++++++--- internal/api/verify.go | 82 +++++++++--------------------------------- 2 files changed, 62 insertions(+), 69 deletions(-) diff --git a/internal/api/user.go b/internal/api/user.go index 83d067a264..02fd099b21 100644 --- a/internal/api/user.go +++ b/internal/api/user.go @@ -7,7 +7,9 @@ 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" @@ -191,7 +193,29 @@ 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) @@ -214,13 +238,29 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error { } if params.Phone != "" && params.Phone != user.GetPhone() { - if config.Sms.Autoconfirm { - if _, terr := a.smsVerify(r, ctx, tx, user, &VerifyParams{ - Type: phoneChangeVerification, - Phone: params.Phone, + 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, }); 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 { @@ -231,6 +271,7 @@ 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 ecacc1e9ac..c338064097 100644 --- a/internal/api/verify.go +++ b/internal/api/verify.go @@ -10,9 +10,7 @@ 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" @@ -260,7 +258,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) + user, terr = a.smsVerify(r, ctx, tx, user, params.Type) default: return unprocessableEntityError("Unsupported verification type") } @@ -369,53 +367,28 @@ 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, params *VerifyParams) (*models.User, error) { +func (a *API) smsVerify(r *http.Request, ctx context.Context, conn *storage.Connection, user *models.User, otpType string) (*models.User, error) { config := a.config err := conn.Transaction(func(tx *storage.Connection) error { - if terr := triggerEventHooks(ctx, tx, SignupEvent, user, config); terr != nil { + var terr error + if terr = models.NewAuditLogEntry(r, tx, user, models.UserSignedUpAction, "", nil); 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 { + if terr = triggerEventHooks(ctx, tx, SignupEvent, user, config); terr != nil { + return terr + } + + if otpType == smsVerification { + 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 - } - // 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 { + } else if otpType == phoneChangeVerification { + 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 { @@ -505,38 +478,17 @@ 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 { - if terr := models.NewAuditLogEntry(r, tx, user, models.UserModifiedAction, "", nil); terr != nil { + var terr error + + 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 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 { + if terr = user.ConfirmEmailChange(tx, zeroConfirmation); terr != nil { return internalServerError("Error confirm email").WithInternalError(terr) } From 7763bd10b44ad5ebace58bb3c9d2e2d595f7e681 Mon Sep 17 00:00:00 2001 From: Stojan Dimitrovski Date: Thu, 8 Feb 2024 18:01:31 +0100 Subject: [PATCH 2/2] ci: allow revert in commits and PRs (#1406) Allows the use of the `revert` semantic commit. --- .github/workflows/conventional-commits-lint.js | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/conventional-commits-lint.js b/.github/workflows/conventional-commits-lint.js index da9b388b60..3c81518e1b 100644 --- a/.github/workflows/conventional-commits-lint.js +++ b/.github/workflows/conventional-commits-lint.js @@ -8,6 +8,7 @@ const RELEASE_AS_DIRECTIVE = /^\s*Release-As:/im; const BREAKING_CHANGE_DIRECTIVE = /^\s*BREAKING[ \t]+CHANGE:/im; const ALLOWED_CONVENTIONAL_COMMIT_PREFIXES = [ + "revert", "feat", "fix", "ci",