Merge branch '217872-audit-dast-scanner-profiles' into 'master'

Audit DAST scanner profile updates

See merge request gitlab-org/gitlab!60287

ee/app/services/app_sec/dast/scanner_profiles/create_service.rb

@@ -16,16 +16,30 @@ module AppSec
             use_ajax_spider: use_ajax_spider,
             show_debug_messages: show_debug_messages
           )
-          return ServiceResponse.success(payload: dast_scanner_profile) if dast_scanner_profile.valid?
 
+          if dast_scanner_profile.valid?
+            create_audit_event(dast_scanner_profile)
+
+            ServiceResponse.success(payload: dast_scanner_profile)
+          else
             ServiceResponse.error(message: dast_scanner_profile.errors.full_messages)
           end
+        end
 
         private
 
         def allowed?
           Ability.allowed?(current_user, :create_on_demand_dast_scan, project)
         end
+
+        def create_audit_event(profile)
+          AuditEventService.new(current_user, project, {
+            add: 'DAST scanner profile',
+            target_id: profile.id,
+            target_type: profile.class.name,
+            target_details: profile.name
+          }).security_event
+        end
       end
     end
   end

ee/app/services/app_sec/dast/scanner_profiles/destroy_service.rb

@@ -14,6 +14,8 @@ module AppSec
           return ServiceResponse.error(message: _('Cannot delete %{profile_name} referenced in security policy') % { profile_name: dast_scanner_profile.name }) if referenced_in_security_policy?(dast_scanner_profile)
 
           if dast_scanner_profile.destroy
+            create_audit_event(dast_scanner_profile)
+
             ServiceResponse.success(payload: dast_scanner_profile)
           else
             ServiceResponse.error(message: _('Scanner profile failed to delete'))
@@ -37,6 +39,15 @@ module AppSec
         def find_dast_scanner_profile(id)
           project.dast_scanner_profiles.id_in(id).first
         end
+
+        def create_audit_event(profile)
+          AuditEventService.new(current_user, project, {
+            remove: 'DAST scanner profile',
+            target_id: profile.id,
+            target_type: profile.class.name,
+            target_details: profile.name
+          }).security_event
+        end
       end
     end
   end

ee/app/services/app_sec/dast/scanner_profiles/update_service.rb

@@ -13,16 +13,19 @@ module AppSec
           return ServiceResponse.error(message: _('Scanner profile not found for given parameters')) unless dast_scanner_profile
           return ServiceResponse.error(message: _('Cannot modify %{profile_name} referenced in security policy') % { profile_name: dast_scanner_profile.name }) if referenced_in_security_policy?(dast_scanner_profile)
 
-          update_args = {
+          old_params = dast_scanner_profile.attributes.symbolize_keys
+          params = {
             name: profile_name,
             target_timeout: target_timeout,
-            spider_timeout: spider_timeout
-          }
-          update_args[:scan_type] = scan_type if scan_type
-          update_args[:use_ajax_spider] = use_ajax_spider unless use_ajax_spider.nil?
-          update_args[:show_debug_messages] = show_debug_messages unless show_debug_messages.nil?
+            spider_timeout: spider_timeout,
+            scan_type: scan_type,
+            use_ajax_spider: use_ajax_spider,
+            show_debug_messages: show_debug_messages
+          }.compact
+
+          if dast_scanner_profile.update(params)
+            create_audit_events(dast_scanner_profile, params, old_params)
 
-          if dast_scanner_profile.update(update_args)
             ServiceResponse.success(payload: dast_scanner_profile)
           else
             ServiceResponse.error(message: dast_scanner_profile.errors.full_messages)
@@ -46,6 +49,23 @@ module AppSec
         def find_dast_scanner_profile(id)
           DastScannerProfilesFinder.new(project_ids: [project.id], ids: [id]).execute.first
         end
+
+        def create_audit_events(profile, params, old_params)
+          params.each do |property, new_value|
+            old_value = old_params[property]
+
+            next if old_value == new_value
+
+            AuditEventService.new(current_user, project, {
+              change: "DAST scanner profile #{property}",
+              from: old_value,
+              to: new_value,
+              target_id: profile.id,
+              target_type: profile.class.name,
+              target_details: profile.name
+            }).security_event
+          end
+        end
       end
     end
   end

ee/changelogs/unreleased/217872-audit-dast-scanner-profiles.yml

+---
+title: Audit DAST scanner profile updates
+merge_request: 60287
+author:
+type: added

ee/spec/services/app_sec/dast/scanner_profiles/create_service_spec.rb

@@ -88,6 +88,26 @@ RSpec.describe AppSec::Dast::ScannerProfiles::CreateService do
         expect(payload).to be_a(DastScannerProfile)
       end
 
+      it 'audits the creation' do
+        profile = payload
+
+        audit_event = AuditEvent.last
+
+        aggregate_failures do
+          expect(audit_event.author).to eq(user)
+          expect(audit_event.entity).to eq(project)
+          expect(audit_event.target_id).to eq(profile.id)
+          expect(audit_event.target_type).to eq('DastScannerProfile')
+          expect(audit_event.target_details).to eq(profile.name)
+          expect(audit_event.details).to eq({
+            add: 'DAST scanner profile',
+            target_id: profile.id,
+            target_type: 'DastScannerProfile',
+            target_details: profile.name
+          })
+        end
+      end
+
       context 'when the dast_scanner_profile name exists' do
         before do
           create(:dast_scanner_profile, project: project, name: name)

