Optimizing ACL policies with caching of group-membership check results.

Author: krulis-martin

app/V1Module/security/Policies/AssignmentSolutionPermissionPolicy.php

@@ -3,10 +3,10 @@
 namespace App\Security\Policies;
 
 use App\Model\Entity\AssignmentSolution;
-use App\Model\Entity\AssignmentSolutionSubmission;
+use App\Model\Entity\GroupMembership;
 use App\Security\Identity;
 
-class AssignmentSolutionPermissionPolicy implements IPermissionPolicy
+class AssignmentSolutionPermissionPolicy extends BasePermissionPolicy implements IPermissionPolicy
 {
     public function getAssociatedClass()
     {
@@ -20,14 +20,11 @@ public function isAdmin(Identity $identity, AssignmentSolution $solution)
             return false;
         }
 
-        $group = $assignment->getGroup();
-        $user = $identity->getUserData();
-
-        if ($user === null) {
-            return false;
-        }
-
-        return $group && $group->isAdminOf($user);
+        return $this->checkMinimalMembership(
+            $identity->getUserData(),
+            $assignment->getGroup(),
+            GroupMembership::TYPE_ADMIN
+        );
     }
 
     public function isSupervisorOrAdmin(Identity $identity, AssignmentSolution $solution)
@@ -37,14 +34,11 @@ public function isSupervisorOrAdmin(Identity $identity, AssignmentSolution $solu
             return false;
         }
 
-        $group = $assignment->getGroup();
-        $user = $identity->getUserData();
-
-        if ($user === null) {
-            return false;
-        }
-
-        return $group && ($group->isSupervisorOf($user) || $group->isAdminOf($user));
+        return $this->checkMinimalMembership(
+            $identity->getUserData(),
+            $assignment->getGroup(),
+            GroupMembership::TYPE_SUPERVISOR
+        );
     }
 
     public function isObserverOrBetter(Identity $identity, AssignmentSolution $solution)
@@ -54,14 +48,11 @@ public function isObserverOrBetter(Identity $identity, AssignmentSolution $solut
             return false;
         }
 
-        $group = $assignment->getGroup();
-        $user = $identity->getUserData();
-
-        if ($user === null) {
-            return false;
-        }
-
-        return $group && ($group->isObserverOf($user) || $group->isSupervisorOf($user) || $group->isAdminOf($user));
+        return $this->checkMinimalMembership(
+            $identity->getUserData(),
+            $assignment->getGroup(),
+            GroupMembership::TYPE_OBSERVER
+        );
     }
 
     public function isAuthor(Identity $identity, AssignmentSolution $solution)

app/V1Module/security/Policies/BasePermissionPolicy.php

@@ -2,25 +2,64 @@
 
 namespace App\Security\Policies;
 
-use App\Model\Entity\Instance;
+use App\Model\Entity\GroupMembership;
+use App\Model\Entity\Group;
 use App\Model\Entity\User;
-use App\Security\Identity;
 use App\Security\Roles;
+use InvalidArgumentException;
 
 /**
- * Base policy is not bound to particular entity.
- * It gathers generic checks performed solely on the entity of the logged-in user.
+ * Base policy class that implements caching mechanisms reusable by other policies.
  */
-class BasePermissionPolicy implements IPermissionPolicy
+class BasePermissionPolicy
 {
-    public function getAssociatedClass()
+    private const MEMBERSHIPS = [
+        GroupMembership::TYPE_STUDENT => 1,
+        GroupMembership::TYPE_OBSERVER => 2,
+        GroupMembership::TYPE_SUPERVISOR => 3,
+        GroupMembership::TYPE_ADMIN => 4,
+    ];
+
+    private array $membershipCache = [];
+
+    protected function getMembershipLevel(User $user, Group $group): int
     {
-        return '';
+        $gid = $group->getId();
+        if (!array_key_exists($gid, $this->membershipCache)) {
+            $this->membershipCache[$gid] = 0; // Not a member
+
+            if ($user->getRole() === Roles::STUDENT_ROLE) {
+                if ($group->isStudentOf($user)) {
+                    $this->membershipCache[$gid] = self::MEMBERSHIPS[GroupMembership::TYPE_STUDENT];
+                }
+            } else {
+                if ($group->isAdminOf($user)) {
+                    $this->membershipCache[$gid] = self::MEMBERSHIPS[GroupMembership::TYPE_ADMIN];
+                } elseif ($group->isSupervisorOf($user)) {
+                    $this->membershipCache[$gid] = self::MEMBERSHIPS[GroupMembership::TYPE_SUPERVISOR];
+                } elseif ($group->isObserverOf($user)) {
+                    $this->membershipCache[$gid] = self::MEMBERSHIPS[GroupMembership::TYPE_OBSERVER];
+                } elseif ($user->getRole() === Roles::STUDENT_ROLE && $group->isStudentOf($user)) {
+                    $this->membershipCache[$gid] = self::MEMBERSHIPS[GroupMembership::TYPE_STUDENT];
+                }
+            }
+        }
+
+        return $this->membershipCache[$gid];
     }
 
-    public function userIsNotGroupLocked(Identity $identity): bool
+    protected function checkMinimalMembership(?User $user, ?Group $group, string $membership): bool
     {
-        $user = $identity->getUserData();
-        return $user && !$user->isGroupLocked();
+        if (!$user || !$group) {
+            return false;
+        }
+
+        $minLevel = self::MEMBERSHIPS[$membership] ?? null;
+        if ($minLevel === null) {
+            throw new InvalidArgumentException("Unknown membership type: $membership");
+        }
+
+        $level = $this->getMembershipLevel($user, $group);
+        return $level >= $minLevel;
     }
 }

app/V1Module/security/Policies/GenericPermissionPolicy.php

@@ -0,0 +1,23 @@
+<?php
+
+namespace App\Security\Policies;
+
+use App\Security\Identity;
+
+/**
+ * A policy that is not bound to a particular entity.
+ * It gathers generic checks performed solely on the entity of the logged-in user.
+ */
+class GenericPermissionPolicy implements IPermissionPolicy
+{
+    public function getAssociatedClass()
+    {
+        return '';
+    }
+
+    public function userIsNotGroupLocked(Identity $identity): bool
+    {
+        $user = $identity->getUserData();
+        return $user && !$user->isGroupLocked();
+    }
+}

app/config/config.neon

@@ -255,7 +255,7 @@ acl:
     asyncJob: App\Security\ACL\IAsyncJobPermissions
     plagiarism: App\Security\ACL\IPlagiarismPermissions
   policies:
-    _: App\Security\Policies\BasePermissionPolicy
+    _: App\Security\Policies\GenericPermissionPolicy
     group: App\Security\Policies\GroupPermissionPolicy
     instance: App\Security\Policies\InstancePermissionPolicy
     user: App\Security\Policies\UserPermissionPolicy