#385 Authorization from Scratch Part 1 pro

Authorization in web applications can be difficult to implement and test as it often involves complex logic that needs to exist throughout all the application’s layers. There are libraries that can help out with this such as Ryan Bates’ CanCan, but as everyone’s authorization needs are needs are different these may or may not be a good fit. Either way it’s a good idea to learn how to implement authorization from scratch. This way we can adapt it to our needs and if nothing else we can get a better understanding of the ideas behind authorization.

We’ll be demonstrating this using a forum application. This has a number of topics and we can view a topic’s posts, create a new topic, edit a topic or destroy one. The application has some authentication and we’re already logged in with a username and password. Adding authentication form scratch was covered in episode 250.

Even though we have authentication set up we don’t have any authorization in this application. Users can edit or destroy any topic, even if they haven’t created it, but we don’t want this. We only want to allow admin users to be able to edit and destroy topics, although any user can currently be marked as administrator by setting a checkbox on their profile page.

Adding Authorization

Let’s get started by adding some restrictions to what the users can do. We’ll do this through test-driven development. We already have some high-level request specs for testing various parts of the application’s functionality. These tests are written with Capybara, which was covered in episode 275. We already have Guard set up for this project to automatically run the tests for us when we make changes to them. The four tests we have currently pass so we can start to add the new functionality. First we’ll add a test for authorization. We want to ensure that a user cannot edit a topic unless he is an admin so we’ll write a quick spec that will test for this.

it "cannot edit topic as non-admin" do
  log_in admin: false
  topic = create(:topic)
  visit edit_topic_path(topic)
  page.should have_content("Not authorized")
end

In this spec we log in as a non-admin user using a log_in support method that we’ve written for this application. We then create a new Topic and try to visit its edit page. When we do this the page should have the content “Not authorized” as the edit page should redirect back to the home page and show a message when a normal user tries to edit a topic. When we save this file Guard reruns our specs and we now have one that fails as the user can edit the edit topic page. We can get it to pass by modifying the TopicsController and adding a before_filter that will run before the edit action.

class TopicsController < ApplicationController
  before_filter :authorize, only: :edit

  # Other methods omitted

  private
  def authorize
    if current_user && !current_user.admin?
      redirect_to root_url, alert: "Not authorized"
    end
  end
end

The method that’s called by the filter checks to see if a current user exists. If one does and they’re not an admin they’ll be redirected to the home page and a flash message will be shown. With this change in place our specs pass again.

Our tests are giving us a false sense of security, though. There are plenty of holes in our authorize method. We check to see that a current user exists and if they’re an admin. This means that if we try to edit a topic while not logged in we’ll still be able to. Also we’re only checking this on the edit action which means that update isn’t protected at all. This means that someone could POST to update directly and still alter a topic.

These problems are easy to fix but we’ll need a add a separate spec for each of the branching possibilities and this will only get worse as our application’s logic becomes more complicated as we also want users to be able to edit topics that they own. This can be difficult to test, especially when we’re testing at such a high level, and isn’t very practical. Whenever we’re faced with this situation it’s a sign that we need to make the controller simpler and move the complex logic into the model layer so that we can test it at a lower level. Back in our TopicsController’s authorize method we’ll delegate any authorization logic into another object that we’ll call current_permission. This will have an allow? method that will return true if the permission is allowed for a given action. We can use this to redirect to the home page if it returns false.

def authorize
  if !current_permission.allow?
    redirect_to root_url, alert: "Not authorized"
  end
end

Next we’ll define the current_permission method. This will return an instance of a Permission class to which we pass the current user. We’ll assign the result of this to an instance variable and cache it so that it’s only instantiated once.

def current_permission
  @current_permission ||= Permission.new(current_user)
end

We need to write this Permission class and we’ll do it in the models directory. Even though this isn’t an ActiveRecord model it can still be considered a model. This class will inherit from a Struct for convenience so that we can pass in the user attribute. We’ll give this class an allow? method which we’ll give the same behaviour as we had in the controller where we check that the user is an admin.

class Permission < Struct.new(:user)
  def allow?
    user && user.admin?
  end
end

When we save this new file our tests all pass again as all we’ve done is some refactoring to move the logic into the model layer. We still have our security hole since we’ve only authorized the edit action. We’ll make this more generic and authorize every action in the same way and we’ll do this by moving the authorize and current_permission methods from the TopicsController into the ApplicationController along with the before_filter. We need to pass in some more information to allow? now so that it knows which controller and action we’re testing the authorization for.

class ApplicationController < ActionController::Base
  protect_from_forgery
  before_filter :authorize

private

  def current_user
    @current_user ||= User.find(session[:user_id]) if session[:user_id]
  end
  helper_method :current_user
  
  def current_permission
    @current_permission ||= Permission.new(current_user)
  end
  
  def authorize
    if !current_permission.allow?(params[:controller], params[:action])
      redirect_to root_url, alert: "Not authorized"
    end
  end
