Skip to content

fix: reauthenticate expired dashboard sessions - #342

Merged
ecarreras merged 2 commits into
mainfrom
fix/issue-341-expired-session
Nov 11, 2026
Merged

ecarreras merged 2 commits into
mainfrom
fix/issue-341-expired-session

Conversation

@giscebot

@giscebot giscebot commented Nov 11, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • redirect dashboard API responses with status 401 to the GitHub OAuth login flow through a page-scoped single-flight guard
  • replace the expired page in browser history to avoid returning to the stale session
  • cover concurrent 401 responses with a regression test and rebuild the packaged dashboard assets

Tests

  • npm test (80 passed)
  • npm run build
  • pytest -q (713 passed; 1 pre-existing Starlette deprecation warning)

Closes #341

Co-authored-by: ecarreras <ecarreras@users.noreply.github.com>

@pilipilisbot pilipilisbot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
@giscebot

Copy link
Copy Markdown
Collaborator Author

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:

  • npm test — 80 passed
  • npm run build — passed
  • pytest -q — 713 passed (1 pre-existing Starlette deprecation warning)

The packaged dashboard assets and PR summary are updated as well.

@pilipilisbot pilipilisbot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@ecarreras
ecarreras merged commit 68c55ff into main Nov 11, 2026
4 checks passed
@ecarreras
ecarreras deleted the fix/issue-341-expired-session branch November 11, 2026 10:09
@pilipilisbot

Copy link
Copy Markdown
Collaborator

Post-merge sync complete. PR #342 is merged as 68c55ff8e1da15b6f39842a20ed26191d071dcc7 (final head 3693320f46c02de3f01ac6446768b5faa0dac8c7). The earlier single-flight OAuth-state review concern was resolved on the final head and approved before merge. Dashboard and Python 3.11/3.12 CI jobs passed.

I removed the clean dedicated worktree /home/openclaw/projects/gisce-ti/worktrees/github-agent-bridge-pr-342 and verified it is no longer registered or present. The canonical checkout was left untouched.

@giscebot

Copy link
Copy Markdown
Collaborator Author

Post-merge follow-up: PR #342 is merged as 68c55ff8e1da15b6f39842a20ed26191d071dcc7. The dedicated worktree github-agent-bridge-issue-341 was clean and has been removed; the canonical checkout was left untouched. CI was green on the merged head (dashboard, pytest 3.11, pytest 3.12). The single-flight redirect fix and concurrent-401 regression are therefore landed; production rollout/observation remains a separate operational step.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Quan tornes a la pàgina i ha caducat la sessió surt 401 i hauria de demanar autentificació

3 participants