diff --git a/lib/Service/ApprovalService.php b/lib/Service/ApprovalService.php index 9be7374b..f92a7faa 100644 --- a/lib/Service/ApprovalService.php +++ b/lib/Service/ApprovalService.php @@ -532,9 +532,27 @@ private function shareWithApprovers(int $fileId, array $rule, string $userId): a } if ($this->shareManager->allowGroupSharing()) { foreach ($rule['approvers'] as $approver) { - if ($approver['type'] === 'group') { - if ($this->utilsService->createShare($node, IShare::TYPE_GROUP, $approver['entityId'], $fileOwner, $label)) { - $createdShares[] = $approver; + if ($approver['type'] === 'group' && !$this->utilsService->groupHasAccessTo($fileOwner, $node, $approver['entityId'])) { + $sharesNeeded = ['groupShare' => true, 'users' => []]; + // Only when the file is shared do you need to find a list of users the document needs to be shared with + if ($this->utilsService->isShared($node)) { + $sharesNeeded = $this->utilsService->usersNeedShare($node, $approver['entityId']); + } + if ($sharesNeeded['groupShare'] === true) { + if ($this->utilsService->createShare($node, IShare::TYPE_GROUP, $approver['entityId'], $fileOwner, $label)) { + $createdShares[] = $approver; + } + } elseif ($sharesNeeded['groupShare'] === false) { + // Goes through every user that doesn't have access and creates a share (very expensive) + $success = true; + foreach ($sharesNeeded['users'] as $user) { + if (!$this->utilsService->createShare($node, IShare::TYPE_USER, $user, $fileOwner, $label)) { + $success = false; + } + } + if ($success === true) { + $createdShares[] = $approver; + } } } } diff --git a/lib/Service/UtilsService.php b/lib/Service/UtilsService.php index cc6092bb..e90b13c2 100644 --- a/lib/Service/UtilsService.php +++ b/lib/Service/UtilsService.php @@ -8,12 +8,16 @@ namespace OCA\Approval\Service; use Exception; +use OC; use OCA\Approval\AppInfo\Application; +use OCA\Circles\CirclesManager; +use OCA\Circles\Exceptions\CircleNotFoundException; use OCP\Constants; use OCP\Files\IRootFolder; use OCP\Files\Node; use OCP\IConfig; +use OCP\IGroupManager; use OCP\IUser; use OCP\IUserManager; use OCP\Security\ICrypto; @@ -31,6 +35,7 @@ class UtilsService { public function __construct( string $appName, private IUserManager $userManager, + private IGroupManager $groupManager, private IShareManager $shareManager, private IRootFolder $root, private ISystemTagManager $tagManager, @@ -121,11 +126,11 @@ public function createShare(Node $node, int $type, string $sharedWith, string $s * @return bool */ public function isUserInCircle(string $userId, string $circleId): bool { - $circlesManager = \OC::$server->get(\OCA\Circles\CirclesManager::class); + $circlesManager = OC::$server->get(CirclesManager::class); $circlesManager->startSuperSession(); try { $circle = $circlesManager->getCircle($circleId); - } catch (\OCA\Circles\Exceptions\CircleNotFoundException $e) { + } catch (CircleNotFoundException $e) { $circlesManager->stopSession(); return false; } @@ -166,6 +171,47 @@ 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; + } + + /** + * Return true if this folder and no parents are shared with anyone + * + * @param Node $node + * @return bool + */ + public function isShared(Node $node): bool { + $userId = $node->getOwner()->getUID(); + do { + if (!empty($this->shareManager->getSharesBy($userId, ISHARE::TYPE_GROUP, $node))) { + return true; + } + if (!empty($this->shareManager->getSharesBy($userId, ISHARE::TYPE_USER, $node))) { + return true; + } + $node = $node->getParent(); + } while ($node->getParentId() !== -1); + return false; + } + /** * @param string $name of the new tag * @return array @@ -191,4 +237,59 @@ public function deleteTag(int $id): array { return ['error' => 'Tag not found']; } } + + /** + * Find users that need a file to be shared with, so all members of the group have it + * Also says if group share is the correct choice. + * + * @param Node $node of the file to check + * @param string|null $groupId the id of the group + * @return array + */ + public function usersNeedShare(Node $node, ?string $groupId): array { + $group = $this->groupManager->get($groupId); + $groupUsers = $group->getUsers(); + if (count($groupUsers) > 10) { // Ensure that this potentially slow operation is not done for big groups + return ['groupShare' => true, 'users' => []]; + } + $usersSet = []; + $groupsSet = []; // Stores the user if a share does not exist directly otherwise it is false + foreach ($groupUsers as $groupUser) { + $usersSet[$groupUser->getUID()] = $groupUser; + } + $ownerid = $node->getOwner()->getUID(); + do { + foreach ($this->shareManager->getSharesBy($ownerid, ISHARE::TYPE_GROUP, $node) as $share) { + $groupsSet[$share->getSharedWith()] = true; + } + foreach ($this->shareManager->getSharesBy($ownerid, ISHARE::TYPE_USER, $node) as $share) { + if (isset($usersSet[$share->getSharedWith()])) { + $usersSet[$share->getSharedWith()] = false; + } + } + $node = $node->getParent(); + } while ($node->getParentId() !== -1); + + $groupShare = true; // true if no user has a share, false otherwise + $users = []; + foreach ($usersSet as $uid => $hasShare) { + if ($hasShare !== false) { // User has no share + // Checks if the user is in a group that is being shared with + $groupList = $this->groupManager->getUserGroupIds($hasShare); + $groupFound = false; + foreach ($groupList as $groupId) { + if (isset($groupsSet[$groupId])) { + $groupFound = true; + break; + } + } + if (!$groupFound) { // User is not in a group that is being shared with + $users[] = $uid; + continue; + } + } + $groupShare = false; + } + return ['groupShare' => $groupShare, 'users' => $users]; + } } diff --git a/tests/unit/Service/ApprovalServiceTest.php b/tests/unit/Service/ApprovalServiceTest.php index 93ae911d..ad4de9e7 100644 --- a/tests/unit/Service/ApprovalServiceTest.php +++ b/tests/unit/Service/ApprovalServiceTest.php @@ -105,6 +105,7 @@ protected function setUp(): void { $this->utilsService = new UtilsService( 'approval', $c->get(IUserManager::class), + $c->get(IGroupManager::class), $c->get(IShareManager::class), $c->get(IRootFolder::class), $c->get(ISystemTagManager::class),