Merge branch 'sk/499432-compliance-framework' into 'master'

Sync security policies when compliance framework is changed

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/173259



Merged-by: Marcos Rocha <mrocha@gitlab.com>
Approved-by: Martin Čavoj <mcavoj@gitlab.com>
Approved-by: Marcos Rocha <mrocha@gitlab.com>
Reviewed-by: Martin Čavoj <mcavoj@gitlab.com>
Co-authored-by: Sashi Kumar Kumaresan <skumar@gitlab.com>

ee/app/models/security/orchestration_policy_configuration.rb

@@ -46,6 +46,7 @@ class OrchestrationPolicyConfiguration < ApplicationRecord
     scope :for_namespace, ->(namespace_id) { where(namespace_id: namespace_id) }
     scope :with_project_and_namespace, -> { includes(:project, :namespace) }
     scope :for_management_project, ->(management_project_id) { where(security_policy_management_project_id: management_project_id) }
+    scope :with_security_policies, -> { includes(:security_policies) }
     scope :with_outdated_configuration, -> do
       joins(:security_policy_management_project)
         .where(arel_table[:configured_at].lt(Project.arel_table[:last_repository_updated_at]).or(arel_table[:configured_at].eq(nil)))

ee/app/models/security/policy.rb

@@ -161,6 +161,12 @@ def scope_applicable?(project)
       policy_scope_checker.security_policy_applicable?(self)
     end
 
+    def scope_has_framework?(compliance_framework_id)
+      scope
+        .deep_symbolize_keys[:compliance_frameworks].to_a
+        .any? { |framework| framework[:id] == compliance_framework_id }
+    end
+
     def delete_approval_policy_rules
       delete_approval_rules
       delete_policy_violations

ee/app/services/security/security_orchestration_policies/sync_policy_event_service.rb

+# frozen_string_literal: true
+
+module Security
+  module SecurityOrchestrationPolicies
+    class SyncPolicyEventService < BaseProjectPolicyService
+      def initialize(project:, security_policy:, event:)
+        super(project: project, security_policy: security_policy)
+        @event = event
+      end
+
+      def execute
+        case event
+        when Projects::ComplianceFrameworkChangedEvent
+          sync_policy_for_compliance_framework(event)
+        end
+      end
+
+      private
+
+      def sync_policy_for_compliance_framework(event)
+        return unless security_policy.scope_has_framework?(event.data[:compliance_framework_id])
+
+        if event.data[:event_type] == Projects::ComplianceFrameworkChangedEvent::EVENT_TYPES[:added]
+          link_policy
+        else
+          unlink_policy
+        end
+      end
+
+      attr_reader :event
+    end
+  end
+end

ee/app/workers/security/refresh_compliance_framework_security_policies_worker.rb

@@ -18,9 +18,20 @@ def handle_event(event)
       policy_configuration_ids = project.all_security_orchestration_policy_configuration_ids
       return unless policy_configuration_ids.any?
 
-      framework.security_orchestration_policy_configurations.id_in(policy_configuration_ids).find_each do |config|
-        Security::ProcessScanResultPolicyWorker.perform_async(project.id, config.id)
-      end
+      framework
+        .security_orchestration_policy_configurations
+        .with_security_policies.id_in(policy_configuration_ids)
+        .find_each do |config|
+          Security::ProcessScanResultPolicyWorker.perform_async(project.id, config.id)
+
+          config.security_policies.undeleted.find_each do |security_policy|
+            Security::SecurityOrchestrationPolicies::SyncPolicyEventService.new(
+              project: project,
+              security_policy: security_policy,
+              event: event
+            ).execute
+          end
+        end
     end
   end
 end

ee/spec/models/security/policy_spec.rb

@@ -409,6 +409,30 @@
     end
   end
 
+  describe '#scope_has_framework?' do
+    let(:framework) { create(:compliance_framework) }
+    let(:policy_scope) { {} }
+    let(:security_policy) { create(:security_policy, scope: policy_scope) }
+
+    subject(:scope_has_framework?) { security_policy.scope_has_framework?(framework.id) }
+
+    context 'when scope is empty' do
+      it { is_expected.to be_falsey }
+    end
+
+    context 'when scope contains framework_id' do
+      let(:policy_scope) { { compliance_frameworks: [{ id: framework.id }] } }
+
+      it { is_expected.to be_truthy }
+    end
+
+    context 'when scope has a non existing framework_id' do
+      let(:policy_scope) { { compliance_frameworks: [{ id: non_existing_record_id }] } }
+
+      it { is_expected.to be_falsey }
+    end
+  end
+
   describe '#delete_approval_policy_rules' do
     let_it_be(:policy) { create(:security_policy, :require_approval) }
     let_it_be(:other_policy) { create(:security_policy, :require_approval) }

