Skip to content

Commit

Permalink
Merge pull request #269 from projecthydra/no_index_removed_permissions
Browse files Browse the repository at this point in the history 
Don't index permissions marked for destruction
  • Loading branch information
@mjgiarlo
mjgiarlo committed Jul 8, 2015
2 parents 68bc139 + f665d95 commit f8d07dd
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 54 deletions.
13 changes: 11 additions & 2 deletions hydra-access-controls/app/models/concerns/hydra/access_controls/permissions.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -392,9 +392,18 @@ def search_by_type(type)
def search_by_type_and_mode(type, mode)
case type
when :group
permissions.to_a.select { |p| group_agent?(p.agent) && p.mode.first.rdf_subject == mode }
search_by_mode(mode) { |agent| group_agent?(agent) }
when :person
permissions.to_a.select { |p| person_agent?(p.agent) && p.mode.first.rdf_subject == mode }
search_by_mode(mode) { |agent| person_agent?(agent) }
end
end

# @param [RDF::URI] mode One of the permissions modes, e.g. ACL.Write, ACL.Read, etc.
# @yieldparam [Array<ActiveFedora::Base>] agent the agent type assertions
# @return [Array<Permission>] list of permissions where the mode is as selected, the block evaluates to true and the target is not marked for delete
def search_by_mode(mode, &block)
permissions.to_a.select do |p|
yield(p.agent) && !p.marked_for_destruction? && p.mode.first.rdf_subject == mode
end
end

Expand Down
129 changes: 77 additions & 52 deletions hydra-access-controls/spec/unit/permissions_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ class Foo < ActiveFedora::Base
subject.read_groups=['group1', 'group2']
subject.edit_users=['user1']
subject.read_users=['user2', 'user3']
expect(subject.permissions).to match_array [Hydra::AccessControls::Permission.new(:type=>"group", :access=>"read", :name=>"group1"),
Hydra::AccessControls::Permission.new({:type=>"group", :access=>"read", :name=>"group2"}),
Hydra::AccessControls::Permission.new({:type=>"person", :access=>"read", :name=>"user2"}),
Hydra::AccessControls::Permission.new({:type=>"person", :access=>"read", :name=>"user3"}),
Hydra::AccessControls::Permission.new({:type=>"person", :access=>"edit", :name=>"user1"})]
expect(subject.permissions).to match_array [Hydra::AccessControls::Permission.new(type: "group", access: "read", name: "group1"),
Hydra::AccessControls::Permission.new({ type: "group", access: "read", name: "group2" }),
Hydra::AccessControls::Permission.new({ type: "person", access: "read", name: "user2" }),
Hydra::AccessControls::Permission.new({ type: "person", access: "read", name: "user3" }),
Hydra::AccessControls::Permission.new({ type: "person", access: "edit", name: "user1" })]
end

describe "building a new permission" do
Expand All @@ -40,103 +40,128 @@ class Foo < ActiveFedora::Base
describe "with nested attributes" do
before do
subject.save!
subject.permissions_attributes = [{:type=>"person", :access=>"edit", :name=>"jcoyne"}]
subject.permissions_attributes = [{ type: "person", access: "edit", name: "jcoyne" }]
end
context "when a hash is passed" do
before do
subject.permissions_attributes = {'0' => {type: "group", access:"read", name:"group1"},
'1' => {type: 'person', access: 'edit', name: 'user2'}}
subject.permissions_attributes = {'0' => { type: "group", access:"read", name:"group1" },
'1' => { type: 'person', access: 'edit', name: 'user2' }}
end
it "should handle a hash" do
expect(subject.permissions.size).to eq 3
expect(subject.permissions.to_a).to all(be_a(Hydra::AccessControls::Permission))
expect(subject.permissions.map(&:to_hash)).to match_array [
{type: "person", access: "edit", name: "jcoyne"},
{type: "group", access: "read", name: "group1"},
{type: "person", access: "edit", name: "user2"}]
{ type: "person", access: "edit", name: "jcoyne" },
{ type: "group", access: "read", name: "group1" },
{ type: "person", access: "edit", name: "user2" }]
end
end

it "should create new group permissions" do
subject.permissions_attributes = [{type: "group", access: "read", name: "group1"}]
it "creates new group permissions" do
subject.permissions_attributes = [{ type: "group", access: "read", name: "group1" }]
expect(subject.permissions.size).to eq 2
expect(subject.permissions.to_a).to all(be_a(Hydra::AccessControls::Permission))
expect(subject.permissions[0].to_hash).to eq(type: "person", access: "edit", name: "jcoyne")
expect(subject.permissions[1].to_hash).to eq(type: "group", access: "read", name: "group1")
end

