Support rails 6.1

[NabeelAhsen]

What this PR does

Part 1 of 2: #1234

This PR introduces support for Rails 6.1. The key thing to support for Rails 6.1 is the following:

• Rails 6.1 configures action_dispatch.cookies_same_site_protection = :lax by default. This setting will not work with apps embedded in the Shopify Admin.

To fix this, embedded apps built with the next version of the Shopify App gem will have all their cookies set to SameSite=None using the SameSiteCookieMiddleware:

# same_site_cookie_middleware.rb
- cookie << '; SameSite=None' unless cookie =~ /;\s*samesite=/i
+ cookie << '; SameSite=None' if ShopifyApp.configuration.embedded_app?

For existing apps that wish to bump their Rails versions to 6.1, but do not want to upgrade their shopify_app gem version, this PR provides a troubleshooting guide to configure the following in their application.rb config:

+ action_dispatch.cookies_same_site_protection = :none

Reviewer's guide to testing

To test this, I used this branch as the source of the shopify_app gem path using a new app built with Rails v6.1.3:

1. rails _6.1.3_ new test-shopify-app
2. cd test-shopify-app
3. Add gem 'dotenv-rails' and gem 'shopify_app', path: 'local/path/to/shopify_app' to the app's Gemfile
4. Create a .env
5. Generate a new shopify_app: rails g shopify_app
6. Register and run the app

✅ Cookie behaviour using the current release of shopify_app `v8.3`

Rails 6.1 sets all cookies to LAX by default

✅ Cookie behaviour using this branch's `SameSiteCookieMiddleware`

Middleware sets all cookies to None by default if the app is embedded

✅ Cookie behaviour using Rails `application.rb` configurations

By adding the following:

# config/application.rb
require_relative "boot"
require "rails/all"

Bundler.require(*Rails.groups)

module TestShopifyApp
  class Application < Rails::Application
    config.load_defaults 6.1
+  config.action_dispatch.cookies_same_site_protection = :none
  end
end

Rails 6.1 sets all cookies to None by default

Things to focus on

1. I think this change is fair to ship as the next minor release (shopify_app v17.2). Is there any reason why this should be a major release?
2. Is there a way to mark future work for developers who are looking to support Rails 7+ to first handle changes made to the asset pipeline?
3. Should the release be part of a separate PR?

Checklist

Before submitting the PR, please consider if any of the following are needed:

• Update CHANGELOG.md if the changes would impact users
• Update README.md, if appropriate.
• Update any relevant pages in /docs, if necessary
• For security fixes, the Disclosure Policy must be followed.

[paulomarg]

If I remember correctly, there were also issues with our asset pipeline (specifically with webpack, if I'm not mistaken). Have those issues been solved?

[NabeelAhsen]

If I remember correctly, there were also issues with our asset pipeline (specifically with webpack, if I'm not mistaken). Have those issues been solved?

Is there an issue/discussion for more context around this? I can see if this is still outstanding

[andyw8]

#1163

[andyw8]

^ Ah, that issue is not so informative. I think there's more detail elsewhere...

[paulomarg]

I wonder if we can just change this requirement to ~> 6.1 now that we know it'll be safe until v7.

[rezaansyed]

Yeah same here, I think we can probably just keep the rails requirement to be: rails (> 5.2.1, < 7)

[paulomarg]

I tested running the code for this PR with Ruby v3 to see if that fixed the problems we had with returning JSON, and it seems to. We may want to tag that this closes #1178 as well!

[NabeelAhsen]

@andyw8 @paulomarg it looks like there are a few Shopify teams waiting for this change. I'm confident to merge it, but I'd like to get your final opinions on the gemspec constraint I've added.

Is it better to restrict support to Rails version <6.2 or <7?

[rezaansyed]

LGTM

[paulomarg]

I'm fine with this, it even fixed other problems 🙂

As for the version, I think either works for now and we can always tweak that later.

[spickermann]

The README.md still tells that Rails 6.1 or above is not yet supported.