use new #verification_status

app/models/gpg_key.rb

@@ -82,11 +82,14 @@ class GpgKey < ActiveRecord::Base
   end
 
   def revoke
-    GpgSignature.where(gpg_key: self, valid_signature: true).update_all(
-      gpg_key_id: nil,
-      valid_signature: false,
-      updated_at: Time.zone.now
-    )
+    GpgSignature
+      .where(gpg_key: self)
+      .where.not(verification_status: GpgSignature.verification_statuses[:unknown_key])
+      .update_all(
+        gpg_key_id: nil,
+        verification_status: GpgSignature.verification_statuses[:unknown_key],
+        updated_at: Time.zone.now
+      )
 
     destroy
   end

app/models/gpg_signature.rb

@@ -20,6 +20,14 @@ class GpgSignature < ActiveRecord::Base
   validates :project_id, presence: true
   validates :gpg_key_primary_keyid, presence: true
 
+  # backwards compatibility: legacy records that weren't migrated to use the
+  # new `#verification_status` have `#valid_signature` set instead
+  def verified?
+    return valid_signature if verification_status.nil?
+
+    super
+  end
+
   def gpg_key_primary_keyid
     super&.upcase
   end

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

 - if signature
-  - if signature.valid_signature?
+  - if signature.verified?
     = render partial: 'projects/commit/valid_signature_badge', locals: { signature: signature }
   - else
     = render partial: 'projects/commit/invalid_signature_badge', locals: { signature: signature }

lib/gitlab/gpg/commit.rb

@@ -77,7 +77,6 @@ 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: verification_status == :verified,
           verification_status: verification_status
         }
       end

lib/gitlab/gpg/invalid_gpg_signature_updater.rb

@@ -6,9 +6,15 @@ module Gitlab
       end
 
       def run
+        # `OR valid_signature` is for backwards compatibility: legacy records
+        # that weren't migrated to use the new `#verification_status` have
+        # `#valid_signature` set instead
         GpgSignature
           .select(:id, :commit_sha, :project_id)
-          .where('gpg_key_id IS NULL OR valid_signature = ?', false)
+          .where('gpg_key_id IS NULL OR valid_signature = ? OR verification_status <> ?',
+            false,
+            GpgSignature.verification_statuses[:verified]
+          )
           .where(gpg_key_primary_keyid: @gpg_key.primary_keyid)
           .find_each { |sig| sig.gpg_commit.update_signature!(sig) }
       end

spec/factories/gpg_signature.rb

@@ -6,6 +6,6 @@ FactoryGirl.define do
     project
     gpg_key
     gpg_key_primary_keyid { gpg_key.primary_keyid }
-    valid_signature true
+    verification_status :verified
   end
 end

spec/features/profiles/gpg_keys_spec.rb

@@ -42,7 +42,7 @@ feature 'Profile > GPG Keys' do
 
   scenario 'User revokes a key via the key index' do
     gpg_key = create :gpg_key, user: user, key: GpgHelpers::User2.public_key
-    gpg_signature = create :gpg_signature, gpg_key: gpg_key, valid_signature: true
+    gpg_signature = create :gpg_signature, gpg_key: gpg_key, verification_status: :verified
 
     visit profile_gpg_keys_path
 
@@ -51,7 +51,7 @@ feature 'Profile > GPG Keys' do
     expect(page).to have_content('Your GPG keys (0)')
 
     expect(gpg_signature.reload).to have_attributes(
-      valid_signature: false,
+      verification_status: 'unknown_key',
       gpg_key: nil
     )
   end

spec/lib/gitlab/gpg/commit_spec.rb

@@ -56,7 +56,6 @@ describe Gitlab::Gpg::Commit do
               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
@@ -96,7 +95,6 @@ describe Gitlab::Gpg::Commit do
               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: 'same_user_different_email'
             )
           end
@@ -132,7 +130,6 @@ describe Gitlab::Gpg::Commit do
               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
@@ -169,7 +166,6 @@ describe Gitlab::Gpg::Commit do
             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
@@ -200,7 +196,6 @@ describe Gitlab::Gpg::Commit do
           gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid,
           gpg_key_user_name: nil,
           gpg_key_user_email: nil,
-          valid_signature: false,
           verification_status: 'unknown_key'
         )
       end

spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb

@@ -46,7 +46,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do
           commit_sha: commit_sha,
           gpg_key: nil,
           gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid,
-          valid_signature: true
+          verification_status: 'verified'
       end
 
       it 'assigns the gpg key to the signature when the missing gpg key is added' do
@@ -60,7 +60,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do
           commit_sha: commit_sha,
           gpg_key: gpg_key,
           gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid,
-          valid_signature: true
+          verification_status: 'verified'
         )
       end
 
