Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why "Enable Form Key Validation On Checkout" is set to Yes by default? #3218

Closed
addison74 opened this issue Apr 24, 2023 · 6 comments
Closed

Comments

@addison74
Copy link
Contributor

I cloned OpenMage 20.1.0-rc4 to do more test without Magento Sample Pack. After installing it from scratch when I accessed the Backend, I no longer saw the familiar message under the menu like before

Important: Formkey validation on checkout disabled. This may expose security risks. We strongly recommend to Enable Form Key Validation On Checkout in Admin / Security, to protect your own checkout process

form_key_2

When I checked the value, I found that it is now set to Yes by default. When did this happen? It would have been natural for the value to remain set to No precisely as a warning to modify the template in case a custom theme is used.

form_key

@fballiano
Copy link
Contributor

it should come from #871 but I agree that it should be on, it is an important security feature :-)

@addison74
Copy link
Contributor Author

I have some comments related to this issue:

  1. After installation and accessing the Backend, that warning message was extremely important because it leaves it up to the administrator to activate the option, only after checking that his templates are ready. As it is now, if it uses a theme other than RWD, without changes, being activated by default then it will obviously create problems.

  2. The fact that it is activated by default, the administrator will not get to read the Important label and understand that if his custom templates are not updated according to the change, then the checkout process will not be completed.

  3. The change should have been mentioned in the README because it creates a beautiful BC with the other custom themes.

My opinion is that we should revert that PR. as a strong argument I come from the fact that the displayed warning message causes me to take actions. The fact that the warning is no longer displayed, I consider it a serious problem.

@fballiano
Copy link
Contributor

The configuration for the CSRF is here:
Screenshot 2023-04-29 alle 17 58 20

the fact that it's enabled by default is actually a very old commit:
Screenshot 2023-04-29 alle 18 00 30

at least if I'm not mistaken

@fballiano
Copy link
Contributor

On new installs CSRF really should be enabled by default, since the default checkout works perfectly with CSRF. That setting can be altered by people who need to disable it but it shouldn't be encouraged. IMHO this bug report could be closed.

@addison74
Copy link
Contributor Author

I agree, but we must insert an information in the README about this change and those who have not activated it so far to modify their custom templates (if they use them).

@sreichel
Copy link
Contributor

sreichel commented Aug 1, 2023

Just an idea ... maybe its possible to search custom templates for missing formkey and show this hint depending on it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants