diff --git a/doc/administration/audit_event_streaming/audit_event_types.md b/doc/administration/audit_event_streaming/audit_event_types.md index 1a2bf0df2d9a8010b1320598a66081201fe5706d..0139721df7a61e94668b7176073c5aace7b9f4c1 100644 --- a/doc/administration/audit_event_streaming/audit_event_types.md +++ b/doc/administration/audit_event_streaming/audit_event_types.md @@ -27,6 +27,9 @@ Audit event types are used to [filter streamed audit events](index.md#update-eve | [`audit_events_streaming_headers_create`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/92068) | Triggered when a streaming header for audit events is created | **{check-circle}** Yes | **{check-circle}** Yes | `audit_events` | [15.3](https://gitlab.com/gitlab-org/gitlab/-/issues/366350) | | [`audit_events_streaming_headers_destroy`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/92068) | Triggered when a streaming header for audit events is deleted | **{check-circle}** Yes | **{check-circle}** Yes | `audit_events` | [15.3](https://gitlab.com/gitlab-org/gitlab/-/issues/366350) | | [`audit_events_streaming_headers_update`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/92068) | Triggered when a streaming header for audit events is updated | **{check-circle}** Yes | **{check-circle}** Yes | `compliance_management` | [15.3](https://gitlab.com/gitlab-org/gitlab/-/issues/366350) | +| [`audit_events_streaming_instance_headers_create`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/125870) | Triggered when a streaming header for instance level external audit event destination is created | **{check-circle}** Yes | **{check-circle}** Yes | `audit_events` | [16.3](https://gitlab.com/gitlab-org/gitlab/-/issues/417433) | +| [`audit_events_streaming_instance_headers_destroy`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/127228) | Triggered when a streaming header for instance level external audit event destination is deleted | **{check-circle}** Yes | **{check-circle}** Yes | `audit_events` | [16.3](https://gitlab.com/gitlab-org/gitlab/-/issues/417433) | +| [`audit_events_streaming_instance_headers_update`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/127228) | Triggered when a streaming header for instance level external audit event destination is updated | **{check-circle}** Yes | **{check-circle}** Yes | `audit_events` | [16.3](https://gitlab.com/gitlab-org/gitlab/-/issues/417433) | | [`authenticated_with_group_saml`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/28575) | Triggered after successfully signing in with SAML authentication | **{check-circle}** Yes | **{check-circle}** Yes | `user_management` | [12.10](https://gitlab.com/gitlab-org/gitlab/-/issues/35710) | | [`ban_user`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/116103) | Event triggered on user ban action | **{check-circle}** Yes | **{check-circle}** Yes | `user_management` | [15.11](https://gitlab.com/gitlab-org/gitlab/-/issues/377620) | | [`change_membership_state`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/87924) | Event triggered on a users membership is updated | **{check-circle}** Yes | **{check-circle}** Yes | `user_management` | [15.1](https://gitlab.com/gitlab-org/gitlab/-/issues/362200) | diff --git a/ee/app/graphql/mutations/audit_events/streaming/instance_headers/destroy.rb b/ee/app/graphql/mutations/audit_events/streaming/instance_headers/destroy.rb index 1490eaa52e5bc1b123ae5ea9a92a5e7f404e6887..9210001ea3b04e1389b2572359ccd21410b29e2c 100644 --- a/ee/app/graphql/mutations/audit_events/streaming/instance_headers/destroy.rb +++ b/ee/app/graphql/mutations/audit_events/streaming/instance_headers/destroy.rb @@ -15,7 +15,8 @@ def resolve(header_id:) header = authorized_find!(id: header_id) response = ::AuditEvents::Streaming::InstanceHeaders::DestroyService.new( - params: { header: header } + params: { header: header }, + current_user: current_user ).execute if response.success? diff --git a/ee/app/graphql/mutations/audit_events/streaming/instance_headers/update.rb b/ee/app/graphql/mutations/audit_events/streaming/instance_headers/update.rb index 008c2365737c22f6577833eecbbdc97bbe326de0..3675cd84db6f75a2388c6025e2417ea4cada915b 100644 --- a/ee/app/graphql/mutations/audit_events/streaming/instance_headers/update.rb +++ b/ee/app/graphql/mutations/audit_events/streaming/instance_headers/update.rb @@ -27,7 +27,8 @@ def resolve(header_id:, key:, value:) header = find_header(header_id) response = ::AuditEvents::Streaming::InstanceHeaders::UpdateService.new( - 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/destroy_service.rb b/ee/app/services/audit_events/streaming/headers/destroy_service.rb index 64ca50b188040fbce4094307a56b1b80bfd191f3..7301a5aa25bc0b4678de070a12c7ae5c2abdc09d 100644 --- a/ee/app/services/audit_events/streaming/headers/destroy_service.rb +++ b/ee/app/services/audit_events/streaming/headers/destroy_service.rb @@ -9,11 +9,7 @@ def execute header = params[:header] return header_error if header.blank? - success, response = destroy_header(params[:header]) - - audit(action: :destroy, header: header, message: audit_message(header.key)) if success - - response + destroy_header(params[:header]) end private @@ -21,10 +17,6 @@ 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 b6bba694ef93660b72eb000c6c3bb30ef99cb33e..e157de9c7dadb04642592e7e783b45ee1c32a574 100644 --- a/ee/app/services/audit_events/streaming/headers/update_service.rb +++ b/ee/app/services/audit_events/streaming/headers/update_service.rb @@ -9,12 +9,7 @@ def execute header = params[:header] return header_error if header.blank? - audit_message = audit_message(header.key, params[:key]) - - success, response = update_header(header, params[:key], params[:value]) - - audit(action: :update, header: header, message: audit_message) if success - response + update_header(header, params[:key], params[:value]) end private @@ -22,14 +17,6 @@ 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/app/services/audit_events/streaming/headers_operations.rb b/ee/app/services/audit_events/streaming/headers_operations.rb index 10df3bcb34cfd9c0f47d938d2aa94fbb8630aeab..ba1a4454318571adf2f76905095b2d3453cb0409 100644 --- a/ee/app/services/audit_events/streaming/headers_operations.rb +++ b/ee/app/services/audit_events/streaming/headers_operations.rb @@ -18,16 +18,39 @@ def create_header(destination, key, value) def update_header(header, key, value) if header.update(key: key, value: value) - [true, ServiceResponse.success(payload: { header: header, errors: [] })] + log_update_audit_event(header) + ServiceResponse.success(payload: { header: header, errors: [] }) else - [false, ServiceResponse.error(message: Array(header.errors))] + ServiceResponse.error(message: Array(header.errors)) end end def destroy_header(header) - return true, ServiceResponse.success if header.destroy + if header.destroy + audit_message = "Destroyed a custom HTTP header with key #{header.key}." + audit(action: :destroy, header: header, message: audit_message) + + ServiceResponse.success + else + ServiceResponse.error(message: Array(header.errors)) + end + end - [false, ServiceResponse.error(message: Array(header.errors))] + private + + def log_update_audit_event(header) + return if header.previous_changes.except(:updated_at).empty? + + audit(action: :update, header: header, message: update_audit_message(header)) + end + + def update_audit_message(header) + changes = header.previous_changes.except(:updated_at) + if changes.key?(:key) + "Updated a custom HTTP header from key #{changes[:key].first} to have a key #{changes[:key].last}." + else + "Updated a custom HTTP header with key #{header.key} to have a new value." + end end end end diff --git a/ee/app/services/audit_events/streaming/instance_headers/destroy_service.rb b/ee/app/services/audit_events/streaming/instance_headers/destroy_service.rb index a53d3361e94b39eaafb278cca9fba2d733579fe9..960971879c4124716d40297185515871b50fa786 100644 --- a/ee/app/services/audit_events/streaming/instance_headers/destroy_service.rb +++ b/ee/app/services/audit_events/streaming/instance_headers/destroy_service.rb @@ -5,8 +5,7 @@ module Streaming module InstanceHeaders class DestroyService < BaseService def execute - _, response = destroy_header(params[:header]) - response + destroy_header(params[:header]) end end end diff --git a/ee/app/services/audit_events/streaming/instance_headers/update_service.rb b/ee/app/services/audit_events/streaming/instance_headers/update_service.rb index d21cbc1fd5c75e142e97d4c0242c2e5503423982..f4d2a1e8ce217c5c4bb3c804f22f66f64d59cd1c 100644 --- a/ee/app/services/audit_events/streaming/instance_headers/update_service.rb +++ b/ee/app/services/audit_events/streaming/instance_headers/update_service.rb @@ -5,8 +5,7 @@ module Streaming module InstanceHeaders class UpdateService < BaseService def execute - _, response = update_header(params[:header], params[:key], params[:value]) - response + update_header(params[:header], params[:key], params[:value]) end end end diff --git a/ee/config/audit_events/types/audit_events_streaming_instance_headers_create.yml b/ee/config/audit_events/types/audit_events_streaming_instance_headers_create.yml new file mode 100644 index 0000000000000000000000000000000000000000..be07477ac858b4d9774a0de5cab181e5aa1877d5 --- /dev/null +++ b/ee/config/audit_events/types/audit_events_streaming_instance_headers_create.yml @@ -0,0 +1,9 @@ +--- +name: audit_events_streaming_instance_headers_create +description: Triggered when a streaming header for instance level external audit event destination is created +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/417433 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/125870 +milestone: '16.3' +feature_category: audit_events +saved_to_database: true +streamed: true diff --git a/ee/config/audit_events/types/audit_events_streaming_instance_headers_destroy.yml b/ee/config/audit_events/types/audit_events_streaming_instance_headers_destroy.yml new file mode 100644 index 0000000000000000000000000000000000000000..48c25d4db3d96f6557b3c06934f549b9b8fae9c2 --- /dev/null +++ b/ee/config/audit_events/types/audit_events_streaming_instance_headers_destroy.yml @@ -0,0 +1,9 @@ +--- +name: audit_events_streaming_instance_headers_destroy +description: Triggered when a streaming header for instance level external audit event destination is deleted +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/417433 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/127228 +milestone: '16.3' +feature_category: audit_events +saved_to_database: true +streamed: true diff --git a/ee/config/audit_events/types/audit_events_streaming_instance_headers_update.yml b/ee/config/audit_events/types/audit_events_streaming_instance_headers_update.yml new file mode 100644 index 0000000000000000000000000000000000000000..21c3b0aadf93394ef928e266fc7e514d8eda6144 --- /dev/null +++ b/ee/config/audit_events/types/audit_events_streaming_instance_headers_update.yml @@ -0,0 +1,9 @@ +--- +name: audit_events_streaming_instance_headers_update +description: Triggered when a streaming header for instance level external audit event destination is updated +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/417433 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/127228 +milestone: '16.3' +feature_category: audit_events +saved_to_database: true +streamed: true 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 954b29fc3346e41bd9f60062ac72c023433d076a..922c840ec4d67bdd74ad1b69bc02dfc479af1cb3 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 @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe AuditEvents::Streaming::Headers::DestroyService do +RSpec.describe AuditEvents::Streaming::Headers::DestroyService, feature_category: :audit_events do 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" } @@ -36,34 +36,9 @@ end end - it_behaves_like 'header deletion' - - context 'when the header is destroyed successfully' do - 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 + it_behaves_like 'header deletion' do + let(:audit_scope) { destination.group } + let(:extra_audit_context) { { stream_only: false } } 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 5e54087578976d72d189b91e17ced071136c0a89..fa10543b2fc7376a14a272b6043c3a742de6a642 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 @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe AuditEvents::Streaming::Headers::UpdateService do +RSpec.describe AuditEvents::Streaming::Headers::UpdateService, feature_category: :audit_events do let_it_be(:user) { create(:user) } let_it_be(:event_type) { "audit_events_streaming_headers_update" } @@ -41,52 +41,9 @@ end end - it_behaves_like 'header updation' - - context 'when the header is updated successfully' do - 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 + it_behaves_like 'header updation' do + let(:audit_scope) { destination.group } + let(:extra_audit_context) { { stream_only: false } } end end end diff --git a/ee/spec/services/audit_events/streaming/instance_headers/destroy_service_spec.rb b/ee/spec/services/audit_events/streaming/instance_headers/destroy_service_spec.rb index 2615b57c685f63f46dc7b53bbf25d8b273e542e8..42b94cb471aa0145ddb57b99891dd2e72b24c483 100644 --- a/ee/spec/services/audit_events/streaming/instance_headers/destroy_service_spec.rb +++ b/ee/spec/services/audit_events/streaming/instance_headers/destroy_service_spec.rb @@ -4,6 +4,8 @@ RSpec.describe AuditEvents::Streaming::InstanceHeaders::DestroyService, feature_category: :audit_events do let_it_be(:header) { create(:instance_audit_events_streaming_header) } + let_it_be(:user) { create(:admin) } + let_it_be(:event_type) { "audit_events_streaming_instance_headers_destroy" } let(:destination) { header.instance_external_audit_event_destination } @@ -11,13 +13,17 @@ subject(:service) do described_class.new( - params: params + params: params, + current_user: user ) end describe '#execute' do let(:response) { service.execute } - it_behaves_like 'header deletion' + it_behaves_like 'header deletion' do + let(:audit_scope) { be_an_instance_of(Gitlab::Audit::InstanceScope) } + let(:extra_audit_context) { {} } + end end end diff --git a/ee/spec/services/audit_events/streaming/instance_headers/update_service_spec.rb b/ee/spec/services/audit_events/streaming/instance_headers/update_service_spec.rb index bee161718b92f6694204f788d6aa7def4dc4915f..d6db04f5957271953849376978139261a758289b 100644 --- a/ee/spec/services/audit_events/streaming/instance_headers/update_service_spec.rb +++ b/ee/spec/services/audit_events/streaming/instance_headers/update_service_spec.rb @@ -4,6 +4,8 @@ RSpec.describe AuditEvents::Streaming::InstanceHeaders::UpdateService, feature_category: :audit_events do let(:header) { create(:instance_audit_events_streaming_header, key: 'old', value: 'old') } + let_it_be(:user) { create(:admin) } + let_it_be(:event_type) { "audit_events_streaming_instance_headers_update" } let(:params) do { @@ -15,13 +17,17 @@ subject(:service) do described_class.new( - params: params + params: params, + current_user: user ) end describe '#execute' do subject(:response) { service.execute } - it_behaves_like 'header updation' + it_behaves_like 'header updation' do + let(:audit_scope) { be_an_instance_of(Gitlab::Audit::InstanceScope) } + let(:extra_audit_context) { {} } + end end end diff --git a/ee/spec/support/shared_examples/services/audit_events/streaming/headers_operations_shared_examples.rb b/ee/spec/support/shared_examples/services/audit_events/streaming/headers_operations_shared_examples.rb index 6f85b477507aa221f9220208d70d2fd05d0f41a9..0019e51d25dc7add482b514576b9d215a54ebebd 100644 --- a/ee/spec/support/shared_examples/services/audit_events/streaming/headers_operations_shared_examples.rb +++ b/ee/spec/support/shared_examples/services/audit_events/streaming/headers_operations_shared_examples.rb @@ -9,6 +9,12 @@ end end +RSpec.shared_examples 'does not create audit event' do + it do + expect { response }.to not_change { AuditEvent.count } + end +end + RSpec.shared_examples 'header creation successful' do it 'has the header in the response payload' do expect(response).to be_success @@ -53,6 +59,62 @@ expect(header.reload.key).to eq 'new' expect(header.value).to eq 'new' end + + context 'with audit events' do + it 'sends the audit streaming event' do + audit_context = { + name: event_type, + author: user, + scope: audit_scope, + target: header, + message: "Updated a custom HTTP header from key old to have a key new." + } + + audit_context = audit_context.merge(extra_audit_context) + + 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 + + context 'when both key and value are updated' do + it 'creates audit event' do + expect(AuditEvents::AuditEventStreamingWorker).to receive(:perform_async).with(event_type, nil, anything) + + response + end + 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: event_type, + author: user, + scope: audit_scope, + target: header, + message: "Updated a custom HTTP header with key old to have a new value." + } + + audit_context = audit_context.merge(extra_audit_context) + + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(audit_context) + response + end + end + + context 'when neither key nor value is updated' do + let(:params) { super().merge(key: 'old', value: 'old') } + + it_behaves_like 'does not create audit event' + end + end end context 'when header updation is unsuccessful' do @@ -74,6 +136,8 @@ expect(response.errors) .to match_array ["Key can't be blank"] end + + it_behaves_like 'does not create audit event' end end @@ -83,6 +147,35 @@ expect { response }.to change { destination.headers.count }.by(-1) expect(response).to be_success end + + context 'with audit events' do + it 'sends the audit streaming event' do + audit_context = { + name: event_type, + author: user, + scope: audit_scope, + target: header, + message: "Destroyed a custom HTTP header with key #{header.key}." + } + + audit_context = audit_context.merge(extra_audit_context) + + 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 context 'when deletion is unsuccessful' do @@ -102,5 +195,7 @@ expect(response.errors) .to match_array ['foo'] end + + it_behaves_like 'does not create audit event' end end