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.

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

Admin policy test coverage #5262

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions app/policies/admin/audit_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
9 changes: 9 additions & 0 deletions app/policies/admin/nil_class_policy.rb
Original file line number Diff line number Diff line change
@@ -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
5 changes: 0 additions & 5 deletions app/policies/api/application_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions test/policies/admin/api_key_policy_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
15 changes: 15 additions & 0 deletions test/policies/admin/application_policy_test.rb
Original file line number Diff line number Diff line change
@@ -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
27 changes: 27 additions & 0 deletions test/policies/admin/audit_policy_test.rb
Original file line number Diff line number Diff line change
@@ -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
4 changes: 4 additions & 0 deletions test/policies/admin/deletion_policy_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
22 changes: 22 additions & 0 deletions test/policies/admin/dependency_policy_test.rb
Original file line number Diff line number Diff line change
@@ -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
4 changes: 4 additions & 0 deletions test/policies/admin/geoip_info_policy_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
34 changes: 34 additions & 0 deletions test/policies/admin/ip_address_policy_test.rb
Original file line number Diff line number Diff line change
@@ -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
27 changes: 27 additions & 0 deletions test/policies/admin/nil_class_policy_test.rb
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 5 additions & 0 deletions test/policies/admin/organization_onboarding_policy_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 5 additions & 0 deletions test/policies/admin/organization_policy_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 5 additions & 0 deletions test/policies/admin/rubygem_policy_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
38 changes: 38 additions & 0 deletions test/policies/admin/version_policy_test.rb
Original file line number Diff line number Diff line change
@@ -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
17 changes: 17 additions & 0 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Copy link
Member Author

Choose a reason for hiding this comment

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

My intention here was to use the passed policy to assert that it refers to that policy, but I'm unclear on how that works.

Is it really getting passed the "object" of the association as the record or are we just looking up the same policy as self because it's not a different record? That would imply that record is not a RubyGem in the RubyGemPolicy when we plan on checking show_association?

# assert_authorizes(user, record, :"show_#{association}?")
end

def policy_class
nil
end
Expand Down