match the committer's email against the gpg key

the updated verification of a gpg signature requires the committer's
email to also match the user's and the key's emails.

app/models/gpg_key.rb

@@ -73,6 +73,10 @@ class GpgKey < ActiveRecord::Base
     emails_with_verified_status.any? { |_email, verified| verified }
   end
 
+  def verified_and_belongs_to_email?(email)
+    emails_with_verified_status.any? { |key_email, verified| key_email == email && verified }
+  end
+
   def update_invalid_gpg_signatures
     InvalidGpgSignatureUpdateWorker.perform_async(self.id)
   end

app/models/gpg_signature.rb

@@ -4,6 +4,14 @@ class GpgSignature < ActiveRecord::Base
   sha_attribute :commit_sha
   sha_attribute :gpg_key_primary_keyid
 
+  enum verification_status: {
+    unverified: 0,
+    verified: 1,
+    other_user: 2,
+    unverified_key: 3,
+    unknown_key: 4
+  }
+
   belongs_to :project
   belongs_to :gpg_key
 

lib/gitlab/gpg/commit.rb

@@ -68,6 +68,7 @@ module Gitlab
 
       def attributes(gpg_key)
         user_infos = user_infos(gpg_key)
+        verification_status = verification_status(gpg_key)
 
         {
           commit_sha: @commit.sha,
@@ -76,12 +77,21 @@ module Gitlab
           gpg_key_primary_keyid: gpg_key&.primary_keyid || verified_signature.fingerprint,
           gpg_key_user_name: user_infos[:name],
           gpg_key_user_email: user_infos[:email],
-          valid_signature: gpg_signature_valid_signature_value(gpg_key)
+          valid_signature: verification_status == GpgSignature.verification_statuses[:verified],
+          verification_status: verification_status
         }
       end
 
-      def gpg_signature_valid_signature_value(gpg_key)
-        !!(gpg_key && gpg_key.verified? && verified_signature.valid?)
+      def verification_status(gpg_key)
+        if gpg_key && gpg_key.verified_and_belongs_to_email?(@commit.committer_email) && verified_signature.valid?
+          GpgSignature.verification_statuses[:verified]
+        elsif gpg_key && gpg_key.verified? && verified_signature.valid?
+          GpgSignature.verification_statuses[:other_user]
+        elsif gpg_key
+          GpgSignature.verification_statuses[:unverified_key]
+        else
+          GpgSignature.verification_statuses[:unknown_key]
+        end
       end
 
       def user_infos(gpg_key)

spec/lib/gitlab/gpg/commit_spec.rb

@@ -3,59 +3,109 @@ require 'rails_helper'
 describe Gitlab::Gpg::Commit do
   describe '#signature' do
     let!(:project) { create :project, :repository, path: 'sample-project' }
