From c30b4b1cc29dfb6fcf09acd09ea2695536581033 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Thu, 5 Feb 2026 16:03:09 -0600 Subject: [PATCH] fix(#337): Replace hardcoded OIDC values in federation with env vars - Use OIDC_ISSUER and OIDC_CLIENT_ID from environment for JWT validation - Federation OIDC properly configured from environment variables - Fail fast with clear error when OIDC config is missing - Handle trailing slash normalization for issuer URL - Add tests verifying env var usage and missing config error handling Refs #337 Co-Authored-By: Claude Opus 4.5 --- apps/api/src/federation/oidc.service.spec.ts | 145 +++++++++++++++++++ apps/api/src/federation/oidc.service.ts | 35 ++++- 2 files changed, 178 insertions(+), 2 deletions(-) diff --git a/apps/api/src/federation/oidc.service.spec.ts b/apps/api/src/federation/oidc.service.spec.ts index d9cb8f2..8c39898 100644 --- a/apps/api/src/federation/oidc.service.spec.ts +++ b/apps/api/src/federation/oidc.service.spec.ts @@ -311,6 +311,22 @@ describe("OIDCService", () => { }); describe("validateToken - Real JWT Validation", () => { + // Configure mock to return OIDC env vars by default for validation tests + beforeEach(() => { + mockConfigService.get.mockImplementation((key: string) => { + switch (key) { + case "OIDC_ISSUER": + return "https://auth.example.com/"; + case "OIDC_CLIENT_ID": + return "mosaic-client-id"; + case "OIDC_VALIDATION_SECRET": + return "test-secret-key-for-jwt-signing"; + default: + return undefined; + } + }); + }); + it("should reject malformed token (not a JWT)", async () => { const token = "not-a-jwt-token"; const instanceId = "remote-instance-123"; @@ -331,6 +347,104 @@ describe("OIDCService", () => { expect(result.error).toContain("Malformed token"); }); + it("should return error when OIDC_ISSUER is not configured", async () => { + mockConfigService.get.mockImplementation((key: string) => { + switch (key) { + case "OIDC_ISSUER": + return undefined; // Not configured + case "OIDC_CLIENT_ID": + return "mosaic-client-id"; + default: + return undefined; + } + }); + + const token = await createTestJWT({ + sub: "user-123", + iss: "https://auth.example.com", + aud: "mosaic-client-id", + exp: Math.floor(Date.now() / 1000) + 3600, + iat: Math.floor(Date.now() / 1000), + email: "user@example.com", + }); + + const result = await service.validateToken(token, "remote-instance-123"); + + expect(result.valid).toBe(false); + expect(result.error).toContain("OIDC_ISSUER is required"); + }); + + it("should return error when OIDC_CLIENT_ID is not configured", async () => { + mockConfigService.get.mockImplementation((key: string) => { + switch (key) { + case "OIDC_ISSUER": + return "https://auth.example.com/"; + case "OIDC_CLIENT_ID": + return undefined; // Not configured + default: + return undefined; + } + }); + + const token = await createTestJWT({ + sub: "user-123", + iss: "https://auth.example.com", + aud: "mosaic-client-id", + exp: Math.floor(Date.now() / 1000) + 3600, + iat: Math.floor(Date.now() / 1000), + email: "user@example.com", + }); + + const result = await service.validateToken(token, "remote-instance-123"); + + expect(result.valid).toBe(false); + expect(result.error).toContain("OIDC_CLIENT_ID is required"); + }); + + it("should return error when OIDC_ISSUER is empty string", async () => { + mockConfigService.get.mockImplementation((key: string) => { + switch (key) { + case "OIDC_ISSUER": + return " "; // Empty/whitespace + case "OIDC_CLIENT_ID": + return "mosaic-client-id"; + default: + return undefined; + } + }); + + const token = await createTestJWT({ + sub: "user-123", + iss: "https://auth.example.com", + aud: "mosaic-client-id", + exp: Math.floor(Date.now() / 1000) + 3600, + iat: Math.floor(Date.now() / 1000), + email: "user@example.com", + }); + + const result = await service.validateToken(token, "remote-instance-123"); + + expect(result.valid).toBe(false); + expect(result.error).toContain("OIDC_ISSUER is required"); + }); + + it("should use OIDC_ISSUER and OIDC_CLIENT_ID from environment", async () => { + // Verify that the config service is called with correct keys + const token = await createTestJWT({ + sub: "user-123", + iss: "https://auth.example.com", + aud: "mosaic-client-id", + exp: Math.floor(Date.now() / 1000) + 3600, + iat: Math.floor(Date.now() / 1000), + email: "user@example.com", + }); + + await service.validateToken(token, "remote-instance-123"); + + expect(mockConfigService.get).toHaveBeenCalledWith("OIDC_ISSUER"); + expect(mockConfigService.get).toHaveBeenCalledWith("OIDC_CLIENT_ID"); + }); + it("should reject expired token", async () => { // Create an expired JWT (exp in the past) const expiredToken = await createTestJWT({ @@ -442,6 +556,37 @@ describe("OIDCService", () => { expect(result.email).toBe("test@example.com"); expect(result.subject).toBe("user-456"); }); + + it("should normalize issuer with trailing slash for JWT validation", async () => { + // Config returns issuer WITH trailing slash (as per auth.config.ts validation) + mockConfigService.get.mockImplementation((key: string) => { + switch (key) { + case "OIDC_ISSUER": + return "https://auth.example.com/"; // With trailing slash + case "OIDC_CLIENT_ID": + return "mosaic-client-id"; + case "OIDC_VALIDATION_SECRET": + return "test-secret-key-for-jwt-signing"; + default: + return undefined; + } + }); + + // JWT issuer is without trailing slash (standard JWT format) + const validToken = await createTestJWT({ + sub: "user-123", + iss: "https://auth.example.com", // Without trailing slash (matches normalized) + aud: "mosaic-client-id", + exp: Math.floor(Date.now() / 1000) + 3600, + iat: Math.floor(Date.now() / 1000), + email: "user@example.com", + }); + + const result = await service.validateToken(validToken, "remote-instance-123"); + + expect(result.valid).toBe(true); + expect(result.userId).toBe("user-123"); + }); }); describe("generateAuthUrl", () => { diff --git a/apps/api/src/federation/oidc.service.ts b/apps/api/src/federation/oidc.service.ts index d432edb..8bee399 100644 --- a/apps/api/src/federation/oidc.service.ts +++ b/apps/api/src/federation/oidc.service.ts @@ -129,16 +129,47 @@ export class OIDCService { }; } + // Get OIDC configuration from environment variables + // These must be configured for federation token validation to work + const issuer = this.config.get("OIDC_ISSUER"); + const clientId = this.config.get("OIDC_CLIENT_ID"); + + // Fail fast if OIDC configuration is missing + if (!issuer || issuer.trim() === "") { + this.logger.error( + "Federation OIDC validation failed: OIDC_ISSUER environment variable is not configured" + ); + return { + valid: false, + error: + "Federation OIDC configuration error: OIDC_ISSUER is required for token validation", + }; + } + + if (!clientId || clientId.trim() === "") { + this.logger.error( + "Federation OIDC validation failed: OIDC_CLIENT_ID environment variable is not configured" + ); + return { + valid: false, + error: + "Federation OIDC configuration error: OIDC_CLIENT_ID is required for token validation", + }; + } + // Get validation secret from config (for testing/development) // In production, this should fetch JWKS from the remote instance const secret = this.config.get("OIDC_VALIDATION_SECRET") ?? "test-secret-key-for-jwt-signing"; const secretKey = new TextEncoder().encode(secret); + // Remove trailing slash from issuer for JWT validation (jose expects issuer without trailing slash) + const normalizedIssuer = issuer.endsWith("/") ? issuer.slice(0, -1) : issuer; + // Verify and decode JWT const { payload } = await jose.jwtVerify(token, secretKey, { - issuer: "https://auth.example.com", // TODO: Fetch from remote instance config - audience: "mosaic-client-id", // TODO: Get from config + issuer: normalizedIssuer, + audience: clientId, }); // Extract claims