From c0b531ea5bbed4b06121d7214b9c1a0dee272a8f Mon Sep 17 00:00:00 2001 From: Anna Baker Date: Mon, 31 Aug 2026 16:23:47 -0400 Subject: [PATCH 1/5] feat: write expires_at when creating one_time_tokens CreateOneTimeToken now takes the validity window and stores expires_at on insert. Call sites pass Mailer.OtpExp for email flows and Sms.OtpExp for phone flows, matching the windows the verify path infers from users.*_sent_at today. Reads are unchanged. Adds OtpExpAsDuration helpers on the mailer and SMS configurations. Existing tests are updated for the new signature. --- internal/api/admin_test.go | 12 ++++----- internal/api/external_test.go | 4 +-- internal/api/invite_test.go | 2 +- internal/api/mail.go | 24 ++++++++--------- internal/api/phone.go | 6 ++--- internal/api/resend_test.go | 12 ++++----- internal/api/signup_test.go | 2 +- internal/api/verify_test.go | 43 ++++++++++++++++--------------- internal/conf/configuration.go | 8 ++++++ internal/models/one_time_token.go | 12 ++++++++- internal/models/user_test.go | 8 +++--- 11 files changed, 76 insertions(+), 57 deletions(-) diff --git a/internal/api/admin_test.go b/internal/api/admin_test.go index 9ce1eaf103..08c7c31799 100644 --- a/internal/api/admin_test.go +++ b/internal/api/admin_test.go @@ -649,7 +649,7 @@ func (ts *AdminTestSuite) TestAdminUserUpdateClearsPendingTokensOnEmailChange() u.RecoveryToken = recoveryHash u.RecoverySentAt = &now require.NoError(ts.T(), ts.API.db.UpdateOnly(u, "recovery_token", "recovery_sent_at")) - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), recoveryHash, models.RecoveryToken)) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), recoveryHash, models.RecoveryToken, ts.Config.Mailer.OtpExpAsDuration())) // sanity check: the token is redeemable before the email change _, err = models.FindUserByRecoveryToken(ts.API.db, recoveryHash) @@ -877,11 +877,11 @@ func (ts *AdminTestSuite) TestAdminUserSoftDeletion() { "provider": "email", } require.NoError(ts.T(), ts.API.db.Create(u)) - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), u.ConfirmationToken, models.ConfirmationToken)) - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), u.RecoveryToken, models.RecoveryToken)) - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), u.EmailChangeTokenCurrent, models.EmailChangeTokenCurrent)) - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), u.EmailChangeTokenNew, models.EmailChangeTokenNew)) - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetPhone(), u.PhoneChangeToken, models.PhoneChangeToken)) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), u.ConfirmationToken, models.ConfirmationToken, ts.Config.Mailer.OtpExpAsDuration())) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), u.RecoveryToken, models.RecoveryToken, ts.Config.Mailer.OtpExpAsDuration())) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), u.EmailChangeTokenCurrent, models.EmailChangeTokenCurrent, ts.Config.Mailer.OtpExpAsDuration())) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), u.EmailChangeTokenNew, models.EmailChangeTokenNew, ts.Config.Mailer.OtpExpAsDuration())) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetPhone(), u.PhoneChangeToken, models.PhoneChangeToken, ts.Config.Sms.OtpExpAsDuration())) // create user identities _, err = ts.API.createNewIdentity(ts.API.db, u, "email", map[string]interface{}{ diff --git a/internal/api/external_test.go b/internal/api/external_test.go index 62fdaf9d6f..f84862bfd7 100644 --- a/internal/api/external_test.go +++ b/internal/api/external_test.go @@ -110,7 +110,7 @@ func (ts *ExternalTestSuite) createUser(providerId string, email string, name st ts.Require().NoError(ts.API.db.Create(u), "Error creating user") if confirmationToken != "" { - ts.Require().NoError(models.CreateOneTimeToken(ts.API.db, u.ID, email, u.ConfirmationToken, models.ConfirmationToken), "Error creating one-time confirmation/invite token") + ts.Require().NoError(models.CreateOneTimeToken(ts.API.db, u.ID, email, u.ConfirmationToken, models.ConfirmationToken, ts.Config.Mailer.OtpExpAsDuration()), "Error creating one-time confirmation/invite token") } i, err := models.NewIdentity(u, "email", map[string]interface{}{ @@ -142,7 +142,7 @@ func (ts *ExternalTestSuite) createUserWithIdentity(providerType, providerId str ts.Require().NoError(ts.API.db.Create(u), "Error creating user") if confirmationToken != "" { - ts.Require().NoError(models.CreateOneTimeToken(ts.API.db, u.ID, email, u.ConfirmationToken, models.ConfirmationToken), "Error creating one-time confirmation/invite token") + ts.Require().NoError(models.CreateOneTimeToken(ts.API.db, u.ID, email, u.ConfirmationToken, models.ConfirmationToken, ts.Config.Mailer.OtpExpAsDuration()), "Error creating one-time confirmation/invite token") } if email != "" { diff --git a/internal/api/invite_test.go b/internal/api/invite_test.go index bd4fdd6484..9c035dab54 100644 --- a/internal/api/invite_test.go +++ b/internal/api/invite_test.go @@ -263,7 +263,7 @@ func (ts *InviteTestSuite) TestVerifyInvite() { user.ConfirmationToken = crypto.GenerateTokenHash(c.email, c.requestBody["token"].(string)) require.NoError(ts.T(), err) require.NoError(ts.T(), ts.API.db.Create(user)) - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, user.ID, user.GetEmail(), user.ConfirmationToken, models.ConfirmationToken)) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, user.ID, user.GetEmail(), user.ConfirmationToken, models.ConfirmationToken, ts.Config.Mailer.OtpExpAsDuration())) // Find test user _, err = models.FindUserByEmailAndAudience(ts.API.db, c.email, ts.Config.JWT.Aud) diff --git a/internal/api/mail.go b/internal/api/mail.go index 9a42651c42..780a66948e 100644 --- a/internal/api/mail.go +++ b/internal/api/mail.go @@ -155,7 +155,7 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { return terr } - terr = models.CreateOneTimeToken(tx, user.ID, user.GetEmail(), user.RecoveryToken, models.RecoveryToken) + terr = models.CreateOneTimeToken(tx, user.ID, user.GetEmail(), user.RecoveryToken, models.RecoveryToken, config.Mailer.OtpExpAsDuration()) if terr != nil { terr = errors.Wrap(terr, "Database error creating recovery token in admin") return terr @@ -194,7 +194,7 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { terr = errors.Wrap(terr, "Database error updating user for invite") return terr } - terr = models.CreateOneTimeToken(tx, user.ID, user.GetEmail(), user.ConfirmationToken, models.ConfirmationToken) + terr = models.CreateOneTimeToken(tx, user.ID, user.GetEmail(), user.ConfirmationToken, models.ConfirmationToken, config.Mailer.OtpExpAsDuration()) if terr != nil { terr = errors.Wrap(terr, "Database error creating confirmation token for invite in admin") return terr @@ -233,7 +233,7 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { terr = errors.Wrap(terr, "Database error updating user for confirmation") return terr } - terr = models.CreateOneTimeToken(tx, user.ID, user.GetEmail(), user.ConfirmationToken, models.ConfirmationToken) + terr = models.CreateOneTimeToken(tx, user.ID, user.GetEmail(), user.ConfirmationToken, models.ConfirmationToken, config.Mailer.OtpExpAsDuration()) if terr != nil { terr = errors.Wrap(terr, "Database error creating confirmation token for signup in admin") return terr @@ -266,14 +266,14 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { return terr } if user.EmailChangeTokenCurrent != "" { - terr = models.CreateOneTimeToken(tx, user.ID, user.GetEmail(), user.EmailChangeTokenCurrent, models.EmailChangeTokenCurrent) + terr = models.CreateOneTimeToken(tx, user.ID, user.GetEmail(), user.EmailChangeTokenCurrent, models.EmailChangeTokenCurrent, config.Mailer.OtpExpAsDuration()) if terr != nil { terr = errors.Wrap(terr, "Database error creating email change token current in admin") return terr } } if user.EmailChangeTokenNew != "" { - terr = models.CreateOneTimeToken(tx, user.ID, user.EmailChange, user.EmailChangeTokenNew, models.EmailChangeTokenNew) + terr = models.CreateOneTimeToken(tx, user.ID, user.EmailChange, user.EmailChangeTokenNew, models.EmailChangeTokenNew, config.Mailer.OtpExpAsDuration()) if terr != nil { terr = errors.Wrap(terr, "Database error creating email change token new in admin") return terr @@ -349,7 +349,7 @@ func (a *API) sendConfirmation(r *http.Request, tx *storage.Connection, u *model return apierrors.NewInternalServerError("Error sending confirmation email").WithInternalError(errors.Wrap(err, "Database error updating user for confirmation")) } - if err := models.CreateOneTimeToken(tx, u.ID, u.GetEmail(), u.ConfirmationToken, models.ConfirmationToken); err != nil { + if err := models.CreateOneTimeToken(tx, u.ID, u.GetEmail(), u.ConfirmationToken, models.ConfirmationToken, config.Mailer.OtpExpAsDuration()); err != nil { return apierrors.NewInternalServerError("Error sending confirmation email").WithInternalError(errors.Wrap(err, "Database error creating confirmation token")) } @@ -386,7 +386,7 @@ func (a *API) sendInvite(r *http.Request, tx *storage.Connection, u *models.User return apierrors.NewInternalServerError("Error inviting user").WithInternalError(errors.Wrap(err, "Database error updating user for invite")) } - err = models.CreateOneTimeToken(tx, u.ID, u.GetEmail(), u.ConfirmationToken, models.ConfirmationToken) + err = models.CreateOneTimeToken(tx, u.ID, u.GetEmail(), u.ConfirmationToken, models.ConfirmationToken, config.Mailer.OtpExpAsDuration()) if err != nil { return apierrors.NewInternalServerError("Error inviting user").WithInternalError(errors.Wrap(err, "Database error creating confirmation token for invite")) } @@ -428,7 +428,7 @@ func (a *API) sendPasswordRecovery(r *http.Request, tx *storage.Connection, u *m return apierrors.NewInternalServerError("Error sending recovery email").WithInternalError(errors.Wrap(err, "Database error updating user for recovery")) } - if err := models.CreateOneTimeToken(tx, u.ID, u.GetEmail(), u.RecoveryToken, models.RecoveryToken); err != nil { + if err := models.CreateOneTimeToken(tx, u.ID, u.GetEmail(), u.RecoveryToken, models.RecoveryToken, config.Mailer.OtpExpAsDuration()); err != nil { return apierrors.NewInternalServerError("Error sending recovery email").WithInternalError(errors.Wrap(err, "Database error creating recovery token")) } @@ -469,7 +469,7 @@ func (a *API) sendReauthenticationOtp(r *http.Request, tx *storage.Connection, u return apierrors.NewInternalServerError("Error sending reauthentication email").WithInternalError(errors.Wrap(err, "Database error updating user for reauthentication")) } - if err := models.CreateOneTimeToken(tx, u.ID, u.GetEmail(), u.ReauthenticationToken, models.ReauthenticationToken); err != nil { + if err := models.CreateOneTimeToken(tx, u.ID, u.GetEmail(), u.ReauthenticationToken, models.ReauthenticationToken, config.Mailer.OtpExpAsDuration()); err != nil { return apierrors.NewInternalServerError("Error sending reauthentication email").WithInternalError(errors.Wrap(err, "Database error creating reauthentication token")) } @@ -512,7 +512,7 @@ func (a *API) sendMagicLink(r *http.Request, tx *storage.Connection, u *models.U return apierrors.NewInternalServerError("Error sending magic link email").WithInternalError(errors.Wrap(err, "Database error updating user for recovery")) } - if err := models.CreateOneTimeToken(tx, u.ID, u.GetEmail(), u.RecoveryToken, models.RecoveryToken); err != nil { + if err := models.CreateOneTimeToken(tx, u.ID, u.GetEmail(), u.RecoveryToken, models.RecoveryToken, config.Mailer.OtpExpAsDuration()); err != nil { return apierrors.NewInternalServerError("Error sending magic link email").WithInternalError(errors.Wrap(err, "Database error creating recovery token")) } @@ -573,13 +573,13 @@ func (a *API) sendEmailChange(r *http.Request, tx *storage.Connection, u *models } if u.EmailChangeTokenCurrent != "" { - if err := models.CreateOneTimeToken(tx, u.ID, u.GetEmail(), u.EmailChangeTokenCurrent, models.EmailChangeTokenCurrent); err != nil { + if err := models.CreateOneTimeToken(tx, u.ID, u.GetEmail(), u.EmailChangeTokenCurrent, models.EmailChangeTokenCurrent, config.Mailer.OtpExpAsDuration()); err != nil { return apierrors.NewInternalServerError("Error sending email change email").WithInternalError(errors.Wrap(err, "Database error creating email change token current")) } } if u.EmailChangeTokenNew != "" { - if err := models.CreateOneTimeToken(tx, u.ID, u.EmailChange, u.EmailChangeTokenNew, models.EmailChangeTokenNew); err != nil { + if err := models.CreateOneTimeToken(tx, u.ID, u.EmailChange, u.EmailChangeTokenNew, models.EmailChangeTokenNew, config.Mailer.OtpExpAsDuration()); err != nil { return apierrors.NewInternalServerError("Error sending email change email").WithInternalError(errors.Wrap(err, "Database error creating email change token new")) } } diff --git a/internal/api/phone.go b/internal/api/phone.go index 77f46ca294..988270ede7 100644 --- a/internal/api/phone.go +++ b/internal/api/phone.go @@ -142,15 +142,15 @@ func (a *API) sendPhoneConfirmation(r *http.Request, tx *storage.Connection, use var ottErr error switch otpType { case phoneConfirmationOtp: - if err := models.CreateOneTimeToken(tx, user.ID, user.GetPhone(), user.ConfirmationToken, models.ConfirmationToken); err != nil { + if err := models.CreateOneTimeToken(tx, user.ID, user.GetPhone(), user.ConfirmationToken, models.ConfirmationToken, config.Sms.OtpExpAsDuration()); err != nil { ottErr = errors.Wrap(err, "Database error creating confirmation token for phone") } case phoneChangeVerification: - if err := models.CreateOneTimeToken(tx, user.ID, user.PhoneChange, user.PhoneChangeToken, models.PhoneChangeToken); err != nil { + if err := models.CreateOneTimeToken(tx, user.ID, user.PhoneChange, user.PhoneChangeToken, models.PhoneChangeToken, config.Sms.OtpExpAsDuration()); err != nil { ottErr = errors.Wrap(err, "Database error creating phone change token") } case phoneReauthenticationOtp: - if err := models.CreateOneTimeToken(tx, user.ID, user.GetPhone(), user.ReauthenticationToken, models.ReauthenticationToken); err != nil { + if err := models.CreateOneTimeToken(tx, user.ID, user.GetPhone(), user.ReauthenticationToken, models.ReauthenticationToken, config.Sms.OtpExpAsDuration()); err != nil { ottErr = errors.Wrap(err, "Database error creating reauthentication token for phone") } } diff --git a/internal/api/resend_test.go b/internal/api/resend_test.go index 90e556fb40..fe5bdb6258 100644 --- a/internal/api/resend_test.go +++ b/internal/api/resend_test.go @@ -191,8 +191,8 @@ func (ts *ResendTestSuite) TestResendSuccess() { u.EmailChangeSentAt = &now u.EmailChangeTokenNew = "123456" require.NoError(ts.T(), ts.API.db.Create(u), "Error saving new test user") - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), u.ConfirmationToken, models.ConfirmationToken)) - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.EmailChange, u.EmailChangeTokenNew, models.EmailChangeTokenNew)) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), u.ConfirmationToken, models.ConfirmationToken, ts.Config.Mailer.OtpExpAsDuration())) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.EmailChange, u.EmailChangeTokenNew, models.EmailChangeTokenNew, ts.Config.Mailer.OtpExpAsDuration())) phoneUser, err := models.NewUser("1234567890", "", "password", ts.Config.JWT.Aud, nil) require.NoError(ts.T(), err, "Error creating test user model") @@ -200,7 +200,7 @@ func (ts *ResendTestSuite) TestResendSuccess() { phoneUser.EmailChangeSentAt = &now phoneUser.EmailChangeTokenNew = "123456" require.NoError(ts.T(), ts.API.db.Create(phoneUser), "Error saving new test user") - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, phoneUser.ID, phoneUser.EmailChange, phoneUser.EmailChangeTokenNew, models.EmailChangeTokenNew)) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, phoneUser.ID, phoneUser.EmailChange, phoneUser.EmailChangeTokenNew, models.EmailChangeTokenNew, ts.Config.Mailer.OtpExpAsDuration())) emailUser, err := models.NewUser("", "bar@example.com", "password", ts.Config.JWT.Aud, nil) require.NoError(ts.T(), err, "Error creating test user model") @@ -208,7 +208,7 @@ func (ts *ResendTestSuite) TestResendSuccess() { phoneUser.PhoneChangeSentAt = &now phoneUser.PhoneChangeToken = "123456" require.NoError(ts.T(), ts.API.db.Create(emailUser), "Error saving new test user") - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, phoneUser.ID, phoneUser.PhoneChange, phoneUser.PhoneChangeToken, models.PhoneChangeToken)) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, phoneUser.ID, phoneUser.PhoneChange, phoneUser.PhoneChangeToken, models.PhoneChangeToken, ts.Config.Sms.OtpExpAsDuration())) cases := []struct { desc string @@ -292,7 +292,7 @@ func (ts *ResendTestSuite) TestResendPKCESuccess() { signupUser.ConfirmationToken = "oldtoken" signupUser.ConfirmationSentAt = &now require.NoError(ts.T(), ts.API.db.Create(signupUser)) - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, signupUser.ID, signupUser.GetEmail(), signupUser.ConfirmationToken, models.ConfirmationToken)) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, signupUser.ID, signupUser.GetEmail(), signupUser.ConfirmationToken, models.ConfirmationToken, ts.Config.Mailer.OtpExpAsDuration())) // Fresh user for email_change PKCE resend emailChangeUser, err := models.NewUser("", "pkce-change@example.com", "password", ts.Config.JWT.Aud, nil) @@ -301,7 +301,7 @@ func (ts *ResendTestSuite) TestResendPKCESuccess() { emailChangeUser.EmailChangeSentAt = &now emailChangeUser.EmailChangeTokenNew = "oldchangetoken" require.NoError(ts.T(), ts.API.db.Create(emailChangeUser)) - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, emailChangeUser.ID, emailChangeUser.EmailChange, emailChangeUser.EmailChangeTokenNew, models.EmailChangeTokenNew)) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, emailChangeUser.ID, emailChangeUser.EmailChange, emailChangeUser.EmailChangeTokenNew, models.EmailChangeTokenNew, ts.Config.Mailer.OtpExpAsDuration())) ts.Run("Resend signup confirmation with PKCE", func() { var buffer bytes.Buffer diff --git a/internal/api/signup_test.go b/internal/api/signup_test.go index 606de1fe19..00558b1076 100644 --- a/internal/api/signup_test.go +++ b/internal/api/signup_test.go @@ -128,7 +128,7 @@ func (ts *SignupTestSuite) TestVerifySignup() { user.ConfirmationSentAt = &now require.NoError(ts.T(), err) require.NoError(ts.T(), ts.API.db.Create(user)) - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, user.ID, user.GetEmail(), user.ConfirmationToken, models.ConfirmationToken)) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, user.ID, user.GetEmail(), user.ConfirmationToken, models.ConfirmationToken, ts.Config.Mailer.OtpExpAsDuration())) // Find test user u, err := models.FindUserByEmailAndAudience(ts.API.db, "test@example.com", ts.Config.JWT.Aud) diff --git a/internal/api/verify_test.go b/internal/api/verify_test.go index a75df9c15d..2e0d377851 100644 --- a/internal/api/verify_test.go +++ b/internal/api/verify_test.go @@ -303,7 +303,8 @@ func (ts *VerifyTestSuite) TestExpiredConfirmationToken() { sentTime := time.Now().Add(-48 * time.Hour) u.ConfirmationSentAt = &sentTime require.NoError(ts.T(), ts.API.db.Update(u)) - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), u.ConfirmationToken, models.ConfirmationToken)) + // negative duration so expires_at agrees with the already-expired ConfirmationSentAt + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), u.ConfirmationToken, models.ConfirmationToken, -24*time.Hour)) // Setup request reqURL := fmt.Sprintf("http://localhost/verify?type=%s&token=%s", mail.SignupVerification, u.ConfirmationToken) @@ -338,10 +339,10 @@ func (ts *VerifyTestSuite) TestInvalidOtp() { u.EmailChangeTokenNew = "123456" u.EmailChangeTokenCurrent = "123456" require.NoError(ts.T(), ts.API.db.Update(u)) - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), u.ConfirmationToken, models.ConfirmationToken)) - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.PhoneChange, u.PhoneChangeToken, models.PhoneChangeToken)) - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), u.EmailChangeTokenCurrent, models.EmailChangeTokenCurrent)) - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.EmailChange, u.EmailChangeTokenNew, models.EmailChangeTokenNew)) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), u.ConfirmationToken, models.ConfirmationToken, ts.Config.Mailer.OtpExpAsDuration())) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.PhoneChange, u.PhoneChangeToken, models.PhoneChangeToken, ts.Config.Sms.OtpExpAsDuration())) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), u.EmailChangeTokenCurrent, models.EmailChangeTokenCurrent, ts.Config.Mailer.OtpExpAsDuration())) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.EmailChange, u.EmailChangeTokenNew, models.EmailChangeTokenNew, ts.Config.Mailer.OtpExpAsDuration())) type ResponseBody struct { Code int `json:"code"` @@ -695,7 +696,7 @@ func (ts *VerifyTestSuite) TestVerifySignupWithRedirectURLContainedPath() { sendTime := time.Now().Add(time.Hour) u.ConfirmationSentAt = &sendTime require.NoError(ts.T(), ts.API.db.Update(u)) - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), u.ConfirmationToken, models.ConfirmationToken)) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), u.ConfirmationToken, models.ConfirmationToken, ts.Config.Mailer.OtpExpAsDuration())) reqURL := fmt.Sprintf("http://localhost/verify?type=%s&token=%s&redirect_to=%s", "signup", u.ConfirmationToken, redirectURL) req := httptest.NewRequest(http.MethodGet, reqURL, nil) @@ -752,9 +753,9 @@ func (ts *VerifyTestSuite) TestVerifyPKCEOTP() { // since the test user is the same, the tokens are being cleared after each successful verification attempt // so we create them on each run if c.payload.Type == "signup" { - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), c.payload.Token, models.ConfirmationToken)) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), c.payload.Token, models.ConfirmationToken, ts.Config.Mailer.OtpExpAsDuration())) } else if c.payload.Type == "magiclink" { - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), c.payload.Token, models.RecoveryToken)) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), c.payload.Token, models.RecoveryToken, ts.Config.Mailer.OtpExpAsDuration())) } require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(c.payload)) @@ -805,10 +806,10 @@ func (ts *VerifyTestSuite) TestVerifyBannedUser() { t = time.Now().Add(24 * time.Hour) u.BannedUntil = &t require.NoError(ts.T(), ts.API.db.Update(u)) - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), u.ConfirmationToken, models.ConfirmationToken)) - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), u.RecoveryToken, models.RecoveryToken)) - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), u.EmailChangeTokenCurrent, models.EmailChangeTokenCurrent)) - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), u.EmailChangeTokenNew, models.EmailChangeTokenNew)) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), u.ConfirmationToken, models.ConfirmationToken, ts.Config.Mailer.OtpExpAsDuration())) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), u.RecoveryToken, models.RecoveryToken, ts.Config.Mailer.OtpExpAsDuration())) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), u.EmailChangeTokenCurrent, models.EmailChangeTokenCurrent, ts.Config.Mailer.OtpExpAsDuration())) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), u.EmailChangeTokenNew, models.EmailChangeTokenNew, ts.Config.Mailer.OtpExpAsDuration())) cases := []struct { desc string @@ -1039,10 +1040,10 @@ func (ts *VerifyTestSuite) TestVerifyValidOtp() { u.EmailChangeTokenNew = c.expected.tokenHash u.PhoneChangeToken = c.expected.tokenHash - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, "relates_to not used", u.ConfirmationToken, models.ConfirmationToken)) - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, "relates_to not used", u.RecoveryToken, models.RecoveryToken)) - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, "relates_to not used", u.EmailChangeTokenNew, models.EmailChangeTokenNew)) - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, "relates_to not used", u.PhoneChangeToken, models.PhoneChangeToken)) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, "relates_to not used", u.ConfirmationToken, models.ConfirmationToken, ts.Config.Mailer.OtpExpAsDuration())) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, "relates_to not used", u.RecoveryToken, models.RecoveryToken, ts.Config.Mailer.OtpExpAsDuration())) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, "relates_to not used", u.EmailChangeTokenNew, models.EmailChangeTokenNew, ts.Config.Mailer.OtpExpAsDuration())) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, "relates_to not used", u.PhoneChangeToken, models.PhoneChangeToken, ts.Config.Sms.OtpExpAsDuration())) require.NoError(ts.T(), ts.API.db.Update(u)) @@ -1110,8 +1111,8 @@ func (ts *VerifyTestSuite) TestSecureEmailChangeWithTokenHash() { u.EmailChangeTokenNew = newEmailChangeToken require.NoError(ts.T(), models.ClearAllOneTimeTokensForUser(ts.API.db, u.ID)) - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, "relates_to not used", currentEmailChangeToken, models.EmailChangeTokenCurrent)) - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, "relates_to not used", newEmailChangeToken, models.EmailChangeTokenNew)) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, "relates_to not used", currentEmailChangeToken, models.EmailChangeTokenCurrent, ts.Config.Mailer.OtpExpAsDuration())) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, "relates_to not used", newEmailChangeToken, models.EmailChangeTokenNew, ts.Config.Mailer.OtpExpAsDuration())) currentTime := time.Now() u.EmailChangeSentAt = ¤tTime @@ -1467,7 +1468,7 @@ func (ts *VerifyTestSuite) TestVerifyPhoneChangeSendsNotificationEmailEnabled() u.PhoneChangeSentAt = &sentTime u.PhoneChangeToken = expectedTokenHash - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, "relates_to not used", u.PhoneChangeToken, models.PhoneChangeToken)) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, "relates_to not used", u.PhoneChangeToken, models.PhoneChangeToken, ts.Config.Sms.OtpExpAsDuration())) require.NoError(ts.T(), ts.API.db.Update(u)) var buffer bytes.Buffer @@ -1517,7 +1518,7 @@ func (ts *VerifyTestSuite) TestVerifyPhoneChangeSendsNotificationEmailEnabled_No u.PhoneChangeSentAt = &sentTime u.PhoneChangeToken = expectedTokenHash - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, "relates_to not used", u.PhoneChangeToken, models.PhoneChangeToken)) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, "relates_to not used", u.PhoneChangeToken, models.PhoneChangeToken, ts.Config.Sms.OtpExpAsDuration())) require.NoError(ts.T(), ts.API.db.Update(u)) var buffer bytes.Buffer @@ -1564,7 +1565,7 @@ func (ts *VerifyTestSuite) TestVerifyPhoneChangeSendsNotificationEmailDisabled() u.PhoneChangeSentAt = &sentTime u.PhoneChangeToken = expectedTokenHash - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, "relates_to not used", u.PhoneChangeToken, models.PhoneChangeToken)) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, "relates_to not used", u.PhoneChangeToken, models.PhoneChangeToken, ts.Config.Sms.OtpExpAsDuration())) require.NoError(ts.T(), ts.API.db.Update(u)) var buffer bytes.Buffer diff --git a/internal/conf/configuration.go b/internal/conf/configuration.go index 63a8144d29..1c6ea2dcbf 100644 --- a/internal/conf/configuration.go +++ b/internal/conf/configuration.go @@ -695,6 +695,10 @@ type MailerConfiguration struct { blockedMXRecordsVal cachedValue[map[string]bool] `json:"-"` } +func (c *MailerConfiguration) OtpExpAsDuration() time.Duration { + return time.Duration(c.OtpExp) * time.Second +} + func (c *MailerConfiguration) Validate() error { c.serviceHeadersVal = c.buildServiceHeaders() c.blockedMXRecordsVal = c.buildBlockedMXRecords() @@ -784,6 +788,10 @@ type SmsProviderConfiguration struct { Vonage VonageProviderConfiguration `json:"vonage"` } +func (c *SmsProviderConfiguration) OtpExpAsDuration() time.Duration { + return time.Duration(c.OtpExp) * time.Second +} + func (c *SmsProviderConfiguration) GetTestOTP(phone string, now time.Time) (string, bool) { if c.TestOTP != nil && (c.TestOTPValidUntil.Time.IsZero() || now.Before(c.TestOTPValidUntil.Time)) { testOTP, ok := c.TestOTP[phone] diff --git a/internal/models/one_time_token.go b/internal/models/one_time_token.go index 34e4309f3f..ce1dc660d9 100644 --- a/internal/models/one_time_token.go +++ b/internal/models/one_time_token.go @@ -114,6 +114,8 @@ type OneTimeToken struct { CreatedAt time.Time `json:"created_at" db:"created_at"` UpdatedAt time.Time `json:"updated_at" db:"updated_at"` + + ExpiresAt *time.Time `json:"expires_at" db:"expires_at"` } func (OneTimeToken) TableName() string { @@ -132,17 +134,25 @@ func ClearOneTimeTokenForUser(tx *storage.Connection, userID uuid.UUID, tokenTyp return nil } -func CreateOneTimeToken(tx *storage.Connection, userID uuid.UUID, relatesTo, tokenHash string, tokenType OneTimeTokenType) error { +func CreateOneTimeToken( + tx *storage.Connection, + userID uuid.UUID, + relatesTo, tokenHash string, + tokenType OneTimeTokenType, + validityDuration time.Duration) error { if err := ClearOneTimeTokenForUser(tx, userID, tokenType); err != nil { return err } + expiresAt := time.Now().Add(validityDuration) + oneTimeToken := &OneTimeToken{ ID: uuid.Must(uuid.NewV4()), UserID: userID, TokenType: tokenType, TokenHash: tokenHash, RelatesTo: strings.ToLower(relatesTo), + ExpiresAt: &expiresAt, } if err := tx.Eager().Create(oneTimeToken); err != nil { diff --git a/internal/models/user_test.go b/internal/models/user_test.go index 9e6e3cb43c..d76adbf1e8 100644 --- a/internal/models/user_test.go +++ b/internal/models/user_test.go @@ -90,7 +90,7 @@ func (ts *UserTestSuite) TestUpdateUserMetadata() { func (ts *UserTestSuite) TestFindUserByConfirmationToken() { u := ts.createUser() tokenHash := "test_confirmation_token" - require.NoError(ts.T(), CreateOneTimeToken(ts.db, u.ID, "relates_to not used", tokenHash, ConfirmationToken)) + require.NoError(ts.T(), CreateOneTimeToken(ts.db, u.ID, "relates_to not used", tokenHash, ConfirmationToken, ts.config.Mailer.OtpExpAsDuration())) n, err := FindUserByConfirmationToken(ts.db, tokenHash) require.NoError(ts.T(), err) @@ -270,7 +270,7 @@ func (ts *UserTestSuite) TestFindUserByID() { func (ts *UserTestSuite) TestFindUserByRecoveryToken() { u := ts.createUser() tokenHash := "test_recovery_token" - require.NoError(ts.T(), CreateOneTimeToken(ts.db, u.ID, "relates_to not used", tokenHash, RecoveryToken)) + require.NoError(ts.T(), CreateOneTimeToken(ts.db, u.ID, "relates_to not used", tokenHash, RecoveryToken, ts.config.Mailer.OtpExpAsDuration())) n, err := FindUserByRecoveryToken(ts.db, tokenHash) require.NoError(ts.T(), err) @@ -677,7 +677,7 @@ func (ts *UserTestSuite) TestUpdateUserEmailClearsStaleTokens() { "reauthentication_token", "reauthentication_sent_at", )) - require.NoError(ts.T(), CreateOneTimeToken(ts.db, userA.ID, userA.GetEmail(), userA.ConfirmationToken, ConfirmationToken)) + require.NoError(ts.T(), CreateOneTimeToken(ts.db, userA.ID, userA.GetEmail(), userA.ConfirmationToken, ConfirmationToken, ts.config.Mailer.OtpExpAsDuration())) // promoting another identity's email must revoke every outstanding // token addressed to the previous email @@ -731,7 +731,7 @@ func (ts *UserTestSuite) TestUpdateUserEmailFromEmptyClearsStaleTokens() { "phone_change_token", "phone_change_sent_at", )) - require.NoError(ts.T(), CreateOneTimeToken(ts.db, userA.ID, userA.PhoneChange, userA.PhoneChangeToken, PhoneChangeToken)) + require.NoError(ts.T(), CreateOneTimeToken(ts.db, userA.ID, userA.PhoneChange, userA.PhoneChangeToken, PhoneChangeToken, ts.config.Sms.OtpExpAsDuration())) // promoting an identity's email over an empty one is still a primary // email transition, so outstanding tokens must be revoked From 753094d88f9f8bcc592bbcde9ea08d2fae3de71d Mon Sep 17 00:00:00 2001 From: Anna Baker Date: Mon, 31 Aug 2026 16:23:52 -0400 Subject: [PATCH 2/5] chore: add tests asserting expires_at matches the validity window Add a models suite covering persistence, past windows, and resend replacement, and an api suite asserting expires_at equals sent_at + OtpExp per channel. The api suite pins Mailer.OtpExp and Sms.OtpExp to different values so a swapped config field fails the test. --- internal/api/one_time_token_expiry_test.go | 90 ++++++++++++++++++ internal/models/one_time_token_test.go | 101 +++++++++++++++++++++ 2 files changed, 191 insertions(+) create mode 100644 internal/api/one_time_token_expiry_test.go create mode 100644 internal/models/one_time_token_test.go diff --git a/internal/api/one_time_token_expiry_test.go b/internal/api/one_time_token_expiry_test.go new file mode 100644 index 0000000000..ad39c4f5e8 --- /dev/null +++ b/internal/api/one_time_token_expiry_test.go @@ -0,0 +1,90 @@ +package api + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" + "github.com/supabase/auth/internal/api/sms_provider" + "github.com/supabase/auth/internal/conf" + "github.com/supabase/auth/internal/models" +) + +type OneTimeTokenExpiryTestSuite struct { + suite.Suite + API *API + Config *conf.GlobalConfiguration +} + +func TestOneTimeTokenExpiry(t *testing.T) { + api, config, err := setupAPIForTest() + require.NoError(t, err) + + ts := &OneTimeTokenExpiryTestSuite{API: api, Config: config} + defer api.db.Close() + + suite.Run(t, ts) +} + +func (ts *OneTimeTokenExpiryTestSuite) SetupTest() { + models.TruncateAll(ts.API.db) + + // Two different windows to ensure we're using the right config values + ts.Config.Mailer.OtpExp = 3600 + ts.Config.Sms.OtpExp = 60 +} + +func (ts *OneTimeTokenExpiryTestSuite) TestEmailTokenMatchesMailerWindow() { + u, err := models.NewUser("", "otp-exp@example.com", "password", ts.Config.JWT.Aud, nil) + require.NoError(ts.T(), err) + require.NoError(ts.T(), ts.API.db.Create(u)) + + var buffer bytes.Buffer + require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{ + "email": "otp-exp@example.com", + })) + req := httptest.NewRequest(http.MethodPost, "http://localhost/recover", &buffer) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + ts.API.handler.ServeHTTP(w, req) + require.Equal(ts.T(), http.StatusOK, w.Code) + + u, err = models.FindUserByID(ts.API.db, u.ID) + require.NoError(ts.T(), err) + + ott, err := models.FindOneTimeToken(ts.API.db, u.RecoveryToken, models.RecoveryToken) + require.NoError(ts.T(), err) + require.NotNil(ts.T(), ott.ExpiresAt) + + want := u.RecoverySentAt.Add(ts.Config.Mailer.OtpExpAsDuration()) + require.WithinDuration(ts.T(), want, *ott.ExpiresAt, 2*time.Second) +} + +func (ts *OneTimeTokenExpiryTestSuite) TestPhoneTokenMatchesSmsWindow() { + u, err := models.NewUser("123456789", "", "password", ts.Config.JWT.Aud, nil) + require.NoError(ts.T(), err) + require.NoError(ts.T(), ts.API.db.Create(u)) + + sms_provider.MockProvider = &TestSmsProvider{} + defer func() { sms_provider.MockProvider = nil }() + + req, err := http.NewRequest(http.MethodPost, "http://localhost/otp", nil) + require.NoError(ts.T(), err) + _, err = ts.API.sendPhoneConfirmation(req, ts.API.db, u, "123456789", phoneConfirmationOtp, sms_provider.SMSProvider) + require.NoError(ts.T(), err) + + u, err = models.FindUserByID(ts.API.db, u.ID) + require.NoError(ts.T(), err) + + ott, err := models.FindOneTimeToken(ts.API.db, u.ConfirmationToken, models.ConfirmationToken) + require.NoError(ts.T(), err) + require.NotNil(ts.T(), ott.ExpiresAt) + + want := u.ConfirmationSentAt.Add(ts.Config.Sms.OtpExpAsDuration()) + require.WithinDuration(ts.T(), want, *ott.ExpiresAt, 2*time.Second) +} diff --git a/internal/models/one_time_token_test.go b/internal/models/one_time_token_test.go new file mode 100644 index 0000000000..f4f430e97b --- /dev/null +++ b/internal/models/one_time_token_test.go @@ -0,0 +1,101 @@ +package models + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" + "github.com/supabase/auth/internal/conf" + "github.com/supabase/auth/internal/conf/confload" + "github.com/supabase/auth/internal/storage" + "github.com/supabase/auth/internal/storage/test" +) + +// The caller chooses the validity window, so this suite asserts only what this +// layer owns: the value survives the round trip, and a resend replaces it. The +// per-flow window choice is asserted in the api package. +type OneTimeTokenTestSuite struct { + suite.Suite + db *storage.Connection + config *conf.GlobalConfiguration +} + +func TestOneTimeToken(t *testing.T) { + globalConfig, err := confload.LoadGlobal(modelsTestConfig) + require.NoError(t, err) + + conn, err := test.SetupDBConnection(globalConfig) + require.NoError(t, err) + + ts := &OneTimeTokenTestSuite{ + db: conn, + config: globalConfig, + } + defer ts.db.Close() + + suite.Run(t, ts) +} + +func (ts *OneTimeTokenTestSuite) SetupTest() { + TruncateAll(ts.db) +} + +func (ts *OneTimeTokenTestSuite) createUser() *User { + u, err := NewUser("", "test@example.com", "password", ts.config.JWT.Aud, nil) + require.NoError(ts.T(), err, "Error creating test user model") + require.NoError(ts.T(), ts.db.Create(u), "Error saving new test user") + return u +} + +func (ts *OneTimeTokenTestSuite) TestCreateOneTimeTokenPersistsExpiresAt() { + u := ts.createUser() + const validity = 15 * time.Minute + + before := time.Now() + require.NoError(ts.T(), CreateOneTimeToken(ts.db, u.ID, u.GetEmail(), "token-hash", ConfirmationToken, validity)) + after := time.Now() + + ott, err := FindOneTimeToken(ts.db, "token-hash", ConfirmationToken) + require.NoError(ts.T(), err) + require.NotNil(ts.T(), ott.ExpiresAt) + + require.False(ts.T(), ott.ExpiresAt.Before(before.Add(validity)), + "expires_at %s precedes the window opened at %s", ott.ExpiresAt, before.Add(validity)) + require.False(ts.T(), ott.ExpiresAt.After(after.Add(validity)), + "expires_at %s follows the window closed at %s", ott.ExpiresAt, after.Add(validity)) +} + +// CreateOneTimeToken neither validates nor clamps the window; the caller owns +// it. The api verify tests rely on this to build expired tokens. +func (ts *OneTimeTokenTestSuite) TestCreateOneTimeTokenAcceptsPastWindow() { + u := ts.createUser() + + require.NoError(ts.T(), CreateOneTimeToken(ts.db, u.ID, u.GetEmail(), "stale-hash", ConfirmationToken, -24*time.Hour)) + + ott, err := FindOneTimeToken(ts.db, "stale-hash", ConfirmationToken) + require.NoError(ts.T(), err) + require.NotNil(ts.T(), ott.ExpiresAt) + require.True(ts.T(), ott.ExpiresAt.Before(time.Now()), + "expected an already-expired token, got expires_at %s", ott.ExpiresAt) +} + +func (ts *OneTimeTokenTestSuite) TestCreateOneTimeTokenResendReplacesWindow() { + u := ts.createUser() + + require.NoError(ts.T(), CreateOneTimeToken(ts.db, u.ID, u.GetEmail(), "first-hash", ConfirmationToken, time.Minute)) + first, err := FindOneTimeToken(ts.db, "first-hash", ConfirmationToken) + require.NoError(ts.T(), err) + require.NotNil(ts.T(), first.ExpiresAt) + + require.NoError(ts.T(), CreateOneTimeToken(ts.db, u.ID, u.GetEmail(), "second-hash", ConfirmationToken, time.Hour)) + + _, err = FindOneTimeToken(ts.db, "first-hash", ConfirmationToken) + require.True(ts.T(), IsNotFoundError(err), "resend must clear the previous token, got %v", err) + + second, err := FindOneTimeToken(ts.db, "second-hash", ConfirmationToken) + require.NoError(ts.T(), err) + require.NotNil(ts.T(), second.ExpiresAt) + require.True(ts.T(), second.ExpiresAt.After(*first.ExpiresAt), + "resend must move expires_at forward, first=%s second=%s", first.ExpiresAt, second.ExpiresAt) +} From 1e3ea6e71517f806357ba8b575f4df33eef83e7d Mon Sep 17 00:00:00 2001 From: Anna Baker Date: Mon, 31 Aug 2026 16:49:48 -0400 Subject: [PATCH 3/5] chore(otp): cleanup tests --- internal/models/one_time_token_test.go | 50 +++++++++++++------------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/internal/models/one_time_token_test.go b/internal/models/one_time_token_test.go index f4f430e97b..8664ab579b 100644 --- a/internal/models/one_time_token_test.go +++ b/internal/models/one_time_token_test.go @@ -48,36 +48,34 @@ func (ts *OneTimeTokenTestSuite) createUser() *User { return u } -func (ts *OneTimeTokenTestSuite) TestCreateOneTimeTokenPersistsExpiresAt() { - u := ts.createUser() - const validity = 15 * time.Minute +func (ts *OneTimeTokenTestSuite) TestCreateOneTimeToken() { + cases := map[string]time.Duration{ + "future window": 15 * time.Minute, + // CreateOneTimeToken neither validates nor clamps the window. + // The caller owns it. The api verify tests rely on this to build + // expired tokens. + "past window": -24 * time.Hour, + } - before := time.Now() - require.NoError(ts.T(), CreateOneTimeToken(ts.db, u.ID, u.GetEmail(), "token-hash", ConfirmationToken, validity)) - after := time.Now() + for name, validity := range cases { + ts.Run(name, func() { + TruncateAll(ts.db) + u := ts.createUser() - ott, err := FindOneTimeToken(ts.db, "token-hash", ConfirmationToken) - require.NoError(ts.T(), err) - require.NotNil(ts.T(), ott.ExpiresAt) + before := time.Now() + require.NoError(ts.T(), CreateOneTimeToken(ts.db, u.ID, u.GetEmail(), name, ConfirmationToken, validity)) + after := time.Now() - require.False(ts.T(), ott.ExpiresAt.Before(before.Add(validity)), - "expires_at %s precedes the window opened at %s", ott.ExpiresAt, before.Add(validity)) - require.False(ts.T(), ott.ExpiresAt.After(after.Add(validity)), - "expires_at %s follows the window closed at %s", ott.ExpiresAt, after.Add(validity)) -} + ott, err := FindOneTimeToken(ts.db, name, ConfirmationToken) + require.NoError(ts.T(), err) + require.NotNil(ts.T(), ott.ExpiresAt) -// CreateOneTimeToken neither validates nor clamps the window; the caller owns -// it. The api verify tests rely on this to build expired tokens. -func (ts *OneTimeTokenTestSuite) TestCreateOneTimeTokenAcceptsPastWindow() { - u := ts.createUser() - - require.NoError(ts.T(), CreateOneTimeToken(ts.db, u.ID, u.GetEmail(), "stale-hash", ConfirmationToken, -24*time.Hour)) - - ott, err := FindOneTimeToken(ts.db, "stale-hash", ConfirmationToken) - require.NoError(ts.T(), err) - require.NotNil(ts.T(), ott.ExpiresAt) - require.True(ts.T(), ott.ExpiresAt.Before(time.Now()), - "expected an already-expired token, got expires_at %s", ott.ExpiresAt) + require.False(ts.T(), ott.ExpiresAt.Before(before.Add(validity)), + "expires_at %s precedes the window opened at %s", ott.ExpiresAt, before.Add(validity)) + require.False(ts.T(), ott.ExpiresAt.After(after.Add(validity)), + "expires_at %s follows the window closed at %s", ott.ExpiresAt, after.Add(validity)) + }) + } } func (ts *OneTimeTokenTestSuite) TestCreateOneTimeTokenResendReplacesWindow() { From fcaf26e13065b6d95416c4c5c3b6ae8746d9ac50 Mon Sep 17 00:00:00 2001 From: Anna Baker Date: Mon, 31 Aug 2026 17:05:42 -0400 Subject: [PATCH 4/5] chore: fix ci tests by adding no-gosec --- internal/conf/configuration.go | 4 ++-- internal/models/one_time_token_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/conf/configuration.go b/internal/conf/configuration.go index 1c6ea2dcbf..7484c1a610 100644 --- a/internal/conf/configuration.go +++ b/internal/conf/configuration.go @@ -696,7 +696,7 @@ type MailerConfiguration struct { } func (c *MailerConfiguration) OtpExpAsDuration() time.Duration { - return time.Duration(c.OtpExp) * time.Second + return time.Duration(c.OtpExp) * time.Second // #nosec G115 -- OtpExp comes from trusted config, not user input } func (c *MailerConfiguration) Validate() error { @@ -789,7 +789,7 @@ type SmsProviderConfiguration struct { } func (c *SmsProviderConfiguration) OtpExpAsDuration() time.Duration { - return time.Duration(c.OtpExp) * time.Second + return time.Duration(c.OtpExp) * time.Second // #nosec G115 -- OtpExp comes from trusted config, not user input } func (c *SmsProviderConfiguration) GetTestOTP(phone string, now time.Time) (string, bool) { diff --git a/internal/models/one_time_token_test.go b/internal/models/one_time_token_test.go index 8664ab579b..929998e763 100644 --- a/internal/models/one_time_token_test.go +++ b/internal/models/one_time_token_test.go @@ -51,7 +51,7 @@ func (ts *OneTimeTokenTestSuite) createUser() *User { func (ts *OneTimeTokenTestSuite) TestCreateOneTimeToken() { cases := map[string]time.Duration{ "future window": 15 * time.Minute, - // CreateOneTimeToken neither validates nor clamps the window. + // CreateOneTimeToken neither validates nor clamps the window. // The caller owns it. The api verify tests rely on this to build // expired tokens. "past window": -24 * time.Hour, From 78044409a2eee198226121470612f0f41799108d Mon Sep 17 00:00:00 2001 From: Anna Baker Date: Thu, 3 Sep 2026 16:02:28 -0400 Subject: [PATCH 5/5] test(otp): pass validity duration to CreateOneTimeToken in user test The test came in from master via merge after this branch added the validityDuration parameter, so the models package did not compile. Co-Authored-By: Claude Sonnet 5 --- internal/models/user_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/models/user_test.go b/internal/models/user_test.go index 0432b875a5..f751cdcc0b 100644 --- a/internal/models/user_test.go +++ b/internal/models/user_test.go @@ -285,7 +285,7 @@ func (ts *UserTestSuite) TestFindUserByRecoveryToken() { func (ts *UserTestSuite) TestFindUserByOneTimeTokenMultipleTypes() { u := ts.createUser() tokenHash := "test_confirmation_or_recovery_token" - require.NoError(ts.T(), CreateOneTimeToken(ts.db, u.ID, "relates_to not used", tokenHash, RecoveryToken)) + require.NoError(ts.T(), CreateOneTimeToken(ts.db, u.ID, "relates_to not used", tokenHash, RecoveryToken, time.Minute)) n, err := FindUserByOneTimeToken(ts.db, tokenHash, ConfirmationToken, RecoveryToken) require.NoError(ts.T(), err)