Add latest changes from gitlab-org/security/gitlab@15-0-stable-ee

16 files changed, 248 insertions, 98 deletions

diff --git a/app/models/error_tracking/project_error_tracking_setting.rb b/app/models/error_tracking/project_error_tracking_setting.rb
index 30382a1c205..4953f24755c 100644
--- a/app/models/error_tracking/project_error_tracking_setting.rb
+++ b/app/models/error_tracking/project_error_tracking_setting.rb
@@ -44,6 +44,8 @@ module ErrorTracking
       key: Settings.attr_encrypted_db_key_base_32,
       algorithm: 'aes-256-gcm'
 
+    before_validation :reset_token
+
     after_save :clear_reactive_cache!
 
     # When a user enables the integrated error tracking
@@ -182,6 +184,12 @@ module ErrorTracking
 
     private
 
+    def reset_token
+      if api_url_changed? && !encrypted_token_changed?
+        self.token = nil
+      end
+    end
+
     def ensure_issue_belongs_to_project!(project_id_from_api)
       raise 'The Sentry issue appers to be outside of the configured Sentry project' if Integer(project_id_from_api) != ensure_sentry_project_id!
     end
diff --git a/app/models/grafana_integration.rb b/app/models/grafana_integration.rb
index 00213732fee..0358e37c58b 100644
--- a/app/models/grafana_integration.rb
+++ b/app/models/grafana_integration.rb
@@ -18,6 +18,8 @@ class GrafanaIntegration < ApplicationRecord
 
   validates :enabled, inclusion: { in: [true, false] }
 
+  before_validation :reset_token
+
   scope :enabled, -> { where(enabled: true) }
 
   def client
@@ -36,6 +38,12 @@ class GrafanaIntegration < ApplicationRecord
 
   private
 
+  def reset_token
+    if grafana_url_changed? && !encrypted_token_changed?
+      self.token = nil
+    end
+  end
+
   def token
     decrypt(:token, encrypted_token)
   end
diff --git a/app/models/integrations/campfire.rb b/app/models/integrations/campfire.rb
index 7889cd8f9a9..3e19ec5bc4b 100644
--- a/app/models/integrations/campfire.rb
+++ b/app/models/integrations/campfire.rb
@@ -2,8 +2,35 @@
 
 module Integrations
   class Campfire < Integration
-    prop_accessor :token, :subdomain, :room
+    SUBDOMAIN_REGEXP = %r{\A[a-z](?:[a-z0-9-]*[a-z0-9])?\z}i.freeze
+
     validates :token, presence: true, if: :activated?
+    validates :room,
+      allow_blank: true,
+      numericality: { only_integer: true, greater_than: 0 }
+    validates :subdomain,
+      allow_blank: true,
+      format: { with: SUBDOMAIN_REGEXP }, length: { in: 1..63 }
+
+    field :token,
+      type: 'password',
+      title:  -> { s_('Campfire token') },
+      help:  -> { s_('CampfireService|API authentication token from Campfire.') },
+      non_empty_password_title:  -> { s_('ProjectService|Enter new token') },
+      non_empty_password_help:  -> { s_('ProjectService|Leave blank to use your current token.') },
+      placeholder: '',
+      required: true
+
+    field :subdomain,
+      title:  -> { s_('Campfire subdomain (optional)') },
+      placeholder: '',
+      exposes_secrets: true,
+      help:  -> { s_('CampfireService|The %{code_open}.campfirenow.com%{code_close} subdomain.') % { code_open: '<code>'.html_safe, code_close: '</code>'.html_safe } }
+
+    field :room,
+      title:  -> { s_('Campfire room ID (optional)') },
+      placeholder: '123456',
+      help:  -> { s_('CampfireService|From the end of the room URL.') }
 
     def title
       'Campfire'
@@ -22,35 +49,6 @@ module Integrations
       'campfire'
     end
 
-    def fields
-      [
-        {
-          type: 'password',
-          name: 'token',
-          title: _('Campfire token'),
-          help: s_('CampfireService|API authentication token from Campfire.'),
-          non_empty_password_title: s_('ProjectService|Enter new token'),
-          non_empty_password_help: s_('ProjectService|Leave blank to use your current token.'),
-          placeholder: '',
-          required: true
-        },
-        {
-          type: 'text',
-          name: 'subdomain',
-          title: _('Campfire subdomain (optional)'),
-          placeholder: '',
-          help: s_('CampfireService|The %{code_open}.campfirenow.com%{code_close} subdomain.') % { code_open: '<code>'.html_safe, code_close: '</code>'.html_safe }
-        },
-        {
-          type: 'text',
-          name: 'room',
-          title: _('Campfire room ID (optional)'),
-          placeholder: '123456',
-          help: s_('CampfireService|From the end of the room URL.')
-        }
-      ]
-    end
-
     def self.supported_events
       %w(push)
     end
