Skip to content

Commit 4e48f72

Browse files
author
Stanislav Idolov
authored
Merge pull request magento#3083 from magento-tsg/MC-34751
[TSG] MC-34751: [Backport for 2.3.x] Improper Authorization in Inventory module in Store allows Edit Stocks, Delete Stocks, Source to Stock Assigning without permissions
2 parents 36a853b + ec29a67 commit 4e48f72

File tree

18 files changed

+644
-43
lines changed

18 files changed

+644
-43
lines changed

‎Inventory/Test/Integration/Stock/StockSourceLinkProcessorTest.php‎

Lines changed: 77 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
namespace Magento\Inventory\Test\Integration\Stock;
99

10+
use Magento\Backend\Model\Auth;
1011
use Magento\Framework\Api\SearchCriteriaBuilder;
1112
use Magento\Framework\Api\SortOrderBuilder;
1213
use Magento\InventoryAdminUi\Model\Stock\StockSourceLinkProcessor;
@@ -15,6 +16,11 @@
1516
use Magento\TestFramework\Helper\Bootstrap;
1617
use PHPUnit\Framework\TestCase;
1718

19+
/**
20+
* Tests for \Magento\InventoryAdminUi\Model\Stock\StockSourceLinkProcessor.
21+
*
22+
* @magentoAppArea adminhtml
23+
*/
1824
class StockSourceLinkProcessorTest extends TestCase
1925
{
2026
/**
@@ -37,23 +43,86 @@ class StockSourceLinkProcessorTest extends TestCase
3743
*/
3844
private $getStockSourceLinks;
3945

46+
/**
47+
* @var Auth
48+
*/
49+
private $auth;
50+
4051
/**
4152
* @inheritdoc
4253
*/
4354
protected function setUp()
4455
{
45-
$this->searchCriteriaBuilder = Bootstrap::getObjectManager()->get(SearchCriteriaBuilder::class);
46-
$this->getStockSourceLinks = Bootstrap::getObjectManager()->get(GetStockSourceLinksInterface::class);
47-
$this->stockSourceLinkProcessor = Bootstrap::getObjectManager()->get(StockSourceLinkProcessor::class);
48-
$this->sortOrderBuilder = Bootstrap::getObjectManager()->get(SortOrderBuilder::class);
56+
$objectManager = Bootstrap::getObjectManager();
57+
$this->searchCriteriaBuilder = $objectManager->get(SearchCriteriaBuilder::class);
58+
$this->getStockSourceLinks = $objectManager->get(GetStockSourceLinksInterface::class);
59+
$this->stockSourceLinkProcessor = $objectManager->get(StockSourceLinkProcessor::class);
60+
$this->sortOrderBuilder = $objectManager->get(SortOrderBuilder::class);
61+
$this->auth = $objectManager->get(Auth::class);
62+
}
63+
64+
protected function tearDown()
65+
{
66+
$this->auth->logout();
67+
parent::tearDown();
4968
}
5069

5170
/**
52-
* @magentoDataFixture ../../../../app/code/Magento/InventoryApi/Test/_files/sources.php
53-
* @magentoDataFixture ../../../../app/code/Magento/InventoryApi/Test/_files/stocks.php
54-
* @magentoDataFixture ../../../../app/code/Magento/InventoryApi/Test/_files/stock_source_links.php
71+
* @magentoDataFixture Magento_InventoryApi::Test/_files/sources.php
72+
* @magentoDataFixture Magento_InventoryApi::Test/_files/stocks.php
73+
* @magentoDataFixture Magento_InventoryApi::Test/_files/stock_source_links.php
5574
*/
5675
public function testProcess()
76+
{
77+
$this->auth->login(
78+
\Magento\TestFramework\Bootstrap::ADMIN_NAME,
79+
\Magento\TestFramework\Bootstrap::ADMIN_PASSWORD
80+
);
81+
82+
$linksData = $this->getLinksData();
83+
$stockId = 10;
84+
85+
$this->stockSourceLinkProcessor->process($stockId, $linksData);
86+
87+
$sortOrder = $this->sortOrderBuilder
88+
->setField(StockSourceLinkInterface::PRIORITY)
89+
->setAscendingDirection()
90+
->create();
91+
$searchCriteria = $this->searchCriteriaBuilder
92+
->addFilter(StockSourceLinkInterface::STOCK_ID, $stockId)
93+
->addSortOrder($sortOrder)
94+
->create();
95+
$searchResult = $this->getStockSourceLinks->execute($searchCriteria);
96+
97+
self::assertCount(2, $searchResult->getItems());
98+
}
99+
100+
/**
101+
* @magentoDataFixture Magento_InventoryAdminUi::Test/Integration/_files/user_assigned_to_stocks.php
102+
* @magentoDataFixture Magento_InventoryApi::Test/_files/sources.php
103+
* @magentoDataFixture Magento_InventoryApi::Test/_files/stocks.php
104+
* @magentoDataFixture Magento_InventoryApi::Test/_files/stock_source_links.php
105+
*/
106+
public function testProcessUserWithoutPermissions()
107+
{
108+
$this->auth->login(
109+
'stocksAccessUser',
110+
\Magento\TestFramework\Bootstrap::ADMIN_PASSWORD
111+
);
112+
113+
$linksData = $this->getLinksData();
114+
$stockId = 10;
115+
116+
$this->expectException(\Magento\Framework\Exception\AuthorizationException::class);
117+
$this->expectExceptionMessage('It is not allowed to change sources');
118+
119+
$this->stockSourceLinkProcessor->process($stockId, $linksData);
120+
}
121+
122+
/**
123+
* @return array
124+
*/
125+
private function getLinksData(): array
57126
{
58127
/**
59128
* eu-3 - should be updated
@@ -70,20 +139,7 @@ public function testProcess()
70139
StockSourceLinkInterface::PRIORITY => 2,
71140
],
72141
];
73-
$stockId = 10;
74142

75-
$this->stockSourceLinkProcessor->process($stockId, $linksData);
76-
77-
$sortOrder = $this->sortOrderBuilder
78-
->setField(StockSourceLinkInterface::PRIORITY)
79-
->setAscendingDirection()
80-
->create();
81-
$searchCriteria = $this->searchCriteriaBuilder
82-
->addFilter(StockSourceLinkInterface::STOCK_ID, $stockId)
83-
->addSortOrder($sortOrder)
84-
->create();
85-
$searchResult = $this->getStockSourceLinks->execute($searchCriteria);
86-
87-
self::assertCount(2, $searchResult->getItems());
143+
return $linksData;
88144
}
89145
}

‎InventoryAdminUi/Controller/Adminhtml/Stock/Delete.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ class Delete extends Action implements HttpPostActionInterface
2323
/**
2424
* @see _isAllowed()
2525
*/
26-
const ADMIN_RESOURCE = 'Magento_InventoryApi::stock';
26+
const ADMIN_RESOURCE = 'Magento_InventoryApi::stock_delete';
2727

2828
/**
2929
* @var StockRepositoryInterface

‎InventoryAdminUi/Controller/Adminhtml/Stock/Edit.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class Edit extends Action implements HttpGetActionInterface
2626
/**
2727
* @see _isAllowed()
2828
*/
29-
const ADMIN_RESOURCE = 'Magento_InventoryApi::stock';
29+
const ADMIN_RESOURCE = 'Magento_InventoryApi::stock_edit';
3030

3131
/**
3232
* @var StockRepositoryInterface

‎InventoryAdminUi/Controller/Adminhtml/Stock/InlineEdit.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ class InlineEdit extends Action implements HttpPostActionInterface
2828
/**
2929
* @see _isAllowed()
3030
*/
31-
const ADMIN_RESOURCE = 'Magento_InventoryApi::stock';
31+
const ADMIN_RESOURCE = 'Magento_InventoryApi::stock_edit';
3232

3333
/**
3434
* @var DataObjectHelper

‎InventoryAdminUi/Controller/Adminhtml/Stock/MassDelete.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ class MassDelete extends Action implements HttpPostActionInterface
2323
/**
2424
* @see _isAllowed()
2525
*/
26-
const ADMIN_RESOURCE = 'Magento_InventoryApi::stock';
26+
const ADMIN_RESOURCE = 'Magento_InventoryApi::stock_delete';
2727

2828
/**
2929
* @var StockRepositoryInterface

‎InventoryAdminUi/Controller/Adminhtml/Stock/NewAction.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ class NewAction extends Action implements HttpGetActionInterface
2222
/**
2323
* @see _isAllowed()
2424
*/
25-
const ADMIN_RESOURCE = 'Magento_InventoryApi::stock';
25+
const ADMIN_RESOURCE = 'Magento_InventoryApi::stock_edit';
2626

2727
/**
2828
* @inheritdoc

‎InventoryAdminUi/Controller/Adminhtml/Stock/Save.php‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ class Save extends Action implements HttpPostActionInterface
2929
/**
3030
* @see _isAllowed()
3131
*/
32-
const ADMIN_RESOURCE = 'Magento_InventoryApi::stock';
32+
const ADMIN_RESOURCE = 'Magento_InventoryApi::stock_edit';
3333

3434
/**
3535
* @var StockSaveProcessor
@@ -65,6 +65,8 @@ public function execute(): ResultInterface
6565
}
6666

6767
/**
68+
* Save stock process.
69+
*
6870
* @param array $requestData
6971
* @param RequestInterface $request
7072
* @param Redirect $resultRedirect
@@ -106,6 +108,8 @@ private function processSave(
106108
}
107109

108110
/**
111+
* Process result redirect after success save according params.
112+
*
109113
* @param Redirect $resultRedirect
110114
* @param int $stockId
111115
*
@@ -128,6 +132,8 @@ private function processRedirectAfterSuccessSave(Redirect $resultRedirect, int $
128132
}
129133

130134
/**
135+
* Process result redirect after failed save according params.
136+
*
131137
* @param Redirect $resultRedirect
132138
* @param int|null $stockId
133139
*

‎InventoryAdminUi/Model/Stock/StockSourceLinkProcessor.php‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
namespace Magento\InventoryAdminUi\Model\Stock;
99

1010
use Magento\Framework\Api\DataObjectHelper;
11+
use Magento\Framework\AuthorizationInterface;
12+
use Magento\Framework\Exception\AuthorizationException;
1113
use Magento\Framework\Exception\InputException;
1214
use Magento\Framework\Api\SearchCriteriaBuilder;
1315
use Magento\InventoryApi\Api\Data\StockSourceLinkInterfaceFactory;
@@ -17,7 +19,8 @@
1719
use Magento\InventoryApi\Api\StockSourceLinksSaveInterface;
1820

1921
/**
20-
* At the time of processing Stock save form this class used to save links correctly
22+
* At the time of processing Stock save form this class used to save links correctly.
23+
*
2124
* Performs replace strategy of sources for the stock
2225
*/
2326
class StockSourceLinkProcessor
@@ -52,35 +55,46 @@ class StockSourceLinkProcessor
5255
*/
5356
private $dataObjectHelper;
5457

58+
/**
59+
* @var AuthorizationInterface
60+
*/
61+
private $authorization;
62+
5563
/**
5664
* @param SearchCriteriaBuilder $searchCriteriaBuilder
5765
* @param StockSourceLinkInterfaceFactory $stockSourceLinkFactory
5866
* @param StockSourceLinksSaveInterface $stockSourceLinksSave
5967
* @param StockSourceLinksDeleteInterface $stockSourceLinksDelete
6068
* @param GetStockSourceLinksInterface $getStockSourceLinks
6169
* @param DataObjectHelper $dataObjectHelper
70+
* @param AuthorizationInterface $authorization
6271
*/
6372
public function __construct(
6473
SearchCriteriaBuilder $searchCriteriaBuilder,
6574
StockSourceLinkInterfaceFactory $stockSourceLinkFactory,
6675
StockSourceLinksSaveInterface $stockSourceLinksSave,
6776
StockSourceLinksDeleteInterface $stockSourceLinksDelete,
6877
GetStockSourceLinksInterface $getStockSourceLinks,
69-
DataObjectHelper $dataObjectHelper
78+
DataObjectHelper $dataObjectHelper,
79+
AuthorizationInterface $authorization
7080
) {
7181
$this->searchCriteriaBuilder = $searchCriteriaBuilder;
7282
$this->stockSourceLinkFactory = $stockSourceLinkFactory;
7383
$this->stockSourceLinksSave = $stockSourceLinksSave;
7484
$this->stockSourceLinksDelete = $stockSourceLinksDelete;
7585
$this->getStockSourceLinks = $getStockSourceLinks;
7686
$this->dataObjectHelper = $dataObjectHelper;
87+
$this->authorization = $authorization;
7788
}
7889

7990
/**
91+
* Performs replace strategy of sources for the stock.
92+
*
8093
* @param int $stockId
8194
* @param array $linksData
8295
* @return void
8396
* @throws InputException
97+
* @throws AuthorizationException
8498
*/
8599
public function process(int $stockId, array $linksData)
86100
{
@@ -100,6 +114,12 @@ public function process(int $stockId, array $linksData)
100114
$linkData[StockSourceLinkInterface::STOCK_ID] = $stockId;
101115
$this->dataObjectHelper->populateWithArray($link, $linkData, StockSourceLinkInterface::class);
102116

117+
if (!$this->authorization->isAllowed('Magento_InventoryApi::stock_source_link')
118+
&& $link->getData() != $link->getOrigData()
119+
) {
120+
throw new AuthorizationException(__('It is not allowed to change sources'));
121+
}
122+
103123
$linksForSave[] = $link;
104124
unset($linksForDelete[$sourceCode]);
105125
}
@@ -108,6 +128,9 @@ public function process(int $stockId, array $linksData)
108128
$this->stockSourceLinksSave->execute($linksForSave);
109129
}
110130
if (count($linksForDelete) > 0) {
131+
if (!$this->authorization->isAllowed('Magento_InventoryApi::stock_source_link')) {
132+
throw new AuthorizationException(__('It is not allowed to change sources'));
133+
}
111134
$this->stockSourceLinksDelete->execute($linksForDelete);
112135
}
113136
}

0 commit comments

Comments
 (0)