From 02516e9b7bdd214f9c817c7ccd0314a3951f7955 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Nov 2025 16:57:34 +0000 Subject: [PATCH 01/32] Initial plan From 867fd1d2e593ca12b9c60e8f5f1bb5b4e05ed6f2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Nov 2025 17:03:36 +0000 Subject: [PATCH 02/32] Create InTeamAssertion class and refactor Module.php to use it Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com> --- Module.php | 378 ++------------------- config/module.config.php | 5 + src/Acl/InTeamAssertion.php | 313 +++++++++++++++++ src/Service/Acl/InTeamAssertionFactory.php | 18 + 4 files changed, 362 insertions(+), 352 deletions(-) create mode 100644 src/Acl/InTeamAssertion.php create mode 100644 src/Service/Acl/InTeamAssertionFactory.php diff --git a/Module.php b/Module.php index e62f7f5..89f6c34 100644 --- a/Module.php +++ b/Module.php @@ -283,16 +283,6 @@ protected function addAclRules() Entity\TeamRole::class, $entityRights ); - $acl->allow( - null, - [ - Entity\Team::class, - Entity\TeamUser::class, - Entity\TeamResource::class, - - ], - $entityRights - ); // Only admin can manage teams. $adminRoles = [ @@ -339,34 +329,6 @@ protected function addAclRules() ); } - $acl->allow( - $viewerRoles, - [Entity\TeamResource::class], - ['read', 'create', 'update', 'delete'] - ); - - $acl->allow( - $viewerRoles, - [Entity\Team::class], - ['read', 'create', 'update', 'delete'] - ); - - $acl->allow( - $viewerRoles, - [TeamUser::class, Entity\TeamResource::class], - ['read', 'create', 'update', 'delete'] - ); - - $acl->allow( - $viewerRoles, - [Entity\Team::class], - ['read', 'create', 'update', 'delete'] - ); - $acl->allow( - $viewerRoles, - [Entity\TeamUser::class, Entity\TeamResource::class, Entity\TeamRole::class], - ['read', 'create', 'update', 'delete'] - ); $acl->allow( $viewerRoles, [Api\Adapter\TeamRoleAdapter::class], @@ -432,6 +394,32 @@ protected function addAclRules() 'update' ); + // Use InTeamAssertion for entity-level permissions + $assertion = $services->get(\Teams\Acl\InTeamAssertion::class); + + // Apply the assertion to all entity resources for non-admin roles + $acl->allow( + $viewerRoles, + [ + Entity\Team::class, + Entity\TeamUser::class, + Entity\TeamResource::class, + Entity\TeamRole::class, + Entity\TeamAsset::class, + Entity\TeamSite::class, + Entity\TeamResourceTemplate::class, + \Omeka\Entity\Item::class, + \Omeka\Entity\ItemSet::class, + \Omeka\Entity\Media::class, + \Omeka\Entity\Asset::class, + \Omeka\Entity\Site::class, + \Omeka\Entity\SitePage::class, + \Omeka\Entity\ResourceTemplate::class, + ], + $entityRights, + $assertion + ); + } /** @@ -2104,95 +2092,12 @@ public function siteEdit(Event $event) echo $view->partial('teams/partial/site-admin/edit', ['site_teams' => $site_teams, 'team_ids' => $team_ids]); } - /** - * Returns the expected string for proxied resource class in cases where the class returned is the doctrine proxy. - * - * @param $resource - * @return false|string - */ - public function getResourceClass($resource) - { - $doctrine_ent = 'DoctrineProxies\__CG__'; - $doctrine_test = strpos(get_class($resource), $doctrine_ent); - if ($doctrine_test === false) { - $res_class = get_class($resource); - } else { - $res_class = substr(get_class($resource), strlen($doctrine_ent) + 1); - } - return $res_class; - } public function getModules() { $manager = $this->getServiceLocator()->get('Omeka\ModuleManager'); } - /** - * Check to see if the user and the object they are attempting to access or change are part of the same team. - * - * @param $resource - * @param $team_user - * @return bool - */ - public function inTeam($resource, $team_user) - { - $em = $this->getServiceLocator()->get('Omeka\EntityManager'); - $resource_domains = ['Omeka\Entity\Item', 'Omeka\Entity\ItemSet', 'Omeka\Entity\Media']; - $fk_id = $resource->getId(); - $team = $team_user->getTeam(); - $user = $team_user->getUser(); - $res_class = $this->getResourceClass($resource); - - if (in_array($res_class, $resource_domains)) { - $teamsRepo = 'Teams\Entity\TeamResource'; - $fk = 'resource'; - $criteria = ['team' => $team->getId(), $fk => $fk_id]; - } elseif ($res_class == 'Omeka\Entity\Site') { - $teamsRepo = 'Teams\Entity\TeamSite'; - $fk = 'site'; - $criteria = ['team' => $team->getId(), $fk => $fk_id]; - } elseif ($res_class == 'Omeka\Entity\SitePage') { - $teamsRepo = 'Teams\Entity\TeamSite'; - $fk = 'site'; - $fk_id = $resource->getSite()->getId(); - $criteria = ['team' => $team->getId(), $fk => $fk_id]; - } elseif ($res_class == 'Omeka\Entity\ResourceTemplate') { - $teamsRepo = 'Teams\Entity\TeamResourceTemplate'; - $fk = 'resource_template'; - $criteria = ['team' => $team->getId(), $fk => $fk_id]; - } elseif ($res_class == 'Teams\Entity\Team') { - $teamsRepo = 'Teams\Entity\TeamUser'; - $fk = 'user'; - $criteria = ['team' => $team->getId(), $fk => $user->getId()]; - } elseif ($res_class == 'Teams\Entity\TeamSite') { - $teamsRepo = 'Teams\Entity\TeamUser'; - $fk = 'user'; - $criteria = ['team' => $resource->getTeam()->getId(), $fk => $user->getId()]; - } elseif ($res_class == 'Teams\Entity\TeamResource') { - $teamsRepo = 'Teams\Entity\TeamResource'; - $fk = 'resource'; - $fk_id = $resource->getResource()->getId(); - $criteria = ['team' => $team->getId(), $fk => $fk_id]; - } - /* - * TeamRole is only accessible by global admin so doesn't need to be checked. - * Other classes should be controlled by their respective modules and components. - */ - - else { - return true; - } - - if ($team_resource = $em->getRepository($teamsRepo) - ->findOneBy($criteria)) { - $in_team = true; - } else { - $in_team = false; - } - - return $in_team; - } - public function getUser() { if ($this->getServiceLocator() @@ -2204,224 +2109,6 @@ public function getUser() } } - /** - * Checks to see if the user has the authority to do something based on their role in their current team. Users can - * have different roles with different permissions in different teams. - * - * @param EntityInterface $resource - * @param $action - * @param Event $event - * @return bool - */ - public function teamAuthority(EntityInterface $resource, $action, Event $event) - { - $user = $this->getUser(); - - /* - * first go through a couple of common cases where we don't need to judge permissions and don't bother checking - * any other condition - */ - - //if the user isn't logged in (e.g., the public), use the default settings - if (!$user) { - return true; - } - - /* - * This may make the previous rule redundant, but keeping both for now - * No matter who is looking at the front-end, don't consider team authority - * based on the user - */ - - //if it isn't on the backend, let the public vs private rules take over - if ($this->getServiceLocator()->get('Omeka\Status')->isSiteRequest()) { - return true; - } - - $is_glob_admin = ($user->getRole() === 'global_admin'); - - //if it is the global admin, bypass any team controls (but will still apply filters) - if ($is_glob_admin) { - return true; - } - $messenger = new Messenger(); - - $authorized = false; - - $res_class = $this->getResourceClass($resource); - - //case that I don't fully understand. When selecting resource template on new item form - //the Omeka\AuthenticationService->getIdentity() returns null - - $em = $this->getServiceLocator()->get('Omeka\EntityManager'); - $user_id = $user->getId(); - $team_user = $em->getRepository('Teams\Entity\TeamUser') - ->findOneBy(['is_current' => true, 'user' => $user_id]); - $team = $team_user->getTeam(); - $team_user_role = $team_user->getRole() ; - - //don't check if in same team if a create action, because items only assigned teams after creation - if ($action != 'create') { - //if resource not part of user's current team, no action at all - if (! $this->inTeam($resource, $team_user)) { - $authorized = false; - $err = sprintf( - // $this->getTranslator()->translate( - 'Permission denied. Resource "%1$s: %2$s" is not part of your current team, %3$s. - If you feel this is an error, try changing teams or talk to the administrator. - Action: %4$s - ' -// ) - , - get_class($resource), - $resource->getId(), - $team->getName(), - $action - ); - $messenger->addError($err); - throw new Exception\PermissionDeniedException( - $err - ); - } - } - - $resource_domains = [ - 'Omeka\Entity\Item', - 'Omeka\Entity\ItemSet', - 'Omeka\Entity\Media', - 'Omeka\Entity\Asset', - 'Omeka\Entity\ResourceTemplate', - 'Teams\Entity\TeamResource', - 'Teams\Entity\TeamAsset', - ]; - - if (in_array($res_class, $resource_domains)) { - if ($action == 'create') { - $authorized = $team_user_role->getCanAddItems(); - } elseif ($action == 'delete' || $action == 'batch_delete') { - $authorized = $team_user_role->getCanDeleteResources(); - } elseif ($action == 'update') { - $authorized = $team_user_role->getCanModifyResources(); - } elseif ($action == 'read' || $action == 'search') { - $authorized = true; - } - } elseif ($res_class == 'Omeka\Entity\Site') { - if ($action == 'create') { - $globalSettings = $this->getServiceLocator()->get('Omeka\Settings'); - if ($globalSettings->get('teams_site_admin_make_site') && $user->getRole() == 'site_admin') { - $authorized = true; - } elseif ($globalSettings->get('teams_editor_make_site') && $user->getRole() == 'editor') { - $authorized = true; - } else { - $authorized = $is_glob_admin; - } - } elseif ($action == 'delete' || $action == 'batch_delete') { - $authorized = $team_user_role->getCanAddSitePages(); - } elseif ($action == 'update') { - $authorized = $team_user_role->getCanAddSitePages(); - } elseif ($action == 'read') { - $authorized = true; - } - } elseif ($res_class == 'Omeka\Entity\SitePage') { - if ($action == 'create') { - $authorized = $team_user_role->getCanAddSitePages(); - } elseif ($action == 'delete' || $action == 'batch_delete') { - $authorized = $team_user_role->getCanAddSitePages(); - } elseif ($action == 'update') { - $authorized = $team_user_role->getCanAddSitePages(); - } elseif ($action == 'read') { - $authorized = true; - } - } elseif ($res_class == 'Teams\Entity\Team' || $res_class == 'Teams\Entity\TeamRole') { - if ($action == 'create') { - $authorized = $is_glob_admin; - } elseif ($action == 'delete' || $action == 'batch_delete') { - $authorized = $is_glob_admin; - - //this is going to let them update the name and description - } elseif ($action == 'update') { - $authorized = $team_user_role->getCanAddUsers(); - } elseif ($action == 'read') { - $authorized = true; - } - } elseif ($res_class == 'Teams\Entity\TeamSite') { - if ($action == 'read') { - $authorized = true; - } - //adding or removing sites from a team - else { - $authorized = $team_user_role->getCanAddSitePages(); - } - } elseif ($res_class == 'Teams\Entity\TeamUser') { - if ($action == 'read') { - $authorized = true; - //deleting a site from a team - } elseif ($action == 'create') { - $authorized = $team_user_role->getCanAddUsers(); - } elseif ($action == 'delete') { - $authorized = $team_user_role->getCanAddUsers(); - } else { - $authorized = $is_glob_admin; - } - } - - //a list of classes where we don't need to check teams - //TODO: this should be refactored and go with the checks in the beginning - elseif ($res_class == 'Omeka\Entity\User') { - return true; - } elseif ($res_class == 'Omeka\Entity\Job') { - return true; - } elseif ($res_class == 'Omeka\Entity\Property') { - return true; - } elseif ($res_class == 'Teams\Entity\TeamRole') { - $authorized = $is_glob_admin; - } - - //don't police other modules by default - elseif (substr($res_class, 1, 12) !== 'Omeka\Entity') { - return true; - } - - if (!$authorized) { - $msg = sprintf( - 'Permission denied. Your role in %5$s, %4$s, does not permit you to %3$s this resource.', - get_class($resource), - $resource->getId(), - $action, - $team_user_role->getName(), - $team->getName() - ); - $diagnostic = sprintf( - 'Diagnostic: -- Resource type: %1$s. Resource id: %2$s. Action: %3$s. Your role: %4$s', - get_class($resource), - $resource->getId(), - $action, - $team_user_role->getName(), - $team->getName() - ); - - $messenger->addError($msg); - $messenger->addError($diagnostic); - throw new Exception\PermissionDeniedException($msg); - } - return $authorized; - } - - public function teamAuthorizeOnRead(Event $event) - { - $entity = $event->getParam('entity'); - $request = $event->getParam('request'); - $operation = $request->getOperation(); - $this->teamAuthority($entity, $operation, $event); - } - - public function teamAuthorizeOnHydrate(Event $event) - { - $request = $event->getParam('request'); - $entity = $event->getParam('entity'); - $operation = $request->getOperation(); - $this->teamAuthority($entity, $operation, $event); - } /** * Disable the auto-add field on the site edit form. Teams manages this by automatically adding items to the @@ -2621,25 +2308,12 @@ public function attachListeners(SharedEventManagerInterface $sharedEventManager) [$this, 'getOrphans'] ); - $sharedEventManager->attach( - '*', - 'api.find.post', - [$this, 'teamAuthorizeOnRead'] - ); - $sharedEventManager->attach( 'Teams\Controller\Index', 'view.browse.before', [$this, 'teamSelectorBrowse'] ); - //on api calls, make sure the users has the team authority to do action as well. - $sharedEventManager->attach( - '*', - 'api.hydrate.pre', - [$this, 'teamAuthorizeOnHydrate'] - ); - $sharedEventManager->attach( ItemSetAdapter::class, 'api.execute.post', diff --git a/config/module.config.php b/config/module.config.php index 41dff7b..d8c4a28 100644 --- a/config/module.config.php +++ b/config/module.config.php @@ -74,6 +74,11 @@ dirname(__DIR__) . '/data/doctrine-proxies', ], ], + 'service_manager' => [ + 'factories' => [ + Acl\InTeamAssertion::class => Service\Acl\InTeamAssertionFactory::class, + ], + ], 'form_elements' => [ 'invokables' => [ Form\TeamForm::class => Form\TeamForm::class, diff --git a/src/Acl/InTeamAssertion.php b/src/Acl/InTeamAssertion.php new file mode 100644 index 0000000..f112b5a --- /dev/null +++ b/src/Acl/InTeamAssertion.php @@ -0,0 +1,313 @@ +auth = $auth; + $this->entityManager = $entityManager; + $this->status = $status; + } + + /** + * Assert whether the current user has permission based on team membership and role. + * + * @param Acl $acl + * @param RoleInterface|null $role + * @param ResourceInterface|null $resource + * @param string|null $privilege + * @return bool + */ + public function assert( + Acl $acl, + RoleInterface $role = null, + ResourceInterface $resource = null, + $privilege = null + ) { + // If no resource is provided, we can't check team membership + if (!$resource instanceof EntityInterface) { + return true; + } + + $user = $this->auth->getIdentity(); + + /* + * First go through a couple of common cases where we don't need to judge permissions + */ + + // If the user isn't logged in (e.g., the public), use the default settings + if (!$user) { + return true; + } + + // If it isn't on the backend, let the public vs private rules take over + if ($this->status->isSiteRequest()) { + return true; + } + + $is_glob_admin = ($user->getRole() === 'global_admin'); + + // If it is the global admin, bypass any team controls + if ($is_glob_admin) { + return true; + } + + $messenger = new Messenger(); + $authorized = false; + + $res_class = $this->getResourceClass($resource); + + $user_id = $user->getId(); + $team_user = $this->entityManager->getRepository('Teams\Entity\TeamUser') + ->findOneBy(['is_current' => true, 'user' => $user_id]); + + // If user doesn't have a current team, deny access + if (!$team_user) { + return false; + } + + $team = $team_user->getTeam(); + $team_user_role = $team_user->getRole(); + + // Don't check if in same team if a create action, because items only assigned teams after creation + if ($privilege != 'create') { + // If resource not part of user's current team, no action at all + if (!$this->inTeam($resource, $team_user)) { + $authorized = false; + $err = sprintf( + 'Permission denied. Resource "%1$s: %2$s" is not part of your current team, %3$s. + If you feel this is an error, try changing teams or talk to the administrator. + Action: %4$s', + get_class($resource), + $resource->getId(), + $team->getName(), + $privilege + ); + $messenger->addError($err); + throw new Exception\PermissionDeniedException($err); + } + } + + $resource_domains = [ + 'Omeka\Entity\Item', + 'Omeka\Entity\ItemSet', + 'Omeka\Entity\Media', + 'Omeka\Entity\Asset', + 'Omeka\Entity\ResourceTemplate', + 'Teams\Entity\TeamResource', + 'Teams\Entity\TeamAsset', + ]; + + if (in_array($res_class, $resource_domains)) { + if ($privilege == 'create') { + $authorized = $team_user_role->getCanAddItems(); + } elseif ($privilege == 'delete' || $privilege == 'batch_delete') { + $authorized = $team_user_role->getCanDeleteResources(); + } elseif ($privilege == 'update') { + $authorized = $team_user_role->getCanModifyResources(); + } elseif ($privilege == 'read' || $privilege == 'search') { + $authorized = true; + } + } elseif ($res_class == 'Omeka\Entity\Site') { + if ($privilege == 'create') { + // Site creation permissions are handled separately in addAclRules + return true; + } elseif ($privilege == 'delete' || $privilege == 'batch_delete') { + $authorized = $team_user_role->getCanAddSitePages(); + } elseif ($privilege == 'update') { + $authorized = $team_user_role->getCanAddSitePages(); + } elseif ($privilege == 'read') { + $authorized = true; + } + } elseif ($res_class == 'Omeka\Entity\SitePage') { + if ($privilege == 'create') { + $authorized = $team_user_role->getCanAddSitePages(); + } elseif ($privilege == 'delete' || $privilege == 'batch_delete') { + $authorized = $team_user_role->getCanAddSitePages(); + } elseif ($privilege == 'update') { + $authorized = $team_user_role->getCanAddSitePages(); + } elseif ($privilege == 'read') { + $authorized = true; + } + } elseif ($res_class == 'Teams\Entity\Team' || $res_class == 'Teams\Entity\TeamRole') { + if ($privilege == 'create') { + $authorized = $is_glob_admin; + } elseif ($privilege == 'delete' || $privilege == 'batch_delete') { + $authorized = $is_glob_admin; + } elseif ($privilege == 'update') { + $authorized = $team_user_role->getCanAddUsers(); + } elseif ($privilege == 'read') { + $authorized = true; + } + } elseif ($res_class == 'Teams\Entity\TeamSite') { + if ($privilege == 'read') { + $authorized = true; + } else { + // Adding or removing sites from a team + $authorized = $team_user_role->getCanAddSitePages(); + } + } elseif ($res_class == 'Teams\Entity\TeamUser') { + if ($privilege == 'read') { + $authorized = true; + } elseif ($privilege == 'create') { + $authorized = $team_user_role->getCanAddUsers(); + } elseif ($privilege == 'delete') { + $authorized = $team_user_role->getCanAddUsers(); + } else { + $authorized = $is_glob_admin; + } + } elseif ($res_class == 'Omeka\Entity\User') { + return true; + } elseif ($res_class == 'Omeka\Entity\Job') { + return true; + } elseif ($res_class == 'Omeka\Entity\Property') { + return true; + } elseif ($res_class == 'Teams\Entity\TeamRole') { + $authorized = $is_glob_admin; + } elseif (substr($res_class, 0, 13) !== 'Omeka\Entity\\') { + // Don't police other modules by default + return true; + } + + if (!$authorized) { + $msg = sprintf( + 'Permission denied. Your role in %5$s, %4$s, does not permit you to %3$s this resource.', + get_class($resource), + $resource->getId(), + $privilege, + $team_user_role->getName(), + $team->getName() + ); + $diagnostic = sprintf( + 'Diagnostic: -- Resource type: %1$s. Resource id: %2$s. Action: %3$s. Your role: %4$s', + get_class($resource), + $resource->getId(), + $privilege, + $team_user_role->getName(), + $team->getName() + ); + + $messenger->addError($msg); + $messenger->addError($diagnostic); + throw new Exception\PermissionDeniedException($msg); + } + + return $authorized; + } + + /** + * Check to see if the user and the object they are attempting to access or change are part of the same team. + * + * @param EntityInterface $resource + * @param \Teams\Entity\TeamUser $team_user + * @return bool + */ + private function inTeam($resource, $team_user) + { + $resource_domains = ['Omeka\Entity\Item', 'Omeka\Entity\ItemSet', 'Omeka\Entity\Media']; + $fk_id = $resource->getId(); + $team = $team_user->getTeam(); + $user = $team_user->getUser(); + $res_class = $this->getResourceClass($resource); + + if (in_array($res_class, $resource_domains)) { + $teamsRepo = 'Teams\Entity\TeamResource'; + $fk = 'resource'; + $criteria = ['team' => $team->getId(), $fk => $fk_id]; + } elseif ($res_class == 'Omeka\Entity\Site') { + $teamsRepo = 'Teams\Entity\TeamSite'; + $fk = 'site'; + $criteria = ['team' => $team->getId(), $fk => $fk_id]; + } elseif ($res_class == 'Omeka\Entity\SitePage') { + $teamsRepo = 'Teams\Entity\TeamSite'; + $fk = 'site'; + $fk_id = $resource->getSite()->getId(); + $criteria = ['team' => $team->getId(), $fk => $fk_id]; + } elseif ($res_class == 'Omeka\Entity\ResourceTemplate') { + $teamsRepo = 'Teams\Entity\TeamResourceTemplate'; + $fk = 'resource_template'; + $criteria = ['team' => $team->getId(), $fk => $fk_id]; + } elseif ($res_class == 'Teams\Entity\Team') { + $teamsRepo = 'Teams\Entity\TeamUser'; + $fk = 'user'; + $criteria = ['team' => $team->getId(), $fk => $user->getId()]; + } elseif ($res_class == 'Teams\Entity\TeamSite') { + $teamsRepo = 'Teams\Entity\TeamUser'; + $fk = 'user'; + $criteria = ['team' => $resource->getTeam()->getId(), $fk => $user->getId()]; + } elseif ($res_class == 'Teams\Entity\TeamResource') { + $teamsRepo = 'Teams\Entity\TeamResource'; + $fk = 'resource'; + $fk_id = $resource->getResource()->getId(); + $criteria = ['team' => $team->getId(), $fk => $fk_id]; + } else { + /* + * TeamRole is only accessible by global admin so doesn't need to be checked. + * Other classes should be controlled by their respective modules and components. + */ + return true; + } + + if ($team_resource = $this->entityManager->getRepository($teamsRepo) + ->findOneBy($criteria)) { + $in_team = true; + } else { + $in_team = false; + } + + return $in_team; + } + + /** + * Returns the expected string for proxied resource class in cases where the class returned is the doctrine proxy. + * + * @param mixed $resource + * @return string + */ + private function getResourceClass($resource) + { + $doctrine_ent = 'DoctrineProxies\__CG__'; + $doctrine_test = strpos(get_class($resource), $doctrine_ent); + if ($doctrine_test === false) { + $res_class = get_class($resource); + } else { + $res_class = substr(get_class($resource), strlen($doctrine_ent) + 1); + } + return $res_class; + } +} diff --git a/src/Service/Acl/InTeamAssertionFactory.php b/src/Service/Acl/InTeamAssertionFactory.php new file mode 100644 index 0000000..59811ea --- /dev/null +++ b/src/Service/Acl/InTeamAssertionFactory.php @@ -0,0 +1,18 @@ +get('Omeka\AuthenticationService'), + $services->get('Omeka\EntityManager'), + $services->get('Omeka\Status') + ); + } +} From fc91fd0bd24071c3eff7dedb6251a807db91172e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Nov 2025 17:06:03 +0000 Subject: [PATCH 03/32] Update event listeners to use ACL system with assertion Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com> --- Module.php | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 65 insertions(+), 2 deletions(-) diff --git a/Module.php b/Module.php index 89f6c34..92d3347 100644 --- a/Module.php +++ b/Module.php @@ -397,9 +397,12 @@ protected function addAclRules() // Use InTeamAssertion for entity-level permissions $assertion = $services->get(\Teams\Acl\InTeamAssertion::class); - // Apply the assertion to all entity resources for non-admin roles + // Combine admin and viewer roles (excluding global_admin which is handled separately) + $rolesWithAssertion = array_merge($viewerRoles, ['site_admin']); + + // Apply the assertion to all entity resources for non-global-admin roles $acl->allow( - $viewerRoles, + $rolesWithAssertion, [ Entity\Team::class, Entity\TeamUser::class, @@ -2109,6 +2112,52 @@ public function getUser() } } + /** + * Check team authorization when reading an entity via API + * + * @param Event $event + */ + public function teamAuthorizeOnRead(Event $event) + { + $entity = $event->getParam('entity'); + $request = $event->getParam('request'); + $operation = $request->getOperation(); + + // Use ACL to check authorization + $acl = $this->getServiceLocator()->get('Omeka\Acl'); + $user = $this->getUser(); + $role = $user ? $user->getRole() : null; + + if (!$acl->isAllowed($role, $entity, $operation)) { + throw new Exception\PermissionDeniedException( + sprintf('Permission denied for operation "%s" on resource.', $operation) + ); + } + } + + /** + * Check team authorization when hydrating an entity via API + * + * @param Event $event + */ + public function teamAuthorizeOnHydrate(Event $event) + { + $request = $event->getParam('request'); + $entity = $event->getParam('entity'); + $operation = $request->getOperation(); + + // Use ACL to check authorization + $acl = $this->getServiceLocator()->get('Omeka\Acl'); + $user = $this->getUser(); + $role = $user ? $user->getRole() : null; + + if (!$acl->isAllowed($role, $entity, $operation)) { + throw new Exception\PermissionDeniedException( + sprintf('Permission denied for operation "%s" on resource.', $operation) + ); + } + } + /** * Disable the auto-add field on the site edit form. Teams manages this by automatically adding items to the @@ -2308,12 +2357,26 @@ public function attachListeners(SharedEventManagerInterface $sharedEventManager) [$this, 'getOrphans'] ); + // Use ACL assertion for authorization on API operations + $sharedEventManager->attach( + '*', + 'api.find.post', + [$this, 'teamAuthorizeOnRead'] + ); + $sharedEventManager->attach( 'Teams\Controller\Index', 'view.browse.before', [$this, 'teamSelectorBrowse'] ); + // Use ACL assertion for authorization on API operations + $sharedEventManager->attach( + '*', + 'api.hydrate.pre', + [$this, 'teamAuthorizeOnHydrate'] + ); + $sharedEventManager->attach( ItemSetAdapter::class, 'api.execute.post', From fb6288f10997225381b8cf8b3f5741a75d443e80 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Nov 2025 17:08:31 +0000 Subject: [PATCH 04/32] Address code review feedback: fix namespaces, remove messenger, fix magic numbers Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com> --- config/module.config.php | 2 +- src/Acl/InTeamAssertion.php | 29 ++++++++--------------------- 2 files changed, 9 insertions(+), 22 deletions(-) diff --git a/config/module.config.php b/config/module.config.php index d8c4a28..3aa3ac4 100644 --- a/config/module.config.php +++ b/config/module.config.php @@ -76,7 +76,7 @@ ], 'service_manager' => [ 'factories' => [ - Acl\InTeamAssertion::class => Service\Acl\InTeamAssertionFactory::class, + \Teams\Acl\InTeamAssertion::class => \Teams\Service\Acl\InTeamAssertionFactory::class, ], ], 'form_elements' => [ diff --git a/src/Acl/InTeamAssertion.php b/src/Acl/InTeamAssertion.php index f112b5a..25a58f2 100644 --- a/src/Acl/InTeamAssertion.php +++ b/src/Acl/InTeamAssertion.php @@ -7,8 +7,6 @@ use Laminas\Permissions\Acl\Role\RoleInterface; use Laminas\Authentication\AuthenticationService; use Doctrine\ORM\EntityManager; -use Omeka\Mvc\Controller\Plugin\Messenger; -use Omeka\Permissions\Acl as OmekaAcl; use Omeka\Api\Exception; use Omeka\Entity\EntityInterface; @@ -87,7 +85,6 @@ public function assert( return true; } - $messenger = new Messenger(); $authorized = false; $res_class = $this->getResourceClass($resource); @@ -98,7 +95,9 @@ public function assert( // If user doesn't have a current team, deny access if (!$team_user) { - return false; + throw new Exception\PermissionDeniedException( + 'Permission denied. User does not have a current team assignment.' + ); } $team = $team_user->getTeam(); @@ -108,17 +107,15 @@ public function assert( if ($privilege != 'create') { // If resource not part of user's current team, no action at all if (!$this->inTeam($resource, $team_user)) { - $authorized = false; $err = sprintf( - 'Permission denied. Resource "%1$s: %2$s" is not part of your current team, %3$s. - If you feel this is an error, try changing teams or talk to the administrator. - Action: %4$s', + 'Permission denied. Resource "%1$s: %2$s" is not part of your current team, %3$s. ' . + 'If you feel this is an error, try changing teams or talk to the administrator. ' . + 'Action: %4$s', get_class($resource), $resource->getId(), $team->getName(), $privilege ); - $messenger->addError($err); throw new Exception\PermissionDeniedException($err); } } @@ -199,7 +196,7 @@ public function assert( return true; } elseif ($res_class == 'Teams\Entity\TeamRole') { $authorized = $is_glob_admin; - } elseif (substr($res_class, 0, 13) !== 'Omeka\Entity\\') { + } elseif (substr($res_class, 0, strlen('Omeka\\Entity\\')) !== 'Omeka\\Entity\\') { // Don't police other modules by default return true; } @@ -213,17 +210,7 @@ public function assert( $team_user_role->getName(), $team->getName() ); - $diagnostic = sprintf( - 'Diagnostic: -- Resource type: %1$s. Resource id: %2$s. Action: %3$s. Your role: %4$s', - get_class($resource), - $resource->getId(), - $privilege, - $team_user_role->getName(), - $team->getName() - ); - $messenger->addError($msg); - $messenger->addError($diagnostic); throw new Exception\PermissionDeniedException($msg); } @@ -306,7 +293,7 @@ private function getResourceClass($resource) if ($doctrine_test === false) { $res_class = get_class($resource); } else { - $res_class = substr(get_class($resource), strlen($doctrine_ent) + 1); + $res_class = substr(get_class($resource), strlen($doctrine_ent) + strlen('\\')); } return $res_class; } From d033d0dafad6e70c25b87681392e165c2039e1d0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Nov 2025 17:10:31 +0000 Subject: [PATCH 05/32] Fix string escaping and comment in getResourceClass method Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com> --- src/Acl/InTeamAssertion.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Acl/InTeamAssertion.php b/src/Acl/InTeamAssertion.php index 25a58f2..6c2db1f 100644 --- a/src/Acl/InTeamAssertion.php +++ b/src/Acl/InTeamAssertion.php @@ -196,7 +196,7 @@ public function assert( return true; } elseif ($res_class == 'Teams\Entity\TeamRole') { $authorized = $is_glob_admin; - } elseif (substr($res_class, 0, strlen('Omeka\\Entity\\')) !== 'Omeka\\Entity\\') { + } elseif (substr($res_class, 0, strlen('Omeka\Entity\\')) !== 'Omeka\Entity\\') { // Don't police other modules by default return true; } @@ -293,7 +293,8 @@ private function getResourceClass($resource) if ($doctrine_test === false) { $res_class = get_class($resource); } else { - $res_class = substr(get_class($resource), strlen($doctrine_ent) + strlen('\\')); + // Skip the doctrine proxy prefix and the backslash separator + $res_class = substr(get_class($resource), strlen($doctrine_ent) + 1); } return $res_class; } From 0bfcab2301db9f89aadda2d4e7c6038f028b8883 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Nov 2025 17:12:30 +0000 Subject: [PATCH 06/32] Fix duplicate TeamRole condition and incorrect string comparison logic Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com> --- src/Acl/InTeamAssertion.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Acl/InTeamAssertion.php b/src/Acl/InTeamAssertion.php index 6c2db1f..30a9a38 100644 --- a/src/Acl/InTeamAssertion.php +++ b/src/Acl/InTeamAssertion.php @@ -194,10 +194,8 @@ public function assert( return true; } elseif ($res_class == 'Omeka\Entity\Property') { return true; - } elseif ($res_class == 'Teams\Entity\TeamRole') { - $authorized = $is_glob_admin; - } elseif (substr($res_class, 0, strlen('Omeka\Entity\\')) !== 'Omeka\Entity\\') { - // Don't police other modules by default + } elseif (strpos($res_class, 'Omeka\Entity') !== 0) { + // Don't police other modules by default (not an Omeka entity) return true; } From 60712ef154dc82fe6e52f60636ef0163f4fc7682 Mon Sep 17 00:00:00 2001 From: Alex Dryden Date: Mon, 24 Nov 2025 21:41:35 -0500 Subject: [PATCH 07/32] fix: remove permissions unless granted by team Acl exhibits some counterintuitive behavior, inheritance, etc. You can't seem to deny all then allow where team grants permit becuase of hard to explain inheritance rules. So, resorting to denying unless the team permission resolves to true--which is very awkward in the acl syntax because you have to do a double-negative: (deny when when permission assertion is not true) --- Module.php | 341 +++++++++++++----------------------- src/Acl/InTeamAssertion.php | 245 ++++++++++---------------- 2 files changed, 215 insertions(+), 371 deletions(-) diff --git a/Module.php b/Module.php index 92d3347..e9e1597 100644 --- a/Module.php +++ b/Module.php @@ -13,6 +13,10 @@ use Omeka\Api\Adapter\SiteAdapter; use Omeka\Entity\EntityInterface; use Omeka\Permissions\Acl; +use Omeka\Permissions\Assertion\AssertionNegation; +use Omeka\Permissions\Assertion\IsSelfAssertion; +use Omeka\Permissions\Assertion\OwnsEntityAssertion; +use Teams\Acl\InTeamAssertion; use Teams\Entity\Team; use Teams\Entity\TeamAsset; use Teams\Entity\TeamResource; @@ -48,6 +52,12 @@ public function getConfig() public function onBootstrap(MvcEvent $event) { parent::onBootstrap($event); + } + /** + * The listener that calls our ACL setup. + */ + public function addAclRulesOnDispatch(MvcEvent $event) + { $this->addAclRules(); } @@ -237,194 +247,73 @@ public function createNamedParameter( //TODO need to refactor to normalize and condense protected function addAclRules() { - $services = $this->getServiceLocator(); - $acl = $services->get('Omeka\Acl'); - - $roles = $acl->getRoles(); - //entity rights are the actions of controllers - $entityRights = ['read', 'create', 'update', 'delete']; - - //allow everyone to see their teams - $acl->allow( - $roles, - 'Teams\Controller\Index', - ['index', 'teamDetail'] - ); - //any level of core role can be given the team permission to update the team - $acl->allow( - $roles, - 'Teams\Controller\Update', - ['teamUpdate'] - ); - - //allow everyone to change their current team - $acl->allow( - $roles, - 'Teams\Controller\Update', - ['currentTeam'] - ); - - $acl->allow( - 'global_admin', - [ - 'Teams\Controller\Index', - 'Teams\Controller\Add', - 'Teams\Controller\Update', - ], - [ - 'index', - 'teamAdd', - 'teamDetail', - 'teamUpdate', - ] - ); - $acl->allow( - 'global_admin', - Entity\TeamRole::class, - $entityRights - ); - - // Only admin can manage teams. - $adminRoles = [ - Acl::ROLE_GLOBAL_ADMIN, - Acl::ROLE_SITE_ADMIN, - + $acl = $this->getServiceLocator()->get('Omeka\Acl'); + $inTeamAssertion = $this->getServiceLocator()->get(\Teams\Acl\InTeamAssertion::class); + + $omekaResources = [ + \Omeka\Entity\Item::class, + \Omeka\Entity\ItemSet::class, + \Omeka\Entity\Media::class, + \Omeka\Entity\Site::class, + \Omeka\Entity\SitePage::class, + \Omeka\Entity\ResourceTemplate::class, + \Omeka\Entity\Asset::class, ]; - $viewerRoles = [ - Acl::ROLE_AUTHOR, - Acl::ROLE_EDITOR, - Acl::ROLE_RESEARCHER, - Acl::ROLE_REVIEWER, + $teamResources = [ + \Teams\Entity\Team::class, + \Teams\Entity\TeamUser::class, + \Teams\Entity\TeamSite::class, + \Teams\Entity\TeamResource::class, + \Teams\Entity\TeamAsset::class, ]; - - $acl->allow( - $viewerRoles, - [Api\Adapter\TeamAdapter::class, Api\Adapter\TeamResourceAdapter::class], - ['search', 'read'] - ); - $acl->allow( - $viewerRoles, - [ - Api\Adapter\TeamResourceAdapter::class, - Api\Adapter\TeamResourceTemplateAdapter::class, - Api\Adapter\TeamRoleAdapter::class, - Api\Adapter\TeamUserAdapter::class, - ], - ['search', 'read'] - ); - - $globalSettings = $this->getServiceLocator()->get('Omeka\Settings'); - if (! $globalSettings->get('teams_site_admin_make_user')) { - $acl->deny( - 'site_admin', - 'Omeka\Entity\User', - ['create', 'delete', 'change-password', 'edit-keys'] - ); - - $acl->deny( - 'site_admin', - 'Omeka\Api\Adapter\UserAdapter', - ['create', 'delete'] - ); + $adapters = [ + 'Omeka\Api\Adapter\ItemAdapter', + 'Omeka\Api\Adapter\ItemSetAdapter', + 'Omeka\Api\Adapter\MediaAdapter', + 'Omeka\Api\Adapter\AssetAdapter', + ]; + $rolesToControl = ['site_admin', 'editor', 'author']; + + $privileges =[ + 'update', + 'delete', + 'create', + 'batch-delete', + 'batch-update', + 'batch-create', + 'batch_update', + 'batch_delete', + 'batch-edit-all', + 'batch-upate-all', + 'batch-edit', + 'batch-delete-all', + 'batch_delete_all', + ]; + foreach ($omekaResources as $resource) { + foreach ($rolesToControl as $role) { + foreach ( $privileges as $privilege) { + $acl->deny($role, $resource, $privilege,new AssertionNegation($inTeamAssertion)); + } + } } - - $acl->allow( - $viewerRoles, - [Api\Adapter\TeamRoleAdapter::class], - ['search', 'read'] - ); - $acl->allow( - $viewerRoles, - [Api\Adapter\TeamAdapter::class], - ['search', 'read', 'create', 'update', 'delete'] - ); - $acl->allow( - $viewerRoles, - [Controller\AddController::class], - ['show', 'browse', 'add', 'edit', 'delete', 'delete-confirm'] - ); - $acl->allow( - $viewerRoles, - [Controller\DeleteController::class], - ['show', 'browse', 'add', 'edit', 'deleteRole', 'delete-confirm'] - ); - $acl->allow( - $viewerRoles, - [Controller\IndexController::class], - ['show', 'browse', 'add', 'edit', 'delete', 'delete-confirm'] - ); - - $acl->allow( - $viewerRoles, - [Controller\UpdateController::class], - ['show', 'browse', 'add', 'edit', 'delete', 'delete-confirm'] - ); - $acl->allow( - $adminRoles, - [Controller\IndexController::class], - ['roleIndex'] - ); - $acl->allow( - $viewerRoles, - [Controller\IndexController::class], - ['roleIndex'] - ); +// --- Team specific controls. --- + $acl->allow(null, 'Teams\Controller\Index', ['index', 'teamDetail', 'currentTeam']); + $acl->allow('global_admin', [ + 'Teams\Controller\Index', + 'Teams\Controller\Add', + 'Teams\Controller\Update', + ]); + $acl->allow('global_admin', \Teams\Entity\TeamRole::class); $globalSettings = $this->getServiceLocator()->get('Omeka\Settings'); - if (! $globalSettings->get('teams_site_admin_make_site')) { - $acl->deny( - 'site_admin', - 'Omeka\Entity\Site', - 'create' - ); + if (!$globalSettings->get('teams_site_admin_make_site')) { + $acl->deny('site_admin', \Omeka\Entity\Site::class, 'create'); } - if (!$globalSettings->get('teams_editor_make_site')) { - $acl->deny( - 'editor', - 'Omeka\Entity\Site', - 'create' - ); + $acl->deny('editor', \Omeka\Entity\Site::class, 'create'); } - - $acl->deny( - ['site_admin', 'editor', 'reviewer', 'author', 'researcher'], - 'Teams\Controller\Trash', - 'update' - ); - - // Use InTeamAssertion for entity-level permissions - $assertion = $services->get(\Teams\Acl\InTeamAssertion::class); - - // Combine admin and viewer roles (excluding global_admin which is handled separately) - $rolesWithAssertion = array_merge($viewerRoles, ['site_admin']); - - // Apply the assertion to all entity resources for non-global-admin roles - $acl->allow( - $rolesWithAssertion, - [ - Entity\Team::class, - Entity\TeamUser::class, - Entity\TeamResource::class, - Entity\TeamRole::class, - Entity\TeamAsset::class, - Entity\TeamSite::class, - Entity\TeamResourceTemplate::class, - \Omeka\Entity\Item::class, - \Omeka\Entity\ItemSet::class, - \Omeka\Entity\Media::class, - \Omeka\Entity\Asset::class, - \Omeka\Entity\Site::class, - \Omeka\Entity\SitePage::class, - \Omeka\Entity\ResourceTemplate::class, - ], - $entityRights, - $assertion - ); - } - /** * @param Event $event * The default teams behavior is to filter API class, including the ones that populate the available sites in forms. @@ -2117,46 +2006,46 @@ public function getUser() * * @param Event $event */ - public function teamAuthorizeOnRead(Event $event) - { - $entity = $event->getParam('entity'); - $request = $event->getParam('request'); - $operation = $request->getOperation(); - - // Use ACL to check authorization - $acl = $this->getServiceLocator()->get('Omeka\Acl'); - $user = $this->getUser(); - $role = $user ? $user->getRole() : null; - - if (!$acl->isAllowed($role, $entity, $operation)) { - throw new Exception\PermissionDeniedException( - sprintf('Permission denied for operation "%s" on resource.', $operation) - ); - } - } +// public function teamAuthorizeOnRead(Event $event) +// { +// $entity = $event->getParam('entity'); +// $request = $event->getParam('request'); +// $operation = $request->getOperation(); +// +// // Use ACL to check authorization +// $acl = $this->getServiceLocator()->get('Omeka\Acl'); +// $user = $this->getUser(); +// $role = $user ? $user->getRole() : null; +// +// if (!$acl->isAllowed($role, $entity, $operation)) { +// throw new Exception\PermissionDeniedException( +// sprintf('Permission denied for operation "%s" on resource.', $operation) +// ); +// } +// } /** * Check team authorization when hydrating an entity via API * * @param Event $event */ - public function teamAuthorizeOnHydrate(Event $event) - { - $request = $event->getParam('request'); - $entity = $event->getParam('entity'); - $operation = $request->getOperation(); - - // Use ACL to check authorization - $acl = $this->getServiceLocator()->get('Omeka\Acl'); - $user = $this->getUser(); - $role = $user ? $user->getRole() : null; - - if (!$acl->isAllowed($role, $entity, $operation)) { - throw new Exception\PermissionDeniedException( - sprintf('Permission denied for operation "%s" on resource.', $operation) - ); - } - } +// public function teamAuthorizeOnHydrate(Event $event) +// { +// $request = $event->getParam('request'); +// $entity = $event->getParam('entity'); +// $operation = $request->getOperation(); +// +// // Use ACL to check authorization +// $acl = $this->getServiceLocator()->get('Omeka\Acl'); +// $user = $this->getUser(); +// $role = $user ? $user->getRole() : null; +// +// if (!$acl->isAllowed($role, $entity, $operation)) { +// throw new Exception\PermissionDeniedException( +// sprintf('Permission denied for operation "%s" on resource.', $operation) +// ); +// } +// } /** @@ -2267,6 +2156,14 @@ public function attachListeners(SharedEventManagerInterface $sharedEventManager) { $services = $this->getServiceLocator(); + // The key change: attach the listener with a low priority to run it last. + $sharedEventManager->attach( + 'Laminas\Mvc\Application', + MvcEvent::EVENT_DISPATCH, + [$this, 'addAclRulesOnDispatch'], + -1000 // Run after Omeka's default rules are established. + ); + $sharedEventManager->attach( 'Omeka\Controller\Admin\Item', 'view.advanced_search', @@ -2358,11 +2255,11 @@ public function attachListeners(SharedEventManagerInterface $sharedEventManager) ); // Use ACL assertion for authorization on API operations - $sharedEventManager->attach( - '*', - 'api.find.post', - [$this, 'teamAuthorizeOnRead'] - ); +// $sharedEventManager->attach( +// '*', +// 'api.find.post', +// [$this, 'teamAuthorizeOnRead'] +// ); $sharedEventManager->attach( 'Teams\Controller\Index', @@ -2371,11 +2268,11 @@ public function attachListeners(SharedEventManagerInterface $sharedEventManager) ); // Use ACL assertion for authorization on API operations - $sharedEventManager->attach( - '*', - 'api.hydrate.pre', - [$this, 'teamAuthorizeOnHydrate'] - ); +// $sharedEventManager->attach( +// '*', +// 'api.hydrate.pre', +// [$this, 'teamAuthorizeOnHydrate'] +// ); $sharedEventManager->attach( ItemSetAdapter::class, diff --git a/src/Acl/InTeamAssertion.php b/src/Acl/InTeamAssertion.php index 30a9a38..cd2b217 100644 --- a/src/Acl/InTeamAssertion.php +++ b/src/Acl/InTeamAssertion.php @@ -9,33 +9,35 @@ use Doctrine\ORM\EntityManager; use Omeka\Api\Exception; use Omeka\Entity\EntityInterface; +use Omeka\Mvc\Status; +use Teams\Entity\TeamUser; class InTeamAssertion implements AssertionInterface { /** * @var AuthenticationService */ - protected $auth; + protected AuthenticationService $auth; /** * @var EntityManager */ - protected $entityManager; + protected EntityManager $entityManager; /** - * @var \Omeka\Mvc\Status + * @var Status */ - protected $status; + protected Status $status; /** * @param AuthenticationService $auth * @param EntityManager $entityManager - * @param \Omeka\Mvc\Status $status + * @param Status $status */ public function __construct( AuthenticationService $auth, EntityManager $entityManager, - $status + Status $status ) { $this->auth = $auth; $this->entityManager = $entityManager; @@ -56,173 +58,117 @@ public function assert( RoleInterface $role = null, ResourceInterface $resource = null, $privilege = null - ) { - // If no resource is provided, we can't check team membership - if (!$resource instanceof EntityInterface) { - return true; - } + ): bool + { + $resourceDomains = [ + \Omeka\Entity\Item::class, \Omeka\Entity\ItemSet::class, \Omeka\Entity\Media::class, + \Omeka\Entity\Asset::class, \Omeka\Entity\ResourceTemplate::class, + \Teams\Entity\TeamResource::class, \Teams\Entity\TeamAsset::class, + ]; $user = $this->auth->getIdentity(); - /* - * First go through a couple of common cases where we don't need to judge permissions - */ - - // If the user isn't logged in (e.g., the public), use the default settings - if (!$user) { - return true; - } - - // If it isn't on the backend, let the public vs private rules take over - if ($this->status->isSiteRequest()) { - return true; + // The ACL rules in Module.php already handle bypass cases (global_admin). + // This assertion only runs for logged-in users on team-policed resources. + if (!$user || !$resource instanceof EntityInterface) { + return false; } - $is_glob_admin = ($user->getRole() === 'global_admin'); + $teamUser = $this->entityManager->getRepository(TeamUser::class) + ->findOneBy(['is_current' => true, 'user' => $user->getId()]); - // If it is the global admin, bypass any team controls - if ($is_glob_admin) { - return true; + if (!$teamUser) { + // User has no active team, so they cannot perform any team-restricted actions. + return false; } - $authorized = false; + $teamUserRole = $teamUser->getRole(); + $resClass = $this->getResourceClass($resource); - $res_class = $this->getResourceClass($resource); - - $user_id = $user->getId(); - $team_user = $this->entityManager->getRepository('Teams\Entity\TeamUser') - ->findOneBy(['is_current' => true, 'user' => $user_id]); - - // If user doesn't have a current team, deny access - if (!$team_user) { - throw new Exception\PermissionDeniedException( - 'Permission denied. User does not have a current team assignment.' - ); - } - - $team = $team_user->getTeam(); - $team_user_role = $team_user->getRole(); - - // Don't check if in same team if a create action, because items only assigned teams after creation - if ($privilege != 'create') { - // If resource not part of user's current team, no action at all - if (!$this->inTeam($resource, $team_user)) { - $err = sprintf( - 'Permission denied. Resource "%1$s: %2$s" is not part of your current team, %3$s. ' . - 'If you feel this is an error, try changing teams or talk to the administrator. ' . - 'Action: %4$s', - get_class($resource), - $resource->getId(), - $team->getName(), - $privilege - ); - throw new Exception\PermissionDeniedException($err); + // For 'create' actions, we only check if the user's role has permission, + // as the resource doesn't belong to a team yet. + if ($privilege == 'create') { + if ($resClass == \Omeka\Entity\Site::class || $resClass == \Omeka\Entity\SitePage::class) { + return (bool)$teamUserRole->getCanAddSitePages(); + } elseif (in_array($resClass,$resourceDomains)){ + return (bool)$teamUserRole->getCanAddItems(); + } else { + // Other resources are not part of this scope + return false; } } - $resource_domains = [ - 'Omeka\Entity\Item', - 'Omeka\Entity\ItemSet', - 'Omeka\Entity\Media', - 'Omeka\Entity\Asset', - 'Omeka\Entity\ResourceTemplate', - 'Teams\Entity\TeamResource', - 'Teams\Entity\TeamAsset', - ]; + // For all other actions, first check if the resource is in the user's current team. + if (!$this->isResourceInTeam($resource, $teamUser)) { + return false; + } - if (in_array($res_class, $resource_domains)) { - if ($privilege == 'create') { - $authorized = $team_user_role->getCanAddItems(); - } elseif ($privilege == 'delete' || $privilege == 'batch_delete') { - $authorized = $team_user_role->getCanDeleteResources(); - } elseif ($privilege == 'update') { - $authorized = $team_user_role->getCanModifyResources(); - } elseif ($privilege == 'read' || $privilege == 'search') { - $authorized = true; - } - } elseif ($res_class == 'Omeka\Entity\Site') { - if ($privilege == 'create') { - // Site creation permissions are handled separately in addAclRules - return true; - } elseif ($privilege == 'delete' || $privilege == 'batch_delete') { - $authorized = $team_user_role->getCanAddSitePages(); - } elseif ($privilege == 'update') { - $authorized = $team_user_role->getCanAddSitePages(); - } elseif ($privilege == 'read') { - $authorized = true; - } - } elseif ($res_class == 'Omeka\Entity\SitePage') { - if ($privilege == 'create') { - $authorized = $team_user_role->getCanAddSitePages(); - } elseif ($privilege == 'delete' || $privilege == 'batch_delete') { - $authorized = $team_user_role->getCanAddSitePages(); - } elseif ($privilege == 'update') { - $authorized = $team_user_role->getCanAddSitePages(); - } elseif ($privilege == 'read') { - $authorized = true; + // The resource is in the team. Now check if the team role grants the specific privilege. + $isAuthorized = false; + + if (in_array($resClass, $resourceDomains)) { + switch ($privilege) { + case 'delete': case 'batch_delete': + $isAuthorized = (bool)$teamUserRole->getCanDeleteResources(); + break; + case 'update': + $isAuthorized = (bool)$teamUserRole->getCanModifyResources(); + break; + case 'read': case 'search': + $isAuthorized = true; // If it's in the team, they can read it. + break; } - } elseif ($res_class == 'Teams\Entity\Team' || $res_class == 'Teams\Entity\TeamRole') { - if ($privilege == 'create') { - $authorized = $is_glob_admin; - } elseif ($privilege == 'delete' || $privilege == 'batch_delete') { - $authorized = $is_glob_admin; - } elseif ($privilege == 'update') { - $authorized = $team_user_role->getCanAddUsers(); - } elseif ($privilege == 'read') { - $authorized = true; + } elseif ($resClass == \Omeka\Entity\Site::class || $resClass == \Omeka\Entity\SitePage::class) { + switch ($privilege) { + case 'delete': case 'batch_delete': case 'update': + $isAuthorized = (bool)$teamUserRole->getCanAddSitePages(); + break; + case 'read': + $isAuthorized = true; + break; } - } elseif ($res_class == 'Teams\Entity\TeamSite') { - if ($privilege == 'read') { - $authorized = true; - } else { - // Adding or removing sites from a team - $authorized = $team_user_role->getCanAddSitePages(); + } elseif ($resClass == \Teams\Entity\Team::class || $resClass == \Teams\Entity\TeamRole::class) { + switch ($privilege) { + case 'create': case 'delete': case 'batch_delete': + $isAuthorized = false; // Only global admins can do this. + break; + case 'update': + $isAuthorized = (bool)$teamUserRole->getCanAddUsers(); + break; + case 'read': + $isAuthorized = true; + break; } - } elseif ($res_class == 'Teams\Entity\TeamUser') { - if ($privilege == 'read') { - $authorized = true; - } elseif ($privilege == 'create') { - $authorized = $team_user_role->getCanAddUsers(); - } elseif ($privilege == 'delete') { - $authorized = $team_user_role->getCanAddUsers(); - } else { - $authorized = $is_glob_admin; + } elseif ($resClass == \Teams\Entity\TeamSite::class) { + $isAuthorized = ($privilege == 'read') ? true : (bool)$teamUserRole->getCanAddSitePages(); + } elseif ($resClass == TeamUser::class) { + switch ($privilege) { + case 'create': case 'delete': + $isAuthorized = (bool)$teamUserRole->getCanAddUsers(); + break; + case 'read': + $isAuthorized = true; + break; + default: + $isAuthorized = false; } - } elseif ($res_class == 'Omeka\Entity\User') { - return true; - } elseif ($res_class == 'Omeka\Entity\Job') { - return true; - } elseif ($res_class == 'Omeka\Entity\Property') { - return true; - } elseif (strpos($res_class, 'Omeka\Entity') !== 0) { - // Don't police other modules by default (not an Omeka entity) - return true; - } - - if (!$authorized) { - $msg = sprintf( - 'Permission denied. Your role in %5$s, %4$s, does not permit you to %3$s this resource.', - get_class($resource), - $resource->getId(), - $privilege, - $team_user_role->getName(), - $team->getName() - ); - - throw new Exception\PermissionDeniedException($msg); + } else { + // If it's a resource we don't explicitly police (e.g., Job, Property), + // this assertion does not authorize it. + $isAuthorized = false; } - return $authorized; + return $isAuthorized; } /** * Check to see if the user and the object they are attempting to access or change are part of the same team. * * @param EntityInterface $resource - * @param \Teams\Entity\TeamUser $team_user + * @param TeamUser $team_user * @return bool */ - private function inTeam($resource, $team_user) + private function isResourceInTeam(EntityInterface $resource, TeamUser $team_user): bool { $resource_domains = ['Omeka\Entity\Item', 'Omeka\Entity\ItemSet', 'Omeka\Entity\Media']; $fk_id = $resource->getId(); @@ -263,9 +209,10 @@ private function inTeam($resource, $team_user) } else { /* * TeamRole is only accessible by global admin so doesn't need to be checked. - * Other classes should be controlled by their respective modules and components. + * Other classes should be controlled by their respective modules and components + * and should not be using this assertion. */ - return true; + return false; } if ($team_resource = $this->entityManager->getRepository($teamsRepo) @@ -284,7 +231,7 @@ private function inTeam($resource, $team_user) * @param mixed $resource * @return string */ - private function getResourceClass($resource) + private function getResourceClass($resource): string { $doctrine_ent = 'DoctrineProxies\__CG__'; $doctrine_test = strpos(get_class($resource), $doctrine_ent); From 1d660ecb8265e4a6ce5f33a72c00ca729005775a Mon Sep 17 00:00:00 2001 From: Alex Dryden Date: Mon, 24 Nov 2025 22:15:02 -0500 Subject: [PATCH 08/32] refactor: clean up funtions that can now be contolled via ACL --- Module.php | 75 ------------------------------------------------------ 1 file changed, 75 deletions(-) diff --git a/Module.php b/Module.php index e9e1597..cc64395 100644 --- a/Module.php +++ b/Module.php @@ -52,12 +52,6 @@ public function getConfig() public function onBootstrap(MvcEvent $event) { parent::onBootstrap($event); - } - /** - * The listener that calls our ACL setup. - */ - public function addAclRulesOnDispatch(MvcEvent $event) - { $this->addAclRules(); } @@ -2001,53 +1995,6 @@ public function getUser() } } - /** - * Check team authorization when reading an entity via API - * - * @param Event $event - */ -// public function teamAuthorizeOnRead(Event $event) -// { -// $entity = $event->getParam('entity'); -// $request = $event->getParam('request'); -// $operation = $request->getOperation(); -// -// // Use ACL to check authorization -// $acl = $this->getServiceLocator()->get('Omeka\Acl'); -// $user = $this->getUser(); -// $role = $user ? $user->getRole() : null; -// -// if (!$acl->isAllowed($role, $entity, $operation)) { -// throw new Exception\PermissionDeniedException( -// sprintf('Permission denied for operation "%s" on resource.', $operation) -// ); -// } -// } - - /** - * Check team authorization when hydrating an entity via API - * - * @param Event $event - */ -// public function teamAuthorizeOnHydrate(Event $event) -// { -// $request = $event->getParam('request'); -// $entity = $event->getParam('entity'); -// $operation = $request->getOperation(); -// -// // Use ACL to check authorization -// $acl = $this->getServiceLocator()->get('Omeka\Acl'); -// $user = $this->getUser(); -// $role = $user ? $user->getRole() : null; -// -// if (!$acl->isAllowed($role, $entity, $operation)) { -// throw new Exception\PermissionDeniedException( -// sprintf('Permission denied for operation "%s" on resource.', $operation) -// ); -// } -// } - - /** * Disable the auto-add field on the site edit form. Teams manages this by automatically adding items to the * appropriate team. @@ -2156,14 +2103,6 @@ public function attachListeners(SharedEventManagerInterface $sharedEventManager) { $services = $this->getServiceLocator(); - // The key change: attach the listener with a low priority to run it last. - $sharedEventManager->attach( - 'Laminas\Mvc\Application', - MvcEvent::EVENT_DISPATCH, - [$this, 'addAclRulesOnDispatch'], - -1000 // Run after Omeka's default rules are established. - ); - $sharedEventManager->attach( 'Omeka\Controller\Admin\Item', 'view.advanced_search', @@ -2254,26 +2193,12 @@ public function attachListeners(SharedEventManagerInterface $sharedEventManager) [$this, 'getOrphans'] ); - // Use ACL assertion for authorization on API operations -// $sharedEventManager->attach( -// '*', -// 'api.find.post', -// [$this, 'teamAuthorizeOnRead'] -// ); - $sharedEventManager->attach( 'Teams\Controller\Index', 'view.browse.before', [$this, 'teamSelectorBrowse'] ); - // Use ACL assertion for authorization on API operations -// $sharedEventManager->attach( -// '*', -// 'api.hydrate.pre', -// [$this, 'teamAuthorizeOnHydrate'] -// ); - $sharedEventManager->attach( ItemSetAdapter::class, 'api.execute.post', From 6e7e91ca614a346ecd0aaccfe7f7318fd11a8153 Mon Sep 17 00:00:00 2001 From: Alex Dryden Date: Tue, 25 Nov 2025 11:22:10 -0500 Subject: [PATCH 09/32] fix: not get class from ResourceInterface Class inspection is no longer needed when strictly using ResourceInterface and at times will give the wrong result --- src/Acl/InTeamAssertion.php | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/src/Acl/InTeamAssertion.php b/src/Acl/InTeamAssertion.php index cd2b217..e4f03a8 100644 --- a/src/Acl/InTeamAssertion.php +++ b/src/Acl/InTeamAssertion.php @@ -67,10 +67,7 @@ public function assert( ]; $user = $this->auth->getIdentity(); - - // The ACL rules in Module.php already handle bypass cases (global_admin). - // This assertion only runs for logged-in users on team-policed resources. - if (!$user || !$resource instanceof EntityInterface) { + if (!$user ) { return false; } @@ -83,14 +80,12 @@ public function assert( } $teamUserRole = $teamUser->getRole(); - $resClass = $this->getResourceClass($resource); - // For 'create' actions, we only check if the user's role has permission, // as the resource doesn't belong to a team yet. if ($privilege == 'create') { - if ($resClass == \Omeka\Entity\Site::class || $resClass == \Omeka\Entity\SitePage::class) { + if ($resource == \Omeka\Entity\Site::class || $resource == \Omeka\Entity\SitePage::class) { return (bool)$teamUserRole->getCanAddSitePages(); - } elseif (in_array($resClass,$resourceDomains)){ + } elseif (in_array($resource,$resourceDomains)){ return (bool)$teamUserRole->getCanAddItems(); } else { // Other resources are not part of this scope @@ -106,7 +101,7 @@ public function assert( // The resource is in the team. Now check if the team role grants the specific privilege. $isAuthorized = false; - if (in_array($resClass, $resourceDomains)) { + if (in_array($resource, $resourceDomains)) { switch ($privilege) { case 'delete': case 'batch_delete': $isAuthorized = (bool)$teamUserRole->getCanDeleteResources(); @@ -118,7 +113,7 @@ public function assert( $isAuthorized = true; // If it's in the team, they can read it. break; } - } elseif ($resClass == \Omeka\Entity\Site::class || $resClass == \Omeka\Entity\SitePage::class) { + } elseif ($resource == \Omeka\Entity\Site::class || $resource == \Omeka\Entity\SitePage::class) { switch ($privilege) { case 'delete': case 'batch_delete': case 'update': $isAuthorized = (bool)$teamUserRole->getCanAddSitePages(); @@ -127,7 +122,7 @@ public function assert( $isAuthorized = true; break; } - } elseif ($resClass == \Teams\Entity\Team::class || $resClass == \Teams\Entity\TeamRole::class) { + } elseif ($resource == \Teams\Entity\Team::class || $resource == \Teams\Entity\TeamRole::class) { switch ($privilege) { case 'create': case 'delete': case 'batch_delete': $isAuthorized = false; // Only global admins can do this. @@ -139,9 +134,9 @@ public function assert( $isAuthorized = true; break; } - } elseif ($resClass == \Teams\Entity\TeamSite::class) { + } elseif ($resource == \Teams\Entity\TeamSite::class) { $isAuthorized = ($privilege == 'read') ? true : (bool)$teamUserRole->getCanAddSitePages(); - } elseif ($resClass == TeamUser::class) { + } elseif ($resource == TeamUser::class) { switch ($privilege) { case 'create': case 'delete': $isAuthorized = (bool)$teamUserRole->getCanAddUsers(); From 0dcd49d4d8cf62bb6ff475c8ebd2a81a13f0c908 Mon Sep 17 00:00:00 2001 From: Alex Dryden Date: Tue, 25 Nov 2025 11:25:19 -0500 Subject: [PATCH 10/32] add SiteAdapter to list of adapters --- Module.php | 1 + 1 file changed, 1 insertion(+) diff --git a/Module.php b/Module.php index cc64395..68da858 100644 --- a/Module.php +++ b/Module.php @@ -266,6 +266,7 @@ protected function addAclRules() 'Omeka\Api\Adapter\ItemSetAdapter', 'Omeka\Api\Adapter\MediaAdapter', 'Omeka\Api\Adapter\AssetAdapter', + 'Omeka\Api\Adapter\SiteAdapter', ]; $rolesToControl = ['site_admin', 'editor', 'author']; From 835424d7588e4812c2d7f0896fbc738e5dc525ee Mon Sep 17 00:00:00 2001 From: Alex Dryden Date: Tue, 25 Nov 2025 13:02:51 -0500 Subject: [PATCH 11/32] fix: normalize GenericResource class test Generally it should work to just test classes from resouces but sites are being passed as Laminas\Permissions\Acl\Resource\GenericResource --- src/Acl/InTeamAssertion.php | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/src/Acl/InTeamAssertion.php b/src/Acl/InTeamAssertion.php index e4f03a8..11a0b33 100644 --- a/src/Acl/InTeamAssertion.php +++ b/src/Acl/InTeamAssertion.php @@ -83,9 +83,9 @@ public function assert( // For 'create' actions, we only check if the user's role has permission, // as the resource doesn't belong to a team yet. if ($privilege == 'create') { - if ($resource == \Omeka\Entity\Site::class || $resource == \Omeka\Entity\SitePage::class) { + if ($this->getResourceClass($resource) == \Omeka\Entity\Site::class || $this->getResourceClass($resource) == \Omeka\Entity\SitePage::class) { return (bool)$teamUserRole->getCanAddSitePages(); - } elseif (in_array($resource,$resourceDomains)){ + } elseif (in_array($this->getResourceClass($resource),$resourceDomains)){ return (bool)$teamUserRole->getCanAddItems(); } else { // Other resources are not part of this scope @@ -101,7 +101,7 @@ public function assert( // The resource is in the team. Now check if the team role grants the specific privilege. $isAuthorized = false; - if (in_array($resource, $resourceDomains)) { + if (in_array($this->getResourceClass($resource), $resourceDomains)) { switch ($privilege) { case 'delete': case 'batch_delete': $isAuthorized = (bool)$teamUserRole->getCanDeleteResources(); @@ -113,7 +113,7 @@ public function assert( $isAuthorized = true; // If it's in the team, they can read it. break; } - } elseif ($resource == \Omeka\Entity\Site::class || $resource == \Omeka\Entity\SitePage::class) { + } elseif ($this->getResourceClass($resource) == \Omeka\Entity\Site::class || $this->getResourceClass($resource) == \Omeka\Entity\SitePage::class) { switch ($privilege) { case 'delete': case 'batch_delete': case 'update': $isAuthorized = (bool)$teamUserRole->getCanAddSitePages(); @@ -122,7 +122,7 @@ public function assert( $isAuthorized = true; break; } - } elseif ($resource == \Teams\Entity\Team::class || $resource == \Teams\Entity\TeamRole::class) { + } elseif ($this->getResourceClass($resource) == \Teams\Entity\Team::class || $this->getResourceClass($resource) == \Teams\Entity\TeamRole::class) { switch ($privilege) { case 'create': case 'delete': case 'batch_delete': $isAuthorized = false; // Only global admins can do this. @@ -134,9 +134,9 @@ public function assert( $isAuthorized = true; break; } - } elseif ($resource == \Teams\Entity\TeamSite::class) { + } elseif ($this->getResourceClass($resource) == \Teams\Entity\TeamSite::class) { $isAuthorized = ($privilege == 'read') ? true : (bool)$teamUserRole->getCanAddSitePages(); - } elseif ($resource == TeamUser::class) { + } elseif ($this->getResourceClass($resource) == TeamUser::class) { switch ($privilege) { case 'create': case 'delete': $isAuthorized = (bool)$teamUserRole->getCanAddUsers(); @@ -148,6 +148,7 @@ public function assert( $isAuthorized = false; } } else { + // If it's a resource we don't explicitly police (e.g., Job, Property), // this assertion does not authorize it. $isAuthorized = false; @@ -159,11 +160,11 @@ public function assert( /** * Check to see if the user and the object they are attempting to access or change are part of the same team. * - * @param EntityInterface $resource + * @param ResourceInterface $resource * @param TeamUser $team_user * @return bool */ - private function isResourceInTeam(EntityInterface $resource, TeamUser $team_user): bool + private function isResourceInTeam( ResourceInterface $resource, TeamUser $team_user): bool { $resource_domains = ['Omeka\Entity\Item', 'Omeka\Entity\ItemSet', 'Omeka\Entity\Media']; $fk_id = $resource->getId(); @@ -228,14 +229,11 @@ private function isResourceInTeam(EntityInterface $resource, TeamUser $team_user */ private function getResourceClass($resource): string { - $doctrine_ent = 'DoctrineProxies\__CG__'; - $doctrine_test = strpos(get_class($resource), $doctrine_ent); - if ($doctrine_test === false) { - $res_class = get_class($resource); + if( get_class($resource) === 'Laminas\Permissions\Acl\Resource\GenericResource'){ + return $resource; } else { - // Skip the doctrine proxy prefix and the backslash separator - $res_class = substr(get_class($resource), strlen($doctrine_ent) + 1); + return get_class($resource); } - return $res_class; + } } From 814f805eec1308342d8f906d36e1f1dfde4b2a48 Mon Sep 17 00:00:00 2001 From: Alex Dryden Date: Tue, 25 Nov 2025 14:31:18 -0500 Subject: [PATCH 12/32] add acl handling for adapters that display some form buttons --- Module.php | 30 +++++++++++++++++++++++++----- src/Acl/InTeamAssertion.php | 10 +++++++++- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/Module.php b/Module.php index 68da858..4c88033 100644 --- a/Module.php +++ b/Module.php @@ -261,37 +261,57 @@ protected function addAclRules() \Teams\Entity\TeamResource::class, \Teams\Entity\TeamAsset::class, ]; + $adapters = [ 'Omeka\Api\Adapter\ItemAdapter', 'Omeka\Api\Adapter\ItemSetAdapter', 'Omeka\Api\Adapter\MediaAdapter', 'Omeka\Api\Adapter\AssetAdapter', 'Omeka\Api\Adapter\SiteAdapter', + 'Omeka\Api\Adapter\SitePageAdapter', + 'Omeka\Api\Adapter\ResourceTemplateAdapter', ]; + $rolesToControl = ['site_admin', 'editor', 'author']; - $privileges =[ + $entityPrivileges =[ 'update', 'delete', 'create', 'batch-delete', 'batch-update', 'batch-create', - 'batch_update', 'batch_delete', 'batch-edit-all', - 'batch-upate-all', + 'batch-update-all', 'batch-edit', 'batch-delete-all', - 'batch_delete_all', ]; + $adapterPrivileges = [ + 'create', + 'batch_delete', + 'batch_create', + 'batch_update', + 'batch_delete_all', + 'batch_update_all', + ]; foreach ($omekaResources as $resource) { foreach ($rolesToControl as $role) { - foreach ( $privileges as $privilege) { + foreach ( $entityPrivileges as $privilege) { $acl->deny($role, $resource, $privilege,new AssertionNegation($inTeamAssertion)); } } } + + //there are some display adapters that use this permission to show add/edit links + foreach ($adapters as $adapter) { + foreach ($rolesToControl as $role) { + foreach ( $adapterPrivileges as $privilege) { + $acl->deny($role, $adapter, $privilege, new AssertionNegation($inTeamAssertion)); + } + } + } + // --- Team specific controls. --- $acl->allow(null, 'Teams\Controller\Index', ['index', 'teamDetail', 'currentTeam']); $acl->allow('global_admin', [ diff --git a/src/Acl/InTeamAssertion.php b/src/Acl/InTeamAssertion.php index 11a0b33..8909103 100644 --- a/src/Acl/InTeamAssertion.php +++ b/src/Acl/InTeamAssertion.php @@ -63,7 +63,8 @@ public function assert( $resourceDomains = [ \Omeka\Entity\Item::class, \Omeka\Entity\ItemSet::class, \Omeka\Entity\Media::class, \Omeka\Entity\Asset::class, \Omeka\Entity\ResourceTemplate::class, - \Teams\Entity\TeamResource::class, \Teams\Entity\TeamAsset::class, + \Teams\Entity\TeamResource::class, \Teams\Entity\TeamAsset::class, \Omeka\Api\Adapter\ItemAdapter::class, + \Omeka\Api\Adapter\ItemSetAdapter::class, \Omeka\Api\Adapter\ResourceTemplateAdapter::class ]; $user = $this->auth->getIdentity(); @@ -93,6 +94,13 @@ public function assert( } } + if (in_array($privilege, ['batch_delete', 'batch_delete_all']) ) { + return (bool)$teamUserRole->getCanDeleteResources(); + } + if (in_array($privilege, ['batch_update', 'batch_update_all']) ) { + return (bool)$teamUserRole->getCanModifyResources(); + } + // For all other actions, first check if the resource is in the user's current team. if (!$this->isResourceInTeam($resource, $teamUser)) { return false; From a330545c2d171ce910ba822505e83987c03d65c0 Mon Sep 17 00:00:00 2001 From: Alex Dryden Date: Tue, 25 Nov 2025 14:32:47 -0500 Subject: [PATCH 13/32] ensure all roles but the global admin are added --- Module.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Module.php b/Module.php index 4c88033..bdb8e1c 100644 --- a/Module.php +++ b/Module.php @@ -272,7 +272,8 @@ protected function addAclRules() 'Omeka\Api\Adapter\ResourceTemplateAdapter', ]; - $rolesToControl = ['site_admin', 'editor', 'author']; + $rolesToControl = $acl->getRoles(); + $rolesToControl = array_diff($rolesToControl, ["global_admin"]); $entityPrivileges =[ 'update', From da2cfc2fa4ad16776be235f06c7109bb07f9140b Mon Sep 17 00:00:00 2001 From: Alex Dryden Date: Tue, 25 Nov 2025 15:22:39 -0500 Subject: [PATCH 14/32] refactor: rename acl asertion for team role authority --- Module.php | 8 ++++---- config/module.config.php | 2 +- ...nTeamAssertion.php => TeamRolePermissionAssertion.php} | 2 +- src/Service/Acl/InTeamAssertionFactory.php | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) rename src/Acl/{InTeamAssertion.php => TeamRolePermissionAssertion.php} (99%) diff --git a/Module.php b/Module.php index bdb8e1c..982c494 100644 --- a/Module.php +++ b/Module.php @@ -16,7 +16,7 @@ use Omeka\Permissions\Assertion\AssertionNegation; use Omeka\Permissions\Assertion\IsSelfAssertion; use Omeka\Permissions\Assertion\OwnsEntityAssertion; -use Teams\Acl\InTeamAssertion; +use Teams\Acl\TeamRolePermissionAssertion; use Teams\Entity\Team; use Teams\Entity\TeamAsset; use Teams\Entity\TeamResource; @@ -242,7 +242,7 @@ public function createNamedParameter( protected function addAclRules() { $acl = $this->getServiceLocator()->get('Omeka\Acl'); - $inTeamAssertion = $this->getServiceLocator()->get(\Teams\Acl\InTeamAssertion::class); + $teamRolePermissionAssertion = $this->getServiceLocator()->get(\Teams\Acl\TeamRolePermissionAssertion::class); $omekaResources = [ \Omeka\Entity\Item::class, @@ -299,7 +299,7 @@ protected function addAclRules() foreach ($omekaResources as $resource) { foreach ($rolesToControl as $role) { foreach ( $entityPrivileges as $privilege) { - $acl->deny($role, $resource, $privilege,new AssertionNegation($inTeamAssertion)); + $acl->deny($role, $resource, $privilege,new AssertionNegation($teamRolePermissionAssertion)); } } } @@ -308,7 +308,7 @@ protected function addAclRules() foreach ($adapters as $adapter) { foreach ($rolesToControl as $role) { foreach ( $adapterPrivileges as $privilege) { - $acl->deny($role, $adapter, $privilege, new AssertionNegation($inTeamAssertion)); + $acl->deny($role, $adapter, $privilege, new AssertionNegation($teamRolePermissionAssertion)); } } } diff --git a/config/module.config.php b/config/module.config.php index 3aa3ac4..60b1eb9 100644 --- a/config/module.config.php +++ b/config/module.config.php @@ -76,7 +76,7 @@ ], 'service_manager' => [ 'factories' => [ - \Teams\Acl\InTeamAssertion::class => \Teams\Service\Acl\InTeamAssertionFactory::class, + \Teams\Acl\TeamRolePermissionAssertion::class => \Teams\Service\Acl\InTeamAssertionFactory::class, ], ], 'form_elements' => [ diff --git a/src/Acl/InTeamAssertion.php b/src/Acl/TeamRolePermissionAssertion.php similarity index 99% rename from src/Acl/InTeamAssertion.php rename to src/Acl/TeamRolePermissionAssertion.php index 8909103..0560f40 100644 --- a/src/Acl/InTeamAssertion.php +++ b/src/Acl/TeamRolePermissionAssertion.php @@ -12,7 +12,7 @@ use Omeka\Mvc\Status; use Teams\Entity\TeamUser; -class InTeamAssertion implements AssertionInterface +class TeamRolePermissionAssertion implements AssertionInterface { /** * @var AuthenticationService diff --git a/src/Service/Acl/InTeamAssertionFactory.php b/src/Service/Acl/InTeamAssertionFactory.php index 59811ea..f3261e4 100644 --- a/src/Service/Acl/InTeamAssertionFactory.php +++ b/src/Service/Acl/InTeamAssertionFactory.php @@ -3,13 +3,13 @@ use Interop\Container\ContainerInterface; use Laminas\ServiceManager\Factory\FactoryInterface; -use Teams\Acl\InTeamAssertion; +use Teams\Acl\TeamRolePermissionAssertion; class InTeamAssertionFactory implements FactoryInterface { public function __invoke(ContainerInterface $services, $requestedName, array $options = null) { - return new InTeamAssertion( + return new TeamRolePermissionAssertion( $services->get('Omeka\AuthenticationService'), $services->get('Omeka\EntityManager'), $services->get('Omeka\Status') From 891859ce392701f0a9eedf366d076ac8ac2b35c8 Mon Sep 17 00:00:00 2001 From: Alex Dryden Date: Tue, 25 Nov 2025 15:27:10 -0500 Subject: [PATCH 15/32] fix: cleanup --- src/Acl/TeamRolePermissionAssertion.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Acl/TeamRolePermissionAssertion.php b/src/Acl/TeamRolePermissionAssertion.php index 0560f40..54c7070 100644 --- a/src/Acl/TeamRolePermissionAssertion.php +++ b/src/Acl/TeamRolePermissionAssertion.php @@ -218,9 +218,9 @@ private function isResourceInTeam( ResourceInterface $resource, TeamUser $team_u */ return false; } - - if ($team_resource = $this->entityManager->getRepository($teamsRepo) - ->findOneBy($criteria)) { + $team_resource = $this->entityManager->getRepository($teamsRepo) + ->findOneBy($criteria); + if ($team_resource) { $in_team = true; } else { $in_team = false; From d001086d5464bae2f1a51e85b718b6514034010d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 20:38:37 +0000 Subject: [PATCH 16/32] Code quality improvements: fix formatting, rename factory, add documentation Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com> --- Module.php | 2 +- config/module.config.php | 2 +- src/Acl/TeamRolePermissionAssertion.php | 87 +++++++++++-------- ...=> TeamRolePermissionAssertionFactory.php} | 2 +- 4 files changed, 53 insertions(+), 40 deletions(-) rename src/Service/Acl/{InTeamAssertionFactory.php => TeamRolePermissionAssertionFactory.php} (87%) diff --git a/Module.php b/Module.php index 982c494..a87f092 100644 --- a/Module.php +++ b/Module.php @@ -299,7 +299,7 @@ protected function addAclRules() foreach ($omekaResources as $resource) { foreach ($rolesToControl as $role) { foreach ( $entityPrivileges as $privilege) { - $acl->deny($role, $resource, $privilege,new AssertionNegation($teamRolePermissionAssertion)); + $acl->deny($role, $resource, $privilege, new AssertionNegation($teamRolePermissionAssertion)); } } } diff --git a/config/module.config.php b/config/module.config.php index 60b1eb9..c334bbb 100644 --- a/config/module.config.php +++ b/config/module.config.php @@ -76,7 +76,7 @@ ], 'service_manager' => [ 'factories' => [ - \Teams\Acl\TeamRolePermissionAssertion::class => \Teams\Service\Acl\InTeamAssertionFactory::class, + \Teams\Acl\TeamRolePermissionAssertion::class => \Teams\Service\Acl\TeamRolePermissionAssertionFactory::class, ], ], 'form_elements' => [ diff --git a/src/Acl/TeamRolePermissionAssertion.php b/src/Acl/TeamRolePermissionAssertion.php index 54c7070..0039e5a 100644 --- a/src/Acl/TeamRolePermissionAssertion.php +++ b/src/Acl/TeamRolePermissionAssertion.php @@ -12,6 +12,16 @@ use Omeka\Mvc\Status; use Teams\Entity\TeamUser; +/** + * ACL assertion for team-based authorization. + * + * This assertion enforces that users can only access resources that belong to their current team, + * and that their role within that team permits the requested action. It handles various resource + * types including Items, Sites, Media, and Team-specific entities. + * + * The assertion is used with AssertionNegation in ACL deny rules, creating a pattern where + * access is denied unless the assertion grants permission (double-negative pattern). + */ class TeamRolePermissionAssertion implements AssertionInterface { /** @@ -68,7 +78,7 @@ public function assert( ]; $user = $this->auth->getIdentity(); - if (!$user ) { + if (!$user) { return false; } @@ -81,12 +91,14 @@ public function assert( } $teamUserRole = $teamUser->getRole(); + $resourceClass = $this->getResourceClass($resource); + // For 'create' actions, we only check if the user's role has permission, // as the resource doesn't belong to a team yet. if ($privilege == 'create') { - if ($this->getResourceClass($resource) == \Omeka\Entity\Site::class || $this->getResourceClass($resource) == \Omeka\Entity\SitePage::class) { + if ($resourceClass == \Omeka\Entity\Site::class || $resourceClass == \Omeka\Entity\SitePage::class) { return (bool)$teamUserRole->getCanAddSitePages(); - } elseif (in_array($this->getResourceClass($resource),$resourceDomains)){ + } elseif (in_array($resourceClass, $resourceDomains)) { return (bool)$teamUserRole->getCanAddItems(); } else { // Other resources are not part of this scope @@ -94,10 +106,10 @@ public function assert( } } - if (in_array($privilege, ['batch_delete', 'batch_delete_all']) ) { + if (in_array($privilege, ['batch_delete', 'batch_delete_all'])) { return (bool)$teamUserRole->getCanDeleteResources(); } - if (in_array($privilege, ['batch_update', 'batch_update_all']) ) { + if (in_array($privilege, ['batch_update', 'batch_update_all'])) { return (bool)$teamUserRole->getCanModifyResources(); } @@ -109,32 +121,38 @@ public function assert( // The resource is in the team. Now check if the team role grants the specific privilege. $isAuthorized = false; - if (in_array($this->getResourceClass($resource), $resourceDomains)) { + if (in_array($resourceClass, $resourceDomains)) { switch ($privilege) { - case 'delete': case 'batch_delete': - $isAuthorized = (bool)$teamUserRole->getCanDeleteResources(); - break; + case 'delete': + case 'batch_delete': + $isAuthorized = (bool)$teamUserRole->getCanDeleteResources(); + break; case 'update': $isAuthorized = (bool)$teamUserRole->getCanModifyResources(); - break; - case 'read': case 'search': + break; + case 'read': + case 'search': $isAuthorized = true; // If it's in the team, they can read it. - break; + break; } - } elseif ($this->getResourceClass($resource) == \Omeka\Entity\Site::class || $this->getResourceClass($resource) == \Omeka\Entity\SitePage::class) { + } elseif ($resourceClass == \Omeka\Entity\Site::class || $resourceClass == \Omeka\Entity\SitePage::class) { switch ($privilege) { - case 'delete': case 'batch_delete': case 'update': - $isAuthorized = (bool)$teamUserRole->getCanAddSitePages(); - break; + case 'delete': + case 'batch_delete': + case 'update': + $isAuthorized = (bool)$teamUserRole->getCanAddSitePages(); + break; case 'read': $isAuthorized = true; break; } - } elseif ($this->getResourceClass($resource) == \Teams\Entity\Team::class || $this->getResourceClass($resource) == \Teams\Entity\TeamRole::class) { + } elseif ($resourceClass == \Teams\Entity\Team::class || $resourceClass == \Teams\Entity\TeamRole::class) { switch ($privilege) { - case 'create': case 'delete': case 'batch_delete': - $isAuthorized = false; // Only global admins can do this. - break; + case 'create': + case 'delete': + case 'batch_delete': + $isAuthorized = false; // Only global admins can do this. + break; case 'update': $isAuthorized = (bool)$teamUserRole->getCanAddUsers(); break; @@ -142,13 +160,14 @@ public function assert( $isAuthorized = true; break; } - } elseif ($this->getResourceClass($resource) == \Teams\Entity\TeamSite::class) { + } elseif ($resourceClass == \Teams\Entity\TeamSite::class) { $isAuthorized = ($privilege == 'read') ? true : (bool)$teamUserRole->getCanAddSitePages(); - } elseif ($this->getResourceClass($resource) == TeamUser::class) { + } elseif ($resourceClass == TeamUser::class) { switch ($privilege) { - case 'create': case 'delete': - $isAuthorized = (bool)$teamUserRole->getCanAddUsers(); - break; + case 'create': + case 'delete': + $isAuthorized = (bool)$teamUserRole->getCanAddUsers(); + break; case 'read': $isAuthorized = true; break; @@ -172,7 +191,7 @@ public function assert( * @param TeamUser $team_user * @return bool */ - private function isResourceInTeam( ResourceInterface $resource, TeamUser $team_user): bool + private function isResourceInTeam(ResourceInterface $resource, TeamUser $team_user): bool { $resource_domains = ['Omeka\Entity\Item', 'Omeka\Entity\ItemSet', 'Omeka\Entity\Media']; $fk_id = $resource->getId(); @@ -220,28 +239,22 @@ private function isResourceInTeam( ResourceInterface $resource, TeamUser $team_u } $team_resource = $this->entityManager->getRepository($teamsRepo) ->findOneBy($criteria); - if ($team_resource) { - $in_team = true; - } else { - $in_team = false; - } - - return $in_team; + + return (bool)$team_resource; } /** * Returns the expected string for proxied resource class in cases where the class returned is the doctrine proxy. * * @param mixed $resource - * @return string + * @return string|ResourceInterface */ - private function getResourceClass($resource): string + private function getResourceClass($resource) { - if( get_class($resource) === 'Laminas\Permissions\Acl\Resource\GenericResource'){ + if (get_class($resource) === 'Laminas\Permissions\Acl\Resource\GenericResource') { return $resource; } else { return get_class($resource); } - } } diff --git a/src/Service/Acl/InTeamAssertionFactory.php b/src/Service/Acl/TeamRolePermissionAssertionFactory.php similarity index 87% rename from src/Service/Acl/InTeamAssertionFactory.php rename to src/Service/Acl/TeamRolePermissionAssertionFactory.php index f3261e4..546c684 100644 --- a/src/Service/Acl/InTeamAssertionFactory.php +++ b/src/Service/Acl/TeamRolePermissionAssertionFactory.php @@ -5,7 +5,7 @@ use Laminas\ServiceManager\Factory\FactoryInterface; use Teams\Acl\TeamRolePermissionAssertion; -class InTeamAssertionFactory implements FactoryInterface +class TeamRolePermissionAssertionFactory implements FactoryInterface { public function __invoke(ContainerInterface $services, $requestedName, array $options = null) { From f391b977f55d67e905ae43b5b76952e4a93533b4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 20:40:17 +0000 Subject: [PATCH 17/32] Improve documentation for getResourceClass method Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com> --- src/Acl/TeamRolePermissionAssertion.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Acl/TeamRolePermissionAssertion.php b/src/Acl/TeamRolePermissionAssertion.php index 0039e5a..ae7abea 100644 --- a/src/Acl/TeamRolePermissionAssertion.php +++ b/src/Acl/TeamRolePermissionAssertion.php @@ -244,10 +244,13 @@ private function isResourceInTeam(ResourceInterface $resource, TeamUser $team_us } /** - * Returns the expected string for proxied resource class in cases where the class returned is the doctrine proxy. + * Returns the class name of a resource, handling Doctrine proxies and GenericResource. + * + * For GenericResource instances, returns the resource itself (as it contains the actual resource). + * For other resources, returns the fully qualified class name as a string. * * @param mixed $resource - * @return string|ResourceInterface + * @return string|object The class name as string, or the GenericResource object */ private function getResourceClass($resource) { From 07641a5e668066d45a9c4d6b941bb96d8d1973ee Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 20:55:45 +0000 Subject: [PATCH 18/32] Refactor getResourceClass to use ResourceInterface::getResourceId() Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com> --- src/Acl/TeamRolePermissionAssertion.php | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/Acl/TeamRolePermissionAssertion.php b/src/Acl/TeamRolePermissionAssertion.php index ae7abea..3b5df7e 100644 --- a/src/Acl/TeamRolePermissionAssertion.php +++ b/src/Acl/TeamRolePermissionAssertion.php @@ -244,20 +244,23 @@ private function isResourceInTeam(ResourceInterface $resource, TeamUser $team_us } /** - * Returns the class name of a resource, handling Doctrine proxies and GenericResource. + * Returns the class name of a resource, handling both entity instances and ACL resources. * - * For GenericResource instances, returns the resource itself (as it contains the actual resource). - * For other resources, returns the fully qualified class name as a string. + * When the resource implements ResourceInterface (e.g., GenericResource), uses getResourceId() + * to get the resource identifier (typically a class name). + * For entity instances, returns the fully qualified class name. * * @param mixed $resource - * @return string|object The class name as string, or the GenericResource object + * @return string The class name or resource identifier */ - private function getResourceClass($resource) + private function getResourceClass($resource): string { - if (get_class($resource) === 'Laminas\Permissions\Acl\Resource\GenericResource') { - return $resource; - } else { - return get_class($resource); + // If it's an ACL resource (like GenericResource), use its resource ID + if ($resource instanceof ResourceInterface) { + return $resource->getResourceId(); } + + // Otherwise, get the class name directly + return get_class($resource); } } From 9864ae50827f8ae6fb7b78baa66b457f3dab7f7e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 20:58:01 +0000 Subject: [PATCH 19/32] Add GenericResource check and fix type hints for isResourceInTeam Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com> --- src/Acl/TeamRolePermissionAssertion.php | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/Acl/TeamRolePermissionAssertion.php b/src/Acl/TeamRolePermissionAssertion.php index 3b5df7e..9e5388f 100644 --- a/src/Acl/TeamRolePermissionAssertion.php +++ b/src/Acl/TeamRolePermissionAssertion.php @@ -114,6 +114,12 @@ public function assert( } // For all other actions, first check if the resource is in the user's current team. + // If the resource is just a GenericResource (only has class name, not entity instance), + // we can't verify team membership, so deny access. + if ($resource instanceof \Laminas\Permissions\Acl\Resource\GenericResource) { + return false; + } + if (!$this->isResourceInTeam($resource, $teamUser)) { return false; } @@ -186,12 +192,15 @@ public function assert( /** * Check to see if the user and the object they are attempting to access or change are part of the same team. + * + * This method requires an actual entity instance (not just a ResourceInterface) because it needs + * to access entity-specific methods like getId() and getSite(). * - * @param ResourceInterface $resource + * @param object $resource The entity instance to check * @param TeamUser $team_user * @return bool */ - private function isResourceInTeam(ResourceInterface $resource, TeamUser $team_user): bool + private function isResourceInTeam(object $resource, TeamUser $team_user): bool { $resource_domains = ['Omeka\Entity\Item', 'Omeka\Entity\ItemSet', 'Omeka\Entity\Media']; $fk_id = $resource->getId(); From 43307342e961f71c0257d7865ffefb58d362e9ba Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 20:59:59 +0000 Subject: [PATCH 20/32] Import GenericResource and improve type safety in isResourceInTeam Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com> --- src/Acl/TeamRolePermissionAssertion.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Acl/TeamRolePermissionAssertion.php b/src/Acl/TeamRolePermissionAssertion.php index 9e5388f..808fce6 100644 --- a/src/Acl/TeamRolePermissionAssertion.php +++ b/src/Acl/TeamRolePermissionAssertion.php @@ -4,6 +4,7 @@ use Laminas\Permissions\Acl\Acl; use Laminas\Permissions\Acl\Assertion\AssertionInterface; use Laminas\Permissions\Acl\Resource\ResourceInterface; +use Laminas\Permissions\Acl\Resource\GenericResource; use Laminas\Permissions\Acl\Role\RoleInterface; use Laminas\Authentication\AuthenticationService; use Doctrine\ORM\EntityManager; @@ -116,7 +117,7 @@ public function assert( // For all other actions, first check if the resource is in the user's current team. // If the resource is just a GenericResource (only has class name, not entity instance), // we can't verify team membership, so deny access. - if ($resource instanceof \Laminas\Permissions\Acl\Resource\GenericResource) { + if ($resource instanceof GenericResource) { return false; } @@ -193,10 +194,10 @@ public function assert( /** * Check to see if the user and the object they are attempting to access or change are part of the same team. * - * This method requires an actual entity instance (not just a ResourceInterface) because it needs + * This method requires an actual entity instance (not a GenericResource) because it needs * to access entity-specific methods like getId() and getSite(). * - * @param object $resource The entity instance to check + * @param EntityInterface|object $resource The entity instance to check * @param TeamUser $team_user * @return bool */ From 934f31aa596558050c83b6e4d67b78bb8fec2c0e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 21:01:51 +0000 Subject: [PATCH 21/32] Final cleanup: import GenericResource, improve documentation and type safety Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com> --- src/Acl/TeamRolePermissionAssertion.php | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/Acl/TeamRolePermissionAssertion.php b/src/Acl/TeamRolePermissionAssertion.php index 808fce6..418aefb 100644 --- a/src/Acl/TeamRolePermissionAssertion.php +++ b/src/Acl/TeamRolePermissionAssertion.php @@ -115,8 +115,10 @@ public function assert( } // For all other actions, first check if the resource is in the user's current team. - // If the resource is just a GenericResource (only has class name, not entity instance), - // we can't verify team membership, so deny access. + // If the resource is a GenericResource (only has class identifier, not entity instance), + // we cannot verify team membership. Since team membership is required for these operations, + // we deny access as a conservative security measure. ACL checks with actual entity instances + // will properly evaluate team membership. if ($resource instanceof GenericResource) { return false; } @@ -195,13 +197,14 @@ public function assert( * Check to see if the user and the object they are attempting to access or change are part of the same team. * * This method requires an actual entity instance (not a GenericResource) because it needs - * to access entity-specific methods like getId() and getSite(). + * to access entity-specific methods like getId() and getSite(). The caller should verify + * the resource is not a GenericResource before calling this method. * - * @param EntityInterface|object $resource The entity instance to check + * @param EntityInterface $resource The entity instance to check * @param TeamUser $team_user * @return bool */ - private function isResourceInTeam(object $resource, TeamUser $team_user): bool + private function isResourceInTeam(EntityInterface $resource, TeamUser $team_user): bool { $resource_domains = ['Omeka\Entity\Item', 'Omeka\Entity\ItemSet', 'Omeka\Entity\Media']; $fk_id = $resource->getId(); From ffcf350783859b141b43abfa9c0f50dca44989bd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 21:03:32 +0000 Subject: [PATCH 22/32] Improve documentation clarity for GenericResource handling and type expectations Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com> --- src/Acl/TeamRolePermissionAssertion.php | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/Acl/TeamRolePermissionAssertion.php b/src/Acl/TeamRolePermissionAssertion.php index 418aefb..32ed372 100644 --- a/src/Acl/TeamRolePermissionAssertion.php +++ b/src/Acl/TeamRolePermissionAssertion.php @@ -114,10 +114,11 @@ public function assert( return (bool)$teamUserRole->getCanModifyResources(); } - // For all other actions, first check if the resource is in the user's current team. - // If the resource is a GenericResource (only has class identifier, not entity instance), - // we cannot verify team membership. Since team membership is required for these operations, - // we deny access as a conservative security measure. ACL checks with actual entity instances + // For all other actions (update, delete, read, etc.), first check if the resource + // is in the user's current team. If the resource is a GenericResource (only has + // class identifier, not entity instance), we cannot verify team membership. + // Since team membership is required for non-create operations, we deny access + // as a conservative security measure. ACL checks with actual entity instances // will properly evaluate team membership. if ($resource instanceof GenericResource) { return false; @@ -194,15 +195,15 @@ public function assert( } /** - * Check to see if the user and the object they are attempting to access or change are part of the same team. + * Check if a resource belongs to the user's current team. * - * This method requires an actual entity instance (not a GenericResource) because it needs - * to access entity-specific methods like getId() and getSite(). The caller should verify - * the resource is not a GenericResource before calling this method. + * Requires an entity instance (implementing EntityInterface) because it needs to access + * entity-specific methods like getId(), getSite(), getTeam(), and getResource(). + * The caller should filter out GenericResource instances before calling this method. * - * @param EntityInterface $resource The entity instance to check - * @param TeamUser $team_user - * @return bool + * @param EntityInterface $resource The entity instance to check (e.g., Item, Site, TeamResource) + * @param TeamUser $team_user The user's team membership record + * @return bool True if the resource belongs to the user's team, false otherwise */ private function isResourceInTeam(EntityInterface $resource, TeamUser $team_user): bool { From 19600aff5e440ec5e45e815b417710e3df53f958 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 21:05:15 +0000 Subject: [PATCH 23/32] Add EntityInterface type guard for type safety Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com> --- src/Acl/TeamRolePermissionAssertion.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Acl/TeamRolePermissionAssertion.php b/src/Acl/TeamRolePermissionAssertion.php index 32ed372..8a53ee4 100644 --- a/src/Acl/TeamRolePermissionAssertion.php +++ b/src/Acl/TeamRolePermissionAssertion.php @@ -124,6 +124,11 @@ public function assert( return false; } + // Ensure we have an actual entity instance for team membership check + if (!$resource instanceof EntityInterface) { + return false; + } + if (!$this->isResourceInTeam($resource, $teamUser)) { return false; } From fdb74ebf69b7bd8c2a935630b77816f29db9bfc4 Mon Sep 17 00:00:00 2001 From: Alex Dryden Date: Tue, 25 Nov 2025 16:08:36 -0500 Subject: [PATCH 24/32] add documentation --- src/Acl/TeamRolePermissionAssertion.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Acl/TeamRolePermissionAssertion.php b/src/Acl/TeamRolePermissionAssertion.php index 8a53ee4..d9ba534 100644 --- a/src/Acl/TeamRolePermissionAssertion.php +++ b/src/Acl/TeamRolePermissionAssertion.php @@ -107,6 +107,8 @@ public function assert( } } + //In practice, batch operations are handled as a series of individual operations, + //but the batch privileges are checked first and used to control certain form controls if (in_array($privilege, ['batch_delete', 'batch_delete_all'])) { return (bool)$teamUserRole->getCanDeleteResources(); } From 4463b3697a9be8ad80bf578f91bbd5bea1f7eb95 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 21:13:32 +0000 Subject: [PATCH 25/32] Move type guards into isResourceInTeam for better encapsulation Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com> --- src/Acl/TeamRolePermissionAssertion.php | 38 ++++++++++++------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/Acl/TeamRolePermissionAssertion.php b/src/Acl/TeamRolePermissionAssertion.php index d9ba534..c7e5a65 100644 --- a/src/Acl/TeamRolePermissionAssertion.php +++ b/src/Acl/TeamRolePermissionAssertion.php @@ -116,21 +116,7 @@ public function assert( return (bool)$teamUserRole->getCanModifyResources(); } - // For all other actions (update, delete, read, etc.), first check if the resource - // is in the user's current team. If the resource is a GenericResource (only has - // class identifier, not entity instance), we cannot verify team membership. - // Since team membership is required for non-create operations, we deny access - // as a conservative security measure. ACL checks with actual entity instances - // will properly evaluate team membership. - if ($resource instanceof GenericResource) { - return false; - } - - // Ensure we have an actual entity instance for team membership check - if (!$resource instanceof EntityInterface) { - return false; - } - + // For all other actions, check if the resource is in the user's current team if (!$this->isResourceInTeam($resource, $teamUser)) { return false; } @@ -204,16 +190,30 @@ public function assert( /** * Check if a resource belongs to the user's current team. * - * Requires an entity instance (implementing EntityInterface) because it needs to access + * Returns false if: + * - Resource is a GenericResource (only has class identifier, no entity data) + * - Resource is not an EntityInterface (cannot access entity methods) + * - Resource is not found in the team's resource associations + * + * Requires an entity instance to verify team membership because it needs to access * entity-specific methods like getId(), getSite(), getTeam(), and getResource(). - * The caller should filter out GenericResource instances before calling this method. * - * @param EntityInterface $resource The entity instance to check (e.g., Item, Site, TeamResource) + * @param ResourceInterface|null $resource The resource to check * @param TeamUser $team_user The user's team membership record * @return bool True if the resource belongs to the user's team, false otherwise */ - private function isResourceInTeam(EntityInterface $resource, TeamUser $team_user): bool + private function isResourceInTeam($resource, TeamUser $team_user): bool { + // Can't verify team membership without an actual entity instance + if ($resource instanceof GenericResource) { + return false; + } + + // Ensure we have an entity instance with required methods + if (!$resource instanceof EntityInterface) { + return false; + } + $resource_domains = ['Omeka\Entity\Item', 'Omeka\Entity\ItemSet', 'Omeka\Entity\Media']; $fk_id = $resource->getId(); $team = $team_user->getTeam(); From 166f3f342c61bd9e176f741c93b33b32f56f7171 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 21:15:21 +0000 Subject: [PATCH 26/32] Add explicit mixed type declaration to isResourceInTeam parameter Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com> --- src/Acl/TeamRolePermissionAssertion.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Acl/TeamRolePermissionAssertion.php b/src/Acl/TeamRolePermissionAssertion.php index c7e5a65..e0ecafa 100644 --- a/src/Acl/TeamRolePermissionAssertion.php +++ b/src/Acl/TeamRolePermissionAssertion.php @@ -198,11 +198,11 @@ public function assert( * Requires an entity instance to verify team membership because it needs to access * entity-specific methods like getId(), getSite(), getTeam(), and getResource(). * - * @param ResourceInterface|null $resource The resource to check + * @param mixed $resource The resource to check (accepts ResourceInterface or entity instances) * @param TeamUser $team_user The user's team membership record * @return bool True if the resource belongs to the user's team, false otherwise */ - private function isResourceInTeam($resource, TeamUser $team_user): bool + private function isResourceInTeam(mixed $resource, TeamUser $team_user): bool { // Can't verify team membership without an actual entity instance if ($resource instanceof GenericResource) { From 754e8d35e4a492a889c8882144e7f0aa4bbda15f Mon Sep 17 00:00:00 2001 From: Alex Dryden Date: Tue, 25 Nov 2025 16:23:22 -0500 Subject: [PATCH 27/32] fix: add site adapters to permissions that should pass site gate Users where getting back a Forbidden message when submitting a new site form because site adapter permissions were getting flagged as false. --- src/Acl/TeamRolePermissionAssertion.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Acl/TeamRolePermissionAssertion.php b/src/Acl/TeamRolePermissionAssertion.php index d9ba534..2973b15 100644 --- a/src/Acl/TeamRolePermissionAssertion.php +++ b/src/Acl/TeamRolePermissionAssertion.php @@ -77,6 +77,10 @@ public function assert( \Teams\Entity\TeamResource::class, \Teams\Entity\TeamAsset::class, \Omeka\Api\Adapter\ItemAdapter::class, \Omeka\Api\Adapter\ItemSetAdapter::class, \Omeka\Api\Adapter\ResourceTemplateAdapter::class ]; + $siteDomains = [ + \Omeka\Entity\Site::class, \Omeka\Entity\SitePage::class, + \Omeka\Api\Adapter\SiteAdapter::class, \Omeka\Api\Adapter\SitePageAdapter::class + ]; $user = $this->auth->getIdentity(); if (!$user) { @@ -97,7 +101,7 @@ public function assert( // For 'create' actions, we only check if the user's role has permission, // as the resource doesn't belong to a team yet. if ($privilege == 'create') { - if ($resourceClass == \Omeka\Entity\Site::class || $resourceClass == \Omeka\Entity\SitePage::class) { + if (in_array($resourceClass,$siteDomains)) { return (bool)$teamUserRole->getCanAddSitePages(); } elseif (in_array($resourceClass, $resourceDomains)) { return (bool)$teamUserRole->getCanAddItems(); From 27c37c9703b64427275426366b365cef17406c34 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 21:43:14 +0000 Subject: [PATCH 28/32] Use public constants for resource domains to ensure Module.php and assertion stay synchronized Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com> --- Module.php | 33 +++++++---------- src/Acl/TeamRolePermissionAssertion.php | 49 +++++++++++++++++-------- 2 files changed, 48 insertions(+), 34 deletions(-) diff --git a/Module.php b/Module.php index a87f092..02edb43 100644 --- a/Module.php +++ b/Module.php @@ -244,15 +244,11 @@ protected function addAclRules() $acl = $this->getServiceLocator()->get('Omeka\Acl'); $teamRolePermissionAssertion = $this->getServiceLocator()->get(\Teams\Acl\TeamRolePermissionAssertion::class); - $omekaResources = [ - \Omeka\Entity\Item::class, - \Omeka\Entity\ItemSet::class, - \Omeka\Entity\Media::class, - \Omeka\Entity\Site::class, - \Omeka\Entity\SitePage::class, - \Omeka\Entity\ResourceTemplate::class, - \Omeka\Entity\Asset::class, - ]; + // Use constants from assertion class to ensure synchronization between ACL rules and assertion logic + $allResourceDomains = array_merge( + \Teams\Acl\TeamRolePermissionAssertion::ITEM_RESOURCE_DOMAINS, + \Teams\Acl\TeamRolePermissionAssertion::SITE_RESOURCE_DOMAINS + ); $teamResources = [ \Teams\Entity\Team::class, @@ -262,15 +258,14 @@ protected function addAclRules() \Teams\Entity\TeamAsset::class, ]; - $adapters = [ - 'Omeka\Api\Adapter\ItemAdapter', - 'Omeka\Api\Adapter\ItemSetAdapter', - 'Omeka\Api\Adapter\MediaAdapter', - 'Omeka\Api\Adapter\AssetAdapter', - 'Omeka\Api\Adapter\SiteAdapter', - 'Omeka\Api\Adapter\SitePageAdapter', - 'Omeka\Api\Adapter\ResourceTemplateAdapter', - ]; + // Extract entities and adapters from the unified resource list + $entities = array_filter($allResourceDomains, function($resource) { + return strpos($resource, 'Entity') !== false; + }); + + $adapters = array_filter($allResourceDomains, function($resource) { + return strpos($resource, 'Adapter') !== false; + }); $rolesToControl = $acl->getRoles(); $rolesToControl = array_diff($rolesToControl, ["global_admin"]); @@ -296,7 +291,7 @@ protected function addAclRules() 'batch_delete_all', 'batch_update_all', ]; - foreach ($omekaResources as $resource) { + foreach ($entities as $resource) { foreach ($rolesToControl as $role) { foreach ( $entityPrivileges as $privilege) { $acl->deny($role, $resource, $privilege, new AssertionNegation($teamRolePermissionAssertion)); diff --git a/src/Acl/TeamRolePermissionAssertion.php b/src/Acl/TeamRolePermissionAssertion.php index b6a9641..30dcb11 100644 --- a/src/Acl/TeamRolePermissionAssertion.php +++ b/src/Acl/TeamRolePermissionAssertion.php @@ -25,6 +25,36 @@ */ class TeamRolePermissionAssertion implements AssertionInterface { + /** + * Resource domains that are controlled by item/resource permissions. + * These constants ensure synchronization between ACL rules and assertion logic. + */ + public const ITEM_RESOURCE_DOMAINS = [ + \Omeka\Entity\Item::class, + \Omeka\Entity\ItemSet::class, + \Omeka\Entity\Media::class, + \Omeka\Entity\Asset::class, + \Omeka\Entity\ResourceTemplate::class, + \Teams\Entity\TeamResource::class, + \Teams\Entity\TeamAsset::class, + \Omeka\Api\Adapter\ItemAdapter::class, + \Omeka\Api\Adapter\ItemSetAdapter::class, + \Omeka\Api\Adapter\MediaAdapter::class, + \Omeka\Api\Adapter\AssetAdapter::class, + \Omeka\Api\Adapter\ResourceTemplateAdapter::class, + ]; + + /** + * Site-related resource domains. + * These constants ensure synchronization between ACL rules and assertion logic. + */ + public const SITE_RESOURCE_DOMAINS = [ + \Omeka\Entity\Site::class, + \Omeka\Entity\SitePage::class, + \Omeka\Api\Adapter\SiteAdapter::class, + \Omeka\Api\Adapter\SitePageAdapter::class, + ]; + /** * @var AuthenticationService */ @@ -71,17 +101,6 @@ public function assert( $privilege = null ): bool { - $resourceDomains = [ - \Omeka\Entity\Item::class, \Omeka\Entity\ItemSet::class, \Omeka\Entity\Media::class, - \Omeka\Entity\Asset::class, \Omeka\Entity\ResourceTemplate::class, - \Teams\Entity\TeamResource::class, \Teams\Entity\TeamAsset::class, \Omeka\Api\Adapter\ItemAdapter::class, - \Omeka\Api\Adapter\ItemSetAdapter::class, \Omeka\Api\Adapter\ResourceTemplateAdapter::class - ]; - $siteDomains = [ - \Omeka\Entity\Site::class, \Omeka\Entity\SitePage::class, - \Omeka\Api\Adapter\SiteAdapter::class, \Omeka\Api\Adapter\SitePageAdapter::class - ]; - $user = $this->auth->getIdentity(); if (!$user) { return false; @@ -101,9 +120,9 @@ public function assert( // For 'create' actions, we only check if the user's role has permission, // as the resource doesn't belong to a team yet. if ($privilege == 'create') { - if (in_array($resourceClass,$siteDomains)) { + if (in_array($resourceClass, self::SITE_RESOURCE_DOMAINS)) { return (bool)$teamUserRole->getCanAddSitePages(); - } elseif (in_array($resourceClass, $resourceDomains)) { + } elseif (in_array($resourceClass, self::ITEM_RESOURCE_DOMAINS)) { return (bool)$teamUserRole->getCanAddItems(); } else { // Other resources are not part of this scope @@ -128,7 +147,7 @@ public function assert( // The resource is in the team. Now check if the team role grants the specific privilege. $isAuthorized = false; - if (in_array($resourceClass, $resourceDomains)) { + if (in_array($resourceClass, self::ITEM_RESOURCE_DOMAINS)) { switch ($privilege) { case 'delete': case 'batch_delete': @@ -142,7 +161,7 @@ public function assert( $isAuthorized = true; // If it's in the team, they can read it. break; } - } elseif ($resourceClass == \Omeka\Entity\Site::class || $resourceClass == \Omeka\Entity\SitePage::class) { + } elseif (in_array($resourceClass, self::SITE_RESOURCE_DOMAINS)) { switch ($privilege) { case 'delete': case 'batch_delete': From 57bbb598c33eb65fac6d5b60b4dda5f336d8b59e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 21:45:37 +0000 Subject: [PATCH 29/32] Add ITEM_ENTITIES constant for clearer separation in isResourceInTeam Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com> --- src/Acl/TeamRolePermissionAssertion.php | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/Acl/TeamRolePermissionAssertion.php b/src/Acl/TeamRolePermissionAssertion.php index 30dcb11..49d3c3e 100644 --- a/src/Acl/TeamRolePermissionAssertion.php +++ b/src/Acl/TeamRolePermissionAssertion.php @@ -25,6 +25,15 @@ */ class TeamRolePermissionAssertion implements AssertionInterface { + /** + * Item-like entities that map to TeamResource + */ + private const ITEM_ENTITIES = [ + \Omeka\Entity\Item::class, + \Omeka\Entity\ItemSet::class, + \Omeka\Entity\Media::class, + ]; + /** * Resource domains that are controlled by item/resource permissions. * These constants ensure synchronization between ACL rules and assertion logic. @@ -237,13 +246,12 @@ private function isResourceInTeam(mixed $resource, TeamUser $team_user): bool return false; } - $resource_domains = ['Omeka\Entity\Item', 'Omeka\Entity\ItemSet', 'Omeka\Entity\Media']; $fk_id = $resource->getId(); $team = $team_user->getTeam(); $user = $team_user->getUser(); $res_class = $this->getResourceClass($resource); - if (in_array($res_class, $resource_domains)) { + if (in_array($res_class, self::ITEM_ENTITIES)) { $teamsRepo = 'Teams\Entity\TeamResource'; $fk = 'resource'; $criteria = ['team' => $team->getId(), $fk => $fk_id]; From 1cc75a90970da0008441a9dda8319c8b559a21dc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 21:49:51 +0000 Subject: [PATCH 30/32] Use separate entity/adapter constants to eliminate fragile string filtering Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com> --- Module.php | 20 ++++----- src/Acl/TeamRolePermissionAssertion.php | 58 ++++++++++++++++++++++--- 2 files changed, 59 insertions(+), 19 deletions(-) diff --git a/Module.php b/Module.php index 02edb43..a1d2cb3 100644 --- a/Module.php +++ b/Module.php @@ -245,9 +245,14 @@ protected function addAclRules() $teamRolePermissionAssertion = $this->getServiceLocator()->get(\Teams\Acl\TeamRolePermissionAssertion::class); // Use constants from assertion class to ensure synchronization between ACL rules and assertion logic - $allResourceDomains = array_merge( - \Teams\Acl\TeamRolePermissionAssertion::ITEM_RESOURCE_DOMAINS, - \Teams\Acl\TeamRolePermissionAssertion::SITE_RESOURCE_DOMAINS + $entities = array_merge( + \Teams\Acl\TeamRolePermissionAssertion::ITEM_ENTITIES_FOR_ACL, + \Teams\Acl\TeamRolePermissionAssertion::SITE_ENTITIES_FOR_ACL + ); + + $adapters = array_merge( + \Teams\Acl\TeamRolePermissionAssertion::ITEM_ADAPTERS_FOR_ACL, + \Teams\Acl\TeamRolePermissionAssertion::SITE_ADAPTERS_FOR_ACL ); $teamResources = [ @@ -258,15 +263,6 @@ protected function addAclRules() \Teams\Entity\TeamAsset::class, ]; - // Extract entities and adapters from the unified resource list - $entities = array_filter($allResourceDomains, function($resource) { - return strpos($resource, 'Entity') !== false; - }); - - $adapters = array_filter($allResourceDomains, function($resource) { - return strpos($resource, 'Adapter') !== false; - }); - $rolesToControl = $acl->getRoles(); $rolesToControl = array_diff($rolesToControl, ["global_admin"]); diff --git a/src/Acl/TeamRolePermissionAssertion.php b/src/Acl/TeamRolePermissionAssertion.php index 49d3c3e..6ae887e 100644 --- a/src/Acl/TeamRolePermissionAssertion.php +++ b/src/Acl/TeamRolePermissionAssertion.php @@ -26,7 +26,7 @@ class TeamRolePermissionAssertion implements AssertionInterface { /** - * Item-like entities that map to TeamResource + * Item-like entities that map to TeamResource repository */ private const ITEM_ENTITIES = [ \Omeka\Entity\Item::class, @@ -35,10 +35,36 @@ class TeamRolePermissionAssertion implements AssertionInterface ]; /** - * Resource domains that are controlled by item/resource permissions. - * These constants ensure synchronization between ACL rules and assertion logic. + * Entity classes controlled by item/resource permissions. + * Used by Module.php for configuring entity-level ACL rules. */ - public const ITEM_RESOURCE_DOMAINS = [ + public const ITEM_ENTITIES_FOR_ACL = [ + \Omeka\Entity\Item::class, + \Omeka\Entity\ItemSet::class, + \Omeka\Entity\Media::class, + \Omeka\Entity\Asset::class, + \Omeka\Entity\ResourceTemplate::class, + \Teams\Entity\TeamResource::class, + \Teams\Entity\TeamAsset::class, + ]; + + /** + * API adapter classes controlled by item/resource permissions. + * Used by Module.php for configuring adapter-level ACL rules. + */ + public const ITEM_ADAPTERS_FOR_ACL = [ + \Omeka\Api\Adapter\ItemAdapter::class, + \Omeka\Api\Adapter\ItemSetAdapter::class, + \Omeka\Api\Adapter\MediaAdapter::class, + \Omeka\Api\Adapter\AssetAdapter::class, + \Omeka\Api\Adapter\ResourceTemplateAdapter::class, + ]; + + /** + * All item/resource domains (entities + adapters combined). + * Used internally by assert() method. + */ + private const ITEM_RESOURCE_DOMAINS = [ \Omeka\Entity\Item::class, \Omeka\Entity\ItemSet::class, \Omeka\Entity\Media::class, @@ -54,10 +80,28 @@ class TeamRolePermissionAssertion implements AssertionInterface ]; /** - * Site-related resource domains. - * These constants ensure synchronization between ACL rules and assertion logic. + * Site entity classes controlled by site permissions. + * Used by Module.php for configuring entity-level ACL rules. + */ + public const SITE_ENTITIES_FOR_ACL = [ + \Omeka\Entity\Site::class, + \Omeka\Entity\SitePage::class, + ]; + + /** + * Site API adapter classes controlled by site permissions. + * Used by Module.php for configuring adapter-level ACL rules. + */ + public const SITE_ADAPTERS_FOR_ACL = [ + \Omeka\Api\Adapter\SiteAdapter::class, + \Omeka\Api\Adapter\SitePageAdapter::class, + ]; + + /** + * All site-related domains (entities + adapters combined). + * Used internally by assert() method. */ - public const SITE_RESOURCE_DOMAINS = [ + private const SITE_RESOURCE_DOMAINS = [ \Omeka\Entity\Site::class, \Omeka\Entity\SitePage::class, \Omeka\Api\Adapter\SiteAdapter::class, From 1f2d21ce87793ce5397dd9f144febb4ff2255aaf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 21:52:21 +0000 Subject: [PATCH 31/32] Eliminate constant duplication by computing combined domains dynamically Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com> --- src/Acl/TeamRolePermissionAssertion.php | 44 +++++-------------------- 1 file changed, 9 insertions(+), 35 deletions(-) diff --git a/src/Acl/TeamRolePermissionAssertion.php b/src/Acl/TeamRolePermissionAssertion.php index 6ae887e..0c27c72 100644 --- a/src/Acl/TeamRolePermissionAssertion.php +++ b/src/Acl/TeamRolePermissionAssertion.php @@ -26,7 +26,7 @@ class TeamRolePermissionAssertion implements AssertionInterface { /** - * Item-like entities that map to TeamResource repository + * Item-like entities that map to TeamResource repository (Item, ItemSet, Media only) */ private const ITEM_ENTITIES = [ \Omeka\Entity\Item::class, @@ -60,25 +60,6 @@ class TeamRolePermissionAssertion implements AssertionInterface \Omeka\Api\Adapter\ResourceTemplateAdapter::class, ]; - /** - * All item/resource domains (entities + adapters combined). - * Used internally by assert() method. - */ - private const ITEM_RESOURCE_DOMAINS = [ - \Omeka\Entity\Item::class, - \Omeka\Entity\ItemSet::class, - \Omeka\Entity\Media::class, - \Omeka\Entity\Asset::class, - \Omeka\Entity\ResourceTemplate::class, - \Teams\Entity\TeamResource::class, - \Teams\Entity\TeamAsset::class, - \Omeka\Api\Adapter\ItemAdapter::class, - \Omeka\Api\Adapter\ItemSetAdapter::class, - \Omeka\Api\Adapter\MediaAdapter::class, - \Omeka\Api\Adapter\AssetAdapter::class, - \Omeka\Api\Adapter\ResourceTemplateAdapter::class, - ]; - /** * Site entity classes controlled by site permissions. * Used by Module.php for configuring entity-level ACL rules. @@ -96,17 +77,6 @@ class TeamRolePermissionAssertion implements AssertionInterface \Omeka\Api\Adapter\SiteAdapter::class, \Omeka\Api\Adapter\SitePageAdapter::class, ]; - - /** - * All site-related domains (entities + adapters combined). - * Used internally by assert() method. - */ - private const SITE_RESOURCE_DOMAINS = [ - \Omeka\Entity\Site::class, - \Omeka\Entity\SitePage::class, - \Omeka\Api\Adapter\SiteAdapter::class, - \Omeka\Api\Adapter\SitePageAdapter::class, - ]; /** * @var AuthenticationService @@ -170,12 +140,16 @@ public function assert( $teamUserRole = $teamUser->getRole(); $resourceClass = $this->getResourceClass($resource); + // Get combined resource domains dynamically + $itemResourceDomains = array_merge(self::ITEM_ENTITIES_FOR_ACL, self::ITEM_ADAPTERS_FOR_ACL); + $siteResourceDomains = array_merge(self::SITE_ENTITIES_FOR_ACL, self::SITE_ADAPTERS_FOR_ACL); + // For 'create' actions, we only check if the user's role has permission, // as the resource doesn't belong to a team yet. if ($privilege == 'create') { - if (in_array($resourceClass, self::SITE_RESOURCE_DOMAINS)) { + if (in_array($resourceClass, $siteResourceDomains)) { return (bool)$teamUserRole->getCanAddSitePages(); - } elseif (in_array($resourceClass, self::ITEM_RESOURCE_DOMAINS)) { + } elseif (in_array($resourceClass, $itemResourceDomains)) { return (bool)$teamUserRole->getCanAddItems(); } else { // Other resources are not part of this scope @@ -200,7 +174,7 @@ public function assert( // The resource is in the team. Now check if the team role grants the specific privilege. $isAuthorized = false; - if (in_array($resourceClass, self::ITEM_RESOURCE_DOMAINS)) { + if (in_array($resourceClass, $itemResourceDomains)) { switch ($privilege) { case 'delete': case 'batch_delete': @@ -214,7 +188,7 @@ public function assert( $isAuthorized = true; // If it's in the team, they can read it. break; } - } elseif (in_array($resourceClass, self::SITE_RESOURCE_DOMAINS)) { + } elseif (in_array($resourceClass, $siteResourceDomains)) { switch ($privilege) { case 'delete': case 'batch_delete': From ad97d5d30fa09005d366f6e09cdd5e9d05d32f95 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 22:07:45 +0000 Subject: [PATCH 32/32] Rename ITEM constants to RESOURCE for domain clarity Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com> --- Module.php | 4 ++-- src/Acl/TeamRolePermissionAssertion.php | 17 +++++++++-------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/Module.php b/Module.php index a1d2cb3..3d8117c 100644 --- a/Module.php +++ b/Module.php @@ -246,12 +246,12 @@ protected function addAclRules() // Use constants from assertion class to ensure synchronization between ACL rules and assertion logic $entities = array_merge( - \Teams\Acl\TeamRolePermissionAssertion::ITEM_ENTITIES_FOR_ACL, + \Teams\Acl\TeamRolePermissionAssertion::RESOURCE_ENTITIES_FOR_ACL, \Teams\Acl\TeamRolePermissionAssertion::SITE_ENTITIES_FOR_ACL ); $adapters = array_merge( - \Teams\Acl\TeamRolePermissionAssertion::ITEM_ADAPTERS_FOR_ACL, + \Teams\Acl\TeamRolePermissionAssertion::RESOURCE_ADAPTERS_FOR_ACL, \Teams\Acl\TeamRolePermissionAssertion::SITE_ADAPTERS_FOR_ACL ); diff --git a/src/Acl/TeamRolePermissionAssertion.php b/src/Acl/TeamRolePermissionAssertion.php index 0c27c72..54941ef 100644 --- a/src/Acl/TeamRolePermissionAssertion.php +++ b/src/Acl/TeamRolePermissionAssertion.php @@ -26,7 +26,7 @@ class TeamRolePermissionAssertion implements AssertionInterface { /** - * Item-like entities that map to TeamResource repository (Item, ItemSet, Media only) + * Item-like entities (Item, ItemSet, Media) that map to TeamResource repository */ private const ITEM_ENTITIES = [ \Omeka\Entity\Item::class, @@ -35,10 +35,11 @@ class TeamRolePermissionAssertion implements AssertionInterface ]; /** - * Entity classes controlled by item/resource permissions. + * Resource entity classes controlled by resource permissions. + * Includes Items, ItemSets, Media, Assets, ResourceTemplates, and team resource entities. * Used by Module.php for configuring entity-level ACL rules. */ - public const ITEM_ENTITIES_FOR_ACL = [ + public const RESOURCE_ENTITIES_FOR_ACL = [ \Omeka\Entity\Item::class, \Omeka\Entity\ItemSet::class, \Omeka\Entity\Media::class, @@ -49,10 +50,10 @@ class TeamRolePermissionAssertion implements AssertionInterface ]; /** - * API adapter classes controlled by item/resource permissions. + * Resource API adapter classes controlled by resource permissions. * Used by Module.php for configuring adapter-level ACL rules. */ - public const ITEM_ADAPTERS_FOR_ACL = [ + public const RESOURCE_ADAPTERS_FOR_ACL = [ \Omeka\Api\Adapter\ItemAdapter::class, \Omeka\Api\Adapter\ItemSetAdapter::class, \Omeka\Api\Adapter\MediaAdapter::class, @@ -141,7 +142,7 @@ public function assert( $resourceClass = $this->getResourceClass($resource); // Get combined resource domains dynamically - $itemResourceDomains = array_merge(self::ITEM_ENTITIES_FOR_ACL, self::ITEM_ADAPTERS_FOR_ACL); + $resourceDomains = array_merge(self::RESOURCE_ENTITIES_FOR_ACL, self::RESOURCE_ADAPTERS_FOR_ACL); $siteResourceDomains = array_merge(self::SITE_ENTITIES_FOR_ACL, self::SITE_ADAPTERS_FOR_ACL); // For 'create' actions, we only check if the user's role has permission, @@ -149,7 +150,7 @@ public function assert( if ($privilege == 'create') { if (in_array($resourceClass, $siteResourceDomains)) { return (bool)$teamUserRole->getCanAddSitePages(); - } elseif (in_array($resourceClass, $itemResourceDomains)) { + } elseif (in_array($resourceClass, $resourceDomains)) { return (bool)$teamUserRole->getCanAddItems(); } else { // Other resources are not part of this scope @@ -174,7 +175,7 @@ public function assert( // The resource is in the team. Now check if the team role grants the specific privilege. $isAuthorized = false; - if (in_array($resourceClass, $itemResourceDomains)) { + if (in_array($resourceClass, $resourceDomains)) { switch ($privilege) { case 'delete': case 'batch_delete':