feat(auth): add GET /auth/portal-logout for cross-app logout chain#24
Open
awais786 wants to merge 3 commits into
Open
feat(auth): add GET /auth/portal-logout for cross-app logout chain#24awais786 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>
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>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| const nextRaw = String(ctx.query.next ?? "").trim(); | ||
| if (nextRaw && isAllowedSignOutNext(nextRaw)) { | ||
| ctx.redirect(nextRaw); |
There was a problem hiding this comment.
Pull request overview
Adds a portal-driven logout endpoint so Outline can participate in a cross-app logout redirect chain by clearing its own cookies on its own origin before optionally redirecting to the next app.
Changes:
- Adds
GET /auth/portal-logoutto clearaccessTokenandlastSignedIn. - Adds redirect validation against
PLATFORM_DOMAIN. - Adds tests for cookie clearing, allowlisted redirects, rejected redirects, malformed URLs, and missing
next.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
server/routes/auth/index.ts |
Adds portal logout handler and redirect target validation. |
server/routes/auth/index.test.ts |
Adds endpoint coverage for redirect and cookie-clearing behavior. |
server/env.ts |
Adds PLATFORM_DOMAIN configuration used for redirect allowlisting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+117
to
+130
| // Reject non-http(s) schemes outright — javascript: and data: parse fine | ||
| // but should never be a valid next-hop. https is required in production; | ||
| // http is allowed for dev/localhost flows. | ||
| if (parsed.protocol !== "https:" && parsed.protocol !== "http:") { | ||
| return false; | ||
| } | ||
| const host = parsed.hostname.toLowerCase(); | ||
| if (!host) { | ||
| return false; | ||
| } | ||
| const platformDomain = env.PLATFORM_DOMAIN; | ||
| if (!platformDomain) { | ||
| return false; | ||
| } |
Comment on lines
+167
to
+172
| ctx.cookies.set("lastSignedIn", "", { | ||
| httpOnly: false, | ||
| sameSite: "lax", | ||
| expires: epoch, | ||
| path: "/", | ||
| }); |
| if (!platformDomain) { | ||
| return false; | ||
| } | ||
| return host === platformDomain || host.endsWith("." + platformDomain); |
Comment on lines
+551
to
+553
| .toLowerCase() | ||
| .replace(/^\.+/, "") | ||
| .trim(); |
This was referenced May 18, 2026
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
Adds `GET /auth/portal-logout?next=` so the foss-server-bundle portal's "Log out of all apps" button can include Outline in its cross-origin redirect chain.
Same pattern as Pressingly/plane#35. Each app in the chain clears its own native cookies while the browser is on that app's origin (cross-origin Set-Cookie is impossible from the portal).
What it does
`GET /auth/portal-logout?next=<absolute_url>`:
Files
Total: +252 lines.
Tests
7 cases:
Design choice: cookie-clear only, no JWT rotation
The existing `POST /api/auth.delete` rotates the user's JWT secret (invalidates every device's JWT). The new `GET /portal-logout` only clears this browser's cookies and does not rotate.
Rationale: GET endpoint is force-logout-able (an `
` triggers it). If we rotated the JWT secret on every GET hit, an attacker's force-logout would kick the victim out of all their devices, not just this browser. Cookie-only clear keeps the blast radius to "current browser only" while still serving the portal chain's purpose. ForwardAuth blocks reuse of stale cookies anyway.
If stronger per-user invalidation is needed later, can be added as a follow-up.
Out of scope
History
Previously opened as #22, closed (was based on pre-PR-#19 foss-main). Rebased on current foss-main and reopened.