ee/spec/services/security/security_orchestration_policies/sync_policy_event_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Security::SecurityOrchestrationPolicies::SyncPolicyEventService, feature_category: :security_policy_management do
+  let_it_be(:project) { create(:project) }
+  let_it_be(:compliance_framework) { create(:compliance_framework) }
+
+  let(:policy_scope) { { compliance_frameworks: [{ id: compliance_framework.id }] } }
+  let(:security_policy) do
+    create(:security_policy, scope: policy_scope)
+  end
+
+  before do
+    create(:compliance_framework_project_setting,
+      project: project,
+      compliance_management_framework: compliance_framework
+    )
+  end
+
+  subject(:execute) { described_class.new(project: project, security_policy: security_policy, event: event).execute }
+
+  describe '#execute' do
+    context 'when event is ComplianceFrameworkChangedEvent' do
+      let(:event) do
+        Projects::ComplianceFrameworkChangedEvent.new(data: {
+          project_id: project.id,
+          compliance_framework_id: compliance_framework.id,
+          event_type: event_type
+        })
+      end
+
+      shared_examples 'when policy scope does not match compliance_framework' do
+        context 'when policy scope does not have compliance_framework' do
+          let(:policy_scope) { {} }
+
+          it 'does nothing' do
+            expect { execute }.not_to change { Security::PolicyProjectLink.count }
+          end
+        end
+
+        context 'when policy scope has a different compliance framework' do
+          let_it_be(:other_compliance_framework) { create(:compliance_framework) }
+          let(:policy_scope) { { compliance_frameworks: [{ id: other_compliance_framework.id }] } }
+
+          it 'does nothing' do
+            expect { execute }.not_to change { Security::PolicyProjectLink.count }
+          end
+        end
+      end
+
+      context 'when framework is added' do
+        let(:event_type) { Projects::ComplianceFrameworkChangedEvent::EVENT_TYPES[:added] }
+
+        it 'links policy to project' do
+          expect { execute }.to change { Security::PolicyProjectLink.count }.by(1)
+
+          expect(project.security_policies).to contain_exactly(security_policy)
+        end
+
+        it_behaves_like 'when policy scope does not match compliance_framework'
+      end
+
+      context 'when framework is removed' do
+        let(:event_type) { Projects::ComplianceFrameworkChangedEvent::EVENT_TYPES[:removed] }
+
+        context 'when policy is linked to the project' do
+          before do
+            create(:security_policy_project_link, project: project, security_policy: security_policy)
+          end
+
+          it 'unlinks policy from project' do
+            expect { execute }.to change { Security::PolicyProjectLink.count }.by(-1)
+
+            expect(project.reload.security_policies).to be_empty
+          end
+        end
+
+        context 'when policy is not linked to the project' do
+          it 'does nothing' do
+            expect { execute }.not_to change { Security::PolicyProjectLink.count }
+          end
+        end
+
+        it_behaves_like 'when policy scope does not match compliance_framework'
+      end
+    end
+  end
+end

ee/spec/workers/security/refresh_compliance_framework_security_policies_worker_spec.rb

@@ -65,4 +65,62 @@
 
     consume_event(subscriber: described_class, event: compliance_framework_changed_event)
   end
+
+  context 'with security_policies' do
+    let_it_be(:security_policy) do
+      create(:security_policy,
+        security_orchestration_policy_configuration: policy_configuration,
+        scope: { compliance_frameworks: [{ id: compliance_framework.id }] }
+      )
+    end
+
+    let_it_be(:deleted_security_policy) do
+      create(:security_policy, :deleted,
+        security_orchestration_policy_configuration: policy_configuration,
+        scope: { compliance_frameworks: [{ id: compliance_framework.id }] }
+      )
+    end
+
+    let_it_be(:project_security_policy) do
+      create(:security_policy,
+        security_orchestration_policy_configuration: project_policy_configuration,
+        scope: { compliance_frameworks: [{ id: compliance_framework.id }] }
+      )
+    end
+
+    let_it_be(:other_security_policy) do
+      create(:security_policy,
+        security_orchestration_policy_configuration: other_policy_configuration,
+        scope: { compliance_frameworks: [{ id: compliance_framework.id }] }
+      )
+    end
+
+    before do
+      create(:compliance_framework_project_setting,
+        project: project,
+        compliance_management_framework: compliance_framework
+      )
+    end
+
+    it 'invokes Security::SecurityOrchestrationPolicies::SyncPolicyEventService for undeleted policies' do
+      expect_next_instance_of(
+        Security::SecurityOrchestrationPolicies::SyncPolicyEventService,
+        project: project,
+        security_policy: security_policy,
+        event: an_instance_of(::Projects::ComplianceFrameworkChangedEvent)
+      ) do |instance|
+        expect(instance).to receive(:execute)
+      end
+      expect_next_instance_of(
+        Security::SecurityOrchestrationPolicies::SyncPolicyEventService,
+        project: project,
+        security_policy: project_security_policy,
+        event: an_instance_of(::Projects::ComplianceFrameworkChangedEvent)
+      ) do |instance|
+        expect(instance).to receive(:execute)
+      end
+
+      consume_event(subscriber: described_class, event: compliance_framework_changed_event)
+    end
+  end
 end