move assign and deassign checks to each descendant instead of all

usnistgov · Sep 7, 2024 · c12b180 · c12b180
1 parent a80314e
commit c12b180
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 44 deletions.
diff --git a/src/main/java/gov/nist/csd/pm/pap/GraphModifier.java b/src/main/java/gov/nist/csd/pm/pap/GraphModifier.java
@@ -87,22 +87,22 @@ public void deleteNode(String name) throws PMException {
 
     @Override
     public void assign(String ascendant, Collection<String> descendants) throws PMException {
-        if(!checkAssignInput(ascendant, descendants)) {
-            return;
-        }
-
         for (String descendant : descendants) {
+            if(!checkAssignInput(ascendant, descendant)) {
+                continue;
+            }
+
             store.graph().createAssignment(ascendant, descendant);
         }
     }
 
     @Override
     public void deassign(String ascendant, Collection<String> descendants) throws PMException {
-        if(!checkDeassignInput(ascendant, descendants)) {
-            return;
-        }
-
         for (String descendant : descendants) {
+            if(!checkDeassignInput(ascendant, descendant)) {
+                continue;
+            }
+
             store.graph().deleteAssignment(ascendant, descendant);
         }
     }
@@ -326,29 +326,27 @@ private boolean checkPatternForNode(String entity, Pattern pattern) {
      * @return True if the execution should proceed, false otherwise.
      * @throws PMException If any PM related exceptions occur in the implementing class.
      */
-    protected boolean checkAssignInput(String ascendant, Collection<String> descendants) throws PMException {
-        for (String descendant : descendants) {
-            // getting both nodes will check if they exist
-            if (!store.graph().nodeExists(ascendant)) {
-                throw new NodeDoesNotExistException(ascendant);
-            } else if (!store.graph().nodeExists(descendant)) {
-                throw new NodeDoesNotExistException(descendant);
-            }
+    protected boolean checkAssignInput(String ascendant, String descendant) throws PMException {
+        // getting both nodes will check if they exist
+        if (!store.graph().nodeExists(ascendant)) {
+            throw new NodeDoesNotExistException(ascendant);
+        } else if (!store.graph().nodeExists(descendant)) {
+            throw new NodeDoesNotExistException(descendant);
+        }
 
-            // ignore if assignment already exists
-            if (store.graph().getAdjacentDescendants(ascendant).contains(descendant)) {
-                return false;
-            }
+        // ignore if assignment already exists
+        if (store.graph().getAdjacentDescendants(ascendant).contains(descendant)) {
+            return false;
+        }
 
-            Node ascNode = store.graph().getNode(ascendant);
-            Node descNode = store.graph().getNode(descendant);
+        Node ascNode = store.graph().getNode(ascendant);
+        Node descNode = store.graph().getNode(descendant);
 
-            // check node types make a valid assignment relation
-            Assignment.checkAssignment(ascNode.getType(), descNode.getType());
+        // check node types make a valid assignment relation
+        Assignment.checkAssignment(ascNode.getType(), descNode.getType());
 
-            // check the assignment won't create a loop
-            checkAssignmentDoesNotCreateLoop(ascendant, descendant);
-        }
+        // check the assignment won't create a loop
+        checkAssignmentDoesNotCreateLoop(ascendant, descendant);
 
         return true;
     }
@@ -363,25 +361,23 @@ protected boolean checkAssignInput(String ascendant, Collection<String> descenda
      * @return True if the execution should proceed, false otherwise.
      * @throws PMException If any PM related exceptions occur in the implementing class.
      */
-    protected boolean checkDeassignInput(String ascendant, Collection<String> descendants) throws PMException {
-        for (String descendant : descendants) {
-            if (!store.graph().nodeExists(ascendant)) {
-                throw new NodeDoesNotExistException(ascendant);
-            } else if (!store.graph().nodeExists(descendant)) {
-                throw new NodeDoesNotExistException(descendant);
-            } else if (ascendant.equals(AdminPolicyNode.PM_ADMIN_OBJECT.nodeName()) &&
-                    descendant.equals(AdminPolicyNode.PM_ADMIN_PC.nodeName())) {
-                throw new CannotDeleteAdminPolicyConfigException();
-            }
+    protected boolean checkDeassignInput(String ascendant, String descendant) throws PMException {
+        if (!store.graph().nodeExists(ascendant)) {
+            throw new NodeDoesNotExistException(ascendant);
+        } else if (!store.graph().nodeExists(descendant)) {
+            throw new NodeDoesNotExistException(descendant);
+        } else if (ascendant.equals(AdminPolicyNode.PM_ADMIN_OBJECT.nodeName()) &&
+                descendant.equals(AdminPolicyNode.PM_ADMIN_PC.nodeName())) {
+            throw new CannotDeleteAdminPolicyConfigException();
+        }
 
-            Collection<String> descs = store.graph().getAdjacentDescendants(ascendant);
-            if (!descs.contains(descendant)) {
-                return false;
-            }
+        Collection<String> descs = store.graph().getAdjacentDescendants(ascendant);
+        if (!descs.contains(descendant)) {
+            return false;
+        }
 
-            if (descs.size() == 1) {
-                throw new DisconnectedNodeException(ascendant, descendant);
-            }
+        if (descs.size() == 1) {
+            throw new DisconnectedNodeException(ascendant, descendant);
         }
 
         return true;

diff --git a/src/test/java/gov/nist/csd/pm/pap/modification/GraphModifierTest.java b/src/test/java/gov/nist/csd/pm/pap/modification/GraphModifierTest.java
@@ -611,6 +611,18 @@ void testTx() throws PMException {
             assertTrue(pap.query().graph().isAscendant("ua4", "ua2"));
             assertFalse(pap.query().graph().isAscendant("ua4", "ua3"));
         }
+
+        @Test
+        void testOneDescendantIsAlreadyAssigned() throws PMException {
+            pap.modify().graph().createPolicyClass("pc1");
+            pap.modify().graph().createUserAttribute("ua1", List.of("pc1"));
+            pap.modify().graph().createUserAttribute("ua2", List.of("pc1"));
+
+            pap.modify().graph().assign("ua2", List.of("pc1", "ua1"));
+
+            assertTrue(pap.query().graph().isAscendant("ua2", "pc1"));
+            assertTrue(pap.query().graph().isAscendant("ua2", "ua1"));
+        }
     }
 
     @Nested
@@ -681,6 +693,18 @@ void testTx() throws PMException {
             assertTrue(pap.query().graph().isAscendant("ua4", "ua3"));
         }
 
+        @Test
+        void testOneDescendantIsAlreadyDeassigned() throws PMException {
+            pap.modify().graph().createPolicyClass("pc1");
+            pap.modify().graph().createUserAttribute("ua1", List.of("pc1"));
+            pap.modify().graph().createUserAttribute("ua2", List.of("pc1"));
+            pap.modify().graph().createUserAttribute("ua3", List.of("ua1", "ua2"));
+
+            pap.modify().graph().deassign("ua3", List.of("pc1", "ua1"));
+
+            assertFalse(pap.query().graph().getAdjacentDescendants("ua3").contains("pc1"));
+            assertFalse(pap.query().graph().getAdjacentDescendants("ua3").contains("ua1"));
+        }
     }
 
     @Nested