Skip to content

Commit

Permalink
Fix password strength feedback for empty password (#6008)
Browse files Browse the repository at this point in the history
* Mark password strings as used

**Why**: To ensure they're included in the locale bundles generated by RailsI18nWebpackPlugin

* Join password strength feedback strings as sentences

**Why**: So that they don't create a run-on sentence, since each string does not terminate as a full sentence

* Generate password strength feedback only if password non-empty

**Why**: So that the feedback matches the initial state on a new page load, per the expectation of LG-5461

* Add changelog

changelog: Bug Fixes, Password Entry, Add missing messages for password strength feedback

* Add specs for getFeedback
  • Loading branch information
aduth authored Feb 28, 2022
1 parent 686f25b commit 054e892
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 4 deletions.
8 changes: 5 additions & 3 deletions app/javascript/packs/pw-strength.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ function getStrength(z) {
return z && z.password.length ? scale[z.score] : fallback;
}

function getFeedback(z) {
if (!z || z.score > 2) {
export function getFeedback(z) {
if (!z || !z.password || z.score > 2) {
return ' ';
}

Expand All @@ -56,6 +56,7 @@ function getFeedback(z) {
// i18n-tasks-use t('zxcvbn.feedback.dates_are_often_easy_to_guess')
// i18n-tasks-use t('zxcvbn.feedback.for_a_stronger_password_use_a_few_words_separated_by_spaces_but_avoid_common_phrases')
// i18n-tasks-use t('zxcvbn.feedback.names_and_surnames_by_themselves_are_easy_to_guess')
// i18n-tasks-use t('zxcvbn.feedback.no_need_for_symbols_digits_or_uppercase_letters')
// i18n-tasks-use t('zxcvbn.feedback.predictable_substitutions_like__instead_of_a_dont_help_very_much')
// i18n-tasks-use t('zxcvbn.feedback.recent_years_are_easy_to_guess')
// i18n-tasks-use t('zxcvbn.feedback.repeats_like_aaa_are_easy_to_guess')
Expand All @@ -69,6 +70,7 @@ function getFeedback(z) {
// i18n-tasks-use t('zxcvbn.feedback.this_is_a_top_10_common_password')
// i18n-tasks-use t('zxcvbn.feedback.this_is_a_very_common_password')
// i18n-tasks-use t('zxcvbn.feedback.this_is_similar_to_a_commonly_used_password')
// i18n-tasks-use t('zxcvbn.feedback.use_a_few_words_avoid_common_phrases')
// i18n-tasks-use t('zxcvbn.feedback.use_a_longer_keyboard_pattern_with_more_turns')
return t(`zxcvbn.feedback.${snakeCase(str)}`);
}
Expand All @@ -80,7 +82,7 @@ function getFeedback(z) {
return lookup(warning);
}

return `${suggestions.map((s) => lookup(s)).join('')}`;
return `${suggestions.map((s) => lookup(s)).join('. ')}`;
}

function disableSubmit(submitEl, length = 0, score = 0) {
Expand Down
25 changes: 24 additions & 1 deletion spec/javascripts/packs/pw-strength-spec.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { getForbiddenPasswords } from '../../../app/javascript/packs/pw-strength';
import zxcvbn from 'zxcvbn';
import { getForbiddenPasswords, getFeedback } from '../../../app/javascript/packs/pw-strength';

describe('pw-strength', () => {
describe('getForbiddenPasswords', () => {
Expand Down Expand Up @@ -32,4 +33,26 @@ describe('pw-strength', () => {
expect(result).to.be.deep.equal(['foo', 'bar', 'baz']);
});
});

describe('getFeedback', () => {
const EMPTY_RESULT = ' ';

it('returns an empty result for empty password', () => {
const z = zxcvbn('');

expect(getFeedback(z)).to.equal(EMPTY_RESULT);
});

it('returns an empty result for a strong password', () => {
const z = zxcvbn('!Juq2Uk2**RBEsA8');

expect(getFeedback(z)).to.equal(EMPTY_RESULT);
});

it('returns feedback for a weak password', () => {
const z = zxcvbn('password');

expect(getFeedback(z)).to.equal('zxcvbn.feedback.this_is_a_top_10_common_password');
});
});
});

0 comments on commit 054e892

Please sign in to comment.