From 4809db59da513c0e8d7457d0ae615601d767151a Mon Sep 17 00:00:00 2001 From: "Aleksandar N. Kostadinov" Date: Thu, 1 Aug 2024 01:21:33 +0300 Subject: [PATCH 1/2] tenant checker should recognize master better there are more means master can authenticate which tenant leak checker should recognize fixes THREESCALE-11214 --- lib/three_scale/middleware/multitenant.rb | 22 ++++++++--- .../providers_controller_integration_test.rb | 2 - .../multitenant_enforcement_test.rb | 37 ++++++++++++++++++- 3 files changed, 53 insertions(+), 8 deletions(-) diff --git a/lib/three_scale/middleware/multitenant.rb b/lib/three_scale/middleware/multitenant.rb index 06661ee664..41dfd57620 100644 --- a/lib/three_scale/middleware/multitenant.rb +++ b/lib/three_scale/middleware/multitenant.rb @@ -75,14 +75,26 @@ def master? @master ||= ::Account.unscoped.master + @is_master = session_of_master? || provider_key_of_master? || token_of_master? + end + + def session_of_master? # this is supposed to match how we get the user_session in app/lib/authenticated_system/request.rb # on API calls cookies are not present though, so we need to use safe navigation - @user_session ||= UserSession.authenticate(@env['action_dispatch.cookies']&.signed&.public_send(:[], :user_session)) + user_session ||= UserSession.authenticate(@env['action_dispatch.cookies']&.signed&.public_send(:[], :user_session)) + user_session&.user&.account == @master + end + + def provider_key_of_master? + param_places = %w[action_dispatch.request.query_parameters action_dispatch.request.request_parameters] + possible_keys = %w[provider_key api_key] + param_places.product(possible_keys).any? { |params, key| @env[params][key] == @master.api_key } + end - @is_master = @user_session&.user&.account == @master || - @env["action_dispatch.request.query_parameters"]["provider_key"] == @master.api_key || - # must match how we check access token in app/lib/api_authentication/by_access_token.rb - @master.access_tokens.find_from_value(@env["action_dispatch.request.query_parameters"]["access_token"]) + def token_of_master? + token = @env["action_dispatch.request.query_parameters"]["access_token"] || + @env["action_dispatch.request.request_parameters"]["access_token"] + @master.access_tokens.find_from_value(token) end end diff --git a/test/integration/master/api/providers_controller_integration_test.rb b/test/integration/master/api/providers_controller_integration_test.rb index 3d6b7fde42..b376e1542d 100644 --- a/test/integration/master/api/providers_controller_integration_test.rb +++ b/test/integration/master/api/providers_controller_integration_test.rb @@ -19,8 +19,6 @@ def setup end test '#create' do - AccessToken.stubs(:random_id).returns('randomValue') - # sends activation email ProviderUserMailer.expects(:activation).returns(mock(deliver_later: true)) diff --git a/test/integration/multitenant_enforcement_test.rb b/test/integration/multitenant_enforcement_test.rb index d194320d8a..7e11939453 100644 --- a/test/integration/multitenant_enforcement_test.rb +++ b/test/integration/multitenant_enforcement_test.rb @@ -57,9 +57,11 @@ class MultitenantEnforcementTest < ActionDispatch::IntegrationTest AccessToken.new.value get admin_api_service_application_plans_path(service_id: service.id, format: :json, access_token: token) assert_response :success + put admin_api_service_application_plan_path(service_id: service.id, id: plan.id, format: :json), params: {access_token: token, description: "desc1"} + assert_response :success end - test "multitenant account can retrieve from multiple tenants by master key" do + test "multitenant master can retrieve from multiple tenants by provider key" do host! master_account.external_admin_domain service = master_account.first_service! plan = FactoryBot.create(:application_plan, issuer: service) @@ -67,6 +69,39 @@ class MultitenantEnforcementTest < ActionDispatch::IntegrationTest plan.update_column(:tenant_id, @provider.tenant_id + 1) get admin_api_service_application_plans_path(service_id: service.id, format: :json, provider_key: master_account.provider_key) assert_response :success + put admin_api_service_application_plan_path(service_id: service.id, id: plan.id, format: :json), params: {provider_key: master_account.provider_key, description: "desc1"} + assert_response :success + end + + test "multitenant master retrieve from multiple tenants in master API by api_key in query" do + proxies = FactoryBot.create_list(:proxy, 2) + proxies.each do |proxy| + FactoryBot.create_list(:proxy_config, 1, proxy: proxy, environment: 'sandbox') + FactoryBot.create_list(:proxy_config, 1, proxy: proxy, environment: 'production') + end + proxies[1].update(tenant_id: proxies[0].reload.tenant_id + 1) + + host! master_account.internal_admin_domain + get master_api_proxy_configs_path(environment: 'production', api_key: master_account.provider_key) + assert_response :success + end + + test "multitenant master retrieve from multiple tenants in master API by api_key in request" do + User.any_instance.expects(:tenant_id).at_least(2).returns(@provider.reload.tenant_id) + + signup_params = { + api_key: master_account.api_key, + org_name: 'Alaska', + username: 'person', + email: 'person@example.com', + password: '123456', + user_extra_field: 'hi-user', + account_extra_field: 'hi-account' + } + + host! master_account.internal_admin_domain + post master_api_providers_path, params: signup_params + assert_response :created end test "multitenant account can retrieve from multiple tenants by user" do From b6c54838beb1daaf6b756fffa07cf918fcf18842 Mon Sep 17 00:00:00 2001 From: "Aleksandar N. Kostadinov" Date: Thu, 1 Aug 2024 16:03:55 +0300 Subject: [PATCH 2/2] Update lib/three_scale/middleware/multitenant.rb Co-authored-by: Daria Mayorova --- lib/three_scale/middleware/multitenant.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/three_scale/middleware/multitenant.rb b/lib/three_scale/middleware/multitenant.rb index 41dfd57620..6c099a1ce7 100644 --- a/lib/three_scale/middleware/multitenant.rb +++ b/lib/three_scale/middleware/multitenant.rb @@ -88,7 +88,7 @@ def session_of_master? def provider_key_of_master? param_places = %w[action_dispatch.request.query_parameters action_dispatch.request.request_parameters] possible_keys = %w[provider_key api_key] - param_places.product(possible_keys).any? { |params, key| @env[params][key] == @master.api_key } + param_places.product(possible_keys).any? { |params, key| @env[params][key] == @master.provider_key } end def token_of_master?