search

supabase / auth-js

An isomorphic Javascript library for Supabase Auth.
MIT License
355 stars 160 forks source link

fix: don't refresh session if autoRefreshToken setting is set to false #925

Open thomasconner opened 3 months ago

thomasconner commented 3 months ago

What kind of change does this PR introduce?

Bug fix - Supabase client will obey autoRefreshToken setting when setting a session with an expired access token

What is the current behavior?

When a Supabase client is initialized with { auth: { autoRefreshToken: false } }

const client = createClient(options.supabaseUrl, options.supabaseKey, {
    auth: { autoRefreshToken: false }
});

but a session is set with client.setSession() and the access token has expired the client will disregard the autoRefreshToken setting and attempt to refresh the session.

What is the new behavior?

The client will not attempt to refresh an expired session when it is set with client.setSession() and autoRefreshToken is set to false.

j4w8n commented 3 months ago

What's your use case here, and what issue are you running into?

thomasconner commented 3 months ago

I am using the Supabase JavaScript client in a NestJS service. This service retrieves the access token from the incoming request to set the session. This setting of the session should not refresh the token automatically on the server side but instead just return an UnauthorizedError by the service to the requesting application. It is up to the requesting application to then properly refresh the access token.

thomasconner commented 3 months ago

I removed the change that makes the refresh_token optional on a session. This minimizes the changes but still achieves the behavior I would expect.

j4w8n commented 3 months ago

After setting the session, do you use methods like getSession() and getUser()?

If not, then this PR would be more applicable once it's released: https://github.com/supabase/supabase-js/pull/1004

thomasconner commented 3 months ago

In my opinion, supabase/supabase-js#1004 and my PR solve/fix two different things. supabase/supabase-js#1004 solves the problem of providing an access token to the Supabase client to make API requests. However, my PR I believe still is fixing a bug where the autoRefreshToken setting is not obeyed when calling supabase.setSession().

I would end up using the new API on the server side as described in that PR you linked to. That is a nice solution.

thomasconner commented 3 months ago

Do you have anymore feedback regarding this PR?

j4w8n commented 3 months ago

Do you have anymore feedback regarding this PR?

Moving this PR forward isn't up to me, as I don't have approval permissions. I know the auth team can get pretty busy, so I'm not sure if/when they will comment.

I think there could be some more things to think about with this, because the _loadSession method also checks for token expiration.