Nil subject

[fdeschenes]

According to one of your examples (the first one under Basics), the "subject" can be nil but in reality, that's not the case because an exception is thrown in https://github.com/inossidabile/protector/blob/master/lib/protector/dsl.rb#L210.

I can think of many reasons why the subject could be nil and was curious as to whether or not throwing an exception there was intentional or not.

[inossidabile]

It's wrong behaviour and should be fixed. I've added this raise at the end to protect people from running queries without having any subject set. But you are right, nil is a valuable subject and it can be part of security scope for sure (current_user is the first sample): restrict!(nil) is not equal to not calling restrict! at all. I would like to keep it safe though. Give me 3-4 hours, I will make something about it.

[inossidabile]

This is amazing how deep this issue ingrained to the ground of Protector. 0.3.0.beta.1 released. Please let me know if it works well for you so I could drop beta suffix.

[inossidabile]

Doh. Had to yank and release 0.3.0.beta.2 because of the typo :scream:

[fdeschenes]

Thank you! That's exactly why I reported the issue rather than attempted to fix it myself – you know the code much better than I do and it looked like quite a bit would have had to be changed.

I'll put it through it's paces today and let you know if I come across any issues.

[fdeschenes]

That seems to be working quite well at first glance. I'll do some further testing over the next few days and will let you know if anything comes up. Thanks again!

[inossidabile]

Alright. I'll let it hang as beta for a while then :)

[fdeschenes]

I did find an issue which may be unrelated to this but if I protect an association and try to make sure that it belongs to the subject for destroy or update for instance, it seems to cause an endless loop. I'll try and document it this evening but here's a sample (from memory) in case you want to try and tackle this earlier:

class Message < ActiveRecord::Base
  protect do |user, message|
    can :view

    # ...

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

class User < ActiveRecord::Base
  protect do |subject, user|
    can :view
    can :create

    # ...
  end
end

Then if I use Message.restrict!(current_user) when current_user is not nil, it endlessly loops. I'll document this and report later – unless you fix it before then.

[inossidabile]

I see. We should probably unrestrict entities prior to passing em in block. Could you please set a separate issue? Will fix that tomorrow.

[fdeschenes]

I most certainly can. I'll submit what I have documented so far including a trace later this evening (6-8 hour from now).