Ensure sessions and tokens are synced between tabs #2498
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
estrattonbailey
commented
Jan 11, 2024
| {}, | ||
| logger.DebugContext.session, | ||
| ) | ||
| logger.warn(`session: clear current account`) |
There was a problem hiding this comment.
Sending these to Sentry so we can monitor for spikes
estrattonbailey
commented
Jan 11, 2024
| @@ -322,8 +318,8 @@ export function Provider({children}: React.PropsWithChildren<{}>) { | |||
| ) | |||
|
|
|||
| const logout = React.useCallback<ApiContext['logout']>(async () => { | |||
| logger.info(`session: logout`) | |||
There was a problem hiding this comment.
Will be included in any session: clear current account warnings
estrattonbailey
commented
Jan 11, 2024
| @@ -551,41 +547,59 @@ export function Provider({children}: React.PropsWithChildren<{}>) { | |||
| return persisted.onUpdate(() => { | |||
| const session = persisted.get('session') | |||
|
|
|||
| logger.debug(`session: onUpdate`, {}, logger.DebugContext.session) | |||
| logger.info(`session: persisted onUpdate`, {}) | |||
There was a problem hiding this comment.
Sending these to Sentry as breadcrumbs so we know if a broadcast resulted in a logout
estrattonbailey
commented
Jan 11, 2024
Comment on lines
+596
to
+600
| setState(s => ({ | ||
| ...s, | ||
| accounts: session.accounts, | ||
| currentAccount: session.currentAccount, | ||
| })) |
There was a problem hiding this comment.
We don't directly access tokens anywhere atm, but I thought it made sense to keep each tab's React state in sync too so we don't surprise ourselves later.
pfrazee
approved these changes
Jan 12, 2024
estrattonbailey
added a commit
that referenced
this pull request
Jan 18, 2024
* origin/main: (44 commits) Patch `@lingui/core` to fix `unraw` import resolution error (#2548) 1.65 Fix the fallback to discover behavior on the home feed (#2546) Bump android version code Bump ios build number Add a new home feed-api wrapper and give a header indicating the fallback behavior (#2534) Add accept-language header (#2457) rss: filter out replies server-side (#2518) feat: show muted/blocked status on list card (#2523) 1.64 (#2521) Bump [email protected] (#2519) Create a profile record on new user (#2520) fix: truncate long email address (#2493) fix: set html lang according to app language (#2496) Ensure sessions and tokens are synced between tabs (#2498) package.json: cp --verbose doesn't exist on macos (#2501) Suggest post language correction (#2486) (optional) In app browser (#2490) Toggle minimal shell on any scroll for web (#2499) ✨ New report type, appeal (#2455) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This should address the multiple reports of unexpected signouts.
The issue was with users using multiple tabs on web. Both tabs would retain the same
refreshJwtin memory, and refresh tokens can be used only once. So once a refresh token was rotated in tab A, the tab B was notified but did not update that token on theagent, and so when tab B tried to get a new access token,refreshSessionwould fail.This was a little tricky to pinpoint because if both tabs were periodically reloaded within the access token expiry window, our session handling would pick up the new refresh token from storage and continue as normal for a time. So this issue only existed in long-running multi-tab sessions.