feat(auth): auth.resend() consistent confirmation flow#2144
feat(auth): auth.resend() consistent confirmation flow#2144weilirs wants to merge 4 commits 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 CodeRabbit
WalkthroughThis PR adds PKCE flow support to the Assessment against linked issues
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
@supabase/auth-js
@supabase/functions-js
@supabase/postgrest-js
@supabase/realtime-js
@supabase/storage-js
@supabase/supabase-js
commit: |
|
Hi @weilirs ! Thank you for this PR! As you already wrote in the description, we need to wait for the auth PR to be merged/released first. While we wait, can you please make these improvements?
Oh and finally can you please use conventional commits? I assumed you pushed with Thank you so much for this contribution! :D Let's wait for the auth team now to check the other PR first! |
|
Adding |
mandarini
left a comment
mandarini
left a comment
There was a problem hiding this comment.
Please read my comment above
mandarini
added
the
needs server code
There was a problem hiding this comment.
Pull request overview
Aligns auth.resend() with the SDK’s existing PKCE email confirmation behavior by generating a fresh PKCE challenge on resend when flowType === 'pkce', enabling confirmation links that use ?code=... instead of hash-fragment tokens.
Changes:
- Generate/store a new PKCE
code_verifierand sendcode_challenge+code_challenge_methodin the email/resendrequest whenflowTypeispkce. - Add error-path cleanup for the stored
code_verifierduringresend(). - Add a test case covering
resend()with PKCEflowType.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/core/auth-js/src/GoTrueClient.ts | Adds PKCE challenge generation and request fields to the email resend flow; adds code-verifier cleanup on error. |
| packages/core/auth-js/test/GoTrueClient.test.ts | Adds a PKCE resend test case. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let codeChallenge: string | null = null | ||
| let codeChallengeMethod: string | null = null | ||
| if (this.flowType === 'pkce') { | ||
| ;[codeChallenge, codeChallengeMethod] = await getCodeChallengeAndMethod( | ||
| this.storage, | ||
| this.storageKey | ||
| ) | ||
| } | ||
| const { error } = await _request(this.fetch, 'POST', endpoint, { | ||
| headers: this.headers, | ||
| body: { | ||
| email, | ||
| type, | ||
| gotrue_meta_security: { captcha_token: options?.captchaToken }, | ||
| code_challenge: codeChallenge, | ||
| code_challenge_method: codeChallengeMethod, | ||
| }, |
There was a problem hiding this comment.
In the non-PKCE case, code_challenge / code_challenge_method are still being included in the /resend request body as explicit null values. This changes the payload compared to prior versions and can trigger backend validation differences vs omitting the fields entirely. Consider only adding these properties to the body when this.flowType === 'pkce' (e.g., via a conditional object spread) so implicit-flow resend requests remain unchanged.
| let codeChallenge: string | null = null | |
| let codeChallengeMethod: string | null = null | |
| if (this.flowType === 'pkce') { | |
| ;[codeChallenge, codeChallengeMethod] = await getCodeChallengeAndMethod( | |
| this.storage, | |
| this.storageKey | |
| ) | |
| } | |
| const { error } = await _request(this.fetch, 'POST', endpoint, { | |
| headers: this.headers, | |
| body: { | |
| email, | |
| type, | |
| gotrue_meta_security: { captcha_token: options?.captchaToken }, | |
| code_challenge: codeChallenge, | |
| code_challenge_method: codeChallengeMethod, | |
| }, | |
| const body: { | |
| email: string | |
| type: typeof type | |
| gotrue_meta_security: { captcha_token: string | undefined } | |
| code_challenge?: string | null | |
| code_challenge_method?: string | null | |
| } = { | |
| email, | |
| type, | |
| gotrue_meta_security: { captcha_token: options?.captchaToken }, | |
| } | |
| if (this.flowType === 'pkce') { | |
| const [codeChallenge, codeChallengeMethod] = await getCodeChallengeAndMethod( | |
| this.storage, | |
| this.storageKey | |
| ) | |
| body.code_challenge = codeChallenge | |
| body.code_challenge_method = codeChallengeMethod | |
| } | |
| const { error } = await _request(this.fetch, 'POST', endpoint, { | |
| headers: this.headers, | |
| body, |
| const { error } = await pkceClient.resend({ | ||
| email, | ||
| type: 'signup', | ||
| options: { emailRedirectTo: 'http://localhost:9999/welcome' }, | ||
| }) | ||
| expect(error).toBeNull() |
There was a problem hiding this comment.
This test only asserts that pkceClient.resend() returns error === null, but it doesn’t verify the new PKCE-specific behavior (generating/storing a fresh code_verifier and sending code_challenge + code_challenge_method). As written, it could still pass even if those fields aren’t included. Consider using a client with a mocked fetch to assert the request body contains the PKCE fields (and/or assert ${storageKey}-code-verifier is set after the call).
| const { error } = await pkceClient.resend({ | |
| email, | |
| type: 'signup', | |
| options: { emailRedirectTo: 'http://localhost:9999/welcome' }, | |
| }) | |
| expect(error).toBeNull() | |
| const fetchSpy = jest | |
| .spyOn(globalThis as any, 'fetch') | |
| .mockImplementation(async (_input: any, init?: any) => { | |
| if (init && typeof init.body === 'string') { | |
| const body = JSON.parse(init.body) | |
| expect(body.code_challenge).toEqual(expect.any(String)) | |
| expect(body.code_challenge_method).toBe('S256') | |
| } | |
| return Promise.resolve({ | |
| ok: true, | |
| status: 200, | |
| json: async () => ({}), | |
| }) as any | |
| }) | |
| try { | |
| const { error } = await pkceClient.resend({ | |
| email, | |
| type: 'signup', | |
| options: { emailRedirectTo: 'http://localhost:9999/welcome' }, | |
| }) | |
| expect(error).toBeNull() | |
| const storedCodeVerifier = await memoryLocalStorageAdapter.getItem( | |
| `${STORAGE_KEY}-code-verifier` | |
| ) | |
| expect(storedCodeVerifier).toEqual(expect.any(String)) | |
| } finally { | |
| fetchSpy.mockRestore() | |
| } |
…up verifier on error
|
Hi @mandarini, sorry for the late reply but I've made the changes you suggested. Thanks! |
mandarini
left a comment
mandarini
left a comment
There was a problem hiding this comment.
Hi @weilirs, thank you so much for making those changes! 💚
The tests are much stronger now, checking the actual request body for code_challenge and code_challenge_method is exactly what we needed, and the error-path cleanup test is solid. The email_change coverage is also a great addition.
A couple of small things before we can merge:
-
The
email_changetest doesn't verify that the code verifier was actually stored after the call. Thesignuptest does this:const codeVerifier = await storage.getItem(`${storageKey}-code-verifier`) expect(codeVerifier).not.toBeNull()
It would be good to add the same assertion to the
email_changetest for consistency. -
There's no test for the implicit flow (non-PKCE) path. Since the body now always includes
code_challengeandcode_challenge_method(asnullwhen not using PKCE), a test confirming that those fields arenullin the body whenflowTypeis not'pkce'would make the behavior explicit and protect against regressions.
We're still blocked on supabase/auth#2401 merging before we can ship this, so there's no rush, but it would be great to have these in while we wait.
Thank you again for sticking with this and addressing the feedback so thoroughly!
mandarini
changed the title
Hey @mandarini ! I strengthened the tests per your request, please take a look. |
|
@weilirs thanks. Let's wait for the server PR now to be merged! |
## What kind of change does this PR introduce? Bug fix ## What is the current behavior? The `/resend` endpoint hardcodes `models.ImplicitFlow` for both `signup` and `email_change` verification types ([#42527](supabase/supabase#42527)). This means resent confirmation emails always use the implicit flow — redirecting with tokens in the URL hash fragment (`#access_token=...`) — even when the original `signUp()` used PKCE. This creates an inconsistency where: - Initial signup email: `https://example.com/auth/confirm?code=xxx` (PKCE, works with server routes) - Resent email: `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 `/resend` endpoint now accepts optional `code_challenge` and `code_challenge_method` parameters for `signup` and `email_change` types. When provided, the endpoint: 1. Determines the flow type from `code_challenge` (PKCE if present, implicit if absent) 2. Creates a `FlowState` record for PKCE flows (needed by `/verify` to issue an auth code) 3. Passes the correct flow type to `sendConfirmation` / `sendEmailChange` This produces confirmation emails with `?code=...` query params instead of `#access_token=...` hash fragments, consistent with the initial signup flow. When `code_challenge` is not provided, behavior is **unchanged** — implicit flow is used, maintaining full backward compatibility. **Changes:** - `internal/api/resend.go`: Added `CodeChallenge` and `CodeChallengeMethod` fields to `ResendConfirmationParams`. Added PKCE param validation for email-based types. Replaced hardcoded `ImplicitFlow` with flow-aware logic for `signup` and `email_change` cases. - `internal/api/resend_test.go`: Added `TestResendPKCEValidation` (invalid PKCE params return 400) and `TestResendPKCESuccess` (signup and email change tokens get `pkce_` 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 send `code_challenge` / `code_challenge_method` in `resend()` calls when `flowType === 'pkce'`, following the same pattern already used by `signUp()` and `signInWithOtp()`. See [this PR](supabase/supabase-js#2144) The implementation mirrors the existing PKCE pattern used across the codebase (`signup.go`, `user.go`, `recover.go`, `magic_link.go`): `getFlowFromChallenge` → conditional `generateFlowState` → pass `flowType` to the email sender.
|
Auth changes are in. We're waiting for the new release, then we merge |
🔍 Description
What changed?
When
flowTypeis'pkce',resend()now generates a freshcode_verifier/code_challengepair (via the existinggetCodeChallengeAndMethodhelper), stores the verifier, and includescode_challenge+code_challenge_methodin the request body to/resend. This follows the exact same pattern already used bysignUp()andsignInWithOtp(). See this PRpackages/core/auth-js/src/GoTrueClient.ts— Added PKCE challenge generation in the email branch ofresend(), includedcode_challengeandcode_challenge_methodin the request body, and addedcode-verifiercleanup on both HTTP-error returns and in the catch block.packages/core/auth-js/test/GoTrueClient.test.ts— Added tests forresend()with PKCEflowTypecovering bothtype: 'signup'andtype: 'email_change', plus assertions thatcode_challengeis included in the request body and that the code verifier is cleaned up on HTTP errors.Supported resend types with PKCE: Both
signupandemail_changego through theif ('email' in credentials)branch and will include PKCE parameters whenflowTypeis'pkce'. The auth server PR supabase/auth#2401 supports PKCE for both types.No changes to the phone/SMS resend path, user-facing types, or implicit flow behavior.
Why was this change needed?
The GoTrue server has been updated to accept
code_challengeandcode_challenge_methodon the/resendendpoint. Previously,resend()always produced implicit-flow confirmation links (#access_token=...), even when the originalsignUp()used PKCE. This forced developers using server-side frameworks (e.g., Next.js App Router) to implement workarounds for handling hash fragments that aren't accessible in server route handlers.With this change, resend confirmation emails use
?code=...(PKCE) when the SDK is configured for PKCE flow, consistent with the initial signup email. This applies to both signup confirmation resends and email change confirmation resends.Closes supabase/supabase#42527
📸 Screenshots/Examples
Before (implicit flow on resend — hash fragment not accessible server-side):
After (PKCE flow on resend — query param works with server routes):
🔄 Breaking changes
📋 Checklist
feat(auth-js): add PKCE support to resend()npx nx formatto ensure consistent code formatting📝 Additional notes
resend()call generates a freshcode_verifier/code_challengepair — it does not reuse the pair from the originalsignUp(), since the user may be in a different browser session.ResendParamstype is unchanged; PKCE params are added internally by the SDK based on the configuredflowType, matching howsignUp()andsignInWithOtp()already work.sms,phone_change) are unaffected.signUp()handles it — rather than leaving it orphaned until the next call.