Readded module level archive, move, clone and manag permissions. Corrected their usage and add the permissions check in some additional places.

app/controllers/api/v20170715/core_api_controller.rb

@@ -20,7 +20,7 @@ module Api
               exp.my_modules.find_each do |tk|
                 task = tk.as_json(only: %i(name description archived))
                 task['task_id'] = tk.id.to_s
-                task['editable'] = can_manage_experiment?(tk.experiment)
+                task['editable'] = can_manage_module?(tk)
                 tasks << task
               end
               experiment['tasks'] = tasks

app/controllers/canvas_controller.rb

@@ -31,9 +31,12 @@ class CanvasController < ApplicationController
   def update
     # Make sure that remove parameter is valid
     to_archive = []
-    if can_manage_experiment?(@experiment) && update_params[:remove].present?
+    if update_params[:remove].present?
       to_archive = update_params[:remove].split(',')
-      if to_archive.all? { |id| is_int? id }
+      if to_archive.all? do |id|
+           is_int?(id) &&
+           can_manage_module?(MyModule.find_by_id(id))
+         end
         to_archive.collect!(&:to_i)
       else
         return render_403
@@ -104,7 +107,10 @@ class CanvasController < ApplicationController
         # Okay, JSON parsed!
         unless to_rename.is_a?(Hash) &&
                to_rename.keys.all? { |k| k.is_a? String } &&
-               to_rename.values.all? { |k| k.is_a? String }
+               to_rename.values.all? { |k| k.is_a? String } &&
+               to_rename.keys.all? do |id|
+                 can_manage_module?(MyModule.find_by_id(id))
+               end
           return render_403
         end
       rescue
@@ -114,13 +120,16 @@ class CanvasController < ApplicationController
 
     # Make sure move parameter is valid
     to_move = {}
-    if can_manage_experiment?(@experiment) && update_params[:move].present?
+    if update_params[:move].present?
       begin
         to_move = JSON.parse(update_params[:move])
         # Okay, JSON parsed!
         unless to_move.is_a?(Hash) &&
                to_move.keys.all? { |k| k.is_a? String } &&
-               to_move.values.all? { |k| k.is_a? String }
+               to_move.values.all? { |k| k.is_a? String } &&
+               to_rename.keys.all? do |id|
+                 can_manage_module?(MyModule.find_by_id(id))
+               end
           return render_403
         end
       rescue

app/controllers/experiments_controller.rb

@@ -14,8 +14,8 @@ class ExperimentsController < ApplicationController
                        :clone_modal, :move_modal, :delete_samples]
   before_action :check_view_permissions,
                 only: [:canvas, :module_archive]
-  before_action :check_experiment_move_or_clone_permissions,
-                only: %i(clone_modal clone move_modal move)
+  before_action :check_clone_permissions, only: %i(clone_modal clone)
+  before_action :check_move_permissions, only: %i(move_modal move)
 
   # except parameter could be used but it is not working.
   layout :choose_layout
@@ -344,8 +344,12 @@ class ExperimentsController < ApplicationController
     render_403 unless can_read_experiment?(@experiment)
   end
 
-  def check_experiment_move_or_clone_permissions
-    render_403 unless can_move_or_clone_experiment?(@experiment)
+  def check_clone_permissions
+    render_403 unless can_clone_experiment?(@experiment)
+  end
+
+  def check_move_permissions
+    render_403 unless can_move_experiment?(@experiment)
   end
 
   def choose_layout

app/controllers/my_modules_controller.rb

@@ -602,7 +602,7 @@ class MyModulesController < ApplicationController
   end
 
   def check_manage_permissions
-    render_403 unless can_manage_experiment?(@my_module.experiment)
+    render_403 unless can_manage_module?(@my_module)
   end
 
   def check_view_info_permissions

app/permissions/experiment.rb

@@ -12,29 +12,64 @@ Canaid::Permissions.register_for(Experiment) do
 
   # experiment: create, update, delete
   # canvas/workflow: edit
-  # module: create, edit, delete, archive, move
+  # module: create
   can :manage_experiment do |user, experiment|
     user.is_user_or_higher_of_project?(experiment.project)
   end
 
-  can :restore_experiment do |user, experiment|
-    experiment.archived? && can_manage_experiment?(user, experiment)
+  # experiment: archive
+  can :archive_experiment do |user, experiment|
+    can_manage_experiment?(user, experiment)
   end
 
-  can :move_or_clone_experiment do |user, experiment|
+  # experiment: restore
+  can :restore_experiment do |user, experiment|
+    can_manage_experiment?(user, experiment) && experiment.archived?
+  end
+
+  # experiment: clone
+  can :clone_experiment do |user, experiment|
     user.is_user_or_higher_of_project?(experiment.project) &&
       user.is_normal_user_or_admin_of_team?(experiment.project.team)
   end
+
+  # experiment: move
+  can :move_experiment do |user, experiment|
+    can_clone_experiment?(user, experiment)
+  end
+
+  %i(read_experiment
+     manage_experiment
+     archive_experiment
+     clone_experiment
+     move_experiment)
+    .each do |perm|
+    can perm do |_, experiment|
+      experiment.project.active?
+    end
+  end
 end
 
 Canaid::Permissions.register_for(MyModule) do
+  # module: restore
   can :restore_module do |user, my_module|
-    my_module.archived? && can_manage_experiment?(user, experiment)
+    can_manage_experiment?(user, my_module.experiment) && my_module.archived?
+  end
+
+  # module: edit, archive, move
+  can :manage_module do |user, my_module|
+    can_manage_experiment?(user, my_module.experiment)
+  end
+
+  %i(manage_module).each do |perm|
+    can perm do |_, my_module|
+      my_module.experiment.project.active?
+    end
   end
 end
 
 Canaid::Permissions.register_for(Protocol) do
-  # protocol: read
+  # protocol in module: read
   # step: read, read comments, read assets, download assets
   can :read_protocol_in_module do |user, protocol|
     if protocol.in_module?
@@ -48,8 +83,8 @@ Canaid::Permissions.register_for(Protocol) do
     end
   end
 
-  # protocol: create, update, delete, unlink, revert, update from protocol in
-  # repository
+  # protocol in module: create, update, delete, unlink, revert, update from
+  # protocol in repository, update from file
   # step: create, update, delete, reorder
   can :manage_protocol_in_module do |user, protocol|
     if protocol.in_module?
@@ -57,9 +92,17 @@ Canaid::Permissions.register_for(Protocol) do
       my_module.active? &&
         my_module.experiment.active? &&
         my_module.experiment.project.active? &&
-        can_manage_experiment?(user, my_module.experiment)
+        can_manage_module?(user, my_module)
     else
       false
     end
   end
+
+  %i(read_protocol_in_module
+     manage_protocol_in_module)
+    .each do |perm|
+    can perm do |_, protocol|
+      protocol.my_module.experiment.project.active?
+    end
+  end
 end

app/views/canvas/_edit.html.erb

@@ -71,12 +71,16 @@
   </span>
 </div>
 <div id="diagram-container">
+  <% if can_manage_experiment?(@experiment) %>
     <div id="diagram" class="diagram">
       <% my_modules.each do |my_module| %>
         <%= render partial: "canvas/edit/my_module", locals: {experiment: @experiment, my_module: my_module} %>
       <% end %>
+    </div>
+  <% end %>
 </div>
 
+<%-# Since we need to preload modals, we just check permission for experiment, instead of permissions for every module and module group -%>
 <% if can_manage_experiment?(@experiment) %>
   <%= render partial: "canvas/edit/modal/new_module", locals: {experiment: @experiment} %>
   <%= render partial: "canvas/edit/modal/edit_module", locals: {experiment: @experiment } %>

app/views/canvas/edit/_my_module.html.erb

@@ -16,7 +16,7 @@
       </a>
       <ul class="dropdown-menu no-scale" aria-labelledby="<%= my_module.id %>_options">
         <li class="dropdown-header"><%= t('projects.index.options_header') %></li>
-        <% if can_manage_experiment?(my_module.experiment) %>
+        <% if can_manage_module?(my_module) %>
           <li>
               <a class="edit-module" href="" data-module-id="<%= my_module.id %>"><%=t "experiments.canvas.edit.edit_module" %></a>
           </li>
@@ -29,7 +29,7 @@
             <a class ="clone-module-group" href="" data-module-id="<%= my_module.id %>"><%=t "experiments.canvas.edit.clone_module_group" %></a>
           </li>
         <% end %>
-        <% if can_manage_experiment?(my_module.experiment) %>
+        <% if can_manage_module?(my_module) %>
           <li>
             <a class="move-module" href="" data-module-id="<%= my_module.id %>"><%=t "experiments.canvas.edit.move_module" %></a>
           </li>
@@ -39,6 +39,8 @@
           <li>
             <a class="delete-module" href="" data-module-id="<%= my_module.id %>"><%=t "experiments.canvas.edit.delete_module" %></a>
           </li>
+        <% end %>
+        <% if my_module.my_module_group && my_module.my_module_group.my_modules.all? { |my_module| can_manage_module?(my_module) } %>
           <li data-hook="archive-module-group"
               <%= 'style=display:none;' if my_module.my_module_group.blank? %>>
             <a class ="delete-module-group" href="" data-module-id="<%= my_module.id %>"><%=t "experiments.canvas.edit.delete_module_group" %></a>

app/views/canvas/full_zoom/_my_module.html.erb

@@ -31,7 +31,7 @@
   </div>
 
   <div class="panel-body">
-    <% if can_manage_experiment?(my_module.experiment) %>
+    <% if can_manage_module?(my_module) %>
       <%= link_to due_date_my_module_path(my_module, format: :json), remote: true, class: "due-date-link due-date-refresh" do %>
         <%= render partial: "my_modules/due_date_label.html.erb", locals: { my_module: my_module } %>
       <% end %>

app/views/experiments/_dropdown_actions.html.erb

@@ -10,12 +10,14 @@
                 data: { id: experiment.id },
                 class: 'edit-experiment' %></li>
   <% end %>
-  <% if can_move_or_clone_experiment?(experiment) %>
+  <% if can_clone_experiment?(experiment) %>
     <li><%= link_to t('experiments.clone.label_title'),
                   clone_modal_experiment_url(experiment),
                   remote: true,
                   type: 'button',
                   class: 'clone-experiment' %></li>
+  <% end %>
+  <% if can_move_experiment?(experiment) %>
     <li><%= link_to t('experiments.move.label_title'),
                   move_modal_experiment_url(experiment),
                   remote: true,

app/views/my_modules/_module_header.html.erb

@@ -11,7 +11,7 @@
 
   <div class="col-xs-6 col-sm-6 col-md-4">
     <div class="badge-icon bg-primary">
-      <% if can_manage_experiment?(@my_module.experiment) %>
+      <% if can_manage_module?(@my_module) %>
         <%= link_to due_date_my_module_path(@my_module, format: :json), remote: true, class: "due-date-link", style: "color: inherit" do %>
           <span class="glyphicon glyphicon-calendar"></span>
         <% end %>
@@ -21,7 +21,7 @@
     </div>
     <div class="well well-sm">
       <span class="hidden-xs hidden-sm  hidden-md"><%=t "my_modules.module_header.due_date" %></span>
-      <% if can_manage_experiment?(@my_module.experiment) %>
+      <% if can_manage_module?(@my_module) %>
         <%= link_to due_date_my_module_path(@my_module, format: :json), remote: true, class: "due-date-link", style: "color: inherit" do %>
           <span class="task-due-date">
             <%= render partial: "module_header_due_date_label.html.erb",
@@ -62,7 +62,7 @@
     </div>
     <div class="well well-sm">
       <span class="hidden-xs hidden-sm"><%=t "my_modules.module_header.tags" %></span>
-      <% if can_manage_experiment?(@my_module.experiment) %>
+      <% if can_manage_module?(@my_module) %>
         <%= link_to my_module_tags_edit_url(@my_module, format: :json), remote: true, class: "edit-tags-link tags-refresh", style: "color: inherit" do %>
           <%= render partial: "my_modules/tags", locals: { my_module: @my_module } %>
         <% end %>
@@ -75,7 +75,7 @@
 
 <div>
   <div class="badge-icon bg-primary">
-     <% if can_manage_experiment?(@my_module.experiment) %>
+     <% if can_manage_module?(@my_module) %>
       <%= link_to description_my_module_path(@my_module, format: :json), remote: true, class: "description-link", style: "color: inherit" do %>
         <span class="glyphicon glyphicon-info-sign"></span>
       <% end %>
@@ -84,7 +84,7 @@
     <% end %>
   </div>
   <div class="well well-sm">
-    <% if can_manage_experiment?(@my_module.experiment) %>
+    <% if can_manage_module?(@my_module) %>
       <%= link_to description_my_module_path(@my_module, format: :json), remote: true, class: "description-label description-link description-refresh", style: "color: inherit" do %>
         <% if @my_module.description.present? and not @my_module.description.empty? %>
           <%= @my_module.description %>

app/views/my_modules/_show.html.erb

@@ -6,7 +6,7 @@
       <%= render partial: "description_label.html.erb" %>
     </span>
   </li>
-  <% if can_manage_experiment?(@my_module.experiment) %>
+  <% if can_manage_module?(@my_module) %>
     <hr>
     <li>
       <%= link_to t("experiments.canvas.popups.full_info"), description_my_module_path(@my_module, format: :json), class: "description-link", remote: true %>

app/views/my_modules/protocols/_protocol_status_bar_buttons.html.erb

@@ -81,8 +81,8 @@
         <span class="caret"></span>
       </a>
       <ul class="dropdown-menu">
+        <%-# For updating module protocol from repository protocol we don't need to check read permission for the repository protocol -%>
         <% if can_manage_protocol_in_module?(@protocol) %>
-          <%-# For update_from_parent and update_from_parent_modal we don't need to check read permission for the parent protocol -%>
           <li><%= link_to update_from_parent_modal_protocol_path(@protocol, format: :json), remote: true, title: t("my_modules.protocols.protocol_status_bar.btns.update_self_title"), data: { action: "update-self" } do %>
               <%= t("my_modules.protocols.protocol_status_bar.btns.update_self") %>
             <% end %>
@@ -122,8 +122,8 @@
           <li class="disabled"><a href="#" title="<%= t("my_modules.protocols.protocol_status_bar.btns.update_parent_title") %>"><%= t("my_modules.protocols.protocol_status_bar.btns.update_parent") %></a>
           </li>
         <% end %>
+        <%-# For updating module protocol from repository protocol we don't need to check read permission for the repository protocol -%>
         <% if can_manage_protocol_in_module?(@protocol) %>
-          <%-# && can_read_protocol_in_repository?(@protocol) not needed - user can update protocol in module even if it's private and he's not the owner -%>
           <li>
             <%= link_to update_from_parent_modal_protocol_path(@protocol, format: :json), remote: true, title: t("my_modules.protocols.protocol_status_bar.btns.update_self_title"), data: { action: "update-self" } do %>
               <%= t("my_modules.protocols.protocol_status_bar.btns.update_self") %>