From 10f00cba543e55e6b339d58e705183e3301661d3 Mon Sep 17 00:00:00 2001 From: Dylan Klein Date: Mon, 3 Aug 2026 22:06:10 +0000 Subject: [PATCH 1/2] fix: resolve infinite redirect loops in auth flows - Fixes a redirect loop in service_account mode when anonymous session creation fails by rendering a static HTML error page instead of redirecting to the landing page. - Fixes a dashboard lock-in bug in user_oauth mode when the Google OAuth token expires but the BetterAuth session remains valid. requireSession now verifies Google token validity and clears cookies early if invalid, allowing navigation back to / for re-authentication. - Added integration and unit tests for the updated auth routing behaviors. TAG=agy CONV=b3471264-2592-4924-b031-267b4bf62326 --- .../integration/api/auth-health-route.test.ts | 38 +++++++- .../api/auto-session-route.test.ts | 90 +++++++++++++++++++ .../src/__tests__/unit/auth-errors.test.ts | 4 +- .../src/__tests__/unit/session.test.ts | 27 +++++- .../src/app/api/auth/auto-session/route.ts | 15 +++- .../src/app/api/auth/health/route.ts | 3 +- .../pocket-cep/src/lib/activity-data.ts | 6 +- .../pocket-cep/src/lib/env-error-page.ts | 20 +++++ mcp-examples/pocket-cep/src/lib/session.ts | 70 +++++++++------ 9 files changed, 237 insertions(+), 36 deletions(-) create mode 100644 mcp-examples/pocket-cep/src/__tests__/integration/api/auto-session-route.test.ts diff --git a/mcp-examples/pocket-cep/src/__tests__/integration/api/auth-health-route.test.ts b/mcp-examples/pocket-cep/src/__tests__/integration/api/auth-health-route.test.ts index 7662705..c3353c4 100644 --- a/mcp-examples/pocket-cep/src/__tests__/integration/api/auth-health-route.test.ts +++ b/mcp-examples/pocket-cep/src/__tests__/integration/api/auth-health-route.test.ts @@ -7,9 +7,14 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; -const { mockGetSession, mockGetGoogleAccessToken } = vi.hoisted(() => ({ +const { mockGetSession, mockGetGoogleAccessToken, mockGetEnv } = vi.hoisted(() => ({ mockGetSession: vi.fn(), mockGetGoogleAccessToken: vi.fn(), + mockGetEnv: vi.fn(), +})); + +vi.mock("@/lib/env", () => ({ + getEnv: mockGetEnv, })); vi.mock("@/lib/auth", () => ({ @@ -35,6 +40,10 @@ describe("GET /api/auth/health", () => { beforeEach(() => { vi.clearAllMocks(); mockGetSession.mockResolvedValue({ user: { id: "u1" } }); + mockGetEnv.mockReturnValue({ + AUTH_MODE: "service_account", + BETTER_AUTH_SECRET: "mock-secret", + }); }); it("returns 200 { ok: true } when token acquisition succeeds", async () => { @@ -51,7 +60,6 @@ describe("GET /api/auth/health", () => { const res = await GET(); expect(res.status).toBe(401); const body = await res.json(); - expect(body.ok).toBe(false); expect(body.error.code).toBe("no_credentials"); expect(body.error.source).toBe("admin-sdk"); }); @@ -62,4 +70,30 @@ describe("GET /api/auth/health", () => { const res = await GET(); expect(res.status).toBe(401); }); + + describe("user_oauth mode", () => { + beforeEach(() => { + mockGetEnv.mockReturnValue({ + AUTH_MODE: "user_oauth", + BETTER_AUTH_SECRET: "mock-secret", + GOOGLE_CLIENT_ID: "123-abc.apps.googleusercontent.com", + GOOGLE_CLIENT_SECRET: "secret", + }); + }); + + it("returns 200 when both session and Google token are valid", async () => { + mockGetGoogleAccessToken.mockResolvedValue("mock-oauth-token"); + const res = await GET(); + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ ok: true }); + }); + + it("returns 401 unauthenticated when Google token is expired/missing", async () => { + mockGetGoogleAccessToken.mockResolvedValue(undefined); + const res = await GET(); + expect(res.status).toBe(401); + const body = await res.json(); + expect(body.error.code).toBe("unauthenticated"); + }); + }); }); diff --git a/mcp-examples/pocket-cep/src/__tests__/integration/api/auto-session-route.test.ts b/mcp-examples/pocket-cep/src/__tests__/integration/api/auto-session-route.test.ts new file mode 100644 index 0000000..8a907ea --- /dev/null +++ b/mcp-examples/pocket-cep/src/__tests__/integration/api/auto-session-route.test.ts @@ -0,0 +1,90 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; + +const { mockGetEnv } = vi.hoisted(() => ({ + mockGetEnv: vi.fn(), +})); + +vi.mock("@/lib/env", () => ({ + getEnv: mockGetEnv, +})); + +import { GET } from "@/app/api/auth/auto-session/route"; + +describe("GET /api/auth/auto-session", () => { + const originalFetch = global.fetch; + + beforeEach(() => { + vi.clearAllMocks(); + mockGetEnv.mockReturnValue({ + AUTH_MODE: "service_account", + BETTER_AUTH_SECRET: "mock-secret", + BETTER_AUTH_URL: "http://localhost:3000", + }); + }); + + afterEach(() => { + global.fetch = originalFetch; + }); + + it("returns 404 when AUTH_MODE is not service_account", async () => { + mockGetEnv.mockReturnValue({ + AUTH_MODE: "user_oauth", + }); + + const req = new Request("http://localhost:3000/api/auth/auto-session"); + const res = await GET(req); + expect(res.status).toBe(404); + expect(await res.text()).toBe("Not found"); + }); + + it("redirects to /dashboard and sets cookies on successful anonymous sign-in", async () => { + const mockFetchResponse = { + ok: true, + headers: { + getSetCookie: () => ["session_token=valid-token; Path=/"], + }, + }; + global.fetch = vi.fn().mockResolvedValue(mockFetchResponse); + + const req = new Request("http://localhost:3000/api/auth/auto-session"); + const res = await GET(req); + + expect(res.status).toBe(302); + expect(res.headers.get("location")).toBe("http://localhost:3000/dashboard"); + expect(res.headers.getSetCookie()).toEqual(["session_token=valid-token; Path=/"]); + }); + + it("returns 503 HTML page when fetch to anonymous sign-in fails", async () => { + global.fetch = vi.fn().mockRejectedValue(new Error("Network connection lost")); + + const req = new Request("http://localhost:3000/api/auth/auto-session"); + const res = await GET(req); + + expect(res.status).toBe(503); + expect(res.headers.get("content-type")).toMatch(/text\/html/); + const body = await res.text(); + expect(body).toContain("Service Account Session Failed"); + expect(body).toContain("Fetch failed: Network connection lost"); + }); + + it("returns error HTML page when anonymous sign-in returns non-200", async () => { + const mockFetchResponse = { + ok: false, + status: 500, + text: async () => "Internal Database Error", + headers: { + getSetCookie: () => [], + }, + }; + global.fetch = vi.fn().mockResolvedValue(mockFetchResponse); + + const req = new Request("http://localhost:3000/api/auth/auto-session"); + const res = await GET(req); + + expect(res.status).toBe(500); + expect(res.headers.get("content-type")).toMatch(/text\/html/); + const body = await res.text(); + expect(body).toContain("Service Account Session Failed"); + expect(body).toContain("API returned 500: Internal Database Error"); + }); +}); diff --git a/mcp-examples/pocket-cep/src/__tests__/unit/auth-errors.test.ts b/mcp-examples/pocket-cep/src/__tests__/unit/auth-errors.test.ts index d4e1486..8e88e3b 100644 --- a/mcp-examples/pocket-cep/src/__tests__/unit/auth-errors.test.ts +++ b/mcp-examples/pocket-cep/src/__tests__/unit/auth-errors.test.ts @@ -166,7 +166,9 @@ describe("toAuthError", () => { const err = new Error("Invalid Customer Id"); const result = toAuthError(err, "admin-sdk"); expect(result?.code).toBe("invalid_customer_id"); - expect(result?.remedy).toContain("Ensure the Customer ID parameter passed to the tool is correct"); + expect(result?.remedy).toContain( + "Ensure the Customer ID parameter passed to the tool is correct", + ); expect(result?.remedy).not.toContain("/sa-setup"); }); }); diff --git a/mcp-examples/pocket-cep/src/__tests__/unit/session.test.ts b/mcp-examples/pocket-cep/src/__tests__/unit/session.test.ts index e7788e8..77ddfcd 100644 --- a/mcp-examples/pocket-cep/src/__tests__/unit/session.test.ts +++ b/mcp-examples/pocket-cep/src/__tests__/unit/session.test.ts @@ -1,11 +1,12 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; -const { mockGetSession, mockCookies, mockGetEnv } = vi.hoisted(() => ({ +const { mockGetSession, mockCookies, mockGetGoogleAccessToken, mockGetEnv } = vi.hoisted(() => ({ mockGetSession: vi.fn(), mockCookies: { getAll: vi.fn(() => [] as { name: string; value: string }[]), delete: vi.fn(), }, + mockGetGoogleAccessToken: vi.fn(), mockGetEnv: vi.fn(() => ({ AUTH_MODE: "service_account" as "service_account" | "user_oauth", BETTER_AUTH_SECRET: "mock-secret", @@ -29,6 +30,10 @@ vi.mock("@/lib/env", () => ({ getEnv: mockGetEnv, })); +vi.mock("@/lib/access-token", () => ({ + getGoogleAccessToken: mockGetGoogleAccessToken, +})); + import { requireSession } from "@/lib/session"; describe("requireSession", () => { @@ -71,17 +76,20 @@ describe("requireSession", () => { expect(mockCookies.delete).not.toHaveBeenCalled(); }); - describe("in user_oauth mode", () => { + describe("user_oauth mode", () => { beforeEach(() => { mockGetEnv.mockReturnValue({ AUTH_MODE: "user_oauth", BETTER_AUTH_SECRET: "mock-secret", + GOOGLE_CLIENT_ID: "123-abc.apps.googleusercontent.com", + GOOGLE_CLIENT_SECRET: "secret", }); }); - it("returns session when getSession succeeds with a normal user email", async () => { + it("returns session when session is valid and Google token is valid", async () => { const mockSession = { user: { id: "u1", email: "admin@company.com" } }; mockGetSession.mockResolvedValue(mockSession); + mockGetGoogleAccessToken.mockResolvedValue("valid-google-token"); const session = await requireSession(); expect(session).toBe(mockSession); @@ -99,5 +107,18 @@ describe("requireSession", () => { expect(session).toBeNull(); expect(mockCookies.delete).toHaveBeenCalledWith("better-auth.session_token"); }); + + it("clears cookies and returns null when session is valid but Google token is expired/missing", async () => { + const mockSession = { user: { id: "u1", email: "admin@company.com" } }; + mockGetSession.mockResolvedValue(mockSession); + mockGetGoogleAccessToken.mockResolvedValue(undefined); // expired + mockCookies.getAll.mockReturnValue([ + { name: "better-auth.session_token", value: "stale-val" }, + ]); + + const session = await requireSession(); + expect(session).toBeNull(); + expect(mockCookies.delete).toHaveBeenCalledWith("better-auth.session_token"); + }); }); }); diff --git a/mcp-examples/pocket-cep/src/app/api/auth/auto-session/route.ts b/mcp-examples/pocket-cep/src/app/api/auth/auto-session/route.ts index 1373894..8d9104b 100644 --- a/mcp-examples/pocket-cep/src/app/api/auth/auto-session/route.ts +++ b/mcp-examples/pocket-cep/src/app/api/auth/auto-session/route.ts @@ -18,6 +18,7 @@ import { NextResponse } from "next/server"; import { getEnv } from "@/lib/env"; +import { renderSaSessionErrorHtml } from "@/lib/env-error-page"; /** * Creates an anonymous session and redirects to the dashboard. @@ -48,18 +49,26 @@ export async function GET(request: Request) { body: "{}", signal: AbortSignal.timeout(5000), }); - } catch { + } catch (err) { if (acceptJson) { return NextResponse.json({ error: "session_unavailable" }, { status: 503 }); } - return NextResponse.redirect(new URL("/?error=session_unavailable", base)); + const errMsg = err instanceof Error ? err.message : String(err); + return new NextResponse(renderSaSessionErrorHtml(`Fetch failed: ${errMsg}`), { + status: 503, + headers: { "Content-Type": "text/html; charset=utf-8" }, + }); } if (!response.ok) { if (acceptJson) { return NextResponse.json({ error: "session_failed" }, { status: 401 }); } - return NextResponse.redirect(new URL("/?error=session_failed", base)); + const body = await response.text().catch(() => "Unknown error"); + return new NextResponse(renderSaSessionErrorHtml(`API returned ${response.status}: ${body}`), { + status: response.status, + headers: { "Content-Type": "text/html; charset=utf-8" }, + }); } const cookies = response.headers.getSetCookie(); diff --git a/mcp-examples/pocket-cep/src/app/api/auth/health/route.ts b/mcp-examples/pocket-cep/src/app/api/auth/health/route.ts index 5a297ac..c757ee3 100644 --- a/mcp-examples/pocket-cep/src/app/api/auth/health/route.ts +++ b/mcp-examples/pocket-cep/src/app/api/auth/health/route.ts @@ -15,6 +15,7 @@ import { getGoogleAccessToken } from "@/lib/access-token"; import { AuthError, isAuthError } from "@/lib/auth-errors"; import { requireSession } from "@/lib/session"; import { getErrorMessage } from "@/lib/errors"; +import { unauthenticatedResponse } from "@/lib/api-response"; /** * Probes Google credentials by requesting a fresh access token. Returns 200 @@ -22,7 +23,7 @@ import { getErrorMessage } from "@/lib/errors"; */ export async function GET() { if (!(await requireSession())) { - return NextResponse.json({ ok: false, error: "Not authenticated" }, { status: 401 }); + return unauthenticatedResponse(); } try { diff --git a/mcp-examples/pocket-cep/src/lib/activity-data.ts b/mcp-examples/pocket-cep/src/lib/activity-data.ts index 564be1d..83ee38a 100644 --- a/mcp-examples/pocket-cep/src/lib/activity-data.ts +++ b/mcp-examples/pocket-cep/src/lib/activity-data.ts @@ -100,7 +100,11 @@ export async function getActivitySafe(days: number = DEFAULT_ACTIVITY_DAYS): Pro * Pulls and groups Chrome audit events for the given caller, scoped to * `days` of history. Pagination stops at {@link ACTIVITY_MAX_EVENTS}. */ -async function fetchActivity(tokenToUse: string, days: number, impersonatedUser?: string): Promise { +async function fetchActivity( + tokenToUse: string, + days: number, + impersonatedUser?: string, +): Promise { const requestHeaders = await buildGoogleApiHeaders(tokenToUse); const baseUrl = new URL( diff --git a/mcp-examples/pocket-cep/src/lib/env-error-page.ts b/mcp-examples/pocket-cep/src/lib/env-error-page.ts index 5281f1b..c6ab78c 100644 --- a/mcp-examples/pocket-cep/src/lib/env-error-page.ts +++ b/mcp-examples/pocket-cep/src/lib/env-error-page.ts @@ -299,3 +299,23 @@ export function renderMcpUnreachableHtml(url: string): string { footerHint: "Once the MCP server is running, refresh this page.", }); } + +/** + * Builds the error page when the Service Account anonymous session fails to establish. + * Prevents redirect loops in service_account mode when the BetterAuth backend is failing. + */ +export function renderSaSessionErrorHtml(error: string): string { + return renderSetupBlockedHtml({ + pageTitle: "Pocket CEP — Service Account Session Failed", + heading: "Service Account Session Failed", + lede: "Pocket CEP could not establish an anonymous session for Service Account mode.", + failuresHeading: "Error Details", + failures: [{ code: "session_failed", message: error }], + primaryAction: { + command: "npm run doctor", + description: + "Run diagnostic checks. This will verify your Service Account credentials and connection to Google APIs.", + }, + footerHint: "Check your console logs and try refreshing this page.", + }); +} diff --git a/mcp-examples/pocket-cep/src/lib/session.ts b/mcp-examples/pocket-cep/src/lib/session.ts index 44c8b1f..fb8407b 100644 --- a/mcp-examples/pocket-cep/src/lib/session.ts +++ b/mcp-examples/pocket-cep/src/lib/session.ts @@ -15,6 +15,7 @@ import { headers, cookies } from "next/headers"; import { getAuth } from "./auth"; import { getEnv } from "./env"; import { SA_EMAIL_DOMAIN } from "./constants"; +import { getGoogleAccessToken } from "./access-token"; /** * Resolves the current BetterAuth session or null. Reads cookies from @@ -27,40 +28,59 @@ import { SA_EMAIL_DOMAIN } from "./constants"; * * It also invalidates and clears anonymous sessions if the app has been * switched to `user_oauth` mode. + * + * In user_oauth mode, it also verifies that the Google access token is + * still valid. If the Google token is expired (even if BetterAuth session + * is technically still valid), it clears the session cookies to force + * a fresh sign-in, preventing the user from being locked in the dashboard. */ export async function requireSession() { const auth = getAuth(); const session = await auth.api.getSession({ headers: await headers() }); - + const config = getEnv(); + + if (!session) { + await clearSessionCookies(); + return null; + } + let isStaleAnonymous = false; - try { - const config = getEnv(); - isStaleAnonymous = Boolean( - session && - config.AUTH_MODE === "user_oauth" && - session.user.email?.endsWith(`@${SA_EMAIL_DOMAIN}`), - ); - } catch { - // If env cannot be loaded, fallback to safe false + if (config.AUTH_MODE === "user_oauth" && session.user.email?.endsWith(`@${SA_EMAIL_DOMAIN}`)) { + isStaleAnonymous = true; } - if (!session || isStaleAnonymous) { - const cookieStore = await cookies(); - // Better Auth session cookie names contain "session_token" - const sessionCookies = cookieStore.getAll().filter((c) => c.name.includes("session_token")); - if (sessionCookies.length > 0) { - console.warn( - `requireSession: ${ - isStaleAnonymous ? "Stale anonymous session in OAuth mode" : "Invalid session" - }. Clearing stale cookies:`, - sessionCookies.map((c) => c.name), - ); - for (const cookie of sessionCookies) { - cookieStore.delete(cookie.name); - } - } + if (isStaleAnonymous) { + console.warn("requireSession: Stale anonymous session in OAuth mode. Clearing session."); + await clearSessionCookies(); return null; } + if (config.AUTH_MODE === "user_oauth") { + const googleToken = await getGoogleAccessToken(); + if (!googleToken) { + console.warn("requireSession: User OAuth token expired/missing. Clearing session."); + await clearSessionCookies(); + return null; + } + } + return session; } + +/** + * Clears all BetterAuth session cookies from the browser. + */ +async function clearSessionCookies() { + const cookieStore = await cookies(); + // Better Auth session cookie names contain "session_token" + const sessionCookies = cookieStore.getAll().filter((c) => c.name.includes("session_token")); + if (sessionCookies.length > 0) { + console.warn( + "requireSession: Clearing stale session cookies:", + sessionCookies.map((c) => c.name), + ); + for (const cookie of sessionCookies) { + cookieStore.delete(cookie.name); + } + } +} From a7c94513926913119f5bed87a4c7ce80a1801b39 Mon Sep 17 00:00:00 2001 From: Dylan Klein Date: Tue, 4 Aug 2026 22:14:45 +0000 Subject: [PATCH 2/2] fix: add automatic client-side signout on stale user session to prevent redirect loops - Restored client-side redirect to root page on unauthenticated session in user_oauth mode. - Added explicit authClient.signOut() call before redirecting to ensure local and database-level session cookies are destroyed, preventing concurrent get-session calls from resurrecting the stale cookie. TAG=agy CONV=b3471264-2592-4924-b031-267b4bf62326 --- .../pocket-cep/src/__tests__/unit/session.test.ts | 11 +++++++---- .../src/components/auth-health-provider.tsx | 7 +++++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/mcp-examples/pocket-cep/src/__tests__/unit/session.test.ts b/mcp-examples/pocket-cep/src/__tests__/unit/session.test.ts index 77ddfcd..7bb53e2 100644 --- a/mcp-examples/pocket-cep/src/__tests__/unit/session.test.ts +++ b/mcp-examples/pocket-cep/src/__tests__/unit/session.test.ts @@ -7,10 +7,13 @@ const { mockGetSession, mockCookies, mockGetGoogleAccessToken, mockGetEnv } = vi delete: vi.fn(), }, mockGetGoogleAccessToken: vi.fn(), - mockGetEnv: vi.fn(() => ({ - AUTH_MODE: "service_account" as "service_account" | "user_oauth", - BETTER_AUTH_SECRET: "mock-secret", - })), + mockGetEnv: vi.fn( + () => + ({ + AUTH_MODE: "service_account" as "service_account" | "user_oauth", + BETTER_AUTH_SECRET: "mock-secret", + }) as Record, + ), })); vi.mock("next/headers", () => ({ diff --git a/mcp-examples/pocket-cep/src/components/auth-health-provider.tsx b/mcp-examples/pocket-cep/src/components/auth-health-provider.tsx index ebbaf5a..b965b18 100644 --- a/mcp-examples/pocket-cep/src/components/auth-health-provider.tsx +++ b/mcp-examples/pocket-cep/src/components/auth-health-provider.tsx @@ -18,6 +18,7 @@ import type { ReactNode } from "react"; import type { AuthErrorPayload } from "@/lib/auth-errors"; import { AUTH_ERROR_EVENT } from "@/lib/auth-aware-fetch"; import { useMode } from "./mode-provider"; +import { authClient } from "@/lib/auth-client"; /** * Value exposed by the AuthHealthContext. `clear()` drops the current @@ -80,9 +81,11 @@ export function AuthHealthProvider({ children }: { children: ReactNode }) { } else { console.warn( "AuthHealthProvider: Stale session detected in User OAuth mode. " + - "Redirecting to login page.", + "Logging out and redirecting to login page.", ); - window.location.href = "/"; + authClient.signOut().finally(() => { + window.location.href = "/"; + }); } } else if (detail && typeof detail.code === "string") { setError(detail);