Skip to content

Commit 285c6f4

Browse files
SeruyVSeruyV
committed
MAGETWO-96010: [2.2] Restricted admin still able to send order comment email modifying POST request
- Controller fix - Added Unit test
1 parent d20f927 commit 285c6f4

File tree

2 files changed

+205
-4
lines changed

2 files changed

+205
-4
lines changed

‎app/code/Magento/Sales/Controller/Adminhtml/Order/AddComment.php

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,27 @@
99
use Magento\Backend\App\Action;
1010
use Magento\Sales\Model\Order\Email\Sender\OrderCommentSender;
1111

12+
/**
13+
* Controller to execute Adding Comments.
14+
*/
1215
class AddComment extends \Magento\Sales\Controller\Adminhtml\Order
1316
{
1417
/**
15-
* Authorization level of a basic admin session
18+
* Authorization level of a basic admin session.
1619
*
1720
* @see _isAllowed()
1821
*/
1922
const ADMIN_RESOURCE = 'Magento_Sales::comment';
2023

2124
/**
22-
* Add order comment action
25+
* ACL resource needed to send comment email notification.
26+
*
27+
* @see _isAllowed()
28+
*/
29+
const ADMIN_SALES_EMAIL_RESOURCE = 'Magento_Sales::emails';
30+
31+
/**
32+
* Add order comment action.
2333
*
2434
* @return \Magento\Framework\Controller\ResultInterface
2535
*/
@@ -33,8 +43,12 @@ public function execute()
3343
throw new \Magento\Framework\Exception\LocalizedException(__('Please enter a comment.'));
3444
}
3545

36-
$notify = isset($data['is_customer_notified']) ? $data['is_customer_notified'] : false;
37-
$visible = isset($data['is_visible_on_front']) ? $data['is_visible_on_front'] : false;
46+
$notify = $data['is_customer_notified'] ?? false;
47+
$visible = $data['is_visible_on_front'] ?? false;
48+
49+
if ($notify && !$this->_authorization->isAllowed(self::ADMIN_SALES_EMAIL_RESOURCE)) {
50+
$notify = false;
51+
}
3852

3953
$history = $order->addStatusHistoryComment($data['comment'], $data['status']);
4054
$history->setIsVisibleOnFront($visible);
@@ -59,9 +73,11 @@ public function execute()
5973
if (is_array($response)) {
6074
$resultJson = $this->resultJsonFactory->create();
6175
$resultJson->setData($response);
76+
6277
return $resultJson;
6378
}
6479
}
80+
6581
return $this->resultRedirectFactory->create()->setPath('sales/*/');
6682
}
6783
}

