Merge pull request #306 from magento-api/api-2.0.1patch

[API] 2.0.1 Patch Bugs

Author: Korshenko, Olexii(okorshenko)

app/code/Magento/Store/Model/Plugin/StoreCookie.php

@@ -52,29 +52,25 @@ public function __construct(
      * Delete cookie "store" if the store (a value in the cookie) does not exist or is inactive
      *
      * @param \Magento\Framework\App\FrontController $subject
-     * @param callable $proceed
      * @param \Magento\Framework\App\RequestInterface $request
-     * @return mixed
+     * @return void
      * @SuppressWarnings(PHPMD.UnusedFormalParameter)
      */
-    public function aroundDispatch(
+    public function beforeDispatch(
         \Magento\Framework\App\FrontController $subject,
-        \Closure $proceed,
         \Magento\Framework\App\RequestInterface $request
     ) {
-        $defaultStore = $this->storeManager->getDefaultStoreView();
         $storeCodeFromCookie = $this->storeCookieManager->getStoreCodeFromCookie();
         if ($storeCodeFromCookie) {
             try {
                 $this->storeRepository->getActiveStoreByCode($storeCodeFromCookie);
             } catch (StoreIsInactiveException $e) {
-                $this->storeCookieManager->deleteStoreCookie($defaultStore);
+                $this->storeCookieManager->deleteStoreCookie($this->storeManager->getDefaultStoreView());
             } catch (NoSuchEntityException $e) {
-                $this->storeCookieManager->deleteStoreCookie($defaultStore);
+                $this->storeCookieManager->deleteStoreCookie($this->storeManager->getDefaultStoreView());
             } catch (InvalidArgumentException $e) {
-                $this->storeCookieManager->deleteStoreCookie($defaultStore);
+                $this->storeCookieManager->deleteStoreCookie($this->storeManager->getDefaultStoreView());
             }
         }
-        return $proceed($request);
     }
 }

app/code/Magento/Store/Model/StoreResolver.php

@@ -45,6 +45,11 @@ class StoreResolver implements \Magento\Store\Api\StoreResolverInterface
      */
     protected $scopeCode;
 
+    /*
+     * @var \Magento\Framework\App\RequestInterface
+     */
+    protected $request;
+
     /**
      * @param \Magento\Store\Api\StoreRepositoryInterface $storeRepository
      * @param StoreCookieManagerInterface $storeCookieManager

app/code/Magento/Store/Test/Unit/Model/Plugin/StoreCookieTest.php

@@ -37,11 +37,6 @@ class StoreCookieTest extends \PHPUnit_Framework_TestCase
      */
     protected $storeMock;
 
-    /**
-     * @var \Closure
-     */
-    protected $closureMock;
-
     /**
      * @var \Magento\Framework\App\FrontController|\PHPUnit_Framework_MockObject_MockObject
      */
@@ -77,10 +72,6 @@ public function setUp()
             ->setMethods([])
             ->getMock();
 
-        $this->closureMock = function () {
-            return 'ExpectedValue';
-        };
-
         $this->subjectMock = $this->getMockBuilder('Magento\Framework\App\FrontController')
             ->disableOriginalConstructor()
             ->setMethods([])
@@ -106,7 +97,7 @@ public function setUp()
         );
     }
 
-    public function testAroundDispatchNoSuchEntity()
+    public function testBeforeDispatchNoSuchEntity()
     {
         $storeCode = 'store';
         $this->storeManagerMock->expects($this->once())->method('getDefaultStoreView')->willReturn($this->storeMock);
@@ -115,13 +106,10 @@ public function testAroundDispatchNoSuchEntity()
             ->method('getActiveStoreByCode')
             ->willThrowException(new NoSuchEntityException);
         $this->storeCookieManagerMock->expects($this->once())->method('deleteStoreCookie')->with($this->storeMock);
-        $this->assertEquals(
-            'ExpectedValue',
-            $this->plugin->aroundDispatch($this->subjectMock, $this->closureMock, $this->requestMock)
-        );
+        $this->plugin->beforeDispatch($this->subjectMock, $this->requestMock);
     }
 
-    public function testAroundDispatchStoreIsInactive()
+    public function testBeforeDispatchStoreIsInactive()
     {
         $storeCode = 'store';
         $this->storeManagerMock->expects($this->once())->method('getDefaultStoreView')->willReturn($this->storeMock);
@@ -130,13 +118,10 @@ public function testAroundDispatchStoreIsInactive()
             ->method('getActiveStoreByCode')
             ->willThrowException(new StoreIsInactiveException);
         $this->storeCookieManagerMock->expects($this->once())->method('deleteStoreCookie')->with($this->storeMock);
-        $this->assertEquals(
-            'ExpectedValue',
-            $this->plugin->aroundDispatch($this->subjectMock, $this->closureMock, $this->requestMock)
-        );
+        $this->plugin->beforeDispatch($this->subjectMock, $this->requestMock);
     }
 
