From 627d3b6199d1f840a9c3dd5351b7c298b2cb6193 Mon Sep 17 00:00:00 2001 From: Alan Buscaglia Date: Wed, 29 Jul 2026 11:35:14 +0200 Subject: [PATCH] fix(ui): return 401 for unauthenticated report routes - Preserve redirects for server-rendered authentication flows - Return explicit unauthorized responses from report route handlers - Cover invalid sessions and callback fallback behavior --- .../api/scans/[scanId]/report/route.test.ts | 52 +++++++++++++++---- ui/app/api/scans/[scanId]/report/route.ts | 11 +++- ui/lib/auth-headers.test.ts | 34 ++++++++++-- ui/lib/auth-headers.ts | 17 +++++- ui/lib/server-auth.test.ts | 41 ++++++++++++++- ui/lib/server-auth.ts | 21 +++++++- 6 files changed, 155 insertions(+), 21 deletions(-) diff --git a/ui/app/api/scans/[scanId]/report/route.test.ts b/ui/app/api/scans/[scanId]/report/route.test.ts index 81201f7315..ede7f10991 100644 --- a/ui/app/api/scans/[scanId]/report/route.test.ts +++ b/ui/app/api/scans/[scanId]/report/route.test.ts @@ -2,8 +2,8 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { GET } from "./route"; -const { getAuthHeadersMock } = vi.hoisted(() => ({ - getAuthHeadersMock: vi.fn(), +const { getRouteAuthHeadersMock } = vi.hoisted(() => ({ + getRouteAuthHeadersMock: vi.fn(), })); vi.mock("@/lib", () => ({ @@ -11,7 +11,7 @@ vi.mock("@/lib", () => ({ })); vi.mock("@/lib/auth-headers", () => ({ - getAuthHeaders: getAuthHeadersMock, + getRouteAuthHeaders: getRouteAuthHeadersMock, })); describe("GET /api/scans/[scanId]/report", () => { @@ -20,6 +20,24 @@ describe("GET /api/scans/[scanId]/report", () => { vi.clearAllMocks(); }); + it("returns 401 without fetching upstream when authentication is invalid", async () => { + // Given + const fetchMock = vi.fn(); + vi.stubGlobal("fetch", fetchMock); + getRouteAuthHeadersMock.mockResolvedValue(null); + + // When + const response = await GET(new Request("http://localhost/api"), { + params: Promise.resolve({ scanId: "scan-123" }), + }); + + // Then + expect(response.status).toBe(401); + await expect(response.json()).resolves.toEqual({ error: "Unauthorized." }); + expect(response.headers.get("location")).toBeNull(); + expect(fetchMock).not.toHaveBeenCalled(); + }); + it("streams the upstream report body without buffering it", async () => { const upstreamBody = new ReadableStream({ start(controller) { @@ -37,7 +55,9 @@ describe("GET /api/scans/[scanId]/report", () => { }), ); vi.stubGlobal("fetch", fetchMock); - getAuthHeadersMock.mockResolvedValue({ Authorization: "Bearer token" }); + getRouteAuthHeadersMock.mockResolvedValue({ + Authorization: "Bearer token", + }); const response = await GET(new Request("http://localhost/api"), { params: Promise.resolve({ scanId: "scan-123" }), @@ -72,7 +92,9 @@ describe("GET /api/scans/[scanId]/report", () => { "fetch", vi.fn().mockResolvedValue(new Response(upstreamBody, { status: 200 })), ); - getAuthHeadersMock.mockResolvedValue({ Authorization: "Bearer token" }); + getRouteAuthHeadersMock.mockResolvedValue({ + Authorization: "Bearer token", + }); const response = await GET( new Request("http://localhost/api?preflight=1"), @@ -95,7 +117,9 @@ describe("GET /api/scans/[scanId]/report", () => { }), ); vi.stubGlobal("fetch", fetchMock); - getAuthHeadersMock.mockResolvedValue({ Authorization: "Bearer token" }); + getRouteAuthHeadersMock.mockResolvedValue({ + Authorization: "Bearer token", + }); const response = await GET(new Request("http://localhost/api"), { params: Promise.resolve({ scanId: "scan-123" }), @@ -122,7 +146,9 @@ describe("GET /api/scans/[scanId]/report", () => { }), ), ); - getAuthHeadersMock.mockResolvedValue({ Authorization: "Bearer token" }); + getRouteAuthHeadersMock.mockResolvedValue({ + Authorization: "Bearer token", + }); const response = await GET( new Request("http://localhost/api?preflight=1"), @@ -145,7 +171,9 @@ describe("GET /api/scans/[scanId]/report", () => { Response.json({ data: { id: "task-1" } }, { status: 202 }), ), ); - getAuthHeadersMock.mockResolvedValue({ Authorization: "Bearer token" }); + getRouteAuthHeadersMock.mockResolvedValue({ + Authorization: "Bearer token", + }); const response = await GET(new Request("http://localhost/api"), { params: Promise.resolve({ scanId: "scan-123" }), @@ -160,7 +188,9 @@ describe("GET /api/scans/[scanId]/report", () => { "fetch", vi.fn().mockRejectedValue(new DOMException("Timed out", "TimeoutError")), ); - getAuthHeadersMock.mockResolvedValue({ Authorization: "Bearer token" }); + getRouteAuthHeadersMock.mockResolvedValue({ + Authorization: "Bearer token", + }); const response = await GET( new Request("http://localhost/api?preflight=1"), @@ -187,7 +217,9 @@ describe("GET /api/scans/[scanId]/report", () => { ), ), ); - getAuthHeadersMock.mockResolvedValue({ Authorization: "Bearer token" }); + getRouteAuthHeadersMock.mockResolvedValue({ + Authorization: "Bearer token", + }); const response = await GET( new Request("http://localhost/api?preflight=1"), diff --git a/ui/app/api/scans/[scanId]/report/route.ts b/ui/app/api/scans/[scanId]/report/route.ts index 3707f14e04..2ac7013728 100644 --- a/ui/app/api/scans/[scanId]/report/route.ts +++ b/ui/app/api/scans/[scanId]/report/route.ts @@ -1,7 +1,7 @@ import { NextResponse } from "next/server"; import { apiBaseUrl } from "@/lib"; -import { getAuthHeaders } from "@/lib/auth-headers"; +import { getRouteAuthHeaders } from "@/lib/auth-headers"; export const dynamic = "force-dynamic"; export const runtime = "nodejs"; @@ -64,7 +64,14 @@ export async function GET( { params }: ScanReportRouteContext, ) { const { scanId } = await params; - const headers = await getAuthHeaders({ contentType: false }); + const headers = await getRouteAuthHeaders({ contentType: false }); + if (!headers) { + return NextResponse.json( + { error: "Unauthorized." }, + { status: 401, headers: { "Cache-Control": "no-store" } }, + ); + } + const upstreamUrl = `${apiBaseUrl}/scans/${encodeURIComponent(scanId)}/report`; const isPreflight = new URL(request.url).searchParams.get("preflight") === "1"; diff --git a/ui/lib/auth-headers.test.ts b/ui/lib/auth-headers.test.ts index 6488eeb2ff..688431758f 100644 --- a/ui/lib/auth-headers.test.ts +++ b/ui/lib/auth-headers.test.ts @@ -1,9 +1,11 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; -const { authMock, getRequiredAuthHeadersMock } = vi.hoisted(() => ({ - authMock: vi.fn(), - getRequiredAuthHeadersMock: vi.fn(), -})); +const { authMock, getAuthHeadersIfAvailableMock, getRequiredAuthHeadersMock } = + vi.hoisted(() => ({ + authMock: vi.fn(), + getAuthHeadersIfAvailableMock: vi.fn(), + getRequiredAuthHeadersMock: vi.fn(), + })); vi.mock("server-only", () => ({})); @@ -12,10 +14,11 @@ vi.mock("@/auth.config", () => ({ })); vi.mock("./server-auth", () => ({ + getAuthHeadersIfAvailable: getAuthHeadersIfAvailableMock, getRequiredAuthHeaders: getRequiredAuthHeadersMock, })); -import { getAuthHeaders } from "./auth-headers"; +import { getAuthHeaders, getRouteAuthHeaders } from "./auth-headers"; describe("getAuthHeaders", () => { beforeEach(() => { @@ -40,4 +43,25 @@ describe("getAuthHeaders", () => { "RefreshAccessTokenError", ); }); + + it("returns no route headers for a terminal session", async () => { + // Given + authMock.mockResolvedValue({ + accessToken: "stale-access-token", + error: "RefreshAccessTokenError", + }); + getAuthHeadersIfAvailableMock.mockReturnValue(null); + + // When + const result = await getRouteAuthHeaders(); + + // Then + expect(result).toBeNull(); + expect(getAuthHeadersIfAvailableMock).toHaveBeenCalledWith( + "stale-access-token", + undefined, + "RefreshAccessTokenError", + ); + expect(getRequiredAuthHeadersMock).not.toHaveBeenCalled(); + }); }); diff --git a/ui/lib/auth-headers.ts b/ui/lib/auth-headers.ts index e497df26bf..f95e9a63f2 100644 --- a/ui/lib/auth-headers.ts +++ b/ui/lib/auth-headers.ts @@ -2,10 +2,25 @@ import "server-only"; import { auth } from "@/auth.config"; -import { getRequiredAuthHeaders } from "./server-auth"; +import { + getAuthHeadersIfAvailable, + getRequiredAuthHeaders, +} from "./server-auth"; export const getAuthHeaders = async (options?: { contentType?: boolean }) => { const session = await auth(); return getRequiredAuthHeaders(session?.accessToken, options, session?.error); }; + +export const getRouteAuthHeaders = async (options?: { + contentType?: boolean; +}) => { + const session = await auth(); + + return getAuthHeadersIfAvailable( + session?.accessToken, + options, + session?.error, + ); +}; diff --git a/ui/lib/server-auth.test.ts b/ui/lib/server-auth.test.ts index 8d1815787d..824efffe91 100644 --- a/ui/lib/server-auth.test.ts +++ b/ui/lib/server-auth.test.ts @@ -17,7 +17,11 @@ vi.mock("next/navigation", () => ({ redirect: redirectMock, })); -import { getRequiredAuthHeaders, redirectToSignIn } from "./server-auth"; +import { + getAuthHeadersIfAvailable, + getRequiredAuthHeaders, + redirectToSignIn, +} from "./server-auth"; describe("server authentication", () => { beforeEach(() => { @@ -51,6 +55,31 @@ describe("server authentication", () => { expect(redirectMock).toHaveBeenCalledOnce(); }); + it("returns no headers for a terminal session without redirecting", () => { + // Given + const accessToken = "stale-access-token"; + + // When + const result = getAuthHeadersIfAvailable( + accessToken, + undefined, + "RefreshAccessTokenError", + ); + + // Then + expect(result).toBeNull(); + expect(redirectMock).not.toHaveBeenCalled(); + }); + + it("returns no headers for a missing session without redirecting", () => { + // When + const result = getAuthHeadersIfAvailable(undefined); + + // Then + expect(result).toBeNull(); + expect(redirectMock).not.toHaveBeenCalled(); + }); + it("creates bearer headers only when an access token is present", async () => { // Given const accessToken = "access-token"; @@ -75,4 +104,14 @@ describe("server authentication", () => { "NEXT_REDIRECT:/sign-in?callbackUrl=%2Fproviders%3Fpage%3D2", ); }); + + it("falls back to the root callback when the current path is unavailable", async () => { + // Given + headersMock.mockResolvedValue(new Headers()); + + // When / Then + await expect(redirectToSignIn()).rejects.toThrow( + "NEXT_REDIRECT:/sign-in?callbackUrl=%2F", + ); + }); }); diff --git a/ui/lib/server-auth.ts b/ui/lib/server-auth.ts index da7a0e64a7..53091a04fa 100644 --- a/ui/lib/server-auth.ts +++ b/ui/lib/server-auth.ts @@ -17,13 +17,13 @@ export const redirectToSignIn = async (): Promise => { type SessionError = "RefreshAccessTokenError" | (string & {}); -export const getRequiredAuthHeaders = async ( +export const getAuthHeadersIfAvailable = ( accessToken: string | undefined, options?: { contentType?: boolean }, sessionError?: SessionError, ) => { if (!accessToken || sessionError === "RefreshAccessTokenError") { - return redirectToSignIn(); + return null; } const authHeaders: Record = { @@ -37,3 +37,20 @@ export const getRequiredAuthHeaders = async ( return authHeaders; }; + +export const getRequiredAuthHeaders = async ( + accessToken: string | undefined, + options?: { contentType?: boolean }, + sessionError?: SessionError, +) => { + const authHeaders = getAuthHeadersIfAvailable( + accessToken, + options, + sessionError, + ); + if (!authHeaders) { + return redirectToSignIn(); + } + + return authHeaders; +};