diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ad8573116..5006130004 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index db51f7fcc2..55866e1f19 100644 --- a/app/controllers/application_controller.rb +++ b/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 diff --git a/app/models/exceptions/invalid_policy_object.rb b/app/models/exceptions/invalid_policy_object.rb new file mode 100644 index 0000000000..b5b9b2280e --- /dev/null +++ b/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 diff --git a/app/models/loader/types.rb b/app/models/loader/types.rb index 846ee8d69a..3660abb1cc 100644 --- a/app/models/loader/types.rb +++ b/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 diff --git a/cucumber/policy/features/host_factory.feature b/cucumber/policy/features/host_factory.feature index 747ceab0e5..03fc9eba50 100644 --- a/cucumber/policy/features/host_factory.feature +++ b/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"