From afa934643cecd89a7df3fbfd8a48a5f03bbc5663 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Mon, 4 May 2026 18:09:45 +0200 Subject: [PATCH 1/5] Replacing lock-strict flag with lock-type enum for exam groups. --- app/model/entity/Group.php | 27 ++++-- app/model/entity/GroupExam.php | 39 +++++---- app/model/entity/GroupExamLockType.php | 36 ++++++++ app/model/entity/User.php | 19 +++-- app/model/repository/GroupExams.php | 17 ++-- migrations/Version20211015163456.php | 10 +-- migrations/Version20260504154333.php | 111 +++++++++++++++++++++++++ 7 files changed, 216 insertions(+), 43 deletions(-) create mode 100644 app/model/entity/GroupExamLockType.php create mode 100644 migrations/Version20260504154333.php diff --git a/app/model/entity/Group.php b/app/model/entity/Group.php index 1238e7978..939ee7d92 100644 --- a/app/model/entity/Group.php +++ b/app/model/entity/Group.php @@ -2,7 +2,7 @@ namespace App\Model\Entity; -use DateTime; +use App\Model\GroupExamLockType; use Doctrine\Common\Collections\Collection; use Doctrine\Common\Collections\ReadableCollection; use Doctrine\Common\Collections\ArrayCollection; @@ -11,6 +11,7 @@ use Gedmo\Mapping\Annotation as Gedmo; use LogicException; use InvalidArgumentException; +use DateTime; /** * @ORM\Entity @@ -231,6 +232,7 @@ public function isDirectlyArchived(): bool * @ORM\Column(type="datetime", nullable=true) * When an exam in this groups begins. In the exam period, a user must lock in a group to be allowed * submitting solutions. This is completely independent of the isExam flag. + * @var DateTime|null */ protected $examBegin = null; @@ -238,15 +240,17 @@ public function isDirectlyArchived(): bool * @ORM\Column(type="datetime", nullable=true) * When an exam in this groups ends. In the exam period, a user must lock in a group to be allowed * submitting solutions. This is completely independent of the isExam flag. + * @var DateTime|null */ protected $examEnd = null; /** - * @ORM\Column(type="boolean") - * Whether the group-lock for the exam should be strict + * @ORM\Column(type="string") + * The type of lock for the exam. * (under strict lock, the user cannot read data from other groups). + * @var string */ - protected $examLockStrict = false; + protected $examLockType = GroupExamLockType::Visible->value; /** * @var Collection @@ -260,9 +264,9 @@ public function isDirectlyArchived(): bool * Switch the group into an exam group by setting the begin and end dates of the exam. * @param DateTime $begin when the exam starts * @param DateTime $end when the exam ends - * @param bool $strict if true, locked users cannot access other groups (for reading) + * @param GroupExamLockType $type the type of lock for the exam */ - public function setExamPeriod(DateTime $begin, DateTime $end, bool $strict = false): void + public function setExamPeriod(DateTime $begin, DateTime $end, GroupExamLockType $type): void { // asserts if ($begin >= $end) { @@ -275,7 +279,7 @@ public function setExamPeriod(DateTime $begin, DateTime $end, bool $strict = fal $this->examBegin = $begin; $this->examEnd = $end; - $this->examLockStrict = $strict; + $this->examLockType = $type->value; $this->isOrganizational = false; } @@ -286,7 +290,7 @@ public function removeExamPeriod(): void { $this->examBegin = null; $this->examEnd = null; - $this->examLockStrict = false; + $this->examLockType = GroupExamLockType::Visible->value; } /** @@ -301,7 +305,12 @@ public function hasExamPeriodSet(?DateTime $at = null): bool public function isExamLockStrict(): bool { - return $this->examLockStrict; + return $this->examLockType === GroupExamLockType::Restricted->value; + } + + public function getExamLockType(): GroupExamLockType + { + return GroupExamLockType::from($this->examLockType); } /** diff --git a/app/model/entity/GroupExam.php b/app/model/entity/GroupExam.php index bcbdfd3ca..a871f27a3 100644 --- a/app/model/entity/GroupExam.php +++ b/app/model/entity/GroupExam.php @@ -2,6 +2,7 @@ namespace App\Model\Entity; +use App\Model\GroupExamLockType; use Doctrine\ORM\Mapping as ORM; use DateTime; use JsonSerializable; @@ -11,7 +12,7 @@ * @ORM\Table(uniqueConstraints={@ORM\UniqueConstraint(columns={"group_id", "begin"})}) * Holds history record of an exam that took place in a group. * The `examBegin`, `examEnd` fields are copied from group to `begin`, `end` fields here, - * `examLockStrict` is copied to `lockStrict` field. + * `examLockType` is copied to `lockType` field. * This entity is created when the first user locks in (i.e., only exams with users are recorded in history). */ class GroupExam implements JsonSerializable @@ -20,58 +21,61 @@ class GroupExam implements JsonSerializable * @ORM\Id * @ORM\Column(type="integer") * @ORM\GeneratedValue(strategy="AUTO") + * @var int|null */ protected $id; /** * @ORM\ManyToOne(targetEntity="Group", inversedBy="exams") + * @var Group */ protected $group; /** * @ORM\Column(type="datetime") - * @var DateTime + * @var DateTime|null */ protected $begin = null; /** * @ORM\Column(type="datetime") - * @var DateTime + * @var DateTime|null */ protected $end = null; /** - * @ORM\Column(type="boolean") - * Saved value from examLockStrict flag. + * @ORM\Column(type="string") + * Saved value from examLockType flag. + * @var string */ - protected $lockStrict = false; + protected $lockType = GroupExamLockType::Visible->value; /** * Constructor * @param Group $group * @param DateTime $begin * @param DateTime $end - * @param bool $strict + * @param GroupExamLockType $type */ - public function __construct(Group $group, DateTime $begin, DateTime $end, bool $strict) + public function __construct(Group $group, DateTime $begin, DateTime $end, GroupExamLockType $type) { $this->group = $group; $this->begin = $begin; $this->end = $end; - $this->lockStrict = $strict; + $this->lockType = $type->value; } /** * Update the parameters (happens if pending exam is cut short, for instance). * @param DateTime $begin * @param DateTime $end - * @param bool $strict + * @param GroupExamLockType $type */ - public function update(DateTime $begin, DateTime $end, bool $strict): void + public function update(DateTime $begin, DateTime $end, GroupExamLockType $type): void { $this->begin = $begin; $this->end = $end; - $this->lockStrict = $strict; + $this->lockType = $type->value; } public function jsonSerialize(): mixed @@ -82,7 +86,9 @@ public function jsonSerialize(): mixed "groupId" => $group ? $group->getId() : null, "begin" => $this->getBegin()->getTimestamp(), "end" => $this->getEnd()->getTimestamp(), - "strict" => $this->lockStrict, + "type" => $this->lockType, + // BC only, DEPRECATED + "strict" => $this->lockType === GroupExamLockType::Restricted->value, ]; } @@ -113,6 +119,11 @@ public function getEnd(): DateTime public function isLockStrict(): bool { - return $this->lockStrict; + return $this->lockType === GroupExamLockType::Restricted->value; + } + + public function getLockType(): GroupExamLockType + { + return GroupExamLockType::from($this->lockType); } } diff --git a/app/model/entity/GroupExamLockType.php b/app/model/entity/GroupExamLockType.php new file mode 100644 index 000000000..d3a627d6f --- /dev/null +++ b/app/model/entity/GroupExamLockType.php @@ -0,0 +1,36 @@ + $case->value, self::cases()); + } +} diff --git a/app/model/entity/User.php b/app/model/entity/User.php index c9cc1b7bf..b984a5e10 100644 --- a/app/model/entity/User.php +++ b/app/model/entity/User.php @@ -2,6 +2,7 @@ namespace App\Model\Entity; +use App\Model\GroupExamLockType; use App\Security\Roles; use Doctrine\Common\Collections\Collection; use Doctrine\ORM\Mapping as ORM; @@ -420,6 +421,7 @@ public function updateLastAuthenticationAt(?DateTime $time = null) * @ORM\Column(type="string", nullable=true) * IP address (either IPv4 or IPv6) the user is locked to. API requests from different addresses * will be treated as unauthorized. + * @var string|null */ protected $ipLock = null; @@ -499,13 +501,16 @@ public function verifyIpLock(string $currentIp): bool * @ORM\ManyToOne(targetEntity="Group") * If set, any user actions will be restricted to this group only. * (except for fundamental operations like listing groups or getting group name) + * @var Group|null */ protected $groupLock = null; /** - * @ORM\Column(type="boolean") + * @ORM\Column(type="string") + * String representation of the GroupExamLockType enum (copied from the group exam). + * @var string */ - protected $groupLockStrict = false; + protected $groupLockType = GroupExamLockType::Visible->value; /** * @ORM\Column(type="datetime", nullable=true) @@ -528,7 +533,7 @@ public function isGroupLocked(): bool */ public function isGroupLockStrict(): bool { - return $this->groupLockStrict; + return $this->groupLockType === GroupExamLockType::Restricted->value; } /** @@ -548,9 +553,9 @@ public function getGroupLockExpiration(): ?DateTimeInterface * Lock the user within a group. * @param Group $group * @param DateTime|null $expiration of the lock, if null, the lock will never expire - * @param bool $strict if true, the user may not even access other groups for reading + * @param GroupExamLockType $type if true, the user may not even access other groups for reading */ - public function setGroupLock(Group $group, ?DateTime $expiration = null, bool $strict = false): void + public function setGroupLock(Group $group, ?DateTime $expiration, GroupExamLockType $type): void { // basic asserts to be on the safe side if (!$group->hasExamPeriodSet()) { @@ -562,14 +567,14 @@ public function setGroupLock(Group $group, ?DateTime $expiration = null, bool $s $this->groupLock = $group; $this->groupLockExpiration = $expiration; - $this->groupLockStrict = $strict; + $this->groupLockType = $type->value; } public function removeGroupLock(): void { $this->groupLock = null; $this->groupLockExpiration = null; - $this->groupLockStrict = false; + $this->groupLockType = GroupExamLockType::Visible->value; } diff --git a/app/model/repository/GroupExams.php b/app/model/repository/GroupExams.php index 48c0c3d92..efdd4cfed 100644 --- a/app/model/repository/GroupExams.php +++ b/app/model/repository/GroupExams.php @@ -4,6 +4,7 @@ use App\Model\Entity\Group; use App\Model\Entity\GroupExam; +use App\Model\GroupExamLockType; use Doctrine\ORM\EntityManagerInterface; use Doctrine\DBAL\Exception\UniqueConstraintViolationException; use DateTime; @@ -45,10 +46,10 @@ public function findPendingForGroup(Group $group): ?GroupExam * @param Group $group * @param DateTime $begin * @param DateTime $end - * @param bool $strict + * @param GroupExamLockType $type * @return GroupExam|null */ - private function tryFindOrCreate(Group $group, DateTime $begin, DateTime $end, bool $strict): ?GroupExam + private function tryFindOrCreate(Group $group, DateTime $begin, DateTime $end, GroupExamLockType $type): ?GroupExam { $exam = $this->findBy(["group" => $group, "begin" => $begin]); if (count($exam) > 1) { @@ -58,12 +59,12 @@ private function tryFindOrCreate(Group $group, DateTime $begin, DateTime $end, b if (!$exam) { try { $this->em->getConnection()->executeQuery( - "INSERT INTO group_exam (group_id, begin, end, lock_strict) VALUES (:gid, :begin, :end, :strict)", + "INSERT INTO group_exam (group_id, begin, end, lock_type) VALUES (:gid, :begin, :end, :type)", [ 'gid' => $group->getId(), 'begin' => $begin->format('Y-m-d H:i:s'), 'end' => $end->format('Y-m-d H:i:s'), - 'strict' => $strict ? 1 : 0 + 'type' => $type->value ] ); } catch (UniqueConstraintViolationException) { @@ -82,21 +83,21 @@ private function tryFindOrCreate(Group $group, DateTime $begin, DateTime $end, b * @param Group $group * @param DateTime|null $begin if null, exam begin from the group is taken * @param DateTime|null $end if null, exam end from the group is taken - * @param bool|null $strict if null, examLockStrict value is taken + * @param GroupExamLockType|null $type if null, examLockType value is taken * @return GroupExam */ public function findOrCreate( Group $group, ?DateTime $begin = null, ?DateTime $end = null, - ?bool $strict = null + ?GroupExamLockType $type = null ): GroupExam { $begin = $begin ?? $group->getExamBegin(); $end = $end ?? $group->getExamEnd(); - $strict = $strict === null ? $group->isExamLockStrict() : $strict; + $type = $type ?? $group->getExamLockType(); for ($retries = 0; $retries < 3; $retries++) { - $exam = $this->tryFindOrCreate($group, $begin, $end, $strict); + $exam = $this->tryFindOrCreate($group, $begin, $end, $type); if ($exam !== null) { return $exam; } diff --git a/migrations/Version20211015163456.php b/migrations/Version20211015163456.php index 834ca4aa7..c5c13376c 100644 --- a/migrations/Version20211015163456.php +++ b/migrations/Version20211015163456.php @@ -62,18 +62,18 @@ public function preUp(Schema $schema): void // update $this->connection->executeStatement( "UPDATE user_ui_data SET `data` = :newData WHERE id = :id", - [ 'id' => $uiId, 'newData' => json_encode($json) ] + ['id' => $uiId, 'newData' => json_encode($json)] ); } else { // insert new $uuid = Uuid::uuid(); $this->connection->executeStatement( "INSERT INTO user_ui_data (id, `data`) VALUES (:id, :newData)", - [ 'id' => $uuid, 'newData' => json_encode($json) ] + ['id' => $uuid, 'newData' => json_encode($json)] ); $this->connection->executeStatement( "UPDATE `user` SET `ui_data_id` = :uuid WHERE id = :id", - [ 'id' => $userId,'uuid' => $uuid ] + ['id' => $userId, 'uuid' => $uuid] ); } } @@ -115,7 +115,7 @@ public function postDown(Schema $schema): void $json = []; } - $updateData = [ 'id' => $sId ]; + $updateData = ['id' => $sId]; foreach (self::$transforms as $col => $uiProp) { $updateData[$col] = $json[$uiProp] ?? self::$defaults[$col]; unset($json[$uiProp]); @@ -123,7 +123,7 @@ public function postDown(Schema $schema): void $this->connection->executeStatement("UPDATE user_settings SET $sets WHERE id = :id", $updateData); $this->connection->executeStatement( "UPDATE user_ui_data SET `data` = :newData WHERE id = :id", - [ 'id' => $uiId, 'newData' => json_encode($json) ] + ['id' => $uiId, 'newData' => json_encode($json)] ); } } diff --git a/migrations/Version20260504154333.php b/migrations/Version20260504154333.php new file mode 100644 index 000000000..e16a1f067 --- /dev/null +++ b/migrations/Version20260504154333.php @@ -0,0 +1,111 @@ +connection->fetchAllAssociative("SELECT id, exam_lock_strict FROM `group`"); + foreach ($result as $row) { + $this->groupStrict[$row['id']] = $row['exam_lock_strict']; + } + + $result = $this->connection->fetchAllAssociative("SELECT id, lock_strict FROM `group_exam`"); + foreach ($result as $row) { + $this->examStrict[$row['id']] = $row['lock_strict']; + } + + $result = $this->connection->fetchAllAssociative("SELECT id, group_lock_strict FROM `user`"); + foreach ($result as $row) { + $this->userStrict[$row['id']] = $row['group_lock_strict']; + } + } + + public function up(Schema $schema): void + { + // this up() migration is auto-generated, please modify it to your needs + $this->addSql('ALTER TABLE `group` ADD exam_lock_type VARCHAR(255) NOT NULL, DROP exam_lock_strict'); + $this->addSql('ALTER TABLE `group_exam` ADD lock_type VARCHAR(255) NOT NULL, DROP lock_strict'); + $this->addSql('ALTER TABLE `user` ADD group_lock_type VARCHAR(255) NOT NULL, DROP group_lock_strict'); + } + + public function postUp(Schema $schema): void + { + foreach ($this->groupStrict as $id => $strict) { + $type = $strict ? GroupExamLockType::Restricted->value : GroupExamLockType::Visible->value; + $this->connection->executeQuery("UPDATE `group` SET exam_lock_type = '$type' WHERE id = '$id'"); + } + + foreach ($this->examStrict as $id => $strict) { + $type = $strict ? GroupExamLockType::Restricted->value : GroupExamLockType::Visible->value; + $this->connection->executeQuery("UPDATE `group_exam` SET lock_type = '$type' WHERE id = '$id'"); + } + + foreach ($this->userStrict as $id => $strict) { + $type = $strict ? GroupExamLockType::Restricted->value : GroupExamLockType::Visible->value; + $this->connection->executeQuery("UPDATE `user` SET group_lock_type = '$type' WHERE id = '$id'"); + } + } + + public function preDown(Schema $schema): void + { + $result = $this->connection->fetchAllAssociative("SELECT id, exam_lock_type FROM `group`"); + foreach ($result as $row) { + $this->groupStrict[$row['id']] = $row['exam_lock_type'] === GroupExamLockType::Restricted->value ? 1 : 0; + } + + $result = $this->connection->fetchAllAssociative("SELECT id, lock_type FROM `group_exam`"); + foreach ($result as $row) { + $this->examStrict[$row['id']] = $row['lock_type'] === GroupExamLockType::Restricted->value ? 1 : 0; + } + + $result = $this->connection->fetchAllAssociative("SELECT id, group_lock_type FROM `user`"); + foreach ($result as $row) { + $this->userStrict[$row['id']] = $row['group_lock_type'] === GroupExamLockType::Restricted->value ? 1 : 0; + } + } + + public function down(Schema $schema): void + { + // this down() migration is auto-generated, please modify it to your needs + $this->addSql('ALTER TABLE `group` ADD exam_lock_strict TINYINT(1) NOT NULL, DROP exam_lock_type'); + $this->addSql('ALTER TABLE `group_exam` ADD lock_strict TINYINT(1) NOT NULL, DROP lock_type'); + $this->addSql('ALTER TABLE `user` ADD group_lock_strict TINYINT(1) NOT NULL, DROP group_lock_type'); + } + + public function postDown(Schema $schema): void + { + foreach ($this->groupStrict as $id => $strict) { + $this->connection->executeQuery("UPDATE `group` SET exam_lock_strict = $strict WHERE id = '$id'"); + } + + foreach ($this->examStrict as $id => $strict) { + $this->connection->executeQuery("UPDATE `group_exam` SET lock_strict = $strict WHERE id = '$id'"); + } + + foreach ($this->userStrict as $id => $strict) { + $this->connection->executeQuery("UPDATE `user` SET group_lock_strict = $strict WHERE id = '$id'"); + } + } +} From 23193145585138d35f01165bdcb13bde22de198f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Mon, 4 May 2026 23:06:46 +0200 Subject: [PATCH 2/5] Updating presenters and views to match lock-related changes in the model. --- app/V1Module/presenters/GroupsPresenter.php | 38 +++++++++++++-------- app/commands/security/ListExamEvents.php | 2 ++ app/model/entity/User.php | 5 +++ app/model/view/GroupViewFactory.php | 3 +- app/model/view/UserViewFactory.php | 2 ++ 5 files changed, 35 insertions(+), 15 deletions(-) diff --git a/app/V1Module/presenters/GroupsPresenter.php b/app/V1Module/presenters/GroupsPresenter.php index 883fd7a93..1e4ec21fd 100644 --- a/app/V1Module/presenters/GroupsPresenter.php +++ b/app/V1Module/presenters/GroupsPresenter.php @@ -42,6 +42,7 @@ use App\Model\View\ShadowAssignmentViewFactory; use App\Model\View\GroupViewFactory; use App\Model\View\UserViewFactory; +use App\Model\GroupExamLockType; use App\Security\ACL\IAssignmentPermissions; use App\Security\ACL\IAssignmentSolutionPermissions; use App\Security\ACL\IShadowAssignmentPermissions; @@ -608,7 +609,7 @@ public function checkSetExamPeriod(string $id) "When the exam ends (unix ts in the future, no more than a day after 'begin').", required: true, )] - #[Post("strict", new VBool(), "Whether locked users are prevented from accessing other groups.", required: false)] + #[Post("type", new VString(), "Lock type ('visible', 'reviewed', 'accepted', 'restricted').", required: false)] #[Path("id", new VUuid(), "An identifier of the updated group", required: true)] #[ResponseFormat(GroupFormat::class)] public function actionSetExamPeriod(string $id) @@ -618,19 +619,28 @@ public function actionSetExamPeriod(string $id) $req = $this->getRequest(); $beginTs = (int)$req->getPost("begin"); $endTs = (int)$req->getPost("end"); - $strict = $req->getPost("strict") !== null - ? filter_var($req->getPost("strict"), FILTER_VALIDATE_BOOLEAN) : null; - $now = (new DateTime())->getTimestamp(); - $nowTolerance = 60; // 60s is a tolerance when comparing with "now" - if ($strict === null) { + $typeStr = $req->getPost("type"); + $type = null; + if ($typeStr !== null) { + $type = GroupExamLockType::tryFrom($typeStr); + if ($type === null) { + throw new InvalidApiArgumentException( + 'type', + "Invalid lock type. Allowed values are: " . implode(", ", GroupExamLockType::values()) + ); + } + } else { if ($group->hasExamPeriodSet()) { - $strict = $group->isExamLockStrict(); // flag is not present -> is not changing + $type = $group->getExamLockType(); // type is not present -> is not changing } else { - throw new BadRequestException("The strict flag must be present when new exam is being set."); + throw new BadRequestException("The lock type must be present when new exam is being set."); } } + $now = (new DateTime())->getTimestamp(); + $nowTolerance = 60; // 60s is a tolerance when comparing with "now" + // beginning must be in the future (or must not be modified) if ((!$group->hasExamPeriodSet() || $beginTs) && $beginTs < $now - $nowTolerance) { throw new BadRequestException("The exam must be set in the future."); @@ -655,14 +665,14 @@ public function actionSetExamPeriod(string $id) if ($group->hasExamPeriodSet()) { if ($group->getExamBegin()->getTimestamp() <= $now) { // ... already begun - if ($strict !== $group->isExamLockStrict()) { - throw new BadRequestException("The strict flag cannot be changed once the exam begins."); + if ($type !== $group->getExamLockType()) { + throw new BadRequestException("The lock type cannot be changed once the exam begins."); } // the exam already begun, we need to fix any group-locked users foreach ($group->getStudents() as $student) { if ($student->getGroupLock()?->getId() === $id) { - $student->setGroupLock($group, $end, $strict); + $student->setGroupLock($group, $end, $type); if ($student->isIpLocked()) { $student->setIpLock($student->getIpLockRaw(), $end); } @@ -696,11 +706,11 @@ public function actionSetExamPeriod(string $id) $exam = $this->groupExams->findPendingForGroup($group); if ($exam) { - $exam->update($begin, $end, $strict); + $exam->update($begin, $end, $type); $this->groupExams->persist($exam, false); } - $group->setExamPeriod($begin, $end, $strict); + $group->setExamPeriod($begin, $end, $type); $this->groups->persist($group); $this->sendSuccessResponse($this->groupViewFactory->getGroup($group)); @@ -1298,7 +1308,7 @@ public function actionLockStudent(string $id, string $userId) $expiration = $group->getExamEnd(); $user->setIpLock($this->getHttpRequest()->getRemoteAddress(), $expiration); - $user->setGroupLock($group, $expiration, $group->isExamLockStrict()); + $user->setGroupLock($group, $expiration, $group->getExamLockType()); $this->users->persist($user, false); // make sure the locking is also logged diff --git a/app/commands/security/ListExamEvents.php b/app/commands/security/ListExamEvents.php index 8cd7ad98d..262649b5e 100644 --- a/app/commands/security/ListExamEvents.php +++ b/app/commands/security/ListExamEvents.php @@ -85,6 +85,8 @@ protected function getLockData(GroupExamLock $lock): array 'remote_addr' => $lock->getRemoteAddr(), 'exam_begin_at' => $lock->getGroupExam()->getBegin()->getTimestamp(), 'exam_end_at' => $lock->getGroupExam()->getEnd()->getTimestamp(), + 'lock_type' => $lock->getGroupExam()->getLockType()->value, + // DEPRECATED, use lock_type instead 'lock_strict' => $lock->getGroupExam()->isLockStrict(), ]; } diff --git a/app/model/entity/User.php b/app/model/entity/User.php index b984a5e10..bbc7559b6 100644 --- a/app/model/entity/User.php +++ b/app/model/entity/User.php @@ -536,6 +536,11 @@ public function isGroupLockStrict(): bool return $this->groupLockType === GroupExamLockType::Restricted->value; } + public function getGroupLockType(): ?GroupExamLockType + { + return $this->isGroupLocked() ? GroupExamLockType::from($this->groupLockType) : null; + } + /** * @return Group|null The group in which the user is currently locked, null if no lock is active. */ diff --git a/app/model/view/GroupViewFactory.php b/app/model/view/GroupViewFactory.php index 3f3dfc482..4e317851f 100644 --- a/app/model/view/GroupViewFactory.php +++ b/app/model/view/GroupViewFactory.php @@ -281,7 +281,8 @@ function (ShadowAssignment $assignment) { "bindings" => $this->bindings->getBindingsForGroup($group), "examBegin" => $group->hasExamPeriodSet() ? $group->getExamBegin()?->getTimestamp() : null, "examEnd" => $group->hasExamPeriodSet() ? $group->getExamEnd()?->getTimestamp() : null, - "examLockStrict" => $group->hasExamPeriodSet() ? $group->isExamLockStrict() : null, + "examLockType" => $group->hasExamPeriodSet() ? $group->getExamLockType()->value : null, + "examLockStrict" => $group->hasExamPeriodSet() ? $group->isExamLockStrict() : null, // DEPRECATED "exams" => $group->getExams()->getValues(), ]; } diff --git a/app/model/view/UserViewFactory.php b/app/model/view/UserViewFactory.php index 6dbe8bd05..19463076e 100644 --- a/app/model/view/UserViewFactory.php +++ b/app/model/view/UserViewFactory.php @@ -88,6 +88,8 @@ private function getUserData(User $user, bool $canViewPrivate, bool $reallyShowE "externalIds" => $this->getExternalIds($user, $reallyShowEverything), "ipLock" => $user->isIpLocked() ? $user->getIpLockRaw() : null, "groupLock" => $user->getGroupLock()?->getId(), + "groupLockType" => $user->getGroupLockType()?->value, + // DEPRECATED, use groupLockType instead "isGroupLockStrict" => $user->isGroupLockStrict(), ]; From da7afd877cb932c34dbae0d378aa6a2d2de59144 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Mon, 4 May 2026 23:38:25 +0200 Subject: [PATCH 3/5] Fixing tests. --- tests/Console/ListExamEvents.phpt | 14 +++-- tests/Presenters/GroupsPresenter.phpt | 88 +++++++++++++++++++++------ tests/Security/UserLocking.phpt | 62 +++++++++---------- 3 files changed, 111 insertions(+), 53 deletions(-) diff --git a/tests/Console/ListExamEvents.phpt b/tests/Console/ListExamEvents.phpt index 954c5251c..91008b71b 100644 --- a/tests/Console/ListExamEvents.phpt +++ b/tests/Console/ListExamEvents.phpt @@ -10,6 +10,7 @@ use App\Model\Repository\Users; use App\Model\Repository\Groups; use App\Model\Repository\GroupExams; use App\Model\Repository\GroupExamLocks; +use App\Model\GroupExamLockType; use Doctrine\ORM\EntityManagerInterface; use Tester\Assert; @@ -102,7 +103,12 @@ class ListExamEventsTest extends Tester\TestCase $now = (new DateTime())->getTimestamp(); $begin = $now - 3600; $end = $now - 1800; - $exam = new GroupExam($group, DateTime::createFromFormat('U', $begin), DateTime::createFromFormat('U', $end), true); + $exam = new GroupExam( + $group, + DateTime::createFromFormat('U', $begin), + DateTime::createFromFormat('U', $end), + GroupExamLockType::Restricted + ); $this->exams->persist($exam); $lock = new GroupExamLock($exam, $user, '127.0.0.1'); @@ -143,7 +149,7 @@ class ListExamEventsTest extends Tester\TestCase $group, DateTime::createFromFormat('U', $now - 100000), DateTime::createFromFormat('U', $now - 98000), - true + GroupExamLockType::Restricted ); $this->exams->persist($exam); $lock = new GroupExamLock($exam, $user, '127.0.0.1'); @@ -155,7 +161,7 @@ class ListExamEventsTest extends Tester\TestCase $group, DateTime::createFromFormat('U', $now - 3000), DateTime::createFromFormat('U', $now), - true + GroupExamLockType::Restricted ); $this->exams->persist($exam); $lock = new GroupExamLock($exam, $user, '127.0.0.1'); @@ -167,7 +173,7 @@ class ListExamEventsTest extends Tester\TestCase $group, DateTime::createFromFormat('U', $now - 7200), DateTime::createFromFormat('U', $now - 3600), - true + GroupExamLockType::Restricted ); $this->exams->persist($exam); $lock = new GroupExamLock($exam, $user, '127.0.0.1'); diff --git a/tests/Presenters/GroupsPresenter.phpt b/tests/Presenters/GroupsPresenter.phpt index 02d1aefe4..6d2895997 100644 --- a/tests/Presenters/GroupsPresenter.phpt +++ b/tests/Presenters/GroupsPresenter.phpt @@ -10,6 +10,7 @@ use App\Model\Entity\GroupExamLock; use App\Model\Entity\Instance; use App\Model\Entity\User; use App\Model\Repository\Users; +use App\Model\GroupExamLockType; use App\Helpers\FileStorageManager; use App\Helpers\TmpFilesHelper; use App\Helpers\FileStorage\LocalFileStorage; @@ -1464,7 +1465,7 @@ class TestGroupsPresenter extends Tester\TestCase 'V1:Groups', 'POST', ['action' => 'setExamPeriod', 'id' => $group->getId()], - ['begin' => $begin, 'end' => $end, 'strict' => true] + ['begin' => $begin, 'end' => $end, 'type' => 'restricted'] ); Assert::equal($group->getId(), $payload['id']); @@ -1493,7 +1494,7 @@ class TestGroupsPresenter extends Tester\TestCase 'V1:Groups', 'POST', ['action' => 'setExamPeriod', 'id' => $group->getId()], - ['begin' => $begin, 'end' => $end, 'strict' => true] + ['begin' => $begin, 'end' => $end, 'type' => 'restricted'] ); }, BadRequestException::class @@ -1507,7 +1508,11 @@ class TestGroupsPresenter extends Tester\TestCase $now = (new DateTime())->getTimestamp(); $begin = $now + 3600; $end = $now + 7200; - $group->setExamPeriod(DateTime::createFromFormat('U', $begin), DateTime::createFromFormat('U', $end), true); + $group->setExamPeriod( + DateTime::createFromFormat('U', $begin), + DateTime::createFromFormat('U', $end), + GroupExamLockType::Restricted + ); $this->presenter->groups->persist($group); Assert::true($group->isExamLockStrict()); @@ -1519,7 +1524,7 @@ class TestGroupsPresenter extends Tester\TestCase 'V1:Groups', 'POST', ['action' => 'setExamPeriod', 'id' => $group->getId()], - ['begin' => $begin, 'end' => $end, 'strict' => false] + ['begin' => $begin, 'end' => $end, 'type' => 'visible'] ); Assert::equal($group->getId(), $payload['id']); @@ -1541,7 +1546,11 @@ class TestGroupsPresenter extends Tester\TestCase $now = (new DateTime())->getTimestamp(); $begin = $now - 3600; $end = $now + 3600; - $group->setExamPeriod(DateTime::createFromFormat('U', $begin), DateTime::createFromFormat('U', $end)); + $group->setExamPeriod( + DateTime::createFromFormat('U', $begin), + DateTime::createFromFormat('U', $end), + GroupExamLockType::Visible + ); $this->presenter->groups->persist($group); $end += 3600; // let's give it another hour @@ -1570,7 +1579,11 @@ class TestGroupsPresenter extends Tester\TestCase $now = (new DateTime())->getTimestamp(); $begin = $now - 3600; $end = $now + 3600; - $group->setExamPeriod(DateTime::createFromFormat('U', $begin), DateTime::createFromFormat('U', $end)); + $group->setExamPeriod( + DateTime::createFromFormat('U', $begin), + DateTime::createFromFormat('U', $end), + GroupExamLockType::Visible + ); $this->presenter->groups->persist($group); $end = $now; // truncate the rest of the exam @@ -1598,7 +1611,11 @@ class TestGroupsPresenter extends Tester\TestCase $now = (new DateTime())->getTimestamp(); $begin = $now - 3600; $end = $now + 3600; - $group->setExamPeriod(DateTime::createFromFormat('U', $begin), DateTime::createFromFormat('U', $end)); + $group->setExamPeriod( + DateTime::createFromFormat('U', $begin), + DateTime::createFromFormat('U', $end), + GroupExamLockType::Visible + ); $this->presenter->groups->persist($group); $exam = $this->presenter->groupExams->findOrCreate($group); @@ -1634,7 +1651,11 @@ class TestGroupsPresenter extends Tester\TestCase $now = (new DateTime())->getTimestamp(); $begin = $now - 3600; $end = $now + 3600; - $group->setExamPeriod(DateTime::createFromFormat('U', $begin), DateTime::createFromFormat('U', $end)); + $group->setExamPeriod( + DateTime::createFromFormat('U', $begin), + DateTime::createFromFormat('U', $end), + GroupExamLockType::Visible + ); $this->presenter->groups->persist($group); $begin += 100; @@ -1661,7 +1682,11 @@ class TestGroupsPresenter extends Tester\TestCase $now = (new DateTime())->getTimestamp(); $begin = $now - 3600; $end = $now + 3600; - $group->setExamPeriod(DateTime::createFromFormat('U', $begin), DateTime::createFromFormat('U', $end), true); + $group->setExamPeriod( + DateTime::createFromFormat('U', $begin), + DateTime::createFromFormat('U', $end), + GroupExamLockType::Restricted + ); $this->presenter->groups->persist($group); $end += 100; @@ -1673,7 +1698,7 @@ class TestGroupsPresenter extends Tester\TestCase 'V1:Groups', 'POST', ['action' => 'setExamPeriod', 'id' => $group->getId()], - ['end' => $end, 'strict' => false] + ['end' => $end, 'type' => 'visible'] ); }, BadRequestException::class @@ -1687,7 +1712,11 @@ class TestGroupsPresenter extends Tester\TestCase $now = (new DateTime())->getTimestamp(); $begin = $now - 7200; $end = $now - 3600; - $group->setExamPeriod(DateTime::createFromFormat('U', $begin), DateTime::createFromFormat('U', $end)); + $group->setExamPeriod( + DateTime::createFromFormat('U', $begin), + DateTime::createFromFormat('U', $end), + GroupExamLockType::Visible + ); $this->presenter->groups->persist($group); $end = $now; @@ -1713,7 +1742,11 @@ class TestGroupsPresenter extends Tester\TestCase $now = (new DateTime())->getTimestamp(); $begin = $now + 3600; $end = $now + 7200; - $group->setExamPeriod(DateTime::createFromFormat('U', $begin), DateTime::createFromFormat('U', $end)); + $group->setExamPeriod( + DateTime::createFromFormat('U', $begin), + DateTime::createFromFormat('U', $end), + GroupExamLockType::Visible + ); $this->presenter->groups->persist($group); $payload = PresenterTestHelper::performPresenterRequest( @@ -1737,7 +1770,11 @@ class TestGroupsPresenter extends Tester\TestCase $now = (new DateTime())->getTimestamp(); $begin = $now - 3600; $end = $now + 3600; - $group->setExamPeriod(DateTime::createFromFormat('U', $begin), DateTime::createFromFormat('U', $end)); + $group->setExamPeriod( + DateTime::createFromFormat('U', $begin), + DateTime::createFromFormat('U', $end), + GroupExamLockType::Visible + ); $this->presenter->groups->persist($group); Assert::exception( @@ -1760,7 +1797,11 @@ class TestGroupsPresenter extends Tester\TestCase $now = (new DateTime())->getTimestamp(); $begin = $now - 7200; $end = $now - 3600; - $group->setExamPeriod(DateTime::createFromFormat('U', $begin), DateTime::createFromFormat('U', $end)); + $group->setExamPeriod( + DateTime::createFromFormat('U', $begin), + DateTime::createFromFormat('U', $end), + GroupExamLockType::Visible + ); $this->presenter->groups->persist($group); Assert::exception( @@ -1784,10 +1825,19 @@ class TestGroupsPresenter extends Tester\TestCase $now = (new DateTime())->getTimestamp(); $begin = $now - 7200; $end = $now - 3600; - $group->setExamPeriod(DateTime::createFromFormat('U', $begin), DateTime::createFromFormat('U', $end)); + $group->setExamPeriod( + DateTime::createFromFormat('U', $begin), + DateTime::createFromFormat('U', $end), + GroupExamLockType::Visible + ); $this->presenter->groups->persist($group); - $exam = new GroupExam($group, DateTime::createFromFormat('U', $begin), DateTime::createFromFormat('U', $end), false); + $exam = new GroupExam( + $group, + DateTime::createFromFormat('U', $begin), + DateTime::createFromFormat('U', $end), + GroupExamLockType::Visible + ); $this->presenter->groupExams->persist($exam); $lock = new GroupExamLock($exam, $student, '1.2.3.4'); @@ -1930,7 +1980,11 @@ class TestGroupsPresenter extends Tester\TestCase $now = (new DateTime())->getTimestamp(); $begin = $now + 3600; $end = $now + 7200; - $group->setExamPeriod(DateTime::createFromFormat('U', $begin), DateTime::createFromFormat('U', $end)); + $group->setExamPeriod( + DateTime::createFromFormat('U', $begin), + DateTime::createFromFormat('U', $end), + GroupExamLockType::Visible + ); $this->presenter->groups->persist($group); Assert::exception( diff --git a/tests/Security/UserLocking.phpt b/tests/Security/UserLocking.phpt index f98e0dbc1..1397ec2a9 100644 --- a/tests/Security/UserLocking.phpt +++ b/tests/Security/UserLocking.phpt @@ -2,18 +2,13 @@ $container = require_once __DIR__ . "/../bootstrap.php"; -use App\Exceptions\BadRequestException; +use App\Model\GroupExamLockType; use App\Model\Entity\Group; -use App\Model\Entity\Instance; -use App\Model\Entity\User; -use App\Model\Entity\GroupMembership; use App\Model\Entity\CommentThread; -use App\Model\Repository\Users; use App\V1Module\Presenters\AssignmentsPresenter; use App\V1Module\Presenters\CommentsPresenter; use App\V1Module\Presenters\GroupsPresenter; use App\Helpers\FileStorageManager; -use App\Helpers\FileStorage\LocalImmutableFile; use App\Helpers\TmpFilesHelper; use App\Helpers\FileStorage\LocalFileStorage; use App\Helpers\FileStorage\LocalHashFileStorage; @@ -29,7 +24,6 @@ $_SERVER['REMOTE_ADDR'] = '2001:db8::1428:57ab'; class UserLocking extends Tester\TestCase { private $studentLogin = "submitUser1@example.com"; - private $studentPassword = "password"; private $student2Login = "demoGroupMember1@example.com"; private $ip = '2001:0db8:0:0::1428:57ab'; // must be compatible with what's in $_SERVER['REMOTE_ADDR'] @@ -107,8 +101,12 @@ class UserLocking extends Tester\TestCase return $res; } - private function prepExamGroup($student, $relBegin = null, $relEnd = null, $strict = true): Group - { + private function prepExamGroup( + $student, + $relBegin = null, + $relEnd = null, + GroupExamLockType $type = GroupExamLockType::Restricted + ): Group { $groups = $this->getAllGroupsInDepth( 2, function (Group $g) { @@ -124,7 +122,7 @@ class UserLocking extends Tester\TestCase $group->setExamPeriod( DateTime::createFromFormat('U', $now + $relBegin), DateTime::createFromFormat('U', $now + $relEnd), - $strict + $type ); } @@ -259,7 +257,7 @@ class UserLocking extends Tester\TestCase $exp = new DateTime(); $exp->modify("-1 hour"); // already expired $student->setIpLock($this->ip, $exp); - $student->setGroupLock($group, $exp, $group->isExamLockStrict()); + $student->setGroupLock($group, $exp, $group->getExamLockType()); Assert::false($student->isIpLocked()); Assert::true($student->verifyIpLock('127.0.0.1')); Assert::false($student->isGroupLocked()); @@ -326,7 +324,7 @@ class UserLocking extends Tester\TestCase $group = $this->prepExamGroup($student, -3600, 3600); $student->setIpLock($this->ip, $group->getExamEnd()); - $student->setGroupLock($group, $group->getExamEnd(), $group->isExamLockStrict()); + $student->setGroupLock($group, $group->getExamEnd(), $group->getExamLockType()); $this->presenter->users->persist($student); Assert::exception( @@ -348,7 +346,7 @@ class UserLocking extends Tester\TestCase $group = $this->prepExamGroup($student, -3600, 3600); $student->setIpLock($this->ip, $group->getExamEnd()); - $student->setGroupLock($group, $group->getExamEnd(), $group->isExamLockStrict()); + $student->setGroupLock($group, $group->getExamEnd(), $group->getExamLockType()); $this->presenter->users->persist($student); $payload = PresenterTestHelper::performPresenterRequest( @@ -417,7 +415,7 @@ class UserLocking extends Tester\TestCase $assignment = $assignments->toArray()[0]; $student->setIpLock($this->ip, $group->getExamEnd()); - $student->setGroupLock($group, $group->getExamEnd(), $group->isExamLockStrict()); + $student->setGroupLock($group, $group->getExamEnd(), $group->getExamLockType()); $this->presenter->users->persist($student); $payload = PresenterTestHelper::performPresenterRequest( @@ -432,10 +430,10 @@ class UserLocking extends Tester\TestCase public function testStrictLockedUserCannotSeeAssignmentsInOtherGroups() { $student = $this->presenter->users->getByEmail($this->studentLogin); - $group = $this->prepExamGroup($student, -3600, 3600, true); // true = strict + $group = $this->prepExamGroup($student, -3600, 3600); $student->setIpLock($this->ip, $group->getExamEnd()); - $student->setGroupLock($group, $group->getExamEnd(), $group->isExamLockStrict()); + $student->setGroupLock($group, $group->getExamEnd(), $group->getExamLockType()); $this->presenter->users->persist($student); Assert::exception( @@ -454,10 +452,10 @@ class UserLocking extends Tester\TestCase public function testLockedUserCanSeeAssignmentsInOtherGroups() { $student = $this->presenter->users->getByEmail($this->studentLogin); - $group = $this->prepExamGroup($student, -3600, 3600, false); // false = not strict + $group = $this->prepExamGroup($student, -3600, 3600, GroupExamLockType::Visible); $student->setIpLock($this->ip, $group->getExamEnd()); - $student->setGroupLock($group, $group->getExamEnd(), $group->isExamLockStrict()); + $student->setGroupLock($group, $group->getExamEnd(), $group->getExamLockType()); $this->presenter->users->persist($student); $payload = PresenterTestHelper::performPresenterRequest( @@ -473,10 +471,10 @@ class UserLocking extends Tester\TestCase { $this->presenter = PresenterTestHelper::createPresenter($this->container, AssignmentsPresenter::class); $student = $this->presenter->users->getByEmail($this->studentLogin); - $group = $this->prepExamGroup($student, -3600, 3600, true); // true = strict + $group = $this->prepExamGroup($student, -3600, 3600); $student->setIpLock($this->ip, $group->getExamEnd()); - $student->setGroupLock($group, $group->getExamEnd(), $group->isExamLockStrict()); + $student->setGroupLock($group, $group->getExamEnd(), $group->getExamLockType()); $this->presenter->users->persist($student); $group = $group->getParentGroup(); @@ -501,10 +499,10 @@ class UserLocking extends Tester\TestCase { $this->presenter = PresenterTestHelper::createPresenter($this->container, AssignmentsPresenter::class); $student = $this->presenter->users->getByEmail($this->studentLogin); - $group = $this->prepExamGroup($student, -3600, 3600, false); // false = not strict + $group = $this->prepExamGroup($student, -3600, 3600, GroupExamLockType::Visible); $student->setIpLock($this->ip, $group->getExamEnd()); - $student->setGroupLock($group, $group->getExamEnd(), $group->isExamLockStrict()); + $student->setGroupLock($group, $group->getExamEnd(), $group->getExamLockType()); $this->presenter->users->persist($student); $group = $group->getParentGroup(); @@ -528,7 +526,7 @@ class UserLocking extends Tester\TestCase // unexpected IP $student->setIpLock('192.168.42.54', $group->getExamEnd()); - $student->setGroupLock($group, $group->getExamEnd(), $group->isExamLockStrict()); + $student->setGroupLock($group, $group->getExamEnd(), $group->getExamLockType()); $this->presenter->users->persist($student); Assert::exception( @@ -548,7 +546,7 @@ class UserLocking extends Tester\TestCase { PresenterTestHelper::login($this->container, $this->student2Login); $student = $this->presenter->users->getByEmail($this->student2Login); - $group = $this->prepExamGroup($student, -3600, 3600, false); + $group = $this->prepExamGroup($student, -3600, 3600, GroupExamLockType::Visible); $this->presenter = PresenterTestHelper::createPresenter($this->container, CommentsPresenter::class); $assignments = $group->getAssignments()->filter(function ($a) { @@ -562,7 +560,7 @@ class UserLocking extends Tester\TestCase $solution = $solutions->toArray()[0]; $student->setIpLock($this->ip, $group->getExamEnd()); - $student->setGroupLock($group, $group->getExamEnd(), $group->isExamLockStrict()); + $student->setGroupLock($group, $group->getExamEnd(), $group->getExamLockType()); $this->presenter->users->persist($student); Assert::exception( @@ -583,7 +581,7 @@ class UserLocking extends Tester\TestCase { PresenterTestHelper::login($this->container, $this->student2Login); $student = $this->presenter->users->getByEmail($this->student2Login); - $group = $this->prepExamGroup($student, -3600, 3600, false); + $group = $this->prepExamGroup($student, -3600, 3600, GroupExamLockType::Visible); $this->presenter = PresenterTestHelper::createPresenter($this->container, CommentsPresenter::class); $assignments = $group->getAssignments()->filter(function ($a) { @@ -597,7 +595,7 @@ class UserLocking extends Tester\TestCase $solution = $solutions->toArray()[0]; $student->setIpLock($this->ip, $group->getExamEnd()); - $student->setGroupLock($group, $group->getExamEnd(), $group->isExamLockStrict()); + $student->setGroupLock($group, $group->getExamEnd(), $group->getExamLockType()); $this->presenter->users->persist($student); $thread = CommentThread::createThread($solution->getId()); @@ -621,7 +619,7 @@ class UserLocking extends Tester\TestCase { PresenterTestHelper::login($this->container, $this->student2Login); $student = $this->presenter->users->getByEmail($this->student2Login); - $group = $this->prepExamGroup($student, -3600, 3600, false); + $group = $this->prepExamGroup($student, -3600, 3600, GroupExamLockType::Visible); $this->presenter = PresenterTestHelper::createPresenter($this->container, CommentsPresenter::class); $assignments = $group->getAssignments(); @@ -629,7 +627,7 @@ class UserLocking extends Tester\TestCase $assignment = $assignments->toArray()[0]; $student->setIpLock($this->ip, $group->getExamEnd()); - $student->setGroupLock($group, $group->getExamEnd(), $group->isExamLockStrict()); + $student->setGroupLock($group, $group->getExamEnd(), $group->getExamLockType()); $this->presenter->users->persist($student); Assert::exception( @@ -650,7 +648,7 @@ class UserLocking extends Tester\TestCase { PresenterTestHelper::login($this->container, $this->student2Login); $student = $this->presenter->users->getByEmail($this->student2Login); - $group = $this->prepExamGroup($student, -3600, 3600, false); + $group = $this->prepExamGroup($student, -3600, 3600, GroupExamLockType::Visible); $this->presenter = PresenterTestHelper::createPresenter($this->container, CommentsPresenter::class); $assignments = $group->getAssignments(); @@ -658,7 +656,7 @@ class UserLocking extends Tester\TestCase $assignment = $assignments->toArray()[0]; $student->setIpLock($this->ip, $group->getExamEnd()); - $student->setGroupLock($group, $group->getExamEnd(), $group->isExamLockStrict()); + $student->setGroupLock($group, $group->getExamEnd(), $group->getExamLockType()); $this->presenter->users->persist($student); $thread = CommentThread::createThread($assignment->getId()); @@ -685,7 +683,7 @@ class UserLocking extends Tester\TestCase PresenterTestHelper::loginDefaultAdmin($this->container); $student->setIpLock($this->ip, $group->getExamEnd()); - $student->setGroupLock($group, $group->getExamEnd(), $group->isExamLockStrict()); + $student->setGroupLock($group, $group->getExamEnd(), $group->getExamLockType()); $this->presenter->users->persist($student); Assert::same($group->getExamEnd()->getTimestamp(), $student->getGroupLockExpiration()?->getTimestamp()); From ff022c80338637d63dd9e86bb926ca1a6085d478 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Tue, 5 May 2026 13:27:13 +0200 Subject: [PATCH 4/5] Implementing necessary changes in ACL group-lock checks. --- .../AssignmentSolutionPermissionPolicy.php | 30 ++++++++++++++++--- app/config/permissions.neon | 6 ++-- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/app/V1Module/security/Policies/AssignmentSolutionPermissionPolicy.php b/app/V1Module/security/Policies/AssignmentSolutionPermissionPolicy.php index da78606e1..d049efd56 100644 --- a/app/V1Module/security/Policies/AssignmentSolutionPermissionPolicy.php +++ b/app/V1Module/security/Policies/AssignmentSolutionPermissionPolicy.php @@ -4,6 +4,7 @@ use App\Model\Entity\AssignmentSolution; use App\Model\Entity\GroupMembership; +use App\Model\GroupExamLockType; use App\Security\Identity; class AssignmentSolutionPermissionPolicy extends BasePermissionPolicy implements IPermissionPolicy @@ -107,9 +108,10 @@ public function userIsNotLockedElsewhere(Identity $identity, AssignmentSolution } /** - * Current user is either not locked at all, or locked to this group, or the current lock is not strict. + * Current user is either not locked at all, or locked to this group (where the solution is), + * or the current lock type allows (read-only) access to this solution. */ - public function userIsNotLockedElsewhereStrictly(Identity $identity, AssignmentSolution $solution): bool + public function userGroupLockTypeAllowsReadAccess(Identity $identity, AssignmentSolution $solution): bool { $user = $identity->getUserData(); $group = $solution->getAssignment()?->getGroup(); @@ -117,7 +119,27 @@ public function userIsNotLockedElsewhereStrictly(Identity $identity, AssignmentS return false; } - return !$user->isGroupLocked() || $user->getGroupLock()->getId() === $group->getId() - || !$user->isGroupLockStrict(); + if (!$user->isGroupLocked() || $user->getGroupLock()->getId() === $group->getId()) { + return true; + } + + $lockType = $user->getGroupLockType(); + if ($lockType === null || $lockType === GroupExamLockType::Visible) { + return true; + } + + if ($lockType === GroupExamLockType::Restricted) { + return false; // a shortcut (false is also at the end) + } + + if ($lockType === GroupExamLockType::Accepted) { + return $solution->isAccepted(); + } + + if ($lockType === GroupExamLockType::Reviewed) { + return $solution->isAccepted() || $solution->isReviewed(); + } + + return false; } } diff --git a/app/config/permissions.neon b/app/config/permissions.neon index 8a55558f2..efeda0b7a 100644 --- a/app/config/permissions.neon +++ b/app/config/permissions.neon @@ -612,7 +612,7 @@ permissions: - viewReview conditions: - assignmentSolution.isAuthor - - assignmentSolution.userIsNotLockedElsewhereStrictly + - assignmentSolution.userGroupLockTypeAllowsReadAccess - allow: true role: student @@ -633,7 +633,7 @@ permissions: conditions: - assignmentSolution.areEvaluationDetailsPublic - assignmentSolution.isAuthor - - assignmentSolution.userIsNotLockedElsewhereStrictly + - assignmentSolution.userGroupLockTypeAllowsReadAccess - allow: true role: student @@ -644,7 +644,7 @@ permissions: - assignmentSolution.areEvaluationDetailsPublic - assignmentSolution.areMeasuredValuesPublic - assignmentSolution.isAuthor - - assignmentSolution.userIsNotLockedElsewhereStrictly + - assignmentSolution.userGroupLockTypeAllowsReadAccess - allow: true role: supervisor-student From 049ed164d9b2c9d988fb39563379fbca34d47b55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Tue, 5 May 2026 18:00:04 +0200 Subject: [PATCH 5/5] Adding tests for new lock-types. --- .../GroupPrivateDataFormat.php | 9 +- tests/Presenters/GroupsPresenter.phpt | 4 +- tests/Security/UserLocking.phpt | 280 +++++++++++++++++- 3 files changed, 285 insertions(+), 8 deletions(-) diff --git a/app/helpers/MetaFormats/FormatDefinitions/GroupPrivateDataFormat.php b/app/helpers/MetaFormats/FormatDefinitions/GroupPrivateDataFormat.php index 454e69617..e18c5c94f 100644 --- a/app/helpers/MetaFormats/FormatDefinitions/GroupPrivateDataFormat.php +++ b/app/helpers/MetaFormats/FormatDefinitions/GroupPrivateDataFormat.php @@ -11,6 +11,7 @@ use App\Helpers\MetaFormats\Validators\VInt; use App\Helpers\MetaFormats\Validators\VTimestamp; use App\Helpers\MetaFormats\Validators\VUuid; +use App\Helpers\MetaFormats\Validators\VString; /** * Nested Format definition used by the GroupFormat. @@ -71,8 +72,12 @@ class GroupPrivateDataFormat extends MetaFormat #[FPost(new VTimestamp(), "The time when the exam ends if there is an exam scheduled", required: false)] public ?int $examEnd; - #[FPost(new VBool(), "Whether the scheduled exam requires a strict access lock", required: false)] - public ?bool $examLockStrict; + #[FPost( + new VString(), + "Type (restriction level) of the exam lock ('restricted', 'accepted', 'reviewed', 'visible')", + required: false + )] + public ?string $examLockType; #[FPost(new VArray(), "All past exams (with at least one student locked)")] public array $exams; diff --git a/tests/Presenters/GroupsPresenter.phpt b/tests/Presenters/GroupsPresenter.phpt index 6d2895997..7e3b09726 100644 --- a/tests/Presenters/GroupsPresenter.phpt +++ b/tests/Presenters/GroupsPresenter.phpt @@ -1530,13 +1530,13 @@ class TestGroupsPresenter extends Tester\TestCase Assert::equal($group->getId(), $payload['id']); Assert::equal($begin, $payload['privateData']['examBegin']); Assert::equal($end, $payload['privateData']['examEnd']); - Assert::false($payload['privateData']['examLockStrict']); + Assert::equal('visible', $payload['privateData']['examLockType']); $this->presenter->groups->refresh($group); Assert::true($group->hasExamPeriodSet()); Assert::equal($begin, $group->getExamBegin()?->getTimestamp()); Assert::equal($end, $group->getExamEnd()?->getTimestamp()); - Assert::false($group->isExamLockStrict()); + Assert::equal(GroupExamLockType::Visible, $group->getExamLockType()); } public function testUpdatePendingExamPeriod() diff --git a/tests/Security/UserLocking.phpt b/tests/Security/UserLocking.phpt index 1397ec2a9..2bcc6d2c7 100644 --- a/tests/Security/UserLocking.phpt +++ b/tests/Security/UserLocking.phpt @@ -8,6 +8,7 @@ use App\Model\Entity\CommentThread; use App\V1Module\Presenters\AssignmentsPresenter; use App\V1Module\Presenters\CommentsPresenter; use App\V1Module\Presenters\GroupsPresenter; +use App\V1Module\Presenters\AssignmentSolutionsPresenter; use App\Helpers\FileStorageManager; use App\Helpers\TmpFilesHelper; use App\Helpers\FileStorage\LocalFileStorage; @@ -32,6 +33,9 @@ class UserLocking extends Tester\TestCase /** @var GroupsPresenter */ protected $presenter; + /** @var AssignmentSolutionsPresenter */ + protected $assignmentSolutionsPresenter; + /** @var EntityManagerInterface */ protected $em; @@ -48,7 +52,7 @@ class UserLocking extends Tester\TestCase $this->em = PresenterTestHelper::getEntityManager($container); $this->user = $container->getByType(\Nette\Security\User::class); - // patch container, since we cannot create actual file storage manarer + // patch container, since we cannot create actual file storage manager $fsName = current($this->container->findByType(FileStorageManager::class)); $this->container->removeService($fsName); $this->container->addService($fsName, new FileStorageManager( @@ -64,6 +68,7 @@ class UserLocking extends Tester\TestCase PresenterTestHelper::fillDatabase($this->container); PresenterTestHelper::login($this->container, $this->studentLogin); $this->presenter = PresenterTestHelper::createPresenter($this->container, GroupsPresenter::class); + $this->assignmentSolutionsPresenter = PresenterTestHelper::createPresenter($this->container, AssignmentSolutionsPresenter::class); } protected function tearDown() @@ -149,6 +154,7 @@ class UserLocking extends Tester\TestCase Assert::equal($student->getId(), $payload['user']['id']); Assert::equal($group->getId(), $payload['user']['privateData']['groupLock']); + Assert::equal(GroupExamLockType::Restricted->value, $payload['user']['privateData']['groupLockType']); Assert::equal($_SERVER['REMOTE_ADDR'], $payload['user']['privateData']['ipLock']); Assert::equal($group->getExamEnd()->getTimestamp(), $payload['user']['privateData']['groupLockExpiration']); Assert::equal($group->getExamEnd()->getTimestamp(), $payload['user']['privateData']['ipLockExpiration']); @@ -169,10 +175,10 @@ class UserLocking extends Tester\TestCase public function testSecondStudentLocksInGroup() { $student = $this->presenter->users->getByEmail($this->studentLogin); - $group = $this->prepExamGroup($student, -3600, 3600); // exam in progress + $group = $this->prepExamGroup($student, -3600, 3600, GroupExamLockType::Accepted); // exam in progress // create group exam simulates situation where some previous student locked in - $groupExam = $this->presenter->groupExams->findOrCreate($group); + $this->presenter->groupExams->findOrCreate($group); $payload = PresenterTestHelper::performPresenterRequest( $this->presenter, @@ -188,6 +194,7 @@ class UserLocking extends Tester\TestCase Assert::equal($student->getId(), $payload['user']['id']); Assert::equal($group->getId(), $payload['user']['privateData']['groupLock']); + Assert::equal(GroupExamLockType::Accepted->value, $payload['user']['privateData']['groupLockType']); Assert::equal($_SERVER['REMOTE_ADDR'], $payload['user']['privateData']['ipLock']); Assert::equal($group->getExamEnd()->getTimestamp(), $payload['user']['privateData']['groupLockExpiration']); Assert::equal($group->getExamEnd()->getTimestamp(), $payload['user']['privateData']['ipLockExpiration']); @@ -519,7 +526,272 @@ class UserLocking extends Tester\TestCase Assert::count(4, $payload); } - public function testIpLockPrevetsOtherIps() + public function testLockedUserCanSeeSolutionDetailsInOtherGroups() + { + $this->presenter = PresenterTestHelper::createPresenter($this->container, AssignmentsPresenter::class); + $student = $this->presenter->users->getByEmail($this->studentLogin); + $group = $this->prepExamGroup($student, -3600, 3600, GroupExamLockType::Visible); + + $student->setIpLock($this->ip, $group->getExamEnd()); + $student->setGroupLock($group, $group->getExamEnd(), $group->getExamLockType()); + $this->presenter->users->persist($student); + + $group = $group->getParentGroup(); + $assignments = $group->getAssignments(); + Assert::count(1, $assignments); + $assignment = $assignments->toArray()[0]; + Assert::count(4, $assignment->getAssignmentSolutions()); + $solutions = $assignment->getAssignmentSolutions()->filter(function ($s) { + return !$s->isAccepted() && !$s->isReviewed() && $s->getReviewStartedAt() !== null; + }); + Assert::count(1, $solutions); + $solution = $solutions->first(); + + $payload = PresenterTestHelper::performPresenterRequest( + $this->assignmentSolutionsPresenter, + 'V1:AssignmentSolutions', + 'GET', + ['action' => 'solution', 'id' => $solution->getId()] + ); + Assert::equal($solution->getId(), $payload["id"] ?? null); + } + + public function testLockedUserCannotUpdateSolutionsInOtherGroups() + { + $this->presenter = PresenterTestHelper::createPresenter($this->container, AssignmentsPresenter::class); + $student = $this->presenter->users->getByEmail($this->studentLogin); + $group = $this->prepExamGroup($student, -3600, 3600, GroupExamLockType::Visible); + + $student->setIpLock($this->ip, $group->getExamEnd()); + $student->setGroupLock($group, $group->getExamEnd(), $group->getExamLockType()); + $this->presenter->users->persist($student); + + $group = $group->getParentGroup(); + $assignments = $group->getAssignments(); + Assert::count(1, $assignments); + $assignment = $assignments->toArray()[0]; + Assert::count(4, $assignment->getAssignmentSolutions()); + $solutions = $assignment->getAssignmentSolutions()->filter(function ($s) { + return !$s->isAccepted() && !$s->isReviewed() && $s->getReviewStartedAt() !== null; + }); + Assert::count(1, $solutions); + $solution = $solutions->first(); + + Assert::exception( + function () use ($solution) { + PresenterTestHelper::performPresenterRequest( + $this->assignmentSolutionsPresenter, + 'V1:AssignmentSolutions', + 'POST', + ['action' => 'updateSolution', 'id' => $solution->getId()], + ['note' => 'new note'] + ); + }, + App\Exceptions\ForbiddenRequestException::class + ); + } + + public function testLockedUserCannotSeeSolutionDetailsInOtherGroupsWhenRestricted() + { + $this->presenter = PresenterTestHelper::createPresenter($this->container, AssignmentsPresenter::class); + $student = $this->presenter->users->getByEmail($this->studentLogin); + $group = $this->prepExamGroup($student, -3600, 3600, GroupExamLockType::Restricted); + + $student->setIpLock($this->ip, $group->getExamEnd()); + $student->setGroupLock($group, $group->getExamEnd(), $group->getExamLockType()); + $this->presenter->users->persist($student); + + $group = $group->getParentGroup(); + $assignments = $group->getAssignments(); + Assert::count(1, $assignments); + $assignment = $assignments->toArray()[0]; + Assert::count(4, $assignment->getAssignmentSolutions()); + $solutions = $assignment->getAssignmentSolutions()->filter(function ($s) { + return !$s->isAccepted() && !$s->isReviewed() && $s->getReviewStartedAt() !== null; + }); + Assert::count(1, $solutions); + $solution = $solutions->first(); + + Assert::exception( + function () use ($solution) { + PresenterTestHelper::performPresenterRequest( + $this->assignmentSolutionsPresenter, + 'V1:AssignmentSolutions', + 'GET', + ['action' => 'solution', 'id' => $solution->getId()] + ); + }, + App\Exceptions\ForbiddenRequestException::class + ); + } + + public function testLockedUserCanSeeAcceptedSolutionDetailsInOtherGroupsWhenAccepted() + { + $this->presenter = PresenterTestHelper::createPresenter($this->container, AssignmentsPresenter::class); + $student = $this->presenter->users->getByEmail($this->studentLogin); + $group = $this->prepExamGroup($student, -3600, 3600, GroupExamLockType::Accepted); + + $student->setIpLock($this->ip, $group->getExamEnd()); + $student->setGroupLock($group, $group->getExamEnd(), $group->getExamLockType()); + $this->presenter->users->persist($student); + + $group = $group->getParentGroup(); + $assignments = $group->getAssignments(); + Assert::count(1, $assignments); + $assignment = $assignments->toArray()[0]; + Assert::count(4, $assignment->getAssignmentSolutions()); + $solutions = $assignment->getAssignmentSolutions()->filter(function ($s) { + return !$s->isAccepted() && $s->isReviewed(); + }); + Assert::count(1, $solutions); + $solution = $solutions->first(); + $solution->setAccepted(true); + $this->presenter->assignmentSolutions->persist($solution); + + $payload = PresenterTestHelper::performPresenterRequest( + $this->assignmentSolutionsPresenter, + 'V1:AssignmentSolutions', + 'GET', + ['action' => 'solution', 'id' => $solution->getId()] + ); + Assert::equal($solution->getId(), $payload["id"] ?? null); + } + + public function testLockedUserCannotSeeSolutionDetailsInOtherGroupsWhenAccepted() + { + $this->presenter = PresenterTestHelper::createPresenter($this->container, AssignmentsPresenter::class); + $student = $this->presenter->users->getByEmail($this->studentLogin); + $group = $this->prepExamGroup($student, -3600, 3600, GroupExamLockType::Accepted); + + $student->setIpLock($this->ip, $group->getExamEnd()); + $student->setGroupLock($group, $group->getExamEnd(), $group->getExamLockType()); + $this->presenter->users->persist($student); + + $group = $group->getParentGroup(); + $assignments = $group->getAssignments(); + Assert::count(1, $assignments); + $assignment = $assignments->toArray()[0]; + Assert::count(4, $assignment->getAssignmentSolutions()); + $solutions = $assignment->getAssignmentSolutions()->filter(function ($s) { + return !$s->isAccepted() && $s->isReviewed(); + }); + Assert::count(1, $solutions); + $solution = $solutions->first(); + + Assert::exception( + function () use ($solution) { + PresenterTestHelper::performPresenterRequest( + $this->assignmentSolutionsPresenter, + 'V1:AssignmentSolutions', + 'GET', + ['action' => 'solution', 'id' => $solution->getId()] + ); + }, + App\Exceptions\ForbiddenRequestException::class + ); + } + + public function testLockedUserCanSeeAcceptedSolutionDetailsInOtherGroupsWhenReviewed() + { + $this->presenter = PresenterTestHelper::createPresenter($this->container, AssignmentsPresenter::class); + $student = $this->presenter->users->getByEmail($this->studentLogin); + $group = $this->prepExamGroup($student, -3600, 3600, GroupExamLockType::Reviewed); + + $student->setIpLock($this->ip, $group->getExamEnd()); + $student->setGroupLock($group, $group->getExamEnd(), $group->getExamLockType()); + $this->presenter->users->persist($student); + + $group = $group->getParentGroup(); + $assignments = $group->getAssignments(); + Assert::count(1, $assignments); + $assignment = $assignments->toArray()[0]; + Assert::count(4, $assignment->getAssignmentSolutions()); + $solutions = $assignment->getAssignmentSolutions()->filter(function ($s) { + return !$s->isAccepted() && $s->isReviewed(); + }); + Assert::count(1, $solutions); + $solution = $solutions->first(); + $solution->setAccepted(true); + $this->presenter->assignmentSolutions->persist($solution); + + $payload = PresenterTestHelper::performPresenterRequest( + $this->assignmentSolutionsPresenter, + 'V1:AssignmentSolutions', + 'GET', + ['action' => 'solution', 'id' => $solution->getId()] + ); + Assert::equal($solution->getId(), $payload["id"] ?? null); + } + + public function testLockedUserCanSeeReviewedSolutionDetailsInOtherGroupsWhenReviewed() + { + $this->presenter = PresenterTestHelper::createPresenter($this->container, AssignmentsPresenter::class); + $student = $this->presenter->users->getByEmail($this->studentLogin); + $group = $this->prepExamGroup($student, -3600, 3600, GroupExamLockType::Reviewed); + + $student->setIpLock($this->ip, $group->getExamEnd()); + $student->setGroupLock($group, $group->getExamEnd(), $group->getExamLockType()); + $this->presenter->users->persist($student); + + $group = $group->getParentGroup(); + $assignments = $group->getAssignments(); + Assert::count(1, $assignments); + $assignment = $assignments->toArray()[0]; + Assert::count(4, $assignment->getAssignmentSolutions()); + $solutions = $assignment->getAssignmentSolutions()->filter(function ($s) { + return !$s->isAccepted() && $s->isReviewed(); + }); + Assert::count(1, $solutions); + $solution = $solutions->first(); + $solution->setReviewedAt(new DateTime()); + $this->presenter->assignmentSolutions->persist($solution); + + $payload = PresenterTestHelper::performPresenterRequest( + $this->assignmentSolutionsPresenter, + 'V1:AssignmentSolutions', + 'GET', + ['action' => 'solution', 'id' => $solution->getId()] + ); + Assert::equal($solution->getId(), $payload["id"] ?? null); + } + + public function testLockedUserCannotSeeSolutionDetailsInOtherGroupsWhenReviewed() + { + $this->presenter = PresenterTestHelper::createPresenter($this->container, AssignmentsPresenter::class); + $student = $this->presenter->users->getByEmail($this->studentLogin); + $group = $this->prepExamGroup($student, -3600, 3600, GroupExamLockType::Reviewed); + + $student->setIpLock($this->ip, $group->getExamEnd()); + $student->setGroupLock($group, $group->getExamEnd(), $group->getExamLockType()); + $this->presenter->users->persist($student); + + $group = $group->getParentGroup(); + $assignments = $group->getAssignments(); + Assert::count(1, $assignments); + $assignment = $assignments->toArray()[0]; + Assert::count(4, $assignment->getAssignmentSolutions()); + $solutions = $assignment->getAssignmentSolutions()->filter(function ($s) { + return !$s->isAccepted() && !$s->isReviewed() && $s->getReviewStartedAt() !== null; + }); + Assert::count(1, $solutions); + $solution = $solutions->first(); + Assert::false($solution->isAccepted()); + Assert::false($solution->isReviewed()); + + Assert::exception( + function () use ($solution) { + PresenterTestHelper::performPresenterRequest( + $this->assignmentSolutionsPresenter, + 'V1:AssignmentSolutions', + 'GET', + ['action' => 'solution', 'id' => $solution->getId()] + ); + }, + App\Exceptions\ForbiddenRequestException::class + ); + } + + public function testIpLockPreventsOtherIps() { $student = $this->presenter->users->getByEmail($this->studentLogin); $group = $this->prepExamGroup($student, -3600, 3600);