-
Notifications
You must be signed in to change notification settings - Fork 9
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
Improvement: DO-3034 support seamless handling of refresh tokens #345
Improvement: DO-3034 support seamless handling of refresh tokens #345
Conversation
…endpoint, add autorefresh logic to request helper
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.
LGTM, just missing changelog entry
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.
Looks Good Krys, left a couple of comments to think about.
|
…Vars up-to-date in WS message handlers
I realized there was an issue where in-progress live WS connections would have stale ContextVar values, such as ID_TOKEN if one is used by an auth config. ContextVars can't be updated from the outside, so I've added a new memory object stream into the WS handler object:
Added a test to verify the above behaviour. Tagging another alpha version - I'll test how that interacts with multi-tab tomorrow, I have a feeling we might run into issues when two tabs decide to refresh their tokens simultaneously |
…ad. Update GlobalState to wrap localStorage instead of in-memory for cross-tab syncing
…uent ones within a short window
Looking more into the cross-tab issues, we've decided to take a different approach.
|
Tested the new alpha locally, seems to work well - both when trying to recreate multiple tabs refreshing at the same time, and token updates are immediately propagated cross-tab. |
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.
This LGTM, thanks for this Krys much appreciated.
Motivation and Context
This PR introduces handling of refresh tokens to the framework core.
When any request fails with a 401 or 403 status code, we initiate a token refresh process.
If the current auth config supports token refresh, it can choose to use the previous expired token and attached
dara_refresh_token
cookie to produce a new session token.If successful, the token is replaced in a new global store and the request is retried.
If unsuccessful, the original failed response is returned to the caller.
Implementation Description
Client-side
Session token has been moved out of a React context into a singleton global store. This gives us more control over refreshing tokens. For example, once a token refresh is already initiated, other attempts to retrieve the token will block and wait for the in-flight refresh to go through.
The above is achieved utilizing a simple store with a locking mechanism, where in-flight refreshes are represented by promises.
Auth internals like the
useSessionToken
hook have been updated to pull/subscribe to the store instead of the context.Backend
The
refresh-token
endpoint is now included by default in dara core. The base config provides an implementation of arefresh_token
method which raises a 400 - refresh not supported - error.An auth config can be configured to support token refresh by:
dara_refresh_token
used, e.g. via customcomponents_config
and endpoints such assso-callback
for SSO authThe above is utilized in the internal SSO auth package.
Any new dependencies Introduced
How Has This Been Tested?
Unit tests added
Manually tested with internal SSO auth implementation
PR Checklist:
Screenshots (if appropriate):