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.

Sign up for GitHub

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

Always register webhooks with offline sessions #1788

Merged
merged 2 commits into from
Feb 9, 2024
Merged

Always register webhooks with offline sessions #1788

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4188d32
Always register webhooks with offline sessions
paulomarg Feb 8, 2024
7cb554d
Applying comments from review
paulomarg Feb 9, 2024
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ Unreleased
----------
* Make type param for webhooks route optional. This will fix a bug with CLI initiated webhooks.[1786](https://github.com/Shopify/shopify_app/pull/1786)
* Fix redirecting to login when we catch a 401 response from Shopify, so that it can also handle cases where the app is already embedded when that happens.[1787](https://github.com/Shopify/shopify_app/pull/1787)
* Always register webhooks with offline sessions.[1788](https://github.com/Shopify/shopify_app/pull/1788)

21.10.0 (January 24, 2024)
----------
Expand Down
8 changes: 6 additions & 2 deletions app/controllers/shopify_app/callback_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,12 @@ def user_access_scopes_strategy
end

def perform_post_authenticate_jobs(session)
install_webhooks(session)
install_scripttags(session)
# Ensure we use the shop session to install webhooks and scripttags
session_for_shop = session.online? ? shop_session : session

install_webhooks(session_for_shop)
install_scripttags(session_for_shop)

perform_after_authenticate_job(session)
end

Expand Down
105 changes: 71 additions & 34 deletions test/controllers/callback_controller_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,21 @@ def perform; end
end

module ShopifyApp
SHOP_DOMAIN = "shop.myshopify.io"

class CallbackControllerTest < ActionController::TestCase
setup do
@routes = ShopifyApp::Engine.routes
ShopifyApp::SessionRepository.shop_storage = ShopifyApp::InMemoryShopSessionStore
ShopifyApp::SessionRepository.user_storage = nil
ShopifyAppConfigurer.setup_context
I18n.locale = :en
@stubbed_session = ShopifyAPI::Auth::Session.new(shop: "shop", access_token: "token")
@stubbed_session = ShopifyAPI::Auth::Session.new(shop: SHOP_DOMAIN, access_token: "token")
@stubbed_cookie = ShopifyAPI::Auth::Oauth::SessionCookie.new(value: "", expires: Time.now)
@host = "little-shoppe-of-horrors.#{ShopifyApp.configuration.myshopify_domain}"
host = Base64.strict_encode64(@host + "/admin")
@callback_params = {
shop: "shop",
shop: SHOP_DOMAIN,
code: "code",
state: "state",
timestamp: "timestamp",
Expand All @@ -47,17 +49,27 @@ class CallbackControllerTest < ActionController::TestCase
ShopifyApp::SessionRepository.stubs(:store_session)
end

teardown do
SessionRepository.shop_storage.clear
end

test "#callback flashes error in Spanish" do
I18n.expects(:t).with("could_not_log_in")
get :callback,
params: { shop: "shop", code: "code", state: "state", timestamp: "timestamp", host: "host", hmac: "hmac" }
params: { shop: SHOP_DOMAIN, code: "code", state: "state", timestamp: "timestamp", host: "host", hmac: "hmac" }
end

test "#callback rescued errors of ShopifyAPI::Error will not emit a deprecation notice" do
ShopifyAPI::Auth::Oauth.expects(:validate_auth_callback).raises(ShopifyAPI::Errors::MissingRequiredArgumentError)
assert_not_deprecated do
get :callback,
params: { shop: "shop", code: "code", state: "state", timestamp: "timestamp", host: "host", hmac: "hmac" }
get :callback, params: {
shop: SHOP_DOMAIN,
code: "code",
state: "state",
timestamp: "timestamp",
host: "host",
hmac: "hmac",
}
end
assert_equal flash[:error], "Could not log in to Shopify store"
end
Expand All @@ -69,7 +81,7 @@ class CallbackControllerTest < ActionController::TestCase

ShopifyApp::Logger.expects(:deprecated).never
get :callback,
params: { shop: "shop", code: "code", state: "state", timestamp: "timestamp", host: "host", hmac: "hmac" }
params: { shop: SHOP_DOMAIN, code: "code", state: "state", timestamp: "timestamp", host: "host", hmac: "hmac" }
end

test "#callback rescued non-shopify errors will be deprecated" do
Expand All @@ -86,7 +98,7 @@ class CallbackControllerTest < ActionController::TestCase
assert_within_deprecation_schedule(version)
ShopifyApp::Logger.expects(:deprecated).with(message, version)
get :callback,
params: { shop: "shop", code: "code", state: "state", timestamp: "timestamp", host: "host", hmac: "hmac" }
params: { shop: SHOP_DOMAIN, code: "code", state: "state", timestamp: "timestamp", host: "host", hmac: "hmac" }
end

test "#callback calls ShopifyAPI::Auth::Oauth.validate_auth_callback" do
Expand Down Expand Up @@ -118,29 +130,18 @@ class CallbackControllerTest < ActionController::TestCase
end

test "#callback sets the shopify_user_id in the Rails session when session is online" do
associated_user = ShopifyAPI::Auth::AssociatedUser.new(
id: 42,
first_name: "LeeeEEeeeeee3roy",
last_name: "Jenkins",
email: "[email protected]",
email_verified: true,
locale: "en",
collaborator: true,
account_owner: true,
)
mock_session = ShopifyAPI::Auth::Session.new(
shop: "shop",
access_token: "token",
is_online: true,
associated_user: associated_user,
)
mock_oauth(session: mock_session)
ShopifyApp::SessionRepository.shop_storage.store(@stubbed_session)
ShopifyApp::SessionRepository.user_storage = ShopifyApp::InMemoryUserSessionStore

mock_session = online_session
mock_oauth(session: online_session)

get :callback, params: @callback_params
assert_equal session[:shopify_user_id], associated_user.id
assert_equal session[:shopify_user_id], mock_session.associated_user.id
end

test "#callback DOES NOT set the shopify_user_id in the Rails session when session is offline" do
mock_session = ShopifyAPI::Auth::Session.new(shop: "shop", access_token: "token", is_online: false)
mock_session = ShopifyAPI::Auth::Session.new(shop: SHOP_DOMAIN, access_token: "token", is_online: false)
mock_oauth(session: mock_session)
get :callback, params: @callback_params
assert_nil session[:shopify_user_id]
Expand All @@ -166,7 +167,7 @@ class CallbackControllerTest < ActionController::TestCase
config.webhooks = [{ topic: "carts/update", address: "example-app.com/webhooks" }]
end

ShopifyApp::WebhooksManager.expects(:queue).with("shop", "token")
ShopifyApp::WebhooksManager.expects(:queue).with(SHOP_DOMAIN, "token")

mock_oauth
get :callback, params: @callback_params
Expand All @@ -189,7 +190,7 @@ class CallbackControllerTest < ActionController::TestCase
config.after_authenticate_job = { job: Shopify::AfterAuthenticateJob, inline: true }
end

Shopify::AfterAuthenticateJob.expects(:perform_now).with(shop_domain: "shop")
Shopify::AfterAuthenticateJob.expects(:perform_now).with(shop_domain: SHOP_DOMAIN)

mock_oauth
get :callback, params: @callback_params
Expand All @@ -200,7 +201,7 @@ class CallbackControllerTest < ActionController::TestCase
config.after_authenticate_job = { job: Shopify::AfterAuthenticateJob, inline: false }
end

Shopify::AfterAuthenticateJob.expects(:perform_later).with(shop_domain: "shop")
Shopify::AfterAuthenticateJob.expects(:perform_later).with(shop_domain: SHOP_DOMAIN)

mock_oauth
get :callback, params: @callback_params
Expand All @@ -222,7 +223,7 @@ class CallbackControllerTest < ActionController::TestCase
config.after_authenticate_job = { job: Shopify::AfterAuthenticateJob }
end

Shopify::AfterAuthenticateJob.expects(:perform_later).with(shop_domain: "shop")
Shopify::AfterAuthenticateJob.expects(:perform_later).with(shop_domain: SHOP_DOMAIN)

mock_oauth
get :callback, params: @callback_params
Expand All @@ -233,7 +234,7 @@ class CallbackControllerTest < ActionController::TestCase
config.after_authenticate_job = { job: "Shopify::AfterAuthenticateJob", inline: false }
end

Shopify::AfterAuthenticateJob.expects(:perform_later).with(shop_domain: "shop")
Shopify::AfterAuthenticateJob.expects(:perform_later).with(shop_domain: SHOP_DOMAIN)

mock_oauth
get :callback, params: @callback_params
Expand Down Expand Up @@ -296,10 +297,27 @@ class CallbackControllerTest < ActionController::TestCase
config.webhooks = [{ topic: "carts/update", address: "example-app.com/webhooks" }]
end

ShopifyApp::WebhooksManager.expects(:queue).with("shop", "token")
ShopifyApp::WebhooksManager.expects(:queue).with(SHOP_DOMAIN, "token")

get :callback, params: @callback_params
assert_response 302
end

test "#callback performs install_webhook job with an offline session after an online session OAuth" do
ShopifyApp.configure do |config|
config.webhooks = [{ topic: "carts/update", address: "example-app.com/webhooks" }]
end
ShopifyApp::SessionRepository.shop_storage.store(@stubbed_session)
ShopifyApp::SessionRepository.user_storage = ShopifyApp::InMemoryUserSessionStore

mock_oauth(session: online_session)

ShopifyApp::WebhooksManager.expects(:queue).with(SHOP_DOMAIN, "token")

get :callback, params: @callback_params
assert_response 302
ensure
ShopifyApp::SessionRepository.shop_storage.clear
end

test "#callback performs install_scripttags job after authentication" do
Expand All @@ -309,7 +327,7 @@ class CallbackControllerTest < ActionController::TestCase
config.scripttags = [{ event: "onload", src: "https://example.com/fancy.js" }]
end

ShopifyApp::ScripttagsManager.expects(:queue).with("shop", "token", ShopifyApp.configuration.scripttags)
ShopifyApp::ScripttagsManager.expects(:queue).with(SHOP_DOMAIN, "token", ShopifyApp.configuration.scripttags)

get :callback, params: @callback_params
assert_response 302
Expand All @@ -322,7 +340,7 @@ class CallbackControllerTest < ActionController::TestCase
config.after_authenticate_job = { job: Shopify::AfterAuthenticateJob, inline: true }
end

Shopify::AfterAuthenticateJob.expects(:perform_now).with(shop_domain: "shop")
Shopify::AfterAuthenticateJob.expects(:perform_now).with(shop_domain: SHOP_DOMAIN)

get :callback, params: @callback_params
assert_response 302
Expand All @@ -348,5 +366,24 @@ def mock_oauth(cookie: @stubbed_cookie, session: @stubbed_session)
session: session,
})
end

def online_session
associated_user = ShopifyAPI::Auth::AssociatedUser.new(
id: 42,
first_name: "LeeeEEeeeeee3roy",
last_name: "Jenkins",
email: "[email protected]",
email_verified: true,
locale: "en",
collaborator: true,
account_owner: true,
)
ShopifyAPI::Auth::Session.new(
shop: SHOP_DOMAIN,
access_token: "online-token",
is_online: true,
associated_user: associated_user,
)
end
end
end
Loading