end

We’ll also need to modify the Permission class to accept the controller and action parameters.

class Permission < Struct.new(:user)
  def allow?(controller, action)
    user && user.admin?
  end
end

This will break our request specs since we’re now authorizing every action but this is one of those cases where it’s OK to have failing tests for a while as we’re spiking out the authorization functionality.

Whenever we’re building an authorization system it’s a good idea to start with the case where everything is locked down, except maybe the root URL, so that we can try it out. We’ll modify our Permission class to do this.

class Permission < Struct.new(:user)
  def allow?(controller, action)
    controller == "topics" && action == "index"
  end
end

Now when we try to visit any page apart from the home page we’re redirected back to the home page with a “Not authorized” message. This is a great place to be as it means that the authorization can now be controlled from one location. Our controller is simple and we can handle the complex logic through our Permission class and test it directly so that we don’t have to do the high-level request specs.

We’ll write a model spec for Permission now so that we can write some tests for it. In this spec we’ll require the spec_helper then describe the Permission class. Here we set focus to true so that only the tests inside this class are run. This is set up in the SpecHelper class so that only the focussed tests is run. If no test is set as focussed then all the tests are run. First we describe the case where the user is a guest, i.e that they aren’t signed in. The subject in this test is a Permission instance with nil passed in as the current user. We’ll then say that this user should have access to the topics index page.

require "spec_helper"

describe Permission, focus: true do
  describe "as guest" do
    subject { Permission.new(nil) }
    it { should allow("topics", "index") }
  end
end

This currently won’t work. When the test runs Guard complains about an undefined allow method as there’s no matcher with this name. The test reads well, though, so we’ll define an allow matcher. For convenience we’ll add this definition in the same class but it would be better in a file in the spec/support directory.

RSpec::Matchers.define :allow do |*args|
  match do |permission|
    permission.allow?(*args).should be_true
  end
end

Here we define our new matcher, which takes a Permission object that we call allow? on passing in the arguments that we passed to the matcher. We then check that this returns true. Our spec now passes as this is the one page that guests can view. We can now easily define the permissions for the other actions.

describe Permission, focus: true do
  describe "as guest" do
    subject { Permission.new(nil) }
    it { should allow("topics", "index") }
    it { should_not allow("topics", "show") }
    it { should_not allow("topics", "new") }
    it { should_not allow("topics", "create") }
    it { should_not allow("topics", "edit") }
    it { should_not allow("topics", "update") }
    it { should_not allow("topics", "destroy") }
  end
end

Guests should be able to view the index and show actions but not any of the others. When this test runs now if fails because guests don’t have permissions to view the show action. We can get this passing again by modifying the Permission object and adding show to the list of permitted actions.

class Permission < Struct.new(:user)
  def allow?(controller, action)
    controller == "topics" && action.in?(%w[index show])
  end
end

Our test now passes again so we’ll add permissions for admins. These should be able to access all the actions.

describe "as admin" do
  subject { Permission.new(build(:user, admin: true)) }
  it { should allow("topics", "index") }
  it { should allow("topics", "show") }
  it { should allow("topics", "new") }
  it { should allow("topics", "create") }
  it { should allow("topics", "edit") }
  it { should allow("topics", "update") }
  it { should allow("topics", "destroy") }
end

We pass in an admin user to Permission by using Factory Girl and then test each action to make sure that they can access it. Most of these tests all fail but this should be easy to fix.

class Permission < Struct.new(:user)
  def allow?(controller, action)
    if user.nil?
      controller == "topics" && action.in?(%w[index show])
    elsif user.admin?
      true
    end
  end
end

All our tests now pass again.

We have one more type of user left to describe: a logged in user who isn’t an admin. These should have access to everything except the destroy action.

describe "as member" do
  subject { Permission.new(build(:user, admin: false)) }
  it { should allow("topics", "index") }
  it { should allow("topics", "show") }
  it { should allow("topics", "new") }
  it { should allow("topics", "create") }
  it { should allow("topics", "edit") }
  it { should allow("topics", "update") }
  it { should_not allow("topics", "destroy") }
end

Some of these tests fail, as we’d expect. We can make it pass by making another change to the Permission class.

class Permission < Struct.new(:user)
  def allow?(controller, action)
    if user.nil?
      controller == "topics" && action.in?(%w[index show])
    elsif user.admin?
      true
    else
      controller == "topics" && action != "destroy"
    end
  end
end

With this in place the tests all now pass again. If we try this our by visiting various pages in our application while logged as a non-admin user it behaves exactly as we expect it to and we can visit all the pages but not destroy any topics.

