Skip to content

fix(pwa): include the CSRF token in the app-bar sign-out form#52

Open
khipu-agent wants to merge 1 commit into
moshcoder:mainfrom
khipu-agent:fix/logout-csrf
Open

fix(pwa): include the CSRF token in the app-bar sign-out form#52
khipu-agent wants to merge 1 commit into
moshcoder:mainfrom
khipu-agent:fix/logout-csrf

Conversation

@khipu-agent

Copy link
Copy Markdown
Contributor

Bug

The global csrfGuard rejects every browser POST that doesn't carry a _csrf field matching the mc_csrf cookie. Every form in the app includes one via csrfInput(req) — except the Sign out form rendered by appBar() in src/lib/html.mjs, which posts to /auth/logout with no token. So clicking "Sign out" anywhere in the app always returns 403 bad csrf token and the session stays alive. Signing out is impossible from the UI.

Repro: sign in, open the dashboard (or Settings, the CLI-authorize page, or an approval page), click "Sign out" → 403. The new test in apps/pwa/test/logout-csrf.test.mjs renders the dashboard with a real session and asserts the sign-out form carries the token — it fails on current code.

Fix

appBar() now takes the request's CSRF token as a third argument and renders it as a hidden _csrf input, exactly like the other forms; the five call sites pass req.csrfToken. The guard itself is unchanged (a tokenless POST still gets 403, covered by the second test).

Tests

New integration tests in apps/pwa/test/logout-csrf.test.mjs:

  • the rendered sign-out form includes the session's _csrf value, and submitting it logs out (302 + session row deleted);
  • a POST to /auth/logout without a token is still rejected with 403 and the session survives.

Full suite: 166/166 green.

The global csrfGuard rejects any browser POST without a matching _csrf
field, and the sign-out form rendered by appBar() didn't carry one — so
clicking "Sign out" always returned 403 and the session stayed alive.
Thread req.csrfToken into appBar() like every other form already does.
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.

1 participant