From aa9f137cc9c7225b9b603e95562610ce415ba1b4 Mon Sep 17 00:00:00 2001 From: deepshekhardas Date: Tue, 18 Aug 2026 19:42:25 +0530 Subject: [PATCH] fix(verify): honor double-confirm email change when mailer_autoconfirm is enabled (fixes #2600) --- internal/api/verify.go | 3 +- internal/api/verify_test.go | 77 +++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 2 deletions(-) diff --git a/internal/api/verify.go b/internal/api/verify.go index c0b58e4a68..1f7639b68c 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 { diff --git a/internal/api/verify_test.go b/internal/api/verify_test.go index a75df9c15d..64bf2b9f10 100644 --- a/internal/api/verify_test.go +++ b/internal/api/verify_test.go @@ -293,6 +293,83 @@ func (ts *VerifyTestSuite) TestVerifySecureEmailChange() { } } +func (ts *VerifyTestSuite) TestVerifySecureEmailChangeAutoconfirmEnabled() { + // double-confirm must still hold when mailer_autoconfirm is enabled + ts.Config.Mailer.Autoconfirm = true + ts.Config.Mailer.SecureEmailChangeEnabled = true + defer func() { + ts.Config.Mailer.Autoconfirm = false + ts.Config.Mailer.SecureEmailChangeEnabled = false + }() + + currentEmail := "test@example.com" + newEmail := "new@example.com" + + u, err := models.FindUserByEmailAndAudience(ts.API.db, currentEmail, ts.Config.JWT.Aud) + require.NoError(ts.T(), err) + + u.EmailChangeSentAt = nil + u.EmailChangeTokenCurrent = "" + u.EmailChangeTokenNew = "" + require.NoError(ts.T(), ts.API.db.Update(u)) + require.NoError(ts.T(), models.ClearAllOneTimeTokensForUser(ts.API.db, u.ID)) + + var buffer bytes.Buffer + require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{ + "email": newEmail, + })) + + req := httptest.NewRequest(http.MethodPut, "http://localhost/user", &buffer) + req.Header.Set("Content-Type", "application/json") + + session, err := models.NewSession(u.ID, nil) + require.NoError(ts.T(), err) + require.NoError(ts.T(), ts.API.db.Create(session)) + + token, _, err := ts.API.generateAccessToken(req, ts.API.db, u, &session.ID, models.MagicLink) + require.NoError(ts.T(), err) + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token)) + + w := httptest.NewRecorder() + ts.API.handler.ServeHTTP(w, req) + assert.Equal(ts.T(), http.StatusOK, w.Code) + + u, err = models.FindUserByEmailAndAudience(ts.API.db, currentEmail, ts.Config.JWT.Aud) + require.NoError(ts.T(), err) + + currentTokenHash := u.EmailChangeTokenCurrent + newTokenHash := u.EmailChangeTokenNew + + // Verify new email: should NOT commit the change yet + reqURL := fmt.Sprintf("http://localhost/verify?type=%s&token=%s", mail.EmailChangeVerification, newTokenHash) + req = httptest.NewRequest(http.MethodGet, reqURL, nil) + + w = httptest.NewRecorder() + ts.API.handler.ServeHTTP(w, req) + require.Equal(ts.T(), http.StatusSeeOther, w.Code) + + u, err = models.FindUserByEmailAndAudience(ts.API.db, currentEmail, ts.Config.JWT.Aud) + require.NoError(ts.T(), err) + assert.Equal(ts.T(), singleConfirmation, u.EmailChangeConfirmStatus) + assert.Equal(ts.T(), currentEmail, u.GetEmail()) + + // Verify old email: commits the change + reqURL = fmt.Sprintf("http://localhost/verify?type=%s&token=%s", mail.EmailChangeVerification, currentTokenHash) + req = httptest.NewRequest(http.MethodGet, reqURL, nil) + + w = httptest.NewRecorder() + ts.API.handler.ServeHTTP(w, req) + require.Equal(ts.T(), http.StatusSeeOther, w.Code) + + u, err = models.FindUserByEmailAndAudience(ts.API.db, newEmail, ts.Config.JWT.Aud) + require.NoError(ts.T(), err) + require.Equal(ts.T(), zeroConfirmation, u.EmailChangeConfirmStatus) + + // Reset for other tests + u.EmailConfirmedAt = nil + require.NoError(ts.T(), ts.API.db.Update(u)) +} + func (ts *VerifyTestSuite) TestExpiredConfirmationToken() { // verify variant testing not necessary in this test as it's testing // the ConfirmationSentAt behavior, not the ConfirmationToken behavior