From d2666fc1eb5a87e0c0ae693bdb09675cef8b31c0 Mon Sep 17 00:00:00 2001 From: OZGCloud <ozgcloud@mgm-tp.com> Date: Thu, 15 Feb 2024 13:43:09 +0100 Subject: [PATCH] OZG-4939 added PR13 comments --- pom.xml | 5 -- .../errorhandling/AdminExceptionHandler.java | 9 ---- .../AdminAuthenticationEntryPoint.java | 6 +-- .../admin/security/SecurityConfiguration.java | 7 +-- src/main/resources/application-dev.yaml | 5 +- src/main/resources/application-remotekc.yaml | 3 ++ src/main/resources/application.yaml | 8 ++- .../AdminAuthenticationEntryPointTest.java | 51 ++++++++----------- .../AuthenticationExceptionTestFactory.java | 4 +- ....java => SecurityConfigurationITCase.java} | 39 ++------------ ...curityConfigurationWithKeycloakITCase.java | 30 ++++------- src/test/resources/application.yaml | 24 +++++++++ 12 files changed, 80 insertions(+), 111 deletions(-) create mode 100644 src/main/resources/application-remotekc.yaml rename src/test/java/de/ozgcloud/admin/security/{SecurityConfigurationLocalITCase.java => SecurityConfigurationITCase.java} (74%) create mode 100644 src/test/resources/application.yaml diff --git a/pom.xml b/pom.xml index 1ac1a465..ce74a874 100644 --- a/pom.xml +++ b/pom.xml @@ -119,11 +119,6 @@ <version>${keycloak-admin-client.version}</version> <scope>test</scope> </dependency> - <dependency> - <groupId>org.springframework.boot</groupId> - <artifactId>spring-boot-starter-webflux</artifactId> - <scope>test</scope> - </dependency> </dependencies> <profiles> <profile> diff --git a/src/main/java/de/ozgcloud/admin/errorhandling/AdminExceptionHandler.java b/src/main/java/de/ozgcloud/admin/errorhandling/AdminExceptionHandler.java index d3594101..27f9c626 100644 --- a/src/main/java/de/ozgcloud/admin/errorhandling/AdminExceptionHandler.java +++ b/src/main/java/de/ozgcloud/admin/errorhandling/AdminExceptionHandler.java @@ -25,11 +25,9 @@ import java.util.Map; import jakarta.validation.ConstraintViolationException; -import org.apache.http.HttpHeaders; import org.springframework.data.rest.webmvc.ResourceNotFoundException; import org.springframework.http.HttpStatus; import org.springframework.security.access.AccessDeniedException; -import org.springframework.security.authentication.InsufficientAuthenticationException; import org.springframework.web.ErrorResponse; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.RestControllerAdvice; @@ -62,11 +60,4 @@ public class AdminExceptionHandler extends ResponseEntityExceptionHandler { return ErrorResponse.builder(ex, status, ex.getLocalizedMessage()).build(); } - @ExceptionHandler(InsufficientAuthenticationException.class) - public ErrorResponse handleInsufficientException(InsufficientAuthenticationException ex) { - return ErrorResponse - .builder(ex, HttpStatus.UNAUTHORIZED, ex.getLocalizedMessage()) - .header(HttpHeaders.WWW_AUTHENTICATE, "Bearer realm=\"Restricted Content\"") - .build(); - } } diff --git a/src/main/java/de/ozgcloud/admin/security/AdminAuthenticationEntryPoint.java b/src/main/java/de/ozgcloud/admin/security/AdminAuthenticationEntryPoint.java index 7d85f880..f519b200 100644 --- a/src/main/java/de/ozgcloud/admin/security/AdminAuthenticationEntryPoint.java +++ b/src/main/java/de/ozgcloud/admin/security/AdminAuthenticationEntryPoint.java @@ -43,7 +43,7 @@ import lombok.RequiredArgsConstructor; @Component @RequiredArgsConstructor -public class AdminAuthenticationEntryPoint implements AuthenticationEntryPoint { +class AdminAuthenticationEntryPoint implements AuthenticationEntryPoint { private final List<HttpMessageConverter<?>> converters; @@ -51,11 +51,11 @@ public class AdminAuthenticationEntryPoint implements AuthenticationEntryPoint { public void commence(HttpServletRequest request, HttpServletResponse response, AuthenticationException authException) throws IOException, ServletException { ServerResponse - .from(errorResponse(authException, request.getRequestURI())) + .from(buildErrorResponse(authException, request.getRequestURI())) .writeTo(request, response, () -> converters); } - ErrorResponse errorResponse(AuthenticationException ex, String requestUri) { + ErrorResponse buildErrorResponse(AuthenticationException ex, String requestUri) { return ErrorResponse .builder(ex, HttpStatus.UNAUTHORIZED, ex.getLocalizedMessage()) .header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_PROBLEM_JSON_VALUE) diff --git a/src/main/java/de/ozgcloud/admin/security/SecurityConfiguration.java b/src/main/java/de/ozgcloud/admin/security/SecurityConfiguration.java index 540ba7ff..568d79a7 100644 --- a/src/main/java/de/ozgcloud/admin/security/SecurityConfiguration.java +++ b/src/main/java/de/ozgcloud/admin/security/SecurityConfiguration.java @@ -28,7 +28,6 @@ import org.springframework.security.config.Customizer; import org.springframework.security.config.annotation.method.configuration.EnableMethodSecurity; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; -import org.springframework.security.config.annotation.web.configurers.AbstractHttpConfigurer; import org.springframework.security.config.http.SessionCreationPolicy; import org.springframework.security.oauth2.core.oidc.StandardClaimNames; import org.springframework.security.oauth2.server.resource.authentication.JwtAuthenticationConverter; @@ -47,13 +46,9 @@ public class SecurityConfiguration { @Bean SecurityFilterChain filterChain(HttpSecurity http) throws Exception { - // Configure a resource server with JWT decoder http.oauth2ResourceServer(oauth2 -> oauth2.jwt(Customizer.withDefaults())); - // State-less session (state in access-token only) - http.sessionManagement(sm -> sm.sessionCreationPolicy(SessionCreationPolicy.STATELESS)); - // Disable CSRF because of state-less session-management - http.csrf(AbstractHttpConfigurer::disable); + http.sessionManagement(sm -> sm.sessionCreationPolicy(SessionCreationPolicy.STATELESS)); http.exceptionHandling(eh -> eh.authenticationEntryPoint(authenticationEntryPoint)); diff --git a/src/main/resources/application-dev.yaml b/src/main/resources/application-dev.yaml index c3514d2f..8e95ffe2 100644 --- a/src/main/resources/application-dev.yaml +++ b/src/main/resources/application-dev.yaml @@ -1,2 +1,5 @@ ozgcloud: - production: false \ No newline at end of file + production: false +keycloak: + realm: by-kiel-dev + auth-server-url: https://sso.dev.by.ozg-cloud.de \ No newline at end of file diff --git a/src/main/resources/application-remotekc.yaml b/src/main/resources/application-remotekc.yaml new file mode 100644 index 00000000..b874dfa6 --- /dev/null +++ b/src/main/resources/application-remotekc.yaml @@ -0,0 +1,3 @@ +keycloak: + realm: by-kiel-dev + auth-server-url: https://sso.dev.by.ozg-cloud.de \ No newline at end of file diff --git a/src/main/resources/application.yaml b/src/main/resources/application.yaml index 6da3e117..3b5cfa55 100644 --- a/src/main/resources/application.yaml +++ b/src/main/resources/application.yaml @@ -54,7 +54,6 @@ spring: authentication-database: admin rest: basePath: /api/configuration - cloud: config: server: @@ -63,4 +62,9 @@ spring: oauth2: resourceserver: jwt: - issuer-uri: https://sso.dev.by.ozg-cloud.de/realms/by-kiel-dev \ No newline at end of file + issuer-uri: ${ozgcloud.oauth2.issuer-uri} +ozgcloud: + oauth2: + auth-server-url: ${keycloak.auth-server-url} + realm: ${keycloak.realm} + issuer-uri: ${ozgcloud.oauth2.auth-server-url}/realms/${ozgcloud.oauth2.realm} \ No newline at end of file diff --git a/src/test/java/de/ozgcloud/admin/security/AdminAuthenticationEntryPointTest.java b/src/test/java/de/ozgcloud/admin/security/AdminAuthenticationEntryPointTest.java index f2278aeb..d8cc03da 100644 --- a/src/test/java/de/ozgcloud/admin/security/AdminAuthenticationEntryPointTest.java +++ b/src/test/java/de/ozgcloud/admin/security/AdminAuthenticationEntryPointTest.java @@ -22,6 +22,7 @@ package de.ozgcloud.admin.security; import static org.assertj.core.api.Assertions.*; +import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.*; import java.net.URI; @@ -48,6 +49,7 @@ import org.springframework.http.ProblemDetail; import org.springframework.http.converter.HttpMessageConverter; import org.springframework.web.ErrorResponse; import org.springframework.web.servlet.function.ServerResponse; +import org.springframework.web.servlet.function.ServerResponse.Context; import lombok.SneakyThrows; @@ -61,20 +63,19 @@ public class AdminAuthenticationEntryPointTest { private AdminAuthenticationEntryPoint authenticationEntryPoint; @Mock - List<HttpMessageConverter<?>> converters; + private List<HttpMessageConverter<?>> converters; @Mock private HttpServletRequest request; - @Mock - private HttpServletResponse response; - - @Mock - private ServerResponse serverResponse; - @DisplayName("consume") @Nested class TestConsume { + @Mock + private HttpServletResponse response; + + @Mock + private ServerResponse serverResponse; @BeforeEach void initMock() { @@ -82,7 +83,7 @@ public class AdminAuthenticationEntryPointTest { } @Captor - ArgumentCaptor<ServerResponse.Context> contextCaptor; + ArgumentCaptor<Context> contextCaptor; @DisplayName("call writeTo") @SneakyThrows @@ -102,71 +103,63 @@ public class AdminAuthenticationEntryPointTest { @DisplayName("error response") @Nested - class TestErrorResponse { + class TestBuildErrorResponse { - @DisplayName("body") @Nested class TestBody { - @DisplayName("have instance") @Test void shouldHaveInstance() { - var problemDetail = body(); + var problemDetail = getErrorResponseBody(); assertThat(problemDetail.getInstance()).isEqualTo(URI.create(REQUEST_URI)); } - @DisplayName("have detail") @Test void shouldHaveDetail() { - var problemDetail = body(); + var problemDetail = getErrorResponseBody(); assertThat(problemDetail.getDetail()).isEqualTo(AuthenticationExceptionTestFactory.AUTH_MESSAGE); } - private ProblemDetail body() { - return serverErrorResponse().getBody(); + private ProblemDetail getErrorResponseBody() { + return getServerErrorResponse().getBody(); } } - @DisplayName("headers") @Nested class TestHeaders { - @DisplayName("have content type") @Test void shouldHaveContentType() { - var headers = httpHeaders(); + var headers = getHttpHeaders(); assertThat(headers.getContentType()).isEqualTo(MediaType.APPLICATION_PROBLEM_JSON); } - @DisplayName("have www authentication") @Test void shouldHaveWwwAuthentication() { - var headers = httpHeaders(); + var headers = getHttpHeaders(); assertThat(headers.getFirst(HttpHeaders.WWW_AUTHENTICATE)).isEqualTo("Bearer realm=\"Restricted Content\""); } - private HttpHeaders httpHeaders() { - return serverErrorResponse().getHeaders(); + private HttpHeaders getHttpHeaders() { + return getServerErrorResponse().getHeaders(); } } - @DisplayName("have status") @Test void shouldHaveStatus() { - var response = serverErrorResponse(); + var response = getServerErrorResponse(); assertThat(response.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED); } - private ErrorResponse serverErrorResponse() { - return authenticationEntryPoint.errorResponse( + private ErrorResponse getServerErrorResponse() { + return authenticationEntryPoint.buildErrorResponse( AuthenticationExceptionTestFactory.create(), - REQUEST_URI - ); + REQUEST_URI); } } diff --git a/src/test/java/de/ozgcloud/admin/security/AuthenticationExceptionTestFactory.java b/src/test/java/de/ozgcloud/admin/security/AuthenticationExceptionTestFactory.java index b5cd3996..3727bfc0 100644 --- a/src/test/java/de/ozgcloud/admin/security/AuthenticationExceptionTestFactory.java +++ b/src/test/java/de/ozgcloud/admin/security/AuthenticationExceptionTestFactory.java @@ -40,10 +40,10 @@ public class AuthenticationExceptionTestFactory { } public static DummyAuthenticationException create() { - return builder().msg(AUTH_MESSAGE).build(); + return builder().build(); } public static DummyAuthenticationException.DummyAuthenticationExceptionBuilder builder() { - return DummyAuthenticationException.builder(); + return DummyAuthenticationException.builder().msg(AUTH_MESSAGE); } } diff --git a/src/test/java/de/ozgcloud/admin/security/SecurityConfigurationLocalITCase.java b/src/test/java/de/ozgcloud/admin/security/SecurityConfigurationITCase.java similarity index 74% rename from src/test/java/de/ozgcloud/admin/security/SecurityConfigurationLocalITCase.java rename to src/test/java/de/ozgcloud/admin/security/SecurityConfigurationITCase.java index 4047dcce..653c7b68 100644 --- a/src/test/java/de/ozgcloud/admin/security/SecurityConfigurationLocalITCase.java +++ b/src/test/java/de/ozgcloud/admin/security/SecurityConfigurationITCase.java @@ -21,12 +21,9 @@ */ package de.ozgcloud.admin.security; -import static org.junit.jupiter.api.Assertions.*; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; -import java.net.URI; - import org.apache.http.HttpHeaders; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; @@ -36,23 +33,15 @@ import org.junit.jupiter.params.provider.ValueSource; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; import org.springframework.http.HttpStatus; -import org.springframework.security.core.Authentication; -import org.springframework.security.core.AuthenticationException; -import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.ResultActions; -import org.springframework.web.ErrorResponse; - -import com.fasterxml.jackson.annotation.JsonInclude.Include; -import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.ObjectMapper; import de.ozgcloud.common.test.DataITCase; import lombok.SneakyThrows; @DataITCase @AutoConfigureMockMvc -public class SecurityConfigurationLocalITCase { +class SecurityConfigurationITCase { @Autowired private MockMvc mockMvc; @@ -121,30 +110,16 @@ public class SecurityConfigurationLocalITCase { @Test @SneakyThrows - void shouldHaveErrorInfoInBody() { - var expected = getExpectedProblemDetailsAsString("/api"); - + void shouldHaveErrorDetailInBody() { var result = doPerform("/api"); - result.andExpect(content().string(expected)); - } + result.andExpect(jsonPath("$.detail").value("Full authentication is required to access this resource")); - private String getExpectedProblemDetailsAsString(String requestUri) throws JsonProcessingException { - var ex = new AuthenticationException("Full authentication is required to access this resource") { - }; - var problemDetail = ErrorResponse - .builder(ex, HttpStatus.UNAUTHORIZED, ex.getLocalizedMessage()) - .instance(URI.create(requestUri)) - .build() - .getBody(); - - var objectMapper = new ObjectMapper().setSerializationInclusion(Include.NON_NULL); - return objectMapper.writeValueAsString(problemDetail); } @Test @SneakyThrows - void shouldHaveWWW_AUTHENTICATEHeader2() { + void shouldHaveHeader() { var result = doPerform("/api"); result.andExpect(header().string(HttpHeaders.WWW_AUTHENTICATE, "Bearer realm=\"Restricted Content\"")); @@ -183,13 +158,7 @@ public class SecurityConfigurationLocalITCase { @SneakyThrows private ResultActions doPerformAuthenticated(String path) { - var auth = authentication(); - assertTrue(auth.isAuthenticated()); return mockMvc.perform(get(path)); } } - - private Authentication authentication() { - return SecurityContextHolder.getContext().getAuthentication(); - } } diff --git a/src/test/java/de/ozgcloud/admin/security/SecurityConfigurationWithKeycloakITCase.java b/src/test/java/de/ozgcloud/admin/security/SecurityConfigurationWithKeycloakITCase.java index 2d58184a..e336b21c 100644 --- a/src/test/java/de/ozgcloud/admin/security/SecurityConfigurationWithKeycloakITCase.java +++ b/src/test/java/de/ozgcloud/admin/security/SecurityConfigurationWithKeycloakITCase.java @@ -25,6 +25,7 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers. import java.net.URI; import java.util.Collections; +import java.util.Map; import org.apache.http.client.utils.URIBuilder; import org.junit.jupiter.api.AfterAll; @@ -33,7 +34,6 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.boot.json.JacksonJsonParser; import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; import org.springframework.http.MediaType; import org.springframework.test.context.DynamicPropertyRegistry; @@ -41,8 +41,7 @@ import org.springframework.test.context.DynamicPropertySource; import org.springframework.test.web.servlet.MockMvc; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; -import org.springframework.web.reactive.function.BodyInserters; -import org.springframework.web.reactive.function.client.WebClient; +import org.springframework.web.client.RestClient; import dasniko.testcontainers.keycloak.KeycloakContainer; import de.ozgcloud.admin.RootController; @@ -94,9 +93,9 @@ class SecurityConfigurationWithKeycloakITCase { String getToken() { MultiValueMap<String, String> formData = setPostBodyForToken(); - String result = performPostRequestToKeycloak(formData); + Map<String, String> resultBody = performPostRequestToKeycloak(formData); - return extractTokenFromResponse(result); + return "Bearer " + resultBody.get("access_token").toString(); } MultiValueMap<String, String> setPostBodyForToken() { @@ -108,24 +107,17 @@ class SecurityConfigurationWithKeycloakITCase { return formData; } + @SuppressWarnings("unchecked") @SneakyThrows - String performPostRequestToKeycloak(MultiValueMap<String, String> formData) { + Map<String, String> performPostRequestToKeycloak(MultiValueMap<String, String> formData) { + RestClient restClient = RestClient.create(); URI authorizationURI = new URIBuilder(keycloak.getAuthServerUrl() + "/realms/by-kiel-dev/protocol/openid-connect/token").build(); - WebClient webclient = WebClient.builder().build(); - return webclient.post() - .uri(authorizationURI) + var response = restClient.post().uri(authorizationURI) .contentType(MediaType.APPLICATION_FORM_URLENCODED) - .body(BodyInserters.fromFormData(formData)) - .retrieve() - .bodyToMono(String.class) - .block(); - } + .body(formData); + return response.retrieve().body(Map.class); - String extractTokenFromResponse(String result) { - JacksonJsonParser jsonParser = new JacksonJsonParser(); - return "Bearer " + jsonParser.parseMap(result) - .get("access_token") - .toString(); } + } } \ No newline at end of file diff --git a/src/test/resources/application.yaml b/src/test/resources/application.yaml new file mode 100644 index 00000000..dd4fdd1c --- /dev/null +++ b/src/test/resources/application.yaml @@ -0,0 +1,24 @@ + +management: + server: + port: 8081 +spring: + application: + name: OzgCloud_Administration + data: + mongodb: + authentication-database: admin + rest: + basePath: /api/configuration + cloud: + config: + server: + prefix: /configserver + security: + oauth2: + resourceserver: + jwt: + issuer-uri: ${keycloak.auth-server-url}/realms/${keycloak.realm} +keycloak: + auth-server-url: https://sso.dev.by.ozg-cloud.de + realm: by-kiel-dev \ No newline at end of file -- GitLab