Skip to content

Commit

Permalink
Merge pull request #4497 from samvera/#3166-lock_unauthorized_permiss…
Browse files Browse the repository at this point in the history
…ions_from_update

Lock unauthorized permissions from update
  • Loading branch information
dlpierce authored Mar 1, 2021
2 parents a06cb87 + c14aacf commit 321a771
Show file tree
Hide file tree
Showing 18 changed files with 489 additions and 111 deletions.
201 changes: 201 additions & 0 deletions app/services/hyrax/edit_permissions_service.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,201 @@
# frozen_string_literal: true
module Hyrax
# Encapsulates the logic to determine which object permissions may be edited by a given user
# - user is permitted to update any work permissions coming ONLY from collections they manage
# - user is not permitted to update a work permission if it comes from a collection they do not manage, even if also from a managed collection
# - user is permitted to update only non-manager permissions from any Collections
# - user is permitted to update any non-collection permissions
class EditPermissionsService
# @api public
# @since v3.0.0
#
# @param form [SimpleForm::FormBuilder]
# @param current_ability [Ability]
# @return [Hyrax::EditPermissionService]
#
# @note
# form object.class = SimpleForm::FormBuilder
# For works (i.e. GenericWork):
# - form object.object = Hyrax::GenericWorkForm
# - form object.object.model = GenericWork
# - use the work itself
# For file_sets:
# - form object.object.class = FileSet
# - use work the file_set is in
# No other object types are supported by this view. %>
def self.build_service_object_from(form:, ability:)
if form.object.respond_to?(:model) && form.object.model.work?
new(object: form.object, ability: ability)
elsif form.object.file_set?
new(object: form.object.in_works.first, ability: ability)
end
end

attr_reader :depositor, :unauthorized_collection_managers

# @param object [#depositor, #admin_set_id, #member_of_collection_ids] GenericWorkForm (if called for object) or GenericWork (if called for file set)
# @param ability [Ability] user's current_ability
def initialize(object:, ability:)
@object = object
@ability = ability
@depositor = object.depositor
unauthorized = manager_permissions_to_block
@unauthorized_managers = unauthorized.unauthorized_managers
@unauthorized_collection_managers = unauthorized.unauthorized_collection_managers
end

# @api private
# @todo refactor this code to use "can_edit?"; Thinking in negations can be challenging.
#
# @param permission_hash [Hash] one set of permission fields for object {:name, :access}
# @return [Boolean] true if user cannot edit the given permissions
def cannot_edit_permissions?(permission_hash)
permission_hash.fetch(:access) == "edit" && @unauthorized_managers.include?(permission_hash.fetch(:name))
end

# @api private
#
# @param permission_hash [Hash] one set of permission fields for object {:name, :access}
# @return [Boolean] true if given permissions are one of fixed exclusions
def excluded_permission?(permission_hash)
exclude_from_display.include? permission_hash.fetch(:name).downcase
end

# @api public
#
# This method either:
#
# * returns false if the given permission_hash is part of the fixed exclusions.
# * yields a PermissionPresenter to provide additional logic and text for rendering
#
# @param permission_hash [Hash<:name, :access>]
# @return false if the given permission_hash is a fixed exclusion
# @yield PermissionPresenter
#
# @see #excluded_permission?
def with_applicable_permission(permission_hash:)
return false if excluded_permission?(permission_hash)
yield(PermissionPresenter.new(service: self, permission_hash: permission_hash))
end

# @api private
#
# A helper class to contain specific presentation logic related to
# the EditPermissionsService
class PermissionPresenter
# @param service [Hyrax::EditPermissionsService]
# @param permission_hash [Hash]
def initialize(service:, permission_hash:)
@service = service
@permission_hash = permission_hash
end

