From 642d6b56550047dbb4a4b08c7532e171c476c75a Mon Sep 17 00:00:00 2001
From: Martin Emde <martinemde@users.noreply.github.com>
Date: Sun, 1 Dec 2024 13:59:48 -0800
Subject: [PATCH] Admin policy test coverage (#5262)

---
 app/policies/admin/audit_policy.rb            |  6 +--
 app/policies/admin/nil_class_policy.rb        |  9 +++++
 app/policies/api/application_policy.rb        |  5 ---
 test/policies/admin/api_key_policy_test.rb    |  6 +++
 .../policies/admin/application_policy_test.rb | 15 ++++++++
 test/policies/admin/audit_policy_test.rb      | 27 +++++++++++++
 test/policies/admin/deletion_policy_test.rb   |  4 ++
 test/policies/admin/dependency_policy_test.rb | 22 +++++++++++
 test/policies/admin/geoip_info_policy_test.rb |  4 ++
 test/policies/admin/ip_address_policy_test.rb | 34 +++++++++++++++++
 test/policies/admin/nil_class_policy_test.rb  | 27 +++++++++++++
 ...anization_onboarding_invite_policy_test.rb |  5 +++
 .../organization_onboarding_policy_test.rb    |  5 +++
 .../admin/organization_policy_test.rb         |  5 +++
 test/policies/admin/rubygem_policy_test.rb    |  5 +++
 test/policies/admin/version_policy_test.rb    | 38 +++++++++++++++++++
 test/test_helper.rb                           | 17 +++++++++
 17 files changed, 226 insertions(+), 8 deletions(-)
 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/audit_policy_test.rb
 create mode 100644 test/policies/admin/dependency_policy_test.rb
 create mode 100644 test/policies/admin/ip_address_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/audit_policy.rb b/app/policies/admin/audit_policy.rb
index db45584e4c9..843ed1034fa 100644
--- a/app/policies/admin/audit_policy.rb
+++ b/app/policies/admin/audit_policy.rb
@@ -5,16 +5,16 @@ def resolve
       if rubygems_org_admin?
         scope.all
       else
-        scope.where(admin_github_user: current_user)
+        scope.none
       end
     end
   end
 
   def avo_index?
-    true
+    rubygems_org_admin?
   end
 
   def avo_show?
-    true
+    rubygems_org_admin?
   end
 end
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/app/policies/api/application_policy.rb b/app/policies/api/application_policy.rb
index 15b57a4bb8e..c88c0dba936 100644
--- a/app/policies/api/application_policy.rb
+++ b/app/policies/api/application_policy.rb
@@ -2,11 +2,6 @@
 
 class Api::ApplicationPolicy
   class Scope
-    def initialize(api_key, scope)
-      @api_key = api_key
-      @scope = scope
-    end
-
     private
 
     attr_reader :api_key, :scope
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/audit_policy_test.rb b/test/policies/admin/audit_policy_test.rb
new file mode 100644
index 00000000000..867c11c8202
--- /dev/null
+++ b/test/policies/admin/audit_policy_test.rb
@@ -0,0 +1,27 @@
+require "test_helper"
+
+class Admin::AuditPolicyTest < AdminPolicyTestCase
+  setup do
+    @audit = create(:audit)
+    @admin = create(:admin_github_user, :is_admin)
+    @non_admin = create(:admin_github_user)
+  end
+
+  def test_scope
+    assert_equal [@audit], policy_scope!(
+      @admin,
+      Audit
+    ).to_a
+
+    assert_empty policy_scope!(
+      @non_admin,
+      Audit
+    ).to_a
+  end
+
+  def test_avo_show
+    assert_authorizes @admin, @audit, :avo_show?
+
+    refute_authorizes @non_admin, @audit, :avo_show?
+  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/dependency_policy_test.rb b/test/policies/admin/dependency_policy_test.rb
new file mode 100644
index 00000000000..5aed69e5958
--- /dev/null
+++ b/test/policies/admin/dependency_policy_test.rb
@@ -0,0 +1,22 @@
+require "test_helper"
+
+class Admin::DependencyPolicyTest < AdminPolicyTestCase
+  setup do
+    @dependency = create(:dependency)
+    @admin = create(:admin_github_user, :is_admin)
+    @non_admin = create(:admin_github_user)
+  end
+
+  def test_scope
+    assert_equal [@dependency], policy_scope!(
+      @admin,
+      Dependency
+    ).to_a
+  end
+
+  def test_avo_show
+    assert_authorizes @admin, @dependency, :avo_show?
+
+    refute_authorizes @non_admin, @dependency, :avo_show?
+  end
+end
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
new file mode 100644
index 00000000000..74f5dcc7b3c
--- /dev/null
+++ b/test/policies/admin/ip_address_policy_test.rb
@@ -0,0 +1,34 @@
+require "test_helper"
+
+class Admin::IpAddressPolicyTest < AdminPolicyTestCase
+  setup do
+    @ip_address = create(:ip_address)
+    @admin = create(:admin_github_user, :is_admin)
+    @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,
+      IpAddress
+    ).to_a
+  end
+
+  def test_avo_index
+    assert_authorizes @admin, @ip_address, :avo_index?
+
+    refute_authorizes @non_admin, @ip_address, :avo_index?
+  end
+
+  def test_avo_show
+    assert_authorizes @admin, @ip_address, :avo_show?
+
+    refute_authorizes @non_admin, @ip_address, :avo_show?
+  end
+end
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..063481c94be
--- /dev/null
+++ b/test/policies/admin/nil_class_policy_test.rb
@@ -0,0 +1,27 @@
+require "test_helper"
+
+class Admin::NilClassPolicyTest < AdminPolicyTestCase
+  def policy!(api_key)
+    Pundit.policy!(api_key, [:api, nil])
+  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/organization_onboarding_invite_policy_test.rb b/test/policies/admin/organization_onboarding_invite_policy_test.rb
index 164c423fc36..7d8b9e6302e 100644
--- a/test/policies/admin/organization_onboarding_invite_policy_test.rb
+++ b/test/policies/admin/organization_onboarding_invite_policy_test.rb
@@ -35,4 +35,9 @@ def test_avo_destroy
     refute_authorizes @admin, @onboarding_invite, :avo_destroy?
     refute_authorizes @non_admin, @onboarding_invite, :avo_destroy?
   end
