Merge pull request #363 from magento-fearless-kiwis/FearlessKiwis-MAGETWO-57065-admin-stuck-on-changing-password-2.0

Oleksii Korshenko · web-flow · commit 0b8db7e40647 · 2016-09-09T14:11:34.000-05:00
Fixed issue: - MAGETWO-57065: [Github] admin stuck on " It's time to change your password." #4331- fix for 2.0
diff --git a/app/code/Magento/Backend/Controller/Adminhtml/System/Account/Save.php b/app/code/Magento/Backend/Controller/Adminhtml/System/Account/Save.php
@@ -52,12 +52,19 @@ public function execute()
                 $user->setPassword($password);
                 $user->setPasswordConfirmation($passwordConfirmation);
             }
-            $user->save();
-            /** Send password reset email notification only when password was changed */
-            if ($password !== '') {
-                $user->sendPasswordResetNotificationEmail();
+            $errors = $user->validate();
+            if ($errors !== true && !empty($errors)) {
+                foreach ($errors as $error) {
+                    $this->messageManager->addError($error);
+                }
+            } else {
+                $user->save();
+                /** Send password reset email notification only when password was changed */
+                if ($password !== '') {
+                    $user->sendPasswordResetNotificationEmail();
+                }
+                $this->messageManager->addSuccess(__('You saved the account.'));
             }
-            $this->messageManager->addSuccess(__('You saved the account.'));
         } catch (ValidatorException $e) {
             $this->messageManager->addMessages($e->getMessages());
             if ($e->getMessage()) {
diff --git a/app/code/Magento/Backend/Test/Unit/Controller/Adminhtml/System/Account/SaveTest.php b/app/code/Magento/Backend/Test/Unit/Controller/Adminhtml/System/Account/SaveTest.php
@@ -76,7 +76,10 @@ protected function setUp()
         $this->_userMock = $this->getMockBuilder('Magento\User\Model\User')
             ->disableOriginalConstructor()
             ->setMethods(
-                ['load', 'save', 'sendPasswordResetNotificationEmail', 'verifyIdentity', '__sleep', '__wakeup']
+                [
+                    'load', 'save', 'sendPasswordResetNotificationEmail',
+                    'verifyIdentity', 'validate', '__sleep', '__wakeup'
+                ]
             )
             ->getMock();
 
@@ -193,6 +196,7 @@ public function testSaveAction()
 
         $this->_userMock->expects($this->once())->method('save');
         $this->_userMock->expects($this->once())->method('verifyIdentity')->will($this->returnValue(true));
+        $this->_userMock->expects($this->once())->method('validate')->willReturn(true);
         $this->_userMock->expects($this->once())->method('sendPasswordResetNotificationEmail');
 
         $this->_requestMock->setParams($requestParams);
diff --git a/app/code/Magento/User/Controller/Adminhtml/Auth/ResetPasswordPost.php b/app/code/Magento/User/Controller/Adminhtml/Auth/ResetPasswordPost.php
@@ -40,11 +40,22 @@ public function execute()
         $user->setRpToken(null);
         $user->setRpTokenCreatedAt(null);
         try {
-            $user->save();
-            $this->messageManager->addSuccess(__('You updated your password.'));
-            $this->getResponse()->setRedirect(
-                $this->_objectManager->get('Magento\Backend\Helper\Data')->getHomePageUrl()
-            );
+            $errors = $user->validate();
+            if ($errors !== true && !empty($errors)) {
+                foreach ($errors as $error) {
+                    $this->messageManager->addError($error);
+                    $this->_redirect(
+                        'adminhtml/auth/resetpassword',
+                        ['_nosecret' => true, '_query' => ['id' => $userId, 'token' => $passwordResetToken]]
+                    );
+                }
+            } else {
+                $user->save();
+                $this->messageManager->addSuccess(__('You updated your password.'));
+                $this->getResponse()->setRedirect(
+                    $this->_objectManager->get(\Magento\Backend\Helper\Data::class)->getHomePageUrl()
+                );
+            }
         } catch (\Magento\Framework\Validator\Exception $exception) {
             $this->messageManager->addMessages($exception->getMessages());
             $this->_redirect(
diff --git a/app/code/Magento/User/Model/User.php b/app/code/Magento/User/Model/User.php
@@ -293,9 +293,8 @@ protected function validatePasswordChange()
             }
 
             // Check whether password was used before
-            $passwordHash = $this->_encryptor->getHash($password, false);
             foreach ($this->getResource()->getOldPasswords($this) as $oldPasswordHash) {
-                if ($passwordHash === $oldPasswordHash) {
+                if ($this->_encryptor->isValidHash($password, $oldPasswordHash)) {
                     return [$errorMessage];
                 }
             }
diff --git a/app/code/Magento/User/Observer/Backend/TrackAdminNewPasswordObserver.php b/app/code/Magento/User/Observer/Backend/TrackAdminNewPasswordObserver.php
@@ -35,13 +35,6 @@ class TrackAdminNewPasswordObserver implements ObserverInterface
      */
     protected $authSession;
 
-    /**
-     * Encryption model
-     *
-     * @var \Magento\Framework\Encryption\EncryptorInterface
-     */
-    protected $encryptor;
-
     /**
      * Message manager interface
      *
@@ -53,20 +46,17 @@ class TrackAdminNewPasswordObserver implements ObserverInterface
      * @param \Magento\User\Model\Backend\Config\ObserverConfig $observerConfig
      * @param \Magento\User\Model\ResourceModel\User $userResource
      * @param \Magento\Backend\Model\Auth\Session $authSession
-     * @param \Magento\Framework\Encryption\EncryptorInterface $encryptor
      * @param \Magento\Framework\Message\ManagerInterface $messageManager
      */
     public function __construct(
         \Magento\User\Model\Backend\Config\ObserverConfig $observerConfig,
         \Magento\User\Model\ResourceModel\User $userResource,
         \Magento\Backend\Model\Auth\Session $authSession,
-        \Magento\Framework\Encryption\EncryptorInterface $encryptor,
         \Magento\Framework\Message\ManagerInterface $messageManager
     ) {
         $this->observerConfig = $observerConfig;
         $this->userResource = $userResource;
         $this->authSession = $authSession;
-        $this->encryptor = $encryptor;
         $this->messageManager = $messageManager;
     }
 
@@ -81,10 +71,9 @@ public function execute(EventObserver $observer)
         /* @var $user \Magento\User\Model\User */
         $user = $observer->getEvent()->getObject();
         if ($user->getId()) {
-            $password = $user->getCurrentPassword();
+            $passwordHash = $user->getPassword();
             $passwordLifetime = $this->observerConfig->getAdminPasswordLifetime();
-            if ($passwordLifetime && $password && !$user->getForceNewPassword()) {
-                $passwordHash = $this->encryptor->getHash($password, false);
+            if ($passwordLifetime && $passwordHash && !$user->getForceNewPassword()) {
                 $this->userResource->trackPassword($user, $passwordHash, $passwordLifetime);
                 $this->messageManager->getMessages()->deleteMessageByIdentifier('magento_user_password_expired');
                 $this->authSession->unsPciAdminUserIsPasswordExpired();
diff --git a/app/code/Magento/User/Test/Unit/Model/UserTest.php b/app/code/Magento/User/Test/Unit/Model/UserTest.php
@@ -658,15 +658,9 @@ public function testCheckPasswordChangeEqualToPrevious()
         $this->model->setPassword($newPassword)
             ->setId(1)
             ->setOrigData('password', $oldPassword);
-        $this->encryptorMock->expects($this->once())
+        $this->encryptorMock->expects($this->atLeastOnce())
             ->method('isValidHash')
-            ->with($newPassword, $oldPassword)
-            ->willReturn(false);
-
-        $this->encryptorMock->expects($this->once())
-            ->method('getHash')
-            ->with($newPassword, false)
-            ->willReturn($newPasswordHash);
+            ->will($this->onConsecutiveCalls(false, true));
 
         $this->resourceMock->expects($this->once())->method('getOldPasswords')->willReturn(['hash1', $newPasswordHash]);
 
@@ -690,20 +684,13 @@ public function testCheckPasswordChangeValid()
         $validatorMock->expects($this->once())->method('isValid')->willReturn(true);
 
         $newPassword = "NEWmYn3wpassw0rd";
-        $newPasswordHash = "new password hash";
         $oldPassword = "OLDmYn3wpassw0rd";
         $this->model->setPassword($newPassword)
             ->setId(1)
             ->setOrigData('password', $oldPassword);
-        $this->encryptorMock->expects($this->once())
+        $this->encryptorMock->expects($this->atLeastOnce())
             ->method('isValidHash')
-            ->with($newPassword, $oldPassword)
-            ->willReturn(false);
-
-        $this->encryptorMock->expects($this->once())
-            ->method('getHash')
-            ->with($newPassword, false)
-            ->willReturn($newPasswordHash);
+            ->will($this->onConsecutiveCalls(false, false, false));
 
         $this->resourceMock->expects($this->once())->method('getOldPasswords')->willReturn(['hash1', 'hash2']);
 
diff --git a/app/code/Magento/User/Test/Unit/Observer/Backend/TrackAdminNewPasswordObserverTest.php b/app/code/Magento/User/Test/Unit/Observer/Backend/TrackAdminNewPasswordObserverTest.php
@@ -24,9 +24,6 @@ class TrackAdminNewPasswordObserverTest extends \PHPUnit_Framework_TestCase
     /** @var \Magento\Backend\Model\Auth\Session|\PHPUnit_Framework_MockObject_MockObject */
     protected $authSessionMock;
 
-    /** @var \Magento\Framework\Encryption\EncryptorInterface|\PHPUnit_Framework_MockObject_MockObject */
-    protected $encryptorMock;
-
     /** @var \Magento\Framework\Message\ManagerInterface|\PHPUnit_Framework_MockObject_MockObject */
     protected $managerInterfaceMock;
 
@@ -57,11 +54,6 @@ public function setUp()
                 ]
             )->getMock();
 
-        $this->encryptorMock = $this->getMockBuilder('\Magento\Framework\Encryption\EncryptorInterface')
-            ->disableOriginalConstructor()
-            ->setMethods([])
-            ->getMock();
-
         $this->managerInterfaceMock = $this->getMockBuilder('Magento\Framework\Message\ManagerInterface')
             ->disableOriginalConstructor()
             ->setMethods([])
@@ -82,7 +74,6 @@ public function setUp()
                 'observerConfig' => $this->observerConfig,
                 'userResource' => $this->userMock,
                 'authSession' => $this->authSessionMock,
-                'encryptor' => $this->encryptorMock,
                 'messageManager' => $this->managerInterfaceMock,
             ]
         );
@@ -91,7 +82,6 @@ public function setUp()
     public function testTrackAdminPassword()
     {
         $newPW = "mYn3wpassw0rd";
-        $oldPW = "notsecure";
         $uid = 123;
         /** @var \Magento\Framework\Event\Observer|\PHPUnit_Framework_MockObject_MockObject $eventObserverMock */
         $eventObserverMock = $this->getMockBuilder('Magento\Framework\Event\Observer')
@@ -108,19 +98,18 @@ public function testTrackAdminPassword()
         /** @var \Magento\User\Model\User|\PHPUnit_Framework_MockObject_MockObject $userMock */
         $userMock = $this->getMockBuilder('Magento\User\Model\User')
             ->disableOriginalConstructor()
-            ->setMethods(['getId', 'getCurrentPassword', 'getForceNewPassword'])
+            ->setMethods(['getId', 'getPassword', 'getForceNewPassword'])
             ->getMock();
 
         $eventObserverMock->expects($this->once())->method('getEvent')->willReturn($eventMock);
         $eventMock->expects($this->once())->method('getObject')->willReturn($userMock);
         $userMock->expects($this->once())->method('getId')->willReturn($uid);
-        $userMock->expects($this->once())->method('getCurrentPassword')->willReturn($newPW);
+        $userMock->expects($this->once())->method('getPassword')->willReturn($newPW);
         $this->configInterfaceMock
             ->expects($this->atLeastOnce())
             ->method('getValue')
             ->willReturn(1);
         $userMock->expects($this->once())->method('getForceNewPassword')->willReturn(false);
-        $this->encryptorMock->expects($this->once())->method('getHash')->willReturn(md5($oldPW));
 
         /** @var \Magento\Framework\Message\Collection|\PHPUnit_Framework_MockObject_MockObject $collectionMock */
         $collectionMock = $this->getMockBuilder('Magento\Framework\Message\Collection')

Original file line number	Diff line number	Diff line change
`@@ -293,9 +293,8 @@ protected function validatePasswordChange()`
`293`	`293`	`}`
`294`	`294`
`295`	`295`	`// Check whether password was used before`
`296`		`- $passwordHash = $this->_encryptor->getHash($password, false);`
`297`	`296`	`foreach ($this->getResource()->getOldPasswords($this) as $oldPasswordHash) {`
`298`		`- if ($passwordHash === $oldPasswordHash) {`
	`297`	`+ if ($this->_encryptor->isValidHash($password, $oldPasswordHash)) {`
`299`	`298`	`return [$errorMessage];`
`300`	`299`	`}`
`301`	`300`	`}`