# A hint at how permissions are granted.
#
# @return String
# rubocop:disable Rails/OutputSafety
def granted_by_html_hint
html = ""
@service.unauthorized_collection_managers.each do |managers|
next unless name == managers.fetch(:name)
html += "<br />Access granted via collection #{managers.fetch(:id)}"
end
html.html_safe
end
# rubocop:enable Rails/OutputSafety

# @return String
def name
@permission_hash.fetch(:name)
end

# @return String
def access
@permission_hash.fetch(:access)
end

# @return Boolean
# @see EditPermissionsService#cannot_edit_permissions?
def can_edit?
!@service.cannot_edit_permissions?(@permission_hash)
end
end

private

# Fixed set of users & groups to exclude from "editable" section of display
def exclude_from_display
[::Ability.public_group_name, ::Ability.registered_group_name, ::Ability.admin_group_name, @depositor]
end

BlockedPermissions = Struct.new(:unauthorized_managers, :unauthorized_collection_managers)

# find all of the other managers of collections which a user cannot manage
#
# Process used:
# - find all of the work's collections which a user can manage
# - find all of the work's collections (of a type which shares permissions) that a user cannot manage
# - find all of the managers of these collections the user cannot manage
# This gives us the manager permissions the user is not authorized to update.
#
# @return [Struct] BlockedPermissions
# - unauthorized_managers [Array] ids of managers of all collections
# - unauthorized_collection_managers [Array hashes] manager ids & collection_ids [{:name, :id}]
def manager_permissions_to_block
unauthorized_managers = []
unauthorized_collection_managers = []
object_unauthorized_collection_ids.each do |id|
Hyrax::PermissionTemplate.find_by(source_id: id).access_grants.each do |grant|
if grant.access == "manage"
unauthorized_managers << grant.agent_id
unauthorized_collection_managers += Array.wrap({ name: grant.agent_id }.merge(id: id))
end
end
end
BlockedPermissions.new(unauthorized_managers, unauthorized_collection_managers)
end

# find all of the work's collections a user can manage
# @return [Array] of collection ids
def object_managed_collection_ids
@object_managed_collection_ids ||= object_member_of & managed_collection_ids
end

# find all of the work's collections a user cannot manage
# note: if the collection type doesn't include "sharing_applies_to_new_works", we don't limit access
# @return [Array] of collection ids with limited access
def object_unauthorized_collection_ids
@object_unauthorized_collection_ids ||= begin
limited_access = []
unauthorized_collection_ids = object_member_of - object_managed_collection_ids
if unauthorized_collection_ids.any?
unauthorized_collection_ids.each do |id|
# TODO: Can we instead use a SOLR query? This seems to be somewhat expensive. However, as this is
# used in administration instead of user front-end displays, I'm not as concerned.
collection = ActiveFedora::Base.find(id)
limited_access << id if (collection.instance_of? AdminSet) || collection.share_applies_to_new_works?
end
end
limited_access
end
end

# find all of the collection ids an object is a member of
# @return [Array] array of collection ids
def object_member_of
@object_member_of ||= begin
belongs_to = []
# get all of work's collection ids from the form
belongs_to += @object.member_of_collection_ids
belongs_to << @object.admin_set_id if @object.admin_set_id.present?
belongs_to
end
end

# The list of all collections this user has manage rights on
# @return [Array] array of all collection ids that user can manage
def managed_collection_ids
Hyrax::Collections::PermissionsService.source_ids_for_manage(ability: @ability)
end
end
end
46 changes: 46 additions & 0 deletions app/views/hyrax/base/_currently_shared.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
<% permission_service = Hyrax::EditPermissionsService.build_service_object_from(form: f, ability: current_ability) %>

<h2><%= t('.currently_sharing') %></h2>

