Roles Testing For Security

Noel Rappin, Friday, October 10, 2008 @ 3:32 pm

Flickr Image By Darwin Bell

Every web site that has some concept of user login also has some concept of user roles. Administrators have special editing access or behind the scenes report views. Users with different relationships have different ability to view the other user's data, and so on. In my experience, every site's requirements along this line are just different enough that a one-size-fits all plugin approach to roles never quite works.

That said, there are some common principles that I've found helpful in implementing roles and security.

When blocking entire controller actions, use filters

This one should be pretty familiar, since the restful_authentication plugin recommends placing before_filter :login_required at the top of any controller that requires the user to be logged in. Similarly, you'll want something like before_filter :admin_required, nly => [:create] for the controller actions that require administrative users.

Before filters are simple and powerful. It's also very easy to forget to add a new action to the nly list (especially for non-RESTful controllers). So, be sure and test with something like this, assuming that logged_in? and the home controller are defined.

should "not let regular users see the edit page" do
  login_as :normal_user
  post :create
  assert not logged_in?
  assert_redirected_to :controller => "home", :action => "index"
end

Note this test also asserts that a user who tries to access a forbidden page is logged out. You probably also want to test for the case where nobody is logged in -- at least where the functionality isn't already covered by restful_authentication or whatever login package you are using.

Also, the Footnotes plugin contains a tab for checking all the filters called on a given request, which is very helpful when trying to debug weird behavior.

When blocking part of a page, try a block helper

Another common role based security need is to only expose part of a page to a particular user role. For instance, only an administrator or the actual poster can edit a forum post. What I like to do here is combine partials and block helpers, with a helper that looks like this:

def if_current_admin
  yield if current_user.admin?
end

And an ERb file that looks like this:

<% if_current_admin %>
  <%= render :partial => admin_edit_content %>
<% end %>

(Alternately, if the partial is always going to be restricted, the if statement could go in the partial).

Unfortunately, if you have if/else functionality, the block helper is less pretty because you wind up with something like this:

<% if_current_admin %>
  <%= render :partial => admin_content %>
<% end %>
<% unless_current_admin %>
  <%= render :partial => user_content %>
<% end %>

Which is a lot less nice than:

<% if current_admin? %>
  <%= render :partial => admin_content %>
<% else %>
  <%= render :partial => user_content %>
<% end %>

The advantage of moving all this stuff into guarded partials it it keeps your main page somewhat clean and makes it easier to figure out what's going on.

Use negative assert_select testing

When doing a guarded view block like the ones above, you also should test to make sure that the correct content is displayed and that the incorrect content is not displayed. The assert_select function lets you assert that content is not in a result, which is helpful.

Let's say you have two tests, and assume that everything's been defined properly:

should "show admins the administrative content" do
  login_as users(:admin)
  get show, :id => thing(:one)
  assert_select "div#admin_only_content"
end

should "show users the user only content" do
  login_as users(:normal_user)
  get show, :id => thing(:one)
  assert_select "div#user_only_content"
end

The tests would still pass even if the user gets the admin_only_content div. You can avoid that potential problem by using the :count option of assert_select like so:

should "show admins the administrative content" do
  login_as users(:admin)
  get show, :id => thing(:one)
  assert_select "div#admin_only_content", :count => 1
  assert_select "div#user_only_content", :count => 0
end

should "show users the user only content" do
  login_as users(:normal_user)
  get show, :id => thing(:one)
  assert_select "div#admin_only_content", :count => 0
  assert_select "div#user_only_content", :count => 1
end

In the second batch of tests, you are explicitly asserting that the incorrect DOM element is not in the output. This is a very handy feature when testing for behavior under different roles, I use this all the time.

Put active record guards in the model

Eventually, the question of whether active record content can be saved by a particular user is a business logic question that should be handled by the model layer. I don't think this can be made totally goof-proof (you might be able to do something with callbacks, but I haven't hit on the magic words yet). However, you can promote an API that makes it easier for your development team to do the right thing:

def self.create_from_params_if_user_can(current_user, params = {})
  return unless self.user_can_create(current_user)
  create(params)
end

From the controller, this would look like:

def create
  Thing.create_from_params_if_user_can(current_user, params[:thing])
end

Putting a guard in the ActiveRecord class may seem redundant, if you have a good before filter in place, then the create method will only be called by valid users. However, as your application gets more complicated, the create method may be called by different user types, each with different rights to do different actions. Alternately, Thing objects might be created from other places. Keeping the actual model rights logic in the model makes it easier to keep the logic clear and straight.

Don't take inputs for granted

Even in cases where the user ID might be passed back via a form or URL parameter, you still should use the stored session current user to determine rights (with the obvious exception of the actual login where you are authenticating with a password). You want to prevent somebody from hijacking the session by passing in the parameter of a user with more or different rights.

Along the same lines, when checking for objects via a relationship, always check via the association proxy. That is, do this:

current_user.articles.find_by_active(true)

And not this:

Articles.find_by_user_id_and_active(params[:user_id], true)

This limits the ability of a user to URL surf.

Related posts:

1. Testing various roles in ruby on rails
2. DRYing up Rails Controllers: Polymorphic and Super Controllers
3. Hide, Seek, and Stay Dry, part two: Controllers
4. ActiveRecord create_or_update based on natural-key
5. Integrating Design Drafts Into Your Rails App

Comments (2)

Software Development

Comments: 2 so far

• Leave a comment
• Comment feed for this post
• TrackBack URL

1. 

   Nice and usable idea, but it can be even simpler and chainable too

   def admin?
   current_user and current_user.admin?
   end

   if admin?
   xxx
   end

   Thing.create_from_params_if_user_can(current_user, params[:thing])

   why should all things know about the user, let the user know what he can or cannot, so the knowledge is at one place(and mostly the logic is the same, so everything can be handled in an case statement)

   Thing.create(params[:thing]) if current_user.can_create?(Thing)

   and in the views, the new link is only shown if the user can create this thing

   The idea is outlined here: http://pragmatig.wordpress.com/2008/06/29/separate-rights-management-from-controllers/
   and some helpers that are based on this concept here: http://pragmatig.wordpress.com/2008/07/09/generic-smart-link_to_s-link_to_edit-link_to_destroy/

   Comment by grosser, Saturday, October 11, 2008 @ 12:54 pm

2. 

   Hi,

   I like your idea of using block helpers to limit access to certain parts of a view.

   I describe an alternative approach to checking permissions at the ActiveRecord level in my post RestFul permissions in Rails

   Comment by Jo Hund, Sunday, October 12, 2008 @ 6:03 pm

Leave a comment