From fde0590eeb56241cb8ed45ed24f50c5da3f127ef Mon Sep 17 00:00:00 2001 From: LaRita Robinson Date: Tue, 26 Feb 2019 16:44:16 -0500 Subject: [PATCH] Lock unauthorized permissions from update Fixes #3166 - user is permitted to update any work permissions coming from collections they manage - user is permitted to update non-manager permissions from any Collections - user is permitted to update any non-collection permissions Process used: - find all of the work's collections a user can manage - find all of the work's collections a user cannot manage - find all of the other managers of collections a user can manage - find all of the other managers of collections a user cannot manage who are not also managers in collections that the user CAN manage. This gives us the permissions the user is not authorized to update. EditPermissionsService embeds all of the above logic and uses it to display all of the work or file set's permissions in either a fixed or editable format. The new shared partial results in a slight reformat of the display of these permissions, as there were previously minor differences between the two displays. Additionally, a new partial was created for batch edit, as the above rules cannot be enforced at a batch level. Dealing with batch editing sharing issues is beyond the scope of this issue. Restrict user from changing managers which are in both authorized & unauthorized collections --- .../hyrax/edit_permissions_service.rb | 113 +++++++++++++ .../hyrax/base/_currently_shared.html.erb | 66 ++++++++ app/views/hyrax/base/_form_share.html.erb | 38 +---- .../batch_edits/_currently_shared.html.erb | 8 + .../hyrax/file_sets/_permission_form.html.erb | 33 +--- config/locales/hyrax.en.yml | 10 +- spec/features/batch_edit_spec.rb | 2 +- .../collection_multi_membership_spec.rb | 2 +- spec/features/edit_file_spec.rb | 5 +- spec/features/edit_work_spec.rb | 24 ++- .../hyrax/edit_permissions_service_spec.rb | 148 ++++++++++++++++++ .../base/_currently_shared.html.erb_spec.rb | 37 +++++ spec/views/hyrax/base/_form.html.erb_spec.rb | 6 + .../_permission_form.html.erb_spec.rb | 25 +-- 14 files changed, 414 insertions(+), 103 deletions(-) create mode 100644 app/services/hyrax/edit_permissions_service.rb create mode 100644 app/views/hyrax/base/_currently_shared.html.erb create mode 100644 app/views/hyrax/batch_edits/_currently_shared.html.erb create mode 100644 spec/services/hyrax/edit_permissions_service_spec.rb create mode 100644 spec/views/hyrax/base/_currently_shared.html.erb_spec.rb diff --git a/app/services/hyrax/edit_permissions_service.rb b/app/services/hyrax/edit_permissions_service.rb new file mode 100644 index 0000000000..a740f2aa1e --- /dev/null +++ b/app/services/hyrax/edit_permissions_service.rb @@ -0,0 +1,113 @@ +# frozen_string_literal: true +module Hyrax + class EditPermissionsService + # 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 + attr_reader :depositor, :unauthorized_collection_managers + + # @param [Object] GenericWorkForm (if called for object) or GenericWork (if called for file set) + # @param [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 + + # @param [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) + @unauthorized_managers.include?(permission_hash[:name]) && permission_hash[:access] == "edit" + end + + # @param [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[:name].downcase + 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 = [] + if object_unauthorized_collection_ids.any? + 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 + 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| + 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 + @object.member_of_collections.each do |collection| + belongs_to << collection.id + end + belongs_to << @object.admin_set_id unless @object.admin_set_id.empty? + 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 diff --git a/app/views/hyrax/base/_currently_shared.html.erb b/app/views/hyrax/base/_currently_shared.html.erb new file mode 100644 index 0000000000..694430b16a --- /dev/null +++ b/app/views/hyrax/base/_currently_shared.html.erb @@ -0,0 +1,66 @@ +<%# +# 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. %> +<% if f.object.respond_to?(:model) && f.object.model.work? + object_acting_upon = f.object +elsif f.object.file_set? + object_acting_upon = f.object.in_works.first +end %> + +<% permission_service = Hyrax::EditPermissionsService.new(object: object_acting_upon, ability: current_ability) %> +<% depositor = permission_service.depositor %> + +

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

