Merge pull request #4419 from magento-tsg/2.2.10-develop-pr101

[TSG] Fixes for 2.2.10 (pr101) (2.2.10-develop)

Author: zakdma

app/code/Magento/CatalogImportExport/Model/Import/Uploader.php

@@ -6,6 +6,8 @@
 namespace Magento\CatalogImportExport\Model\Import;
 
 use Magento\Framework\App\Filesystem\DirectoryList;
+use Magento\Framework\Exception\ValidatorException;
+use Magento\Framework\Filesystem\Directory\ReadInterface;
 use Magento\Framework\Filesystem\DriverPool;
 use Magento\Framework\App\ObjectManager;
 
@@ -18,7 +20,6 @@
  */
 class Uploader extends \Magento\MediaStorage\Model\File\Uploader
 {
-
     /**
      * HTTP scheme
      * used to compare against the filename and select the proper DriverPool adapter
@@ -103,6 +104,11 @@ class Uploader extends \Magento\MediaStorage\Model\File\Uploader
      */
     protected $_coreFileStorage;
 
+    /**
+     * @var \Magento\Framework\Filesystem
+     */
+    private $filesystem;
+
     /**
      * Instance of random data generator.
      *
@@ -115,6 +121,11 @@ class Uploader extends \Magento\MediaStorage\Model\File\Uploader
      */
     private $directoryResolver;
 
+    /**
+     * @var \Magento\Framework\Filesystem\Directory\ReadFactory
+     */
+    private $directoryReadFactory;
+
     /**
      * @param \Magento\MediaStorage\Helper\File\Storage\Database $coreFileStorageDb
      * @param \Magento\MediaStorage\Helper\File\Storage $coreFileStorage
@@ -125,8 +136,8 @@ class Uploader extends \Magento\MediaStorage\Model\File\Uploader
      * @param string|null $filePath
      * @param \Magento\Framework\App\Filesystem\DirectoryResolver|null $directoryResolver
      * @param \Magento\Framework\Math\Random|null $random
-     * @throws \Magento\Framework\Exception\FileSystemException
-     * @throws \Magento\Framework\Exception\LocalizedException
+     * @param \Magento\Framework\Filesystem\Directory\ReadFactory|null $directoryReadFactory
+     * @SuppressWarnings(PHPMD.ExcessiveParameterList)
      */
     public function __construct(
         \Magento\MediaStorage\Helper\File\Storage\Database $coreFileStorageDb,
@@ -137,7 +148,8 @@ public function __construct(
         \Magento\Framework\Filesystem\File\ReadFactory $readFactory,
         $filePath = null,
         \Magento\Framework\App\Filesystem\DirectoryResolver $directoryResolver = null,
-        \Magento\Framework\Math\Random $random = null
+        \Magento\Framework\Math\Random $random = null,
+        \Magento\Framework\Filesystem\Directory\ReadFactory $directoryReadFactory = null
     ) {
         if ($filePath !== null) {
             $this->_setUploadFile($filePath);
@@ -148,10 +160,13 @@ public function __construct(
         $this->_validator = $validator;
         $this->_directory = $filesystem->getDirectoryWrite(DirectoryList::ROOT);
         $this->_readFactory = $readFactory;
+        $this->filesystem = $filesystem;
         $this->directoryResolver = $directoryResolver
             ?: ObjectManager::getInstance()->get(\Magento\Framework\App\Filesystem\DirectoryResolver::class);
         $this->random = $random
             ?: ObjectManager::getInstance()->get(\Magento\Framework\Math\Random::class);
+        $this->directoryReadFactory = $directoryReadFactory
+            ?: ObjectManager::getInstance()->get(\Magento\Framework\Filesystem\Directory\ReadFactory::class);
         $this->downloadDir = DirectoryList::getDefaultConfig()[DirectoryList::TMP][DirectoryList::PATH];
     }
 
@@ -177,7 +192,7 @@ public function init()
      * @param string $fileName
      * @param bool $renameFileOff
      * @return array
-     * @throws \Magento\Framework\Exception\LocalizedException
+     * @throws LocalizedException
      */
     public function move($fileName, $renameFileOff = false)
     {
@@ -242,7 +257,20 @@ private function downloadFileFromUrl($url, $driver)
      */
     protected function _setUploadFile($filePath)
     {
-        if (!$this->_directory->isReadable($filePath)) {
+        try {
+            $fullPath = $this->_directory->getAbsolutePath($filePath);
+            if ($this->getTmpDir()) {
+                $tmpDir = $this->getDirectoryReadByPath(
+                    $this->_directory->getAbsolutePath($this->getTmpDir())
+                );
+            } else {
+                $tmpDir = $this->_directory;
+            }
+            $readable = $tmpDir->isReadable($fullPath);
+        } catch (ValidatorException $exception) {
+            $readable = false;
+        }
+        if (!$readable) {
             throw new \Magento\Framework\Exception\LocalizedException(
                 __('File \'%1\' was not found or has read restriction.', $filePath)
             );
@@ -397,4 +425,17 @@ protected function chmod($file)
     {
         return;
     }
+
+    /**
+     * Create an instance of directory with read permissions by path.
+     *
+     * @param string $path
+     * @param string $driverCode
+     *
+     * @return ReadInterface
+     */
+    private function getDirectoryReadByPath(string $path, string $driverCode = DriverPool::FILE): ReadInterface
+    {
+        return $this->directoryReadFactory->create($path, $driverCode);
+    }
 }

app/code/Magento/Customer/Controller/Account/LoginPost.php

@@ -18,6 +18,8 @@
 use Magento\Framework\App\Config\ScopeConfigInterface;
 
 /**
+ * Post login customer action.
+ *
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
 class LoginPost extends \Magento\Customer\Controller\AbstractAccount
@@ -151,7 +153,6 @@ public function execute()
                 try {
                     $customer = $this->customerAccountManagement->authenticate($login['username'], $login['password']);
                     $this->session->setCustomerDataAsLoggedIn($customer);
-                    $this->session->regenerateId();
                     if ($this->getCookieManager()->getCookie('mage-cache-sessid')) {
                         $metadata = $this->getCookieMetadataFactory()->createCookieMetadata();
                         $metadata->setPath('/');

app/code/Magento/Customer/Controller/Ajax/Login.php

@@ -195,7 +195,6 @@ public function execute()
                 $credentials['password']
             );
             $this->customerSession->setCustomerDataAsLoggedIn($customer);
-            $this->customerSession->regenerateId();
             $redirectRoute = $this->getAccountRedirect()->getRedirectCookie();
             if ($this->cookieManager->getCookie('mage-cache-sessid')) {
                 $metadata = $this->cookieMetadataFactory->createCookieMetadata();

app/code/Magento/Customer/Model/Customer.php

@@ -19,6 +19,7 @@
 use Magento\Framework\Reflection\DataObjectProcessor;
 use Magento\Store\Model\ScopeInterface;
 use Magento\Framework\App\ObjectManager;
+use Magento\Framework\Math\Random;
 
 /**
  * Customer model
@@ -179,7 +180,7 @@ class Customer extends \Magento\Framework\Model\AbstractModel
     protected $_encryptor;
 
     /**
-     * @var \Magento\Framework\Math\Random
+     * @var Random
      */
     protected $mathRandom;
 
@@ -240,6 +241,7 @@ class Customer extends \Magento\Framework\Model\AbstractModel
      * @param \Magento\Framework\Data\Collection\AbstractDb|null $resourceCollection
      * @param array $data
      * @param AccountConfirmation|null $accountConfirmation
+     * @param Random|null $mathRandom
      *
      * @SuppressWarnings(PHPMD.ExcessiveParameterList)
      */
@@ -264,7 +266,8 @@ public function __construct(
         \Magento\Framework\Indexer\IndexerRegistry $indexerRegistry,
         \Magento\Framework\Data\Collection\AbstractDb $resourceCollection = null,
         array $data = [],
-        AccountConfirmation $accountConfirmation = null
+        AccountConfirmation $accountConfirmation = null,
+        Random $mathRandom = null
     ) {
         $this->metadataService = $metadataService;
         $this->_scopeConfig = $scopeConfig;
@@ -283,6 +286,7 @@ public function __construct(
         $this->indexerRegistry = $indexerRegistry;
         $this->accountConfirmation = $accountConfirmation ?: ObjectManager::getInstance()
             ->get(AccountConfirmation::class);
+        $this->mathRandom = $mathRandom ?: ObjectManager::getInstance()->get(Random::class);
         parent::__construct(
             $context,
             $registry,
@@ -794,7 +798,7 @@ public function isConfirmationRequired()
      */
     public function getRandomConfirmationKey()
     {
-        return md5(uniqid());
+        return $this->mathRandom->getRandomString(32);
     }
 
     /**

app/code/Magento/Customer/Model/Plugin/CustomerNotification.php

@@ -15,6 +15,11 @@
 use Magento\Framework\Exception\NoSuchEntityException;
 use Psr\Log\LoggerInterface;
 
+/**
+ * Plugin before \Magento\Framework\App\Action\AbstractAction::dispatch.
+ *
+ * Plugin to remove notifications from cache.
+ */
 class CustomerNotification
 {
     /**
@@ -66,6 +71,8 @@ public function __construct(
     }
 
     /**
+     * Removes notifications from cache.
+     *
      * @param AbstractAction $subject
      * @param RequestInterface $request
      * @return void
@@ -82,10 +89,10 @@ public function beforeDispatch(AbstractAction $subject, RequestInterface $reques
             )
         ) {
             try {
+                $this->session->regenerateId();
                 $customer = $this->customerRepository->getById($customerId);
                 $this->session->setCustomerData($customer);
                 $this->session->setCustomerGroupId($customer->getGroupId());
-                $this->session->regenerateId();
                 $this->notificationStorage->remove(NotificationStorage::UPDATE_CUSTOMER_SESSION, $customerId);
             } catch (NoSuchEntityException $e) {
                 $this->logger->error($e);

app/code/Magento/Customer/Model/Session.php

@@ -354,8 +354,9 @@ public function setCustomerGroupId($id)
     }
 
     /**
-     * Get customer group id
-     * If customer is not logged in system, 'not logged in' group id will be returned
+     * Get customer group id.
+     *
+     * If customer is not logged in system, 'not logged in' group id will be returned.
      *
      * @return int
      */
@@ -407,24 +408,29 @@ public function checkCustomerId($customerId)
     }
 
     /**
+     * Sets customer as logged in.
+     *
      * @param Customer $customer
      * @return $this
      */
     public function setCustomerAsLoggedIn($customer)
     {
+        $this->regenerateId();
         $this->setCustomer($customer);
         $this->_eventManager->dispatch('customer_login', ['customer' => $customer]);
         $this->_eventManager->dispatch('customer_data_object_login', ['customer' => $this->getCustomerDataObject()]);
-        $this->regenerateId();
         return $this;
     }
 
     /**
+     * Sets customer as logged in.
+     *
      * @param CustomerData $customer
      * @return $this
      */
     public function setCustomerDataAsLoggedIn($customer)
     {
+        $this->regenerateId();
         $this->_httpContext->setValue(Context::CONTEXT_AUTH, true, false);
         $this->setCustomerData($customer);
 
@@ -567,6 +573,8 @@ public function regenerateId()
     }
 
     /**
+     * Creates \Magento\Framework\UrlInterface object.
+     *
      * @return \Magento\Framework\UrlInterface
      */
     protected function _createUrl()

app/code/Magento/Customer/Test/Unit/Controller/Account/LoginPostTest.php

@@ -3,6 +3,7 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+
 namespace Magento\Customer\Test\Unit\Controller\Account;
 
 use Magento\Customer\Api\AccountManagementInterface;
@@ -291,9 +292,8 @@ public function testExecuteSuccessCustomRedirect()
             ->method('setCustomerDataAsLoggedIn')
             ->with($customerMock)
             ->willReturnSelf();
-        $this->session->expects($this->once())
-            ->method('regenerateId')
-            ->willReturnSelf();
+        $this->session->expects($this->never())
+            ->method('regenerateId');
 
         $this->accountRedirect->expects($this->never())
             ->method('getRedirect')
@@ -356,9 +356,8 @@ public function testExecuteSuccess()
             ->method('setCustomerDataAsLoggedIn')
             ->with($customerMock)
             ->willReturnSelf();
-        $this->session->expects($this->once())
-            ->method('regenerateId')
-            ->willReturnSelf();
+        $this->session->expects($this->never())
+            ->method('regenerateId');
 
         $this->accountRedirect->expects($this->once())
             ->method('getRedirect')

app/code/Magento/Customer/Test/Unit/Model/CustomerTest.php

@@ -13,9 +13,11 @@
 
 use Magento\Customer\Model\Customer;
 use Magento\Customer\Model\AccountConfirmation;
+use Magento\Framework\Math\Random;
 
 /**
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
+ * @SuppressWarnings(PHPMD.TooManyFields)
  */
 class CustomerTest extends \PHPUnit\Framework\TestCase
 {
@@ -68,6 +70,14 @@ class CustomerTest extends \PHPUnit\Framework\TestCase
      */
     private $accountConfirmation;
 
+    /**
+     * @var Random|\PHPUnit_Framework_MockObject_MockObject
+     */
+    private $mathRandom;
+
+    /**
+     * @inheritdoc
+     */
     protected function setUp()
     {
         $this->_website = $this->createMock(\Magento\Store\Model\Website::class);
@@ -100,6 +110,7 @@ protected function setUp()
         $this->_encryptor = $this->createMock(\Magento\Framework\Encryption\EncryptorInterface::class);
         $helper = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
         $this->accountConfirmation = $this->createMock(AccountConfirmation::class);
+        $this->mathRandom = $this->createMock(Random::class);
         $this->_model = $helper->getObject(
             \Magento\Customer\Model\Customer::class,
             [
@@ -112,7 +123,8 @@ protected function setUp()
                 'registry' => $this->registryMock,
                 'resource' => $this->resourceMock,
                 'dataObjectProcessor' => $this->dataObjectProcessor,
-                'accountConfirmation' => $this->accountConfirmation
+                'accountConfirmation' => $this->accountConfirmation,
+                'mathRandom' => $this->mathRandom,
             ]
         );
     }
@@ -310,4 +322,20 @@ public function testUpdateData()
 
         $this->assertEquals($this->_model->getData(), $expectedResult);
     }
+
+    /**
+     * Check getRandomConfirmationKey use cryptographically secure function
+     *
+     * @return void
+     */
+    public function testGetRandomConfirmationKey()
+    {
+        $this->mathRandom
+            ->expects($this->once())
+            ->method('getRandomString')
+            ->with(32)
+            ->willReturn('random_string');
+
+        $this->_model->getRandomConfirmationKey();
+    }
 }