From 0c2a6b14cff7f4a8855480ebc2a1ea4562a4b8b5 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Wed, 18 Feb 2026 22:39:54 -0600 Subject: [PATCH] fix(auth): verify BetterAuth sessions via cookie headers --- apps/api/src/auth/auth.service.spec.ts | 46 +++++-- apps/api/src/auth/auth.service.ts | 104 +++++++++++----- .../362-auth-session-chain-debug.md | 114 ++++++++++++++++++ 3 files changed, 224 insertions(+), 40 deletions(-) create mode 100644 docs/scratchpads/362-auth-session-chain-debug.md diff --git a/apps/api/src/auth/auth.service.spec.ts b/apps/api/src/auth/auth.service.spec.ts index 5cc01b9..bc60214 100644 --- a/apps/api/src/auth/auth.service.spec.ts +++ b/apps/api/src/auth/auth.service.spec.ts @@ -410,7 +410,7 @@ describe("AuthService", () => { }, }; - it("should return session data for valid token", async () => { + it("should validate session token using secure BetterAuth cookie header", async () => { const auth = service.getAuth(); const mockGetSession = vi.fn().mockResolvedValue(mockSessionData); auth.api = { getSession: mockGetSession } as any; @@ -418,7 +418,43 @@ describe("AuthService", () => { const result = await service.verifySession("valid-token"); expect(result).toEqual(mockSessionData); + expect(mockGetSession).toHaveBeenCalledTimes(1); expect(mockGetSession).toHaveBeenCalledWith({ + headers: { + cookie: "__Secure-better-auth.session_token=valid-token", + }, + }); + }); + + it("should fall back to Authorization header when cookie-based lookups miss", async () => { + const auth = service.getAuth(); + const mockGetSession = vi + .fn() + .mockResolvedValueOnce(null) + .mockResolvedValueOnce(null) + .mockResolvedValueOnce(null) + .mockResolvedValueOnce(mockSessionData); + auth.api = { getSession: mockGetSession } as any; + + const result = await service.verifySession("valid-token"); + + expect(result).toEqual(mockSessionData); + expect(mockGetSession).toHaveBeenNthCalledWith(1, { + headers: { + cookie: "__Secure-better-auth.session_token=valid-token", + }, + }); + expect(mockGetSession).toHaveBeenNthCalledWith(2, { + headers: { + cookie: "better-auth.session_token=valid-token", + }, + }); + expect(mockGetSession).toHaveBeenNthCalledWith(3, { + headers: { + cookie: "__Host-better-auth.session_token=valid-token", + }, + }); + expect(mockGetSession).toHaveBeenNthCalledWith(4, { headers: { authorization: "Bearer valid-token", }, @@ -517,14 +553,10 @@ describe("AuthService", () => { it("should re-throw 'certificate has expired' as infrastructure error (not auth)", async () => { const auth = service.getAuth(); - const mockGetSession = vi - .fn() - .mockRejectedValue(new Error("certificate has expired")); + const mockGetSession = vi.fn().mockRejectedValue(new Error("certificate has expired")); auth.api = { getSession: mockGetSession } as any; - await expect(service.verifySession("any-token")).rejects.toThrow( - "certificate has expired" - ); + await expect(service.verifySession("any-token")).rejects.toThrow("certificate has expired"); }); it("should re-throw 'Unauthorized: Access denied for user' as infrastructure error (not auth)", async () => { diff --git a/apps/api/src/auth/auth.service.ts b/apps/api/src/auth/auth.service.ts index 97e8d4b..6a4906d 100644 --- a/apps/api/src/auth/auth.service.ts +++ b/apps/api/src/auth/auth.service.ts @@ -21,6 +21,10 @@ interface VerifiedSession { session: Record; } +interface SessionHeaderCandidate { + headers: Record; +} + @Injectable() export class AuthService { private readonly logger = new Logger(AuthService.name); @@ -103,36 +107,27 @@ export class AuthService { * Only known-safe auth errors return null; everything else propagates as 500. */ async verifySession(token: string): Promise { - try { - // TODO(#411): BetterAuth getSession returns opaque types — replace when upstream exports typed interfaces - const session = await this.auth.api.getSession({ - headers: { - authorization: `Bearer ${token}`, - }, - }); + let sawNonError = false; - if (!session) { - return null; - } + for (const candidate of this.buildSessionHeaderCandidates(token)) { + try { + // TODO(#411): BetterAuth getSession returns opaque types — replace when upstream exports typed interfaces + const session = await this.auth.api.getSession(candidate); - return { - user: session.user as Record, - session: session.session as Record, - }; - } catch (error: unknown) { - // Only known-safe auth errors return null - if (error instanceof Error) { - const msg = error.message.toLowerCase(); - const isExpectedAuthError = - msg.includes("invalid token") || - msg.includes("token expired") || - msg.includes("session expired") || - msg.includes("session not found") || - msg.includes("invalid session") || - msg === "unauthorized" || - msg === "expired"; + if (!session) { + continue; + } + + return { + user: session.user as Record, + session: session.session as Record, + }; + } catch (error: unknown) { + if (error instanceof Error) { + if (this.isExpectedAuthError(error.message)) { + continue; + } - if (!isExpectedAuthError) { // Infrastructure or unexpected — propagate as 500 const safeMessage = (error.stack ?? error.message).replace( /Bearer\s+\S+/gi, @@ -141,14 +136,57 @@ export class AuthService { this.logger.error("Session verification failed due to unexpected error", safeMessage); throw error; } + + // Non-Error thrown values — log once for observability, treat as auth failure + if (!sawNonError) { + const errorDetail = typeof error === "string" ? error : JSON.stringify(error); + this.logger.warn("Session verification received non-Error thrown value", errorDetail); + sawNonError = true; + } } - // Non-Error thrown values — log for observability, treat as auth failure - if (!(error instanceof Error)) { - const errorDetail = typeof error === "string" ? error : JSON.stringify(error); - this.logger.warn("Session verification received non-Error thrown value", errorDetail); - } - return null; } + + return null; + } + + private buildSessionHeaderCandidates(token: string): SessionHeaderCandidate[] { + const encodedToken = encodeURIComponent(token); + + return [ + { + headers: { + cookie: `__Secure-better-auth.session_token=${encodedToken}`, + }, + }, + { + headers: { + cookie: `better-auth.session_token=${encodedToken}`, + }, + }, + { + headers: { + cookie: `__Host-better-auth.session_token=${encodedToken}`, + }, + }, + { + headers: { + authorization: `Bearer ${token}`, + }, + }, + ]; + } + + private isExpectedAuthError(message: string): boolean { + const normalized = message.toLowerCase(); + return ( + normalized.includes("invalid token") || + normalized.includes("token expired") || + normalized.includes("session expired") || + normalized.includes("session not found") || + normalized.includes("invalid session") || + normalized === "unauthorized" || + normalized === "expired" + ); } /** diff --git a/docs/scratchpads/362-auth-session-chain-debug.md b/docs/scratchpads/362-auth-session-chain-debug.md new file mode 100644 index 0000000..a06d488 --- /dev/null +++ b/docs/scratchpads/362-auth-session-chain-debug.md @@ -0,0 +1,114 @@ +# 362 - Auth Session Chain Debug (Authentik -> BetterAuth -> API Guard) + +## Context + +- Date (UTC): 2026-02-19 +- Environment under test: production domains + - Web: `https://app.mosaicstack.dev/login` + - API: `https://api.mosaicstack.dev` + - IdP: `https://auth.diversecanvas.com` +- Tooling: Playwright MCP + Chromium + +## Problem Statement + +Users can complete Authentik login and consent, but Mosaic web app returns to login and remains unauthenticated. + +## Timeline and Evidence + +1. Initial reproduction from web login: + - `POST /auth/sign-in/oauth2` returned `200` with Authentik authorize URL. + - Authentik login flow and consent screen loaded correctly. + +2. First callback failure mode (before `jarvis` email fix): + - Callback ended at API error redirect with `error=email_is_missing`. + - Result URL: `https://api.mosaicstack.dev/?error=email_is_missing`. + +3. User updated Authentik account: + - `jarvis` account email set to `jarvis@mosaic.local`. + - `email_is_missing` failure no longer occurs. + +4. Current callback behavior (after email fix): + - `GET /auth/oauth2/callback/authentik?code=...&state=...` returns `302` to `https://app.mosaicstack.dev/`. + - Callback sets BetterAuth cookies: + - `__Secure-better-auth.state=...; Max-Age=0; ...` + - `__Secure-better-auth.session_token=...; Max-Age=604800; Path=/; HttpOnly; Secure; SameSite=Lax` + - Browser cookie jar confirms session cookie present for `api.mosaicstack.dev`. + +5. Session validation mismatch (critical): + - BetterAuth direct session endpoint succeeds: + - `GET /auth/get-session` -> `200` with session payload. + - Guarded API session endpoint fails: + - `GET /auth/session` -> `401` with + `{"message":"Invalid or expired session", ...}` + - Reproduced repeatedly in same browser context immediately after callback. + +## Config Sync Notes + +User synced local files with deployed Portainer stack: + +- `.env` updated with deployed values. +- `docker-compose.swarm.portainer.yml` changed: + - Removed `BETTER_AUTH_URL` env mapping from API service. + +Observed auth behavior after sync: + +- Improvement: removed `email_is_missing` callback error. +- Remaining failure: `/auth/session` still returns 401 despite valid BetterAuth cookie and successful `/auth/get-session`. + +## Root Cause Hypothesis (Strong) + +`AuthGuard` extracts BetterAuth session cookie token correctly, but `AuthService.verifySession()` validates it using `Authorization: Bearer ` instead of a BetterAuth cookie/header context. + +Relevant code paths: + +- `apps/api/src/auth/guards/auth.guard.ts` + - extracts `__Secure-better-auth.session_token` / `better-auth.session_token` +- `apps/api/src/auth/auth.service.ts` + - `verifySession()` calls `auth.api.getSession({ headers: { authorization: "Bearer ..." } })` + +Why this matches evidence: + +- `/auth/get-session` (native BetterAuth endpoint reading request cookie) succeeds. +- `/auth/session` (custom guard + verify path) fails for same browser session. + +## Next Actions + +1. Fix `verifySession()` to validate using BetterAuth-compatible cookie header candidates first, with bearer fallback for API clients. +2. Add/update unit tests in `auth.service.spec.ts` to cover cookie-first validation and bearer fallback. +3. Re-run targeted API auth tests. +4. Re-run Playwright auth chain to confirm: + - callback sets cookie + - `/auth/session` returns `200` + - web app transitions out of `/login`. + +## Implementation Update (2026-02-19) + +Completed items: + +1. Updated backend session verification logic: + - File: `apps/api/src/auth/auth.service.ts` + - `verifySession()` now tries session resolution in this order: + - `cookie: __Secure-better-auth.session_token=` + - `cookie: better-auth.session_token=` + - `cookie: __Host-better-auth.session_token=` + - `authorization: Bearer ` (fallback) + - Added helper methods: + - `buildSessionHeaderCandidates()` + - `isExpectedAuthError()` + +2. Added/updated tests: + - File: `apps/api/src/auth/auth.service.spec.ts` + - Added RED->GREEN test: + - `should validate session token using secure BetterAuth cookie header` + - Updated fallback coverage test: + - `should fall back to Authorization header when cookie-based lookups miss` + +3. Verification: + - Command: `pnpm --filter @mosaic/api test -- src/auth/auth.service.spec.ts` + - Result: pass (all tests green). + - Command: `pnpm --filter @mosaic/api lint` + - Result: pass. + +Remaining step (requires deploy): + +- Redeploy API with this patch and rerun live Playwright flow on `app.mosaicstack.dev` to confirm `/auth/session` returns `200` after callback.