diff --git a/app/models/integrations/drone_ci.rb b/app/models/integrations/drone_ci.rb
index 0c65ed8cd5f..cb81ab21ebe 100644
--- a/app/models/integrations/drone_ci.rb
+++ b/app/models/integrations/drone_ci.rb
@@ -13,6 +13,7 @@ module Integrations
     field :drone_url,
       title: s_('ProjectService|Drone server URL'),
       placeholder: 'http://drone.example.com',
+      exposes_secrets: true,
       required: true
 
     field :token,
diff --git a/app/models/integrations/packagist.rb b/app/models/integrations/packagist.rb
index 758c9e4761b..502e509f7b9 100644
--- a/app/models/integrations/packagist.rb
+++ b/app/models/integrations/packagist.rb
@@ -5,7 +5,27 @@ module Integrations
     include HasWebHook
     extend Gitlab::Utils::Override
 
-    prop_accessor :username, :token, :server
+    field :username,
+      title: -> { s_('Username') },
+      help: -> { s_('Enter your Packagist username.') },
+      placeholder: '',
+      required: true
+
+    field :token,
+      type: 'password',
+      title: -> { s_('Token') },
+      help: -> { s_('Enter your Packagist token.') },
+      non_empty_password_title: -> { s_('ProjectService|Enter new token') },
+      non_empty_password_help: -> { s_('ProjectService|Leave blank to use your current token.') },
+      placeholder: '',
+      required: true
+
+    field :server,
+      title: -> { s_('Server (optional)') },
+      help: -> { s_('Enter your Packagist server. Defaults to https://packagist.org.') },
+      placeholder: 'https://packagist.org',
+      exposes_secrets: true,
+      required: false
 
     validates :username, presence: true, if: :activated?
     validates :token, presence: true, if: :activated?
@@ -22,37 +42,6 @@ module Integrations
       'packagist'
     end
 
-    def fields
-      [
-        {
-          type: 'text',
-          name: 'username',
-          title: _('Username'),
-          help: s_('Enter your Packagist username.'),
-          placeholder: '',
-          required: true
-        },
-        {
-          type: 'password',
-          name: 'token',
-          title: _('Token'),
-          help: s_('Enter your Packagist token.'),
-          non_empty_password_title: s_('ProjectService|Enter new token'),
-          non_empty_password_help: s_('ProjectService|Leave blank to use your current token.'),
-          placeholder: '',
-          required: true
-        },
-        {
-          type: 'text',
-          name: 'server',
-          title: _('Server (optional)'),
-          help: s_('Enter your Packagist server. Defaults to https://packagist.org.'),
-          placeholder: 'https://packagist.org',
-          required: false
-        }
-      ]
-    end
-
     def self.supported_events
       %w(push merge_request tag_push)
     end
diff --git a/app/models/integrations/zentao.rb b/app/models/integrations/zentao.rb
index c33df465fde..53194089296 100644
--- a/app/models/integrations/zentao.rb
+++ b/app/models/integrations/zentao.rb
@@ -1,10 +1,33 @@
 # frozen_string_literal: true
 
 module Integrations
-  class Zentao < Integration
+  class Zentao < BaseIssueTracker
     include Gitlab::Routing
 
-    data_field :url, :api_url, :api_token, :zentao_product_xid
+    self.field_storage = :data_fields
+
+    field :url,
+      title: -> { s_('ZentaoIntegration|ZenTao Web URL') },
+      placeholder: 'https://www.zentao.net',
+      help: -> { s_('ZentaoIntegration|Base URL of the ZenTao instance.') },
+      exposes_secrets: true,
+      required: true
+
+    field :api_url,
+      title: -> { s_('ZentaoIntegration|ZenTao API URL (optional)') },
+      help: -> { s_('ZentaoIntegration|If different from Web URL.') },
+      exposes_secrets: true
+
+    field :api_token,
+      type: 'password',
+      title: -> { s_('ZentaoIntegration|ZenTao API token') },
+      non_empty_password_title: -> { s_('ZentaoIntegration|Enter new ZenTao API token') },
+      non_empty_password_help: -> { s_('ProjectService|Leave blank to use your current token.') },
+      required: true
+
+    field :zentao_product_xid,
+      title: -> { s_('ZentaoIntegration|ZenTao Product ID') },
+      required: true
 
     validates :url, public_url: true, presence: true, if: :activated?
     validates :api_url, public_url: true, allow_blank: true
