diff --git a/apps/ai/src/mcp/lib/resolve-tenant.oauth.test.ts b/apps/ai/src/mcp/lib/resolve-tenant.oauth.test.ts index 783f3dcc54..11c9f3e7db 100644 --- a/apps/ai/src/mcp/lib/resolve-tenant.oauth.test.ts +++ b/apps/ai/src/mcp/lib/resolve-tenant.oauth.test.ts @@ -110,6 +110,16 @@ describe("an MCP OAuth token, resolved by the AI worker", () => { }).pipe(Effect.provide(makeLayer(db))) }) + it.effect("treats a request with cookies and no bearer as carrying no credential", () => { + const db = createTestDb(createdDbs) + return Effect.gen(function* () { + const failure = yield* resolveMcpTenantContext( + new Request(resource, { headers: { cookie: "__session=anything" } }), + ).pipe(Effect.flip) + expect(failure._tag).toBe("@maple/mcp/errors/McpAuthMissingError") + }).pipe(Effect.provide(makeLayer(db))) + }) + it.effect("is refused for a resource it was not bound to", () => { const db = createTestDb(createdDbs) return Effect.gen(function* () { diff --git a/apps/ai/src/mcp/lib/resolve-tenant.ts b/apps/ai/src/mcp/lib/resolve-tenant.ts index 87f4bb36db..35f52cbaf5 100644 --- a/apps/ai/src/mcp/lib/resolve-tenant.ts +++ b/apps/ai/src/mcp/lib/resolve-tenant.ts @@ -122,8 +122,8 @@ export const describeMalformedBearer = (token: string | undefined): string | und */ export const mcpResourceUrl = (apiBaseUrl: string) => `${Str.replace(/\/+$/, "")(apiBaseUrl)}/mcp` -/** A request with neither an `Authorization` header nor a cookie sent nothing a session reads. */ -const carriesCredential = (headers: Headers): boolean => headers.has("authorization") || headers.has("cookie") +/** A request without an `Authorization` header sent nothing a session reads. */ +const carriesCredential = (headers: Headers): boolean => headers.has("authorization") /** * The session resolver's failures as the MCP ones. A provider outage is retryable (503), and a diff --git a/apps/web/src/api-proxy-forward.ts b/apps/web/src/api-proxy-forward.ts index c980cd28cc..e75990635e 100644 --- a/apps/web/src/api-proxy-forward.ts +++ b/apps/web/src/api-proxy-forward.ts @@ -73,6 +73,8 @@ const forwardOverBinding = ( // Copying the request copies the browser's `Host: app.…`, and a service binding passes it // through as-is. The API builds OAuth redirect URIs from Host, so it must name the API. forwarded.headers.set("host", target.host) + // The app authenticates with a bearer. The page's cookies are not the API's to read. + forwarded.headers.delete("cookie") // @effect-diagnostics-next-line preferEffectRecord:off -- not a record type; Rec.toEntries would need a cast Arr.forEach(Object.entries(HttpTraceContext.toHeaders(client)), ([name, value]) => forwarded.headers.set(name, value), diff --git a/apps/web/src/handler.test.ts b/apps/web/src/handler.test.ts index 8b73fb6c47..cb493ed8a4 100644 --- a/apps/web/src/handler.test.ts +++ b/apps/web/src/handler.test.ts @@ -56,6 +56,18 @@ describe("handleRequest API proxy", () => { expect(await request?.text()).toBe("hello") }) + it("keeps the page's cookies out of the forwarded request", async () => { + const { env, forwarded } = recordingEnv() + await handleRequest( + new Request("https://app.maple.test/_api/v2/traces", { + headers: { authorization: "Bearer t", cookie: "__session=s; theme=dark" }, + }), + env, + ) + expect(forwarded[0]?.headers.get("cookie")).toBeNull() + expect(forwarded[0]?.headers.get("authorization")).toBe("Bearer t") + }) + it("sends the API's host, not the app's, so OAuth callbacks stay on the API", async () => { const { env, forwarded } = recordingEnv() await handleRequest( diff --git a/packages/auth/src/auth.test.ts b/packages/auth/src/auth.test.ts index 5104e2b8f4..68de681450 100644 --- a/packages/auth/src/auth.test.ts +++ b/packages/auth/src/auth.test.ts @@ -123,6 +123,44 @@ describe("makeResolveTenant", () => { }), ) + it.effect("signs a request in by its bearer, never by a session cookie", () => + Effect.gen(function* () { + // Stands in for the provider, which accepts either credential it is shown. + const resolveTenant = makeResolveTenant( + { + ...baseEnv, + MAPLE_AUTH_MODE: "clerk", + CLERK_SECRET_KEY: Option.some(Redacted.make("sk_test_123")), + CLERK_JWT_KEY: Option.some(Redacted.make("jwt_test_123")), + }, + async (request) => { + const shown = request.headers.has("authorization") || request.headers.has("cookie") + return { + isAuthenticated: shown, + message: shown ? null : "No credential", + toAuth: () => ({ + isAuthenticated: true, + tokenType: "session_token", + userId: "user_123", + orgId: "org_123", + orgRole: "org:admin", + }), + } + }, + ) + + assertRejected( + yield* Effect.exit(resolveTenant({ cookie: "__session=test-token" })), + "No credential", + ) + const tenant = yield* resolveTenant({ + Authorization: "Bearer test-token", + cookie: "__session=other-token", + }) + assert.strictEqual(tenant.userId, asUserId("user_123")) + }), + ) + it.effect("rejects invalid or expired Clerk tokens", () => Effect.gen(function* () { const resolveTenant = makeResolveTenant( diff --git a/packages/auth/src/index.ts b/packages/auth/src/index.ts index d684dbc7ec..d4e0904e22 100644 --- a/packages/auth/src/index.ts +++ b/packages/auth/src/index.ts @@ -173,11 +173,15 @@ const getBearerToken = (headers: HeaderRecord): string | undefined => { return token } +/** + * The bearer and nothing else. A cookie the browser attaches on its own is never a credential + * here, so only a caller that can read the session token can act with it. + */ const toHeaders = (headers: HeaderRecord): Headers => { const requestHeaders = new Headers() for (const [name, value] of Rec.toEntries(headers)) { - if (value !== undefined) { + if (value !== undefined && Str.toLowerCase(name) === "authorization") { requestHeaders.set(name, value) } }