-    public function testAroundDispatchInvalidArgument()
+    public function testBeforeDispatchInvalidArgument()
     {
         $storeCode = 'store';
         $this->storeManagerMock->expects($this->once())->method('getDefaultStoreView')->willReturn($this->storeMock);
@@ -145,22 +130,16 @@ public function testAroundDispatchInvalidArgument()
             ->method('getActiveStoreByCode')
             ->willThrowException(new InvalidArgumentException);
         $this->storeCookieManagerMock->expects($this->once())->method('deleteStoreCookie')->with($this->storeMock);
-        $this->assertEquals(
-            'ExpectedValue',
-            $this->plugin->aroundDispatch($this->subjectMock, $this->closureMock, $this->requestMock)
-        );
+        $this->plugin->beforeDispatch($this->subjectMock, $this->requestMock);
     }
 
-    public function testAroundDispatchNoStoreCookie()
+    public function testBeforeDispatchNoStoreCookie()
     {
         $storeCode = null;
-        $this->storeManagerMock->expects($this->once())->method('getDefaultStoreView')->willReturn($this->storeMock);
         $this->storeCookieManagerMock->expects($this->once())->method('getStoreCodeFromCookie')->willReturn($storeCode);
+        $this->storeManagerMock->expects($this->never())->method('getDefaultStoreView')->willReturn($this->storeMock);
         $this->storeRepositoryMock->expects($this->never())->method('getActiveStoreByCode');
         $this->storeCookieManagerMock->expects($this->never())->method('deleteStoreCookie')->with($this->storeMock);
-        $this->assertEquals(
-            'ExpectedValue',
-            $this->plugin->aroundDispatch($this->subjectMock, $this->closureMock, $this->requestMock)
-        );
+        $this->plugin->beforeDispatch($this->subjectMock, $this->requestMock);
     }
 }

app/code/Magento/Store/etc/di.xml

@@ -281,6 +281,11 @@
         <plugin name="install" type="Magento\Framework\Module\Plugin\DbStatusValidator" sortOrder="40"/>
         <plugin name="storeCookieValidate" type="Magento\Store\Model\Plugin\StoreCookie" sortOrder="10"/>
     </type>
+    <type name="Magento\Store\Model\Plugin\StoreCookie">
+        <arguments>
+            <argument name="storeManager" xsi:type="object">Magento\Store\Model\StoreManagerInterface\Proxy</argument>
+        </arguments>
+    </type>
     <type name="Magento\Framework\Module\Plugin\DbStatusValidator">
         <arguments>
             <argument name="cache" xsi:type="object">Magento\Framework\App\Cache\Type\Config</argument>

app/code/Magento/User/Model/ResourceModel/User.php

@@ -539,7 +539,7 @@ public function getOldPasswords($user, $retainLimit = 4)
         $userId = (int)$user->getId();
         $table = $this->getTable('admin_passwords');
 
-        // purge expired passwords, except that should retain
+        // purge expired passwords, except those which should be retained
         $retainPasswordIds = $this->getConnection()->fetchCol(
             $this->getConnection()
                 ->select()
@@ -556,7 +556,7 @@ public function getOldPasswords($user, $retainLimit = 4)
         }
         $this->getConnection()->delete($table, $where);
 
-        // now get all remained passwords
+        // get all remaining passwords
         return $this->getConnection()->fetchCol(
             $this->getConnection()
                 ->select()

app/code/Magento/User/Model/User.php

@@ -256,9 +256,9 @@ protected function _getValidationRulesBeforeSave()
     }
 
     /**
-     * Validate customer attribute values.
-     * For existing customer password + confirmation will be validated only when password is set
-     * (i.e. its change is requested)
+     * Validate admin user data.
+     *
+     * Existing user password confirmation will be validated only when password is set
      *
      * @return bool|string[]
      */
@@ -272,8 +272,35 @@ public function validate()
             return $validator->getMessages();
         }
 
-        return true;
+        return $this->validatePasswordChange();
+    }
+
+    /**
+     * Make sure admin password was changed.
+     *
+     * New password is compared to at least 4 previous passwords to prevent setting them again
+     *
+     * @return bool|string[]
+     */
+    protected function validatePasswordChange()
+    {
+        $password = $this->getPassword();
+        if ($password && !$this->getForceNewPassword() && $this->getId()) {
+            $errorMessage = __('Sorry, but this password has already been used. Please create another.');
+            // Check if password is equal to the current one
+            if ($this->_encryptor->isValidHash($password, $this->getOrigData('password'))) {
+                return [$errorMessage];
+            }
 
+            // Check whether password was used before
+            $passwordHash = $this->_encryptor->getHash($password, false);
+            foreach ($this->getResource()->getOldPasswords($this) as $oldPasswordHash) {
+                if ($passwordHash === $oldPasswordHash) {
+                    return [$errorMessage];
+                }
+            }
+        }
+        return true;
     }
 
     /**

app/code/Magento/User/Observer/Backend/CheckAdminPasswordChangeObserver.php

@@ -71,7 +71,7 @@ public function __construct(
     }
 
     /**
-     * Save new admin password
+     * Save current admin password to prevent its usage when changed in the future.
      *
      * @param EventObserver $observer
      * @return void
@@ -81,7 +81,7 @@ public function execute(EventObserver $observer)
         /* @var $user \Magento\User\Model\User */
         $user = $observer->getEvent()->getObject();
         if ($user->getId()) {
-            $password = $user->getNewPassword();
+            $password = $user->getCurrentPassword();
             $passwordLifetime = $this->observerConfig->getAdminPasswordLifetime();
             if ($passwordLifetime && $password && !$user->getForceNewPassword()) {
                 $passwordHash = $this->encryptor->getHash($password, false);