Set dummy app forgery protection to false

[FrancescoAiello01]

Description
This reverts a previous change setting dummy app forgery protection to true. Rails prefers testing to be performed with forgery protection set to false.

Verification effort: test suite passes

Ref #3873

Checklist:

• I have followed Pull Request guidelines
• I have added a detailed description into each commit message
• I have updated Guides and README accordingly to this change (if needed)
• I have added tests to cover this change (if needed)
• I have attached screenshots to this PR for visual changes (if needed)

[kennyadsl]

Thanks @FrancescoAiello01! I have the feeling we can skip some lines of code. I know they were there before but maybe they were useless already. Let me know!

[kennyadsl]

Do you think this is necessary?

[FrancescoAiello01]

I tried to not include that line, but I ran into an issue in frontend/app/controllers/spree/orders_controller.rb on line 14: skip_before_action :verify_authenticity_token, only: [:populate]

The error was:
ArgumentError: Before process_action callback :verify_authenticity_token has not been defined

I think the problem was that :verify_authenticity_token doesn't exist, so it's raising an exception when trying to skip it. My first thought was just to add raise: false like this:

skip_before_action :verify_authenticity_token, only: [:populate], raise: false

Which works, but seems really hacky and not ideal. Instead, I thought if I added the line:

protect_from_forgery with: :exception

It would add verify_authenticity_token to the before_filter list, which can then be skipped in other controllers without an error.

It's possible I'm misunderstanding, so please let me know what you think.

Thanks so much

[kennyadsl]

Does this error happen in the test suite? If yes can you please point a spec that is failing for reference?

[FrancescoAiello01]

This is the error output:

An error occurred while loading ./spec/controllers/spree/orders_controller_ability_spec.rb.
Failure/Error: skip_before_action :verify_authenticity_token, only: [:populate]

ArgumentError:
  Before process_action callback :verify_authenticity_token has not been defined
# ./app/controllers/spree/orders_controller.rb:14:in `<class:OrdersController>'
# ./app/controllers/spree/orders_controller.rb:4:in `<module:Spree>'
# ./app/controllers/spree/orders_controller.rb:3:in `<top (required)>'
# ./spec/controllers/spree/orders_controller_ability_spec.rb:6:in `<module:Spree>'
# ./spec/controllers/spree/orders_controller_ability_spec.rb:5:in `<top (required)>'


Finished in 0.00005 seconds (files took 4.88 seconds to load)
0 examples, 0 failures, 1 error occurred outside of examples

It looks like the error is happening when loading the orders controller before the tests.

[kennyadsl]

Oh ok, it makes sense. I think that we need that protect_from_forgery with: :exception then.
If we add , raise: false it will also change the behavior outside the test environment so I don't think it's a viable solution.

[FrancescoAiello01]

Yeah good point about keeping it in the test environment. Thanks for taking a look

[kennyadsl]

I feel like this is not necessary since we already set this value when defining the dummy app.

[FrancescoAiello01]

That's a good point. It is redundant, I just pushed a change removing the line. Thanks

[SamuelMartini]

Hey!
I have tried to run the solidus_starter_frontend from this branch and they are failing because of the forgery protection.

I think we need to disable the allow_forgery_protection configuration here as well.

The reason is that there are two dummy apps that can be generated.

1 - A Dummy::Application to test extensions that is created by running this task and use this DummyGenerator. This generator creates a full rails application (Dummy::Application) and overrides the rails environments/test.rb file with the above templates/rails/test.rb configuration. This is the reason allow_forgery_protection needs to be disabled in core/lib/generators/spree/dummy/templates/rails/test.rb .

2 - A DummyApp::Application to test the solidus framework and it is an application initialized in-memory here when the spec/rails_helper.rb file is loaded. This is the reason we need to disable in core/lib/spree/testing_support/dummy_app.rb

[kennyadsl]

You are right @SamuelMartini, we need to re-add it to make it work with Dummy apps generated in extensions.

@FrancescoAiello01 sorry for the wrong suggestion here! 😬

[FrancescoAiello01]

That makes sense, sorry for the confusion! Going to push a fix now.

[kennyadsl]

@FrancescoAiello01 please rebase against master to have the spec suite passing!

[kennyadsl]

Thanks!