From 33366949cb3be161f7d79e28cb677fcd1b5e21c8 Mon Sep 17 00:00:00 2001 From: Jason Carter Date: Mon, 19 Mar 2018 17:44:24 -0700 Subject: [PATCH 1/8] Implement Controller#brainstem_validate_params! which validates the given parameters & either raises an error or ignores unknown fields Signed-off-by: Shirish Pampoorickal --- lib/brainstem/concerns/controller_dsl.rb | 49 +++++ lib/brainstem/unknown_params.rb | 4 + .../brainstem/concerns/controller_dsl_spec.rb | 193 ++++++++++++++++++ 3 files changed, 246 insertions(+) create mode 100644 lib/brainstem/unknown_params.rb diff --git a/lib/brainstem/concerns/controller_dsl.rb b/lib/brainstem/concerns/controller_dsl.rb index a2948c5f..d5626c7e 100644 --- a/lib/brainstem/concerns/controller_dsl.rb +++ b/lib/brainstem/concerns/controller_dsl.rb @@ -1,4 +1,5 @@ require 'brainstem/concerns/inheritable_configuration' +require 'brainstem/unknown_params' require 'active_support/core_ext/object/with_options' module Brainstem @@ -380,6 +381,54 @@ def brainstem_valid_params(requested_context = action_name.to_sym, root_param_na end alias_method :brainstem_valid_params_for, :brainstem_valid_params + # + # Ensures that the parameters passed in are valid. + # To force erros on bad paramers set optins[:ignore_unknown_fields] to true, + # and pass the desired custom error message + # + def brainstem_validate_params!(requested_context = action_name.to_sym, root_param_name = brainstem_model_name, options = {}) + error_params = brainstem_valid_params(requested_context, root_param_name) + + object = options[:object] || params.with_indifferent_access[brainstem_model_name] + recursive_message = options[:recursive_key].present? ? " for #{options[:recursive_key]}" : "" + + if !object.is_a?(Hash) || object.keys.length == 0 + raise ::Brainstem::UnknownParams.new("Missing required parameters#{recursive_message}.") + else + object = object.with_indifferent_access + end + + strange_params = [] + object.keys.each do |param| + info = error_params[param] + if info.present? + # It might be a valid param + if info.is_a?(Hash) + if info[:_config][:only] && info[:_config][:only].to_s != requested_context.to_s + strange_params << param + end + if info[:_config][:recursive] && object[param].present? + recursive_params = object[param].is_a?(Hash) ? object[param].values : object[param] + recursive_params.each do |sub_object| + return false unless brainstem_validate_params!(requested_context, root_param_name, options.merge(object: sub_object, recursive_key: param)) + end + end + end + else + # Definitely not on the list. + if options[:ignore_unknown_fields] == 'true' + object.delete(param) + else + strange_params << param + end + end + end + if strange_params.length > 0 + raise ::Brainstem::UnknownParams.new("Unknown params#{recursive_message}: #{strange_params.join(', ')}.") + else + true + end + end # # Lists all incoming param keys that will be rewritten to use a different diff --git a/lib/brainstem/unknown_params.rb b/lib/brainstem/unknown_params.rb new file mode 100644 index 00000000..d0aa64ba --- /dev/null +++ b/lib/brainstem/unknown_params.rb @@ -0,0 +1,4 @@ +module Brainstem + class UnknownParams < StandardError + end +end diff --git a/spec/brainstem/concerns/controller_dsl_spec.rb b/spec/brainstem/concerns/controller_dsl_spec.rb index 4e54f11b..f8fb6472 100644 --- a/spec/brainstem/concerns/controller_dsl_spec.rb +++ b/spec/brainstem/concerns/controller_dsl_spec.rb @@ -983,6 +983,199 @@ module Concerns end end + describe "#brainstem_validate_params!" do + let(:brainstem_model_name) { "widget" } + let(:input_params) { { widget: { sprocket_parent_id: 5, sprocket_name: 'gears' } } } + + before do + stub(subject).brainstem_model_name { brainstem_model_name } + stub.any_instance_of(subject).brainstem_model_name { brainstem_model_name } + stub.any_instance_of(subject).params { input_params } + + subject.brainstem_params do + actions :update do + model_params(brainstem_model_name) do |params| + params.valid :sprocket_parent_id, :long, + info: "sprockets[sprocket_parent_id] is not required" + + params.valid :sprocket_name, :string, + info: "sprockets[sprocket_name] is required", + required: true + end + end + end + end + + it "returns true if params are OK" do + expect(subject.new.brainstem_validate_params!(:update, brainstem_model_name)).to be_truthy + end + + context "when required parameters are invalid" do + context "with an empty hash" do + let(:input_params) { {} } + + it "returns false and says params are missing" do + expect { + subject.new.brainstem_validate_params!(:update, brainstem_model_name) + }.to raise_error(Brainstem::UnknownParams) + end + end + + context "with a non-hash object" do + let(:input_params) { { widget: [{ foo: "bar" }] } } + + it "returns false and says params are missing" do + expect { + subject.new.brainstem_validate_params!(:update, brainstem_model_name) + }.to raise_error(Brainstem::UnknownParams) + end + end + + context "with nil" do + let(:input_params) { { widget: nil } } + + it "returns false and says params are missing" do + expect { + subject.new.brainstem_validate_params!(:update, brainstem_model_name) + }.to raise_error(Brainstem::UnknownParams) + end + end + end + + context "when there are errors due to unknown params" do + let(:input_params) { { widget: { my_cool_param: "something" } } } + + it "lists unknown params" do + expect{ + subject.new.brainstem_validate_params!(:update, brainstem_model_name) + }.to raise_error(Brainstem::UnknownParams, "Unknown params: my_cool_param." ) + end + end + + context "when params are supplied to the wrong action" do + before do + subject.brainstem_params do + actions :create do |params| + model_params(brainstem_model_name) do |params| + params.valid :sprocket_name, :string, + info: "sprockets[sprocket_name] is required", + required: true + end + end + end + end + + it "throws an unknown params error" do + expect{ + subject.new.brainstem_validate_params!(:create, brainstem_model_name) + }.to raise_error(Brainstem::UnknownParams) + end + end + + context "when `object` property is specified in options" do + let(:input_params) { { widget: { do_not_use: true } } } + + it "uses the object instead of the default brainstem valid params" do + validate_params = subject.new.brainstem_validate_params!( + :update, + brainstem_model_name, + :object => { :sprocket_name => "present", :sprocket_parent_id => 0 } + ) + expect(validate_params).to be_truthy + end + end + + context "when recursive attribute is given" do + before do + subject.brainstem_params do + actions :create do + model_params(brainstem_model_name) do |params| + params.valid :title, :string + params.valid :old_title, :string, legacy: true + params.valid :workspace_id, :integer, only: :update, info: "You need a workspace_id" + params.valid :sub_tasks, :array, recursive: true, info: "Sub tasks params are incorrect" + end + end + end + end + + context "when recursive attribute is an array" do + # Context Params For Sub tasks Error Message + TEST_CASES = [ + ["params is empty", {}, "Missing required parameters for sub_tasks"], + ["params includes unknown attributes", { wang: :chung }, "Unknown params for sub_tasks: wang"], + ["params includes only attribute", { title: "Invalid", workspace_id: 0 }, "Unknown params for sub_tasks: workspace_id"] + ] + + context "when attributes are invalid" do + context "when attribute is an array" do + TEST_CASES.each do |context_description, sub_task_params, error_message| + context "when #{context_description}" do + let(:input_params) { { title: "foo", sub_tasks: [sub_task_params] } } + + it "returns false" do + expect { + subject.new.brainstem_validate_params!(:create, brainstem_model_name) + }.to raise_error(Brainstem::UnknownParams) + end + end + end + end + + context "when attribute is a hash" do + TEST_CASES.each do |context_description, sub_task_params, error_message| + context "when #{context_description}" do + let(:input_params) { { title: "foo", sub_tasks: { 0 => sub_task_params } } } + + it "returns false" do + expect { + subject.new.brainstem_validate_params!(:create, brainstem_model_name) + }.to raise_error(Brainstem::UnknownParams) + end + end + end + end + end + + context "when attributes are valid" do + let(:input_params) { { widget: { title: "parent", sub_tasks: sub_task_params } } } + + context "when attribute is nil" do + let(:sub_task_params) { nil } + + it "returns true" do + expect(subject.new.brainstem_validate_params!(:create, brainstem_model_name)).to be_truthy + end + end + + context "when attribute is an empty array" do + let(:sub_task_params) { [] } + + it "returns true" do + expect(subject.new.brainstem_validate_params!(:create, brainstem_model_name)).to be_truthy + end + end + + context "when attributes in an array" do + let(:sub_task_params) { [ { title: "child" } ] } + + it "returns true" do + expect(subject.new.brainstem_validate_params!(:create, brainstem_model_name)).to be_truthy + end + end + + context "when attributes in an hash" do + let(:sub_task_params) { { 0 => { title: "child" } } } + + it "returns true" do + expect(subject.new.brainstem_validate_params!(:create, brainstem_model_name)).to be_truthy + end + end + end + end + end + end + describe "#transforms" do before do subject.brainstem_params do From 5c55e4ab41af515cbbd7b238920f1ce27b66e065 Mon Sep 17 00:00:00 2001 From: Shirish Pampoorickal Date: Fri, 6 Apr 2018 14:17:14 -0700 Subject: [PATCH 2/8] Update ChangeLog with new features for sanitizing and validating input parameters --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cd442de..f3171c5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog ++ **1.4.0** - _04/27/2018_ + - Add the capability to validate params based on documented parameters on endpoints. Invoking + `brainstem_validate_params!` raises an error if unknown parameters are encountered. + - Add the capability to sanitize params based on documented parameters on endpoints. Invoking + `brainstem_sanitize_params` returns sanitized params while excluding unknown parameters. + + **1.3.0** - _04/12/2018_ - Add the capability to nest fields under executable parent blocks where the nested fields are evaluated with the resultant value of the parent field. From 986c4272410a149cfff3410a8e38b4c700fcae9e Mon Sep 17 00:00:00 2001 From: Shirish Pampoorickal Date: Wed, 11 Apr 2018 16:34:01 -0700 Subject: [PATCH 3/8] Add capability to validate and sanitize input params based on documented parameters on the endpoint - 'brainstem_validate_params!' validates input parameters against the documented parameters on the endpoint. It raises Brainstem::UnknownParams error when the input params are malformed or unkown params are encountered - 'brainstem_ignore_unknown_params!' returns a sanitized hash while excluding unknown params --- CHANGELOG.md | 4 +- lib/brainstem/concerns/controller_dsl.rb | 87 +++--- lib/brainstem/params_validator.rb | 88 ++++++ lib/brainstem/unknown_params.rb | 6 + .../brainstem/concerns/controller_dsl_spec.rb | 161 ++++------ spec/brainstem/params_validator_spec.rb | 294 ++++++++++++++++++ spec/brainstem/presenter_collection_spec.rb | 4 +- 7 files changed, 487 insertions(+), 157 deletions(-) create mode 100644 lib/brainstem/params_validator.rb create mode 100644 spec/brainstem/params_validator_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e6f4cfd..b039db05 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,8 +3,8 @@ + **1.4.0** - _04/27/2018_ - Add the capability to validate params based on documented parameters on endpoints. Invoking `brainstem_validate_params!` raises an error if unknown parameters are encountered. - - Add the capability to sanitize params based on documented parameters on endpoints. Invoking - `brainstem_sanitize_params` returns sanitized params while excluding unknown parameters. + - Add the capability to ignore unknown params based off documented parameters on endpoints. Invoking + `brainstem_ignore_unknown_params!` returns a copy of params while excluding unknown parameters. + **1.3.0** - _04/12/2018_ - Add the capability to nest fields under evaluable parent blocks where the nested fields are evaluated diff --git a/lib/brainstem/concerns/controller_dsl.rb b/lib/brainstem/concerns/controller_dsl.rb index d5626c7e..4d139ec5 100644 --- a/lib/brainstem/concerns/controller_dsl.rb +++ b/lib/brainstem/concerns/controller_dsl.rb @@ -1,5 +1,5 @@ require 'brainstem/concerns/inheritable_configuration' -require 'brainstem/unknown_params' +require 'brainstem/params_validator' require 'active_support/core_ext/object/with_options' module Brainstem @@ -371,7 +371,8 @@ def valid_params_tree(requested_context = action_name.to_sym) # Lists all valid parameters for the current action. Falls back to the # valid parameters for the default context. # - # @params [Symbol] requested_context the context which to look up. + # @params [String, Symbol] (Optional) requested_context the context which to look up. + # @params [String, Symbol] (Optional) root_param_name the param name of the model being changed. # # @return [Hash{String => String, Hash] a hash of pairs of param names and # descriptions or sub-hashes. @@ -382,52 +383,46 @@ def brainstem_valid_params(requested_context = action_name.to_sym, root_param_na alias_method :brainstem_valid_params_for, :brainstem_valid_params # - # Ensures that the parameters passed in are valid. - # To force erros on bad paramers set optins[:ignore_unknown_fields] to true, - # and pass the desired custom error message + # Ensures that the parameters passed through to the action are valid. # - def brainstem_validate_params!(requested_context = action_name.to_sym, root_param_name = brainstem_model_name, options = {}) - error_params = brainstem_valid_params(requested_context, root_param_name) - - object = options[:object] || params.with_indifferent_access[brainstem_model_name] - recursive_message = options[:recursive_key].present? ? " for #{options[:recursive_key]}" : "" - - if !object.is_a?(Hash) || object.keys.length == 0 - raise ::Brainstem::UnknownParams.new("Missing required parameters#{recursive_message}.") - else - object = object.with_indifferent_access - end + # It raises Brainstem::UnknownParams.new(message, unknown_params) error, + # when params are missing or unknown params are encountered + # + # @params [String, Symbol] (Optional) requested_context the context which to look up. + # @params [String, Symbol] (Optional) root_param_name the param name of the model being changed. + # + def brainstem_validate_params!(requested_context = action_name.to_sym, root_param_name = brainstem_model_name) + input_params = params.with_indifferent_access[brainstem_model_name] + brainstem_params_config = brainstem_valid_params(requested_context, root_param_name) + + Brainstem::ParamsValidator.validate!( + requested_context, + input_params, + brainstem_params_config + ).present? + end - strange_params = [] - object.keys.each do |param| - info = error_params[param] - if info.present? - # It might be a valid param - if info.is_a?(Hash) - if info[:_config][:only] && info[:_config][:only].to_s != requested_context.to_s - strange_params << param - end - if info[:_config][:recursive] && object[param].present? - recursive_params = object[param].is_a?(Hash) ? object[param].values : object[param] - recursive_params.each do |sub_object| - return false unless brainstem_validate_params!(requested_context, root_param_name, options.merge(object: sub_object, recursive_key: param)) - end - end - end - else - # Definitely not on the list. - if options[:ignore_unknown_fields] == 'true' - object.delete(param) - else - strange_params << param - end - end - end - if strange_params.length > 0 - raise ::Brainstem::UnknownParams.new("Unknown params#{recursive_message}: #{strange_params.join(', ')}.") - else - true - end + # + # Ensures that known / documented parameters are passed through to the action. + # + # It raises Brainstem::UnknownParams.new(message, unknown_params) error, + # when params are empty or not a Hash. + # + # @params [String, Symbol] (Optional) requested_context the context which to look up. + # @params [String, Symbol] (Optional) root_param_name the param name of the model being changed. + # + # @return [Hash{String => String, Hash] a hash of pairs of param names and descriptions or sub-hashes. + # + def brainstem_ignore_unknown_params!(requested_context = action_name.to_sym, root_param_name = brainstem_model_name) + input_params = params.with_indifferent_access[brainstem_model_name] + brainstem_params_config = brainstem_valid_params(requested_context, root_param_name) + + Brainstem::ParamsValidator.validate!( + requested_context, + input_params, + brainstem_params_config, + ignore_unknown_fields: true + ) end # diff --git a/lib/brainstem/params_validator.rb b/lib/brainstem/params_validator.rb new file mode 100644 index 00000000..938ddf8a --- /dev/null +++ b/lib/brainstem/params_validator.rb @@ -0,0 +1,88 @@ +require 'brainstem/unknown_params' + +module Brainstem + class ParamsValidator + attr_reader :unknown_params, :sanitized_params + + def self.validate!(action_name, input_params, valid_params_config, options = {}) + new(action_name, input_params, valid_params_config, options).validate! + end + + def initialize(action_name, input_params, valid_params_config, options = {}) + @valid_params_config = valid_params_config + @options = options + @input_params = sanitize_input_params!(input_params) + @action_name = action_name.to_s + + @unknown_params = [] + @sanitized_params = {} + end + + def validate! + @input_params.each do |param_key, param_value| + param = @valid_params_config[param_key] + + if param.blank? + @unknown_params << param_key + next + end + + param_config = param[:_config] + if param_config[:only].present? && !param_config[:only].map(&:to_s).include?(@action_name) + @unknown_params << param_key + + elsif param_config[:recursive].to_s == 'true' + next if param_value.blank? + + @sanitized_params[param_key] = validate_recursive_params!(param_config[:type], param_key, param_value) + else + @sanitized_params[param_key] = param_value + end + end + + raise_with_unknown_params? ? unknown_params_error! : @sanitized_params + end + + private + + def validate_recursive_params!(param_type, param_key, param_value) + if param_type == 'hash' + validate_recursive_param(param_key, param_value) + else + param_value.each_with_index.map { |value, index| validate_recursive_param(param_key, value, index) } + end + end + + def validate_recursive_param(param_key, param_value, index = nil) + begin + result = self.class.validate!(@action_name, param_value, @valid_params_config, @options) + rescue Brainstem::UnknownParams => e + @unknown_params << { param_key => e.unknown_params } + end + + result + end + + def raise_with_unknown_params? + !ignore_unknown_params? && @unknown_params.present? + end + + def ignore_unknown_params? + @options[:ignore_unknown_fields].to_s == 'true' + end + + def sanitize_input_params!(input_params) + missing_params_error! unless input_params.is_a?(Hash) && input_params.present? + + input_params + end + + def missing_params_error! + raise ::Brainstem::UnknownParams.new("Missing required parameters") + end + + def unknown_params_error! + raise ::Brainstem::UnknownParams.new("Unknown params encountered", @unknown_params) + end + end +end diff --git a/lib/brainstem/unknown_params.rb b/lib/brainstem/unknown_params.rb index d0aa64ba..132c5fe0 100644 --- a/lib/brainstem/unknown_params.rb +++ b/lib/brainstem/unknown_params.rb @@ -1,4 +1,10 @@ module Brainstem class UnknownParams < StandardError + attr_reader :unknown_params + + def initialize(message = "Unidentified Params sighted", unknown_params = []) + @unknown_params = unknown_params + super(message) + end end end diff --git a/spec/brainstem/concerns/controller_dsl_spec.rb b/spec/brainstem/concerns/controller_dsl_spec.rb index f8fb6472..55fe59e8 100644 --- a/spec/brainstem/concerns/controller_dsl_spec.rb +++ b/spec/brainstem/concerns/controller_dsl_spec.rb @@ -1010,7 +1010,7 @@ module Concerns expect(subject.new.brainstem_validate_params!(:update, brainstem_model_name)).to be_truthy end - context "when required parameters are invalid" do + context "when parameters are in an invalid format" do context "with an empty hash" do let(:input_params) { {} } @@ -1046,132 +1046,79 @@ module Concerns let(:input_params) { { widget: { my_cool_param: "something" } } } it "lists unknown params" do - expect{ + expect { subject.new.brainstem_validate_params!(:update, brainstem_model_name) - }.to raise_error(Brainstem::UnknownParams, "Unknown params: my_cool_param." ) + }.to raise_error(Brainstem::UnknownParams, "Unknown params encountered") end end + end - context "when params are supplied to the wrong action" do - before do - subject.brainstem_params do - actions :create do |params| - model_params(brainstem_model_name) do |params| - params.valid :sprocket_name, :string, - info: "sprockets[sprocket_name] is required", - required: true - end - end - end - end - - it "throws an unknown params error" do - expect{ - subject.new.brainstem_validate_params!(:create, brainstem_model_name) - }.to raise_error(Brainstem::UnknownParams) - end - end + describe "#brainstem_ignore_unknown_params!" do + let(:brainstem_model_name) { "widget" } + let(:input_params) { { widget: { sprocket_parent_id: 5, sprocket_name: 'gears' } } } - context "when `object` property is specified in options" do - let(:input_params) { { widget: { do_not_use: true } } } + before do + stub(subject).brainstem_model_name { brainstem_model_name } + stub.any_instance_of(subject).brainstem_model_name { brainstem_model_name } + stub.any_instance_of(subject).params { input_params } - it "uses the object instead of the default brainstem valid params" do - validate_params = subject.new.brainstem_validate_params!( - :update, - brainstem_model_name, - :object => { :sprocket_name => "present", :sprocket_parent_id => 0 } - ) - expect(validate_params).to be_truthy - end - end + subject.brainstem_params do + actions :update do + model_params(brainstem_model_name) do |params| + params.valid :sprocket_parent_id, :long, + info: "sprockets[sprocket_parent_id] is not required" - context "when recursive attribute is given" do - before do - subject.brainstem_params do - actions :create do - model_params(brainstem_model_name) do |params| - params.valid :title, :string - params.valid :old_title, :string, legacy: true - params.valid :workspace_id, :integer, only: :update, info: "You need a workspace_id" - params.valid :sub_tasks, :array, recursive: true, info: "Sub tasks params are incorrect" - end + params.valid :sprocket_name, :string, + info: "sprockets[sprocket_name] is required", + required: true end end end + end - context "when recursive attribute is an array" do - # Context Params For Sub tasks Error Message - TEST_CASES = [ - ["params is empty", {}, "Missing required parameters for sub_tasks"], - ["params includes unknown attributes", { wang: :chung }, "Unknown params for sub_tasks: wang"], - ["params includes only attribute", { title: "Invalid", workspace_id: 0 }, "Unknown params for sub_tasks: workspace_id"] - ] - - context "when attributes are invalid" do - context "when attribute is an array" do - TEST_CASES.each do |context_description, sub_task_params, error_message| - context "when #{context_description}" do - let(:input_params) { { title: "foo", sub_tasks: [sub_task_params] } } - - it "returns false" do - expect { - subject.new.brainstem_validate_params!(:create, brainstem_model_name) - }.to raise_error(Brainstem::UnknownParams) - end - end - end - end + it "returns sanitized params if params are OK" do + expect(subject.new.brainstem_ignore_unknown_params!(:update, brainstem_model_name)). + to eq(input_params[:widget].with_indifferent_access) + end - context "when attribute is a hash" do - TEST_CASES.each do |context_description, sub_task_params, error_message| - context "when #{context_description}" do - let(:input_params) { { title: "foo", sub_tasks: { 0 => sub_task_params } } } + context "when parameters are in an invalid format" do + context "with an empty hash" do + let(:input_params) { {} } - it "returns false" do - expect { - subject.new.brainstem_validate_params!(:create, brainstem_model_name) - }.to raise_error(Brainstem::UnknownParams) - end - end - end - end + it "returns the empty hash" do + expect { + subject.new.brainstem_ignore_unknown_params!(:update, brainstem_model_name) + }.to raise_error(Brainstem::UnknownParams) end + end - context "when attributes are valid" do - let(:input_params) { { widget: { title: "parent", sub_tasks: sub_task_params } } } - - context "when attribute is nil" do - let(:sub_task_params) { nil } - - it "returns true" do - expect(subject.new.brainstem_validate_params!(:create, brainstem_model_name)).to be_truthy - end - end - - context "when attribute is an empty array" do - let(:sub_task_params) { [] } + context "with a non-hash object" do + let(:input_params) { { widget: [{ foo: "bar" }] } } - it "returns true" do - expect(subject.new.brainstem_validate_params!(:create, brainstem_model_name)).to be_truthy - end - end + it "returns the object" do + expect { + subject.new.brainstem_ignore_unknown_params!(:update, brainstem_model_name) + }.to raise_error(Brainstem::UnknownParams) + end + end - context "when attributes in an array" do - let(:sub_task_params) { [ { title: "child" } ] } + context "with nil" do + let(:input_params) { { widget: nil } } - it "returns true" do - expect(subject.new.brainstem_validate_params!(:create, brainstem_model_name)).to be_truthy - end - end + it "returns false and says params are missing" do + expect { + subject.new.brainstem_validate_params!(:update, brainstem_model_name) + }.to raise_error(Brainstem::UnknownParams) + end + end + end - context "when attributes in an hash" do - let(:sub_task_params) { { 0 => { title: "child" } } } + context "when unknown params are present" do + let(:input_params) { { widget: { sprocket_parent_id: 5, sprocket_name: 'gears', my_cool_param: "something" } } } - it "returns true" do - expect(subject.new.brainstem_validate_params!(:create, brainstem_model_name)).to be_truthy - end - end - end + it "returns the params without the unknown keys" do + expect(subject.new.brainstem_ignore_unknown_params!(:update, brainstem_model_name)) + .to eq({ sprocket_parent_id: 5, sprocket_name: 'gears' }.with_indifferent_access) end end end diff --git a/spec/brainstem/params_validator_spec.rb b/spec/brainstem/params_validator_spec.rb new file mode 100644 index 00000000..276f171a --- /dev/null +++ b/spec/brainstem/params_validator_spec.rb @@ -0,0 +1,294 @@ +require 'spec_helper' +require 'brainstem/params_validator' + +describe Brainstem::ParamsValidator do + let(:valid_params_config) do + { + sprocket_parent_id: { :_config => { type: 'integer' } }, + sprocket_name: { :_config => { type: 'string' } } + } + end + let(:options) { {} } + let(:action_name) { :create } + + subject { described_class.new(action_name, input_params, valid_params_config, options) } + + context "when input params has valid keys" do + let(:input_params) { { sprocket_parent_id: 5, sprocket_name: 'gears' } } + + it "returns sanitized params" do + expect(subject.validate!).to eq(input_params) + end + + context "when recursive attribute is given" do + let(:valid_params_config) do + { + sprocket_parent_id: { :_config => { type: 'integer' } }, + sprocket_name: { :_config => { type: 'string' } }, + sub_widget: { :_config => { type: 'hash', recursive: true } }, + sub_widgets: { :_config => { type: 'array', item_type: 'hash', recursive: true } }, + } + end + let(:action) { :create } + + context "when recursive attribute is a hash" do + let(:input_params) do + { + sprocket_parent_id: 5, + sprocket_name: 'gears', + sub_widget: sub_widget_param + } + end + + context "when attribute is nil" do + let(:sub_widget_param) { nil } + + it "returns the sanitized attributes without the empty recursive param" do + expect(subject.validate!).to eq(input_params.except(:sub_widget)) + end + end + + context "when attribute is an empty hash" do + let(:sub_widget_param) { {} } + + it "returns the sanitized attributes without the empty recursive param" do + expect(subject.validate!).to eq(input_params.except(:sub_widget)) + end + end + + context "when attributes are specified" do + let(:sub_widget_param) do + { + sprocket_parent_id: 15, + sprocket_name: 'ten gears', + } + end + + it "returns the sanitized attributes" do + expect(subject.validate!).to eq(input_params) + end + end + end + + context "when recursive attribute is an array" do + let(:input_params) do + { + sprocket_parent_id: 5, + sprocket_name: 'gears', + sub_widgets: sub_widgets_params + } + end + + context "when attribute is nil" do + let(:sub_widgets_params) { nil } + + it "returns the sanitized attributes without the empty recursive param" do + expect(subject.validate!).to eq(input_params.except(:sub_widgets)) + end + end + + context "when attribute is an empty array" do + let(:sub_widgets_params) { [] } + + it "returns the sanitized attributes without the empty recursive param" do + expect(subject.validate!).to eq(input_params.except(:sub_widgets)) + end + end + + context "when attributes in an array" do + let(:sub_widgets_params) { [ { sprocket_parent_id: 15, sprocket_name: 'ten gears' } ] } + + it "returns the sanitized attributes" do + expect(subject.validate!).to eq(input_params) + end + end + + skip "(UNSUPPORTED) when attributes in an hash" do + let(:sub_widgets_params) { { 0 => { title: "child" } } } + + it "returns the sanitized attributes" do + expect(subject.validate!).to eq(input_params) + end + end + end + end + end + + context "when input parameters are invalid" do + context "with an empty hash" do + let(:input_params) { {} } + + it "raises an missing params error" do + expect { subject.validate! }.to raise_error(Brainstem::UnknownParams) + end + end + + context "with a non-hash object" do + let(:input_params) { [{ foo: "bar" }] } + + it "raises an missing params error" do + expect { subject.validate! }.to raise_error(Brainstem::UnknownParams) + end + end + + context "with nil" do + let(:input_params) { nil } + + it "raises an missing params error" do + expect { subject.validate! }.to raise_error(Brainstem::UnknownParams) + end + end + end + + context "when `ignore_unknown_fields` is true" do + let(:options) { { ignore_unknown_fields: true } } + + context "when params are supplied to the wrong action" do + let(:valid_params_config) do + { + sprocket_parent_id: { :_config => { type: 'integer', only: [:update] } }, + sprocket_name: { :_config => { type: 'string' } } + } + end + let(:input_params) { { sprocket_parent_id: 5, sprocket_name: 'gears' } } + let(:action) { :create } + + it "returns the input params without the unknown params" do + expect(subject.validate!).to eq(input_params.except(:sprocket_parent_id)) + end + end + + context "when input params have unknown keys" do + let(:input_params) { { my_cool_param: "something" } } + + it "returns the input params without the unknown params" do + expect(subject.validate!).to eq(input_params.except(:my_cool_param)) + end + + context "when recursive attribute has invalid / unknown values" do + let(:valid_params_config) do + { + sprocket_parent_id: { :_config => { type: 'integer' } }, + sprocket_name: { :_config => { type: 'string' } }, + sub_widget: { :_config => { type: 'hash', recursive: true } }, + sub_widgets: { :_config => { type: 'array', item_type: 'hash', recursive: true } }, + } + end + + context "when recursive attribute is a hash" do + let(:input_params) do + { + sprocket_parent_id: 5, + sprocket_name: 'gears', + sub_widget: { + invalid_id: 15, + sprocket_name: 'ten gears', + } + } + end + + it "returns the input params without the unknown params" do + expect(subject.validate!).to eq({ + sprocket_parent_id: 5, + sprocket_name: 'gears', + sub_widget: { + sprocket_name: 'ten gears', + } + }) + end + end + + context "when recursive attribute is an array" do + let(:input_params) do + { + sprocket_parent_id: 5, + sprocket_name: 'gears', + sub_widgets: [ + { invalid_id: 15, sprocket_name: 'ten gears' } + ] + } + end + + it "returns the input params without the unknown params" do + expect(subject.validate!).to eq({ + sprocket_parent_id: 5, + sprocket_name: 'gears', + sub_widgets: [ + { sprocket_name: 'ten gears' } + ] + }) + end + end + end + end + end + + context "when `ignore_unknown_fields` is false" do + context "when params are supplied to the wrong action" do + let(:valid_params_config) do + { + sprocket_parent_id: { :_config => { type: 'integer', only: [:update] } }, + sprocket_name: { :_config => { type: 'string' } } + } + end + let(:input_params) { { sprocket_parent_id: 5, sprocket_name: 'gears' } } + let(:action) { :create } + + it "throws an unknown params error" do + expect { subject.validate! }.to raise_error(Brainstem::UnknownParams) + end + end + + context "when input params have unknown keys" do + let(:input_params) { { my_cool_param: "something" } } + + it "lists unknown params" do + expect { subject.validate! }.to raise_error(Brainstem::UnknownParams) + end + + context "when recursive attribute has invalid / unknown properties" do + let(:valid_params_config) do + { + sprocket_parent_id: { :_config => { type: 'integer' } }, + sprocket_name: { :_config => { type: 'string' } }, + sub_widget: { :_config => { type: 'hash', recursive: true } }, + sub_widgets: { :_config => { type: 'array', item_type: 'hash', recursive: true } }, + } + end + + context "when recursive attribute is a hash" do + let(:input_params) do + { + sprocket_parent_id: 5, + sprocket_name: 'gears', + sub_widget: { + invalid_id: 15, + sprocket_name: 'ten gears', + } + } + end + + it "raises an error" do + expect { subject.validate! }.to raise_error(Brainstem::UnknownParams) + end + end + + context "when recursive attribute is an array" do + let(:input_params) do + { + sprocket_parent_id: 5, + sprocket_name: 'gears', + sub_widgets: [ + { invalid_id: 15, sprocket_name: 'ten gears' } + ] + } + end + + it "raises an error" do + expect { subject.validate! }.to raise_error(Brainstem::UnknownParams) + end + end + end + end + end +end diff --git a/spec/brainstem/presenter_collection_spec.rb b/spec/brainstem/presenter_collection_spec.rb index a1159c56..12e60078 100644 --- a/spec/brainstem/presenter_collection_spec.rb +++ b/spec/brainstem/presenter_collection_spec.rb @@ -1220,7 +1220,7 @@ class ArrayPresenter < Brainstem::Presenter describe "for! method" do it "raises if there is no presenter for the given class" do - expect{ Brainstem.presenter_collection("v1").for!(String) }.to raise_error(ArgumentError) + expect { Brainstem.presenter_collection("v1").for!(String) }.to raise_error(ArgumentError) end end @@ -1245,7 +1245,7 @@ class AnotherWorkspace < Workspace end it "raises if there is no presenter for the given class" do - expect{ Brainstem.presenter_collection("v1").brainstem_key_for!(String) }.to raise_error(ArgumentError) + expect { Brainstem.presenter_collection("v1").brainstem_key_for!(String) }.to raise_error(ArgumentError) end end end From fe1e4f926ae5a174ebc64b19e231470d9d2ec5aa Mon Sep 17 00:00:00 2001 From: Shirish Pampoorickal Date: Thu, 12 Apr 2018 08:49:27 -0700 Subject: [PATCH 4/8] Allow validating & ignoring unknown params for multi nested params --- lib/brainstem/params_validator.rb | 42 +++++--- spec/brainstem/params_validator_spec.rb | 138 ++++++++++++++++++++++++ 2 files changed, 168 insertions(+), 12 deletions(-) diff --git a/lib/brainstem/params_validator.rb b/lib/brainstem/params_validator.rb index 938ddf8a..081d695e 100644 --- a/lib/brainstem/params_validator.rb +++ b/lib/brainstem/params_validator.rb @@ -20,21 +20,25 @@ def initialize(action_name, input_params, valid_params_config, options = {}) def validate! @input_params.each do |param_key, param_value| - param = @valid_params_config[param_key] + param_data = @valid_params_config[param_key] - if param.blank? + if param_data.blank? @unknown_params << param_key next end - param_config = param[:_config] + param_config = param_data[:_config] if param_config[:only].present? && !param_config[:only].map(&:to_s).include?(@action_name) @unknown_params << param_key elsif param_config[:recursive].to_s == 'true' - next if param_value.blank? + next if param_value.blank? # Doubts - @sanitized_params[param_key] = validate_recursive_params!(param_config[:type], param_key, param_value) + @sanitized_params[param_key] = validate_recursive_params!(param_key, param_data, param_value) + elsif parent_param?(param_data) + # next if param_value.blank? # Doubts + + @sanitized_params[param_key] = validate_nested_params!(param_key, param_data, param_value) else @sanitized_params[param_key] = param_value end @@ -45,19 +49,33 @@ def validate! private - def validate_recursive_params!(param_type, param_key, param_value) - if param_type == 'hash' - validate_recursive_param(param_key, param_value) + def parent_param?(param_data) + param_data.except(:_config).keys.present? + end + + def validate_recursive_params!(parent_param_key, parent_param_config, value) + if parent_param_config[:_config][:type] == 'hash' + validate_nested_param(parent_param_key, value, @valid_params_config) + else + value.map { |value| validate_nested_param(parent_param_key, value, @valid_params_config) } + end + end + + def validate_nested_params!(parent_param_key, parent_param_config, value) + valid_nested_params = parent_param_config.except(:_config) + + if parent_param_config[:_config][:type] == 'hash' + validate_nested_param(parent_param_key, value, valid_nested_params) else - param_value.each_with_index.map { |value, index| validate_recursive_param(param_key, value, index) } + value.map { |value| validate_nested_param(parent_param_key, value, valid_nested_params) } end end - def validate_recursive_param(param_key, param_value, index = nil) + def validate_nested_param(parent_param_key, value, valid_params) begin - result = self.class.validate!(@action_name, param_value, @valid_params_config, @options) + result = self.class.validate!(@action_name, value, valid_params, @options) rescue Brainstem::UnknownParams => e - @unknown_params << { param_key => e.unknown_params } + @unknown_params << { parent_param_key => e.unknown_params } end result diff --git a/spec/brainstem/params_validator_spec.rb b/spec/brainstem/params_validator_spec.rb index 276f171a..d6cfddbe 100644 --- a/spec/brainstem/params_validator_spec.rb +++ b/spec/brainstem/params_validator_spec.rb @@ -112,6 +112,43 @@ end end end + + context "when multi nested attributes are valid" do + let(:valid_params_config) do + { + full_name: { :_config => { type: 'string' } }, + + permissions: { + :_config => { type: 'hash' }, + can_edit: { :_config => { type: 'boolean' } }, + }, + + skills: { + :_config => { type: 'array', item_type: 'hash' }, + name: { :_config => { type: 'string' } }, + category: { + :_config => { type: 'hash' }, + + name: { :_config => { type: 'string' } } + } + } + } + end + let(:input_params) { + { + full_name: 'Buzz Killington', + permissions: { can_edit: true }, + skills: [ + { name: 'Ruby', category: { name: 'Programming' } }, + { name: 'Karate', category: { name: 'Self Defense' } } + ] + } + } + + it "returns the input params" do + expect(subject.validate!).to eq(input_params) + end + end end context "when input parameters are invalid" do @@ -289,6 +326,107 @@ end end end + + context "when multi nested attributes are invalid" do + let(:restrict_to_actions) { [] } + let(:valid_params_config) do + { + full_name: { :_config => { type: 'string' } }, + + permissions: { + :_config => { type: 'hash', only: restrict_to_actions }, + + can_edit: { :_config => { type: 'boolean' } }, + }, + + skills: { + :_config => { type: 'array', item_type: 'hash' }, + + name: { :_config => { type: 'string', only: restrict_to_actions } }, + category: { + :_config => { type: 'hash' }, + + name: { :_config => { type: 'string' } } + } + } + } + end + let(:input_params) { { sprocket_parent_id: 5, sprocket_name: 'gears' } } + + context "when params are supplied to the wrong action" do + let(:action) { :create } + let(:restrict_to_actions) { [:update] } + let(:input_params) { + { + full_name: 'Buzz Killington', + permissions: { can_edit: true }, + skills: [ + { name: 'Ruby', category: { name: 'Programming' } }, + { name: 'Kaarate', category: { name: 'Self Defense' } } + ] + } + } + + it "raises an error" do + expect { subject.validate! }.to raise_error(Brainstem::UnknownParams) + end + end + + context "when params are supplied to the wrong action" do + let(:action) { :create } + let(:restrict_to_actions) { [:update] } + let(:input_params) { + { + full_name: 'Buzz Killington', + permissions: { can_edit: true }, + skills: [ + { name: 'Ruby', category: { name: 'Programming' } }, + { name: 'Kaarate', category: { name: 'Self Defense' } } + ] + } + } + + it "raises an error" do + expect { subject.validate! }.to raise_error(Brainstem::UnknownParams) + end + end + + context "when unknown params are present" do + context "when present in a hash" do + let(:input_params) { + { + full_name: 'Buzz Killington', + permissions: { can_edit: true, invalid: 'blah' }, + skills: [ + { name: 'Ruby', category: { name: 'Programming' } }, + { name: 'Kaarate', category: { name: 'Self Defense' } } + ] + } + } + + it "raises an error" do + expect { subject.validate! }.to raise_error(Brainstem::UnknownParams) + end + end + + context "when present in a hash" do + let(:input_params) { + { + full_name: 'Buzz Killington', + permissions: { can_edit: true }, + skills: [ + { name: 'Ruby', category: { invalid: 'blah' } }, + { name: 'Kaarate', category: { name: 'Self Defense' } } + ] + } + } + + it "raises an error" do + expect { subject.validate! }.to raise_error(Brainstem::UnknownParams) + end + end + end + end end end end From d4670fb3bf08765028736e8237f517da3967c303 Mon Sep 17 00:00:00 2001 From: Shirish Pampoorickal Date: Thu, 12 Apr 2018 15:59:07 -0700 Subject: [PATCH 5/8] Restrict brainstem_validate_params! to only validate rather than returning sanitized params Signed-off-by: Jason Carter --- lib/brainstem/malformed_params.rb | 10 + lib/brainstem/params_validator.rb | 80 +++--- lib/brainstem/validation_error.rb | 12 + spec/brainstem/params_validator_spec.rb | 367 ++++++++++++------------ 4 files changed, 239 insertions(+), 230 deletions(-) create mode 100644 lib/brainstem/malformed_params.rb create mode 100644 lib/brainstem/validation_error.rb diff --git a/lib/brainstem/malformed_params.rb b/lib/brainstem/malformed_params.rb new file mode 100644 index 00000000..ce05e14c --- /dev/null +++ b/lib/brainstem/malformed_params.rb @@ -0,0 +1,10 @@ +module Brainstem + class MalformedParams < StandardError + attr_reader :malformed_params + + def initialize(message = "Malformed Params sighted", malformed_params = []) + @malformed_params = malformed_params + super(message) + end + end +end diff --git a/lib/brainstem/params_validator.rb b/lib/brainstem/params_validator.rb index 081d695e..956c8850 100644 --- a/lib/brainstem/params_validator.rb +++ b/lib/brainstem/params_validator.rb @@ -1,21 +1,22 @@ require 'brainstem/unknown_params' +require 'brainstem/malformed_params' +require 'brainstem/validation_error' module Brainstem class ParamsValidator - attr_reader :unknown_params, :sanitized_params + attr_reader :malformed_params, :unknown_params - def self.validate!(action_name, input_params, valid_params_config, options = {}) - new(action_name, input_params, valid_params_config, options).validate! + def self.validate!(action_name, input_params, valid_params_config) + new(action_name, input_params, valid_params_config).validate! end - def initialize(action_name, input_params, valid_params_config, options = {}) + def initialize(action_name, input_params, valid_params_config) @valid_params_config = valid_params_config - @options = options @input_params = sanitize_input_params!(input_params) @action_name = action_name.to_s @unknown_params = [] - @sanitized_params = {} + @malformed_params = [] end def validate! @@ -27,80 +28,65 @@ def validate! next end - param_config = param_data[:_config] + param_config = param_data[:_config] + nested_valid_params = param_data.except(:_config) + if param_config[:only].present? && !param_config[:only].map(&:to_s).include?(@action_name) @unknown_params << param_key - elsif param_config[:recursive].to_s == 'true' - next if param_value.blank? # Doubts - - @sanitized_params[param_key] = validate_recursive_params!(param_key, param_data, param_value) + validate_nested_params(param_key, param_config, param_value, @valid_params_config) elsif parent_param?(param_data) - # next if param_value.blank? # Doubts - - @sanitized_params[param_key] = validate_nested_params!(param_key, param_data, param_value) - else - @sanitized_params[param_key] = param_value + validate_nested_params(param_key, param_config, param_value, nested_valid_params) end end - raise_with_unknown_params? ? unknown_params_error! : @sanitized_params + raise_when_invalid? ? unknown_params_error! : true end private - def parent_param?(param_data) - param_data.except(:_config).keys.present? + def raise_when_invalid? + @malformed_params.present? || @unknown_params.present? end - def validate_recursive_params!(parent_param_key, parent_param_config, value) - if parent_param_config[:_config][:type] == 'hash' - validate_nested_param(parent_param_key, value, @valid_params_config) - else - value.map { |value| validate_nested_param(parent_param_key, value, @valid_params_config) } - end + def parent_param?(param_data) + param_data.except(:_config).keys.present? end - def validate_nested_params!(parent_param_key, parent_param_config, value) - valid_nested_params = parent_param_config.except(:_config) + def validate_nested_params(param_key, param_config, value, valid_nested_params) + return value if value.nil? - if parent_param_config[:_config][:type] == 'hash' - validate_nested_param(parent_param_key, value, valid_nested_params) + param_type = param_config[:type] + if param_type == 'hash' + validate_nested_param(param_key, param_type, value, valid_nested_params) + elsif param_type == 'array' && !value.is_a?(Array) + @malformed_params << param_key else - value.map { |value| validate_nested_param(parent_param_key, value, valid_nested_params) } + value.each { |value| validate_nested_param(param_key, param_type, value, valid_nested_params) } end end - def validate_nested_param(parent_param_key, value, valid_params) + def validate_nested_param(parent_param_key, parent_param_type, value, valid_nested_params) begin - result = self.class.validate!(@action_name, value, valid_params, @options) - rescue Brainstem::UnknownParams => e + self.class.validate!(@action_name, value, valid_nested_params) + rescue Brainstem::ValidationError => e @unknown_params << { parent_param_key => e.unknown_params } + @malformed_params << { parent_param_key => e.malformed_params } end - - result - end - - def raise_with_unknown_params? - !ignore_unknown_params? && @unknown_params.present? - end - - def ignore_unknown_params? - @options[:ignore_unknown_fields].to_s == 'true' end def sanitize_input_params!(input_params) - missing_params_error! unless input_params.is_a?(Hash) && input_params.present? + malformed_params_error! unless input_params.is_a?(Hash) && input_params.present? input_params end - def missing_params_error! - raise ::Brainstem::UnknownParams.new("Missing required parameters") + def malformed_params_error! + raise ::Brainstem::ValidationError.new("Input params are malformed") end def unknown_params_error! - raise ::Brainstem::UnknownParams.new("Unknown params encountered", @unknown_params) + raise ::Brainstem::ValidationError.new("Invalid params encountered", @unknown_params, @malformed_params) end end end diff --git a/lib/brainstem/validation_error.rb b/lib/brainstem/validation_error.rb new file mode 100644 index 00000000..ce6a0f9d --- /dev/null +++ b/lib/brainstem/validation_error.rb @@ -0,0 +1,12 @@ +module Brainstem + class ValidationError < StandardError + attr_reader :unknown_params, :malformed_params + + def initialize(message = "Invalid params sighted", unknown_params = [], malformed_params = []) + @unknown_params = unknown_params + @malformed_params = malformed_params + + super(message) + end + end +end diff --git a/spec/brainstem/params_validator_spec.rb b/spec/brainstem/params_validator_spec.rb index d6cfddbe..5c4c42fb 100644 --- a/spec/brainstem/params_validator_spec.rb +++ b/spec/brainstem/params_validator_spec.rb @@ -8,16 +8,15 @@ sprocket_name: { :_config => { type: 'string' } } } end - let(:options) { {} } let(:action_name) { :create } - subject { described_class.new(action_name, input_params, valid_params_config, options) } + subject { described_class.new(action_name, input_params, valid_params_config) } context "when input params has valid keys" do let(:input_params) { { sprocket_parent_id: 5, sprocket_name: 'gears' } } - it "returns sanitized params" do - expect(subject.validate!).to eq(input_params) + it "returns true" do + expect(subject.validate!).to be_truthy end context "when recursive attribute is given" do @@ -29,7 +28,7 @@ sub_widgets: { :_config => { type: 'array', item_type: 'hash', recursive: true } }, } end - let(:action) { :create } + let(:action_name) { :create } context "when recursive attribute is a hash" do let(:input_params) do @@ -43,16 +42,8 @@ context "when attribute is nil" do let(:sub_widget_param) { nil } - it "returns the sanitized attributes without the empty recursive param" do - expect(subject.validate!).to eq(input_params.except(:sub_widget)) - end - end - - context "when attribute is an empty hash" do - let(:sub_widget_param) { {} } - - it "returns the sanitized attributes without the empty recursive param" do - expect(subject.validate!).to eq(input_params.except(:sub_widget)) + it "returns true" do + expect(subject.validate!).to be_truthy end end @@ -64,8 +55,8 @@ } end - it "returns the sanitized attributes" do - expect(subject.validate!).to eq(input_params) + it "returns true" do + expect(subject.validate!).to be_truthy end end end @@ -82,32 +73,32 @@ context "when attribute is nil" do let(:sub_widgets_params) { nil } - it "returns the sanitized attributes without the empty recursive param" do - expect(subject.validate!).to eq(input_params.except(:sub_widgets)) + it "returns true" do + expect(subject.validate!).to be_truthy end end context "when attribute is an empty array" do let(:sub_widgets_params) { [] } - it "returns the sanitized attributes without the empty recursive param" do - expect(subject.validate!).to eq(input_params.except(:sub_widgets)) + it "returns true" do + expect(subject.validate!).to be_truthy end end context "when attributes in an array" do let(:sub_widgets_params) { [ { sprocket_parent_id: 15, sprocket_name: 'ten gears' } ] } - it "returns the sanitized attributes" do - expect(subject.validate!).to eq(input_params) + it "returns true" do + expect(subject.validate!).to be_truthy end end skip "(UNSUPPORTED) when attributes in an hash" do let(:sub_widgets_params) { { 0 => { title: "child" } } } - it "returns the sanitized attributes" do - expect(subject.validate!).to eq(input_params) + it "returns true" do + expect(subject.validate!).to be_truthy end end end @@ -134,6 +125,7 @@ } } end + let(:input_params) { { full_name: 'Buzz Killington', @@ -145,8 +137,48 @@ } } - it "returns the input params" do - expect(subject.validate!).to eq(input_params) + it "returns true" do + expect(subject.validate!).to be_truthy + end + + context "when expected param is a hash" do + context "when value is nil" do + let(:input_params) { + { + permissions: nil, + } + } + + it "returns true" do + expect(subject.validate!).to be_truthy + end + end + end + + context "when expected param is an array" do + context "when value is nil" do + let(:input_params) { + { + skills: nil + } + } + + it "returns true" do + expect(subject.validate!).to be_truthy + end + end + + context "when value is an empty array" do + let(:input_params) { + { + skills: [] + } + } + + it "returns true" do + expect(subject.validate!).to be_truthy + end + end end end end @@ -156,7 +188,7 @@ let(:input_params) { {} } it "raises an missing params error" do - expect { subject.validate! }.to raise_error(Brainstem::UnknownParams) + expect { subject.validate! }.to raise_error(Brainstem::ValidationError) end end @@ -164,7 +196,7 @@ let(:input_params) { [{ foo: "bar" }] } it "raises an missing params error" do - expect { subject.validate! }.to raise_error(Brainstem::UnknownParams) + expect { subject.validate! }.to raise_error(Brainstem::ValidationError) end end @@ -172,128 +204,45 @@ let(:input_params) { nil } it "raises an missing params error" do - expect { subject.validate! }.to raise_error(Brainstem::UnknownParams) + expect { subject.validate! }.to raise_error(Brainstem::ValidationError) end end end - context "when `ignore_unknown_fields` is true" do - let(:options) { { ignore_unknown_fields: true } } - - context "when params are supplied to the wrong action" do - let(:valid_params_config) do - { - sprocket_parent_id: { :_config => { type: 'integer', only: [:update] } }, - sprocket_name: { :_config => { type: 'string' } } - } - end - let(:input_params) { { sprocket_parent_id: 5, sprocket_name: 'gears' } } - let(:action) { :create } - - it "returns the input params without the unknown params" do - expect(subject.validate!).to eq(input_params.except(:sprocket_parent_id)) - end + context "when params are supplied to the wrong action" do + let(:valid_params_config) do + { + sprocket_parent_id: { :_config => { type: 'integer', only: [:update] } }, + sprocket_name: { :_config => { type: 'string' } } + } end + let(:input_params) { { sprocket_parent_id: 5, sprocket_name: 'gears' } } + let(:action) { :create } - context "when input params have unknown keys" do - let(:input_params) { { my_cool_param: "something" } } - - it "returns the input params without the unknown params" do - expect(subject.validate!).to eq(input_params.except(:my_cool_param)) - end - - context "when recursive attribute has invalid / unknown values" do - let(:valid_params_config) do - { - sprocket_parent_id: { :_config => { type: 'integer' } }, - sprocket_name: { :_config => { type: 'string' } }, - sub_widget: { :_config => { type: 'hash', recursive: true } }, - sub_widgets: { :_config => { type: 'array', item_type: 'hash', recursive: true } }, - } - end - - context "when recursive attribute is a hash" do - let(:input_params) do - { - sprocket_parent_id: 5, - sprocket_name: 'gears', - sub_widget: { - invalid_id: 15, - sprocket_name: 'ten gears', - } - } - end - - it "returns the input params without the unknown params" do - expect(subject.validate!).to eq({ - sprocket_parent_id: 5, - sprocket_name: 'gears', - sub_widget: { - sprocket_name: 'ten gears', - } - }) - end - end + it "throws an unknown params error" do + expect { subject.validate! }.to raise_error(Brainstem::ValidationError) + end + end - context "when recursive attribute is an array" do - let(:input_params) do - { - sprocket_parent_id: 5, - sprocket_name: 'gears', - sub_widgets: [ - { invalid_id: 15, sprocket_name: 'ten gears' } - ] - } - end + context "when input params have unknown keys" do + let(:input_params) { { my_cool_param: "something" } } - it "returns the input params without the unknown params" do - expect(subject.validate!).to eq({ - sprocket_parent_id: 5, - sprocket_name: 'gears', - sub_widgets: [ - { sprocket_name: 'ten gears' } - ] - }) - end - end - end + it "lists unknown params" do + expect { subject.validate! }.to raise_error(Brainstem::ValidationError) end - end - context "when `ignore_unknown_fields` is false" do - context "when params are supplied to the wrong action" do + context "when recursive attribute has invalid / unknown properties" do let(:valid_params_config) do { - sprocket_parent_id: { :_config => { type: 'integer', only: [:update] } }, - sprocket_name: { :_config => { type: 'string' } } + sprocket_parent_id: { :_config => { type: 'integer' } }, + sprocket_name: { :_config => { type: 'string' } }, + sub_widget: { :_config => { type: 'hash', recursive: true } }, + sub_widgets: { :_config => { type: 'array', item_type: 'hash', recursive: true } }, } end - let(:input_params) { { sprocket_parent_id: 5, sprocket_name: 'gears' } } - let(:action) { :create } - - it "throws an unknown params error" do - expect { subject.validate! }.to raise_error(Brainstem::UnknownParams) - end - end - - context "when input params have unknown keys" do - let(:input_params) { { my_cool_param: "something" } } - - it "lists unknown params" do - expect { subject.validate! }.to raise_error(Brainstem::UnknownParams) - end - - context "when recursive attribute has invalid / unknown properties" do - let(:valid_params_config) do - { - sprocket_parent_id: { :_config => { type: 'integer' } }, - sprocket_name: { :_config => { type: 'string' } }, - sub_widget: { :_config => { type: 'hash', recursive: true } }, - sub_widgets: { :_config => { type: 'array', item_type: 'hash', recursive: true } }, - } - end - context "when recursive attribute is a hash" do + context "when recursive attribute is a hash" do + context "when recursive attribute has invalid keys" do let(:input_params) do { sprocket_parent_id: 5, @@ -306,60 +255,93 @@ end it "raises an error" do - expect { subject.validate! }.to raise_error(Brainstem::UnknownParams) + expect { subject.validate! }.to raise_error(Brainstem::ValidationError) end end - context "when recursive attribute is an array" do + context "when attribute is an empty hash" do let(:input_params) do { sprocket_parent_id: 5, sprocket_name: 'gears', - sub_widgets: [ - { invalid_id: 15, sprocket_name: 'ten gears' } - ] + sub_widget: {} } end it "raises an error" do - expect { subject.validate! }.to raise_error(Brainstem::UnknownParams) + expect { subject.validate! }.to raise_error(Brainstem::ValidationError) end end end - context "when multi nested attributes are invalid" do - let(:restrict_to_actions) { [] } - let(:valid_params_config) do + context "when recursive attribute is an array" do + let(:input_params) do { - full_name: { :_config => { type: 'string' } }, + sprocket_parent_id: 5, + sprocket_name: 'gears', + sub_widgets: [ + { invalid_id: 15, sprocket_name: 'ten gears' } + ] + } + end + + it "raises an error" do + expect { subject.validate! }.to raise_error(Brainstem::ValidationError) + end + end + end - permissions: { - :_config => { type: 'hash', only: restrict_to_actions }, + context "when multi nested attributes are invalid" do + let(:restrict_to_actions) { [] } + let(:valid_params_config) do + { + full_name: { :_config => { type: 'string' } }, - can_edit: { :_config => { type: 'boolean' } }, - }, + permissions: { + :_config => { type: 'hash', only: restrict_to_actions }, - skills: { - :_config => { type: 'array', item_type: 'hash' }, + can_edit: { :_config => { type: 'boolean' } }, + }, - name: { :_config => { type: 'string', only: restrict_to_actions } }, - category: { - :_config => { type: 'hash' }, + skills: { + :_config => { type: 'array', item_type: 'hash' }, - name: { :_config => { type: 'string' } } - } + name: { :_config => { type: 'string', only: restrict_to_actions } }, + category: { + :_config => { type: 'hash' }, + + name: { :_config => { type: 'string' } } } } + } + end + let(:input_params) { { sprocket_parent_id: 5, sprocket_name: 'gears' } } + + context "when params are supplied to the wrong action" do + let(:action) { :create } + let(:restrict_to_actions) { [:update] } + let(:input_params) { + { + full_name: 'Buzz Killington', + permissions: { can_edit: true }, + skills: [ + { name: 'Ruby', category: { name: 'Programming' } }, + { name: 'Kaarate', category: { name: 'Self Defense' } } + ] + } + } + + it "raises an error" do + expect { subject.validate! }.to raise_error(Brainstem::ValidationError) end - let(:input_params) { { sprocket_parent_id: 5, sprocket_name: 'gears' } } + end - context "when params are supplied to the wrong action" do - let(:action) { :create } - let(:restrict_to_actions) { [:update] } + context "when unknown params are present" do + context "when present in a hash" do let(:input_params) { { full_name: 'Buzz Killington', - permissions: { can_edit: true }, + permissions: { can_edit: true, invalid: 'blah' }, skills: [ { name: 'Ruby', category: { name: 'Programming' } }, { name: 'Kaarate', category: { name: 'Self Defense' } } @@ -368,61 +350,80 @@ } it "raises an error" do - expect { subject.validate! }.to raise_error(Brainstem::UnknownParams) + expect { subject.validate! }.to raise_error(Brainstem::ValidationError) end end - context "when params are supplied to the wrong action" do - let(:action) { :create } - let(:restrict_to_actions) { [:update] } + context "when present in an array" do let(:input_params) { { full_name: 'Buzz Killington', permissions: { can_edit: true }, skills: [ - { name: 'Ruby', category: { name: 'Programming' } }, + { name: 'Ruby', category: { invalid: 'blah' } }, { name: 'Kaarate', category: { name: 'Self Defense' } } ] } } it "raises an error" do - expect { subject.validate! }.to raise_error(Brainstem::UnknownParams) + expect { subject.validate! }.to raise_error(Brainstem::ValidationError) end end + end - context "when unknown params are present" do - context "when present in a hash" do + context "when params are malformed" do + context "when expected to be an array" do + context "when given an array with an empty hash" do let(:input_params) { { - full_name: 'Buzz Killington', - permissions: { can_edit: true, invalid: 'blah' }, skills: [ - { name: 'Ruby', category: { name: 'Programming' } }, - { name: 'Kaarate', category: { name: 'Self Defense' } } + { name: 'Ruby', category: { invalid: 'blah' } }, + {}, ] } } it "raises an error" do - expect { subject.validate! }.to raise_error(Brainstem::UnknownParams) + expect { subject.validate! }.to raise_error(Brainstem::ValidationError) end end - context "when present in a hash" do + context "when given a hash" do let(:input_params) { { - full_name: 'Buzz Killington', - permissions: { can_edit: true }, - skills: [ - { name: 'Ruby', category: { invalid: 'blah' } }, - { name: 'Kaarate', category: { name: 'Self Defense' } } - ] + skills: {}, + } + } + + it "raises an error" do + expect { subject.validate! }.to raise_error(Brainstem::ValidationError) + end + end + end + + context "when expected to be a hash" do + context "when given an array" do + let(:input_params) { + { + permissions: [ { can_edit: true } ] + } + } + + it "raises an error" do + expect { subject.validate! }.to raise_error(Brainstem::ValidationError) + end + end + + context "when given an empty hash" do + let(:input_params) { + { + permissions: {}, } } it "raises an error" do - expect { subject.validate! }.to raise_error(Brainstem::UnknownParams) + expect { subject.validate! }.to raise_error(Brainstem::ValidationError) end end end From 334a1b491f523ea7167665dab0e4e4d6e6c82971 Mon Sep 17 00:00:00 2001 From: Shirish Pampoorickal Date: Thu, 12 Apr 2018 16:26:47 -0700 Subject: [PATCH 6/8] Remove 'brainstem_ignore_unknown_params' implementation --- lib/brainstem/concerns/controller_dsl.rb | 23 ------ .../brainstem/concerns/controller_dsl_spec.rb | 78 +------------------ 2 files changed, 4 insertions(+), 97 deletions(-) diff --git a/lib/brainstem/concerns/controller_dsl.rb b/lib/brainstem/concerns/controller_dsl.rb index 4d139ec5..6b517ac2 100644 --- a/lib/brainstem/concerns/controller_dsl.rb +++ b/lib/brainstem/concerns/controller_dsl.rb @@ -402,29 +402,6 @@ def brainstem_validate_params!(requested_context = action_name.to_sym, root_para ).present? end - # - # Ensures that known / documented parameters are passed through to the action. - # - # It raises Brainstem::UnknownParams.new(message, unknown_params) error, - # when params are empty or not a Hash. - # - # @params [String, Symbol] (Optional) requested_context the context which to look up. - # @params [String, Symbol] (Optional) root_param_name the param name of the model being changed. - # - # @return [Hash{String => String, Hash] a hash of pairs of param names and descriptions or sub-hashes. - # - def brainstem_ignore_unknown_params!(requested_context = action_name.to_sym, root_param_name = brainstem_model_name) - input_params = params.with_indifferent_access[brainstem_model_name] - brainstem_params_config = brainstem_valid_params(requested_context, root_param_name) - - Brainstem::ParamsValidator.validate!( - requested_context, - input_params, - brainstem_params_config, - ignore_unknown_fields: true - ) - end - # # Lists all incoming param keys that will be rewritten to use a different # name for internal usage for the current action. diff --git a/spec/brainstem/concerns/controller_dsl_spec.rb b/spec/brainstem/concerns/controller_dsl_spec.rb index 55fe59e8..fa1b2935 100644 --- a/spec/brainstem/concerns/controller_dsl_spec.rb +++ b/spec/brainstem/concerns/controller_dsl_spec.rb @@ -1017,7 +1017,7 @@ module Concerns it "returns false and says params are missing" do expect { subject.new.brainstem_validate_params!(:update, brainstem_model_name) - }.to raise_error(Brainstem::UnknownParams) + }.to raise_error(Brainstem::ValidationError) end end @@ -1027,7 +1027,7 @@ module Concerns it "returns false and says params are missing" do expect { subject.new.brainstem_validate_params!(:update, brainstem_model_name) - }.to raise_error(Brainstem::UnknownParams) + }.to raise_error(Brainstem::ValidationError) end end @@ -1037,7 +1037,7 @@ module Concerns it "returns false and says params are missing" do expect { subject.new.brainstem_validate_params!(:update, brainstem_model_name) - }.to raise_error(Brainstem::UnknownParams) + }.to raise_error(Brainstem::ValidationError) end end end @@ -1048,77 +1048,7 @@ module Concerns it "lists unknown params" do expect { subject.new.brainstem_validate_params!(:update, brainstem_model_name) - }.to raise_error(Brainstem::UnknownParams, "Unknown params encountered") - end - end - end - - describe "#brainstem_ignore_unknown_params!" do - let(:brainstem_model_name) { "widget" } - let(:input_params) { { widget: { sprocket_parent_id: 5, sprocket_name: 'gears' } } } - - before do - stub(subject).brainstem_model_name { brainstem_model_name } - stub.any_instance_of(subject).brainstem_model_name { brainstem_model_name } - stub.any_instance_of(subject).params { input_params } - - subject.brainstem_params do - actions :update do - model_params(brainstem_model_name) do |params| - params.valid :sprocket_parent_id, :long, - info: "sprockets[sprocket_parent_id] is not required" - - params.valid :sprocket_name, :string, - info: "sprockets[sprocket_name] is required", - required: true - end - end - end - end - - it "returns sanitized params if params are OK" do - expect(subject.new.brainstem_ignore_unknown_params!(:update, brainstem_model_name)). - to eq(input_params[:widget].with_indifferent_access) - end - - context "when parameters are in an invalid format" do - context "with an empty hash" do - let(:input_params) { {} } - - it "returns the empty hash" do - expect { - subject.new.brainstem_ignore_unknown_params!(:update, brainstem_model_name) - }.to raise_error(Brainstem::UnknownParams) - end - end - - context "with a non-hash object" do - let(:input_params) { { widget: [{ foo: "bar" }] } } - - it "returns the object" do - expect { - subject.new.brainstem_ignore_unknown_params!(:update, brainstem_model_name) - }.to raise_error(Brainstem::UnknownParams) - end - end - - context "with nil" do - let(:input_params) { { widget: nil } } - - it "returns false and says params are missing" do - expect { - subject.new.brainstem_validate_params!(:update, brainstem_model_name) - }.to raise_error(Brainstem::UnknownParams) - end - end - end - - context "when unknown params are present" do - let(:input_params) { { widget: { sprocket_parent_id: 5, sprocket_name: 'gears', my_cool_param: "something" } } } - - it "returns the params without the unknown keys" do - expect(subject.new.brainstem_ignore_unknown_params!(:update, brainstem_model_name)) - .to eq({ sprocket_parent_id: 5, sprocket_name: 'gears' }.with_indifferent_access) + }.to raise_error(Brainstem::ValidationError) end end end From f62bc4dfef75254f5278838df857806ca71defc5 Mon Sep 17 00:00:00 2001 From: Shirish Pampoorickal Date: Thu, 12 Apr 2018 16:58:23 -0700 Subject: [PATCH 7/8] Update comments and return value for brainstem_validate_params --- lib/brainstem/concerns/controller_dsl.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/brainstem/concerns/controller_dsl.rb b/lib/brainstem/concerns/controller_dsl.rb index 6b517ac2..fefa56ad 100644 --- a/lib/brainstem/concerns/controller_dsl.rb +++ b/lib/brainstem/concerns/controller_dsl.rb @@ -385,7 +385,7 @@ def brainstem_valid_params(requested_context = action_name.to_sym, root_param_na # # Ensures that the parameters passed through to the action are valid. # - # It raises Brainstem::UnknownParams.new(message, unknown_params) error, + # It raises Brainstem::ValidatorError.new(message, unknown_params, malformed_params) error, # when params are missing or unknown params are encountered # # @params [String, Symbol] (Optional) requested_context the context which to look up. @@ -395,11 +395,7 @@ def brainstem_validate_params!(requested_context = action_name.to_sym, root_para input_params = params.with_indifferent_access[brainstem_model_name] brainstem_params_config = brainstem_valid_params(requested_context, root_param_name) - Brainstem::ParamsValidator.validate!( - requested_context, - input_params, - brainstem_params_config - ).present? + Brainstem::ParamsValidator.validate!(requested_context, input_params, brainstem_params_config) end # From 31ed584c09981141233945f125bda81f3698c520 Mon Sep 17 00:00:00 2001 From: Shirish Date: Wed, 23 May 2018 16:27:01 -0700 Subject: [PATCH 8/8] Update changelog to add new validation functionality `brainstem_validate_params!` --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 38def317..6219703f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog ++ **1.5.0** - _05/23/2018_ + - Add the capability to validate params based on documented parameters on endpoints. Invoking + `brainstem_validate_params!` raises an error if unknown parameters are encountered. + + **1.4.1** - _05/09/2018_ - Add the capability to specify an alternate base application / engine the routes are derived from. This capability is specific to documemtation generation.