MAGETWO-36930: [GITHUB] Store config re-encrypt encrypted values on save #1223

- Fix to make sure values are not encrypted if not changed
- Removed re-encrypting values
- Added integration test

Author: anupdugar

app/code/Magento/Config/Model/Config/Backend/Encrypted.php

@@ -81,12 +81,11 @@ protected function _afterLoad()
      */
     public function beforeSave()
     {
+        $this->_dataSaveAllowed = false;
         $value = (string)$this->getValue();
-        // don't change value, if an obscured value came
-        if (preg_match('/^\*+$/', $this->getValue())) {
-            $value = $this->getOldValue();
-        }
-        if (!empty($value)) {
+        // don't save value, if an obscured value was received. This indicates that data was not changed.
+        if (!preg_match('/^\*+$/', $value) && !empty($value)) {
+            $this->_dataSaveAllowed = true;
             $encrypted = $this->_encryptor->encrypt($value);
             if ($encrypted) {
                 $this->setValue($encrypted);

app/code/Magento/Config/Test/Unit/Model/Config/Backend/EncryptedTest.php

@@ -90,41 +90,22 @@ public function testProcessValue()
      * @param $value
      * @param $valueToSave
      */
-    public function testBeforeSave($value, $valueToSave)
+    public function testBeforeSave($value, $encryptMethodCall)
     {
         $this->_resourceMock->expects($this->any())->method('addCommitCallback')->will($this->returnSelf());
         $this->_resourceMock->expects($this->any())->method('commit')->will($this->returnSelf());
 
-        $this->_configMock->expects(
-            $this->any()
-        )->method(
-            'getValue'
-        )->with(
-            'some/path'
-        )->will(
-            $this->returnValue('oldValue')
-        );
-        $this->_encryptorMock->expects(
-            $this->once()
-        )->method(
-            'encrypt'
-        )->with(
-            $valueToSave
-        )->will(
-            $this->returnValue('encrypted')
-        );
-
         $this->_model->setValue($value);
         $this->_model->setPath('some/path');
         $this->_model->beforeSave();
-        $this->assertEquals($this->_model->getValue(), 'encrypted');
+        $this->_encryptorMock->expects($this->exactly($encryptMethodCall))->method('encrypt')->with($this->any());
     }
 
     /**
      * @return array
      */
     public function beforeSaveDataProvider()
     {
-        return [['****', 'oldValue'], ['newValue', 'newValue']];
+        return [['someValue', 1], ['****', 0]];
     }
 }

dev/tests/integration/testsuite/Magento/Config/Model/Config/Backend/EncryptedTest.php

@@ -0,0 +1,58 @@
+<?php
+/**
+ * Copyright © 2015 Magento. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+
+namespace Magento\Config\Model\Config\Backend;
+
+use Magento\TestFramework\Helper\Bootstrap;
+
+/**
+ * @magentoAppArea adminhtml
+ */
+class EncryptedTest extends \PHPUnit_Framework_TestCase
+{
+    /**
+     * @magentoDbIsolation enabled
+     */
+    public function testEncryptionSave()
+    {
+        $originalValue = '1Password';
+
+        /** @var $model \Magento\Config\Model\Config\Backend\Encrypted */
+        $model = Bootstrap::getObjectManager()->create('Magento\Config\Model\Config\Backend\Encrypted');
+        $model->setPath('carriers/usps/password');
+        $model->setScopeId(0);
+        $model->setScope('default');
+        $model->setScopeCode('');
+        $model->setValue($originalValue);
+        $model->save();
+
+        // Pass in the obscured value
+        $model->setPath('carriers/usps/password');
+        $model->setScopeId(0);
+        $model->setScope('default');
+        $model->setScopeCode('');
+        $model->setValue('*****');
+        $model->save();
+
+        //Verify original value is not changed for obscured value
+        $value = $model->load($model->getId())->getValue();
+        $this->assertEquals($originalValue, $value, 'Original value is not expected to change.');
+
+        // Verify if actual value is changed
+        $changedValue = 'newPassword';
+
+        $model->setPath('carriers/usps/password');
+        $model->setScopeId(0);
+        $model->setScope('default');
+        $model->setScopeCode('');
+        $model->setValue($changedValue);
+        $model->save();
+
+        //Verify original value is changed
+        $value = $model->load($model->getId())->getValue();
+        $this->assertEquals($changedValue, $value, 'Original value is expected to change.');
+    }
+}