From cc61b27a069e7dbf1bc2208055aabdd5878472a9 Mon Sep 17 00:00:00 2001 From: huzaifaiftikhar1 Date: Tue, 23 May 2023 11:55:46 +0530 Subject: [PATCH 1/3] Add audit events for issue creation by project access tokens When an issue, incident, test case or epic is created, closed, reopened by a project or a group access token then an audit event is created. Changelog: added EE: true --- app/services/issues/close_service.rb | 5 +++ app/services/issues/reopen_service.rb | 5 +++ app/workers/new_issue_worker.rb | 10 +++++ ee/app/services/ee/issues/close_service.rb | 16 +++++++ ee/app/services/ee/issues/reopen_service.rb | 16 +++++++ ee/app/services/epics/close_service.rb | 14 ++++++ ee/app/services/epics/reopen_service.rb | 14 ++++++ ee/app/workers/ee/new_issue_worker.rb | 22 ++++++++++ ee/app/workers/new_epic_worker.rb | 16 +++++++ .../types/epic_closed_by_project_bot.yml | 9 ++++ .../types/epic_created_by_project_bot.yml | 9 ++++ .../types/epic_reopened_by_project_bot.yml | 9 ++++ .../types/incident_closed_by_project_bot.yml | 9 ++++ .../types/incident_created_by_project_bot.yml | 9 ++++ .../incident_reopened_by_project_bot.yml | 9 ++++ .../types/issue_closed_by_project_bot.yml | 9 ++++ .../types/issue_created_by_project_bot.yml | 9 ++++ .../types/issue_reopened_by_project_bot.yml | 9 ++++ .../types/test_case_closed_by_project_bot.yml | 9 ++++ .../test_case_created_by_project_bot.yml | 9 ++++ .../test_case_reopened_by_project_bot.yml | 9 ++++ .../services/ee/issues/close_service_spec.rb | 44 +++++++++++++++++++ .../services/ee/issues/reopen_service_spec.rb | 44 +++++++++++++++++++ ee/spec/services/epics/close_service_spec.rb | 33 ++++++++++++++ ee/spec/services/epics/reopen_service_spec.rb | 33 ++++++++++++++ .../audit_event_logging_shared_examples.rb | 6 ++- ee/spec/workers/ee/new_issue_worker_spec.rb | 44 +++++++++++++++++++ ee/spec/workers/new_epic_worker_spec.rb | 29 ++++++++++++ 28 files changed, 458 insertions(+), 1 deletion(-) create mode 100644 ee/app/workers/ee/new_issue_worker.rb create mode 100644 ee/config/audit_events/types/epic_closed_by_project_bot.yml create mode 100644 ee/config/audit_events/types/epic_created_by_project_bot.yml create mode 100644 ee/config/audit_events/types/epic_reopened_by_project_bot.yml create mode 100644 ee/config/audit_events/types/incident_closed_by_project_bot.yml create mode 100644 ee/config/audit_events/types/incident_created_by_project_bot.yml create mode 100644 ee/config/audit_events/types/incident_reopened_by_project_bot.yml create mode 100644 ee/config/audit_events/types/issue_closed_by_project_bot.yml create mode 100644 ee/config/audit_events/types/issue_created_by_project_bot.yml create mode 100644 ee/config/audit_events/types/issue_reopened_by_project_bot.yml create mode 100644 ee/config/audit_events/types/test_case_closed_by_project_bot.yml create mode 100644 ee/config/audit_events/types/test_case_created_by_project_bot.yml create mode 100644 ee/config/audit_events/types/test_case_reopened_by_project_bot.yml create mode 100644 ee/spec/services/ee/issues/close_service_spec.rb create mode 100644 ee/spec/services/ee/issues/reopen_service_spec.rb create mode 100644 ee/spec/workers/ee/new_issue_worker_spec.rb diff --git a/app/services/issues/close_service.rb b/app/services/issues/close_service.rb index e45033f2b916c1..96c76b09b10d50 100644 --- a/app/services/issues/close_service.rb +++ b/app/services/issues/close_service.rb @@ -27,6 +27,7 @@ def close_issue(issue, closed_via: nil, notifications: true, system_note: true) if issue.close(current_user) event_service.close_issue(issue, current_user) create_note(issue, closed_via) if system_note + log_audit_event(issue, current_user) if current_user.project_bot? closed_via = _("commit %{commit_id}") % { commit_id: closed_via.id } if closed_via.is_a?(Commit) @@ -51,6 +52,10 @@ def close_issue(issue, closed_via: nil, notifications: true, system_note: true) private + def log_audit_event(issue, user) + # defined in EE + end + def can_close?(issue, skip_authorization: false) skip_authorization || can?(current_user, :update_issue, issue) || issue.is_a?(ExternalIssue) end diff --git a/app/services/issues/reopen_service.rb b/app/services/issues/reopen_service.rb index f4d229ecec7a59..4f6cb17079dbfc 100644 --- a/app/services/issues/reopen_service.rb +++ b/app/services/issues/reopen_service.rb @@ -7,6 +7,7 @@ def execute(issue, skip_authorization: false) if issue.reopen event_service.reopen_issue(issue, current_user) + log_audit_event(issue, current_user) if current_user.project_bot? create_note(issue, 'reopened') notification_service.async.reopen_issue(issue, current_user) perform_incident_management_actions(issue) @@ -39,6 +40,10 @@ def create_note(issue, state = issue.state) def create_timeline_event(issue) IncidentManagement::TimelineEvents::CreateService.reopen_incident(issue, current_user) end + + def log_audit_event(issue, user) + # defined in EE + end end end diff --git a/app/workers/new_issue_worker.rb b/app/workers/new_issue_worker.rb index 07699a50e36e04..0e7f11debd290e 100644 --- a/app/workers/new_issue_worker.rb +++ b/app/workers/new_issue_worker.rb @@ -28,5 +28,15 @@ def perform(issue_id, user_id, issuable_class = 'Issue') Issues::AfterCreateService .new(container: issuable.project, current_user: user) .execute(issuable) + + log_audit_event if user.project_bot? + end + + private + + def log_audit_event + # defined in EE end end + +NewIssueWorker.prepend_mod diff --git a/ee/app/services/ee/issues/close_service.rb b/ee/app/services/ee/issues/close_service.rb index bd75ab1ba2d452..30870dac820c9d 100644 --- a/ee/app/services/ee/issues/close_service.rb +++ b/ee/app/services/ee/issues/close_service.rb @@ -10,6 +10,22 @@ def perform_incident_management_actions(issue) super update_issuable_sla(issue) end + + private + + override :log_audit_event + def log_audit_event(issue, user) + audit_context = { + name: "#{issue.issue_type}_closed_by_project_bot", + author: user, + scope: issue.respond_to?(:group) ? issue.group : issue.project, + target: issue, + message: "Closed #{issue.issue_type.humanize(capitalize: false)} #{issue.title}", + target_details: { iid: issue.iid, id: issue.id } + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end end end end diff --git a/ee/app/services/ee/issues/reopen_service.rb b/ee/app/services/ee/issues/reopen_service.rb index 95ef2ee89ff81d..a5831e512a79d8 100644 --- a/ee/app/services/ee/issues/reopen_service.rb +++ b/ee/app/services/ee/issues/reopen_service.rb @@ -10,6 +10,22 @@ def perform_incident_management_actions(issue) super update_issuable_sla(issue) end + + private + + override :log_audit_event + def log_audit_event(issue, user) + audit_context = { + name: "#{issue.issue_type}_reopened_by_project_bot", + author: user, + scope: issue.respond_to?(:group) ? issue.group : issue.project, + target: issue, + message: "Reopened #{issue.issue_type.humanize(capitalize: false)} #{issue.title}", + target_details: { iid: issue.iid, id: issue.id } + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end end end end diff --git a/ee/app/services/epics/close_service.rb b/ee/app/services/epics/close_service.rb index f7eda3f441dbda..23a4a560952739 100644 --- a/ee/app/services/epics/close_service.rb +++ b/ee/app/services/epics/close_service.rb @@ -20,7 +20,21 @@ def close_epic(epic) author: current_user, namespace: epic.group ) + log_audit_event(epic) if current_user.project_bot? end end + + def log_audit_event(epic) + audit_context = { + name: "epic_closed_by_project_bot", + author: current_user, + scope: epic.group, + target: epic, + message: "Closed epic #{epic.title}", + target_details: { iid: epic.iid, id: epic.id } + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end end end diff --git a/ee/app/services/epics/reopen_service.rb b/ee/app/services/epics/reopen_service.rb index 2c76579f3654e6..27bacb6e41e656 100644 --- a/ee/app/services/epics/reopen_service.rb +++ b/ee/app/services/epics/reopen_service.rb @@ -19,7 +19,21 @@ def reopen_epic(epic) author: current_user, namespace: epic.group ) + log_audit_event(epic) if current_user.project_bot? end end + + def log_audit_event(epic) + audit_context = { + name: "epic_reopened_by_project_bot", + author: current_user, + scope: epic.group, + target: epic, + message: "Reopened epic #{epic.title}", + target_details: { iid: epic.iid, id: epic.id } + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end end end diff --git a/ee/app/workers/ee/new_issue_worker.rb b/ee/app/workers/ee/new_issue_worker.rb new file mode 100644 index 00000000000000..cff9014d1cd207 --- /dev/null +++ b/ee/app/workers/ee/new_issue_worker.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module EE + module NewIssueWorker + extend ::Gitlab::Utils::Override + + private + + def log_audit_event + audit_context = { + name: "#{issuable.issue_type}_created_by_project_bot", + author: user, + scope: issuable.respond_to?(:group) ? issuable.group : issuable.project, + target: issuable, + message: "Created #{issuable.issue_type.humanize(capitalize: false)} #{issuable.title}", + target_details: { iid: issuable.iid, id: issuable.id } + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end + end +end diff --git a/ee/app/workers/new_epic_worker.rb b/ee/app/workers/new_epic_worker.rb index e22ab28ba5d19d..3bf68f88293039 100644 --- a/ee/app/workers/new_epic_worker.rb +++ b/ee/app/workers/new_epic_worker.rb @@ -18,9 +18,25 @@ def perform(epic_id, user_id) EventCreateService.new.open_epic(issuable, user) NotificationService.new.new_epic(issuable, user) issuable.create_cross_references!(user) + log_audit_event if user.project_bot? end def issuable_class Epic end + + private + + def log_audit_event + audit_context = { + name: "epic_created_by_project_bot", + author: user, + scope: issuable.group, + target: issuable, + message: "Created epic #{issuable.title}", + target_details: { iid: issuable.iid, id: issuable.id } + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end end diff --git a/ee/config/audit_events/types/epic_closed_by_project_bot.yml b/ee/config/audit_events/types/epic_closed_by_project_bot.yml new file mode 100644 index 00000000000000..1b469b6de035be --- /dev/null +++ b/ee/config/audit_events/types/epic_closed_by_project_bot.yml @@ -0,0 +1,9 @@ +--- +name: epic_closed_by_project_bot +description: Triggered when an epic is closed by a group access token +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/323299 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121485 +feature_category: portfolio_management +milestone: '16.1' +saved_to_database: true +streamed: true diff --git a/ee/config/audit_events/types/epic_created_by_project_bot.yml b/ee/config/audit_events/types/epic_created_by_project_bot.yml new file mode 100644 index 00000000000000..57d1a3b0c5678e --- /dev/null +++ b/ee/config/audit_events/types/epic_created_by_project_bot.yml @@ -0,0 +1,9 @@ +--- +name: epic_created_by_project_bot +description: Triggered when an epic is created by a group access token +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/323299 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121485 +feature_category: portfolio_management +milestone: '16.1' +saved_to_database: true +streamed: true diff --git a/ee/config/audit_events/types/epic_reopened_by_project_bot.yml b/ee/config/audit_events/types/epic_reopened_by_project_bot.yml new file mode 100644 index 00000000000000..9457bd5265118a --- /dev/null +++ b/ee/config/audit_events/types/epic_reopened_by_project_bot.yml @@ -0,0 +1,9 @@ +--- +name: epic_reopened_by_project_bot +description: Triggered when an epic is reopened by a group access token +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/323299 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121485 +feature_category: portfolio_management +milestone: '16.1' +saved_to_database: true +streamed: true diff --git a/ee/config/audit_events/types/incident_closed_by_project_bot.yml b/ee/config/audit_events/types/incident_closed_by_project_bot.yml new file mode 100644 index 00000000000000..7c1f9caa50dec6 --- /dev/null +++ b/ee/config/audit_events/types/incident_closed_by_project_bot.yml @@ -0,0 +1,9 @@ +--- +name: incident_closed_by_project_bot +description: Triggered when an incident is closed using a project access token +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/323299 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121485 +feature_category: incident_management +milestone: '16.1' +saved_to_database: true +streamed: true diff --git a/ee/config/audit_events/types/incident_created_by_project_bot.yml b/ee/config/audit_events/types/incident_created_by_project_bot.yml new file mode 100644 index 00000000000000..c5240842f1c6c6 --- /dev/null +++ b/ee/config/audit_events/types/incident_created_by_project_bot.yml @@ -0,0 +1,9 @@ +--- +name: incident_created_by_project_bot +description: Triggered when an incident is created using a project access token +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/323299 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121485 +feature_category: incident_management +milestone: '16.1' +saved_to_database: true +streamed: true diff --git a/ee/config/audit_events/types/incident_reopened_by_project_bot.yml b/ee/config/audit_events/types/incident_reopened_by_project_bot.yml new file mode 100644 index 00000000000000..117cede775e764 --- /dev/null +++ b/ee/config/audit_events/types/incident_reopened_by_project_bot.yml @@ -0,0 +1,9 @@ +--- +name: incident_reopened_by_project_bot +description: Triggered when an incident is reopened using a project access token +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/323299 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121485 +feature_category: incident_management +milestone: '16.1' +saved_to_database: true +streamed: true diff --git a/ee/config/audit_events/types/issue_closed_by_project_bot.yml b/ee/config/audit_events/types/issue_closed_by_project_bot.yml new file mode 100644 index 00000000000000..e25bfa3ae98e50 --- /dev/null +++ b/ee/config/audit_events/types/issue_closed_by_project_bot.yml @@ -0,0 +1,9 @@ +--- +name: issue_closed_by_project_bot +description: Triggered when an issue is closed using a project access token +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/323299 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121485 +feature_category: team_planning +milestone: '16.1' +saved_to_database: true +streamed: true diff --git a/ee/config/audit_events/types/issue_created_by_project_bot.yml b/ee/config/audit_events/types/issue_created_by_project_bot.yml new file mode 100644 index 00000000000000..a73968400bc728 --- /dev/null +++ b/ee/config/audit_events/types/issue_created_by_project_bot.yml @@ -0,0 +1,9 @@ +--- +name: issue_created_by_project_bot +description: Triggered when an issue is created using a project access token +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/323299 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121485 +feature_category: team_planning +milestone: '16.1' +saved_to_database: true +streamed: true diff --git a/ee/config/audit_events/types/issue_reopened_by_project_bot.yml b/ee/config/audit_events/types/issue_reopened_by_project_bot.yml new file mode 100644 index 00000000000000..2c4b8932e1efc3 --- /dev/null +++ b/ee/config/audit_events/types/issue_reopened_by_project_bot.yml @@ -0,0 +1,9 @@ +--- +name: issue_reopened_by_project_bot +description: Triggered when an issue is reopened using a project access token +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/323299 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121485 +feature_category: team_planning +milestone: '16.1' +saved_to_database: true +streamed: true diff --git a/ee/config/audit_events/types/test_case_closed_by_project_bot.yml b/ee/config/audit_events/types/test_case_closed_by_project_bot.yml new file mode 100644 index 00000000000000..1eb4abab535f3e --- /dev/null +++ b/ee/config/audit_events/types/test_case_closed_by_project_bot.yml @@ -0,0 +1,9 @@ +--- +name: test_case_closed_by_project_bot +description: Triggered when a test case is closed using a project access token +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/323299 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121485 +feature_category: quality_management +milestone: '16.1' +saved_to_database: true +streamed: true diff --git a/ee/config/audit_events/types/test_case_created_by_project_bot.yml b/ee/config/audit_events/types/test_case_created_by_project_bot.yml new file mode 100644 index 00000000000000..50032b4248713e --- /dev/null +++ b/ee/config/audit_events/types/test_case_created_by_project_bot.yml @@ -0,0 +1,9 @@ +--- +name: test_case_created_by_project_bot +description: Triggered when a test case is created using a project access token +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/323299 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121485 +feature_category: quality_management +milestone: '16.1' +saved_to_database: true +streamed: true diff --git a/ee/config/audit_events/types/test_case_reopened_by_project_bot.yml b/ee/config/audit_events/types/test_case_reopened_by_project_bot.yml new file mode 100644 index 00000000000000..dbb5c48053a8de --- /dev/null +++ b/ee/config/audit_events/types/test_case_reopened_by_project_bot.yml @@ -0,0 +1,9 @@ +--- +name: test_case_reopened_by_project_bot +description: Triggered when a test case is reopened using a project access token +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/323299 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121485 +feature_category: quality_management +milestone: '16.1' +saved_to_database: true +streamed: true diff --git a/ee/spec/services/ee/issues/close_service_spec.rb b/ee/spec/services/ee/issues/close_service_spec.rb new file mode 100644 index 00000000000000..7f413ccf288af4 --- /dev/null +++ b/ee/spec/services/ee/issues/close_service_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Issues::CloseService, feature_category: :team_planning do + let_it_be(:project) { create(:project, :repository) } + + before do + project.add_maintainer(project_bot) + end + + describe '#execute' do + let(:service) { described_class.new(container: project, current_user: project_bot) } + + context 'for audit events' do + let_it_be(:project_bot) { create(:user, :project_bot, email: "bot@example.com") } + + include_examples 'audit event logging' do + let(:issue) { create(:issue, title: "My issue", project: project, author: project_bot) } + let(:operation) { service.execute(issue) } + let(:event_type) { 'issue_closed_by_project_bot' } + let(:fail_condition!) { expect(project_bot).to receive(:project_bot?).and_return(false) } + let(:attributes) do + { + author_id: project_bot.id, + entity_id: issue.project.id, + entity_type: 'Project', + details: { + author_name: project_bot.name, + target_id: issue.id, + target_type: 'Issue', + target_details: { + iid: issue.iid, + id: issue.id + }.to_s, + author_class: project_bot.class.name, + custom_message: "Closed issue #{issue.title}" + } + } + end + end + end + end +end diff --git a/ee/spec/services/ee/issues/reopen_service_spec.rb b/ee/spec/services/ee/issues/reopen_service_spec.rb new file mode 100644 index 00000000000000..5a70ae22f76abd --- /dev/null +++ b/ee/spec/services/ee/issues/reopen_service_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Issues::ReopenService, feature_category: :team_planning do + let_it_be(:project) { create(:project, :repository) } + + before do + project.add_maintainer(project_bot) + end + + describe '#execute' do + let(:service) { described_class.new(container: project, current_user: project_bot) } + + context 'for audit events' do + let_it_be(:project_bot) { create(:user, :project_bot, email: "bot@example.com") } + + include_examples 'audit event logging' do + let(:issue) { create(:issue, :closed, title: "My issue", project: project, author: project_bot) } + let(:operation) { service.execute(issue) } + let(:event_type) { 'issue_reopened_by_project_bot' } + let(:fail_condition!) { expect(project_bot).to receive(:project_bot?).and_return(false) } + let(:attributes) do + { + author_id: project_bot.id, + entity_id: issue.project.id, + entity_type: 'Project', + details: { + author_name: project_bot.name, + target_id: issue.id, + target_type: 'Issue', + target_details: { + iid: issue.iid, + id: issue.id + }.to_s, + author_class: project_bot.class.name, + custom_message: "Reopened issue #{issue.title}" + } + } + end + end + end + end +end diff --git a/ee/spec/services/epics/close_service_spec.rb b/ee/spec/services/epics/close_service_spec.rb index 5d1f74a36cf834..37d9d61d1db28e 100644 --- a/ee/spec/services/epics/close_service_spec.rb +++ b/ee/spec/services/epics/close_service_spec.rb @@ -69,6 +69,39 @@ subject.execute(epic) end + + context 'for audit events' do + let_it_be(:user) { create(:user, :project_bot, email: "bot@example.com") } + + before_all do + group.add_maintainer(user) + end + + include_examples 'audit event logging' do + let(:licensed_features_to_stub) { { epics: true } } + let(:operation) { subject.execute(epic) } + let(:event_type) { 'epic_closed_by_project_bot' } + let(:fail_condition!) { expect(user).to receive(:project_bot?).and_return(false) } + let(:attributes) do + { + author_id: user.id, + entity_id: epic.group.id, + entity_type: 'Group', + details: { + author_name: user.name, + target_id: epic.id, + target_type: 'Epic', + target_details: { + iid: epic.iid, + id: epic.id + }.to_s, + author_class: user.class.name, + custom_message: "Closed epic #{epic.title}" + } + } + end + end + end end context 'when trying to close a closed epic' do diff --git a/ee/spec/services/epics/reopen_service_spec.rb b/ee/spec/services/epics/reopen_service_spec.rb index 50fe51dfb5ef65..8bbb77d0c74a07 100644 --- a/ee/spec/services/epics/reopen_service_spec.rb +++ b/ee/spec/services/epics/reopen_service_spec.rb @@ -69,6 +69,39 @@ subject.execute(epic) end + + context 'for audit events' do + let_it_be(:user) { create(:user, :project_bot, email: "bot@example.com") } + + before_all do + group.add_maintainer(user) + end + + include_examples 'audit event logging' do + let(:licensed_features_to_stub) { { epics: true } } + let(:operation) { subject.execute(epic) } + let(:event_type) { 'epic_reopened_by_project_bot' } + let(:fail_condition!) { expect(user).to receive(:project_bot?).and_return(false) } + let(:attributes) do + { + author_id: user.id, + entity_id: epic.group.id, + entity_type: 'Group', + details: { + author_name: user.name, + target_id: epic.id, + target_type: 'Epic', + target_details: { + iid: epic.iid, + id: epic.id + }.to_s, + author_class: user.class.name, + custom_message: "Reopened epic #{epic.title}" + } + } + end + end + end end context 'when trying to reopen an opened epic' do diff --git a/ee/spec/support/shared_examples/services/audit_event_logging_shared_examples.rb b/ee/spec/support/shared_examples/services/audit_event_logging_shared_examples.rb index a44124b65a3f98..6b29b654337684 100644 --- a/ee/spec/support/shared_examples/services/audit_event_logging_shared_examples.rb +++ b/ee/spec/support/shared_examples/services/audit_event_logging_shared_examples.rb @@ -3,7 +3,11 @@ RSpec.shared_examples 'audit event logging' do context 'when licensed' do before do - stub_licensed_features(extended_audit_events: true) + if defined?(licensed_features_to_stub) + stub_licensed_features(licensed_features_to_stub.merge(extended_audit_events: true)) + else + stub_licensed_features(extended_audit_events: true) + end end context 'when operation succeeds' do diff --git a/ee/spec/workers/ee/new_issue_worker_spec.rb b/ee/spec/workers/ee/new_issue_worker_spec.rb new file mode 100644 index 00000000000000..ce289b004a5244 --- /dev/null +++ b/ee/spec/workers/ee/new_issue_worker_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe NewIssueWorker, feature_category: :team_planning do + let_it_be(:project) { create(:project, :repository) } + + before do + project.add_maintainer(project_bot) + end + + describe '#perform' do + let(:worker) { described_class.new } + + context 'for audit events' do + let_it_be(:project_bot) { create(:user, :project_bot, email: "bot@example.com") } + + include_examples 'audit event logging' do + let(:issue) { create(:issue, title: "My issue", project: project, author: project_bot) } + let(:operation) { worker.perform(issue.id, project_bot.id) } + let(:event_type) { 'issue_created_by_project_bot' } + let(:fail_condition!) { allow_any_instance_of(User).to receive(:project_bot?).and_return(false) } # rubocop:disable RSpec/AnyInstanceOf + let(:attributes) do + { + author_id: project_bot.id, + entity_id: issue.project.id, + entity_type: 'Project', + details: { + author_name: project_bot.name, + target_id: issue.id, + target_type: 'Issue', + target_details: { + iid: issue.iid, + id: issue.id + }.to_s, + author_class: project_bot.class.name, + custom_message: "Created issue #{issue.title}" + } + } + end + end + end + end +end diff --git a/ee/spec/workers/new_epic_worker_spec.rb b/ee/spec/workers/new_epic_worker_spec.rb index c687d450af85d5..5575bb903690e1 100644 --- a/ee/spec/workers/new_epic_worker_spec.rb +++ b/ee/spec/workers/new_epic_worker_spec.rb @@ -104,6 +104,35 @@ worker.perform(epic.id, user.id) end end + + context 'for audit events' do + let_it_be(:project_bot) { create(:user, :project_bot, email: "bot@example.com") } + + include_examples 'audit event logging' do + let(:epic) { create(:epic, author: project_bot) } + let(:operation) { worker.perform(epic.id, project_bot.id) } + let(:event_type) { 'epic_created_by_project_bot' } + let(:fail_condition!) { allow_any_instance_of(User).to receive(:project_bot?).and_return(false) } # rubocop:disable RSpec/AnyInstanceOf + let(:attributes) do + { + author_id: project_bot.id, + entity_id: epic.group.id, + entity_type: 'Group', + details: { + author_name: project_bot.name, + target_id: epic.id, + target_type: 'Epic', + target_details: { + iid: epic.iid, + id: epic.id + }.to_s, + author_class: project_bot.class.name, + custom_message: "Created epic #{epic.title}" + } + } + end + end + end end end end -- GitLab From fb65e6a20aea438dad00db7ea780622d5f71e836 Mon Sep 17 00:00:00 2001 From: huzaifaiftikhar1 Date: Thu, 25 May 2023 15:19:07 +0530 Subject: [PATCH 2/3] Move log_audit_event methods to base services --- app/services/issues/base_service.rb | 4 ++++ app/services/issues/close_service.rb | 10 +++++----- app/services/issues/reopen_service.rb | 11 ++++++----- ee/app/services/ee/issues/base_service.rb | 13 +++++++++++++ ee/app/services/ee/issues/close_service.rb | 16 ---------------- ee/app/services/ee/issues/reopen_service.rb | 16 ---------------- ee/app/services/epics/base_service.rb | 13 +++++++++++++ ee/app/services/epics/close_service.rb | 15 +-------------- ee/app/services/epics/reopen_service.rb | 18 ++++-------------- 9 files changed, 46 insertions(+), 70 deletions(-) diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb index efe42fb29d50a7..f982d66eb088ee 100644 --- a/app/services/issues/base_service.rb +++ b/app/services/issues/base_service.rb @@ -124,6 +124,10 @@ def execute_incident_hooks(issue, issue_data) def update_project_counter_caches?(issue) super || issue.confidential_changed? end + + def log_audit_event(issue, user, event_type, message) + # defined in EE + end end end diff --git a/app/services/issues/close_service.rb b/app/services/issues/close_service.rb index 96c76b09b10d50..f848a8db12a199 100644 --- a/app/services/issues/close_service.rb +++ b/app/services/issues/close_service.rb @@ -27,7 +27,11 @@ def close_issue(issue, closed_via: nil, notifications: true, system_note: true) if issue.close(current_user) event_service.close_issue(issue, current_user) create_note(issue, closed_via) if system_note - log_audit_event(issue, current_user) if current_user.project_bot? + + if current_user.project_bot? + log_audit_event(issue, current_user, "#{issue.issue_type}_closed_by_project_bot", + "Closed #{issue.issue_type.humanize(capitalize: false)} #{issue.title}") + end closed_via = _("commit %{commit_id}") % { commit_id: closed_via.id } if closed_via.is_a?(Commit) @@ -52,10 +56,6 @@ def close_issue(issue, closed_via: nil, notifications: true, system_note: true) private - def log_audit_event(issue, user) - # defined in EE - end - def can_close?(issue, skip_authorization: false) skip_authorization || can?(current_user, :update_issue, issue) || issue.is_a?(ExternalIssue) end diff --git a/app/services/issues/reopen_service.rb b/app/services/issues/reopen_service.rb index 4f6cb17079dbfc..d71ba4e3414d80 100644 --- a/app/services/issues/reopen_service.rb +++ b/app/services/issues/reopen_service.rb @@ -7,7 +7,12 @@ def execute(issue, skip_authorization: false) if issue.reopen event_service.reopen_issue(issue, current_user) - log_audit_event(issue, current_user) if current_user.project_bot? + + if current_user.project_bot? + log_audit_event(issue, current_user, "#{issue.issue_type}_reopened_by_project_bot", + "Reopened #{issue.issue_type.humanize(capitalize: false)} #{issue.title}") + end + create_note(issue, 'reopened') notification_service.async.reopen_issue(issue, current_user) perform_incident_management_actions(issue) @@ -40,10 +45,6 @@ def create_note(issue, state = issue.state) def create_timeline_event(issue) IncidentManagement::TimelineEvents::CreateService.reopen_incident(issue, current_user) end - - def log_audit_event(issue, user) - # defined in EE - end end end diff --git a/ee/app/services/ee/issues/base_service.rb b/ee/app/services/ee/issues/base_service.rb index 4cd141bb3e90da..b501620cca566b 100644 --- a/ee/app/services/ee/issues/base_service.rb +++ b/ee/app/services/ee/issues/base_service.rb @@ -166,6 +166,19 @@ def process_iteration_id params[:iteration] = iteration if iteration end + + def log_audit_event(issue, user, event_type, message) + audit_context = { + name: event_type, + author: user, + scope: issue.respond_to?(:group) ? issue.group : issue.project, + target: issue, + message: message, + target_details: { iid: issue.iid, id: issue.id } + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end end end end diff --git a/ee/app/services/ee/issues/close_service.rb b/ee/app/services/ee/issues/close_service.rb index 30870dac820c9d..bd75ab1ba2d452 100644 --- a/ee/app/services/ee/issues/close_service.rb +++ b/ee/app/services/ee/issues/close_service.rb @@ -10,22 +10,6 @@ def perform_incident_management_actions(issue) super update_issuable_sla(issue) end - - private - - override :log_audit_event - def log_audit_event(issue, user) - audit_context = { - name: "#{issue.issue_type}_closed_by_project_bot", - author: user, - scope: issue.respond_to?(:group) ? issue.group : issue.project, - target: issue, - message: "Closed #{issue.issue_type.humanize(capitalize: false)} #{issue.title}", - target_details: { iid: issue.iid, id: issue.id } - } - - ::Gitlab::Audit::Auditor.audit(audit_context) - end end end end diff --git a/ee/app/services/ee/issues/reopen_service.rb b/ee/app/services/ee/issues/reopen_service.rb index a5831e512a79d8..95ef2ee89ff81d 100644 --- a/ee/app/services/ee/issues/reopen_service.rb +++ b/ee/app/services/ee/issues/reopen_service.rb @@ -10,22 +10,6 @@ def perform_incident_management_actions(issue) super update_issuable_sla(issue) end - - private - - override :log_audit_event - def log_audit_event(issue, user) - audit_context = { - name: "#{issue.issue_type}_reopened_by_project_bot", - author: user, - scope: issue.respond_to?(:group) ? issue.group : issue.project, - target: issue, - message: "Reopened #{issue.issue_type.humanize(capitalize: false)} #{issue.title}", - target_details: { iid: issue.iid, id: issue.id } - } - - ::Gitlab::Audit::Auditor.audit(audit_context) - end end end end diff --git a/ee/app/services/epics/base_service.rb b/ee/app/services/epics/base_service.rb index 758f66985e1bef..061e2b5e2eb0a6 100644 --- a/ee/app/services/epics/base_service.rb +++ b/ee/app/services/epics/base_service.rb @@ -188,5 +188,18 @@ def create_parent_notes(epic, new_parent, old_parent) def parent_param_present? params.key?(:parent) || params.key?(:parent_id) end + + def log_audit_event(epic, event_type, message) + audit_context = { + name: event_type, + author: current_user, + scope: epic.group, + target: epic, + message: message, + target_details: { iid: epic.iid, id: epic.id } + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end end end diff --git a/ee/app/services/epics/close_service.rb b/ee/app/services/epics/close_service.rb index 23a4a560952739..31904063baed2f 100644 --- a/ee/app/services/epics/close_service.rb +++ b/ee/app/services/epics/close_service.rb @@ -20,21 +20,8 @@ def close_epic(epic) author: current_user, namespace: epic.group ) - log_audit_event(epic) if current_user.project_bot? + log_audit_event(epic, "epic_closed_by_project_bot", "Closed epic #{epic.title}") if current_user.project_bot? end end - - def log_audit_event(epic) - audit_context = { - name: "epic_closed_by_project_bot", - author: current_user, - scope: epic.group, - target: epic, - message: "Closed epic #{epic.title}", - target_details: { iid: epic.iid, id: epic.id } - } - - ::Gitlab::Audit::Auditor.audit(audit_context) - end end end diff --git a/ee/app/services/epics/reopen_service.rb b/ee/app/services/epics/reopen_service.rb index 27bacb6e41e656..8ab5cacfeba3d3 100644 --- a/ee/app/services/epics/reopen_service.rb +++ b/ee/app/services/epics/reopen_service.rb @@ -19,21 +19,11 @@ def reopen_epic(epic) author: current_user, namespace: epic.group ) - log_audit_event(epic) if current_user.project_bot? - end - end - def log_audit_event(epic) - audit_context = { - name: "epic_reopened_by_project_bot", - author: current_user, - scope: epic.group, - target: epic, - message: "Reopened epic #{epic.title}", - target_details: { iid: epic.iid, id: epic.id } - } - - ::Gitlab::Audit::Auditor.audit(audit_context) + if current_user.project_bot? + log_audit_event(epic, "epic_reopened_by_project_bot", "Reopened epic #{epic.title}") + end + end end end end -- GitLab From 70ca67dd99e550d5d7578e99a807253a2db4ea56 Mon Sep 17 00:00:00 2001 From: huzaifaiftikhar1 Date: Wed, 31 May 2023 14:19:57 +0530 Subject: [PATCH 3/3] Update the context name to make it easier to comprehend --- ee/spec/services/ee/issues/close_service_spec.rb | 2 +- ee/spec/services/ee/issues/reopen_service_spec.rb | 2 +- ee/spec/services/epics/close_service_spec.rb | 2 +- ee/spec/services/epics/reopen_service_spec.rb | 2 +- ee/spec/workers/ee/new_issue_worker_spec.rb | 2 +- ee/spec/workers/new_epic_worker_spec.rb | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ee/spec/services/ee/issues/close_service_spec.rb b/ee/spec/services/ee/issues/close_service_spec.rb index 7f413ccf288af4..75f8e45093a97e 100644 --- a/ee/spec/services/ee/issues/close_service_spec.rb +++ b/ee/spec/services/ee/issues/close_service_spec.rb @@ -12,7 +12,7 @@ describe '#execute' do let(:service) { described_class.new(container: project, current_user: project_bot) } - context 'for audit events' do + context 'when project bot it logs audit events' do let_it_be(:project_bot) { create(:user, :project_bot, email: "bot@example.com") } include_examples 'audit event logging' do diff --git a/ee/spec/services/ee/issues/reopen_service_spec.rb b/ee/spec/services/ee/issues/reopen_service_spec.rb index 5a70ae22f76abd..a2fa54224e0be5 100644 --- a/ee/spec/services/ee/issues/reopen_service_spec.rb +++ b/ee/spec/services/ee/issues/reopen_service_spec.rb @@ -12,7 +12,7 @@ describe '#execute' do let(:service) { described_class.new(container: project, current_user: project_bot) } - context 'for audit events' do + context 'when project bot it logs audit events' do let_it_be(:project_bot) { create(:user, :project_bot, email: "bot@example.com") } include_examples 'audit event logging' do diff --git a/ee/spec/services/epics/close_service_spec.rb b/ee/spec/services/epics/close_service_spec.rb index 37d9d61d1db28e..f57321efa6aabf 100644 --- a/ee/spec/services/epics/close_service_spec.rb +++ b/ee/spec/services/epics/close_service_spec.rb @@ -70,7 +70,7 @@ subject.execute(epic) end - context 'for audit events' do + context 'when project bot it logs audit events' do let_it_be(:user) { create(:user, :project_bot, email: "bot@example.com") } before_all do diff --git a/ee/spec/services/epics/reopen_service_spec.rb b/ee/spec/services/epics/reopen_service_spec.rb index 8bbb77d0c74a07..d791f8ead0a592 100644 --- a/ee/spec/services/epics/reopen_service_spec.rb +++ b/ee/spec/services/epics/reopen_service_spec.rb @@ -70,7 +70,7 @@ subject.execute(epic) end - context 'for audit events' do + context 'when project bot it logs audit events' do let_it_be(:user) { create(:user, :project_bot, email: "bot@example.com") } before_all do diff --git a/ee/spec/workers/ee/new_issue_worker_spec.rb b/ee/spec/workers/ee/new_issue_worker_spec.rb index ce289b004a5244..29d5ba4b000357 100644 --- a/ee/spec/workers/ee/new_issue_worker_spec.rb +++ b/ee/spec/workers/ee/new_issue_worker_spec.rb @@ -12,7 +12,7 @@ describe '#perform' do let(:worker) { described_class.new } - context 'for audit events' do + context 'when project bot it logs audit events' do let_it_be(:project_bot) { create(:user, :project_bot, email: "bot@example.com") } include_examples 'audit event logging' do diff --git a/ee/spec/workers/new_epic_worker_spec.rb b/ee/spec/workers/new_epic_worker_spec.rb index 5575bb903690e1..028d0cc56b80cd 100644 --- a/ee/spec/workers/new_epic_worker_spec.rb +++ b/ee/spec/workers/new_epic_worker_spec.rb @@ -105,7 +105,7 @@ end end - context 'for audit events' do + context 'when project bot it logs audit events' do let_it_be(:project_bot) { create(:user, :project_bot, email: "bot@example.com") } include_examples 'audit event logging' do -- GitLab