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

fix(files_reminders): Only allow updating reminders if the file is accessible

2 files changed, 18 insertions, 6 deletions

diff --git a/apps/files_reminders/lib/Controller/ApiController.php b/apps/files_reminders/lib/Controller/ApiController.php
index dbc340610b2..ee29b7ce494 100644
--- a/apps/files_reminders/lib/Controller/ApiController.php
+++ b/apps/files_reminders/lib/Controller/ApiController.php
@@ -57,7 +57,7 @@ class ApiController extends OCSController {
 				'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 @@ class ApiController extends OCSController {
 		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
index 8bd6887e754..32dbab6d2dc 100644
--- a/apps/files_reminders/lib/Service/ReminderService.php
+++ b/apps/files_reminders/lib/Service/ReminderService.php
@@ -47,9 +47,11 @@ class ReminderService {
 	}
 
 	/**
+	 * @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,6 +76,7 @@ class ReminderService {
 	 */
 	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);
@@ -81,10 +84,6 @@ class ReminderService {
 			$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 @@ class ReminderService {
 	}
 
 	/**
+	 * @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 @@ class ReminderService {
 			$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();
+		}
+	}
 }