It would make more sense if the destroy link only appears if the current user has permission to destroy a topic. We can find this link in the index template and we’ll wrap it with an if condition and use a helper method called allow? that takes a the name of a controller and an action and returns true if the current user has permissions on that action. We’ll use this action on the “new” and “edit” links, too.

<h1>Forum</h1>

<div id="topics">
<% @topics.each do |topic| %>
  <h2><%= link_to topic.name, topic %></h2>
  <%= "Sticky |" if topic.sticky? %>
  <%= pluralize topic.posts.size, "post" %>
  <% if allow? "topics", "edit" %>
  | <%= link_to "Edit", edit_topic_path(topic) %>
  <% end %>
  <% if allow? "topics", "destroy" %>
  | <%= link_to "Destroy", topic_path(topic), method: :delete %>
  <% end %>
<% end %>
</div>
<% if allow? "topics", "new" %>
<p><%= link_to "New Topic", new_topic_path %></p>
<% end %>

We need to define the allow? method which we’ll do in the ApplicationController by delegating it to the current_permission object and making it a helper method.

delegate :allow?, to: :current_permission
helper_method :allow?

If we reload the page now these links will disappear as the user we’re logged in as doesn’t have permission to view them.

We didn’t test-drive this change. For most application’s it isn’t necessary to test the view layer extensively with this kind of logic. If the “destroy” links were still there a non-admin user still wouldn’t be able to destroy a topic as the action itself is protected and that is covered by tests.

If we try to edit our profile now to make ourselves an admin user this won’t work as we aren’t authorized to view that page. We need to define permissions for actions in both the SessionsController and UsersController and we’ll start by adding some more tests to the Permission spec.

describe "as guest" do
  subject { Permission.new(nil) }
  # Other specs omitted.
    
  it { should allow("sessions", "new") }
  it { should allow("sessions", "create") }
  it { should allow("sessions", "destroy") }        
    
  it { should allow("users", "new") }
  it { should allow("users", "create") }
  it { should_not allow("users", "edit") }
  it { should_not allow("users", "update") }
end

A guest user should be able to access almost everything in these two controllers apart from being able to edit a user. A logged-in user will have similar permissions except that they will be able to edit and update users.

describe "as member" do
  subject { Permission.new(build(:user, admin: false)) }
  # Other specs omitted.
    
  it { should allow("sessions", "new") }
  it { should allow("sessions", "create") }
  it { should allow("sessions", "destroy") }        
    
  it { should allow("users", "new") }
  it { should allow("users", "create") }
  it { should allow("users", "edit") }
  it { should allow("users", "update") }    
end

We should add similar tests for an admin user, but given that they should have access to every controller and action we’ll replace all their tests with this one.

describe "as admin" do
  subject { Permission.new(build(:user, admin: true)) }
  it { should allow("anything", "here") }
end

When we save this file Guard runs again and we now have all sorts of failing specs so let’s work on getting them to pass. Trying to stick with the pattern we have in the Permission class is a little difficult as it will become more complicated as we add more controllers and actions to it. This is where explicit return statements can be useful in cleaning things up. We’ll use them to change the logic in the allow? method to look like this.

def allow?(controller, action)
  return true if controller == "sessions"
  return true if controller == "users" && action.in?(%w[new create])
  return true if controller == "topics" && action.in?(%w[index show])
  if user
    return true if controller == "users" && action.in?(%w[edit update])
    return true if controller == "topics" && action != "destroy"
    return true if user.admin?
  end
  false
end

All session actions should be available so we can return true if the controller matches “sessions”. For the other controllers we have logic based on the name of the action and whether there’s a current user and we return true if one of these lines matches. If not then we fall through to the bottom of the file and return false.

With this change made our tests all now pass again. Now that our authorization is pretty much done let’s see if our requests specs pass too. If we remove focus: true from the permission spec and save the file all the tests will be rerun. These tests all pass apart from the one that tests that we can’t edit a topic if we’re not an admin user. The problem here is that we accidentally wrote conflicting specs. In the Permission spec we said that a member should be able to edit a topic, while the equivalent request spec says the opposite and checks for the test “not authorized” on the page. Ideally we’d only want a member to be able to edit a topic that they have created but we won’t be doing that in this episode so for now we’ll change the request spec so that it says that guest users can’t edit a topic.

it "cannot edit topic as guest" do
  topic = create(:topic)
  visit edit_topic_path(topic)
  page.should have_content("Not authorized")
end

Now all the specs pass again and we have a pretty nice authorization system. There’s a lot more that we could do to improve it, however. It would be good if the Permission class had a DSL for defining permissions instead of all of the return statements we currently have. Also if we want to change the permissions depending on the attributes of a given topic, such as whether the user owns it or not that’s something our authorization system can’t currently handle. Some of this will be covered in the next episode.