From c342e08b9b05e7fe603fc600da31b323d714e668 Mon Sep 17 00:00:00 2001 From: Michael Becker Date: Thu, 21 Jul 2022 06:59:13 -0400 Subject: [PATCH] New Audit Event for custom HTTP header changes We added custom HTTP headers for streaming audit events in [MR 88063][0] This change adds new audit events that will be emitted whenever one of these custom headers are created, updated, or destroyed. The header `key` will be included in the audit event payload, but the `value` **will not** be included This commit is the part 2 of this [refactor MR][1] This implements [issue 366350][2] [0]:https://gitlab.com/gitlab-org/gitlab/-/merge_requests/88063 [1]:https://gitlab.com/gitlab-org/gitlab/-/merge_requests/91992 [2]:https://gitlab.com/gitlab-org/gitlab/-/issues/366350 Changelog: added MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/92068 EE: true --- doc/administration/audit_events.md | 1 + .../audit_events/streaming/headers/create.rb | 3 +- .../audit_events/streaming/headers/destroy.rb | 3 +- .../audit_events/streaming/headers/update.rb | 3 +- .../audit_events/streaming/headers/base.rb | 13 +++++ .../streaming/headers/create_service.rb | 7 +++ .../streaming/headers/destroy_service.rb | 5 ++ .../streaming/headers/update_service.rb | 11 ++++ .../streaming/headers/create_service_spec.rb | 32 ++++++++++- .../streaming/headers/destroy_service_spec.rb | 41 +++++++++++++- .../streaming/headers/update_service_spec.rb | 56 ++++++++++++++++++- 11 files changed, 166 insertions(+), 9 deletions(-) diff --git a/doc/administration/audit_events.md b/doc/administration/audit_events.md index a329adbed22efd..64e1d6b7d7d1c9 100644 --- a/doc/administration/audit_events.md +++ b/doc/administration/audit_events.md @@ -123,6 +123,7 @@ From there, you can see the following actions: - Prevent editing approval rules in projects and merge requests. - Require user password to approve. - Remove all approvals when commits are added to the source branch. +- Changes to streaming audit destination custom HTTP headers. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/366350) in GitLab 15.3. Group events can also be accessed via the [Group Audit Events API](../api/audit_events.md#group-audit-events) diff --git a/ee/app/graphql/mutations/audit_events/streaming/headers/create.rb b/ee/app/graphql/mutations/audit_events/streaming/headers/create.rb index 82a570d3fa661e..5d7f6a944bdae9 100644 --- a/ee/app/graphql/mutations/audit_events/streaming/headers/create.rb +++ b/ee/app/graphql/mutations/audit_events/streaming/headers/create.rb @@ -27,7 +27,8 @@ class Create < BaseMutation def resolve(destination_id:, key:, value:) response = ::AuditEvents::Streaming::Headers::CreateService.new( destination: authorized_find!(destination_id), - params: { key: key, value: value } + params: { key: key, value: value }, + current_user: current_user ).execute if response.success? diff --git a/ee/app/graphql/mutations/audit_events/streaming/headers/destroy.rb b/ee/app/graphql/mutations/audit_events/streaming/headers/destroy.rb index 682f7205b46bca..8a47bf09b631f6 100644 --- a/ee/app/graphql/mutations/audit_events/streaming/headers/destroy.rb +++ b/ee/app/graphql/mutations/audit_events/streaming/headers/destroy.rb @@ -17,7 +17,8 @@ def resolve(header_id:) response = ::AuditEvents::Streaming::Headers::DestroyService.new( destination: header.external_audit_event_destination, - params: { header: header } + params: { header: header }, + current_user: current_user ).execute if response.success? diff --git a/ee/app/graphql/mutations/audit_events/streaming/headers/update.rb b/ee/app/graphql/mutations/audit_events/streaming/headers/update.rb index 110f6b6dba93df..1f1d79fcd2e393 100644 --- a/ee/app/graphql/mutations/audit_events/streaming/headers/update.rb +++ b/ee/app/graphql/mutations/audit_events/streaming/headers/update.rb @@ -29,7 +29,8 @@ def resolve(header_id:, key:, value:) response = ::AuditEvents::Streaming::Headers::UpdateService.new( destination: header.external_audit_event_destination, - params: { header: header, key: key, value: value } + params: { header: header, key: key, value: value }, + current_user: current_user ).execute if response.success? diff --git a/ee/app/services/audit_events/streaming/headers/base.rb b/ee/app/services/audit_events/streaming/headers/base.rb index dd280575b518a1..0b38a8f1205729 100644 --- a/ee/app/services/audit_events/streaming/headers/base.rb +++ b/ee/app/services/audit_events/streaming/headers/base.rb @@ -24,6 +24,19 @@ def execute def destination_error ServiceResponse.error(message: "missing destination param") end + + def audit(action:, header:, message:, author: current_user) + audit_context = { + name: "audit_events_streaming_headers_#{action}", + author: author, + scope: group, + target: header, + message: message, + stream_only: false + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end end end end diff --git a/ee/app/services/audit_events/streaming/headers/create_service.rb b/ee/app/services/audit_events/streaming/headers/create_service.rb index 6c8acb098c9a12..832c2099285860 100644 --- a/ee/app/services/audit_events/streaming/headers/create_service.rb +++ b/ee/app/services/audit_events/streaming/headers/create_service.rb @@ -9,11 +9,18 @@ def execute header = destination.headers.new(key: params[:key], value: params[:value]) if header.save + audit(action: :create, header: header, message: audit_message(header.key)) ServiceResponse.success(payload: { header: header, errors: [] }) else ServiceResponse.error(message: Array(header.errors)) end end + + private + + def audit_message(key) + "Created custom HTTP header with key #{key}." + end end end end diff --git a/ee/app/services/audit_events/streaming/headers/destroy_service.rb b/ee/app/services/audit_events/streaming/headers/destroy_service.rb index 42e4e7a249701b..8d664d61d5af1c 100644 --- a/ee/app/services/audit_events/streaming/headers/destroy_service.rb +++ b/ee/app/services/audit_events/streaming/headers/destroy_service.rb @@ -10,6 +10,7 @@ def execute return header_error if header.blank? if header.destroy + audit(action: :destroy, header: header, message: audit_message(header.key)) ServiceResponse.success else ServiceResponse.error(message: Array(header.errors)) @@ -21,6 +22,10 @@ def execute def header_error ServiceResponse.error(message: "missing header param") end + + def audit_message(key) + "Destroyed a custom HTTP header with key #{key}." + end end end end diff --git a/ee/app/services/audit_events/streaming/headers/update_service.rb b/ee/app/services/audit_events/streaming/headers/update_service.rb index cd10bb3f4cdf5f..c8bb539c5021a5 100644 --- a/ee/app/services/audit_events/streaming/headers/update_service.rb +++ b/ee/app/services/audit_events/streaming/headers/update_service.rb @@ -9,7 +9,10 @@ def execute header = params[:header] return header_error if header.blank? + audit_message = audit_message(header.key, params[:key]) + if header.update(key: params[:key], value: params[:value]) + audit(action: :update, header: header, message: audit_message) ServiceResponse.success(payload: { header: header, errors: [] }) else ServiceResponse.error(message: Array(header.errors)) @@ -21,6 +24,14 @@ def execute def header_error ServiceResponse.error(message: "missing header param") end + + def audit_message(old_key, new_key) + if old_key == new_key + "Updated a custom HTTP header with key #{new_key} to have a new value." + else + "Updated a custom HTTP header from key #{old_key} to have a key #{new_key}." + end + end end end end diff --git a/ee/spec/services/audit_events/streaming/headers/create_service_spec.rb b/ee/spec/services/audit_events/streaming/headers/create_service_spec.rb index 78663f2f97da3c..4d06030170c408 100644 --- a/ee/spec/services/audit_events/streaming/headers/create_service_spec.rb +++ b/ee/spec/services/audit_events/streaming/headers/create_service_spec.rb @@ -3,11 +3,15 @@ require 'spec_helper' RSpec.describe AuditEvents::Streaming::Headers::CreateService do - let(:destination) { create(:external_audit_event_destination) } + let_it_be(:user) { create(:user) } + let_it_be(:destination) { create(:external_audit_event_destination) } + let_it_be(:event_type) { "audit_events_streaming_headers_create" } + let(:params) { {} } subject(:service) do described_class.new( + current_user: user, destination: destination, params: params ) @@ -33,6 +37,32 @@ expect(response.payload[:header].key).to eq 'a_key' expect(response.payload[:header].value).to eq 'a_value' end + + it 'sends the audit streaming event' do + audit_context = { + name: 'audit_events_streaming_headers_create', + stream_only: false, + author: user, + scope: destination.group, + message: "Created custom HTTP header with key a_key." + } + + expect(::Gitlab::Audit::Auditor).to receive(:audit) + .with(hash_including(audit_context)).and_call_original + expect { response }.to change { AuditEvent.count }.from(0).to(1) + end + + context "with license feature external_audit_events" do + before do + stub_licensed_features(external_audit_events: true) + end + + it 'sends correct event type in audit event stream' do + expect(AuditEvents::AuditEventStreamingWorker).to receive(:perform_async).with(event_type, nil, anything) + + response + end + end end end end diff --git a/ee/spec/services/audit_events/streaming/headers/destroy_service_spec.rb b/ee/spec/services/audit_events/streaming/headers/destroy_service_spec.rb index d3d0661bf7950b..d9f38bc75e1b69 100644 --- a/ee/spec/services/audit_events/streaming/headers/destroy_service_spec.rb +++ b/ee/spec/services/audit_events/streaming/headers/destroy_service_spec.rb @@ -3,11 +3,20 @@ require 'spec_helper' RSpec.describe AuditEvents::Streaming::Headers::DestroyService do - let(:header) { create(:audit_events_streaming_header) } + let_it_be(:user) { create(:user) } + let_it_be(:header) { create(:audit_events_streaming_header) } + let_it_be(:event_type) { "audit_events_streaming_headers_destroy" } + let(:destination) { header.external_audit_event_destination } - let(:params) { { destination: destination, header: header } } + let(:params) { { destination: destination, header: header } } - subject(:service) { described_class.new(destination: destination, params: params ) } + subject(:service) do + described_class.new( + destination: destination, + current_user: user, + params: params + ) + end describe '#execute' do context 'when no header is provided' do @@ -32,6 +41,32 @@ expect { response }.to change { destination.headers.count }.by(-1) expect(response).to be_success end + + it 'sends the audit streaming event' do + audit_context = { + name: 'audit_events_streaming_headers_destroy', + stream_only: false, + author: user, + scope: destination.group, + target: header, + message: "Destroyed a custom HTTP header with key #{header.key}." + } + + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(audit_context).and_call_original + expect { response }.to change { AuditEvent.count }.from(0).to(1) + end + + context "with license feature external_audit_events" do + before do + stub_licensed_features(external_audit_events: true) + end + + it 'sends correct event type in audit event stream' do + expect(AuditEvents::AuditEventStreamingWorker).to receive(:perform_async).with(event_type, nil, anything) + + response + end + end end end end diff --git a/ee/spec/services/audit_events/streaming/headers/update_service_spec.rb b/ee/spec/services/audit_events/streaming/headers/update_service_spec.rb index d28a4f682e1fad..f40e312317caf5 100644 --- a/ee/spec/services/audit_events/streaming/headers/update_service_spec.rb +++ b/ee/spec/services/audit_events/streaming/headers/update_service_spec.rb @@ -3,8 +3,10 @@ require 'spec_helper' RSpec.describe AuditEvents::Streaming::Headers::UpdateService do - let_it_be(:header) { create(:audit_events_streaming_header, key: 'old', value: 'old') } + let_it_be(:user) { create(:user) } + let_it_be(:event_type) { "audit_events_streaming_headers_update" } + let(:header) { create(:audit_events_streaming_header, key: 'old', value: 'old') } let(:destination) { header.external_audit_event_destination } let(:params) do { @@ -14,7 +16,13 @@ } end - subject(:service) { described_class.new(destination: destination, params: params) } + subject(:service) do + described_class.new( + current_user: user, + destination: destination, + params: params + ) + end describe '#execute' do subject(:response) { service.execute } @@ -39,6 +47,50 @@ expect(header.reload.key).to eq 'new' expect(header.value).to eq 'new' end + + it 'sends the audit streaming event' do + audit_context = { + name: 'audit_events_streaming_headers_update', + stream_only: false, + author: user, + scope: destination.group, + target: header, + message: "Updated a custom HTTP header from key old to have a key new." + } + + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(audit_context).and_call_original + expect { response }.to change { AuditEvent.count }.from(0).to(1) + end + + context "with license feature external_audit_events" do + before do + stub_licensed_features(external_audit_events: true) + end + + it 'sends correct event type in audit event stream' do + expect(AuditEvents::AuditEventStreamingWorker).to receive(:perform_async).with(event_type, nil, anything) + + response + end + end + + context 'when only the header value is updated' do + let(:params) { super().merge(key: 'old') } + + it 'has a audit message reflecting just the value was changed' do + audit_context = { + name: 'audit_events_streaming_headers_update', + stream_only: false, + author: user, + scope: destination.group, + target: header, + message: "Updated a custom HTTP header with key old to have a new value." + } + + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(audit_context) + response + end + end end end end -- GitLab