Merge pull request #737 from cyberark/fix_policy_load_exception

Fix policy load exception on host factory with no layers

CHANGELOG.md

@@ -6,6 +6,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ## [Unreleased]
 
+### Fixed
+- Previously, loading a policy with a host factory that doesn't include
+  any layers would cause a `nil` runtime exception. Now this case is checked
+  specifically and raises a policy load error with a description of the problem.
+
 ## [1.2.0] - 2018-09-18
 ### Added
 - Added support for issuing certificates to Hosts using CAs configured as

app/controllers/application_controller.rb

@@ -27,6 +27,7 @@ class RecordExists < Exceptions::RecordExists
   rescue_from Sequel::NoMatchingRow, with: :no_matching_row
   rescue_from Sequel::ForeignKeyConstraintViolation, with: :foreign_key_constraint_violation
   rescue_from Conjur::PolicyParser::Invalid, with: :policy_invalid
+  rescue_from Exceptions::InvalidPolicyObject, with: :policy_invalid
   rescue_from ArgumentError, with: :argument_error
 
   around_action :run_with_transaction
@@ -120,17 +121,23 @@ def validation_failed e
 
   def policy_invalid e
     logger.debug "#{e}\n#{e.backtrace.join "\n"}"
-    render json: {
-      error: {
+
+    error = {
+      code: "policy_invalid",
+      message: e.message
+    }
+
+    if e.instance_of?(Conjur::PolicyParser::Invalid)
+      error[:innererror] = {
         code: "policy_invalid",
-        message: e.message,
-        innererror: {
-          code: "policy_invalid",
-          filename: e.filename,
-          line: e.mark.line,
-          column: e.mark.column
-        }
+        filename: e.filename,
+        line: e.mark.line,
+        column: e.mark.column
       }
+    end
+
+    render json: {
+      error: error
     }, status: :unprocessable_entity
   end
 

app/models/exceptions/invalid_policy_object.rb

@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+
+module Exceptions
+  class InvalidPolicyObject < RuntimeError
+    attr_reader :id, :account, :kind, :identifier
+
+    def initialize id, message: nil
+      super message || self.class.build_message(id)
+
+      @id = id
+      @account, @kind, @identifier = self.class.parse_id(id)
+    end
+
+    class << self
+      def parse_id id
+        id.split(':', 3)
+      end
+
+      def build_message id
+        account, kind, id = parse_id(id)
+        kind ||= 'unknown kind'
+        "The policy definition for #{kind.capitalize} '#{id}' is not valid"
+      end
+    end
+  end
+end

app/models/loader/types.rb

@@ -148,10 +148,23 @@ def create!
       protected
 
       def layer_roleids
-        self.layers.map do |layer|
+        verify_layers_exist!
+
+        Array(self.layers).map do |layer|
           find_roleid layer.roleid
         end
       end
+
+      def verify_layers_exist!
+        if self.layers.nil?
+          message = "Host factory '#{identifier}' does not include any layers"
+          raise Exceptions::InvalidPolicyObject.new(self.id, message: message)
+        end
+      end
+
+      def identifier
+        self.roleid.split(':', 3)[2]
+      end
     end
 
     class Group < Record

cucumber/policy/features/host_factory.feature

@@ -67,3 +67,21 @@ Feature: Host Factories can be managed through policies.
         "cucumber:layer:myapp"
       ]
       """
+
+    Scenario: Load a Host Factory with no layers
+      Given a policy:
+        """ 
+        - !policy
+          id: another-app
+          body:
+            - !policy
+              id: hosts
+              annotations:
+                description: Layer & Host Factory for machines that can read secrets
+              body:
+                - !layer
+                - !host-factory
+        """
+      Then there is an error
+      And the error code is "policy_invalid"
+      And the error message is "Host factory 'another-app/hosts' does not include any layers"