Adding Some Additional Security Measures to restful_authentication

{ Posted on Mar 29 2009 by Dan }

If you’ve been paying attention to the web lately, you’ve probably heard of twitter and probably also remember hearing about this. Well, I’ve been wanting to implement something that would have prevented that hack into restful_authentication for a long time and I finally got around to it. I’ve been sitting on this for a bit because I wasn’t sure if I was ready to show it to the world, but I decided to just write about and release what I have thus far. So I present to you, my first iteration of locking out user accounts after too many incorrect login attempts. There are many ways to go about doing this, but the solution I settled on was to just lockout user accounts that have too many incorrect login attempts over a certain period of time. Its pretty simple really. This is done with the addition of one more table which is used to record incorrect logins.

For my example I’m going to spare you the details I used to get this running and only look at the lockout code. Click here to go directly to the github repo. I’m going to assume that you already have restful_authentication installed and running. First thing to do then, is generate the model that will track incorrect login attempts, as well as add an extra field in the users table to keep track of when the account was locked out. Just run…
./script/generate model login_attempt…and make the migration look like this…

class CreateLoginAttempts < ActiveRecord::Migration
  def self.up
    create_table :login_attempts do |t|
      t.integer :user_id, :null => false
      t.string :remote_ip, :user_agent
      t.timestamps
    end
    add_column :users, :locked_out_at, :datetime
  end
 
  def self.down
    drop_table :login_attempts
    remove_column :users, :locked_out_at
  end
end

…and then migrate the database with…
rake db:migrateFrom here, pretty much everything is in the sessions_controller.rb and user.rb files. For the user model, we need to add a way to set lockout options (I just use a constant, but you could add another model to keep track of of these options too), another state and some transitions, and a couple of methods to determine the state of the user in the lockout process. I’ll just paste my user.rb file, sans the validation methods that would just be taking up unnecessary space here.

require 'digest/sha1'
class User < ActiveRecord::Base
  include Authentication
  include Authentication::ByPassword
  include Authentication::ByCookieToken
  include Authorization::AasmRoles
 
  # here is how I setup the lockout options 
  LOCKOUT_OPTIONS = {
    :lockout_period => 1, # in minutes
    :login_attempts => 3, # number of tries
    :attempt_window => 4 # can have x number of login_attempts in this many minutes
  }
 
  has_many :login_attempts
 
  named_scope :with_allowed_states, :conditions => { :state => [ 'suspended', 'locked_out', 'active' ] }
 
  # A locked out account is different from a suspended account, so these are necessary
  aasm_state :locked_out, :enter => :set_locked_out_at
  aasm_event :lock_out do
    transitions :from => :active, :to => :locked_out
  end
  aasm_event :end_lock_out do
    transitions :from => :locked_out, :to => :active
  end
 
  # HACK HACK HACK -- how to do attr_accessible from here?
  # prevents a user from submitting a crafted form that bypasses activation
  # anything else you want your user to change should be added here.
  attr_accessible :login, :email, :name, :password, :password_confirmation
 
  def max_login_attempts?
    self.login_attempts.by_attempt_window(LOCKOUT_OPTIONS[:attempt_window]).all.size > LOCKOUT_OPTIONS[:login_attempts]
  end
 
  def lock_out_ended?
    self.locked_out_at < (Time.now - LOCKOUT_OPTIONS[:lockout_period].minutes)
  end
 
  def login=(value)
    write_attribute :login, (value ? value.downcase : nil)
  end
 
  def email=(value)
    write_attribute :email, (value ? value.downcase : nil)
  end
 
  def admin?
    self.role == 'admin'
  end
 
protected
 
  def set_locked_out_at
    self.locked_out_at = Time.now
  end
 
  def make_activation_code
    self.deleted_at = nil
    self.activation_code = self.class.make_token
  end
end

Next thing to do is to edit the sessions controller. Open it up and make it look like…

# This controller handles the login/logout function of the site.  
class SessionsController < ApplicationController
  # render new.rhtml
  def new
  end
 
  def create
    logout_keeping_session!
    @login       = params[:login]
    @remember_me = params[:remember_me]
    @user = User.with_allowed_states.find_by_login(@login)
    if @user
      case @user.aasm_current_state
      when :suspended
        flash_and_render(:error, "Your account is suspended")
      when :locked_out
        if @user.lock_out_ended?
          LoginAttempt.delete(@user.login_attempts)
          @user.end_lock_out!
          proceed_to_login
        else
          flash_and_render(:error, "Your account is locked out")
        end
      else # is active
        proceed_to_login
      end
    else
      flash_and_render
    end
  end
 
  def destroy
    logout_killing_session!
    flash[:notice] = "You have been logged out."
    redirect_back_or_default('/')
  end
 
protected
 
  def proceed_to_login
    if @user.authenticated?(params[:password])
      self.current_user = @user
      LoginAttempt.delete(@user.login_attempts)
      new_cookie_flag = (params[:remember_me] == "1")
      handle_remember_cookie! new_cookie_flag
      flash[:success] = "Logged in successfully"
      redirect_back_or_default(root_path)
    else
      @user.login_attempts.create(:remote_ip => request.remote_ip, :user_agent => request.user_agent)
      if @user.max_login_attempts?
        @user.lock_out!
        flash_and_render(:error, "Your account is locked out")
      else
        flash_and_render
      end
    end
  end
 
  def flash_and_render(type = :notice, message = "Incorrect username or password")
    flash[type] = message
    # done to clear out unneeded flash values since this isn't a redirect...know a better way to do this?
    flash.delete_if {|key, value| key != type }
    render :action => 'new'
  end
end

And thats about it. User accounts now get locked out for however many minutes you specify after however many login attempts you deem are inappropriate. In my example I have them set really low for testing purposes, but in the real world, I would probably up the login attempts to 10 or 15 tries and the lock out time for 10 to 15 minutes to balance not annoying your users and securing your site. But, that’s really up to you to decide whats appropriate in your case.

My sample app that uses this code is available on github here. I have some additional plans, but wanted to get this posted before I took any longer. For one, if a user mistakenly locks themselves out of their account, they should be able to click on a link to reset their password and not wait for their account to let them log in again and I would also like to restrict the IP address of users who lock out multiple accounts within a certain amount of time. I also want to get this integrated into restful_authentication so its just as easy to have account lockouts as it is to setup restful_authentication. As always, comments and suggestions are welcome.


3 Responses to “Adding Some Additional Security Measures to restful_authentication”

  1. I’m making similar additions to restful_authentication (without aasm) but I’m planning to have LoginAttempt keep a full login history instead of just the most recent failed attempts.

    I’m somewhat of a rails noob and so I’m wondering where “by_attempt_window” comes from? I tried adding the following to my user model:
    named_scope :by_attempt_window, lambda { |time_ago| { :conditions => ['created_at > ?', time_ago] } }
    but I still get undefined method `by_attempt_window’.

    This is a good project, I agree that restful_authentication needs this.

  2. I thought about keeping the full login history, but I ultimately decided against it because keeping track of all of them could cause the database to grow fairly large and then you’d probably need a cron job of sorts to remove records that are x days or so old, so this was just a simpler solution.

    by_attempt_window is a named_scope of the LoginAttempt model, so you’ll want to place it in there.

    Thanks for the comment!

  3. thanks for this it was an interesting read.

Post a Comment

Powered by WP Hashcash