Move default keys logic into ActorDefaultKeyEndpoint; fix #putjr/pedant-keys-2

9 files changed, 123 insertions, 78 deletions

diff --git a/lib/chef_zero/chef_data/data_normalizer.rb b/lib/chef_zero/chef_data/data_normalizer.rb
index 9cec4f4..da3802d 100644
--- a/lib/chef_zero/chef_data/data_normalizer.rb
+++ b/lib/chef_zero/chef_data/data_normalizer.rb
@@ -18,6 +18,7 @@ module ChefZero
         client['name'] ||= name
         client['clientname'] ||= name
         client['admin'] = !!client['admin'] if client.key?('admin')
+        client['public_key'] = PUBLIC_KEY unless client.key?('public_key')
         client['orgname'] ||= orgname
         client['validator'] ||= false
         client['validator'] = !!client['validator']
@@ -35,6 +36,7 @@ module ChefZero
 
       def self.normalize_user(user, name, identity_keys, osc_compat, method=nil)
         user[identity_keys.first] ||= name
+        user['public_key'] = PUBLIC_KEY unless user.key?('public_key')
         user['admin'] ||= false
         user['admin'] = !!user['admin']
         user['openid'] ||= nil
diff --git a/lib/chef_zero/endpoints/actor_default_key_endpoint.rb b/lib/chef_zero/endpoints/actor_default_key_endpoint.rb
new file mode 100644
index 0000000..f63ffb9
--- /dev/null
+++ b/lib/chef_zero/endpoints/actor_default_key_endpoint.rb
@@ -0,0 +1,70 @@
+require 'chef_zero/rest_base'
+
+module ChefZero
+  module Endpoints
+    # ActorDefaultKeyEndpoint
+    #
+    # This class handles DELETE/GET/PUT requests for client/user default public
+    # keys, i.e. requests with identity key "default". All others are handled
+    # by ActorKeyEndpoint.
+    #
+    # Default public keys are stored with the actor (client or user) instead of
+    # under user/client_keys. Handling those in a separate endpoint offloads
+    # the branching logic onto the router rather than branching in every
+    # endpoint method (`if request.rest_path[-1] == "default" ...`).
+    #
+    # /users/USER/keys/default
+    # /organizations/ORG/clients/CLIENT/keys/default
+    class ActorDefaultKeyEndpoint < RestBase
+      DEFAULT_PUBLIC_KEY_NAME = "default".freeze
+
+      def get(request)
+        # 404 if actor doesn't exist
+        actor_data = get_actor_data(request)
+        json_response(200, default_public_key_from_actor(actor_data))
+      end
+
+      def delete(request)
+        path = actor_path(request)
+        actor_data = get_actor_data(request) # 404 if actor doesn't exist
+
+        default_public_key = delete_actor_default_public_key!(request, path, actor_data)
+        json_response(200, default_public_key)
+      end
+
+      def put(request)
+        # 404 if actor doesn't exist
+        actor_data = get_actor_data(request)
+
+        new_public_key = parse_json(request.body)["public_key"]
+        actor_data["public_key"] = new_public_key
+
+        set_data(request, actor_path(request), to_json(actor_data))
+      end
+
+      private
+
+      def actor_path(request)
+        return request.rest_path[0..3] if request.rest_path[2] == "clients"
+        request.rest_path[0..1]
+      end
+
+      def get_actor_data(request)
+        path = actor_path(request)
+        parse_json(get_data(request, path))
+      end
+
+      def default_public_key_from_actor(actor_data)
+        { "name" => DEFAULT_PUBLIC_KEY_NAME,
+          "public_key" => actor_data["public_key"],
+          "expiration_date" => "infinity" }
+      end
+
+      def delete_actor_default_public_key!(request, path, actor_data)
+        new_actor_data = actor_data.merge("public_key" => nil)
+        set_data(request, path, to_json(new_actor_data))
+        default_public_key_from_actor(actor_data)
+      end
+    end
+  end
+end
diff --git a/lib/chef_zero/endpoints/actor_endpoint.rb b/lib/chef_zero/endpoints/actor_endpoint.rb
index 12fbe11..446c196 100644
--- a/lib/chef_zero/endpoints/actor_endpoint.rb
+++ b/lib/chef_zero/endpoints/actor_endpoint.rb
@@ -87,7 +87,7 @@ module ChefZero
       end
 
       def populate_defaults(request, response_json)
-        response = FFI_Yajl::Parser.parse(response_json, create_additions: false)
+        response = parse_json(response_json)
 
         populated_response =
           if client?(request)
@@ -106,7 +106,7 @@ module ChefZero
             )
           end
 
