Merge pull request #50711 from nextcloud/fix/reminder-node-access

fix(files_reminders): Only allow updating reminders if the file is accessible
nextcloud · Feb 20, 2025 · 047378e · 047378e
2 parents 74c2579 + fd591b0
commit 047378e
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 6 deletions.
diff --git a/apps/files_reminders/lib/Controller/ApiController.php b/apps/files_reminders/lib/Controller/ApiController.php
@@ -57,7 +57,7 @@ public function get(int $fileId): DataResponse {
 				'dueDate' => $reminder->getDueDate()->format(DateTimeInterface::ATOM), // ISO 8601
 			];
 			return new DataResponse($reminderData, Http::STATUS_OK);
-		} catch (DoesNotExistException $e) {
+		} catch (NodeNotFoundException|DoesNotExistException $e) {
 			$reminderData = [
 				'dueDate' => null,
 			];
@@ -125,7 +125,7 @@ public function remove(int $fileId): DataResponse {
 		try {
 			$this->reminderService->remove($user, $fileId);
 			return new DataResponse([], Http::STATUS_OK);
-		} catch (DoesNotExistException $e) {
+		} catch (NodeNotFoundException|DoesNotExistException $e) {
 			return new DataResponse([], Http::STATUS_NOT_FOUND);
 		}
 	}

diff --git a/apps/files_reminders/lib/Service/ReminderService.php b/apps/files_reminders/lib/Service/ReminderService.php
@@ -47,9 +47,11 @@ public function get(int $id): RichReminder {
 	}
 
 	/**
+	 * @throws NodeNotFoundException
 	 * @throws DoesNotExistException
 	 */
 	public function getDueForUser(IUser $user, int $fileId): RichReminder {
+		$this->checkNode($user, $fileId);
 		$reminder = $this->reminderMapper->findDueForUser($user, $fileId);
 		return new RichReminder($reminder, $this->root);
 	}
@@ -74,17 +76,14 @@ public function getAll(?IUser $user = null) {
 	 */
 	public function createOrUpdate(IUser $user, int $fileId, DateTime $dueDate): bool {
 		$now = new DateTime('now', new DateTimeZone('UTC'));
+		$this->checkNode($user, $fileId);
 		try {
 			$reminder = $this->reminderMapper->findDueForUser($user, $fileId);
 			$reminder->setDueDate($dueDate);
 			$reminder->setUpdatedAt($now);
 			$this->reminderMapper->update($reminder);
 			return false;
 		} catch (DoesNotExistException $e) {
-			$node = $this->root->getUserFolder($user->getUID())->getFirstNodeById($fileId);
-			if (!$node) {
-				throw new NodeNotFoundException();
-			}
 			// Create new reminder if no reminder is found
 			$reminder = new Reminder();
 			$reminder->setUserId($user->getUID());
@@ -98,9 +97,11 @@ public function createOrUpdate(IUser $user, int $fileId, DateTime $dueDate): boo
 	}
 
 	/**
+	 * @throws NodeNotFoundException
 	 * @throws DoesNotExistException
 	 */
 	public function remove(IUser $user, int $fileId): void {
+		$this->checkNode($user, $fileId);
 		$reminder = $this->reminderMapper->findDueForUser($user, $fileId);
 		$this->reminderMapper->delete($reminder);
 	}
@@ -161,4 +162,15 @@ public function cleanUp(?int $limit = null): void {
 			$this->reminderMapper->delete($reminder);
 		}
 	}
+
+	/**
+	 * @throws NodeNotFoundException
+	 */
+	private function checkNode(IUser $user, int $fileId): void {
+		$userFolder = $this->root->getUserFolder($user->getUID());
+		$node = $userFolder->getFirstNodeById($fileId);
+		if ($node === null) {
+			throw new NodeNotFoundException();
+		}
+	}
 }