diff --git a/docs/email-verification-contract.md b/docs/email-verification-contract.md index dda99b6ad..de8f5c00b 100644 --- a/docs/email-verification-contract.md +++ b/docs/email-verification-contract.md @@ -236,6 +236,44 @@ Two fixes make the table above actually hold: applied further down, so a call setting only `email_verified` was rejected unless padded with an unrelated field. +### How long a verification link is valid + +**30 minutes.** `CreateVerificationToken` mints the JWT with `exp` 30 minutes out +(`internal/token/verification_token.go`), and the stored row carries a matching +`expires_at`. + +Expiry is enforced by **JWT validation**, not by the row: redemption parses and +validates the token before anything else, so an expired link is refused even if +the row is still present. The `expires_at` column is used elsewhere — the login +path reads it to decide whether a *pending* verification should block a sign-in +or be cleared and replaced. + +A link also stops working **before** its 30 minutes if a newer one is issued: +each request rotates the nonce, and redemption checks the token's nonce against +the stored row. So the most recent link is always the only valid one — requesting +a fresh link invalidates the previous one immediately. + +### The link expired — what now + +Nothing here needs an administrator. + +1. **Request a new link.** `resend_verify_email` with the same `identifier` + (`basic_auth_signup` for a normal signup) mints a fresh request and mails it. + This works whether or not a pending request still exists — it will create one + if the old row is gone. +2. **Or just log in with your password.** An unverified account's password login + emails a one-time code instead of a session; entering that code verifies the + address (`verify_otp.go`). This is why an expired link is rarely noticed by + password users. +3. **Operator fallback.** An admin can force-verify from the dashboard + (**Mark Email Verified**) or via `_update_user { email_verified: true }`. + Prefer **Resend Verification Email** where possible — it has the user prove + control rather than asserting it on their behalf. + +The response to a resend is deliberately generic ("if a verification is pending +…") and identical whether or not the address exists, so the endpoint cannot be +used to test which addresses are registered. + ### Hard requirement: email verification needs a working email service `--enable-email-verification=true` with no SMTP configured is now a **fatal diff --git a/internal/http_handlers/verify_email.go b/internal/http_handlers/verify_email.go index 249afad14..792057394 100644 --- a/internal/http_handlers/verify_email.go +++ b/internal/http_handlers/verify_email.go @@ -47,70 +47,28 @@ func (h *httpProvider) VerifyEmailHandler() gin.HandlerFunc { return } - verificationRequest, err := h.StorageProvider.GetVerificationRequestByToken(c, tokenInQuery) + // Decision logic is shared with the GraphQL/gRPC mutation via + // service.ConsumeEmailVerificationToken — token validation, purpose + // binding, user lookup, the revoked check, and the email_verified_at + // write, in that order. This handler previously reimplemented all of it + // and drifted twice; see that function's comment. Only presentation + // stays here (redirect vs AuthResponse). + verified, err := h.ServiceProvider.ConsumeEmailVerificationToken(c, hostname, tokenInQuery) if err != nil { - log.Debug().Err(err).Msg("Error getting verification request") + log.Debug().Err(err).Msg("Failed to consume verification token") errorRes["error"] = err.Error() utils.HandleRedirectORJsonResponse(c, http.StatusBadRequest, errorRes, generateRedirectURL(redirectURL, errorRes)) return } - - // verify if token exists in db - claim, err := h.TokenProvider.ParseJWTToken(tokenInQuery) - if err != nil { - log.Debug().Err(err).Msg("Error parsing jwt token") - errorRes["error"] = err.Error() - utils.HandleRedirectORJsonResponse(c, http.StatusBadRequest, errorRes, generateRedirectURL(redirectURL, errorRes)) - return - } - // // hostname, verificationRequest.Nonce, verificationRequest.Email - if ok, err := h.TokenProvider.ValidateJWTClaims(claim, &token.AuthTokenConfig{ - HostName: hostname, - Nonce: verificationRequest.Nonce, - User: &schemas.User{ - Email: refs.NewStringRef(verificationRequest.Email), - }, - }); !ok || err != nil { - log.Debug().Err(err).Msg("Error validating jwt token") - errorRes["error"] = err.Error() - utils.HandleRedirectORJsonResponse(c, http.StatusBadRequest, errorRes, generateRedirectURL(redirectURL, errorRes)) - return - } - - // Purpose binding: only the email-verification family may complete here. - // Every flow's token lives in one `verification_requests` table keyed by - // the token string alone, so without this a forgot-password token — the - // one credential this endpoint was never meant to see — redeems for a - // full session AND marks the address verified. Same reason the GraphQL - // mutation gates it (service.VerifyEmail); this handler is a separate - // implementation of the same flow and needs the same gate. Generic error - // so it is not an oracle for which flow a leaked token belongs to. - if !service.IsVerifyEmailPurpose(verificationRequest, claim) { - log.Debug().Str("identifier", verificationRequest.Identifier).Msg("Verification token used for the wrong purpose") - errorRes["error"] = "invalid verification token" - utils.HandleRedirectORJsonResponse(c, http.StatusBadRequest, errorRes, generateRedirectURL(redirectURL, errorRes)) - return - } - - user, err := h.StorageProvider.GetUserByEmail(c, verificationRequest.Email) - if err != nil { - log.Debug().Err(err).Msg("Error getting user by email") - errorRes["error"] = err.Error() - utils.HandleRedirectORJsonResponse(c, http.StatusBadRequest, errorRes, generateRedirectURL(redirectURL, errorRes)) - return - } - - if user.RevokedTimestamp != nil { - log.Debug().Msg("User access has been revoked") - errorRes["error"] = "user access has been revoked" - utils.HandleRedirectORJsonResponse(c, http.StatusBadRequest, errorRes, generateRedirectURL(redirectURL, errorRes)) - return - } + user := verified.User + verificationRequest := verified.Request + isSignUp := verified.IsSignUp + loginMethod := verified.LoginMethod // Resolved once, early: needed both for the MFA-gate-withheld redirect // below and the success redirect further down. if redirectURL == "" { - redirectURL = claimString(claim, "redirect_uri") + redirectURL = verified.RedirectURI } if !validators.IsValidRedirectURI(redirectURL, h.Config.AllowedOrigins, hostname) { log.Debug().Msg("Invalid redirect URI in token claim") @@ -118,37 +76,6 @@ func (h *httpProvider) VerifyEmailHandler() gin.HandlerFunc { return } - // Record the address as verified HERE, BEFORE the MFA gate below. - // - // The gate's withheld branch redirects to MFA setup and returns, so the - // write that used to sit after it never ran for the overwhelmingly - // common case: a fresh signup clicking its verification link, with MFA - // on by default. The user did everything right and their address stayed - // unverified forever. - // - // Only passkey login surfaces it — webauthn.go refuses outright on - // email_verified_at == nil, with an error the user cannot act on. - // Password, TOTP and email/SMS-OTP logins never check the column, so - // they appear to "work" while the account is in exactly the same broken - // state. That asymmetry is why this hid. - // - // Clicking the link IS the proof of mailbox control; whether MFA then - // interrupts token issuance is a separate question and must not discard - // it. Mirrors service.VerifyEmail, which is the GraphQL twin of this - // handler — the two implementations have to agree. - emailJustVerified := user.EmailVerifiedAt == nil - if emailJustVerified { - now := time.Now().Unix() - user.EmailVerifiedAt = &now - user, err = h.StorageProvider.UpdateUser(c, user) - if err != nil { - log.Debug().Err(err).Msg("Error updating user") - errorRes["error"] = err.Error() - utils.HandleRedirectORJsonResponse(c, http.StatusBadRequest, errorRes, generateRedirectURL(redirectURL, errorRes)) - return - } - } - // MFA gate: this REST endpoint is what the emailed verification/magic // link literally points to, so it must enforce the same gate every // other login entry point does (login.go/signup.go/oauth_callback.go). @@ -177,8 +104,6 @@ func (h *httpProvider) VerifyEmailHandler() gin.HandlerFunc { return } - // Set above, before the MFA gate — see the comment there. - isSignUp := emailJustVerified // delete from verification table if err := h.StorageProvider.DeleteVerificationRequest(c, verificationRequest); err != nil { log.Debug().Err(err).Msg("Error deleting verification request") @@ -207,10 +132,6 @@ func (h *httpProvider) VerifyEmailHandler() gin.HandlerFunc { } else { scope = strings.Split(scopeString, " ") } - loginMethod := constants.AuthRecipeMethodBasicAuth - if verificationRequest.Identifier == constants.VerificationTypeMagicLinkLogin { - loginMethod = constants.AuthRecipeMethodMagicLinkLogin - } code := "" // Not required as /oauth/token cannot be resumed from other tab diff --git a/internal/integration_tests/signup_verification_matrix_test.go b/internal/integration_tests/signup_verification_matrix_test.go index a9e98784d..692ea1b81 100644 --- a/internal/integration_tests/signup_verification_matrix_test.go +++ b/internal/integration_tests/signup_verification_matrix_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + "github.com/golang-jwt/jwt/v4" "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -15,6 +16,8 @@ import ( "github.com/authorizerdev/authorizer/internal/constants" "github.com/authorizerdev/authorizer/internal/graph/model" "github.com/authorizerdev/authorizer/internal/refs" + "github.com/authorizerdev/authorizer/internal/storage/schemas" + "github.com/authorizerdev/authorizer/internal/token" ) // Signup × email-verification matrix. @@ -400,3 +403,188 @@ func TestVerifyEmailRESTMarksVerifiedBeforeMFAGate(t *testing.T) { "clicking the emailed link must record the address as verified even when the MFA gate withholds the token — otherwise passkey login rejects the user permanently, while password/TOTP/OTP logins hide it by never checking") assert.Equal(t, before.ID, after.ID) } + +// TestExpiredVerificationLinkIsRefusedAndRecoverable covers the case a real user +// hits most often: they did not click the link within 30 minutes. +// +// Two things must both hold, and only the pair is useful. The stale link must be +// refused — it is a capability, and an expired one that still works is just a +// long-lived one. And the user must be able to get a fresh link WITHOUT an +// admin, or "your link expired" is a dead end. +// +// The 30 minutes comes from CreateVerificationToken's `exp` claim +// (internal/token/verification_token.go); expiry is enforced by JWT validation +// inside ConsumeEmailVerificationToken, not by the row's expires_at column. +func TestExpiredVerificationLinkIsRefusedAndRecoverable(t *testing.T) { + cfg := getTestConfig() + cfg.IsEmailServiceEnabled = true + cfg.EnableEmailVerification = true + cfg.DisableMFA = true + ts := initTestSetup(t, cfg) + _, ctx := createContext(ts) + + email := "expired_link_" + uuid.NewString() + "@authorizer.dev" + _, err := ts.GraphQLProvider.SignUp(ctx, &model.SignUpRequest{ + Email: &email, + Password: "Password@123", + ConfirmPassword: "Password@123", + }) + require.NoError(t, err) + + original, err := ts.StorageProvider.GetVerificationRequestByEmail(ctx, email, constants.VerificationTypeBasicAuthSignup) + require.NoError(t, err) + + // Mint a token that is already past its exp, and swap it onto the stored + // row so the row and the token agree — otherwise the lookup fails first and + // the test would pass without ever exercising expiry. + expiredToken, err := ts.TokenProvider.CreateVerificationToken(&token.AuthTokenConfig{ + User: &schemas.User{Email: &email}, + Nonce: original.Nonce, + HostName: testAuthorizerHost(ts), + LoginMethod: constants.AuthRecipeMethodBasicAuth, + }, original.RedirectURI, constants.VerificationTypeBasicAuthSignup) + require.NoError(t, err) + + expiredRow := *original + expiredRow.Token = expiredToken + expiredRow.ExpiresAt = time.Now().Add(-1 * time.Hour).Unix() + require.NoError(t, ts.StorageProvider.DeleteVerificationRequest(ctx, original)) + _, err = ts.StorageProvider.AddVerificationRequest(ctx, &expiredRow) + require.NoError(t, err) + + t.Run("an expired link is refused", func(t *testing.T) { + // A genuinely expired token, signed with exp in the past. Built from + // claims directly rather than CreateVerificationToken, which hardcodes + // exp to +30m — there is no way to test expiry through that helper + // without actually waiting, and a test that waits 30 minutes is a test + // nobody runs. + expiredClaims := jwt.MapClaims{ + "iss": testAuthorizerHost(ts), + "aud": ts.Config.ClientID, + "sub": email, + "exp": time.Now().Add(-1 * time.Minute).Unix(), + "iat": time.Now().Add(-31 * time.Minute).Unix(), + "token_type": constants.VerificationTypeBasicAuthSignup, + "nonce": expiredRow.Nonce, + "redirect_uri": expiredRow.RedirectURI, + } + signed, sErr := ts.TokenProvider.SignJWTToken(expiredClaims) + require.NoError(t, sErr) + + // Put it on the stored row so the lookup succeeds and the request + // genuinely reaches the expiry check, rather than failing earlier as an + // unknown token — which would pass for the wrong reason. + row := expiredRow + row.Token = signed + require.NoError(t, ts.StorageProvider.DeleteVerificationRequest(ctx, &expiredRow)) + _, aErr := ts.StorageProvider.AddVerificationRequest(ctx, &row) + require.NoError(t, aErr) + expiredRow = row + + _, err := ts.GraphQLProvider.VerifyEmail(ctx, &model.VerifyEmailRequest{Token: signed}) + require.Error(t, err, "an expired verification link must not be redeemable — it is a capability, and an expired one that still works is just a long-lived one") + assert.Contains(t, err.Error(), "invalid verification token") + + user, uErr := ts.StorageProvider.GetUserByEmail(ctx, email) + require.NoError(t, uErr) + assert.Nil(t, user.EmailVerifiedAt, "a refused link must not verify the address") + }) + + t.Run("a stale link whose nonce no longer matches is refused", func(t *testing.T) { + // This is what an expired-then-resent link looks like in practice: the + // resend rotates the nonce, so the OLD link stops validating even before + // its exp. Same refusal path, and testable without waiting 30 minutes. + _, err := ts.GraphQLProvider.ResendVerifyEmail(ctx, &model.ResendVerifyEmailRequest{ + Email: email, + Identifier: constants.VerificationTypeBasicAuthSignup, + }) + require.NoError(t, err) + + _, err = ts.GraphQLProvider.VerifyEmail(ctx, &model.VerifyEmailRequest{Token: expiredRow.Token}) + require.Error(t, err, "the superseded link must stop working once a new one is issued") + assert.Contains(t, err.Error(), "invalid verification token") + + user, uErr := ts.StorageProvider.GetUserByEmail(ctx, email) + require.NoError(t, uErr) + assert.Nil(t, user.EmailVerifiedAt, "a refused link must not verify the address") + }) + + t.Run("the resent link works, so the user is never stuck", func(t *testing.T) { + fresh, err := ts.StorageProvider.GetVerificationRequestByEmail(ctx, email, constants.VerificationTypeBasicAuthSignup) + require.NoError(t, err) + require.NotEqual(t, expiredRow.Token, fresh.Token) + + _, err = ts.GraphQLProvider.VerifyEmail(ctx, &model.VerifyEmailRequest{Token: fresh.Token}) + require.NoError(t, err) + + user, err := ts.StorageProvider.GetUserByEmail(ctx, email) + require.NoError(t, err) + assert.NotNil(t, user.EmailVerifiedAt, "the recovery path must actually complete verification") + }) +} + +// TestVerificationTokenWithEmptySubjectIsRefused pins a guard that does not +// depend on anything else being right. +// +// ValidateJWTClaims checks `sub` as +// +// claims["sub"] != cfg.User.ID && claims["sub"] != cfg.User.Email +// +// and the verification path sets only Email, leaving User.ID as "". A token +// whose `sub` is the empty STRING therefore satisfies the first comparison and +// passes that check. Whether it then does damage depends on whether the storage +// backend keeps a missing email as NULL or as "" — SQL keeps it NULL, so the +// lookup finds nobody, but that is an accident of one backend's representation +// across six implementations, not a declared property. +// +// So the subject is rejected outright. If this ever fails, an empty-subject +// token is reaching account selection and the only thing standing between it +// and a real account is a per-backend NULL convention. +func TestVerificationTokenWithEmptySubjectIsRefused(t *testing.T) { + cfg := getTestConfig() + cfg.IsEmailServiceEnabled = true + cfg.EnableEmailVerification = true + ts := initTestSetup(t, cfg) + _, ctx := createContext(ts) + + email := "empty_sub_" + uuid.NewString() + "@authorizer.dev" + _, err := ts.GraphQLProvider.SignUp(ctx, &model.SignUpRequest{ + Email: &email, + Password: "Password@123", + ConfirmPassword: "Password@123", + }) + require.NoError(t, err) + + row, err := ts.StorageProvider.GetVerificationRequestByEmail(ctx, email, constants.VerificationTypeBasicAuthSignup) + require.NoError(t, err) + + // Everything valid except the subject, which is empty. + forged, err := ts.TokenProvider.SignJWTToken(jwt.MapClaims{ + "iss": testAuthorizerHost(ts), + "aud": ts.Config.ClientID, + "sub": "", + "exp": time.Now().Add(10 * time.Minute).Unix(), + "iat": time.Now().Unix(), + "token_type": constants.VerificationTypeBasicAuthSignup, + "nonce": row.Nonce, + "redirect_uri": row.RedirectURI, + }) + require.NoError(t, err) + + // Attach it to the stored row so the lookup succeeds and the request really + // reaches subject handling instead of failing earlier as an unknown token. + forgedRow := *row + forgedRow.Token = forged + require.NoError(t, ts.StorageProvider.DeleteVerificationRequest(ctx, row)) + _, err = ts.StorageProvider.AddVerificationRequest(ctx, &forgedRow) + require.NoError(t, err) + + _, err = ts.GraphQLProvider.VerifyEmail(ctx, &model.VerifyEmailRequest{Token: forged}) + require.Error(t, err, "an empty subject must never be used to select an account") + assert.Contains(t, err.Error(), "invalid verification token") + + // And nothing was verified as a side effect. + user, err := ts.StorageProvider.GetUserByEmail(ctx, email) + require.NoError(t, err) + assert.Nil(t, user.EmailVerifiedAt) +} diff --git a/internal/service/provider.go b/internal/service/provider.go index 49d7486c0..519510f4d 100644 --- a/internal/service/provider.go +++ b/internal/service/provider.go @@ -170,6 +170,11 @@ type Provider interface { // VerifyEmail completes email verification and logs the user in. Browser // callers get a session cookie via side-effects. Public. VerifyEmail(ctx context.Context, meta RequestMetadata, params *model.VerifyEmailRequest) (*model.AuthResponse, *ResponseSideEffects, error) + // ConsumeEmailVerificationToken is the shared decision core behind both + // implementations of email verification (the GraphQL/gRPC mutation and the + // REST handler behind GET /verify_email). See verify_email_core.go for why + // it is shared rather than duplicated. + ConsumeEmailVerificationToken(ctx context.Context, hostname, rawToken string) (*EmailVerification, error) // VerifyOTP validates an email/SMS OTP or TOTP/recovery code and logs the // user in. Browser callers get a session cookie via side-effects. Public. diff --git a/internal/service/verify_email.go b/internal/service/verify_email.go index 7858972af..ad0fdacf4 100644 --- a/internal/service/verify_email.go +++ b/internal/service/verify_email.go @@ -29,50 +29,21 @@ func (p *provider) VerifyEmail(ctx context.Context, meta RequestMetadata, params log := p.Log.With().Str("func", "VerifyEmail").Logger() side := &ResponseSideEffects{} - verificationRequest, err := p.StorageProvider.GetVerificationRequestByToken(ctx, params.Token) - if err != nil { - log.Debug().Err(err).Msg("failed GetVerificationRequestByToken") - return nil, nil, InvalidArgument(`invalid verification token`) - } - - // verify if token exists in db + // Decision logic lives in ConsumeEmailVerificationToken so the REST handler + // behind GET /verify_email runs exactly the same checks — see + // verify_email_core.go for the two drifts that motivated sharing it. Only + // the shared PREFIX moved: every MFA branch below stays here, because the + // two callers diverge from this point on (this one returns AuthResponse + // screens, the handler redirects). hostname := meta.HostURL - claim, err := p.TokenProvider.ParseJWTToken(params.Token) - if err != nil { - log.Debug().Err(err).Msg("Failed to parse jwt token") - return nil, nil, InvalidArgument(`invalid verification token`) - } - - if ok, err := p.TokenProvider.ValidateJWTClaims(claim, &token.AuthTokenConfig{ - HostName: hostname, - Nonce: verificationRequest.Nonce, - User: &schemas.User{ - Email: &verificationRequest.Email, - }, - }); !ok || err != nil { - log.Debug().Err(err).Msg("Failed to validate jwt claims") - return nil, nil, InvalidArgument(`invalid verification token`) - } - - // Purpose binding: only the email-verification family may complete here. A - // forgot-password token must not be redeemable for a session. - if !IsVerifyEmailPurpose(verificationRequest, claim) { - log.Debug().Str("identifier", verificationRequest.Identifier).Msg("Verification token used for the wrong purpose") - return nil, nil, InvalidArgument(`invalid verification token`) - } - - email := claim["sub"].(string) - log.Debug().Str("email", email).Msg("Email verified successfully") - user, err := p.StorageProvider.GetUserByEmail(ctx, email) + verified, err := p.ConsumeEmailVerificationToken(ctx, hostname, params.Token) if err != nil { - log.Debug().Err(err).Msg("failed GetUserByEmail") return nil, nil, err } - - if user.RevokedTimestamp != nil { - log.Debug().Msg("User access has been revoked") - return nil, nil, FailedPrecondition("user access has been revoked") - } + user := verified.User + verificationRequest := verified.Request + loginMethod := verified.LoginMethod + emailJustVerified := verified.IsSignUp // A single check protecting every MFA branch below, mirroring login.go — // lockout is set only by explicit user action (lock_mfa), never inferred @@ -83,11 +54,6 @@ func (p *provider) VerifyEmail(ctx context.Context, meta RequestMetadata, params return nil, nil, FailedPrecondition("your account's multi-factor authentication is locked; contact your administrator to regain access") } - loginMethod := constants.AuthRecipeMethodBasicAuth - if verificationRequest.Identifier == constants.VerificationTypeMagicLinkLogin { - loginMethod = constants.AuthRecipeMethodMagicLinkLogin - } - isTOTPLoginEnabled := p.Config.EnableTOTPLogin isMFAEnabled := p.Config.EnableMFA @@ -154,31 +120,9 @@ func (p *provider) VerifyEmail(ctx context.Context, meta RequestMetadata, params }, side, nil } - // Record the email as verified HERE, before the MFA gate below, because - // every branch of that gate can return early — and MFA is on by default - // (TOTP needs no external provider, so config.Finalize derives - // EnableMFA=true). A fresh signup clicking its verification link therefore - // lands on the MFA setup screen and used to return with email_verified - // still nil: the user completed setup, got a session, and their address was - // never marked verified. Anything gating on it later refused them - // permanently — passkey login says "email is not verified. please verify - // your email before signing in with a passkey" to a user who did exactly - // that, with no way to fix it. - // - // Clicking the link IS the proof of mailbox control. Whether MFA then - // interrupts session issuance is a separate question and must not discard - // the proof. The verification request itself is still consumed below, on - // the path that completes. - emailJustVerified := user.EmailVerifiedAt == nil - if emailJustVerified { - now := time.Now().Unix() - user.EmailVerifiedAt = &now - user, err = p.StorageProvider.UpdateUser(ctx, user) - if err != nil { - log.Debug().Err(err).Msg("failed UpdateUser") - return nil, nil, err - } - } + // The address was already marked verified by ConsumeEmailVerificationToken, + // before any of the MFA branches above could return early — see the + // ORDER IS LOAD-BEARING note there. // Gate runs whenever MFA applies at all, exactly like login.go/signup.go — // this used to be an ad-hoc TOTP-only check (refs.BoolValue(user.IsMultiFactorAuthEnabled) diff --git a/internal/service/verify_email_core.go b/internal/service/verify_email_core.go new file mode 100644 index 000000000..3db16baa1 --- /dev/null +++ b/internal/service/verify_email_core.go @@ -0,0 +1,162 @@ +package service + +import ( + "context" + "strings" + "time" + + "github.com/authorizerdev/authorizer/internal/constants" + "github.com/authorizerdev/authorizer/internal/refs" + "github.com/authorizerdev/authorizer/internal/storage/schemas" + "github.com/authorizerdev/authorizer/internal/token" +) + +// EmailVerification is the outcome of redeeming a verification token: who the +// principal is, which flow the token belonged to, and whether this redemption is +// the one that verified the address. +type EmailVerification struct { + User *schemas.User + // Request is the consumed verification row. The caller deletes it once it + // has finished with it — deletion is deliberately NOT done here, because the + // two callers finish at different points. + Request *schemas.VerificationRequest + // LoginMethod is basic_auth, or magic_link_login when the token came from a + // magic link. + LoginMethod string + // IsSignUp reports whether THIS redemption flipped the address to verified, + // which is what the callers key their signup-vs-login webhook on. + IsSignUp bool + // RedirectURI is the `redirect_uri` claim the token was minted with. The + // REST handler falls back to it when the query string carries none; it is + // returned here so the claim never has to leave this function. + RedirectURI string +} + +// ConsumeEmailVerificationToken is the single source of truth for what it means +// to redeem an email-verification token. +// +// It exists because there are two implementations of this flow over the same +// token table, and they have now drifted twice: +// +// - the MFA gate was added to the GraphQL mutation and missed the REST +// handler, so the emailed link bypassed MFA entirely (noted in that +// handler's own comment); +// - the email_verified_at write was moved above the MFA gate in the mutation +// and missed the REST handler, so users who clicked the emailed button were +// never marked verified — surfacing only on passkey login, because that is +// the sole login path that checks the column. Password login checks it too +// but silently self-heals via an email-OTP detour, which is why TOTP and +// email-OTP users appeared unaffected. +// +// GET /verify_email is what the button in the email literally points to, so the +// REST handler is the path essentially every real user takes — and it was the +// copy that kept missing fixes. GraphQL and gRPC both already delegate to the +// service; this pulls the REST handler onto the same decision logic so a third +// divergence cannot happen. +// +// What stays with the callers is presentation, which legitimately differs: the +// mutation returns an AuthResponse, the handler redirects with tokens in the +// query string. What lives here is every decision that must be identical. +// +// ORDER IS LOAD-BEARING. The address is marked verified BEFORE this returns, so +// that a caller which then withholds tokens — the MFA gate redirecting to setup +// — cannot lose the fact that the user proved control of their mailbox. +// Clicking the link is the proof; whether MFA interrupts token issuance +// afterwards is a separate question. +func (p *provider) ConsumeEmailVerificationToken(ctx context.Context, hostname, rawToken string) (*EmailVerification, error) { + log := p.Log.With().Str("func", "ConsumeEmailVerificationToken").Logger() + + verificationRequest, err := p.StorageProvider.GetVerificationRequestByToken(ctx, rawToken) + if err != nil { + log.Debug().Err(err).Msg("failed GetVerificationRequestByToken") + return nil, InvalidArgument(`invalid verification token`) + } + + claim, err := p.TokenProvider.ParseJWTToken(rawToken) + if err != nil { + log.Debug().Err(err).Msg("Failed to parse jwt token") + return nil, InvalidArgument(`invalid verification token`) + } + + if ok, err := p.TokenProvider.ValidateJWTClaims(claim, &token.AuthTokenConfig{ + HostName: hostname, + Nonce: verificationRequest.Nonce, + User: &schemas.User{ + Email: &verificationRequest.Email, + }, + }); !ok || err != nil { + log.Debug().Err(err).Msg("Failed to validate jwt claims") + return nil, InvalidArgument(`invalid verification token`) + } + + // Purpose binding: only the email-verification family completes here. A + // forgot-password token redeemed at this endpoint would otherwise hand out a + // full session AND mark the address verified. Generic error so it is not an + // oracle for which flow a leaked token belongs to. + if !IsVerifyEmailPurpose(verificationRequest, claim) { + log.Debug().Str("identifier", verificationRequest.Identifier).Msg("Verification token used for the wrong purpose") + return nil, InvalidArgument(`invalid verification token`) + } + + // `sub` must be a non-empty string before it is used to select an account. + // + // ValidateJWTClaims above does NOT guarantee that. Its subject check is + // + // claims["sub"] != cfg.User.ID && claims["sub"] != cfg.User.Email + // + // and this call site sets only Email, leaving User.ID as "". A token whose + // `sub` is the empty STRING therefore satisfies the first comparison and the + // whole check passes. The lookup would then run as GetUserByEmail(ctx, ""), + // whose result depends entirely on whether a given backend stores a missing + // email as NULL or as "" — SQL keeps it NULL today, so nothing matches, but + // that is an accident of one backend's representation and not a property + // anyone declared. Six storage backends is too many places for a security + // boundary to be implicit. + // + // Rejected explicitly so this does not depend on the empty-User.ID quirk + // above, nor on NULL-vs-empty-string behaviour below. + email, _ := claim["sub"].(string) + if strings.TrimSpace(email) == "" { + log.Debug().Msg("verification token has no subject") + return nil, InvalidArgument(`invalid verification token`) + } + + user, err := p.StorageProvider.GetUserByEmail(ctx, email) + if err != nil { + log.Debug().Err(err).Msg("failed GetUserByEmail") + return nil, err + } + + if user.RevokedTimestamp != nil { + log.Debug().Msg("User access has been revoked") + return nil, FailedPrecondition("user access has been revoked") + } + + loginMethod := constants.AuthRecipeMethodBasicAuth + if verificationRequest.Identifier == constants.VerificationTypeMagicLinkLogin { + loginMethod = constants.AuthRecipeMethodMagicLinkLogin + } + + // See the ORDER IS LOAD-BEARING note above. + isSignUp := user.EmailVerifiedAt == nil + if isSignUp { + now := time.Now().Unix() + user.EmailVerifiedAt = &now + user, err = p.StorageProvider.UpdateUser(ctx, user) + if err != nil { + log.Debug().Err(err).Msg("failed UpdateUser") + return nil, err + } + } + + redirectURI, _ := claim["redirect_uri"].(string) + + log.Debug().Str("email", refs.StringValue(user.Email)).Msg("Email verified successfully") + return &EmailVerification{ + User: user, + Request: verificationRequest, + LoginMethod: loginMethod, + IsSignUp: isSignUp, + RedirectURI: redirectURI, + }, nil +}