-        FFI_Yajl::Encoder.encode(populated_response, pretty: true)
+        to_json(populated_response)
       end
 
       private
diff --git a/lib/chef_zero/endpoints/actor_key_endpoint.rb b/lib/chef_zero/endpoints/actor_key_endpoint.rb
index d45570d..f2b65ed 100644
--- a/lib/chef_zero/endpoints/actor_key_endpoint.rb
+++ b/lib/chef_zero/endpoints/actor_key_endpoint.rb
@@ -1,39 +1,27 @@
-require 'ffi_yajl'
 require 'chef_zero/rest_base'
 
 module ChefZero
   module Endpoints
+    # ActorKeyEndpoint
+    #
+    # This class handles DELETE/GET/PUT requests for all client/user keys
+    # **except** default public keys, i.e. requests with identity key
+    # "default". Those are handled by ActorDefaultKeyEndpoint. See that class
+    # for more information.
+    #
     # /users/USER/keys/NAME
     # /organizations/ORG/clients/CLIENT/keys/NAME
     class ActorKeyEndpoint < RestBase
-      DEFAULT_PUBLIC_KEY_NAME = "default".freeze
-
       def get(request)
-        # Try to get the actor so a 404 is returned if it doesn't exist
-        actor_json = get_actor_json(request)
-
-        if request.rest_path[-1] == DEFAULT_PUBLIC_KEY_NAME
-          actor_data = FFI_Yajl::Parser.parse(actor_json, create_additions: false)
-          default_public_key = default_public_key_from_actor(actor_data)
-          return json_response(200, default_public_key)
-        end
-
+        validate_actor!(request)
         key_path = data_path(request)
         already_json_response(200, get_data(request, key_path))
       end
 
       def delete(request)
-        # Try to get the actor so a 404 is returned if it doesn't exist
-        actor_json = get_actor_json(request)
-
-        if request.rest_path[-1] == DEFAULT_PUBLIC_KEY_NAME
-          actor_data = FFI_Yajl::Parser.parse(actor_json, create_additions: false)
-          default_public_key = delete_actor_default_public_key!(request, actor_data)
-          return json_response(200, default_public_key)
-        end
+        validate_actor!(request) # 404 if actor doesn't exist
 
         key_path = data_path(request)
-
         data = get_data(request, key_path)
         delete_data(request, key_path)
 
@@ -41,17 +29,15 @@ module ChefZero
       end
 
       def put(request)
-        # We grab the old data to trigger a 404 if it doesn't exist
-        get_data(request, data_path(request))
-
-        set_data(request, path, request.body)
+        validate_actor!(request) # 404 if actor doesn't exist
+        set_data(request, data_path(request), request.body)
       end
 
       private
 
       # Returns the keys data store path, which is the same as
-      # `request.rest_path` except with "user_keys" instead of "users" or
-      # "client_keys" instead of "clients."
+      # `request.rest_path` except with "client_keys" instead of "clients" or
+      # "user_keys" instead of "users."
       def data_path(request)
         request.rest_path.dup.tap do |path|
           if client?(request)
@@ -62,36 +48,15 @@ module ChefZero
         end
       end
 
-      def default_public_key_from_actor(actor_data)
-        { "name" => DEFAULT_PUBLIC_KEY_NAME,
-          "public_key" => actor_data["public_key"],
-          "expiration_date" => "infinity" }
-      end
-
-      def delete_actor_default_public_key!(request, actor_data)
-        new_actor_data = actor_data.merge("public_key" => nil)
-
-        set_data(
-          request,
-          actor_path(request),
-          FFI_Yajl::Encoder.encode(new_actor_data, pretty: true)
-        )
-
-        default_public_key_from_actor(actor_data)
-      end
-
-      def get_actor_json(request)
-        get_data(request, actor_path(request))
+      # Raises RestErrorResponse (404) if actor doesn't exist
+      def validate_actor!(request)
+        actor_path = request.rest_path[ client?(request) ? 0..3 : 0..1 ]
+        get_data(request, actor_path)
       end
 
       def client?(request)
         request.rest_path[2] == "clients"
       end
-
-      def actor_path(request)
-        return request.rest_path[0..3] if client?(request)
-        request.rest_path[0..1]
-      end
     end
   end
 end
diff --git a/lib/chef_zero/endpoints/actor_keys_endpoint.rb b/lib/chef_zero/endpoints/actor_keys_endpoint.rb
index 8bf3981..ba91a6b 100644
--- a/lib/chef_zero/endpoints/actor_keys_endpoint.rb
+++ b/lib/chef_zero/endpoints/actor_keys_endpoint.rb
@@ -1,4 +1,3 @@
-require 'ffi_yajl'
 require 'chef_zero/rest_base'
 
 module ChefZero
