Improve the Authenticator ?

[gnomex]

I don't like this code:

  def valid_password?(password, password_from_database)
    return false if password_from_database.blank?
    magic = password_from_database.split('$')[1]
    case magic
    when /\A2a?\z/
      valid_password_with_bcrypt?(password, password_from_database)
    when /\AH\z/, /\AP\z/
      valid_password_with_phpass?(password, password_from_database)
    else
      valid_password_with_unix_crypt?(password, password_from_database)
    end
  end

It can be helpful and works, but I do not like it... I think we can improve, If we refactor it to something like:

class PasswordEncryptor
  # Return the password hash and salt that must be stored on user's record.
  #
  #   PasswordEncryptor.encrypt("test")
  #   #=> {:hash => "abc...", :salt => "salt123"}
  def self.encrypt(password)
    salt = encryptor.hexdigest(rand.to_s)
    hash = hasher(password, salt)

    {:salt => salt, :hash => hash}
  end

  # Password must the plain text version.
  # Salt must be the salt stored on the user record.
  #
  #   PasswordEncryptor.hasher("marmot", "salt123")
  def self.hasher(password, salt)
    encryptor.hexdigest("#{salt}Lol:)#{password}")
  end

  # Return the encryptor adapter. Must always respond to
  # the +hexdigest+ method.
  def self.encryptor
    Digest::SHA1 #Can be others or a delegator
  end
end

class Authenticator
  def self.authenticate(username, password)
    user = User.find_by_user(username) 

    return unless user actual_hash = PasswordEncryptor.hasher(password, user.password_salt)

    return user if user.password_hash == actual_hash
  end
end

What do you think about the idea?

I can provide a PR ;)

[gnomex]

The PR #12 has a good discussion about it....

[pencil]

I think your issue describes the problem better. It's something we will eventually add. Maybe we could use the Decorator Pattern to support various configurations of hashing/salting/peppering?

[gnomex]

Hey, thank you!

About the pattern, the decorator is a good pattern, but works with inheritance, adding behaviors to object, and make it fat.

I think, the Delegation pattern is a best way: "The Delegation Design Pattern is a technique where an object exposes certain behavior but it actually delegates responsibility for implementing that behavior to an associated object."

So, I'll implement and later we can talk more about the best way...

[pencil]

The big advantage of the Decorator pattern is that we could cover all special cases like sha1(salt1 + sha1(salt2 + password) + pepper) with generic components. Inheritance would not be needed, since Ruby has duck typing.

But maybe it's a bit over-engineered. Looking forward to see your suggestion 😄