From 78c5a3b6a9839a8d7a127be4b5b83f657a3d937f Mon Sep 17 00:00:00 2001 From: Kang Ming Date: Wed, 3 Jul 2024 12:38:52 -0700 Subject: [PATCH 1/2] fix: apply mailer autoconfirm config to update user email --- internal/api/user.go | 30 +++++++++++++++++++++--------- internal/api/verify.go | 5 ++++- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/internal/api/user.go b/internal/api/user.go index 4cb1fc1d9..a33304b82 100644 --- a/internal/api/user.go +++ b/internal/api/user.go @@ -8,6 +8,7 @@ import ( "github.com/gofrs/uuid" "github.com/supabase/auth/internal/api/sms_provider" + "github.com/supabase/auth/internal/mailer" "github.com/supabase/auth/internal/models" "github.com/supabase/auth/internal/storage" ) @@ -205,19 +206,30 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error { } if params.Email != "" && params.Email != user.GetEmail() { - flowType := getFlowFromChallenge(params.CodeChallenge) - if isPKCEFlow(flowType) { - _, terr := generateFlowState(tx, models.EmailChange.String(), models.EmailChange, params.CodeChallengeMethod, params.CodeChallenge, &user.ID) - if terr != nil { + if config.Mailer.Autoconfirm { + user.EmailChange = params.Email + if _, terr := a.emailChangeVerify(r, tx, &VerifyParams{ + Type: mailer.EmailChangeVerification, + Email: params.Email, + }, user); terr != nil { return terr } - } - if terr = a.sendEmailChange(r, tx, user, params.Email, flowType); terr != nil { - if errors.Is(terr, MaxFrequencyLimitError) { - return tooManyRequestsError(ErrorCodeOverEmailSendRateLimit, generateFrequencyLimitErrorMessage(user.EmailChangeSentAt, config.SMTP.MaxFrequency)) + } else { + flowType := getFlowFromChallenge(params.CodeChallenge) + if isPKCEFlow(flowType) { + _, terr := generateFlowState(tx, models.EmailChange.String(), models.EmailChange, params.CodeChallengeMethod, params.CodeChallenge, &user.ID) + if terr != nil { + return terr + } + + } + if terr = a.sendEmailChange(r, tx, user, params.Email, flowType); terr != nil { + if errors.Is(terr, MaxFrequencyLimitError) { + return tooManyRequestsError(ErrorCodeOverEmailSendRateLimit, generateFrequencyLimitErrorMessage(user.EmailChangeSentAt, config.SMTP.MaxFrequency)) + } + return internalServerError("Error sending change email").WithInternalError(terr) } - return internalServerError("Error sending change email").WithInternalError(terr) } } diff --git a/internal/api/verify.go b/internal/api/verify.go index c0b78d680..97a13b93b 100644 --- a/internal/api/verify.go +++ b/internal/api/verify.go @@ -491,7 +491,10 @@ func (a *API) prepPKCERedirectURL(rurl, code string) (string, error) { func (a *API) emailChangeVerify(r *http.Request, conn *storage.Connection, params *VerifyParams, user *models.User) (*models.User, error) { config := a.config - if config.Mailer.SecureEmailChangeEnabled && user.EmailChangeConfirmStatus == zeroConfirmation && user.GetEmail() != "" { + if !config.Mailer.Autoconfirm && + config.Mailer.SecureEmailChangeEnabled && + user.EmailChangeConfirmStatus == zeroConfirmation && + user.GetEmail() != "" { err := conn.Transaction(func(tx *storage.Connection) error { currentOTT, terr := models.FindOneTimeToken(tx, params.TokenHash, models.EmailChangeTokenCurrent) if terr != nil && !models.IsNotFoundError(terr) { From afddb2fe7ee01171d6b6c07e3c44d63b847dce03 Mon Sep 17 00:00:00 2001 From: Kang Ming Date: Thu, 4 Jul 2024 14:41:36 -0700 Subject: [PATCH 2/2] chore: add tests for update email with autoconfirm --- internal/api/user_test.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/internal/api/user_test.go b/internal/api/user_test.go index af9cfec37..cef28d607 100644 --- a/internal/api/user_test.go +++ b/internal/api/user_test.go @@ -84,6 +84,7 @@ func (ts *UserTestSuite) TestUserUpdateEmail() { desc string userData map[string]string isSecureEmailChangeEnabled bool + isMailerAutoconfirmEnabled bool expectedCode int }{ { @@ -93,6 +94,7 @@ func (ts *UserTestSuite) TestUserUpdateEmail() { "phone": "", }, isSecureEmailChangeEnabled: false, + isMailerAutoconfirmEnabled: false, expectedCode: http.StatusOK, }, { @@ -102,6 +104,7 @@ func (ts *UserTestSuite) TestUserUpdateEmail() { "phone": "234567890", }, isSecureEmailChangeEnabled: true, + isMailerAutoconfirmEnabled: false, expectedCode: http.StatusOK, }, { @@ -111,6 +114,7 @@ func (ts *UserTestSuite) TestUserUpdateEmail() { "phone": "", }, isSecureEmailChangeEnabled: false, + isMailerAutoconfirmEnabled: false, expectedCode: http.StatusOK, }, { @@ -120,6 +124,17 @@ func (ts *UserTestSuite) TestUserUpdateEmail() { "phone": "", }, isSecureEmailChangeEnabled: true, + isMailerAutoconfirmEnabled: false, + expectedCode: http.StatusOK, + }, + { + desc: "Update email with mailer autoconfirm enabled", + userData: map[string]string{ + "email": "bar@example.com", + "phone": "", + }, + isSecureEmailChangeEnabled: true, + isMailerAutoconfirmEnabled: true, expectedCode: http.StatusOK, }, } @@ -146,9 +161,22 @@ func (ts *UserTestSuite) TestUserUpdateEmail() { w := httptest.NewRecorder() ts.Config.Mailer.SecureEmailChangeEnabled = c.isSecureEmailChangeEnabled + ts.Config.Mailer.Autoconfirm = c.isMailerAutoconfirmEnabled ts.API.handler.ServeHTTP(w, req) require.Equal(ts.T(), c.expectedCode, w.Code) + var data models.User + require.NoError(ts.T(), json.NewDecoder(w.Body).Decode(&data)) + + if c.isMailerAutoconfirmEnabled { + require.Empty(ts.T(), data.EmailChange) + require.Equal(ts.T(), "new@example.com", data.GetEmail()) + require.Len(ts.T(), data.Identities, 1) + } else { + require.Equal(ts.T(), "new@example.com", data.EmailChange) + require.Len(ts.T(), data.Identities, 0) + } + // remove user after each case require.NoError(ts.T(), ts.API.db.Destroy(u)) })