Authorization And You

I have been noticing a dangerous anti-pattern recently. It comes in the form of not enough authorization checking. When you have a function in your website that should only be available for particular users (administrators, moderators, whatever), you might be tempted to implement the code to restrict access once, in the view. Consider the following view to delete from a list of users (I snuck in the %: scriptlet tag which I've discussed before):

<ul>
  <% @users.each do |user| %>
    <li>
      <%: user.name %>
      <% if @me.can_delete_users? %>
        <a href="/users/delete?id=<%: user.id %>">delete</a>
      <% end %>
    </li>
  <% end %>
</ul>

This list might be accessible to everyone, but the deletion should be allowed only to privileged users. This could be a good argument to put all administrative features like this in their own actions/controllers, but there's really no need. Just be sure to check authorization again on the controller side:

class UsersController < ApplicationController
  before_filter :construct_me # Set up @me from session

  def delete
    raise "Nice try!" unless @me.can_delete_users?
    User.delete params["id"].to_i
    redirect_to "/users/list"
  end

  def list
    @users = User.find :all
  end
end

Did you see the extra check that the user is allowed to delete users from within the delete action? That is extra critical. It's easy to consider relying on security through obscurity. Who will try that URL to delete the user if they have no idea what the URL is and what parameters are needed? If the URL were less obvious, you may be even more strongly considering just hiding the URL. While the thought to reduce code clutter and duplicated work may be tempting, it leaves a gaping hole to anyone malicious who decides to start playing around with URLs, searching for administrative functions they shouldn't have access to. You may even get disgruntled former administrators/moderators/whatever who knew the URLs to decide to have some nasty fun with those functions they've been since blocked from.

Security is a hard thing to get right, but there's no excuse to succumb to laziness and knowingly leave dangerous holes in your system. The more powerful the function, the more on edge you need to be for possible ways to break your security.

Note that my snippets include an XSRF hole, but that's a problem for another post.