diff --git a/src/Database/Document.php b/src/Database/Document.php index 73bd458cd5..bd4325c9d1 100644 --- a/src/Database/Document.php +++ b/src/Database/Document.php @@ -149,12 +149,13 @@ public function getWrite(): array public function getPermissionsByType(string $type): array { $typePermissions = []; + $prefix = $type . '("'; foreach ($this->getPermissions() as $permission) { - if (!\str_starts_with($permission, $type)) { + if (!\str_starts_with($permission, $prefix) || !\str_ends_with($permission, '")')) { continue; } - $typePermissions[] = \str_replace([$type . '(', ')', '"', ' '], '', $permission); + $typePermissions[] = \substr($permission, \strlen($prefix), -2); } return \array_unique($typePermissions); diff --git a/src/Database/Helpers/Permission.php b/src/Database/Helpers/Permission.php index 18c4fe5a94..41b2506324 100644 --- a/src/Database/Helpers/Permission.php +++ b/src/Database/Helpers/Permission.php @@ -23,11 +23,13 @@ class Permission public function __construct( private string $permission, - string $role, + string|Role $role, string $identifier = '', string $dimension = '', ) { - $this->role = new Role($role, $identifier, $dimension); + $this->role = $role instanceof Role + ? $role + : new Role($role, $identifier, $dimension); } /** @@ -73,6 +75,14 @@ public function getDimension(): string return $this->role->getDimension(); } + /** + * @return array + */ + public function getRoles(): array + { + return $this->role->getRoles(); + } + /** * Parse a permission string into a Permission object * @@ -82,62 +92,19 @@ public function getDimension(): string */ public static function parse(string $permission): self { - $permissionParts = \explode('("', $permission); - - if (\count($permissionParts) !== 2) { + $separator = \strpos($permission, '("'); + if ($separator === false || !\str_ends_with($permission, '")')) { throw new DatabaseException('Invalid permission string format: "' . $permission . '".'); } - $permission = $permissionParts[0]; + $role = \substr($permission, $separator + 2, -2); + $permission = \substr($permission, 0, $separator); if (!\in_array($permission, array_merge(Database::PERMISSIONS, [Database::PERMISSION_WRITE]))) { throw new DatabaseException('Invalid permission type: "' . $permission . '".'); } - $fullRole = \str_replace('")', '', $permissionParts[1]); - $roleParts = \explode(':', $fullRole); - $role = $roleParts[0]; - - $hasIdentifier = \count($roleParts) > 1; - $hasDimension = \str_contains($fullRole, '/'); - - if (!$hasIdentifier && !$hasDimension) { - return new self($permission, $role); - } - - if ($hasIdentifier && !$hasDimension) { - $identifier = $roleParts[1]; - return new self($permission, $role, $identifier); - } - - if (!$hasIdentifier) { - $dimensionParts = \explode('/', $fullRole); - if (\count($dimensionParts) !== 2) { - throw new DatabaseException('Only one dimension can be provided'); - } - - $role = $dimensionParts[0]; - $dimension = $dimensionParts[1]; - - if (empty($dimension)) { - throw new DatabaseException('Dimension must not be empty'); - } - return new self($permission, $role, '', $dimension); - } - - // Has both identifier and dimension - $dimensionParts = \explode('/', $roleParts[1]); - if (\count($dimensionParts) !== 2) { - throw new DatabaseException('Only one dimension can be provided'); - } - - $identifier = $dimensionParts[0]; - $dimension = $dimensionParts[1]; - - if (empty($dimension)) { - throw new DatabaseException('Dimension must not be empty'); - } - return new self($permission, $role, $identifier, $dimension); + return new self($permission, Role::parse($role)); } /** @@ -167,9 +134,7 @@ public static function aggregate(?array $permissions, array $allowed = Database: } $mutated[] = (new self( $subType, - $permission->getRole(), - $permission->getIdentifier(), - $permission->getDimension() + $permission->role ))->toString(); } } @@ -187,9 +152,7 @@ public static function read(Role $role): string { $permission = new self( 'read', - $role->getRole(), - $role->getIdentifier(), - $role->getDimension() + $role ); return $permission->toString(); } @@ -204,9 +167,7 @@ public static function create(Role $role): string { $permission = new self( 'create', - $role->getRole(), - $role->getIdentifier(), - $role->getDimension() + $role ); return $permission->toString(); } @@ -221,9 +182,7 @@ public static function update(Role $role): string { $permission = new self( 'update', - $role->getRole(), - $role->getIdentifier(), - $role->getDimension() + $role ); return $permission->toString(); } @@ -238,9 +197,7 @@ public static function delete(Role $role): string { $permission = new self( 'delete', - $role->getRole(), - $role->getIdentifier(), - $role->getDimension() + $role ); return $permission->toString(); } @@ -255,9 +212,7 @@ public static function write(Role $role): string { $permission = new self( 'write', - $role->getRole(), - $role->getIdentifier(), - $role->getDimension() + $role ); return $permission->toString(); } diff --git a/src/Database/Helpers/Role.php b/src/Database/Helpers/Role.php index 1682cb5471..d60814b7e8 100644 --- a/src/Database/Helpers/Role.php +++ b/src/Database/Helpers/Role.php @@ -4,10 +4,14 @@ class Role { + /** + * @param array $roles + */ public function __construct( private string $role, private string $identifier = '', private string $dimension = '', + private array $roles = [], ) { } @@ -18,6 +22,13 @@ public function __construct( */ public function toString(): string { + if (!empty($this->roles)) { + return 'allOf(' . \implode(',', \array_map( + fn (Role $role) => $role->toString(), + $this->roles + )) . ')'; + } + $str = $this->role; if ($this->identifier) { $str .= ':' . $this->identifier; @@ -52,6 +63,14 @@ public function getDimension(): string return $this->dimension; } + /** + * @return array + */ + public function getRoles(): array + { + return empty($this->roles) ? [$this] : $this->roles; + } + /** * Parse a role string into a Role object * @@ -61,6 +80,19 @@ public function getDimension(): string */ public static function parse(string $role): self { + if (\str_starts_with($role, 'allOf(')) { + if (!\str_ends_with($role, ')')) { + throw new \Exception('Invalid allOf role format'); + } + + $roles = \explode(',', \substr($role, 6, -1)); + + return self::allOf(\array_map( + fn (string $role) => self::parse($role), + $roles + )); + } + $roleParts = \explode(':', $role); $hasIdentifier = \count($roleParts) > 1; $hasDimension = \str_contains($role, '/'); @@ -175,4 +207,34 @@ public static function member(string $identifier): self { return new self('member', $identifier); } + + /** + * Require both roles to grant access. + * + * @param array $roles + */ + public static function allOf(array $roles): self + { + if (\count($roles) !== 2) { + throw new \InvalidArgumentException('allOf requires exactly two roles'); + } + + foreach ($roles as $role) { + if (!$role instanceof self) { + throw new \InvalidArgumentException('allOf only accepts Role instances'); + } + + if (\count($role->getRoles()) !== 1) { + throw new \InvalidArgumentException('Nested allOf roles are not supported'); + } + } + + \usort($roles, fn (Role $a, Role $b) => $a->toString() <=> $b->toString()); + + if ($roles[0]->toString() === $roles[1]->toString()) { + throw new \InvalidArgumentException('allOf requires two distinct roles'); + } + + return new self('allOf', roles: $roles); + } } diff --git a/src/Database/Validator/Permissions.php b/src/Database/Validator/Permissions.php index 13e7372050..81e4261bfd 100644 --- a/src/Database/Validator/Permissions.php +++ b/src/Database/Validator/Permissions.php @@ -96,12 +96,14 @@ public function isValid($permissions): bool return false; } - $role = $permission->getRole(); - $identifier = $permission->getIdentifier(); - $dimension = $permission->getDimension(); - - if (!$this->isValidRole($role, $identifier, $dimension)) { - return false; + foreach ($permission->getRoles() as $role) { + if (!$this->isValidRole( + $role->getRole(), + $role->getIdentifier(), + $role->getDimension() + )) { + return false; + } } } return true; diff --git a/tests/e2e/Adapter/Scopes/PermissionTests.php b/tests/e2e/Adapter/Scopes/PermissionTests.php index 97e55633fc..dc3a826a7b 100644 --- a/tests/e2e/Adapter/Scopes/PermissionTests.php +++ b/tests/e2e/Adapter/Scopes/PermissionTests.php @@ -1422,6 +1422,50 @@ public function testDocumentPermissionRolesAreMatchedExactly(): void $database->deleteCollection($collection); } + public function testCompositePermissionRolesAreMatchedExactly(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + $authorization = $database->getAuthorization(); + $collection = 'perm_composite_' . uniqid(); + $requiredRole = Role::allOf([ + Role::member('membership-id'), + Role::team('team-id', 'admin'), + ]); + + $database->createCollection($collection, permissions: [ + Permission::create(Role::any()), + ], documentSecurity: true); + $database->createAttribute($collection, 'name', Database::VAR_STRING, 255, true); + + $authorization->skip(function () use ($database, $collection, $requiredRole): void { + $database->createDocument($collection, new Document([ + '$id' => 'protected', + '$permissions' => [Permission::read($requiredRole)], + 'name' => 'Protected', + ])); + }); + + $authorization->cleanRoles(); + $authorization->addRole(Role::member('membership-id')->toString()); + $authorization->addRole(Role::team('team-id', 'admin')->toString()); + $this->assertSame([], $this->documentIds($database->find($collection))); + $this->assertTrue($database->getDocument($collection, 'protected')->isEmpty()); + + $authorization->addRole($requiredRole->toString()); + $this->assertSame(['protected'], $this->documentIds($database->find($collection))); + $this->assertSame('protected', $database->getDocument($collection, 'protected')->getId()); + + $authorization->cleanRoles(); + $authorization->addRole(Role::allOf([ + Role::member('membership-id'), + Role::team('team-id', 'viewer'), + ])->toString()); + $this->assertSame([], $this->documentIds($database->find($collection))); + + $database->deleteCollection($collection); + } + /** * @param array $documents * @return list diff --git a/tests/unit/DocumentTest.php b/tests/unit/DocumentTest.php index 9a41ab5340..67e1665ae7 100644 --- a/tests/unit/DocumentTest.php +++ b/tests/unit/DocumentTest.php @@ -137,6 +137,23 @@ public function testGetPermissionByType(): void $this->assertEquals([], $this->empty->getPermissionsByType(Database::PERMISSION_DELETE)); } + public function testGetCompositePermissionByType(): void + { + $document = new Document([ + '$permissions' => [ + Permission::read(Role::allOf([ + Role::member('membership-id'), + Role::team('team-id', 'admin'), + ])), + ], + ]); + + $this->assertEquals( + ['allOf(member:membership-id,team:team-id/admin)'], + $document->getRead() + ); + } + public function testGetPermissions(): void { $this->assertEquals([ diff --git a/tests/unit/PermissionTest.php b/tests/unit/PermissionTest.php index 6ca554f378..47fbe149a2 100644 --- a/tests/unit/PermissionTest.php +++ b/tests/unit/PermissionTest.php @@ -289,11 +289,35 @@ public function testInvalidFormats(): void } } + public function testCompositeRole(): void + { + $role = Role::allOf([ + Role::member('membership-id'), + Role::team('team-id', 'admin'), + ]); + $permission = Permission::read($role); + + $this->assertEquals('read("allOf(member:membership-id,team:team-id/admin)")', $permission); + $this->assertEquals($permission, Permission::parse($permission)->toString()); + $this->assertCount(2, Permission::parse($permission)->getRoles()); + } + /** * @throws \Exception */ public function testAggregation(): void { + $permissions = [Permission::write(Role::allOf([ + Role::member('membership-id'), + Role::team('team-id', 'admin'), + ]))]; + $parsed = Permission::aggregate($permissions); + $this->assertEquals([ + 'create("allOf(member:membership-id,team:team-id/admin)")', + 'update("allOf(member:membership-id,team:team-id/admin)")', + 'delete("allOf(member:membership-id,team:team-id/admin)")', + ], $parsed); + $permissions = ['write("any")']; $parsed = Permission::aggregate($permissions); $this->assertEquals(['create("any")', 'update("any")', 'delete("any")'], $parsed); diff --git a/tests/unit/RoleTest.php b/tests/unit/RoleTest.php index 2c1cbee27d..4eeb59d2a1 100644 --- a/tests/unit/RoleTest.php +++ b/tests/unit/RoleTest.php @@ -137,4 +137,19 @@ public function testInputFromID(): void $role = Role::team(ID::custom('123'), '456'); $this->assertEquals('team:123/456', $role->toString()); } + + public function testAllOf(): void + { + $role = Role::allOf([ + Role::team('team-id', 'admin'), + Role::member('membership-id'), + ]); + + $this->assertEquals('allOf(member:membership-id,team:team-id/admin)', $role->toString()); + $this->assertEquals($role->toString(), Role::parse($role->toString())->toString()); + $this->assertCount(2, $role->getRoles()); + + $this->expectException(\InvalidArgumentException::class); + Role::allOf([Role::any()]); + } } diff --git a/tests/unit/Validator/PermissionsTest.php b/tests/unit/Validator/PermissionsTest.php index d574644633..770bb6cac2 100644 --- a/tests/unit/Validator/PermissionsTest.php +++ b/tests/unit/Validator/PermissionsTest.php @@ -305,6 +305,21 @@ public function testInvalidPermissions(): void $this->assertEquals('You can only provide up to 100 permissions.', $object->getDescription()); } + public function testCompositePermissions(): void + { + $validator = new Permissions(); + + $this->assertTrue($validator->isValid([ + Permission::read(Role::allOf([ + Role::member('membership-id'), + Role::team('team-id', 'admin'), + ])), + ])); + $this->assertFalse($validator->isValid([ + 'read("allOf(member:membership-id,team:invalid&team/admin)")', + ])); + } + /* * Test for checking duplicate methods input. The getPermissions should return an a list array */