@PreAuthorize and @P not working

[yoshikawaa]

Summary

This method authorization code works fine with Spring Security 5.0.7.RELEASE.
But provides NPE with Spring Security 5.1.2.RELEASE,

• Service

@Service
public class SampleService {
    @PreAuthorize("#username == principal.username")
    String sample(String depertment, @P("username") String username) {
        return "success";
    }
}

• Test

@RunWith(SpringRunner.class)
@ComponentScan
@EnableGlobalMethodSecurity(prePostEnabled = true)
public class SampleServiceTest {
    @Autowired
    private SampleService service;
    @Test
    @WithMockUser(username = "user")
    public void test() {
        service.sample("dep", "user");
    }
}

Actual Behavior

Provices NPE...

java.lang.NullPointerException
	at java.util.concurrent.ConcurrentHashMap.putVal(ConcurrentHashMap.java:1011)
	at java.util.concurrent.ConcurrentHashMap.put(ConcurrentHashMap.java:1006)
	at org.springframework.expression.spel.support.StandardEvaluationContext.setVariable(StandardEvaluationContext.java:234)
	at org.springframework.security.access.expression.method.MethodSecurityEvaluationContext.addArgumentsAsVariables(MethodSecurityEvaluationContext.java:115)
	at org.springframework.security.access.expression.method.MethodSecurityEvaluationContext.lookupVariable(MethodSecurityEvaluationContext.java:70)
	at org.springframework.expression.spel.ExpressionState.lookupVariable(ExpressionState.java:146)
	at org.springframework.expression.spel.ast.VariableReference.getValueInternal(VariableReference.java:76)
	at org.springframework.expression.spel.ast.OpEQ.getValueInternal(OpEQ.java:42)
	at org.springframework.expression.spel.ast.OpEQ.getValueInternal(OpEQ.java:32)
	at org.springframework.expression.spel.ast.SpelNodeImpl.getTypedValue(SpelNodeImpl.java:116)
	at org.springframework.expression.spel.standard.SpelExpression.getValue(SpelExpression.java:300)
	at org.springframework.security.access.expression.ExpressionUtils.evaluateAsBoolean(ExpressionUtils.java:26)
	at org.springframework.security.access.expression.method.ExpressionBasedPreInvocationAdvice.before(ExpressionBasedPreInvocationAdvice.java:59)
	at org.springframework.security.access.prepost.PreInvocationAuthorizationAdviceVoter.vote(PreInvocationAuthorizationAdviceVoter.java:72)
	at org.springframework.security.access.prepost.PreInvocationAuthorizationAdviceVoter.vote(PreInvocationAuthorizationAdviceVoter.java:40)
	at org.springframework.security.access.vote.AffirmativeBased.decide(AffirmativeBased.java:63)
	at org.springframework.security.access.intercept.AbstractSecurityInterceptor.beforeInvocation(AbstractSecurityInterceptor.java:233)
	at org.springframework.security.access.intercept.aopalliance.MethodSecurityInterceptor.invoke(MethodSecurityInterceptor.java:65)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
	at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:688)
	at com.example.SampleService$$EnhancerBySpringCGLIB$$712d7bb4.sample(<generated>)
	at com.example.SampleServiceTest.test(SampleServiceTest.java:23)
        ...

This exception caused by StandardEvaluationContext supports concurrent variable modification (SPR-17448).

AnnotationParameterNameDiscoverer find parameter names with @P using Method#getParameterAnnotations. But not annotated parameter name will be null and MethodSecurityEvaluationContext set null variable to StandardEvaluationContext(ConcurrentHashMap)...

Expected Behavior

MethodSecurityEvaluationContext avoid to set null variable.
Because looking at StandardEvaluationContext#setVariable(String name, @Nullable Object value), name parameter must not null.

Configuration

Version

• Spring Boot 2.1.1.RELEASE
• Spring Framework 5.1.3.RELEASE
• Spring Security 5.1.2.RELEASE

Sample

• https://github.com/yoshikawaa/spring-security-gh6300

[rwinch]

Thanks for the report. This is a duplicate of https://jira.spring.io/browse/SPR-17565 You can either switch to a different version of Spring Framework or you can ensure to annotate all of the parameters.