diff --git a/doc/administration/audit_events.md b/doc/administration/audit_events.md index 8fbf06343a59f8713f4447837f5fef36f6dcc99f..ddc8e8cb270fb2103055dfddbaecc242cab5861c 100644 --- a/doc/administration/audit_events.md +++ b/doc/administration/audit_events.md @@ -73,6 +73,17 @@ From there, you can see the following actions: - Group changed visibility. - User was added to group and with which [permissions](../user/permissions.md). - User sign-in via [Group SAML](../user/group/saml_sso/index.md). +- [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/8071) in GitLab 14.5, changes to the following + [group SAML](../user/group/saml_sso/index.md) configuration: + - Enabled status. + - Enforcing SSO-only authentication for web activity. + - Enforcing SSO-only authentication for Git and Dependency Proxy activity. + - Enforcing users to have dedicated group-managed accounts. + - Prohibiting outer forks. + - Identity provider SSO URL. + - Certificate fingerprint. + - Default membership role. + - SSO-SAML group sync configuration. - Permissions changes of a user assigned to a group. - Removed user from group. - Project repository imported into group. diff --git a/ee/app/controllers/groups/saml_group_links_controller.rb b/ee/app/controllers/groups/saml_group_links_controller.rb index 3e1bffcd24be89eeab29c5976f88fd2f93ddb926..5fcd98f1ac3af09b74c5ee8ff519e4134b84046c 100644 --- a/ee/app/controllers/groups/saml_group_links_controller.rb +++ b/ee/app/controllers/groups/saml_group_links_controller.rb @@ -13,6 +13,7 @@ def create if group_link.save flash[:notice] = s_('GroupSAML|New SAML group link saved.') + create_audit_event('saml_group_links_created', group, "SAML group links created. Group Name - #{group_link.saml_group_name}, Access Level - #{group_link.access_level}") else flash[:alert] = alert(group_link.errors.full_messages.join(', ')) end @@ -21,7 +22,9 @@ def create end def destroy - group.saml_group_links.find(params[:id]).destroy + saml_group_link = group.saml_group_links.find(params[:id]) + saml_group_link.destroy + create_audit_event('saml_group_links_removed', group, "SAML group links removed. Group Name - #{saml_group_link.saml_group_name}") redirect_to group_saml_group_links_path(@group), status: :found, notice: s_('GroupSAML|SAML group link was successfully removed.') end @@ -39,5 +42,15 @@ def saml_group_link_params def alert(error_message) s_('GroupSAML|Could not create SAML group link: %{errors}.') % { errors: error_message } end + + def create_audit_event(name, group, message) + ::Gitlab::Audit::Auditor.audit( + name: name, + author: current_user, + scope: group, + target: group, + message: message + ) + end end end diff --git a/ee/app/services/group_saml/saml_provider/base_service.rb b/ee/app/services/group_saml/saml_provider/base_service.rb index a07da222dcf44cd0ed51880d7bd36260f9149f82..03644aded97bf83352ea586759665f438cae5afb 100644 --- a/ee/app/services/group_saml/saml_provider/base_service.rb +++ b/ee/app/services/group_saml/saml_provider/base_service.rb @@ -9,6 +9,10 @@ class BaseService delegate :group, to: :saml_provider + AUDIT_LOG_ALLOWLIST = %w[ + enabled certificate_fingerprint sso_url enforced_sso enforced_group_managed_accounts prohibited_outer_forks default_membership_role git_check_enforced + ].freeze + def initialize(current_user, saml_provider, params:) @saml_provider = saml_provider @current_user = current_user @@ -25,6 +29,24 @@ def execute require_linked_saml_to_enable_group_managed! end end + + saml_provider.previous_changes.each do |attribute, changes| + next unless AUDIT_LOG_ALLOWLIST.include?(attribute) + + audit_context = { + name: audit_name, + author: current_user, + scope: saml_provider.group, + target: saml_provider.group, + message: message(attribute, changes) + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end + end + + def audit_name + "group_saml_provider" end private @@ -40,6 +62,16 @@ def add_error!(message) raise ActiveRecord::Rollback end + + def message(attribute, changes) + change_text = if changes[0].nil? + "#{attribute} changed to #{changes[1]}. " + else + "#{attribute} changed from #{changes[0]} to #{changes[1]}. " + end + + "Group SAML SSO configuration changed: #{change_text}" + end end end end diff --git a/ee/app/services/group_saml/saml_provider/create_service.rb b/ee/app/services/group_saml/saml_provider/create_service.rb index c4ee7f66abbe8cede3e327dce48d8c45f6b5fab9..70df48a1f38d63ef9fed6da918aa4e7268bc902d 100644 --- a/ee/app/services/group_saml/saml_provider/create_service.rb +++ b/ee/app/services/group_saml/saml_provider/create_service.rb @@ -9,6 +9,10 @@ def initialize(current_user, group, params:) @group = group super(current_user, group.build_saml_provider, params: params) end + + def audit_name + "#{super}_create" + end end end end diff --git a/ee/app/services/group_saml/saml_provider/update_service.rb b/ee/app/services/group_saml/saml_provider/update_service.rb index 3fe6e2e98fec093b2125ae2c2fa7825b4436a455..87576650a03606592252f499e0b246287c0f9b09 100644 --- a/ee/app/services/group_saml/saml_provider/update_service.rb +++ b/ee/app/services/group_saml/saml_provider/update_service.rb @@ -5,6 +5,9 @@ module GroupSaml module SamlProvider class UpdateService < BaseService + def audit_name + "#{super}_update" + end end end end diff --git a/ee/spec/controllers/groups/saml_group_links_controller_spec.rb b/ee/spec/controllers/groups/saml_group_links_controller_spec.rb index d82a352fa955f2469dba176a6063b3479a963816..c8ec1d6a6abdfeae044d734f62d2c4de24407eea 100644 --- a/ee/spec/controllers/groups/saml_group_links_controller_spec.rb +++ b/ee/spec/controllers/groups/saml_group_links_controller_spec.rb @@ -60,13 +60,25 @@ let_it_be(:saml_provider) { create(:saml_provider, group: group, enabled: true) } context 'with valid parameters' do - let_it_be(:params) { route_params.merge(saml_group_link: { access_level: 'Reporter', saml_group_name: generate(:saml_group_name) }) } + let_it_be(:saml_group_name) { generate(:saml_group_name) } + let_it_be(:params) { route_params.merge(saml_group_link: { access_level: 'Reporter', saml_group_name: saml_group_name }) } it 'responds with success' do + expect(::Gitlab::Audit::Auditor) + .to receive(:audit).with( + hash_including( + { name: "saml_group_links_created", + author: user, + scope: group, + target: group, + message: "SAML group links created. Group Name - #{saml_group_name}, Access Level - Reporter" }) + ).and_call_original + call_action expect(response).to have_gitlab_http_status(:found) expect(flash[:notice]).to include('New SAML group link saved.') + expect(AuditEvent.last.details[:custom_message]).to eq("SAML group links created. Group Name - #{saml_group_name}, Access Level - Reporter") end it 'creates the group link' do @@ -102,10 +114,21 @@ let_it_be(:params) { route_params } it 'responds with success' do + expect(::Gitlab::Audit::Auditor) + .to receive(:audit).with( + hash_including( + { name: "saml_group_links_removed", + author: user, + scope: group, + target: group, + message: "SAML group links removed. Group Name - #{group_link.saml_group_name}" }) + ).and_call_original + call_action expect(response).to have_gitlab_http_status(:found) expect(flash[:notice]).to include('SAML group link was successfully removed.') + expect(AuditEvent.last.details[:custom_message]).to eq("SAML group links removed. Group Name - #{group_link.saml_group_name}") end it 'removes the group link' do diff --git a/ee/spec/services/group_saml/saml_provider/create_service_spec.rb b/ee/spec/services/group_saml/saml_provider/create_service_spec.rb index 0180c20bc141478c0deb2cfcd04cc8564be3d611..ec7623b772808e62a1b626e364e41ada21a3e548 100644 --- a/ee/spec/services/group_saml/saml_provider/create_service_spec.rb +++ b/ee/spec/services/group_saml/saml_provider/create_service_spec.rb @@ -8,5 +8,7 @@ let(:group) { create :group } + let(:audit_event_name) { 'group_saml_provider_create' } + include_examples 'base SamlProvider service' end diff --git a/ee/spec/services/group_saml/saml_provider/update_service_spec.rb b/ee/spec/services/group_saml/saml_provider/update_service_spec.rb index 1a0c524dac86b343a53c6d4a8cfe51274ae8582b..aaf6f1d33657892218c2cdee3a31512e3bb8d12e 100644 --- a/ee/spec/services/group_saml/saml_provider/update_service_spec.rb +++ b/ee/spec/services/group_saml/saml_provider/update_service_spec.rb @@ -11,6 +11,7 @@ end let(:group) { saml_provider.group } + let(:audit_event_name) { 'group_saml_provider_update' } include_examples 'base SamlProvider service' include_examples 'SamlProvider service toggles Group Managed Accounts' diff --git a/ee/spec/support/shared_examples/services/group_saml/saml_provider/base_service_shared_examples.rb b/ee/spec/support/shared_examples/services/group_saml/saml_provider/base_service_shared_examples.rb index 244c65dc9f6d441de54f2690fb628fde05a6a392..169c0b51e64e30719d5faff6d563e12da4ff3990 100644 --- a/ee/spec/support/shared_examples/services/group_saml/saml_provider/base_service_shared_examples.rb +++ b/ee/spec/support/shared_examples/services/group_saml/saml_provider/base_service_shared_examples.rb @@ -17,6 +17,15 @@ end it 'updates SAML provider with given params' do + expect(::Gitlab::Audit::Auditor) + .to receive(:audit).with( + hash_including( + { name: audit_event_name, + author: current_user, + scope: group, + target: group }) + ).exactly(4).times.and_call_original + expect do service.execute group.reload @@ -24,6 +33,7 @@ .and change { group.saml_provider&.certificate_fingerprint }.to(fingerprint) .and change { group.saml_provider&.enabled? }.to(true) .and change { group.saml_provider&.enforced_sso? }.to(true) + .and change { AuditEvent.count }.by(4) end end