User.find(3).restrict!(current_user) is giving unrestricted acces to User

[toxix]

User.restrict!(current_user).find(3) is behalving like I'm expecting it to work. But the other way arround User.find(3).restrict!(current_user) doesn't do it's job. The not working way is also in the documentation

@dummy     # => Dummy.find(params[:id]).restrict!(current_user)

so I'm assuming this is a bug.

Way to reproduce

app/models/user.rb

class User < ActiveRecord::Base

protect do |user, u|
  if user.admin?
    #protect_admin
    protector_crud
  else
    scope { where(id: user.id)}
    can :read, %w(name admin)
  end

  if user && u.try(:id) == user.id 
    #debugger
    can :update
  end
end
end

Loading development environment (Rails 4.0.0)

irb(main):001:0> User.all
D, [2013-12-07T19:36:07.254912 #846] DEBUG -- :   User Load (20.4ms)  SELECT "users".* FROM "users"
=> #<ActiveRecord::Relation [#<User id: 1, name: "test", admin: false, created_at: "2013-10-26 18:00:17", updated_at: "2013-10-30 16:37:06">, #<User id: 3, name: "admin user2", admin: true, created_at: "2013-10-28 13:57:47", updated_at: "2013-10-28 13:57:47">, #<User id: 4, name: "test", admin: false, created_at: "2013-10-28 15:14:05", updated_at: "2013-10-28 15:14:05">]>

irb(main):003:0> current_user = User.first
D, [2013-12-07T19:36:47.850368 #846] DEBUG -- :   User Load (0.5ms)  SELECT "users".* FROM "users" ORDER BY "users"."id" ASC LIMIT 1
=> #<User id: 1, name: "test", admin: false, created_at: "2013-10-26 18:00:17", updated_at: "2013-10-30 16:37:06">

irb(main):004:0> User.find(3)
D, [2013-12-07T19:37:07.473280 #846] DEBUG -- :   User Load (7.3ms)  SELECT "users".* FROM "users" WHERE "users"."id" = ? LIMIT 1  [["id", 3]]
=> #<User id: 3, name: "admin user2", admin: true, created_at: "2013-10-28 13:57:47", updated_at: "2013-10-28 13:57:47">

irb(main):005:0> User.find(3).restrict!(current_user)
D, [2013-12-07T19:37:28.472239 #846] DEBUG -- :   User Load (0.1ms)  SELECT "users".* FROM "users" WHERE "users"."id" = ? LIMIT 1  [["id", 3]]
=> #<User id: 3, name: "admin user2", admin: true, created_at: "2013-10-28 13:57:47", updated_at: "2013-10-28 13:57:47">

irb(main):006:0> User.restrict!(current_user).find(3)
D, [2013-12-07T19:37:43.048029 #846] DEBUG -- :   User Load (0.5ms)  SELECT "users".* FROM "users" WHERE "users"."id" = ? AND "users"."id" = 1 LIMIT 1  [["id", 3]]
ActiveRecord::RecordNotFound: Couldn't find User with id=3

used the current github Version of protector

[inossidabile]

Nop, it's not a bug. You simply misunderstand things a little bit.

Protector works on 2 levels: during retrieval and after that on the level of fields. Let's carefully look at what you are doing here: User.find(3).restrict!(current_user) – as a first step you tell AR to load your model without any restrictions. And then you restrict the result. Protector can not change past and you explicitly tell system to find a model without any protection with such call.

Even more – it's sometimes a case when you actually want to retrieve something you normally can't and then apply fields restrictions. So it works perfectly right here.

The sample from documentation you provide is out of context. Originally it shows you how to protect an instance of model comparing it to scope protection. So it's also correct.

Anyway if you have any ideas how to make documentation more explicit about this – pull requests (or just separate issues) are welcome.

[toxix]

Okay, think I get it.

The scope is only used when selecting from the database. After the database results are cached, the can :read, %w(name admin) is evaluated unaffected by scope { where(id: user.id)} ). This grant excess in the following call sequence:

irb(main):002:0> u = User.find(3).restrict!(current_user)
D, [2013-12-11T01:32:01.425081 #15107] DEBUG -- :   User Load (49.3ms)  SELECT "users".* FROM "users" WHERE "users"."id" = ? LIMIT 1  [["id", 3]]
=> #<User id: 3, name: "admin user2", admin: true, created_at: "2013-10-28 13:57:47", updated_at: "2013-10-28 13:57:47">

irb(main):003:0> u.name
=> "admin user2"

I expected the scope would be evaluated in this case too and the u.name fail because it is not in the scope. Maybe a hint in the part where scopes are invented would be nice.

Let me know if I get it the right way, so I'm able to improve the documentation.

[inossidabile]

After the database results are cached

Not cached. Just fetched. User.find(3) doesn't cache anything. It fetches data from database. Other than that it's quite close, yes.

[toxix]

hopefully did my first pull request the right way… If not just give me a hint.