ACP2E-277: treat missing rules from authorization_rule table as deny permission rules; fix static errors / warnings

Author: alzota

app/code/Magento/Authorization/Model/Acl/Loader/Rule.php

@@ -21,7 +21,7 @@ class Rule implements LoaderInterface
     /**
      * Rules array cache key
      */
-    const ACL_RULE_CACHE_KEY = 'authorization_rule_cached_data';
+    public const ACL_RULE_CACHE_KEY = 'authorization_rule_cached_data';
 
     /**
      * @var ResourceConnection
@@ -79,10 +79,19 @@ public function __construct(
      * @return void
      */
     public function populateAcl(\Magento\Framework\Acl $acl)
+    {
+        $result = $this->applyPermissionsAccordingToRules($acl);
+        $this->applyDenyPermissionsForMissingRules($acl, ...$result);
+    }
+
+    /**
+     * @param \Magento\Framework\Acl $acl
+     * @return array[]
+     */
+    private function applyPermissionsAccordingToRules(\Magento\Framework\Acl $acl): array
     {
         $foundResources = [];
         $foundRoles = [];
-
         foreach ($this->getRulesArray() as $rule) {
             $role = $rule['role_id'];
             $resource = $rule['resource_id'];
@@ -101,15 +110,25 @@ public function populateAcl(\Magento\Framework\Acl $acl)
                 }
             }
         }
+        return [$foundResources, $foundRoles];
+    }
 
-        /**
-         * for all rules that were not regenerated in authorization_rule table,
-         * when adding a new module and without re-saving all roles,7
-         * consider not present rules with deny permissions
-         * */
+    /**
+     *
+     * For all rules that were not regenerated in authorization_rule table,
+     * when adding a new module and without re-saving all roles,
+     * consider not present rules with deny permissions
+     *
+     * @param \Magento\Framework\Acl $acl
+     * @param array $resources
+     * @param array $roles
+     * @return void
+     */
+    private function applyDenyPermissionsForMissingRules(\Magento\Framework\Acl $acl, array $resources, array $roles)
+    {
         foreach ($acl->getResources() as $resource) {
-            if (!isset($foundResources[$resource])) {
-                foreach ($foundRoles as $role) {
+            if (!isset($resources[$resource])) {
+                foreach ($roles as $role) {
                     $acl->deny($role, $resource, null);
                 }
             }

app/code/Magento/Authorization/Test/Unit/Model/Acl/Loader/RuleTest.php

@@ -97,6 +97,11 @@ static function ($value) {
      */
     public function testPopulateAclFromCache(): void
     {
+        $rules = [
+            ['role_id' => 1, 'resource_id' => 'Magento_Backend::all', 'permission' => 'allow'],
+            ['role_id' => 2, 'resource_id' => 1, 'permission' => 'allow'],
+            ['role_id' => 3, 'resource_id' => 1, 'permission' => 'deny']
+        ];
         $this->resourceMock->expects($this->never())->method('getTable');
         $this->resourceMock->expects($this->never())
             ->method('getConnection');
@@ -105,13 +110,7 @@ public function testPopulateAclFromCache(): void
             ->method('load')
             ->with(Rule::ACL_RULE_CACHE_KEY)
             ->willReturn(
-                json_encode(
-                    [
-                        ['role_id' => 1, 'resource_id' => 'Magento_Backend::all', 'permission' => 'allow'],
-                        ['role_id' => 2, 'resource_id' => 1, 'permission' => 'allow'],
-                        ['role_id' => 3, 'resource_id' => 1, 'permission' => 'deny']
-                    ]
-                )
+                json_encode($rules)
             );
 
         $aclMock = $this->createMock(Acl::class);
@@ -123,9 +122,21 @@ public function testPopulateAclFromCache(): void
                 ['1', 'Magento_Backend::all', null],
                 ['2', 1, null]
             );
+
+        $aclMock
+            ->method('getResources')
+            ->willReturn([
+                'Vendor_MyModule::menu'
+            ]);
+
         $aclMock
             ->method('deny')
-            ->with('3', 1, null);
+            ->withConsecutive(
+                ['3', 1, null],
+                ['1', 'Vendor_MyModule::menu', null],
+                ['2', 'Vendor_MyModule::menu', null],
+                ['3', 'Vendor_MyModule::menu', null]
+            );
 
         $this->model->populateAcl($aclMock);
     }