Moving to a safer password solution

Manfred Stienstra, 15 Feb 2010, 16:05 in ruby on rails, practices, and ruby (edit).

In an application we wrote back in 2004 I found MD5 hashed passwords. We decided this was too weak for modern standards so we wanted to switch to bcrypt. During the move we wanted the user to be affected as little as possible.

In order to compute the crypted password we need the cleartext version. We only have a hashed version so the user has to type her password. Luckily they do this every time they authenticate, so that is a nice opportunity to upgrade their password.

First I added a crypted_password column to the accounts table. We now have two columns for storing the password: the old hashed_password and the new crypted_password.

add_column :accounts, :crypted_password, :string

After that we updated the password accessor methods; assignment and verification.

class Account
  def password=(password)
    if new_record? or !password.blank?
      self.crypted_password = BCrypt::Password.create(password)
    end
  end
  
  def has_password?(input)
    BCrypt::Password.new(crypted_password) == input
  rescue BCrypt::Errors::InvalidHash
    false
  end
end

Now need to make sure we can authenticate with both the hashed as well as the crypted password stored for an account.

class Account
  def self.authenticate_with_crypt(params={})
    if account = find_by_username(params[:username]) and
       account.has_password?(params[:password])
      account
    end
  end
  
  def self.hash_password(password)
    ::Digest::MD5.hexdigest(password)
  end
  
  def self.authenticate_with_md5(params={})
    find_by_username_and_hashed_password(
      params[:username],
      hash_password(params[:password])
    )
  end  
end

Finally we need to make sure the password automatically updates. We try to authenticate using bcrypt. BCrypt raises an exception when the crypted_password is blank. This makes authentication fail and we fall back to trying the hashed password. When authentication with a hashed password succeeds we know the cleartext password and we can update it.

class Account
  def self.authenticate(params={})
    if account = authenticate_with_crypt(params)
      account
    elsif account = authenticate_with_md5(params)
      account.password = params[:password]
      account.hashed_password = nil
      account.save!
      account
    else
      account = Account.new(params.slice(:username, :password))
      account.errors.add_to_base("The credentials you entered are"
        "invalid. Please try again.")
      account
    end
  end
end

This solution will leave a group of users with a hashed password indefinitely. After a few months we could decide to throw away the hashed passwords. This means that infrequent users will have to reset their password if they do decide to log in again. It could cause some support requests, but I think we can handle them.

Note that this change doesn’t make the application safer. In case we leak information or when the database is somehow stolen it will make it harder to recover passwords. A lot of people use the same password for multiple accounts so this will give them time to reset their other accounts in case it is compromised. The change only took 15 minutes in this application so it’s totally worth the time.

2 comments

Highly interactive JavaScript widgets

Manfred Stienstra, 16 Oct 2009, 13:58 in portfolio, javascript, and practices (edit).

For a project we worked on recently the user needed to categorize works of art into disciplines and movements and tag them. We developed a little control panel that is easy to use for first time users and powerful for daily users.

We used standard form elements to make them recognizable and implemented functionality on top of that using JavaScript.

A lot of stuff happens in that video so I’ll explain a bit about what you’re seeing. You can add new items using either the drop down or the text field. Once the item is added, you can toggle it using the check box. Text fields allow you to add any value, but assist after a while by suggesting values previously filled out by others.

We don’t use spinners for background processing because it’s visually distracting, instead we add something to the interface when you add something and disable check boxes when they’re active.

We’ve made sure the panel stays consistent with the information on the server and kept interaction with the server at a minimum. When you select the same item twice nothing happens. If you select a previously unchecked item it doesn’t create a new item but instead it checks the item already on the page. If any communication with the server fails, the controls are reset to their previous state.

Finally, we make sure ‘Browse’ tab is always up to date, otherwise the user might think we lost her carefully filled out information.

No comments yet

Helpers are for small snippets of code

Manfred Stienstra, 04 Jul 2007, 15:49 in ruby on rails and practices (edit).

Recently we contracted a local company to help out with a Rails project. Their primary job was to implement payments. During an audit or their work I found the following helper:

module CoursesHelper
  include ActiveMerchant::Billing 
  def issuers
    @gateway = IdealGateway.new(
         :merchant            => IDEAL_MERCHANT_ACCOUNT,
         :sub_id              => IDEAL_SUB_ID,
         :password            => IDEAL_PRIVATE_KEY_PASS,
         :private_cert        => IDEAL_PRIVATE_CERT,
         :language            => IDEAL_LANGUAGE,
         :private_key         => IDEAL_PRIVATE_KEY,
         :authentication_type => IDEAL_AUTHENTICATION_TYPE
    )
    
    list = Array.new()
    begin
      response = @gateway.issuers
      if response.success?
        list=Array.new()
        list.push({"issuerName"=>"Kies uw bank...", "issuerID"=>0})
        list=list + (response.params["list"])
      end
    rescue
      list = [{:issuerName=>"Kies uw bank...", :issuerID=>""},
      {:issuerName=>"ofline Simulator", :issuerID=>"00"}]
    end
    return list.map {|i| [ i["issuerName"], i["issuerID"] ] }

  end
  
end

Helpers are for keeping simple code out of your view. They should never include complicated logic. A helper should be so simple that you’d almost dare to deploy without testing.

Also, because it’s a good idea to keep coupling between classes to a minimum, the payment gateway should only be called from classes directly related to payment processing.

Finally, you don’t want such a large number of constants in your code. There are better ways to store the gateway configuration.

