Repository navigation
fix: reauthenticate expired dashboard sessions - #342
Conversation
Co-authored-by: ecarreras <ecarreras@users.noreply.github.com>
pilipilisbot
left a comment
There was a problem hiding this comment.
The redirect is in the right place (the shared API helper), and the OAuth endpoint matches the backend flow. However, the redirect needs to be single-flight before this is merged.
On a dashboard load, /api/me, /api/status, /api/about, and route-specific queries run concurrently. Once the session expires, each 401 calls window.location.replace("/auth/login"); React Query also retries failed queries once. Every /auth/login response generates a fresh OAuth state and overwrites the single OAUTH_STATE_COOKIE. Multiple login navigations can therefore race: the GitHub authorization page may carry one state while the cookie contains another, and /auth/callback then rejects with oauth_state_mismatch (401). That reproduces an authentication failure in the scenario this PR is intended to fix.
Please guard the redirect so only the first 401 starts the login flow (and cover concurrent 401s in a test). I checked the source diff, the dashboard query startup/retry behavior, and the backend login/callback state-cookie contract. The dashboard and Python CI checks are green; this is a flow-level issue those tests do not exercise.
Co-authored-by: pilipilisbot <pilipilisbot@users.noreply.github.com>
|
Addressed the single-flight concern in 3693320. The dashboard now uses one page-scoped unauthorized handler: the first 401 starts /auth/login, while concurrent 401 responses and later React Query retries only reject their requests. The regression runs concurrent /api/me and /api/status requests and asserts one redirect while both requests fail with 401. Validated against current main:
The packaged dashboard assets and PR summary are updated as well. |
pilipilisbot
left a comment
There was a problem hiding this comment.
Reviewed head 3693320. The page-scoped handler now starts /auth/login only on the first API 401, including when dashboard queries run concurrently or React Query retries. This resolves my earlier OAuth-state race concern: the backend replaces the single state cookie on each login request, while the callback requires it to match. The concurrent-401 regression covers one redirect and rejection of both requests; the packaged asset contains the login path.
I checked the source diff, backend login/callback and session-expiry contract, current base mergeability, and the dashboard plus Python CI jobs on this head (all passed). No remaining blocking findings. I did not rerun the suites locally because this review worktree has no dashboard dependencies installed.
|
Post-merge sync complete. PR #342 is merged as I removed the clean dedicated worktree |
|
Post-merge follow-up: PR #342 is merged as |
Summary
Tests
Closes #341