SEC-1699: Make sure a FilterInvocation is passed to the AccessDecisionManager when checking the login page access in DefaultFilterChainValidator.

This commit is contained in:
Luke Taylor 2011-04-14 18:04:29 +01:00
parent acf4b91a89
commit 8d702a4f98
2 changed files with 40 additions and 7 deletions

View File

@ -7,6 +7,7 @@ import javax.servlet.Filter;
import org.apache.commons.logging.Log; import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory; import org.apache.commons.logging.LogFactory;
import org.springframework.security.access.AccessDeniedException;
import org.springframework.security.access.ConfigAttribute; import org.springframework.security.access.ConfigAttribute;
import org.springframework.security.authentication.AnonymousAuthenticationToken; import org.springframework.security.authentication.AnonymousAuthenticationToken;
import org.springframework.security.web.FilterChainProxy; import org.springframework.security.web.FilterChainProxy;
@ -34,10 +35,11 @@ public class DefaultFilterChainValidator implements FilterChainProxy.FilterChain
} }
} }
private Object getFilter(Class<?> type, List<Filter> filters) { @SuppressWarnings({"unchecked"})
private <F extends Filter> F getFilter(Class<F> type, List<Filter> filters) {
for (Filter f : filters) { for (Filter f : filters) {
if (type.isAssignableFrom(f.getClass())) { 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 */ /* Checks for the common error of having a login page URL protected by the security interceptor */
private void checkLoginPageIsntProtected(FilterChainProxy fcp, List<Filter> filterStack) { private void checkLoginPageIsntProtected(FilterChainProxy fcp, List<Filter> filterStack) {
ExceptionTranslationFilter etf = (ExceptionTranslationFilter)getFilter(ExceptionTranslationFilter.class, filterStack); ExceptionTranslationFilter etf = getFilter(ExceptionTranslationFilter.class, filterStack);
if(etf == null || !(etf.getAuthenticationEntryPoint() instanceof LoginUrlAuthenticationEntryPoint)) { if(etf == null || !(etf.getAuthenticationEntryPoint() instanceof LoginUrlAuthenticationEntryPoint)) {
return; return;
@ -98,7 +100,7 @@ public class DefaultFilterChainValidator implements FilterChainProxy.FilterChain
return; return;
} }
FilterSecurityInterceptor fsi = (FilterSecurityInterceptor) getFilter(FilterSecurityInterceptor.class, filters); FilterSecurityInterceptor fsi = getFilter(FilterSecurityInterceptor.class, filters);
DefaultFilterInvocationSecurityMetadataSource fids = DefaultFilterInvocationSecurityMetadataSource fids =
(DefaultFilterInvocationSecurityMetadataSource) fsi.getSecurityMetadataSource(); (DefaultFilterInvocationSecurityMetadataSource) fsi.getSecurityMetadataSource();
@ -113,7 +115,7 @@ public class DefaultFilterChainValidator implements FilterChainProxy.FilterChain
return; return;
} }
AnonymousAuthenticationFilter anonPF = (AnonymousAuthenticationFilter) getFilter(AnonymousAuthenticationFilter.class, filters); AnonymousAuthenticationFilter anonPF = getFilter(AnonymousAuthenticationFilter.class, filters);
if (anonPF == null) { if (anonPF == null) {
logger.warn("The login page is being protected by the filter chain, but you don't appear to have" + 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."); " 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(), AnonymousAuthenticationToken token = new AnonymousAuthenticationToken("key", anonPF.getUserAttribute().getPassword(),
anonPF.getUserAttribute().getAuthorities()); anonPF.getUserAttribute().getAuthorities());
try { try {
fsi.getAccessDecisionManager().decide(token, new Object(), attributes); fsi.getAccessDecisionManager().decide(token, loginRequest, attributes);
} catch (Exception e) { } catch (AccessDeniedException e) {
logger.warn("Anonymous access to the login page doesn't appear to be enabled. This is almost certainly " + 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 " + "an error. Please check your configuration allows unauthenticated access to the configured " +
"login page. (Simulated access was rejected: " + e + ")"); "login page. (Simulated access was rejected: " + e + ")");

View File

@ -47,6 +47,9 @@ import org.springframework.security.web.authentication.logout.CookieClearingLogo
import org.springframework.security.web.firewall.DefaultHttpFirewall import org.springframework.security.web.firewall.DefaultHttpFirewall
import org.springframework.security.BeanNameCollectingPostProcessor import org.springframework.security.BeanNameCollectingPostProcessor
import org.springframework.security.authentication.dao.DaoAuthenticationProvider 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 { class MiscHttpConfigTests extends AbstractHttpConfigTests {
def 'Minimal configuration parses'() { def 'Minimal configuration parses'() {
@ -494,6 +497,17 @@ class MiscHttpConfigTests extends AbstractHttpConfigTests {
thrown(AccessDeniedException) 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() { def disablingUrlRewritingThroughTheNamespaceSetsCorrectPropertyOnContextRepo() {
xml.http('auto-config': 'true', 'disable-url-rewriting': 'true') xml.http('auto-config': 'true', 'disable-url-rewriting': 'true')
createAppContext() createAppContext()
@ -614,6 +628,23 @@ class MiscHttpConfigTests extends AbstractHttpConfigTests {
expect: expect:
fcp.firewall == appContext.getBean('fw') 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 { class MockEntryPoint extends LoginUrlAuthenticationEntryPoint {