Add latest changes from gitlab-org/gitlab@17-2-stable-ee

app/services/gpg_keys/validate_integrations_service.rb

@@ -25,6 +25,7 @@ module GpgKeys
 
       integration.execute({ key_id: key.primary_keyid, committer_email: key.user.email })
 
+      key.externally_verified_at = Time.current
       key.externally_verified = true
     rescue ::Gitlab::BeyondIdentity::Client::ApiError => e
       key.errors.add(:base, "BeyondIdentity: #{e.message}") unless e.acceptable_error?

db/migrate/20240724220706_add_externally_verified_at_to_gpg_keys.rb

@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+class AddExternallyVerifiedAtToGpgKeys < Gitlab::Database::Migration[2.2]
+  milestone '17.3'
+
+  def change
+    add_column :gpg_keys, :externally_verified_at, :datetime_with_timezone, null: true
+  end
+end

db/schema_migrations/20240724220706

@@ -0,0 +1 @@
+84daa7a069f25cd6a25843b9dcfd1c17145c88e896d42b381dc31a2ff21eace5

db/structure.sql

@@ -10814,7 +10814,8 @@ CREATE TABLE gpg_keys (
     primary_keyid bytea,
     fingerprint bytea,
     key text,
-    externally_verified boolean DEFAULT false NOT NULL
+    externally_verified boolean DEFAULT false NOT NULL,
+    externally_verified_at timestamp with time zone
 );
 
 CREATE SEQUENCE gpg_keys_id_seq

lib/gitlab/checks/integrations/beyond_identity_check.rb

@@ -28,9 +28,19 @@ module Gitlab
 
             commits.each do |commit|
               signature = commit.signature
-              if !signature.verified?
+              unless signature.verified?
                 raise ::Gitlab::GitAccess::ForbiddenError, "Signature of the commit #{commit.sha} is not verified"
-              elsif !reverified_with_integration?(signature.gpg_key)
+              end
+
+              key = signature.gpg_key
+              unless key
+                gpg_commit = commit.gpg_commit
+                gpg_commit.update_signature!(signature)
+
+                key = gpg_commit.signature.gpg_key
+              end
+
+              unless reverified_with_integration?(key)
                 raise ::Gitlab::GitAccess::ForbiddenError, "GPG Key used to sign commit #{commit.sha} is not verified"
               end
             end
@@ -52,11 +62,11 @@ module Gitlab
             break false unless key.present?
 
             gpg_key = key.is_a?(GpgKeySubkey) ? key.gpg_key : key
-            break false unless key.externally_verified?
-            break true if gpg_key.updated_at > INTEGRATION_VERIFICATION_PERIOD.ago
+
+            break gpg_key.externally_verified? unless require_reverification?(gpg_key)
 
             verified_externally?(gpg_key).tap do |verified_externally|
-              key.update!(externally_verified: verified_externally)
+              key.update!(externally_verified: verified_externally, externally_verified_at: Time.current)
             end
           end
         end
@@ -69,6 +79,12 @@ module Gitlab
           false
         end
 
+        def require_reverification?(key)
+          return true unless key.externally_verified_at.present?
+
+          key.externally_verified_at <= INTEGRATION_VERIFICATION_PERIOD.ago
+        end
+
         def integration
           project.beyond_identity_integration || ::Integrations::BeyondIdentity.for_instance.first
         end

spec/lib/gitlab/checks/integrations/beyond_identity_check_spec.rb

@@ -96,64 +96,96 @@ RSpec.describe Gitlab::Checks::Integrations::BeyondIdentityCheck, feature_catego
         end
       end
 
-      context 'when key verification by integrations is stale' do
-        let!(:gpg_key) do
-          create :gpg_key, externally_verified: externally_verified,
-            updated_at: (described_class::INTEGRATION_VERIFICATION_PERIOD + 1.day).ago
+      context 'when the signature is verified' do
+        let!(:verified_signature) do
+          create(
+            :gpg_signature,
+            commit_sha: 'f0a5ed60d24c98ec6d00ac010c1f3f01ee0a8373',
+            project: project,
+            gpg_key: gpg_key,
+            gpg_key_primary_keyid: gpg_key.keyid,
+            verification_status: :verified
+          )
         end
 
+        let(:verified_gpg_key) { build(:gpg_key, externally_verified: true, externally_verified_at: Time.current) }
+
         before do
           allow(Integrations::BeyondIdentity).to receive(:for_instance).and_return([beyond_identity_integration])
+          allow_next_instances_of(CommitSignatures::GpgSignature, 2) do |signature|
+            allow(signature).to receive(:verified?).and_return(true)
+            allow(signature).to receive(:gpg_key).and_return(verified_gpg_key)
+          end
         end
 
-        context 'and the signature is verified' do
+        context 'and key was deleted' do
           before do
