From 6cd0f31544227718f3557dfeed7c3aa42eecd0c7 Mon Sep 17 00:00:00 2001 From: Martin Emde Date: Wed, 20 Nov 2024 18:07:52 -0800 Subject: [PATCH] Add more test coverage for admin policies --- app/policies/admin/nil_class_policy.rb | 9 +++++ test/policies/admin/api_key_policy_test.rb | 6 +++ .../policies/admin/application_policy_test.rb | 15 ++++++++ test/policies/admin/deletion_policy_test.rb | 4 ++ test/policies/admin/geoip_info_policy_test.rb | 4 ++ test/policies/admin/ip_address_policy_test.rb | 6 +++ test/policies/admin/nil_class_policy_test.rb | 32 ++++++++++++++++ test/policies/admin/rubygem_policy_test.rb | 5 +++ test/policies/admin/version_policy_test.rb | 38 +++++++++++++++++++ test/test_helper.rb | 17 +++++++++ 10 files changed, 136 insertions(+) create mode 100644 app/policies/admin/nil_class_policy.rb create mode 100644 test/policies/admin/application_policy_test.rb create mode 100644 test/policies/admin/nil_class_policy_test.rb create mode 100644 test/policies/admin/version_policy_test.rb diff --git a/app/policies/admin/nil_class_policy.rb b/app/policies/admin/nil_class_policy.rb new file mode 100644 index 00000000000..8535bd103e0 --- /dev/null +++ b/app/policies/admin/nil_class_policy.rb @@ -0,0 +1,9 @@ +class Admin::NilClassPolicy < Admin::ApplicationPolicy + class Scope < Admin::ApplicationPolicy::Scope + def resolve + raise Pundit::NotDefinedError, "Cannot scope NilClass" + end + end + + # fallback to parent policy which rejects all actions +end diff --git a/test/policies/admin/api_key_policy_test.rb b/test/policies/admin/api_key_policy_test.rb index 97da3b5fb3d..d0771cf10c6 100644 --- a/test/policies/admin/api_key_policy_test.rb +++ b/test/policies/admin/api_key_policy_test.rb @@ -14,6 +14,12 @@ def test_scope ).to_a end + def test_associations + assert_association @admin, @api_key, :api_key_rubygem_scope, Admin::ApiKeyPolicy + assert_association @admin, @api_key, :ownership, Admin::OwnershipPolicy + assert_association @admin, @api_key, :oidc_id_token, Admin::OIDC::IdTokenPolicy + end + def test_avo_index refute_authorizes @admin, ApiKey, :avo_index? refute_authorizes @non_admin, ApiKey, :avo_index? diff --git a/test/policies/admin/application_policy_test.rb b/test/policies/admin/application_policy_test.rb new file mode 100644 index 00000000000..d4221cd9dbc --- /dev/null +++ b/test/policies/admin/application_policy_test.rb @@ -0,0 +1,15 @@ +require "test_helper" + +class Admin::ApplicationPolicyTest < AdminPolicyTestCase + should "onle inherit from Admin::ApplicationPolicy in Admin:: namespace" do + Admin.constants.each do |const| + next if const == :ApplicationPolicy + next unless const.to_s.end_with?("Policy") + + klass = Admin.const_get(const) + + assert_operator klass, :<, Admin::ApplicationPolicy, "#{const} does not inherit from Admin::ApplicationPolicy" + assert_operator klass::Scope, :<, Admin::ApplicationPolicy::Scope, "#{const}::Scope does not inherit from Admin::ApplicationPolicy::Scope" + end + end +end diff --git a/test/policies/admin/deletion_policy_test.rb b/test/policies/admin/deletion_policy_test.rb index 50508bc5863..31e3b436efb 100644 --- a/test/policies/admin/deletion_policy_test.rb +++ b/test/policies/admin/deletion_policy_test.rb @@ -8,6 +8,10 @@ class Admin::DeletionPolicyTest < AdminPolicyTestCase @non_admin = create(:admin_github_user) end + def test_associations + assert_association @admin, @deletion, :version, Admin::VersionPolicy + end + def test_scope assert_equal [@deletion], policy_scope!( @admin, diff --git a/test/policies/admin/geoip_info_policy_test.rb b/test/policies/admin/geoip_info_policy_test.rb index 9c81adb8439..b791cfd399e 100644 --- a/test/policies/admin/geoip_info_policy_test.rb +++ b/test/policies/admin/geoip_info_policy_test.rb @@ -8,6 +8,10 @@ class Admin::GeoipInfoPolicyTest < AdminPolicyTestCase @non_admin = create(:admin_github_user) end + def test_associations + assert_association @admin, @geoip_info, :ip_addresses, Admin::IpAddressPolicy + end + def test_scope assert_equal [@geoip_info], policy_scope!( @admin, diff --git a/test/policies/admin/ip_address_policy_test.rb b/test/policies/admin/ip_address_policy_test.rb index d4204553453..74f5dcc7b3c 100644 --- a/test/policies/admin/ip_address_policy_test.rb +++ b/test/policies/admin/ip_address_policy_test.rb @@ -7,6 +7,12 @@ class Admin::IpAddressPolicyTest < AdminPolicyTestCase @non_admin = create(:admin_github_user) end + def test_associations + assert_association @admin, @ip_address, :user_events, Admin::Events::UserEventPolicy + assert_association @admin, @ip_address, :rubygem_events, Admin::Events::RubygemEventPolicy + assert_association @admin, @ip_address, :organization_events, Admin::Events::OrganizationEventPolicy + end + def test_scope assert_equal [@ip_address], policy_scope!( @admin, diff --git a/test/policies/admin/nil_class_policy_test.rb b/test/policies/admin/nil_class_policy_test.rb new file mode 100644 index 00000000000..3351998da12 --- /dev/null +++ b/test/policies/admin/nil_class_policy_test.rb @@ -0,0 +1,32 @@ +require "test_helper" + +class Admin::NilClassPolicyTest < AdminPolicyTestCase + def policy!(api_key) + Pundit.policy!(api_key, [:api, nil]) + end + + should "inherit from Admin::ApplicationPolicy" do + assert_operator Admin::NilClassPolicy, :<, Admin::ApplicationPolicy + assert_operator Admin::NilClassPolicy::Scope, :<, Admin::ApplicationPolicy::Scope + end + + context "::Scope.resolve" do + should "raise" do + assert_raises Pundit::NotDefinedError do + Admin::NilClassPolicy::Scope.new(nil, nil).resolve + end + end + end + + should "not authorize any avo action" do + refute_authorizes nil, nil, :avo_index? + refute_authorizes nil, nil, :avo_show? + refute_authorizes nil, nil, :avo_create? + refute_authorizes nil, nil, :avo_new? + refute_authorizes nil, nil, :avo_update? + refute_authorizes nil, nil, :avo_edit? + refute_authorizes nil, nil, :avo_destroy? + refute_authorizes nil, nil, :act_on? + refute_authorizes nil, nil, :avo_search? + end +end diff --git a/test/policies/admin/rubygem_policy_test.rb b/test/policies/admin/rubygem_policy_test.rb index 95cd3b67462..5403bcf555d 100644 --- a/test/policies/admin/rubygem_policy_test.rb +++ b/test/policies/admin/rubygem_policy_test.rb @@ -12,6 +12,11 @@ def test_scope @admin, Rubygem ).to_a + + assert_empty policy_scope!( + @non_admin, + Rubygem + ).to_a end def test_avo_index diff --git a/test/policies/admin/version_policy_test.rb b/test/policies/admin/version_policy_test.rb new file mode 100644 index 00000000000..7134915343c --- /dev/null +++ b/test/policies/admin/version_policy_test.rb @@ -0,0 +1,38 @@ +require "test_helper" + +class Admin::VersionPolicyTest < AdminPolicyTestCase + setup do + @admin = FactoryBot.create(:admin_github_user, :is_admin) + @non_admin = FactoryBot.create(:admin_github_user) + @version = FactoryBot.create(:version, :yanked) + end + + def test_scope + assert_equal [@version], policy_scope!( + @admin, + Version + ).to_a + assert_empty policy_scope!( + @non_admin, + Version + ).to_a + end + + def test_avo_index + assert_authorizes @admin, Version, :avo_index? + + refute_authorizes @non_admin, Version, :avo_index? + end + + def test_avo_show + assert_authorizes @admin, @version, :avo_show? + + refute_authorizes @non_admin, @version, :avo_show? + end + + def test_act_on + assert_authorizes @admin, @version, :act_on? + + refute_authorizes @non_admin, @version, :act_on? + end +end diff --git a/test/test_helper.rb b/test/test_helper.rb index ea5aaba7cf0..b5c11b97833 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -286,6 +286,23 @@ def refute_authorizes(user, record, action) end end + def assert_association(user, record, association, _policy_class) + %w[create attach detach destroy edit].each do |action| + refute_authorizes(user, record, :"#{action}_#{association}?") + end + + begin + @authorization_client.authorize(user, record, :avo_show?, policy_class: policy_class) + + assert_authorizes(user, record, :"view_#{association}?") + rescue Avo::NotAuthorizedError + refute_authorizes(user, record, :"view_#{association}?") + end + + # TODO: I'm not clear on what `record` is used in show_association? + # assert_authorizes(user, record, :"show_#{association}?") + end + def policy_class nil end