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 <noreply@anthropic.com>
This commit is contained in:
@@ -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", () => {
|
||||
|
||||
@@ -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<string>("OIDC_ISSUER");
|
||||
const clientId = this.config.get<string>("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<string>("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
|
||||
|
||||
Reference in New Issue
Block a user