From 65a2af3c5d12ed8c1f22d3708ed046d3ce335d66 Mon Sep 17 00:00:00 2001 From: "Donald F. Coffin" Date: Fri, 13 Nov 2026 01:51:25 -0500 Subject: [PATCH 1/6] test(#129): drop impossible security assertions from OAuthAdminControllerTest The standalone MockMvc setup installs no security filter chain and no method-security proxy, so the 401/403 (and "allowed") tests could never exercise access control. Admin API authorization is covered against the real configuration by AdminEndpointAuthorizationIntegrationTest and AdminBearerTokenIntegrationTest. The request-handling tests stay. Co-Authored-By: Claude Opus 5.5 --- .../controller/OAuthAdminControllerTest.java | 88 +++---------------- 1 file changed, 10 insertions(+), 78 deletions(-) diff --git a/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/controller/OAuthAdminControllerTest.java b/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/controller/OAuthAdminControllerTest.java index c62d1b95..5a87bb81 100644 --- a/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/controller/OAuthAdminControllerTest.java +++ b/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/controller/OAuthAdminControllerTest.java @@ -52,11 +52,16 @@ import static org.hamcrest.CoreMatchers.containsString; /** - * Comprehensive unit tests for OAuthAdminController - * - * Tests administrative OAuth2 operations including token management, - * client management, and authorization tracking with proper security. - * + * Unit tests for OAuthAdminController request handling (token management, + * client management, and authorization tracking). + * + *

These tests use {@code MockMvcBuilders.standaloneSetup}, which installs NO Spring Security + * filter chain and no method-security proxy, so {@code @WithMockUser} has no effect here and access + * control cannot be asserted. Authorization of the admin APIs (anonymous, customer session, ADMIN / + * DC_ADMIN with both MFA factors, {@code @PreAuthorize} enforcement) is covered against the real + * security configuration by {@code integration.AdminEndpointAuthorizationIntegrationTest} and + * {@code integration.AdminBearerTokenIntegrationTest} (#129).

+ * * @author Green Button Alliance * @version 1.0.0 * @since Spring Boot 3.5 @@ -117,15 +122,6 @@ void shouldListTokensWithDcAdminRole() throws Exception { .andExpect(jsonPath("$").isArray()); } - @Test - @WithMockUser(roles = "USER") - @DisplayName("Should deny access to tokens with user role") - void shouldDenyAccessToTokensWithUserRole() throws Exception { - // When & Then - mockMvc.perform(get("/admin/oauth2/tokens")) - .andExpect(status().isForbidden()); - } - @Test @WithMockUser(roles = "ADMIN") @DisplayName("Should filter tokens by client ID") @@ -418,70 +414,6 @@ void shouldAllowDcAdminToViewAuthorizations() throws Exception { } } - @Nested - @DisplayName("Security Tests") - class SecurityTests { - - @Test - @DisplayName("Should deny unauthenticated access to admin endpoints") - void shouldDenyUnauthenticatedAccessToAdminEndpoints() throws Exception { - // When & Then - mockMvc.perform(get("/admin/oauth2/tokens")) - .andExpect(status().isUnauthorized()); - - mockMvc.perform(get("/admin/oauth2/clients")) - .andExpect(status().isUnauthorized()); - - mockMvc.perform(get("/admin/oauth2/authorizations")) - .andExpect(status().isUnauthorized()); - } - - @Test - @WithMockUser(roles = "USER") - @DisplayName("Should deny user role access to admin endpoints") - void shouldDenyUserRoleAccessToAdminEndpoints() throws Exception { - // When & Then - mockMvc.perform(get("/admin/oauth2/tokens")) - .andExpect(status().isForbidden()); - - mockMvc.perform(get("/admin/oauth2/clients")) - .andExpect(status().isForbidden()); - - mockMvc.perform(get("/admin/oauth2/authorizations")) - .andExpect(status().isForbidden()); - } - - @Test - @WithMockUser(roles = "CUSTOMER") - @DisplayName("Should deny customer role access to admin endpoints") - void shouldDenyCustomerRoleAccessToAdminEndpoints() throws Exception { - // When & Then - mockMvc.perform(delete("/admin/oauth2/tokens/test-token")) - .andExpect(status().isForbidden()); - - mockMvc.perform(delete("/admin/oauth2/clients/test-client")) - .andExpect(status().isForbidden()); - } - - @Test - @WithMockUser(authorities = "ROLE_ADMIN") - @DisplayName("Should allow ROLE_ADMIN authority access") - void shouldAllowRoleAdminAuthorityAccess() throws Exception { - // When & Then - mockMvc.perform(get("/admin/oauth2/tokens")) - .andExpect(status().isOk()); - } - - @Test - @WithMockUser(authorities = "ROLE_DC_ADMIN") - @DisplayName("Should allow ROLE_DC_ADMIN authority access") - void shouldAllowRoleDcAdminAuthorityAccess() throws Exception { - // When & Then - mockMvc.perform(get("/admin/oauth2/clients")) - .andExpect(status().isOk()); - } - } - @Nested @DisplayName("ESPI Compliance Tests") class EspiComplianceTests { From 282736c16021d96ff8cfc7f685db664d6a5eb76b Mon Sep 17 00:00:00 2001 From: "Donald F. Coffin" Date: Fri, 13 Nov 2026 01:51:32 -0500 Subject: [PATCH 2/6] test(#129): drive EspiTokenCustomizerTest with a real JwtEncodingContext Replace the strict-stubbed JwtEncodingContext mocks (UnnecessaryStubbing, NPE on null claims) with a real context and assertions on the resulting claims. Role-to-resource-URI and customer-grant cases become parameterized; duplicate "ESPI compliance" cases and null-context/null-claims/null-endpoint cases (outside the framework contract) are removed. Co-Authored-By: Claude Opus 5.5 --- .../service/EspiTokenCustomizerTest.java | 574 ++++-------------- 1 file changed, 111 insertions(+), 463 deletions(-) diff --git a/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/service/EspiTokenCustomizerTest.java b/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/service/EspiTokenCustomizerTest.java index 87feefc9..73152ec1 100644 --- a/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/service/EspiTokenCustomizerTest.java +++ b/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/service/EspiTokenCustomizerTest.java @@ -24,47 +24,46 @@ import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.junit.jupiter.MockitoExtension; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.ValueSource; +import org.springframework.security.authentication.TestingAuthenticationToken; import org.springframework.security.core.Authentication; -import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.security.oauth2.core.AuthorizationGrantType; -import org.springframework.security.oauth2.server.authorization.OAuth2TokenType; +import org.springframework.security.oauth2.core.ClientAuthenticationMethod; +import org.springframework.security.oauth2.jose.jws.SignatureAlgorithm; +import org.springframework.security.oauth2.jwt.JwsHeader; import org.springframework.security.oauth2.jwt.JwtClaimsSet; +import org.springframework.security.oauth2.server.authorization.OAuth2TokenType; import org.springframework.security.oauth2.server.authorization.client.RegisteredClient; import org.springframework.security.oauth2.server.authorization.token.JwtEncodingContext; import org.springframework.test.util.ReflectionTestUtils; -import java.util.Collection; -import java.util.List; +import java.util.HashMap; +import java.util.Map; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.*; -import static org.mockito.ArgumentMatchers.*; /** - * Comprehensive unit tests for EspiTokenCustomizer - * - * Tests ESPI-specific JWT token customization including Green Button Alliance - * standard claims, client type determination, and resource URI generation. - * - * Note: ESPI standard uses opaque tokens, JWT customization is experimental. - * - * @author Green Button Alliance - * @version 1.0.0 - * @since Spring Boot 3.5 + * Unit tests for {@link EspiTokenCustomizer}. + * + *

The customizer is EXPERIMENTAL: it only fires when {@code espi.token.format=jwt}; the ESPI + * standard (and this AS's default) is opaque tokens. The tests drive it with a real + * {@link JwtEncodingContext} and inspect the resulting claims, rather than mocking the context + * and verifying builder calls (#129).

*/ -@ExtendWith(MockitoExtension.class) @DisplayName("EspiTokenCustomizer Tests") class EspiTokenCustomizerTest { + private static final String DC_ENDPOINT = "http://localhost:8080"; + private static final String RESOURCE_BASE = DC_ENDPOINT + "/espi/1_1/resource"; + private EspiTokenCustomizer customizer; @BeforeEach void setUp() { customizer = new EspiTokenCustomizer(); - // Set default property values using reflection - ReflectionTestUtils.setField(customizer, "dataCustodianEndpoint", "http://localhost:8080"); + ReflectionTestUtils.setField(customizer, "dataCustodianEndpoint", DC_ENDPOINT); ReflectionTestUtils.setField(customizer, "tokenFormat", "jwt"); } @@ -72,71 +71,32 @@ void setUp() { @DisplayName("Token Format Tests") class TokenFormatTests { - @Test - @DisplayName("Should skip customization when token format is opaque (ESPI standard)") - void shouldSkipCustomizationWhenTokenFormatIsOpaque() { - // Given - ReflectionTestUtils.setField(customizer, "tokenFormat", "opaque"); - - JwtEncodingContext context = createMockJwtEncodingContext(); - JwtClaimsSet.Builder claimsBuilder = spy(JwtClaimsSet.builder()); - when(context.getClaims()).thenReturn(claimsBuilder); - - // When - customizer.customize(context); - - // Then - verify(claimsBuilder, never()).claim(anyString(), any()); - } - - @Test - @DisplayName("Should skip customization when token format is reference") - void shouldSkipCustomizationWhenTokenFormatIsReference() { - // Given - ReflectionTestUtils.setField(customizer, "tokenFormat", "reference"); - - JwtEncodingContext context = createMockJwtEncodingContext(); - JwtClaimsSet.Builder claimsBuilder = spy(JwtClaimsSet.builder()); - when(context.getClaims()).thenReturn(claimsBuilder); - - // When - customizer.customize(context); + @ParameterizedTest(name = "espi.token.format={0} -> no ESPI claims") + @ValueSource(strings = {"opaque", "reference"}) + @DisplayName("Should skip customization when the token format is not jwt (ESPI standard)") + void shouldSkipCustomizationWhenTokenFormatIsNotJwt(String format) { + ReflectionTestUtils.setField(customizer, "tokenFormat", format); - // Then - verify(claimsBuilder, never()).claim(anyString(), any()); + assertThat(customize(clientCredentials("data_custodian_admin", "ROLE_DC_ADMIN"))) + .isEmpty(); } @Test - @DisplayName("Should customize tokens when format is jwt (experimental)") + @DisplayName("Should add the base ESPI claims when the format is jwt (experimental)") void shouldCustomizeTokensWhenFormatIsJwt() { - // Given - JwtEncodingContext context = createMockJwtEncodingContext(); - JwtClaimsSet.Builder claimsBuilder = spy(JwtClaimsSet.builder()); - when(context.getClaims()).thenReturn(claimsBuilder); - - // When - customizer.customize(context); - - // Then - verify(claimsBuilder).claim("espi_version", "1.1"); - verify(claimsBuilder).claim("data_custodian_endpoint", "http://localhost:8080"); + assertThat(customize(clientCredentials("data_custodian_admin", "ROLE_DC_ADMIN"))) + .containsEntry("espi_version", "1.1") + .containsEntry("data_custodian_endpoint", DC_ENDPOINT); } @Test @DisplayName("Should only customize access tokens") void shouldOnlyCustomizeAccessTokens() { - // Given - JwtEncodingContext context = createMockJwtEncodingContext(); - when(context.getTokenType()).thenReturn(OAuth2TokenType.REFRESH_TOKEN); - - JwtClaimsSet.Builder claimsBuilder = spy(JwtClaimsSet.builder()); - when(context.getClaims()).thenReturn(claimsBuilder); - - // When - customizer.customize(context); + JwtEncodingContext.Builder refreshToken = context("third_party", + AuthorizationGrantType.AUTHORIZATION_CODE, customer()) + .tokenType(OAuth2TokenType.REFRESH_TOKEN); - // Then - verify(claimsBuilder, never()).claim(anyString(), any()); + assertThat(customize(refreshToken)).isEmpty(); } } @@ -147,415 +107,103 @@ class ClientCredentialsGrantTests { @Test @DisplayName("Should customize token for DataCustodian admin client") void shouldCustomizeTokenForDataCustodianAdminClient() { - // Given - JwtEncodingContext context = createClientCredentialsContext("data_custodian_admin", "ROLE_DC_ADMIN"); - JwtClaimsSet.Builder claimsBuilder = spy(JwtClaimsSet.builder()); - when(context.getClaims()).thenReturn(claimsBuilder); - - // When - customizer.customize(context); - - // Then - verify(claimsBuilder).claim("espi_version", "1.1"); - verify(claimsBuilder).claim("data_custodian_endpoint", "http://localhost:8080"); - verify(claimsBuilder).claim("espi_client_type", "admin"); - verify(claimsBuilder).claim("espi_grant_type", "client_credentials"); - verify(claimsBuilder).claim("resource_uri", "http://localhost:8080/espi/1_1/resource/"); - verify(claimsBuilder).claim(eq("authorization_uri"), argThat(uri -> - uri.toString().startsWith("http://localhost:8080/espi/1_1/resource/Authorization/"))); - } - - @Test - @DisplayName("Should customize token for ThirdParty admin client") - void shouldCustomizeTokenForThirdPartyAdminClient() { - // Given - JwtEncodingContext context = createClientCredentialsContext("third_party_admin", "ROLE_TP_ADMIN"); - JwtClaimsSet.Builder claimsBuilder = spy(JwtClaimsSet.builder()); - when(context.getClaims()).thenReturn(claimsBuilder); - - // When - customizer.customize(context); - - // Then - verify(claimsBuilder).claim("espi_version", "1.1"); - verify(claimsBuilder).claim("espi_client_type", "admin"); - verify(claimsBuilder).claim("espi_grant_type", "client_credentials"); - verify(claimsBuilder).claim("resource_uri", "http://localhost:8080/espi/1_1/resource/Batch/Bulk/**"); - } - - @Test - @DisplayName("Should customize token for Upload admin client") - void shouldCustomizeTokenForUploadAdminClient() { - // Given - JwtEncodingContext context = createClientCredentialsContext("upload_admin", "ROLE_UL_ADMIN"); - JwtClaimsSet.Builder claimsBuilder = spy(JwtClaimsSet.builder()); - when(context.getClaims()).thenReturn(claimsBuilder); - - // When - customizer.customize(context); - - // Then - verify(claimsBuilder).claim("resource_uri", "http://localhost:8080/espi/1_1/resource/Batch/Upload/**"); - } - - @Test - @DisplayName("Should customize token for Registration admin client") - void shouldCustomizeTokenForRegistrationAdminClient() { - // Given - JwtEncodingContext context = createClientCredentialsContext("registration_admin", "ROLE_TP_REGISTRATION"); - JwtClaimsSet.Builder claimsBuilder = spy(JwtClaimsSet.builder()); - when(context.getClaims()).thenReturn(claimsBuilder); - - // When - customizer.customize(context); - - // Then - verify(claimsBuilder).claim("resource_uri", "http://localhost:8080/espi/1_1/resource/ApplicationInformation/**"); - } - - @Test - @DisplayName("Should use default resource URI for unknown roles") - void shouldUseDefaultResourceUriForUnknownRoles() { - // Given - JwtEncodingContext context = createClientCredentialsContext("unknown_client", "ROLE_UNKNOWN"); - JwtClaimsSet.Builder claimsBuilder = spy(JwtClaimsSet.builder()); - when(context.getClaims()).thenReturn(claimsBuilder); - - // When - customizer.customize(context); - - // Then - verify(claimsBuilder).claim("resource_uri", "http://localhost:8080/espi/1_1/resource/"); - } - - @Test - @DisplayName("Should handle multiple authorities correctly") + assertThat(customize(clientCredentials("data_custodian_admin", "ROLE_DC_ADMIN"))) + .containsEntry("espi_version", "1.1") + .containsEntry("data_custodian_endpoint", DC_ENDPOINT) + .containsEntry("espi_client_type", "admin") + .containsEntry("espi_grant_type", "client_credentials") + .containsEntry("resource_uri", RESOURCE_BASE + "/") + .hasEntrySatisfying("authorization_uri", uri -> assertThat(uri.toString()) + .matches(RESOURCE_BASE.replace(".", "\\.") + "/Authorization/\\d+")); + } + + @ParameterizedTest(name = "{0} -> {1}") + @CsvSource({ + "ROLE_DC_ADMIN, /", + "ROLE_TP_ADMIN, /Batch/Bulk/**", + "ROLE_UL_ADMIN, /Batch/Upload/**", + "ROLE_TP_REGISTRATION, /ApplicationInformation/**", + "ROLE_UNKNOWN, /" + }) + @DisplayName("Should derive the resource URI from the client's role") + void shouldDeriveResourceUriFromRole(String authority, String resourcePath) { + assertThat(customize(clientCredentials("admin_client", authority))) + .containsEntry("resource_uri", RESOURCE_BASE + resourcePath); + } + + @Test + @DisplayName("Should prioritize DC_ADMIN when a client holds several admin roles") void shouldHandleMultipleAuthoritiesCorrectly() { - // Given - Authentication principal = mock(Authentication.class); - when(principal.getAuthorities()).thenReturn((Collection) List.of( - new SimpleGrantedAuthority("ROLE_USER"), - new SimpleGrantedAuthority("ROLE_DC_ADMIN"), - new SimpleGrantedAuthority("ROLE_TP_ADMIN") - )); + Authentication principal = new TestingAuthenticationToken("multi_role_client", null, + "ROLE_USER", "ROLE_DC_ADMIN", "ROLE_TP_ADMIN"); - JwtEncodingContext context = createClientCredentialsContextWithPrincipal("multi_role_client", principal); - JwtClaimsSet.Builder claimsBuilder = spy(JwtClaimsSet.builder()); - when(context.getClaims()).thenReturn(claimsBuilder); - - // When - customizer.customize(context); - - // Then - Should prioritize DC_ADMIN - verify(claimsBuilder).claim("resource_uri", "http://localhost:8080/espi/1_1/resource/"); + assertThat(customize(context("multi_role_client", AuthorizationGrantType.CLIENT_CREDENTIALS, principal))) + .containsEntry("resource_uri", RESOURCE_BASE + "/"); } - } - - @Nested - @DisplayName("Authorization Code Grant Tests") - class AuthorizationCodeGrantTests { @Test - @DisplayName("Should customize token for authorization code grant") - void shouldCustomizeTokenForAuthorizationCodeGrant() { - // Given - JwtEncodingContext context = createAuthorizationCodeContext("third_party"); - JwtClaimsSet.Builder claimsBuilder = spy(JwtClaimsSet.builder()); - when(context.getClaims()).thenReturn(claimsBuilder); - - // When - customizer.customize(context); - - // Then - verify(claimsBuilder).claim("espi_version", "1.1"); - verify(claimsBuilder).claim("data_custodian_endpoint", "http://localhost:8080"); - verify(claimsBuilder).claim("espi_client_type", "customer"); - verify(claimsBuilder).claim("espi_grant_type", "authorization_code"); - verify(claimsBuilder).claim("resource_uri", "http://localhost:8080/espi/1_1/resource/"); - verify(claimsBuilder).claim(eq("authorization_uri"), argThat(uri -> - uri.toString().startsWith("http://localhost:8080/espi/1_1/resource/Authorization/"))); - } - - @Test - @DisplayName("Should customize token for refresh token grant") - void shouldCustomizeTokenForRefreshTokenGrant() { - // Given - JwtEncodingContext context = createRefreshTokenContext("third_party"); - JwtClaimsSet.Builder claimsBuilder = spy(JwtClaimsSet.builder()); - when(context.getClaims()).thenReturn(claimsBuilder); - - // When - customizer.customize(context); - - // Then - verify(claimsBuilder).claim("espi_version", "1.1"); - verify(claimsBuilder).claim("espi_client_type", "customer"); - verify(claimsBuilder).claim("espi_grant_type", "authorization_code"); - verify(claimsBuilder).claim("resource_uri", "http://localhost:8080/espi/1_1/resource/"); - } - } - - @Nested - @DisplayName("Configuration Tests") - class ConfigurationTests { - - @Test - @DisplayName("Should use custom data custodian endpoint") + @DisplayName("Should use the configured data custodian endpoint") void shouldUseCustomDataCustodianEndpoint() { - // Given ReflectionTestUtils.setField(customizer, "dataCustodianEndpoint", "https://datacustodian.example.com"); - - JwtEncodingContext context = createClientCredentialsContext("test_client", "ROLE_DC_ADMIN"); - JwtClaimsSet.Builder claimsBuilder = spy(JwtClaimsSet.builder()); - when(context.getClaims()).thenReturn(claimsBuilder); - - // When - customizer.customize(context); - - // Then - verify(claimsBuilder).claim("data_custodian_endpoint", "https://datacustodian.example.com"); - verify(claimsBuilder).claim("resource_uri", "https://datacustodian.example.com/espi/1_1/resource/"); - verify(claimsBuilder).claim(eq("authorization_uri"), argThat(uri -> - uri.toString().startsWith("https://datacustodian.example.com/espi/1_1/resource/Authorization/"))); - } - - @Test - @DisplayName("Should handle null data custodian endpoint gracefully") - void shouldHandleNullDataCustodianEndpointGracefully() { - // Given - ReflectionTestUtils.setField(customizer, "dataCustodianEndpoint", null); - - JwtEncodingContext context = createClientCredentialsContext("test_client", "ROLE_DC_ADMIN"); - JwtClaimsSet.Builder claimsBuilder = spy(JwtClaimsSet.builder()); - when(context.getClaims()).thenReturn(claimsBuilder); - - // When - customizer.customize(context); - - // Then - verify(claimsBuilder).claim("data_custodian_endpoint", null); - verify(claimsBuilder).claim("resource_uri", "null/espi/1_1/resource/"); - } - } - - @Nested - @DisplayName("Authorization ID Generation Tests") - class AuthorizationIdGenerationTests { - - @Test - @DisplayName("Should generate unique authorization IDs") - void shouldGenerateUniqueAuthorizationIds() { - // Given - JwtEncodingContext context1 = createClientCredentialsContext("client1", "ROLE_DC_ADMIN"); - JwtEncodingContext context2 = createClientCredentialsContext("client2", "ROLE_DC_ADMIN"); - - JwtClaimsSet.Builder claimsBuilder1 = spy(JwtClaimsSet.builder()); - JwtClaimsSet.Builder claimsBuilder2 = spy(JwtClaimsSet.builder()); - - when(context1.getClaims()).thenReturn(claimsBuilder1); - when(context2.getClaims()).thenReturn(claimsBuilder2); - - // When - customizer.customize(context1); - try { - Thread.sleep(1); // Ensure different timestamps - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } - customizer.customize(context2); - - // Then - verify(claimsBuilder1).claim(eq("authorization_uri"), argThat(uri -> - uri.toString().matches("http://localhost:8080/espi/1_1/resource/Authorization/\\d+"))); - verify(claimsBuilder2).claim(eq("authorization_uri"), argThat(uri -> - uri.toString().matches("http://localhost:8080/espi/1_1/resource/Authorization/\\d+"))); - } - } - - @Nested - @DisplayName("ESPI Compliance Tests") - class EspiComplianceTests { - - @Test - @DisplayName("Should add ESPI version 1.1 claim") - void shouldAddEspiVersion11Claim() { - // Given - JwtEncodingContext context = createClientCredentialsContext("test_client", "ROLE_DC_ADMIN"); - JwtClaimsSet.Builder claimsBuilder = spy(JwtClaimsSet.builder()); - when(context.getClaims()).thenReturn(claimsBuilder); - - // When - customizer.customize(context); - - // Then - verify(claimsBuilder).claim("espi_version", "1.1"); - } - - @Test - @DisplayName("Should use ESPI-compliant resource URI patterns") - void shouldUseEspiCompliantResourceUriPatterns() { - // Given - JwtEncodingContext context = createClientCredentialsContext("test_client", "ROLE_TP_ADMIN"); - JwtClaimsSet.Builder claimsBuilder = spy(JwtClaimsSet.builder()); - when(context.getClaims()).thenReturn(claimsBuilder); - - // When - customizer.customize(context); - - // Then - verify(claimsBuilder).claim("resource_uri", "http://localhost:8080/espi/1_1/resource/Batch/Bulk/**"); - } - - @Test - @DisplayName("Should generate ESPI-compliant authorization URIs") - void shouldGenerateEspiCompliantAuthorizationUris() { - // Given - JwtEncodingContext context = createAuthorizationCodeContext("third_party"); - JwtClaimsSet.Builder claimsBuilder = spy(JwtClaimsSet.builder()); - when(context.getClaims()).thenReturn(claimsBuilder); - - // When - customizer.customize(context); - - // Then - verify(claimsBuilder).claim(eq("authorization_uri"), argThat(uri -> - uri.toString().startsWith("http://localhost:8080/espi/1_1/resource/Authorization/"))); - } - - @Test - @DisplayName("Should distinguish between admin and customer client types") - void shouldDistinguishBetweenAdminAndCustomerClientTypes() { - // Given - JwtEncodingContext adminContext = createClientCredentialsContext("admin_client", "ROLE_DC_ADMIN"); - JwtEncodingContext customerContext = createAuthorizationCodeContext("customer_client"); - - JwtClaimsSet.Builder adminClaimsBuilder = spy(JwtClaimsSet.builder()); - JwtClaimsSet.Builder customerClaimsBuilder = spy(JwtClaimsSet.builder()); - - when(adminContext.getClaims()).thenReturn(adminClaimsBuilder); - when(customerContext.getClaims()).thenReturn(customerClaimsBuilder); - - // When - customizer.customize(adminContext); - customizer.customize(customerContext); - // Then - verify(adminClaimsBuilder).claim("espi_client_type", "admin"); - verify(customerClaimsBuilder).claim("espi_client_type", "customer"); + assertThat(customize(clientCredentials("test_client", "ROLE_DC_ADMIN"))) + .containsEntry("data_custodian_endpoint", "https://datacustodian.example.com") + .containsEntry("resource_uri", "https://datacustodian.example.com/espi/1_1/resource/") + .hasEntrySatisfying("authorization_uri", uri -> assertThat(uri.toString()) + .startsWith("https://datacustodian.example.com/espi/1_1/resource/Authorization/")); } } @Nested - @DisplayName("Error Handling Tests") - class ErrorHandlingTests { - - @Test - @DisplayName("Should handle null context gracefully") - void shouldHandleNullContextGracefully() { - // When & Then - Should not throw exception - customizer.customize(null); - } - - @Test - @DisplayName("Should handle context with null claims gracefully") - void shouldHandleContextWithNullClaimsGracefully() { - // Given - JwtEncodingContext context = createMockJwtEncodingContext(); - when(context.getClaims()).thenReturn(null); - - // When & Then - Should not throw exception - customizer.customize(context); - } - - @Test - @DisplayName("Should handle context with null principal gracefully") - void shouldHandleContextWithNullPrincipalGracefully() { - // Given - JwtEncodingContext context = createMockJwtEncodingContext(); - when(context.getPrincipal()).thenReturn(null); - - JwtClaimsSet.Builder claimsBuilder = spy(JwtClaimsSet.builder()); - when(context.getClaims()).thenReturn(claimsBuilder); + @DisplayName("Authorization Code Grant Tests") + class AuthorizationCodeGrantTests { - // When & Then - Should not throw exception - customizer.customize(context); - - // Should still add basic claims - verify(claimsBuilder).claim("espi_version", "1.1"); + @ParameterizedTest(name = "grant_type={0} -> customer claims") + @ValueSource(strings = {"authorization_code", "refresh_token"}) + @DisplayName("Should add customer claims for authorization_code and refresh_token grants") + void shouldCustomizeTokenForCustomerGrants(String grantType) { + assertThat(customize(context("third_party", new AuthorizationGrantType(grantType), customer()))) + .containsEntry("espi_version", "1.1") + .containsEntry("data_custodian_endpoint", DC_ENDPOINT) + .containsEntry("espi_client_type", "customer") + .containsEntry("espi_grant_type", "authorization_code") + .containsEntry("resource_uri", RESOURCE_BASE + "/") + .hasEntrySatisfying("authorization_uri", uri -> assertThat(uri.toString()) + .matches(RESOURCE_BASE.replace(".", "\\.") + "/Authorization/\\d+")); } } - // Helper methods - - private JwtEncodingContext createMockJwtEncodingContext() { - JwtEncodingContext context = mock(JwtEncodingContext.class); - when(context.getTokenType()).thenReturn(OAuth2TokenType.ACCESS_TOKEN); - - RegisteredClient client = mock(RegisteredClient.class); - when(client.getClientId()).thenReturn("test-client"); - when(context.getRegisteredClient()).thenReturn(client); - - when(context.getAuthorizationGrantType()).thenReturn(AuthorizationGrantType.AUTHORIZATION_CODE); - - Authentication principal = mock(Authentication.class); - when(principal.getAuthorities()).thenReturn((Collection) List.of(new SimpleGrantedAuthority("ROLE_USER"))); - when(context.getPrincipal()).thenReturn(principal); - - return context; - } + // Helpers - private JwtEncodingContext createClientCredentialsContext(String clientId, String authority) { - Authentication principal = mock(Authentication.class); - when(principal.getAuthorities()).thenReturn((Collection) List.of(new SimpleGrantedAuthority(authority))); - - return createClientCredentialsContextWithPrincipal(clientId, principal); + private Map customize(JwtEncodingContext.Builder contextBuilder) { + JwtEncodingContext context = contextBuilder.build(); + customizer.customize(context); + Map claims = new HashMap<>(); + context.getClaims().claims(claims::putAll); + return claims; } - private JwtEncodingContext createClientCredentialsContextWithPrincipal(String clientId, Authentication principal) { - JwtEncodingContext context = mock(JwtEncodingContext.class); - when(context.getTokenType()).thenReturn(OAuth2TokenType.ACCESS_TOKEN); - - RegisteredClient client = mock(RegisteredClient.class); - when(client.getClientId()).thenReturn(clientId); - when(context.getRegisteredClient()).thenReturn(client); - - when(context.getAuthorizationGrantType()).thenReturn(AuthorizationGrantType.CLIENT_CREDENTIALS); - when(context.getPrincipal()).thenReturn(principal); - - return context; + private static JwtEncodingContext.Builder clientCredentials(String clientId, String authority) { + return context(clientId, AuthorizationGrantType.CLIENT_CREDENTIALS, + new TestingAuthenticationToken(clientId, null, authority)); } - private JwtEncodingContext createAuthorizationCodeContext(String clientId) { - JwtEncodingContext context = mock(JwtEncodingContext.class); - when(context.getTokenType()).thenReturn(OAuth2TokenType.ACCESS_TOKEN); - - RegisteredClient client = mock(RegisteredClient.class); - when(client.getClientId()).thenReturn(clientId); - when(context.getRegisteredClient()).thenReturn(client); - - when(context.getAuthorizationGrantType()).thenReturn(AuthorizationGrantType.AUTHORIZATION_CODE); - - Authentication principal = mock(Authentication.class); - when(principal.getAuthorities()).thenReturn((Collection) List.of(new SimpleGrantedAuthority("ROLE_USER"))); - when(context.getPrincipal()).thenReturn(principal); - - return context; + private static Authentication customer() { + return new TestingAuthenticationToken("42", null, "ROLE_CUSTOMER"); } - private JwtEncodingContext createRefreshTokenContext(String clientId) { - JwtEncodingContext context = mock(JwtEncodingContext.class); - when(context.getTokenType()).thenReturn(OAuth2TokenType.ACCESS_TOKEN); - - RegisteredClient client = mock(RegisteredClient.class); - when(client.getClientId()).thenReturn(clientId); - when(context.getRegisteredClient()).thenReturn(client); - - when(context.getAuthorizationGrantType()).thenReturn(AuthorizationGrantType.REFRESH_TOKEN); - - Authentication principal = mock(Authentication.class); - when(principal.getAuthorities()).thenReturn((Collection) List.of(new SimpleGrantedAuthority("ROLE_USER"))); - when(context.getPrincipal()).thenReturn(principal); - - return context; + private static JwtEncodingContext.Builder context(String clientId, AuthorizationGrantType grantType, + Authentication principal) { + RegisteredClient client = RegisteredClient.withId(clientId + "-id") + .clientId(clientId) + .clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC) + .authorizationGrantType(grantType) + .redirectUri("https://tp.example/cb") + .build(); + return JwtEncodingContext.with(JwsHeader.with(SignatureAlgorithm.RS256), JwtClaimsSet.builder()) + .registeredClient(client) + .principal(principal) + .authorizationGrantType(grantType) + .tokenType(OAuth2TokenType.ACCESS_TOKEN); } -} \ No newline at end of file +} From b12eaa14f0e2275b826730d107eb25f8828bb88e Mon Sep 17 00:00:00 2001 From: "Donald F. Coffin" Date: Fri, 13 Nov 2026 02:06:47 -0500 Subject: [PATCH 3/6] test(#129): replace mock-HttpSecurity config test with a real context test AuthorizationServerConfigTest called the @Bean filter-chain methods against a mocked HttpSecurity and stubbed JdbcTemplate (ClassCast, UnfinishedStubbing, null @Value settings, removed /connect/register and JWK expectations) - it tested the framework, not our configuration. AuthorizationServerApplicationTests booted on the H2 'local' profile, which no longer starts. AuthorizationServerApplicationTests now extends AbstractAuthserverIntegrationTest (real config, MySQL Testcontainer) and asserts what the started context holds: the four security filter chains, no JwtDecoder/JwtEncoder (opaque only, #134), issuer and protocol endpoints, the JDBC client repository, grant-context authorization service and ESPI customizer beans, and the seeded default clients' grants, scopes, token format, TTLs, consent and PKCE settings. Co-Authored-By: Claude Opus 5.5 --- .../AuthorizationServerApplicationTests.java | 181 ++++++- .../config/AuthorizationServerConfigTest.java | 463 ------------------ 2 files changed, 154 insertions(+), 490 deletions(-) delete mode 100644 openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/config/AuthorizationServerConfigTest.java diff --git a/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/AuthorizationServerApplicationTests.java b/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/AuthorizationServerApplicationTests.java index fd5c3f0e..cc3db2ba 100644 --- a/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/AuthorizationServerApplicationTests.java +++ b/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/AuthorizationServerApplicationTests.java @@ -20,36 +20,163 @@ package org.greenbuttonalliance.espi.authserver; +import org.greenbuttonalliance.espi.authserver.grant.GrantContextEnrichingAuthorizationService; +import org.greenbuttonalliance.espi.authserver.integration.AbstractAuthserverIntegrationTest; +import org.greenbuttonalliance.espi.authserver.service.EspiTokenCustomizer; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; -import org.springframework.boot.test.context.SpringBootTest; -import org.springframework.test.context.ActiveProfiles; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.ApplicationContext; +import org.springframework.security.oauth2.core.AuthorizationGrantType; +import org.springframework.security.oauth2.jwt.JwtDecoder; +import org.springframework.security.oauth2.jwt.JwtEncoder; +import org.springframework.security.oauth2.server.authorization.OAuth2AuthorizationService; +import org.springframework.security.oauth2.server.authorization.client.JdbcRegisteredClientRepository; +import org.springframework.security.oauth2.server.authorization.client.RegisteredClient; +import org.springframework.security.oauth2.server.authorization.client.RegisteredClientRepository; +import org.springframework.security.oauth2.server.authorization.settings.AuthorizationServerSettings; +import org.springframework.security.oauth2.server.authorization.settings.OAuth2TokenFormat; +import org.springframework.security.oauth2.server.authorization.token.JwtEncodingContext; +import org.springframework.security.oauth2.server.authorization.token.OAuth2TokenCustomizer; +import org.springframework.security.web.SecurityFilterChain; + +import java.time.Duration; +import java.util.Set; + +import static org.assertj.core.api.Assertions.assertThat; /** - * Integration tests for OpenESPI Authorization Server Application - * - * Tests basic Spring Boot application startup and configuration - * with Spring Authorization Server 1.3+. - * - * @author Green Button Alliance - * @version 1.0.0 - * @since Spring Boot 3.5 + * Application context test for the OpenESPI Authorization Server, booted with the real configuration + * against the shared MySQL Testcontainer (see {@link AbstractAuthserverIntegrationTest}). + * + *

Replaces the former H2 {@code local}-profile {@code contextLoads} and the mock-{@code HttpSecurity} + * {@code AuthorizationServerConfigTest} (#129): instead of calling {@code @Bean} methods against mocks, + * it asserts what the started context actually contains.

*/ -@SpringBootTest -@ActiveProfiles("local") -class AuthorizationServerApplicationTests { - - /** - * Test that the Spring Boot application context loads successfully - * with all OAuth2 Authorization Server configurations. - */ +@DisplayName("Authorization Server application context") +class AuthorizationServerApplicationTests extends AbstractAuthserverIntegrationTest { + + @Autowired + private ApplicationContext context; + + @Autowired + private AuthorizationServerSettings authorizationServerSettings; + + @Autowired + private RegisteredClientRepository registeredClientRepository; + + @Autowired + private OAuth2AuthorizationService authorizationService; + + @Autowired + private OAuth2TokenCustomizer espiTokenCustomizer; + + @Test + @DisplayName("registers exactly the four application security filter chains") + void registersTheApplicationSecurityFilterChains() { + assertThat(context.getBeansOfType(SecurityFilterChain.class)) + .containsOnlyKeys( + "clientRegistrationSecurityFilterChain", + "authorizationServerSecurityFilterChain", + "adminApiBearerSecurityFilterChain", + "defaultSecurityFilterChain"); + } + + @Test + @DisplayName("is opaque-token only: no JwtDecoder or JwtEncoder bean (#134)") + void hasNoJwtInfrastructure() { + assertThat(context.getBeanNamesForType(JwtDecoder.class)).isEmpty(); + assertThat(context.getBeanNamesForType(JwtEncoder.class)).isEmpty(); + } + + @Test + @DisplayName("exposes the configured issuer and OAuth2 protocol endpoints") + void authorizationServerSettings() { + assertThat(authorizationServerSettings) + .extracting( + AuthorizationServerSettings::getIssuer, + AuthorizationServerSettings::getAuthorizationEndpoint, + AuthorizationServerSettings::getTokenEndpoint, + AuthorizationServerSettings::getTokenRevocationEndpoint, + AuthorizationServerSettings::getTokenIntrospectionEndpoint) + .containsExactly( + "http://localhost:9999", + "/oauth2/authorize", + "/oauth2/token", + "/oauth2/revoke", + "/oauth2/introspect"); + } + + @Test + @DisplayName("wires the JDBC client repository, the grant-context authorization service and the ESPI customizer") + void persistenceAndCustomizerBeans() { + assertThat(registeredClientRepository).isInstanceOf(JdbcRegisteredClientRepository.class); + assertThat(authorizationService).isInstanceOf(GrantContextEnrichingAuthorizationService.class); + assertThat(espiTokenCustomizer).isInstanceOf(EspiTokenCustomizer.class); + } + + @Test + @DisplayName("seeds data_custodian_admin: client_credentials, opaque, 60 min, no consent") + void seedsDataCustodianAdminClient() { + assertThat(registeredClientRepository.findByClientId("data_custodian_admin")) + .isNotNull() + .satisfies(client -> assertThat(client) + .extracting( + RegisteredClient::getAuthorizationGrantTypes, + RegisteredClient::getScopes, + c -> c.getTokenSettings().getAccessTokenFormat(), + c -> c.getTokenSettings().getAccessTokenTimeToLive(), + c -> c.getClientSettings().isRequireAuthorizationConsent()) + .containsExactly( + Set.of(AuthorizationGrantType.CLIENT_CREDENTIALS), + Set.of("DataCustodian_Admin_Access"), + OAuth2TokenFormat.REFERENCE, + Duration.ofMinutes(60), + false)); + } + + @Test + @DisplayName("seeds third_party: authorization_code + refresh_token, ESPI scopes, opaque, 6 h, consent, no PKCE") + void seedsThirdPartyClient() { + assertThat(registeredClientRepository.findByClientId("third_party")) + .isNotNull() + .satisfies(client -> assertThat(client) + .extracting( + RegisteredClient::getAuthorizationGrantTypes, + c -> c.getTokenSettings().getAccessTokenFormat(), + c -> c.getTokenSettings().getAccessTokenTimeToLive(), + c -> c.getClientSettings().isRequireAuthorizationConsent(), + c -> c.getClientSettings().isRequireProofKey()) + .containsExactly( + Set.of(AuthorizationGrantType.AUTHORIZATION_CODE, + AuthorizationGrantType.REFRESH_TOKEN), + OAuth2TokenFormat.REFERENCE, + Duration.ofHours(6), + true, + false)) + .extracting(RegisteredClient::getScopes) + .satisfies(scopes -> assertThat(scopes).contains( + "openid", "profile", + "FB=4_5_15;IntervalDuration=3600;BlockDuration=monthly;HistoryLength=13", + "FB=4_5_15;IntervalDuration=900;BlockDuration=monthly;HistoryLength=13")); + } + @Test - void contextLoads() { - // This test verifies that: - // - Spring Boot application starts successfully - // - All OAuth2 Authorization Server beans are configured - // - Database schema is created (H2 in-memory for tests) - // - JWT key pair is generated - // - Registered clients are loaded - // - Security configurations are applied + @DisplayName("seeds third_party_admin and the datacustodian introspection client with opaque tokens") + void seedsRemainingDefaultClients() { + assertThat(registeredClientRepository.findByClientId("third_party_admin")) + .isNotNull() + .extracting( + RegisteredClient::getAuthorizationGrantTypes, + RegisteredClient::getScopes, + c -> c.getTokenSettings().getAccessTokenFormat()) + .containsExactly( + Set.of(AuthorizationGrantType.CLIENT_CREDENTIALS), + Set.of("ThirdParty_Admin_Access"), + OAuth2TokenFormat.REFERENCE); + assertThat(registeredClientRepository.findByClientId("datacustodian")) + .isNotNull() + .extracting(RegisteredClient::getAuthorizationGrantTypes) + .isEqualTo(Set.of(AuthorizationGrantType.CLIENT_CREDENTIALS)); } -} \ No newline at end of file +} diff --git a/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/config/AuthorizationServerConfigTest.java b/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/config/AuthorizationServerConfigTest.java deleted file mode 100644 index 06cc2bed..00000000 --- a/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/config/AuthorizationServerConfigTest.java +++ /dev/null @@ -1,463 +0,0 @@ -/* - * - * Copyright (c) 2018-2026 Green Button Alliance, Inc. - * - * Portions (c) 2013-2018 EnergyOS.org - * - * 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 org.greenbuttonalliance.espi.authserver.config; - -import com.nimbusds.jose.jwk.JWK; -import com.nimbusds.jose.jwk.JWKSet; -import com.nimbusds.jose.jwk.RSAKey; -import com.nimbusds.jose.jwk.source.JWKSource; -import com.nimbusds.jose.proc.SecurityContext; -import org.greenbuttonalliance.espi.authserver.service.EspiTokenCustomizer; -import org.springframework.security.oauth2.server.authorization.client.JdbcRegisteredClientRepository; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.DisplayName; -import org.junit.jupiter.api.Nested; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.Mock; -import org.mockito.junit.jupiter.MockitoExtension; -import org.springframework.jdbc.core.JdbcTemplate; -import org.springframework.security.config.annotation.web.builders.HttpSecurity; -import org.springframework.security.crypto.password.PasswordEncoder; -import org.springframework.security.oauth2.jwt.JwtDecoder; -import org.springframework.security.oauth2.server.authorization.client.RegisteredClient; -import org.springframework.security.oauth2.server.authorization.client.RegisteredClientRepository; -import org.springframework.security.oauth2.server.authorization.settings.AuthorizationServerSettings; -import org.springframework.security.oauth2.server.authorization.token.JwtEncodingContext; -import org.springframework.security.oauth2.server.authorization.token.OAuth2TokenCustomizer; -import org.springframework.security.web.SecurityFilterChain; -import org.springframework.security.web.DefaultSecurityFilterChain; - -import java.util.List; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.ArgumentMatchers.*; -import static org.mockito.Mockito.*; - -/** - * Comprehensive unit tests for AuthorizationServerConfig - * - * Tests OAuth2 Authorization Server configuration including security filter chains, - * client repository setup, JWK configuration, and ESPI-specific customizations. - * - * @author Green Button Alliance - * @version 1.0.0 - * @since Spring Boot 3.5 - */ -@ExtendWith(MockitoExtension.class) -@DisplayName("AuthorizationServerConfig Tests") -class AuthorizationServerConfigTest { - - @Mock - private JdbcTemplate jdbcTemplate; - - @Mock - private HttpSecurity httpSecurity; - - @Mock - PasswordEncoder passwordEncoder; - - private AuthorizationServerConfig config; - - @BeforeEach - void setUp() { - config = new AuthorizationServerConfig(); - } - - @Nested - @DisplayName("Security Filter Chain Tests") - class SecurityFilterChainTests { - - @Test - @DisplayName("Should create authorization server security filter chain") - void shouldCreateAuthorizationServerSecurityFilterChain() throws Exception { - // Given - HttpSecurity mockHttpSecurity = mock(HttpSecurity.class, RETURNS_DEEP_STUBS); - SecurityFilterChain mockFilterChain = mock(SecurityFilterChain.class); - - when(mockHttpSecurity.build()).thenReturn((DefaultSecurityFilterChain) mockFilterChain); - - // When - SecurityFilterChain filterChain = config.authorizationServerSecurityFilterChain(mockHttpSecurity); - - // Then - assertThat(filterChain).isNotNull(); - verify(mockHttpSecurity).build(); - } - - @Test - @DisplayName("Should create default security filter chain") - void shouldCreateDefaultSecurityFilterChain() throws Exception { - // Given - HttpSecurity mockHttpSecurity = mock(HttpSecurity.class, RETURNS_DEEP_STUBS); - SecurityFilterChain mockFilterChain = mock(SecurityFilterChain.class); - - when(mockHttpSecurity.build()).thenReturn((DefaultSecurityFilterChain) mockFilterChain); - - // When - SecurityFilterChain filterChain = config.defaultSecurityFilterChain(mockHttpSecurity, null, null); - - // Then - assertThat(filterChain).isNotNull(); - verify(mockHttpSecurity).build(); - } - } - - @Nested - @DisplayName("Client Repository Tests") - class ClientRepositoryTests { - - @Test - @DisplayName("Should create registered client repository with default ESPI clients") - void shouldCreateRegisteredClientRepositoryWithDefaultEspiClients() { - // Given - when(jdbcTemplate.queryForObject(anyString(), any(Class.class), anyString())).thenReturn(null); - - // When - RegisteredClientRepository repository = config.registeredClientRepository(jdbcTemplate, passwordEncoder); - - // Then - assertThat(repository).isNotNull(); - assertThat(repository).isInstanceOf(JdbcRegisteredClientRepository.class); - - // Verify default clients are attempted to be saved - verify(jdbcTemplate, atLeast(3)).queryForObject(anyString(), any(Class.class), anyString()); - } - - @Test - @DisplayName("Should not overwrite existing clients") - void shouldNotOverwriteExistingClients() { - // Given - RegisteredClient existingClient = mock(RegisteredClient.class); - when(jdbcTemplate.queryForObject(anyString(), any(Class.class), anyString())).thenReturn(existingClient); - - // When - RegisteredClientRepository repository = config.registeredClientRepository(jdbcTemplate, passwordEncoder); - - // Then - assertThat(repository).isNotNull(); - - // Verify existing clients are found but not overwritten - verify(jdbcTemplate, atLeast(3)).queryForObject(anyString(), any(Class.class), anyString()); - verify(jdbcTemplate, never()).update(anyString(), any(Object[].class)); - } - - @Test - @DisplayName("Should configure DataCustodian admin client correctly") - void shouldConfigureDataCustodianAdminClientCorrectly() { - // Given - JdbcRegisteredClientRepository mockRepo = mock(JdbcRegisteredClientRepository.class); - when(jdbcTemplate.queryForObject(anyString(), any(Class.class), anyString())).thenReturn(null); - - // When - RegisteredClientRepository repository = config.registeredClientRepository(jdbcTemplate, passwordEncoder); - - // Then - Verify DataCustodian admin client configuration through method calls - verify(jdbcTemplate, atLeastOnce()).queryForObject(anyString(), any(Class.class), eq("data_custodian_admin")); - } - - @Test - @DisplayName("Should configure ThirdParty client correctly") - void shouldConfigureThirdPartyClientCorrectly() { - // Given - when(jdbcTemplate.queryForObject(anyString(), any(Class.class), anyString())).thenReturn(null); - - // When - RegisteredClientRepository repository = config.registeredClientRepository(jdbcTemplate, passwordEncoder); - - // Then - Verify ThirdParty client configuration through method calls - verify(jdbcTemplate, atLeastOnce()).queryForObject(anyString(), any(Class.class), eq("third_party")); - } - - @Test - @DisplayName("Should configure ThirdParty admin client correctly") - void shouldConfigureThirdPartyAdminClientCorrectly() { - // Given - when(jdbcTemplate.queryForObject(anyString(), any(Class.class), anyString())).thenReturn(null); - - // When - RegisteredClientRepository repository = config.registeredClientRepository(jdbcTemplate, passwordEncoder); - - // Then - Verify ThirdParty admin client configuration through method calls - verify(jdbcTemplate, atLeastOnce()).queryForObject(anyString(), any(Class.class), eq("third_party_admin")); - } - } - - // JWK Configuration Tests removed in #134 — the auth-server no longer exposes - // jwkSource()/jwtDecoder() (ESPI is opaque-only; no JWK/JWT). - - @Nested - @DisplayName("Authorization Server Settings Tests") - class AuthorizationServerSettingsTests { - - @Test - @DisplayName("Should create authorization server settings with ESPI endpoints") - void shouldCreateAuthorizationServerSettingsWithEspiEndpoints() { - // When - AuthorizationServerSettings settings = config.authorizationServerSettings(); - - // Then - assertThat(settings).isNotNull(); - assertThat(settings.getIssuer()).isEqualTo("http://localhost:9999"); - assertThat(settings.getAuthorizationEndpoint()).isEqualTo("/oauth2/authorize"); - assertThat(settings.getTokenEndpoint()).isEqualTo("/oauth2/token"); - assertThat(settings.getJwkSetEndpoint()).isEqualTo("/oauth2/jwks"); - assertThat(settings.getTokenRevocationEndpoint()).isEqualTo("/oauth2/revoke"); - assertThat(settings.getTokenIntrospectionEndpoint()).isEqualTo("/oauth2/introspect"); - assertThat(settings.getOidcClientRegistrationEndpoint()).isEqualTo("/connect/register"); - assertThat(settings.getOidcUserInfoEndpoint()).isEqualTo("/userinfo"); - } - - @Test - @DisplayName("Should use correct issuer URL") - void shouldUseCorrectIssuerUrl() { - // When - AuthorizationServerSettings settings = config.authorizationServerSettings(); - - // Then - assertThat(settings.getIssuer()).isEqualTo("http://localhost:9999"); - } - } - - @Nested - @DisplayName("Token Customizer Tests") - class TokenCustomizerTests { - - @Test - @DisplayName("Should create ESPI token customizer") - void shouldCreateEspiTokenCustomizer() { - // When - OAuth2TokenCustomizer customizer = config.espiTokenCustomizer(); - - // Then - assertThat(customizer).isNotNull(); - assertThat(customizer).isInstanceOf(EspiTokenCustomizer.class); - } - - @Test - @DisplayName("Should return new instance on each call") - void shouldReturnNewInstanceOnEachCall() { - // When - OAuth2TokenCustomizer customizer1 = config.espiTokenCustomizer(); - OAuth2TokenCustomizer customizer2 = config.espiTokenCustomizer(); - - // Then - assertThat(customizer1).isNotNull(); - assertThat(customizer2).isNotNull(); - assertThat(customizer1).isNotSameAs(customizer2); - } - } - - @Nested - @DisplayName("ESPI Compliance Tests") - class EspiComplianceTests { - - @Test - @DisplayName("Should configure clients with opaque token format (ESPI standard)") - void shouldConfigureClientsWithOpaqueTokenFormat() { - // Given - when(jdbcTemplate.queryForObject(anyString(), any(Class.class), anyString())).thenReturn(null); - - // When - RegisteredClientRepository repository = config.registeredClientRepository(jdbcTemplate, passwordEncoder); - - // Then - // The verification is indirect through repository calls since we can't directly - // access the created clients, but the configuration should use OAuth2TokenFormat.REFERENCE - verify(jdbcTemplate, atLeast(3)).queryForObject(anyString(), any(Class.class), anyString()); - } - - @Test - @DisplayName("Should support ESPI scopes for ThirdParty client") - void shouldSupportEspiScopesForThirdPartyClient() { - // Given - when(jdbcTemplate.queryForObject(anyString(), any(Class.class), anyString())).thenReturn(null); - - // When - RegisteredClientRepository repository = config.registeredClientRepository(jdbcTemplate, passwordEncoder); - - // Then - // Verify that ThirdParty client is queried (which means it was configured) - verify(jdbcTemplate, atLeastOnce()).queryForObject(anyString(), any(Class.class), eq("third_party")); - } - - @Test - @DisplayName("Should configure proper grant types for ESPI clients") - void shouldConfigureProperGrantTypesForEspiClients() { - // Given - when(jdbcTemplate.queryForObject(anyString(), any(Class.class), anyString())).thenReturn(null); - - // When - RegisteredClientRepository repository = config.registeredClientRepository(jdbcTemplate, passwordEncoder); - - // Then - // Verify all default ESPI clients are configured - verify(jdbcTemplate, atLeastOnce()).queryForObject(anyString(), any(Class.class), eq("data_custodian_admin")); - verify(jdbcTemplate, atLeastOnce()).queryForObject(anyString(), any(Class.class), eq("third_party")); - verify(jdbcTemplate, atLeastOnce()).queryForObject(anyString(), any(Class.class), eq("third_party_admin")); - } - - @Test - @DisplayName("Should configure appropriate token lifetimes for ESPI") - void shouldConfigureAppropriateTokenLifetimesForEspi() { - // Given - when(jdbcTemplate.queryForObject(anyString(), any(Class.class), anyString())).thenReturn(null); - - // When - RegisteredClientRepository repository = config.registeredClientRepository(jdbcTemplate, passwordEncoder); - - // Then - // The token lifetimes are configured internally in the registered clients - // We verify through the repository initialization - assertThat(repository).isNotNull(); - } - - @Test - @DisplayName("Should configure consent requirements correctly") - void shouldConfigureConsentRequirementsCorrectly() { - // Given - when(jdbcTemplate.queryForObject(anyString(), any(Class.class), anyString())).thenReturn(null); - - // When - RegisteredClientRepository repository = config.registeredClientRepository(jdbcTemplate, passwordEncoder); - - // Then - // Admin clients should not require consent, customer clients should - // This is configured in the client settings within the method - assertThat(repository).isNotNull(); - } - } - - @Nested - @DisplayName("Error Handling Tests") - class ErrorHandlingTests { - - @Test - @DisplayName("Should handle JdbcTemplate exceptions gracefully") - void shouldHandleJdbcTemplateExceptionsGracefully() { - // Given - when(jdbcTemplate.queryForObject(anyString(), any(Class.class), anyString())) - .thenThrow(new RuntimeException("Database error")); - - // When & Then - Should not throw exception - RegisteredClientRepository repository = config.registeredClientRepository(jdbcTemplate, passwordEncoder); - - assertThat(repository).isNotNull(); - } - - @Test - @DisplayName("Should handle null JdbcTemplate gracefully") - void shouldHandleNullJdbcTemplateGracefully() { - // When & Then - Should throw appropriate exception - try { - config.registeredClientRepository(null, passwordEncoder); - } catch (Exception e) { - assertThat(e).isInstanceOf(NullPointerException.class); - } - } - } - - @Nested - @DisplayName("Bean Configuration Tests") - class BeanConfigurationTests { - - @Test - @DisplayName("Should create all required beans") - void shouldCreateAllRequiredBeans() { - // Given - when(jdbcTemplate.queryForObject(anyString(), any(Class.class), anyString())).thenReturn(null); - - // When - RegisteredClientRepository clientRepository = config.registeredClientRepository(jdbcTemplate, passwordEncoder); - AuthorizationServerSettings serverSettings = config.authorizationServerSettings(); - OAuth2TokenCustomizer tokenCustomizer = config.espiTokenCustomizer(); - - // Then - assertThat(clientRepository).isNotNull(); - assertThat(serverSettings).isNotNull(); - assertThat(tokenCustomizer).isNotNull(); - } - - @Test - @DisplayName("Should create beans with correct types") - void shouldCreateBeansWithCorrectTypes() { - // Given - when(jdbcTemplate.queryForObject(anyString(), any(Class.class), anyString())).thenReturn(null); - - // When & Then - assertThat(config.registeredClientRepository(jdbcTemplate, passwordEncoder)) - .isInstanceOf(JdbcRegisteredClientRepository.class); - - assertThat(config.authorizationServerSettings()) - .isInstanceOf(AuthorizationServerSettings.class); - - assertThat(config.espiTokenCustomizer()) - .isInstanceOf(EspiTokenCustomizer.class); - } - } - - @Nested - @DisplayName("Integration Tests") - class IntegrationTests { - - @Test - @DisplayName("Should work with complete configuration") - void shouldWorkWithCompleteConfiguration() { - // Given - when(jdbcTemplate.queryForObject(anyString(), any(Class.class), anyString())).thenReturn(null); - - // When - RegisteredClientRepository clientRepository = config.registeredClientRepository(jdbcTemplate, passwordEncoder); - AuthorizationServerSettings serverSettings = config.authorizationServerSettings(); - OAuth2TokenCustomizer tokenCustomizer = config.espiTokenCustomizer(); - - // Then - All components should work together - assertThat(clientRepository).isNotNull(); - assertThat(serverSettings).isNotNull(); - assertThat(tokenCustomizer).isNotNull(); - - // Test server settings configuration - assertThat(serverSettings.getIssuer()).isNotNull(); - assertThat(serverSettings.getTokenEndpoint()).isNotNull(); - } - - @Test - @DisplayName("Should maintain ESPI compliance across all components") - void shouldMaintainEspiComplianceAcrossAllComponents() { - // Given - when(jdbcTemplate.queryForObject(anyString(), any(Class.class), anyString())).thenReturn(null); - - // When - RegisteredClientRepository clientRepository = config.registeredClientRepository(jdbcTemplate, passwordEncoder); - AuthorizationServerSettings serverSettings = config.authorizationServerSettings(); - OAuth2TokenCustomizer tokenCustomizer = config.espiTokenCustomizer(); - - // Then - assertThat(clientRepository).isInstanceOf(JdbcRegisteredClientRepository.class); - assertThat(tokenCustomizer).isInstanceOf(EspiTokenCustomizer.class); - - // ESPI endpoints should be properly configured - assertThat(serverSettings.getOidcClientRegistrationEndpoint()).isEqualTo("/connect/register"); - assertThat(serverSettings.getTokenRevocationEndpoint()).isEqualTo("/oauth2/revoke"); - assertThat(serverSettings.getTokenIntrospectionEndpoint()).isEqualTo("/oauth2/introspect"); - } - } -} \ No newline at end of file From 308d344ff59037dbcb333099b80636e220d645ac Mon Sep 17 00:00:00 2001 From: "Donald F. Coffin" Date: Fri, 13 Nov 2026 02:06:56 -0500 Subject: [PATCH 4/6] test(#129): run SecurityIntegrationTest against the real config on MySQL Move SecurityIntegrationTest off the H2 'test' profile (no schema: the H2 profile had no Flyway location) onto AbstractAuthserverIntegrationTest, and correct its stale expectations: - use the seeded clients' real secrets (the "{bcrypt}secret" literal never matched) - malformed Basic header -> 400 invalid_request; client_secret_post on a client_secret_basic client -> 401 invalid_client - protocol endpoints are CSRF-exempt by design; CSRF is asserted on the session-chain admin DELETE with an MFA-complete admin instead - discovery document instead of the removed /oauth2/jwks endpoint - authorize requests use queryParam (Spring AS reads the query string) and a per-test client, and assert delegation/redirect-URI/scope outcomes exactly - introspection by the DataCustodian client of another client's token Removed: the six admin access-control cases (covered by AdminEndpointAuthorizationIntegrationTest / AdminBearerTokenIntegrationTest), the duplicate opaque-token case, the no-op CORS case, and the unused application-test.yml. Co-Authored-By: Claude Opus 5.5 --- .../integration/SecurityIntegrationTest.java | 736 +++++++----------- .../src/test/resources/application-test.yml | 66 -- 2 files changed, 299 insertions(+), 503 deletions(-) delete mode 100644 openespi-authserver/src/test/resources/application-test.yml diff --git a/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/integration/SecurityIntegrationTest.java b/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/integration/SecurityIntegrationTest.java index 3fe1e48f..165cc80a 100644 --- a/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/integration/SecurityIntegrationTest.java +++ b/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/integration/SecurityIntegrationTest.java @@ -25,148 +25,190 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.boot.test.context.SpringBootTest; -import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc; -import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureWebMvc; import org.springframework.http.MediaType; -import org.springframework.security.test.context.support.WithMockUser; -import org.springframework.test.context.ActiveProfiles; +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.security.oauth2.core.AuthorizationGrantType; +import org.springframework.security.oauth2.core.ClientAuthenticationMethod; +import org.springframework.security.oauth2.server.authorization.client.RegisteredClient; +import org.springframework.security.oauth2.server.authorization.client.RegisteredClientRepository; +import org.springframework.security.oauth2.server.authorization.settings.ClientSettings; +import org.springframework.security.oauth2.server.authorization.settings.OAuth2TokenFormat; +import org.springframework.security.oauth2.server.authorization.settings.TokenSettings; import org.springframework.test.web.servlet.MockMvc; -import org.springframework.test.web.servlet.MvcResult; -import org.springframework.transaction.annotation.Transactional; +import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; import tools.jackson.databind.JsonNode; import tools.jackson.databind.ObjectMapper; +import java.time.Duration; +import java.time.Instant; import java.util.Base64; -import java.util.HashMap; -import java.util.List; -import java.util.Map; +import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.greenbuttonalliance.espi.authserver.integration.AdminMfaTestSupport.adminWithBothFactors; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.httpBasic; -import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*; -import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; /** - * Security Integration Tests - * - * Comprehensive security testing for OAuth2 Authorization Server including - * authentication, authorization, access control, and ESPI-specific security requirements. - * - * @author Green Button Alliance - * @version 1.0.0 - * @since Spring Boot 3.5 + * Security integration tests for the OAuth2 protocol endpoints: client authentication, grant/scope/ + * redirect-URI validation, CSRF exemptions, opaque-token properties and response headers. + * + *

Runs against the real security configuration and the shared MySQL Testcontainer (#129). Admin-API + * access control is covered by {@link AdminEndpointAuthorizationIntegrationTest} and + * {@link AdminBearerTokenIntegrationTest}; RFC 7591 registration by + * {@link OAuth2DynamicClientRegistrationIntegrationTest}.

*/ -@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) -@AutoConfigureWebMvc -@AutoConfigureMockMvc -@ActiveProfiles("test") @DisplayName("Security Integration Tests") -@Transactional -class SecurityIntegrationTest { +class SecurityIntegrationTest extends AbstractAuthserverIntegrationTest { + + /** Seeded client_credentials client (AuthorizationServerConfig#registeredClientRepository). */ + private static final String ADMIN_CLIENT_ID = "data_custodian_admin"; + private static final String ADMIN_CLIENT_SECRET = "dc-secret"; + private static final String ADMIN_SCOPE = "DataCustodian_Admin_Access"; + + /** Seeded client_credentials client with a different scope. */ + private static final String TP_ADMIN_CLIENT_ID = "third_party_admin"; + private static final String TP_ADMIN_CLIENT_SECRET = "tpadmin-secret"; + + /** Seeded DC resource-server introspection client. */ + private static final String INTROSPECTION_CLIENT_ID = "datacustodian"; + private static final String INTROSPECTION_CLIENT_SECRET = "datacustodian-secret"; + + /** Seeded authorization_code client (no client_credentials grant). */ + private static final String TP_CLIENT_ID = "third_party"; + private static final String TP_CLIENT_SECRET = "tp-secret"; + + private static final String ESPI_SCOPE_HOURLY = "FB=4_5_15;IntervalDuration=3600;BlockDuration=monthly;HistoryLength=13"; + private static final String ESPI_SCOPE_15_MIN = "FB=4_5_15;IntervalDuration=900;BlockDuration=monthly;HistoryLength=13"; + private static final String REDIRECT_URI = "https://tp.example/espi/callback"; @Autowired private MockMvc mockMvc; @Autowired - private ObjectMapper objectMapper; + private RegisteredClientRepository registeredClientRepository; - private static final String CLIENT_ID = "third_party"; - private static final String CLIENT_SECRET = "{bcrypt}secret"; - private static final String ADMIN_CLIENT_ID = "data_custodian_admin"; - private static final String ADMIN_CLIENT_SECRET = "{bcrypt}secret"; + @Autowired + private JdbcTemplate jdbcTemplate; + + private final ObjectMapper objectMapper = new ObjectMapper(); + + /** Per-test ESPI authorization_code client with an https redirect URI (Spring AS rejects "localhost"). */ + private String authCodeClientId; + + @BeforeEach + void seedAuthorizationCodeClient() { + String suffix = UUID.randomUUID().toString().substring(0, 8); + authCodeClientId = "sec-it-tp-" + suffix; + registeredClientRepository.save(RegisteredClient.withId(UUID.randomUUID().toString()) + .clientId(authCodeClientId) + .clientName("Security IT ThirdParty") + .clientSecret("{noop}sec-it-secret-" + suffix) // JdbcRegisteredClientRepository requires unique secrets + .clientIdIssuedAt(Instant.now()) + .clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC) + .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE) + .redirectUri(REDIRECT_URI) + .scope(ESPI_SCOPE_HOURLY) + .scope(ESPI_SCOPE_15_MIN) + .clientSettings(ClientSettings.builder() + .requireAuthorizationConsent(true) + .requireProofKey(false) + .build()) + .tokenSettings(TokenSettings.builder() + .accessTokenFormat(OAuth2TokenFormat.REFERENCE) + .accessTokenTimeToLive(Duration.ofHours(6)) + .build()) + .build()); + } @Nested @DisplayName("Authentication Tests") class AuthenticationTests { + /* + * Accept: application/json is sent explicitly. The protocol chain's login entry point matches + * text/html, and a request with no Accept header (or a wildcard Accept) is treated as compatible + * with text/html, so such a request is redirected to /authorize/delegate instead of getting a 401. + */ @Test - @DisplayName("Should require authentication for protected endpoints") + @DisplayName("Should require client authentication on token, introspection and revocation endpoints") void shouldRequireAuthenticationForProtectedEndpoints() throws Exception { - // OAuth2 endpoints - mockMvc.perform(post("/oauth2/token")) - .andExpect(status().isUnauthorized()); - - mockMvc.perform(post("/oauth2/introspect")) - .andExpect(status().isUnauthorized()); - - mockMvc.perform(post("/oauth2/revoke")) + mockMvc.perform(post("/oauth2/token") + .accept(MediaType.APPLICATION_JSON) + .param("grant_type", "client_credentials")) .andExpect(status().isUnauthorized()); - // Admin endpoints - mockMvc.perform(get("/admin/oauth2/tokens")) + mockMvc.perform(post("/oauth2/introspect") + .accept(MediaType.APPLICATION_JSON) + .param("token", "anything")) .andExpect(status().isUnauthorized()); - mockMvc.perform(get("/admin/oauth2/clients")) + mockMvc.perform(post("/oauth2/revoke") + .accept(MediaType.APPLICATION_JSON) + .param("token", "anything")) .andExpect(status().isUnauthorized()); } @Test @DisplayName("Should accept valid client credentials") void shouldAcceptValidClientCredentials() throws Exception { - mockMvc.perform(post("/oauth2/token") - .param("grant_type", "client_credentials") - .param("scope", "DataCustodian_Admin_Access") - .with(httpBasic(ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET)) - .with(csrf())) - .andExpect(status().isOk()); + mockMvc.perform(clientCredentials(ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET, ADMIN_SCOPE)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.token_type").value("Bearer")) + .andExpect(jsonPath("$.scope").value(ADMIN_SCOPE)); } @Test @DisplayName("Should reject invalid client credentials") void shouldRejectInvalidClientCredentials() throws Exception { - mockMvc.perform(post("/oauth2/token") - .param("grant_type", "client_credentials") - .param("scope", "DataCustodian_Admin_Access") - .with(httpBasic(ADMIN_CLIENT_ID, "wrong-password")) - .with(csrf())) - .andExpect(status().isUnauthorized()); + mockMvc.perform(clientCredentials(ADMIN_CLIENT_ID, "wrong-password", ADMIN_SCOPE)) + .andExpect(status().isUnauthorized()) + .andExpect(jsonPath("$.error").value("invalid_client")); } @Test @DisplayName("Should reject non-existent client") void shouldRejectNonExistentClient() throws Exception { - mockMvc.perform(post("/oauth2/token") - .param("grant_type", "client_credentials") - .param("scope", "DataCustodian_Admin_Access") - .with(httpBasic("non-existent-client", "password")) - .with(csrf())) - .andExpect(status().isUnauthorized()); + mockMvc.perform(clientCredentials("non-existent-client", "password", ADMIN_SCOPE)) + .andExpect(status().isUnauthorized()) + .andExpect(jsonPath("$.error").value("invalid_client")); } @Test - @DisplayName("Should handle malformed basic auth header") + @DisplayName("Should reject a malformed Basic authorization header as invalid_request") void shouldHandleMalformedBasicAuthHeader() throws Exception { String malformedAuth = Base64.getEncoder().encodeToString("malformed".getBytes()); - + mockMvc.perform(post("/oauth2/token") - .header("Authorization", "Basic " + malformedAuth) - .param("grant_type", "client_credentials") - .param("scope", "DataCustodian_Admin_Access") - .with(csrf())) - .andExpect(status().isUnauthorized()); + .header("Authorization", "Basic " + malformedAuth) + .param("grant_type", "client_credentials") + .param("scope", ADMIN_SCOPE)) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error").value("invalid_request")); } @Test - @DisplayName("Should validate client authentication method") + @DisplayName("Should only accept the client's registered authentication method (client_secret_basic)") void shouldValidateClientAuthenticationMethod() throws Exception { - // Test client_secret_basic (valid) - mockMvc.perform(post("/oauth2/token") - .param("grant_type", "client_credentials") - .param("scope", "DataCustodian_Admin_Access") - .with(httpBasic(ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET)) - .with(csrf())) + mockMvc.perform(clientCredentials(ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET, ADMIN_SCOPE)) .andExpect(status().isOk()); - // Test client_secret_post (should work if supported) + // data_custodian_admin is registered for client_secret_basic only mockMvc.perform(post("/oauth2/token") - .param("grant_type", "client_credentials") - .param("client_id", ADMIN_CLIENT_ID) - .param("client_secret", ADMIN_CLIENT_SECRET) - .param("scope", "DataCustodian_Admin_Access") - .with(csrf())) - .andExpect(status().isOk()); + .param("grant_type", "client_credentials") + .param("client_id", ADMIN_CLIENT_ID) + .param("client_secret", ADMIN_CLIENT_SECRET) + .param("scope", ADMIN_SCOPE)) + .andExpect(status().isUnauthorized()) + .andExpect(jsonPath("$.error").value("invalid_client")); } } @@ -177,153 +219,64 @@ class AuthorizationTests { @Test @DisplayName("Should enforce scope-based authorization") void shouldEnforceScopeBasedAuthorization() throws Exception { - // Admin client should be able to access admin scopes - mockMvc.perform(post("/oauth2/token") - .param("grant_type", "client_credentials") - .param("scope", "DataCustodian_Admin_Access") - .with(httpBasic(ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET)) - .with(csrf())) + mockMvc.perform(clientCredentials(ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET, ADMIN_SCOPE)) .andExpect(status().isOk()); - // Regular client should not be able to access admin scopes - mockMvc.perform(post("/oauth2/token") - .param("grant_type", "client_credentials") - .param("scope", "DataCustodian_Admin_Access") - .with(httpBasic(CLIENT_ID, CLIENT_SECRET)) - .with(csrf())) - .andExpect(status().isBadRequest()); + // A client_credentials client cannot obtain a scope it is not registered for + mockMvc.perform(clientCredentials(TP_ADMIN_CLIENT_ID, TP_ADMIN_CLIENT_SECRET, ADMIN_SCOPE)) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error").value("invalid_scope")); } @Test - @DisplayName("Should validate grant type authorization") + @DisplayName("Should reject grant types the client is not registered for") void shouldValidateGrantTypeAuthorization() throws Exception { - // Admin client authorized for client_credentials + // data_custodian_admin is client_credentials only; Spring AS resolves the code first, and no + // code can exist for a client without the authorization_code grant, so this is invalid_grant mockMvc.perform(post("/oauth2/token") - .param("grant_type", "client_credentials") - .param("scope", "DataCustodian_Admin_Access") - .with(httpBasic(ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET)) - .with(csrf())) - .andExpect(status().isOk()); + .with(httpBasic(ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET)) + .param("grant_type", "authorization_code") + .param("code", "invalid-code") + .param("redirect_uri", REDIRECT_URI)) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error").value("invalid_grant")); - // Admin client not authorized for authorization_code without code - mockMvc.perform(post("/oauth2/token") - .param("grant_type", "authorization_code") - .param("code", "invalid-code") - .param("redirect_uri", "http://localhost:8080/callback") - .with(httpBasic(ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET)) - .with(csrf())) - .andExpect(status().isBadRequest()); + // third_party is authorization_code/refresh_token only + mockMvc.perform(clientCredentials(TP_CLIENT_ID, TP_CLIENT_SECRET, ESPI_SCOPE_HOURLY)) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error").value("unauthorized_client")); } @Test @DisplayName("Should enforce redirect URI validation") void shouldEnforceRedirectUriValidation() throws Exception { - // Valid redirect URI - mockMvc.perform(get("/oauth2/authorize") - .param("response_type", "code") - .param("client_id", CLIENT_ID) - .param("redirect_uri", "http://localhost:8080/DataCustodian/oauth/callback") - .param("scope", "openid") - .param("state", "test-state")) - .andExpect(status().is3xxRedirection()); - - // Invalid redirect URI - mockMvc.perform(get("/oauth2/authorize") - .param("response_type", "code") - .param("client_id", CLIENT_ID) - .param("redirect_uri", "https://malicious.com/callback") - .param("scope", "openid") - .param("state", "test-state")) - .andExpect(status().isBadRequest()); + // Registered redirect URI: the unauthenticated user is sent to the delegated login/consent + mockMvc.perform(authorize(authCodeClientId, REDIRECT_URI, ESPI_SCOPE_HOURLY)) + .andExpect(status().isFound()) + .andExpect(result -> assertThat(result.getResponse().getRedirectedUrl()) + .contains("/authorize/delegate")); + + // Unregistered redirect URI: 400, and never a redirect to the attacker's URI + mockMvc.perform(authorize(authCodeClientId, "https://malicious.example/callback", ESPI_SCOPE_HOURLY)) + .andExpect(status().isBadRequest()) + .andExpect(header().doesNotExist("Location")); } @Test @DisplayName("Should validate requested scopes") void shouldValidateRequestedScopes() throws Exception { - // Valid ESPI scope - mockMvc.perform(get("/oauth2/authorize") - .param("response_type", "code") - .param("client_id", CLIENT_ID) - .param("redirect_uri", "http://localhost:8080/DataCustodian/oauth/callback") - .param("scope", "openid FB=4_5_15;IntervalDuration=3600;BlockDuration=monthly;HistoryLength=13") - .param("state", "test-state")) - .andExpect(status().is3xxRedirection()); - - // Invalid scope for client - mockMvc.perform(post("/oauth2/token") - .param("grant_type", "client_credentials") - .param("scope", "invalid_scope") - .with(httpBasic(ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET)) - .with(csrf())) - .andExpect(status().isBadRequest()); - } - } - - @Nested - @DisplayName("Admin Access Control Tests") - class AdminAccessControlTests { - - @Test - @WithMockUser(roles = "ADMIN") - @DisplayName("Should allow admin access to admin endpoints") - void shouldAllowAdminAccessToAdminEndpoints() throws Exception { - mockMvc.perform(get("/admin/oauth2/tokens")) - .andExpect(status().isOk()); - - mockMvc.perform(get("/admin/oauth2/clients")) - .andExpect(status().isOk()); - - mockMvc.perform(get("/admin/oauth2/authorizations")) - .andExpect(status().isOk()); - } - - @Test - @WithMockUser(roles = "DC_ADMIN") - @DisplayName("Should allow DC_ADMIN access to admin endpoints") - void shouldAllowDcAdminAccessToAdminEndpoints() throws Exception { - mockMvc.perform(get("/admin/oauth2/tokens")) - .andExpect(status().isOk()); - - mockMvc.perform(get("/admin/oauth2/clients")) - .andExpect(status().isOk()); - } - - @Test - @WithMockUser(roles = "USER") - @DisplayName("Should deny user access to admin endpoints") - void shouldDenyUserAccessToAdminEndpoints() throws Exception { - mockMvc.perform(get("/admin/oauth2/tokens")) - .andExpect(status().isForbidden()); - - mockMvc.perform(get("/admin/oauth2/clients")) - .andExpect(status().isForbidden()); - } - - @Test - @WithMockUser(roles = "CUSTOMER") - @DisplayName("Should deny customer access to admin endpoints") - void shouldDenyCustomerAccessToAdminEndpoints() throws Exception { - mockMvc.perform(delete("/admin/oauth2/tokens/test-token")) - .andExpect(status().isForbidden()); - - mockMvc.perform(delete("/admin/oauth2/clients/test-client")) - .andExpect(status().isForbidden()); - } - - @Test - @WithMockUser(authorities = "SCOPE_DataCustodian_Admin_Access") - @DisplayName("Should allow DataCustodian admin scope access") - void shouldAllowDataCustodianAdminScopeAccess() throws Exception { - mockMvc.perform(get("/admin/oauth2/clients")) - .andExpect(status().isOk()); - } - - @Test - @WithMockUser(authorities = "SCOPE_openid") - @DisplayName("Should deny regular scope access to admin endpoints") - void shouldDenyRegularScopeAccessToAdminEndpoints() throws Exception { - mockMvc.perform(get("/admin/oauth2/clients")) - .andExpect(status().isForbidden()); + // Scope the client is not registered for: invalid_scope is returned to the client's redirect URI + mockMvc.perform(authorize(authCodeClientId, REDIRECT_URI, + "FB=4_5_16;IntervalDuration=900;BlockDuration=monthly;HistoryLength=13")) + .andExpect(status().isFound()) + .andExpect(result -> assertThat(result.getResponse().getRedirectedUrl()) + .startsWith(REDIRECT_URI) + .contains("error=invalid_scope")); + + // Unknown scope at the token endpoint + mockMvc.perform(clientCredentials(ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET, "invalid_scope")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error").value("invalid_scope")); } } @@ -332,46 +285,35 @@ void shouldDenyRegularScopeAccessToAdminEndpoints() throws Exception { class CsrfProtectionTests { @Test - @DisplayName("Should require CSRF token for state-changing operations") - void shouldRequireCsrfTokenForStateChangingOperations() throws Exception { - // POST without CSRF token should fail - mockMvc.perform(post("/oauth2/token") - .param("grant_type", "client_credentials") - .param("scope", "DataCustodian_Admin_Access") - .with(httpBasic(ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET))) - .andExpect(status().isForbidden()); - - // POST with CSRF token should succeed - mockMvc.perform(post("/oauth2/token") - .param("grant_type", "client_credentials") - .param("scope", "DataCustodian_Admin_Access") - .with(httpBasic(ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET)) - .with(csrf())) + @DisplayName("OAuth2 protocol endpoints are CSRF-exempt (client-authenticated, not session-based)") + void protocolEndpointsAreCsrfExempt() throws Exception { + // No csrf() post-processor: the token endpoint must still issue a token + mockMvc.perform(clientCredentials(ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET, ADMIN_SCOPE)) .andExpect(status().isOk()); } @Test - @DisplayName("Should not require CSRF for GET operations") + @DisplayName("Should not require authentication or CSRF for the discovery document") void shouldNotRequireCsrfForGetOperations() throws Exception { - // GET operations should work without CSRF - mockMvc.perform(get("/oauth2/jwks")) - .andExpect(status().isOk()); - mockMvc.perform(get("/.well-known/oauth-authorization-server")) - .andExpect(status().isOk()); + .andExpect(status().isOk()) + .andExpect(jsonPath("$.issuer").value("http://localhost:9999")) + .andExpect(jsonPath("$.token_endpoint").value("http://localhost:9999/oauth2/token")) + .andExpect(jsonPath("$.introspection_endpoint").value("http://localhost:9999/oauth2/introspect")) + .andExpect(jsonPath("$.registration_endpoint").value("http://localhost:9999/oauth2/register")); } @Test - @WithMockUser(roles = "ADMIN") - @DisplayName("Should require CSRF for admin operations") + @DisplayName("Should require CSRF for state-changing admin operations on the session chain") void shouldRequireCsrfForAdminOperations() throws Exception { - // DELETE without CSRF should fail - mockMvc.perform(delete("/admin/oauth2/tokens/test-token")) + mockMvc.perform(delete("/admin/oauth2/tokens/test-token") + .with(adminWithBothFactors("sec-it-admin", "ROLE_ADMIN"))) .andExpect(status().isForbidden()); - // DELETE with CSRF should succeed (may return 404 for non-existent token) + // With a CSRF token the request reaches the controller (unknown token -> 404) mockMvc.perform(delete("/admin/oauth2/tokens/test-token") - .with(csrf())) + .with(adminWithBothFactors("sec-it-admin", "ROLE_ADMIN")) + .with(csrf())) .andExpect(status().isNotFound()); } } @@ -381,112 +323,56 @@ void shouldRequireCsrfForAdminOperations() throws Exception { class TokenSecurityTests { @Test - @DisplayName("Should generate secure opaque tokens") + @DisplayName("Should generate secure opaque (non-JWT) access tokens (ESPI)") void shouldGenerateSecureOpaqueTokens() throws Exception { - // Get an access token - MvcResult result = mockMvc.perform(post("/oauth2/token") - .param("grant_type", "client_credentials") - .param("scope", "DataCustodian_Admin_Access") - .with(httpBasic(ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET)) - .with(csrf())) - .andExpect(status().isOk()) - .andReturn(); + String accessToken = obtainAccessToken(ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET, ADMIN_SCOPE); - String responseBody = result.getResponse().getContentAsString(); - JsonNode response = objectMapper.readTree(responseBody); - String accessToken = response.get("access_token").asText(); - - // Verify token properties (ESPI standard: opaque tokens) - assertThat(accessToken).isNotNull(); - assertThat(accessToken.length()).isGreaterThan(20); // Should be reasonably long - assertThat(accessToken).doesNotContain("."); // Should not be JWT format - assertThat(accessToken).matches("[A-Za-z0-9_-]+"); // Should be URL-safe + assertThat(accessToken) + .hasSizeGreaterThan(20) + .doesNotContain(".") + .doesNotStartWith("eyJ") + .matches("[A-Za-z0-9_-]+"); } @Test - @DisplayName("Should protect against token replay attacks") + @DisplayName("A revoked access token introspects as inactive") void shouldProtectAgainstTokenReplayAttacks() throws Exception { - // Get an access token - MvcResult tokenResult = mockMvc.perform(post("/oauth2/token") - .param("grant_type", "client_credentials") - .param("scope", "DataCustodian_Admin_Access") - .with(httpBasic(ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET)) - .with(csrf())) - .andExpect(status().isOk()) - .andReturn(); + String accessToken = obtainAccessToken(ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET, ADMIN_SCOPE); - String tokenResponseBody = tokenResult.getResponse().getContentAsString(); - JsonNode tokenResponse = objectMapper.readTree(tokenResponseBody); - String accessToken = tokenResponse.get("access_token").asText(); - - // Use token for introspection (should work) - mockMvc.perform(post("/oauth2/introspect") - .param("token", accessToken) - .with(httpBasic(ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET)) - .with(csrf())) + mockMvc.perform(introspect(accessToken, ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET)) .andExpect(status().isOk()) .andExpect(jsonPath("$.active").value(true)); - // Revoke the token mockMvc.perform(post("/oauth2/revoke") - .param("token", accessToken) - .with(httpBasic(ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET)) - .with(csrf())) + .with(httpBasic(ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET)) + .param("token", accessToken)) .andExpect(status().isOk()); - // Try to use revoked token (should fail) - mockMvc.perform(post("/oauth2/introspect") - .param("token", accessToken) - .with(httpBasic(ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET)) - .with(csrf())) + mockMvc.perform(introspect(accessToken, ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET)) .andExpect(status().isOk()) .andExpect(jsonPath("$.active").value(false)); } @Test - @DisplayName("Should enforce token expiration") + @DisplayName("Should issue admin tokens that expire within one hour") void shouldEnforceTokenExpiration() throws Exception { - // Get an access token - MvcResult result = mockMvc.perform(post("/oauth2/token") - .param("grant_type", "client_credentials") - .param("scope", "DataCustodian_Admin_Access") - .with(httpBasic(ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET)) - .with(csrf())) - .andExpect(status().isOk()) - .andReturn(); - - String responseBody = result.getResponse().getContentAsString(); - JsonNode response = objectMapper.readTree(responseBody); - - // Verify token has expiration - assertThat(response.has("expires_in")).isTrue(); - int expiresIn = response.get("expires_in").asInt(); - assertThat(expiresIn).isGreaterThan(0); - assertThat(expiresIn).isLessThanOrEqualTo(3600); // Should not exceed 1 hour for admin tokens + JsonNode response = tokenResponse(ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET, ADMIN_SCOPE); + + assertThat(response.path("expires_in").asInt()) + .isPositive() + .isLessThanOrEqualTo(3600); } @Test - @DisplayName("Should validate token ownership") + @DisplayName("The DataCustodian introspection client can introspect a token issued to another client") void shouldValidateTokenOwnership() throws Exception { - // Get token with one client - MvcResult tokenResult = mockMvc.perform(post("/oauth2/token") - .param("grant_type", "client_credentials") - .param("scope", "DataCustodian_Admin_Access") - .with(httpBasic(ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET)) - .with(csrf())) - .andExpect(status().isOk()) - .andReturn(); - - String tokenResponseBody = tokenResult.getResponse().getContentAsString(); - JsonNode tokenResponse = objectMapper.readTree(tokenResponseBody); - String accessToken = tokenResponse.get("access_token").asText(); + String accessToken = obtainAccessToken(ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET, ADMIN_SCOPE); - // Try to introspect with different client (should still work for admin) - mockMvc.perform(post("/oauth2/introspect") - .param("token", accessToken) - .with(httpBasic(ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET)) - .with(csrf())) - .andExpect(status().isOk()); + mockMvc.perform(introspect(accessToken, INTROSPECTION_CLIENT_ID, INTROSPECTION_CLIENT_SECRET)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.active").value(true)) + .andExpect(jsonPath("$.client_id").value(ADMIN_CLIENT_ID)) + .andExpect(jsonPath("$.scope").value(ADMIN_SCOPE)); } } @@ -495,85 +381,33 @@ void shouldValidateTokenOwnership() throws Exception { class EspiSecurityComplianceTests { @Test - @DisplayName("Should enforce ESPI-required opaque tokens") - void shouldEnforceEspiRequiredOpaqueTokens() throws Exception { - // Get token for ESPI client - MvcResult result = mockMvc.perform(post("/oauth2/token") - .param("grant_type", "client_credentials") - .param("scope", "DataCustodian_Admin_Access") - .with(httpBasic(ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET)) - .with(csrf())) - .andExpect(status().isOk()) - .andReturn(); - - String responseBody = result.getResponse().getContentAsString(); - JsonNode response = objectMapper.readTree(responseBody); - String accessToken = response.get("access_token").asText(); - - // Verify token is opaque (not JWT) - assertThat(accessToken.split("\\.").length).isLessThan(3); - assertThat(accessToken).doesNotContain("eyJ"); // Should not start like JWT - } - - @Test - @DisplayName("Should require authorization consent for customer data") + @DisplayName("Should route an unauthenticated customer to the delegated login/consent") void shouldRequireAuthorizationConsentForCustomerData() throws Exception { - // Start authorization flow that should require consent - mockMvc.perform(get("/oauth2/authorize") - .param("response_type", "code") - .param("client_id", CLIENT_ID) - .param("redirect_uri", "http://localhost:8080/DataCustodian/oauth/callback") - .param("scope", "openid profile FB=4_5_15;IntervalDuration=3600;BlockDuration=monthly;HistoryLength=13") - .param("state", "test-state")) - .andExpect(status().is3xxRedirection()); - // Should redirect to login/consent page + mockMvc.perform(authorize(authCodeClientId, REDIRECT_URI, ESPI_SCOPE_HOURLY)) + .andExpect(status().isFound()) + .andExpect(result -> assertThat(result.getResponse().getRedirectedUrl()) + .contains("/authorize/delegate") + .doesNotContain("code=")); } @Test - @DisplayName("Should validate ESPI scope format") + @DisplayName("Should accept multiple registered ESPI Function Block scopes") void shouldValidateEspiScopeFormat() throws Exception { - // Valid ESPI scope format - mockMvc.perform(get("/oauth2/authorize") - .param("response_type", "code") - .param("client_id", CLIENT_ID) - .param("redirect_uri", "http://localhost:8080/DataCustodian/oauth/callback") - .param("scope", "FB=4_5_15;IntervalDuration=3600;BlockDuration=monthly;HistoryLength=13") - .param("state", "test-state")) - .andExpect(status().is3xxRedirection()); - - // Should handle multiple ESPI scopes - mockMvc.perform(get("/oauth2/authorize") - .param("response_type", "code") - .param("client_id", CLIENT_ID) - .param("redirect_uri", "http://localhost:8080/DataCustodian/oauth/callback") - .param("scope", "openid FB=4_5_15;IntervalDuration=3600 FB=4_5_16;IntervalDuration=900") - .param("state", "test-state")) - .andExpect(status().is3xxRedirection()); + mockMvc.perform(authorize(authCodeClientId, REDIRECT_URI, ESPI_SCOPE_HOURLY + " " + ESPI_SCOPE_15_MIN)) + .andExpect(status().isFound()) + .andExpect(result -> assertThat(result.getResponse().getRedirectedUrl()) + .contains("/authorize/delegate") + .doesNotContain("error=")); } @Test - @DisplayName("Should enforce proper token lifetimes for ESPI") - void shouldEnforceProperTokenLifetimesForEspi() throws Exception { - // Get customer access token - MvcResult result = mockMvc.perform(post("/oauth2/token") - .param("grant_type", "authorization_code") - .param("code", "test-code") - .param("redirect_uri", "http://localhost:8080/DataCustodian/oauth/callback") - .with(httpBasic(CLIENT_ID, CLIENT_SECRET)) - .with(csrf())) - .andExpect(status().isOk()) - .andReturn(); - - if (result.getResponse().getStatus() == 200) { - String responseBody = result.getResponse().getContentAsString(); - JsonNode response = objectMapper.readTree(responseBody); - - if (response.has("expires_in")) { - int expiresIn = response.get("expires_in").asInt(); - // ESPI recommends 6-hour access tokens for customer data - assertThat(expiresIn).isLessThanOrEqualTo(21600); // 6 hours - } - } + @DisplayName("The seeded third_party client issues opaque 6-hour access tokens (ESPI)") + void shouldEnforceProperTokenLifetimesForEspi() { + assertThat(registeredClientRepository.findByClientId(TP_CLIENT_ID)) + .isNotNull() + .extracting(RegisteredClient::getTokenSettings) + .extracting(TokenSettings::getAccessTokenFormat, TokenSettings::getAccessTokenTimeToLive) + .containsExactly(OAuth2TokenFormat.REFERENCE, Duration.ofHours(6)); } } @@ -582,33 +416,19 @@ void shouldEnforceProperTokenLifetimesForEspi() throws Exception { class SecurityHeadersTests { @Test - @DisplayName("Should include security headers in responses") + @DisplayName("Should include security headers in protocol responses") void shouldIncludeSecurityHeadersInResponses() throws Exception { mockMvc.perform(get("/.well-known/oauth-authorization-server")) .andExpect(status().isOk()) - .andExpect(header().exists("X-Content-Type-Options")) - .andExpect(header().exists("X-Frame-Options")) - .andExpect(header().exists("X-XSS-Protection")); + .andExpect(header().string("X-Content-Type-Options", "nosniff")) + .andExpect(header().string("X-Frame-Options", "DENY")) + .andExpect(header().string("Cache-Control", containsString("no-store"))); } @Test - @DisplayName("Should set appropriate CORS headers") - void shouldSetAppropriateCorsHeaders() throws Exception { - mockMvc.perform(get("/oauth2/jwks") - .header("Origin", "https://trusted-client.example.com")) - .andExpect(status().isOk()); - // CORS headers should be configured appropriately - } - - @Test - @DisplayName("Should prevent clickjacking attacks") + @DisplayName("Should prevent clickjacking of the authorization endpoint") void shouldPreventClickjackingAttacks() throws Exception { - mockMvc.perform(get("/oauth2/authorize") - .param("response_type", "code") - .param("client_id", CLIENT_ID) - .param("redirect_uri", "http://localhost:8080/DataCustodian/oauth/callback") - .param("scope", "openid") - .param("state", "test-state")) + mockMvc.perform(authorize(authCodeClientId, REDIRECT_URI, ESPI_SCOPE_HOURLY)) .andExpect(header().string("X-Frame-Options", "DENY")); } } @@ -618,42 +438,84 @@ void shouldPreventClickjackingAttacks() throws Exception { class InputValidationTests { @Test - @DisplayName("Should sanitize and validate all inputs") + @DisplayName("Should reject script-injection attempts without redirecting to the injected URI") void shouldSanitizeAndValidateAllInputs() throws Exception { - // Test with potentially malicious inputs mockMvc.perform(get("/oauth2/authorize") - .param("response_type", "") - .param("client_id", CLIENT_ID) - .param("redirect_uri", "javascript:alert('xss')") - .param("scope", "openid") - .param("state", "")) - .andExpect(status().isBadRequest()); + .accept(MediaType.TEXT_HTML) + .queryParam("response_type", "") + .queryParam("client_id", authCodeClientId) + .queryParam("redirect_uri", "javascript:alert('xss')") + .queryParam("scope", ESPI_SCOPE_HOURLY) + .queryParam("state", "")) + .andExpect(status().isBadRequest()) + .andExpect(header().doesNotExist("Location")); } @Test - @DisplayName("Should handle SQL injection attempts") + @DisplayName("Should treat SQL in the scope parameter as an invalid scope") void shouldHandleSqlInjectionAttempts() throws Exception { - mockMvc.perform(post("/oauth2/token") - .param("grant_type", "client_credentials") - .param("scope", "'; DROP TABLE oauth2_registered_client; --") - .with(httpBasic(ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET)) - .with(csrf())) - .andExpect(status().isBadRequest()); + mockMvc.perform(clientCredentials(ADMIN_CLIENT_ID, ADMIN_CLIENT_SECRET, + "'; DROP TABLE oauth2_registered_client; --")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error").value("invalid_scope")); + + assertThat(jdbcTemplate.queryForObject( + "SELECT COUNT(*) FROM oauth2_registered_client WHERE client_id = ?", + Integer.class, ADMIN_CLIENT_ID)) + .isOne(); } @Test - @DisplayName("Should validate parameter lengths") + @DisplayName("Should reject an oversized scope parameter as invalid_scope") void shouldValidateParameterLengths() throws Exception { - String veryLongString = "a".repeat(10000); - - mockMvc.perform(get("/oauth2/authorize") - .param("response_type", "code") - .param("client_id", CLIENT_ID) - .param("redirect_uri", "http://localhost:8080/DataCustodian/oauth/callback") - .param("scope", veryLongString) - .param("state", "test-state")) - .andExpect(status().isBadRequest()); + mockMvc.perform(authorize(authCodeClientId, REDIRECT_URI, "a".repeat(10000))) + .andExpect(status().isFound()) + .andExpect(result -> assertThat(result.getResponse()) + .extracting(MockHttpServletResponse::getRedirectedUrl) + .asString() + .startsWith(REDIRECT_URI) + .contains("error=invalid_scope")); } } -} \ No newline at end of file + // Helpers + + private static MockHttpServletRequestBuilder clientCredentials(String clientId, String secret, String scope) { + return post("/oauth2/token") + .with(httpBasic(clientId, secret)) + .param("grant_type", "client_credentials") + .param("scope", scope); + } + + private static MockHttpServletRequestBuilder introspect(String token, String clientId, String secret) { + return post("/oauth2/introspect") + .with(httpBasic(clientId, secret)) + .param("token", token); + } + + /** + * Browser-style authorization request (Accept: text/html, like a user agent). Uses {@code queryParam}: + * Spring AS reads the authorization request from the query string, so {@code param(...)} on a GET + * (which sets no query string in MockMvc) is reported as a missing {@code response_type}. + */ + private static MockHttpServletRequestBuilder authorize(String clientId, String redirectUri, String scope) { + return get("/oauth2/authorize") + .accept(MediaType.TEXT_HTML) + .queryParam("response_type", "code") + .queryParam("client_id", clientId) + .queryParam("redirect_uri", redirectUri) + .queryParam("scope", scope) + .queryParam("state", "test-state"); + } + + private JsonNode tokenResponse(String clientId, String secret, String scope) throws Exception { + String body = mockMvc.perform(clientCredentials(clientId, secret, scope)) + .andExpect(status().isOk()) + .andReturn().getResponse().getContentAsString(); + return objectMapper.readTree(body); + } + + private String obtainAccessToken(String clientId, String secret, String scope) throws Exception { + return tokenResponse(clientId, secret, scope).get("access_token").asText(); + } +} diff --git a/openespi-authserver/src/test/resources/application-test.yml b/openespi-authserver/src/test/resources/application-test.yml deleted file mode 100644 index 8e1a3d97..00000000 --- a/openespi-authserver/src/test/resources/application-test.yml +++ /dev/null @@ -1,66 +0,0 @@ -# OpenESPI Authorization Server Test Configuration -# Test profile for unit and integration testing - -server: - port: 0 # Random port for tests - -spring: - application: - name: openespi-authorization-server-test - - # H2 In-Memory Database for Tests - datasource: - url: jdbc:h2:mem:testdb;MODE=MySQL;DB_CLOSE_DELAY=-1;DB_CLOSE_ON_EXIT=FALSE - driver-class-name: org.h2.Driver - username: sa - password: "" - hikari: - maximum-pool-size: 10 - minimum-idle: 2 - idle-timeout: 300000 - max-lifetime: 1200000 - - # JPA Configuration for Tests - jpa: - hibernate: - ddl-auto: validate - show-sql: true - - properties: - hibernate: - dialect: org.hibernate.dialect.H2Dialect - format_sql: true - use_sql_comments: true - show_sql: true - jdbc: - batch_size: 20 - open-in-view: false - - # enabled: false - - # Security Configuration for Tests - security: - oauth2: - authorizationserver: - issuer: http://localhost:9999 - -# ESPI Configuration for Tests -espi: - token: - format: opaque # Test with ESPI-compliant opaque tokens - datacustodian: - endpoint: http://localhost:8080 - -# Logging Configuration for Tests -logging: - level: - org.springframework.security.oauth2: DEBUG - org.greenbuttonalliance.espi: DEBUG - org.springframework.web: INFO - org.springframework.security: INFO - org.hibernate.SQL: WARN - pattern: - console: "%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n" - -# Management endpoints: inherited from application.yml, so the tests exercise the real -# exposure and access rules. From 8c69ee791d6eebe8cfe39b2b470ad5839150990a Mon Sep 17 00:00:00 2001 From: "Donald F. Coffin" Date: Fri, 13 Nov 2026 02:14:52 -0500 Subject: [PATCH 5/6] ci(#129): run the full openespi-authserver suite in CI and releases The authserver unit tests are repaired, so drop the -Dgroups=testcontainers-it restriction from the CI test matrix and from release-meta.sh, and update CLAUDE.md, README.md, RELEASING.md, CI_CD_SETUP.md and the testcontainers-it tag comments to say that `mvn verify -pl openespi-authserver` runs everything. Co-Authored-By: Claude Opus 5.5 --- .github/CI_CD_SETUP.md | 6 +++--- .github/scripts/release-meta.sh | 7 ++----- .github/workflows/ci.yml | 3 --- .github/workflows/release.yml | 2 +- CLAUDE.md | 9 ++++----- README.md | 4 ++-- RELEASING.md | 2 +- .../integration/AbstractAuthserverIntegrationTest.java | 4 ++-- .../AuthorizationPurgeVendorPortabilityTest.java | 4 ++-- .../integration/MySqlTestcontainersIntegrationTest.java | 2 +- 10 files changed, 18 insertions(+), 25 deletions(-) diff --git a/.github/CI_CD_SETUP.md b/.github/CI_CD_SETUP.md index 784298b4..ce98c137 100644 --- a/.github/CI_CD_SETUP.md +++ b/.github/CI_CD_SETUP.md @@ -28,8 +28,8 @@ default to `permissions: contents: read`; jobs add only what they need. An install-once job that shares `~/.m2/repository/org/greenbuttonalliance` as an artifact was rejected. It would add a serial job, plus artifact upload and download, that cost more than the 30 s compile it saves. - - `openespi-authserver` runs only `-Dgroups=testcontainers-it` until **#129** fixes the - unit tests on the H2 profiles. + - `openespi-authserver` runs its full suite (unit tests plus the Testcontainers integration + tests) since **#129**. - Testcontainers suites use the Docker daemon on `ubuntu-latest`. - On failure the job uploads `surefire-reports-`. It always uploads that module's `jacoco.xml` for the `quality` job. @@ -190,7 +190,7 @@ create and approve pull requests**. The release workflow needs this to open its ```bash mvn install -DskipTests -pl openespi-datacustodian -am && mvn verify -pl openespi-datacustodian -mvn verify -pl openespi-authserver -Dgroups=testcontainers-it -DfailIfNoTests=false # until #129 +mvn install -DskipTests -pl openespi-authserver -am && mvn verify -pl openespi-authserver mvn install -DskipTests && mvn org.owasp:dependency-check-maven:aggregate docker run --rm -v "$PWD:/repo" -w /repo rhysd/actionlint:latest -color # lint workflows ``` diff --git a/.github/scripts/release-meta.sh b/.github/scripts/release-meta.sh index 9182af3d..0fdf0cc3 100644 --- a/.github/scripts/release-meta.sh +++ b/.github/scripts/release-meta.sh @@ -48,12 +48,9 @@ fi next_version="${major}.${minor}.$((patch + 1))-SNAPSHOT" -# Extra Maven test arguments per module. +# Extra Maven test arguments per module. None today: every module runs its full suite +# (authserver's -Dgroups=testcontainers-it restriction was removed by #129). test_args="" -if [[ "$module" == authserver ]]; then - # TODO(#129): AS unit tests are broken on the H2 test profiles; only the testcontainers-it suite runs. - test_args="-Dgroups=testcontainers-it -DfailIfNoTests=false" -fi # Previous release tag of the SAME module (highest version below this one), for release notes. previous_tag="" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index adf5465f..d8584ad6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,9 +32,6 @@ jobs: - module: openespi-datacustodian - module: openespi-thirdparty - module: openespi-authserver - # TODO(#129): AS unit tests are broken on the H2 test profiles; only the Docker-backed - # @Tag("testcontainers-it") suite runs until #129 is fixed. Remove this line then. - test-args: -Dgroups=testcontainers-it -DfailIfNoTests=false steps: - uses: actions/checkout@v7 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index a52a3159..cd561112 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -74,7 +74,7 @@ jobs: run: bash .github/scripts/set-module-version.sh "$DIR" "$VERSION" "$VERSION_PROPERTY" # Equivalent to `mvn verify -pl -am`, split so that module-specific test arguments - # (authserver's -Dgroups, #129) do not leak into the upstream modules' test runs. + # (TEST_ARGS from release-meta.sh, currently none) do not leak into the upstream modules' test runs. - name: Verify upstream modules (tests on) if: env.UPSTREAM != '' run: mvn install -pl "$UPSTREAM" -am # -am only adds the parent pom diff --git a/CLAUDE.md b/CLAUDE.md index fec7c3a3..31d8063f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -33,11 +33,10 @@ cd openespi-common && mvn test # There are no failsafe *IT classes and no separate integration-test profile. mvn test -pl openespi-common -# What CI runs per module (matrix in .github/workflows/ci.yml): install upstream, then verify +# What CI runs per module (matrix in .github/workflows/ci.yml): install upstream, then verify. +# For openespi-authserver this is the full suite (unit + MySQL Testcontainers tests) since #129; +# add -Dgroups=testcontainers-it to run only the Docker-backed integration tests. mvn install -DskipTests -pl -am && mvn verify -pl - -# Authorization Server: only the TestContainers group runs cleanly today (#129) -mvn verify -pl openespi-authserver -Dgroups=testcontainers-it -DfailIfNoTests=false ``` ### Run Applications @@ -269,7 +268,7 @@ ESPI uses Atom XML feeds for data exchange. Key patterns: - All tests must pass before committing: `mvn test` - Run tests for affected module and its dependents: `mvn test -pl -am` - TestContainers-based tests (Docker required) run inside the normal `mvn test`; there are no failsafe `*IT` classes and no separate integration-test profile (removed in #193) -- Note: openespi-authserver unit tests are currently excluded from CI (#129); CI runs only its `@Tag("testcontainers-it")` tests +- openespi-authserver runs its full suite in CI (#129); its Docker-backed tests are tagged `@Tag("testcontainers-it")` (via `AbstractAuthserverIntegrationTest`) so they can be run alone with `-Dgroups=testcontainers-it` - **IMPORTANT**: Use AssertJ chained assertions - combine all assertions for a single object into one assertion chain - Good: `assertThat(retrieved).isPresent().hasValueSatisfying(device -> assertThat(device).extracting(...).containsExactly(...))` - Bad: Multiple separate `assertThat()` statements for the same object diff --git a/README.md b/README.md index b0c1cd8e..e79772cb 100644 --- a/README.md +++ b/README.md @@ -161,8 +161,8 @@ mvn verify -pl openespi-common -am mvn test -Dtest=ComplexRelationshipMySQLIntegrationTest -pl openespi-common mvn test -Dtest=ComplexRelationshipPostgreSQLIntegrationTest -pl openespi-common -# Authorization Server: only the Testcontainers suite runs until #129 (same as CI) -mvn verify -pl openespi-authserver -am -Dgroups=testcontainers-it -DfailIfNoTests=false +# Authorization Server: full suite, unit + Testcontainers (MySQL) integration tests (same as CI) +mvn verify -pl openespi-authserver -am ``` CI, release and versioning: see [`.github/CI_CD_SETUP.md`](.github/CI_CD_SETUP.md) and [`RELEASING.md`](RELEASING.md). diff --git a/RELEASING.md b/RELEASING.md index 9e293bc0..5186df45 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -49,7 +49,7 @@ The parent pom, `openespi-parent`, is not released on its own. - sets the release version with `versions:set -pl `, **in CI only**; nothing is committed; - runs the upstream modules' tests and then `mvn verify -pl `. This is equivalent - to `-am verify`. `authserver` uses `-Dgroups=testcontainers-it` until #129; + to `-am verify`. Every module, `authserver` included, runs its full test suite; - collects the jar. For applications this is the Spring Boot fat jar. 3. **github-release**: creates the GitHub Release `` with: - generated notes, starting from the previous tag of the same module; diff --git a/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/integration/AbstractAuthserverIntegrationTest.java b/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/integration/AbstractAuthserverIntegrationTest.java index 081ac72a..5005a566 100644 --- a/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/integration/AbstractAuthserverIntegrationTest.java +++ b/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/integration/AbstractAuthserverIntegrationTest.java @@ -46,8 +46,8 @@ * and Flyway wiring are inherited. The {@code testcontainers} Spring profile supplies the rest of the * AS test configuration (see {@code application-testcontainers.yml}).

*/ -// Tagged so CI can run ONLY the Docker-backed integration tests (-Dgroups=testcontainers-it), -// excluding the AS module's pre-existing broken H2-profile unit tests. Inherited by subclasses. +// Tagged (and inherited by subclasses) so the Docker-backed integration tests can be selected on their +// own with -Dgroups=testcontainers-it. CI runs the full module suite (#129), so the tag is optional there. @Tag("testcontainers-it") @SpringBootTest(classes = AuthorizationServerApplication.class) @AutoConfigureMockMvc diff --git a/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/integration/AuthorizationPurgeVendorPortabilityTest.java b/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/integration/AuthorizationPurgeVendorPortabilityTest.java index 7f3ab812..a7ddf8e6 100644 --- a/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/integration/AuthorizationPurgeVendorPortabilityTest.java +++ b/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/integration/AuthorizationPurgeVendorPortabilityTest.java @@ -61,8 +61,8 @@ * (#152). Replace with a full migrate once #152 lands. * * - *

Tagged {@code testcontainers-it} so CI runs it with the other Docker-backed integration tests - * (the PostgreSQL half needs Docker; the H2 half is in-memory).

+ *

Tagged {@code testcontainers-it} with the other Docker-backed integration tests (the PostgreSQL + * half needs Docker; the H2 half is in-memory).

*/ @Tag("testcontainers-it") @DisplayName("Authorization purge (#192) vendor portability") diff --git a/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/integration/MySqlTestcontainersIntegrationTest.java b/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/integration/MySqlTestcontainersIntegrationTest.java index 3949cd44..fc1a50af 100644 --- a/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/integration/MySqlTestcontainersIntegrationTest.java +++ b/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/integration/MySqlTestcontainersIntegrationTest.java @@ -61,7 +61,7 @@ * @version 1.0.0 * @since Spring Boot 3.5 */ -@Tag("testcontainers-it") // CI runs the Docker-backed AS integration tests via -Dgroups=testcontainers-it +@Tag("testcontainers-it") // Docker-backed; selectable alone via -Dgroups=testcontainers-it @SpringBootTest @Testcontainers @ActiveProfiles("testcontainers") From fe59c9376b916183d63ea083d80e7a998ce79990 Mon Sep 17 00:00:00 2001 From: "Donald F. Coffin" Date: Sat, 14 Nov 2026 14:12:14 -0500 Subject: [PATCH 6/6] test(#129): expect the actuator security filter chain added by #240 AuthorizationServerApplicationTests pins the exact set of SecurityFilterChain beans. It was written before #240 added actuatorSecurityFilterChain, so the first CI run of the rebased suite failed on that one assertion (1 of 274). Keep the pin (a new chain changes which requests the others see) and list the fifth chain. Co-Authored-By: Claude Opus 5.5 --- .../espi/authserver/AuthorizationServerApplicationTests.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/AuthorizationServerApplicationTests.java b/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/AuthorizationServerApplicationTests.java index cc3db2ba..05fbe9e5 100644 --- a/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/AuthorizationServerApplicationTests.java +++ b/openespi-authserver/src/test/java/org/greenbuttonalliance/espi/authserver/AuthorizationServerApplicationTests.java @@ -72,10 +72,12 @@ class AuthorizationServerApplicationTests extends AbstractAuthserverIntegrationT private OAuth2TokenCustomizer espiTokenCustomizer; @Test - @DisplayName("registers exactly the four application security filter chains") + @DisplayName("registers exactly the five application security filter chains") void registersTheApplicationSecurityFilterChains() { + // Pinned on purpose: a new chain changes which requests each existing chain sees. assertThat(context.getBeansOfType(SecurityFilterChain.class)) .containsOnlyKeys( + "actuatorSecurityFilterChain", // #240: health public, the rest authserver.admin "clientRegistrationSecurityFilterChain", "authorizationServerSecurityFilterChain", "adminApiBearerSecurityFilterChain",