SEC-501: Fix. Convert secure url paths to lower case if convertUrlToLowercaseBeforeComparison is true.

Also removed unnecessary assertions from PathBasedFilterDefinitionMapTests.
This commit is contained in:
Luke Taylor 2007-08-28 16:53:05 +00:00
parent 036ea034ac
commit 6fe00b3433
2 changed files with 33 additions and 17 deletions

View File

@ -34,17 +34,17 @@ import java.util.Vector;
* Maintains a <code>List</code> of <code>ConfigAttributeDefinition</code>s associated with different HTTP request
* URL Apache Ant path-based patterns.<p>Apache Ant path expressions are used to match a HTTP request URL against a
* <code>ConfigAttributeDefinition</code>.</p>
* <p>The order of registering the Ant paths using the {@link #addSecureUrl(String, ConfigAttributeDefinition)} is
* <p>The order of registering the Ant paths using the {@link #addSecureUrl(String,ConfigAttributeDefinition)} is
* very important. The system will identify the <b>first</b> matching path for a given HTTP URL. It will not proceed
* to evaluate later paths if a match has already been found. Accordingly, the most specific paths should be
* registered first, with the most general paths registered last.</p>
* <p>If no registered paths match the HTTP URL, <code>null</code> is returned.</p>
* <p>If no registered paths match the HTTP URL, <code>null</code> is returned.</p>
*
* @author Ben Alex
* @version $Id$
*/
public class PathBasedFilterInvocationDefinitionMap extends AbstractFilterInvocationDefinitionSource
implements FilterInvocationDefinition {
implements FilterInvocationDefinition {
//~ Static fields/initializers =====================================================================================
private static final Log logger = LogFactory.getLog(PathBasedFilterInvocationDefinitionMap.class);
@ -58,6 +58,12 @@ public class PathBasedFilterInvocationDefinitionMap extends AbstractFilterInvoca
//~ Methods ========================================================================================================
public void addSecureUrl(String antPath, ConfigAttributeDefinition attr) {
// SEC-501: If using lower case comparison, we should convert the paths to lower case
// as any upper case characters included by mistake will prevent the URL from ever being matched.
if (convertUrlToLowercaseBeforeComparison) {
antPath = antPath.toLowerCase();
}
requestMap.add(new EntryHolder(antPath, attr));
if (logger.isDebugEnabled()) {
@ -110,7 +116,7 @@ public class PathBasedFilterInvocationDefinitionMap extends AbstractFilterInvoca
if (logger.isDebugEnabled()) {
logger.debug("Candidate is: '" + url + "'; pattern is " + entryHolder.getAntPath() + "; matched="
+ matched);
+ matched);
}
if (matched) {

View File

@ -36,7 +36,6 @@ public class PathBasedFilterDefinitionMapTests extends TestCase {
//~ Constructors ===================================================================================================
public PathBasedFilterDefinitionMapTests() {
super();
}
public PathBasedFilterDefinitionMapTests(String arg0) {
@ -59,7 +58,6 @@ public class PathBasedFilterDefinitionMapTests extends TestCase {
public void testLookupNotRequiringExactMatchSuccessIfNotMatching() {
PathBasedFilterInvocationDefinitionMap map = new PathBasedFilterInvocationDefinitionMap();
map.setConvertUrlToLowercaseBeforeComparison(true);
assertTrue(map.isConvertUrlToLowercaseBeforeComparison());
ConfigAttributeDefinition def = new ConfigAttributeDefinition();
def.addConfigAttribute(new SecurityConfig("ROLE_ONE"));
@ -71,10 +69,26 @@ public class PathBasedFilterDefinitionMapTests extends TestCase {
assertEquals(def, response);
}
/**
* SEC-501
*/
public void testLookupNotRequiringExactMatchSucceedsIfSecureUrlPathContainsUpperCase() {
PathBasedFilterInvocationDefinitionMap map = new PathBasedFilterInvocationDefinitionMap();
map.setConvertUrlToLowercaseBeforeComparison(true);
ConfigAttributeDefinition def = new ConfigAttributeDefinition();
def.addConfigAttribute(new SecurityConfig("ROLE_ONE"));
map.addSecureUrl("/SeCuRE/super/**", def);
FilterInvocation fi = createFilterinvocation("/secure/super/somefile.html");
ConfigAttributeDefinition response = map.lookupAttributes(fi.getRequestUrl());
assertEquals(def, response);
}
public void testLookupRequiringExactMatchFailsIfNotMatching() {
PathBasedFilterInvocationDefinitionMap map = new PathBasedFilterInvocationDefinitionMap();
assertFalse(map.isConvertUrlToLowercaseBeforeComparison());
ConfigAttributeDefinition def = new ConfigAttributeDefinition();
def.addConfigAttribute(new SecurityConfig("ROLE_ONE"));
map.addSecureUrl("/secure/super/**", def);
@ -87,13 +101,11 @@ public class PathBasedFilterDefinitionMapTests extends TestCase {
public void testLookupRequiringExactMatchIsSuccessful() {
PathBasedFilterInvocationDefinitionMap map = new PathBasedFilterInvocationDefinitionMap();
assertFalse(map.isConvertUrlToLowercaseBeforeComparison());
ConfigAttributeDefinition def = new ConfigAttributeDefinition();
def.addConfigAttribute(new SecurityConfig("ROLE_ONE"));
map.addSecureUrl("/secure/super/**", def);
map.addSecureUrl("/SeCurE/super/**", def);
FilterInvocation fi = createFilterinvocation("/secure/super/somefile.html");
FilterInvocation fi = createFilterinvocation("/SeCurE/super/somefile.html");
ConfigAttributeDefinition response = map.lookupAttributes(fi.getRequestUrl());
assertEquals(def, response);
@ -101,8 +113,6 @@ public class PathBasedFilterDefinitionMapTests extends TestCase {
public void testLookupRequiringExactMatchWithAdditionalSlashesIsSuccessful() {
PathBasedFilterInvocationDefinitionMap map = new PathBasedFilterInvocationDefinitionMap();
assertFalse(map.isConvertUrlToLowercaseBeforeComparison());
ConfigAttributeDefinition def = new ConfigAttributeDefinition();
def.addConfigAttribute(new SecurityConfig("ROLE_ONE"));
map.addSecureUrl("/someAdminPage.html**", def);
@ -113,11 +123,11 @@ public class PathBasedFilterDefinitionMapTests extends TestCase {
assertEquals(def, response); // see SEC-161 (it should truncate after ? sign)
}
/** Check fixes for SEC-321 */
/**
* Check fixes for SEC-321
*/
public void testExtraQuestionMarkStillMatches() {
PathBasedFilterInvocationDefinitionMap map = new PathBasedFilterInvocationDefinitionMap();
assertFalse(map.isConvertUrlToLowercaseBeforeComparison());
ConfigAttributeDefinition def = new ConfigAttributeDefinition();
def.addConfigAttribute(new SecurityConfig("ROLE_ONE"));
map.addSecureUrl("/someAdminPage.html*", def);