diff --git a/hack/test.env b/hack/test.env index 35e4b61c8..3e57e595a 100644 --- a/hack/test.env +++ b/hack/test.env @@ -11,10 +11,11 @@ GOTRUE_API_HOST=localhost PORT=9999 API_EXTERNAL_URL="http://localhost:9999" GOTRUE_LOG_SQL=none -GOTRUE_LOG_LEVEL=warn +GOTRUE_LOG_LEVEL=info GOTRUE_SITE_URL=https://example.netlify.com GOTRUE_URI_ALLOW_LIST="http://localhost:3000" GOTRUE_OPERATOR_TOKEN=foobar +GOTRUE_API_MAX_REQUEST_DURATION="0" GOTRUE_EXTERNAL_APPLE_ENABLED=true GOTRUE_EXTERNAL_APPLE_CLIENT_ID=testclientid GOTRUE_EXTERNAL_APPLE_SECRET=testsecret diff --git a/internal/api/admin_test.go b/internal/api/admin_test.go index e1b2c0328..1a8602607 100644 --- a/internal/api/admin_test.go +++ b/internal/api/admin_test.go @@ -628,20 +628,20 @@ func (ts *AdminTestSuite) TestAdminUserSoftDeletion() { // create user u, err := models.NewUser("123456789", "test@example.com", "secret", ts.Config.JWT.Aud, map[string]interface{}{"name": "test"}) require.NoError(ts.T(), err) - u.ConfirmationToken = "some_token" - u.RecoveryToken = "some_token" - u.EmailChangeTokenCurrent = "some_token" - u.EmailChangeTokenNew = "some_token" - u.PhoneChangeToken = "some_token" u.AppMetaData = map[string]interface{}{ "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)) + _, err = models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), "some_token", models.ConfirmationToken) + require.NoError(ts.T(), err) + _, err = models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), "some_token", models.RecoveryToken) + require.NoError(ts.T(), err) + _, err = models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), "some_token", models.EmailChangeTokenCurrent) + require.NoError(ts.T(), err) + _, err = models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), "some_token", models.EmailChangeTokenNew) + require.NoError(ts.T(), err) + _, err = models.CreateOneTimeToken(ts.API.db, u.ID, u.GetPhone(), "some_token", models.PhoneChangeToken) + require.NoError(ts.T(), err) // create user identities _, err = ts.API.createNewIdentity(ts.API.db, u, "email", map[string]interface{}{ @@ -670,13 +670,10 @@ func (ts *AdminTestSuite) TestAdminUserSoftDeletion() { // get soft-deleted user from db deletedUser, err := models.FindUserByID(ts.API.db, u.ID) require.NoError(ts.T(), err) - - require.Empty(ts.T(), deletedUser.ConfirmationToken) - require.Empty(ts.T(), deletedUser.RecoveryToken) - require.Empty(ts.T(), deletedUser.EmailChangeTokenCurrent) - require.Empty(ts.T(), deletedUser.EmailChangeTokenNew) require.Empty(ts.T(), deletedUser.EncryptedPassword) - require.Empty(ts.T(), deletedUser.PhoneChangeToken) + + // one time tokens table should be empty after a soft deletion + require.Empty(ts.T(), u.OneTimeTokens) require.Empty(ts.T(), deletedUser.UserMetaData) require.Empty(ts.T(), deletedUser.AppMetaData) require.NotEmpty(ts.T(), deletedUser.DeletedAt) diff --git a/internal/api/anonymous_test.go b/internal/api/anonymous_test.go index 6ab2bae04..5befccb48 100644 --- a/internal/api/anonymous_test.go +++ b/internal/api/anonymous_test.go @@ -144,8 +144,9 @@ func (ts *AnonymousTestSuite) TestConvertAnonymousUserToPermanent() { switch c.verificationType { case mail.EmailChangeVerification: + emailChangeToken := user.OneTimeTokens[0].TokenHash require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{ - "token_hash": user.EmailChangeTokenNew, + "token_hash": emailChangeToken, "type": c.verificationType, })) case phoneChangeVerification: diff --git a/internal/api/api_test.go b/internal/api/api_test.go index 87639a09c..c1ca6a120 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/require" "github.com/supabase/auth/internal/conf" "github.com/supabase/auth/internal/crypto" + "github.com/supabase/auth/internal/observability" "github.com/supabase/auth/internal/storage" "github.com/supabase/auth/internal/storage/test" ) @@ -32,6 +33,10 @@ func setupAPIForTestWithCallback(cb func(*conf.GlobalConfiguration, *storage.Con return nil, nil, err } + if err := observability.ConfigureLogging(&config.Logging); err != nil { + return nil, nil, err + } + if cb != nil { cb(config, nil) } diff --git a/internal/api/external_test.go b/internal/api/external_test.go index 09fdcc433..1e1f24a83 100644 --- a/internal/api/external_test.go +++ b/internal/api/external_test.go @@ -49,15 +49,12 @@ func (ts *ExternalTestSuite) createUser(providerId string, email string, name st userData["avatar_url"] = avatar } u, err := models.NewUser("", email, "test", ts.Config.JWT.Aud, userData) - - if confirmationToken != "" { - u.ConfirmationToken = confirmationToken - } ts.Require().NoError(err, "Error making new user") 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") + _, err = models.CreateOneTimeToken(ts.API.db, u.ID, email, confirmationToken, models.ConfirmationToken) + ts.Require().NoError(err, "Error creating one-time confirmation/invite token") } i, err := models.NewIdentity(u, "email", map[string]interface{}{ diff --git a/internal/api/invite_test.go b/internal/api/invite_test.go index ff0baca6a..a31e99339 100644 --- a/internal/api/invite_test.go +++ b/internal/api/invite_test.go @@ -208,10 +208,10 @@ func (ts *InviteTestSuite) TestVerifyInvite() { user.InvitedAt = &now user.ConfirmationSentAt = &now user.EncryptedPassword = nil - 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)) + _, err = models.CreateOneTimeToken(ts.API.db, user.ID, user.GetEmail(), crypto.GenerateTokenHash(c.email, c.requestBody["token"].(string)), models.ConfirmationToken) + require.NoError(ts.T(), err) // Find test user _, err = models.FindUserByEmailAndAudience(ts.API.db, c.email, ts.Config.JWT.Aud) @@ -279,9 +279,10 @@ func (ts *InviteTestSuite) TestInviteExternalGitlab() { // Find test user user, err := models.FindUserByEmailAndAudience(ts.API.db, "gitlab@example.com", ts.Config.JWT.Aud) require.NoError(ts.T(), err) + inviteToken := user.OneTimeTokens[0].TokenHash // get redirect url w/ state - req = httptest.NewRequest(http.MethodGet, "http://localhost/authorize?provider=gitlab&invite_token="+user.ConfirmationToken, nil) + req = httptest.NewRequest(http.MethodGet, "http://localhost/authorize?provider=gitlab&invite_token="+inviteToken, nil) w = httptest.NewRecorder() ts.API.handler.ServeHTTP(w, req) ts.Require().Equal(http.StatusFound, w.Code) @@ -371,9 +372,10 @@ func (ts *InviteTestSuite) TestInviteExternalGitlab_MismatchedEmails() { // Find test user user, err := models.FindUserByEmailAndAudience(ts.API.db, "gitlab@example.com", ts.Config.JWT.Aud) require.NoError(ts.T(), err) + inviteToken := user.OneTimeTokens[0].TokenHash // get redirect url w/ state - req = httptest.NewRequest(http.MethodGet, "http://localhost/authorize?provider=gitlab&invite_token="+user.ConfirmationToken, nil) + req = httptest.NewRequest(http.MethodGet, "http://localhost/authorize?provider=gitlab&invite_token="+inviteToken, nil) w = httptest.NewRecorder() ts.API.handler.ServeHTTP(w, req) ts.Require().Equal(http.StatusFound, w.Code) diff --git a/internal/api/mail.go b/internal/api/mail.go index 30f358ad2..ee3a0e70d 100644 --- a/internal/api/mail.go +++ b/internal/api/mail.go @@ -7,6 +7,7 @@ import ( "github.com/supabase/auth/internal/hooks" mail "github.com/supabase/auth/internal/mailer" + "golang.org/x/sync/errgroup" "github.com/badoux/checkmail" "github.com/fatih/structs" @@ -112,6 +113,7 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { } } + var ott *models.OneTimeToken err = db.Transaction(func(tx *storage.Connection) error { var terr error switch params.Type { @@ -119,15 +121,14 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { if terr = models.NewAuditLogEntry(r, tx, user, models.UserRecoveryRequestedAction, "", nil); terr != nil { return terr } - user.RecoveryToken = hashedToken user.RecoverySentAt = &now - terr = tx.UpdateOnly(user, "recovery_token", "recovery_sent_at") + terr = tx.UpdateOnly(user, "recovery_sent_at") if terr != nil { terr = errors.Wrap(terr, "Database error updating user for recovery") return terr } - terr = models.CreateOneTimeToken(tx, user.ID, user.GetEmail(), user.RecoveryToken, models.RecoveryToken) + ott, terr = models.CreateOneTimeToken(tx, user.ID, user.GetEmail(), hashedToken, models.RecoveryToken) if terr != nil { terr = errors.Wrap(terr, "Database error creating recovery token in admin") return terr @@ -172,15 +173,14 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { }); terr != nil { return terr } - user.ConfirmationToken = hashedToken user.ConfirmationSentAt = &now user.InvitedAt = &now - terr = tx.UpdateOnly(user, "confirmation_token", "confirmation_sent_at", "invited_at") + terr = tx.UpdateOnly(user, "confirmation_sent_at", "invited_at") if terr != nil { terr = errors.Wrap(terr, "Database error updating user for invite") return terr } - terr = models.CreateOneTimeToken(tx, user.ID, user.GetEmail(), user.ConfirmationToken, models.ConfirmationToken) + ott, terr = models.CreateOneTimeToken(tx, user.ID, user.GetEmail(), hashedToken, models.ConfirmationToken) if terr != nil { terr = errors.Wrap(terr, "Database error creating confirmation token for invite in admin") return terr @@ -211,20 +211,19 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { } user.Identities = []models.Identity{*identity} } - user.ConfirmationToken = hashedToken user.ConfirmationSentAt = &now - terr = tx.UpdateOnly(user, "confirmation_token", "confirmation_sent_at") + terr = tx.UpdateOnly(user, "confirmation_sent_at") if terr != nil { terr = errors.Wrap(terr, "Database error updating user for confirmation") return terr } - terr = models.CreateOneTimeToken(tx, user.ID, user.GetEmail(), user.ConfirmationToken, models.ConfirmationToken) + ott, terr = models.CreateOneTimeToken(tx, user.ID, user.GetEmail(), hashedToken, models.ConfirmationToken) if terr != nil { terr = errors.Wrap(terr, "Database error creating confirmation token for signup in admin") return terr } - case mail.EmailChangeCurrentVerification, mail.EmailChangeNewVerification: - if !config.Mailer.SecureEmailChangeEnabled && params.Type == "email_change_current" { + case mail.EmailChangeCurrentVerification: + if !config.Mailer.SecureEmailChangeEnabled { return badRequestError(ErrorCodeValidationFailed, "Enable secure email change to generate link for current email") } params.NewEmail, terr = validateEmail(params.NewEmail) @@ -240,29 +239,33 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { user.EmailChangeSentAt = &now user.EmailChange = params.NewEmail user.EmailChangeConfirmStatus = zeroConfirmation - if params.Type == "email_change_current" { - user.EmailChangeTokenCurrent = hashedToken - } else if params.Type == "email_change_new" { - user.EmailChangeTokenNew = crypto.GenerateTokenHash(params.NewEmail, otp) + if terr := tx.UpdateOnly(user, "email_change", "email_change_sent_at", "email_change_confirm_status"); terr != nil { + return errors.Wrap(terr, "Database error updating user for email change") } - terr = tx.UpdateOnly(user, "email_change_token_current", "email_change_token_new", "email_change", "email_change_sent_at", "email_change_confirm_status") + ott, terr = models.CreateOneTimeToken(tx, user.ID, user.GetEmail(), hashedToken, models.EmailChangeTokenCurrent) + if terr != nil { + return errors.Wrap(terr, "Database error creating email change token current in admin") + } + case mail.EmailChangeNewVerification: + params.NewEmail, terr = validateEmail(params.NewEmail) if terr != nil { - terr = errors.Wrap(terr, "Database error updating user for email change") return terr } - if user.EmailChangeTokenCurrent != "" { - terr = models.CreateOneTimeToken(tx, user.ID, user.GetEmail(), user.EmailChangeTokenCurrent, models.EmailChangeTokenCurrent) - if terr != nil { - terr = errors.Wrap(terr, "Database error creating email change token current in admin") - return terr - } + if duplicateUser, terr := models.IsDuplicatedEmail(tx, params.NewEmail, user.Aud, user); terr != nil { + return internalServerError("Database error checking email").WithInternalError(terr) + } else if duplicateUser != nil { + return unprocessableEntityError(ErrorCodeEmailExists, DuplicateEmailMsg) } - if user.EmailChangeTokenNew != "" { - terr = models.CreateOneTimeToken(tx, user.ID, user.EmailChange, user.EmailChangeTokenNew, models.EmailChangeTokenNew) - if terr != nil { - terr = errors.Wrap(terr, "Database error creating email change token new in admin") - return terr - } + now := time.Now() + user.EmailChangeSentAt = &now + user.EmailChange = params.NewEmail + user.EmailChangeConfirmStatus = zeroConfirmation + if terr := tx.UpdateOnly(user, "email_change", "email_change_sent_at", "email_change_confirm_status"); terr != nil { + return errors.Wrap(terr, "Database error updating user for email change") + } + ott, terr = models.CreateOneTimeToken(tx, user.ID, user.EmailChange, crypto.GenerateTokenHash(params.NewEmail, otp), models.EmailChangeTokenNew) + if terr != nil { + return errors.Wrap(terr, "Database error creating email change token new in admin") } default: return badRequestError(ErrorCodeValidationFailed, "Invalid email action link type requested: %v", params.Type) @@ -273,9 +276,9 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { } externalURL := getExternalHost(ctx) - url, terr = mailer.GetEmailActionLink(user, params.Type, referrer, externalURL) + url, terr = mailer.GetEmailActionLink(ott, params.Type, referrer, externalURL) if terr != nil { - return terr + return internalServerError("Error generating email action link").WithInternalError(terr) } return nil }) @@ -301,65 +304,51 @@ func (a *API) sendConfirmation(r *http.Request, tx *storage.Connection, u *model maxFrequency := config.SMTP.MaxFrequency otpLength := config.Mailer.OtpLength - var err error if err := validateSentWithinFrequencyLimit(u.ConfirmationSentAt, maxFrequency); err != nil { return err } - oldToken := u.ConfirmationToken otp, err := crypto.GenerateOtp(otpLength) if err != nil { // OTP generation must succeeed panic(err) } - token := crypto.GenerateTokenHash(u.GetEmail(), otp) - u.ConfirmationToken = addFlowPrefixToToken(token, flowType) - now := time.Now() - err = a.sendEmail(r, tx, u, mail.SignupVerification, otp, "", u.ConfirmationToken) + token := addFlowPrefixToToken(crypto.GenerateTokenHash(u.GetEmail(), otp), flowType) + ott, err := models.CreateOneTimeToken(tx, u.ID, u.GetEmail(), token, models.ConfirmationToken) if err != nil { - u.ConfirmationToken = oldToken - return errors.Wrap(err, "Error sending confirmation email") + return errors.Wrap(err, "Database error creating confirmation token") } + now := time.Now() u.ConfirmationSentAt = &now - err = tx.UpdateOnly(u, "confirmation_token", "confirmation_sent_at") - if err != nil { + if err := tx.UpdateOnly(u, "confirmation_sent_at"); err != nil { return errors.Wrap(err, "Database error updating user for confirmation") } - - err = models.CreateOneTimeToken(tx, u.ID, u.GetEmail(), u.ConfirmationToken, models.ConfirmationToken) - if err != nil { - return errors.Wrap(err, "Database error creating confirmation token") + if err := a.sendEmail(r, tx, u, ott, mail.SignupVerification, otp); err != nil { + return errors.Wrap(err, "Error sending confirmation email") } - return nil } func (a *API) sendInvite(r *http.Request, tx *storage.Connection, u *models.User) error { config := a.config otpLength := config.Mailer.OtpLength - var err error - oldToken := u.ConfirmationToken otp, err := crypto.GenerateOtp(otpLength) if err != nil { // OTP generation must succeed panic(err) } - u.ConfirmationToken = crypto.GenerateTokenHash(u.GetEmail(), otp) - now := time.Now() - err = a.sendEmail(r, tx, u, mail.InviteVerification, otp, "", u.ConfirmationToken) + token := crypto.GenerateTokenHash(u.GetEmail(), otp) + ott, err := models.CreateOneTimeToken(tx, u.ID, u.GetEmail(), token, models.ConfirmationToken) if err != nil { - u.ConfirmationToken = oldToken - return errors.Wrap(err, "Error sending invite email") + return errors.Wrap(err, "Database error creating confirmation token for invite") } + now := time.Now() u.InvitedAt = &now u.ConfirmationSentAt = &now - err = tx.UpdateOnly(u, "confirmation_token", "confirmation_sent_at", "invited_at") - if err != nil { + if err := tx.UpdateOnly(u, "confirmation_sent_at", "invited_at"); err != nil { return errors.Wrap(err, "Database error updating user for invite") } - - err = models.CreateOneTimeToken(tx, u.ID, u.GetEmail(), u.ConfirmationToken, models.ConfirmationToken) - if err != nil { - return errors.Wrap(err, "Database error creating confirmation token for invite") + if err := a.sendEmail(r, tx, u, ott, mail.InviteVerification, otp); err != nil { + return errors.Wrap(err, "Error sending invite email") } return nil @@ -369,34 +358,27 @@ func (a *API) sendPasswordRecovery(r *http.Request, tx *storage.Connection, u *m config := a.config maxFrequency := config.SMTP.MaxFrequency otpLength := config.Mailer.OtpLength - var err error if err := validateSentWithinFrequencyLimit(u.RecoverySentAt, maxFrequency); err != nil { return err } - - oldToken := u.RecoveryToken otp, err := crypto.GenerateOtp(otpLength) if err != nil { // OTP generation must succeed panic(err) } - token := crypto.GenerateTokenHash(u.GetEmail(), otp) - u.RecoveryToken = addFlowPrefixToToken(token, flowType) - now := time.Now() - err = a.sendEmail(r, tx, u, mail.RecoveryVerification, otp, "", u.RecoveryToken) + token := addFlowPrefixToToken(crypto.GenerateTokenHash(u.GetEmail(), otp), flowType) + ott, err := models.CreateOneTimeToken(tx, u.ID, u.GetEmail(), token, models.RecoveryToken) if err != nil { - u.RecoveryToken = oldToken - return errors.Wrap(err, "Error sending recovery email") + return errors.Wrap(err, "Database error creating recovery token") } + now := time.Now() u.RecoverySentAt = &now - err = tx.UpdateOnly(u, "recovery_token", "recovery_sent_at") + err = tx.UpdateOnly(u, "recovery_sent_at") if err != nil { return errors.Wrap(err, "Database error updating user for recovery") } - - err = models.CreateOneTimeToken(tx, u.ID, u.GetEmail(), u.RecoveryToken, models.RecoveryToken) - if err != nil { - return errors.Wrap(err, "Database error creating recovery token") + if err = a.sendEmail(r, tx, u, ott, mail.RecoveryVerification, otp); err != nil { + return errors.Wrap(err, "Error sending recovery email") } return nil @@ -406,36 +388,27 @@ func (a *API) sendReauthenticationOtp(r *http.Request, tx *storage.Connection, u config := a.config maxFrequency := config.SMTP.MaxFrequency otpLength := config.Mailer.OtpLength - var err error - if err := validateSentWithinFrequencyLimit(u.ReauthenticationSentAt, maxFrequency); err != nil { return err } - - oldToken := u.ReauthenticationToken otp, err := crypto.GenerateOtp(otpLength) if err != nil { // OTP generation must succeed panic(err) } - u.ReauthenticationToken = crypto.GenerateTokenHash(u.GetEmail(), otp) - now := time.Now() - err = a.sendEmail(r, tx, u, mail.ReauthenticationVerification, otp, "", u.ReauthenticationToken) + token := crypto.GenerateTokenHash(u.GetEmail(), otp) + ott, err := models.CreateOneTimeToken(tx, u.ID, u.GetEmail(), token, models.ReauthenticationToken) if err != nil { - u.ReauthenticationToken = oldToken - return errors.Wrap(err, "Error sending reauthentication email") + return errors.Wrap(err, "Database error creating reauthentication token") } + now := time.Now() u.ReauthenticationSentAt = &now - err = tx.UpdateOnly(u, "reauthentication_token", "reauthentication_sent_at") - if err != nil { + if err := tx.UpdateOnly(u, "reauthentication_sent_at"); err != nil { return errors.Wrap(err, "Database error updating user for reauthentication") } - - err = models.CreateOneTimeToken(tx, u.ID, u.GetEmail(), u.ReauthenticationToken, models.ReauthenticationToken) - if err != nil { - return errors.Wrap(err, "Database error creating reauthentication token") + if err := a.sendEmail(r, tx, u, ott, mail.ReauthenticationVerification, otp); err != nil { + return errors.Wrap(err, "Error sending reauthentication email") } - return nil } @@ -449,33 +422,24 @@ func (a *API) sendMagicLink(r *http.Request, tx *storage.Connection, u *models.U if err := validateSentWithinFrequencyLimit(u.RecoverySentAt, maxFrequency); err != nil { return err } - - oldToken := u.RecoveryToken otp, err := crypto.GenerateOtp(otpLength) if err != nil { // OTP generation must succeed panic(err) } - token := crypto.GenerateTokenHash(u.GetEmail(), otp) - u.RecoveryToken = addFlowPrefixToToken(token, flowType) - - now := time.Now() - err = a.sendEmail(r, tx, u, mail.MagicLinkVerification, otp, "", u.RecoveryToken) + token := addFlowPrefixToToken(crypto.GenerateTokenHash(u.GetEmail(), otp), flowType) + ott, err := models.CreateOneTimeToken(tx, u.ID, u.GetEmail(), token, models.RecoveryToken) if err != nil { - u.RecoveryToken = oldToken - return errors.Wrap(err, "Error sending magic link email") + return errors.Wrap(err, "Database error creating recovery token") } + now := time.Now() u.RecoverySentAt = &now - err = tx.UpdateOnly(u, "recovery_token", "recovery_sent_at") - if err != nil { + if err = tx.UpdateOnly(u, "recovery_sent_at"); err != nil { return errors.Wrap(err, "Database error updating user for recovery") } - - err = models.CreateOneTimeToken(tx, u.ID, u.GetEmail(), u.RecoveryToken, models.RecoveryToken) - if err != nil { - return errors.Wrap(err, "Database error creating recovery token") + if err := a.sendEmail(r, tx, u, ott, mail.MagicLinkVerification, otp); err != nil { + return errors.Wrap(err, "Error sending magic link email") } - return nil } @@ -483,7 +447,6 @@ func (a *API) sendMagicLink(r *http.Request, tx *storage.Connection, u *models.U func (a *API) sendEmailChange(r *http.Request, tx *storage.Connection, u *models.User, email string, flowType models.FlowType) error { config := a.config otpLength := config.Mailer.OtpLength - var err error if err := validateSentWithinFrequencyLimit(u.EmailChangeSentAt, config.SMTP.MaxFrequency); err != nil { return err } @@ -494,53 +457,41 @@ func (a *API) sendEmailChange(r *http.Request, tx *storage.Connection, u *models panic(err) } u.EmailChange = email - token := crypto.GenerateTokenHash(u.EmailChange, otpNew) - u.EmailChangeTokenNew = addFlowPrefixToToken(token, flowType) + token := addFlowPrefixToToken(crypto.GenerateTokenHash(u.EmailChange, otpNew), flowType) + ottNew, err := models.CreateOneTimeToken(tx, u.ID, u.EmailChange, token, models.EmailChangeTokenNew) + if err != nil { + return errors.Wrap(err, "Database error creating email change token new") + } otpCurrent := "" + var ottCurrent *models.OneTimeToken if config.Mailer.SecureEmailChangeEnabled && u.GetEmail() != "" { otpCurrent, err = crypto.GenerateOtp(otpLength) if err != nil { // OTP generation must succeed panic(err) } - currentToken := crypto.GenerateTokenHash(u.GetEmail(), otpCurrent) - u.EmailChangeTokenCurrent = addFlowPrefixToToken(currentToken, flowType) + currentToken := addFlowPrefixToToken(crypto.GenerateTokenHash(u.GetEmail(), otpCurrent), flowType) + ottCurrent, err = models.CreateOneTimeToken(tx, u.ID, u.GetEmail(), currentToken, models.EmailChangeTokenCurrent) + if err != nil { + return errors.Wrap(err, "Database error creating email change token current") + } } - u.EmailChangeConfirmStatus = zeroConfirmation now := time.Now() - err = a.sendEmail(r, tx, u, mail.EmailChangeVerification, otpCurrent, otpNew, u.EmailChangeTokenNew) - if err != nil { - return err - } - + u.EmailChangeConfirmStatus = zeroConfirmation u.EmailChangeSentAt = &now - err = tx.UpdateOnly( + if err := tx.UpdateOnly( u, - "email_change_token_current", - "email_change_token_new", "email_change", "email_change_sent_at", "email_change_confirm_status", - ) - - if err != nil { + ); err != nil { return errors.Wrap(err, "Database error updating user for email change") } - if u.EmailChangeTokenCurrent != "" { - err = models.CreateOneTimeToken(tx, u.ID, u.GetEmail(), u.EmailChangeTokenCurrent, models.EmailChangeTokenCurrent) - if err != nil { - return errors.Wrap(err, "Database error creating email change token current") - } - } - - if u.EmailChangeTokenNew != "" { - err = models.CreateOneTimeToken(tx, u.ID, u.EmailChange, u.EmailChangeTokenNew, models.EmailChangeTokenNew) - if err != nil { - return errors.Wrap(err, "Database error creating email change token new") - } + if err = a.sendEmailChangeEmails(r, tx, u, ottNew, ottCurrent, mail.EmailChangeVerification, otpCurrent, otpNew); err != nil { + return err } return nil @@ -566,7 +517,7 @@ func validateSentWithinFrequencyLimit(sentAt *time.Time, frequency time.Duration return nil } -func (a *API) sendEmail(r *http.Request, tx *storage.Connection, u *models.User, emailActionType, otp, otpNew, tokenHashWithPrefix string) error { +func (a *API) sendEmail(r *http.Request, tx *storage.Connection, u *models.User, ott *models.OneTimeToken, emailActionType, otp string) error { mailer := a.Mailer() ctx := r.Context() config := a.config @@ -578,34 +529,66 @@ func (a *API) sendEmail(r *http.Request, tx *storage.Connection, u *models.User, EmailActionType: emailActionType, RedirectTo: referrerURL, SiteURL: externalURL.String(), - TokenHash: tokenHashWithPrefix, - } - if emailActionType == mail.EmailChangeVerification && config.Mailer.SecureEmailChangeEnabled && u.GetEmail() != "" { - emailData.TokenNew = otpNew - emailData.TokenHashNew = u.EmailChangeTokenCurrent + TokenHash: ott.TokenHash, } input := hooks.SendEmailInput{ User: u, EmailData: emailData, } output := hooks.SendEmailOutput{} - return a.invokeHook(tx, r, &input, &output, a.config.Hook.SendEmail.URI) + return a.invokeHook(tx, r, &input, &output, config.Hook.SendEmail.URI) } switch emailActionType { case mail.SignupVerification: - return mailer.ConfirmationMail(r, u, otp, referrerURL, externalURL) + return mailer.ConfirmationMail(r, u, ott, otp, referrerURL, externalURL) case mail.MagicLinkVerification: - return mailer.MagicLinkMail(r, u, otp, referrerURL, externalURL) + return mailer.MagicLinkMail(r, u, ott, otp, referrerURL, externalURL) case mail.ReauthenticationVerification: return mailer.ReauthenticateMail(r, u, otp) case mail.RecoveryVerification: - return mailer.RecoveryMail(r, u, otp, referrerURL, externalURL) + return mailer.RecoveryMail(r, u, ott, otp, referrerURL, externalURL) case mail.InviteVerification: - return mailer.InviteMail(r, u, otp, referrerURL, externalURL) - case mail.EmailChangeVerification: - return mailer.EmailChangeMail(r, u, otpNew, otp, referrerURL, externalURL) + return mailer.InviteMail(r, u, ott, otp, referrerURL, externalURL) default: return errors.New("invalid email action type") } } + +func (a *API) sendEmailChangeEmails(r *http.Request, tx *storage.Connection, u *models.User, ottNew, ottCurrent *models.OneTimeToken, emailActionType, otp, otpNew string) error { + mailer := a.Mailer() + ctx := r.Context() + config := a.config + referrerURL := utilities.GetReferrer(r, config) + externalURL := getExternalHost(ctx) + if config.Hook.SendEmail.Enabled { + emailData := mail.EmailData{ + Token: otpNew, + EmailActionType: emailActionType, + RedirectTo: referrerURL, + SiteURL: externalURL.String(), + TokenHash: ottNew.TokenHash, + } + if config.Mailer.SecureEmailChangeEnabled && u.GetEmail() != "" { + emailData.TokenNew = otp + emailData.TokenHashNew = ottCurrent.TokenHash + } + input := hooks.SendEmailInput{ + User: u, + EmailData: emailData, + } + output := hooks.SendEmailOutput{} + return a.invokeHook(tx, r, &input, &output, config.Hook.SendEmail.URI) + } + + wg := new(errgroup.Group) + if config.Mailer.SecureEmailChangeEnabled && u.GetEmail() != "" { + go mailer.EmailChangeMail(r, u, ottCurrent, otp, referrerURL, externalURL) + } + go mailer.EmailChangeMail(r, u, ottNew, otpNew, referrerURL, externalURL) + if err := wg.Wait(); err != nil { + // we return the first not-nil error observed + return err + } + return nil +} diff --git a/internal/api/mail_test.go b/internal/api/mail_test.go index 90608a13a..54429d1ef 100644 --- a/internal/api/mail_test.go +++ b/internal/api/mail_test.go @@ -135,7 +135,7 @@ func (ts *MailTestSuite) TestGenerateLink() { }, }, { - Desc: "Generate email change link", + Desc: "Generate email change link current", Body: GenerateLinkParams{ Email: "test@example.com", NewEmail: "new@example.com", @@ -147,7 +147,7 @@ func (ts *MailTestSuite) TestGenerateLink() { }, }, { - Desc: "Generate email change link", + Desc: "Generate email change link new", Body: GenerateLinkParams{ Email: "test@example.com", NewEmail: "new@example.com", diff --git a/internal/api/otp_test.go b/internal/api/otp_test.go index c72fbc361..2b980d838 100644 --- a/internal/api/otp_test.go +++ b/internal/api/otp_test.go @@ -280,9 +280,10 @@ func (ts *OtpTestSuite) TestSubsequentOtp() { newUser, err := models.FindUserByEmailAndAudience(ts.API.db, userEmail, ts.Config.JWT.Aud) require.NoError(ts.T(), err) - require.NotEmpty(ts.T(), newUser.ConfirmationToken) + + require.Len(ts.T(), newUser.OneTimeTokens, 1) + require.Equal(ts.T(), models.ConfirmationToken, newUser.OneTimeTokens[0].TokenType) require.NotEmpty(ts.T(), newUser.ConfirmationSentAt) - require.Empty(ts.T(), newUser.RecoveryToken) require.Empty(ts.T(), newUser.RecoverySentAt) require.Empty(ts.T(), newUser.EmailConfirmedAt) @@ -303,9 +304,10 @@ func (ts *OtpTestSuite) TestSubsequentOtp() { user, err := models.FindUserByEmailAndAudience(ts.API.db, userEmail, ts.Config.JWT.Aud) require.NoError(ts.T(), err) - require.NotEmpty(ts.T(), user.ConfirmationToken) + + require.Len(ts.T(), user.OneTimeTokens, 1) + require.Equal(ts.T(), models.ConfirmationToken, user.OneTimeTokens[0].TokenType) require.NotEmpty(ts.T(), user.ConfirmationSentAt) - require.Empty(ts.T(), user.RecoveryToken) require.Empty(ts.T(), user.RecoverySentAt) require.Empty(ts.T(), user.EmailConfirmedAt) } diff --git a/internal/api/phone.go b/internal/api/phone.go index 2147a959b..ce9ca0248 100644 --- a/internal/api/phone.go +++ b/internal/api/phone.go @@ -45,25 +45,20 @@ func formatPhoneNumber(phone string) string { // sendPhoneConfirmation sends an otp to the user's phone number func (a *API) sendPhoneConfirmation(r *http.Request, tx *storage.Connection, user *models.User, phone, otpType string, smsProvider sms_provider.SmsProvider, channel string) (string, error) { config := a.config - - var token *string var sentAt *time.Time includeFields := []string{} switch otpType { case phoneChangeVerification: - token = &user.PhoneChangeToken sentAt = user.PhoneChangeSentAt user.PhoneChange = phone - includeFields = append(includeFields, "phone_change", "phone_change_token", "phone_change_sent_at") + includeFields = append(includeFields, "phone_change", "phone_change_sent_at") case phoneConfirmationOtp: - token = &user.ConfirmationToken sentAt = user.ConfirmationSentAt - includeFields = append(includeFields, "confirmation_token", "confirmation_sent_at") + includeFields = append(includeFields, "confirmation_sent_at") case phoneReauthenticationOtp: - token = &user.ReauthenticationToken sentAt = user.ReauthenticationSentAt - includeFields = append(includeFields, "reauthentication_token", "reauthentication_sent_at") + includeFields = append(includeFields, "reauthentication_sent_at") default: return "", internalServerError("invalid otp type") } @@ -116,7 +111,7 @@ func (a *API) sendPhoneConfirmation(r *http.Request, tx *storage.Connection, use } } - *token = crypto.GenerateTokenHash(phone, otp) + token := crypto.GenerateTokenHash(phone, otp) switch otpType { case phoneConfirmationOtp: @@ -133,15 +128,15 @@ func (a *API) sendPhoneConfirmation(r *http.Request, tx *storage.Connection, use 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(), token, models.ConfirmationToken); err != nil { return messageID, 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, token, models.PhoneChangeToken); err != nil { return messageID, 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(), token, models.ReauthenticationToken); err != nil { return messageID, errors.Wrap(err, "Database error creating reauthentication token for phone") } } diff --git a/internal/api/phone_test.go b/internal/api/phone_test.go index 38daa4941..a48576d38 100644 --- a/internal/api/phone_test.go +++ b/internal/api/phone_test.go @@ -110,6 +110,7 @@ func doTestSendPhoneConfirmation(ts *PhoneTestSuite, useTestOTP bool) { for _, c := range cases { ts.Run(c.desc, func() { + require.NoError(ts.T(), models.ClearAllOneTimeTokensForUser(ts.API.db, u.ID)) provider := &TestSmsProvider{} _, err = ts.API.sendPhoneConfirmation(req, ts.API.db, u, "123456789", c.otpType, provider, sms_provider.SMSProvider) @@ -127,15 +128,19 @@ func doTestSendPhoneConfirmation(ts *PhoneTestSuite, useTestOTP bool) { switch c.otpType { case phoneConfirmationOtp: - require.NotEmpty(ts.T(), u.ConfirmationToken) + require.NotEmpty(ts.T(), u.OneTimeTokens[0].TokenHash) + require.Equal(ts.T(), u.OneTimeTokens[0].TokenType, models.ConfirmationToken) require.NotEmpty(ts.T(), u.ConfirmationSentAt) case phoneChangeVerification: - require.NotEmpty(ts.T(), u.PhoneChangeToken) + require.NotEmpty(ts.T(), u.OneTimeTokens[0].TokenHash) + require.Equal(ts.T(), u.OneTimeTokens[0].TokenType, models.PhoneChangeToken) require.NotEmpty(ts.T(), u.PhoneChangeSentAt) case phoneReauthenticationOtp: - require.NotEmpty(ts.T(), u.ReauthenticationToken) + require.NotEmpty(ts.T(), u.OneTimeTokens[0].TokenHash) + require.Equal(ts.T(), u.OneTimeTokens[0].TokenType, models.ReauthenticationToken) require.NotEmpty(ts.T(), u.ReauthenticationSentAt) default: + require.Empty(ts.T(), u.OneTimeTokens) } }) } diff --git a/internal/api/reauthenticate.go b/internal/api/reauthenticate.go index cf86d102f..7e358895b 100644 --- a/internal/api/reauthenticate.go +++ b/internal/api/reauthenticate.go @@ -80,13 +80,14 @@ func (a *API) Reauthenticate(w http.ResponseWriter, r *http.Request) error { // verifyReauthentication checks if the nonce provided is valid func (a *API) verifyReauthentication(nonce string, tx *storage.Connection, config *conf.GlobalConfiguration, user *models.User) error { - if user.ReauthenticationToken == "" || user.ReauthenticationSentAt == nil { + if user.ReauthenticationSentAt == nil { return unprocessableEntityError(ErrorCodeReauthenticationNotValid, InvalidNonceMessage) } var isValid bool + var otpErr error if user.GetEmail() != "" { tokenHash := crypto.GenerateTokenHash(user.GetEmail(), nonce) - isValid = isOtpValid(tokenHash, user.ReauthenticationToken, user.ReauthenticationSentAt, config.Mailer.OtpExp) + isValid, otpErr = isOtpValid(tx, tokenHash, config.Mailer.OtpExp, models.ReauthenticationToken) } else if user.GetPhone() != "" { if config.Sms.IsTwilioVerifyProvider() { smsProvider, _ := sms_provider.GetSmsProvider(*config) @@ -96,11 +97,14 @@ func (a *API) verifyReauthentication(nonce string, tx *storage.Connection, confi return nil } else { tokenHash := crypto.GenerateTokenHash(user.GetPhone(), nonce) - isValid = isOtpValid(tokenHash, user.ReauthenticationToken, user.ReauthenticationSentAt, config.Sms.OtpExp) + isValid, otpErr = isOtpValid(tx, tokenHash, config.Sms.OtpExp, models.ReauthenticationToken) } } else { return unprocessableEntityError(ErrorCodeReauthenticationNotValid, "Reauthentication requires an email or a phone number") } + if otpErr != nil { + return internalServerError("Database error finding token").WithInternalError(otpErr) + } if !isValid { return unprocessableEntityError(ErrorCodeReauthenticationNotValid, InvalidNonceMessage) } diff --git a/internal/api/resend_test.go b/internal/api/resend_test.go index 83c58c4e4..0a8526193 100644 --- a/internal/api/resend_test.go +++ b/internal/api/resend_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" "github.com/supabase/auth/internal/conf" + "github.com/supabase/auth/internal/crypto" mail "github.com/supabase/auth/internal/mailer" "github.com/supabase/auth/internal/models" ) @@ -122,36 +123,46 @@ func (ts *ResendTestSuite) TestResendSuccess() { // disable secure email change ts.Config.Mailer.SecureEmailChangeEnabled = false - u.ConfirmationToken = "123456" u.ConfirmationSentAt = &now u.EmailChange = "bar@example.com" 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)) + + // see sendConfirmation + token := addFlowPrefixToToken(crypto.GenerateTokenHash(u.GetEmail(), "123456"), models.ImplicitFlow) + confirmationOtt, err := models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), token, models.ConfirmationToken) + require.NoError(ts.T(), err) + emailChangeNewOtt, err := models.CreateOneTimeToken(ts.API.db, u.ID, u.EmailChange, token, models.EmailChangeTokenNew) + require.NoError(ts.T(), err) phoneUser, err := models.NewUser("1234567890", "", "password", ts.Config.JWT.Aud, nil) require.NoError(ts.T(), err, "Error creating test user model") phoneUser.EmailChange = "bar@example.com" 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)) + + // see sendEmailChange + token = addFlowPrefixToToken(crypto.GenerateTokenHash(phoneUser.EmailChange, "123456"), models.ImplicitFlow) + phoneUserEmailChangeOtt, err := models.CreateOneTimeToken(ts.API.db, phoneUser.ID, phoneUser.EmailChange, token, models.EmailChangeTokenNew) + require.NoError(ts.T(), err) emailUser, err := models.NewUser("", "bar@example.com", "password", ts.Config.JWT.Aud, nil) require.NoError(ts.T(), err, "Error creating test user model") phoneUser.PhoneChange = "1234567890" 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)) + + // see sendPhoneChange + token = crypto.GenerateTokenHash(phoneUser.PhoneChange, "123456") + phoneChangeOtt, err := models.CreateOneTimeToken(ts.API.db, phoneUser.ID, phoneUser.PhoneChange, token, models.PhoneChangeToken) + require.NoError(ts.T(), err) cases := []struct { desc string params map[string]interface{} // expected map[string]interface{} user *models.User + ott *models.OneTimeToken }{ { desc: "Resend signup confirmation", @@ -160,6 +171,7 @@ func (ts *ResendTestSuite) TestResendSuccess() { "email": u.GetEmail(), }, user: u, + ott: confirmationOtt, }, { desc: "Resend email change", @@ -168,6 +180,7 @@ func (ts *ResendTestSuite) TestResendSuccess() { "email": u.GetEmail(), }, user: u, + ott: emailChangeNewOtt, }, { desc: "Resend email change for phone user", @@ -176,6 +189,7 @@ func (ts *ResendTestSuite) TestResendSuccess() { "phone": phoneUser.GetPhone(), }, user: phoneUser, + ott: phoneUserEmailChangeOtt, }, { desc: "Resend phone change for email user", @@ -184,6 +198,7 @@ func (ts *ResendTestSuite) TestResendSuccess() { "email": emailUser.GetEmail(), }, user: emailUser, + ott: phoneChangeOtt, }, } @@ -191,6 +206,9 @@ func (ts *ResendTestSuite) TestResendSuccess() { ts.Run(c.desc, func() { var buffer bytes.Buffer require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(c.params)) + + // resend endpoint is expected to generate a new token to send to the user + // we need to check that the original token doesn't match the new token req := httptest.NewRequest(http.MethodPost, "http://localhost/resend", &buffer) req.Header.Set("Content-Type", "application/json") @@ -205,10 +223,10 @@ func (ts *ResendTestSuite) TestResendSuccess() { require.NotEmpty(ts.T(), dbUser) if c.params["type"] == mail.SignupVerification { - require.NotEqual(ts.T(), dbUser.ConfirmationToken, c.user.ConfirmationToken) + require.NotContains(ts.T(), dbUser.OneTimeTokens, c.ott) require.NotEqual(ts.T(), dbUser.ConfirmationSentAt, c.user.ConfirmationSentAt) } else if c.params["type"] == mail.EmailChangeVerification { - require.NotEqual(ts.T(), dbUser.EmailChangeTokenNew, c.user.EmailChangeTokenNew) + require.NotContains(ts.T(), dbUser.OneTimeTokens, c.ott) require.NotEqual(ts.T(), dbUser.EmailChangeSentAt, c.user.EmailChangeSentAt) } } diff --git a/internal/api/signup_test.go b/internal/api/signup_test.go index 3f4783261..4cdd716b3 100644 --- a/internal/api/signup_test.go +++ b/internal/api/signup_test.go @@ -123,19 +123,20 @@ func (ts *SignupTestSuite) TestSignupTwice() { func (ts *SignupTestSuite) TestVerifySignup() { user, err := models.NewUser("123456789", "test@example.com", "testing", ts.Config.JWT.Aud, nil) - user.ConfirmationToken = "asdf3" now := time.Now() 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)) + ott, err := models.CreateOneTimeToken(ts.API.db, user.ID, user.GetEmail(), "test-confirmation-token", models.ConfirmationToken) + require.NoError(ts.T(), err) // Find test user u, err := models.FindUserByEmailAndAudience(ts.API.db, "test@example.com", ts.Config.JWT.Aud) require.NoError(ts.T(), err) + require.NotEmpty(ts.T(), u) // Setup request - reqUrl := fmt.Sprintf("http://localhost/verify?type=%s&token=%s", mail.SignupVerification, u.ConfirmationToken) + reqUrl := fmt.Sprintf("http://localhost/verify?type=%s&token=%s", mail.SignupVerification, ott.TokenHash) req := httptest.NewRequest(http.MethodGet, reqUrl, nil) // Setup response recorder diff --git a/internal/api/token_test.go b/internal/api/token_test.go index 53a492b11..b6dc62c28 100644 --- a/internal/api/token_test.go +++ b/internal/api/token_test.go @@ -557,8 +557,9 @@ func (ts *TokenTestSuite) TestMagicLinkPKCESignIn() { u, err := models.FindUserByEmailAndAudience(ts.API.db, "test@example.com", ts.Config.JWT.Aud) require.NoError(ts.T(), err) + recoveryToken := u.OneTimeTokens[0].TokenHash // Verify OTP - requestUrl := fmt.Sprintf("http://localhost/verify?type=%v&token=%v", "magiclink", u.RecoveryToken) + requestUrl := fmt.Sprintf("http://localhost/verify?type=%v&token=%v", "magiclink", recoveryToken) req = httptest.NewRequest(http.MethodGet, requestUrl, &buffer) req.Header.Set("Content-Type", "application/json") diff --git a/internal/api/user_test.go b/internal/api/user_test.go index cef28d607..b52730492 100644 --- a/internal/api/user_test.go +++ b/internal/api/user_test.go @@ -429,14 +429,17 @@ func (ts *UserTestSuite) TestUserUpdatePasswordReauthentication() { ts.API.handler.ServeHTTP(w, req) require.Equal(ts.T(), w.Code, http.StatusOK) + // check if reauthentication token is created after calling /reauthenticate u, err = models.FindUserByEmailAndAudience(ts.API.db, "test@example.com", ts.Config.JWT.Aud) require.NoError(ts.T(), err) - require.NotEmpty(ts.T(), u.ReauthenticationToken) + reauthenticationToken := u.OneTimeTokens[0].TokenHash + require.NotEmpty(ts.T(), reauthenticationToken) require.NotEmpty(ts.T(), u.ReauthenticationSentAt) // update reauthentication token to a known token - u.ReauthenticationToken = crypto.GenerateTokenHash(u.GetEmail(), "123456") - require.NoError(ts.T(), ts.API.db.Update(u)) + newReauthenticationToken := crypto.GenerateTokenHash(u.GetEmail(), "123456") + _, err = models.CreateOneTimeToken(ts.API.db, u.ID, "relates-to-not-used", newReauthenticationToken, models.ReauthenticationToken) + require.NoError(ts.T(), err) // update password with reauthentication token var buffer bytes.Buffer @@ -462,7 +465,7 @@ func (ts *UserTestSuite) TestUserUpdatePasswordReauthentication() { require.NoError(ts.T(), err) require.True(ts.T(), isAuthenticated) - require.Empty(ts.T(), u.ReauthenticationToken) + require.Empty(ts.T(), u.OneTimeTokens) require.Nil(ts.T(), u.ReauthenticationSentAt) } diff --git a/internal/api/verify.go b/internal/api/verify.go index 97a13b93b..ad52419f7 100644 --- a/internal/api/verify.go +++ b/internal/api/verify.go @@ -508,18 +508,16 @@ func (a *API) emailChangeVerify(r *http.Request, conn *storage.Connection, param user.EmailChangeConfirmStatus = singleConfirmation - if params.Token == user.EmailChangeTokenCurrent || params.TokenHash == user.EmailChangeTokenCurrent || (currentOTT != nil && params.TokenHash == currentOTT.TokenHash) { - user.EmailChangeTokenCurrent = "" + if currentOTT != nil && params.TokenHash == currentOTT.TokenHash { if terr := models.ClearOneTimeTokenForUser(tx, user.ID, models.EmailChangeTokenCurrent); terr != nil { return terr } - } else if params.Token == user.EmailChangeTokenNew || params.TokenHash == user.EmailChangeTokenNew || (newOTT != nil && params.TokenHash == newOTT.TokenHash) { - user.EmailChangeTokenNew = "" + } else if newOTT != nil && params.TokenHash == newOTT.TokenHash { if terr := models.ClearOneTimeTokenForUser(tx, user.ID, models.EmailChangeTokenNew); terr != nil { return terr } } - if terr := tx.UpdateOnly(user, "email_change_confirm_status", "email_change_token_current", "email_change_token_new"); terr != nil { + if terr := tx.UpdateOnly(user, "email_change_confirm_status"); terr != nil { return terr } return nil @@ -613,7 +611,10 @@ func (a *API) verifyTokenHash(conn *storage.Connection, params *VerifyParams) (* case mail.EmailOTPVerification: sentAt := user.ConfirmationSentAt params.Type = "signup" - if user.RecoveryToken == params.TokenHash { + if _, err := models.FindOneTimeToken(conn, params.TokenHash, models.RecoveryToken); err != nil { + if !models.IsNotFoundError(err) { + return nil, internalServerError("Database error finding token").WithInternalError(err) + } sentAt = user.RecoverySentAt params.Type = "magiclink" } @@ -666,38 +667,34 @@ func (a *API) verifyUserAndToken(conn *storage.Connection, params *VerifyParams, } var isValid bool - + var otpErr error smsProvider, _ := sms_provider.GetSmsProvider(*config) switch params.Type { case mail.EmailOTPVerification: // if the type is emailOTPVerification, we'll check both the confirmation_token and recovery_token columns - if isOtpValid(tokenHash, user.ConfirmationToken, user.ConfirmationSentAt, config.Mailer.OtpExp) { - isValid = true + isValid, otpErr = isOtpValid(conn, tokenHash, config.Mailer.OtpExp, models.ConfirmationToken) + if otpErr == nil { params.Type = mail.SignupVerification - } else if isOtpValid(tokenHash, user.RecoveryToken, user.RecoverySentAt, config.Mailer.OtpExp) { - isValid = true + break + } + isValid, otpErr = isOtpValid(conn, tokenHash, config.Mailer.OtpExp, models.RecoveryToken) + if otpErr == nil { params.Type = mail.MagicLinkVerification - } else { - isValid = false + break } case mail.SignupVerification, mail.InviteVerification: - isValid = isOtpValid(tokenHash, user.ConfirmationToken, user.ConfirmationSentAt, config.Mailer.OtpExp) + isValid, otpErr = isOtpValid(conn, tokenHash, config.Mailer.OtpExp, models.ConfirmationToken) case mail.RecoveryVerification, mail.MagicLinkVerification: - isValid = isOtpValid(tokenHash, user.RecoveryToken, user.RecoverySentAt, config.Mailer.OtpExp) + isValid, otpErr = isOtpValid(conn, tokenHash, config.Mailer.OtpExp, models.RecoveryToken) case mail.EmailChangeVerification: - isValid = isOtpValid(tokenHash, user.EmailChangeTokenCurrent, user.EmailChangeSentAt, config.Mailer.OtpExp) || - isOtpValid(tokenHash, user.EmailChangeTokenNew, user.EmailChangeSentAt, config.Mailer.OtpExp) + isValid, otpErr = isOtpValid(conn, tokenHash, config.Mailer.OtpExp, models.EmailChangeTokenCurrent, models.EmailChangeTokenNew) case phoneChangeVerification, smsVerification: phone := params.Phone - sentAt := user.ConfirmationSentAt - expectedToken := user.ConfirmationToken if params.Type == phoneChangeVerification { phone = user.PhoneChange - sentAt = user.PhoneChangeSentAt - expectedToken = user.PhoneChangeToken } if config.Sms.IsTwilioVerifyProvider() { - if testOTP, ok := config.Sms.GetTestOTP(params.Phone, time.Now()); ok { + if testOTP, ok := config.Sms.GetTestOTP(phone, time.Now()); ok { if params.Token == testOTP { return user, nil } @@ -707,7 +704,11 @@ func (a *API) verifyUserAndToken(conn *storage.Connection, params *VerifyParams, } return user, nil } - isValid = isOtpValid(tokenHash, expectedToken, sentAt, config.Sms.OtpExp) + isValid, otpErr = isOtpValid(conn, tokenHash, config.Sms.OtpExp, models.ConfirmationToken, models.PhoneChangeToken) + } + + if otpErr != nil { + return nil, internalServerError("Database error finding token").WithInternalError(err) } if !isValid { @@ -717,11 +718,25 @@ func (a *API) verifyUserAndToken(conn *storage.Connection, params *VerifyParams, } // isOtpValid checks the actual otp sent against the expected otp and ensures that it's within the valid window -func isOtpValid(actual, expected string, sentAt *time.Time, otpExp uint) bool { - if expected == "" || sentAt == nil { - return false +func isOtpValid(tx *storage.Connection, tokenHash string, otpExp uint, tokenTypes ...models.OneTimeTokenType) (bool, error) { + token, err := models.FindOneTimeToken(tx, tokenHash, tokenTypes...) + if err != nil { + if !models.IsNotFoundError(err) { + return false, err + } + // try again with the pkce prefix + token, err = models.FindOneTimeToken(tx, "pkce_"+tokenHash, tokenTypes...) + if err != nil { + if models.IsNotFoundError(err) { + return false, nil + } + return false, err + } + } + if isOtpExpired(&token.CreatedAt, otpExp) { + return false, nil } - return !isOtpExpired(sentAt, otpExp) && ((actual == expected) || ("pkce_"+actual == expected)) + return true, nil } func isOtpExpired(sentAt *time.Time, otpExp uint) bool { diff --git a/internal/api/verify_test.go b/internal/api/verify_test.go index 73ef6f768..cce8f3d77 100644 --- a/internal/api/verify_test.go +++ b/internal/api/verify_test.go @@ -11,6 +11,7 @@ import ( "testing" "time" + "github.com/gofrs/uuid" mail "github.com/supabase/auth/internal/mailer" "github.com/stretchr/testify/assert" @@ -108,7 +109,7 @@ func (ts *VerifyTestSuite) TestVerifyPasswordRecovery() { assert.WithinDuration(ts.T(), time.Now(), *u.RecoverySentAt, 1*time.Second) assert.False(ts.T(), u.IsConfirmed()) - recoveryToken := u.RecoveryToken + recoveryToken := u.OneTimeTokens[0].TokenHash reqURL := fmt.Sprintf("http://localhost/verify?type=%s&token=%s", mail.RecoveryVerification, recoveryToken) req = httptest.NewRequest(http.MethodGet, reqURL, nil) @@ -174,8 +175,6 @@ func (ts *VerifyTestSuite) TestVerifySecureEmailChange() { // reset user 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)) @@ -205,8 +204,14 @@ func (ts *VerifyTestSuite) TestVerifySecureEmailChange() { u, err = models.FindUserByEmailAndAudience(ts.API.db, c.currentEmail, ts.Config.JWT.Aud) require.NoError(ts.T(), err) - currentTokenHash := u.EmailChangeTokenCurrent - newTokenHash := u.EmailChangeTokenNew + var currentTokenHash, newTokenHash string + for _, ott := range u.OneTimeTokens { + if ott.TokenType == models.EmailChangeTokenCurrent { + currentTokenHash = ott.TokenHash + } else if ott.TokenType == models.EmailChangeTokenNew { + newTokenHash = ott.TokenHash + } + } u, err = models.FindUserByEmailAndAudience(ts.API.db, c.currentEmail, ts.Config.JWT.Aud) require.NoError(ts.T(), err) @@ -283,14 +288,15 @@ func (ts *VerifyTestSuite) TestExpiredConfirmationToken() { u, err := models.FindUserByEmailAndAudience(ts.API.db, "test@example.com", ts.Config.JWT.Aud) require.NoError(ts.T(), err) - u.ConfirmationToken = "asdf3" + token := "asdf3" 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)) + ott, err := models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), token, models.ConfirmationToken) + require.NoError(ts.T(), err) // Setup request - reqURL := fmt.Sprintf("http://localhost/verify?type=%s&token=%s", mail.SignupVerification, u.ConfirmationToken) + reqURL := fmt.Sprintf("http://localhost/verify?type=%s&token=%s", mail.SignupVerification, ott.TokenHash) req := httptest.NewRequest(http.MethodGet, reqURL, nil) // Setup response recorder @@ -313,19 +319,15 @@ func (ts *VerifyTestSuite) TestInvalidOtp() { u, err := models.FindUserByPhoneAndAudience(ts.API.db, "12345678", ts.Config.JWT.Aud) require.NoError(ts.T(), err) sentTime := time.Now().Add(-48 * time.Hour) - u.ConfirmationToken = "123456" u.ConfirmationSentAt = &sentTime u.PhoneChange = "22222222" - u.PhoneChangeToken = "123456" u.PhoneChangeSentAt = &sentTime u.EmailChange = "test@gmail.com" - 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)) + confirmationOtt, err := models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), "123456", models.ConfirmationToken) + require.NoError(ts.T(), err) + phoneChangeOtt, err := models.CreateOneTimeToken(ts.API.db, u.ID, u.PhoneChange, "123456", models.PhoneChangeToken) + require.NoError(ts.T(), err) type ResponseBody struct { Code int `json:"code"` @@ -348,7 +350,17 @@ func (ts *VerifyTestSuite) TestInvalidOtp() { sentTime: time.Now().Add(-48 * time.Hour), body: map[string]interface{}{ "type": smsVerification, - "token": u.ConfirmationToken, + "token": confirmationOtt.TokenHash, + "phone": u.GetPhone(), + }, + expected: expectedResponse, + }, + { + desc: "Expired Phone Change OTP", + sentTime: time.Now().Add(-48 * time.Hour), + body: map[string]interface{}{ + "type": smsVerification, + "token": phoneChangeOtt.TokenHash, "phone": u.GetPhone(), }, expected: expectedResponse, @@ -428,18 +440,15 @@ func (ts *VerifyTestSuite) TestInvalidOtp() { } func (ts *VerifyTestSuite) TestExpiredRecoveryToken() { - // verify variant testing not necessary in this test as it's testing - // the RecoverySentAt behavior, not the RecoveryToken behavior - + // Since we are testing the RecoverySentAt behavior, not the RecoveryToken behavior, we don't need to create a recovery token u, err := models.FindUserByEmailAndAudience(ts.API.db, "test@example.com", ts.Config.JWT.Aud) require.NoError(ts.T(), err) - u.RecoveryToken = "asdf3" sentTime := time.Now().Add(-48 * time.Hour) u.RecoverySentAt = &sentTime require.NoError(ts.T(), ts.API.db.Update(u)) // Setup request - reqURL := fmt.Sprintf("http://localhost/verify?type=%s&token=%s", "signup", u.RecoveryToken) + reqURL := fmt.Sprintf("http://localhost/verify?type=%s&token=%s", "signup", "asdf3") req := httptest.NewRequest(http.MethodGet, reqURL, nil) // Setup response recorder @@ -482,7 +491,8 @@ func (ts *VerifyTestSuite) TestVerifyPermitedCustomUri() { redirectURL, _ := url.Parse(ts.Config.URIAllowList[0]) - reqURL := fmt.Sprintf("http://localhost/verify?type=%s&token=%s&redirect_to=%s", "recovery", u.RecoveryToken, redirectURL.String()) + recoveryToken := u.OneTimeTokens[0].TokenHash + reqURL := fmt.Sprintf("http://localhost/verify?type=%s&token=%s&redirect_to=%s", "recovery", recoveryToken, redirectURL.String()) req = httptest.NewRequest(http.MethodGet, reqURL, nil) w = httptest.NewRecorder() @@ -529,7 +539,8 @@ func (ts *VerifyTestSuite) TestVerifyNotPermitedCustomUri() { fakeredirectURL, _ := url.Parse("http://custom-url.com") siteURL, _ := url.Parse(ts.Config.SiteURL) - reqURL := fmt.Sprintf("http://localhost/verify?type=%s&token=%s&redirect_to=%s", "recovery", u.RecoveryToken, fakeredirectURL.String()) + recoveryToken := u.OneTimeTokens[0].TokenHash + reqURL := fmt.Sprintf("http://localhost/verify?type=%s&token=%s&redirect_to=%s", "recovery", recoveryToken, fakeredirectURL.String()) req = httptest.NewRequest(http.MethodGet, reqURL, nil) w = httptest.NewRecorder() @@ -654,13 +665,13 @@ func (ts *VerifyTestSuite) TestVerifySignupWithRedirectURLContainedPath() { // set verify token to user as it actual do in magic link method u, err := models.FindUserByEmailAndAudience(ts.API.db, "test@example.com", ts.Config.JWT.Aud) require.NoError(ts.T(), err) - u.ConfirmationToken = "someToken" 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)) + ott, err := models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), "test-confirmation-token", models.ConfirmationToken) + require.NoError(ts.T(), err) - reqURL := fmt.Sprintf("http://localhost/verify?type=%s&token=%s&redirect_to=%s", "signup", u.ConfirmationToken, redirectURL) + reqURL := fmt.Sprintf("http://localhost/verify?type=%s&token=%s&redirect_to=%s", "signup", ott.TokenHash, redirectURL) req := httptest.NewRequest(http.MethodGet, reqURL, nil) w := httptest.NewRecorder() @@ -713,9 +724,11 @@ 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)) + _, err = models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), c.payload.Token, models.ConfirmationToken) + require.NoError(ts.T(), err) } else if c.payload.Type == "magiclink" { - require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), c.payload.Token, models.RecoveryToken)) + _, err = models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), c.payload.Token, models.RecoveryToken) + require.NoError(ts.T(), err) } require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(c.payload)) @@ -736,6 +749,9 @@ func (ts *VerifyTestSuite) TestVerifyPKCEOTP() { require.NoError(ts.T(), err) assert.True(ts.T(), u.IsConfirmed()) + // one time tokens should be cleared after successful verification + assert.Empty(ts.T(), u.OneTimeTokens) + f, err := url.ParseQuery(rURL.RawQuery) require.NoError(ts.T(), err) assert.NotEmpty(ts.T(), f.Get("code")) @@ -744,13 +760,63 @@ func (ts *VerifyTestSuite) TestVerifyPKCEOTP() { } +func (ts *VerifyTestSuite) TestVerifyPostPKCE() { + var buffer bytes.Buffer + require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{ + "email": "test@example.com", + "password": "test-password", + "code_challenge": "codechallengecodechallengcodechallengcodechallengcodechallenge", + "code_challenge_method": "plain", + })) + req := httptest.NewRequest(http.MethodPost, "http://localhost/signup", &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) + + var data map[string]interface{} + require.NoError(ts.T(), json.NewDecoder(w.Body).Decode(&data)) + userId, err := uuid.FromString(data["id"].(string)) + require.NoError(ts.T(), err) + u, err := models.FindUserByID(ts.API.db, userId) + require.NoError(ts.T(), err) + require.NotEmpty(ts.T(), u.OneTimeTokens) + + // update the token hash to a known value for subsequent verification + ott := u.OneTimeTokens[0] + ott.TokenHash = addFlowPrefixToToken(crypto.GenerateTokenHash("test@example.com", "123456"), models.PKCEFlow) + require.NoError(ts.T(), ts.API.db.Update(&ott)) + + require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{ + "email": "test@example.com", + "token": "123456", + "type": "email", + })) + req = httptest.NewRequest(http.MethodPost, "http://localhost/verify", &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) + + // check that the response contains an access and refresh token + require.NoError(ts.T(), json.NewDecoder(w.Body).Decode(&data)) + require.Contains(ts.T(), data, "access_token") + require.Contains(ts.T(), data, "refresh_token") + + // check that the user in the db is confirmed + u, err = models.FindUserByID(ts.API.db, userId) + require.NoError(ts.T(), err) + require.True(ts.T(), u.IsConfirmed()) + + // // one time tokens should be cleared after successful verification + require.Empty(ts.T(), u.OneTimeTokens) +} + func (ts *VerifyTestSuite) TestVerifyBannedUser() { u, err := models.FindUserByEmailAndAudience(ts.API.db, "test@example.com", ts.Config.JWT.Aud) require.NoError(ts.T(), err) - u.ConfirmationToken = "confirmation_token" - u.RecoveryToken = "recovery_token" - u.EmailChangeTokenCurrent = "current_email_change_token" - u.EmailChangeTokenNew = "new_email_change_token" t := time.Now() u.ConfirmationSentAt = &t u.RecoverySentAt = &t @@ -759,10 +825,12 @@ 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)) + confirmationOtt, err := models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), "test-confirmation-token", models.ConfirmationToken) + require.NoError(ts.T(), err) + recoveryOtt, err := models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), "test-recovery-token", models.RecoveryToken) + require.NoError(ts.T(), err) + emailChangeCurrentOtt, err := models.CreateOneTimeToken(ts.API.db, u.ID, u.GetEmail(), "test-email-change-token-current", models.EmailChangeTokenCurrent) + require.NoError(ts.T(), err) cases := []struct { desc string @@ -772,35 +840,35 @@ func (ts *VerifyTestSuite) TestVerifyBannedUser() { desc: "Verify banned user on signup", payload: &VerifyParams{ Type: "signup", - Token: u.ConfirmationToken, + Token: confirmationOtt.TokenHash, }, }, { desc: "Verify banned user on invite", payload: &VerifyParams{ Type: "invite", - Token: u.ConfirmationToken, + Token: confirmationOtt.TokenHash, }, }, { desc: "Verify banned user on recover", payload: &VerifyParams{ Type: "recovery", - Token: u.RecoveryToken, + Token: recoveryOtt.TokenHash, }, }, { desc: "Verify banned user on magiclink", payload: &VerifyParams{ Type: "magiclink", - Token: u.RecoveryToken, + Token: recoveryOtt.TokenHash, }, }, { desc: "Verify banned user on email change", payload: &VerifyParams{ Type: "email_change", - Token: u.EmailChangeTokenCurrent, + Token: emailChangeCurrentOtt.TokenHash, }, }, } @@ -967,23 +1035,20 @@ func (ts *VerifyTestSuite) TestVerifyValidOtp() { for _, caseItem := range cases { c := caseItem ts.Run(c.desc, func() { - // create user - require.NoError(ts.T(), models.ClearAllOneTimeTokensForUser(ts.API.db, u.ID)) - u.ConfirmationSentAt = &c.sentTime u.RecoverySentAt = &c.sentTime u.EmailChangeSentAt = &c.sentTime u.PhoneChangeSentAt = &c.sentTime + assert.NoError(ts.T(), ts.API.db.Update(u)) - u.ConfirmationToken = c.expected.tokenHash - u.RecoveryToken = c.expected.tokenHash - 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)) + _, err = models.CreateOneTimeToken(ts.API.db, u.ID, "relates_to not used", c.expected.tokenHash, models.ConfirmationToken) + require.NoError(ts.T(), err) + _, err = models.CreateOneTimeToken(ts.API.db, u.ID, "relates_to not used", c.expected.tokenHash, models.RecoveryToken) + require.NoError(ts.T(), err) + _, err = models.CreateOneTimeToken(ts.API.db, u.ID, "relates_to not used", c.expected.tokenHash, models.EmailChangeTokenNew) + require.NoError(ts.T(), err) + _, err = models.CreateOneTimeToken(ts.API.db, u.ID, "relates_to not used", c.expected.tokenHash, models.PhoneChangeToken) + require.NoError(ts.T(), err) require.NoError(ts.T(), ts.API.db.Update(u)) @@ -1047,12 +1112,12 @@ func (ts *VerifyTestSuite) TestSecureEmailChangeWithTokenHash() { for _, c := range cases { ts.Run(c.desc, func() { // Set the corresponding email change tokens - u.EmailChangeTokenCurrent = currentEmailChangeToken - 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)) + _, err = models.CreateOneTimeToken(ts.API.db, u.ID, "relates_to not used", currentEmailChangeToken, models.EmailChangeTokenCurrent) + require.NoError(ts.T(), err) + _, err = models.CreateOneTimeToken(ts.API.db, u.ID, "relates_to not used", newEmailChangeToken, models.EmailChangeTokenNew) + require.NoError(ts.T(), err) currentTime := time.Now() u.EmailChangeSentAt = ¤tTime diff --git a/internal/mailer/mailer.go b/internal/mailer/mailer.go index ff19239d8..37ffc05da 100644 --- a/internal/mailer/mailer.go +++ b/internal/mailer/mailer.go @@ -15,15 +15,14 @@ import ( // Mailer defines the interface a mailer must implement. type Mailer interface { - Send(user *models.User, subject, body string, data map[string]interface{}) error - InviteMail(r *http.Request, user *models.User, otp, referrerURL string, externalURL *url.URL) error - ConfirmationMail(r *http.Request, user *models.User, otp, referrerURL string, externalURL *url.URL) error - RecoveryMail(r *http.Request, user *models.User, otp, referrerURL string, externalURL *url.URL) error - MagicLinkMail(r *http.Request, user *models.User, otp, referrerURL string, externalURL *url.URL) error - EmailChangeMail(r *http.Request, user *models.User, otpNew, otpCurrent, referrerURL string, externalURL *url.URL) error + InviteMail(r *http.Request, user *models.User, ott *models.OneTimeToken, otp, referrerURL string, externalURL *url.URL) error + ConfirmationMail(r *http.Request, user *models.User, ott *models.OneTimeToken, otp, referrerURL string, externalURL *url.URL) error + RecoveryMail(r *http.Request, user *models.User, ott *models.OneTimeToken, otp, referrerURL string, externalURL *url.URL) error + MagicLinkMail(r *http.Request, user *models.User, ott *models.OneTimeToken, otp, referrerURL string, externalURL *url.URL) error + EmailChangeMail(r *http.Request, user *models.User, ott *models.OneTimeToken, otp, referrerURL string, externalURL *url.URL) error ReauthenticateMail(r *http.Request, user *models.User, otp string) error ValidateEmail(email string) error - GetEmailActionLink(user *models.User, actionType, referrerURL string, externalURL *url.URL) (string, error) + GetEmailActionLink(ott *models.OneTimeToken, actionType, referrerURL string, externalURL *url.URL) (string, error) } type EmailParams struct { diff --git a/internal/mailer/template.go b/internal/mailer/template.go index cd1aacf95..506fbaf21 100644 --- a/internal/mailer/template.go +++ b/internal/mailer/template.go @@ -88,9 +88,9 @@ func (m TemplateMailer) ValidateEmail(email string) error { } // InviteMail sends a invite mail to a new user -func (m *TemplateMailer) InviteMail(r *http.Request, user *models.User, otp, referrerURL string, externalURL *url.URL) error { +func (m *TemplateMailer) InviteMail(r *http.Request, user *models.User, ott *models.OneTimeToken, otp, referrerURL string, externalURL *url.URL) error { path, err := getPath(m.Config.Mailer.URLPaths.Invite, &EmailParams{ - Token: user.ConfirmationToken, + Token: ott.TokenHash, Type: "invite", RedirectTo: referrerURL, }) @@ -102,9 +102,9 @@ func (m *TemplateMailer) InviteMail(r *http.Request, user *models.User, otp, ref data := map[string]interface{}{ "SiteURL": m.Config.SiteURL, "ConfirmationURL": externalURL.ResolveReference(path).String(), - "Email": user.Email, + "Email": user.GetEmail(), "Token": otp, - "TokenHash": user.ConfirmationToken, + "TokenHash": ott.TokenHash, "Data": user.UserMetaData, "RedirectTo": referrerURL, } @@ -119,9 +119,9 @@ func (m *TemplateMailer) InviteMail(r *http.Request, user *models.User, otp, ref } // ConfirmationMail sends a signup confirmation mail to a new user -func (m *TemplateMailer) ConfirmationMail(r *http.Request, user *models.User, otp, referrerURL string, externalURL *url.URL) error { +func (m *TemplateMailer) ConfirmationMail(r *http.Request, user *models.User, ott *models.OneTimeToken, otp, referrerURL string, externalURL *url.URL) error { path, err := getPath(m.Config.Mailer.URLPaths.Confirmation, &EmailParams{ - Token: user.ConfirmationToken, + Token: ott.TokenHash, Type: "signup", RedirectTo: referrerURL, }) @@ -132,9 +132,9 @@ func (m *TemplateMailer) ConfirmationMail(r *http.Request, user *models.User, ot data := map[string]interface{}{ "SiteURL": m.Config.SiteURL, "ConfirmationURL": externalURL.ResolveReference(path).String(), - "Email": user.Email, + "Email": user.GetEmail(), "Token": otp, - "TokenHash": user.ConfirmationToken, + "TokenHash": ott.TokenHash, "Data": user.UserMetaData, "RedirectTo": referrerURL, } @@ -152,7 +152,7 @@ func (m *TemplateMailer) ConfirmationMail(r *http.Request, user *models.User, ot func (m *TemplateMailer) ReauthenticateMail(r *http.Request, user *models.User, otp string) error { data := map[string]interface{}{ "SiteURL": m.Config.SiteURL, - "Email": user.Email, + "Email": user.GetEmail(), "Token": otp, "Data": user.UserMetaData, } @@ -167,84 +167,48 @@ func (m *TemplateMailer) ReauthenticateMail(r *http.Request, user *models.User, } // EmailChangeMail sends an email change confirmation mail to a user -func (m *TemplateMailer) EmailChangeMail(r *http.Request, user *models.User, otpNew, otpCurrent, referrerURL string, externalURL *url.URL) error { - type Email struct { - Address string - Otp string - TokenHash string - Subject string - Template string - } - emails := []Email{ - { - Address: user.EmailChange, - Otp: otpNew, - TokenHash: user.EmailChangeTokenNew, - Subject: withDefault(m.Config.Mailer.Subjects.EmailChange, "Confirm Email Change"), - Template: m.Config.Mailer.Templates.EmailChange, +func (m *TemplateMailer) EmailChangeMail(r *http.Request, user *models.User, ott *models.OneTimeToken, otp, referrerURL string, externalURL *url.URL) error { + path, err := getPath( + m.Config.Mailer.URLPaths.EmailChange, + &EmailParams{ + Token: ott.TokenHash, + Type: "email_change", + RedirectTo: referrerURL, }, + ) + if err != nil { + return err } - currentEmail := user.GetEmail() - if m.Config.Mailer.SecureEmailChangeEnabled && currentEmail != "" { - emails = append(emails, Email{ - Address: currentEmail, - Otp: otpCurrent, - TokenHash: user.EmailChangeTokenCurrent, - Subject: withDefault(m.Config.Mailer.Subjects.Confirmation, "Confirm Email Address"), - Template: m.Config.Mailer.Templates.EmailChange, - }) - } - - errors := make(chan error) - for _, email := range emails { - path, err := getPath( - m.Config.Mailer.URLPaths.EmailChange, - &EmailParams{ - Token: email.TokenHash, - Type: "email_change", - RedirectTo: referrerURL, - }, - ) - if err != nil { - return err - } - go func(address, token, tokenHash, template string) { - data := map[string]interface{}{ - "SiteURL": m.Config.SiteURL, - "ConfirmationURL": externalURL.ResolveReference(path).String(), - "Email": user.GetEmail(), - "NewEmail": user.EmailChange, - "Token": token, - "TokenHash": tokenHash, - "SendingTo": address, - "Data": user.UserMetaData, - "RedirectTo": referrerURL, - } - errors <- m.Mailer.Mail( - address, - withDefault(m.Config.Mailer.Subjects.EmailChange, "Confirm Email Change"), - template, - defaultEmailChangeMail, - data, - ) - }(email.Address, email.Otp, email.TokenHash, email.Template) + sendingTo := user.EmailChange + if ott.TokenType == models.EmailChangeTokenCurrent { + sendingTo = user.GetEmail() } - for i := 0; i < len(emails); i++ { - e := <-errors - if e != nil { - return e - } + data := map[string]interface{}{ + "SiteURL": m.Config.SiteURL, + "ConfirmationURL": externalURL.ResolveReference(path).String(), + "Email": user.GetEmail(), + "NewEmail": user.EmailChange, + "Token": otp, + "TokenHash": ott.TokenHash, + "SendingTo": sendingTo, + "Data": user.UserMetaData, + "RedirectTo": referrerURL, } - - return nil + return m.Mailer.Mail( + sendingTo, + withDefault(m.Config.Mailer.Subjects.EmailChange, "Confirm Email Change"), + m.Config.Mailer.Templates.EmailChange, + defaultEmailChangeMail, + data, + ) } // RecoveryMail sends a password recovery mail -func (m *TemplateMailer) RecoveryMail(r *http.Request, user *models.User, otp, referrerURL string, externalURL *url.URL) error { +func (m *TemplateMailer) RecoveryMail(r *http.Request, user *models.User, ott *models.OneTimeToken, otp, referrerURL string, externalURL *url.URL) error { path, err := getPath(m.Config.Mailer.URLPaths.Recovery, &EmailParams{ - Token: user.RecoveryToken, + Token: ott.TokenHash, Type: "recovery", RedirectTo: referrerURL, }) @@ -254,9 +218,9 @@ func (m *TemplateMailer) RecoveryMail(r *http.Request, user *models.User, otp, r data := map[string]interface{}{ "SiteURL": m.Config.SiteURL, "ConfirmationURL": externalURL.ResolveReference(path).String(), - "Email": user.Email, + "Email": user.GetEmail(), "Token": otp, - "TokenHash": user.RecoveryToken, + "TokenHash": ott.TokenHash, "Data": user.UserMetaData, "RedirectTo": referrerURL, } @@ -271,9 +235,9 @@ func (m *TemplateMailer) RecoveryMail(r *http.Request, user *models.User, otp, r } // MagicLinkMail sends a login link mail -func (m *TemplateMailer) MagicLinkMail(r *http.Request, user *models.User, otp, referrerURL string, externalURL *url.URL) error { +func (m *TemplateMailer) MagicLinkMail(r *http.Request, user *models.User, ott *models.OneTimeToken, otp, referrerURL string, externalURL *url.URL) error { path, err := getPath(m.Config.Mailer.URLPaths.Recovery, &EmailParams{ - Token: user.RecoveryToken, + Token: ott.TokenHash, Type: "magiclink", RedirectTo: referrerURL, }) @@ -284,9 +248,9 @@ func (m *TemplateMailer) MagicLinkMail(r *http.Request, user *models.User, otp, data := map[string]interface{}{ "SiteURL": m.Config.SiteURL, "ConfirmationURL": externalURL.ResolveReference(path).String(), - "Email": user.Email, + "Email": user.GetEmail(), "Token": otp, - "TokenHash": user.RecoveryToken, + "TokenHash": ott.TokenHash, "Data": user.UserMetaData, "RedirectTo": referrerURL, } @@ -300,56 +264,49 @@ func (m *TemplateMailer) MagicLinkMail(r *http.Request, user *models.User, otp, ) } -// Send can be used to send one-off emails to users -func (m TemplateMailer) Send(user *models.User, subject, body string, data map[string]interface{}) error { - return m.Mailer.Mail( - user.GetEmail(), - subject, - "", - body, - data, - ) -} - // GetEmailActionLink returns a magiclink, recovery or invite link based on the actionType passed. -func (m TemplateMailer) GetEmailActionLink(user *models.User, actionType, referrerURL string, externalURL *url.URL) (string, error) { +func (m TemplateMailer) GetEmailActionLink(ott *models.OneTimeToken, actionType, referrerURL string, externalURL *url.URL) (string, error) { + if ott == nil { + return "", fmt.Errorf("ott is nil") + } + var err error var path *url.URL switch actionType { case "magiclink": path, err = getPath(m.Config.Mailer.URLPaths.Recovery, &EmailParams{ - Token: user.RecoveryToken, + Token: ott.TokenHash, Type: "magiclink", RedirectTo: referrerURL, }) case "recovery": path, err = getPath(m.Config.Mailer.URLPaths.Recovery, &EmailParams{ - Token: user.RecoveryToken, + Token: ott.TokenHash, Type: "recovery", RedirectTo: referrerURL, }) case "invite": path, err = getPath(m.Config.Mailer.URLPaths.Invite, &EmailParams{ - Token: user.ConfirmationToken, + Token: ott.TokenHash, Type: "invite", RedirectTo: referrerURL, }) case "signup": path, err = getPath(m.Config.Mailer.URLPaths.Confirmation, &EmailParams{ - Token: user.ConfirmationToken, + Token: ott.TokenHash, Type: "signup", RedirectTo: referrerURL, }) case "email_change_current": path, err = getPath(m.Config.Mailer.URLPaths.EmailChange, &EmailParams{ - Token: user.EmailChangeTokenCurrent, + Token: ott.TokenHash, Type: "email_change", RedirectTo: referrerURL, }) case "email_change_new": path, err = getPath(m.Config.Mailer.URLPaths.EmailChange, &EmailParams{ - Token: user.EmailChangeTokenNew, + Token: ott.TokenHash, Type: "email_change", RedirectTo: referrerURL, }) diff --git a/internal/models/one_time_token.go b/internal/models/one_time_token.go index 3077647a5..b7ebcc53a 100644 --- a/internal/models/one_time_token.go +++ b/internal/models/one_time_token.go @@ -128,9 +128,9 @@ 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) (*OneTimeToken, error) { if err := ClearOneTimeTokenForUser(tx, userID, tokenType); err != nil { - return err + return nil, err } oneTimeToken := &OneTimeToken{ @@ -139,13 +139,14 @@ func CreateOneTimeToken(tx *storage.Connection, userID uuid.UUID, relatesTo, tok TokenType: tokenType, TokenHash: tokenHash, RelatesTo: strings.ToLower(relatesTo), + CreatedAt: time.Now(), } if err := tx.Eager().Create(oneTimeToken); err != nil { - return err + return nil, err } - return nil + return oneTimeToken, nil } func FindOneTimeToken(tx *storage.Connection, tokenHash string, tokenTypes ...OneTimeTokenType) (*OneTimeToken, error) { diff --git a/internal/models/user.go b/internal/models/user.go index 12ac52816..21ddc11ac 100644 --- a/internal/models/user.go +++ b/internal/models/user.go @@ -33,27 +33,19 @@ type User struct { Phone storage.NullString `json:"phone" db:"phone"` PhoneConfirmedAt *time.Time `json:"phone_confirmed_at,omitempty" db:"phone_confirmed_at"` - ConfirmationToken string `json:"-" db:"confirmation_token"` ConfirmationSentAt *time.Time `json:"confirmation_sent_at,omitempty" db:"confirmation_sent_at"` // For backward compatibility only. Use EmailConfirmedAt or PhoneConfirmedAt instead. - ConfirmedAt *time.Time `json:"confirmed_at,omitempty" db:"confirmed_at" rw:"r"` - - RecoveryToken string `json:"-" db:"recovery_token"` + ConfirmedAt *time.Time `json:"confirmed_at,omitempty" db:"confirmed_at" rw:"r"` RecoverySentAt *time.Time `json:"recovery_sent_at,omitempty" db:"recovery_sent_at"` - EmailChangeTokenCurrent string `json:"-" db:"email_change_token_current"` - EmailChangeTokenNew string `json:"-" db:"email_change_token_new"` + ReauthenticationSentAt *time.Time `json:"reauthentication_sent_at,omitempty" db:"reauthentication_sent_at"` + EmailChange string `json:"new_email,omitempty" db:"email_change"` EmailChangeSentAt *time.Time `json:"email_change_sent_at,omitempty" db:"email_change_sent_at"` EmailChangeConfirmStatus int `json:"-" db:"email_change_confirm_status"` - - PhoneChangeToken string `json:"-" db:"phone_change_token"` - PhoneChange string `json:"new_phone,omitempty" db:"phone_change"` - PhoneChangeSentAt *time.Time `json:"phone_change_sent_at,omitempty" db:"phone_change_sent_at"` - - ReauthenticationToken string `json:"-" db:"reauthentication_token"` - ReauthenticationSentAt *time.Time `json:"reauthentication_sent_at,omitempty" db:"reauthentication_sent_at"` + PhoneChange string `json:"new_phone,omitempty" db:"phone_change"` + PhoneChangeSentAt *time.Time `json:"phone_change_sent_at,omitempty" db:"phone_change_sent_at"` LastSignInAt *time.Time `json:"last_sign_in_at,omitempty" db:"last_sign_in_at"` @@ -70,6 +62,8 @@ type User struct { IsAnonymous bool `json:"is_anonymous" db:"is_anonymous"` DONTUSEINSTANCEID uuid.UUID `json:"-" db:"instance_id"` + + OneTimeTokens []OneTimeToken `json:"-" has_many:"one_time_tokens"` } func NewUserWithPasswordHash(phone, email, passwordHash, aud string, userData map[string]interface{}) (*User, error) { @@ -347,19 +341,13 @@ func (u *User) SetPassword(ctx context.Context, password string, encrypt bool, e // UpdatePassword updates the user's password. Use SetPassword outside of a transaction first! func (u *User) UpdatePassword(tx *storage.Connection, sessionID *uuid.UUID) error { // These need to be reset because password change may mean the user no longer trusts the actions performed by the previous password. - u.ConfirmationToken = "" u.ConfirmationSentAt = nil - u.RecoveryToken = "" u.RecoverySentAt = nil - u.EmailChangeTokenCurrent = "" - u.EmailChangeTokenNew = "" u.EmailChangeSentAt = nil - u.PhoneChangeToken = "" u.PhoneChangeSentAt = nil - u.ReauthenticationToken = "" u.ReauthenticationSentAt = nil - if err := tx.UpdateOnly(u, "encrypted_password", "confirmation_token", "confirmation_sent_at", "recovery_token", "recovery_sent_at", "email_change_token_current", "email_change_token_new", "email_change_sent_at", "phone_change_token", "phone_change_sent_at", "reauthentication_token", "reauthentication_sent_at"); err != nil { + if err := tx.UpdateOnly(u, "encrypted_password", "confirmation_sent_at", "recovery_sent_at", "email_change_sent_at", "phone_change_sent_at", "reauthentication_sent_at"); err != nil { return err } @@ -421,11 +409,6 @@ func (u *User) Authenticate(ctx context.Context, tx *storage.Connection, passwor // ConfirmReauthentication resets the reauthentication token func (u *User) ConfirmReauthentication(tx *storage.Connection) error { - u.ReauthenticationToken = "" - if err := tx.UpdateOnly(u, "reauthentication_token"); err != nil { - return err - } - if err := ClearAllOneTimeTokensForUser(tx, u.ID); err != nil { return err } @@ -435,11 +418,10 @@ func (u *User) ConfirmReauthentication(tx *storage.Connection) error { // Confirm resets the confimation token and sets the confirm timestamp func (u *User) Confirm(tx *storage.Connection) error { - u.ConfirmationToken = "" now := time.Now() u.EmailConfirmedAt = &now - if err := tx.UpdateOnly(u, "confirmation_token", "email_confirmed_at"); err != nil { + if err := tx.UpdateOnly(u, "email_confirmed_at"); err != nil { return err } @@ -452,10 +434,9 @@ func (u *User) Confirm(tx *storage.Connection) error { // ConfirmPhone resets the confimation token and sets the confirm timestamp func (u *User) ConfirmPhone(tx *storage.Connection) error { - u.ConfirmationToken = "" now := time.Now() u.PhoneConfirmedAt = &now - if err := tx.UpdateOnly(u, "confirmation_token", "phone_confirmed_at"); err != nil { + if err := tx.UpdateOnly(u, "phone_confirmed_at"); err != nil { return nil } @@ -473,16 +454,12 @@ func (u *User) ConfirmEmailChange(tx *storage.Connection, status int) error { u.Email = storage.NullString(email) u.EmailChange = "" - u.EmailChangeTokenCurrent = "" - u.EmailChangeTokenNew = "" u.EmailChangeConfirmStatus = status if err := tx.UpdateOnly( u, "email", "email_change", - "email_change_token_current", - "email_change_token_new", "email_change_confirm_status", ); err != nil { return err @@ -524,14 +501,12 @@ func (u *User) ConfirmPhoneChange(tx *storage.Connection) error { u.Phone = storage.NullString(phone) u.PhoneChange = "" - u.PhoneChangeToken = "" u.PhoneConfirmedAt = &now if err := tx.UpdateOnly( u, "phone", "phone_change", - "phone_change_token", "phone_confirmed_at", ); err != nil { return err @@ -564,11 +539,6 @@ func (u *User) ConfirmPhoneChange(tx *storage.Connection) error { // Recover resets the recovery token func (u *User) Recover(tx *storage.Connection) error { - u.RecoveryToken = "" - if err := tx.UpdateOnly(u, "recovery_token"); err != nil { - return err - } - return ClearAllOneTimeTokensForUser(tx, u.ID) } @@ -817,11 +787,6 @@ func (u *User) SoftDeleteUser(tx *storage.Connection) error { u.EmailChange = obfuscateEmail(u, u.EmailChange) u.PhoneChange = obfuscatePhone(u, u.PhoneChange) u.EncryptedPassword = nil - u.ConfirmationToken = "" - u.RecoveryToken = "" - u.EmailChangeTokenCurrent = "" - u.EmailChangeTokenNew = "" - u.PhoneChangeToken = "" // set deleted_at time now := time.Now() @@ -834,11 +799,6 @@ func (u *User) SoftDeleteUser(tx *storage.Connection) error { "encrypted_password", "email_change", "phone_change", - "confirmation_token", - "recovery_token", - "email_change_token_current", - "email_change_token_new", - "phone_change_token", "deleted_at", ); err != nil { return err diff --git a/internal/models/user_test.go b/internal/models/user_test.go index 92b0858ce..f76d6b9a0 100644 --- a/internal/models/user_test.go +++ b/internal/models/user_test.go @@ -85,7 +85,8 @@ 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)) + _, err := CreateOneTimeToken(ts.db, u.ID, "relates_to not used", tokenHash, ConfirmationToken) + require.NoError(ts.T(), err) n, err := FindUserByConfirmationToken(ts.db, tokenHash) require.NoError(ts.T(), err) @@ -140,7 +141,8 @@ 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)) + _, err := CreateOneTimeToken(ts.db, u.ID, "relates_to not used", tokenHash, RecoveryToken) + require.NoError(ts.T(), err) n, err := FindUserByRecoveryToken(ts.db, tokenHash) require.NoError(ts.T(), err)