diff --git a/lib/ACL/ACLManager.php b/lib/ACL/ACLManager.php index 44dcd03f8..00055f14a 100644 --- a/lib/ACL/ACLManager.php +++ b/lib/ACL/ACLManager.php @@ -35,11 +35,12 @@ class ACLManager { private $rootFolderProvider; public function __construct( - private RuleManager $ruleManager, + private RuleManager $ruleManager, private TrashManager $trashManager, - private IUser $user, - callable $rootFolderProvider, - private ?int $rootStorageId = null, + private IUser $user, + callable $rootFolderProvider, + private ?int $rootStorageId = null, + private bool $inheritMergePerUser = false, ) { $this->ruleCache = new CappedMemoryCache(); $this->rootFolderProvider = $rootFolderProvider; @@ -168,12 +169,49 @@ public function getPermissionsForPathFromRules(string $path, array $rules): int * @return int */ private function calculatePermissionsForPath(array $rules): int { - // first combine all rules with the same path, then apply them on top of the current permissions - // since $rules is sorted parent first rules for subfolders overwrite the rules from the parent - return array_reduce($rules, function (int $permissions, array $rules): int { - $mergedRule = Rule::mergeRules($rules); - return $mergedRule->applyPermissions($permissions); - }, Constants::PERMISSION_ALL); + // given the following rules + // + // | Folder Rule | Read | Update | Share | Delete | + // |-------------|------|--------|-------|--------| + // | a: g1 | 1 | 1 | 1 | 1 | + // | a: g2 | - | - | - | - | + // | a/b: g1 | - | - | - | 0 | + // | a/b: g2 | 0 | - | - | - | + // |-------------|------|--------|-------|--------| + // + // and a user that is a member of g1 and g2 + // + // Without `inheritMergePerUser` the user will not have access to `a/b` + // as the merged rules for `a/b` ("-read,-delete") will overwrite the merged for `a` ("+read,+write+share+delete") + // + // With b`inheritMergePerUser` the user will have access to `a/b` + // as the applied rules for `g1` ("+read,+write+share") merges with the applied rules for `g2` ("-read") + if ($this->inheritMergePerUser) { + // first combine all rules for the same user-mapping by path order + // then merge the results with allow overwrites deny + $rulesPerMapping = []; + foreach ($rules as $rulesForPath) { + foreach ($rulesForPath as $rule) { + $mapping = $rule->getUserMapping(); + $key = $mapping->getType() . '/' . $mapping->getId(); + if (!isset($rulesPerMapping[$key])) { + $rulesPerMapping[$key] = Rule::defaultRule(); + } + + $rulesPerMapping[$key]->applyRule($rule); + } + } + + $mergedRule = Rule::mergeRules($rulesPerMapping); + return $mergedRule->applyPermissions(Constants::PERMISSION_ALL); + } else { + // first combine all rules with the same path, then apply them on top of the current permissions + // since $rules is sorted parent first rules for subfolders overwrite the rules from the parent + return array_reduce($rules, function (int $permissions, array $rules): int { + $mergedRule = Rule::mergeRules($rules); + return $mergedRule->applyPermissions($permissions); + }, Constants::PERMISSION_ALL); + } } /** diff --git a/lib/ACL/ACLManagerFactory.php b/lib/ACL/ACLManagerFactory.php index 805d78a3d..11ea7ddfb 100644 --- a/lib/ACL/ACLManagerFactory.php +++ b/lib/ACL/ACLManagerFactory.php @@ -24,6 +24,7 @@ namespace OCA\GroupFolders\ACL; use OCA\GroupFolders\Trash\TrashManager; +use OCP\IConfig; use OCP\IUser; class ACLManagerFactory { @@ -32,12 +33,20 @@ class ACLManagerFactory { public function __construct( private RuleManager $ruleManager, private TrashManager $trashManager, + private IConfig $config, callable $rootFolderProvider, ) { $this->rootFolderProvider = $rootFolderProvider; } public function getACLManager(IUser $user, ?int $rootStorageId = null): ACLManager { - return new ACLManager($this->ruleManager, $this->trashManager, $user, $this->rootFolderProvider, $rootStorageId); + return new ACLManager( + $this->ruleManager, + $this->trashManager, + $user, + $this->rootFolderProvider, + $rootStorageId, + $this->config->getAppValue('groupfolders', 'acl-inherit-per-user', 'false') === 'true', + ); } } diff --git a/lib/ACL/Rule.php b/lib/ACL/Rule.php index a0245d0b2..299f7cc58 100644 --- a/lib/ACL/Rule.php +++ b/lib/ACL/Rule.php @@ -155,4 +155,31 @@ public static function mergeRules(array $rules): Rule { $permissions ); } + + /** + * apply a new rule on top of the existing + * + * All non-inherit fields of the new rule will overwrite the current permissions + * + * @param array $rules + * @return void + */ + public function applyRule(Rule $rule): void { + $this->permissions = $rule->applyPermissions($this->permissions); + $this->mask |= $rule->getMask(); + } + + /** + * Create a default, no-op rule + * + * @return Rule + */ + public static function defaultRule(): Rule { + return new Rule( + new UserMapping('dummy', ''), + -1, + 0, + 0 + ); + } } diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index ef002ea6e..46f82cf42 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -224,6 +224,7 @@ public function register(IRegistrationContext $context): void { return new ACLManagerFactory( $c->get(RuleManager::class), $c->get(TrashManager::class), + $c->get(IConfig::class), $rootFolderProvider ); }); diff --git a/tests/ACL/ACLManagerTest.php b/tests/ACL/ACLManagerTest.php index eacb23a8e..1b0ea88dc 100644 --- a/tests/ACL/ACLManagerTest.php +++ b/tests/ACL/ACLManagerTest.php @@ -53,17 +53,9 @@ protected function setUp(): void { $this->user = $this->createMock(IUser::class); $this->ruleManager = $this->createMock(RuleManager::class); - $rootMountPoint = $this->createMock(IMountPoint::class); - $rootMountPoint->method('getNumericStorageId') - ->willReturn(1); - $rootFolder = $this->createMock(IRootFolder::class); - $rootFolder->method('getMountPoint') - ->willReturn($rootMountPoint); $this->trashManager = $this->createMock(TrashManager::class); - $this->aclManager = new ACLManager($this->ruleManager, $this->trashManager, $this->user, function () use ($rootFolder) { - return $rootFolder; - }); - $this->dummyMapping = $this->createMock(IUserMapping::class); + $this->aclManager = $this->getAclManager(); + $this->dummyMapping = $this->createMapping('dummy'); $this->ruleManager->method('getRulesForFilesByPath') ->willReturnCallback(function (IUser $user, int $storageId, array $paths) { @@ -77,6 +69,27 @@ protected function setUp(): void { }); } + private function createMapping(string $id): IUserMapping { + $mapping = $this->createMock(IUserMapping::class); + $mapping->method('getType')->willReturn('dummy'); + $mapping->method('getId')->willReturn($id); + $mapping->method('getDisplayName')->willReturn("display name for $id"); + return $mapping; + } + + private function getAclManager(bool $perUserMerge = false) { + $rootMountPoint = $this->createMock(IMountPoint::class); + $rootMountPoint->method('getNumericStorageId') + ->willReturn(1); + $rootFolder = $this->createMock(IRootFolder::class); + $rootFolder->method('getMountPoint') + ->willReturn($rootMountPoint); + + return new ACLManager($this->ruleManager, $this->trashManager, $this->user, function () use ($rootFolder) { + return $rootFolder; + }, null, $perUserMerge); + } + public function testGetACLPermissionsForPathNoRules() { $this->rules = []; $this->assertEquals(Constants::PERMISSION_ALL, $this->aclManager->getACLPermissionsForPath('foo')); @@ -85,19 +98,27 @@ public function testGetACLPermissionsForPathNoRules() { public function testGetACLPermissionsForPath() { $this->rules = [ 'foo' => [ - new Rule($this->dummyMapping, 10, Constants::PERMISSION_READ + Constants::PERMISSION_UPDATE, Constants::PERMISSION_READ), // read only - new Rule($this->dummyMapping, 10, Constants::PERMISSION_SHARE, 0) // deny share + new Rule($this->createMapping('1'), 10, Constants::PERMISSION_READ + Constants::PERMISSION_UPDATE, Constants::PERMISSION_READ), // read only + new Rule($this->createMapping('2'), 10, Constants::PERMISSION_SHARE, 0) // deny share ], 'foo/bar' => [ - new Rule($this->dummyMapping, 10, Constants::PERMISSION_UPDATE, Constants::PERMISSION_UPDATE) // add write + new Rule($this->createMapping('1'), 10, Constants::PERMISSION_UPDATE, Constants::PERMISSION_UPDATE) // add write ], 'foo/bar/sub' => [ - new Rule($this->dummyMapping, 10, Constants::PERMISSION_SHARE, Constants::PERMISSION_SHARE) // add share - ] + new Rule($this->createMapping('1'), 10, Constants::PERMISSION_SHARE, Constants::PERMISSION_SHARE) // add share + ], + 'foo/blocked' => [ + new Rule($this->createMapping('2'), 10, Constants::PERMISSION_READ, 0) // remove read + ], + 'foo/blocked2' => [ + new Rule($this->createMapping('1'), 10, Constants::PERMISSION_READ, 0) // remove read + ], ]; $this->assertEquals(Constants::PERMISSION_ALL - Constants::PERMISSION_SHARE - Constants::PERMISSION_UPDATE, $this->aclManager->getACLPermissionsForPath('foo')); $this->assertEquals(Constants::PERMISSION_ALL - Constants::PERMISSION_SHARE, $this->aclManager->getACLPermissionsForPath('foo/bar')); $this->assertEquals(Constants::PERMISSION_ALL, $this->aclManager->getACLPermissionsForPath('foo/bar/sub')); + $this->assertEquals(Constants::PERMISSION_ALL - Constants::PERMISSION_SHARE - Constants::PERMISSION_UPDATE - Constants::PERMISSION_READ, $this->aclManager->getACLPermissionsForPath('foo/blocked')); + $this->assertEquals(Constants::PERMISSION_ALL - Constants::PERMISSION_SHARE - Constants::PERMISSION_UPDATE - Constants::PERMISSION_READ, $this->aclManager->getACLPermissionsForPath('foo/blocked2')); } public function testGetACLPermissionsForPathInTrashbin() { @@ -127,4 +148,33 @@ public function testGetACLPermissionsForPathInTrashbin() { ]); $this->assertEquals(Constants::PERMISSION_ALL, $this->aclManager->getACLPermissionsForPath('__groupfolders/trash/1/subfolder2.d1700752274/coucou.md')); } + + + + public function testGetACLPermissionsForPathPerUserMerge() { + $aclManager = $this->getAclManager(true); + $this->rules = [ + 'foo' => [ + new Rule($this->createMapping('1'), 10, Constants::PERMISSION_READ + Constants::PERMISSION_UPDATE, Constants::PERMISSION_READ), // read only + new Rule($this->createMapping('2'), 10, Constants::PERMISSION_SHARE, 0) // deny share + ], + 'foo/bar' => [ + new Rule($this->createMapping('1'), 10, Constants::PERMISSION_UPDATE, Constants::PERMISSION_UPDATE) // add write + ], + 'foo/bar/sub' => [ + new Rule($this->createMapping('1'), 10, Constants::PERMISSION_SHARE, Constants::PERMISSION_SHARE) // add share + ], + 'foo/blocked' => [ + new Rule($this->createMapping('2'), 10, Constants::PERMISSION_READ, 0) // remove read + ], + 'foo/blocked2' => [ + new Rule($this->createMapping('1'), 10, Constants::PERMISSION_READ, 0) // remove read + ], + ]; + $this->assertEquals(Constants::PERMISSION_ALL - Constants::PERMISSION_SHARE - Constants::PERMISSION_UPDATE, $aclManager->getACLPermissionsForPath('foo')); + $this->assertEquals(Constants::PERMISSION_ALL - Constants::PERMISSION_SHARE, $aclManager->getACLPermissionsForPath('foo/bar')); + $this->assertEquals(Constants::PERMISSION_ALL, $aclManager->getACLPermissionsForPath('foo/bar/sub')); + $this->assertEquals(Constants::PERMISSION_ALL - Constants::PERMISSION_SHARE - Constants::PERMISSION_UPDATE, $aclManager->getACLPermissionsForPath('foo/blocked')); + $this->assertEquals(Constants::PERMISSION_ALL - Constants::PERMISSION_SHARE - Constants::PERMISSION_UPDATE - Constants::PERMISSION_READ, $aclManager->getACLPermissionsForPath('foo/blocked2')); + } }