search

chaps-io / access-granted

Multi-role and whitelist based authorization gem for Rails (and not only Rails!)
MIT License
774 stars 41 forks source link

Doesn't work with ActiveRecord relations #23

Closed TrevorHinesley closed 8 years ago

TrevorHinesley commented 8 years ago

For instance

@artists = Artist.all
authorize! :read, @artists

Will not work, no matter if can :read, Artist is specified in the policy. This does work:

@artists = Artist.all
authorize! :read, Artist

But is insufficient for obvious reasons. Each one needs to be checked individually to make sure this collection is viable for this user in this case.

Also, something like this seems like it may work:

@artists = Artist.all
authorize! :read, @artists.first

But if @artists.first == nil in this case of an empty collection, this will result in access denied.

pokonski commented 8 years ago

It is not intended to work with relations because they are a different class than Artist. If you just want to check if user has a general permission to read Artists just use a class as the second argument:

authorize! :read, Artist
@artists = Artist.all

Not sure what your aim is here, but generally looping through every artist and filtering it in the controller is a terrible practice.

If you know what Artists belong to the user just filter them instead of doing all. Use your database to only return the results that user can see or owns, but this is out of the scope for this gem.

TrevorHinesley commented 8 years ago

They're being filtered, I think the best option here would be to do the .first alternative that I mentioned, but if it's an empty collection let it pass or allow conditions for that empty case? Just a thought. On Fri, Dec 11, 2015 at 1:48 AM Piotr Okoński notifications@github.com wrote:

It is not intended to work with relations because they are a different class than Artist. If you just want to check if user has a general permission to read Artists just use a class as the second argument:

authorize! :read, Artist @artists = Artist.all

Not sure what your aim is here, but generally looping through every artist and filtering it in the controller is a terrible practice. If you know what Artists belong to the user just filter them instead of doing all.

— Reply to this email directly or view it on GitHub https://github.com/chaps-io/access-granted/issues/23#issuecomment-163867304 .

ambethia commented 8 years ago

@TrevorHinesley Rolify handles what I think you're trying to do.