@@ -19,6 +42,17 @@ module Integrations
       zentao_tracker_data || self.build_zentao_tracker_data
     end
 
+    alias_method :project_url, :url
+
+    def set_default_data
+      return unless issues_tracker.present?
+
+      return if url
+
+      data_fields.url ||= issues_tracker['url']
+      data_fields.api_url ||= issues_tracker['api_url']
+    end
+
     def title
       'ZenTao'
     end
@@ -47,39 +81,6 @@ module Integrations
       %w()
     end
 
-    def fields
-      [
-        {
-          type: 'text',
-          name: 'url',
-          title: s_('ZentaoIntegration|ZenTao Web URL'),
-          placeholder: 'https://www.zentao.net',
-          help: s_('ZentaoIntegration|Base URL of the ZenTao instance.'),
-          required: true
-        },
-        {
-          type: 'text',
-          name: 'api_url',
-          title: s_('ZentaoIntegration|ZenTao API URL (optional)'),
-          help: s_('ZentaoIntegration|If different from Web URL.')
-        },
-        {
-          type: 'password',
-          name: 'api_token',
-          title: s_('ZentaoIntegration|ZenTao API token'),
-          non_empty_password_title: s_('ZentaoIntegration|Enter new ZenTao API token'),
-          non_empty_password_help: s_('ProjectService|Leave blank to use your current token.'),
-          required: true
-        },
-        {
-          type: 'text',
-          name: 'zentao_product_xid',
-          title: s_('ZentaoIntegration|ZenTao Product ID'),
-          required: true
-        }
-      ]
-    end
-
     private
 
     def client
diff --git a/app/services/groups/destroy_service.rb b/app/services/groups/destroy_service.rb
index c88c139a22e..bcf3110ca21 100644
--- a/app/services/groups/destroy_service.rb
+++ b/app/services/groups/destroy_service.rb
@@ -35,6 +35,8 @@ module Groups
 
       user_ids_for_project_authorizations_refresh = obtain_user_ids_for_project_authorizations_refresh
 
+      destroy_group_bots
+
       group.destroy
 
       if user_ids_for_project_authorizations_refresh.present?
@@ -76,6 +78,19 @@ module Groups
 
       group.users_ids_of_direct_members
     end
+
+    # rubocop:disable CodeReuse/ActiveRecord
+    def destroy_group_bots
+      bot_ids = group.members_and_requesters.joins(:user).merge(User.project_bot).pluck(:user_id)
+      current_user_id = current_user.id
+
+      group.run_after_commit do
+        bot_ids.each do |user_id|
+          DeleteUserWorker.perform_async(current_user_id, user_id, skip_authorization: true)
+        end
+      end
+    end
+    # rubocop:enable CodeReuse/ActiveRecord
   end
 end
 
diff --git a/spec/models/error_tracking/project_error_tracking_setting_spec.rb b/spec/models/error_tracking/project_error_tracking_setting_spec.rb
index 15b6b45eaba..0685144dea6 100644
--- a/spec/models/error_tracking/project_error_tracking_setting_spec.rb
+++ b/spec/models/error_tracking/project_error_tracking_setting_spec.rb
@@ -121,6 +121,38 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do
         end
       end
     end
+
+    describe 'before_validation :reset_token' do
+      context 'when a token was previously set' do
+        subject { create(:project_error_tracking_setting, project: project) }
+
+        it 'resets token if url changed' do
+          subject.api_url = 'http://sentry.com/api/0/projects/org-slug/proj-slug/'
+
+          expect(subject).not_to be_valid
+          expect(subject.token).to be_nil
+        end
+
+        it "does not reset token if new url is set together with the same token" do
+          subject.api_url = 'http://sentrytest.com/api/0/projects/org-slug/proj-slug/'
+          current_token = subject.token
+          subject.token = current_token
+
+          expect(subject).to be_valid
+          expect(subject.token).to eq(current_token)
+          expect(subject.api_url).to eq('http://sentrytest.com/api/0/projects/org-slug/proj-slug/')
+        end
+
+        it 'does not reset token if new url is set together with a new token' do
+          subject.api_url = 'http://sentrytest.com/api/0/projects/org-slug/proj-slug/'
+          subject.token = 'token'
+
+          expect(subject).to be_valid
+          expect(subject.token).to eq('token')
+          expect(subject.api_url).to eq('http://sentrytest.com/api/0/projects/org-slug/proj-slug/')
+        end
+      end
+    end
   end
 
   describe '.extract_sentry_external_url' do
diff --git a/spec/models/grafana_integration_spec.rb b/spec/models/grafana_integration_spec.rb
index bb822187e0c..73ec2856c05 100644
--- a/spec/models/grafana_integration_spec.rb
+++ b/spec/models/grafana_integration_spec.rb
@@ -86,4 +86,38 @@ RSpec.describe GrafanaIntegration do
       end
     end
   end
+
+  describe 'Callbacks' do
+    describe 'before_validation :reset_token' do
+      context 'when a token was previously set' do
+        subject(:grafana_integration) { create(:grafana_integration) }
+
+        it 'resets token if url changed' do
+          grafana_integration.grafana_url = 'http://gitlab1.com'
+
+          expect(grafana_integration).not_to be_valid
+          expect(grafana_integration.send(:token)).to be_nil
+        end
+
+        it "does not reset token if new url is set together with the same token" do
+          grafana_integration.grafana_url = 'http://gitlab_edited.com'
+          current_token = grafana_integration.send(:token)
+          grafana_integration.token = current_token
+
+          expect(grafana_integration).to be_valid
+          expect(grafana_integration.send(:token)).to eq(current_token)
+          expect(grafana_integration.grafana_url).to eq('http://gitlab_edited.com')
+        end
+
+        it 'does not reset token if new url is set together with a new token' do
+          grafana_integration.grafana_url = 'http://gitlab_edited.com'
+          grafana_integration.token = 'token'
+
+          expect(grafana_integration).to be_valid
+          expect(grafana_integration.send(:token)).to eq('token')
+          expect(grafana_integration.grafana_url).to eq('http://gitlab_edited.com')
+        end
+      end
+    end
+  end
 end
diff --git a/spec/models/integrations/campfire_spec.rb b/spec/models/integrations/campfire_spec.rb
index 0044e6fae21..d249c8470ca 100644
--- a/spec/models/integrations/campfire_spec.rb
+++ b/spec/models/integrations/campfire_spec.rb
@@ -5,7 +5,17 @@ require 'spec_helper'
 RSpec.describe Integrations::Campfire do
   include StubRequests
 
+  it_behaves_like Integrations::ResetSecretFields do
+    let(:integration) { described_class.new }
+  end
+
   describe 'Validations' do
+    it { is_expected.to validate_numericality_of(:room).is_greater_than(0).only_integer }
+    it { is_expected.to validate_length_of(:subdomain).is_at_most(63) }
+    it { is_expected.to allow_value("foo").for(:subdomain) }
+    it { is_expected.not_to allow_value("foo.bar").for(:subdomain) }
+    it { is_expected.not_to allow_value("foo.bar/#").for(:subdomain) }
+
     context 'when integration is active' do
       before do
         subject.active = true
diff --git a/spec/models/integrations/drone_ci_spec.rb b/spec/models/integrations/drone_ci_spec.rb
index 78d55c49e7b..5ae4af1a665 100644
--- a/spec/models/integrations/drone_ci_spec.rb
+++ b/spec/models/integrations/drone_ci_spec.rb
@@ -7,6 +7,10 @@ RSpec.describe Integrations::DroneCi, :use_clean_rails_memory_store_caching do
 
   subject(:integration) { described_class.new }
 
+  it_behaves_like Integrations::ResetSecretFields do
+    let(:integration) { subject }
+  end
+
   describe 'validations' do
     context 'active' do
       before do
diff --git a/spec/models/integrations/packagist_spec.rb b/spec/models/integrations/packagist_spec.rb
index dce96890522..d1976e73e2e 100644
--- a/spec/models/integrations/packagist_spec.rb
+++ b/spec/models/integrations/packagist_spec.rb
@@ -29,6 +29,10 @@ RSpec.describe Integrations::Packagist do
     let(:hook_url) { "#{packagist_server}/api/update-package?username=#{packagist_username}&apiToken=#{packagist_token}" }
   end
 
+  it_behaves_like Integrations::ResetSecretFields do
+    let(:integration) { described_class.new(packagist_params) }
+  end
+
   describe '#execute' do
     let(:user)    { create(:user) }
     let(:project) { create(:project, :repository) }
