From 15b49336dedb46efa13b3cc0dd23e4dcafa49b22 Mon Sep 17 00:00:00 2001 From: awais786 Date: Sun, 17 May 2026 00:27:30 +0500 Subject: [PATCH 1/3] feat(auth): add GET /auth/portal-logout for cross-app logout chain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. directly — it has to navigate the browser to a same-origin Outline URL that clears the cookie itself. GET /auth/portal-logout?next= 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) --- server/env.ts | 17 ++++++ server/routes/auth/index.test.ts | 102 +++++++++++++++++++++++++++++++ server/routes/auth/index.ts | 70 +++++++++++++++++++++ 3 files changed, 189 insertions(+) diff --git a/server/env.ts b/server/env.ts index bba2e35bdab0..fa26131f6f98 100644 --- a/server/env.ts +++ b/server/env.ts @@ -536,6 +536,23 @@ export class Environment { @IsOptional() public DEFAULT_EMAIL_DOMAIN = environment.DEFAULT_EMAIL_DOMAIN ?? "askii.ai"; + /** + * Comma-separated host suffixes the `/auth/portal-logout?next=…` endpoint + * is allowed to redirect to. Each entry matches its exact host plus all + * subdomains (e.g. `foss.arbisoft.com` matches `pm.foss.arbisoft.com`). + * Empty list rejects every `?next=` — the endpoint still clears the + * accessToken cookie, it just won't follow a redirect. + * + * Used by the foss-server-bundle portal's "Log out of all apps" + * redirect chain. + */ + @IsOptional() + public MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS = ( + this.toOptionalCommaList(environment.MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS) ?? [] + ) + .map((h) => h.toLowerCase().replace(/^\.+/, "")) + .filter((h) => h.length > 0); + /** * A boolean switch to toggle the rate limiter at application web server. */ diff --git a/server/routes/auth/index.test.ts b/server/routes/auth/index.test.ts index 9f29fc5a97d1..aa4c065d6b87 100644 --- a/server/routes/auth/index.test.ts +++ b/server/routes/auth/index.test.ts @@ -1,3 +1,4 @@ +import env from "@server/env"; import { buildUser, buildCollection } from "@server/test/factories"; import { getTestServer } from "@server/test/support"; import { JWT_COOKIE_TTL_DAYS } from "@server/utils/authentication"; @@ -81,3 +82,104 @@ describe("auth/redirect", () => { expect(ageMs).toBeLessThan(expectedMs + 60_000); }); }); + +describe("auth/portal-logout", () => { + let originalAllowlist: string[]; + + beforeEach(() => { + originalAllowlist = env.MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS; + }); + + afterEach(() => { + env.MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS = originalAllowlist; + }); + + const setAllowlist = (entries: string[]) => { + env.MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS = entries; + }; + + it("should clear accessToken and lastSignedIn cookies on every call", async () => { + setAllowlist([]); + const res = await server.get("/auth/portal-logout", { redirect: "manual" }); + expect(res.status).toEqual(200); + const setCookie = res.headers.get("set-cookie") ?? ""; + expect(setCookie).toMatch(/accessToken=;/); + expect(setCookie).toMatch(/lastSignedIn=;/); + }); + + it("should 302 to next when host is in the allowlist", async () => { + setAllowlist(["foss.arbisoft.com"]); + const target = "https://pm.foss.arbisoft.com/auth/portal-sign-out/"; + const res = await server.get( + `/auth/portal-logout?next=${encodeURIComponent(target)}`, + { redirect: "manual" } + ); + expect(res.status).toEqual(302); + expect(res.headers.get("location")).toEqual(target); + // cookies still cleared + const setCookie = res.headers.get("set-cookie") ?? ""; + expect(setCookie).toMatch(/accessToken=;/); + }); + + it("should match subdomains of allowlist entries", async () => { + setAllowlist(["foss.arbisoft.com"]); + const target = "https://docs.foss.arbisoft.com/done"; + const res = await server.get( + `/auth/portal-logout?next=${encodeURIComponent(target)}`, + { redirect: "manual" } + ); + expect(res.status).toEqual(302); + expect(res.headers.get("location")).toEqual(target); + }); + + it("should reject next on a host outside the allowlist", async () => { + setAllowlist(["foss.arbisoft.com"]); + const target = "https://evil.example/steal"; + const res = await server.get( + `/auth/portal-logout?next=${encodeURIComponent(target)}`, + { redirect: "manual" } + ); + expect(res.status).toEqual(200); + // still clears cookies — non-redirect rejection isn't an error path + const setCookie = res.headers.get("set-cookie") ?? ""; + expect(setCookie).toMatch(/accessToken=;/); + }); + + it("should enforce dot-boundary on suffix matches", async () => { + // "foss.arbisoft.com.evil" must NOT match "foss.arbisoft.com". + setAllowlist(["foss.arbisoft.com"]); + const target = "https://foss.arbisoft.com.evil.example/x"; + const res = await server.get( + `/auth/portal-logout?next=${encodeURIComponent(target)}`, + { redirect: "manual" } + ); + expect(res.status).toEqual(200); + }); + + it("should reject every next when the allowlist is empty", async () => { + setAllowlist([]); + const res = await server.get( + "/auth/portal-logout?next=https%3A%2F%2Ffoss.arbisoft.com%2F", + { redirect: "manual" } + ); + expect(res.status).toEqual(200); + }); + + it("should reject malformed next values", async () => { + setAllowlist(["foss.arbisoft.com"]); + const res = await server.get( + "/auth/portal-logout?next=not-a-url", + { redirect: "manual" } + ); + expect(res.status).toEqual(200); + }); + + it("should not 302 when next is missing", async () => { + setAllowlist(["foss.arbisoft.com"]); + const res = await server.get("/auth/portal-logout", { redirect: "manual" }); + expect(res.status).toEqual(200); + // cookies still cleared + const setCookie = res.headers.get("set-cookie") ?? ""; + expect(setCookie).toMatch(/accessToken=;/); + }); +}); diff --git a/server/routes/auth/index.ts b/server/routes/auth/index.ts index d0d6be6c52b1..ef9cd8bf3942 100644 --- a/server/routes/auth/index.ts +++ b/server/routes/auth/index.ts @@ -3,6 +3,7 @@ import { addDays } from "date-fns"; import Koa from "koa"; import bodyParser from "koa-body"; import Router from "koa-router"; +import env from "@server/env"; import { AuthenticationError } from "@server/errors"; import authMiddleware from "@server/middlewares/authentication"; import coalesceBody from "@server/middlewares/coaleseBody"; @@ -93,6 +94,75 @@ router.get("/redirect", authMiddleware(), async (ctx: APIContext) => { ); }); +/** + * Returns true iff `url`'s hostname matches an entry in + * `MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS`. Suffix match enforces a dot boundary: + * `foss.arbisoft.com` matches `foss.arbisoft.com` and `*.foss.arbisoft.com` + * but NOT `foss.arbisoft.com.evil.example`. Closes the obvious open-redirect + * surface the endpoint would otherwise expose. + */ +function isAllowedSignOutNext(url: string): boolean { + let host: string; + try { + host = new URL(url).hostname.toLowerCase(); + } catch { + return false; + } + if (!host) { + return false; + } + for (const entry of env.MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS) { + if (!entry) { + continue; + } + if (host === entry || host.endsWith("." + entry)) { + return true; + } + } + return false; +} + +/** + * GET /auth/portal-logout?next= + * + * Clears the accessToken + lastSignedIn cookies and 302-redirects to + * `next`. Designed for the foss-server-bundle portal's "Log out of all + * apps" redirect chain — the portal navigates the browser through each + * app's logout URL, each step clearing its own cookies while the browser + * is on that app's own domain (so the Set-Cookie scope is correct). + * + * CSRF-exempt by design: the portal cannot share Outline's CSRF token + * cross-origin, so the existing POST + CSRF flow isn't usable here. The + * residual risk is force-logout (an attacker embeds an `` and the + * victim's session ends). Low impact — the only state lost is the + * cookie, and ForwardAuth re-auths on the next request. + * + * Open-redirect protection: `next` is validated against + * `MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS` (suffix-match on a dot boundary). + * Empty allowlist rejects every `next` — cookies are still cleared, the + * endpoint just returns 200 instead of 302. + */ +router.get("/portal-logout", async (ctx: APIContext) => { + const epoch = new Date(0); + // Match the cookie-clear shape used by the auth() catch block at + // server/middlewares/authentication.ts: same names, same path, same + // sameSite. lastSignedIn is non-HttpOnly because the frontend reads it. + ctx.cookies.set("accessToken", "", { sameSite: "lax", expires: epoch }); + ctx.cookies.set("lastSignedIn", "", { + httpOnly: false, + sameSite: "lax", + expires: epoch, + }); + + const nextRaw = String(ctx.query.next ?? "").trim(); + if (nextRaw && isAllowedSignOutNext(nextRaw)) { + ctx.redirect(nextRaw); + return; + } + + ctx.body = { ok: true }; +}); + app.use(bodyParser()); app.use(coalesceBody()); app.use(verifyCSRFToken()); From 2a2aca94ae7e67cf726f4854d3aea84cdb83c9c5 Mon Sep 17 00:00:00 2001 From: awais786 Date: Sun, 17 May 2026 00:32:02 +0500 Subject: [PATCH 2/3] fix(auth): add path:/ to cookie clears + gate next= scheme MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- server/routes/auth/index.test.ts | 61 ++++++++++++++++++++++++++++---- server/routes/auth/index.ts | 41 ++++++++++++++------- 2 files changed, 82 insertions(+), 20 deletions(-) diff --git a/server/routes/auth/index.test.ts b/server/routes/auth/index.test.ts index aa4c065d6b87..4224edca620c 100644 --- a/server/routes/auth/index.test.ts +++ b/server/routes/auth/index.test.ts @@ -98,13 +98,25 @@ describe("auth/portal-logout", () => { env.MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS = entries; }; - it("should clear accessToken and lastSignedIn cookies on every call", async () => { + /** + * Matches `=;` followed (anywhere up to the next cookie boundary) + * by `path=/`. Regression guard for the path-scoping bug: without + * `path=/`, Set-Cookie defaults to `/auth/portal-logout`, which fails + * to shadow the original `path=/` cookie — the browser keeps the JWT + * and "logout" silently does nothing. + */ + const cookieClearedAtRoot = (name: string) => + new RegExp(`${name}=;[^,]*\\bpath=/(?:[;,]|$)`, "i"); + + it("should clear accessToken and lastSignedIn cookies at path=/ on every call", async () => { setAllowlist([]); const res = await server.get("/auth/portal-logout", { redirect: "manual" }); expect(res.status).toEqual(200); const setCookie = res.headers.get("set-cookie") ?? ""; - expect(setCookie).toMatch(/accessToken=;/); - expect(setCookie).toMatch(/lastSignedIn=;/); + // Both cookies cleared AND scoped to path=/ so they actually shadow + // the original login cookies in the browser. + expect(setCookie).toMatch(cookieClearedAtRoot("accessToken")); + expect(setCookie).toMatch(cookieClearedAtRoot("lastSignedIn")); }); it("should 302 to next when host is in the allowlist", async () => { @@ -116,9 +128,9 @@ describe("auth/portal-logout", () => { ); expect(res.status).toEqual(302); expect(res.headers.get("location")).toEqual(target); - // cookies still cleared + // cookies still cleared at path=/ const setCookie = res.headers.get("set-cookie") ?? ""; - expect(setCookie).toMatch(/accessToken=;/); + expect(setCookie).toMatch(cookieClearedAtRoot("accessToken")); }); it("should match subdomains of allowlist entries", async () => { @@ -174,12 +186,47 @@ describe("auth/portal-logout", () => { expect(res.status).toEqual(200); }); + it("should reject next with non-http(s) schemes", async () => { + // javascript:, data:, file:, etc. parse fine via new URL() but must + // never be a valid next-hop — `` style would + // execute in the user's browser if we 302'd to it. + setAllowlist(["foss.arbisoft.com"]); + for (const target of [ + "javascript:alert(1)", + "data:text/html,", + "file:///etc/passwd", + ]) { + const res = await server.get( + `/auth/portal-logout?next=${encodeURIComponent(target)}`, + { redirect: "manual" } + ); + expect(res.status).toEqual(200); + } + }); + + it("should accept both http and https for allowlisted hosts", async () => { + // http allowed for local-dev / localhost flows; https for production. + // Beyond the two are rejected by the scheme gate. + setAllowlist(["foss.arbisoft.com"]); + for (const target of [ + "https://docs.foss.arbisoft.com/x", + "http://docs.foss.arbisoft.com/x", + ]) { + const res = await server.get( + `/auth/portal-logout?next=${encodeURIComponent(target)}`, + { redirect: "manual" } + ); + expect(res.status).toEqual(302); + expect(res.headers.get("location")).toEqual(target); + } + }); + it("should not 302 when next is missing", async () => { setAllowlist(["foss.arbisoft.com"]); const res = await server.get("/auth/portal-logout", { redirect: "manual" }); expect(res.status).toEqual(200); - // cookies still cleared + // cookies still cleared at path=/ const setCookie = res.headers.get("set-cookie") ?? ""; - expect(setCookie).toMatch(/accessToken=;/); + expect(setCookie).toMatch(cookieClearedAtRoot("accessToken")); }); }); diff --git a/server/routes/auth/index.ts b/server/routes/auth/index.ts index ef9cd8bf3942..f27ab5f19eec 100644 --- a/server/routes/auth/index.ts +++ b/server/routes/auth/index.ts @@ -95,26 +95,33 @@ router.get("/redirect", authMiddleware(), async (ctx: APIContext) => { }); /** - * Returns true iff `url`'s hostname matches an entry in - * `MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS`. Suffix match enforces a dot boundary: - * `foss.arbisoft.com` matches `foss.arbisoft.com` and `*.foss.arbisoft.com` - * but NOT `foss.arbisoft.com.evil.example`. Closes the obvious open-redirect + * Returns true iff `url` is a safe redirect target: + * - scheme is http or https (rejects javascript:, data:, etc.) + * - hostname matches an entry in `MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS` + * + * Suffix match enforces a dot boundary: `foss.arbisoft.com` matches + * `foss.arbisoft.com` and `*.foss.arbisoft.com` but NOT + * `foss.arbisoft.com.evil.example`. Closes the obvious open-redirect * surface the endpoint would otherwise expose. */ function isAllowedSignOutNext(url: string): boolean { - let host: string; + let parsed: URL; try { - host = new URL(url).hostname.toLowerCase(); + parsed = new URL(url); } catch { return false; } + // 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; } for (const entry of env.MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS) { - if (!entry) { - continue; - } if (host === entry || host.endsWith("." + entry)) { return true; } @@ -144,14 +151,22 @@ function isAllowedSignOutNext(url: string): boolean { */ router.get("/portal-logout", async (ctx: APIContext) => { const epoch = new Date(0); - // Match the cookie-clear shape used by the auth() catch block at - // server/middlewares/authentication.ts: same names, same path, same - // sameSite. lastSignedIn is non-HttpOnly because the frontend reads it. - ctx.cookies.set("accessToken", "", { sameSite: "lax", expires: epoch }); + // path:/ is load-bearing — without it Koa defaults the Set-Cookie path + // to the request URL (`/auth/portal-logout`), which doesn't shadow the + // original `accessToken` cookie's path:/ scope. The browser keeps the + // JWT. Matches the cookie-clear shape used by the auth() catch block + // at server/middlewares/authentication.ts:114-117. + ctx.cookies.set("accessToken", "", { + sameSite: "lax", + expires: epoch, + path: "/", + }); + // lastSignedIn is non-HttpOnly because the frontend reads it. ctx.cookies.set("lastSignedIn", "", { httpOnly: false, sameSite: "lax", expires: epoch, + path: "/", }); const nextRaw = String(ctx.query.next ?? "").trim(); From 92b912547855a8638123fd60797ced5f2125bce2 Mon Sep 17 00:00:00 2001 From: awais786 Date: Sun, 17 May 2026 00:39:33 +0500 Subject: [PATCH 3/3] refactor(auth): derive portal-logout allowlist from PLATFORM_DOMAIN MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- server/env.ts | 23 ++++++++++---------- server/routes/auth/index.test.ts | 36 ++++++++++++++++---------------- server/routes/auth/index.ts | 22 ++++++++++--------- 3 files changed, 41 insertions(+), 40 deletions(-) diff --git a/server/env.ts b/server/env.ts index fa26131f6f98..0506a214bd8d 100644 --- a/server/env.ts +++ b/server/env.ts @@ -537,21 +537,20 @@ export class Environment { public DEFAULT_EMAIL_DOMAIN = environment.DEFAULT_EMAIL_DOMAIN ?? "askii.ai"; /** - * Comma-separated host suffixes the `/auth/portal-logout?next=…` endpoint - * is allowed to redirect to. Each entry matches its exact host plus all - * subdomains (e.g. `foss.arbisoft.com` matches `pm.foss.arbisoft.com`). - * Empty list rejects every `?next=` — the endpoint still clears the - * accessToken cookie, it just won't follow a redirect. + * Root domain of the foss-server-bundle deployment (set by + * `foss-server-bundle/platform.sh`). All app subdomains hang off this + * (e.g. `docs.${PLATFORM_DOMAIN}`, `pm.${PLATFORM_DOMAIN}`). * - * Used by the foss-server-bundle portal's "Log out of all apps" - * redirect chain. + * Used by `/auth/portal-logout?next=…` as the redirect allowlist: the + * endpoint will follow a `next` URL whose host matches `PLATFORM_DOMAIN` + * or any of its subdomains, and reject everything else. Unset → + * endpoint still clears cookies, just won't follow any redirect. */ @IsOptional() - public MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS = ( - this.toOptionalCommaList(environment.MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS) ?? [] - ) - .map((h) => h.toLowerCase().replace(/^\.+/, "")) - .filter((h) => h.length > 0); + public PLATFORM_DOMAIN = (environment.PLATFORM_DOMAIN ?? "") + .toLowerCase() + .replace(/^\.+/, "") + .trim(); /** * A boolean switch to toggle the rate limiter at application web server. diff --git a/server/routes/auth/index.test.ts b/server/routes/auth/index.test.ts index 4224edca620c..1e87471d97c7 100644 --- a/server/routes/auth/index.test.ts +++ b/server/routes/auth/index.test.ts @@ -84,18 +84,18 @@ describe("auth/redirect", () => { }); describe("auth/portal-logout", () => { - let originalAllowlist: string[]; + let originalPlatformDomain: string; beforeEach(() => { - originalAllowlist = env.MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS; + originalPlatformDomain = env.PLATFORM_DOMAIN; }); afterEach(() => { - env.MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS = originalAllowlist; + env.PLATFORM_DOMAIN = originalPlatformDomain; }); - const setAllowlist = (entries: string[]) => { - env.MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS = entries; + const setPlatformDomain = (domain: string) => { + env.PLATFORM_DOMAIN = domain; }; /** @@ -109,7 +109,7 @@ describe("auth/portal-logout", () => { new RegExp(`${name}=;[^,]*\\bpath=/(?:[;,]|$)`, "i"); it("should clear accessToken and lastSignedIn cookies at path=/ on every call", async () => { - setAllowlist([]); + setPlatformDomain(""); const res = await server.get("/auth/portal-logout", { redirect: "manual" }); expect(res.status).toEqual(200); const setCookie = res.headers.get("set-cookie") ?? ""; @@ -120,7 +120,7 @@ describe("auth/portal-logout", () => { }); it("should 302 to next when host is in the allowlist", async () => { - setAllowlist(["foss.arbisoft.com"]); + setPlatformDomain("foss.arbisoft.com"); const target = "https://pm.foss.arbisoft.com/auth/portal-sign-out/"; const res = await server.get( `/auth/portal-logout?next=${encodeURIComponent(target)}`, @@ -134,7 +134,7 @@ describe("auth/portal-logout", () => { }); it("should match subdomains of allowlist entries", async () => { - setAllowlist(["foss.arbisoft.com"]); + setPlatformDomain("foss.arbisoft.com"); const target = "https://docs.foss.arbisoft.com/done"; const res = await server.get( `/auth/portal-logout?next=${encodeURIComponent(target)}`, @@ -145,21 +145,21 @@ describe("auth/portal-logout", () => { }); it("should reject next on a host outside the allowlist", async () => { - setAllowlist(["foss.arbisoft.com"]); + setPlatformDomain("foss.arbisoft.com"); const target = "https://evil.example/steal"; const res = await server.get( `/auth/portal-logout?next=${encodeURIComponent(target)}`, { redirect: "manual" } ); expect(res.status).toEqual(200); - // still clears cookies — non-redirect rejection isn't an error path + // still clears cookies at path=/ — non-redirect rejection isn't an error path const setCookie = res.headers.get("set-cookie") ?? ""; - expect(setCookie).toMatch(/accessToken=;/); + expect(setCookie).toMatch(cookieClearedAtRoot("accessToken")); }); it("should enforce dot-boundary on suffix matches", async () => { // "foss.arbisoft.com.evil" must NOT match "foss.arbisoft.com". - setAllowlist(["foss.arbisoft.com"]); + setPlatformDomain("foss.arbisoft.com"); const target = "https://foss.arbisoft.com.evil.example/x"; const res = await server.get( `/auth/portal-logout?next=${encodeURIComponent(target)}`, @@ -168,8 +168,8 @@ describe("auth/portal-logout", () => { expect(res.status).toEqual(200); }); - it("should reject every next when the allowlist is empty", async () => { - setAllowlist([]); + it("should reject every next when PLATFORM_DOMAIN is unset", async () => { + setPlatformDomain(""); const res = await server.get( "/auth/portal-logout?next=https%3A%2F%2Ffoss.arbisoft.com%2F", { redirect: "manual" } @@ -178,7 +178,7 @@ describe("auth/portal-logout", () => { }); it("should reject malformed next values", async () => { - setAllowlist(["foss.arbisoft.com"]); + setPlatformDomain("foss.arbisoft.com"); const res = await server.get( "/auth/portal-logout?next=not-a-url", { redirect: "manual" } @@ -190,7 +190,7 @@ describe("auth/portal-logout", () => { // javascript:, data:, file:, etc. parse fine via new URL() but must // never be a valid next-hop — `` style would // execute in the user's browser if we 302'd to it. - setAllowlist(["foss.arbisoft.com"]); + setPlatformDomain("foss.arbisoft.com"); for (const target of [ "javascript:alert(1)", "data:text/html,", @@ -207,7 +207,7 @@ describe("auth/portal-logout", () => { it("should accept both http and https for allowlisted hosts", async () => { // http allowed for local-dev / localhost flows; https for production. // Beyond the two are rejected by the scheme gate. - setAllowlist(["foss.arbisoft.com"]); + setPlatformDomain("foss.arbisoft.com"); for (const target of [ "https://docs.foss.arbisoft.com/x", "http://docs.foss.arbisoft.com/x", @@ -222,7 +222,7 @@ describe("auth/portal-logout", () => { }); it("should not 302 when next is missing", async () => { - setAllowlist(["foss.arbisoft.com"]); + setPlatformDomain("foss.arbisoft.com"); const res = await server.get("/auth/portal-logout", { redirect: "manual" }); expect(res.status).toEqual(200); // cookies still cleared at path=/ diff --git a/server/routes/auth/index.ts b/server/routes/auth/index.ts index f27ab5f19eec..bcbc2c034d17 100644 --- a/server/routes/auth/index.ts +++ b/server/routes/auth/index.ts @@ -97,12 +97,15 @@ router.get("/redirect", authMiddleware(), async (ctx: APIContext) => { /** * Returns true iff `url` is a safe redirect target: * - scheme is http or https (rejects javascript:, data:, etc.) - * - hostname matches an entry in `MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS` + * - hostname is `PLATFORM_DOMAIN` itself or a subdomain of it * * Suffix match enforces a dot boundary: `foss.arbisoft.com` matches * `foss.arbisoft.com` and `*.foss.arbisoft.com` but NOT * `foss.arbisoft.com.evil.example`. Closes the obvious open-redirect * surface the endpoint would otherwise expose. + * + * When `PLATFORM_DOMAIN` is unset, every `next` is rejected — the + * endpoint still clears the cookie, it just won't follow a redirect. */ function isAllowedSignOutNext(url: string): boolean { let parsed: URL; @@ -121,12 +124,11 @@ function isAllowedSignOutNext(url: string): boolean { if (!host) { return false; } - for (const entry of env.MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS) { - if (host === entry || host.endsWith("." + entry)) { - return true; - } + const platformDomain = env.PLATFORM_DOMAIN; + if (!platformDomain) { + return false; } - return false; + return host === platformDomain || host.endsWith("." + platformDomain); } /** @@ -144,10 +146,10 @@ function isAllowedSignOutNext(url: string): boolean { * victim's session ends). Low impact — the only state lost is the * cookie, and ForwardAuth re-auths on the next request. * - * Open-redirect protection: `next` is validated against - * `MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS` (suffix-match on a dot boundary). - * Empty allowlist rejects every `next` — cookies are still cleared, the - * endpoint just returns 200 instead of 302. + * Open-redirect protection: `next` is validated against `PLATFORM_DOMAIN` + * (suffix-match on a dot boundary). Unset `PLATFORM_DOMAIN` rejects every + * `next` — cookies are still cleared, the endpoint just returns 200 + * instead of 302. */ router.get("/portal-logout", async (ctx: APIContext) => { const epoch = new Date(0);