Merge pull request #3557 from okriuchykhin/ok_SCI_6061

Update/implement permission checks in the my_modules_controller and my_modules_status_flow_controller [SCI-6061][SCI-6063]

app/controllers/my_module_comments_controller.rb

@@ -7,9 +7,10 @@ class MyModuleCommentsController < ApplicationController
   include CommentHelper
 
   before_action :load_vars
+  before_action :load_comment, only: %i(update destroy)
   before_action :check_view_permissions, only: :index
-  before_action :check_add_permissions, only: [:create]
-  before_action :check_manage_permissions, only: %i(edit update destroy)
+  before_action :check_create_permissions, only: :create
+  before_action :check_manage_permissions, only: %i(update destroy)
 
   def index
     comments = @my_module.last_comments(@last_comment_id, @per_page)
@@ -43,23 +44,25 @@ class MyModuleCommentsController < ApplicationController
   def load_vars
     @last_comment_id = params[:from].to_i
     @per_page = Constants::COMMENTS_SEARCH_LIMIT
-    @my_module = MyModule.find_by_id(params[:my_module_id])
+    @my_module = MyModule.find_by(id: params[:my_module_id])
 
     render_404 unless @my_module
   end
 
+  def load_comment
+    @comment = @my_module.task_comments.find(params[:id])
+  end
+
   def check_view_permissions
     render_403 unless can_read_my_module?(@my_module)
   end
 
-  def check_add_permissions
+  def check_create_permissions
     render_403 unless can_create_my_module_comments?(@my_module)
   end
 
   def check_manage_permissions
-    @comment = TaskComment.find_by_id(params[:id])
-    render_403 unless @comment.present? &&
-                      can_manage_my_module_comments?(@comment)
+    render_403 unless can_manage_my_module_comment?(@comment)
   end
 
   def comment_params

app/controllers/my_module_repositories_controller.rb

@@ -5,7 +5,7 @@ class MyModuleRepositoriesController < ApplicationController
 
   before_action :load_my_module
   before_action :load_repository, except: %i(repositories_dropdown_list repositories_list_html)
-  before_action :check_my_module_view_permissions
+  before_action :check_my_module_view_permissions, except: :update
   before_action :check_repository_view_permissions, except: %i(repositories_dropdown_list repositories_list_html)
   before_action :check_assign_repository_records_permissions, only: :update
 

app/controllers/my_module_repository_snapshots_controller.rb

@@ -5,8 +5,8 @@ class MyModuleRepositorySnapshotsController < ApplicationController
   before_action :load_repository, only: :create
   before_action :load_repository_snapshot, except: %i(create full_view_sidebar select)
   before_action :check_view_permissions, except: %i(create destroy select)
-  before_action :check_manage_permissions, only: %i(destroy select)
   before_action :check_create_permissions, only: %i(create)
+  before_action :check_manage_permissions, only: %i(destroy select)
 
   def index_dt
     @draw = params[:draw].to_i

app/controllers/my_module_status_flow_controller.rb

@@ -21,6 +21,6 @@ class MyModuleStatusFlowController < ApplicationController
   end
 
   def check_view_permissions
-    render_403 unless can_read_experiment?(@my_module.experiment)
+    render_403 unless can_read_my_module?(@my_module)
   end
 end

app/controllers/project_comments_controller.rb

@@ -9,7 +9,7 @@ class ProjectCommentsController < ApplicationController
   before_action :load_vars
   before_action :check_view_permissions, only: :index
   before_action :check_create_permissions, only: :create
-  before_action :check_manage_permissions, only: %i(edit update destroy)
+  before_action :check_manage_permissions, only: %i(update destroy)
 
   def index
     comments = @project.last_comments(@last_comment_id, @per_page)

app/controllers/result_comments_controller.rb

@@ -10,7 +10,7 @@ class ResultCommentsController < ApplicationController
 
   before_action :check_view_permissions, only: [:index]
   before_action :check_add_permissions, only: [:create]
-  before_action :check_manage_permissions, only: %i(edit update destroy)
+  before_action :check_manage_permissions, only: %i(update destroy)
 
   def index
     comments = @result.last_comments(@last_comment_id, @per_page)

app/controllers/step_comments_controller.rb

@@ -10,7 +10,7 @@ class StepCommentsController < ApplicationController
 
   before_action :check_view_permissions, only: [:index]
   before_action :check_add_permissions, only: [:create]
-  before_action :check_manage_permissions, only: %i(edit update destroy)
+  before_action :check_manage_permissions, only: %i(update destroy)
 
   def index
     comments = @step.last_comments(@last_comment_id, @per_page)

app/controllers/user_my_modules_controller.rb

