From 2f3f91690d5545b2d3d5d71b0c7b311c7d2681f2 Mon Sep 17 00:00:00 2001 From: Nicolae Rotaru Date: Thu, 16 Oct 2025 13:18:54 +0200 Subject: [PATCH 1/8] Add audit events for categories and attributes changes Changelog: added EE: true --- doc/user/compliance/audit_event_types.md | 13 +++++ .../security/attributes/create_service.rb | 27 ++++++++++- .../security/attributes/destroy_service.rb | 21 ++++++++ .../update_project_attributes_service.rb | 48 +++++++++++++++++++ .../security/attributes/update_service.rb | 28 ++++++++++- .../security/categories/create_service.rb | 19 ++++++++ .../security/categories/destroy_service.rb | 19 ++++++++ .../security/categories/update_service.rb | 17 +++++++ ...security_attribute_attached_to_project.yml | 10 ++++ .../types/security_attribute_created.yml | 10 ++++ .../types/security_attribute_deleted.yml | 10 ++++ ...curity_attribute_detached_from_project.yml | 10 ++++ .../types/security_attribute_updated.yml | 10 ++++ .../types/security_category_created.yml | 10 ++++ .../types/security_category_deleted.yml | 10 ++++ .../types/security_category_updated.yml | 10 ++++ .../attributes/create_service_spec.rb | 21 ++++++++ .../attributes/destroy_service_spec.rb | 12 +++++ .../update_project_attributes_service_spec.rb | 39 +++++++++++++++ .../attributes/update_service_spec.rb | 15 ++++++ .../categories/create_service_spec.rb | 12 +++++ .../categories/destroy_service_spec.rb | 12 +++++ .../categories/update_service_spec.rb | 13 +++++ 23 files changed, 393 insertions(+), 3 deletions(-) create mode 100644 ee/config/audit_events/types/security_attribute_attached_to_project.yml create mode 100644 ee/config/audit_events/types/security_attribute_created.yml create mode 100644 ee/config/audit_events/types/security_attribute_deleted.yml create mode 100644 ee/config/audit_events/types/security_attribute_detached_from_project.yml create mode 100644 ee/config/audit_events/types/security_attribute_updated.yml create mode 100644 ee/config/audit_events/types/security_category_created.yml create mode 100644 ee/config/audit_events/types/security_category_deleted.yml create mode 100644 ee/config/audit_events/types/security_category_updated.yml diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index 6257492f7ea317..850e73c5067e51 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -561,6 +561,19 @@ Audit event types belong to the following product categories. | [`merge_request_merged_with_dismissed_security_policy`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/205857) | When a merge request violated a security policy in warn-mode that was dismissed and the MR was merged | {{< icon name="check-circle" >}} Yes | GitLab [18.5](https://gitlab.com/gitlab-org/gitlab/-/issues/569628) | Project | | [`security_policy_merge_request_bypass`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/205601) | A security policy is bypassed for a merge request | {{< icon name="check-circle" >}} Yes | GitLab [18.5](https://gitlab.com/gitlab-org/gitlab/-/issues/549797) | Project | +### Security risk management + +| Type name | Event triggered when | Saved to database | Introduced in | Scope | +|:----------|:---------------------|:------------------|:--------------|:------| +| [`security_attribute_attached_to_project`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD) | A security attribute is attached to a project | {{< icon name="check-circle" >}} Yes | GitLab [18.6](https://gitlab.com/gitlab-org/gitlab/-/issues/568959) | Project | +| [`security_attribute_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD) | A security attribute is created | {{< icon name="check-circle" >}} Yes | GitLab [18.6](https://gitlab.com/gitlab-org/gitlab/-/issues/568959) | Group | +| [`security_attribute_deleted`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD) | A security attribute is deleted | {{< icon name="check-circle" >}} Yes | GitLab [18.6](https://gitlab.com/gitlab-org/gitlab/-/issues/568959) | Group | +| [`security_attribute_detached_from_project`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD) | A security attribute is detached from a project | {{< icon name="check-circle" >}} Yes | GitLab [18.6](https://gitlab.com/gitlab-org/gitlab/-/issues/568959) | Project | +| [`security_attribute_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD) | A security attribute is updated | {{< icon name="check-circle" >}} Yes | GitLab [18.6](https://gitlab.com/gitlab-org/gitlab/-/issues/568959) | Group | +| [`security_category_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD) | A security category is created | {{< icon name="check-circle" >}} Yes | GitLab [18.6](https://gitlab.com/gitlab-org/gitlab/-/issues/568959) | Group | +| [`security_category_deleted`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD) | A security category is deleted | {{< icon name="check-circle" >}} Yes | GitLab [18.6](https://gitlab.com/gitlab-org/gitlab/-/issues/568959) | Group | +| [`security_category_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD) | A security category is updated | {{< icon name="check-circle" >}} Yes | GitLab [18.6](https://gitlab.com/gitlab-org/gitlab/-/issues/568959) | Group | + ### Security testing configuration | Type name | Event triggered when | Saved to database | Introduced in | Scope | diff --git a/ee/app/services/security/attributes/create_service.rb b/ee/app/services/security/attributes/create_service.rb index 3e0db6699cf174..7de522a365fa3c 100644 --- a/ee/app/services/security/attributes/create_service.rb +++ b/ee/app/services/security/attributes/create_service.rb @@ -31,7 +31,12 @@ def execute return error(attributes.select(&:invalid?)) if attributes.any?(&:invalid?) || !category.valid? - category.save ? success : error # Saving the category saves its attributes + if category.save # Saving the category saves its attributes + create_audit_events + success + else + error + end end attr_reader :category, :root_namespace, :params, :current_user, :attributes @@ -56,6 +61,26 @@ def error(failed_attributes = attributes) message += ": #{errors.join(', ')}" if errors.any? ServiceResponse.error(message: message, payload: errors) end + + def create_audit_events + attributes.each do |attribute| + audit_context = { + name: 'security_attribute_created', + author: current_user, + scope: root_namespace, + target: attribute, + message: "Created security attribute #{attribute.name}", + additional_details: { + attribute_name: attribute.name, + attribute_description: attribute.description, + attribute_color: attribute.color, + category_name: category.name + } + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end + end end end end diff --git a/ee/app/services/security/attributes/destroy_service.rb b/ee/app/services/security/attributes/destroy_service.rb index c35bffa83769b3..f212037d492a11 100644 --- a/ee/app/services/security/attributes/destroy_service.rb +++ b/ee/app/services/security/attributes/destroy_service.rb @@ -17,6 +17,8 @@ def execute deleted_attribute_gid = attribute.to_global_id + create_audit_event + attribute.destroy ? success(deleted_attribute_gid) : deletion_failed_error("Failed to delete attribute") rescue ActiveRecord::RecordNotDestroyed => e @@ -56,6 +58,25 @@ def not_editable_error def deletion_failed_error(error_message) error_response("Failed to delete attributes: #{error_message}") end + + def create_audit_event + root_namespace = attribute.security_category.namespace&.root_ancestor + + audit_context = { + name: 'security_attribute_deleted', + author: current_user, + scope: root_namespace, + target: attribute, + message: "Deleted security attribute #{attribute.name}", + additional_details: { + attribute_name: attribute.name, + attribute_description: attribute.description, + category_name: attribute.security_category.name + } + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end end end end diff --git a/ee/app/services/security/attributes/update_project_attributes_service.rb b/ee/app/services/security/attributes/update_project_attributes_service.rb index be504d2f5d1a63..84f3b7ae4f94bb 100644 --- a/ee/app/services/security/attributes/update_project_attributes_service.rb +++ b/ee/app/services/security/attributes/update_project_attributes_service.rb @@ -28,6 +28,7 @@ def execute return error_response('Invalid attributes', errors: validation_errors) if validation_errors.present? apply_changes + create_audit_events ServiceResponse.success(payload: { project: project.reset, @@ -172,6 +173,53 @@ def validate_one_attribute_per_category(attribute) validation_errors << "Cannot add multiple attributes from the same category #{category_id}" end + + def create_audit_events + create_attach_audit_events + create_detach_audit_events + end + + def create_attach_audit_events + associations_to_create.each do |association| + attribute = association.security_attribute + audit_context = { + name: 'security_attribute_attached_to_project', + author: current_user, + scope: project, + target: attribute, + message: "Attached security attribute #{attribute.name} to project #{project.name}", + additional_details: { + attribute_name: attribute.name, + category_name: attribute.security_category.name, + project_name: project.name, + project_path: project.full_path + } + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end + end + + def create_detach_audit_events + associations_to_destroy.each do |association| + attribute = association.security_attribute + audit_context = { + name: 'security_attribute_detached_from_project', + author: current_user, + scope: project, + target: attribute, + message: "Detached security attribute #{attribute.name} from project #{project.name}", + additional_details: { + attribute_name: attribute.name, + category_name: attribute.security_category.name, + project_name: project.name, + project_path: project.full_path + } + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end + end end end end diff --git a/ee/app/services/security/attributes/update_service.rb b/ee/app/services/security/attributes/update_service.rb index 053f4aff3c3df3..1a1c860ca99f44 100644 --- a/ee/app/services/security/attributes/update_service.rb +++ b/ee/app/services/security/attributes/update_service.rb @@ -18,8 +18,15 @@ def execute return UnauthorizedError unless permitted? return ServiceResponse.error(message: 'Cannot update non editable attribute') unless attribute.editable? - attribute.assign_attributes(params.slice(:name, :description, :color)) - attribute.save ? success : error + update_params = params.slice(:name, :description, :color) + attribute.assign_attributes(update_params) + + if attribute.save + create_audit_event(update_params) + success + else + error + end end attr_reader :attribute, :root_namespace, :params, :current_user @@ -40,6 +47,23 @@ def error message += ": #{errors.join(', ')}" if errors.any? ServiceResponse.error(message: message, payload: errors) end + + def create_audit_event(update_params) + audit_context = { + name: 'security_attribute_updated', + author: current_user, + scope: root_namespace, + target: attribute, + message: "Updated security attribute #{attribute.name}", + additional_details: { + attribute_name: attribute.name, + category_name: attribute.security_category.name, + updated_fields: update_params.keys + }.merge(update_params) + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end end end end diff --git a/ee/app/services/security/categories/create_service.rb b/ee/app/services/security/categories/create_service.rb index 019bc1d93cbf54..7e691f4a8a8b7b 100644 --- a/ee/app/services/security/categories/create_service.rb +++ b/ee/app/services/security/categories/create_service.rb @@ -30,6 +30,7 @@ def execute ) return error unless category.save + create_audit_event success end @@ -48,6 +49,24 @@ def success def error ServiceResponse.error(message: _('Failed to create security category'), payload: category.errors) end + + def create_audit_event + audit_context = { + name: 'security_category_created', + author: current_user, + scope: root_namespace, + target: category, + message: "Created security category #{category.name}", + additional_details: { + category_name: category.name, + category_description: category.description, + multiple_selection: category.multiple_selection, + template_type: category.template_type + } + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end end end end diff --git a/ee/app/services/security/categories/destroy_service.rb b/ee/app/services/security/categories/destroy_service.rb index a87526448c4dc7..b026999f47729e 100644 --- a/ee/app/services/security/categories/destroy_service.rb +++ b/ee/app/services/security/categories/destroy_service.rb @@ -19,6 +19,8 @@ def execute [attribute.id, attribute.to_global_id] end + create_audit_event + Security::Category.transaction do category.security_attributes.destroy_all # rubocop:disable Cop/DestroyAll -- Need destroy callbacks to trigger worker for project association cleanup category.destroy @@ -66,6 +68,23 @@ def not_editable_error def deletion_failed_error(error_message) error_response("Failed to delete category: #{error_message}") end + + def create_audit_event + audit_context = { + name: 'security_category_deleted', + author: current_user, + scope: category.namespace, + target: category, + message: "Deleted security category #{category.name}", + additional_details: { + category_name: category.name, + category_description: category.description, + attributes_count: category.security_attributes.count + } + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end end end end diff --git a/ee/app/services/security/categories/update_service.rb b/ee/app/services/security/categories/update_service.rb index bfc1281d294ae4..f560ea0d5dd131 100644 --- a/ee/app/services/security/categories/update_service.rb +++ b/ee/app/services/security/categories/update_service.rb @@ -20,6 +20,7 @@ def execute update_params = params.except(:namespace, :editable_state, :template_type, :multiple_selection) return error unless update_params.present? && category.update(update_params) + create_audit_event(update_params) success end @@ -44,6 +45,22 @@ def error message += ": #{category.errors.full_messages.join(', ')}" if category.errors.present? ServiceResponse.error(message: message, payload: category.errors) end + + def create_audit_event(update_params) + audit_context = { + name: 'security_category_updated', + author: current_user, + scope: category.namespace, + target: category, + message: "Updated security category #{category.name}", + additional_details: { + category_name: category.name, + updated_fields: update_params.keys + }.merge(update_params) + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end end end end diff --git a/ee/config/audit_events/types/security_attribute_attached_to_project.yml b/ee/config/audit_events/types/security_attribute_attached_to_project.yml new file mode 100644 index 00000000000000..329d1de2eebe03 --- /dev/null +++ b/ee/config/audit_events/types/security_attribute_attached_to_project.yml @@ -0,0 +1,10 @@ +--- +name: security_attribute_attached_to_project +description: A security attribute is attached to a project +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/568959 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD +feature_category: security_risk_management +milestone: '18.6' +saved_to_database: true +streamed: true +scope: [Project] \ No newline at end of file diff --git a/ee/config/audit_events/types/security_attribute_created.yml b/ee/config/audit_events/types/security_attribute_created.yml new file mode 100644 index 00000000000000..a3835537a56703 --- /dev/null +++ b/ee/config/audit_events/types/security_attribute_created.yml @@ -0,0 +1,10 @@ +--- +name: security_attribute_created +description: A security attribute is created +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/568959 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD +feature_category: security_risk_management +milestone: '18.6' +saved_to_database: true +streamed: true +scope: [Group] \ No newline at end of file diff --git a/ee/config/audit_events/types/security_attribute_deleted.yml b/ee/config/audit_events/types/security_attribute_deleted.yml new file mode 100644 index 00000000000000..00daa65bd423c3 --- /dev/null +++ b/ee/config/audit_events/types/security_attribute_deleted.yml @@ -0,0 +1,10 @@ +--- +name: security_attribute_deleted +description: A security attribute is deleted +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/568959 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD +feature_category: security_risk_management +milestone: '18.6' +saved_to_database: true +streamed: true +scope: [Group] \ No newline at end of file diff --git a/ee/config/audit_events/types/security_attribute_detached_from_project.yml b/ee/config/audit_events/types/security_attribute_detached_from_project.yml new file mode 100644 index 00000000000000..9eb1f1855e321f --- /dev/null +++ b/ee/config/audit_events/types/security_attribute_detached_from_project.yml @@ -0,0 +1,10 @@ +--- +name: security_attribute_detached_from_project +description: A security attribute is detached from a project +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/568959 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD +feature_category: security_risk_management +milestone: '18.6' +saved_to_database: true +streamed: true +scope: [Project] \ No newline at end of file diff --git a/ee/config/audit_events/types/security_attribute_updated.yml b/ee/config/audit_events/types/security_attribute_updated.yml new file mode 100644 index 00000000000000..393aead05df41f --- /dev/null +++ b/ee/config/audit_events/types/security_attribute_updated.yml @@ -0,0 +1,10 @@ +--- +name: security_attribute_updated +description: A security attribute is updated +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/568959 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD +feature_category: security_risk_management +milestone: '18.6' +saved_to_database: true +streamed: true +scope: [Group] \ No newline at end of file diff --git a/ee/config/audit_events/types/security_category_created.yml b/ee/config/audit_events/types/security_category_created.yml new file mode 100644 index 00000000000000..a70701dc261175 --- /dev/null +++ b/ee/config/audit_events/types/security_category_created.yml @@ -0,0 +1,10 @@ +--- +name: security_category_created +description: A security category is created +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/568959 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD +feature_category: security_risk_management +milestone: '18.6' +saved_to_database: true +streamed: true +scope: [Group] \ No newline at end of file diff --git a/ee/config/audit_events/types/security_category_deleted.yml b/ee/config/audit_events/types/security_category_deleted.yml new file mode 100644 index 00000000000000..34e8640efa9733 --- /dev/null +++ b/ee/config/audit_events/types/security_category_deleted.yml @@ -0,0 +1,10 @@ +--- +name: security_category_deleted +description: A security category is deleted +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/568959 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD +feature_category: security_risk_management +milestone: '18.6' +saved_to_database: true +streamed: true +scope: [Group] \ No newline at end of file diff --git a/ee/config/audit_events/types/security_category_updated.yml b/ee/config/audit_events/types/security_category_updated.yml new file mode 100644 index 00000000000000..cec9e6527fa853 --- /dev/null +++ b/ee/config/audit_events/types/security_category_updated.yml @@ -0,0 +1,10 @@ +--- +name: security_category_updated +description: A security category is updated +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/568959 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD +feature_category: security_risk_management +milestone: '18.6' +saved_to_database: true +streamed: true +scope: [Group] \ No newline at end of file diff --git a/ee/spec/services/security/attributes/create_service_spec.rb b/ee/spec/services/security/attributes/create_service_spec.rb index 63d4f399ec6a4e..fe102ccc766bd3 100644 --- a/ee/spec/services/security/attributes/create_service_spec.rb +++ b/ee/spec/services/security/attributes/create_service_spec.rb @@ -82,6 +82,27 @@ expect(execute).to be_success end + it 'creates audit events for each attribute' do + expect { execute }.to change { AuditEvent.count }.by(2) + + audit_events = AuditEvent.last(2) + audit_events.each do |audit_event| + expect(audit_event.details[:event_name]).to eq('security_attribute_created') + expect(audit_event.details[:author_name]).to eq(user.name) + expect(audit_event.details[:category_name]).to eq(category.name) + end + + expect(audit_events.first.details[:custom_message]).to eq("Created security attribute Critical") + expect(audit_events.first.details[:attribute_name]).to eq('Critical') + expect(audit_events.first.details[:attribute_description]).to eq('Critical security level') + expect(audit_events.first.details[:attribute_color]).to eq(::Gitlab::Color.of('#FF0000')) + + expect(audit_events.second.details[:custom_message]).to eq("Created security attribute High") + expect(audit_events.second.details[:attribute_name]).to eq('High') + expect(audit_events.second.details[:attribute_description]).to eq('High security level') + expect(audit_events.second.details[:attribute_color]).to eq(::Gitlab::Color.of('#FF8C00')) + end + context 'when attribute validation fails' do let(:params) { { attributes: [{ name: '', description: 'Test description', color: '#FF0000' }] } } diff --git a/ee/spec/services/security/attributes/destroy_service_spec.rb b/ee/spec/services/security/attributes/destroy_service_spec.rb index 3780187f0349fc..b9648f8cc533a5 100644 --- a/ee/spec/services/security/attributes/destroy_service_spec.rb +++ b/ee/spec/services/security/attributes/destroy_service_spec.rb @@ -81,6 +81,18 @@ execute end + it 'creates an audit event' do + expect { execute }.to change { AuditEvent.count }.by(1) + + audit_event = AuditEvent.last + expect(audit_event.details[:event_name]).to eq('security_attribute_deleted') + expect(audit_event.details[:author_name]).to eq(user.name) + expect(audit_event.details[:custom_message]).to eq("Deleted security attribute #{attribute.name}") + expect(audit_event.details[:attribute_name]).to eq(attribute.name) + expect(audit_event.details[:attribute_description]).to eq(attribute.description) + expect(audit_event.details[:category_name]).to eq(category.name) + end + it 'does not enqueue worker when deletion fails' do allow(attribute).to receive(:destroy).and_return(false) diff --git a/ee/spec/services/security/attributes/update_project_attributes_service_spec.rb b/ee/spec/services/security/attributes/update_project_attributes_service_spec.rb index 5d8a8290f11fed..b83517b096ac12 100644 --- a/ee/spec/services/security/attributes/update_project_attributes_service_spec.rb +++ b/ee/spec/services/security/attributes/update_project_attributes_service_spec.rb @@ -176,6 +176,30 @@ def expect_error_with_payload_errors(*errors) expect(execute.payload[:added_count]).to eq(2) expect(execute.payload[:removed_count]).to eq(0) end + + it 'creates audit events for attached attributes' do + expect { execute }.to change { AuditEvent.count }.by(2) + + audit_events = AuditEvent.last(2) + audit_events.each do |audit_event| + expect(audit_event.details[:event_name]).to eq('security_attribute_attached_to_project') + expect(audit_event.details[:author_name]).to eq(user.name) + expect(audit_event.details[:project_name]).to eq(project.name) + expect(audit_event.details[:project_path]).to eq(project.full_path) + end + + expect(audit_events.first.details[:custom_message]).to eq( + "Attached security attribute #{attribute1.name} to project #{project.name}" + ) + expect(audit_events.first.details[:attribute_name]).to eq(attribute1.name) + expect(audit_events.first.details[:category_name]).to eq(single_selection_category.name) + + expect(audit_events.second.details[:custom_message]).to eq( + "Attached security attribute #{other_attribute.name} to project #{project.name}" + ) + expect(audit_events.second.details[:attribute_name]).to eq(other_attribute.name) + expect(audit_events.second.details[:category_name]).to eq(other_category.name) + end end context 'when removing attributes' do @@ -196,6 +220,21 @@ def expect_error_with_payload_errors(*errors) expect(execute.payload[:added_count]).to eq(0) expect(execute.payload[:removed_count]).to eq(1) end + + it 'creates audit event for detached attribute' do + expect { execute }.to change { AuditEvent.count }.by(1) + + audit_event = AuditEvent.last + expect(audit_event.details[:event_name]).to eq('security_attribute_detached_from_project') + expect(audit_event.details[:author_name]).to eq(user.name) + expect(audit_event.details[:custom_message]).to eq( + "Detached security attribute #{other_attribute.name} from project #{project.name}" + ) + expect(audit_event.details[:attribute_name]).to eq(other_attribute.name) + expect(audit_event.details[:category_name]).to eq(other_category.name) + expect(audit_event.details[:project_name]).to eq(project.name) + expect(audit_event.details[:project_path]).to eq(project.full_path) + end end context 'when attribute does not exist' do diff --git a/ee/spec/services/security/attributes/update_service_spec.rb b/ee/spec/services/security/attributes/update_service_spec.rb index 840370414342e8..c803c4c6c3a2e7 100644 --- a/ee/spec/services/security/attributes/update_service_spec.rb +++ b/ee/spec/services/security/attributes/update_service_spec.rb @@ -66,6 +66,21 @@ expect(attribute.color).to eq(::Gitlab::Color.of('#00FF00')) end + it 'creates an audit event' do + expect { execute }.to change { AuditEvent.count }.by(1) + + audit_event = AuditEvent.last + expect(audit_event.details[:event_name]).to eq('security_attribute_updated') + expect(audit_event.details[:author_name]).to eq(user.name) + expect(audit_event.details[:custom_message]).to eq("Updated security attribute Updated Name") + expect(audit_event.details[:attribute_name]).to eq('Updated Name') + expect(audit_event.details[:category_name]).to eq(category.name) + expect(audit_event.details[:updated_fields]).to eq(%i[name description color]) + expect(audit_event.details[:name]).to eq('Updated Name') + expect(audit_event.details[:description]).to eq('Updated Description') + expect(audit_event.details[:color]).to eq(::Gitlab::Color.of('#00FF00')) + end + context 'when attribute is not editable' do let(:attribute) do create(:security_attribute, security_category: category, namespace: namespace, editable_state: :locked) diff --git a/ee/spec/services/security/categories/create_service_spec.rb b/ee/spec/services/security/categories/create_service_spec.rb index 25e970b19531b9..f102c658c9e4e1 100644 --- a/ee/spec/services/security/categories/create_service_spec.rb +++ b/ee/spec/services/security/categories/create_service_spec.rb @@ -174,6 +174,18 @@ execute end + + it 'creates an audit event' do + expect { execute }.to change { AuditEvent.count }.by(1) + + audit_event = AuditEvent.last + expect(audit_event.details[:event_name]).to eq('security_category_created') + expect(audit_event.details[:author_name]).to eq(current_user.name) + expect(audit_event.details[:custom_message]).to eq("Created security category #{params[:name]}") + expect(audit_event.details[:category_name]).to eq(params[:name]) + expect(audit_event.details[:category_description]).to eq(params[:description]) + expect(audit_event.details[:multiple_selection]).to eq(params[:multiple_selection]) + end end context 'when name already exists in the same namespace' do diff --git a/ee/spec/services/security/categories/destroy_service_spec.rb b/ee/spec/services/security/categories/destroy_service_spec.rb index 331cb1b686a1cf..28a0e0b83d1dce 100644 --- a/ee/spec/services/security/categories/destroy_service_spec.rb +++ b/ee/spec/services/security/categories/destroy_service_spec.rb @@ -107,6 +107,18 @@ let(:test_category) { category } include_examples 'successful category deletion' + + it 'creates an audit event' do + expect { execute }.to change { AuditEvent.count }.by(1) + + audit_event = AuditEvent.last + expect(audit_event.details[:event_name]).to eq('security_category_deleted') + expect(audit_event.details[:author_name]).to eq(user.name) + expect(audit_event.details[:custom_message]).to eq("Deleted security category #{category.name}") + expect(audit_event.details[:category_name]).to eq(category.name) + expect(audit_event.details[:category_description]).to eq(category.description) + expect(audit_event.details[:attributes_count]).to eq(1) + end end context 'when deletion fails due to database error' do diff --git a/ee/spec/services/security/categories/update_service_spec.rb b/ee/spec/services/security/categories/update_service_spec.rb index dae1c508ce63dc..c5ba4da4b8f19d 100644 --- a/ee/spec/services/security/categories/update_service_spec.rb +++ b/ee/spec/services/security/categories/update_service_spec.rb @@ -126,6 +126,19 @@ result = execute expect(result.payload[:category]).to eq(category) end + + it 'creates an audit event' do + expect { execute }.to change { AuditEvent.count }.by(1) + + audit_event = AuditEvent.last + expect(audit_event.details[:event_name]).to eq('security_category_updated') + expect(audit_event.details[:author_name]).to eq(current_user.name) + expect(audit_event.details[:custom_message]).to eq("Updated security category #{params[:name]}") + expect(audit_event.details[:category_name]).to eq(params[:name]) + expect(audit_event.details[:updated_fields]).to eq(%i[name description]) + expect(audit_event.details[:name]).to eq(params[:name]) + expect(audit_event.details[:description]).to eq(params[:description]) + end end context 'when updating only specific fields' do -- GitLab From 4e57f19c3379bff0e9efaf9ef60e4f8f7b209008 Mon Sep 17 00:00:00 2001 From: Nicolae Rotaru Date: Thu, 16 Oct 2025 13:50:28 +0200 Subject: [PATCH 2/8] Fix spec, update events MR link --- doc/user/compliance/audit_event_types.md | 16 ++++++++-------- .../security/attributes/destroy_service.rb | 10 ++++++---- .../security_attribute_attached_to_project.yml | 2 +- .../types/security_attribute_created.yml | 2 +- .../types/security_attribute_deleted.yml | 2 +- .../security_attribute_detached_from_project.yml | 2 +- .../types/security_attribute_updated.yml | 2 +- .../types/security_category_created.yml | 2 +- .../types/security_category_deleted.yml | 2 +- .../types/security_category_updated.yml | 2 +- .../security/attributes/update_service_spec.rb | 2 +- 11 files changed, 23 insertions(+), 21 deletions(-) diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index 850e73c5067e51..73410628778d5f 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -565,14 +565,14 @@ Audit event types belong to the following product categories. | Type name | Event triggered when | Saved to database | Introduced in | Scope | |:----------|:---------------------|:------------------|:--------------|:------| -| [`security_attribute_attached_to_project`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD) | A security attribute is attached to a project | {{< icon name="check-circle" >}} Yes | GitLab [18.6](https://gitlab.com/gitlab-org/gitlab/-/issues/568959) | Project | -| [`security_attribute_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD) | A security attribute is created | {{< icon name="check-circle" >}} Yes | GitLab [18.6](https://gitlab.com/gitlab-org/gitlab/-/issues/568959) | Group | -| [`security_attribute_deleted`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD) | A security attribute is deleted | {{< icon name="check-circle" >}} Yes | GitLab [18.6](https://gitlab.com/gitlab-org/gitlab/-/issues/568959) | Group | -| [`security_attribute_detached_from_project`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD) | A security attribute is detached from a project | {{< icon name="check-circle" >}} Yes | GitLab [18.6](https://gitlab.com/gitlab-org/gitlab/-/issues/568959) | Project | -| [`security_attribute_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD) | A security attribute is updated | {{< icon name="check-circle" >}} Yes | GitLab [18.6](https://gitlab.com/gitlab-org/gitlab/-/issues/568959) | Group | -| [`security_category_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD) | A security category is created | {{< icon name="check-circle" >}} Yes | GitLab [18.6](https://gitlab.com/gitlab-org/gitlab/-/issues/568959) | Group | -| [`security_category_deleted`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD) | A security category is deleted | {{< icon name="check-circle" >}} Yes | GitLab [18.6](https://gitlab.com/gitlab-org/gitlab/-/issues/568959) | Group | -| [`security_category_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD) | A security category is updated | {{< icon name="check-circle" >}} Yes | GitLab [18.6](https://gitlab.com/gitlab-org/gitlab/-/issues/568959) | Group | +| [`security_attribute_attached_to_project`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/209118) | A security attribute is attached to a project | {{< icon name="check-circle" >}} Yes | GitLab [18.6](https://gitlab.com/gitlab-org/gitlab/-/issues/568959) | Project | +| [`security_attribute_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/209118) | A security attribute is created | {{< icon name="check-circle" >}} Yes | GitLab [18.6](https://gitlab.com/gitlab-org/gitlab/-/issues/568959) | Group | +| [`security_attribute_deleted`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/209118) | A security attribute is deleted | {{< icon name="check-circle" >}} Yes | GitLab [18.6](https://gitlab.com/gitlab-org/gitlab/-/issues/568959) | Group | +| [`security_attribute_detached_from_project`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/209118) | A security attribute is detached from a project | {{< icon name="check-circle" >}} Yes | GitLab [18.6](https://gitlab.com/gitlab-org/gitlab/-/issues/568959) | Project | +| [`security_attribute_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/209118) | A security attribute is updated | {{< icon name="check-circle" >}} Yes | GitLab [18.6](https://gitlab.com/gitlab-org/gitlab/-/issues/568959) | Group | +| [`security_category_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/209118) | A security category is created | {{< icon name="check-circle" >}} Yes | GitLab [18.6](https://gitlab.com/gitlab-org/gitlab/-/issues/568959) | Group | +| [`security_category_deleted`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/209118) | A security category is deleted | {{< icon name="check-circle" >}} Yes | GitLab [18.6](https://gitlab.com/gitlab-org/gitlab/-/issues/568959) | Group | +| [`security_category_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/209118) | A security category is updated | {{< icon name="check-circle" >}} Yes | GitLab [18.6](https://gitlab.com/gitlab-org/gitlab/-/issues/568959) | Group | ### Security testing configuration diff --git a/ee/app/services/security/attributes/destroy_service.rb b/ee/app/services/security/attributes/destroy_service.rb index f212037d492a11..3bb4647c0e1e64 100644 --- a/ee/app/services/security/attributes/destroy_service.rb +++ b/ee/app/services/security/attributes/destroy_service.rb @@ -17,10 +17,12 @@ def execute deleted_attribute_gid = attribute.to_global_id - create_audit_event - - attribute.destroy ? success(deleted_attribute_gid) : deletion_failed_error("Failed to delete attribute") - + if attribute.destroy + create_audit_event + success(deleted_attribute_gid) + else + deletion_failed_error("Failed to delete attribute") + end rescue ActiveRecord::RecordNotDestroyed => e deletion_failed_error(e.message) end diff --git a/ee/config/audit_events/types/security_attribute_attached_to_project.yml b/ee/config/audit_events/types/security_attribute_attached_to_project.yml index 329d1de2eebe03..28c4f8a8a55135 100644 --- a/ee/config/audit_events/types/security_attribute_attached_to_project.yml +++ b/ee/config/audit_events/types/security_attribute_attached_to_project.yml @@ -2,7 +2,7 @@ name: security_attribute_attached_to_project description: A security attribute is attached to a project introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/568959 -introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/209118 feature_category: security_risk_management milestone: '18.6' saved_to_database: true diff --git a/ee/config/audit_events/types/security_attribute_created.yml b/ee/config/audit_events/types/security_attribute_created.yml index a3835537a56703..23be286dfbf87c 100644 --- a/ee/config/audit_events/types/security_attribute_created.yml +++ b/ee/config/audit_events/types/security_attribute_created.yml @@ -2,7 +2,7 @@ name: security_attribute_created description: A security attribute is created introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/568959 -introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/209118 feature_category: security_risk_management milestone: '18.6' saved_to_database: true diff --git a/ee/config/audit_events/types/security_attribute_deleted.yml b/ee/config/audit_events/types/security_attribute_deleted.yml index 00daa65bd423c3..29b13349a4018c 100644 --- a/ee/config/audit_events/types/security_attribute_deleted.yml +++ b/ee/config/audit_events/types/security_attribute_deleted.yml @@ -2,7 +2,7 @@ name: security_attribute_deleted description: A security attribute is deleted introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/568959 -introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/209118 feature_category: security_risk_management milestone: '18.6' saved_to_database: true diff --git a/ee/config/audit_events/types/security_attribute_detached_from_project.yml b/ee/config/audit_events/types/security_attribute_detached_from_project.yml index 9eb1f1855e321f..14d96841033b85 100644 --- a/ee/config/audit_events/types/security_attribute_detached_from_project.yml +++ b/ee/config/audit_events/types/security_attribute_detached_from_project.yml @@ -2,7 +2,7 @@ name: security_attribute_detached_from_project description: A security attribute is detached from a project introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/568959 -introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/209118 feature_category: security_risk_management milestone: '18.6' saved_to_database: true diff --git a/ee/config/audit_events/types/security_attribute_updated.yml b/ee/config/audit_events/types/security_attribute_updated.yml index 393aead05df41f..3bd15fb3bd6a0a 100644 --- a/ee/config/audit_events/types/security_attribute_updated.yml +++ b/ee/config/audit_events/types/security_attribute_updated.yml @@ -2,7 +2,7 @@ name: security_attribute_updated description: A security attribute is updated introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/568959 -introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/209118 feature_category: security_risk_management milestone: '18.6' saved_to_database: true diff --git a/ee/config/audit_events/types/security_category_created.yml b/ee/config/audit_events/types/security_category_created.yml index a70701dc261175..feb78b8b9f34a5 100644 --- a/ee/config/audit_events/types/security_category_created.yml +++ b/ee/config/audit_events/types/security_category_created.yml @@ -2,7 +2,7 @@ name: security_category_created description: A security category is created introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/568959 -introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/209118 feature_category: security_risk_management milestone: '18.6' saved_to_database: true diff --git a/ee/config/audit_events/types/security_category_deleted.yml b/ee/config/audit_events/types/security_category_deleted.yml index 34e8640efa9733..1a5755f8f06556 100644 --- a/ee/config/audit_events/types/security_category_deleted.yml +++ b/ee/config/audit_events/types/security_category_deleted.yml @@ -2,7 +2,7 @@ name: security_category_deleted description: A security category is deleted introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/568959 -introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/209118 feature_category: security_risk_management milestone: '18.6' saved_to_database: true diff --git a/ee/config/audit_events/types/security_category_updated.yml b/ee/config/audit_events/types/security_category_updated.yml index cec9e6527fa853..4905690c75ff74 100644 --- a/ee/config/audit_events/types/security_category_updated.yml +++ b/ee/config/audit_events/types/security_category_updated.yml @@ -2,7 +2,7 @@ name: security_category_updated description: A security category is updated introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/568959 -introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TBD +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/209118 feature_category: security_risk_management milestone: '18.6' saved_to_database: true diff --git a/ee/spec/services/security/attributes/update_service_spec.rb b/ee/spec/services/security/attributes/update_service_spec.rb index c803c4c6c3a2e7..2de6bb47e3a5ff 100644 --- a/ee/spec/services/security/attributes/update_service_spec.rb +++ b/ee/spec/services/security/attributes/update_service_spec.rb @@ -78,7 +78,7 @@ expect(audit_event.details[:updated_fields]).to eq(%i[name description color]) expect(audit_event.details[:name]).to eq('Updated Name') expect(audit_event.details[:description]).to eq('Updated Description') - expect(audit_event.details[:color]).to eq(::Gitlab::Color.of('#00FF00')) + expect(audit_event.details[:color]).to eq('#00FF00') end context 'when attribute is not editable' do -- GitLab From 8364b961b4285414e9e47992db2f3ed8d76a09a4 Mon Sep 17 00:00:00 2001 From: Nicolae Rotaru Date: Thu, 16 Oct 2025 18:07:27 +0200 Subject: [PATCH 3/8] Implement batching, update specs --- .../security/attributes/create_service.rb | 48 +++++--- .../update_project_attributes_service.rb | 107 +++++++++++------- .../attributes/create_service_spec.rb | 19 +++- .../update_project_attributes_service_spec.rb | 38 ++++++- 4 files changed, 151 insertions(+), 61 deletions(-) diff --git a/ee/app/services/security/attributes/create_service.rb b/ee/app/services/security/attributes/create_service.rb index 7de522a365fa3c..49a725219df224 100644 --- a/ee/app/services/security/attributes/create_service.rb +++ b/ee/app/services/security/attributes/create_service.rb @@ -3,6 +3,8 @@ module Security module Attributes class CreateService < BaseService + AUDIT_EVENT_NAME = 'security_attribute_created' + def initialize(category:, namespace:, params:, current_user:) @category = category @root_namespace = namespace&.root_ancestor @@ -63,24 +65,38 @@ def error(failed_attributes = attributes) end def create_audit_events - attributes.each do |attribute| - audit_context = { - name: 'security_attribute_created', - author: current_user, - scope: root_namespace, - target: attribute, - message: "Created security attribute #{attribute.name}", - additional_details: { - attribute_name: attribute.name, - attribute_description: attribute.description, - attribute_color: attribute.color, - category_name: category.name - } - } - - ::Gitlab::Audit::Auditor.audit(audit_context) + return if attributes.empty? + + ::Gitlab::Audit::Auditor.audit(created_audit_context) do + attributes.each do |attribute| + event = AuditEvents::BuildService.new( + author: current_user, + scope: root_namespace, + target: attribute, + created_at: Time.current, + message: "Created security attribute #{attribute.name}", + additional_details: { + name: AUDIT_EVENT_NAME, + attribute_name: attribute.name, + attribute_description: attribute.description, + attribute_color: attribute.color, + category_name: category.name + } + ).execute + + ::Gitlab::Audit::EventQueue.push(event) + end end end + + def created_audit_context + { + author: current_user, + scope: root_namespace, + target: root_namespace, + name: AUDIT_EVENT_NAME + } + end end end end diff --git a/ee/app/services/security/attributes/update_project_attributes_service.rb b/ee/app/services/security/attributes/update_project_attributes_service.rb index 84f3b7ae4f94bb..4ef5c4b2e37ca4 100644 --- a/ee/app/services/security/attributes/update_project_attributes_service.rb +++ b/ee/app/services/security/attributes/update_project_attributes_service.rb @@ -7,6 +7,8 @@ class UpdateProjectAttributesService < BaseService MAX_PROJECT_ATTRIBUTES = 20 MAX_ATTRIBUTES = MAX_PROJECT_ATTRIBUTES * 2 + ATTACHED_AUDIT_EVENT_NAME = 'security_attribute_attached_to_project' + DETACHED_AUDIT_EVENT_NAME = 'security_attribute_detached_from_project' def initialize(project:, current_user:, params:) @project = project @@ -175,50 +177,77 @@ def validate_one_attribute_per_category(attribute) end def create_audit_events - create_attach_audit_events - create_detach_audit_events - end - - def create_attach_audit_events - associations_to_create.each do |association| - attribute = association.security_attribute - audit_context = { - name: 'security_attribute_attached_to_project', - author: current_user, - scope: project, - target: attribute, - message: "Attached security attribute #{attribute.name} to project #{project.name}", - additional_details: { - attribute_name: attribute.name, - category_name: attribute.security_category.name, - project_name: project.name, - project_path: project.full_path - } - } + if associations_to_create.any? + ::Gitlab::Audit::Auditor.audit(attach_audit_context) do + associations_to_create.each do |association| + event = build_attached_audit_event(association) + ::Gitlab::Audit::EventQueue.push(event) + end + end + end + + return if associations_to_destroy.none? - ::Gitlab::Audit::Auditor.audit(audit_context) + ::Gitlab::Audit::Auditor.audit(detach_audit_context) do + associations_to_destroy.each do |association| + event = build_detached_audit_event(association) + ::Gitlab::Audit::EventQueue.push(event) + end end end - def create_detach_audit_events - associations_to_destroy.each do |association| - attribute = association.security_attribute - audit_context = { - name: 'security_attribute_detached_from_project', - author: current_user, - scope: project, - target: attribute, - message: "Detached security attribute #{attribute.name} from project #{project.name}", - additional_details: { - attribute_name: attribute.name, - category_name: attribute.security_category.name, - project_name: project.name, - project_path: project.full_path - } + def attached_audit_context + { + author: current_user, + scope: project, + target: project, + name: ATTACHED_AUDIT_EVENT_NAME + } + end + + def detached_audit_context + { + author: current_user, + scope: project, + target: project, + name: DETACHED_AUDIT_EVENT_NAME + } + end + + def build_attach_audit_event(association) + attribute = association.security_attribute + AuditEvents::BuildService.new( + author: current_user, + scope: project, + target: attribute, + created_at: now, + message: "Attached security attribute #{attribute.name} to project #{project.name}", + additional_details: { + name: ATTACHED_AUDIT_EVENT_NAME, + attribute_name: attribute.name, + category_name: attribute.security_category.name, + project_name: project.name, + project_path: project.full_path } - - ::Gitlab::Audit::Auditor.audit(audit_context) - end + ).execute + end + + def build_detach_audit_event(association) + attribute = association.security_attribute + AuditEvents::BuildService.new( + author: current_user, + scope: project, + target: attribute, + created_at: now, + message: "Detached security attribute #{attribute.name} from project #{project.name}", + additional_details: { + name: DETACHED_AUDIT_EVENT_NAME, + attribute_name: attribute.name, + category_name: attribute.security_category.name, + project_name: project.name, + project_path: project.full_path + } + ).execute end end end diff --git a/ee/spec/services/security/attributes/create_service_spec.rb b/ee/spec/services/security/attributes/create_service_spec.rb index fe102ccc766bd3..af13db7d4cfa70 100644 --- a/ee/spec/services/security/attributes/create_service_spec.rb +++ b/ee/spec/services/security/attributes/create_service_spec.rb @@ -82,12 +82,23 @@ expect(execute).to be_success end - it 'creates audit events for each attribute' do + it 'creates audit events for each attribute', :request_store do + expect(::Gitlab::Audit::Auditor).to receive(:audit).with({ + author: user, + scope: namespace, + target: namespace, + name: 'security_attribute_created' + }).and_call_original + + expect(::Gitlab::Audit::EventQueue).to receive(:push).twice.with( + satisfy { |event| security_attribute_created_event?(event) } + ).and_call_original + expect { execute }.to change { AuditEvent.count }.by(2) audit_events = AuditEvent.last(2) audit_events.each do |audit_event| - expect(audit_event.details[:event_name]).to eq('security_attribute_created') + expect(audit_event.details[:name]).to eq('security_attribute_created') expect(audit_event.details[:author_name]).to eq(user.name) expect(audit_event.details[:category_name]).to eq(category.name) end @@ -196,4 +207,8 @@ end end end + + def security_attribute_created_event?(event) + event.is_a?(AuditEvent) && event.details[:name] == 'security_attribute_created' + end end diff --git a/ee/spec/services/security/attributes/update_project_attributes_service_spec.rb b/ee/spec/services/security/attributes/update_project_attributes_service_spec.rb index b83517b096ac12..ebf4a05eaf70c6 100644 --- a/ee/spec/services/security/attributes/update_project_attributes_service_spec.rb +++ b/ee/spec/services/security/attributes/update_project_attributes_service_spec.rb @@ -177,12 +177,23 @@ def expect_error_with_payload_errors(*errors) expect(execute.payload[:removed_count]).to eq(0) end - it 'creates audit events for attached attributes' do + it 'creates audit events for attached attributes', :request_store do + expect(::Gitlab::Audit::Auditor).to receive(:audit).with({ + author: user, + scope: project, + target: project, + name: 'security_attribute_attached_to_project' + }).and_call_original + + expect(::Gitlab::Audit::EventQueue).to receive(:push).twice.with( + satisfy { |event| security_attribute_attached_event?(event) } + ).and_call_original + expect { execute }.to change { AuditEvent.count }.by(2) audit_events = AuditEvent.last(2) audit_events.each do |audit_event| - expect(audit_event.details[:event_name]).to eq('security_attribute_attached_to_project') + expect(audit_event.details[:name]).to eq('security_attribute_attached_to_project') expect(audit_event.details[:author_name]).to eq(user.name) expect(audit_event.details[:project_name]).to eq(project.name) expect(audit_event.details[:project_path]).to eq(project.full_path) @@ -221,11 +232,22 @@ def expect_error_with_payload_errors(*errors) expect(execute.payload[:removed_count]).to eq(1) end - it 'creates audit event for detached attribute' do + it 'creates audit event for detached attribute', :request_store do + expect(::Gitlab::Audit::Auditor).to receive(:audit).with({ + author: user, + scope: project, + target: project, + name: 'security_attribute_detached_from_project' + }).and_call_original + + expect(::Gitlab::Audit::EventQueue).to receive(:push).once.with( + satisfy { |event| security_attribute_detached_event?(event) } + ).and_call_original + expect { execute }.to change { AuditEvent.count }.by(1) audit_event = AuditEvent.last - expect(audit_event.details[:event_name]).to eq('security_attribute_detached_from_project') + expect(audit_event.details[:name]).to eq('security_attribute_detached_from_project') expect(audit_event.details[:author_name]).to eq(user.name) expect(audit_event.details[:custom_message]).to eq( "Detached security attribute #{other_attribute.name} from project #{project.name}" @@ -429,4 +451,12 @@ def expect_error_with_payload_errors(*errors) it_behaves_like 'does not change project security attributes count' end end + + def security_attribute_attached_event?(event) + event.is_a?(AuditEvent) && event.details[:name] == 'security_attribute_attached_to_project' + end + + def security_attribute_detached_event?(event) + event.is_a?(AuditEvent) && event.details[:name] == 'security_attribute_detached_from_project' + end end -- GitLab From f2e9a95e565f62868e66a09664b5bb002d0a7583 Mon Sep 17 00:00:00 2001 From: Nicolae Rotaru Date: Fri, 17 Oct 2025 09:46:02 +0200 Subject: [PATCH 4/8] Fix typos --- .../attributes/update_project_attributes_service.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ee/app/services/security/attributes/update_project_attributes_service.rb b/ee/app/services/security/attributes/update_project_attributes_service.rb index 4ef5c4b2e37ca4..51a54619f8bcc4 100644 --- a/ee/app/services/security/attributes/update_project_attributes_service.rb +++ b/ee/app/services/security/attributes/update_project_attributes_service.rb @@ -178,7 +178,7 @@ def validate_one_attribute_per_category(attribute) def create_audit_events if associations_to_create.any? - ::Gitlab::Audit::Auditor.audit(attach_audit_context) do + ::Gitlab::Audit::Auditor.audit(attached_audit_context) do associations_to_create.each do |association| event = build_attached_audit_event(association) ::Gitlab::Audit::EventQueue.push(event) @@ -188,7 +188,7 @@ def create_audit_events return if associations_to_destroy.none? - ::Gitlab::Audit::Auditor.audit(detach_audit_context) do + ::Gitlab::Audit::Auditor.audit(detached_audit_context) do associations_to_destroy.each do |association| event = build_detached_audit_event(association) ::Gitlab::Audit::EventQueue.push(event) @@ -214,7 +214,7 @@ def detached_audit_context } end - def build_attach_audit_event(association) + def build_attached_audit_event(association) attribute = association.security_attribute AuditEvents::BuildService.new( author: current_user, @@ -232,7 +232,7 @@ def build_attach_audit_event(association) ).execute end - def build_detach_audit_event(association) + def build_detached_audit_event(association) attribute = association.security_attribute AuditEvents::BuildService.new( author: current_user, -- GitLab From 81264227c067b088efa4a64a96249b65dbe19aa8 Mon Sep 17 00:00:00 2001 From: Nicolae Rotaru Date: Fri, 17 Oct 2025 14:45:16 +0200 Subject: [PATCH 5/8] Improve consistency, fix colors saving --- .../security/attributes/create_service.rb | 4 ++-- .../security/attributes/destroy_service.rb | 4 +++- .../update_project_attributes_service.rb | 4 ++-- .../security/attributes/update_service.rb | 21 ++++++++++++------- .../attributes/create_service_spec.rb | 8 +++---- .../update_project_attributes_service_spec.rb | 8 +++---- .../attributes/update_service_spec.rb | 13 ++++++++---- 7 files changed, 38 insertions(+), 24 deletions(-) diff --git a/ee/app/services/security/attributes/create_service.rb b/ee/app/services/security/attributes/create_service.rb index 49a725219df224..fba355a2447c01 100644 --- a/ee/app/services/security/attributes/create_service.rb +++ b/ee/app/services/security/attributes/create_service.rb @@ -76,10 +76,10 @@ def create_audit_events created_at: Time.current, message: "Created security attribute #{attribute.name}", additional_details: { - name: AUDIT_EVENT_NAME, + event_name: AUDIT_EVENT_NAME, attribute_name: attribute.name, attribute_description: attribute.description, - attribute_color: attribute.color, + attribute_color: attribute.color.to_s, category_name: category.name } ).execute diff --git a/ee/app/services/security/attributes/destroy_service.rb b/ee/app/services/security/attributes/destroy_service.rb index 3bb4647c0e1e64..0dcb7f1d793aa3 100644 --- a/ee/app/services/security/attributes/destroy_service.rb +++ b/ee/app/services/security/attributes/destroy_service.rb @@ -3,6 +3,8 @@ module Security module Attributes class DestroyService < BaseService + AUDIT_EVENT_NAME = 'security_attribute_deleted' + def initialize(attribute:, current_user:) @attribute = attribute @current_user = current_user @@ -65,7 +67,7 @@ def create_audit_event root_namespace = attribute.security_category.namespace&.root_ancestor audit_context = { - name: 'security_attribute_deleted', + name: AUDIT_EVENT_NAME, author: current_user, scope: root_namespace, target: attribute, diff --git a/ee/app/services/security/attributes/update_project_attributes_service.rb b/ee/app/services/security/attributes/update_project_attributes_service.rb index 51a54619f8bcc4..b00037055447d5 100644 --- a/ee/app/services/security/attributes/update_project_attributes_service.rb +++ b/ee/app/services/security/attributes/update_project_attributes_service.rb @@ -223,7 +223,7 @@ def build_attached_audit_event(association) created_at: now, message: "Attached security attribute #{attribute.name} to project #{project.name}", additional_details: { - name: ATTACHED_AUDIT_EVENT_NAME, + event_name: ATTACHED_AUDIT_EVENT_NAME, attribute_name: attribute.name, category_name: attribute.security_category.name, project_name: project.name, @@ -241,7 +241,7 @@ def build_detached_audit_event(association) created_at: now, message: "Detached security attribute #{attribute.name} from project #{project.name}", additional_details: { - name: DETACHED_AUDIT_EVENT_NAME, + event_name: DETACHED_AUDIT_EVENT_NAME, attribute_name: attribute.name, category_name: attribute.security_category.name, project_name: project.name, diff --git a/ee/app/services/security/attributes/update_service.rb b/ee/app/services/security/attributes/update_service.rb index 1a1c860ca99f44..3cf0ec177f9222 100644 --- a/ee/app/services/security/attributes/update_service.rb +++ b/ee/app/services/security/attributes/update_service.rb @@ -3,6 +3,8 @@ module Security module Attributes class UpdateService < BaseService + AUDIT_EVENT_NAME = 'security_attribute_updated' + def initialize(attribute:, params:, current_user:) @attribute = attribute @root_namespace = attribute.namespace&.root_ancestor @@ -18,11 +20,10 @@ def execute return UnauthorizedError unless permitted? return ServiceResponse.error(message: 'Cannot update non editable attribute') unless attribute.editable? - update_params = params.slice(:name, :description, :color) - attribute.assign_attributes(update_params) + attribute.assign_attributes(params.slice(:name, :description, :color)) if attribute.save - create_audit_event(update_params) + create_audit_event success else error @@ -48,18 +49,24 @@ def error ServiceResponse.error(message: message, payload: errors) end - def create_audit_event(update_params) + def create_audit_event audit_context = { - name: 'security_attribute_updated', + name: AUDIT_EVENT_NAME, author: current_user, scope: root_namespace, target: attribute, message: "Updated security attribute #{attribute.name}", additional_details: { attribute_name: attribute.name, + attribute_description: attribute.description, + attribute_color: attribute.color.to_s, category_name: attribute.security_category.name, - updated_fields: update_params.keys - }.merge(update_params) + previous_values: { + name: attribute.name_previously_was, + description: attribute.description_previously_was, + color: attribute.color_previously_was.to_s + } + } } ::Gitlab::Audit::Auditor.audit(audit_context) diff --git a/ee/spec/services/security/attributes/create_service_spec.rb b/ee/spec/services/security/attributes/create_service_spec.rb index af13db7d4cfa70..49767071e57ee1 100644 --- a/ee/spec/services/security/attributes/create_service_spec.rb +++ b/ee/spec/services/security/attributes/create_service_spec.rb @@ -98,7 +98,7 @@ audit_events = AuditEvent.last(2) audit_events.each do |audit_event| - expect(audit_event.details[:name]).to eq('security_attribute_created') + expect(audit_event.details[:event_name]).to eq('security_attribute_created') expect(audit_event.details[:author_name]).to eq(user.name) expect(audit_event.details[:category_name]).to eq(category.name) end @@ -106,12 +106,12 @@ expect(audit_events.first.details[:custom_message]).to eq("Created security attribute Critical") expect(audit_events.first.details[:attribute_name]).to eq('Critical') expect(audit_events.first.details[:attribute_description]).to eq('Critical security level') - expect(audit_events.first.details[:attribute_color]).to eq(::Gitlab::Color.of('#FF0000')) + expect(audit_events.first.details[:attribute_color]).to eq('#FF0000') expect(audit_events.second.details[:custom_message]).to eq("Created security attribute High") expect(audit_events.second.details[:attribute_name]).to eq('High') expect(audit_events.second.details[:attribute_description]).to eq('High security level') - expect(audit_events.second.details[:attribute_color]).to eq(::Gitlab::Color.of('#FF8C00')) + expect(audit_events.second.details[:attribute_color]).to eq('#FF8C00') end context 'when attribute validation fails' do @@ -209,6 +209,6 @@ end def security_attribute_created_event?(event) - event.is_a?(AuditEvent) && event.details[:name] == 'security_attribute_created' + event.is_a?(AuditEvent) && event.details[:event_name] == 'security_attribute_created' end end diff --git a/ee/spec/services/security/attributes/update_project_attributes_service_spec.rb b/ee/spec/services/security/attributes/update_project_attributes_service_spec.rb index ebf4a05eaf70c6..267ceab016fa32 100644 --- a/ee/spec/services/security/attributes/update_project_attributes_service_spec.rb +++ b/ee/spec/services/security/attributes/update_project_attributes_service_spec.rb @@ -193,7 +193,7 @@ def expect_error_with_payload_errors(*errors) audit_events = AuditEvent.last(2) audit_events.each do |audit_event| - expect(audit_event.details[:name]).to eq('security_attribute_attached_to_project') + expect(audit_event.details[:event_name]).to eq('security_attribute_attached_to_project') expect(audit_event.details[:author_name]).to eq(user.name) expect(audit_event.details[:project_name]).to eq(project.name) expect(audit_event.details[:project_path]).to eq(project.full_path) @@ -247,7 +247,7 @@ def expect_error_with_payload_errors(*errors) expect { execute }.to change { AuditEvent.count }.by(1) audit_event = AuditEvent.last - expect(audit_event.details[:name]).to eq('security_attribute_detached_from_project') + expect(audit_event.details[:event_name]).to eq('security_attribute_detached_from_project') expect(audit_event.details[:author_name]).to eq(user.name) expect(audit_event.details[:custom_message]).to eq( "Detached security attribute #{other_attribute.name} from project #{project.name}" @@ -453,10 +453,10 @@ def expect_error_with_payload_errors(*errors) end def security_attribute_attached_event?(event) - event.is_a?(AuditEvent) && event.details[:name] == 'security_attribute_attached_to_project' + event.is_a?(AuditEvent) && event.details[:event_name] == 'security_attribute_attached_to_project' end def security_attribute_detached_event?(event) - event.is_a?(AuditEvent) && event.details[:name] == 'security_attribute_detached_from_project' + event.is_a?(AuditEvent) && event.details[:event_name] == 'security_attribute_detached_from_project' end end diff --git a/ee/spec/services/security/attributes/update_service_spec.rb b/ee/spec/services/security/attributes/update_service_spec.rb index 2de6bb47e3a5ff..3cdbc27d294aaf 100644 --- a/ee/spec/services/security/attributes/update_service_spec.rb +++ b/ee/spec/services/security/attributes/update_service_spec.rb @@ -74,11 +74,16 @@ expect(audit_event.details[:author_name]).to eq(user.name) expect(audit_event.details[:custom_message]).to eq("Updated security attribute Updated Name") expect(audit_event.details[:attribute_name]).to eq('Updated Name') + expect(audit_event.details[:attribute_description]).to eq('Updated Description') + expect(audit_event.details[:attribute_color]).to eq('#00FF00') expect(audit_event.details[:category_name]).to eq(category.name) - expect(audit_event.details[:updated_fields]).to eq(%i[name description color]) - expect(audit_event.details[:name]).to eq('Updated Name') - expect(audit_event.details[:description]).to eq('Updated Description') - expect(audit_event.details[:color]).to eq('#00FF00') + expect(audit_event.details[:previous_values]).to eq( + { + name: 'Original Name', + description: 'Original Description', + color: '#FF0000' + } + ) end context 'when attribute is not editable' do -- GitLab From 7b9bbefc8ae1c70a60de3b16f151390e8ac4c783 Mon Sep 17 00:00:00 2001 From: Nicolae Rotaru Date: Mon, 20 Oct 2025 14:32:28 +0200 Subject: [PATCH 6/8] Address feedback: add missing spec, refactor spec --- .../attributes/destroy_service_spec.rb | 5 +++++ .../security/attributes/update_service_spec.rb | 18 +++++++++--------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/ee/spec/services/security/attributes/destroy_service_spec.rb b/ee/spec/services/security/attributes/destroy_service_spec.rb index b9648f8cc533a5..eed4dc15f24e8b 100644 --- a/ee/spec/services/security/attributes/destroy_service_spec.rb +++ b/ee/spec/services/security/attributes/destroy_service_spec.rb @@ -93,6 +93,11 @@ expect(audit_event.details[:category_name]).to eq(category.name) end + it 'does not create an audit event when deletion fails' do + allow(attribute).to receive(:destroy).and_return(false) + expect { execute }.not_to change { AuditEvent.count } + end + it 'does not enqueue worker when deletion fails' do allow(attribute).to receive(:destroy).and_return(false) diff --git a/ee/spec/services/security/attributes/update_service_spec.rb b/ee/spec/services/security/attributes/update_service_spec.rb index 3cdbc27d294aaf..34cfdbadf32d73 100644 --- a/ee/spec/services/security/attributes/update_service_spec.rb +++ b/ee/spec/services/security/attributes/update_service_spec.rb @@ -70,15 +70,15 @@ expect { execute }.to change { AuditEvent.count }.by(1) audit_event = AuditEvent.last - expect(audit_event.details[:event_name]).to eq('security_attribute_updated') - expect(audit_event.details[:author_name]).to eq(user.name) - expect(audit_event.details[:custom_message]).to eq("Updated security attribute Updated Name") - expect(audit_event.details[:attribute_name]).to eq('Updated Name') - expect(audit_event.details[:attribute_description]).to eq('Updated Description') - expect(audit_event.details[:attribute_color]).to eq('#00FF00') - expect(audit_event.details[:category_name]).to eq(category.name) - expect(audit_event.details[:previous_values]).to eq( - { + expect(audit_event.details).to include( + event_name: 'security_attribute_updated', + author_name: user.name, + custom_message: "Updated security attribute Updated Name", + attribute_name: 'Updated Name', + attribute_description: 'Updated Description', + attribute_color: '#00FF00', + category_name: category.name, + previous_values: { name: 'Original Name', description: 'Original Description', color: '#FF0000' -- GitLab From f5a2e029ac821e2d0341bf37f9daf394e8ca2fd7 Mon Sep 17 00:00:00 2001 From: Nicolae Rotaru Date: Mon, 20 Oct 2025 16:39:02 +0200 Subject: [PATCH 7/8] Update spec assertion format --- .../attributes/create_service_spec.rb | 30 +++++++------ .../attributes/destroy_service_spec.rb | 14 ++++--- .../update_project_attributes_service_spec.rb | 42 ++++++++++--------- .../categories/create_service_spec.rb | 14 ++++--- .../categories/destroy_service_spec.rb | 15 ++++--- .../categories/update_service_spec.rb | 17 ++++---- 6 files changed, 75 insertions(+), 57 deletions(-) diff --git a/ee/spec/services/security/attributes/create_service_spec.rb b/ee/spec/services/security/attributes/create_service_spec.rb index 49767071e57ee1..00b2668f5555e7 100644 --- a/ee/spec/services/security/attributes/create_service_spec.rb +++ b/ee/spec/services/security/attributes/create_service_spec.rb @@ -98,20 +98,26 @@ audit_events = AuditEvent.last(2) audit_events.each do |audit_event| - expect(audit_event.details[:event_name]).to eq('security_attribute_created') - expect(audit_event.details[:author_name]).to eq(user.name) - expect(audit_event.details[:category_name]).to eq(category.name) + expect(audit_event.details).to include( + event_name: 'security_attribute_created', + author_name: user.name, + category_name: category.name + ) end - expect(audit_events.first.details[:custom_message]).to eq("Created security attribute Critical") - expect(audit_events.first.details[:attribute_name]).to eq('Critical') - expect(audit_events.first.details[:attribute_description]).to eq('Critical security level') - expect(audit_events.first.details[:attribute_color]).to eq('#FF0000') - - expect(audit_events.second.details[:custom_message]).to eq("Created security attribute High") - expect(audit_events.second.details[:attribute_name]).to eq('High') - expect(audit_events.second.details[:attribute_description]).to eq('High security level') - expect(audit_events.second.details[:attribute_color]).to eq('#FF8C00') + expect(audit_events.first.details).to include( + custom_message: 'Created security attribute Critical', + attribute_name: 'Critical', + attribute_description: 'Critical security level', + attribute_color: '#FF0000' + ) + + expect(audit_events.second.details).to include( + custom_message: 'Created security attribute High', + attribute_name: 'High', + attribute_description: 'High security level', + attribute_color: '#FF8C00' + ) end context 'when attribute validation fails' do diff --git a/ee/spec/services/security/attributes/destroy_service_spec.rb b/ee/spec/services/security/attributes/destroy_service_spec.rb index eed4dc15f24e8b..7a22268004f1fc 100644 --- a/ee/spec/services/security/attributes/destroy_service_spec.rb +++ b/ee/spec/services/security/attributes/destroy_service_spec.rb @@ -85,12 +85,14 @@ expect { execute }.to change { AuditEvent.count }.by(1) audit_event = AuditEvent.last - expect(audit_event.details[:event_name]).to eq('security_attribute_deleted') - expect(audit_event.details[:author_name]).to eq(user.name) - expect(audit_event.details[:custom_message]).to eq("Deleted security attribute #{attribute.name}") - expect(audit_event.details[:attribute_name]).to eq(attribute.name) - expect(audit_event.details[:attribute_description]).to eq(attribute.description) - expect(audit_event.details[:category_name]).to eq(category.name) + expect(audit_event.details).to include( + event_name: 'security_attribute_deleted', + author_name: user.name, + custom_message: "Deleted security attribute #{attribute.name}", + attribute_name: attribute.name, + attribute_description: attribute.description, + category_name: category.name + ) end it 'does not create an audit event when deletion fails' do diff --git a/ee/spec/services/security/attributes/update_project_attributes_service_spec.rb b/ee/spec/services/security/attributes/update_project_attributes_service_spec.rb index 267ceab016fa32..e69fc166f7ec77 100644 --- a/ee/spec/services/security/attributes/update_project_attributes_service_spec.rb +++ b/ee/spec/services/security/attributes/update_project_attributes_service_spec.rb @@ -193,23 +193,25 @@ def expect_error_with_payload_errors(*errors) audit_events = AuditEvent.last(2) audit_events.each do |audit_event| - expect(audit_event.details[:event_name]).to eq('security_attribute_attached_to_project') - expect(audit_event.details[:author_name]).to eq(user.name) - expect(audit_event.details[:project_name]).to eq(project.name) - expect(audit_event.details[:project_path]).to eq(project.full_path) + expect(audit_event.details).to include( + event_name: 'security_attribute_attached_to_project', + author_name: user.name, + project_name: project.name, + project_path: project.full_path + ) end - expect(audit_events.first.details[:custom_message]).to eq( - "Attached security attribute #{attribute1.name} to project #{project.name}" + expect(audit_events.first.details).to include( + custom_message: "Attached security attribute #{attribute1.name} to project #{project.name}", + attribute_name: attribute1.name, + category_name: single_selection_category.name ) - expect(audit_events.first.details[:attribute_name]).to eq(attribute1.name) - expect(audit_events.first.details[:category_name]).to eq(single_selection_category.name) - expect(audit_events.second.details[:custom_message]).to eq( - "Attached security attribute #{other_attribute.name} to project #{project.name}" + expect(audit_events.second.details).to include( + custom_message: "Attached security attribute #{other_attribute.name} to project #{project.name}", + attribute_name: other_attribute.name, + category_name: other_category.name ) - expect(audit_events.second.details[:attribute_name]).to eq(other_attribute.name) - expect(audit_events.second.details[:category_name]).to eq(other_category.name) end end @@ -247,15 +249,15 @@ def expect_error_with_payload_errors(*errors) expect { execute }.to change { AuditEvent.count }.by(1) audit_event = AuditEvent.last - expect(audit_event.details[:event_name]).to eq('security_attribute_detached_from_project') - expect(audit_event.details[:author_name]).to eq(user.name) - expect(audit_event.details[:custom_message]).to eq( - "Detached security attribute #{other_attribute.name} from project #{project.name}" + expect(audit_event.details).to include( + event_name: 'security_attribute_detached_from_project', + author_name: user.name, + custom_message: "Detached security attribute #{other_attribute.name} from project #{project.name}", + attribute_name: other_attribute.name, + category_name: other_category.name, + project_name: project.name, + project_path: project.full_path ) - expect(audit_event.details[:attribute_name]).to eq(other_attribute.name) - expect(audit_event.details[:category_name]).to eq(other_category.name) - expect(audit_event.details[:project_name]).to eq(project.name) - expect(audit_event.details[:project_path]).to eq(project.full_path) end end diff --git a/ee/spec/services/security/categories/create_service_spec.rb b/ee/spec/services/security/categories/create_service_spec.rb index f102c658c9e4e1..964c6b2d961512 100644 --- a/ee/spec/services/security/categories/create_service_spec.rb +++ b/ee/spec/services/security/categories/create_service_spec.rb @@ -179,12 +179,14 @@ expect { execute }.to change { AuditEvent.count }.by(1) audit_event = AuditEvent.last - expect(audit_event.details[:event_name]).to eq('security_category_created') - expect(audit_event.details[:author_name]).to eq(current_user.name) - expect(audit_event.details[:custom_message]).to eq("Created security category #{params[:name]}") - expect(audit_event.details[:category_name]).to eq(params[:name]) - expect(audit_event.details[:category_description]).to eq(params[:description]) - expect(audit_event.details[:multiple_selection]).to eq(params[:multiple_selection]) + expect(audit_event.details).to include( + event_name: 'security_category_created', + author_name: current_user.name, + custom_message: "Created security category #{params[:name]}", + category_name: params[:name], + category_description: params[:description], + multiple_selection: params[:multiple_selection] + ) end end diff --git a/ee/spec/services/security/categories/destroy_service_spec.rb b/ee/spec/services/security/categories/destroy_service_spec.rb index 28a0e0b83d1dce..5963ff8cbb4543 100644 --- a/ee/spec/services/security/categories/destroy_service_spec.rb +++ b/ee/spec/services/security/categories/destroy_service_spec.rb @@ -112,12 +112,15 @@ expect { execute }.to change { AuditEvent.count }.by(1) audit_event = AuditEvent.last - expect(audit_event.details[:event_name]).to eq('security_category_deleted') - expect(audit_event.details[:author_name]).to eq(user.name) - expect(audit_event.details[:custom_message]).to eq("Deleted security category #{category.name}") - expect(audit_event.details[:category_name]).to eq(category.name) - expect(audit_event.details[:category_description]).to eq(category.description) - expect(audit_event.details[:attributes_count]).to eq(1) + + expect(audit_event.details).to include( + event_name: 'security_category_deleted', + author_name: user.name, + custom_message: "Deleted security category #{category.name}", + category_name: category.name, + category_description: category.description, + attributes_count: 1 + ) end end diff --git a/ee/spec/services/security/categories/update_service_spec.rb b/ee/spec/services/security/categories/update_service_spec.rb index c5ba4da4b8f19d..d02727b3c0aef4 100644 --- a/ee/spec/services/security/categories/update_service_spec.rb +++ b/ee/spec/services/security/categories/update_service_spec.rb @@ -131,13 +131,16 @@ expect { execute }.to change { AuditEvent.count }.by(1) audit_event = AuditEvent.last - expect(audit_event.details[:event_name]).to eq('security_category_updated') - expect(audit_event.details[:author_name]).to eq(current_user.name) - expect(audit_event.details[:custom_message]).to eq("Updated security category #{params[:name]}") - expect(audit_event.details[:category_name]).to eq(params[:name]) - expect(audit_event.details[:updated_fields]).to eq(%i[name description]) - expect(audit_event.details[:name]).to eq(params[:name]) - expect(audit_event.details[:description]).to eq(params[:description]) + + expect(audit_event.details).to include( + event_name: 'security_category_updated', + author_name: current_user.name, + custom_message: "Updated security category #{params[:name]}", + category_name: params[:name], + updated_fields: %i[name description], + name: params[:name], + description: params[:description] + ) end end -- GitLab From 664352b5073a178f00627491ef085d342aec3db8 Mon Sep 17 00:00:00 2001 From: Nicolae Rotaru Date: Thu, 23 Oct 2025 10:09:16 +0200 Subject: [PATCH 8/8] Fix coverage: add missing specs --- .../attributes/create_service_spec.rb | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/ee/spec/services/security/attributes/create_service_spec.rb b/ee/spec/services/security/attributes/create_service_spec.rb index 00b2668f5555e7..91a91f83d90d54 100644 --- a/ee/spec/services/security/attributes/create_service_spec.rb +++ b/ee/spec/services/security/attributes/create_service_spec.rb @@ -210,6 +210,45 @@ expect(execute.message).to include("Description can't be blank") end end + + context 'when category is not editable' do + before do + category.update!(editable_state: :locked) + end + + it 'returns non-editable error' do + expect(execute).to be_error + expect(execute.message).to eq("You can not edit this category's attributes.") + end + end + + context 'when category save fails' do + before do + # Mock the category to fail validation but allow other methods + allow(category).to receive_messages(valid?: true, save: false) + allow(category.errors).to receive(:full_messages).and_return(['Category validation failed']) + end + + it 'returns error with category errors' do + expect(execute).to be_error + expect(execute.message).to include('Failed to create security attributes') + expect(execute.message).to include('Category validation failed') + end + end + + context 'when no attributes are provided' do + let(:params) { { attributes: [] } } + + it 'succeeds with empty attributes' do + expect(execute).to be_success + expect(execute.payload[:attributes]).to be_empty + end + + it 'does not create audit events when no attributes' do + expect(::Gitlab::Audit::Auditor).not_to receive(:audit) + expect(execute).to be_success + end + end end end end -- GitLab