SEC-1663: Expression support in protect-pointcut xml config

[spring-projects-issues]

Brian Relph (Migrated from SEC-1663) said:

Enable Spring-EL support for protect-pointcut xml.

Example:

<security:global-method-security pre-post-annotations="enabled">
<security:protect-pointcut expression="execution(* com.acl.controllers.Controller.(..))"
access="isFullyAuthenticated()" />
/security:global-method-security

[spring-projects-issues]

Luke Taylor said:

I'm not sure there is such a pressing use-case for EL in protect-pointcuts. Since they are generally used across multiple methods and classes, you lose the ability to reference method arguments directly in an expression, which is where EL is most likely to add value.

[spring-projects-issues]

Andrew said:

I believe that you don't loose ability to reference method arguments.

You can create protect-pointcut for single method and reference it's arguments like:

<protect-pointcut expression="execution(* com.novage.services.TokenManager.updateExpirationDate(..))" access="#token == principal.token" />

also you can create a pointcut for many methods that has the same arguments like:

<protect-pointcut expression="execution(* com.novage.services.UserService.update*(..))" access="#username == principal.username" />

(all the UserService.update* methods must have 'username' parameter)

I already implemented an AccessDecisionVoter that is capable of handling expressions. It uses the same MethodSecurityExpressionHandler as in annotations so all expressions from annotations should work.

Sample configuration:

<beans:bean id="methodSecurityAccessDecisionManager" class="org.springframework.security.access.vote.AffirmativeBased">
    <beans:property name="decisionVoters">
    <beans:list>
        <beans:bean class="com.novage.security.MethodExpressionVoter" />
    </beans:list>
    </beans:property>
</beans:bean>

<global-method-security access-decision-manager-ref="methodSecurityAccessDecisionManager">
    <protect-pointcut expression="execution(* com.novage.services.SettingManager.*(..))" access="permitAll" />
    <protect-pointcut expression="execution(* com.novage.services.TokenManager.updateExpirationDate(..))" access="#token == principal.token" />
</global-method-security>

Class com.novage.security.MethodExpressionVoter:

package com.novage.security;

import java.util.Collection;
import java.util.HashMap;

import org.aopalliance.intercept.MethodInvocation;
import org.springframework.expression.EvaluationContext;
import org.springframework.expression.Expression;
import org.springframework.expression.spel.standard.SpelExpressionParser;
import org.springframework.security.access.AccessDecisionVoter;
import org.springframework.security.access.ConfigAttribute;
import org.springframework.security.access.expression.ExpressionUtils;
import org.springframework.security.access.expression.method.DefaultMethodSecurityExpressionHandler;
import org.springframework.security.access.expression.method.MethodSecurityExpressionHandler;
import org.springframework.security.core.Authentication;

public class MethodExpressionVoter implements AccessDecisionVoter<MethodInvocation> {
    private MethodSecurityExpressionHandler expressionHandler = new DefaultMethodSecurityExpressionHandler();

    private final HashMap<String, Expression> expressionsCache = new HashMap<String, Expression>();

    @Override
    public boolean supports(ConfigAttribute attribute) {
        String expressionAsString = attribute.getAttribute();
        SpelExpressionParser parser = new SpelExpressionParser();

        if (expressionAsString == null) {
            return false;
        }

        try {
            parser.parseExpression(expressionAsString);
        } catch (RuntimeException e) {
            return false;
        }

        return true;
    }

    @Override
    public boolean supports(Class<?> clazz) {
        return MethodInvocation.class.isAssignableFrom(clazz);
    }

    @Override
    public int vote(Authentication authentication, MethodInvocation methodInvocation, Collection<ConfigAttribute> attributes) {     
        EvaluationContext ctx = expressionHandler.createEvaluationContext(authentication, methodInvocation);

        int resultAccess = ACCESS_ABSTAIN;

        for (ConfigAttribute attribute : attributes) {
            String expressionAsString = attribute.getAttribute();

            if (expressionAsString != null) {

                // Try to get expression from cache
                Expression authorizeExpression = expressionsCache.get(expressionAsString);

                if (authorizeExpression == null) {
                    // Expression not in cache. Parse and add it to cache.

                    SpelExpressionParser parser = new SpelExpressionParser();

                    authorizeExpression = parser.parseExpression(expressionAsString);

                    expressionsCache.put(expressionAsString, authorizeExpression);
                }

                if (!ExpressionUtils.evaluateAsBoolean(authorizeExpression, ctx)) {
                    return ACCESS_DENIED;
                } else {
                    resultAccess = ACCESS_GRANTED;
                }
            }
        }

        return resultAccess;
    }

    public void setExpressionHandler(MethodSecurityExpressionHandler expressionHandler) {
        this.expressionHandler = expressionHandler;
    }
}

Class MethodExpressionVoter uses caching to not parse same expressions many times (to work faster). This is basically not very good. I believe approach with custom MethodSecurityMetadataSource that parses the expressions and saves into custom ConfigAttribute should be used like it is already implemented in HTTP security schema with enabled expressions (i.e. <security:http use-expressions="true"> ... /security:http).

[shollander]

+1 For this feature. I think it would be very useful.

Another element of this along the same lines would be to add support for all method security expressions such as the @Pre and @Post annotations.

Something like this would be very useful:

<global-method-security>
<protect-pointcut expression="execution(MyObject com.mycompany.*Service.*(..))"
    access="ROLE_USER" postAuthorize="hasPermission(returnValue, 'read')"/>
</global-method-security>

If I have a service that contains many methods for returning an object or collection (different queries for example), I don't want to have to duplicate the same annotation on each method. It is also error prone since when someone else comes along to add a new method to the service, they might forget to add the @PostAuthorize for example. AOP provides a good way of catching everything in one sweep.

[codemasterover9000]

+1