Merge branch 'ACP2E-277' of https://github.com/magento-l3/magento2ce into PR-2022-03-11

Author: Anna Bukatar

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

@@ -7,6 +7,7 @@
 
 namespace Magento\Authorization\Model\Acl\Loader;
 
+use Magento\Framework\Acl;
 use Magento\Framework\Acl\Data\CacheInterface;
 use Magento\Framework\Acl\LoaderInterface;
 use Magento\Framework\Acl\RootResource;
@@ -21,7 +22,12 @@ 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';
+
+    /**
+     * Allow everything resource id
+     */
+    private const ALLOW_EVERYTHING = 'Magento_Backend::all';
 
     /**
      * @var ResourceConnection
@@ -75,27 +81,74 @@ public function __construct(
     /**
      * Populate ACL with rules from external storage
      *
-     * @param \Magento\Framework\Acl $acl
+     * @param Acl $acl
      * @return void
      */
-    public function populateAcl(\Magento\Framework\Acl $acl)
+    public function populateAcl(Acl $acl)
     {
+        $result = $this->applyPermissionsAccordingToRules($acl);
+        $this->applyDenyPermissionsForMissingRules($acl, ...$result);
+    }
+
+    /**
+     * Apply ACL with rules
+     *
+     * @param Acl $acl
+     * @return array[]
+     */
+    private function applyPermissionsAccordingToRules(Acl $acl): array
+    {
+        $foundResources = $foundDeniedRoles = [];
         foreach ($this->getRulesArray() as $rule) {
             $role = $rule['role_id'];
             $resource = $rule['resource_id'];
             $privileges = !empty($rule['privileges']) ? explode(',', $rule['privileges']) : null;
 
             if ($acl->has($resource)) {
+                $foundResources[$resource] = $resource;
                 if ($rule['permission'] == 'allow') {
                     if ($resource === $this->_rootResource->getId()) {
                         $acl->allow($role, null, $privileges);
                     }
                     $acl->allow($role, $resource, $privileges);
                 } elseif ($rule['permission'] == 'deny') {
+                    $foundDeniedRoles[$role] = $role;
                     $acl->deny($role, $resource, $privileges);
                 }
             }
         }
+        return [$foundResources, $foundDeniedRoles];
+    }
+
+    /**
+     * Apply deny permissions for missing rules
+     *
+     * 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 Acl $acl
+     * @param array $resources
+     * @param array $deniedRoles
+     * @return void
+     */
+    private function applyDenyPermissionsForMissingRules(
+        Acl $acl,
+        array $resources,
+        array $deniedRoles
+    ) {
+        if (count($resources) && count($deniedRoles)
+            //ignore denying missing permission if all are allowed
+            && !(count($resources) === 1 && isset($resources[static::ALLOW_EVERYTHING]))
+        ) {
+            foreach ($acl->getResources() as $resource) {
+                if (!isset($resources[$resource])) {
+                    foreach ($deniedRoles 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('deny')
-            ->with('3', 1, null);
+            ->withConsecutive(
+                ['3', 1, null]
+            );
+
+        $aclMock
+            ->method('getResources')
+            ->willReturn([
+                'Magento_Backend::all',
+                'Magento_Backend::admin',
+                'Vendor_MyModule::menu',
+                'Vendor_MyModule::index'
+            ]);
 
         $this->model->populateAcl($aclMock);
     }