Blog Archives

Using CanCan to implement an “Agree to Terms” workflow

Posted in Code, Inside TFG, Ruby on Rails

Terms & Conditions
Recently in a Rails application I was tasked with adding in a basic “terms and conditions” page.

There was nothing special about the feature, but I was really happy with my solution so I decided to write about it briefly.

Thinking about a solution

So, my initial plan was:

  1. Add a agreed_to_term_and_conditions_at datetime field to the User model. Use a datetime here so that if we change the conditions later we can check against the time.
  2. Perform checks in the app to prevent users that haven’t agreed to the terms and conditions will be redirected to the T&C workflow

Step 1 was straight forward, but step 2 got me thinking.

Initially I had considered implementing a before_filter in ApplicationController that would check whether the User had agreed to the T&Cs and redirect them to the T&Cs page if they hadn’t.

After thinking for a moment I decided that it was really a question of authorization, and as a result should be managed by an Ability file.

The reasoning I used is that I would say a user should not be able to access the site until they had agreed to the terms. That sounds suspiciously like a cannot statement in CanCanCan.

Implementing a solution with CanCanCan

Once I had decided to use CanCanCan to implement the solution, it was just a matter of getting all the parts together.

Firstly, I had my abilities split into separate files in the way I have suggested in this post. I had an Ability::Factory that would take a User (or nil) and return the appropriate ability file. It looks something like:

class Ability::Factory

  def self.build_ability_for(user)
    return Ability::Anonymous.new if user.nil?

    case user.role
    when :admin
      Ability::Admin.new(user)
    when :supervisor
      Ability::Supervisor.new(user)
    when :doctor
      Ability::Doctor.new(user)
    when :patient
      Ability::Patient.new(user)
    else
      raise(Ability::UnknownRoleError, "Unknown role passed through: #{user.role}")
    end
  end

end

My initial idea was to do some checks for each role and basically say something like:

if user.has_agreed_to_terms_and_conditions?
  # Implement abilities as per usual
else
  cannot :manage, :all
end

But that would lead to a lot of duplication in both implementation and tests. Plus, just a lot of code in general, which I despise.

Thinking further, I decided that a User who hadn’t agreed to the terms and conditions had a set of abilities of their own, independently to their role. I created a new Ability for such a condition: Ability::PendingAgreementToTermsAndConditions. The class was implemented like:

class Ability::PendingAgreementToTermsAndConditions < Ability

  def initialize(user)
    cannot :manage, :all
    can :agree_to_terms_and_conditions, User, id: user.id
  end

end

I amended my Ability::Factory so that it would return the pending ability in the right conditions:

class Ability::Factory

  def self.build_ability_for(user)
    return Ability::Anonymous.new if user.nil?

    if user.has_agreed_to_terms_and_conditions?
      ability_class_for(user.role).new(user)
    else
      Ability::PendingAgreementToTermsAndConditions.new(user)
    end
  end

private

  def ability_class_for(role)
    case role
    when :admin
      Ability::Admin
    when :supervisor
      Ability::Supervisor
    when :doctor
      Ability::Doctor
    when :patient
      Ability::Patient
    else
      raise(Ability::UnknownRoleError, "Unknown role passed through: #{user.role}")
    end
  end

end

Great, so now I had all the abilities I needed. It was time to incorporate the logic into my controller so the application would handle users who hadn’t agreed to the T&Cs.

I had to ensure two things:

  1. Users can’t access other pages in the app that aren’t the T&Cs. When they do, they will be redirected to the T&Cs page.
  2. When a user who hasn’t agreed to the T&Cs signs in, they are redirected to the T&Cs page.

Handling access to pages when terms and conditions aren’t agreed to

Regarding the first objective: Anyone who has used CanCan or CanCanCan will know that since the ability file prohibits users from accessing other pages (cannot :manage, :all), a CanCan::AccessDenied exception will be raised if those pages are hit.

That means that I just had to handle that exception, and redirect the user to the T&Cs page. The CanCanCan README explains how to catch this exception in detail, but I’ll post the code I used anyway:

class ApplicationController < ActionController::Base

  rescue_from CanCan::AccessDenied do |exception|
    if current_user.present?
      # You could also do: current_ability.can?(:agree_to_terms_and_conditions, current_user)
      # but I think the following reads better
      if current_user.has_agreed_to_terms_and_conditions?
        # Redirect as usual
      else
        # Redirect to the terms page
      end
    else
      # Do whatever for unauthed users
    end
  end

end

Moving on, let’s ensure the user isn’t sent straight to another redirect when they sign in.

Redirecting users to the terms and conditions page when they sign in

This is a problem for your authentication system. I use devise, so I was able to override the after_sign_in_path_for method in my ApplicationController as outlined in the documentation. The code looks like:

