Merge pull request #19 from martijnvdkerkhof/feature/implement-fallba…

…ck-directives Feature/implement fallback directives
experius · Sep 16, 2021 · dd5c807 · dd5c807
2 parents f883080 + caca293
commit dd5c807
Show file tree

Hide file tree

Showing 6 changed files with 86 additions and 28 deletions.
diff --git a/Api/Data/ReportInterface.php b/Api/Data/ReportInterface.php
@@ -18,6 +18,15 @@ interface ReportInterface extends \Magento\Framework\Api\ExtensibleDataInterface
     const BLOCKED_URI = 'blocked_uri';
     const COUNT = 'count';
     const WHITELIST = 'whitelist';
+    // Array of directives that are not in Magento\Csp\Model\Policy\FetchPolicy::POLICIES
+    // but do have a fallback directive (cf. see https://www.w3.org/TR/CSP3/)
+    const DIRECTIVES_TO_FALLBACKS = [
+        'script-src-attr' => 'script-src',
+        'script-src-elem' => 'script-src',
+        'style-src-attr' => 'style-src',
+        'style-src-elem' => 'style-src',
+        'worker-src' => 'child-src',
+    ];
 
     /**
      * Get report_id

diff --git a/Controller/Adminhtml/Report/Whitelist.php b/Controller/Adminhtml/Report/Whitelist.php
@@ -25,10 +25,9 @@ class Whitelist extends \Experius\Csp\Controller\Adminhtml\Report
      */
     public function __construct(
         \Magento\Backend\App\Action\Context $context,
-        \Magento\Framework\Registry $coreRegistry,
-        ReportRepository $reportRepository
-    )
-    {
+        \Magento\Framework\Registry         $coreRegistry,
+        ReportRepository                    $reportRepository
+    ) {
         $this->reportRepository = $reportRepository;
         parent::__construct($context, $coreRegistry);
     }
