Merge pull request #4208 from magento-qwerty/MC-16342

Fixed Issues:
- MC-16342: [2.2.x] Fix CompanyUserManagerInterfaceTest failing on mainline

Author: dvoskoboinikov

app/code/Magento/Sales/Model/Rss/OrderStatus.php

@@ -155,7 +155,7 @@ protected function getOrder()
         }
 
         $data = (string)$this->request->getParam('data');
-        if ((string)$this->request->getParam('signature') !== $this->signature->signData($data)) {
+        if (!$this->signature->isValid($data, (string)$this->request->getParam('signature'))) {
             return null;
         }
         $json = base64_decode($data);

app/code/Magento/Sales/Model/Rss/Signature.php

@@ -8,65 +8,47 @@
 
 namespace Magento\Sales\Model\Rss;
 
-use Magento\Framework\Config\ConfigOptionsListConstants;
+use Magento\Framework\Encryption\EncryptorInterface;
 
 /**
  * Class for generating signature.
  */
 class Signature
 {
     /**
-     * Version of encryption key.
-     *
-     * @var int
-     */
-    private $keyVersion;
-
-    /**
-     * Array of encryption keys.
-     *
-     * @var string[]
-     */
-    private $keys = [];
-
-    /**
-     * @var \Magento\Framework\App\DeploymentConfig
+     * @var EncryptorInterface
      */
-    private $deploymentConfig;
+    private $encryptor;
 
     /**
-     * @param \Magento\Framework\App\DeploymentConfig $deploymentConfig
+     * @param EncryptorInterface $encryptor
      */
     public function __construct(
-        \Magento\Framework\App\DeploymentConfig $deploymentConfig
+        EncryptorInterface $encryptor
     ) {
-        $this->deploymentConfig = $deploymentConfig;
-        // load all possible keys
-        $this->keys = preg_split(
-            '/\s+/s',
-            (string)$this->deploymentConfig->get(ConfigOptionsListConstants::CONFIG_PATH_CRYPT_KEY)
-        );
-        $this->keyVersion = count($this->keys) - 1;
+        $this->encryptor = $encryptor;
     }
 
     /**
-     * Get secret key.
+     * Sign data.
      *
+     * @param string $data
      * @return string
      */
-    private function getSecretKey(): string
+    public function signData(string $data): string
     {
-        return (string)$this->keys[$this->keyVersion];
+        return $this->encryptor->hash($data);
     }
 
     /**
-     * Sign data.
+     * Check if valid signature is provided for given data.
      *
      * @param string $data
-     * @return string
+     * @param string $signature
+     * @return bool
      */
-    public function signData(string $data): string
+    public function isValid(string $data, string $signature): bool
     {
-        return hash_hmac('sha256', $data, pack('H*', $this->getSecretKey()));
+        return $this->encryptor->validateHash($data, $signature);
     }
 }

app/code/Magento/Sales/Test/Unit/Model/Rss/OrderStatusTest.php

@@ -10,6 +10,7 @@
 
 /**
  * Class OrderStatusTest
+ *
  * @package Magento\Sales\Model\Rss
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
@@ -92,6 +93,9 @@ class OrderStatusTest extends \PHPUnit\Framework\TestCase
         ],
     ];
 
+    /**
+     * @inheritdoc
+     */
     protected function setUp()
     {
         $this->objectManager = $this->createMock(\Magento\Framework\ObjectManagerInterface::class);
@@ -142,11 +146,18 @@ protected function setUp()
         );
     }
 
