SEC-1342: intercept-url EL expressions are parsed or validated incorrectly #1587

Closed
spring-issuemaster opened this Issue Dec 22, 2009 · 3 comments

1 participant

@spring-issuemaster

Christopher G. Stach II (Migrated from SEC-1342) said:

<security:http use-expressions="true">

    <security:access-denied-handler error-page="/authfail.html" />

    <!-- XXX requires-channel="https" -->
    <security:intercept-url pattern="/app/**" access="hasAnyRole('ROLE_A','ROLE_B','ROLE_C','ROLE_D')" />

    <security:intercept-url pattern="/css/**" access="isAuthenticated()" />

    <security:intercept-url pattern="/images/logo.png" access="permitAll" />

    <security:intercept-url pattern="/images/**" access="isAuthenticated()" />

    <security:intercept-url pattern="/xmlhttp/**" access="isAuthenticated()" />

    <security:intercept-url pattern="/authfail.html" access="permitAll" />

    <security:intercept-url pattern="/login.jsp" access="permitAll" />

    <security:intercept-url pattern="/**" access="denyAll" />

    <security:form-login always-use-default-target="true" default-target-url="/app/index.jspx" login-page="/" />

    <security:logout invalidate-session="true" logout-success-url="/" />

    <security:anonymous />

    <security:session-management>
        <security:concurrency-control max-sessions="1" />
    </security:session-management>

</security:http>

When the context loads, this happens:

Caused by: java.lang.IllegalArgumentException: Expected a single expression attribute for []
at org.springframework.util.Assert.isTrue(Assert.java:65)
at org.springframework.security.web.access.expression.ExpressionBasedFilterInvocationSecurityMetadataSource.processMap(ExpressionBasedFilterInvocationSecurityMetadataSource.java:43)
at org.springframework.security.web.access.expression.ExpressionBasedFilterInvocationSecurityMetadataSource.(ExpressionBasedFilterInvocationSecurityMetadataSource.java:30)
at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:39)
at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:27)
at java.lang.reflect.Constructor.newInstance(Constructor.java:513)
at org.springframework.beans.BeanUtils.instantiateClass(BeanUtils.java:126)
... 37 more

That bit of code in o.s.s.web.access.expression.ExpressionBasedFilterInvocationSecurityMetadataSource looks like this:

public ExpressionBasedFilterInvocationSecurityMetadataSource(UrlMatcher urlMatcher,
        LinkedHashMap<RequestKey, Collection<ConfigAttribute>> requestMap, WebSecurityExpressionHandler expressionHandler) {
    super(urlMatcher, processMap(requestMap, expressionHandler.getExpressionParser()));
    Assert.notNull(expressionHandler, "A non-null SecurityExpressionHandler is required");
}

private static LinkedHashMap<RequestKey, Collection<ConfigAttribute>> processMap(
        LinkedHashMap<RequestKey,Collection<ConfigAttribute>> requestMap, ExpressionParser parser) {

[...]
for (Map.Entry> entry : requestMap.entrySet()) {
RequestKey request = entry.getKey();
Assert.isTrue(entry.getValue().size() == 1, "Expected a single expression attribute for " + request);

The Assert is line 43, where the failure happens. In the debugger, entry.getValue() has 4 entries. It is basically the expression hasAnyRole('ROLE_A','ROLE_B','ROLE_C','ROLE_D') tokenized on the commas.

@spring-issuemaster

Christopher G. Stach II said:

It looks like ConstructorResolver.instantiateUsingFactoryMethod determines that SecurityConfig.createList wants a String[], so it hands it off to BeanWrapperImpl.convertIfNecessary at line 447 to convert. It happily does so at line 225 by tokenizing on commas, and then things proceed to break.

@spring-issuemaster

Luke Taylor said:

Yes, it looks like Spring is trying to be helpful and converting the String to an array. You can use an "or" expression in the meantime as a workaround.

@spring-issuemaster

Luke Taylor said:

I've added another factory method to SecurityConfig - createSingleAttributeList, which prevents this problem.

@spring-issuemaster spring-issuemaster added this to the 3.0.1 milestone Feb 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment