From 78cb8f13ad900b187f8467696dda7e7ab342c1c8 Mon Sep 17 00:00:00 2001 From: Hitesh Raghuvanshi Date: Mon, 30 Jun 2025 21:06:04 +0530 Subject: [PATCH 1/3] Update project violation status Changelog: added EE: true --- doc/api/graphql/reference/_index.md | 25 +++ doc/user/compliance/audit_event_types.md | 1 + ee/app/graphql/ee/types/mutation_type.rb | 1 + .../projects/compliance_violations/update.rb | 58 ++++++ .../update_project_compliance_violation.yml | 10 ++ .../compliance_violations/update_spec.rb | 81 +++++++++ .../compliance_violations/update_spec.rb | 167 ++++++++++++++++++ 7 files changed, 343 insertions(+) create mode 100644 ee/app/graphql/mutations/compliance_management/projects/compliance_violations/update.rb create mode 100644 ee/config/audit_events/types/update_project_compliance_violation.yml create mode 100644 ee/spec/graphql/mutations/compliance_management/projects/compliance_violations/update_spec.rb create mode 100644 ee/spec/requests/api/graphql/mutations/compliance_management/projects/compliance_violations/update_spec.rb diff --git a/doc/api/graphql/reference/_index.md b/doc/api/graphql/reference/_index.md index 6f8ad6ccf6bb9a..dc95b896a9ea28 100644 --- a/doc/api/graphql/reference/_index.md +++ b/doc/api/graphql/reference/_index.md @@ -12191,6 +12191,31 @@ Input type: `UpdatePackagesProtectionRuleInput` | `errors` | [`[String!]!`](#string) | Errors encountered during the mutation. | | `packageProtectionRule` | [`PackagesProtectionRule`](#packagesprotectionrule) | Packages protection rule after mutation. | +### `Mutation.updateProjectComplianceViolation` + +{{< details >}} +**Introduced** in GitLab 18.2. +**Status**: Experiment. +{{< /details >}} + +Input type: `UpdateProjectComplianceViolationInput` + +#### Arguments + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| `id` | [`ComplianceManagementProjectsComplianceViolationID!`](#compliancemanagementprojectscomplianceviolationid) | Global ID of the project compliance violation to update. | +| `status` | [`ComplianceViolationStatus!`](#complianceviolationstatus) | New status for the project compliance violation. | + +#### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| `complianceViolation` | [`ProjectComplianceViolation`](#projectcomplianceviolation) | Compliance violation after status update. | +| `errors` | [`[String!]!`](#string) | Errors encountered during the mutation. | + ### `Mutation.updateRequirement` Input type: `UpdateRequirementInput` diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index 85a789aa9bf080..3c9fbdd6aef471 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -216,6 +216,7 @@ Audit event types belong to the following product categories. | [`update_approval_rules`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/89939) | Updating a merge approval rule | {{< icon name="check-circle" >}} Yes | GitLab [15.2](https://gitlab.com/gitlab-org/gitlab/-/issues/363092) | Project | | [`update_compliance_framework`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74292) | A compliance framework is updated | {{< icon name="check-circle" >}} Yes | GitLab [14.6](https://gitlab.com/gitlab-org/gitlab/-/issues/340649) | Group | | [`update_compliance_requirement`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/169485) | A compliance framework requirement is updated. | {{< icon name="check-circle" >}} Yes | GitLab [17.7](https://gitlab.com/gitlab-org/gitlab/-/issues/470695) | Group | +| [`update_project_compliance_violation`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/195768) | Project compliance violation is updated | {{< icon name="check-circle" >}} Yes | GitLab [18.2](https://gitlab.com/gitlab-org/gitlab/-/issues/542343) | Project | | [`update_status_check`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/84624) | An external status check is updated | {{< icon name="check-circle" >}} Yes | GitLab [15.9](https://gitlab.com/gitlab-org/gitlab/-/issues/355805) | Project | | [`updated_compliance_requirement_control`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/177557) | Compliance requirement control updated. | {{< icon name="check-circle" >}} Yes | GitLab [17.9](https://gitlab.com/gitlab-org/gitlab/-/issues/512381) | Group | diff --git a/ee/app/graphql/ee/types/mutation_type.rb b/ee/app/graphql/ee/types/mutation_type.rb index d6adc01483127c..c126b3c7d8a3d9 100644 --- a/ee/app/graphql/ee/types/mutation_type.rb +++ b/ee/app/graphql/ee/types/mutation_type.rb @@ -320,6 +320,7 @@ def self.authorization_scopes mount_mutation ::Mutations::WorkItems::Lifecycles::Update, experiment: { milestone: '18.1' } mount_mutation ::Mutations::VirtualRegistries::Packages::Maven::MavenUpstreamCreateMutation, experiment: { milestone: '18.2' } + mount_mutation ::Mutations::ComplianceManagement::Projects::ComplianceViolations::Update, experiment: { milestone: '18.2' } prepend(Types::DeprecatedMutations) end diff --git a/ee/app/graphql/mutations/compliance_management/projects/compliance_violations/update.rb b/ee/app/graphql/mutations/compliance_management/projects/compliance_violations/update.rb new file mode 100644 index 00000000000000..e8176e54fac2fb --- /dev/null +++ b/ee/app/graphql/mutations/compliance_management/projects/compliance_violations/update.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +module Mutations + module ComplianceManagement + module Projects + module ComplianceViolations + class Update < ::Mutations::BaseMutation + graphql_name 'UpdateProjectComplianceViolation' + + authorize :read_compliance_violations_report + + argument :id, + ::Types::GlobalIDType[::ComplianceManagement::Projects::ComplianceViolation], + required: true, + description: 'Global ID of the project compliance violation to update.' + + argument :status, ::Types::ComplianceManagement::Projects::ComplianceViolationStatusEnum, + required: true, + description: 'New status for the project compliance violation.' + + field :compliance_violation, + ::Types::ComplianceManagement::Projects::ComplianceViolationType, + null: true, + description: "Compliance violation after status update." + + def resolve(id:, **args) + violation = authorized_find!(id: id) + + audit_event(violation) if violation.update(status: args[:status]) + + { compliance_violation: violation, errors: errors_on_object(violation) } + end + + private + + def audit_event(violation) + old_status = violation.status_before_last_save + new_status = violation.status + + audit_context = { + name: 'update_project_compliance_violation', + author: current_user, + scope: violation.project, + target: violation, + message: "Changed project compliance violation's status from #{old_status} to #{new_status}", + additional_details: { + old_status: old_status, + new_status: new_status + } + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end + end + end + end + end +end diff --git a/ee/config/audit_events/types/update_project_compliance_violation.yml b/ee/config/audit_events/types/update_project_compliance_violation.yml new file mode 100644 index 00000000000000..1130b484b95da9 --- /dev/null +++ b/ee/config/audit_events/types/update_project_compliance_violation.yml @@ -0,0 +1,10 @@ +--- +name: update_project_compliance_violation +description: Project compliance violation is updated +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/542343 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/195768 +feature_category: compliance_management +milestone: '18.2' +saved_to_database: true +streamed: true +scope: [Project] diff --git a/ee/spec/graphql/mutations/compliance_management/projects/compliance_violations/update_spec.rb b/ee/spec/graphql/mutations/compliance_management/projects/compliance_violations/update_spec.rb new file mode 100644 index 00000000000000..ce87dffc31da45 --- /dev/null +++ b/ee/spec/graphql/mutations/compliance_management/projects/compliance_violations/update_spec.rb @@ -0,0 +1,81 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Mutations::ComplianceManagement::Projects::ComplianceViolations::Update, + feature_category: :compliance_management do + include GraphqlHelpers + + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:current_user) { create(:user) } + let_it_be(:compliance_violation) { create(:project_compliance_violation, project: project, namespace: group) } + + let(:mutation) { described_class.new(object: nil, context: query_context, field: nil) } + + before_all do + group.add_owner(current_user) + end + + describe '#resolve' do + context 'when feature is licensed' do + before do + stub_licensed_features(group_level_compliance_violations_report: true) + end + + context 'when user is authorized' do + context 'when updating status successfully' do + it 'updates the violation status' do + result = mutation.resolve(id: compliance_violation.to_global_id, status: 'in_review') + + expect(result[:compliance_violation].status).to eq('in_review') + expect(result[:errors]).to be_empty + end + + it 'persists the status change' do + mutation.resolve(id: compliance_violation.to_global_id, status: 'resolved') + + expect(compliance_violation.reload.status).to eq('resolved') + end + end + + context 'when validation fails' do + before do + allow_next_instance_of(ComplianceManagement::Projects::ComplianceViolation) do |violation| + allow(violation).to receive_messages(update: false, + errors: instance_double(ActiveModel::Errors, full_messages: ['Status is invalid'])) + end + end + + it 'returns validation errors' do + expect do + mutation.resolve(id: compliance_violation.to_global_id, status: 'invalid_status') + end.to raise_error(ArgumentError) + end + end + end + + context 'when user is not authorized' do + before_all do + group.add_maintainer(current_user) + end + + it 'raises authorization error' do + expect { mutation.resolve(id: compliance_violation.to_global_id, status: 'in_review') } + .to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) + end + end + end + + context 'when feature is not licensed' do + before do + stub_licensed_features(group_level_compliance_violations_report: false) + end + + it 'raises authorization error' do + expect { mutation.resolve(id: compliance_violation.to_global_id, status: 'in_review') } + .to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) + end + end + end +end diff --git a/ee/spec/requests/api/graphql/mutations/compliance_management/projects/compliance_violations/update_spec.rb b/ee/spec/requests/api/graphql/mutations/compliance_management/projects/compliance_violations/update_spec.rb new file mode 100644 index 00000000000000..da46761932cccf --- /dev/null +++ b/ee/spec/requests/api/graphql/mutations/compliance_management/projects/compliance_violations/update_spec.rb @@ -0,0 +1,167 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'UpdateProjectComplianceViolation', feature_category: :compliance_management do + include GraphqlHelpers + + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:user) { create(:user) } + let_it_be(:compliance_violation) { create(:project_compliance_violation, project: project, namespace: group) } + + let(:mutation) do + graphql_mutation( + :update_project_compliance_violation, + { + id: compliance_violation.to_global_id.to_s, + status: new_status + } + ) + end + + let(:mutation_response) { graphql_mutation_response(:update_project_compliance_violation) } + + let(:error_message) do + "The resource that you are attempting to access does not exist or you don't have permission to perform this action" + end + + before do + stub_licensed_features(group_level_compliance_violations_report: true) + end + + before_all do + group.add_owner(user) + end + + context 'when user has permission to read compliance violations report' do + context 'with valid parameters' do + let(:new_status) { 'IN_REVIEW' } + + it 'updates the compliance violation status' do + post_graphql_mutation(mutation, current_user: user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['complianceViolation']['id']).to eq(compliance_violation.to_global_id.to_s) + expect(mutation_response['errors']).to be_empty + + expect(compliance_violation.reload.status).to eq('in_review') + end + + it 'returns the updated compliance violation' do + post_graphql_mutation(mutation, current_user: user) + + expect(mutation_response['complianceViolation']).to include( + 'id' => compliance_violation.to_global_id.to_s, + 'status' => new_status + ) + end + + it 'audits the change' do + expect { post_graphql_mutation(mutation, current_user: user) }.to change { + AuditEvent.where("details LIKE '%update_project_compliance_violation%'").count + }.by(1) + end + end + + context 'with different status values' do + %w[DETECTED IN_REVIEW RESOLVED DISMISSED].each do |status| + context "when updating to #{status}" do + let(:new_status) { status } + + it "successfully updates status to #{status}" do + post_graphql_mutation(mutation, current_user: user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['errors']).to be_empty + expect(compliance_violation.reload.status).to eq(status.downcase) + end + end + end + end + + context 'with invalid parameters' do + context 'when compliance violation does not exist' do + let(:mutation) do + graphql_mutation( + :update_project_compliance_violation, + { + id: "gid://gitlab/ComplianceManagement::Projects::ComplianceViolation/#{non_existing_record_id}", + status: 'IN_REVIEW' + } + ) + end + + it 'returns an error' do + post_graphql_mutation(mutation, current_user: user) + + expect(graphql_errors).to include(a_hash_including('message' => error_message)) + end + end + + context 'when status is invalid' do + let(:new_status) { 'INVALID_STATUS' } + + it 'returns a validation error' do + post_graphql_mutation(mutation, current_user: user) + + expect(graphql_errors).to include( + a_hash_including('message' => a_string_matching(/was provided invalid value for status/i)) + ) + end + end + + context 'when compliance violation belongs to different project' do + let_it_be(:other_group) { create(:group) } + let_it_be(:other_project) { create(:project, group: other_group) } + let_it_be(:other_violation) do + create(:project_compliance_violation, project: other_project, namespace: other_group) + end + + let(:mutation) do + graphql_mutation( + :update_project_compliance_violation, + { + id: other_violation.to_global_id.to_s, + status: 'IN_REVIEW' + } + ) + end + + it 'returns an authorization error' do + post_graphql_mutation(mutation, current_user: user) + + expect(graphql_errors).to include(a_hash_including('message' => error_message)) + end + end + end + end + + context 'when user does not have permission' do + let(:new_status) { 'IN_REVIEW' } + + before_all do + group.add_maintainer(user) + end + + it 'returns an authorization error' do + post_graphql_mutation(mutation, current_user: user) + + expect(graphql_errors).to include(a_hash_including('message' => error_message)) + end + end + + context 'when feature is not licensed' do + let(:new_status) { 'IN_REVIEW' } + + before do + stub_licensed_features(group_level_compliance_violations_report: false) + end + + it 'returns an error' do + post_graphql_mutation(mutation, current_user: user) + + expect(graphql_errors).to include(a_hash_including('message' => error_message)) + end + end +end -- GitLab From 9e1a50ed19322d2a804dc648f406d84a846faf29 Mon Sep 17 00:00:00 2001 From: Hitesh Raghuvanshi Date: Mon, 30 Jun 2025 21:33:29 +0530 Subject: [PATCH 2/3] Fixed rubocop offence --- ee/app/graphql/ee/types/mutation_type.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ee/app/graphql/ee/types/mutation_type.rb b/ee/app/graphql/ee/types/mutation_type.rb index c126b3c7d8a3d9..a30d0340e12610 100644 --- a/ee/app/graphql/ee/types/mutation_type.rb +++ b/ee/app/graphql/ee/types/mutation_type.rb @@ -320,7 +320,8 @@ def self.authorization_scopes mount_mutation ::Mutations::WorkItems::Lifecycles::Update, experiment: { milestone: '18.1' } mount_mutation ::Mutations::VirtualRegistries::Packages::Maven::MavenUpstreamCreateMutation, experiment: { milestone: '18.2' } - mount_mutation ::Mutations::ComplianceManagement::Projects::ComplianceViolations::Update, experiment: { milestone: '18.2' } + mount_mutation ::Mutations::ComplianceManagement::Projects::ComplianceViolations::Update, + experiment: { milestone: '18.2' } prepend(Types::DeprecatedMutations) end -- GitLab From 50a1a1cb3d8f7d8ec9b2cd7c9c6264286af80818 Mon Sep 17 00:00:00 2001 From: Hitesh Raghuvanshi Date: Wed, 2 Jul 2025 08:02:21 +0530 Subject: [PATCH 3/3] Not logging audit event in case of no change --- .../projects/compliance_violations/update.rb | 2 + .../compliance_violations/update_spec.rb | 59 +++++++++++++++---- 2 files changed, 48 insertions(+), 13 deletions(-) diff --git a/ee/app/graphql/mutations/compliance_management/projects/compliance_violations/update.rb b/ee/app/graphql/mutations/compliance_management/projects/compliance_violations/update.rb index e8176e54fac2fb..5462493805e3fd 100644 --- a/ee/app/graphql/mutations/compliance_management/projects/compliance_violations/update.rb +++ b/ee/app/graphql/mutations/compliance_management/projects/compliance_violations/update.rb @@ -37,6 +37,8 @@ def audit_event(violation) old_status = violation.status_before_last_save new_status = violation.status + return if old_status == new_status + audit_context = { name: 'update_project_compliance_violation', author: current_user, diff --git a/ee/spec/requests/api/graphql/mutations/compliance_management/projects/compliance_violations/update_spec.rb b/ee/spec/requests/api/graphql/mutations/compliance_management/projects/compliance_violations/update_spec.rb index da46761932cccf..16addb29418114 100644 --- a/ee/spec/requests/api/graphql/mutations/compliance_management/projects/compliance_violations/update_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/compliance_management/projects/compliance_violations/update_spec.rb @@ -6,9 +6,13 @@ include GraphqlHelpers let_it_be(:group) { create(:group) } + let(:error_message) do + "The resource that you are attempting to access does not exist or you don't have permission to perform this action" + end + let_it_be(:project) { create(:project, group: group) } let_it_be(:user) { create(:user) } - let_it_be(:compliance_violation) { create(:project_compliance_violation, project: project, namespace: group) } + let(:compliance_violation) { create(:project_compliance_violation, project: project, namespace: group) } let(:mutation) do graphql_mutation( @@ -22,9 +26,7 @@ let(:mutation_response) { graphql_mutation_response(:update_project_compliance_violation) } - let(:error_message) do - "The resource that you are attempting to access does not exist or you don't have permission to perform this action" - end + subject(:mutate) { post_graphql_mutation(mutation, current_user: user) } before do stub_licensed_features(group_level_compliance_violations_report: true) @@ -35,11 +37,19 @@ end context 'when user has permission to read compliance violations report' do + shared_examples 'does not create audit event' do + it 'does not create any audit event' do + expect { mutate }.not_to change { + AuditEvent.where("details LIKE '%update_project_compliance_violation%'").count + } + end + end + context 'with valid parameters' do let(:new_status) { 'IN_REVIEW' } it 'updates the compliance violation status' do - post_graphql_mutation(mutation, current_user: user) + mutate expect(response).to have_gitlab_http_status(:success) expect(mutation_response['complianceViolation']['id']).to eq(compliance_violation.to_global_id.to_s) @@ -49,7 +59,7 @@ end it 'returns the updated compliance violation' do - post_graphql_mutation(mutation, current_user: user) + mutate expect(mutation_response['complianceViolation']).to include( 'id' => compliance_violation.to_global_id.to_s, @@ -58,10 +68,23 @@ end it 'audits the change' do - expect { post_graphql_mutation(mutation, current_user: user) }.to change { + expect { mutate }.to change { AuditEvent.where("details LIKE '%update_project_compliance_violation%'").count }.by(1) end + + context 'when status is same as previous one' do + let(:new_status) { compliance_violation.status.upcase } + + it 'does return any error' do + mutate + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['errors']).to be_empty + end + + it_behaves_like 'does not create audit event' + end end context 'with different status values' do @@ -70,7 +93,7 @@ let(:new_status) { status } it "successfully updates status to #{status}" do - post_graphql_mutation(mutation, current_user: user) + mutate expect(response).to have_gitlab_http_status(:success) expect(mutation_response['errors']).to be_empty @@ -93,22 +116,26 @@ end it 'returns an error' do - post_graphql_mutation(mutation, current_user: user) + mutate expect(graphql_errors).to include(a_hash_including('message' => error_message)) end + + it_behaves_like 'does not create audit event' end context 'when status is invalid' do let(:new_status) { 'INVALID_STATUS' } it 'returns a validation error' do - post_graphql_mutation(mutation, current_user: user) + mutate expect(graphql_errors).to include( a_hash_including('message' => a_string_matching(/was provided invalid value for status/i)) ) end + + it_behaves_like 'does not create audit event' end context 'when compliance violation belongs to different project' do @@ -129,10 +156,12 @@ end it 'returns an authorization error' do - post_graphql_mutation(mutation, current_user: user) + mutate expect(graphql_errors).to include(a_hash_including('message' => error_message)) end + + it_behaves_like 'does not create audit event' end end end @@ -145,10 +174,12 @@ end it 'returns an authorization error' do - post_graphql_mutation(mutation, current_user: user) + mutate expect(graphql_errors).to include(a_hash_including('message' => error_message)) end + + it_behaves_like 'does not create audit event' end context 'when feature is not licensed' do @@ -159,9 +190,11 @@ end it 'returns an error' do - post_graphql_mutation(mutation, current_user: user) + mutate expect(graphql_errors).to include(a_hash_including('message' => error_message)) end + + it_behaves_like 'does not create audit event' end end -- GitLab