+
+  def test_act_on
+    assert_authorizes @admin, @onboarding_invite, :act_on?
+    refute_authorizes @non_admin, @onboarding_invite, :act_on?
+  end
 end
diff --git a/test/policies/admin/organization_onboarding_policy_test.rb b/test/policies/admin/organization_onboarding_policy_test.rb
index f708c49f43e..7f08b217e4d 100644
--- a/test/policies/admin/organization_onboarding_policy_test.rb
+++ b/test/policies/admin/organization_onboarding_policy_test.rb
@@ -35,4 +35,9 @@ def test_avo_destroy
     refute_authorizes @admin, @onboarding, :avo_destroy?
     refute_authorizes @non_admin, @onboarding, :avo_destroy?
   end
+
+  def test_act_on
+    assert_authorizes @admin, @onboarding, :act_on?
+    refute_authorizes @non_admin, @onboarding, :act_on?
+  end
 end
diff --git a/test/policies/admin/organization_policy_test.rb b/test/policies/admin/organization_policy_test.rb
index 2bda6ccfc65..a1874980ff2 100644
--- a/test/policies/admin/organization_policy_test.rb
+++ b/test/policies/admin/organization_policy_test.rb
@@ -35,4 +35,9 @@ def test_search
     assert_authorizes @admin, @organization, :avo_search?
     refute_authorizes @non_admin, @organization, :avo_search?
   end
+
+  def test_act_on
+    assert_authorizes @admin, @organization, :act_on?
+    refute_authorizes @non_admin, @organization, :act_on?
+  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