Skip to content

Commit 1f7c39a

Browse files
le0n4egle0n4eg
committed
MAGETWO-55849: Customer can be deleted without Merchant permissions verification
2 parents 3853022 + f8a4e64 commit 1f7c39a

File tree

9 files changed

+332
-9
lines changed

9 files changed

+332
-9
lines changed

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

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,20 +50,17 @@ 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(
5757
'delete',
5858
[
5959
'label' => __('Delete User'),
6060
'class' => 'delete',
61-
'onclick' => sprintf(
62-
'deleteConfirm("%s", "%s", %s)',
63-
__('Are you sure you want to do this?'),
64-
$this->getUrl('adminhtml/*/delete'),
65-
json_encode(['data' => ['user_id' => $objId]])
66-
),
61+
'data_attribute' => [
62+
'role' => 'delete-user'
63+
]
6764
]
6865
);
6966

@@ -79,6 +76,44 @@ protected function _construct()
7976
}
8077
}
8178

79+
/**
80+
* Returns message that is displayed for admin when he deletes user from the system.
81+
* To see this message admin must do the following:
82+
* - open user's account for editing;
83+
* - type current user's password in the "Current User Identity Verification" field
84+
* - click "Delete User" at top left part of the page;
85+
*
86+
* @return \Magento\Framework\Phrase
87+
*/
88+
public function getDeleteMessage()
89+
{
90+
return __('Are you sure you want to do this?');
91+
}
92+
93+
/**
94+
* Returns the URL that is used for user deletion.
95+
* The following Action is executed if admin navigates to returned url
96+
* Magento\User\Controller\Adminhtml\User\Delete
97+
*
98+
* @return string
99+
*/
100+
public function getDeleteUrl()
101+
{
102+
return $this->getUrl('adminhtml/*/delete');
103+
}
104+
105+
/**
106+
* This method is used to get the ID of the user who's account the Admin is editing.
107+
* It can be used to determine the reason Admin opens the page:
108+
* to create a new user account OR to edit the previously created user account
109+
*
110+
* @return int
111+
*/
112+
public function getObjectId()
113+
{
114+
return (int)$this->getRequest()->getParam($this->_objectId);
115+
}
116+
82117
/**
83118
* @return \Magento\Framework\Phrase
84119
*/

‎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/requirejs-config.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66
var config = {
77
map: {
88
'*': {
9-
rolesTree: 'Magento_User/js/roles-tree'
9+
rolesTree: 'Magento_User/js/roles-tree',
10+
deleteUserAccount: 'Magento_User/js/delete-user-account'
1011
}
1112
}
1213
};

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,3 +71,18 @@ require([
7171

7272
});
7373
</script>
74+
75+
<?php $editBlock = $block->getLayout()->getBlock('adminhtml.user.edit'); ?>
76+
<?php if (is_object($editBlock)): ?>
77+
<script type="text/x-magento-init">
78+
{
79+
"[data-role=delete-user]" : {
80+
"deleteUserAccount" : {
81+
"message": "<?php echo $editBlock->escapeHtml($editBlock->getDeleteMessage()) ?>",
82+
"url": "<?php /* @noEscape */ echo $editBlock->getDeleteUrl(); ?>",
83+
"objId": "<?php echo $editBlock->escapeHtml($editBlock->getObjectId()) ?>"
84+
}
85+
}
86+
}
87+
</script>
88+
<?php endif; ?>

‎app/code/Magento/User/view/adminhtml/web/js/delete-user-account.js

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/**
2+
* Copyright © 2016 Magento. All rights reserved.
3+
* See COPYING.txt for license details.
4+
*/
5+
define([
6+
'jquery'
7+
], function ($) {
8+
'use strict';
9+
10+
var postData;
11+
12+
return function (params, elem) {
13+
14+
elem.on('click', function () {
15+
16+
postData = {
17+
'data': {
18+
'user_id': params.objId,
19+
'current_password': $('[name="current_password"]').val()
20+
}
21+
};
22+
23+
if ($.validator.validateElement($('[name="current_password"]'))) {
24+
window.deleteConfirm(params.message, params.url, postData);
25+
}
26+
});
27+
};
28+
});

‎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>

0 commit comments

Comments
 (0)