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

capybara tests failing because of secure attribute on shopify session cookie #874

Closed
shravanjoopally opened this issue Jan 25, 2020 · 10 comments · Fixed by #876
Closed

capybara tests failing because of secure attribute on shopify session cookie #874

shravanjoopally opened this issue Jan 25, 2020 · 10 comments · Fixed by #876

Comments

@shravanjoopally
Copy link

Unable to run rspecs with js: true. Because while setting the shopify session cookie secure attribute is set by default.
As Rspecs are run mostly on http not on https protocol authentication is failing, becuase cookie is not set correctly on browser and its not sent back to server on following requests.

work around can be disabling the same site option in test mode.
ShopifyApp.configuration.enable_same_site_none = false

@shravanjoopally shravanjoopally changed the title capybara tests failing because of secure attribute on cookies capybara tests failing because of secure attribute on shopify session cookie Jan 25, 2020
@shravanjoopally
Copy link
Author

shravanjoopally commented Jan 25, 2020

issue introduced by #851

@tylerball
Copy link
Contributor

Thanks for the report, do you have some instructions for how we can replicate this? We don't run an capybara tests currently.

@vfonic
Copy link
Contributor

vfonic commented Jan 27, 2020

I'm seeing similar issue where I'm setting an app-related session key that doesn't get passed through. My app is working find in dev and production, but the tests are failing.

I'm trying to make a workaround now. I'll update here if there's any progress.

@vfonic
Copy link
Contributor

vfonic commented Jan 27, 2020

This is the gist of my test:

    describe 'Installing the app in a new shop', type: :system, js: true do
      it 'is able to install app from login homepage' do
        visit root_path # redirects to '/login'

        fill_in 'shop', with: 'my-test-shop.myshopify.com'

        click_on 'Install'

        expect(page).to have_selector('a[href="/"]', text: app_name)
      end
    end

@vfonic
Copy link
Contributor

vfonic commented Jan 27, 2020

I realized that if I disable somehow the SameSite cookie modification, all of my tests pass. So I simply added this to the bottom of my spec/rails_helper.rb:

module ShopifyApp
  class SameSiteCookieMiddleware
    def call(env)
      @app.call(env)
    end
  end
end

There are probably better ways though...

@edavis10
Copy link

Same issue here but I just disabled it in the shopify_app configuration for the test environment. e.g.

ShopifyApp.configure do |config|
  # ...
  config.embedded_app = true
  # Defaults to on but breaks all cookies in the test environment with capybara-webkit
  config.enable_same_site_none = if Rails.env.test?
                                   false
                                 else
                                   true
                                 end
end

While running the test if this is enabled (default) there is no Set-Cookie header getting set. When it is disabled using this code, Set-Cookie is getting set with the correct session values and path=/; HttpOnly and all the tests pass.

My guess is that the user agent sniffer is part of the problem but I'm not digging into it. Here's what capybara is setting on a Linux host:

"HTTP_USER_AGENT"=>"Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.21 (KHTML, like Gecko) capybara-webkit Safari/537.21"

@vfonic
Copy link
Contributor

vfonic commented Jan 28, 2020

Ah I didn't know there's a config option for this. That looks like much cleaner solution.

You could simplify it further like this:

ShopifyApp.configure do |config|
  # ...
  config.embedded_app = true
  # Defaults to on but breaks all cookies in the test environment with capybara-webkit
  config.enable_same_site_none = !Rails.env.test?
end

@tanema
Copy link
Contributor

tanema commented Jan 28, 2020

Okay I will look into adding this configuration into our gem so that you do not need to worry about this in the future. Thanks for the deep dive into the problem.

@vfonic
Copy link
Contributor

vfonic commented Jan 28, 2020

Thanks for taking care of this @tanema! 👍

Looking at this again, maybe just a mention in the README would be enough. 🤔

@tanema
Copy link
Contributor

tanema commented Jan 28, 2020

I think if we can just make the gem do this automatically so that the developer does not have to come back to the repo/README to figure out how to make their tests work then it should be done that way.

People already have enough frustrations right now 😓

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

Successfully merging a pull request may close this issue.

5 participants