-    let!(:commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33'  }
+    let!(:commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33' }
 
     context 'unsigned commit' do
+      let!(:commit) { create :commit, project: project, sha: commit_sha }
+
       it 'returns nil' do
-        expect(described_class.new(project, commit_sha).signature).to be_nil
+        expect(described_class.new(commit).signature).to be_nil
       end
     end
 
-    context 'known and verified public key' do
-      let!(:gpg_key) do
-        create :gpg_key, key: GpgHelpers::User1.public_key, user: create(:user, email: GpgHelpers::User1.emails.first)
-      end
-
-      before do
-        allow(Rugged::Commit).to receive(:extract_signature)
-          .with(Rugged::Repository, commit_sha)
-          .and_return(
-            [
-              GpgHelpers::User1.signed_commit_signature,
-              GpgHelpers::User1.signed_commit_base_data
-            ]
-          )
-      end
-
-      it 'returns a valid signature' do
-        expect(described_class.new(project, commit_sha).signature).to have_attributes(
-          commit_sha: commit_sha,
-          project: project,
-          gpg_key: gpg_key,
-          gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid,
-          gpg_key_user_name: GpgHelpers::User1.names.first,
-          gpg_key_user_email: GpgHelpers::User1.emails.first,
-          valid_signature: true
-        )
+    context 'known key' do
+      context 'user matches the key uid' do
+        context 'user matches the committer' do
+          let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: GpgHelpers::User1.emails.first }
+
+          let!(:user) { create(:user, email: GpgHelpers::User1.emails.first) }
+
+          let!(:gpg_key) do
+            create :gpg_key, key: GpgHelpers::User1.public_key, user: user
+          end
+
+          before do
+            allow(Rugged::Commit).to receive(:extract_signature)
+            .with(Rugged::Repository, commit_sha)
+            .and_return(
+              [
+                GpgHelpers::User1.signed_commit_signature,
+                GpgHelpers::User1.signed_commit_base_data
+              ]
+            )
+          end
+
+          it 'returns a valid signature' do
+            expect(described_class.new(commit).signature).to have_attributes(
+              commit_sha: commit_sha,
+              project: project,
+              gpg_key: gpg_key,
+              gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid,
+              gpg_key_user_name: GpgHelpers::User1.names.first,
+              gpg_key_user_email: GpgHelpers::User1.emails.first,
+              valid_signature: true,
+              verification_status: 'verified'
+            )
+          end
+
+          it 'returns the cached signature on second call' do
+            gpg_commit = described_class.new(commit)
+
+            expect(gpg_commit).to receive(:using_keychain).and_call_original
+            gpg_commit.signature
+
+            # consecutive call
+            expect(gpg_commit).not_to receive(:using_keychain).and_call_original
+            gpg_commit.signature
+          end
+        end
+
+        context 'user does not match the committer' do
+          let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: GpgHelpers::User2.emails.first }
+
+          let(:user) { create(:user, email: GpgHelpers::User1.emails.first) }
+
+          let!(:gpg_key) do
+            create :gpg_key, key: GpgHelpers::User1.public_key, user: user
+          end
+
+          before do
+            allow(Rugged::Commit).to receive(:extract_signature)
+            .with(Rugged::Repository, commit_sha)
+            .and_return(
+              [
+                GpgHelpers::User1.signed_commit_signature,
+                GpgHelpers::User1.signed_commit_base_data
+              ]
+            )
+          end
+
+          it 'returns an invalid signature' do
+            expect(described_class.new(commit).signature).to have_attributes(
+              commit_sha: commit_sha,
+              project: project,
+              gpg_key: gpg_key,
+              gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid,
+              gpg_key_user_name: GpgHelpers::User1.names.first,
+              gpg_key_user_email: GpgHelpers::User1.emails.first,
+              valid_signature: false,
+              verification_status: 'other_user'
+            )
+          end
+        end
       end
 
-      it 'returns the cached signature on second call' do
-        gpg_commit = described_class.new(project, commit_sha)
-
-        expect(gpg_commit).to receive(:using_keychain).and_call_original
-        gpg_commit.signature
+      context 'user does not match the key uid' do
+        let!(:commit) { create :commit, project: project, sha: commit_sha }
 
-        # consecutive call
-        expect(gpg_commit).not_to receive(:using_keychain).and_call_original
-        gpg_commit.signature
-      end
-    end
+        let(:user) { create(:user, email: GpgHelpers::User2.emails.first) }
 
-    context 'known but unverified public key' do
-      let!(:gpg_key) { create :gpg_key, key: GpgHelpers::User1.public_key }
+        let!(:gpg_key) do
+          create :gpg_key, key: GpgHelpers::User1.public_key, user: user
+        end
 
-      before do
-        allow(Rugged::Commit).to receive(:extract_signature)
+        before do
+          allow(Rugged::Commit).to receive(:extract_signature)
           .with(Rugged::Repository, commit_sha)
           .and_return(
             [
@@ -63,33 +113,26 @@ describe Gitlab::Gpg::Commit do
               GpgHelpers::User1.signed_commit_base_data
             ]
           )