+ + + + + + + + + + + <%= f.fields_for :permissions do |permission_fields| %> + <% perm_hash = permission_fields.object.to_hash %> + <% next if permission_service.excluded_permission?(perm_hash) %> + <% cannot_edit_perms = permission_service.cannot_edit_permissions?(perm_hash) %> + + + + + <% end %> +
<%= t('.table_title_user') %>
<%= t('.table_title_access') %>
+ <%= label_tag :owner_access, class: "control-label" do %> + Depositor (<%= link_to_profile depositor %>) + <% end %> + +
+ <%= Hyrax.config.owner_permission_levels.keys[0] %> +
+
+ <%= permission_fields.label :agent_name, class: "control-label" do %> + <%= user_display_name_and_key(perm_hash[:name]) %> + <% permission_service.unauthorized_collection_managers.select {|mgrs| mgrs[:name] == perm_hash[:name] }.each do |coll| %> +
Access granted via collection <%= coll[:id] %> + <% end %> + <% end %> + 		+
+ <% if cannot_edit_perms %> + <%= Hyrax.config.permission_levels.key(perm_hash[:access]) %> + <% else %> + <%= permission_fields.select :access, Hyrax.config.permission_levels, {}, class: 'form-control select_perm' %> + <% end %> +
+ <% if !cannot_edit_perms %> + + <% end %> +
diff --git a/app/views/hyrax/base/_form_share.html.erb b/app/views/hyrax/base/_form_share.html.erb index e5c13100be..3a7bbf733b 100644 --- a/app/views/hyrax/base/_form_share.html.erb +++ b/app/views/hyrax/base/_form_share.html.erb @@ -49,39 +49,11 @@ -

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

- - - - - - - <%= 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 %> - - - - - <% end %> -
- <%= configured_owner_permission_levels.keys.first %> - - <%= label_tag :owner_access, class: "control-label" do %> - <%= t('.depositor') %>(<%= link_to_profile depositor %>) - <% end %> -
- <%= permission_fields.select :access, configured_permission_levels, {}, class: 'form-control select_perm' %> - - <%= permission_fields.label :agent_name, class: "control-label" do %> - <%= user_display_name_and_key(permission_fields.object.agent_name) %> - <% end %> - -
+<%= render 'currently_shared', f: f %> diff --git a/app/views/hyrax/batch_edits/_currently_shared.html.erb b/app/views/hyrax/batch_edits/_currently_shared.html.erb new file mode 100644 index 0000000000..68cf3d6222 --- /dev/null +++ b/app/views/hyrax/batch_edits/_currently_shared.html.erb @@ -0,0 +1,8 @@ +

<%= t('.share_batch_with') %>

+ + + + + + +
<%= t('.table_title_user') %>
<%= t('.table_title_access') %>
diff --git a/app/views/hyrax/file_sets/_permission_form.html.erb b/app/views/hyrax/file_sets/_permission_form.html.erb index 4992f8164f..8e01aa96f8 100644 --- a/app/views/hyrax/file_sets/_permission_form.html.erb +++ b/app/views/hyrax/file_sets/_permission_form.html.erb @@ -65,37 +65,8 @@ - - - - - - - - - - <%= 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 %> - - - - - <% end %> -
<%= t('.table_title_user') %><%= t('.table_title_access') %>
- <%= label_tag :owner_access, class: "control-label" do %> - <%= t('.depositor') %> (<%= link_to_profile depositor %>) - <% end %> - - <%= configured_owner_permission_levels.keys.first %> -
<%= permission_fields.label :agent_name, class: "control-label" do %> - <%= user_display_name_and_key(permission_fields.object.agent_name) %> - <% end %> -
- <%= permission_fields.select :access, Hyrax.config.permission_levels, {}, class: 'form-control select_perm' %> -
- -
+<%= render 'currently_shared', f: f %> +