From 527dea27cd724b8c1f3da09e3865fc0cfc80f327 Mon Sep 17 00:00:00 2001 From: Rachel Carvalho Date: Mon, 15 Apr 2024 12:48:14 -0400 Subject: [PATCH 1/3] bump shopify_api to 14.2 --- Gemfile.lock | 4 ++-- shopify_app.gemspec | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 786a5d853..ba1fb479c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -7,7 +7,7 @@ PATH jwt (>= 2.2.3) rails (> 5.2.1) redirect_safely (~> 1.0) - shopify_api (>= 14.1.0, < 15.0) + shopify_api (>= 14.2.0, < 15.0) sprockets-rails (>= 2.0.0) GEM @@ -217,7 +217,7 @@ GEM ruby-progressbar (1.13.0) ruby2_keywords (0.0.5) securerandom (0.2.2) - shopify_api (14.1.0) + shopify_api (14.2.0) activesupport concurrent-ruby hash_diff diff --git a/shopify_app.gemspec b/shopify_app.gemspec index 078d10ffb..713c6ef5d 100644 --- a/shopify_app.gemspec +++ b/shopify_app.gemspec @@ -19,7 +19,7 @@ Gem::Specification.new do |s| s.add_runtime_dependency("jwt", ">= 2.2.3") s.add_runtime_dependency("rails", "> 5.2.1") s.add_runtime_dependency("redirect_safely", "~> 1.0") - s.add_runtime_dependency("shopify_api", ">= 14.1.0", "< 15.0") + s.add_runtime_dependency("shopify_api", ">= 14.2.0", "< 15.0") s.add_runtime_dependency("sprockets-rails", ">= 2.0.0") s.add_development_dependency("byebug") From ec50b58f9fff96be35d90a5397af99a9811db239 Mon Sep 17 00:00:00 2001 From: Rachel Carvalho Date: Fri, 5 Apr 2024 16:19:58 -0400 Subject: [PATCH 2/3] extract token exchange class from concern --- lib/shopify_app.rb | 1 + lib/shopify_app/auth/token_exchange.rb | 73 +++++++ .../controller_concerns/token_exchange.rb | 59 +----- test/shopify_app/auth/token_exchange_test.rb | 179 ++++++++++++++++++ .../token_exchange_test.rb | 93 ++------- 5 files changed, 272 insertions(+), 133 deletions(-) create mode 100644 lib/shopify_app/auth/token_exchange.rb create mode 100644 test/shopify_app/auth/token_exchange_test.rb diff --git a/lib/shopify_app.rb b/lib/shopify_app.rb index 0880d3a67..3c947050a 100644 --- a/lib/shopify_app.rb +++ b/lib/shopify_app.rb @@ -56,6 +56,7 @@ def self.use_webpacker? # Auth helpers require "shopify_app/auth/post_authenticate_tasks" + require "shopify_app/auth/token_exchange" # jobs require "shopify_app/jobs/webhooks_manager_job" diff --git a/lib/shopify_app/auth/token_exchange.rb b/lib/shopify_app/auth/token_exchange.rb new file mode 100644 index 000000000..d278ed943 --- /dev/null +++ b/lib/shopify_app/auth/token_exchange.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +module ShopifyApp + module Auth + class TokenExchange + attr_reader :session_token + + def self.perform(session_token) + new(session_token).perform + end + + def initialize(session_token) + @session_token = session_token + end + + def perform + domain = ShopifyApp::JWT.new(session_token).shopify_domain + + Logger.info("Performing Token Exchange for [#{domain}] - (Offline)") + session = exchange_token( + shop: domain, + session_token: session_token, + requested_token_type: ShopifyAPI::Auth::TokenExchange::RequestedTokenType::OFFLINE_ACCESS_TOKEN, + ) + + if online_token_configured? + Logger.info("Performing Token Exchange for [#{domain}] - (Online)") + session = exchange_token( + shop: domain, + session_token: session_token, + requested_token_type: ShopifyAPI::Auth::TokenExchange::RequestedTokenType::ONLINE_ACCESS_TOKEN, + ) + end + + ShopifyApp.configuration.post_authenticate_tasks.perform(session) + + session + end + + private + + def exchange_token(shop:, session_token:, requested_token_type:) + session = ShopifyAPI::Auth::TokenExchange.exchange_token( + shop: shop, + session_token: session_token, + requested_token_type: requested_token_type, + ) + + SessionRepository.store_session(session) + + session + rescue ShopifyAPI::Errors::InvalidJwtTokenError + Logger.error("Invalid session token '#{session_token}' during token exchange") + raise + rescue ShopifyAPI::Errors::HttpResponseError => error + Logger.error( + "A #{error.code} error (#{error.class}) occurred during the token exchange. Response: #{error.response.body}", + ) + raise + rescue ActiveRecord::RecordNotUnique + Logger.debug("Session not stored due to concurrent token exchange calls") + session + rescue => error + Logger.error("An error occurred during the token exchange: [#{error.class}] #{error.message}") + raise + end + + def online_token_configured? + ShopifyApp.configuration.online_token_configured? + end + end + end +end diff --git a/lib/shopify_app/controller_concerns/token_exchange.rb b/lib/shopify_app/controller_concerns/token_exchange.rb index ba9621f93..6d2dc916e 100644 --- a/lib/shopify_app/controller_concerns/token_exchange.rb +++ b/lib/shopify_app/controller_concerns/token_exchange.rb @@ -49,61 +49,10 @@ def retrieve_session_from_token_exchange # TODO: Right now JWT Middleware only updates env['jwt.shopify_domain'] from request headers tokens, # which won't work for new installs. # we need to update the middleware to also update the env['jwt.shopify_domain'] from the query params - domain = ShopifyApp::JWT.new(session_token).shopify_domain - - ShopifyApp::Logger.info("Performing Token Exchange for [#{domain}] - (Offline)") - session = exchange_token( - shop: domain, # TODO: use jwt_shopify_domain ? - session_token: session_token, - requested_token_type: ShopifyAPI::Auth::TokenExchange::RequestedTokenType::OFFLINE_ACCESS_TOKEN, - ) - - if session && online_token_configured? - ShopifyApp::Logger.info("Performing Token Exchange for [#{domain}] - (Online)") - session = exchange_token( - shop: domain, # TODO: use jwt_shopify_domain ? - session_token: session_token, - requested_token_type: ShopifyAPI::Auth::TokenExchange::RequestedTokenType::ONLINE_ACCESS_TOKEN, - ) - end - - ShopifyApp.configuration.post_authenticate_tasks.perform(session) - end - - def exchange_token(shop:, session_token:, requested_token_type:) - if session_token.blank? - # respond_to_invalid_session_token - return - end - - begin - session = ShopifyAPI::Auth::TokenExchange.exchange_token( - shop: shop, - session_token: session_token, - requested_token_type: requested_token_type, - ) - rescue ShopifyAPI::Errors::InvalidJwtTokenError - # respond_to_invalid_session_token - return - rescue ShopifyAPI::Errors::HttpResponseError => error - ShopifyApp::Logger.error( - "A #{error.code} error (#{error.class}) occurred during the token exchange. Response: #{error.response.body}", - ) - raise - rescue => error - ShopifyApp::Logger.error("An error occurred during the token exchange: #{error.message}") - raise - end - - if session - begin - ShopifyApp::SessionRepository.store_session(session) - rescue ActiveRecord::RecordNotUnique - ShopifyApp::Logger.debug("Session not stored due to concurrent token exchange calls") - end - end - - session + ShopifyApp::Auth::TokenExchange.perform(session_token) + # TODO: Rescue JWT validation errors when bounce page is ready + # rescue ShopifyAPI::Errors::InvalidJwtTokenError + # respond_to_invalid_session_token end def session_token diff --git a/test/shopify_app/auth/token_exchange_test.rb b/test/shopify_app/auth/token_exchange_test.rb new file mode 100644 index 000000000..8f55def8f --- /dev/null +++ b/test/shopify_app/auth/token_exchange_test.rb @@ -0,0 +1,179 @@ +# frozen_string_literal: true + +require "test_helper" + +class ShopifyApp::Auth::TokenExchangeTest < ActiveSupport::TestCase + OFFLINE_ACCESS_TOKEN_TYPE = ShopifyAPI::Auth::TokenExchange::RequestedTokenType::OFFLINE_ACCESS_TOKEN + ONLINE_ACCESS_TOKEN_TYPE = ShopifyAPI::Auth::TokenExchange::RequestedTokenType::ONLINE_ACCESS_TOKEN + + def setup + ShopifyApp::SessionRepository.shop_storage = ShopifyApp::InMemoryShopSessionStore + ShopifyApp::SessionRepository.user_storage = nil + + @shop = "my-shop.myshopify.com" + @user_id = 1 + @session_token = build_jwt + + @offline_session = build_offline_session + @online_session = build_online_session + + ShopifyApp.configuration.post_authenticate_tasks.stubs(:perform) + end + + test "#perform exchanges offline token then stores it when only shop session store is configured" do + ShopifyAPI::Auth::TokenExchange.expects(:exchange_token).with( + shop: @shop, + session_token: @session_token, + requested_token_type: OFFLINE_ACCESS_TOKEN_TYPE, + ).once.returns(@offline_session) + + assert_nil ShopifyApp::SessionRepository.load_session(@offline_session.id) + + new_session = ShopifyApp::Auth::TokenExchange.perform(@session_token) + + assert_equal @offline_session, new_session + assert_equal @offline_session, ShopifyApp::SessionRepository.load_session(@offline_session.id) + end + + test "#perform exchanges both online and offline tokens then stores it when user session store is configured" do + ShopifyApp::SessionRepository.user_storage = ShopifyApp::InMemoryUserSessionStore + + ShopifyAPI::Auth::TokenExchange.expects(:exchange_token).with( + shop: @shop, + session_token: @session_token, + requested_token_type: OFFLINE_ACCESS_TOKEN_TYPE, + ).returns(@offline_session) + + ShopifyAPI::Auth::TokenExchange.expects(:exchange_token).with( + shop: @shop, + session_token: @session_token, + requested_token_type: ONLINE_ACCESS_TOKEN_TYPE, + ).returns(@online_session) + + assert_nil ShopifyApp::SessionRepository.load_session(@offline_session.id) + assert_nil ShopifyApp::SessionRepository.load_session(@online_session.id) + + new_session = ShopifyApp::Auth::TokenExchange.perform(@session_token) + + assert_equal @online_session, new_session + assert_equal @offline_session, ShopifyApp::SessionRepository.load_session(@offline_session.id) + assert_equal @online_session, ShopifyApp::SessionRepository.load_session(@online_session.id) + end + + test "#perform triggers post_authenticate_tasks after token exchange is complete" do + ShopifyAPI::Auth::TokenExchange.stubs(:exchange_token).returns(@offline_session) + ShopifyApp.configuration.post_authenticate_tasks.expects(:perform).with(@offline_session) + + ShopifyApp::Auth::TokenExchange.perform(@session_token) + end + + test "#perform triggers post_authenticate_tasks after token exchange is complete for online session" do + ShopifyApp::SessionRepository.user_storage = ShopifyApp::InMemoryUserSessionStore + + ShopifyAPI::Auth::TokenExchange.stubs(:exchange_token).returns(@offline_session, @online_session) + ShopifyApp.configuration.post_authenticate_tasks.expects(:perform).with(@online_session) + + ShopifyApp::Auth::TokenExchange.perform(@session_token) + end + + test "#perform logs invalid JWT errors from the API and re-raises them" do + ShopifyAPI::Auth::TokenExchange.stubs(:exchange_token).raises(ShopifyAPI::Errors::InvalidJwtTokenError) + + ShopifyApp::Logger.expects(:error).with(regexp_matches(/Invalid session token/)) + + assert_raises ShopifyAPI::Errors::InvalidJwtTokenError do + ShopifyApp::Auth::TokenExchange.perform(@session_token) + end + end + + test "#perform logs HTTP errors coming from Shopify API and re-raises them" do + response = ShopifyAPI::Clients::HttpResponse.new(code: 401, body: { error: "oops" }.to_json, headers: {}) + error = ShopifyAPI::Errors::HttpResponseError.new(response: response) + + ShopifyAPI::Auth::TokenExchange.stubs(:exchange_token).raises(error) + + ShopifyApp::Logger.expects(:error).with("A 401 error (ShopifyAPI::Errors::HttpResponseError) occurred " \ + "during the token exchange. Response: {\"error\":\"oops\"}") + + assert_raises ShopifyAPI::Errors::HttpResponseError do + ShopifyApp::Auth::TokenExchange.perform(@session_token) + end + end + + test "#perform ignores ActiveRecord::RecordNotUnique when trying to store the token and returns the new token" do + ShopifyApp::SessionRepository.user_storage = ShopifyApp::InMemoryUserSessionStore + + ShopifyAPI::Auth::TokenExchange.expects(:exchange_token).with( + shop: @shop, + session_token: @session_token, + requested_token_type: OFFLINE_ACCESS_TOKEN_TYPE, + ).returns(@offline_session) + + ShopifyAPI::Auth::TokenExchange.expects(:exchange_token).with( + shop: @shop, + session_token: @session_token, + requested_token_type: ONLINE_ACCESS_TOKEN_TYPE, + ).returns(@online_session) + + ShopifyApp::SessionRepository.stubs(:store_session).raises(ActiveRecord::RecordNotUnique) + + ShopifyApp::Logger.stubs(:debug) + ShopifyApp::Logger.expects(:debug).twice.with("Session not stored due to concurrent token exchange calls") + + new_session = ShopifyApp::Auth::TokenExchange.perform(@session_token) + + assert_equal @online_session, new_session + end + + test "#perform logs unexpected errors coming from Shopify API and re-raises them" do + ShopifyAPI::Auth::TokenExchange.stubs(:exchange_token).raises("not today!") + + ShopifyApp::Logger.expects(:error).with("An error occurred during the token exchange: [RuntimeError] not today!") + + assert_raises "not today!" do + ShopifyApp::Auth::TokenExchange.perform(@session_token) + end + end + + private + + def build_jwt(shop: @shop) + payload = { + iss: "https://#{shop}/admin", + dest: "https://#{shop}", + aud: ShopifyAPI::Context.api_key, + sub: @user_id.to_s, + exp: (Time.now + 10).to_i, + nbf: 1234, + iat: 1234, + jti: "4321", + sid: "abc123", + } + JWT.encode(payload, ShopifyAPI::Context.api_secret_key, "HS256") + end + + def build_offline_session(shop: @shop) + ShopifyAPI::Auth::Session.new(id: "offline_#{shop}", shop: shop, access_token: "offline-token") + end + + def build_online_session(shop: @shop, user_id: @user_id) + user = ShopifyAPI::Auth::AssociatedUser.new( + id: user_id, + first_name: "Hello", + last_name: "World", + email: "Email", + email_verified: true, + account_owner: true, + locale: "en", + collaborator: false, + ) + + ShopifyAPI::Auth::Session.new( + id: "#{shop}_#{user_id}", + shop: shop, + is_online: true, + access_token: "online-token", + associated_user: user, + ) + end +end diff --git a/test/shopify_app/controller_concerns/token_exchange_test.rb b/test/shopify_app/controller_concerns/token_exchange_test.rb index fd5d8b02a..5f03e5131 100644 --- a/test/shopify_app/controller_concerns/token_exchange_test.rb +++ b/test/shopify_app/controller_concerns/token_exchange_test.rb @@ -54,11 +54,11 @@ class TokenExchangeControllerTest < ActionController::TestCase @offline_session_id = "offline_#{@shop}" @online_session_id = "online_#{@user.id}" - ShopifyApp.configuration.check_session_expiry_date = false + ShopifyApp.configuration.check_session_expiry_date = true ShopifyApp.configuration.custom_post_authenticate_tasks = MockPostAuthenticateTasks end - test "Exchanges offline token when session doesn't exist" do + test "Exchanges token when session doesn't exist" do with_application_test_routes do ShopifyAPI::Utils::SessionUtils.stubs(:current_session_id).with( @session_token_in_header, @@ -66,11 +66,9 @@ class TokenExchangeControllerTest < ActionController::TestCase false, ).returns(nil, @offline_session_id) - ShopifyAPI::Auth::TokenExchange.expects(:exchange_token).with( - shop: @shop, - session_token: @session_token, - requested_token_type: ShopifyAPI::Auth::TokenExchange::RequestedTokenType::OFFLINE_ACCESS_TOKEN, - ).returns(@offline_session) + ShopifyApp::Auth::TokenExchange.expects(:perform).with(@session_token) do + ShopifyApp::SessionRepository.store_session(@offline_session) + end ShopifyAPI::Context.expects(:activate_session).with(@offline_session) @@ -78,34 +76,6 @@ class TokenExchangeControllerTest < ActionController::TestCase end end - test "Exchanges online token if user session store is configured" do - ShopifyApp::SessionRepository.user_storage = ShopifyApp::InMemoryUserSessionStore - - with_application_test_routes do - ShopifyAPI::Utils::SessionUtils.stubs(:current_session_id).with( - @session_token_in_header, - nil, - true, - ).returns(nil, @online_session_id) - - ShopifyAPI::Auth::TokenExchange.expects(:exchange_token).with( - shop: @shop, - session_token: @session_token, - requested_token_type: ShopifyAPI::Auth::TokenExchange::RequestedTokenType::OFFLINE_ACCESS_TOKEN, - ).returns(@offline_session) - - ShopifyAPI::Auth::TokenExchange.expects(:exchange_token).with( - shop: @shop, - session_token: @session_token, - requested_token_type: ShopifyAPI::Auth::TokenExchange::RequestedTokenType::ONLINE_ACCESS_TOKEN, - ).returns(@online_session) - - ShopifyAPI::Context.expects(:activate_session).with(@online_session) - - get :index, params: { shop: @shop } - end - end - test "Use existing shop session if it exists" do ShopifyApp::SessionRepository.store_shop_session(@offline_session) @@ -116,7 +86,7 @@ class TokenExchangeControllerTest < ActionController::TestCase false, ).returns(@offline_session_id) - ShopifyAPI::Auth::TokenExchange.expects(:exchange_token).never + ShopifyApp::Auth::TokenExchange.expects(:perform).never ShopifyAPI::Context.expects(:activate_session).with(@offline_session) @@ -135,7 +105,7 @@ class TokenExchangeControllerTest < ActionController::TestCase true, ).returns(@online_session_id) - ShopifyAPI::Auth::TokenExchange.expects(:exchange_token).never + ShopifyApp::Auth::TokenExchange.expects(:perform).never ShopifyAPI::Context.expects(:activate_session).with(@online_session) @@ -144,7 +114,6 @@ class TokenExchangeControllerTest < ActionController::TestCase end test "Exchange token again if current user session is expired" do - ShopifyApp.configuration.check_session_expiry_date = true ShopifyApp::SessionRepository.user_storage = ShopifyApp::InMemoryUserSessionStore ShopifyApp::SessionRepository.store_user_session(@online_session, @user) @online_session.stubs(:expired?).returns(true) @@ -156,20 +125,10 @@ class TokenExchangeControllerTest < ActionController::TestCase true, ).returns(@online_session_id) - ShopifyAPI::Auth::TokenExchange.expects(:exchange_token).with( - shop: @shop, - session_token: @session_token, - requested_token_type: ShopifyAPI::Auth::TokenExchange::RequestedTokenType::OFFLINE_ACCESS_TOKEN, - ).returns(@offline_session) - - ShopifyAPI::Auth::TokenExchange.expects(:exchange_token).with( - shop: @shop, - session_token: @session_token, - requested_token_type: ShopifyAPI::Auth::TokenExchange::RequestedTokenType::ONLINE_ACCESS_TOKEN, - ).returns(@online_session) - - ShopifyApp::SessionRepository.shop_storage.expects(:store).with(@offline_session) - ShopifyApp::SessionRepository.user_storage.expects(:store).with(@online_session, @user) + ShopifyApp::Auth::TokenExchange.expects(:perform).with(@session_token) do + ShopifyApp::SessionRepository.store_session(@offline_session) + ShopifyApp::SessionRepository.store_session(@online_session) + end ShopifyAPI::Context.expects(:activate_session).with(@online_session) @@ -177,11 +136,11 @@ class TokenExchangeControllerTest < ActionController::TestCase end end - test "Don't exchange token if current user session is not expired" do - ShopifyApp.configuration.check_session_expiry_date = true + test "Don't exchange token if check_session_expiry_date config is false" do + ShopifyApp.configuration.check_session_expiry_date = false ShopifyApp::SessionRepository.user_storage = ShopifyApp::InMemoryUserSessionStore ShopifyApp::SessionRepository.store_user_session(@online_session, @user) - @online_session.stubs(:expired?).returns(false) + @online_session.stubs(:expired?).returns(true) with_application_test_routes do ShopifyAPI::Utils::SessionUtils.stubs(:current_session_id).with( @@ -190,7 +149,7 @@ class TokenExchangeControllerTest < ActionController::TestCase true, ).returns(@online_session_id) - ShopifyAPI::Auth::TokenExchange.expects(:exchange_token).never + ShopifyApp::Auth::TokenExchange.expects(:perform).never ShopifyAPI::Context.expects(:activate_session).with(@online_session) @@ -198,28 +157,6 @@ class TokenExchangeControllerTest < ActionController::TestCase end end - test "Triggers post_authenticate_tasks after token exchange is complete" do - with_application_test_routes do - ShopifyAPI::Utils::SessionUtils.stubs(:current_session_id).returns(nil) - ShopifyAPI::Auth::TokenExchange.stubs(:exchange_token).returns(@offline_session) - ShopifyApp.configuration.post_authenticate_tasks.expects(:perform).with(@offline_session) - - get :index, params: { shop: @shop } - end - end - - test "Triggers post_authenticate_tasks after token exchange is complete for online session" do - ShopifyApp::SessionRepository.user_storage = ShopifyApp::InMemoryUserSessionStore - - with_application_test_routes do - ShopifyAPI::Utils::SessionUtils.stubs(:current_session_id).returns(nil) - ShopifyAPI::Auth::TokenExchange.stubs(:exchange_token).returns(@offline_session, @online_session) - ShopifyApp.configuration.post_authenticate_tasks.expects(:perform).with(@online_session) - - get :index, params: { shop: @shop } - end - end - private def with_application_test_routes From 0ddf6529398fe47978b1e27457ca4f415a002300 Mon Sep 17 00:00:00 2001 From: Zoey Lan Date: Mon, 15 Apr 2024 15:06:23 -0400 Subject: [PATCH 3/3] retry action when unauthorized is raised in controller with TokenExchange concern Co-authored-by: Rachel Carvalho --- lib/shopify_app.rb | 3 + .../admin_api/with_token_refetch.rb | 28 ++++++ lib/shopify_app/auth/token_exchange.rb | 22 ++--- .../controller_concerns/token_exchange.rb | 53 +++++----- .../admin_api/with_token_refetch_test.rb | 98 +++++++++++++++++++ test/shopify_app/auth/token_exchange_test.rb | 30 +++--- .../token_exchange_test.rb | 42 ++++++-- 7 files changed, 212 insertions(+), 64 deletions(-) create mode 100644 lib/shopify_app/admin_api/with_token_refetch.rb create mode 100644 test/shopify_app/admin_api/with_token_refetch_test.rb diff --git a/lib/shopify_app.rb b/lib/shopify_app.rb index 3c947050a..83140ba0f 100644 --- a/lib/shopify_app.rb +++ b/lib/shopify_app.rb @@ -40,6 +40,9 @@ def self.use_webpacker? require "shopify_app/logger" + # Admin API helpers + require "shopify_app/admin_api/with_token_refetch" + # controller concerns require "shopify_app/controller_concerns/csrf_protection" require "shopify_app/controller_concerns/localization" diff --git a/lib/shopify_app/admin_api/with_token_refetch.rb b/lib/shopify_app/admin_api/with_token_refetch.rb new file mode 100644 index 000000000..47522ab0c --- /dev/null +++ b/lib/shopify_app/admin_api/with_token_refetch.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module ShopifyApp + module AdminAPI + module WithTokenRefetch + def with_token_refetch(session, shopify_id_token) + retrying = false if retrying.nil? + yield + rescue ShopifyAPI::Errors::HttpResponseError => error + if error.code != 401 + ShopifyApp::Logger.debug("Encountered error: #{error.code} - #{error.response.inspect}, re-raising") + elsif retrying + ShopifyApp::Logger.debug("Shopify API returned a 401 Unauthorized error that was not corrected " \ + "with token exchange, deleting current session and re-raising") + ShopifyApp::SessionRepository.delete_session(session.id) + else + retrying = true + ShopifyApp::Logger.debug("Shopify API returned a 401 Unauthorized error, exchanging token and " \ + "retrying with new session") + new_session = ShopifyApp::Auth::TokenExchange.perform(shopify_id_token) + session.copy_attributes_from(new_session) + retry + end + raise + end + end + end +end diff --git a/lib/shopify_app/auth/token_exchange.rb b/lib/shopify_app/auth/token_exchange.rb index d278ed943..54d550282 100644 --- a/lib/shopify_app/auth/token_exchange.rb +++ b/lib/shopify_app/auth/token_exchange.rb @@ -3,23 +3,23 @@ module ShopifyApp module Auth class TokenExchange - attr_reader :session_token + attr_reader :id_token - def self.perform(session_token) - new(session_token).perform + def self.perform(id_token) + new(id_token).perform end - def initialize(session_token) - @session_token = session_token + def initialize(id_token) + @id_token = id_token end def perform - domain = ShopifyApp::JWT.new(session_token).shopify_domain + domain = ShopifyApp::JWT.new(id_token).shopify_domain Logger.info("Performing Token Exchange for [#{domain}] - (Offline)") session = exchange_token( shop: domain, - session_token: session_token, + id_token: id_token, requested_token_type: ShopifyAPI::Auth::TokenExchange::RequestedTokenType::OFFLINE_ACCESS_TOKEN, ) @@ -27,7 +27,7 @@ def perform Logger.info("Performing Token Exchange for [#{domain}] - (Online)") session = exchange_token( shop: domain, - session_token: session_token, + id_token: id_token, requested_token_type: ShopifyAPI::Auth::TokenExchange::RequestedTokenType::ONLINE_ACCESS_TOKEN, ) end @@ -39,10 +39,10 @@ def perform private - def exchange_token(shop:, session_token:, requested_token_type:) + def exchange_token(shop:, id_token:, requested_token_type:) session = ShopifyAPI::Auth::TokenExchange.exchange_token( shop: shop, - session_token: session_token, + session_token: id_token, requested_token_type: requested_token_type, ) @@ -50,7 +50,7 @@ def exchange_token(shop:, session_token:, requested_token_type:) session rescue ShopifyAPI::Errors::InvalidJwtTokenError - Logger.error("Invalid session token '#{session_token}' during token exchange") + Logger.error("Invalid id token '#{id_token}' during token exchange") raise rescue ShopifyAPI::Errors::HttpResponseError => error Logger.error( diff --git a/lib/shopify_app/controller_concerns/token_exchange.rb b/lib/shopify_app/controller_concerns/token_exchange.rb index 6d2dc916e..a3f45938b 100644 --- a/lib/shopify_app/controller_concerns/token_exchange.rb +++ b/lib/shopify_app/controller_concerns/token_exchange.rb @@ -3,38 +3,37 @@ module ShopifyApp module TokenExchange extend ActiveSupport::Concern + include ShopifyApp::AdminAPI::WithTokenRefetch - def activate_shopify_session - if current_shopify_session.blank? - retrieve_session_from_token_exchange - end - - if ShopifyApp.configuration.check_session_expiry_date && current_shopify_session.expired? - @current_shopify_session = nil - retrieve_session_from_token_exchange - end + 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) - yield + with_token_refetch(current_shopify_session, shopify_id_token, &block) ensure ShopifyApp::Logger.debug("Deactivating session") ShopifyAPI::Context.deactivate_session end end + def should_exchange_expired_token? + ShopifyApp.configuration.check_session_expiry_date && current_shopify_session.expired? + end + def current_shopify_session - @current_shopify_session ||= begin - session_id = ShopifyAPI::Utils::SessionUtils.current_session_id( - request.headers["HTTP_AUTHORIZATION"], - nil, - online_token_configured?, - ) - return nil unless session_id - - ShopifyApp::SessionRepository.load_session(session_id) - end + return unless current_shopify_session_id + + @current_shopify_session ||= ShopifyApp::SessionRepository.load_session(current_shopify_session_id) + end + + def current_shopify_session_id + @current_shopify_session_id ||= ShopifyAPI::Utils::SessionUtils.current_session_id( + request.headers["HTTP_AUTHORIZATION"], + nil, + online_token_configured?, + ) end def current_shopify_domain @@ -46,24 +45,22 @@ def current_shopify_domain private def retrieve_session_from_token_exchange - # TODO: Right now JWT Middleware only updates env['jwt.shopify_domain'] from request headers tokens, - # which won't work for new installs. - # we need to update the middleware to also update the env['jwt.shopify_domain'] from the query params - ShopifyApp::Auth::TokenExchange.perform(session_token) + @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_session_token + # respond_to_invalid_shopify_id_token end - def session_token - @session_token ||= id_token_header + def shopify_id_token + @shopify_id_token ||= id_token_header end def id_token_header request.headers["HTTP_AUTHORIZATION"]&.match(/^Bearer (.+)$/)&.[](1) end - def respond_to_invalid_session_token + def respond_to_invalid_shopify_id_token # TODO: Implement this method to handle invalid session tokens # if request.xhr? diff --git a/test/shopify_app/admin_api/with_token_refetch_test.rb b/test/shopify_app/admin_api/with_token_refetch_test.rb new file mode 100644 index 000000000..880b5c8b5 --- /dev/null +++ b/test/shopify_app/admin_api/with_token_refetch_test.rb @@ -0,0 +1,98 @@ +# frozen_string_literal: true + +require "test_helper" + +class ShopifyApp::AdminAPI::WithTokenRefetchTest < ActiveSupport::TestCase + include ShopifyApp::AdminAPI::WithTokenRefetch + + def setup + @session = ShopifyAPI::Auth::Session.new(id: "session-id", shop: "shop", access_token: "old", expires: 1.hour.ago) + @id_token = "an-id-token" + + tomorrow = 1.day.from_now + @new_session = ShopifyAPI::Auth::Session.new(id: "session-id", shop: "shop", access_token: "new", expires: tomorrow) + + @fake_admin_api = stub(:admin_api) + end + + test "#with_token_refetch takes a block and returns its value" do + result = with_token_refetch(@session, @id_token) do + "returned by block" + end + + assert_equal "returned by block", result + end + + test "#with_token_refetch rescues Admin API HttpResponseError 401, performs token exchange and retries block" do + response = ShopifyAPI::Clients::HttpResponse.new(code: 401, body: { error: "oops" }.to_json, headers: {}) + error = ShopifyAPI::Errors::HttpResponseError.new(response: response) + @fake_admin_api.stubs(:query).raises(error).then.returns("oh now we're good") + + ShopifyApp::Logger.expects(:debug).with("Shopify API returned a 401 Unauthorized error, exchanging token " \ + "and retrying with new session") + + ShopifyApp::Auth::TokenExchange.expects(:perform).with(@id_token).returns(@new_session) + + result = with_token_refetch(@session, @id_token) do + @fake_admin_api.query + end + + assert_equal "oh now we're good", result + end + + test "#with_token_refetch updates original session's attributes when token exchange is performed" do + response = ShopifyAPI::Clients::HttpResponse.new(code: 401, body: "", headers: {}) + error = ShopifyAPI::Errors::HttpResponseError.new(response: response) + @fake_admin_api.stubs(:query).raises(error).then.returns("oh now we're good") + + ShopifyApp::Auth::TokenExchange.stubs(:perform).with(@id_token).returns(@new_session) + + with_token_refetch(@session, @id_token) do + @fake_admin_api.query + end + + assert_equal @new_session.access_token, @session.access_token + assert_equal @new_session.expires, @session.expires + end + + test "#with_token_refetch deletes existing token and re-raises when 401 persists" do + response = ShopifyAPI::Clients::HttpResponse.new(code: 401, body: "401 message", headers: {}) + api_error = ShopifyAPI::Errors::HttpResponseError.new(response: response) + + ShopifyApp::Auth::TokenExchange.stubs(:perform).with(@id_token).returns(@new_session) + + @fake_admin_api.expects(:query).twice.raises(api_error) + + ShopifyApp::Logger.expects(:debug).with("Shopify API returned a 401 Unauthorized error, exchanging token " \ + "and retrying with new session") + + ShopifyApp::Logger.expects(:debug).with("Shopify API returned a 401 Unauthorized error that was not corrected " \ + "with token exchange, deleting current session and re-raising") + ShopifyApp::SessionRepository.expects(:delete_session).with("session-id") + + reraised_error = assert_raises ShopifyAPI::Errors::HttpResponseError do + with_token_refetch(@session, @id_token) do + @fake_admin_api.query + end + end + + assert_equal reraised_error, api_error + end + + test "#with_token_refetch re-raises without deleting session when error is not a 401" do + response = ShopifyAPI::Clients::HttpResponse.new(code: 500, body: { error: "ooops" }.to_json, headers: {}) + api_error = ShopifyAPI::Errors::HttpResponseError.new(response: response) + + @fake_admin_api.expects(:query).raises(api_error) + ShopifyApp::SessionRepository.expects(:delete_session).never + ShopifyApp::Logger.expects(:debug).with(regexp_matches(/Encountered error: 500 \- .*ooops.*, re-raising/)) + + reraised_error = assert_raises ShopifyAPI::Errors::HttpResponseError do + with_token_refetch(@session, @id_token) do + @fake_admin_api.query + end + end + + assert_equal reraised_error, api_error + end +end diff --git a/test/shopify_app/auth/token_exchange_test.rb b/test/shopify_app/auth/token_exchange_test.rb index 8f55def8f..e0fe48455 100644 --- a/test/shopify_app/auth/token_exchange_test.rb +++ b/test/shopify_app/auth/token_exchange_test.rb @@ -12,7 +12,7 @@ def setup @shop = "my-shop.myshopify.com" @user_id = 1 - @session_token = build_jwt + @id_token = build_jwt @offline_session = build_offline_session @online_session = build_online_session @@ -23,13 +23,13 @@ def setup test "#perform exchanges offline token then stores it when only shop session store is configured" do ShopifyAPI::Auth::TokenExchange.expects(:exchange_token).with( shop: @shop, - session_token: @session_token, + session_token: @id_token, requested_token_type: OFFLINE_ACCESS_TOKEN_TYPE, ).once.returns(@offline_session) assert_nil ShopifyApp::SessionRepository.load_session(@offline_session.id) - new_session = ShopifyApp::Auth::TokenExchange.perform(@session_token) + new_session = ShopifyApp::Auth::TokenExchange.perform(@id_token) assert_equal @offline_session, new_session assert_equal @offline_session, ShopifyApp::SessionRepository.load_session(@offline_session.id) @@ -40,20 +40,20 @@ def setup ShopifyAPI::Auth::TokenExchange.expects(:exchange_token).with( shop: @shop, - session_token: @session_token, + session_token: @id_token, requested_token_type: OFFLINE_ACCESS_TOKEN_TYPE, ).returns(@offline_session) ShopifyAPI::Auth::TokenExchange.expects(:exchange_token).with( shop: @shop, - session_token: @session_token, + session_token: @id_token, requested_token_type: ONLINE_ACCESS_TOKEN_TYPE, ).returns(@online_session) assert_nil ShopifyApp::SessionRepository.load_session(@offline_session.id) assert_nil ShopifyApp::SessionRepository.load_session(@online_session.id) - new_session = ShopifyApp::Auth::TokenExchange.perform(@session_token) + new_session = ShopifyApp::Auth::TokenExchange.perform(@id_token) assert_equal @online_session, new_session assert_equal @offline_session, ShopifyApp::SessionRepository.load_session(@offline_session.id) @@ -64,7 +64,7 @@ def setup ShopifyAPI::Auth::TokenExchange.stubs(:exchange_token).returns(@offline_session) ShopifyApp.configuration.post_authenticate_tasks.expects(:perform).with(@offline_session) - ShopifyApp::Auth::TokenExchange.perform(@session_token) + ShopifyApp::Auth::TokenExchange.perform(@id_token) end test "#perform triggers post_authenticate_tasks after token exchange is complete for online session" do @@ -73,16 +73,16 @@ def setup ShopifyAPI::Auth::TokenExchange.stubs(:exchange_token).returns(@offline_session, @online_session) ShopifyApp.configuration.post_authenticate_tasks.expects(:perform).with(@online_session) - ShopifyApp::Auth::TokenExchange.perform(@session_token) + ShopifyApp::Auth::TokenExchange.perform(@id_token) end test "#perform logs invalid JWT errors from the API and re-raises them" do ShopifyAPI::Auth::TokenExchange.stubs(:exchange_token).raises(ShopifyAPI::Errors::InvalidJwtTokenError) - ShopifyApp::Logger.expects(:error).with(regexp_matches(/Invalid session token/)) + ShopifyApp::Logger.expects(:error).with(regexp_matches(/Invalid id token/)) assert_raises ShopifyAPI::Errors::InvalidJwtTokenError do - ShopifyApp::Auth::TokenExchange.perform(@session_token) + ShopifyApp::Auth::TokenExchange.perform(@id_token) end end @@ -96,7 +96,7 @@ def setup "during the token exchange. Response: {\"error\":\"oops\"}") assert_raises ShopifyAPI::Errors::HttpResponseError do - ShopifyApp::Auth::TokenExchange.perform(@session_token) + ShopifyApp::Auth::TokenExchange.perform(@id_token) end end @@ -105,13 +105,13 @@ def setup ShopifyAPI::Auth::TokenExchange.expects(:exchange_token).with( shop: @shop, - session_token: @session_token, + session_token: @id_token, requested_token_type: OFFLINE_ACCESS_TOKEN_TYPE, ).returns(@offline_session) ShopifyAPI::Auth::TokenExchange.expects(:exchange_token).with( shop: @shop, - session_token: @session_token, + session_token: @id_token, requested_token_type: ONLINE_ACCESS_TOKEN_TYPE, ).returns(@online_session) @@ -120,7 +120,7 @@ def setup ShopifyApp::Logger.stubs(:debug) ShopifyApp::Logger.expects(:debug).twice.with("Session not stored due to concurrent token exchange calls") - new_session = ShopifyApp::Auth::TokenExchange.perform(@session_token) + new_session = ShopifyApp::Auth::TokenExchange.perform(@id_token) assert_equal @online_session, new_session end @@ -131,7 +131,7 @@ def setup ShopifyApp::Logger.expects(:error).with("An error occurred during the token exchange: [RuntimeError] not today!") assert_raises "not today!" do - ShopifyApp::Auth::TokenExchange.perform(@session_token) + ShopifyApp::Auth::TokenExchange.perform(@id_token) end end diff --git a/test/shopify_app/controller_concerns/token_exchange_test.rb b/test/shopify_app/controller_concerns/token_exchange_test.rb index 5f03e5131..77510d000 100644 --- a/test/shopify_app/controller_concerns/token_exchange_test.rb +++ b/test/shopify_app/controller_concerns/token_exchange_test.rb @@ -4,6 +4,10 @@ require "action_controller" require "action_controller/base" +class ApiClass + def self.perform; end +end + class TokenExchangeController < ActionController::Base include ShopifyApp::TokenExchange @@ -12,6 +16,11 @@ class TokenExchangeController < ActionController::Base def index render(plain: "OK") end + + def make_api_call + ApiClass.perform + render(plain: "OK") + end end class MockPostAuthenticateTasks @@ -26,9 +35,9 @@ class TokenExchangeControllerTest < ActionController::TestCase @shop = "my-shop.myshopify.com" ShopifyApp::JWT.any_instance.stubs(:shopify_domain).returns(@shop) - @session_token = "this-is-a-session-token" - @session_token_in_header = "Bearer #{@session_token}" - request.headers["HTTP_AUTHORIZATION"] = @session_token_in_header + @id_token = "this-is-an-id-token" + @id_token_in_header = "Bearer #{@id_token}" + request.headers["HTTP_AUTHORIZATION"] = @id_token_in_header ShopifyApp::SessionRepository.shop_storage = ShopifyApp::InMemoryShopSessionStore ShopifyApp::SessionRepository.user_storage = nil @@ -61,12 +70,12 @@ class TokenExchangeControllerTest < ActionController::TestCase test "Exchanges token when session doesn't exist" do with_application_test_routes do ShopifyAPI::Utils::SessionUtils.stubs(:current_session_id).with( - @session_token_in_header, + @id_token_in_header, nil, false, ).returns(nil, @offline_session_id) - ShopifyApp::Auth::TokenExchange.expects(:perform).with(@session_token) do + ShopifyApp::Auth::TokenExchange.expects(:perform).with(@id_token) do ShopifyApp::SessionRepository.store_session(@offline_session) end @@ -81,7 +90,7 @@ class TokenExchangeControllerTest < ActionController::TestCase with_application_test_routes do ShopifyAPI::Utils::SessionUtils.stubs(:current_session_id).with( - @session_token_in_header, + @id_token_in_header, nil, false, ).returns(@offline_session_id) @@ -100,7 +109,7 @@ class TokenExchangeControllerTest < ActionController::TestCase with_application_test_routes do ShopifyAPI::Utils::SessionUtils.stubs(:current_session_id).with( - @session_token_in_header, + @id_token_in_header, nil, true, ).returns(@online_session_id) @@ -120,12 +129,12 @@ class TokenExchangeControllerTest < ActionController::TestCase with_application_test_routes do ShopifyAPI::Utils::SessionUtils.stubs(:current_session_id).with( - @session_token_in_header, + @id_token_in_header, nil, true, ).returns(@online_session_id) - ShopifyApp::Auth::TokenExchange.expects(:perform).with(@session_token) do + ShopifyApp::Auth::TokenExchange.expects(:perform).with(@id_token) do ShopifyApp::SessionRepository.store_session(@offline_session) ShopifyApp::SessionRepository.store_session(@online_session) end @@ -144,7 +153,7 @@ class TokenExchangeControllerTest < ActionController::TestCase with_application_test_routes do ShopifyAPI::Utils::SessionUtils.stubs(:current_session_id).with( - @session_token_in_header, + @id_token_in_header, nil, true, ).returns(@online_session_id) @@ -157,12 +166,25 @@ class TokenExchangeControllerTest < ActionController::TestCase end end + test "Wraps action in with_token_refetch" do + ShopifyApp::SessionRepository.store_shop_session(@offline_session) + ShopifyAPI::Utils::SessionUtils.stubs(:current_session_id).returns(@offline_session_id) + + ApiClass.expects(:perform) + @controller.expects(:with_token_refetch).yields + + with_application_test_routes do + get :make_api_call, params: { shop: @shop } + end + end + private def with_application_test_routes with_routing do |set| set.draw do get "/" => "token_exchange#index" + get "/make_api_call" => "token_exchange#make_api_call" end yield end