diff options
-rw-r--r-- | app/models/gpg_key.rb | 13 | ||||
-rw-r--r-- | app/models/gpg_signature.rb | 8 | ||||
-rw-r--r-- | app/views/projects/commit/_signature.html.haml | 2 | ||||
-rw-r--r-- | lib/gitlab/gpg/commit.rb | 1 | ||||
-rw-r--r-- | lib/gitlab/gpg/invalid_gpg_signature_updater.rb | 8 | ||||
-rw-r--r-- | spec/factories/gpg_signature.rb | 2 | ||||
-rw-r--r-- | spec/features/profiles/gpg_keys_spec.rb | 4 | ||||
-rw-r--r-- | spec/lib/gitlab/gpg/commit_spec.rb | 5 | ||||
-rw-r--r-- | spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb | 22 | ||||
-rw-r--r-- | spec/models/gpg_key_spec.rb | 8 | ||||
-rw-r--r-- | spec/models/gpg_signature_spec.rb | 30 |
11 files changed, 72 insertions, 31 deletions
diff --git a/app/models/gpg_key.rb b/app/models/gpg_key.rb index 1c1e2b585f0..1633acd4fa9 100644 --- a/app/models/gpg_key.rb +++ b/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 diff --git a/app/models/gpg_signature.rb b/app/models/gpg_signature.rb index 1f047a32c84..4266b1927d8 100644 --- a/app/models/gpg_signature.rb +++ b/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 diff --git a/app/views/projects/commit/_signature.html.haml b/app/views/projects/commit/_signature.html.haml index 60fa52557ef..7598aa90b5b 100644 --- a/app/views/projects/commit/_signature.html.haml +++ b/app/views/projects/commit/_signature.html.haml @@ -1,5 +1,5 @@ - 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 } diff --git a/lib/gitlab/gpg/commit.rb b/lib/gitlab/gpg/commit.rb index a206249ef5a..86bd9f5b125 100644 --- a/lib/gitlab/gpg/commit.rb +++ b/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 diff --git a/lib/gitlab/gpg/invalid_gpg_signature_updater.rb b/lib/gitlab/gpg/invalid_gpg_signature_updater.rb index a525ee7a9ee..7bdf6760295 100644 --- a/lib/gitlab/gpg/invalid_gpg_signature_updater.rb +++ b/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 diff --git a/spec/factories/gpg_signature.rb b/spec/factories/gpg_signature.rb index a5aeffbe12d..c0beecf0bea 100644 --- a/spec/factories/gpg_signature.rb +++ b/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 diff --git a/spec/features/profiles/gpg_keys_spec.rb b/spec/features/profiles/gpg_keys_spec.rb index 6edc482b47e..623e4f341c5 100644 --- a/spec/features/profiles/gpg_keys_spec.rb +++ b/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 diff --git a/spec/lib/gitlab/gpg/commit_spec.rb b/spec/lib/gitlab/gpg/commit_spec.rb index 40113429d23..b07462e4978 100644 --- a/spec/lib/gitlab/gpg/commit_spec.rb +++ b/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 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 120cd1b74f7..b9fd4d02156 100644 --- a/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb +++ b/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 diff --git a/spec/models/gpg_key_spec.rb b/spec/models/gpg_key_spec.rb index d436aee6ea3..9c99c3e5c08 100644 --- a/spec/models/gpg_key_spec.rb +++ b/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 ) diff --git a/spec/models/gpg_signature_spec.rb b/spec/models/gpg_signature_spec.rb index c58fd46762a..d77eeda4a79 100644 --- a/spec/models/gpg_signature_spec.rb +++ b/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 |