From aebe872b440e9ec541eddda03a18d6b2cfc328b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Mon, 8 Jan 2024 14:47:28 +0100 Subject: [PATCH] Adjusting ACLs to reflect user-group locking and handle security for exam groups. --- .../Policies/AssignmentPermissionPolicy.php | 31 ++++- .../AssignmentSolutionPermissionPolicy.php | 14 ++ .../Policies/CommentPermissionPolicy.php | 6 + .../CommentThreadPermissionPolicy.php | 20 +++ .../Policies/GroupPermissionPolicy.php | 37 ++++++ app/config/config.neon | 1 + app/config/permissions.neon | 124 +++++++++++++----- 7 files changed, 197 insertions(+), 36 deletions(-) create mode 100644 app/V1Module/security/Policies/CommentThreadPermissionPolicy.php diff --git a/app/V1Module/security/Policies/AssignmentPermissionPolicy.php b/app/V1Module/security/Policies/AssignmentPermissionPolicy.php index d5b6733e6..8292f2b20 100644 --- a/app/V1Module/security/Policies/AssignmentPermissionPolicy.php +++ b/app/V1Module/security/Policies/AssignmentPermissionPolicy.php @@ -34,9 +34,21 @@ public function isPublic(Identity $identity, Assignment $assignment) public function isVisible(Identity $identity, Assignment $assignment) { + $user = $identity->getUserData(); + if ($user === null) { + return false; + } + $now = new DateTime(); - return $assignment->isPublic() && - ($assignment->getVisibleFrom() === null || $assignment->getVisibleFrom() <= $now); + $group = $assignment->getGroup(); + if (!$group || ($group->isExam() && $now < $group->getExamBegin())) { + return false; // exam groups hide all assignments before the exam starts + } + + $visibleFromOk = $assignment->getVisibleFrom() === null || $assignment->getVisibleFrom() <= $now; + return $assignment->isPublic() && $visibleFromOk && + // not an exam, or it over (so the assignments are visible to all), or the student is currently doing it + (!$group->isExam() || $group->getExamEnd() < $now || $user->getGroupLock()?->getId() === $group->getId()); } public function isInActiveGroup(Identity $identity, Assignment $assignment) @@ -48,7 +60,6 @@ public function isInActiveGroup(Identity $identity, Assignment $assignment) public function isAssignee(Identity $identity, Assignment $assignment) { $user = $identity->getUserData(); - if ($user === null) { return false; } @@ -79,4 +90,18 @@ public function isObserverOrBetter(Identity $identity, Assignment $assignment) return $group && ($group->isObserverOf($user) || $group->isSupervisorOf($user) || $group->isAdminOf($user)); } + + /** + * Current user is either not locked at all, or locked to this group (where the assignment is). + */ + public function userIsNotLockedElsewhere(Identity $identity, Assignment $assignment): bool + { + $user = $identity->getUserData(); + $group = $assignment->getGroup(); + if ($user === null || $group === null || $group->isArchived()) { + return false; + } + + return !$user->isGroupLocked() || $user->getGroupLock()->getId() === $group->getId(); + } } diff --git a/app/V1Module/security/Policies/AssignmentSolutionPermissionPolicy.php b/app/V1Module/security/Policies/AssignmentSolutionPermissionPolicy.php index aba48c03b..b88dbf12a 100644 --- a/app/V1Module/security/Policies/AssignmentSolutionPermissionPolicy.php +++ b/app/V1Module/security/Policies/AssignmentSolutionPermissionPolicy.php @@ -95,4 +95,18 @@ public function isInActiveGroup(Identity $identity, AssignmentSolution $solution $group = $assignment->getGroup(); return $group && !$group->isArchived(); // active = not deleted and not archived } + + /** + * Current user is either not locked at all, or locked in the group where the solution is. + */ + public function userIsNotLockedElsewhere(Identity $identity, AssignmentSolution $solution): bool + { + $user = $identity->getUserData(); + $group = $solution->getAssignment()?->getGroup(); + if ($user === null || $group === null || $group->isArchived()) { + return false; + } + + return !$user->isGroupLocked() || $user->getGroupLock()->getId() === $group->getId(); + } } diff --git a/app/V1Module/security/Policies/CommentPermissionPolicy.php b/app/V1Module/security/Policies/CommentPermissionPolicy.php index c7b93889c..60580d984 100644 --- a/app/V1Module/security/Policies/CommentPermissionPolicy.php +++ b/app/V1Module/security/Policies/CommentPermissionPolicy.php @@ -83,4 +83,10 @@ 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 new file mode 100644 index 000000000..872d64999 --- /dev/null +++ b/app/V1Module/security/Policies/CommentThreadPermissionPolicy.php @@ -0,0 +1,20 @@ +getUserData(); + return $user && !$user->isGroupLocked(); + } +} diff --git a/app/V1Module/security/Policies/GroupPermissionPolicy.php b/app/V1Module/security/Policies/GroupPermissionPolicy.php index 3c4497606..17118ba05 100644 --- a/app/V1Module/security/Policies/GroupPermissionPolicy.php +++ b/app/V1Module/security/Policies/GroupPermissionPolicy.php @@ -98,9 +98,46 @@ function ($key, Instance $instance) use ($group) { ); } + public function isNotExam(Identity $identity, Group $group): bool + { + return !$group->isExam(); + } + public function isExamInProgress(Identity $identity, Group $group): bool { $now = new DateTime(); return $group->isExam() && $group->getExamBegin() <= $now && $now <= $group->getExamEnd(); } + + public function isExamOver(Identity $identity, Group $group): bool + { + $now = new DateTime(); + return $group->isExam() && $group->getExamEnd() < $now; + } + + /** + * Current user is locked to the selected group. + */ + public function userIsLocked(Identity $identity, Group $group): bool + { + $user = $identity->getUserData(); + if ($user === null) { + return false; + } + + return $user->getGroupLock()?->getId() === $group->getId(); + } + + /** + * Current user is either not locked at all, or locked to this group. + */ + public function userIsNotLockedElsewhere(Identity $identity, Group $group): bool + { + $user = $identity->getUserData(); + if ($user === null) { + return false; + } + + return !$user->isGroupLocked() || $user->getGroupLock()->getId() === $group->getId(); + } } diff --git a/app/config/config.neon b/app/config/config.neon index 9b46ad2c6..55707ea86 100644 --- a/app/config/config.neon +++ b/app/config/config.neon @@ -254,6 +254,7 @@ acl: user: App\Security\Policies\UserPermissionPolicy assignment: App\Security\Policies\AssignmentPermissionPolicy comment: App\Security\Policies\CommentPermissionPolicy + thread: App\Security\Policies\CommentThreadPermissionPolicy exercise: App\Security\Policies\ExercisePermissionPolicy referenceExerciseSolution: App\Security\Policies\ReferenceExerciseSolutionPermissionPolicy assignmentSolution: App\Security\Policies\AssignmentSolutionPermissionPolicy diff --git a/app/config/permissions.neon b/app/config/permissions.neon index 2da2be646..b03d92872 100644 --- a/app/config/permissions.neon +++ b/app/config/permissions.neon @@ -82,6 +82,13 @@ permissions: actions: viewAssignments conditions: - group.isMember + - group.userIsNotLockedElsewhere + - or: + - group.isNotExam + - group.isExamOver + - group.userIsLocked + - group.isSupervisor + - group.isObserver - allow: true role: student @@ -92,6 +99,7 @@ permissions: - viewSubgroups - viewMembers conditions: + - group.userIsNotLockedElsewhere - or: - group.isAdmin - group.isMember @@ -103,55 +111,83 @@ permissions: role: student resource: group actions: - - viewPublicDetail - - viewInvitations - conditions: group.isInSameInstance + - viewPublicDetail + conditions: + - group.isInSameInstance - allow: true role: student resource: group actions: - - acceptInvitation + - viewInvitations conditions: - - group.isInSameInstance - - group.isNotArchived + - group.isInSameInstance + - group.userIsNotLockedElsewhere - - allow: true # A student can join public groups + - allow: true role: student resource: group actions: - - addStudent + - acceptInvitation conditions: - - group.isPublic - - group.isInSameInstance - - student.isSameUser - - group.isNotArchived + - group.isInSameInstance + - group.isNotArchived + - group.userIsNotLockedElsewhere + + - allow: true # A student can join public groups + role: student + resource: group + actions: + - addStudent + conditions: + - group.isPublic + - group.isInSameInstance + - student.isSameUser + - group.isNotArchived + - group.userIsNotLockedElsewhere - allow: true role: student resource: group actions: - - removeStudent # A student can leave any group... + - removeStudent # A student can leave any group... conditions: - - group.isNotDetainingStudents # ... which does not detain students - - group.isMember - - student.isSameUser - - group.isNotArchived + - group.isNotDetainingStudents # ... which does not detain students + - group.isNotExam # exam groups automatically detain students + - group.isMember + - student.isSameUser + - group.isNotArchived + - group.userIsNotLockedElsewhere - allow: true role: student resource: group actions: - - viewStudentStats + - viewStudentStats conditions: - - group.isMember - - student.isSameUser + - group.isMember + - student.isSameUser + - group.userIsNotLockedElsewhere + - or: + - group.isNotExam + - group.isExamOver + - group.userIsLocked + - group.isSupervisor + - group.isObserver - allow: true role: student resource: group actions: viewStats - conditions: group.areStatsPublic + conditions: + - group.areStatsPublic + - group.userIsNotLockedElsewhere + - or: + - group.isNotExam + - group.isExamOver + - group.userIsLocked + - group.isSupervisor + - group.isObserver - allow: true resource: group @@ -233,22 +269,22 @@ permissions: role: student resource: group actions: - - lockStudent # students lock themselves + - lockStudent # students lock themselves conditions: - - group.isNotArchived - - group.isExamInProgress - - group.isMember - - student.isSameUser - - student.isNotGroupLocked + - group.isNotArchived + - group.isExamInProgress + - group.isMember + - student.isSameUser + - student.isNotGroupLocked - allow: true role: supervisor-student resource: group actions: - - unlockStudent # only a supervisor of the same group can unlock the student + - unlockStudent # only a supervisor of the same group can unlock the student conditions: - - group.isExamInProgress - - group.isSupervisor + - group.isExamInProgress + - group.isSupervisor ######################## # Instance permissions # @@ -398,6 +434,7 @@ permissions: conditions: - assignment.isAssignee - assignment.isVisible + - assignment.userIsNotLockedElsewhere - allow: true role: student @@ -409,9 +446,11 @@ permissions: - assignment.isInActiveGroup - assignment.isAssignee - assignment.isVisible + - assignment.userIsNotLockedElsewhere - student.isSameUser - allow: true + role: supervisor-student resource: assignment actions: - viewDetail @@ -422,6 +461,7 @@ permissions: - assignment.isObserverOrBetter - allow: true + role: supervisor-student resource: assignment actions: - update @@ -435,6 +475,7 @@ permissions: - assignment.isSupervisorOrAdmin - allow: true + role: supervisor-student resource: group actions: - assignExercise @@ -449,6 +490,8 @@ permissions: - viewSubmissions conditions: - student.isSameUser + - assignment.isVisible + - assignment.userIsNotLockedElsewhere ################################### # Assignment Solution permissions # @@ -483,6 +526,7 @@ permissions: - viewReview conditions: - assignmentSolution.isAuthor + - assignmentSolution.userIsNotLockedElsewhere - allow: true role: student @@ -492,6 +536,7 @@ permissions: conditions: - assignmentSolution.isAuthor - assignmentSolution.isInActiveGroup + - assignmentSolution.userIsNotLockedElsewhere - allow: true role: student @@ -501,8 +546,10 @@ permissions: conditions: - assignmentSolution.areEvaluationDetailsPublic - assignmentSolution.isAuthor + - assignmentSolution.userIsNotLockedElsewhere - allow: true + role: supervisor-student resource: assignmentSolution actions: - viewDetail @@ -517,6 +564,7 @@ permissions: - assignmentSolution.isObserverOrBetter - allow: true + role: supervisor-student resource: assignmentSolution actions: - deleteEvaluation @@ -532,6 +580,7 @@ permissions: - assignmentSolution.isInActiveGroup - allow: true + role: supervisor-student resource: assignmentSolution actions: - editReviewComment @@ -542,6 +591,7 @@ permissions: - assignmentSolution.isAdmin - allow: true + role: supervisor-student resource: assignmentSolution actions: - deleteReview @@ -583,18 +633,26 @@ permissions: - alter - delete conditions: - or: + - comment.userIsNotGroupLocked + - or: - comment.isAuthor - comment.isSupervisorInGroupOfCommentedSolution - comment.isSupervisorInGroupOfCommentedAssignment - - allow: true # TODO + - allow: true role: student resource: comment actions: - - createThread - viewThread - addComment + conditions: # TODO - make sure only apropriate group members/authors can do this + - thread.userIsNotGroupLocked + + - allow: true + role: student + resource: comment + actions: + - createThread ######################## # Exercise permissions #