Merge branch '215669-security-bot-permissions' into 'master'

Permissions for Security bot

See merge request gitlab-org/gitlab!49167

app/models/user.rb

@@ -722,6 +722,7 @@ class User < ApplicationRecord
         u.name = 'GitLab Security Bot'
         u.website_url = Gitlab::Routing.url_helpers.help_page_url('user/application_security/security_bot/index.md')
         u.avatar = bot_avatar(image: 'security-bot.png')
+        u.confirmed_at = Time.zone.now
       end
     end
 

app/policies/base_policy.rb

@@ -25,6 +25,10 @@ class BasePolicy < DeclarativePolicy::Base
   with_options scope: :user, score: 0
   condition(:support_bot) { @user&.support_bot? }
 
+  desc "User is security bot"
+  with_options scope: :user, score: 0
+  condition(:security_bot) { @user&.security_bot? }
+
   desc "User email is unconfirmed or user account is locked"
   with_options scope: :user, score: 0
   condition(:inactive) { @user&.confirmation_required_on_sign_in? || @user&.access_locked? }

app/policies/concerns/policy_actor.rb

@@ -49,6 +49,10 @@ module PolicyActor
     false
   end
 
+  def security_bot?
+    false
+  end
+
   def deactivated?
     false
   end

app/policies/global_policy.rb

@@ -48,7 +48,7 @@ class GlobalPolicy < BasePolicy
     prevent :use_slash_commands
   end
 
-  rule { blocked | (internal & ~migration_bot) }.policy do
+  rule { blocked | (internal & ~migration_bot & ~security_bot) }.policy do
     prevent :access_git
   end
 

app/serializers/pipeline_serializer.rb

@@ -75,3 +75,5 @@ class PipelineSerializer < BaseSerializer
     ]
   end
 end
+
+PipelineSerializer.prepend_if_ee('EE::PipelineSerializer')

ee/app/models/ee/project.rb

@@ -31,6 +31,8 @@ module EE
       after_update :remove_mirror_repository_reference,
         if: ->(project) { project.mirror? && project.import_url_updated? }
 
+      after_create :create_security_setting, unless: :security_setting
+
       belongs_to :mirror_user, class_name: 'User'
       belongs_to :deleting_user, foreign_key: 'marked_for_deletion_by_user_id', class_name: 'User'
 

ee/app/models/license.rb

@@ -163,6 +163,7 @@ class License < ApplicationRecord
     status_page
     subepics
     threat_monitoring
+    vulnerability_auto_fix
     quality_management
   ]
   EEU_FEATURES.freeze

ee/app/models/project_security_setting.rb

@@ -3,11 +3,23 @@
 class ProjectSecuritySetting < ApplicationRecord
   self.primary_key = :project_id
 
+  # Note: Even if we store settings for all types of security scanning
+  # Currently, Auto-fix feature is available only for container_scanning and
+  # dependency_scanning features.
+  AVAILABLE_AUTO_FIX_TYPES = [:dependency_scanning, :container_scanning].freeze
+
   belongs_to :project, inverse_of: :security_setting
 
-  def self.safe_find_or_create_for(project)
-    project.security_setting || project.create_security_setting
-  rescue ActiveRecord::RecordNotUnique
-    retry
+  def auto_fix_enabled?
+    return false if Feature.disabled?(:security_auto_fix, project)
+    return false unless project.feature_available?(:vulnerability_auto_fix)
+
+    auto_fix_enabled_types.any?
+  end
+
+  def auto_fix_enabled_types
+    AVAILABLE_AUTO_FIX_TYPES.filter_map do |type|
+      type if public_send("auto_fix_#{type}") # rubocop:disable GitlabSecurity/PublicSend
+    end
   end
 end

ee/app/policies/ee/project_policy.rb

@@ -6,6 +6,9 @@ module EE
     extend ::Gitlab::Utils::Override
 
     prepended do
