Devise / Rolify / CanCan 2.0: Preventing users from changing a user's role

Question

I am using Devise for authentication, Rolify for role management and CanCan 2.0 for authorization.

I am trying to allow the :admin role to change a user's roles, but disallow all other users access.

Here is what I have tried and is not working:

#ability.rb
class Ability
  include CanCan::Ability

  def initialize(user)
    if user.has_role? :admin
      can :access, :all
    elsif user.has_role? :moderator
      can [:index, :read, :update, :destroy], :users, :user_id => user.id
      cannot :access, :users, [:role_ids]
    end
end

#application_controller.rb
...
rescue_from CanCan::Unauthorized do |exception|
    redirect_to root_url, :alert => exception.message
  end

I have intentionally left the association in my user form:

#_form.html.erb
<%= simple_form_for @user do |f| %>
  <%= f.association :roles, as: :check_boxes %>
  <%#= f.association :roles, as: :check_boxes if can? :update, @user, :roles %>
  <%= f.button :submit %>
<% end %>

controller

#users_controller.rb
class UsersController < ApplicationController

  before_filter :authenticate_user!
  load_and_authorize_resource

  def index
    @users = User.accessible_by(current_ability)
  end

  def new
    @user = User.new
  end

  def create
    @user = User.new(params[:user])
  end

  def show
    @user = User.find(params[:id])
  end

  def edit
    @user = User.find(params[:id])
  end

  def update
    @user = User.find(params[:id])

    @user.update_without_password(params[:user])

    if successfully_updated
      redirect_to @user
    else
      render :action => "edit"
    end
  end
end

and the model:

#user.rb
    class User < ActiveRecord::Base
      rolify

      attr_accessible :role_ids
    ...

Now if a user who has the role of :moderator tries to change another user's (or his own) roles, here is what happens:

1. A CanCan::Unauthorized exception is thrown and the user is redirected to root_url
2. The roles are changed for the user

I am confused. If the exception happens, why are the changes still made? I am probably doing something very wrong :)

I have tried manipulating the query params depending on a users role in users_controller.rb If I put a log statement right after def update, here is my output:

2013-04-24 12:42:21 [4161] DEBUG    (0.1ms)  BEGIN
2013-04-24 12:42:21 [4161] DEBUG    (0.3ms)  INSERT INTO "users_roles" ("user_id", "role_id") VALUES (5, 1)
2013-04-24 12:42:21 [4161] DEBUG    (0.4ms)  COMMIT
2013-04-24 12:42:21 [4161] DEBUG   User Load (0.5ms)  SELECT "users".* FROM "users" WHERE "users"."id" = $1 LIMIT 1  [["id", "5"]]
2013-04-24 12:42:21 [4161] DEBUG {"username"=>"Blabla", "email"=>"bla@bla.com", "password"=>"", "password_confirmation"=>"", "approved"=>"1", "role_ids"=>["1", "2", ""]}

I must be overlooking something...

Answer 1

I ended up using a before_filter, like this:

before_filter :prevent_unauthorized_role_setting, :only => [ :create, :update ]

def prevent_unauthorized_role_setting
  if cannot? :manage_roles, current_user
    params[:user].delete_if { |k, v| k.to_sym == :role_ids }
  end
end

while following Zaid's suggestion in ability.rb:

cannot :manage_roles, :users
if user.has_role? :admin
  can :manage_roles, :users
end

Also, I dropped Rolify and managed roles on my own.

Answer 2

Firstly, you probably want to clean up your abilities, as things seem a little confused at the moment. Ignoring everything else, it's probably a good idea to specify a custom action for modifying passwords, for clarity's sake. For example:

# ability.rb

def initialize(user)
  ...

  cannot :manage_roles, :user
  if user.has_role? :admin
    can :manage_roles, :user
  else
end

(What exactly are you trying to achieve with the rest of the rules? Currently if seems you're letting moderators read, edit and delete themselves only, is this what you intended?)

You'll probably want to hide or disable the roles section of your form for anyone that can't actually do anything with it:

#_form.html.erb
<%= simple_form_for @user do |f| %>
  <%= f.association :roles, as: :check_boxes if can? :manage_roles, @user %>
  <%= f.button :submit %>
<% end %>

(Note that can? only takes two arguments, the action, and the object/class.)

If you want to be extra secure, you could also use the following check in the controller:

# users_controller.rb
def update
  @user = User.find(params[:id])

  user_params = params[:user]
  if cannot? :manage_roles, @user
    user_params.delete_if { |k, v| k.to_sym == :role_ids }
  end

  if @user.update_without_password(user_params)
    redirect_to @user
  else
    render :action => "edit"
  end
end

(You will need to double check what the correct key to remove from the params hash is, I'm assuming it's :role_ids based on your attr_accessible, but I don't really know simple_form that well.)

Answer 3

I am not sure if I understood what you want the moderators to do but here is a basic configuration I use. I adapted it to your scenario as far as I could. If moderators do not need individual rights simple remove the inner clause.

class Ability
  include CanCan::Ability

  def initialize(user)
    # Create guest user aka. anonymous (not logged-in) when user is nil.
    user ||= User.new

    if user.has_role? :admin
      can :manage, :all
    elsif user.has_role? :moderator
      can :manage, User, user_id: user.id
      can :create, User
      can :read, :all
    else
       # Guest user aka. anonymous
      can :read, :all
    end
  end
end

Answer 4

Wrap the roles with this in your users/_form.html.erb file:

With simple form:

<% if can? :manage, User %>
  <%= f.association :roles, as: :check_boxes %>
<% end %>

Without simple form:

<% if can? :manage, User %>
    <div class="control-group">
      <%= f.label :roles, class: "control-label" %>
      <div class="controls">
        <% Role.all.each do |role| %>
            <%= check_box_tag "user[role_ids][]", role.id, @user.role_ids.include?(role.id) %>
            <%= role.name %><br />
        <% end %>
      </div>
    </div>
<% end %>