From 3c42d730986222d891c9b7985edf3942021afcef Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Tue, 13 Jun 2017 13:46:43 +0200 Subject: [PATCH] add primary keyid attribute to gpg keys --- app/models/gpg_key.rb | 15 ++++++++++++++- ...70613103429_add_primary_keyid_to_gpg_keys.rb | 17 +++++++++++++++++ db/schema.rb | 2 ++ lib/gitlab/gpg.rb | 12 ++++++++++++ spec/features/commits_spec.rb | 4 ++-- spec/lib/gitlab/gpg_spec.rb | 14 ++++++++++++++ spec/models/gpg_key_spec.rb | 8 ++++++++ spec/support/gpg_helpers.rb | 8 ++++---- 8 files changed, 73 insertions(+), 7 deletions(-) create mode 100644 db/migrate/20170613103429_add_primary_keyid_to_gpg_keys.rb diff --git a/app/models/gpg_key.rb b/app/models/gpg_key.rb index d4570e36e06..26f9a3975c9 100644 --- a/app/models/gpg_key.rb +++ b/app/models/gpg_key.rb @@ -20,7 +20,14 @@ class GpgKey < ActiveRecord::Base # the error about the fingerprint unless: -> { errors.has_key?(:key) } - before_validation :extract_fingerprint + validates :primary_keyid, + presence: true, + uniqueness: true, + # only validate when the `key` is valid, as we don't want the user to show + # the error about the fingerprint + unless: -> { errors.has_key?(:key) } + + before_validation :extract_fingerprint, :extract_primary_keyid after_create :notify_user def key=(value) @@ -49,6 +56,12 @@ class GpgKey < ActiveRecord::Base self.fingerprint = Gitlab::Gpg.fingerprints_from_key(key).first end + def extract_primary_keyid + # we can assume that the result only contains one item as the validation + # only allows one key + self.primary_keyid = Gitlab::Gpg.primary_keyids_from_key(key).first + end + def notify_user run_after_commit { NotificationService.new.new_gpg_key(self) } end diff --git a/db/migrate/20170613103429_add_primary_keyid_to_gpg_keys.rb b/db/migrate/20170613103429_add_primary_keyid_to_gpg_keys.rb new file mode 100644 index 00000000000..13f0500971b --- /dev/null +++ b/db/migrate/20170613103429_add_primary_keyid_to_gpg_keys.rb @@ -0,0 +1,17 @@ +class AddPrimaryKeyidToGpgKeys < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column :gpg_keys, :primary_keyid, :string + add_concurrent_index :gpg_keys, :primary_keyid + end + + def down + remove_concurrent_index :gpg_keys, :primary_keyid if index_exists?(:gpg_keys, :primary_keyid) + remove_column :gpg_keys, :primary_keyid, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 54f98559243..fbf20f4eb66 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -546,8 +546,10 @@ ActiveRecord::Schema.define(version: 20170725145659) do t.integer "user_id" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.string "primary_keyid" end + add_index "gpg_keys", ["primary_keyid"], name: "index_gpg_keys_on_primary_keyid", using: :btree add_index "gpg_keys", ["user_id"], name: "index_gpg_keys_on_user_id", using: :btree create_table "identities", force: :cascade do |t| diff --git a/lib/gitlab/gpg.rb b/lib/gitlab/gpg.rb index 384a9138fa1..486e040adb6 100644 --- a/lib/gitlab/gpg.rb +++ b/lib/gitlab/gpg.rb @@ -12,6 +12,18 @@ module Gitlab end end + def primary_keyids_from_key(key) + using_tmp_keychain do + import = GPGME::Key.import(key) + + return [] if import.imported == 0 + + fingerprints = import.imports.map(&:fingerprint) + + GPGME::Key.find(:public, fingerprints).map { |raw_key| raw_key.primary_subkey.keyid } + end + end + def emails_from_key(key) using_tmp_keychain do import = GPGME::Key.import(key) diff --git a/spec/features/commits_spec.rb b/spec/features/commits_spec.rb index 79952eda2ff..1dbcf09d4a0 100644 --- a/spec/features/commits_spec.rb +++ b/spec/features/commits_spec.rb @@ -220,8 +220,8 @@ describe 'Commits' do Dir.mktmpdir do |dir| FileUtils.cd dir do `git clone --quiet #{remote_path} .` - `git commit --quiet -S#{GpgHelpers::User1.key_id} --allow-empty -m "signed commit, verified key/email"` - `git commit --quiet -S#{GpgHelpers::User2.key_id} --allow-empty -m "signed commit, unverified key/email"` + `git commit --quiet -S#{GpgHelpers::User1.primary_keyid} --allow-empty -m "signed commit, verified key/email"` + `git commit --quiet -S#{GpgHelpers::User2.primary_keyid} --allow-empty -m "signed commit, unverified key/email"` `git push --quiet` end end diff --git a/spec/lib/gitlab/gpg_spec.rb b/spec/lib/gitlab/gpg_spec.rb index bdcf9ee0e65..55f34e0cf99 100644 --- a/spec/lib/gitlab/gpg_spec.rb +++ b/spec/lib/gitlab/gpg_spec.rb @@ -15,6 +15,20 @@ describe Gitlab::Gpg do end end + describe '.primary_keyids_from_key' do + it 'returns the keyid' do + expect( + described_class.primary_keyids_from_key(GpgHelpers::User1.public_key) + ).to eq [GpgHelpers::User1.primary_keyid] + end + + it 'returns an empty array when the key is invalid' do + expect( + described_class.primary_keyids_from_key('bogus') + ).to eq [] + end + end + describe '.emails_from_key' do it 'returns the emails' do expect( diff --git a/spec/models/gpg_key_spec.rb b/spec/models/gpg_key_spec.rb index 6ee436b6a6d..ac446fca819 100644 --- a/spec/models/gpg_key_spec.rb +++ b/spec/models/gpg_key_spec.rb @@ -21,6 +21,14 @@ describe GpgKey do expect(gpg_key.fingerprint).to eq GpgHelpers::User1.fingerprint end end + + describe 'extract_primary_keyid' do + it 'extracts the primary keyid from the gpg key' do + gpg_key = described_class.new(key: GpgHelpers::User1.public_key) + gpg_key.valid? + expect(gpg_key.primary_keyid).to eq GpgHelpers::User1.primary_keyid + end + end end describe '#key=' do diff --git a/spec/support/gpg_helpers.rb b/spec/support/gpg_helpers.rb index 52c478e1976..f9128a629f2 100644 --- a/spec/support/gpg_helpers.rb +++ b/spec/support/gpg_helpers.rb @@ -90,8 +90,8 @@ module GpgHelpers KEY end - def key_id - '00AC8B1D' + def primary_keyid + fingerprint[-16..-1] end def fingerprint @@ -179,8 +179,8 @@ module GpgHelpers KEY end - def key_id - '911EFD65' + def primary_keyid + fingerprint[-16..-1] end def fingerprint -- GitLab