diff --git a/ee/app/models/compliance_management/compliance_framework/project_settings.rb b/ee/app/models/compliance_management/compliance_framework/project_settings.rb index 6008df03d158e8cfa1941bf4dfb73f35fa5e589a..e5a7010ac7adb1d3964d313f0ebe201e3d8827d6 100644 --- a/ee/app/models/compliance_management/compliance_framework/project_settings.rb +++ b/ee/app/models/compliance_management/compliance_framework/project_settings.rb @@ -16,6 +16,7 @@ class ProjectSettings < ApplicationRecord delegate :full_path, to: :project scope :by_framework_and_project, ->(project_id, framework_id) { where(project_id: project_id, framework_id: framework_id) } + scope :by_project_id, ->(project_id) { where(project_id: project_id) } def self.find_or_create_by_project(project, framework) find_or_initialize_by(project: project).tap do |setting| 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 index 23cdb02d2db570487e3dd941aaa3b8b6fa321f87..c664c298139a4f60af83134c22c9fd2008931a40 100644 --- 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 @@ -23,8 +23,6 @@ def execute audit_changes success - rescue ArgumentError => e - error(e.message) end private diff --git a/ee/lib/api/compliance_external_controls.rb b/ee/lib/api/compliance_external_controls.rb new file mode 100644 index 0000000000000000000000000000000000000000..53bcd02bbdc3256b851985ddd73222e9f81a2dff --- /dev/null +++ b/ee/lib/api/compliance_external_controls.rb @@ -0,0 +1,129 @@ +# frozen_string_literal: true + +module API + class ComplianceExternalControls < ::API::Base + VALID_STATUS_VALUES = %w[pass fail].freeze + VERIFICATION_TIMESTAMP_EXPIRY = 15.seconds + HMAC_ALGORITHM = 'SHA256' + SIGNATURE_HEADER = 'X-Gitlab-Hmac-Sha256' + TIMESTAMP_HEADER = 'X-Gitlab-Timestamp' + NONCE_HEADER = 'X-Gitlab-Nonce' + NONCE_NAMESPACE = 'control_statuses:nonce' + NONCE_LENGTH = 32 + feature_category :compliance_management + + params do + requires :id, types: [String, Integer], + desc: 'The ID or URL-encoded path of the project', + documentation: { example: 1 } + end + resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do + helpers do + def verify_hmac_signature!(control) + return error!('Control is not external', :forbidden) unless control.external? + return error!('Missing required headers', :unauthorized) unless valid_hmac_headers? + + timestamp = headers[TIMESTAMP_HEADER] + nonce = headers[NONCE_HEADER] + + return error!('Invalid nonce', :unauthorized) if invalid_nonce?(nonce) + + store_nonce(nonce) + + error!('Invalid signature', :unauthorized) unless valid_signature?(control, params, timestamp, nonce) + error!('Invalid timestamp', :unauthorized) if invalid_timestamp?(timestamp) + error!('Request has expired', :unauthorized) if expired_request?(timestamp) + end + + def expired_request?(timestamp) + Time.current.to_i - timestamp.to_i >= VERIFICATION_TIMESTAMP_EXPIRY + end + + def invalid_timestamp?(timestamp) + timestamp.to_i == 0 || timestamp.to_i > Time.current.to_i + end + + def valid_hmac_headers? + [TIMESTAMP_HEADER, NONCE_HEADER, SIGNATURE_HEADER].all? { |header| headers[header].present? } + end + + def valid_signature?(control, params, timestamp, nonce) + path = "/api/v4/projects/#{params[:id]}/compliance_external_controls/#{params[:control_id]}/status" + data = "status=#{params[:status]}" + sign_payload = "#{timestamp}#{nonce}#{path}#{data}" + + expected_signature = OpenSSL::HMAC.hexdigest( + HMAC_ALGORITHM, + control.secret_token, + sign_payload + ) + + ActiveSupport::SecurityUtils.secure_compare(headers[SIGNATURE_HEADER], expected_signature) + end + + def valid_project(control) + project = find_project(params[:id]) + error!('Project not found', :not_found) if project.nil? + + unless project.licensed_feature_available?(:custom_compliance_frameworks) + return error!('Not permitted to update compliance control status', + :unauthorized) + end + + return project if control.compliance_requirement.framework.project_settings.by_project_id(project.id).any? + + error!('Project not found', :not_found) + end + + def valid_control(control_id) + control = ComplianceManagement::ComplianceFramework::ComplianceRequirementsControl.find_by_id(control_id) + error!('Control not found', :not_found) if control.nil? + + control + end + + def invalid_nonce?(nonce) + return true if nonce.blank? || nonce.length != NONCE_LENGTH + + Gitlab::Redis::SharedState.with do |redis| + redis.exists?(nonce_key(nonce)) # rubocop:disable CodeReuse/ActiveRecord -- not using ActiveRecord + end + end + + def store_nonce(nonce) + Gitlab::Redis::SharedState.with do |redis| + redis.set(nonce_key(nonce), '1', ex: VERIFICATION_TIMESTAMP_EXPIRY.to_i + 1) + end + end + + def nonce_key(nonce) + "#{NONCE_NAMESPACE}:#{nonce}" + end + end + + desc "Update the status of a control" + params do + requires :control_id, type: Integer, desc: 'The ID of the control' + requires :status, type: String, values: VALID_STATUS_VALUES, desc: 'The status of the control' + end + patch ':id/compliance_external_controls/:control_id/status' do + control = valid_control(params[:control_id]) + verify_hmac_signature!(control) + project = valid_project(control) + user = ::Gitlab::Audit::UnauthenticatedAuthor.new + status = ComplianceManagement::ComplianceFramework::ComplianceRequirementsControls::UpdateStatusService.new( + current_user: user, + control: control, + project: project, + status_value: params[:status] + ).execute + + if status.success? + status.payload + else + error!(status.message, :bad_request) + end + end + end + end +end diff --git a/ee/lib/ee/api/api.rb b/ee/lib/ee/api/api.rb index dba3ed0e650004f73abf489283656966109024bb..922f9903251c108c498ec1a59ac36683f960699a 100644 --- a/ee/lib/ee/api/api.rb +++ b/ee/lib/ee/api/api.rb @@ -80,6 +80,7 @@ module API mount ::API::Chat mount ::API::DuoCodeReview mount ::API::SecurityScans + mount ::API::ComplianceExternalControls mount ::API::VirtualRegistries::Packages::Maven::Registries mount ::API::VirtualRegistries::Packages::Maven::Upstreams mount ::API::VirtualRegistries::Packages::Maven::Cache::Entries diff --git a/ee/spec/models/compliance_management/compliance_framework/project_settings_spec.rb b/ee/spec/models/compliance_management/compliance_framework/project_settings_spec.rb index f42b55329a828ff51fa4386834426bf27f30b375..45b15ef0e17471a126db09dcec2bb4f35ddacc35 100644 --- a/ee/spec/models/compliance_management/compliance_framework/project_settings_spec.rb +++ b/ee/spec/models/compliance_management/compliance_framework/project_settings_spec.rb @@ -53,20 +53,26 @@ end describe 'scopes' do - describe '.by_framework_and_project' do - let_it_be(:framework1) do - create(:compliance_framework, namespace: project.group.root_ancestor, name: 'framework1') - end + let_it_be(:framework1) do + create(:compliance_framework, namespace: project.group.root_ancestor, name: 'framework1') + end - let_it_be(:setting) do - create(:compliance_framework_project_setting, project: project, compliance_management_framework: framework1) - end + let_it_be(:setting) do + create(:compliance_framework_project_setting, project: project, compliance_management_framework: framework1) + end + describe '.by_framework_and_project' do it 'returns the setting' do expect(described_class.by_framework_and_project(project.id, framework1.id)) .to eq([setting]) end end + + describe '.by_project_id' do + it 'returns the setting' do + expect(described_class.by_project_id(project.id)).to eq([setting]) + end + end end describe 'creation of ComplianceManagement::Framework record' do diff --git a/ee/spec/requests/api/compliance_external_controls_spec.rb b/ee/spec/requests/api/compliance_external_controls_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..3daa6ebd80e6e871c971a1a0666dfb536e61c25b --- /dev/null +++ b/ee/spec/requests/api/compliance_external_controls_spec.rb @@ -0,0 +1,352 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::ComplianceExternalControls, feature_category: :compliance_management do + let_it_be(:project) { create(:project) } + let_it_be(:project2) { create(:project) } + let_it_be(:framework) { create(:compliance_framework, projects: [project]) } + let_it_be(:requirement) { create(:compliance_requirement, framework: framework) } + let_it_be(:control) do + create(:compliance_requirements_control, control_type: :external, secret_token: 'foo', + compliance_requirement: requirement, external_url: 'https://example.com') + end + + let_it_be(:project_control_status) do + create(:project_control_compliance_status, + project: project, + compliance_requirements_control: control, + compliance_requirement: requirement, + status: 'pending') + end + + let_it_be(:number_used_once) { SecureRandom.hex(16) } + + describe 'PATCH /projects/:id/compliance_external_controls/:control_id/status' do + let(:path) { api("/projects/#{project.id}/compliance_external_controls/#{control.id}/status") } + let(:mock_redis) { instance_double(Redis) } + + def generate_headers( + path:, + data:, + secret: control.secret_token, + timestamp: Time.now.to_i.to_s, + nonce: number_used_once + ) + sign_payload = "#{timestamp}#{nonce}#{path}#{data}" + signature = OpenSSL::HMAC.hexdigest('SHA256', secret, sign_payload) + + { + 'X-Gitlab-Timestamp' => timestamp, + 'X-Gitlab-Nonce' => nonce, + 'X-Gitlab-Hmac-Sha256' => signature + } + end + + before do + allow(Gitlab::Redis::SharedState).to receive(:with).and_yield(mock_redis) + + allow(mock_redis).to receive(:exists?) + .with("control_statuses:nonce:#{number_used_once}") + .and_return(false) + + allow(mock_redis).to receive(:set) + .with("control_statuses:nonce:#{number_used_once}", '1', ex: 16) + .and_return('OK') + stub_licensed_features(custom_compliance_frameworks: true) + end + + describe 'updates the control status' do + it 'updates the control status' do + %w[pass fail].each do |status| + data = "status=#{status}" + headers = generate_headers(path:, data:) + + patch path, params: { status: }, headers: headers + + aggregate_failures do + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).to eq({ status: }.to_json) + expect(project_control_status.reload.status).to eq(status) + end + end + end + + it 'calls UpdateStatusService with correct parameters' do + service_double = instance_double( + ComplianceManagement::ComplianceFramework::ComplianceRequirementsControls::UpdateStatusService + ) + expect(ComplianceManagement::ComplianceFramework::ComplianceRequirementsControls::UpdateStatusService) + .to receive(:new) + .with(hash_including( + control: control, + project: project, + status_value: 'pass' + )) do |args| + expect(args[:current_user]).to be_a(::Gitlab::Audit::UnauthenticatedAuthor) + service_double + end + expect(service_double).to receive(:execute).and_return(ServiceResponse.success(payload: { status: 'pass' })) + data = "status=pass" + headers = generate_headers(path:, data:) + + patch path, params: { status: 'pass' }, headers: headers + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'stores the nonce in Redis' do + expect(mock_redis).to receive(:set).with("control_statuses:nonce:#{number_used_once}", '1', ex: 16) + headers = generate_headers(path: path, data: "status=pass") + + patch path, params: { status: 'pass' }, headers: headers + + expect(response).to have_gitlab_http_status(:ok) + end + end + + describe 'returns errors' do + before do + allow(project_control_status).to receive(:update!).and_raise("Unexpected update attempt") + end + + context 'with control type restrictions' do + let_it_be(:internal_control) { create(:compliance_requirements_control, control_type: :internal) } + let(:path) { api("/projects/#{project.id}/compliance_external_controls/#{internal_control.id}/status") } + + it 'returns forbidden for internal controls' do + data = "status=pass" + headers = generate_headers(path:, data:) + + patch path, params: { status: 'pass' }, headers: headers + + aggregate_failures do + expect(response).to have_gitlab_http_status(:forbidden) + expect(json_response['error']).to eq('Control is not external') + end + end + end + + context 'with invalid request' do + it 'does not update the control status with invalid status' do + data = "status=invalid" + headers = generate_headers(path:, data:) + + patch path, params: { status: 'invalid' }, headers: headers + + aggregate_failures do + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('status does not have a valid value') + end + end + + it 'returns bad request when the status is not provided' do + data = "status=" + headers = generate_headers(path:, data:) + + patch path, params: { status: }, headers: headers + + aggregate_failures do + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('status does not have a valid value') + end + end + end + + context 'with resource not found' do + it 'returns not found when the control id is not provided' do + data = "status=pass" + missing_control_path = "/projects/#{project.id}/compliance_external_controls/" + headers = generate_headers(path: missing_control_path, data: data) + + patch api(missing_control_path), params: { status: 'pass' }, headers: headers + + aggregate_failures do + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['error']).to eq('404 Not Found') + end + end + + it 'returns not found when the control id is not found' do + data = "status=pass" + missing_control_path = "/projects/#{project.id}/compliance_external_controls/123/status" + headers = generate_headers(path: missing_control_path, data: data) + + patch api(missing_control_path), params: { status: 'pass' }, headers: headers + + aggregate_failures do + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['error']).to eq('Control not found') + end + end + + it 'returns not found when the project id is not provided' do + data = "status=pass" + missing_project_path = "/projects/compliance_external_controls/#{control.id}/status" + headers = generate_headers(path: missing_project_path, data: data) + + patch api(missing_project_path), params: { status: 'pass' }, headers: headers + + aggregate_failures do + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['error']).to eq('404 Not Found') + end + end + + it 'returns not found when the project id is not found' do + data = "status=pass" + missing_project_path = api("/projects/123/compliance_external_controls/#{control.id}/status") + headers = generate_headers(path: missing_project_path, data: data) + + patch missing_project_path, params: { status: 'pass' }, headers: headers + + aggregate_failures do + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['error']).to eq('Project not found') + end + end + + it 'returns not found when the project id does not match the control id' do + data = "status=pass" + missing_project_path = api("/projects/#{project2.id}/compliance_external_controls/#{control.id}/status") + headers = generate_headers(path: missing_project_path, data: data) + + patch missing_project_path, params: { status: 'pass' }, headers: headers + + aggregate_failures do + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['error']).to eq('Project not found') + end + end + end + + context 'with unauthorized request' do + it 'returns unauthorized when the sha256 header is missing' do + data = "status=pass" + headers = generate_headers(path: path, data: data) + headers.delete('X-Gitlab-Hmac-Sha256') + + patch path, params: { status: 'pass' }, headers: headers + + aggregate_failures do + expect(response).to have_gitlab_http_status(:unauthorized) + expect(json_response['error']).to eq('Missing required headers') + end + end + + it 'returns unauthorized when secret keys do not match' do + data = "status=pass" + wrong_secret = 'wrong_secret' + headers = generate_headers(path: path, data: data, secret: wrong_secret) + + patch path, params: { status: 'pass' }, headers: headers + + aggregate_failures do + expect(response).to have_gitlab_http_status(:unauthorized) + expect(json_response['error']).to eq('Invalid signature') + end + end + + it 'returns unauthorized when the timestamp is too old' do + [15, 16].each do |duration| + data = "status=pass" + headers = generate_headers(path: path, data: data, timestamp: (Time.now.to_i - duration).to_s) + + patch path, params: { status: 'pass' }, headers: headers + + aggregate_failures do + expect(response).to have_gitlab_http_status(:unauthorized) + expect(json_response['error']).to eq('Request has expired') + end + end + end + + it 'returns bad request when the nonce is invalid' do + data = "status=pass" + headers = generate_headers(path: path, data: data, nonce: nil) + + patch path, params: { status: 'pass' }, headers: headers + + aggregate_failures do + expect(response).to have_gitlab_http_status(:unauthorized) + expect(json_response['error']).to eq('Missing required headers') + end + end + + it 'returns unauthorized with invalid nonces' do + ['too_short', 'a' * 64].each do |invalid_nonce| + data = "status=pass" + headers = generate_headers(path: path, data: data, nonce: invalid_nonce) + + patch path, params: { status: 'pass' }, headers: headers + + aggregate_failures do + expect(response).to have_gitlab_http_status(:unauthorized) + expect(json_response['error']).to eq('Invalid nonce') + end + end + end + + it 'returns unauthorized when the nonce is already used' do + allow(mock_redis).to receive(:exists?) + .with("control_statuses:nonce:#{number_used_once}") + .and_return(true) + headers = generate_headers(path: path, data: "status=pass") + + patch path, params: { status: 'pass' }, headers: headers + + aggregate_failures do + expect(response).to have_gitlab_http_status(:unauthorized) + expect(json_response['error']).to eq('Invalid nonce') + end + end + + it 'returns unauthorized when the feature is not licensed' do + stub_licensed_features(custom_compliance_frameworks: false) + data = "status=pass" + headers = generate_headers(path:, data:) + + patch path, params: { status: 'pass' }, headers: headers + + aggregate_failures do + expect(response).to have_gitlab_http_status(:unauthorized) + expect(json_response['error']).to eq('Not permitted to update compliance control status') + end + end + + it 'returns unauthorized when the timestamp is in the future' do + [0, (Time.now.to_i + 60).to_s].each do |invalid_timestamp| + data = "status=pass" + headers = generate_headers(path: path, data: data, timestamp: invalid_timestamp) + + patch path, params: { status: 'pass' }, headers: headers + + aggregate_failures do + expect(response).to have_gitlab_http_status(:unauthorized) + expect(json_response['error']).to eq('Invalid timestamp') + end + end + end + end + + it 'returns error when the UpdateStatusService fails' do + allow_next_instance_of( + ComplianceManagement::ComplianceFramework::ComplianceRequirementsControls::UpdateStatusService + ) do |instance| + allow(instance).to receive(:execute).and_return( + ServiceResponse.error(message: 'Failed to update status') + ) + end + data = "status=pass" + headers = generate_headers(path: path, data: data) + + patch path, params: { status: 'pass' }, headers: headers + + aggregate_failures do + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('Failed to update status') + end + 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 index a8c8c0ea4b7b2980cefabe3dc6e3d6a5537db7e6..b25f578f783e4b206ab8f782565f3b373643448b 100644 --- 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 @@ -157,18 +157,5 @@ 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