it "should create new user permissions" do
subject.permissions_attributes = [{:type=>"person", :access=>"read", :name=>"user1"}]
it "creates new user permissions" do
subject.permissions_attributes = [{ type: "person", access: "read", name: "user1" }]
expect(subject.permissions.size).to eq 2
expect(subject.permissions.to_a).to all(be_a(Hydra::AccessControls::Permission))
expect(subject.permissions[0].to_hash).to eq(type: "person", access: "edit", name: "jcoyne")
expect(subject.permissions[1].to_hash).to eq(type: "person", access: "read", name: "user1")
end

context "when called multiple times" do
it "should not replace existing groups" do
subject.permissions_attributes = [{:type=>"group", :access=>"read", :name=>"group1"}]
subject.permissions_attributes = [{:type=>"group", :access=>"read", :name=>"group2"}]
it "doesn't replace existing groups" do
subject.permissions_attributes = [{ type: "group", access: "read", name: "group1" }]
subject.permissions_attributes = [{ type: "group", access: "read", name: "group2" }]
expect(subject.permissions.size).to eq 3
expect(subject.permissions.to_a).to all(be_a(Hydra::AccessControls::Permission))
expect(subject.permissions[0].to_hash).to eq(type: "person", access: "edit", name: "jcoyne")
expect(subject.permissions[1].to_hash).to eq(type: "group", access: "read", name: "group1")
expect(subject.permissions[2].to_hash).to eq(type: "group", access: "read", name: "group2")
end

it "should not replace existing users" do
subject.permissions_attributes = [{:type=>"person", :access=>"read", :name=>"user1"}]
subject.permissions_attributes = [{:type=>"person", :access=>"read", :name=>"user2"}]
it "doesn't replace existing users" do
subject.permissions_attributes = [{ type: "person", access: "read", name: "user1" }]
subject.permissions_attributes = [{ type: "person", access: "read", name: "user2" }]
expect(subject.permissions.size).to eq 3
expect(subject.permissions.to_a).to all(be_a(Hydra::AccessControls::Permission))
expect(subject.permissions[0].to_hash).to eq(type: "person", access: "edit", name: "jcoyne")
expect(subject.permissions[1].to_hash).to eq(type: "person", access: "read", name: "user1")
expect(subject.permissions[2].to_hash).to eq(type: "person", access: "read", name: "user2")
end

it "should update permissions on existing users" do
subject.update permissions_attributes: [{:type=>"person", :access=>"read", :name=>"user1"}]
subject.update permissions_attributes: [{:type=>"person", :access=>"edit", :name=>"user1"}]
it "updates permissions on existing users" do
subject.update permissions_attributes: [{ type: "person", access: "read", name: "user1" }]
subject.update permissions_attributes: [{ type: "person", access: "edit", name: "user1" }]
expect(subject.permissions.size).to eq 2
expect(subject.permissions.to_a).to all(be_a(Hydra::AccessControls::Permission))
expect(subject.permissions[0].to_hash).to eq(type: "person", access: "edit", name: "jcoyne")
expect(subject.permissions[1].to_hash).to eq(type: "person", access: "edit", name: "user1")
end

it "should update permissions on existing groups" do
subject.update permissions_attributes: [{:type=>"group", :access=>"read", :name=>"group1"}]
subject.update permissions_attributes: [{:type=>"group", :access=>"edit", :name=>"group1"}]
it "updates permissions on existing groups" do
subject.update permissions_attributes: [{ type: "group", access: "read", name: "group1" }]
subject.update permissions_attributes: [{ type: "group", access: "edit", name: "group1" }]
expect(subject.permissions.map(&:to_hash)).to match_array [
{:type=>"group", :access=>"edit", :name=>"group1"},
{:type=>"person", :access=>"edit", :name=>"jcoyne"}]
{ type: "group", access: "edit", name: "group1" },
{ type: "person", access: "edit", name: "jcoyne" }]
end
end

it "should remove permissions on existing users" do
subject.update permissions_attributes: [{:type=>"person", :access=>"read", :name=>"user1"}]
subject.update permissions_attributes: [{:id=>ActiveFedora::Base.uri_to_id(subject.permissions.last.rdf_subject.to_s), :type=>"person", :access=>"edit", :name=>"user1", _destroy: true}]
expect(subject.permissions.reload.map(&:to_hash)).to eq [{ :name=>"jcoyne", :type=>"person", :access=>"edit" }]
end
context "when the destroy flag is set" do
let(:reloaded) { subject.permissions.reload.map(&:to_hash) }
let(:permissions_id) { ActiveFedora::Base.uri_to_id(subject.permissions.last.rdf_subject.to_s) }

