Rails authorization CanCanCan

Question

I'm trying to implement some authorization to routes with the CanCanCan gem but for some routes, it won't work and it's either always authorizes no matter what or not authorized at all.

I want only users with a role id of 5 (admin) to access the update action of the prices controller, this is my ability.rb code:

class Ability
  include CanCan::Ability

  def initialize(user)

    user ||= User.new
    # Define abilities for the passed in user here. For example:
    if user.present?
      can :show, User

      if user.role.id == 5
        can [:index, :update, :create], User
      end

      can :update, PricesController if user.role.id == 3
    #
    # The first argument to `can` is the action you are giving the user
    # permission to do.
    # If you pass :manage it will apply to every action. Other common actions
    # here are :read, :create, :update and :destroy.
    #
    # The second argument is the resource the user can perform the action on.
    # If you pass :all it will apply to every resource. Otherwise pass a Ruby
    # class of the resource.
    #
    # The third argument is an optional hash of conditions to further filter the
    # objects.
    # For example, here the user can only update published articles.
    #
    #   can :update, Article, :published => true
    end
  end
end

The first action for index etc. is working correctly and for the second action, I debugged that the role id is found correctly aswell. So the fault has to be in my controller, here is my code:

def update
    authorize! :update, current_user

    if @prices.where(description: params[:description]).update(price_params)
      respond_to do |format|
        format.html { redirect_to prices_path }
        format.json { render json: @prices }
      end
    end  
  end

If I use current_user to check in the authorized method everyone can change the values, if I use an instance variable of @prices then nobody can execute the controller action.

I'm also handling the exception:

rescue_from CanCan::AccessDenied do |e|
    respond_to do |format|
      format.html { redirect_to current_user, flash: { alert: "Sie besitzen dafür keine Berechtigung!" } }
      format.json { render json: { success: false }, status: 401 }
    end
  end

I read the documentation over and over again but I can't figure out where my fault is.

Answer 1

The ability definition should be:

can :update, Price if user.role.id == 3

You authorize models - not controllers.

When calling authorize you should pass the resource instance you are updating or the class if there is no instance:

authorize! :read, @thing
authorize! :index, Thing

But the controller is fundamentally broken in itself in a way that has nothing to do with CanCanCan. This is where it all falls apart:

@prices.where(description: params[:description]).update(price_params)

where returns a collection of records - #update is a method which is called on a single record. I have no idea if this is a very naive and failed attempt at mass updates or if you're trying to do something like a slug column (pretty urls). But you should probably stick with the rails conventions until you know what you are doing:

def update
  @price = Price.find(params[:id])
  authorize!(:update, @price)
  if @price.update
    # ...
  else
    # ...
  end
end

In this case you can also just use load_and_authorize_resource instead of manually finding and authorizing.

Answer 2

A few different comments:

In your ability.rb I'd say use

if user.role.name == 'admin'

instead of

if user.role.id == 5

as unless you set your ids manually you may well have to change this for production.

Also

can :update, PricesController if user.role.id == 3

should be

can :update, Price if user.role.id == 3

and in your controller replace

authorize! :update, current_user

with

authorize! :update, Price

Usually in a rails update action you would be updating just one object and you would authorise it using:

authorize! :update, @price

but in your case I would guess authorising via the model is your best route.