From a3dbe77af9dbd513bb4ccd850a46a7be2e071e47 Mon Sep 17 00:00:00 2001 From: Chris Colvard Date: Fri, 28 Jan 2022 14:08:31 -0500 Subject: [PATCH] Delete access control object when destroying a resource --- .rubocop_fixme.yml | 1 + app/services/hyrax/access_control_list.rb | 13 ++++ lib/hyrax/transactions/admin_set_destroy.rb | 3 +- lib/hyrax/transactions/container.rb | 13 ++++ .../steps/delete_access_control.rb | 32 ++++++++++ lib/hyrax/transactions/work_destroy.rb | 3 +- .../steps/delete_access_control_spec.rb | 63 +++++++++++++++++++ .../hyrax/access_control_list_spec.rb | 21 +++++++ 8 files changed, 147 insertions(+), 2 deletions(-) create mode 100644 lib/hyrax/transactions/steps/delete_access_control.rb create mode 100644 spec/hyrax/transactions/steps/delete_access_control_spec.rb diff --git a/.rubocop_fixme.yml b/.rubocop_fixme.yml index cf7f0b930c..06063becef 100644 --- a/.rubocop_fixme.yml +++ b/.rubocop_fixme.yml @@ -114,6 +114,7 @@ RSpec/AnyInstance: - 'spec/controllers/hyrax/batch_edits_controller_spec.rb' - 'spec/controllers/hyrax/stats_controller_spec.rb' - 'spec/controllers/hyrax/users_controller_spec.rb' + - 'spec/hyrax/transactions/steps/delete_access_control_spec.rb' - 'spec/jobs/content_restored_version_event_job_spec.rb' - 'spec/jobs/file_set_attached_event_job_spec.rb' - 'spec/jobs/hyrax/grant_edit_to_members_job_spec.rb' diff --git a/app/services/hyrax/access_control_list.rb b/app/services/hyrax/access_control_list.rb index 615141290d..09c39bec78 100644 --- a/app/services/hyrax/access_control_list.rb +++ b/app/services/hyrax/access_control_list.rb @@ -174,6 +174,19 @@ def save true end + ## + # @api public + # + # Deletes the ACL for the resource + # + # @return [Boolean] + def destroy + persister.delete(resource: change_set.resource) if change_set.resource.persisted? + @change_set = nil + + true + end + private ## diff --git a/lib/hyrax/transactions/admin_set_destroy.rb b/lib/hyrax/transactions/admin_set_destroy.rb index 0953498aca..06f031dbc5 100644 --- a/lib/hyrax/transactions/admin_set_destroy.rb +++ b/lib/hyrax/transactions/admin_set_destroy.rb @@ -9,7 +9,8 @@ module Transactions # @since 3.4.0 class AdminSetDestroy < Transaction DEFAULT_STEPS = ['admin_set_resource.check_empty', - 'admin_set_resource.delete'].freeze + 'admin_set_resource.delete', + 'admin_set_resource.delete_acl'].freeze ## # @see Hyrax::Transactions::Transaction diff --git a/lib/hyrax/transactions/container.rb b/lib/hyrax/transactions/container.rb index d0e93c3ff8..49124d1df4 100644 --- a/lib/hyrax/transactions/container.rb +++ b/lib/hyrax/transactions/container.rb @@ -38,6 +38,7 @@ class Container # rubocop:disable Metrics/ClassLength require 'hyrax/transactions/steps/apply_permission_template' require 'hyrax/transactions/steps/apply_visibility' require 'hyrax/transactions/steps/check_for_empty_admin_set' + require 'hyrax/transactions/steps/delete_access_control' require 'hyrax/transactions/steps/delete_resource' require 'hyrax/transactions/steps/destroy_work' require 'hyrax/transactions/steps/ensure_admin_set' @@ -159,6 +160,10 @@ class Container # rubocop:disable Metrics/ClassLength Steps::ApplyCollectionTypePermissions.new end + ops.register 'delete_acl' do + Steps::DeleteAccessControl.new + end + ops.register 'save_acl' do Steps::SaveAccessControl.new end @@ -169,6 +174,10 @@ class Container # rubocop:disable Metrics/ClassLength Steps::ApplyCollectionTypePermissions.new end + ops.register 'delete_acl' do + Steps::DeleteAccessControl.new + end + ops.register 'save_acl' do Steps::SaveAccessControl.new end @@ -191,6 +200,10 @@ class Container # rubocop:disable Metrics/ClassLength WorkDestroy.new end + ops.register 'delete_acl' do + Steps::DeleteAccessControl.new + end + ops.register 'save_acl' do Steps::SaveAccessControl.new end diff --git a/lib/hyrax/transactions/steps/delete_access_control.rb b/lib/hyrax/transactions/steps/delete_access_control.rb new file mode 100644 index 0000000000..ccca29da0d --- /dev/null +++ b/lib/hyrax/transactions/steps/delete_access_control.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true +require 'dry/monads' + +module Hyrax + module Transactions + module Steps + ## + # Deletes the Hyrax::AccessControlList for any resource with a `#permission_manager`. + # If `#permission_manager` is undefined, succeeds. + # + # @see https://dry-rb.org/gems/dry-monads/1.0/result/ + class DeleteAccessControl + include Dry::Monads[:result] + + ## + # @param [Valkyrie::Resource] obj + # + # @return [Dry::Monads::Result] + def call(obj) + return Success(obj) unless obj.respond_to?(:permission_manager) + + acl = obj.permission_manager&.acl + return Success(obj) if acl.nil? + + acl.destroy || (return Failure[:failed_to_delete_acl, acl]) + + Success(obj) + end + end + end + end +end diff --git a/lib/hyrax/transactions/work_destroy.rb b/lib/hyrax/transactions/work_destroy.rb index 26cff0d3ff..06501dd7cc 100644 --- a/lib/hyrax/transactions/work_destroy.rb +++ b/lib/hyrax/transactions/work_destroy.rb @@ -8,7 +8,8 @@ module Transactions # # @since 3.0.0 class WorkDestroy < Transaction - DEFAULT_STEPS = ['work_resource.delete'].freeze + DEFAULT_STEPS = ['work_resource.delete', + 'work_resource.delete_acl'].freeze ## # @see Hyrax::Transactions::Transaction diff --git a/spec/hyrax/transactions/steps/delete_access_control_spec.rb b/spec/hyrax/transactions/steps/delete_access_control_spec.rb new file mode 100644 index 0000000000..81f9cd9bbb --- /dev/null +++ b/spec/hyrax/transactions/steps/delete_access_control_spec.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true +require 'spec_helper' +require 'dry/monads' +require 'hyrax/transactions' + +RSpec.describe Hyrax::Transactions::Steps::DeleteAccessControl, valkyrie_adapter: :test_adapter do + subject(:step) { described_class.new } + let(:work) { FactoryBot.valkyrie_create(:hyrax_work) } + + context 'when acl has not been persisted' do + it 'gives Success(obj) in basic case' do + expect(step.call(work).value!).to eql(work) + end + end + + context 'when existing permissions exist' do + let(:user) { FactoryBot.create(:user) } + + before do + work.permission_manager.read_users = [user.user_key] + work.permission_manager.acl.save + end + + it 'deletes the access control resource' do + expect { step.call(work) } + .to change { Hyrax::AccessControl.for(resource: work).persisted? } + .from(true) + .to(false) + end + + context 'when it fails to destroy' do + before { allow_any_instance_of(Hyrax::AccessControlList).to receive(:destroy).and_return(false) } + + it 'returns a Failure' do + result = step.call(work) + + expect(result).to be_failure + expect(result.failure).to contain_exactly(Symbol, Hyrax::AccessControlList) + end + end + end + + context 'when the resource has no permission_manager' do + before do + module Hyrax + module Test + module DeleteAccessControlStep + class SimpleResource < Valkyrie::Resource + end + end + end + end + end + + after { Hyrax::Test.send(:remove_const, :DeleteAccessControlStep) } + + let(:resource) { Hyrax.persister.save(resource: Hyrax::Test::DeleteAccessControlStep::SimpleResource.new) } + + it 'succeeds happily' do + expect(step.call(work).value!).to eql(work) + end + end +end diff --git a/spec/services/hyrax/access_control_list_spec.rb b/spec/services/hyrax/access_control_list_spec.rb index 488311f12d..62e2ff6259 100644 --- a/spec/services/hyrax/access_control_list_spec.rb +++ b/spec/services/hyrax/access_control_list_spec.rb @@ -164,4 +164,25 @@ end end end + + describe "#destroy" do + let(:listener) { Hyrax::Specs::SpyListener.new } + + before do + acl << permission + acl.save + + # Subscribe to events after acl has been persisted + Hyrax.publisher.subscribe(listener) + end + + after { Hyrax.publisher.unsubscribe(listener) } + + it 'deletes the acl resource' do + expect { acl.destroy } + .to change { Hyrax::AccessControl.for(resource: resource, query_service: acl.query_service).persisted? } + .from(true) + .to(false) + end + end end