Skip to content

Changed password validation to check in range #2025

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tbaxter-18f merged 3 commits into from
Feb 27, 2018
Merged

Changed password validation to check in range #2025

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
18a699b
Changed password validation to check in range
tbaxter-18f Feb 27, 2018
6a7a381
Increased minimum password length to 9
tbaxter-18f Feb 27, 2018
187d0c9
Changed password length to 9 characters (with tests)
tbaxter-18f Feb 27, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/validators/form_password_validator.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ module FormPasswordValidator

validates :password,
presence: true,
length: Devise.password_length
length: { in: Devise.password_length }
Copy link
Copy Markdown
Contributor

@jmhooper jmhooper Feb 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exactly is the purpose of this change? It does not look like this changes anything?

Copy link
Copy Markdown
Contributor Author

@tbaxter-18f tbaxter-18f Feb 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm understanding correctly -- and I may not be -- this will make it confirm that it's within the range, so the 8/9 character mismatch will sort out. But I could be looking at it wrong.

Copy link
Copy Markdown
Contributor

@jmhooper jmhooper Feb 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think what we have now is a shorthand for that. I don't think the problem is with the Devise validation, but with the zxcvbn password score on the frontend.

So the issue is not length, but rather lack of complexity.


validate :strong_password, if: :password_strength_enabled?
end
Expand Down
2 changes: 1 addition & 1 deletion config/initializers/devise.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
config.mailer = 'CustomDeviseMailer'
config.mailer_sender = email_with_name(Figaro.env.email_from, Figaro.env.email_from)
config.paranoid = true
config.password_length = 8..128
config.password_length = 9..128
config.reconfirmable = true
config.reset_password_within = 6.hours
config.secret_key = Figaro.env.secret_key_base
Expand Down
2 changes: 1 addition & 1 deletion spec/controllers/sign_up/passwords_controller_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@

analytics_hash = {
success: false,
errors: { password: ['is too short (minimum is 8 characters)'] },
errors: { password: ['is too short (minimum is 9 characters)'] },
user_id: user.uuid,
request_id_present: false,
}
Expand Down
4 changes: 2 additions & 2 deletions spec/controllers/users/reset_passwords_controller_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
analytics_hash = {
success: false,
errors: {
password: ['is too short (minimum is 8 characters)'],
password: ['is too short (minimum is 9 characters)'],
reset_password_token: ['token_expired'],
},
user_id: user.uuid,
Expand Down Expand Up @@ -122,7 +122,7 @@
form_params = { password: 'short', reset_password_token: raw_reset_token }
analytics_hash = {
success: false,
errors: { password: ['is too short (minimum is 8 characters)'] },
errors: { password: ['is too short (minimum is 9 characters)'] },
user_id: user.uuid,
active_profile: false,
confirmed: true,
Expand Down
2 changes: 1 addition & 1 deletion spec/features/visitors/password_recovery_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@
fill_in 'New password', with: '1234'
click_button t('forms.passwords.edit.buttons.submit')

expect(page).to have_content 'is too short (minimum is 8 characters)'
expect(page).to have_content 'is too short (minimum is 9 characters)'
end

it "does not update the user's password when password is invalid" do
Expand Down
4 changes: 2 additions & 2 deletions spec/features/visitors/set_password_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
it 'has dynamic password strength feedback' do
expect(page).to have_content '...'

fill_in 'password_form_password', with: 'password'
fill_in 'password_form_password', with: '123456789'
expect(page).to have_content t('zxcvbn.feedback.this_is_a_top_10_common_password')
end
end
Expand Down Expand Up @@ -88,7 +88,7 @@

create(:user, :unconfirmed)
confirm_last_user
fill_in 'password_form_password', with: 'password'
fill_in 'password_form_password', with: '123456789'

click_button t('forms.buttons.continue')

Expand Down
2 changes: 1 addition & 1 deletion spec/forms/password_form_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
password = 'invalid'

errors = {
password: ['is too short (minimum is 8 characters)'],
password: ['is too short (minimum is 9 characters)'],
}

extra = {
Expand Down
4 changes: 2 additions & 2 deletions spec/forms/reset_password_form_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@

password = 'invalid'

errors = { password: ['is too short (minimum is 8 characters)'] }
errors = { password: ['is too short (minimum is 9 characters)'] }

extra = {
user_id: '123',
Expand Down Expand Up @@ -89,7 +89,7 @@
password = 'short'

errors = {
password: ['is too short (minimum is 8 characters)'],
password: ['is too short (minimum is 9 characters)'],
reset_password_token: ['token_expired'],
}

Expand Down
2 changes: 1 addition & 1 deletion spec/support/shared_examples/password_strength.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
user = build_stubbed(:user, email: 'test@test.com', uuid: '123')
allow(user).to receive(:reset_password_period_valid?).and_return(true)
form = form_class.constantize.new(user)
password = 'custom!@'
password = 'custom!@Z'
errors = {
password: ['Your password is not strong enough.' \
' This is similar to a commonly used password.' \
Expand Down
7 changes: 1 addition & 6 deletions spec/support/shared_examples_for_password_validation.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,7 @@

it do
is_expected.to validate_length_of(:password).
is_at_least(Devise.password_length.first)
end

it do
is_expected.to validate_length_of(:password).
is_at_most(Devise.password_length.last)
is_at_least(Devise.password_length.first).is_at_most(Devise.password_length.last)
end

it do
Expand Down