@@ -13,8 +12,7 @@ module ChefZero
         path = data_path(request)
 
         # Get actor or 404 if it doesn't exist
-        actor_path = request.rest_path[ client?(request) ? 0..3 : 0..1 ]
-        actor_json = get_data(request, actor_path)
+        actor_json = get_data(request, actor_path(request))
 
         key_names = list_data_or_else(request, path, [])
         key_names.unshift(DEFAULT_PUBLIC_KEY_NAME) if actor_has_default_public_key?(actor_json)
@@ -27,11 +25,10 @@ module ChefZero
       end
 
       def post(request)
-        request_body = FFI_Yajl::Parser.parse(request.body, create_additions: false)
+        request_body = parse_json(request.body)
 
         # Try loading the client or user so a 404 is returned if it doesn't exist
-        actor_path = request.rest_path[ client?(request) ? 0..3 : 0..1 ]
-        actor_json = get_data(request, actor_path)
+        actor_json = get_data(request, actor_path(request))
 
         generate_keys = request_body["public_key"].nil?
 
@@ -44,7 +41,7 @@ module ChefZero
         key_name = request_body["name"]
 
         if key_name == DEFAULT_PUBLIC_KEY_NAME
-          store_actor_default_public_key!(request, actor_path, actor_json, public_key)
+          store_actor_default_public_key!(request, actor_json, public_key)
         else
           store_actor_public_key!(request, key_name, public_key, request_body["expiration_date"])
         end
@@ -59,7 +56,7 @@ module ChefZero
       private
 
       def store_actor_public_key!(request, name, public_key, expiration_date)
