Skip to content

Add ACL role ID to category tree cache id #27429

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,16 @@

namespace Magento\Catalog\Test\Unit\Ui\DataProvider\Product\Form\Modifier;

use Magento\Catalog\Model\ResourceModel\Category\Collection as CategoryCollection;
use Magento\Catalog\Model\ResourceModel\Category\CollectionFactory as CategoryCollectionFactory;
use Magento\Catalog\Ui\DataProvider\Product\Form\Modifier\Categories;
use Magento\Catalog\Model\ResourceModel\Category\CollectionFactory as CategoryCollectionFactory;
use Magento\Catalog\Model\ResourceModel\Category\Collection as CategoryCollection;
use Magento\Framework\AuthorizationInterface;
use Magento\Framework\DB\Helper as DbHelper;
use Magento\Framework\UrlInterface;
use Magento\Store\Model\Store;
use Magento\Backend\Model\Auth\Session;
use Magento\Authorization\Model\Role;
use Magento\User\Model\User;
use PHPUnit\Framework\MockObject\MockObject;

/**
Expand Down Expand Up @@ -51,6 +54,11 @@ class CategoriesTest extends AbstractModifierTest
*/
private $authorizationMock;

/**
* @var Session|MockObject
*/
private $sessionMock;

protected function setUp(): void
{
parent::setUp();
Expand All @@ -72,7 +80,10 @@ protected function setUp(): void
$this->authorizationMock = $this->getMockBuilder(AuthorizationInterface::class)
->disableOriginalConstructor()
->getMockForAbstractClass();

$this->sessionMock = $this->getMockBuilder(Session::class)
->setMethods(['getUser'])
->disableOriginalConstructor()
->getMock();
$this->categoryCollectionFactoryMock->expects($this->any())
->method('create')
->willReturn($this->categoryCollectionMock);
Expand All @@ -88,6 +99,26 @@ protected function setUp(): void
$this->categoryCollectionMock->expects($this->any())
->method('getIterator')
->willReturn(new \ArrayIterator([]));

$roleAdmin = $this->getMockBuilder(Role::class)
->setMethods(['getId'])
->disableOriginalConstructor()
->getMock();
$roleAdmin->expects($this->any())
->method('getId')
->willReturn(0);

$userAdmin = $this->getMockBuilder(User::class)
->setMethods(['getRole'])
->disableOriginalConstructor()
->getMock();
$userAdmin->expects($this->any())
->method('getRole')
->willReturn($roleAdmin);

$this->sessionMock->expects($this->any())
->method('getUser')
->willReturn($userAdmin);
}

/**
Expand All @@ -101,11 +132,28 @@ protected function createModel()
'locator' => $this->locatorMock,
'categoryCollectionFactory' => $this->categoryCollectionFactoryMock,
'arrayManager' => $this->arrayManagerMock,
'authorization' => $this->authorizationMock
'authorization' => $this->authorizationMock,
'session' => $this->sessionMock
]
);
}

/**
* @param object $object
* @param string $method
* @param array $args
* @return mixed
* @throws \ReflectionException
*/
private function invokeMethod($object, $method, $args = [])
{
$class = new \ReflectionClass(Categories::class);
$method = $class->getMethod($method);
$method->setAccessible(true);

return $method->invokeArgs($object, $args);
}

public function testModifyData()
{
$this->assertSame([], $this->getModel()->modifyData([]));
Expand Down Expand Up @@ -176,4 +224,44 @@ public function modifyMetaLockedDataProvider()
{
return [[true], [false]];
}

/**
* Asserts that a user with an ACL role ID of 0 and a user with an ACL role ID of 1 do not have the same cache IDs
* Assumes a store ID of 0
*
* @throws \ReflectionException
*/
public function testAclCacheIds()
{
$categoriesAdmin = $this->createModel();
$cacheIdAdmin = $this->invokeMethod($categoriesAdmin, 'getCategoriesTreeCacheId', [0]);

$roleAclUser = $this->getMockBuilder(Role::class)
->disableOriginalConstructor()
->getMock();
$roleAclUser->expects($this->any())
->method('getId')
->willReturn(1);

$userAclUser = $this->getMockBuilder(User::class)
->disableOriginalConstructor()
->getMock();
$userAclUser->expects($this->any())
->method('getRole')
->will($this->returnValue($roleAclUser));

$this->sessionMock = $this->getMockBuilder(Session::class)
->setMethods(['getUser'])
->disableOriginalConstructor()
->getMock();

$this->sessionMock->expects($this->any())
->method('getUser')
->will($this->returnValue($userAclUser));

$categoriesAclUser = $this->createModel();
$cacheIdAclUser = $this->invokeMethod($categoriesAclUser, 'getCategoriesTreeCacheId', [0]);

$this->assertNotEquals($cacheIdAdmin, $cacheIdAclUser);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@
use Magento\Framework\UrlInterface;
use Magento\Framework\Stdlib\ArrayManager;
use Magento\Framework\AuthorizationInterface;
use Magento\Backend\Model\Auth\Session;

/**
* Data provider for categories field of product page
*
* @api
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
* @SuppressWarnings(PHPMD.CookieAndSessionMisuse)
* @since 101.0.0
*/
class Categories extends AbstractModifier
Expand Down Expand Up @@ -86,6 +88,11 @@ class Categories extends AbstractModifier
*/
private $authorization;

/**
* @var Session
*/
private $session;

/**
* @param LocatorInterface $locator
* @param CategoryCollectionFactory $categoryCollectionFactory
Expand All @@ -94,6 +101,7 @@ class Categories extends AbstractModifier
* @param ArrayManager $arrayManager
* @param SerializerInterface $serializer
* @param AuthorizationInterface $authorization
* @param Session $session
*/
public function __construct(
LocatorInterface $locator,
Expand All @@ -102,7 +110,8 @@ public function __construct(
UrlInterface $urlBuilder,
ArrayManager $arrayManager,
SerializerInterface $serializer = null,
AuthorizationInterface $authorization = null
AuthorizationInterface $authorization = null,
Session $session = null
) {
$this->locator = $locator;
$this->categoryCollectionFactory = $categoryCollectionFactory;
Expand All @@ -111,6 +120,7 @@ public function __construct(
$this->arrayManager = $arrayManager;
$this->serializer = $serializer ?: ObjectManager::getInstance()->get(SerializerInterface::class);
$this->authorization = $authorization ?: ObjectManager::getInstance()->get(AuthorizationInterface::class);
$this->session = $session ?: ObjectManager::getInstance()->get(Session::class);
}

/**
Expand Down Expand Up @@ -370,10 +380,16 @@ protected function getCategoriesTree($filter = null)
* @param string $filter
* @return string
*/
private function getCategoriesTreeCacheId(int $storeId, string $filter = '') : string
private function getCategoriesTreeCacheId(int $storeId, string $filter = ''): string
{
if ($this->session->getUser() !== null) {
return self::CATEGORY_TREE_ID
. '_' . (string)$storeId
. '_' . $this->session->getUser()->getAclRole()
. '_' . $filter;
}
return self::CATEGORY_TREE_ID
. '_' . (string) $storeId
. '_' . (string)$storeId
. '_' . $filter;
}

Expand Down