Skip to content

Commit 726928a

Browse files
RuslanKostiv1RuslanKostiv1
committed
MAGETWO-55849: Customer can be deleted without Merchant permissions verification
1 parent e3c222c commit 726928a

File tree

7 files changed

+257
-4
lines changed

7 files changed

+257
-4
lines changed

‎app/code/Magento/User/Block/User/Edit.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ protected function _construct()
5050
$this->buttonList->update('save', 'label', __('Save User'));
5151
$this->buttonList->remove('delete');
5252

53-
$objId = $this->getRequest()->getParam($this->_objectId);
53+
$objId = (int)$this->getRequest()->getParam($this->_objectId);
5454

5555
if (!empty($objId)) {
5656
$this->addButton(
@@ -59,10 +59,10 @@ protected function _construct()
5959
'label' => __('Delete User'),
6060
'class' => 'delete',
6161
'onclick' => sprintf(
62-
'deleteConfirm("%s", "%s", %s)',
62+
'deleteUserAccount("%s", "%s", %s)',
6363
__('Are you sure you want to do this?'),
6464
$this->getUrl('adminhtml/*/delete'),
65-
json_encode(['data' => ['user_id' => $objId]])
65+
$objId
6666
),
6767
]
6868
);

‎app/code/Magento/User/Controller/Adminhtml/User/Delete.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,22 +6,32 @@
66
*/
77
namespace Magento\User\Controller\Adminhtml\User;
88