-        data = FFI_Yajl::Encoder.encode(
+        data = to_json(
           "name" => name,
           "public_key" => public_key,
           "expiration_date" => expiration_date
@@ -68,15 +65,16 @@ module ChefZero
         create_data(request, data_path(request), name, data, :create_dir)
       end
 
-      def store_actor_default_public_key!(request, actor_path, actor_json, public_key)
-        actor_data = FFI_Yajl::Parser.parse(actor_json, create_additions: false)
+      def store_actor_default_public_key!(request, actor_json, public_key)
+        actor_data = parse_json(actor_json)
 
         if actor_data["public_key"]
           raise RestErrorResponse.new(409, "Object already exists: #{key_uri(request, DEFAULT_PUBLIC_KEY_NAME)}")
         end
 
         actor_data["public_key"] = public_key
-        set_data(request, actor_path, FFI_Yajl::Encoder.encode(actor_data, pretty: true))
+        set_data(request, actor_path(request), to_json(actor_data))
+
       end
 
       # Returns the keys data store path, which is the same as
@@ -97,7 +95,7 @@ module ChefZero
           if data_path[-1] == DEFAULT_PUBLIC_KEY_NAME
             [ DEFAULT_PUBLIC_KEY_NAME, "infinity" ]
           else
-            FFI_Yajl::Parser.parse(get_data(request, data_path), create_additions: false)
+            parse_json(get_data(request, data_path))
               .values_at("name", "expiration_date")
           end
 
@@ -117,9 +115,13 @@ module ChefZero
         build_uri(request.base_uri, [ *request.rest_path, key_name ])
       end
 
+      def actor_path(request)
+        return request.rest_path[0..3] if client?(request)
+        request.rest_path[0..1]
+      end
+
       def actor_has_default_public_key?(actor_json)
-        actor_data = FFI_Yajl::Parser.parse(actor_json, create_additions: false)
-        !!actor_data["public_key"]
+        !!parse_json(actor_json)["public_key"]
       end
     end
   end
diff --git a/lib/chef_zero/endpoints/rest_object_endpoint.rb b/lib/chef_zero/endpoints/rest_object_endpoint.rb
index 95a3122..7e839c0 100644
--- a/lib/chef_zero/endpoints/rest_object_endpoint.rb
+++ b/lib/chef_zero/endpoints/rest_object_endpoint.rb
@@ -63,7 +63,7 @@ module ChefZero
 
       # Get the value of the (first existing) identity key from the request body or nil
       def identity_key_value(request)
-        request_json = FFI_Yajl::Parser.parse(request.body, :create_additions => false)
+        request_json = parse_json(request.body)
         identity_keys.map { |k| request_json[k] }.compact.first
       end
 
diff --git a/lib/chef_zero/rest_base.rb b/lib/chef_zero/rest_base.rb
index a0c9633..71f6f15 100644
--- a/lib/chef_zero/rest_base.rb
+++ b/lib/chef_zero/rest_base.rb
@@ -267,12 +267,12 @@ module ChefZero
         # Strip off /organizations/chef if we are in single org mode
         if rest_path[0..1] != [ 'organizations', server.options[:single_org] ]
           raise "Unexpected URL #{rest_path[0..1]} passed to build_uri in single org mode"
-        else
-          "#{base_uri}/#{rest_path[2..-1].join('/')}"
         end
-      else
-        "#{base_uri}/#{rest_path.join('/')}"
+
+        return self.class.build_uri(base_uri, rest_path[2..-1])
       end
+
+      self.class.build_uri(base_uri, rest_path)
     end
 
     def self.build_uri(base_uri, rest_path)
diff --git a/lib/chef_zero/server.rb b/lib/chef_zero/server.rb
index da277b6..7d508ca 100644
--- a/lib/chef_zero/server.rb
+++ b/lib/chef_zero/server.rb
@@ -40,8 +40,11 @@ require 'chef_zero/endpoints/rest_list_endpoint'
 require 'chef_zero/endpoints/authenticate_user_endpoint'
 require 'chef_zero/endpoints/acls_endpoint'
 require 'chef_zero/endpoints/acl_endpoint'
-require 'chef_zero/endpoints/actors_endpoint'
 require 'chef_zero/endpoints/actor_endpoint'
+require 'chef_zero/endpoints/actors_endpoint'
+require 'chef_zero/endpoints/actor_key_endpoint'
+require 'chef_zero/endpoints/actor_default_key_endpoint'
+require 'chef_zero/endpoints/actor_keys_endpoint'
 require 'chef_zero/endpoints/cookbooks_endpoint'
 require 'chef_zero/endpoints/cookbook_endpoint'
 require 'chef_zero/endpoints/cookbook_version_endpoint'
@@ -94,8 +97,6 @@ require 'chef_zero/endpoints/system_recovery_endpoint'
 require 'chef_zero/endpoints/user_association_requests_endpoint'
 require 'chef_zero/endpoints/user_association_requests_count_endpoint'
 require 'chef_zero/endpoints/user_association_request_endpoint'
-require 'chef_zero/endpoints/actor_key_endpoint'
-require 'chef_zero/endpoints/actor_keys_endpoint'
 require 'chef_zero/endpoints/user_organizations_endpoint'
 require 'chef_zero/endpoints/file_store_file_endpoint'
 require 'chef_zero/endpoints/not_found_endpoint'
@@ -540,6 +541,7 @@ module ChefZero
           [ "/users/*/association_requests/count", UserAssociationRequestsCountEndpoint.new(self) ],
           [ "/users/*/association_requests/*", UserAssociationRequestEndpoint.new(self) ],
           [ "/users/*/keys", ActorKeysEndpoint.new(self) ],
+          [ "/users/*/keys/default", ActorDefaultKeyEndpoint.new(self) ],
           [ "/users/*/keys/*", ActorKeyEndpoint.new(self) ],
           [ "/users/*/organizations", UserOrganizationsEndpoint.new(self) ],
           [ "/authenticate_user", AuthenticateUserEndpoint.new(self) ],
@@ -569,6 +571,7 @@ module ChefZero
         [ "/organizations/*/clients", ActorsEndpoint.new(self) ],
         [ "/organizations/*/clients/*", ActorEndpoint.new(self) ],
         [ "/organizations/*/clients/*/keys", ActorKeysEndpoint.new(self) ],
+        [ "/organizations/*/clients/*/keys/default", ActorDefaultKeyEndpoint.new(self) ],
         [ "/organizations/*/clients/*/keys/*", ActorKeyEndpoint.new(self) ],
         [ "/organizations/*/controls", ControlsEndpoint.new(self) ],
         [ "/organizations/*/cookbooks", CookbooksEndpoint.new(self) ],
diff --git a/spec/run_oc_pedant.rb b/spec/run_oc_pedant.rb
index c3d4e7f..b8c6898 100644
--- a/spec/run_oc_pedant.rb
+++ b/spec/run_oc_pedant.rb
@@ -140,14 +140,17 @@ begin
   chef_fs_skips << '--skip-cookbook-artifacts'
   chef_fs_skips << '--skip-policies'
 
+  # Multi-keys don't work prior to 12.8
+  unless Gem::Requirement.new(">= 12.8.0").satisfied_by?(Gem::Version.new(Chef::VERSION))
+    chef_fs_skips << '--skip-keys'
+  end
+
   # These things aren't supported by Chef Zero in any mode of operation:
   default_skips = [
     # "the goal is that only authorization, authentication and validation tests
     # are turned off" - @jkeiser
     #
     # ...but we're not there yet
-    '--skip-controls',
-    '--skip-acl',
 
     # Chef Zero does not intend to support validation the way erchef does.
     '--skip-validation',