diff --git a/src/control-plane-services/instance-cluster-management/icms-core/src/main/java/com/nvidia/icms/configuration/security/AuthManagerResolver.java b/src/control-plane-services/instance-cluster-management/icms-core/src/main/java/com/nvidia/icms/configuration/security/AuthManagerResolver.java index dd337b3d3a..6b2ecc6fa8 100644 --- a/src/control-plane-services/instance-cluster-management/icms-core/src/main/java/com/nvidia/icms/configuration/security/AuthManagerResolver.java +++ b/src/control-plane-services/instance-cluster-management/icms-core/src/main/java/com/nvidia/icms/configuration/security/AuthManagerResolver.java @@ -43,6 +43,7 @@ import java.util.Set; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Value; +import org.springframework.cloud.context.config.annotation.RefreshScope; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.http.HttpHeaders; @@ -90,14 +91,11 @@ public class AuthManagerResolver { private final ApiKeysService apiKeysService; private final ClusterRepository clusterRepository; private final NvcaConfigurationProperties nvcaConfig; - /** - * Trusted static JWT issuers, mapping each accepted {@code iss} claim to the JWKS URI - * used to verify its signatures. Built once at construction from the primary issuer, - * the legacy single admin issuer, and any configured {@code trusted-issuers[]} entries. - * Insertion order is preserved for deterministic manager registration; the primary - * issuer is always first. - */ - private final Map trustedIssuerJwkSetUris; + private final TrustedJwtIssuerProperties trustedIssuerProperties; + private final String issuerUri; + private final String jwkSetUri; + private final String adminIssuerUri; + private final String adminJwkSetUri; private final SignatureAlgorithm jwsAlgorithm; private final boolean apiKeyAuthEnabled; @@ -121,21 +119,36 @@ public AuthManagerResolver( String adminJwkSetUri, // Zero or more additional trusted static issuers (trusted-issuers[]), so tokens // from multiple external issuers can be trusted, keyed by iss. Empty by default, - // preserving single/dual-issuer behavior. + // preserving single/dual-issuer behavior. Held as a field (not flattened here) so the + // refresh-scoped authenticationManagerResolver() re-reads it on rebuild. TrustedJwtIssuerProperties trustedIssuerProperties, @Value("${icms.nvca.api-key.enabled:true}") boolean apiKeyAuthEnabled) { this.apiKeysService = apiKeysService; this.clusterRepository = clusterRepository; this.nvcaConfig = nvcaConfig; - this.trustedIssuerJwkSetUris = buildTrustedIssuerJwkSetUris( - issuerUri, jwkSetUri, adminIssuerUri, adminJwkSetUri, trustedIssuerProperties); + this.trustedIssuerProperties = trustedIssuerProperties; + this.issuerUri = issuerUri; + this.jwkSetUri = jwkSetUri; + this.adminIssuerUri = adminIssuerUri; + this.adminJwkSetUri = adminJwkSetUri; this.jwsAlgorithm = SignatureAlgorithm.valueOf(jwsAlgorithm); this.apiKeyAuthEnabled = apiKeyAuthEnabled; } + /** + * Refresh-scoped so {@code trusted-issuers[]} entries added or removed at runtime take effect + * without a restart: the bean is rebuilt on the first request after a configuration refresh, + * re-reading the refresh-scoped {@link TrustedJwtIssuerProperties}. + * + *

The primary and legacy admin issuers are {@code @Value}-bound at construction and stay + * fixed for the life of the context; only {@code trusted-issuers[]} participates in refresh.

+ */ @Bean + @RefreshScope AuthenticationManagerResolver authenticationManagerResolver() { - var jwtResolver = jwtResolver(); + Map trustedIssuerJwkSetUris = buildTrustedIssuerJwkSetUris( + issuerUri, jwkSetUri, adminIssuerUri, adminJwkSetUri, trustedIssuerProperties); + var jwtResolver = jwtResolver(trustedIssuerJwkSetUris); var authenticationManager = apiKeyAuthenticationManager(); return request -> { var authorization = request.getHeader(HttpHeaders.AUTHORIZATION); @@ -151,7 +164,7 @@ AuthenticationManagerResolver authenticationManagerResolver( // Known static issuer (primary, admin-issuer-proxy, or a configured trusted // external issuer) — use native Spring resolver. String issuer = extractIssuerFromToken(authorization); - if (isTrustedStaticIssuer(issuer)) { + if (isTrustedStaticIssuer(trustedIssuerJwkSetUris, issuer)) { return jwtResolver.resolve(request); } @@ -185,10 +198,11 @@ private static OpaqueTokenAuthenticationConverter apiKeyConverter() { }; } - private JwtIssuerAuthenticationManagerResolver jwtResolver() { + private JwtIssuerAuthenticationManagerResolver jwtResolver(Map jwkSetUris) { Map managers = new HashMap<>(); - // One JwtAuthenticationManager per trusted static issuer, keyed by iss. - trustedIssuerJwkSetUris.forEach((iss, jwks) -> + // One JwtAuthenticationManager per trusted static issuer, keyed by iss. Decoders fetch + // their JWKS lazily on first use, so rebuilding them on refresh costs nothing here. + jwkSetUris.forEach((iss, jwks) -> managers.put(iss, jwtAuthenticationManagerFor(iss, jwks))); return new JwtIssuerAuthenticationManagerResolver(managers::get); } @@ -461,8 +475,8 @@ private static boolean isConfigured(String value) { return value != null && !value.isBlank(); } - private boolean isTrustedStaticIssuer(String issuer) { - return issuer != null && trustedIssuerJwkSetUris.containsKey(issuer); + private static boolean isTrustedStaticIssuer(Map jwkSetUris, String issuer) { + return issuer != null && jwkSetUris.containsKey(issuer); } } diff --git a/src/control-plane-services/instance-cluster-management/icms-core/src/main/java/com/nvidia/icms/configuration/security/TrustedJwtIssuerProperties.java b/src/control-plane-services/instance-cluster-management/icms-core/src/main/java/com/nvidia/icms/configuration/security/TrustedJwtIssuerProperties.java index ae56826052..37129dbbbd 100644 --- a/src/control-plane-services/instance-cluster-management/icms-core/src/main/java/com/nvidia/icms/configuration/security/TrustedJwtIssuerProperties.java +++ b/src/control-plane-services/instance-cluster-management/icms-core/src/main/java/com/nvidia/icms/configuration/security/TrustedJwtIssuerProperties.java @@ -20,6 +20,7 @@ import java.util.List; import lombok.Data; import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.cloud.context.config.annotation.RefreshScope; import org.springframework.context.annotation.Configuration; /** @@ -34,7 +35,12 @@ *

An empty list (the default) preserves single/dual-issuer behavior, so * existing deployments that only set {@code issuer-uri} (and optionally * {@code admin-issuer-uri}) are unaffected.

+ * + *

Refresh-scoped: entries can be added or removed at runtime, and + * {@link AuthManagerResolver#authenticationManagerResolver()} is rebuilt from the + * new value without a restart.

*/ +@RefreshScope @Configuration @ConfigurationProperties(prefix = "icms.security.jwt") @Data diff --git a/src/control-plane-services/instance-cluster-management/icms-core/src/test/java/com/nvidia/icms/configuration/security/AuthManagerResolverTest.java b/src/control-plane-services/instance-cluster-management/icms-core/src/test/java/com/nvidia/icms/configuration/security/AuthManagerResolverTest.java index f76ea93a8e..e0db1e8ce8 100644 --- a/src/control-plane-services/instance-cluster-management/icms-core/src/test/java/com/nvidia/icms/configuration/security/AuthManagerResolverTest.java +++ b/src/control-plane-services/instance-cluster-management/icms-core/src/test/java/com/nvidia/icms/configuration/security/AuthManagerResolverTest.java @@ -754,6 +754,80 @@ void authenticationManagerResolver_trustedIssuerConfigured_unknownIssFallsThroug assertThrows(AuthenticationServiceException.class, () -> authResolver.resolve(req)); } + // --- trusted-issuers[] refresh tests --- + + @Test + void trustedIssuers_entryAddedThenRebuilt_isTrusted() { + TrustedJwtIssuerProperties props = new TrustedJwtIssuerProperties(); + AuthManagerResolver multi = buildResolverWithTrustedIssuers(props); + + HttpServletRequest req = mock(HttpServletRequest.class); + when(req.getHeader("Authorization")) + .thenReturn(bearerWithIssuer("http://api.external-nvcf.example.com", "admin")); + + AuthenticationManagerResolver before = + multi.authenticationManagerResolver(); + assertThrows(AuthenticationServiceException.class, () -> before.resolve(req)); + + props.setTrustedIssuers(List.of(trustedIssuerEntry( + "http://api.external-nvcf.example.com", + "http://openbao.external/v1/services/nvcf-api/jwt/jwks"))); + + assertNotNull(multi.authenticationManagerResolver().resolve(req), + "issuer added at runtime must be trusted after the bean is rebuilt"); + } + + @Test + void trustedIssuers_entryRemovedThenRebuilt_isNoLongerTrusted() { + TrustedJwtIssuerProperties props = trustedIssuers( + "http://api.external-nvcf.example.com", + "http://openbao.external/v1/services/nvcf-api/jwt/jwks"); + AuthManagerResolver multi = buildResolverWithTrustedIssuers(props); + + HttpServletRequest req = mock(HttpServletRequest.class); + when(req.getHeader("Authorization")) + .thenReturn(bearerWithIssuer("http://api.external-nvcf.example.com", "admin")); + assertNotNull(multi.authenticationManagerResolver().resolve(req)); + + props.setTrustedIssuers(List.of()); + + AuthenticationManagerResolver after = + multi.authenticationManagerResolver(); + assertThrows(AuthenticationServiceException.class, () -> after.resolve(req), + "issuer removed at runtime must stop being trusted — revocation must apply"); + } + + @Test + void trustedIssuers_primaryIssuerSurvivesRebuild() { + TrustedJwtIssuerProperties props = new TrustedJwtIssuerProperties(); + AuthManagerResolver multi = buildResolverWithTrustedIssuers(props); + + props.setTrustedIssuers(List.of(trustedIssuerEntry( + "http://api.external-nvcf.example.com", + "http://openbao.external/v1/services/nvcf-api/jwt/jwks"))); + + HttpServletRequest req = mock(HttpServletRequest.class); + when(req.getHeader("Authorization")) + .thenReturn(bearerWithIssuer("http://api.sis.svc.cluster.local", "admin")); + assertNotNull(multi.authenticationManagerResolver().resolve(req), + "primary issuer must remain trusted across a rebuild"); + } + + @Test + void trustedIssuers_conflictingJwksOnRebuild_failsLoudly() { + // Rejected on rebuild exactly as at startup, rather than silently keeping stale trust. + TrustedJwtIssuerProperties props = new TrustedJwtIssuerProperties(); + AuthManagerResolver multi = buildResolverWithTrustedIssuers(props); + assertNotNull(multi.authenticationManagerResolver()); + + props.setTrustedIssuers(List.of(trustedIssuerEntry( + "http://api.sis.svc.cluster.local", "http://openbao.other/jwks"))); + + IllegalStateException ex = assertThrows(IllegalStateException.class, + multi::authenticationManagerResolver); + assertTrue(ex.getMessage().contains("http://api.sis.svc.cluster.local")); + } + @Test void authenticationManagerResolver_clusterOidcUnknownCluster_throwsGenericMessage() { AuthenticationManagerResolver authResolver = @@ -869,12 +943,31 @@ void authenticationManagerResolver_apiKeyAuthDisabled_jwtIssuerStillRoutes() { /** Build a TrustedJwtIssuerProperties carrying one {issuer-uri, jwk-set-uri} entry. */ private static TrustedJwtIssuerProperties trustedIssuers(String issuerUri, String jwkSetUri) { - TrustedJwtIssuerProperties.TrustedIssuer entry = - new TrustedJwtIssuerProperties.TrustedIssuer(); - entry.setIssuerUri(issuerUri); - entry.setJwkSetUri(jwkSetUri); TrustedJwtIssuerProperties props = new TrustedJwtIssuerProperties(); - props.setTrustedIssuers(List.of(entry)); + props.setTrustedIssuers(List.of(trustedIssuerEntry(issuerUri, jwkSetUri))); return props; } + + private static TrustedJwtIssuerProperties.TrustedIssuer trustedIssuerEntry( + String issuerUri, String jwkSetUri) { + TrustedJwtIssuerProperties.TrustedIssuer entry = + new TrustedJwtIssuerProperties.TrustedIssuer(); + entry.setIssuerUri(issuerUri); + entry.setJwkSetUri(jwkSetUri); + return entry; + } + + /** Resolver over a caller-held properties instance, so the test can mutate it and refresh. */ + private AuthManagerResolver buildResolverWithTrustedIssuers(TrustedJwtIssuerProperties props) { + NvcaConfigurationProperties cfg = new NvcaConfigurationProperties(); + cfg.setOidcClusterIdentityEnabled(true); + return new AuthManagerResolver( + apiKeysService, clusterRepository, cfg, "ES256", + "http://api.sis.svc.cluster.local", + "http://openbao/v1/services/sis-api/jwt/jwks", + "", + "", + props, + /* apiKeyAuthEnabled */ true); + } } diff --git a/src/control-plane-services/instance-cluster-management/icms-core/src/test/java/com/nvidia/icms/configuration/security/TrustedIssuerRefreshIntegrationTest.java b/src/control-plane-services/instance-cluster-management/icms-core/src/test/java/com/nvidia/icms/configuration/security/TrustedIssuerRefreshIntegrationTest.java new file mode 100644 index 0000000000..1632698650 --- /dev/null +++ b/src/control-plane-services/instance-cluster-management/icms-core/src/test/java/com/nvidia/icms/configuration/security/TrustedIssuerRefreshIntegrationTest.java @@ -0,0 +1,125 @@ +/* + * SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.nvidia.icms.configuration.security; + +import static org.junit.jupiter.api.Assertions.*; +import static org.springframework.test.context.support.TestPropertySourceUtils.INLINED_PROPERTIES_PROPERTY_SOURCE_NAME; + +import com.nvidia.icms.integration.IntegrationTest; +import jakarta.servlet.http.HttpServletRequest; +import java.util.Base64; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.cloud.context.refresh.ContextRefresher; +import org.springframework.context.ApplicationContextInitializer; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.core.env.MapPropertySource; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.security.authentication.AuthenticationManagerResolver; +import org.springframework.security.authentication.AuthenticationServiceException; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.TestPropertySource; + +/** + * Not redundant with {@code AuthManagerResolverTest}, which calls the factory method directly and + * passes whether or not the {@code @RefreshScope} annotations are present. Removing one would + * silently restore the defect they exist to prevent, and this is what catches that. + */ +// ContextRefresher.copyEnvironment() keeps only commandLineArgs and defaultProperties, so the +// properties @SpringBootTest inlines — spring.profiles.active among them — are dropped on every +// refresh and ValidateEnvironmentPostProcessor then fails it. Retain that source explicitly. +@TestPropertySource(properties = "spring.cloud.refresh.additional-property-sources-to-retain=" + + INLINED_PROPERTIES_PROPERTY_SOURCE_NAME) +@ContextConfiguration(initializers = TrustedIssuerRefreshIntegrationTest.MutableOverrides.class) +class TrustedIssuerRefreshIntegrationTest extends IntegrationTest { + + private static final String ADDED_ISSUER = "http://api.external-nvcf.example.com"; + private static final String ADDED_JWKS = "http://openbao.external/v1/services/nvcf-api/jwt/jwks"; + + private static final String TRUSTED_ISSUER_URI = "icms.security.jwt.trusted-issuers[0].issuer-uri"; + private static final String TRUSTED_JWK_SET_URI = "icms.security.jwt.trusted-issuers[0].jwk-set-uri"; + + /** The property source below wraps this instance, so mutating it changes the environment. */ + private static final Map OVERRIDES = new HashMap<>(); + + @Autowired + private ContextRefresher contextRefresher; + + @Autowired + private AuthenticationManagerResolver authenticationManagerResolver; + + @Value("${spring.security.oauth2.resourceserver.jwt.issuer-uri}") + private String primaryIssuer; + + @Test + void trustedIssuerAddedAndRemovedAtRuntime_appliesWithoutRestart() { + HttpServletRequest primary = requestWithIssuer(primaryIssuer); + HttpServletRequest added = requestWithIssuer(ADDED_ISSUER); + + // resolve() returns a manager for trusted and untrusted alike, deferring the issuer + // lookup to authenticate(). The throw below is the discriminator: an untrusted iss falls + // through to cluster OIDC, which rejects the missing nvcf-icms:{clusterId} audience. + assertNotNull(authenticationManagerResolver.resolve(primary), + "primary issuer must be trusted at startup"); + assertThrows(AuthenticationServiceException.class, + () -> authenticationManagerResolver.resolve(added), + "issuer must not be trusted before it is configured"); + + OVERRIDES.put(TRUSTED_ISSUER_URI, ADDED_ISSUER); + OVERRIDES.put(TRUSTED_JWK_SET_URI, ADDED_JWKS); + contextRefresher.refresh(); + + assertNotNull(authenticationManagerResolver.resolve(added), + "issuer added at runtime must be accepted after a refresh, without a restart"); + assertNotNull(authenticationManagerResolver.resolve(primary), + "primary issuer must still be trusted after the refresh"); + + // Revocation is the half a stale set gets silently wrong. + OVERRIDES.remove(TRUSTED_ISSUER_URI); + OVERRIDES.remove(TRUSTED_JWK_SET_URI); + contextRefresher.refresh(); + + assertThrows(AuthenticationServiceException.class, + () -> authenticationManagerResolver.resolve(added), + "issuer removed at runtime must stop being trusted after a refresh"); + assertNotNull(authenticationManagerResolver.resolve(primary), + "primary issuer must survive the removal refresh"); + } + + /** {@link IntegrationTest.Initializer} uses static TestPropertyValues; this one can change. */ + public static class MutableOverrides + implements ApplicationContextInitializer { + + @Override + public void initialize(ConfigurableApplicationContext applicationContext) { + applicationContext.getEnvironment().getPropertySources() + .addFirst(new MapPropertySource("trusted-issuer-overrides", OVERRIDES)); + } + } + + private static HttpServletRequest requestWithIssuer(String issuer) { + String payload = String.format("{\"iss\":\"%s\",\"sub\":\"probe\",\"aud\":[\"x\"]}", issuer); + String b64 = Base64.getUrlEncoder().withoutPadding().encodeToString(payload.getBytes()); + MockHttpServletRequest request = new MockHttpServletRequest(); + request.addHeader("Authorization", "Bearer eyJhbGciOiJFUzI1NiJ9." + b64 + ".sig"); + return request; + } +}