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

Jump to bottom

Set CurrentTenant to nil when gem is present #1898

Closed
wants to merge 1 commit into from
Closed

Set CurrentTenant to nil when gem is present #1898

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions lib/shopify_app/controller_concerns/login_protection.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,11 @@ def host
end

def redirect_to_login
if defined?(ShopifySecurityBase::CurrentTenant)
ShopifyApp::Logger.debug("ShopifySecurityBase::CurrentTenant detected, setting current tenant to NilTenant")
ShopifySecurityBase::CurrentTenant.tenant = ShopifySecurityBase::NilTenant.new
end

if requested_by_javascript?
add_top_level_redirection_headers(ignore_response_code: true)
ShopifyApp::Logger.debug("Login redirect request is a XHR")
Expand Down
5 changes: 5 additions & 0 deletions lib/shopify_app/controller_concerns/token_exchange.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ def respond_to_invalid_shopify_id_token(error)
ShopifyApp::Logger.debug("Responding to invalid Shopify ID token: #{error.message}")
return if performed?

if defined?(ShopifySecurityBase::CurrentTenant)
ShopifyApp::Logger.debug("ShopifySecurityBase::CurrentTenant detected, setting current tenant to NilTenant")
ShopifySecurityBase::CurrentTenant.tenant = ShopifySecurityBase::NilTenant.new
end

if request.headers["HTTP_AUTHORIZATION"].blank?
if embedded?
redirect_to_bounce_page
Expand Down
47 changes: 47 additions & 0 deletions test/shopify_app/controller_concerns/login_protection_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,35 @@ class LoginProtectionControllerTest < ActionController::TestCase
end
end

test "#redirect_to_login sets current tenant to nil before redirecting when ShopifySecurityBase::CurrentTenant is defined" do
::ShopifyAPI::Utils::SessionUtils.stubs(:current_session_id).returns(nil)
ShopifyApp::Logger.stubs(:debug)

with_shopify_security_base_current_tenant do |current_tenant_class, nil_tenant|
with_application_test_routes do
current_tenant_class.expects(:tenant=).with(nil_tenant)
ShopifyApp::Logger.expects(:debug)
.with("ShopifySecurityBase::CurrentTenant detected, setting current tenant to NilTenant")

get :index, params: { shop: "foobar" }
assert_redirected_to "/login?shop=foobar.myshopify.com"
end
end
end

test "#redirect_to_login ignores ShopifySecurityBase::CurrentTenant when not defined" do
::ShopifyAPI::Utils::SessionUtils.stubs(:current_session_id).returns(nil)
ShopifyApp::Logger.stubs(:debug)

with_application_test_routes do
ShopifyApp::Logger.expects(:debug)
.with("ShopifySecurityBase::CurrentTenant detected, setting current tenant to NilTenant").never

get :index, params: { shop: "foobar" }
assert_redirected_to "/login?shop=foobar.myshopify.com"
end
end

private

def assert_fullpage_redirected(shop_domain, expect_embedded)
Expand Down Expand Up @@ -656,4 +685,22 @@ def with_custom_login_url(url)
ensure
ShopifyApp.configure { |config| config.login_url = original_url }
end

def with_shopify_security_base_current_tenant
shopify_security_base_module = Module.new
ShopifyApp::LoginProtection.const_set("ShopifySecurityBase", shopify_security_base_module)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find a nicer way to stub the existence of a class than using const_set then remove_const (which for some reason is private even though const_set isn't). Open to other ideas if anyone has them!

current_tenant_class = stub(:current_tenant_class, "tenant=": nil)
shopify_security_base_module.const_set("CurrentTenant", current_tenant_class)
nil_tenant_class = stub(:nil_tenant_class)
shopify_security_base_module.const_set("NilTenant", nil_tenant_class)

nil_tenant = stub(:nil_tenant)
nil_tenant_class.stubs(:new).returns(nil_tenant)

yield(current_tenant_class, nil_tenant)
ensure
if ShopifyApp::LoginProtection.const_defined?("ShopifySecurityBase")
ShopifyApp::LoginProtection.send(:remove_const, "ShopifySecurityBase")
end
end
end
53 changes: 53 additions & 0 deletions test/shopify_app/controller_concerns/token_exchange_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,41 @@ class TokenExchangeControllerTest < ActionController::TestCase
end
end

test "sets current tenant to nil before redirecting when ShopifySecurityBase::CurrentTenant is defined" do
ShopifyAPI::Utils::SessionUtils.stubs(:session_id_from_shopify_id_token)
.raises(ShopifyAPI::Errors::InvalidJwtTokenError)
request.headers["HTTP_AUTHORIZATION"] = nil

ShopifyApp::Logger.stubs(:debug)

with_shopify_security_base_current_tenant do |current_tenant_class, nil_tenant|
with_application_test_routes do
current_tenant_class.expects(:tenant=).with(nil_tenant)
ShopifyApp::Logger.expects(:debug)
.with("ShopifySecurityBase::CurrentTenant detected, setting current tenant to NilTenant")

get :index, params: { shop: @shop, host: Base64.encode64("#{@shop}/admin") }
assert_response :redirect
end
end
end

test "ignores ShopifySecurityBase::CurrentTenant when not defined when redirecting" do
ShopifyAPI::Utils::SessionUtils.stubs(:session_id_from_shopify_id_token)
.raises(ShopifyAPI::Errors::InvalidJwtTokenError)
request.headers["HTTP_AUTHORIZATION"] = nil

ShopifyApp::Logger.stubs(:debug)

with_application_test_routes do
ShopifyApp::Logger.expects(:debug)
.with("ShopifySecurityBase::CurrentTenant detected, setting current tenant to NilTenant").never

get :index, params: { shop: @shop, host: Base64.encode64("#{@shop}/admin") }
assert_response :redirect
end
end

private

def with_application_test_routes
Expand All @@ -383,4 +418,22 @@ def with_application_test_routes
yield
end
end

def with_shopify_security_base_current_tenant
shopify_security_base_module = Module.new
ShopifyApp::TokenExchange.const_set("ShopifySecurityBase", shopify_security_base_module)
current_tenant_class = stub(:current_tenant_class, "tenant=": nil)
shopify_security_base_module.const_set("CurrentTenant", current_tenant_class)
nil_tenant_class = stub(:nil_tenant_class)
shopify_security_base_module.const_set("NilTenant", nil_tenant_class)

nil_tenant = stub(:nil_tenant)
nil_tenant_class.stubs(:new).returns(nil_tenant)

yield(current_tenant_class, nil_tenant)
ensure
if ShopifyApp::TokenExchange.const_defined?("ShopifySecurityBase")
ShopifyApp::TokenExchange.send(:remove_const, "ShopifySecurityBase")
end
end
end
Loading