From c8820166c817e3d55c1aca33bc5fa0b2ff958003 Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Thu, 16 Dec 2010 15:21:22 +0000 Subject: [PATCH] SEC-1576: Parameterize the secured object type in AccessDecisionVoter. --- acl/acl.gradle | 1 + .../security/acls/AclEntryVoter.java | 3 +- .../security/access/AccessDecisionVoter.java | 49 +++++++++---------- .../access/annotation/Jsr250Voter.java | 2 +- ...PreInvocationAuthorizationAdviceVoter.java | 6 +-- .../access/vote/AbstractAclVoter.java | 7 ++- .../access/vote/AuthenticatedVoter.java | 6 +-- .../security/access/vote/RoleVoter.java | 2 +- .../AbstractAccessDecisionManagerTests.java | 2 +- .../access/vote/AbstractAclVoterTests.java | 2 +- .../security/access/vote/DenyAgainVoter.java | 2 +- .../security/access/vote/DenyVoter.java | 2 +- .../access/expression/WebExpressionVoter.java | 7 ++- 13 files changed, 45 insertions(+), 46 deletions(-) diff --git a/acl/acl.gradle b/acl/acl.gradle index 0aa25c3178..4fdd9c44db 100644 --- a/acl/acl.gradle +++ b/acl/acl.gradle @@ -2,6 +2,7 @@ dependencies { compile project(':spring-security-core'), + 'aopalliance:aopalliance:1.0', "net.sf.ehcache:ehcache:$ehcacheVersion", "org.springframework:spring-aop:$springVersion", "org.springframework:spring-context:$springVersion", diff --git a/acl/src/main/java/org/springframework/security/acls/AclEntryVoter.java b/acl/src/main/java/org/springframework/security/acls/AclEntryVoter.java index f41f902277..240f8e43e5 100644 --- a/acl/src/main/java/org/springframework/security/acls/AclEntryVoter.java +++ b/acl/src/main/java/org/springframework/security/acls/AclEntryVoter.java @@ -20,6 +20,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.List; +import org.aopalliance.intercept.MethodInvocation; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.security.access.AuthorizationServiceException; @@ -148,7 +149,7 @@ public class AclEntryVoter extends AbstractAclVoter { return (attribute.getAttribute() != null) && attribute.getAttribute().equals(getProcessConfigAttribute()); } - public int vote(Authentication authentication, Object object, Collection attributes) { + public int vote(Authentication authentication, MethodInvocation object, Collection attributes) { for(ConfigAttribute attr : attributes) { diff --git a/core/src/main/java/org/springframework/security/access/AccessDecisionVoter.java b/core/src/main/java/org/springframework/security/access/AccessDecisionVoter.java index e38bf1c935..cfc3715a3b 100644 --- a/core/src/main/java/org/springframework/security/access/AccessDecisionVoter.java +++ b/core/src/main/java/org/springframework/security/access/AccessDecisionVoter.java @@ -22,16 +22,14 @@ import org.springframework.security.core.Authentication; /** * Indicates a class is responsible for voting on authorization decisions. - * *

- * The coordination of voting (ie polling AccessDecisionVoters, + * The coordination of voting (ie polling {@code AccessDecisionVoter}s, * tallying their responses, and making the final authorization decision) is * performed by an {@link org.springframework.security.access.AccessDecisionManager}. - *

* * @author Ben Alex */ -public interface AccessDecisionVoter { +public interface AccessDecisionVoter { //~ Static fields/initializers ===================================================================================== int ACCESS_GRANTED = 1; @@ -41,20 +39,20 @@ public interface AccessDecisionVoter { //~ Methods ======================================================================================================== /** - * Indicates whether this AccessDecisionVoter is able to vote on the passed - * ConfigAttribute.

This allows the AbstractSecurityInterceptor to check every - * configuration attribute can be consumed by the configured AccessDecisionManager and/or - * RunAsManager and/or AfterInvocationManager.

+ * Indicates whether this {@code AccessDecisionVoter} is able to vote on the passed {@code ConfigAttribute}. + *

+ * This allows the {@code AbstractSecurityInterceptor} to check every configuration attribute can be consumed by + * the configured {@code AccessDecisionManager} and/or {@code RunAsManager} and/or {@code AfterInvocationManager}. * * @param attribute a configuration attribute that has been configured against the - * AbstractSecurityInterceptor + * {@code AbstractSecurityInterceptor} * - * @return true if this AccessDecisionVoter can support the passed configuration attribute + * @return true if this {@code AccessDecisionVoter} can support the passed configuration attribute */ boolean supports(ConfigAttribute attribute); /** - * Indicates whether the AccessDecisionVoter implementation is able to provide access control + * Indicates whether the {@code AccessDecisionVoter} implementation is able to provide access control * votes for the indicated secured object type. * * @param clazz the class that is being queried @@ -65,26 +63,27 @@ public interface AccessDecisionVoter { /** * Indicates whether or not access is granted. - *

The decision must be affirmative (ACCESS_GRANTED), negative (ACCESS_DENIED) - * or the AccessDecisionVoter can abstain (ACCESS_ABSTAIN) from voting. + *

+ * The decision must be affirmative ({@code ACCESS_GRANTED}), negative ({@code ACCESS_DENIED}) + * or the {@code AccessDecisionVoter} can abstain ({@code ACCESS_ABSTAIN}) from voting. * Under no circumstances should implementing classes return any other value. If a weighting of results is desired, * this should be handled in a custom {@link org.springframework.security.access.AccessDecisionManager} instead. - *

- *

Unless an AccessDecisionVoter is specifically intended to vote on an access control + *

+ * Unless an {@code AccessDecisionVoter} is specifically intended to vote on an access control * decision due to a passed method invocation or configuration attribute parameter, it must return - * ACCESS_ABSTAIN. This prevents the coordinating AccessDecisionManager from counting - * votes from those AccessDecisionVoters without a legitimate interest in the access control + * {@code ACCESS_ABSTAIN}. This prevents the coordinating {@code AccessDecisionManager} from counting + * votes from those {@code AccessDecisionVoter}s without a legitimate interest in the access control * decision. - *

- *

Whilst the method invocation is passed as a parameter to maximise flexibility in making access - * control decisions, implementing classes must never modify the behaviour of the method invocation (such as - * calling MethodInvocation.proceed()).

+ *

+ * Whilst the secured object (such as a {@code MethodInvocation}) is passed as a parameter to maximise flexibility + * in making access control decisions, implementing classes should not modify it or cause the represented invocation + * to take place (for example, by calling {@code MethodInvocation.proceed()}). * - * @param authentication the caller invoking the method - * @param object the secured object - * @param attributes the configuration attributes associated with the method being invoked + * @param authentication the caller making the invocation + * @param object the secured object being invoked + * @param attributes the configuration attributes associated with the secured object * * @return either {@link #ACCESS_GRANTED}, {@link #ACCESS_ABSTAIN} or {@link #ACCESS_DENIED} */ - int vote(Authentication authentication, Object object, Collection attributes); + int vote(Authentication authentication, S object, Collection attributes); } diff --git a/core/src/main/java/org/springframework/security/access/annotation/Jsr250Voter.java b/core/src/main/java/org/springframework/security/access/annotation/Jsr250Voter.java index 7e21b481bb..82f6948782 100644 --- a/core/src/main/java/org/springframework/security/access/annotation/Jsr250Voter.java +++ b/core/src/main/java/org/springframework/security/access/annotation/Jsr250Voter.java @@ -13,7 +13,7 @@ import org.springframework.security.core.GrantedAuthority; * @author Ryan Heaton * @since 2.0 */ -public class Jsr250Voter implements AccessDecisionVoter { +public class Jsr250Voter implements AccessDecisionVoter { /** * The specified config attribute is supported if its an instance of a {@link Jsr250SecurityConfig}. diff --git a/core/src/main/java/org/springframework/security/access/prepost/PreInvocationAuthorizationAdviceVoter.java b/core/src/main/java/org/springframework/security/access/prepost/PreInvocationAuthorizationAdviceVoter.java index b4e5b071b1..f2ab4d3482 100644 --- a/core/src/main/java/org/springframework/security/access/prepost/PreInvocationAuthorizationAdviceVoter.java +++ b/core/src/main/java/org/springframework/security/access/prepost/PreInvocationAuthorizationAdviceVoter.java @@ -21,7 +21,7 @@ import org.springframework.security.core.Authentication; * @author Luke Taylor * @since 3.0 */ -public class PreInvocationAuthorizationAdviceVoter implements AccessDecisionVoter { +public class PreInvocationAuthorizationAdviceVoter implements AccessDecisionVoter { protected final Log logger = LogFactory.getLog(getClass()); private final PreInvocationAuthorizationAdvice preAdvice; @@ -38,7 +38,7 @@ public class PreInvocationAuthorizationAdviceVoter implements AccessDecisionVote return clazz.isAssignableFrom(MethodInvocation.class); } - public int vote(Authentication authentication, Object object, Collection attributes) { + public int vote(Authentication authentication, MethodInvocation method, Collection attributes) { // Find prefilter and preauth (or combined) attributes // if both null, abstain @@ -51,7 +51,7 @@ public class PreInvocationAuthorizationAdviceVoter implements AccessDecisionVote return ACCESS_ABSTAIN; } - boolean allowed = preAdvice.before(authentication, (MethodInvocation)object, preAttr); + boolean allowed = preAdvice.before(authentication, method, preAttr); return allowed ? ACCESS_GRANTED : ACCESS_DENIED; } diff --git a/core/src/main/java/org/springframework/security/access/vote/AbstractAclVoter.java b/core/src/main/java/org/springframework/security/access/vote/AbstractAclVoter.java index 0c1efead41..b24ef142da 100644 --- a/core/src/main/java/org/springframework/security/access/vote/AbstractAclVoter.java +++ b/core/src/main/java/org/springframework/security/access/vote/AbstractAclVoter.java @@ -26,18 +26,17 @@ import org.springframework.util.Assert; * * @author Ben Alex */ -public abstract class AbstractAclVoter implements AccessDecisionVoter { +public abstract class AbstractAclVoter implements AccessDecisionVoter { //~ Instance fields ================================================================================================ private Class processDomainObjectClass; //~ Methods ======================================================================================================== - protected Object getDomainObjectInstance(Object secureObject) { + protected Object getDomainObjectInstance(MethodInvocation invocation) { Object[] args; Class[] params; - MethodInvocation invocation = (MethodInvocation) secureObject; params = invocation.getMethod().getParameterTypes(); args = invocation.getArguments(); @@ -47,7 +46,7 @@ public abstract class AbstractAclVoter implements AccessDecisionVoter { } } - throw new AuthorizationServiceException("Secure object: " + secureObject + throw new AuthorizationServiceException("MethodInvocation: " + invocation + " did not provide any argument of type: " + processDomainObjectClass); } diff --git a/core/src/main/java/org/springframework/security/access/vote/AuthenticatedVoter.java b/core/src/main/java/org/springframework/security/access/vote/AuthenticatedVoter.java index d4ea93d8b9..f08df32c74 100644 --- a/core/src/main/java/org/springframework/security/access/vote/AuthenticatedVoter.java +++ b/core/src/main/java/org/springframework/security/access/vote/AuthenticatedVoter.java @@ -41,7 +41,7 @@ import org.springframework.util.Assert; * * @author Ben Alex */ -public class AuthenticatedVoter implements AccessDecisionVoter { +public class AuthenticatedVoter implements AccessDecisionVoter { //~ Static fields/initializers ===================================================================================== public static final String IS_AUTHENTICATED_FULLY = "IS_AUTHENTICATED_FULLY"; @@ -77,9 +77,9 @@ public class AuthenticatedVoter implements AccessDecisionVoter { /** * This implementation supports any type of class, because it does not query the presented secure object. * - * @param clazz the secure object + * @param clazz the secure object type * - * @return always true + * @return always {@code true} */ public boolean supports(Class clazz) { return true; diff --git a/core/src/main/java/org/springframework/security/access/vote/RoleVoter.java b/core/src/main/java/org/springframework/security/access/vote/RoleVoter.java index 803692f6ff..29b5af2660 100644 --- a/core/src/main/java/org/springframework/security/access/vote/RoleVoter.java +++ b/core/src/main/java/org/springframework/security/access/vote/RoleVoter.java @@ -49,7 +49,7 @@ import org.springframework.security.core.GrantedAuthority; * @author Ben Alex * @author colin sampaleanu */ -public class RoleVoter implements AccessDecisionVoter { +public class RoleVoter implements AccessDecisionVoter { //~ Instance fields ================================================================================================ private String rolePrefix = "ROLE_"; diff --git a/core/src/test/java/org/springframework/security/access/vote/AbstractAccessDecisionManagerTests.java b/core/src/test/java/org/springframework/security/access/vote/AbstractAccessDecisionManagerTests.java index 0cabf9e636..25857ad7d1 100644 --- a/core/src/test/java/org/springframework/security/access/vote/AbstractAccessDecisionManagerTests.java +++ b/core/src/test/java/org/springframework/security/access/vote/AbstractAccessDecisionManagerTests.java @@ -150,7 +150,7 @@ public class AbstractAccessDecisionManagerTests extends TestCase { } } - private class MockStringOnlyVoter implements AccessDecisionVoter { + private class MockStringOnlyVoter implements AccessDecisionVoter { public boolean supports(Class clazz) { return String.class.isAssignableFrom(clazz); } diff --git a/core/src/test/java/org/springframework/security/access/vote/AbstractAclVoterTests.java b/core/src/test/java/org/springframework/security/access/vote/AbstractAclVoterTests.java index fa24e3dd45..8c0ad5954c 100644 --- a/core/src/test/java/org/springframework/security/access/vote/AbstractAclVoterTests.java +++ b/core/src/test/java/org/springframework/security/access/vote/AbstractAclVoterTests.java @@ -19,7 +19,7 @@ public class AbstractAclVoterTests { public boolean supports(ConfigAttribute attribute) { return false; } - public int vote(Authentication authentication, Object object, Collection attributes) { + public int vote(Authentication authentication, MethodInvocation object, Collection attributes) { return 0; } }; diff --git a/core/src/test/java/org/springframework/security/access/vote/DenyAgainVoter.java b/core/src/test/java/org/springframework/security/access/vote/DenyAgainVoter.java index 236fce6643..aaa5affae6 100644 --- a/core/src/test/java/org/springframework/security/access/vote/DenyAgainVoter.java +++ b/core/src/test/java/org/springframework/security/access/vote/DenyAgainVoter.java @@ -32,7 +32,7 @@ import java.util.Iterator; * * @author Ben Alex */ -public class DenyAgainVoter implements AccessDecisionVoter { +public class DenyAgainVoter implements AccessDecisionVoter { // ~ Methods // ======================================================================================================== diff --git a/core/src/test/java/org/springframework/security/access/vote/DenyVoter.java b/core/src/test/java/org/springframework/security/access/vote/DenyVoter.java index a660703309..e621000081 100644 --- a/core/src/test/java/org/springframework/security/access/vote/DenyVoter.java +++ b/core/src/test/java/org/springframework/security/access/vote/DenyVoter.java @@ -30,7 +30,7 @@ import java.util.Iterator; * * @author Ben Alex */ -public class DenyVoter implements AccessDecisionVoter { +public class DenyVoter implements AccessDecisionVoter { //~ Methods ======================================================================================================== public boolean supports(ConfigAttribute attribute) { diff --git a/web/src/main/java/org/springframework/security/web/access/expression/WebExpressionVoter.java b/web/src/main/java/org/springframework/security/web/access/expression/WebExpressionVoter.java index 8a2e80e7f4..916a23451a 100644 --- a/web/src/main/java/org/springframework/security/web/access/expression/WebExpressionVoter.java +++ b/web/src/main/java/org/springframework/security/web/access/expression/WebExpressionVoter.java @@ -15,12 +15,12 @@ import org.springframework.security.web.FilterInvocation; * @author Luke Taylor * @since 3.0 */ -public class WebExpressionVoter implements AccessDecisionVoter { +public class WebExpressionVoter implements AccessDecisionVoter { private SecurityExpressionHandler expressionHandler = new DefaultWebSecurityExpressionHandler(); - public int vote(Authentication authentication, Object object, Collection attributes) { + public int vote(Authentication authentication, FilterInvocation fi, Collection attributes) { assert authentication != null; - assert object != null; + assert fi != null; assert attributes != null; WebExpressionConfigAttribute weca = findConfigAttribute(attributes); @@ -29,7 +29,6 @@ public class WebExpressionVoter implements AccessDecisionVoter { return ACCESS_ABSTAIN; } - FilterInvocation fi = (FilterInvocation)object; EvaluationContext ctx = expressionHandler.createEvaluationContext(authentication, fi); return ExpressionUtils.evaluateAsBoolean(weca.getAuthorizeExpression(), ctx) ?