fix: implement 'merge rules per user' logic for folder delete permiss…

…ions check Signed-off-by: Robin Appelman <[email protected]> [skip ci]
nextcloud · Nov 18, 2024 · 493f00d · 493f00d
1 parent 68995da
commit 493f00d
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 10 deletions.
diff --git a/lib/ACL/ACLManager.php b/lib/ACL/ACLManager.php
@@ -230,16 +230,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/tests/ACL/ACLManagerTest.php b/tests/ACL/ACLManagerTest.php
@@ -65,6 +65,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 {
@@ -175,4 +182,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'));
+	}
 }