lookupAttributes(String url) of PathBasedFilterInvocationDefinitionMap

[j_idris]

Hi there,
It seems to me that the method lookupAttributes is logically flawed.

Given the following source code of lookUpAttributes() from PathBasedFilterInvocationDefinitionMap:

public ConfigAttributeDefinition lookupAttributes(String url) {
        // Strip anything after a question mark symbol, as per SEC-161. See also SEC-321
        int firstQuestionMarkIndex = url.indexOf("?");

        if (firstQuestionMarkIndex != -1) {
            url = url.substring(0, firstQuestionMarkIndex);
        }

        if (isConvertUrlToLowercaseBeforeComparison()) {
            url = url.toLowerCase();

            if (logger.isDebugEnabled()) {
                logger.debug("Converted URL to lowercase, from: '" + url + "'; to: '" + url + "'");
            }
        }

        Iterator iter = requestMap.iterator();

        while (iter.hasNext()) {
            EntryHolder entryHolder = (EntryHolder) iter.next();

            boolean matched = pathMatcher.match(entryHolder.getAntPath(), url);

            if (logger.isDebugEnabled()) {
                logger.debug("Candidate is: '" + url + "'; pattern is " + entryHolder.getAntPath() + "; matched="
                    + matched);
            }

            if (matched) {
                return entryHolder.getConfigAttributeDefinition();
            }
        }

        return null;
    }

Assuming that the value of parameter url is "/index.jsp", and the iterator loops through each of the entry holder comparing the ant path from the entry holder against the url parameter.

What happens if a given entry holder ant path "/**" is evaluated after entry holder ant path "/index.jsp"? Assuming that authentication has taken place, [User: User1 Role: ROLE_TEST] and ROLE TEST is given rights to "/**" only.

This means that if the current entry holder ant path in the iterator loop evaluates "/index.jsp" first, the local variable matched will be true since there's a pattern matching. But I will be denied access from accessing "/index.jsp" because it has no authorities.

Therefore, RoleVoter will return ACCESS_DENIED after voting whereas it should return ACCESS_GRANTED. ACCESS_DENIED because "/index.jsp" is evaluated first before "/**".

In order to correct this, I had to extend from PathBasedFilterInvocationDefinitionMap, overload lookupAttributes() and loop through a list of sorted keys of requestMap where "/**" will be evaluated first.

Am I doing this right way? Should I even extend from PathBasedFilterInvocationDefinitionMap in the first place?

Regards

[karldmoore]

I'm not quite sure I'm understanding what you are getting at. If the paths are evaluated in order, the isn't the answer simply to ensure you order most specific first to least specific last?

[j_idris]

It simply means that if u map /** to a role and a user of that role tries to access a page say /index.jsp, there's bound to be some problems when PathBasedFilterInvocationDefinitionMap decides the access because although index.jsp matches the ant pattern /**, the user will be denied access.

[Luke Taylor]

It simply means that if u map /** to a role and a user of that role tries to access a page say /index.jsp, there's bound to be some problems when PathBasedFilterInvocationDefinitionMap decides the access because although index.jsp matches the ant pattern /**, the user will be denied access.

This isn't correct. The ant path /** matches /index.jsp so the required authorities for /index.jsp will be those specified for /**. That's the logic in the code. Using ant paths would be pretty pointless otherwise.

[karldmoore]

This isn't correct. The ant path /** matches /index.jsp so the required authorities for /index.jsp will be those specified for /**. That's the logic in the code. Using ant paths would be pretty pointless otherwise.

Well exactly! I'm not sure I understand the original confusion. If this still doesn't make sense, can you have another stab at explaining the bits you don't understand?

[j_idris]

Here's another simpler scenario to clear things up:

Given the following role configuration:
<property name="objectDefinitionSource">
<value>
/**=ROLE_ADMIN
/index.jsp=ROLE_TEST
</value>
</property>

Assuming that a user of ROLE_TEST is authenticated, and calls upon /index.jsp.
The SecurityInterceptor comes into the picture, calling on lookUpAttributes().

At the first iteration of the while loop in lookUpAttributes(),
entryHolder has the following properties:

entryHolder
antPath = /** (String)
configAttributeDefinition (ConfgiAttributeDefinition)
configAttributes (Vector)
[0] (SecurityConfig)
attrib (String) = "ROLE_ADMIN"

Comparing the ant path /** against /index.jsp will result a true matched value. However, the user with role ROLE_TEST is denied access because the entryHolder holds only one authority. But from the user point of view, he does not understand why he is denied access. /index.jsp is already mapped to ROLE_TEST.

I came to realise that it doesnt matter if /** is at the top of the list of middle or bottom. Whats not right here is that PathBasedFilterInvocationDefinitionMap finds the first match, and immediately returns the entryHolder object irregardless if there are more "relevant" matches in the list. And of course, the access decision manager denies the access.

[karldmoore]

I don't think what you are saying is correct. The order of entries is the key here, you have put the least specific first. You need to put the least specific last.

/**
* 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
* 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>
*
* @author Ben Alex
* @version $Id: PathBasedFilterInvocationDefinitionMap.java 1636 2006-09-03 22:12:13 +0000 (Sun, 03 Sep 2006) luke_t $
*/

http://www.acegisecurity.org/multipr...nitionMap.html