Skip to content

Commit 78eb01f

Browse files
committed
fix: make canManageACL work with circles
Signed-off-by: Robin Appelman <[email protected]>
1 parent a4718f3 commit 78eb01f

File tree

6 files changed

+64
-21
lines changed

6 files changed

+64
-21
lines changed

lib/ACL/UserMapping/IUserMapping.php

+2
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,6 @@ public function getType(): string;
3030
public function getId(): string;
3131

3232
public function getDisplayName(): string;
33+
34+
public function getKey(): string;
3335
}

lib/ACL/UserMapping/IUserMappingManager.php

+9
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,13 @@ public function getMappingsForUser(IUser $user, bool $userAssignable = true): ar
3939
* @return IUserMapping|null
4040
*/
4141
public function mappingFromId(string $type, string $id): ?IUserMapping;
42+
43+
/**
44+
* Check if a user is a member of one of the provided user mappings
45+
*
46+
* @param IUser $user
47+
* @param IUserMapping[] $mappings
48+
* @return bool
49+
*/
50+
public function userInMappings(IUser $user, array $mappings): bool;
4251
}

lib/ACL/UserMapping/UserMapping.php

+4
Original file line numberDiff line numberDiff line change
@@ -49,4 +49,8 @@ public function getId(): string {
4949
public function getDisplayName(): string {
5050
return $this->displayName;
5151
}
52+
53+
public function getKey(): string {
54+
return $this->getType() . ':' . $this->getId();
55+
}
5256
}

lib/ACL/UserMapping/UserMappingManager.php

+18
Original file line numberDiff line numberDiff line change
@@ -131,4 +131,22 @@ public function getCirclesManager(): ?CirclesManager {
131131
return null;
132132
}
133133
}
134+
135+
public function userInMappings(IUser $user, array $mappings): bool {
136+
foreach ($mappings as $mapping) {
137+
if ($mapping->getType() === 'user' && $mapping->getId() === $user->getUID()) {
138+
return true;
139+
}
140+
}
141+
142+
$mappingKeys = array_map(fn (IUserMapping $mapping) => $mapping->getKey(), $mappings);
143+
144+
$userMappings = $this->getMappingsForUser($user);
145+
foreach ($userMappings as $userMapping) {
146+
if (in_array($userMapping->getKey(), $mappingKeys, true)) {
147+
return true;
148+
}
149+
}
150+
return false;
151+
}
134152
}

lib/Folder/FolderManager.php

+21-18
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@
2929
use OCA\Circles\Model\Circle;
3030
use OCA\Circles\Model\Member;
3131
use OCA\Circles\Model\Probes\CircleProbe;
32+
use OCA\GroupFolders\ACL\UserMapping\IUserMapping;
33+
use OCA\GroupFolders\ACL\UserMapping\IUserMappingManager;
34+
use OCA\GroupFolders\ACL\UserMapping\UserMapping;
3235
use OCA\GroupFolders\Mount\GroupMountPoint;
3336
use OCA\GroupFolders\ResponseDefinitions;
3437
use OCP\AutoloadNotAllowedException;
@@ -58,6 +61,8 @@ public function __construct(
5861
private IMimeTypeLoader $mimeTypeLoader,
5962
private LoggerInterface $logger,
6063
private IEventDispatcher $eventDispatcher,
64+
private IConfig $config,
65+
private IUserMappingManager $userMappingManager,
6166
) {
6267
}
6368

@@ -464,28 +469,26 @@ public function canManageACL(int $folderId, IUser $user): bool {
464469
}
465470
}
466471

467-
$query = $this->connection->getQueryBuilder();
468-
$query->select('*')
469-
->from('group_folders_manage')
470-
->where($query->expr()->eq('folder_id', $query->createNamedParameter($folderId, IQueryBuilder::PARAM_INT)))
471-
->andWhere($query->expr()->eq('mapping_type', $query->createNamedParameter('user')))
472-
->andWhere($query->expr()->eq('mapping_id', $query->createNamedParameter($userId)));
473-
if ($query->executeQuery()->rowCount() === 1) {
474-
return true;
475-
}
472+
$managerMappings = $this->getManagerMappings($folderId);
473+
return $this->userMappingManager->userInMappings($user, $managerMappings);
474+
}
476475

