Add latest changes from gitlab-org/security/gitlab@13-8-stable-ee

27 files changed, 300 insertions, 8 deletions

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 4a579892e3f..88c7002b1b6 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -16,6 +16,7 @@ module Ci
     include ShaAttribute
     include FromUnion
     include UpdatedAtFilterable
+    include EachBatch
 
     MAX_OPEN_MERGE_REQUESTS_REFS = 4
 
diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb
index a399ffc32de..c2aecc524d4 100644
--- a/app/models/commit_status.rb
+++ b/app/models/commit_status.rb
@@ -55,6 +55,7 @@ class CommitStatus < ApplicationRecord
   scope :for_ids, -> (ids) { where(id: ids) }
   scope :for_ref, -> (ref) { where(ref: ref) }
   scope :by_name, -> (name) { where(name: name) }
+  scope :in_pipelines, ->(pipelines) { where(pipeline: pipelines) }
 
   scope :for_project_paths, -> (paths) do
     where(project: Project.where_full_path_in(Array(paths)))
diff --git a/app/models/member.rb b/app/models/member.rb
index 2e79b50d6b7..62fe757683f 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -47,6 +47,19 @@ class Member < ApplicationRecord
     },
     if: :project_bot?
 
+  scope :in_hierarchy, ->(source) do
+    groups = source.root_ancestor.self_and_descendants
+    group_members = Member.default_scoped.where(source: groups)
+
+    projects = source.root_ancestor.all_projects
+    project_members = Member.default_scoped.where(source: projects)
+
+    Member.default_scoped.from_union([
+      group_members,
+      project_members
+    ]).merge(self)
+  end
+
   # This scope encapsulates (most of) the conditions a row in the member table
   # must satisfy if it is a valid permission. Of particular note:
   #
@@ -79,12 +92,18 @@ class Member < ApplicationRecord
 
   scope :invite, -> { where.not(invite_token: nil) }
   scope :non_invite, -> { where(invite_token: nil) }
+
   scope :request, -> { where.not(requested_at: nil) }
   scope :non_request, -> { where(requested_at: nil) }
 
   scope :not_accepted_invitations, -> { invite.where(invite_accepted_at: nil) }
   scope :not_accepted_invitations_by_user, -> (user) { not_accepted_invitations.where(created_by: user) }
   scope :not_expired, -> (today = Date.current) { where(arel_table[:expires_at].gt(today).or(arel_table[:expires_at].eq(nil))) }
+
+  scope :created_today, -> do
+    now = Date.current
+    where(created_at: now.beginning_of_day..now.end_of_day)
+  end
   scope :last_ten_days_excluding_today, -> (today = Date.current) { where(created_at: (today - 10).beginning_of_day..(today - 1).end_of_day) }
 
   scope :has_access, -> { active.where('access_level > 0') }
diff --git a/app/services/ci/abort_project_pipelines_service.rb b/app/services/ci/abort_project_pipelines_service.rb
new file mode 100644
index 00000000000..0b2fa9ed3c0
--- /dev/null
+++ b/app/services/ci/abort_project_pipelines_service.rb
@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+module Ci
+  class AbortProjectPipelinesService
+    # Danger: Cancels in bulk without callbacks
+    # Only for pipeline abandonment scenarios (current example: project delete)
+    def execute(project)
+      return unless Feature.enabled?(:abort_deleted_project_pipelines, default_enabled: :yaml)
+
+      pipelines = project.all_pipelines.cancelable
+      bulk_abort!(pipelines, status: :canceled)
+
+      ServiceResponse.success(message: 'Pipelines canceled')
+    end
+
+    private
+
+    def bulk_abort!(pipelines, status:)
+      pipelines.each_batch do |pipeline_batch|
+        CommitStatus.in_pipelines(pipeline_batch).in_batches.update_all(status: status) # rubocop: disable Cop/InBatches
+        pipeline_batch.update_all(status: status)
+      end
+    end
+  end
+end
diff --git a/app/services/ci/cancel_user_pipelines_service.rb b/app/services/ci/cancel_user_pipelines_service.rb
index 3a8b5e91088..3d3a8032e8e 100644
--- a/app/services/ci/cancel_user_pipelines_service.rb
+++ b/app/services/ci/cancel_user_pipelines_service.rb
@@ -6,6 +6,7 @@ module Ci
     # This is a bug with CodeReuse/ActiveRecord cop
     # https://gitlab.com/gitlab-org/gitlab/issues/32332
     def execute(user)
