[BC] Unified CSRF configuration (#3147)

OpenMage · May 3, 2023 · 0a16e58 · 0a16e58
1 parent 78bd803
commit 0a16e58
Show file tree

Hide file tree

Showing 15 changed files with 35 additions and 83 deletions.
diff --git a/README.md b/README.md
@@ -272,7 +272,6 @@ If you see SQL errors after upgrading please remember to check for this specific
 - `catalog/product_image/progressive_threshold`
 - `catalog/search/search_separator`
 - `dev/log/max_level`
-- `newsletter/security/enable_form_key`
 - `sitemap/category/lastmod`
 - `sitemap/page/lastmod`
 - `sitemap/product/lastmod`

diff --git a/app/code/core/Mage/Adminhtml/Block/Checkout/Formkey.php b/app/code/core/Mage/Adminhtml/Block/Checkout/Formkey.php
@@ -29,16 +29,25 @@ class Mage_Adminhtml_Block_Checkout_Formkey extends Mage_Adminhtml_Block_Templat
      */
     public function canShow()
     {
-        return !Mage::getStoreConfigFlag('admin/security/validate_formkey_checkout');
+        return !Mage::helper('core')->isFormKeyEnabled();
     }
 
     /**
      * Get url for edit Advanced -> Admin section
      *
      * @return string
+     * @deprecated
      */
     public function getSecurityAdminUrl()
     {
         return Mage::helper("adminhtml")->getUrl('adminhtml/system_config/edit/section/admin');
     }
+
+    /**
+     * @return string
+     */
+    public function getEnableCSRFUrl()
+    {
+        return Mage::helper("adminhtml")->getUrl('adminhtml/system_config/edit/section/system');
+    }
 }
diff --git a/app/code/core/Mage/Checkout/controllers/MultishippingController.php b/app/code/core/Mage/Checkout/controllers/MultishippingController.php
@@ -227,7 +227,7 @@ public function addressesPostAction()
             return;
         }
 
