-
-
Notifications
You must be signed in to change notification settings - Fork 153
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add setSession
support for a SSR context
#445
Conversation
d8f83b1
to
b8ddf6e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we're trying to do too much here and i'm not sure what this PR is trying to change - the current method just requires a refresh token, forces a refresh and then sets the current session.
Why do we also want to support setting a session via an access token? If no refresh token is being passed in, it could result in a incomplete session returned.
Here's a Mermaid description of the issue: sequenceDiagram
participant G as GoTrue
participant S as Server
participant B as Browser
participant C as Cookies
participant L as Local Storage
B-->>S: GET /login
S-->>B: 200 OK Render /login
B-->>G: POST /sign-in { email, password }
G-->>B: 303 See Other server/authenticate...
B-->>S: GET /authenticate
S-->>B: 200 OK Render /authenticate
activate B
B-->>B: getSessionFromURL
B-->>L: putItem(session, ...)
B-->>C: document.cookie = 'my-access-token...'
B-->>C: document.cookie = 'my-refresh-token...'
deactivate B
note over G, L: After 1 week the user comes back to /route on server -- no issues
activate B
B-->>S: GET /route
B-->>S: Cookie: my-access-token ...
B-->>S: Cookie: my-refresh-token ...
activate S
S-->>S: setSession(req.cookies['my-refresh-token'])
S-->>G: POST /verify?grant_type=refresh_token
G-->>S: 200 OK access token, refresh token
S-->>S: return session information to app
S-->>B: 200 OK prerendered content
deactivate S
B-->>B: _refreshSession
B-->>L: getItem(session)
L-->>B: session { access_token, refresh_token, expires_at }
B-->>G: POST /verify?grant_type=refresh_token
G-->>B: 200 OK access token, refresh token
B-->>L: putItem(session)
deactivate B
note over G, L: Immediately after login user navigates to /route -- issue after 1 hour
activate B
B-->>S: GET /route
B-->>S: Cookie: my-access-token ...
B-->>S: Cookie: my-refresh-token ...
activate S
S-->>S: setSession(req.cookies['my-refresh-token'])
S-->>G: POST /token?grant_type=refresh_token
G-->>S: 200 OK access token, refresh token
S-->>S: return session information to app
note over B, S: Start reuse timer!
S-->>B: 200 OK prerendered content
deactivate S
B-->>B: _refreshSession
B-->>L: getItem(session)
L-->>B: session { access_token, refresh_token, expires_at }
note over L, B: now is before expires_at!
note over L, B: No need to renew refresh token
note over G, L: 1 hour passes, app notices expires_at and tries to refresh token
B-->>G: POST /token?grant_type=refresh_token
G-->>B: 401 Unauthorized refresh token already used
note over G, B: 1 hour passed after the first validation of the refresh token!
deactivate B
note over G, L: With setSession({ access_token, refresh_token }) fix -- no-issue
activate B
B-->>S: GET /route
B-->>S: Cookie: my-access-token ...
B-->>S: Cookie: my-refresh-token ...
activate S
S-->>S: setSession({ refresh_token: req.cookies['my-refresh-token']), access_token: ... })
S-->>S: access token is not expired no post to GoTrue
S-->>S: return session information to app
note over B, S: Start reuse timer!
S-->>B: 200 OK prerendered content
deactivate S
B-->>B: _refreshSession
B-->>L: getItem(session)
L-->>B: session { access_token, refresh_token, expires_at }
note over L, B: now is before expires_at!
note over L, B: No need to renew refresh token
note over G, L: 1 hour passes, app notices expires_at and tries to refresh token
B-->>G: POST /token?grant_type=refresh_token
G-->>B: 200 OK access_token, refresh_token
note over G, B: 1 hour passed after the first validation of the refresh token!
B-->>L: putItem(session, ...)
B-->>C: document.cookie = 'my-access-token...'
B-->>C: document.cookie = 'my-refresh-token...'
note over L, G: Ok but what if the user closes the tab until 1 hour passes and then navigates to /route
B-->>S: GET /route
B-->>S: Cookie: my-access-token ...
B-->>S: Cookie: my-refresh-token ...
activate S
S-->>S: setSession({ refresh_token: req.cookies['my-refresh-token']), access_token: ... })
S-->>S: access token IS expired, refreshing in GoTrue
S-->>G: POST /token?grant_type=refresh_token
G-->>S: 200 OK access token, refresh token
S-->>S: return session information to app
note over B, S: Start reuse timer!
S-->>B: 200 OK prerendered content
deactivate S
B-->>B: _refreshSession
B-->>L: getItem(session)
L-->>B: session { access_token, refresh_token, expires_at }
B-->>B: now is after expires_at!
B-->>G: POST /token?grant_type=refresh_token
note over B, G: this occurs milliseconds after the refresh from the server
note over B, G: thus the refresh_token although used is within a reuse period
G-->>B: 200 OK access_token, refresh_token
B-->>L: putItem(session, ...)
B-->>C: document.cookie = 'my-access-token...'
B-->>C: document.cookie = 'my-refresh-token...'
note over G, L: All is good in the world!
deactivate B
|
b8ddf6e
to
9bc250c
Compare
9bc250c
to
1813591
Compare
a610ec3
to
0d33675
Compare
I need to improve the types still, I don't want someone merging this accidentally. :D
0d33675
to
e8a5355
Compare
e8a5355
to
be413ca
Compare
🎉 This PR is included in version 1.24.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
🎉 This PR is included in version 2.0.0-rc.11 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Adds support for calling
setSession()
in a server-side rendering context. When used in SSR, the access and refresh tokens are typically sent from the browser to the server. Both of them represent the user's session.Users can thus call:
To correctly extract the session information from the two cookies. The client can then be used to call other APIs and automatic refresh token management will take place.