Skip to content

CanCan and InheritedResources #23

Closed
mhfs opened this Issue Dec 26, 2009 · 20 comments

8 participants

@mhfs
mhfs commented Dec 26, 2009

Hi Ryan,

I was trying to use CanCan with Inherited Resources but I got to a problem when using the "authorize_resource" alone, without "load_resource".

This method ends up counting on an instance variable @model but that doesn't get set when using Inherited Resources until you call the "resource" helper (lazy loading). The way to make it work would be fetching it from the "resource" helper.

What do you think of adding a param/option to "authorize_resource" which would allow determining an alternative strategy of fetching the resource?

Thanks,
Marcelo Silveira

@ryanb
Owner
ryanb commented Dec 31, 2009

There are a number of other resource loading utilities (such as make_resourceful) and ideally these should be supported as well.

The lazy loading method does seem like a popular approach which several of them use so I think this is definitely worth adding. How about an :accessor_method option?

authorize_resource :accessor_method => :item

Any suggestions for an alternative name?

@mhfs
mhfs commented Dec 31, 2009

I'm not a fan of this name but couldn't think of anything better. =)

Would you be interested in a patch?

Happy new year!

@hexorx
hexorx commented Jan 6, 2010

On my app I just extended InheritedResource with the following code:

def authorized?
  unauthorized! if cannot?(params[:action].to_sym, asset) 
end

def asset
  @asset ||= case params[:action].to_sym
  when :index
    resource_class
  when :new, :create
    build_resource
  else
    resource || resource_class
  end
end

then I just do a before_filter :authorized?

I like this method because it makes it just work if you are using inherited_resource. I will fork and add as a patch if interested.

@mhfs
mhfs commented Jan 13, 2010

Hey @hexorx, thanks for the heads up. It made cancan work immediately with IR which is great.

As per a definitive solution I'd go the way Ryan suggested.

Cheers!

@mhfs
mhfs commented Jan 17, 2010

After some study it seems the place to handle the option would be ControllerResource::model_instance.

However, to call the accessor_method from there, I'd need to do the same checks that are done at ResourceAuthorization regarding collection_actions and new_actions in order to just call the accessor when appropriate.

I thought it'd be a good idea to validate the approach before moving on.

Suggestions?

@millisami

@hexorx, thanks. I was going to leave the CanCan due to this same error while I was using it with IH. But the hack worked for me as well.
I just love both CanCan and IH in combination.

@ryanb
Owner
ryanb commented Feb 2, 2010

Seeing the hack, I realize this problem is more complex then I first assumed. There isn't just a single accessor method which returns an object in various states (new, existing, etc) but multiple methods to help build the resource. Therefore I don't think a simple option will be sufficient because it is very specific to ResourceAuthorization.

Probably the best thing is to deal with ResourceAuthorization directly and detect if it is being used. It would be nice to abstract this out so it works with other resource loading plugins as well. I'm not certain when I will get around to this, so if someone wants to work on this in a fork, be my guest. :)

@markmcdonald51

Hi Ryan, et. al.

I am using cancan (v1.1.1) with inherited_resources (v 1.0.6) and work excellent with one exception.

When I have a IR controller that has a belongs_to relationship and I add 'load_and_authorize_resource' into it, IR is missing the respected association id when doing an insert of a new record. It has the field in it's sql insert, but only a null value. When I remove load_and_authorize_resource the belongs_to id is included. This seems to be a big problem because the insert continues and I would be left to believe that everything worked correctly. I have been thinking about going thru your code and writing a patch but I was wondering if anyone here had already done so.

Anyway, thanks for the great plugin Ryan. This has become a big part of my app.

All the best,
Mark McDonald

@markmcdonald51

So my belongs_to problem about is easily fixed (or so it seems right now) if you just add except:

load_and_authorize_resource :except => [:create, :update]

this is because cancan is trying to load/build all resources...so it Inherited Resources however cancan doesnt know about a belongs_to function and doesnt add in the respected belongs_to association id. Anyway, I am glad I got this worked out. Hopefully this will sameone else some time and grief!

Thank for this very cool gem Ryan!

Mark

@ryanb
Owner
ryanb commented Sep 9, 2010

adding support for loading through Inherited Resources - closed by 4eee637

@ryanb
Owner
ryanb commented Sep 9, 2010

If someone wants to try this out the latest commit that would be great. You need to do load_and_authorize_resource and the loading should now happen through Inherited Resources when that is available. The reason you can't just do authorize_resource is because InheritedResources does lazy loading. Also because CanCan does some additional loading functionality such as the accessible_by on the index action.

@mhfs
mhfs commented Sep 10, 2010

Hey Ryan, thanks a lot for coding it. I'll try to test it out in the weekend.

@jbarreneche

Hi Ryan, I ran into some issues (in before filter the filter was still using ControllerResource instead of .cancan_resource_class, and the methods #resource, and #build_resource are protected in ::InheritedResources::BaseHelpers)
So I made those little changes in my branch (sorry, no tests though :( )
http://github.com/jbarreneche/cancan

BTW, it's the first time I use cancan and I liked it :)
Thanks!

@ryanb
Owner
ryanb commented Sep 16, 2010

Thanks for pointing these problems out, I'll merge them in.

@contentfree

With these changes for InheritedResources, should we have to override the collection and resource methods to use accessible_by?

@ryanb
Owner
ryanb commented Sep 23, 2010

@contentfree, Nope, it should do it automatically as long as load_resource is in the controller.

@contentfree

Very right, sir. Thanks.

@bmihelac
bmihelac commented Oct 1, 2010

I am currently having issue described above with belongs_to and load_and_authorize_resource.
Is this fixed?
I use it as follows:

belongs_to :business_unit, :optional => true
load_and_authorize_resource

@contentfree

@bmihelac, you need to load_and_authorize_resource for the business_unit too, I believe. Should probably look something like the following:

belongs_to :business_unit, :optional => true
load_and_authorize_resource :business_unit
load_and_authorize_resource :your_resource_name, :through => :business_unit
@bmihelac
bmihelac commented Oct 3, 2010

oh yes, that works. thanks @contentfree

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.