From c7c46b278a0d83012e2a04b882450c0b685d491b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Sat, 19 Apr 2025 13:12:49 +0200 Subject: [PATCH 1/2] Restricting rules when an attachment file can actually be deleted (correctly handling exercise cloning and shared links). --- .../presenters/ExerciseFilesPresenter.php | 39 ++++++++++++++++++- app/model/entity/AttachmentFile.php | 15 +++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/app/V1Module/presenters/ExerciseFilesPresenter.php b/app/V1Module/presenters/ExerciseFilesPresenter.php index 85b7611fc..76845b930 100644 --- a/app/V1Module/presenters/ExerciseFilesPresenter.php +++ b/app/V1Module/presenters/ExerciseFilesPresenter.php @@ -17,6 +17,7 @@ use App\Model\Entity\SupplementaryExerciseFile; use App\Model\Entity\UploadedFile; use App\Model\Entity\AttachmentFile; +use App\Model\Repository\Assignments; use App\Model\Repository\AttachmentFiles; use App\Model\Repository\Exercises; use App\Model\Entity\Exercise; @@ -37,6 +38,12 @@ class ExerciseFilesPresenter extends BasePresenter */ public $exercises; + /** + * @var Assignments + * @inject + */ + public $assignments; + /** * @var UploadedFiles * @inject @@ -345,7 +352,37 @@ public function actionDeleteAttachmentFile(string $id, string $fileId) $exercise->removeAttachmentFile($file); $this->exercises->flush(); - $this->fileStorage->deleteAttachmentFile($file); + $this->attachmentFiles->refresh($file); + if ($file->getExercises()->isEmpty()) { + // file has no attachments to exercises, let's check the assignments + $isUsed = false; + foreach ($file->getAssignments() as $assignment) { + $group = $assignment->getGroup(); + if ($group && !$group->isArchived()) { + $isUsed = true; // only non-archived assignments are considered relevant + break; + } + } + + if (!$isUsed) { + $this->fileStorage->deleteAttachmentFile($file); + + if ($file->getAssignments()->isEmpty()) { + // only if no attachments exists (except for deleted ones) + // remove all links to deleted entities and remove the file record + foreach ($file->getExercisesAndIReallyMeanAllOkay() as $exercise) { + $exercise->removeAttachmentFile($file); + $this->exercises->persist($exercise, false); + } + foreach ($file->getAssignmentsAndIReallyMeanAllOkay() as $assignment) { + $assignment->removeAttachmentFile($file); + $this->assignments->persist($assignment, false); + } + + $this->attachmentFiles->remove($file); + } + } + } $this->sendSuccessResponse("OK"); } diff --git a/app/model/entity/AttachmentFile.php b/app/model/entity/AttachmentFile.php index a4d6252e8..05be5348a 100644 --- a/app/model/entity/AttachmentFile.php +++ b/app/model/entity/AttachmentFile.php @@ -32,6 +32,14 @@ function (Exercise $exercise) { ); } + /** + * @return Collection + */ + public function getExercisesAndIReallyMeanAllOkay() + { + return $this->exercises; + } + /** * @ORM\ManyToMany(targetEntity="Assignment", mappedBy="attachmentFiles") */ @@ -49,6 +57,13 @@ function (Assignment $assignment) { ); } + /** + * @return Collection + */ + public function getAssignmentsAndIReallyMeanAllOkay() + { + return $this->assignments; + } /** * AttachmentFile constructor. From ffbf4740810a5948a9a9460ef5c77e113f97057e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Sat, 19 Apr 2025 15:43:04 +0200 Subject: [PATCH 2/2] Fixing ACL checks for delete-attachment endpoint to verify the selected file belongs to the selected exercise. --- app/V1Module/presenters/ExerciseFilesPresenter.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/V1Module/presenters/ExerciseFilesPresenter.php b/app/V1Module/presenters/ExerciseFilesPresenter.php index 76845b930..f1f67c37a 100644 --- a/app/V1Module/presenters/ExerciseFilesPresenter.php +++ b/app/V1Module/presenters/ExerciseFilesPresenter.php @@ -7,6 +7,7 @@ use App\Helpers\MetaFormats\Validators\VMixed; use App\Helpers\MetaFormats\Validators\VString; use App\Helpers\MetaFormats\Validators\VUuid; +use App\Exceptions\BadRequestException; use App\Exceptions\ForbiddenRequestException; use App\Exceptions\InvalidApiArgumentException; use App\Exceptions\NotFoundException; @@ -330,6 +331,10 @@ public function actionGetAttachmentFiles(string $id) public function checkDeleteAttachmentFile(string $id, string $fileId) { $exercise = $this->exercises->findOrThrow($id); + $file = $this->attachmentFiles->findOrThrow($fileId); + if (!$file->getExercises()->contains($exercise)) { + throw new BadRequestException("Selected file is not an attachment file for given exercise."); + } if (!$this->exerciseAcl->canUpdate($exercise)) { throw new ForbiddenRequestException("You cannot delete attachment files for this exercise."); }