+      with_scope :subject
+      condition(:auto_fix_enabled) { @subject.security_setting&.auto_fix_enabled? }
+
       with_scope :subject
       condition(:repository_mirrors_enabled) { @subject.feature_available?(:repository_mirrors) }
 
@@ -226,6 +229,12 @@ module EE
         enable :admin_vulnerability_external_issue_link
       end
 
+      rule { security_bot & auto_fix_enabled }.policy do
+        enable :push_code
+        enable :create_merge_request_from
+        enable :create_vulnerability_feedback
+      end
+
       rule { issues_disabled & merge_requests_disabled }.policy do
         prevent(*create_read_update_admin_destroy(:iteration))
       end
@@ -375,6 +384,7 @@ module EE
     def lookup_access_level!
       return ::Gitlab::Access::NO_ACCESS if needs_new_sso_session?
       return ::Gitlab::Access::NO_ACCESS if visual_review_bot?
+      return ::Gitlab::Access::REPORTER if security_bot? && auto_fix_enabled?
 
       super
     end

ee/app/policies/vulnerabilities/feedback_policy.rb

@@ -7,6 +7,7 @@ module Vulnerabilities
     condition(:issue) { @subject.for_issue? }
     condition(:merge_request) { @subject.for_merge_request? }
     condition(:dismissal) { @subject.for_dismissal? }
+    condition(:auto_fix_enabled) { @subject.project.security_setting&.auto_fix_enabled? }
 
     rule { issue & ~can?(:create_issue) }.prevent :create_vulnerability_feedback
 
@@ -14,6 +15,14 @@ module Vulnerabilities
       merge_request & ~can?(:create_merge_request_in)
     end.prevent :create_vulnerability_feedback
 
+    # Prevent Security bot from creating vulnerability feedback
+    # if auto-fix feature is disabled
+    rule do
+      merge_request &
+        security_bot &
+        ~auto_fix_enabled
+    end.prevent :create_vulnerability_feedback
+
     rule { ~dismissal }.prevent :destroy_vulnerability_feedback, :update_vulnerability_feedback
   end
 end

ee/app/presenters/projects/security/configuration_presenter.rb

@@ -82,8 +82,8 @@ module Projects
 
       def autofix_enabled
         {
-          dependency_scanning: project_settings.auto_fix_dependency_scanning,
-          container_scanning: project_settings.auto_fix_container_scanning
+          dependency_scanning: project_settings&.auto_fix_dependency_scanning,
+          container_scanning: project_settings&.auto_fix_container_scanning
         }
       end
 
@@ -177,7 +177,7 @@ module Projects
       end
 
       def project_settings
-        ProjectSecuritySetting.safe_find_or_create_for(project)
+        project.security_setting
       end
 
       def configuration_path(type)

ee/app/serializers/ee/pipeline_serializer.rb

+# frozen_string_literal: true
+
+module EE
+  module PipelineSerializer
+    extend ActiveSupport::Concern
+
+    private
+
+    def preloaded_relations
+      relations = super
+
+      project_relation = relations.detect { |item| item.is_a?(Hash) }
+      project_relation[:project].push(:security_setting)
+      relations
+    end
+  end
+end

ee/app/services/security/configuration/save_auto_fix_service.rb

@@ -15,17 +15,13 @@ module Security
       def execute(enabled:)
         return unless valid?
 
-        project_settings.update(toggle_params(enabled))
+        project&.security_setting&.update(toggle_params(enabled))
       end
 
       private
 
       attr_reader :enabled, :feature, :project
 
