inossidabile / protector

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

Endless loop when checking associations in DSL #7

Closed fdeschenes closed 11 years ago

fdeschenes commented 11 years ago

There seems to be something wrong with the way it handles associations. In the example below, the check for message.user_id == user.id causes an endless loop.

class Message < ActiveRecord::Base
  belongs_to :user

  protect do |user, message|
    can :view

    # ...

    if message.try(:user_id) == user.try(:id)
      can :destroy
    end
  end
end

class User < ActiveRecord::Base
  has_many :messages

  protect do |subject, user|
    can :view
    can :create

    # ...
  end
end

Here's a trace of the loop (this is repeated several times) because the server returns a 500.

/Users/fdeschenes/.rvm/gems/ruby-2.0.0-p195/gems/protector-0.3.0.beta.2/lib/protector/dsl.rb:25:in `instance_exec'
/Users/fdeschenes/.rvm/gems/ruby-2.0.0-p195/gems/protector-0.3.0.beta.2/lib/protector/dsl.rb:25:in `block in initialize'
/Users/fdeschenes/.rvm/gems/ruby-2.0.0-p195/gems/protector-0.3.0.beta.2/lib/protector/dsl.rb:22:in `each'
/Users/fdeschenes/.rvm/gems/ruby-2.0.0-p195/gems/protector-0.3.0.beta.2/lib/protector/dsl.rb:22:in `initialize'
/Users/fdeschenes/.rvm/gems/ruby-2.0.0-p195/gems/protector-0.3.0.beta.2/lib/protector/dsl.rb:211:in `new'
/Users/fdeschenes/.rvm/gems/ruby-2.0.0-p195/gems/protector-0.3.0.beta.2/lib/protector/dsl.rb:211:in `evaluate'
/Users/fdeschenes/.rvm/gems/ruby-2.0.0-p195/gems/protector-0.3.0.beta.2/lib/protector/adapters/active_record/base.rb:82:in `protector_meta'
/Users/fdeschenes/.rvm/gems/ruby-2.0.0-p195/gems/protector-0.3.0.beta.2/lib/protector/adapters/active_record/base.rb:69:in `user_id'
/Users/fdeschenes/.rvm/gems/ruby-2.0.0-p195/gems/activesupport-4.0.0/lib/active_support/core_ext/object/try.rb:45:in `public_send'
/Users/fdeschenes/.rvm/gems/ruby-2.0.0-p195/gems/activesupport-4.0.0/lib/active_support/core_ext/object/try.rb:45:in `try'

I've temporarily resolved this locally changing the Protector::DSL::Meta#initialize method to unrestrict the "entry" and then restrict it again. Here's my solution:

if entry.try(:protector_subject?)
  # Capture the original protector subject
  protector_subject = entry.protector_subject

  # Call instance_exec but unrestricted the entry
  instance_exec subject, entry.unrestrict!, &b

  # Restrict it again using the old subject.
  entry.restrict!(protector_subject)
else
  instance_exec subject, entry, &b
end

I'm not sure if this is the best solution but it's definitely fixed the issue. Let me know if you want me to commit my change or if you'd rather fix it yourself.

inossidabile commented 11 years ago

image

Subject can be restricted too as well. So I've decided to solve it cardinally. I've added special wrapper: Protector.insecurely. It works this way:

Protector.insecurely do
  # whatever you do here, even new restrictions, your entities will stay unprotected
end

So meta block evaluation is now wrapped in such thing. Should work, check it out.