From 8bfa9dfa29a1635decbc4992e6236d21258e898e Mon Sep 17 00:00:00 2001 From: Jonathan Treffler Date: Tue, 12 Nov 2024 15:36:07 +0100 Subject: [PATCH] added user-has-manager-permissions dav prop; added principal model and simplified the ACL code with it --- lib/Dav/PropFindPlugin.php | 13 +++ lib/Db/ResourceMapper.php | 7 +- lib/Db/ResourceMember.php | 43 +++----- lib/Db/ResourceMemberMapper.php | 2 + .../{MemberType.php => PrincipalType.php} | 2 +- lib/Manager/ACLManager.php | 38 +++++++ .../Version000000Date20241025120000.php | 10 +- lib/Model/Principal.php | 29 +++++ lib/Security/ResourceVoter.php | 18 ++-- lib/Service/ResourceMemberService.php | 24 ++--- lib/Service/ResourceService.php | 100 +++++++----------- 11 files changed, 172 insertions(+), 114 deletions(-) rename lib/Enum/{MemberType.php => PrincipalType.php} (83%) create mode 100644 lib/Model/Principal.php diff --git a/lib/Dav/PropFindPlugin.php b/lib/Dav/PropFindPlugin.php index 390b01d..f17b24b 100644 --- a/lib/Dav/PropFindPlugin.php +++ b/lib/Dav/PropFindPlugin.php @@ -14,14 +14,17 @@ use OCA\GroupFolders\Folder\FolderManager; use OCA\GroupFolders\Mount\GroupMountPoint; use OCA\OrganizationFolders\Service\ResourceService; +use OCA\OrganizationFolders\Security\AuthorizationService; class PropFindPlugin extends ServerPlugin { public const ORGANIZATION_FOLDER_ID_PROPERTYNAME = '{http://verdigado.com/ns}organization-folder-id'; public const ORGANIZATION_FOLDER_RESOURCE_ID_PROPERTYNAME = '{http://verdigado.com/ns}organization-folder-resource-id'; + public const ORGANIZATION_FOLDER_RESOURCE_MANAGER_PERMISSIONS_PROPERTYNAME = '{http://verdigado.com/ns}organization-folder-resource-user-has-manager-permissions'; public function __construct( private FolderManager $folderManager, private ResourceService $resourceService, + private AuthorizationService $authorizationService, ) { } @@ -41,6 +44,7 @@ class PropFindPlugin extends ServerPlugin { return; } + $propFind->handle(self::ORGANIZATION_FOLDER_ID_PROPERTYNAME, function () use ($fileInfo): int { return $this->folderManager->getFolderByPath($fileInfo->getPath()); }); @@ -52,5 +56,14 @@ class PropFindPlugin extends ServerPlugin { return null; } }); + + $propFind->handle(self::ORGANIZATION_FOLDER_RESOURCE_MANAGER_PERMISSIONS_PROPERTYNAME, function () use ($node) { + try { + $resource = $this->resourceService->findByFileId($node->getId()); + return $this->authorizationService->isGranted(["READ"], $resource) ? 'true' : 'false'; + } catch (\Exception $e) { + return null; + } + }); } } \ No newline at end of file diff --git a/lib/Db/ResourceMapper.php b/lib/Db/ResourceMapper.php index d27e64b..d676f5d 100644 --- a/lib/Db/ResourceMapper.php +++ b/lib/Db/ResourceMapper.php @@ -67,8 +67,13 @@ class ResourceMapper extends QBMapper { /** * @param int $organizationFolderId - * @param int $parentResourceId + * @psalm-param int $organizationFolderId + * @param int|null $parentResourceId + * @psalm-param int|null $parentResourceId + * @param array $filters + * @psalm-param array $filters * @return array + * @psalm-return Resource[] */ public function findAll(int $organizationFolderId, ?int $parentResourceId = null, array $filters = []): array { /* @var $qb IQueryBuilder */ diff --git a/lib/Db/ResourceMember.php b/lib/Db/ResourceMember.php index d41a8cb..9f0c0d1 100644 --- a/lib/Db/ResourceMember.php +++ b/lib/Db/ResourceMember.php @@ -8,31 +8,39 @@ use OCA\OrganizationFolders\Interface\TableSerializable; use OCP\AppFramework\Db\Entity; use OCA\OrganizationFolders\Enum\MemberPermissionLevel; -use OCA\OrganizationFolders\Enum\MemberType; +use OCA\OrganizationFolders\Enum\PrincipalType; +use OCA\OrganizationFolders\Model\Principal; class ResourceMember extends Entity implements JsonSerializable, TableSerializable { protected $resourceId; protected $permissionLevel; - protected $type; - protected $principal; + protected $principalType; + protected $principalId; protected $createdTimestamp; protected $lastUpdatedTimestamp; public function __construct() { $this->addType('resourceId','integer'); $this->addType('permissionLevel','integer'); - $this->addType('type','integer'); + $this->addType('principalType','integer'); $this->addType('createdTimestamp','integer'); $this->addType('lastUpdatedTimestamp','integer'); } + public function getPrincipal(): Principal { + return new Principal(PrincipalType::from($this->principalType), $this->principalId); + } + + public function setPrincipal(Principal $principal) { + $this->setPrincipalType($principal->getType()->value); + $this->setPrincipalId($principal->getId()); + } public function jsonSerialize(): array { return [ 'id' => $this->id, 'resourceId' => $this->resourceId, 'permissionLevel' => $this->permissionLevel, - 'type' => $this->type, - 'principal' => $this->principal, + 'principal' => $this->getPrincipal(), 'createdTimestamp' => $this->createdTimestamp, 'lastUpdatedTimestamp' => $this->lastUpdatedTimestamp, ]; @@ -43,29 +51,10 @@ class ResourceMember extends Entity implements JsonSerializable, TableSerializab 'Id' => $this->id, 'Resource Id' => $this->resourceId, 'Permission Level' => MemberPermissionLevel::from($this->permissionLevel)->name, - 'Type' => MemberType::from($this->type)->name, - 'Principal' => $this->principal, + 'Principal Type' => PrincipalType::from($this->principalType)->name, + 'Principal Id' => $this->principalId, 'Created' => $this->createdTimestamp, 'LastUpdated' => $this->lastUpdatedTimestamp, ]; } - - public function getParsedPrincipal() { - if($this->type === MemberType::USER->value) { - return [ - "userId" => $this->principal, - ]; - } else if($this->type === MemberType::GROUP->value) { - return [ - "groupId" => $this->principal, - ]; - } else if($this->type === MemberType::ROLE->value) { - [$organizationProviderId, $roleId] = explode(":", $this->principal, 2); - - return [ - "organizationProviderId" => $organizationProviderId, - "roleId" => $roleId, - ]; - } - } } diff --git a/lib/Db/ResourceMemberMapper.php b/lib/Db/ResourceMemberMapper.php index 74b8b66..7ada460 100644 --- a/lib/Db/ResourceMemberMapper.php +++ b/lib/Db/ResourceMemberMapper.php @@ -34,7 +34,9 @@ class ResourceMemberMapper extends QBMapper { /** * @param int $resourceId + * @psalm-param int $resourceId * @return array + * @psalm-return ResourceMember[] */ public function findAll(int $resourceId): array { /* @var $qb IQueryBuilder */ diff --git a/lib/Enum/MemberType.php b/lib/Enum/PrincipalType.php similarity index 83% rename from lib/Enum/MemberType.php rename to lib/Enum/PrincipalType.php index 529a570..695015d 100644 --- a/lib/Enum/MemberType.php +++ b/lib/Enum/PrincipalType.php @@ -2,7 +2,7 @@ namespace OCA\OrganizationFolders\Enum; -enum MemberType: int { +enum PrincipalType: int { use FromNameEnum; case USER = 1; diff --git a/lib/Manager/ACLManager.php b/lib/Manager/ACLManager.php index eedd17f..6036fb8 100644 --- a/lib/Manager/ACLManager.php +++ b/lib/Manager/ACLManager.php @@ -8,16 +8,22 @@ use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; use OCA\GroupFolders\ACL\Rule; +use OCA\GroupFolders\ACL\UserMapping\IUserMapping; use OCA\GroupFolders\ACL\UserMapping\IUserMappingManager; use OCA\GroupFolders\ACL\RuleManager; use OCA\GroupFolders\Folder\FolderManager; +use OCA\OrganizationFolders\OrganizationProvider\OrganizationProviderManager; +use OCA\OrganizationFolders\Model\Principal; +use OCA\OrganizationFolders\Enum\PrincipalType; + class ACLManager { public function __construct( protected IDBConnection $db, protected FolderManager $folderManager, protected IUserMappingManager $userMappingManager, protected RuleManager $ruleManager, + protected OrganizationProviderManager $organizationProviderManager ) { } @@ -48,6 +54,38 @@ class ACLManager { return array_map($this->createRuleEntityFromRow(...), $rows); } + public function getMappingForPrincipal(Principal $principal): IUserMapping { + if($principal->getType() === PrincipalType::USER) { + return $this->userMappingManager->mappingFromId("user", $principal->getId()); + } else if($principal->getType() === PrincipalType::GROUP) { + return $this->userMappingManager->mappingFromId("group", $principal->getId()); + } else if($principal->getType() === PrincipalType::ROLE) { + [$organizationProviderId, $roleId] = explode(":", $principal->getId(), 2); + + $organizationProvider = $this->organizationProviderManager->getOrganizationProvider($organizationProviderId); + $role = $organizationProvider->getRole($roleId); + + return $this->userMappingManager->mappingFromId("group", $role->getMembersGroup()); + } else { + throw new \Exception("invalid resource member type"); + } + } + + public function createAclRuleForPrincipal(Principal $principal, int $fileId, int $mask, int $permissions): ?Rule { + $mapping = $this->getMappingForPrincipal($principal); + + if(is_null($mapping)) { + return null; + } + + return new Rule( + userMapping: $mapping, + fileId: $fileId, + mask: $mask, + permissions: $permissions, + ); + } + protected function ruleMappingComparison(Rule $rule1, Rule $rule2): int { $mapping1 = $rule1->getUserMapping(); $mapping2 = $rule2->getUserMapping(); diff --git a/lib/Migration/Version000000Date20241025120000.php b/lib/Migration/Version000000Date20241025120000.php index 3cbad8c..4866f2d 100644 --- a/lib/Migration/Version000000Date20241025120000.php +++ b/lib/Migration/Version000000Date20241025120000.php @@ -42,13 +42,13 @@ class Version000000Date20241025120000 extends SimpleMigrationStep { // 0: user // 1: group // 2: role - $table->addColumn('type', Types::INTEGER, [ + $table->addColumn('principal_type', Types::INTEGER, [ 'notnull' => true, ]); - // for type user: "[user_id]" - // for type group: "[group_name]" - // for type role: "[organization_provider_id]:[role_id]" - $table->addColumn('principal', Types::STRING, [ + // for principal type user: "[user_id]" + // for principal type group: "[group_name]" + // for principal type role: "[organization_provider_id]:[role_id]" + $table->addColumn('principal_id', Types::STRING, [ 'length' => 128, 'notnull' => true, ]); diff --git a/lib/Model/Principal.php b/lib/Model/Principal.php new file mode 100644 index 0000000..bddf0c2 --- /dev/null +++ b/lib/Model/Principal.php @@ -0,0 +1,29 @@ +type; + } + + public function getId(): string { + return $this->id; + } + + public function jsonSerialize(): array { + return [ + 'type' => $this->type->value, + 'id' => $this->id, + ]; + } +} \ No newline at end of file diff --git a/lib/Security/ResourceVoter.php b/lib/Security/ResourceVoter.php index 8854ec9..9fb61ab 100644 --- a/lib/Security/ResourceVoter.php +++ b/lib/Security/ResourceVoter.php @@ -9,7 +9,7 @@ use OCA\OrganizationFolders\Db\Resource; use OCA\OrganizationFolders\Service\ResourceService; use OCA\OrganizationFolders\Service\ResourceMemberService; use OCA\OrganizationFolders\Enum\MemberPermissionLevel; -use OCA\OrganizationFolders\Enum\MemberType; +use OCA\OrganizationFolders\Enum\PrincipalType; use OCA\OrganizationFolders\OrganizationProvider\OrganizationProviderManager; class ResourceVoter extends Voter { @@ -38,6 +38,7 @@ class ResourceVoter extends Voter { 'READ' => $this->isGranted($user, $resource), 'UPDATE' => $this->isGranted($user, $resource), 'DELETE' => $this->isGranted($user, $resource), + 'UPDATE_MEMBERS' => $this->isGranted($user, $resource), default => throw new \LogicException('This code should not be reached!') }; } @@ -59,19 +60,22 @@ class ResourceVoter extends Voter { foreach($resourceMembers as $resourceMember) { if($resourceMember->getPermissionLevel() === MemberPermissionLevel::MANAGER->value) { - if($resourceMember->getType() === MemberType::USER->value) { - if($resourceMember->getPrincipal() === $user->getUID()) { + $principal = $resourceMember->getPrincipal(); + + if($principal->getType() === PrincipalType::USER) { + if($principal->getId() === $user->getUID()) { return true; } - } else if($resourceMember->getType() === MemberType::GROUP->value) { - if($this->groupManager->isInGroup($user->getUID(), $resourceMember->getPrincipal())) { + } else if($principal->getType() === PrincipalType::GROUP) { + if($this->groupManager->isInGroup($user->getUID(), $principal->getId())) { return true; } - } else if($resourceMember->getType() === MemberType::ROLE->value) { - ['organizationProviderId' => $organizationProviderId, 'roleId' => $roleId] = $resourceMember->getParsedPrincipal(); + } else if($principal->getType() === PrincipalType::ROLE) { + [$organizationProviderId, $roleId] = explode(":", $principal->getId(), 2); $organizationProvider = $this->organizationProviderManager->getOrganizationProvider($organizationProviderId); $role = $organizationProvider->getRole($roleId); + if($this->groupManager->isInGroup($user->getUID(), $role->getMembersGroup())) { return true; } diff --git a/lib/Service/ResourceMemberService.php b/lib/Service/ResourceMemberService.php index 0253281..ebab643 100644 --- a/lib/Service/ResourceMemberService.php +++ b/lib/Service/ResourceMemberService.php @@ -13,7 +13,7 @@ use OCA\OrganizationFolders\Db\ResourceMember; use OCA\OrganizationFolders\Db\ResourceMemberMapper; use OCA\OrganizationFolders\Enum\MemberPermissionLevel; -use OCA\OrganizationFolders\Enum\MemberType; +use OCA\OrganizationFolders\Model\Principal; class ResourceMemberService { public function __construct( @@ -23,6 +23,12 @@ class ResourceMemberService { ) { } + /** + * @param int $resourceId + * @psalm-param int $resourceId + * @return array + * @psalm-return ResourceMember[] + */ public function findAll(int $resourceId): array { return $this->mapper->findAll($resourceId); } @@ -47,8 +53,7 @@ class ResourceMemberService { public function create( int $resourceId, MemberPermissionLevel $permissionLevel, - MemberType $type, - string $principal + Principal $principal, ): ResourceMember { $resource = $this->resourceService->find($resourceId); @@ -56,10 +61,7 @@ class ResourceMemberService { $member->setResourceId($resource->getId()); $member->setPermissionLevel($permissionLevel->value); - $member->setType($type->value); - - // TODO: check if principal fits format - $member->setPrincipal($principal); + $member->setPrincipal($principal); $member->setCreatedTimestamp(time()); $member->setLastUpdatedTimestamp(time()); @@ -70,7 +72,7 @@ class ResourceMemberService { return $member; } - public function update(int $id, ?MemberPermissionLevel $permissionLevel = null, ?MemberType $type = null, ?string $principal = null): ResourceMember { + public function update(int $id, ?MemberPermissionLevel $permissionLevel = null, ?Principal $principal = null): ResourceMember { try { $member = $this->mapper->find($id); @@ -78,12 +80,8 @@ class ResourceMemberService { $member->setPermissionLevel($permissionLevel->value); } - if(isset($type)) { - $member->setType($type->value); - } - if(isset($principal)) { - $member->setPrincipal($principal); + $member->setPrincipal($principal); } if(count($member->getUpdatedFields()) > 0) { diff --git a/lib/Service/ResourceService.php b/lib/Service/ResourceService.php index a1cd335..434b35e 100644 --- a/lib/Service/ResourceService.php +++ b/lib/Service/ResourceService.php @@ -16,8 +16,9 @@ use OCA\OrganizationFolders\Db\Resource; use OCA\OrganizationFolders\Db\FolderResource; use OCA\OrganizationFolders\Db\ResourceMapper; use OCA\OrganizationFolders\Model\OrganizationFolder; +use \OCA\OrganizationFolders\Model\Principal; use OCA\OrganizationFolders\Enum\MemberPermissionLevel; -use OCA\OrganizationFolders\Enum\MemberType; +use OCA\OrganizationFolders\Enum\PrincipalType; use OCA\OrganizationFolders\Errors\InvalidResourceType; use OCA\OrganizationFolders\Errors\ResourceNotFound; use OCA\OrganizationFolders\Errors\ResourceNameNotUnique; @@ -37,7 +38,16 @@ class ResourceService { ) { } - public function findAll(int $organizationFolderId, int $parentResourceId = null, array $filters = []) { + /** + * @param int $organizationFolderId + * @psalm-param int $organizationFolderId + * @param int|null $parentResourceId + * @psalm-param int|null $parentResourceId + * @param array $filters + * @psalm-param array $filters + * @psalm-return Resource[] + */ + public function findAll(int $organizationFolderId, ?int $parentResourceId = null, array $filters = []): array { return $this->mapper->findAll($organizationFolderId, $parentResourceId, $filters); } @@ -93,7 +103,11 @@ class ResourceService { $parentResource = $this->find($parentResourceId); if($parentResource->getOrganizationFolderId() === $organizationFolderId) { - $resource->setParentResource($parentResource->getId()); + if($parentResource->getType() !== "folder") { + throw new Exception("Only folder resources can have sub-resources"); + } else { + $resource->setParentResource($parentResource->getId()); + } } else { throw new Exception("Cannot create child-resource of parent in different organizationId"); } @@ -136,7 +150,6 @@ class ResourceService { int $id, ?string $name = null, - ?int $parentResource = null, ?bool $active = null, ?bool $inheritManagers = null, @@ -146,10 +159,6 @@ class ResourceService { ): Resource { $resource = $this->find($id); - if(isset($parentResource)) { - $resource->setParentResource($parentResource); - } - if(isset($name)) { if($this->mapper->existsWithName($resource->getOrganizationFolderId(), $resource->getParentResource(), $name)) { throw new ResourceNameNotUnique(); @@ -206,10 +215,7 @@ class ResourceService { $inheritingPrincipals = []; foreach($inheritingGroups as $inheritingGroup) { - $inheritingPrincipals[] = [ - "type" => "group", - "groupId" => $inheritingGroup, - ]; + $inheritingPrincipals[] = new Principal(PrincipalType::GROUP, $inheritingGroup); } return $this->recursivelySetFolderResourceALCs($topLevelFolderResources, "", $inheritingPrincipals); @@ -222,8 +228,8 @@ class ResourceService { * @psalm-param FolderResource[] $folderResources * @param string $path * @psalm-param string $path - * @param array $inheritingGroups - * @psalm-param string[] $inheritingGroups + * @param array $inheritingPrincipals + * @psalm-param Principal[] $inheritingPrincipals */ public function recursivelySetFolderResourceALCs(array $folderResources, string $path, array $inheritingPrincipals) { foreach($folderResources as $folderResource) { @@ -232,20 +238,16 @@ class ResourceService { // inherit ACLs foreach($inheritingPrincipals as $inheritingPrincipal) { - if($inheritingPrincipal["type"] === "group") { - $acls[] = new Rule( - userMapping: $this->userMappingManager->mappingFromId("group", $inheritingPrincipal["groupId"]), - fileId: $resourceFileId, - mask: 31, - permissions: $folderResource->getInheritedAclPermission(), - ); - } else if($inheritingPrincipal["type"] === "user") { - $acls[] = new Rule( - userMapping: $this->userMappingManager->mappingFromId("user", $inheritingPrincipal["userId"]), - fileId: $resourceFileId, - mask: 31, - permissions: $folderResource->getInheritedAclPermission(), - ); + $newACL = $this->aclManager->createAclRuleForPrincipal( + principal: $inheritingPrincipal, + fileId: $resourceFileId, + mask: 31, + permissions: $folderResource->getInheritedAclPermission(), + ); + + // if mapping for principal could not be created, skip creating rule for it + if(!is_null($newACL)) { + $acls[] = $newACL; } } @@ -260,6 +262,7 @@ class ResourceService { /** @var ResourceService */ $resourceMemberService = $this->container->get(ResourceMemberService::class); $resourceMembers = $resourceMemberService->findAll($folderResource->getId()); + foreach($resourceMembers as $resourceMember) { if($resourceMember->getPermissionLevel() === MemberPermissionLevel::MANAGER->value) { $resourceMemberPermissions = $folderResource->getManagersAclPermission(); @@ -270,43 +273,20 @@ class ResourceService { } if($resourceMemberPermissions !== 0) { - if($resourceMember->getType() === MemberType::USER->value) { - $mapping = $this->userMappingManager->mappingFromId("user", $resourceMember->getPrincipal()); - $nextInheritingPrincipals[] = [ - "type" => "user", - "userId" => $resourceMember->getPrincipal(), - ]; - } else if($resourceMember->getType() === MemberType::GROUP->value) { - $mapping = $this->userMappingManager->mappingFromId("group", $resourceMember->getPrincipal()); - $nextInheritingPrincipals[] = [ - "type" => "group", - "groupId" => $resourceMember->getPrincipal(), - ]; - } else if($resourceMember->getType() === MemberType::ROLE->value) { - ['organizationProviderId' => $organizationProviderId, 'roleId' => $roleId] = $resourceMember->getParsedPrincipal(); + $resourceMemberPrincipal = $resourceMember->getPrincipal(); - $organizationProvider = $this->organizationProviderManager->getOrganizationProvider($organizationProviderId); - $role = $organizationProvider->getRole($roleId); - $mapping = $this->userMappingManager->mappingFromId("group", $role->getMembersGroup()); - $nextInheritingPrincipals[] = [ - "type" => "group", - "groupId" => $role->getMembersGroup(), - ]; - } else { - throw new Exception("invalid resource member type"); - } - - if(is_null($mapping)) { - // TODO: skip member instead of crashing - throw new Exception(message: "invalid mapping, likely non-existing group"); - } - - $acls[] = new Rule( - userMapping: $mapping, + $newACL = $this->aclManager->createAclRuleForPrincipal( + principal: $resourceMemberPrincipal, fileId: $resourceFileId, mask: 31, permissions: $resourceMemberPermissions, ); + + // if mapping for principal could not be created, skip creating rule for it + if(!is_null($newACL)) { + $acls[] = $newACL; + $nextInheritingPrincipals[] = $resourceMemberPrincipal; + } } }