From fbaee5d5937a6c51e3b4aa2b80335f23fb076441 Mon Sep 17 00:00:00 2001 From: harsimarsandhu Date: Fri, 3 Mar 2023 14:26:09 +0530 Subject: [PATCH 1/3] Audit event for event type filter deletion Creates audit event whenever audit event type filters are deleted EE: true Changelog: added --- .../streaming/event_type_filters/destroy.rb | 9 +- .../event_type_filters/destroy_service.rb | 22 +++-- .../types/event_type_filters_deleted.yml | 9 ++ .../event_type_filters/destroy_spec.rb | 2 +- .../event_type_filters/create_spec.rb | 4 +- .../event_type_filters/delete_spec.rb | 91 +++++++++++++++++++ .../destroy_service_spec.rb | 26 +++++- 7 files changed, 148 insertions(+), 15 deletions(-) create mode 100644 ee/config/audit_events/types/event_type_filters_deleted.yml create mode 100644 ee/spec/requests/api/graphql/audit_events/streaming/event_type_filters/delete_spec.rb diff --git a/ee/app/graphql/mutations/audit_events/streaming/event_type_filters/destroy.rb b/ee/app/graphql/mutations/audit_events/streaming/event_type_filters/destroy.rb index d905ea2c5f6b19..39493fae27c429 100644 --- a/ee/app/graphql/mutations/audit_events/streaming/event_type_filters/destroy.rb +++ b/ee/app/graphql/mutations/audit_events/streaming/event_type_filters/destroy.rb @@ -15,14 +15,19 @@ class Destroy < BaseMutation argument :event_type_filters, [GraphQL::Types::String], required: true, - description: 'List of event type filters to remove from streaming.' + description: 'List of event type filters to remove from streaming.', + prepare: ->(filters, _ctx) do + filters.presence || (raise ::Gitlab::Graphql::Errors::ArgumentError, + 'event type filters must be present') + end def resolve(destination_id:, event_type_filters:) destination = authorized_find!(destination_id) response = ::AuditEvents::Streaming::EventTypeFilters::DestroyService.new( destination: destination, - event_type_filters: event_type_filters + event_type_filters: event_type_filters, + current_user: current_user ).execute if response.success? diff --git a/ee/app/services/audit_events/streaming/event_type_filters/destroy_service.rb b/ee/app/services/audit_events/streaming/event_type_filters/destroy_service.rb index 1c6c6c06cdda11..d472e3c27a98d1 100644 --- a/ee/app/services/audit_events/streaming/event_type_filters/destroy_service.rb +++ b/ee/app/services/audit_events/streaming/event_type_filters/destroy_service.rb @@ -3,19 +3,13 @@ module AuditEvents module Streaming module EventTypeFilters - class DestroyService - attr_reader :destination, :event_type_filters - - def initialize(destination:, event_type_filters:) - @destination = destination - @event_type_filters = event_type_filters - end - + class DestroyService < BaseService def execute errors = validate! if errors.blank? destination.event_type_filters.audit_event_type_in(event_type_filters).delete_all + log_audit_event ServiceResponse.success else ServiceResponse.error(message: errors) @@ -36,6 +30,18 @@ def error_message(missing_filters) format(_("Couldn't find event type filters where audit event type(s): %{missing_filters}"), missing_filters: missing_filters.join(', ')) end + + def log_audit_event + audit_context = { + name: 'event_type_filters_deleted', + author: current_user, + scope: destination.group, + target: destination, + message: "Deleted audit event type filter(s): #{event_type_filters.to_sentence}" + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end end end end diff --git a/ee/config/audit_events/types/event_type_filters_deleted.yml b/ee/config/audit_events/types/event_type_filters_deleted.yml new file mode 100644 index 00000000000000..dd8a73e2104464 --- /dev/null +++ b/ee/config/audit_events/types/event_type_filters_deleted.yml @@ -0,0 +1,9 @@ +--- +name: event_type_filters_deleted +description: Event triggered when audit events streaming event type filters are deleted +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/344848 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/113081 +feature_category: audit_events +milestone: '15.10' +saved_to_database: true +streamed: true diff --git a/ee/spec/graphql/mutations/audit_events/streaming/event_type_filters/destroy_spec.rb b/ee/spec/graphql/mutations/audit_events/streaming/event_type_filters/destroy_spec.rb index 4a3dad0fdfe7e1..a48c60033caebe 100644 --- a/ee/spec/graphql/mutations/audit_events/streaming/event_type_filters/destroy_spec.rb +++ b/ee/spec/graphql/mutations/audit_events/streaming/event_type_filters/destroy_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Mutations::AuditEvents::Streaming::EventTypeFilters::Destroy do +RSpec.describe Mutations::AuditEvents::Streaming::EventTypeFilters::Destroy, feature_category: :audit_events do let_it_be(:current_user) { create(:user) } let_it_be(:event_type_filter) { create(:audit_events_streaming_event_type_filter, audit_event_type: 'filter_1') } diff --git a/ee/spec/requests/api/graphql/audit_events/streaming/event_type_filters/create_spec.rb b/ee/spec/requests/api/graphql/audit_events/streaming/event_type_filters/create_spec.rb index bdd1033970fd30..671a6f95e149b5 100644 --- a/ee/spec/requests/api/graphql/audit_events/streaming/event_type_filters/create_spec.rb +++ b/ee/spec/requests/api/graphql/audit_events/streaming/event_type_filters/create_spec.rb @@ -64,7 +64,7 @@ errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR] end - context 'when current user is a group user' do + context 'when current user is a group owner' do before do group.add_owner(user) end @@ -77,7 +77,7 @@ expect(response["eventTypeFilters"]).to eq(destination.event_type_filters.pluck(:audit_event_type)) end - context 'when event type filters in input in empty' do + context 'when event type filters in input is empty' do let(:input) { { destinationId: destination.to_gid, eventTypeFilters: [] } } it 'returns graphql error' do diff --git a/ee/spec/requests/api/graphql/audit_events/streaming/event_type_filters/delete_spec.rb b/ee/spec/requests/api/graphql/audit_events/streaming/event_type_filters/delete_spec.rb new file mode 100644 index 00000000000000..cd7974167456d1 --- /dev/null +++ b/ee/spec/requests/api/graphql/audit_events/streaming/event_type_filters/delete_spec.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Delete an audit event type filter', feature_category: :audit_events do + include GraphqlHelpers + + let_it_be(:destination) { create(:external_audit_event_destination) } + let_it_be(:event_type_filter) do + create(:audit_events_streaming_event_type_filter, external_audit_event_destination: destination, + audit_event_type: 'filter_1') + end + + let_it_be(:user) { create(:user) } + + let(:current_user) { user } + let(:group) { destination.group } + let(:mutation_name) { :audit_events_streaming_destination_events_add } + let(:mutation) { graphql_mutation(mutation_name, input) } + let(:mutation_response) { graphql_mutation_response(mutation_name) } + let(:input) { { destinationId: destination.to_gid, eventTypeFilters: ['filter_1'] } } + + context 'when unlicensed' do + subject { post_graphql_mutation(mutation, current_user: user) } + + before do + stub_licensed_features(external_audit_events: false) + end + + it_behaves_like 'a mutation on an unauthorized resource' + end + + context 'when licensed' do + subject(:mutate) { post_graphql_mutation(mutation, current_user: user) } + + before do + stub_licensed_features(external_audit_events: true) + end + + context 'when current user is a group maintainer' do + before do + group.add_maintainer(user) + end + + it_behaves_like 'a mutation that returns top-level errors', + errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR] + end + + context 'when current user is a group developer' do + before do + group.add_developer(user) + end + + it_behaves_like 'a mutation that returns top-level errors', + errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR] + end + + context 'when current user is a group guest' do + before do + group.add_guest(user) + end + + it_behaves_like 'a mutation that returns top-level errors', + errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR] + end + + context 'when current user is a group owner' do + before do + group.add_owner(user) + end + + it 'returns success response', :aggregate_failures do + mutate + + response = mutation_response + expect(response["errors"]).to be_empty + expect(response["eventTypeFilters"]).to eq(destination.event_type_filters.pluck(:audit_event_type)) + end + + context 'when event type filters in input is empty' do + let(:input) { { destinationId: destination.to_gid, eventTypeFilters: [] } } + + it 'returns graphql error' do + mutate + + expect(graphql_errors).to include(a_hash_including('message' => 'event type filters must be present')) + end + end + end + end +end diff --git a/ee/spec/services/audit_events/streaming/event_type_filters/destroy_service_spec.rb b/ee/spec/services/audit_events/streaming/event_type_filters/destroy_service_spec.rb index 46ab6a3d5c360c..654495f61e8259 100644 --- a/ee/spec/services/audit_events/streaming/event_type_filters/destroy_service_spec.rb +++ b/ee/spec/services/audit_events/streaming/event_type_filters/destroy_service_spec.rb @@ -2,10 +2,13 @@ require 'spec_helper' -RSpec.describe AuditEvents::Streaming::EventTypeFilters::DestroyService do +RSpec.describe AuditEvents::Streaming::EventTypeFilters::DestroyService, feature_category: :audit_events do let_it_be(:destination) { create(:external_audit_event_destination) } + let_it_be(:user) { create(:user) } - subject(:response) { described_class.new(destination: destination, event_type_filters: event_type_filters).execute } + subject(:response) do + described_class.new(destination: destination, event_type_filters: event_type_filters, current_user: user).execute + end describe '#execute' do context 'when event type filter is not already present' do @@ -16,6 +19,10 @@ expect { subject }.not_to change { destination.event_type_filters.count } expect(response.errors).to match_array(expected_error) end + + it 'does not create audit event' do + expect { subject }.not_to change { AuditEvent.count } + end end context 'when event type filter is already present' do @@ -32,6 +39,21 @@ expect(response).to be_success expect(response.errors).to match_array(expected_error) end + + it 'creates audit event', :aggregate_failures do + audit_context = { + name: 'event_type_filters_deleted', + author: user, + scope: destination.group, + target: destination, + message: "Deleted audit event type filter(s): #{event_type_filter.audit_event_type}" + } + + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(audit_context) + .and_call_original + + expect { subject }.to change { AuditEvent.count }.by(1) + end end end end -- GitLab From d8846f86e2b0f303bd7a981034d712687e94cf33 Mon Sep 17 00:00:00 2001 From: harsimarsandhu Date: Fri, 3 Mar 2023 14:37:07 +0530 Subject: [PATCH 2/3] Update introduced by mr url --- ee/config/audit_events/types/event_type_filters_deleted.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/config/audit_events/types/event_type_filters_deleted.yml b/ee/config/audit_events/types/event_type_filters_deleted.yml index dd8a73e2104464..ea83784ba15923 100644 --- a/ee/config/audit_events/types/event_type_filters_deleted.yml +++ b/ee/config/audit_events/types/event_type_filters_deleted.yml @@ -2,7 +2,7 @@ name: event_type_filters_deleted description: Event triggered when audit events streaming event type filters are deleted introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/344848 -introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/113081 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/113489 feature_category: audit_events milestone: '15.10' saved_to_database: true -- GitLab From 9a90832cb18d8a049473d2f245bcf5aedbb848de Mon Sep 17 00:00:00 2001 From: harsimarsandhu Date: Fri, 3 Mar 2023 16:27:27 +0530 Subject: [PATCH 3/3] Update delete spec --- .../streaming/event_type_filters/delete_spec.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/ee/spec/requests/api/graphql/audit_events/streaming/event_type_filters/delete_spec.rb b/ee/spec/requests/api/graphql/audit_events/streaming/event_type_filters/delete_spec.rb index cd7974167456d1..00e43a0f710f79 100644 --- a/ee/spec/requests/api/graphql/audit_events/streaming/event_type_filters/delete_spec.rb +++ b/ee/spec/requests/api/graphql/audit_events/streaming/event_type_filters/delete_spec.rb @@ -15,7 +15,7 @@ let(:current_user) { user } let(:group) { destination.group } - let(:mutation_name) { :audit_events_streaming_destination_events_add } + let(:mutation_name) { :audit_events_streaming_destination_events_remove } let(:mutation) { graphql_mutation(mutation_name, input) } let(:mutation_response) { graphql_mutation_response(mutation_name) } let(:input) { { destinationId: destination.to_gid, eventTypeFilters: ['filter_1'] } } @@ -69,12 +69,10 @@ group.add_owner(user) end - it 'returns success response', :aggregate_failures do + it 'returns success response' do mutate - response = mutation_response - expect(response["errors"]).to be_empty - expect(response["eventTypeFilters"]).to eq(destination.event_type_filters.pluck(:audit_event_type)) + expect(mutation_response["errors"]).to be_empty end context 'when event type filters in input is empty' do -- GitLab