From ba2b4ce7fa89a1f14d77b5ce35123f02afa2f1f2 Mon Sep 17 00:00:00 2001 From: Anton Smith Date: Fri, 24 May 2024 09:57:52 +1200 Subject: [PATCH 1/5] Add audit event for web hook creation When a user creates a project/group/system hook, this will now generate an audit event via the appropriate scope. Changelog: added EE: true --- app/services/web_hooks/create_service.rb | 8 +++- config/audit_events/types/webhook_created.yml | 10 +++++ doc/user/compliance/audit_event_types.md | 1 + .../services/ee/web_hooks/create_service.rb | 31 +++++++++++++++ .../ee/web_hooks/create_service_spec.rb | 39 +++++++++++++++++++ 5 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 config/audit_events/types/webhook_created.yml create mode 100644 ee/app/services/ee/web_hooks/create_service.rb create mode 100644 ee/spec/services/ee/web_hooks/create_service_spec.rb diff --git a/app/services/web_hooks/create_service.rb b/app/services/web_hooks/create_service.rb index fe2d28ae145ae3..c187590fd61c64 100644 --- a/app/services/web_hooks/create_service.rb +++ b/app/services/web_hooks/create_service.rb @@ -12,7 +12,7 @@ def execute(hook_params, relation) hook = relation.new(hook_params) if hook.save - success({ hook: hook, async: false }) + after_create(hook) else return error("Invalid url given", 422) if hook.errors[:url].present? return error("Invalid branch filter given", 422) if hook.errors[:push_events_branch_filter].present? @@ -23,6 +23,12 @@ def execute(hook_params, relation) private + def after_create(hook) + success({ hook: hook, async: false }) + end + attr_reader :current_user end end + +WebHooks::CreateService.prepend_mod_with('WebHooks::CreateService') diff --git a/config/audit_events/types/webhook_created.yml b/config/audit_events/types/webhook_created.yml new file mode 100644 index 00000000000000..5261eb34705baa --- /dev/null +++ b/config/audit_events/types/webhook_created.yml @@ -0,0 +1,10 @@ +--- +name: webhook_created +description: Event triggered when a webhook is created. +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/8068 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/issues/8068 +feature_category: webhooks +milestone: '17.1' +saved_to_database: true +streamed: true +scope: [Project, Group, Instance] \ No newline at end of file diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index b5483ceaf877c1..0c4d31913cc9a8 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -542,4 +542,5 @@ Audit event types belong to the following product categories. | Name | Description | Saved to database | Streamed | Introduced in | Scope | |:------------|:------------|:------------------|:---------|:--------------|:--------------| +| [`webhook_created`](https://gitlab.com/gitlab-org/gitlab/-/issues/8068) | Event triggered when a webhook is created.| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.1](https://gitlab.com/gitlab-org/gitlab/-/issues/8068) | Project, Group, Instance | | [`webhook_destroyed`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/102342) | Event triggered when a webhook is destroyed.| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.0](https://gitlab.com/gitlab-org/gitlab/-/issues/458817) | Project, Group, Instance | diff --git a/ee/app/services/ee/web_hooks/create_service.rb b/ee/app/services/ee/web_hooks/create_service.rb new file mode 100644 index 00000000000000..2fb97c6ccc8cb3 --- /dev/null +++ b/ee/app/services/ee/web_hooks/create_service.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module EE + module WebHooks + module CreateService + extend ::Gitlab::Utils::Override + + private + + override :after_create + def after_create(hook) + result = super + log_audit_event(hook) + result + end + + def log_audit_event(hook) + audit_context = { + name: "webhook_created", + author: current_user, + scope: hook.parent || current_user, + target: hook, + message: "Created #{hook.model_name.human.downcase}", + target_details: { id: hook.id, url: hook.url } + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end + end + end +end diff --git a/ee/spec/services/ee/web_hooks/create_service_spec.rb b/ee/spec/services/ee/web_hooks/create_service_spec.rb new file mode 100644 index 00000000000000..2134baf6bc7091 --- /dev/null +++ b/ee/spec/services/ee/web_hooks/create_service_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe WebHooks::CreateService, :sidekiq_inline, feature_category: :webhooks do + let_it_be(:current_user) { create(:user) } + + describe '#execute' do + # Testing with a project hook only - for permission tests, see policy specs. + let_it_be(:project) { create(:project) } + let_it_be(:relation) { ProjectHook.none } + let(:hook_params) { { url: 'https://example.com/hook', project_id: project.id } } + + subject(:webhook_created) { described_class.new(current_user) } + + context 'when creating a project hook succeeds' do + it 'creates an audit event', :aggregate_failures do + webhook_created.execute(hook_params, relation) + + expect(AuditEvent.last).to have_attributes( + author_name: current_user.name, + author_id: current_user.id, + target_type: "ProjectHook", + details: include(custom_message: "Created project hook") + ) + end + end + + context 'when creating a project hook fails' do + it 'does not create an audit event' do + hook_params[:url] = 'invalid_url' + + response = webhook_created.execute(hook_params, relation) + + expect { response }.not_to change { AuditEvent.count } + end + end + end +end -- GitLab From 5a92aafbe5bb14f63e528459593011a37bd7d3af Mon Sep 17 00:00:00 2001 From: Anton Smith Date: Fri, 24 May 2024 10:10:20 +1200 Subject: [PATCH 2/5] Add correct MR to audit event types --- config/audit_events/types/webhook_created.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/audit_events/types/webhook_created.yml b/config/audit_events/types/webhook_created.yml index 5261eb34705baa..17612416a318a1 100644 --- a/config/audit_events/types/webhook_created.yml +++ b/config/audit_events/types/webhook_created.yml @@ -2,7 +2,7 @@ name: webhook_created description: Event triggered when a webhook is created. introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/8068 -introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/issues/8068 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/154046 feature_category: webhooks milestone: '17.1' saved_to_database: true -- GitLab From 314468af64c72b7bcc152a5ff6b7e8c62a8d2764 Mon Sep 17 00:00:00 2001 From: Anton Smith Date: Mon, 27 May 2024 10:33:19 +1200 Subject: [PATCH 3/5] Also check the target_details --- ee/spec/services/ee/web_hooks/create_service_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/ee/spec/services/ee/web_hooks/create_service_spec.rb b/ee/spec/services/ee/web_hooks/create_service_spec.rb index 2134baf6bc7091..45bcb9f3381ea8 100644 --- a/ee/spec/services/ee/web_hooks/create_service_spec.rb +++ b/ee/spec/services/ee/web_hooks/create_service_spec.rb @@ -21,6 +21,7 @@ author_name: current_user.name, author_id: current_user.id, target_type: "ProjectHook", + target_details: "{:id=>#{ProjectHook.last.id}, :url=>\"#{hook_params[:url]}\"}", details: include(custom_message: "Created project hook") ) end -- GitLab From 044fe6af77b13607b7ef86b6e3c8e6f9bc8a0865 Mon Sep 17 00:00:00 2001 From: Anton Smith Date: Tue, 28 May 2024 12:31:49 +1200 Subject: [PATCH 4/5] Tweak target_details to only return the hook ID --- ee/app/services/ee/web_hooks/create_service.rb | 2 +- ee/spec/services/ee/web_hooks/create_service_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/app/services/ee/web_hooks/create_service.rb b/ee/app/services/ee/web_hooks/create_service.rb index 2fb97c6ccc8cb3..1799373bc0a371 100644 --- a/ee/app/services/ee/web_hooks/create_service.rb +++ b/ee/app/services/ee/web_hooks/create_service.rb @@ -21,7 +21,7 @@ def log_audit_event(hook) scope: hook.parent || current_user, target: hook, message: "Created #{hook.model_name.human.downcase}", - target_details: { id: hook.id, url: hook.url } + target_details: "Hook #{hook.id}" } ::Gitlab::Audit::Auditor.audit(audit_context) diff --git a/ee/spec/services/ee/web_hooks/create_service_spec.rb b/ee/spec/services/ee/web_hooks/create_service_spec.rb index 45bcb9f3381ea8..b1505f0c0b7544 100644 --- a/ee/spec/services/ee/web_hooks/create_service_spec.rb +++ b/ee/spec/services/ee/web_hooks/create_service_spec.rb @@ -21,7 +21,7 @@ author_name: current_user.name, author_id: current_user.id, target_type: "ProjectHook", - target_details: "{:id=>#{ProjectHook.last.id}, :url=>\"#{hook_params[:url]}\"}", + target_details: "Hook #{ProjectHook.last.id}", details: include(custom_message: "Created project hook") ) end -- GitLab From 97b94367957bc394c01d93a3a7ffc1e0f67f6542 Mon Sep 17 00:00:00 2001 From: Anton Smith Date: Thu, 30 May 2024 19:01:37 +1200 Subject: [PATCH 5/5] Recompile the docs --- doc/user/compliance/audit_event_types.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index 0c4d31913cc9a8..d42185b7c18bd0 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -542,5 +542,5 @@ Audit event types belong to the following product categories. | Name | Description | Saved to database | Streamed | Introduced in | Scope | |:------------|:------------|:------------------|:---------|:--------------|:--------------| -| [`webhook_created`](https://gitlab.com/gitlab-org/gitlab/-/issues/8068) | Event triggered when a webhook is created.| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.1](https://gitlab.com/gitlab-org/gitlab/-/issues/8068) | Project, Group, Instance | +| [`webhook_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/154046) | Event triggered when a webhook is created.| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.1](https://gitlab.com/gitlab-org/gitlab/-/issues/8068) | Project, Group, Instance | | [`webhook_destroyed`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/102342) | Event triggered when a webhook is destroyed.| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.0](https://gitlab.com/gitlab-org/gitlab/-/issues/458817) | Project, Group, Instance | -- GitLab