From 7a76a2c665caddc38cbdc2011fe9d1b3e72f871a Mon Sep 17 00:00:00 2001
From: Smriti Garg <sgarg@gitlab.com>
Date: Mon, 7 Oct 2024 17:54:37 +0000
Subject: [PATCH] Changes for allowed_email_domain update audit event
Group setting allowed_email_domains update is an admin action
and must be audited
Changelog: added
---
doc/user/compliance/audit_event_types.md | 1 +
.../allowed_email_domains/update_service.rb | 17 +++++++++
ee/app/services/ee/groups/update_service.rb | 6 ++-
.../types/allowed_email_domain_updated.yml | 10 +++++
.../services/groups/update_service_spec.rb | 37 ++++++++++++++++---
5 files changed, 64 insertions(+), 7 deletions(-)
create mode 100644 ee/config/audit_events/types/allowed_email_domain_updated.yml
diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md
index 75f1634d66db1..43fab03244851 100644
--- a/doc/user/compliance/audit_event_types.md
+++ b/doc/user/compliance/audit_event_types.md
@@ -298,6 +298,7 @@ Audit event types belong to the following product categories.
|:------------|:------------|:------------------|:---------|:--------------|:--------------|
| [`allow_mfa_for_subgroups_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/164973) | Triggered when setting for Subgroups can set up their own two-factor authentication rules updated | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.4](https://gitlab.com/gitlab-org/gitlab/-/issues/486532) | Group |
| [`allow_runner_registration_token_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/164973) | Triggered when setting Allow members of projects and groups to create runners with runner registration tokens is updated | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [16.0](https://gitlab.com/gitlab-org/gitlab/-/issues/486532) | Group, Project |
+| [`allowed_email_domain_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/166105) | Triggered when group setting allowed email domain entry is updated | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.5](https://gitlab.com/gitlab-org/gitlab/-/issues/486532) | Group |
| [`create_ssh_certificate`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/134556) | Triggered when an SSH certificate is created | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [16.6](https://gitlab.com/gitlab-org/gitlab/-/issues/427413) | Group |
| [`default_branch_name_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/164973) | Triggered when default branch name for the group repository is changed | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.4](https://gitlab.com/gitlab-org/gitlab/-/issues/486532) | Group |
| [`delete_ssh_certificate`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/134556) | Triggered when an SSH certificate is deleted | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [16.6](https://gitlab.com/gitlab-org/gitlab/-/issues/427413) | Group |
diff --git a/ee/app/services/ee/allowed_email_domains/update_service.rb b/ee/app/services/ee/allowed_email_domains/update_service.rb
index 43251dadf76be..c0bff0d37f688 100644
--- a/ee/app/services/ee/allowed_email_domains/update_service.rb
+++ b/ee/app/services/ee/allowed_email_domains/update_service.rb
@@ -30,6 +30,23 @@ def execute
build_new_allowed_emails_domains_records
end
+ def log_audit_event(updated_domain_list)
+ return unless existing_domains && updated_domain_list
+
+ return if updated_domain_list.sort == existing_domains.sort
+
+ message = "Allowed email domain names updated from '#{existing_domains.join(',')}' to '#{updated_domain_list.join(',')}'"
+
+ ::Gitlab::Audit::Auditor.audit({
+ name: "allowed_email_domain_updated",
+ author: current_user,
+ scope: group,
+ target: group,
+ target_details: group.path,
+ message: _(message)
+ })
+ end
+
private
attr_reader :current_user, :group, :current_domains
diff --git a/ee/app/services/ee/groups/update_service.rb b/ee/app/services/ee/groups/update_service.rb
index 5002fa61290b3..c6cfdf0371478 100644
--- a/ee/app/services/ee/groups/update_service.rb
+++ b/ee/app/services/ee/groups/update_service.rb
@@ -133,7 +133,10 @@ def handle_allowed_email_domains_update
comma_separated_domains = params.delete(:allowed_email_domains_list)
- AllowedEmailDomains::UpdateService.new(current_user, group, comma_separated_domains).execute
+ # rubocop:disable Gitlab/ModuleWithInstanceVariables -- Reason: We need this instance to log audit event post save
+ @allowed_email_domains_update_service = AllowedEmailDomains::UpdateService.new(current_user, group, comma_separated_domains)
+ @allowed_email_domains_update_service.execute
+ # rubocop:enable Gitlab/ModuleWithInstanceVariables
end
override :allowed_settings_params
@@ -143,6 +146,7 @@ def allowed_settings_params
def log_audit_events
@ip_restriction_update_service&.log_audit_event # rubocop:disable Gitlab/ModuleWithInstanceVariables
+ @allowed_email_domains_update_service&.log_audit_event(group.allowed_email_domains.map(&:domain)) # rubocop:disable Gitlab/ModuleWithInstanceVariables
Audit::GroupChangesAuditor.new(current_user, group).execute
end
diff --git a/ee/config/audit_events/types/allowed_email_domain_updated.yml b/ee/config/audit_events/types/allowed_email_domain_updated.yml
new file mode 100644
index 0000000000000..e71cdd4c7e58e
--- /dev/null
+++ b/ee/config/audit_events/types/allowed_email_domain_updated.yml
@@ -0,0 +1,10 @@
+---
+name: allowed_email_domain_updated
+description: Triggered when group setting allowed email domain entry is updated
+introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/486532
+introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/166105
+feature_category: groups_and_projects
+milestone: '17.5'
+saved_to_database: true
+streamed: true
+scope: [Group]
diff --git a/ee/spec/services/groups/update_service_spec.rb b/ee/spec/services/groups/update_service_spec.rb
index 06720a3452164..d5e826b5744cd 100644
--- a/ee/spec/services/groups/update_service_spec.rb
+++ b/ee/spec/services/groups/update_service_spec.rb
@@ -32,9 +32,7 @@
allow(group).to receive(:save).and_return(false)
end
- def operation
- update_group(group, user, visibility_level: Gitlab::VisibilityLevel::PRIVATE)
- end
+ let(:operation_params) { { visibility_level: Gitlab::VisibilityLevel::PRIVATE } }
let(:attributes) do
audit_event_params.tap do |param|
@@ -61,9 +59,7 @@ def operation
allow(group).to receive(:save).and_return(false)
end
- def operation
- update_group(group, user, ip_restriction_ranges: '192.168.0.0/24,10.0.0.0/8')
- end
+ let(:operation_params) { { ip_restriction_ranges: '192.168.0.0/24,10.0.0.0/8' } }
let(:attributes) do
audit_event_params.tap do |param|
@@ -76,6 +72,35 @@ def operation
end
end
end
+
+ describe 'allowed email domain' do
+ context 'when allowed email domains were changed' do
+ before do
+ group.add_owner(user)
+ end
+
+ include_examples 'audit event logging' do
+ let(:fail_condition!) do
+ allow(group).to receive(:save).and_return(false)
+ end
+
+ let(:operation_params) { { allowed_email_domains_list: 'abcd.com,test.com' } }
+
+ let(:attributes) do
+ audit_event_params.tap do |param|
+ param[:details].merge!(
+ event_name: 'allowed_email_domain_updated',
+ custom_message: "Allowed email domain names updated from '' to 'abcd.com,test.com'"
+ )
+ end
+ end
+ end
+ end
+ end
+
+ def operation(update_params = operation_params)
+ update_group(group, user, **update_params)
+ end
end
context 'sub group' do
--
GitLab