Merge pull request #8971 from magento-cia/cia-2.4.8-beta1-develop-bugfix-05292024

Cia 2.4.8 beta1 develop bugfix 05292024

Author: pawan-adobe-security

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

@@ -278,8 +278,10 @@ protected function _getAllowedExtensions()
      */
     private function setValueAfterValidation(string $value): void
     {
-        // avoid intercepting value
-        if (preg_match('/[^a-z0-9_\/\\-\\.]+/i', $value)) {
+        if (preg_match('/[^a-z0-9_\/\\-\\.]+/i', $value)
+            // phpcs:ignore Magento2.Functions.DiscouragedFunction
+            || !$this->_mediaDirectory->isFile($this->_getUploadDir() . DIRECTORY_SEPARATOR . basename($value))
+        ) {
             throw new LocalizedException(__('Invalid file name'));
         }
 

app/code/Magento/ImportExport/Controller/Adminhtml/Export/File/Download.php

@@ -16,6 +16,8 @@
 use Magento\Framework\Filesystem;
 use Magento\ImportExport\Model\LocalizedFileName;
 use Throwable;
+use Magento\Framework\Controller\Result\Redirect;
+use Magento\Framework\App\ResponseInterface;
 
 /**
  * Controller that download file by name.
@@ -25,7 +27,7 @@ class Download extends ExportController implements HttpGetActionInterface
     /**
      * Url to this controller
      */
-    const URL = 'adminhtml/export_file/download/';
+    public const URL = 'adminhtml/export_file/download/';
 
     /**
      * @var FileFactory
@@ -64,13 +66,24 @@ public function __construct(
     /**
      * Controller basic method implementation.
      *
-     * @return \Magento\Framework\Controller\Result\Redirect | \Magento\Framework\App\ResponseInterface
+     * @return Redirect|ResponseInterface
      */
     public function execute()
     {
         $resultRedirect = $this->resultRedirectFactory->create();
         $resultRedirect->setPath('adminhtml/export/index');
+
         $fileName = $this->getRequest()->getParam('filename');
+
+        if (empty($fileName)) {
+            $this->messageManager->addErrorMessage(__('Please provide valid export file name'));
+
+            return $resultRedirect;
+        }
+
+        // phpcs:ignore Magento2.Functions.DiscouragedFunction
+        $fileName = basename($fileName);
+
         $exportDirectory = $this->filesystem->getDirectoryRead(DirectoryList::VAR_IMPORT_EXPORT);
 
         try {

app/code/Magento/ImportExport/Controller/Adminhtml/History/Download.php

@@ -3,11 +3,18 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+
+declare(strict_types=1);
+
 namespace Magento\ImportExport\Controller\Adminhtml\History;
 
 use Magento\Framework\App\Action\HttpGetActionInterface;
 use Magento\Framework\App\Filesystem\DirectoryList;
+use Magento\Framework\Controller\ResultInterface;
+use Magento\ImportExport\Helper\Report;
 use Magento\ImportExport\Model\Import;
+use Magento\Framework\Controller\Result\Redirect;
+use Magento\Framework\App\ResponseInterface;
 
 /**
  * Download history controller
@@ -44,20 +51,27 @@ public function __construct(
     /**
      * Download backup action
      *
-     * @return void|\Magento\Backend\App\Action
+     * @return ResponseInterface|Redirect|ResultInterface
+     * @throws \Exception
      */
     public function execute()
     {
+        $resultRedirect = $this->resultRedirectFactory->create();
+        $resultRedirect->setPath('*/history');
+
+        $fileName = $this->getRequest()->getParam('filename');
+
+        if (empty($fileName)) {
+            return $resultRedirect;
+        }
+
         // phpcs:ignore Magento2.Functions.DiscouragedFunction
-        $fileName = basename($this->getRequest()->getParam('filename'));
+        $fileName = basename($fileName);
 
-        /** @var \Magento\ImportExport\Helper\Report $reportHelper */
-        $reportHelper = $this->_objectManager->get(\Magento\ImportExport\Helper\Report::class);
+        /** @var Report $reportHelper */
+        $reportHelper = $this->_objectManager->get(Report::class);
 
         if (!$reportHelper->importFileExists($fileName)) {
-            /** @var \Magento\Backend\Model\View\Result\Redirect $resultRedirect */
-            $resultRedirect = $this->resultRedirectFactory->create();
-            $resultRedirect->setPath('*/history');
             return $resultRedirect;
         }
 

app/code/Magento/ImportExport/Test/Unit/Controller/Adminhtml/History/DownloadTest.php

@@ -8,13 +8,13 @@
 namespace Magento\ImportExport\Test\Unit\Controller\Adminhtml\History;
 
 use Magento\Backend\App\Action\Context;
-use Magento\Backend\Model\View\Result\Redirect;
 use Magento\Framework\App\Filesystem\DirectoryList;
 use Magento\Framework\App\Request\Http;
 use Magento\Framework\App\Response\Http\FileFactory;
 use Magento\Framework\App\ResponseInterface;
 use Magento\Framework\Controller\Result\Raw;
 use Magento\Framework\Controller\Result\RawFactory;
+use Magento\Framework\Controller\Result\Redirect;
 use Magento\Framework\Controller\Result\RedirectFactory;
 use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper;
 use Magento\ImportExport\Controller\Adminhtml\History\Download;
@@ -181,8 +181,7 @@ public function executeDataProvider()
     {
         return [
             'Normal file name' => ['filename.csv', 'filename.csv'],
-            'Relative file name' => ['../../../../../../../../etc/passwd', 'passwd'],
-            'Empty file name' => ['', ''],
+            'Relative file name' => ['../../../../../../../../etc/passwd', 'passwd']
         ];
     }
 
@@ -196,4 +195,27 @@ public function testExecuteFileNotFound()
         $this->resultRaw->expects($this->never())->method('setContents');
         $this->downloadController->execute();
     }
+
+    /**
+     * Test execute() with return Redirect
+     * @param string|null $requestFilename
+     * @dataProvider executeWithRedirectDataProvider
+     */
+    public function testExecuteWithRedirect(?string $requestFilename): void
+    {
+        $this->request->method('getParam')->with('filename')->willReturn($requestFilename);
+        $this->resultRaw->expects($this->never())->method('setContents');
+        $this->assertSame($this->redirect, $this->downloadController->execute());
+    }
+
+    /**
+     * @return array
+     */
+    public function executeWithRedirectDataProvider(): array
+    {
+        return [
+            'null file name' => [null],
+            'empty file name' => [''],
+        ];
+    }
 }

app/code/Magento/Integration/Block/Adminhtml/Integration/Edit/Tab/Info.php

@@ -3,6 +3,9 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+
+declare(strict_types=1);
+
 namespace Magento\Integration\Block\Adminhtml\Integration\Edit\Tab;
 
 use Magento\Integration\Controller\Adminhtml\Integration;
@@ -19,23 +22,23 @@ class Info extends \Magento\Backend\Block\Widget\Form\Generic implements \Magent
     /**#@+
      * Form elements names.
      */
-    const HTML_ID_PREFIX = 'integration_properties_';
+    public const HTML_ID_PREFIX = 'integration_properties_';
 
-    const DATA_ID = 'integration_id';
+    public const DATA_ID = 'integration_id';
 
-    const DATA_NAME = 'name';
+    public const DATA_NAME = 'name';
 
-    const DATA_EMAIL = 'email';
+    public const DATA_EMAIL = 'email';
 
-    const DATA_ENDPOINT = 'endpoint';
+    public const DATA_ENDPOINT = 'endpoint';
 
-    const DATA_IDENTITY_LINK_URL = 'identity_link_url';
+    public const DATA_IDENTITY_LINK_URL = 'identity_link_url';
 
-    const DATA_SETUP_TYPE = 'setup_type';
+    public const DATA_SETUP_TYPE = 'setup_type';
 
-    const DATA_CONSUMER_ID = 'consumer_id';
+    public const DATA_CONSUMER_ID = 'consumer_id';
 
-    const DATA_CONSUMER_PASSWORD = 'current_password';
+    public const DATA_CONSUMER_PASSWORD = 'current_password';
 
     /**#@-*/
 
@@ -161,6 +164,7 @@ protected function _addGeneralFieldset($form, $integrationData)
                 'label' => __('Identity link URL'),
                 'name' => self::DATA_IDENTITY_LINK_URL,
                 'disabled' => $disabled,
+                'class' => 'validate-url',
                 'note' => __(
                     'URL to redirect user to link their 3rd party account with this Magento integration credentials.'
                 )

app/code/Magento/Integration/Controller/Adminhtml/Integration.php

@@ -3,10 +3,14 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+
+declare(strict_types=1);
+
 namespace Magento\Integration\Controller\Adminhtml;
 
 use Magento\Backend\App\Action;
-use Magento\Integration\Api\OauthServiceInterface as IntegrationOauthService;
+use Magento\Framework\App\ObjectManager;
+use Magento\Framework\Url\Validator;
 
 /**
  * Controller for integrations management.
@@ -20,18 +24,18 @@ abstract class Integration extends Action
      *
      * @see _isAllowed()
      */
-    const ADMIN_RESOURCE = 'Magento_Integration::integrations';
+    public const ADMIN_RESOURCE = 'Magento_Integration::integrations';
 
     /** Param Key for extracting integration id from Request */
-    const PARAM_INTEGRATION_ID = 'id';
+    public const PARAM_INTEGRATION_ID = 'id';
 
     /** Reauthorize flag is used to distinguish activation from reauthorization */
-    const PARAM_REAUTHORIZE = 'reauthorize';
+    public const PARAM_REAUTHORIZE = 'reauthorize';
 
-    const REGISTRY_KEY_CURRENT_INTEGRATION = 'current_integration';
+    public const REGISTRY_KEY_CURRENT_INTEGRATION = 'current_integration';
 
     /** Saved API form data session key */
-    const REGISTRY_KEY_CURRENT_RESOURCE = 'current_resource';
+    public const REGISTRY_KEY_CURRENT_RESOURCE = 'current_resource';
 
     /**
      * @var \Magento\Framework\Registry
@@ -73,6 +77,11 @@ abstract class Integration extends Action
      */
     protected $escaper;
 
+    /**
+     * @var Validator
+     */
+    protected $urlValidator;
+
     /**
      * @param \Magento\Backend\App\Action\Context $context
      * @param \Magento\Framework\Registry $registry
@@ -83,6 +92,9 @@ abstract class Integration extends Action
      * @param \Magento\Integration\Helper\Data $integrationData
      * @param \Magento\Framework\Escaper $escaper
      * @param \Magento\Integration\Model\ResourceModel\Integration\Collection $integrationCollection
+     * @param Validator|null $urlValidator
+     *
+     * @SuppressWarnings(PHPMD.ExcessiveParameterList)
      */
     public function __construct(
         \Magento\Backend\App\Action\Context $context,
@@ -93,7 +105,8 @@ public function __construct(
         \Magento\Framework\Json\Helper\Data $jsonHelper,
         \Magento\Integration\Helper\Data $integrationData,
         \Magento\Framework\Escaper $escaper,
-        \Magento\Integration\Model\ResourceModel\Integration\Collection $integrationCollection
+        \Magento\Integration\Model\ResourceModel\Integration\Collection $integrationCollection,
+        Validator $urlValidator = null
     ) {
         parent::__construct($context);
         $this->_registry = $registry;
@@ -104,6 +117,7 @@ public function __construct(
         $this->_integrationData = $integrationData;
         $this->escaper = $escaper;
         $this->_integrationCollection = $integrationCollection;
+        $this->urlValidator = $urlValidator ?: ObjectManager::getInstance()->get(Validator::class);
         parent::__construct($context);
     }
 

app/code/Magento/Integration/Controller/Adminhtml/Integration/Save.php

@@ -3,6 +3,9 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+
+declare(strict_types=1);
+
 namespace Magento\Integration\Controller\Adminhtml\Integration;
 
 use Magento\Framework\App\Action\HttpPostActionInterface as HttpPostActionInterface;
@@ -30,6 +33,7 @@ class Save extends \Magento\Integration\Controller\Adminhtml\Integration impleme
      *
      * @return SecurityCookie
      * @deprecated 100.1.0
+     * @see we don't recommend this approach anymore
      */
     private function getSecurityCookie()
     {
@@ -76,7 +80,7 @@ public function execute()
             $this->messageManager->addErrorMessage($this->escaper->escapeHtml($e->getMessage()));
             $this->_getSession()->setIntegrationData($this->getRequest()->getPostValue());
             $this->_redirectOnSaveError();
-        } catch (\Magento\Framework\Exception\LocalizedException $e) {
+        } catch (LocalizedException $e) {
             $this->messageManager->addErrorMessage($this->escaper->escapeHtml($e->getMessage()));
             $this->_redirectOnSaveError();
         } catch (\Exception $e) {
@@ -148,6 +152,8 @@ protected function _redirectOnSaveError()
      *
      * @param array $integrationData
      * @return void
+     * @throws IntegrationException
+     * @throws LocalizedException
      */
     private function processData($integrationData)
     {
@@ -157,7 +163,15 @@ private function processData($integrationData)
             if (!isset($data['resource'])) {
                 $integrationData['resource'] = [];
             }
+
             $integrationData = array_merge($integrationData, $data);
+
+            // Check if the Identity Link URL field is not empty and then validate it
+            $url = $integrationData[Info::DATA_IDENTITY_LINK_URL] ?? null;
+            if (!empty($url) && !$this->urlValidator->isValid($url)) {
+                throw new LocalizedException(__('Invalid Identity Link URL'));
+            }
+
             if (!isset($integrationData[Info::DATA_ID])) {
                 $integration = $this->_integrationService->create($integrationData);
             } else {

app/code/Magento/Integration/i18n/en_US.csv

@@ -125,3 +125,4 @@ OAuth,OAuth
 "Integrations API configuration file","Integrations API configuration file"
 "We couldn't find any records.","We couldn't find any records."
 Status,Status
+"Invalid Identity Link URL", "Invalid Identity Link URL"