feat(auth): add GET /auth/portal-logout for cross-app logout chain#22
Closed
awais786 wants to merge 3 commits into
Closed
feat(auth): add GET /auth/portal-logout for cross-app logout chain#22awais786 wants to merge 3 commits into
awais786 wants to merge 3 commits into
Conversation
Lets the foss-server-bundle portal's "Log out of all apps" button
include Outline in the redirect chain. Browser security forbids
cross-origin Set-Cookie, so the portal cannot clear Outline's
accessToken cookie on docs.<domain> directly — it has to navigate the
browser to a same-origin Outline URL that clears the cookie itself.
GET /auth/portal-logout?next=<absolute_url>
1. Clears accessToken + lastSignedIn cookies (same shape as the
auth() catch block already uses).
2. Validates next against MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS — suffix
match on a dot boundary, so foss.arbisoft.com matches its
subdomains but NOT foss.arbisoft.com.evil.example.
3. 302s to next, or returns 200 if next is missing/invalid (cookies
still cleared).
CSRF-exempt because the portal cannot share Outline's CSRF token
cross-origin. Residual risk is force-logout (img-tag embedding) — low
impact: ForwardAuth re-auths on the next request.
Empty allowlist rejects every ?next= — endpoint still clears cookies,
just won't redirect. Non-SSO deployments unaffected by default.
Mirrors Pressingly/plane#31 for the same flow on Plane's side.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
|
||
| const nextRaw = String(ctx.query.next ?? "").trim(); | ||
| if (nextRaw && isAllowedSignOutNext(nextRaw)) { | ||
| ctx.redirect(nextRaw); |
Two bugs from the review of the initial commit: 1. Cookies were not actually being cleared. Koa's ctx.cookies.set defaults Set-Cookie's path to the request URL, so the clear Set-Cookie was scoped to /auth/portal-logout instead of /. Browsers identify cookies by (name, domain, path) — a path=/auth/portal-logout Set-Cookie does not shadow the path=/ accessToken cookie issued at login. End-to-end: clicking "Log out of all apps" would 302 cleanly but leave the JWT in the browser. Adding explicit path:"/" matches the cookie-clear shape already used in auth() catch block. 2. next= validation accepted any URL scheme that new URL() could parse. javascript:alert(1) and data: URLs passed (hostname is empty so the allowlist still rejected them, but defense-in-depth wants an explicit scheme gate). Now: only http: and https: are accepted; everything else is rejected before the host check. Tests: - New cookieClearedAtRoot regex helper asserts path=/ in Set-Cookie attributes — would have caught bug #1. - New "should reject next with non-http(s) schemes" case covers javascript:, data:, file:. - New "should accept both http and https for allowlisted hosts" case pins the allowed scheme set. - Removed the dead `if (!entry) continue` guard — the env normalisation already filters empties. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS was redundant — the foss-server-bundle already sets PLATFORM_DOMAIN (via platform.sh), and every legitimate next-hop is by definition a subdomain of that. One fewer env var for operators to set, one fewer place where the bundle config can drift. Behaviour preserved exactly: PLATFORM_DOMAIN treated as a single host suffix, matched on a dot boundary. Unset → every ?next= rejected (the same "safe-fail" the old empty-list case had). Also tightens a leftover loose cookie-clear assertion the second review flagged — the "reject host outside allowlist" test now asserts cookies cleared at path=/, same as its peers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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.
Summary
Lets the foss-server-bundle portal's "Log out of all apps" button include Outline in the redirect chain. Browser security forbids cross-origin
Set-Cookie, so the portal cannot clear Outline'saccessTokencookie ondocs.<domain>directly — it has to navigate the browser to a same-origin Outline URL that clears the cookie itself.Endpoint
GET /auth/portal-logout?next=<absolute_url>accessToken+lastSignedIncookies (same shape as theauth()catch block already uses for cookie-transported 401s).nextagainstMPASS_SIGNOUT_NEXT_ALLOWED_HOSTS— suffix match on a dot boundary, sofoss.arbisoft.commatches subdomains but notfoss.arbisoft.com.evil.example.next, or returns 200 +{ok: true}ifnextis missing/invalid (cookies still cleared either way).CSRF-exempt: the portal cannot share Outline's CSRF token cross-origin. Residual risk is force-logout (
<img>embedding) — low impact, ForwardAuth re-auths on the next request.New env var
Operators normalise on write: leading dots are stripped, lowercased, blanks filtered.
Portal-side wiring
Once this lands, the portal can do:
Or chain through multiple apps:
Test plan
pnpm test server/routes/auth/index.test.ts— 8 new cases in theauth/portal-logoutdescribe block:accessToken+lastSignedIncookies on every call (incl. empty allowlist)nexthostdocs.foss.arbisoft.comagainstfoss.arbisoft.com)foss.arbisoft.com.evil)?next=?next=rejected?next=→ 200 + cookies clearedpnpm tsc --noEmitcleanMPASS_SIGNOUT_NEXT_ALLOWED_HOSTS=foss.arbisoft.com,localhostin dev env, log in to Outline, thencurl -i 'http://docs.localhost/auth/portal-logout?next=http://localhost'— expect 302 + Set-Cookie clearing both cookies.Relation to other work
MPASS_SIGNOUT_NEXT_ALLOWED_HOSTSenv var name across apps so operators set it once.Risk / rollback
Zero behaviour change for non-SSO deployments:
MPASS_SIGNOUT_NEXT_ALLOWED_HOSTSdefaults to empty → every?next=is rejected (cookies cleared, no redirect). To disable post-merge, unset the env var.🤖 Generated with Claude Code