From fa47985a965003aaf5de769e76ab3aed29c77984 Mon Sep 17 00:00:00 2001 From: Martin Emde Date: Wed, 20 Nov 2024 12:33:13 -0800 Subject: [PATCH 1/5] Remove unused stubs from ApplicationPolicy --- app/policies/application_policy.rb | 32 ------------------------------ 1 file changed, 32 deletions(-) diff --git a/app/policies/application_policy.rb b/app/policies/application_policy.rb index 45051f2f443..499d99b6c0b 100644 --- a/app/policies/application_policy.rb +++ b/app/policies/application_policy.rb @@ -26,38 +26,6 @@ def initialize(user, record) @error = nil end - def index? - false - end - - def show? - false - end - - def create? - false - end - - def new? - create? - end - - def update? - false - end - - def edit? - update? - end - - def destroy? - false - end - - def search? - index? - end - private delegate :t, to: I18n From d5c27a995a26e544d72895f0aa790cf19bec2014 Mon Sep 17 00:00:00 2001 From: Martin Emde Date: Wed, 20 Nov 2024 12:46:35 -0800 Subject: [PATCH 2/5] Fix membership policy tests --- test/policies/membership_policy_test.rb | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/test/policies/membership_policy_test.rb b/test/policies/membership_policy_test.rb index cc3e3465a3c..39a383f707e 100644 --- a/test/policies/membership_policy_test.rb +++ b/test/policies/membership_policy_test.rb @@ -96,26 +96,24 @@ def policy!(user, record = Membership) context "removing owner" do should "be authorized for org owners only" do membership = create(:membership, :owner, organization: @organization) - membership.role = :admin - assert_authorized policy!(@owner, membership), :update? + assert_authorized policy!(@owner, membership), :destroy? - refute_authorized policy!(@admin, membership), :update? - refute_authorized policy!(@maintainer, membership), :update? - refute_authorized policy!(@guest, membership), :update? + refute_authorized policy!(@admin, membership), :destroy? + refute_authorized policy!(@maintainer, membership), :destroy? + refute_authorized policy!(@guest, membership), :destroy? end end context "removing admin" do should "be authorized for org admins and owners" do membership = create(:membership, :admin, organization: @organization) - membership.role = :maintainer - assert_authorized policy!(@owner, membership), :update? - assert_authorized policy!(@admin, membership), :update? + assert_authorized policy!(@owner, membership), :destroy? + assert_authorized policy!(@admin, membership), :destroy? - refute_authorized policy!(@maintainer, membership), :update? - refute_authorized policy!(@guest, membership), :update? + refute_authorized policy!(@maintainer, membership), :destroy? + refute_authorized policy!(@guest, membership), :destroy? end end end From 6d396ba1cd3e0376d5fd45ffb7063ce838610044 Mon Sep 17 00:00:00 2001 From: Martin Emde Date: Wed, 20 Nov 2024 12:49:55 -0800 Subject: [PATCH 3/5] Remove unused RubygemPolicy predicates, update tests --- app/policies/rubygem_policy.rb | 12 ------------ test/policies/rubygem_policy_test.rb | 8 ++++++++ 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/app/policies/rubygem_policy.rb b/app/policies/rubygem_policy.rb index 41d761d014c..8c23df1c0d0 100644 --- a/app/policies/rubygem_policy.rb +++ b/app/policies/rubygem_policy.rb @@ -8,22 +8,10 @@ class Scope < ApplicationPolicy::Scope alias rubygem record delegate :organization, to: :rubygem - def show? - true - end - def create? user.present? end - def update? - false - end - - def destroy? - false - end - def configure_oidc? rubygem_owned_by_with_role?(user, minimum_required_role: :owner, minimum_required_org_role: :admin) end diff --git a/test/policies/rubygem_policy_test.rb b/test/policies/rubygem_policy_test.rb index 4979fa27697..505dc93a4fd 100644 --- a/test/policies/rubygem_policy_test.rb +++ b/test/policies/rubygem_policy_test.rb @@ -33,6 +33,14 @@ def org_policy!(user) Pundit.policy!(user, @org_rubygem) end + context "#create?" do + should "allow users" do + assert_authorized policy!(@owner), :create? + assert_authorized policy!(@user), :create? + refute_authorized policy!(nil), :create? + end + end + context "#configure_oidc?" do should "only allow the owner" do assert_authorized policy!(@owner), :configure_oidc? From f350ef861f14e10dad207c726ba7a07556f38126 Mon Sep 17 00:00:00 2001 From: Martin Emde Date: Wed, 20 Nov 2024 12:51:20 -0800 Subject: [PATCH 4/5] Remove unused resolve not implemented, rely instead on NoMethodError --- app/policies/api/application_policy.rb | 4 ---- app/policies/application_policy.rb | 4 ---- 2 files changed, 8 deletions(-) diff --git a/app/policies/api/application_policy.rb b/app/policies/api/application_policy.rb index 3d7d9b7f873..15b57a4bb8e 100644 --- a/app/policies/api/application_policy.rb +++ b/app/policies/api/application_policy.rb @@ -7,10 +7,6 @@ def initialize(api_key, scope) @scope = scope end - def resolve - raise NotImplementedError, "You must define #resolve in #{self.class}" - end - private attr_reader :api_key, :scope diff --git a/app/policies/application_policy.rb b/app/policies/application_policy.rb index 499d99b6c0b..d7bf716f226 100644 --- a/app/policies/application_policy.rb +++ b/app/policies/application_policy.rb @@ -9,10 +9,6 @@ def initialize(user, scope) @scope = scope end - def resolve - raise NotImplementedError, "You must define #resolve in #{self.class}" - end - private attr_reader :user, :scope From 779a8ab184c669c1b0bfed85707615738f9a13a8 Mon Sep 17 00:00:00 2001 From: Martin Emde Date: Wed, 20 Nov 2024 12:55:44 -0800 Subject: [PATCH 5/5] Add NilClassPolicy tests --- app/policies/api/nil_class_policy.rb | 2 +- test/policies/api/nil_class_policy_test.rb | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 test/policies/api/nil_class_policy_test.rb diff --git a/app/policies/api/nil_class_policy.rb b/app/policies/api/nil_class_policy.rb index 820bb9d6e89..f9718480442 100644 --- a/app/policies/api/nil_class_policy.rb +++ b/app/policies/api/nil_class_policy.rb @@ -6,6 +6,6 @@ def resolve end def destroy? - false + deny t(:forbidden) end end diff --git a/test/policies/api/nil_class_policy_test.rb b/test/policies/api/nil_class_policy_test.rb new file mode 100644 index 00000000000..3472f8c9147 --- /dev/null +++ b/test/policies/api/nil_class_policy_test.rb @@ -0,0 +1,21 @@ +require "test_helper" + +class Api::NilClassPolicyTest < ApiPolicyTestCase + def policy!(api_key) + Pundit.policy!(api_key, [:api, nil]) + end + + context "::Scope.resolve" do + should "raise" do + assert_raises Pundit::NotDefinedError do + Api::NilClassPolicy::Scope.new(nil, nil).resolve + end + end + end + + context "#destroy?" do + should "not be authorized" do + refute_authorized policy!(nil), :destroy?, "Forbidden" + end + end +end