diff --git a/doc/api/status_checks.md b/doc/api/status_checks.md index 5361ffe47d73da4c01f958ca0bce1b77df693228..36a00c4c8aa5468d85481b2145bbaa93da645398 100644 --- a/doc/api/status_checks.md +++ b/doc/api/status_checks.md @@ -31,6 +31,7 @@ GET /projects/:id/external_status_checks "name": "Compliance Tool", "project_id": 6, "external_url": "https://gitlab.com/example/compliance-tool", + "hmac": true, "protected_branches": [ { "id": 14, @@ -62,6 +63,7 @@ defined external service. This includes confidential merge requests. | `id` | integer | yes | ID of a project | | `name` | string | yes | Display name of external status check service | | `external_url` | string | yes | URL of external status check service | +| `shared_secret` | string | no | HMAC secret for external status check | | `protected_branch_ids` | `array<Integer>` | no | IDs of protected branches to scope the rule by | ## Update external status check service diff --git a/ee/app/models/merge_requests/external_status_check.rb b/ee/app/models/merge_requests/external_status_check.rb index bfed21b6505f711e156039a4e383f8ff552d5be1..e185363653af6e7f06368a97e795c1ffd2255d44 100644 --- a/ee/app/models/merge_requests/external_status_check.rb +++ b/ee/app/models/merge_requests/external_status_check.rb @@ -7,6 +7,11 @@ class ExternalStatusCheck < ApplicationRecord include Auditable include EachBatch + attr_encrypted :shared_secret, + mode: :per_attribute_iv, + algorithm: 'aes-256-cbc', + key: Settings.attr_encrypted_db_key_base_32 + scope :with_api_entity_associations, -> { preload(:protected_branches) } scope :applicable_to_branch, ->(branch) do includes(:protected_branches) @@ -85,6 +90,10 @@ def audit_protected_branch_remove(model) push_audit_event(message) end + def hmac? + shared_secret.present? + end + private def protected_branches_names diff --git a/ee/app/services/branch_rules/external_status_checks/create_service.rb b/ee/app/services/branch_rules/external_status_checks/create_service.rb index 98faf966bbde68167d28e8aecf58b59d98dfae2d..503b02d4e460c07d88cb52a5f83c65c7ef393dbb 100644 --- a/ee/app/services/branch_rules/external_status_checks/create_service.rb +++ b/ee/app/services/branch_rules/external_status_checks/create_service.rb @@ -36,7 +36,7 @@ def execute_on_all_protected_branches_rule end def permitted_params - %i[name external_url] + %i[name external_url shared_secret] end end end diff --git a/ee/app/services/external_status_checks/create_service.rb b/ee/app/services/external_status_checks/create_service.rb index dbe2a443bb2f5b214197ca45d4479e0b104681b4..60e5b4c99a82099fba6335d510ab03c15312a6d5 100644 --- a/ee/app/services/external_status_checks/create_service.rb +++ b/ee/app/services/external_status_checks/create_service.rb @@ -9,6 +9,7 @@ def execute(skip_authorization: false) rule = container.external_status_checks.new(name: params[:name], external_url: params[:external_url], + shared_secret: params[:shared_secret], protected_branch_ids: params[:protected_branch_ids]) if with_audit_logged(rule, 'create_status_check') { rule.save } diff --git a/ee/lib/api/entities/external_status_check.rb b/ee/lib/api/entities/external_status_check.rb index f5cb4421dbc1d3a94fa5ac68467f288eb30b88dd..3777e215e6744672eb4dd898bc1e0c1c3522a82c 100644 --- a/ee/lib/api/entities/external_status_check.rb +++ b/ee/lib/api/entities/external_status_check.rb @@ -8,6 +8,7 @@ class ExternalStatusCheck < Grape::Entity expose :project_id, documentation: { type: 'integer', example: 1 } expose :external_url, documentation: { type: 'string', example: 'https://www.example.com' } expose :protected_branches, using: ::API::Entities::ProtectedBranch, documentation: { is_array: true } + expose :hmac?, as: :hmac, documentation: { type: 'boolean', example: 'true' } end end end diff --git a/ee/lib/api/status_checks.rb b/ee/lib/api/status_checks.rb index ca4ede2d55e2db1b4821ae4d0f50847c6cd9581e..2b81fb330b3f18d4ad17bb962d851d0ea9442535 100644 --- a/ee/lib/api/status_checks.rb +++ b/ee/lib/api/status_checks.rb @@ -24,6 +24,7 @@ def check_feature_enabled! end params do requires :name, type: String, desc: 'Display name of external status check', documentation: { example: 'QA' } + optional :shared_secret, type: String, desc: 'HMAC shared secret', documentation: { example: 'hmac-sha256' } requires :external_url, type: String, desc: 'URL of external status check resource', diff --git a/ee/spec/lib/api/entities/external_status_check_spec.rb b/ee/spec/lib/api/entities/external_status_check_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..f4096dfd21795faff5fb27e798dc24018fe65d19 --- /dev/null +++ b/ee/spec/lib/api/entities/external_status_check_spec.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Entities::ExternalStatusCheck, feature_category: :source_code_management do + subject { described_class.new(external_status_check).as_json } + + let(:external_status_check) { build(:external_status_check) } + + it 'exposes correct attributes' do + is_expected.to include(:id, :name, :project_id, :hmac, :protected_branches, :external_url) + end +end diff --git a/ee/spec/models/merge_requests/external_status_check_spec.rb b/ee/spec/models/merge_requests/external_status_check_spec.rb index de9eb7390125ebad6cccd1c751d065f283854ce2..f0d912cd3548e6a6627512f11f07fe4b3d502d86 100644 --- a/ee/spec/models/merge_requests/external_status_check_spec.rb +++ b/ee/spec/models/merge_requests/external_status_check_spec.rb @@ -179,6 +179,39 @@ it { is_expected.to eq(second_response) } end + describe 'hmac?' do + let_it_be(:merge_request) { create(:merge_request) } + let(:project) { merge_request.source_project } + + context 'when shared secret is saved' do + let_it_be(:rule) { create(:external_status_check, shared_secret: 'shared_secret') } + + subject { rule.hmac? } + + it { is_expected.to eq(true) } + end + + context 'when shared secret is not saved' do + let_it_be(:rule) { create(:external_status_check) } + + subject { rule.hmac? } + + it { is_expected.to eq(false) } + end + + context 'when shared secret had invalid value' do + ['', ' ', nil].each do |value| + context "with invalid value #{value}" do + let_it_be(:rule) { create(:external_status_check, shared_secret: value) } + + subject { rule.hmac? } + + it { is_expected.to eq(false) } + end + end + end + end + describe 'callbacks', :request_store do let_it_be(:project) { create(:project) } let_it_be(:master_branch) { create(:protected_branch, project: project, name: 'master') } diff --git a/ee/spec/requests/api/status_checks_spec.rb b/ee/spec/requests/api/status_checks_spec.rb index 4b71313cd3d41b4300d9d605fac8764b3a380efe..a27802fb646e82dda5d9530f7452f4ef98b6d922 100644 --- a/ee/spec/requests/api/status_checks_spec.rb +++ b/ee/spec/requests/api/status_checks_spec.rb @@ -215,7 +215,7 @@ let_it_be(:protected_branch) { create(:protected_branch, project: project) } let(:params) do - { name: 'New rule', external_url: 'https://gitlab.com/test/example.json', protected_branch_ids: protected_branch.id } + { name: 'New rule', external_url: 'https://gitlab.com/test/example.json', protected_branch_ids: protected_branch.id, shared_secret: 'shared_secret' } end subject do @@ -239,6 +239,7 @@ expect(json_response['id']).not_to be_nil expect(json_response['name']).to eq('New rule') + expect(json_response['hmac']).to eq(true) expect(json_response['external_url']).to eq('https://gitlab.com/test/example.json') expect(json_response['protected_branches'].size).to eq(1) end @@ -502,6 +503,7 @@ expect(json_response['id']).not_to be_nil expect(json_response['name']).to eq('New rule') + expect(json_response['hmac']).to eq(false) expect(json_response['external_url']).to eq('https://gitlab.com/test/example.json') expect(json_response['protected_branches'].size).to eq(1) end diff --git a/ee/spec/services/branch_rules/external_status_checks/create_service_spec.rb b/ee/spec/services/branch_rules/external_status_checks/create_service_spec.rb index aaefb33f3ccd070b0d772fd96770a34ef8a53839..4723570980d3839d9ebcc7a462c81ba560434d65 100644 --- a/ee/spec/services/branch_rules/external_status_checks/create_service_spec.rb +++ b/ee/spec/services/branch_rules/external_status_checks/create_service_spec.rb @@ -12,7 +12,7 @@ let(:create_service) { ExternalStatusChecks::CreateService } let(:create_service_instance) { instance_double(update_service) } let(:status_check_name) { 'Test' } - let(:params) { { name: status_check_name, external_url: 'https://external_url.text/hello.json' } } + let(:params) { { name: status_check_name, external_url: 'https://external_url.text/hello.json', shared_secret: 'shared_secret' } } subject(:execute) { described_class.new(branch_rule, user, params).execute } diff --git a/ee/spec/services/external_status_checks/create_service_spec.rb b/ee/spec/services/external_status_checks/create_service_spec.rb index 885a2e704b820cb24e9ba8c24c955a6f95f6ed21..0a3b6e91ecbdef56499c0d752801eafd88ce2c23 100644 --- a/ee/spec/services/external_status_checks/create_service_spec.rb +++ b/ee/spec/services/external_status_checks/create_service_spec.rb @@ -13,7 +13,8 @@ { name: 'Test', external_url: 'https://external_url.text/hello.json', - protected_branch_ids: [protected_branch.id] + protected_branch_ids: [protected_branch.id], + shared_secret: 'shared_secret' } end diff --git a/ee/spec/support/shared_examples/external_status_checks/create_external_status_shared_examples.rb b/ee/spec/support/shared_examples/external_status_checks/create_external_status_shared_examples.rb index 1c55eab81d1eba7fb4382b80f10f8dfcb5f52cd8..72b2103c205d614a9c35f9585ce45964e3e8c0a2 100644 --- a/ee/spec/support/shared_examples/external_status_checks/create_external_status_shared_examples.rb +++ b/ee/spec/support/shared_examples/external_status_checks/create_external_status_shared_examples.rb @@ -46,6 +46,7 @@ expect(rule.project).to eq(project) expect(rule.external_url).to eq('https://external_url.text/hello.json') expect(rule.name).to eq 'Test' + expect(rule.shared_secret).to eq 'shared_secret' expect(rule.protected_branches).to contain_exactly(protected_branch) end end