simple validation for pgp key format

4 files changed, 64 insertions, 19 deletions

diff --git a/users/app/models/pgp_key.rb b/users/app/models/pgp_key.rb
index fddec1e..66f8660 100644
--- a/users/app/models/pgp_key.rb
+++ b/users/app/models/pgp_key.rb
@@ -1,25 +1,48 @@
 class PgpKey
   include ActiveModel::Validations
 
+  KEYBLOCK_IDENTIFIERS = [
+    '-----BEGIN PGP PUBLIC KEY BLOCK-----',
+    '-----END PGP PUBLIC KEY BLOCK-----',
+  ]
+
   # mostly for testing.
-  attr_accessor :key_block
+  attr_accessor :keyblock
+
+  validate :validate_keyblock_format
 
-  def initialize(key_block = nil)
-    @key_block = key_block
+  def initialize(keyblock = nil)
+    @keyblock = keyblock
   end
 
   def to_s
-    @key_block
+    @keyblock
   end
 
   def present?
-    @key_block.present?
+    @keyblock.present?
   end
 
-  # let's allow comparison with plain key_block strings.
+  # allow comparison with plain keyblock strings.
   def ==(other)
     self.equal?(other) or
-    self.to_s == other
+    # relax the comparison on line ends.
+    self.to_s.tr_s("\n\r", '') == other.tr_s("\r\n", '')
+  end
+
+  protected
+
+  def validate_keyblock_format
+    if keyblock_identifier_missing?
+      errors.add :public_key_block,
+        "does not look like an armored pgp public key block"
+    end
+  end
+
+  def keyblock_identifier_missing?
+    KEYBLOCK_IDENTIFIERS.find do |identify|
+      !@keyblock.include?(identify)
+    end
   end
 
 end
diff --git a/users/test/factories.rb b/users/test/factories.rb
index f5fb77d..ae00d43 100644
--- a/users/test/factories.rb
+++ b/users/test/factories.rb
@@ -23,4 +23,12 @@ FactoryGirl.define do
     user
   end
 
+  factory :pgp_key do
+    keyblock <<-EOPGP
+-----BEGIN PGP PUBLIC KEY BLOCK-----
++Dummy+PGP+KEY+++Dummy+PGP+KEY+++Dummy+PGP+KEY+++Dummy+PGP+KEY+
+#{SecureRandom.base64(4032)}
+-----END PGP PUBLIC KEY BLOCK-----
+    EOPGP
+  end
 end
diff --git a/users/test/integration/api/account_flow_test.rb b/users/test/integration/api/account_flow_test.rb
index 90f2a97..9aee38b 100644
--- a/users/test/integration/api/account_flow_test.rb
+++ b/users/test/integration/api/account_flow_test.rb
@@ -96,27 +96,41 @@ class AccountFlowTest < RackTest
     assert server_auth["M2"]
   end
 
-  test "update user" do
+  test "changing login" do
     server_auth = @srp.authenticate(self)
-    test_public_key = 'asdlfkjslfdkjasd'
     original_login = @user.login
     new_login = 'zaph'
     User.find_by_login(new_login).try(:destroy)
     Identity.by_address.key(new_login + '@' + APP_CONFIG[:domain]).each do |identity|
       identity.destroy
     end
-    put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', :user => {:public_key => test_public_key, :login => new_login}, :format => :json
+    put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', :user => {:login => new_login}, :format => :json
     assert last_response.successful?
-    assert_equal test_public_key, Identity.for(@user).keys[:pgp]
     # does not change login if no password_verifier is present
     assert_equal original_login, @user.login
-    # eventually probably want to remove most of this into a non-integration functional test
-    # should not overwrite public key:
-    put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', :user => {:blee => :blah}, :format => :json
-    assert_equal test_public_key, Identity.for(@user).keys[:pgp]
-    # should not empty public key:
+  end
+
+  test "upload pgp key" do
+    server_auth = @srp.authenticate(self)
+    key = FactoryGirl.build :pgp_key
+    put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', :user => {:public_key => key}, :format => :json
+    assert_equal key, Identity.for(@user).keys[:pgp]
+  end
+
+  # eventually probably want to remove most of this into a non-integration
+  # functional test
+  test "prevent uploading invalid key" do
+    server_auth = @srp.authenticate(self)
+    put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', :user => {:public_key => :blah}, :format => :json
+    assert_nil Identity.for(@user).keys[:pgp]
+  end
+
+  test "prevent emptying public key" do
+    server_auth = @srp.authenticate(self)
+    key = FactoryGirl.build :pgp_key
+    put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', :user => {:public_key => key}, :format => :json
     put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', :user => {:public_key => ""}, :format => :json
-    assert_equal test_public_key, Identity.for(@user).keys[:pgp]
+    assert_equal key, Identity.for(@user).keys[:pgp]
   end
 
 end
diff --git a/users/test/integration/browser/account_test.rb b/users/test/integration/browser/account_test.rb
index b349489..3d281ae 100644
--- a/users/test/integration/browser/account_test.rb
+++ b/users/test/integration/browser/account_test.rb
@@ -66,7 +66,7 @@ class AccountTest < BrowserIntegrationTest
   end
 
   test "change pgp key" do
-    pgp_key = "My PGP Key Stub"
+    pgp_key = FactoryGirl.build :pgp_key
     username, password = submit_signup
     click_on "Account Settings"
     within('#update_pgp_key') do
@@ -76,7 +76,7 @@ class AccountTest < BrowserIntegrationTest
     page.assert_selector 'input[value="Saving..."]'
     # at some point we're done:
     page.assert_no_selector 'input[value="Saving..."]'
-    assert page.has_field? 'Public key', with: pgp_key
+    assert page.has_field? 'Public key', with: pgp_key.to_s
     user = User.find_by_login(username)
     assert_equal pgp_key, user.public_key
     user.account.destroy