Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent sharing works and filesets with unintended groups #6823

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
4 changes: 3 additions & 1 deletion app/assets/javascripts/hyrax/permissions/control.es6
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Registry } from './registry'
import { UserControls } from './user_controls'
import { GroupControls } from './group_controls'
import VisibilityComponent from '../save_work/visibility_component'
import AdminSetWidget from 'hyrax/editor/admin_set_widget'

export default class PermissionsControl {
/**
Expand All @@ -14,13 +15,14 @@ export default class PermissionsControl {
if (element.length === 0) {
return
}
this.adminSetWidget = new AdminSetWidget(element.find('select[id="admin_set_id"]'))
this.element = element

this.registry = new Registry(this.element, this.object_name(), template_id)
this.user_controls = new UserControls(this.element, this.registry)
this.group_controls = new GroupControls(this.element, this.registry)
if (with_visibility_component) {
this.visibility_component = new VisibilityComponent(this.element)
this.visibility_component = new VisibilityComponent(this.element, this.adminSetWidget)
} else {
this.visibility_component = null
}
Expand Down
15 changes: 15 additions & 0 deletions app/controllers/hyrax/file_sets_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class FileSetsController < ApplicationController

# GET /concern/file_sets/:id
def edit
@file_set_admin_set_option = file_set_admin_set_option
initialize_edit_form
end

Expand Down Expand Up @@ -255,6 +256,19 @@ def add_breadcrumb_for_action
end
end

# Retrieves the admin set the file_set belongs to
def file_set_admin_set_option
return @admin_set_option if @admin_set_option
parent_work = parent(file_set: presenter)
admin_set_tesim = parent_work.solr_document[:admin_set_tesim].first

service = Hyrax::AdminSetService.new(self)
admin_set_option = Hyrax::AdminSetOptionsPresenter.new(service).select_options.reject do |option|
option[0] != admin_set_tesim
end
admin_set_option
end

def initialize_edit_form
guard_for_workflow_restriction_on!(parent: parent)

Expand All @@ -267,6 +281,7 @@ def initialize_edit_form
end
@version_list = Hyrax::VersionListPresenter.for(file_set: file_set)
@groups = current_user.groups
@is_admin = current_ability.admin?
end

include WorkflowsHelper # Provides #workflow_restriction?, and yes I mean include not helper; helper exposes the module methods
Expand Down
7 changes: 5 additions & 2 deletions app/helpers/hyrax/hyrax_helper_behavior.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,11 @@ module HyraxHelperBehavior
# @return [Array<String>] the list of all user groups
def available_user_groups(ability:)
return ::User.group_service.role_names if ability.admin?

ability.user_groups
# Excluding "public" and "registered" groups if non-admin user
user_groups = ability.user_groups.dup
user_groups.delete("public")
user_groups.delete("registered")
user_groups
end

# Which translations are available for the user to select
Expand Down
9 changes: 9 additions & 0 deletions app/views/hyrax/file_sets/_admin_set_options.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<!-- Including non-visible admin set dropdown to trigger VisibilityComponent JavaScript. -->
<% if Flipflop.assign_admin_set? %>
<% if @file_set_admin_set_option.present? %>
<%= select_tag :admin_set_id,
options_for_select(@file_set_admin_set_option.map { |option| [option[0], option[1], option[2]] }),
include_blank: false,
class: 'form-control d-none' %>
<% end %>
<% end %>
8 changes: 8 additions & 0 deletions app/views/hyrax/file_sets/_permission.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@
data: { param_key: file_set.model_name.param_key },
class: 'nav-safety'
} do |f| %>
<!-- Only apply admin set restrictions for non admins -->
<% if !@is_admin %>
<div class="tab-content">
<div role="tabpanel" class="tab-pane show active" id="admin_sets">
<%= render 'admin_set_options' %>
</div>
</div>
<% end %>
<%= hidden_field_tag('redirect_tab', 'permissions') %>
<%= render "permission_form", f: f %>
<div id="permissions_submit">
Expand Down
2 changes: 1 addition & 1 deletion app/views/hyrax/file_sets/_permission_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
<div class="row">
<div class="col-sm-5">
<label for="new_group_name_skel" class="sr-only"><%= t('.new_group_name_skel') %></label>
<%= select_tag 'new_group_name_skel', options_for_select([t('.select_group')] + current_user.groups), class: 'form-control' %>
<%= select_tag 'new_group_name_skel', options_for_select([t('.select_group')] + available_user_groups(ability: current_ability)), class: 'form-control' %>
</div>
<div class="col-sm-4">
<label for="new_group_permission_skel" class="sr-only"><%= t('.new_group_permission_skel') %></label>
Expand Down
5 changes: 4 additions & 1 deletion spec/controllers/hyrax/file_sets_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
RSpec.describe Hyrax::FileSetsController do
routes { Rails.application.routes }
let(:user) { FactoryBot.create(:user) }
let(:admin_set) { FactoryBot.valkyrie_create(:hyrax_admin_set, user: user, with_permission_template: true) }
let(:work_user) { user }
before do
allow(Hyrax.config.characterization_service).to receive(:run).and_return(true)
Expand Down Expand Up @@ -545,7 +546,9 @@
let(:user) { FactoryBot.create(:user) }
before { sign_in user }
let(:file) { fixture_file_upload('/world.png', 'image/png') }
let(:work) { FactoryBot.valkyrie_create(:hyrax_work, title: "test title", uploaded_files: [FactoryBot.create(:uploaded_file, user: work_user)], edit_users: [work_user]) }
let(:work) do
FactoryBot.valkyrie_create(:hyrax_work, title: "test title", admin_set_id: admin_set.id, uploaded_files: [FactoryBot.create(:uploaded_file, user: work_user)], edit_users: [work_user])
end
let(:file_set) { query_service.find_members(resource: work).first }
let(:file_metadata) { query_service.custom_queries.find_files(file_set: file_set).first }
let(:uploaded) { storage_adapter.find_by(id: file_metadata.file_identifier) }
Expand Down
3 changes: 3 additions & 0 deletions spec/features/edit_file_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
let(:user) { create(:user) }
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(:options_presenter) { double(select_options: []) }

let(:admin_set) do
if Hyrax.config.disable_wings
Expand Down Expand Up @@ -32,6 +33,8 @@

before do
sign_in user
# mock the admin set options presenter to avoid hitting Solr
allow(Hyrax::AdminSetOptionsPresenter).to receive(:new).and_return(options_presenter)

unless Hyrax.config.disable_wings
Hydra::Works::AddFileToFileSet.call(file_set, file, :original_file)
Expand Down
Loading