mark credential notification as processed even if the corresponding c…

…redential does not exist #488
nextcloud · Apr 5, 2024 · b89c03e · b89c03e
1 parent a31dee3
commit b89c03e
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 12 deletions.
diff --git a/lib/Controller/InternalController.php b/lib/Controller/InternalController.php
@@ -12,12 +12,13 @@
 namespace OCA\Passman\Controller;
 
 use OCA\Passman\Service\CredentialService;
+use OCA\Passman\Service\NotificationService;
 use OCP\App\IAppManager;
 use OCP\AppFramework\ApiController;
+use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\IConfig;
 use OCP\IRequest;
-use OCP\Notification\IManager;
 
 class InternalController extends ApiController {
 	private $userId;
@@ -27,8 +28,8 @@ public function __construct(
 		IRequest $request,
 		$UserId,
 		private CredentialService $credentialService,
+		private NotificationService $notificationService,
 		private IConfig $config,
-		private IManager $manager,
 		private IAppManager $appManager,
 	) {
 		parent::__construct(
@@ -49,28 +50,31 @@ public function remind($credential_id) {
 			$credential->setExpireTime(time() + (24 * 60 * 60));
 			$this->credentialService->upd($credential);
 
-			$notification = $this->manager->createNotification();
-			$notification->setApp('passman')
-				->setObject('credential', $credential_id)
-				->setUser($this->userId);
-			$this->manager->markProcessed($notification);
+			$this->notificationService->markNotificationOfCredentialAsProcessed($credential_id, $this->userId);
 		}
 	}
 
 	/**
 	 * @NoAdminRequired
 	 */
 	public function read($credential_id) {
+		try {
+			// need to check overall credential existence before, since getCredentialById() method call below throws a
+			// DoesNotExistException in two different cases, that we cannot differentiate in retrospect
+			$this->credentialService->credentialExistsById($credential_id);
+		} catch (DoesNotExistException) {
+			// got DoesNotExistException from CredentialMapper, means the credential does not even exist for any user,
+			// so we can also delete or mark the corresponding notification message as processed
+			$this->notificationService->markNotificationOfCredentialAsProcessed($credential_id, $this->userId);
+			return;
+		}
+
 		$credential = $this->credentialService->getCredentialById($credential_id, $this->userId);
 		if ($credential) {
 			$credential->setExpireTime(0);
 			$this->credentialService->upd($credential);
 
-			$notification = $this->manager->createNotification();
-			$notification->setApp('passman')
-				->setObject('credential', $credential_id)
-				->setUser($this->userId);
-			$this->manager->markProcessed($notification);
+			$this->notificationService->markNotificationOfCredentialAsProcessed($credential_id, $this->userId);
 		}
 	}
 

diff --git a/lib/Service/CredentialService.php b/lib/Service/CredentialService.php
@@ -193,6 +193,18 @@ public function getCredentialById(int $credential_id, ?string $user_id) {
 		}
 	}
 
+	/**
+	 * Check if a credential exists by id.
+	 *
+	 * @param int $credential_id
+	 * @return bool
+	 * @throws DoesNotExistException
+	 * @throws MultipleObjectsReturnedException
+	 */
+	public function credentialExistsById(int $credential_id): bool {
+		return $this->credentialMapper->getCredentialById($credential_id) !== null;
+	}
+
 	/**
 	 * Get credential label by credential id.
 	 *

diff --git a/lib/Service/NotificationService.php b/lib/Service/NotificationService.php
@@ -120,4 +120,12 @@ function deleteNotificationsOfCredential($credential) {
 			->andWhere($qb->expr()->eq('object_type', 'credential'));
 		return $qb->execute();
 	}
+
+	function markNotificationOfCredentialAsProcessed(int $credential_id, string $user_id): void {
+		$notification = $this->manager->createNotification();
+		$notification->setApp('passman')
+			->setObject('credential', $credential_id)
+			->setUser($user_id);
+		$this->manager->markProcessed($notification);
+	}
 }