Merge branch '344664-audit-event-streaming' into 'master'

Auditing for changes to event streaming destinations

See merge request gitlab-org/gitlab!74632

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)
 

ee/app/graphql/mutations/audit_events/external_audit_event_destinations/base.rb

+# 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

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 @@ module Mutations
 
         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

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 @@ module Mutations
         def resolve(id:)
           destination = authorized_find!(id)
 
-          destination.destroy if destination
+          if destination.destroy
+            audit(destination, action: :destroy)
+          end
 
           {
             external_audit_event_destination: nil,

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 @@ module Mutations
         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

ee/spec/requests/api/graphql/mutations/audit_events/external_audit_event_destinations/create_spec.rb

@@ -9,6 +9,8 @@ RSpec.describe 'Create an external audit event destination' do
   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 @@ RSpec.describe 'Create an external audit event destination' do
     }
   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 @@ RSpec.describe 'Create an external audit event destination' do
       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
 

ee/spec/requests/api/graphql/mutations/audit_events/external_audit_event_destinations/destroy_spec.rb

@@ -26,6 +26,11 @@ RSpec.describe 'Destroy an external audit event destination' do
       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 @@ RSpec.describe 'Destroy an external audit event destination' do
         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

ee/spec/requests/api/graphql/mutations/audit_events/external_audit_event_destinations/update_spec.rb

@@ -27,6 +27,11 @@ RSpec.describe 'Update an external audit event destination' do
       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 @@ RSpec.describe 'Update an external audit event destination' do
         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