diff --git a/app/assets/javascripts/shopify_app/itp_polyfill.js b/app/assets/javascripts/shopify_app/itp_polyfill.js new file mode 100644 index 000000000..400da2f88 --- /dev/null +++ b/app/assets/javascripts/shopify_app/itp_polyfill.js @@ -0,0 +1,30 @@ +(function() { + function setCookieAndRedirect() { + document.cookie = "shopify.cookies_persist=true"; + window.location.href = window.shopOrigin + "/admin/apps/" + window.apiKey; + } + + function shouldDisplayPrompt() { + if (navigator.userAgent.indexOf('com.jadedpixel.pos') !== -1) { + return false; + } + + if (navigator.userAgent.indexOf('Shopify Mobile/iOS') !== -1) { + return false; + } + + return Boolean(document.hasStorageAccess); + } + + document.addEventListener("DOMContentLoaded", function() { + if (shouldDisplayPrompt()) { + var itpContent = document.querySelector('#CookiePartitionPrompt'); + itpContent.style.display = 'block'; + + var button = document.querySelector('#AcceptCookies'); + button.addEventListener('click', setCookieAndRedirect); + } else { + setCookieAndRedirect(); + } + }); +})(); diff --git a/app/assets/javascripts/shopify_app/redirect.js b/app/assets/javascripts/shopify_app/redirect.js index aa7d62fdf..bc5c2b12b 100644 --- a/app/assets/javascripts/shopify_app/redirect.js +++ b/app/assets/javascripts/shopify_app/redirect.js @@ -1,19 +1,33 @@ -document.addEventListener("DOMContentLoaded", function() { - var redirectTargetElement = document.getElementById("redirection-target"); - var targetInfo = JSON.parse(redirectTargetElement.dataset.target) +(function() { + function redirect() { + var redirectTargetElement = document.getElementById("redirection-target"); - if (window.top == window.self) { - // If the current window is the 'parent', change the URL by setting location.href - window.top.location.href = targetInfo.url; - } else { - // If the current window is the 'child', change the parent's URL with postMessage - normalizedLink = document.createElement('a'); - normalizedLink.href = targetInfo.url; + if (!redirectTargetElement) { + return; + } - data = JSON.stringify({ - message: 'Shopify.API.remoteRedirect', - data: { location: normalizedLink.href } - }); - window.parent.postMessage(data, targetInfo.myshopifyUrl); + var targetInfo = JSON.parse(redirectTargetElement.dataset.target) + + if (window.top == window.self) { + // If the current window is the 'parent', change the URL by setting location.href + window.top.location.href = targetInfo.url; + } else { + // If the current window is the 'child', change the parent's URL with postMessage + normalizedLink = document.createElement('a'); + normalizedLink.href = targetInfo.url; + + data = JSON.stringify({ + message: 'Shopify.API.remoteRedirect', + data: {location: normalizedLink.href} + }); + window.parent.postMessage(data, targetInfo.myshopifyUrl); + } } -}); + + document.addEventListener("DOMContentLoaded", redirect); + + // In the turbolinks context, neither DOMContentLoaded nor turbolinks:load + // consistently fires. This ensures that we at least attempt to fire in the + // turbolinks situation as well. + redirect(); +})(); diff --git a/app/controllers/shopify_app/sessions_controller.rb b/app/controllers/shopify_app/sessions_controller.rb index 6ddf923f6..0fcfd7436 100644 --- a/app/controllers/shopify_app/sessions_controller.rb +++ b/app/controllers/shopify_app/sessions_controller.rb @@ -14,6 +14,11 @@ def create authenticate end + def enable_cookies + @shop = sanitized_shop_name + render_invalid_shop_error unless @shop + end + def callback if auth_hash login_shop @@ -37,15 +42,47 @@ def destroy private def authenticate - if sanitized_shop_name.present? - session['shopify.omniauth_params'] = { shop: sanitized_shop_name } - fullpage_redirect_to "#{main_app.root_path}auth/shopify" + return render_invalid_shop_error unless sanitized_shop_name.present? + session['shopify.omniauth_params'] = { shop: sanitized_shop_name } + + if redirect_for_cookie_access? + fullpage_redirect_to enable_cookies_path(shop: sanitized_shop_name) + elsif authenticate_in_context? + authenticate_in_context else - flash[:error] = I18n.t('invalid_shop_url') - redirect_to return_address + authenticate_at_top_level end end + def render_invalid_shop_error + flash[:error] = I18n.t('invalid_shop_url') + redirect_to return_address + end + + def authenticate_in_context + clear_top_level_oauth_cookie + redirect_to "#{main_app.root_path}auth/shopify" + end + + def authenticate_at_top_level + set_top_level_oauth_cookie + fullpage_redirect_to login_url(top_level: true) + end + + def authenticate_in_context? + return true unless ShopifyApp.configuration.embedded_app? + return true if params[:top_level] + session['shopify.top_level_oauth'] + end + + def redirect_for_cookie_access? + return false unless ShopifyApp.configuration.embedded_app? + return false if params[:top_level] + return false if session['shopify.cookies_persist'] + + true + end + def login_shop sess = ShopifyAPI::Session.new(shop_name, token) diff --git a/app/views/shopify_app/sessions/enable_cookies.html.erb b/app/views/shopify_app/sessions/enable_cookies.html.erb new file mode 100644 index 000000000..b95822f7e --- /dev/null +++ b/app/views/shopify_app/sessions/enable_cookies.html.erb @@ -0,0 +1,386 @@ + + + + + + + Redirecting… + + + + <%= javascript_include_tag('shopify_app/itp_polyfill', crossorigin: 'anonymous', integrity: true) %> + + +
+
+
+
+
+
+
+
+
+

<%= I18n.t('enable_cookies_heading', app: ShopifyApp.configuration.application_name) %>

+
+
+

<%= I18n.t('enable_cookies_body', app: ShopifyApp.configuration.application_name) %>

+
+
+

<%= I18n.t('enable_cookies_footer') %>

+
+
+
+
+
+
+ +
+
+
+
+
+
+
+
+
+ + diff --git a/config/locales/en.yml b/config/locales/en.yml index faca6a5b1..81a082fe9 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2,3 +2,7 @@ en: logged_out: 'Successfully logged out' could_not_log_in: 'Could not log in to Shopify store' invalid_shop_url: 'Invalid shop domain' + enable_cookies_heading: "Enable cookies from %{app}" + enable_cookies_body: "You must manually enable cookies in this browser in order to use %{app} within Shopify." + enable_cookies_footer: 'Cookies let the app authenticate you by temporarily storing your preferences and personal information. They expire after 30 days.' + enable_cookies_action: 'Enable cookies' diff --git a/config/routes.rb b/config/routes.rb index 02abbcaa5..eaa75eb0a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -2,6 +2,7 @@ controller :sessions do get 'login' => :new, :as => :login post 'login' => :create, :as => :authenticate + get 'enable_cookies' => :enable_cookies, :as => :enable_cookies get 'auth/shopify/callback' => :callback get 'logout' => :destroy, :as => :logout end diff --git a/lib/shopify_app/controller_concerns/login_protection.rb b/lib/shopify_app/controller_concerns/login_protection.rb index fcb7afec0..b5df7457e 100644 --- a/lib/shopify_app/controller_concerns/login_protection.rb +++ b/lib/shopify_app/controller_concerns/login_protection.rb @@ -5,19 +5,19 @@ module LoginProtection class ShopifyDomainNotFound < StandardError; end included do + after_action :set_test_cookie rescue_from ActiveResource::UnauthorizedAccess, :with => :close_session end def shopify_session - if shop_session - begin - ShopifyAPI::Base.activate_session(shop_session) - yield - ensure - ShopifyAPI::Base.clear_session - end - else - redirect_to_login + return redirect_to_login unless shop_session + clear_top_level_oauth_cookie + + begin + ShopifyAPI::Base.activate_session(shop_session) + yield + ensure + ShopifyAPI::Base.clear_session end end @@ -57,17 +57,29 @@ def clear_shop_session session[:shopify_user] = nil end - def login_url + def login_url(top_level: false) url = ShopifyApp.configuration.login_url - if params[:shop].present? - query = { shop: sanitized_params[:shop] }.to_query - url = "#{url}?#{query}" - end + query_params = login_url_params(top_level: top_level) + url = "#{url}?#{query_params.to_query}" if query_params.present? url end + def login_url_params(top_level:) + query_params = {} + query_params[:shop] = sanitized_params[:shop] if params[:shop].present? + + has_referer_shop_name = referer_sanitized_shop_name.present? + + if has_referer_shop_name + query_params[:shop] ||= referer_sanitized_shop_name + end + + query_params[:top_level] = true if top_level + query_params + end + def fullpage_redirect_to(url) if ShopifyApp.configuration.embedded_app? render 'shopify_app/shared/redirect', layout: false, locals: { url: url, current_shopify_domain: current_shopify_domain } @@ -87,6 +99,17 @@ def sanitized_shop_name @sanitized_shop_name ||= sanitize_shop_param(params) end + def referer_sanitized_shop_name + return unless request.referer.present? + + @referer_sanitized_shop_name ||= begin + referer_uri = URI(request.referer) + query_params = Rack::Utils.parse_query(referer_uri.query) + + sanitize_shop_param(query_params.with_indifferent_access) + end + end + def sanitize_shop_param(params) return unless params[:shop].present? ShopifyApp::Utils.sanitize_shop_domain(params[:shop]) @@ -99,5 +122,18 @@ def sanitized_params end end end + + def set_test_cookie + return unless ShopifyApp.configuration.embedded_app? + session['shopify.cookies_persist'] = true + end + + def clear_top_level_oauth_cookie + session.delete('shopify.top_level_oauth') + end + + def set_top_level_oauth_cookie + session['shopify.top_level_oauth'] = true + end end end diff --git a/lib/shopify_app/engine.rb b/lib/shopify_app/engine.rb index eb7f491ff..4dfcb09d3 100644 --- a/lib/shopify_app/engine.rb +++ b/lib/shopify_app/engine.rb @@ -4,7 +4,10 @@ class Engine < Rails::Engine isolate_namespace ShopifyApp initializer "shopify_app.assets.precompile" do |app| - app.config.assets.precompile += %w( shopify_app/redirect.js ) + app.config.assets.precompile += %w[ + shopify_app/redirect.js + shopify_app/itp_polyfill.js + ] end end end diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index 97c89d0e9..1b0b1a3e4 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -13,29 +13,65 @@ class SessionsControllerTest < ActionController::TestCase @routes = ShopifyApp::Engine.routes ShopifyApp::SessionRepository.storage = ShopifyApp::InMemorySessionStore ShopifyApp.configuration = nil + ShopifyApp.configuration.embedded_app = true I18n.locale = :en + + session['shopify.cookies_persist'] = true + end + + test '#new redirects to the enable_cookies page if we can\'t set cookies' do + session.delete('shopify.cookies_persist') + shopify_domain = 'my-shop.myshopify.com' + get :new, params: { shop: 'my-shop' } + assert_redirected_to_top_level(shopify_domain, '/enable_cookies?shop=my-shop.myshopify.com') end - test "#new should authenticate the shop if a valid shop param exists" do + test '#new redirects to the top-level login if a valid shop param exists' do shopify_domain = 'my-shop.myshopify.com' get :new, params: { shop: 'my-shop' } - assert_redirected_to_authentication(shopify_domain, response) + assert_redirected_to_top_level(shopify_domain) + assert_equal true, session['shopify.top_level_oauth'] + end + + test '#new sets the top_level_oauth cookie if a valid shop param exists' do + get :new, params: { shop: 'my-shop' } + assert_equal true, session['shopify.top_level_oauth'] + end + + test '#new redirects to the auth page if top_level param' do + session.delete('shopify.cookies_persist') + get :new, params: { shop: 'my-shop', top_level: true } + assert_redirected_to '/auth/shopify' end test "#new should authenticate the shop if a valid shop param exists non embedded" do + session.delete('shopify.cookies_persist') ShopifyApp.configuration.embedded_app = false get :new, params: { shop: 'my-shop' } assert_redirected_to '/auth/shopify' assert_equal session['shopify.omniauth_params'][:shop], 'my-shop.myshopify.com' end + test '#new authenticates the shop if we\'ve just returned from top-level login flow' do + session['shopify.top_level_oauth'] = true + get :new, params: { shop: 'my-shop' } + assert_redirected_to '/auth/shopify' + assert_equal session['shopify.omniauth_params'][:shop], 'my-shop.myshopify.com' + end + + test '#new removes the top_level_oauth cookie if we\'ve just returned from top-level login flow' do + session['shopify.top_level_oauth'] = true + get :new, params: { shop: 'my-shop' } + assert_nil session['shopify.top_level_oauth'] + end + test "#new should trust the shop param over the current session" do previously_logged_in_shop_id = 1 session[:shopify] = previously_logged_in_shop_id new_shop_domain = "new-shop.myshopify.com" get :new, params: { shop: new_shop_domain } - assert_redirected_to_authentication(new_shop_domain, response) + assert_redirected_to_top_level(new_shop_domain) end test "#new should render a full-page if the shop param doesn't exist" do @@ -55,7 +91,7 @@ class SessionsControllerTest < ActionController::TestCase test "#create should authenticate the shop for the URL (#{good_url})" do shopify_domain = 'my-shop.myshopify.com' post :create, params: { shop: good_url } - assert_redirected_to_authentication(shopify_domain, response) + assert_redirected_to_top_level(shopify_domain) end end @@ -64,7 +100,7 @@ class SessionsControllerTest < ActionController::TestCase ShopifyApp.configuration.myshopify_domain = 'myshopify.io' shopify_domain = 'my-shop.myshopify.io' post :create, params: { shop: good_url } - assert_redirected_to_authentication(shopify_domain, response) + assert_redirected_to_top_level(shopify_domain) end end @@ -82,6 +118,17 @@ class SessionsControllerTest < ActionController::TestCase assert_redirected_to '/' end + test '#enable_cookies renders the correct template' do + get :enable_cookies, params: { shop: 'shop' } + assert_template 'sessions/enable_cookies' + end + + test '#enable_cookies displays an error if no shop is provided' do + get :enable_cookies + assert_redirected_to ShopifyApp.configuration.root_url + assert_equal I18n.t('invalid_shop_url'), flash[:error] + end + test '#callback should flash error when omniauth is not present' do get :callback, params: { shop: 'shop' } assert_equal flash[:error], 'Could not log in to Shopify store' @@ -221,12 +268,12 @@ def mock_shopify_user_omniauth request.env['omniauth.params'] = { shop: 'shop.myshopify.com' } if request end - def assert_redirected_to_authentication(shop_domain, response) - auth_url = "/auth/shopify" + def assert_redirected_to_top_level(shop_domain, expected_url = nil) + expected_url ||= "/login?shop=#{shop_domain}\\u0026top_level=true" assert_template 'shared/redirect' assert_select '[id=redirection-target]', 1 do |elements| - assert_equal "{\"myshopifyUrl\":\"https://#{shop_domain}\",\"url\":\"#{auth_url}\"}", + assert_equal "{\"myshopifyUrl\":\"https://#{shop_domain}\",\"url\":\"#{expected_url}\"}", elements.first['data-target'] end end diff --git a/test/integration/webhooks_controller_test.rb b/test/integration/webhooks_controller_test.rb index 44169d1f7..e2a390101 100644 --- a/test/integration/webhooks_controller_test.rb +++ b/test/integration/webhooks_controller_test.rb @@ -10,6 +10,7 @@ class WebhooksControllerTest < ActionDispatch::IntegrationTest setup do WebhooksController.any_instance.stubs(:verify_request).returns(true) + WebhooksController.any_instance.stubs(:webhook_namespace).returns(nil) end test "receives webhook and performs job" do diff --git a/test/shopify_app/controller_concerns/login_protection_test.rb b/test/shopify_app/controller_concerns/login_protection_test.rb index d9de5e0b2..388bc1ad1 100644 --- a/test/shopify_app/controller_concerns/login_protection_test.rb +++ b/test/shopify_app/controller_concerns/login_protection_test.rb @@ -35,6 +35,22 @@ class LoginProtectionTest < ActionController::TestCase ShopifyApp::SessionRepository.storage = ShopifyApp::InMemorySessionStore end + test '#index sets test cookie if embedded app' do + with_application_test_routes do + get :index + assert_equal true, session['shopify.cookies_persist'] + end + end + + test '#index doesn\'t set test cookie if non embedded app' do + with_application_test_routes do + ShopifyApp.configuration.embedded_app = false + + get :index + assert_nil session['shopify.cookies_persist'] + end + end + test "#shop_session returns nil when session is nil" do with_application_test_routes do session[:shopify] = nil @@ -89,6 +105,18 @@ class LoginProtectionTest < ActionController::TestCase end end + test '#shopify_session with Shopify session, clears top-level auth cookie' do + with_application_test_routes do + session['shopify.top_level_oauth'] = true + sess = stub(url: 'https://foobar.myshopify.com') + @controller.expects(:shop_session).returns(sess).at_least_once + ShopifyAPI::Base.expects(:activate_session).with(sess) + + get :index, params: { shop: 'foobar' } + assert_nil session['shopify.top_level_oauth'] + end + end + test '#shopify_session with no Shopify session, redirects to the login url' do with_application_test_routes do get :index, params: { shop: 'foobar' } @@ -96,6 +124,17 @@ class LoginProtectionTest < ActionController::TestCase end end + test "#shopify_session with no Shopify session, redirects to login_url with \ + shop param of referer" do + with_application_test_routes do + @controller.expects(:shop_session).returns(nil) + request.headers['Referer'] = 'https://example.com/?shop=my-shop.myshopify.com' + + get :index + assert_redirected_to '/login?shop=my-shop.myshopify.com' + end + end + test '#shopify_session with no Shopify session, redirects to the login url \ with non-String shop param' do with_application_test_routes do