-      def project_settings
-        @project_settings ||= ProjectSecuritySetting.safe_find_or_create_for(project)
-      end
-
       def toggle_params(enabled)
         if feature == 'all'
           {

ee/spec/controllers/projects/security/configuration_controller_spec.rb

@@ -107,7 +107,6 @@ RSpec.describe Projects::Security::ConfigurationController do
 
       context 'with sufficient permissions' do
         let(:user) { maintainer }
-        let(:setting) { project.security_setting }
 
         it 'shows auto fix disable for dependency scanning for json format' do
           get :show, params: { namespace_id: project.namespace, project_id: project, format: :json }
@@ -121,7 +120,7 @@ RSpec.describe Projects::Security::ConfigurationController do
 
           it 'processes request and updates setting' do
             expect(response).to have_gitlab_http_status(:ok)
-            expect(setting.auto_fix_dependency_scanning).to be_falsey
+            expect(project.security_setting.reload.auto_fix_dependency_scanning).to be_falsey
             expect(response[:dependency_scanning]).to be_falsey
           end
         end
@@ -130,6 +129,8 @@ RSpec.describe Projects::Security::ConfigurationController do
           let(:feature) { '' }
 
           it 'processes request and updates setting' do
+            setting = project.reload.security_setting
+
             expect(response).to have_gitlab_http_status(:ok)
             expect(setting.auto_fix_dependency_scanning).to be_falsey
             expect(setting.auto_fix_dast).to be_falsey
@@ -139,11 +140,10 @@ RSpec.describe Projects::Security::ConfigurationController do
 
         context 'without processable feature' do
           let(:feature) { :dep_scan }
-          let(:setting) { project.create_security_setting }
 
           it 'does not pass validation' do
             expect(response).to have_gitlab_http_status(:unprocessable_entity)
-            expect(setting.auto_fix_dependency_scanning).to be_truthy
+            expect(project.security_setting.auto_fix_dependency_scanning).to be_truthy
           end
         end
       end

ee/spec/factories/project_security_settings.rb

@@ -7,5 +7,12 @@ FactoryBot.define do
     auto_fix_dast { true }
     auto_fix_dependency_scanning { true }
     auto_fix_sast { true }
+
+    trait :disabled_auto_fix do
+      auto_fix_container_scanning { false }
+      auto_fix_dast { false }
+      auto_fix_dependency_scanning { false }
+      auto_fix_sast { false }
+    end
   end
 end

ee/spec/lib/ee/gitlab/import_export/project/tree_restorer_spec.rb

@@ -102,12 +102,10 @@ RSpec.describe Gitlab::ImportExport::Project::TreeRestorer do
 
     before do
       setup_import_export_config('complex', 'ee')
+      restored_project_json
     end
 
     it 'creates security setting' do
-      project = Project.find_by_path('project')
-
-      expect { restored_project_json }.to change { ProjectSecuritySetting.count }.from(0).to(1)
       expect(project.security_setting.auto_fix_dependency_scanning).to be_falsey
       expect(project.security_setting.auto_fix_container_scanning).to be_truthy
     end

ee/spec/lib/ee/gitlab/import_export/project/tree_saver_spec.rb

@@ -15,7 +15,6 @@ RSpec.describe Gitlab::ImportExport::Project::TreeSaver do
 
   let_it_be(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec_ee" }
 
-  let_it_be(:security_setting) { create(:project_security_setting, project: project, auto_fix_dast: false) }
   let_it_be(:push_rule) { create(:push_rule, project: project, max_file_size: 10) }
 
   after :all do
@@ -75,7 +74,6 @@ RSpec.describe Gitlab::ImportExport::Project::TreeSaver do
       end
 
       it 'has security settings' do
-        expect(security_json['auto_fix_dast']).to be_falsey
         expect(security_json['auto_fix_dependency_scanning']).to be_truthy
       end
     end

ee/spec/models/project_security_setting_spec.rb

@@ -4,32 +4,79 @@ require 'spec_helper'
 
 RSpec.describe ProjectSecuritySetting do
   describe 'associations' do
-    subject { create(:project_security_setting) }
+    subject { create(:project).security_setting }
 
     it { is_expected.to belong_to(:project) }
   end
 
-  describe '.safe_find_or_create_for' do
-    subject { described_class.safe_find_or_create_for(project) }
+  describe '#auto_fix_enabled?' do
+    subject { setting.auto_fix_enabled? }
 
-    let_it_be(:project) { create :project }
+    let_it_be(:setting) { build(:project_security_setting) }
 
-    context 'without existing setting' do
-      it 'creates a new entry' do
-        expect { subject }.to change { ProjectSecuritySetting.count }.by(1)
-        expect(subject).to be_a_kind_of(ProjectSecuritySetting)
+    context 'when licensed feature is enabled' do
+      before do
+        stub_licensed_features(vulnerability_auto_fix: true)
+      end
+
+      context 'when auto fix is enabled for available feature' do
+        before do
+          setting.auto_fix_container_scanning = false
+          setting.auto_fix_dependency_scanning = true
+        end
+
+        it 'marks auto_fix as enabled' do
+          is_expected.to be_truthy
+        end
+      end
+
+      context 'when a auto_fix setting is disabled for available features' do
+        before do
+          setting.auto_fix_container_scanning = false
+          setting.auto_fix_dependency_scanning = false
+          setting.auto_fix_sast = false
+        end
+
+        it 'marks auto_fix as disabled' do
+          is_expected.to be_falsey
+        end
+      end
+
+      context 'when feature is disabled' do
+        before do
+          stub_feature_flags(security_auto_fix: false)
+        end
+
+        it 'marks auto_fix as disabled' do
+          is_expected.to be_falsey
+        end
       end
     end
 
-    context 'with existing setting' do
+    context 'when license feature is disabled' do
       before do
-        project.create_security_setting
+        stub_licensed_features(vulnerability_auto_fix: false)
       end
 
-      it 'reuses existing entry' do
-        expect { subject }.not_to change { ProjectSecuritySetting.count }
-        expect(subject).to be_a_kind_of(ProjectSecuritySetting)
+      it 'marks auto_fix as disabled' do
+        is_expected.to be_falsey
       end
     end
   end
+
+  describe '#auto_fix_enabled_types' do
+    subject { setting.auto_fix_enabled_types }
+
+    let_it_be(:setting) { build(:project_security_setting) }
+
+    before do
+      setting.auto_fix_container_scanning = false
+      setting.auto_fix_dependency_scanning = true
+      setting.auto_fix_sast = true
+    end
+
+    it 'return status only for available types' do
+      is_expected.to eq([:dependency_scanning])
+    end
+  end
 end

ee/spec/policies/project_policy_spec.rb

@@ -527,6 +527,69 @@ RSpec.describe ProjectPolicy do
     end
   end
 
+  describe 'permissions for security bot' do
+    let_it_be(:current_user) { create(:user, :security_bot) }
+    let(:project) { private_project }
+
+    let(:permissions) do
+      %i(
+        reporter_access
+        push_code
+        create_merge_request_from
+        create_merge_request_in
+        create_vulnerability_feedback
+        read_project
+      )
+    end
+
+    context 'when auto_fix feature is enabled' do
+      context 'when licensed feature is enabled' do
+        before do
+          stub_licensed_features(vulnerability_auto_fix: true)
+        end
+
+        it { is_expected.to be_allowed(*permissions) }
+
+        context 'when feature flag is disabled' do
+          before do
+            stub_feature_flags(security_auto_fix: false)
+          end
+
+          it { is_expected.to be_disallowed(*permissions) }
+        end
+      end
+
+      context 'when licensed feature is disabled' do
+        before do
+          stub_licensed_features(vulnerability_auto_fix: false)
+        end
+
+        it { is_expected.to be_disallowed(*permissions) }
+      end
+    end
+
+    context 'when auto_fix feature is disabled' do
+      before do
+        stub_licensed_features(vulnerability_auto_fix: true)
+        project.security_setting.update!(auto_fix_dependency_scanning: false, auto_fix_container_scanning: false)
+      end
+
+      it { is_expected.to be_disallowed(*permissions) }
+    end
+
+    context 'when project does not have a security_setting' do
+      before do
+        stub_licensed_features(vulnerability_auto_fix: true)
+        project.security_setting.delete
+        project.reload
+      end
+
+      it do
+        is_expected.to be_disallowed(*permissions)
+      end
+    end
+  end
+
   describe 'read_threat_monitoring' do
     context 'when threat monitoring feature is available' do
       before do

ee/spec/policies/vulnerabilities/feedback_policy_spec.rb

@@ -71,6 +71,27 @@ RSpec.describe Vulnerabilities::FeedbackPolicy do
           is_expected.to be_disallowed(:create_vulnerability_feedback)
         end
       end
+
+      context 'with security bot' do
+        let(:current_user) { create(:user, :security_bot) }
+
+        before do
+          stub_licensed_features(vulnerability_auto_fix: true)
+        end
+
+        context 'when auto-fix is enabled' do
+          it { is_expected.to be_allowed(:create_vulnerability_feedback) }
+        end
+
+        context 'when auto-fix is disabled' do
+          before do
+            project.security_setting.update!(auto_fix_dependency_scanning: false,
+                                             auto_fix_container_scanning: false)
+          end
+
+          it { is_expected.to be_disallowed(:create_vulnerability_feedback) }
+        end
+      end
     end
   end
 

ee/spec/services/security/configuration/save_auto_fix_service_spec.rb

@@ -4,7 +4,7 @@ require 'spec_helper'
 
 RSpec.describe Security::Configuration::SaveAutoFixService do
   describe '#execute' do
-    let_it_be(:project) { create(:project) }
+    let_it_be_with_reload(:project) { create(:project) }
 
     subject(:service) { described_class.new(project, feature) }
 
@@ -32,10 +32,6 @@ RSpec.describe Security::Configuration::SaveAutoFixService do
     context 'with not supported scanner type' do
       let(:feature) { :dep_scan }
 
-      before do
-        project.create_security_setting
-      end
-
       it 'does not change setting' do
         expect(project.security_setting.auto_fix_dependency_scanning).to be_truthy
       end

spec/factories/users.rb

@@ -47,6 +47,10 @@ FactoryBot.define do
       user_type { :migration_bot }
     end
 
+    trait :security_bot do
+      user_type { :security_bot }
+    end
+
     trait :external do
       external { true }
     end

spec/policies/global_policy_spec.rb

@@ -7,6 +7,8 @@ RSpec.describe GlobalPolicy do
 
   let_it_be(:project_bot) { create(:user, :project_bot) }
   let_it_be(:migration_bot) { create(:user, :migration_bot) }
+  let_it_be(:security_bot) { create(:user, :security_bot) }
+
   let(:current_user) { create(:user) }
   let(:user) { create(:user) }
 
@@ -223,6 +225,12 @@ RSpec.describe GlobalPolicy do
       it { is_expected.not_to be_allowed(:access_api) }
     end
 
+    context 'security bot' do
+      let(:current_user) { security_bot }
+
+      it { is_expected.not_to be_allowed(:access_api) }
+    end
+
     context 'user blocked pending approval' do
       before do
         current_user.block_pending_approval
@@ -353,6 +361,12 @@ RSpec.describe GlobalPolicy do
       it { is_expected.to be_allowed(:access_git) }
     end
 
+    context 'security bot' do
+      let(:current_user) { security_bot }
+
+      it { is_expected.to be_allowed(:access_git) }
+    end
+
     describe 'deactivated user' do
       before do
         current_user.deactivate
@@ -513,6 +527,12 @@ RSpec.describe GlobalPolicy do
       it { is_expected.not_to be_allowed(:log_in) }
     end
 
+    context 'security bot' do
+      let(:current_user) { security_bot }
+
+      it { is_expected.not_to be_allowed(:log_in) }
+    end
+
     context 'user blocked pending approval' do
       before do
         current_user.block_pending_approval