Merge branch '433035-status-checks-update-model' into 'master'

Adds shared secret for external_status_check model

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/159983



Merged-by: Marcos Rocha <mrocha@gitlab.com>
Approved-by: Alan (Maciej) Paruszewski <mparuszewski@gitlab.com>
Approved-by: Marcos Rocha <mrocha@gitlab.com>
Reviewed-by: Artur Fedorov <afedorov@gitlab.com>
Co-authored-by: Artur Fedorov <afedorov@gitlab.com>

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

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

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

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 }

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

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',

ee/spec/lib/api/entities/external_status_check_spec.rb

+# 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

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') }

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

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 }
 

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
 

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