@@ -48,7 +47,7 @@ public function execute()
             try {
                 $report = $this->reportRepository->get($id);
 
-                $message = $message = 'You whitelisted the Csp Report.';
+                $message = 'You whitelisted the Csp Report.';
                 if ($report) {
                     $report->getWhitelist() ? $report->setWhitelist(false) && $message = 'You removed the Csp whitelisting for this Report.' : $report->setWhitelist(true);
                     $this->reportRepository->update($report);

diff --git a/Model/Data/Report.php b/Model/Data/Report.php
@@ -11,7 +11,6 @@
 
 class Report extends \Magento\Framework\Api\AbstractExtensibleObject implements ReportInterface
 {
-
     /**
      * Get report_id
      * @return string|null
@@ -95,7 +94,8 @@ public function setReferrer($referrer)
      */
     public function getViolatedDirective()
     {
-        return $this->_get(self::VIOLATED_DIRECTIVE);
+        $directive = $this->_get(self::VIOLATED_DIRECTIVE);
+        return self::DIRECTIVES_TO_FALLBACKS[$directive] ?? $directive;
     }
 
     /**

diff --git a/Model/Report.php b/Model/Report.php
@@ -13,12 +13,20 @@
 
 class Report extends \Magento\Framework\Model\AbstractModel
 {
-
+    /**
+     * @var DataObjectHelper
+     */
     protected $dataObjectHelper;
 
-    protected $_eventPrefix = 'experius_csp_report';
+    /**
+     * @var ReportInterfaceFactory
+     */
     protected $reportDataFactory;
 
+    /**
+     * @var string
+     */
+    protected $_eventPrefix = 'experius_csp_report';
 
     /**
      * @param \Magento\Framework\Model\Context $context
@@ -50,14 +58,14 @@ public function __construct(
     public function getDataModel()
     {
         $reportData = $this->getData();
-        
+
         $reportDataObject = $this->reportDataFactory->create();
         $this->dataObjectHelper->populateWithArray(
             $reportDataObject,
             $reportData,
             ReportInterface::class
         );
-        
+
         return $reportDataObject;
     }
 }

diff --git a/Model/ReportRepository.php b/Model/ReportRepository.php
@@ -23,7 +23,6 @@
 use Magento\Framework\Exception\CouldNotSaveException;
 use Magento\Framework\Exception\NoSuchEntityException;
 use Magento\Framework\Reflection\DataObjectProcessor;
-use Magento\Store\Model\StoreManagerInterface;
 use Magento\Framework\Api\SearchCriteriaBuilder;
 
 class ReportRepository implements ReportRepositoryInterface
@@ -103,18 +102,18 @@ class ReportRepository implements ReportRepositoryInterface
      * @param FetchPolicyReader $fetchPolicyReader
      */
     public function __construct(
-        ResourceReport $resource,
-        ReportFactory $reportFactory,
-        ReportInterfaceFactory $dataReportFactory,
-        ReportCollectionFactory $reportCollectionFactory,
+        ResourceReport                      $resource,
+        ReportFactory                       $reportFactory,
+        ReportInterfaceFactory              $dataReportFactory,
+        ReportCollectionFactory             $reportCollectionFactory,
         ReportSearchResultsInterfaceFactory $searchResultsFactory,
-        DataObjectHelper $dataObjectHelper,
-        DataObjectProcessor $dataObjectProcessor,
-        CollectionProcessorInterface $collectionProcessor,
-        JoinProcessorInterface $extensionAttributesJoinProcessor,
-        ExtensibleDataObjectConverter $extensibleDataObjectConverter,
-        SearchCriteriaBuilder $searchCriteriaBuilder,
-        FetchPolicyReader $fetchPolicyReader
+        DataObjectHelper                    $dataObjectHelper,
+        DataObjectProcessor                 $dataObjectProcessor,
+        CollectionProcessorInterface        $collectionProcessor,
+        JoinProcessorInterface              $extensionAttributesJoinProcessor,
+        ExtensibleDataObjectConverter       $extensibleDataObjectConverter,
+        SearchCriteriaBuilder               $searchCriteriaBuilder,
+        FetchPolicyReader                   $fetchPolicyReader
     ) {
         $this->resource = $resource;
         $this->reportFactory = $reportFactory;
@@ -147,7 +146,7 @@ public function save(
 
         if (!$existingReport) {
             try {
-                if (!$this->fetchPolicyReader->canRead($report->getViolatedDirective())) {
+                if (!$this->canDirectiveBeWhitelisted($report->getViolatedDirective())) {
                     $report->setWhitelist(Whitelist::STATUS_NOT_ALLOWED);
                 }
 
@@ -161,7 +160,7 @@ public function save(
             return $report;
         } else {
             try {
-                if (!$this->fetchPolicyReader->canRead($report->getViolatedDirective())) {
+                if (!$this->canDirectiveBeWhitelisted($report->getViolatedDirective())) {
                     $existingReport->setWhitelist(Whitelist::STATUS_NOT_ALLOWED);
                 }
 
@@ -277,10 +276,13 @@ public function doesReportExistAlready($report)
             return false;
         }
 
+        $strippedBlockedUri = explode('?', $report->getBlockedUri())[0];
+        $strippedDocumentUri = explode('?', $report->getDocumentUri())[0];
+
         $searchCriteria = $this->searchCriteriaBuilder
             ->addFilter(ReportInterface::VIOLATED_DIRECTIVE, $report->getViolatedDirective())
-            ->addFilter(ReportInterface::BLOCKED_URI, $report->getBlockedUri())
-            ->addFilter(ReportInterface::DOCUMENT_URI, $report->getDocumentUri())
+            ->addFilter(ReportInterface::BLOCKED_URI, $strippedBlockedUri . '%', 'like')
+            ->addFilter(ReportInterface::DOCUMENT_URI, $strippedDocumentUri . '%', 'like')
             ->create();
 
         if ($this->getList($searchCriteria)->getTotalCount() < 1) {
@@ -295,6 +297,10 @@ public function doesReportExistAlready($report)
         return false;
     }
 
+    /**
+     * @param $report
+     * @return Report
+     */
     public function createReportModel($report)
     {
         $reportData = $this->extensibleDataObjectConverter->toNestedArray(
@@ -323,4 +329,39 @@ public function extractHostSource(string $url): string
         }
         return $url;
     }
+
+    /**
+     * Check if the violated directive can be whitelisted
+     *
+     * @param string $directive
+     * @return bool
+     */
+    public function canDirectiveBeWhitelisted(string $directive): bool
+    {
+        if ($this->fetchPolicyReader->canRead($directive)) {
+            return true;
+        }
+
+        if ($this->directiveHasFallbackDirective($directive)) {
+            return true;
+        }
+
+        return false;
+    }
+
+    /**
+     * Check if a given directive has a fallback.
+     *
+     * see https://www.w3.org/TR/CSP3/
+     *
+     * @param string $directive
+     * @return bool
+     */
+    public function directiveHasFallbackDirective(string $directive): bool
+    {
+        if (in_array($directive, ReportInterface::DIRECTIVES_TO_FALLBACKS)) {
+            return true;
+        }
+        return false;
+    }
 }
diff --git a/Ui/Component/Listing/Column/ReportActions.php b/Ui/Component/Listing/Column/ReportActions.php
@@ -8,6 +8,7 @@
 namespace Experius\Csp\Ui\Component\Listing\Column;
 
 use Experius\Csp\Model\ReportRepository;
+use Experius\Csp\Api\Data\ReportInterface;
 use Magento\Csp\Model\Collector\Config\FetchPolicyReader;
 use Magento\Framework\UrlInterface;
 use Magento\Framework\View\Element\UiComponent\ContextInterface;
@@ -95,7 +96,7 @@ public function prepareDataSource(array $dataSource)
                             ]
                         ],
                     ];
-                    if ($this->fetchPolicyReader->canRead($item['violated_directive'])) {
+                    if ($this->reportRepository->canDirectiveBeWhitelisted($item['violated_directive'])) {
                         $message = $item['whitelist'] ? __('Are you sure you want to de-whitelist this record?') : __('Are you sure you want to whitelist this record?');
                         $item[$this->getData('name')]['whitelist'] = [
                             'href' => $this->urlBuilder->getUrl(