-      end
-
-      it 'returns an invalid signature' do
-        expect(described_class.new(project, commit_sha).signature).to have_attributes(
-          commit_sha: commit_sha,
-          project: project,
-          gpg_key: gpg_key,
-          gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid,
-          gpg_key_user_name: GpgHelpers::User1.names.first,
-          gpg_key_user_email: GpgHelpers::User1.emails.first,
-          valid_signature: false
-        )
-      end
-
-      it 'returns the cached signature on second call' do
-        gpg_commit = described_class.new(project, commit_sha)
-
-        expect(gpg_commit).to receive(:using_keychain).and_call_original
-        gpg_commit.signature
-
-        # consecutive call
-        expect(gpg_commit).not_to receive(:using_keychain).and_call_original
-        gpg_commit.signature
+        end
+
+        it 'returns an invalid signature' do
+          expect(described_class.new(commit).signature).to have_attributes(
+            commit_sha: commit_sha,
+            project: project,
+            gpg_key: gpg_key,
+            gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid,
+            gpg_key_user_name: GpgHelpers::User1.names.first,
+            gpg_key_user_email: GpgHelpers::User1.emails.first,
+            valid_signature: false,
+            verification_status: 'unverified_key'
+          )
+        end
       end
     end
 
-    context 'unknown public key' do
+    context 'unknown key' do
+      let!(:commit) { create :commit, project: project, sha: commit_sha }
+
       before do
         allow(Rugged::Commit).to receive(:extract_signature)
           .with(Rugged::Repository, commit_sha)
@@ -102,19 +145,20 @@ describe Gitlab::Gpg::Commit do
       end
 
       it 'returns an invalid signature' do
-        expect(described_class.new(project, commit_sha).signature).to have_attributes(
+        expect(described_class.new(commit).signature).to have_attributes(
           commit_sha: commit_sha,
           project: project,
           gpg_key: nil,
           gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid,
           gpg_key_user_name: nil,
           gpg_key_user_email: nil,
-          valid_signature: false
+          valid_signature: false,
+          verification_status: 'unknown_key'
         )
       end
 
       it 'returns the cached signature on second call' do
-        gpg_commit = described_class.new(project, commit_sha)
+        gpg_commit = described_class.new(commit)
 
         expect(gpg_commit).to receive(:using_keychain).and_call_original
         gpg_commit.signature

spec/models/gpg_key_spec.rb

@@ -99,14 +99,14 @@ describe GpgKey do
   end
 
   describe '#verified?' do
-    it 'returns true one of the email addresses in the key belongs to the user' do
+    it 'returns true if one of the email addresses in the key belongs to the user' do
       user = create :user, email: 'bette.cartwright@example.com'
       gpg_key = create :gpg_key, key: GpgHelpers::User2.public_key, user: user
 
       expect(gpg_key.verified?).to be_truthy
     end
 
-    it 'returns false if one of the email addresses in the key does not belong to the user' do
+    it 'returns false if none of the email addresses in the key does not belong to the user' do
       user = create :user, email: 'someone.else@example.com'
       gpg_key = create :gpg_key, key: GpgHelpers::User2.public_key, user: user
 
@@ -114,6 +114,32 @@ describe GpgKey do
     end
   end
 
+  describe 'verified_and_belongs_to_email?' do
+    it 'returns false if none of the email addresses in the key does not belong to the user' do
+      user = create :user, email: 'someone.else@example.com'
+      gpg_key = create :gpg_key, key: GpgHelpers::User2.public_key, user: user
+
+      expect(gpg_key.verified?).to be_falsey
+      expect(gpg_key.verified_and_belongs_to_email?('someone.else@example.com')).to be_falsey
+    end
+
+    it 'returns false if one of the email addresses in the key belongs to the user and does not match the provided email' do
+      user = create :user, email: 'bette.cartwright@example.com'
+      gpg_key = create :gpg_key, key: GpgHelpers::User2.public_key, user: user
+
+      expect(gpg_key.verified?).to be_truthy
+      expect(gpg_key.verified_and_belongs_to_email?('bette.cartwright@example.net')).to be_falsey
+    end
+
+    it 'returns true if one of the email addresses in the key belongs to the user and matches the provided email' do
+      user = create :user, email: 'bette.cartwright@example.com'
+      gpg_key = create :gpg_key, key: GpgHelpers::User2.public_key, user: user
+
+      expect(gpg_key.verified?).to be_truthy
+      expect(gpg_key.verified_and_belongs_to_email?('bette.cartwright@example.com')).to be_truthy
+    end
+  end
+
   describe 'notification', :mailer do
     let(:user) { create(:user) }