@@ -1,6 +1,6 @@
 class UserMyModulesController < ApplicationController
   before_action :load_vars
-  before_action :check_view_permissions, only: %i(index index_old index_edit)
+  before_action :check_view_permissions, except: %i(create destroy)
   before_action :check_manage_permissions, only: %i(create destroy)
 
   def index_old
@@ -114,7 +114,7 @@ class UserMyModulesController < ApplicationController
   end
 
   def check_view_permissions
-    render_403 unless can_read_experiment?(@my_module.experiment)
+    render_403 unless can_read_my_module?(@my_module)
   end
 
   def check_manage_permissions

app/helpers/comment_helper.rb

@@ -71,7 +71,9 @@ module CommentHelper
 
   def comment_editable?(comment)
     case comment.type
-    when 'TaskComment', 'StepComment', 'ResultComment'
+    when 'TaskComment'
+      can_manage_my_module_comment?(comment)
+    when 'StepComment', 'ResultComment'
       can_manage_comment_in_module?(comment.becomes(Comment))
     when 'ProjectComment'
       can_manage_comment_in_project?(comment)

app/permissions/experiment.rb

@@ -97,7 +97,7 @@ Canaid::Permissions.register_for(Protocol) do
   # protocol in module: read
   # step in module: read, read comments, read/download assets
   can :read_protocol_in_module do |user, protocol|
-    can_read_experiment?(user, protocol.my_module.experiment)
+    protocol.my_module.permission_granted?(user, MyModulePermissions::READ)
   end
 
   # protocol in module: create/update/delete, unlink, revert, update from

app/permissions/my_module.rb

@@ -39,11 +39,11 @@ Canaid::Permissions.register_for(MyModule) do
     my_module.permission_granted?(user, MyModulePermissions::UPDATE_START_DATE)
   end
 
-  can :update_my_module_start_date do |user, my_module|
+  can :update_my_module_due_date do |user, my_module|
     my_module.permission_granted?(user, MyModulePermissions::UPDATE_DUE_DATE)
   end
 
-  can :update_my_module_start_date do |user, my_module|
+  can :update_my_module_notes do |user, my_module|
     my_module.permission_granted?(user, MyModulePermissions::UPDATE_NOTES)
   end
 
@@ -140,23 +140,22 @@ Canaid::Permissions.register_for(MyModule) do
   end
 end
 
-Canaid::Permissions.register_for(Comment) do
+Canaid::Permissions.register_for(TaskComment) do
   # Module, its experiment and its project must be active for all the specified
   # permissions
-  %i(manage_my_module_comments)
+  %i(manage_my_module_comment)
     .each do |perm|
     can perm do |_, comment|
       my_module = ::PermissionsUtil.get_comment_module(comment)
-      !my_module.archived_branch?
+      my_module.active? &&
+        my_module.experiment.active? &&
+        my_module.experiment.project.active?
     end
   end
 
-  # module: update/delete comment
-  # result: update/delete comment
-  # step: update/delete comment
-  can :manage_my_module_comments do |user, comment|
+  can :manage_my_module_comment do |user, comment|
     my_module = ::PermissionsUtil.get_comment_module(comment)
-    (comment.user == user && my_module.permission_granted?(user, MyModulePermissions::COMMENTS_MANAGE_OWN)) ||
-      my_module.permission_granted?(user, MyModulePermissions::COMMENTS_MANAGE)
+    my_module.permission_granted?(user, MyModulePermissions::COMMENTS_MANAGE) ||
+      ((comment.user == user) && my_module.permission_granted?(user, MyModulePermissions::COMMENTS_MANAGE_OWN))
   end
 end

config/initializers/extends/permission_extends.rb

@@ -40,21 +40,16 @@ module PermissionExtends
     %w(
       READ
       MANAGE
-      UPDATE_START_DATE
-      UPDATE_DUE_DATE
-      UPDATE_NOTES
-      TAGS_MANAGE
       STEPS_MANAGE
+      UPDATE_STATUS
+      COMMENTS_CREATE
       COMMENTS_MANAGE
       COMMENTS_MANAGE_OWN
-      COMMENTS_CREATE
-      REPOSITORY_ROWS_ASSIGN
-      REPOSITORY_ROWS_MANAGE
       RESULTS_MANAGE
       RESULTS_DELETE_ARCHIVED
+      TAGS_MANAGE
       PROTOCOL_MANAGE
       COMPLETE
-      UPDATE_STATUS
       STEPS_COMPLETE
       STEPS_UNCOMPLETE
       STEPS_CHECK
@@ -64,8 +59,9 @@ module PermissionExtends
       STEPS_COMMENTS_DELETE_OWN
       STEPS_COMMENTS_UPDATE
       STEPS_COMMENT_UPDATE_OWN
-      REPOSITORY_ROWS_MANAGE
       REPOSITORY_ROWS_ASSIGN
+      REPOSITORY_ROWS_MANAGE
+      USERS_MANAGE
     ).each { |permission| const_set(permission, "task_#{permission.underscore}") }
   end
 