context "to a truthy value" do
context "when updating users" do
before do
subject.update permissions_attributes: [{ type: "person", access: "read", name: "user1" }]
subject.update permissions_attributes: [{ id: permissions_id, type: "person", access: "edit", name: "user1", _destroy: true}]
end

it "removes permissions on existing users" do
indexed_result = ActiveFedora::SolrService.query("id:#{subject.id}").first['edit_access_person_ssim']
expect(indexed_result).to eq ['jcoyne']
expect(reloaded).to eq [{ name: "jcoyne", type: "person", access: "edit" }]
end
end

context "when updating groups" do
before do
subject.update permissions_attributes: [{ type: "group", access: "read", name: "group1" }]
subject.update permissions_attributes: [{ id: permissions_id, type: "group", access: "edit", name: "group1", _destroy: '1' }]
end

it "removes permissions on existing groups" do
#See what actually gets stored in solr
indexed_result = ActiveFedora::SolrService.query("id:#{subject.id}").first['edit_access_group_ssim']
expect(indexed_result).to be_nil
expect(reloaded).to eq [{ type: "person", access: "edit", name: "jcoyne" }]
end
end
end

it "should remove permissions on existing groups" do
subject.update permissions_attributes: [{:type=>"group", :access=>"read", :name=>"group1"}]
subject.update permissions_attributes: [{:id=>ActiveFedora::Base.uri_to_id(subject.permissions.last.rdf_subject.to_s), :type=>"group", :access=>"edit", :name=>"group1", _destroy: '1'}]
expect(subject.permissions.reload.map(&:to_hash)).to eq [{:type=>"person", :access=>"edit", :name=>"jcoyne"}]
end
context "to a falsy value" do

it "should not remove when destroy flag is falsy" do
subject.update permissions_attributes: [{:type=>"group", :access=>"read", :name=>"group1"}]
subject.update permissions_attributes: [{:id=>ActiveFedora::Base.uri_to_id(subject.permissions.last.rdf_subject.to_s), :type=>"group", :access=>"edit", :name=>"group1", _destroy: '0'}]
expect(subject.permissions.reload.map(&:to_hash)).to match_array [{:type=>"group", :access=>"edit", :name=>"group1"},
{:type=>"person", :access=>"edit", :name=>"jcoyne"}]
it "doesn't remove the record" do
subject.update permissions_attributes: [{ type: "group", access: "read", name: "group1" }]
subject.update permissions_attributes: [{ id: permissions_id, type: "group", access: "edit", name: "group1", _destroy: '0' }]
expect(reloaded).to match_array [{ type: "group", access: "edit", name: "group1" },
{ type: "person", access: "edit", name: "jcoyne" }]
end
end
end
end

describe "with the setter" do
before do
subject.permissions = [
Hydra::AccessControls::Permission.new(:type=>"group", :access=>"edit", :name=>"group1"),
Hydra::AccessControls::Permission.new(:type=>"person", :access=>"edit", :name=>"jcoyne")]
Hydra::AccessControls::Permission.new(type: "group", access: "edit", name: "group1"),
Hydra::AccessControls::Permission.new(type: "person", access: "edit", name: "jcoyne")]
subject.save!
end
it "should set the permissions" do
Expand Down Expand Up @@ -177,12 +202,12 @@ class Foo < ActiveFedora::Base
subject.set_read_groups(['group-2', 'group-3'], ['group-6'])
# 'group-7' is not eligible to be revoked
expect(subject.permissions.map(&:to_hash)).to match_array([
{name: 'group-2', type: 'group', access: 'read'},
{name: 'group-3', type: 'group', access: 'read'},
{name: 'group-7', type: 'group', access: 'read'},
{name: 'group-8', type: 'group', access: 'edit'},
{name: 'person1', type: 'person', access: 'read'},
{name: 'person2', type: 'person', access: 'discover'}])
{ name: 'group-2', type: 'group', access: 'read' },
{ name: 'group-3', type: 'group', access: 'read' },
{ name: 'group-7', type: 'group', access: 'read' },
{ name: 'group-8', type: 'group', access: 'edit' },
{ name: 'person1', type: 'person', access: 'read' },
{ name: 'person2', type: 'person', access: 'discover' }])
end
end
end

0 comments on commit f8d07dd

Please sign in to comment.