Repository navigation
test(auth): prove two tabs refreshing at once stay signed in (#700) - #896
Merged
Merged
Conversation
…nt burst (#700) A burst of refreshes on one cookie must answer 200 to every request, hand each the same refresh cookie and never clear the session another tab is setting. A rotated token replayed after the grace window must still answer REFRESH_REUSE_DETECTED, clear the cookies and take the family with it. The burst test fails on the code before #777 and passes on master. The handler comment still described a lost concurrent race as reuse; it no longer is since D-048. Signed-off-by: alex-dembele <alexandredembele16@gmail.com>
…700) One browser context, two tabs. The first test drops the access cookie and reloads both tabs together; the second fires /auth/refresh from both at the same instant, as in the issue's reproduction. Both tabs must keep a working or_access / or_refresh pair and stay off /login. Fails 6/6 against the backend before #777, passes 6/6 on master. Signed-off-by: alex-dembele <alexandredembele16@gmail.com>
Signed-off-by: alex-dembele <alexandredembele16@gmail.com>
Signed-off-by: alex-dembele <alexandredembele16@gmail.com>
2 tasks
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Closes #700
What
The fix itself already shipped. D-048 was implemented in #777, so a refresh replayed inside the 10-second grace window gets the same successor as the request it raced, instead of
REFRESH_REUSE_DETECTED. This PR proves that on the running product and adds the tests the issue asked for. It changes no auth code; the only production line touched is a comment inhandler.gothat still described a concurrent race as reuse.backend/internal/handler/auth/refresh_concurrency_e2e_test.go: a burst of 6 refreshes on one cookie must all answer 200, carry the sameor_refreshand clear nothing. A token replayed after the window must still get401 REFRESH_REUSE_DETECTED, clear the three cookies and kill the family.frontend/e2e/session-tabs.spec.ts: one browser context, two tabs. The first test dropsor_accessand reloads both tabs together. The second fires/auth/refreshfrom both tabs at the same instant, as in the issue's reproduction.docs/700_CONCURRENT_REFRESH.md: the spec and the results.Verified (throwaway Postgres 16 + Redis 7, master
cefe453d)9dd1c3ee)or_refresh, jar intact,/auth/me200, next refresh 200REFRESH_REUSE_DETECTED; the jar ends emptyor_refreshREFRESH_REUSE_DETECTED, 3 cookies cleared; the legitimate successor then gets 401refresh_tokensper familysession-tabs.spec.ts --repeat-each 3go test ./internal/handler/auth -run TestRefreshHandler -racego test ./internal/auth -run TestRefresh_ -race(existing, unchanged)go test ./...Depends on
The Playwright run needs #893 (PR #894). The sign-in screen crashes on master since PR #880, so
session-tabs.spec.tscannot sign in until that merges. The numbers above were taken with that one-line fix applied locally.Not done
/auth/refreshstill sends the user to/login(api.ts). That is not part of this issue, and I did not see it happen in the live pass.e2e/auth.spec.tsskips itself on every stack because of a wrong health probe (test(e2e): auth.spec.ts probes /health instead of /api/v1/health, so the whole auth suite silently skips #895). The new tests live in their own file so they actually run.