From db5f3e1fc72065397898bf5e8d1f03f3140600d0 Mon Sep 17 00:00:00 2001 From: Remigiusz Janeczek Date: Thu, 24 Oct 2019 08:50:49 +0200 Subject: Fix security issue in api interceptor Issue-ID: DCAEGEN2-1880 Change-Id: I5b93dd8405ef9a0a364c6e1224afcfacc9df1fba Signed-off-by: Remigiusz Janeczek --- .../onap/dcae/restapi/ApiAuthInterceptionTest.java | 73 +++++++++++++++------- 1 file changed, 49 insertions(+), 24 deletions(-) (limited to 'src/test') diff --git a/src/test/java/org/onap/dcae/restapi/ApiAuthInterceptionTest.java b/src/test/java/org/onap/dcae/restapi/ApiAuthInterceptionTest.java index c80b56cb..250292f3 100644 --- a/src/test/java/org/onap/dcae/restapi/ApiAuthInterceptionTest.java +++ b/src/test/java/org/onap/dcae/restapi/ApiAuthInterceptionTest.java @@ -32,6 +32,7 @@ import org.onap.dcae.common.configuration.AuthMethodType; import org.slf4j.Logger; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; +import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors; import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; @@ -50,6 +51,9 @@ public class ApiAuthInterceptionTest { private static final String USERNAME = "Foo"; private static final String PASSWORD = "Bar"; private static final Map CREDENTIALS = HashMap.of(USERNAME, PASSWORD); + private static final int HTTP_PORT = 8080; + private static final int OUTSIDE_PORT = 30235; + public static final String HEALTHCHECK_URL = "/healthcheck"; @Mock private Logger log; @@ -70,21 +74,6 @@ public class ApiAuthInterceptionTest { private ApiAuthInterceptor sut; - private HttpServletRequest createEmptyRequest() { - return MockMvcRequestBuilders - .post("") - .buildRequest(null); - } - - private HttpServletRequest createRequestWithAuthorizationHeader() { - return SecurityMockMvcRequestPostProcessors - .httpBasic(USERNAME, PASSWORD) - .postProcessRequest( - MockMvcRequestBuilders - .post("") - .buildRequest(null)); - } - @Test public void shouldSucceedWhenAuthorizationIsDisabled() throws IOException { // given @@ -176,16 +165,12 @@ public class ApiAuthInterceptionTest { } @Test - public void shouldSucceedForHealthcheckOnHealthcheckPort() throws IOException { + public void shouldSucceedForHealthcheckOnHealthcheckPortWhenRequestFromInsideCluster() throws IOException { // given - final HttpServletRequest request = - MockMvcRequestBuilders - .get("/healthcheck") - .buildRequest(null); + final HttpServletRequest request = createRequestWithPorts(HTTP_PORT, HTTP_PORT, HEALTHCHECK_URL); when(settings.authMethod()).thenReturn(AuthMethodType.CERT_BASIC_AUTH.value()); - when(settings.httpPort()).thenReturn(request.getServerPort()); - + when(settings.httpPort()).thenReturn(HTTP_PORT); // when final boolean isAuthorized = sut.preHandle(request, response, obj); @@ -193,13 +178,30 @@ public class ApiAuthInterceptionTest { assertTrue(isAuthorized); } + @Test + public void shouldFailForHealthcheckOnHealthcheckPortWhenRequestFromOutsideCluster() throws IOException { + // given + final HttpServletRequest request = createRequestWithPorts(HTTP_PORT, OUTSIDE_PORT, HEALTHCHECK_URL); + + when(settings.authMethod()).thenReturn(AuthMethodType.CERT_BASIC_AUTH.value()); + when(settings.httpPort()).thenReturn(HTTP_PORT); + when(response.getWriter()).thenReturn(writer); + + // when + final boolean isAuthorized = sut.preHandle(request, response, obj); + + // then + assertFalse(isAuthorized); + verify(response).setStatus(HttpStatus.BAD_REQUEST.value()); + } + @Test public void shouldFailDueToNotPermittedOperationOnHealthcheckPort() throws IOException { // given - final HttpServletRequest request = createEmptyRequest(); + final HttpServletRequest request = createRequestWithPorts(HTTP_PORT, HTTP_PORT, "/"); when(settings.authMethod()).thenReturn(AuthMethodType.CERT_BASIC_AUTH.value()); - when(settings.httpPort()).thenReturn(request.getServerPort()); + when(settings.httpPort()).thenReturn(HTTP_PORT); when(response.getWriter()).thenReturn(writer); // when @@ -210,4 +212,27 @@ public class ApiAuthInterceptionTest { verify(response).setStatus(HttpStatus.BAD_REQUEST.value()); } + private HttpServletRequest createEmptyRequest() { + return MockMvcRequestBuilders + .post("") + .buildRequest(null); + } + + private HttpServletRequest createRequestWithAuthorizationHeader() { + return SecurityMockMvcRequestPostProcessors + .httpBasic(USERNAME, PASSWORD) + .postProcessRequest( + MockMvcRequestBuilders + .post("") + .buildRequest(null)); + } + + private HttpServletRequest createRequestWithPorts(int localPort, int serverPort, String urlTemplate) { + MockHttpServletRequest healthcheckRequest = MockMvcRequestBuilders + .get(urlTemplate) + .buildRequest(null); + healthcheckRequest.setLocalPort(localPort); + healthcheckRequest.setServerPort(serverPort); + return healthcheckRequest; + } } -- cgit 1.2.3-korg