diff --git a/doc/PR/2026-12-16-PR#212-cors-config-using-spring-props-and-adding-tests.md b/doc/PR/2026-12-16-PR#212-cors-config-using-spring-props-and-adding-tests.md new file mode 100644 index 00000000..7259474e --- /dev/null +++ b/doc/PR/2026-12-16-PR#212-cors-config-using-spring-props-and-adding-tests.md @@ -0,0 +1,30 @@ +# PR#212: CORS-config using spring-props and adding tests + +## The Problems + +- CORS handling was configured via `System.getenv("ALLOWED_ORIGINS")` in `HsadminNgApplication`, which made configuration and testing harder. +- Spring Security had CORS disabled, so CORS behavior was not aligned with the security filter chain. +`/api/pong` only supported `GET`, which limited testing and client integration scenarios for CORS-enabled protected endpoints. + +In total, with this PR we want the CORS configuration to work properly and to be configurable for: +- prod env +- dev env +- local env +- JUnit-based tests + +## The Solution + +- Introduced a `WebMvcConfigurer` bean that reads `hsadminng.cors.allowed-origins` and applies origin and method rules for `/api/**`. +- Kept `/api/ping` explicitly open for `GET` from any origin to preserve its public health-check style behavior. +- Added CORS integration tests for preflight and actual requests, including allowed and denied origins and unauthorized token scenarios. +- Added `POST /api/pong` to the OpenAPI definition and implemented `pongPost()` in `PingController` using the same response logic as `pong()`. +- Added REST and acceptance tests for `POST /api/pong` to verify translated responses and authenticated behavior. + +## Additional Changes + +- Moved CORS configuration into `BaseWebSecurityConfig`, thus it's closer to related configurations. +- Included cleanup changes from rebasing and cyclic reference fixes while keeping the final behavior covered by tests. + +## Attachments + +None. diff --git a/src/main/java/net/hostsharing/hsadminng/HsadminNgApplication.java b/src/main/java/net/hostsharing/hsadminng/HsadminNgApplication.java index 2816aeb7..a1180099 100644 --- a/src/main/java/net/hostsharing/hsadminng/HsadminNgApplication.java +++ b/src/main/java/net/hostsharing/hsadminng/HsadminNgApplication.java @@ -3,9 +3,6 @@ package net.hostsharing.hsadminng; import io.swagger.v3.oas.annotations.OpenAPIDefinition; import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.SpringBootApplication; -import org.springframework.context.annotation.Bean; -import org.springframework.web.servlet.config.annotation.CorsRegistry; -import org.springframework.web.servlet.config.annotation.WebMvcConfigurer; @SpringBootApplication @OpenAPIDefinition @@ -14,23 +11,4 @@ public class HsadminNgApplication { public static void main(String[] args) { SpringApplication.run(HsadminNgApplication.class, args); } - - @Bean - public WebMvcConfigurer corsConfigurer() { - return new WebMvcConfigurer() { - - @Override - public void addCorsMappings(CorsRegistry registry) { - // TODO: to enable testing, we should use Spring config - String allowedOrigins = System.getenv("ALLOWED_ORIGINS"); - if (allowedOrigins == null || allowedOrigins.length() <= 1) { - allowedOrigins = "/**"; - } - registry.addMapping("/api/**") - .allowedOrigins(allowedOrigins) - .allowedMethods("GET", "PUT", "POST", "PATCH", "DELETE"); - } - }; - } - } diff --git a/src/main/java/net/hostsharing/hsadminng/config/BaseWebSecurityConfig.java b/src/main/java/net/hostsharing/hsadminng/config/BaseWebSecurityConfig.java index fe9fb407..4471fd7b 100644 --- a/src/main/java/net/hostsharing/hsadminng/config/BaseWebSecurityConfig.java +++ b/src/main/java/net/hostsharing/hsadminng/config/BaseWebSecurityConfig.java @@ -3,10 +3,12 @@ package net.hostsharing.hsadminng.config; import io.swagger.v3.oas.annotations.enums.SecuritySchemeType; import io.swagger.v3.oas.annotations.security.SecurityScheme; import lombok.SneakyThrows; +import lombok.val; import org.springframework.beans.factory.annotation.Value; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Profile; +import org.springframework.lang.NonNull; import org.springframework.security.config.Customizer; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; @@ -14,6 +16,8 @@ import org.springframework.security.config.annotation.web.configurers.AbstractHt import org.springframework.security.oauth2.jwt.JwtDecoder; import org.springframework.security.oauth2.jwt.NimbusJwtDecoder; import org.springframework.security.web.SecurityFilterChain; +import org.springframework.web.servlet.config.annotation.CorsRegistry; +import org.springframework.web.servlet.config.annotation.WebMvcConfigurer; import jakarta.servlet.http.HttpServletResponse; @@ -48,7 +52,7 @@ public abstract class BaseWebSecurityConfig { .oauth2ResourceServer(oauth -> oauth.jwt(Customizer.withDefaults())) .csrf(AbstractHttpConfigurer::disable) - .cors(AbstractHttpConfigurer::disable) + .cors(Customizer.withDefaults()) .exceptionHandling(exception -> exception .authenticationEntryPoint((request, response, authException) -> // For unknown reason Spring security returns 403 FORBIDDEN for a BadCredentialsException. @@ -75,4 +79,24 @@ public abstract class BaseWebSecurityConfig { // For fake-jwt profile, use the same RSA key as JwtFakeBearer return NimbusJwtDecoder.withPublicKey(RSA_KEY.toRSAPublicKey()).build(); } + + @Bean + public WebMvcConfigurer corsConfigurer( + @Value("${hsadminng.cors.allowed-origins:*}") final String corsAllowedOrigins) { + + return new WebMvcConfigurer() { + + @Override + public void addCorsMappings(@NonNull final CorsRegistry registry) { + val allowedOrigins = (corsAllowedOrigins != null && !corsAllowedOrigins.isEmpty()) + ? corsAllowedOrigins.split(",") + : new String[]{"*"}; + registry.addMapping("/api/ping") + .allowedOrigins("*") + .allowedMethods("GET"); + registry.addMapping("/api/**").allowedOrigins(allowedOrigins) + .allowedMethods("GET", "PUT", "POST", "PATCH", "DELETE"); + } + }; + } } diff --git a/src/main/java/net/hostsharing/hsadminng/ping/PingController.java b/src/main/java/net/hostsharing/hsadminng/ping/PingController.java index 7925a398..735465d1 100644 --- a/src/main/java/net/hostsharing/hsadminng/ping/PingController.java +++ b/src/main/java/net/hostsharing/hsadminng/ping/PingController.java @@ -17,6 +17,7 @@ public class PingController implements TestApi { private MessageTranslator messageTranslator; @Timed("app.api.ping") + @Override public ResponseEntity ping() { // HOWTO translate text with placeholders - also see in resource files i18n/messages_*.properties. final var translatedMessage = messageTranslator.translate("test.pinged--in-your-language"); @@ -24,7 +25,18 @@ public class PingController implements TestApi { } @Timed("app.api.pong") + @Override public ResponseEntity pong() { + return createPongResponse(); + } + + @Timed("app.api.pong") + @Override + public ResponseEntity pongPost() { + return createPongResponse(); + } + + private ResponseEntity createPongResponse() { final var userName = SecurityContextHolder.getContext().getAuthentication().getName(); // HOWTO translate text with placeholders - also see in resource files i18n/messages_*.properties. final var translatedMessage = messageTranslator.translate("test.ponged-{0}--in-your-language", userName); diff --git a/src/main/resources/api-definition/api-definition.yaml b/src/main/resources/api-definition/api-definition.yaml index 17cb76d6..612e988b 100644 --- a/src/main/resources/api-definition/api-definition.yaml +++ b/src/main/resources/api-definition/api-definition.yaml @@ -33,3 +33,14 @@ paths: 'text/plain': schema: type: string + post: + tags: + - test + operationId: pongPost + responses: + "200": + description: OK + content: + 'text/plain': + schema: + type: string diff --git a/src/main/resources/application.yml b/src/main/resources/application.yml index b9a2700a..ca177d90 100644 --- a/src/main/resources/application.yml +++ b/src/main/resources/application.yml @@ -70,6 +70,8 @@ spring: hsadminng: postgres: leakproof: + cors: + allowed-origin: ${ALLOWED_ORIGINS} // for compatibility with env-based CORS-configuration metrics: distribution: diff --git a/src/test/java/net/hostsharing/hsadminng/config/WebSecurityConfigIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/config/WebSecurityConfigIntegrationTest.java index a5bc714f..6482040c 100644 --- a/src/test/java/net/hostsharing/hsadminng/config/WebSecurityConfigIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/config/WebSecurityConfigIntegrationTest.java @@ -12,6 +12,8 @@ import org.springframework.http.HttpEntity; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; +import org.springframework.http.MediaType; +import org.springframework.http.ResponseEntity; import org.springframework.test.context.ActiveProfiles; import org.springframework.test.context.TestPropertySource; @@ -25,6 +27,7 @@ import static org.assertj.core.api.Assertions.assertThat; @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) @TestPropertySource(properties = { "management.port=0", "server.port=0" }) @ActiveProfiles("fake-jwt") // IMPORTANT: In this test, want to test the prod config, do NOT use test profile! +@TestPropertySource(properties = "hsadminng.cors.allowed-origins=https://allowed.example") class WebSecurityConfigIntegrationTest { public static final String GIVEN_FAKE_SUBJECT = "fake-user-name"; @@ -39,37 +42,32 @@ class WebSecurityConfigIntegrationTest { @Test void accessToApiWithValidJwtShouldBePermitted() { - // when val result = restTemplate.exchange( serverUrl("/api/pong"), HttpMethod.GET, - httpHeaders(entry("Authorization", bearer(GIVEN_FAKE_SUBJECT))), + httpHeaders(entry(HttpHeaders.AUTHORIZATION, bearer(GIVEN_FAKE_SUBJECT))), String.class ); - // then assertThat(result.getStatusCode()).isEqualTo(HttpStatus.OK); assertThat(result.getBody()).startsWith("ponged " + GIVEN_FAKE_SUBJECT); } @Test void accessToOpenApiWithoutTokenShouldBePermitted() { - val result = this.restTemplate.getForEntity( - serverUrl("/api/ping"), String.class); + val result = this.restTemplate.getForEntity(serverUrl("/api/ping"), String.class); assertThat(result.getStatusCode()).isEqualTo(HttpStatus.OK); } @Test void accessToProtectedApiWithInvalidTokenShouldBeDenied() { - // when val result = restTemplate.exchange( serverUrl("/api/pong"), HttpMethod.GET, - httpHeaders(entry("Authorization", "Bearer INVALID-JWT")), + httpHeaders(entry(HttpHeaders.AUTHORIZATION, "Bearer INVALID-JWT")), String.class ); - // then assertThat(result.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED); } @@ -82,15 +80,13 @@ class WebSecurityConfigIntegrationTest { @Test void accessToSwaggerUiShouldBePermitted() { - val result = this.restTemplate.getForEntity( - serverUrl("/swagger-ui/index.html"), String.class); + val result = this.restTemplate.getForEntity(serverUrl("/swagger-ui/index.html"), String.class); assertThat(result.getStatusCode()).isEqualTo(HttpStatus.OK); } @Test void accessToApiDocsEndpointShouldBePermitted() { - val result = this.restTemplate.getForEntity( - serverUrl("/v3/api-docs/swagger-config"), String.class); + val result = this.restTemplate.getForEntity(serverUrl("/v3/api-docs/swagger-config"), String.class); assertThat(result.getStatusCode()).isEqualTo(HttpStatus.OK); assertThat(result.getBody()).contains("\"configUrl\":\"/v3/api-docs/swagger-config\""); } @@ -103,6 +99,156 @@ class WebSecurityConfigIntegrationTest { assertThat(result.getBody().get("status")).isEqualTo("UP"); } + @Test + void preflightToPingAllowsAnyOrigin() { + val response = corsPreflightRequest("/api/ping", "https://anywhere.example"); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK); + assertThat(response.getHeaders().getFirst(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN)).isEqualTo("*"); + assertThat(response.getHeaders().getFirst(HttpHeaders.ACCESS_CONTROL_ALLOW_METHODS)).contains("GET"); + } + + @Test + void actualPingRequestAllowsAnyOrigin() { + val response = corsGetRequest("/api/ping", "https://anywhere.example", null); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK); + assertThat(response.getHeaders().getFirst(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN)).isEqualTo("*"); + } + + @Test + void preflightToPongAllowsConfiguredOrigin() { + val response = corsPreflightRequest("/api/pong", "https://allowed.example", "GET"); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK); + assertThat(response.getHeaders().getFirst(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN)) + .isEqualTo("https://allowed.example"); + assertThat(response.getHeaders().getFirst(HttpHeaders.ACCESS_CONTROL_ALLOW_METHODS)).contains("GET"); + } + + @Test + void preflightToPongBlocksOtherOrigin() { + val response = corsPreflightRequest("/api/pong", "https://denied.example", "GET"); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.FORBIDDEN); + assertThat(response.getHeaders().getFirst(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN)).isNull(); + } + + @Test + void actualPongRequestWithInvalidTokenAndAllowedOriginReturnsUnauthorizedWithCorsHeader() { + val response = corsGetRequest("/api/pong", "https://allowed.example", "Bearer INVALID-JWT"); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED); + assertThat(response.getHeaders().getFirst(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN)) + .isEqualTo("https://allowed.example"); + } + + @Test + void actualPongRequestWithInvalidTokenAndDeniedOriginIsRejectedByCors() { + val response = corsGetRequest("/api/pong", "https://denied.example", "Bearer INVALID-JWT"); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.FORBIDDEN); + assertThat(response.getHeaders().getFirst(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN)).isNull(); + } + + @Test + void preflightPostToPongAllowsConfiguredOrigin() { + val response = corsPreflightRequest("/api/pong", "https://allowed.example", "POST"); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK); + assertThat(response.getHeaders().getFirst(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN)) + .isEqualTo("https://allowed.example"); + assertThat(response.getHeaders().getFirst(HttpHeaders.ACCESS_CONTROL_ALLOW_METHODS)).contains("POST"); + } + + @Test + void preflightPostToPongBlocksOtherOrigin() { + val response = corsPreflightRequest("/api/pong", "https://denied.example", "POST"); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.FORBIDDEN); + assertThat(response.getHeaders().getFirst(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN)).isNull(); + } + + @Test + void actualPongPostWithInvalidTokenAndAllowedOriginReturnsUnauthorizedWithCorsHeader() { + val response = corsPostRequest("/api/pong", "https://allowed.example", "Bearer INVALID-JWT"); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED); + assertThat(response.getHeaders().getFirst(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN)) + .isEqualTo("https://allowed.example"); + } + + @Test + void actualPongPostWithInvalidTokenAndDeniedOriginIsRejectedByCors() { + val response = corsPostRequest("/api/pong", "https://denied.example", "Bearer INVALID-JWT"); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.FORBIDDEN); + assertThat(response.getHeaders().getFirst(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN)).isNull(); + } + + private ResponseEntity corsPreflightRequest(final String path, final String origin) { + return corsPreflightRequest(path, origin, "GET"); + } + + private ResponseEntity corsPreflightRequest( + final String path, + final String origin, + final String accessControlRequestMethod) { + return restTemplate.exchange( + serverUrl(path), + HttpMethod.OPTIONS, + httpHeaders( + entry(HttpHeaders.ORIGIN, origin), + entry(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, accessControlRequestMethod) + ), + String.class + ); + } + + private ResponseEntity corsGetRequest( + final String path, + final String origin, + final String authorization) { + if (authorization != null) { + return restTemplate.exchange( + serverUrl(path), + HttpMethod.GET, + httpHeaders( + entry(HttpHeaders.ORIGIN, origin), + entry(HttpHeaders.AUTHORIZATION, authorization), + entry(HttpHeaders.ACCEPT, MediaType.TEXT_PLAIN_VALUE) + ), + String.class + ); + } + + return restTemplate.exchange( + serverUrl(path), + HttpMethod.GET, + httpHeaders( + entry(HttpHeaders.ORIGIN, origin), + entry(HttpHeaders.ACCEPT, MediaType.TEXT_PLAIN_VALUE) + ), + String.class + ); + } + + private ResponseEntity corsPostRequest( + final String path, + final String origin, + final String authorization) { + return restTemplate.exchange( + serverUrl(path), + HttpMethod.POST, + httpHeaders( + entry(HttpHeaders.ORIGIN, origin), + entry(HttpHeaders.AUTHORIZATION, authorization), + entry(HttpHeaders.ACCEPT, MediaType.TEXT_PLAIN_VALUE) + ), + String.class + ); + } + private @NotNull String serverUrl(final String path) { return "http://localhost:" + this.serverPort + path; } diff --git a/src/test/java/net/hostsharing/hsadminng/ping/PingControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/ping/PingControllerAcceptanceTest.java index b0da6500..9383aba1 100644 --- a/src/test/java/net/hostsharing/hsadminng/ping/PingControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/ping/PingControllerAcceptanceTest.java @@ -72,6 +72,24 @@ class PingControllerAcceptanceTest { assertThat(responseBody).isEqualTo(testCase.expectedPongTranslation + "\n"); } + @Test + void pongPostRepliesWithTranslatedPongResponse() { + final var responseBody = RestAssured // @formatter:off + .given() + .header("Authorization", bearer("superuser-alex@hostsharing.net")) + .header("Accept-Language", Locale.GERMAN) + .port(port) + .when() + .post("http://localhost/api/pong") + .then().log().all().assertThat() + .statusCode(200) + .contentType("text/plain;charset=UTF-8") + .extract().body().asString(); + // @formatter:on + + assertThat(responseBody).isEqualTo("ponged superuser-alex@hostsharing.net - auf Deutsch\n"); + } + @Test void pingRepliesWithTranslatedPongResponse() { final var responseBody = RestAssured // @formatter:off diff --git a/src/test/java/net/hostsharing/hsadminng/ping/PingControllerRestTest.java b/src/test/java/net/hostsharing/hsadminng/ping/PingControllerRestTest.java index f54b2ea8..250bc9a9 100644 --- a/src/test/java/net/hostsharing/hsadminng/ping/PingControllerRestTest.java +++ b/src/test/java/net/hostsharing/hsadminng/ping/PingControllerRestTest.java @@ -81,4 +81,21 @@ class PingControllerRestTest { .andExpect(status().isOk()) .andExpect(content().string(containsString("superuser-alex@hostsharing.net"))); } + + @Test + void pongPostReturnsPongedWithSubject() throws Exception { + + // when + final var request = mockMvc.perform(MockMvcRequestBuilders + .post("/api/pong") + .header("Authorization", bearer("superuser-alex@hostsharing.net")) + .header("Accept-Language", "de") + .accept(MediaType.TEXT_PLAIN)) + .andDo(print()); + + // then + request + .andExpect(status().isOk()) + .andExpect(content().string(containsString("superuser-alex@hostsharing.net"))); + } }