diff --git a/src/main/java/uk/gov/hmcts/reform/idam/web/strategic/StepUpAuthenticationZuulFilter.java b/src/main/java/uk/gov/hmcts/reform/idam/web/strategic/StepUpAuthenticationZuulFilter.java index b7ff178fb..43e477b0f 100644 --- a/src/main/java/uk/gov/hmcts/reform/idam/web/strategic/StepUpAuthenticationZuulFilter.java +++ b/src/main/java/uk/gov/hmcts/reform/idam/web/strategic/StepUpAuthenticationZuulFilter.java @@ -16,7 +16,6 @@ import javax.annotation.Nonnull; import javax.servlet.http.Cookie; import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; import java.util.Arrays; import java.util.Optional; @@ -63,7 +62,6 @@ public boolean shouldFilter() { return isAuthorizeRequest(request) && hasSessionCookie(request); } - @Override public Object run() { final RequestContext ctx = RequestContext.getCurrentContext(); @@ -77,20 +75,17 @@ public Object run() { final String originIp = ObjectUtils.defaultIfNull(request.getHeader(X_FORWARDED_FOR), request.getRemoteAddr()); final String redirectUri = request.getParameter(MvcKeys.REDIRECT_URI); final ApiAuthResult authenticationResult = spiService.authenticate(tokenId, redirectUri, originIp); - if (!authenticationResult.isSuccess()) { - return unauthorizedResponse("AuthTree check for session token failed", ctx); - } if (authenticationResult.requiresMfa()) { dropCookie(idamSessionCookieName, ctx); } - // continue as usual (delegate to idam-api) - ctx.setSendZuulResponse(true); - return null; } catch (final JsonProcessingException e) { - return unauthorizedResponse(ZUUL_PROCESSING_ERROR, ctx); + log.error(ZUUL_PROCESSING_ERROR, e); } + // continue as usual (delegate to idam-api) + ctx.setSendZuulResponse(true); + return null; } protected void dropCookie(@Nonnull final String cookieName, @Nonnull final RequestContext context) { @@ -114,13 +109,6 @@ protected boolean hasSessionCookie(@Nonnull final HttpServletRequest request) { return Arrays.stream(getCookiesFromRequest(request)).anyMatch(cookie -> idamSessionCookieName.equals(cookie.getName())); } - protected Object unauthorizedResponse(@Nonnull final String errorCause, @Nonnull final RequestContext context) { - log.error("StepUp authentication failed: {}", errorCause); - context.setResponseStatusCode(HttpServletResponse.SC_UNAUTHORIZED); - context.setSendZuulResponse(false); - return null; - } - @Nonnull protected Cookie[] getCookiesFromRequest(@Nonnull final HttpServletRequest request) { return Optional.ofNullable(request.getCookies()).orElse(new Cookie[]{}); diff --git a/src/test/java/uk/gov/hmcts/reform/idam/web/strategic/StepUpAuthenticationZuulFilterTest.java b/src/test/java/uk/gov/hmcts/reform/idam/web/strategic/StepUpAuthenticationZuulFilterTest.java index eeb5cf941..4dc891e2c 100644 --- a/src/test/java/uk/gov/hmcts/reform/idam/web/strategic/StepUpAuthenticationZuulFilterTest.java +++ b/src/test/java/uk/gov/hmcts/reform/idam/web/strategic/StepUpAuthenticationZuulFilterTest.java @@ -1,22 +1,29 @@ package uk.gov.hmcts.reform.idam.web.strategic; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.netflix.zuul.context.RequestContext; import junitparams.JUnitParamsRunner; import junitparams.Parameters; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.springframework.http.HttpStatus; import uk.gov.hmcts.reform.idam.web.config.properties.ConfigurationProperties; import uk.gov.hmcts.reform.idam.web.config.properties.StrategicConfigurationProperties; import uk.gov.hmcts.reform.idam.web.sso.SSOZuulFilter; import javax.servlet.http.Cookie; import javax.servlet.http.HttpServletRequest; +import java.util.Map; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; @@ -27,6 +34,7 @@ public class StepUpAuthenticationZuulFilterTest { private StepUpAuthenticationZuulFilter filter; + private SPIService spiService; @Before public void setUp() { @@ -36,7 +44,8 @@ public void setUp() { session.setIdamSessionCookie("Idam.Session"); strategicProperties.setSession(session); config.setStrategic(strategicProperties); - this.filter = spy(new StepUpAuthenticationZuulFilter(config, null)); + this.spiService = mock(SPIService.class); + this.filter = spy(new StepUpAuthenticationZuulFilter(config, this.spiService)); } @Test @@ -49,15 +58,23 @@ public void filterOrder() { assertTrue(filter.filterOrder() > SSOZuulFilter.FILTER_ORDER); } - @Test - public void shouldFilter() { - doReturn(true).when(filter).isAuthorizeRequest(any()); - doReturn(true).when(filter).hasSessionCookie(any()); + private Object shouldFilterParams() { + return new Object[]{ + new Object[]{true, true, true}, + new Object[]{true, false, false}, + new Object[]{false, true, false}, + new Object[]{false, false, false}, + }; + } - filter.shouldFilter(); + @Test + @Parameters(method = "shouldFilterParams") + public void shouldFilter(final boolean isAuthorize, final boolean hasSessionCookie, final boolean expectedResult) { + doReturn(isAuthorize).when(filter).isAuthorizeRequest(any()); + doReturn(hasSessionCookie).when(filter).hasSessionCookie(any()); - verify(filter, times(1)).isAuthorizeRequest(any()); - verify(filter, times(1)).hasSessionCookie(any()); + final boolean shouldFilter = filter.shouldFilter(); + assertEquals(shouldFilter, expectedResult); } private Object isAuthorizeRequestParams() { @@ -79,7 +96,6 @@ public void isAuthorizeRequest(String requestUrl, String httpMethod, Boolean exp assertEquals(expectedResult, filter.isAuthorizeRequest(request)); } - @Test public void hasSessionCookie() { final HttpServletRequest request = mock(HttpServletRequest.class); @@ -92,4 +108,87 @@ public void hasSessionCookie() { assertTrue(filter.hasSessionCookie(request)); } + @Test + public void getSessionToken() { + final HttpServletRequest request = mock(HttpServletRequest.class); + final String sessionCookieValue = "test"; + Cookie[] cookies = new Cookie[]{ + new Cookie("xyz", "abc"), + new Cookie("Idam.Session", sessionCookieValue), + }; + doReturn(cookies).when(request).getCookies(); + + final String sessionToken = filter.getSessionToken(request); + + assertEquals(sessionCookieValue, sessionToken); + } + + @Test + public void dropCookie() { + final String cookieName = "test"; + final RequestContext context = new RequestContext(); + filter.dropCookie(cookieName, context); + final Map.Entry cookie = context.getZuulRequestHeaders() + .entrySet() + .stream() + .filter(h -> "cookie".equalsIgnoreCase(h.getKey())) + .findFirst() + .orElseThrow(); + + assertEquals(cookieName + "=", cookie.getValue()); + } + + @Test + public void run_shouldDelegateOnUnauthorized() throws JsonProcessingException { + final String sessionToken = "sessionToken"; + doReturn(sessionToken).when(filter).getSessionToken(any()); + + final RequestContext context = new RequestContext(); + final HttpServletRequest request = mock(HttpServletRequest.class); + context.setRequest(request); + RequestContext.testSetCurrentContext(context); + + final ApiAuthResult authResult = ApiAuthResult.builder().httpStatus(HttpStatus.UNAUTHORIZED).build(); + doReturn(authResult).when(spiService).authenticate(eq(sessionToken), any(), any()); + + assertNull(filter.run()); + assertTrue(RequestContext.getCurrentContext().sendZuulResponse()); + } + + @Test + public void run_shouldDelegateOnJsonException() throws JsonProcessingException { + final String sessionToken = "sessionToken"; + doReturn(sessionToken).when(filter).getSessionToken(any()); + + final RequestContext context = new RequestContext(); + final HttpServletRequest request = mock(HttpServletRequest.class); + context.setRequest(request); + RequestContext.testSetCurrentContext(context); + + doThrow(mock(JsonProcessingException.class)).when(spiService).authenticate(eq(sessionToken), any(), any()); + + assertNull(filter.run()); + assertTrue(RequestContext.getCurrentContext().sendZuulResponse()); + } + + @Test + public void run_shouldCallDropCookieIfMfaRequired() throws JsonProcessingException { + final String sessionToken = "sessionToken"; + doReturn(sessionToken).when(filter).getSessionToken(any()); + + final RequestContext context = new RequestContext(); + final HttpServletRequest request = mock(HttpServletRequest.class); + context.setRequest(request); + RequestContext.testSetCurrentContext(context); + + final ApiAuthResult authResult = + ApiAuthResult.builder() + .policiesAction(EvaluatePoliciesAction.MFA_REQUIRED) + .build(); + doReturn(authResult).when(spiService).authenticate(eq(sessionToken), any(), any()); + + assertNull(filter.run()); + assertTrue(RequestContext.getCurrentContext().sendZuulResponse()); + verify(filter, times(1)).dropCookie(any(), any()); + } } \ No newline at end of file