Add latest changes from gitlab-org/security/gitlab@14-4-stable-ee

14 files changed, 94 insertions, 39 deletions

diff --git a/app/models/concerns/bulk_member_access_load.rb b/app/models/concerns/bulk_member_access_load.rb
index e252ca36629..927d6ccb28f 100644
--- a/app/models/concerns/bulk_member_access_load.rb
+++ b/app/models/concerns/bulk_member_access_load.rb
@@ -9,11 +9,15 @@ module BulkMemberAccessLoad
     # Determine the maximum access level for a group of resources in bulk.
     #
     # Returns a Hash mapping resource ID -> maximum access level.
-    def max_member_access_for_resource_ids(resource_klass, resource_ids, memoization_index = self.id, &block)
+    def max_member_access_for_resource_ids(resource_klass, resource_ids, &block)
       raise 'Block is mandatory' unless block_given?
 
+      memoization_index = self.id
+      memoization_class = self.class
+
       resource_ids = resource_ids.uniq
-      access = load_access_hash(resource_klass, memoization_index)
+      memo_id = "#{memoization_class}:#{memoization_index}"
+      access = load_access_hash(resource_klass, memo_id)
 
       # Look up only the IDs we need
       resource_ids -= access.keys
@@ -33,8 +37,8 @@ module BulkMemberAccessLoad
       access
     end
 
-    def merge_value_to_request_store(resource_klass, resource_id, memoization_index, value)
-      max_member_access_for_resource_ids(resource_klass, [resource_id], memoization_index) do
+    def merge_value_to_request_store(resource_klass, resource_id, value)
+      max_member_access_for_resource_ids(resource_klass, [resource_id]) do
         { resource_id => value }
       end
     end
@@ -45,16 +49,13 @@ module BulkMemberAccessLoad
       "max_member_access_for_#{klass.name.underscore.pluralize}:#{memoization_index}"
     end
 
-    def load_access_hash(resource_klass, memoization_index)
-      key = max_member_access_for_resource_key(resource_klass, memoization_index)
+    def load_access_hash(resource_klass, memo_id)
+      return {} unless Gitlab::SafeRequestStore.active?
 
-      access = {}
-      if Gitlab::SafeRequestStore.active?
-        Gitlab::SafeRequestStore[key] ||= {}
-        access = Gitlab::SafeRequestStore[key]
-      end
+      key = max_member_access_for_resource_key(resource_klass, memo_id)
+      Gitlab::SafeRequestStore[key] ||= {}
 
-      access
+      Gitlab::SafeRequestStore[key]
     end
   end
 end
diff --git a/app/models/preloaders/user_max_access_level_in_groups_preloader.rb b/app/models/preloaders/user_max_access_level_in_groups_preloader.rb
index 14f1d271572..8ef8b9763a4 100644
--- a/app/models/preloaders/user_max_access_level_in_groups_preloader.rb
+++ b/app/models/preloaders/user_max_access_level_in_groups_preloader.rb
@@ -5,8 +5,6 @@ module Preloaders
   # stores the values in requests store.
   # Will only be able to preload max access level for groups where the user is a direct member
   class UserMaxAccessLevelInGroupsPreloader
-    include BulkMemberAccessLoad
-
     def initialize(groups, user)
       @groups = groups
       @user = user
@@ -19,8 +17,9 @@ module Preloaders
                                      .group(:source_id)
                                      .maximum(:access_level)
 
-      group_memberships.each do |group_id, max_access_level|
-        merge_value_to_request_store(User, @user.id, group_id, max_access_level)
+      @groups.each do |group|
+        access_level = group_memberships[group.id]
+        group.merge_value_to_request_store(User, @user.id, access_level) if access_level.present?
       end
     end
   end
diff --git a/app/models/project.rb b/app/models/project.rb
index 2ceba10e86e..6a5cf00aba1 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -37,6 +37,7 @@ class Project < ApplicationRecord
   include Repositories::CanHousekeepRepository
   include EachBatch
   include GitlabRoutingHelper
+  include BulkMemberAccessLoad
 
   extend Gitlab::Cache::RequestCache
   extend Gitlab::Utils::Override
diff --git a/app/models/project_team.rb b/app/models/project_team.rb
index 774d81156b7..b40c4366c2a 100644
--- a/app/models/project_team.rb
+++ b/app/models/project_team.rb
@@ -1,8 +1,6 @@
 # frozen_string_literal: true
 
 class ProjectTeam
-  include BulkMemberAccessLoad
-
   attr_accessor :project
 
   def initialize(project)
@@ -169,7 +167,7 @@ class ProjectTeam
   #
   # Returns a Hash mapping user ID -> maximum access level.
   def max_member_access_for_user_ids(user_ids)
-    max_member_access_for_resource_ids(User, user_ids, project.id) do |user_ids|
+    project.max_member_access_for_resource_ids(User, user_ids) do |user_ids|
       project.project_authorizations
              .where(user: user_ids)
              .group(:user_id)
