Analysis paralysis: access control

thoughtbot-giantrobots:

Have you ever analyzed the “access control” (also called “authorization”) options in Rails and thought, “something better must be out there”?

I’ve analyzed libraries and concluded “authorization needs to be custom in each app.” Many will disagree.

If you’ve reached the same conclusion, however, your decisions are not over:

  • Do you test access control in Cucumber? … or controller specs/functional tests?
  • Does it belong at the model or controller layer?
  • Are before filters, flash messages, and redirects the right tools?

I’m happily considering the following approach as my new standard. What do you think?

Flat routes

map.resources :accounts, :only => [:new, :create, :show]
map.resources :brands, :only => [:new, :create, :show] do |brands|
  brands.resources :offers, :only => [:new]
end

Brands belong to accounts. Offers belong to brands. Users belong to accounts.

Prefer flat routes (and no subdomains) when at all possible. It keeps the mental overhead low everywhere in your app. Read: new_brand_path versus new_account_brand_path(@account). Stop forgetting to include :account_id as an a parameter in your controller specs / functional tests.

Made possible by authentication

We’re using the domain model to simplify the app’s implementation. Users are authenticated using Clearance. They’ve got a simple account_id foreign key.

This means that with an authenticated user in a typical “account” application, we can lean on Clearance’s :authenticate before filter and regular ActiveRecord finders.

class BrandsController < ActionController
  before_filter :authenticate

  def new
    @brand = current_user.account.brands.build
  end

  def create
    @brand = current_user.account.brands.build(params[:brand])
    # ...
  end

  def show
    @brand = current_user.account.brands.find(params[:id])
  end
end

Don’t repeat RESTful conventions

This is repetitive, however… and who writes RESTful controllers by hand these days? Check out how nice the same thing looks with Inherited Resources:

class BrandsController < InheritedResources::Base
  before_filter :authenticate

  actions :new, :create, :show

  protected

  def begin_of_association_chain
    current_user.account
  end
end

With this pattern, we do not have to test access control on the new and create actions because the user will only be able to create brands for their account.

Looking back at the routes and our domain model, however, we will still need to test access control for creating offers that belong to brands.

Test at the controller level

should "not find brands not associated with signed in user" do
  brand    = Factory(:brand)
  sign_in_as Factory(:user)

  assert_raises(ActiveRecord::RecordNotFound) do
    get :new, :brand_id => brand.to_param
  end
end

Done for the new action.

Rails returns a 404 when ActiveRecord::RecordNotFound is raised. This error will be raised in our access control scheme because there is no record of the current_user having a relationship to this brand. Pretty reasonable, right?

Let’s get to green:

class OffersController < InheritedResources::Base
  before_filter :authenticate

  belongs_to :brand

  def begin_of_association_chain
    current_user
  end
end

Magic?

How did this last association chain work? Inherited Resources knows Offers belong to Brands and to start the chain with current_user. This is the equivalent of:

@brand = current_user.brands.find(params[:brand_id])
@offer = @brand.offers.build

If you were really paying attention, you noticed that User belongs to accounts and Account has many brands. I could have said current_user.sponsor but I kept the chain from the perspective of the controller shorter using delegation:

class User < ActiveRecord::Base
  include Clearance::User

  belongs_to :account
  delegate   :brands,  :to => :account
end

This may seem pedantic but it will make my life easier when the rules around users’ relationship to brands get more complex, which is in the plans based on stories in Pivotal Tracker

Too lightweight?

I’m digging this approach from a few perspectives. It is not a lot of lines of code. It leans heavily on the framework, stays DRY, and uses normal authentication and RESTful conventions. It’s easy to test and I know where those tests should go.

What do you think?