@@ -75,7 +75,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do
           commit_sha: commit_sha,
           gpg_key: nil,
           gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid,
-          valid_signature: true
+          verification_status: 'verified'
         )
       end
     end
@@ -89,7 +89,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do
           commit_sha: commit_sha,
           gpg_key: nil,
           gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid,
-          valid_signature: false
+          verification_status: 'unknown_key'
       end
 
       it 'updates the signature to being valid when the missing gpg key is added' do
@@ -103,7 +103,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do
           commit_sha: commit_sha,
           gpg_key: gpg_key,
           gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid,
-          valid_signature: true
+          verification_status: 'verified'
         )
       end
 
@@ -118,7 +118,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do
           commit_sha: commit_sha,
           gpg_key: nil,
           gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid,
-          valid_signature: false
+          verification_status: 'unknown_key'
         )
       end
     end
@@ -136,7 +136,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do
           commit_sha: commit_sha,
           gpg_key: nil,
           gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid,
-          valid_signature: false
+          verification_status: 'unknown_key'
       end
 
       it 'updates the signature to being valid when the user updates the email address' do
@@ -144,7 +144,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do
           key: GpgHelpers::User1.public_key,
           user: user
 
-        expect(invalid_gpg_signature.reload.valid_signature).to be_falsey
+        expect(invalid_gpg_signature.reload.verification_status).to eq 'unverified_key'
 
         # InvalidGpgSignatureUpdater is called by the after_update hook
         user.update_attributes!(email: GpgHelpers::User1.emails.first)
@@ -154,7 +154,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do
           commit_sha: commit_sha,
           gpg_key: gpg_key,
           gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid,
-          valid_signature: true
+          verification_status: 'verified'
         )
       end
 
@@ -168,7 +168,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do
           commit_sha: commit_sha,
           gpg_key: gpg_key,
           gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid,
-          valid_signature: false
+          verification_status: 'unverified_key'
         )
 
         # InvalidGpgSignatureUpdater is called by the after_update hook
@@ -179,7 +179,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do
           commit_sha: commit_sha,
           gpg_key: gpg_key,
           gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid,
-          valid_signature: false
+          verification_status: 'unverified_key'
         )
       end
     end

spec/models/gpg_key_spec.rb

@@ -155,15 +155,15 @@ describe GpgKey do
   describe '#revoke' do
     it 'invalidates all associated gpg signatures and destroys the key' do
       gpg_key = create :gpg_key
-      gpg_signature = create :gpg_signature, valid_signature: true, gpg_key: gpg_key
+      gpg_signature = create :gpg_signature, verification_status: :verified, gpg_key: gpg_key
 
       unrelated_gpg_key = create :gpg_key, key: GpgHelpers::User2.public_key
-      unrelated_gpg_signature = create :gpg_signature, valid_signature: true, gpg_key: unrelated_gpg_key
+      unrelated_gpg_signature = create :gpg_signature, verification_status: :verified, gpg_key: unrelated_gpg_key
 
       gpg_key.revoke
 
       expect(gpg_signature.reload).to have_attributes(
-        valid_signature: false,
+        verification_status: 'unknown_key',
         gpg_key: nil
       )
 
@@ -171,7 +171,7 @@ describe GpgKey do
 
       # unrelated signature is left untouched
       expect(unrelated_gpg_signature.reload).to have_attributes(
-        valid_signature: true,
+        verification_status: 'verified',
         gpg_key: unrelated_gpg_key
       )
 

spec/models/gpg_signature_spec.rb

@@ -25,4 +25,34 @@ RSpec.describe GpgSignature do
       gpg_signature.commit
     end
   end
+
+  describe '#verified?' do
+    it 'returns true when `verification_status` is not set, but `valid_signature` is true' do
+      signature = create :gpg_signature, valid_signature: true, verification_status: nil
+
+      expect(signature.verified?).to be true
+      expect(signature.reload.verified?).to be true
+    end
+
+    it 'returns true when `verification_status` is set to :verified' do
+      signature = create :gpg_signature, verification_status: :verified
+
+      expect(signature.verified?).to be true
+      expect(signature.reload.verified?).to be true
+    end
+
+    it 'returns false when `verification_status` is set to :unknown_key' do
+      signature = create :gpg_signature, verification_status: :unknown_key
+
+      expect(signature.verified?).to be false
+      expect(signature.reload.verified?).to be false
+    end
+
+    it 'returns false when `verification_status` is not set, but `valid_signature` is false' do
+      signature = create :gpg_signature, valid_signature: false, verification_status: nil
+
+      expect(signature.verified?).to be false
+      expect(signature.reload.verified?).to be false
+    end
+  end
 end