Cwikius-Spring/spring-security
1
0
Fork 0
mirror of https://github.com/spring-projects/spring-security.git synced 2025-03-06 05:19:15 +00:00
Code Issues Packages Projects Releases Wiki Activity

SEC-818: Changed redirect URL validation to ignore potential property placeholders at parsing time and report a warning through the parser context rather than an error. Also validated the URLs in the beans themselves using Asserts, so an exception will occur later when the beans have been created rather than while assembling the bean definitions.

Browse Source
This commit is contained in:
Luke Taylor 2008-05-07 13:49:20 +00:00
parent 5cf0c84e2f
commit 8ad2d681ab
7 changed files with 26 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
core/src/main/java/org/springframework/security/concurrent/ConcurrentSessionFilter.java
View File

@ -21,6 +21,7 @@ import org.springframework.security.ui.FilterChainOrder;
import org.springframework.security.ui.SpringSecurityFilter;
import org.springframework.security.ui.logout.LogoutHandler;
import org.springframework.security.ui.logout.SecurityContextLogoutHandler;
import org.springframework.security.util.UrlUtils;
import org.springframework.beans.factory.InitializingBean;
import org.springframework.util.Assert;
@ -60,6 +61,7 @@ public class ConcurrentSessionFilter extends SpringSecurityFilter implements Ini
public void afterPropertiesSet() throws Exception {
Assert.notNull(sessionRegistry, "SessionRegistry required");
Assert.isTrue(UrlUtils.isValidRedirectUrl(expiredUrl), expiredUrl + " isn't a valid redirect URL");
}
public void doFilterHttp(HttpServletRequest request, HttpServletResponse response, FilterChain chain)

7
core/src/main/java/org/springframework/security/config/ConfigUtils.java
View File

