summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/models/gpg_key.rb13
-rw-r--r--app/models/gpg_signature.rb8
-rw-r--r--app/views/projects/commit/_signature.html.haml2
-rw-r--r--lib/gitlab/gpg/commit.rb1
-rw-r--r--lib/gitlab/gpg/invalid_gpg_signature_updater.rb8
-rw-r--r--spec/factories/gpg_signature.rb2
-rw-r--r--spec/features/profiles/gpg_keys_spec.rb4
-rw-r--r--spec/lib/gitlab/gpg/commit_spec.rb5
-rw-r--r--spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb22
-rw-r--r--spec/models/gpg_key_spec.rb8
-rw-r--r--spec/models/gpg_signature_spec.rb30
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