+      # TODO: fix N+1 queries https://gitlab.com/gitlab-org/gitlab/-/issues/300685
       user.pipelines.cancelable.find_each(&:cancel_running)
 
       ServiceResponse.success(message: 'Pipeline canceled')
diff --git a/app/services/members/create_service.rb b/app/services/members/create_service.rb
index 5fcf2d711b0..cffccda1a44 100644
--- a/app/services/members/create_service.rb
+++ b/app/services/members/create_service.rb
@@ -2,12 +2,12 @@
 
 module Members
   class CreateService < Members::BaseService
+    include Gitlab::Utils::StrongMemoize
+
     DEFAULT_LIMIT = 100
 
     def execute(source)
-      return error(s_('AddMember|No users specified.')) if params[:user_ids].blank?
-
-      user_ids = params[:user_ids].split(',').uniq.flatten
+      return error(s_('AddMember|No users specified.')) if user_ids.blank?
 
       return error(s_("AddMember|Too many users specified (limit is %{user_limit})") % { user_limit: user_limit }) if
         user_limit && user_ids.size > user_limit
@@ -47,6 +47,13 @@ module Members
 
     private
 
+    def user_ids
+      strong_memoize(:user_ids) do
+        ids = params[:user_ids] || ''
+        ids.split(',').uniq.flatten
+      end
+    end
+
     def user_limit
       limit = params.fetch(:limit, DEFAULT_LIMIT)
 
diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb
index bec75657530..c1501625300 100644
--- a/app/services/projects/destroy_service.rb
+++ b/app/services/projects/destroy_service.rb
@@ -21,11 +21,14 @@ module Projects
     def execute
       return false unless can?(current_user, :remove_project, project)
 
+      project.update_attribute(:pending_delete, true)
       # Flush the cache for both repositories. This has to be done _before_
       # removing the physical repositories as some expiration code depends on
       # Git data (e.g. a list of branch names).
       flush_caches(project)
 
+      ::Ci::AbortProjectPipelinesService.new.execute(project)
+
       Projects::UnlinkForkService.new(project, current_user).execute
 
       attempt_destroy(project)
diff --git a/changelogs/unreleased/security-cancel-pipelines-for-deleted-project.yml b/changelogs/unreleased/security-cancel-pipelines-for-deleted-project.yml
new file mode 100644
index 00000000000..de92707cb8f
--- /dev/null
+++ b/changelogs/unreleased/security-cancel-pipelines-for-deleted-project.yml
@@ -0,0 +1,5 @@
+---
+title: Cancel running and pending jobs when a project is deleted
+merge_request: 1220
+author:
+type: security
diff --git a/changelogs/unreleased/security-limit-invitations.yml b/changelogs/unreleased/security-limit-invitations.yml
new file mode 100644
index 00000000000..353d1cec727
--- /dev/null
+++ b/changelogs/unreleased/security-limit-invitations.yml
@@ -0,0 +1,5 @@
+---
+title: Limit daily invitations to groups and projects
+merge_request:
+author:
+type: security
diff --git a/config/feature_flags/development/abort_deleted_project_pipelines.yml b/config/feature_flags/development/abort_deleted_project_pipelines.yml
new file mode 100644
index 00000000000..f09cc9dd86b
--- /dev/null
+++ b/config/feature_flags/development/abort_deleted_project_pipelines.yml
@@ -0,0 +1,8 @@
+---
+name: abort_deleted_project_pipelines
+introduced_by_url: https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/1220
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/301106
+milestone: '13.9'
+type: development
+group: group::continuous integration
+default_enabled: true
diff --git a/db/migrate/20201007033527_add_daily_invites_to_plan_limits.rb b/db/migrate/20201007033527_add_daily_invites_to_plan_limits.rb
new file mode 100644
index 00000000000..8f0079cd639
--- /dev/null
+++ b/db/migrate/20201007033527_add_daily_invites_to_plan_limits.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+class AddDailyInvitesToPlanLimits < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def change
+    add_column(:plan_limits, :daily_invites, :integer, default: 0, null: false)
+  end
+end
diff --git a/db/migrate/20201007033723_insert_daily_invites_plan_limits.rb b/db/migrate/20201007033723_insert_daily_invites_plan_limits.rb
new file mode 100644
index 00000000000..dcdcbbb0964
--- /dev/null
+++ b/db/migrate/20201007033723_insert_daily_invites_plan_limits.rb
@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+class InsertDailyInvitesPlanLimits < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    return unless Gitlab.com?
+
+    create_or_update_plan_limit('daily_invites', 'free', 20)
+    create_or_update_plan_limit('daily_invites', 'bronze', 0)
+    create_or_update_plan_limit('daily_invites', 'silver', 0)
+    create_or_update_plan_limit('daily_invites', 'gold', 0)
+  end
+
+  def down
+    return unless Gitlab.com?
+
+    create_or_update_plan_limit('daily_invites', 'free', 0)
+    create_or_update_plan_limit('daily_invites', 'bronze', 0)
+    create_or_update_plan_limit('daily_invites', 'silver', 0)
+    create_or_update_plan_limit('daily_invites', 'gold', 0)
+  end
+end
diff --git a/db/schema_migrations/20201007033527 b/db/schema_migrations/20201007033527
new file mode 100644
index 00000000000..b2cedd57973
--- /dev/null
+++ b/db/schema_migrations/20201007033527
@@ -0,0 +1 @@
+1200747265d5095a86250020786d6f1e9e50bc75328a71de497046807afa89d7
\ No newline at end of file
diff --git a/db/schema_migrations/20201007033723 b/db/schema_migrations/20201007033723
new file mode 100644
index 00000000000..c874ae0475b
--- /dev/null
+++ b/db/schema_migrations/20201007033723
@@ -0,0 +1 @@
+febefead6f966960f6493d29add5f35fc4a1080b5118c5526502fa5fe1d29023
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index de4218ed405..c1f13a2fca4 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -15254,7 +15254,8 @@ CREATE TABLE plan_limits (
     project_feature_flags integer DEFAULT 200 NOT NULL,
     ci_max_artifact_size_api_fuzzing integer DEFAULT 0 NOT NULL,
     ci_pipeline_deployments integer DEFAULT 500 NOT NULL,
-    pull_mirror_interval_seconds integer DEFAULT 300 NOT NULL
+    pull_mirror_interval_seconds integer DEFAULT 300 NOT NULL,
+    daily_invites integer DEFAULT 0 NOT NULL
 );
 
 CREATE SEQUENCE plan_limits_id_seq