-        if ($this->isFormkeyValidationOnCheckoutEnabled() && !$this->_validateFormKey()) {
+        if (!$this->_validateFormKey()) {
             $this->_redirect('*/*/addresses');
             return;
         }
@@ -348,7 +348,7 @@ public function backToShippingAction()
      */
     public function shippingPostAction()
     {
-        if ($this->isFormkeyValidationOnCheckoutEnabled() && !$this->_validateFormKey()) {
+        if (!$this->_validateFormKey()) {
             $this->_redirect('*/*/shipping');
             return;
         }
@@ -461,7 +461,7 @@ public function overviewAction()
             return $this;
         }
 
-        if ($this->isFormkeyValidationOnCheckoutEnabled() && !$this->_validateFormKey()) {
+        if (!$this->_validateFormKey()) {
             $this->_redirect('*/*/billing');
             return;
         }

diff --git a/app/code/core/Mage/Checkout/controllers/OnepageController.php b/app/code/core/Mage/Checkout/controllers/OnepageController.php
@@ -354,7 +354,7 @@ public function saveBillingAction()
             return;
         }
 
-        if ($this->isFormkeyValidationOnCheckoutEnabled() && !$this->_validateFormKey()) {
+        if (!$this->_validateFormKey()) {
             return;
         }
 
@@ -401,7 +401,7 @@ public function saveShippingAction()
             return;
         }
 
-        if ($this->isFormkeyValidationOnCheckoutEnabled() && !$this->_validateFormKey()) {
+        if (!$this->_validateFormKey()) {
             return;
         }
 
@@ -430,7 +430,7 @@ public function saveShippingMethodAction()
             return;
         }
 
-        if ($this->isFormkeyValidationOnCheckoutEnabled() && !$this->_validateFormKey()) {
+        if (!$this->_validateFormKey()) {
             return;
         }
 
@@ -470,7 +470,7 @@ public function savePaymentAction()
             return;
         }
 
-        if ($this->isFormkeyValidationOnCheckoutEnabled() && !$this->_validateFormKey()) {
+        if (!$this->_validateFormKey()) {
             return;
         }
 
@@ -553,7 +553,7 @@ protected function _initInvoice()
      */
     public function saveOrderAction()
     {
-        if ($this->isFormkeyValidationOnCheckoutEnabled() && !$this->_validateFormKey()) {
+        if (!$this->_validateFormKey()) {
             $this->_redirect('*/*');
             return;
         }

diff --git a/app/code/core/Mage/Checkout/etc/system.xml b/app/code/core/Mage/Checkout/etc/system.xml
@@ -215,23 +215,5 @@
                 </payment_failed>
             </groups>
         </checkout>
-        <admin>
-            <groups>
-                <security>
-                    <fields>
-                        <validate_formkey_checkout translate="label">
-                            <label>Enable Form Key Validation On Checkout</label>
-                            <frontend_type>select</frontend_type>
-                            <source_model>adminhtml/system_config_source_yesno</source_model>
-                            <sort_order>4</sort_order>
-                            <comment><![CDATA[<strong style="color:red">Important!</strong> Enabling this option means
-                            that your custom templates used in checkout process contain form_key output.
-                            Otherwise checkout may not work.]]></comment>
-                            <show_in_default>1</show_in_default>
-                        </validate_formkey_checkout>
-                    </fields>
-                </security>
-            </groups>
-        </admin>
     </sections>
 </config>
diff --git a/app/code/core/Mage/Checkout/sql/checkout_setup/install-1.6.0.0.php b/app/code/core/Mage/Checkout/sql/checkout_setup/install-1.6.0.0.php
@@ -779,12 +779,4 @@
     }
 }
 
-$setup->insert(
-    $this->getTable('core_config_data'),
-    [
-        'path' => 'admin/security/validate_formkey_checkout',
-        'value' => '1'
-    ]
-);
-
 $installer->endSetup();
diff --git a/app/code/core/Mage/Core/Controller/Front/Action.php b/app/code/core/Mage/Core/Controller/Front/Action.php
@@ -177,16 +177,18 @@ protected function _validateFormKey()
      */
     protected function _isFormKeyEnabled()
     {
-        return Mage::getStoreConfigFlag(self::XML_CSRF_USE_FLAG_CONFIG_PATH);
+        return Mage::helper('core')->isFormKeyEnabled();
     }
 
     /**
      * Check if form_key validation enabled on checkout process
      *
+     * @deprecated
+     * @see _isFormKeyEnabled
      * @return bool
      */
     protected function isFormkeyValidationOnCheckoutEnabled()
     {
-        return Mage::getStoreConfigFlag('admin/security/validate_formkey_checkout');
+        return $this->_isFormKeyEnabled();
     }
 }
diff --git a/app/code/core/Mage/Core/Helper/Data.php b/app/code/core/Mage/Core/Helper/Data.php
@@ -1000,4 +1000,12 @@ public function unEscapeCSVData($data)
         }
         return $data;
     }
+
+    /**
+     * @return bool
+     */
+    public function isFormKeyEnabled(): bool
+    {
+        return Mage::getStoreConfigFlag(Mage_Core_Controller_Front_Action::XML_CSRF_USE_FLAG_CONFIG_PATH);
+    }
 }
diff --git a/app/code/core/Mage/Newsletter/controllers/SubscriberController.php b/app/code/core/Mage/Newsletter/controllers/SubscriberController.php
@@ -21,11 +21,6 @@
  */
 class Mage_Newsletter_SubscriberController extends Mage_Core_Controller_Front_Action
 {
-    /**
-     * Use CSRF validation flag from newsletter config
-     */
-    public const XML_CSRF_USE_FLAG_CONFIG_PATH = 'newsletter/security/enable_form_key';
-
     /**
       * New subscription action
       */
@@ -127,14 +122,4 @@ public function unsubscribeAction()
         }
         $this->_redirectReferer();
     }
-
-    /**
-     * Check if form key validation is enabled in newsletter config.
-     *
-     * @return bool
-     */
-    protected function _isFormKeyEnabled()
-    {
-        return Mage::getStoreConfigFlag(self::XML_CSRF_USE_FLAG_CONFIG_PATH);
-    }
 }
diff --git a/app/code/core/Mage/Newsletter/etc/config.xml b/app/code/core/Mage/Newsletter/etc/config.xml
@@ -185,9 +185,6 @@
             <sending>
                 <set_return_path>0</set_return_path>
             </sending>
-            <security>
-                <enable_form_key>0</enable_form_key>
-            </security>
         </newsletter>
     </default>
     <crontab>

diff --git a/app/code/core/Mage/Newsletter/etc/system.xml b/app/code/core/Mage/Newsletter/etc/system.xml
@@ -105,25 +105,6 @@
                         </un_email_template>
                     </fields>
                 </subscription>