config/routes.rb

@@ -270,7 +270,7 @@ Rails.application.routes.draw do
     resources :projects, except: [:destroy] do
       resources :project_comments,
                 path: '/comments',
-                only: [:create, :index, :edit, :update, :destroy]
+                only: %i(create index update destroy)
       # Activities popup (JSON) for individual project in projects index,
       # as well as all activities page for single project (HTML)
       resources :project_activities, path: '/activities', only: [:index]
@@ -372,7 +372,7 @@ Rails.application.routes.draw do
 
       resources :my_module_comments,
                 path: '/comments',
-                only: [:index, :create, :edit, :update, :destroy]
+                only: %i(create index update destroy)
 
       get :repositories_dropdown_list, controller: :my_module_repositories
       get :repositories_list_html, controller: :my_module_repositories
@@ -438,7 +438,7 @@ Rails.application.routes.draw do
     resources :steps, only: [:edit, :update, :destroy, :show] do
       resources :step_comments,
                 path: '/comments',
-                only: [:create, :index, :edit, :update, :destroy]
+                only: %i(create index update destroy)
       member do
         post 'checklistitem_state'
         post 'toggle_step_state'
@@ -475,7 +475,7 @@ Rails.application.routes.draw do
     resources :results, only: [:update, :destroy] do
       resources :result_comments,
                 path: '/comments',
-                only: [:create, :index, :edit, :update, :destroy]
+                only: %i(create index update destroy)
     end
 
     resources :result_texts, only: [:edit, :update, :destroy]

spec/permissions/controllers/my_module_status_flow_controller_spec.rb

@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe MyModuleStatusFlowController, type: :controller do
+  include PermissionExtends
+
+  it_behaves_like "a controller with authentication", {
+    show: { my_module_id: 1 }
+  }, []
+
+  login_user
+
+  describe 'permissions checking' do
+    include_context 'reference_project_structure', {
+      team_role: :normal_user
+    }
+
+    it_behaves_like "a controller action with permissions checking", :get, :show do
+      let(:testable) { my_module }
+      let(:permissions) { [MyModulePermissions::READ] }
+      let(:action_params) { { my_module_id: my_module.id } }
+    end
+  end
+end

spec/permissions/controllers/my_modules_controller_spec.rb

