Make MethodSecurityEvaluationContext Delegate to MethodBasedEvaluationContext

Spring Security's MethodSecurityEvaluationContext should delegate to Spring Framework's
MethodBasedEvaluationContext

Fixes: gh-6224
This commit is contained in:
Daniel Bustamante Ospina 2018-12-06 17:00:23 -05:00 committed by Eleftheria Stein
parent 96d82ecbf2
commit 150b66824d
2 changed files with 16 additions and 72 deletions

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2016 the original author or authors.
* Copyright 2002-2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -22,8 +22,8 @@ import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.aop.framework.AopProxyUtils;
import org.springframework.aop.support.AopUtils;
import org.springframework.context.expression.MethodBasedEvaluationContext;
import org.springframework.core.ParameterNameDiscoverer;
import org.springframework.expression.spel.support.StandardEvaluationContext;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.parameters.DefaultSecurityParameterNameDiscoverer;
@ -33,16 +33,13 @@ import org.springframework.security.core.parameters.DefaultSecurityParameterName
* when they are required.
*
* @author Luke Taylor
* @author Daniel Bustamante
* @since 3.0
*/
class MethodSecurityEvaluationContext extends StandardEvaluationContext {
class MethodSecurityEvaluationContext extends MethodBasedEvaluationContext {
private static final Log logger = LogFactory
.getLog(MethodSecurityEvaluationContext.class);
private ParameterNameDiscoverer parameterNameDiscoverer;
private final MethodInvocation mi;
private boolean argumentsAdded;
/**
* Intended for testing. Don't use in practice as it creates a new parameter resolver
* for each instance. Use the constructor which takes the resolver, as an argument
@ -54,68 +51,10 @@ class MethodSecurityEvaluationContext extends StandardEvaluationContext {
MethodSecurityEvaluationContext(Authentication user, MethodInvocation mi,
ParameterNameDiscoverer parameterNameDiscoverer) {
this.mi = mi;
this.parameterNameDiscoverer = parameterNameDiscoverer;
super(mi.getThis(), getSpecificMethod(mi), mi.getArguments(), parameterNameDiscoverer);
}
@Override
public Object lookupVariable(String name) {
Object variable = super.lookupVariable(name);
if (variable != null) {
return variable;
}
if (!argumentsAdded) {
addArgumentsAsVariables();
argumentsAdded = true;
}
variable = super.lookupVariable(name);
if (variable != null) {
return variable;
}
return null;
private static Method getSpecificMethod(MethodInvocation mi) {
return AopUtils.getMostSpecificMethod(mi.getMethod(), AopProxyUtils.ultimateTargetClass(mi.getThis()));
}
public void setParameterNameDiscoverer(ParameterNameDiscoverer parameterNameDiscoverer) {
this.parameterNameDiscoverer = parameterNameDiscoverer;
}
private void addArgumentsAsVariables() {
Object[] args = mi.getArguments();
if (args.length == 0) {
return;
}
Object targetObject = mi.getThis();
// SEC-1454
Class<?> targetClass = AopProxyUtils.ultimateTargetClass(targetObject);
if (targetClass == null) {
// TODO: Spring should do this, but there's a bug in ultimateTargetClass()
// which returns null
targetClass = targetObject.getClass();
}
Method method = AopUtils.getMostSpecificMethod(mi.getMethod(), targetClass);
String[] paramNames = parameterNameDiscoverer.getParameterNames(method);
if (paramNames == null) {
logger.warn("Unable to resolve method parameter names for method: "
+ method
+ ". Debug symbol information is required if you are using parameter names in expressions.");
return;
}
for (int i = 0; i < args.length; i++) {
if (paramNames[i] != null) {
setVariable(paramNames[i], args[i]);
}
}
}
}

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -35,9 +35,7 @@ import org.springframework.security.core.context.SecurityContextHolder;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.*;
@RunWith(MockitoJUnitRunner.class)
public class DefaultMethodSecurityExpressionHandlerTests {
@ -53,6 +51,8 @@ public class DefaultMethodSecurityExpressionHandlerTests {
@Before
public void setup() {
handler = new DefaultMethodSecurityExpressionHandler();
when(methodInvocation.getThis()).thenReturn(new Foo());
when(methodInvocation.getMethod()).thenReturn(Foo.class.getMethods()[0]);
}
@After
@ -108,4 +108,9 @@ public class DefaultMethodSecurityExpressionHandlerTests {
((Stream) handler.filter(upstream, expression, context)).close();
verify(upstream).close();
}
private static class Foo {
public void bar(){
}
}
}