diff --git a/packages/control-plane/src/routes/browser-auth.test.ts b/packages/control-plane/src/routes/browser-auth.test.ts index 363433e1b..2dcb62110 100644 --- a/packages/control-plane/src/routes/browser-auth.test.ts +++ b/packages/control-plane/src/routes/browser-auth.test.ts @@ -1,5 +1,53 @@ -import { describe, expect, it, vi } from "vitest"; -import { forwardBrowserAuthRequest } from "./browser-auth"; +import { BROWSER_AUTH_PROXY_ROUTES } from "@open-inspect/shared/browser-auth-routes"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type * as AuthenticateModule from "../auth/authenticate"; +import { UserAuthConfigurationError } from "../auth/user/runtime"; +import type * as UserRuntimeModule from "../auth/user/runtime"; +import { + createTestRequestHandler, + ownerAuthorizationDatabase, + TEST_BACKGROUND_TASK_CONTEXT, + TEST_SERVICE_SECRETS, +} from "../router.test-support"; +import type { Env } from "../types"; +import { browserAuthRoutes, forwardBrowserAuthRequest } from "./browser-auth"; + +const mocks = vi.hoisted(() => ({ + authenticate: vi.fn(), + getUserAuth: vi.fn(), +})); + +vi.mock("../auth/authenticate", async (importOriginal) => ({ + ...(await importOriginal()), + authenticate: mocks.authenticate, +})); + +vi.mock("../auth/user/runtime", async (importOriginal) => ({ + ...(await importOriginal()), + getUserAuth: mocks.getUserAuth, +})); + +const handleRequest = createTestRequestHandler([browserAuthRoutes]); +const env = { + ...TEST_SERVICE_SECRETS, + SCM_PROVIDER: "github", + DB: ownerAuthorizationDatabase(), +} as unknown as Env; + +function webServicePrincipal() { + mocks.authenticate.mockImplementation(async (request: Request) => ({ + principal: { kind: "service", service: "web", actor: null }, + request, + })); +} + +function callRoute(method: string, path: string): Promise { + return handleRequest( + new Request(`https://test.local${path}`, { method }), + env, + TEST_BACKGROUND_TASK_CONTEXT + ); +} describe("forwardBrowserAuthRequest", () => { it("uses the direct API wrapper for session lookup", async () => { @@ -25,3 +73,94 @@ describe("forwardBrowserAuthRequest", () => { expect(handler).not.toHaveBeenCalled(); }); }); + +describe("browser auth routes", () => { + beforeEach(() => { + vi.clearAllMocks(); + webServicePrincipal(); + }); + + it.each(BROWSER_AUTH_PROXY_ROUTES.map(([method, path]) => [method, path]))( + "proxies %s %s to Better Auth", + async (method, path) => { + const handler = vi.fn(async (request: Request) => + Response.json({ path: new URL(request.url).pathname }, { status: 202 }) + ); + const getSession = vi.fn(async () => + Response.json({ path: "/api/auth/get-session" }, { status: 202 }) + ); + mocks.getUserAuth.mockReturnValue({ api: { getSession }, handler }); + + const response = await callRoute(method, path); + + expect(response.status).toBe(202); + await expect(response.json()).resolves.toEqual({ path }); + // Session reads take Better Auth's direct API; everything else its HTTP handler. + const direct = method === "GET" && path === "/api/auth/get-session"; + expect(getSession).toHaveBeenCalledTimes(direct ? 1 : 0); + expect(handler).toHaveBeenCalledTimes(direct ? 0 : 1); + } + ); + + it("passes Better Auth's status through with no-store and no-referrer headers", async () => { + const handler = vi.fn( + async () => + new Response("redirecting", { + status: 302, + headers: { + Location: "https://web.test/", + "Set-Cookie": "session=abc; Path=/; HttpOnly", + }, + }) + ); + mocks.getUserAuth.mockReturnValue({ api: { getSession: vi.fn() }, handler }); + + const response = await callRoute("GET", "/api/auth/callback/github"); + + expect(response.status).toBe(302); + expect(response.headers.get("Location")).toBe("https://web.test/"); + expect(response.headers.get("Set-Cookie")).toBe("session=abc; Path=/; HttpOnly"); + expect(response.headers.get("Cache-Control")).toBe("no-store"); + expect(response.headers.get("Referrer-Policy")).toBe("no-referrer"); + expect(response.headers.get("Access-Control-Allow-Origin")).toBe("*"); + expect(response.headers.get("x-request-id")).toBeTruthy(); + }); + + it("answers 503 when browser authentication is not configured", async () => { + mocks.getUserAuth.mockImplementation(() => { + throw new UserAuthConfigurationError("missing secret"); + }); + const log = vi.spyOn(console, "error").mockImplementation(() => {}); + + const response = await callRoute("GET", "/api/auth/get-session"); + + expect(response.status).toBe(503); + await expect(response.json()).resolves.toEqual({ + error: "Browser authentication is not configured", + }); + expect(log).toHaveBeenCalled(); + log.mockRestore(); + }); + + it("refuses a caller that is not the signed web service", async () => { + mocks.authenticate.mockImplementation(async (request: Request) => ({ + principal: { kind: "user", userId: "user-1" }, + request, + })); + mocks.getUserAuth.mockReturnValue({ api: { getSession: vi.fn() }, handler: vi.fn() }); + + const response = await callRoute("GET", "/api/auth/get-session"); + + expect(response.status).toBe(401); + expect(mocks.getUserAuth).not.toHaveBeenCalled(); + }); + + it("does not expose paths outside the allowlist", async () => { + mocks.getUserAuth.mockReturnValue({ api: { getSession: vi.fn() }, handler: vi.fn() }); + + const response = await callRoute("GET", "/api/auth/sign-in/social"); + + expect(response.status).toBe(404); + expect(mocks.getUserAuth).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/control-plane/src/routes/browser-auth.ts b/packages/control-plane/src/routes/browser-auth.ts index 8e275a584..7b0d5f3b5 100644 --- a/packages/control-plane/src/routes/browser-auth.ts +++ b/packages/control-plane/src/routes/browser-auth.ts @@ -1,12 +1,15 @@ import { BROWSER_AUTH_PROXY_ROUTES } from "@open-inspect/shared/browser-auth-routes"; +import { Hono } from "hono"; import { type BetterAuthRuntime, UserAuthConfigurationError } from "../auth/user/runtime"; import { createLogger } from "../logger"; +import { admit, dispatch } from "../routing/admit"; +import type { ControlPlaneHonoEnv } from "../routing/hono-env"; +import type { Env } from "../types"; import { - defineRoutes, error, NO_AUTHORIZATION, + type RequestContext, SCM_AGNOSTIC_WEB_SERVICE_ROUTE, - type Route, } from "./shared"; const logger = createLogger("browser-auth"); @@ -49,7 +52,12 @@ export async function forwardBrowserAuthRequest( return auth.handler(request); } -const handleBrowserAuth: Route["handler"] = async (request, _env, _match, ctx) => { +async function handleBrowserAuth( + request: Request, + _env: Env, + _params: Record, + ctx: RequestContext +): Promise { try { if (!ctx.getUserAuth) { throw new UserAuthConfigurationError("User authentication runtime is unavailable"); @@ -76,18 +84,16 @@ const handleBrowserAuth: Route["handler"] = async (request, _env, _match, ctx) = } throw cause; } -}; +} /** * The browser can reach only this positive Better Auth allowlist, and only * through a freshly signed service:web proxy request. */ -export const browserAuthRoutes: Route[] = defineRoutes( - SCM_AGNOSTIC_WEB_SERVICE_ROUTE, - BROWSER_AUTH_PROXY_ROUTES.map(([method, path]) => ({ - method, - path: path, - authorization: NO_AUTHORIZATION, - handler: handleBrowserAuth, - })) -); +export const browserAuthRoutes = new Hono(); + +const BROWSER_AUTH = admit({ ...SCM_AGNOSTIC_WEB_SERVICE_ROUTE, authorization: NO_AUTHORIZATION }); + +for (const [method, path] of BROWSER_AUTH_PROXY_ROUTES) { + browserAuthRoutes.on(method, path, BROWSER_AUTH, (c) => dispatch(c, handleBrowserAuth)); +} diff --git a/packages/control-plane/src/routes/catalog.ts b/packages/control-plane/src/routes/catalog.ts index 72bc1a728..4c27e26a3 100644 --- a/packages/control-plane/src/routes/catalog.ts +++ b/packages/control-plane/src/routes/catalog.ts @@ -38,7 +38,7 @@ import { skillRoutes } from "./skills"; export const catalog: RouteCatalogEntry[] = [ healthRoutes, - ...browserAuthRoutes, + browserAuthRoutes, signInProviderRoutes, // Session management, then the agent-initiated Slack notification