class ApplicationController < ActionController::Base

  # Override: Devise method
  def after_sign_in_path_for(user)
    # You could also do: current_ability.can?(:agree_to_terms_and_conditions, current_user)
    # but I think the following reads better
    if user.agreed_to_terms_and_conditions_at.present?
      # Redirect as usual
    else
      # Redirect to the terms page
    end
  end

end

Now the user will get one redirect, instead of being redirected to a page they can’t access.

Conclusion

So that’s my solution. About 20 extra lines of code (plus tests) and now you’ve got all the logic for implementing a terms and conditions workflow.

I really enjoyed implementing that solution. It was easy to write and has had no maintenance cost.

Separating abilities in CanCan

Posted in Code, Inside TFG, Ruby on Rails

Users and Authorization
Authorization is simple to implement in Rails thanks to gems like CanCan and its chubby cheeked offspring CanCanCan. When getting started with CanCanCan, the documentation suggests that you use the generator to create a single Ability file to store your abilities in. This is a great starting point, but in my experience few projects using CanCanCan ever evolve past the use of a single Ability file – much to their detriment.

In this post I’ll have a look at an example Ability file and enumerate some flaws in such a system. Following that I’ll discuss a way to improve it by breaking the Ability file out into multiple Ability files.

Defining a real-world example

Let’s imagine a somewhat complicated application that has:

4 different roles, that may have the ability to perform up to 50 different actions.

For some context, let’s say the application stores medical imaging scans (ultrasounds and so forth) and has the following roles:

  1. Patients that sign in and view information about their scans, and can grant access to their scans to doctors.
  2. Doctors that sign in and add notes and attach dictations to these scans, and can look over all their patients’ scans.
  3. Supervisors that sign in and manage the doctors, assigning them to hospitals and medical practices. Assigning patients to doctors.
  4. Admins that sign in and manage all the above user accounts, and perform basic CRUD for hospitals and medical practices.

An Ability file for such an application might look like:

class Ability
  include CanCan::Ability

  def initialize(user)
    # Anonymous users don't have access to anything
    return if user.nil?

    case user.role
    when :admin
      can :manage, :all
    when :supervisor
      # Between 1-50 can/cannot statements
    when :doctor
      # Between 1-50 can/cannot statements
    when :patient
      # Between 1-50 can/cannot statements
    else
      raise(Ability::UnknownRoleError, "Unknown role passed through: #{user.role}")
    end
  end

end

 

The problems with a single Ability file

As you can see, if we have many different abilities per user this file will get quite large.

Let’s say that there are very few shared abilities and that for each of the supervisor, doctor, and patient roles we have 50 lines of ability declarations. That equates to roughly 170 lines of code in the file. Historically, I’ve found that spec to implementation ratio is about 2:1, so let’s imagine there’s a 340 line spec file that corresponds to this implementation.

There are many problems with an Ability file of this size.

  1. Larger files are harder to understand, maintain, and debug. There are just too many different concepts for a developer to deal with in one location, many of which will be irrelevant for whatever their task at hand is.
  2. Spec files becomes even harder to maintain for larger implementations. Since spec:code ratio can bloat in excess of 2:1, it will be even harder to maintain the specs.
  3. The Ability file will have far too many responsibilities, violating the Single Responsibility Principle and suffering from the drawbacks of such behaviour. This boils down to a higher maintenance cost and defect rate. If you don’t like OO sophistry, let me put it another way: The file tries to do too much. The class answers the question “what can every user in the system conceivably do?”. Whereas, we are far more likely to be interested in what just one of those users can do at any given time.
  4. In a similar vein, the Ability class becomes a god class for authorization. I feel like CanCan and CanCanCan only encourage this behaviour by having an opaque default for determining which Ability to use. By default, CanCanCan will assume that you have an Ability class in its current_ability function. There is a section in the GitHub wiki on changing this, though.
  5. Large classes and files suffer from the broken windows theory – that is: since the class is already so massive and bloated, new developers on the project will just pile more functionality on top of the existing mess – despite the cost in readability and maintainability. Further, the scope of the class starts to spread as more and more code is tacked on. You might hear such excuses from developers as “I’m just following the convention of the existing app” or “look, it’s already screwed so it’s not like I can make it any worse”. You may also hear my favourite “we don’t have budget to refactor”. Yeah, if you don’t have budget to refactor imagine how much budget you DON’T have for fixing defects all the time because you keep piling shit on more shit.

Deconstructing the monolithic Ability file

In order to resolve the issues above, we must break the Ability file apart. Usually, my first tactic is to identify the responsibilities of the class and break the code out into classes that represent each of these responsibilities.

