k-ye commented 1 week ago

Bug report

[x] I confirm this is a bug with Supabase, not with my own application.
[x] I confirm I have searched the Docs, GitHub Discussions, and Discord.

Describe the bug

Currently we are using Supabase's client-side Auth in our next.js app, and bumped into a problem where a few users kept getting session expired and thus requesting new refresh tokens in supa.auth.getSession() call. After following with these users, we found that their local times were a lot earlier than the access token expiration time, causing newly fetched sessions to expire immediately (https://github.com/supabase/auth-js/blob/f131300d753634fcf3fbc93dc7a762031f096749/src/GoTrueClient.ts#L1081-L1090).

Given that this Date.now() reads the epoch time purely using the user's local system info, potentially with a time drift against the server, we wonder if it is recommended, or even required, to run the Auth logic on the server side?

To Reproduce

We can fairly easily reproduce this problem:

Setting our system time to be JWT.exp + 1 minute before the actual time.
Invoking supabase.auth.getSession() would timeout the session

Expected behavior

getSession() is independent of the user's local system time.

Screenshots

If applicable, add screenshots to help explain your problem.

System information

OS: In practice, we found this to happen mostly on Windows users. (Our guess is that Mac's settings enable automatic time sync by default)
Browser (if applies) Tested on Chrome
Version of supabase-js:
- supabase-js: 2.43.4
- auth-js: 2.64.2
Version of Node.js: v18.19.0

Additional context

Add any other context about the problem here.

k-ye commented 1 week ago

By turning on debug logging, I think it was pretty obvious what has happened. Here's the log for two consecutive call to auth.getSession(), after advancing my local system's time.

# 1st getSession()
GoTrueClient.js:47 GoTrueClient@0 (2.64.2) 2024-06-19T10:19:47.181Z #_useSession begin
GoTrueClient.js:47 GoTrueClient@0 (2.64.2) 2024-06-19T10:19:47.181Z #__loadSession() begin
GoTrueClient.js:47 GoTrueClient@0 (2.64.2) 2024-06-19T10:19:47.181Z #getSession() session from storage {access_token: '***', token_type: 'bearer', expires_in: 900, expires_at: 1718789692, refresh_token: '***', …}
GoTrueClient.js:47 GoTrueClient@0 (2.64.2) 2024-06-19T10:19:47.181Z #__loadSession() session has expired expires_at 1718789692
GoTrueClient.js:47 GoTrueClient@0 (2.64.2) 2024-06-19T10:19:47.181Z #_callRefreshToken(*...) begin
GoTrueClient.js:47 GoTrueClient@0 (2.64.2) 2024-06-19T10:19:47.181Z #_refreshAccessToken(*...) begin
GoTrueClient.js:47 GoTrueClient@0 (2.64.2) 2024-06-19T10:19:47.181Z #_refreshAccessToken(*...) refreshing attempt 0
GoTrueClient.js:47 GoTrueClient@0 (2.64.2) 2024-06-19T10:19:47.496Z #_refreshAccessToken(*...) end
GoTrueClient.js:47 GoTrueClient@0 (2.64.2) 2024-06-19T10:19:47.496Z #_saveSession() {access_token: '***', token_type: 'bearer', expires_in: 900, expires_at: 1718789692, refresh_token: '***', …}
GoTrueClient.js:47 GoTrueClient@0 (2.64.2) 2024-06-19T10:19:47.496Z #_notifyAllSubscribers(TOKEN_REFRESHED) begin {access_token: '***', token_type: 'bearer', expires_in: 900, expires_at: 1718789692, refresh_token: '***', …} broadcast = true
GoTrueClient.js:47 GoTrueClient@0 (2.64.2) 2024-06-19T10:19:47.496Z #_notifyAllSubscribers(TOKEN_REFRESHED) end
GoTrueClient.js:47 GoTrueClient@0 (2.64.2) 2024-06-19T10:19:47.496Z #_callRefreshToken(*...) end
GoTrueClient.js:47 GoTrueClient@0 (2.64.2) 2024-06-19T10:19:47.497Z #__loadSession() end
GoTrueClient.js:47 GoTrueClient@0 (2.64.2) 2024-06-19T10:19:47.497Z #_useSession end
# 2nd getSession()
GoTrueClient.js:47 GoTrueClient@0 (2.64.2) 2024-06-19T10:19:47.497Z #_useSession begin
GoTrueClient.js:47 GoTrueClient@0 (2.64.2) 2024-06-19T10:19:47.497Z #__loadSession() begin
GoTrueClient.js:47 GoTrueClient@0 (2.64.2) 2024-06-19T10:19:47.497Z #getSession() session from storage {access_token: '***', token_type: 'bearer', expires_in: 900, expires_at: 1718789692, refresh_token: '***', …}
GoTrueClient.js:47 GoTrueClient@0 (2.64.2) 2024-06-19T10:19:47.497Z #__loadSession() session has expired expires_at 1718789692
GoTrueClient.js:47 GoTrueClient@0 (2.64.2) 2024-06-19T10:19:47.497Z #_callRefreshToken(*...) begin
GoTrueClient.js:47 GoTrueClient@0 (2.64.2) 2024-06-19T10:19:47.497Z #_refreshAccessToken(*...) begin
GoTrueClient.js:47 GoTrueClient@0 (2.64.2) 2024-06-19T10:19:47.497Z #_refreshAccessToken(*...) refreshing attempt 0
GoTrueClient.js:47 GoTrueClient@0 (2.64.2) 2024-06-19T10:19:47.771Z #_refreshAccessToken(*...) end
GoTrueClient.js:47 GoTrueClient@0 (2.64.2) 2024-06-19T10:19:47.771Z #_saveSession() {access_token: '***', token_type: 'bearer', expires_in: 900, expires_at: 1718789692, refresh_token: '***', …}
GoTrueClient.js:47 GoTrueClient@0 (2.64.2) 2024-06-19T10:19:47.771Z #_notifyAllSubscribers(TOKEN_REFRESHED) begin {access_token: '***', token_type: 'bearer', expires_in: 900, expires_at: 1718789692, refresh_token: '***', …} broadcast = true
GoTrueClient.js:47 GoTrueClient@0 (2.64.2) 2024-06-19T10:19:47.771Z #_notifyAllSubscribers(TOKEN_REFRESHED) end
GoTrueClient.js:47 GoTrueClient@0 (2.64.2) 2024-06-19T10:19:47.772Z #_callRefreshToken(*...) end
GoTrueClient.js:47 GoTrueClient@0 (2.64.2) 2024-06-19T10:19:47.772Z #__loadSession() end
GoTrueClient.js:47 GoTrueClient@0 (2.64.2) 2024-06-19T10:19:47.772Z #_useSession end

I believe the key problem is that, as mentioned above, the SDK is effectively comparing expires_at (timestamp returned by server) against the client's local time: https://github.com/supabase/auth-js/blob/f131300d753634fcf3fbc93dc7a762031f096749/src/GoTrueClient.ts#L1081-L1082

session's expire_at is set here: https://github.com/supabase/auth-js/blob/f131300d753634fcf3fbc93dc7a762031f096749/src/lib/fetch.ts#L206-L208. /token?grant_type=refresh_token's response does contain an expire_at field, among other things like access_token and refresh_token.

k-ye commented 1 week ago

I think this is caused by a server change last year, where you've introduced the expire_at field in https://github.com/supabase/auth/pull/1183.

Then the client started reading expire_at in https://github.com/supabase/auth-js/pull/735, instead of computing a local expiry based on Date.now() + expire_in.

hf commented 1 week ago

Clock drift issues are not something we can fix easily. The change to use expire_at instead of expire_in was intentional as that calculation with expire_in is not mathematically sound.

One way you can handle this in your app is to detect that the user's clock has drift and show them a banner. You can do this easily by having an API endpoint in your app that returns the current server time, and compare it with the Date.now() time. If it's off by more than a minute or so you should show that banner.

In future versions of the library we may add native support for detecting clock drift, but there is no way for us to use "relative" time.

Please also be aware that the user's clock is not absolute. A user can change their clock at any time, causing Date.now() to jump back/forth a lot, so there really is nothing we can do about issues like these. But you can handle it in your app with a warning.

supabase / auth-js

Client-side Supabase Auth Session/Access Token Expires Too Soon #926

Bug report

Describe the bug

To Reproduce

Expected behavior

Screenshots

System information

Additional context