Critical: Nested resources are still accessible

[Zaijo]

Following the original guidline in CanCan wiki - Nested Resources leads to unwanted behaviour.

CanCan checks access rights for project given in params[:project_id]. It means, that there is still a possibility of bypassing the check for tasks. Consider we have read access to project #5. Navigating to projects/5/tasks/47 will show task 47 no matter what is it's parent project.

To fix this, always set the following in a before_filter

@project = @task.project if @task

I updated the wiki page, but i need your consent for such advice to public.
Many sites might be vulberable this way.

[graywh]

If you've followed the wiki and used the :through option to load_and_authorize_resource, this can't happen because it will use @project.tasks.find(params[:id]) to find a task. When it tries to load a task through a project it doesn't belong to, you'll get ActiveRecord::RecordNotFound.

[Zaijo]

OK.
I was not missing the load_and_authorize_resource through: :project, but the following code has overwritten the CanCan behaviour.
The problem was, that I used this for loading:

# WRONG DEFAULT (generated by rails generate ...)
# Use callbacks to share common setup or constraints between actions.
def set_task
  @task = Tasks.find(params[:id])
end

This is a consequnece of the 'Rails magic'. I didn't realize this.

I don't know. Maybe mentioning this problem in Controller Authorization Example - CanCan Wiki could help someone. What do you think?

[graywh]

I wasn't aware the generator did that now. Of course you can mention that
on the wiki.

On Saturday, August 30, 2014, Zaijo [email protected] wrote:

OK.
I was not missing the load_and_authorize_resource through: :project, but
the following code has overwritten the CanCan behaviour.
The problem was, that I used this for loading:

WRONG DEFAULT (generated by rails new project)

Use callbacks to share common setup or constraints between actions.

def set_task
@task = Tasks.find(params[:id])
end

This is a consequnece of the 'Rails magic'. I didn't realize this.

I don't know. Maybe mentioning this problem in Controller Authorization
Example - CanCan Wiki
https://github.com/ryanb/cancan/wiki/Controller-Authorization-Example
could help someone. What do you think?

—
Reply to this email directly or view it on GitHub
#1014 (comment).

Will Gray
Nashville, TN

[Zaijo]

Wiki page Controller Authorization Example is updated and linked to this issue.

[dkonayuki]

Is there any solutions for this issue yet?
In my application, user can still access to other user's item index page. ( show page is blocked as expected though). For example: user 2 can still navigate to users/3/educations.
My EducationsController is like this:

  load_and_authorize_resource :user
  load_and_authorize_resource :education, through: :user, only: [:index, :show, :new, :edit, :destroy]
  before_action :set_education, only: [:show, :edit, :update, :destroy]
  before_action :set_user

    # Use callbacks to share common setup or constraints between actions.
    def set_education
      @education = Education.find(params[:id])
    end

    def set_user
      @user = User.find(params[:user_id])
      #@user = current_user
    end

[graywh]

@dkonayuki Yes, delete the set_education function and filter. Cancan already sets @education for you.

[dkonayuki]

Yes, I did that. It's not working.
Again, the show page is not accessible. However the index page always shows.
This is the related part in ability.rb:

      can :manage, Education do |e|                     # only owner can manage his educations
        e.user == user
      end

[graywh]

Abilities defined in blocks don't work with #accessible_by. This is all described in the wiki: https://github.com/ryanb/cancan/wiki/Defining-Abilities-with-Blocks. You should use a conditions hash (e.g. can :manage, Education, :user_id => user.id) or provide an SQL fragment.

If a user has read access to any education records, the index page will be accessible. I'm assuming you want to filter the records available there.

[dkonayuki]

Thank you for your reply.
However none of those works. ( Block and conditions hash )
Btw, I'm using rails 4, so I don't have any #accessible_by attribute
And also educations belong to user exclusively, other user should not have any access to other user's educations.

[dkonayuki]

I figured out how to fix this. By adding these in index action, instead of load_and_authorize_resource:

educations = @user.educations
authorize! :index, educations