Idiomatic password encryption.

[IAIHMB]

Let's talk theory, specifically, from the model's point of view in a user management system. What is the idiomatic way of handling password encryption? Basically, I'm shooting for:

-Validation before the password is encrypted.
-Password encryption when a user is created or said user's password is updated.

Nothing to complex, but I can think of a hand full of solutions, none of which seem ideal:

1) Take the syntactic sugar approach.

def password=(unencrypted_password)
  self.salt = generate_salt
  self.password_hash = Digest::SHA1.hexdigest(unencrypted_password + self.salt)
end

This would be good because the password is encrypted whenever it's set, however it makes validation a hassle. One could validat the encrypted_password parameter, but that seems half baked.

2) Use callbacks.

after_validation :encrypt_password

There are a number of possibilities here, this would be good because you could validate the user submitted password and then encrypt it. However, it would make subsequent validations inaccurate (If you were validating the password for a length between 5 and 30 characters, a 40 character SHA1 hash and an update would break things.). Another play on this one, pass around a constant, REQUIRES_ENCRYPTION perhaps.

I've been playing with this for a couple of hours now, but I still cannot find a solution that I'm happy with. No luck with Google, Agile Web Development with Rails and Rails Recipes only cover authentication and authorization. How are you guys going about things? Thanks for any insight.

Edit: Another play on number two, I like this idea a bit better. Limit the user submitted password to 30 characters or so. Since an SHA1 hash is 40 characters the password's length could be checked in a callback, if it's anything but 40 characters it could be encrypted. Validating the user submitted password (Prior to encryption.) would still be odd though.

[Luke Redpath]

Here's how I handle it. Database has a crypted_password and a salt column. Model has a non-database attribute, @password. So:

class User < ActiveRecord::Base
  attr_accessor :password

  validates_confirmation_of :password, :if => :password_required?
  validates_length_of          :password, :in => 1..20, :if => :password_required?

  before_save :encrypt_password

  def self.authenticate(login, password)
    user = User.find_by_email(login)
    return false unless user
    return user if user.authenticate(password)
    return false
  end
  
  def authenticate(password)
    return true if (encrypt(password) == self.crypted_password)
    return false
  end

  protected
    def encrypt_password
      return if @password.nil? or @password.blank?
      self.salt = Digest::SHA1.hexdigest("--#{Time.now.to_s}--#{@password}--")
      self.crypted_password = encrypt(@password)
    end
    
    def encrypt(string)
      Digest::SHA1.hexdigest("--#{self.salt}--#{string}--")
    end

    def password_required?
      crypted_password.blank? or not password.blank?
    end
end

class LoginController < ActionController::Base
  def signup
    # params include :password and :password_confirmation fields
    @user = User.new(params[:user)
    if @user.save
       # do something
    end
    # do something else
  end

  def login
    @user = User.authenticate(params[:username], params[:password])
    if @user
      # user logged in
    end
    # user failed login
  end
end

If you find yourself using the above method frequently, its best to wrap it up as a plugin.

[IAIHMB]

Thanks for the response, I'll have to check it out.

[Luke Redpath]

FWIW the above code is also unit tested, if you would like the unit tests let me know although its a good exercise to try and write them yourself

[IAIHMB]

Thanks again mister Luke, I appreciate it. I took your approach, the only difference thus far is style (And lack of authorization.). I wouldn't mind seing your tests.

So far I've only 2 tests and 9 assertions. Is it just me, but you cannot append logic to callback handlers can you? Ala:

# 1
before_save :encrypt_password, :if => :encryption_required?
# 2
before_save :encrypt_password if :encryption_required?

Thanks again.

[Luke Redpath]

No you can't, thats why I just do the checks in the callback.

I don't have the unit tests available at the moment, they are at work I'm afraid, and I'm off on holiday tonight. From what I recall there are probably a couple of test cases, with about 4/5 tests per case, and one or two assertions per test (I write my tests in a Behaviour-Driven style - one test case per context, as few assertions per test as possible).

[IAIHMB]

No worries, you've given me enough already. I've reworked it a hand full of times now, I think I'm fairly satisfied with this. Anyone else come up with a method that they are particularly fond of?