diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index 9d7d2a92d104bb4657cbb59347bea763145997a9..5975d684912528ed5be2655a18b00e7b7591fb50 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -148,6 +148,8 @@ Audit event types belong to the following product categories. | [`allow_committer_approval_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/102256) | Prevent approvals by users who add commits setting is updated | {{< icon name="check-circle" >}} Yes | GitLab [15.6](https://gitlab.com/gitlab-org/gitlab/-/issues/373949) | Group | | [`allow_overrides_to_approver_list_per_merge_request_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/102256) | Prevent editing approval rules in projects and merge requests setting is updated | {{< icon name="check-circle" >}} Yes | GitLab [15.6](https://gitlab.com/gitlab-org/gitlab/-/issues/373949) | Group | | [`audit_events_streaming_headers_update`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/92068) | A streaming header for audit events is updated | {{< icon name="check-circle" >}} Yes | GitLab [15.3](https://gitlab.com/gitlab-org/gitlab/-/issues/366350) | Group | +| [`compliance_control_status_fail`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/180107) | A compliance control status is updated to fail | {{< icon name="check-circle" >}} Yes | GitLab [17.10](https://gitlab.com/gitlab-org/gitlab/-/issues/513425) | Project | +| [`compliance_control_status_pass`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/180107) | A compliance control status is updated to pass | {{< icon name="check-circle" >}} Yes | GitLab [17.10](https://gitlab.com/gitlab-org/gitlab/-/issues/513425) | Project | | [`compliance_framework_added`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/157893) | A compliance framework is applied to a project | {{< icon name="check-circle" >}} Yes | GitLab [17.2](https://gitlab.com/gitlab-org/gitlab/-/issues/464160) | Project | | [`compliance_framework_deleted`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/65343) | A compliance framework is removed from a project | {{< icon name="check-circle" >}} Yes | GitLab [14.1](https://gitlab.com/gitlab-org/gitlab/-/issues/329362) | Project | | [`compliance_framework_id_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/94711) | A compliance framework is updated for a project | {{< icon name="check-circle" >}} Yes | GitLab [15.7](https://gitlab.com/gitlab-org/gitlab/-/issues/369310) | Project | diff --git a/ee/app/models/compliance_management/compliance_framework/project_control_compliance_status.rb b/ee/app/models/compliance_management/compliance_framework/project_control_compliance_status.rb index c0b2ea27c5d444055f9a1114d82c8e066c7dcedd..4a3a5468f1732f65e8f2ad37baf9b7b5578853a0 100644 --- a/ee/app/models/compliance_management/compliance_framework/project_control_compliance_status.rb +++ b/ee/app/models/compliance_management/compliance_framework/project_control_compliance_status.rb @@ -21,6 +21,26 @@ class ProjectControlComplianceStatus < ApplicationRecord scope :for_project_and_control, ->(project_id, control_id) { where(project_id: project_id, compliance_requirements_control_id: control_id) } + + def self.create_or_find_for_project_and_control(project, control) + record = for_project_and_control(project.id, control.id).first + return record if record.present? + + create!( + compliance_requirements_control: control, + project: project, + compliance_requirement_id: control.compliance_requirement_id, + namespace_id: project.namespace_id, + status: :pending + ) + rescue ActiveRecord::RecordNotUnique, ActiveRecord::RecordInvalid => e + if e.is_a?(ActiveRecord::RecordNotUnique) || + (e.is_a?(ActiveRecord::RecordInvalid) && e.record&.errors&.of_kind?(:project_id, :taken)) + for_project_and_control(project.id, control.id).first + else + raise e + end + end end end end diff --git a/ee/app/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service.rb b/ee/app/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..23cdb02d2db570487e3dd941aaa3b8b6fa321f87 --- /dev/null +++ b/ee/app/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +module ComplianceManagement + module ComplianceFramework + module ComplianceRequirementsControls + class UpdateStatusService < BaseService + include Gitlab::Utils::StrongMemoize + + VALID_STATUSES = %w[pass fail].freeze + + def initialize(current_user:, control:, project:, status_value:) + @current_user = current_user + @control = control + @project = project + @status_value = status_value + end + + def execute + return error('Not permitted to update compliance control status') unless permitted? + return error("'#{status_value}' is not a valid status") unless valid_status? + + return error(control_status.errors.full_messages.join(', ')) unless update_control_status + + audit_changes + success + rescue ArgumentError => e + error(e.message) + end + + private + + attr_reader :control, :project, :status_value + + def audit_changes + old_status = control_status.status_before_last_save + new_status = control_status.status + + audit_context = { + name: "compliance_control_status_#{new_status}", + scope: project, + target: control_status, + message: "Changed compliance control status from '#{old_status}' to '#{new_status}'", + author: current_user + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end + + def permitted? + project.licensed_feature_available?(:custom_compliance_frameworks) + end + + def update_control_status + control_status.update(status: status_value) + end + + def success + ServiceResponse.success(payload: { status: control_status.status }) + end + + def valid_status? + status_value.in?(VALID_STATUSES) + end + + def control_status + ComplianceManagement::ComplianceFramework::ProjectControlComplianceStatus + .create_or_find_for_project_and_control(project, control) + end + strong_memoize_attr :control_status + + def error(error_message) + ServiceResponse.error( + message: format( + _("Failed to update compliance control status. Error: %{error_message}"), + error_message: error_message + ) + ) + end + end + end + end +end diff --git a/ee/config/audit_events/types/compliance_control_status_fail.yml b/ee/config/audit_events/types/compliance_control_status_fail.yml new file mode 100644 index 0000000000000000000000000000000000000000..e4a4216dac5fd3e90a672f6fc0ea7336d045f842 --- /dev/null +++ b/ee/config/audit_events/types/compliance_control_status_fail.yml @@ -0,0 +1,10 @@ +--- +name: compliance_control_status_fail +description: A compliance control status is updated to fail +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/513425 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/180107 +feature_category: compliance_management +milestone: '17.10' +saved_to_database: true +streamed: true +scope: [Project] diff --git a/ee/config/audit_events/types/compliance_control_status_pass.yml b/ee/config/audit_events/types/compliance_control_status_pass.yml new file mode 100644 index 0000000000000000000000000000000000000000..a3e056b3c0843205af5ad13918169569eb12a789 --- /dev/null +++ b/ee/config/audit_events/types/compliance_control_status_pass.yml @@ -0,0 +1,10 @@ +--- +name: compliance_control_status_pass +description: A compliance control status is updated to pass +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/513425 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/180107 +feature_category: compliance_management +milestone: '17.10' +saved_to_database: true +streamed: true +scope: [Project] diff --git a/ee/spec/models/compliance_management/compliance_framework/project_control_compliance_status_spec.rb b/ee/spec/models/compliance_management/compliance_framework/project_control_compliance_status_spec.rb index f0ae6bec404e70923c269ee7b8f001e05854570b..8b39a9a56cd2f4f578fbd4c1993d480ada13ddc0 100644 --- a/ee/spec/models/compliance_management/compliance_framework/project_control_compliance_status_spec.rb +++ b/ee/spec/models/compliance_management/compliance_framework/project_control_compliance_status_spec.rb @@ -87,4 +87,132 @@ expect(result).to contain_exactly(another_project_status) end end + + describe '.create_or_find_for_project_and_control' do + let_it_be(:project) { create(:project) } + let_it_be(:control) { create(:compliance_requirements_control) } + + context 'when record does not exist' do + it 'creates a new record' do + expect do + described_class.create_or_find_for_project_and_control(project, control) + end.to change { described_class.count }.by(1) + end + + it 'sets correct attributes', :aggregate_failures do + status = described_class.create_or_find_for_project_and_control(project, control) + + expect(status.project_id).to eq(project.id) + expect(status.compliance_requirements_control_id).to eq(control.id) + expect(status.compliance_requirement_id).to eq(control.compliance_requirement_id) + expect(status.namespace_id).to eq(project.namespace_id) + expect(status).to be_pending + end + end + + context 'when record exists' do + let_it_be(:existing_status) do + create(:project_control_compliance_status, + project: project, + compliance_requirements_control: control) + end + + it 'returns existing record' do + status = described_class.create_or_find_for_project_and_control(project, control) + + expect(status).to eq(existing_status) + end + + it 'does not create a new record' do + expect do + described_class.create_or_find_for_project_and_control(project, control) + end.not_to change { described_class.count } + end + end + + context 'when concurrent creation occurs' do + context "when ActiveRecord::RecordNotUnique is raised" do + let!(:existing_status) do + create(:project_control_compliance_status, + project: project, + compliance_requirements_control: control) + end + + before do + empty_relation = described_class.none + record_relation = described_class.where(id: existing_status.id) + + allow(described_class).to receive(:for_project_and_control) + .with(project.id, control.id) + .and_return(empty_relation, record_relation) + + allow(described_class).to receive(:create!) + .and_raise(ActiveRecord::RecordNotUnique) + end + + it 'handles race condition and returns existing record' do + status = described_class.create_or_find_for_project_and_control(project, control) + + expect(status).to eq(existing_status) + end + end + + context "when ActiveRecord::RecordInvalid is raised" do + let!(:existing_status) do + create(:project_control_compliance_status, + project: project, + compliance_requirements_control: control) + end + + before do + empty_relation = described_class.none + record_relation = described_class.where(id: existing_status.id) + + allow(described_class).to receive(:for_project_and_control) + .with(project.id, control.id) + .and_return(empty_relation, record_relation) + + allow(described_class).to receive(:create!) + .and_raise( + ActiveRecord::RecordInvalid.new( + existing_status.tap do |status| + status.errors.add(:project_id, :taken, message: "has already been taken") + end + ) + ) + end + + it 'handles race condition and returns existing record' do + status = described_class.create_or_find_for_project_and_control(project, control) + + expect(status).to eq(existing_status) + end + end + end + + context "when ActiveRecord::RecordInvalid isn't cause by project_id" do + let!(:existing_status) do + create(:project_control_compliance_status, + project: project, + compliance_requirements_control: control) + end + + before do + empty_relation = described_class.none + record_relation = described_class.where(id: existing_status.id) + + allow(described_class).to receive(:for_project_and_control) + .with(project.id, control.id) + .and_return(empty_relation, record_relation) + + allow(described_class).to receive(:create!).and_raise(ActiveRecord::RecordInvalid) + end + + it 'raises the error' do + expect do + described_class.create_or_find_for_project_and_control(project, control) + end.to raise_error(ActiveRecord::RecordInvalid) + end + end + end end diff --git a/ee/spec/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service_spec.rb b/ee/spec/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..a8c8c0ea4b7b2980cefabe3dc6e3d6a5537db7e6 --- /dev/null +++ b/ee/spec/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service_spec.rb @@ -0,0 +1,174 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ComplianceManagement::ComplianceFramework::ComplianceRequirementsControls::UpdateStatusService, + feature_category: :compliance_management do + let_it_be(:project) { create(:project) } + let_it_be(:requirement) do + create(:compliance_requirement, framework: create(:compliance_framework)) + end + + let_it_be(:control) do + create(:compliance_requirements_control, + compliance_requirement: requirement, + control_type: :external, + external_url: 'https://example.com', + secret_token: 'foo') + end + + let_it_be(:project_control_compliance_status) do + create(:project_control_compliance_status, + project: project, + compliance_requirements_control: control, + compliance_requirement: requirement, + status: 'pending') + end + + let_it_be(:user) { create(:user) } + + subject(:service) do + described_class.new(current_user: user, control: control, project: project, status_value: 'pass') + end + + context 'when feature is disabled' do + before do + stub_licensed_features(custom_compliance_frameworks: false) + allow(::Gitlab::Audit::Auditor).to receive(:audit) + end + + it 'returns an error' do + result = service.execute + + expect(result.success?).to be false + expect(result.message).to eq( + _('Failed to update compliance control status. Error: Not permitted to update compliance control status') + ) + end + end + + context 'when feature is enabled' do + before do + stub_licensed_features(custom_compliance_frameworks: true) + allow(::Gitlab::Audit::Auditor).to receive(:audit) + end + + context 'when status is valid' do + it 'updates the compliance requirement control status' do + expect { service.execute }.to change { project_control_compliance_status.reload.status }.to('pass') + end + + it 'is successful' do + result = service.execute + + expect(result.success?).to be true + expect(result.payload[:status]).to eq('pass') + end + + it 'audits the changes' do + service.execute + + expect(::Gitlab::Audit::Auditor).to have_received(:audit).with( + name: 'compliance_control_status_pass', + scope: project, + target: project_control_compliance_status, + message: "Changed compliance control status from 'pending' to 'pass'", + author: user + ) + end + + context 'when project control status does not exist' do + before do + project_control_compliance_status.destroy! + end + + it 'creates a new project control compliance status' do + expect { service.execute }.to change { + ComplianceManagement::ComplianceFramework::ProjectControlComplianceStatus.count + }.by(1) + end + + it 'sets the correct attributes' do + service.execute + + new_status = ComplianceManagement::ComplianceFramework::ProjectControlComplianceStatus.last + expect(new_status.status).to eq('pass') + expect(new_status.project_id).to eq(project.id) + expect(new_status.compliance_requirements_control_id).to eq(control.id) + end + end + end + + context 'with invalid params' do + shared_examples 'rejects invalid status' do |status| + let(:service) do + described_class.new(current_user: user, control: control, project: project, status_value: status) + end + + it "does not update project control compliance status" do + expect { service.execute }.not_to change { project_control_compliance_status.reload.attributes } + end + + it "is unsuccessful" do + result = service.execute + + expect(result.success?).to be false + expect(result.message).to eq( + "Failed to update compliance control status. Error: '#{status}' is not a valid status" + ) + end + + it "does not audit the changes" do + service.execute + + expect(::Gitlab::Audit::Auditor).not_to have_received(:audit) + end + end + + it_behaves_like 'rejects invalid status', 'pending' + it_behaves_like 'rejects invalid status', 'invalid' + end + + context 'when status update fails' do + before do + # Stub the existing status record to fail on update + allow(project_control_compliance_status).to receive_messages(update: false, + errors: instance_double(ActiveModel::Errors, full_messages: ['Some validation error'])) + + # Ensure create_or_find_for_project_and_control returns our stubbed instance + allow(ComplianceManagement::ComplianceFramework::ProjectControlComplianceStatus) + .to receive(:create_or_find_for_project_and_control) + .with(project, control) + .and_return(project_control_compliance_status) + end + + it 'returns an error with validation messages' do + result = service.execute + + expect(result.success?).to be false + expect(result.message).to eq( + 'Failed to update compliance control status. Error: Some validation error' + ) + end + + it 'does not audit the changes' do + service.execute + + expect(::Gitlab::Audit::Auditor).not_to have_received(:audit) + end + end + + context 'when an ArgumentError is raised' do + before do + allow(service).to receive(:update_control_status).and_raise(ArgumentError, 'test error message') + end + + it 'returns an error response with the error message' do + result = service.execute + + expect(result).to be_error + expect(result.message).to eq('Failed to update compliance control status. Error: test error message') + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index f89965a0ffa9bd2756d92be83babc5eb3e4760ee..ed44aab0e62ccadaf2771e30ccfcbe0bc486a6f0 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -24604,6 +24604,9 @@ msgstr "" msgid "Failed to update branch!" msgstr "" +msgid "Failed to update compliance control status. Error: %{error_message}" +msgstr "" + msgid "Failed to update compliance requirement" msgstr ""