fix(#412): add OIDC_REDIRECT_URI to startup validation
All checks were successful
ci/woodpecker/push/api Pipeline was successful
All checks were successful
ci/woodpecker/push/api Pipeline was successful
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 <noreply@anthropic.com>
This commit is contained in:
@@ -1,5 +1,24 @@
|
|||||||
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
|
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", () => {
|
describe("auth.config", () => {
|
||||||
// Store original env vars to restore after each test
|
// 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_ISSUER;
|
||||||
delete process.env.OIDC_CLIENT_ID;
|
delete process.env.OIDC_CLIENT_ID;
|
||||||
delete process.env.OIDC_CLIENT_SECRET;
|
delete process.env.OIDC_CLIENT_SECRET;
|
||||||
|
delete process.env.OIDC_REDIRECT_URI;
|
||||||
|
delete process.env.NODE_ENV;
|
||||||
});
|
});
|
||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
@@ -70,6 +91,7 @@ describe("auth.config", () => {
|
|||||||
it("should throw when OIDC_ISSUER is missing", () => {
|
it("should throw when OIDC_ISSUER is missing", () => {
|
||||||
process.env.OIDC_CLIENT_ID = "test-client-id";
|
process.env.OIDC_CLIENT_ID = "test-client-id";
|
||||||
process.env.OIDC_CLIENT_SECRET = "test-client-secret";
|
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_ISSUER");
|
||||||
expect(() => validateOidcConfig()).toThrow("OIDC authentication is enabled");
|
expect(() => validateOidcConfig()).toThrow("OIDC authentication is enabled");
|
||||||
@@ -78,6 +100,7 @@ describe("auth.config", () => {
|
|||||||
it("should throw when OIDC_CLIENT_ID is missing", () => {
|
it("should throw when OIDC_CLIENT_ID is missing", () => {
|
||||||
process.env.OIDC_ISSUER = "https://auth.example.com/";
|
process.env.OIDC_ISSUER = "https://auth.example.com/";
|
||||||
process.env.OIDC_CLIENT_SECRET = "test-client-secret";
|
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");
|
expect(() => validateOidcConfig()).toThrow("OIDC_CLIENT_ID");
|
||||||
});
|
});
|
||||||
@@ -85,13 +108,22 @@ describe("auth.config", () => {
|
|||||||
it("should throw when OIDC_CLIENT_SECRET is missing", () => {
|
it("should throw when OIDC_CLIENT_SECRET is missing", () => {
|
||||||
process.env.OIDC_ISSUER = "https://auth.example.com/";
|
process.env.OIDC_ISSUER = "https://auth.example.com/";
|
||||||
process.env.OIDC_CLIENT_ID = "test-client-id";
|
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");
|
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", () => {
|
it("should throw when all required vars are missing", () => {
|
||||||
expect(() => validateOidcConfig()).toThrow(
|
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_ISSUER = "";
|
||||||
process.env.OIDC_CLIENT_ID = "";
|
process.env.OIDC_CLIENT_ID = "";
|
||||||
process.env.OIDC_CLIENT_SECRET = "";
|
process.env.OIDC_CLIENT_SECRET = "";
|
||||||
|
process.env.OIDC_REDIRECT_URI = "";
|
||||||
|
|
||||||
expect(() => validateOidcConfig()).toThrow(
|
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_ISSUER = " ";
|
||||||
process.env.OIDC_CLIENT_ID = "test-client-id";
|
process.env.OIDC_CLIENT_ID = "test-client-id";
|
||||||
process.env.OIDC_CLIENT_SECRET = "test-client-secret";
|
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_ISSUER");
|
||||||
});
|
});
|
||||||
@@ -117,6 +151,7 @@ describe("auth.config", () => {
|
|||||||
process.env.OIDC_ISSUER = "https://auth.example.com/application/o/mosaic";
|
process.env.OIDC_ISSUER = "https://auth.example.com/application/o/mosaic";
|
||||||
process.env.OIDC_CLIENT_ID = "test-client-id";
|
process.env.OIDC_CLIENT_ID = "test-client-id";
|
||||||
process.env.OIDC_CLIENT_SECRET = "test-client-secret";
|
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("OIDC_ISSUER must end with a trailing slash");
|
||||||
expect(() => validateOidcConfig()).toThrow("https://auth.example.com/application/o/mosaic");
|
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_ISSUER = "https://auth.example.com/application/o/mosaic-stack/";
|
||||||
process.env.OIDC_CLIENT_ID = "test-client-id";
|
process.env.OIDC_CLIENT_ID = "test-client-id";
|
||||||
process.env.OIDC_CLIENT_SECRET = "test-client-secret";
|
process.env.OIDC_CLIENT_SECRET = "test-client-secret";
|
||||||
|
process.env.OIDC_REDIRECT_URI = "https://app.example.com/auth/callback/authentik";
|
||||||
|
|
||||||
expect(() => validateOidcConfig()).not.toThrow();
|
expect(() => validateOidcConfig()).not.toThrow();
|
||||||
});
|
});
|
||||||
@@ -133,6 +169,116 @@ describe("auth.config", () => {
|
|||||||
it("should suggest disabling OIDC in error message", () => {
|
it("should suggest disabling OIDC in error message", () => {
|
||||||
expect(() => validateOidcConfig()).toThrow("OIDC_ENABLED=false");
|
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();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -6,7 +6,12 @@ import type { PrismaClient } from "@prisma/client";
|
|||||||
/**
|
/**
|
||||||
* Required OIDC environment variables when OIDC is enabled
|
* 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
|
* 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.`
|
`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<typeof genericOAuth>[] {
|
|||||||
clientId: process.env.OIDC_CLIENT_ID ?? "",
|
clientId: process.env.OIDC_CLIENT_ID ?? "",
|
||||||
clientSecret: process.env.OIDC_CLIENT_SECRET ?? "",
|
clientSecret: process.env.OIDC_CLIENT_SECRET ?? "",
|
||||||
discoveryUrl: `${process.env.OIDC_ISSUER ?? ""}.well-known/openid-configuration`,
|
discoveryUrl: `${process.env.OIDC_ISSUER ?? ""}.well-known/openid-configuration`,
|
||||||
|
pkce: true,
|
||||||
scopes: ["openid", "profile", "email"],
|
scopes: ["openid", "profile", "email"],
|
||||||
},
|
},
|
||||||
],
|
],
|
||||||
|
|||||||
Reference in New Issue
Block a user