From 91585f8563ebefb8df8a8d5763331d2733e4d69f Mon Sep 17 00:00:00 2001 From: emilienbondu Date: Mon, 12 Jun 2017 18:19:39 +0200 Subject: [PATCH] Changing request matcher to attempt auth on /sso/login or Auhtorization header Add default login URL. Throwing exception if login fails to enable auth entry point Adding a test for invalid token and bearer-only handle redirect correctly --- ...eycloakAuthenticationProcessingFilter.java | 39 ++++++++++++------- ...oakAuthenticationProcessingFilterTest.java | 17 +++++--- 2 files changed, 36 insertions(+), 20 deletions(-) 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 fabf30e62d..7e235ae520 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 @@ -61,15 +61,19 @@ import org.springframework.util.Assert; * @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 all requests. + * Request matcher that matches requests to the {@link KeycloakAuthenticationEntryPoint#DEFAULT_LOGIN_URI default login URI} + * and any request with a Authorization header. */ - private static RequestMatcher DEFAULT_REQUEST_MATCHER = new AntPathRequestMatcher("/**"); + public static final RequestMatcher DEFAULT_REQUEST_MATCHER = + new OrRequestMatcher(new AntPathRequestMatcher(DEFAULT_LOGIN_URL), new RequestHeaderRequestMatcher(AUTHORIZATION_HEADER)); private static final Logger log = LoggerFactory.getLogger(KeycloakAuthenticationProcessingFilter.class); @@ -107,7 +111,7 @@ public class KeycloakAuthenticationProcessingFilter extends AbstractAuthenticati * */ public KeycloakAuthenticationProcessingFilter(AuthenticationManager authenticationManager, RequestMatcher - requiresAuthenticationRequestMatcher) { + requiresAuthenticationRequestMatcher) { super(requiresAuthenticationRequestMatcher); Assert.notNull(authenticationManager, "authenticationManager cannot be null"); this.authenticationManager = authenticationManager; @@ -138,20 +142,27 @@ public class KeycloakAuthenticationProcessingFilter extends AbstractAuthenticati log.debug("Auth outcome: {}", result); if (AuthOutcome.FAILED.equals(result)) { - AuthChallenge challenge = authenticator.getChallenge(); + 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(); + AuthChallenge challenge = authenticator.getChallenge(); if (challenge != null) { challenge.challenge(facade); } - throw new KeycloakAuthenticationException("Authorization header not found, see WWW-Authenticate header"); + if (deployment.isBearerOnly()) { + // no redirection in this mode, throwing exception for the spring handler + throw new KeycloakAuthenticationException("Authorization header not found, see WWW-Authenticate header"); + } else { + // let continue if challenged, it may redirect + return null; + } } - + else if (AuthOutcome.AUTHENTICATED.equals(result)) { Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); Assert.notNull(authentication, "Authentication SecurityContextHolder was null"); @@ -193,7 +204,7 @@ public class KeycloakAuthenticationProcessingFilter extends AbstractAuthenticati @Override protected void successfulAuthentication(HttpServletRequest request, HttpServletResponse response, FilterChain chain, - Authentication authResult) throws IOException, ServletException { + Authentication authResult) throws IOException, ServletException { if (!(this.isBearerTokenRequest(request) || this.isBasicAuthRequest(request))) { super.successfulAuthentication(request, response, chain, authResult); @@ -220,10 +231,10 @@ public class KeycloakAuthenticationProcessingFilter extends AbstractAuthenticati } @Override - protected void unsuccessfulAuthentication(HttpServletRequest request, HttpServletResponse response, - AuthenticationException failed) throws IOException, ServletException { - 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 { @@ -259,4 +270,4 @@ public class KeycloakAuthenticationProcessingFilter extends AbstractAuthenticati public final void setContinueChainBeforeSuccessfulAuthentication(boolean continueChainBeforeSuccessfulAuthentication) { throw new UnsupportedOperationException("This filter does not support explicitly setting a continue chain before success policy"); } -} +} \ No newline at end of file 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 2414c38f24..a6a378a34c 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 @@ -159,12 +159,10 @@ public class KeycloakAuthenticationProcessingFilterTest { when(keycloakDeployment.getStateCookieName()).thenReturn("kc-cookie"); when(keycloakDeployment.getSslRequired()).thenReturn(SslRequired.NONE); when(keycloakDeployment.isBearerOnly()).thenReturn(Boolean.FALSE); - try { - filter.attemptAuthentication(request, response); - } catch (KeycloakAuthenticationException e) { - verify(response).setStatus(302); - verify(response).setHeader(eq("Location"), startsWith("http://localhost:8080/auth")); - } + + filter.attemptAuthentication(request, response); + verify(response).setStatus(302); + verify(response).setHeader(eq("Location"), startsWith("http://localhost:8080/auth")); } @Test(expected = KeycloakAuthenticationException.class) @@ -173,6 +171,13 @@ public class KeycloakAuthenticationProcessingFilterTest { filter.attemptAuthentication(request, response); } + @Test(expected = KeycloakAuthenticationException.class) + public void testAttemptAuthenticationWithInvalidTokenBearerOnly() throws Exception { + when(keycloakDeployment.isBearerOnly()).thenReturn(Boolean.TRUE); + request.addHeader("Authorization", "Bearer xxx"); + filter.attemptAuthentication(request, response); + } + @Test public void testSuccessfulAuthenticationInteractive() throws Exception { Authentication authentication = new KeycloakAuthenticationToken(keycloakAccount, authorities);