From 80d95243b403c6a881ca441838b3e6b41d59d39b Mon Sep 17 00:00:00 2001 From: Aaron Huntsman Date: Sun, 15 Jan 2023 10:19:47 -0600 Subject: [PATCH 1/7] Refactor ProjectGroupLink create service auditing --- .../ee/projects/group_links/create_service.rb | 26 +++++++------------ .../types/project_group_link_created.yml | 9 +++++++ .../group_links/create_service_spec.rb | 17 +++++++----- 3 files changed, 30 insertions(+), 22 deletions(-) create mode 100644 ee/config/audit_events/types/project_group_link_created.yml diff --git a/ee/app/services/ee/projects/group_links/create_service.rb b/ee/app/services/ee/projects/group_links/create_service.rb index a89ecc54cc15b5..ce3963811ee4a5 100644 --- a/ee/app/services/ee/projects/group_links/create_service.rb +++ b/ee/app/services/ee/projects/group_links/create_service.rb @@ -18,8 +18,7 @@ def execute def after_successful_save super - log_audit_event - project_stream_audit_event + send_audit_event end def allowed_to_be_shared_with? @@ -39,22 +38,17 @@ def error_message _('This group cannot be invited to a project inside a group with enforced SSO') end - def log_audit_event - ::AuditEventService.new( - current_user, - link.group, - action: :create - ).for_project_group_link(link).security_event - end - - def project_stream_audit_event + def send_audit_event audit_context = { - name: 'project_group_link_create', - stream_only: true, + name: 'project_group_link_created', author: current_user, - scope: project, - target: link.group, - message: "Added project group link" + scope: link.group, + target: project, + message: 'Added project group link', + additional_details: { + add: 'project_access', + as: link.human_access + } } ::Gitlab::Audit::Auditor.audit(audit_context) diff --git a/ee/config/audit_events/types/project_group_link_created.yml b/ee/config/audit_events/types/project_group_link_created.yml new file mode 100644 index 00000000000000..bbff91e491833e --- /dev/null +++ b/ee/config/audit_events/types/project_group_link_created.yml @@ -0,0 +1,9 @@ +--- +name: project_group_link_created +description: Event triggered when a group is invited to a project +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/374114 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/108918 +feature_category: compliance_management +milestone: '15.8' +saved_to_database: true +streamed: true diff --git a/ee/spec/services/projects/group_links/create_service_spec.rb b/ee/spec/services/projects/group_links/create_service_spec.rb index fcf37fd9030926..fa51f24da1c405 100644 --- a/ee/spec/services/projects/group_links/create_service_spec.rb +++ b/ee/spec/services/projects/group_links/create_service_spec.rb @@ -32,9 +32,11 @@ add: 'project_access', as: 'Developer', author_name: user.name, + author_class: 'User', + custom_message: 'Added project group link', target_id: project.id, target_type: 'Project', - target_details: project.full_path + target_details: project.name } } end @@ -42,12 +44,15 @@ it 'sends the audit streaming event' do audit_context = { - name: 'project_group_link_create', - stream_only: true, + name: 'project_group_link_created', author: user, - scope: project, - target: group, - message: "Added project group link" + scope: group, + target: project, + message: 'Added project group link', + additional_details: { + add: 'project_access', + as: 'Developer' + } } expect(::Gitlab::Audit::Auditor).to receive(:audit).with(audit_context) -- GitLab From 9a9064b905948b04ce73a218b9ed3894703607b3 Mon Sep 17 00:00:00 2001 From: Aaron Huntsman Date: Sun, 15 Jan 2023 19:24:18 -0600 Subject: [PATCH 2/7] Refactor ProjectGroupLink update service auditing --- .../ee/projects/group_links/update_service.rb | 19 +++++++----- .../types/project_group_link_updated.yml | 9 ++++++ .../group_links/update_service_spec.rb | 31 +++++++++++++------ 3 files changed, 43 insertions(+), 16 deletions(-) create mode 100644 ee/config/audit_events/types/project_group_link_updated.yml diff --git a/ee/app/services/ee/projects/group_links/update_service.rb b/ee/app/services/ee/projects/group_links/update_service.rb index cf0e0790230604..33a7739b2e5ecc 100644 --- a/ee/app/services/ee/projects/group_links/update_service.rb +++ b/ee/app/services/ee/projects/group_links/update_service.rb @@ -10,21 +10,23 @@ module UpdateService def execute(group_link_params) super - project_stream_audit_event(group_link) + send_audit_event end private - def project_stream_audit_event(group_link) + def send_audit_event return unless saved_changes_present? + message, details = audit_message + audit_context = { - name: 'project_group_link_update', - stream_only: true, + name: 'project_group_link_updated', author: current_user, scope: project, target: group_link.group, - message: audit_message(group_link) + message: message, + additional_details: details } ::Gitlab::Audit::Auditor.audit(audit_context) @@ -34,22 +36,25 @@ def saved_changes_present? group_link.saved_changes['group_access'].present? || group_link.saved_changes['expires_at'].present? end - def audit_message(group_link) + def audit_message changes = [] + details = { change: {} } if group_link.saved_changes['group_access'].present? old_value, new_value = group_link.saved_changes['group_access'].map { |v| ::Gitlab::Access.human_access(v) } property = :group_access changes << "profile #{property} from #{old_value} to #{new_value}" + details[:change].update({ access_level: { from: old_value, to: new_value } }) end if group_link.saved_changes['expires_at'].present? old_value, new_value = group_link.saved_changes['expires_at'] property = :expires_at changes << "profile #{property} from #{old_value || 'nil'} to #{new_value || 'nil'}" + details[:change].update({ invite_expiry: { from: old_value || 'nil', to: new_value || 'nil' } }) end - "Changed project group link #{changes.join(' ')}" + ["Changed project group link #{changes.join(' ')}", details] end end end diff --git a/ee/config/audit_events/types/project_group_link_updated.yml b/ee/config/audit_events/types/project_group_link_updated.yml new file mode 100644 index 00000000000000..0561a1856980ec --- /dev/null +++ b/ee/config/audit_events/types/project_group_link_updated.yml @@ -0,0 +1,9 @@ +--- +name: project_group_link_created +description: Event triggered when a project group link is updated +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/374114 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/108918 +feature_category: compliance_management +milestone: '15.8' +saved_to_database: true +streamed: true diff --git a/ee/spec/services/projects/group_links/update_service_spec.rb b/ee/spec/services/projects/group_links/update_service_spec.rb index 366538abdd8090..6a1407084d5934 100644 --- a/ee/spec/services/projects/group_links/update_service_spec.rb +++ b/ee/spec/services/projects/group_links/update_service_spec.rb @@ -24,13 +24,18 @@ context 'audit events' do it 'sends the audit streaming event' do audit_context = { - name: 'project_group_link_update', - stream_only: true, + name: 'project_group_link_updated', author: user, scope: project, target: group, message: "Changed project group link profile group_access from Developer to Guest \ -profile expires_at from nil to #{expiry_date}" +profile expires_at from nil to #{expiry_date}", + additional_details: { + change: { + access_level: { from: 'Developer', to: 'Guest' }, + invite_expiry: { from: 'nil', to: expiry_date } + } + } } expect(::Gitlab::Audit::Auditor).to receive(:audit).with(audit_context) @@ -44,12 +49,16 @@ it 'sends the audit streaming event' do audit_context = { - name: 'project_group_link_update', - stream_only: true, + name: 'project_group_link_updated', author: user, scope: project, target: group, - message: "Changed project group link profile expires_at from nil to #{expiry_date}" + message: "Changed project group link profile expires_at from nil to #{expiry_date}", + additional_details: { + change: { + invite_expiry: { from: 'nil', to: expiry_date } + } + } } expect(::Gitlab::Audit::Auditor).to receive(:audit).with(audit_context) @@ -80,12 +89,16 @@ it 'sends the audit streaming event' do audit_context = { - name: 'project_group_link_update', - stream_only: true, + name: 'project_group_link_updated', author: user, scope: project, target: group, - message: "Changed project group link profile group_access from Developer to Guest" + message: "Changed project group link profile group_access from Developer to Guest", + additional_details: { + change: { + access_level: { from: 'Developer', to: 'Guest' } + } + } } expect(::Gitlab::Audit::Auditor).to receive(:audit).with(audit_context) -- GitLab From a8f2f35e1e6b020cb010ddd200328b0371107014 Mon Sep 17 00:00:00 2001 From: Aaron Huntsman Date: Sun, 15 Jan 2023 19:33:39 -0600 Subject: [PATCH 3/7] Add project_group_link_updated audit event type; fix project_group_link_updated --- .../audit_events/types/project_group_link_deleted.yml | 9 +++++++++ .../audit_events/types/project_group_link_updated.yml | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 ee/config/audit_events/types/project_group_link_deleted.yml diff --git a/ee/config/audit_events/types/project_group_link_deleted.yml b/ee/config/audit_events/types/project_group_link_deleted.yml new file mode 100644 index 00000000000000..f8ec55391bd71d --- /dev/null +++ b/ee/config/audit_events/types/project_group_link_deleted.yml @@ -0,0 +1,9 @@ +--- +name: project_group_link_deleted +description: Event triggered when a project group link is deleted +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/374114 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/108918 +feature_category: compliance_management +milestone: '15.8' +saved_to_database: true +streamed: true diff --git a/ee/config/audit_events/types/project_group_link_updated.yml b/ee/config/audit_events/types/project_group_link_updated.yml index 0561a1856980ec..9069f5a8f35872 100644 --- a/ee/config/audit_events/types/project_group_link_updated.yml +++ b/ee/config/audit_events/types/project_group_link_updated.yml @@ -1,5 +1,5 @@ --- -name: project_group_link_created +name: project_group_link_updated description: Event triggered when a project group link is updated introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/374114 introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/108918 -- GitLab From cff25bcf9fd189daaff35766fc98430c609d14a0 Mon Sep 17 00:00:00 2001 From: Aaron Huntsman Date: Sun, 15 Jan 2023 22:32:09 -0600 Subject: [PATCH 4/7] Refactor ProjectGroupLink destroy service auditing --- .../projects/group_links/destroy_service.rb | 27 +++++++------------ .../group_links/destroy_service_spec.rb | 16 ++++++----- 2 files changed, 19 insertions(+), 24 deletions(-) diff --git a/ee/app/services/ee/projects/group_links/destroy_service.rb b/ee/app/services/ee/projects/group_links/destroy_service.rb index 1aa5b8eb6a5613..0c775e1fb41256 100644 --- a/ee/app/services/ee/projects/group_links/destroy_service.rb +++ b/ee/app/services/ee/projects/group_links/destroy_service.rb @@ -9,33 +9,24 @@ module DestroyService override :execute def execute(group_link) super.tap do |link| - if link && !link&.persisted? - log_audit_event(link) - project_stream_audit_event(link) - end + send_audit_event(link) if link && !link&.persisted? end end private - def log_audit_event(group_link) - ::AuditEventService.new( - current_user, - group_link.group, - action: :destroy - ).for_project_group_link(group_link).security_event - end - - def project_stream_audit_event(group_link) + def send_audit_event(group_link) return unless current_user audit_context = { - name: 'project_group_link_destroy', - stream_only: true, + name: 'project_group_link_deleted', author: current_user, - scope: project, - target: group_link.group, - message: "Removed project group link" + scope: group_link.group, + target: project, + message: 'Removed project group link', + additional_details: { + remove: 'project_access' + } } ::Gitlab::Audit::Auditor.audit(audit_context) diff --git a/ee/spec/services/projects/group_links/destroy_service_spec.rb b/ee/spec/services/projects/group_links/destroy_service_spec.rb index 669d727b69f83e..420203ae488a75 100644 --- a/ee/spec/services/projects/group_links/destroy_service_spec.rb +++ b/ee/spec/services/projects/group_links/destroy_service_spec.rb @@ -26,9 +26,11 @@ details: { remove: 'project_access', author_name: user.name, + author_class: 'User', target_id: project.id, target_type: 'Project', - target_details: project.full_path + target_details: project.name, + custom_message: 'Removed project group link' } } end @@ -36,12 +38,14 @@ it 'sends the audit streaming event' do audit_context = { - name: 'project_group_link_destroy', - stream_only: true, + name: 'project_group_link_deleted', author: user, - scope: project, - target: group, - message: "Removed project group link" + scope: group, + target: project, + message: 'Removed project group link', + additional_details: { + remove: 'project_access' + } } expect(::Gitlab::Audit::Auditor).to receive(:audit).with(audit_context) -- GitLab From c203f67005f7c78dae7f3168e36a6caac6b0c0f3 Mon Sep 17 00:00:00 2001 From: Aaron Huntsman Date: Sun, 15 Jan 2023 22:57:10 -0600 Subject: [PATCH 5/7] Remove AuditEventService helpers related to project group links --- ee/app/services/ee/audit_event_service.rb | 35 -------------------- ee/spec/services/audit_event_service_spec.rb | 16 --------- 2 files changed, 51 deletions(-) diff --git a/ee/app/services/ee/audit_event_service.rb b/ee/app/services/ee/audit_event_service.rb index 21c18ca3430a02..cf807a128ce5ce 100644 --- a/ee/app/services/ee/audit_event_service.rb +++ b/ee/app/services/ee/audit_event_service.rb @@ -68,24 +68,6 @@ def for_member(member) self end - # Builds the @details attribute for project group link - # - # This expects [String] :action of :destroy, :create, :update to be - # specified in @details attribute - # - # @param [ProjectGroupLink] group_link object being audited - # - # @return [AuditEventService] - def for_project_group_link(group_link) - @details = custom_project_link_group_attributes(group_link) - .merge(author_name: @author.name, - target_id: group_link.project.id, - target_type: 'Project', - target_details: group_link.project.full_path) - - self - end - # Builds the @details attribute for a failed login # # @return [AuditEventService] @@ -310,23 +292,6 @@ def add_impersonation_details! end end - def custom_project_link_group_attributes(group_link) - case @details[:action] - when :destroy - { remove: 'project_access' } - when :create - { - add: 'project_access', - as: group_link.human_access - } - when :update - { - change: 'access_level', - from: @details[:old_access_level], - to: group_link.human_access - } - end - end # rubocop:enable Gitlab/ModuleWithInstanceVariables end end diff --git a/ee/spec/services/audit_event_service_spec.rb b/ee/spec/services/audit_event_service_spec.rb index e95e7a631ccfea..40a591e34895f1 100644 --- a/ee/spec/services/audit_event_service_spec.rb +++ b/ee/spec/services/audit_event_service_spec.rb @@ -347,22 +347,6 @@ end end - describe '#for_project_group_link' do - let_it_be(:current_user) { create(:user) } - let_it_be(:project) { create(:project) } - let_it_be(:group) { create(:group) } - let_it_be(:link) { create(:project_group_link, group: group, project: project) } - - let(:options) { { action: :create } } - - subject(:event) { described_class.new(current_user, project, options).for_project_group_link(link).security_event } - - it 'sets the target_type attribute' do - expect(event.details[:target_type]).to eq('Project') - expect(event.target_type).to eq('Project') - end - end - describe '#for_user' do let(:current_user) { create(:user) } let(:user) { create(:user) } -- GitLab From d6f912bec21b3b10ff1f83443dafac0fd79cf9c5 Mon Sep 17 00:00:00 2001 From: Aaron Huntsman Date: Fri, 20 Jan 2023 12:40:18 -0600 Subject: [PATCH 6/7] Update milestone from 15.8 to 15.9 --- ee/config/audit_events/types/project_group_link_created.yml | 2 +- ee/config/audit_events/types/project_group_link_deleted.yml | 2 +- ee/config/audit_events/types/project_group_link_updated.yml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ee/config/audit_events/types/project_group_link_created.yml b/ee/config/audit_events/types/project_group_link_created.yml index bbff91e491833e..ecf6071adb82b4 100644 --- a/ee/config/audit_events/types/project_group_link_created.yml +++ b/ee/config/audit_events/types/project_group_link_created.yml @@ -4,6 +4,6 @@ description: Event triggered when a group is invited to a project introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/374114 introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/108918 feature_category: compliance_management -milestone: '15.8' +milestone: '15.9' saved_to_database: true streamed: true diff --git a/ee/config/audit_events/types/project_group_link_deleted.yml b/ee/config/audit_events/types/project_group_link_deleted.yml index f8ec55391bd71d..a9fc9c0ef139d7 100644 --- a/ee/config/audit_events/types/project_group_link_deleted.yml +++ b/ee/config/audit_events/types/project_group_link_deleted.yml @@ -4,6 +4,6 @@ description: Event triggered when a project group link is deleted introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/374114 introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/108918 feature_category: compliance_management -milestone: '15.8' +milestone: '15.9' saved_to_database: true streamed: true diff --git a/ee/config/audit_events/types/project_group_link_updated.yml b/ee/config/audit_events/types/project_group_link_updated.yml index 9069f5a8f35872..f024e13c87065b 100644 --- a/ee/config/audit_events/types/project_group_link_updated.yml +++ b/ee/config/audit_events/types/project_group_link_updated.yml @@ -4,6 +4,6 @@ description: Event triggered when a project group link is updated introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/374114 introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/108918 feature_category: compliance_management -milestone: '15.8' +milestone: '15.9' saved_to_database: true streamed: true -- GitLab From 1247715a956b78b03086127760472e0b37808a92 Mon Sep 17 00:00:00 2001 From: Aaron Huntsman Date: Mon, 30 Jan 2023 07:02:12 -0600 Subject: [PATCH 7/7] Add project.full_path to audit event details --- ee/app/services/ee/projects/group_links/create_service.rb | 1 + ee/app/services/ee/projects/group_links/destroy_service.rb | 1 + ee/spec/services/projects/group_links/create_service_spec.rb | 3 ++- ee/spec/services/projects/group_links/destroy_service_spec.rb | 3 ++- 4 files changed, 6 insertions(+), 2 deletions(-) diff --git a/ee/app/services/ee/projects/group_links/create_service.rb b/ee/app/services/ee/projects/group_links/create_service.rb index ce3963811ee4a5..09d9faf592256e 100644 --- a/ee/app/services/ee/projects/group_links/create_service.rb +++ b/ee/app/services/ee/projects/group_links/create_service.rb @@ -44,6 +44,7 @@ def send_audit_event author: current_user, scope: link.group, target: project, + target_details: project.full_path, message: 'Added project group link', additional_details: { add: 'project_access', diff --git a/ee/app/services/ee/projects/group_links/destroy_service.rb b/ee/app/services/ee/projects/group_links/destroy_service.rb index 0c775e1fb41256..6fbea796ba364f 100644 --- a/ee/app/services/ee/projects/group_links/destroy_service.rb +++ b/ee/app/services/ee/projects/group_links/destroy_service.rb @@ -23,6 +23,7 @@ def send_audit_event(group_link) author: current_user, scope: group_link.group, target: project, + target_details: project.full_path, message: 'Removed project group link', additional_details: { remove: 'project_access' diff --git a/ee/spec/services/projects/group_links/create_service_spec.rb b/ee/spec/services/projects/group_links/create_service_spec.rb index fa51f24da1c405..5a54140e9c52ed 100644 --- a/ee/spec/services/projects/group_links/create_service_spec.rb +++ b/ee/spec/services/projects/group_links/create_service_spec.rb @@ -36,7 +36,7 @@ custom_message: 'Added project group link', target_id: project.id, target_type: 'Project', - target_details: project.name + target_details: project.full_path } } end @@ -48,6 +48,7 @@ author: user, scope: group, target: project, + target_details: project.full_path, message: 'Added project group link', additional_details: { add: 'project_access', diff --git a/ee/spec/services/projects/group_links/destroy_service_spec.rb b/ee/spec/services/projects/group_links/destroy_service_spec.rb index 420203ae488a75..4591a3912c28a2 100644 --- a/ee/spec/services/projects/group_links/destroy_service_spec.rb +++ b/ee/spec/services/projects/group_links/destroy_service_spec.rb @@ -29,7 +29,7 @@ author_class: 'User', target_id: project.id, target_type: 'Project', - target_details: project.name, + target_details: project.full_path, custom_message: 'Removed project group link' } } @@ -42,6 +42,7 @@ author: user, scope: group, target: project, + target_details: project.full_path, message: 'Removed project group link', additional_details: { remove: 'project_access' -- GitLab