Skip to content

Commit

Permalink
Validate store sessions (Shopify#1612)
Browse files Browse the repository at this point in the history
* Check shop offline session is still valid when embedded

* only redirect on 401 and use better naming

* use guard clause instead of conditional filter

* redirect to shop_login if token failed

* update tests to match expected behavior

* Changelog updates

* raise error if not 401

* better redirect test
  • Loading branch information
nelsonwittwer authored and fabriazza committed Feb 1, 2023
1 parent 2a77b30 commit f1eeacc
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
Unreleased
----------
* Removed Logged output for rescued JWT exceptions [#1610](https://github.com/Shopify/shopify_app/pull/1610)
* Validates shop's offline session token is still valid when using `EnsureInstalled`[#1612](https://github.com/Shopify/shopify_app/pull/1612)

21.3.1 (Dec 12, 2022)
----------
Expand Down
17 changes: 16 additions & 1 deletion app/controllers/concerns/shopify_app/ensure_installed.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ module EnsureInstalled

before_action :check_shop_domain
before_action :check_shop_known
before_action :validate_non_embedded_session
end

def current_shopify_domain
Expand All @@ -30,14 +31,18 @@ def current_shopify_domain
@shopify_domain
end

def installed_shop_session
@installed_shop_session ||= SessionRepository.retrieve_shop_session_by_shopify_domain(current_shopify_domain)
end

private

def check_shop_domain
redirect_to(ShopifyApp.configuration.login_url) unless current_shopify_domain
end

def check_shop_known
@shop = SessionRepository.retrieve_shop_session_by_shopify_domain(current_shopify_domain)
@shop = installed_shop_session
unless @shop
if embedded_param?
redirect_for_embedded
Expand All @@ -58,5 +63,15 @@ def shop_login

url.to_s
end

def validate_non_embedded_session
return if loaded_directly_from_admin?

client = ShopifyAPI::Clients::Rest::Admin.new(session: installed_shop_session)
client.get(path: "shop")
rescue ShopifyAPI::Errors::HttpResponseError => error
redirect_to(shop_login) if error.code == 401
raise error if error.code != 401
end
end
end
6 changes: 5 additions & 1 deletion lib/shopify_app/controller_concerns/redirect_for_embedded.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@ def embedded_redirect_url?
end

def embedded_param?
embedded_redirect_url? && params[:embedded].present? && params[:embedded] == "1"
embedded_redirect_url? && params[:embedded].present? && loaded_directly_from_admin?
end

def loaded_directly_from_admin?
ShopifyApp.configuration.embedded_app? && params[:embedded] == "1"
end

def redirect_for_embedded
Expand Down
80 changes: 79 additions & 1 deletion test/controllers/concerns/ensure_installed_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,12 @@ def index
end

test "returns :ok if the shop is installed" do
ShopifyApp::SessionRepository.expects(:retrieve_shop_session_by_shopify_domain).returns(true)
session = mock
ShopifyApp::SessionRepository.stubs(:retrieve_shop_session_by_shopify_domain).returns(session)

client = mock
ShopifyAPI::Clients::Rest::Admin.expects(:new).with(session: session).returns(client)
client.expects(:get)

shopify_domain = "shop1.myshopify.com"

Expand All @@ -61,6 +66,79 @@ def index
assert_response :ok
end

test "redirects to login_url (oauth path) to reinstall the app if the store's session token is no longer valid" do
session = mock
ShopifyApp::SessionRepository.stubs(:retrieve_shop_session_by_shopify_domain).returns(session)

client = mock
ShopifyAPI::Clients::Rest::Admin.expects(:new).with(session: session).returns(client)
uninstalled_http_error = ShopifyAPI::Errors::HttpResponseError.new(
response: ShopifyAPI::Clients::HttpResponse.new(
code: 401,
headers: {},
body: "Invalid API key or access token (unrecognized login or wrong password)",
),
)
client.expects(:get).with(path: "shop").raises(uninstalled_http_error)

shopify_domain = "shop1.myshopify.com"
host = "https://tunnel.vision.for.webhooks.com"

get :index, params: { shop: shopify_domain, host: host }

url = URI(ShopifyApp.configuration.login_url)
url.query = URI.encode_www_form(
shop: shopify_domain,
host: host,
return_to: request.fullpath,
)
assert_redirected_to url.to_s
end

test "throws an error if the shopify error isn't a 401" do
session = mock
ShopifyApp::SessionRepository.stubs(:retrieve_shop_session_by_shopify_domain).returns(session)

client = mock
ShopifyAPI::Clients::Rest::Admin.expects(:new).with(session: session).returns(client)
uninstalled_http_error = ShopifyAPI::Errors::HttpResponseError.new(
response: ShopifyAPI::Clients::HttpResponse.new(
code: 404,
headers: {},
body: "Insert generic message about how we can't find your requests here.",
),
)
client.expects(:get).with(path: "shop").raises(uninstalled_http_error)

shopify_domain = "shop1.myshopify.com"

assert_raises ShopifyAPI::Errors::HttpResponseError do
get :index, params: { shop: shopify_domain }
end
end

test "throws an error if the request session validation API check fails with an" do
session = mock
ShopifyApp::SessionRepository.stubs(:retrieve_shop_session_by_shopify_domain).returns(session)

client = mock
ShopifyAPI::Clients::Rest::Admin.expects(:new).with(session: session).returns(client)
client.expects(:get).with(path: "shop").raises(RuntimeError)

shopify_domain = "shop1.myshopify.com"

assert_raises RuntimeError do
get :index, params: { shop: shopify_domain }
end
end

test "does not perform a session validation check if coming from an embedded" do
ShopifyApp::SessionRepository.stubs(:retrieve_shop_session_by_shopify_domain)
ShopifyAPI::Clients::Rest::Admin.expects(:new).never

get :index, params: { shop: "shop1.myshopify.com" }
end

test "detects incompatible controller concerns" do
version = "22.0.0"
ShopifyApp::Logger.expects(:deprecated).with(regexp_matches(/incompatible concerns/), version)
Expand Down
7 changes: 6 additions & 1 deletion test/controllers/concerns/require_known_shop_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,12 @@ def index
end

test "returns :ok if the shop is installed" do
ShopifyApp::SessionRepository.expects(:retrieve_shop_session_by_shopify_domain).returns(true)
session = mock
ShopifyApp::SessionRepository.stubs(:retrieve_shop_session_by_shopify_domain).returns(session)

client = mock
ShopifyAPI::Clients::Rest::Admin.expects(:new).with(session: session).returns(client)
client.expects(:get)

shopify_domain = "shop1.myshopify.com"

Expand Down

0 comments on commit f1eeacc

Please sign in to comment.