ee/spec/services/app_sec/dast/scanner_profiles/destroy_service_spec.rb

@@ -51,6 +51,26 @@ RSpec.describe AppSec::Dast::ScannerProfiles::DestroyService do
         expect(payload).to be_a(DastScannerProfile)
       end
 
+      it 'audits the deletion' do
+        profile = payload
+
+        audit_event = AuditEvent.last
+
+        aggregate_failures do
+          expect(audit_event.author).to eq(user)
+          expect(audit_event.entity).to eq(project)
+          expect(audit_event.target_id).to eq(profile.id)
+          expect(audit_event.target_type).to eq('DastScannerProfile')
+          expect(audit_event.target_details).to eq(profile.name)
+          expect(audit_event.details).to eq({
+            remove: 'DAST scanner profile',
+            target_id: profile.id,
+            target_type: 'DastScannerProfile',
+            target_details: profile.name
+          })
+        end
+      end
+
       context 'when the dast_scanner_profile doesn\'t exist' do
         let(:dast_scanner_profile_id) do
           Gitlab::GlobalId.build(nil, model_name: 'DastScannerProfile', id: 'does_not_exist')

ee/spec/services/app_sec/dast/scanner_profiles/update_service_spec.rb

@@ -73,15 +73,40 @@ RSpec.describe AppSec::Dast::ScannerProfiles::UpdateService do
     end
 
     context 'when the user can run a dast scan' do
-      before do
-        project.add_developer(user)
+      let(:base_audit_details) do
+        [
+          {
+            change: "DAST scanner profile name",
+            from: dast_profile.name,
+            to: new_profile_name,
+            target_id: dast_profile.id,
+            target_type: 'DastScannerProfile',
+            target_details: new_profile_name
+          },
+          {
+            change: "DAST scanner profile target_timeout",
+            from: dast_profile.target_timeout,
+            to: new_target_timeout,
+            target_id: dast_profile.id,
+            target_type: 'DastScannerProfile',
+            target_details: new_profile_name
+          },
+          {
+            change: "DAST scanner profile spider_timeout",
+            from: dast_profile.spider_timeout,
+            to: new_spider_timeout,
+            target_id: dast_profile.id,
+            target_type: 'DastScannerProfile',
+            target_details: new_profile_name
+          }
+        ]
       end
 
-      context 'when the user omits unrequired elements' do
       before do
         project.add_developer(user)
       end
 
+      context 'when the user omits unrequired elements' do
         subject do
           described_class.new(project, user).execute(
             id: dast_scanner_profile_id,
@@ -100,6 +125,15 @@ RSpec.describe AppSec::Dast::ScannerProfiles::UpdateService do
             expect(updated_dast_scanner_profile.show_debug_messages).to eq(dast_profile.show_debug_messages)
           end
         end
+
+        it 'omits those elements from the audit' do
+          subject
+
+          audit_events = AuditEvent.all
+          audit_events_details = audit_events.map(&:details)
+
+          expect(audit_events_details).to match_array(base_audit_details)
+        end
       end
 
       it 'returns a success status' do
@@ -119,6 +153,49 @@ RSpec.describe AppSec::Dast::ScannerProfiles::UpdateService do
         end
       end
 
+      it 'audits the update' do
+        profile = payload.reload
+        audit_events = AuditEvent.all
+        audit_events_details = audit_events.map(&:details)
+
+        aggregate_failures do
+          audit_events.each do |event|
+            expect(event.author).to eq(user)
+            expect(event.entity).to eq(project)
+            expect(event.target_id).to eq(profile.id)
+            expect(event.target_type).to eq('DastScannerProfile')
+            expect(event.target_details).to eq(profile.name)
+          end
+
+          expect(audit_events_details).to match_array(base_audit_details + [
+            {
+              change: "DAST scanner profile scan_type",
+              from: dast_profile.scan_type,
+              to: new_scan_type,
+              target_id: profile.id,
+              target_type: 'DastScannerProfile',
+              target_details: new_profile_name
+            },
+            {
+              change: "DAST scanner profile use_ajax_spider",
+              from: dast_profile.use_ajax_spider,
+              to: new_use_ajax_spider,
+              target_id: profile.id,
+              target_type: 'DastScannerProfile',
+              target_details: new_profile_name
+            },
+            {
+              change: "DAST scanner profile show_debug_messages",
+              from: dast_profile.show_debug_messages,
+              to: new_show_debug_messages,
+              target_id: profile.id,
+              target_type: 'DastScannerProfile',
+              target_details: new_profile_name
+            }
+          ])
+        end
+      end
+
       context 'when setting properties to false' do
         let_it_be(:dast_scanner_profile, reload: true) { create(:dast_scanner_profile, target_timeout: 200, spider_timeout: 5000, use_ajax_spider: true, show_debug_messages: true) }