Skip to content

Commit b7f91e8

Browse files
committed
filters by shares instead of checking for access
Signed-off-by: Lukas Schaefer <lukas@lschaefer.xyz>
1 parent b52930c commit b7f91e8

File tree

2 files changed

+42
-21
lines changed

2 files changed

+42
-21
lines changed

lib/Service/ApprovalService.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -534,7 +534,7 @@ private function shareWithApprovers(int $fileId, array $rule, string $userId): a
534534
$sharesNeeded = ['groupShare' => true, 'users' => []];
535535
// Only when the file is shared do you need to find a list of users the document needs to be shared with
536536
if ($this->utilsService->isShared($node)) {
537-
$sharesNeeded = $this->utilsService->usersNeedShare($fileId, $approver['entityId']);
537+
$sharesNeeded = $this->utilsService->usersNeedShare($node, $approver['entityId']);
538538
}
539539
if ($sharesNeeded['groupShare'] === true) {
540540
if ($this->utilsService->createShare($node, IShare::TYPE_GROUP, $approver['entityId'], $fileOwner, $label)) {

lib/Service/UtilsService.php

Lines changed: 41 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -181,22 +181,15 @@ public function userHasAccessTo(int $fileId, ?string $userId): bool {
181181
* @return bool
182182
*/
183183
public function groupHasAccessTo(string $userId, Node $fileNode, ?string $groupId): bool {
184-
$groupShares = $this->shareManager->getSharesBy($userId, ISHARE::TYPE_GROUP, $fileNode);
185-
foreach ($groupShares as $groupShare) {
186-
if ($groupShare->getSharedWith() === $groupId) {
187-
return true;
188-
}
189-
}
190-
$folderNode = $fileNode->getParent();
191-
while ($folderNode->getParentId() !== -1) {
192-
$groupShares = $this->shareManager->getSharesBy($userId, ISHARE::TYPE_GROUP, $folderNode);
184+
do {
185+
$groupShares = $this->shareManager->getSharesBy($userId, ISHARE::TYPE_GROUP, $fileNode);
193186
foreach ($groupShares as $groupShare) {
194187
if ($groupShare->getSharedWith() === $groupId) {
195188
return true;
196189
}
197190
}
198-
$folderNode = $folderNode->getParent();
199-
}
191+
$fileNode = $fileNode->getParent();
192+
} while ($fileNode->getParentId() !== -1);
200193
return false;
201194
}
202195

@@ -250,22 +243,50 @@ public function deleteTag(int $id): array {
250243
* Find users that need a file to be shared with, so all members of the group have it
251244
* Also says if group share is the correct choice.
252245
*
253-
* @param int $fileId of the file to check what
246+
* @param Node $node of the file to check
254247
* @param string|null $groupId the id of the group
255248
* @return array
256249
*/
257-
public function usersNeedShare(int $fileId, ?string $groupId): array {
250+
public function usersNeedShare(Node $node, ?string $groupId): array {
251+
$group = $this->groupManager->get($groupId);
252+
$groupUsers = $group->getUsers();
253+
$usersSet = [];
254+
$groupsSet = []; // Stores the user if a share does not exist directly otherwise it is false
255+
foreach ($groupUsers as $groupUser) {
256+
$usersSet[$groupUser->getUID()] = $groupUser;
257+
}
258+
$ownerid = $node->getOwner()->getUID();
259+
do {
260+
foreach ($this->shareManager->getSharesBy($ownerid, ISHARE::TYPE_GROUP, $node) as $share) {
261+
$groupsSet[$share->getSharedWith()] = true;
262+
}
263+
foreach ($this->shareManager->getSharesBy($ownerid, ISHARE::TYPE_USER, $node) as $share) {
264+
if (isset($usersSet[$share->getSharedWith()])) {
265+
$usersSet[$share->getSharedWith()] = false;
266+
}
267+
}
268+
$node = $node->getParent();
269+
} while ($node->getParentId() !== -1);
270+
258271
$groupShare = true;
259272
$users = [];
260-
$group = $this->groupManager->get($groupId);
261-
if ($group instanceof IGroup) {
262-
foreach ($group->getUsers() as $groupUser) {
263-
if ($this->userHasAccessTo($fileId, $groupUser->getUID())) {
264-
$groupShare = false;
265-
} else {
266-
$users[] = $groupUser->getUID();
273+
foreach ($usersSet as $uid => $hasShare) {
274+
if ($hasShare !== false) { // User has no share
275+
// Checks if the user is in a group that is being shared with
276+
$groupList = $this->groupManager->getUserGroupIds($hasShare);
277+
$groupFound = false;
278+
foreach ($groupList as $groupId) {
279+
if (isset($groups[$groupId])) {
280+
$groupFound = true;
281+
break;
282+
}
283+
}
284+
if (!$groupFound) { // User is not in a group that is being shared with
285+
$users[] = $uid;
286+
continue;
267287
}
268288
}
289+
$groupShare = false;
269290
}
270291
return ['groupShare' => $groupShare, 'users' => $users];
271292
}

0 commit comments

Comments
 (0)