‎app/code/Magento/Sales/Test/Unit/Controller/Adminhtml/Order/AddCommentTest.php

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,185 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
namespace Magento\Sales\Test\Unit\Controller\Adminhtml\Order;
7+
8+
/**
9+
* Test for AddComment.
10+
*
11+
*/
12+
class AddCommentTest extends \PHPUnit\Framework\TestCase
13+
{
14+
/**
15+
* @var \Magento\Sales\Controller\Adminhtml\Order\AddComment
16+
*/
17+
private $addCommentController;
18+
19+
/**
20+
* @var \Magento\Backend\App\Action\Context|\PHPUnit_Framework_MockObject_MockObject
21+
*/
22+
private $contextMock;
23+
24+
/**
25+
* @var \Magento\Sales\Model\Order|\PHPUnit_Framework_MockObject_MockObject
26+
*/
27+
private $orderMock;
28+
29+
/**
30+
* @var \Magento\Backend\Model\View\Result\RedirectFactory|\PHPUnit_Framework_MockObject_MockObject
31+
*/
32+
private $resultRedirectFactoryMock;
33+
34+
/**
35+
* @var \Magento\Backend\Model\View\Result\Redirect|\PHPUnit_Framework_MockObject_MockObject
36+
*/
37+
private $resultRedirectMock;
38+
39+
/**
40+
* @var \Magento\Framework\App\Request\Http|\PHPUnit_Framework_MockObject_MockObject
41+
*/
42+
private $requestMock;
43+
44+
/**
45+
* @var \Magento\Sales\Api\OrderRepositoryInterface|\PHPUnit_Framework_MockObject_MockObject
46+
*/
47+
private $orderRepositoryMock;
48+
49+
/**
50+
* @var \Magento\Framework\AuthorizationInterface|\PHPUnit_Framework_MockObject_MockObject
51+
*/
52+
private $authorizationMock;
53+
54+
/**
55+
* @var \Magento\Sales\Model\Order\Status\History|\PHPUnit_Framework_MockObject_MockObject
56+
*/
57+
private $statusHistoryCommentMock;
58+
59+
/**
60+
* @var \Magento\Framework\ObjectManagerInterface|\PHPUnit_Framework_MockObject_MockObject
61+
*/
62+
private $objectManagerMock;
63+
64+
/**
65+
* @inheritdoc
66+
*/
67+
protected function setUp()
68+
{
69+
$this->contextMock = $this->createMock(\Magento\Backend\App\Action\Context::class);
70+
$this->requestMock = $this->createMock(\Magento\Framework\App\Request\Http::class);
71+
$this->orderRepositoryMock = $this->createMock(\Magento\Sales\Api\OrderRepositoryInterface::class);
72+
$this->orderMock = $this->createMock(\Magento\Sales\Model\Order::class);
73+
$this->resultRedirectFactoryMock = $this->createMock(\Magento\Backend\Model\View\Result\RedirectFactory::class);
74+
$this->resultRedirectMock = $this->createMock(\Magento\Backend\Model\View\Result\Redirect::class);
75+
$this->authorizationMock = $this->createMock(\Magento\Framework\AuthorizationInterface::class);
76+
$this->statusHistoryCommentMock = $this->createMock(\Magento\Sales\Model\Order\Status\History::class);
77+
$this->objectManagerMock = $this->createMock(\Magento\Framework\ObjectManagerInterface::class);
78+
79+
$this->contextMock->expects($this->once())->method('getRequest')->willReturn($this->requestMock);
80+
81+
$objectManagerHelper = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
82+
$this->addCommentController = $objectManagerHelper->getObject(
83+
\Magento\Sales\Controller\Adminhtml\Order\AddComment::class,
84+
[
85+
'context' => $this->contextMock,
86+
'orderRepository' => $this->orderRepositoryMock,
87+
'_authorization' => $this->authorizationMock,
88+
'_objectManager' => $this->objectManagerMock,
89+
]
90+
);
91+
}
92+
93+
/**
94+
* Test for execute method with different data.
95+
*
96+
* @param array $historyData
97+
* @param bool $userHasResource
98+
* @param bool $expectedNotify
99+
*
100+
* @return void
101+
* @dataProvider executeWillNotifyCustomerDataProvider
102+
*/
103+
public function testExecuteWillNotifyCustomer(array $historyData, bool $userHasResource, bool $expectedNotify)
104+
{
105+
$orderId = 30;
106+
$this->requestMock->expects($this->once())->method('getParam')->with('order_id')->willReturn($orderId);
107+
$this->orderRepositoryMock->expects($this->once())
108+
->method('get')
109+
->willReturn($this->orderMock);
110+
$this->requestMock->expects($this->once())->method('getPost')->with('history')->willReturn($historyData);
111+
$this->authorizationMock->expects($this->any())->method('isAllowed')->willReturn($userHasResource);
112+
$this->orderMock->expects($this->once())
113+
->method('addStatusHistoryComment')
114+
->willReturn($this->statusHistoryCommentMock);
115+
$this->statusHistoryCommentMock->expects($this->once())->method('setIsCustomerNotified')->with($expectedNotify);
116+
$this->objectManagerMock->expects($this->once())->method('create')->willReturn(
117+
$this->createMock(\Magento\Sales\Model\Order\Email\Sender\OrderCommentSender::class)
118+
);
119+
120+
$this->addCommentController->execute();
121+
}
122+
123+
/**
124+
* Data provider for testExecuteWillNotifyCustomer method.
125+
*
126+
* @return array
127+
*/
128+
public function executeWillNotifyCustomerDataProvider(): array
129+
{
130+
return [
131+
'User Has Access - Notify True' => [
132+
'postData' => [
133+
'comment' => 'Great Product!',
134+
'is_customer_notified' => true,
135+
'status' => 'Processing',
136+
],
137+
'userHasResource' => true,
138+
'expectedNotify' => true,
139+
],
140+
'User Has Access - Notify False' => [
141+
'postData' => [
142+
'comment' => 'Great Product!',
143+
'is_customer_notified' => false,
144+
'status' => 'Processing',
145+
],
146+
'userHasResource' => true,
147+
'expectedNotify' => false,
148+
],
149+
'User Has Access - Notify Unset' => [
150+
'postData' => [
151+
'comment' => 'Great Product!',
152+
'status' => 'Processing',
153+
],
154+
'userHasResource' => true,
155+
'expectedNotify' => false,
156+
],
157+
'User No Access - Notify True' => [
158+
'postData' => [
159+
'comment' => 'Great Product!',
160+
'is_customer_notified' => true,
161+
'status' => 'Processing',
162+
],
163+
'userHasResource' => false,
164+
'expectedNotify' => false,
165+
],
166+
'User No Access - Notify False' => [
167+
'postData' => [
168+
'comment' => 'Great Product!',
169+
'is_customer_notified' => false,
170+
'status' => 'Processing',
171+
],
172+
'userHasResource' => false,
173+
'expectedNotify' => false,
174+
],
175+
'User No Access - Notify Unset' => [
176+
'postData' => [
177+
'comment' => 'Great Product!',
178+
'status' => 'Processing',
179+
],
180+
'userHasResource' => false,
181+
'expectedNotify' => false,
182+
],
183+
];
184+
}
185+
}

0 commit comments

Comments
 (0)