Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Remove weak_passwords list and the length/strong password validator, …

…leave that up to the programmer
  • Loading branch information...
commit 08ccd29b5b1e3badc2176a8036fea138b774c38f 1 parent 37e6431
@spastorino spastorino authored
View
4 activemodel/lib/active_model/locale/en.yml
@@ -25,7 +25,3 @@ en:
less_than_or_equal_to: "must be less than or equal to %{count}"
odd: "must be odd"
even: "must be even"
-
- attributes:
- password:
- insecure: "is too weak and common"
View
37 activemodel/lib/active_model/secure_password.rb
@@ -1,22 +1,16 @@
-require 'active_support/core_ext/object/blank'
-require 'active_support/core_ext/class/attribute'
+require 'active_support/concern'
require 'bcrypt'
module ActiveModel
module SecurePassword
extend ActiveSupport::Concern
- included do
- class_attribute :weak_passwords
- self.weak_passwords = %w( password qwerty 123456 )
- end
-
module ClassMethods
# Adds methods to set and authenticate against a BCrypt password.
# This mechanism requires you to have a password_digest attribute.
#
- # Validations for presence of password, confirmation of password (using a "password_confirmation" attribute),
- # and strength of password (at least 6 chars, not "password", etc) are automatically added.
+ # Validations for presence of password, confirmation of password (using
+ # a "password_confirmation" attribute) are automatically added.
# You can add more validations by hand if need be.
#
# Example using Active Record (which automatically includes ActiveModel::SecurePassword):
@@ -26,8 +20,8 @@ module ClassMethods
# has_secure_password
# end
#
- # user = User.new(:name => "david", :password => "secret", :password_confirmation => "nomatch")
- # user.save # => false, password not long enough
+ # user = User.new(:name => "david", :password => "", :password_confirmation => "nomatch")
+ # user.save # => false, password required
# user.password = "mUc3m00RsqyRe"
# user.save # => false, confirmation doesn't match
# user.password_confirmation = "mUc3m00RsqyRe"
@@ -44,16 +38,6 @@ def has_secure_password
validates_confirmation_of :password
validates_presence_of :password_digest
- validate :password_must_be_strong
- end
-
- # Specify the weak passwords to be used in the model:
- #
- # class User
- # set_weak_passwords %w( password qwerty 123456 mypass )
- # end
- def set_weak_passwords(values)
- self.weak_passwords = values
end
end
@@ -71,14 +55,5 @@ def password=(unencrypted_password)
@password = unencrypted_password
self.password_digest = BCrypt::Password.create(unencrypted_password)
end
-
- private
-
- def password_must_be_strong
- if password.present?
- errors.add(:password, :too_short, :count => 7) unless password.size > 6
- errors.add(:password, :insecure) if self.class.weak_passwords.include?(password)
- end
- end
end
-end
+end
View
50 activemodel/test/cases/secure_password_test.rb
@@ -4,33 +4,9 @@
class SecurePasswordTest < ActiveModel::TestCase
setup do
- User.weak_passwords = %w( password qwerty 123456 )
@user = User.new
end
- test "there should be a list of default weak passwords" do
- assert_equal %w( password qwerty 123456 ), User.weak_passwords
- end
-
- test "specifying the list of passwords" do
- User.weak_passwords = %w( pass )
- assert_equal %w( pass ), User.weak_passwords
- end
-
- test "specifying the list of passwords in the class" do
- User.send(:set_weak_passwords, ['pass'])
- assert_equal %w( pass ), User.weak_passwords
- end
-
- test "adding to the list of passwords" do
- User.weak_passwords << 'pass'
- @user.password = "password"
- assert !@user.valid?
-
- @user.password = "pass"
- assert !@user.valid?
- end
-
test "password must be present" do
assert !@user.valid?
assert_equal 1, @user.errors.size
@@ -47,34 +23,10 @@ class SecurePasswordTest < ActiveModel::TestCase
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 "too weak passwords" do
- @user.password = "012345"
- assert !@user.valid?
- assert_equal ["is too short (minimum is 7 characters)"], @user.errors[:password]
-
- @user.password = "password"
- assert !@user.valid?
- assert_equal ["is too weak and common"], @user.errors[:password]
-
- @user.password = "d9034rfjlakj34RR$!!"
- assert @user.valid?
- end
-
test "authenticate" do
@user.password = "secret"
assert !@user.authenticate("wrong")
assert @user.authenticate("secret")
end
-end
+end
View
10 activerecord/CHANGELOG
@@ -6,12 +6,12 @@
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 = User.new(:name => "david", :password => "", :password_confirmation => "nomatch")
+ user.save # => false, password required
+ user.password = "mUc3m00RsqyRe"
user.save # => false, confirmation doesn't match
- user.password_confirmation = "mUc3m00RsqyRe"
+ user.password_confirmation = "mUc3m00RsqyRe"
user.save # => true
user.authenticate("notright") # => false
user.authenticate("mUc3m00RsqyRe") # => user

4 comments on commit 08ccd29

@daeltar

I would remove also validates_confirmation_of :password

@nicolasblanco

daeltar >
may you elaborate on that?

Adding a weak password validator in Rails may be controversial. Because every developer may have his own rules to decide which password is weak and which password isn't depending on his applications and their security rules.

But I do not think that a validator that only checks if another field has the same value is controversial or should not be in Rails core.

@daeltar

slainer68: for example we do not have confirmation field for :password in our app. (and we optionaly show you typed password if you are not sure what you typed in).

@fxn
Owner

@daeltar yes, I've implemented the same solution sometimes: a toggler let's the user see what he types if he wants. No email or password confirmation fields. I don't like them personally, they are tedious and add fields. No complaints by users so far.

Please sign in to comment.
Something went wrong with that request. Please try again.