diff --git a/spec/models/integrations/zentao_spec.rb b/spec/models/integrations/zentao_spec.rb
index 2b0532c7930..4ef977ba3d2 100644
--- a/spec/models/integrations/zentao_spec.rb
+++ b/spec/models/integrations/zentao_spec.rb
@@ -9,6 +9,31 @@ RSpec.describe Integrations::Zentao do
   let(:zentao_product_xid) { '3' }
   let(:zentao_integration) { create(:zentao_integration) }
 
+  it_behaves_like Integrations::ResetSecretFields do
+    let(:integration) { zentao_integration }
+  end
+
+  describe 'set_default_data' do
+    let(:project) { create(:project, :repository) }
+
+    context 'when gitlab.yml was initialized' do
+      it 'is prepopulated with the settings' do
+        settings = {
+          'zentao' => {
+            'url' => 'http://zentao.sample/projects/project_a',
+            'api_url' => 'http://zentao.sample/api'
+          }
+        }
+        allow(Gitlab.config).to receive(:issues_tracker).and_return(settings)
+
+        integration = project.create_zentao_integration(active: true)
+
+        expect(integration.url).to eq('http://zentao.sample/projects/project_a')
+        expect(integration.api_url).to eq('http://zentao.sample/api')
+      end
+    end
+  end
+
   describe '#create' do
     let(:project) { create(:project, :repository) }
     let(:params) do
diff --git a/spec/services/groups/destroy_service_spec.rb b/spec/services/groups/destroy_service_spec.rb
index 628943e40ff..161a0907870 100644
--- a/spec/services/groups/destroy_service_spec.rb
+++ b/spec/services/groups/destroy_service_spec.rb
@@ -35,6 +35,20 @@ RSpec.describe Groups::DestroyService do
       it { expect(NotificationSetting.unscoped.all).not_to include(notification_setting) }
     end
 
+    context 'bot tokens', :sidekiq_might_not_need_inline do
+      it 'removes group bot', :aggregate_failures do
+        bot = create(:user, :project_bot)
+        group.add_developer(bot)
+        token = create(:personal_access_token, user: bot)
+
+        destroy_group(group, user, async)
+
+        expect(PersonalAccessToken.find_by(id: token.id)).to be_nil
+        expect(User.find_by(id: bot.id)).to be_nil
+        expect(User.find_by(id: user.id)).not_to be_nil
+      end
+    end
+
     context 'mattermost team', :sidekiq_might_not_need_inline do
       let!(:chat_team) { create(:chat_team, namespace: group) }
 
diff --git a/spec/services/projects/operations/update_service_spec.rb b/spec/services/projects/operations/update_service_spec.rb
index 3ee867ba6f2..57d0e824a83 100644
--- a/spec/services/projects/operations/update_service_spec.rb
+++ b/spec/services/projects/operations/update_service_spec.rb
@@ -306,6 +306,11 @@ RSpec.describe Projects::Operations::UpdateService do
         let(:params) do
           {
             error_tracking_setting_attributes: {
+              api_host: 'https://sentrytest.gitlab.com/',
+              project: {
+                slug: 'sentry-project',
+                organization_slug: 'sentry-org'
+              },
               enabled: false,
               token: '*' * 8
             }
@@ -313,7 +318,7 @@ RSpec.describe Projects::Operations::UpdateService do
         end
 
         before do
-          create(:project_error_tracking_setting, project: project, token: 'token')
+          create(:project_error_tracking_setting, project: project, token: 'token', api_url: 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project/')
         end
 
         it 'does not update token' do
diff --git a/spec/support/shared_contexts/features/integrations/integrations_shared_context.rb b/spec/support/shared_contexts/features/integrations/integrations_shared_context.rb
index 3ea6658c0c1..d0f7853eb58 100644
--- a/spec/support/shared_contexts/features/integrations/integrations_shared_context.rb
+++ b/spec/support/shared_contexts/features/integrations/integrations_shared_context.rb
@@ -36,6 +36,8 @@ Integration.available_integration_names.each do |integration|
           hash.merge!(k => 'foo@bar.com')
         elsif integration == 'slack' || integration == 'mattermost' && k == :labels_to_be_notified_behavior
           hash.merge!(k => "match_any")
+        elsif integration == 'campfire' && k = :room
+          hash.merge!(k => '1234')
         else
           hash.merge!(k => "someword")
         end