The contracter was asked to solve these issues. The gateway initialization was refactored to only appear once and the gateway object was assigned to a constant named IDEAL_GATEWAY. The code to find all issuers was moved to the Payment model.

class Payment < ActiveRecord::Base
  def self.ideal_issuers
    response = IDEAL_GATEWAY.issuers
    response.params["list"]
  end
end
module CoursesHelper
  
  def issuers
    list=Array.new()
    # 'Kies uw bank' means 'Choose your bank'
    list.push({"issuerName"=>"Kies uw bank...", "issuerID"=>0}) 
    list=list + Payment.ideal_issuers
    return list.map {|i| [i["issuerName"], i["issuerID"]] }
  end
  
end

Unfortunately, the issuers helper still looks more like Python than Ruby. I would have written it like this:

module CoursesHelper
  def issuers
    [['Kies uw bank...', 0]] + Payment.ideal_issuers.map \
      { |i|[i['issuerName'], i['issuerID']] }
  end  
end

I don’t think you should include instructions on how to use a drop-down as one of the options inside it. Furthermore, a default value makes no sense in this case. People always have to choose a bank, otherwise they can’t continue with the payment process.

After some refactoring I was left with the code below. I’ve also moved it to the helpers module for the payments controller, because that’s where the form is rendered.

module PaymentsHelper
  def issuers
    Payment.ideal_issuers.map { |i| [i['issuerName'], i['issuerID']] }
  end
end

8 comments

Adaptive planning

Thijs van der Vossen, 02 Nov 2006, 20:19 in practices (edit).

I very much like Martin Fowler’s latest Bliki entry on FeatureDevotion:

The key to beating off the waterfall is to realize that, as Dan puts it, agilists value Outcomes over Features. The feature list is a valuable tool, but it’s a means not an end. What really matters is the overall outcome, which I think of as value to the customers.

An important part of this thinking is that you expect the feature list to change as the project goes on. This happens you discover new things that you can do, and re-prioritize old things. This is the essence of adaptive planning, which has always been a key indicator of agile thinking. This results a big shift in how people think about a plan. In plan-driven projects, success and failure is often worded in terms of “did things go according to the plan?” In agile projects this is a meaningless question, because plans change so often. The plan is a tool, primarily one that you use to gauge the effect of changes: “how will adding this feature affect what we do”. The plan is a tool to figure out what should fit in the FivePoundBag. If your plan’s not constantly changing, you are very unlikely to be doing adaptive planning, and hence aren’t agile.

No comments yet

Getting Real, the book now free

Thijs van der Vossen, 26 Oct 2006, 09:11 in web, practices, and design (edit).

Getting Real, the book, self-published in PDF format this march by 37signals, is now also available as a paperback and in a free html version.

I never finished reading the pdf version because I find reading PDF documents on a computer screen really annoying.

Reading a well-designed html document from screen is a much, much better user experience; it’s easy to change font size, the position and width of page elements changes to fit the browser window and you can just scroll down and keep on reading without being interrupted by page boundaries.

For reading from screen, the html version of Getting Real just works better.

One thing is driving my crazy though; why on earth haven’t they used curved quotation marks in the html version? Proper punctuation just doesn’t matter anymore?

2 comments

Screencast Scripting

Manfred Stienstra, 09 Oct 2006, 22:07 in ruby on rails, practices, and video (edit).

Last week I posted a short screencast to show some features of ActiveSupport::Multibyte. After typing through the entire screencast twice I decided to automate the process. Screenager, the automated screencast typer, was born.

Download screencast (QuickTime, 544KB)

You can download Screenager from my personal Subversion repository, you will also need a recent version of ActiveSupport.

svn export https://dwerg.net/svn/screenager/trunk screenager
cd screenager
svn export --force http://svn.rubyonrails.org/rails/trunk/activesupport/lib
./screenager --speed 2 http://www.fngtps.com/files/2/2006/10/activesupport.rb

The version currently in SVN evaluates the Ruby code with eval using a clean binding every time you start a new screenplay. I really wanted to use the Freaky Freaky Sandbox for this, but it’s in heavy development and didn’t run at all when I was coding this. Sandbox and multiline Ruby statements are planned for future versions.

16 comments

It sounded so good on the golf course

Thijs van der Vossen, 16 May 2006, 13:24 in ruby on rails and practices (edit).

Martin Fowler in ’Evaluating Ruby’ (emphasis added):

Too many things are hard to judge that way - hence we spend so much of our time on client projects being slowed down by technology that sounded good on a golf course. My preference is to make this judgment based on experience - find people who have a track record for delivering in the mainstream environments and who have tried using Ruby.

No comments yet

Pair programming

Thijs van der Vossen, 10 Mar 2006, 14:41 in practices (edit).

Martin has taken some baby steps toward pair programming. One thing holding them back is that they work on al lot of different projects, so it’s rare for two people to be assigned to the same thing.

Although we’ve taken a lot of our practices from Extreme Programming, we don’t do full-time pair programming for much of the same reasons. I don’t see how pairing is going to help me when I’m designing a user interface, or when I’m doing server maintenance.

Having said that, we find ourselves programming in pairs more and more often. It really is much faster to solve complex problems together. It also works great for refactoring, because it’s much easier to keep track of all the changes when you’re working as a pair.

The only problem with pair programming is that it’s hard to explain to people who have no experience with it. Why would you want to have two programmers working on something that one can do alone? Isn’t that just wasting valuable programming hours?

Although there is a lot of evidence suggesting that it’s not, we’ve sidestepped the issue somewhat by only pairing for short periods of time when it’s really clear that it helps the project going forward.

No comments yet