From 79a50b6f6fd7145b866b9f7ea23226d12026d1b2 Mon Sep 17 00:00:00 2001 From: Max Woolf Date: Thu, 18 Nov 2021 13:40:18 +0000 Subject: [PATCH] Auditing for changes to event streaming destinations Adds group-level audit events for CRUD actions on audit streaming destinations. Changelog: added EE: true --- doc/administration/audit_events.md | 1 + .../external_audit_event_destinations/base.rb | 23 ++++++++++ .../create.rb | 11 +++-- .../destroy.rb | 6 ++- .../update.rb | 14 ++++-- .../create_spec.rb | 43 ++++++++++++++----- .../destroy_spec.rb | 12 ++++++ .../update_spec.rb | 12 ++++++ 8 files changed, 100 insertions(+), 22 deletions(-) create mode 100644 ee/app/graphql/mutations/audit_events/external_audit_event_destinations/base.rb diff --git a/doc/administration/audit_events.md b/doc/administration/audit_events.md index 6dce74267fb9ff..d20c593f020700 100644 --- a/doc/administration/audit_events.md +++ b/doc/administration/audit_events.md @@ -98,6 +98,7 @@ From there, you can see the following actions: - Roles allowed to create project changed. - Group CI/CD variable added, removed, or protected status changed. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/30857) in GitLab 13.3. - Compliance framework created, updated, or deleted. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/340649) in GitLab 14.5. +- Event streaming destination created, updated, or deleted. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/344664) in GitLab 14.6. 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/external_audit_event_destinations/base.rb b/ee/app/graphql/mutations/audit_events/external_audit_event_destinations/base.rb new file mode 100644 index 00000000000000..f7ffd682687383 --- /dev/null +++ b/ee/app/graphql/mutations/audit_events/external_audit_event_destinations/base.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module Mutations + module AuditEvents + module ExternalAuditEventDestinations + class Base < BaseMutation + private + + def audit(destination, action:, extra_context: {}) + audit_context = { + name: "#{action}_event_streaming_destination", + author: current_user, + scope: destination.group, + target: destination.group, + message: "#{action.capitalize} event streaming destination #{destination.destination_url}" + } + + ::Gitlab::Audit::Auditor.audit(audit_context.merge(extra_context)) + end + end + end + end +end diff --git a/ee/app/graphql/mutations/audit_events/external_audit_event_destinations/create.rb b/ee/app/graphql/mutations/audit_events/external_audit_event_destinations/create.rb index 80bbe479d51525..b13a6d9582b1a9 100644 --- a/ee/app/graphql/mutations/audit_events/external_audit_event_destinations/create.rb +++ b/ee/app/graphql/mutations/audit_events/external_audit_event_destinations/create.rb @@ -3,7 +3,7 @@ module Mutations module AuditEvents module ExternalAuditEventDestinations - class Create < BaseMutation + class Create < Base graphql_name 'ExternalAuditEventDestinationCreate' authorize :admin_external_audit_events @@ -22,12 +22,11 @@ class Create < BaseMutation def resolve(destination_url:, group_path:) group = authorized_find!(group_path) - destination = ::AuditEvents::ExternalAuditEventDestination.create(group: group, destination_url: destination_url) + destination = ::AuditEvents::ExternalAuditEventDestination.new(group: group, destination_url: destination_url) - { - external_audit_event_destination: destination&.persisted? ? destination : nil, - errors: Array(destination.errors) - } + audit(destination, action: :create) if destination.save + + { external_audit_event_destination: (destination if destination.persisted?), errors: Array(destination.errors) } end private diff --git a/ee/app/graphql/mutations/audit_events/external_audit_event_destinations/destroy.rb b/ee/app/graphql/mutations/audit_events/external_audit_event_destinations/destroy.rb index c35246fac42596..498be62f7962fe 100644 --- a/ee/app/graphql/mutations/audit_events/external_audit_event_destinations/destroy.rb +++ b/ee/app/graphql/mutations/audit_events/external_audit_event_destinations/destroy.rb @@ -3,7 +3,7 @@ module Mutations module AuditEvents module ExternalAuditEventDestinations - class Destroy < BaseMutation + class Destroy < Base graphql_name 'ExternalAuditEventDestinationDestroy' authorize :admin_external_audit_events @@ -15,7 +15,9 @@ class Destroy < BaseMutation def resolve(id:) destination = authorized_find!(id) - destination.destroy if destination + if destination.destroy + audit(destination, action: :destroy) + end { external_audit_event_destination: nil, diff --git a/ee/app/graphql/mutations/audit_events/external_audit_event_destinations/update.rb b/ee/app/graphql/mutations/audit_events/external_audit_event_destinations/update.rb index 8d34f36bb9587f..ff3c0cb914e507 100644 --- a/ee/app/graphql/mutations/audit_events/external_audit_event_destinations/update.rb +++ b/ee/app/graphql/mutations/audit_events/external_audit_event_destinations/update.rb @@ -3,7 +3,7 @@ module Mutations module AuditEvents module ExternalAuditEventDestinations - class Update < BaseMutation + class Update < Base graphql_name 'ExternalAuditEventDestinationUpdate' authorize :admin_external_audit_events @@ -23,16 +23,24 @@ class Update < BaseMutation def resolve(id:, destination_url:) destination = authorized_find!(id) - destination.update(destination_url: destination_url) if destination + audit_update(destination) if destination.update(destination_url: destination_url) { - external_audit_event_destination: destination, + external_audit_event_destination: (destination if destination.persisted?), errors: Array(destination.errors) } end private + def audit_update(destination) + return unless destination.previous_changes.any? + + message = "Updated event streaming destination from #{destination.previous_changes['destination_url'].join(' to ')}" + + audit(destination, action: :update, extra_context: { message: message }) + end + def find_object(destination_gid) GitlabSchema.object_from_id(destination_gid, expected_type: ::AuditEvents::ExternalAuditEventDestination).sync end diff --git a/ee/spec/requests/api/graphql/mutations/audit_events/external_audit_event_destinations/create_spec.rb b/ee/spec/requests/api/graphql/mutations/audit_events/external_audit_event_destinations/create_spec.rb index 818cdf2e7c1282..7050dbc777fd9b 100644 --- a/ee/spec/requests/api/graphql/mutations/audit_events/external_audit_event_destinations/create_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/audit_events/external_audit_event_destinations/create_spec.rb @@ -9,6 +9,8 @@ let_it_be(:owner) { create(:user) } let(:current_user) { owner } + let(:mutation) { graphql_mutation(:external_audit_event_destination_create, input) } + let(:mutation_response) { graphql_mutation_response(:external_audit_event_destination_create) } let(:input) do { @@ -17,18 +19,28 @@ } end - let(:mutation) { graphql_mutation(:external_audit_event_destination_create, input) } - - let(:mutation_response) { graphql_mutation_response(:external_audit_event_destination_create) } + let(:invalid_input) do + { + 'groupPath': group.full_path, + 'destinationUrl': 'ftp://gitlab.com/example/testendpoint' + } + end shared_examples 'a mutation that does not create a destination' do it 'does not destroy the destination' do expect { post_graphql_mutation(mutation, current_user: owner) } .not_to change { AuditEvents::ExternalAuditEventDestination.count } end + + it 'does not audit the creation' do + expect { post_graphql_mutation(mutation, current_user: owner) } + .not_to change { AuditEvent.count } + end end context 'when feature is licensed' do + subject { post_graphql_mutation(mutation, current_user: owner) } + before do stub_licensed_features(external_audit_events: true) end @@ -39,19 +51,28 @@ end it 'creates the destination' do - expect { post_graphql_mutation(mutation, current_user: owner) } + expect { subject } .to change { AuditEvents::ExternalAuditEventDestination.count }.by(1) end - end - context 'when current user is a group owner' do - before do - group.add_owner(owner) + it 'audits the creation' do + expect { subject } + .to change { AuditEvent.count }.by(1) + + expect(AuditEvent.last.details[:custom_message]).to eq("Create event streaming destination https://gitlab.com/example/testendpoint") end - it 'creates the destination' do - expect { post_graphql_mutation(mutation, current_user: owner) } - .to change { AuditEvents::ExternalAuditEventDestination.count }.by(1) + context 'when destination is invalid' do + let(:mutation) { graphql_mutation(:external_audit_event_destination_create, invalid_input) } + + it 'returns correct errors' do + post_graphql_mutation(mutation, current_user: owner) + + expect(mutation_response['externalAuditEventDestination']).to be_nil + expect(mutation_response['errors']).to contain_exactly('Destination url is blocked: Only allowed schemes are http, https') + end + + it_behaves_like 'a mutation that does not create a destination' end end diff --git a/ee/spec/requests/api/graphql/mutations/audit_events/external_audit_event_destinations/destroy_spec.rb b/ee/spec/requests/api/graphql/mutations/audit_events/external_audit_event_destinations/destroy_spec.rb index e5caa363de3530..1dab250691fef5 100644 --- a/ee/spec/requests/api/graphql/mutations/audit_events/external_audit_event_destinations/destroy_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/audit_events/external_audit_event_destinations/destroy_spec.rb @@ -26,6 +26,11 @@ expect { post_graphql_mutation(mutation, current_user: owner) } .not_to change { AuditEvents::ExternalAuditEventDestination.count } end + + it 'does not audit the destruction' do + expect { post_graphql_mutation(mutation, current_user: owner) } + .not_to change { AuditEvent.count } + end end context 'when feature is licensed' do @@ -62,6 +67,13 @@ expect { post_graphql_mutation(mutation, current_user: owner) } .to change { AuditEvents::ExternalAuditEventDestination.count }.by(-1) end + + it 'audits the destruction' do + expect { post_graphql_mutation(mutation, current_user: owner) } + .to change { AuditEvent.count }.by(1) + + expect(AuditEvent.last.details[:custom_message]).to match /Destroy event streaming destination/ + end end context 'when current user is a group maintainer' do 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 644b53a66fb733..0afd3e30c5e411 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 @@ -27,6 +27,11 @@ expect { post_graphql_mutation(mutation, current_user: owner) } .not_to change { destination.reload.destination_url } end + + it 'does not audit the update' do + expect { post_graphql_mutation(mutation, current_user: owner) } + .not_to change { AuditEvent.count } + end end context 'when feature is licensed' do @@ -63,6 +68,13 @@ expect { post_graphql_mutation(mutation, current_user: owner) } .to change { destination.reload.destination_url }.to("https://example.com/test") 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(/Updated event streaming destination from .* to .*/) + end end context 'when current user is a group maintainer' do -- GitLab