Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Unexpected behavior for load_and_authorize_resource through #638

Closed
flatrocks opened this Issue · 4 comments

3 participants

@flatrocks

I'm creating a nested route for a singleton resource, and using load_and_authorize_resource through in my controller for the child object. I was expecting load_and_authorize_resource to assign a parent ID to my child object, but as far as I can tell, that's not how it works.

I have tried to boil this down to the basics:

App1::Application.routes.draw do

  resources :users do
    resource :import
  end
  # results in:
  #      user_import POST   /users/:user_id/import(.:format)       {:action=>"create", :controller=>"imports"}
  #  new_user_import GET    /users/:user_id/import/new(.:format)   {:action=>"new", :controller=>"imports"}
  # edit_user_import GET    /users/:user_id/import/edit(.:format)  {:action=>"edit", :controller=>"imports"}
  #                  GET    /users/:user_id/import(.:format)       {:action=>"show", :controller=>"imports"}
  #                  PUT    /users/:user_id/import(.:format)       {:action=>"update", :controller=>"imports"}
  #                  DELETE /users/:user_id/import(.:format)       {:action=>"destroy", :controller=>"imports"}
end
class User < ActiveRecord::Base
  has_one :import, :inverse_of => :user, :dependent => :destroy
  ...
end
class Import < ActiveRecord::Base
  belongs_to :user, :inverse_of => :import
 ...
end
class ImportsController < ApplicationController
  load_and_authorize_resource :user
  load_and_authorize_resource through: :user, singleton: true

  def create
    if @import.save
      redirect_to @import, :notice => "Successfully created import."
    else
      render :action => 'new'
    end
  end

end

When I run the create action, I get a validation error on "User can't be blank" and when I inspect the intermediate results, I see that load_and_authorize_resource is building an Import object without setting its parent (user) id.

This is easy to fix using hidden form fields, a before filter, or even just an inline assignment in the controller method to directly set the parent ID.

Can anyone verify whether I setting this up wrong, or if it just does not work as I had imagined ??

Assuming this is the intended behavior, I propose an example would be helpful to clarify that load_and_authorize_resource does not assign an object's parent ID in these cases.

@mikepack
Collaborator

@flatrocks Looks like you've got everything set up properly. I've duplicated your setup and have not experienced the same issue that you're seeing. What you're experiencing is not the intended behavior.

CanCan will effectively perform the following operations:
1. @user = User.find(params[:user_id])
2. @import = Import.new(params[:import])
3. @import.user = @user

Can you check to see if the above steps produce the same result in Rails console?

Also, it might be worth pasting your Import class's validation on #user as well.

@ollym

@flatrocks Sounds like you're running into this bug: #635 @ryanb is taking his time merging the fix so you might as well use the fork for now.

gem 'cancan',
  git: 'https://github.com/ollym/cancan.git',
  branch: '2.0'
@mikepack
Collaborator

@ollym Is this issue resolved by #635?

@flatrocks

I am using CanCan 1.6.7. I have verified this (read: several long days digging in to the details... arggggh) and my results are that nested controller actions DO NOT automatically get the parent ID as I'd expected when using "load_and_authorize_resource through" as shown above.

After trying a few options, it turns out the cleanest solution seems to be adding a before_filter to my affected controllers. See commented lines:

class ImportsController < ApplicationController
  # prepend so this fires before load_and_authorize, making ability rules work as expected
  prepend_before_filter :assign_parent_id  
  load_and_authorize_resource :user
  load_and_authorize_resource through: :user, singleton: true

  def create
    if @import.save
      redirect_to @import, :notice => "Successfully created import."
    else
      render :action => 'new'
    end
  end

private

  # Manually assign the parent id from the params.  
  # This works just like adding a hidden field, but without the view mess.
  def assign_parent_id  
    params[:import][:user_id] ||= params[:user_id]
  end

end

So... after reading the discussion, I am still not sure if load_and_authorize_resource through is really supposed to automagically set the nested singleton object's parent ID in the resource. But in my case, it simply does not.

And it really is OK. But would be nice to know that's what to expect, and an official example showing this scenario would have been really helpful.

@flatrocks flatrocks closed this
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.