diff --git a/example.env b/example.env index 92bb7d917..1a9f31070 100644 --- a/example.env +++ b/example.env @@ -244,6 +244,7 @@ GOTRUE_LOG_LEVEL="debug" GOTRUE_SECURITY_REFRESH_TOKEN_ROTATION_ENABLED="false" GOTRUE_SECURITY_REFRESH_TOKEN_REUSE_INTERVAL="0" GOTRUE_SECURITY_UPDATE_PASSWORD_REQUIRE_REAUTHENTICATION="false" +GOTRUE_SECURITY_OTP_TOKEN_HASH_SALT="" GOTRUE_OPERATOR_TOKEN="unused-operator-token" # Trusted rate limiting header - this should be set by a trusted upstream proxy diff --git a/internal/api/e2e_test.go b/internal/api/e2e_test.go index 2231ac129..4851ead37 100644 --- a/internal/api/e2e_test.go +++ b/internal/api/e2e_test.go @@ -193,7 +193,7 @@ func signupAndConfirmEmail( require.False(t, hookReq.Metadata.Time.IsZero()) // verify that the latest user from find user matches OTP - otpHash := crypto.GenerateTokenHash( + otpHash := crypto.GenerateTokenHash("", expUser.GetEmail(), hookReq.EmailData.Token) require.NotEmpty(t, hookReq.EmailData.Token) require.Equal(t, otpHash, hookReq.EmailData.TokenHash) @@ -208,7 +208,7 @@ func signupAndConfirmEmail( // otp matches the user id and email ott, err := models.FindOneTimeToken( inst.Conn, - hookReq.EmailData.TokenHash, + []string{hookReq.EmailData.TokenHash}, models.ConfirmationToken) require.NoError(t, err) require.Equal(t, expUser.ID.String(), ott.UserID.String()) @@ -312,12 +312,12 @@ func TestE2EHooks(t *testing.T) { require.NotNil(t, latestUser) otp := hookReq.SMS.OTP - otpHash := crypto.GenerateTokenHash( + otpHash := crypto.GenerateTokenHash("", signupUser.GetPhone(), hookReq.SMS.OTP) ott, err := models.FindOneTimeToken( inst.Conn, - otpHash, + []string{otpHash}, models.ConfirmationToken) require.NoError(t, err) require.Equal(t, signupUser.ID.String(), ott.UserID.String()) @@ -417,12 +417,12 @@ func TestE2EHooks(t *testing.T) { require.Equal(t, currentUser.AppMetaData, hookReq.User.AppMetaData) otp = hookReq.SMS.OTP - otpHash := crypto.GenerateTokenHash( + otpHash := crypto.GenerateTokenHash("", currentUser.PhoneChange, hookReq.SMS.OTP) ott, err := models.FindOneTimeToken( inst.Conn, - otpHash, + []string{otpHash}, models.PhoneChangeToken) require.NoError(t, err) require.Equal(t, currentUser.ID.String(), ott.UserID.String()) @@ -1184,9 +1184,9 @@ func TestE2EHooks(t *testing.T) { require.Equal(t, newEmail, hookReq.User.EmailChange) // verify otps - curOtpHash := crypto.GenerateTokenHash( + curOtpHash := crypto.GenerateTokenHash("", curEmail, hookReq.EmailData.Token) - newOtpHash := crypto.GenerateTokenHash( + newOtpHash := crypto.GenerateTokenHash("", newEmail, hookReq.EmailData.TokenNew) // The hashes are switched incorrectly in the current code, i.e.: @@ -1208,7 +1208,7 @@ func TestE2EHooks(t *testing.T) { // verify there is an ott generated ott, err := models.FindOneTimeToken( inst.Conn, - hookReq.EmailData.TokenHash, + []string{hookReq.EmailData.TokenHash}, models.EmailChangeTokenNew) require.NoError(t, err) require.Equal(t, signupUser.ID.String(), ott.UserID.String()) @@ -1290,7 +1290,7 @@ func TestE2EHooks(t *testing.T) { // verify there is an ott generated ott, err := models.FindOneTimeToken( inst.Conn, - hookReq.EmailData.TokenHash, + []string{hookReq.EmailData.TokenHash}, models.EmailChangeTokenNew) require.NoError(t, err) require.Equal(t, signupUser.ID.String(), ott.UserID.String()) @@ -1313,7 +1313,7 @@ func TestE2EHooks(t *testing.T) { require.Empty(t, hookReq.EmailData.TokenHashNew) // verify otps - newOtpHash := crypto.GenerateTokenHash( + newOtpHash := crypto.GenerateTokenHash("", newEmail, hookReq.EmailData.Token) // The new email is stored on fields without _new suffix. @@ -1413,7 +1413,7 @@ func TestE2EHooks(t *testing.T) { require.Equal(t, newEmail, hookReq.User.EmailChange) // verify otps - newOtpHash := crypto.GenerateTokenHash( + newOtpHash := crypto.GenerateTokenHash("", newEmail, hookReq.EmailData.Token) // The new email is stored on fields without _new suffix. @@ -1433,7 +1433,7 @@ func TestE2EHooks(t *testing.T) { // verify there is an ott generated ott, err := models.FindOneTimeToken( inst.Conn, - hookReq.EmailData.TokenHash, + []string{hookReq.EmailData.TokenHash}, models.EmailChangeTokenNew) require.NoError(t, err) require.Equal(t, signupUser.ID.String(), ott.UserID.String()) @@ -1515,7 +1515,7 @@ func TestE2EHooks(t *testing.T) { // verify there is an ott generated ott, err := models.FindOneTimeToken( inst.Conn, - hookReq.EmailData.TokenHash, + []string{hookReq.EmailData.TokenHash}, models.EmailChangeTokenNew) require.NoError(t, err) require.Equal(t, signupUser.ID.String(), ott.UserID.String()) @@ -1538,7 +1538,7 @@ func TestE2EHooks(t *testing.T) { require.Empty(t, hookReq.EmailData.TokenHashNew) // verify otps - newOtpHash := crypto.GenerateTokenHash( + newOtpHash := crypto.GenerateTokenHash("", newEmail, hookReq.EmailData.Token) // The new email is stored on fields without _new suffix. diff --git a/internal/api/invite_test.go b/internal/api/invite_test.go index bd4fdd648..30e012e94 100644 --- a/internal/api/invite_test.go +++ b/internal/api/invite_test.go @@ -260,7 +260,7 @@ func (ts *InviteTestSuite) TestVerifyInvite() { user.InvitedAt = &now user.ConfirmationSentAt = &now user.EncryptedPassword = nil - user.ConfirmationToken = crypto.GenerateTokenHash(c.email, c.requestBody["token"].(string)) + 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)) diff --git a/internal/api/mail.go b/internal/api/mail.go index 1c42ef787..13e5b2882 100644 --- a/internal/api/mail.go +++ b/internal/api/mail.go @@ -94,7 +94,7 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { now := time.Now() otp := crypto.GenerateOtp(config.Mailer.OtpLength) - hashedToken := crypto.GenerateTokenHash(params.Email, otp) + hashedToken := crypto.GenerateTokenHash(config.Security.TokenHashSalt, params.Email, otp) var ( createdUser bool @@ -258,7 +258,7 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { if params.Type == "email_change_current" { user.EmailChangeTokenCurrent = hashedToken } else if params.Type == "email_change_new" { - user.EmailChangeTokenNew = crypto.GenerateTokenHash(params.NewEmail, otp) + user.EmailChangeTokenNew = crypto.GenerateTokenHash(config.Security.TokenHashSalt, params.NewEmail, otp) } terr = tx.UpdateOnly(user, "email_change_token_current", "email_change_token_new", "email_change", "email_change_sent_at", "email_change_confirm_status") if terr != nil { @@ -328,7 +328,7 @@ func (a *API) sendConfirmation(r *http.Request, tx *storage.Connection, u *model oldToken := u.ConfirmationToken otp := crypto.GenerateOtp(otpLength) - token := crypto.GenerateTokenHash(u.GetEmail(), otp) + token := crypto.GenerateTokenHash(config.Security.TokenHashSalt, u.GetEmail(), otp) u.ConfirmationToken = addFlowPrefixToToken(token, flowType) now := time.Now() if err = a.sendEmail(r, tx, u, sendEmailParams{ @@ -363,7 +363,7 @@ func (a *API) sendInvite(r *http.Request, tx *storage.Connection, u *models.User oldToken := u.ConfirmationToken otp := crypto.GenerateOtp(otpLength) - u.ConfirmationToken = crypto.GenerateTokenHash(u.GetEmail(), otp) + u.ConfirmationToken = crypto.GenerateTokenHash(config.Security.TokenHashSalt, u.GetEmail(), otp) now := time.Now() err = a.sendEmail(r, tx, u, sendEmailParams{ emailActionType: mail.InviteVerification, @@ -405,7 +405,7 @@ func (a *API) sendPasswordRecovery(r *http.Request, tx *storage.Connection, u *m oldToken := u.RecoveryToken otp := crypto.GenerateOtp(otpLength) - token := crypto.GenerateTokenHash(u.GetEmail(), otp) + token := crypto.GenerateTokenHash(config.Security.TokenHashSalt, u.GetEmail(), otp) u.RecoveryToken = addFlowPrefixToToken(token, flowType) now := time.Now() err := a.sendEmail(r, tx, u, sendEmailParams{ @@ -447,7 +447,7 @@ func (a *API) sendReauthenticationOtp(r *http.Request, tx *storage.Connection, u oldToken := u.ReauthenticationToken otp := crypto.GenerateOtp(otpLength) - u.ReauthenticationToken = crypto.GenerateTokenHash(u.GetEmail(), otp) + u.ReauthenticationToken = crypto.GenerateTokenHash(config.Security.TokenHashSalt, u.GetEmail(), otp) now := time.Now() err := a.sendEmail(r, tx, u, sendEmailParams{ @@ -490,7 +490,7 @@ func (a *API) sendMagicLink(r *http.Request, tx *storage.Connection, u *models.U oldToken := u.RecoveryToken otp := crypto.GenerateOtp(otpLength) - token := crypto.GenerateTokenHash(u.GetEmail(), otp) + token := crypto.GenerateTokenHash(config.Security.TokenHashSalt, u.GetEmail(), otp) u.RecoveryToken = addFlowPrefixToToken(token, flowType) now := time.Now() @@ -531,14 +531,14 @@ func (a *API) sendEmailChange(r *http.Request, tx *storage.Connection, u *models otpNew := crypto.GenerateOtp(otpLength) u.EmailChange = email - token := crypto.GenerateTokenHash(u.EmailChange, otpNew) + token := crypto.GenerateTokenHash(config.Security.TokenHashSalt, u.EmailChange, otpNew) u.EmailChangeTokenNew = addFlowPrefixToToken(token, flowType) otpCurrent := "" if config.Mailer.SecureEmailChangeEnabled && u.GetEmail() != "" { otpCurrent = crypto.GenerateOtp(otpLength) - currentToken := crypto.GenerateTokenHash(u.GetEmail(), otpCurrent) + currentToken := crypto.GenerateTokenHash(config.Security.TokenHashSalt, u.GetEmail(), otpCurrent) u.EmailChangeTokenCurrent = addFlowPrefixToToken(currentToken, flowType) } diff --git a/internal/api/mail_test.go b/internal/api/mail_test.go index 97ab2df89..2c9fedd16 100644 --- a/internal/api/mail_test.go +++ b/internal/api/mail_test.go @@ -237,7 +237,7 @@ func (ts *MailTestSuite) TestGenerateLink() { require.Equal(ts.T(), c.ExpectedResponse["redirect_to"], data["redirect_to"]) // check if hashed_token matches hash function of email and the raw otp - require.Equal(ts.T(), crypto.GenerateTokenHash(c.Body.Email, data["email_otp"].(string)), data["hashed_token"]) + require.Equal(ts.T(), crypto.GenerateTokenHash("", c.Body.Email, data["email_otp"].(string)), data["hashed_token"]) // check if the host used in the email link matches the initial request host u, err := url.ParseRequestURI(data["action_link"].(string)) diff --git a/internal/api/oauthserver/service.go b/internal/api/oauthserver/service.go index 5ea9db46e..d083d5f85 100644 --- a/internal/api/oauthserver/service.go +++ b/internal/api/oauthserver/service.go @@ -374,12 +374,12 @@ func (s *Server) regenerateOAuthServerClientSecret(ctx context.Context, clientID // OAuthServerClientUpdateParams contains parameters for updating an OAuth client type OAuthServerClientUpdateParams struct { - RedirectURIs *[]string `json:"redirect_uris,omitempty"` - GrantTypes *[]string `json:"grant_types,omitempty"` - ClientName *string `json:"client_name,omitempty"` - ClientURI *string `json:"client_uri,omitempty"` - LogoURI *string `json:"logo_uri,omitempty"` - TokenEndpointAuthMethod *string `json:"token_endpoint_auth_method,omitempty"` + RedirectURIs *[]string `json:"redirect_uris,omitempty"` + GrantTypes *[]string `json:"grant_types,omitempty"` + ClientName *string `json:"client_name,omitempty"` + ClientURI *string `json:"client_uri,omitempty"` + LogoURI *string `json:"logo_uri,omitempty"` + TokenEndpointAuthMethod *string `json:"token_endpoint_auth_method,omitempty"` } // isEmpty returns true if no fields are set for update diff --git a/internal/api/phone.go b/internal/api/phone.go index 77f46ca29..e81879410 100644 --- a/internal/api/phone.go +++ b/internal/api/phone.go @@ -124,7 +124,7 @@ func (a *API) sendPhoneConfirmation(r *http.Request, tx *storage.Connection, use } } - *token = crypto.GenerateTokenHash(phone, otp) + *token = crypto.GenerateTokenHash(config.Security.TokenHashSalt, phone, otp) switch otpType { case phoneConfirmationOtp: diff --git a/internal/api/provider/custom_oauth_claims_test.go b/internal/api/provider/custom_oauth_claims_test.go index b543e3a08..185fbd228 100644 --- a/internal/api/provider/custom_oauth_claims_test.go +++ b/internal/api/provider/custom_oauth_claims_test.go @@ -191,7 +191,7 @@ func TestCustomOIDCProvider_GetUserData_UserinfoAllowlist(t *testing.T) { provider, err := NewCustomOIDCProvider( context.Background(), "client-id", "client-secret", "https://myapp.com/callback", - []string{"openid"}, server.URL, server.URL + "/.well-known/openid-configuration", false, + []string{"openid"}, server.URL, server.URL+"/.well-known/openid-configuration", false, nil, nil, nil, []string{"mail", "sn"}, newTestOIDCProviderCache(t, 0), @@ -257,7 +257,7 @@ func TestCustomOIDCProvider_GetUserData_IDTokenAllowlist(t *testing.T) { provider, err := NewCustomOIDCProvider( context.Background(), "client-id", "client-secret", "https://myapp.com/callback", - []string{"openid"}, server.URL, server.URL + "/.well-known/openid-configuration", false, + []string{"openid"}, server.URL, server.URL+"/.well-known/openid-configuration", false, nil, nil, nil, []string{"groups", "org_id"}, newTestOIDCProviderCache(t, 0), diff --git a/internal/api/provider/provider.go b/internal/api/provider/provider.go index c3ab0781f..efbd08a18 100644 --- a/internal/api/provider/provider.go +++ b/internal/api/provider/provider.go @@ -92,24 +92,24 @@ type Claims struct { Exp float64 `json:"exp,omitempty" structs:"exp,omitempty"` // Default profile claims - Name string `json:"name,omitempty" structs:"name,omitempty"` - FamilyName string `json:"family_name,omitempty" structs:"family_name,omitempty"` - GivenName string `json:"given_name,omitempty" structs:"given_name,omitempty"` - MiddleName string `json:"middle_name,omitempty" structs:"middle_name,omitempty"` - NickName string `json:"nickname,omitempty" structs:"nickname,omitempty"` - PreferredUsername string `json:"preferred_username,omitempty" structs:"preferred_username,omitempty"` - Profile string `json:"profile,omitempty" structs:"profile,omitempty"` - Picture string `json:"picture,omitempty" structs:"picture,omitempty"` - Website string `json:"website,omitempty" structs:"website,omitempty"` - Gender string `json:"gender,omitempty" structs:"gender,omitempty"` - Birthdate string `json:"birthdate,omitempty" structs:"birthdate,omitempty"` - ZoneInfo string `json:"zoneinfo,omitempty" structs:"zoneinfo,omitempty"` - Locale string `json:"locale,omitempty" structs:"locale,omitempty"` + Name string `json:"name,omitempty" structs:"name,omitempty"` + FamilyName string `json:"family_name,omitempty" structs:"family_name,omitempty"` + GivenName string `json:"given_name,omitempty" structs:"given_name,omitempty"` + MiddleName string `json:"middle_name,omitempty" structs:"middle_name,omitempty"` + NickName string `json:"nickname,omitempty" structs:"nickname,omitempty"` + PreferredUsername string `json:"preferred_username,omitempty" structs:"preferred_username,omitempty"` + Profile string `json:"profile,omitempty" structs:"profile,omitempty"` + Picture string `json:"picture,omitempty" structs:"picture,omitempty"` + Website string `json:"website,omitempty" structs:"website,omitempty"` + Gender string `json:"gender,omitempty" structs:"gender,omitempty"` + Birthdate string `json:"birthdate,omitempty" structs:"birthdate,omitempty"` + ZoneInfo string `json:"zoneinfo,omitempty" structs:"zoneinfo,omitempty"` + Locale string `json:"locale,omitempty" structs:"locale,omitempty"` UpdatedAt *UnixTimeOrString `json:"updated_at,omitempty" structs:"updated_at,omitempty"` - Email string `json:"email,omitempty" structs:"email,omitempty"` - EmailVerified bool `json:"email_verified,omitempty" structs:"email_verified"` - Phone string `json:"phone,omitempty" structs:"phone,omitempty"` - PhoneVerified bool `json:"phone_verified,omitempty" structs:"phone_verified"` + Email string `json:"email,omitempty" structs:"email,omitempty"` + EmailVerified bool `json:"email_verified,omitempty" structs:"email_verified"` + Phone string `json:"phone,omitempty" structs:"phone,omitempty"` + PhoneVerified bool `json:"phone_verified,omitempty" structs:"phone_verified"` // Custom profile claims that are provider specific CustomClaims map[string]interface{} `json:"custom_claims,omitempty" structs:"custom_claims,omitempty"` diff --git a/internal/api/reauthenticate.go b/internal/api/reauthenticate.go index 687b559cd..6c59c46d8 100644 --- a/internal/api/reauthenticate.go +++ b/internal/api/reauthenticate.go @@ -3,6 +3,7 @@ package api import ( "net/http" + "github.com/sirupsen/logrus" "github.com/supabase/auth/internal/api/apierrors" "github.com/supabase/auth/internal/api/sms_provider" "github.com/supabase/auth/internal/conf" @@ -72,9 +73,17 @@ func (a *API) verifyReauthentication(nonce string, tx *storage.Connection, confi return apierrors.NewUnprocessableEntityError(apierrors.ErrorCodeReauthenticationNotValid, InvalidNonceMessage) } var isValid bool + salt := config.Security.TokenHashSalt if user.GetEmail() != "" { - tokenHash := crypto.GenerateTokenHash(user.GetEmail(), nonce) + tokenHash := crypto.GenerateTokenHash(salt, user.GetEmail(), nonce) isValid = isOtpValid(tokenHash, user.ReauthenticationToken, user.ReauthenticationSentAt, config.Mailer.OtpExp) + if !isValid && salt != "" { + legacyHash := crypto.GenerateTokenHash("", user.GetEmail(), nonce) + if isOtpValid(legacyHash, user.ReauthenticationToken, user.ReauthenticationSentAt, config.Mailer.OtpExp) { + isValid = true + logrus.Info("reauthentication token verified using legacy (pre-salt) token hash fallback") + } + } } else if user.GetPhone() != "" { if config.Sms.IsTwilioVerifyProvider() { smsProvider, _ := sms_provider.GetSmsProvider(*config) @@ -83,8 +92,15 @@ func (a *API) verifyReauthentication(nonce string, tx *storage.Connection, confi } return nil } else { - tokenHash := crypto.GenerateTokenHash(user.GetPhone(), nonce) + tokenHash := crypto.GenerateTokenHash(salt, user.GetPhone(), nonce) isValid = isOtpValid(tokenHash, user.ReauthenticationToken, user.ReauthenticationSentAt, config.Sms.OtpExp) + if !isValid && salt != "" { + legacyHash := crypto.GenerateTokenHash("", user.GetPhone(), nonce) + if isOtpValid(legacyHash, user.ReauthenticationToken, user.ReauthenticationSentAt, config.Sms.OtpExp) { + isValid = true + logrus.Info("reauthentication token verified using legacy (pre-salt) token hash fallback") + } + } } } else { return apierrors.NewUnprocessableEntityError(apierrors.ErrorCodeReauthenticationNotValid, "Reauthentication requires an email or a phone number") diff --git a/internal/api/settings.go b/internal/api/settings.go index 7fb21a05f..80c481e9b 100644 --- a/internal/api/settings.go +++ b/internal/api/settings.go @@ -32,14 +32,14 @@ type ProviderSettings struct { } type Settings struct { - ExternalProviders ProviderSettings `json:"external"` - DisableSignup bool `json:"disable_signup"` - MailerAutoconfirm bool `json:"mailer_autoconfirm"` - PhoneAutoconfirm bool `json:"phone_autoconfirm"` - SmsProvider string `json:"sms_provider"` - SAMLEnabled bool `json:"saml_enabled"` - SAMLPrivateKeyNextConfigured bool `json:"saml_private_key_next_configured"` - PasskeysEnabled bool `json:"passkeys_enabled"` + ExternalProviders ProviderSettings `json:"external"` + DisableSignup bool `json:"disable_signup"` + MailerAutoconfirm bool `json:"mailer_autoconfirm"` + PhoneAutoconfirm bool `json:"phone_autoconfirm"` + SmsProvider string `json:"sms_provider"` + SAMLEnabled bool `json:"saml_enabled"` + SAMLPrivateKeyNextConfigured bool `json:"saml_private_key_next_configured"` + PasskeysEnabled bool `json:"passkeys_enabled"` } func (a *API) Settings(w http.ResponseWriter, r *http.Request) error { @@ -74,9 +74,9 @@ func (a *API) Settings(w http.ResponseWriter, r *http.Request) error { Phone: config.External.Phone.Enabled, Zoom: config.External.Zoom.Enabled, }, - DisableSignup: config.DisableSignup, - MailerAutoconfirm: config.Mailer.Autoconfirm, - PhoneAutoconfirm: config.Sms.Autoconfirm, + DisableSignup: config.DisableSignup, + MailerAutoconfirm: config.Mailer.Autoconfirm, + PhoneAutoconfirm: config.Sms.Autoconfirm, SmsProvider: config.Sms.Provider, SAMLEnabled: config.SAML.Enabled, SAMLPrivateKeyNextConfigured: config.SAML.CertificateNext != nil, diff --git a/internal/api/user_test.go b/internal/api/user_test.go index 352b7fcc1..7ee0279b5 100644 --- a/internal/api/user_test.go +++ b/internal/api/user_test.go @@ -586,7 +586,7 @@ func (ts *UserTestSuite) TestUserUpdatePasswordReauthentication() { require.NotEmpty(ts.T(), u.ReauthenticationSentAt) // update reauthentication token to a known token - u.ReauthenticationToken = crypto.GenerateTokenHash(u.GetEmail(), "123456") + u.ReauthenticationToken = crypto.GenerateTokenHash("", u.GetEmail(), "123456") require.NoError(ts.T(), ts.API.db.Update(u)) // update password with reauthentication token diff --git a/internal/api/verify.go b/internal/api/verify.go index 212d7388e..14affbfd0 100644 --- a/internal/api/verify.go +++ b/internal/api/verify.go @@ -46,6 +46,23 @@ type VerifyParams struct { Email string `json:"email"` Phone string `json:"phone"` RedirectTo string `json:"redirect_to"` + + // legacyTokenHash is only set when TokenHash was recomputed from a raw + // Token in Validate (never for a passthrough token_hash/magic-link + // value) and a Security.TokenHashSalt is configured. It holds the same + // hash computed under the legacy unsalted formula, so verification can + // fall back to it for tokens issued before the salt was configured. + legacyTokenHash string +} + +// tokenHashCandidates returns every token hash that should be accepted for +// this request: just TokenHash, or TokenHash plus the legacy fallback when +// one was computed. +func (p *VerifyParams) tokenHashCandidates() []string { + if p.legacyTokenHash == "" { + return []string{p.TokenHash} + } + return []string{p.TokenHash, p.legacyTokenHash} } func (p *VerifyParams) Validate(r *http.Request, a *API) error { @@ -65,18 +82,25 @@ func (p *VerifyParams) Validate(r *http.Request, a *API) error { return apierrors.NewBadRequestError(apierrors.ErrorCodeValidationFailed, "Verify requires either a token or a token hash") } if p.Token != "" { + salt := a.config.Security.TokenHashSalt if isPhoneOtpVerification(p) { p.Phone, err = validatePhone(p.Phone) if err != nil { return err } - p.TokenHash = crypto.GenerateTokenHash(p.Phone, p.Token) + p.TokenHash = crypto.GenerateTokenHash(salt, p.Phone, p.Token) + if salt != "" { + p.legacyTokenHash = crypto.GenerateTokenHash("", p.Phone, p.Token) + } } else if isEmailOtpVerification(p) { p.Email, err = a.validateEmail(p.Email) if err != nil { return apierrors.NewUnprocessableEntityError(apierrors.ErrorCodeValidationFailed, "Invalid email format").WithInternalError(err) } - p.TokenHash = crypto.GenerateTokenHash(p.Email, p.Token) + p.TokenHash = crypto.GenerateTokenHash(salt, p.Email, p.Token) + if salt != "" { + p.legacyTokenHash = crypto.GenerateTokenHash("", p.Email, p.Token) + } } else { return apierrors.NewBadRequestError(apierrors.ErrorCodeValidationFailed, "Only an email address or phone number should be provided on verify") } @@ -549,24 +573,43 @@ func (a *API) emailChangeVerify(r *http.Request, conn *storage.Connection, param user.EmailChangeConfirmStatus == zeroConfirmation && user.GetEmail() != "" { err := conn.Transaction(func(tx *storage.Connection) error { - currentOTT, terr := models.FindOneTimeToken(tx, params.TokenHash, models.EmailChangeTokenCurrent) + baseCandidates := params.tokenHashCandidates() + candidates := make([]string, 0, len(baseCandidates)*2) + for _, c := range baseCandidates { + candidates = append(candidates, c, PKCEPrefix+c) + } + + currentOTT, terr := models.FindOneTimeToken(tx, candidates, models.EmailChangeTokenCurrent) if terr != nil && !models.IsNotFoundError(terr) { return terr } - newOTT, terr := models.FindOneTimeToken(tx, params.TokenHash, models.EmailChangeTokenNew) + newOTT, terr := models.FindOneTimeToken(tx, candidates, models.EmailChangeTokenNew) if terr != nil && !models.IsNotFoundError(terr) { return terr } user.EmailChangeConfirmStatus = singleConfirmation - if params.Token == user.EmailChangeTokenCurrent || params.TokenHash == user.EmailChangeTokenCurrent || (currentOTT != nil && params.TokenHash == currentOTT.TokenHash) { + // matchesStoredToken accepts the raw token, the primary + // (salted) hash, or - during a salt cut-over - the legacy + // unsalted hash, against a stored token field. + matchesStoredToken := func(stored string) bool { + if stored == "" { + return false + } + if params.Token == stored || params.TokenHash == stored { + return true + } + return params.legacyTokenHash != "" && params.legacyTokenHash == stored + } + + if matchesStoredToken(user.EmailChangeTokenCurrent) || currentOTT != nil { user.EmailChangeTokenCurrent = "" 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) { + } else if matchesStoredToken(user.EmailChangeTokenNew) || newOTT != nil { user.EmailChangeTokenNew = "" if terr := models.ClearOneTimeTokenForUser(tx, user.ID, models.EmailChangeTokenNew); terr != nil { return terr @@ -701,7 +744,6 @@ func (a *API) verifyUserAndToken(conn *storage.Connection, params *VerifyParams, var user *models.User var err error - tokenHash := params.TokenHash switch params.Type { case phoneChangeVerification: @@ -711,7 +753,7 @@ func (a *API) verifyUserAndToken(conn *storage.Connection, params *VerifyParams, case mail.EmailChangeVerification: // Since the email change could be trigger via the implicit or PKCE flow, // the query used has to also check if the token saved in the db contains the pkce_ prefix - user, err = models.FindUserForEmailChange(conn, params.Email, tokenHash, aud, config.Mailer.SecureEmailChangeEnabled) + user, err = models.FindUserForEmailChange(conn, params.Email, params.tokenHashCandidates(), aud, config.Mailer.SecureEmailChangeEnabled) default: user, err = models.FindUserByEmailAndAudience(conn, params.Email, aud) } @@ -733,22 +775,22 @@ func (a *API) verifyUserAndToken(conn *storage.Connection, params *VerifyParams, 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) { + if otpValidWithFallback(params, user.ConfirmationToken, user.ConfirmationSentAt, config.Mailer.OtpExp) { isValid = true params.Type = mail.SignupVerification - } else if isOtpValid(tokenHash, user.RecoveryToken, user.RecoverySentAt, config.Mailer.OtpExp) { + } else if otpValidWithFallback(params, user.RecoveryToken, user.RecoverySentAt, config.Mailer.OtpExp) { isValid = true params.Type = mail.MagicLinkVerification } else { isValid = false } case mail.SignupVerification, mail.InviteVerification: - isValid = isOtpValid(tokenHash, user.ConfirmationToken, user.ConfirmationSentAt, config.Mailer.OtpExp) + isValid = otpValidWithFallback(params, user.ConfirmationToken, user.ConfirmationSentAt, config.Mailer.OtpExp) case mail.RecoveryVerification, mail.MagicLinkVerification: - isValid = isOtpValid(tokenHash, user.RecoveryToken, user.RecoverySentAt, config.Mailer.OtpExp) + isValid = otpValidWithFallback(params, user.RecoveryToken, user.RecoverySentAt, config.Mailer.OtpExp) case mail.EmailChangeVerification: - isValid = isOtpValid(tokenHash, user.EmailChangeTokenCurrent, user.EmailChangeSentAt, config.Mailer.OtpExp) || - isOtpValid(tokenHash, user.EmailChangeTokenNew, user.EmailChangeSentAt, config.Mailer.OtpExp) + isValid = otpValidWithFallback(params, user.EmailChangeTokenCurrent, user.EmailChangeSentAt, config.Mailer.OtpExp) || + otpValidWithFallback(params, user.EmailChangeTokenNew, user.EmailChangeSentAt, config.Mailer.OtpExp) case phoneChangeVerification, smsVerification: if testOTP, ok := config.Sms.GetTestOTP(params.Phone, time.Now()); ok { if params.Token == testOTP { @@ -771,7 +813,7 @@ func (a *API) verifyUserAndToken(conn *storage.Connection, params *VerifyParams, } return user, nil } - isValid = isOtpValid(tokenHash, expectedToken, sentAt, config.Sms.OtpExp) + isValid = otpValidWithFallback(params, expectedToken, sentAt, config.Sms.OtpExp) } if !isValid { @@ -788,6 +830,21 @@ func isOtpValid(actual, expected string, sentAt *time.Time, otpExp uint) bool { return !isOtpExpired(sentAt, otpExp) && ((actual == expected) || ("pkce_"+actual == expected)) } +// otpValidWithFallback checks the primary (salted) token hash against +// expected and, only while a salt is configured, also tries the legacy +// unsalted hash as a fallback, so tokens issued before Security.TokenHashSalt +// was set keep verifying until they expire or are consumed. +func otpValidWithFallback(params *VerifyParams, expected string, sentAt *time.Time, otpExp uint) bool { + if isOtpValid(params.TokenHash, expected, sentAt, otpExp) { + return true + } + if params.legacyTokenHash != "" && isOtpValid(params.legacyTokenHash, expected, sentAt, otpExp) { + logrus.WithField("type", params.Type).Info("one-time token verified using legacy (pre-salt) token hash fallback") + return true + } + return false +} + func isOtpExpired(sentAt *time.Time, otpExp uint) bool { return time.Now().After(sentAt.Add(time.Second * time.Duration(otpExp))) // #nosec G115 } diff --git a/internal/api/verify_test.go b/internal/api/verify_test.go index a75df9c15..36b649e97 100644 --- a/internal/api/verify_test.go +++ b/internal/api/verify_test.go @@ -904,7 +904,7 @@ func (ts *VerifyTestSuite) TestVerifyValidOtp() { }, expected: expected{ code: http.StatusOK, - tokenHash: crypto.GenerateTokenHash(u.GetPhone(), "123456"), + tokenHash: crypto.GenerateTokenHash("", u.GetPhone(), "123456"), }, }, { @@ -917,7 +917,7 @@ func (ts *VerifyTestSuite) TestVerifyValidOtp() { }, expected: expected{ code: http.StatusOK, - tokenHash: crypto.GenerateTokenHash(u.GetEmail(), "123456"), + tokenHash: crypto.GenerateTokenHash("", u.GetEmail(), "123456"), }, }, { @@ -925,11 +925,11 @@ func (ts *VerifyTestSuite) TestVerifyValidOtp() { sentTime: time.Now(), body: map[string]interface{}{ "type": mail.SignupVerification, - "token_hash": crypto.GenerateTokenHash(u.GetEmail(), "123456"), + "token_hash": crypto.GenerateTokenHash("", u.GetEmail(), "123456"), }, expected: expected{ code: http.StatusOK, - tokenHash: crypto.GenerateTokenHash(u.GetEmail(), "123456"), + tokenHash: crypto.GenerateTokenHash("", u.GetEmail(), "123456"), }, }, { @@ -942,7 +942,7 @@ func (ts *VerifyTestSuite) TestVerifyValidOtp() { }, expected: expected{ code: http.StatusOK, - tokenHash: crypto.GenerateTokenHash(u.GetEmail(), "123456"), + tokenHash: crypto.GenerateTokenHash("", u.GetEmail(), "123456"), }, }, { @@ -955,7 +955,7 @@ func (ts *VerifyTestSuite) TestVerifyValidOtp() { }, expected: expected{ code: http.StatusOK, - tokenHash: crypto.GenerateTokenHash(u.GetEmail(), "123456"), + tokenHash: crypto.GenerateTokenHash("", u.GetEmail(), "123456"), }, }, { @@ -968,7 +968,7 @@ func (ts *VerifyTestSuite) TestVerifyValidOtp() { }, expected: expected{ code: http.StatusOK, - tokenHash: crypto.GenerateTokenHash(u.GetEmail(), "123456"), + tokenHash: crypto.GenerateTokenHash("", u.GetEmail(), "123456"), }, }, { @@ -981,7 +981,7 @@ func (ts *VerifyTestSuite) TestVerifyValidOtp() { }, expected: expected{ code: http.StatusOK, - tokenHash: crypto.GenerateTokenHash(u.EmailChange, "123456"), + tokenHash: crypto.GenerateTokenHash("", u.EmailChange, "123456"), }, }, { @@ -994,7 +994,7 @@ func (ts *VerifyTestSuite) TestVerifyValidOtp() { }, expected: expected{ code: http.StatusOK, - tokenHash: crypto.GenerateTokenHash(u.PhoneChange, "123456"), + tokenHash: crypto.GenerateTokenHash("", u.PhoneChange, "123456"), }, }, { @@ -1002,11 +1002,11 @@ func (ts *VerifyTestSuite) TestVerifyValidOtp() { sentTime: time.Now(), body: map[string]interface{}{ "type": mail.EmailChangeVerification, - "token_hash": crypto.GenerateTokenHash(u.EmailChange, "123456"), + "token_hash": crypto.GenerateTokenHash("", u.EmailChange, "123456"), }, expected: expected{ code: http.StatusOK, - tokenHash: crypto.GenerateTokenHash(u.EmailChange, "123456"), + tokenHash: crypto.GenerateTokenHash("", u.EmailChange, "123456"), }, }, { @@ -1014,11 +1014,11 @@ func (ts *VerifyTestSuite) TestVerifyValidOtp() { sentTime: time.Now(), body: map[string]interface{}{ "type": mail.EmailOTPVerification, - "token_hash": crypto.GenerateTokenHash(u.GetEmail(), "123456"), + "token_hash": crypto.GenerateTokenHash("", u.GetEmail(), "123456"), }, expected: expected{ code: http.StatusOK, - tokenHash: crypto.GenerateTokenHash(u.GetEmail(), "123456"), + tokenHash: crypto.GenerateTokenHash("", u.GetEmail(), "123456"), }, }, } @@ -1068,8 +1068,8 @@ func (ts *VerifyTestSuite) TestSecureEmailChangeWithTokenHash() { u.EmailChange = "new@example.com" require.NoError(ts.T(), ts.API.db.Update(u)) - currentEmailChangeToken := crypto.GenerateTokenHash(string(u.Email), "123456") - newEmailChangeToken := crypto.GenerateTokenHash(u.EmailChange, "123456") + currentEmailChangeToken := crypto.GenerateTokenHash("", string(u.Email), "123456") + newEmailChangeToken := crypto.GenerateTokenHash("", u.EmailChange, "123456") cases := []struct { desc string @@ -1141,6 +1141,162 @@ func (ts *VerifyTestSuite) TestSecureEmailChangeWithTokenHash() { } } +// TestVerifyOtpTokenHashSaltCutover exercises Security.TokenHashSalt's +// cut-over behavior for the raw-OTP verification path (verifyUserAndToken), +// which recomputes the token hash from the submitted otp+email/phone rather +// than passing through an already-hashed value. +func (ts *VerifyTestSuite) TestVerifyOtpTokenHashSaltCutover() { + const testSalt = "test-salt-value-1234567890" + const otp = "654321" + + u, err := models.FindUserByEmailAndAudience(ts.API.db, "test@example.com", ts.Config.JWT.Aud) + require.NoError(ts.T(), err) + + setupConfirmationToken := func(tokenHash string, sentAt time.Time) { + require.NoError(ts.T(), models.ClearAllOneTimeTokensForUser(ts.API.db, u.ID)) + u.ConfirmationToken = tokenHash + sa := sentAt + u.ConfirmationSentAt = &sa + require.NoError(ts.T(), ts.API.db.Update(u)) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, "relates_to not used", tokenHash, models.ConfirmationToken)) + } + + postVerify := func() int { + body := map[string]interface{}{ + "type": mail.SignupVerification, + "token": otp, + "email": u.GetEmail(), + } + var buffer bytes.Buffer + require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(body)) + 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) + return w.Code + } + + ts.Run("new token verifies under the active salt", func() { + ts.Config.Security.TokenHashSalt = testSalt + defer func() { ts.Config.Security.TokenHashSalt = "" }() + + setupConfirmationToken(crypto.GenerateTokenHash(testSalt, u.GetEmail(), otp), time.Now()) + + assert.Equal(ts.T(), http.StatusOK, postVerify()) + }) + + ts.Run("pre-existing legacy (unsalted) token still verifies once a salt is configured", func() { + setupConfirmationToken(crypto.GenerateTokenHash("", u.GetEmail(), otp), time.Now()) + + ts.Config.Security.TokenHashSalt = testSalt + defer func() { ts.Config.Security.TokenHashSalt = "" }() + + assert.Equal(ts.T(), http.StatusOK, postVerify()) + }) + + ts.Run("legacy fallback does not resurrect an expired token", func() { + // well beyond Mailer.OtpExp's default of 1 day + setupConfirmationToken(crypto.GenerateTokenHash("", u.GetEmail(), otp), time.Now().Add(-72*time.Hour)) + + ts.Config.Security.TokenHashSalt = testSalt + defer func() { ts.Config.Security.TokenHashSalt = "" }() + + assert.Equal(ts.T(), http.StatusForbidden, postVerify()) + }) +} + +// TestEmailChangeVerifyRawOTPSaltCutover is a regression test: emailChangeVerify +// (used when Mailer.SecureEmailChangeEnabled) does its own direct +// models.FindOneTimeToken lookups separate from verifyUserAndToken's user +// lookup. Both must honor the same salted/legacy candidate hashes, otherwise +// a legacy-salt raw-OTP email-change confirmation would authenticate the +// user but fail to find/clear the matching one-time-token row, corrupting +// EmailChangeConfirmStatus bookkeeping. +func (ts *VerifyTestSuite) TestEmailChangeVerifyRawOTPSaltCutover() { + const testSalt = "test-salt-value-1234567890" + + ts.Config.Mailer.SecureEmailChangeEnabled = true + + postVerify := func(otp, email string) int { + body := map[string]interface{}{ + "type": mail.EmailChangeVerification, + "token": otp, + "email": email, + } + var buffer bytes.Buffer + require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(body)) + 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) + return w.Code + } + + ts.Run("legacy-salt current-email-change token clears its one-time-token row", func() { + u, err := models.FindUserByEmailAndAudience(ts.API.db, "test@example.com", ts.Config.JWT.Aud) + require.NoError(ts.T(), err) + + const otp = "112233" + legacyHash := crypto.GenerateTokenHash("", u.GetEmail(), otp) + + require.NoError(ts.T(), models.ClearAllOneTimeTokensForUser(ts.API.db, u.ID)) + u.EmailChange = "new-current-arm@example.com" + u.EmailChangeTokenCurrent = legacyHash + u.EmailChangeTokenNew = "" + u.EmailChangeConfirmStatus = zeroConfirmation + now := time.Now() + u.EmailChangeSentAt = &now + require.NoError(ts.T(), ts.API.db.Update(u)) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, "relates_to not used", legacyHash, models.EmailChangeTokenCurrent)) + + ts.Config.Security.TokenHashSalt = testSalt + defer func() { ts.Config.Security.TokenHashSalt = "" }() + + require.Equal(ts.T(), http.StatusOK, postVerify(otp, u.GetEmail())) + + reloaded, err := models.FindUserByEmailAndAudience(ts.API.db, u.GetEmail(), ts.Config.JWT.Aud) + require.NoError(ts.T(), err) + assert.Empty(ts.T(), reloaded.EmailChangeTokenCurrent) + assert.Equal(ts.T(), singleConfirmation, reloaded.EmailChangeConfirmStatus) + + _, err = models.FindOneTimeToken(ts.API.db, []string{legacyHash}, models.EmailChangeTokenCurrent) + assert.True(ts.T(), models.IsNotFoundError(err), "expected the matching one-time-token row to have been cleared") + }) + + ts.Run("legacy-salt PKCE-prefixed new-email-change token clears its one-time-token row", func() { + u, err := models.FindUserByEmailAndAudience(ts.API.db, "test@example.com", ts.Config.JWT.Aud) + require.NoError(ts.T(), err) + + const otp = "998877" + newEmail := "new-pkce-arm@example.com" + legacyHash := crypto.GenerateTokenHash("", newEmail, otp) + prefixedLegacyHash := "pkce_" + legacyHash + + require.NoError(ts.T(), models.ClearAllOneTimeTokensForUser(ts.API.db, u.ID)) + u.EmailChange = newEmail + u.EmailChangeTokenCurrent = "" + u.EmailChangeTokenNew = prefixedLegacyHash + u.EmailChangeConfirmStatus = zeroConfirmation + now := time.Now() + u.EmailChangeSentAt = &now + require.NoError(ts.T(), ts.API.db.Update(u)) + require.NoError(ts.T(), models.CreateOneTimeToken(ts.API.db, u.ID, "relates_to not used", prefixedLegacyHash, models.EmailChangeTokenNew)) + + ts.Config.Security.TokenHashSalt = testSalt + defer func() { ts.Config.Security.TokenHashSalt = "" }() + + require.Equal(ts.T(), http.StatusOK, postVerify(otp, newEmail)) + + reloaded, err := models.FindUserByEmailAndAudience(ts.API.db, u.GetEmail(), ts.Config.JWT.Aud) + require.NoError(ts.T(), err) + assert.Empty(ts.T(), reloaded.EmailChangeTokenNew) + assert.Equal(ts.T(), singleConfirmation, reloaded.EmailChangeConfirmStatus) + + _, err = models.FindOneTimeToken(ts.API.db, []string{prefixedLegacyHash}, models.EmailChangeTokenNew) + assert.True(ts.T(), models.IsNotFoundError(err), "expected the matching one-time-token row to have been cleared") + }) +} + func (ts *VerifyTestSuite) TestPrepRedirectURL() { escapedMessage := url.QueryEscape(singleConfirmationAccepted) cases := []struct { @@ -1454,7 +1610,7 @@ func (ts *VerifyTestSuite) TestVerifyPhoneChangeSendsNotificationEmailEnabled() "phone": u.PhoneChange, } sentTime := time.Now() - expectedTokenHash := crypto.GenerateTokenHash(u.PhoneChange, "123456") + expectedTokenHash := crypto.GenerateTokenHash("", u.PhoneChange, "123456") // Get the mock mailer and reset it mockMailer, ok := ts.Mailer.(*mockclient.MockMailer) @@ -1504,7 +1660,7 @@ func (ts *VerifyTestSuite) TestVerifyPhoneChangeSendsNotificationEmailEnabled_No "phone": u.PhoneChange, } sentTime := time.Now() - expectedTokenHash := crypto.GenerateTokenHash(u.PhoneChange, "123456") + expectedTokenHash := crypto.GenerateTokenHash("", u.PhoneChange, "123456") // Get the mock mailer and reset it mockMailer, ok := ts.Mailer.(*mockclient.MockMailer) @@ -1551,7 +1707,7 @@ func (ts *VerifyTestSuite) TestVerifyPhoneChangeSendsNotificationEmailDisabled() "phone": u.PhoneChange, } sentTime := time.Now() - expectedTokenHash := crypto.GenerateTokenHash(u.PhoneChange, "123456") + expectedTokenHash := crypto.GenerateTokenHash("", u.PhoneChange, "123456") // Get the mock mailer and reset it mockMailer, ok := ts.Mailer.(*mockclient.MockMailer) diff --git a/internal/conf/configuration.go b/internal/conf/configuration.go index 2d3ca6b5b..240cdd350 100644 --- a/internal/conf/configuration.go +++ b/internal/conf/configuration.go @@ -870,6 +870,8 @@ type SecurityConfiguration struct { ManualLinkingEnabled bool `json:"manual_linking_enabled" split_words:"true" default:"false"` SbForwardedForEnabled bool `json:"sb_forwarded_for_enabled" split_words:"true" default:"false"` + TokenHashSalt string `json:"otp_token_hash_salt" split_words:"true"` + DBEncryption DatabaseEncryptionConfiguration `json:"database_encryption" split_words:"true"` } @@ -890,6 +892,10 @@ func (c *SecurityConfiguration) Validate() error { return fmt.Errorf("refresh token upgrade percentage must be between 0 and 100, but was %v", c.RefreshTokenUpgradePercentage) } + if c.TokenHashSalt != "" && len(c.TokenHashSalt) < 16 { + logrus.Warn("GOTRUE_SECURITY_OTP_TOKEN_HASH_SALT is set but shorter than 16 characters, consider using a longer, random value") + } + return nil } diff --git a/internal/conf/confload/confload.go b/internal/conf/confload/confload.go index aa0170cc0..2315ba882 100644 --- a/internal/conf/confload/confload.go +++ b/internal/conf/confload/confload.go @@ -134,4 +134,4 @@ func loadEnvironment(filename string) error { } } return err -} \ No newline at end of file +} diff --git a/internal/crypto/crypto.go b/internal/crypto/crypto.go index 1d5417024..8aaf7770d 100644 --- a/internal/crypto/crypto.go +++ b/internal/crypto/crypto.go @@ -42,8 +42,12 @@ func generateOtp(r io.Reader, digits int) string { return otp } -func GenerateTokenHash(emailOrPhone, otp string) string { - return fmt.Sprintf("%x", sha256.Sum224([]byte(emailOrPhone+otp))) +// GenerateTokenHash computes a one-time-token/OTP hash. salt should be the +// server's configured Security.TokenHashSalt (or "" to reproduce the legacy +// unsalted formula, used as a fallback while verifying tokens issued before +// a salt was configured). +func GenerateTokenHash(salt, emailOrPhone, otp string) string { + return fmt.Sprintf("%x", sha256.Sum224([]byte(salt+emailOrPhone+otp))) } // Generated a random secure integer from [0, max[ diff --git a/internal/crypto/crypto_test.go b/internal/crypto/crypto_test.go index 0344a1b95..a3c4386f4 100644 --- a/internal/crypto/crypto_test.go +++ b/internal/crypto/crypto_test.go @@ -77,6 +77,29 @@ func TestGenerateOtp(t *testing.T) { } } +func TestGenerateTokenHash(t *testing.T) { + // empty salt reproduces the legacy unsalted formula exactly + assert.Equal(t, GenerateTokenHash("", "test@example.com", "123456"), GenerateTokenHash("", "test@example.com", "123456")) + + // a different salt produces a different hash for identical inputs + assert.NotEqual(t, + GenerateTokenHash("salt-a", "test@example.com", "123456"), + GenerateTokenHash("salt-b", "test@example.com", "123456"), + ) + + // salted and unsalted hashes differ for identical inputs + assert.NotEqual(t, + GenerateTokenHash("", "test@example.com", "123456"), + GenerateTokenHash("some-salt", "test@example.com", "123456"), + ) + + // deterministic for identical (salt, emailOrPhone, otp) + assert.Equal(t, + GenerateTokenHash("some-salt", "test@example.com", "123456"), + GenerateTokenHash("some-salt", "test@example.com", "123456"), + ) +} + func TestEncryptedStringPositive(t *testing.T) { id := uuid.Must(uuid.NewV4()).String() diff --git a/internal/models/one_time_token.go b/internal/models/one_time_token.go index 34e4309f3..ff4bd3ea3 100644 --- a/internal/models/one_time_token.go +++ b/internal/models/one_time_token.go @@ -152,22 +152,42 @@ func CreateOneTimeToken(tx *storage.Connection, userID uuid.UUID, relatesTo, tok return nil } -func FindOneTimeToken(tx *storage.Connection, tokenHash string, tokenTypes ...OneTimeTokenType) (*OneTimeToken, error) { +// FindOneTimeToken looks up a token by any of the provided tokenHashes +// (accepts more than one so a caller can verify a hash generated under the +// current formula alongside a legacy fallback hash, without needing +// multiple round trips). +func FindOneTimeToken(tx *storage.Connection, tokenHashes []string, tokenTypes ...OneTimeTokenType) (*OneTimeToken, error) { + if len(tokenHashes) == 0 { + panic("at least one token hash is required") + } + oneTimeToken := &OneTimeToken{} query := tx.Eager().Q() switch len(tokenTypes) { case 2: - query = query.Where("(token_type = ? or token_type = ?) and token_hash = ?", tokenTypes[0], tokenTypes[1], tokenHash) // #nosec G602 + query = query.Where("(token_type = ? or token_type = ?)", tokenTypes[0], tokenTypes[1]) // #nosec G602 case 1: - query = query.Where("token_type = ? and token_hash = ?", tokenTypes[0], tokenHash) + query = query.Where("token_type = ?", tokenTypes[0]) default: panic("at most 2 token types are accepted") } + // A separate Where() call containing ONLY the hash args: pop's "in (?)" + // auto-expansion sizes itself off every arg passed to that SAME Where() + // call, so mixing it with the token_type args above caused a "expected N + // arguments, got M" driver error whenever there was exactly one + // candidate hash (the single "?" placeholder would additionally match + // pop's own regex-based expansion, which used the combined arg count). + hashArgs := make([]interface{}, len(tokenHashes)) + for i, hash := range tokenHashes { + hashArgs[i] = hash + } + query = query.Where("token_hash in (?)", hashArgs...) + if err := query.First(oneTimeToken); err != nil { if errors.Cause(err) == sql.ErrNoRows { return nil, OneTimeTokenNotFoundError{} @@ -181,7 +201,7 @@ func FindOneTimeToken(tx *storage.Connection, tokenHash string, tokenTypes ...On // FindUserByConfirmationToken finds users with the matching confirmation token. func FindUserByConfirmationOrRecoveryToken(tx *storage.Connection, token string) (*User, error) { - ott, err := FindOneTimeToken(tx, token, ConfirmationToken, RecoveryToken) + ott, err := FindOneTimeToken(tx, []string{token}, ConfirmationToken, RecoveryToken) if err != nil { return nil, err } @@ -191,7 +211,7 @@ func FindUserByConfirmationOrRecoveryToken(tx *storage.Connection, token string) // FindUserByConfirmationToken finds users with the matching confirmation token. func FindUserByConfirmationToken(tx *storage.Connection, token string) (*User, error) { - ott, err := FindOneTimeToken(tx, token, ConfirmationToken) + ott, err := FindOneTimeToken(tx, []string{token}, ConfirmationToken) if err != nil { return nil, err } @@ -201,7 +221,7 @@ func FindUserByConfirmationToken(tx *storage.Connection, token string) (*User, e // FindUserByRecoveryToken finds a user with the matching recovery token. func FindUserByRecoveryToken(tx *storage.Connection, token string) (*User, error) { - ott, err := FindOneTimeToken(tx, token, RecoveryToken) + ott, err := FindOneTimeToken(tx, []string{token}, RecoveryToken) if err != nil { return nil, err } @@ -211,7 +231,7 @@ func FindUserByRecoveryToken(tx *storage.Connection, token string) (*User, error // FindUserByEmailChangeToken finds a user with the matching email change token. func FindUserByEmailChangeToken(tx *storage.Connection, token string) (*User, error) { - ott, err := FindOneTimeToken(tx, token, EmailChangeTokenCurrent, EmailChangeTokenNew) + ott, err := FindOneTimeToken(tx, []string{token}, EmailChangeTokenCurrent, EmailChangeTokenNew) if err != nil { return nil, err } @@ -219,20 +239,21 @@ func FindUserByEmailChangeToken(tx *storage.Connection, token string) (*User, er return FindUserByID(tx, ott.UserID) } -// FindUserByEmailChangeCurrentAndAudience finds a user with the matching email change and audience. -func FindUserByEmailChangeCurrentAndAudience(tx *storage.Connection, email, token, aud string) (*User, error) { - ott, err := FindOneTimeToken(tx, token, EmailChangeTokenCurrent) - if err != nil && !IsNotFoundError(err) { - return nil, err +// tokenHashesWithPKCEVariants expands each candidate hash into itself plus +// its "pkce_"-prefixed form, so a single FindOneTimeToken call can match +// either variant of every candidate in one query. +func tokenHashesWithPKCEVariants(tokenHashes []string) []string { + variants := make([]string, 0, len(tokenHashes)*2) + for _, hash := range tokenHashes { + variants = append(variants, hash, "pkce_"+hash) } + return variants +} - if ott == nil { - ott, err = FindOneTimeToken(tx, "pkce_"+token, EmailChangeTokenCurrent) - if err != nil { - return nil, err - } - } - if ott == nil { +// FindUserByEmailChangeCurrentAndAudience finds a user with the matching email change and audience. +func FindUserByEmailChangeCurrentAndAudience(tx *storage.Connection, email string, tokenHashes []string, aud string) (*User, error) { + ott, err := FindOneTimeToken(tx, tokenHashesWithPKCEVariants(tokenHashes), EmailChangeTokenCurrent) + if err != nil { return nil, err } @@ -249,19 +270,9 @@ func FindUserByEmailChangeCurrentAndAudience(tx *storage.Connection, email, toke } // FindUserByEmailChangeNewAndAudience finds a user with the matching email change and audience. -func FindUserByEmailChangeNewAndAudience(tx *storage.Connection, email, token, aud string) (*User, error) { - ott, err := FindOneTimeToken(tx, token, EmailChangeTokenNew) - if err != nil && !IsNotFoundError(err) { - return nil, err - } - - if ott == nil { - ott, err = FindOneTimeToken(tx, "pkce_"+token, EmailChangeTokenNew) - if err != nil && !IsNotFoundError(err) { - return nil, err - } - } - if ott == nil { +func FindUserByEmailChangeNewAndAudience(tx *storage.Connection, email string, tokenHashes []string, aud string) (*User, error) { + ott, err := FindOneTimeToken(tx, tokenHashesWithPKCEVariants(tokenHashes), EmailChangeTokenNew) + if err != nil { return nil, err } @@ -278,13 +289,13 @@ func FindUserByEmailChangeNewAndAudience(tx *storage.Connection, email, token, a } // FindUserForEmailChange finds a user requesting for an email change -func FindUserForEmailChange(tx *storage.Connection, email, token, aud string, secureEmailChangeEnabled bool) (*User, error) { +func FindUserForEmailChange(tx *storage.Connection, email string, tokenHashes []string, aud string, secureEmailChangeEnabled bool) (*User, error) { if secureEmailChangeEnabled { - if user, err := FindUserByEmailChangeCurrentAndAudience(tx, email, token, aud); err == nil { + if user, err := FindUserByEmailChangeCurrentAndAudience(tx, email, tokenHashes, aud); err == nil { return user, err } else if !IsNotFoundError(err) { return nil, err } } - return FindUserByEmailChangeNewAndAudience(tx, email, token, aud) + return FindUserByEmailChangeNewAndAudience(tx, email, tokenHashes, aud) } diff --git a/internal/models/one_time_token_test.go b/internal/models/one_time_token_test.go new file mode 100644 index 000000000..4b66d0687 --- /dev/null +++ b/internal/models/one_time_token_test.go @@ -0,0 +1,114 @@ +package models + +import ( + "testing" + + "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" +) + +type OneTimeTokenTestSuite struct { + suite.Suite + db *storage.Connection + config *conf.GlobalConfiguration +} + +func (ts *OneTimeTokenTestSuite) SetupTest() { + TruncateAll(ts.db) +} + +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) createUser() *User { + user, err := NewUser("", "one-time-token@example.com", "secret", "test", nil) + require.NoError(ts.T(), err) + require.NoError(ts.T(), ts.db.Create(user)) + return user +} + +// TestFindOneTimeTokenCandidateList covers the salt cut-over use case: a +// single lookup that matches any of several candidate hashes (e.g. a +// current, salted hash alongside a legacy unsalted fallback). +func (ts *OneTimeTokenTestSuite) TestFindOneTimeTokenCandidateList() { + u := ts.createUser() + + const actualHash = "actual-hash-value" + require.NoError(ts.T(), CreateOneTimeToken(ts.db, u.ID, "relates_to not used", actualHash, ConfirmationToken)) + + ts.Run("matches when the actual hash is one of several candidates", func() { + ott, err := FindOneTimeToken(ts.db, []string{"wrong-hash", actualHash}, ConfirmationToken) + require.NoError(ts.T(), err) + require.Equal(ts.T(), actualHash, ott.TokenHash) + }) + + ts.Run("matches when the actual hash is the only candidate", func() { + ott, err := FindOneTimeToken(ts.db, []string{actualHash}, ConfirmationToken) + require.NoError(ts.T(), err) + require.Equal(ts.T(), actualHash, ott.TokenHash) + }) + + ts.Run("not found when no candidate matches", func() { + _, err := FindOneTimeToken(ts.db, []string{"wrong-hash-a", "wrong-hash-b"}, ConfirmationToken) + require.Error(ts.T(), err) + require.True(ts.T(), IsNotFoundError(err)) + }) +} + +// TestFindUserByEmailChangeAndAudienceCandidateList covers the combination +// of the candidate-hash-list mechanism (salted + legacy fallback) with the +// existing "pkce_" prefix retry: a single lookup must match a stored, +// PKCE-prefixed hash against any of the unprefixed candidates. +func (ts *OneTimeTokenTestSuite) TestFindUserByEmailChangeAndAudienceCandidateList() { + u := ts.createUser() + u.EmailChange = "changed@example.com" + require.NoError(ts.T(), ts.db.Update(u)) + + const legacyHash = "legacy-unsalted-hash" + const saltedHash = "current-salted-hash" + prefixedLegacyHash := "pkce_" + legacyHash + + ts.Run("FindUserByEmailChangeCurrentAndAudience matches a pkce_-prefixed legacy candidate", func() { + require.NoError(ts.T(), ClearOneTimeTokenForUser(ts.db, u.ID, EmailChangeTokenCurrent)) + require.NoError(ts.T(), CreateOneTimeToken(ts.db, u.ID, u.GetEmail(), prefixedLegacyHash, EmailChangeTokenCurrent)) + + found, err := FindUserByEmailChangeCurrentAndAudience(ts.db, u.GetEmail(), []string{saltedHash, legacyHash}, u.Aud) + require.NoError(ts.T(), err) + require.Equal(ts.T(), u.ID, found.ID) + }) + + ts.Run("FindUserByEmailChangeNewAndAudience matches a pkce_-prefixed legacy candidate", func() { + require.NoError(ts.T(), ClearOneTimeTokenForUser(ts.db, u.ID, EmailChangeTokenNew)) + require.NoError(ts.T(), CreateOneTimeToken(ts.db, u.ID, u.EmailChange, prefixedLegacyHash, EmailChangeTokenNew)) + + found, err := FindUserByEmailChangeNewAndAudience(ts.db, u.EmailChange, []string{saltedHash, legacyHash}, u.Aud) + require.NoError(ts.T(), err) + require.Equal(ts.T(), u.ID, found.ID) + }) + + ts.Run("no match when none of the candidates (prefixed or not) match", func() { + require.NoError(ts.T(), ClearOneTimeTokenForUser(ts.db, u.ID, EmailChangeTokenCurrent)) + require.NoError(ts.T(), CreateOneTimeToken(ts.db, u.ID, u.GetEmail(), prefixedLegacyHash, EmailChangeTokenCurrent)) + + _, err := FindUserByEmailChangeCurrentAndAudience(ts.db, u.GetEmail(), []string{"unrelated-hash"}, u.Aud) + require.Error(ts.T(), err) + require.True(ts.T(), IsNotFoundError(err)) + }) +}