Skip to content

Commit 692c869

Browse files
abukatarabukatar
committed
ACP2E-4114: Production ACL Permission Check caused Performance Degradation – populateAcl Method is the bottleneck
1 parent 3026734 commit 692c869

File tree

4 files changed

+231
-43
lines changed

4 files changed

+231
-43
lines changed

‎app/code/Magento/Authorization/Model/Acl/AclRetriever.php‎

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use Magento\Authorization\Model\ResourceModel\Rules\CollectionFactory as RulesCollectionFactory;
1111
use Magento\Authorization\Model\Role;
1212
use Magento\Authorization\Model\UserContextInterface;
13+
use Magento\Framework\Acl\Role\CurrentRoleContext;
1314
use Magento\Framework\Acl\Builder as AclBuilder;
1415
use Magento\Framework\Exception\AuthorizationException;
1516
use Magento\Framework\Exception\LocalizedException;
@@ -44,24 +45,33 @@ class AclRetriever
4445
*/
4546
protected $roleCollectionFactory;
4647

48+
/**
49+
* @var CurrentRoleContext
50+
*/
51+
private $currentRoleContext;
52+
4753
/**
4854
* Initialize dependencies.
4955
*
5056
* @param AclBuilder $aclBuilder
5157
* @param RoleCollectionFactory $roleCollectionFactory
5258
* @param RulesCollectionFactory $rulesCollectionFactory
5359
* @param Logger $logger
60+
* @param ?CurrentRoleContext $currentRoleContext
5461
*/
5562
public function __construct(
5663
AclBuilder $aclBuilder,
5764
RoleCollectionFactory $roleCollectionFactory,
5865
RulesCollectionFactory $rulesCollectionFactory,
59-
Logger $logger
66+
Logger $logger,
67+
?CurrentRoleContext $currentRoleContext = null
6068
) {
6169
$this->logger = $logger;
6270
$this->rulesCollectionFactory = $rulesCollectionFactory;
6371
$this->aclBuilder = $aclBuilder;
6472
$this->roleCollectionFactory = $roleCollectionFactory;
73+
$this->currentRoleContext = $currentRoleContext ?: \Magento\Framework\App\ObjectManager::getInstance()
74+
->get(CurrentRoleContext::class);
6575
}
6676

