From beac31a4c9ad4f905e59f5fb82ba99e37aabe37f Mon Sep 17 00:00:00 2001 From: MontrealSergiy Date: Tue, 30 Jul 2024 08:18:00 -0400 Subject: [PATCH 1/4] Only ensure a CbrainFileList read access at task launch, resolves #1057 --- BrainPortal/app/controllers/tasks_controller.rb | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/BrainPortal/app/controllers/tasks_controller.rb b/BrainPortal/app/controllers/tasks_controller.rb index ae42a9d55..d14d9cb07 100644 --- a/BrainPortal/app/controllers/tasks_controller.rb +++ b/BrainPortal/app/controllers/tasks_controller.rb @@ -235,14 +235,19 @@ def new #:nodoc: # Filter list of files as provided by the get request file_ids = params[:file_ids] || [] + cb_file_ids = CbrainFileList.where(:id => file_ids).pluck(:id) + other_file_ids = file_ids.map(&:to_i) - cb_file_ids if @tool_config.try(: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.count == 0 - flash[:error] = "You must select at least one file to which you have write access." + cb_files = Userfile.find_accessible_by_user(cb_file_ids, current_user, :access_requested => :read) rescue [] + other_files = Userfile.find_accessible_by_user(other_file_ids, current_user, :access_requested => access) rescue [] + @files = cb_files + other_files + if @files.count != file_ids.count + flash[:error] = "Select files to which you have #{access} access." + flash[:error] = "You must select at least one file." if @files.count == 0 redirect_to :controller => :userfiles, :action => :index return end From 8096a38965a6138bca337dfee5c3dc901c17da3d Mon Sep 17 00:00:00 2001 From: MontrealSergiy Date: Tue, 30 Jul 2024 17:54:23 -0400 Subject: [PATCH 2/4] Only ensure a CbrainFileList read access at task submission, resolves #1057 --- .../templates/portal.rb.erb | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/BrainPortal/lib/cbrain_task_generators/templates/portal.rb.erb b/BrainPortal/lib/cbrain_task_generators/templates/portal.rb.erb index ab6e1bcfc..a81c71f3c 100644 --- a/BrainPortal/lib/cbrain_task_generators/templates/portal.rb.erb +++ b/BrainPortal/lib/cbrain_task_generators/templates/portal.rb.erb @@ -439,7 +439,8 @@ class CbrainTask::<%= name %> < <%= (descriptor['custom'] || {})['cbrain:inherit begin # Check that the user has access to all of the files in the cbcsv fs = f.userfiles_accessible_by_user!(self.user,nil,nil,file_access) for i in f.ordered_raw_ids.select{ |r| (! r.nil?) && (r.to_s != '0') } - accessible = ! ( Userfile.find_accessible_by_user( i, self.user, :access_requested => file_access ) rescue nil ).nil? + accessible = ! ( Userfile.find_accessible_by_user( i, self.user, :access_requested => file_access ) rescue nil ).nil? + accessible |= ! ( CbrainFileList.find_accessible_by_user( i, self.user, :access_requested => :read ) rescue nil ).nil? params_errors.add( id, msg1.(i) ) unless accessible errFlag = false unless accessible end @@ -463,14 +464,17 @@ class CbrainTask::<%= name %> < <%= (descriptor['custom'] || {})['cbrain:inherit # Get cbcsvs (note: we get files that end with cbcsv, but may not be of that class; the user is warned when this occurs, i.e. after_form fails) files = self.params[:interface_userfile_ids].map do |f| begin - Userfile.find_accessible_by_user( f, self.user, :access_requested => file_access ) + # for file list read access is enough + file = CbrainFileList.find_accessible_by_user( f, self.user, :access_requested => :read ) rescue nil + # for individual files or collections, write access is needed if tool may mutate inputs + file ||= Userfile.find_accessible_by_user( f, self.user, :access_requested => file_access ) rescue => e params_errors.add(<%= ":'#{single_file['id']}'" %>, "encountered an error trying to find file #{f}. Ensure the file exists and you can access it.") return "" end end cbcsvs = files.select(&:presence).map do |f| - Userfile.find_accessible_by_user( f, self.user, :access_requested => file_access ) + Userfile.find_accessible_by_user( f, self.user, :access_requested => :read ) end.select do |f| f.is_a?(CbrainFileList) || (f.suggested_file_type || Object) <= CbrainFileList end @@ -531,7 +535,7 @@ class CbrainTask::<%= name %> < <%= (descriptor['custom'] || {})['cbrain:inherit <%= "file_lists = [%s]" % file_lists.map { |f| ":'#{f['id']}'" }.join( ', ' ) %> return [] if files.nil? || files.length == 0 files.select { |f| self.params[f].present? && ! file_lists.include?(f) } # Prevent problems with file-type inputs with list=true - .map { |f| [f, Userfile.find_accessible_by_user(self.params[f], self.user, :access_requested => file_access)] } + .map { |f| [f, Userfile.find_accessible_by_user(self.params[f], self.user, :access_requested => :read)] } .select { |f| f[1].is_a?(CbrainFileList) || (f[1].suggested_file_type || Object) <= CbrainFileList } end @@ -567,7 +571,8 @@ class CbrainTask::<%= name %> < <%= (descriptor['custom'] || {})['cbrain:inherit tsk end # Expand cbcsvs and generate tasks from them - f = Userfile.find_accessible_by_user( id, self.user, :access_requested => file_access ) + f = CbrainFileList.find_accessible_by_user( id, self.user, :access_requested => :read ) rescue nil + f ||= Userfile.find_accessible_by_user( id, self.user, :access_requested => file_access ) if f.is_a?( CbrainFileList ) ufiles = f.userfiles_accessible_by_user!( self.user, nil, nil, file_access ) # Skip files that are purposefully nil (e.g. given id 0 by the user) @@ -724,7 +729,8 @@ class CbrainTask::<%= name %> < <%= (descriptor['custom'] || {})['cbrain:inherit next value end - unless (file = Userfile.find_accessible_by_user(value, self.user, :access_requested => file_access) rescue nil) + unless (file = Userfile.find_accessible_by_user(value, self.user, :access_requested => file_access) rescue nil || + CbrainFileList.find_accessible_by_user(value, self.user, :access_requested => :read) rescue nil ) params_errors.add(name, ": cannot find userfile (ID #{value})") next value end From 79117c76b12bcdfacc0c2d35f7a5552736954364 Mon Sep 17 00:00:00 2001 From: MontrealSergiy Date: Mon, 5 Aug 2024 16:50:37 -0400 Subject: [PATCH 3/4] Removing a redundancy/ beatification for ensure a CbrainFileList read access at task submission, resolves #1057 --- .../lib/cbrain_task_generators/templates/portal.rb.erb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/BrainPortal/lib/cbrain_task_generators/templates/portal.rb.erb b/BrainPortal/lib/cbrain_task_generators/templates/portal.rb.erb index a81c71f3c..fbfc66060 100644 --- a/BrainPortal/lib/cbrain_task_generators/templates/portal.rb.erb +++ b/BrainPortal/lib/cbrain_task_generators/templates/portal.rb.erb @@ -729,8 +729,9 @@ class CbrainTask::<%= name %> < <%= (descriptor['custom'] || {})['cbrain:inherit next value end - unless (file = Userfile.find_accessible_by_user(value, self.user, :access_requested => file_access) rescue nil || - CbrainFileList.find_accessible_by_user(value, self.user, :access_requested => :read) rescue nil ) + file = Userfile.find_accessible_by_user(value, self.user, :access_requested => file_access) rescue nil + file ||= CbrainFileList.find_accessible_by_user(value, self.user, :access_requested => :read) rescue nil + unless file params_errors.add(name, ": cannot find userfile (ID #{value})") next value end From ea91eb42886ba90212d5d02afaf205a5fa111bb3 Mon Sep 17 00:00:00 2001 From: MontrealSergiy Date: Tue, 10 Jun 2025 07:57:49 -0400 Subject: [PATCH 4/4] drop old style boutiques and additional checks --- .../templates/portal.rb.erb | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/BrainPortal/lib/cbrain_task_generators/templates/portal.rb.erb b/BrainPortal/lib/cbrain_task_generators/templates/portal.rb.erb index fbfc66060..ab6e1bcfc 100644 --- a/BrainPortal/lib/cbrain_task_generators/templates/portal.rb.erb +++ b/BrainPortal/lib/cbrain_task_generators/templates/portal.rb.erb @@ -439,8 +439,7 @@ class CbrainTask::<%= name %> < <%= (descriptor['custom'] || {})['cbrain:inherit begin # Check that the user has access to all of the files in the cbcsv fs = f.userfiles_accessible_by_user!(self.user,nil,nil,file_access) for i in f.ordered_raw_ids.select{ |r| (! r.nil?) && (r.to_s != '0') } - accessible = ! ( Userfile.find_accessible_by_user( i, self.user, :access_requested => file_access ) rescue nil ).nil? - accessible |= ! ( CbrainFileList.find_accessible_by_user( i, self.user, :access_requested => :read ) rescue nil ).nil? + accessible = ! ( Userfile.find_accessible_by_user( i, self.user, :access_requested => file_access ) rescue nil ).nil? params_errors.add( id, msg1.(i) ) unless accessible errFlag = false unless accessible end @@ -464,17 +463,14 @@ class CbrainTask::<%= name %> < <%= (descriptor['custom'] || {})['cbrain:inherit # Get cbcsvs (note: we get files that end with cbcsv, but may not be of that class; the user is warned when this occurs, i.e. after_form fails) files = self.params[:interface_userfile_ids].map do |f| begin - # for file list read access is enough - file = CbrainFileList.find_accessible_by_user( f, self.user, :access_requested => :read ) rescue nil - # for individual files or collections, write access is needed if tool may mutate inputs - file ||= Userfile.find_accessible_by_user( f, self.user, :access_requested => file_access ) + Userfile.find_accessible_by_user( f, self.user, :access_requested => file_access ) rescue => e params_errors.add(<%= ":'#{single_file['id']}'" %>, "encountered an error trying to find file #{f}. Ensure the file exists and you can access it.") return "" end end cbcsvs = files.select(&:presence).map do |f| - Userfile.find_accessible_by_user( f, self.user, :access_requested => :read ) + Userfile.find_accessible_by_user( f, self.user, :access_requested => file_access ) end.select do |f| f.is_a?(CbrainFileList) || (f.suggested_file_type || Object) <= CbrainFileList end @@ -535,7 +531,7 @@ class CbrainTask::<%= name %> < <%= (descriptor['custom'] || {})['cbrain:inherit <%= "file_lists = [%s]" % file_lists.map { |f| ":'#{f['id']}'" }.join( ', ' ) %> return [] if files.nil? || files.length == 0 files.select { |f| self.params[f].present? && ! file_lists.include?(f) } # Prevent problems with file-type inputs with list=true - .map { |f| [f, Userfile.find_accessible_by_user(self.params[f], self.user, :access_requested => :read)] } + .map { |f| [f, Userfile.find_accessible_by_user(self.params[f], self.user, :access_requested => file_access)] } .select { |f| f[1].is_a?(CbrainFileList) || (f[1].suggested_file_type || Object) <= CbrainFileList } end @@ -571,8 +567,7 @@ class CbrainTask::<%= name %> < <%= (descriptor['custom'] || {})['cbrain:inherit tsk end # Expand cbcsvs and generate tasks from them - f = CbrainFileList.find_accessible_by_user( id, self.user, :access_requested => :read ) rescue nil - f ||= Userfile.find_accessible_by_user( id, self.user, :access_requested => file_access ) + f = Userfile.find_accessible_by_user( id, self.user, :access_requested => file_access ) if f.is_a?( CbrainFileList ) ufiles = f.userfiles_accessible_by_user!( self.user, nil, nil, file_access ) # Skip files that are purposefully nil (e.g. given id 0 by the user) @@ -729,9 +724,7 @@ class CbrainTask::<%= name %> < <%= (descriptor['custom'] || {})['cbrain:inherit next value end - file = Userfile.find_accessible_by_user(value, self.user, :access_requested => file_access) rescue nil - file ||= CbrainFileList.find_accessible_by_user(value, self.user, :access_requested => :read) rescue nil - unless file + unless (file = Userfile.find_accessible_by_user(value, self.user, :access_requested => file_access) rescue nil) params_errors.add(name, ": cannot find userfile (ID #{value})") next value end