-                <security translate="label">
-                    <label>Security</label>
-                    <sort_order>1</sort_order>
-                    <show_in_default>1</show_in_default>
-                    <show_in_website>1</show_in_website>
-                    <show_in_store>1</show_in_store>
-                    <fields>
-                        <enable_form_key translate="label comment">
-                            <label>Enable Form Key Validation</label>
-                            <frontend_type>select</frontend_type>
-                            <source_model>adminhtml/system_config_source_yesno</source_model>
-                            <sort_order>1</sort_order>
-                            <show_in_default>1</show_in_default>
-                            <show_in_website>1</show_in_website>
-                            <show_in_store>1</show_in_store>
-                            <comment><![CDATA[<strong style="color:red">Important!</strong> Enabling this option means that your custom templates used for newsletter subscription must contain <code>form_key</code> block output. Otherwise newsletter subscription will not work.]]></comment>
-                        </enable_form_key>
-                    </fields>
-                </security>
             </groups>
         </newsletter>
     </sections>

diff --git a/app/design/adminhtml/default/default/template/notification/formkey.phtml b/app/design/adminhtml/default/default/template/notification/formkey.phtml
@@ -20,9 +20,9 @@
 <?php if ($this->canShow()): ?>
     <div class="notification-global notification-global-warning">
         <strong style="color:red">Important: </strong>
-        <span>Formkey validation on checkout disabled. This may expose security risks.
-        We strongly recommend to Enable Form Key Validation On Checkout in
-        <a href="<?php echo $this->getSecurityAdminUrl(); ?>">Admin / Security</a>,
-        for protect your own checkout process. </span>
+        <span>Formkey validation is disabled. This may expose you to security risks.
+        We strongly recommend to enable in
+        <a href="<?php echo $this->getEnableCSRFUrl(); ?>">Advanced / System / CSRF Protection</a>,
+        to protect your checkout, contact and newsletter forms.</span>
     </div>
 <?php endif ?>
diff --git a/app/locale/en_US/Mage_Checkout.csv b/app/locale/en_US/Mage_Checkout.csv
@@ -120,7 +120,6 @@
 "Email Address","Email Address"
 "Empty Cart","Empty Cart"
 "Empty Shopping Cart Content Before","Empty Shopping Cart Content Before"
-"Enable Form Key Validation On Checkout","Enable Form Key Validation On Checkout"
 "Enable Onepage Checkout","Enable Onepage Checkout"
 "Enable Terms and Conditions","Enable Terms and Conditions"
 "Enabled","Enabled"

diff --git a/app/locale/en_US/Mage_Core.csv b/app/locale/en_US/Mage_Core.csv
@@ -50,7 +50,7 @@
 "Before modifying the website code please make sure that it is not used in index.php.","Before modifying the website code please make sure that it is not used in index.php."
 "Block with name ""%s"" already exists","Block with name ""%s"" already exists"
 "Browser Capabilities Detection","Browser Capabilities Detection"
-"CSRF protection","CSRF protection"
+"CSRF protection","CSRF Protection"
 "CSS Settings","CSS Settings"
 "Cache Storage Management","Cache Storage Management"
 "Cache storage may contain additional data. Are you sure that you want flush it?","Cache storage may contain additional data. Are you sure that you want flush it?"

diff --git a/app/locale/en_US/Mage_Newsletter.csv b/app/locale/en_US/Mage_Newsletter.csv
@@ -1,5 +1,4 @@
 " Copy"," Copy"
-"<strong style=""color:red"">Important!</strong> Enabling this option means that your custom templates used for newsletter subscription must contain <code>form_key</code> block output. Otherwise newsletter subscription will not work.","<strong style=""color:red"">Important!</strong> Enabling this option means that your custom templates used for newsletter subscription must contain <code>form_key</code> block output. Otherwise newsletter subscription will not work."
 "Action","Action"
 "Add New Template","Add New Template"
 "Add to Queue","Add to Queue"
@@ -89,7 +88,6 @@
 "Save Newsletter","Save Newsletter"
 "Save Template","Save Template"
 "Save and Resume","Save and Resume"
-"Security","Security"
 "Selected problem subscribers have been unsubscribed.","Selected problem subscribers have been unsubscribed."
 "Selected problems have been deleted.","Selected problems have been deleted."
 "Sender","Sender"