amzn / amazon-payments-magento-2-plugin

Extension to enable Amazon Pay on Magento 2
https://amzn.github.io/amazon-payments-magento-2-plugin/
Apache License 2.0
109 stars 77 forks source link

CSP compliant inline-script rendering in Magento Checkout #1242

Closed nige-one closed 5 months ago

nige-one commented 5 months ago

Use usual extension's implementation and configuration in vanilla Magento 2.4.7 and browse to checkout. The extension's UI component inline configuration will be refused to evaluate by CSP ruling. The extension has to be fixed by using Magento's CSP functionalities to correctly inline Javascript.

image

image

Your setup

sgabhart22 commented 5 months ago

Hello @nige-one ,

Thanks for bringing this up, this is an issue we'd investigated a bit before, but I believe your screenshot highlighted the major problem! I've done a little more testing locally, and I'm attaching a patch here that should restore Amazon Pay functionality at checkout and conform to the stricter CSP requirements in M2.4.7. Please let us know if you find any issues with it!

Also note that this patch was created against version 5.17.0 of the module, but should work for a number of previous versions as well since this area of frontend code isn't often changed.

csp.patch.txt

Thanks, Spencer

nige-one commented 5 months ago

Hi @sgabhart22, thanks for your effort and sending over the patch. Before you are planning to release this I would suggest that you align more to the Magento way of implementing this. Please take a look:

https://github.com/magento/magento2/blob/e81f28e4e6739924594c266b011c68b08a227c2d/app/code/Magento/Bundle/view/adminhtml/templates/product/stock/disabler.phtml#L9-L20

This will also address the system's configuration thus providing the needed <script> tag attribution (i.e. nonce). Remember that the checkout is non-cachable area of Magento but the the rest of the FE mostly is. Your implementation will lead to FE caching issues (haven't tested)! The Magento implementation will handle this correctly.

I hope this helps.

sgabhart22 commented 5 months ago

Thanks for the tip @nige-one , please have a look at the changes on this commit and let me know if this approach looks a little better to you:

https://github.com/BearGroup/amazon-payments-magento-2-plugin/commit/78b0cd4de138a9471d40141403e4c6d009e5412e

sfritzsche commented 5 months ago

@nige-one The fact that a script is written via the "renderer" is understandable. Can you explain the sentence "Your implementation will lead to FE caching issues (haven't tested)! The Magento implementation will handle this correctly." in more detail ? We do not understand the connection to caching.

sfritzsche commented 5 months ago

Fyi: We have added a temporary patch (vaimo/composer-patches) for version 5.17.0:

--- etc/csp_whitelist.xml   
+++ etc/csp_whitelist.xml
@@ -82,7 +82,7 @@
                 <value id="media_amazon_fr" type="host">*.media-amazon.fr</value>
                 <value id="media_amazon_es" type="host">*.media-amazon.es</value>
                 <value id="media_amazon_de" type="host">*.media-amazon.de</value>
-                <value id="static-eu_payments-amazon_com" type="host">static-eu.payments-amazon.com</value>
+                <value id="payments-amazon_com" type="host">*.payments-amazon.com</value>
             </values>
         </policy>
         <policy id="form-action">
--- view/frontend/templates/config.phtml    
+++ view/frontend/templates/config.phtml
@@ -19,18 +19,21 @@
     <?php
     $frontName = $block->getRequest()->getFrontName();
     $pathInfo = $block->getRequest()->getPathInfo();
-    ?>
-<script>
+    $config = $block->getJsonConfig();

+    $script = <<<script
     require (['uiRegistry'], function(registry) {
-        registry.set('amazonPay', <?= /* @noEscape */ $block->getJsonConfig() ?>)
+        registry.set('amazonPay', ${config});
     });
-
+    script;
+    ?>
     <?php if ($frontName != 'checkout' || (strpos($pathInfo, 'checkout/cart') !== false)): ?>
-    require (['Amazon_Pay/js/model/storage'], function(amazonStorage) {
-        amazonStorage.clearAmazonCheckout();
-    });
+        <?php
+        $script .= PHP_EOL . "require (['Amazon_Pay/js/model/storage'], function(amazonStorage) {
+            amazonStorage.clearAmazonCheckout();
+        });";
+        ?>
     <?php endif; ?>

-</script>
+    <?= /* noEscape */ $csp->renderTag('script', [], $script) ?>
 <?php endif ?>
nige-one commented 5 months ago

Hi @sfritzsche in the patch before @sgabhart22 was rendering the JS nonce directly into the <script> which could have lead to a caching issue by caching the nonce value. Since the nonce would change in the none cached HTTP header, it would have been invalidated. By using the Magento functionalities this should be no problem anymore.

sgabhart22 commented 5 months ago

Hello @nige-one and @sfritzsche ,

These fixes have been included in version 5.17.1, which is now live on the Marketplace! We'll close this issue shortly, but please feel free to reach out in the future with any other problems. Thanks again for the contributions!

Spencer