diff --git a/config/audit_events/types/policy_project_updated.yml b/config/audit_events/types/policy_project_updated.yml new file mode 100644 index 0000000000000000000000000000000000000000..8692486b5cb628fad109c533bf4e3fa2ac30ebd1 --- /dev/null +++ b/config/audit_events/types/policy_project_updated.yml @@ -0,0 +1,8 @@ +name: policy_project_updated +description: "This event is triggered whenever the security policy project is updated for a project." +introduced_by_issue: "https://gitlab.com/gitlab-org/gitlab/-/issues/377877" +introduced_by_mr: "https://gitlab.com/gitlab-org/gitlab/-/merge_requests/102154" +milestone: 15.6 +group: "govern::security policies" +saved_to_database: true +streamed: false diff --git a/ee/app/services/security/orchestration/assign_service.rb b/ee/app/services/security/orchestration/assign_service.rb index e3d4b3bd63412a818c309b4a4fa9d73fa81222c6..89ace4bbfc7f943df209dd8890971db3d0260af8 100644 --- a/ee/app/services/security/orchestration/assign_service.rb +++ b/ee/app/services/security/orchestration/assign_service.rb @@ -17,25 +17,63 @@ def execute private def create_or_update_security_policy_configuration - if policy_project_id.blank? && has_existing_policy? - return unassign_policy_project - end + return unassign_policy_project if policy_project_id.blank? && has_existing_policy? policy_project = Project.find(policy_project_id) - if has_existing_policy? - container.security_orchestration_policy_configuration.update!( - security_policy_management_project_id: policy_project.id - ) - else - container.create_security_orchestration_policy_configuration! do |p| - p.security_policy_management_project_id = policy_project.id - end + return reassign_policy_project(policy_project) if has_existing_policy? + + assign_policy_project(policy_project) + end + + def assign_policy_project(policy_project) + create_result = container.create_security_orchestration_policy_configuration! do |p| + p.security_policy_management_project_id = policy_project.id end + + if create_result + audit(policy_project, "Linked #{policy_project.name} as the security policy project") + end + + create_result end def unassign_policy_project - container.security_orchestration_policy_configuration.delete + old_policy_project = container.security_orchestration_policy_configuration.security_policy_management_project + delete_result = container.security_orchestration_policy_configuration.delete + + if delete_result + audit(old_policy_project, "Unlinked #{old_policy_project.name} as the security policy project") + end + + delete_result + end + + def reassign_policy_project(policy_project) + old_policy_project = container.security_orchestration_policy_configuration.security_policy_management_project + + update_result = container.security_orchestration_policy_configuration.update!( + security_policy_management_project_id: policy_project.id + ) + + if update_result + audit( + policy_project, + "Changed the linked security policy project from #{old_policy_project.name} to #{policy_project.name}" + ) + end + + update_result + end + + def audit(policy_project, message) + ::Gitlab::Audit::Auditor.audit( + name: 'policy_project_updated', + author: current_user, + scope: container, + target: policy_project, + message: message + ) end def success diff --git a/ee/app/services/security/orchestration/unassign_service.rb b/ee/app/services/security/orchestration/unassign_service.rb index 8ff033788dff3e870dd48f58db40e1e1c83cb4ad..3d275122e156eafb27d46eb8e6c65a1d802edfff 100644 --- a/ee/app/services/security/orchestration/unassign_service.rb +++ b/ee/app/services/security/orchestration/unassign_service.rb @@ -6,10 +6,21 @@ class UnassignService < ::BaseContainerService def execute return error(_('Policy project doesn\'t exist')) unless security_orchestration_policy_configuration - security_orchestration_policy_configuration.delete_scan_finding_rules # To be removed in https://gitlab.com/gitlab-org/gitlab/-/issues/369473#feature-update + old_policy_project = security_orchestration_policy_configuration.security_policy_management_project + security_orchestration_policy_configuration.delete_scan_finding_rules # To be removed in https://gitlab.com/gitlab-org/gitlab/-/issues/369473#feature-update result = security_orchestration_policy_configuration.delete - return success if result + + if result + ::Gitlab::Audit::Auditor.audit( + name: 'policy_project_updated', + author: current_user, + scope: container, + target: old_policy_project, + message: "Unlinked #{old_policy_project.name} as the security policy project" + ) + return success + end error(container.security_orchestration_policy_configuration.errors.full_messages.to_sentence) end diff --git a/ee/spec/services/security/orchestration/assign_service_spec.rb b/ee/spec/services/security/orchestration/assign_service_spec.rb index 3daf3b0c84134c45e015a470b54d4589ed8b34fa..21aac4384999ba7dd7b27f44af10fe065a479a82 100644 --- a/ee/spec/services/security/orchestration/assign_service_spec.rb +++ b/ee/spec/services/security/orchestration/assign_service_spec.rb @@ -5,6 +5,7 @@ RSpec.describe Security::Orchestration::AssignService do let_it_be(:project, reload: true) { create(:project) } let_it_be(:another_project) { create(:project) } + let_it_be(:current_user) { create(:user) } let_it_be(:namespace, reload: true) { create(:group) } let_it_be(:another_namespace) { create(:group) } @@ -17,72 +18,139 @@ describe '#execute' do subject(:service) do - described_class.new(container: container, current_user: nil, params: { policy_project_id: policy_project.id }).execute + described_class.new(container: container, current_user: current_user, params: { policy_project_id: policy_project.id }).execute end - before do - service - end + shared_examples 'executes assign service' do + context 'when policy project is assigned' do + it 'assigns policy project to container and logs audit event' do + audit_context = { + name: "policy_project_updated", + author: current_user, + scope: container, + target: policy_project, + message: "Linked #{policy_project.name} as the security policy project" + } + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(audit_context) + + expect(service).to be_success + + expect( + container.security_orchestration_policy_configuration.security_policy_management_project_id + ).to eq(policy_project.id) + end - shared_examples 'assigns policy project' do - it 'assigns policy project to container' do - expect(service).to be_success - expect( - container.security_orchestration_policy_configuration.security_policy_management_project_id - ).to eq(policy_project.id) + it 'assigns same policy to different container' do + repeated_service = + described_class.new(container: another_container, current_user: current_user, params: { policy_project_id: policy_project.id }).execute + expect(repeated_service).to be_success + end end - it 'updates container with new policy project' do - repeated_service = - described_class.new(container: container, current_user: nil, params: { policy_project_id: new_policy_project.id }).execute + context 'when policy project is unassigned' do + before do + service + end - expect(repeated_service).to be_success - expect( - container.security_orchestration_policy_configuration.security_policy_management_project_id - ).to eq(new_policy_project.id) - end + let(:repeated_service) { described_class.new(container: container, current_user: current_user, params: { policy_project_id: nil }).execute } + + it 'unassigns project' do + expect { repeated_service }.to change { + container.reload.security_orchestration_policy_configuration + }.to(nil) + end - it 'assigns same policy to different container' do - repeated_service = - described_class.new(container: another_container, current_user: nil, params: { policy_project_id: policy_project.id }).execute - expect(repeated_service).to be_success + it 'logs audit event' do + old_policy_project = container.security_orchestration_policy_configuration.security_policy_management_project + audit_context = { + name: "policy_project_updated", + author: current_user, + scope: container, + target: old_policy_project, + message: "Unlinked #{old_policy_project.name} as the security policy project" + } + + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(audit_context) + + repeated_service + end end - it 'unassigns project' do - expect { described_class.new(container: container, current_user: nil, params: { policy_project_id: nil }).execute }.to change { - container.reload.security_orchestration_policy_configuration - }.to(nil) + context 'when policy project is reassigned' do + before do + service + end + + let(:repeated_service) { described_class.new(container: container, current_user: current_user, params: { policy_project_id: new_policy_project.id }).execute } + + it 'updates container with new policy project' do + expect(repeated_service).to be_success + expect( + container.security_orchestration_policy_configuration.security_policy_management_project_id + ).to eq(new_policy_project.id) + end + + it 'logs audit event' do + old_policy_project = container.security_orchestration_policy_configuration.security_policy_management_project + audit_context = { + name: "policy_project_updated", + author: current_user, + scope: container, + target: new_policy_project, + message: "Changed the linked security policy project from #{old_policy_project.name} to #{new_policy_project.name}" + } + + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(audit_context) + + repeated_service + end end - it 'returns error when db has problem' do - dbl_error = double('ActiveRecord') - dbl = - double( - 'Security::OrchestrationPolicyConfiguration', - security_orchestration_policy_configuration: dbl_error - ) + context 'when failure in db' do + let(:repeated_service) { described_class.new(container: container, current_user: current_user, params: { policy_project_id: new_policy_project.id }).execute } + + before do + dbl_error = double('ActiveRecord') + dbl = + double( + 'Security::OrchestrationPolicyConfiguration', + security_orchestration_policy_configuration: dbl_error + ) - allow(dbl_error).to receive(:update!).and_raise(ActiveRecord::RecordInvalid) + allow(dbl_error).to receive(:security_policy_management_project).and_return(policy_project) + allow(dbl_error).to receive(:update!).and_raise(ActiveRecord::RecordInvalid) - allow_next_instance_of(described_class) do |instance| - allow(instance).to receive(:has_existing_policy?).and_return(true) - allow(instance).to receive(:container).and_return(dbl) + allow_next_instance_of(described_class) do |instance| + allow(instance).to receive(:has_existing_policy?).and_return(true) + allow(instance).to receive(:container).and_return(dbl) + end end - repeated_service = - described_class.new(container: container, current_user: nil, params: { policy_project_id: new_policy_project.id }).execute + it 'returns error when db has problem' do + expect(repeated_service).to be_error + end + + it 'does not log audit event' do + expect(::Gitlab::Audit::Auditor).not_to receive(:audit) - expect(repeated_service).to be_error + repeated_service + end end describe 'with invalid project id' do - subject(:service) { described_class.new(container: container, current_user: nil, params: { policy_project_id: 345 }).execute } + subject(:service) { described_class.new(container: container, current_user: current_user, params: { policy_project_id: non_existing_record_id }).execute } it 'does not change policy project' do expect(service).to be_error expect { service }.not_to change { container.security_orchestration_policy_configuration } end + + it 'does not log audit event' do + expect(::Gitlab::Audit::Auditor).not_to receive(:audit) + + service + end end end @@ -90,14 +158,14 @@ let(:container) { project } let(:another_container) { another_project } - it_behaves_like 'assigns policy project' + it_behaves_like 'executes assign service' end context 'for namespace' do let(:container) { namespace } let(:another_container) { another_namespace } - it_behaves_like 'assigns policy project' + it_behaves_like 'executes assign service' end end end diff --git a/ee/spec/services/security/orchestration/unassign_service_spec.rb b/ee/spec/services/security/orchestration/unassign_service_spec.rb index 6f939863913409dd6d2cefb9705091a7101c23c5..b0bf408cb0dd6062138ace853769917a284caa5f 100644 --- a/ee/spec/services/security/orchestration/unassign_service_spec.rb +++ b/ee/spec/services/security/orchestration/unassign_service_spec.rb @@ -6,9 +6,11 @@ describe '#execute' do subject(:result) { service.execute } + let_it_be(:current_user) { create(:user) } + shared_examples 'unassigns policy project' do context 'when policy project is assigned to a project or namespace' do - let(:service) { described_class.new(container: container, current_user: nil) } + let(:service) { described_class.new(container: container, current_user: current_user) } let_it_be(:rule_schedule) do create(:security_orchestration_policy_rule_schedule, @@ -24,6 +26,21 @@ expect { result }.to change(Security::OrchestrationPolicyRuleSchedule, :count).from(1).to(0) end + it 'logs audit event' do + old_policy_project = container.security_orchestration_policy_configuration.security_policy_management_project + audit_context = { + name: "policy_project_updated", + author: current_user, + scope: container, + target: old_policy_project, + message: "Unlinked #{old_policy_project.name} as the security policy project" + } + + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(audit_context) + + result + end + context 'when destroy fails' do before do allow(container.security_orchestration_policy_configuration).to receive(:delete).and_return(false) @@ -34,11 +51,17 @@ it 'does not delete rule schedules related to the project' do expect { result }.not_to change(Security::OrchestrationPolicyRuleSchedule, :count) end + + it 'does not log audit event' do + expect(::Gitlab::Audit::Auditor).not_to receive(:audit) + + result + end end end context 'when policy project is not assigned to a project or namespace' do - let(:service) { described_class.new(container: container_without_policy_project, current_user: nil) } + let(:service) { described_class.new(container: container_without_policy_project, current_user: current_user) } it 'respond with an error', :aggregate_failures do expect(result).not_to be_success @@ -52,7 +75,7 @@ let_it_be(:container_without_policy_project, reload: true) { create(:project) } context 'with approval rules' do - let(:service) { described_class.new(container: container, current_user: nil) } + let(:service) { described_class.new(container: container, current_user: current_user) } context 'with scan_finding rule' do let_it_be(:scan_finding_rule) { create(:approval_project_rule, :scan_finding, project: container) }