diff options
| author | Alexis Reigel <mail@koffeinfrei.org> | 2017-07-26 15:47:00 +0200 |
|---|---|---|
| committer | Alexis Reigel <mail@koffeinfrei.org> | 2017-07-27 15:46:04 +0200 |
| commit | f1ccecc9979e3091e7cf54f98508f6bc7c01a7f5 (patch) | |
| tree | 783fc914089c4dc032e1d2b9722598197ce14e4e | |
| parent | 9488b7780edc57193cd1c51888478538ddc94e51 (diff) | |
| download | gitlab-ce-f1ccecc9979e3091e7cf54f98508f6bc7c01a7f5.tar.gz | |
improve gpg key validation
when omitting the end part of the key ('-----END PGP PUBLIC KEY
BLOCK-----') the error message was not about the key anymore, but about
the missing fingerprint and primary_keyid, which was confusing for the
user.
the new validation checks that the end also matches the expected format.
| -rw-r--r-- | app/models/gpg_key.rb | 5 | ||||
| -rw-r--r-- | spec/models/gpg_key_spec.rb | 10 |
2 files changed, 12 insertions, 3 deletions
diff --git a/app/models/gpg_key.rb b/app/models/gpg_key.rb index 535b40472b0..3df60ddc950 100644 --- a/app/models/gpg_key.rb +++ b/app/models/gpg_key.rb @@ -1,5 +1,6 @@ class GpgKey < ActiveRecord::Base KEY_PREFIX = '-----BEGIN PGP PUBLIC KEY BLOCK-----'.freeze + KEY_SUFFIX = '-----END PGP PUBLIC KEY BLOCK-----'.freeze include ShaAttribute @@ -15,8 +16,8 @@ class GpgKey < ActiveRecord::Base presence: true, uniqueness: true, format: { - with: /\A#{KEY_PREFIX}((?!#{KEY_PREFIX}).)+\Z/m, - message: "is invalid. A valid public GPG key begins with '#{KEY_PREFIX}'" + with: /\A#{KEY_PREFIX}((?!#{KEY_PREFIX})(?!#{KEY_SUFFIX}).)+#{KEY_SUFFIX}\Z/m, + message: "is invalid. A valid public GPG key begins with '#{KEY_PREFIX}' and ends with '#{KEY_SUFFIX}'" } validates :fingerprint, diff --git a/spec/models/gpg_key_spec.rb b/spec/models/gpg_key_spec.rb index 1242f0b2e2a..59c074199db 100644 --- a/spec/models/gpg_key_spec.rb +++ b/spec/models/gpg_key_spec.rb @@ -7,10 +7,18 @@ describe GpgKey do describe "validation" do it { is_expected.to validate_presence_of(:user) } + it { is_expected.to validate_presence_of(:key) } it { is_expected.to validate_uniqueness_of(:key) } - it { is_expected.to allow_value("-----BEGIN PGP PUBLIC KEY BLOCK-----\nkey").for(:key) } + + it { is_expected.to allow_value("-----BEGIN PGP PUBLIC KEY BLOCK-----\nkey\n-----END PGP PUBLIC KEY BLOCK-----").for(:key) } + + it { is_expected.not_to allow_value("-----BEGIN PGP PUBLIC KEY BLOCK-----\nkey").for(:key) } it { is_expected.not_to allow_value("-----BEGIN PGP PUBLIC KEY BLOCK-----\nkey\n-----BEGIN PGP PUBLIC KEY BLOCK-----").for(:key) } + it { is_expected.not_to allow_value("-----BEGIN PGP PUBLIC KEY BLOCK----------END PGP PUBLIC KEY BLOCK-----").for(:key) } + it { is_expected.not_to allow_value("-----BEGIN PGP PUBLIC KEY BLOCK-----").for(:key) } + it { is_expected.not_to allow_value("-----END PGP PUBLIC KEY BLOCK-----").for(:key) } + it { is_expected.not_to allow_value("key\n-----END PGP PUBLIC KEY BLOCK-----").for(:key) } it { is_expected.not_to allow_value('BEGIN PGP').for(:key) } end |