diff --git a/lib/ACL/ACLManager.php b/lib/ACL/ACLManager.php index 4e6891568..ad1041f4d 100644 --- a/lib/ACL/ACLManager.php +++ b/lib/ACL/ACLManager.php @@ -211,16 +211,19 @@ public function getPermissionsForTree(string $path): int { $path = ltrim($path, '/'); $rules = $this->ruleManager->getRulesForPrefix($this->user, $this->getRootStorageId(), $path); - return array_reduce($rules, function (int $permissions, array $rules): int { - $mergedRule = Rule::mergeRules($rules); - - $invertedMask = ~$mergedRule->getMask(); - // create a bitmask that has all inherit and allow bits set to 1 and all deny bits to 0 - $denyMask = $invertedMask | $mergedRule->getPermissions(); - - // since we only care about the lower permissions, we ignore the allow values - return $permissions & $denyMask; - }, Constants::PERMISSION_ALL); + if ($this->inheritMergePerUser) { + $pathsWithRules = array_keys($rules); + $permissions = Constants::PERMISSION_ALL; + foreach ($pathsWithRules as $path) { + $permissions &= $this->getACLPermissionsForPath($path); + } + return $permissions; + } else { + return array_reduce($rules, function (int $permissions, array $rules): int { + $mergedRule = Rule::mergeRules($rules); + return $mergedRule->applyDenyPermissions($permissions); + }, Constants::PERMISSION_ALL); + } } public function preloadRulesForFolder(string $path): void { diff --git a/lib/ACL/Rule.php b/lib/ACL/Rule.php index 123fa0013..d06f2212a 100644 --- a/lib/ACL/Rule.php +++ b/lib/ACL/Rule.php @@ -82,6 +82,25 @@ public function applyPermissions(int $permissions): int { return $permissions | $allowMask; } + /** + * Apply the deny permissions this rule to an existing permission set, returning the resulting permissions + * + * Only the deny permissions included in the current mask will overwrite the existing permissions + * + * @param int $permissions + * @return int + */ + public function applyDenyPermissions(int $permissions): int { + $invertedMask = ~$this->mask; + // create a bitmask that has all inherit and allow bits set to 1 and all deny bits to 0 + $denyMask = $invertedMask | $this->permissions; + + return $permissions & $denyMask; + } + + /** + * @return void + */ public function xmlSerialize(Writer $writer): void { $data = [ self::ACL => [ diff --git a/lib/ACL/RuleManager.php b/lib/ACL/RuleManager.php index 19b08179a..81b5905c9 100644 --- a/lib/ACL/RuleManager.php +++ b/lib/ACL/RuleManager.php @@ -193,6 +193,8 @@ private function rulesByPath(array $rows, array $result = []): array { } } + ksort($result); + return $result; } diff --git a/tests/ACL/ACLManagerTest.php b/tests/ACL/ACLManagerTest.php index 9b6d5deec..136d4865b 100644 --- a/tests/ACL/ACLManagerTest.php +++ b/tests/ACL/ACLManagerTest.php @@ -49,6 +49,13 @@ protected function setUp(): void { return array_merge($rules, $actualRules); }); + + $this->ruleManager->method('getRulesForPrefix') + ->willReturnCallback(function (IUser $user, int $storageId, string $prefix) { + return array_filter($this->rules, function (string $path) use ($prefix) { + return $prefix === $path || str_starts_with($path, $prefix . '/'); + }, ARRAY_FILTER_USE_KEY); + }); } private function createMapping(string $id): IUserMapping&MockObject { @@ -158,4 +165,67 @@ public function testGetACLPermissionsForPathPerUserMerge(): void { $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')); } + + public function testGetPermissionsForTree(): void { + $perUserAclManager = $this->getAclManager(true); + + $this->rules = [ + 'foo' => [ + new Rule($this->createMapping('1'), 10, Constants::PERMISSION_ALL, Constants::PERMISSION_ALL), + ], + 'foo/bar' => [ + new Rule($this->createMapping('2'), 10, Constants::PERMISSION_DELETE, 0) // remove delete + ], + 'foo/bar/asd' => [ + new Rule($this->createMapping('2'), 10, Constants::PERMISSION_DELETE, Constants::PERMISSION_DELETE) // re-add delete + ], + ]; + $this->assertEquals(Constants::PERMISSION_ALL - Constants::PERMISSION_DELETE, $this->aclManager->getPermissionsForTree('foo')); + $this->assertEquals(Constants::PERMISSION_ALL - Constants::PERMISSION_DELETE, $this->aclManager->getPermissionsForTree('foo/bar')); + + $this->assertEquals(Constants::PERMISSION_ALL, $perUserAclManager->getACLPermissionsForPath('foo')); + $this->assertEquals(Constants::PERMISSION_ALL, $perUserAclManager->getACLPermissionsForPath('foo/bar')); + $this->assertEquals(Constants::PERMISSION_ALL, $perUserAclManager->getACLPermissionsForPath('foo/bar/asd')); + + $this->assertEquals(Constants::PERMISSION_ALL, $perUserAclManager->getPermissionsForTree('foo')); + $this->assertEquals(Constants::PERMISSION_ALL, $perUserAclManager->getPermissionsForTree('foo/bar')); + + $this->rules = [ + 'foo2' => [ + new Rule($this->createMapping('1'), 10, Constants::PERMISSION_ALL, Constants::PERMISSION_ALL), + ], + 'foo2/bar' => [ + new Rule($this->createMapping('1'), 10, Constants::PERMISSION_DELETE, 0) // remove delete + ], + 'foo2/bar/asd' => [ + new Rule($this->createMapping('2'), 10, Constants::PERMISSION_DELETE, Constants::PERMISSION_DELETE) // re-add delete + ], + ]; + + $this->assertEquals(Constants::PERMISSION_ALL, $perUserAclManager->getACLPermissionsForPath('foo2')); + $this->assertEquals(Constants::PERMISSION_ALL - Constants::PERMISSION_DELETE, $perUserAclManager->getACLPermissionsForPath('foo2/bar')); + $this->assertEquals(Constants::PERMISSION_ALL, $perUserAclManager->getACLPermissionsForPath('foo2/bar/asd')); + + $this->assertEquals(Constants::PERMISSION_ALL - Constants::PERMISSION_DELETE, $perUserAclManager->getPermissionsForTree('foo2')); + $this->assertEquals(Constants::PERMISSION_ALL - Constants::PERMISSION_DELETE, $perUserAclManager->getPermissionsForTree('foo2/bar')); + + $this->rules = [ + 'foo3' => [ + new Rule($this->createMapping('1'), 10, Constants::PERMISSION_ALL, Constants::PERMISSION_ALL), + ], + 'foo3/bar' => [ + new Rule($this->createMapping('1'), 10, Constants::PERMISSION_DELETE, 0) // remove delete + ], + 'foo3/bar/asd' => [ + new Rule($this->createMapping('1'), 10, Constants::PERMISSION_DELETE, Constants::PERMISSION_DELETE) // re-add delete + ], + ]; + + $this->assertEquals(Constants::PERMISSION_ALL, $perUserAclManager->getACLPermissionsForPath('foo3')); + $this->assertEquals(Constants::PERMISSION_ALL - Constants::PERMISSION_DELETE, $perUserAclManager->getACLPermissionsForPath('foo3/bar')); + $this->assertEquals(Constants::PERMISSION_ALL, $perUserAclManager->getACLPermissionsForPath('foo3/bar/asd')); + + $this->assertEquals(Constants::PERMISSION_ALL - Constants::PERMISSION_DELETE, $perUserAclManager->getPermissionsForTree('foo3')); + $this->assertEquals(Constants::PERMISSION_ALL - Constants::PERMISSION_DELETE, $perUserAclManager->getPermissionsForTree('foo3/bar')); + } }