Browse Source

SEC-1023: Added support for hasPermission() based on Id and type

Luke Taylor 17 years ago
parent
commit
d33b13e52e

+ 16 - 2
acl/src/main/java/org/springframework/security/acls/AclPermissionEvaluator.java

@@ -1,10 +1,13 @@
 package org.springframework.security.acls;
 
+import java.io.Serializable;
+
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.springframework.security.Authentication;
 import org.springframework.security.acls.domain.BasePermission;
 import org.springframework.security.acls.objectidentity.ObjectIdentity;
+import org.springframework.security.acls.objectidentity.ObjectIdentityGenerator;
 import org.springframework.security.acls.objectidentity.ObjectIdentityRetrievalStrategy;
 import org.springframework.security.acls.objectidentity.ObjectIdentityRetrievalStrategyImpl;
 import org.springframework.security.acls.sid.Sid;
@@ -27,6 +30,7 @@ public class AclPermissionEvaluator implements PermissionEvaluator {
 
     private AclService aclService;
     private ObjectIdentityRetrievalStrategy objectIdentityRetrievalStrategy = new ObjectIdentityRetrievalStrategyImpl();
+    private ObjectIdentityGenerator objectIdentityGenerator = new ObjectIdentityRetrievalStrategyImpl();
     private SidRetrievalStrategy sidRetrievalStrategy = new SidRetrievalStrategyImpl();
 
     public AclPermissionEvaluator(AclService aclService) {
@@ -45,13 +49,23 @@ public class AclPermissionEvaluator implements PermissionEvaluator {
 
         ObjectIdentity objectIdentity = objectIdentityRetrievalStrategy.getObjectIdentity(domainObject);
 
+        return checkPermission(authentication, objectIdentity, permission);
+    }
+
+    public boolean hasPermission(Authentication authentication, Serializable targetId, String targetType, Object permission) {
+        ObjectIdentity objectIdentity = objectIdentityGenerator.createObjectIdentity(targetId, targetType);
+
+        return checkPermission(authentication, objectIdentity, permission);
+    }
+
+    private boolean checkPermission(Authentication authentication, ObjectIdentity oid, Object permission) {
         // Obtain the SIDs applicable to the principal
         Sid[] sids = sidRetrievalStrategy.getSids(authentication);
         Permission[] requiredPermission = resolvePermission(permission);
 
         try {
             // Lookup only ACLs for SIDs we're interested in
-            Acl acl = aclService.readAclById(objectIdentity, sids);
+            Acl acl = aclService.readAclById(oid, sids);
 
             if (acl.isGranted(requiredPermission, sids, false)) {
                 if (logger.isDebugEnabled()) {
@@ -72,6 +86,7 @@ public class AclPermissionEvaluator implements PermissionEvaluator {
         }
 
         return false;
+
     }
 
     // TODO: Add permission resolver/PermissionFactory rewrite
@@ -111,5 +126,4 @@ public class AclPermissionEvaluator implements PermissionEvaluator {
     public void setSidRetrievalStrategy(SidRetrievalStrategy sidRetrievalStrategy) {
         this.sidRetrievalStrategy = sidRetrievalStrategy;
     }
-
 }

+ 23 - 0
acl/src/main/java/org/springframework/security/acls/objectidentity/ObjectIdentityGenerator.java

@@ -0,0 +1,23 @@
+package org.springframework.security.acls.objectidentity;
+
+import java.io.Serializable;
+
+/**
+ * Strategy which creates an <tt>ObjectIdentity</tt> from object identity and type information.
+ * Used in situations when the actual object instance isn't available.
+ *
+ * @author Luke Taylor
+ * @version $Id$
+ * @since 2.5
+ */
+public interface ObjectIdentityGenerator {
+
+    /**
+     *
+     * @param id the identifier of the domain object, not null
+     * @param type the type of the object (usually a class name), not null
+     * @return
+     */
+    ObjectIdentity createObjectIdentity(Serializable id, String type);
+
+}

+ 2 - 2
acl/src/main/java/org/springframework/security/acls/objectidentity/ObjectIdentityImpl.java

@@ -63,7 +63,7 @@ public class ObjectIdentityImpl implements ObjectIdentity {
         this.identifier = identifier;
     }
 
-/**
+    /**
      * Creates the <code>ObjectIdentityImpl</code> based on the passed
      * object instance. The passed object must provide a <code>getId()</code>
      * method, otherwise an exception will be thrown. The object passed will
@@ -98,7 +98,7 @@ public class ObjectIdentityImpl implements ObjectIdentity {
     /**
      * Important so caching operates properly.<P>Considers an object of the same class equal if it has the same
      * <code>classname</code> and <code>id</code> properties.</p>
-     * 
+     *
      * <p>
      * Note that this class uses string equality for the identifier field, which ensures it better supports
      * differences between {@link LookupStrategy} requirements and the domain object represented by this

+ 9 - 3
acl/src/main/java/org/springframework/security/acls/objectidentity/ObjectIdentityRetrievalStrategyImpl.java

@@ -15,17 +15,23 @@
 
 package org.springframework.security.acls.objectidentity;
 
+import java.io.Serializable;
+
 /**
- * Basic implementation of {@link ObjectIdentityRetrievalStrategy} that uses the constructor of {@link
- * ObjectIdentityImpl} to create the {@link ObjectIdentity}.
+ * Basic implementation of {@link ObjectIdentityRetrievalStrategy} and <tt>ObjectIdentityGenerator</tt>
+ * that uses the constructors of {@link ObjectIdentityImpl} to create the {@link ObjectIdentity}.
  *
  * @author Ben Alex
  * @version $Id$
  */
-public class ObjectIdentityRetrievalStrategyImpl implements ObjectIdentityRetrievalStrategy {
+public class ObjectIdentityRetrievalStrategyImpl implements ObjectIdentityRetrievalStrategy, ObjectIdentityGenerator {
     //~ Methods ========================================================================================================
 
     public ObjectIdentity getObjectIdentity(Object domainObject) {
         return new ObjectIdentityImpl(domainObject);
     }
+
+    public ObjectIdentity createObjectIdentity(Serializable id, String type) {
+        return new ObjectIdentityImpl(type, id);
+    }
 }

+ 10 - 0
core/src/main/java/org/springframework/security/expression/DenyAllPermissionEvaluator.java

@@ -1,5 +1,7 @@
 package org.springframework.security.expression;
 
+import java.io.Serializable;
+
 import org.springframework.security.Authentication;
 
 /**
@@ -19,4 +21,12 @@ public class DenyAllPermissionEvaluator implements PermissionEvaluator {
         return false;
     }
 
+    /**
+     * @return false always
+     */
+    public boolean hasPermission(Authentication authentication, Serializable targetId, String targetType,
+                    Object permission) {
+        return false;
+    }
+
 }

+ 14 - 0
core/src/main/java/org/springframework/security/expression/PermissionEvaluator.java

@@ -1,5 +1,7 @@
 package org.springframework.security.expression;
 
+import java.io.Serializable;
+
 import org.springframework.security.Authentication;
 
 /**
@@ -22,4 +24,16 @@ public interface PermissionEvaluator {
      * @return true if the permission is granted, false otherwise
      */
     boolean hasPermission(Authentication authentication, Object targetDomainObject, Object permission);
+
+    /**
+     * Alternative method for evaluating a permission where only the identifier of the target object
+     * is available, rather than the target instance itself.
+     *
+     * @param authentication represents the user in question. Should not be null.
+     * @param targetId the identifier for the object instance (usually a Long)
+     * @param targetType a String representing the target's type (usually a Java classname). Not null.
+     * @param permission a representation of the permission object as supplied by the expression system. Not null.
+     * @return true if the permission is granted, false otherwise
+     */
+    boolean hasPermission(Authentication authentication, Serializable targetId, String targetType, Object permission);
 }

+ 6 - 0
core/src/main/java/org/springframework/security/expression/SecurityExpressionRoot.java

@@ -1,5 +1,6 @@
 package org.springframework.security.expression;
 
+import java.io.Serializable;
 import java.util.Set;
 
 import org.springframework.security.Authentication;
@@ -7,6 +8,7 @@ import org.springframework.security.AuthenticationTrustResolver;
 import org.springframework.security.GrantedAuthority;
 import org.springframework.security.util.AuthorityUtils;
 
+
 /**
  * Default root object for use in Spring Security expression evaluations.
  *
@@ -87,6 +89,10 @@ public class SecurityExpressionRoot {
         return permissionEvaluator.hasPermission(authentication, target, permission);
     }
 
+    public boolean hasPermission(Object targetId, String targetType, Object permission) {
+        return permissionEvaluator.hasPermission(authentication, (Serializable)targetId, targetType, permission);
+    }
+
     public Authentication getAuthentication() {
         return authentication;
     }

+ 63 - 11
core/src/test/java/org/springframework/security/expression/SecurityExpressionRootTests.java

@@ -2,14 +2,16 @@ package org.springframework.security.expression;
 
 import static org.junit.Assert.*;
 
+import org.jmock.Expectations;
+import org.jmock.Mockery;
 import org.junit.Before;
 import org.junit.Test;
 import org.springframework.expression.Expression;
 import org.springframework.expression.spel.SpelExpressionParser;
 import org.springframework.expression.spel.standard.StandardEvaluationContext;
 import org.springframework.security.Authentication;
+import org.springframework.security.AuthenticationTrustResolver;
 import org.springframework.security.expression.SecurityExpressionRoot;
-import org.springframework.security.providers.UsernamePasswordAuthenticationToken;
 
 
 /**
@@ -20,15 +22,21 @@ import org.springframework.security.providers.UsernamePasswordAuthenticationToke
  */
 public class SecurityExpressionRootTests {
     SpelExpressionParser parser = new SpelExpressionParser();
-    UsernamePasswordAuthenticationToken joe = new UsernamePasswordAuthenticationToken("joe", "password");
     SecurityExpressionRoot root;
     StandardEvaluationContext ctx;
+    Mockery jmock = new Mockery();
+    private AuthenticationTrustResolver trustResolver;
+    private Authentication user;
+
 
     @Before
     public void createContext() {
-        root = new SecurityExpressionRoot(joe);
+        user = jmock.mock(Authentication.class);
+        root = new SecurityExpressionRoot(user);
         ctx = new StandardEvaluationContext();
         ctx.setRootObject(root);
+        trustResolver = jmock.mock(AuthenticationTrustResolver.class);
+        root.setTrustResolver(trustResolver);
     }
 
     @Test
@@ -40,24 +48,68 @@ public class SecurityExpressionRootTests {
     }
 
     @Test
-    public void hasPermissionWorksWithIntegerExpressions() throws Exception {
+    public void isAnonymousReturnsTrueIfTrustResolverReportsAnonymous() {
+        jmock.checking(new Expectations() {{
+            oneOf(trustResolver).isAnonymous(user); will(returnValue(true));
+        }});
+        assertTrue(root.isAnonymous());
+    }
+
+    @Test
+    public void isAnonymousReturnsFalseIfTrustResolverReportsNonAnonymous() {
+        jmock.checking(new Expectations() {{
+            oneOf(trustResolver).isAnonymous(user); will(returnValue(false));
+        }});
+        assertFalse(root.isAnonymous());
+    }
+
+    @Test
+    public void hasPermissionOnDomainObjectReturnsFalseIfPermissionEvaluatorDoes() throws Exception {
         final Object dummyDomainObject = new Object();
+        final PermissionEvaluator pe = jmock.mock(PermissionEvaluator.class);
         ctx.setVariable("domainObject", dummyDomainObject);
+        root.setPermissionEvaluator(pe);
+        jmock.checking(new Expectations() {{
+            oneOf(pe).hasPermission(user, dummyDomainObject, "ignored"); will(returnValue(false));
+        }});
 
-        root.setPermissionEvaluator(new PermissionEvaluator () {
-            public boolean hasPermission(Authentication authentication, Object targetDomainObject, Object permission) {
-                // Check the correct target object is passed in
-                assertEquals(dummyDomainObject, targetDomainObject);
+        assertFalse(root.hasPermission(dummyDomainObject, "ignored"));
+    }
+
+    @Test
+    public void hasPermissionOnDomainObjectReturnsTrueIfPermissionEvaluatorDoes() throws Exception {
+        final Object dummyDomainObject = new Object();
+        final PermissionEvaluator pe = jmock.mock(PermissionEvaluator.class);
+        ctx.setVariable("domainObject", dummyDomainObject);
+        root.setPermissionEvaluator(pe);
+        jmock.checking(new Expectations() {{
+            oneOf(pe).hasPermission(user, dummyDomainObject, "ignored"); will(returnValue(true));
+        }});
+
+        assertTrue(root.hasPermission(dummyDomainObject, "ignored"));
+    }
+
+
+    @Test
+    public void hasPermissionOnDomainObjectWorksWithIntegerExpressions() throws Exception {
+        final Object dummyDomainObject = new Object();
+        ctx.setVariable("domainObject", dummyDomainObject);
+        final PermissionEvaluator pe = jmock.mock(PermissionEvaluator.class);
+        root.setPermissionEvaluator(pe);
 
-                return permission instanceof Integer && ((Integer)permission).intValue() == 10;
-            }
-        });
+        jmock.checking(new Expectations() {{
+            exactly(3).of(pe).hasPermission(with(user), with(dummyDomainObject), with(any(Integer.class)));
+                will(onConsecutiveCalls(returnValue(true), returnValue(true), returnValue(false)));
+        }});
 
         Expression e = parser.parseExpression("hasPermission(#domainObject, 0xA)");
+        // evaluator returns true
         assertTrue(ExpressionUtils.evaluateAsBoolean(e, ctx));
         e = parser.parseExpression("hasPermission(#domainObject, 10)");
+     // evaluator returns true
         assertTrue(ExpressionUtils.evaluateAsBoolean(e, ctx));
         e = parser.parseExpression("hasPermission(#domainObject, 0xFF)");
+     // evaluator returns false, make sure return value matches
         assertFalse(ExpressionUtils.evaluateAsBoolean(e, ctx));
     }
 }