<table class="table table-bordered">
<tr>
<th><%= t('.table_title_user') %></th>
<th><div class="col-sm-10"><%= t('.table_title_access') %></div></th>
</tr>
<tr id="file_permissions">
<td>
<%= label_tag :owner_access, class: "control-label" do %>
Depositor (<span id="file_owner" data-depositor="<%= permission_service.depositor %>"><%= link_to_profile permission_service.depositor %></span>)
<% end %>
</td>
<td>
<div class="col-sm-10">
<%= Hyrax.config.owner_permission_levels.keys[0] %>
</div>
</td>
</tr>
<%= f.fields_for :permissions do |permission_fields| %>
<% permission_service.with_applicable_permission(permission_hash: permission_fields.object.to_hash) do |permission| %>
<tr>
<td>
<%= permission_fields.label :agent_name, class: "control-label" do %>
<%= user_display_name_and_key(permission.name) %>
<%= permission.granted_by_html_hint %>
<% end %>
</td>
<td>
<div class="col-sm-10">
<% if permission.can_edit? %>
<%= permission_fields.select :access, Hyrax.config.permission_levels, {}, class: 'form-control select_perm' %>
<% else %>
<%= Hyrax.config.permission_levels.key(permission.access) %>
<% end %>
</div>
<% if permission.can_edit? %>
<button class="btn close remove_perm" data-index="<%= permission_fields.index %>">&times;</button>
<% end %>
</td>
</tr>
<% end %>
<% end %>
</table>
38 changes: 5 additions & 33 deletions app/views/hyrax/base/_form_share.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -49,39 +49,11 @@
</div>
</fieldset>

<h2><%= t('.currently_sharing') %></h2>

<table class="table">
<tr id="file_permissions">
<td width="20%">
<%= configured_owner_permission_levels.keys.first %>
</td>
<td width="60%">
<%= label_tag :owner_access, class: "control-label" do %>
<%= t('.depositor') %>(<span id="file_owner" data-depositor="<%= depositor %>"><%= link_to_profile depositor %></span>)
<% end %>
</td>
</tr>
<%= f.fields_for :permissions do |permission_fields| %>
<%# skip the public, registered, and depositor perms as they are displayed first at the top %>
<% next if ['public', 'registered', depositor].include? permission_fields.object.agent_name.downcase %>
<tr>
<td>
<%= permission_fields.select :access, configured_permission_levels, {}, class: 'form-control select_perm' %>
</td>
<td>
<%= permission_fields.label :agent_name, class: "control-label" do %>
<%= user_display_name_and_key(permission_fields.object.agent_name) %>
<% end %>
<button class="btn close remove_perm" data-index="<%= permission_fields.index %>">&times;</button>
</td>
</tr>
<% end %>
</table>
<%= render 'currently_shared', f: f %>

<script type="text/x-tmpl" id="tmpl-work-grant">
<tr>
<td>{%= o.accessLabel %}</td>
<td><label class="control-label">{%= o.name %}</label> <button class="btn close">&times;</button></td>
</tr>
<tr>
<td><label class="control-label">{%= o.name %}</label></td>
<td><div class="col-sm-10">{%= o.accessLabel %}<button class="btn close">&times;</button></div></td>
</tr>
</script>
8 changes: 8 additions & 0 deletions app/views/hyrax/batch_edits/_currently_shared.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<h2><%= t('.share_batch_with') %></h2>

<table class="table table-bordered">
<tr id="file_permissions">
<th width="65%"><%= t('.table_title_user') %></th>
<th><div class="col-sm-10"><%= t('.table_title_access') %></div></th>
</tr>
</table>
33 changes: 2 additions & 31 deletions app/views/hyrax/file_sets/_permission_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -65,37 +65,8 @@
</div>
</div>

