Only share to users and groups when necessary

README.md

@@ -34,6 +34,8 @@ Workflows can be chained. For example, if the approved tag of a workflow A is us
 then once a file is approved by the workflow A, it becomes pending for the B one. For an example of a chain involving a leave
 request that needs to be approved by a manager and then the department head, there is a screenshot below.
 
+**Notice**: If the approvers are a group the file may be shared with the individual users of the group instead of the group.
+
 ![Workflow chain](https://github.com/nextcloud/approval/raw/main/img/screenshot_chained.jpg)
 
 ## Tag assignment

lib/Service/ApprovalService.php

@@ -13,7 +13,8 @@
 use OCA\Approval\Exceptions\OutdatedEtagException;
 use OCA\DAV\Connector\Sabre\Node as SabreNode;
 use OCP\App\IAppManager;
-use OCP\Files\File;
+use OCP\Files\Config\ICachedMountFileInfo;
+use OCP\Files\Config\IUserMountCache;
 use OCP\Files\FileInfo;
 use OCP\Files\IRootFolder;
 use OCP\Files\Node;
@@ -50,6 +51,7 @@ public function __construct(
 		private IL10N $l10n,
 		private LoggerInterface $logger,
 		private ?string $userId,
+		private IUserMountCache $userMountCache,
 	) {
 	}
 
@@ -527,17 +529,16 @@ public function requestViaTagAssignment(int $fileId, int $ruleId, string $reques
 	 * @param int $fileId
 	 * @param Rule $rule
 	 * @param string $userId
-	 * @return array list of created shares
+	 * @return void
 	 * @throws \OCP\Files\NotPermittedException
 	 * @throws \OC\User\NoUserException
 	 */
-	private function shareWithApprovers(int $fileId, array $rule, string $userId): array {
-		$createdShares = [];
+	private function shareWithApprovers(int $fileId, array $rule, string $userId): void {
 		// get node
 		$userFolder = $this->root->getUserFolder($userId);
 		$node = $userFolder->getFirstNodeById($fileId);
 		if ($node === null) {
-			return [];
+			return;
 		}
 		// get the node again from the owner's storage to avoid sharing permission issues
 		$ownerId = $node->getOwner()->getUID();
@@ -549,19 +550,36 @@ private function shareWithApprovers(int $fileId, array $rule, string $userId): a
 		$label = $this->l10n->t('Please check my approval request');
 		$fileOwner = $node->getOwner()->getUID();
 
+		// Gets all users that have access to the file
+		$mounts = $this->userMountCache->getMountsForFileId($fileId);
+		$userIdsWithAccess = array_unique(array_map(static fn (ICachedMountFileInfo $fileInfo) => $fileInfo->getUser()->getUID(), $mounts));
+
 		foreach ($rule['approvers'] as $approver) {
-			if ($approver['type'] === 'user' && !$this->utilsService->userHasAccessTo($fileId, $approver['entityId'])) {
+			if ($approver['type'] === 'user' && !in_array($approver['entityId'], $userIdsWithAccess, true)) {
 				// create user share
-				if ($this->utilsService->createShare($node, IShare::TYPE_USER, $approver['entityId'], $fileOwner, $label)) {
-					$createdShares[] = $approver;
+				if (!$this->utilsService->createShare($node, IShare::TYPE_USER, $approver['entityId'], $fileOwner, $label)) {
+					$this->logger->warning('Failed to create user share for file {fileId} with approver {approverId}', ['fileId' => $fileId, 'approverId' => $approver['entityId']]);
 				}
 			}
 		}
 		if ($this->shareManager->allowGroupSharing()) {
 			foreach ($rule['approvers'] as $approver) {
-				if ($approver['type'] === 'group' && !$this->utilsService->groupHasAccessTo($fileOwner, $node, $approver['entityId'])) {
-					if ($this->utilsService->createShare($node, IShare::TYPE_GROUP, $approver['entityId'], $fileOwner, $label)) {
-						$createdShares[] = $approver;
+				if ($approver['type'] === 'group' && $this->groupManager->groupExists($approver['entityId'])) {
+					$groupMembers = $this->groupManager->get($approver['entityId'])->getUsers();
+					$groupMemberIds = array_map(static fn (IUser $user) => $user->getUID(), $groupMembers);
+					$groupMembersThatNeedAccess = array_diff($groupMemberIds, $userIdsWithAccess);
+					// Create group share if everyone in the group needs access
+					if (count($groupMembersThatNeedAccess) === count($groupMemberIds)) {
+						if (!$this->utilsService->createShare($node, IShare::TYPE_GROUP, $approver['entityId'], $fileOwner, $label)) {
+							$this->logger->warning('Failed to create group share for file {fileId} with approver {approverId}', ['fileId' => $fileId, 'approverId' => $approver['entityId']]);
+						}
+					} elseif (count($groupMembersThatNeedAccess) > 0) {
+						// Create user shares for each member that needs access
+						foreach ($groupMembersThatNeedAccess as $groupMemberId) {
+							if (!$this->utilsService->createShare($node, IShare::TYPE_USER, $groupMemberId, $fileOwner, $label)) {
+								$this->logger->warning('Failed to create user share for file {fileId} with approver {approverId}', ['fileId' => $fileId, 'approverId' => $groupMemberId]);
+							}
+						}
 					}
 				}
 			}
@@ -571,14 +589,12 @@ private function shareWithApprovers(int $fileId, array $rule, string $userId): a
 		if ($circlesEnabled) {
 			foreach ($rule['approvers'] as $approver) {
 				if ($approver['type'] === 'circle') {
-					if ($this->utilsService->createShare($node, IShare::TYPE_CIRCLE, $approver['entityId'], $fileOwner, $label)) {
-						$createdShares[] = $approver;
+					if (!$this->utilsService->createShare($node, IShare::TYPE_CIRCLE, $approver['entityId'], $fileOwner, $label)) {
+						$this->logger->warning('Failed to create circle share for file {fileId} with approver {approverId}', ['fileId' => $fileId, 'approverId' => $approver['entityId']]);
 					}
 				}
 			}
 		}
-
-		return $createdShares;
 	}
 
 	/**

lib/Service/UtilsService.php

@@ -14,7 +14,6 @@
 use OCP\Constants;
 use OCP\Files\IRootFolder;
 use OCP\Files\Node;
-
 use OCP\IUser;
 use OCP\IUserManager;
 use OCP\Share\IManager as IShareManager;
@@ -135,27 +134,6 @@ public function userHasAccessTo(int $fileId, ?string $userId): bool {
 		return false;
 	}
 
-	/**
-	 * Return false if this folder and no parents are shared with that group
-	 *
-	 * @param string $userId
-	 * @param Node $fileNode
-	 * @param string|null $groupId
-	 * @return bool
-	 */
-	public function groupHasAccessTo(string $userId, Node $fileNode, ?string $groupId): bool {
-		do {
-			$groupShares = $this->shareManager->getSharesBy($userId, ISHARE::TYPE_GROUP, $fileNode);
-			foreach ($groupShares as $groupShare) {
-				if ($groupShare->getSharedWith() === $groupId) {
-					return true;
-				}
-			}
-			$fileNode = $fileNode->getParent();
-		} while ($fileNode->getParentId() !== -1);
-		return false;
-	}
-
 	/**
 	 * @param string $name of the new tag
 	 * @return array

tests/unit/Service/ApprovalServiceTest.php

@@ -12,6 +12,7 @@
 use OCA\Approval\Activity\ActivityManager;
 use OCA\Approval\AppInfo\Application;
 use OCP\App\IAppManager;
+use OCP\Files\Config\IUserMountCache;
 use OCP\Files\IRootFolder;
 use OCP\ICacheFactory;
 use OCP\IDBConnection;
@@ -98,7 +99,8 @@ protected function setUp(): void {
 			$c->get(IShareManager::class),
 			$c->get(IL10N::class),
 			$c->get(LoggerInterface::class),
-			'user1'
+			'user1',
+			$c->get(IUserMountCache::class)
 		);
 
 		// add some tags