diff --git a/core/src/main/java/org/springframework/security/access/expression/method/MethodSecurityEvaluationContext.java b/core/src/main/java/org/springframework/security/access/expression/method/MethodSecurityEvaluationContext.java index 3688f01c480..916dd380455 100644 --- a/core/src/main/java/org/springframework/security/access/expression/method/MethodSecurityEvaluationContext.java +++ b/core/src/main/java/org/springframework/security/access/expression/method/MethodSecurityEvaluationContext.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2018 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. @@ -15,17 +15,16 @@ */ package org.springframework.security.access.expression.method; -import java.lang.reflect.Method; - import org.aopalliance.intercept.MethodInvocation; 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; + +import java.lang.reflect.Method; /** * Internal security-specific EvaluationContext implementation which lazily adds the @@ -33,87 +32,21 @@ * 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 - * thus allowing for caching. - */ - public MethodSecurityEvaluationContext(Authentication user, MethodInvocation mi) { - this(user, mi, new DefaultSecurityParameterNameDiscoverer()); - } - public MethodSecurityEvaluationContext(Authentication user, MethodInvocation mi, + 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; - } - - 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++) { - super.setVariable(paramNames[i], args[i]); - } + private static Method getSpecificMethod(MethodInvocation mi) { + return AopUtils.getMostSpecificMethod(mi.getMethod(), AopProxyUtils.ultimateTargetClass(mi.getThis())); } } diff --git a/core/src/test/java/org/springframework/security/access/expression/method/DefaultMethodSecurityExpressionHandlerTests.java b/core/src/test/java/org/springframework/security/access/expression/method/DefaultMethodSecurityExpressionHandlerTests.java index 2cdc31a1786..02ea54bafd6 100644 --- a/core/src/test/java/org/springframework/security/access/expression/method/DefaultMethodSecurityExpressionHandlerTests.java +++ b/core/src/test/java/org/springframework/security/access/expression/method/DefaultMethodSecurityExpressionHandlerTests.java @@ -35,9 +35,7 @@ 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 @@ -82,7 +82,6 @@ public void createEvaluationContextCustomTrustResolver() { @SuppressWarnings("unchecked") public void filterWhenUsingStreamThenFiltersStream() { final Stream stream = Stream.of("1", "2", "3"); - Expression expression = handler.getExpressionParser().parseExpression("filterObject ne '2'"); EvaluationContext context = handler.createEvaluationContext(authentication, @@ -108,4 +107,9 @@ public void filterStreamWhenClosedThenUpstreamGetsClosed() { ((Stream) handler.filter(upstream, expression, context)).close(); verify(upstream).close(); } + + private static class Foo { + public void bar(){ + } + } }