auth.resend() consistent confirmation flow#2401
auth.resend() consistent confirmation flow#2401weilirs wants to merge 1 commit intosupabase:masterfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Cache: Disabled due to Reviews > Disable Cache setting Disabled knowledge base sources:
📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThe changes add PKCE (Proof Key for Code Exchange) support to the resend confirmation API. The Sequence Diagram(s)sequenceDiagram
participant Client
participant ResendHandler as Resend Handler
participant Validator as PKCE Validator
participant FlowState as Flow State Generator
participant MailService as Mail Service
Client->>ResendHandler: POST /resend<br/>(with code_challenge, code_challenge_method)
ResendHandler->>Validator: validatePKCEParams(method, challenge)
alt PKCE params invalid
Validator-->>ResendHandler: Error
ResendHandler-->>Client: HTTP 400
else PKCE params valid
Validator-->>ResendHandler: Valid
ResendHandler->>ResendHandler: Determine flowType from CodeChallenge
alt PKCE flow detected
ResendHandler->>FlowState: Generate flow state
FlowState-->>ResendHandler: Flow state data
end
ResendHandler->>MailService: sendConfirmation/sendEmailChange(flowType)
MailService->>MailService: Create PKCE-prefixed token
MailService-->>Client: Confirmation sent
end
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![depthfirst-app[bot]](https://avatars.githubusercontent.com/in/1021002?s=60&v=4){kind=small}
| return a.sendConfirmation(r, tx, user, models.ImplicitFlow) | ||
| flowType := getFlowFromChallenge(params.CodeChallenge) | ||
| if isPKCEFlow(flowType) { | ||
| if _, terr := generateFlowState(tx, models.EmailSignup.String(), models.EmailSignup, params.CodeChallengeMethod, params.CodeChallenge, &user.ID); terr != nil { |
There was a problem hiding this comment.
🟠 Severity: HIGH
The /resend endpoint allows unauthenticated users to create a new PKCE FlowState for any user. Since verification retrieves the latest FlowState by user_id, an attacker can overwrite a victim's flow. By providing a controlled redirect_to URL, the attacker can intercept the auth_code and hijack the session.
Helpful? Add 👍 / 👎
💡 Fix Suggestion
Suggestion: The /resend endpoint should not allow unauthenticated users to create or modify FlowState records with PKCE parameters for arbitrary users. The most secure fix is to reject PKCE parameters entirely on the /resend endpoint for signup and email_change verification types. Remove the PKCE flow handling code (lines 130-135 for signup and lines 147-152 for email_change) that calls generateFlowState(). The /resend endpoint is meant for retrying delivery of verification emails, not for initiating new PKCE authentication flows. Since this endpoint is unauthenticated and allows lookups by email, allowing PKCE flow creation creates an inherent authorization vulnerability where attackers can overwrite legitimate users' FlowState records with attacker-controlled code_challenge values.
1 participant
What kind of change does this PR introduce?
Bug fix
What is the current behavior?
The
/resendendpoint hardcodesmodels.ImplicitFlowfor bothsignupandemail_changeverification types (#42527). This means resent confirmation emails always use the implicit flow — redirecting with tokens in the URL hash fragment (#access_token=...) — even when the originalsignUp()used PKCE.This creates an inconsistency where:
https://example.com/auth/confirm?code=xxx(PKCE, works with server routes)https://example.com/auth/confirm#access_token=xxx(implicit, requires client-side handling)Server-side route handlers (e.g., Next.js
route.ts) cannot read hash fragments, forcing developers to implement workarounds with client components and dual flow handling.Closes #42527
What is the new behavior?
The
/resendendpoint now accepts optionalcode_challengeandcode_challenge_methodparameters forsignupandemail_changetypes. When provided, the endpoint:code_challenge(PKCE if present, implicit if absent)FlowStaterecord for PKCE flows (needed by/verifyto issue an auth code)sendConfirmation/sendEmailChangeThis produces confirmation emails with
?code=...query params instead of#access_token=...hash fragments, consistent with the initial signup flow.When
code_challengeis not provided, behavior is unchanged — implicit flow is used, maintaining full backward compatibility.Changes:
internal/api/resend.go: AddedCodeChallengeandCodeChallengeMethodfields toResendConfirmationParams. Added PKCE param validation for email-based types. Replaced hardcodedImplicitFlowwith flow-aware logic forsignupandemail_changecases.internal/api/resend_test.go: AddedTestResendPKCEValidation(invalid PKCE params return 400) andTestResendPKCESuccess(signup and email change tokens getpkce_prefix when PKCE params are provided).Additional context
This is the server-side half of the fix. The JS SDK (
auth-js) needs a corresponding update to sendcode_challenge/code_challenge_methodinresend()calls whenflowType === 'pkce', following the same pattern already used bysignUp()andsignInWithOtp(). See this PRThe implementation mirrors the existing PKCE pattern used across the codebase (
signup.go,user.go,recover.go,magic_link.go):getFlowFromChallenge→ conditionalgenerateFlowState→ passflowTypeto the email sender.