From 9ebae48238b8a9e1cc41497c29b5e9f8744e7f33 Mon Sep 17 00:00:00 2001 From: Jonathan Treffler Date: Sat, 16 Nov 2024 03:06:21 +0100 Subject: [PATCH] many small resource member related improvements --- .../ResourceMember/CreateResourceMember.php | 22 +++++++++--------- lib/Dav/PropFindPlugin.php | 2 +- lib/Db/ResourceMember.php | 19 +++++++++++++-- lib/Db/ResourceMemberMapper.php | 6 ++--- ....php => ResourceMemberPermissionLevel.php} | 2 +- .../Version000000Date20241025120000.php | 18 +++++++-------- lib/Security/ResourceVoter.php | 23 +++++++++++++------ lib/Service/ResourceMemberService.php | 17 +++++++------- lib/Service/ResourceService.php | 6 ++--- 9 files changed, 70 insertions(+), 45 deletions(-) rename lib/Enum/{MemberPermissionLevel.php => ResourceMemberPermissionLevel.php} (74%) diff --git a/lib/Command/ResourceMember/CreateResourceMember.php b/lib/Command/ResourceMember/CreateResourceMember.php index 433683c..4864917 100644 --- a/lib/Command/ResourceMember/CreateResourceMember.php +++ b/lib/Command/ResourceMember/CreateResourceMember.php @@ -8,8 +8,9 @@ use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; use OCA\OrganizationFolders\Command\BaseCommand; -use OCA\OrganizationFolders\Enum\MemberType; -use OCA\OrganizationFolders\Enum\MemberPermissionLevel; +use OCA\OrganizationFolders\Model\Principal; +use OCA\OrganizationFolders\Enum\PrincipalType; +use OCA\OrganizationFolders\Enum\ResourceMemberPermissionLevel; class CreateResourceMember extends BaseCommand { protected function configure(): void { @@ -18,27 +19,26 @@ class CreateResourceMember extends BaseCommand { ->setDescription('Create a new member of resource') ->addOption('resource-id', null, InputOption::VALUE_REQUIRED, 'Id of resource to create member of') ->addOption('permission-level', null, InputOption::VALUE_REQUIRED, 'Permissions level of member (valid values: MEMBER, MANAGER)') - ->addOption('type', null, InputOption::VALUE_REQUIRED, 'Type of principal (valid values: USER, GROUP, ROLE)') - ->addOption('principal', null, InputOption::VALUE_OPTIONAL, 'For type user: "[user_id]", for group: "[group_name]", for role: "[organization_provider_id]:[role_id]"'); + ->addOption('principal-type', null, InputOption::VALUE_REQUIRED, 'Type of principal (valid values: USER, GROUP, ROLE)') + ->addOption('principal-id', null, InputOption::VALUE_OPTIONAL, 'For type user: "[user_id]", for group: "[group_name]", for role: "[organization_provider_id]:[role_id]"'); parent::configure(); } protected function execute(InputInterface $input, OutputInterface $output): int { $resourceId = $input->getOption('resource-id'); - $permissionLevel = MemberPermissionLevel::fromNameOrValue($input->getOption('permission-level')); - $type = MemberType::fromNameOrValue($input->getOption('type')); - $principal = $input->getOption('principal'); + $permissionLevel = ResourceMemberPermissionLevel::fromNameOrValue($input->getOption('permission-level')); + $principalType = PrincipalType::fromNameOrValue($input->getOption('principal-type')); + $principalId = $input->getOption('principal-id'); try { - $resource = $this->resourceMemberService->create( + $member = $this->resourceMemberService->create( resourceId: $resourceId, permissionLevel: $permissionLevel, - type: $type, - principal: $principal, + principal: new Principal($principalType, $principalId), ); - $this->writeTableInOutputFormat($input, $output, [$this->formatTableSerializable($resource)]); + $this->writeTableInOutputFormat($input, $output, [$this->formatTableSerializable($member)]); return 0; } catch (Exception $e) { $output->writeln("Exception \"{$e->getMessage()}\" at {$e->getFile()} line {$e->getLine()}"); diff --git a/lib/Dav/PropFindPlugin.php b/lib/Dav/PropFindPlugin.php index f17b24b..1f08500 100644 --- a/lib/Dav/PropFindPlugin.php +++ b/lib/Dav/PropFindPlugin.php @@ -60,7 +60,7 @@ class PropFindPlugin extends ServerPlugin { $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'; + return $this->authorizationService->isGranted(["UPDATE"], $resource) ? 'true' : 'false'; } catch (\Exception $e) { return null; } diff --git a/lib/Db/ResourceMember.php b/lib/Db/ResourceMember.php index 9f0c0d1..c0ebcc0 100644 --- a/lib/Db/ResourceMember.php +++ b/lib/Db/ResourceMember.php @@ -7,7 +7,7 @@ use OCA\OrganizationFolders\Interface\TableSerializable; use OCP\AppFramework\Db\Entity; -use OCA\OrganizationFolders\Enum\MemberPermissionLevel; +use OCA\OrganizationFolders\Enum\ResourceMemberPermissionLevel; use OCA\OrganizationFolders\Enum\PrincipalType; use OCA\OrganizationFolders\Model\Principal; @@ -26,6 +26,7 @@ class ResourceMember extends Entity implements JsonSerializable, TableSerializab $this->addType('createdTimestamp','integer'); $this->addType('lastUpdatedTimestamp','integer'); } + public function getPrincipal(): Principal { return new Principal(PrincipalType::from($this->principalType), $this->principalId); } @@ -35,6 +36,20 @@ class ResourceMember extends Entity implements JsonSerializable, TableSerializab $this->setPrincipalId($principal->getId()); } + public function setPermissionLevel(int $permissionLevel) { + if($permissionLevel >= 1 && $permissionLevel <= 2) { + if ($permissionLevel === $this->permissionLevel) { + // no change + return; + } + + $this->markFieldUpdated("permissionLevel"); + $this->permissionLevel = $permissionLevel; + } else { + throw new \Exception("invalid resource member permission level"); + } + } + public function jsonSerialize(): array { return [ 'id' => $this->id, @@ -50,7 +65,7 @@ class ResourceMember extends Entity implements JsonSerializable, TableSerializab return [ 'Id' => $this->id, 'Resource Id' => $this->resourceId, - 'Permission Level' => MemberPermissionLevel::from($this->permissionLevel)->name, + 'Permission Level' => ResourceMemberPermissionLevel::from($this->permissionLevel)->name, 'Principal Type' => PrincipalType::from($this->principalType)->name, 'Principal Id' => $this->principalId, 'Created' => $this->createdTimestamp, diff --git a/lib/Db/ResourceMemberMapper.php b/lib/Db/ResourceMemberMapper.php index 7ada460..475210f 100644 --- a/lib/Db/ResourceMemberMapper.php +++ b/lib/Db/ResourceMemberMapper.php @@ -49,15 +49,15 @@ class ResourceMemberMapper extends QBMapper { return $this->findEntities($qb); } - public function exists(int $resourceId, int $type, string $principal): bool { + public function exists(int $resourceId, int $principalType, string $principalId): bool { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); $qb->select($qb->createFunction('COUNT(1)')) ->from(self::RESOURCE_MEMBERS_TABLE) ->where($qb->expr()->eq('resource_id', $qb->createNamedParameter($resourceId, IQueryBuilder::PARAM_INT))) - ->andWhere($qb->expr()->eq('type', $qb->createNamedParameter($type, IQueryBuilder::PARAM_INT))) - ->andWhere($qb->expr()->eq('principal', $qb->createNamedParameter($principal))); + ->andWhere($qb->expr()->eq('principal_type', $qb->createNamedParameter($principalType, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->eq('principal_id', $qb->createNamedParameter($principalId))); return $qb->executeQuery()->fetch()["COUNT(1)"] === 1; } diff --git a/lib/Enum/MemberPermissionLevel.php b/lib/Enum/ResourceMemberPermissionLevel.php similarity index 74% rename from lib/Enum/MemberPermissionLevel.php rename to lib/Enum/ResourceMemberPermissionLevel.php index f61e99c..55cb2ec 100644 --- a/lib/Enum/MemberPermissionLevel.php +++ b/lib/Enum/ResourceMemberPermissionLevel.php @@ -2,7 +2,7 @@ namespace OCA\OrganizationFolders\Enum; -enum MemberPermissionLevel: int { +enum ResourceMemberPermissionLevel: int { use FromNameEnum; case MEMBER = 1; diff --git a/lib/Migration/Version000000Date20241025120000.php b/lib/Migration/Version000000Date20241025120000.php index 4866f2d..d74428b 100644 --- a/lib/Migration/Version000000Date20241025120000.php +++ b/lib/Migration/Version000000Date20241025120000.php @@ -34,20 +34,20 @@ class Version000000Date20241025120000 extends SimpleMigrationStep { $table->addColumn('resource_id', Types::INTEGER, [ 'notnull' => true, ]); - // 0: member - // 1: manager + // 1: MEMBER + // 2: MANAGER $table->addColumn('permission_level', Types::INTEGER, [ 'notnull' => true, ]); - // 0: user - // 1: group - // 2: role + // 1: USER + // 2: GROUP + // 3: ROLE $table->addColumn('principal_type', Types::INTEGER, [ 'notnull' => true, ]); - // for principal type user: "[user_id]" - // for principal type group: "[group_name]" - // for principal type role: "[organization_provider_id]:[role_id]" + // 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, @@ -67,7 +67,7 @@ class Version000000Date20241025120000 extends SimpleMigrationStep { ['onDelete' => 'CASCADE'], 'organizationfolders_resource_members_resource_id_fk'); $table->addIndex(['resource_id'], 'organizationfolders_resource_members_resource_id_index'); - $table->addUniqueConstraint(['resource_id', 'type', 'principal'], "organizationfolders_resource_members_unique"); + $table->addUniqueConstraint(['resource_id', 'principal_type', 'principal_id'], "organizationfolders_resource_members_unique"); } return $schema; diff --git a/lib/Security/ResourceVoter.php b/lib/Security/ResourceVoter.php index 9fb61ab..422b77a 100644 --- a/lib/Security/ResourceVoter.php +++ b/lib/Security/ResourceVoter.php @@ -8,7 +8,7 @@ use OCP\IGroupManager; use OCA\OrganizationFolders\Db\Resource; use OCA\OrganizationFolders\Service\ResourceService; use OCA\OrganizationFolders\Service\ResourceMemberService; -use OCA\OrganizationFolders\Enum\MemberPermissionLevel; +use OCA\OrganizationFolders\Enum\ResourceMemberPermissionLevel; use OCA\OrganizationFolders\Enum\PrincipalType; use OCA\OrganizationFolders\OrganizationProvider\OrganizationProviderManager; @@ -39,6 +39,7 @@ class ResourceVoter extends Voter { 'UPDATE' => $this->isGranted($user, $resource), 'DELETE' => $this->isGranted($user, $resource), 'UPDATE_MEMBERS' => $this->isGranted($user, $resource), + 'CREATE_SUBRESOURCE' => $this->isGranted($user, $resource), default => throw new \LogicException('This code should not be reached!') }; } @@ -59,7 +60,7 @@ class ResourceVoter extends Voter { $resourceMembers = $this->resourceMemberService->findAll($resource->getId()); foreach($resourceMembers as $resourceMember) { - if($resourceMember->getPermissionLevel() === MemberPermissionLevel::MANAGER->value) { + if($resourceMember->getPermissionLevel() === ResourceMemberPermissionLevel::MANAGER->value) { $principal = $resourceMember->getPrincipal(); if($principal->getType() === PrincipalType::USER) { @@ -67,16 +68,13 @@ class ResourceVoter extends Voter { return true; } } else if($principal->getType() === PrincipalType::GROUP) { - if($this->groupManager->isInGroup($user->getUID(), $principal->getId())) { + if($this->userIsInGroup($user, $principal->getId())) { return true; } } 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())) { + if($this->userHasRole($user, $organizationProviderId, $roleId)) { return true; } } @@ -97,4 +95,15 @@ class ResourceVoter extends Voter { protected function isGranted(IUser $user, Resource $resource): bool { return $this->isResourceOrganizationFolderAdmin($user, $resource) || $this->isResourceManager($user, $resource); } + + private function userIsInGroup(IUser $user, string $groupId): bool { + return $this->groupManager->isInGroup($user->getUID(), $groupId); + } + + private function userHasRole(IUser $user, string $organizationProviderId, string $roleId): bool { + $organizationProvider = $this->organizationProviderManager->getOrganizationProvider($organizationProviderId); + $role = $organizationProvider->getRole($roleId); + + return $this->userIsInGroup($user, $role->getMembersGroup()); + } } diff --git a/lib/Service/ResourceMemberService.php b/lib/Service/ResourceMemberService.php index ebab643..2ae7315 100644 --- a/lib/Service/ResourceMemberService.php +++ b/lib/Service/ResourceMemberService.php @@ -11,8 +11,7 @@ use OCA\OrganizationFolders\Errors\ResourceMemberNotFound; use OCA\OrganizationFolders\Db\ResourceMember; use OCA\OrganizationFolders\Db\ResourceMemberMapper; - -use OCA\OrganizationFolders\Enum\MemberPermissionLevel; +use OCA\OrganizationFolders\Enum\ResourceMemberPermissionLevel; use OCA\OrganizationFolders\Model\Principal; class ResourceMemberService { @@ -52,7 +51,7 @@ class ResourceMemberService { public function create( int $resourceId, - MemberPermissionLevel $permissionLevel, + ResourceMemberPermissionLevel $permissionLevel, Principal $principal, ): ResourceMember { $resource = $this->resourceService->find($resourceId); @@ -62,8 +61,10 @@ class ResourceMemberService { $member->setResourceId($resource->getId()); $member->setPermissionLevel($permissionLevel->value); $member->setPrincipal($principal); - $member->setCreatedTimestamp(time()); - $member->setLastUpdatedTimestamp(time()); + + $creationTime = time(); + $member->setCreatedTimestamp($creationTime); + $member->setLastUpdatedTimestamp($creationTime); $member = $this->mapper->insert($member); @@ -72,7 +73,7 @@ class ResourceMemberService { return $member; } - public function update(int $id, ?MemberPermissionLevel $permissionLevel = null, ?Principal $principal = null): ResourceMember { + public function update(int $id, ?ResourceMemberPermissionLevel $permissionLevel = null, ?Principal $principal = null): ResourceMember { try { $member = $this->mapper->find($id); @@ -86,9 +87,9 @@ class ResourceMemberService { if(count($member->getUpdatedFields()) > 0) { $member->setLastUpdatedTimestamp(time()); - } - $member = $this->mapper->update($member); + $member = $this->mapper->update($member); + } $resource = $this->resourceService->find($member->getResourceId()); $this->organizationFolderService->applyPermissions($resource->getOrganizationFolderId()); diff --git a/lib/Service/ResourceService.php b/lib/Service/ResourceService.php index 434b35e..25c4618 100644 --- a/lib/Service/ResourceService.php +++ b/lib/Service/ResourceService.php @@ -17,7 +17,7 @@ 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\ResourceMemberPermissionLevel; use OCA\OrganizationFolders\Enum\PrincipalType; use OCA\OrganizationFolders\Errors\InvalidResourceType; use OCA\OrganizationFolders\Errors\ResourceNotFound; @@ -264,9 +264,9 @@ class ResourceService { $resourceMembers = $resourceMemberService->findAll($folderResource->getId()); foreach($resourceMembers as $resourceMember) { - if($resourceMember->getPermissionLevel() === MemberPermissionLevel::MANAGER->value) { + if($resourceMember->getPermissionLevel() === ResourceMemberPermissionLevel::MANAGER->value) { $resourceMemberPermissions = $folderResource->getManagersAclPermission(); - } else if($resourceMember->getPermissionLevel() === MemberPermissionLevel::MEMBER->value) { + } else if($resourceMember->getPermissionLevel() === ResourceMemberPermissionLevel::MEMBER->value) { $resourceMemberPermissions = $folderResource->getMembersAclPermission(); } else { throw new Exception("invalid resource member permission level");