inossidabile / protector

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

restrict! on association #34

Closed pavel-rosputko closed 10 years ago

pavel-rosputko commented 10 years ago

I expect that Account.first.projects.restrict!(user).create!(name: 'test') should throw Access denied to 'name' when name is not whitelisted but it does not. Project.restrict!(user).create!(name: 'test', account_id: Account.first) works as expected but it is required to whitelist account_id and it is not what I want to do. Can protector handle this? Or I should fallback to manually use strong_parameters?

inossidabile commented 10 years ago

It should work. Show me your protection block pls.

pavel-rosputko commented 10 years ago

I can reproduce this issue when block is

protect do |user|
  scope { all }
end

And that's what is in IRB

[23] pry(main)> Project.restrict!(User.first).create!(name: 'test', account_id: Account.first)
  User Load (0.9ms)  SELECT "users".* FROM "users" ORDER BY "users"."id" ASC LIMIT 1
  Account Load (0.6ms)  SELECT "accounts".* FROM "accounts" ORDER BY "accounts"."id" ASC LIMIT 1
   (0.3ms)  BEGIN
   (0.3ms)  ROLLBACK
ActiveRecord::RecordInvalid: Validation failed: Access denied to 'name', Name can't be blank
from /home/pavel/.rbenv/versions/2.0.0-p247/lib/ruby/gems/2.0.0/gems/activerecord-4.0.0/lib/active_record/validations.rb:57:in `save!'

and

[25] pry(main)> Account.first.projects.restrict!(User.first).create!(name: 'test')                        
  Account Load (0.7ms)  SELECT "accounts".* FROM "accounts" ORDER BY "accounts"."id" ASC LIMIT 1
  User Load (0.7ms)  SELECT "users".* FROM "users" ORDER BY "users"."id" ASC LIMIT 1
   (0.3ms)  BEGIN
  SQL (0.7ms)  INSERT INTO "projects" ("account_id", "created_at", "name", "updated_at") VALUES ($1, $2, $3, $4) RETURNING "id"  [["account_id", 1], ["created_at", Sun, 01 Dec 2013 07:20:21 UTC +00:00], ["name", "test"], ["updated_at", Sun, 01 Dec 2013 07:20:21 UTC +00:00]]
   (8.4ms)  COMMIT
=> #<Project id: 38, name: "test", account_id: 1, created_at: "2013-12-01 07:20:21", updated_at: "2013-12-01 07:20:21">
pavel-rosputko commented 10 years ago

There's some other code that I expect should throw 'Access denied' :(

[14] pry(main)>  Project.restrict!(User.first).new(name: 'test') do |p|
  p.account = Account.first
  p.save!
end
  User Load (1.6ms)  SELECT "users".* FROM "users" ORDER BY "users"."id" ASC LIMIT 1
  Account Load (1.0ms)  SELECT "accounts".* FROM "accounts" ORDER BY "accounts"."id" ASC LIMIT 1
   (0.5ms)  BEGIN
   (0.4ms)  ROLLBACK
ActiveRecord::RecordInvalid: Validation failed: Account can't be blank

But it doesn't

p = Project.restrict!(User.first).new(name: 'test')
p.account = Account.first
p.save!

  User Load (1.6ms)  SELECT "users".* FROM "users" ORDER BY "users"."id" ASC LIMIT 1
  Account Load (1.3ms)  SELECT "accounts".* FROM "accounts" ORDER BY "accounts"."id" ASC LIMIT 1
  Account Load (0.9ms)  SELECT "accounts".* FROM "accounts" ORDER BY "accounts"."id" ASC LIMIT 1
  User Exists (1.0ms)  SELECT 1 AS one FROM "users" INNER JOIN "staff_memberships" ON "users"."id" = "staff_memberships"."user_id" WHERE "staff_memberships"."a
ccount_id" = $1 AND "users"."id" = 1 LIMIT 1  [["account_id", 1]]
   (0.3ms)  BEGIN
   (0.4ms)  ROLLBACK
ActiveRecord::RecordInvalid: Validation failed: Access denied to 'name'
from /home/pavel/.rbenv/versions/2.0.0-p247/lib/ruby/gems/2.0.0/gems/activerecord-4.0.0/lib/active_record/validations.rb:57:in `save!'
[22] pry(main)> 

throws

inossidabile commented 10 years ago

Yeah, new with block was actual issue which is now fixed. But create! thing should work. I'm unable to reproduce the bug and it's covered in specs as well. What is the version of Protector you use?

pavel-rosputko commented 10 years ago

0.7.0

inossidabile commented 10 years ago

I've just released 0.7.1 with the fix of new. Go try it. However it's unlikely that it will do something about the first issue of yours. Any other details you can provide to help me reproduce it?

pavel-rosputko commented 10 years ago

Yep. Clone this https://github.com/pavel-rosputko/protector-test and try in IRB: Account.first.projects.restrict!(User.first).create!(name: 'test') successfully creates Project but name is not whitelisted Project.restrict!(User.first).create!(name: 'test') do |user| user.account = Account.firs t; end throws Exception

inossidabile commented 10 years ago

0.7.2 released.