From 23230a5da7af8a4f34faa5e6795cca529b99e90d Mon Sep 17 00:00:00 2001 From: sparve Date: Mon, 21 Sep 2026 15:43:56 +0530 Subject: [PATCH 1/2] fix(icms): apply trusted JWT issuer changes without a restart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit icms.security.jwt.trusted-issuers[] was flattened into an immutable map in AuthManagerResolver's constructor and baked into the SecurityFilterChain at startup, so a ConfigMap edit was accepted by the reload machinery and then silently ignored. Adding an issuer had no effect, and — more importantly — removing one appeared to revoke trust but did not. Mark TrustedJwtIssuerProperties @RefreshScope and make authenticationManagerResolver() @RefreshScope, following the @Bean + @RefreshScope pattern already used by TelemetryConfiguration and EventFormatConfiguration. The trusted-issuer set is derived inside the bean method, which a refresh re-invokes, so the existing injected handle picks up the new set without a restart. The primary and legacy admin issuers stay @Value-bound at construction; only trusted-issuers[] participates in refresh. With trusted-issuers[] unset or unchanged, behavior is identical to before. Fixes #1999 Co-Authored-By: Claude Opus 5 Signed-off-by: sparve --- .../security/AuthManagerResolver.java | 50 ++++-- .../security/TrustedJwtIssuerProperties.java | 6 + .../security/AuthManagerResolverTest.java | 112 +++++++++++- .../security/TrustedIssuerRefreshTest.java | 165 ++++++++++++++++++ 4 files changed, 310 insertions(+), 23 deletions(-) create mode 100644 src/control-plane-services/instance-cluster-management/icms-core/src/test/java/com/nvidia/icms/configuration/security/TrustedIssuerRefreshTest.java 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..545fead968 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. The equally + * refresh-scoped {@link AuthManagerResolver#authenticationManagerResolver()} is + * rebuilt from the new value on the next request, 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..bcb7118ced 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,89 @@ void authenticationManagerResolver_trustedIssuerConfigured_unknownIssFallsThroug assertThrows(AuthenticationServiceException.class, () -> authResolver.resolve(req)); } + // --- trusted-issuers[] refresh tests --- + + // A refresh rebuilds the bean by re-invoking the factory method; these call it directly to + // exercise what that rebuild produces. TrustedIssuerRefreshTest drives the refresh itself. + + @Test + void trustedIssuers_entryAddedThenRebuilt_isTrusted() { + // Given: no trusted-issuers[] entries. + 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")); + + // Initially untrusted: falls through to cluster OIDC, which throws on the missing marker. + AuthenticationManagerResolver before = + multi.authenticationManagerResolver(); + assertThrows(AuthenticationServiceException.class, () -> before.resolve(req)); + + // When: an entry is added to the refresh-scoped properties and the bean is rebuilt. + props.setTrustedIssuers(List.of(trustedIssuerEntry( + "http://api.external-nvcf.example.com", + "http://openbao.external/v1/services/nvcf-api/jwt/jwks"))); + + // Then: the rebuilt resolver routes the new issuer to the static resolver. + 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() { + // The primary issuer is not part of trusted-issuers[]; a rebuild must not drop it. + 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() { + // Re-declaring an already-trusted issuer with a different JWKS is rejected on rebuild + // exactly as it is 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 +952,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/TrustedIssuerRefreshTest.java b/src/control-plane-services/instance-cluster-management/icms-core/src/test/java/com/nvidia/icms/configuration/security/TrustedIssuerRefreshTest.java new file mode 100644 index 0000000000..6376fe8541 --- /dev/null +++ b/src/control-plane-services/instance-cluster-management/icms-core/src/test/java/com/nvidia/icms/configuration/security/TrustedIssuerRefreshTest.java @@ -0,0 +1,165 @@ +/* + * 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.mockito.Mockito.*; + +import com.nvidia.icms.configuration.nvca.NvcaConfigurationProperties; +import com.nvidia.icms.outbound.apikeys.ApiKeysService; +import com.nvidia.icms.outbound.cassandra.byoc.ClusterRepository; +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.cloud.autoconfigure.RefreshAutoConfiguration; +import org.springframework.cloud.context.refresh.ContextRefresher; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.core.env.MapPropertySource; +import org.springframework.security.authentication.AuthenticationManagerResolver; +import org.springframework.security.authentication.AuthenticationServiceException; + +/** + * Ties the {@code @RefreshScope} annotations on {@link AuthManagerResolver} and + * {@link TrustedJwtIssuerProperties} to the behavior they exist for, by driving a real + * {@link ContextRefresher#refresh()} over the real beans through the same resolver handle a + * {@code SecurityFilterChain} holds. + * + *

