From f1c3295cece32bc041143895750df158f69feefd Mon Sep 17 00:00:00 2001 From: Scott Rossillo Date: Fri, 20 Nov 2015 17:56:39 -0500 Subject: [PATCH] KEYCLOAK-1391: Return an HTTP 401 for API requests Non browser HTTP requests shouldn't redirect to the Keycloak login page. Instead, return an HTTP 401 with a proper WWW-Authenticate header. --- ...HttpHeaderInspectingApiRequestMatcher.java | 38 +++++++++++++ .../KeycloakAuthenticationEntryPoint.java | 57 +++++++++++++++++-- ...HeaderInspectingApiRequestMatcherTest.java | 43 ++++++++++++++ .../KeycloakAuthenticationEntryPointTest.java | 20 ++++++- 4 files changed, 151 insertions(+), 7 deletions(-) create mode 100644 integration/spring-security/src/main/java/org/keycloak/adapters/springsecurity/authentication/HttpHeaderInspectingApiRequestMatcher.java create mode 100644 integration/spring-security/src/test/java/org/keycloak/adapters/springsecurity/authentication/HttpHeaderInspectingApiRequestMatcherTest.java diff --git a/integration/spring-security/src/main/java/org/keycloak/adapters/springsecurity/authentication/HttpHeaderInspectingApiRequestMatcher.java b/integration/spring-security/src/main/java/org/keycloak/adapters/springsecurity/authentication/HttpHeaderInspectingApiRequestMatcher.java new file mode 100644 index 0000000000..fd2b927588 --- /dev/null +++ b/integration/spring-security/src/main/java/org/keycloak/adapters/springsecurity/authentication/HttpHeaderInspectingApiRequestMatcher.java @@ -0,0 +1,38 @@ +package org.keycloak.adapters.springsecurity.authentication; + +import org.apache.http.HttpHeaders; +import org.springframework.http.MediaType; +import org.springframework.security.web.util.matcher.RequestMatcher; + +import javax.servlet.http.HttpServletRequest; + +/** + * {@link RequestMatcher} that determines if a given request is an API request or an + * interactive login request. + * + * @author Scott Rossillo + * @see RequestMatcher + */ +public class HttpHeaderInspectingApiRequestMatcher implements RequestMatcher { + + protected static final String X_REQUESTED_WITH_HEADER = "X-Requested-With"; + protected static final String X_REQUESTED_WITH_HEADER_AJAX_VALUE = "XMLHttpRequest"; + + /** + * Returns true if the given request is an API request or false if it's an interactive + * login request. + * + * @param request the HttpServletRequest + * @return true if the given request is an API request; + * false otherwise + */ + @Override + public boolean matches(HttpServletRequest request) { + boolean ajax = X_REQUESTED_WITH_HEADER_AJAX_VALUE.equals(request.getHeader(X_REQUESTED_WITH_HEADER)); + boolean html = request.getHeader(HttpHeaders.ACCEPT) != null && request.getHeader(HttpHeaders.ACCEPT).contains( + MediaType.TEXT_HTML_VALUE); + + return ajax || !html; + } + +} diff --git a/integration/spring-security/src/main/java/org/keycloak/adapters/springsecurity/authentication/KeycloakAuthenticationEntryPoint.java b/integration/spring-security/src/main/java/org/keycloak/adapters/springsecurity/authentication/KeycloakAuthenticationEntryPoint.java index 3357806d85..f498fe6f3e 100644 --- a/integration/spring-security/src/main/java/org/keycloak/adapters/springsecurity/authentication/KeycloakAuthenticationEntryPoint.java +++ b/integration/spring-security/src/main/java/org/keycloak/adapters/springsecurity/authentication/KeycloakAuthenticationEntryPoint.java @@ -1,9 +1,12 @@ package org.keycloak.adapters.springsecurity.authentication; +import org.apache.http.HttpHeaders; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.http.HttpStatus; import org.springframework.security.core.AuthenticationException; import org.springframework.security.web.AuthenticationEntryPoint; +import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.util.Assert; import javax.servlet.ServletException; @@ -12,10 +15,15 @@ import javax.servlet.http.HttpServletResponse; import java.io.IOException; /** - * Provides a Keycloak {@link AuthenticationEntryPoint authentication entry point}. + * Provides a Keycloak {@link AuthenticationEntryPoint authentication entry point}. Uses a + * {@link RequestMatcher} to determine if the request is an interactive login request or a + * API request, which should not be redirected to an interactive login page. By default, + * this entry point uses a {@link HttpHeaderInspectingApiRequestMatcher} but can be overridden using in the + * constructor. * * @author Scott Rossillo - * @version $Revision: 1 $ + * + * @see HttpHeaderInspectingApiRequestMatcher */ public class KeycloakAuthenticationEntryPoint implements AuthenticationEntryPoint { @@ -23,23 +31,62 @@ public class KeycloakAuthenticationEntryPoint implements AuthenticationEntryPoin * Default Keycloak authentication login URI */ public static final String DEFAULT_LOGIN_URI = "/sso/login"; + private static final String DEFAULT_REALM = "Unknown"; + private static final RequestMatcher DEFAULT_API_REQUEST_MATCHER = new HttpHeaderInspectingApiRequestMatcher(); private final static Logger log = LoggerFactory.getLogger(KeycloakAuthenticationEntryPoint.class); + private final RequestMatcher apiRequestMatcher; private String loginUri = DEFAULT_LOGIN_URI; + private String realm = DEFAULT_REALM; + + /** + * Creates a new Keycloak authentication entry point. + */ + public KeycloakAuthenticationEntryPoint() { + this(DEFAULT_API_REQUEST_MATCHER); + } + + /** + * Creates a new Keycloak authentication entry point using the given request + * matcher to determine if the current request is an API request or a browser request. + * + * @param apiRequestMatcher the RequestMatcher to use to determine + * if the current request is an API request or a browser request (required) + */ + public KeycloakAuthenticationEntryPoint(RequestMatcher apiRequestMatcher) { + Assert.notNull(apiRequestMatcher, "apiRequestMatcher required"); + this.apiRequestMatcher = apiRequestMatcher; + } @Override - public void commence(HttpServletRequest request, HttpServletResponse response, AuthenticationException authException) - throws IOException, ServletException { + public void commence(HttpServletRequest request, HttpServletResponse response, AuthenticationException authException) throws IOException, ServletException + { + if (apiRequestMatcher.matches(request)) { + commenceUnauthorizedResponse(request, response); + } else { + commenceLoginRedirect(request, response); + } + } + protected void commenceLoginRedirect(HttpServletRequest request, HttpServletResponse response) throws IOException { String contextAwareLoginUri = request.getContextPath() + loginUri; - log.debug("Redirecting to login URI {}", contextAwareLoginUri); response.sendRedirect(contextAwareLoginUri); } + protected void commenceUnauthorizedResponse(HttpServletRequest request, HttpServletResponse response) throws IOException { + response.addHeader(HttpHeaders.WWW_AUTHENTICATE, String.format("Bearer realm=\"%s\"", realm)); + response.sendError(HttpStatus.UNAUTHORIZED.value(), HttpStatus.UNAUTHORIZED.getReasonPhrase()); + } + public void setLoginUri(String loginUri) { Assert.notNull(loginUri, "loginUri cannot be null"); this.loginUri = loginUri; } + + public void setRealm(String realm) { + Assert.notNull(realm, "realm cannot be null"); + this.realm = realm; + } } diff --git a/integration/spring-security/src/test/java/org/keycloak/adapters/springsecurity/authentication/HttpHeaderInspectingApiRequestMatcherTest.java b/integration/spring-security/src/test/java/org/keycloak/adapters/springsecurity/authentication/HttpHeaderInspectingApiRequestMatcherTest.java new file mode 100644 index 0000000000..faa800d39e --- /dev/null +++ b/integration/spring-security/src/test/java/org/keycloak/adapters/springsecurity/authentication/HttpHeaderInspectingApiRequestMatcherTest.java @@ -0,0 +1,43 @@ +package org.keycloak.adapters.springsecurity.authentication; + +import org.apache.http.HttpHeaders; +import org.junit.Before; +import org.junit.Test; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.security.web.util.matcher.RequestMatcher; + +import static org.junit.Assert.*; + +/** + * HTTP header inspecting API request matcher tests. + */ +public class HttpHeaderInspectingApiRequestMatcherTest { + + private RequestMatcher apiRequestMatcher = new HttpHeaderInspectingApiRequestMatcher(); + private MockHttpServletRequest request; + + @Before + public void setUp() throws Exception { + request = new MockHttpServletRequest(); + } + + @Test + public void testMatches() throws Exception { + assertTrue(apiRequestMatcher.matches(request)); + } + + @Test + public void testMatchesBrowserRequest() throws Exception { + request.addHeader(HttpHeaders.ACCEPT, "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8"); + assertFalse(apiRequestMatcher.matches(request)); + } + + @Test + public void testMatchesRequestedWith() throws Exception { + request.addHeader( + HttpHeaderInspectingApiRequestMatcher.X_REQUESTED_WITH_HEADER, + HttpHeaderInspectingApiRequestMatcher.X_REQUESTED_WITH_HEADER_AJAX_VALUE); + assertTrue(apiRequestMatcher.matches(request)); + } + +} diff --git a/integration/spring-security/src/test/java/org/keycloak/adapters/springsecurity/authentication/KeycloakAuthenticationEntryPointTest.java b/integration/spring-security/src/test/java/org/keycloak/adapters/springsecurity/authentication/KeycloakAuthenticationEntryPointTest.java index b7204c32fa..f026015e58 100644 --- a/integration/spring-security/src/test/java/org/keycloak/adapters/springsecurity/authentication/KeycloakAuthenticationEntryPointTest.java +++ b/integration/spring-security/src/test/java/org/keycloak/adapters/springsecurity/authentication/KeycloakAuthenticationEntryPointTest.java @@ -1,5 +1,6 @@ package org.keycloak.adapters.springsecurity.authentication; +import org.apache.http.HttpHeaders; import org.junit.Before; import org.junit.Test; import org.springframework.http.HttpStatus; @@ -25,14 +26,16 @@ public class KeycloakAuthenticationEntryPointTest { } @Test - public void testCommence() throws Exception { + public void testCommenceWithRedirect() throws Exception { + configureBrowserRequest(); authenticationEntryPoint.commence(request, response, null); assertEquals(HttpStatus.FOUND.value(), response.getStatus()); assertEquals(KeycloakAuthenticationEntryPoint.DEFAULT_LOGIN_URI, response.getHeader("Location")); } @Test - public void testCommenceNotRootContext() throws Exception { + public void testCommenceWithRedirectNotRootContext() throws Exception { + configureBrowserRequest(); String contextPath = "/foo"; request.setContextPath(contextPath); authenticationEntryPoint.commence(request, response, null); @@ -40,12 +43,25 @@ public class KeycloakAuthenticationEntryPointTest { assertEquals(contextPath + KeycloakAuthenticationEntryPoint.DEFAULT_LOGIN_URI, response.getHeader("Location")); } + @Test + public void testCommenceWithUnauthorizedWithAccept() throws Exception { + request.addHeader(HttpHeaders.ACCEPT, "application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8"); + authenticationEntryPoint.commence(request, response, null); + assertEquals(HttpStatus.UNAUTHORIZED.value(), response.getStatus()); + assertNotNull(response.getHeader(HttpHeaders.WWW_AUTHENTICATE)); + } + @Test public void testSetLoginUri() throws Exception { + configureBrowserRequest(); final String logoutUri = "/foo"; authenticationEntryPoint.setLoginUri(logoutUri); authenticationEntryPoint.commence(request, response, null); assertEquals(HttpStatus.FOUND.value(), response.getStatus()); assertEquals(logoutUri, response.getHeader("Location")); } + + private void configureBrowserRequest() { + request.addHeader(HttpHeaders.ACCEPT, "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8"); + } }