From 477fe1a51e6bed6b15710860057b8af22d60e06d Mon Sep 17 00:00:00 2001 From: Jay Montal Date: Fri, 11 Aug 2023 14:20:17 -0600 Subject: [PATCH 1/3] Add audit event for changed user profile visiblity - Event is stream only - See: https://gitlab.com/gitlab-org/gitlab/-/issues/377627#streaming-only-event-or-normal-event Changelog: changed EE: true --- .../audit_event_types.md | 1 + ee/app/services/ee/users/update_service.rb | 6 +++ .../types/user_profile_visiblity_updated.yml | 9 ++++ ee/lib/audit/user_setting_changes_auditor.rb | 30 ++++++++++++++ .../user_setting_changes_auditor_spec.rb | 41 +++++++++++++++++++ 5 files changed, 87 insertions(+) create mode 100644 ee/config/audit_events/types/user_profile_visiblity_updated.yml create mode 100644 ee/lib/audit/user_setting_changes_auditor.rb create mode 100644 ee/spec/lib/audit/user_setting_changes_auditor_spec.rb diff --git a/doc/administration/audit_event_streaming/audit_event_types.md b/doc/administration/audit_event_streaming/audit_event_types.md index 2a7c3367f6a466..1a515ebd1ebd4d 100644 --- a/doc/administration/audit_event_streaming/audit_event_types.md +++ b/doc/administration/audit_event_streaming/audit_event_types.md @@ -291,6 +291,7 @@ Every audit event is associated with an event type. The association with the eve | [`user_enable_admin_mode`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/104754) | Event triggered on enabling admin mode | **{check-circle}** Yes | **{check-circle}** Yes | `system_access` | GitLab [15.7](https://gitlab.com/gitlab-org/gitlab/-/issues/362101) | | [`user_impersonation`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/79340) | Triggered when an instance administrator starts or stops impersonating a user | **{check-circle}** Yes | **{check-circle}** Yes | `user_management` | GitLab [14.8](https://gitlab.com/gitlab-org/gitlab/-/issues/300961) | | [`user_password_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/106086) | audit when user password is updated | **{check-circle}** Yes | **{check-circle}** Yes | `user_management` | GitLab [15.7](https://gitlab.com/gitlab-org/gitlab/-/issues/369330) | +| [`user_profile_visiblity_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129149) | Triggered when user toggles private profile user setting | **{check-circle}** Yes | **{check-circle}** Yes | `user_profile` | GitLab [16.3](https://gitlab.com/gitlab-org/gitlab/-/issues/377627) | | [`user_rejected`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/113784) | Event triggered when a user registration is rejected | **{check-circle}** Yes | **{dotted-circle}** No | `user_management` | GitLab [15.11](https://gitlab.com/gitlab-org/gitlab/-/issues/374107) | | [`user_username_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/106086) | Event triggered on updating a user's username | **{check-circle}** Yes | **{check-circle}** Yes | `user_profile` | GitLab [15.7](https://gitlab.com/gitlab-org/gitlab/-/issues/369329) | | [`feature_flag_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/113453) | Triggered when a feature flag is created. | **{check-circle}** Yes | **{check-circle}** Yes | `feature_flags` | GitLab [15.10](https://gitlab.com/gitlab-org/gitlab/-/issues/374109) | diff --git a/ee/app/services/ee/users/update_service.rb b/ee/app/services/ee/users/update_service.rb index 2f263cad4bec11..ed0a6313bbdc57 100644 --- a/ee/app/services/ee/users/update_service.rb +++ b/ee/app/services/ee/users/update_service.rb @@ -32,6 +32,8 @@ def notify_success(user_exists) audit_changes(:admin, as: 'admin status', event_type: 'user_admin_status_updated') + + log_audit_events end def model @@ -86,6 +88,10 @@ def saml_provider_id end end end + + def log_audit_events + Audit::UserSettingChangesAuditor.new(current_user).execute + end end end end diff --git a/ee/config/audit_events/types/user_profile_visiblity_updated.yml b/ee/config/audit_events/types/user_profile_visiblity_updated.yml new file mode 100644 index 00000000000000..b454b0c273c877 --- /dev/null +++ b/ee/config/audit_events/types/user_profile_visiblity_updated.yml @@ -0,0 +1,9 @@ +--- +name: user_profile_visiblity_updated +description: Triggered when user toggles private profile user setting +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/377627 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129149 +feature_category: user_profile +milestone: '16.3' +saved_to_database: false +streamed: true diff --git a/ee/lib/audit/user_setting_changes_auditor.rb b/ee/lib/audit/user_setting_changes_auditor.rb new file mode 100644 index 00000000000000..ae0cc2a0acd5be --- /dev/null +++ b/ee/lib/audit/user_setting_changes_auditor.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +module Audit + class UserSettingChangesAuditor < BaseChangesAuditor + def initialize(current_user) + super(current_user, current_user) + end + + def execute + return if model.blank? + + audit_changes( + :private_profile, + as: 'user_profile_visiblity', + entity: @current_user, + model: model, + event_type: 'user_profile_visiblity_updated' + ) + end + + private + + def attributes_from_auditable_model(column) + { + from: model.previous_changes[column].first, + to: model.previous_changes[column].last + } + end + end +end diff --git a/ee/spec/lib/audit/user_setting_changes_auditor_spec.rb b/ee/spec/lib/audit/user_setting_changes_auditor_spec.rb new file mode 100644 index 00000000000000..73223db44c2cf1 --- /dev/null +++ b/ee/spec/lib/audit/user_setting_changes_auditor_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Audit::UserSettingChangesAuditor, feature_category: :user_profile do + using RSpec::Parameterized::TableSyntax + describe '#execute' do + let_it_be(:user) { create(:user) } + let_it_be(:user_setting_changes_auditor) { described_class.new(user) } + + before do + stub_licensed_features(extended_audit_events: true, external_audit_events: true) + end + + context 'when user setting is updated' do + context 'when user profile visiblity (`private_profile`) is changed' do + where(:prev_value, :new_value) do + true | false + false | true + end + + with_them do + before do + user.update!(private_profile: prev_value) + end + + it 'creates an audit event' do + user.update!(private_profile: new_value) + + expect { user_setting_changes_auditor.execute }.to change { AuditEvent.count }.by(1) + expect(AuditEvent.last.details).to include({ + change: 'user_profile_visiblity', + from: prev_value, + to: new_value + }) + end + end + end + end + end +end -- GitLab From 9160a48f432662d68b201435f368b65a4551ed84 Mon Sep 17 00:00:00 2001 From: Jay Montal Date: Wed, 16 Aug 2023 08:45:28 -0600 Subject: [PATCH 2/3] Updates based on initial feedback --- .../audit_event_types.md | 2 +- .../types/user_profile_visiblity_updated.yml | 2 +- .../user_setting_changes_auditor_spec.rb | 50 +++++++++++-------- 3 files changed, 32 insertions(+), 22 deletions(-) diff --git a/doc/administration/audit_event_streaming/audit_event_types.md b/doc/administration/audit_event_streaming/audit_event_types.md index 1a515ebd1ebd4d..dc4a2cbf71e85c 100644 --- a/doc/administration/audit_event_streaming/audit_event_types.md +++ b/doc/administration/audit_event_streaming/audit_event_types.md @@ -291,7 +291,7 @@ Every audit event is associated with an event type. The association with the eve | [`user_enable_admin_mode`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/104754) | Event triggered on enabling admin mode | **{check-circle}** Yes | **{check-circle}** Yes | `system_access` | GitLab [15.7](https://gitlab.com/gitlab-org/gitlab/-/issues/362101) | | [`user_impersonation`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/79340) | Triggered when an instance administrator starts or stops impersonating a user | **{check-circle}** Yes | **{check-circle}** Yes | `user_management` | GitLab [14.8](https://gitlab.com/gitlab-org/gitlab/-/issues/300961) | | [`user_password_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/106086) | audit when user password is updated | **{check-circle}** Yes | **{check-circle}** Yes | `user_management` | GitLab [15.7](https://gitlab.com/gitlab-org/gitlab/-/issues/369330) | -| [`user_profile_visiblity_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129149) | Triggered when user toggles private profile user setting | **{check-circle}** Yes | **{check-circle}** Yes | `user_profile` | GitLab [16.3](https://gitlab.com/gitlab-org/gitlab/-/issues/377627) | +| [`user_profile_visiblity_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129149) | Triggered when user toggles private profile user setting | **{dotted-circle}** No | **{check-circle}** Yes | `user_profile` | GitLab [16.3](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129149) | | [`user_rejected`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/113784) | Event triggered when a user registration is rejected | **{check-circle}** Yes | **{dotted-circle}** No | `user_management` | GitLab [15.11](https://gitlab.com/gitlab-org/gitlab/-/issues/374107) | | [`user_username_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/106086) | Event triggered on updating a user's username | **{check-circle}** Yes | **{check-circle}** Yes | `user_profile` | GitLab [15.7](https://gitlab.com/gitlab-org/gitlab/-/issues/369329) | | [`feature_flag_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/113453) | Triggered when a feature flag is created. | **{check-circle}** Yes | **{check-circle}** Yes | `feature_flags` | GitLab [15.10](https://gitlab.com/gitlab-org/gitlab/-/issues/374109) | diff --git a/ee/config/audit_events/types/user_profile_visiblity_updated.yml b/ee/config/audit_events/types/user_profile_visiblity_updated.yml index b454b0c273c877..fb522db73845e3 100644 --- a/ee/config/audit_events/types/user_profile_visiblity_updated.yml +++ b/ee/config/audit_events/types/user_profile_visiblity_updated.yml @@ -1,7 +1,7 @@ --- name: user_profile_visiblity_updated description: Triggered when user toggles private profile user setting -introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/377627 +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129149 introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129149 feature_category: user_profile milestone: '16.3' diff --git a/ee/spec/lib/audit/user_setting_changes_auditor_spec.rb b/ee/spec/lib/audit/user_setting_changes_auditor_spec.rb index 73223db44c2cf1..a9224d3ed2f0d0 100644 --- a/ee/spec/lib/audit/user_setting_changes_auditor_spec.rb +++ b/ee/spec/lib/audit/user_setting_changes_auditor_spec.rb @@ -6,34 +6,44 @@ using RSpec::Parameterized::TableSyntax describe '#execute' do let_it_be(:user) { create(:user) } - let_it_be(:user_setting_changes_auditor) { described_class.new(user) } + + subject(:user_setting_changes_auditor) { described_class.new(user) } before do stub_licensed_features(extended_audit_events: true, external_audit_events: true) end context 'when user setting is updated' do - context 'when user profile visiblity (`private_profile`) is changed' do - where(:prev_value, :new_value) do - true | false - false | true + where(:column, :change, :event, :change_from, :change_to) do + 'private_profile' | 'user_profile_visiblity' | 'user_profile_visiblity_updated' | true | false + 'private_profile' | 'user_profile_visiblity' | 'user_profile_visiblity_updated' | false | true + end + + with_them do + before do + user.update!(column.to_sym => change_from) end - with_them do - before do - user.update!(private_profile: prev_value) - end - - it 'creates an audit event' do - user.update!(private_profile: new_value) - - expect { user_setting_changes_auditor.execute }.to change { AuditEvent.count }.by(1) - expect(AuditEvent.last.details).to include({ - change: 'user_profile_visiblity', - from: prev_value, - to: new_value - }) - end + it 'calls auditor' do + user.update!(column.to_sym => change_to) + + expect(Gitlab::Audit::Auditor).to receive(:audit).with( + { + name: event.to_s, + author: user, + scope: user, + target: user, + message: "Changed #{change} from #{change_from} to #{change_to}", + additional_details: { + change: change.to_s, + from: change_from, + to: change_to + }, + target_details: nil + } + ).and_call_original + + user_setting_changes_auditor.execute end end end -- GitLab From d732c5a6d7a4a5054c2ba40db1119ceca5f2d90e Mon Sep 17 00:00:00 2001 From: Huzaifa Iftikhar Date: Tue, 5 Sep 2023 13:17:52 +0000 Subject: [PATCH 3/3] Update specs based on feedback --- ee/spec/lib/audit/user_setting_changes_auditor_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/lib/audit/user_setting_changes_auditor_spec.rb b/ee/spec/lib/audit/user_setting_changes_auditor_spec.rb index a9224d3ed2f0d0..3b71845d948238 100644 --- a/ee/spec/lib/audit/user_setting_changes_auditor_spec.rb +++ b/ee/spec/lib/audit/user_setting_changes_auditor_spec.rb @@ -29,7 +29,7 @@ expect(Gitlab::Audit::Auditor).to receive(:audit).with( { - name: event.to_s, + name: event, author: user, scope: user, target: user, -- GitLab