This is not redundant with {@code AuthManagerResolverTest}: those tests call the factory + * method directly and pass whether or not either annotation is present. Without this test, + * removing {@code @RefreshScope} silently restores the defect this class exists to prevent — + * configuration accepted, never applied, and revocation that appears to work but does not.

+ */ +class TrustedIssuerRefreshTest { + + private static final String APPLICATION_NAME = "spring.application.name"; + private static final String ACTIVE_PROFILES = "spring.profiles.active"; + + private static final String PRIMARY_ISSUER = "http://api.sis.svc.cluster.local"; + private static final String PRIMARY_JWKS = "http://openbao/v1/services/sis-api/jwt/jwks"; + 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"; + + @Test + void trustedIssuerAddedAndRemovedAtRuntime_appliesWithoutRestart() { + Map source = new HashMap<>(Map.of( + "spring.security.oauth2.resourceserver.jwt.issuer-uri", PRIMARY_ISSUER, + "spring.security.oauth2.resourceserver.jwt.jwk-set-uri", PRIMARY_JWKS, + "icms.nvca.api-key.enabled", "false")); + + // ContextRefresher re-reads the Environment through a SpringApplication, which this + // codebase's ValidateEnvironmentPostProcessor gates on these two being set. Tests share + // one JVM, so capture whatever was there and put it back rather than clearing blindly. + String priorName = System.getProperty(APPLICATION_NAME); + String priorProfiles = System.getProperty(ACTIVE_PROFILES); + System.setProperty(APPLICATION_NAME, "trusted-issuer-refresh-test"); + System.setProperty(ACTIVE_PROFILES, "test"); + try (var ctx = new AnnotationConfigApplicationContext()) { + ctx.getEnvironment().getPropertySources() + .addFirst(new MapPropertySource("test-source", source)); + ctx.register(RefreshAutoConfiguration.class, ResolverDependencies.class, + TrustedJwtIssuerProperties.class, AuthManagerResolver.class); + ctx.refresh(); + + @SuppressWarnings("unchecked") + AuthenticationManagerResolver resolver = + ctx.getBean(AuthenticationManagerResolver.class); + + HttpServletRequest primary = requestWithIssuer(PRIMARY_ISSUER); + HttpServletRequest added = requestWithIssuer(ADDED_ISSUER); + + assertNotNull(resolver.resolve(primary), "primary issuer must be trusted at startup"); + assertThrows(AuthenticationServiceException.class, () -> resolver.resolve(added), + "issuer must not be trusted before it is configured"); + + // Add the issuer the way an operator would — a new entry in the property source. + source.put(TRUSTED_ISSUER_URI, ADDED_ISSUER); + source.put(TRUSTED_JWK_SET_URI, ADDED_JWKS); + ctx.getBean(ContextRefresher.class).refresh(); + + assertNotNull(resolver.resolve(added), + "issuer added at runtime must be accepted after a refresh, without a restart"); + assertNotNull(resolver.resolve(primary), + "primary issuer must still be trusted after the refresh"); + + // Revocation must apply too — this is the case a stale set gets silently wrong. + source.remove(TRUSTED_ISSUER_URI); + source.remove(TRUSTED_JWK_SET_URI); + ctx.getBean(ContextRefresher.class).refresh(); + + assertEquals(List.of(), + ctx.getBean(TrustedJwtIssuerProperties.class).getTrustedIssuers(), + "the removal must reach the bound properties"); + assertThrows(AuthenticationServiceException.class, () -> resolver.resolve(added), + "issuer removed at runtime must stop being trusted after a refresh"); + assertNotNull(resolver.resolve(primary), + "primary issuer must survive the removal refresh"); + } finally { + restoreProperty(APPLICATION_NAME, priorName); + restoreProperty(ACTIVE_PROFILES, priorProfiles); + } + } + + @Configuration(proxyBeanMethods = false) + static class ResolverDependencies { + + @Bean + ApiKeysService apiKeysService() { + return mock(ApiKeysService.class); + } + + @Bean + ClusterRepository clusterRepository() { + return mock(ClusterRepository.class); + } + + @Bean + NvcaConfigurationProperties nvcaConfigurationProperties() { + // Flag on so an untrusted iss falls through to cluster OIDC and throws on the missing + // nvcf-icms:{clusterId} audience. That throw is what distinguishes "not trusted" here: + // JwtIssuerAuthenticationManagerResolver.resolve() returns a manager either way and + // defers the issuer lookup to authenticate(). + NvcaConfigurationProperties cfg = new NvcaConfigurationProperties(); + cfg.setOidcClusterIdentityEnabled(true); + return cfg; + } + } + + 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()); + HttpServletRequest req = mock(HttpServletRequest.class); + when(req.getHeader("Authorization")) + .thenReturn("Bearer eyJhbGciOiJFUzI1NiJ9." + b64 + ".sig"); + return req; + } + + private static void restoreProperty(String key, String priorValue) { + if (priorValue == null) { + System.clearProperty(key); + } else { + System.setProperty(key, priorValue); + } + } +} From 96ef7a543d4f96ea8dfb66b31401f6cc5f0f261f Mon Sep 17 00:00:00 2001 From: sparve Date: Mon, 21 Sep 2026 18:33:26 +0530 Subject: [PATCH 2/2] feat(improve/multi-issuer/refresh): Improving Integration test Signed-off-by: sparve --- .../security/TrustedJwtIssuerProperties.java | 6 +- .../security/AuthManagerResolverTest.java | 11 +- .../TrustedIssuerRefreshIntegrationTest.java | 125 +++++++++++++ .../security/TrustedIssuerRefreshTest.java | 165 ------------------ 4 files changed, 129 insertions(+), 178 deletions(-) create mode 100644 src/control-plane-services/instance-cluster-management/icms-core/src/test/java/com/nvidia/icms/configuration/security/TrustedIssuerRefreshIntegrationTest.java delete mode 100644 src/control-plane-services/instance-cluster-management/icms-core/src/test/java/com/nvidia/icms/configuration/security/TrustedIssuerRefreshTest.java 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 545fead968..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 @@ -36,9 +36,9 @@ * 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. The equally - * refresh-scoped {@link AuthManagerResolver#authenticationManagerResolver()} is - * rebuilt from the new value on the next request, without a restart.

+ *

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 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 bcb7118ced..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 @@ -756,12 +756,8 @@ void authenticationManagerResolver_trustedIssuerConfigured_unknownIssFallsThroug // --- trusted-issuers[] refresh tests --- - // A refresh rebuilds the bean by re-invoking the factory method; these call it directly to - // exercise what that rebuild produces. TrustedIssuerRefreshTest drives the refresh itself. - @Test void trustedIssuers_entryAddedThenRebuilt_isTrusted() { - // Given: no trusted-issuers[] entries. TrustedJwtIssuerProperties props = new TrustedJwtIssuerProperties(); AuthManagerResolver multi = buildResolverWithTrustedIssuers(props); @@ -769,17 +765,14 @@ void trustedIssuers_entryAddedThenRebuilt_isTrusted() { when(req.getHeader("Authorization")) .thenReturn(bearerWithIssuer("http://api.external-nvcf.example.com", "admin")); - // Initially untrusted: falls through to cluster OIDC, which throws on the missing marker. AuthenticationManagerResolver before = multi.authenticationManagerResolver(); assertThrows(AuthenticationServiceException.class, () -> before.resolve(req)); - // When: an entry is added to the refresh-scoped properties and the bean is rebuilt. props.setTrustedIssuers(List.of(trustedIssuerEntry( "http://api.external-nvcf.example.com", "http://openbao.external/v1/services/nvcf-api/jwt/jwks"))); - // Then: the rebuilt resolver routes the new issuer to the static resolver. assertNotNull(multi.authenticationManagerResolver().resolve(req), "issuer added at runtime must be trusted after the bean is rebuilt"); } @@ -806,7 +799,6 @@ void trustedIssuers_entryRemovedThenRebuilt_isNoLongerTrusted() { @Test void trustedIssuers_primaryIssuerSurvivesRebuild() { - // The primary issuer is not part of trusted-issuers[]; a rebuild must not drop it. TrustedJwtIssuerProperties props = new TrustedJwtIssuerProperties(); AuthManagerResolver multi = buildResolverWithTrustedIssuers(props); @@ -823,8 +815,7 @@ void trustedIssuers_primaryIssuerSurvivesRebuild() { @Test void trustedIssuers_conflictingJwksOnRebuild_failsLoudly() { - // Re-declaring an already-trusted issuer with a different JWKS is rejected on rebuild - // exactly as it is at startup, rather than silently keeping stale trust. + // Rejected on rebuild exactly as at startup, rather than silently keeping stale trust. TrustedJwtIssuerProperties props = new TrustedJwtIssuerProperties(); AuthManagerResolver multi = buildResolverWithTrustedIssuers(props); assertNotNull(multi.authenticationManagerResolver()); 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; + } +} diff --git a/src/control-plane-services/instance-cluster-management/icms-core/src/test/java/com/nvidia/icms/configuration/security/TrustedIssuerRefreshTest.java b/src/control-plane-services/instance-cluster-management/icms-core/src/test/java/com/nvidia/icms/configuration/security/TrustedIssuerRefreshTest.java deleted file mode 100644 index 6376fe8541..0000000000 --- a/src/control-plane-services/instance-cluster-management/icms-core/src/test/java/com/nvidia/icms/configuration/security/TrustedIssuerRefreshTest.java +++ /dev/null @@ -1,165 +0,0 @@ -/* - * 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.mockito.Mockito.*; - -import com.nvidia.icms.configuration.nvca.NvcaConfigurationProperties; -import com.nvidia.icms.outbound.apikeys.ApiKeysService; -import com.nvidia.icms.outbound.cassandra.byoc.ClusterRepository; -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.cloud.autoconfigure.RefreshAutoConfiguration; -import org.springframework.cloud.context.refresh.ContextRefresher; -import org.springframework.context.annotation.AnnotationConfigApplicationContext; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; -import org.springframework.core.env.MapPropertySource; -import org.springframework.security.authentication.AuthenticationManagerResolver; -import org.springframework.security.authentication.AuthenticationServiceException; - -/** - * Ties the {@code @RefreshScope} annotations on {@link AuthManagerResolver} and - * {@link TrustedJwtIssuerProperties} to the behavior they exist for, by driving a real - * {@link ContextRefresher#refresh()} over the real beans through the same resolver handle a - * {@code SecurityFilterChain} holds. - * - *

This is not redundant with {@code AuthManagerResolverTest}: those tests call the factory - * method directly and pass whether or not either annotation is present. Without this test, - * removing {@code @RefreshScope} silently restores the defect this class exists to prevent — - * configuration accepted, never applied, and revocation that appears to work but does not.

- */ -class TrustedIssuerRefreshTest { - - private static final String APPLICATION_NAME = "spring.application.name"; - private static final String ACTIVE_PROFILES = "spring.profiles.active"; - - private static final String PRIMARY_ISSUER = "http://api.sis.svc.cluster.local"; - private static final String PRIMARY_JWKS = "http://openbao/v1/services/sis-api/jwt/jwks"; - 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"; - - @Test - void trustedIssuerAddedAndRemovedAtRuntime_appliesWithoutRestart() { - Map source = new HashMap<>(Map.of( - "spring.security.oauth2.resourceserver.jwt.issuer-uri", PRIMARY_ISSUER, - "spring.security.oauth2.resourceserver.jwt.jwk-set-uri", PRIMARY_JWKS, - "icms.nvca.api-key.enabled", "false")); - - // ContextRefresher re-reads the Environment through a SpringApplication, which this - // codebase's ValidateEnvironmentPostProcessor gates on these two being set. Tests share - // one JVM, so capture whatever was there and put it back rather than clearing blindly. - String priorName = System.getProperty(APPLICATION_NAME); - String priorProfiles = System.getProperty(ACTIVE_PROFILES); - System.setProperty(APPLICATION_NAME, "trusted-issuer-refresh-test"); - System.setProperty(ACTIVE_PROFILES, "test"); - try (var ctx = new AnnotationConfigApplicationContext()) { - ctx.getEnvironment().getPropertySources() - .addFirst(new MapPropertySource("test-source", source)); - ctx.register(RefreshAutoConfiguration.class, ResolverDependencies.class, - TrustedJwtIssuerProperties.class, AuthManagerResolver.class); - ctx.refresh(); - - @SuppressWarnings("unchecked") - AuthenticationManagerResolver resolver = - ctx.getBean(AuthenticationManagerResolver.class); - - HttpServletRequest primary = requestWithIssuer(PRIMARY_ISSUER); - HttpServletRequest added = requestWithIssuer(ADDED_ISSUER); - - assertNotNull(resolver.resolve(primary), "primary issuer must be trusted at startup"); - assertThrows(AuthenticationServiceException.class, () -> resolver.resolve(added), - "issuer must not be trusted before it is configured"); - - // Add the issuer the way an operator would — a new entry in the property source. - source.put(TRUSTED_ISSUER_URI, ADDED_ISSUER); - source.put(TRUSTED_JWK_SET_URI, ADDED_JWKS); - ctx.getBean(ContextRefresher.class).refresh(); - - assertNotNull(resolver.resolve(added), - "issuer added at runtime must be accepted after a refresh, without a restart"); - assertNotNull(resolver.resolve(primary), - "primary issuer must still be trusted after the refresh"); - - // Revocation must apply too — this is the case a stale set gets silently wrong. - source.remove(TRUSTED_ISSUER_URI); - source.remove(TRUSTED_JWK_SET_URI); - ctx.getBean(ContextRefresher.class).refresh(); - - assertEquals(List.of(), - ctx.getBean(TrustedJwtIssuerProperties.class).getTrustedIssuers(), - "the removal must reach the bound properties"); - assertThrows(AuthenticationServiceException.class, () -> resolver.resolve(added), - "issuer removed at runtime must stop being trusted after a refresh"); - assertNotNull(resolver.resolve(primary), - "primary issuer must survive the removal refresh"); - } finally { - restoreProperty(APPLICATION_NAME, priorName); - restoreProperty(ACTIVE_PROFILES, priorProfiles); - } - } - - @Configuration(proxyBeanMethods = false) - static class ResolverDependencies { - - @Bean - ApiKeysService apiKeysService() { - return mock(ApiKeysService.class); - } - - @Bean - ClusterRepository clusterRepository() { - return mock(ClusterRepository.class); - } - - @Bean - NvcaConfigurationProperties nvcaConfigurationProperties() { - // Flag on so an untrusted iss falls through to cluster OIDC and throws on the missing - // nvcf-icms:{clusterId} audience. That throw is what distinguishes "not trusted" here: - // JwtIssuerAuthenticationManagerResolver.resolve() returns a manager either way and - // defers the issuer lookup to authenticate(). - NvcaConfigurationProperties cfg = new NvcaConfigurationProperties(); - cfg.setOidcClusterIdentityEnabled(true); - return cfg; - } - } - - 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()); - HttpServletRequest req = mock(HttpServletRequest.class); - when(req.getHeader("Authorization")) - .thenReturn("Bearer eyJhbGciOiJFUzI1NiJ9." + b64 + ".sig"); - return req; - } - - private static void restoreProperty(String key, String priorValue) { - if (priorValue == null) { - System.clearProperty(key); - } else { - System.setProperty(key, priorValue); - } - } -}