diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index cc7c8c13e42ed40a5838d1f537061675acd69807..90ecb8469049b5989609aa38418b8d417d68b1c8 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -530,6 +530,9 @@ Audit event types belong to the following product categories. | Type name | Event triggered when | Saved to database | Introduced in | Scope | |:----------|:---------------------|:------------------|:--------------|:------| | [`policy_project_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/102154) | The security policy project is updated for a project | {{< icon name="check-circle" >}} Yes | GitLab [15.6](https://gitlab.com/gitlab-org/gitlab/-/issues/377877) | Group, Project | +| [`security_policy_create`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/192797) | A security policy is created | {{< icon name="check-circle" >}} Yes | GitLab [18.1](https://gitlab.com/gitlab-org/gitlab/-/issues/539230) | Project | +| [`security_policy_delete`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/192797) | A security policy is deleted | {{< icon name="check-circle" >}} Yes | GitLab [18.1](https://gitlab.com/gitlab-org/gitlab/-/issues/539230) | Project | +| [`security_policy_update`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/192797) | A security policy is updated | {{< icon name="check-circle" >}} Yes | GitLab [18.1](https://gitlab.com/gitlab-org/gitlab/-/issues/539230) | Project | ### Security testing configuration diff --git a/ee/app/models/security/orchestration_policy_configuration.rb b/ee/app/models/security/orchestration_policy_configuration.rb index 03c2247c639222b2b615e3b2b1f8b925aed796ed..1e094f9c511b0cafea97c90bd58ae6a423685aa9 100644 --- a/ee/app/models/security/orchestration_policy_configuration.rb +++ b/ee/app/models/security/orchestration_policy_configuration.rb @@ -113,6 +113,19 @@ def policy_last_updated_at end end + def latest_commit_before_configured_at + return if configured_at.nil? + + capture_git_error(:commits) do + policy_repo.commits( + default_branch_or_main, + before: configured_at, + limit: 1 + ).first + end + end + strong_memoize_attr :latest_commit_before_configured_at + def policy_by_type(type_or_types) return [] if policy_hash.blank? @@ -244,6 +257,12 @@ def experiment_configuration(experimental_feature_name) experiments.dig(experimental_feature_name.to_s, 'configuration') || {} end + def first_configuration_for_the_management_project? + self.class.for_management_project(security_policy_management_project_id) + .where(self.class.arel_table[:created_at].lt(created_at)) + .none? + end + private def yaml_differs_from_db?(policies_persisted_in_database, policies_in_policy_yaml) diff --git a/ee/app/services/security/security_orchestration_policies/collect_policies_audit_events.rb b/ee/app/services/security/security_orchestration_policies/collect_policies_audit_events.rb new file mode 100644 index 0000000000000000000000000000000000000000..1fad11b3f5680202a12b0920f3dfae4e598193dd --- /dev/null +++ b/ee/app/services/security/security_orchestration_policies/collect_policies_audit_events.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +module Security + module SecurityOrchestrationPolicies + class CollectPoliciesAuditEvents + include BaseServiceUtility + include Gitlab::Utils::StrongMemoize + + def initialize(policy_configuration:, created_policies: [], updated_policies: [], deleted_policies: []) + @policy_configuration = policy_configuration + @created_policies = created_policies + @updated_policies = updated_policies + @deleted_policies = deleted_policies + end + + def execute + bulk_audit_policies(created_policies, 'security_policy_create', 'Created') + bulk_audit_policies(updated_policies, 'security_policy_update', 'Updated') + bulk_audit_policies(deleted_policies, 'security_policy_delete', 'Deleted') + end + + private + + attr_reader :created_policies, :updated_policies, :deleted_policies, :policy_configuration + + def commit + policy_configuration.latest_commit_before_configured_at + end + + def policy_management_project + policy_configuration.security_policy_management_project + end + strong_memoize_attr :policy_management_project + + def bulk_audit_policies(policies, event_name, action) + policies.each do |policy| + audit_context = build_audit_context(policy, event_name, action) + + ::Gitlab::Audit::Auditor.audit(audit_context) + end + end + + def build_audit_context(policy, event_name, action) + { + name: event_name, + author: commit&.author || Gitlab::Audit::DeletedAuthor.new(id: -3, name: 'Unknown User'), + scope: policy_management_project, + target: policy, + target_details: policy.name, + message: audit_message(policy, action), + created_at: Time.current, + additional_details: { + policy_name: policy.name, + event_name: event_name, + policy_type: policy.type, + security_policy_project_commit_sha: commit&.sha, + security_policy_project_id: policy_management_project.id, + policy_configured_at: policy_configuration.configured_at + } + } + end + + def audit_message(policy, action) + "#{action} security policy with the name: \"#{policy.name}\"" + end + end + end +end diff --git a/ee/app/services/security/security_orchestration_policies/persist_policy_service.rb b/ee/app/services/security/security_orchestration_policies/persist_policy_service.rb index b97f9418fd721acdea593d4f16811ca2ca670391..7bc65f589b9b7cb6bac80dcdb854185a0dc5f8f8 100644 --- a/ee/app/services/security/security_orchestration_policies/persist_policy_service.rb +++ b/ee/app/services/security/security_orchestration_policies/persist_policy_service.rb @@ -32,12 +32,13 @@ def execute db_policies, policies ) created_policies = [] + updated_policies = [] ApplicationRecord.transaction do mark_policies_for_deletion(deleted_policies) update_rearranged_policies(rearranged_policies) created_policies = create_policies(new_policies) - update_policies(policies_changes) + updated_policies = update_policies(policies_changes) end Gitlab::AppJsonLogger.debug( @@ -52,6 +53,15 @@ def execute ) ) + if collect_audit_events_feature_enabled? && collect_audit_events_for_the_config? + CollectPoliciesAuditEvents.new( + policy_configuration: policy_configuration, + created_policies: created_policies, + updated_policies: updated_policies, + deleted_policies: deleted_policies + ).execute + end + Security::SecurityOrchestrationPolicies::EventPublisher.new( db_policies: db_policies.undeleted, created_policies: created_policies, @@ -79,7 +89,7 @@ def create_policies(new_policies) end def update_policies(policies_changes) - return if policies_changes.empty? + return [] if policies_changes.empty? Security::SecurityOrchestrationPolicies::UpdateSecurityPoliciesService.new( policies_changes: policies_changes @@ -116,6 +126,15 @@ def relation_scope when :pipeline_execution_schedule_policy then Security::Policy.type_pipeline_execution_schedule_policy end end + + def collect_audit_events_feature_enabled? + Feature.enabled?(:collect_security_policy_manage_audit_events, + policy_configuration.security_policy_management_project) + end + + def collect_audit_events_for_the_config? + policy_configuration.first_configuration_for_the_management_project? + end end end end diff --git a/ee/app/services/security/security_orchestration_policies/update_security_policies_service.rb b/ee/app/services/security/security_orchestration_policies/update_security_policies_service.rb index c91ea77440fb1bdc3becfa6bbd5ddc2380c71fa4..d75ab811b7c4f4f6bfb4cc45ddb8fbf93e1a0c53 100644 --- a/ee/app/services/security/security_orchestration_policies/update_security_policies_service.rb +++ b/ee/app/services/security/security_orchestration_policies/update_security_policies_service.rb @@ -8,13 +8,15 @@ def initialize(policies_changes:) end def execute - policies_changes.each do |policy_changes| + policies_changes.each_with_object([]) do |policy_changes, updated_policies| # diff should be computed before updating policy attributes diff = policy_changes.diff policy = update_policy_attributes!(policy_changes.db_policy, policy_changes.yaml_policy) update_policy_rules(policy, diff.rules_diff) policy.update_pipeline_execution_policy_config_link! if policy_changes.diff.content_project_changed? + + updated_policies << policy end end diff --git a/ee/app/workers/concerns/update_orchestration_policy_configuration.rb b/ee/app/workers/concerns/update_orchestration_policy_configuration.rb index 09134f18fee156a5443e6784a219a35bc58ce231..c8c2c44d9082577d62adf751abdd7c3cb2d95562 100644 --- a/ee/app/workers/concerns/update_orchestration_policy_configuration.rb +++ b/ee/app/workers/concerns/update_orchestration_policy_configuration.rb @@ -19,8 +19,6 @@ def update_policy_configuration(configuration, force_resync = false) return end - Security::PersistSecurityPoliciesWorker.perform_async(configuration.id, { force_resync: force_resync }) - configuration.delete_all_schedules configuration.active_scan_execution_policies.each_with_index do |policy, policy_index| Security::SecurityOrchestrationPolicies::ProcessRuleService @@ -29,6 +27,8 @@ def update_policy_configuration(configuration, force_resync = false) end update_configuration_timestamp!(configuration) + + Security::PersistSecurityPoliciesWorker.perform_async(configuration.id, { force_resync: force_resync }) end private diff --git a/ee/config/audit_events/types/security_policy_create.yml b/ee/config/audit_events/types/security_policy_create.yml new file mode 100644 index 0000000000000000000000000000000000000000..6b7d2ff3de6df87d4778f5d4ca7441841301b0f6 --- /dev/null +++ b/ee/config/audit_events/types/security_policy_create.yml @@ -0,0 +1,10 @@ +--- +name: security_policy_create +description: A security policy is created +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/539230 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/192797 +milestone: '18.1' +feature_category: security_policy_management +saved_to_database: true +streamed: true +scope: [Project] diff --git a/ee/config/audit_events/types/security_policy_delete.yml b/ee/config/audit_events/types/security_policy_delete.yml new file mode 100644 index 0000000000000000000000000000000000000000..49d0cf491583411cfde64e6134b5cb979a5aad18 --- /dev/null +++ b/ee/config/audit_events/types/security_policy_delete.yml @@ -0,0 +1,10 @@ +--- +name: security_policy_delete +description: A security policy is deleted +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/539230 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/192797 +milestone: '18.1' +feature_category: security_policy_management +saved_to_database: true +streamed: true +scope: [Project] diff --git a/ee/config/audit_events/types/security_policy_update.yml b/ee/config/audit_events/types/security_policy_update.yml new file mode 100644 index 0000000000000000000000000000000000000000..ff6b32cb5c04b17d208d13915d3fbd857b5a5f47 --- /dev/null +++ b/ee/config/audit_events/types/security_policy_update.yml @@ -0,0 +1,10 @@ +--- +name: security_policy_update +description: A security policy is updated +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/539230 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/192797 +milestone: '18.1' +feature_category: security_policy_management +saved_to_database: true +streamed: true +scope: [Project] diff --git a/ee/config/feature_flags/gitlab_com_derisk/collect_security_policy_manage_audit_events.yml b/ee/config/feature_flags/gitlab_com_derisk/collect_security_policy_manage_audit_events.yml new file mode 100644 index 0000000000000000000000000000000000000000..90875445a005cc8718006375ceb7f3241d66736f --- /dev/null +++ b/ee/config/feature_flags/gitlab_com_derisk/collect_security_policy_manage_audit_events.yml @@ -0,0 +1,10 @@ +--- +name: collect_security_policy_manage_audit_events +description: Enables the collection of security policy management(create/update/delete) related audit events in the security policy project +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/539230 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/192797 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/538934 +milestone: '18.1' +group: group::security policies +type: gitlab_com_derisk +default_enabled: false diff --git a/ee/spec/models/security/orchestration_policy_configuration_spec.rb b/ee/spec/models/security/orchestration_policy_configuration_spec.rb index 6ace2154c13d73ebc203b10c7cbd271a188d4048..56e4661fc71cc3027687f4f91b5abf7da6923812 100644 --- a/ee/spec/models/security/orchestration_policy_configuration_spec.rb +++ b/ee/spec/models/security/orchestration_policy_configuration_spec.rb @@ -2662,6 +2662,67 @@ it_behaves_like 'captures git errors', :last_commit_for_path end + describe '#latest_commit_before_configured_at' do + let_it_be(:repository) { security_policy_management_project.repository } + let_it_be(:default_branch) { security_policy_management_project.default_branch_or_main } + let_it_be(:commit_before) { create(:commit, committed_date: 20.seconds.ago) } + let_it_be(:commit_after) { create(:commit, committed_date: 10.seconds.from_now) } + let_it_be(:configured_at) { Time.current } + + subject(:latest_commit) { security_orchestration_policy_configuration.latest_commit_before_configured_at } + + context 'when configured_at is set' do + before do + security_orchestration_policy_configuration.update!(configured_at: configured_at) + end + + context 'when a commit exists before configured_at' do + before do + allow(repository).to receive(:commits) + .with(default_branch, before: configured_at, limit: 1) + .and_return([commit_before]) + end + + it 'returns the latest commit before configured_at' do + expect(latest_commit).to eq(commit_before) + end + + it 'memoizes the result' do + expect(repository).to receive(:commits).once.and_return([commit_before]) + + security_orchestration_policy_configuration.latest_commit_before_configured_at + security_orchestration_policy_configuration.latest_commit_before_configured_at + end + end + + context 'when no commit exists before configured_at' do + before do + allow(repository).to receive(:commits) + .with(default_branch, before: configured_at, limit: 1) + .and_return([]) + end + + it { is_expected.to be_nil } + end + end + + context 'when configured_at is nil' do + before do + security_orchestration_policy_configuration.update!(configured_at: nil) + end + + it 'returns nil' do + expect(latest_commit).to be_nil + end + end + + it_behaves_like 'captures git errors', :commits do + before do + security_orchestration_policy_configuration.update!(configured_at: configured_at) + end + end + end + describe '#delete_all_schedules' do let(:rule_schedule) { create(:security_orchestration_policy_rule_schedule, security_orchestration_policy_configuration: security_orchestration_policy_configuration) } @@ -3527,6 +3588,40 @@ end end + describe '#first_configuration_for_the_management_project?' do + let_it_be(:management_project_1) { create(:project) } + let_it_be(:management_project_2) { create(:project) } + + let!(:first_config_for_project_1) do + create(:security_orchestration_policy_configuration, + security_policy_management_project: management_project_1, + created_at: 1.day.ago) + end + + let!(:second_config_for_project_1) do + create(:security_orchestration_policy_configuration, + security_policy_management_project: management_project_1, + created_at: Time.current) + end + + let!(:config_other_project) do + create(:security_orchestration_policy_configuration, + security_policy_management_project: management_project_2) + end + + it 'returns true if it is the first configuration for the project' do + expect(first_config_for_project_1.first_configuration_for_the_management_project?).to be true + end + + it 'returns false if there is an older configuration for the same project' do + expect(second_config_for_project_1.first_configuration_for_the_management_project?).to be false + end + + it 'returns true for a configuration that is the only configuration for its own project' do + expect(config_other_project.first_configuration_for_the_management_project?).to be true + end + end + describe '#active_pipeline_execution_policies' do let(:policy_yaml) { fixture_file('security_orchestration.yml', dir: 'ee') } diff --git a/ee/spec/services/security/security_orchestration_policies/collect_policies_audit_events_spec.rb b/ee/spec/services/security/security_orchestration_policies/collect_policies_audit_events_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..d0ab330b137b04bff871ea3ffcd5440c8c87ef66 --- /dev/null +++ b/ee/spec/services/security/security_orchestration_policies/collect_policies_audit_events_spec.rb @@ -0,0 +1,191 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Security::SecurityOrchestrationPolicies::CollectPoliciesAuditEvents, feature_category: :security_policy_management do + let_it_be(:user) { create(:user) } + let_it_be(:policy_project) { create(:project, :repository) } + let_it_be(:fixed_time) { Time.current.round } + let_it_be(:policy_configuration) do + create( + :security_orchestration_policy_configuration, + security_policy_management_project: policy_project, + configured_at: fixed_time + ) + end + + let(:commit_sha) { 'a1b2c3d4e5f6' } + let(:commit) do + create( + :commit, + project: policy_project, + sha: commit_sha, + author: user + ) + end + + let_it_be(:scan_execution_policy) { create(:security_policy, type: :scan_execution_policy) } + let_it_be(:approval_policy) { create(:security_policy, type: :approval_policy) } + let_it_be(:vulnerability_policy) { create(:security_policy, type: :vulnerability_management_policy) } + + let(:created_policies) { [] } + let(:updated_policies) { [] } + let(:deleted_policies) { [] } + + let(:service) do + described_class.new( + policy_configuration: policy_configuration, + created_policies: created_policies, + updated_policies: updated_policies, + deleted_policies: deleted_policies + ) + end + + subject(:execute_service) { service.execute } + + before do + allow(policy_configuration).to receive(:latest_commit_before_configured_at).and_return(commit) + + allow(Time).to receive(:current).and_return(fixed_time) + allow(Gitlab::Audit::Auditor).to receive(:audit) + end + + describe '#execute' do + context 'when created policies are present' do + let(:created_policies) { [scan_execution_policy, approval_policy] } + + it 'audits created policies with correct context' do + execute_service + + created_policies.each do |policy| + expected_message = "Created security policy with the name: \"#{policy.name}\"" + expect(Gitlab::Audit::Auditor).to have_received(:audit).with( + hash_including( + name: 'security_policy_create', + author: user, + scope: policy_project, + target: policy, + target_details: policy.name, + message: expected_message, + created_at: fixed_time, + additional_details: { + policy_name: policy.name, + event_name: 'security_policy_create', + policy_type: policy.type, + security_policy_project_commit_sha: commit_sha, + security_policy_project_id: policy_project.id, + policy_configured_at: policy_configuration.configured_at + } + ) + ) + end + end + end + + context 'when updated policies are present' do + let(:updated_policies) { [approval_policy] } + + it 'audits updated policies with correct context' do + execute_service + + updated_policies.each do |policy| + expected_message = "Updated security policy with the name: \"#{policy.name}\"" + expect(Gitlab::Audit::Auditor).to have_received(:audit).with( + hash_including( + name: 'security_policy_update', + author: user, + scope: policy_project, + target: policy, + target_details: policy.name, + message: expected_message, + created_at: fixed_time, + additional_details: { + policy_name: policy.name, + event_name: 'security_policy_update', + policy_type: policy.type, + security_policy_project_commit_sha: commit_sha, + security_policy_project_id: policy_project.id, + policy_configured_at: policy_configuration.configured_at + } + ) + ) + end + end + end + + context 'when deleted policies are present' do + let(:deleted_policies) { [vulnerability_policy] } + + it 'audits deleted policies with correct context' do + execute_service + + deleted_policies.each do |policy| + expected_message = "Deleted security policy with the name: \"#{policy.name}\"" + expect(Gitlab::Audit::Auditor).to have_received(:audit).with( + hash_including( + name: 'security_policy_delete', + author: user, + scope: policy_project, + target: policy, + target_details: policy.name, + message: expected_message, + created_at: fixed_time, + additional_details: { + policy_name: policy.name, + event_name: 'security_policy_delete', + policy_type: policy.type, + security_policy_project_commit_sha: commit_sha, + security_policy_project_id: policy_project.id, + policy_configured_at: policy_configuration.configured_at + } + ) + ) + end + end + end + + context 'when policy lists are empty' do + it 'does not call the auditor' do + execute_service + expect(Gitlab::Audit::Auditor).not_to have_received(:audit) + end + end + + context 'when commit cannot be in the repository' do + let(:created_policies) { [scan_execution_policy] } + let(:default_user) { Gitlab::Audit::DeletedAuthor.new(id: -3, name: 'Unknown User') } + + before do + allow(policy_configuration).to receive(:latest_commit_before_configured_at).and_return(nil) + allow(Gitlab::Audit::DeletedAuthor).to receive(:new).and_return(default_user) + end + + it 'uses default user as the author and commit sha as nil for audit context' do + execute_service + + created_policies.each do |policy| + expected_message = "Created security policy with the name: \"#{policy.name}\"" + expect(Gitlab::Audit::Auditor).to have_received(:audit).with( + hash_including( + name: 'security_policy_create', + author: default_user, + scope: policy_project, + target: policy, + target_details: policy.name, + message: expected_message, + created_at: fixed_time, + additional_details: { + policy_name: policy.name, + event_name: 'security_policy_create', + policy_type: policy.type, + security_policy_project_commit_sha: nil, + security_policy_project_id: policy_project.id, + policy_configured_at: policy_configuration.configured_at + } + ) + ) + end + end + end + end +end diff --git a/ee/spec/services/security/security_orchestration_policies/persist_policy_service_spec.rb b/ee/spec/services/security/security_orchestration_policies/persist_policy_service_spec.rb index a2c9cbefb514404e66c416a5e494c36a2bccebc1..3e43a92032548e19ebeeb700f447f44b9dc86e4e 100644 --- a/ee/spec/services/security/security_orchestration_policies/persist_policy_service_spec.rb +++ b/ee/spec/services/security/security_orchestration_policies/persist_policy_service_spec.rb @@ -3,7 +3,10 @@ require 'spec_helper' RSpec.describe Security::SecurityOrchestrationPolicies::PersistPolicyService, '#execute', feature_category: :security_policy_management do - let_it_be_with_reload(:policy_configuration) { create(:security_orchestration_policy_configuration) } + let_it_be(:policy_project) { create(:project, :repository) } + let_it_be_with_reload(:policy_configuration) do + create(:security_orchestration_policy_configuration, security_policy_management_project: policy_project) + end def persist!(policies) described_class @@ -87,6 +90,17 @@ def persist_with_force_resync! expect { persist }.to change { policy_configuration.security_policies.reload.type_approval_policy.count }.by(3) end + it 'calls CollectPoliciesAuditService with created policies' do + expect(Security::SecurityOrchestrationPolicies::CollectPoliciesAuditEvents).to receive(:new).with( + policy_configuration: policy_configuration.reload, + created_policies: policy_configuration.security_policies.reload.type_approval_policy, + updated_policies: [], + deleted_policies: [] + ).and_call_original + + persist + end + it 'calls EventPublisher with created policies' do expect(Security::SecurityOrchestrationPolicies::EventPublisher).to receive(:new).with({ db_policies: policy_configuration.security_policies.reload.type_approval_policy.undeleted, @@ -306,6 +320,17 @@ def persist_with_force_resync! persist end + it 'calls CollectPoliciesAuditService with deleted policies' do + expect(Security::SecurityOrchestrationPolicies::CollectPoliciesAuditEvents).to receive(:new).with({ + policy_configuration: policy_configuration, + created_policies: [], + updated_policies: [], + deleted_policies: [Security::Policy.first] + }).and_call_original + + persist + end + it 'calls EventPublisher with deleted policies' do expect(Security::SecurityOrchestrationPolicies::EventPublisher).to receive(:new).with({ db_policies: policy_configuration.security_policies.reload.type_approval_policy.undeleted, @@ -319,6 +344,17 @@ def persist_with_force_resync! end context 'with force_resync' do + it 'calls CollectPoliciesAuditService with deleted policies' do + expect(Security::SecurityOrchestrationPolicies::CollectPoliciesAuditEvents).to receive(:new).with({ + policy_configuration: policy_configuration, + created_policies: [], + updated_policies: [], + deleted_policies: [Security::Policy.first] + }).and_call_original + + persist_with_force_resync! + end + it 'calls EventPublisher with force_resync set to true' do expect(Security::SecurityOrchestrationPolicies::EventPublisher).to receive(:new).with({ db_policies: policy_configuration.security_policies.reload.type_approval_policy.undeleted, @@ -404,6 +440,17 @@ def persist_with_force_resync! }.from(contain_exactly(0, 1)).to(contain_exactly(0, -1)) end + it 'calls CollectPoliciesAuditService with updated policies' do + expect(Security::SecurityOrchestrationPolicies::CollectPoliciesAuditEvents).to receive(:new).with({ + policy_configuration: policy_configuration, + created_policies: [], + updated_policies: [Security::Policy.first], + deleted_policies: [] + }).and_call_original + + persist + end + it 'calls EventPublisher with deleted policies' do expect(Security::SecurityOrchestrationPolicies::EventPublisher).to receive(:new).with({ db_policies: policy_configuration.security_policies.reload.type_approval_policy.undeleted, @@ -417,6 +464,17 @@ def persist_with_force_resync! end context 'with force_resync' do + it 'calls CollectPoliciesAuditService with updated policies' do + expect(Security::SecurityOrchestrationPolicies::CollectPoliciesAuditEvents).to receive(:new).with({ + policy_configuration: policy_configuration, + created_policies: [], + updated_policies: [Security::Policy.first], + deleted_policies: [] + }).and_call_original + + persist_with_force_resync! + end + it 'calls EventPublisher with force_resync set to true' do expect(Security::SecurityOrchestrationPolicies::EventPublisher).to receive(:new).with({ db_policies: policy_configuration.security_policies.reload.type_approval_policy.undeleted, @@ -733,6 +791,64 @@ def persist_with_force_resync! end end + describe "collect policy audit events" do + context "when feature is disabled" do + let(:policies) { [build(:approval_policy)] } + let(:policy_type) { :approval_policy } + + before do + stub_feature_flags(collect_security_policy_manage_audit_events: false) + end + + it 'does not call CollectPoliciesAuditService' do + expect(Security::SecurityOrchestrationPolicies::CollectPoliciesAuditEvents).not_to receive(:new) + + persist!(policies) + end + end + + context "when policy project is linked to multiple projects" do + let_it_be(:another_policy_configuration) do + create(:security_orchestration_policy_configuration, security_policy_management_project: policy_project) + end + + let(:approval_policy) { build(:approval_policy) } + let(:scan_execution_policy) { build(:scan_execution_policy) } + + subject(:execute) do + described_class + .new(policy_configuration: policy_configuration, + policies: [approval_policy], + policy_type: :approval_policy) + .execute + + described_class + .new(policy_configuration: another_policy_configuration, + policies: [scan_execution_policy], + policy_type: :scan_execution_policy) + .execute + end + + it 'calls CollectPoliciesAuditService for the first policy configuration only' do + expect(Security::SecurityOrchestrationPolicies::CollectPoliciesAuditEvents).to receive(:new).with( + anything).and_call_original.once + + execute + end + + it 'calls CollectPoliciesAuditService with correct attributes' do + expect(Security::SecurityOrchestrationPolicies::CollectPoliciesAuditEvents).to receive(:new).with({ + policy_configuration: policy_configuration, + created_policies: policy_configuration.security_policies.reload.type_approval_policy, + updated_policies: [], + deleted_policies: [] + }).and_call_original + + execute + end + end + end + context "with unrecognized policy type" do let(:policies) { [] } let(:policy_type) { :foobar } diff --git a/ee/spec/services/security/security_orchestration_policies/update_security_policies_service_spec.rb b/ee/spec/services/security/security_orchestration_policies/update_security_policies_service_spec.rb index c296cde9dc138022eac0f4df8090a5f7875a7a60..9b52ad3a960d640e68460c8e08b8d2b2ede399f7 100644 --- a/ee/spec/services/security/security_orchestration_policies/update_security_policies_service_spec.rb +++ b/ee/spec/services/security/security_orchestration_policies/update_security_policies_service_spec.rb @@ -5,7 +5,7 @@ RSpec.describe Security::SecurityOrchestrationPolicies::UpdateSecurityPoliciesService, feature_category: :security_policy_management do let(:service) { described_class.new(policies_changes: policies_changes) } - let_it_be(:db_policy) do + let_it_be_with_reload(:db_policy) do create(:security_policy, name: 'Policy', description: 'description') end @@ -41,7 +41,7 @@ } end - let_it_be(:yaml_policy) do + let(:yaml_policy) do { name: 'Policy', description: 'description', @@ -66,7 +66,15 @@ end describe '#execute' do + shared_examples 'returning the policies array' do + it 'returns the policies array' do + expect(service.execute).to eq([db_policy]) + end + end + context 'when there are no policy changes' do + it_behaves_like 'returning the policies array' + it 'does not update any policies' do expect { service.execute }.to not_change { db_policy.name } .and not_change { db_policy.description } @@ -83,6 +91,8 @@ } end + it_behaves_like 'returning the policies array' + it 'updates policy attributes and rules' do expect { service.execute }.to change { db_policy.name }.to('Updated Policy') .and change { db_policy.description }.to('Updated description') @@ -108,6 +118,8 @@ } end + it_behaves_like 'returning the policies array' + it 'sets the deleted rules to be deleted' do expect { service.execute }.to change { Security::ApprovalPolicyRule.last.rule_index }.from(2).to(-2) .and change { @@ -125,6 +137,8 @@ } end + it_behaves_like 'returning the policies array' + it 'sets the rule_index to negative values' do service.execute @@ -142,6 +156,8 @@ } end + it_behaves_like 'returning the policies array' + it 'updates the existing rules' do expect { service.execute }.to change { Security::ApprovalPolicyRule.last.typed_content.deep_symbolize_keys @@ -158,6 +174,8 @@ } end + it_behaves_like 'returning the policies array' + it 'updates the existing rule and creates a new rule' do service.execute @@ -190,6 +208,8 @@ db_policy.update_pipeline_execution_policy_config_link! end + it_behaves_like 'returning the policies array' + context 'when project in the content gets updated' do it 'updates the policy config links' do expect { service.execute }.to change { db_policy.reload.security_pipeline_execution_policy_config_link } @@ -205,6 +225,8 @@ } end + it_behaves_like 'returning the policies array' + it 'does not update the policy config links' do expect { service.execute }.not_to change { db_policy.reload.security_pipeline_execution_policy_config_link } end @@ -218,6 +240,8 @@ } end + it_behaves_like 'returning the policies array' + it 'does not update the policy config links' do expect { service.execute }.not_to change { db_policy.reload.security_pipeline_execution_policy_config_link } end diff --git a/ee/spec/workers/concerns/update_orchestration_policy_configuration_spec.rb b/ee/spec/workers/concerns/update_orchestration_policy_configuration_spec.rb index 09811ea372bfd7e242c54709c59be6e7515e91e3..671187a8c3ba8b11d291baf7201387910f3ef413 100644 --- a/ee/spec/workers/concerns/update_orchestration_policy_configuration_spec.rb +++ b/ee/spec/workers/concerns/update_orchestration_policy_configuration_spec.rb @@ -84,6 +84,24 @@ def self.name expect { execute }.to change { configuration.reload.configured_at }.from(nil).to(Time.current) end + # This spec ensures that `configured_at` is updated before enqueuing the `PersistSecurityPoliciesWorker` + # as we fetch the `latest_commit_before_configured_at` inside `CollectPoliciesAuditEvents`. + it 'updates the configuration.configured_at before enqueuing PersistSecurityPoliciesWorker', :freeze_time do + expect(configuration).to receive(:update!).with(experiments: anything).ordered.and_call_original + + expect(configuration).to receive(:update!) + .with(configured_at: Time.current) + .ordered + .and_call_original + + expect(Security::PersistSecurityPoliciesWorker).to receive(:perform_async) + .with(configuration.id, { force_resync: false }) + .ordered + .and_call_original + + execute + end + it 'executes ProcessRuleService for each policy' do active_policies[:scan_execution_policy].each_with_index do |policy, policy_index| expect_next_instance_of(