6777
/**
@@ -110,17 +120,26 @@ public function getAllowedResourcesByUser($userType, $userId)
110120
*/
111121
public function getAllowedResourcesByRole($roleId)
112122
{
113-
$allowedResources = [];
114-
$rulesCollection = $this->rulesCollectionFactory->create();
115-
$rulesCollection->getByRoles($roleId)->load();
116-
$acl = $this->aclBuilder->getAcl();
117-
/** @var \Magento\Authorization\Model\Rules $ruleItem */
118-
foreach ($rulesCollection->getItems() as $ruleItem) {
119-
$resourceId = $ruleItem->getResourceId();
120-
if ($acl->hasResource($resourceId) && $acl->isAllowed($roleId, $resourceId)) {
121-
$allowedResources[] = $resourceId;
123+
try {
124+
$allowedResources = [];
125+
$rulesCollection = $this->rulesCollectionFactory->create();
126+
$rulesCollection->getByRoles($roleId)->load();
127+
if ($roleId && (int) $roleId !== $this->currentRoleContext->getRoleId()) {
128+
$this->aclBuilder->resetRuntimeAcl();
129+
}
130+
$this->currentRoleContext->setRoleId((int) $roleId);
131+
$acl = $this->aclBuilder->getAcl();
132+
/** @var \Magento\Authorization\Model\Rules $ruleItem */
133+
foreach ($rulesCollection->getItems() as $ruleItem) {
134+
$resourceId = $ruleItem->getResourceId();
135+
if ($acl->hasResource($resourceId) && $acl->isAllowed($roleId, $resourceId)) {
136+
$allowedResources[] = $resourceId;
137+
}
122138
}
139+
} finally {
140+
$this->currentRoleContext->_resetState();
123141
}
142+
124143
return $allowedResources;
125144
}
126145

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

Lines changed: 132 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use Magento\Framework\Acl\Data\CacheInterface;
1212
use Magento\Framework\Acl\LoaderInterface;
1313
use Magento\Framework\Acl\RootResource;
14+
use Magento\Framework\Acl\Role\CurrentRoleContext;
1415
use Magento\Framework\App\ResourceConnection;
1516
use Magento\Framework\Serialize\Serializer\Json;
1617

@@ -54,28 +55,38 @@ class Rule implements LoaderInterface
5455
*/
5556
private $cacheKey;
5657

58+
/**
59+
* @var CurrentRoleContext
60+
*/
61+
private $roleContext;
62+
5763
/**
5864
* @param RootResource $rootResource
5965
* @param ResourceConnection $resource
6066
* @param CacheInterface $aclDataCache
6167
* @param Json $serializer
6268
* @param array $data
6369
* @param string $cacheKey
70+
* @param CurrentRoleContext|null $roleContext
6471
* @SuppressWarnings(PHPMD.UnusedFormalParameter):
6572
*/
6673
public function __construct(
67-
RootResource $rootResource,
68-
ResourceConnection $resource,
69-
CacheInterface $aclDataCache,
70-
Json $serializer,
71-
array $data = [],
72-
$cacheKey = self::ACL_RULE_CACHE_KEY
74+
RootResource $rootResource,
75+
ResourceConnection $resource,
76+
CacheInterface $aclDataCache,
77+
Json $serializer,
78+
array $data = [],
79+
string $cacheKey = self::ACL_RULE_CACHE_KEY,
80+
?CurrentRoleContext $roleContext = null
7381
) {
7482
$this->_rootResource = $rootResource;
7583
$this->_resource = $resource;
7684
$this->aclDataCache = $aclDataCache;
7785
$this->serializer = $serializer;
7886
$this->cacheKey = $cacheKey;
87+
88+
$this->roleContext = $roleContext ?? \Magento\Framework\App\ObjectManager::getInstance()
89+
->get(CurrentRoleContext::class);
7990
}
8091

8192
/**
@@ -86,10 +97,31 @@ public function __construct(
8697
*/
8798
public function populateAcl(Acl $acl)
8899
{
89-
$result = $this->applyPermissionsAccordingToRules($acl);
100+
$roleId = $this->roleContext->getRoleId();
101+
$result = ($roleId)
102+
? $this->applyPermissionsForRole($acl, (int)$roleId)
103+
: $this->applyPermissionsAccordingToRules($acl);
90104
$this->denyPermissionsForMissingRules($acl, $result);
91105
}
92106

107+
/**
108+
* Apply permissions for a specific role
109+
*
110+
* @param Acl $acl
111+
* @param int $roleId
112+
* @return array
113+
*/
114+
private function applyPermissionsForRole(Acl $acl, int $roleId): array
115+
{
116+
$appliedRolePermissionsPerResource = [];
117+
foreach ($this->getRulesArrayForRole($roleId) as $rule) {
118+
$appliedRolePermissionsPerResource =
119+
$this->getAppliedRolePermissionsPerResource($rule, $acl, $appliedRolePermissionsPerResource);
120+
}
121+
122+
return $appliedRolePermissionsPerResource;
123+
}
124+
93125
/**
94126
* Apply ACL with rules
95127
*
@@ -100,28 +132,8 @@ private function applyPermissionsAccordingToRules(Acl $acl): array
100132
{
101133
$appliedRolePermissionsPerResource = [];
102134
foreach ($this->getRulesArray() as $rule) {
103-
$role = $rule['role_id'];
104-
$resource = $rule['resource_id'];
105-
$privileges = !empty($rule['privileges']) ? explode(',', $rule['privileges']) : null;
106-
107-
if ($acl->hasResource($resource)) {
108-
109-
$appliedRolePermissionsPerResource[$resource]['allow'] =
110-
$appliedRolePermissionsPerResource[$resource]['allow'] ?? [];
111-
$appliedRolePermissionsPerResource[$resource]['deny'] =
112-
$appliedRolePermissionsPerResource[$resource]['deny'] ?? [];
113-
114-
if ($rule['permission'] == 'allow') {
115-
if ($resource === $this->_rootResource->getId()) {
116-
$acl->allow($role, null, $privileges);
117-
}
118-
$acl->allow($role, $resource, $privileges);
119-
$appliedRolePermissionsPerResource[$resource]['allow'][] = $role;
120-
} elseif ($rule['permission'] == 'deny') {
121-
$acl->deny($role, $resource, $privileges);
122-
$appliedRolePermissionsPerResource[$resource]['deny'][] = $role;
123-
}
124-
}
135+
$appliedRolePermissionsPerResource =
136+
$this->getAppliedRolePermissionsPerResource($rule, $acl, $appliedRolePermissionsPerResource);
125137
}
126138

127139
return $appliedRolePermissionsPerResource;
@@ -202,4 +214,95 @@ private function getRulesArray()
202214

203215
return $rulesArr;
204216
}
217+
218+
/**
219+
* Get application ACL rules array for a specific role.
220+
*
221+
* @param int $roleId
222+
* @return array
223+
*/
224+
private function getRulesArrayForRole(int $roleId): array
225+
{
226+
$groupRoleId = $this->resolveGroupRoleId($roleId);
227+
$cacheKey = self::ACL_RULE_CACHE_KEY . '_' . $groupRoleId;
228+
$rulesCachedData = $this->aclDataCache->load($cacheKey);
229+
if ($rulesCachedData) {
230+
return $this->serializer->unserialize($rulesCachedData);
231+
}
232+
233+
$ruleTable = $this->_resource->getTableName('authorization_rule');
234+
$connection = $this->_resource->getConnection();
235+
$select = $connection->select()
236+
->from(['r' => $ruleTable])
237+
->where('role_id = ?', $groupRoleId)
238+
->order('rule_id ASC');
239+
240+
$rulesArr = $connection->fetchAll($select);
241+
242+
$this->aclDataCache->save($this->serializer->serialize($rulesArr), $cacheKey);
243+
244+
return $rulesArr;
245+
}
246+
247+
/**
248+
* Resolve the group role id for a given role id
249+
*
250+
* @param int $roleId
251+
* @return int
252+
*/
253+
private function resolveGroupRoleId(int $roleId): int
254+
{
255+
$roleTable = $this->_resource->getTableName('authorization_role');
256+
$connection = $this->_resource->getConnection();
257+
$select = $connection->select()
258+
->from($roleTable, ['role_type', 'parent_id'])
259+
->where('role_id = ?', $roleId)
260+
->limit(1);
261+
262+
$row = $connection->fetchRow($select);
263+
if (is_array($row) && isset($row['role_type']) && $row['role_type'] === 'U'
264+
&& (int)($row['parent_id'] ?? 0) > 0
265+
) {
266+
return (int)$row['parent_id'];
267+
}
268+
return $roleId;
269+
}
270+
271+
/**
272+
* Apply rule to ACL and return applied permissions per resource
273+
*
274+
* @param array $rule
275+
* @param Acl $acl
276+
* @param array $appliedRolePermissionsPerResource
277+
* @return array
278+
*/
279+
private function getAppliedRolePermissionsPerResource(
280+
array $rule,
281+
Acl $acl,
282+
array $appliedRolePermissionsPerResource
283+
): array {
284+
$role = $rule['role_id'];
285+
$resource = $rule['resource_id'];
286+
$privileges = !empty($rule['privileges']) ? explode(',', $rule['privileges']) : null;
287+
288+
if ($acl->hasResource($resource)) {
289+
290+
$appliedRolePermissionsPerResource[$resource]['allow'] =
291+
$appliedRolePermissionsPerResource[$resource]['allow'] ?? [];
292+
$appliedRolePermissionsPerResource[$resource]['deny'] =
293+
$appliedRolePermissionsPerResource[$resource]['deny'] ?? [];
294+
295+
if ($rule['permission'] == 'allow') {
296+
if ($resource === $this->_rootResource->getId()) {
297+
$acl->allow($role, null, $privileges);
298+
}
299+
$acl->allow($role, $resource, $privileges);
300+
$appliedRolePermissionsPerResource[$resource]['allow'][] = $role;
301+
} elseif ($rule['permission'] == 'deny') {
302+
$acl->deny($role, $resource, $privileges);
303+
$appliedRolePermissionsPerResource[$resource]['deny'][] = $role;
304+
}
305+
}
306+
return $appliedRolePermissionsPerResource;
307+
}
205308
}

‎lib/internal/Magento/Framework/Acl/Role/CurrentRoleContext.php‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<?php
2+
/**
3+
* Copyright 2025 Adobe
4+
* All Rights Reserved.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\Framework\Acl\Role;
9+
10+
use Magento\Framework\ObjectManager\ResetAfterRequestInterface;
11+
12+
/**
13+
* Holds the current role id during ACL building within a single request.
14+
*/
15+
class CurrentRoleContext implements ResetAfterRequestInterface
16+
{
17+
/**
18+
* @var int|null
19+
*/
20+
private $roleId = null;
21+
22+
/**
23+
* Set the current role ID.
24+
*
25+
* @param int|null $roleId
26+
* @return void
27+
*/
28+
public function setRoleId(int|null $roleId): void
29+
{
30+
$this->roleId = $roleId;
31+
}
32+
33+
/**
34+
* Get the current role ID.
35+
*
36+
* @return int|null
37+
*/
38+
public function getRoleId(): int|null
39+
{
40+
return $this->roleId;
41+
}
42+
43+
/**
44+
* Reset the state after a request.
45+
*
46+
* @return void
47+
*/
48+
public function _resetState(): void
49+
{
50+
$this->roleId = null;
51+
}
52+
}

0 commit comments

Comments
 (0)