API: implement deleting keys through new keys api

8 files changed, 177 insertions, 8 deletions

diff --git a/app/controllers/api/keys_controller.rb b/app/controllers/api/keys_controller.rb
index d4cb759..7eb76ee 100644
--- a/app/controllers/api/keys_controller.rb
+++ b/app/controllers/api/keys_controller.rb
@@ -25,10 +25,22 @@ class Api::KeysController < ApiController
   def update
     keyring.update type, rev: rev, value: value
     head :no_content
+  rescue Keyring::NotFound => e
+    render status: 404, json: {error: e.message}
   rescue Keyring::Error, ActionController::ParameterMissing => e
     render status: 422, json: {error: e.message}
   end
 
+  def destroy
+    keyring.delete type, rev: rev
+    head :no_content
+  rescue Keyring::NotFound => e
+    render status: 404, json: {error: e.message}
+  rescue Keyring::Error, ActionController::ParameterMissing => e
+    render status: 422, json: {error: e.message}
+  end
+
+
   protected
 
   def require_enabled
diff --git a/app/models/identity.rb b/app/models/identity.rb
index 92f8f7a..b8c2245 100644
--- a/app/models/identity.rb
+++ b/app/models/identity.rb
@@ -136,6 +136,11 @@ class Identity < CouchRest::Model::Base
     write_attribute('keys', keys.merge(type => key.to_s))
   end
 
+  def delete_key(type)
+    raise 'key not found' unless keys[type]
+    write_attribute('keys', keys.except(type))
+  end
+
   def cert_fingerprints
     read_attribute('cert_fingerprints') || Hash.new
   end
diff --git a/app/models/keyring.rb b/app/models/keyring.rb
index 6779d5d..66f7bfd 100644
--- a/app/models/keyring.rb
+++ b/app/models/keyring.rb
@@ -8,6 +8,12 @@ class Keyring
   class Error < RuntimeError
   end
 
+  class NotFound < Error
+    def initialize(type)
+      super "no such key: #{type}"
+    end
+  end
+
   def initialize(storage)
     @storage = storage
   end
@@ -19,19 +25,30 @@ class Keyring
   end
 
   def update(type, rev:, value:)
-    old_rev = key_of_type(type)['rev']
-    raise Error, "wrong revision: #{rev}" unless old_rev == rev
+    check_rev type, rev
     storage.set_key type, {type: type, value: value, rev: new_rev}.to_json
     storage.save
   end
 
+  def delete(type, rev:)
+    check_rev type, rev
+    storage.delete_key type
+    storage.save
+  end
+
   def key_of_type(type)
-    JSON.parse(storage.keys[type])
+    JSON.parse(storage.keys[type]) if storage.keys[type]
   end
 
   protected
   attr_reader :storage
 
+  def check_rev(type, rev)
+    old = key_of_type(type)
+    raise NotFound, type unless old
+    raise Error, "wrong revision: #{rev}" unless old['rev'] == rev
+  end
+
   def new_rev
     SecureRandom.urlsafe_base64(8)
   end
diff --git a/config/routes.rb b/config/routes.rb
index ba8f168..55d03fa 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -41,7 +41,7 @@ LeapWeb::Application.routes.draw do
     resource :service, :only => [:show]
     resources :configs, :only => [:index, :show]
     resources :identities, :only => [:show]
-    resources :keys, :only=> [:index, :show, :create, :update]
+    resources :keys, :except=> [:edit, :new]
   end
 
   scope "(:locale)", :locale => CommonLanguages.match_available do
diff --git a/features/2/keys.feature b/features/2/keys.feature
index cc87da0..83e70e7 100644
--- a/features/2/keys.feature
+++ b/features/2/keys.feature
@@ -114,6 +114,19 @@ Feature: Handle current users collection of keys
       }
     """
 
+  Scenario: Publishing an empty key fails
+    When I send a POST request to "2/keys" with the following:
+    """
+      {}
+    """
+    Then the response status should be "422"
+    And the response should be:
+    """
+      {
+      "error": "param is missing or the value is empty: type"
+      }
+    """
+
   Scenario: Updating an existing key
     Given I have published a "openpgp" key
     When I send a PATCH request to "2/keys/openpgp" with the following:
@@ -127,6 +140,24 @@ Feature: Handle current users collection of keys
     Then the response status should be "204"
     And I should have published a "openpgp" key with value "QWER"
 
+  Scenario: Updating a missing key raises
+    When I send a PATCH request to "2/keys/openpgp" with the following:
+    """
+      {
+      "type": "openpgp",
+      "value": "QWER",
+      "rev": "DUMMY_REV"
+      }
+    """
+    Then the response status should be "404"
+    And the response should be:
+    """
+      {
+      "error": "no such key: openpgp"
+      }
+    """
+    And I should not have published a "openpgp" key
+
   Scenario: Updating an existing key require revision
     Given I have published a "openpgp" key
     When I send a PATCH request to "2/keys/openpgp" with the following:
@@ -162,15 +193,65 @@ Feature: Handle current users collection of keys
       }
     """
 