-            allow_next_instances_of(CommitSignatures::GpgSignature, 3) do |signature|
-              allow(signature).to receive(:verified?).and_return(true)
-              allow(signature).to receive(:gpg_key).and_return(gpg_key)
-            end
+            gpg_key.destroy!
           end
 
-          let(:externally_verified) { true }
-
-          context 'and the key is not verified' do
-            let(:externally_verified) { false }
-
-            it 'raises an error without calling integrations' do
-              expect(GpgKeys::ValidateIntegrationsService).not_to receive(:new)
-              expect { check.validate! }
-              .to raise_error(::Gitlab::GitAccess::ForbiddenError,
-                'GPG Key used to sign commit f0a5ed60d24c98ec6d00ac010c1f3f01ee0a8373 is not verified')
-            end
+          it 'raises an error without calling integrations' do
+            expect(GpgKeys::ValidateIntegrationsService).not_to receive(:new)
+            expect { check.validate! }
+            .to raise_error(::Gitlab::GitAccess::ForbiddenError,
+              'GPG Key used to sign commit f0a5ed60d24c98ec6d00ac010c1f3f01ee0a8373 is not verified')
           end
 
-          context 'when not verified by integrations' do
-            before do
-              allow(beyond_identity_integration).to receive(:execute).and_raise(
-                ::Gitlab::BeyondIdentity::Client::ApiError.new('error', 403)
-              )
-            end
-
-            it 'raises an error' do
-              expect { check.validate! }
-              .to raise_error(::Gitlab::GitAccess::ForbiddenError,
-                'GPG Key used to sign commit f0a5ed60d24c98ec6d00ac010c1f3f01ee0a8373 is not verified')
-              expect(gpg_key.reload.externally_verified).to eq(false)
-            end
-          end
-
-          context 'when verified by integrations' do
+          context 'and the key is added again' do
+            let(:new_gpg_key) { create :gpg_key, externally_verified: true, externally_verified_at: Time.current }
+
             before do
+              new_gpg_key.update_column(:fingerprint, 'A328467F793DBC6033FEA1B9EDD30D2BEB691AC9')
               allow(beyond_identity_integration).to receive(:execute)
             end
 
             it 'does not raise an error' do
               expect { check.validate! }.not_to raise_error
+              expect(verified_signature.reload.gpg_key).to eq(new_gpg_key)
             end
+          end
+        end
 
-            it 'updates updated_at' do
-              freeze_time do
-                expect { check.validate! }.to change { gpg_key.reload.updated_at }.to(Time.current)
+        context 'when key verification by integrations is stale' do
+          let!(:gpg_key) do
+            create :gpg_key, externally_verified: externally_verified,
+              externally_verified_at: (described_class::INTEGRATION_VERIFICATION_PERIOD + 1.day).ago
+          end
+
+          context 'and the key is verified' do
+            let(:externally_verified) { true }
+
+            context 'when not verified by integrations' do
+              before do
+                allow(beyond_identity_integration).to receive(:execute).and_raise(
+                  ::Gitlab::BeyondIdentity::Client::ApiError.new('error', 403)
+                )
+              end
+
+              it 'raises an error' do
+                expect { check.validate! }
+                .to raise_error(::Gitlab::GitAccess::ForbiddenError,
+                  'GPG Key used to sign commit f0a5ed60d24c98ec6d00ac010c1f3f01ee0a8373 is not verified')
+                expect(gpg_key.reload.externally_verified).to eq(false)
+              end
+            end
+          end
+
+          context 'and the key is not verified' do
+            let(:externally_verified) { false }
+
+            context 'when verified by integrations' do
+              before do
+                allow(beyond_identity_integration).to receive(:execute)
+              end
+
+              it 'does not raise an error' do
+                expect { check.validate! }.not_to raise_error
+              end
+
+              it 'updates externally_verified_at' do
+                freeze_time do
+                  expect { check.validate! }.to change { gpg_key.reload.externally_verified_at }.to(Time.current)
+                end
               end
             end
           end

spec/services/gpg_keys/validate_integrations_service_spec.rb

@@ -39,6 +39,7 @@ RSpec.describe GpgKeys::ValidateIntegrationsService, feature_category: :source_c
 
       expect(service.execute).to eq(true)
       expect(gpg_key.externally_verified).to be_truthy
+      expect(gpg_key.externally_verified_at).to be_present
     end
 
     context 'when the check is unsuccessful' do
@@ -58,6 +59,7 @@ RSpec.describe GpgKeys::ValidateIntegrationsService, feature_category: :source_c
           expect(service.execute).to eq(false)
           expect(gpg_key.errors.full_messages).to eq(["BeyondIdentity: #{error_message}"])
           expect(gpg_key.externally_verified).to be_falsey
+          expect(gpg_key.externally_verified_at).not_to be_present
         end
       end