Permalink
Browse files

Added ActiveRecord::Base#has_secure_password (via ActiveModel::Secure…

…Password) to encapsulate dead-simple password usage with SHA2 encryption and salting
  • Loading branch information...
dhh committed Dec 18, 2010
1 parent d9b732f commit bcf4e4f2b02157cecc1f1727a95cdf5bfa471771
View
@@ -1,15 +1,18 @@
*Rails 3.1.0 (unreleased)*
-* No changes
+* Added ActiveModel::SecurePassword to encapsulate dead-simple password usage with SHA2 encryption and salting [DHH]
+
*Rails 3.0.2 (unreleased)*
* No changes
+
*Rails 3.0.1 (October 15, 2010)*
* No Changes, just a version bump.
+
*Rails 3.0.0 (August 29, 2010)*
* Added ActiveModel::MassAssignmentSecurity [Eric Chapweske, Josh Kalderimis]
@@ -42,6 +42,7 @@ module ActiveModel
autoload :Naming
autoload :Observer, 'active_model/observing'
autoload :Observing
+ autoload :SecurePassword
autoload :Serialization
autoload :TestCase
autoload :Translation
@@ -0,0 +1,73 @@
+require 'digest/sha2'
+
+module ActiveModel
+ module SecurePassword
+ extend ActiveSupport::Concern
+
+ module ClassMethods
+ # Adds methods to set and authenticate against a SHA2-encrypted and salted password.
+ # This mechanism requires you to have password_digest and password_salt attributes.
+ #
+ # Validations for presence of password, confirmation of password (using a "password_confirmation" attribute),
+ # and strength of password (at least 6 chars, not "password") are automatically added.
+ # You can add more validations by hand if need be.
+ #
+ # Example using Active Record (which automatically includes ActiveModel::SecurePassword):
+ #
+ # # Schema: User(name:string, password_digest:string, password_salt:string)
+ # class User < ActiveRecord::Base
+ # has_secure_password
+ # end
+ #
+ # user = User.new(:name => "david", :password => "secret", :password_confirmation => "nomatch")
+ # user.save # => false, password not long enough
+ # user.password = "mUc3m00RsqyRe"
+ # user.save # => false, confirmation doesn't match
+ # user.password_confirmation = "mUc3m00RsqyRe"
+ # user.save # => true
+ # user.authenticate("notright") # => false
+ # user.authenticate("mUc3m00RsqyRe") # => user
+ # User.find_by_name("david").try(:authenticate, "notright") # => nil
+ # User.find_by_name("david").try(:authenticate, "mUc3m00RsqyRe") # => user
+ def has_secure_password
+ attr_reader :password
+ attr_accessor :password_confirmation
+
+ attr_protected(:password_digest, :password_salt) if respond_to?(:attr_protected)
+
+ validates_confirmation_of :password
+ validates_presence_of :password_digest
+ validate :password_must_be_strong
+ end
+ end
+
+ module InstanceMethods
+ # Returns self if the password is correct, otherwise false.
+ def authenticate(unencrypted_password)
+ password_digest == encrypt_password(unencrypted_password) ? self : false
+ end
+
+ # Encrypts the password into the password_digest attribute.
+ def password=(unencrypted_password)
+ @password = unencrypted_password
+ self.password_digest = encrypt_password(unencrypted_password)
+ end
+
+ private
+ def salt_for_password
+ self.password_salt ||= self.object_id.to_s + rand.to_s
+ end
+
+ def encrypt_password(unencrypted_password)
+ Digest::SHA2.hexdigest(unencrypted_password + salt_for_password)
+ end
+
+ def password_must_be_strong
+ if @password.present?
+ errors.add(:password, "must be longer than 6 characters") unless @password.size > 6

This comment has been minimized.

Show comment
Hide comment
@trevorturk

trevorturk Dec 19, 2010

Contributor

Any chance of making this minimum size configurable like weak_passwords?

@trevorturk

trevorturk Dec 19, 2010

Contributor

Any chance of making this minimum size configurable like weak_passwords?

+ errors.add(:password, "can't be 'password'") if @password == "password"

This comment has been minimized.

Show comment
Hide comment
@joshsusser

joshsusser Dec 19, 2010

Contributor

It's really nice to be able to use "password" as the password in development and tests. Can this check be enforced only in production? Also, why prevent "password" but not "123456" (the most common) or other top-10 common passwords?

@joshsusser

joshsusser Dec 19, 2010

Contributor

It's really nice to be able to use "password" as the password in development and tests. Can this check be enforced only in production? Also, why prevent "password" but not "123456" (the most common) or other top-10 common passwords?

This comment has been minimized.

Show comment
Hide comment
@tenderlove

tenderlove Dec 19, 2010

Member

I don't think we should have this at all. If someone wants to prevent certain passwords, it's easy enough to write their own validation. I frequently use password as my development password. This code would cause a hassle, as well as make tests fail.

@tenderlove

tenderlove Dec 19, 2010

Member

I don't think we should have this at all. If someone wants to prevent certain passwords, it's easy enough to write their own validation. I frequently use password as my development password. This code would cause a hassle, as well as make tests fail.

This comment has been minimized.

Show comment
Hide comment
@parndt

parndt Dec 19, 2010

Contributor

Would rather not have a hardcoded fail on one particular string. Infact, could this instead be some sort of config based array so that one could provide invalid passwords for their own application? This has the added bonus of allowing developers to specify none, too.

e.g.

invalid_passwords.each do |password|
  errors.add(:password, "can't be '#{password}'") if @password == password
end

That way, there is not just one default "bad", English, word that fails.

@parndt

parndt Dec 19, 2010

Contributor