@@ -178,7 +176,7 @@ class ProjectTeam
   end
 
   def write_member_access_for_user_id(user_id, project_access_level)
-    merge_value_to_request_store(User, user_id, project.id, project_access_level)
+    project.merge_value_to_request_store(User, user_id, project_access_level)
   end
 
   def max_member_access(user_id)
diff --git a/lib/api/lint.rb b/lib/api/lint.rb
index fa871b4bc57..d7d1f52c02e 100644
--- a/lib/api/lint.rb
+++ b/lib/api/lint.rb
@@ -4,6 +4,16 @@ module API
   class Lint < ::API::Base
     feature_category :pipeline_authoring
 
+    helpers do
+      def can_lint_ci?
+        signup_unrestricted = Gitlab::CurrentSettings.signup_enabled? && !Gitlab::CurrentSettings.signup_limited?
+        internal_user = current_user.present? && !current_user.external?
+        is_developer = current_user.present? && current_user.projects.any? { |p| p.team.member?(current_user, Gitlab::Access::DEVELOPER) }
+
+        signup_unrestricted || internal_user || is_developer
+      end
+    end
+
     namespace :ci do
       desc 'Validation of .gitlab-ci.yml content'
       params do
@@ -11,7 +21,7 @@ module API
         optional :include_merged_yaml, type: Boolean, desc: 'Whether or not to include merged CI config yaml in the response'
       end
       post '/lint' do
-        unauthorized! if (Gitlab::CurrentSettings.signup_disabled? || Gitlab::CurrentSettings.signup_limited?) && current_user.nil?
+        unauthorized! unless can_lint_ci?
 
         result = Gitlab::Ci::Lint.new(project: nil, current_user: current_user)
           .validate(params[:content], dry_run: false)
diff --git a/lib/banzai/filter/front_matter_filter.rb b/lib/banzai/filter/front_matter_filter.rb
index d47900b816a..705400a5497 100644
--- a/lib/banzai/filter/front_matter_filter.rb
+++ b/lib/banzai/filter/front_matter_filter.rb
@@ -9,7 +9,7 @@ module Banzai
         html.sub(Gitlab::FrontMatter::PATTERN) do |_match|
           lang = $~[:lang].presence || lang_mapping[$~[:delim]]
 
-          ["```#{lang}:frontmatter", $~[:front_matter], "```", "\n"].join("\n")
+          ["```#{lang}:frontmatter", $~[:front_matter].strip!, "```", "\n"].join("\n")
         end
       end
     end
diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb
index bfe3f06a56b..ab55a592ded 100644
--- a/lib/gitlab/current_settings.rb
+++ b/lib/gitlab/current_settings.rb
@@ -8,7 +8,7 @@ module Gitlab
       end
 
       def signup_limited?
-        domain_allowlist.present? || email_restrictions_enabled? || require_admin_approval_after_user_signup?
+        domain_allowlist.present? || email_restrictions_enabled? || require_admin_approval_after_user_signup? || user_default_external?
       end
 
       def current_application_settings
