Adding base ACL policy which is detached from entities and performs c…

…hecks only on the identity.
ReCodEx · Jan 25, 2024 · a48ded4 · a48ded4
1 parent adce06d
commit a48ded4
Show file tree

Hide file tree

Showing 7 changed files with 62 additions and 22 deletions.
diff --git a/app/V1Module/security/AuthorizatorBuilder.php b/app/V1Module/security/AuthorizatorBuilder.php
@@ -92,7 +92,14 @@ private function loadConditionClauses($conditions, $interface, &$actions, array
         $type = "and";
 
         if (!is_array($conditions)) {
-            list($conditionTarget, $condition) = explode(".", $conditions, 2);
+            $tokens = explode(".", $conditions, 2);
+            if (count($tokens) === 1) {
+                // if '.' is missing ... let's use base policy class (identified by null target)
+                $conditionTarget = null;
+                $condition = $conditions;
+            } else {
+                list($conditionTarget, $condition) = $tokens;
+            }
 
             foreach ($actions as $action) {
                 $this->checkActionProvidesContext($interface, $action, $conditionTarget);
@@ -101,7 +108,7 @@ private function loadConditionClauses($conditions, $interface, &$actions, array
             $checkVariable = "\$check_" . $this->checkCounter++;
             $checkValues[$checkVariable] = $this->dumper->format(
                 '$this->policy->check(?, ?, $this->queriedIdentity)',
-                new PhpLiteral(sprintf('$this->queriedContext["%s"]', $conditionTarget)),
+                $conditionTarget ? new PhpLiteral(sprintf('$this->queriedContext["%s"]', $conditionTarget)) : null,
                 $condition
             );
 
@@ -137,7 +144,7 @@ private function loadConditionClauses($conditions, $interface, &$actions, array
         }
     }
 
-    private function checkActionProvidesContext(?ReflectionClass $interface, string $action, string $contextItem)
+    private function checkActionProvidesContext(?ReflectionClass $interface, string $action, ?string $contextItem)
     {
         if ($interface === null) {
             throw new LogicException(
@@ -157,6 +164,10 @@ private function checkActionProvidesContext(?ReflectionClass $interface, string
             );
         }
 
+        if ($contextItem === null) {
+            return;
+        }
+
         foreach ($method->getParameters() as $parameter) {
             if ($parameter->getName() === $contextItem) {
                 return;

diff --git a/app/V1Module/security/Policies/BasePermissionPolicy.php b/app/V1Module/security/Policies/BasePermissionPolicy.php
@@ -0,0 +1,26 @@
+<?php
+
+namespace App\Security\Policies;
+
+use App\Model\Entity\Instance;
+use App\Model\Entity\User;
+use App\Security\Identity;
+use App\Security\Roles;
+
+/**
+ * Base policy is not bound to particular entity.
+ * It gathers generic checks performed solely on the entity of the logged-in user.
+ */
+class BasePermissionPolicy implements IPermissionPolicy
+{
+    public function getAssociatedClass()
+    {
+        return '';
+    }
+
+    public function userIsNotGroupLocked(Identity $identity): bool
+    {
+        $user = $identity->getUserData();
+        return $user && !$user->isGroupLocked();
+    }
+}
diff --git a/app/V1Module/security/Policies/CommentPermissionPolicy.php b/app/V1Module/security/Policies/CommentPermissionPolicy.php
@@ -83,10 +83,4 @@ public function isSupervisorInGroupOfCommentedAssignment(Identity $identity, Com
         $group = $assignment->getGroup();
         return $group && ($group->isSupervisorOf($user) || $group->isAdminOf($user));
     }
-
-    public function userIsNotGroupLocked(Identity $identity, Comment $comment): bool
-    {
-        $user = $identity->getUserData();
-        return $user && !$user->isGroupLocked();
-    }
 }
diff --git a/app/V1Module/security/Policies/CommentThreadPermissionPolicy.php b/app/V1Module/security/Policies/CommentThreadPermissionPolicy.php
@@ -11,10 +11,4 @@ public function getAssociatedClass()
     {
         return CommentThread::class;
     }
-
-    public function userIsNotGroupLocked(Identity $identity, CommentThread $thread): bool
-    {
-        $user = $identity->getUserData();
-        return $user && !$user->isGroupLocked();
-    }
 }
diff --git a/app/V1Module/security/PolicyRegistry.php b/app/V1Module/security/PolicyRegistry.php
@@ -9,10 +9,16 @@ class PolicyRegistry
 {
     /** @var IPermissionPolicy[] */
     private $policies = [];
+    private $basePolicy = null;
 
     public function addPolicy($policy)
     {
-        $this->policies[] = $policy;
+        $class = $policy->getAssociatedClass();
+        if ($class) {
+            $this->policies[] = $policy;
+        } else {
+            $this->basePolicy = $policy;
+        }
     }
 
     public function get($resource, $policyName)
@@ -32,11 +38,17 @@ public function check($subject, $policyName, $queriedIdentity): bool
 
     private function findPolicyOrThrow($subject, $policyName): IPermissionPolicy
     {
-        foreach ($this->policies as $policy) {
-            $associatedClass = $policy->getAssociatedClass();
-            if ($subject instanceof $associatedClass && method_exists($policy, $policyName)) {
-                return $policy;
+        if ($subject !== null) {
+            foreach ($this->policies as $policy) {
+                // se need this search due to entity inheritance (TODO: find a better way in the future)
+                $associatedClass = $policy->getAssociatedClass();
+                if ($subject instanceof $associatedClass && method_exists($policy, $policyName)) {
+                    return $policy;
+                }
             }
+        } elseif ($this->basePolicy !== null && method_exists($this->basePolicy, $policyName)) {
+            // if no subject is given, lets try base policy...
+            return $this->basePolicy;
         }
 
         throw new InvalidArgumentException(

diff --git a/app/config/config.neon b/app/config/config.neon
@@ -249,6 +249,7 @@ acl:
     asyncJob: App\Security\ACL\IAsyncJobPermissions
     plagiarism: App\Security\ACL\IPlagiarismPermissions
   policies:
+    _: App\Security\Policies\BasePermissionPolicy
     group: App\Security\Policies\GroupPermissionPolicy
     instance: App\Security\Policies\InstancePermissionPolicy
     user: App\Security\Policies\UserPermissionPolicy

diff --git a/app/config/permissions.neon b/app/config/permissions.neon
@@ -669,7 +669,7 @@ permissions:
           - alter
           - delete
       conditions:
-          - comment.userIsNotGroupLocked
+          - userIsNotGroupLocked
           - or:
               - comment.isAuthor
               - comment.isSupervisorInGroupOfCommentedSolution
@@ -682,13 +682,15 @@ permissions:
           - viewThread
           - addComment
       conditions:  # TODO - make sure only apropriate group members/authors can do this
-          - thread.userIsNotGroupLocked
+          - userIsNotGroupLocked
 
     - allow: true
       role: student
       resource: comment
       actions:
           - createThread
+      conditions:
+          - userIsNotGroupLocked
 
     ########################
     # Exercise permissions #