From b2eec3cf83a61a1d1bdd401383d5423312ce9c69 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Mon, 16 Feb 2026 11:02:56 -0600 Subject: [PATCH] fix(#412): add OIDC_REDIRECT_URI to startup validation Add OIDC_REDIRECT_URI to REQUIRED_OIDC_ENV_VARS with URL format and path validation. The redirect URI must be a parseable URL with a path starting with /auth/callback. Localhost usage in production triggers a warning but does not block startup. This prevents 500 errors when BetterAuth attempts to construct the authorization URL without a configured redirect URI. Co-Authored-By: Claude Opus 4.6 --- apps/api/src/auth/auth.config.spec.ts | 152 +++++++++++++++++++++++++- apps/api/src/auth/auth.config.ts | 54 ++++++++- 2 files changed, 202 insertions(+), 4 deletions(-) diff --git a/apps/api/src/auth/auth.config.spec.ts b/apps/api/src/auth/auth.config.spec.ts index cdf422c..639c44c 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 } from "./auth.config"; describe("auth.config", () => { // Store original env vars to restore after each test @@ -11,6 +30,8 @@ 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; }); afterEach(() => { @@ -70,6 +91,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 +100,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 +108,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 +131,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 +142,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 +151,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 +161,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 +169,116 @@ 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"); + }); + + 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(); }); }); }); diff --git a/apps/api/src/auth/auth.config.ts b/apps/api/src/auth/auth.config.ts index c3d0f78..6b80e97 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,52 @@ 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 { + throw new Error( + `OIDC_REDIRECT_URI must be a valid URL. Current value: "${redirectUri}". ` + + `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.` + ); + } } /** @@ -71,6 +122,7 @@ function getOidcPlugins(): ReturnType[] { clientId: process.env.OIDC_CLIENT_ID ?? "", clientSecret: process.env.OIDC_CLIENT_SECRET ?? "", discoveryUrl: `${process.env.OIDC_ISSUER ?? ""}.well-known/openid-configuration`, + pkce: true, scopes: ["openid", "profile", "email"], }, ],