From 7e13d96715750f74db399bf40ee4ec9679bbe806 Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Mon, 12 Jun 2017 16:16:33 +0200 Subject: [PATCH] don't sync to keychain file --- app/models/gpg_key.rb | 26 +---------- app/models/user.rb | 7 --- lib/gitlab/gpg.rb | 18 ------- spec/lib/gitlab/gpg_spec.rb | 33 ------------- spec/models/gpg_key_spec.rb | 93 +++---------------------------------- spec/models/user_spec.rb | 20 -------- 6 files changed, 8 insertions(+), 189 deletions(-) diff --git a/app/models/gpg_key.rb b/app/models/gpg_key.rb index 8332ba3ee6e..d4570e36e06 100644 --- a/app/models/gpg_key.rb +++ b/app/models/gpg_key.rb @@ -21,9 +21,7 @@ class GpgKey < ActiveRecord::Base unless: -> { errors.has_key?(:key) } before_validation :extract_fingerprint - after_create :synchronize_keychain after_create :notify_user - after_destroy :synchronize_keychain def key=(value) value.strip! unless value.blank? @@ -34,29 +32,15 @@ class GpgKey < ActiveRecord::Base @emails ||= Gitlab::Gpg.emails_from_key(key) end - def emails_in_keychain - @emails_in_keychain ||= Gitlab::Gpg::CurrentKeyChain.emails(fingerprint) - end - def emails_with_verified_status emails.map do |email| [ email, - email == user.email && emails_in_keychain.include?(email) + email == user.email ] end end - def synchronize_keychain - if emails.include?(user.email) - add_to_keychain - else - remove_from_keychain - end - - @emails_in_keychain = nil - end - private def extract_fingerprint @@ -65,14 +49,6 @@ class GpgKey < ActiveRecord::Base self.fingerprint = Gitlab::Gpg.fingerprints_from_key(key).first end - def add_to_keychain - Gitlab::Gpg::CurrentKeyChain.add(key) - end - - def remove_from_keychain - Gitlab::Gpg::CurrentKeyChain.remove(fingerprint) - end - def notify_user run_after_commit { NotificationService.new.new_gpg_key(self) } end diff --git a/app/models/user.rb b/app/models/user.rb index 42c83e3d206..5aebd36cf8a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -155,7 +155,6 @@ class User < ActiveRecord::Base before_validation :set_public_email, if: :public_email_changed? after_update :update_emails_with_primary_email, if: :email_changed? - after_update :synchronize_gpg_keys, if: :email_changed? before_save :ensure_authentication_token, :ensure_incoming_email_token before_save :ensure_user_rights_and_limits, if: :external_changed? after_save :ensure_namespace_correct @@ -1158,10 +1157,4 @@ class User < ActiveRecord::Base ensure Gitlab::ExclusiveLease.cancel(lease_key, uuid) end - - def synchronize_gpg_keys - gpg_keys.each do |gpg_key| - gpg_key.synchronize_keychain - end - end end diff --git a/lib/gitlab/gpg.rb b/lib/gitlab/gpg.rb index ee0467ae264..384a9138fa1 100644 --- a/lib/gitlab/gpg.rb +++ b/lib/gitlab/gpg.rb @@ -2,24 +2,6 @@ module Gitlab module Gpg extend self - module CurrentKeyChain - extend self - - def add(key) - GPGME::Key.import(key) - end - - def remove(fingerprint) - # `#get` raises an EOFError if the keychain is empty, which is why we - # use the friendlier `#find` - GPGME::Key.find(:public, fingerprint).each(&:delete!) - end - - def emails(fingerprint) - GPGME::Key.find(:public, fingerprint).flat_map { |raw_key| raw_key.uids.map(&:email) } - end - end - def fingerprints_from_key(key) using_tmp_keychain do import = GPGME::Key.import(key) diff --git a/spec/lib/gitlab/gpg_spec.rb b/spec/lib/gitlab/gpg_spec.rb index c0df719c0c2..bdcf9ee0e65 100644 --- a/spec/lib/gitlab/gpg_spec.rb +++ b/spec/lib/gitlab/gpg_spec.rb @@ -29,36 +29,3 @@ describe Gitlab::Gpg do end end end - -describe Gitlab::Gpg::CurrentKeyChain, :gpg do - describe '.emails' do - it 'returns the emails' do - Gitlab::Gpg::CurrentKeyChain.add(GpgHelpers::User2.public_key) - - expect( - described_class.emails(GpgHelpers::User2.fingerprint) - ).to match_array GpgHelpers::User2.emails - end - end - - describe '.add', :gpg do - it 'stores the key in the keychain' do - expect(GPGME::Key.find(:public, GpgHelpers::User1.fingerprint)).to eq [] - - Gitlab::Gpg::CurrentKeyChain.add(GpgHelpers::User1.public_key) - - expect(GPGME::Key.find(:public, GpgHelpers::User1.fingerprint)).not_to eq [] - end - end - - describe '.remove', :gpg do - it 'removes the key from the keychain' do - Gitlab::Gpg::CurrentKeyChain.add(GpgHelpers::User1.public_key) - expect(GPGME::Key.find(:public, GpgHelpers::User1.fingerprint)).not_to eq [] - - Gitlab::Gpg::CurrentKeyChain.remove(GpgHelpers::User1.fingerprint) - - expect(GPGME::Key.find(:public, GpgHelpers::User1.fingerprint)).to eq [] - end - end -end diff --git a/spec/models/gpg_key_spec.rb b/spec/models/gpg_key_spec.rb index 18746ad9d88..6ee436b6a6d 100644 --- a/spec/models/gpg_key_spec.rb +++ b/spec/models/gpg_key_spec.rb @@ -21,20 +21,6 @@ describe GpgKey do expect(gpg_key.fingerprint).to eq GpgHelpers::User1.fingerprint end end - - describe 'synchronize_keychain' do - it 'calls #synchronize_keychain after create' do - gpg_key = build :gpg_key - expect(gpg_key).to receive(:synchronize_keychain) - gpg_key.save! - end - - it 'calls #remove_from_keychain after destroy' do - gpg_key = create :gpg_key - expect(gpg_key).to receive(:synchronize_keychain) - gpg_key.destroy! - end - end end describe '#key=' do @@ -59,80 +45,15 @@ describe GpgKey do end end - describe '#emails_in_keychain', :gpg do - it 'returns the emails from the keychain' do - user = create :user, email: GpgHelpers::User1.emails.first - gpg_key = create :gpg_key, key: GpgHelpers::User1.public_key, user: user - - expect(gpg_key.emails_in_keychain).to eq GpgHelpers::User1.emails - end - end - describe '#emails_with_verified_status', :gpg do - context 'key is in the keychain' do - it 'email is verified if the user has the matching email' do - user = create :user, email: 'bette.cartwright@example.com' - gpg_key = create :gpg_key, key: GpgHelpers::User2.public_key, user: user + it 'email is verified if the user has the matching email' do + user = create :user, email: 'bette.cartwright@example.com' + gpg_key = create :gpg_key, key: GpgHelpers::User2.public_key, user: user - expect(gpg_key.emails_with_verified_status).to match_array [ - ['bette.cartwright@example.com', true], - ['bette.cartwright@example.net', false] - ] - end - end - - context 'key is in not the keychain' do - it 'emails are unverified' do - user = create :user, email: 'bette.cartwright@example.com' - gpg_key = create :gpg_key, key: GpgHelpers::User2.public_key, user: user - - Gitlab::Gpg::CurrentKeyChain.remove(GpgHelpers::User2.fingerprint) - - expect(gpg_key.emails_with_verified_status).to match_array [ - ['bette.cartwright@example.com', false], - ['bette.cartwright@example.net', false] - ] - end - end - end - - describe '#synchronize_keychain', :gpg do - context "user's email matches one of the key's emails" do - it 'adds the key to the keychain' do - user = create :user, email: GpgHelpers::User1.emails.first - gpg_key = create :gpg_key, user: user - - expect(gpg_key).to receive(:add_to_keychain) - - gpg_key.synchronize_keychain - end - end - - context "user's email does not match one of the key's emails" do - it 'does not add the key to the keychain' do - user = create :user, email: 'stepanie@cole.us' - gpg_key = create :gpg_key, user: user - - expect(gpg_key).to receive(:remove_from_keychain) - - gpg_key.synchronize_keychain - end - end - end - - describe '#add_to_keychain', :gpg do - it 'calls .add_to_keychain' do - expect(Gitlab::Gpg::CurrentKeyChain).to receive(:add).with(GpgHelpers::User2.public_key) - gpg_key = create :gpg_key, key: GpgHelpers::User2.public_key - gpg_key.send(:add_to_keychain) - end - end - - describe '#remove_from_keychain', :gpg do - it 'calls .remove_from_keychain' do - allow(Gitlab::Gpg::CurrentKeyChain).to receive(:remove).with(GpgHelpers::User2.fingerprint) - gpg_key = create :gpg_key, key: GpgHelpers::User2.public_key - gpg_key.send(:remove_from_keychain) + expect(gpg_key.emails_with_verified_status).to match_array [ + ['bette.cartwright@example.com', true], + ['bette.cartwright@example.net', false] + ] end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 60979fd6c06..20bdb7e37da 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1956,24 +1956,4 @@ describe User, models: true do expect(user.allow_password_authentication?).to be_falsey end end - - context 'callbacks' do - context '.synchronize_gpg_keys' do - let(:user) do - create(:user, email: 'tula.torphy@abshire.ca').tap do |user| - user.skip_reconfirmation! - end - end - - it 'does nothing when the name is updated' do - expect(user).not_to receive(:synchronize_gpg_keys) - user.update_attributes!(name: 'Bette') - end - - it 'synchronizes the gpg keys when the email is updated' do - expect(user).to receive(:synchronize_gpg_keys) - user.update_attributes!(email: 'shawnee.ritchie@denesik.com') - end - end - end end