cache the gpg commit signature

we store the result of the gpg commit verification in the db because the
gpg verification is an expensive operation.

app/models/commit.rb

@@ -239,29 +239,14 @@ class Commit
 
     @signature = nil
 
-    signature, signed_text = @raw.signature(project.repository)
+    cached_signature = GpgSignature.find_by(commit_sha: sha)
+    return cached_signature if cached_signature.present?
 
-    return unless signature && signed_text
+    gpg_commit = Gitlab::Gpg::Commit.new(self)
 
-    Gitlab::Gpg.using_tmp_keychain do
-      # first we need to get the keyid from the signature...
-      GPGME::Crypto.new.verify(signature, signed_text: signed_text) do |verified_signature|
-        @signature = verified_signature
-      end
-
-      # ... then we query the gpg key belonging to the keyid.
-      gpg_key = GpgKey.find_by(primary_keyid: @signature.fingerprint)
-
-      return @signature unless gpg_key
-
-      Gitlab::Gpg::CurrentKeyChain.add(gpg_key.key)
-
-      GPGME::Crypto.new.verify(signature, signed_text: signed_text) do |verified_signature|
-        @signature = verified_signature
-      end
-    end
+    return unless gpg_commit.has_signature?
 
-    @signature
+    @signature = gpg_commit.signature
   end
 
   def revert_branch_name

app/views/projects/commit/_signature.html.haml

 - if signature
-  %a.btn.disabled.btn-xs{ class: ('btn-success' if signature.valid?) }
-    %i.fa.fa-key{ class: ('fa-inverse' if signature.valid?) }
-    = signature.valid? ? 'Verified': 'Unverified'
+  %a.btn.disabled.btn-xs{ class: ('btn-success' if signature.valid_signature?) }
+    %i.fa.fa-key{ class: ('fa-inverse' if signature.valid_signature?) }
+    = signature.valid_signature? ? 'Verified' : 'Unverified'

lib/gitlab/gpg/commit.rb

+module Gitlab
+  module Gpg
+    class Commit
+      attr_reader :commit
+
+      def initialize(commit)
+        @commit = commit
+
+        @signature_text, @signed_text = commit.raw.signature(commit.project.repository)
+      end
+
+      def has_signature?
+        @signature_text && @signed_text
+      end
+
+      def signature
+        Gitlab::Gpg.using_tmp_keychain do
+          # first we need to get the keyid from the signature to query the gpg
+          # key belonging to the keyid.
+          # This way we can add the key to the temporary keychain and extract
+          # the proper signature.
+          gpg_key = GpgKey.find_by(primary_keyid: verified_signature.fingerprint)
+
+          if gpg_key
+            Gitlab::Gpg::CurrentKeyChain.add(gpg_key.key)
+          end
+
+          create_cached_signature!(gpg_key)
+        end
+      end
+
+      private
+
+      def verified_signature
+        GPGME::Crypto.new.verify(@signature_text, signed_text: @signed_text) do |verified_signature|
+          return verified_signature
+        end
+      end
+
+      def create_cached_signature!(gpg_key)
+        GpgSignature.create!(
+          commit_sha: commit.sha,
+          project: commit.project,
+          gpg_key: gpg_key,
+          gpg_key_primary_keyid: gpg_key&.primary_keyid,
+          valid_signature: !!(gpg_key && verified_signature&.valid?)
+        )
+      end
+    end
+  end
+end

spec/lib/gitlab/gpg/commit_spec.rb