Let’s review the responsibilities of the Ability class above:

  1. It defines what an anonymous user can do (EG: handling user.nil?)
  2. It defines what an admin can do
  3. It defines what a supervisor can do
  4. It defines what a doctor can do
  5. It defines what a patient can do
  6. It handles unknown roles

Now we create a class for each of those responsibilities. Which would leave us with classes like:

  1. AnonymousUserAbility – basically a null object for abilities.
  2. AdminAbility
  3. SupervisorAbility
  4. DoctorAbility
  5. PatientAbility, and
  6. UserAbilityFactory (or Ability::Factory if using namespaces), which takes a User (or nil) and returns the corresponding Ability class above. This class also handles roles without defined abilities by raising an exception.

You may also like to keep an Ability file that includes the CanCan::Ability module and contains some shared functions that will be used in the other ability files.

You should store these files in app/abilities. They are not models as defined by MVC, so they don’t belong in app/models, which is where CanCan and CanCanCan stash the Ability file by default.

You may also like to namespace these classes (EG: Ability::AnonymousUser), since namespaces can also improve the organisation of an application.

An example of one of the Ability files is:

class Ability::Patient < Ability

  def initialize(user)
    [:show, :invite_doctor].each do |ability|
      can ability, Result, patient_id: user.id
    end
    # etc
  end

end

Now I can have private methods that are specific to just the abilities of the Patient, rather than private methods being for all the different roles. I have a single function that can tell me the sum total of a Patient’s abilities. I can include some additional documentation in the class explaining the role of the Patient in our application for future developers.

Let’s have a look at the Ability::Factory now. I named this class after the factory pattern since its job is to take a User (or nil) and build us a corresponding Ability file. If you wanted, you could just put the function in Ability. I prefer the new class implementation, which would look like:

class Ability::Factory

  def self.build_ability_for(user)
    return Ability::Anonymous.new if user.nil?

    case user.role
    when :admin
      Ability::Admin.new(user)
    when :supervisor
      Ability::Supervisor.new(user)
    when :doctor
      Ability::Doctor.new(user)
    when :patient
      Ability::Patient.new(user)
    else
      raise(Ability::UnknownRoleError, "Unknown role passed through: #{user.role}")
    end
  end

end

The corresponding controller change to get CanCan or CanCanCan to play nice with your new Abilities would be:

class ApplicationController

  # Override CanCan method to provide custom Ability files
  def current_ability
    @current_ability ||= Ability::Factory.build_ability_for(current_user)
  end

end

Please note: If you are using an additional engine like RailsAdmin or ActiveAdmin, some more work might need to be done in order to get the engine to play nice. You will have to do some spelunking the engine’s codebase to determine how CanCan or CanCanCan is integrated.

Conclusion

Now our large Ability file is broken into smaller, more manageable files. Each file now has a single responsibility and is easier to test. If we need to add a new role it won’t be a nightmare to patch the Ability file. We just build a new file and ensure it is in the Ability::Factory. Luckily, since our factory handles unknown roles by raising an exception, we’ll find out pretty quickly if there’s no corresponding Ability file.

Having a single file per role increases the ease with which we can verify the responsibilities of that role. We can read a single file and determine exactly what the Patient does, for example. Before, it was hidden in the guts of the Ability file.

When it comes to authorization, you want as high a level of visibility as you can on roles, so you don’t have anyone doing things they shouldn’t be able to.

Courtney’s Breakfast Farewell

Posted in Inside TFG

Nearly 3 years ago we hired Courtney de Lautour as a developer and he quickly rose to being a core member of our team. With tomorrow marking his final day with us, we figured it would be great to get out together as a team for breakfast at The Terrace Hotel and celebrate his time with us.

Fun team breakfast #powerfulteam

A photo posted by The Frontier Group (@thefrontiergroup) on

The last time we got together as a team for breakfast was back in 2013.

Solid start to the day and end of the week @frontiergroup @ruxton @jordan_maguire

A photo posted by Matt Lambie (@mlambie) on

Tech talks

Posted in Inside TFG

At The Frontier Group we like to keep things current and all of us love a good technical discussion.

Tech talks are a presentation based and topic focussed approach that’s been in the company for many years. Our resident doctor Steve Webb recently presented some of the approaches and complexities of WordPress integration with Ruby on Rails and this (as it often does) broke out into an interesting conversation about deployment, maintenance and testing methodologies.

TFG Tech Talk in Progress

Steve Webb discusses WordPress integration with Ruby on Rails

Over the years one of the most highly debated topics for us has been testing. Best practices, tool sets, approaches and our own stance on what has also been debated around the world.

Another topic that has grown in value over the last few years has been automation both for deployment and continuous integration. Getting code to our servers after lengthy automated testing is a constantly refined and extremely important process.

Does your company have tech talks? is there are topic of great interest to your team?

Search Posts

Featured Posts

Categories

Archives

View more archives