-
Notifications
You must be signed in to change notification settings - Fork 706
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
Add token exchange concern #1817
Changes from all commits
8c72def
0efe87f
b53fe3d
fcef5f2
c3c6fb5
525aeea
6079cf4
5448d55
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,14 +6,20 @@ module EnsureHasSession | |
|
||
included do | ||
include ShopifyApp::Localization | ||
include ShopifyApp::LoginProtection | ||
|
||
if ShopifyApp.configuration.use_new_embedded_auth_strategy? | ||
include ShopifyApp::TokenExchange | ||
around_action :activate_shopify_session | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just wondering - is this repeated in both paths because we need the Not that I mind the repetition, just curious. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I repeated it so that it's explicit in which action is called, if we leave this as a shared/common action outside of this condition, then we're implicitly forcing the method name to be the same between the 2 concerns. Which would also work, but I feel like less implicit dependency the better? 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I agree - less magic, more good. |
||
else | ||
include ShopifyApp::LoginProtection | ||
before_action :login_again_if_different_user_or_shop | ||
around_action :activate_shopify_session | ||
after_action :add_top_level_redirection_headers | ||
end | ||
|
||
include ShopifyApp::CsrfProtection | ||
include ShopifyApp::EmbeddedApp | ||
include ShopifyApp::EnsureBilling | ||
|
||
before_action :login_again_if_different_user_or_shop | ||
around_action :activate_shopify_session | ||
after_action :add_top_level_redirection_headers | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,142 @@ | ||
# frozen_string_literal: true | ||
|
||
module ShopifyApp | ||
module TokenExchange | ||
extend ActiveSupport::Concern | ||
|
||
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 | ||
|
||
begin | ||
ShopifyApp::Logger.debug("Activating Shopify session") | ||
ShopifyAPI::Context.activate_session(current_shopify_session) | ||
yield | ||
ensure | ||
ShopifyApp::Logger.debug("Deactivating session") | ||
ShopifyAPI::Context.deactivate_session | ||
end | ||
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 | ||
end | ||
|
||
def current_shopify_domain | ||
return if params[:shop].blank? | ||
|
||
ShopifyApp::Utils.sanitize_shop_domain(params[:shop]) | ||
end | ||
|
||
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 | ||
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)") | ||
exchange_token( | ||
shop: domain, # TODO: use jwt_shopify_domain ? | ||
session_token: session_token, | ||
requested_token_type: ShopifyAPI::Auth::TokenExchange::RequestedTokenType::ONLINE_ACCESS_TOKEN, | ||
) | ||
end | ||
|
||
# TODO: Refactor and add post authenticate tasks | ||
# ShopifyApp.configuration.post_authenticate_tasks.perform(session) if 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 | ||
end | ||
|
||
def session_token | ||
@session_token ||= id_token_header | ||
end | ||
|
||
def id_token_header | ||
request.headers["HTTP_AUTHORIZATION"]&.match(/^Bearer (.+)$/)&.[](1) | ||
end | ||
|
||
def respond_to_invalid_session_token | ||
# TODO: Implement this method to handle invalid session tokens | ||
|
||
# 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) | ||
|
||
# bounce_url = "#{ShopifyAPI::Context.host}#{request.path}?#{patch_session_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 | ||
|
||
# redirect_to("#{patch_session_token_url}?#{patch_session_token_params.to_query}", allow_other_host: true) | ||
# end | ||
end | ||
|
||
def online_token_configured? | ||
ShopifyApp.configuration.online_token_configured? | ||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe these conditions should only apply if the app is embedded, right? We'd still want to use
LoginProtection
for non-embedded?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes that's right,
use_new_embedded_auth_strategy
is only true if app is embedded AND haswip_new_embedded_auth_strategy
enabled.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh perfect, I missed that part :)