+require 'rails_helper'
+
+RSpec.describe Gitlab::Gpg::Commit do
+  describe '#signature' do
+    let!(:project) { create :project, :repository, path: 'sample-project' }
+
+    context 'known public key' do
+      it 'returns a valid signature' do
+        gpg_key = create :gpg_key, key: GpgHelpers::User1.public_key
+
+        raw_commit = double(:raw_commit, signature: [
+          GpgHelpers::User1.signed_commit_signature,
+          GpgHelpers::User1.signed_commit_base_data
+        ], sha: '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33')
+        allow(raw_commit).to receive :save!
+
+        commit = create :commit,
+          git_commit: raw_commit,
+          project: project
+
+        expect(described_class.new(commit).signature).to have_attributes(
+          commit_sha: '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33',
+          project: project,
+          gpg_key: gpg_key,
+          gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid,
+          valid_signature: true
+        )
+      end
+    end
+
+    context 'unknown public key' do
+      it 'returns an invalid signature', :gpg do
+        raw_commit = double(:raw_commit, signature: [
+          GpgHelpers::User1.signed_commit_signature,
+          GpgHelpers::User1.signed_commit_base_data
+        ], sha: '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33')
+        allow(raw_commit).to receive :save!
+
+        commit = create :commit,
+          git_commit: raw_commit,
+          project: project
+
+        expect(described_class.new(commit).signature).to have_attributes(
+          commit_sha: '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33',
+          project: project,
+          gpg_key: nil,
+          gpg_key_primary_keyid: nil,
+          valid_signature: false
+        )
+      end
+    end
+  end
+end

spec/models/commit_spec.rb

@@ -421,36 +421,78 @@ eos
     end
 
     context 'signed commit', :gpg do
-      it 'returns a valid signature if the public key is known' do
+      context 'known public key' do
+        it 'returns a valid signature' do
           create :gpg_key, key: GpgHelpers::User1.public_key
 
           raw_commit = double(:raw_commit, signature: [
             GpgHelpers::User1.signed_commit_signature,
             GpgHelpers::User1.signed_commit_base_data
-        ])
+          ], sha: '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33')
           allow(raw_commit).to receive :save!
 
           commit = create :commit,
             git_commit: raw_commit,
             project: project
 
-        expect(commit.signature).to be_a GPGME::Signature
-        expect(commit.signature.valid?).to be_truthy
+          expect(commit.signature.valid_signature?).to be_truthy
         end
 
+        it 'returns the cached validation result on second call', :gpg do
+          create :gpg_key, key: GpgHelpers::User1.public_key
+
+          raw_commit = double(:raw_commit, signature: [
+            GpgHelpers::User1.signed_commit_signature,
+            GpgHelpers::User1.signed_commit_base_data
+          ], sha: '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33')
+          allow(raw_commit).to receive :save!
+
+          commit = create :commit,
+            git_commit: raw_commit,
+            project: project
+
+          expect(Gitlab::Gpg::Commit).to receive(:new).and_call_original
+          expect(commit.signature.valid_signature?).to be_truthy
+
+          # second call returns the cache
+          expect(Gitlab::Gpg::Commit).not_to receive(:new).and_call_original
+          expect(commit.signature.valid_signature?).to be_truthy
+        end
+      end
+
+      context 'unknown public key' do
         it 'returns an invalid signature if the public key is unknown', :gpg do
           raw_commit = double(:raw_commit, signature: [
             GpgHelpers::User1.signed_commit_signature,
             GpgHelpers::User1.signed_commit_base_data
-        ])
+          ], sha: '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33')
+          allow(raw_commit).to receive :save!
+
+          commit = create :commit,
+            git_commit: raw_commit,
+            project: project
+
+          expect(commit.signature.valid_signature?).to be_falsey
+        end
+
+        it 'returns the cached validation result on second call', :gpg do
+          raw_commit = double(:raw_commit, signature: [
+            GpgHelpers::User1.signed_commit_signature,
+            GpgHelpers::User1.signed_commit_base_data
+          ], sha: '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33')
           allow(raw_commit).to receive :save!
 
           commit = create :commit,
             git_commit: raw_commit,
             project: project
 
-        expect(commit.signature).to be_a GPGME::Signature
-        expect(commit.signature.valid?).to be_falsey
+          expect(Gitlab::Gpg::Commit).to receive(:new).and_call_original
+          expect(commit.signature.valid_signature?).to be_falsey
+
+          # second call returns the cache
+          expect(Gitlab::Gpg::Commit).not_to receive(:new).and_call_original
+          expect(commit.signature.valid_signature?).to be_falsey
+        end
       end
     end
   end