Merge pull request #8674 from andrej-scinote/aj_SCI_11958

Fix permission scopes [SCI-11958]

app/controllers/experiments_controller.rb

@@ -202,7 +202,7 @@ class ExperimentsController < ApplicationController
 
   def projects_to_clone
     projects = @experiment.project.team.projects.active
-                          .with_user_permission(current_user, ProjectPermissions::EXPERIMENTS_CREATE)
+                          .with_granted_permissions(current_user, ProjectPermissions::EXPERIMENTS_CREATE)
                           .where('trim_html_tags(projects.name) ILIKE ?',
                                  "%#{ActiveRecord::Base.sanitize_sql_like(params['query'])}%")
                           .map { |p| [p.id, p.name] }

app/models/concerns/assignable.rb

@@ -24,34 +24,50 @@ module Assignable
              class_name: 'TeamAssignment',
              inverse_of: :assignable
 
-    scope :readable_by_user, lambda { |user|
+    scope :readable_by_user, lambda { |user, teams = user.permission_team|
-      joins("INNER JOIN user_assignments reading_user_assignments " \
+      read_permission = "::#{self.class.to_s.split('::').first}Permissions".constantize::READ
-            "ON reading_user_assignments.assignable_type = '#{base_class.name}' " \
+      with_user_assignments = joins(user_assignments: :user_role)
-            "AND reading_user_assignments.assignable_id = #{table_name}.id " \
+                              .where(user_assignments: { user: user, team: teams })
-            "INNER JOIN user_roles reading_user_roles " \
+
-            "ON reading_user_assignments.user_role_id = reading_user_roles.id")
+      # direct user assignments take precedence over group assignments, thus skipping objects that already have user assignments.
-        .where(reading_user_assignments: { user_id: user.id })
+      with_group_assignments = left_outer_joins(user_group_assignments: [:user_role, { user_group: :users }], team_assignments: :user_role)
-        .where('? = ANY(reading_user_roles.permissions)', "::#{self.class.to_s.split('::').first}Permissions".constantize::READ)
+                               .where.not(id: with_user_assignments)
+
+      with_granted_user_permissions = with_user_assignments.where('? = ANY(user_roles.permissions)', read_permission)
+      with_granted_group_permissions = with_group_assignments
+                                       .where(user_group_assignments: { assignable: self, user_groups: { users: user } })
+                                       .where('? = ANY(user_roles.permissions)', read_permission)
+                                       .or(
+                                         with_group_assignments
+                                         .where(team_assignments: { assignable: self, team: teams })
+                                         .where('? = ANY(user_roles.permissions)', read_permission)
+                                       )
+                                       .distinct
+      where(id: with_granted_user_permissions.reselect(:id))
+        .or(where(id: with_granted_group_permissions.reselect(:id)))
     }
 
-    scope :managable_by_user, lambda { |user|
+    scope :managable_by_user, lambda { |user, teams = user.permission_team|
-      joins("INNER JOIN user_assignments managing_user_assignments " \
+      manage_permission = "::#{self.class.to_s.split('::').first}Permissions".constantize::MANAGE
-            "ON managing_user_assignments.assignable_type = '#{base_class.name}' " \
+      with_user_assignments = joins(user_assignments: :user_role)
-            "AND managing_user_assignments.assignable_id = #{table_name}.id " \
+                              .where(user_assignments: { user: user, team: teams })
-            "INNER JOIN user_roles managing_user_roles " \
-            "ON managing_user_assignments.user_role_id = managing_user_roles.id")
-        .where(managing_user_assignments: { user_id: user.id })
-        .where('? = ANY(managing_user_roles.permissions)', "::#{self.class.to_s.split('::').first}Permissions".constantize::MANAGE)
-    }
 
-    scope :with_user_permission, lambda { |user, permission|
+      # direct user assignments take precedence over group assignments, thus skipping objects that already have user assignments.
-      joins("INNER JOIN user_assignments permission_checking_user_assignments " \
+      with_group_assignments = left_outer_joins(user_group_assignments: [:user_role, { user_group: :users }], team_assignments: :user_role)
-            "ON permission_checking_user_assignments.assignable_type = '#{base_class.name}' " \
+                               .where.not(id: with_user_assignments)
-            "AND permission_checking_user_assignments.assignable_id = #{table_name}.id " \
+
-            "INNER JOIN user_roles permission_checking_user_roles " \
+      with_granted_user_permissions = with_user_assignments.where('? = ANY(user_roles.permissions)', manage_permission)
-            "ON permission_checking_user_assignments.user_role_id = permission_checking_user_roles.id")
+      with_granted_group_permissions = with_group_assignments
-        .where(permission_checking_user_assignments: { user_id: user.id })
+                                       .where(user_group_assignments: { assignable: self, user_groups: { users: user } })
-        .where('? = ANY(permission_checking_user_roles.permissions)', permission)
+                                       .where('? = ANY(user_roles.permissions)', manage_permission)
+                                       .or(
+                                         with_group_assignments
+                                         .where(team_assignments: { assignable: self, team: teams })
+                                         .where('? = ANY(user_roles.permissions)', manage_permission)
+                                       )
+                                       .distinct
+      where(id: with_granted_user_permissions.reselect(:id))
+        .or(where(id: with_granted_group_permissions.reselect(:id)))
     }
 
     after_create :create_users_assignments

app/models/concerns/shareable.rb

@@ -27,7 +27,11 @@ module Shareable
     end
 
     scope :viewable_by_user, lambda { |user, teams = user.current_team|
-      readable_ids = with_granted_permissions(user, "#{permission_class.name}Permissions::READ".constantize, teams).pluck(:id)
+      readable_ids = if permission_class == StorageLocation
+                       readable_by_user(user).where(team: teams).pluck(:id)
+                     else
+                       with_granted_permissions(user, "#{permission_class.name}Permissions::READ".constantize, teams).pluck(:id)
+                     end
       shared_with_team_ids = joins(:team_shared_objects, :team).where(team_shared_objects: { team: teams }).pluck(:id)
       globally_shared_ids =
         if column_names.include?('permission_level')

app/models/storage_location.rb

@@ -38,6 +38,10 @@ class StorageLocation < ApplicationRecord
     storage_location_repository_rows.each(&:discard)
   end
 
+  def self.permission_class
+    StorageLocation
+  end
+
   def shared_with?(team)
     return false if self.team == team
 

app/models/user_group_assignment.rb

@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
 class UserGroupAssignment < ApplicationRecord
+  before_validation :set_assignable_team
+
   belongs_to :assignable, polymorphic: true, touch: true
   belongs_to :team
   belongs_to :user_group
@@ -21,4 +23,10 @@ class UserGroupAssignment < ApplicationRecord
   def user_group_name_with_role
     "#{user_group.name} - #{user_role.name}"
   end
+
+  private
+
+  def set_assignable_team
+    self.team = assignable.team
+  end
 end

app/permissions/experiment.rb

@@ -62,8 +62,7 @@ Canaid::Permissions.register_for(Experiment) do
   end
 
   can :manage_all_experiment_my_modules do |user, experiment|
-    experiment.my_modules.where.not(id: experiment.my_modules.with_user_permission(user,
+    experiment.my_modules.where.not(id: experiment.my_modules.with_granted_permissions(user, MyModulePermissions::MANAGE)).none?
-                                                                                   MyModulePermissions::MANAGE)).none?
   end
 
   can :archive_experiment do |user, experiment|

app/services/activities_service.rb

@@ -37,7 +37,7 @@ class ActivitiesService
     visible_my_modules = MyModule.viewable_by_user(user, teams)
     visible_forms = Form.viewable_by_user(user, teams)
     # Temporary solution until handling of deleted subjects is fully implemented
-    visible_repository_teams = user.teams.where(id: teams).with_user_permission(user, RepositoryPermissions::READ)
+    visible_repository_teams = user.teams.with_granted_permissions(user, RepositoryPermissions::READ, teams)
 
     activities = Activity.from(activities, 'activities')
     activities = activities.where(project: nil, team_id: teams).where.not(subject_type: %w(RepositoryBase RepositoryRow Protocol Form))

app/views/global_activities/references/_repository_base.html.erb

@@ -2,13 +2,15 @@
            locals: { team: team, subject: team, breadcrumbs: breadcrumbs, values: values, type_of: type_of } %>
 <div class="ga-breadcrumb">
   <span class="sn-icon sn-icon-inventory"></span>
-  <%if !can_read_repository?(subject)%>
+  <% if subject %>
-    <%= I18n.t('repositories.private') %>
+    <% if can_read_repository?(subject) %>
-  <% elsif subject %>
+      <%= route_to_other_team(repository_path(subject.id, team: subject.team.id),
-    <%= route_to_other_team(repository_path(subject.id, team: subject.team.id),
+                              team,
-                            team,
+                              subject.name&.truncate(Constants::NAME_TRUNCATION_LENGTH),
-                            subject.name&.truncate(Constants::NAME_TRUNCATION_LENGTH),
+                              title: subject.name) %>
-                            title: subject.name) %>
+    <% else %>
+       <%= I18n.t('repositories.private') %>
+    <% end %>
   <% else %>
     <span title="<%= breadcrumbs['repository'] %>">
       <%= breadcrumbs['repository']&.truncate(Constants::NAME_TRUNCATION_LENGTH) %>

app/views/global_activities/references/_repository_row.html.erb

@@ -2,18 +2,24 @@
            locals: { team: team, subject: team, breadcrumbs: breadcrumbs, values: values, type_of: type_of } %>
 <div class="ga-breadcrumb">
   <span class="sn-icon sn-icon-inventory"></span>
-  <%if !can_read_repository?(subject.repository)%>
+  <% if subject %>
-    <%= I18n.t('repositories.private') %>
+    <% if can_read_repository?(subject.repository) %>
-  <% elsif subject %>
+      <%= route_to_other_team(repository_path(subject.repository.id, team: subject.repository.team.id),
-    <%= route_to_other_team(repository_path(subject.repository.id, team: subject.repository.team.id),
+                              team,
-                            team,
+                              subject.repository.name&.truncate(Constants::NAME_TRUNCATION_LENGTH),
-                            subject.repository.name&.truncate(Constants::NAME_TRUNCATION_LENGTH),
+                              title: subject.repository.name) %>
-                            title: subject.repository.name) %>
+    <% else %>
+      <%= I18n.t('repositories.private') %>
+    <% end %>
   <% elsif repository = Repository.find_by(id: values.dig('message_items', 'repository', 'id')) %>
-    <%= route_to_other_team(repository_path(repository.id, team: repository.team.id),
+    <% if can_read_repository?(repository) %>
-                        team,
+      <%= route_to_other_team(repository_path(repository.id, team: repository.team.id),
-                        repository.name&.truncate(Constants::NAME_TRUNCATION_LENGTH),
+                          team,
-                        title: repository.name) %>
+                          repository.name&.truncate(Constants::NAME_TRUNCATION_LENGTH),
+                          title: repository.name) %>
+    <% else %>
+      <%= I18n.t('repositories.private') %>
+    <% end %>
   <% else %>
     <span title="<%= breadcrumbs['repository'] %>">
       <%= breadcrumbs['repository']&.truncate(Constants::NAME_TRUNCATION_LENGTH) %>