From 01ffc93062a64cd06e09d2ba16354908252978e5 Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Thu, 7 Jul 2022 12:51:52 -0600 Subject: [PATCH] Add AuthorizationFilter to filter chain validator Closes gh-11327 --- .../http/DefaultFilterChainValidator.java | 94 +++++++++++++++---- .../DefaultFilterChainValidatorTests.java | 30 +++++- 2 files changed, 106 insertions(+), 18 deletions(-) diff --git a/config/src/main/java/org/springframework/security/config/http/DefaultFilterChainValidator.java b/config/src/main/java/org/springframework/security/config/http/DefaultFilterChainValidator.java index 01a09ba49f..3362909916 100644 --- a/config/src/main/java/org/springframework/security/config/http/DefaultFilterChainValidator.java +++ b/config/src/main/java/org/springframework/security/config/http/DefaultFilterChainValidator.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,21 +18,29 @@ package org.springframework.security.config.http; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.Iterator; import java.util.List; +import java.util.function.Supplier; import jakarta.servlet.Filter; +import jakarta.servlet.http.HttpServletRequest; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.security.access.AccessDeniedException; import org.springframework.security.access.ConfigAttribute; import org.springframework.security.authentication.AnonymousAuthenticationToken; +import org.springframework.security.authentication.TestingAuthenticationToken; +import org.springframework.security.authorization.AuthorizationDecision; +import org.springframework.security.authorization.AuthorizationManager; +import org.springframework.security.core.Authentication; import org.springframework.security.web.DefaultSecurityFilterChain; import org.springframework.security.web.FilterChainProxy; import org.springframework.security.web.FilterInvocation; import org.springframework.security.web.SecurityFilterChain; import org.springframework.security.web.access.ExceptionTranslationFilter; +import org.springframework.security.web.access.intercept.AuthorizationFilter; import org.springframework.security.web.access.intercept.FilterInvocationSecurityMetadataSource; import org.springframework.security.web.access.intercept.FilterSecurityInterceptor; import org.springframework.security.web.authentication.AnonymousAuthenticationFilter; @@ -49,6 +57,8 @@ import org.springframework.security.web.util.matcher.RequestMatcher; public class DefaultFilterChainValidator implements FilterChainProxy.FilterChainValidator { + private static final Authentication TEST = new TestingAuthenticationToken("", "", Collections.emptyList()); + private final Log logger = LogFactory.getLog(getClass()); @Override @@ -109,6 +119,7 @@ public class DefaultFilterChainValidator implements FilterChainProxy.FilterChain checkForDuplicates(JaasApiIntegrationFilter.class, filters); checkForDuplicates(ExceptionTranslationFilter.class, filters); checkForDuplicates(FilterSecurityInterceptor.class, filters); + checkForDuplicates(AuthorizationFilter.class, filters); } private void checkForDuplicates(Class clazz, List filters) { @@ -160,15 +171,7 @@ public class DefaultFilterChainValidator implements FilterChainProxy.FilterChain this.logger.debug("Default generated login page is in use"); return; } - FilterSecurityInterceptor authorizationInterceptor = getFilter(FilterSecurityInterceptor.class, filters); - FilterInvocationSecurityMetadataSource fids = authorizationInterceptor.getSecurityMetadataSource(); - Collection attributes = fids.getAttributes(loginRequest); - if (attributes == null) { - this.logger.debug("No access attributes defined for login page URL"); - if (authorizationInterceptor.isRejectPublicInvocations()) { - this.logger.warn("FilterSecurityInterceptor is configured to reject public invocations." - + " Your login page may not be accessible."); - } + if (checkLoginPageIsPublic(filters, loginRequest)) { return; } AnonymousAuthenticationFilter anonymous = getFilter(AnonymousAuthenticationFilter.class, filters); @@ -180,13 +183,14 @@ public class DefaultFilterChainValidator implements FilterChainProxy.FilterChain // Simulate an anonymous access with the supplied attributes. AnonymousAuthenticationToken token = new AnonymousAuthenticationToken("key", anonymous.getPrincipal(), anonymous.getAuthorities()); + Supplier check = deriveAnonymousCheck(filters, loginRequest, token); try { - authorizationInterceptor.getAccessDecisionManager().decide(token, loginRequest, attributes); - } - catch (AccessDeniedException ex) { - this.logger.warn("Anonymous access to the login page doesn't appear to be enabled. " - + "This is almost certainly an error. Please check your configuration allows unauthenticated " - + "access to the configured login page. (Simulated access was rejected: " + ex + ")"); + boolean allowed = check.get(); + if (!allowed) { + this.logger.warn("Anonymous access to the login page doesn't appear to be enabled. " + + "This is almost certainly an error. Please check your configuration allows unauthenticated " + + "access to the configured login page. (Simulated access was rejected)"); + } } catch (Exception ex) { // May happen legitimately if a filter-chain request matcher requires more @@ -197,4 +201,62 @@ public class DefaultFilterChainValidator implements FilterChainProxy.FilterChain } } + private boolean checkLoginPageIsPublic(List filters, FilterInvocation loginRequest) { + FilterSecurityInterceptor authorizationInterceptor = getFilter(FilterSecurityInterceptor.class, filters); + if (authorizationInterceptor != null) { + FilterInvocationSecurityMetadataSource fids = authorizationInterceptor.getSecurityMetadataSource(); + Collection attributes = fids.getAttributes(loginRequest); + if (attributes == null) { + this.logger.debug("No access attributes defined for login page URL"); + if (authorizationInterceptor.isRejectPublicInvocations()) { + this.logger.warn("FilterSecurityInterceptor is configured to reject public invocations." + + " Your login page may not be accessible."); + } + return true; + } + return false; + } + AuthorizationFilter authorizationFilter = getFilter(AuthorizationFilter.class, filters); + if (authorizationFilter != null) { + AuthorizationManager authorizationManager = authorizationFilter + .getAuthorizationManager(); + try { + AuthorizationDecision decision = authorizationManager.check(() -> TEST, loginRequest.getHttpRequest()); + return decision != null && decision.isGranted(); + } + catch (Exception ex) { + return false; + } + } + return false; + } + + private Supplier deriveAnonymousCheck(List filters, FilterInvocation loginRequest, + AnonymousAuthenticationToken token) { + FilterSecurityInterceptor authorizationInterceptor = getFilter(FilterSecurityInterceptor.class, filters); + if (authorizationInterceptor != null) { + return () -> { + FilterInvocationSecurityMetadataSource source = authorizationInterceptor.getSecurityMetadataSource(); + Collection attributes = source.getAttributes(loginRequest); + try { + authorizationInterceptor.getAccessDecisionManager().decide(token, loginRequest, attributes); + return true; + } + catch (AccessDeniedException ex) { + return false; + } + }; + } + AuthorizationFilter authorizationFilter = getFilter(AuthorizationFilter.class, filters); + if (authorizationFilter != null) { + return () -> { + AuthorizationManager authorizationManager = authorizationFilter + .getAuthorizationManager(); + AuthorizationDecision decision = authorizationManager.check(() -> token, loginRequest.getHttpRequest()); + return decision != null && decision.isGranted(); + }; + } + return () -> true; + } + } diff --git a/config/src/test/java/org/springframework/security/config/http/DefaultFilterChainValidatorTests.java b/config/src/test/java/org/springframework/security/config/http/DefaultFilterChainValidatorTests.java index 4c17673c0a..e84421a2b4 100644 --- a/config/src/test/java/org/springframework/security/config/http/DefaultFilterChainValidatorTests.java +++ b/config/src/test/java/org/springframework/security/config/http/DefaultFilterChainValidatorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,6 +18,7 @@ package org.springframework.security.config.http; import java.util.Collection; +import jakarta.servlet.http.HttpServletRequest; import org.apache.commons.logging.Log; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -26,11 +27,14 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.security.access.AccessDecisionManager; +import org.springframework.security.authorization.AuthorizationDecision; +import org.springframework.security.authorization.AuthorizationManager; import org.springframework.security.core.Authentication; import org.springframework.security.web.AuthenticationEntryPoint; import org.springframework.security.web.DefaultSecurityFilterChain; import org.springframework.security.web.FilterChainProxy; import org.springframework.security.web.access.ExceptionTranslationFilter; +import org.springframework.security.web.access.intercept.AuthorizationFilter; import org.springframework.security.web.access.intercept.DefaultFilterInvocationSecurityMetadataSource; import org.springframework.security.web.access.intercept.FilterInvocationSecurityMetadataSource; import org.springframework.security.web.access.intercept.FilterSecurityInterceptor; @@ -41,7 +45,9 @@ import org.springframework.test.util.ReflectionTestUtils; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyObject; +import static org.mockito.BDDMockito.given; import static org.mockito.BDDMockito.willThrow; +import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -55,6 +61,8 @@ public class DefaultFilterChainValidatorTests { private FilterChainProxy chain; + private FilterChainProxy chainAuthorizationFilter; + @Mock private Log logger; @@ -66,17 +74,26 @@ public class DefaultFilterChainValidatorTests { private FilterSecurityInterceptor authorizationInterceptor; + @Mock + private AuthorizationManager authorizationManager; + + private AuthorizationFilter authorizationFilter; + @BeforeEach public void setUp() { AnonymousAuthenticationFilter aaf = new AnonymousAuthenticationFilter("anonymous"); this.authorizationInterceptor = new FilterSecurityInterceptor(); this.authorizationInterceptor.setAccessDecisionManager(this.accessDecisionManager); this.authorizationInterceptor.setSecurityMetadataSource(this.metadataSource); + this.authorizationFilter = new AuthorizationFilter(this.authorizationManager); AuthenticationEntryPoint authenticationEntryPoint = new LoginUrlAuthenticationEntryPoint("/login"); ExceptionTranslationFilter etf = new ExceptionTranslationFilter(authenticationEntryPoint); DefaultSecurityFilterChain securityChain = new DefaultSecurityFilterChain(AnyRequestMatcher.INSTANCE, aaf, etf, this.authorizationInterceptor); this.chain = new FilterChainProxy(securityChain); + DefaultSecurityFilterChain securityChainAuthorizationFilter = new DefaultSecurityFilterChain( + AnyRequestMatcher.INSTANCE, aaf, etf, this.authorizationFilter); + this.chainAuthorizationFilter = new FilterChainProxy(securityChainAuthorizationFilter); this.validator = new DefaultFilterChainValidator(); ReflectionTestUtils.setField(this.validator, "logger", this.logger); } @@ -94,6 +111,15 @@ public class DefaultFilterChainValidatorTests { toBeThrown); } + @Test + public void validateCheckLoginPageAllowsAnonymous() { + given(this.authorizationManager.check(any(), any())).willReturn(new AuthorizationDecision(false)); + this.validator.validate(this.chainAuthorizationFilter); + verify(this.logger).warn("Anonymous access to the login page doesn't appear to be enabled. " + + "This is almost certainly an error. Please check your configuration allows unauthenticated " + + "access to the configured login page. (Simulated access was rejected)"); + } + // SEC-1957 @Test public void validateCustomMetadataSource() { @@ -101,7 +127,7 @@ public class DefaultFilterChainValidatorTests { FilterInvocationSecurityMetadataSource.class); this.authorizationInterceptor.setSecurityMetadataSource(customMetaDataSource); this.validator.validate(this.chain); - verify(customMetaDataSource).getAttributes(any()); + verify(customMetaDataSource, atLeastOnce()).getAttributes(any()); } }