diff --git a/doc/administration/instance_limits.md b/doc/administration/instance_limits.md
index 0eede5f3ca5..a3452a1a605 100644
--- a/doc/administration/instance_limits.md
+++ b/doc/administration/instance_limits.md
@@ -96,6 +96,13 @@ Read more on the [Rack Attack initializer](../security/rack_attack.md) method of
 
 - **Default rate limit** - Disabled
 
+### Member Invitations
+
+Limit the maximum daily member invitations allowed per group hierarchy.
+
+- GitLab.com: Free members may invite 20 members per day.
+- Self-managed: Invites are not limited.
+
 ## Gitaly concurrency limit
 
 Clone traffic can put a large strain on your Gitaly service. To prevent such workloads from overwhelming your Gitaly server, you can set concurrency limits in Gitaly’s configuration file.
diff --git a/lib/gitlab/ci/pipeline/chain/validate/abilities.rb b/lib/gitlab/ci/pipeline/chain/validate/abilities.rb
index e68d9020a21..55c125e03d5 100644
--- a/lib/gitlab/ci/pipeline/chain/validate/abilities.rb
+++ b/lib/gitlab/ci/pipeline/chain/validate/abilities.rb
@@ -10,6 +10,10 @@ module Gitlab
             include Chain::Helpers
 
             def perform!
+              if project.pending_delete?
+                return error('Project is deleted!')
+              end
+
               unless project.builds_enabled?
                 return error('Pipelines are disabled!')
               end
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index e47bbec804f..0b1b80d59ff 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -1807,6 +1807,9 @@ msgstr ""
 msgid "AddContextCommits|Add/remove"
 msgstr ""
 
+msgid "AddMember|Invite limit of %{daily_invites} per day exceeded"
+msgstr ""
+
 msgid "AddMember|No users specified."
 msgstr ""
 
diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb
index ae3270cb9b2..7aaeee32f49 100644
--- a/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb
@@ -74,6 +74,14 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Validate::Abilities do
     it 'does not break the chain' do
       expect(step.break?).to eq false
     end
+
+    context 'when project is deleted' do
+      before do
+        project.update!(pending_delete: true)
+      end
+
+      specify { expect(step.perform!).to contain_exactly('Project is deleted!') }
+    end
   end
 
   describe '#allowed_to_write_ref?' do
