From fbabdbdb4893d4338d9c3a3329c0bcd4b375c8c6 Mon Sep 17 00:00:00 2001 From: MontrealSergiy Date: Mon, 23 Nov 2020 11:32:50 -0500 Subject: [PATCH 1/6] Tool config data immutability flag resolves #1054 --- BrainPortal/app/controllers/tasks_controller.rb | 6 +++++- BrainPortal/app/controllers/tool_configs_controller.rb | 2 ++ BrainPortal/app/views/tool_configs/_form_fields.html.erb | 8 ++++++++ .../20201123133913_add_inputs_readonly_to_tool_config.rb | 5 +++++ BrainPortal/db/schema.rb | 3 ++- 5 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 BrainPortal/db/migrate/20201123133913_add_inputs_readonly_to_tool_config.rb diff --git a/BrainPortal/app/controllers/tasks_controller.rb b/BrainPortal/app/controllers/tasks_controller.rb index 849cb62f9..e44a6e873 100644 --- a/BrainPortal/app/controllers/tasks_controller.rb +++ b/BrainPortal/app/controllers/tasks_controller.rb @@ -205,7 +205,11 @@ def new #:nodoc: # Filter list of files as provided by the get request file_ids = params[:file_ids] || [] - access = @task.class.properties[:readonly_input_files] ? :read : :write + if @tool_config.inputs_readonly || @task.class.properties[:readonly_input_files] + access = :read + else + access = :write + end @files = Userfile.find_accessible_by_user(file_ids, current_user, :access_requested => access) rescue [] if @files.empty? flash[:error] = "You must select at least one file to which you have write access." diff --git a/BrainPortal/app/controllers/tool_configs_controller.rb b/BrainPortal/app/controllers/tool_configs_controller.rb index d0be92185..52e4ffb86 100644 --- a/BrainPortal/app/controllers/tool_configs_controller.rb +++ b/BrainPortal/app/controllers/tool_configs_controller.rb @@ -200,6 +200,7 @@ def update #:nodoc: @tool_config.version_name = other_tc.version_name @tool_config.group = other_tc.group @tool_config.ncpus = other_tc.ncpus + @tool_config.inputs_readonly = other_tc.inputs_readonly @tool_config.container_engine = other_tc.container_engine @tool_config.containerhub_image_name = other_tc.containerhub_image_name @tool_config.container_image_userfile_id = other_tc.container_image_userfile_id @@ -283,6 +284,7 @@ def tool_config_params #:nodoc: params.require(:tool_config).permit( :version_name, :description, :tool_id, :bourreau_id, :env_array, :script_prologue, :script_epilogue, :group_id, :ncpus, :container_image_userfile_id, :containerhub_image_name, :container_index_location, + :inputs_readonly, :container_engine, :extra_qsub_args, :singularity_overlays_specs, :container_exec_args, # The configuration of a tool in a VM managed by a # ScirCloud Bourreau is defined by the following diff --git a/BrainPortal/app/views/tool_configs/_form_fields.html.erb b/BrainPortal/app/views/tool_configs/_form_fields.html.erb index 6f404c03a..9fc4786fa 100644 --- a/BrainPortal/app/views/tool_configs/_form_fields.html.erb +++ b/BrainPortal/app/views/tool_configs/_form_fields.html.erb @@ -67,6 +67,14 @@ <% end %> + <% t.edit_cell(:inputs_readonly, :header => "Read only input files", :show_width => 2) do |f| %> + <%= f.check_box :inputs_readonly %> +
+ Are you sure that input files are not mutated by any task? It will allow execution with data + users cannot write to. +
+ <% end %> + <% end %> <% end %> diff --git a/BrainPortal/db/migrate/20201123133913_add_inputs_readonly_to_tool_config.rb b/BrainPortal/db/migrate/20201123133913_add_inputs_readonly_to_tool_config.rb new file mode 100644 index 000000000..ec7ca8148 --- /dev/null +++ b/BrainPortal/db/migrate/20201123133913_add_inputs_readonly_to_tool_config.rb @@ -0,0 +1,5 @@ +class AddInputsReadonlyToToolConfig < ActiveRecord::Migration[5.0] + def change + add_column :tool_configs, :inputs_readonly, :boolean, default: false + end +end diff --git a/BrainPortal/db/schema.rb b/BrainPortal/db/schema.rb index ce0ae80e9..e147b5151 100644 --- a/BrainPortal/db/schema.rb +++ b/BrainPortal/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20201119174821) do +ActiveRecord::Schema.define(version: 20201123133913) do create_table "access_profiles", force: :cascade, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci" do |t| t.string "name", null: false @@ -437,6 +437,7 @@ t.string "container_index_location" t.text "singularity_overlays_specs", limit: 65535 t.string "container_exec_args" + t.boolean "inputs_readonly", default: false t.index ["bourreau_id"], name: "index_tool_configs_on_bourreau_id", using: :btree t.index ["tool_id"], name: "index_tool_configs_on_tool_id", using: :btree end From 9803f76a44a5517ebb2e08a5a4a6739a51daa366 Mon Sep 17 00:00:00 2001 From: MontrealSergiy Date: Tue, 1 Dec 2020 16:28:05 -0500 Subject: [PATCH 2/6] non mutating tool config flag's verbiage resolves #1054 --- BrainPortal/app/views/tool_configs/_form_fields.html.erb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/BrainPortal/app/views/tool_configs/_form_fields.html.erb b/BrainPortal/app/views/tool_configs/_form_fields.html.erb index 9fc4786fa..714b34317 100644 --- a/BrainPortal/app/views/tool_configs/_form_fields.html.erb +++ b/BrainPortal/app/views/tool_configs/_form_fields.html.erb @@ -67,11 +67,11 @@ <% end %> - <% t.edit_cell(:inputs_readonly, :header => "Read only input files", :show_width => 2) do |f| %> + <% t.edit_cell(:inputs_readonly, :header => "Does not modify its inputs files", :show_width => 2) do |f| %> <%= f.check_box :inputs_readonly %>
- Are you sure that input files are not mutated by any task? It will allow execution with data - users cannot write to. + Check this if the tool is known not to modify its input files . + This will allow a user to launch the tool on files that are not marked as group-writable in the file manager.
<% end %> From 43fdcd0d80d777a1b33afb21f3544e19521fab4c Mon Sep 17 00:00:00 2001 From: MontrealSergiy Date: Tue, 1 Dec 2020 17:28:50 -0500 Subject: [PATCH 3/6] non mutating tool config flag's migration file header resolves #1054 --- ...3913_add_inputs_readonly_to_tool_config.rb | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/BrainPortal/db/migrate/20201123133913_add_inputs_readonly_to_tool_config.rb b/BrainPortal/db/migrate/20201123133913_add_inputs_readonly_to_tool_config.rb index ec7ca8148..aeabcd754 100644 --- a/BrainPortal/db/migrate/20201123133913_add_inputs_readonly_to_tool_config.rb +++ b/BrainPortal/db/migrate/20201123133913_add_inputs_readonly_to_tool_config.rb @@ -1,3 +1,26 @@ + +# +# CBRAIN Project +# +# Copyright (C) 2020 +# The Royal Institution for the Advancement of Learning +# McGill University +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + + class AddInputsReadonlyToToolConfig < ActiveRecord::Migration[5.0] def change add_column :tool_configs, :inputs_readonly, :boolean, default: false From 521774d3363e5b2d9e4e7d9762cd2c93f4b2b3f8 Mon Sep 17 00:00:00 2001 From: MontrealSergiy Date: Tue, 8 Dec 2020 13:38:55 -0500 Subject: [PATCH 4/6] non mutating tool config flag goes into boutiques task generator resolves #1054 --- BrainPortal/app/controllers/tasks_controller.rb | 2 +- BrainPortal/lib/cbrain_task_generators/templates/portal.rb.erb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/BrainPortal/app/controllers/tasks_controller.rb b/BrainPortal/app/controllers/tasks_controller.rb index e44a6e873..45c55fbc6 100644 --- a/BrainPortal/app/controllers/tasks_controller.rb +++ b/BrainPortal/app/controllers/tasks_controller.rb @@ -201,7 +201,7 @@ def new #:nodoc: @task.tool_config = lastest_toolconfig if lastest_toolconfig end - @tool_config = @task.tool_config # for acces in view + @tool_config = @task.tool_config # for access in view # Filter list of files as provided by the get request file_ids = params[:file_ids] || [] diff --git a/BrainPortal/lib/cbrain_task_generators/templates/portal.rb.erb b/BrainPortal/lib/cbrain_task_generators/templates/portal.rb.erb index 23143fbd8..8fecf8ead 100644 --- a/BrainPortal/lib/cbrain_task_generators/templates/portal.rb.erb +++ b/BrainPortal/lib/cbrain_task_generators/templates/portal.rb.erb @@ -117,7 +117,7 @@ class CbrainTask::<%= name %> < <%= (descriptor['custom'] || {})['cbrain:inherit # The symbol can be passed to methods such as Userfile.find_accessible_by_user(). # Depending on the value, more or less files are allowed to be processed. def file_access - @_file_access ||= (self.class.properties[:readonly_input_files].present? ? :read : :write) + @_file_access ||= (self.class.properties[:readonly_input_files].present? || self.tool_config.inputs_readonly ? :read : :write) end % unless defaults.empty? From e4dbd0da4eb48fd22be30943672eb1455574cc02 Mon Sep 17 00:00:00 2001 From: MontrealSergiy Date: Tue, 15 Dec 2020 12:33:53 -0500 Subject: [PATCH 5/6] fix tests of boutiques task resolves #1054 --- BrainPortal/spec/boutiques/boutiques_tester_spec.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/BrainPortal/spec/boutiques/boutiques_tester_spec.rb b/BrainPortal/spec/boutiques/boutiques_tester_spec.rb index 088e38ed8..932c95e2e 100644 --- a/BrainPortal/spec/boutiques/boutiques_tester_spec.rb +++ b/BrainPortal/spec/boutiques/boutiques_tester_spec.rb @@ -151,6 +151,7 @@ before(:each) do # Create environment execer = FactoryBot.create(:bourreau) + @task.tool_config = FactoryBot.create(:tool_config) schema = SchemaTaskGenerator.default_schema descriptor = File.join(__dir__, TestScriptDescriptor) @boutiquesTask = SchemaTaskGenerator.generate(schema, descriptor) @@ -443,6 +444,7 @@ @task.bourreau = FactoryBot.create(:bourreau) @task.user_id, @task.group_id, @task.params = @user.id, @group.id, {} @task.params[:interface_userfile_ids] = [] + @task.tool_config = FactoryBot.create(:tool_config) # Generate some userfiles for testing @f1, @f2, @f3 = @addUserFile.('f1.cpp',@task,false), @addUserFile.('f2.java',@task,false), @addUserFile.('f3.j',@task,false) end From e26bc11d5fe29b7edcaad474c945b2ccb5cc1436 Mon Sep 17 00:00:00 2001 From: MontrealSergiy Date: Tue, 15 Dec 2020 12:41:57 -0500 Subject: [PATCH 6/6] fix tests of boutiques task resolves #1054 --- BrainPortal/spec/boutiques/boutiques_tester_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/BrainPortal/spec/boutiques/boutiques_tester_spec.rb b/BrainPortal/spec/boutiques/boutiques_tester_spec.rb index 932c95e2e..fe622f7a4 100644 --- a/BrainPortal/spec/boutiques/boutiques_tester_spec.rb +++ b/BrainPortal/spec/boutiques/boutiques_tester_spec.rb @@ -151,7 +151,6 @@ before(:each) do # Create environment execer = FactoryBot.create(:bourreau) - @task.tool_config = FactoryBot.create(:tool_config) schema = SchemaTaskGenerator.default_schema descriptor = File.join(__dir__, TestScriptDescriptor) @boutiquesTask = SchemaTaskGenerator.generate(schema, descriptor) @@ -159,6 +158,7 @@ # Instantiate a task object @task = CbrainTask::BoutiquesTest.new @task.bourreau = execer + @task.tool_config = FactoryBot.create(:tool_config) @task.user_id, @task.group_id, @task.params = @user.id, @group.id, {} # Setup for holding the files the user had selected in the UI @task.params[:interface_userfile_ids] = []