<table class="table table-bordered">
<tr>
<th width="60%"><%= t('.table_title_user') %></th>
<th width="40%"><%= t('.table_title_access') %></th>
</tr>
<tr id="file_permissions">
<td>
<%= label_tag :owner_access, class: "control-label" do %>
<%= t('.depositor') %> (<span id="file_owner" data-depositor="<%= depositor %>"><%= link_to_profile depositor %></span>)
<% end %>
</td>
<td>
<%= configured_owner_permission_levels.keys.first %>
</td>
</tr>
<%= f.fields_for :permissions do |permission_fields| %>
<%# skip the public, registered, and depositor perms as they are displayed first at the top %>
<% next if ['public', 'registered', depositor].include? permission_fields.object.agent_name.downcase %>
<tr>
<td><%= permission_fields.label :agent_name, class: "control-label" do %>
<%= user_display_name_and_key(permission_fields.object.agent_name) %>
<% end %></td>
<td>
<div class="col-sm-8">
<%= permission_fields.select :access, Hyrax.config.permission_levels, {}, class: 'form-control select_perm' %>
</div>
<button class="btn close remove_perm" data-index="<%= permission_fields.index %>">X</button>
</td>
</tr>
<% end %>
</table>
<%= render 'currently_shared', f: f %>

<script type="text/x-tmpl" id="tmpl-file-set-grant">
<tr>
<td><label class="control-label">{%= o.name %}</label></td>
Expand Down
10 changes: 8 additions & 2 deletions config/locales/hyrax.en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,10 @@ en:
header: 'Citations:'
mendeley: Mendeley
zotero: Zotero
currently_shared:
currently_sharing: Currently Shared With
table_title_access: Access Level
table_title_user: Person/Group
file_manager:
back_to: Back to
toolbar: Toolbar
Expand Down Expand Up @@ -534,6 +538,10 @@ en:
batch_edits:
check_all:
select_to_access_selection_options: Select to access selection option
currently_shared:
share_batch_with: Share Batch With
table_title_access: Access Level
table_title_user: Person/Group
delete_selected:
button_label: Delete Selected
deleting_file_from: Deleting a file from %{application_name} is permanent. Click OK to delete this file from %{application_name}, or Cancel to cancel this operation
Expand Down Expand Up @@ -1089,8 +1097,6 @@ en:
save_note_html: Permissions are <strong>not</strong> saved until the &quot;Save&quot; button is pressed at the bottom of the page.
select_group: Select a group
share_with: Share With
table_title_access: Access Level
table_title_user: Person/Group
user_search: Search for a user
proxy:
message: "%{sending_user} has deposited the file %{title} on your behalf."
Expand Down
2 changes: 1 addition & 1 deletion spec/features/batch_edit_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
let(:permission_template) { create(:permission_template, source_id: admin_set.id) }
let!(:workflow) { create(:workflow, allows_access_grant: true, active: true, permission_template_id: permission_template.id) }

let!(:work1) { create(:public_work, admin_set_id: admin_set.id, user: current_user, members: [file_set]) }
let!(:work1) { create(:public_work, admin_set_id: admin_set.id, user: current_user, ordered_members: [file_set]) }
let!(:work2) { create(:public_work, admin_set_id: admin_set.id, user: current_user) }
let!(:file_set) { create(:file_set) }

Expand Down
2 changes: 1 addition & 1 deletion spec/features/collection_multi_membership_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
end

describe 'when both collections require single membership' do
let(:old_collection) { FactoryBot.build(:collection_lw, user: admin_user, collection_type: single_membership_type_1, title: ['OldCollectionTitle']) }
let(:old_collection) { FactoryBot.build(:collection_lw, user: admin_user, collection_type: single_membership_type_1, title: ['OldCollectionTitle'], with_permission_template: true) }
let!(:work) do
create(:generic_work,
user: admin_user,
Expand Down
1 change: 1 addition & 0 deletions spec/features/create_work_resource_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
let!(:uploaded_file2) { Hyrax::UploadedFile.create(file: file2, user: user) }

before do
Hyrax::EnsureWellFormedAdminSetService.call
visit root_path
# Grant the user access to deposit into an admin set.
create(:permission_template_access,
Expand Down
Loading

0 comments on commit 321a771

Please sign in to comment.