476+
/**
477+
* @param int $folderId
478+
* @return IUserMapping[]
479+
*/
480+
private function getManagerMappings(int $folderId): array {
477481
$query = $this->connection->getQueryBuilder();
478-
$query->select('*')
482+
$query->select('mapping_type', 'mapping_id')
479483
->from('group_folders_manage')
480-
->where($query->expr()->eq('folder_id', $query->createNamedParameter($folderId)))
481-
->andWhere($query->expr()->eq('mapping_type', $query->createNamedParameter('group')));
482-
$groups = $query->executeQuery()->fetchAll();
483-
foreach ($groups as $manageRule) {
484-
if ($this->groupManager->isInGroup($userId, $manageRule['mapping_id'])) {
485-
return true;
486-
}
484+
->where($query->expr()->eq('folder_id', $query->createNamedParameter($folderId, IQueryBuilder::PARAM_INT)));
485+
$managerMappings = [];
486+
487+
$rows = $query->executeQuery()->fetchAll();
488+
foreach ($rows as $manageRule) {
489+
$managerMappings[] = new UserMapping($manageRule['mapping_type'], $manageRule['mapping_id']);
487490
}
488-
return false;
491+
return $managerMappings;
489492
}
490493

491494
/**

tests/Folder/FolderManagerTest.php

+10-3
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
namespace OCA\GroupFolders\Tests\Folder;
2323

24+
use OCA\GroupFolders\ACL\UserMapping\IUserMappingManager;
2425
use OCA\GroupFolders\Folder\FolderManager;
2526
use OCP\Constants;
2627
use OCP\EventDispatcher\IEventDispatcher;
@@ -40,6 +41,8 @@ class FolderManagerTest extends TestCase {
4041
private IMimeTypeLoader $mimeLoader;
4142
private LoggerInterface $logger;
4243
private IEventDispatcher $eventDispatcher;
44+
private IConfig $config;
45+
private IUserMappingManager $userMappingManager;
4346

4447
protected function setUp(): void {
4548
parent::setUp();
@@ -48,12 +51,16 @@ protected function setUp(): void {
4851
$this->mimeLoader = $this->createMock(IMimeTypeLoader::class);
4952
$this->logger = $this->createMock(LoggerInterface::class);
5053
$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
54+
$this->config = $this->createMock(IConfig::class);
55+
$this->userMappingManager = $this->createMock(IUserMappingManager::class);
5156
$this->manager = new FolderManager(
5257
\OC::$server->getDatabaseConnection(),
5358
$this->groupManager,
5459
$this->mimeLoader,
5560
$this->logger,
5661
$this->eventDispatcher,
62+
$this->config,
63+
$this->userMappingManager,
5764
);
5865
$this->clean();
5966
}
@@ -323,7 +330,7 @@ public function testGetFoldersForUserSimple() {
323330
$db = $this->createMock(IDBConnection::class);
324331
/** @var FolderManager|\PHPUnit_Framework_MockObject_MockObject $manager */
325332
$manager = $this->getMockBuilder(FolderManager::class)
326-
->setConstructorArgs([$db, $this->groupManager, $this->mimeLoader, $this->logger, $this->eventDispatcher])
333+
->setConstructorArgs([$db, $this->groupManager, $this->mimeLoader, $this->logger, $this->eventDispatcher, $this->config, $this->userMappingManager])
327334
->setMethods(['getFoldersForGroups'])
328335
->getMock();
329336

@@ -346,7 +353,7 @@ public function testGetFoldersForUserMerge() {
346353
$db = $this->createMock(IDBConnection::class);
347354
/** @var FolderManager|\PHPUnit_Framework_MockObject_MockObject $manager */
348355
$manager = $this->getMockBuilder(FolderManager::class)
349-
->setConstructorArgs([$db, $this->groupManager, $this->mimeLoader, $this->logger, $this->eventDispatcher])
356+
->setConstructorArgs([$db, $this->groupManager, $this->mimeLoader, $this->logger, $this->eventDispatcher, $this->config, $this->userMappingManager])
350357
->setMethods(['getFoldersForGroups'])
351358
->getMock();
352359

@@ -382,7 +389,7 @@ public function testGetFolderPermissionsForUserMerge() {
382389
$db = $this->createMock(IDBConnection::class);
383390
/** @var FolderManager|\PHPUnit_Framework_MockObject_MockObject $manager */
384391
$manager = $this->getMockBuilder(FolderManager::class)
385-
->setConstructorArgs([$db, $this->groupManager, $this->mimeLoader, $this->logger, $this->eventDispatcher])
392+
->setConstructorArgs([$db, $this->groupManager, $this->mimeLoader, $this->logger, $this->eventDispatcher, $this->config, $this->userMappingManager])
386393
->setMethods(['getFoldersForGroups'])
387394
->getMock();
388395

0 commit comments

Comments
 (0)