Authorizing access to specific fields on a model

[macek]

Cancan is my favorite authorization plugin for Rails but I seem to find difficulty when getting down to the nitty gritty. How would we go about role-based permissions for specific fields on any given model?

For example, say sales and customer service staff can edit the address fields on a user, but only admin or sales can update a user's customer number. Loading the user resource with cancan is no problem, but the field-based permission logic in the views get very ugly very quickly.

# e.g., app/views/users/_form.html.erb

<% if current_user.role?(:admin, :sales) %>
  <%= f.text_field :customer_number %>
<% else %>
  <%= @user.customer_number %>
<% end %>

<% if current_user.role?(:sales, :csr) %>
  <%# edit address fields %>
<% else %>
  <%# display address %>
<% end %>

This seems to be a very common issue I see people running into (myself included), but I'm never quite sure how to address it in the best way. If possible, I'd like to get a discussion going on this topic.

Note: If it's not in cancan's scope, I completely understand closing this issue.

[dapi]

I'm very interested in this feature too.

[cwade]

Me too.

[ryanb]

One solution is to use a custom action, see this wiki page for details.

Another solution is to use the trusted-params plugin, but that is not yet Rails 3 compatible. It also does some ugly overriding of internal Rails methods and all hashes, which is not something I want built into CanCan.

I agree neither of these are ideal and don't flow with CanCan's handling of permissions. Does anyone have some other ideas on better ways to do this through CanCan?

[dapi]

Yes, overriding ActiveRecord methods is not the best way.

Now I use solution like this:

Controller:

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

  attrs = params[:user]
  authorize? :update, @user, attrs

  @user.update_attributes(attrs)

 ...

Ability:

can :update, User do | subject, params |
  ! user.toucheble_parameters(subject).each { |p|
    break if params.exists? p
  }.nil?
end

Where User#toucheble_parameters is a list of the subjects parameters that can be touched by the user.

May be better to have just methods like can/can? but without actions?

Example:

can_touch_attributes User, [:name, :email], :user_id=>user.id  # Anybody can touch his name and email

can_touch_attributes User, [:is_admin], :role => 'admin'  # Only admin can touch is_admin

...

can_touch_attributes? @user, attrs

I think we don't need actions for checking attributes. But if somebody needs it, he can use the first solution.

[macek]

dapi,

Excellent comments here. I like the technique you're using in Ability.rb.

I'm still thinking about situations like this:

<%= @product.price if current_user.role?(:admin, :sales :distributor) %>
<%= @product.cost if current_user.role?(:admin, :sales) %>

In these examples, I need to control who can :read certain attributes. This worsens; look at an edit product form.

<% form_for @product do |f| %>
  ...
  <% if current_user.role?(:admin) %>
    <%= f.text_field :price %>
    <%= f.text_field :cost %>
  <% elsif current_user.role?(:sales) %>
    <%= @product.price %>
    <%= @product.cost %>
  <%= elsif current_user.role?(:customer_service) %>
    <%= @product.price %>
  <% end %>
  ...
<% end %>

I think you're on to something with your can_touch_attributes macro, but I can think of several examples where I'd want permission based on the action/attribute combination; not just the attribute alone.

[macek]

dapi,

I'm trying to come up with a good combination of cancan Custom Actions and the techniques you described above...

Keeping everything defined in Ability.rb is very attractive to me. Let's keep poking at this and see if can come up with something :)

[ryanb]

What if can supported an additional parameter that is a symbol representing the attribute. This could be an array of symbols too.

can :manage, Article, :content, :user_id => user.id

And similar for can?.

can? :update, @article, :content

In both cases the last symbol is optional, and if not present it will represent all attributes which is the current behavior. The check for each attribute would happen automatically in authorize_resource in the controller.

This conflicts a little with the existing API but let's not worry about that when brainstorming.

[dapi]

Yes. Something like this.

Views

To check parameters that are not permitted to update. Don't show it in the form.

<% if can? :update, @user, [:is_admin, :roles] %>
  <%= form.input :is_admin %>
  <%= form.input :roles %>

or

<% if can? :update, @article, :banned  %>
  <%= form.input :banned %>

Abilities

Admin can update any attributes:

can :update, Article, user.is_admin => true

# is similar to

can :update, Article, :attributes => :all, user.is_admin=>true

Owner can update any attributes except :banned

can :update, Article, :exclude => [ :banned ], :user_id=>user.id 

or

can :update, Article, :attributes => [ :subject, :content, :published ], :user_id=>user.id

or just

can :update, Article, [ :subject, :content, :published ], :user_id=>user.id

Controller

Raise an exception if the user has no the ability or if he try to update banned attributes.

authorize! :update, @article, params[:user]

# or

authorize! :update, @article, params[:user].keys

Raise an exception if the user has no any declared abilities to update the Article.

authorize! :update, Article

And the hit of brainstorming: If he has some abilities then remove banned attributes silently.

#  params[:user] = { :content=>'test', :banned=>false }

sanitize :update, @article, params[:user]