diff --git a/lib/gitlab/front_matter.rb b/lib/gitlab/front_matter.rb
index 7612bd36aca..5c5c74ca1a0 100644
--- a/lib/gitlab/front_matter.rb
+++ b/lib/gitlab/front_matter.rb
@@ -11,13 +11,11 @@ module Gitlab
     DELIM = Regexp.union(DELIM_LANG.keys)
 
     PATTERN = %r{
-      \A(?:[^\r\n]*coding:[^\r\n]*)?         # optional encoding line
+      \A(?:[^\r\n]*coding:[^\r\n]*\R)?        # optional encoding line
       \s*
-      ^(?<delim>#{DELIM})[ \t]*(?<lang>\S*)  # opening front matter marker (optional language specifier)
-      \s*
-      ^(?<front_matter>.*?)                  # front matter block content (not greedy)
-      \s*
-      ^(\k<delim> | \.{3})                   # closing front matter marker
+      ^(?<delim>#{DELIM})[ \t]*(?<lang>\S*)\R # opening front matter marker (optional language specifier)
+      (?<front_matter>.*?)                    # front matter block content (not greedy)
+      ^(\k<delim> | \.{3})                    # closing front matter marker
       \s*
     }mx.freeze
   end
diff --git a/lib/gitlab/wiki_pages/front_matter_parser.rb b/lib/gitlab/wiki_pages/front_matter_parser.rb
index 45dc6cf7fd1..0ceec39782c 100644
--- a/lib/gitlab/wiki_pages/front_matter_parser.rb
+++ b/lib/gitlab/wiki_pages/front_matter_parser.rb
@@ -54,7 +54,7 @@ module Gitlab
 
         def initialize(delim = nil, lang = '', text = nil)
           @lang = lang.downcase.presence || Gitlab::FrontMatter::DELIM_LANG[delim]
-          @text = text
+          @text = text&.strip!
         end
 
         def data
diff --git a/spec/lib/banzai/filter/front_matter_filter_spec.rb b/spec/lib/banzai/filter/front_matter_filter_spec.rb
index cef6a2ddcce..1562c388296 100644
--- a/spec/lib/banzai/filter/front_matter_filter_spec.rb
+++ b/spec/lib/banzai/filter/front_matter_filter_spec.rb
@@ -139,4 +139,20 @@ RSpec.describe Banzai::Filter::FrontMatterFilter do
       end
     end
   end
+
+  it 'fails fast for strings with many spaces' do
+    content = "coding:" + " " * 50_000 + ";"
+
+    expect do
+      Timeout.timeout(3.seconds) { filter(content) }
+    end.not_to raise_error
+  end
+
+  it 'fails fast for strings with many newlines' do
+    content = "coding:\n" + ";;;" + "\n" * 10_000 + "x"
+
+    expect do
+      Timeout.timeout(3.seconds) { filter(content) }
+    end.not_to raise_error
+  end
 end
diff --git a/spec/lib/gitlab/current_settings_spec.rb b/spec/lib/gitlab/current_settings_spec.rb
index a5ab1047a40..46c33d7b7b2 100644
--- a/spec/lib/gitlab/current_settings_spec.rb
+++ b/spec/lib/gitlab/current_settings_spec.rb
@@ -51,9 +51,17 @@ RSpec.describe Gitlab::CurrentSettings do
       it { is_expected.to be_truthy }
     end
 
+    context 'when new users are set to external' do
+      before do
+        create(:application_setting, user_default_external: true)
+      end
+
+      it { is_expected.to be_truthy }
+    end
+
     context 'when there are no restrictions' do
       before do
-        create(:application_setting, domain_allowlist: [], email_restrictions_enabled: false, require_admin_approval_after_user_signup: false)
+        create(:application_setting, domain_allowlist: [], email_restrictions_enabled: false, require_admin_approval_after_user_signup: false, user_default_external: false)
       end
 
       it { is_expected.to be_falsey }
diff --git a/spec/lib/gitlab/wiki_pages/front_matter_parser_spec.rb b/spec/lib/gitlab/wiki_pages/front_matter_parser_spec.rb
index c78103f33f4..3152dc2ad2f 100644
--- a/spec/lib/gitlab/wiki_pages/front_matter_parser_spec.rb
+++ b/spec/lib/gitlab/wiki_pages/front_matter_parser_spec.rb
@@ -118,7 +118,7 @@ RSpec.describe Gitlab::WikiPages::FrontMatterParser do
         MD
       end
 
-      it { is_expected.to have_attributes(reason: :not_mapping) }
+      it { is_expected.to have_attributes(reason: :no_match) }
     end
 
     context 'there is a string in the YAML block' do
diff --git a/spec/models/preloaders/user_max_access_level_in_groups_preloader_spec.rb b/spec/models/preloaders/user_max_access_level_in_groups_preloader_spec.rb
index 8144e1ad233..1ad744db76d 100644
--- a/spec/models/preloaders/user_max_access_level_in_groups_preloader_spec.rb
+++ b/spec/models/preloaders/user_max_access_level_in_groups_preloader_spec.rb
@@ -13,13 +13,8 @@ RSpec.describe Preloaders::UserMaxAccessLevelInGroupsPreloader do
 
   shared_examples 'executes N max member permission queries to the DB' do
     it 'executes the specified max membership queries' do
-      queries = ActiveRecord::QueryRecorder.new do
-        groups.each { |group| user.can?(:read_group, group) }
-      end
-
-      max_queries = queries.log.grep(max_query_regex)
-
-      expect(max_queries.count).to eq(expected_query_count)
+      expect { groups.each { |group| user.can?(:read_group, group) } }
+        .to make_queries_matching(max_query_regex, expected_query_count)
     end
   end
 
diff --git a/spec/requests/api/lint_spec.rb b/spec/requests/api/lint_spec.rb
index d7f22b9d619..8c701414be0 100644
--- a/spec/requests/api/lint_spec.rb
+++ b/spec/requests/api/lint_spec.rb
@@ -26,6 +26,35 @@ RSpec.describe API::Lint do
           expect(response).to have_gitlab_http_status(:ok)
         end
       end
+
+      context 'when authenticated as external user' do
+        let(:project) { create(:project) }
+        let(:api_user) { create(:user, :external) }
+
+        context 'when reporter in a project' do
+          before do
+            project.add_reporter(api_user)
+          end
+
+          it 'returns authorization failure' do
+            post api('/ci/lint', api_user), params: { content: 'content' }
+
+            expect(response).to have_gitlab_http_status(:unauthorized)
+          end
+        end
+
+        context 'when developer in a project' do
+          before do
+            project.add_developer(api_user)
+          end
+
+          it 'returns authorization success' do
+            post api('/ci/lint', api_user), params: { content: 'content' }
+
+            expect(response).to have_gitlab_http_status(:ok)
+          end
+        end
+      end
     end
 
     context 'when signup is enabled and not limited' do