From cd01e82873b3cd471203dbf557c71571fd683d16 Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Thu, 13 Jul 2017 15:22:15 +0200 Subject: [PATCH] store gpg user name and email on the signature --- app/models/gpg_key.rb | 16 ++++++++---- ...add_gpg_key_user_info_to_gpg_signatures.rb | 11 ++++++++ db/schema.rb | 2 ++ lib/gitlab/gpg.rb | 6 +++-- lib/gitlab/gpg/commit.rb | 21 ++++++++++----- spec/lib/gitlab/gpg/commit_spec.rb | 6 +++++ spec/lib/gitlab/gpg_spec.rb | 14 +++++----- spec/models/gpg_key_spec.rb | 26 ++++++++++++++++--- spec/support/gpg_helpers.rb | 8 ++++++ 9 files changed, 88 insertions(+), 22 deletions(-) create mode 100644 db/migrate/20170713104235_add_gpg_key_user_info_to_gpg_signatures.rb diff --git a/app/models/gpg_key.rb b/app/models/gpg_key.rb index 1977023536e..31a25f3e2f0 100644 --- a/app/models/gpg_key.rb +++ b/app/models/gpg_key.rb @@ -37,15 +37,21 @@ class GpgKey < ActiveRecord::Base write_attribute(:key, value) end - def emails - @emails ||= Gitlab::Gpg.emails_from_key(key) + def user_infos + @user_infos ||= Gitlab::Gpg.user_infos_from_key(key) + end + + def verified_user_infos + user_infos.select do |user_info| + user_info[:email] == user.email + end end def emails_with_verified_status - emails.map do |email| + user_infos.map do |user_info| [ - email, - email == user.email + user_info[:email], + user_info[:email] == user.email ] end.to_h end diff --git a/db/migrate/20170713104235_add_gpg_key_user_info_to_gpg_signatures.rb b/db/migrate/20170713104235_add_gpg_key_user_info_to_gpg_signatures.rb new file mode 100644 index 00000000000..0e51a86e64c --- /dev/null +++ b/db/migrate/20170713104235_add_gpg_key_user_info_to_gpg_signatures.rb @@ -0,0 +1,11 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddGpgKeyUserInfoToGpgSignatures < ActiveRecord::Migration + DOWNTIME = false + + def change + add_column :gpg_signatures, :gpg_key_user_name, :string + add_column :gpg_signatures, :gpg_key_user_email, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 53b1e83ddab..b76a5efbbd7 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -560,6 +560,8 @@ ActiveRecord::Schema.define(version: 20170725145659) do t.boolean "valid_signature" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.string "gpg_key_user_name" + t.string "gpg_key_user_email" end add_index "gpg_signatures", ["commit_sha"], name: "index_gpg_signatures_on_commit_sha", using: :btree diff --git a/lib/gitlab/gpg.rb b/lib/gitlab/gpg.rb index 582347019e5..e1d1724295a 100644 --- a/lib/gitlab/gpg.rb +++ b/lib/gitlab/gpg.rb @@ -32,11 +32,13 @@ module Gitlab end end - def emails_from_key(key) + def user_infos_from_key(key) using_tmp_keychain do fingerprints = CurrentKeyChain.fingerprints_from_key(key) - GPGME::Key.find(:public, fingerprints).flat_map { |raw_key| raw_key.uids.map(&:email) } + GPGME::Key.find(:public, fingerprints).flat_map do |raw_key| + raw_key.uids.map { |uid| { name: uid.name, email: uid.email } } + end end end diff --git a/lib/gitlab/gpg/commit.rb b/lib/gitlab/gpg/commit.rb index 50e8d71bb13..55428b85207 100644 --- a/lib/gitlab/gpg/commit.rb +++ b/lib/gitlab/gpg/commit.rb @@ -26,10 +26,7 @@ module Gitlab def update_signature!(cached_signature) using_keychain do |gpg_key| - cached_signature.update_attributes!( - valid_signature: gpg_signature_valid_signature_value(gpg_key), - gpg_key: gpg_key - ) + cached_signature.update_attributes!(attributes(gpg_key)) end end @@ -59,18 +56,30 @@ module Gitlab end def create_cached_signature!(gpg_key) - GpgSignature.create!( + GpgSignature.create!(attributes(gpg_key)) + end + + def attributes(gpg_key) + user_infos = user_infos(gpg_key) + + { commit_sha: commit.sha, project: commit.project, gpg_key: gpg_key, 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) - ) + } end def gpg_signature_valid_signature_value(gpg_key) !!(gpg_key && gpg_key.verified? && verified_signature.valid?) end + + def user_infos(gpg_key) + gpg_key&.verified_user_infos&.first || gpg_key&.user_infos&.first || {} + end end end end diff --git a/spec/lib/gitlab/gpg/commit_spec.rb b/spec/lib/gitlab/gpg/commit_spec.rb index 661956b7bb7..ddb8dd9f0f4 100644 --- a/spec/lib/gitlab/gpg/commit_spec.rb +++ b/spec/lib/gitlab/gpg/commit_spec.rb @@ -32,6 +32,8 @@ RSpec.describe Gitlab::Gpg::Commit do 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 ) end @@ -67,6 +69,8 @@ RSpec.describe Gitlab::Gpg::Commit do 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 @@ -102,6 +106,8 @@ RSpec.describe Gitlab::Gpg::Commit do 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 ) end diff --git a/spec/lib/gitlab/gpg_spec.rb b/spec/lib/gitlab/gpg_spec.rb index ebb7720eaea..8041518117d 100644 --- a/spec/lib/gitlab/gpg_spec.rb +++ b/spec/lib/gitlab/gpg_spec.rb @@ -28,16 +28,18 @@ describe Gitlab::Gpg do end end - describe '.emails_from_key' do - it 'returns the emails' do - expect( - described_class.emails_from_key(GpgHelpers::User1.public_key) - ).to eq GpgHelpers::User1.emails + describe '.user_infos_from_key' do + it 'returns the names and emails' do + user_infos = described_class.user_infos_from_key(GpgHelpers::User1.public_key) + expect(user_infos).to eq([{ + name: GpgHelpers::User1.names.first, + email: GpgHelpers::User1.emails.first + }]) end it 'returns an empty array when the key is invalid' do expect( - described_class.emails_from_key('bogus') + described_class.user_infos_from_key('bogus') ).to eq [] end end diff --git a/spec/models/gpg_key_spec.rb b/spec/models/gpg_key_spec.rb index ddd0bbfb9ba..06bdbb59a11 100644 --- a/spec/models/gpg_key_spec.rb +++ b/spec/models/gpg_key_spec.rb @@ -46,11 +46,31 @@ describe GpgKey do end end - describe '#emails' do - it 'returns the emails from the gpg key' do + describe '#user_infos' do + it 'returns the user infos from the gpg key' do gpg_key = create :gpg_key, key: GpgHelpers::User1.public_key + expect(Gitlab::Gpg).to receive(:user_infos_from_key).with(gpg_key.key) - expect(gpg_key.emails).to eq GpgHelpers::User1.emails + gpg_key.user_infos + end + end + + describe '#verified_user_infos' do + it 'returns the user infos if it is verified' do + user = create :user, email: GpgHelpers::User1.emails.first + gpg_key = create :gpg_key, key: GpgHelpers::User1.public_key, user: user + + expect(gpg_key.verified_user_infos).to eq([{ + name: GpgHelpers::User1.names.first, + email: GpgHelpers::User1.emails.first + }]) + end + + it 'returns an empty array if the user info is not verified' do + user = create :user, email: 'unrelated@example.com' + gpg_key = create :gpg_key, key: GpgHelpers::User1.public_key, user: user + + expect(gpg_key.verified_user_infos).to eq([]) end end diff --git a/spec/support/gpg_helpers.rb b/spec/support/gpg_helpers.rb index f9128a629f2..96ea6f28b30 100644 --- a/spec/support/gpg_helpers.rb +++ b/spec/support/gpg_helpers.rb @@ -98,6 +98,10 @@ module GpgHelpers '5F7EA3981A5845B141ABD522CCFBE19F00AC8B1D' end + def names + ['Nannie Bernhard'] + end + def emails ['nannie.bernhard@example.com'] end @@ -187,6 +191,10 @@ module GpgHelpers '6D494CA6FC90C0CAE0910E42BF9D925F911EFD65' end + def names + ['Bette Cartwright', 'Bette Cartwright'] + end + def emails ['bette.cartwright@example.com', 'bette.cartwright@example.net'] end