Access nesting resources in Ability class #64

Closed
mryan43 opened this Issue Apr 29, 2010 · 21 comments

6 participants

@mryan43

Hello,

Let's supose we have an "article" resource with nested "sub_articles" resources.

we define in the SubArticle class :

load_and_authorize_resource :nested => :article

Now let's imagine we want to allow users to manage SubArticles only for Articles that they own.

we would like to put something like this in our ability class :

can? :manage, SubArticle do
  @article.created_by == user
end

however we do not have an @article variable, we are in the Ability class scope and not in the SubArticles controller's scope.

It would be really nice if the can? method in the Ability class supported resources nesting as tree (hash) of models like so :

can? :manage, {Article => SubArticle} do |article, sub_article|
  article.created_by == user
end

Thanks again for this great library :)

@sergio-fry

Hi! May be this will work it out:

can? :manage, SubArticle do |sub_article|
  sub_article && sub_article.article.created_by == user
end

check out this screencast: http://railscasts.com/episodes/192-authorization-with-cancan

@mryan43

Hi !

Thanks for the suggestion but I'm affraid this solution only works for "edit", "update", "destroy" actions...

The solution you are suggesting needs a record to work. When you call "create", "new" or "index", you don't have a record and therefore you can only determine the abilities by other means, like the user's role or request IP or whatever.

In the railscast, it works because the article being commented is never a criteria for creating or listing comments.

@sergio-fry

Just today i've used this solution to create memberships in community (user can add only himself to community). I think, you are not right.

@mryan43

i've double checked and load_and_authorize_resource :nested seems to deal correctly with "new" and "create", making the association between the new record and the nesting resource so that sub_article is not nil and sub_article.article correctly gives the article object. So i was wrong about the new and create methods, sorry.

However i'm still out of luck for the index method (sub_articles is nil, so I can't get the article). Any idea ?

@sergio-fry

What do you want not to allow to your users? :) Describe, please

@mryan43

I want to prevent my users to list (index action in SubArticle controller) the sub_articles that correspond to an article which they do not own.

In terms of URI :

User Alice owns article 1
User Bob owns article 2

i want Alice to be forbidden access to http://host/articles/2/sub_articles

@sergio-fry

heh, i've got it. may you can use before_filter in this controller?

@mryan43

putting such a test in a before_filter would kind of defeat the whole purpose of cancan, which is to have all your abilities defined declaratively in a single file.

wouldn't it ?

@sergio-fry

you are right. CanCan is very handy when we have simple resources. Try to ask Ryan or just browse the source of CanCan to find the best way.

Good luck!

@stevewilhelm

I am trying to do something similar. Sensor belongs_to User, LogFile belongs_to Sensor, want logfiles_path to only display log files created by sensors owned by the particular user.

Is this what deep nested on load_and_authorize_resource is supposed to address? If so, an example Ability can rule would be helpful.

@sergio-fry

I think, that you are trying to move a controller logic to the ability class. In a controller#index you should select logs created by user's sensors. So everyone can open page controller#index, but each user could see list of different logs.

@mryan43

The purpose of cancan is to move controller logic to the ability class, but only authorization logic.

Don't forget that the resources are nested !
that means we want the log_file index to be "per sensor" and have paths like
http://host/sensors/:sensor_id/log_files
to access our index action in the LogFilesController.

If the user doesn't own the sensor, we don't want to just "filter" the data in this index action, we want the user to be forbidden/redirected, and that logic should clearly go into the Ability class.

@sergio-fry

agreed

@ryanb
Owner

From my understanding, will this do the job for you?

# in SubArticlesController
load_and_authorize_resource :nested => :article
before_filter :authorize_article
private
def authorize_article
  authorize! :read, @article
end

Basically, this will check that the user has :read permission on the @article resource before accessing the subarticles. I will consider building this into the authorize_resource method if it solves the problem.

@mryan43

Yes, this works very well, thanks a lot ! But there's still a bit of authorization logic in the controller itself when you call the authorize! method.
If you move this to the authorize_resource method ( I think it would be a great addition), it would be really nice to have the flexibility to specify the action that is authorized on the parent resource (in your example you use :read).

I will definitely use your solution for now, it's much cleaner than what i've come up with :) thanks !

@jondkinney

Ryanb, did you build this in? If so how do we use it? I'm struggling with this at the moment on a very time sensitive project (not your fault I know but any help is very much appreciated!). Thank you very much.
-Jon

@sickill

@ryanb: This could work, but only in some scenarios. Here :read permission for @article is enough. But I have the case where user can manage project's memberships, and he's required to be manager of the project.

I think better would be to just pass parent resource to can block as the last argument:

can :index, Membership do |membership, project|
  user.managable_projects.include?(project || membership.project)
end

It would be more flexible this way I think.

@sickill

Btw, it has been implemented in junaid fork here:

http://github.com/junaid/cancan/commit/fdb7510737e925752a7f60cbf0f9c5f583a4a734

This commit adds other things but you can see there it's adding parent resource to can block. In my opinion this is the way to go.

@ryanb
Owner

Also see issue #83.

I'm still investigating this and will try to integrate a good solution.

@ryanb
Owner

Closing this because the nesting behavior in CanCan 1.3 will be changed significantly. I added some documentation and also tried to address the issue mentioned here at the bottom of the Nested Resources wiki page.

Once CanCan 1.3 is out, give this a try and see if it fixes your problem. If not please post a separate issue describing why it does not solve it.

@mryan43

Amazing ! Thank you so much !

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment