Skip to content

Commit

Permalink
[BC] Unified CSRF configuration (OpenMage#3147)
Browse files Browse the repository at this point in the history
  • Loading branch information
fballiano authored May 3, 2023
1 parent 78bd803 commit 0a16e58
Show file tree
Hide file tree
Showing 15 changed files with 35 additions and 83 deletions.
1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
11 changes: 10 additions & 1 deletion app/code/core/Mage/Adminhtml/Block/Checkout/Formkey.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ public function addressesPostAction()
return;
}

if ($this->isFormkeyValidationOnCheckoutEnabled() && !$this->_validateFormKey()) {
if (!$this->_validateFormKey()) {
$this->_redirect('*/*/addresses');
return;
}
Expand Down Expand Up @@ -348,7 +348,7 @@ public function backToShippingAction()
*/
public function shippingPostAction()
{
if ($this->isFormkeyValidationOnCheckoutEnabled() && !$this->_validateFormKey()) {
if (!$this->_validateFormKey()) {
$this->_redirect('*/*/shipping');
return;
}
Expand Down Expand Up @@ -461,7 +461,7 @@ public function overviewAction()
return $this;
}

if ($this->isFormkeyValidationOnCheckoutEnabled() && !$this->_validateFormKey()) {
if (!$this->_validateFormKey()) {
$this->_redirect('*/*/billing');
return;
}
Expand Down
10 changes: 5 additions & 5 deletions app/code/core/Mage/Checkout/controllers/OnepageController.php
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ public function saveBillingAction()
return;
}

if ($this->isFormkeyValidationOnCheckoutEnabled() && !$this->_validateFormKey()) {
if (!$this->_validateFormKey()) {
return;
}

Expand Down Expand Up @@ -401,7 +401,7 @@ public function saveShippingAction()
return;
}

if ($this->isFormkeyValidationOnCheckoutEnabled() && !$this->_validateFormKey()) {
if (!$this->_validateFormKey()) {
return;
}

Expand Down Expand Up @@ -430,7 +430,7 @@ public function saveShippingMethodAction()
return;
}

if ($this->isFormkeyValidationOnCheckoutEnabled() && !$this->_validateFormKey()) {
if (!$this->_validateFormKey()) {
return;
}

Expand Down Expand Up @@ -470,7 +470,7 @@ public function savePaymentAction()
return;
}

if ($this->isFormkeyValidationOnCheckoutEnabled() && !$this->_validateFormKey()) {
if (!$this->_validateFormKey()) {
return;
}

Expand Down Expand Up @@ -553,7 +553,7 @@ protected function _initInvoice()
*/
public function saveOrderAction()
{
if ($this->isFormkeyValidationOnCheckoutEnabled() && !$this->_validateFormKey()) {
if (!$this->_validateFormKey()) {
$this->_redirect('*/*');
return;
}
Expand Down
18 changes: 0 additions & 18 deletions app/code/core/Mage/Checkout/etc/system.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Original file line number Diff line number Diff line change
Expand Up @@ -779,12 +779,4 @@
}
}

$setup->insert(
$this->getTable('core_config_data'),
[
'path' => 'admin/security/validate_formkey_checkout',
'value' => '1'
]
);

$installer->endSetup();
6 changes: 4 additions & 2 deletions app/code/core/Mage/Core/Controller/Front/Action.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
8 changes: 8 additions & 0 deletions app/code/core/Mage/Core/Helper/Data.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
15 changes: 0 additions & 15 deletions app/code/core/Mage/Newsletter/controllers/SubscriberController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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);
}
}
3 changes: 0 additions & 3 deletions app/code/core/Mage/Newsletter/etc/config.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
19 changes: 0 additions & 19 deletions app/code/core/Mage/Newsletter/etc/system.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 ?>
1 change: 0 additions & 1 deletion app/locale/en_US/Mage_Checkout.csv
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion app/locale/en_US/Mage_Core.csv
Original file line number Diff line number Diff line change
Expand Up @@ -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?"
Expand Down
2 changes: 0 additions & 2 deletions app/locale/en_US/Mage_Newsletter.csv
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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"
Expand Down

0 comments on commit 0a16e58

Please sign in to comment.