From b60b5f8cb2e52768a00f744021b880b3b86b2ecf Mon Sep 17 00:00:00 2001 From: Oleh Dmytrychenko Date: Fri, 28 Feb 2025 13:42:25 +0200 Subject: [PATCH 1/5] magento/magento2#38933: Putting csp_whitelist.xml in theme does not work and creates intermittent issue - Implemented caching of CSP whitelist per website area. --- .../CspWhitelistXml/FileResolver.php | 47 +++++++++++-------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/app/code/Magento/Csp/Model/Collector/CspWhitelistXml/FileResolver.php b/app/code/Magento/Csp/Model/Collector/CspWhitelistXml/FileResolver.php index acc0dd1600db1..70993731fecaf 100644 --- a/app/code/Magento/Csp/Model/Collector/CspWhitelistXml/FileResolver.php +++ b/app/code/Magento/Csp/Model/Collector/CspWhitelistXml/FileResolver.php @@ -8,15 +8,15 @@ namespace Magento\Csp\Model\Collector\CspWhitelistXml; +use Magento\Framework\App\Filesystem\DirectoryList; +use Magento\Framework\Config\CompositeFileIteratorFactory; use Magento\Framework\Config\FileResolverInterface; use Magento\Framework\Filesystem; -use Magento\Framework\View\Design\ThemeInterface; -use Magento\Framework\View\DesignInterface; +use Magento\Framework\Filesystem\Directory\ReadInterface as DirectoryRead; use Magento\Framework\View\Design\Theme\CustomizationInterface; use Magento\Framework\View\Design\Theme\CustomizationInterfaceFactory; -use Magento\Framework\App\Filesystem\DirectoryList; -use Magento\Framework\Filesystem\Directory\ReadInterface as DirectoryRead; -use Magento\Framework\Config\CompositeFileIteratorFactory; +use Magento\Framework\View\Design\ThemeInterface; +use Magento\Framework\View\DesignInterface; /** * Combines configuration files from both modules and current theme. @@ -74,22 +74,29 @@ public function __construct( */ public function get($filename, $scope) { - $configs = $this->moduleFileResolver->get($filename, $scope); - if ($scope === 'global') { - $files = []; - $theme = $this->theme; - while ($theme) { - /** @var CustomizationInterface $info */ - $info = $this->themeInfoFactory->create(['theme' => $theme]); - $file = $info->getThemeFilesPath() .'/etc/' .$filename; - if ($this->rootDir->isExist($file)) { - $files[] = $file; + $configs = $this->moduleFileResolver->get($filename, $scope); + + switch ($scope) { + case 'frontend': + case 'adminhtml': + $files = []; + $theme = $this->theme; + while ($theme) { + /** @var CustomizationInterface $info */ + $info = $this->themeInfoFactory->create(['theme' => $theme]); + $file = $info->getThemeFilesPath() . '/etc/' . $filename; + if ($this->rootDir->isExist($file)) { + $files[] = $file; + } + $theme = $theme->getParentTheme(); } - $theme = $theme->getParentTheme(); - } - $configs = $this->iteratorFactory->create( - ['paths' => array_reverse($files), 'existingIterator' => $configs] - ); + $configs = $this->iteratorFactory->create( + ['paths' => array_reverse($files), 'existingIterator' => $configs] + ); + break; + case 'global': + default: + break; } return $configs; From f65f4ab479bed133382423b10d16cf7a1f516eed Mon Sep 17 00:00:00 2001 From: Oleh Dmytrychenko Date: Fri, 28 Feb 2025 15:24:53 +0200 Subject: [PATCH 2/5] magento/magento2#38933: Putting csp_whitelist.xml in theme does not work and creates intermittent issue - Updated copyright notice. --- .../Csp/Model/Collector/CspWhitelistXml/FileResolver.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Csp/Model/Collector/CspWhitelistXml/FileResolver.php b/app/code/Magento/Csp/Model/Collector/CspWhitelistXml/FileResolver.php index 70993731fecaf..25f2633ba8402 100644 --- a/app/code/Magento/Csp/Model/Collector/CspWhitelistXml/FileResolver.php +++ b/app/code/Magento/Csp/Model/Collector/CspWhitelistXml/FileResolver.php @@ -1,7 +1,7 @@ Date: Tue, 25 Mar 2025 20:41:02 +0300 Subject: [PATCH 3/5] Update app/code/Magento/Csp/Model/Collector/CspWhitelistXml/FileResolver.php Co-authored-by: Abhinav Pathak <51681618+engcom-Hotel@users.noreply.github.com> --- .../Csp/Model/Collector/CspWhitelistXml/FileResolver.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Csp/Model/Collector/CspWhitelistXml/FileResolver.php b/app/code/Magento/Csp/Model/Collector/CspWhitelistXml/FileResolver.php index 25f2633ba8402..58a7f2e58f127 100644 --- a/app/code/Magento/Csp/Model/Collector/CspWhitelistXml/FileResolver.php +++ b/app/code/Magento/Csp/Model/Collector/CspWhitelistXml/FileResolver.php @@ -1,6 +1,6 @@ Date: Thu, 10 Apr 2025 19:34:08 +0530 Subject: [PATCH 4/5] Added Unit Test Cases for CSP FileResolver --- .../CspWhitelistXml/FileResolverTest.php | 221 ++++++++++++++++++ 1 file changed, 221 insertions(+) create mode 100644 app/code/Magento/Csp/Test/Unit/Model/Collector/CspWhitelistXml/FileResolverTest.php diff --git a/app/code/Magento/Csp/Test/Unit/Model/Collector/CspWhitelistXml/FileResolverTest.php b/app/code/Magento/Csp/Test/Unit/Model/Collector/CspWhitelistXml/FileResolverTest.php new file mode 100644 index 0000000000000..83917453c857d --- /dev/null +++ b/app/code/Magento/Csp/Test/Unit/Model/Collector/CspWhitelistXml/FileResolverTest.php @@ -0,0 +1,221 @@ +moduleFileResolverMock = $this->getMockBuilder(FileResolverInterface::class) + ->disableOriginalConstructor() + ->onlyMethods(['get']) + ->getMockForAbstractClass(); + + $this->designMock = $this->getMockBuilder(DesignInterface::class) + ->disableOriginalConstructor() + ->onlyMethods(['getDesignTheme']) + ->getMockForAbstractClass(); + + $this->themeInterFaceMock = $this->getMockBuilder(ThemeInterface::class) + ->getMockForAbstractClass(); + + $this->designMock->expects($this->once()) + ->method('getDesignTheme') + ->willReturn($this->themeInterFaceMock); + + $this->customizationFactoryMock = $this->getMockBuilder(CustomizationInterfaceFactory::class) + ->disableOriginalConstructor() + ->onlyMethods(['create']) + ->getMock(); + + $this->customizationInterfaceMock = $this->getMockBuilder(CustomizationInterface::class) + ->disableOriginalConstructor() + ->getMockForAbstractClass(); + + $this->filesystemMock = $this->createPartialMock(Filesystem::class, ['getDirectoryRead']); + + $this->readInterfaceMock = $this->getMockBuilder(ReadInterface::class) + ->disableOriginalConstructor() + ->getMockForAbstractClass(); + + $this->filesystemMock->expects($this->once()) + ->method('getDirectoryRead') + ->with(DirectoryList::ROOT) + ->willReturn($this->readInterfaceMock); + + $this->iteratorFactoryMock = $this->getMockBuilder(CompositeFileIteratorFactory::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->model = new FileResolver( + $this->moduleFileResolverMock, + $this->designMock, + $this->customizationFactoryMock, + $this->filesystemMock, + $this->iteratorFactoryMock + ); + } + + /** + * Test for get method with frontend scope. + * + * @param string $filename + * @param array $fileList + * + * @return void + * @dataProvider providerGetFrontend + */ + public function testGetFrontend(string $scope, string $filename, array $fileList, string $themeFilesPath): void + { + $this->moduleFileResolverMock->expects($this->once()) + ->method('get') + ->with($filename, $scope) + ->willReturn($fileList); + + $this->customizationFactoryMock->expects($this->any()) + ->method('create') + ->with(['theme' => $this->themeInterFaceMock]) + ->willReturn($this->customizationInterfaceMock); + + $this->customizationInterfaceMock->expects($this->once()) + ->method('getThemeFilesPath') + ->willReturn($themeFilesPath); + + $this->readInterfaceMock->expects($this->once()) + ->method('isExist') + ->with($themeFilesPath.'/etc/'.$filename) + ->willReturn(true); + + $this->iteratorFactoryMock->expects($this->once()) + ->method('create') + ->with( + [ + 'paths' => array_reverse([$themeFilesPath.'/etc/'.$filename]), + 'existingIterator' => $fileList + ] + ) + ->willReturn($fileList); + + $this->assertEquals($fileList, $this->model->get($filename, $scope)); + } + + /** + * Test for get method with global scope. + * + * @param string $filename + * @param array $fileList + * + * @return void + * @dataProvider providerGetGlobal + */ + public function testGetGlobal(string $scope, string $fileName, array $fileList): void + { + $this->moduleFileResolverMock->expects($this->once()) + ->method('get') + ->with($fileName, $scope) + ->willReturn($fileList); + $this->assertEquals($fileList, $this->model->get($fileName, $scope)); + } + + /** + * Data provider for get glocal scope tests. + * + * @return array + */ + public static function providerGetGlobal(): array + { + return [ + [ + 'global', + 'csp_whitelist.xml', + ['anyvendor/anymodule/etc/csp_whitelist.xml'] + ] + ]; + } + + /** + * Data provider for get frontend & adminhtml scope tests. + * + * @return array + */ + public static function providerGetFrontend(): array + { + return [ + [ + 'frontend', + 'csp_whitelist.xml', + ['themevendor/theme/etc/csp_whitelist.xml'], + 'themevendor/theme' + ], + [ + 'adminhtml', + 'csp_whitelist.xml', + ['adminthemevendor/admintheme/etc/csp_whitelist.xml'], + 'adminthemevendor/admintheme' + ] + ]; + } +} From e7b3597e0ed3907b0108d0dbc2d28e65f9137c57 Mon Sep 17 00:00:00 2001 From: engcom-Dash Date: Wed, 16 Apr 2025 13:57:34 +0530 Subject: [PATCH 5/5] 39672: Fixed Review Comments --- .../CspWhitelistXml/FileResolverTest.php | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/app/code/Magento/Csp/Test/Unit/Model/Collector/CspWhitelistXml/FileResolverTest.php b/app/code/Magento/Csp/Test/Unit/Model/Collector/CspWhitelistXml/FileResolverTest.php index 83917453c857d..cb691d69d10d3 100644 --- a/app/code/Magento/Csp/Test/Unit/Model/Collector/CspWhitelistXml/FileResolverTest.php +++ b/app/code/Magento/Csp/Test/Unit/Model/Collector/CspWhitelistXml/FileResolverTest.php @@ -71,16 +71,15 @@ protected function setUp(): void { $this->moduleFileResolverMock = $this->getMockBuilder(FileResolverInterface::class) ->disableOriginalConstructor() - ->onlyMethods(['get']) - ->getMockForAbstractClass(); + ->getMock(); $this->designMock = $this->getMockBuilder(DesignInterface::class) ->disableOriginalConstructor() - ->onlyMethods(['getDesignTheme']) - ->getMockForAbstractClass(); + ->getMock(); $this->themeInterFaceMock = $this->getMockBuilder(ThemeInterface::class) - ->getMockForAbstractClass(); + ->disableOriginalConstructor() + ->getMock(); $this->designMock->expects($this->once()) ->method('getDesignTheme') @@ -93,13 +92,13 @@ protected function setUp(): void $this->customizationInterfaceMock = $this->getMockBuilder(CustomizationInterface::class) ->disableOriginalConstructor() - ->getMockForAbstractClass(); + ->getMock(); $this->filesystemMock = $this->createPartialMock(Filesystem::class, ['getDirectoryRead']); $this->readInterfaceMock = $this->getMockBuilder(ReadInterface::class) - ->disableOriginalConstructor() - ->getMockForAbstractClass(); + ->disableOriginalConstructor() + ->getMock(); $this->filesystemMock->expects($this->once()) ->method('getDirectoryRead') @@ -122,17 +121,19 @@ protected function setUp(): void /** * Test for get method with frontend scope. * - * @param string $filename + * @param string $scope + * @param string $fileName * @param array $fileList + * @param string $themeFilesPath * * @return void * @dataProvider providerGetFrontend */ - public function testGetFrontend(string $scope, string $filename, array $fileList, string $themeFilesPath): void + public function testGetFrontend(string $scope, string $fileName, array $fileList, string $themeFilesPath): void { $this->moduleFileResolverMock->expects($this->once()) ->method('get') - ->with($filename, $scope) + ->with($fileName, $scope) ->willReturn($fileList); $this->customizationFactoryMock->expects($this->any()) @@ -146,26 +147,27 @@ public function testGetFrontend(string $scope, string $filename, array $fileList $this->readInterfaceMock->expects($this->once()) ->method('isExist') - ->with($themeFilesPath.'/etc/'.$filename) + ->with($themeFilesPath.'/etc/'.$fileName) ->willReturn(true); $this->iteratorFactoryMock->expects($this->once()) ->method('create') ->with( [ - 'paths' => array_reverse([$themeFilesPath.'/etc/'.$filename]), + 'paths' => array_reverse([$themeFilesPath.'/etc/'.$fileName]), 'existingIterator' => $fileList ] ) ->willReturn($fileList); - $this->assertEquals($fileList, $this->model->get($filename, $scope)); + $this->assertEquals($fileList, $this->model->get($fileName, $scope)); } /** * Test for get method with global scope. * - * @param string $filename + * @param string $scope + * @param string $fileName * @param array $fileList * * @return void @@ -181,7 +183,7 @@ public function testGetGlobal(string $scope, string $fileName, array $fileList): } /** - * Data provider for get glocal scope tests. + * Data provider for get global scope tests. * * @return array */