From fd0db5a22d3dba7d60a98388bea5bc2bd74b25f2 Mon Sep 17 00:00:00 2001
From: Julien Veyssier <eneiluj@posteo.net>
Date: Thu, 25 Mar 2021 14:32:32 +0100
Subject: [PATCH] check if user has access to file for any request, factorize
 some functions

Signed-off-by: Julien Veyssier <eneiluj@posteo.net>
---
 lib/Activity/ActivityManager.php |  4 ++--
 lib/Notification/Notifier.php    |  6 +++---
 lib/Service/ApprovalService.php  | 36 ++++++++++++++++++--------------
 3 files changed, 25 insertions(+), 21 deletions(-)

diff --git a/lib/Activity/ActivityManager.php b/lib/Activity/ActivityManager.php
index 56acc5b..9105aeb 100644
--- a/lib/Activity/ActivityManager.php
+++ b/lib/Activity/ActivityManager.php
@@ -66,10 +66,10 @@ class ActivityManager {
 		$subject = '';
 		switch ($subjectIdentifier) {
 			case self::SUBJECT_APPROVED:
-				$subject = $ownActivity ? $this->l10n->t('You have approved {file}'): $this->l10n->t('{user} has approved {file}');
+				$subject = $ownActivity ? $this->l10n->t('You approved {file}'): $this->l10n->t('{user} approved {file}');
 				break;
 			case self::SUBJECT_REJECTED:
-				$subject = $ownActivity ? $this->l10n->t('You have rejected {file}'): $this->l10n->t('{user} has rejected {file}');
+				$subject = $ownActivity ? $this->l10n->t('You rejected {file}'): $this->l10n->t('{user} rejected {file}');
 				break;
 			default:
 				break;
diff --git a/lib/Notification/Notifier.php b/lib/Notification/Notifier.php
index 01ceebb..16893af 100644
--- a/lib/Notification/Notifier.php
+++ b/lib/Notification/Notifier.php
@@ -116,8 +116,8 @@ class Notifier implements INotifier {
 						? $l->t('A directory was approved')
 						: $l->t('A directory was rejected'));
 				$content = $notification->getSubject() === 'approved'
-					? $l->t('%1$s has approved %2$s.', [$p['approverId'], $p['fileName']])
-					: $l->t('%1$s has rejected %2$s.', [$p['approverId'], $p['fileName']]);
+					? $l->t('%1$s approved %2$s.', [$p['approverId'], $p['fileName']])
+					: $l->t('%1$s rejected %2$s.', [$p['approverId'], $p['fileName']]);
 				$iconUrl = $notification->getSubject() === 'approved'
 					? $this->url->getAbsoluteURL($this->url->imagePath(Application::APP_ID, 'approved.svg'))
 					: $this->url->getAbsoluteURL($this->url->imagePath(Application::APP_ID, 'rejected.svg'));
@@ -127,7 +127,7 @@ class Notifier implements INotifier {
 					->setParsedMessage($content)
 					->setLink($linkToFile)
 					->setRichMessage(
-						$notification->getSubject() === 'approved' ? $l->t('{user} has approved {node}') : $l->t('{user} has rejected {node}'),
+						$notification->getSubject() === 'approved' ? $l->t('{user} approved {node}') : $l->t('{user} rejected {node}'),
 						[
 							'user' => $richSubjectUser,
 							'node' => $richSubjectNode,
diff --git a/lib/Service/ApprovalService.php b/lib/Service/ApprovalService.php
index 4933610..65d3243 100644
--- a/lib/Service/ApprovalService.php
+++ b/lib/Service/ApprovalService.php
@@ -72,30 +72,34 @@ class ApprovalService {
 		}
 	}
 
+	private function userHasAccessTo(int $fileId, ?string $userId): bool {
+		$user = $this->userManager->get($userId);
+		if ($user instanceof IUser) {
+			$userFolder = $this->root->getUserFolder($userId);
+			$found = $userFolder->getById($fileId);
+			return count($found) > 0;
+		}
+		return false;
+	}
+
 	/**
 	 * @param int $fileId
 	 * @return bool
 	 */
 	public function getApprovalState(int $fileId, ?string $userId): int {
-		// to return PENDING, 2 conditions:
-		// - user matches
-		// - tag matches
+		if (!$this->userHasAccessTo($fileId, $userId)) {
+			return Application::STATE_NOTHING;
+		}
+
 		$rules = $this->ruleService->getRules();
-		foreach ($rules as $id => $rule) {
-			try {
-				if ($this->tagObjectMapper->haveTag($fileId, 'files', $rule['tagPending'])
-					&& in_array($userId, $rule['users'])) {
-					return Application::STATE_APPROVABLE;
-				}
-			} catch (TagNotFoundException $e) {
-			}
-		}
-
-		// now check approved and rejected, we don't care about the user here
 		foreach ($rules as $id => $rule) {
 			try {
 				if ($this->tagObjectMapper->haveTag($fileId, 'files', $rule['tagPending'])) {
-					return Application::STATE_PENDING;
+					if (in_array($userId, $rule['users'])) {
+						return Application::STATE_APPROVABLE;
+					} else {
+						return Application::STATE_PENDING;
+					}
 				} elseif ($this->tagObjectMapper->haveTag($fileId, 'files', $rule['tagApproved'])) {
 					return Application::STATE_APPROVED;
 				} elseif ($this->tagObjectMapper->haveTag($fileId, 'files', $rule['tagRejected'])) {
@@ -170,7 +174,7 @@ class ApprovalService {
 		return false;
 	}
 
-	private function sendNotification(int $fileId, ?string $approverId, bool $approved) {
+	private function sendNotification(int $fileId, ?string $approverId, bool $approved): void {
 		$paramsByUser = [];
 		$root = $this->root;
 		// notification for eveyone having access except the one approving/rejecting