Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Whitelist keys #66

Closed
ineu opened this issue Feb 16, 2016 · 24 comments
Closed

Whitelist keys #66

ineu opened this issue Feb 16, 2016 · 24 comments
Labels

Comments

@ineu
Copy link

ineu commented Feb 16, 2016

Is it possible to create a white list of keys for a hash? I'm using DryValidation instead of StrongParameters and it's sometimes fine, but in some other places I have to ensure that only whitelisted attributes are allowed to be passed. Basically that's what params[:foo].permit(:bar, :baz) does.

@kwando
Copy link

kwando commented Feb 16, 2016

Dry::Data['hash'] is your friend.

https://github.com/dryrb/dry-types#hashes

@ineu
Copy link
Author

ineu commented Feb 16, 2016

Sorry, I don't really understand how it is related to whitelisted attributes. I don't want to enforce key presence, I want to make sure that no other keys but allowed can be passed.

@kwando
Copy link

kwando commented Feb 16, 2016

It is not forcing you to have certain keys.

white_lister = Dry::Data['hash'].schema(bar: 'string', baz: 'string')
white_lister.call(params) # => only "white listed keys"

Using Dry::Validation::Schema

result = my_form_validation_schema.call(params)
if result.errors.empty?
  result.params # => only contains keys specified in the schema, i.e "white listed"
end

Note that you can have optional keys in a validation schema too.

@ineu
Copy link
Author

ineu commented Feb 16, 2016

That's exactly what I needed, thank you.
Do I have to use Dry::Validation::Schema::Form for this to work? As of dry-validation 0.6.0 Schema::Form seems to be broken:

[1] pry(main)> UpdateProfileSchema.new.call('foo' => 123).params
NoMethodError: undefined method `each_with_object' for nil:NilClass
from /home/ineu/.rbenv/versions/2.3.0/lib/ruby/gems/2.3.0/gems/dry-data-0.5.1/lib/dry/data/type/hash.rb:51:in `schema'

where UpdateProfileSchema is empty:

class UpdateProfileSchema < Dry::Validation::Schema::Form
end

@waiting-for-dev
Copy link
Member

Hey,

I don't see this happening with params method (which, by the way, has been renamed to output in master):

require 'dry-validation'

class Schema < Dry::Validation::Schema
  key(:email) { |email| email.filled? }

  key(:age) do |age|
    age.int? & age.gt?(18)
  end
end

schema = Schema.new

schema.call(email: 'jane@doe.org', age: 19, other: 'aha!').params
# => {:email=>"jane@doe.org", :age=>19, :other=>"aha!"}

@solnic
Copy link
Member

solnic commented Mar 2, 2016

I'd consider this as a bug. schemas should only output defined keys. I'll fix that in next release.

@solnic solnic added the bug label Mar 2, 2016
@kwando
Copy link

kwando commented Mar 2, 2016

@waiting-for-dev yeah, currently you need to check the result for errors first.

@waiting-for-dev
Copy link
Member

Ok, thanks you both for the quick feedback

@solnic
Copy link
Member

solnic commented Mar 2, 2016

BTW I'm planning to experiment with a setup where separate schemas are used for pre-filtering input and then actual validation schemas can be inferred from these in order to apply additional business validation rules. This way we could have pure application validation schemas with business rules and a nice sanitization layer in front of it. The benefit would be a really good separation of concerns where input is sanitized and properly coerced so that we don't have to worry about our business validation process to crash on some stupid type-related issues. Currently my thinking is that handling both in one place is too much complexity and it mixes too many concerns (ie type coercions vs applying a business rule to a coerced value).

@waiting-for-dev
Copy link
Member

And do you think this should be in dry-validation library or in a new one? I'm not taking side, yet, but I'm thinking whether sanitizing and validating business logic rules are too different responsibilities or not.

I have these doubts because, in my current situation, I need to perform one sanitation or another depending on the user, because not all users are allowed to update the same set of attributes.

In this scenario and with current dry-validation approach, I would need different schemes for each situation and I should perform the appropriate schema selection in a controller or service layer. I'm thinking whether this is too overkilling or it may exists a different kind of schemes (or whatever name) which would take the user as an argument... Something like:

# Everybody can update `quantity` attribute, but only users with `amount_updater` role can update `amount` attribute
class Schema < Dry::SanitationSchema
  key(:amount) { |amount, user| user.has_role?(:amount_updater) & amount.str? }
  key(:quantity) { |quantity| quantity.float? }
