Commit c50725fe authored by Rubén Dávila's avatar Rubén Dávila

Address feedback from last code review

parent c2c35ae7
...@@ -44,6 +44,7 @@ class GpgKey < ActiveRecord::Base ...@@ -44,6 +44,7 @@ class GpgKey < ActiveRecord::Base
def primary_keyid def primary_keyid
super&.upcase super&.upcase
end end
alias_method :keyid, :primary_keyid
def fingerprint def fingerprint
super&.upcase super&.upcase
...@@ -53,6 +54,10 @@ class GpgKey < ActiveRecord::Base ...@@ -53,6 +54,10 @@ class GpgKey < ActiveRecord::Base
super(value&.strip) super(value&.strip)
end end
def keyids
[keyid].concat(subkeys.map(&:keyid))
end
def user_infos def user_infos
@user_infos ||= Gitlab::Gpg.user_infos_from_key(key) @user_infos ||= Gitlab::Gpg.user_infos_from_key(key)
end end
......
...@@ -9,6 +9,9 @@ class GpgKeySubkey < ActiveRecord::Base ...@@ -9,6 +9,9 @@ class GpgKeySubkey < ActiveRecord::Base
validates :gpg_key_id, presence: true validates :gpg_key_id, presence: true
validates :fingerprint, :keyid, presence: true, uniqueness: true validates :fingerprint, :keyid, presence: true, uniqueness: true
delegate :key, :user, :user_infos, :verified?, :verified_user_infos,
:verified_and_belongs_to_email?, to: :gpg_key
def keyid def keyid
super&.upcase super&.upcase
end end
......
...@@ -24,7 +24,7 @@ class GpgSignature < ActiveRecord::Base ...@@ -24,7 +24,7 @@ class GpgSignature < ActiveRecord::Base
def gpg_key=(model) def gpg_key=(model)
case model case model
when GpgKey then super when GpgKey then super
when GpgKeySubkey then write_attribute(:gpg_key_subkey_id, model.id) when GpgKeySubkey then self.gpg_key_subkey = model
end end
end end
......
...@@ -38,13 +38,12 @@ module Gitlab ...@@ -38,13 +38,12 @@ module Gitlab
using_tmp_keychain do using_tmp_keychain do
fingerprints = CurrentKeyChain.fingerprints_from_key(key) fingerprints = CurrentKeyChain.fingerprints_from_key(key)
raw_keys = GPGME::Key.find(:public, fingerprints) raw_keys = GPGME::Key.find(:public, fingerprints)
grouped_subkeys = Hash.new { |h, k| h[k] = [] }
raw_keys.each_with_object(grouped_subkeys).each do |raw_key, subkeys| raw_keys.each_with_object({}) do |raw_key, grouped_subkeys|
primary_subkey_id = raw_key.primary_subkey.keyid primary_subkey_id = raw_key.primary_subkey.keyid
raw_key.subkeys[1..-1].each do |subkey| grouped_subkeys[primary_subkey_id] = raw_key.subkeys[1..-1].map do |s|
subkeys[primary_subkey_id] << { keyid: subkey.keyid, fingerprint: subkey.fingerprint } { keyid: s.keyid, fingerprint: s.fingerprint }
end end
end end
end end
......
...@@ -74,7 +74,7 @@ module Gitlab ...@@ -74,7 +74,7 @@ module Gitlab
commit_sha: @commit.sha, commit_sha: @commit.sha,
project: @commit.project, project: @commit.project,
gpg_key: gpg_key, gpg_key: gpg_key,
gpg_key_primary_keyid: gpg_keyid(gpg_key) || verified_signature.fingerprint, gpg_key_primary_keyid: gpg_key&.keyid || verified_signature.fingerprint,
gpg_key_user_name: user_infos[:name], gpg_key_user_name: user_infos[:name],
gpg_key_user_email: user_infos[:email], gpg_key_user_email: user_infos[:email],
verification_status: verification_status verification_status: verification_status
...@@ -99,12 +99,6 @@ module Gitlab ...@@ -99,12 +99,6 @@ module Gitlab
gpg_key&.verified_user_infos&.first || gpg_key&.user_infos&.first || {} gpg_key&.verified_user_infos&.first || gpg_key&.user_infos&.first || {}
end end
def gpg_keyid(gpg_key)
return nil unless gpg_key
gpg_key.is_a?(GpgKey) ? gpg_key.primary_keyid : gpg_key.keyid
end
def find_gpg_key(keyid) def find_gpg_key(keyid)
GpgKey.find_by(primary_keyid: keyid) || GpgKeySubkey.find_by(keyid: keyid) GpgKey.find_by(primary_keyid: keyid) || GpgKeySubkey.find_by(keyid: keyid)
end end
......
...@@ -3,14 +3,13 @@ module Gitlab ...@@ -3,14 +3,13 @@ module Gitlab
class InvalidGpgSignatureUpdater class InvalidGpgSignatureUpdater
def initialize(gpg_key) def initialize(gpg_key)
@gpg_key = gpg_key @gpg_key = gpg_key
@gpg_keyids = gpg_key.subkeys.map(&:keyid).push(gpg_key.primary_keyid)
end end
def run def run
GpgSignature GpgSignature
.select(:id, :commit_sha, :project_id) .select(:id, :commit_sha, :project_id)
.where('gpg_key_id IS NULL OR verification_status <> ?', GpgSignature.verification_statuses[:verified]) .where('gpg_key_id IS NULL OR verification_status <> ?', GpgSignature.verification_statuses[:verified])
.where(gpg_key_primary_keyid: @gpg_keyids) .where(gpg_key_primary_keyid: @gpg_key.keyids)
.find_each { |sig| sig.gpg_commit.update_signature!(sig) } .find_each { |sig| sig.gpg_commit.update_signature!(sig) }
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment