diff --git a/app/V1Module/security/Policies/AssignmentPermissionPolicy.php b/app/V1Module/security/Policies/AssignmentPermissionPolicy.php index 266b5c101..9f175b2ea 100644 --- a/app/V1Module/security/Policies/AssignmentPermissionPolicy.php +++ b/app/V1Module/security/Policies/AssignmentPermissionPolicy.php @@ -3,11 +3,12 @@ namespace App\Security\Policies; use App\Model\Entity\Assignment; +use App\Model\Entity\GroupMembership; use App\Security\Identity; use App\Helpers\SubmissionConfigHelper; use DateTime; -class AssignmentPermissionPolicy implements IPermissionPolicy +class AssignmentPermissionPolicy extends BasePermissionPolicy implements IPermissionPolicy { /** @var SubmissionConfigHelper */ private $submissionHelper; @@ -70,26 +71,20 @@ public function isAssignee(Identity $identity, Assignment $assignment) public function isSupervisorOrAdmin(Identity $identity, Assignment $assignment) { - $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, Assignment $assignment) { - $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 + ); } /** diff --git a/app/V1Module/security/Policies/AssignmentSolutionPermissionPolicy.php b/app/V1Module/security/Policies/AssignmentSolutionPermissionPolicy.php index e26373118..da78606e1 100644 --- a/app/V1Module/security/Policies/AssignmentSolutionPermissionPolicy.php +++ b/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) diff --git a/app/V1Module/security/Policies/BasePermissionPolicy.php b/app/V1Module/security/Policies/BasePermissionPolicy.php index edf69b77b..1c8be405d 100644 --- a/app/V1Module/security/Policies/BasePermissionPolicy.php +++ b/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 static array $membershipCache = []; + + protected function getMembershipLevel(User $user, Group $group): int { - return ''; + $gid = $group->getId(); + if (!array_key_exists($gid, self::$membershipCache)) { + self::$membershipCache[$gid] = 0; // Not a member + + if ($user->getRole() === Roles::STUDENT_ROLE) { + if ($group->isStudentOf($user)) { + self::$membershipCache[$gid] = self::MEMBERSHIPS[GroupMembership::TYPE_STUDENT]; + } + } else { + if ($group->isAdminOf($user)) { + self::$membershipCache[$gid] = self::MEMBERSHIPS[GroupMembership::TYPE_ADMIN]; + } elseif ($group->isSupervisorOf($user)) { + self::$membershipCache[$gid] = self::MEMBERSHIPS[GroupMembership::TYPE_SUPERVISOR]; + } elseif ($group->isObserverOf($user)) { + self::$membershipCache[$gid] = self::MEMBERSHIPS[GroupMembership::TYPE_OBSERVER]; + } elseif ($user->getRole() === Roles::STUDENT_ROLE && $group->isStudentOf($user)) { + self::$membershipCache[$gid] = self::MEMBERSHIPS[GroupMembership::TYPE_STUDENT]; + } + } + } + + return self::$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; } } diff --git a/app/V1Module/security/Policies/CommentPermissionPolicy.php b/app/V1Module/security/Policies/CommentPermissionPolicy.php index c7b93889c..11bcc8cf7 100644 --- a/app/V1Module/security/Policies/CommentPermissionPolicy.php +++ b/app/V1Module/security/Policies/CommentPermissionPolicy.php @@ -5,11 +5,12 @@ use App\Model\Entity\Assignment; use App\Model\Entity\AssignmentSolution; use App\Model\Entity\Comment; +use App\Model\Entity\GroupMembership; use App\Model\Repository\Assignments; use App\Model\Repository\AssignmentSolutions; use App\Security\Identity; -class CommentPermissionPolicy implements IPermissionPolicy +class CommentPermissionPolicy extends BasePermissionPolicy implements IPermissionPolicy { private $assignments; private $assignmentSolutions; @@ -62,8 +63,11 @@ public function isSupervisorInGroupOfCommentedSolution(Identity $identity, Comme return false; } - $group = $solution->getAssignment()->getGroup(); - return $group && ($group->isSupervisorOf($user) || $group->isAdminOf($user)); + return $this->checkMinimalMembership( + $user, + $solution->getAssignment()->getGroup(), + GroupMembership::TYPE_SUPERVISOR + ); } @@ -80,7 +84,6 @@ public function isSupervisorInGroupOfCommentedAssignment(Identity $identity, Com return false; } - $group = $assignment->getGroup(); - return $group && ($group->isSupervisorOf($user) || $group->isAdminOf($user)); + return $this->checkMinimalMembership($user, $assignment->getGroup(), GroupMembership::TYPE_SUPERVISOR); } } diff --git a/app/V1Module/security/Policies/ExercisePermissionPolicy.php b/app/V1Module/security/Policies/ExercisePermissionPolicy.php index eb2e03b85..ee1615a1a 100644 --- a/app/V1Module/security/Policies/ExercisePermissionPolicy.php +++ b/app/V1Module/security/Policies/ExercisePermissionPolicy.php @@ -4,10 +4,11 @@ use App\Model\Entity\Exercise; use App\Model\Entity\Group; +use App\Model\Entity\GroupMembership; use App\Helpers\SubmissionConfigHelper; use App\Security\Identity; -class ExercisePermissionPolicy implements IPermissionPolicy +class ExercisePermissionPolicy extends BasePermissionPolicy implements IPermissionPolicy { /** @var SubmissionConfigHelper */ private $submissionHelper; @@ -121,15 +122,6 @@ public function isSubGroupNonStudentMember(Identity $identity, Exercise $exercis return false; } - /** - * @var Array[] - * A cache holding the result of isAdminOf invocation for given groups. - * The cache is structures as [user-id][group-id] => boolean - * Under normal circumstances, the cache should hold only one (logged in) user, - * but it was written as generic cache just in case. - */ - private $supergroupAdminCache = []; - public function isSuperGroupAdmin(Identity $identity, Exercise $exercise) { $user = $identity->getUserData(); @@ -141,21 +133,12 @@ public function isSuperGroupAdmin(Identity $identity, Exercise $exercise) return false; } - if (empty($this->supergroupAdminCache[$user->getId()])) { - $this->supergroupAdminCache[$user->getId()] = []; - } - $supergroupCache = &$this->supergroupAdminCache[$user->getId()]; - /** @var Group $group */ foreach ($exercise->getGroups() as $group) { - if (!array_key_exists($group->getId(), $supergroupCache)) { - $supergroupCache[$group->getId()] = $group->isAdminOf($user); - } - if ($supergroupCache[$group->getId()]) { + if ($this->checkMinimalMembership($user, $group, GroupMembership::TYPE_ADMIN)) { return true; } } - return false; } diff --git a/app/V1Module/security/Policies/GenericPermissionPolicy.php b/app/V1Module/security/Policies/GenericPermissionPolicy.php new file mode 100644 index 000000000..4ec58860c --- /dev/null +++ b/app/V1Module/security/Policies/GenericPermissionPolicy.php @@ -0,0 +1,23 @@ +getUserData(); + return $user && !$user->isGroupLocked(); + } +} diff --git a/app/V1Module/security/Policies/GroupPermissionPolicy.php b/app/V1Module/security/Policies/GroupPermissionPolicy.php index 9b12188f8..6637c5f5c 100644 --- a/app/V1Module/security/Policies/GroupPermissionPolicy.php +++ b/app/V1Module/security/Policies/GroupPermissionPolicy.php @@ -3,11 +3,12 @@ namespace App\Security\Policies; use App\Model\Entity\Group; +use App\Model\Entity\GroupMembership; use App\Model\Entity\Instance; use App\Security\Identity; use DateTIme; -class GroupPermissionPolicy implements IPermissionPolicy +class GroupPermissionPolicy extends BasePermissionPolicy implements IPermissionPolicy { public function getAssociatedClass() { @@ -21,17 +22,20 @@ public function isMember(Identity $identity, Group $group): bool return false; } - return $group->isMemberOf($user) || $group->isAdminOf($user); + return $group->isMemberOf($user) || $this->checkMinimalMembership( + $identity->getUserData(), + $group, + GroupMembership::TYPE_ADMIN + ); } public function isSupervisorOrAdmin(Identity $identity, Group $group): bool { - $user = $identity->getUserData(); - if (!$user) { - return false; - } - - return $group->isSupervisorOf($user) || $group->isAdminOf($user); + return $this->checkMinimalMembership( + $identity->getUserData(), + $group, + GroupMembership::TYPE_SUPERVISOR + ); } public function isObserver(Identity $identity, Group $group): bool @@ -46,12 +50,11 @@ public function isObserver(Identity $identity, Group $group): bool public function isAdmin(Identity $identity, Group $group): bool { - $user = $identity->getUserData(); - if (!$user) { - return false; - } - - return $group->isAdminOf($user); + return $this->checkMinimalMembership( + $identity->getUserData(), + $group, + GroupMembership::TYPE_ADMIN + ); } public function isPublic(Identity $identity, Group $group): bool diff --git a/app/V1Module/security/Policies/NotificationPermissionPolicy.php b/app/V1Module/security/Policies/NotificationPermissionPolicy.php index bd69898e1..e0e3a90f7 100644 --- a/app/V1Module/security/Policies/NotificationPermissionPolicy.php +++ b/app/V1Module/security/Policies/NotificationPermissionPolicy.php @@ -3,11 +3,12 @@ namespace App\Security\Policies; use App\Model\Entity\Group; +use App\Model\Entity\GroupMembership; use App\Model\Entity\Notification; use App\Security\Identity; use App\Security\Roles; -class NotificationPermissionPolicy implements IPermissionPolicy +class NotificationPermissionPolicy extends BasePermissionPolicy implements IPermissionPolicy { /** @var Roles */ private $roles; @@ -57,7 +58,7 @@ public function isAncestorGroupAdmin(Identity $identity, Notification $notificat /** @var Group $group */ foreach ($notification->getGroups() as $group) { - if ($group->isAdminOf($user)) { + if ($this->checkMinimalMembership($user, $group, GroupMembership::TYPE_ADMIN)) { return true; } } diff --git a/app/V1Module/security/Policies/ReferenceExerciseSolutionPermissionPolicy.php b/app/V1Module/security/Policies/ReferenceExerciseSolutionPermissionPolicy.php index 2676a4281..211ca2b1e 100644 --- a/app/V1Module/security/Policies/ReferenceExerciseSolutionPermissionPolicy.php +++ b/app/V1Module/security/Policies/ReferenceExerciseSolutionPermissionPolicy.php @@ -3,11 +3,12 @@ namespace App\Security\Policies; use App\Model\Entity\Group; +use App\Model\Entity\GroupMembership; use App\Model\Entity\ReferenceExerciseSolution; use App\Helpers\SubmissionConfigHelper; use App\Security\Identity; -class ReferenceExerciseSolutionPermissionPolicy implements IPermissionPolicy +class ReferenceExerciseSolutionPermissionPolicy extends BasePermissionPolicy implements IPermissionPolicy { /** @var SubmissionConfigHelper */ private $submissionHelper; @@ -33,7 +34,7 @@ public function getAssociatedClass() return ReferenceExerciseSolution::class; } - public function isAuthor(Identity $identity, ReferenceExerciseSolution $referenceExerciseSolution = null) + public function isAuthor(Identity $identity, ?ReferenceExerciseSolution $referenceExerciseSolution = null) { if ($referenceExerciseSolution === null) { return false; @@ -74,7 +75,7 @@ public function isExerciseSuperGroupAdmin(Identity $identity, ReferenceExerciseS /** @var Group $group */ foreach ($exercise->getGroups() as $group) { - if ($group->isAdminOf($user)) { + if ($this->checkMinimalMembership($user, $group, GroupMembership::TYPE_ADMIN)) { return true; } } @@ -92,7 +93,7 @@ public function isExerciseSubGroupNonStudentMember( public function isExerciseNotArchived( Identity $identity, - ReferenceExerciseSolution $referenceExerciseSolution = null + ?ReferenceExerciseSolution $referenceExerciseSolution = null ) { if ($referenceExerciseSolution === null) { return false; @@ -107,7 +108,7 @@ public function isExerciseNotArchived( return $exercise && !$exercise->isArchived(); } - public function isPublic(Identity $identity, ReferenceExerciseSolution $referenceExerciseSolution = null) + public function isPublic(Identity $identity, ?ReferenceExerciseSolution $referenceExerciseSolution = null) { return $referenceExerciseSolution !== null && $referenceExerciseSolution->getVisibility() >= ReferenceExerciseSolution::VISIBILITY_PUBLIC; diff --git a/app/V1Module/security/Policies/ShadowAssignmentPermissionPolicy.php b/app/V1Module/security/Policies/ShadowAssignmentPermissionPolicy.php index 694dd8bdd..a03688fa8 100644 --- a/app/V1Module/security/Policies/ShadowAssignmentPermissionPolicy.php +++ b/app/V1Module/security/Policies/ShadowAssignmentPermissionPolicy.php @@ -2,11 +2,11 @@ namespace App\Security\Policies; +use App\Model\Entity\GroupMembership; use App\Model\Entity\ShadowAssignment; use App\Security\Identity; -use DateTime; -class ShadowAssignmentPermissionPolicy implements IPermissionPolicy +class ShadowAssignmentPermissionPolicy extends BasePermissionPolicy implements IPermissionPolicy { public function getAssociatedClass() { @@ -38,14 +38,11 @@ public function isAssignee(Identity $identity, ShadowAssignment $assignment) public function isSupervisor(Identity $identity, ShadowAssignment $assignment) { - $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 + ); } /** diff --git a/app/V1Module/security/Policies/UploadedFilePermissionPolicy.php b/app/V1Module/security/Policies/UploadedFilePermissionPolicy.php index fb6787ea3..2a6636bc9 100644 --- a/app/V1Module/security/Policies/UploadedFilePermissionPolicy.php +++ b/app/V1Module/security/Policies/UploadedFilePermissionPolicy.php @@ -6,12 +6,13 @@ use App\Model\Entity\Exercise; use App\Model\Entity\ExerciseFile; use App\Model\Entity\Group; +use App\Model\Entity\GroupMembership; use App\Model\Entity\UploadedFile; use App\Model\Repository\Assignments; use App\Model\Repository\UploadedFiles; use App\Security\Identity; -class UploadedFilePermissionPolicy implements IPermissionPolicy +class UploadedFilePermissionPolicy extends BasePermissionPolicy implements IPermissionPolicy { /** @var Assignments */ private $assignments; @@ -125,7 +126,7 @@ public function isSolutionInSupervisedOrObservedGroup(Identity $identity, Upload } $group = $this->files->findGroupForSolutionFile($file); - return $group && ($group->isObserverOf($user) || $group->isSupervisorOf($user) || $group->isAdminOf($user)); + return $this->checkMinimalMembership($user, $group, GroupMembership::TYPE_OBSERVER); } public function isRelatedToAssignment(Identity $identity, UploadedFile $file) diff --git a/app/V1Module/security/Policies/UserPermissionPolicy.php b/app/V1Module/security/Policies/UserPermissionPolicy.php index 61718dac0..aa4ea7744 100644 --- a/app/V1Module/security/Policies/UserPermissionPolicy.php +++ b/app/V1Module/security/Policies/UserPermissionPolicy.php @@ -2,12 +2,13 @@ namespace App\Security\Policies; +use App\Model\Entity\GroupMembership; use App\Model\Entity\Instance; use App\Model\Entity\User; use App\Security\Identity; use App\Security\Roles; -class UserPermissionPolicy implements IPermissionPolicy +class UserPermissionPolicy extends BasePermissionPolicy implements IPermissionPolicy { public function getAssociatedClass() { @@ -65,11 +66,7 @@ public function isReaderOfJoinedGroup(Identity $identity, User $user): bool } foreach ($user->getGroupsAsStudent() as $group) { - if ( - $group->isSupervisorOf($currentUser) - || $group->isObserverOf($currentUser) - || $group->isAdminOf($currentUser) - ) { + if ($this->checkMinimalMembership($currentUser, $group, GroupMembership::TYPE_OBSERVER)) { return true; } } diff --git a/app/config/config.neon b/app/config/config.neon index 848781606..516e851b5 100644 --- a/app/config/config.neon +++ b/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