9+
use Magento\User\Block\User\Edit\Tab\Main as UserEdit;
10+
use Magento\Framework\Exception\AuthenticationException;
11+
912
class Delete extends \Magento\User\Controller\Adminhtml\User
1013
{
1114
/**
1215
* @return void
1316
*/
1417
public function execute()
1518
{
19+
/** @var \Magento\User\Model\User */
1620
$currentUser = $this->_objectManager->get(\Magento\Backend\Model\Auth\Session::class)->getUser();
1721
$userId = (int)$this->getRequest()->getPost('user_id');
22+
1823
if ($userId) {
1924
if ($currentUser->getId() == $userId) {
2025
$this->messageManager->addError(__('You cannot delete your own account.'));
2126
$this->_redirect('adminhtml/*/edit', ['user_id' => $userId]);
2227
return;
2328
}
2429
try {
30+
$currentUserPassword = (string)$this->getRequest()->getPost(UserEdit::CURRENT_USER_PASSWORD_FIELD);
31+
if (empty($currentUserPassword)) {
32+
throw new AuthenticationException(__('You have entered an invalid password for current user.'));
33+
}
34+
$currentUser->performIdentityCheck($currentUserPassword);
2535
/** @var \Magento\User\Model\User $model */
2636
$model = $this->_userFactory->create();
2737
$model->setId($userId);

‎app/code/Magento/User/Test/Unit/Controller/Adminhtml/User/DeleteTest.php

Lines changed: 225 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,225 @@
1+
<?php
2+
/**
3+
* Copyright © 2016 Magento. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
namespace Magento\User\Test\Unit\Controller\Adminhtml\User;
8+
9+
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper;
10+
use Magento\User\Block\User\Edit\Tab\Main as UserEdit;
11+
use Magento\Backend\Model\Auth\Session as Session;
12+
use Magento\Framework\Exception\AuthenticationException;
13+
14+
/**
15+
* Test class for \Magento\User\Controller\Adminhtml\User\Delete testing
16+
*
17+
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
18+
*/
19+
class DeleteTest extends \PHPUnit_Framework_TestCase
20+
{
21+
/**
22+
* @var \Magento\User\Controller\Adminhtml\User\Delete
23+
*/
24+
private $controller;
25+
26+
/**
27+
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\App\RequestInterface
28+
*/
29+
private $requestMock;
30+
31+
/**
32+
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\App\ResponseInterface
33+
*/
34+
private $responseMock;
35+
36+
/**
37+
* @var \PHPUnit_Framework_MockObject_MockObject|Session
38+
*/
39+
private $authSessionMock;
40+
41+
/**
42+
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\ObjectManagerInterface
43+
*/
44+
private $objectManagerMock;
45+
46+
/**
47+
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\User\Model\UserFactory
48+
*/
49+
private $userFactoryMock;
50+
51+
/**
52+
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\User\Model\User
53+
*/
54+
private $userMock;
55+
56+
/**
57+
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\Message\ManagerInterface
58+
*/
59+
private $messageManagerMock;
60+
61+
/**
62+
* @return void
63+
*/
64+
protected function setUp()
65+
{
66+
$this->objectManagerMock = $this->getMockBuilder(\Magento\Framework\ObjectManager\ObjectManager::class)
67+
->disableOriginalConstructor()
68+
->setMethods(['get', 'create'])
69+
->getMock();
70+
71+
$this->responseMock = $this->getMockBuilder(\Magento\Framework\App\ResponseInterface::class)
72+
->disableOriginalConstructor()
73+
->setMethods(['setRedirect'])
74+
->getMockForAbstractClass();
75+
76+
$this->requestMock = $this->getMockBuilder(\Magento\Framework\App\RequestInterface::class)
77+
->disableOriginalConstructor()
78+
->setMethods(['getPost'])
79+
->getMockForAbstractClass();
80+
81+
$this->authSessionMock = $this->getMockBuilder(Session::class)
82+
->disableOriginalConstructor()
83+
->setMethods(['getUser'])
84+
->getMock();
85+
86+
$this->userMock = $this->getMockBuilder(\Magento\User\Model\User::class)
87+
->disableOriginalConstructor()
88+
->setMethods(['getId', 'performIdentityCheck', 'delete'])
89+
->getMock();
90+
91+
$this->userFactoryMock = $this->getMockBuilder(\Magento\User\Model\UserFactory::class)
92+
->disableOriginalConstructor()
93+
->setMethods(['create'])
94+
->getMock();
95+
96+
$this->messageManagerMock = $this->getMockBuilder(\Magento\Framework\Message\ManagerInterface::class)
97+
->disableOriginalConstructor()
98+
->getMock();
99+
100+
$objectManager = new ObjectManagerHelper($this);
101+
$context = $objectManager->getObject(
102+
\Magento\Backend\App\Action\Context::class,
103+
[
104+
'request' => $this->requestMock,
105+
'response' => $this->responseMock,
106+
'objectManager' => $this->objectManagerMock,
107+
'messageManager' => $this->messageManagerMock,
108+
]
109+
);
110+
111+
$this->controller = $objectManager->getObject(
112+
\Magento\User\Controller\Adminhtml\User\Delete::class,
113+
[
114+
'context' => $context,
115+
'userFactory' => $this->userFactoryMock,
116+
]
117+
);
118+
}
119+
120+
/**
121+
* Test method \Magento\User\Controller\Adminhtml\User\Delete::execute
122+
*
123+
* @param string $currentUserPassword
124+
* @param int $userId
125+
* @param int $currentUserId
126+
* @param string $resultMethod
127+
*
128+
* @dataProvider executeDataProvider
129+
* @return void
130+
*
131+
*/
132+
public function testExecute($currentUserPassword, $userId, $currentUserId, $resultMethod)
133+
{
134+
$currentUserMock = $this->userMock;
135+
$this->authSessionMock->expects($this->any())->method('getUser')->will($this->returnValue($currentUserMock));
136+
137+
$currentUserMock->expects($this->any())->method('getId')->willReturn($currentUserId);
138+
139+
$this->objectManagerMock
140+
->expects($this->any())
141+
->method('get')
142+
->with(Session::class)
143+
->willReturn($this->authSessionMock);
144+
145+
$this->requestMock->expects($this->any())
146+
->method('getPost')
147+
->willReturnMap([
148+
['user_id', $userId],
149+
[UserEdit::CURRENT_USER_PASSWORD_FIELD, $currentUserPassword],
150+
]);
151+
152+
$userMock = clone $currentUserMock;
153+
154+
$this->userFactoryMock->expects($this->any())->method('create')->will($this->returnValue($userMock));
155+
$this->responseMock->expects($this->any())->method('setRedirect')->willReturnSelf();
156+
$this->userMock->expects($this->any())->method('delete')->willReturnSelf();
157+
$this->messageManagerMock->expects($this->once())->method($resultMethod);
158+
159+
$this->controller->execute();
160+
}
161+
162+
/**
163+
* @return void
164+
*/
165+
public function testEmptyPasswordThrowsException()
166+
{
167+
try {
168+
$currentUserId = 1;
169+
$userId = 2;
170+
171+
$currentUserMock = $this->userMock;
172+
$this->authSessionMock->expects($this->any())
173+
->method('getUser')
174+
->will($this->returnValue($currentUserMock));
175+
176+
$currentUserMock->expects($this->any())->method('getId')->willReturn($currentUserId);
177+
178+
$this->objectManagerMock
179+
->expects($this->any())
180+
->method('get')
181+
->with(Session::class)
182+
->willReturn($this->authSessionMock);
183+
184+
$this->requestMock->expects($this->any())
185+
->method('getPost')
186+
->willReturnMap([
187+
['user_id', $userId],
188+
[UserEdit::CURRENT_USER_PASSWORD_FIELD, ''],
189+
]);
190+
191+
$this->controller->execute();
192+
} catch (AuthenticationException $e) {
193+
$this->assertEquals($e->getMessage(), 'You have entered an invalid password for current user.');
194+
}
195+
}
196+
197+
/**
198+
* Data Provider for execute method
199+
*
200+
* @return array
201+
*/
202+
public function executeDataProvider()
203+
{
204+
return [
205+
[
206+
'currentUserPassword' => '123123q',
207+
'userId' => 1,
208+
'currentUserId' => 2,
209+
'resultMethod' => 'addSuccess',
210+
],
211+
[
212+
'currentUserPassword' => '123123q',
213+
'userId' => 0,
214+
'currentUserId' => 2,
215+
'resultMethod' => 'addError',
216+
],
217+
[
218+
'currentUserPassword' => '123123q',
219+
'userId' => 1,
220+
'currentUserId' => 1,
221+
'resultMethod' => 'addError',
222+
],
223+
];
224+
}
225+
}

‎app/code/Magento/User/view/adminhtml/templates/user/roles_grid_js.phtml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,4 +70,13 @@ require([
7070
<?php endif; ?>
7171

7272
});
73+
function deleteUserAccount(message, url, objId) {
74+
if (jQuery.validator.validateElement(jQuery('[name="current_password"]'))) {
75+
postData = {'data' : {
76+
'user_id': objId,
77+
'current_password': jQuery('[name="current_password"]').val()
78+
}}
79+
deleteConfirm(message, url, postData);
80+
}
81+
}
7382
</script>

‎dev/tests/functional/tests/app/Magento/User/Test/Repository/User.xml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,5 +41,9 @@
4141
<field name="current_password" xsi:type="string">%current_password%</field>
4242
<field name="is_active" xsi:type="string">Active</field>
4343
</dataset>
44+
45+
<dataset name="system_admin">
46+
<field name="current_password" xsi:type="string">123123q</field>
47+
</dataset>
4448
</repository>
4549
</config>

‎dev/tests/functional/tests/app/Magento/User/Test/TestCase/DeleteAdminUserEntityTest.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,11 +101,13 @@ public function __inject(
101101
*
102102
* @param User $user
103103
* @param string $isDefaultUser
104+
* @param User $systemAdmin
104105
* @return void
105106
*/
106107
public function testDeleteAdminUserEntity(
107108
User $user,
108-
$isDefaultUser
109+
$isDefaultUser,
110+
User $systemAdmin = null
109111
) {
110112
$filter = [
111113
'username' => $user->getUsername(),
@@ -118,6 +120,7 @@ public function testDeleteAdminUserEntity(
118120
}
119121
$this->userIndex->open();
120122
$this->userIndex->getUserGrid()->searchAndOpen($filter);
123+
$this->userEdit->getUserForm()->fill($systemAdmin);
121124
$this->userEdit->getPageActions()->delete();
122125
$this->userEdit->getModalBlock()->acceptAlert();
123126
}

‎dev/tests/functional/tests/app/Magento/User/Test/TestCase/DeleteAdminUserEntityTest.xml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,13 @@
99
<testCase name="Magento\User\Test\TestCase\DeleteAdminUserEntityTest" summary="Delete Admin User" ticketId="MAGETWO-23416">
1010
<variation name="DeleteAdminUserEntityTestVariation1">
1111
<data name="isDefaultUser" xsi:type="string">0</data>
12+
<data name="systemAdmin/dataset" xsi:type="string">system_admin</data>
1213
<constraint name="Magento\User\Test\Constraint\AssertImpossibleDeleteYourOwnAccount" />
1314
<constraint name="Magento\User\Test\Constraint\AssertUserInGrid" />
1415
</variation>
1516
<variation name="DeleteAdminUserEntityTestVariation2">
1617
<data name="isDefaultUser" xsi:type="string">1</data>
18+
<data name="systemAdmin/dataset" xsi:type="string">system_admin</data>
1719
<constraint name="Magento\User\Test\Constraint\AssertUserSuccessDeleteMessage" />
1820
<constraint name="Magento\User\Test\Constraint\AssertUserNotInGrid" />
1921
</variation>

0 commit comments

Comments
 (0)