# now params[:user] = { :content=>'test' }

[ryanb]

Perhaps the :exclude option can be handled with cannot.

can :update, Article
cannot :update, Article, :banned

Also silently removing attributes with sanitize might cause some confusion. I find that to be the case with the current behavior of attr_accessible. Better to raise an authorization error and let the developer rescue from that early if he wants.

Everything else looks good though. I'll definitely consider this in a major update to CanCan.

[macek]

dapi, this looks great. A couple comments.

Abilities

You use the hash key :exclude. The Rails convention here seems to be the key :except. I think we should stick with that.

If we use with :attributes => :all or :attributes => [...], We might not need an API change. We could simply treat :attributes and :except as reserved keys. This might not be entirely favorable, though, as someone might have an attributes field on one of their models.

# user can only see blue products
can :read, Product, :attributes=>"blue"

Seems unlikely, but it shouldn't be overlooked entirely.

Controller

First, a little error in your code. You're authorizing @article but using params[:user].

I think we'd want to use authorize! :update, @article, params[:article]. I think it's assumed that we'd want to use the keys of the params hash, so calling params[:user].keys every time would be kind of silly. Plus, I think it would be confusing for people that are new to the plugin. It'd be easy to do something like,

def check_attributes(attributes)
  if attributes.is_a? Hash
    attributes = attributes.keys
  end
  ...
end

Moving on to your sanitize method. I love this. I've recently been tinkering with Searchlogic and sanitizing form params can be tedious. I like the idea of not shoving sanitization aside or implementing it as an afterthought.

[macek]

Ryan,

Ah, I was likely writing my previous comment the same time as you. I didn't see your comment until after I posted.

Handling :except with cannot seems to do the trick. This actually seems very elegant. If the attribute list is always an array, you can skip the need for :attributes => :all too.

dapi, to be more explicit, I think this is what Ryan is going for.

can :update, Article                        # all attributes
cannot :update, Article, [:banned]          # except for :banned

can :update, Project, [:priority, :status]  # only :priority and :status

This brings up another idea. What if the attributes array contains associations or nested attributes? Would something like this work?

# associations
can update, Project, [:categories, :members]

# nested attributes
can :update, Project, [:notes_attributes]

---

I also agree that raising an exception is good for the default behavior. If desired, perhaps a flag could be passed to sanitize to "force" cleanse the params?

[ryanb]

@macek, nested attributes partially work since they are considered an attribute on the model. However, if you want to control access to specific, deeply nested attributes then that will be a problem. This happens outside of the controller, so there's no way to know that notes_attributes should be checked on the Note model.

Actually nested attributes are a problem with CanCan's current behavior as well, but I haven't heard any complaints. I'm guessing they just work around it with before filters.

Regarding sanitize, I have a hard time seeing the use case, but that doesn't mean there isn't one. I'm up to discussing it further, but maybe under a separate issue after we see this in action more.

[ryanb]

Just some more brainstorming ideas. It will be tedious to perform the can? check on each individual field in the view. Maybe there can be a separate form builder gem which accepts the current_ability and automatically hides/shows each field depending on the user's permission. This could integrate with Formtastic as well for further convenience.

[dapi]

Ryan,

I agree with you, it's better to raise an authorization error when user tries to update secret fields.

About Formtastic. It's very good idea.
Can we use sanitize to automaticly remove fields and have the clear form? There is no needs to any Formtastic's changes.

def new
  authorize! :create, User
  @user = User.new
  sanitize :create, @user # Removes secret fields
end

...

= semantic_form_for @user do |form|
  = form.inputs      # Shows existent fields only

or

= semantic_form_for sanitize( @user ) do |form|
  = form.inputs

In this case sanitize automaticly detects an action.

[macek]

dapi,

I'm pretty sure most people won't be using the <%= f.inputs %> macro to dump all the fields at once. The ability to build the form manually would be essential.

[rbritom]

@ryanb I guess you already know but rails 3(activerecord) has a mass assignment security module,

http://github.com/rails/rails/blob/master/activemodel/lib/active_model/mass_assignment_security.rb

cancan should take advantage of this and i think trusted params could be reworked to take advantage of this.

[ryanb]

@rbritom, wow that's awesome, I did not know that. Time to investigate and maybe I'll do a Railscasts episode on it.

[rbritom]

@ryanb =D

I said is part of activerecord but is really part of active model so it can be applied to classes that dont have a DB backing them up.

[dapi]

Will be nice to have something like sanitize_for_mass_assignment from CanCan :)

[ryanb]

I'll be moving forward with this soon in a new 2.0 branch. Please see issue #191 for a good overview of the different changes planned for 2.0 and provide your feedback there.

[macek]

Awesome. I'm super excited for this :)

[ryanb]

Closing because this is now in the 2.0 branch. It may not be completely polished and fully working yet, but the functionality is there.

[ssoulless]

Hi, what happened with this? it seems to be abandoned, any new?

[Fedcomp]

@ssoulless use CanCanCan

[pedrocarmona]

reference new PR: CanCanCommunity/cancancan#474