@@ -0,0 +1,128 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe MyModulesController, type: :controller do
+  include PermissionExtends
+
+  it_behaves_like "a controller with authentication", {
+    show: { id: 1 },
+    description: { id: 1 },
+    status_state: { id: 1 },
+    activities: { id: 1 },
+    activities_tab: { id: 1 },
+    due_date: { id: 1 },
+    update: { id: 1 },
+    update_description: { id: 1 },
+    update_protocol_description: { id: 1 },
+    protocols: { id: 1 },
+    results: { id: 1 },
+    archive: { id: 1 },
+    restore_group: { id: 1 },
+    update_state: { id: 1 }
+  }, []
+
+  login_user
+
+  describe 'permissions checking' do
+    include_context 'reference_project_structure', {
+      team_role: :normal_user
+    }
+
+    it_behaves_like "a controller action with permissions checking", :get, :show do
+      let(:testable) { my_module }
+      let(:permissions) { [MyModulePermissions::READ] }
+      let(:action_params) { { id: my_module.id } }
+    end
+
+    it_behaves_like "a controller action with permissions checking", :get, :description do
+      let(:testable) { my_module }
+      let(:permissions) { [MyModulePermissions::READ] }
+      let(:action_params) { { id: my_module.id } }
+    end
+
+    it_behaves_like "a controller action with permissions checking", :get, :status_state do
+      let(:testable) { my_module }
+      let(:permissions) { [MyModulePermissions::READ] }
+      let(:action_params) { { id: my_module.id } }
+    end
+
+    it_behaves_like "a controller action with permissions checking", :get, :activities do
+      let(:testable) { my_module }
+      let(:permissions) { [MyModulePermissions::READ] }
+      let(:action_params) { { id: my_module.id } }
+    end
+
+    it_behaves_like "a controller action with permissions checking", :get, :activities_tab do
+      let(:testable) { my_module }
+      let(:permissions) { [MyModulePermissions::READ] }
+      let(:action_params) { { id: my_module.id } }
+    end
+
+    it_behaves_like "a controller action with permissions checking", :get, :due_date do
+      let(:testable) { my_module }
+      let(:permissions) { [MyModulePermissions::READ] }
+      let(:action_params) { { id: my_module.id } }
+    end
+
+    it_behaves_like "a controller action with permissions checking", :put, :update do
+      let(:testable) { my_module }
+      let(:permissions) { [MyModulePermissions::MANAGE] }
+      let(:action_params) { { id: my_module.id, my_module: { name: 'Test1' } } }
+    end
+
+    it_behaves_like "a controller action with permissions checking", :put, :update_description do
+      let(:testable) { my_module }
+      let(:permissions) { [MyModulePermissions::MANAGE] }
+      let(:action_params) { { id: my_module.id, my_module: { description: 'Test description' } } }
+    end
+
+    it_behaves_like "a controller action with permissions checking", :put, :update_protocol_description do
+      let(:testable) { my_module }
+      let(:permissions) { [MyModulePermissions::MANAGE] }
+      let(:action_params) { { id: my_module.id, protocol: { description: 'Test description' } } }
+    end
+
+    it_behaves_like "a controller action with permissions checking", :get, :protocols do
+      let(:testable) { my_module }
+      let(:permissions) { [MyModulePermissions::READ] }
+      let(:action_params) { { id: my_module.id } }
+    end
+
+    it_behaves_like "a controller action with permissions checking", :get, :results do
+      let(:testable) { my_module }
+      let(:permissions) { [MyModulePermissions::READ] }
+      let(:action_params) { { id: my_module.id } }
+    end
+
+    it_behaves_like "a controller action with permissions checking", :get, :archive do
+      let(:testable) { my_module }
+      let(:permissions) { [MyModulePermissions::READ] }
+      let(:action_params) { { id: my_module.id } }
+    end
+
+    it_behaves_like "a controller action with permissions checking", :post, :update_state do
+      let(:testable) { my_module }
+      let(:permissions) { [MyModulePermissions::UPDATE_STATUS] }
+      let(:action_params) { { id: my_module.id, my_module: { status_id: my_module.my_module_status_id } } }
+    end
+
+
+    describe 'POST restore_group' do
+      let(:action) { post :restore_group, params: { id: my_module.experiment.id, my_modules_ids: [my_module.id] } }
+
+      context 'when task is not restored' do
+        context 'when user does not have permissions for the task' do
+          it 'task is not restored' do
+            my_module.archive!(user)
+            testable_role = my_module.user_assignments.find_by(user: user ).user_role
+            testable_role.update_column(:permissions, testable_role.permissions - [MyModulePermissions::MANAGE])
+            action
+            expect(response).to have_http_status(302)
+            expect(my_module.reload.archived?).to be_truthy
+          end
+        end
+      end
+    end
+  end
+end

spec/permissions/controllers/user_my_modules_controller_spec.rb

@@ -0,0 +1,54 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe UserMyModulesController, type: :controller do
+  include PermissionExtends
+
+  it_behaves_like "a controller with authentication", {
+    index_old: { my_module_id: 1 },
+    index: { my_module_id: 1 },
+    index_edit: { my_module_id: 1 },
+    create: { my_module_id: 1 },
+    destroy: { my_module_id: 1, id: 1 }
+  }, []
+
+  login_user
+
+  describe 'permissions checking' do
+    include_context 'reference_project_structure', {
+      team_role: :normal_user
+    }
+
+    it_behaves_like "a controller action with permissions checking", :get, :index_old do
+      let(:testable) { my_module }
+      let(:permissions) { [MyModulePermissions::READ] }
+      let(:action_params) { { my_module_id: my_module.id } }
+    end
+
+    it_behaves_like "a controller action with permissions checking", :get, :index do
+      let(:testable) { my_module }
+      let(:permissions) { [MyModulePermissions::READ] }
+      let(:action_params) { { my_module_id: my_module.id } }
+    end
+
+    it_behaves_like "a controller action with permissions checking", :get, :index_edit do
+      let(:testable) { my_module }
+      let(:permissions) { [MyModulePermissions::READ] }
+      let(:action_params) { { my_module_id: my_module.id } }
+    end
+
+    it_behaves_like "a controller action with permissions checking", :post, :create do
+      let(:testable) { my_module }
+      let(:permissions) { [MyModulePermissions::MANAGE] }
+      let(:action_params) { { my_module_id: my_module.id } }
+    end
+
+    it_behaves_like "a controller action with permissions checking", :post, :destroy do
+      let(:testable) { my_module }
+      let(:user_my_module) { UserMyModule.create!(my_module: my_module, user: user) }
+      let(:permissions) { [MyModulePermissions::MANAGE] }
+      let(:action_params) { { my_module_id: my_module.id, id: user_my_module.id } }
+    end
+  end
+end