diff --git a/spec/migrations/insert_daily_invites_plan_limits_spec.rb b/spec/migrations/insert_daily_invites_plan_limits_spec.rb
new file mode 100644
index 00000000000..3265efcb0ce
--- /dev/null
+++ b/spec/migrations/insert_daily_invites_plan_limits_spec.rb
@@ -0,0 +1,55 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'migrate', '20201007033723_insert_daily_invites_plan_limits.rb')
+
+RSpec.describe InsertDailyInvitesPlanLimits do
+  let(:plans) { table(:plans) }
+  let(:plan_limits) { table(:plan_limits) }
+  let!(:free_plan) { plans.create!(name: 'free') }
+  let!(:bronze_plan) { plans.create!(name: 'bronze') }
+  let!(:silver_plan) { plans.create!(name: 'silver') }
+  let!(:gold_plan) { plans.create!(name: 'gold') }
+
+  context 'when on Gitlab.com' do
+    before do
+      expect(Gitlab).to receive(:com?).at_most(:twice).and_return(true)
+    end
+
+    it 'correctly migrates up and down' do
+      reversible_migration do |migration|
+        migration.before -> {
+          expect(plan_limits.where.not(daily_invites: 0)).to be_empty
+        }
+
+        # Expectations will run after the up migration.
+        migration.after -> {
+          expect(plan_limits.pluck(:plan_id, :daily_invites)).to contain_exactly(
+            [free_plan.id, 20],
+            [bronze_plan.id, 0],
+            [silver_plan.id, 0],
+            [gold_plan.id, 0]
+          )
+        }
+      end
+    end
+  end
+
+  context 'when on self hosted' do
+    before do
+      expect(Gitlab).to receive(:com?).at_most(:twice).and_return(false)
+    end
+
+    it 'correctly migrates up and down' do
+      reversible_migration do |migration|
+        migration.before -> {
+          expect(plan_limits.pluck(:daily_invites)).to eq []
+        }
+
+        migration.after -> {
+          expect(plan_limits.pluck(:daily_invites)).to eq []
+        }
+      end
+    end
+  end
+end
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index f5e824bb066..140527e4414 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -34,6 +34,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
   it { is_expected.to have_many(:auto_canceled_jobs) }
   it { is_expected.to have_many(:sourced_pipelines) }
   it { is_expected.to have_many(:triggered_pipelines) }
+  it { is_expected.to have_many(:pipeline_artifacts) }
 
   it { is_expected.to have_one(:chat_data) }
   it { is_expected.to have_one(:source_pipeline) }
@@ -41,14 +42,15 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
   it { is_expected.to have_one(:source_job) }
   it { is_expected.to have_one(:pipeline_config) }
 
-  it { is_expected.to validate_presence_of(:sha) }
-  it { is_expected.to validate_presence_of(:status) }
-
   it { is_expected.to respond_to :git_author_name }
   it { is_expected.to respond_to :git_author_email }
   it { is_expected.to respond_to :short_sha }
   it { is_expected.to delegate_method(:full_path).to(:project).with_prefix }
-  it { is_expected.to have_many(:pipeline_artifacts) }
+
+  describe 'validations' do
+    it { is_expected.to validate_presence_of(:sha) }
+    it { is_expected.to validate_presence_of(:status) }
+  end
 
   describe 'associations' do
     it 'has a bidirectional relationship with projects' do
diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb
index 1a791820f1b..b60af7abade 100644
--- a/spec/models/member_spec.rb
+++ b/spec/models/member_spec.rb
@@ -171,6 +171,43 @@ RSpec.describe Member do
       end
     end
 
+    describe '.in_hierarchy' do
+      let(:root_ancestor) { create(:group) }
+      let(:project) { create(:project, group: root_ancestor) }
+      let(:subgroup) { create(:group, parent: root_ancestor) }
+      let(:subgroup_project) { create(:project, group: subgroup) }
+
+      let!(:root_ancestor_member) { create(:group_member, group: root_ancestor) }
+      let!(:project_member) { create(:project_member, project: project) }
+      let!(:subgroup_member) { create(:group_member, group: subgroup) }
+      let!(:subgroup_project_member) { create(:project_member, project: subgroup_project) }
+
+      let(:hierarchy_members) do
+        [
+          root_ancestor_member,
+          project_member,
+          subgroup_member,
+          subgroup_project_member
+        ]
+      end
+
+      subject { Member.in_hierarchy(project) }
+
+      it { is_expected.to contain_exactly(*hierarchy_members) }
+
+      context 'with scope prefix' do
+        subject { Member.where.not(source: project).in_hierarchy(subgroup) }
+
+        it { is_expected.to contain_exactly(root_ancestor_member, subgroup_member, subgroup_project_member) }
+      end
+
+      context 'with scope suffix' do
+        subject { Member.in_hierarchy(project).where.not(source: project) }
+
+        it { is_expected.to contain_exactly(root_ancestor_member, subgroup_member, subgroup_project_member) }
+      end
+    end
+
     describe '.invite' do
       it { expect(described_class.invite).not_to include @maintainer }
       it { expect(described_class.invite).to include @invited_member }
