fix: apply mailer autoconfirm config to update user email (#1646)

## What kind of change does this PR introduce? * `GOTRUE_MAILER_AUTOCONFIRM` setting should be respected in the update email flow via `PUT /user` ## What is the current behavior? * When `GOTRUE_MAILER_AUTOCONFIRM=true`, updating a user's email still sends an email and requires user confirmation * Difficult for anonymous users to upgrade to a permanent user seamlessly without requiring email confirmation * Fixes #1619 ## What is the new behavior? * When `GOTRUE_MAILER_AUTOCONFIRM=true`, updating a user's email will not require email confirmation. ## Additional context Add any other context or screenshots.
supabase · Jul 4, 2024 · a518505 · a518505
1 parent 42c1d45
commit a518505
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 10 deletions.
diff --git 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/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))
 		})

diff --git 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) {