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

Support rails 6.1 #1221

Merged
merged 7 commits into from
Apr 1, 2021
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
139 changes: 70 additions & 69 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5,76 +5,80 @@ PATH
browser_sniffer (~> 1.2.2)
jwt (~> 2.2.1)
omniauth-shopify-oauth2 (~> 2.2.2)
rails (> 5.2.1, < 6.1)
rails (> 5.2.1, < 6.2)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

redirect_safely (~> 1.0)
shopify_api (~> 9.4)

GEM
remote: https://rubygems.org/
specs:
actioncable (6.0.3.5)
actionpack (= 6.0.3.5)
actioncable (6.1.3.1)
actionpack (= 6.1.3.1)
activesupport (= 6.1.3.1)
nio4r (~> 2.0)
websocket-driver (>= 0.6.1)
actionmailbox (6.0.3.5)
actionpack (= 6.0.3.5)
activejob (= 6.0.3.5)
activerecord (= 6.0.3.5)
activestorage (= 6.0.3.5)
activesupport (= 6.0.3.5)
actionmailbox (6.1.3.1)
actionpack (= 6.1.3.1)
activejob (= 6.1.3.1)
activerecord (= 6.1.3.1)
activestorage (= 6.1.3.1)
activesupport (= 6.1.3.1)
mail (>= 2.7.1)
actionmailer (6.0.3.5)
actionpack (= 6.0.3.5)
actionview (= 6.0.3.5)
activejob (= 6.0.3.5)
actionmailer (6.1.3.1)
actionpack (= 6.1.3.1)
actionview (= 6.1.3.1)
activejob (= 6.1.3.1)
activesupport (= 6.1.3.1)
mail (~> 2.5, >= 2.5.4)
rails-dom-testing (~> 2.0)
actionpack (6.0.3.5)
actionview (= 6.0.3.5)
activesupport (= 6.0.3.5)
rack (~> 2.0, >= 2.0.8)
actionpack (6.1.3.1)
actionview (= 6.1.3.1)
activesupport (= 6.1.3.1)
rack (~> 2.0, >= 2.0.9)
rack-test (>= 0.6.3)
rails-dom-testing (~> 2.0)
rails-html-sanitizer (~> 1.0, >= 1.2.0)
actiontext (6.0.3.5)
actionpack (= 6.0.3.5)
activerecord (= 6.0.3.5)
activestorage (= 6.0.3.5)
activesupport (= 6.0.3.5)
actiontext (6.1.3.1)
actionpack (= 6.1.3.1)
activerecord (= 6.1.3.1)
activestorage (= 6.1.3.1)
activesupport (= 6.1.3.1)
nokogiri (>= 1.8.5)
actionview (6.0.3.5)
activesupport (= 6.0.3.5)
actionview (6.1.3.1)
activesupport (= 6.1.3.1)
builder (~> 3.1)
erubi (~> 1.4)
rails-dom-testing (~> 2.0)
rails-html-sanitizer (~> 1.1, >= 1.2.0)
activejob (6.0.3.5)
activesupport (= 6.0.3.5)
activejob (6.1.3.1)
activesupport (= 6.1.3.1)
globalid (>= 0.3.6)
activemodel (6.0.3.5)
activesupport (= 6.0.3.5)
activemodel (6.1.3.1)
activesupport (= 6.1.3.1)
activemodel-serializers-xml (1.0.2)
activemodel (> 5.x)
activesupport (> 5.x)
builder (~> 3.1)
activerecord (6.0.3.5)
activemodel (= 6.0.3.5)
activesupport (= 6.0.3.5)
activerecord (6.1.3.1)
activemodel (= 6.1.3.1)
activesupport (= 6.1.3.1)
activeresource (5.1.1)
activemodel (>= 5.0, < 7)
activemodel-serializers-xml (~> 1.0)
activesupport (>= 5.0, < 7)
activestorage (6.0.3.5)
actionpack (= 6.0.3.5)
activejob (= 6.0.3.5)
activerecord (= 6.0.3.5)
marcel (~> 0.3.1)
activesupport (6.0.3.5)
activestorage (6.1.3.1)
actionpack (= 6.1.3.1)
activejob (= 6.1.3.1)
activerecord (= 6.1.3.1)
activesupport (= 6.1.3.1)
marcel (~> 1.0.0)
mini_mime (~> 1.0.2)
activesupport (6.1.3.1)
concurrent-ruby (~> 1.0, >= 1.0.2)
i18n (>= 0.7, < 2)
minitest (~> 5.1)
tzinfo (~> 1.1)
zeitwerk (~> 2.2, >= 2.2.2)
i18n (>= 1.6, < 2)
minitest (>= 5.1)
tzinfo (~> 2.0)
zeitwerk (~> 2.3)
addressable (2.7.0)
public_suffix (>= 2.0.2, < 5.0)
ast (2.4.1)
Expand All @@ -88,7 +92,7 @@ GEM
crack (0.4.4)
crass (1.0.6)
debug_inspector (0.0.3)
erubi (1.9.0)
erubi (1.10.0)
faraday (1.3.0)
faraday-net_http (~> 1.0)
multipart-post (>= 1.2, < 3)
Expand All @@ -105,24 +109,22 @@ GEM
i18n (1.8.9)
concurrent-ruby (~> 1.0)
jwt (2.2.2)
loofah (2.7.0)
loofah (2.9.0)
crass (~> 1.0.2)
nokogiri (>= 1.5.9)
mail (2.7.1)
mini_mime (>= 0.1.1)
marcel (0.3.3)
mimemagic (~> 0.3.2)
marcel (1.0.0)
method_source (0.9.2)
mimemagic (0.3.5)
mini_mime (1.0.2)
mini_mime (1.0.3)
mini_portile2 (2.5.0)
minitest (5.14.4)
mocha (1.11.2)
multi_json (1.15.0)
multi_xml (0.6.0)
multipart-post (2.1.1)
nio4r (2.5.7)
nokogiri (1.11.1)
nokogiri (1.11.2)
mini_portile2 (~> 2.5.0)
racc (~> 1.4)
oauth2 (1.4.4)
Expand Down Expand Up @@ -156,20 +158,20 @@ GEM
rack (2.2.3)
rack-test (1.1.0)
rack (>= 1.0, < 3)
rails (6.0.3.5)
actioncable (= 6.0.3.5)
actionmailbox (= 6.0.3.5)
actionmailer (= 6.0.3.5)
actionpack (= 6.0.3.5)
actiontext (= 6.0.3.5)
actionview (= 6.0.3.5)
activejob (= 6.0.3.5)
activemodel (= 6.0.3.5)
activerecord (= 6.0.3.5)
activestorage (= 6.0.3.5)
activesupport (= 6.0.3.5)
bundler (>= 1.3.0)
railties (= 6.0.3.5)
rails (6.1.3.1)
actioncable (= 6.1.3.1)
actionmailbox (= 6.1.3.1)
actionmailer (= 6.1.3.1)
actionpack (= 6.1.3.1)
actiontext (= 6.1.3.1)
actionview (= 6.1.3.1)
activejob (= 6.1.3.1)
activemodel (= 6.1.3.1)
activerecord (= 6.1.3.1)
activestorage (= 6.1.3.1)
activesupport (= 6.1.3.1)
bundler (>= 1.15.0)
railties (= 6.1.3.1)
sprockets-rails (>= 2.0.0)
rails-controller-testing (1.0.5)
actionpack (>= 5.0.1.rc1)
Expand All @@ -180,12 +182,12 @@ GEM
nokogiri (>= 1.6)
rails-html-sanitizer (1.3.0)
loofah (~> 2.3)
railties (6.0.3.5)
actionpack (= 6.0.3.5)
activesupport (= 6.0.3.5)
railties (6.1.3.1)
actionpack (= 6.1.3.1)
activesupport (= 6.1.3.1)
method_source
rake (>= 0.8.7)
thor (>= 0.20.3, < 2.0)
thor (~> 1.0)
rainbow (3.0.0)
rake (13.0.3)
rb-readline (0.5.5)
Expand Down Expand Up @@ -221,9 +223,8 @@ GEM
sprockets (>= 3.0.0)
sqlite3 (1.4.2)
thor (1.1.0)
thread_safe (0.3.6)
tzinfo (1.2.9)
thread_safe (~> 0.1)
tzinfo (2.0.4)
concurrent-ruby (~> 1.0)
unicode-display_width (1.7.0)
webmock (3.9.1)
addressable (>= 2.3.6)
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

