CCM NG: PermissionManager finished

git-svn-id: https://svn.libreccm.org/ccm/ccm_ng@3548 8810af33-2d31-482b-a856-94f89814c4df

ccm-core/src/main/java/org/libreccm/core/PermissionManager.java

@@ -31,17 +31,23 @@ import javax.inject.Inject;
 public class PermissionManager {
 
     @Inject
+    @SuppressWarnings("PMD.LongVariable") //Nothing wrong with this name
     private transient PermissionRepository permissionRepository;
 
     @Inject
+    @SuppressWarnings("PMD.LongVariable") //Nothing wrong with this name
     private transient PrivilegeRepository privilegeRepository;
 
     @Inject
+    @SuppressWarnings("PMD.LongVariable") //Nothing wrong with this name
     private transient CcmObjectRepository ccmObjectRepository;
 
     @Inject
     private transient SubjectRepository subjectRepository;
 
+    @Inject
+    private transient UserRepository userRepository;
+
     /**
      * Creates a new permission granting the provided {@code privilege} on the
      * provided {@code object} to the provided {@code subject}. If the
@@ -57,6 +63,16 @@ public class PermissionManager {
     public void grantPermission(final Privilege privilege,
                                 final CcmObject object,
                                 final Subject subject) {
+        if (privilege == null) {
+            throw new IllegalArgumentException(
+                "Illegal argument 'null' provided for parameter privilege.");
+        }
+
+        if (subject == null) {
+            throw new IllegalArgumentException(
+                "Illegal argument 'null' provided for parameter subject");
+        }
+
         if (!isPermitted(privilege, object, subject)) {
             final Permission permission = new Permission();
             permission.setGrantedPrivilege(privilege);
@@ -110,6 +126,11 @@ public class PermissionManager {
      * Checks if the the provided {@code subject} has a permission granting the
      * provided {@code privilege} on the provided {@code object}.
      *
+     * The method will also check if the subject has a permission granting the
+     * {@code admin} privilege on the provided object or on all objects. The
+     * {@code admin} privilege implies that the user is granted all other
+     * privileges on the object.
+     *
      * If the provided subject is {@code null} the method will try to retrieve
      * the public user from the database. If there is no public user the method
      * will return {@code false}.
@@ -128,53 +149,105 @@ public class PermissionManager {
     public boolean isPermitted(final Privilege privilege,
                                final CcmObject object,
                                final Subject subject) {
-        if (subject instanceof User) {
+        if (privilege == null) {
-            return isPermitted(privilege, object, (User) subject);
+            throw new IllegalArgumentException(
-        } else if (subject instanceof Group) {
+                "Illegal value 'null' provided for parameter privilege");
-            return isPermitted(privilege, object, (Group) subject);
+        }
+
+        //Check if the current subject is null. If yes try to retrieve the 
+        //public user
+        Subject subjectObj = null;
+        if (subject == null) {
+            final User publicUser = userRepository.retrievePublicUser();
+
+            if (publicUser == null) {
+
+                 //If the public user is not available an null value for the 
+                //subject parameter is an illegal argument.
+                throw new IllegalArgumentException(
+                    "Illegal value 'null' provided for parameter privilege");
             } else {
+                subjectObj = publicUser;
+            }
+        } else {
+            //Subject is not null. Use provided subject
+            subjectObj = subject;
+        }
+
+        //Depending if the subject is a user or a group delegate to the correct
+        //method. For users we have to check the permissions of the groups the
+        //user is member of also, for group we don't have.
+        if (subjectObj instanceof User) {
+            return isPermitted(privilege, object, (User) subjectObj);
+        } else if (subjectObj instanceof Group) {
+            return isPermitted(privilege, object, (Group) subjectObj);
+        } else {
+            //For unknown subclasses of subject return false.
             return false;
         }
     }
 
-    public boolean isPermitted(final Privilege privilege,
+    /**
+     * Checks if a user has a permission granting a privilege on an object. If
+     * the provided {@code object} is {@code null} the method will only check
+     * for wildcard permission (permissions for all objects).
+     *
+     * @param privilege The privilege. Can't be null.
+     * @param object    The object. Can be null.
+     * @param user      The user. Can't be null.
+     *
+     * @return {@code true} if the provided {@code user} has a permission
+     *         granting the provided privilege for the provided object,
+     *         {@code false} if not.
+     */
+    @SuppressWarnings("PMD.CyclomaticComplexity")
+    private boolean isPermitted(final Privilege privilege,
                                 final CcmObject object,
                                 final User user) {
-        boolean result;
+        boolean result = false;
+        final Privilege admin = privilegeRepository.retrievePrivilege(
+            Privilege.ADMIN);
 
-        final List<Permission> directPermissions = permissionRepository
+        //Check if the there is direct permission granting the provided
-            .findPermissionsForUserPrivilegeAndObject(user, privilege, object);
+        //privilege on the provided object. If the object is null or the
-        result = !directPermissions.isEmpty();
+        //privilege is the admin privilege this check is not performed.
+        if (object != null && !privilege.equals(admin)) {
+            final List<Permission> permissions = permissionRepository
+                .findPermissionsForUserPrivilegeAndObject(user, privilege,
+                                                          object);
+            result = !permissions.isEmpty();
+        }
 
-        if (!result) {
+        //Check for an wildcard permission (a permission for all objects) for 
+        //the provided privilege. This check is only performed when result is
+        //still false the provided privilege is not the admin privilege.
+        if (!result && !privilege.equals(admin)) {
             final List<Permission> permissions = permissionRepository
                 .findPermissionsForUserPrivilegeAndObject(user, privilege, null);
             result = !permissions.isEmpty();
         }
 
-        if (!result) {
+        //Check for a permission granting the admin privilege on the provided 
-            final Privilege admin = privilegeRepository.retrievePrivilege(
+        //object. This check is only performed if result is still false, 
-                "admin");
+        //the admin variable is not null (null means that there is no admin 
-            if (admin != null) {
+        //privilege) and the provided object is not null.
+        if (!result && admin != null && object != null) {
             final List<Permission> permissions = permissionRepository
                 .findPermissionsForUserPrivilegeAndObject(user,
-                                                              privilege,
+                                                          admin,
                                                           object);
             result = !permissions.isEmpty();
         }
-        }
 
-        if (!result) {
+        //Check for a permission granting the admin privilege systemwide. This
-            final Privilege admin = privilegeRepository.retrievePrivilege(
+        //check in only performed if result is still false and admin is not null.
-                "admin");
+        if (!result && admin != null) {
-            if (admin != null) {
             final List<Permission> permissions = permissionRepository
                 .findPermissionsForUserPrivilegeAndObject(user,
-                                                              privilege,
+                                                          admin,
                                                           null);
             result = !permissions.isEmpty();
         }
-        }
 
         return result;
     }

ccm-core/src/main/java/org/libreccm/core/Privilege.java

@@ -63,6 +63,9 @@ public class Privilege implements Serializable {
 
     private static final long serialVersionUID = -3986038536996049440L;
     
+    //Constant for the admin privilege.
+    public static final String ADMIN = "admin";
+
     @Id
     @Column(name = "privilege_id")
     @GeneratedValue(strategy = GenerationType.AUTO)

ccm-core/src/test/java/org/libreccm/core/PermissionManagerTest.java

@@ -208,7 +208,7 @@ public class PermissionManagerTest {
     @UsingDataSet("datasets/org/libreccm/core/PermissionManagerTest/"
                       + "data.json")
     @InSequence(10)
-    public void isPermittedWebmaster() {
+    public void isPermittedWebmasterAdmin() {
         final User webmaster = userRepository.findByScreenName("webmaster");
         final Map<String, CcmObject> testObjects = retrieveTestObjects();
         final Map<String, Privilege> privileges = retrievePrivileges();
@@ -224,10 +224,55 @@ public class PermissionManagerTest {
         expected.put(testObjects.get(TEST_OBJECT_8), true);
 
         verifyIsPermitted(webmaster, privileges.get(ADMIN), expected);
+    }
+    
+    @Test
+    @UsingDataSet("datasets/org/libreccm/core/PermissionManagerTest/"
+                      + "data.json")
+    @InSequence(11)
+    public void isPermittedWebmasterRead() {
+        final User webmaster = userRepository.findByScreenName("webmaster");
+        final Map<String, CcmObject> testObjects = retrieveTestObjects();
+        final Map<String, Privilege> privileges = retrievePrivileges();
+
+        final Map<CcmObject, Boolean> expected = new LinkedHashMap<>();
+        expected.put(testObjects.get(TEST_OBJECT_1), true);
+        expected.put(testObjects.get(TEST_OBJECT_2), true);
+        expected.put(testObjects.get(TEST_OBJECT_3), true);
+        expected.put(testObjects.get(TEST_OBJECT_4), true);
+        expected.put(testObjects.get(TEST_OBJECT_5), true);
+        expected.put(testObjects.get(TEST_OBJECT_6), true);
+        expected.put(testObjects.get(TEST_OBJECT_7), true);
+        expected.put(testObjects.get(TEST_OBJECT_8), true);
+
         verifyIsPermitted(webmaster, privileges.get(READ), expected);
+    }
+
+        @Test
+    @UsingDataSet("datasets/org/libreccm/core/PermissionManagerTest/"
+                      + "data.json")
+    @InSequence(12)
+    public void isPermittedWebmasterWrite() {
+        final User webmaster = userRepository.findByScreenName("webmaster");
+        final Map<String, CcmObject> testObjects = retrieveTestObjects();
+        final Map<String, Privilege> privileges = retrievePrivileges();
+
+        final Map<CcmObject, Boolean> expected = new LinkedHashMap<>();
+        expected.put(testObjects.get(TEST_OBJECT_1), true);
+        expected.put(testObjects.get(TEST_OBJECT_2), true);
+        expected.put(testObjects.get(TEST_OBJECT_3), true);
+        expected.put(testObjects.get(TEST_OBJECT_4), true);
+        expected.put(testObjects.get(TEST_OBJECT_5), true);
+        expected.put(testObjects.get(TEST_OBJECT_6), true);
+        expected.put(testObjects.get(TEST_OBJECT_7), true);
+        expected.put(testObjects.get(TEST_OBJECT_8), true);
+
         verifyIsPermitted(webmaster, privileges.get(WRITE), expected);
     }
 
+    
+    
+    
     @Test
     @UsingDataSet("datasets/org/libreccm/core/PermissionManagerTest/"
                       + "data.json")
@@ -405,17 +450,16 @@ public class PermissionManagerTest {
         permissionManager.isPermitted(null, object, user);
     }
 
-    @Test(expected = IllegalArgumentException.class)
+    @Test
     @UsingDataSet("datasets/org/libreccm/core/PermissionManagerTest/"
                       + "data.json")
-    @ShouldThrowException(IllegalArgumentException.class)
     @InSequence(80)
     public void isPermittedNullObject() {
         final Privilege privilege = privilegeRepository
             .retrievePrivilege(READ);
         final User user = userRepository.findByScreenName("webmaster");
 
-        permissionManager.isPermitted(privilege, null, user);
+        assertThat(permissionManager.isPermitted(privilege, null, user), is(true));
     }
 
     @Test
@@ -458,10 +502,9 @@ public class PermissionManagerTest {
         permissionManager.checkPermission(null, object, user);
     }
 
-    @Test(expected = IllegalArgumentException.class)
+    @Test
     @UsingDataSet("datasets/org/libreccm/core/PermissionManagerTest/"
                       + "data.json")
-    @ShouldThrowException(IllegalArgumentException.class)
     @InSequence(130)
     public void checkPermissionNullObject() throws UnauthorizedAcccessException {
         final Privilege privilege = privilegeRepository
@@ -471,10 +514,9 @@ public class PermissionManagerTest {
         permissionManager.checkPermission(privilege, null, user);
     }
 
-    @Test(expected = IllegalArgumentException.class)
+    @Test
     @UsingDataSet("datasets/org/libreccm/core/PermissionManagerTest/"
                       + "data.json")
-    @ShouldThrowException(IllegalArgumentException.class)
     @InSequence(140)
     public void checkPermissionNullSubject() throws UnauthorizedAcccessException {
         final Privilege privilege = privilegeRepository
@@ -487,8 +529,9 @@ public class PermissionManagerTest {
     @Test
     @UsingDataSet("datasets/org/libreccm/core/PermissionManagerTest/"
                       + "data.json")
-    @ShouldMatchDataSet("datasets/org/libreccm/core/PermissionManagerTest/"
+    @ShouldMatchDataSet(value = "datasets/org/libreccm/core/"
-                            + "after-grant.json")
+                        + "PermissionManagerTest/after-grant.json",
+                        excludeColumns = {"permission_id"})
     @InSequence(150)
     public void grantPermission() {
         final Privilege read = privilegeRepository.retrievePrivilege(READ);
@@ -509,8 +552,9 @@ public class PermissionManagerTest {
     @Test
     @UsingDataSet("datasets/org/libreccm/core/PermissionManagerTest/"
                       + "data.json")
-    @ShouldMatchDataSet("datasets/org/libreccm/core/PermissionManagerTest/"
+    @ShouldMatchDataSet(value = "datasets/org/libreccm/core/"
-                            + "after-grant-wildcard.json")
+                        + "PermissionManagerTest/after-grant-wildcard.json",
+                        excludeColumns = {"permission_id"})
     @InSequence(160)
     public void grantWildcardPermission() {
         final Privilege read = privilegeRepository.retrievePrivilege(READ);
@@ -546,8 +590,9 @@ public class PermissionManagerTest {
     @Test
     @UsingDataSet("datasets/org/libreccm/core/PermissionManagerTest/"
                       + "data.json")
-    @ShouldMatchDataSet("datasets/org/libreccm/core/PermissionManagerTest/"
+    @ShouldMatchDataSet(value = "datasets/org/libreccm/core/"
-                            + "after-revoke.json")
+                        + "PermissionManagerTest/after-revoke.json",
+                        excludeColumns = {"permission_id"})
     @InSequence(190)
     public void revokePermission() {
         final Privilege read = privilegeRepository.retrievePrivilege(READ);

ccm-core/src/test/resources/datasets/org/libreccm/core/PermissionManagerTest/after-grant-wildcard.json

@@ -301,13 +301,13 @@
                 {
                     "permission_id": -250,
                     "grantee_id": -50,
-                    "object_id": -70,
+                    "object_id": -60,
                     "granted_privilege_id": -20
                 },
                 {
                     "permission_id": -260,
                     "grantee_id": -50,
-                    "object_id": -70,
+                    "object_id": -60,
                     "granted_privilege_id": -30
                 },
                 {

ccm-core/src/test/resources/datasets/org/libreccm/core/PermissionManagerTest/after-grant.json

@@ -208,12 +208,6 @@
                     "object_id": -60,
                     "granted_privilege_id": -20
                 },
-                {
-                    "permission_id": -70,
-                    "grantee_id": -50,
-                    "object_id": -60,
-                    "granted_privilege_id": -30
-                },
                 {
                     "permission_id": -80,
                     "grantee_id": -30,
@@ -303,6 +297,36 @@
                     "grantee_id": -40,
                     "object_id": -80,
                     "granted_privilege_id": -30
+                },
+                {
+                    "permission_id": -250,
+                    "grantee_id": -50,
+                    "object_id": -60,
+                    "granted_privilege_id": -20
+                },
+                {
+                    "permission_id": -260,
+                    "grantee_id": -50,
+                    "object_id": -60,
+                    "granted_privilege_id": -30
+                },
+                {
+                    "permission_id": -270,
+                    "grantee_id": -10,
+                    "object_id": -60,
+                    "granted_privilege_id": -20
+                },
+                {
+                    "permission_id": -280,
+                    "grantee_id": -50,
+                    "object_id": -70,
+                    "granted_privilege_id": -20
+                },
+                {
+                    "permission_id": -290,
+                    "grantee_id": -50,
+                    "object_id": -70,
+                    "granted_privilege_id": -30
                 }
             ]
 }

ccm-core/src/test/resources/datasets/org/libreccm/core/PermissionManagerTest/after-revoke.json

@@ -184,12 +184,6 @@
                     "object_id": -50,
                     "granted_privilege_id": -20
                 },
-                {
-                    "permission_id": -30,
-                    "grantee_id": -10,
-                    "object_id": -50,
-                    "granted_privilege_id": -20
-                },
                 {
                     "permission_id": -40,
                     "grantee_id": -50,
@@ -208,12 +202,6 @@
                     "object_id": -60,
                     "granted_privilege_id": -20
                 },
-                {
-                    "permission_id": -70,
-                    "grantee_id": -50,
-                    "object_id": -60,
-                    "granted_privilege_id": -30
-                },
                 {
                     "permission_id": -80,
                     "grantee_id": -30,
@@ -304,23 +292,11 @@
                     "object_id": -80,
                     "granted_privilege_id": -30
                 },
-                {
-                    "permission_id": -240,
-                    "grantee_id": -10,
-                    "object_id": -60,
-                    "granted_privilege_id": -20
-                },
                 {
                     "permission_id": -250,
                     "grantee_id": -50,
-                    "object_id": -70,
+                    "object_id": -60,
                     "granted_privilege_id": -20
-                },
-                {
-                    "permission_id": -260,
-                    "grantee_id": -50,
-                    "object_id": -70,
-                    "granted_privilege_id": -30
                 }
             ]
 }

ccm-core/src/test/resources/datasets/org/libreccm/core/PermissionManagerTest/data.json

@@ -301,13 +301,13 @@
                 {
                     "permission_id": -250,
                     "grantee_id": -50,
-                    "object_id": -70,
+                    "object_id": -60,
                     "granted_privilege_id": -20
                 },
                 {
                     "permission_id": -260,
                     "grantee_id": -50,
-                    "object_id": -70,
+                    "object_id": -60,
                     "granted_privilege_id": -30
                 }
             ]