inossidabile / protector

Comfortable (seriously) white-list security restrictions for models on a field level
MIT License
270 stars 31 forks source link

SimpleForm #11

Closed fdeschenes closed 11 years ago

fdeschenes commented 11 years ago

I use SimpleForm and I noticed that when I use f.association without specifically passing a collection, it creates an unprotected object and thus bypasses Protector.

While this isn't a Protector specific issue, I was wondering if you see any value in adding this monkey patch to the gem, to a How To wiki page, or perhaps even the README file.

module SimpleForm
  class FormBuilder < ActionView::Helpers::FormBuilder
    alias_method :association_without_monkey_patch, :association
    def association(association, options={}, &block)
      options = options.dup

      # Always protect unless otherwise specified.
      options[:protect] = true unless options[:protect] == false

      if options[:protect] == true && @object.respond_to?(:protector_subject?) && @object.protector_subject?
        return simple_fields_for(*[association, options.delete(:collection), options].compact, &block) if block_given?

        reflection = find_association_reflection(association)
        raise "Association #{association.inspect} not found" unless reflection

        options[:collection] ||= options.fetch(:collection) do
          # Protect using the same subject as the the current object.
          reflection.klass.restrict!(@object.protector_subject).where(reflection.options[:conditions]).order(reflection.options[:order]).to_a
        end
      end

      association_without_monkey_patch association, options, &block
    end
  end
end
inossidabile commented 11 years ago

Mhm... I'm currently investigating if I can make AR propagate restriction through reflect_on_association call. Then we wouldn't need monkeypatching at all.

inossidabile commented 11 years ago

On the other side I should not. It's unclear from the start but SimpleForm behaves correctly. Yet it's not what user expect to happen. The problem with this fix is that it depends on gems loading order. I think the best thing we can do is to put this patch into something like protector-simple_form and reference from README (or Wiki). The gem will depend on both, protector and simple_form and therefore will be loaded correctly.

Can you make such a gem?

fdeschenes commented 11 years ago

Good point about the order the gems are loaded in – that didn't cross my mind at all! As for propagating restrictions through reflect_on_association, I think you're right that you shouldn't.

I'll make a gem this evening and let you know as soon as it's up so you can add a link to the README.

inossidabile commented 11 years ago

Alright! Thank you! :+1:

fdeschenes commented 11 years ago

Done and done! It's at https://github.com/deversus/protector-simple_form.