diff --git a/.env.example b/.env.example index 1d260de..396d74e 100644 --- a/.env.example +++ b/.env.example @@ -61,7 +61,7 @@ KNOWLEDGE_CACHE_TTL=300 # Authentication (Authentik OIDC) # ====================== # Set to 'true' to enable OIDC authentication with Authentik -# When enabled, OIDC_ISSUER, OIDC_CLIENT_ID, and OIDC_CLIENT_SECRET are required +# When enabled, OIDC_ISSUER, OIDC_CLIENT_ID, OIDC_CLIENT_SECRET, and OIDC_REDIRECT_URI are required OIDC_ENABLED=false # Authentik Server URLs (required when OIDC_ENABLED=true) @@ -117,6 +117,14 @@ JWT_EXPIRATION=24h # Example: openssl rand -base64 32 BETTER_AUTH_SECRET=REPLACE_WITH_RANDOM_SECRET_MINIMUM_32_CHARS +# Trusted Origins (comma-separated list of additional trusted origins for CORS and auth) +# These are added to NEXT_PUBLIC_APP_URL and NEXT_PUBLIC_API_URL automatically +TRUSTED_ORIGINS= + +# Cookie Domain (for cross-subdomain session sharing) +# Leave empty for single-domain setups. Set to ".example.com" for cross-subdomain. +COOKIE_DOMAIN= + # ====================== # Encryption (Credential Security) # ====================== diff --git a/apps/api/src/auth/auth.config.spec.ts b/apps/api/src/auth/auth.config.spec.ts index cdf422c..794ebb6 100644 --- a/apps/api/src/auth/auth.config.spec.ts +++ b/apps/api/src/auth/auth.config.spec.ts @@ -1,5 +1,24 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; -import { isOidcEnabled, validateOidcConfig } from "./auth.config"; +import type { PrismaClient } from "@prisma/client"; + +// Mock better-auth modules to inspect genericOAuth plugin configuration +const mockGenericOAuth = vi.fn().mockReturnValue({ id: "generic-oauth" }); +const mockBetterAuth = vi.fn().mockReturnValue({ handler: vi.fn() }); +const mockPrismaAdapter = vi.fn().mockReturnValue({}); + +vi.mock("better-auth/plugins", () => ({ + genericOAuth: (...args: unknown[]) => mockGenericOAuth(...args), +})); + +vi.mock("better-auth", () => ({ + betterAuth: (...args: unknown[]) => mockBetterAuth(...args), +})); + +vi.mock("better-auth/adapters/prisma", () => ({ + prismaAdapter: (...args: unknown[]) => mockPrismaAdapter(...args), +})); + +import { isOidcEnabled, validateOidcConfig, createAuth, getTrustedOrigins } from "./auth.config"; describe("auth.config", () => { // Store original env vars to restore after each test @@ -11,6 +30,12 @@ describe("auth.config", () => { delete process.env.OIDC_ISSUER; delete process.env.OIDC_CLIENT_ID; delete process.env.OIDC_CLIENT_SECRET; + delete process.env.OIDC_REDIRECT_URI; + delete process.env.NODE_ENV; + delete process.env.NEXT_PUBLIC_APP_URL; + delete process.env.NEXT_PUBLIC_API_URL; + delete process.env.TRUSTED_ORIGINS; + delete process.env.COOKIE_DOMAIN; }); afterEach(() => { @@ -70,6 +95,7 @@ describe("auth.config", () => { it("should throw when OIDC_ISSUER is missing", () => { process.env.OIDC_CLIENT_ID = "test-client-id"; process.env.OIDC_CLIENT_SECRET = "test-client-secret"; + process.env.OIDC_REDIRECT_URI = "https://app.example.com/auth/callback/authentik"; expect(() => validateOidcConfig()).toThrow("OIDC_ISSUER"); expect(() => validateOidcConfig()).toThrow("OIDC authentication is enabled"); @@ -78,6 +104,7 @@ describe("auth.config", () => { it("should throw when OIDC_CLIENT_ID is missing", () => { process.env.OIDC_ISSUER = "https://auth.example.com/"; process.env.OIDC_CLIENT_SECRET = "test-client-secret"; + process.env.OIDC_REDIRECT_URI = "https://app.example.com/auth/callback/authentik"; expect(() => validateOidcConfig()).toThrow("OIDC_CLIENT_ID"); }); @@ -85,13 +112,22 @@ describe("auth.config", () => { it("should throw when OIDC_CLIENT_SECRET is missing", () => { process.env.OIDC_ISSUER = "https://auth.example.com/"; process.env.OIDC_CLIENT_ID = "test-client-id"; + process.env.OIDC_REDIRECT_URI = "https://app.example.com/auth/callback/authentik"; expect(() => validateOidcConfig()).toThrow("OIDC_CLIENT_SECRET"); }); + it("should throw when OIDC_REDIRECT_URI is missing", () => { + process.env.OIDC_ISSUER = "https://auth.example.com/"; + process.env.OIDC_CLIENT_ID = "test-client-id"; + process.env.OIDC_CLIENT_SECRET = "test-client-secret"; + + expect(() => validateOidcConfig()).toThrow("OIDC_REDIRECT_URI"); + }); + it("should throw when all required vars are missing", () => { expect(() => validateOidcConfig()).toThrow( - "OIDC_ISSUER, OIDC_CLIENT_ID, OIDC_CLIENT_SECRET" + "OIDC_ISSUER, OIDC_CLIENT_ID, OIDC_CLIENT_SECRET, OIDC_REDIRECT_URI" ); }); @@ -99,9 +135,10 @@ describe("auth.config", () => { process.env.OIDC_ISSUER = ""; process.env.OIDC_CLIENT_ID = ""; process.env.OIDC_CLIENT_SECRET = ""; + process.env.OIDC_REDIRECT_URI = ""; expect(() => validateOidcConfig()).toThrow( - "OIDC_ISSUER, OIDC_CLIENT_ID, OIDC_CLIENT_SECRET" + "OIDC_ISSUER, OIDC_CLIENT_ID, OIDC_CLIENT_SECRET, OIDC_REDIRECT_URI" ); }); @@ -109,6 +146,7 @@ describe("auth.config", () => { process.env.OIDC_ISSUER = " "; process.env.OIDC_CLIENT_ID = "test-client-id"; process.env.OIDC_CLIENT_SECRET = "test-client-secret"; + process.env.OIDC_REDIRECT_URI = "https://app.example.com/auth/callback/authentik"; expect(() => validateOidcConfig()).toThrow("OIDC_ISSUER"); }); @@ -117,6 +155,7 @@ describe("auth.config", () => { process.env.OIDC_ISSUER = "https://auth.example.com/application/o/mosaic"; process.env.OIDC_CLIENT_ID = "test-client-id"; process.env.OIDC_CLIENT_SECRET = "test-client-secret"; + process.env.OIDC_REDIRECT_URI = "https://app.example.com/auth/callback/authentik"; expect(() => validateOidcConfig()).toThrow("OIDC_ISSUER must end with a trailing slash"); expect(() => validateOidcConfig()).toThrow("https://auth.example.com/application/o/mosaic"); @@ -126,6 +165,7 @@ describe("auth.config", () => { process.env.OIDC_ISSUER = "https://auth.example.com/application/o/mosaic-stack/"; process.env.OIDC_CLIENT_ID = "test-client-id"; process.env.OIDC_CLIENT_SECRET = "test-client-secret"; + process.env.OIDC_REDIRECT_URI = "https://app.example.com/auth/callback/authentik"; expect(() => validateOidcConfig()).not.toThrow(); }); @@ -133,6 +173,455 @@ describe("auth.config", () => { it("should suggest disabling OIDC in error message", () => { expect(() => validateOidcConfig()).toThrow("OIDC_ENABLED=false"); }); + + describe("OIDC_REDIRECT_URI validation", () => { + beforeEach(() => { + process.env.OIDC_ISSUER = "https://auth.example.com/application/o/mosaic-stack/"; + process.env.OIDC_CLIENT_ID = "test-client-id"; + process.env.OIDC_CLIENT_SECRET = "test-client-secret"; + }); + + it("should throw when OIDC_REDIRECT_URI is not a valid URL", () => { + process.env.OIDC_REDIRECT_URI = "not-a-url"; + + expect(() => validateOidcConfig()).toThrow("OIDC_REDIRECT_URI must be a valid URL"); + expect(() => validateOidcConfig()).toThrow("not-a-url"); + expect(() => validateOidcConfig()).toThrow("Parse error:"); + }); + + it("should throw when OIDC_REDIRECT_URI path does not start with /auth/callback", () => { + process.env.OIDC_REDIRECT_URI = "https://app.example.com/oauth/callback"; + + expect(() => validateOidcConfig()).toThrow( + 'OIDC_REDIRECT_URI path must start with "/auth/callback"' + ); + expect(() => validateOidcConfig()).toThrow("/oauth/callback"); + }); + + it("should accept a valid OIDC_REDIRECT_URI with /auth/callback path", () => { + process.env.OIDC_REDIRECT_URI = "https://app.example.com/auth/callback/authentik"; + + expect(() => validateOidcConfig()).not.toThrow(); + }); + + it("should accept OIDC_REDIRECT_URI with exactly /auth/callback path", () => { + process.env.OIDC_REDIRECT_URI = "https://app.example.com/auth/callback"; + + expect(() => validateOidcConfig()).not.toThrow(); + }); + + it("should warn but not throw when using localhost in production", () => { + process.env.NODE_ENV = "production"; + process.env.OIDC_REDIRECT_URI = "http://localhost:3000/auth/callback/authentik"; + + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + + expect(() => validateOidcConfig()).not.toThrow(); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining("OIDC_REDIRECT_URI uses localhost") + ); + + warnSpy.mockRestore(); + }); + + it("should warn but not throw when using 127.0.0.1 in production", () => { + process.env.NODE_ENV = "production"; + process.env.OIDC_REDIRECT_URI = "http://127.0.0.1:3000/auth/callback/authentik"; + + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + + expect(() => validateOidcConfig()).not.toThrow(); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining("OIDC_REDIRECT_URI uses localhost") + ); + + warnSpy.mockRestore(); + }); + + it("should not warn about localhost when not in production", () => { + process.env.NODE_ENV = "development"; + process.env.OIDC_REDIRECT_URI = "http://localhost:3000/auth/callback/authentik"; + + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + + expect(() => validateOidcConfig()).not.toThrow(); + expect(warnSpy).not.toHaveBeenCalled(); + + warnSpy.mockRestore(); + }); + }); + }); + }); + + describe("createAuth - genericOAuth PKCE configuration", () => { + beforeEach(() => { + mockGenericOAuth.mockClear(); + mockBetterAuth.mockClear(); + mockPrismaAdapter.mockClear(); + }); + + it("should enable PKCE in the genericOAuth provider config when OIDC is enabled", () => { + process.env.OIDC_ENABLED = "true"; + process.env.OIDC_ISSUER = "https://auth.example.com/application/o/mosaic-stack/"; + process.env.OIDC_CLIENT_ID = "test-client-id"; + process.env.OIDC_CLIENT_SECRET = "test-client-secret"; + process.env.OIDC_REDIRECT_URI = "https://app.example.com/auth/callback/authentik"; + + const mockPrisma = {} as PrismaClient; + createAuth(mockPrisma); + + expect(mockGenericOAuth).toHaveBeenCalledOnce(); + const callArgs = mockGenericOAuth.mock.calls[0][0] as { + config: Array<{ pkce?: boolean }>; + }; + expect(callArgs.config[0].pkce).toBe(true); + }); + + it("should not call genericOAuth when OIDC is disabled", () => { + process.env.OIDC_ENABLED = "false"; + + const mockPrisma = {} as PrismaClient; + createAuth(mockPrisma); + + expect(mockGenericOAuth).not.toHaveBeenCalled(); + }); + + it("should throw if OIDC_CLIENT_ID is missing when OIDC is enabled", () => { + process.env.OIDC_ENABLED = "true"; + process.env.OIDC_ISSUER = "https://auth.example.com/application/o/mosaic-stack/"; + process.env.OIDC_CLIENT_SECRET = "test-client-secret"; + process.env.OIDC_REDIRECT_URI = "https://app.example.com/auth/callback/authentik"; + // OIDC_CLIENT_ID deliberately not set + + // validateOidcConfig will throw first, so we need to bypass it + // by setting the var then deleting it after validation + // Instead, test via the validation path which is fine — but let's + // verify the plugin-level guard by using a direct approach: + // Set env to pass validateOidcConfig, then delete OIDC_CLIENT_ID + // The validateOidcConfig will catch this first, which is correct behavior + const mockPrisma = {} as PrismaClient; + expect(() => createAuth(mockPrisma)).toThrow("OIDC_CLIENT_ID"); + }); + + it("should throw if OIDC_CLIENT_SECRET is missing when OIDC is enabled", () => { + process.env.OIDC_ENABLED = "true"; + process.env.OIDC_ISSUER = "https://auth.example.com/application/o/mosaic-stack/"; + process.env.OIDC_CLIENT_ID = "test-client-id"; + process.env.OIDC_REDIRECT_URI = "https://app.example.com/auth/callback/authentik"; + // OIDC_CLIENT_SECRET deliberately not set + + const mockPrisma = {} as PrismaClient; + expect(() => createAuth(mockPrisma)).toThrow("OIDC_CLIENT_SECRET"); + }); + + it("should throw if OIDC_ISSUER is missing when OIDC is enabled", () => { + process.env.OIDC_ENABLED = "true"; + process.env.OIDC_CLIENT_ID = "test-client-id"; + process.env.OIDC_CLIENT_SECRET = "test-client-secret"; + process.env.OIDC_REDIRECT_URI = "https://app.example.com/auth/callback/authentik"; + // OIDC_ISSUER deliberately not set + + const mockPrisma = {} as PrismaClient; + expect(() => createAuth(mockPrisma)).toThrow("OIDC_ISSUER"); + }); + }); + + describe("getTrustedOrigins", () => { + it("should return localhost URLs when NODE_ENV is not production", () => { + process.env.NODE_ENV = "development"; + + const origins = getTrustedOrigins(); + + expect(origins).toContain("http://localhost:3000"); + expect(origins).toContain("http://localhost:3001"); + }); + + it("should return localhost URLs when NODE_ENV is not set", () => { + // NODE_ENV is deleted in beforeEach, so it's undefined here + const origins = getTrustedOrigins(); + + expect(origins).toContain("http://localhost:3000"); + expect(origins).toContain("http://localhost:3001"); + }); + + it("should exclude localhost URLs in production", () => { + process.env.NODE_ENV = "production"; + + const origins = getTrustedOrigins(); + + expect(origins).not.toContain("http://localhost:3000"); + expect(origins).not.toContain("http://localhost:3001"); + }); + + it("should parse TRUSTED_ORIGINS comma-separated values", () => { + process.env.TRUSTED_ORIGINS = + "https://app.mosaicstack.dev,https://api.mosaicstack.dev"; + + const origins = getTrustedOrigins(); + + expect(origins).toContain("https://app.mosaicstack.dev"); + expect(origins).toContain("https://api.mosaicstack.dev"); + }); + + it("should trim whitespace from TRUSTED_ORIGINS entries", () => { + process.env.TRUSTED_ORIGINS = + " https://app.mosaicstack.dev , https://api.mosaicstack.dev "; + + const origins = getTrustedOrigins(); + + expect(origins).toContain("https://app.mosaicstack.dev"); + expect(origins).toContain("https://api.mosaicstack.dev"); + }); + + it("should filter out empty strings from TRUSTED_ORIGINS", () => { + process.env.TRUSTED_ORIGINS = "https://app.mosaicstack.dev,,, ,"; + + const origins = getTrustedOrigins(); + + expect(origins).toContain("https://app.mosaicstack.dev"); + // No empty strings in the result + origins.forEach((o) => expect(o).not.toBe("")); + }); + + it("should include NEXT_PUBLIC_APP_URL", () => { + process.env.NEXT_PUBLIC_APP_URL = "https://my-app.example.com"; + + const origins = getTrustedOrigins(); + + expect(origins).toContain("https://my-app.example.com"); + }); + + it("should include NEXT_PUBLIC_API_URL", () => { + process.env.NEXT_PUBLIC_API_URL = "https://my-api.example.com"; + + const origins = getTrustedOrigins(); + + expect(origins).toContain("https://my-api.example.com"); + }); + + it("should deduplicate origins", () => { + process.env.NEXT_PUBLIC_APP_URL = "http://localhost:3000"; + process.env.TRUSTED_ORIGINS = "http://localhost:3000,http://localhost:3001"; + // NODE_ENV not set, so localhost fallbacks are also added + + const origins = getTrustedOrigins(); + + const countLocalhost3000 = origins.filter((o) => o === "http://localhost:3000").length; + const countLocalhost3001 = origins.filter((o) => o === "http://localhost:3001").length; + expect(countLocalhost3000).toBe(1); + expect(countLocalhost3001).toBe(1); + }); + + it("should handle all env vars missing gracefully", () => { + // All env vars deleted in beforeEach; NODE_ENV is also deleted (not production) + const origins = getTrustedOrigins(); + + // Should still return localhost fallbacks since not in production + expect(origins).toContain("http://localhost:3000"); + expect(origins).toContain("http://localhost:3001"); + expect(origins).toHaveLength(2); + }); + + it("should return empty array when all env vars missing in production", () => { + process.env.NODE_ENV = "production"; + + const origins = getTrustedOrigins(); + + expect(origins).toHaveLength(0); + }); + + it("should combine all sources correctly", () => { + process.env.NEXT_PUBLIC_APP_URL = "https://app.mosaicstack.dev"; + process.env.NEXT_PUBLIC_API_URL = "https://api.mosaicstack.dev"; + process.env.TRUSTED_ORIGINS = "https://extra.example.com"; + process.env.NODE_ENV = "development"; + + const origins = getTrustedOrigins(); + + expect(origins).toContain("https://app.mosaicstack.dev"); + expect(origins).toContain("https://api.mosaicstack.dev"); + expect(origins).toContain("https://extra.example.com"); + expect(origins).toContain("http://localhost:3000"); + expect(origins).toContain("http://localhost:3001"); + expect(origins).toHaveLength(5); + }); + + it("should reject invalid URLs in TRUSTED_ORIGINS with a warning including error details", () => { + process.env.TRUSTED_ORIGINS = "not-a-url,https://valid.example.com"; + process.env.NODE_ENV = "production"; + + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + + const origins = getTrustedOrigins(); + + expect(origins).toContain("https://valid.example.com"); + expect(origins).not.toContain("not-a-url"); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('Ignoring invalid URL in TRUSTED_ORIGINS: "not-a-url"') + ); + // Verify that error detail is included in the warning + const warnCall = warnSpy.mock.calls.find( + (call) => typeof call[0] === "string" && call[0].includes("not-a-url") + ); + expect(warnCall).toBeDefined(); + expect(warnCall![0]).toMatch(/\(.*\)$/); + + warnSpy.mockRestore(); + }); + + it("should reject non-HTTP origins in TRUSTED_ORIGINS with a warning", () => { + process.env.TRUSTED_ORIGINS = "ftp://files.example.com,https://valid.example.com"; + process.env.NODE_ENV = "production"; + + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + + const origins = getTrustedOrigins(); + + expect(origins).toContain("https://valid.example.com"); + expect(origins).not.toContain("ftp://files.example.com"); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining("Ignoring non-HTTP origin in TRUSTED_ORIGINS") + ); + + warnSpy.mockRestore(); + }); + }); + + describe("createAuth - session and cookie configuration", () => { + beforeEach(() => { + mockGenericOAuth.mockClear(); + mockBetterAuth.mockClear(); + mockPrismaAdapter.mockClear(); + }); + + it("should configure session expiresIn to 7 days (604800 seconds)", () => { + const mockPrisma = {} as PrismaClient; + createAuth(mockPrisma); + + expect(mockBetterAuth).toHaveBeenCalledOnce(); + const config = mockBetterAuth.mock.calls[0][0] as { + session: { expiresIn: number; updateAge: number }; + }; + expect(config.session.expiresIn).toBe(604800); + }); + + it("should configure session updateAge to 2 hours (7200 seconds)", () => { + const mockPrisma = {} as PrismaClient; + createAuth(mockPrisma); + + expect(mockBetterAuth).toHaveBeenCalledOnce(); + const config = mockBetterAuth.mock.calls[0][0] as { + session: { expiresIn: number; updateAge: number }; + }; + expect(config.session.updateAge).toBe(7200); + }); + + it("should set httpOnly cookie attribute to true", () => { + const mockPrisma = {} as PrismaClient; + createAuth(mockPrisma); + + expect(mockBetterAuth).toHaveBeenCalledOnce(); + const config = mockBetterAuth.mock.calls[0][0] as { + advanced: { + defaultCookieAttributes: { + httpOnly: boolean; + secure: boolean; + sameSite: string; + }; + }; + }; + expect(config.advanced.defaultCookieAttributes.httpOnly).toBe(true); + }); + + it("should set sameSite cookie attribute to lax", () => { + const mockPrisma = {} as PrismaClient; + createAuth(mockPrisma); + + expect(mockBetterAuth).toHaveBeenCalledOnce(); + const config = mockBetterAuth.mock.calls[0][0] as { + advanced: { + defaultCookieAttributes: { + httpOnly: boolean; + secure: boolean; + sameSite: string; + }; + }; + }; + expect(config.advanced.defaultCookieAttributes.sameSite).toBe("lax"); + }); + + it("should set secure cookie attribute to true in production", () => { + process.env.NODE_ENV = "production"; + const mockPrisma = {} as PrismaClient; + createAuth(mockPrisma); + + expect(mockBetterAuth).toHaveBeenCalledOnce(); + const config = mockBetterAuth.mock.calls[0][0] as { + advanced: { + defaultCookieAttributes: { + httpOnly: boolean; + secure: boolean; + sameSite: string; + }; + }; + }; + expect(config.advanced.defaultCookieAttributes.secure).toBe(true); + }); + + it("should set secure cookie attribute to false in non-production", () => { + process.env.NODE_ENV = "development"; + const mockPrisma = {} as PrismaClient; + createAuth(mockPrisma); + + expect(mockBetterAuth).toHaveBeenCalledOnce(); + const config = mockBetterAuth.mock.calls[0][0] as { + advanced: { + defaultCookieAttributes: { + httpOnly: boolean; + secure: boolean; + sameSite: string; + }; + }; + }; + expect(config.advanced.defaultCookieAttributes.secure).toBe(false); + }); + + it("should set cookie domain when COOKIE_DOMAIN env var is present", () => { + process.env.COOKIE_DOMAIN = ".mosaicstack.dev"; + const mockPrisma = {} as PrismaClient; + createAuth(mockPrisma); + + expect(mockBetterAuth).toHaveBeenCalledOnce(); + const config = mockBetterAuth.mock.calls[0][0] as { + advanced: { + defaultCookieAttributes: { + httpOnly: boolean; + secure: boolean; + sameSite: string; + domain?: string; + }; + }; + }; + expect(config.advanced.defaultCookieAttributes.domain).toBe(".mosaicstack.dev"); + }); + + it("should not set cookie domain when COOKIE_DOMAIN env var is absent", () => { + delete process.env.COOKIE_DOMAIN; + const mockPrisma = {} as PrismaClient; + createAuth(mockPrisma); + + expect(mockBetterAuth).toHaveBeenCalledOnce(); + const config = mockBetterAuth.mock.calls[0][0] as { + advanced: { + defaultCookieAttributes: { + httpOnly: boolean; + secure: boolean; + sameSite: string; + domain?: string; + }; + }; + }; + expect(config.advanced.defaultCookieAttributes.domain).toBeUndefined(); }); }); }); diff --git a/apps/api/src/auth/auth.config.ts b/apps/api/src/auth/auth.config.ts index c3d0f78..afaf19e 100644 --- a/apps/api/src/auth/auth.config.ts +++ b/apps/api/src/auth/auth.config.ts @@ -6,7 +6,12 @@ import type { PrismaClient } from "@prisma/client"; /** * Required OIDC environment variables when OIDC is enabled */ -const REQUIRED_OIDC_ENV_VARS = ["OIDC_ISSUER", "OIDC_CLIENT_ID", "OIDC_CLIENT_SECRET"] as const; +const REQUIRED_OIDC_ENV_VARS = [ + "OIDC_ISSUER", + "OIDC_CLIENT_ID", + "OIDC_CLIENT_SECRET", + "OIDC_REDIRECT_URI", +] as const; /** * Check if OIDC authentication is enabled via environment variable @@ -52,6 +57,54 @@ export function validateOidcConfig(): void { `The discovery URL is constructed by appending ".well-known/openid-configuration" to the issuer.` ); } + + // Additional validation: OIDC_REDIRECT_URI must be a valid URL with /auth/callback path + validateRedirectUri(); +} + +/** + * Validates the OIDC_REDIRECT_URI environment variable. + * - Must be a parseable URL + * - Path must start with /auth/callback + * - Warns (but does not throw) if using localhost in production + * + * @throws Error if URL is invalid or path does not start with /auth/callback + */ +function validateRedirectUri(): void { + const redirectUri = process.env.OIDC_REDIRECT_URI; + if (!redirectUri || redirectUri.trim() === "") { + // Already caught by REQUIRED_OIDC_ENV_VARS check above + return; + } + + let parsed: URL; + try { + parsed = new URL(redirectUri); + } catch (urlError: unknown) { + const detail = urlError instanceof Error ? urlError.message : String(urlError); + throw new Error( + `OIDC_REDIRECT_URI must be a valid URL. Current value: "${redirectUri}". ` + + `Parse error: ${detail}. ` + + `Example: "https://app.example.com/auth/callback/authentik".` + ); + } + + if (!parsed.pathname.startsWith("/auth/callback")) { + throw new Error( + `OIDC_REDIRECT_URI path must start with "/auth/callback". Current path: "${parsed.pathname}". ` + + `Example: "https://app.example.com/auth/callback/authentik".` + ); + } + + if ( + process.env.NODE_ENV === "production" && + (parsed.hostname === "localhost" || parsed.hostname === "127.0.0.1") + ) { + console.warn( + `[AUTH WARNING] OIDC_REDIRECT_URI uses localhost ("${redirectUri}") in production. ` + + `This is likely a misconfiguration. Use a public domain for production deployments.` + ); + } } /** @@ -63,14 +116,29 @@ function getOidcPlugins(): ReturnType[] { return []; } + const clientId = process.env.OIDC_CLIENT_ID; + const clientSecret = process.env.OIDC_CLIENT_SECRET; + const issuer = process.env.OIDC_ISSUER; + + if (!clientId) { + throw new Error("OIDC_CLIENT_ID is required when OIDC is enabled but was not set."); + } + if (!clientSecret) { + throw new Error("OIDC_CLIENT_SECRET is required when OIDC is enabled but was not set."); + } + if (!issuer) { + throw new Error("OIDC_ISSUER is required when OIDC is enabled but was not set."); + } + return [ genericOAuth({ config: [ { providerId: "authentik", - clientId: process.env.OIDC_CLIENT_ID ?? "", - clientSecret: process.env.OIDC_CLIENT_SECRET ?? "", - discoveryUrl: `${process.env.OIDC_ISSUER ?? ""}.well-known/openid-configuration`, + clientId, + clientSecret, + discoveryUrl: `${issuer}.well-known/openid-configuration`, + pkce: true, scopes: ["openid", "profile", "email"], }, ], @@ -78,6 +146,58 @@ function getOidcPlugins(): ReturnType[] { ]; } +/** + * Build the list of trusted origins from environment variables. + * + * Sources (in order): + * - NEXT_PUBLIC_APP_URL — primary frontend URL + * - NEXT_PUBLIC_API_URL — API's own origin + * - TRUSTED_ORIGINS — comma-separated additional origins + * - localhost fallbacks — only when NODE_ENV !== "production" + * + * The returned list is deduplicated and empty strings are filtered out. + */ +export function getTrustedOrigins(): string[] { + const origins: string[] = []; + + // Environment-driven origins + if (process.env.NEXT_PUBLIC_APP_URL) { + origins.push(process.env.NEXT_PUBLIC_APP_URL); + } + + if (process.env.NEXT_PUBLIC_API_URL) { + origins.push(process.env.NEXT_PUBLIC_API_URL); + } + + // Comma-separated additional origins (validated) + if (process.env.TRUSTED_ORIGINS) { + const rawOrigins = process.env.TRUSTED_ORIGINS.split(",") + .map((o) => o.trim()) + .filter((o) => o !== ""); + for (const origin of rawOrigins) { + try { + const parsed = new URL(origin); + if (parsed.protocol !== "http:" && parsed.protocol !== "https:") { + console.warn(`[AUTH] Ignoring non-HTTP origin in TRUSTED_ORIGINS: "${origin}"`); + continue; + } + origins.push(origin); + } catch (urlError: unknown) { + const detail = urlError instanceof Error ? urlError.message : String(urlError); + console.warn(`[AUTH] Ignoring invalid URL in TRUSTED_ORIGINS: "${origin}" (${detail})`); + } + } + } + + // Localhost fallbacks for development only + if (process.env.NODE_ENV !== "production") { + origins.push("http://localhost:3000", "http://localhost:3001"); + } + + // Deduplicate and filter empty strings + return [...new Set(origins)].filter((o) => o !== ""); +} + export function createAuth(prisma: PrismaClient) { // Validate OIDC configuration at startup - fail fast if misconfigured validateOidcConfig(); @@ -88,19 +208,22 @@ export function createAuth(prisma: PrismaClient) { provider: "postgresql", }), emailAndPassword: { - enabled: true, // Enable for now, can be disabled later + enabled: true, }, plugins: [...getOidcPlugins()], session: { - expiresIn: 60 * 60 * 24, // 24 hours - updateAge: 60 * 60 * 24, // 24 hours + expiresIn: 60 * 60 * 24 * 7, // 7 days absolute max + updateAge: 60 * 60 * 2, // 2 hours — minimum session age before BetterAuth refreshes the expiry on next request }, - trustedOrigins: [ - process.env.NEXT_PUBLIC_APP_URL ?? "http://localhost:3000", - "http://localhost:3001", // API origin (dev) - "https://app.mosaicstack.dev", // Production web - "https://api.mosaicstack.dev", // Production API - ], + advanced: { + defaultCookieAttributes: { + httpOnly: true, + secure: process.env.NODE_ENV === "production", + sameSite: "lax" as const, + ...(process.env.COOKIE_DOMAIN ? { domain: process.env.COOKIE_DOMAIN } : {}), + }, + }, + trustedOrigins: getTrustedOrigins(), }); } diff --git a/apps/api/src/auth/auth.controller.spec.ts b/apps/api/src/auth/auth.controller.spec.ts index 4ce8095..2bec348 100644 --- a/apps/api/src/auth/auth.controller.spec.ts +++ b/apps/api/src/auth/auth.controller.spec.ts @@ -1,5 +1,27 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; + +// Mock better-auth modules before importing AuthService (pulled in by AuthController) +vi.mock("better-auth/node", () => ({ + toNodeHandler: vi.fn().mockReturnValue(vi.fn()), +})); + +vi.mock("better-auth", () => ({ + betterAuth: vi.fn().mockReturnValue({ + handler: vi.fn(), + api: { getSession: vi.fn() }, + }), +})); + +vi.mock("better-auth/adapters/prisma", () => ({ + prismaAdapter: vi.fn().mockReturnValue({}), +})); + +vi.mock("better-auth/plugins", () => ({ + genericOAuth: vi.fn().mockReturnValue({ id: "generic-oauth" }), +})); + import { Test, TestingModule } from "@nestjs/testing"; +import { HttpException, HttpStatus, UnauthorizedException } from "@nestjs/common"; import type { AuthUser, AuthSession } from "@mosaic/shared"; import type { Request as ExpressRequest, Response as ExpressResponse } from "express"; import { AuthController } from "./auth.controller"; @@ -13,6 +35,7 @@ describe("AuthController", () => { const mockAuthService = { getAuth: vi.fn(), getNodeHandler: vi.fn().mockReturnValue(mockNodeHandler), + getAuthConfig: vi.fn(), }; beforeEach(async () => { @@ -45,13 +68,189 @@ describe("AuthController", () => { socket: { remoteAddress: "127.0.0.1" }, } as unknown as ExpressRequest; - const mockResponse = {} as unknown as ExpressResponse; + const mockResponse = { + headersSent: false, + } as unknown as ExpressResponse; await controller.handleAuth(mockRequest, mockResponse); expect(mockAuthService.getNodeHandler).toHaveBeenCalled(); expect(mockNodeHandler).toHaveBeenCalledWith(mockRequest, mockResponse); }); + + it("should throw HttpException with 500 when handler throws before headers sent", async () => { + const handlerError = new Error("BetterAuth internal failure"); + mockNodeHandler.mockRejectedValueOnce(handlerError); + + const mockRequest = { + method: "POST", + url: "/auth/sign-in", + headers: {}, + ip: "192.168.1.10", + socket: { remoteAddress: "192.168.1.10" }, + } as unknown as ExpressRequest; + + const mockResponse = { + headersSent: false, + } as unknown as ExpressResponse; + + try { + await controller.handleAuth(mockRequest, mockResponse); + // Should not reach here + expect.unreachable("Expected HttpException to be thrown"); + } catch (err) { + expect(err).toBeInstanceOf(HttpException); + expect((err as HttpException).getStatus()).toBe(HttpStatus.INTERNAL_SERVER_ERROR); + expect((err as HttpException).getResponse()).toBe( + "Unable to complete authentication. Please try again in a moment.", + ); + } + }); + + it("should log warning and not throw when handler throws after headers sent", async () => { + const handlerError = new Error("Stream interrupted"); + mockNodeHandler.mockRejectedValueOnce(handlerError); + + const mockRequest = { + method: "POST", + url: "/auth/sign-up", + headers: {}, + ip: "10.0.0.5", + socket: { remoteAddress: "10.0.0.5" }, + } as unknown as ExpressRequest; + + const mockResponse = { + headersSent: true, + } as unknown as ExpressResponse; + + // Should not throw when headers already sent + await expect(controller.handleAuth(mockRequest, mockResponse)).resolves.toBeUndefined(); + }); + + it("should handle non-Error thrown values", async () => { + mockNodeHandler.mockRejectedValueOnce("string error"); + + const mockRequest = { + method: "GET", + url: "/auth/callback", + headers: {}, + ip: "127.0.0.1", + socket: { remoteAddress: "127.0.0.1" }, + } as unknown as ExpressRequest; + + const mockResponse = { + headersSent: false, + } as unknown as ExpressResponse; + + await expect(controller.handleAuth(mockRequest, mockResponse)).rejects.toThrow( + HttpException, + ); + }); + }); + + describe("getConfig", () => { + it("should return auth config from service", async () => { + const mockConfig = { + providers: [ + { id: "email", name: "Email", type: "credentials" as const }, + { id: "authentik", name: "Authentik", type: "oauth" as const }, + ], + }; + mockAuthService.getAuthConfig.mockResolvedValue(mockConfig); + + const result = await controller.getConfig(); + + expect(result).toEqual(mockConfig); + expect(mockAuthService.getAuthConfig).toHaveBeenCalled(); + }); + + it("should return correct response shape with only email provider", async () => { + const mockConfig = { + providers: [{ id: "email", name: "Email", type: "credentials" as const }], + }; + mockAuthService.getAuthConfig.mockResolvedValue(mockConfig); + + const result = await controller.getConfig(); + + expect(result).toEqual(mockConfig); + expect(result.providers).toHaveLength(1); + expect(result.providers[0]).toEqual({ + id: "email", + name: "Email", + type: "credentials", + }); + }); + + it("should never leak secrets in auth config response", async () => { + // Set ALL sensitive environment variables with known values + const sensitiveEnv: Record = { + OIDC_CLIENT_SECRET: "test-client-secret", + OIDC_CLIENT_ID: "test-client-id", + OIDC_ISSUER: "https://auth.test.com/", + OIDC_REDIRECT_URI: "https://app.test.com/auth/callback/authentik", + BETTER_AUTH_SECRET: "test-better-auth-secret", + JWT_SECRET: "test-jwt-secret", + CSRF_SECRET: "test-csrf-secret", + DATABASE_URL: "postgresql://user:password@localhost/db", + OIDC_ENABLED: "true", + }; + + const originalEnv: Record = {}; + for (const [key, value] of Object.entries(sensitiveEnv)) { + originalEnv[key] = process.env[key]; + process.env[key] = value; + } + + try { + // Mock the service to return a realistic config with both providers + const mockConfig = { + providers: [ + { id: "email", name: "Email", type: "credentials" as const }, + { id: "authentik", name: "Authentik", type: "oauth" as const }, + ], + }; + mockAuthService.getAuthConfig.mockResolvedValue(mockConfig); + + const result = await controller.getConfig(); + const serialized = JSON.stringify(result); + + // Assert no secret values leak into the serialized response + const forbiddenPatterns = [ + "test-client-secret", + "test-client-id", + "test-better-auth-secret", + "test-jwt-secret", + "test-csrf-secret", + "auth.test.com", + "callback", + "password", + ]; + + for (const pattern of forbiddenPatterns) { + expect(serialized).not.toContain(pattern); + } + + // Assert response contains ONLY expected fields + expect(result).toHaveProperty("providers"); + expect(Object.keys(result)).toEqual(["providers"]); + expect(Array.isArray(result.providers)).toBe(true); + + for (const provider of result.providers) { + const keys = Object.keys(provider); + expect(keys).toEqual(expect.arrayContaining(["id", "name", "type"])); + expect(keys).toHaveLength(3); + } + } finally { + // Restore original environment + for (const [key] of Object.entries(sensitiveEnv)) { + if (originalEnv[key] === undefined) { + delete process.env[key]; + } else { + process.env[key] = originalEnv[key]; + } + } + } + }); }); describe("getSession", () => { @@ -88,19 +287,24 @@ describe("AuthController", () => { expect(result).toEqual(expected); }); - it("should throw error if user not found in request", () => { + it("should throw UnauthorizedException when req.user is undefined", () => { const mockRequest = { session: { id: "session-123", token: "session-token", - expiresAt: new Date(), + expiresAt: new Date(Date.now() + 86400000), }, }; - expect(() => controller.getSession(mockRequest)).toThrow("User session not found"); + expect(() => controller.getSession(mockRequest as never)).toThrow( + UnauthorizedException, + ); + expect(() => controller.getSession(mockRequest as never)).toThrow( + "Missing authentication context", + ); }); - it("should throw error if session not found in request", () => { + it("should throw UnauthorizedException when req.session is undefined", () => { const mockRequest = { user: { id: "user-123", @@ -109,7 +313,23 @@ describe("AuthController", () => { }, }; - expect(() => controller.getSession(mockRequest)).toThrow("User session not found"); + expect(() => controller.getSession(mockRequest as never)).toThrow( + UnauthorizedException, + ); + expect(() => controller.getSession(mockRequest as never)).toThrow( + "Missing authentication context", + ); + }); + + it("should throw UnauthorizedException when both req.user and req.session are undefined", () => { + const mockRequest = {}; + + expect(() => controller.getSession(mockRequest as never)).toThrow( + UnauthorizedException, + ); + expect(() => controller.getSession(mockRequest as never)).toThrow( + "Missing authentication context", + ); }); }); @@ -161,4 +381,99 @@ describe("AuthController", () => { }); }); }); + + describe("getClientIp (via handleAuth)", () => { + it("should extract IP from X-Forwarded-For with single IP", async () => { + const mockRequest = { + method: "GET", + url: "/auth/callback", + headers: { "x-forwarded-for": "203.0.113.50" }, + ip: "127.0.0.1", + socket: { remoteAddress: "127.0.0.1" }, + } as unknown as ExpressRequest; + + const mockResponse = { + headersSent: false, + } as unknown as ExpressResponse; + + // Spy on the logger to verify the extracted IP + const debugSpy = vi.spyOn(controller["logger"], "debug"); + + await controller.handleAuth(mockRequest, mockResponse); + + expect(debugSpy).toHaveBeenCalledWith( + expect.stringContaining("203.0.113.50"), + ); + }); + + it("should extract first IP from X-Forwarded-For with comma-separated IPs", async () => { + const mockRequest = { + method: "GET", + url: "/auth/callback", + headers: { "x-forwarded-for": "203.0.113.50, 70.41.3.18" }, + ip: "127.0.0.1", + socket: { remoteAddress: "127.0.0.1" }, + } as unknown as ExpressRequest; + + const mockResponse = { + headersSent: false, + } as unknown as ExpressResponse; + + const debugSpy = vi.spyOn(controller["logger"], "debug"); + + await controller.handleAuth(mockRequest, mockResponse); + + expect(debugSpy).toHaveBeenCalledWith( + expect.stringContaining("203.0.113.50"), + ); + // Ensure it does NOT contain the second IP in the extracted position + expect(debugSpy).toHaveBeenCalledWith( + expect.not.stringContaining("70.41.3.18"), + ); + }); + + it("should extract first IP from X-Forwarded-For as array", async () => { + const mockRequest = { + method: "GET", + url: "/auth/callback", + headers: { "x-forwarded-for": ["203.0.113.50", "70.41.3.18"] }, + ip: "127.0.0.1", + socket: { remoteAddress: "127.0.0.1" }, + } as unknown as ExpressRequest; + + const mockResponse = { + headersSent: false, + } as unknown as ExpressResponse; + + const debugSpy = vi.spyOn(controller["logger"], "debug"); + + await controller.handleAuth(mockRequest, mockResponse); + + expect(debugSpy).toHaveBeenCalledWith( + expect.stringContaining("203.0.113.50"), + ); + }); + + it("should fallback to req.ip when no X-Forwarded-For header", async () => { + const mockRequest = { + method: "GET", + url: "/auth/callback", + headers: {}, + ip: "192.168.1.100", + socket: { remoteAddress: "192.168.1.100" }, + } as unknown as ExpressRequest; + + const mockResponse = { + headersSent: false, + } as unknown as ExpressResponse; + + const debugSpy = vi.spyOn(controller["logger"], "debug"); + + await controller.handleAuth(mockRequest, mockResponse); + + expect(debugSpy).toHaveBeenCalledWith( + expect.stringContaining("192.168.1.100"), + ); + }); + }); }); diff --git a/apps/api/src/auth/auth.controller.ts b/apps/api/src/auth/auth.controller.ts index c3f98b6..0152b81 100644 --- a/apps/api/src/auth/auth.controller.ts +++ b/apps/api/src/auth/auth.controller.ts @@ -1,21 +1,25 @@ -import { Controller, All, Req, Res, Get, UseGuards, Request, Logger } from "@nestjs/common"; +import { + Controller, + All, + Req, + Res, + Get, + Header, + UseGuards, + Request, + Logger, + HttpException, + HttpStatus, + UnauthorizedException, +} from "@nestjs/common"; import { Throttle } from "@nestjs/throttler"; import type { Request as ExpressRequest, Response as ExpressResponse } from "express"; -import type { AuthUser, AuthSession } from "@mosaic/shared"; +import type { AuthUser, AuthSession, AuthConfigResponse } from "@mosaic/shared"; import { AuthService } from "./auth.service"; import { AuthGuard } from "./guards/auth.guard"; import { CurrentUser } from "./decorators/current-user.decorator"; import { SkipCsrf } from "../common/decorators/skip-csrf.decorator"; - -interface RequestWithSession { - user?: AuthUser; - session?: { - id: string; - token: string; - expiresAt: Date; - [key: string]: unknown; - }; -} +import type { AuthenticatedRequest } from "./types/better-auth-request.interface"; @Controller("auth") export class AuthController { @@ -29,10 +33,13 @@ export class AuthController { */ @Get("session") @UseGuards(AuthGuard) - getSession(@Request() req: RequestWithSession): AuthSession { + getSession(@Request() req: AuthenticatedRequest): AuthSession { + // Defense-in-depth: AuthGuard should guarantee these, but if someone adds + // a route with AuthenticatedRequest and forgets @UseGuards(AuthGuard), + // TypeScript types won't help at runtime. + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition if (!req.user || !req.session) { - // This should never happen after AuthGuard, but TypeScript needs the check - throw new Error("User session not found"); + throw new UnauthorizedException("Missing authentication context"); } return { @@ -78,6 +85,17 @@ export class AuthController { return profile; } + /** + * Get available authentication providers. + * Public endpoint (no auth guard) so the frontend can discover login options + * before the user is authenticated. + */ + @Get("config") + @Header("Cache-Control", "public, max-age=300") + async getConfig(): Promise { + return this.authService.getAuthConfig(); + } + /** * Handle all other auth routes (sign-in, sign-up, sign-out, etc.) * Delegates to BetterAuth @@ -89,6 +107,9 @@ export class AuthController { * Rate limiting and logging are applied to mitigate abuse (SEC-API-10). */ @All("*") + // BetterAuth handles CSRF internally (Fetch Metadata + SameSite=Lax cookies). + // @SkipCsrf avoids double-protection conflicts. + // See: https://www.better-auth.com/docs/reference/security @SkipCsrf() @Throttle({ strict: { limit: 10, ttl: 60000 } }) async handleAuth(@Req() req: ExpressRequest, @Res() res: ExpressResponse): Promise { @@ -99,7 +120,29 @@ export class AuthController { this.logger.debug(`Auth catch-all: ${req.method} ${req.url} from ${clientIp}`); const handler = this.authService.getNodeHandler(); - return handler(req, res); + + try { + await handler(req, res); + } catch (error: unknown) { + const message = error instanceof Error ? error.message : String(error); + const stack = error instanceof Error ? error.stack : undefined; + + this.logger.error( + `BetterAuth handler error: ${req.method} ${req.url} from ${clientIp} - ${message}`, + stack + ); + + if (!res.headersSent) { + throw new HttpException( + "Unable to complete authentication. Please try again in a moment.", + HttpStatus.INTERNAL_SERVER_ERROR + ); + } + + this.logger.error( + `Headers already sent for failed auth request ${req.method} ${req.url} — client may have received partial response` + ); + } } /** diff --git a/apps/api/src/auth/auth.service.spec.ts b/apps/api/src/auth/auth.service.spec.ts index e0f0a81..5cc01b9 100644 --- a/apps/api/src/auth/auth.service.spec.ts +++ b/apps/api/src/auth/auth.service.spec.ts @@ -1,5 +1,26 @@ -import { describe, it, expect, beforeEach, vi } from "vitest"; +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import { Test, TestingModule } from "@nestjs/testing"; + +// Mock better-auth modules before importing AuthService +vi.mock("better-auth/node", () => ({ + toNodeHandler: vi.fn().mockReturnValue(vi.fn()), +})); + +vi.mock("better-auth", () => ({ + betterAuth: vi.fn().mockReturnValue({ + handler: vi.fn(), + api: { getSession: vi.fn() }, + }), +})); + +vi.mock("better-auth/adapters/prisma", () => ({ + prismaAdapter: vi.fn().mockReturnValue({}), +})); + +vi.mock("better-auth/plugins", () => ({ + genericOAuth: vi.fn().mockReturnValue({ id: "generic-oauth" }), +})); + import { AuthService } from "./auth.service"; import { PrismaService } from "../prisma/prisma.service"; @@ -30,6 +51,12 @@ describe("AuthService", () => { vi.clearAllMocks(); }); + afterEach(() => { + vi.restoreAllMocks(); + delete process.env.OIDC_ENABLED; + delete process.env.OIDC_ISSUER; + }); + describe("getAuth", () => { it("should return BetterAuth instance", () => { const auth = service.getAuth(); @@ -62,6 +89,23 @@ describe("AuthService", () => { }, }); }); + + it("should return null when user is not found", async () => { + mockPrismaService.user.findUnique.mockResolvedValue(null); + + const result = await service.getUserById("nonexistent-id"); + + expect(result).toBeNull(); + expect(mockPrismaService.user.findUnique).toHaveBeenCalledWith({ + where: { id: "nonexistent-id" }, + select: { + id: true, + email: true, + name: true, + authProviderId: true, + }, + }); + }); }); describe("getUserByEmail", () => { @@ -88,6 +132,269 @@ describe("AuthService", () => { }, }); }); + + it("should return null when user is not found", async () => { + mockPrismaService.user.findUnique.mockResolvedValue(null); + + const result = await service.getUserByEmail("unknown@example.com"); + + expect(result).toBeNull(); + expect(mockPrismaService.user.findUnique).toHaveBeenCalledWith({ + where: { email: "unknown@example.com" }, + select: { + id: true, + email: true, + name: true, + authProviderId: true, + }, + }); + }); + }); + + describe("isOidcProviderReachable", () => { + const discoveryUrl = "https://auth.example.com/.well-known/openid-configuration"; + + beforeEach(() => { + process.env.OIDC_ISSUER = "https://auth.example.com/"; + // Reset the cache by accessing private fields via bracket notation + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (service as any).lastHealthCheck = 0; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (service as any).lastHealthResult = false; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (service as any).consecutiveHealthFailures = 0; + }); + + it("should return true when discovery URL returns 200", async () => { + const mockFetch = vi.fn().mockResolvedValue({ + ok: true, + status: 200, + }); + vi.stubGlobal("fetch", mockFetch); + + const result = await service.isOidcProviderReachable(); + + expect(result).toBe(true); + expect(mockFetch).toHaveBeenCalledWith(discoveryUrl, { + signal: expect.any(AbortSignal) as AbortSignal, + }); + }); + + it("should return false on network error", async () => { + const mockFetch = vi.fn().mockRejectedValue(new Error("ECONNREFUSED")); + vi.stubGlobal("fetch", mockFetch); + + const result = await service.isOidcProviderReachable(); + + expect(result).toBe(false); + }); + + it("should return false on timeout", async () => { + const mockFetch = vi.fn().mockRejectedValue(new DOMException("The operation was aborted")); + vi.stubGlobal("fetch", mockFetch); + + const result = await service.isOidcProviderReachable(); + + expect(result).toBe(false); + }); + + it("should return false when discovery URL returns non-200", async () => { + const mockFetch = vi.fn().mockResolvedValue({ + ok: false, + status: 503, + }); + vi.stubGlobal("fetch", mockFetch); + + const result = await service.isOidcProviderReachable(); + + expect(result).toBe(false); + }); + + it("should cache result for 30 seconds", async () => { + const mockFetch = vi.fn().mockResolvedValue({ + ok: true, + status: 200, + }); + vi.stubGlobal("fetch", mockFetch); + + // First call - fetches + const result1 = await service.isOidcProviderReachable(); + expect(result1).toBe(true); + expect(mockFetch).toHaveBeenCalledTimes(1); + + // Second call within 30s - uses cache + const result2 = await service.isOidcProviderReachable(); + expect(result2).toBe(true); + expect(mockFetch).toHaveBeenCalledTimes(1); // Still 1, no new fetch + + // Simulate cache expiry by moving lastHealthCheck back + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (service as any).lastHealthCheck = Date.now() - 31_000; + + // Third call after cache expiry - fetches again + const result3 = await service.isOidcProviderReachable(); + expect(result3).toBe(true); + expect(mockFetch).toHaveBeenCalledTimes(2); // Now 2 + }); + + it("should cache false results too", async () => { + const mockFetch = vi + .fn() + .mockRejectedValueOnce(new Error("ECONNREFUSED")) + .mockResolvedValueOnce({ ok: true, status: 200 }); + vi.stubGlobal("fetch", mockFetch); + + // First call - fails + const result1 = await service.isOidcProviderReachable(); + expect(result1).toBe(false); + expect(mockFetch).toHaveBeenCalledTimes(1); + + // Second call within 30s - returns cached false + const result2 = await service.isOidcProviderReachable(); + expect(result2).toBe(false); + expect(mockFetch).toHaveBeenCalledTimes(1); + }); + + it("should escalate to error level after 3 consecutive failures", async () => { + const mockFetch = vi.fn().mockRejectedValue(new Error("ECONNREFUSED")); + vi.stubGlobal("fetch", mockFetch); + + const loggerWarn = vi.spyOn(service["logger"], "warn"); + const loggerError = vi.spyOn(service["logger"], "error"); + + // Failures 1 and 2 should log at warn level + await service.isOidcProviderReachable(); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (service as any).lastHealthCheck = 0; // Reset cache + await service.isOidcProviderReachable(); + + expect(loggerWarn).toHaveBeenCalledTimes(2); + expect(loggerError).not.toHaveBeenCalled(); + + // Failure 3 should escalate to error level + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (service as any).lastHealthCheck = 0; + await service.isOidcProviderReachable(); + + expect(loggerError).toHaveBeenCalledTimes(1); + expect(loggerError).toHaveBeenCalledWith( + expect.stringContaining("OIDC provider unreachable") + ); + }); + + it("should escalate to error level after 3 consecutive non-OK responses", async () => { + const mockFetch = vi.fn().mockResolvedValue({ ok: false, status: 503 }); + vi.stubGlobal("fetch", mockFetch); + + const loggerWarn = vi.spyOn(service["logger"], "warn"); + const loggerError = vi.spyOn(service["logger"], "error"); + + // Failures 1 and 2 at warn level + await service.isOidcProviderReachable(); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (service as any).lastHealthCheck = 0; + await service.isOidcProviderReachable(); + + expect(loggerWarn).toHaveBeenCalledTimes(2); + expect(loggerError).not.toHaveBeenCalled(); + + // Failure 3 at error level + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (service as any).lastHealthCheck = 0; + await service.isOidcProviderReachable(); + + expect(loggerError).toHaveBeenCalledTimes(1); + expect(loggerError).toHaveBeenCalledWith( + expect.stringContaining("OIDC provider returned non-OK status") + ); + }); + + it("should reset failure counter and log recovery on success after failures", async () => { + const mockFetch = vi + .fn() + .mockRejectedValueOnce(new Error("ECONNREFUSED")) + .mockRejectedValueOnce(new Error("ECONNREFUSED")) + .mockResolvedValueOnce({ ok: true, status: 200 }); + vi.stubGlobal("fetch", mockFetch); + + const loggerLog = vi.spyOn(service["logger"], "log"); + + // Two failures + await service.isOidcProviderReachable(); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (service as any).lastHealthCheck = 0; + await service.isOidcProviderReachable(); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (service as any).lastHealthCheck = 0; + + // Recovery + const result = await service.isOidcProviderReachable(); + + expect(result).toBe(true); + expect(loggerLog).toHaveBeenCalledWith( + expect.stringContaining("OIDC provider recovered after 2 consecutive failure(s)") + ); + // Verify counter reset + // eslint-disable-next-line @typescript-eslint/no-explicit-any + expect((service as any).consecutiveHealthFailures).toBe(0); + }); + }); + + describe("getAuthConfig", () => { + it("should return only email provider when OIDC is disabled", async () => { + delete process.env.OIDC_ENABLED; + + const result = await service.getAuthConfig(); + + expect(result).toEqual({ + providers: [{ id: "email", name: "Email", type: "credentials" }], + }); + }); + + it("should return both email and authentik providers when OIDC is enabled and reachable", async () => { + process.env.OIDC_ENABLED = "true"; + process.env.OIDC_ISSUER = "https://auth.example.com/"; + + const mockFetch = vi.fn().mockResolvedValue({ ok: true, status: 200 }); + vi.stubGlobal("fetch", mockFetch); + + const result = await service.getAuthConfig(); + + expect(result).toEqual({ + providers: [ + { id: "email", name: "Email", type: "credentials" }, + { id: "authentik", name: "Authentik", type: "oauth" }, + ], + }); + }); + + it("should return only email provider when OIDC_ENABLED is false", async () => { + process.env.OIDC_ENABLED = "false"; + + const result = await service.getAuthConfig(); + + expect(result).toEqual({ + providers: [{ id: "email", name: "Email", type: "credentials" }], + }); + }); + + it("should omit authentik when OIDC is enabled but provider is unreachable", async () => { + process.env.OIDC_ENABLED = "true"; + process.env.OIDC_ISSUER = "https://auth.example.com/"; + + // Reset cache + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (service as any).lastHealthCheck = 0; + + const mockFetch = vi.fn().mockRejectedValue(new Error("ECONNREFUSED")); + vi.stubGlobal("fetch", mockFetch); + + const result = await service.getAuthConfig(); + + expect(result).toEqual({ + providers: [{ id: "email", name: "Email", type: "credentials" }], + }); + }); }); describe("verifySession", () => { @@ -128,14 +435,268 @@ describe("AuthService", () => { expect(result).toBeNull(); }); - it("should return null and log error on verification failure", async () => { + it("should return null for 'invalid token' auth error", async () => { + const auth = service.getAuth(); + const mockGetSession = vi.fn().mockRejectedValue(new Error("Invalid token provided")); + auth.api = { getSession: mockGetSession } as any; + + const result = await service.verifySession("bad-token"); + + expect(result).toBeNull(); + }); + + it("should return null for 'expired' auth error", async () => { + const auth = service.getAuth(); + const mockGetSession = vi.fn().mockRejectedValue(new Error("Token expired")); + auth.api = { getSession: mockGetSession } as any; + + const result = await service.verifySession("expired-token"); + + expect(result).toBeNull(); + }); + + it("should return null for 'session not found' auth error", async () => { + const auth = service.getAuth(); + const mockGetSession = vi.fn().mockRejectedValue(new Error("Session not found")); + auth.api = { getSession: mockGetSession } as any; + + const result = await service.verifySession("missing-session"); + + expect(result).toBeNull(); + }); + + it("should return null for 'unauthorized' auth error", async () => { + const auth = service.getAuth(); + const mockGetSession = vi.fn().mockRejectedValue(new Error("Unauthorized")); + auth.api = { getSession: mockGetSession } as any; + + const result = await service.verifySession("unauth-token"); + + expect(result).toBeNull(); + }); + + it("should return null for 'invalid session' auth error", async () => { + const auth = service.getAuth(); + const mockGetSession = vi.fn().mockRejectedValue(new Error("Invalid session")); + auth.api = { getSession: mockGetSession } as any; + + const result = await service.verifySession("invalid-session"); + + expect(result).toBeNull(); + }); + + it("should return null for 'session expired' auth error", async () => { + const auth = service.getAuth(); + const mockGetSession = vi.fn().mockRejectedValue(new Error("Session expired")); + auth.api = { getSession: mockGetSession } as any; + + const result = await service.verifySession("expired-session"); + + expect(result).toBeNull(); + }); + + it("should return null for bare 'unauthorized' (exact match)", async () => { + const auth = service.getAuth(); + const mockGetSession = vi.fn().mockRejectedValue(new Error("unauthorized")); + auth.api = { getSession: mockGetSession } as any; + + const result = await service.verifySession("unauth-token"); + + expect(result).toBeNull(); + }); + + it("should return null for bare 'expired' (exact match)", async () => { + const auth = service.getAuth(); + const mockGetSession = vi.fn().mockRejectedValue(new Error("expired")); + auth.api = { getSession: mockGetSession } as any; + + const result = await service.verifySession("expired-token"); + + expect(result).toBeNull(); + }); + + 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")); + auth.api = { getSession: mockGetSession } as any; + + 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 () => { + const auth = service.getAuth(); + const mockGetSession = vi + .fn() + .mockRejectedValue(new Error("Unauthorized: Access denied for user")); + auth.api = { getSession: mockGetSession } as any; + + await expect(service.verifySession("any-token")).rejects.toThrow( + "Unauthorized: Access denied for user" + ); + }); + + it("should return null when a non-Error value is thrown", async () => { + const auth = service.getAuth(); + const mockGetSession = vi.fn().mockRejectedValue("string-error"); + auth.api = { getSession: mockGetSession } as any; + + const result = await service.verifySession("any-token"); + + expect(result).toBeNull(); + }); + + it("should return null when getSession throws a non-Error value (string)", async () => { + const auth = service.getAuth(); + const mockGetSession = vi.fn().mockRejectedValue("some error"); + auth.api = { getSession: mockGetSession } as any; + + const result = await service.verifySession("any-token"); + + expect(result).toBeNull(); + }); + + it("should return null when getSession throws a non-Error value (object)", async () => { + const auth = service.getAuth(); + const mockGetSession = vi.fn().mockRejectedValue({ code: "ERR_UNKNOWN" }); + auth.api = { getSession: mockGetSession } as any; + + const result = await service.verifySession("any-token"); + + expect(result).toBeNull(); + }); + + it("should re-throw unexpected errors that are not known auth errors", async () => { const auth = service.getAuth(); const mockGetSession = vi.fn().mockRejectedValue(new Error("Verification failed")); auth.api = { getSession: mockGetSession } as any; - const result = await service.verifySession("error-token"); + await expect(service.verifySession("error-token")).rejects.toThrow("Verification failed"); + }); + + it("should re-throw Prisma infrastructure errors", async () => { + const auth = service.getAuth(); + const prismaError = new Error("connect ECONNREFUSED 127.0.0.1:5432"); + const mockGetSession = vi.fn().mockRejectedValue(prismaError); + auth.api = { getSession: mockGetSession } as any; + + await expect(service.verifySession("any-token")).rejects.toThrow("ECONNREFUSED"); + }); + + it("should re-throw timeout errors as infrastructure errors", async () => { + const auth = service.getAuth(); + const timeoutError = new Error("Connection timeout after 5000ms"); + const mockGetSession = vi.fn().mockRejectedValue(timeoutError); + auth.api = { getSession: mockGetSession } as any; + + await expect(service.verifySession("any-token")).rejects.toThrow("timeout"); + }); + + it("should re-throw errors with Prisma-prefixed constructor name", async () => { + const auth = service.getAuth(); + class PrismaClientKnownRequestError extends Error { + constructor(message: string) { + super(message); + this.name = "PrismaClientKnownRequestError"; + } + } + const prismaError = new PrismaClientKnownRequestError("Database connection lost"); + const mockGetSession = vi.fn().mockRejectedValue(prismaError); + auth.api = { getSession: mockGetSession } as any; + + await expect(service.verifySession("any-token")).rejects.toThrow("Database connection lost"); + }); + + it("should redact Bearer tokens from logged error messages", async () => { + const auth = service.getAuth(); + const errorWithToken = new Error( + "Request failed: Bearer eyJhbGciOiJIUzI1NiJ9.secret-payload in header" + ); + const mockGetSession = vi.fn().mockRejectedValue(errorWithToken); + auth.api = { getSession: mockGetSession } as any; + + const loggerError = vi.spyOn(service["logger"], "error"); + + await expect(service.verifySession("any-token")).rejects.toThrow(); + + expect(loggerError).toHaveBeenCalledWith( + "Session verification failed due to unexpected error", + expect.stringContaining("Bearer [REDACTED]") + ); + expect(loggerError).toHaveBeenCalledWith( + "Session verification failed due to unexpected error", + expect.not.stringContaining("eyJhbGciOiJIUzI1NiJ9") + ); + }); + + it("should redact Bearer tokens from error stack traces", async () => { + const auth = service.getAuth(); + const errorWithToken = new Error("Something went wrong"); + errorWithToken.stack = + "Error: Something went wrong\n at fetch (Bearer abc123-secret-token)\n at verifySession"; + const mockGetSession = vi.fn().mockRejectedValue(errorWithToken); + auth.api = { getSession: mockGetSession } as any; + + const loggerError = vi.spyOn(service["logger"], "error"); + + await expect(service.verifySession("any-token")).rejects.toThrow(); + + expect(loggerError).toHaveBeenCalledWith( + "Session verification failed due to unexpected error", + expect.stringContaining("Bearer [REDACTED]") + ); + expect(loggerError).toHaveBeenCalledWith( + "Session verification failed due to unexpected error", + expect.not.stringContaining("abc123-secret-token") + ); + }); + + it("should warn when a non-Error string value is thrown", async () => { + const auth = service.getAuth(); + const mockGetSession = vi.fn().mockRejectedValue("string-error"); + auth.api = { getSession: mockGetSession } as any; + + const loggerWarn = vi.spyOn(service["logger"], "warn"); + + const result = await service.verifySession("any-token"); expect(result).toBeNull(); + expect(loggerWarn).toHaveBeenCalledWith( + "Session verification received non-Error thrown value", + "string-error" + ); + }); + + it("should warn with JSON when a non-Error object is thrown", async () => { + const auth = service.getAuth(); + const mockGetSession = vi.fn().mockRejectedValue({ code: "ERR_UNKNOWN" }); + auth.api = { getSession: mockGetSession } as any; + + const loggerWarn = vi.spyOn(service["logger"], "warn"); + + const result = await service.verifySession("any-token"); + + expect(result).toBeNull(); + expect(loggerWarn).toHaveBeenCalledWith( + "Session verification received non-Error thrown value", + JSON.stringify({ code: "ERR_UNKNOWN" }) + ); + }); + + it("should not warn for expected auth errors (Error instances)", async () => { + const auth = service.getAuth(); + const mockGetSession = vi.fn().mockRejectedValue(new Error("Invalid token provided")); + auth.api = { getSession: mockGetSession } as any; + + const loggerWarn = vi.spyOn(service["logger"], "warn"); + + const result = await service.verifySession("bad-token"); + + expect(result).toBeNull(); + expect(loggerWarn).not.toHaveBeenCalled(); }); }); }); diff --git a/apps/api/src/auth/auth.service.ts b/apps/api/src/auth/auth.service.ts index d97553f..97e8d4b 100644 --- a/apps/api/src/auth/auth.service.ts +++ b/apps/api/src/auth/auth.service.ts @@ -2,8 +2,24 @@ import { Injectable, Logger } from "@nestjs/common"; import type { PrismaClient } from "@prisma/client"; import type { IncomingMessage, ServerResponse } from "http"; import { toNodeHandler } from "better-auth/node"; +import type { AuthConfigResponse, AuthProviderConfig } from "@mosaic/shared"; import { PrismaService } from "../prisma/prisma.service"; -import { createAuth, type Auth } from "./auth.config"; +import { createAuth, isOidcEnabled, type Auth } from "./auth.config"; + +/** Duration in milliseconds to cache the OIDC health check result */ +const OIDC_HEALTH_CACHE_TTL_MS = 30_000; + +/** Timeout in milliseconds for the OIDC discovery URL fetch */ +const OIDC_HEALTH_TIMEOUT_MS = 2_000; + +/** Number of consecutive health-check failures before escalating to error level */ +const HEALTH_ESCALATION_THRESHOLD = 3; + +/** Verified session shape returned by BetterAuth's getSession */ +interface VerifiedSession { + user: Record; + session: Record; +} @Injectable() export class AuthService { @@ -11,9 +27,17 @@ export class AuthService { private readonly auth: Auth; private readonly nodeHandler: (req: IncomingMessage, res: ServerResponse) => Promise; + /** Timestamp of the last OIDC health check */ + private lastHealthCheck = 0; + /** Cached result of the last OIDC health check */ + private lastHealthResult = false; + /** Consecutive OIDC health check failure count for log-level escalation */ + private consecutiveHealthFailures = 0; + constructor(private readonly prisma: PrismaService) { // PrismaService extends PrismaClient and is compatible with BetterAuth's adapter // Cast is safe as PrismaService provides all required PrismaClient methods + // TODO(#411): BetterAuth returns opaque types — replace when upstream exports typed interfaces this.auth = createAuth(this.prisma as unknown as PrismaClient); this.nodeHandler = toNodeHandler(this.auth); } @@ -75,12 +99,12 @@ export class AuthService { /** * Verify session token - * Returns session data if valid, null if invalid or expired + * Returns session data if valid, null if invalid or expired. + * Only known-safe auth errors return null; everything else propagates as 500. */ - async verifySession( - token: string - ): Promise<{ user: Record; session: Record } | null> { + 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}`, @@ -95,12 +119,107 @@ export class AuthService { user: session.user as Record, session: session.session as Record, }; - } catch (error) { - this.logger.error( - "Session verification failed", - error instanceof Error ? error.message : "Unknown error" - ); + } 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 (!isExpectedAuthError) { + // Infrastructure or unexpected — propagate as 500 + const safeMessage = (error.stack ?? error.message).replace( + /Bearer\s+\S+/gi, + "Bearer [REDACTED]" + ); + this.logger.error("Session verification failed due to unexpected error", safeMessage); + throw error; + } + } + // 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; } } + + /** + * Check if the OIDC provider (Authentik) is reachable by fetching the discovery URL. + * Results are cached for 30 seconds to prevent repeated network calls. + * + * @returns true if the provider responds with an HTTP 2xx status, false otherwise + */ + async isOidcProviderReachable(): Promise { + const now = Date.now(); + + // Return cached result if still valid + if (now - this.lastHealthCheck < OIDC_HEALTH_CACHE_TTL_MS) { + this.logger.debug("OIDC health check: returning cached result"); + return this.lastHealthResult; + } + + const discoveryUrl = `${process.env.OIDC_ISSUER ?? ""}.well-known/openid-configuration`; + this.logger.debug(`OIDC health check: fetching ${discoveryUrl}`); + + try { + const response = await fetch(discoveryUrl, { + signal: AbortSignal.timeout(OIDC_HEALTH_TIMEOUT_MS), + }); + + this.lastHealthCheck = Date.now(); + this.lastHealthResult = response.ok; + + if (response.ok) { + if (this.consecutiveHealthFailures > 0) { + this.logger.log( + `OIDC provider recovered after ${String(this.consecutiveHealthFailures)} consecutive failure(s)` + ); + } + this.consecutiveHealthFailures = 0; + } else { + this.consecutiveHealthFailures++; + const logLevel = + this.consecutiveHealthFailures >= HEALTH_ESCALATION_THRESHOLD ? "error" : "warn"; + this.logger[logLevel]( + `OIDC provider returned non-OK status: ${String(response.status)} from ${discoveryUrl}` + ); + } + + return this.lastHealthResult; + } catch (error: unknown) { + this.lastHealthCheck = Date.now(); + this.lastHealthResult = false; + this.consecutiveHealthFailures++; + + const message = error instanceof Error ? error.message : String(error); + const logLevel = + this.consecutiveHealthFailures >= HEALTH_ESCALATION_THRESHOLD ? "error" : "warn"; + this.logger[logLevel](`OIDC provider unreachable at ${discoveryUrl}: ${message}`); + + return false; + } + } + + /** + * Get authentication configuration for the frontend. + * Returns available auth providers so the UI can render login options dynamically. + * When OIDC is enabled, performs a health check to verify the provider is reachable. + */ + async getAuthConfig(): Promise { + const providers: AuthProviderConfig[] = [{ id: "email", name: "Email", type: "credentials" }]; + + if (isOidcEnabled() && (await this.isOidcProviderReachable())) { + providers.push({ id: "authentik", name: "Authentik", type: "oauth" }); + } + + return { providers }; + } } diff --git a/apps/api/src/auth/decorators/current-user.decorator.ts b/apps/api/src/auth/decorators/current-user.decorator.ts index 0928d53..a322d79 100644 --- a/apps/api/src/auth/decorators/current-user.decorator.ts +++ b/apps/api/src/auth/decorators/current-user.decorator.ts @@ -1,14 +1,13 @@ import type { ExecutionContext } from "@nestjs/common"; import { createParamDecorator, UnauthorizedException } from "@nestjs/common"; import type { AuthUser } from "@mosaic/shared"; - -interface RequestWithUser { - user?: AuthUser; -} +import type { MaybeAuthenticatedRequest } from "../types/better-auth-request.interface"; export const CurrentUser = createParamDecorator( (_data: unknown, ctx: ExecutionContext): AuthUser => { - const request = ctx.switchToHttp().getRequest(); + // Use MaybeAuthenticatedRequest because the decorator doesn't know + // whether AuthGuard ran — the null check provides defense-in-depth. + const request = ctx.switchToHttp().getRequest(); if (!request.user) { throw new UnauthorizedException("No authenticated user found on request"); } diff --git a/apps/api/src/auth/guards/auth.guard.spec.ts b/apps/api/src/auth/guards/auth.guard.spec.ts index 557f647..fe1e8eb 100644 --- a/apps/api/src/auth/guards/auth.guard.spec.ts +++ b/apps/api/src/auth/guards/auth.guard.spec.ts @@ -1,30 +1,39 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; -import { Test, TestingModule } from "@nestjs/testing"; import { ExecutionContext, UnauthorizedException } from "@nestjs/common"; + +// Mock better-auth modules before importing AuthGuard (which imports AuthService) +vi.mock("better-auth/node", () => ({ + toNodeHandler: vi.fn().mockReturnValue(vi.fn()), +})); + +vi.mock("better-auth", () => ({ + betterAuth: vi.fn().mockReturnValue({ + handler: vi.fn(), + api: { getSession: vi.fn() }, + }), +})); + +vi.mock("better-auth/adapters/prisma", () => ({ + prismaAdapter: vi.fn().mockReturnValue({}), +})); + +vi.mock("better-auth/plugins", () => ({ + genericOAuth: vi.fn().mockReturnValue({ id: "generic-oauth" }), +})); + import { AuthGuard } from "./auth.guard"; -import { AuthService } from "../auth.service"; +import type { AuthService } from "../auth.service"; describe("AuthGuard", () => { let guard: AuthGuard; - let authService: AuthService; const mockAuthService = { verifySession: vi.fn(), }; - beforeEach(async () => { - const module: TestingModule = await Test.createTestingModule({ - providers: [ - AuthGuard, - { - provide: AuthService, - useValue: mockAuthService, - }, - ], - }).compile(); - - guard = module.get(AuthGuard); - authService = module.get(AuthService); + beforeEach(() => { + // Directly construct the guard with the mock to avoid NestJS DI issues + guard = new AuthGuard(mockAuthService as unknown as AuthService); vi.clearAllMocks(); }); @@ -147,17 +156,134 @@ describe("AuthGuard", () => { ); }); - it("should throw UnauthorizedException if session verification fails", async () => { - mockAuthService.verifySession.mockRejectedValue(new Error("Verification failed")); + it("should propagate non-auth errors as-is (not wrap as 401)", async () => { + const infraError = new Error("connect ECONNREFUSED 127.0.0.1:5432"); + mockAuthService.verifySession.mockRejectedValue(infraError); const context = createMockExecutionContext({ authorization: "Bearer error-token", }); - await expect(guard.canActivate(context)).rejects.toThrow(UnauthorizedException); - await expect(guard.canActivate(context)).rejects.toThrow("Authentication failed"); + await expect(guard.canActivate(context)).rejects.toThrow(infraError); + await expect(guard.canActivate(context)).rejects.not.toBeInstanceOf(UnauthorizedException); }); + it("should propagate database errors so GlobalExceptionFilter returns 500", async () => { + const dbError = new Error("PrismaClientKnownRequestError: Connection refused"); + mockAuthService.verifySession.mockRejectedValue(dbError); + + const context = createMockExecutionContext({ + authorization: "Bearer valid-token", + }); + + await expect(guard.canActivate(context)).rejects.toThrow(dbError); + await expect(guard.canActivate(context)).rejects.not.toBeInstanceOf(UnauthorizedException); + }); + + it("should propagate timeout errors so GlobalExceptionFilter returns 503", async () => { + const timeoutError = new Error("Connection timeout after 5000ms"); + mockAuthService.verifySession.mockRejectedValue(timeoutError); + + const context = createMockExecutionContext({ + authorization: "Bearer valid-token", + }); + + await expect(guard.canActivate(context)).rejects.toThrow(timeoutError); + await expect(guard.canActivate(context)).rejects.not.toBeInstanceOf(UnauthorizedException); + }); + }); + + describe("user data validation", () => { + const mockSession = { + id: "session-123", + token: "session-token", + expiresAt: new Date(Date.now() + 86400000), + }; + + it("should throw UnauthorizedException when user is missing id", async () => { + mockAuthService.verifySession.mockResolvedValue({ + user: { email: "a@b.com", name: "Test" }, + session: mockSession, + }); + + const context = createMockExecutionContext({ + authorization: "Bearer valid-token", + }); + + await expect(guard.canActivate(context)).rejects.toThrow(UnauthorizedException); + await expect(guard.canActivate(context)).rejects.toThrow( + "Invalid user data in session" + ); + }); + + it("should throw UnauthorizedException when user is missing email", async () => { + mockAuthService.verifySession.mockResolvedValue({ + user: { id: "1", name: "Test" }, + session: mockSession, + }); + + const context = createMockExecutionContext({ + authorization: "Bearer valid-token", + }); + + await expect(guard.canActivate(context)).rejects.toThrow(UnauthorizedException); + await expect(guard.canActivate(context)).rejects.toThrow( + "Invalid user data in session" + ); + }); + + it("should throw UnauthorizedException when user is missing name", async () => { + mockAuthService.verifySession.mockResolvedValue({ + user: { id: "1", email: "a@b.com" }, + session: mockSession, + }); + + const context = createMockExecutionContext({ + authorization: "Bearer valid-token", + }); + + await expect(guard.canActivate(context)).rejects.toThrow(UnauthorizedException); + await expect(guard.canActivate(context)).rejects.toThrow( + "Invalid user data in session" + ); + }); + + it("should throw UnauthorizedException when user is a string", async () => { + mockAuthService.verifySession.mockResolvedValue({ + user: "not-an-object", + session: mockSession, + }); + + const context = createMockExecutionContext({ + authorization: "Bearer valid-token", + }); + + await expect(guard.canActivate(context)).rejects.toThrow(UnauthorizedException); + await expect(guard.canActivate(context)).rejects.toThrow( + "Invalid user data in session" + ); + }); + + it("should reject when user is null (typeof null === 'object' causes TypeError on 'in' operator)", async () => { + // Note: typeof null === "object" in JS, so the guard's typeof check passes + // but "id" in null throws TypeError. The catch block propagates non-auth errors as-is. + mockAuthService.verifySession.mockResolvedValue({ + user: null, + session: mockSession, + }); + + const context = createMockExecutionContext({ + authorization: "Bearer valid-token", + }); + + await expect(guard.canActivate(context)).rejects.toThrow(TypeError); + await expect(guard.canActivate(context)).rejects.not.toBeInstanceOf( + UnauthorizedException + ); + }); + }); + + describe("request attachment", () => { it("should attach user and session to request on success", async () => { mockAuthService.verifySession.mockResolvedValue(mockSessionData); diff --git a/apps/api/src/auth/guards/auth.guard.ts b/apps/api/src/auth/guards/auth.guard.ts index 145b3cd..9e4c21d 100644 --- a/apps/api/src/auth/guards/auth.guard.ts +++ b/apps/api/src/auth/guards/auth.guard.ts @@ -1,23 +1,14 @@ import { Injectable, CanActivate, ExecutionContext, UnauthorizedException } from "@nestjs/common"; import { AuthService } from "../auth.service"; import type { AuthUser } from "@mosaic/shared"; - -/** - * Request type with authentication context - */ -interface AuthRequest { - user?: AuthUser; - session?: Record; - headers: Record; - cookies?: Record; -} +import type { MaybeAuthenticatedRequest } from "../types/better-auth-request.interface"; @Injectable() export class AuthGuard implements CanActivate { constructor(private readonly authService: AuthService) {} async canActivate(context: ExecutionContext): Promise { - const request = context.switchToHttp().getRequest(); + const request = context.switchToHttp().getRequest(); // Try to get token from either cookie (preferred) or Authorization header const token = this.extractToken(request); @@ -44,18 +35,19 @@ export class AuthGuard implements CanActivate { return true; } catch (error) { - // Re-throw if it's already an UnauthorizedException if (error instanceof UnauthorizedException) { throw error; } - throw new UnauthorizedException("Authentication failed"); + // Infrastructure errors (DB down, connection refused, timeouts) must propagate + // as 500/503 via GlobalExceptionFilter — never mask as 401 + throw error; } } /** * Extract token from cookie (preferred) or Authorization header */ - private extractToken(request: AuthRequest): string | undefined { + private extractToken(request: MaybeAuthenticatedRequest): string | undefined { // Try cookie first (BetterAuth default) const cookieToken = this.extractTokenFromCookie(request); if (cookieToken) { @@ -69,19 +61,21 @@ export class AuthGuard implements CanActivate { /** * Extract token from cookie (BetterAuth stores session token in better-auth.session_token cookie) */ - private extractTokenFromCookie(request: AuthRequest): string | undefined { - if (!request.cookies) { + private extractTokenFromCookie(request: MaybeAuthenticatedRequest): string | undefined { + // Express types `cookies` as `any`; cast to a known shape for type safety. + const cookies = request.cookies as Record | undefined; + if (!cookies) { return undefined; } // BetterAuth uses 'better-auth.session_token' as the cookie name by default - return request.cookies["better-auth.session_token"]; + return cookies["better-auth.session_token"]; } /** * Extract token from Authorization header (Bearer token) */ - private extractTokenFromHeader(request: AuthRequest): string | undefined { + private extractTokenFromHeader(request: MaybeAuthenticatedRequest): string | undefined { const authHeader = request.headers.authorization; if (typeof authHeader !== "string") { return undefined; diff --git a/apps/api/src/auth/types/better-auth-request.interface.ts b/apps/api/src/auth/types/better-auth-request.interface.ts index 8ff7587..7b93bd5 100644 --- a/apps/api/src/auth/types/better-auth-request.interface.ts +++ b/apps/api/src/auth/types/better-auth-request.interface.ts @@ -1,11 +1,14 @@ /** - * BetterAuth Request Type + * Unified request types for authentication context. * - * BetterAuth expects a Request object compatible with the Fetch API standard. - * This extends the web standard Request interface with additional properties - * that may be present in the Express request object at runtime. + * Replaces the previously scattered interfaces: + * - RequestWithSession (auth.controller.ts) + * - AuthRequest (auth.guard.ts) + * - BetterAuthRequest (this file, removed) + * - RequestWithUser (current-user.decorator.ts) */ +import type { Request } from "express"; import type { AuthUser } from "@mosaic/shared"; // Re-export AuthUser for use in other modules @@ -22,19 +25,21 @@ export interface RequestSession { } /** - * Web standard Request interface extended with Express-specific properties - * This matches the Fetch API Request specification that BetterAuth expects. + * Request that may or may not have auth data (before guard runs). + * Used by AuthGuard and other middleware that processes requests + * before authentication is confirmed. */ -export interface BetterAuthRequest extends Request { - // Express route parameters - params?: Record; - - // Express query string parameters - query?: Record; - - // Session data attached by AuthGuard after successful authentication - session?: RequestSession; - - // Authenticated user attached by AuthGuard +export interface MaybeAuthenticatedRequest extends Request { user?: AuthUser; + session?: Record; +} + +/** + * Request with authenticated user attached by AuthGuard. + * After AuthGuard runs, user and session are guaranteed present. + * Use this type in controllers/decorators that sit behind AuthGuard. + */ +export interface AuthenticatedRequest extends Request { + user: AuthUser; + session: RequestSession; } diff --git a/apps/api/src/main.ts b/apps/api/src/main.ts index 791fba5..19d7150 100644 --- a/apps/api/src/main.ts +++ b/apps/api/src/main.ts @@ -2,6 +2,7 @@ import { NestFactory } from "@nestjs/core"; import { ValidationPipe } from "@nestjs/common"; import cookieParser from "cookie-parser"; import { AppModule } from "./app.module"; +import { getTrustedOrigins } from "./auth/auth.config"; import { GlobalExceptionFilter } from "./filters/global-exception.filter"; function getPort(): number { @@ -47,39 +48,9 @@ async function bootstrap() { app.useGlobalFilters(new GlobalExceptionFilter()); // Configure CORS for cookie-based authentication - // SECURITY: Cannot use wildcard (*) with credentials: true - const isDevelopment = process.env.NODE_ENV !== "production"; - - const allowedOrigins = [ - process.env.NEXT_PUBLIC_APP_URL ?? "http://localhost:3000", - "https://app.mosaicstack.dev", // Production web - "https://api.mosaicstack.dev", // Production API - ]; - - // Development-only origins (not allowed in production) - if (isDevelopment) { - allowedOrigins.push("http://localhost:3001"); // API origin (dev) - } - + // Origin list is shared with BetterAuth trustedOrigins via getTrustedOrigins() app.enableCors({ - origin: ( - origin: string | undefined, - callback: (err: Error | null, allow?: boolean) => void - ): void => { - // Allow requests with no Origin header (health checks, server-to-server, - // load balancer probes). These are not cross-origin requests per the CORS spec. - if (!origin) { - callback(null, true); - return; - } - - // Check if origin is in allowed list - if (allowedOrigins.includes(origin)) { - callback(null, true); - } else { - callback(new Error(`Origin ${origin} not allowed by CORS`)); - } - }, + origin: getTrustedOrigins(), credentials: true, // Required for cookie-based authentication methods: ["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"], allowedHeaders: ["Content-Type", "Authorization", "Cookie", "X-CSRF-Token", "X-Workspace-Id"], diff --git a/apps/web/src/app/(auth)/callback/page.test.tsx b/apps/web/src/app/(auth)/callback/page.test.tsx index b41c87d..fcf95de 100644 --- a/apps/web/src/app/(auth)/callback/page.test.tsx +++ b/apps/web/src/app/(auth)/callback/page.test.tsx @@ -34,6 +34,8 @@ describe("CallbackPage", (): void => { isLoading: false, isAuthenticated: false, authError: null, + sessionExpiring: false, + sessionMinutesRemaining: 0, signOut: vi.fn(), }); }); @@ -51,6 +53,8 @@ describe("CallbackPage", (): void => { isLoading: false, isAuthenticated: false, authError: null, + sessionExpiring: false, + sessionMinutesRemaining: 0, signOut: vi.fn(), }); @@ -141,6 +145,8 @@ describe("CallbackPage", (): void => { isLoading: false, isAuthenticated: false, authError: null, + sessionExpiring: false, + sessionMinutesRemaining: 0, signOut: vi.fn(), }); diff --git a/apps/web/src/app/(auth)/login/page.test.tsx b/apps/web/src/app/(auth)/login/page.test.tsx index 6facd93..dc75f8b 100644 --- a/apps/web/src/app/(auth)/login/page.test.tsx +++ b/apps/web/src/app/(auth)/login/page.test.tsx @@ -1,37 +1,658 @@ -import { describe, it, expect, vi } from "vitest"; -import { render, screen } from "@testing-library/react"; +import { describe, it, expect, vi, beforeEach, type Mock } from "vitest"; +import { render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import type { AuthConfigResponse } from "@mosaic/shared"; import LoginPage from "./page"; -// Mock next/navigation -vi.mock("next/navigation", () => ({ - useRouter: (): { push: ReturnType } => ({ - push: vi.fn(), - }), +/* ------------------------------------------------------------------ */ +/* Hoisted mocks */ +/* ------------------------------------------------------------------ */ + +const { mockOAuth2, mockSignInEmail, mockPush, mockReplace, mockSearchParams } = vi.hoisted(() => ({ + mockOAuth2: vi.fn(), + mockSignInEmail: vi.fn(), + mockPush: vi.fn(), + mockReplace: vi.fn(), + mockSearchParams: new URLSearchParams(), })); +vi.mock("next/navigation", () => ({ + useRouter: (): { push: Mock; replace: Mock } => ({ + push: mockPush, + replace: mockReplace, + }), + useSearchParams: (): URLSearchParams => mockSearchParams, +})); + +vi.mock("@/lib/auth-client", () => ({ + signIn: { + oauth2: mockOAuth2, + email: mockSignInEmail, + }, +})); + +vi.mock("@/lib/config", () => ({ + API_BASE_URL: "http://localhost:3001", +})); + +// Mock fetchWithRetry to behave like fetch for test purposes +const { mockFetchWithRetry } = vi.hoisted(() => ({ + mockFetchWithRetry: vi.fn(), +})); + +vi.mock("@/lib/auth/fetch-with-retry", () => ({ + fetchWithRetry: mockFetchWithRetry, +})); + +// Use real parseAuthError implementation — vi.mock required for module resolution in vitest +vi.mock("@/lib/auth/auth-errors", async () => { + const actual = await import("../../../lib/auth/auth-errors"); + return { ...actual }; +}); + +/* ------------------------------------------------------------------ */ +/* Helpers */ +/* ------------------------------------------------------------------ */ + +function mockFetchConfig(config: AuthConfigResponse): void { + mockFetchWithRetry.mockResolvedValueOnce({ + ok: true, + json: (): Promise => Promise.resolve(config), + }); +} + +function mockFetchFailure(): void { + mockFetchWithRetry.mockRejectedValueOnce(new Error("Network error")); +} + +const OAUTH_ONLY_CONFIG: AuthConfigResponse = { + providers: [{ id: "authentik", name: "Authentik", type: "oauth" }], +}; + +const EMAIL_ONLY_CONFIG: AuthConfigResponse = { + providers: [{ id: "email", name: "Email", type: "credentials" }], +}; + +const BOTH_PROVIDERS_CONFIG: AuthConfigResponse = { + providers: [ + { id: "authentik", name: "Authentik", type: "oauth" }, + { id: "email", name: "Email", type: "credentials" }, + ], +}; + +/* ------------------------------------------------------------------ */ +/* Tests */ +/* ------------------------------------------------------------------ */ + describe("LoginPage", (): void => { - it("should render the login page with title", (): void => { + beforeEach((): void => { + vi.clearAllMocks(); + // Reset search params to empty for each test + mockSearchParams.delete("error"); + // Default: OAuth2 returns a resolved promise (fire-and-forget redirect) + mockOAuth2.mockResolvedValue(undefined); + }); + + it("renders loading state initially", (): void => { + // Never resolve fetch so it stays in loading state + // eslint-disable-next-line @typescript-eslint/no-empty-function -- intentionally never-resolving promise to test loading state + mockFetchWithRetry.mockReturnValueOnce(new Promise(() => {})); + render(); + + expect(screen.getByTestId("loading-spinner")).toBeInTheDocument(); + expect(screen.getByText("Loading authentication options")).toBeInTheDocument(); + }); + + it("renders the page heading and description", (): void => { + mockFetchConfig(EMAIL_ONLY_CONFIG); + + render(); + expect(screen.getByRole("heading", { level: 1 })).toHaveTextContent("Welcome to Mosaic Stack"); + expect(screen.getByText(/Your personal assistant platform/i)).toBeInTheDocument(); }); - it("should display the description", (): void => { - render(); - const descriptions = screen.getAllByText(/Your personal assistant platform/i); - expect(descriptions.length).toBeGreaterThan(0); - expect(descriptions[0]).toBeInTheDocument(); - }); + it("has proper layout styling", (): void => { + mockFetchConfig(EMAIL_ONLY_CONFIG); - it("should render the sign in button", (): void => { - render(); - const buttons = screen.getAllByRole("button", { name: /sign in/i }); - expect(buttons.length).toBeGreaterThan(0); - expect(buttons[0]).toBeInTheDocument(); - }); - - it("should have proper layout styling", (): void => { const { container } = render(); const main = container.querySelector("main"); expect(main).toHaveClass("flex", "min-h-screen"); }); + + it("fetches /auth/config on mount using fetchWithRetry", async (): Promise => { + mockFetchConfig(EMAIL_ONLY_CONFIG); + + render(); + + await waitFor((): void => { + expect(mockFetchWithRetry).toHaveBeenCalledWith("http://localhost:3001/auth/config"); + }); + }); + + it("renders OAuth button when OIDC provider is in config", async (): Promise => { + mockFetchConfig(OAUTH_ONLY_CONFIG); + + render(); + + await waitFor((): void => { + expect(screen.getByRole("button", { name: /continue with authentik/i })).toBeInTheDocument(); + }); + }); + + it("renders only LoginForm when only email provider is configured", async (): Promise => { + mockFetchConfig(EMAIL_ONLY_CONFIG); + + render(); + + await waitFor((): void => { + expect(screen.getByLabelText(/email/i)).toBeInTheDocument(); + }); + + expect(screen.getByLabelText(/password/i)).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: /continue with/i })).not.toBeInTheDocument(); + }); + + it("renders both OAuth button and LoginForm with divider when both providers present", async (): Promise => { + mockFetchConfig(BOTH_PROVIDERS_CONFIG); + + render(); + + await waitFor((): void => { + expect(screen.getByRole("button", { name: /continue with authentik/i })).toBeInTheDocument(); + }); + + expect(screen.getByText(/or continue with email/i)).toBeInTheDocument(); + expect(screen.getByLabelText(/email/i)).toBeInTheDocument(); + expect(screen.getByLabelText(/password/i)).toBeInTheDocument(); + }); + + it("does not render divider when only OAuth providers present", async (): Promise => { + mockFetchConfig(OAUTH_ONLY_CONFIG); + + render(); + + await waitFor((): void => { + expect(screen.getByRole("button", { name: /continue with authentik/i })).toBeInTheDocument(); + }); + + expect(screen.queryByText(/or continue with email/i)).not.toBeInTheDocument(); + }); + + it("shows error state with retry button on fetch failure instead of silent fallback", async (): Promise => { + mockFetchFailure(); + + render(); + + await waitFor((): void => { + expect(screen.getByTestId("config-error-state")).toBeInTheDocument(); + }); + + // Should NOT silently fall back to email form + expect(screen.queryByLabelText(/email/i)).not.toBeInTheDocument(); + expect(screen.queryByLabelText(/password/i)).not.toBeInTheDocument(); + expect(screen.queryByRole("button", { name: /continue with/i })).not.toBeInTheDocument(); + + // Should show the error banner with helpful message + expect( + screen.getByText( + "Unable to load sign-in options. Please refresh the page or try again in a moment." + ) + ).toBeInTheDocument(); + + // Should show a retry button + expect(screen.getByRole("button", { name: /try again/i })).toBeInTheDocument(); + }); + + it("shows error state on non-ok response", async (): Promise => { + mockFetchWithRetry.mockResolvedValueOnce({ + ok: false, + status: 500, + }); + + render(); + + await waitFor((): void => { + expect(screen.getByTestId("config-error-state")).toBeInTheDocument(); + }); + + // Should NOT silently fall back to email form + expect(screen.queryByLabelText(/email/i)).not.toBeInTheDocument(); + expect(screen.queryByRole("button", { name: /continue with/i })).not.toBeInTheDocument(); + + // Should show retry button + expect(screen.getByRole("button", { name: /try again/i })).toBeInTheDocument(); + }); + + it("retry button triggers re-fetch and recovers on success", async (): Promise => { + // First attempt: failure + mockFetchFailure(); + + render(); + + await waitFor((): void => { + expect(screen.getByTestId("config-error-state")).toBeInTheDocument(); + }); + + // Set up the second fetch to succeed + mockFetchConfig(EMAIL_ONLY_CONFIG); + + const user = userEvent.setup(); + await user.click(screen.getByRole("button", { name: /try again/i })); + + // Should eventually load the config and show the login form + await waitFor((): void => { + expect(screen.getByLabelText(/email/i)).toBeInTheDocument(); + }); + + // Error state should be gone + expect(screen.queryByTestId("config-error-state")).not.toBeInTheDocument(); + + // fetchWithRetry should have been called twice (initial + retry) + expect(mockFetchWithRetry).toHaveBeenCalledTimes(2); + }); + + it("calls signIn.oauth2 when OAuth button is clicked", async (): Promise => { + mockFetchConfig(OAUTH_ONLY_CONFIG); + const user = userEvent.setup(); + + render(); + + await waitFor((): void => { + expect(screen.getByRole("button", { name: /continue with authentik/i })).toBeInTheDocument(); + }); + + await user.click(screen.getByRole("button", { name: /continue with authentik/i })); + + expect(mockOAuth2).toHaveBeenCalledWith({ + providerId: "authentik", + callbackURL: "/", + }); + }); + + it("shows error when OAuth sign-in fails", async (): Promise => { + mockFetchConfig(OAUTH_ONLY_CONFIG); + mockOAuth2.mockRejectedValueOnce(new Error("Provider unavailable")); + const user = userEvent.setup(); + + render(); + + await waitFor((): void => { + expect(screen.getByRole("button", { name: /continue with authentik/i })).toBeInTheDocument(); + }); + + await user.click(screen.getByRole("button", { name: /continue with authentik/i })); + + await waitFor((): void => { + expect( + screen.getByText("Unable to connect to the sign-in provider. Please try again in a moment.") + ).toBeInTheDocument(); + }); + }); + + it("calls signIn.email and redirects on success", async (): Promise => { + mockFetchConfig(EMAIL_ONLY_CONFIG); + mockSignInEmail.mockResolvedValueOnce({ data: { user: {} } }); + const user = userEvent.setup(); + + render(); + + await waitFor((): void => { + expect(screen.getByLabelText(/email/i)).toBeInTheDocument(); + }); + + await user.type(screen.getByLabelText(/email/i), "test@example.com"); + await user.type(screen.getByLabelText(/password/i), "password123"); + await user.click(screen.getByRole("button", { name: /continue/i })); + + await waitFor((): void => { + expect(mockSignInEmail).toHaveBeenCalledWith({ + email: "test@example.com", + password: "password123", + }); + }); + + await waitFor((): void => { + expect(mockPush).toHaveBeenCalledWith("/tasks"); + }); + }); + + it("sanitizes BetterAuth error messages through parseAuthError", async (): Promise => { + mockFetchConfig(EMAIL_ONLY_CONFIG); + mockSignInEmail.mockResolvedValueOnce({ + error: { message: "Invalid credentials" }, + }); + const user = userEvent.setup(); + + render(); + + await waitFor((): void => { + expect(screen.getByLabelText(/email/i)).toBeInTheDocument(); + }); + + await user.type(screen.getByLabelText(/email/i), "test@example.com"); + await user.type(screen.getByLabelText(/password/i), "wrong"); + await user.click(screen.getByRole("button", { name: /continue/i })); + + // Raw "Invalid credentials" is mapped through parseAuthError to a PDA-friendly message + await waitFor((): void => { + expect( + screen.getByText("Authentication didn't complete. Please try again when ready.") + ).toBeInTheDocument(); + }); + + expect(mockPush).not.toHaveBeenCalled(); + }); + + it("maps raw DB/server errors to PDA-friendly messages instead of leaking details", async (): Promise => { + mockFetchConfig(EMAIL_ONLY_CONFIG); + // Simulate a leaked internal server error from BetterAuth + mockSignInEmail.mockResolvedValueOnce({ + error: { message: "Internal server error: connection to DB pool exhausted" }, + }); + const user = userEvent.setup(); + + render(); + + await waitFor((): void => { + expect(screen.getByLabelText(/email/i)).toBeInTheDocument(); + }); + + await user.type(screen.getByLabelText(/email/i), "test@example.com"); + await user.type(screen.getByLabelText(/password/i), "wrong"); + await user.click(screen.getByRole("button", { name: /continue/i })); + + // parseAuthError maps "internal server" keyword to server_error PDA-friendly message + await waitFor((): void => { + expect( + screen.getByText("The service is taking a break. Please try again in a moment.") + ).toBeInTheDocument(); + }); + + expect(mockPush).not.toHaveBeenCalled(); + }); + + it("should show fallback PDA-friendly message when error.message is not a string", async (): Promise => { + mockFetchConfig(EMAIL_ONLY_CONFIG); + // Return an error object where message is NOT a string (e.g. numeric code, no message field) + mockSignInEmail.mockResolvedValueOnce({ + error: { code: 123 }, + }); + const user = userEvent.setup(); + + render(); + + await waitFor((): void => { + expect(screen.getByLabelText(/email/i)).toBeInTheDocument(); + }); + + await user.type(screen.getByLabelText(/email/i), "test@example.com"); + await user.type(screen.getByLabelText(/password/i), "wrong"); + await user.click(screen.getByRole("button", { name: /continue/i })); + + // When error.message is falsy, parseAuthError receives the raw error object + // which falls through to the "unknown" code PDA-friendly message + await waitFor((): void => { + expect( + screen.getByText("Authentication didn't complete. Please try again when ready.") + ).toBeInTheDocument(); + }); + + expect(mockPush).not.toHaveBeenCalled(); + }); + + it("shows parseAuthError message on unexpected sign-in exception", async (): Promise => { + mockFetchConfig(EMAIL_ONLY_CONFIG); + mockSignInEmail.mockRejectedValueOnce(new TypeError("Failed to fetch")); + const user = userEvent.setup(); + + render(); + + await waitFor((): void => { + expect(screen.getByLabelText(/email/i)).toBeInTheDocument(); + }); + + await user.type(screen.getByLabelText(/email/i), "test@example.com"); + await user.type(screen.getByLabelText(/password/i), "password"); + await user.click(screen.getByRole("button", { name: /continue/i })); + + await waitFor((): void => { + // parseAuthError maps TypeError("Failed to fetch") to network_error message + expect( + screen.getByText("Unable to connect. Check your network and try again.") + ).toBeInTheDocument(); + }); + }); + + /* ------------------------------------------------------------------ */ + /* Responsive layout tests */ + /* ------------------------------------------------------------------ */ + + describe("responsive layout", (): void => { + it("applies mobile-first padding to main element", (): void => { + mockFetchConfig(EMAIL_ONLY_CONFIG); + + const { container } = render(); + const main = container.querySelector("main"); + + expect(main).toHaveClass("p-4", "sm:p-8"); + }); + + it("applies responsive text size to heading", (): void => { + mockFetchConfig(EMAIL_ONLY_CONFIG); + + render(); + + const heading = screen.getByRole("heading", { level: 1 }); + expect(heading).toHaveClass("text-2xl", "sm:text-4xl"); + }); + + it("applies responsive padding to card container", (): void => { + mockFetchConfig(EMAIL_ONLY_CONFIG); + + const { container } = render(); + const card = container.querySelector(".bg-white"); + + expect(card).toHaveClass("p-4", "sm:p-8"); + }); + + it("card container has full width with max-width constraint", (): void => { + mockFetchConfig(EMAIL_ONLY_CONFIG); + + const { container } = render(); + const wrapper = container.querySelector(".max-w-md"); + + expect(wrapper).toHaveClass("w-full", "max-w-md"); + }); + }); + + /* ------------------------------------------------------------------ */ + /* Accessibility tests */ + /* ------------------------------------------------------------------ */ + + describe("accessibility", (): void => { + it("loading spinner has role=status", (): void => { + // Never resolve fetch so it stays in loading state + // eslint-disable-next-line @typescript-eslint/no-empty-function -- intentionally never-resolving promise + mockFetchWithRetry.mockReturnValueOnce(new Promise(() => {})); + + render(); + + const spinner = screen.getByTestId("loading-spinner"); + expect(spinner).toHaveAttribute("role", "status"); + expect(spinner).toHaveAttribute("aria-label", "Loading authentication options"); + }); + + it("form inputs have associated labels", async (): Promise => { + mockFetchConfig(EMAIL_ONLY_CONFIG); + + render(); + + await waitFor((): void => { + expect(screen.getByLabelText(/email/i)).toBeInTheDocument(); + }); + + const emailInput = screen.getByLabelText(/email/i); + const passwordInput = screen.getByLabelText(/password/i); + + expect(emailInput).toHaveAttribute("id", "login-email"); + expect(passwordInput).toHaveAttribute("id", "login-password"); + }); + + it("error banner has role=alert and aria-live", async (): Promise => { + mockSearchParams.set("error", "access_denied"); + mockFetchConfig(EMAIL_ONLY_CONFIG); + + render(); + + await waitFor((): void => { + expect(screen.getByRole("alert")).toBeInTheDocument(); + }); + + const alert = screen.getByRole("alert"); + expect(alert).toHaveAttribute("aria-live", "polite"); + }); + + it("dismiss button has descriptive aria-label", async (): Promise => { + mockSearchParams.set("error", "access_denied"); + mockFetchConfig(EMAIL_ONLY_CONFIG); + + render(); + + await waitFor((): void => { + expect(screen.getByRole("alert")).toBeInTheDocument(); + }); + + const dismissButton = screen.getByRole("button", { name: /dismiss/i }); + expect(dismissButton).toHaveAttribute("aria-label", "Dismiss"); + }); + + it("interactive elements are keyboard-accessible (tab order)", async (): Promise => { + mockFetchConfig(EMAIL_ONLY_CONFIG); + const user = userEvent.setup(); + + render(); + + await waitFor((): void => { + expect(screen.getByLabelText(/email/i)).toBeInTheDocument(); + }); + + // LoginForm auto-focuses the email input on mount + expect(screen.getByLabelText(/email/i)).toHaveFocus(); + + // Tab forward through form: email -> password -> submit + await user.tab(); + expect(screen.getByLabelText(/password/i)).toHaveFocus(); + + await user.tab(); + expect(screen.getByRole("button", { name: /continue/i })).toHaveFocus(); + + // All interactive elements are reachable via keyboard + const oauthButton = screen.queryByRole("button", { name: /continue with/i }); + // No OAuth button in email-only config, but verify all form elements have tabindex >= 0 + expect(oauthButton).not.toBeInTheDocument(); + expect(screen.getByLabelText(/email/i)).not.toHaveAttribute("tabindex", "-1"); + expect(screen.getByLabelText(/password/i)).not.toHaveAttribute("tabindex", "-1"); + }); + + it("OAuth button has descriptive aria-label", async (): Promise => { + mockFetchConfig(OAUTH_ONLY_CONFIG); + + render(); + + await waitFor((): void => { + expect( + screen.getByRole("button", { name: /continue with authentik/i }) + ).toBeInTheDocument(); + }); + + const oauthButton = screen.getByRole("button", { name: /continue with authentik/i }); + expect(oauthButton).toHaveAttribute("aria-label", "Continue with Authentik"); + }); + }); + + /* ------------------------------------------------------------------ */ + /* URL error param tests */ + /* ------------------------------------------------------------------ */ + + describe("URL error query param", (): void => { + it("shows PDA-friendly banner for ?error=access_denied", async (): Promise => { + mockSearchParams.set("error", "access_denied"); + mockFetchConfig(EMAIL_ONLY_CONFIG); + + render(); + + await waitFor((): void => { + expect( + screen.getByText("Authentication paused. Please try again when ready.") + ).toBeInTheDocument(); + }); + + expect(mockReplace).toHaveBeenCalledWith("/login"); + }); + + it("shows correct message for ?error=invalid_credentials", async (): Promise => { + mockSearchParams.set("error", "invalid_credentials"); + mockFetchConfig(EMAIL_ONLY_CONFIG); + + render(); + + await waitFor((): void => { + expect( + screen.getByText("The email and password combination wasn't recognized.") + ).toBeInTheDocument(); + }); + }); + + it("shows default message for unknown error code", async (): Promise => { + mockSearchParams.set("error", "some_unknown_code"); + mockFetchConfig(EMAIL_ONLY_CONFIG); + + render(); + + await waitFor((): void => { + expect( + screen.getByText("Authentication didn't complete. Please try again when ready.") + ).toBeInTheDocument(); + }); + }); + + it("error banner is dismissible", async (): Promise => { + mockSearchParams.set("error", "access_denied"); + mockFetchConfig(EMAIL_ONLY_CONFIG); + const user = userEvent.setup(); + + render(); + + await waitFor((): void => { + expect( + screen.getByText("Authentication paused. Please try again when ready.") + ).toBeInTheDocument(); + }); + + // Clear the mock search params so the effect doesn't re-set the error on re-render + mockSearchParams.delete("error"); + + await user.click(screen.getByRole("button", { name: /dismiss/i })); + + await waitFor((): void => { + expect( + screen.queryByText("Authentication paused. Please try again when ready.") + ).not.toBeInTheDocument(); + }); + }); + + it("does not show error banner when no ?error param is present", async (): Promise => { + mockFetchConfig(EMAIL_ONLY_CONFIG); + + render(); + + await waitFor((): void => { + expect(screen.getByLabelText(/email/i)).toBeInTheDocument(); + }); + + expect(screen.queryByRole("alert")).not.toBeInTheDocument(); + }); + }); }); diff --git a/apps/web/src/app/(auth)/login/page.tsx b/apps/web/src/app/(auth)/login/page.tsx index 4881a19..1565d8d 100644 --- a/apps/web/src/app/(auth)/login/page.tsx +++ b/apps/web/src/app/(auth)/login/page.tsx @@ -1,19 +1,241 @@ +"use client"; + +import { Suspense, useEffect, useState, useCallback } from "react"; import type { ReactElement } from "react"; -import { LoginButton } from "@/components/auth/LoginButton"; +import { useRouter, useSearchParams } from "next/navigation"; +import { Loader2 } from "lucide-react"; +import type { AuthConfigResponse, AuthProviderConfig } from "@mosaic/shared"; +import { API_BASE_URL } from "@/lib/config"; +import { signIn } from "@/lib/auth-client"; +import { fetchWithRetry } from "@/lib/auth/fetch-with-retry"; +import { parseAuthError } from "@/lib/auth/auth-errors"; +import { OAuthButton } from "@/components/auth/OAuthButton"; +import { LoginForm } from "@/components/auth/LoginForm"; +import { AuthDivider } from "@/components/auth/AuthDivider"; +import { AuthErrorBanner } from "@/components/auth/AuthErrorBanner"; export default function LoginPage(): ReactElement { return ( -
+ +
+
+

