mirror of
https://github.com/spring-projects/spring-security.git
synced 2025-07-03 17:22:13 +00:00
SEC-795: Add check for protected login page when using namespace
http://jira.springframework.org/browse/SEC-795. I've added checks for the various scenarios which will result in a protected login page and suitable warning messages.
This commit is contained in:
parent
882509fb2a
commit
1090072fff
@ -16,8 +16,21 @@ import org.springframework.beans.factory.ListableBeanFactory;
|
|||||||
import org.springframework.beans.factory.config.BeanPostProcessor;
|
import org.springframework.beans.factory.config.BeanPostProcessor;
|
||||||
import org.springframework.core.OrderComparator;
|
import org.springframework.core.OrderComparator;
|
||||||
import org.springframework.core.Ordered;
|
import org.springframework.core.Ordered;
|
||||||
|
import org.springframework.security.ConfigAttributeDefinition;
|
||||||
import org.springframework.security.config.ConfigUtils.FilterChainList;
|
import org.springframework.security.config.ConfigUtils.FilterChainList;
|
||||||
|
import org.springframework.security.context.HttpSessionContextIntegrationFilter;
|
||||||
|
import org.springframework.security.intercept.web.DefaultFilterInvocationDefinitionSource;
|
||||||
|
import org.springframework.security.intercept.web.FilterSecurityInterceptor;
|
||||||
|
import org.springframework.security.providers.anonymous.AnonymousAuthenticationToken;
|
||||||
|
import org.springframework.security.providers.anonymous.AnonymousProcessingFilter;
|
||||||
|
import org.springframework.security.ui.ExceptionTranslationFilter;
|
||||||
|
import org.springframework.security.ui.SessionFixationProtectionFilter;
|
||||||
|
import org.springframework.security.ui.basicauth.BasicProcessingFilter;
|
||||||
|
import org.springframework.security.ui.webapp.AuthenticationProcessingFilter;
|
||||||
|
import org.springframework.security.ui.webapp.AuthenticationProcessingFilterEntryPoint;
|
||||||
|
import org.springframework.security.ui.webapp.DefaultLoginPageGeneratingFilter;
|
||||||
import org.springframework.security.util.FilterChainProxy;
|
import org.springframework.security.util.FilterChainProxy;
|
||||||
|
import org.springframework.security.wrapper.SecurityContextHolderAwareRequestFilter;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
*
|
*
|
||||||
@ -59,23 +72,114 @@ public class FilterChainProxyPostProcessor implements BeanPostProcessor, BeanFac
|
|||||||
}
|
}
|
||||||
|
|
||||||
logger.info("Filter chain...");
|
logger.info("Filter chain...");
|
||||||
for(int i=0; i < filters.size(); i++) {
|
for (int i=0; i < filters.size(); i++) {
|
||||||
// Remove the ordered wrapper from the filter and put it back in the chain at the same position.
|
// Remove the ordered wrapper from the filter and put it back in the chain at the same position.
|
||||||
Filter filter = unwrapFilter(filters.get(i));
|
Filter filter = unwrapFilter(filters.get(i));
|
||||||
logger.info("[" + i + "] - " + filter);
|
logger.info("[" + i + "] - " + filter);
|
||||||
filters.set(i, filter);
|
filters.set(i, filter);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
checkFilterStack(filters);
|
||||||
|
|
||||||
// Note that this returns a copy
|
// Note that this returns a copy
|
||||||
Map filterMap = filterChainProxy.getFilterChainMap();
|
Map filterMap = filterChainProxy.getFilterChainMap();
|
||||||
filterMap.put(filterChainProxy.getMatcher().getUniversalMatchPattern(), filters);
|
filterMap.put(filterChainProxy.getMatcher().getUniversalMatchPattern(), filters);
|
||||||
filterChainProxy.setFilterChainMap(filterMap);
|
filterChainProxy.setFilterChainMap(filterMap);
|
||||||
|
|
||||||
logger.info("FilterChainProxy: " + filterChainProxy);
|
checkLoginPageIsntProtected(filterChainProxy);
|
||||||
|
|
||||||
|
logger.info("FilterChainProxy: " + filterChainProxy);
|
||||||
|
|
||||||
return bean;
|
return bean;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Checks the filter list for possible errors and logs them
|
||||||
|
*/
|
||||||
|
private void checkFilterStack(List filters) {
|
||||||
|
checkForDuplicates(HttpSessionContextIntegrationFilter.class, filters);
|
||||||
|
checkForDuplicates(AuthenticationProcessingFilter.class, filters);
|
||||||
|
checkForDuplicates(SessionFixationProtectionFilter.class, filters);
|
||||||
|
checkForDuplicates(BasicProcessingFilter.class, filters);
|
||||||
|
checkForDuplicates(SecurityContextHolderAwareRequestFilter.class, filters);
|
||||||
|
checkForDuplicates(ExceptionTranslationFilter.class, filters);
|
||||||
|
checkForDuplicates(FilterSecurityInterceptor.class, filters);
|
||||||
|
}
|
||||||
|
|
||||||
|
private void checkForDuplicates(Class clazz, List filters) {
|
||||||
|
for (int i=0; i < filters.size(); i++) {
|
||||||
|
Filter f1 = (Filter)filters.get(i);
|
||||||
|
if (clazz.isAssignableFrom(f1.getClass())) {
|
||||||
|
// Found the first one, check remaining for another
|
||||||
|
for (int j=i+1; j < filters.size(); j++) {
|
||||||
|
Filter f2 = (Filter)filters.get(j);
|
||||||
|
if (clazz.isAssignableFrom(f2.getClass())) {
|
||||||
|
logger.warn("Possible error: Filters at position " + i + " and " + j + " are both " +
|
||||||
|
"instances of " + clazz.getName());
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/* Checks for the common error of having a login page URL protected by the security interceptor */
|
||||||
|
private void checkLoginPageIsntProtected(FilterChainProxy fcp) {
|
||||||
|
ExceptionTranslationFilter etf = (ExceptionTranslationFilter) beanFactory.getBean(BeanIds.EXCEPTION_TRANSLATION_FILTER);
|
||||||
|
|
||||||
|
if (etf.getAuthenticationEntryPoint() instanceof AuthenticationProcessingFilterEntryPoint) {
|
||||||
|
String loginPage =
|
||||||
|
((AuthenticationProcessingFilterEntryPoint)etf.getAuthenticationEntryPoint()).getLoginFormUrl();
|
||||||
|
List filters = fcp.getFilters(loginPage);
|
||||||
|
logger.info("Checking whether login URL '" + loginPage + "' is accessible with your configuration");
|
||||||
|
|
||||||
|
if (filters == null || filters.isEmpty()) {
|
||||||
|
logger.debug("Filter chain is empty for the login page");
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (loginPage.equals(DefaultLoginPageGeneratingFilter.DEFAULT_LOGIN_PAGE_URL) &&
|
||||||
|
beanFactory.containsBean(BeanIds.DEFAULT_LOGIN_PAGE_GENERATING_FILTER)) {
|
||||||
|
logger.debug("Default generated login page is in use");
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
FilterSecurityInterceptor fsi =
|
||||||
|
((FilterSecurityInterceptor)beanFactory.getBean(BeanIds.FILTER_SECURITY_INTERCEPTOR));
|
||||||
|
DefaultFilterInvocationDefinitionSource fids =
|
||||||
|
(DefaultFilterInvocationDefinitionSource) fsi.getObjectDefinitionSource();
|
||||||
|
ConfigAttributeDefinition cad = fids.lookupAttributes(loginPage, "POST");
|
||||||
|
|
||||||
|
if (cad == null) {
|
||||||
|
logger.debug("No access attributes defined for login page URL");
|
||||||
|
if (fsi.isRejectPublicInvocations()) {
|
||||||
|
logger.warn("FilterSecurityInterceptor is configured to reject public invocations." +
|
||||||
|
" Your login page may not be accessible.");
|
||||||
|
}
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!beanFactory.containsBean(BeanIds.ANONYMOUS_PROCESSING_FILTER)) {
|
||||||
|
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.");
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Simulate an anonymous access with the supplied attributes.
|
||||||
|
AnonymousProcessingFilter anonPF = (AnonymousProcessingFilter) beanFactory.getBean(BeanIds.ANONYMOUS_PROCESSING_FILTER);
|
||||||
|
AnonymousAuthenticationToken token =
|
||||||
|
new AnonymousAuthenticationToken("key", anonPF.getUserAttribute().getPassword(),
|
||||||
|
anonPF.getUserAttribute().getAuthorities());
|
||||||
|
try {
|
||||||
|
fsi.getAccessDecisionManager().decide(token, new Object(), cad);
|
||||||
|
} catch (Exception 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 + ")");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Returns the delegate filter of a wrapper, or the unchanged filter if it isn't wrapped.
|
* Returns the delegate filter of a wrapper, or the unchanged filter if it isn't wrapped.
|
||||||
*/
|
*/
|
||||||
|
@ -179,7 +179,7 @@ public class DefaultFilterInvocationDefinitionSource implements FilterInvocation
|
|||||||
* @return the <code>ConfigAttributeDefinition</code> that applies to the specified <code>FilterInvocation</code>
|
* @return the <code>ConfigAttributeDefinition</code> that applies to the specified <code>FilterInvocation</code>
|
||||||
* or null if no match is foud
|
* or null if no match is foud
|
||||||
*/
|
*/
|
||||||
protected ConfigAttributeDefinition lookupAttributes(String url, String method) {
|
public ConfigAttributeDefinition lookupAttributes(String url, String method) {
|
||||||
if (stripQueryStringFromUrls) {
|
if (stripQueryStringFromUrls) {
|
||||||
// Strip anything after a question mark symbol, as per SEC-161. See also SEC-321
|
// Strip anything after a question mark symbol, as per SEC-161. See also SEC-321
|
||||||
int firstQuestionMarkIndex = url.indexOf("?");
|
int firstQuestionMarkIndex = url.indexOf("?");
|
||||||
|
@ -180,7 +180,7 @@ public class FilterChainProxy implements Filter, InitializingBean, ApplicationCo
|
|||||||
* @param url the request URL
|
* @param url the request URL
|
||||||
* @return an ordered array of Filters defining the filter chain
|
* @return an ordered array of Filters defining the filter chain
|
||||||
*/
|
*/
|
||||||
List getFilters(String url) {
|
public List getFilters(String url) {
|
||||||
Iterator filterChains = filterChainMap.entrySet().iterator();
|
Iterator filterChains = filterChainMap.entrySet().iterator();
|
||||||
|
|
||||||
while (filterChains.hasNext()) {
|
while (filterChains.hasNext()) {
|
||||||
|
@ -8,6 +8,7 @@ import java.util.List;
|
|||||||
|
|
||||||
import org.junit.After;
|
import org.junit.After;
|
||||||
import org.junit.Test;
|
import org.junit.Test;
|
||||||
|
import org.springframework.beans.factory.BeanCreationException;
|
||||||
import org.springframework.beans.factory.parsing.BeanDefinitionParsingException;
|
import org.springframework.beans.factory.parsing.BeanDefinitionParsingException;
|
||||||
import org.springframework.context.support.AbstractXmlApplicationContext;
|
import org.springframework.context.support.AbstractXmlApplicationContext;
|
||||||
import org.springframework.mock.web.MockHttpServletRequest;
|
import org.springframework.mock.web.MockHttpServletRequest;
|
||||||
@ -282,6 +283,15 @@ public class HttpSecurityBeanDefinitionParserTests {
|
|||||||
assertTrue(filters.get(1) instanceof SecurityContextHolderAwareRequestFilter);
|
assertTrue(filters.get(1) instanceof SecurityContextHolderAwareRequestFilter);
|
||||||
assertTrue(filters.get(5) instanceof SecurityContextHolderAwareRequestFilter);
|
assertTrue(filters.get(5) instanceof SecurityContextHolderAwareRequestFilter);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test(expected=BeanCreationException.class)
|
||||||
|
public void twoFiltersWithSameOrderAreRejected() {
|
||||||
|
setContext(
|
||||||
|
"<http auto-config='true'/>" + AUTH_PROVIDER_XML +
|
||||||
|
"<b:bean id='userFilter' class='org.springframework.security.wrapper.SecurityContextHolderAwareRequestFilter'>" +
|
||||||
|
" <custom-filter position='LOGOUT_FILTER'/>" +
|
||||||
|
"</b:bean>");
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void rememberMeServiceWorksWithTokenRepoRef() {
|
public void rememberMeServiceWorksWithTokenRepoRef() {
|
||||||
@ -414,6 +424,50 @@ public class HttpSecurityBeanDefinitionParserTests {
|
|||||||
assertEquals("Hello from the post processor!", service.getPostProcessorWasHere());
|
assertEquals("Hello from the post processor!", service.getPostProcessorWasHere());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* SEC-795. Two methods that exercise the scenarios that will or won't result in a protected login page warning.
|
||||||
|
* Check the log.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
public void unprotectedLoginPageDoesntResultInWarning() {
|
||||||
|
// Anonymous access configured
|
||||||
|
setContext(
|
||||||
|
" <http>" +
|
||||||
|
" <intercept-url pattern='/login.jsp*' access='IS_AUTHENTICATED_ANONYMOUSLY'/>" +
|
||||||
|
" <intercept-url pattern='/**' access='ROLE_A'/>" +
|
||||||
|
" <anonymous />" +
|
||||||
|
" <form-login login-page='/login.jsp' default-target-url='/messageList.html'/>" +
|
||||||
|
" </http>" + AUTH_PROVIDER_XML);
|
||||||
|
closeAppContext();
|
||||||
|
// No filters applied to login page
|
||||||
|
setContext(
|
||||||
|
" <http>" +
|
||||||
|
" <intercept-url pattern='/login.jsp*' filters='none'/>" +
|
||||||
|
" <intercept-url pattern='/**' access='ROLE_A'/>" +
|
||||||
|
" <anonymous />" +
|
||||||
|
" <form-login login-page='/login.jsp' default-target-url='/messageList.html'/>" +
|
||||||
|
" </http>" + AUTH_PROVIDER_XML);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void protectedLoginPageResultsInWarning() {
|
||||||
|
// Protected, no anonymous filter configured.
|
||||||
|
setContext(
|
||||||
|
" <http>" +
|
||||||
|
" <intercept-url pattern='/**' access='ROLE_A'/>" +
|
||||||
|
" <form-login login-page='/login.jsp' default-target-url='/messageList.html'/>" +
|
||||||
|
" </http>" + AUTH_PROVIDER_XML);
|
||||||
|
closeAppContext();
|
||||||
|
// Protected, anonymous provider but no access
|
||||||
|
setContext(
|
||||||
|
" <http>" +
|
||||||
|
" <intercept-url pattern='/**' access='ROLE_A'/>" +
|
||||||
|
" <anonymous />" +
|
||||||
|
" <form-login login-page='/login.jsp' default-target-url='/messageList.html'/>" +
|
||||||
|
" </http>" + AUTH_PROVIDER_XML);
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
private void setContext(String context) {
|
private void setContext(String context) {
|
||||||
appContext = new InMemoryXmlApplicationContext(context);
|
appContext = new InMemoryXmlApplicationContext(context);
|
||||||
}
|
}
|
||||||
|
Loading…
x
Reference in New Issue
Block a user