From b27549df97151f773c6fbfac4c9dc3aa491a8b5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Thu, 28 Sep 2017 13:26:16 -0500 Subject: [PATCH] Add some basic specs and refactor model and validations --- app/models/gpg_key_subkey.rb | 11 +++++ .../20170927161718_create_gpg_key_subkeys.rb | 14 ++++++- ...add_gpg_key_subkey_id_to_gpg_signatures.rb | 10 ----- db/schema.rb | 2 + spec/factories/gpg_key_subkeys.rb | 10 +++++ spec/factories/gpg_keys.rb | 4 ++ spec/models/gpg_key_spec.rb | 9 +++++ spec/models/gpg_key_subkey_spec.rb | 15 +++++++ spec/services/gpg_keys/create_service_spec.rb | 10 +++++ spec/support/gpg_helpers.rb | 40 +++++++++++++++++++ 10 files changed, 114 insertions(+), 11 deletions(-) delete mode 100644 db/migrate/20170927232658_add_gpg_key_subkey_id_to_gpg_signatures.rb create mode 100644 spec/factories/gpg_key_subkeys.rb create mode 100644 spec/models/gpg_key_subkey_spec.rb diff --git a/app/models/gpg_key_subkey.rb b/app/models/gpg_key_subkey.rb index 8222e5606aa..b4f146e5647 100644 --- a/app/models/gpg_key_subkey.rb +++ b/app/models/gpg_key_subkey.rb @@ -6,6 +6,17 @@ class GpgKeySubkey < ActiveRecord::Base belongs_to :gpg_key + validates :gpg_key_id, presence: true + validates :fingerprint, :keyid, presence: true, uniqueness: true + + def keyid + super&.upcase + end + + def fingerprint + super&.upcase + end + def method_missing(m, *a, &b) return super unless gpg_key.respond_to?(m) diff --git a/db/migrate/20170927161718_create_gpg_key_subkeys.rb b/db/migrate/20170927161718_create_gpg_key_subkeys.rb index 9f69e299874..ffe06ce1231 100644 --- a/db/migrate/20170927161718_create_gpg_key_subkeys.rb +++ b/db/migrate/20170927161718_create_gpg_key_subkeys.rb @@ -1,11 +1,23 @@ class CreateGpgKeySubkeys < ActiveRecord::Migration DOWNTIME = false - def change + def up create_table :gpg_key_subkeys do |t| t.binary :keyid t.binary :fingerprint + t.references :gpg_key, null: false, index: true, foreign_key: { on_delete: :cascade } + + t.index :keyid, unique: true, length: Gitlab::Database.mysql? ? 20 : nil + t.index :fingerprint, unique: true, length: Gitlab::Database.mysql? ? 20 : nil end + + add_reference :gpg_signatures, :gpg_key_subkey, index: true, foreign_key: { on_delete: :nullify } + end + + def down + remove_reference(:gpg_signatures, :gpg_key_subkey, index: true, foreign_key: true) + + drop_table :gpg_key_subkeys end end diff --git a/db/migrate/20170927232658_add_gpg_key_subkey_id_to_gpg_signatures.rb b/db/migrate/20170927232658_add_gpg_key_subkey_id_to_gpg_signatures.rb deleted file mode 100644 index 74b43d732d5..00000000000 --- a/db/migrate/20170927232658_add_gpg_key_subkey_id_to_gpg_signatures.rb +++ /dev/null @@ -1,10 +0,0 @@ -# See http://doc.gitlab.com/ce/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - -class AddGpgKeySubkeyIdToGpgSignatures < ActiveRecord::Migration - DOWNTIME = false - - def change - add_reference(:gpg_signatures, :gpg_key_subkey, index: true, foreign_key: { on_delete: :nullify }) - end -end diff --git a/db/schema.rb b/db/schema.rb index 0beebc16cbb..b9de70b742a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -585,7 +585,9 @@ ActiveRecord::Schema.define(version: 20171004121444) do t.integer "gpg_key_id", null: false end + add_index "gpg_key_subkeys", ["fingerprint"], name: "index_gpg_key_subkeys_on_fingerprint", unique: true, using: :btree add_index "gpg_key_subkeys", ["gpg_key_id"], name: "index_gpg_key_subkeys_on_gpg_key_id", using: :btree + add_index "gpg_key_subkeys", ["keyid"], name: "index_gpg_key_subkeys_on_keyid", unique: true, using: :btree create_table "gpg_keys", force: :cascade do |t| t.datetime_with_timezone "created_at", null: false diff --git a/spec/factories/gpg_key_subkeys.rb b/spec/factories/gpg_key_subkeys.rb new file mode 100644 index 00000000000..d896b4e217e --- /dev/null +++ b/spec/factories/gpg_key_subkeys.rb @@ -0,0 +1,10 @@ +require_relative '../support/gpg_helpers' + +FactoryGirl.define do + factory :gpg_key_subkey do + gpg_key + + keyid { gpg_key.subkeys.last.keyid } + fingerprint { gpg_key.subkeys.last.fingerprint } + end +end diff --git a/spec/factories/gpg_keys.rb b/spec/factories/gpg_keys.rb index 1258dce8940..93218e5763e 100644 --- a/spec/factories/gpg_keys.rb +++ b/spec/factories/gpg_keys.rb @@ -4,5 +4,9 @@ FactoryGirl.define do factory :gpg_key do key GpgHelpers::User1.public_key user + + factory :gpg_key_with_subkeys do + key GpgHelpers::User1.public_key_with_extra_signing_key + end end end diff --git a/spec/models/gpg_key_spec.rb b/spec/models/gpg_key_spec.rb index 743f2cfcab5..69d6090e034 100644 --- a/spec/models/gpg_key_spec.rb +++ b/spec/models/gpg_key_spec.rb @@ -3,6 +3,7 @@ require 'rails_helper' describe GpgKey do describe "associations" do it { is_expected.to belong_to(:user) } + it { is_expected.to have_many(:subkeys) } end describe "validation" do @@ -38,6 +39,14 @@ describe GpgKey do expect(gpg_key.primary_keyid).to eq GpgHelpers::User1.primary_keyid end end + + describe 'generate_subkeys' do + it 'extracts the subkeys from the gpg key' do + gpg_key = create(:gpg_key, key: GpgHelpers::User1.public_key_with_extra_signing_key) + + expect(gpg_key.subkeys.count).to eq(2) + end + end end describe '#key=' do diff --git a/spec/models/gpg_key_subkey_spec.rb b/spec/models/gpg_key_subkey_spec.rb new file mode 100644 index 00000000000..3c86837f47f --- /dev/null +++ b/spec/models/gpg_key_subkey_spec.rb @@ -0,0 +1,15 @@ +require 'rails_helper' + +describe GpgKeySubkey do + subject { build(:gpg_key_subkey) } + + describe 'associations' do + it { is_expected.to belong_to(:gpg_key) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:gpg_key_id) } + it { is_expected.to validate_presence_of(:fingerprint) } + it { is_expected.to validate_presence_of(:keyid) } + end +end diff --git a/spec/services/gpg_keys/create_service_spec.rb b/spec/services/gpg_keys/create_service_spec.rb index 20382a3a618..1cd2625531e 100644 --- a/spec/services/gpg_keys/create_service_spec.rb +++ b/spec/services/gpg_keys/create_service_spec.rb @@ -18,4 +18,14 @@ describe GpgKeys::CreateService do it 'creates a gpg key' do expect { subject.execute }.to change { user.gpg_keys.where(params).count }.by(1) end + + context 'when the public key contains subkeys' do + let(:params) { attributes_for(:gpg_key_with_subkeys) } + + it 'generates the gpg subkeys' do + gpg_key = subject.execute + + expect(gpg_key.subkeys.count).to eq(2) + end + end end diff --git a/spec/support/gpg_helpers.rb b/spec/support/gpg_helpers.rb index 65b38626a51..a8cfd72954e 100644 --- a/spec/support/gpg_helpers.rb +++ b/spec/support/gpg_helpers.rb @@ -92,6 +92,46 @@ module GpgHelpers KEY end + def public_key_with_extra_signing_key + <<~KEY.strip + -----BEGIN PGP PUBLIC KEY BLOCK----- + Version: GnuPG v1 + + mI0EWK7VJwEEANSFayuVYenl7sBKUjmIxwDRc3jd+K+FWUZgknLgiLcevaLh/mxV + 98dLxDKGDHHNKc/B7Y4qdlZYv1wfNQVuIbd8dqUQFOOkH7ukbgcGBTxH+2IM67y+ + QBH618luS5Gz1d4bd0YoFf/xZGEh9G5xicz7TiXYzLKjnMjHu2EmbFePABEBAAG0 + LU5hbm5pZSBCZXJuaGFyZCA8bmFubmllLmJlcm5oYXJkQGV4YW1wbGUuY29tPoi4 + BBMBAgAiBQJYrtUnAhsDBgsJCAcDAgYVCAIJCgsEFgIDAQIeAQIXgAAKCRDM++Gf + AKyLHaeSA/99oUWpb02PlfkALcx5RncboMHkgczYEU9wOFIgvXIReswThCMOvPZa + piui+ItyJfV3ijJfO8IvbbFcvU7jjGA073Bb7tbzAEOQLA16mWgBLQlGaRWbHDW4 + uwFxvkJKA0GzEsadEXeniESaZPc4rOXKPO3+/MSQWS2bmvvGsBTEuriNBFiu1ScB + BADIXkITf+kKCkD+n8tMsdTLInefu8KrJ8p7YRYCCabEXnWRsDb5zxUAG2VXCVUh + Yl6QXQybkNiBaduS+uxilz7gtYZUMFJvQ09+fV7D2N9B7u/1bGdIYz+cDFJnEJit + LY4w/nju2Sno5CL5Ead8sZuslKetSXPYHR/kbW462EOw5wARAQABiJ8EGAECAAkF + Aliu1ScCGwwACgkQzPvhnwCsix2WRQQAtOXpBS60myrBUXhlcqabDQgSTw+Spbgb + 61hEMckyzpk7SfMNLz0EbYMvj9SU6znBG8RGeUljPTVMxPGr9yIpoFMSPKAUi/0K + AgRmH3tVpxlMipwXjST1Jukk2eHckt/3jGw3E1ElMSFtULe6u5p4gu578hHukEwT + IKzj0ZyC7DK5AQ0EWcx23AEIANwpAq85bT10JCBuNhOMyF2jKVt5wHbI9wBtjWYG + fgJFBkRvm6IsbmR0Y5DSBvF/of0UX1iGMfx6mvCDJkb1okquhCUef6MONWRpzXYE + CIZDm1TXu6yv0D35tkLfPo+/sY9UHHp1zGRcPAU46e8ztRwoD+zEJwy7lobLHGOL + 9OdWtCGjsutLOTqKRK4jsifr8n3rePU09rejhDkRONNs7ufn9GRcWMN7RWiFDtpU + gNe84AJ38qaXPU8GHNTrDtDtRRPmn68ezMmE1qTNsxQxD4Isexe5Wsfc4+ElaP9s + zaHgij7npX1HS9RpmhnOa2h1ESroM9cqDh3IJVhf+eP6/uMAEQEAAYkBxAQYAQIA + DwUCWcx23AIbAgUJAeEzgAEpCRDM++GfAKyLHcBdIAQZAQIABgUCWcx23AAKCRDk + garE0uOuES7DCAC2Kgl6zO+NqIBIS6McgcEN0sGyvOvZ8Ps4hBiMwCyDAnsIRAUi + v4KZMtQMAyl9njJ3YjPWBsdieuTz45O06DDnrzJpZO5rUGJjAcEue4zvRRWIyu3H + qHC8MsvkslsNCygJHoWlknm+HucroskTNtxHQ+FdKZ6Tey+twl1u+PhV8PQVyFkl + 4G1chO90EP4dvYrye26CC+ik2JkvC7Vy5M+U0PJikme8pFMjcdNks25BnAKcdqKU + AU8RTkSjoYvb8qSmZyldJjYjQRkTPRX1ZdaOID1EdiWl+s5cn0Oypo3z7BChcEMx + IWB/gmXQJQXVr5qNQnJObyMO/RczXYi9qNnyGMED/2EJJERLR0nefjHQalwDKQVP + s5lX1OKAcf2CrV6ZarckqaQgtqjZbuV2C2mrOHUs5uojlXaopj5gA4yJSGDcYhj1 + Rg9jdHWBtkHBj3eL32ZqrHDs3ap8ErZMmPE8A+mn9TTnQS+FY2QF7vBjJKM3qPT7 + DMVGWrg4m1NF8N6yMPMP + =RB1y + -----END PGP PUBLIC KEY BLOCK----- + KEY + end + def primary_keyid fingerprint[-16..-1] end