Log audit events when updating and deleting member roles

Whenever a member role is updated or deleted, log an audit event.

Changelog: added
EE: true

doc/administration/audit_event_streaming/audit_event_types.md

@@ -64,12 +64,6 @@ Audit event types belong to the following product categories.
 | [`update_event_streaming_destination`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74632) | Event triggered when an external audit event destination is updated| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [14.6](https://gitlab.com/gitlab-org/gitlab/-/issues/344664) |
 | [`update_instance_event_streaming_destination`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/125846) | Event triggered when an instance level external audit event destination is updated| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [16.2](https://gitlab.com/gitlab-org/gitlab/-/issues/404730) |
 
-### Authorization
-
-| Name | Description | Saved to database | Streamed | Introduced in |
-|:-----|:------------|:------------------|:---------|:--------------|
-| [`member_role_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/137087) | Event triggered when a custom role is created.| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [16.7](https://gitlab.com/gitlab-org/gitlab/-/issues/388934) |
-
 ### Code review
 
 | Name | Description | Saved to database | Streamed | Introduced in |
@@ -332,6 +326,14 @@ Audit event types belong to the following product categories.
 |:-----|:------------|:------------------|:---------|:--------------|
 | [`experiment_features_enabled_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/118222) | Event triggered on toggling setting for enabling experiment AI features| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [16.0](https://gitlab.com/gitlab-org/gitlab/-/issues/404856/) |
 
+### Permissions
+
+| Name | Description | Saved to database | Streamed | Introduced in |
+|:-----|:------------|:------------------|:---------|:--------------|
+| [`member_role_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/137087) | Event triggered when a custom role is created.| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [16.7](https://gitlab.com/gitlab-org/gitlab/-/issues/388934) |
+| [`member_role_deleted`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/141630) | Event triggered when a custom role is deleted.| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [16.9](https://gitlab.com/gitlab-org/gitlab/-/issues/437672) |
+| [`member_role_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/141630) | Event triggered when a custom role is updated.| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [16.9](https://gitlab.com/gitlab-org/gitlab/-/issues/437672) |
+
 ### Portfolio management
 
 | Name | Description | Saved to database | Streamed | Introduced in |

ee/app/services/member_roles/base_service.rb

@@ -24,5 +24,26 @@ def authorized_error
     def group
       params[:namespace] || member_role&.namespace
     end
+
+    def log_audit_event(member_role, action:)
+      audit_context = {
+        name: "member_role_#{action}",
+        author: current_user,
+        scope: audit_event_scope,
+        target: member_role,
+        target_details: {
+          name: member_role.name,
+          description: member_role.description,
+          abilities: member_role.enabled_permissions.join(', ')
+        },
+        message: "Member role was #{action}"
+      }
+
+      ::Gitlab::Audit::Auditor.audit(audit_context)
+    end
+
+    def audit_event_scope
+      group || Gitlab::Audit::InstanceScope.new
+    end
   end
 end

ee/app/services/member_roles/create_service.rb

@@ -46,29 +46,12 @@ def create_member_role
       member_role = MemberRole.new(params.merge(namespace: group))
 
       if member_role.save
-        log_audit_event(member_role)
+        log_audit_event(member_role, action: :created)
 
         ::ServiceResponse.success(payload: { member_role: member_role })
       else
         ::ServiceResponse.error(message: member_role.errors.full_messages.join(', '))
       end
     end
-
-    def log_audit_event(member_role)
-      audit_context = {
-        name: 'member_role_created',
-        author: current_user,
-        scope: audit_event_scope,
-        target: member_role,
-        target_details: member_role.enabled_permissions.join(', '),
-        message: "Member role #{member_role.name} was created"
-      }
-
-      ::Gitlab::Audit::Auditor.audit(audit_context)
-    end
-
-    def audit_event_scope
-      group || Gitlab::Audit::InstanceScope.new
-    end
   end
 end

ee/app/services/member_roles/delete_service.rb

@@ -8,6 +8,8 @@ def execute(member_role)
       return authorized_error unless allowed?
 
       if member_role.destroy
+        log_audit_event(member_role, action: :deleted)
+
         ::ServiceResponse.success(payload: {
           member_role: member_role
         })

ee/app/services/member_roles/update_service.rb

@@ -16,6 +16,8 @@ def update_member_role
       member_role.assign_attributes(params.slice(:name, :description, *MemberRole.all_customizable_permissions.keys))
 
       if member_role.save
+        log_audit_event(member_role, action: :updated)
+
         ::ServiceResponse.success(payload: { member_role: member_role })
       else
         ::ServiceResponse.error(message: member_role.errors.full_messages, payload: { member_role: member_role.reset })

ee/config/audit_events/types/member_role_created.yml

@@ -3,7 +3,7 @@ name: member_role_created
 description: Event triggered when a custom role is created.
 introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/388934
 introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/137087
-feature_category: authorization
+feature_category: permissions
 milestone: '16.7'
 saved_to_database: true
 streamed: true

ee/config/audit_events/types/member_role_deleted.yml

+---
+name: member_role_deleted
+description: Event triggered when a custom role is deleted.
+introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/437672
+introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/141630
+feature_category: permissions
+milestone: "16.9"
+saved_to_database: true
+streamed: true

ee/config/audit_events/types/member_role_updated.yml

+---
+name: member_role_updated
+description: Event triggered when a custom role is updated.
+introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/437672
+introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/141630
+feature_category: permissions
+milestone: '16.9'
+saved_to_database: true
+streamed: true

ee/spec/services/member_roles/create_service_spec.rb

@@ -33,6 +33,10 @@
       it 'does not create the member role' do
         expect { create_member_role }.not_to change { MemberRole.count }
       end
+
+      it 'does not log an audit event' do
+        expect { create_member_role }.not_to change { AuditEvent.count }
+      end
     end
 
     shared_examples 'member role creation' do
@@ -66,9 +70,13 @@
               details: {
                 author_name: user.name,
                 target_id: operation.id,
-                target_type: 'MemberRole',
-                target_details: 'admin_merge_request, read_vulnerability',
-                custom_message: "Member role #{operation.name} was created",
+                target_type: operation.class.name,
+                target_details: {
+                  name: operation.name,
+                  description: operation.description,
+                  abilities: operation.enabled_permissions.join(', ')
+                }.to_s,
+                custom_message: 'Member role was created',
                 author_class: user.class.name
               }
             }

ee/spec/services/member_roles/delete_service_spec.rb

@@ -37,6 +37,33 @@
 
           expect(member_role).to be_destroyed
         end
+
+        include_examples 'audit event logging' do
+          let(:licensed_features_to_stub) { { custom_roles: true } }
+          let(:event_type) { 'member_role_deleted' }
+          let(:operation) { result }
+          let(:fail_condition!) { allow(member_role).to receive(:destroy).and_return(false) }
+
+          let(:attributes) do
+            {
+              author_id: user.id,
+              entity_id: group.id,
+              entity_type: group.class.name,
+              details: {
+                author_name: user.name,
+                target_id: member_role.id,
+                target_type: member_role.class.name,
+                target_details: {
+                  name: member_role.name,
+                  description: member_role.description,
+                  abilities: member_role.enabled_permissions.join(', ')
+                }.to_s,
+                custom_message: 'Member role was deleted',
+                author_class: user.class.name
+              }
+            }
+          end
+        end
       end
 
       context 'when failing to destroy the member role' do
@@ -50,6 +77,10 @@
           expect(result).to be_error
           expect(result.message).to match_array(['error message'])
         end
+
+        it 'does not log an audit event' do
+          expect { result }.not_to change { AuditEvent.count }
+        end
       end
     end
   end

ee/spec/services/member_roles/update_service_spec.rb

@@ -8,7 +8,14 @@
   let_it_be(:member_role) { create(:member_role, :guest, namespace: group, read_vulnerability: true) }
 
   describe '#execute' do
-    let(:params) { { name: 'new name', read_vulnerability: false, base_access_level: Gitlab::Access::DEVELOPER } }
+    let(:params) do
+      {
+        name: 'new name',
+        description: 'new description',
+        read_vulnerability: false,
+        base_access_level: Gitlab::Access::DEVELOPER
+      }
+    end
 
     subject(:result) { described_class.new(user, params).execute(member_role) }
 
@@ -45,6 +52,33 @@
         it 'does not update unpermitted attributes' do
           expect { result }.not_to change { member_role.reload.base_access_level }
         end
+
+        include_examples 'audit event logging' do
+          let(:licensed_features_to_stub) { { custom_roles: true } }
+          let(:event_type) { 'member_role_updated' }
+          let(:operation) { result }
+          let(:fail_condition!) { allow(member_role).to receive(:save).and_return(false) }
+
+          let(:attributes) do
+            {
+              author_id: user.id,
+              entity_id: group.id,
+              entity_type: group.class.name,
+              details: {
+                author_name: user.name,
+                target_id: member_role.id,
+                target_type: member_role.class.name,
+                target_details: {
+                  name: 'new name',
+                  description: 'new description',
+                  abilities: ''
+                }.to_s,
+                custom_message: 'Member role was updated',
+                author_class: user.class.name
+              }
+            }
+          end
+        end
       end
 
       context 'when member role can not be updated' do
@@ -63,6 +97,10 @@
         it 'includes the object errors' do
           expect(result.message).to eq(['this is wrong'])
         end
+
+        it 'does not log an audit event' do
+          expect { result }.not_to change { AuditEvent.count }
+        end
       end
     end
   end