Merge pull request #4227 from sebastienblanc/KEYCLOAK-3492-rebased

KEYCLOAK-3492 : Changing request matcher to attempt auth on /sso/login or Auhtorizati…

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 <code>Authorization</code> 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,
+    protected void unsuccessfulAuthentication(HttpServletRequest request, HttpServletResponse response,
-			AuthenticationException failed) throws IOException, ServletException {
+                                              AuthenticationException failed) throws IOException, ServletException {
-		super.unsuccessfulAuthentication(request, response, failed);
+        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");
     }
-}
+}

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);
+        filter.attemptAuthentication(request, response);
-        } catch (KeycloakAuthenticationException e) {
+        verify(response).setStatus(302);
-        	verify(response).setStatus(302);
+        verify(response).setHeader(eq("Location"), startsWith("http://localhost:8080/auth"));
-            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);