Add latest changes from gitlab-org/gitlab@master

10 files changed, 120 insertions, 46 deletions

diff --git a/app/controllers/concerns/uploads_actions.rb b/app/controllers/concerns/uploads_actions.rb
index db756ae336f..0d64a685065 100644
--- a/app/controllers/concerns/uploads_actions.rb
+++ b/app/controllers/concerns/uploads_actions.rb
@@ -10,6 +10,10 @@ module UploadsActions
   included do
     prepend_before_action :set_request_format_from_path_extension
     rescue_from FileUploader::InvalidSecret, with: :render_404
+
+    rescue_from ::Gitlab::Utils::PathTraversalAttackError do
+      head :bad_request
+    end
   end
 
   def create
@@ -33,6 +37,8 @@ module UploadsActions
   #   - or redirect to its URL
   #
   def show
+    Gitlab::Utils.check_path_traversal!(params[:filename])
+
     return render_404 unless uploader&.exists?
 
     ttl, directives = *cache_settings
diff --git a/app/finders/releases_finder.rb b/app/finders/releases_finder.rb
index 78240e0a050..16d3f46e1af 100644
--- a/app/finders/releases_finder.rb
+++ b/app/finders/releases_finder.rb
@@ -6,7 +6,7 @@ class ReleasesFinder
   attr_reader :parent, :current_user, :params
 
   def initialize(parent, current_user = nil, params = {})
-    @parent = parent
+    @parent = Array.wrap(parent)
     @current_user = current_user
     @params = params
 
@@ -15,7 +15,7 @@ class ReleasesFinder
   end
 
   def execute(preload: true)
-    return Release.none if projects.empty?
+    return Release.none if authorized_projects.empty?
 
     releases = get_releases
     releases = by_tag(releases)
@@ -26,16 +26,17 @@ class ReleasesFinder
   private
 
   def get_releases
-    Release.where(project_id: projects).where.not(tag: nil) # rubocop: disable CodeReuse/ActiveRecord
+    Release.where(project_id: authorized_projects).where.not(tag: nil) # rubocop: disable CodeReuse/ActiveRecord
   end
 
-  def projects
-    strong_memoize(:projects) do
-      if parent.is_a?(Project)
-        Ability.allowed?(current_user, :read_release, parent) ? [parent] : []
-      end
-    end
+  def authorized_projects
+    # Preload policy for all projects to avoid N+1 queries
+    projects = Project.id_in(parent.map(&:id)).include_project_feature
+    Preloaders::ProjectPolicyPreloader.new(projects, current_user).execute
+
+    projects.select { |project| authorized?(project) }
   end
+  strong_memoize_attr :authorized_projects
 
   # rubocop: disable CodeReuse/ActiveRecord
   def by_tag(releases)
@@ -48,4 +49,8 @@ class ReleasesFinder
   def order_releases(releases)
     releases.sort_by_attribute("#{params[:order_by]}_#{params[:sort]}")
   end
+
+  def authorized?(project)
+    Ability.allowed?(current_user, :read_release, project)
+  end
 end
diff --git a/app/graphql/resolvers/releases_resolver.rb b/app/graphql/resolvers/releases_resolver.rb
index 06f4ca2065c..8c9235c2b5f 100644
--- a/app/graphql/resolvers/releases_resolver.rb
+++ b/app/graphql/resolvers/releases_resolver.rb
@@ -8,8 +8,6 @@ module Resolvers
              required: false, default_value: :released_at_desc,
              description: 'Sort releases by given criteria.'
 
-    alias_method :project, :object
-
     # This resolver has a custom singular resolver
     def self.single
       Resolvers::ReleaseResolver
@@ -23,11 +21,24 @@ module Resolvers
     }.freeze
 
     def resolve(sort:)
-      ReleasesFinder.new(
-        project,
-        current_user,
-        SORT_TO_PARAMS_MAP[sort]
-      ).execute
+      BatchLoader::GraphQL.for(project).batch do |projects, loader|
+        releases = ReleasesFinder.new(
+          projects,
+          current_user,
+          SORT_TO_PARAMS_MAP[sort]
+        ).execute
+
+        # group_by will not cause N+1 queries here because ReleasesFinder preloads projects
+        releases.group_by(&:project).each do |project, versions|
+          loader.call(project, versions)
+        end
+      end
+    end
+
+    private
+
+    def project
+      object.respond_to?(:project) ? object.project : object
     end
   end
 end
diff --git a/app/graphql/types/ci/catalog/resource_type.rb b/app/graphql/types/ci/catalog/resource_type.rb
index b5947826fa1..e4566aac9aa 100644
--- a/app/graphql/types/ci/catalog/resource_type.rb
+++ b/app/graphql/types/ci/catalog/resource_type.rb
@@ -20,6 +20,11 @@ module Types
 
         field :icon, GraphQL::Types::String, null: true, description: 'Icon for the catalog resource.',
           method: :avatar_path, alpha: { milestone: '15.11' }
