Permalink
Browse files

nil password should be allowed

User registration process can be divided into several steps.
If the user registration form No. 1, for example, does not have
password field, validation never succeeds with current implementation.
  • Loading branch information...
1 parent 885a599 commit cee252af464bda91cb879bea597edd98f58a0829 @kuroda committed Dec 7, 2011
Showing with 15 additions and 7 deletions.
  1. +6 −2 activemodel/lib/active_model/secure_password.rb
  2. +9 −5 activemodel/test/cases/secure_password_test.rb
@@ -31,6 +31,10 @@ module ClassMethods
# user.authenticate("mUc3m00RsqyRe") # => user
# User.find_by_name("david").try(:authenticate, "notright") # => nil
# User.find_by_name("david").try(:authenticate, "mUc3m00RsqyRe") # => user
+ #
+ # user = User.new(:name => "david")
+ # user.valid? # => true, password can be nil
+ # user.save! # => true, but cannot be authenticated
def has_secure_password
# Load bcrypt-ruby only when has_secure_password is used.
# This is to avoid ActiveModel (and by extension the entire framework) being dependent on a binary library.
@@ -40,7 +44,7 @@ def has_secure_password
attr_reader :password
validates_confirmation_of :password
- validates_presence_of :password_digest
+ validates_presence_of :password_digest, :if => :password
include InstanceMethodsOnActivation
@@ -55,7 +59,7 @@ def self.attributes_protected_by_default
module InstanceMethodsOnActivation
# Returns self if the password is correct, otherwise false.
def authenticate(unencrypted_password)
- if BCrypt::Password.new(password_digest) == unencrypted_password
+ if password_digest && BCrypt::Password.new(password_digest) == unencrypted_password
self
else
false
@@ -14,14 +14,13 @@ class SecurePasswordTest < ActiveModel::TestCase
assert !@user.valid?, 'user should be invalid'
end
- test "nil password" do
+ test "nil password may be allowed" do
@user.password = nil
- assert !@user.valid?, 'user should be invalid'
+ assert @user.valid?
end
- test "password must be present" do
- assert !@user.valid?
- assert_equal 1, @user.errors.size
+ test "password may not be present" do
+ assert @user.valid?
end
test "password must match confirmation" do
@@ -42,6 +41,11 @@ class SecurePasswordTest < ActiveModel::TestCase
assert @user.authenticate("secret")
end
+ test "never authenticate if password_digest is nil" do
+ assert !@user.authenticate("")
+ assert !@user.authenticate(nil)
+ end
+
test "visitor#password_digest should be protected against mass assignment" do
assert Visitor.active_authorizers[:default].kind_of?(ActiveModel::MassAssignmentSecurity::BlackList)
assert Visitor.active_authorizers[:default].include?(:password_digest)

0 comments on commit cee252a

Please sign in to comment.