Skip to content

Conversation

mandarini
Copy link
Contributor

Moved from: supabase/auth-js#986

Author: @j4w8n

What kind of change does this PR introduce?

Bug fix

What is the current behavior?

In some scenarios, the sb-<project-id>-auth-token-code-verifier will get created in storage, but is either not used or an error will happen during calls like updateUser(), etc. In these cases, the code verifier remains in storage, which can cause issues if third-party code any non-Supabase code uses the code search param in a url later on. See issue #1704 for an example.

This requires a client-side supabase client to be running on the page that recieves the code param.

What is the new behavior?

When the code verifier is set, and the function throws an error or returns an error (in one case), we now remove the code verifier from storage. To cover success scenarios, we remove any potential code verifier whenever a session is saved; as I wasn't sure where else to handle this.

This should resolve the issue when strictly using supabase-js, but this is not necessarily the case for the ssr library because it only enforces storage changes when a whitelisted auth event also occurs (e.g. SIGNED_IN, TOKEN_REFRESHED). Since none of the existing events seem relevant in this case, I created a new one called STORAGE_UPDATED, although this might be too generic. I tried to ensure that the session is passed along with the event; so we hopefully don't disrupt any action logic within onAuthStateChange() that a lot of devs put there. The ssr PR that coincides with this is supabase/ssr#82.

I'm not sure if this is how the team wants to resolve this issue or not. Feel free to delete this PR or slice and dice it; as it didn't spend too much time on it.

UPDATE: Noting this PR will no longer resolve this issue for users of the ssr library. I've also closed the related ssr PR.

Alternatives Considered

After some short discussion with @silentworks, one thing which could be done is renaming code; which would be breaking of course.

One other idea is to somehow introduce a new name while keeping the old name. Might be a bit complicated, even if it is possible. Then, deprecate the old usage and remove in auth-js v3.

Or, you could implement something like I've proposed here, as a temporary fix, then deprecate code in favor of something else in auth-js v3, and then roll my proposed changes back. Although, if one of the goals is to resolve the "orphaned" code verifier, then you may want to keep some of the changes.

Also, to make my proposed change a bit smaller, perhaps the ssr library can be tweaked to where storage changes are written immediately, instead of waiting for an auth state change event. That would allow you to remove the STORAGE_UPDATED event proposed here.

Additional context

fixes #1704
fixes #1697

https://discord.com/channels/839993398554656828/1312930639066169365

As seen here, https://github.com/supabase/auth-js/blob/master/src/GoTrueClient.ts#L1535, the _isPKCEFlow() check only returns true if there is a code search param in the url and the code verifier is in storage. So, if we can cleanup the code verifier once it's no longer needed in these edge cases, this resolves the issue.

@mandarini mandarini force-pushed the fix/cleanup-code-verifier branch from 2427bab to f693665 Compare October 9, 2025 13:56
@coveralls
Copy link

coveralls commented Oct 9, 2025

Coverage Status

coverage: 95.455% (+13.0%) from 82.5%
when pulling f693665 on fix/cleanup-code-verifier
into 188fa17 on master.

@mandarini mandarini requested a review from hf October 9, 2025 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PKCE flow issue with other than supabase code query in URL GoTrueClient intercepts non-Supabase initiated OAuth preventing manual login flows

3 participants