From 508ff17b3405a4e2275fa137bd7322b728db8ed4 Mon Sep 17 00:00:00 2001
From: Alexis Reigel <mail@koffeinfrei.org>
Date: Thu, 24 Aug 2017 14:21:26 +0200
Subject: [PATCH] pass whole commit to Gitlab::Gpg::Commit again

we need the commit object for the updated verification that also checks
the committer's email to match the gpg key and user's emails.
---
 app/models/commit.rb                          |  2 +-
 app/models/gpg_signature.rb                   |  2 +-
 app/workers/create_gpg_signature_worker.rb    |  6 +++++-
 lib/gitlab/gpg/commit.rb                      | 17 ++++++---------
 .../gpg/invalid_gpg_signature_updater_spec.rb | 21 +++++++++++++++++++
 .../create_gpg_signature_worker_spec.rb       |  9 ++++++--
 6 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/app/models/commit.rb b/app/models/commit.rb
index c943365016f..ba3845df867 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -405,6 +405,6 @@ class Commit
   end
 
   def gpg_commit
-    @gpg_commit ||= Gitlab::Gpg::Commit.for_commit(self)
+    @gpg_commit ||= Gitlab::Gpg::Commit.new(self)
   end
 end
diff --git a/app/models/gpg_signature.rb b/app/models/gpg_signature.rb
index 50fb35c77ec..6680bc4da0b 100644
--- a/app/models/gpg_signature.rb
+++ b/app/models/gpg_signature.rb
@@ -20,6 +20,6 @@ class GpgSignature < ActiveRecord::Base
   end
 
   def gpg_commit
-    Gitlab::Gpg::Commit.new(project, commit_sha)
+    Gitlab::Gpg::Commit.new(commit)
   end
 end
diff --git a/app/workers/create_gpg_signature_worker.rb b/app/workers/create_gpg_signature_worker.rb
index f34dff2d656..9b5ff17aafa 100644
--- a/app/workers/create_gpg_signature_worker.rb
+++ b/app/workers/create_gpg_signature_worker.rb
@@ -6,7 +6,11 @@ class CreateGpgSignatureWorker
     project = Project.find_by(id: project_id)
     return unless project
 
+    commit = project.commit(commit_sha)
+
+    return unless commit
+
     # This calculates and caches the signature in the database
-    Gitlab::Gpg::Commit.new(project, commit_sha).signature
+    Gitlab::Gpg::Commit.new(commit).signature
   end
 end
diff --git a/lib/gitlab/gpg/commit.rb b/lib/gitlab/gpg/commit.rb
index 606c7576f70..f701897955b 100644
--- a/lib/gitlab/gpg/commit.rb
+++ b/lib/gitlab/gpg/commit.rb
@@ -1,17 +1,12 @@
 module Gitlab
   module Gpg
     class Commit
-      def self.for_commit(commit)
-        new(commit.project, commit.sha)
-      end
-
-      def initialize(project, sha)
-        @project = project
-        @sha = sha
+      def initialize(commit)
+        @commit = commit
 
         @signature_text, @signed_text =
           begin
-            Rugged::Commit.extract_signature(project.repository.rugged, sha)
+            Rugged::Commit.extract_signature(@commit.project.repository.rugged, @commit.sha)
           rescue Rugged::OdbError
             nil
           end
@@ -26,7 +21,7 @@ module Gitlab
 
         return @signature if @signature
 
-        cached_signature = GpgSignature.find_by(commit_sha: @sha)
+        cached_signature = GpgSignature.find_by(commit_sha: @commit.sha)
         return @signature = cached_signature if cached_signature.present?
 
         @signature = create_cached_signature!
@@ -75,8 +70,8 @@ module Gitlab
         user_infos = user_infos(gpg_key)
 
         {
-          commit_sha: @sha,
-          project: @project,
+          commit_sha: @commit.sha,
+          project: @commit.project,
           gpg_key: gpg_key,
           gpg_key_primary_keyid: gpg_key&.primary_keyid || verified_signature.fingerprint,
           gpg_key_user_name: user_infos[:name],
diff --git a/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb b/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb
index 4de4419de27..120cd1b74f7 100644
--- a/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb
+++ b/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb
@@ -4,8 +4,29 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do
   describe '#run' do
     let!(:commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33' }
     let!(:project) { create :project, :repository, path: 'sample-project' }
+    let!(:raw_commit) do
+      raw_commit = double(
+        :raw_commit,
+        signature: [
+          GpgHelpers::User1.signed_commit_signature,
+          GpgHelpers::User1.signed_commit_base_data
+        ],
+        sha: commit_sha,
+        committer_email: GpgHelpers::User1.emails.first
+      )
+
+      allow(raw_commit).to receive :save!
+
+      raw_commit
+    end
+
+    let!(:commit) do
+      create :commit, git_commit: raw_commit, project: project
+    end
 
     before do
+      allow_any_instance_of(Project).to receive(:commit).and_return(commit)
+
       allow(Rugged::Commit).to receive(:extract_signature)
         .with(Rugged::Repository, commit_sha)
         .and_return(
diff --git a/spec/workers/create_gpg_signature_worker_spec.rb b/spec/workers/create_gpg_signature_worker_spec.rb
index 54978baca88..aa6c347d738 100644
--- a/spec/workers/create_gpg_signature_worker_spec.rb
+++ b/spec/workers/create_gpg_signature_worker_spec.rb
@@ -7,9 +7,14 @@ describe CreateGpgSignatureWorker do
     let(:commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33' }
 
     it 'calls Gitlab::Gpg::Commit#signature' do
-      expect(Gitlab::Gpg::Commit).to receive(:new).with(project, commit_sha).and_call_original
+      commit = instance_double(Commit)
+      gpg_commit = instance_double(Gitlab::Gpg::Commit)
 
-      expect_any_instance_of(Gitlab::Gpg::Commit).to receive(:signature)
+      allow(Project).to receive(:find_by).with(id: project.id).and_return(project)
+      allow(project).to receive(:commit).with(commit_sha).and_return(commit)
+
+      expect(Gitlab::Gpg::Commit).to receive(:new).with(commit).and_return(gpg_commit)
+      expect(gpg_commit).to receive(:signature)
 
       described_class.new.perform(commit_sha, project.id)
     end