Skip to content
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

Reset multi_auth cookies on error #1957

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

ekzyis
Copy link
Member

@ekzyis ekzyis commented Mar 9, 2025

Description

fix #1821 based on #1956

I found out that we have a race condition on logout that exists without any account switching code: if a request starts before we receive the signout response (that deletes next-auth.session-token) and finishes after it, we're logged back in because it includes next-auth.session-token in the request headers and therefore also in the response headers thanks to the implicit refresh on all requests.

However, I have also seen in the timings of the network tab that it's possible that a request still includes the cookie after it was deleted AND after we reloaded the page. This is somehow related to slow requests since this is reliably reproducible if we poll for me every second but sleep for 5 seconds on the server before replying.

Since I wasn't able to prevent this from happening and I am not 100% sure that this is what causes #1821 or other issues with account switching that appear non-deterministic, I decided to show an error in the account switch dialog when we noticed that something is off with the cookies on the server.

TODO

  • also reset if user JWTs that should exist according to multi_auth are missing

Screenshots

2025-03-09-000834_515x373_scrot

Checklist

Are your changes backwards compatible? Please answer below:

yes

On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below:

tbd

For frontend changes: Tested on mobile, light and dark mode? Please answer below:

yes, tested mobile, light and dark mode

Did you introduce any new environment variables? If so, call them out explicitly here:

yes

@ekzyis ekzyis added the bug label Mar 9, 2025
@ekzyis ekzyis marked this pull request as draft March 9, 2025 06:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

logged out randomly can't switch back to account
1 participant