end

params = Request.give_me_the_params
user = User.give_me_an_user
schema = Schema.new
schema.call(params, user)

On the other side, different schemes would be better for very complex scenarios (but anyway you would be able to do that).

This could be a complete replacement (and improvement) for strong-parameters, without the need of carrying all business logic features along if people don't want to use them.

@solnic
Copy link
Member

solnic commented Mar 7, 2016

@waiting-for-dev I don't think you you would need separate schemas for each scenario. You just need a context-aware schema where your user is the context. So we can make something like that possible:

class Schema < Dry::Validation::Schema
  key(:amount).maybe

  rule(:amount_valid) do
    account_updater? & value(:amount).filled?
  end

  def account_updater?
    user.has_role?(:amount_updater)
  end
end

schema.with(user: user).call(params)

@solnic
Copy link
Member

solnic commented Mar 7, 2016

@waiting-for-dev oh and no, there's no need for a new library, I'm just trying to figure out how exactly I want to use this one :)

@blelump
Copy link

blelump commented Mar 7, 2016

@solnic , running validation against certain context (with(x: :y)) definitely sounds like a good idea 👍

@waiting-for-dev
Copy link
Member

Oh, great, I didn't know about the with method :D My fault. Thank you!!

@solnic
Copy link
Member

solnic commented Mar 7, 2016

@waiting-for-dev uhm, I wrote that we can make it possible, it's not implemented yet :)

@kwando
Copy link

kwando commented Mar 7, 2016

Damn, I was already excited by that =P

@solnic
Copy link
Member

solnic commented Mar 7, 2016

I'll add it tomorrow

@kwando
Copy link

kwando commented Mar 7, 2016

❤️

@solnic
Copy link
Member

solnic commented Mar 9, 2016

@waiting-for-dev @kwando Schema#with is in master, see spec

@solnic
Copy link
Member

solnic commented Mar 9, 2016

Anyhow, I'm now wondering if Schema should remove undefined keys or not. Form does do that because it uses hash type from dry-types to pre-process the input. I feel like this should actually be optional for "plain" schemas. I don't think that schemas must absolutely always sanitize the input...any thoughts?

@solnic solnic added discussion and removed bug labels Mar 9, 2016
@waiting-for-dev
Copy link
Member

Hey! Many, many thanks @solnic :) Now I'm out for three weeks without the computer, but when I come back I'll try it.

About your comment, to be honest I'm not sure. Looking for a way to sanitize has been my first approach to dry-validation in a project, so I have no experience in other use cases. But surely it makes more sense to make it optional, or you'll end up with another issue asking for the opposite :)

@solnic
Copy link
Member

solnic commented Mar 11, 2016

This is now done. You can configure schema's input processor, ie:

Dry::Validation.Schema do
  configure do
    config.input_processor = :sanitizer
  end

  key(:email).required

  key(:age).maybe(:int?, gt?: 18)

  key(:address).schema do
    key(:city).required
    key(:street).required
  end
end

result = schema.(
  email: 'jane@doe',
  age: 19,
  such: 'key',
  address: { city: 'NYC', street: 'Street', wow: 'bad' }
)

result.output
# {  email: 'jane@doe', age: 19, address: { city: 'NYC', street: 'Street' }

This comes with a bonus - a sanitization input processor holds information about the schema structure and types. It's using dry-types' hash with a safe constructor (which provides sanitization), and you have type definitions that you can use for some introspection. In example:

schema = Dry::Validation.Schema do
  configure { config.input_processor = :sanitizer }

  key(:email).required(:str?)
  key(:admin).required(:bool?)
  key(:age).maybe(:int?)
end

types = schema.input_processor.type.options[:schema]

puts types[:email]
# #<Dry::Types::Definition primitive=String options={}>

puts types[:admin]
# #<Dry::Types::Sum:0x007fe86b53c270 @left=#<Dry::Types::Definition primitive=TrueClass options={}>, @right=#<Dry::Types::Definition primitive=FalseClass options={}>>

puts types[:age]
# #<Dry::Types::Sum:0x007fe86b9da250 @left=#<Dry::Types::Definition primitive=NilClass options={}>, @right=#<Dry::Types::Definition primitive=Integer options={}>>

@waiting-for-dev
Copy link
Member

Hey, that's great! Thank you very much for your effort on this :)

@ineu
Copy link
Author

ineu commented Mar 11, 2016

That's great. Thank you, @solnic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants