From 3580dea399bba80fb391abba31cc787e81f9b4b1 Mon Sep 17 00:00:00 2001 From: emilienbondu Date: Wed, 11 Jan 2017 15:52:31 +0100 Subject: [PATCH] Fix https://issues.jboss.org/browse/KEYCLOAK-3492 --- .../KeycloakAuthenticationFailureHandler.java | 41 +++++++++++++ ...eycloakAuthenticationProcessingFilter.java | 59 +++++++++---------- ...oakAuthenticationProcessingFilterTest.java | 29 ++++++--- 3 files changed, 90 insertions(+), 39 deletions(-) create mode 100644 adapters/oidc/spring-security/src/main/java/org/keycloak/adapters/springsecurity/authentication/KeycloakAuthenticationFailureHandler.java diff --git a/adapters/oidc/spring-security/src/main/java/org/keycloak/adapters/springsecurity/authentication/KeycloakAuthenticationFailureHandler.java b/adapters/oidc/spring-security/src/main/java/org/keycloak/adapters/springsecurity/authentication/KeycloakAuthenticationFailureHandler.java new file mode 100644 index 0000000000..fcff678d5b --- /dev/null +++ b/adapters/oidc/spring-security/src/main/java/org/keycloak/adapters/springsecurity/authentication/KeycloakAuthenticationFailureHandler.java @@ -0,0 +1,41 @@ +/* + * Copyright 2016 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.keycloak.adapters.springsecurity.authentication; + +import java.io.IOException; + +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.springframework.security.core.AuthenticationException; +import org.springframework.security.web.authentication.AuthenticationFailureHandler; + +/** + * To return the forbidden code with the corresponding message. + * + * @author emilienbondu + * + */ +public class KeycloakAuthenticationFailureHandler implements AuthenticationFailureHandler { + + @Override + public void onAuthenticationFailure(HttpServletRequest request, HttpServletResponse response, AuthenticationException exception) throws IOException, ServletException { + response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Unable to authenticate using the Authorization header"); + } +} diff --git a/adapters/oidc/spring-security/src/main/java/org/keycloak/adapters/springsecurity/filter/KeycloakAuthenticationProcessingFilter.java b/adapters/oidc/spring-security/src/main/java/org/keycloak/adapters/springsecurity/filter/KeycloakAuthenticationProcessingFilter.java index f9fb19e54c..fabf30e62d 100644 --- a/adapters/oidc/spring-security/src/main/java/org/keycloak/adapters/springsecurity/filter/KeycloakAuthenticationProcessingFilter.java +++ b/adapters/oidc/spring-security/src/main/java/org/keycloak/adapters/springsecurity/filter/KeycloakAuthenticationProcessingFilter.java @@ -17,6 +17,13 @@ package org.keycloak.adapters.springsecurity.filter; +import java.io.IOException; + +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + import org.keycloak.adapters.AdapterDeploymentContext; import org.keycloak.adapters.AdapterTokenStore; import org.keycloak.adapters.KeycloakDeployment; @@ -25,7 +32,7 @@ import org.keycloak.adapters.spi.AuthChallenge; import org.keycloak.adapters.spi.AuthOutcome; import org.keycloak.adapters.spi.HttpFacade; import org.keycloak.adapters.springsecurity.KeycloakAuthenticationException; -import org.keycloak.adapters.springsecurity.authentication.KeycloakAuthenticationEntryPoint; +import org.keycloak.adapters.springsecurity.authentication.KeycloakAuthenticationFailureHandler; import org.keycloak.adapters.springsecurity.authentication.SpringSecurityRequestAuthenticator; import org.keycloak.adapters.springsecurity.facade.SimpleHttpFacade; import org.keycloak.adapters.springsecurity.token.AdapterTokenStoreFactory; @@ -41,19 +48,12 @@ import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.web.authentication.AbstractAuthenticationProcessingFilter; -import org.springframework.security.web.authentication.SimpleUrlAuthenticationFailureHandler; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.OrRequestMatcher; import org.springframework.security.web.util.matcher.RequestHeaderRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.util.Assert; -import javax.servlet.FilterChain; -import javax.servlet.ServletException; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import java.io.IOException; - /** * Provides a Keycloak authentication processing filter. * @@ -61,17 +61,15 @@ import java.io.IOException; * @version $Revision: 1 $ */ public class KeycloakAuthenticationProcessingFilter extends AbstractAuthenticationProcessingFilter implements ApplicationContextAware { - public static final String DEFAULT_LOGIN_URL = "/sso/login"; + public static final String AUTHORIZATION_HEADER = "Authorization"; public static final String SCHEME_BEARER = "bearer "; public static final String SCHEME_BASIC = "basic "; /** - * Request matcher that matches requests to the {@link KeycloakAuthenticationEntryPoint#DEFAULT_LOGIN_URI default login URI} - * and any request with a Authorization header. + * Request matcher that matches all requests. */ - public static final RequestMatcher DEFAULT_REQUEST_MATCHER = - new OrRequestMatcher(new AntPathRequestMatcher(DEFAULT_LOGIN_URL), new RequestHeaderRequestMatcher(AUTHORIZATION_HEADER)); + private static RequestMatcher DEFAULT_REQUEST_MATCHER = new AntPathRequestMatcher("/**"); private static final Logger log = LoggerFactory.getLogger(KeycloakAuthenticationProcessingFilter.class); @@ -89,7 +87,7 @@ public class KeycloakAuthenticationProcessingFilter extends AbstractAuthenticati */ public KeycloakAuthenticationProcessingFilter(AuthenticationManager authenticationManager) { this(authenticationManager, DEFAULT_REQUEST_MATCHER); - setAuthenticationFailureHandler(new SimpleUrlAuthenticationFailureHandler(DEFAULT_LOGIN_URL)); + setAuthenticationFailureHandler(new KeycloakAuthenticationFailureHandler()); } /** @@ -140,7 +138,18 @@ public class KeycloakAuthenticationProcessingFilter extends AbstractAuthenticati log.debug("Auth outcome: {}", result); if (AuthOutcome.FAILED.equals(result)) { - throw new KeycloakAuthenticationException("Auth outcome: " + result); + AuthChallenge challenge = authenticator.getChallenge(); + if (challenge != null) { + challenge.challenge(facade); + } + throw new KeycloakAuthenticationException("Invalid authorization header, see WWW-Authenticate header for details"); + } + if (AuthOutcome.NOT_ATTEMPTED.equals(result)) { + AuthChallenge challenge = authenticator.getChallenge(); + if (challenge != null) { + challenge.challenge(facade); + } + throw new KeycloakAuthenticationException("Authorization header not found, see WWW-Authenticate header"); } else if (AuthOutcome.AUTHENTICATED.equals(result)) { @@ -211,22 +220,10 @@ public class KeycloakAuthenticationProcessingFilter extends AbstractAuthenticati } @Override - protected void unsuccessfulAuthentication(HttpServletRequest request, HttpServletResponse response, - AuthenticationException failed) throws IOException, ServletException { - - if (this.isBearerTokenRequest(request)) { - SecurityContextHolder.clearContext(); - response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Unable to authenticate bearer token"); - return; - } - else if (this.isBasicAuthRequest(request)) { - SecurityContextHolder.clearContext(); - response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Unable to authenticate with basic authentication"); - return; - } - - super.unsuccessfulAuthentication(request, response, failed); - } + protected void unsuccessfulAuthentication(HttpServletRequest request, HttpServletResponse response, + AuthenticationException failed) throws IOException, ServletException { + super.unsuccessfulAuthentication(request, response, failed); + } @Override public void setApplicationContext(ApplicationContext applicationContext) throws BeansException { diff --git a/adapters/oidc/spring-security/src/test/java/org/keycloak/adapters/springsecurity/filter/KeycloakAuthenticationProcessingFilterTest.java b/adapters/oidc/spring-security/src/test/java/org/keycloak/adapters/springsecurity/filter/KeycloakAuthenticationProcessingFilterTest.java index 515de4b302..2414c38f24 100755 --- a/adapters/oidc/spring-security/src/test/java/org/keycloak/adapters/springsecurity/filter/KeycloakAuthenticationProcessingFilterTest.java +++ b/adapters/oidc/spring-security/src/test/java/org/keycloak/adapters/springsecurity/filter/KeycloakAuthenticationProcessingFilterTest.java @@ -27,6 +27,7 @@ import org.keycloak.adapters.OidcKeycloakAccount; import org.keycloak.adapters.spi.HttpFacade; import org.keycloak.adapters.springsecurity.KeycloakAuthenticationException; import org.keycloak.adapters.springsecurity.account.KeycloakRole; +import org.keycloak.adapters.springsecurity.authentication.KeycloakAuthenticationFailureHandler; import org.keycloak.adapters.springsecurity.token.KeycloakAuthenticationToken; import org.keycloak.common.enums.SslRequired; import org.keycloak.common.util.KeycloakUriBuilder; @@ -89,6 +90,8 @@ public class KeycloakAuthenticationProcessingFilterTest { @Mock private AuthenticationFailureHandler failureHandler; + + private KeycloakAuthenticationFailureHandler keycloakFailureHandler; @Mock private OidcKeycloakAccount keycloakAccount; @@ -106,6 +109,7 @@ public class KeycloakAuthenticationProcessingFilterTest { MockitoAnnotations.initMocks(this); request = spy(new MockHttpServletRequest()); filter = new KeycloakAuthenticationProcessingFilter(authenticationManager); + keycloakFailureHandler = new KeycloakAuthenticationFailureHandler(); filter.setApplicationContext(applicationContext); filter.setAuthenticationSuccessHandler(successHandler); @@ -155,10 +159,12 @@ public class KeycloakAuthenticationProcessingFilterTest { when(keycloakDeployment.getStateCookieName()).thenReturn("kc-cookie"); when(keycloakDeployment.getSslRequired()).thenReturn(SslRequired.NONE); when(keycloakDeployment.isBearerOnly()).thenReturn(Boolean.FALSE); - filter.attemptAuthentication(request, response); - - verify(response).setStatus(302); - verify(response).setHeader(eq("Location"), startsWith("http://localhost:8080/auth")); + try { + filter.attemptAuthentication(request, response); + } catch (KeycloakAuthenticationException e) { + verify(response).setStatus(302); + verify(response).setHeader(eq("Location"), startsWith("http://localhost:8080/auth")); + } } @Test(expected = KeycloakAuthenticationException.class) @@ -210,8 +216,7 @@ public class KeycloakAuthenticationProcessingFilterTest { AuthenticationException exception = new BadCredentialsException("OOPS"); this.setBearerAuthHeader(request); filter.unsuccessfulAuthentication(request, response, exception); - verify(response).sendError(eq(HttpServletResponse.SC_UNAUTHORIZED), anyString()); - verify(failureHandler, never()).onAuthenticationFailure(any(HttpServletRequest.class), any(HttpServletResponse.class), + verify(failureHandler).onAuthenticationFailure(any(HttpServletRequest.class), any(HttpServletResponse.class), any(AuthenticationException.class)); } @@ -220,10 +225,18 @@ public class KeycloakAuthenticationProcessingFilterTest { AuthenticationException exception = new BadCredentialsException("OOPS"); this.setBasicAuthHeader(request); filter.unsuccessfulAuthentication(request, response, exception); - verify(response).sendError(eq(HttpServletResponse.SC_UNAUTHORIZED), anyString()); - verify(failureHandler, never()).onAuthenticationFailure(any(HttpServletRequest.class), any(HttpServletResponse.class), + verify(failureHandler).onAuthenticationFailure(any(HttpServletRequest.class), any(HttpServletResponse.class), any(AuthenticationException.class)); } + + @Test + public void testDefaultFailureHanlder() throws Exception { + AuthenticationException exception = new BadCredentialsException("OOPS"); + filter.setAuthenticationFailureHandler(keycloakFailureHandler); + filter.unsuccessfulAuthentication(request, response, exception); + + verify(response).sendError(eq(HttpServletResponse.SC_UNAUTHORIZED), any(String.class)); + } @Test(expected = UnsupportedOperationException.class) public void testSetAllowSessionCreation() throws Exception {