-
Notifications
You must be signed in to change notification settings - Fork 4
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Sidm 5013 step up authentication (#513)
* SIDM-5013 Stash. * SIDM-5013 StepUp autn implementation. * SIDM-5013 Build file fixes. * SIDM-5013 Initial tests/ * add mfa tests * SIDM-5013 More tests. * update mfa tests * SIDM-5013 Add support for GET method in StepUp authn. * SIDM-5013 Make filter ordering more explicit. * SIDM-5013 Attempt to fix a CVE by bumping tomcat version. * CVE-2020-13943 Fix. * CVE-2020-13943 Fix. * SIDM-5013 Always delegate to idam-api. * SIDM-5013 Remove unused code. * mfa e2e tests * fix mfa tests Co-authored-by: shravan mechineni <shravanmechineni5@gmail.com>
- Loading branch information
1 parent
a905834
commit 5209b79
Showing
9 changed files
with
461 additions
and
263 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
126 changes: 126 additions & 0 deletions
126
src/main/java/uk/gov/hmcts/reform/idam/web/strategic/StepUpAuthenticationZuulFilter.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,126 @@ | ||
| package uk.gov.hmcts.reform.idam.web.strategic; | ||
|
|
||
| import com.fasterxml.jackson.core.JsonProcessingException; | ||
| import com.netflix.zuul.ZuulFilter; | ||
| import com.netflix.zuul.context.RequestContext; | ||
| import lombok.extern.slf4j.Slf4j; | ||
| import org.apache.commons.lang3.ObjectUtils; | ||
| import org.springframework.beans.factory.annotation.Autowired; | ||
| import org.springframework.http.HttpHeaders; | ||
| import org.springframework.stereotype.Component; | ||
| import uk.gov.hmcts.reform.idam.web.config.properties.ConfigurationProperties; | ||
| import uk.gov.hmcts.reform.idam.web.helper.MvcKeys; | ||
| import uk.gov.hmcts.reform.idam.web.sso.SSOZuulFilter; | ||
|
|
||
| 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; | ||
|
|
||
| import static com.netflix.zuul.constants.ZuulHeaders.X_FORWARDED_FOR; | ||
| import static org.springframework.cloud.netflix.zuul.filters.support.FilterConstants.PRE_TYPE; | ||
|
|
||
| @Slf4j | ||
| @Component | ||
| public class StepUpAuthenticationZuulFilter extends ZuulFilter { | ||
|
|
||
| public static final String ZUUL_PROCESSING_ERROR = "Cannot process authentication response"; | ||
| public static final String OIDC_AUTHORIZE_ENDPOINT = "/o/authorize"; | ||
|
|
||
| private final SPIService spiService; | ||
| private final String idamSessionCookieName; | ||
|
|
||
| @Autowired | ||
| public StepUpAuthenticationZuulFilter(@Nonnull final ConfigurationProperties configurationProperties, @Nonnull final SPIService spiService) { | ||
| this.idamSessionCookieName = configurationProperties.getStrategic().getSession().getIdamSessionCookie(); | ||
| this.spiService = spiService; | ||
| } | ||
|
|
||
| @Override | ||
| public String filterType() { | ||
| return PRE_TYPE; | ||
| } | ||
|
|
||
| /** | ||
| * {@inheritDoc} | ||
| * | ||
| * <p>Makes sure it runs AFTER the {@link SSOZuulFilter}.</p> | ||
| * | ||
| * @return | ||
| */ | ||
| @Override | ||
| public int filterOrder() { | ||
| return SSOZuulFilter.FILTER_ORDER + 1; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean shouldFilter() { | ||
| final HttpServletRequest request = RequestContext.getCurrentContext().getRequest(); | ||
| return isAuthorizeRequest(request) && hasSessionCookie(request); | ||
| } | ||
|
|
||
|
|
||
| @Override | ||
| public Object run() { | ||
| final RequestContext ctx = RequestContext.getCurrentContext(); | ||
| final HttpServletRequest request = ctx.getRequest(); | ||
|
|
||
| log.info("StepUp filter triggered."); | ||
|
|
||
| final String tokenId = getSessionToken(request); | ||
|
|
||
| try { | ||
| 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); | ||
| } | ||
| } | ||
|
|
||
| protected void dropCookie(@Nonnull final String cookieName, @Nonnull final RequestContext context) { | ||
| context.addZuulRequestHeader(HttpHeaders.COOKIE, cookieName + "="); | ||
| } | ||
|
|
||
| protected String getSessionToken(@Nonnull final HttpServletRequest request) { | ||
| return Arrays.stream(getCookiesFromRequest(request)) | ||
| .filter(cookie -> idamSessionCookieName.equals(cookie.getName())) | ||
| .map(Cookie::getValue) | ||
| .findAny() | ||
| .orElseThrow(); | ||
| } | ||
|
|
||
| protected boolean isAuthorizeRequest(@Nonnull final HttpServletRequest request) { | ||
| return request.getRequestURI().contains(OIDC_AUTHORIZE_ENDPOINT) && | ||
| ("post".equalsIgnoreCase(request.getMethod()) || "get".equalsIgnoreCase(request.getMethod())); | ||
| } | ||
|
|
||
| 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[]{}); | ||
| } | ||
| } |
95 changes: 95 additions & 0 deletions
95
src/test/java/uk/gov/hmcts/reform/idam/web/strategic/StepUpAuthenticationZuulFilterTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,95 @@ | ||
| package uk.gov.hmcts.reform.idam.web.strategic; | ||
|
|
||
| import junitparams.JUnitParamsRunner; | ||
| import junitparams.Parameters; | ||
| import org.junit.Before; | ||
| import org.junit.Test; | ||
| import org.junit.runner.RunWith; | ||
| 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 static org.junit.Assert.assertEquals; | ||
| import static org.junit.Assert.assertFalse; | ||
| import static org.junit.jupiter.api.Assertions.assertTrue; | ||
| import static org.mockito.ArgumentMatchers.any; | ||
| import static org.mockito.Mockito.doReturn; | ||
| import static org.mockito.Mockito.mock; | ||
| import static org.mockito.Mockito.spy; | ||
| import static org.mockito.Mockito.times; | ||
| import static org.mockito.Mockito.verify; | ||
| import static org.springframework.cloud.netflix.zuul.filters.support.FilterConstants.PRE_TYPE; | ||
|
|
||
| @RunWith(JUnitParamsRunner.class) | ||
| public class StepUpAuthenticationZuulFilterTest { | ||
|
|
||
| private StepUpAuthenticationZuulFilter filter; | ||
|
|
||
| @Before | ||
| public void setUp() { | ||
| final ConfigurationProperties config = new ConfigurationProperties(); | ||
| final StrategicConfigurationProperties strategicProperties = new StrategicConfigurationProperties(); | ||
| final StrategicConfigurationProperties.Session session = new StrategicConfigurationProperties.Session(); | ||
| session.setIdamSessionCookie("Idam.Session"); | ||
| strategicProperties.setSession(session); | ||
| config.setStrategic(strategicProperties); | ||
| this.filter = spy(new StepUpAuthenticationZuulFilter(config, null)); | ||
| } | ||
|
|
||
| @Test | ||
| public void filterType() { | ||
| assertEquals(PRE_TYPE, filter.filterType()); | ||
| } | ||
|
|
||
| @Test | ||
| 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()); | ||
|
|
||
| filter.shouldFilter(); | ||
|
|
||
| verify(filter, times(1)).isAuthorizeRequest(any()); | ||
| verify(filter, times(1)).hasSessionCookie(any()); | ||
| } | ||
|
|
||
| private Object isAuthorizeRequestParams() { | ||
| return new Object[]{ | ||
| new Object[]{"http://localhost:1234/o/authorize", "POST", true}, | ||
| new Object[]{"http://localhost:1234/o/authorize", "GET", true}, | ||
| new Object[]{"http://localhost:1234/o/authorize", "PUT", false}, | ||
| new Object[]{"http://localhost:1234/login?param=1", "POST", false}, | ||
| new Object[]{"http://localhost:1234/login?param=1", "GET", false}, | ||
| }; | ||
| } | ||
|
|
||
| @Test | ||
| @Parameters(method = "isAuthorizeRequestParams") | ||
| public void isAuthorizeRequest(String requestUrl, String httpMethod, Boolean expectedResult) { | ||
| final HttpServletRequest request = mock(HttpServletRequest.class); | ||
| doReturn(requestUrl).when(request).getRequestURI(); | ||
| doReturn(httpMethod).when(request).getMethod(); | ||
| assertEquals(expectedResult, filter.isAuthorizeRequest(request)); | ||
| } | ||
|
|
||
|
|
||
| @Test | ||
| public void hasSessionCookie() { | ||
| final HttpServletRequest request = mock(HttpServletRequest.class); | ||
|
|
||
| doReturn(new Cookie[]{}).when(request).getCookies(); | ||
| assertFalse(filter.hasSessionCookie(request)); | ||
|
|
||
| Cookie cookie = new Cookie("Idam.Session", "value"); | ||
| doReturn(new Cookie[]{cookie}).when(request).getCookies(); | ||
| assertTrue(filter.hasSessionCookie(request)); | ||
| } | ||
|
|
||
| } |
Oops, something went wrong.