From 1ccbd68558ba312d6a6a8370984737bf37bb2c72 Mon Sep 17 00:00:00 2001 From: Brendan Sudol Date: Wed, 25 May 2016 13:41:52 -0400 Subject: [PATCH] Update password requirements **Why**: to use latest NIST guidance / best practices --- app/models/user.rb | 8 - app/views/devise/confirmations/show.html.slim | 8 +- .../locales/devise.security_extension.en.yml | 2 +- lib/saml_idp_constants.rb | 1 - spec/models/user_spec.rb | 150 +----------------- 5 files changed, 9 insertions(+), 160 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index ec6d2516c61..1987f23f787 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -35,14 +35,6 @@ class User < ActiveRecord::Base validates :ial_token, uniqueness: true, allow_nil: true - validates :password, - format: { - with: /(?=.*\d)(?=.*[a-z])(?=.*[A-Z]) - (?=.*[#{Regexp.escape(Saml::Idp::Constants::PASSWORD_SPECIAL_CHARS)}])/x, - message: :password_format - }, - if: :password_required? - has_many :authorizations, dependent: :destroy has_many :identities, dependent: :destroy diff --git a/app/views/devise/confirmations/show.html.slim b/app/views/devise/confirmations/show.html.slim index 114a97ed753..0dff7aa0013 100644 --- a/app/views/devise/confirmations/show.html.slim +++ b/app/views/devise/confirmations/show.html.slim @@ -6,13 +6,7 @@ .panel-heading h2.m0 = t('upaya.forms.confirmation.show_hdr') .mb2.p2.alert-info - h4.mt0.mb1 Your password must: - ul.m0 - li be at least 8 characters in length - li contain at least one upper case letter - li contain at least one lower case letter - li contain at least one number - li contain at least one special character + .h4 Your password must be at least 8 characters. = simple_form_for(resource, as: resource_name, url: confirm_path, html: { role: 'form', autocomplete: 'off' }) do |f| = f.error_notification = f.input :password, required: true, input_html: { autofocus: true } diff --git a/config/locales/devise.security_extension.en.yml b/config/locales/devise.security_extension.en.yml index cb54bc8cd0d..6ccddf977cd 100644 --- a/config/locales/devise.security_extension.en.yml +++ b/config/locales/devise.security_extension.en.yml @@ -3,7 +3,7 @@ en: messages: taken_in_past: "was already taken in the past!" equal_to_current_password: "must be different from the current password!" - password_format: "must be between 8 and 128 characters, contain at least one upper case letter, at least one lower case letter, at least one number, and at least one \"special\" character. Accepted \"special\" characters are: !\"#$%&'()*+,-.:;<=>?@[]{}/^_~`|" + password_format: "must be between 8 and 128 characters" devise: invalid_captcha: "The captcha input is not valid!" password_expired: diff --git a/lib/saml_idp_constants.rb b/lib/saml_idp_constants.rb index fb1b47dbb4e..078abfa4766 100644 --- a/lib/saml_idp_constants.rb +++ b/lib/saml_idp_constants.rb @@ -2,7 +2,6 @@ module Saml module Idp module Constants - PASSWORD_SPECIAL_CHARS = "!\"\#$%&'()*+,-.:;<=>?@[]{}/\\^_~`|".freeze LOA1_AUTHNCONTEXT_CLASSREF = 'http://idmanagement.gov/ns/assurance/loa/1'.freeze LOA3_AUTHNCONTEXT_CLASSREF = 'http://idmanagement.gov/ns/assurance/loa/3'.freeze end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index a80987f29ec..54377f436d2 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -136,149 +136,6 @@ end context '.password_strength' do - it 'requires a digit' do - NO_NUM_PASSWORD = 'abcdABCD!@#$'.freeze - - # Verify failure on create. - # TODO: JJG restore check for error message - expect do - create(:user, - password: NO_NUM_PASSWORD, - password_confirmation: NO_NUM_PASSWORD) - end. - to raise_error(ActiveRecord::RecordInvalid) - - prototype_user = create(:user) - - # Verify success. - (0..9).each do |digit| - password = "#{NO_NUM_PASSWORD}#{digit}" - user = create(:user, - email: "#{digit}.#{prototype_user.email}", - password: password, - password_confirmation: password) - expect(user.errors.any?).to be_falsey - end - - # Verifying password updating enforces complexity requirements. - (0..9).each do |digit| - prototype_user.password = NO_NUM_PASSWORD - prototype_user.password_confirmation = NO_NUM_PASSWORD - expect(prototype_user.valid?).to be_falsey - prototype_user.password = "#{NO_NUM_PASSWORD}#{digit}" - prototype_user.password_confirmation = "#{NO_NUM_PASSWORD}#{digit}" - expect(prototype_user.valid?).to be_truthy - end - end - - it 'requires a capital letter' do - NO_CAP_PASSWORD = 'abcd1234!@#$'.freeze - - # Verify failure on create. - # TODO: JJG restore check for error message - expect do - create(:user, - password: NO_CAP_PASSWORD, - password_confirmation: NO_CAP_PASSWORD) - end. - to raise_error(ActiveRecord::RecordInvalid) - - prototype_user = create(:user) - - # Verify success. - ('A'..'Z').each do |capital| - password = "#{NO_CAP_PASSWORD}#{capital}" - user = create(:user, - email: "#{capital}.#{prototype_user.email}", - password: password, - password_confirmation: password) - expect(user.errors.any?).to be_falsey - end - - # Verifying password updating enforces complexity requirements. - ('A'..'Z').each do |capital| - prototype_user.password = NO_CAP_PASSWORD - prototype_user.password_confirmation = NO_CAP_PASSWORD - expect(prototype_user.valid?).to be_falsey - prototype_user.password = "#{NO_CAP_PASSWORD}#{capital}" - prototype_user.password_confirmation = "#{NO_CAP_PASSWORD}#{capital}" - expect(prototype_user.valid?).to be_truthy - end - end - - it 'requires a lowercase letter' do - NO_LOWER_PASSWORD = 'ABCD1234!@#$'.freeze - - # Verify failure on create. - # TODO: JJG restore check for error message - expect do - create(:user, - password: NO_LOWER_PASSWORD, - password_confirmation: NO_LOWER_PASSWORD) - end.to raise_error(ActiveRecord::RecordInvalid) - - prototype_user = create(:user) - - # Verify success. - ('a'..'z').each do |lower| - password = "#{NO_LOWER_PASSWORD}#{lower}" - user = create(:user, - email: "#{lower}.#{prototype_user.email}", - password: password, - password_confirmation: password) - expect(user.errors.any?).to be_falsey - end - - # Verifying password updating enforces complexity requirements. - ('a'..'z').each do |lower| - prototype_user.password = NO_LOWER_PASSWORD - prototype_user.password_confirmation = NO_LOWER_PASSWORD - expect(prototype_user.valid?).to be_falsey - prototype_user.password = "#{NO_LOWER_PASSWORD}#{lower}" - prototype_user.password_confirmation = "#{NO_LOWER_PASSWORD}#{lower}" - expect(prototype_user.valid?).to be_truthy - end - end - - it 'requires a special character' do - # Verify failure. - NO_SPECIAL_PASSWORD = 'ABCD1234abcd'.freeze - - # Verify failure on create. - expect do - create(:user, - password: NO_SPECIAL_PASSWORD, - password_confirmation: NO_SPECIAL_PASSWORD) - end. - to raise_error(ActiveRecord::RecordInvalid - ) - - prototype_user = create(:user) - - # Verify success. - i = 1 - Saml::Idp::Constants::PASSWORD_SPECIAL_CHARS.each_char do |special| - password = "#{NO_SPECIAL_PASSWORD}#{special}" - user = create(:user, - email: "#{i}.#{prototype_user.email}", - password: password, - password_confirmation: password) - expect(user.errors.any?).to be_falsey - i += 1 - end - - # Verifying password updating enforces complexity requirements. - Saml::Idp::Constants::PASSWORD_SPECIAL_CHARS.each_char do |special| - prototype_user.password = NO_SPECIAL_PASSWORD - prototype_user.password_confirmation = NO_SPECIAL_PASSWORD - expect(prototype_user.valid?).to be_falsey - prototype_user.password = "#{NO_SPECIAL_PASSWORD}#{special}" - prototype_user.password_confirmation = "#{NO_SPECIAL_PASSWORD}#{special}" - expect(prototype_user.valid?).to be_truthy - i += 1 - end - end - it 'must be more than 8 characters' do prototype_user = create(:user) expect do @@ -302,6 +159,13 @@ to raise_error(ActiveRecord::RecordInvalid, /Validation failed: Password is too long \(maximum is 128 characters\)/) end + + it 'works with spaces' do + pw = 'this has a few spaces' + user = build_stubbed(:user, password: pw, password_confirmation: pw) + + expect(user).to be_valid + end end context '#two_factor_enabled?' do