diff --git a/ee/app/services/security/findings/severity_override_service.rb b/ee/app/services/security/findings/severity_override_service.rb index 6616253e51e9af1e946364a971dff0d2e1fa8dd3..a96f0706138cc3f4f119a8d24ac6d92d4d67c121 100644 --- a/ee/app/services/security/findings/severity_override_service.rb +++ b/ee/app/services/security/findings/severity_override_service.rb @@ -27,8 +27,15 @@ def execute return lookup_error(vulnerability_result) unless vulnerability_result[:status] == :success vulnerability = vulnerability_result.payload[:vulnerability] - update_severity(vulnerability) + @original_severity = vulnerability.severity + + if @original_severity != @severity + update_severity(vulnerability) + audit + end + service_success + rescue ArgumentError, ActiveRecord::RecordInvalid => error service_error(format_error(error.message), :unprocessable_entity) end @@ -41,8 +48,6 @@ def authorized? end def update_severity(vulnerability) - return if vulnerability.severity == @severity - vulnerability.transaction do create_severity_override_record(vulnerability) vulnerability.update!(severity: @severity) @@ -61,6 +66,23 @@ def create_severity_override_record(vulnerability) }) end + def audit + message = "Vulnerability finding severity was changed from #{@original_severity.capitalize} " \ + "to #{@severity.capitalize}" + details = "Vulnerability finding uuid: #{@security_finding.uuid}" + + audit_context = { + name: 'vulnerability_severity_override', + author: @current_user, + scope: project, + target: project, + message: message, + target_details: details + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end + def service_success # Reset cached associations to later use the updated vulnerability severity @security_finding.reset diff --git a/ee/spec/services/security/findings/severity_override_service_spec.rb b/ee/spec/services/security/findings/severity_override_service_spec.rb index 8520acf38fd78953d988337d99754baf519c3389..4f7bb4aaa0e11286fc7ab8ea6a0cb416e2b8ca2f 100644 --- a/ee/spec/services/security/findings/severity_override_service_spec.rb +++ b/ee/spec/services/security/findings/severity_override_service_spec.rb @@ -38,6 +38,24 @@ def override_severity(severity: new_severity) let_it_be(:current_user) { create(:user) } let_it_be(:new_severity) { 'high' } + shared_examples 'creates project audit event' do + it 'creates project audit event' do + original_severity = security_finding.severity + expected_details = "Vulnerability finding uuid: #{security_finding.uuid}" + expected_message = "Vulnerability finding severity was changed from #{original_severity.capitalize} " \ + "to #{new_severity.capitalize}" + + expect { execute }.to change { AuditEvent.count }.by(1) + + last_audit_event = AuditEvent.last&.details + expect(last_audit_event[:event_name]).to eq('vulnerability_severity_override') + expect(last_audit_event[:author_name]).to eq(current_user.name) + expect(last_audit_event[:target_id]).to eq(project.id) + expect(last_audit_event[:target_details]).to eq(expected_details) + expect(last_audit_event[:custom_message]).to eq(expected_message) + end + end + context 'when the user is authorized' do before do security_finding.project.add_maintainer(current_user) @@ -51,6 +69,10 @@ def override_severity(severity: new_severity) .and change { Vulnerabilities::Finding.count }.by(1) .and not_change { Vulnerabilities::SeverityOverride.count } end + + it 'doesnt create audit event' do + expect { execute }.not_to change { AuditEvent.count } + end end context 'when severity is overridden' do @@ -73,6 +95,8 @@ def override_severity(severity: new_severity) vulnerability: security_finding.vulnerability ) end + + it_behaves_like 'creates project audit event' end context 'when a vulnerability matching the security finding already exists' do @@ -117,6 +141,8 @@ def override_severity(severity: new_severity) ) end end + + it_behaves_like 'creates project audit event' end end