@ -16,6 +16,7 @@ import org.springframework.beans.factory.xml.ParserContext;
import org.springframework.security.afterinvocation.AfterInvocationProviderManager;
import org.springframework.security.providers.ProviderManager;
import org.springframework.security.userdetails.UserDetailsService;
import org.springframework.security.util.UrlUtils;
import org.springframework.security.vote.AffirmativeBased;
import org.springframework.security.vote.AuthenticatedVoter;
import org.springframework.security.vote.RoleVoter;
@ -180,12 +181,12 @@ public abstract class ConfigUtils {
/**
* Checks the value of an XML attribute which represents a redirect URL.
* If not empty or starting with "/" or "http" it will raise an error.
* If not empty or starting with "$" (potential placeholder), "/" or "http" it will raise an error.
*/
static void validateHttpRedirect(String url, ParserContext pc, Object source) {
if (!StringUtils.hasText(url) || url.startsWith("/") || url.toLowerCase().startsWith("http")) {
if (UrlUtils.isValidRedirectUrl(url) || url.startsWith("$")) {
return;
}
pc.getReaderContext().error(url + " is not a valid redirect URL (must start with '/' or http(s))", source);
pc.getReaderContext().warning(url + " is not a valid redirect URL (must start with '/' or http(s))", source);
}
}

8
core/src/main/java/org/springframework/security/ui/AbstractProcessingFilter.java
View File

@ -21,6 +21,7 @@ import org.springframework.security.AuthenticationException;
import org.springframework.security.AuthenticationManager;
import org.springframework.security.util.RedirectUtils;
import org.springframework.security.util.SessionUtils;
import org.springframework.security.util.UrlUtils;
import org.springframework.security.context.SecurityContextHolder;
@ -43,10 +44,6 @@ import org.springframework.util.Assert;
import java.io.IOException;
import java.util.Properties;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
import javax.servlet.FilterChain;
import javax.servlet.ServletException;
@ -215,8 +212,11 @@ public abstract class AbstractProcessingFilter extends SpringSecurityFilter impl
public void afterPropertiesSet() throws Exception {
Assert.hasLength(filterProcessesUrl, "filterProcessesUrl must be specified");
Assert.isTrue(UrlUtils.isValidRedirectUrl(filterProcessesUrl), filterProcessesUrl + " isn't a valid redirect URL");
Assert.hasLength(defaultTargetUrl, "defaultTargetUrl must be specified");
Assert.isTrue(UrlUtils.isValidRedirectUrl(defaultTargetUrl), defaultTargetUrl + " isn't a valid redirect URL");
// Assert.hasLength(authenticationFailureUrl, "authenticationFailureUrl must be specified");
Assert.isTrue(UrlUtils.isValidRedirectUrl(authenticationFailureUrl), authenticationFailureUrl + " isn't a valid redirect URL");
Assert.notNull(authenticationManager, "authenticationManager must be specified");
Assert.notNull(rememberMeServices, "rememberMeServices cannot be null");
Assert.notNull(targetUrlResolver, "targetUrlResolver cannot be null");

2
core/src/main/java/org/springframework/security/ui/AccessDeniedHandlerImpl.java
View File

@ -80,7 +80,7 @@ public class AccessDeniedHandlerImpl implements AccessDeniedHandler {
*/
public void setErrorPage(String errorPage) {
if ((errorPage != null) && !errorPage.startsWith("/")) {
throw new IllegalArgumentException("ErrorPage must begin with '/'");
throw new IllegalArgumentException("errorPage must begin with '/'");
}
this.errorPage = errorPage;

3
core/src/main/java/org/springframework/security/ui/logout/LogoutFilter.java
View File

@ -26,6 +26,7 @@ import org.springframework.security.Authentication;
import org.springframework.security.ui.SpringSecurityFilter;
import org.springframework.security.ui.FilterChainOrder;
import org.springframework.security.util.RedirectUtils;
import org.springframework.security.util.UrlUtils;
import org.springframework.security.context.SecurityContextHolder;
import org.springframework.util.Assert;
import org.springframework.util.StringUtils;
@ -58,6 +59,7 @@ public class LogoutFilter extends SpringSecurityFilter {
public LogoutFilter(String logoutSuccessUrl, LogoutHandler[] handlers) {
Assert.notEmpty(handlers, "LogoutHandlers are required");
this.logoutSuccessUrl = logoutSuccessUrl;
Assert.isTrue(UrlUtils.isValidRedirectUrl(logoutSuccessUrl), logoutSuccessUrl + " isn't a valid redirect URL");
this.handlers = handlers;
}
@ -161,6 +163,7 @@ public class LogoutFilter extends SpringSecurityFilter {
public void setFilterProcessesUrl(String filterProcessesUrl) {
Assert.hasText(filterProcessesUrl, "FilterProcessesUrl required");
Assert.isTrue(UrlUtils.isValidRedirectUrl(filterProcessesUrl), filterProcessesUrl + " isn't a valid redirect URL");
this.filterProcessesUrl = filterProcessesUrl;
}

5
core/src/main/java/org/springframework/security/util/UrlUtils.java
View File

@ -18,6 +18,7 @@ package org.springframework.security.util;
import org.springframework.security.intercept.web.FilterInvocation;
import org.springframework.security.ui.savedrequest.SavedRequest;
import org.springframework.util.StringUtils;
import javax.servlet.http.HttpServletRequest;
@ -117,4 +118,8 @@ public final class UrlUtils {
return buildRequestUrl(sr.getServletPath(), sr.getRequestURI(), sr.getContextPath(), sr.getPathInfo(),
sr.getQueryString());
}
public static boolean isValidRedirectUrl(String url) {
return !StringUtils.hasText(url) || url.startsWith("/") || url.toLowerCase().startsWith("http");
}
}

11
core/src/test/java/org/springframework/security/config/HttpSecurityBeanDefinitionParserTests.java
View File

@ -9,6 +9,7 @@ import java.util.List;
import org.junit.After;
import org.junit.Test;
import org.springframework.beans.factory.BeanCreationException;
import org.springframework.beans.factory.BeanDefinitionStoreException;
import org.springframework.beans.factory.parsing.BeanDefinitionParsingException;
import org.springframework.context.support.AbstractXmlApplicationContext;
import org.springframework.mock.web.MockHttpServletRequest;
@ -174,7 +175,7 @@ public class HttpSecurityBeanDefinitionParserTests {
assertEquals(Boolean.TRUE, FieldUtils.getFieldValue(filter, "alwaysUseDefaultTargetUrl"));
}
@Test(expected=BeanDefinitionParsingException.class)
@Test(expected=BeanCreationException.class)
public void invalidLoginPageIsDetected() throws Exception {
setContext(
"<http>" +
@ -182,7 +183,7 @@ public class HttpSecurityBeanDefinitionParserTests {
"</http>" + AUTH_PROVIDER_XML);
}
@Test(expected=BeanDefinitionParsingException.class)
@Test(expected=BeanCreationException.class)
public void invalidDefaultTargetUrlIsDetected() throws Exception {
setContext(
"<http>" +
@ -190,7 +191,7 @@ public class HttpSecurityBeanDefinitionParserTests {
"</http>" + AUTH_PROVIDER_XML);
}
@Test(expected=BeanDefinitionParsingException.class)
@Test(expected=BeanCreationException.class)
public void invalidLogoutUrlIsDetected() throws Exception {
setContext(
"<http>" +
@ -199,7 +200,7 @@ public class HttpSecurityBeanDefinitionParserTests {
"</http>" + AUTH_PROVIDER_XML);
}
@Test(expected=BeanDefinitionParsingException.class)
@Test(expected=BeanCreationException.class)
public void invalidLogoutSuccessUrlIsDetected() throws Exception {
setContext(
"<http>" +
@ -265,7 +266,7 @@ public class HttpSecurityBeanDefinitionParserTests {
assertEquals("/access-denied", FieldUtils.getFieldValue(etf, "accessDeniedHandler.errorPage"));
}
@Test(expected=BeanDefinitionParsingException.class)
@Test(expected=BeanDefinitionStoreException.class)
public void invalidAccessDeniedUrlIsDetected() throws Exception {
setContext("<http auto-config='true' access-denied-page='noLeadingSlash'/>" + AUTH_PROVIDER_XML);
}