Would rather not have a hardcoded fail on one particular string. Infact, could this instead be some sort of config based array so that one could provide invalid passwords for their own application? This has the added bonus of allowing developers to specify none, too.

e.g.

invalid_passwords.each do |password|
  errors.add(:password, "can't be '#{password}'") if @password == password
end

That way, there is not just one default "bad", English, word that fails.

This comment has been minimized.

Show comment
Hide comment
@fxn

fxn Dec 19, 2010

Member

I agree with Aaron, don't see this control at all in core.

@fxn

fxn Dec 19, 2010

Member

I agree with Aaron, don't see this control at all in core.

+ end
+ end
+ end
+ end
+end
@@ -0,0 +1,42 @@
+require 'cases/helper'
+require 'models/user'
+
+class SecurePasswordTest < ActiveModel::TestCase
+ setup do
+ @user = User.new
+ end
+
+ test "password must be present" do
+ assert !@user.valid?
+ assert_equal 1, @user.errors.size
+ end
+
+ test "password must match confirmation" do
+ @user.password = "thiswillberight"
+ @user.password_confirmation = "wrong"
+
+ assert !@user.valid?
+
+ @user.password_confirmation = "thiswillberight"
+
+ assert @user.valid?
+ end
+
+ test "password must pass validation rules" do
+ @user.password = "password"
+ assert !@user.valid?
+
+ @user.password = "short"
+ assert !@user.valid?
+
+ @user.password = "plentylongenough"
+ assert @user.valid?
+ end
+
+ test "authenticate" do
+ @user.password = "secret"
+
+ assert !@user.authenticate("wrong")
+ assert @user.authenticate("secret")
+ end
+end
@@ -0,0 +1,8 @@
+class User
+ include ActiveModel::Validations
+ include ActiveModel::SecurePassword
+
+ has_secure_password
+
+ attr_accessor :password_digest, :password_salt
+end
View
@@ -1,5 +1,24 @@
*Rails 3.1.0 (unreleased)*
+* Added ActiveRecord::Base#has_secure_password (via ActiveModel::SecurePassword) to encapsulate dead-simple password usage with SHA2 encryption and salting [DHH]. Example:

This comment has been minimized.

Show comment
Hide comment
@mitchellh

mitchellh Dec 19, 2010

I think the end of this line is meant to say BCrypt and not SHA2.

@mitchellh

mitchellh Dec 19, 2010

I think the end of this line is meant to say BCrypt and not SHA2.

+
+ # Schema: User(name:string, password_digest:string, password_salt:string)
+ class User < ActiveRecord::Base
+ has_secure_password
+ end
+
+ user = User.new(:name => "david", :password => "secret", :password_confirmation => "nomatch")
+ user.save # => false, password not long enough
+ user.password = "mUc3m00RsqyRe"
+ user.save # => false, confirmation doesn't match
+ user.password_confirmation = "mUc3m00RsqyRe"
+ user.save # => true
+ user.authenticate("notright") # => false
+ user.authenticate("mUc3m00RsqyRe") # => user
+ User.find_by_name("david").try(:authenticate, "notright") # => nil
+ User.find_by_name("david").try(:authenticate, "mUc3m00RsqyRe") # => user
+
+
* When a model is generated add_index is added by default for belongs_to or references columns
rails g model post user:belongs_to will generate the following:
@@ -1851,6 +1851,7 @@ def clear_timestamp_attributes
include ActiveModel::MassAssignmentSecurity
include Callbacks, ActiveModel::Observing, Timestamp
include Associations, AssociationPreload, NamedScope
+ include ActiveModel::SecurePassword
# AutosaveAssociation needs to be included before Transactions, because we want
# #save_with_autosave_associations to be wrapped inside a transaction.

5 comments on commit bcf4e4f

@dmathieu

This comment has been minimized.

Show comment
Hide comment
@dmathieu

dmathieu Dec 18, 2010

Contributor

Interesting. Is that a first step on including an authentication system in rails core ? (I hope not).

Contributor

dmathieu replied Dec 18, 2010

Interesting. Is that a first step on including an authentication system in rails core ? (I hope not).

@dhh

This comment has been minimized.

Show comment
Hide comment
@dhh

dhh Dec 18, 2010

Member

dmathieu, absolutely not. This is wrapping up the low-level task of proper hashing. Nothing is coming to core that'll encapsulate how to send reset emails or whatever else the whole-hog authentication solutions do.

Member

dhh replied Dec 18, 2010

dmathieu, absolutely not. This is wrapping up the low-level task of proper hashing. Nothing is coming to core that'll encapsulate how to send reset emails or whatever else the whole-hog authentication solutions do.

@dhh

This comment has been minimized.

Show comment
Hide comment
@dhh

dhh Dec 18, 2010

Member

Also, we switched to BCrypt for added security: 39b5ea6

Member

dhh replied Dec 18, 2010

Also, we switched to BCrypt for added security: 39b5ea6

@dmathieu

This comment has been minimized.

Show comment
Hide comment
@dmathieu

dmathieu Dec 19, 2010

Contributor

Another thing : is there a reason why you didn't allow to manually specify the password field name ? Something like this :

secure :password
Contributor

dmathieu replied Dec 19, 2010

Another thing : is there a reason why you didn't allow to manually specify the password field name ? Something like this :

secure :password
@radar

This comment has been minimized.

Show comment
Hide comment
@radar

radar Dec 19, 2010

Contributor

What is the backstory on this commit? Why does this functionality need to be in Rails rather than inside the libraries that implement authentication?

Contributor

radar replied Dec 19, 2010

What is the backstory on this commit? Why does this functionality need to be in Rails rather than inside the libraries that implement authentication?

Please sign in to comment.