Add AuthorizationFilter to filter chain validator

Closes gh-11327
This commit is contained in:
Josh Cummings 2022-07-07 12:51:52 -06:00
parent cdafa4ee21
commit d27d431bbc
No known key found for this signature in database
GPG Key ID: A306A51F43B8E5A5
2 changed files with 107 additions and 18 deletions

View File

@ -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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -18,10 +18,13 @@ package org.springframework.security.config.http;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collection; import java.util.Collection;
import java.util.Collections;
import java.util.Iterator; import java.util.Iterator;
import java.util.List; import java.util.List;
import java.util.function.Supplier;
import javax.servlet.Filter; import javax.servlet.Filter;
import javax.servlet.http.HttpServletRequest;
import org.apache.commons.logging.Log; import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory; import org.apache.commons.logging.LogFactory;
@ -29,11 +32,16 @@ import org.apache.commons.logging.LogFactory;
import org.springframework.security.access.AccessDeniedException; 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.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.DefaultSecurityFilterChain;
import org.springframework.security.web.FilterChainProxy; import org.springframework.security.web.FilterChainProxy;
import org.springframework.security.web.FilterInvocation; import org.springframework.security.web.FilterInvocation;
import org.springframework.security.web.SecurityFilterChain; import org.springframework.security.web.SecurityFilterChain;
import org.springframework.security.web.access.ExceptionTranslationFilter; 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.FilterInvocationSecurityMetadataSource;
import org.springframework.security.web.access.intercept.FilterSecurityInterceptor; import org.springframework.security.web.access.intercept.FilterSecurityInterceptor;
import org.springframework.security.web.authentication.AnonymousAuthenticationFilter; import org.springframework.security.web.authentication.AnonymousAuthenticationFilter;
@ -50,6 +58,8 @@ import org.springframework.security.web.util.matcher.RequestMatcher;
public class DefaultFilterChainValidator implements FilterChainProxy.FilterChainValidator { public class DefaultFilterChainValidator implements FilterChainProxy.FilterChainValidator {
private static final Authentication TEST = new TestingAuthenticationToken("", "", Collections.emptyList());
private final Log logger = LogFactory.getLog(getClass()); private final Log logger = LogFactory.getLog(getClass());
@Override @Override
@ -110,6 +120,7 @@ public class DefaultFilterChainValidator implements FilterChainProxy.FilterChain
checkForDuplicates(JaasApiIntegrationFilter.class, filters); checkForDuplicates(JaasApiIntegrationFilter.class, filters);
checkForDuplicates(ExceptionTranslationFilter.class, filters); checkForDuplicates(ExceptionTranslationFilter.class, filters);
checkForDuplicates(FilterSecurityInterceptor.class, filters); checkForDuplicates(FilterSecurityInterceptor.class, filters);
checkForDuplicates(AuthorizationFilter.class, filters);
} }
private void checkForDuplicates(Class<? extends Filter> clazz, List<Filter> filters) { private void checkForDuplicates(Class<? extends Filter> clazz, List<Filter> filters) {
@ -161,15 +172,7 @@ public class DefaultFilterChainValidator implements FilterChainProxy.FilterChain
this.logger.debug("Default generated login page is in use"); this.logger.debug("Default generated login page is in use");
return; return;
} }
FilterSecurityInterceptor authorizationInterceptor = getFilter(FilterSecurityInterceptor.class, filters); if (checkLoginPageIsPublic(filters, loginRequest)) {
FilterInvocationSecurityMetadataSource fids = authorizationInterceptor.getSecurityMetadataSource();
Collection<ConfigAttribute> 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; return;
} }
AnonymousAuthenticationFilter anonymous = getFilter(AnonymousAuthenticationFilter.class, filters); AnonymousAuthenticationFilter anonymous = getFilter(AnonymousAuthenticationFilter.class, filters);
@ -181,13 +184,14 @@ public class DefaultFilterChainValidator implements FilterChainProxy.FilterChain
// Simulate an anonymous access with the supplied attributes. // Simulate an anonymous access with the supplied attributes.
AnonymousAuthenticationToken token = new AnonymousAuthenticationToken("key", anonymous.getPrincipal(), AnonymousAuthenticationToken token = new AnonymousAuthenticationToken("key", anonymous.getPrincipal(),
anonymous.getAuthorities()); anonymous.getAuthorities());
Supplier<Boolean> check = deriveAnonymousCheck(filters, loginRequest, token);
try { try {
authorizationInterceptor.getAccessDecisionManager().decide(token, loginRequest, attributes); boolean allowed = check.get();
} if (!allowed) {
catch (AccessDeniedException ex) { this.logger.warn("Anonymous access to the login page doesn't appear to be enabled. "
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 "
+ "This is almost certainly an error. Please check your configuration allows unauthenticated " + "access to the configured login page. (Simulated access was rejected)");
+ "access to the configured login page. (Simulated access was rejected: " + ex + ")"); }
} }
catch (Exception ex) { catch (Exception ex) {
// May happen legitimately if a filter-chain request matcher requires more // May happen legitimately if a filter-chain request matcher requires more
@ -198,4 +202,62 @@ public class DefaultFilterChainValidator implements FilterChainProxy.FilterChain
} }
} }
private boolean checkLoginPageIsPublic(List<Filter> filters, FilterInvocation loginRequest) {
FilterSecurityInterceptor authorizationInterceptor = getFilter(FilterSecurityInterceptor.class, filters);
if (authorizationInterceptor != null) {
FilterInvocationSecurityMetadataSource fids = authorizationInterceptor.getSecurityMetadataSource();
Collection<ConfigAttribute> 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<HttpServletRequest> 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<Boolean> deriveAnonymousCheck(List<Filter> filters, FilterInvocation loginRequest,
AnonymousAuthenticationToken token) {
FilterSecurityInterceptor authorizationInterceptor = getFilter(FilterSecurityInterceptor.class, filters);
if (authorizationInterceptor != null) {
return () -> {
FilterInvocationSecurityMetadataSource source = authorizationInterceptor.getSecurityMetadataSource();
Collection<ConfigAttribute> 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<HttpServletRequest> authorizationManager = authorizationFilter
.getAuthorizationManager();
AuthorizationDecision decision = authorizationManager.check(() -> token, loginRequest.getHttpRequest());
return decision != null && decision.isGranted();
};
}
return () -> true;
}
} }

View File

@ -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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -18,6 +18,8 @@ package org.springframework.security.config.http;
import java.util.Collection; import java.util.Collection;
import javax.servlet.http.HttpServletRequest;
import org.apache.commons.logging.Log; import org.apache.commons.logging.Log;
import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
@ -26,11 +28,14 @@ import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension; import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.security.access.AccessDecisionManager; 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.core.Authentication;
import org.springframework.security.web.AuthenticationEntryPoint; import org.springframework.security.web.AuthenticationEntryPoint;
import org.springframework.security.web.DefaultSecurityFilterChain; import org.springframework.security.web.DefaultSecurityFilterChain;
import org.springframework.security.web.FilterChainProxy; import org.springframework.security.web.FilterChainProxy;
import org.springframework.security.web.access.ExceptionTranslationFilter; 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.DefaultFilterInvocationSecurityMetadataSource;
import org.springframework.security.web.access.intercept.FilterInvocationSecurityMetadataSource; import org.springframework.security.web.access.intercept.FilterInvocationSecurityMetadataSource;
import org.springframework.security.web.access.intercept.FilterSecurityInterceptor; import org.springframework.security.web.access.intercept.FilterSecurityInterceptor;
@ -41,7 +46,9 @@ import org.springframework.test.util.ReflectionTestUtils;
import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyObject; import static org.mockito.ArgumentMatchers.anyObject;
import static org.mockito.BDDMockito.given;
import static org.mockito.BDDMockito.willThrow; import static org.mockito.BDDMockito.willThrow;
import static org.mockito.Mockito.atLeastOnce;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verify;
@ -55,6 +62,8 @@ public class DefaultFilterChainValidatorTests {
private FilterChainProxy chain; private FilterChainProxy chain;
private FilterChainProxy chainAuthorizationFilter;
@Mock @Mock
private Log logger; private Log logger;
@ -66,17 +75,26 @@ public class DefaultFilterChainValidatorTests {
private FilterSecurityInterceptor authorizationInterceptor; private FilterSecurityInterceptor authorizationInterceptor;
@Mock
private AuthorizationManager<HttpServletRequest> authorizationManager;
private AuthorizationFilter authorizationFilter;
@BeforeEach @BeforeEach
public void setUp() { public void setUp() {
AnonymousAuthenticationFilter aaf = new AnonymousAuthenticationFilter("anonymous"); AnonymousAuthenticationFilter aaf = new AnonymousAuthenticationFilter("anonymous");
this.authorizationInterceptor = new FilterSecurityInterceptor(); this.authorizationInterceptor = new FilterSecurityInterceptor();
this.authorizationInterceptor.setAccessDecisionManager(this.accessDecisionManager); this.authorizationInterceptor.setAccessDecisionManager(this.accessDecisionManager);
this.authorizationInterceptor.setSecurityMetadataSource(this.metadataSource); this.authorizationInterceptor.setSecurityMetadataSource(this.metadataSource);
this.authorizationFilter = new AuthorizationFilter(this.authorizationManager);
AuthenticationEntryPoint authenticationEntryPoint = new LoginUrlAuthenticationEntryPoint("/login"); AuthenticationEntryPoint authenticationEntryPoint = new LoginUrlAuthenticationEntryPoint("/login");
ExceptionTranslationFilter etf = new ExceptionTranslationFilter(authenticationEntryPoint); ExceptionTranslationFilter etf = new ExceptionTranslationFilter(authenticationEntryPoint);
DefaultSecurityFilterChain securityChain = new DefaultSecurityFilterChain(AnyRequestMatcher.INSTANCE, aaf, etf, DefaultSecurityFilterChain securityChain = new DefaultSecurityFilterChain(AnyRequestMatcher.INSTANCE, aaf, etf,
this.authorizationInterceptor); this.authorizationInterceptor);
this.chain = new FilterChainProxy(securityChain); this.chain = new FilterChainProxy(securityChain);
DefaultSecurityFilterChain securityChainAuthorizationFilter = new DefaultSecurityFilterChain(
AnyRequestMatcher.INSTANCE, aaf, etf, this.authorizationFilter);
this.chainAuthorizationFilter = new FilterChainProxy(securityChainAuthorizationFilter);
this.validator = new DefaultFilterChainValidator(); this.validator = new DefaultFilterChainValidator();
ReflectionTestUtils.setField(this.validator, "logger", this.logger); ReflectionTestUtils.setField(this.validator, "logger", this.logger);
} }
@ -94,6 +112,15 @@ public class DefaultFilterChainValidatorTests {
toBeThrown); 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 // SEC-1957
@Test @Test
public void validateCustomMetadataSource() { public void validateCustomMetadataSource() {
@ -101,7 +128,7 @@ public class DefaultFilterChainValidatorTests {
FilterInvocationSecurityMetadataSource.class); FilterInvocationSecurityMetadataSource.class);
this.authorizationInterceptor.setSecurityMetadataSource(customMetaDataSource); this.authorizationInterceptor.setSecurityMetadataSource(customMetaDataSource);
this.validator.validate(this.chain); this.validator.validate(this.chain);
verify(customMetaDataSource).getAttributes(any()); verify(customMetaDataSource, atLeastOnce()).getAttributes(any());
} }
} }