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.

improve gpg key validation
f1ccecc9 · Alexis Reigel · 9488b778 · f1ccecc9 · f1ccecc9
Commit f1ccecc9 authored Jul 26, 2017 by Alexis Reigel
Hide whitespace changes
Inline Side-by-side

Showing with 12 additions and 3 deletions

gpg_key.rb app/models/gpg_key.rb +3 -2

gpg_key_spec.rb spec/models/gpg_key_spec.rb +9 -1

No files found.
--- a/app/models/gpg_key.rb
+++ b/app/models/gpg_key.rb
 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,

--- 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