diff --git a/core/src/main/java/org/springframework/security/access/expression/method/DefaultMethodSecurityExpressionHandler.java b/core/src/main/java/org/springframework/security/access/expression/method/DefaultMethodSecurityExpressionHandler.java index f5094efb5b9..c2de6d715f9 100644 --- a/core/src/main/java/org/springframework/security/access/expression/method/DefaultMethodSecurityExpressionHandler.java +++ b/core/src/main/java/org/springframework/security/access/expression/method/DefaultMethodSecurityExpressionHandler.java @@ -102,10 +102,7 @@ public Object filter(Object filterTarget, Expression filterExpression, Evaluatio logger.debug("Retaining elements: " + retainList); } - collection.clear(); - collection.addAll(retainList); - - return filterTarget; + return retainList; } if (filterTarget.getClass().isArray()) { diff --git a/core/src/main/java/org/springframework/security/access/expression/method/ExpressionBasedPreInvocationAdvice.java b/core/src/main/java/org/springframework/security/access/expression/method/ExpressionBasedPreInvocationAdvice.java index 4a613087931..7ff1c9af70e 100644 --- a/core/src/main/java/org/springframework/security/access/expression/method/ExpressionBasedPreInvocationAdvice.java +++ b/core/src/main/java/org/springframework/security/access/expression/method/ExpressionBasedPreInvocationAdvice.java @@ -30,8 +30,8 @@ public boolean before(Authentication authentication, MethodInvocation mi, PreInv if (preFilter != null) { Object filterTarget = findFilterTarget(preAttr.getFilterTarget(), ctx, mi); - - expressionHandler.filter(filterTarget, preFilter, ctx); + Object filterResult = expressionHandler.filter(filterTarget, preFilter, ctx); + replaceFirstMethodArgument(mi, filterResult); } if (preAuthorize == null) { @@ -69,6 +69,14 @@ private Object findFilterTarget(String filterTargetName, EvaluationContext ctx, return filterTarget; } + private void replaceFirstMethodArgument(MethodInvocation mi, Object newFirstArgument) { + Object[] arguments = mi.getArguments(); + if (arguments.length == 0) { + throw new IllegalArgumentException("Filter target had no argument to replace."); + } + arguments[0] = newFirstArgument; + } + public void setExpressionHandler(MethodSecurityExpressionHandler expressionHandler) { this.expressionHandler = expressionHandler; diff --git a/core/src/test/java/org/springframework/security/access/expression/method/MethodExpressionVoterTests.java b/core/src/test/java/org/springframework/security/access/expression/method/MethodExpressionVoterTests.java index a72741b9e13..fc7a3d783ac 100644 --- a/core/src/test/java/org/springframework/security/access/expression/method/MethodExpressionVoterTests.java +++ b/core/src/test/java/org/springframework/security/access/expression/method/MethodExpressionVoterTests.java @@ -6,13 +6,13 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.List; import org.aopalliance.intercept.MethodInvocation; import org.junit.Test; import org.springframework.security.access.AccessDecisionVoter; import org.springframework.security.access.ConfigAttribute; -import org.springframework.security.access.expression.method.PreInvocationExpressionAttribute; import org.springframework.security.access.prepost.PreInvocationAuthorizationAdviceVoter; import org.springframework.security.authentication.TestingAuthenticationToken; import org.springframework.security.util.SimpleMethodInvocation; @@ -51,7 +51,8 @@ public void accessIsGrantedIfNoPreAuthorizeAttributeIsUsed() throws Exception { assertEquals(AccessDecisionVoter.ACCESS_GRANTED, am.vote(joe, mi, createAttributes(new PreInvocationExpressionAttribute("(filterObject == 'jim')", "collection", null)))); // All objects should have been removed, because the expression is always false - assertEquals(0, arg.size()); + List filteredArg = ((List) mi.getArguments()[0]); + assertEquals(0, filteredArg.size()); } @Test @@ -59,9 +60,19 @@ public void collectionPreFilteringIsSuccessful() throws Exception { List arg = createCollectionArg("joe", "bob", "sam"); MethodInvocation mi = new SimpleMethodInvocation(new TargetImpl(), methodTakingACollection(), arg); am.vote(joe, mi, createAttributes(new PreInvocationExpressionAttribute("(filterObject == 'joe' or filterObject == 'sam')", "collection", "permitAll"))); - assertEquals("joe and sam should still be in the list", 2, arg.size()); - assertEquals("joe", arg.get(0)); - assertEquals("sam", arg.get(1)); + List filteredArg = ((List) mi.getArguments()[0]); + assertEquals("joe and sam should still be in the list", 2, filteredArg.size()); + assertEquals("joe", filteredArg.get(0)); + assertEquals("sam", filteredArg.get(1)); + } + + @Test + public void collectionPreFilteringIsSuccessfulEvenWithAnImmutableList() throws Exception { + List arg = Collections.singletonList("joe"); + MethodInvocation mi = new SimpleMethodInvocation(new TargetImpl(), methodTakingACollection(), arg); + assertEquals(AccessDecisionVoter.ACCESS_GRANTED, + am.vote(joe, mi, createAttributes(new PreInvocationExpressionAttribute("(filterObject == 'joe')", "collection", "permitAll")))); + assertEquals("joe should still be the argument list", arg, mi.getArguments()[0]); } @Test(expected=IllegalArgumentException.class)