From 2ad15a56c8e5ff4f38f4133a2f5aef2684c7405f Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Wed, 16 Sep 2026 09:10:19 -0700 Subject: [PATCH] Restart hosted invitation logins with fresh state --- .changeset/restart-hosted-invitation-login.md | 5 ++ apps/cloud/src/auth/handlers.ts | 19 ++++--- .../auth/workos-callback-state.node.test.ts | 41 ++++++++----- e2e/cloud/login-csrf.test.ts | 57 ++++++++++++++++++- 4 files changed, 97 insertions(+), 25 deletions(-) create mode 100644 .changeset/restart-hosted-invitation-login.md diff --git a/.changeset/restart-hosted-invitation-login.md b/.changeset/restart-hosted-invitation-login.md new file mode 100644 index 0000000000..c76b90fa2b --- /dev/null +++ b/.changeset/restart-hosted-invitation-login.md @@ -0,0 +1,5 @@ +--- +"@executor-js/cloud": patch +--- + +Restart hosted invitation logins that return without state, while keeping authorization codes bound to the browser that started the login. diff --git a/apps/cloud/src/auth/handlers.ts b/apps/cloud/src/auth/handlers.ts index 1eefa96df7..45dab43006 100644 --- a/apps/cloud/src/auth/handlers.ts +++ b/apps/cloud/src/auth/handlers.ts @@ -188,14 +188,19 @@ export const CloudAuthPublicHandlers = HttpApiBuilder.group( Effect.gen(function* () { const workos = yield* WorkOSClient; const users = yield* UserStoreService; + // Hosted invitations can start at WorkOS without app-issued state. + // Discard that unbound code and start a fresh browser-bound login. + // Exchanging it here would allow login CSRF. + if (query.state === undefined) { + return deleteResponseCookie( + HttpServerResponse.redirect(AUTH_PATHS.login, { status: 302 }), + STATE_COOKIE, + ); + } + const cookieState = request.cookies[STATE_COOKIE] ?? null; - // CSRF is unconditional: every callback must carry a state that - // matches the cookie set on /login. There is no legitimate - // no-state entry path — omitting state previously allowed an - // attacker to complete their own OAuth round-trip and redirect a - // victim's browser through this callback, signing the victim into - // the attacker's account (login CSRF). - if (!cookieState || !timingSafeEqual(cookieState, query.state ?? "")) { + // Only exchange codes bound to the state cookie set on /login. + if (!cookieState || !timingSafeEqual(cookieState, query.state)) { return deleteResponseCookie( HttpServerResponse.text("Invalid login state", { status: 400 }), STATE_COOKIE, diff --git a/apps/cloud/src/auth/workos-callback-state.node.test.ts b/apps/cloud/src/auth/workos-callback-state.node.test.ts index 4c1799428b..d4f8970735 100644 --- a/apps/cloud/src/auth/workos-callback-state.node.test.ts +++ b/apps/cloud/src/auth/workos-callback-state.node.test.ts @@ -1,9 +1,8 @@ // --------------------------------------------------------------------------- // Focused tests — the WorkOS login callback's CSRF gate. // -// The callback's CSRF check must be unconditional: no state ⇒ 400 before any -// WorkOS call; a replayed (already consumed) state ⇒ 400; a fresh state -// matching the cookie ⇒ 302 + session. +// Codes without state restart login without a WorkOS exchange; a replayed +// state still fails with 400; a fresh state matching the cookie issues a session. // // Test seams follow repo conventions: @effect/vitest, Layer.succeed stubs // (see org-selector-auth.node.test.ts), and HttpRouter.toWebHandler for the @@ -38,12 +37,14 @@ const stubWorkOS = Layer.succeed( new Proxy({} as WorkOSClientService, { get: (_t, prop) => { if (prop === "authenticateWithCode") { - return () => - Effect.succeed({ - user: { id: STUB_USER_ID, email: "u@test" }, - organizationId: STUB_ORG_ID, - sealedSession: STUB_SESSION, - }); + return (code: string) => + code === "unbound-code" + ? Effect.die("An unbound authorization code must never be exchanged") + : Effect.succeed({ + user: { id: STUB_USER_ID, email: "u@test" }, + organizationId: STUB_ORG_ID, + sealedSession: STUB_SESSION, + }); } if (prop === "listUserMemberships") { return () => Effect.succeed({ data: [] }); @@ -128,20 +129,30 @@ describe("workos callback · CSRF state hardening", () => { }); } - it("rejects a callback with NO state (the former bypass) before any WorkOS call", async () => { - const res = await run(new Request(callbackUrl(undefined), { redirect: "manual" })); - expect(res.status).toBe(400); - expect(await res.text()).toContain("Invalid login state"); + it("restarts login without exchanging a code that has no state", async () => { + const res = await run( + new Request(callbackUrl(undefined, "unbound-code"), { redirect: "manual" }), + ); + expect(res.status).toBe(302); + expect(res.headers.get("location")).toBe("/api/auth/login"); expect(res.headers.get("set-cookie") ?? "").not.toContain(SESSION_COOKIE); }); - it("rejects missing state even when the browser has a login cookie", async () => { + it("discards an existing login cookie when restarting a callback without state", async () => { const res = await run( - new Request(callbackUrl(undefined), { + new Request(callbackUrl(undefined, "unbound-code"), { headers: { cookie: `${STATE_COOKIE}=victim-login-state` }, redirect: "manual", }), ); + expect(res.status).toBe(302); + expect(res.headers.get("location")).toBe("/api/auth/login"); + expect(res.headers.get("set-cookie")).toContain(`${STATE_COOKIE}=; Max-Age=0`); + expect(res.headers.get("set-cookie") ?? "").not.toContain(SESSION_COOKIE); + }); + + it("rejects empty state instead of treating it as a provider-initiated login", async () => { + const res = await run(new Request(`${callbackUrl(undefined, "unbound-code")}&state=`)); expect(res.status).toBe(400); expect(await res.text()).toBe("Invalid login state"); expect(res.headers.get("set-cookie") ?? "").not.toContain(SESSION_COOKIE); diff --git a/e2e/cloud/login-csrf.test.ts b/e2e/cloud/login-csrf.test.ts index fbc237e8b3..183eb72a16 100644 --- a/e2e/cloud/login-csrf.test.ts +++ b/e2e/cloud/login-csrf.test.ts @@ -36,12 +36,12 @@ scenario( if (!callback) throw new Error("AuthKit did not return a callback"); return callback; }; - await step("Refuse a valid authorization code with no state", async () => { + await step("Discard a valid code without state and restart login", async () => { const callback = new URL(await interceptCallback()); callback.searchParams.delete("state"); const response = await page.request.get(callback.toString(), { maxRedirects: 0 }); - expect(response.status()).toBe(400); - expect(await response.text()).toBe("Invalid login state"); + expect(response.status()).toBe(302); + expect(response.headers().location).toBe("/api/auth/login"); expect( (await page.context().cookies()).some((cookie) => cookie.name === "wos-session"), ).toBe(false); @@ -73,3 +73,54 @@ scenario( }); }), ); + +scenario( + "Auth · a provider-initiated login restarts with browser-bound state", + { timeout: 180_000 }, + Effect.gen(function* () { + const target = yield* Target; + const browser = yield* Browser; + const email = `provider-login-${randomUUID()}@e2e.test`; + // Discover this deployment's provider URL without setting a browser cookie. + const login = yield* Effect.promise(() => + fetch(new URL("/api/auth/login", target.baseUrl), { redirect: "manual" }), + ); + expect(login.status).toBe(302); + const location = login.headers.get("location"); + if (!location) throw new Error("Login did not redirect to AuthKit"); + const providerUrl = new URL(location); + providerUrl.searchParams.delete("state"); + + yield* browser.session({ label: "anonymous" }, async ({ page, step }) => { + await step("Sign in directly at the provider, as a hosted invitation does", async () => { + await page.goto(providerUrl.toString()); + await page.getByPlaceholder("new-user@example.com").fill(email); + const callbackResponse = page.waitForResponse( + (response) => new URL(response.url()).pathname === "/api/auth/callback", + ); + await page.getByRole("button", { name: /Continue/ }).click(); + const callback = await callbackResponse; + expect(new URL(callback.url()).searchParams.has("state")).toBe(false); + expect(callback.status()).toBe(302); + expect(callback.headers().location).toBe("/api/auth/login"); + await page.waitForURL((url) => url.searchParams.has("state")); + expect((await page.context().cookies()).map((cookie) => cookie.name)).not.toContain( + "wos-session", + ); + }); + + await step("Complete the fresh login and reach the signed-in app", async () => { + // The emulator asks again; hosted AuthKit can reuse its browser session. + await page.getByPlaceholder("new-user@example.com").fill(email); + await page.getByRole("button", { name: /Continue/ }).click(); + await page.waitForURL((url) => url.pathname === "/create-org", { timeout: 30_000 }); + const me = await page.request.get(new URL("/api/auth/me", target.baseUrl).toString()); + expect(me.status()).toBe(200); + expect(await me.json()).toMatchObject({ user: { email } }); + const cookieNames = (await page.context().cookies()).map((cookie) => cookie.name); + expect(cookieNames).toContain("wos-session"); + expect(cookieNames).not.toContain("wos-login-state"); + }); + }); + }), +);