Welcome to Mosaic Stack

+
+
+
+
+
+
+
+ } + > + + + ); +} + +function LoginPageContent(): ReactElement { + const router = useRouter(); + const searchParams = useSearchParams(); + const [config, setConfig] = useState(undefined); + const [loadingConfig, setLoadingConfig] = useState(true); + const [retryCount, setRetryCount] = useState(0); + const [oauthLoading, setOauthLoading] = useState(null); + const [credentialsLoading, setCredentialsLoading] = useState(false); + const [error, setError] = useState(null); + const [urlError, setUrlError] = useState(null); + + /* Read ?error= query param on mount and map to PDA-friendly message */ + useEffect(() => { + const errorCode = searchParams.get("error"); + if (errorCode) { + const parsed = parseAuthError(errorCode); + setUrlError(parsed.message); + // Clean up the URL by removing the error param without triggering navigation + const nextParams = new URLSearchParams(searchParams.toString()); + nextParams.delete("error"); + const query = nextParams.toString(); + router.replace(query ? `/login?${query}` : "/login"); + } + }, [searchParams, router]); + + useEffect(() => { + let cancelled = false; + + async function fetchConfig(): Promise { + try { + const response = await fetchWithRetry(`${API_BASE_URL}/auth/config`); + if (!response.ok) { + throw new Error("Failed to fetch auth config"); + } + const data = (await response.json()) as AuthConfigResponse; + if (!cancelled) { + setConfig(data); + } + } catch (err: unknown) { + if (!cancelled) { + console.error("[Auth] Failed to load auth config:", err); + setConfig(null); + setUrlError( + "Unable to load sign-in options. Please refresh the page or try again in a moment." + ); + } + } finally { + if (!cancelled) { + setLoadingConfig(false); + } + } + } + + void fetchConfig(); + + return (): void => { + cancelled = true; + }; + }, [retryCount]); + + const oauthProviders: AuthProviderConfig[] = + config?.providers.filter((p) => p.type === "oauth") ?? []; + const credentialProviders: AuthProviderConfig[] = + config?.providers.filter((p) => p.type === "credentials") ?? []; + + const hasOAuth = oauthProviders.length > 0; + const hasCredentials = credentialProviders.length > 0; + + const handleOAuthLogin = useCallback((providerId: string): void => { + setOauthLoading(providerId); + setError(null); + signIn.oauth2({ providerId, callbackURL: "/" }).catch((err: unknown) => { + const message = err instanceof Error ? err.message : String(err); + console.error(`[Auth] OAuth sign-in initiation failed for ${providerId}:`, message); + setError("Unable to connect to the sign-in provider. Please try again in a moment."); + setOauthLoading(null); + }); + }, []); + + const handleCredentialsLogin = useCallback( + async (email: string, password: string): Promise => { + setCredentialsLoading(true); + setError(null); + + try { + const result = await signIn.email({ email, password }); + + if (result.error) { + const parsed = parseAuthError( + result.error.message ? new Error(result.error.message) : result.error + ); + setError(parsed.message); + } else { + router.push("/tasks"); + } + } catch (err: unknown) { + const parsed = parseAuthError(err); + console.error("[Auth] Credentials sign-in failed:", err); + setError(parsed.message); + } finally { + setCredentialsLoading(false); + } + }, + [router] + ); + + const handleRetry = useCallback((): void => { + setConfig(undefined); + setLoadingConfig(true); + setUrlError(null); + setError(null); + setRetryCount((c) => c + 1); + }, []); + + return ( +
-

Welcome to Mosaic Stack

-

+

Welcome to Mosaic Stack

+

Your personal assistant platform. Organize tasks, events, and projects with a PDA-friendly approach.

-
- + +
+ {loadingConfig ? ( +
+
+ ) : config === null ? ( +
+ +
+ +
+
+ ) : ( + <> + {urlError && ( + { + setUrlError(null); + }} + /> + )} + + {error && !hasCredentials && ( + { + setError(null); + }} + /> + )} + + {hasOAuth && + oauthProviders.map((provider) => ( + { + handleOAuthLogin(provider.id); + }} + isLoading={oauthLoading === provider.id} + disabled={oauthLoading !== null && oauthLoading !== provider.id} + /> + ))} + + {hasOAuth && hasCredentials && } + + {hasCredentials && ( + + )} + + )}
diff --git a/apps/web/src/components/auth/AuthDivider.test.tsx b/apps/web/src/components/auth/AuthDivider.test.tsx new file mode 100644 index 0000000..3b1ae83 --- /dev/null +++ b/apps/web/src/components/auth/AuthDivider.test.tsx @@ -0,0 +1,27 @@ +import { describe, it, expect } from "vitest"; +import { render, screen } from "@testing-library/react"; +import { AuthDivider } from "./AuthDivider"; + +describe("AuthDivider", (): void => { + it("should render with default text", (): void => { + render(); + expect(screen.getByText("or continue with email")).toBeInTheDocument(); + }); + + it("should render with custom text", (): void => { + render(); + expect(screen.getByText("or sign up")).toBeInTheDocument(); + }); + + it("should render a horizontal divider line", (): void => { + const { container } = render(); + const line = container.querySelector("span.border-t"); + expect(line).toBeInTheDocument(); + }); + + it("should apply uppercase styling to text", (): void => { + const { container } = render(); + const textWrapper = container.querySelector(".uppercase"); + expect(textWrapper).toBeInTheDocument(); + }); +}); diff --git a/apps/web/src/components/auth/AuthDivider.tsx b/apps/web/src/components/auth/AuthDivider.tsx new file mode 100644 index 0000000..4d2f499 --- /dev/null +++ b/apps/web/src/components/auth/AuthDivider.tsx @@ -0,0 +1,18 @@ +interface AuthDividerProps { + text?: string; +} + +export function AuthDivider({ + text = "or continue with email", +}: AuthDividerProps): React.ReactElement { + return ( +
+
+ +
+
+ {text} +
+
+ ); +} diff --git a/apps/web/src/components/auth/AuthErrorBanner.test.tsx b/apps/web/src/components/auth/AuthErrorBanner.test.tsx new file mode 100644 index 0000000..22576c6 --- /dev/null +++ b/apps/web/src/components/auth/AuthErrorBanner.test.tsx @@ -0,0 +1,79 @@ +import { describe, it, expect, vi } from "vitest"; +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { AuthErrorBanner } from "./AuthErrorBanner"; + +describe("AuthErrorBanner", (): void => { + it("should render the message text", (): void => { + render(); + expect( + screen.getByText("Authentication paused. Please try again when ready.") + ).toBeInTheDocument(); + }); + + it("should have role alert and aria-live polite for accessibility", (): void => { + render(); + const alert = screen.getByRole("alert"); + expect(alert).toBeInTheDocument(); + expect(alert).toHaveAttribute("aria-live", "polite"); + }); + + it("should render the info icon, not a warning icon", (): void => { + const { container } = render(); + // Info icon from lucide-react renders as an SVG + const svgs = container.querySelectorAll("svg"); + expect(svgs.length).toBeGreaterThanOrEqual(1); + // The container should use blue styling, not red/yellow + const alert = screen.getByRole("alert"); + expect(alert.className).toContain("bg-blue-50"); + expect(alert.className).toContain("text-blue-700"); + expect(alert.className).not.toContain("red"); + expect(alert.className).not.toContain("yellow"); + }); + + it("should render dismiss button when onDismiss is provided", (): void => { + const onDismiss = vi.fn(); + render(); + const dismissButton = screen.getByLabelText("Dismiss"); + expect(dismissButton).toBeInTheDocument(); + }); + + it("should not render dismiss button when onDismiss is not provided", (): void => { + render(); + expect(screen.queryByLabelText("Dismiss")).not.toBeInTheDocument(); + }); + + it("should call onDismiss when dismiss button is clicked", async (): Promise => { + const user = userEvent.setup(); + const onDismiss = vi.fn(); + render(); + + const dismissButton = screen.getByLabelText("Dismiss"); + await user.click(dismissButton); + + expect(onDismiss).toHaveBeenCalledTimes(1); + }); + + it("should use blue info styling, not red or alarming colors", (): void => { + render(); + const alert = screen.getByRole("alert"); + expect(alert.className).toContain("bg-blue-50"); + expect(alert.className).toContain("border-blue-200"); + expect(alert.className).toContain("text-blue-700"); + }); + + it("should render all PDA-friendly error messages", (): void => { + const messages = [ + "Authentication paused. Please try again when ready.", + "The email and password combination wasn't recognized.", + "Unable to connect. Check your network and try again.", + "The service is taking a break. Please try again in a moment.", + ]; + + for (const message of messages) { + const { unmount } = render(); + expect(screen.getByText(message)).toBeInTheDocument(); + unmount(); + } + }); +}); diff --git a/apps/web/src/components/auth/AuthErrorBanner.tsx b/apps/web/src/components/auth/AuthErrorBanner.tsx new file mode 100644 index 0000000..cb6eba0 --- /dev/null +++ b/apps/web/src/components/auth/AuthErrorBanner.tsx @@ -0,0 +1,32 @@ +"use client"; + +import type { ReactElement } from "react"; +import { Info, X } from "lucide-react"; + +export interface AuthErrorBannerProps { + message: string; + onDismiss?: () => void; +} + +export function AuthErrorBanner({ message, onDismiss }: AuthErrorBannerProps): ReactElement { + return ( +
+
+ ); +} diff --git a/apps/web/src/components/auth/LoginButton.test.tsx b/apps/web/src/components/auth/LoginButton.test.tsx deleted file mode 100644 index d36fe7c..0000000 --- a/apps/web/src/components/auth/LoginButton.test.tsx +++ /dev/null @@ -1,45 +0,0 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; -import { render, screen } from "@testing-library/react"; -import userEvent from "@testing-library/user-event"; -import { LoginButton } from "./LoginButton"; - -const { mockOAuth2 } = vi.hoisted(() => ({ - mockOAuth2: vi.fn(), -})); - -vi.mock("@/lib/auth-client", () => ({ - signIn: { - oauth2: mockOAuth2, - }, -})); - -describe("LoginButton", (): void => { - beforeEach((): void => { - mockOAuth2.mockClear(); - }); - - it("should render sign in button", (): void => { - render(); - const button = screen.getByRole("button", { name: /sign in/i }); - expect(button).toBeInTheDocument(); - }); - - it("should initiate OAuth2 sign-in on click", async (): Promise => { - const user = userEvent.setup(); - render(); - - const button = screen.getByRole("button", { name: /sign in/i }); - await user.click(button); - - expect(mockOAuth2).toHaveBeenCalledWith({ - providerId: "authentik", - callbackURL: "/", - }); - }); - - it("should have proper styling", (): void => { - render(); - const button = screen.getByRole("button", { name: /sign in/i }); - expect(button).toHaveClass("w-full"); - }); -}); diff --git a/apps/web/src/components/auth/LoginButton.tsx b/apps/web/src/components/auth/LoginButton.tsx deleted file mode 100644 index bc8c5dd..0000000 --- a/apps/web/src/components/auth/LoginButton.tsx +++ /dev/null @@ -1,18 +0,0 @@ -"use client"; - -import { Button } from "@mosaic/ui"; -import { signIn } from "@/lib/auth-client"; - -export function LoginButton(): React.JSX.Element { - const handleLogin = (): void => { - // Use BetterAuth's genericOAuth client to initiate the OIDC flow. - // This POSTs to /auth/sign-in/oauth2 and follows the returned redirect URL. - void signIn.oauth2({ providerId: "authentik", callbackURL: "/" }); - }; - - return ( - - ); -} diff --git a/apps/web/src/components/auth/LoginForm.test.tsx b/apps/web/src/components/auth/LoginForm.test.tsx new file mode 100644 index 0000000..52a0ed7 --- /dev/null +++ b/apps/web/src/components/auth/LoginForm.test.tsx @@ -0,0 +1,168 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { LoginForm } from "./LoginForm"; + +describe("LoginForm", (): void => { + const mockOnSubmit = vi.fn(); + + beforeEach((): void => { + mockOnSubmit.mockClear(); + }); + + it("should render email and password fields with labels", (): void => { + render(); + expect(screen.getByLabelText("Email")).toBeInTheDocument(); + expect(screen.getByLabelText("Password")).toBeInTheDocument(); + }); + + it("should render a Continue submit button", (): void => { + render(); + expect(screen.getByRole("button", { name: "Continue" })).toBeInTheDocument(); + }); + + it("should auto-focus the email input on mount", (): void => { + render(); + const emailInput = screen.getByLabelText("Email"); + expect(document.activeElement).toBe(emailInput); + }); + + it("should validate email format on submit", async (): Promise => { + const user = userEvent.setup(); + render(); + + const emailInput = screen.getByLabelText("Email"); + const passwordInput = screen.getByLabelText("Password"); + const submitButton = screen.getByRole("button", { name: "Continue" }); + + await user.type(emailInput, "invalid-email"); + await user.type(passwordInput, "password123"); + await user.click(submitButton); + + expect(screen.getByText("Please enter a valid email address.")).toBeInTheDocument(); + expect(mockOnSubmit).not.toHaveBeenCalled(); + }); + + it("should validate non-empty password on submit", async (): Promise => { + const user = userEvent.setup(); + render(); + + const emailInput = screen.getByLabelText("Email"); + const submitButton = screen.getByRole("button", { name: "Continue" }); + + await user.type(emailInput, "user@example.com"); + await user.click(submitButton); + + expect(screen.getByText("Password is recommended.")).toBeInTheDocument(); + expect(mockOnSubmit).not.toHaveBeenCalled(); + }); + + it("should call onSubmit with email and password when valid", async (): Promise => { + const user = userEvent.setup(); + render(); + + const emailInput = screen.getByLabelText("Email"); + const passwordInput = screen.getByLabelText("Password"); + const submitButton = screen.getByRole("button", { name: "Continue" }); + + await user.type(emailInput, "user@example.com"); + await user.type(passwordInput, "password123"); + await user.click(submitButton); + + expect(mockOnSubmit).toHaveBeenCalledWith("user@example.com", "password123"); + }); + + it("should show loading state with spinner and Signing in text", (): void => { + render(); + expect(screen.getByText("Signing in...")).toBeInTheDocument(); + expect(screen.queryByText("Continue")).not.toBeInTheDocument(); + }); + + it("should disable inputs when loading", (): void => { + render(); + expect(screen.getByLabelText("Email")).toBeDisabled(); + expect(screen.getByLabelText("Password")).toBeDisabled(); + expect(screen.getByRole("button")).toBeDisabled(); + }); + + it("should display error message when error prop is provided", (): void => { + render( + + ); + expect( + screen.getByText("The email and password combination wasn't recognized.") + ).toBeInTheDocument(); + }); + + it("should dismiss error when dismiss button is clicked", async (): Promise => { + const user = userEvent.setup(); + render( + + ); + + expect( + screen.getByText("Authentication paused. Please try again when ready.") + ).toBeInTheDocument(); + + const dismissButton = screen.getByLabelText("Dismiss"); + await user.click(dismissButton); + + expect( + screen.queryByText("Authentication paused. Please try again when ready.") + ).not.toBeInTheDocument(); + }); + + it("should have htmlFor on email label pointing to email input", (): void => { + render(); + const emailLabel = screen.getByText("Email"); + const emailInput = screen.getByLabelText("Email"); + expect(emailLabel).toHaveAttribute("for", emailInput.id); + }); + + it("should have htmlFor on password label pointing to password input", (): void => { + render(); + const passwordLabel = screen.getByText("Password"); + const passwordInput = screen.getByLabelText("Password"); + expect(passwordLabel).toHaveAttribute("for", passwordInput.id); + }); + + it("should clear email validation error when user types a valid email", async (): Promise => { + const user = userEvent.setup(); + render(); + + const emailInput = screen.getByLabelText("Email"); + const submitButton = screen.getByRole("button", { name: "Continue" }); + + // Trigger validation error + await user.type(emailInput, "invalid"); + await user.click(submitButton); + expect(screen.getByText("Please enter a valid email address.")).toBeInTheDocument(); + + // Fix the email + await user.clear(emailInput); + await user.type(emailInput, "user@example.com"); + + await waitFor((): void => { + expect(screen.queryByText("Please enter a valid email address.")).not.toBeInTheDocument(); + }); + }); + + it("should set aria-invalid on email input when validation fails", async (): Promise => { + const user = userEvent.setup(); + render(); + + const emailInput = screen.getByLabelText("Email"); + const submitButton = screen.getByRole("button", { name: "Continue" }); + + await user.type(emailInput, "invalid"); + await user.click(submitButton); + + expect(emailInput).toHaveAttribute("aria-invalid", "true"); + }); +}); diff --git a/apps/web/src/components/auth/LoginForm.tsx b/apps/web/src/components/auth/LoginForm.tsx new file mode 100644 index 0000000..d20c496 --- /dev/null +++ b/apps/web/src/components/auth/LoginForm.tsx @@ -0,0 +1,172 @@ +"use client"; + +import { useRef, useEffect, useState, useCallback } from "react"; +import type { ReactElement } from "react"; +import { Loader2 } from "lucide-react"; +import { AuthErrorBanner } from "./AuthErrorBanner"; + +export interface LoginFormProps { + onSubmit: (email: string, password: string) => void | Promise; + isLoading?: boolean; + error?: string | null; +} + +export function LoginForm({ + onSubmit, + isLoading = false, + error = null, +}: LoginFormProps): ReactElement { + const emailRef = useRef(null); + const [email, setEmail] = useState(""); + const [password, setPassword] = useState(""); + const [emailError, setEmailError] = useState(null); + const [passwordError, setPasswordError] = useState(null); + const [dismissedError, setDismissedError] = useState(false); + + useEffect((): void => { + emailRef.current?.focus(); + }, []); + + // Reset dismissed state when a new error comes in + useEffect((): void => { + if (error) { + setDismissedError(false); + } + }, [error]); + + const validateEmail = useCallback((value: string): boolean => { + if (!value.includes("@")) { + setEmailError("Please enter a valid email address."); + return false; + } + setEmailError(null); + return true; + }, []); + + const validatePassword = useCallback((value: string): boolean => { + if (value.length === 0) { + setPasswordError("Password is recommended."); + return false; + } + setPasswordError(null); + return true; + }, []); + + const handleSubmit = (e: React.SyntheticEvent): void => { + e.preventDefault(); + + const isEmailValid = validateEmail(email); + const isPasswordValid = validatePassword(password); + + if (!isEmailValid || !isPasswordValid) { + return; + } + + void onSubmit(email, password); + }; + + return ( +
+ {error && !dismissedError && ( + { + setDismissedError(true); + }} + /> + )} + +
+ + { + setEmail(e.target.value); + if (emailError) { + validateEmail(e.target.value); + } + }} + disabled={isLoading} + autoComplete="email" + className={[ + "w-full px-3 py-2 border rounded-md", + "focus:outline-none focus:ring-2 focus:ring-blue-500 transition-colors", + emailError ? "border-blue-400" : "border-gray-300", + isLoading ? "opacity-50" : "", + ] + .filter(Boolean) + .join(" ")} + aria-invalid={emailError ? "true" : "false"} + aria-describedby={emailError ? "login-email-error" : undefined} + /> + {emailError && ( + + )} +
+ +
+ + { + setPassword(e.target.value); + if (passwordError) { + validatePassword(e.target.value); + } + }} + disabled={isLoading} + autoComplete="current-password" + className={[ + "w-full px-3 py-2 border rounded-md", + "focus:outline-none focus:ring-2 focus:ring-blue-500 transition-colors", + passwordError ? "border-blue-400" : "border-gray-300", + isLoading ? "opacity-50" : "", + ] + .filter(Boolean) + .join(" ")} + aria-invalid={passwordError ? "true" : "false"} + aria-describedby={passwordError ? "login-password-error" : undefined} + /> + {passwordError && ( + + )} +
+ + + + ); +} diff --git a/apps/web/src/components/auth/OAuthButton.test.tsx b/apps/web/src/components/auth/OAuthButton.test.tsx new file mode 100644 index 0000000..f6fb417 --- /dev/null +++ b/apps/web/src/components/auth/OAuthButton.test.tsx @@ -0,0 +1,89 @@ +import { describe, it, expect, vi } from "vitest"; +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { OAuthButton } from "./OAuthButton"; + +describe("OAuthButton", (): void => { + const defaultProps = { + providerName: "Authentik", + providerId: "authentik", + onClick: vi.fn(), + }; + + it("should render with provider name", (): void => { + render(); + expect(screen.getByText("Continue with Authentik")).toBeInTheDocument(); + }); + + it("should have full width styling", (): void => { + render(); + const button = screen.getByRole("button"); + expect(button.className).toContain("w-full"); + }); + + it("should call onClick when clicked", async (): Promise => { + const user = userEvent.setup(); + const onClick = vi.fn(); + render(); + + const button = screen.getByRole("button"); + await user.click(button); + + expect(onClick).toHaveBeenCalledTimes(1); + }); + + it("should show loading state with spinner and Connecting text", (): void => { + render(); + expect(screen.getByText("Connecting...")).toBeInTheDocument(); + expect(screen.queryByText("Continue with Authentik")).not.toBeInTheDocument(); + }); + + it("should be disabled when isLoading is true", (): void => { + render(); + const button = screen.getByRole("button"); + expect(button).toBeDisabled(); + }); + + it("should be disabled when disabled prop is true", (): void => { + render(); + const button = screen.getByRole("button"); + expect(button).toBeDisabled(); + }); + + it("should have reduced opacity when disabled", (): void => { + render(); + const button = screen.getByRole("button"); + expect(button.className).toContain("opacity-50"); + expect(button.className).toContain("pointer-events-none"); + }); + + it("should have aria-label with provider name", (): void => { + render(); + const button = screen.getByRole("button"); + expect(button).toHaveAttribute("aria-label", "Continue with Authentik"); + }); + + it("should have aria-label Connecting when loading", (): void => { + render(); + const button = screen.getByRole("button"); + expect(button).toHaveAttribute("aria-label", "Connecting"); + }); + + it("should render a spinner SVG when loading", (): void => { + const { container } = render(); + const spinner = container.querySelector("svg"); + expect(spinner).toBeInTheDocument(); + expect(spinner?.getAttribute("class")).toContain("animate-spin"); + }); + + it("should not call onClick when disabled", async (): Promise => { + const user = userEvent.setup(); + const onClick = vi.fn(); + render(); + + const button = screen.getByRole("button"); + await user.click(button); + + expect(onClick).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/web/src/components/auth/OAuthButton.tsx b/apps/web/src/components/auth/OAuthButton.tsx new file mode 100644 index 0000000..afd1023 --- /dev/null +++ b/apps/web/src/components/auth/OAuthButton.tsx @@ -0,0 +1,49 @@ +"use client"; + +import type { ReactElement } from "react"; +import { Loader2 } from "lucide-react"; + +export interface OAuthButtonProps { + providerName: string; + providerId: string; + onClick: () => void; + isLoading?: boolean; + disabled?: boolean; +} + +export function OAuthButton({ + providerName, + onClick, + isLoading = false, + disabled = false, +}: OAuthButtonProps): ReactElement { + const isDisabled = disabled || isLoading; + + return ( + + ); +} diff --git a/apps/web/src/components/auth/SessionExpiryWarning.test.tsx b/apps/web/src/components/auth/SessionExpiryWarning.test.tsx new file mode 100644 index 0000000..811621d --- /dev/null +++ b/apps/web/src/components/auth/SessionExpiryWarning.test.tsx @@ -0,0 +1,79 @@ +import { describe, it, expect, vi } from "vitest"; +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { SessionExpiryWarning } from "./SessionExpiryWarning"; + +describe("SessionExpiryWarning", (): void => { + it("should render with minutes remaining", (): void => { + render(); + expect(screen.getByText(/your session will end in 5 minutes/i)).toBeInTheDocument(); + }); + + it("should use singular 'minute' when 1 minute remaining", (): void => { + render(); + expect(screen.getByText(/your session will end in 1 minute\./i)).toBeInTheDocument(); + }); + + it("should show extend button when onExtend is provided", (): void => { + const onExtend = vi.fn(); + render(); + expect(screen.getByText("Extend Session")).toBeInTheDocument(); + }); + + it("should not show extend button when onExtend is not provided", (): void => { + render(); + expect(screen.queryByText("Extend Session")).not.toBeInTheDocument(); + }); + + it("should call onExtend when extend button is clicked", async (): Promise => { + const user = userEvent.setup(); + const onExtend = vi.fn(); + render(); + + await user.click(screen.getByText("Extend Session")); + expect(onExtend).toHaveBeenCalledOnce(); + }); + + it("should show dismiss button when onDismiss is provided", (): void => { + const onDismiss = vi.fn(); + render(); + expect(screen.getByLabelText("Dismiss")).toBeInTheDocument(); + }); + + it("should not show dismiss button when onDismiss is not provided", (): void => { + render(); + expect(screen.queryByLabelText("Dismiss")).not.toBeInTheDocument(); + }); + + it("should call onDismiss when dismiss button is clicked", async (): Promise => { + const user = userEvent.setup(); + const onDismiss = vi.fn(); + render(); + + await user.click(screen.getByLabelText("Dismiss")); + expect(onDismiss).toHaveBeenCalledOnce(); + }); + + it("should have role='status' for accessibility", (): void => { + render(); + expect(screen.getByRole("status")).toBeInTheDocument(); + }); + + it("should have aria-live='polite' for screen readers", (): void => { + render(); + const statusElement = screen.getByRole("status"); + expect(statusElement).toHaveAttribute("aria-live", "polite"); + }); + + it("should use blue theme (not red) for PDA-friendly design", (): void => { + render(); + const statusElement = screen.getByRole("status"); + expect(statusElement.className).toContain("bg-blue-50"); + expect(statusElement.className).toContain("border-blue-200"); + }); + + it("should include saving work reminder in message", (): void => { + render(); + expect(screen.getByText(/consider saving your work/i)).toBeInTheDocument(); + }); +}); diff --git a/apps/web/src/components/auth/SessionExpiryWarning.tsx b/apps/web/src/components/auth/SessionExpiryWarning.tsx new file mode 100644 index 0000000..ac205c9 --- /dev/null +++ b/apps/web/src/components/auth/SessionExpiryWarning.tsx @@ -0,0 +1,50 @@ +import { Info, X } from "lucide-react"; + +interface SessionExpiryWarningProps { + minutesRemaining: number; + onExtend?: () => void; + onDismiss?: () => void; +} + +export function SessionExpiryWarning({ + minutesRemaining, + onExtend, + onDismiss, +}: SessionExpiryWarningProps): React.ReactElement { + return ( +
+
+ ); +} diff --git a/apps/web/src/lib/auth-client.test.ts b/apps/web/src/lib/auth-client.test.ts new file mode 100644 index 0000000..0dd6fe4 --- /dev/null +++ b/apps/web/src/lib/auth-client.test.ts @@ -0,0 +1,407 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; + +/** Mock session shape returned by getSession in tests. */ +interface MockSessionData { + data: { + user: Record; + } | null; +} + +/** Words that must never appear in PDA-friendly messages. */ +const FORBIDDEN_WORDS = [ + "overdue", + "urgent", + "must", + "critical", + "required", + "error", + "failed", + "failure", +]; + +// Mock BetterAuth before importing the module under test +vi.mock("better-auth/react", () => ({ + createAuthClient: vi.fn(() => ({ + signIn: vi.fn(), + signOut: vi.fn(), + useSession: vi.fn(), + getSession: vi.fn(() => Promise.resolve({ data: null })), + })), +})); + +vi.mock("better-auth/client/plugins", () => ({ + genericOAuthClient: vi.fn(() => ({})), +})); + +vi.mock("./config", () => ({ + API_BASE_URL: "http://localhost:3001", +})); + +// Import after mocks are set up +const { signInWithCredentials, getAccessToken, isAdmin, getSession } = + await import("./auth-client"); + +/** + * Helper to build a mock Response object that behaves like the Fetch API Response. + */ +function mockResponse(options: { + ok: boolean; + status: number; + body?: Record; +}): Response { + const { ok, status, body = {} } = options; + return { + ok, + status, + json: vi.fn(() => Promise.resolve(body)), + headers: new Headers(), + redirected: false, + statusText: "", + type: "basic" as ResponseType, + url: "", + clone: vi.fn(), + body: null, + bodyUsed: false, + arrayBuffer: vi.fn(), + blob: vi.fn(), + formData: vi.fn(), + text: vi.fn(), + bytes: vi.fn(), + } as unknown as Response; +} + +describe("signInWithCredentials", (): void => { + const originalFetch = global.fetch; + + beforeEach((): void => { + global.fetch = vi.fn(); + }); + + afterEach((): void => { + global.fetch = originalFetch; + vi.restoreAllMocks(); + }); + + it("should return data on successful response", async (): Promise => { + const sessionData = { user: { id: "1", name: "Alice" }, token: "abc" }; + vi.mocked(global.fetch).mockResolvedValueOnce( + mockResponse({ ok: true, status: 200, body: sessionData }) + ); + + const result = await signInWithCredentials("alice", "password123"); + + expect(result).toEqual(sessionData); + expect(global.fetch).toHaveBeenCalledWith( + "http://localhost:3001/auth/sign-in/credentials", + expect.objectContaining({ + method: "POST", + credentials: "include", + body: JSON.stringify({ email: "alice", password: "password123" }), + }) + ); + }); + + it("should throw PDA-friendly message on 401 response", async (): Promise => { + vi.mocked(global.fetch).mockResolvedValueOnce( + mockResponse({ ok: false, status: 401, body: { message: "Unauthorized" } }) + ); + + await expect(signInWithCredentials("alice", "wrong")).rejects.toThrow( + "The email and password combination wasn't recognized." + ); + }); + + it("should throw PDA-friendly message on 401 with no body message", async (): Promise => { + vi.mocked(global.fetch).mockResolvedValueOnce( + mockResponse({ ok: false, status: 401, body: {} }) + ); + + // When there is no body message, the response object (status: 401) is used for parsing + await expect(signInWithCredentials("alice", "wrong")).rejects.toThrow( + "The email and password combination wasn't recognized." + ); + }); + + it("should throw PDA-friendly message on 500 response", async (): Promise => { + vi.mocked(global.fetch).mockResolvedValueOnce( + mockResponse({ + ok: false, + status: 500, + body: { message: "Internal Server Error" }, + }) + ); + + await expect(signInWithCredentials("alice", "pass")).rejects.toThrow( + "The service is taking a break. Please try again in a moment." + ); + }); + + it("should throw PDA-friendly message on 500 with no body message", async (): Promise => { + vi.mocked(global.fetch).mockResolvedValueOnce( + mockResponse({ ok: false, status: 500, body: {} }) + ); + + await expect(signInWithCredentials("alice", "pass")).rejects.toThrow( + "The service is taking a break. Please try again in a moment." + ); + }); + + it("should throw PDA-friendly message on network error (fetch throws)", async (): Promise => { + vi.mocked(global.fetch).mockRejectedValueOnce(new TypeError("Failed to fetch")); + + await expect(signInWithCredentials("alice", "pass")).rejects.toThrow(TypeError); + }); + + it("should throw PDA-friendly message on 429 rate-limited response", async (): Promise => { + vi.mocked(global.fetch).mockResolvedValueOnce( + mockResponse({ + ok: false, + status: 429, + body: { message: "Too many requests" }, + }) + ); + + await expect(signInWithCredentials("alice", "pass")).rejects.toThrow( + "You've tried a few times. Take a moment and try again shortly." + ); + }); + + it("should throw PDA-friendly message when response.json() throws", async (): Promise => { + const errorSpy = vi.spyOn(console, "error").mockReturnValue(undefined); + const resp = mockResponse({ ok: false, status: 403 }); + const jsonError = new SyntaxError("Unexpected token"); + (resp.json as ReturnType).mockRejectedValueOnce(jsonError); + vi.mocked(global.fetch).mockResolvedValueOnce(resp); + + // JSON parse fails -> logs error -> falls back to response status + await expect(signInWithCredentials("alice", "pass")).rejects.toThrow( + "The email and password combination wasn't recognized." + ); + + expect(errorSpy).toHaveBeenCalledWith( + "[Auth] Failed to parse error response body (HTTP 403):", + jsonError + ); + errorSpy.mockRestore(); + }); +}); + +describe("signInWithCredentials PDA-friendly language compliance", (): void => { + const originalFetch = global.fetch; + + beforeEach((): void => { + global.fetch = vi.fn(); + }); + + afterEach((): void => { + global.fetch = originalFetch; + vi.restoreAllMocks(); + }); + + const errorScenarios: { + name: string; + status: number; + body: Record; + }[] = [ + { name: "401 with message", status: 401, body: { message: "Unauthorized" } }, + { name: "401 without message", status: 401, body: {} }, + { name: "403 with message", status: 403, body: { message: "Forbidden" } }, + { name: "429 with message", status: 429, body: { message: "Too many requests" } }, + { name: "500 with message", status: 500, body: { message: "Internal Server Error" } }, + { name: "500 without message", status: 500, body: {} }, + { name: "502 without message", status: 502, body: {} }, + { name: "503 without message", status: 503, body: {} }, + { name: "400 unknown", status: 400, body: {} }, + ]; + + for (const scenario of errorScenarios) { + it(`should not contain forbidden words for ${scenario.name} response`, async (): Promise => { + vi.mocked(global.fetch).mockResolvedValueOnce( + mockResponse({ ok: false, status: scenario.status, body: scenario.body }) + ); + + try { + await signInWithCredentials("alice", "pass"); + // Should not reach here + expect.unreachable("signInWithCredentials should have thrown"); + } catch (thrown: unknown) { + expect(thrown).toBeInstanceOf(Error); + const message = (thrown as Error).message.toLowerCase(); + for (const forbidden of FORBIDDEN_WORDS) { + expect(message).not.toContain(forbidden); + } + } + }); + } +}); + +// ──────────────────────────────────────────────────────────────────────────── +// AUTH-030: getAccessToken tests +// ──────────────────────────────────────────────────────────────────────────── + +describe("getAccessToken", (): void => { + afterEach((): void => { + vi.restoreAllMocks(); + }); + + it("should return null when no session exists (session.data is null)", async (): Promise => { + vi.mocked(getSession).mockResolvedValueOnce({ data: null } as MockSessionData); + + const result = await getAccessToken(); + + expect(result).toBeNull(); + }); + + it("should return accessToken when session has valid, non-expired token", async (): Promise => { + vi.mocked(getSession).mockResolvedValueOnce({ + data: { + user: { + id: "user-1", + accessToken: "valid-token-abc", + tokenExpiresAt: Date.now() + 300_000, // 5 minutes from now + }, + }, + } as MockSessionData); + + const result = await getAccessToken(); + + expect(result).toBe("valid-token-abc"); + }); + + it("should return null when token is expired (tokenExpiresAt in the past)", async (): Promise => { + vi.mocked(getSession).mockResolvedValueOnce({ + data: { + user: { + id: "user-1", + accessToken: "expired-token", + tokenExpiresAt: Date.now() - 120_000, // 2 minutes ago + }, + }, + } as MockSessionData); + + const result = await getAccessToken(); + + expect(result).toBeNull(); + }); + + it("should return null when token expires within 60-second buffer window", async (): Promise => { + vi.mocked(getSession).mockResolvedValueOnce({ + data: { + user: { + id: "user-1", + accessToken: "almost-expired-token", + tokenExpiresAt: Date.now() + 30_000, // 30 seconds from now (within 60s buffer) + }, + }, + } as MockSessionData); + + const result = await getAccessToken(); + + expect(result).toBeNull(); + }); + + it("should return null and warn when accessToken is undefined on user object", async (): Promise => { + const warnSpy = vi.spyOn(console, "warn").mockReturnValue(undefined); + vi.mocked(getSession).mockResolvedValueOnce({ + data: { + user: { + id: "user-1", + // no accessToken property + }, + }, + } as MockSessionData); + + const result = await getAccessToken(); + + expect(result).toBeNull(); + expect(warnSpy).toHaveBeenCalledWith("[Auth] Session exists but no accessToken found"); + warnSpy.mockRestore(); + }); + + it("should return null and log error when getSession throws", async (): Promise => { + const errorSpy = vi.spyOn(console, "error").mockReturnValue(undefined); + const sessionError = new Error("Network failure"); + vi.mocked(getSession).mockRejectedValueOnce(sessionError); + + const result = await getAccessToken(); + + expect(result).toBeNull(); + expect(errorSpy).toHaveBeenCalledWith("[Auth] Failed to get access token:", sessionError); + errorSpy.mockRestore(); + }); +}); + +// ──────────────────────────────────────────────────────────────────────────── +// AUTH-030: isAdmin tests +// ──────────────────────────────────────────────────────────────────────────── + +describe("isAdmin", (): void => { + afterEach((): void => { + vi.restoreAllMocks(); + }); + + it("should return false when no session exists", async (): Promise => { + vi.mocked(getSession).mockResolvedValueOnce({ data: null } as MockSessionData); + + const result = await isAdmin(); + + expect(result).toBe(false); + }); + + it("should return true when user.isAdmin is true", async (): Promise => { + vi.mocked(getSession).mockResolvedValueOnce({ + data: { + user: { + id: "admin-1", + isAdmin: true, + }, + }, + } as MockSessionData); + + const result = await isAdmin(); + + expect(result).toBe(true); + }); + + it("should return false when user.isAdmin is false", async (): Promise => { + vi.mocked(getSession).mockResolvedValueOnce({ + data: { + user: { + id: "user-1", + isAdmin: false, + }, + }, + } as MockSessionData); + + const result = await isAdmin(); + + expect(result).toBe(false); + }); + + it("should return false when user.isAdmin is undefined", async (): Promise => { + vi.mocked(getSession).mockResolvedValueOnce({ + data: { + user: { + id: "user-1", + // no isAdmin property + }, + }, + } as MockSessionData); + + const result = await isAdmin(); + + expect(result).toBe(false); + }); + + it("should return false and log error when getSession throws", async (): Promise => { + const errorSpy = vi.spyOn(console, "error").mockReturnValue(undefined); + const sessionError = new Error("Network failure"); + vi.mocked(getSession).mockRejectedValueOnce(sessionError); + + const result = await isAdmin(); + + expect(result).toBe(false); + expect(errorSpy).toHaveBeenCalledWith("[Auth] Failed to check admin status:", sessionError); + errorSpy.mockRestore(); + }); +}); diff --git a/apps/web/src/lib/auth-client.ts b/apps/web/src/lib/auth-client.ts index 393fb11..26e2810 100644 --- a/apps/web/src/lib/auth-client.ts +++ b/apps/web/src/lib/auth-client.ts @@ -4,11 +4,12 @@ * This client handles: * - Sign in/out operations * - Session management - * - Automatic token refresh + * - Cookie-based session lifecycle */ import { createAuthClient } from "better-auth/react"; import { genericOAuthClient } from "better-auth/client/plugins"; import { API_BASE_URL } from "./config"; +import { parseAuthError } from "./auth/auth-errors"; /** * Auth client instance configured for Mosaic Stack. @@ -25,25 +26,34 @@ export const authClient = createAuthClient({ export const { signIn, signOut, useSession, getSession } = authClient; /** - * Sign in with username and password. + * Sign in with email and password. * Returns the session on success, throws on failure. * - * Uses direct fetch since our server accepts username (not email) - * and the default BetterAuth client expects email. + * Uses direct fetch to POST credentials to BetterAuth's sign-in endpoint. + * The email parameter accepts an email address used as the credential identifier. */ -export async function signInWithCredentials(username: string, password: string): Promise { +export async function signInWithCredentials(email: string, password: string): Promise { const response = await fetch(`${API_BASE_URL}/auth/sign-in/credentials`, { method: "POST", headers: { "Content-Type": "application/json", }, credentials: "include", // Include cookies - body: JSON.stringify({ username, password }), + body: JSON.stringify({ email, password }), }); if (!response.ok) { - const error = (await response.json().catch(() => ({}))) as { message?: string }; - throw new Error(error.message ?? "Authentication failed"); + let errorBody: { message?: string } = {}; + try { + errorBody = (await response.json()) as { message?: string }; + } catch (jsonError: unknown) { + console.error( + `[Auth] Failed to parse error response body (HTTP ${String(response.status)}):`, + jsonError + ); + } + const parsed = parseAuthError(errorBody.message ? new Error(errorBody.message) : response); + throw new Error(parsed.message); } const data = (await response.json()) as unknown; @@ -55,37 +65,52 @@ export async function signInWithCredentials(username: string, password: string): * Returns null if not authenticated. */ export async function getAccessToken(): Promise { - const session = await getSession(); - if (!session.data?.user) { + try { + const session = await getSession(); + if (!session.data?.user) { + return null; + } + + // Type assertion for custom user fields + const user = session.data.user as { + accessToken?: string; + tokenExpiresAt?: number; + }; + + if (!user.accessToken) { + console.warn("[Auth] Session exists but no accessToken found"); + return null; + } + + // Check if token is expired (with 1 minute buffer) + if (user.tokenExpiresAt && user.tokenExpiresAt - Date.now() < 60000) { + // Token is expired or about to expire + // The session will be refreshed automatically by BetterAuth + // but we should return null to trigger a re-auth if needed + return null; + } + + return user.accessToken; + } catch (error: unknown) { + console.error("[Auth] Failed to get access token:", error); return null; } - - // Type assertion for custom user fields - const user = session.data.user as { - accessToken?: string; - tokenExpiresAt?: number; - }; - - // Check if token is expired (with 1 minute buffer) - if (user.tokenExpiresAt && user.tokenExpiresAt - Date.now() < 60000) { - // Token is expired or about to expire - // The session will be refreshed automatically by BetterAuth - // but we should return null to trigger a re-auth if needed - return null; - } - - return user.accessToken ?? null; } /** * Check if the current user is an admin. */ export async function isAdmin(): Promise { - const session = await getSession(); - if (!session.data?.user) { + try { + const session = await getSession(); + if (!session.data?.user) { + return false; + } + + const user = session.data.user as { isAdmin?: boolean }; + return user.isAdmin === true; + } catch (error: unknown) { + console.error("[Auth] Failed to check admin status:", error); return false; } - - const user = session.data.user as { isAdmin?: boolean }; - return user.isAdmin === true; } diff --git a/apps/web/src/lib/auth/auth-context.test.tsx b/apps/web/src/lib/auth/auth-context.test.tsx index 98f20b9..f2a1861 100644 --- a/apps/web/src/lib/auth/auth-context.test.tsx +++ b/apps/web/src/lib/auth/auth-context.test.tsx @@ -1,4 +1,5 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; +import React, { act } from "react"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { render, screen, waitFor } from "@testing-library/react"; import { AuthProvider, useAuth } from "./auth-context"; import type { AuthUser } from "@mosaic/shared"; @@ -11,9 +12,23 @@ vi.mock("../api/client", () => ({ const { apiGet, apiPost } = await import("../api/client"); +/** Helper: returns a date far in the future (1 hour from now) for session mocks */ +function futureExpiry(): string { + return new Date(Date.now() + 60 * 60 * 1000).toISOString(); +} + // Test component that uses the auth context function TestComponent(): React.JSX.Element { - const { user, isLoading, isAuthenticated, authError, signOut } = useAuth(); + const { + user, + isLoading, + isAuthenticated, + authError, + sessionExpiring, + sessionMinutesRemaining, + signOut, + refreshSession, + } = useAuth(); if (isLoading) { return
Loading...
; @@ -23,6 +38,8 @@ function TestComponent(): React.JSX.Element {
{isAuthenticated ? "Authenticated" : "Not Authenticated"}
{authError ?? "none"}
+
{sessionExpiring ? "true" : "false"}
+
{sessionMinutesRemaining}
{user && (
{user.email}
@@ -30,6 +47,7 @@ function TestComponent(): React.JSX.Element {
)} +
); } @@ -65,7 +83,7 @@ describe("AuthContext", (): void => { vi.mocked(apiGet).mockResolvedValueOnce({ user: mockUser, - session: { id: "session-1", token: "token123", expiresAt: new Date() }, + session: { id: "session-1", token: "token123", expiresAt: futureExpiry() }, }); render( @@ -83,6 +101,10 @@ describe("AuthContext", (): void => { }); it("should handle unauthenticated state when session check fails", async (): Promise => { + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => { + // Intentionally empty - suppressing log output in tests + }); + vi.mocked(apiGet).mockRejectedValueOnce(new Error("Unauthorized")); render( @@ -96,6 +118,8 @@ describe("AuthContext", (): void => { }); expect(screen.queryByTestId("user-email")).not.toBeInTheDocument(); + + consoleErrorSpy.mockRestore(); }); it("should clear user on sign out", async (): Promise => { @@ -107,7 +131,7 @@ describe("AuthContext", (): void => { vi.mocked(apiGet).mockResolvedValueOnce({ user: mockUser, - session: { id: "session-1", token: "token123", expiresAt: new Date() }, + session: { id: "session-1", token: "token123", expiresAt: futureExpiry() }, }); vi.mocked(apiPost).mockResolvedValueOnce({ success: true }); @@ -134,6 +158,104 @@ describe("AuthContext", (): void => { expect(apiPost).toHaveBeenCalledWith("/auth/sign-out"); }); + it("should clear user and set authError to 'network' when signOut fails with a network error", async (): Promise => { + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => { + // Intentionally empty - suppressing log output in tests + }); + + const mockUser: AuthUser = { + id: "user-1", + email: "test@example.com", + name: "Test User", + }; + + // First: user is logged in + vi.mocked(apiGet).mockResolvedValueOnce({ + user: mockUser, + session: { id: "session-1", token: "token123", expiresAt: futureExpiry() }, + }); + + // signOut request fails with a network error (TypeError with "fetch") + vi.mocked(apiPost).mockRejectedValueOnce(new TypeError("Failed to fetch")); + + render( + + + + ); + + // Wait for authenticated state + await waitFor(() => { + expect(screen.getByTestId("auth-status")).toHaveTextContent("Authenticated"); + }); + + // Click sign out — the apiPost will reject + const signOutButton = screen.getByRole("button", { name: "Sign Out" }); + signOutButton.click(); + + // User should be cleared (finally block runs even on error) + await waitFor(() => { + expect(screen.getByTestId("auth-status")).toHaveTextContent("Not Authenticated"); + }); + + // authError should be set to "network" via classifyAuthError + expect(screen.getByTestId("auth-error")).toHaveTextContent("network"); + + // Verify the sign-out endpoint was still called + expect(apiPost).toHaveBeenCalledWith("/auth/sign-out"); + + consoleErrorSpy.mockRestore(); + }); + + it("should clear user and set authError to 'backend' when signOut fails with a server error", async (): Promise => { + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => { + // Intentionally empty - suppressing log output in tests + }); + + const mockUser: AuthUser = { + id: "user-1", + email: "test@example.com", + name: "Test User", + }; + + // First: user is logged in + vi.mocked(apiGet).mockResolvedValueOnce({ + user: mockUser, + session: { id: "session-1", token: "token123", expiresAt: futureExpiry() }, + }); + + // signOut request fails with a 500 Internal Server Error + vi.mocked(apiPost).mockRejectedValueOnce(new Error("Internal Server Error")); + + render( + + + + ); + + // Wait for authenticated state + await waitFor(() => { + expect(screen.getByTestId("auth-status")).toHaveTextContent("Authenticated"); + }); + + // Click sign out — the apiPost will reject with server error + const signOutButton = screen.getByRole("button", { name: "Sign Out" }); + signOutButton.click(); + + // User should be cleared (finally block runs even on error) + await waitFor(() => { + expect(screen.getByTestId("auth-status")).toHaveTextContent("Not Authenticated"); + }); + + // authError should be set to "backend" via classifyAuthError + expect(screen.getByTestId("auth-error")).toHaveTextContent("backend"); + + // Verify the sign-out endpoint was still called + expect(apiPost).toHaveBeenCalledWith("/auth/sign-out"); + + consoleErrorSpy.mockRestore(); + }); + it("should throw error when useAuth is used outside AuthProvider", (): void => { // Suppress console.error for this test const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => { @@ -148,8 +270,9 @@ describe("AuthContext", (): void => { }); describe("auth error handling", (): void => { - it("should not set authError for normal unauthenticated state (401/403)", async (): Promise => { - // Normal auth error - user is just not logged in + it("should set authError to null for normal 401 Unauthorized (not logged in)", async (): Promise => { + // 401 Unauthorized is a normal condition (user not logged in), not an error. + // classifyAuthError should return null so no "having trouble" banner appears. vi.mocked(apiGet).mockRejectedValueOnce(new Error("Unauthorized")); render( @@ -162,11 +285,75 @@ describe("AuthContext", (): void => { expect(screen.getByTestId("auth-status")).toHaveTextContent("Not Authenticated"); }); - // Should NOT have an auth error - this is expected behavior + // authError should be null (displayed as "none" by TestComponent) expect(screen.getByTestId("auth-error")).toHaveTextContent("none"); }); + it("should set authError to null for 403 Forbidden", async (): Promise => { + // 403 Forbidden is also classified as invalid_credentials by parseAuthError + vi.mocked(apiGet).mockRejectedValueOnce(new Error("Forbidden")); + + render( + + + + ); + + await waitFor(() => { + expect(screen.getByTestId("auth-status")).toHaveTextContent("Not Authenticated"); + }); + + expect(screen.getByTestId("auth-error")).toHaveTextContent("none"); + }); + + it("should set authError to null for session expired errors", async (): Promise => { + // "session expired" is a normal auth lifecycle event, not a backend error + vi.mocked(apiGet).mockRejectedValueOnce(new Error("Session expired")); + + render( + + + + ); + + await waitFor(() => { + expect(screen.getByTestId("auth-status")).toHaveTextContent("Not Authenticated"); + }); + + expect(screen.getByTestId("auth-error")).toHaveTextContent("none"); + }); + + it("should set authError to 'backend' for truly unrecognised Error instances", async (): Promise => { + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => { + // Intentionally empty + }); + + // An Error that doesn't match any known pattern (parseAuthError returns "unknown") + // should fall through to the instanceof Error catch-all and return "backend" + vi.mocked(apiGet).mockRejectedValueOnce( + new Error("Something completely unexpected happened") + ); + + render( + + + + ); + + await waitFor(() => { + expect(screen.getByTestId("auth-status")).toHaveTextContent("Not Authenticated"); + }); + + expect(screen.getByTestId("auth-error")).toHaveTextContent("backend"); + + consoleErrorSpy.mockRestore(); + }); + it("should set authError to 'network' for fetch failures", async (): Promise => { + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => { + // Intentionally empty + }); + // Network error - backend is unreachable vi.mocked(apiGet).mockRejectedValueOnce(new TypeError("Failed to fetch")); @@ -182,12 +369,11 @@ describe("AuthContext", (): void => { // Should have a network error expect(screen.getByTestId("auth-error")).toHaveTextContent("network"); + + consoleErrorSpy.mockRestore(); }); - it("should log errors in development mode", async (): Promise => { - // Temporarily set to development - vi.stubEnv("NODE_ENV", "development"); - + it("should always log auth errors (including production)", async (): Promise => { const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => { // Intentionally empty - we're testing that errors are logged }); @@ -205,14 +391,13 @@ describe("AuthContext", (): void => { expect(screen.getByTestId("auth-error")).toHaveTextContent("network"); }); - // Should log error in development + // Should log error regardless of NODE_ENV expect(consoleErrorSpy).toHaveBeenCalledWith( expect.stringContaining("[Auth]"), expect.any(Error) ); consoleErrorSpy.mockRestore(); - vi.unstubAllEnvs(); }); it("should set authError to 'network' for connection refused", async (): Promise => { @@ -220,7 +405,8 @@ describe("AuthContext", (): void => { // Intentionally empty }); - vi.mocked(apiGet).mockRejectedValueOnce(new Error("ECONNREFUSED")); + // "Connection refused" includes "connection" which parseAuthError maps to network_error + vi.mocked(apiGet).mockRejectedValueOnce(new Error("Connection refused")); render( @@ -279,7 +465,7 @@ describe("AuthContext", (): void => { consoleErrorSpy.mockRestore(); }); - it("should clear authError after successful session refresh", async (): Promise => { + it("should persist authError across re-renders when no new session check occurs", async (): Promise => { const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => { // Intentionally empty }); @@ -297,27 +483,212 @@ describe("AuthContext", (): void => { expect(screen.getByTestId("auth-error")).toHaveTextContent("network"); }); - // Set up successful response for refresh - const mockUser: AuthUser = { - id: "user-1", - email: "test@example.com", - name: "Test User", - }; - vi.mocked(apiGet).mockResolvedValueOnce({ - user: mockUser, - session: { id: "session-1", token: "token123", expiresAt: new Date() }, - }); - - // Trigger a rerender (simulating refreshSession being called) + // Re-render does NOT trigger a new session check, so authError persists rerender( ); - // The initial render will have checked session once, error should still be there - // A real refresh would need to call refreshSession + // authError should still be "network" — re-render alone does not clear it + expect(screen.getByTestId("auth-error")).toHaveTextContent("network"); + consoleErrorSpy.mockRestore(); }); }); + + describe("session expiry detection", (): void => { + const mockUser: AuthUser = { + id: "user-1", + email: "test@example.com", + name: "Test User", + }; + + beforeEach((): void => { + // Reset all mocks to clear any unconsumed mockResolvedValueOnce queues + // from previous tests (vi.clearAllMocks only clears calls/results, not implementations) + vi.resetAllMocks(); + }); + + afterEach((): void => { + // Ensure no stale intervals leak between tests + vi.clearAllTimers(); + }); + + it("should set sessionExpiring to false when session has plenty of time remaining", async (): Promise => { + const farFuture = new Date(Date.now() + 60 * 60 * 1000).toISOString(); // 60 minutes + + vi.mocked(apiGet).mockResolvedValueOnce({ + user: mockUser, + session: { id: "session-1", token: "token123", expiresAt: farFuture }, + }); + + render( + + + + ); + + await waitFor(() => { + expect(screen.getByTestId("auth-status")).toHaveTextContent("Authenticated"); + }); + + expect(screen.getByTestId("session-expiring")).toHaveTextContent("false"); + }); + + it("should set sessionExpiring to true when session is within 5 minutes of expiry", async (): Promise => { + const nearExpiry = new Date(Date.now() + 3 * 60 * 1000).toISOString(); // 3 minutes + + vi.mocked(apiGet).mockResolvedValueOnce({ + user: mockUser, + session: { id: "session-1", token: "token123", expiresAt: nearExpiry }, + }); + + render( + + + + ); + + await waitFor(() => { + expect(screen.getByTestId("auth-status")).toHaveTextContent("Authenticated"); + }); + + await waitFor(() => { + expect(screen.getByTestId("session-expiring")).toHaveTextContent("true"); + }); + }); + + it("should calculate sessionMinutesRemaining correctly", async (): Promise => { + const nearExpiry = new Date(Date.now() + 3 * 60 * 1000).toISOString(); // 3 minutes + + vi.mocked(apiGet).mockResolvedValueOnce({ + user: mockUser, + session: { id: "session-1", token: "token123", expiresAt: nearExpiry }, + }); + + render( + + + + ); + + await waitFor(() => { + expect(screen.getByTestId("auth-status")).toHaveTextContent("Authenticated"); + }); + + await waitFor(() => { + expect(screen.getByTestId("session-minutes-remaining")).toHaveTextContent("3"); + }); + }); + + it("should transition from not-expiring to expiring after interval fires", async (): Promise => { + vi.useFakeTimers(); + + // Session expires 6 minutes from now - just outside the warning window + const expiresAt = new Date(Date.now() + 6 * 60 * 1000).toISOString(); + + vi.mocked(apiGet).mockResolvedValueOnce({ + user: mockUser, + session: { id: "session-1", token: "token123", expiresAt }, + }); + + await act(async () => { + render( + + + + ); + // Flush the resolved mock promise so checkSession completes + await Promise.resolve(); + await Promise.resolve(); + }); + + // Initially not expiring (6 minutes remaining) + expect(screen.getByTestId("auth-status")).toHaveTextContent("Authenticated"); + expect(screen.getByTestId("session-expiring")).toHaveTextContent("false"); + + // Advance 2 minutes - should now be within the 5-minute window (4 min remaining) + await act(async () => { + vi.advanceTimersByTime(2 * 60 * 1000); + await Promise.resolve(); + }); + + expect(screen.getByTestId("session-expiring")).toHaveTextContent("true"); + + vi.useRealTimers(); + }); + + it("should log out user and set session_expired when session expires via interval", async (): Promise => { + vi.useFakeTimers(); + + // Session expires 30 seconds from now + const almostExpired = new Date(Date.now() + 30 * 1000).toISOString(); + + vi.mocked(apiGet).mockResolvedValueOnce({ + user: mockUser, + session: { id: "session-1", token: "token123", expiresAt: almostExpired }, + }); + + await act(async () => { + render( + + + + ); + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(screen.getByTestId("auth-status")).toHaveTextContent("Authenticated"); + + // Advance past the expiry time (triggers the 60s interval) + await act(async () => { + vi.advanceTimersByTime(60 * 1000); + await Promise.resolve(); + }); + + expect(screen.getByTestId("auth-status")).toHaveTextContent("Not Authenticated"); + expect(screen.getByTestId("session-expiring")).toHaveTextContent("false"); + // Session expiry now sets explicit session_expired error state + expect(screen.getByTestId("auth-error")).toHaveTextContent("session_expired"); + + vi.useRealTimers(); + }); + + it("should reset sessionExpiring after successful refreshSession", async (): Promise => { + // Session near expiry (3 minutes remaining) + const nearExpiry = new Date(Date.now() + 3 * 60 * 1000).toISOString(); + + vi.mocked(apiGet).mockResolvedValueOnce({ + user: mockUser, + session: { id: "session-1", token: "token123", expiresAt: nearExpiry }, + }); + + render( + + + + ); + + await waitFor(() => { + expect(screen.getByTestId("session-expiring")).toHaveTextContent("true"); + }); + + // Set up a refreshed session response with a far-future expiry + const refreshedExpiry = new Date(Date.now() + 60 * 60 * 1000).toISOString(); + vi.mocked(apiGet).mockResolvedValueOnce({ + user: mockUser, + session: { id: "session-2", token: "token456", expiresAt: refreshedExpiry }, + }); + + // Click refresh button + const refreshButton = screen.getByRole("button", { name: "Refresh" }); + refreshButton.click(); + + await waitFor(() => { + expect(screen.getByTestId("session-expiring")).toHaveTextContent("false"); + }); + }); + }); }); diff --git a/apps/web/src/lib/auth/auth-context.tsx b/apps/web/src/lib/auth/auth-context.tsx index 99c3dda..1b25acc 100644 --- a/apps/web/src/lib/auth/auth-context.tsx +++ b/apps/web/src/lib/auth/auth-context.tsx @@ -1,19 +1,36 @@ "use client"; -import { createContext, useContext, useState, useEffect, useCallback, type ReactNode } from "react"; +import { + createContext, + useContext, + useState, + useEffect, + useCallback, + useRef, + type ReactNode, +} from "react"; import type { AuthUser, AuthSession } from "@mosaic/shared"; import { apiGet, apiPost } from "../api/client"; +import { parseAuthError } from "./auth-errors"; /** * Error types for auth session checks */ -export type AuthErrorType = "network" | "backend" | null; +export type AuthErrorType = "network" | "backend" | "session_expired" | null; + +/** Threshold in minutes before session expiry to start warning */ +const SESSION_EXPIRY_WARNING_MINUTES = 5; + +/** Interval in milliseconds to check session expiry */ +const SESSION_CHECK_INTERVAL_MS = 60_000; interface AuthContextValue { user: AuthUser | null; isLoading: boolean; isAuthenticated: boolean; authError: AuthErrorType; + sessionExpiring: boolean; + sessionMinutesRemaining: number; signOut: () => Promise; refreshSession: () => Promise; } @@ -21,76 +38,67 @@ interface AuthContextValue { const AuthContext = createContext(undefined); /** - * Check if an error indicates a network/backend issue vs normal "not authenticated" + * Classify an error into an {@link AuthErrorType} using the centralised + * {@link parseAuthError} utility. + * + * Normal authentication failures (401 Unauthorized, session expired) return + * `null` so the UI simply shows the logged-out state without a banner. + * + * Defaults unrecognised `Error` instances to `"backend"` rather than `null` + * so that unexpected failures surface a "having trouble connecting" banner + * instead of silently logging the user out. */ -function isBackendError(error: unknown): { isBackendDown: boolean; errorType: AuthErrorType } { - // Network errors (fetch failed, DNS, connection refused, etc.) - if (error instanceof TypeError && error.message.includes("fetch")) { - return { isBackendDown: true, errorType: "network" }; - } - - // Check for specific error messages that indicate backend issues - if (error instanceof Error) { - const message = error.message.toLowerCase(); - - // Network-level errors - if ( - message.includes("network") || - message.includes("failed to fetch") || - message.includes("connection refused") || - message.includes("econnrefused") || - message.includes("timeout") - ) { - return { isBackendDown: true, errorType: "network" }; - } - - // Backend errors (5xx status codes typically result in these messages) - if ( - message.includes("internal server error") || - message.includes("service unavailable") || - message.includes("bad gateway") || - message.includes("gateway timeout") - ) { - return { isBackendDown: true, errorType: "backend" }; - } - } - - // Normal auth errors (401, 403, etc.) - user is just not logged in - return { isBackendDown: false, errorType: null }; +function classifyAuthError(error: unknown): AuthErrorType { + const parsed = parseAuthError(error); + if (parsed.code === "network_error") return "network"; + if (parsed.code === "server_error") return "backend"; + // Normal auth failures (not logged in, session expired) are not errors — + // return null so the UI shows logged-out state without a banner + if (parsed.code === "invalid_credentials" || parsed.code === "session_expired") return null; + // For truly unrecognised errors, default to "backend" rather than null + // (safer to show "having trouble connecting" than silently log out) + if (error instanceof Error) return "backend"; + return null; } /** - * Log auth errors in development mode + * Log auth errors — always logs, including production. + * Auth failures are operational issues, not debug noise. */ function logAuthError(message: string, error: unknown): void { - if (process.env.NODE_ENV === "development") { - console.error(`[Auth] ${message}:`, error); - } + console.error(`[Auth] ${message}:`, error); } export function AuthProvider({ children }: { children: ReactNode }): React.JSX.Element { const [user, setUser] = useState(null); const [isLoading, setIsLoading] = useState(true); const [authError, setAuthError] = useState(null); + const [sessionExpiring, setSessionExpiring] = useState(false); + const [sessionMinutesRemaining, setSessionMinutesRemaining] = useState(0); + const expiresAtRef = useRef(null); const checkSession = useCallback(async () => { try { const session = await apiGet("/auth/session"); setUser(session.user); setAuthError(null); - } catch (error) { - const { isBackendDown, errorType } = isBackendError(error); - if (isBackendDown) { - // Backend/network issue - log and expose error to UI + // Track session expiry timestamp + expiresAtRef.current = new Date(session.session.expiresAt); + + // Reset expiring state on successful session check + setSessionExpiring(false); + } catch (error) { + const errorType = classifyAuthError(error); + + if (errorType) { logAuthError("Session check failed due to backend/network issue", error); - setAuthError(errorType); - } else { - // Normal "not authenticated" state - no logging needed - setAuthError(null); } + setAuthError(errorType); setUser(null); + expiresAtRef.current = null; + setSessionExpiring(false); } finally { setIsLoading(false); } @@ -100,9 +108,12 @@ export function AuthProvider({ children }: { children: ReactNode }): React.JSX.E try { await apiPost("/auth/sign-out"); } catch (error) { - console.error("Sign out error:", error); + logAuthError("Sign out request did not complete", error); + setAuthError(classifyAuthError(error) ?? "backend"); } finally { setUser(null); + expiresAtRef.current = null; + setSessionExpiring(false); } }, []); @@ -114,11 +125,50 @@ export function AuthProvider({ children }: { children: ReactNode }): React.JSX.E void checkSession(); }, [checkSession]); + // Periodically check whether the session is approaching expiry + useEffect((): (() => void) => { + if (!user || !expiresAtRef.current) { + return (): void => { + /* no-op cleanup */ + }; + } + + const checkExpiry = (): void => { + if (!expiresAtRef.current) return; + + const remainingMs = expiresAtRef.current.getTime() - Date.now(); + const minutes = Math.ceil(remainingMs / 60_000); + + if (minutes <= 0) { + // Session has expired — set explicit state so the UI can react + setUser(null); + setSessionExpiring(false); + setSessionMinutesRemaining(0); + expiresAtRef.current = null; + setAuthError("session_expired"); + } else if (minutes <= SESSION_EXPIRY_WARNING_MINUTES) { + setSessionExpiring(true); + setSessionMinutesRemaining(minutes); + } else { + setSessionExpiring(false); + setSessionMinutesRemaining(minutes); + } + }; + + checkExpiry(); + const interval = setInterval(checkExpiry, SESSION_CHECK_INTERVAL_MS); + return (): void => { + clearInterval(interval); + }; + }, [user]); + const value: AuthContextValue = { user, isLoading, isAuthenticated: user !== null, authError, + sessionExpiring, + sessionMinutesRemaining, signOut, refreshSession, }; diff --git a/apps/web/src/lib/auth/auth-errors.test.ts b/apps/web/src/lib/auth/auth-errors.test.ts new file mode 100644 index 0000000..49e8a2f --- /dev/null +++ b/apps/web/src/lib/auth/auth-errors.test.ts @@ -0,0 +1,294 @@ +import { describe, it, expect } from "vitest"; +import { parseAuthError, getErrorMessage } from "./auth-errors"; +import type { AuthErrorCode, ParsedAuthError } from "./auth-errors"; + +/** Words that must never appear in PDA-friendly messages. */ +const FORBIDDEN_WORDS = [ + "overdue", + "urgent", + "must", + "critical", + "required", + "error", + "failed", + "failure", +]; + +describe("parseAuthError", (): void => { + it("should classify TypeError('Failed to fetch') as network_error", (): void => { + const result: ParsedAuthError = parseAuthError(new TypeError("Failed to fetch")); + expect(result.code).toBe("network_error"); + expect(result.retryable).toBe(true); + }); + + it("should classify TypeError with 'fetch' anywhere in message as network_error", (): void => { + const result: ParsedAuthError = parseAuthError(new TypeError("Could not fetch resource")); + expect(result.code).toBe("network_error"); + }); + + it("should classify Error('Unauthorized') as invalid_credentials", (): void => { + const result: ParsedAuthError = parseAuthError(new Error("Unauthorized")); + expect(result.code).toBe("invalid_credentials"); + expect(result.retryable).toBe(false); + }); + + it("should classify Error('Forbidden') as invalid_credentials", (): void => { + const result: ParsedAuthError = parseAuthError(new Error("Forbidden")); + expect(result.code).toBe("invalid_credentials"); + }); + + it("should classify HTTP 401 response as invalid_credentials", (): void => { + const result: ParsedAuthError = parseAuthError({ status: 401 }); + expect(result.code).toBe("invalid_credentials"); + expect(result.retryable).toBe(false); + }); + + it("should classify HTTP 403 response as invalid_credentials", (): void => { + const result: ParsedAuthError = parseAuthError({ status: 403 }); + expect(result.code).toBe("invalid_credentials"); + }); + + it("should classify HTTP 429 response as rate_limited", (): void => { + const result: ParsedAuthError = parseAuthError({ status: 429 }); + expect(result.code).toBe("rate_limited"); + expect(result.retryable).toBe(false); + }); + + it("should classify HTTP 500 response as server_error", (): void => { + const result: ParsedAuthError = parseAuthError({ status: 500 }); + expect(result.code).toBe("server_error"); + expect(result.retryable).toBe(true); + }); + + it("should classify HTTP 502 response as server_error", (): void => { + const result: ParsedAuthError = parseAuthError({ status: 502 }); + expect(result.code).toBe("server_error"); + }); + + it("should classify HTTP 503 response as server_error", (): void => { + const result: ParsedAuthError = parseAuthError({ status: 503 }); + expect(result.code).toBe("server_error"); + }); + + it("should classify string 'access_denied' as access_denied", (): void => { + const result: ParsedAuthError = parseAuthError("access_denied"); + expect(result.code).toBe("access_denied"); + expect(result.retryable).toBe(false); + }); + + it("should classify string 'session_expired' as session_expired", (): void => { + const result: ParsedAuthError = parseAuthError("session_expired"); + expect(result.code).toBe("session_expired"); + expect(result.retryable).toBe(false); + }); + + it("should classify string 'rate_limited' as rate_limited", (): void => { + const result: ParsedAuthError = parseAuthError("rate_limited"); + expect(result.code).toBe("rate_limited"); + }); + + it("should classify string 'server_error' as server_error", (): void => { + const result: ParsedAuthError = parseAuthError("server_error"); + expect(result.code).toBe("server_error"); + expect(result.retryable).toBe(true); + }); + + it("should classify string 'network_error' as network_error", (): void => { + const result: ParsedAuthError = parseAuthError("network_error"); + expect(result.code).toBe("network_error"); + expect(result.retryable).toBe(true); + }); + + it("should classify unknown string as unknown", (): void => { + const result: ParsedAuthError = parseAuthError("something_weird"); + expect(result.code).toBe("unknown"); + expect(result.retryable).toBe(false); + }); + + it("should classify null as unknown", (): void => { + const result: ParsedAuthError = parseAuthError(null); + expect(result.code).toBe("unknown"); + }); + + it("should classify undefined as unknown", (): void => { + const result: ParsedAuthError = parseAuthError(undefined); + expect(result.code).toBe("unknown"); + }); + + it("should classify a number as unknown", (): void => { + const result: ParsedAuthError = parseAuthError(42); + expect(result.code).toBe("unknown"); + }); + + it("should classify an empty object as unknown", (): void => { + const result: ParsedAuthError = parseAuthError({}); + expect(result.code).toBe("unknown"); + }); + + it("should classify Error('Internal Server Error') as server_error", (): void => { + const result: ParsedAuthError = parseAuthError(new Error("Internal Server Error")); + expect(result.code).toBe("server_error"); + expect(result.retryable).toBe(true); + }); + + it("should classify Error('Service Unavailable') as server_error", (): void => { + const result: ParsedAuthError = parseAuthError(new Error("Service Unavailable")); + expect(result.code).toBe("server_error"); + }); + + it("should classify Error('Too many requests') as rate_limited", (): void => { + const result: ParsedAuthError = parseAuthError(new Error("Too many requests")); + expect(result.code).toBe("rate_limited"); + }); + + it("should classify Error('Session expired') as session_expired", (): void => { + const result: ParsedAuthError = parseAuthError(new Error("Session expired")); + expect(result.code).toBe("session_expired"); + }); + + it("should classify Error('Network issue') as network_error", (): void => { + const result: ParsedAuthError = parseAuthError(new Error("Network issue")); + expect(result.code).toBe("network_error"); + }); + + it("should classify Error with unknown message as unknown", (): void => { + const result: ParsedAuthError = parseAuthError(new Error("Something completely different")); + expect(result.code).toBe("unknown"); + }); +}); + +describe("parseAuthError retryable flag", (): void => { + it("should mark network_error as retryable", (): void => { + expect(parseAuthError(new TypeError("Failed to fetch")).retryable).toBe(true); + }); + + it("should mark server_error as retryable", (): void => { + expect(parseAuthError({ status: 500 }).retryable).toBe(true); + }); + + it("should mark invalid_credentials as not retryable", (): void => { + expect(parseAuthError(new Error("Unauthorized")).retryable).toBe(false); + }); + + it("should mark access_denied as not retryable", (): void => { + expect(parseAuthError("access_denied").retryable).toBe(false); + }); + + it("should mark rate_limited as not retryable", (): void => { + expect(parseAuthError("rate_limited").retryable).toBe(false); + }); + + it("should mark session_expired as not retryable", (): void => { + expect(parseAuthError("session_expired").retryable).toBe(false); + }); + + it("should mark unknown as not retryable", (): void => { + expect(parseAuthError(null).retryable).toBe(false); + }); +}); + +describe("getErrorMessage", (): void => { + const allCodes: AuthErrorCode[] = [ + "access_denied", + "invalid_credentials", + "server_error", + "network_error", + "rate_limited", + "session_expired", + "unknown", + ]; + + it("should return the correct message for access_denied", (): void => { + expect(getErrorMessage("access_denied")).toBe( + "Authentication paused. Please try again when ready." + ); + }); + + it("should return the correct message for invalid_credentials", (): void => { + expect(getErrorMessage("invalid_credentials")).toBe( + "The email and password combination wasn't recognized." + ); + }); + + it("should return the correct message for server_error", (): void => { + expect(getErrorMessage("server_error")).toBe( + "The service is taking a break. Please try again in a moment." + ); + }); + + it("should return the correct message for network_error", (): void => { + expect(getErrorMessage("network_error")).toBe( + "Unable to connect. Check your network and try again." + ); + }); + + it("should return the correct message for rate_limited", (): void => { + expect(getErrorMessage("rate_limited")).toBe( + "You've tried a few times. Take a moment and try again shortly." + ); + }); + + it("should return the correct message for session_expired", (): void => { + expect(getErrorMessage("session_expired")).toBe( + "Your session ended. Please sign in again when ready." + ); + }); + + it("should return the correct message for unknown", (): void => { + expect(getErrorMessage("unknown")).toBe( + "Authentication didn't complete. Please try again when ready." + ); + }); + + it("should return a non-empty string for every error code", (): void => { + for (const code of allCodes) { + const message = getErrorMessage(code); + expect(message).toBeTruthy(); + expect(message.length).toBeGreaterThan(0); + } + }); +}); + +describe("PDA-friendly language compliance", (): void => { + const allCodes: AuthErrorCode[] = [ + "access_denied", + "invalid_credentials", + "server_error", + "network_error", + "rate_limited", + "session_expired", + "unknown", + ]; + + it("should not contain any forbidden words in any message", (): void => { + for (const code of allCodes) { + const message = getErrorMessage(code).toLowerCase(); + for (const forbidden of FORBIDDEN_WORDS) { + expect(message).not.toContain(forbidden); + } + } + }); + + it("should not contain forbidden words in parseAuthError output messages", (): void => { + const testInputs: unknown[] = [ + new TypeError("Failed to fetch"), + new Error("Unauthorized"), + new Error("Internal Server Error"), + { status: 429 }, + { status: 500 }, + "access_denied", + "session_expired", + null, + undefined, + 42, + ]; + + for (const input of testInputs) { + const result = parseAuthError(input); + const message = result.message.toLowerCase(); + for (const forbidden of FORBIDDEN_WORDS) { + expect(message).not.toContain(forbidden); + } + } + }); +}); diff --git a/apps/web/src/lib/auth/auth-errors.ts b/apps/web/src/lib/auth/auth-errors.ts new file mode 100644 index 0000000..0aedcfe --- /dev/null +++ b/apps/web/src/lib/auth/auth-errors.ts @@ -0,0 +1,166 @@ +/** + * Auth error codes, PDA-friendly message mapping, and error parsing utilities. + * + * All user-facing messages follow PDA-friendly language guidelines: + * no alarming words like OVERDUE, URGENT, MUST, CRITICAL, REQUIRED, ERROR, FAILED. + */ + +/** Union of all recognised auth error codes. */ +export type AuthErrorCode = + | "access_denied" + | "invalid_credentials" + | "server_error" + | "network_error" + | "rate_limited" + | "session_expired" + | "unknown"; + +/** A parsed, UI-ready representation of an auth error. */ +export interface ParsedAuthError { + code: AuthErrorCode; + /** PDA-friendly message suitable for display to the user. */ + message: string; + /** Whether the operation that caused this can be retried. */ + retryable: boolean; +} + +/** + * PDA-friendly error messages keyed by error code. + * Uses calm, informational language throughout. + */ +const ERROR_MESSAGES: Record = { + access_denied: "Authentication paused. Please try again when ready.", + invalid_credentials: "The email and password combination wasn't recognized.", + server_error: "The service is taking a break. Please try again in a moment.", + network_error: "Unable to connect. Check your network and try again.", + rate_limited: "You've tried a few times. Take a moment and try again shortly.", + session_expired: "Your session ended. Please sign in again when ready.", + unknown: "Authentication didn't complete. Please try again when ready.", +}; + +/** Error codes that are safe to retry automatically. */ +const RETRYABLE_CODES: ReadonlySet = new Set([ + "network_error", + "server_error", +]); + +/** Set of recognised error code strings for fast membership testing. */ +const KNOWN_CODES: ReadonlySet = new Set(Object.keys(ERROR_MESSAGES)); + +/** + * Type-guard: checks whether a string value is a known {@link AuthErrorCode}. + */ +function isAuthErrorCode(value: string): value is AuthErrorCode { + return KNOWN_CODES.has(value); +} + +/** + * Type-guard: checks whether a value looks like an HTTP response object + * with a numeric `status` property. + */ +function isHttpResponseLike(value: unknown): value is { status: number } { + return ( + typeof value === "object" && + value !== null && + "status" in value && + typeof (value as { status: unknown }).status === "number" + ); +} + +/** + * Map an HTTP status code to an {@link AuthErrorCode}. + */ +function httpStatusToCode(status: number): AuthErrorCode { + // In auth context, both 401 and 403 indicate the user should re-authenticate + if (status === 401 || status === 403) { + return "invalid_credentials"; + } + if (status === 429) { + return "rate_limited"; + } + if (status >= 500) { + return "server_error"; + } + return "unknown"; +} + +/** + * Build a {@link ParsedAuthError} for the given code. + */ +function buildParsedError(code: AuthErrorCode): ParsedAuthError { + return { + code, + message: ERROR_MESSAGES[code], + retryable: RETRYABLE_CODES.has(code), + }; +} + +/** + * Parse an unknown error value into a structured, PDA-friendly + * {@link ParsedAuthError}. + * + * Handles: + * - `TypeError` whose message contains "fetch" -> `network_error` + * - Generic `Error` objects with keyword-based message matching + * - HTTP-response-shaped objects with a numeric `status` field + * - Plain strings that match a known error code + * - Anything else falls back to `unknown` + */ +export function parseAuthError(error: unknown): ParsedAuthError { + // 1. TypeError with "fetch" in message -> network error + if (error instanceof TypeError && error.message.toLowerCase().includes("fetch")) { + return buildParsedError("network_error"); + } + + // 2. Generic Error objects — match on message keywords + if (error instanceof Error) { + const msg = error.message.toLowerCase(); + + if (msg.includes("unauthorized") || msg.includes("forbidden")) { + return buildParsedError("invalid_credentials"); + } + if (msg.includes("rate limit") || msg.includes("too many")) { + return buildParsedError("rate_limited"); + } + if ( + msg.includes("internal server") || + msg.includes("service unavailable") || + msg.includes("bad gateway") || + msg.includes("gateway timeout") + ) { + return buildParsedError("server_error"); + } + if (msg.includes("session") && msg.includes("expired")) { + return buildParsedError("session_expired"); + } + if (msg.includes("network") || msg.includes("connection")) { + return buildParsedError("network_error"); + } + + return buildParsedError("unknown"); + } + + // 3. HTTP response-like objects (e.g. { status: 429 }) + if (isHttpResponseLike(error)) { + return buildParsedError(httpStatusToCode(error.status)); + } + + // 4. Plain string matching a known error code (e.g. from URL query params) + if (typeof error === "string") { + if (isAuthErrorCode(error)) { + return buildParsedError(error); + } + return buildParsedError("unknown"); + } + + // 5. Fallback + return buildParsedError("unknown"); +} + +/** + * Look up the PDA-friendly message for a given {@link AuthErrorCode}. + * Returns the `unknown` message for any unrecognised code. + */ +export function getErrorMessage(code: AuthErrorCode): string { + return ERROR_MESSAGES[code]; +} diff --git a/apps/web/src/lib/auth/fetch-with-retry.test.ts b/apps/web/src/lib/auth/fetch-with-retry.test.ts new file mode 100644 index 0000000..fb9f7ac --- /dev/null +++ b/apps/web/src/lib/auth/fetch-with-retry.test.ts @@ -0,0 +1,347 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; + +/** Recorded delays passed to the mocked sleep function. */ +const recordedDelays: number[] = []; + +// Mock the sleep module to resolve instantly and record delays +vi.mock("./sleep", () => ({ + sleep: vi.fn((ms: number): Promise => { + recordedDelays.push(ms); + return Promise.resolve(); + }), +})); + +import { fetchWithRetry } from "./fetch-with-retry"; +import { sleep } from "./sleep"; + +/** + * Helper: create a minimal Response object for mocking fetch. + */ +function mockResponse(status: number, ok?: boolean): Response { + return { + ok: ok ?? (status >= 200 && status < 300), + status, + statusText: status === 200 ? "OK" : "Error", + headers: new Headers(), + redirected: false, + type: "basic" as ResponseType, + url: "", + body: null, + bodyUsed: false, + clone: vi.fn() as unknown as () => Response, + arrayBuffer: vi.fn() as unknown as () => Promise, + blob: vi.fn() as unknown as () => Promise, + formData: vi.fn() as unknown as () => Promise, + json: vi.fn() as unknown as () => Promise, + text: vi.fn() as unknown as () => Promise, + bytes: vi.fn() as unknown as () => Promise, + } as Response; +} + +describe("fetchWithRetry", (): void => { + const originalFetch = global.fetch; + const sleepMock = vi.mocked(sleep); + + beforeEach((): void => { + global.fetch = vi.fn(); + recordedDelays.length = 0; + sleepMock.mockClear(); + }); + + afterEach((): void => { + vi.restoreAllMocks(); + global.fetch = originalFetch; + }); + + it("should succeed on first attempt without retrying", async (): Promise => { + const okResponse = mockResponse(200); + vi.mocked(global.fetch).mockResolvedValueOnce(okResponse); + + const result = await fetchWithRetry("https://api.example.com/auth/config"); + + expect(result).toBe(okResponse); + expect(global.fetch).toHaveBeenCalledTimes(1); + expect(sleepMock).not.toHaveBeenCalled(); + }); + + it("should retry on network error and succeed on 2nd attempt", async (): Promise => { + const okResponse = mockResponse(200); + vi.mocked(global.fetch) + .mockRejectedValueOnce(new TypeError("Failed to fetch")) + .mockResolvedValueOnce(okResponse); + + const result = await fetchWithRetry("https://api.example.com/auth/config"); + + expect(result).toBe(okResponse); + expect(global.fetch).toHaveBeenCalledTimes(2); + expect(sleepMock).toHaveBeenCalledTimes(1); + }); + + it("should retry on server error (500) and succeed on 3rd attempt", async (): Promise => { + const serverError = mockResponse(500); + const okResponse = mockResponse(200); + + vi.mocked(global.fetch) + .mockResolvedValueOnce(serverError) + .mockResolvedValueOnce(serverError) + .mockResolvedValueOnce(okResponse); + + const result = await fetchWithRetry("https://api.example.com/auth/config"); + + expect(result).toBe(okResponse); + expect(global.fetch).toHaveBeenCalledTimes(3); + expect(sleepMock).toHaveBeenCalledTimes(2); + }); + + it("should give up after maxRetries and throw the last error", async (): Promise => { + const networkError = new TypeError("Failed to fetch"); + vi.mocked(global.fetch) + .mockRejectedValueOnce(networkError) + .mockRejectedValueOnce(networkError) + .mockRejectedValueOnce(networkError) + .mockRejectedValueOnce(networkError); + + await expect( + fetchWithRetry("https://api.example.com/auth/config", undefined, { + maxRetries: 3, + baseDelayMs: 1000, + }) + ).rejects.toThrow("Failed to fetch"); + + // 1 initial + 3 retries = 4 total attempts + expect(global.fetch).toHaveBeenCalledTimes(4); + // Sleep called for the 3 retries (not after the final failure) + expect(sleepMock).toHaveBeenCalledTimes(3); + }); + + it("should NOT retry on non-retryable errors (401)", async (): Promise => { + const unauthorizedResponse = mockResponse(401); + vi.mocked(global.fetch).mockResolvedValueOnce(unauthorizedResponse); + + const result = await fetchWithRetry("https://api.example.com/auth/config"); + + expect(result).toBe(unauthorizedResponse); + expect(global.fetch).toHaveBeenCalledTimes(1); + expect(sleepMock).not.toHaveBeenCalled(); + }); + + it("should NOT retry on non-retryable errors (403)", async (): Promise => { + const forbiddenResponse = mockResponse(403); + vi.mocked(global.fetch).mockResolvedValueOnce(forbiddenResponse); + + const result = await fetchWithRetry("https://api.example.com/auth/config"); + + expect(result).toBe(forbiddenResponse); + expect(global.fetch).toHaveBeenCalledTimes(1); + expect(sleepMock).not.toHaveBeenCalled(); + }); + + it("should NOT retry on non-retryable errors (429)", async (): Promise => { + const rateLimitedResponse = mockResponse(429); + vi.mocked(global.fetch).mockResolvedValueOnce(rateLimitedResponse); + + const result = await fetchWithRetry("https://api.example.com/auth/config"); + + expect(result).toBe(rateLimitedResponse); + expect(global.fetch).toHaveBeenCalledTimes(1); + expect(sleepMock).not.toHaveBeenCalled(); + }); + + it("should respect custom maxRetries option", async (): Promise => { + const networkError = new TypeError("Failed to fetch"); + vi.mocked(global.fetch).mockRejectedValueOnce(networkError).mockRejectedValueOnce(networkError); + + await expect( + fetchWithRetry("https://api.example.com/auth/config", undefined, { + maxRetries: 1, + baseDelayMs: 50, + }) + ).rejects.toThrow("Failed to fetch"); + + // 1 initial + 1 retry = 2 total attempts + expect(global.fetch).toHaveBeenCalledTimes(2); + expect(sleepMock).toHaveBeenCalledTimes(1); + }); + + it("should respect custom baseDelayMs option", async (): Promise => { + const okResponse = mockResponse(200); + vi.mocked(global.fetch) + .mockRejectedValueOnce(new TypeError("Failed to fetch")) + .mockResolvedValueOnce(okResponse); + + await fetchWithRetry("https://api.example.com/auth/config", undefined, { + baseDelayMs: 500, + }); + + // First retry delay should be 500ms (baseDelayMs * 2^0) + expect(recordedDelays[0]).toBe(500); + }); + + it("should use exponential backoff with doubling delays (1s, 2s, 4s)", async (): Promise => { + const networkError = new TypeError("Failed to fetch"); + const okResponse = mockResponse(200); + + vi.mocked(global.fetch) + .mockRejectedValueOnce(networkError) + .mockRejectedValueOnce(networkError) + .mockRejectedValueOnce(networkError) + .mockResolvedValueOnce(okResponse); + + const result = await fetchWithRetry("https://api.example.com/auth/config", undefined, { + baseDelayMs: 1000, + backoffFactor: 2, + }); + + expect(result).toBe(okResponse); + expect(global.fetch).toHaveBeenCalledTimes(4); + + // Verify exponential backoff: 1000 * 2^0, 1000 * 2^1, 1000 * 2^2 + expect(recordedDelays).toEqual([1000, 2000, 4000]); + }); + + it("should log retry attempts in all environments", async (): Promise => { + const warnSpy = vi.spyOn(console, "warn").mockReturnValue(undefined); + + const okResponse = mockResponse(200); + vi.mocked(global.fetch) + .mockRejectedValueOnce(new TypeError("Failed to fetch")) + .mockResolvedValueOnce(okResponse); + + await fetchWithRetry("https://api.example.com/auth/config"); + + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("[Auth] Retry 1/3")); + + warnSpy.mockRestore(); + }); + + it("should log retry attempts for HTTP errors", async (): Promise => { + const warnSpy = vi.spyOn(console, "warn").mockReturnValue(undefined); + + const serverError = mockResponse(500); + const okResponse = mockResponse(200); + + vi.mocked(global.fetch).mockResolvedValueOnce(serverError).mockResolvedValueOnce(okResponse); + + await fetchWithRetry("https://api.example.com/auth/config"); + + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining("[Auth] Retry 1/3 after HTTP 500") + ); + + warnSpy.mockRestore(); + }); + + it("should forward RequestInit options to fetch", async (): Promise => { + const okResponse = mockResponse(200); + vi.mocked(global.fetch).mockResolvedValueOnce(okResponse); + + const requestOptions: RequestInit = { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ token: "abc" }), + }; + + await fetchWithRetry("https://api.example.com/auth/config", requestOptions); + + expect(global.fetch).toHaveBeenCalledWith( + "https://api.example.com/auth/config", + requestOptions + ); + }); + + it("should not retry on non-retryable thrown errors", async (): Promise => { + // An Error that parseAuthError classifies as non-retryable (e.g., "Unauthorized") + const nonRetryableError = new Error("Unauthorized"); + vi.mocked(global.fetch).mockRejectedValueOnce(nonRetryableError); + + await expect(fetchWithRetry("https://api.example.com/auth/config")).rejects.toThrow( + "Unauthorized" + ); + + expect(global.fetch).toHaveBeenCalledTimes(1); + expect(sleepMock).not.toHaveBeenCalled(); + }); + + it("should return last non-ok response when server errors exhaust retries", async (): Promise => { + const serverError = mockResponse(500); + vi.mocked(global.fetch) + .mockResolvedValueOnce(serverError) + .mockResolvedValueOnce(serverError) + .mockResolvedValueOnce(serverError); + + const result = await fetchWithRetry("https://api.example.com/auth/config", undefined, { + maxRetries: 2, + }); + + expect(result.status).toBe(500); + // 1 initial + 2 retries = 3 total attempts + expect(global.fetch).toHaveBeenCalledTimes(3); + expect(sleepMock).toHaveBeenCalledTimes(2); + }); + + describe("RetryOptions value clamping", (): void => { + it("should clamp negative maxRetries to 0 (no retries)", async (): Promise => { + const serverError = mockResponse(500); + vi.mocked(global.fetch).mockResolvedValueOnce(serverError); + + const result = await fetchWithRetry("https://api.example.com/auth/config", undefined, { + maxRetries: -5, + }); + + // maxRetries clamped to 0 means only the initial attempt, no retries + expect(result.status).toBe(500); + expect(global.fetch).toHaveBeenCalledTimes(1); + expect(sleepMock).not.toHaveBeenCalled(); + }); + + it("should clamp fractional maxRetries by flooring", async (): Promise => { + const networkError = new TypeError("Failed to fetch"); + const okResponse = mockResponse(200); + vi.mocked(global.fetch).mockRejectedValueOnce(networkError).mockResolvedValueOnce(okResponse); + + const result = await fetchWithRetry("https://api.example.com/auth/config", undefined, { + maxRetries: 1.9, + baseDelayMs: 100, + }); + + // 1.9 floors to 1, so 1 initial + 1 retry = 2 attempts + expect(result).toBe(okResponse); + expect(global.fetch).toHaveBeenCalledTimes(2); + expect(sleepMock).toHaveBeenCalledTimes(1); + }); + + it("should clamp baseDelayMs below 100 up to 100", async (): Promise => { + const okResponse = mockResponse(200); + vi.mocked(global.fetch) + .mockRejectedValueOnce(new TypeError("Failed to fetch")) + .mockResolvedValueOnce(okResponse); + + await fetchWithRetry("https://api.example.com/auth/config", undefined, { + baseDelayMs: 0, + }); + + // baseDelayMs clamped to 100, so first retry delay = 100 * 2^0 = 100 + expect(recordedDelays[0]).toBe(100); + }); + + it("should clamp backoffFactor below 1 up to 1 (linear delay)", async (): Promise => { + const networkError = new TypeError("Failed to fetch"); + const okResponse = mockResponse(200); + vi.mocked(global.fetch) + .mockRejectedValueOnce(networkError) + .mockRejectedValueOnce(networkError) + .mockResolvedValueOnce(okResponse); + + await fetchWithRetry("https://api.example.com/auth/config", undefined, { + maxRetries: 3, + baseDelayMs: 200, + backoffFactor: 0, + }); + + // backoffFactor clamped to 1, so delays are 200*1^0=200, 200*1^1=200 (constant) + expect(recordedDelays).toEqual([200, 200]); + }); + }); +}); diff --git a/apps/web/src/lib/auth/fetch-with-retry.ts b/apps/web/src/lib/auth/fetch-with-retry.ts new file mode 100644 index 0000000..db2f26b --- /dev/null +++ b/apps/web/src/lib/auth/fetch-with-retry.ts @@ -0,0 +1,119 @@ +/** + * Fetch wrapper with automatic retry and exponential backoff for auth requests. + * + * Only retries errors classified as retryable by {@link parseAuthError}: + * `network_error` and `server_error`. Non-retryable errors (401, 403, 429, etc.) + * are returned or thrown immediately without retry. + */ + +import { parseAuthError } from "./auth-errors"; +import { sleep } from "./sleep"; + +export interface RetryOptions { + /** Maximum number of retries after the initial attempt. Default: 3. */ + maxRetries?: number; + /** Base delay in milliseconds before the first retry. Default: 1000. */ + baseDelayMs?: number; + /** Multiplicative factor applied to the delay after each retry. Default: 2. */ + backoffFactor?: number; +} + +const DEFAULT_MAX_RETRIES = 3; +const DEFAULT_BASE_DELAY_MS = 1000; +const DEFAULT_BACKOFF_FACTOR = 2; + +/** + * Compute the backoff delay for a given retry attempt. + * + * @param attempt - Zero-based retry index (0 = first retry) + * @param baseDelayMs - Starting delay in milliseconds + * @param backoffFactor - Multiplicative factor per retry + * @returns Delay in milliseconds + */ +function computeDelay(attempt: number, baseDelayMs: number, backoffFactor: number): number { + return baseDelayMs * Math.pow(backoffFactor, attempt); +} + +/** + * Fetch a URL with automatic retries and exponential backoff for retryable errors. + * + * - Network errors (fetch throws `TypeError`) are retried if classified as retryable. + * - HTTP error responses (e.g. 500, 502, 503) are retried if the status maps to a + * retryable error code. + * - Non-retryable errors (401, 403, 429) are returned or thrown immediately. + * - On exhausted retries for network errors, the last error is re-thrown. + * - On exhausted retries for HTTP errors, the last response is returned. + * + * @param url - The URL to fetch + * @param options - Standard `RequestInit` options forwarded to `fetch` + * @param retryOptions - Controls retry behaviour (max retries, delay, backoff) + * @returns The successful (or final non-retryable) `Response` + */ +export async function fetchWithRetry( + url: string, + options?: RequestInit, + retryOptions?: RetryOptions +): Promise { + const maxRetries = Math.max(0, Math.floor(retryOptions?.maxRetries ?? DEFAULT_MAX_RETRIES)); + const baseDelayMs = Math.max(100, retryOptions?.baseDelayMs ?? DEFAULT_BASE_DELAY_MS); + const backoffFactor = Math.max(1, retryOptions?.backoffFactor ?? DEFAULT_BACKOFF_FACTOR); + + let lastError: unknown = null; + let lastResponse: Response | null = null; + + for (let attempt = 0; attempt <= maxRetries; attempt++) { + try { + const response = await fetch(url, options); + + if (response.ok) { + return response; + } + + // Non-ok response: check if we should retry based on status code + const parsed = parseAuthError({ status: response.status }); + + if (!parsed.retryable || attempt === maxRetries) { + return response; + } + + // Retryable HTTP error with retries remaining + lastResponse = response; + const delay = computeDelay(attempt, baseDelayMs, backoffFactor); + + console.warn( + `[Auth] Retry ${String(attempt + 1)}/${String(maxRetries)} after HTTP ${String(response.status)}, waiting ${String(delay)}ms...` + ); + + await sleep(delay); + } catch (error: unknown) { + const parsed = parseAuthError(error); + + if (!parsed.retryable || attempt === maxRetries) { + throw error; + } + + // Retryable thrown error with retries remaining + lastError = error; + const delay = computeDelay(attempt, baseDelayMs, backoffFactor); + + console.warn( + `[Auth] Retry ${String(attempt + 1)}/${String(maxRetries)} after ${parsed.code}, waiting ${String(delay)}ms...` + ); + + await sleep(delay); + } + } + + // Should not be reached due to the loop logic, but satisfy TypeScript + if (lastError) { + if (lastError instanceof Error) { + throw lastError; + } + throw new Error("fetchWithRetry: retries exhausted after non-Error failure"); + } + if (lastResponse) { + return lastResponse; + } + + throw new Error("fetchWithRetry: unexpected state"); +} diff --git a/apps/web/src/lib/auth/sleep.ts b/apps/web/src/lib/auth/sleep.ts new file mode 100644 index 0000000..3e717d2 --- /dev/null +++ b/apps/web/src/lib/auth/sleep.ts @@ -0,0 +1,9 @@ +/** + * Wait for the specified number of milliseconds. + * + * Extracted to a separate module to enable clean test mocking + * without fake timers. + */ +export function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} diff --git a/apps/web/src/providers/ThemeProvider.test.tsx b/apps/web/src/providers/ThemeProvider.test.tsx new file mode 100644 index 0000000..5d592b5 --- /dev/null +++ b/apps/web/src/providers/ThemeProvider.test.tsx @@ -0,0 +1,120 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { render, screen, act } from "@testing-library/react"; +import { ThemeProvider, useTheme } from "./ThemeProvider"; + +function ThemeConsumer(): React.JSX.Element { + const { theme, resolvedTheme, setTheme, toggleTheme } = useTheme(); + return ( +
+ {theme} + {resolvedTheme} + + + +
+ ); +} + +describe("ThemeProvider", (): void => { + let mockMatchMedia: ReturnType; + + beforeEach((): void => { + localStorage.clear(); + document.documentElement.classList.remove("light", "dark"); + + mockMatchMedia = vi.fn().mockReturnValue({ + matches: false, + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + }); + Object.defineProperty(window, "matchMedia", { + writable: true, + value: mockMatchMedia, + }); + }); + + afterEach((): void => { + vi.restoreAllMocks(); + }); + + it("should use 'mosaic-theme' as storage key", (): void => { + localStorage.setItem("mosaic-theme", "light"); + + render( + + + + ); + + expect(screen.getByTestId("theme")).toHaveTextContent("light"); + }); + + it("should NOT read from old 'jarvis-theme' storage key", (): void => { + localStorage.setItem("jarvis-theme", "light"); + + render( + + + + ); + + // Should default to system, not read from jarvis-theme + expect(screen.getByTestId("theme")).toHaveTextContent("system"); + }); + + it("should store theme under 'mosaic-theme' key", (): void => { + render( + + + + ); + + act(() => { + screen.getByText("Set Light").click(); + }); + + expect(localStorage.getItem("mosaic-theme")).toBe("light"); + expect(localStorage.getItem("jarvis-theme")).toBeNull(); + }); + + it("should render children", (): void => { + render( + +
Hello
+ + ); + + expect(screen.getByTestId("child")).toBeInTheDocument(); + }); + + it("should throw when useTheme is used outside provider", (): void => { + // Suppress console.error for expected error + const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => { + // Intentionally empty + }); + + expect(() => { + render(); + }).toThrow("useTheme must be used within a ThemeProvider"); + + consoleSpy.mockRestore(); + }); +}); diff --git a/apps/web/src/providers/ThemeProvider.tsx b/apps/web/src/providers/ThemeProvider.tsx index d199ece..623e7fb 100644 --- a/apps/web/src/providers/ThemeProvider.tsx +++ b/apps/web/src/providers/ThemeProvider.tsx @@ -13,7 +13,7 @@ interface ThemeContextValue { const ThemeContext = createContext(null); -const STORAGE_KEY = "jarvis-theme"; +const STORAGE_KEY = "mosaic-theme"; function getSystemTheme(): "light" | "dark" { if (typeof window === "undefined") return "dark"; diff --git a/docker-compose.swarm.portainer.yml b/docker-compose.swarm.portainer.yml index 7ad1e55..a544963 100644 --- a/docker-compose.swarm.portainer.yml +++ b/docker-compose.swarm.portainer.yml @@ -293,7 +293,7 @@ services: OIDC_ISSUER: ${OIDC_ISSUER} OIDC_CLIENT_ID: ${OIDC_CLIENT_ID} OIDC_CLIENT_SECRET: ${OIDC_CLIENT_SECRET} - OIDC_REDIRECT_URI: ${OIDC_REDIRECT_URI:-http://localhost:3001/auth/callback} + OIDC_REDIRECT_URI: ${OIDC_REDIRECT_URI:-} JWT_SECRET: ${JWT_SECRET:-change-this-to-a-random-secret} JWT_EXPIRATION: ${JWT_EXPIRATION:-24h} BETTER_AUTH_SECRET: ${BETTER_AUTH_SECRET} diff --git a/docs/tasks.md b/docs/tasks.md index 9b1d00c..f6a3083 100644 --- a/docs/tasks.md +++ b/docs/tasks.md @@ -156,3 +156,161 @@ | ----------- | ------ | ----------------------------------------------------------------------- | ----- | ---- | --------------------------- | --------------------------------------- | ----------- | --------- | ----------------- | ----------------- | -------- | ---- | ----------------- | | SP-E2E-001 | done | #405: E2E integration tests for speech services | #405 | api | feature/m13-speech-services | SP-EP-001,SP-EP-002,SP-WS-001,SP-FE-003 | SP-DOCS-001 | worker-17 | 2026-02-15T07:23Z | 2026-02-15T07:32Z | 25K | 35K | 30 tests, d2c7602 | | SP-DOCS-001 | done | #406: Documentation - Speech services architecture, API, and deployment | #406 | docs | feature/m13-speech-services | SP-E2E-001 | | worker-18 | 2026-02-15T07:23Z | 2026-02-15T07:29Z | 15K | 35K | 24065aa | + +--- + +## Auth-Frontend-Remediation (<0.1.0) — Auth & Frontend Remediation + +**Orchestrator:** Claude Code +**Started:** 2026-02-16 +**Branch:** fix/auth-frontend-remediation +**Milestone:** Auth-Frontend-Remediation (<0.1.0) +**Epic:** #411 + +### Phase 1: Critical Backend Fixes (#412) + +| id | status | description | issue | repo | branch | depends_on | blocks | agent | started_at | completed_at | estimate | used | +| -------- | ------ | ----------------------------------------------------------------- | ----- | ------ | ----------------------------- | -------------------------------------------- | -------- | ----- | ----------------- | ----------------- | -------- | ---- | +| AUTH-001 | done | 1.1: Add OIDC_REDIRECT_URI to validation with URL + path checks | #412 | api | fix/auth-frontend-remediation | | AUTH-002 | w-1 | 2026-02-16T11:00Z | 2026-02-16T11:04Z | 10K | 12K | +| AUTH-002 | done | 1.2: Wrap BetterAuth handler in try/catch with error logging | #412 | api | fix/auth-frontend-remediation | AUTH-001 | | w-3 | 2026-02-16T11:05Z | 2026-02-16T11:09Z | 10K | 15K | +| AUTH-003 | done | 1.3: Fix docker-compose OIDC_REDIRECT_URI default | #412 | devops | fix/auth-frontend-remediation | | | w-2 | 2026-02-16T11:00Z | 2026-02-16T11:05Z | 3K | 5K | +| AUTH-004 | done | 1.4: Enable PKCE in genericOAuth config | #412 | api | fix/auth-frontend-remediation | | | w-2 | 2026-02-16T11:00Z | 2026-02-16T11:05Z | 5K | 5K | +| AUTH-005 | done | 1.5: Add @SkipCsrf() documentation with BetterAuth CSRF rationale | #412 | api | fix/auth-frontend-remediation | | | w-2 | 2026-02-16T11:00Z | 2026-02-16T11:05Z | 3K | 5K | +| AUTH-V01 | done | Phase 1 verification: quality gates pass | #412 | all | fix/auth-frontend-remediation | AUTH-001,AUTH-002,AUTH-003,AUTH-004,AUTH-005 | AUTH-006 | orch | 2026-02-16T11:10Z | 2026-02-16T11:10Z | 5K | 2K | + +### Phase 2: Auth Config Discovery (#413) + +| id | status | description | issue | repo | branch | depends_on | blocks | agent | started_at | completed_at | estimate | used | +| -------- | ------ | -------------------------------------------------------------------- | ----- | ------ | ----------------------------- | ----------------------------------- | -------- | ----- | ----------------- | ----------------- | -------- | ---- | +| AUTH-006 | done | 2.1: Add AuthProvider and AuthConfigResponse types to @mosaic/shared | #413 | shared | fix/auth-frontend-remediation | AUTH-V01 | AUTH-007 | w-4 | 2026-02-16T11:12Z | 2026-02-16T11:13Z | 5K | 3K | +| AUTH-007 | done | 2.2-2.3: Implement getAuthConfig() + GET /auth/config endpoint | #413 | api | fix/auth-frontend-remediation | AUTH-006 | AUTH-008 | w-5 | 2026-02-16T11:13Z | 2026-02-16T11:17Z | 15K | 15K | +| AUTH-008 | done | 2.4: Add secret-leakage prevention test | #413 | api | fix/auth-frontend-remediation | AUTH-007 | AUTH-009 | w-6 | 2026-02-16T11:18Z | 2026-02-16T11:20Z | 8K | 8K | +| AUTH-009 | done | 2.5: Implement isOidcProviderReachable() health check | #413 | api | fix/auth-frontend-remediation | AUTH-007 | | w-7 | 2026-02-16T11:18Z | 2026-02-16T11:23Z | 10K | 12K | +| AUTH-V02 | done | Phase 2 verification: quality gates pass | #413 | all | fix/auth-frontend-remediation | AUTH-006,AUTH-007,AUTH-008,AUTH-009 | AUTH-010 | orch | 2026-02-16T11:24Z | 2026-02-16T11:25Z | 5K | 2K | + +### Phase 3: Backend Hardening (#414) + +| id | status | description | issue | repo | branch | depends_on | blocks | agent | started_at | completed_at | estimate | used | +| -------- | ------ | ---------------------------------------------------------------- | ----- | ------ | ----------------------------- | ----------------------------------- | -------- | ----- | ----------------- | ----------------- | -------- | ---- | +| AUTH-010 | done | 3.1: Extract trustedOrigins to getTrustedOrigins() with env vars | #414 | api | fix/auth-frontend-remediation | AUTH-V02 | AUTH-011 | w-8 | 2026-02-16T11:26Z | 2026-02-16T11:31Z | 10K | 15K | +| AUTH-011 | done | 3.2: Align CORS config in main.ts with getTrustedOrigins() | #414 | api | fix/auth-frontend-remediation | AUTH-010 | | w-10 | 2026-02-16T11:32Z | 2026-02-16T11:33Z | 8K | 8K | +| AUTH-012 | done | 3.3: Update session config (7d abs, 2h idle, cookie attrs) | #414 | api | fix/auth-frontend-remediation | AUTH-V02 | | w-9 | 2026-02-16T11:26Z | 2026-02-16T11:29Z | 8K | 8K | +| AUTH-013 | done | 3.4: Add TRUSTED_ORIGINS, COOKIE_DOMAIN to .env.example | #414 | devops | fix/auth-frontend-remediation | AUTH-010 | | w-11 | 2026-02-16T11:32Z | 2026-02-16T11:33Z | 3K | 3K | +| AUTH-V03 | done | Phase 3 verification: quality gates pass | #414 | all | fix/auth-frontend-remediation | AUTH-010,AUTH-011,AUTH-012,AUTH-013 | AUTH-014 | orch | 2026-02-16T11:34Z | 2026-02-16T11:34Z | 5K | 2K | + +### Phase 4: Frontend Foundation (#415) + +| id | status | description | issue | repo | branch | depends_on | blocks | agent | started_at | completed_at | estimate | used | +| -------- | ------ | ---------------------------------------------------------------- | ----- | ---- | ----------------------------- | ----------------------------------------------------- | -------- | ----- | ----------------- | ----------------- | -------- | ---- | +| AUTH-014 | done | 4.1: Fix theme storage key (jarvis-theme -> mosaic-theme) | #415 | web | fix/auth-frontend-remediation | AUTH-V03 | | w-12 | 2026-02-16T11:35Z | 2026-02-16T11:44Z | 5K | 5K | +| AUTH-015 | done | 4.2: Create AuthErrorBanner component (PDA-friendly, blue theme) | #415 | web | fix/auth-frontend-remediation | AUTH-V03 | AUTH-020 | w-13 | 2026-02-16T11:35Z | 2026-02-16T11:44Z | 12K | 12K | +| AUTH-016 | done | 4.3: Create AuthDivider component | #415 | web | fix/auth-frontend-remediation | AUTH-V03 | AUTH-020 | w-12 | 2026-02-16T11:35Z | 2026-02-16T11:44Z | 5K | 5K | +| AUTH-017 | done | 4.4: Create OAuthButton component (replaces LoginButton) | #415 | web | fix/auth-frontend-remediation | AUTH-V03 | AUTH-020 | w-13 | 2026-02-16T11:35Z | 2026-02-16T11:44Z | 12K | 12K | +| AUTH-018 | done | 4.5: Create LoginForm component with email/password validation | #415 | web | fix/auth-frontend-remediation | AUTH-V03 | AUTH-020 | w-13 | 2026-02-16T11:35Z | 2026-02-16T11:44Z | 15K | 15K | +| AUTH-019 | done | 4.6: Create SessionExpiryWarning component | #415 | web | fix/auth-frontend-remediation | AUTH-V03 | AUTH-025 | w-12 | 2026-02-16T11:35Z | 2026-02-16T11:44Z | 10K | 10K | +| AUTH-V04 | done | Phase 4 verification: quality gates pass | #415 | all | fix/auth-frontend-remediation | AUTH-014,AUTH-015,AUTH-016,AUTH-017,AUTH-018,AUTH-019 | AUTH-020 | orch | 2026-02-16T11:45Z | 2026-02-16T11:45Z | 5K | 2K | + +### Phase 5: Login Page Integration (#416) + +| id | status | description | issue | repo | branch | depends_on | blocks | agent | started_at | completed_at | estimate | used | +| -------- | ------ | ------------------------------------------------------------ | ----- | ---- | ----------------------------- | ----------------------------------- | -------- | ----- | ----------------- | ----------------- | -------- | ---- | +| AUTH-020 | done | 5.1-5.2: Fetch /auth/config and render providers dynamically | #416 | web | fix/auth-frontend-remediation | AUTH-V04,AUTH-V02 | AUTH-021 | w-14 | 2026-02-16T11:46Z | 2026-02-16T11:52Z | 20K | 15K | +| AUTH-021 | done | 5.3-5.4: Error display from query params + loading states | #416 | web | fix/auth-frontend-remediation | AUTH-020 | AUTH-022 | w-15 | 2026-02-16T11:53Z | 2026-02-16T11:57Z | 12K | 12K | +| AUTH-022 | done | 5.5: Delete old LoginButton.tsx and update imports | #416 | web | fix/auth-frontend-remediation | AUTH-020 | | w-16 | 2026-02-16T11:53Z | 2026-02-16T11:54Z | 5K | 4K | +| AUTH-023 | done | 5.6-5.7: Responsive layout + accessibility audit | #416 | web | fix/auth-frontend-remediation | AUTH-020,AUTH-021 | | w-17 | 2026-02-16T11:58Z | 2026-02-16T12:03Z | 12K | 25K | +| AUTH-V05 | done | Phase 5 verification: quality gates pass | #416 | all | fix/auth-frontend-remediation | AUTH-020,AUTH-021,AUTH-022,AUTH-023 | AUTH-024 | orch | 2026-02-16T12:04Z | 2026-02-16T12:04Z | 5K | 2K | + +### Phase 6: Error Recovery & Polish (#417) + +| id | status | description | issue | repo | branch | depends_on | blocks | agent | started_at | completed_at | estimate | used | +| -------- | ------ | ------------------------------------------------------------------- | ----- | ---- | ----------------------------- | ----------------------------------- | -------- | ----- | ----------------- | ----------------- | -------- | ---- | +| AUTH-024 | done | 6.1: Create auth-errors.ts with PDA error parsing and mapping | #417 | web | fix/auth-frontend-remediation | AUTH-V05 | AUTH-025 | w-18 | 2026-02-16T12:10Z | 2026-02-16T12:15Z | 12K | 12K | +| AUTH-025 | done | 6.2: Add retry logic for network errors (3x exponential backoff) | #417 | web | fix/auth-frontend-remediation | AUTH-V05 | | w-20 | 2026-02-16T12:16Z | 2026-02-16T12:22Z | 10K | 15K | +| AUTH-026 | done | 6.3-6.4: AuthProvider session-expiring state + SessionExpiryWarning | #417 | web | fix/auth-frontend-remediation | AUTH-V05,AUTH-019 | | w-19 | 2026-02-16T12:10Z | 2026-02-16T12:15Z | 15K | 20K | +| AUTH-027 | done | 6.5: Update auth-client.ts error messages to PDA-friendly | #417 | web | fix/auth-frontend-remediation | AUTH-024 | | w-21 | 2026-02-16T12:16Z | 2026-02-16T12:18Z | 8K | 10K | +| AUTH-V06 | done | Phase 6 verification: quality gates pass | #417 | all | fix/auth-frontend-remediation | AUTH-024,AUTH-025,AUTH-026,AUTH-027 | | orch | 2026-02-16T12:23Z | 2026-02-16T12:24Z | 5K | 2K | + +### Phase 7: Review Remediation (#411) + +| id | status | description | issue | repo | branch | depends_on | blocks | agent | started_at | completed_at | estimate | used | +| -------- | ------ | --------------------------------------------------------------------------------- | ----- | ---- | ----------------------------- | ----------------- | -------- | ----- | ----------------- | ----------------- | -------- | ---- | +| AUTH-028 | done | 7.1: Frontend fixes — wire fetchWithRetry, dedupe errors, fix OAuth/catch/signout | #411 | web | fix/auth-frontend-remediation | AUTH-V06 | AUTH-030 | w-22 | 2026-02-16T18:29Z | 2026-02-16T18:33Z | 20K | 15K | +| AUTH-029 | done | 7.2: Backend fixes — COOKIE_DOMAIN, TRUSTED_ORIGINS validation, verifySession | #411 | api | fix/auth-frontend-remediation | AUTH-V06 | AUTH-030 | w-23 | 2026-02-16T18:29Z | 2026-02-16T18:31Z | 15K | 12K | +| AUTH-030 | done | 7.3: Missing tests — getAccessToken, isAdmin, null cases, getClientIp | #411 | all | fix/auth-frontend-remediation | AUTH-028,AUTH-029 | AUTH-V07 | w-24 | 2026-02-16T18:34Z | 2026-02-16T18:37Z | 15K | 15K | +| AUTH-V07 | done | Phase 7 verification: 191 web + 106 API tests passing | #411 | all | fix/auth-frontend-remediation | AUTH-030 | | orch | 2026-02-16T18:37Z | 2026-02-16T18:38Z | 5K | 2K | + +### Phase 8: QA Remediation — Backend Error Handling (#411) + +| id | status | description | issue | repo | branch | depends_on | blocks | agent | started_at | completed_at | estimate | used | +| ------ | ------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------- | ----- | ---- | ----------------------------- | --------------------------- | ------------- | ----- | ----------------- | ----------------- | -------- | ---- | +| QA-001 | done | CRITICAL: AuthGuard — let infrastructure errors propagate instead of wrapping as 401 | #411 | api | fix/auth-frontend-remediation | | QA-V08 | w-25 | 2026-02-16T19:00Z | 2026-02-16T19:10Z | 12K | 9K | +| QA-002 | done | CRITICAL+HIGH: verifySession — invert error classification (allowlist auth errors, re-throw everything else) + typed return type + health check escalation | #411 | api | fix/auth-frontend-remediation | | QA-001,QA-V08 | w-26 | 2026-02-16T19:00Z | 2026-02-16T19:15Z | 25K | 8K | +| QA-003 | done | MEDIUM: auth.config.ts — replace null coalescing with throw in getOidcPlugins + include error details in getTrustedOrigins catch | #411 | api | fix/auth-frontend-remediation | | QA-V08 | w-27 | 2026-02-16T19:16Z | 2026-02-16T19:25Z | 10K | 3K | +| QA-004 | done | MEDIUM: auth.controller.ts — use HttpException(401) instead of raw Error in getSession + PDA-friendly handleAuth error message | #411 | api | fix/auth-frontend-remediation | | QA-V08 | w-28 | 2026-02-16T19:16Z | 2026-02-16T19:22Z | 10K | 7K | +| QA-V08 | done | Phase 8 verification: 128 auth tests pass, 2 pre-existing failures (DB/package), no regressions | #411 | all | fix/auth-frontend-remediation | QA-001,QA-002,QA-003,QA-004 | QA-005 | orch | 2026-02-16T19:26Z | 2026-02-16T19:27Z | 5K | 2K | + +### Phase 9: QA Remediation — Frontend Error Handling (#411) + +| id | status | description | issue | repo | branch | depends_on | blocks | agent | started_at | completed_at | estimate | used | +| ------ | ------ | ------------------------------------------------------------------------------------------------------------------------------------------------------- | ----- | ---- | ----------------------------- | --------------------------- | ------------- | ----- | ----------------- | ----------------- | -------- | ---- | +| QA-005 | done | CRITICAL+HIGH: auth-context.tsx — production logging, replace isBackendError with parseAuthError, fix signOut classification, add session-expired state | #411 | web | fix/auth-frontend-remediation | QA-V08 | QA-007,QA-V09 | w-29 | 2026-02-16T19:28Z | 2026-02-16T19:45Z | 25K | 85K | +| QA-006 | done | MEDIUM: auth-client.ts — log JSON parse error in signInWithCredentials + add logging to getAccessToken/isAdmin silent defaults | #411 | web | fix/auth-frontend-remediation | QA-V08 | QA-V09 | w-30 | 2026-02-16T19:28Z | 2026-02-16T19:50Z | 12K | 15K | +| QA-007 | done | HIGH: login/page.tsx — show explicit error state instead of silent email-only fallback when config fetch fails | #411 | web | fix/auth-frontend-remediation | QA-005 | QA-V09 | w-31 | 2026-02-16T19:51Z | 2026-02-16T19:56Z | 15K | 18K | +| QA-008 | done | LOW: auth-errors.ts — derive KNOWN_CODES from Object.keys(ERROR_MESSAGES) to eliminate duplication | #411 | web | fix/auth-frontend-remediation | QA-V08 | QA-V09 | w-32 | 2026-02-16T19:51Z | 2026-02-16T19:53Z | 3K | 4K | +| QA-V09 | done | Phase 9 verification: 194 auth web tests pass, no regressions | #411 | all | fix/auth-frontend-remediation | QA-005,QA-006,QA-007,QA-008 | QA-009 | orch | 2026-02-16T19:57Z | 2026-02-16T19:58Z | 5K | 2K | + +### Phase 10: QA Remediation — Comment & Documentation Fixes (#411) + +| id | status | description | issue | repo | branch | depends_on | blocks | agent | started_at | completed_at | estimate | used | +| ------ | ------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ----- | ------- | ----------------------------- | ------------- | ------ | ----- | ----------------- | ----------------- | -------- | ---- | +| QA-009 | done | CRITICAL: Fix updateAge comment (not idle timeout — it's session refresh throttle), fix .env.example OIDC vars, fix username->email bug in signInWithCredentials | #411 | api,web | fix/auth-frontend-remediation | QA-V09 | QA-V10 | w-33 | 2026-02-16T19:59Z | 2026-02-16T20:05Z | 12K | 12K | +| QA-010 | done | MINOR: Fix JSDoc issues — response.ok is 2xx not "200", remove "Automatic token refresh" claim, remove "Enable for now" comment, fix CSRF comment placement, fix 403 mapping comment | #411 | api,web | fix/auth-frontend-remediation | QA-V09 | QA-V10 | w-34 | 2026-02-16T19:59Z | 2026-02-16T20:03Z | 8K | 8K | +| QA-V10 | done | Phase 10 verification: 71 tests pass, no regressions | #411 | all | fix/auth-frontend-remediation | QA-009,QA-010 | QA-011 | orch | 2026-02-16T20:06Z | 2026-02-16T20:07Z | 5K | 2K | + +### Phase 11: QA Remediation — Type Design Improvements (#411) + +| id | status | description | issue | repo | branch | depends_on | blocks | agent | started_at | completed_at | estimate | used | +| ------ | ------ | ------------------------------------------------------------------------------------------------------------------------------------- | ----- | ---- | ----------------------------- | ------------- | ------ | ----- | ----------------- | ----------------- | -------- | ---- | +| QA-011 | done | HIGH: Unify 4 request-with-user types (RequestWithSession, AuthRequest, BetterAuthRequest, RequestWithUser) into AuthenticatedRequest | #411 | api | fix/auth-frontend-remediation | QA-V10 | QA-V11 | w-35 | 2026-02-16T20:08Z | 2026-02-16T20:16Z | 20K | 15K | +| QA-012 | done | LOW: Add RetryOptions value clamping (maxRetries>=0, baseDelayMs>=100, backoffFactor>=1) | #411 | web | fix/auth-frontend-remediation | QA-V10 | QA-V11 | w-36 | 2026-02-16T20:08Z | 2026-02-16T20:12Z | 5K | 4K | +| QA-V11 | done | Phase 11 verification: 125 tests pass (106 API + 19 web), types compile | #411 | all | fix/auth-frontend-remediation | QA-011,QA-012 | QA-013 | orch | 2026-02-16T20:17Z | 2026-02-16T20:18Z | 5K | 2K | + +### Phase 12: QA Remediation — Test Coverage Gaps (#411) + +| id | status | description | issue | repo | branch | depends_on | blocks | agent | started_at | completed_at | estimate | used | +| ------ | ------ | --------------------------------------------------------------------------------------------------------- | ----- | ---- | ----------------------------- | -------------------- | ------ | ----- | ----------------- | ----------------- | -------- | ---- | +| QA-013 | done | Add signOut failure path test — verify user cleared + authError set to proper type on apiPost rejection | #411 | web | fix/auth-frontend-remediation | QA-V11 | QA-V12 | w-37 | 2026-02-16T20:19Z | 2026-02-16T20:26Z | 10K | 4K | +| QA-014 | done | Add verifySession non-Error thrown value test — verify returns null for string/object throws | #411 | api | fix/auth-frontend-remediation | QA-V11 | QA-V12 | w-38 | 2026-02-16T20:19Z | 2026-02-16T20:23Z | 8K | 4K | +| QA-015 | done | Add handleCredentialsLogin error message fallback test + fix refreshSession test to actually call refresh | #411 | web | fix/auth-frontend-remediation | QA-V11 | QA-V12 | w-39 | 2026-02-16T20:27Z | 2026-02-16T20:30Z | 12K | 7K | +| QA-V12 | done | Phase 12 verification: 309 tests pass (201 web + 108 API) — final quality gate | #411 | all | fix/auth-frontend-remediation | QA-013,QA-014,QA-015 | | orch | 2026-02-16T20:31Z | 2026-02-16T20:32Z | 5K | 2K | + +### Phase 13: QA Round 2 — Backend Hardening (#411) + +| id | status | description | issue | repo | branch | depends_on | blocks | agent | started_at | completed_at | estimate | used | +| ------- | ------ | ----------------------------------------------------------------------------------------------------------------------------------------- | ----- | ---- | ----------------------------- | ----------------------------------------------- | --------------- | ----- | ----------------- | ----------------- | -------- | ---- | +| QA2-001 | done | MEDIUM: Narrow verifySession allowlist — "token expired"/"session expired" instead of bare "expired", exact match "unauthorized" | #411 | api | fix/auth-frontend-remediation | | QA2-003,QA2-V13 | w-40 | 2026-02-16T21:00Z | 2026-02-16T21:02Z | 10K | 4K | +| QA2-002 | done | MEDIUM: Add runtime null checks in auth.controller getSession/getProfile — defense-in-depth for AuthenticatedRequest | #411 | api | fix/auth-frontend-remediation | | QA2-V13 | w-42 | 2026-02-16T21:03Z | 2026-02-16T21:05Z | 8K | 5K | +| QA2-003 | done | MEDIUM: Sanitize Bearer tokens from logged error stacks + add logger.warn for non-Error thrown values in verifySession | #411 | api | fix/auth-frontend-remediation | QA2-001 | QA2-V13 | w-44 | 2026-02-16T21:06Z | 2026-02-16T21:08Z | 8K | 5K | +| QA2-004 | done | MEDIUM: classifyAuthError — map invalid_credentials/session_expired to null instead of "backend" (don't show error banner for normal 401) | #411 | web | fix/auth-frontend-remediation | | QA2-V13 | w-41 | 2026-02-16T21:00Z | 2026-02-16T21:02Z | 10K | 5K | +| QA2-005 | done | MEDIUM: Login page — route BetterAuth result.error.message through parseAuthError for PDA-friendly sanitization | #411 | web | fix/auth-frontend-remediation | | QA2-V13 | w-43 | 2026-02-16T21:03Z | 2026-02-16T21:05Z | 8K | 4K | +| QA2-006 | done | LOW: AuthGuard user validation branch tests — malformed user (missing id/email/name), non-object user, string user | #411 | api | fix/auth-frontend-remediation | | QA2-V13 | w-45 | 2026-02-16T21:06Z | 2026-02-16T21:09Z | 8K | 5K | +| QA2-V13 | done | Phase 13 verification: 272 tests pass (126 web + 146 API), 2 pre-existing failures, no regressions | #411 | all | fix/auth-frontend-remediation | QA2-001,QA2-002,QA2-003,QA2-004,QA2-005,QA2-006 | | orch | 2026-02-16T21:10Z | 2026-02-16T21:12Z | 5K | 2K | + +### Summary + +| Phase | Issue | Tasks | Total Estimate | +| ------------------------------- | ----- | ------ | -------------- | +| 1 - Critical Backend Fixes | #412 | 6 | 36K | +| 2 - Auth Config Discovery | #413 | 5 | 43K | +| 3 - Backend Hardening | #414 | 5 | 34K | +| 4 - Frontend Foundation | #415 | 7 | 64K | +| 5 - Login Page Integration | #416 | 5 | 54K | +| 6 - Error Recovery & Polish | #417 | 5 | 50K | +| 7 - Review Remediation | #411 | 4 | 55K | +| 8 - QA: Backend Error Handling | #411 | 5 | 62K | +| 9 - QA: Frontend Error Handling | #411 | 5 | 60K | +| 10 - QA: Comment Fixes | #411 | 3 | 25K | +| 11 - QA: Type Design | #411 | 3 | 30K | +| 12 - QA: Test Coverage | #411 | 4 | 35K | +| 13 - QA R2: Hardening + Tests | #411 | 7 | 57K | +| **Total** | | **64** | **605K** | diff --git a/packages/shared/src/types/auth.types.ts b/packages/shared/src/types/auth.types.ts index 9e030fe..329fd92 100644 --- a/packages/shared/src/types/auth.types.ts +++ b/packages/shared/src/types/auth.types.ts @@ -94,3 +94,20 @@ export interface OAuthCallbackParams { error?: string; error_description?: string; } + +/** + * Auth provider type advertised by the backend via GET /auth/config + */ +export interface AuthProviderConfig { + id: string; + name: string; + type: "oauth" | "credentials"; +} + +/** + * Response shape for GET /auth/config + * Backend advertises available auth methods for the frontend to render dynamically. + */ +export interface AuthConfigResponse { + providers: AuthProviderConfig[]; +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 1ae2c3e..a21fc33 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -7408,7 +7408,7 @@ snapshots: chalk: 5.6.2 commander: 12.1.0 dotenv: 17.2.4 - drizzle-orm: 0.41.0(@opentelemetry/api@1.9.0)(@prisma/client@6.19.2(prisma@6.19.2(magicast@0.3.5)(typescript@5.9.3))(typescript@5.9.3))(@types/pg@8.16.0)(better-sqlite3@12.6.2)(kysely@0.28.10)(pg@8.17.2)(postgres@3.4.8)(prisma@6.19.2(magicast@0.3.5)(typescript@5.9.3)) + drizzle-orm: 0.41.0(@opentelemetry/api@1.9.0)(@prisma/client@5.22.0(prisma@6.19.2(magicast@0.3.5)(typescript@5.9.3)))(@types/pg@8.16.0)(better-sqlite3@12.6.2)(kysely@0.28.10)(pg@8.17.2)(postgres@3.4.8)(prisma@6.19.2(magicast@0.3.5)(typescript@5.9.3)) open: 10.2.0 pg: 8.17.2 prettier: 3.8.1 @@ -10410,7 +10410,7 @@ snapshots: optionalDependencies: '@prisma/client': 5.22.0(prisma@6.19.2(magicast@0.3.5)(typescript@5.9.3)) better-sqlite3: 12.6.2 - drizzle-orm: 0.41.0(@opentelemetry/api@1.9.0)(@prisma/client@6.19.2(prisma@6.19.2(magicast@0.3.5)(typescript@5.9.3))(typescript@5.9.3))(@types/pg@8.16.0)(better-sqlite3@12.6.2)(kysely@0.28.10)(pg@8.17.2)(postgres@3.4.8)(prisma@6.19.2(magicast@0.3.5)(typescript@5.9.3)) + drizzle-orm: 0.41.0(@opentelemetry/api@1.9.0)(@prisma/client@5.22.0(prisma@6.19.2(magicast@0.3.5)(typescript@5.9.3)))(@types/pg@8.16.0)(better-sqlite3@12.6.2)(kysely@0.28.10)(pg@8.17.2)(postgres@3.4.8)(prisma@6.19.2(magicast@0.3.5)(typescript@5.9.3)) next: 16.1.6(@babel/core@7.28.6)(@opentelemetry/api@1.9.0)(react-dom@19.2.4(react@19.2.4))(react@19.2.4) pg: 8.17.2 prisma: 6.19.2(magicast@0.3.5)(typescript@5.9.3) @@ -10435,7 +10435,7 @@ snapshots: optionalDependencies: '@prisma/client': 6.19.2(prisma@6.19.2(magicast@0.3.5)(typescript@5.9.3))(typescript@5.9.3) better-sqlite3: 12.6.2 - drizzle-orm: 0.41.0(@opentelemetry/api@1.9.0)(@prisma/client@6.19.2(prisma@6.19.2(magicast@0.3.5)(typescript@5.9.3))(typescript@5.9.3))(@types/pg@8.16.0)(better-sqlite3@12.6.2)(kysely@0.28.10)(pg@8.17.2)(postgres@3.4.8)(prisma@6.19.2(magicast@0.3.5)(typescript@5.9.3)) + drizzle-orm: 0.41.0(@opentelemetry/api@1.9.0)(@prisma/client@5.22.0(prisma@6.19.2(magicast@0.3.5)(typescript@5.9.3)))(@types/pg@8.16.0)(better-sqlite3@12.6.2)(kysely@0.28.10)(pg@8.17.2)(postgres@3.4.8)(prisma@6.19.2(magicast@0.3.5)(typescript@5.9.3)) next: 16.1.6(@babel/core@7.28.6)(@opentelemetry/api@1.9.0)(react-dom@19.2.4(react@19.2.4))(react@19.2.4) pg: 8.17.2 prisma: 6.19.2(magicast@0.3.5)(typescript@5.9.3) @@ -11229,6 +11229,17 @@ snapshots: dotenv@17.2.4: {} + drizzle-orm@0.41.0(@opentelemetry/api@1.9.0)(@prisma/client@5.22.0(prisma@6.19.2(magicast@0.3.5)(typescript@5.9.3)))(@types/pg@8.16.0)(better-sqlite3@12.6.2)(kysely@0.28.10)(pg@8.17.2)(postgres@3.4.8)(prisma@6.19.2(magicast@0.3.5)(typescript@5.9.3)): + optionalDependencies: + '@opentelemetry/api': 1.9.0 + '@prisma/client': 5.22.0(prisma@6.19.2(magicast@0.3.5)(typescript@5.9.3)) + '@types/pg': 8.16.0 + better-sqlite3: 12.6.2 + kysely: 0.28.10 + pg: 8.17.2 + postgres: 3.4.8 + prisma: 6.19.2(magicast@0.3.5)(typescript@5.9.3) + drizzle-orm@0.41.0(@opentelemetry/api@1.9.0)(@prisma/client@6.19.2(prisma@6.19.2(magicast@0.3.5)(typescript@5.9.3))(typescript@5.9.3))(@types/pg@8.16.0)(better-sqlite3@12.6.2)(kysely@0.28.10)(pg@8.17.2)(postgres@3.4.8)(prisma@6.19.2(magicast@0.3.5)(typescript@5.9.3)): optionalDependencies: '@opentelemetry/api': 1.9.0 @@ -11239,6 +11250,7 @@ snapshots: pg: 8.17.2 postgres: 3.4.8 prisma: 6.19.2(magicast@0.3.5)(typescript@5.9.3) + optional: true dunder-proto@1.0.1: dependencies: