diff --git a/app/code/Magento/User/Block/User/Edit.php b/app/code/Magento/User/Block/User/Edit.php index f6c5b484861de36850d848e0eba2382d9688dade..116acd132a8487dbe56d933f307c40e62327bf6e 100644 --- a/app/code/Magento/User/Block/User/Edit.php +++ b/app/code/Magento/User/Block/User/Edit.php @@ -50,7 +50,7 @@ class Edit extends \Magento\Backend\Block\Widget\Form\Container $this->buttonList->update('save', 'label', __('Save User')); $this->buttonList->remove('delete'); - $objId = $this->getRequest()->getParam($this->_objectId); + $objId = (int)$this->getRequest()->getParam($this->_objectId); if (!empty($objId)) { $this->addButton( @@ -58,12 +58,9 @@ class Edit extends \Magento\Backend\Block\Widget\Form\Container [ 'label' => __('Delete User'), 'class' => 'delete', - 'onclick' => sprintf( - 'deleteConfirm("%s", "%s", %s)', - __('Are you sure you want to do this?'), - $this->getUrl('adminhtml/*/delete'), - json_encode(['data' => ['user_id' => $objId]]) - ), + 'data_attribute' => [ + 'role' => 'delete-user' + ] ] ); @@ -79,6 +76,44 @@ class Edit extends \Magento\Backend\Block\Widget\Form\Container } } + /** + * Returns message that is displayed for admin when he deletes user from the system. + * To see this message admin must do the following: + * - open user's account for editing; + * - type current user's password in the "Current User Identity Verification" field + * - click "Delete User" at top left part of the page; + * + * @return \Magento\Framework\Phrase + */ + public function getDeleteMessage() + { + return __('Are you sure you want to do this?'); + } + + /** + * Returns the URL that is used for user deletion. + * The following Action is executed if admin navigates to returned url + * Magento\User\Controller\Adminhtml\User\Delete + * + * @return string + */ + public function getDeleteUrl() + { + return $this->getUrl('adminhtml/*/delete'); + } + + /** + * This method is used to get the ID of the user who's account the Admin is editing. + * It can be used to determine the reason Admin opens the page: + * to create a new user account OR to edit the previously created user account + * + * @return int + */ + public function getObjectId() + { + return (int)$this->getRequest()->getParam($this->_objectId); + } + /** * @return \Magento\Framework\Phrase */ diff --git a/app/code/Magento/User/Controller/Adminhtml/User/Delete.php b/app/code/Magento/User/Controller/Adminhtml/User/Delete.php index d892c8533a29716b1206ba8ee8dbeaccfc3defce..3ff336eeecff87361e79f260f30f0b77cd2c1a03 100644 --- a/app/code/Magento/User/Controller/Adminhtml/User/Delete.php +++ b/app/code/Magento/User/Controller/Adminhtml/User/Delete.php @@ -6,6 +6,9 @@ */ namespace Magento\User\Controller\Adminhtml\User; +use Magento\User\Block\User\Edit\Tab\Main as UserEdit; +use Magento\Framework\Exception\AuthenticationException; + class Delete extends \Magento\User\Controller\Adminhtml\User { /** @@ -13,8 +16,10 @@ class Delete extends \Magento\User\Controller\Adminhtml\User */ public function execute() { + /** @var \Magento\User\Model\User */ $currentUser = $this->_objectManager->get(\Magento\Backend\Model\Auth\Session::class)->getUser(); $userId = (int)$this->getRequest()->getPost('user_id'); + if ($userId) { if ($currentUser->getId() == $userId) { $this->messageManager->addError(__('You cannot delete your own account.')); @@ -22,6 +27,11 @@ class Delete extends \Magento\User\Controller\Adminhtml\User return; } try { + $currentUserPassword = (string)$this->getRequest()->getPost(UserEdit::CURRENT_USER_PASSWORD_FIELD); + if (empty($currentUserPassword)) { + throw new AuthenticationException(__('You have entered an invalid password for current user.')); + } + $currentUser->performIdentityCheck($currentUserPassword); /** @var \Magento\User\Model\User $model */ $model = $this->_userFactory->create(); $model->setId($userId); diff --git a/app/code/Magento/User/Test/Unit/Controller/Adminhtml/User/DeleteTest.php b/app/code/Magento/User/Test/Unit/Controller/Adminhtml/User/DeleteTest.php new file mode 100644 index 0000000000000000000000000000000000000000..5e2d0d6bcfdbfdda7f7f8d7d36fb02609d187cfc --- /dev/null +++ b/app/code/Magento/User/Test/Unit/Controller/Adminhtml/User/DeleteTest.php @@ -0,0 +1,225 @@ +<?php +/** + * Copyright © 2016 Magento. All rights reserved. + * See COPYING.txt for license details. + */ + +namespace Magento\User\Test\Unit\Controller\Adminhtml\User; + +use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper; +use Magento\User\Block\User\Edit\Tab\Main as UserEdit; +use Magento\Backend\Model\Auth\Session as Session; +use Magento\Framework\Exception\AuthenticationException; + +/** + * Test class for \Magento\User\Controller\Adminhtml\User\Delete testing + * + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) + */ +class DeleteTest extends \PHPUnit_Framework_TestCase +{ + /** + * @var \Magento\User\Controller\Adminhtml\User\Delete + */ + private $controller; + + /** + * @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\App\RequestInterface + */ + private $requestMock; + + /** + * @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\App\ResponseInterface + */ + private $responseMock; + + /** + * @var \PHPUnit_Framework_MockObject_MockObject|Session + */ + private $authSessionMock; + + /** + * @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\ObjectManagerInterface + */ + private $objectManagerMock; + + /** + * @var \PHPUnit_Framework_MockObject_MockObject|\Magento\User\Model\UserFactory + */ + private $userFactoryMock; + + /** + * @var \PHPUnit_Framework_MockObject_MockObject|\Magento\User\Model\User + */ + private $userMock; + + /** + * @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\Message\ManagerInterface + */ + private $messageManagerMock; + + /** + * @return void + */ + protected function setUp() + { + $this->objectManagerMock = $this->getMockBuilder(\Magento\Framework\ObjectManager\ObjectManager::class) + ->disableOriginalConstructor() + ->setMethods(['get', 'create']) + ->getMock(); + + $this->responseMock = $this->getMockBuilder(\Magento\Framework\App\ResponseInterface::class) + ->disableOriginalConstructor() + ->setMethods(['setRedirect']) + ->getMockForAbstractClass(); + + $this->requestMock = $this->getMockBuilder(\Magento\Framework\App\RequestInterface::class) + ->disableOriginalConstructor() + ->setMethods(['getPost']) + ->getMockForAbstractClass(); + + $this->authSessionMock = $this->getMockBuilder(Session::class) + ->disableOriginalConstructor() + ->setMethods(['getUser']) + ->getMock(); + + $this->userMock = $this->getMockBuilder(\Magento\User\Model\User::class) + ->disableOriginalConstructor() + ->setMethods(['getId', 'performIdentityCheck', 'delete']) + ->getMock(); + + $this->userFactoryMock = $this->getMockBuilder(\Magento\User\Model\UserFactory::class) + ->disableOriginalConstructor() + ->setMethods(['create']) + ->getMock(); + + $this->messageManagerMock = $this->getMockBuilder(\Magento\Framework\Message\ManagerInterface::class) + ->disableOriginalConstructor() + ->getMock(); + + $objectManager = new ObjectManagerHelper($this); + $context = $objectManager->getObject( + \Magento\Backend\App\Action\Context::class, + [ + 'request' => $this->requestMock, + 'response' => $this->responseMock, + 'objectManager' => $this->objectManagerMock, + 'messageManager' => $this->messageManagerMock, + ] + ); + + $this->controller = $objectManager->getObject( + \Magento\User\Controller\Adminhtml\User\Delete::class, + [ + 'context' => $context, + 'userFactory' => $this->userFactoryMock, + ] + ); + } + + /** + * Test method \Magento\User\Controller\Adminhtml\User\Delete::execute + * + * @param string $currentUserPassword + * @param int $userId + * @param int $currentUserId + * @param string $resultMethod + * + * @dataProvider executeDataProvider + * @return void + * + */ + public function testExecute($currentUserPassword, $userId, $currentUserId, $resultMethod) + { + $currentUserMock = $this->userMock; + $this->authSessionMock->expects($this->any())->method('getUser')->will($this->returnValue($currentUserMock)); + + $currentUserMock->expects($this->any())->method('getId')->willReturn($currentUserId); + + $this->objectManagerMock + ->expects($this->any()) + ->method('get') + ->with(Session::class) + ->willReturn($this->authSessionMock); + + $this->requestMock->expects($this->any()) + ->method('getPost') + ->willReturnMap([ + ['user_id', $userId], + [UserEdit::CURRENT_USER_PASSWORD_FIELD, $currentUserPassword], + ]); + + $userMock = clone $currentUserMock; + + $this->userFactoryMock->expects($this->any())->method('create')->will($this->returnValue($userMock)); + $this->responseMock->expects($this->any())->method('setRedirect')->willReturnSelf(); + $this->userMock->expects($this->any())->method('delete')->willReturnSelf(); + $this->messageManagerMock->expects($this->once())->method($resultMethod); + + $this->controller->execute(); + } + + /** + * @return void + */ + public function testEmptyPasswordThrowsException() + { + try { + $currentUserId = 1; + $userId = 2; + + $currentUserMock = $this->userMock; + $this->authSessionMock->expects($this->any()) + ->method('getUser') + ->will($this->returnValue($currentUserMock)); + + $currentUserMock->expects($this->any())->method('getId')->willReturn($currentUserId); + + $this->objectManagerMock + ->expects($this->any()) + ->method('get') + ->with(Session::class) + ->willReturn($this->authSessionMock); + + $this->requestMock->expects($this->any()) + ->method('getPost') + ->willReturnMap([ + ['user_id', $userId], + [UserEdit::CURRENT_USER_PASSWORD_FIELD, ''], + ]); + + $this->controller->execute(); + } catch (AuthenticationException $e) { + $this->assertEquals($e->getMessage(), 'You have entered an invalid password for current user.'); + } + } + + /** + * Data Provider for execute method + * + * @return array + */ + public function executeDataProvider() + { + return [ + [ + 'currentUserPassword' => '123123q', + 'userId' => 1, + 'currentUserId' => 2, + 'resultMethod' => 'addSuccess', + ], + [ + 'currentUserPassword' => '123123q', + 'userId' => 0, + 'currentUserId' => 2, + 'resultMethod' => 'addError', + ], + [ + 'currentUserPassword' => '123123q', + 'userId' => 1, + 'currentUserId' => 1, + 'resultMethod' => 'addError', + ], + ]; + } +} diff --git a/app/code/Magento/User/view/adminhtml/requirejs-config.js b/app/code/Magento/User/view/adminhtml/requirejs-config.js index 0424073c4d3db45d39394a95a99b9c697c88367d..c5f45b480308e675153469a68d479bde79e2c03d 100644 --- a/app/code/Magento/User/view/adminhtml/requirejs-config.js +++ b/app/code/Magento/User/view/adminhtml/requirejs-config.js @@ -6,7 +6,8 @@ var config = { map: { '*': { - rolesTree: 'Magento_User/js/roles-tree' + rolesTree: 'Magento_User/js/roles-tree', + deleteUserAccount: 'Magento_User/js/delete-user-account' } } }; \ No newline at end of file diff --git a/app/code/Magento/User/view/adminhtml/templates/user/roles_grid_js.phtml b/app/code/Magento/User/view/adminhtml/templates/user/roles_grid_js.phtml index bac0dd0b3f869bd33ad5d7e12886f47daded00c2..9d403a898920c7e459d1bf657c19f2507089c5ca 100644 --- a/app/code/Magento/User/view/adminhtml/templates/user/roles_grid_js.phtml +++ b/app/code/Magento/User/view/adminhtml/templates/user/roles_grid_js.phtml @@ -71,3 +71,18 @@ require([ }); </script> + +<?php $editBlock = $block->getLayout()->getBlock('adminhtml.user.edit'); ?> +<?php if (is_object($editBlock)): ?> + <script type="text/x-magento-init"> + { + "[data-role=delete-user]" : { + "deleteUserAccount" : { + "message": "<?php echo $editBlock->escapeHtml($editBlock->getDeleteMessage()) ?>", + "url": "<?php /* @noEscape */ echo $editBlock->getDeleteUrl(); ?>", + "objId": "<?php echo $editBlock->escapeHtml($editBlock->getObjectId()) ?>" + } + } + } + </script> +<?php endif; ?> \ No newline at end of file diff --git a/app/code/Magento/User/view/adminhtml/web/js/delete-user-account.js b/app/code/Magento/User/view/adminhtml/web/js/delete-user-account.js new file mode 100644 index 0000000000000000000000000000000000000000..b326c7aeb20d06953baf406ecf339d40f5be8bc4 --- /dev/null +++ b/app/code/Magento/User/view/adminhtml/web/js/delete-user-account.js @@ -0,0 +1,28 @@ +/** + * Copyright © 2016 Magento. All rights reserved. + * See COPYING.txt for license details. + */ +define([ + 'jquery' +], function ($) { + 'use strict'; + + var postData; + + return function (params, elem) { + + elem.on('click', function () { + + postData = { + 'data': { + 'user_id': params.objId, + 'current_password': $('[name="current_password"]').val() + } + }; + + if ($.validator.validateElement($('[name="current_password"]'))) { + window.deleteConfirm(params.message, params.url, postData); + } + }); + }; +}); diff --git a/dev/tests/functional/tests/app/Magento/User/Test/Repository/User.xml b/dev/tests/functional/tests/app/Magento/User/Test/Repository/User.xml index 25b812bf77665d09877835e676fc5d11fdd1434a..dbb7a4aeb53bb667bf22e99e0198d1b0e3cab25c 100644 --- a/dev/tests/functional/tests/app/Magento/User/Test/Repository/User.xml +++ b/dev/tests/functional/tests/app/Magento/User/Test/Repository/User.xml @@ -41,5 +41,9 @@ <field name="current_password" xsi:type="string">%current_password%</field> <field name="is_active" xsi:type="string">Active</field> </dataset> + + <dataset name="system_admin"> + <field name="current_password" xsi:type="string">123123q</field> + </dataset> </repository> </config> diff --git a/dev/tests/functional/tests/app/Magento/User/Test/TestCase/DeleteAdminUserEntityTest.php b/dev/tests/functional/tests/app/Magento/User/Test/TestCase/DeleteAdminUserEntityTest.php index 57de56c7f56f7397282e1863ec43a3f42c6a9ec2..67ede3855e7a77b7da397a42c4de161289387b8f 100644 --- a/dev/tests/functional/tests/app/Magento/User/Test/TestCase/DeleteAdminUserEntityTest.php +++ b/dev/tests/functional/tests/app/Magento/User/Test/TestCase/DeleteAdminUserEntityTest.php @@ -101,11 +101,13 @@ class DeleteAdminUserEntityTest extends Injectable * * @param User $user * @param string $isDefaultUser + * @param User $systemAdmin * @return void */ public function testDeleteAdminUserEntity( User $user, - $isDefaultUser + $isDefaultUser, + User $systemAdmin = null ) { $filter = [ 'username' => $user->getUsername(), @@ -118,6 +120,7 @@ class DeleteAdminUserEntityTest extends Injectable } $this->userIndex->open(); $this->userIndex->getUserGrid()->searchAndOpen($filter); + $this->userEdit->getUserForm()->fill($systemAdmin); $this->userEdit->getPageActions()->delete(); $this->userEdit->getModalBlock()->acceptAlert(); } diff --git a/dev/tests/functional/tests/app/Magento/User/Test/TestCase/DeleteAdminUserEntityTest.xml b/dev/tests/functional/tests/app/Magento/User/Test/TestCase/DeleteAdminUserEntityTest.xml index 782c3243eaa6be76578a9fc9f8b670fb6920c4da..fd10c53dfd53393dfcf122888a4d7b9e1003228a 100644 --- a/dev/tests/functional/tests/app/Magento/User/Test/TestCase/DeleteAdminUserEntityTest.xml +++ b/dev/tests/functional/tests/app/Magento/User/Test/TestCase/DeleteAdminUserEntityTest.xml @@ -9,11 +9,13 @@ <testCase name="Magento\User\Test\TestCase\DeleteAdminUserEntityTest" summary="Delete Admin User" ticketId="MAGETWO-23416"> <variation name="DeleteAdminUserEntityTestVariation1"> <data name="isDefaultUser" xsi:type="string">0</data> + <data name="systemAdmin/dataset" xsi:type="string">system_admin</data> <constraint name="Magento\User\Test\Constraint\AssertImpossibleDeleteYourOwnAccount" /> <constraint name="Magento\User\Test\Constraint\AssertUserInGrid" /> </variation> <variation name="DeleteAdminUserEntityTestVariation2"> <data name="isDefaultUser" xsi:type="string">1</data> + <data name="systemAdmin/dataset" xsi:type="string">system_admin</data> <constraint name="Magento\User\Test\Constraint\AssertUserSuccessDeleteMessage" /> <constraint name="Magento\User\Test\Constraint\AssertUserNotInGrid" /> </variation>