diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b3699d..2e5b2b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ ## [Unreleased] -No notable changes. +### Added +- Support for schema validation in `Cerbos::Client#plan_resources` ([#32](https://github.com/cerbos/cerbos-sdk-ruby/pull/32)) + + Requires Cerbos 0.19+. + `Cerbos::Output::PlanResources#validation_errors` will always return an empty array if the client is connected to an earlier version of Cerbos. + + As a result, `Cerbos::Output::CheckResources::Result::ValidationError` has moved to `Cerbos::Output::ValidationError`. + Attempting to access the class via the old namespace will print a deprecation warning and return the new class. ## [0.5.0] - 2022-06-09 ### Added diff --git a/lib/cerbos/client.rb b/lib/cerbos/client.rb index d058555..8a6a65c 100644 --- a/lib/cerbos/client.rb +++ b/lib/cerbos/client.rb @@ -187,7 +187,9 @@ def plan_resources(principal:, resource:, action:, aux_data: nil, include_metada response = perform_request(@cerbos_service, :plan_resources, request) - Output::PlanResources.from_protobuf(response) + Output::PlanResources.from_protobuf(response).tap do |output| + handle_validation_errors output + end end end @@ -221,7 +223,7 @@ def handle_errors def handle_validation_errors(output) return if @on_validation_error == :return - validation_errors = output.results.flat_map(&:validation_errors) + validation_errors = output.validation_errors return if validation_errors.empty? raise Error::ValidationFailed.new(validation_errors) if @on_validation_error == :raise diff --git a/lib/cerbos/output.rb b/lib/cerbos/output.rb index bf24127..1c2cc29 100644 --- a/lib/cerbos/output.rb +++ b/lib/cerbos/output.rb @@ -32,6 +32,7 @@ def hash end end +require_relative "output/validation_error" require_relative "output/check_resources" require_relative "output/plan_resources" require_relative "output/server_info" diff --git a/lib/cerbos/output/check_resources.rb b/lib/cerbos/output/check_resources.rb index e0f4bfc..410749e 100644 --- a/lib/cerbos/output/check_resources.rb +++ b/lib/cerbos/output/check_resources.rb @@ -55,6 +55,13 @@ def find_result(resource) results.find { |result| matching_resource?(search, result.resource) } end + # List unique schema validation errors for the principal or resource attributes. + # + # @return [Array] + def validation_errors + results.flat_map(&:validation_errors).uniq + end + private def matching_resource?(search, candidate) @@ -88,11 +95,21 @@ def matching_resource?(search, candidate) # @return [Metadata] # @return [nil] if `include_metadata` was `false`. + # @private + def self.const_missing(const) + if const == :ValidationError + warn "#{name}::ValidationError is deprecated; use #{ValidationError.name} instead (called from #{caller(1..1).first})" + return ValidationError + end + + super + end + def self.from_protobuf(entry) new( resource: CheckResources::Result::Resource.from_protobuf(entry.resource), actions: entry.actions.to_h, - validation_errors: (entry.validation_errors || []).map { |validation_error| CheckResources::Result::ValidationError.from_protobuf(validation_error) }, + validation_errors: (entry.validation_errors || []).map { |validation_error| ValidationError.from_protobuf(validation_error) }, metadata: CheckResources::Result::Metadata.from_protobuf(entry.meta) ) end @@ -154,46 +171,6 @@ def self.from_protobuf(resource) end end - # An error that occurred while validating the principal or resource attributes against a schema. - CheckResources::Result::ValidationError = Output.new_class(:path, :message, :source) do - # @!attribute [r] path - # The path to the attribute that failed validation. - # - # @return [String] - - # @!attribute [r] message - # The error message. - # - # @return [String] - - # @!attribute [r] source - # The source of the invalid attributes. - # - # @return [:SOURCE_PRINCIPAL, :SOURCE_RESOURCE] - - def self.from_protobuf(validation_error) - new( - path: validation_error.path, - message: validation_error.message, - source: validation_error.source - ) - end - - # Check if the principal's attributes failed schema validation. - # - # @return [Boolean] - def from_principal? - source == :SOURCE_PRINCIPAL - end - - # Check if the resource's attributes failed schema validation. - # - # @return [Boolean] - def from_resource? - source == :SOURCE_RESOURCE - end - end - # Additional information about how policy decisions were reached. CheckResources::Result::Metadata = Output.new_class(:actions, :effective_derived_roles) do # @!attribute [r] actions diff --git a/lib/cerbos/output/plan_resources.rb b/lib/cerbos/output/plan_resources.rb index ac5f0d6..8425c58 100644 --- a/lib/cerbos/output/plan_resources.rb +++ b/lib/cerbos/output/plan_resources.rb @@ -5,7 +5,7 @@ module Output # A query plan that can be used to obtain a list of resources on which a principal is allowed to perform a particular action. # # @see Client#plan_resources - PlanResources = Output.new_class(:request_id, :kind, :condition, :metadata) do + PlanResources = Output.new_class(:request_id, :kind, :condition, :validation_errors, :metadata) do # @!attribute [r] request_id # The identifier for tracing the request. # @@ -26,6 +26,11 @@ module Output # @see #always_denied? # @see #conditional? + # @!attribute [r] validation_errors + # Any schema validation errors for the principal or resource attributes. + # + # @return [Array] + # @!attribute [r] metadata # Additional information about the query plan. # @@ -37,6 +42,7 @@ def self.from_protobuf(plan_resources) request_id: plan_resources.request_id, kind: plan_resources.filter.kind, condition: PlanResources::Expression::Operand.from_protobuf(plan_resources.filter.condition), + validation_errors: (plan_resources.validation_errors || []).map { |validation_error| ValidationError.from_protobuf(validation_error) }, metadata: PlanResources::Metadata.from_protobuf(plan_resources.meta) ) end diff --git a/lib/cerbos/output/validation_error.rb b/lib/cerbos/output/validation_error.rb new file mode 100644 index 0000000..5a6e7f6 --- /dev/null +++ b/lib/cerbos/output/validation_error.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +module Cerbos + module Output + # An error that occurred while validating the principal or resource attributes against a schema. + ValidationError = Output.new_class(:path, :message, :source) do + # @!attribute [r] path + # The path to the attribute that failed validation. + # + # @return [String] + + # @!attribute [r] message + # The error message. + # + # @return [String] + + # @!attribute [r] source + # The source of the invalid attributes. + # + # @return [:SOURCE_PRINCIPAL, :SOURCE_RESOURCE] + + def self.from_protobuf(validation_error) + new( + path: validation_error.path, + message: validation_error.message, + source: validation_error.source + ) + end + + # Check if the principal's attributes failed schema validation. + # + # @return [Boolean] + def from_principal? + source == :SOURCE_PRINCIPAL + end + + # Check if the resource's attributes failed schema validation. + # + # @return [Boolean] + def from_resource? + source == :SOURCE_RESOURCE + end + end + end +end diff --git a/lib/cerbos/protobuf/cerbos/engine/v1/engine_pb.rb b/lib/cerbos/protobuf/cerbos/engine/v1/engine_pb.rb index f41f099..66665c5 100644 --- a/lib/cerbos/protobuf/cerbos/engine/v1/engine_pb.rb +++ b/lib/cerbos/protobuf/cerbos/engine/v1/engine_pb.rb @@ -75,6 +75,7 @@ optional :scope, :string, 5, json_name: "scope" optional :filter, :message, 6, "cerbos.engine.v1.PlanResourcesFilter", json_name: "filter" optional :filter_debug, :string, 7, json_name: "filterDebug" + repeated :validation_errors, :message, 8, "cerbos.schema.v1.ValidationError", json_name: "validationErrors" end add_message "cerbos.engine.v1.CheckInput" do optional :request_id, :string, 1, json_name: "requestId" diff --git a/lib/cerbos/protobuf/cerbos/response/v1/response_pb.rb b/lib/cerbos/protobuf/cerbos/response/v1/response_pb.rb index 96feedf..ddd3419 100644 --- a/lib/cerbos/protobuf/cerbos/response/v1/response_pb.rb +++ b/lib/cerbos/protobuf/cerbos/response/v1/response_pb.rb @@ -20,6 +20,7 @@ optional :policy_version, :string, 4, json_name: "policyVersion" optional :filter, :message, 5, "cerbos.engine.v1.PlanResourcesFilter", json_name: "filter" optional :meta, :message, 6, "cerbos.response.v1.PlanResourcesResponse.Meta", json_name: "meta" + repeated :validation_errors, :message, 7, "cerbos.schema.v1.ValidationError", json_name: "validationErrors" end add_message "cerbos.response.v1.PlanResourcesResponse.Meta" do optional :filter_debug, :string, 1, json_name: "filterDebug" diff --git a/spec/cerbos/client_spec.rb b/spec/cerbos/client_spec.rb index 5741c1a..149b598 100644 --- a/spec/cerbos/client_spec.rb +++ b/spec/cerbos/client_spec.rb @@ -19,7 +19,7 @@ roles: ["USER"], attributes: { country: { - alpha2: "NZ", + alpha2: "", alpha3: "NZL" } } @@ -58,7 +58,7 @@ roles: ["USER"], attributes: { country: { - alpha2: "NZ", + alpha2: "", alpha3: "NZL" } } @@ -96,7 +96,13 @@ "edit" => :EFFECT_ALLOW, "delete" => :EFFECT_ALLOW }, - validation_errors: [], + validation_errors: [ + Cerbos::Output::ValidationError.new( + path: "/country/alpha2", + message: "does not match pattern '[A-Z]{2}'", + source: :SOURCE_PRINCIPAL + ) + ], metadata: Cerbos::Output::CheckResources::Result::Metadata.new( actions: { "view" => Cerbos::Output::CheckResources::Result::Metadata::Effect.new( @@ -128,7 +134,7 @@ roles: ["USER"], attributes: { country: { - alpha2: "NZ", + alpha2: "", alpha3: "NZL" } } @@ -197,7 +203,13 @@ "edit" => :EFFECT_ALLOW, "delete" => :EFFECT_ALLOW }, - validation_errors: [], + validation_errors: [ + Cerbos::Output::ValidationError.new( + path: "/country/alpha2", + message: "does not match pattern '[A-Z]{2}'", + source: :SOURCE_PRINCIPAL + ) + ], metadata: Cerbos::Output::CheckResources::Result::Metadata.new( actions: { "view" => Cerbos::Output::CheckResources::Result::Metadata::Effect.new( @@ -228,7 +240,13 @@ "edit" => :EFFECT_DENY, "delete" => :EFFECT_ALLOW }, - validation_errors: [], + validation_errors: [ + Cerbos::Output::ValidationError.new( + path: "/country/alpha2", + message: "does not match pattern '[A-Z]{2}'", + source: :SOURCE_PRINCIPAL + ) + ], metadata: Cerbos::Output::CheckResources::Result::Metadata.new( actions: { "view" => Cerbos::Output::CheckResources::Result::Metadata::Effect.new( @@ -255,12 +273,17 @@ scope: "test" ), actions: { - "view" => :EFFECT_DENY, + "view" => :EFFECT_ALLOW, "edit" => :EFFECT_DENY, - "delete" => :EFFECT_DENY + "delete" => :EFFECT_ALLOW }, validation_errors: [ - Cerbos::Output::CheckResources::Result::ValidationError.new( + Cerbos::Output::ValidationError.new( + path: "/country/alpha2", + message: "does not match pattern '[A-Z]{2}'", + source: :SOURCE_PRINCIPAL + ), + Cerbos::Output::ValidationError.new( path: "/owner", message: "expected string, but got number", source: :SOURCE_RESOURCE @@ -270,7 +293,7 @@ actions: { "view" => Cerbos::Output::CheckResources::Result::Metadata::Effect.new( matched_policy: "resource.document.v1/test", - matched_scope: "" + matched_scope: "test" ), "edit" => Cerbos::Output::CheckResources::Result::Metadata::Effect.new( matched_policy: "resource.document.v1/test", @@ -299,7 +322,7 @@ roles: ["USER"], attributes: { country: { - alpha2: "NZ", + alpha2: "", alpha3: "NZL" } } @@ -332,6 +355,18 @@ Cerbos::Output::PlanResources::Expression::Value.new(value: "me@example.com") ] ), + validation_errors: + if cerbos_version_at_least?("0.19.0-prerelease") + [ + Cerbos::Output::ValidationError.new( + path: "/country/alpha2", + message: "does not match pattern '[A-Z]{2}'", + source: :SOURCE_PRINCIPAL + ) + ] + else + [] + end, metadata: Cerbos::Output::PlanResources::Metadata.new( condition_string: if cerbos_version_at_least?("0.18.0") @@ -360,7 +395,7 @@ context "when configured to raise on validation error" do let(:on_validation_error) { :raise } - it "raises an error when validation fails", :aggregate_failures do + it "raises an error when validation fails in #check_resources", :aggregate_failures do expect { client.allow?( principal: { @@ -370,7 +405,7 @@ roles: ["USER"], attributes: { country: { - alpha2: "NZ", + alpha2: "", alpha3: "NZL" } } @@ -389,7 +424,12 @@ }.to raise_error { |error| expect(error).to be_a(Cerbos::Error::ValidationFailed).and(have_attributes( validation_errors: [ - Cerbos::Output::CheckResources::Result::ValidationError.new( + Cerbos::Output::ValidationError.new( + path: "/country/alpha2", + message: "does not match pattern '[A-Z]{2}'", + source: :SOURCE_PRINCIPAL + ), + Cerbos::Output::ValidationError.new( path: "/owner", message: "expected string, but got number", source: :SOURCE_RESOURCE @@ -398,12 +438,50 @@ )) } end + + it "raises an error when validation fails in #plan_resources", :aggregate_failures do + skip "Not supported before Cerbos 0.19.0" unless cerbos_version_at_least?("0.19.0-prerelease") + + expect { + client.plan_resources( + principal: { + id: "me@example.com", + policy_version: "1", + scope: "test", + roles: ["USER"], + attributes: { + country: { + alpha2: "", + alpha3: "NZL" + } + } + }, + resource: { + kind: "document", + policy_version: "1", + scope: "test", + attributes: {} + }, + action: "edit" + ) + }.to raise_error { |error| + expect(error).to be_a(Cerbos::Error::ValidationFailed).and(have_attributes( + validation_errors: [ + Cerbos::Output::ValidationError.new( + path: "/country/alpha2", + message: "does not match pattern '[A-Z]{2}'", + source: :SOURCE_PRINCIPAL + ) + ] + )) + } + end end context "when configured with a callback on validation error" do let(:on_validation_error) { instance_double(Proc, call: nil) } - it "raises an error when validation fails", :aggregate_failures do + it "invokes the callback when validation fails in #check_resources", :aggregate_failures do client.allow?( principal: { id: "me@example.com", @@ -412,7 +490,7 @@ roles: ["USER"], attributes: { country: { - alpha2: "NZ", + alpha2: "", alpha3: "NZL" } } @@ -430,13 +508,52 @@ ) expect(on_validation_error).to have_received(:call).with([ - Cerbos::Output::CheckResources::Result::ValidationError.new( + Cerbos::Output::ValidationError.new( + path: "/country/alpha2", + message: "does not match pattern '[A-Z]{2}'", + source: :SOURCE_PRINCIPAL + ), + Cerbos::Output::ValidationError.new( path: "/owner", message: "expected string, but got number", source: :SOURCE_RESOURCE ) ]) end + + it "invokes the callback when validation fails in #plan_resources", :aggregate_failures do + skip "Not supported before Cerbos 0.19.0" unless cerbos_version_at_least?("0.19.0-prerelease") + + client.plan_resources( + principal: { + id: "me@example.com", + policy_version: "1", + scope: "test", + roles: ["USER"], + attributes: { + country: { + alpha2: "", + alpha3: "NZL" + } + } + }, + resource: { + kind: "document", + policy_version: "1", + scope: "test", + attributes: {} + }, + action: "edit" + ) + + expect(on_validation_error).to have_received(:call).with([ + Cerbos::Output::ValidationError.new( + path: "/country/alpha2", + message: "does not match pattern '[A-Z]{2}'", + source: :SOURCE_PRINCIPAL + ) + ]) + end end end diff --git a/spec/cerbos/output/check_resources_spec.rb b/spec/cerbos/output/check_resources_spec.rb index dfd4fe4..e3eb87d 100644 --- a/spec/cerbos/output/check_resources_spec.rb +++ b/spec/cerbos/output/check_resources_spec.rb @@ -181,4 +181,67 @@ def build_result(**resource) ) end end + + describe "#validation_errors" do + subject(:validation_errors) { check_resources.validation_errors } + + let(:results) do + [ + Cerbos::Output::CheckResources::Result.new( + resource: Cerbos::Output::CheckResources::Result::Resource.new( + kind: "document", + id: "1", + policy_version: "default", + scope: "" + ), + actions: {"allowed" => :EFFECT_ALLOW}, + validation_errors: [ + Cerbos::Output::ValidationError.new( + path: "/country/alpha2", + message: "does not match pattern '[A-Z]{2}'", + source: :SOURCE_PRINCIPAL + ) + ], + metadata: nil + ), + Cerbos::Output::CheckResources::Result.new( + resource: Cerbos::Output::CheckResources::Result::Resource.new( + kind: "document", + id: "2", + policy_version: "default", + scope: "" + ), + actions: {"allowed" => :EFFECT_ALLOW}, + validation_errors: [ + Cerbos::Output::ValidationError.new( + path: "/country/alpha2", + message: "does not match pattern '[A-Z]{2}'", + source: :SOURCE_PRINCIPAL + ), + Cerbos::Output::ValidationError.new( + path: "/owner", + message: "expected string, but got number", + source: :SOURCE_RESOURCE + ) + ], + metadata: nil + ) + ] + end + + it "lists validation errors from results" do + expect(validation_errors).to eq([ + Cerbos::Output::ValidationError.new( + path: "/country/alpha2", + message: "does not match pattern '[A-Z]{2}'", + source: :SOURCE_PRINCIPAL + ), + Cerbos::Output::ValidationError.new( + path: "/owner", + message: "expected string, but got number", + source: :SOURCE_RESOURCE + ) + ]) + end + end end diff --git a/spec/cerbos/output/plan_resources_spec.rb b/spec/cerbos/output/plan_resources_spec.rb index 10f4953..ed7060e 100644 --- a/spec/cerbos/output/plan_resources_spec.rb +++ b/spec/cerbos/output/plan_resources_spec.rb @@ -1,7 +1,15 @@ # frozen_string_literal: true RSpec.describe Cerbos::Output::PlanResources do - subject(:plan_resources) { described_class.new(request_id: "42", kind: kind, condition: Cerbos::Output::PlanResources::Expression::Value.new(value: true), metadata: nil) } + subject(:plan_resources) do + described_class.new( + request_id: "42", + kind: kind, + condition: Cerbos::Output::PlanResources::Expression::Value.new(value: true), + validation_errors: [], + metadata: nil + ) + end context "when always allowed" do let(:kind) { :KIND_ALWAYS_ALLOWED } diff --git a/spec/cerbos/output/check_resources/result/validation_error_spec.rb b/spec/cerbos/output/validation_error_spec.rb similarity index 86% rename from spec/cerbos/output/check_resources/result/validation_error_spec.rb rename to spec/cerbos/output/validation_error_spec.rb index c5eabf4..f92978d 100644 --- a/spec/cerbos/output/check_resources/result/validation_error_spec.rb +++ b/spec/cerbos/output/validation_error_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -RSpec.describe Cerbos::Output::CheckResources::Result::ValidationError do +RSpec.describe Cerbos::Output::ValidationError do subject(:validation_error) { described_class.new(path: "/foo", message: "bar", source: source) } context "when source is principal" do diff --git a/spec/servers/config/plaintext.cerbos.yaml b/spec/servers/config/plaintext.cerbos.yaml index 7bdb092..8c8d4dc 100644 --- a/spec/servers/config/plaintext.cerbos.yaml +++ b/spec/servers/config/plaintext.cerbos.yaml @@ -3,7 +3,7 @@ auxData: disableVerification: true schema: - enforcement: reject + enforcement: warn storage: driver: disk diff --git a/spec/servers/config/socket.cerbos.yaml b/spec/servers/config/socket.cerbos.yaml index 86ba864..7fd76f9 100644 --- a/spec/servers/config/socket.cerbos.yaml +++ b/spec/servers/config/socket.cerbos.yaml @@ -3,7 +3,7 @@ auxData: disableVerification: true schema: - enforcement: reject + enforcement: warn server: grpcListenAddr: unix:/socket/cerbos diff --git a/spec/servers/config/tls.cerbos.yaml b/spec/servers/config/tls.cerbos.yaml index dc06820..c678418 100644 --- a/spec/servers/config/tls.cerbos.yaml +++ b/spec/servers/config/tls.cerbos.yaml @@ -3,7 +3,7 @@ auxData: disableVerification: true schema: - enforcement: reject + enforcement: warn server: tls: diff --git a/spec/servers/policies/_schemas/principal.json b/spec/servers/policies/_schemas/principal.json new file mode 100644 index 0000000..0a741dc --- /dev/null +++ b/spec/servers/policies/_schemas/principal.json @@ -0,0 +1,19 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "country": { + "type": "object", + "properties": { + "alpha2": { + "type": "string", + "pattern": "[A-Z]{2}" + }, + "alpha3": { + "type": "string", + "pattern": "[A-Z]{3}" + } + } + } + } +} diff --git a/spec/servers/policies/document.yaml b/spec/servers/policies/document.yaml index b0f3e00..aa44288 100644 --- a/spec/servers/policies/document.yaml +++ b/spec/servers/policies/document.yaml @@ -3,6 +3,8 @@ resourcePolicy: resource: document version: "1" schemas: + principalSchema: + ref: cerbos:///principal.json resourceSchema: ref: cerbos:///document.json rules: diff --git a/spec/servers/policies/test/document.yaml b/spec/servers/policies/test/document.yaml index 223d9b5..991ee16 100644 --- a/spec/servers/policies/test/document.yaml +++ b/spec/servers/policies/test/document.yaml @@ -4,6 +4,8 @@ resourcePolicy: version: "1" scope: test schemas: + principalSchema: + ref: cerbos:///principal.json resourceSchema: ref: cerbos:///document.json importDerivedRoles: