From df43199d6622aae094f84ef3eb23e6a309e23a52 Mon Sep 17 00:00:00 2001 From: shkuls Date: Mon, 29 Jun 2026 12:56:54 +0530 Subject: [PATCH 1/3] fix: don't bypass double_confirm_changes when mailer_autoconfirm is true When GOTRUE_MAILER_AUTOCONFIRM=true, the guard in emailChangeVerify that holds the flow after the first OTP is verified was skipped entirely due to the !config.Mailer.Autoconfirm condition. This caused the email change to commit after a single confirmation regardless of SecureEmailChangeEnabled. Autoconfirm and SecureEmailChangeEnabled are orthogonal: autoconfirm applies to signup flows, SecureEmailChangeEnabled applies to email change flows. Remove !Autoconfirm from the guard so double_confirm_changes works independently of the autoconfirm setting. Fixes #2600 --- internal/api/verify.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/api/verify.go b/internal/api/verify.go index 212d7388eb..306361d9ea 100644 --- a/internal/api/verify.go +++ b/internal/api/verify.go @@ -544,8 +544,7 @@ 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.Autoconfirm && - config.Mailer.SecureEmailChangeEnabled && + if config.Mailer.SecureEmailChangeEnabled && user.EmailChangeConfirmStatus == zeroConfirmation && user.GetEmail() != "" { err := conn.Transaction(func(tx *storage.Connection) error { From 6264fb2dfdbb5ebf23ab78c89d4ae562ae783350 Mon Sep 17 00:00:00 2001 From: shkuls Date: Mon, 29 Jun 2026 13:01:49 +0530 Subject: [PATCH 2/3] test: verify double_confirm_changes works when mailer_autoconfirm is true Adds a regression test for the bug where SecureEmailChangeEnabled was silently bypassed when Autoconfirm=true. The test asserts that: - after verifying the first token, the email has NOT changed and EmailChangeConfirmStatus is singleConfirmation - only after verifying the second token does the email change commit Covers: https://github.com/supabase/auth/issues/2600 --- internal/api/verify_test.go | 62 +++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/internal/api/verify_test.go b/internal/api/verify_test.go index a75df9c15d..506d64e8bb 100644 --- a/internal/api/verify_test.go +++ b/internal/api/verify_test.go @@ -1141,6 +1141,68 @@ func (ts *VerifyTestSuite) TestSecureEmailChangeWithTokenHash() { } } +// TestSecureEmailChangeNotBypassedByAutoconfirm asserts that +// double_confirm_changes (SecureEmailChangeEnabled) works correctly even when +// mailer_autoconfirm is true. Previously, !Autoconfirm was ANDed into the +// double-confirm guard, causing the email change to commit after a single +// verification regardless of SecureEmailChangeEnabled. +// See: https://github.com/supabase/auth/issues/2600 +func (ts *VerifyTestSuite) TestSecureEmailChangeNotBypassedByAutoconfirm() { + ts.Config.Mailer.SecureEmailChangeEnabled = true + ts.Config.Mailer.Autoconfirm = true + defer func() { ts.Config.Mailer.Autoconfirm = false }() + + u, err := models.FindUserByEmailAndAudience(ts.API.db, "test@example.com", ts.Config.JWT.Aud) + require.NoError(ts.T(), err) + + u.EmailChange = "new@example.com" + currentEmailChangeToken := crypto.GenerateTokenHash(string(u.Email), "123456") + newEmailChangeToken := crypto.GenerateTokenHash(u.EmailChange, "654321") + u.EmailChangeTokenCurrent = currentEmailChangeToken + u.EmailChangeTokenNew = newEmailChangeToken + u.EmailChangeConfirmStatus = zeroConfirmation + currentTime := time.Now() + u.EmailChangeSentAt = ¤tTime + require.NoError(ts.T(), ts.API.db.Update(u)) + require.NoError(ts.T(), models.ClearAllOneTimeTokensForUser(ts.API.db, u.ID)) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, string(u.Email), currentEmailChangeToken, models.EmailChangeTokenCurrent)) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.EmailChange, newEmailChangeToken, models.EmailChangeTokenNew)) + + // Step 1: verify new email — should NOT commit the change yet + var buf bytes.Buffer + require.NoError(ts.T(), json.NewEncoder(&buf).Encode(map[string]interface{}{ + "type": mail.EmailChangeVerification, + "token_hash": newEmailChangeToken, + })) + req := httptest.NewRequest(http.MethodPost, "http://localhost/verify", &buf) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + ts.API.handler.ServeHTTP(w, req) + require.Equal(ts.T(), http.StatusOK, w.Code) + + // Email must still be the old address — change should not have committed yet + u, err = models.FindUserByEmailAndAudience(ts.API.db, "test@example.com", ts.Config.JWT.Aud) + require.NoError(ts.T(), err, "user should still have old email after first verification") + assert.Equal(ts.T(), singleConfirmation, u.EmailChangeConfirmStatus, "status should be singleConfirmation after first verify") + + // Step 2: verify old email — should now commit the change + buf.Reset() + require.NoError(ts.T(), json.NewEncoder(&buf).Encode(map[string]interface{}{ + "type": mail.EmailChangeVerification, + "token_hash": currentEmailChangeToken, + })) + req = httptest.NewRequest(http.MethodPost, "http://localhost/verify", &buf) + req.Header.Set("Content-Type", "application/json") + w = httptest.NewRecorder() + ts.API.handler.ServeHTTP(w, req) + require.Equal(ts.T(), http.StatusOK, w.Code) + + // Email must now be the new address + u, err = models.FindUserByEmailAndAudience(ts.API.db, "new@example.com", ts.Config.JWT.Aud) + require.NoError(ts.T(), err, "user should have new email after both verifications") + assert.Equal(ts.T(), zeroConfirmation, u.EmailChangeConfirmStatus) +} + func (ts *VerifyTestSuite) TestPrepRedirectURL() { escapedMessage := url.QueryEscape(singleConfirmationAccepted) cases := []struct { From 1f61ded183ce5241f39a84ca033e25cee6227278 Mon Sep 17 00:00:00 2001 From: shkuls <132422864+shkuls@users.noreply.github.com> Date: Mon, 29 Jun 2026 17:05:00 +0530 Subject: [PATCH 3/3] Delete outdated TestSecureEmailChange fn comments --- internal/api/verify_test.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/internal/api/verify_test.go b/internal/api/verify_test.go index 506d64e8bb..5b73e6a2c2 100644 --- a/internal/api/verify_test.go +++ b/internal/api/verify_test.go @@ -1141,12 +1141,6 @@ func (ts *VerifyTestSuite) TestSecureEmailChangeWithTokenHash() { } } -// TestSecureEmailChangeNotBypassedByAutoconfirm asserts that -// double_confirm_changes (SecureEmailChangeEnabled) works correctly even when -// mailer_autoconfirm is true. Previously, !Autoconfirm was ANDed into the -// double-confirm guard, causing the email change to commit after a single -// verification regardless of SecureEmailChangeEnabled. -// See: https://github.com/supabase/auth/issues/2600 func (ts *VerifyTestSuite) TestSecureEmailChangeNotBypassedByAutoconfirm() { ts.Config.Mailer.SecureEmailChangeEnabled = true ts.Config.Mailer.Autoconfirm = true