-  Scenario: Publishing an empty key fails
-    When I send a POST request to "2/keys" with the following:
+  Scenario: Deleting an existing key
+    Given I have published a "openpgp" key
+    When I send a DELETE request to "2/keys/openpgp" with the following:
     """
-      {}
+      {
+      "type": "openpgp",
+      "rev": "DUMMY_REV"
+      }
+    """
+    Then the response status should be "204"
+    And I should not have published a "openpgp" key
+
+  Scenario: Deleting a missing key raises
+    When I send a DELETE request to "2/keys/openpgp" with the following:
+    """
+      {
+      "type": "openpgp",
+      "rev": "DUMMY_REV"
+      }
+    """
+    Then the response status should be "404"
+    And the response should be:
+    """
+      {
+      "error": "no such key: openpgp"
+      }
+    """
+
+  Scenario: Deleting an existing key require revision
+    Given I have published a "openpgp" key
+    When I send a DELETE request to "2/keys/openpgp" with the following:
+    """
+      {
+      "type": "openpgp"
+      }
     """
     Then the response status should be "422"
     And the response should be:
     """
       {
-      "error": "param is missing or the value is empty: type"
+      "error": "param is missing or the value is empty: rev"
+      }
+    """
+    And I should have published a "openpgp" key
+
+  Scenario: Deleting an existing key require right revision
+    Given I have published a "openpgp" key
+    When I send a DELETE request to "2/keys/openpgp" with the following:
+    """
+      {
+      "type": "openpgp",
+      "rev": "WRONG_REV"
       }
     """
+    Then the response status should be "422"
+    And the response should be:
+    """
+      {
+      "error": "wrong revision: WRONG_REV"
+      }
+    """
+    And I should have published a "openpgp" key
diff --git a/features/step_definitions/key_steps.rb b/features/step_definitions/key_steps.rb
index 70a13bd..3d5e015 100644
--- a/features/step_definitions/key_steps.rb
+++ b/features/step_definitions/key_steps.rb
@@ -18,3 +18,9 @@ Then /^I should have published an? "([^"]*)" key(?: with value "([^"]*)")?$/ do
   assert_includes keys.keys, type
   assert_equal value, JSON.parse(keys[type])['value'] if value
 end
+
+Then /^I should not have published an? "([^"]*)" key$/ do |type|
+  identity = Identity.for(@user)
+  keys = identity.keys
+  refute_includes keys.keys, type
+end
diff --git a/test/unit/identity_test.rb b/test/unit/identity_test.rb
index 6836487..43f644a 100644
--- a/test/unit/identity_test.rb
+++ b/test/unit/identity_test.rb
@@ -80,6 +80,14 @@ class IdentityTest < ActiveSupport::TestCase
     assert_equal pgp_key_string, @id.keys[:pgp]
   end
 
+  test "deleting pgp key" do
+    @id = Identity.for(@user)
+    @id.set_key(:pgp, pgp_key_string)
+    @id.delete_key(:pgp)
+    assert_nil @id.keys[:pgp]
+    assert_equal Hash.new, @id.keys
+  end
+
   test "querying pgp key via couch" do
     @id = Identity.for(@user)
     @id.set_key(:pgp, pgp_key_string)
diff --git a/test/unit/keyring_test.rb b/test/unit/keyring_test.rb
index 059b8dd..c7df63e 100644
--- a/test/unit/keyring_test.rb
+++ b/test/unit/keyring_test.rb
@@ -21,6 +21,13 @@ class KeyringTest < ActiveSupport::TestCase
     assert_equal 'new value', keyring.key_of_type('type')['value']
   end
 
+  test 'raise on updating missing key' do
+    assert_raises Keyring::NotFound do
+      keyring.update 'type', rev: nil ,value: 'new value'
+    end
+    assert_nil keyring.key_of_type('type')
+  end
+
   test 'raise on updating without rev' do
     keyring.create 'type', 'value'
     assert_raises Keyring::Error do
@@ -37,6 +44,35 @@ class KeyringTest < ActiveSupport::TestCase
     assert_equal 'value', keyring.key_of_type('type')['value']
   end
 
+  test 'delete key' do
+    keyring.create 'type', 'value'
+    initial_rev = keyring.key_of_type('type')['rev']
+    keyring.delete 'type', rev: initial_rev
+    assert_nil keyring.key_of_type('type')
+  end
+
+  test 'raise on deleting missing key' do
+    assert_raises Keyring::NotFound do
+      keyring.delete 'type', rev: nil
+    end
+  end
+
+  test 'raise on deleting without rev' do
+    keyring.create 'type', 'value'
+    assert_raises Keyring::Error do
+      keyring.delete 'type', rev: nil
+    end
+    assert_equal 'value', keyring.key_of_type('type')['value']
+  end
+
+  test 'raise on deleting with wrong rev' do
+    keyring.create 'type', 'value'
+    assert_raises Keyring::Error do
+      keyring.delete 'type', rev: 'wrong rev'
+    end
+    assert_equal 'value', keyring.key_of_type('type')['value']
+  end
+
 
   protected
 
@@ -54,6 +90,10 @@ class KeyringTest < ActiveSupport::TestCase
         self
       end
 
+      def dummy.delete_key(type)
+        self.delete(type)
+      end
+
       def dummy.save; end
     end
   end