Merge pull request #13683 from opf/fix/improve_project_allowed_to_sco…

…pe_performance

Fix/improve project allowed to scope performance

app/controllers/placeholder_users_controller.rb

@@ -50,9 +50,14 @@ def index
   end
 
   def show
-    # show projects based on current user visibility
-    @memberships = @placeholder_user.memberships
-      .visible(current_user)
+    # show projects based on current user visibility.
+    # But don't simply concatenate the .visible scope to the memberships
+    # as .memberships has an include and an order which for whatever reason
+    # also gets applied to the Project.allowed_to parts concatenated by a UNION
+    # and an order inside a UNION is not allowed in postgres.
+    @memberships = @placeholder_user
+                     .memberships
+                     .where(id: Member.visible(current_user))
 
     respond_to do |format|
       format.html { render layout: 'no_menu' }

app/controllers/users_controller.rb

@@ -65,10 +65,14 @@ def index
   end
 
   def show
-    # show projects based on current user visibility
+    # show projects based on current user visibility.
+    # But don't simply concatenate the .visible scope to the memberships
+    # as .memberships has an include and an order which for whatever reason
+    # also gets applied to the Project.allowed_to parts concatenated by a UNION
+    # and an order inside a UNION is not allowed in postgres.
     @memberships = @user.memberships
                         .where.not(project_id: nil)
-                        .visible(current_user)
+                        .where(id: Member.visible(current_user))
 
     if can_show_user?
       @events = events

app/models/members/scopes/visible.rb

@@ -43,16 +43,16 @@ def visible(user)
       private
 
       def visible_for_non_admins(user)
-        view_members = Project.where(id: Project.allowed_to(user, :view_members))
-        manage_members = Project.where(id: Project.allowed_to(user, :manage_members))
+        view_members = Project.allowed_to(user, :view_members)
+        manage_members = Project.allowed_to(user, :manage_members)
 
         project_scope = view_members.or(manage_members)
 
-        Member.where(project_id: project_scope.select(:id))
+        where(project_id: project_scope.select(:id))
       end
 
       def visible_for_admins
-        Member.all
+        all
       end
     end
   end

app/models/project.rb

@@ -150,6 +150,9 @@ class Project < ApplicationRecord
 
   friendly_id :identifier, use: :finders
 
+  include ::Scopes::Scoped
+  scopes :allowed_to
+
   scope :has_module, ->(mod) {
     where(["#{Project.table_name}.id IN (SELECT em.project_id FROM #{EnabledModule.table_name} em WHERE em.name=?)", mod.to_s])
   }
@@ -208,12 +211,6 @@ def self.visible_by(user = User.current)
     allowed_to(user, :view_project)
   end
 
-  # Returns a ActiveRecord::Relation to find all projects for which
-  # +user+ has the given +permission+
-  def self.allowed_to(user, permission)
-    Authorization.projects(permission, user)
-  end
-
   # Returns a :conditions SQL string that can be used to find the issues associated with this project.
   #
   # Examples:

app/models/projects/scopes/allowed_to.rb

