diff --git a/ee/app/services/app_sec/dast/site_profiles/audit/update_service.rb b/ee/app/services/app_sec/dast/site_profiles/audit/update_service.rb
new file mode 100644
index 0000000000000000000000000000000000000000..593e3227b598ef7f4566099b266b347d13fe02e9
--- /dev/null
+++ b/ee/app/services/app_sec/dast/site_profiles/audit/update_service.rb
@@ -0,0 +1,56 @@
+# frozen_string_literal: true
+
+module AppSec
+ module Dast
+ module SiteProfiles
+ module Audit
+ class UpdateService < BaseService
+ def execute
+ new_params.each do |property, new_value|
+ old_value = old_params[property]
+
+ if new_value.is_a?(Array)
+ next if old_value.sort == new_value.sort
+ else
+ next if old_value == new_value
+ end
+
+ ::Gitlab::Audit::Auditor.audit(
+ name: 'dast_site_profile_update',
+ author: current_user,
+ scope: project,
+ target: dast_site_profile,
+ message: audit_message(property, new_value, old_value)
+ )
+ end
+ end
+
+ private
+
+ def dast_site_profile
+ params[:dast_site_profile]
+ end
+
+ def new_params
+ params[:new_params]
+ end
+
+ def old_params
+ params[:old_params]
+ end
+
+ def audit_message(property, new_value, old_value)
+ case property
+ when :auth_password, :request_headers
+ "Changed DAST site profile #{property} (secret value omitted)"
+ when :excluded_urls
+ "Changed DAST site profile #{property} (long value omitted)"
+ else
+ "Changed DAST site profile #{property} from #{old_value} to #{new_value}"
+ end
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/ee/app/services/app_sec/dast/site_profiles/create_service.rb b/ee/app/services/app_sec/dast/site_profiles/create_service.rb
index 727fba940cbf2d8b4182b1815106dcd575c626e5..78142a0493c1929e83538694f96b9622b6c722f1 100644
--- a/ee/app/services/app_sec/dast/site_profiles/create_service.rb
+++ b/ee/app/services/app_sec/dast/site_profiles/create_service.rb
@@ -28,6 +28,8 @@ def execute(name:, target_url:, **params)
create_secret_variable!(::Dast::SiteProfileSecretVariable::PASSWORD, params[:auth_password])
create_secret_variable!(::Dast::SiteProfileSecretVariable::REQUEST_HEADERS, params[:request_headers])
+ create_audit_event
+
ServiceResponse.success(payload: dast_site_profile)
end
rescue Rollback => e
@@ -68,6 +70,16 @@ def find_existing_dast_site_validation
url_base: url_base
).execute.first
end
+
+ def create_audit_event
+ ::Gitlab::Audit::Auditor.audit(
+ name: 'dast_site_profile_create',
+ author: current_user,
+ scope: project,
+ target: dast_site_profile,
+ message: 'Added DAST site profile'
+ )
+ end
end
end
end
diff --git a/ee/app/services/app_sec/dast/site_profiles/destroy_service.rb b/ee/app/services/app_sec/dast/site_profiles/destroy_service.rb
index 50a8bab97c707954174642bb6e288f10d59b1132..dfbd9a0b4db564d90fb1623b6d25f53247999786 100644
--- a/ee/app/services/app_sec/dast/site_profiles/destroy_service.rb
+++ b/ee/app/services/app_sec/dast/site_profiles/destroy_service.rb
@@ -14,6 +14,8 @@ def execute(id:)
return ServiceResponse.error(message: _('Cannot delete %{profile_name} referenced in security policy') % { profile_name: dast_site_profile.name }) if referenced_in_security_policy?(dast_site_profile)
if dast_site_profile.destroy
+ create_audit_event(dast_site_profile)
+
ServiceResponse.success(payload: dast_site_profile)
else
ServiceResponse.error(message: _('Site profile failed to delete'))
@@ -37,6 +39,16 @@ def can_delete_site_profile?
def find_dast_site_profile(id)
project.dast_site_profiles.id_in(id).first
end
+
+ def create_audit_event(profile)
+ ::Gitlab::Audit::Auditor.audit(
+ name: 'dast_site_profile_destroy',
+ author: current_user,
+ scope: project,
+ target: profile,
+ message: 'Removed DAST site profile'
+ )
+ end
end
end
end
diff --git a/ee/app/services/app_sec/dast/site_profiles/update_service.rb b/ee/app/services/app_sec/dast/site_profiles/update_service.rb
index 4d5cf8c0d7434cf9b66dee312afd24dcfa1e5246..7a1c25100269389fbc8b91046a8be46026e2d297 100644
--- a/ee/app/services/app_sec/dast/site_profiles/update_service.rb
+++ b/ee/app/services/app_sec/dast/site_profiles/update_service.rb
@@ -22,6 +22,14 @@ def execute(id:, **params)
return ServiceResponse.error(message: _('Cannot modify %{profile_name} referenced in security policy') % { profile_name: dast_site_profile.name }) if referenced_in_security_policy?
ActiveRecord::Base.transaction do
+ auditor = Audit::UpdateService.new(project, current_user, {
+ dast_site_profile: dast_site_profile,
+ new_params: params.dup,
+ old_params: dast_site_profile.attributes.symbolize_keys.merge(
+ target_url: dast_site_profile.dast_site.url
+ )
+ })
+
if target_url = params.delete(:target_url)
params[:dast_site] = DastSites::FindOrCreateService.new(project, current_user).execute!(url: target_url)
end
@@ -30,8 +38,8 @@ def execute(id:, **params)
handle_secret_variable!(params, :auth_password, ::Dast::SiteProfileSecretVariable::PASSWORD)
params.compact!
-
dast_site_profile.update!(params)
+ auditor.execute
ServiceResponse.success(payload: dast_site_profile)
end
diff --git a/ee/spec/services/app_sec/dast/site_profiles/audit/update_service_spec.rb b/ee/spec/services/app_sec/dast/site_profiles/audit/update_service_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..ee09b0051266448db37c3e7a7187dd1a4ce83294
--- /dev/null
+++ b/ee/spec/services/app_sec/dast/site_profiles/audit/update_service_spec.rb
@@ -0,0 +1,72 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe AppSec::Dast::SiteProfiles::Audit::UpdateService do
+ let_it_be(:profile) { create(:dast_site_profile) }
+ let_it_be(:project) { create(:project) }
+ let_it_be(:user) { create(:user) }
+
+ describe '#execute' do
+ it 'audits the changes in the given properties', :aggregate_failures do
+ auditor = described_class.new(project, user, {
+ dast_site_profile: profile,
+ new_params: { name: 'Updated DAST profile' },
+ old_params: { name: 'Old DAST profile' }
+ })
+
+ auditor.execute
+
+ audit_event = AuditEvent.find_by(author_id: user.id)
+ 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('DastSiteProfile')
+ expect(audit_event.details[:custom_message]).to eq(
+ 'Changed DAST site profile name from Old DAST profile to Updated DAST profile'
+ )
+ end
+
+ it 'omits the values for secret properties' do
+ auditor = described_class.new(project, user, {
+ dast_site_profile: profile,
+ new_params: { auth_password: 'newpassword', request_headers: 'A new header' },
+ old_params: { auth_password: 'oldpassword', request_headers: 'An old header' }
+ })
+
+ auditor.execute
+
+ audit_events = AuditEvent.where(author_id: user.id)
+ audit_events_messages = audit_events.map(&:details).pluck(:custom_message)
+ expect(audit_events_messages).to contain_exactly(
+ 'Changed DAST site profile auth_password (secret value omitted)',
+ 'Changed DAST site profile request_headers (secret value omitted)'
+ )
+ end
+
+ it 'omits the values for properties too long to be displayed' do
+ auditor = described_class.new(project, user, {
+ dast_site_profile: profile,
+ new_params: { excluded_urls: ['https://target.test/signout'] },
+ old_params: { excluded_urls: ['https://target.test/signin'] }
+ })
+
+ auditor.execute
+
+ audit_event = AuditEvent.find_by(author_id: user.id)
+ expect(audit_event.details[:custom_message]).to eq(
+ 'Changed DAST site profile excluded_urls (long value omitted)'
+ )
+ end
+
+ it 'sorts properties that are arrays before comparing them' do
+ auditor = described_class.new(project, user, {
+ dast_site_profile: profile,
+ new_params: { excluded_urls: ['https://target.test/signin', 'https://target.test/signout'] },
+ old_params: { excluded_urls: ['https://target.test/signout', 'https://target.test/signin'] }
+ })
+
+ expect { auditor.execute }.not_to change { AuditEvent.count }
+ end
+ end
+end
diff --git a/ee/spec/services/app_sec/dast/site_profiles/create_service_spec.rb b/ee/spec/services/app_sec/dast/site_profiles/create_service_spec.rb
index c622976cddd8be08ca0759eadf985b09a1630715..5586357d8f34984addebe0c0cfe82e62c8012f73 100644
--- a/ee/spec/services/app_sec/dast/site_profiles/create_service_spec.rb
+++ b/ee/spec/services/app_sec/dast/site_profiles/create_service_spec.rb
@@ -76,6 +76,27 @@
expect(payload).to be_a(DastSiteProfile)
end
+ it 'audits the creation' do
+ profile = payload
+
+ audit_event = AuditEvent.find_by(author_id: user.id)
+
+ 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('DastSiteProfile')
+ expect(audit_event.target_details).to eq(profile.name)
+ expect(audit_event.details).to eq({
+ author_name: user.name,
+ custom_message: 'Added DAST site profile',
+ target_id: profile.id,
+ target_type: 'DastSiteProfile',
+ target_details: profile.name
+ })
+ end
+ end
+
context 'when the dast_site already exists' do
before do
create(:dast_site, project: project, url: target_url)
diff --git a/ee/spec/services/app_sec/dast/site_profiles/destroy_service_spec.rb b/ee/spec/services/app_sec/dast/site_profiles/destroy_service_spec.rb
index 6425b57c775380d445819f504ce4024b7cc87ef8..3178fda02c50b20d5e2260423e32e90d916330ac 100644
--- a/ee/spec/services/app_sec/dast/site_profiles/destroy_service_spec.rb
+++ b/ee/spec/services/app_sec/dast/site_profiles/destroy_service_spec.rb
@@ -51,6 +51,27 @@
expect(payload).to be_a(DastSiteProfile)
end
+ it 'audits the deletion' do
+ profile = payload
+
+ audit_event = AuditEvent.find_by(author_id: user.id)
+
+ 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('DastSiteProfile')
+ expect(audit_event.target_details).to eq(profile.name)
+ expect(audit_event.details).to eq({
+ author_name: user.name,
+ custom_message: 'Removed DAST site profile',
+ target_id: profile.id,
+ target_type: 'DastSiteProfile',
+ target_details: profile.name
+ })
+ end
+ end
+
context 'when the dast_site_profile does not exist' do
let(:dast_site_profile_id) do
Gitlab::GlobalId.build(nil, model_name: 'DastSiteProfile', id: 'does_not_exist')
diff --git a/ee/spec/services/app_sec/dast/site_profiles/update_service_spec.rb b/ee/spec/services/app_sec/dast/site_profiles/update_service_spec.rb
index aaeb0ce2c6841ecf1a120713bb8f89bfef0b4333..44bba6ab55d0c54cb4e1bcdbd4d9bcaf20ca1efc 100644
--- a/ee/spec/services/app_sec/dast/site_profiles/update_service_spec.rb
+++ b/ee/spec/services/app_sec/dast/site_profiles/update_service_spec.rb
@@ -17,6 +17,7 @@
let_it_be(:new_request_headers) { "Authorization: Bearer #{SecureRandom.hex}" }
let_it_be(:new_auth_url) { "#{new_target_url}/login" }
let_it_be(:new_auth_password) { SecureRandom.hex }
+ let_it_be(:new_auth_username) { generate(:email) }
let(:default_params) do
{
@@ -29,7 +30,7 @@
auth_url: new_auth_url,
auth_username_field: 'login[username]',
auth_password_field: 'login[password]',
- auth_username: generate(:email),
+ auth_username: new_auth_username,
auth_password: new_auth_password
}
end
@@ -81,6 +82,37 @@
expect(payload).to be_a(DastSiteProfile)
end
+ it 'audits the update' do
+ profile = payload.reload
+ audit_events = AuditEvent.where(author_id: user.id)
+
+ aggregate_failures do
+ expect(audit_events.count).to be(9)
+
+ 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('DastSiteProfile')
+ expect(event.target_details).to eq(profile.name)
+ end
+
+ custom_messages = audit_events.map(&:details).pluck(:custom_message)
+ expected_custom_messages = [
+ "Changed DAST site profile name from #{dast_profile.name} to #{new_profile_name}",
+ "Changed DAST site profile target_url from #{dast_profile.dast_site.url} to #{new_target_url}",
+ 'Changed DAST site profile excluded_urls (long value omitted)',
+ "Changed DAST site profile auth_url from #{dast_profile.auth_url} to #{new_auth_url}",
+ "Changed DAST site profile auth_username_field from #{dast_profile.auth_username_field} to login[username]",
+ "Changed DAST site profile auth_password_field from #{dast_profile.auth_password_field} to login[password]",
+ "Changed DAST site profile auth_username from #{dast_profile.auth_username} to #{new_auth_username}",
+ "Changed DAST site profile auth_password (secret value omitted)",
+ "Changed DAST site profile request_headers (secret value omitted)"
+ ]
+ expect(custom_messages).to match_array(expected_custom_messages)
+ end
+ end
+
context 'when the target url is localhost' do
let(:new_target_url) { 'http://localhost:3000/hello-world' }