diff --git a/lib/internal/Magento/Framework/Config/Data/ConfigDataFactory.php b/lib/internal/Magento/Framework/Config/Data/ConfigDataFactory.php new file mode 100644 index 0000000000000..85232b35ac6f4 --- /dev/null +++ b/lib/internal/Magento/Framework/Config/Data/ConfigDataFactory.php @@ -0,0 +1,42 @@ +objectManager = $objectManager; + } + + /** + * Returns a new instance of ConfigData on every call. + * + * @param string $fileKey + * @return ConfigData + */ + public function create($fileKey) + { + return $this->objectManager->create(ConfigData::class, ['fileKey' => $fileKey]); + } +} diff --git a/setup/src/Magento/Setup/Model/ConfigGenerator.php b/setup/src/Magento/Setup/Model/ConfigGenerator.php index 00ed16db74d8a..e3ab070bd8fba 100644 --- a/setup/src/Magento/Setup/Model/ConfigGenerator.php +++ b/setup/src/Magento/Setup/Model/ConfigGenerator.php @@ -6,13 +6,14 @@ namespace Magento\Setup\Model; +use Magento\Framework\App\ObjectManager; use Magento\Framework\Config\Data\ConfigData; +use Magento\Framework\Config\Data\ConfigDataFactory; use Magento\Framework\Config\File\ConfigFilePool; -use Magento\Framework\Math\Random; use Magento\Framework\App\DeploymentConfig; use Magento\Framework\Config\ConfigOptionsListConstants; use Magento\Framework\App\State; -use Magento\Framework\App\ObjectManagerFactory; +use Magento\Framework\Math\Random; /** * Creates deployment config data based on user input array @@ -26,39 +27,58 @@ class ConfigGenerator * @var array */ private static $paramMap = [ - ConfigOptionsListConstants::INPUT_KEY_DB_HOST => ConfigOptionsListConstants::KEY_HOST, - ConfigOptionsListConstants::INPUT_KEY_DB_NAME => ConfigOptionsListConstants::KEY_NAME, - ConfigOptionsListConstants::INPUT_KEY_DB_USER => ConfigOptionsListConstants::KEY_USER, - ConfigOptionsListConstants::INPUT_KEY_DB_PASSWORD => ConfigOptionsListConstants::KEY_PASSWORD, - ConfigOptionsListConstants::INPUT_KEY_DB_PREFIX => ConfigOptionsListConstants::KEY_PREFIX, - ConfigOptionsListConstants::INPUT_KEY_DB_MODEL => ConfigOptionsListConstants::KEY_MODEL, - ConfigOptionsListConstants::INPUT_KEY_DB_ENGINE => ConfigOptionsListConstants::KEY_ENGINE, + ConfigOptionsListConstants::INPUT_KEY_DB_HOST => ConfigOptionsListConstants::KEY_HOST, + ConfigOptionsListConstants::INPUT_KEY_DB_NAME => ConfigOptionsListConstants::KEY_NAME, + ConfigOptionsListConstants::INPUT_KEY_DB_USER => ConfigOptionsListConstants::KEY_USER, + ConfigOptionsListConstants::INPUT_KEY_DB_PASSWORD => ConfigOptionsListConstants::KEY_PASSWORD, + ConfigOptionsListConstants::INPUT_KEY_DB_PREFIX => ConfigOptionsListConstants::KEY_PREFIX, + ConfigOptionsListConstants::INPUT_KEY_DB_MODEL => ConfigOptionsListConstants::KEY_MODEL, + ConfigOptionsListConstants::INPUT_KEY_DB_ENGINE => ConfigOptionsListConstants::KEY_ENGINE, ConfigOptionsListConstants::INPUT_KEY_DB_INIT_STATEMENTS => ConfigOptionsListConstants::KEY_INIT_STATEMENTS, - ConfigOptionsListConstants::INPUT_KEY_ENCRYPTION_KEY => ConfigOptionsListConstants::KEY_ENCRYPTION_KEY, - ConfigOptionsListConstants::INPUT_KEY_SESSION_SAVE => ConfigOptionsListConstants::KEY_SAVE, - ConfigOptionsListConstants::INPUT_KEY_RESOURCE => ConfigOptionsListConstants::KEY_RESOURCE, + ConfigOptionsListConstants::INPUT_KEY_ENCRYPTION_KEY => ConfigOptionsListConstants::KEY_ENCRYPTION_KEY, + ConfigOptionsListConstants::INPUT_KEY_SESSION_SAVE => ConfigOptionsListConstants::KEY_SAVE, + ConfigOptionsListConstants::INPUT_KEY_RESOURCE => ConfigOptionsListConstants::KEY_RESOURCE, ]; + /** + * @var DeploymentConfig + */ + protected $deploymentConfig; + /** * @var Random + * @deprecated since 100.2.0 */ protected $random; /** - * @var DeploymentConfig + * @var ConfigDataFactory */ - protected $deploymentConfig; + private $configDataFactory; + + /** + * @var CryptKeyGeneratorInterface + */ + private $cryptKeyGenerator; /** * Constructor * - * @param Random $random + * @param Random $random Deprecated since 100.2.0 * @param DeploymentConfig $deploymentConfig + * @param ConfigDataFactory|null $configDataFactory + * @param CryptKeyGeneratorInterface|null $cryptKeyGenerator */ - public function __construct(Random $random, DeploymentConfig $deploymentConfig) - { + public function __construct( + Random $random, + DeploymentConfig $deploymentConfig, + ConfigDataFactory $configDataFactory = null, + CryptKeyGeneratorInterface $cryptKeyGenerator = null + ) { $this->random = $random; $this->deploymentConfig = $deploymentConfig; + $this->configDataFactory = $configDataFactory ?? ObjectManager::getInstance()->get(ConfigDataFactory::class); + $this->cryptKeyGenerator = $cryptKeyGenerator ?? ObjectManager::getInstance()->get(CryptKeyGenerator::class); } /** @@ -70,23 +90,17 @@ public function createCryptConfig(array $data) { $currentKey = $this->deploymentConfig->get(ConfigOptionsListConstants::CONFIG_PATH_CRYPT_KEY); - $configData = new ConfigData(ConfigFilePool::APP_ENV); - if (isset($data[ConfigOptionsListConstants::INPUT_KEY_ENCRYPTION_KEY])) { - if ($currentKey !== null) { - $key = $currentKey . "\n" . $data[ConfigOptionsListConstants::INPUT_KEY_ENCRYPTION_KEY]; - } else { - $key = $data[ConfigOptionsListConstants::INPUT_KEY_ENCRYPTION_KEY]; - } + $configData = $this->configDataFactory->create(ConfigFilePool::APP_ENV); - $configData->set(ConfigOptionsListConstants::CONFIG_PATH_CRYPT_KEY, $key); - } else { - if ($currentKey === null) { - $configData->set( - ConfigOptionsListConstants::CONFIG_PATH_CRYPT_KEY, - md5($this->random->getRandomString(ConfigOptionsListConstants::STORE_KEY_RANDOM_STRING_SIZE)) - ); - } - } + // Use given key if set, else use current + $key = $data[ConfigOptionsListConstants::INPUT_KEY_ENCRYPTION_KEY] ?? $currentKey; + + // If there is no key given or currently set, generate a new one + $key = $key ?? $this->cryptKeyGenerator->generate(); + + // Chaining of ".. ?? .." is not used to keep it simpler to understand + + $configData->set(ConfigOptionsListConstants::CONFIG_PATH_CRYPT_KEY, $key); return $configData; } @@ -99,7 +113,7 @@ public function createCryptConfig(array $data) */ public function createSessionConfig(array $data) { - $configData = new ConfigData(ConfigFilePool::APP_ENV); + $configData = $this->configDataFactory->create(ConfigFilePool::APP_ENV); if (isset($data[ConfigOptionsListConstants::INPUT_KEY_SESSION_SAVE])) { $configData->set( @@ -132,7 +146,7 @@ public function createDefinitionsConfig(array $data) */ public function createDbConfig(array $data) { - $configData = new ConfigData(ConfigFilePool::APP_ENV); + $configData = $this->configDataFactory->create(ConfigFilePool::APP_ENV); $optional = [ ConfigOptionsListConstants::INPUT_KEY_DB_HOST, @@ -151,25 +165,18 @@ public function createDbConfig(array $data) ); } + $dbConnectionPrefix = ConfigOptionsListConstants::CONFIG_PATH_DB_CONNECTION_DEFAULT . '/'; + foreach ($optional as $key) { if (isset($data[$key])) { - $configData->set( - ConfigOptionsListConstants::CONFIG_PATH_DB_CONNECTION_DEFAULT . '/' . self::$paramMap[$key], - $data[$key] - ); + $configData->set($dbConnectionPrefix . self::$paramMap[$key], $data[$key]); } } - $currentStatus = $this->deploymentConfig->get( - ConfigOptionsListConstants::CONFIG_PATH_DB_CONNECTION_DEFAULT . '/' . ConfigOptionsListConstants::KEY_ACTIVE - ); + $currentStatus = $this->deploymentConfig->get($dbConnectionPrefix . ConfigOptionsListConstants::KEY_ACTIVE); if ($currentStatus === null) { - $configData->set( - ConfigOptionsListConstants::CONFIG_PATH_DB_CONNECTION_DEFAULT - . '/' . ConfigOptionsListConstants::KEY_ACTIVE, - '1' - ); + $configData->set($dbConnectionPrefix . ConfigOptionsListConstants::KEY_ACTIVE, '1'); } return $configData; @@ -182,7 +189,7 @@ public function createDbConfig(array $data) */ public function createResourceConfig() { - $configData = new ConfigData(ConfigFilePool::APP_ENV); + $configData = $this->configDataFactory->create(ConfigFilePool::APP_ENV); if ($this->deploymentConfig->get(ConfigOptionsListConstants::CONFIG_PATH_RESOURCE_DEFAULT_SETUP) === null) { $configData->set(ConfigOptionsListConstants::CONFIG_PATH_RESOURCE_DEFAULT_SETUP, 'default'); @@ -198,10 +205,12 @@ public function createResourceConfig() */ public function createXFrameConfig() { - $configData = new ConfigData(ConfigFilePool::APP_ENV); + $configData = $this->configDataFactory->create(ConfigFilePool::APP_ENV); + if ($this->deploymentConfig->get(ConfigOptionsListConstants::CONFIG_PATH_X_FRAME_OPT) === null) { $configData->set(ConfigOptionsListConstants::CONFIG_PATH_X_FRAME_OPT, 'SAMEORIGIN'); } + return $configData; } @@ -212,10 +221,12 @@ public function createXFrameConfig() */ public function createModeConfig() { - $configData = new ConfigData(ConfigFilePool::APP_ENV); + $configData = $this->configDataFactory->create(ConfigFilePool::APP_ENV); + if ($this->deploymentConfig->get(State::PARAM_MODE) === null) { $configData->set(State::PARAM_MODE, State::MODE_DEFAULT); } + return $configData; } @@ -227,21 +238,29 @@ public function createModeConfig() */ public function createCacheHostsConfig(array $data) { - $configData = new ConfigData(ConfigFilePool::APP_ENV); + $configData = $this->configDataFactory->create(ConfigFilePool::APP_ENV); + if (isset($data[ConfigOptionsListConstants::INPUT_KEY_CACHE_HOSTS])) { - $hostData = explode(',', $data[ConfigOptionsListConstants::INPUT_KEY_CACHE_HOSTS]); - $hosts = []; - foreach ($hostData as $data) { - $dataArray = explode(':', trim($data)); - $host = []; - $host['host'] = $dataArray[0]; - if (isset($dataArray[1])) { - $host['port'] = $dataArray[1]; - } - $hosts[] = $host; - } + $hosts = explode(',', $data[ConfigOptionsListConstants::INPUT_KEY_CACHE_HOSTS]); + + $hosts = array_map( + function ($hostData) { + $hostDataParts = explode(':', trim($hostData)); + + $tmp = ['host' => $hostDataParts[0]]; + + if (isset($hostDataParts[1])) { + $tmp['port'] = $hostDataParts[1]; + } + + return $tmp; + }, + $hosts + ); + $configData->set(ConfigOptionsListConstants::CONFIG_PATH_CACHE_HOSTS, $hosts); } + $configData->setOverrideWhenSave(true); return $configData; } diff --git a/setup/src/Magento/Setup/Model/CryptKeyGenerator.php b/setup/src/Magento/Setup/Model/CryptKeyGenerator.php new file mode 100644 index 0000000000000..857a603631ed7 --- /dev/null +++ b/setup/src/Magento/Setup/Model/CryptKeyGenerator.php @@ -0,0 +1,55 @@ +random = $random; + } + + /** + * Generates & returns a string to be used as crypt key. + * + * The key length is not a parameter, but an implementation detail. + * + * @return string + * + * @throws \Magento\Framework\Exception\LocalizedException + */ + public function generate() + { + return md5($this->getRandomString()); + } + + /** + * Returns a random string. + * + * @return string + */ + private function getRandomString() + { + return $this->random->getRandomString(ConfigOptionsListConstants::STORE_KEY_RANDOM_STRING_SIZE); + } +} diff --git a/setup/src/Magento/Setup/Model/CryptKeyGeneratorInterface.php b/setup/src/Magento/Setup/Model/CryptKeyGeneratorInterface.php new file mode 100644 index 0000000000000..f96f9b37ac9a7 --- /dev/null +++ b/setup/src/Magento/Setup/Model/CryptKeyGeneratorInterface.php @@ -0,0 +1,22 @@ +deploymentConfigMock = $this->getMockBuilder(\Magento\Framework\App\DeploymentConfig::class) + $this->deploymentConfigMock = $this->getMockBuilder(DeploymentConfig::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->configDataMock = $this->getMockBuilder(ConfigData::class) + ->disableOriginalConstructor() + ->setMethods(['set']) + ->getMock(); + + $configDataFactoryMock = $this->getMockBuilder(ConfigDataFactory::class) ->disableOriginalConstructor() + ->setMethods(['create']) ->getMock(); + + $configDataFactoryMock->method('create') + ->willReturn($this->configDataMock); + $this->model = $objectManager->getObject( - \Magento\Setup\Model\ConfigGenerator::class, - ['deploymentConfig' => $this->deploymentConfigMock] + ConfigGenerator::class, + [ + 'deploymentConfig' => $this->deploymentConfigMock, + 'configDataFactory' => $configDataFactoryMock, + ] ); } @@ -35,8 +65,13 @@ public function testCreateXFrameConfig() ->method('get') ->with(ConfigOptionsListConstants::CONFIG_PATH_X_FRAME_OPT) ->willReturn(null); - $configData = $this->model->createXFrameConfig(); - $this->assertSame('SAMEORIGIN', $configData->getData()[ConfigOptionsListConstants::CONFIG_PATH_X_FRAME_OPT]); + + $this->configDataMock + ->expects($this->once()) + ->method('set') + ->with(ConfigOptionsListConstants::CONFIG_PATH_X_FRAME_OPT, 'SAMEORIGIN'); + + $this->model->createXFrameConfig(); } public function testCreateCacheHostsConfig() @@ -55,8 +90,13 @@ public function testCreateCacheHostsConfig() 'port' => '90', ], ]; - $configData = $this->model->createCacheHostsConfig($data); - $this->assertEquals($expectedData, $configData->getData()[ConfigOptionsListConstants::CONFIG_PATH_CACHE_HOSTS]); + + $this->configDataMock + ->expects($this->once()) + ->method('set') + ->with(ConfigOptionsListConstants::CONFIG_PATH_CACHE_HOSTS, $expectedData); + + $this->model->createCacheHostsConfig($data); } public function testCreateModeConfig() @@ -65,8 +105,13 @@ public function testCreateModeConfig() ->method('get') ->with(State::PARAM_MODE) ->willReturn(null); - $configData = $this->model->createModeConfig(); - $this->assertSame(State::MODE_DEFAULT, $configData->getData()[State::PARAM_MODE]); + + $this->configDataMock + ->expects($this->once()) + ->method('set') + ->with(State::PARAM_MODE, State::MODE_DEFAULT); + + $this->model->createModeConfig(); } public function testCreateModeConfigIfAlreadySet() @@ -78,4 +123,22 @@ public function testCreateModeConfigIfAlreadySet() $configData = $this->model->createModeConfig(); $this->assertSame([], $configData->getData()); } + + public function testCreateCryptKeyConfig() + { + $key = 'my-new-key'; + $data = [ConfigOptionsListConstants::INPUT_KEY_ENCRYPTION_KEY => $key]; + + $this->deploymentConfigMock + ->method('get') + ->with(ConfigOptionsListConstants::CONFIG_PATH_CRYPT_KEY) + ->willReturn(null); + + $this->configDataMock + ->expects($this->once()) + ->method('set') + ->with(ConfigOptionsListConstants::CONFIG_PATH_CRYPT_KEY, $key); + + $this->model->createCryptConfig($data); + } } diff --git a/setup/src/Magento/Setup/Test/Unit/Model/CryptKeyGeneratorTest.php b/setup/src/Magento/Setup/Test/Unit/Model/CryptKeyGeneratorTest.php new file mode 100644 index 0000000000000..d7a4d477de7b0 --- /dev/null +++ b/setup/src/Magento/Setup/Test/Unit/Model/CryptKeyGeneratorTest.php @@ -0,0 +1,59 @@ +randomMock = $this->getMockBuilder(Random::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->cryptKeyGenerator = new CryptKeyGenerator($this->randomMock); + } + + public function testStringForHashingIsReadFromRandom() + { + $this->randomMock + ->expects($this->once()) + ->method('getRandomString') + ->willReturn(''); + + $this->cryptKeyGenerator->generate(); + } + + public function testReturnsMd5OfRandomString() + { + $expected = 'fdb7594e77f1ad5fbb8e6c917b6012ce'; // == 'magento2' + + $this->randomMock + ->method('getRandomString') + ->willReturn('magento2'); + + $actual = $this->cryptKeyGenerator->generate(); + + $this->assertEquals($expected, $actual); + } +} diff --git a/setup/src/Magento/Setup/Test/Unit/Module/ConfigGeneratorTest.php b/setup/src/Magento/Setup/Test/Unit/Module/ConfigGeneratorTest.php index 69f320bccd5ce..65fe4a5f47711 100644 --- a/setup/src/Magento/Setup/Test/Unit/Module/ConfigGeneratorTest.php +++ b/setup/src/Magento/Setup/Test/Unit/Module/ConfigGeneratorTest.php @@ -5,11 +5,18 @@ */ namespace Magento\Setup\Test\Unit\Module; +use Magento\Framework\App\DeploymentConfig; +use Magento\Framework\Config\Data\ConfigData; +use Magento\Framework\Config\Data\ConfigDataFactory; use Magento\Framework\Config\File\ConfigFilePool; +use Magento\Framework\Math\Random; +use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; use Magento\Setup\Model\ConfigGenerator; use Magento\Framework\Config\ConfigOptionsListConstants; +use Magento\Setup\Model\CryptKeyGenerator; +use PHPUnit\Framework\TestCase; -class ConfigGeneratorTest extends \PHPUnit\Framework\TestCase +class ConfigGeneratorTest extends TestCase { /** * @var ConfigGenerator @@ -18,11 +25,31 @@ class ConfigGeneratorTest extends \PHPUnit\Framework\TestCase protected function setUp() { - $random = $this->createMock(\Magento\Framework\Math\Random::class); - $random->expects($this->any())->method('getRandomString')->willReturn('key'); - $deployConfig= $this->createMock(\Magento\Framework\App\DeploymentConfig::class); + /** @var DeploymentConfig|\PHPUnit_Framework_MockObject_MockObject $deployConfig */ + $deployConfig = $this->createMock(DeploymentConfig::class); $deployConfig->expects($this->any())->method('isAvailable')->willReturn(false); - $this->configGeneratorObject = new ConfigGenerator($random, $deployConfig); + + /** @var Random|\PHPUnit_Framework_MockObject_MockObject $randomMock */ + $randomMock = $this->createMock(Random::class); + $randomMock->expects($this->any())->method('getRandomString')->willReturn('key'); + + $cryptKeyGenerator = new CryptKeyGenerator($randomMock); + + $objectManagerMock = $this->getMockBuilder(\Magento\Framework\App\ObjectManager::class) + ->disableOriginalConstructor() + ->getMock(); + + $objectManagerMock->method('create')->willReturn(new ConfigData('app_env')); + + $configDataFactoryMock = (new ObjectManager($this)) + ->getObject(ConfigDataFactory::class, ['objectManager' => $objectManagerMock]); + + $this->configGeneratorObject = new ConfigGenerator( + $randomMock, + $deployConfig, + $configDataFactoryMock, + $cryptKeyGenerator + ); } public function testCreateCryptConfigWithInput()