@@ -0,0 +1,172 @@
+# -- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2010-2023 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# 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 2
+# 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, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+# ++
+
+module Projects::Scopes
+  module AllowedTo
+    extend ActiveSupport::Concern
+
+    class_methods do
+      # Returns an ActiveRecord::Relation to find all projects for which
+      # +user+ has the given +permission+
+      def allowed_to(user, permission)
+        permissions = allowed_to_permissions(permission)
+
+        if user.admin? && permissions.all?(&:grant_to_admin?)
+          where(id: allowed_to_admin_relation(permissions))
+        elsif user.anonymous?
+          where(id: allowed_to_non_member_relation(user, permissions))
+        else
+          where(arel_table[:id].in(allowed_to_member_relation(user, permissions)
+                                     .arel
+                                     .union(:all, allowed_to_non_member_relation(user, permissions).arel)))
+        end
+      end
+
+      private
+
+      def allowed_to_admin_relation(permissions)
+        joins(allowed_to_enabled_module_join(permissions))
+          .where(arel_table[:active].eq(true))
+      end
+
+      def allowed_to_non_member_relation(user, permissions)
+        joins(allowed_to_enabled_module_join(permissions))
+          .joins(allowed_to_builtin_roles_in_active_project_join(user))
+          .joins(allowed_to_role_permission_join(permissions))
+          .select(:id)
+      end
+
+      def allowed_to_member_relation(user, permissions)
+        Member
+          .joins(allowed_to_member_in_active_project_join(user))
+          .joins(allowed_to_enabled_module_join(permissions))
+          .joins(:roles, :member_roles)
+          .joins(allowed_to_role_permission_join(permissions))
+          .select('projects.id')
+      end
+
+      def allowed_to_enabled_module_join(permissions)
+        project_module = permissions.filter_map(&:project_module).uniq
+        enabled_module_table = EnabledModule.arel_table
+
+        if project_module.any?
+          arel_table.join(enabled_module_table, Arel::Nodes::InnerJoin)
+                    .on(arel_table[:id].eq(enabled_module_table[:project_id])
+                          .and(enabled_module_table[:name].in(project_module))
+                          .and(arel_table[:active].eq(true)))
+                    .join_sources
+        end
+      end
+
+      def allowed_to_role_permission_join(permissions)
+        return if permissions.all?(&:public?)
+
+        role_permissions_table = RolePermission.arel_table
+        enabled_modules_table = EnabledModule.arel_table
+        roles_table = Role.arel_table
+
+        condition = permissions.inject(Arel::Nodes::False.new) do |or_condition, permission|
+          permission_condition = role_permissions_table[:permission].eq(permission.name)
+
+          if permission.project_module.present?
+            permission_condition = permission_condition.and(enabled_modules_table[:name].eq(permission.project_module))
+          end
+
+          or_condition.or(permission_condition)
+        end
+
+        arel_table
+          .join(role_permissions_table, Arel::Nodes::InnerJoin)
+          .on(roles_table[:id].eq(role_permissions_table[:role_id])
+                              .and(condition))
+          .join_sources
+      end
+
+      def allowed_to_permissions(permission)
+        if permission.is_a?(Hash)
+          OpenProject::AccessControl.allow_actions(permission)
+        else
+          [OpenProject::AccessControl.permission(permission)].compact
+        end
+      end
+
+      def allowed_to_members_condition(user)
+        members_table = Member.arel_table
+
+        members_table[:project_id].eq(arel_table[:id])
+                                  .and(members_table[:user_id].eq(user.id))
+                                  .and(members_table[:entity_type].eq(nil))
+                                  .and(members_table[:entity_id].eq(nil))
+      end
+
+      def allowed_to_builtin_roles_in_active_project_join(user)
+        condition = allowed_to_built_roles_in_active_project_condition(user)
+
+        if user.logged?
+          condition = condition.and(allowed_to_no_member_exists_condition(user))
+        end
+
+        roles_table = Role.arel_table
+
+        arel_table.join(roles_table)
+                  .on(condition)
+                  .join_sources
+      end
+
+      def allowed_to_member_in_active_project_join(user)
+        arel_table.join(arel_table)
+                  .on(arel_table[:active].eq(true)
+                                         .and(allowed_to_members_condition(user)))
+                  .join_sources
+      end
+
+      def allowed_to_built_roles_in_active_project_condition(user)
+        builtin = if user.logged?
+                    Role::BUILTIN_NON_MEMBER
+                  else
+                    Role::BUILTIN_ANONYMOUS
+                  end
+
+        roles_table = Role.arel_table
+
+        roles_table[:builtin].eq(builtin)
+                             .and(arel_table[:active])
+                             .and(arel_table[:public])
+      end
+
+      def allowed_to_no_member_exists_condition(user)
+        Member
+          .select(1)
+          .where(allowed_to_members_condition(user))
+          .arel
+          .exists
+          .not
+      end
+    end
+  end
+end

app/services/authorization.rb

@@ -36,7 +36,7 @@ def users(action, project)
 
   # Returns all projects a user has a certain permission in
   def projects(action, user)
-    Authorization::ProjectQuery.query(user, action)
+    Project.allowed_to(action, user)
   end
 
   # Returns all roles a user has in a certain project, for a specific entity or globally

app/views/individual_principals/_memberships.html.erb

@@ -31,7 +31,7 @@ See COPYRIGHT and LICENSE files for more details.
   .new(@individual_principal.memberships.build, current_user)
   .assignable_projects
   .order(Arel.sql('lft')) %>
-<% memberships = @individual_principal.memberships.visible(current_user) %>
+<% memberships = @individual_principal.memberships.where(id: Member.visible(current_user)) %>
 
 <div class="grid-block">
   <div class="grid-content">