[gem]: https://img.shields.io/gem/v/shopify_app.svg
[gem_url]: https://rubygems.org/gems/shopify_app
[supported_rails_version]: https://img.shields.io/badge/rails-%3C6.1.0-orange
[supported_rails_version]: https://img.shields.io/badge/rails-%3C6.2.0-orange

This gem builds Rails applications that can be embedded in the Shopify Admin.

Expand Down
33 changes: 33 additions & 0 deletions docs/Troubleshooting.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,12 @@
[Generators](#generators)
* [The `shopify_app:install` generator hangs](#the-shopifyappinstall-generator-hangs)

[Rails](#rails)
* [Known issues with Rails `v6.1`](#known-issues-with-rails-v61)

[App installation](#app-installation)
* [My app won't install](#my-app-wont-install)
* [My app keeps redirecting to login](#my-app-keeps-redirecting-to-login)

[JWT session tokens](#jwt-session-tokens)
* [My app is still using cookies to authenticate](#my-app-is-still-using-cookies-to-authenticate)
Expand All @@ -24,6 +28,35 @@ $ bundle exec spring stop

Run shopify_app generator again.

## Rails

### Known issues with Rails `v6.1`

If you recently upgraded your application's `Rails::Application` configuration to load the default configuration for Rails `v6.1`, then you will need to update the following `cookies_same_site_protection` ActionDispatch configuration.

```diff
# config/application.rb

require_relative 'boot'

require 'rails/all'

Bundler.require(*Rails.groups)

module AppName
class Application < Rails::Application
+ config.load_defaults 6.1

+ config.action_dispatch.cookies_same_site_protection = :none
...
end
end
```

As of Rails `v6.1`, the same-site cookie protection setting defaults to `Lax`. This does not allow an embedded app to make cross-domain requests in the Shopify Admin.
NabeelAhsen marked this conversation as resolved.
Show resolved Hide resolved

Alternatively, you can upgrade to [`v17.2.0` of the shopify_app gem](/docs/Upgrading.md#upgrading-to-v1720).

## App installation

### My app won't install
Expand Down
16 changes: 16 additions & 0 deletions docs/Upgrading.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,28 @@ This file documents important changes needed to upgrade your app's Shopify App v

#### Table of contents

[Upgrading to `v17.2.0`](#upgrading-to-v1720)

[Upgrading to `v13.0.0`](#upgrading-to-v1300)

[Upgrading to `v11.7.0`](#upgrading-to-v1170)

[Upgrading from `v8.6` to `v9.0.0`](#upgrading-from-v86-to-v900)

## Upgrading to `v17.2.0`

### Different SameSite cookie attribute behaviour

To support Rails `v6.1`, the [`SameSiteCookieMiddleware`](/lib/shopify_app/middleware/same_site_cookie_middleware.rb) was updated to configure cookies to `SameSite=None` if the app is embedded. Before this release, cookies were configured to `SameSite=None` only if this attribute had not previously been set before.

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

By default, Rails `v6.1` configures `SameSite=Lax` on all cookies that don't specify this attribute.

## Upgrading to `v13.0.0`

Version 13.0.0 adds the ability to use both user and shop sessions, concurrently. This however involved a large
Expand Down
2 changes: 1 addition & 1 deletion lib/shopify_app/middleware/same_site_cookie_middleware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def call(env)
.compact
.map do |cookie|
cookie << '; Secure' unless cookie =~ /;\s*secure/i
cookie << '; SameSite=None' unless cookie =~ /;\s*samesite=/i
cookie << '; SameSite=None' if ShopifyApp.configuration.embedded_app?
cookie
end

Expand Down
2 changes: 1 addition & 1 deletion shopify_app.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ Gem::Specification.new do |s|
s.metadata['allowed_push_host'] = 'https://rubygems.org'

s.add_runtime_dependency('browser_sniffer', '~> 1.2.2')
s.add_runtime_dependency('rails', '> 5.2.1', '< 6.1')
s.add_runtime_dependency('rails', '> 5.2.1', '< 6.2')
s.add_runtime_dependency('shopify_api', '~> 9.4')
s.add_runtime_dependency('omniauth-shopify-oauth2', '~> 2.2.2')
s.add_runtime_dependency('jwt', '~> 2.2.1')
Expand Down
25 changes: 18 additions & 7 deletions test/shopify_app/middleware/same_site_cookie_middleware_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@
require 'test_helper'

class ShopifyApp::SameSiteCookieMiddlewareTest < ActiveSupport::TestCase
attr_reader :non_ssl_env, :ssl_env

def setup
@non_ssl_env = env_for_url("http://test.com/")
@ssl_env = env_for_url("https://test.com/")
end

def app
Rack::Lint.new(lambda { |env|
Rack::Request.new(env)
Expand All @@ -26,18 +33,22 @@ def middleware
ShopifyApp::SameSiteCookieMiddleware.new(app)
end

test 'SameSite cookie attributes should be added on SSL' do
env = env_for_url("https://test.com/")
test 'SameSite cookie attributes should be added on SSL requests if app is embedded' do
_status, headers, _body = middleware.call(ssl_env)

assert_includes headers['Set-Cookie'], 'SameSite=None'
end

test 'SameSite cookie attributes should not be added on SSL requests if app is not embedded' do
ShopifyApp.configuration.stubs(:embedded_app?).returns(false)

_status, headers, _body = middleware.call(env)
_status, headers, _body = middleware.call(ssl_env)

assert_includes headers['Set-Cookie'], 'SameSite'
assert_not_includes headers['Set-Cookie'], 'SameSite'
end

test 'SameSite cookie attributes should not be added on non SSL requests' do
env = env_for_url("http://test.com/")

_status, headers, _body = middleware.call(env)
_status, headers, _body = middleware.call(non_ssl_env)

assert_not_includes headers['Set-Cookie'], 'SameSite'
end
Expand Down