+
+        field :versions, Types::ReleaseType.connection_type, null: true,
+          description: 'Versions of the catalog resource.',
+          resolver: Resolvers::ReleasesResolver,
+          alpha: { milestone: '16.1' }
       end
       # rubocop: enable Graphql/AuthorizeTypes
     end
diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index 52a6cbdafad..a36ceb77d8f 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -57,7 +57,7 @@ class Namespace < ApplicationRecord
   # This should _not_ be `inverse_of: :namespace`, because that would also set
   # `user.namespace` when this user creates a group with themselves as `owner`.
   belongs_to :owner, class_name: 'User'
-  belongs_to :organization
+  belongs_to :organization, class_name: 'Organizations::Organization'
 
   belongs_to :parent, class_name: "Namespace"
   has_many :children, -> { where(type: Group.sti_name) }, class_name: "Namespace", foreign_key: :parent_id
diff --git a/app/models/organization.rb b/app/models/organization.rb
deleted file mode 100644
index 0d03d95b5ff..00000000000
--- a/app/models/organization.rb
+++ /dev/null
@@ -1,29 +0,0 @@
-# frozen_string_literal: true
-
-class Organization < ApplicationRecord
-  DEFAULT_ORGANIZATION_ID = 1
-
-  scope :without_default, -> { where.not(id: DEFAULT_ORGANIZATION_ID) }
-
-  before_destroy :check_if_default_organization
-
-  has_many :namespaces
-  has_many :groups
-
-  validates :name,
-    presence: true,
-    length: { maximum: 255 },
-    uniqueness: { case_sensitive: false }
-
-  def default?
-    id == DEFAULT_ORGANIZATION_ID
-  end
-
-  private
-
-  def check_if_default_organization
-    return unless default?
-
-    raise ActiveRecord::RecordNotDestroyed, _('Cannot delete the default organization')
-  end
-end
diff --git a/app/models/organizations/organization.rb b/app/models/organizations/organization.rb
new file mode 100644
index 00000000000..425869b1bb0
--- /dev/null
+++ b/app/models/organizations/organization.rb
@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+module Organizations
+  class Organization < ApplicationRecord
+    DEFAULT_ORGANIZATION_ID = 1
+
+    scope :without_default, -> { where.not(id: DEFAULT_ORGANIZATION_ID) }
+
+    before_destroy :check_if_default_organization
+
+    has_many :namespaces
+    has_many :groups
+
+    validates :name,
+      presence: true,
+      length: { maximum: 255 },
+      uniqueness: { case_sensitive: false }
+
+    def default?
+      id == DEFAULT_ORGANIZATION_ID
+    end
+
+    private
+
+    def check_if_default_organization
+      return unless default?
+
+      raise ActiveRecord::RecordNotDestroyed, _('Cannot delete the default organization')
+    end
+  end
+end
diff --git a/app/uploaders/object_storage.rb b/app/uploaders/object_storage.rb
index e1e103e20f8..70f7625ffbd 100644
--- a/app/uploaders/object_storage.rb
+++ b/app/uploaders/object_storage.rb
@@ -426,6 +426,8 @@ module ObjectStorage
     end
 
     def retrieve_from_store!(identifier)
+      Gitlab::Utils.check_path_traversal!(identifier)
+
       # We need to force assign the value of @filename so that we will still
       # get the original_filename in cases wherein the file points to a random generated
       # path format. This happens for direct uploaded files to final location.
diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml
index 16ab9b625bf..06e07a59311 100644
--- a/app/workers/all_queues.yml
+++ b/app/workers/all_queues.yml
@@ -3027,6 +3027,15 @@
   :weight: 1
   :idempotent: true
   :tags: []
+- :name: merge_requests_mergeability_check_batch
+  :worker_name: MergeRequests::MergeabilityCheckBatchWorker
+  :feature_category: :code_review_workflow
+  :has_external_dependencies: false
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent: true
+  :tags: []
 - :name: merge_requests_resolve_todos
   :worker_name: MergeRequests::ResolveTodosWorker
   :feature_category: :code_review_workflow
diff --git a/app/workers/merge_requests/mergeability_check_batch_worker.rb b/app/workers/merge_requests/mergeability_check_batch_worker.rb
new file mode 100644
index 00000000000..cbe34ac3790
--- /dev/null
+++ b/app/workers/merge_requests/mergeability_check_batch_worker.rb
@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+module MergeRequests
+  class MergeabilityCheckBatchWorker
+    include ApplicationWorker
+
+    data_consistency :sticky
+
+    sidekiq_options retry: 3
+
+    feature_category :code_review_workflow
+    idempotent!
+
+    def logger
+      @logger ||= Sidekiq.logger
+    end
+
+    def perform(merge_request_ids)
+      merge_requests = MergeRequest.id_in(merge_request_ids)
+
+      merge_requests.each do |merge_request|
+        result = merge_request.check_mergeability
+
+        next unless result&.error?
+
+        logger.error(
+          worker: self.class.name,
+          message: "Failed to check mergeability of merge request: #{result.message}",
+          merge_request_id: merge_request.id
+        )
+      end
+    end
+  end
+end