diff --git a/app/controllers/concerns/shopify_app/ensure_installed.rb b/app/controllers/concerns/shopify_app/ensure_installed.rb
index b23ec502f..ba06427bf 100644
--- a/app/controllers/concerns/shopify_app/ensure_installed.rb
+++ b/app/controllers/concerns/shopify_app/ensure_installed.rb
@@ -17,9 +17,11 @@ module EnsureInstalled
before_action :check_shop_domain
- unless ShopifyApp.configuration.use_new_embedded_auth_strategy?
- # TODO: Add support to use new embedded auth strategy here when invalid
- # session token can be handled by AppBridge app reload
+ if ShopifyApp.configuration.use_new_embedded_auth_strategy?
+ include ShopifyApp::TokenExchange
+ include ShopifyApp::EmbeddedApp
+ around_action :activate_shopify_session
+ else
before_action :check_shop_known
before_action :validate_non_embedded_session
end
diff --git a/app/controllers/shopify_app/sessions_controller.rb b/app/controllers/shopify_app/sessions_controller.rb
index 080abdb9a..2e5c977f6 100644
--- a/app/controllers/shopify_app/sessions_controller.rb
+++ b/app/controllers/shopify_app/sessions_controller.rb
@@ -7,7 +7,7 @@ class SessionsController < ActionController::Base
layout false, only: :new
- after_action only: [:new, :create] do |controller|
+ after_action only: [:new, :create, :patch_shopify_id_token] do |controller|
controller.response.headers.except!("X-Frame-Options")
end
@@ -19,6 +19,10 @@ def create
authenticate
end
+ def patch_shopify_id_token
+ render(layout: "shopify_app/layouts/app_bridge")
+ end
+
def top_level_interaction
@url = login_url_with_optional_shop(top_level: true)
validate_shop_presence
diff --git a/app/views/shopify_app/layouts/app_bridge.html.erb b/app/views/shopify_app/layouts/app_bridge.html.erb
new file mode 100644
index 000000000..d9c681735
--- /dev/null
+++ b/app/views/shopify_app/layouts/app_bridge.html.erb
@@ -0,0 +1,17 @@
+
+
+
+
+ <%= ShopifyApp.configuration.application_name %>
+ <%= yield :head %>
+
+ <%= csrf_meta_tags %>
+
+
+
+ <%= yield %>
+
+
diff --git a/app/views/shopify_app/sessions/patch_shopify_id_token.html.erb b/app/views/shopify_app/sessions/patch_shopify_id_token.html.erb
new file mode 100644
index 000000000..e69de29bb
diff --git a/config/routes.rb b/config/routes.rb
index 2f22c152d..d3e755b36 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -8,6 +8,7 @@
get login_url => :new, :as => :login
post login_url => :create, :as => :authenticate
get "logout" => :destroy, :as => :logout
+ get "patch_shopify_id_token" => :patch_shopify_id_token
# Kept to prevent apps relying on these routes from breaking
if login_url.gsub(%r{^/}, "") != "login"
diff --git a/lib/shopify_app/controller_concerns/token_exchange.rb b/lib/shopify_app/controller_concerns/token_exchange.rb
index a3f45938b..cf6b28fb3 100644
--- a/lib/shopify_app/controller_concerns/token_exchange.rb
+++ b/lib/shopify_app/controller_concerns/token_exchange.rb
@@ -5,17 +5,23 @@ module TokenExchange
extend ActiveSupport::Concern
include ShopifyApp::AdminAPI::WithTokenRefetch
+ INVALID_SHOPIFY_ID_TOKEN_ERRORS = [
+ ShopifyAPI::Errors::CookieNotFoundError,
+ ShopifyAPI::Errors::InvalidJwtTokenError,
+ ].freeze
+
def activate_shopify_session(&block)
retrieve_session_from_token_exchange if current_shopify_session.blank? || should_exchange_expired_token?
- begin
- ShopifyApp::Logger.debug("Activating Shopify session")
- ShopifyAPI::Context.activate_session(current_shopify_session)
- with_token_refetch(current_shopify_session, shopify_id_token, &block)
- ensure
- ShopifyApp::Logger.debug("Deactivating session")
- ShopifyAPI::Context.deactivate_session
- end
+ ShopifyApp::Logger.debug("Activating Shopify session")
+ ShopifyAPI::Context.activate_session(current_shopify_session)
+ with_token_refetch(current_shopify_session, shopify_id_token, &block)
+ rescue *INVALID_SHOPIFY_ID_TOKEN_ERRORS => e
+ ShopifyApp::Logger.debug("Responding to invalid Shopify ID token: #{e.message}")
+ respond_to_invalid_shopify_id_token unless performed?
+ ensure
+ ShopifyApp::Logger.debug("Deactivating session")
+ ShopifyAPI::Context.deactivate_session
end
def should_exchange_expired_token?
@@ -47,9 +53,6 @@ def current_shopify_domain
def retrieve_session_from_token_exchange
@current_shopify_session = nil
ShopifyApp::Auth::TokenExchange.perform(shopify_id_token)
- # TODO: Rescue JWT validation errors when bounce page is ready
- # rescue ShopifyAPI::Errors::InvalidJwtTokenError
- # respond_to_invalid_shopify_id_token
end
def shopify_id_token
@@ -61,23 +64,28 @@ def id_token_header
end
def respond_to_invalid_shopify_id_token
- # TODO: Implement this method to handle invalid session tokens
+ return redirect_to_bounce_page if request.headers["HTTP_AUTHORIZATION"].blank?
+
+ ShopifyApp::Logger.debug("Responding to invalid Shopify ID token with unauthorized response")
+ response.set_header("X-Shopify-Retry-Invalid-Session-Request", 1)
+ unauthorized_response = { message: :unauthorized }
+ render(json: { errors: [unauthorized_response] }, status: :unauthorized)
+ end
- # if request.xhr?
- # response.set_header("X-Shopify-Retry-Invalid-Session-Request", 1)
- # unauthorized_response = { message: :unauthorized }
- # render(json: { errors: [unauthorized_response] }, status: :unauthorized)
- # else
- # patch_session_token_url = "#{ShopifyAPI::Context.host}/patch_session_token"
- # patch_session_token_params = request.query_parameters.except(:id_token)
+ def redirect_to_bounce_page
+ ShopifyApp::Logger.debug("Redirecting to bounce page for patching Shopify ID token")
+ patch_shopify_id_token_url = "#{ShopifyApp.configuration.root_url}/patch_shopify_id_token"
+ patch_shopify_id_token_params = request.query_parameters.except(:id_token)
- # bounce_url = "#{ShopifyAPI::Context.host}#{request.path}?#{patch_session_token_params.to_query}"
+ bounce_url = "#{request.path}?#{patch_shopify_id_token_params.to_query}"
- # # App Bridge will trigger a fetch to the URL in shopify-reload, with a new session token in headers
- # patch_session_token_params["shopify-reload"] = bounce_url
+ # App Bridge will trigger a fetch to the URL in shopify-reload, with a new session token in headers
+ patch_shopify_id_token_params["shopify-reload"] = bounce_url
- # redirect_to("#{patch_session_token_url}?#{patch_session_token_params.to_query}", allow_other_host: true)
- # end
+ redirect_to(
+ "#{patch_shopify_id_token_url}?#{patch_shopify_id_token_params.to_query}",
+ allow_other_host: true,
+ )
end
def online_token_configured?
diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb
index 84dc518ff..649a15885 100644
--- a/test/controllers/sessions_controller_test.rb
+++ b/test/controllers/sessions_controller_test.rb
@@ -402,6 +402,11 @@ class SessionsControllerTest < ActionController::TestCase
end
end
+ test "#patch_shopify_id_token renders the app bridge layout" do
+ get :patch_shopify_id_token, params: { shop: "my-shop" }
+ assert_template "shopify_app/layouts/app_bridge"
+ end
+
private
def assert_redirected_to_top_level(shop_domain, expected_url = nil)
diff --git a/test/dummy/config/initializers/shopify_app.rb b/test/dummy/config/initializers/shopify_app.rb
index e78ca8437..e935e2568 100644
--- a/test/dummy/config/initializers/shopify_app.rb
+++ b/test/dummy/config/initializers/shopify_app.rb
@@ -17,8 +17,13 @@ def self.call
config.embedded_redirect_url = nil
config.shop_session_repository = ShopifyApp::InMemorySessionStore
+ config.user_session_repository = nil
config.after_authenticate_job = false
config.reauth_on_access_scope_changes = true
+ config.root_url = "/"
+ config.wip_new_embedded_auth_strategy = false
+ config.check_session_expiry_date = false
+ config.custom_post_authenticate_tasks = nil
end
setup_context
diff --git a/test/routes/sessions_routes_test.rb b/test/routes/sessions_routes_test.rb
index 414535b67..f2db9093b 100644
--- a/test/routes/sessions_routes_test.rb
+++ b/test/routes/sessions_routes_test.rb
@@ -25,6 +25,10 @@ class SessionsRoutesTest < ActionController::TestCase
assert_routing "/logout", { controller: "shopify_app/sessions", action: "destroy" }
end
+ test "patch_shopify_id_token routes to sessions#patch_shopify_id_token" do
+ assert_routing "/patch_shopify_id_token", { controller: "shopify_app/sessions", action: "patch_shopify_id_token" }
+ end
+
test "login route doesn't change with custom root URL because it is in an engine" do
ShopifyApp.configuration.root_url = "/new-root"
Rails.application.reload_routes!
diff --git a/test/shopify_app/controller_concerns/token_exchange_test.rb b/test/shopify_app/controller_concerns/token_exchange_test.rb
index 77510d000..a603bd7f5 100644
--- a/test/shopify_app/controller_concerns/token_exchange_test.rb
+++ b/test/shopify_app/controller_concerns/token_exchange_test.rb
@@ -3,6 +3,7 @@
require "test_helper"
require "action_controller"
require "action_controller/base"
+require "json"
class ApiClass
def self.perform; end
@@ -17,10 +18,19 @@ def index
render(plain: "OK")
end
+ def reloaded_path
+ render(plain: "OK")
+ end
+
def make_api_call
ApiClass.perform
render(plain: "OK")
end
+
+ def ensure_render
+ ensure
+ render(plain: "OK")
+ end
end
class MockPostAuthenticateTasks
@@ -178,6 +188,138 @@ class TokenExchangeControllerTest < ActionController::TestCase
end
end
+ [
+ ShopifyAPI::Errors::InvalidJwtTokenError,
+ ShopifyAPI::Errors::CookieNotFoundError,
+ ].each do |invalid_shopify_id_token_error|
+ test "Redirects to bounce page if Shopify ID token is invalid with #{invalid_shopify_id_token_error}" do
+ ShopifyApp.configuration.root_url = "/my-root"
+ ShopifyAPI::Utils::SessionUtils.stubs(:current_session_id).raises(invalid_shopify_id_token_error)
+ request.headers["HTTP_AUTHORIZATION"] = nil
+
+ params = { shop: @shop, my_param: "for-keeps", id_token: "dont-include-this-id-token" }
+ reload_url = CGI.escape("/reloaded_path?my_param=for-keeps&shop=#{@shop}")
+ expected_redirect_url = "/my-root/patch_shopify_id_token"\
+ "?my_param=for-keeps&shop=#{@shop}"\
+ "&shopify-reload=#{reload_url}"
+
+ with_application_test_routes do
+ get :reloaded_path, params: params
+ assert_redirected_to expected_redirect_url
+ end
+ end
+
+ test "Responds with unauthorized if Shopify Id token is invalid with #{invalid_shopify_id_token_error} and authorization header exists" do
+ ShopifyAPI::Utils::SessionUtils.stubs(:current_session_id).raises(invalid_shopify_id_token_error)
+ request.headers["HTTP_AUTHORIZATION"] = @id_token_in_header
+ expected_response = { errors: [{ message: :unauthorized }] }
+
+ with_application_test_routes do
+ get :make_api_call, params: { shop: @shop }
+
+ assert_response :unauthorized
+ assert_equal expected_response.to_json, response.body
+ assert_equal 1, response.headers["X-Shopify-Retry-Invalid-Session-Request"]
+ end
+ end
+
+ test "Redirects to bounce page from Token Exchange if Shopify ID token is invalid with #{invalid_shopify_id_token_error}" do
+ ShopifyApp.configuration.root_url = "/my-root"
+ ShopifyAPI::Utils::SessionUtils.stubs(:current_session_id).returns(nil, @offline_session_id)
+ ShopifyApp::Auth::TokenExchange.expects(:perform).raises(invalid_shopify_id_token_error)
+ request.headers["HTTP_AUTHORIZATION"] = nil
+
+ params = { shop: @shop, my_param: "for-keeps", id_token: "dont-include-this-id-token" }
+ reload_url = CGI.escape("/reloaded_path?my_param=for-keeps&shop=#{@shop}")
+ expected_redirect_url = "/my-root/patch_shopify_id_token"\
+ "?my_param=for-keeps&shop=#{@shop}"\
+ "&shopify-reload=#{reload_url}"
+
+ with_application_test_routes do
+ get :reloaded_path, params: params
+ assert_redirected_to expected_redirect_url
+ end
+ end
+
+ test "Responds with unauthorized from Token Exchange if Shopify Id token is invalid with #{invalid_shopify_id_token_error} and authorization header exists" do
+ ShopifyAPI::Utils::SessionUtils.stubs(:current_session_id).returns(nil, @offline_session_id)
+ ShopifyApp::Auth::TokenExchange.expects(:perform).raises(invalid_shopify_id_token_error)
+
+ expected_response = { errors: [{ message: :unauthorized }] }
+
+ with_application_test_routes do
+ get :make_api_call, params: { shop: @shop }
+
+ assert_response :unauthorized
+ assert_equal expected_response.to_json, response.body
+ assert_equal 1, response.headers["X-Shopify-Retry-Invalid-Session-Request"]
+ end
+ end
+
+ test "Redirects to bounce page from with_token_refetch if Shopify ID token is invalid with #{invalid_shopify_id_token_error}" do
+ ShopifyApp.configuration.root_url = "/my-root"
+ ShopifyAPI::Utils::SessionUtils.stubs(:current_session_id).returns(@offline_session_id)
+ ShopifyApp::Auth::TokenExchange.stubs(:perform)
+ request.headers["HTTP_AUTHORIZATION"] = nil
+
+ @controller.expects(:with_token_refetch).raises(invalid_shopify_id_token_error)
+
+ params = { shop: @shop, my_param: "for-keeps", id_token: "dont-include-this-id-token" }
+ reload_url = CGI.escape("/reloaded_path?my_param=for-keeps&shop=#{@shop}")
+ expected_redirect_url = "/my-root/patch_shopify_id_token"\
+ "?my_param=for-keeps&shop=#{@shop}"\
+ "&shopify-reload=#{reload_url}"
+
+ with_application_test_routes do
+ get :reloaded_path, params: params
+ assert_redirected_to expected_redirect_url
+ end
+ end
+
+ test "Responds with unauthorized from with_token_refetch if Shopify Id token is invalid with #{invalid_shopify_id_token_error} and authorization header exists" do
+ ShopifyAPI::Utils::SessionUtils.stubs(:current_session_id).returns(@offline_session_id)
+ ShopifyApp::Auth::TokenExchange.stubs(:perform)
+ expected_response = { errors: [{ message: :unauthorized }] }
+
+ @controller.expects(:with_token_refetch).raises(invalid_shopify_id_token_error)
+
+ with_application_test_routes do
+ get :make_api_call, params: { shop: @shop }
+
+ assert_response :unauthorized
+ assert_equal expected_response.to_json, response.body
+ assert_equal 1, response.headers["X-Shopify-Retry-Invalid-Session-Request"]
+ end
+ end
+
+ test "Does not redirect to bounce page if redirect/response has been performed already - #{invalid_shopify_id_token_error}" do
+ ShopifyAPI::Utils::SessionUtils.stubs(:current_session_id).returns(@offline_session_id)
+ ShopifyApp::Auth::TokenExchange.stubs(:perform)
+ request.headers["HTTP_AUTHORIZATION"] = nil
+
+ @controller.expects(:with_token_refetch).raises(invalid_shopify_id_token_error)
+ @controller.stubs(:performed?).returns(true)
+
+ with_application_test_routes do
+ get :ensure_render, params: { shop: @shop }
+ assert_response :ok
+ end
+ end
+
+ test "Does not respond 401 if redirect/response has been performed already - #{invalid_shopify_id_token_error}" do
+ ShopifyAPI::Utils::SessionUtils.stubs(:current_session_id).returns(@offline_session_id)
+ ShopifyApp::Auth::TokenExchange.stubs(:perform)
+
+ @controller.expects(:with_token_refetch).raises(invalid_shopify_id_token_error)
+ @controller.stubs(:performed?).returns(true)
+
+ with_application_test_routes do
+ get :ensure_render, params: { shop: @shop }
+ assert_response :ok
+ end
+ end
+ end
+
private
def with_application_test_routes
@@ -185,6 +327,8 @@ def with_application_test_routes
set.draw do
get "/" => "token_exchange#index"
get "/make_api_call" => "token_exchange#make_api_call"
+ get "/reloaded_path" => "token_exchange#reloaded_path"
+ get "/ensure_render" => "token_exchange#ensure_render"
end
yield
end