+    /**
+     * Positive scenario.
+     */
     public function testGetRssData()
     {
         $this->orderFactory->expects($this->once())->method('create')->willReturn($this->order);
         $requestData = base64_encode('{"order_id":1,"increment_id":"100000001","customer_id":1}');
-        $this->signature->expects($this->any())->method('signData')->willReturn('signature');
+        $this->signature->expects($this->never())->method('signData');
+        $this->signature->expects($this->any())
+            ->method('isValid')
+            ->with($requestData, 'signature')
+            ->willReturn(true);
 
         $this->requestInterface->expects($this->any())
             ->method('getParam')
@@ -177,14 +188,20 @@ public function testGetRssData()
     }
 
     /**
+     * Case when invalid data is provided.
+     *
      * @expectedException \InvalidArgumentException
      * @expectedExceptionMessage Order not found.
      */
     public function testGetRssDataWithError()
     {
         $this->orderFactory->expects($this->once())->method('create')->willReturn($this->order);
         $requestData = base64_encode('{"order_id":"1","increment_id":true,"customer_id":true}');
-        $this->signature->expects($this->any())->method('signData')->willReturn('signature');
+        $this->signature->expects($this->never())->method('signData');
+        $this->signature->expects($this->any())
+            ->method('isValid')
+            ->with($requestData, 'signature')
+            ->willReturn(true);
         $this->requestInterface->expects($this->any())
             ->method('getParam')
             ->willReturnMap(
@@ -199,16 +216,20 @@ public function testGetRssDataWithError()
     }
 
     /**
+     * Case when invalid signature is provided.
+     *
      * @expectedException \InvalidArgumentException
      * @expectedExceptionMessage Order not found.
      */
     public function testGetRssDataWithWrongSignature()
     {
         $requestData = base64_encode('{"order_id":"1","increment_id":true,"customer_id":true}');
+        $this->signature->expects($this->never())
+            ->method('signData');
         $this->signature->expects($this->any())
-            ->method('signData')
-            ->with($requestData)
-            ->willReturn('wrong_signature');
+            ->method('isValid')
+            ->with($requestData, 'signature')
+            ->willReturn(false);
         $this->requestInterface->expects($this->any())
             ->method('getParam')
             ->willReturnMap(
@@ -222,6 +243,9 @@ public function testGetRssDataWithWrongSignature()
         $this->assertEquals($this->feedData, $this->model->getRssData());
     }
 
+    /**
+     * Testing allowed getter.
+     */
     public function testIsAllowed()
     {
         $this->scopeConfigInterface->expects($this->once())->method('getValue')
@@ -231,6 +255,8 @@ public function testIsAllowed()
     }
 
     /**
+     * Test caching.
+     *
      * @param string $requestData
      * @param string $result
      * @dataProvider getCacheKeyDataProvider
@@ -242,12 +268,18 @@ public function testGetCacheKey($requestData, $result)
                 ['data', null, $requestData],
                 ['signature', null, 'signature'],
             ]);
-        $this->signature->expects($this->any())->method('signData')->willReturn('signature');
+        $this->signature->expects($this->never())->method('signData');
+        $this->signature->expects($this->any())
+            ->method('isValid')
+            ->with($requestData, 'signature')
+            ->willReturn(true);
         $this->orderFactory->expects($this->once())->method('create')->will($this->returnValue($this->order));
         $this->assertEquals('rss_order_status_data_' . $result, $this->model->getCacheKey());
     }
 
     /**
+     * Test data for caching test.
+     *
      * @return array
      */
     public function getCacheKeyDataProvider()
@@ -258,6 +290,9 @@ public function getCacheKeyDataProvider()
         ];
     }
 
+    /**
+     * Test for cache lifetime getter.
+     */
     public function testGetCacheLifetime()
     {
         $this->assertEquals(600, $this->model->getCacheLifetime());

app/code/Magento/Sales/Test/Unit/Model/Rss/SignatureTest.php

@@ -18,6 +18,9 @@
 use Magento\Vault\Observer\AfterPaymentSaveObserver;
 use PHPUnit_Framework_MockObject_MockObject as MockObject;
 
+/**
+ * Test for payment observer.
+ */
 class AfterPaymentSaveObserverTest extends \PHPUnit\Framework\TestCase
 {
     /**
@@ -61,14 +64,18 @@ class AfterPaymentSaveObserverTest extends \PHPUnit\Framework\TestCase
     protected $salesOrderPaymentMock;
 
     /**
-     * @return void
+     * @inheritdoc
      */
     protected function setUp()
     {
         /** @var Random|MockObject $encryptorRandomGenerator */
         $encryptorRandomGenerator = $this->createMock(Random::class);
         /** @var DeploymentConfig|MockObject $deploymentConfigMock */
         $deploymentConfigMock = $this->createMock(DeploymentConfig::class);
+        $deploymentConfigMock->expects($this->any())
+            ->method('get')
+            ->with(Encryptor::PARAM_CRYPT_KEY)
+            ->willReturn('g9mY9KLrcuAVJfsmVUSRkKFLDdUPVkaZ');
         $this->encryptorModel = new Encryptor($encryptorRandomGenerator, $deploymentConfigMock);
 
         $this->paymentExtension = $this->getMockBuilder(OrderPaymentExtension::class)
@@ -117,6 +124,8 @@ protected function setUp()
     }
 
     /**
+     * Case when payment successfully made.
+     *
      * @param int $customerId
      * @param string $createdAt
      * @param string $token
@@ -161,6 +170,8 @@ public function testPositiveCase($customerId, $createdAt, $token, $isActive, $me
     }
 
     /**
+     * Data for positiveCase test.
+     *
      * @return array
      */
     public function positiveCaseDataProvider()