cancan icon indicating copy to clipboard operation
cancan copied to clipboard

authorize_resource preventing a destroy that should be allowed

Open giuseb opened this issue 12 years ago • 8 comments

Apologies in advance if not a proper issue, but the following behavior is baffling to me.

In abilities.rb:

class Ability
  include CanCan::Ability

  def initialize(user)
    if user.admin
      can :manage, Course
      cannot(:destroy, Course) { |c| c.locked? }
    end    
  end
end

where locked? is defined in the Course model.

Then I have a standard, CanCan-augmented RESTful controller:

class CoursesController < ApplicationController
  authorize_resource
  [snip]
  def destroy
    @course = Course.find(params[:id])
    @course.destroy
    redirect_to courses_path, notice: t('notices.success_destroying_course')
  end
end

Everything seems to work well, except for the destroy action. If I am logged in as an admin, and a course is not locked, my view helper correctly shows the destroy link:

if can? :destroy, @course
  link_to t('gen.delete'), course_path(@course), {confirm: t('gen.r_u_sure'), method: :delete
end

But when I actually try to destroy the course, CanCan raises the AccessDenied error. However, if I modify the controller like so:

class CoursesController < ApplicationController
  authorize_resource except: :destroy
  [snip]
  def destroy
    @course = Course.find(params[:id])
    authorize! :destroy, @course
    @course.destroy
    redirect_to courses_path, notice: t('notices.success_destroying_course')
  end
end

...then the authorization rule seems to work correctly! I suspect that my abilities' logic is messed-up, but then why does can? behave as expected? And why the different behavior between the authorize_resource and the explicit authorize! :destroy, @course call right in the action?

giuseb avatar May 24 '12 06:05 giuseb

After sprinkling some debugging code, I found out that the block:

  cannot(:destroy, Course) { |c| puts "LOCKED? #{c.locked?.inspect}"; c.locked? }

gets executed both during the show action, when can? :destroy, @course is called, and during the destroy action, if authorization is explicitly enforced there. The same block is skipped altogether during the destroy action, if I only rely on authorize_resource.

giuseb avatar May 24 '12 07:05 giuseb

@giuseb Based on the Only for object attributes section of defining abilities documentation, the block is only called when there is an actual instance object present. Look through the documentation and make the changes accordingly. If you are still experiencing the issue after the changes, you can re-open the issue.

tundal45 avatar Jun 02 '12 15:06 tundal45

@tundal45, unlike the index action, the first line in the destroy action does instantiate an actual @course object, so I remain at loss as to why the block is not executed. Should I re-open the issue? Thanks!

giuseb avatar Jun 03 '12 08:06 giuseb

@giuseb Sorry for a late response but feel free to open the issue. I will try to recreate the issue on my end and I will reach out to you with any setup questions. I may not be able to get back to you for a few days though.

tundal45 avatar Jun 07 '12 12:06 tundal45

@tundal45, I have put together a skeleton rails app that shows the behavior: https://github.com/giuseb/cancan_test

As it's written now, deleting an "unlocked" course will raise the error, in spite of the fact that can? :destroy, @course returns true.

The problem is resolved by setting authorize_resource except: :destroy at the top of the controller, and authorize! :destroy, @course after instantiating the @course in the destroy action

giuseb avatar Jun 08 '12 12:06 giuseb

BTW, I do not have permission to reopen the issue...

giuseb avatar Jun 08 '12 12:06 giuseb

Just found the same problem here, rails 4, cancan 1.6.10.

In order to perform authorization check on my destroy action I had to add:

authorize! :destroy, @event

Destroy action becomes:

def destroy
  authorize! :destroy, @event
  @event.destroy
  respond_to do |format|
    format.html { redirect_to events_url }
    format.json { head :no_content }
  end
end

Is this the expected behaviour or is it a bug?

rafaelcgo avatar Aug 23 '13 14:08 rafaelcgo

Thanks for your submission! The ryanb/cancan repository has been inactive since Sep 06, 2013. Since only Ryan himself has commit permissions, the CanCan project is on a standstill.

CanCan has many open issues, including missing support for Rails 4. To keep CanCan alive, an active fork exists at cancancommunity/cancancan. The new gem is cancancan. More info is available at #994.

If your pull request or issue is still applicable, it would be really appreciated if you resubmit it to CanCanCan.

We hope to see you on the other side!

xhoy avatar Jul 01 '14 07:07 xhoy