Refactor ACL logic into AclRuleManager service

[Copilot]

Extracts assertion-based ACL deny rules from Module::addAclRules() into a dedicated, testable service class to improve separation of concerns.

Note: This PR is rebased on the develop branch and builds upon the merged work from PR #190 (copilot/refactor-teams-module-authorization), which introduced the full TeamRolePermissionAssertion implementation with real authorization logic.

Changes

• New service: AclRuleManager applies assertion-based deny rules for Omeka resources and adapters across controlled roles (excluding global_admin)

• Uses existing assertion: Leverages the TeamRolePermissionAssertion from PR Refactor Teams authorization to use Laminas ACL assertions #190 with full team-based authorization logic (not a placeholder)

• Service registration: Added AclRuleManager to service_manager configuration in module.config.php with factory using ::class constants

• Simplified Module.php: addAclRules() now delegates ACL deny rule configuration to AclRuleManager::applyRules(), reducing the method from ~80 lines to ~27 lines while retaining team-specific controller permissions and site creation controls

• Test coverage: Unit tests verify ACL deny rules override previously allowed permissions and that global_admin role is excluded from restrictions

Before

protected function addAclRules() {
    $acl = $this->getServiceLocator()->get('Omeka\Acl');
    $teamRolePermissionAssertion = $this->getServiceLocator()->get(\Teams\Acl\TeamRolePermissionAssertion::class);
    
    // ~80 lines of entity/adapter privilege configuration with nested loops
    foreach ($entities as $resource) {
        foreach ($rolesToControl as $role) {
            foreach ($entityPrivileges as $privilege) {
                $acl->deny($role, $resource, $privilege, new AssertionNegation($teamRolePermissionAssertion));
            }
        }
    }
    // ... more ACL configuration
}

After

protected function addAclRules() {
    $serviceLocator = $this->getServiceLocator();
    $acl = $serviceLocator->get('Omeka\Acl');
    
    // Delegate ACL deny rules to our new, testable service
    $aclRuleManager = $serviceLocator->get(\Teams\Service\AclRuleManager::class);
    $aclRuleManager->applyRules($acl);
    
    // Remaining team-specific ACL rules...
}

The extracted logic uses constants from TeamRolePermissionAssertion to ensure synchronization between ACL rules and assertion logic, and wraps the assertion in AssertionNegation to deny privileges conditionally based on team permissions.

Original prompt

Hello Copilot,

Please open a new pull request against the development branch of the UIUCLibrary/teams repository with the title "Refactor ACL logic into a new AclRuleManager service".

The goal of this pull request is to refactor the Access Control List (ACL) logic out of Module.php into a new, dedicated, and testable service class. This improves code quality by separating concerns.

The pull request should contain the following exact changes:

1. Create a new service class AclRuleManager

• File Path: src/Service/AclRuleManager.php
• Content:

  <?php
  namespace Teams\Service;
  
  use Laminas\Permissions\Acl\Acl;
  use Teams\Acl\TeamRolePermissionAssertion;
  use Omeka\Permissions\Assertion\AssertionNegation;
  
  class AclRuleManager
  {
      /**
       * @var TeamRolePermissionAssertion
       */
      private $assertion;
  
      public function __construct(TeamRolePermissionAssertion $assertion)
      {
          $this->assertion = $assertion;
      }
  
      public function applyRules(Acl $acl)
      {
          $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,
          ];
          
          $rolesToControl = ['site_admin', 'editor', 'author'];
  
          $privilegesToControl =[
              'update', 'edit',
              'delete', 'delete-confirm',
              'create', 'add',
              'batch-delete', 'batch_delete', 'batch_delete_all',
              'batch-update', 'batch_update_all',
              'batch-edit', 'batch-edit-all',
          ];
          
          $denyAssertion = new AssertionNegation($this->assertion);
  
          foreach ($rolesToControl as $role) {
              $acl->deny($role, $omekaResources, $privilegesToControl, $denyAssertion);
          }
      }
  }

2. Create a factory for the new service

• File Path: src/Service/AclRuleManagerFactory.php
• Content:

  <?php
  namespace Teams\Service;
  
  use Interop\Container\ContainerInterface;
  use Laminas\ServiceManager\Factory\FactoryInterface;
  use Teams\Acl\TeamRolePermissionAssertion;
  
  class AclRuleManagerFactory implements FactoryInterface
  {
      public function __invoke(ContainerInterface $container, $requestedName, array $options = null)
      {
          $assertion = $container->get(TeamRolePermissionAssertion::class);
          return new AclRuleManager($assertion);
      }
  }

3. Create a new unit test for the service

• File Path: tests/Service/AclRuleManagerTest.php
• Content:

  <?php
  namespace Teams\Tests\Service;
  
  use Teams\Service\AclRuleManager;
  use Teams\Acl\TeamRolePermissionAssertion;
  use Laminas\Permissions\Acl\Acl;
  use Laminas\Permissions\Acl\Role\GenericRole;
  use Omeka\Entity\Item;
  use Tests\TestCase;
  
  class AclRuleManagerTest extends TestCase
  {
      /** @var AclRuleManager */
      private $ruleManager;
  
      /** @var TeamRolePermissionAssertion|\PHPUnit\Framework\MockObject\MockObject */
      private $assertionMock;
  
      public function setUp(): void
      {
          parent::setUp();
  
          $this->assertionMock = $this->createMock(TeamRolePermissionAssertion::class);
          $this->ruleManager = new AclRuleManager($this->assertionMock);
      }
  
      public function testApplyRulesDeniesPreviouslyAllowedPermission()
      {
          // Arrange
          $acl = new Acl;
          $role = new GenericRole('editor');
          $resource = new Item;
          $acl->addRole($role);
          $acl->addResource($resource);
          $acl->allow($role, $resource, 'update');
          $this->assertTrue($acl->isAllowed($role, $resource, 'update'));
          
          $this->assertionMock->method('assert')->willReturn(false);
  
          // Act
          $this->ruleManager->applyRules($acl);
  
          // Assert
          $this->assertFalse($acl->isAllowed($role, $resource, 'update'));
      }
  }

4. Update the module configuration

• File Path: config/module.config.php
• Change: Add the new AclRuleManagerFactory to the service_manager.factories array.
• New Content Snippet:

  // ...
  'service_manager' => [
      // ...
      'factories' => [
          'Teams\Acl\TeamRolePermissionAssertion' => 'Teams\Acl\TeamRolePermissionAssertionFactory',
          'Teams\Service\AclRuleManager' => 'Teams\Service\AclRuleManagerFactory', // <-- Add this line
          'Teams\Service\TeamService' => 'Teams\Service\TeamServiceFactory',
      ],
  ],
  // ...

...

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[alexdryden]

@copilot I think I jumped the gun with this request, lets get copilot/refactor-teams-module-authorization merged in first. This work needs to build on that work.

Okay, that is merged. Can you revise your work by reevaluating the promt with the current state of the develop branch?

[Copilot]

@copilot I think I jumped the gun with this request, lets get copilot/refactor-teams-module-authorization merged in first. This work needs to build on that work.

Okay, that is merged. Can you revise...

Done! I've rebased the work on the current develop branch which includes the merged copilot/refactor-teams-module-authorization PR.

The implementation now:

• Uses the existing TeamRolePermissionAssertion with full authorization logic from PR Refactor Teams authorization to use Laminas ACL assertions #190
• Extracts the ACL deny rules from Module::addAclRules() into the new AclRuleManager service
• Simplifies Module.php from ~80 lines down to ~12 lines for ACL configuration
• Includes unit tests with two test cases

Commit: 16eaadd