Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

authorize_resource preventing a destroy that should be allowed #626

Open
giuseb opened this Issue May 24, 2012 · 8 comments

Comments

Projects
None yet
4 participants

giuseb commented May 24, 2012

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 commented May 24, 2012

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.

Collaborator

tundal45 commented Jun 2, 2012

@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 tundal45 closed this Jun 2, 2012

giuseb commented Jun 3, 2012

@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!

Collaborator

tundal45 commented Jun 7, 2012

@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.

giuseb commented Jun 8, 2012

@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 commented Jun 8, 2012

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

@tundal45 tundal45 reopened this Jun 8, 2012

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?

xhoy commented Jul 1, 2014

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!

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