Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

New password default minimum length is now 8

  • Loading branch information...
commit 2950434ed344610770ad83dd77c9f45b9f01713c 1 parent 1aca139
Rodrigo Flores authored
View
2  lib/devise.rb
@@ -90,7 +90,7 @@ module Strategies
# Range validation for password length
mattr_accessor :password_length
- @@password_length = 8..128
+ @@password_length = 6..128
# The time the user will be remembered without asking for credentials again.
mattr_accessor :remember_for
View
2  lib/generators/templates/devise.rb
@@ -114,7 +114,7 @@
# ==> Configuration for :validatable
# Range for password length. Default is 8..128.
- # config.password_length = 8..128
+ config.password_length = 8..128
# Email regex used to validate email formats. It simply asserts that
# an one (and only one) @ exists in the given string. This is mainly
View
6 test/models/validatable_test.rb
@@ -76,10 +76,10 @@ class ValidatableTest < ActiveSupport::TestCase
assert_equal 'doesn\'t match confirmation', user.errors[:password].join
end
- test 'should require a password with minimum of 8 characters' do
- user = new_user(:password => '1234567', :password_confirmation => '1234567')
+ test 'should require a password with minimum of 6 characters' do
+ user = new_user(:password => '12345', :password_confirmation => '12345')
assert user.invalid?
- assert_equal 'is too short (minimum is 8 characters)', user.errors[:password].join
+ assert_equal 'is too short (minimum is 6 characters)', user.errors[:password].join
end
test 'should require a password with maximum of 128 characters long' do

11 comments on commit 2950434

@carlosantoniodasilva
Collaborator

Ahm, are we keeping 6 as lib's default, but 8 in the generated template? Any reasoning?

@josevalim
Owner
@rafaelfranca

Yes, I agree that is better to change the generator default.

@carlosantoniodasilva
Collaborator

My opinion? Change it, put in the changelog and we may blog about the update later on, when releasing a new version - it's a one line change for any developer who wants to keep at 6. A change from 6 to 8 chars is probably not enough to break existing functionality, since current passwords will still work (they should), and the user will be asked to enter an 8 digit one only when making a change on the password (ie registrations/edit or password reset).

This will avoid us to have to sanity check why in the world the generator is so different from the lib one. We've had some headaches with this in SimpleForm. Anyway, the important bit is that new apps will default to 8, so consider this just my 2 cents to ignore ;)

@josevalim
Owner

Unfortunately people don't read the CHANGELOG. :( Given everyone's feedback, I think the best solution is: make the default to be nil and provide an initializer that checks if config.password_length is set. If not, show the user a warning message, saying we are now defaulting to 8..120, but if he wants to keep the previous value, he should set it in his initializer.

This way, everyone is kind of forced to upgrade do 8..120, but users will at least receive a warning.

@rodrigoflores
Collaborator

I agree that we should do a smooth change, although it will not break production environments (a already set password with a length of 6, still works, right?) . So, I propose we do this:

  • New applications come with this new definition (but it can be changed);
  • We add a deprecation warning telling that conf will change. If you want to continue using 6 passwords as minimum, set this on the configuration. So, in the minor version after the next minor version (I'm talking about the Devise 2.3 or 3.0) we set this as default.

Wdyt ?

@josevalim
Owner

@rodrigoflores we suggested pretty much the same thing, except that my proposal is: 1) we change and tell the users. Yours is: 1) tell the users it will change and 2) then change. For me both are fine.

@carlosantoniodasilva
Collaborator

Unfortunately people don't read the CHANGELOG. :(

That's their fault :trollface:

I'm kidding bro :D. That still makes things different everywhere, but it seems like a possible good solution. Will we someday be able to put the default back into lib?

@josevalim
Owner

Yeah, both solutions will allow us to set the default back into lib (this should be able in Simple Form too).

@carlosantoniodasilva
Collaborator
@josevalim
Owner
Please sign in to comment.
Something went wrong with that request. Please try again.