diff --git a/ee/app/graphql/mutations/audit_events/instance_external_audit_event_destinations/base.rb b/ee/app/graphql/mutations/audit_events/instance_external_audit_event_destinations/base.rb index fc68738d118024d86932fcd74745ccc35d8128ae..eaccbf168e5d988659c8e92c0c511bbeb4fbbb97 100644 --- a/ee/app/graphql/mutations/audit_events/instance_external_audit_event_destinations/base.rb +++ b/ee/app/graphql/mutations/audit_events/instance_external_audit_event_destinations/base.rb @@ -26,7 +26,7 @@ def find_object(destination_gid) destination end - def audit(destination, action:) + def audit(destination, action:, extra_context: {}) audit_context = { name: "#{action}_instance_event_streaming_destination", author: current_user, @@ -35,7 +35,7 @@ def audit(destination, action:) message: "#{action.capitalize} instance event streaming destination #{destination.destination_url}" } - ::Gitlab::Audit::Auditor.audit(audit_context) + ::Gitlab::Audit::Auditor.audit(audit_context.merge(extra_context)) end end end diff --git a/ee/app/graphql/mutations/audit_events/instance_external_audit_event_destinations/destroy.rb b/ee/app/graphql/mutations/audit_events/instance_external_audit_event_destinations/destroy.rb index 36093d8861645d5ff61856a2fcc10858fbb337c1..e87c56e187ece5e16bdc881c4faa63f83801b8d3 100644 --- a/ee/app/graphql/mutations/audit_events/instance_external_audit_event_destinations/destroy.rb +++ b/ee/app/graphql/mutations/audit_events/instance_external_audit_event_destinations/destroy.rb @@ -13,7 +13,8 @@ class Destroy < Base description: 'ID of the external instance audit event destination to destroy.' def resolve(id:) - find_object(id).destroy + destination = find_object(id) + audit(destination, action: :destroy) if destination.destroy { instance_external_audit_event_destination: nil, diff --git a/ee/app/graphql/mutations/audit_events/instance_external_audit_event_destinations/update.rb b/ee/app/graphql/mutations/audit_events/instance_external_audit_event_destinations/update.rb index fad0e88ec22d2964aa99065a0e592d65a5e8ec6f..b01e4c4f429509968f2f3035b9847f0b0dcfed8f 100644 --- a/ee/app/graphql/mutations/audit_events/instance_external_audit_event_destinations/update.rb +++ b/ee/app/graphql/mutations/audit_events/instance_external_audit_event_destinations/update.rb @@ -6,8 +6,12 @@ module InstanceExternalAuditEventDestinations class Update < Base graphql_name 'InstanceExternalAuditEventDestinationUpdate' + include ::Audit::Changes + authorize :admin_instance_external_audit_events + UPDATE_EVENT_NAME = 'update_instance_event_streaming_destination' + argument :id, ::Types::GlobalIDType[::AuditEvents::InstanceExternalAuditEventDestination], required: true, description: 'ID of the external instance audit event destination to update.' @@ -30,13 +34,27 @@ def resolve(id:, destination_url: nil, name: nil) destination_attributes = { destination_url: destination_url, name: name }.compact - destination.update(destination_attributes) + audit_update(destination) if destination.update(destination_attributes) { instance_external_audit_event_destination: (destination if destination.persisted?), errors: Array(destination.errors) } end + + private + + def audit_update(destination) + [:destination_url, :name].each do |column| + audit_changes( + column, + as: column.to_s, + entity: Gitlab::Audit::InstanceScope.new, + model: destination, + event_type: UPDATE_EVENT_NAME + ) + end + end end end end diff --git a/ee/config/audit_events/types/destroy_instance_event_streaming_destination.yml b/ee/config/audit_events/types/destroy_instance_event_streaming_destination.yml new file mode 100644 index 0000000000000000000000000000000000000000..0adcf2c85d1e58d3e19fb021973d875a682f0381 --- /dev/null +++ b/ee/config/audit_events/types/destroy_instance_event_streaming_destination.yml @@ -0,0 +1,8 @@ +name: destroy_instance_event_streaming_destination +description: Event triggered when an instance level external audit event destination is deleted +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/404730 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/125846 +feature_category: audit_events +milestone: "16.2" +saved_to_database: true +streamed: true diff --git a/ee/config/audit_events/types/update_instance_event_streaming_destination.yml b/ee/config/audit_events/types/update_instance_event_streaming_destination.yml new file mode 100644 index 0000000000000000000000000000000000000000..2d256d1475ffcd99d679243dc3002a2f5d718df3 --- /dev/null +++ b/ee/config/audit_events/types/update_instance_event_streaming_destination.yml @@ -0,0 +1,8 @@ +name: update_instance_event_streaming_destination +description: Event triggered when an instance level external audit event destination is updated +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/404730 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/125846 +feature_category: audit_events +milestone: "16.2" +saved_to_database: true +streamed: true diff --git a/ee/lib/audit/changes.rb b/ee/lib/audit/changes.rb index 1c8333ea94017056b72bb93b34eebcca294a273c..24c36db5fc9e844550e1fb9768b1f45a8dca009e 100644 --- a/ee/lib/audit/changes.rb +++ b/ee/lib/audit/changes.rb @@ -100,7 +100,7 @@ def audit_enabled? return true if ::License.feature_available?(:admin_audit_log) return true if ::License.feature_available?(:extended_audit_events) - entity.respond_to?(:feature_available?) && entity.licensed_feature_available?(:audit_events) + entity.respond_to?(:licensed_feature_available?) && entity.licensed_feature_available?(:audit_events) end end end diff --git a/ee/spec/requests/api/graphql/mutations/audit_events/external_audit_event_destinations/update_spec.rb b/ee/spec/requests/api/graphql/mutations/audit_events/external_audit_event_destinations/update_spec.rb index a6bbd7c63467888caf2044a7b64087a7c156a180..f0cd43939b2b96f55a205343e4a914bb06024493 100644 --- a/ee/spec/requests/api/graphql/mutations/audit_events/external_audit_event_destinations/update_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/audit_events/external_audit_event_destinations/update_spec.rb @@ -77,51 +77,8 @@ end.to change { destination.reload.name }.to("New Destination") end - context 'when both destination url and destination name are updated' do - it 'audits the update' do - expect { post_graphql_mutation(mutation, current_user: owner) } - .to change { AuditEvent.count }.by(2) - - audit_events = AuditEvent.last(2) - expect(audit_events[0].details[:custom_message]).to match("Changed destination_url " \ - "from https://example.com/old to https://example.com/new") - expect(audit_events[1].details[:custom_message]).to match("Changed name from Old Destination " \ - "to New Destination") - end - end - - context 'when only destination url is updated' do - let(:input) do - { - 'id': GitlabSchema.id_from_object(destination).to_s, - 'destinationUrl': "https://example.com/new" - } - end - - it 'audits the update' do - expect { post_graphql_mutation(mutation, current_user: owner) } - .to change { AuditEvent.count }.by(1) - - expect(AuditEvent.last.details[:custom_message]).to match("Changed destination_url from " \ - "https://example.com/old to https://example.com/new") - end - end - - context 'when only destination name is updated' do - let(:input) do - { - 'id': GitlabSchema.id_from_object(destination).to_s, - 'name': "New Destination" - } - end - - it 'audits the update' do - expect { post_graphql_mutation(mutation, current_user: owner) } - .to change { AuditEvent.count }.by(1) - - expect(AuditEvent.last.details[:custom_message]).to match("Changed name from Old Destination " \ - "to New Destination") - end + it_behaves_like 'audits update to external streaming destination' do + let_it_be(:current_user) { owner } end context 'when there is no change in values' do diff --git a/ee/spec/requests/api/graphql/mutations/audit_events/instance_external_audit_event_destinations/destroy_spec.rb b/ee/spec/requests/api/graphql/mutations/audit_events/instance_external_audit_event_destinations/destroy_spec.rb index d76d607803c04962a100b88151be451ebebe3f6b..9d76342a9fd8dc59673720f66f7b57ae04e70f38 100644 --- a/ee/spec/requests/api/graphql/mutations/audit_events/instance_external_audit_event_destinations/destroy_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/audit_events/instance_external_audit_event_destinations/destroy_spec.rb @@ -29,6 +29,11 @@ it_behaves_like 'a mutation that returns top-level errors', errors: ['You do not have access to this mutation.'] + + it 'does not audit the destruction' do + expect { post_graphql_mutation(mutation, current_user: current_user) } + .not_to change { AuditEvent.count } + end end context 'when feature is licensed' do @@ -43,6 +48,14 @@ .to change { AuditEvents::InstanceExternalAuditEventDestination.count }.by(-1) end + it 'audits the destruction' do + expect { post_graphql_mutation(mutation, current_user: admin) } + .to change { AuditEvent.count }.by(1) + + expect(AuditEvent.last.details[:custom_message]) + .to eq("Destroy instance event streaming destination #{destination.destination_url}") + end + context 'when the destination id is invalid' do let_it_be(:invalid_destination_input) do { diff --git a/ee/spec/requests/api/graphql/mutations/audit_events/instance_external_audit_event_destinations/update_spec.rb b/ee/spec/requests/api/graphql/mutations/audit_events/instance_external_audit_event_destinations/update_spec.rb index 00b033c8609065b97b705e9f394306e0f2a9df36..08b6d1c32daad2ed46f60a53a25de68bfda229d7 100644 --- a/ee/spec/requests/api/graphql/mutations/audit_events/instance_external_audit_event_destinations/update_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/audit_events/instance_external_audit_event_destinations/update_spec.rb @@ -5,9 +5,15 @@ RSpec.describe 'Update an instance external audit event destination', feature_category: :audit_events do include GraphqlHelpers + let_it_be(:old_destination_url) { "https://example.com/old" } let_it_be(:admin) { create(:admin) } let_it_be(:user) { create(:user) } - let_it_be(:destination) { create(:instance_external_audit_event_destination) } + let_it_be_with_reload(:destination) do + create(:instance_external_audit_event_destination, + name: "Old Destination", + destination_url: old_destination_url) + end + let_it_be(:destination_url) { 'https://example.com/test' } let_it_be(:name) { "My Destination" } @@ -35,6 +41,11 @@ it_behaves_like 'a mutation that returns top-level errors', errors: ['You do not have access to this mutation.'] + + it 'does not audit the update' do + expect { post_graphql_mutation(mutation, current_user: current_user) } + .not_to change { AuditEvent.count } + end end context 'when feature is licensed' do @@ -55,6 +66,23 @@ expect(mutation_response['instanceExternalAuditEventDestination']['verificationToken']).not_to be_empty end + it_behaves_like 'audits update to external streaming destination' + + context 'when destination is same as previous one' do + let(:input) { super().merge(destinationUrl: old_destination_url) } + + it 'updates the destination with correct response' do + expect { post_graphql_mutation(mutation, current_user: admin) } + .not_to change { destination.reload.destination_url } + + expect(mutation_response['errors']).to be_empty + expect(mutation_response['instanceExternalAuditEventDestination']['destinationUrl']) + .to eq(old_destination_url) + expect(mutation_response['instanceExternalAuditEventDestination']['id']).not_to be_empty + expect(mutation_response['instanceExternalAuditEventDestination']['verificationToken']).not_to be_empty + end + end + context 'when the destination id is invalid' do let_it_be(:invalid_destination_input) do { diff --git a/ee/spec/support/shared_examples/audit_events/streaming_destinations_shared_examples.rb b/ee/spec/support/shared_examples/audit_events/streaming_destinations_shared_examples.rb new file mode 100644 index 0000000000000000000000000000000000000000..abfe865c00711398c301a21ccddac5f5e37df2da --- /dev/null +++ b/ee/spec/support/shared_examples/audit_events/streaming_destinations_shared_examples.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +# It expects a variable `current_user` which will be a user who has authorization for the resource , +# it maybe a group owner or instance admin as per the type of destination. +# It also assumes the old name and url of the destination are `Old Destination` and `https://example.com/old` +# respectively. +RSpec.shared_examples 'audits update to external streaming destination' do + context 'when both destination url and destination name are updated' do + let(:input) do + { + id: GitlabSchema.id_from_object(destination).to_s, + destinationUrl: "https://example.com/new", + name: "New Destination" + } + end + + it 'audits the update' do + expect { post_graphql_mutation(mutation, current_user: current_user) } + .to change { AuditEvent.count }.by(2) + + audit_events = AuditEvent.last(2) + expect(audit_events[0].details[:custom_message]) + .to match("Changed destination_url from https://example.com/old to https://example.com/new") + expect(audit_events[1].details[:custom_message]) + .to match("Changed name from Old Destination to New Destination") + end + end + + context 'when only destination url is updated' do + let(:input) do + { + id: GitlabSchema.id_from_object(destination).to_s, + destinationUrl: "https://example.com/new" + } + end + + it 'audits the update' do + expect { post_graphql_mutation(mutation, current_user: current_user) } + .to change { AuditEvent.count }.by(1) + + expect(AuditEvent.last.details[:custom_message]) + .to match("Changed destination_url from https://example.com/old to https://example.com/new") + end + end + + context 'when only destination name is updated' do + let(:input) do + { + id: GitlabSchema.id_from_object(destination).to_s, + name: "New Destination" + } + end + + it 'audits the update' do + expect { post_graphql_mutation(mutation, current_user: current_user) } + .to change { AuditEvent.count }.by(1) + + expect(AuditEvent.last.details[:custom_message]).to match("Changed name from Old Destination to New Destination") + end + end +end