From 3882b595e31e0288e4cba1f4b61c5fa20d4892e4 Mon Sep 17 00:00:00 2001 From: Eddie Rubeiz Date: Tue, 6 Dec 2022 16:59:37 -0500 Subject: [PATCH 1/3] addign tests for our access policy --- spec/policies/access_policy_spec.rb | 134 ++++++++++++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 spec/policies/access_policy_spec.rb diff --git a/spec/policies/access_policy_spec.rb b/spec/policies/access_policy_spec.rb new file mode 100644 index 000000000..ea9523c59 --- /dev/null +++ b/spec/policies/access_policy_spec.rb @@ -0,0 +1,134 @@ +require 'rails_helper' + +describe "policies" do + + let!(:admin_user) { FactoryBot.create(:admin_user, email: "admin@b.c") } + let!(:staff_user) { FactoryBot.create(:user, email: "staff@b.c") } + + let!(:admin_policy) { AccessPolicy.new(admin_user) } + let!(:staff_policy) { AccessPolicy.new(staff_user) } + let!(:public_policy) { AccessPolicy.new(nil) } + + let!(:published_asset) { create(:asset_with_faked_file, published: true) } + let!(:unpublished_asset) { create(:asset_with_faked_file, published: false) } + let!(:collection) { create(:collection) } + + describe 'admin' do + let(:policy) { admin_policy } + let(:user) { admin_user } + + it "is in fact an admin user" do + expect(user.admin_user?).to be true + end + it "is not an editor user" do + expect(user.editor_user?).to be false + end + it "can read an unpublished asset" do + expect(policy.can?(:read, unpublished_asset)).to be true + end + it "can destroy a particular collection" do + expect(policy.can?(:destroy, collection)).to be true + end + it "can destroy any Collection" do + expect(policy.can?(:destroy, Collection)).to be true + end + it "can manage users" do + expect(policy.can?(:admin, User)).to be true + end + it "can access staff functions" do + expect(policy.can?(:access_staff_functions)).to be true + end + + it "can create a new collection" do + skip "This isn't explicitly stated in the policy description for :admin" + expect(policy.can?(:create, Collection)).to be true + end + it "can create a new work" do + skip "This isn't explicitly stated in the policy description for :admin" + expect(policy.can?(:create, Work)).to be true + end + it "can create a new asset" do + skip "This isn't explicitly stated in the policy description for :admin" + expect(policy.can?(:create, Asset)).to be true + end + it "can read a Kithe::Model" do + skip "Kithe::Model isn't explicitly stated in the policy description for :admin" + expect(policy.can?(:read, Kithe::Model)).to be false + end + it "can update a Kithe::Model" do + skip "Kithe::Model isn't explicitly stated in the policy description for :admin" + expect(policy.can?(:update, Kithe::Model)).to be false + end + + end + + describe 'staff' do + let(:policy) { staff_policy } + let(:user) { staff_user } + + it "user is not an admin user" do + expect(user.admin_user?).to be false + end + it "user is an editor user" do + expect(user.editor_user?).to be true + end + it "cannot create a new collection" do + expect(policy.can?(:create, Collection)).to be false + end + it "cannot create a new work" do + expect(policy.can?(:create, Work)).to be false + end + it "cannot create a new asset" do + expect(policy.can?(:create, Asset)).to be false + end + it "can update a Kithe::Model" do + expect(policy.can?(:read, Kithe::Model)).to be true + end + it "can read a Kithe::Model" do + expect(policy.can?(:read, Kithe::Model)).to be true + end + it "can, however, read a particular unpublished asset" do + expect(policy.can?(:read, unpublished_asset)).to be true + end + it "cannot destroy a collection" do + expect(policy.can?(:destroy, collection)).to be false + end + it "cannot manage users" do + expect(policy.can?(:manage, User)).to be false + end + it "can access staff functions" do + expect(policy.can?(:access_staff_functions)).to be true + end + it "can read an Asset" do + skip "Policy mentions Kithe::Model, not its subclasses." + expect(policy.can?(:read, Asset)).to be true + end + it "can read a Work" do + skip "Policy mentions Kithe::Model, not its subclasses." + expect(policy.can?(:read, Work)).to be true + end + it "can read a Collection" do + skip "Policy mentions Kithe::Model, not its subclasses." + expect(policy.can?(:read, Collection)).to be true + end + end + + describe 'public' do + let(:policy) { public_policy } + let(:user) { staff_user } + it "cannot read an unpublished asset" do + expect(policy.can?(:read, unpublished_asset)).to be false + end + it "can read a published asset" do + expect(policy.can?(:read, published_asset)).to be true + end + it "cannot read any Kithe::Model" do + skip "`published` isn't defined on Kithe::Model so we can't use this right now" + expect(policy.can?(:read, Kithe::Model)).to be false + end + it "cannot access staff functions" do + expect(policy.can?(:access_staff_functions)).to be false + end + end + +end From 3e8ddf74bebb6541d99483265def10459b8d13e5 Mon Sep 17 00:00:00 2001 From: Eddie Rubeiz Date: Wed, 7 Dec 2022 10:19:53 -0500 Subject: [PATCH 2/3] cleaning this up and making it easier to read --- spec/policies/access_policy_spec.rb | 236 +++++++++++++++------------- 1 file changed, 131 insertions(+), 105 deletions(-) diff --git a/spec/policies/access_policy_spec.rb b/spec/policies/access_policy_spec.rb index ea9523c59..06b2dde85 100644 --- a/spec/policies/access_policy_spec.rb +++ b/spec/policies/access_policy_spec.rb @@ -1,133 +1,159 @@ require 'rails_helper' -describe "policies" do - - let!(:admin_user) { FactoryBot.create(:admin_user, email: "admin@b.c") } - let!(:staff_user) { FactoryBot.create(:user, email: "staff@b.c") } - - let!(:admin_policy) { AccessPolicy.new(admin_user) } - let!(:staff_policy) { AccessPolicy.new(staff_user) } - let!(:public_policy) { AccessPolicy.new(nil) } - +describe "access policies:" do let!(:published_asset) { create(:asset_with_faked_file, published: true) } let!(:unpublished_asset) { create(:asset_with_faked_file, published: false) } let!(:collection) { create(:collection) } describe 'admin' do - let(:policy) { admin_policy } - let(:user) { admin_user } + let(:user) { FactoryBot.create(:admin_user, email: "admin@b.c") } + let(:policy) { AccessPolicy.new(user) } + let(:comment_you_can_delete) { Admin::QueueItemComment.new(user: user)} + let(:coment_you_can_t_delete) { Admin::QueueItemComment.new(user: nil)} - it "is in fact an admin user" do - expect(user.admin_user?).to be true - end - it "is not an editor user" do - expect(user.editor_user?).to be false - end - it "can read an unpublished asset" do - expect(policy.can?(:read, unpublished_asset)).to be true - end - it "can destroy a particular collection" do - expect(policy.can?(:destroy, collection)).to be true - end - it "can destroy any Collection" do - expect(policy.can?(:destroy, Collection)).to be true - end - it "can manage users" do - expect(policy.can?(:admin, User)).to be true - end - it "can access staff functions" do - expect(policy.can?(:access_staff_functions)).to be true + context "is the right kind of user" do + it "is in fact an admin user" do + expect(user.admin_user?).to be true + end + it "is not an editor user" do + expect(user.editor_user?).to be false + end end - it "can create a new collection" do - skip "This isn't explicitly stated in the policy description for :admin" - expect(policy.can?(:create, Collection)).to be true - end - it "can create a new work" do - skip "This isn't explicitly stated in the policy description for :admin" - expect(policy.can?(:create, Work)).to be true + context "rules explicitly stated in the admin role are followed" do + it "can read an unpublished asset" do + expect(policy.can?(:read, unpublished_asset)).to be true + end + it "can destroy a particular collection" do + expect(policy.can?(:destroy, collection)).to be true + end + it "can destroy any Collection" do + expect(policy.can?(:destroy, Collection)).to be true + end + it "can manage users" do + expect(policy.can?(:admin, User)).to be true + end + it "can access staff functions" do + expect(policy.can?(:access_staff_functions)).to be true + end end - it "can create a new asset" do - skip "This isn't explicitly stated in the policy description for :admin" - expect(policy.can?(:create, Asset)).to be true - end - it "can read a Kithe::Model" do - skip "Kithe::Model isn't explicitly stated in the policy description for :admin" - expect(policy.can?(:read, Kithe::Model)).to be false + + context "checks rules defined in the editor role, which implicitly apply" do + it "can delete a QueueItemComment" do + expect(policy.can?(:destroy, comment_you_can_delete)).to be true + end + it "can't delete a QueueItemComment by someone else" do + expect(policy.can?(:destroy, coment_you_can_t_delete)).to be false + end end - it "can update a Kithe::Model" do - skip "Kithe::Model isn't explicitly stated in the policy description for :admin" - expect(policy.can?(:update, Kithe::Model)).to be false + + context "rules defined on Kithe::Model in the staff role should apply to its subclasses, and be applied to the admin role as well" do + it "can create a new collection" do + skip "This isn't explicitly stated in the policy description for :admin" + expect(policy.can?(:create, Collection)).to be true + end + it "can create a new work" do + skip "This isn't explicitly stated in the policy description for :admin" + expect(policy.can?(:create, Work)).to be true + end + it "can create a new asset" do + skip "This isn't explicitly stated in the policy description for :admin" + expect(policy.can?(:create, Asset)).to be true + end + it "can read a Kithe::Model" do + skip "Kithe::Model isn't explicitly stated in the policy description for :admin" + expect(policy.can?(:read, Kithe::Model)).to be false + end + it "can update a Kithe::Model" do + skip "Kithe::Model isn't explicitly stated in the policy description for :admin" + expect(policy.can?(:update, Kithe::Model)).to be false + end end + end describe 'staff' do - let(:policy) { staff_policy } - let(:user) { staff_user } + let(:user) { FactoryBot.create(:user, email: "staff@b.c") } + let(:policy) { AccessPolicy.new(user) } - it "user is not an admin user" do - expect(user.admin_user?).to be false - end - it "user is an editor user" do - expect(user.editor_user?).to be true + context "is the right kind of user" do + it "user is not an admin user" do + expect(user.admin_user?).to be false + end + it "user is an editor user" do + expect(user.editor_user?).to be true + end end - it "cannot create a new collection" do - expect(policy.can?(:create, Collection)).to be false - end - it "cannot create a new work" do - expect(policy.can?(:create, Work)).to be false - end - it "cannot create a new asset" do - expect(policy.can?(:create, Asset)).to be false - end - it "can update a Kithe::Model" do - expect(policy.can?(:read, Kithe::Model)).to be true - end - it "can read a Kithe::Model" do - expect(policy.can?(:read, Kithe::Model)).to be true - end - it "can, however, read a particular unpublished asset" do - expect(policy.can?(:read, unpublished_asset)).to be true - end - it "cannot destroy a collection" do - expect(policy.can?(:destroy, collection)).to be false - end - it "cannot manage users" do - expect(policy.can?(:manage, User)).to be false - end - it "can access staff functions" do - expect(policy.can?(:access_staff_functions)).to be true - end - it "can read an Asset" do - skip "Policy mentions Kithe::Model, not its subclasses." - expect(policy.can?(:read, Asset)).to be true + + context "does not allow unpermitted operations on collections, works and assets" do + it "cannot create a new collection" do + expect(policy.can?(:create, Collection)).to be false + end + it "cannot destroy a particular collection" do + expect(policy.can?(:destroy, collection)).to be false + end + it "cannot create a new work" do + expect(policy.can?(:create, Work)).to be false + end + it "cannot create a new asset" do + expect(policy.can?(:create, Asset)).to be false + end end - it "can read a Work" do - skip "Policy mentions Kithe::Model, not its subclasses." - expect(policy.can?(:read, Work)).to be true + + context "operations on Kithe::Model that are explicitly allowed" do + it "can update a Kithe::Model" do + expect(policy.can?(:read, Kithe::Model)).to be true + end + it "can read a Kithe::Model" do + expect(policy.can?(:read, Kithe::Model)).to be true + end + it "can, however, read a particular unpublished asset" do + expect(policy.can?(:read, unpublished_asset)).to be true + end + it "cannot manage users" do + expect(policy.can?(:manage, User)).to be false + end + it "can access staff functions" do + expect(policy.can?(:access_staff_functions)).to be true + end end - it "can read a Collection" do - skip "Policy mentions Kithe::Model, not its subclasses." - expect(policy.can?(:read, Collection)).to be true + + context "rules about Kithe::Model should apply to its subclasses" do + it "can read an Asset" do + skip "Policy mentions Kithe::Model, not its subclasses." + expect(policy.can?(:read, Asset)).to be true + end + it "can read a Work" do + skip "Policy mentions Kithe::Model, not its subclasses." + expect(policy.can?(:read, Work)).to be true + end + it "can read a Collection" do + skip "Policy mentions Kithe::Model, not its subclasses." + expect(policy.can?(:read, Collection)).to be true + end end end - describe 'public' do - let(:policy) { public_policy } - let(:user) { staff_user } - it "cannot read an unpublished asset" do - expect(policy.can?(:read, unpublished_asset)).to be false + describe 'non-logged-in user' do + let(:policy) { AccessPolicy.new(nil) } + context "smoke tests" do + it "can read a published asset" do + expect(policy.can?(:read, published_asset)).to be true + end + it "cannot read an unpublished asset" do + expect(policy.can?(:read, unpublished_asset)).to be false + end + it "cannot access staff functions" do + expect(policy.can?(:access_staff_functions)).to be false + end end - it "can read a published asset" do - expect(policy.can?(:read, published_asset)).to be true - end - it "cannot read any Kithe::Model" do - skip "`published` isn't defined on Kithe::Model so we can't use this right now" - expect(policy.can?(:read, Kithe::Model)).to be false - end - it "cannot access staff functions" do - expect(policy.can?(:access_staff_functions)).to be false + + context "rules about Kithe::Model should apply to its subclasses" do + it "cannot read any Kithe::Model" do + skip "`published` isn't defined on Kithe::Model so we can't use this right now" + expect(policy.can?(:read, Kithe::Model)).to be false + end end end From 164c7b220e895aaf834d94381b423201b9cebcd7 Mon Sep 17 00:00:00 2001 From: Eddie Rubeiz Date: Thu, 8 Dec 2022 12:09:21 -0500 Subject: [PATCH 3/3] amending the test subsequent to simplifying the policy --- spec/policies/access_policy_spec.rb | 185 +++++++++------------------- 1 file changed, 60 insertions(+), 125 deletions(-) diff --git a/spec/policies/access_policy_spec.rb b/spec/policies/access_policy_spec.rb index 06b2dde85..ba1c772c3 100644 --- a/spec/policies/access_policy_spec.rb +++ b/spec/policies/access_policy_spec.rb @@ -11,149 +11,84 @@ let(:comment_you_can_delete) { Admin::QueueItemComment.new(user: user)} let(:coment_you_can_t_delete) { Admin::QueueItemComment.new(user: nil)} - context "is the right kind of user" do - it "is in fact an admin user" do - expect(user.admin_user?).to be true - end - it "is not an editor user" do - expect(user.editor_user?).to be false - end + it "is in fact an admin user" do + expect(user.admin_user?).to be true end - - context "rules explicitly stated in the admin role are followed" do - it "can read an unpublished asset" do - expect(policy.can?(:read, unpublished_asset)).to be true - end - it "can destroy a particular collection" do - expect(policy.can?(:destroy, collection)).to be true - end - it "can destroy any Collection" do - expect(policy.can?(:destroy, Collection)).to be true - end - it "can manage users" do - expect(policy.can?(:admin, User)).to be true - end - it "can access staff functions" do - expect(policy.can?(:access_staff_functions)).to be true - end + it "is not an editor user" do + expect(user.editor_user?).to be false end - - context "checks rules defined in the editor role, which implicitly apply" do - it "can delete a QueueItemComment" do - expect(policy.can?(:destroy, comment_you_can_delete)).to be true - end - it "can't delete a QueueItemComment by someone else" do - expect(policy.can?(:destroy, coment_you_can_t_delete)).to be false - end + it "can read an unpublished asset" do + expect(policy.can?(:read, unpublished_asset)).to be true end - - context "rules defined on Kithe::Model in the staff role should apply to its subclasses, and be applied to the admin role as well" do - it "can create a new collection" do - skip "This isn't explicitly stated in the policy description for :admin" - expect(policy.can?(:create, Collection)).to be true - end - it "can create a new work" do - skip "This isn't explicitly stated in the policy description for :admin" - expect(policy.can?(:create, Work)).to be true - end - it "can create a new asset" do - skip "This isn't explicitly stated in the policy description for :admin" - expect(policy.can?(:create, Asset)).to be true - end - it "can read a Kithe::Model" do - skip "Kithe::Model isn't explicitly stated in the policy description for :admin" - expect(policy.can?(:read, Kithe::Model)).to be false - end - it "can update a Kithe::Model" do - skip "Kithe::Model isn't explicitly stated in the policy description for :admin" - expect(policy.can?(:update, Kithe::Model)).to be false - end + it "can destroy a particular collection" do + expect(policy.can?(:destroy, collection)).to be true + end + it "can destroy any Kithe::Model" do + expect(policy.can?(:destroy, Kithe::Model)).to be true + end + it "can manage users" do + expect(policy.can?(:admin, User)).to be true + end + it "can access staff functions" do + expect(policy.can?(:access_staff_functions)).to be true + end + it "can delete a QueueItemComment" do + expect(policy.can?(:destroy, comment_you_can_delete)).to be true + end + it "can't delete a QueueItemComment by someone else" do + expect(policy.can?(:destroy, coment_you_can_t_delete)).to be false + end + it "can read a Kithe::Model" do + expect(policy.can?(:read, Kithe::Model)).to be true + end + it "can update a Kithe::Model" do + expect(policy.can?(:update, Kithe::Model)).to be true end - - end - describe 'staff' do + describe 'editor / staff' do let(:user) { FactoryBot.create(:user, email: "staff@b.c") } let(:policy) { AccessPolicy.new(user) } - context "is the right kind of user" do - it "user is not an admin user" do - expect(user.admin_user?).to be false - end - it "user is an editor user" do - expect(user.editor_user?).to be true - end + it "user is not an admin user" do + expect(user.admin_user?).to be false end - - context "does not allow unpermitted operations on collections, works and assets" do - it "cannot create a new collection" do - expect(policy.can?(:create, Collection)).to be false - end - it "cannot destroy a particular collection" do - expect(policy.can?(:destroy, collection)).to be false - end - it "cannot create a new work" do - expect(policy.can?(:create, Work)).to be false - end - it "cannot create a new asset" do - expect(policy.can?(:create, Asset)).to be false - end + it "user is an editor user" do + expect(user.editor_user?).to be true end - - context "operations on Kithe::Model that are explicitly allowed" do - it "can update a Kithe::Model" do - expect(policy.can?(:read, Kithe::Model)).to be true - end - it "can read a Kithe::Model" do - expect(policy.can?(:read, Kithe::Model)).to be true - end - it "can, however, read a particular unpublished asset" do - expect(policy.can?(:read, unpublished_asset)).to be true - end - it "cannot manage users" do - expect(policy.can?(:manage, User)).to be false - end - it "can access staff functions" do - expect(policy.can?(:access_staff_functions)).to be true - end + it "cannot destroy a particular collection" do + expect(policy.can?(:destroy, collection)).to be false end - - context "rules about Kithe::Model should apply to its subclasses" do - it "can read an Asset" do - skip "Policy mentions Kithe::Model, not its subclasses." - expect(policy.can?(:read, Asset)).to be true - end - it "can read a Work" do - skip "Policy mentions Kithe::Model, not its subclasses." - expect(policy.can?(:read, Work)).to be true - end - it "can read a Collection" do - skip "Policy mentions Kithe::Model, not its subclasses." - expect(policy.can?(:read, Collection)).to be true - end + it "can update a Kithe::Model" do + expect(policy.can?(:update, Kithe::Model)).to be true + end + it "can read a Kithe::Model" do + expect(policy.can?(:read, Kithe::Model)).to be true + end + it "cannot destroy a Kithe::Model" do + expect(policy.can?(:destroy, Kithe::Model)).to be false + end + it "can read a particular unpublished asset" do + expect(policy.can?(:read, unpublished_asset)).to be true + end + it "cannot manage users" do + expect(policy.can?(:manage, User)).to be false + end + it "can access staff functions" do + expect(policy.can?(:access_staff_functions)).to be true end end describe 'non-logged-in user' do let(:policy) { AccessPolicy.new(nil) } - context "smoke tests" do - it "can read a published asset" do - expect(policy.can?(:read, published_asset)).to be true - end - it "cannot read an unpublished asset" do - expect(policy.can?(:read, unpublished_asset)).to be false - end - it "cannot access staff functions" do - expect(policy.can?(:access_staff_functions)).to be false - end + it "can read a published asset" do + expect(policy.can?(:read, published_asset)).to be true end - - context "rules about Kithe::Model should apply to its subclasses" do - it "cannot read any Kithe::Model" do - skip "`published` isn't defined on Kithe::Model so we can't use this right now" - expect(policy.can?(:read, Kithe::Model)).to be false - end + it "cannot read an unpublished asset" do + expect(policy.can?(:read, unpublished_asset)).to be false + end + it "cannot access staff functions" do + expect(policy.can?(:access_staff_functions)).to be false end end