diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index 9d7d2a92d104bb4657cbb59347bea763145997a9..4a0c7c2d8cf3d7275e2d8101bd1db08364920d8b 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -532,6 +532,12 @@ Audit event types belong to the following product categories. |:----------|:---------------------|:------------------|:--------------|:------| | [`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 testing configuration + +| Type name | Event triggered when | Saved to database | Introduced in | Scope | +|:----------|:---------------------|:------------------|:--------------|:------| +| [`vulnerability_severity_override`](https://gitlab.com/gitlab-org/gitlab/-/issues/515327) | When user overrides vulnerability severity | {{< icon name="check-circle" >}} Yes | GitLab [17.10](https://gitlab.com/gitlab-org/gitlab/-/issues/515327) | Project | + ### Self-hosted models | Type name | Event triggered when | Saved to database | Introduced in | Scope | diff --git a/ee/app/services/vulnerabilities/bulk_severity_override_service.rb b/ee/app/services/vulnerabilities/bulk_severity_override_service.rb index 7987d7a4081f9aa0f17e36ec6ecbeef640e25dd1..f7de09d839ff6b3f701087f3c6a260c207c61d96 100644 --- a/ee/app/services/vulnerabilities/bulk_severity_override_service.rb +++ b/ee/app/services/vulnerabilities/bulk_severity_override_service.rb @@ -25,6 +25,8 @@ def update(vulnerabilities_ids) notes_ids = Note.insert_all!(db_attributes[:system_notes], returning: %w[id]) SystemNoteMetadata.insert_all!(system_note_metadata_attributes_for(notes_ids)) end + + audit_severity_changes(vulnerability_attrs) end def authorized_for_project(project) @@ -45,6 +47,23 @@ def severity_overrides_attributes_for(vulnerability_attrs) end end + def audit_severity_changes(vulnerability_attrs) + vulnerabilities_audit_attrs = vulnerability_attrs.map do |_, severity, _, _, project, vulnerability| + { + old_severity: severity, + project: project, + vulnerability: vulnerability + } + end + + SeverityOverrideAuditService.new( + vulnerabilities_audit_attrs: vulnerabilities_audit_attrs, + now: now, + current_user: user, + new_severity: @new_severity + ).execute + end + def db_attributes_for(vulnerability_attrs) { vulnerabilities: vulnerabilities_update_attributes, @@ -65,9 +84,8 @@ def update_support_tables(vulnerabilities, db_attributes) end def vulnerabilities_attributes(vulnerabilities) - vulnerabilities - .select(:id, :severity, :project_id).with_projects - .map { |v| [v.id, v.severity, v.project_id, v.project.project_namespace_id] } + attributes_relation = vulnerabilities.select(:id, :severity, :project_id).with_projects + attributes_relation.map { |v| [v.id, v.severity, v.project_id, v.project.project_namespace_id, v.project, v] } end def vulnerabilities_update_attributes diff --git a/ee/app/services/vulnerabilities/severity_override_audit_service.rb b/ee/app/services/vulnerabilities/severity_override_audit_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..7c2dc9c392563e04cb01632db7cfb5c9a909b56d --- /dev/null +++ b/ee/app/services/vulnerabilities/severity_override_audit_service.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +module Vulnerabilities + class SeverityOverrideAuditService + AUDIT_EVENT_NAME = 'vulnerability_severity_override' + + def initialize(vulnerabilities_audit_attrs:, now:, current_user:, new_severity:) + @vulnerabilities_audit_attrs = vulnerabilities_audit_attrs + @now = now + @current_user = current_user + @new_severity = new_severity + end + + def execute + return if @vulnerabilities_audit_attrs.empty? + + push_events_to_queue(build_projects_events) + end + + private + + def build_projects_events + projects_events = Hash.new { |hash, key| hash[key] = [] } + + @vulnerabilities_audit_attrs.each do |vulnerability_attrs| + project, vulnerability, old_severity = extract_attributes(vulnerability_attrs) + next unless project.present? && vulnerability.present? && old_severity.present? + + projects_events[project] << build_audit_event(project, vulnerability, old_severity) + end + + projects_events + end + + def extract_attributes(vulnerability_attrs) + vulnerability_attrs.values_at(:project, :vulnerability, :old_severity) + end + + def build_audit_event(project, vulnerability, old_severity) + AuditEvents::BuildService.new( + author: @current_user, + scope: project, + target: project, + created_at: @now, + message: build_message(old_severity), + target_details: vulnerability_url(project, vulnerability), + additional_details: { + name: AUDIT_EVENT_NAME + } + ).execute + end + + def build_message(old_severity) + "Vulnerability severity was changed from #{old_severity.capitalize} to #{@new_severity.capitalize}" + end + + def audit_context(project) + { + author: @current_user, + scope: project, + target: project, + name: AUDIT_EVENT_NAME + } + end + + def push_events_to_queue(projects_events) + total_events = 0 + + projects_events.each do |project, events| + ::Gitlab::Audit::Auditor.audit(audit_context(project)) do + events.each do |event| + ::Gitlab::Audit::EventQueue.push(event) + end + total_events += events.size + end + end + + total_events + end + + def vulnerability_url(project, vulnerability) + ::Gitlab::Routing.url_helpers.project_security_vulnerability_url(project, vulnerability) + end + end +end diff --git a/ee/config/audit_events/types/vulnerability_severity_override.yml b/ee/config/audit_events/types/vulnerability_severity_override.yml new file mode 100644 index 0000000000000000000000000000000000000000..104af6345175e475e64d5ea742877fea8ee20a1d --- /dev/null +++ b/ee/config/audit_events/types/vulnerability_severity_override.yml @@ -0,0 +1,10 @@ +--- +name: vulnerability_severity_override +description: When user overrides vulnerability severity +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/515327 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/issues/515327 +feature_category: security_testing_configuration +milestone: '17.10' +saved_to_database: true +streamed: true +scope: [Project] diff --git a/ee/lib/ee/gitlab/audit/auditor.rb b/ee/lib/ee/gitlab/audit/auditor.rb index 7663ee71acf462f18b1c77494e3442be84663a5e..d98e2d49492386f72e9fbd09e43d406ad5f0a83c 100644 --- a/ee/lib/ee/gitlab/audit/auditor.rb +++ b/ee/lib/ee/gitlab/audit/auditor.rb @@ -13,8 +13,8 @@ def multiple_audit return_value = yield ::Gitlab::Audit::EventQueue.current - .map { |message| build_event(message) } - .then { |events| record(events) } + .map { |audit| audit.is_a?(AuditEvent) ? audit : build_event(audit) } + .then { |events| record(events) } return_value ensure diff --git a/ee/spec/services/vulnerabilities/bulk_severity_override_service_spec.rb b/ee/spec/services/vulnerabilities/bulk_severity_override_service_spec.rb index d9cd7bd7ed6f14ad667b085326dfaeb1e2e736cd..f9c21d39fbc19458072979c53650bc7223825919 100644 --- a/ee/spec/services/vulnerabilities/bulk_severity_override_service_spec.rb +++ b/ee/spec/services/vulnerabilities/bulk_severity_override_service_spec.rb @@ -6,7 +6,8 @@ let_it_be(:user) { create(:user) } let_it_be(:namespace) { create(:namespace) } let_it_be(:project) { create(:project, namespace: namespace) } - let_it_be(:vulnerability) { create(:vulnerability, :with_findings, :high_severity, project: project) } + let_it_be(:original_severity) { :high } + let_it_be(:vulnerability) { create(:vulnerability, :with_findings, project: project, severity: original_severity) } let(:vulnerability_ids) { [vulnerability.id] } let(:comment) { "Severity needs to be updated." } let(:new_severity) { 'critical' } @@ -63,7 +64,7 @@ vulnerability.reload last_override = Vulnerabilities::SeverityOverride.last expect(last_override.vulnerability_id).to eq(vulnerability.id) - expect(last_override.original_severity).to eq('high') + expect(last_override.original_severity).to eq(original_severity.to_s) expect(last_override.new_severity).to eq(new_severity) expect(last_override.author).to eq(user) end @@ -77,7 +78,7 @@ expect(last_note.project).to eq(project) expect(last_note.namespace_id).to eq(project.project_namespace_id) expect(last_note.note).to eq( - "changed vulnerability severity from High to #{new_severity.titleize} " \ + "changed vulnerability severity from #{original_severity.to_s.titleize} to #{new_severity.titleize} " \ "with the following comment: \"#{comment}\"") expect(last_note).to be_system @@ -92,6 +93,22 @@ expect(result.payload[:vulnerabilities].count).to eq(vulnerability_ids.count) end + it 'creates audit events for each vulnerability', :request_store do + expect { service.execute }.to change { AuditEvent.count }.by(1) + + last_audit_event = AuditEvent.last&.details + + expect(last_audit_event[:name]).to eq('vulnerability_severity_override') + expect(last_audit_event[:author_name]).to eq(user.name) + expect(last_audit_event[:target_id]).to eq(project.id) + expect(last_audit_event[:target_details]).to eq( + ::Gitlab::Routing.url_helpers.project_security_vulnerability_url(project, vulnerability) + ) + expect(last_audit_event[:custom_message]).to eq( + "Vulnerability severity was changed from #{original_severity.to_s.titleize} to #{new_severity.capitalize}" + ) + end + context 'when an error occurs during update' do before do allow(Vulnerabilities::SeverityOverride).to receive(:insert_all!).and_raise(ActiveRecord::RecordNotUnique) diff --git a/ee/spec/services/vulnerabilities/severity_override_audit_service_spec.rb b/ee/spec/services/vulnerabilities/severity_override_audit_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..1a2972d2809338a9b381ba17adc4a183e75b1664 --- /dev/null +++ b/ee/spec/services/vulnerabilities/severity_override_audit_service_spec.rb @@ -0,0 +1,199 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Vulnerabilities::SeverityOverrideAuditService, feature_category: :vulnerability_management do + let_it_be(:project) { create(:project) } + let_it_be(:current_user) { create(:user) } + let_it_be(:vulnerability) { create(:vulnerability, project: project) } + let_it_be(:now) { Time.current } + let(:new_severity) { 'high' } + let(:service) do + described_class.new( + vulnerabilities_audit_attrs: vulnerabilities_audit_attrs, + now: now, + current_user: current_user, + new_severity: new_severity + ) + end + + subject(:execute) { service.execute } + + describe '#execute' do + context 'when vulnerabilities_audit_attrs is empty' do + let(:vulnerabilities_audit_attrs) { [] } + + it 'returns nil without creating any events' do + expect(::Gitlab::Audit::EventQueue).not_to receive(:push) + expect(::Gitlab::Audit::Auditor).not_to receive(:audit) + expect { execute }.not_to change { AuditEvent.count } + expect(execute).to be_nil + end + end + + context 'when vulnerabilities_audit_attrs contains valid data', :request_store do + let(:vulnerabilities_audit_attrs) do + [ + { + project: project, + old_severity: vulnerability.severity, + vulnerability: vulnerability + } + ] + end + + before do + allow(::Gitlab::Routing.url_helpers).to receive(:project_security_vulnerability_url) + .with(project, vulnerability).and_return(expected_url(project, vulnerability)) + end + + it 'creates and pushes audit events through the Auditor' do + expect(::Gitlab::Audit::Auditor).to receive(:audit).with({ + author: current_user, + scope: project, + target: project, + name: 'vulnerability_severity_override' + }).and_call_original + + expect(::Gitlab::Audit::EventQueue).to receive(:push).with( + satisfy { |event| severity_override_event?(event) } + ).and_call_original + expect(execute).to eq(1) + + verify_audit_event_details(AuditEvent.last&.details, project, vulnerability) + end + + context 'with multiple events for the same project' do + before do + allow(::Gitlab::Routing.url_helpers).to receive(:project_security_vulnerability_url) + .with(project, another_vulnerability).and_return(expected_url(project, another_vulnerability)) + end + + let(:another_vulnerability) { create(:vulnerability, project: project, severity: :medium) } + let(:vulnerabilities_audit_attrs) do + [ + { + project: project, + old_severity: vulnerability.severity, + vulnerability: vulnerability + }, + { + project: project, + old_severity: another_vulnerability.severity, + vulnerability: another_vulnerability + } + ] + end + + it 'processes all events within a single audit context with the correct values' do + expect(::Gitlab::Audit::Auditor).to receive(:audit).with({ + author: current_user, + scope: project, + target: project, + name: 'vulnerability_severity_override' + }).and_call_original + + expect(::Gitlab::Audit::EventQueue).to receive(:push).twice.with( + satisfy { |event| severity_override_event?(event) } + ).and_call_original + + expect { execute }.to change { AuditEvent.count }.by(2) + expect(execute).to eq(2) + + audit_events = AuditEvent.last(2) + verify_audit_event_details(audit_events.first.details, project, vulnerability) + verify_audit_event_details(audit_events.second.details, project, another_vulnerability) + end + end + + context 'with events for different projects' do + let_it_be(:another_project) { build(:project) } + let(:another_vulnerability) { create(:vulnerability, project: another_project, severity: :medium) } + let(:vulnerabilities_audit_attrs) do + [ + { + project: project, + old_severity: vulnerability.severity, + vulnerability: vulnerability + }, + { + project: another_project, + old_severity: another_vulnerability.severity, + vulnerability: another_vulnerability + } + ] + end + + before do + allow(::Gitlab::Routing.url_helpers).to receive(:project_security_vulnerability_url) + .with(another_project, another_vulnerability) + .and_return(expected_url(another_project, another_vulnerability)) + end + + it 'creates separate Auditors for each project with the correct values' do + expect(::Gitlab::Audit::Auditor).to receive(:audit).with({ + author: current_user, + scope: project, + target: project, + name: 'vulnerability_severity_override' + }).and_call_original + + expect(::Gitlab::Audit::Auditor).to receive(:audit).with({ + author: current_user, + scope: another_project, + target: another_project, + name: 'vulnerability_severity_override' + }).and_call_original + + expect(::Gitlab::Audit::EventQueue).to receive(:push).twice.with( + satisfy { |event| severity_override_event?(event) } + ).and_call_original + + expect { execute }.to change { AuditEvent.count }.by(2) + expect(execute).to eq(2) + + audit_events = AuditEvent.last(2) + verify_audit_event_details(audit_events.first.details, project, vulnerability) + verify_audit_event_details(audit_events.second.details, another_project, another_vulnerability) + end + end + end + end + + describe '#audit_context' do + let(:vulnerabilities_audit_attrs) { [] } + + it 'returns the correct audit context hash' do + context = service.send(:audit_context, project) + + expect(context).to eq({ + author: current_user, + scope: project, + target: project, + name: 'vulnerability_severity_override' + }) + end + end + + def expected_url(project, vulnerability) + "http://localhost/#{project.full_path}/-/security/vulnerabilities/#{vulnerability.id}" + end + + def expected_message(vulnerability) + "Vulnerability severity was changed from #{vulnerability.severity.capitalize} to #{new_severity.capitalize}" + end + + def severity_override_event?(event) + event.is_a?(AuditEvent) && event.details[:name] == 'vulnerability_severity_override' + end + + def verify_audit_event_details(audit_event, expected_project, expected_vulnerability) + aggregate_failures "audit event details" do + expect(audit_event[:name]).to eq('vulnerability_severity_override') + expect(audit_event[:author_name]).to eq(current_user.name) + expect(audit_event[:target_id]).to eq(expected_project.id) + expect(audit_event[:target_details]).to eq(expected_url(expected_project, expected_vulnerability)) + expect(audit_event[:custom_message]).to eq(expected_message(expected_vulnerability)) + end + end +end