@@ -251,6 +288,21 @@ RSpec.describe Member do
       it { is_expected.to include(expiring_tomorrow, not_expiring) }
     end
 
+    describe '.created_today' do
+      let_it_be(:now) { Time.current }
+      let_it_be(:created_today) { create(:group_member, created_at: now.beginning_of_day) }
+      let_it_be(:created_yesterday) { create(:group_member, created_at: now - 1.day) }
+
+      before do
+        travel_to now
+      end
+
+      subject { described_class.created_today }
+
+      it { is_expected.not_to include(created_yesterday) }
+      it { is_expected.to include(created_today) }
+    end
+
     describe '.last_ten_days_excluding_today' do
       let_it_be(:now) { Time.current }
       let_it_be(:created_today) { create(:group_member, created_at: now.beginning_of_day) }
diff --git a/spec/models/plan_limits_spec.rb b/spec/models/plan_limits_spec.rb
index 67fb11f34e0..4259c8b708b 100644
--- a/spec/models/plan_limits_spec.rb
+++ b/spec/models/plan_limits_spec.rb
@@ -209,6 +209,7 @@ RSpec.describe PlanLimits do
         ci_pipeline_size
         ci_active_jobs
         storage_size_limit
+        daily_invites
       ] + disabled_max_artifact_size_columns
     end
 
diff --git a/spec/services/ci/abort_project_pipelines_service_spec.rb b/spec/services/ci/abort_project_pipelines_service_spec.rb
new file mode 100644
index 00000000000..9af909ac2ab
--- /dev/null
+++ b/spec/services/ci/abort_project_pipelines_service_spec.rb
@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Ci::AbortProjectPipelinesService do
+  let_it_be(:project) { create(:project) }
+  let_it_be(:pipeline) { create(:ci_pipeline, :running, project: project) }
+  let_it_be(:build) { create(:ci_build, :running, pipeline: pipeline) }
+
+  describe '#execute' do
+    it 'cancels all running pipelines and related jobs' do
+      result = described_class.new.execute(project)
+
+      expect(result).to be_success
+      expect(pipeline.reload).to be_canceled
+      expect(build.reload).to be_canceled
+    end
+
+    it 'avoids N+1 queries' do
+      control_count = ActiveRecord::QueryRecorder.new { described_class.new.execute(project) }.count
+
+      pipelines = create_list(:ci_pipeline, 5, :running, project: project)
+      create_list(:ci_build, 5, :running, pipeline: pipelines.first)
+
+      expect { described_class.new.execute(project) }.not_to exceed_query_limit(control_count)
+    end
+  end
+
+  context 'when feature disabled' do
+    before do
+      stub_feature_flags(abort_deleted_project_pipelines: false)
+    end
+
+    it 'does not abort the pipeline' do
+      result = described_class.new.execute(project)
+
+      expect(result).to be(nil)
+      expect(pipeline.reload).to be_running
+      expect(build.reload).to be_running
+    end
+  end
+end
diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb
index f0f09218b06..75d1c98923a 100644
--- a/spec/services/projects/destroy_service_spec.rb
+++ b/spec/services/projects/destroy_service_spec.rb
@@ -69,6 +69,12 @@ RSpec.describe Projects::DestroyService, :aggregate_failures do
       destroy_project(project, user, {})
     end
 
+    it 'performs cancel for project ci pipelines' do
+      expect(::Ci::AbortProjectPipelinesService).to receive_message_chain(:new, :execute).with(project)
+
+      destroy_project(project, user, {})
+    end
+
     context 'when project has remote mirrors' do
       let!(:project) do
         create(:project, :repository, namespace: user.namespace).tap do |project|
diff --git a/vendor/gitignore/C++.gitignore b/vendor/gitignore/C++.gitignore
index 259148fa18f..259148fa18f 100755..100644
--- a/vendor/gitignore/C++.gitignore
+++ b/vendor/gitignore/C++.gitignore
diff --git a/vendor/gitignore/Java.gitignore b/vendor/gitignore/Java.gitignore
index a1c2a238a96..a1c2a238a96 100755..100644
--- a/vendor/gitignore/Java.gitignore
+++ b/vendor/gitignore/Java.gitignore