Skip to content

Commit 7989cc9

Browse files
chittimachittima
committed
Merge branch 'ACP2E-3503' of https://github.com/adobe-commerce-tier-4/magento2ce into PR-12-05-2024
2 parents 8c4a100 + e3c82e3 commit 7989cc9

File tree

2 files changed

+191
-23
lines changed

2 files changed

+191
-23
lines changed

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

Lines changed: 51 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
<?php
22
/**
3-
* Copyright © Magento, Inc. All rights reserved.
4-
* See COPYING.txt for license details.
3+
* Copyright 2012 Adobe
4+
* All Rights Reserved.
55
*/
66
declare(strict_types=1);
77

@@ -87,7 +87,7 @@ public function __construct(
8787
public function populateAcl(Acl $acl)
8888
{
8989
$result = $this->applyPermissionsAccordingToRules($acl);
90-
$this->applyDenyPermissionsForMissingRules($acl, ...$result);
90+
$this->denyPermissionsForMissingRules($acl, $result);
9191
}
9292

9393
/**
@@ -98,56 +98,84 @@ public function populateAcl(Acl $acl)
9898
*/
9999
private function applyPermissionsAccordingToRules(Acl $acl): array
100100
{
101-
$foundResources = $foundDeniedRoles = [];
101+
$appliedRolePermissionsPerResource = [];
102102
foreach ($this->getRulesArray() as $rule) {
103103
$role = $rule['role_id'];
104104
$resource = $rule['resource_id'];
105105
$privileges = !empty($rule['privileges']) ? explode(',', $rule['privileges']) : null;
106106

107107
if ($acl->hasResource($resource)) {
108-
$foundResources[$resource] = $resource;
108+
109+
$appliedRolePermissionsPerResource[$resource]['allow'] =
110+
$appliedRolePermissionsPerResource[$resource]['allow'] ?? [];
111+
$appliedRolePermissionsPerResource[$resource]['deny'] =
112+
$appliedRolePermissionsPerResource[$resource]['deny'] ?? [];
113+
109114
if ($rule['permission'] == 'allow') {
110115
if ($resource === $this->_rootResource->getId()) {
111116
$acl->allow($role, null, $privileges);
112117
}
113118
$acl->allow($role, $resource, $privileges);
119+
$appliedRolePermissionsPerResource[$resource]['allow'][] = $role;
114120
} elseif ($rule['permission'] == 'deny') {
115-
$foundDeniedRoles[$role] = $role;
116121
$acl->deny($role, $resource, $privileges);
122+
$appliedRolePermissionsPerResource[$resource]['deny'][] = $role;
117123
}
118124
}
119125
}
120-
return [$foundResources, $foundDeniedRoles];
126+
127+
return $appliedRolePermissionsPerResource;
121128
}
122129

123130
/**
124-
* Apply deny permissions for missing rules
131+
* Deny permissions for missing rules
125132
*
126133
* For all rules that were not regenerated in authorization_rule table,
127134
* when adding a new module and without re-saving all roles,
128135
* consider not present rules with deny permissions
129136
*
130137
* @param Acl $acl
131-
* @param array $resources
132-
* @param array $deniedRoles
138+
* @param array $appliedRolePermissionsPerResource
133139
* @return void
134140
*/
135-
private function applyDenyPermissionsForMissingRules(
136-
Acl $acl,
137-
array $resources,
138-
array $deniedRoles
141+
private function denyPermissionsForMissingRules(
142+
Acl $acl,
143+
array $appliedRolePermissionsPerResource,
139144
) {
140-
if (count($resources) && count($deniedRoles)
141-
//ignore denying missing permission if all are allowed
142-
&& !(count($resources) === 1 && isset($resources[static::ALLOW_EVERYTHING]))
143-
) {
144-
foreach ($acl->getResources() as $resource) {
145-
if (!isset($resources[$resource])) {
146-
foreach ($deniedRoles as $role) {
147-
$acl->deny($role, $resource, null);
148-
}
145+
$consolidatedDeniedRoleIds = array_unique(
146+
array_merge(
147+
...array_column($appliedRolePermissionsPerResource, 'deny')
148+
)
149+
);
150+
151+
$hasAppliedPermissions = count($appliedRolePermissionsPerResource) > 0;
152+
$hasDeniedRoles = count($consolidatedDeniedRoleIds) > 0;
153+
$allAllowed = count($appliedRolePermissionsPerResource) === 1
154+
&& isset($appliedRolePermissionsPerResource[static::ALLOW_EVERYTHING]);
155+
156+
if ($hasAppliedPermissions && $hasDeniedRoles && !$allAllowed) {
157+
// Add the resources that are not present in the rules at all,
158+
// assuming that they must be denied for all roles by default
159+
$resourcesUndefinedInAuthorizationRules =
160+
array_diff($acl->getResources(), array_keys($appliedRolePermissionsPerResource));
161+
$assumeDeniedRoleListPerResource =
162+
array_fill_keys($resourcesUndefinedInAuthorizationRules, $consolidatedDeniedRoleIds);
163+
164+
// Add the resources that are permitted for one role and not present in others at all,
165+
// assuming that they must be denied for all other roles by default
166+
foreach ($appliedRolePermissionsPerResource as $resource => $permissions) {
167+
$allowedRoles = $permissions['allow'];
168+
$deniedRoles = $permissions['deny'];
169+
$assumedDeniedRoles = array_diff($consolidatedDeniedRoleIds, $allowedRoles, $deniedRoles);
170+
if ($assumedDeniedRoles) {
171+
$assumeDeniedRoleListPerResource[$resource] = $assumedDeniedRoles;
149172
}
150173
}
174+
175+
// Deny permissions for missing rules
176+
foreach ($assumeDeniedRoleListPerResource as $resource => $denyRoles) {
177+
$acl->deny($denyRoles, $resource, null);
178+
}
151179
}
152180
}
153181

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

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
<?php
2+
/**
3+
* Copyright 2024 Adobe
4+
* All Rights Reserved.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\Authorization\Test\Unit\Model\Acl\Loader;
9+
10+
use Magento\Authorization\Model\Acl\Loader\Rule;
11+
use Magento\Framework\Acl;
12+
use Magento\Framework\Acl\Data\CacheInterface;
13+
use Magento\Framework\Acl\RootResource;
14+
use Magento\Framework\App\ResourceConnection;
15+
use Magento\Framework\Serialize\Serializer\Json;
16+
use PHPUnit\Framework\MockObject\Exception;
17+
use PHPUnit\Framework\MockObject\MockObject;
18+
use PHPUnit\Framework\TestCase;
19+
20+
/**
21+
* @covers \Magento\Authorization\Model\Acl\Loader\Rule
22+
*/
23+
class MissingDeclineRuleTest extends TestCase
24+
{
25+
/**
26+
* @var Rule
27+
*/
28+
private $model;
29+
30+
/**
31+
* @var RootResource
32+
*/
33+
private $rootResource;
34+
35+
/**
36+
* @var ResourceConnection|MockObject
37+
*/
38+
private $resourceMock;
39+
40+
/**
41+
* @var CacheInterface|MockObject
42+
*/
43+
private $aclDataCacheMock;
44+
45+
/**
46+
* @var Json|MockObject
47+
*/
48+
private $serializerMock;
49+
50+
/**
51+
* @inheritDoc
52+
*/
53+
protected function setUp(): void
54+
{
55+
$this->rootResource = new RootResource('Magento_Backend::all');
56+
57+
$this->resourceMock = $this->getMockBuilder(ResourceConnection::class)
58+
->addMethods(['getTable'])
59+
->onlyMethods(['getConnection'])
60+
->disableOriginalConstructor()
61+
->getMock();
62+
63+
$this->aclDataCacheMock = $this->createMock(CacheInterface::class);
64+
$this->serializerMock = $this->createPartialMock(
65+
Json::class,
66+
['serialize', 'unserialize']
67+
);
68+
69+
$this->serializerMock->method('serialize')
70+
->willReturnCallback(
71+
static function ($value) {
72+
return json_encode($value);
73+
}
74+
);
75+
76+
$this->serializerMock->method('unserialize')
77+
->willReturnCallback(
78+
static function ($value) {
79+
return json_decode($value, true);
80+
}
81+
);
82+
83+
$this->model = new Rule(
84+
$this->rootResource,
85+
$this->resourceMock,
86+
$this->aclDataCacheMock,
87+
$this->serializerMock
88+
);
89+
}
90+
91+
/**
92+
* This test ensures that any new resources, which have not been explicitly defined in the authorization_rule table,
93+
* are automatically denied for all roles unless explicitly allowed.
94+
*
95+
* @return void
96+
* @throws Exception
97+
*/
98+
public function testDenyAbsentResources(): void
99+
{
100+
// Vendor_MyModule::menu and Vendor_MyModule::report permissions are not present in the authorization_rules
101+
// table for role 3, and should be denied by default
102+
$authorizationRulesData = [
103+
['rule_id' => 1, 'role_id' => 2, 'resource_id' => 'Magento_Backend::all', 'permission' => 'deny'],
104+
['rule_id' => 2, 'role_id' => 2, 'resource_id' => 'Vendor_MyModule::index', 'permission' => 'allow'],
105+
['rule_id' => 3, 'role_id' => 2, 'resource_id' => 'Vendor_MyModule::menu', 'permission' => 'deny'],
106+
['rule_id' => 4, 'role_id' => 2, 'resource_id' => 'Vendor_MyModule::report', 'permission' => 'deny'],
107+
['rule_id' => 5, 'role_id' => 3, 'resource_id' => 'Magento_Backend::all', 'permission' => 'deny'],
108+
['rule_id' => 6, 'role_id' => 3, 'resource_id' => 'Vendor_MyModule::index', 'permission' => 'allow'],
109+
];
110+
111+
// Vendor_MyModule::configuration is a new resource that has not been defined in the authorization_rules table
112+
// for any role, and should be denied by default
113+
$getAclResourcesData = array_unique(array_column($authorizationRulesData, 'resource_id'));
114+
$getAclResourcesData[] = 'Vendor_MyModule::configuration';
115+
116+
$this->aclDataCacheMock->expects($this->once())
117+
->method('load')
118+
->with(Rule::ACL_RULE_CACHE_KEY)
119+
->willReturn(
120+
json_encode($authorizationRulesData)
121+
);
122+
123+
$aclMock = $this->createMock(Acl::class);
124+
$aclMock->method('hasResource')->willReturn(true);
125+
126+
$aclMock
127+
->expects($this->exactly(2))
128+
->method('allow');
129+
130+
$aclMock
131+
->expects($this->exactly(7))
132+
->method('deny');
133+
134+
$aclMock
135+
->method('getResources')
136+
->willReturn($getAclResourcesData);
137+
138+
$this->model->populateAcl($aclMock);
139+
}
140+
}

0 commit comments

Comments
 (0)