fix(auth): verify BetterAuth sessions via cookie headers
This commit is contained in:
@@ -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 () => {
|
||||
|
||||
@@ -21,6 +21,10 @@ interface VerifiedSession {
|
||||
session: Record<string, unknown>;
|
||||
}
|
||||
|
||||
interface SessionHeaderCandidate {
|
||||
headers: Record<string, string>;
|
||||
}
|
||||
|
||||
@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<VerifiedSession | null> {
|
||||
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<string, unknown>,
|
||||
session: session.session as Record<string, unknown>,
|
||||
};
|
||||
} 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<string, unknown>,
|
||||
session: session.session as Record<string, unknown>,
|
||||
};
|
||||
} 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"
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
114
docs/scratchpads/362-auth-session-chain-debug.md
Normal file
114
docs/scratchpads/362-auth-session-chain-debug.md
Normal file
@@ -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 <token>` 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=<token>`
|
||||
- `cookie: better-auth.session_token=<token>`
|
||||
- `cookie: __Host-better-auth.session_token=<token>`
|
||||
- `authorization: Bearer <token>` (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.
|
||||
Reference in New Issue
Block a user