From 8d702a4f98c138e946e7c7479e61fb47bee685c6 Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Thu, 14 Apr 2011 18:04:29 +0100 Subject: [PATCH] SEC-1699: Make sure a FilterInvocation is passed to the AccessDecisionManager when checking the login page access in DefaultFilterChainValidator. --- .../http/DefaultFilterChainValidator.java | 16 +++++----- .../config/http/MiscHttpConfigTests.groovy | 31 +++++++++++++++++++ 2 files changed, 40 insertions(+), 7 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 3bb5a29ab4..6f06695cba 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 @@ -7,6 +7,7 @@ import javax.servlet.Filter; 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.web.FilterChainProxy; @@ -34,10 +35,11 @@ public class DefaultFilterChainValidator implements FilterChainProxy.FilterChain } } - private Object getFilter(Class type, List filters) { + @SuppressWarnings({"unchecked"}) + private F getFilter(Class type, List filters) { for (Filter f : filters) { if (type.isAssignableFrom(f.getClass())) { - return f; + return (F) f; } } @@ -77,7 +79,7 @@ public class DefaultFilterChainValidator implements FilterChainProxy.FilterChain /* Checks for the common error of having a login page URL protected by the security interceptor */ private void checkLoginPageIsntProtected(FilterChainProxy fcp, List filterStack) { - ExceptionTranslationFilter etf = (ExceptionTranslationFilter)getFilter(ExceptionTranslationFilter.class, filterStack); + ExceptionTranslationFilter etf = getFilter(ExceptionTranslationFilter.class, filterStack); if(etf == null || !(etf.getAuthenticationEntryPoint() instanceof LoginUrlAuthenticationEntryPoint)) { return; @@ -98,7 +100,7 @@ public class DefaultFilterChainValidator implements FilterChainProxy.FilterChain return; } - FilterSecurityInterceptor fsi = (FilterSecurityInterceptor) getFilter(FilterSecurityInterceptor.class, filters); + FilterSecurityInterceptor fsi = getFilter(FilterSecurityInterceptor.class, filters); DefaultFilterInvocationSecurityMetadataSource fids = (DefaultFilterInvocationSecurityMetadataSource) fsi.getSecurityMetadataSource(); @@ -113,7 +115,7 @@ public class DefaultFilterChainValidator implements FilterChainProxy.FilterChain return; } - AnonymousAuthenticationFilter anonPF = (AnonymousAuthenticationFilter) getFilter(AnonymousAuthenticationFilter.class, filters); + AnonymousAuthenticationFilter anonPF = getFilter(AnonymousAuthenticationFilter.class, filters); if (anonPF == null) { logger.warn("The login page is being protected by the filter chain, but you don't appear to have" + " anonymous authentication enabled. This is almost certainly an error."); @@ -124,8 +126,8 @@ public class DefaultFilterChainValidator implements FilterChainProxy.FilterChain AnonymousAuthenticationToken token = new AnonymousAuthenticationToken("key", anonPF.getUserAttribute().getPassword(), anonPF.getUserAttribute().getAuthorities()); try { - fsi.getAccessDecisionManager().decide(token, new Object(), attributes); - } catch (Exception e) { + fsi.getAccessDecisionManager().decide(token, loginRequest, attributes); + } catch (AccessDeniedException e) { 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: " + e + ")"); diff --git a/config/src/test/groovy/org/springframework/security/config/http/MiscHttpConfigTests.groovy b/config/src/test/groovy/org/springframework/security/config/http/MiscHttpConfigTests.groovy index 8a606b56ff..b25c59adac 100644 --- a/config/src/test/groovy/org/springframework/security/config/http/MiscHttpConfigTests.groovy +++ b/config/src/test/groovy/org/springframework/security/config/http/MiscHttpConfigTests.groovy @@ -47,6 +47,9 @@ import org.springframework.security.web.authentication.logout.CookieClearingLogo import org.springframework.security.web.firewall.DefaultHttpFirewall import org.springframework.security.BeanNameCollectingPostProcessor import org.springframework.security.authentication.dao.DaoAuthenticationProvider +import org.springframework.security.access.vote.RoleVoter +import org.springframework.security.web.access.expression.WebExpressionVoter +import org.springframework.security.access.vote.AffirmativeBased class MiscHttpConfigTests extends AbstractHttpConfigTests { def 'Minimal configuration parses'() { @@ -494,6 +497,17 @@ class MiscHttpConfigTests extends AbstractHttpConfigTests { thrown(AccessDeniedException) } + def protectedLoginPageReportsWarning() { + when: + xml.http('use-expressions': 'true') { + 'form-login'('login-page': '/login') + interceptUrl('/login*', "hasRole('ROLE_A')") + } + createAppContext() + then: + notThrown(BeansException) + } + def disablingUrlRewritingThroughTheNamespaceSetsCorrectPropertyOnContextRepo() { xml.http('auto-config': 'true', 'disable-url-rewriting': 'true') createAppContext() @@ -614,6 +628,23 @@ class MiscHttpConfigTests extends AbstractHttpConfigTests { expect: fcp.firewall == appContext.getBean('fw') } + + def customAccessDecisionManagerIsSupported() { + xml.http('auto-config': 'true', 'access-decision-manager-ref': 'adm') + xml.'b:bean'(id: 'adm', 'class': AffirmativeBased.class.name) { + 'b:property'(name: 'decisionVoters') { + 'b:list'() { + 'b:bean'('class': RoleVoter.class.name) + 'b:bean'('class': RoleVoter.class.name) + 'b:bean'('class': RoleVoter.class.name) + 'b:bean'('class': WebExpressionVoter.class.name) + } + } + } + createAppContext() + expect: + getFilter(FilterSecurityInterceptor.class).accessDecisionManager.decisionVoters[3] instanceof WebExpressionVoter + } } class MockEntryPoint extends LoginUrlAuthenticationEntryPoint {