diff --git a/src/lib/supabase/auth.ts b/src/lib/supabase/auth.ts index 7e29a31cd..8e9a2e673 100644 --- a/src/lib/supabase/auth.ts +++ b/src/lib/supabase/auth.ts @@ -11,6 +11,9 @@ export type AuthenticatedUser = { appMetadata: Record; }; +export type OptionalAuthenticationResult = + { status: "absent" } | { status: "valid"; user: AuthenticatedUser } | { status: "invalid" }; + type AuthenticationRequirement = { administrator?: boolean; }; @@ -65,6 +68,11 @@ function extractCookieSessionAccessToken(request: Request): string | null { return null; } +function hasSessionCookie(request: Request): boolean { + const cookies = readCookies(request.headers.get("cookie")); + return [...cookies.keys()].some((name) => name === "sb-access-token" || /^sb-.+-auth-token(?:\.\d+)?$/.test(name)); +} + function extractSessionAccessToken(request: Request): string | null { return extractBearerAccessToken(request) ?? extractCookieSessionAccessToken(request); } @@ -124,23 +132,28 @@ async function getUserFromRequestCookies(request: Request): Promise { - const bearerToken = extractBearerAccessToken(request); - if (bearerToken) { +): Promise { + if (request.headers.has("authorization")) { + const bearerToken = extractBearerAccessToken(request); + if (!bearerToken) return { status: "invalid" }; + const bearerUser = await getUserFromAccessToken(supabase, bearerToken); - if (bearerUser) return bearerUser; + return bearerUser ? { status: "valid", user: bearerUser } : { status: "invalid" }; } const cookieToken = extractCookieSessionAccessToken(request); - if (cookieToken && cookieToken !== bearerToken) { + if (cookieToken) { const cookieTokenUser = await getUserFromAccessToken(supabase, cookieToken); - if (cookieTokenUser) return cookieTokenUser; + return cookieTokenUser ? { status: "valid", user: cookieTokenUser } : { status: "invalid" }; } - return getUserFromRequestCookies(request); + if (!hasSessionCookie(request)) return { status: "absent" }; + + const cookieUser = await getUserFromRequestCookies(request); + return cookieUser ? { status: "valid", user: cookieUser } : { status: "invalid" }; } export async function requireAuthenticatedUser( @@ -148,8 +161,13 @@ export async function requireAuthenticatedUser( supabase: AdminClient, requirement: AuthenticationRequirement = {}, ): Promise { - const user = await resolveOptionalAuthenticatedUser(request, supabase); - if (!user) throw new AuthenticationError(); + const authentication = await resolveOptionalAuthentication(request, supabase); + if (authentication.status !== "valid") { + throw new AuthenticationError( + authentication.status === "invalid" ? "Invalid authentication credentials." : undefined, + ); + } + const { user } = authentication; if (requirement.administrator && !isAdministratorAppMetadata(user.appMetadata)) { throw new PublicApiError("Administrator access required.", 403, { code: "administrator_required" }); } @@ -160,7 +178,11 @@ export async function getOptionalAuthenticatedUser( request: Request, supabase: AdminClient, ): Promise { - return resolveOptionalAuthenticatedUser(request, supabase); + const authentication = await resolveOptionalAuthentication(request, supabase); + if (authentication.status === "invalid") { + throw new AuthenticationError("Invalid authentication credentials."); + } + return authentication.status === "valid" ? authentication.user : null; } // Retained for callers that only need a single token string. diff --git a/tests/auth-tri-state.test.ts b/tests/auth-tri-state.test.ts new file mode 100644 index 000000000..7d08d47c9 --- /dev/null +++ b/tests/auth-tri-state.test.ts @@ -0,0 +1,140 @@ +import { describe, expect, it, vi } from "vitest"; + +import { publicAccessContext } from "@/lib/public-api-access"; +import { AuthenticationError, getOptionalAuthenticatedUser, resolveOptionalAuthentication } from "@/lib/supabase/auth"; + +function authClient(result: unknown) { + return { + auth: { + getUser: vi.fn(async () => result), + }, + }; +} + +describe("optional authentication", () => { + it("distinguishes absent credentials without calling Supabase auth", async () => { + const client = authClient({ data: { user: null }, error: null }); + const request = new Request("http://localhost/api/search"); + + await expect(resolveOptionalAuthentication(request, client as never)).resolves.toEqual({ status: "absent" }); + expect(client.auth.getUser).not.toHaveBeenCalled(); + await expect(getOptionalAuthenticatedUser(request, client as never)).resolves.toBeNull(); + }); + + it("returns a valid bearer user with immutable app metadata", async () => { + const client = authClient({ + data: { user: { id: "user-1", app_metadata: { site_role: "administrator" } } }, + error: null, + }); + const request = new Request("http://localhost/api/search", { + headers: { authorization: "Bearer valid-token" }, + }); + + await expect(resolveOptionalAuthentication(request, client as never)).resolves.toEqual({ + status: "valid", + user: { id: "user-1", appMetadata: { site_role: "administrator" } }, + }); + expect(client.auth.getUser).toHaveBeenCalledWith("valid-token"); + }); + + it("rejects a presented bearer credential that Supabase reports as invalid", async () => { + const client = authClient({ + data: { user: null }, + error: { message: "Invalid token" }, + }); + const request = new Request("http://localhost/api/search", { + headers: { authorization: "Bearer expired-token" }, + }); + + await expect(getOptionalAuthenticatedUser(request, client as never)).rejects.toBeInstanceOf(AuthenticationError); + }); + + it("rejects a malformed authorization header without trying cookie fallback", async () => { + const client = authClient({ + data: { user: { id: "user-1", app_metadata: {} } }, + error: null, + }); + const request = new Request("http://localhost/api/search", { + headers: { + authorization: "Basic credentials", + cookie: "sb-access-token=valid-cookie-token", + }, + }); + + await expect(resolveOptionalAuthentication(request, client as never)).resolves.toEqual({ status: "invalid" }); + expect(client.auth.getUser).not.toHaveBeenCalled(); + }); + + it("does not let a valid cookie override an invalid bearer credential", async () => { + const client = { + auth: { + getUser: vi.fn(async (token: string) => + token === "valid-cookie-token" + ? { data: { user: { id: "user-1", app_metadata: {} } }, error: null } + : { data: { user: null }, error: { message: "Invalid token" } }, + ), + }, + }; + const request = new Request("http://localhost/api/search", { + headers: { + authorization: "Bearer expired-token", + cookie: "sb-access-token=valid-cookie-token", + }, + }); + + await expect(resolveOptionalAuthentication(request, client as never)).resolves.toEqual({ status: "invalid" }); + expect(client.auth.getUser).toHaveBeenCalledTimes(1); + expect(client.auth.getUser).toHaveBeenCalledWith("expired-token"); + }); + + it("validates a legacy session cookie when no authorization header is present", async () => { + const client = authClient({ data: { user: { id: "user-1", app_metadata: {} } }, error: null }); + const request = new Request("http://localhost/api/search", { + headers: { cookie: "sb-access-token=valid-cookie-token" }, + }); + + await expect(resolveOptionalAuthentication(request, client as never)).resolves.toMatchObject({ + status: "valid", + user: { id: "user-1" }, + }); + expect(client.auth.getUser).toHaveBeenCalledWith("valid-cookie-token"); + }); + + it("rejects a legacy session cookie that Supabase reports as invalid", async () => { + const client = authClient({ data: { user: null }, error: { message: "Invalid token" } }); + const request = new Request("http://localhost/api/search", { + headers: { cookie: "sb-access-token=expired-cookie-token" }, + }); + + await expect(getOptionalAuthenticatedUser(request, client as never)).rejects.toBeInstanceOf(AuthenticationError); + expect(client.auth.getUser).toHaveBeenCalledWith("expired-cookie-token"); + }); + + it("propagates dependency failures instead of treating them as anonymous", async () => { + const client = { + auth: { getUser: vi.fn(async () => Promise.reject(new Error("Supabase unavailable"))) }, + }; + const request = new Request("http://localhost/api/search", { + headers: { authorization: "Bearer valid-token" }, + }); + + await expect(getOptionalAuthenticatedUser(request, client as never)).rejects.toThrow("Supabase unavailable"); + }); + + it("never constructs anonymous access for invalid credentials", async () => { + const client = authClient({ data: { user: null }, error: { message: "Invalid token" } }); + const invalidRequest = new Request("http://localhost/api/search", { + headers: { authorization: "Bearer expired-token", "x-real-ip": "198.51.100.10" }, + }); + const anonymousRequest = new Request("http://localhost/api/search", { + headers: { "x-real-ip": "198.51.100.10" }, + }); + + await expect(publicAccessContext(invalidRequest, client as never)).rejects.toBeInstanceOf(AuthenticationError); + await expect(publicAccessContext(anonymousRequest, client as never)).resolves.toMatchObject({ + authenticated: false, + ownerId: undefined, + rateLimitSubject: { kind: "anonymous" }, + }); + }); +}); diff --git a/tests/private-access-routes.test.ts b/tests/private-access-routes.test.ts index 4b072610d..57d3b4c6d 100644 --- a/tests/private-access-routes.test.ts +++ b/tests/private-access-routes.test.ts @@ -841,7 +841,7 @@ describe("private document API access", () => { expect((await payload(response)).url).toContain("public/documents/guideline.pdf"); }); - it("recovers valid cookie auth when a stale bearer header is also present", async () => { + it("rejects an invalid bearer header even when a valid cookie is also present", async () => { const documents = [{ id: documentId, owner_id: userId, title: "Owned document" }]; const client = createSupabaseMock((call) => (call.table === "documents" ? ok(documents) : ok([]))); mockRuntime(client); @@ -857,9 +857,10 @@ describe("private document API access", () => { ); const body = await payload(response); - expect(response.status).toBe(200); - expect(body.documents).toEqual(documents.map((document) => ({ ...document, labels: [], summary: null }))); - expect(client.auth.getUser).toHaveBeenCalledWith(token); + expect(response.status).toBe(401); + expect(body).toMatchObject({ code: "authentication_required" }); + expect(client.auth.getUser).toHaveBeenCalledTimes(1); + expect(client.auth.getUser).toHaveBeenCalledWith("expired-token"); }); it("omits internal document list fields for anonymous callers", async () => { @@ -3541,7 +3542,7 @@ describe("private document API access", () => { ); }); - it("degrades invalid bearer tokens to anonymous search scope", async () => { + it("rejects invalid bearer tokens instead of using anonymous search scope", async () => { const searchChunksWithTelemetry = vi.fn(async () => ({ results: [], telemetry: { @@ -3570,10 +3571,9 @@ describe("private document API access", () => { }), ); - expect(response.status).toBe(200); - expect(searchChunksWithTelemetry).toHaveBeenCalledWith( - expect.objectContaining({ ownerId: undefined, allowGlobalSearch: true }), - ); + expect(response.status).toBe(401); + expect(await payload(response)).toMatchObject({ code: "authentication_required" }); + expect(searchChunksWithTelemetry).not.toHaveBeenCalled(); }); it("rate limits anonymous answer bursts before generation", async () => {