diff --git a/app/assets/stylesheets/components/_index.scss b/app/assets/stylesheets/components/_index.scss index 0c37bdfb6b8..d9a3bed2f35 100644 --- a/app/assets/stylesheets/components/_index.scss +++ b/app/assets/stylesheets/components/_index.scss @@ -16,7 +16,6 @@ @forward 'modal'; @forward 'nav'; @forward 'page-heading'; -@forward 'password'; @forward 'profile-section'; @forward 'personal-key'; @forward 'radio-button'; diff --git a/app/components/password_confirmation_component.html.erb b/app/components/password_confirmation_component.html.erb index 49014e370ad..638293ef292 100644 --- a/app/components/password_confirmation_component.html.erb +++ b/app/components/password_confirmation_component.html.erb @@ -43,4 +43,5 @@ > <%= t('components.password_confirmation.toggle_label') %> + <%= render PasswordStrengthComponent.new(input_id:, forbidden_passwords:) %> <% end %> diff --git a/app/components/password_confirmation_component.rb b/app/components/password_confirmation_component.rb index 508188f5649..afb2f909b01 100644 --- a/app/components/password_confirmation_component.rb +++ b/app/components/password_confirmation_component.rb @@ -1,17 +1,19 @@ class PasswordConfirmationComponent < BaseComponent - attr_reader :form, :field_options, :tag_options + attr_reader :form, :field_options, :forbidden_passwords, :tag_options def initialize( form:, password_label: nil, confirmation_label: nil, field_options: {}, + forbidden_passwords: [], **tag_options ) @form = form @password_label = password_label @confirmation_label = confirmation_label @field_options = field_options + @forbidden_passwords = forbidden_passwords @tag_options = tag_options end diff --git a/app/components/password_strength_component.html.erb b/app/components/password_strength_component.html.erb new file mode 100644 index 00000000000..d7263054c6f --- /dev/null +++ b/app/components/password_strength_component.html.erb @@ -0,0 +1,18 @@ +<%= content_tag( + :'lg-password-strength', + 'input-id': input_id, + 'minimum-length': minimum_length, + 'forbidden-passwords': forbidden_passwords.to_json, + **tag_options, + class: [*tag_options[:class], 'display-none'], + ) do %> +
+
+
+
+
+
+ <%= t('instructions.password.strength.intro') %> + +
+<% end %> diff --git a/app/components/password_strength_component.rb b/app/components/password_strength_component.rb new file mode 100644 index 00000000000..6d431a49ba3 --- /dev/null +++ b/app/components/password_strength_component.rb @@ -0,0 +1,15 @@ +class PasswordStrengthComponent < BaseComponent + attr_reader :input_id, :forbidden_passwords, :minimum_length, :tag_options + + def initialize( + input_id:, + minimum_length: Devise.password_length.min, + forbidden_passwords: [], + **tag_options + ) + @input_id = input_id + @minimum_length = minimum_length + @forbidden_passwords = forbidden_passwords + @tag_options = tag_options + end +end diff --git a/app/assets/stylesheets/components/_password.scss b/app/components/password_strength_component.scss similarity index 58% rename from app/assets/stylesheets/components/_password.scss rename to app/components/password_strength_component.scss index 34ea8217bb2..e7c89578ac2 100644 --- a/app/assets/stylesheets/components/_password.scss +++ b/app/components/password_strength_component.scss @@ -16,19 +16,27 @@ margin-left: units(1); } - .pw-weak &:nth-child(-n + 1) { + lg-password-strength[score='1'] &:nth-child(-n + 1) { background-color: color('error'); } - .pw-average &:nth-child(-n + 2) { + lg-password-strength[score='2'] &:nth-child(-n + 2) { background-color: color('warning'); } - .pw-good &:nth-child(-n + 3) { + lg-password-strength[score='3'] &:nth-child(-n + 3) { background-color: color('success-light'); } - .pw-great &:nth-child(-n + 4) { + lg-password-strength[score='4'] &:nth-child(-n + 4) { background-color: color('success'); } } + +.password-strength__strength { + @include u-text(bold); +} + +.password-strength__feedback { + @include u-text(italic); +} diff --git a/app/components/password_strength_component.ts b/app/components/password_strength_component.ts new file mode 100644 index 00000000000..1ef5aee95d7 --- /dev/null +++ b/app/components/password_strength_component.ts @@ -0,0 +1 @@ +import '@18f/identity-password-strength/password-strength-element'; diff --git a/app/components/password_toggle_component.rb b/app/components/password_toggle_component.rb index 539da619f2f..82a1ad840b5 100644 --- a/app/components/password_toggle_component.rb +++ b/app/components/password_toggle_component.rb @@ -28,6 +28,6 @@ def toggle_id end def input_id - "password-toggle-input-#{unique_id}" + field_options.dig(:input_html, :id) || "password-toggle-input-#{unique_id}" end end diff --git a/app/javascript/packages/password-strength/README.md b/app/javascript/packages/password-strength/README.md new file mode 100644 index 00000000000..7ded5901d8f --- /dev/null +++ b/app/javascript/packages/password-strength/README.md @@ -0,0 +1,32 @@ +# `@18f/password-strength` + +Custom element implementation that displays a strength meter and feedback for an associated password input element. + +## Usage + +Importing the element will register the `` custom element: + +```ts +import '@18f/password-strength/password-strength-element'; +``` + +The custom element will implement interactive behaviors, but all markup must already exist. + +```html + +``` diff --git a/app/javascript/packages/password-strength/package.json b/app/javascript/packages/password-strength/package.json new file mode 100644 index 00000000000..0b364ce0299 --- /dev/null +++ b/app/javascript/packages/password-strength/package.json @@ -0,0 +1,8 @@ +{ + "name": "@18f/identity-password-strength", + "private": true, + "version": "1.0.0", + "dependencies": { + "zxcvbn": "^4.4.2" + } +} diff --git a/app/javascript/packages/password-strength/password-strength-element.spec.ts b/app/javascript/packages/password-strength/password-strength-element.spec.ts new file mode 100644 index 00000000000..3183b3be64b --- /dev/null +++ b/app/javascript/packages/password-strength/password-strength-element.spec.ts @@ -0,0 +1,125 @@ +import { screen } from '@testing-library/dom'; +import userEvent from '@testing-library/user-event'; +import './password-strength-element'; + +describe('PasswordStrengthElement', () => { + function createElement() { + document.body.innerHTML = ` + + + `; + + return document.querySelector('lg-password-strength')!; + } + + it('is shown when a value is entered', async () => { + const element = createElement(); + + const input = screen.getByRole('textbox'); + await userEvent.type(input, 'p'); + + expect(element.classList.contains('display-none')).to.be.false(); + }); + + it('is hidden when a value is removed', async () => { + const element = createElement(); + + const input = screen.getByRole('textbox'); + await userEvent.type(input, 'p'); + await userEvent.clear(input); + + expect(element.classList.contains('display-none')).to.be.true(); + }); + + it('displays strength and feedback for a given password', async () => { + const element = createElement(); + + const input = screen.getByRole('textbox'); + await userEvent.type(input, 'p'); + + expect(element.getAttribute('score')).to.equal('0'); + expect(screen.getByText('instructions.password.strength.0')).to.exist(); + expect( + screen.getByText('zxcvbn.feedback.add_another_word_or_two_uncommon_words_are_better'), + ).to.exist(); + }); + + it('invalidates input when value is not strong enough', async () => { + createElement(); + + const input: HTMLInputElement = screen.getByRole('textbox'); + await userEvent.type(input, 'p'); + + expect(input.validity.valid).to.be.false(); + }); + + it('shows custom feedback for forbidden password', async () => { + const element = createElement(); + + const input: HTMLInputElement = screen.getByRole('textbox'); + await userEvent.type(input, 'password'); + + expect(element.getAttribute('score')).to.equal('0'); + expect(screen.getByText('instructions.password.strength.0')).to.exist(); + expect( + screen.getByText('errors.attributes.password.avoid_using_phrases_that_are_easily_guessed'), + ).to.exist(); + expect(input.validity.valid).to.be.false(); + }); + + it('shows concatenated suggestions from zxcvbn if there is no specific warning', async () => { + createElement(); + + const input: HTMLInputElement = screen.getByRole('textbox'); + await userEvent.type(input, 'PASSWORD'); + + expect( + screen.getByText( + 'zxcvbn.feedback.add_another_word_or_two_uncommon_words_are_better. ' + + 'zxcvbn.feedback.all_uppercase_is_almost_as_easy_to_guess_as_all_lowercase', + ), + ).to.exist(); + expect(input.validity.valid).to.be.false(); + }); + + it('shows feedback for a password that satisfies zxcvbn but is too short', async () => { + const element = createElement(); + + const input: HTMLInputElement = screen.getByRole('textbox'); + await userEvent.type(input, 'mRd@fX!f&G'); + + expect(element.getAttribute('score')).to.equal('2'); + expect(screen.getByText('instructions.password.strength.2')).to.exist(); + expect(screen.getByText('errors.attributes.password.too_short.other')).to.exist(); + expect(input.validity.valid).to.be.false(); + }); + + it('shows feedback for a password that is valid', async () => { + const element = createElement(); + + const input: HTMLInputElement = screen.getByRole('textbox'); + await userEvent.type(input, 'mRd@fX!f&G?_*'); + + expect(element.getAttribute('score')).to.equal('4'); + expect(screen.getByText('instructions.password.strength.4')).to.exist(); + expect( + element.querySelector('.password-strength__feedback')!.textContent!.trim(), + ).to.be.empty(); + expect(input.validity.valid).to.be.true(); + }); +}); diff --git a/app/javascript/packages/password-strength/password-strength-element.ts b/app/javascript/packages/password-strength/password-strength-element.ts new file mode 100644 index 00000000000..c8a0735e71e --- /dev/null +++ b/app/javascript/packages/password-strength/password-strength-element.ts @@ -0,0 +1,182 @@ +import zxcvbn from 'zxcvbn'; +import { t } from '@18f/identity-i18n'; +import type { ZXCVBNResult, ZXCVBNScore } from 'zxcvbn'; + +const MINIMUM_STRENGTH: ZXCVBNScore = 3; + +const snakeCase = (string: string): string => + string.replace(/[ -]/g, '_').replace(/\W/g, '').toLowerCase(); + +class PasswordStrengthElement extends HTMLElement { + connectedCallback() { + this.input.addEventListener('input', () => this.#handleValueChange()); + } + + get strength(): HTMLElement { + return this.querySelector('.password-strength__strength')!; + } + + get feedback(): HTMLElement { + return this.querySelector('.password-strength__feedback')!; + } + + get input(): HTMLInputElement { + return this.ownerDocument.getElementById(this.getAttribute('input-id')!) as HTMLInputElement; + } + + get minimumLength(): number { + return Number(this.getAttribute('minimum-length')!); + } + + get forbiddenPasswords(): string[] { + return JSON.parse(this.getAttribute('forbidden-passwords')!); + } + + /** + * Returns a normalized score on zxcvbn's scale. Notably, this artificially lowers a score if it + * does not meet the minimum length requires, to avoid confusion where an invalid value would + * display as being a great password. + * + * @param result zxcvbn result + * + * @return Normalized zxcvbn score + */ + #getNormalizedScore(result: ZXCVBNResult): ZXCVBNScore { + const { score } = result; + + if (score >= MINIMUM_STRENGTH && this.input.value.length < this.minimumLength) { + return Math.max(MINIMUM_STRENGTH - 1, 0) as ZXCVBNScore; + } + + return score; + } + + /** + * Returns true if the input's value is considered valid for submission, or false otherwise. + * + * @param result zxcvbn result + * + * @return Whether the input's value is valid for submission + */ + #isValid(result: ZXCVBNResult): boolean { + return result.score >= MINIMUM_STRENGTH && this.input.value.length >= this.minimumLength; + } + + /** + * Given a zxcvbn default feedback string hardcoded in English, returns a localized equivalent + * string translated to the current language. + * + * @param englishFeedback Default feedback string from zxcvbn + * + * @return Localized equivalent string translated to the current language + */ + #getLocalizedFeedback(englishFeedback: string): string { + // i18n-tasks-use t('zxcvbn.feedback.a_word_by_itself_is_easy_to_guess') + // i18n-tasks-use t('zxcvbn.feedback.add_another_word_or_two_uncommon_words_are_better') + // i18n-tasks-use t('zxcvbn.feedback.all_uppercase_is_almost_as_easy_to_guess_as_all_lowercase') + // i18n-tasks-use t('zxcvbn.feedback.avoid_dates_and_years_that_are_associated_with_you') + // i18n-tasks-use t('zxcvbn.feedback.avoid_recent_years') + // i18n-tasks-use t('zxcvbn.feedback.avoid_repeated_words_and_characters') + // i18n-tasks-use t('zxcvbn.feedback.avoid_sequences') + // i18n-tasks-use t('zxcvbn.feedback.avoid_years_that_are_associated_with_you') + // i18n-tasks-use t('zxcvbn.feedback.capitalization_doesnt_help_very_much') + // i18n-tasks-use t('zxcvbn.feedback.common_names_and_surnames_are_easy_to_guess') + // 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') + // i18n-tasks-use t('zxcvbn.feedback.repeats_like_abcabcabc_are_only_slightly_harder_to_guess_than_abc') + // i18n-tasks-use t('zxcvbn.feedback.reversed_words_arent_much_harder_to_guess') + // i18n-tasks-use t('zxcvbn.feedback.sequences_like_abc_or_6543_are_easy_to_guess') + // i18n-tasks-use t('zxcvbn.feedback.short_keyboard_patterns_are_easy_to_guess') + // i18n-tasks-use t('zxcvbn.feedback.straight_rows_of_keys_are_easy_to_guess') + // i18n-tasks-use t('zxcvbn.feedback.there_is_no_need_for_symbols_digits_or_uppercase_letters') + // i18n-tasks-use t('zxcvbn.feedback.this_is_a_top_100_common_password') + // 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(englishFeedback)}`); + } + + /** + * Returns text to be shown as feedback for the current input value, based on the zxcvbn result + * and other factors such as minimum password length or use of a forbidden password. + * + * @param result zxcvbn result + * + * @return Localized feedback text + */ + #getNormalizedFeedback(result: ZXCVBNResult): string | null { + const { warning, suggestions } = result.feedback; + + if (this.forbiddenPasswords.includes(this.input.value)) { + return t('errors.attributes.password.avoid_using_phrases_that_are_easily_guessed'); + } + + if (warning) { + return this.#getLocalizedFeedback(warning); + } + + if (suggestions.length) { + return suggestions.map((suggestion) => this.#getLocalizedFeedback(suggestion)).join('. '); + } + + if (this.input.value.length < this.minimumLength) { + return t('errors.attributes.password.too_short.other', { count: this.minimumLength }); + } + + return null; + } + + /** + * Returns the strength label associated with a given score. + * + * @param score Score + * + * @return Strength label. + */ + #getStrengthLabel(score: number): string { + // i18n-tasks-use t('instructions.password.strength.0') + // i18n-tasks-use t('instructions.password.strength.1') + // i18n-tasks-use t('instructions.password.strength.2') + // i18n-tasks-use t('instructions.password.strength.3') + // i18n-tasks-use t('instructions.password.strength.4') + return t(`instructions.password.strength.${score}`); + } + + /** + * Updates the current strength and feedback indicators in response to a changed input value. + */ + #handleValueChange() { + const hasValue = !!this.input.value; + this.classList.toggle('display-none', !hasValue); + this.removeAttribute('score'); + if (hasValue) { + const result = zxcvbn(this.input.value, this.forbiddenPasswords); + const score = this.#getNormalizedScore(result); + this.setAttribute('score', String(score)); + this.input.setCustomValidity( + this.#isValid(result) ? '' : t('errors.messages.stronger_password'), + ); + this.strength.textContent = this.#getStrengthLabel(score); + this.feedback.textContent = this.#getNormalizedFeedback(result); + } + } +} + +declare global { + interface HTMLElementTagNameMap { + 'lg-password-strength': PasswordStrengthElement; + } +} + +if (!customElements.get('lg-password-strength')) { + customElements.define('lg-password-strength', PasswordStrengthElement); +} + +export default PasswordStrengthElement; diff --git a/app/javascript/packs/pw-strength.js b/app/javascript/packs/pw-strength.js deleted file mode 100644 index 54143a7ce8c..00000000000 --- a/app/javascript/packs/pw-strength.js +++ /dev/null @@ -1,144 +0,0 @@ -import zxcvbn from 'zxcvbn'; -import { t } from '@18f/identity-i18n'; - -// zxcvbn returns a strength score from 0 to 4 -// we map those scores to: -// 1. a CSS class to the pw strength module -// 2. text describing the score -const scale = { - 0: ['pw-very-weak', t('instructions.password.strength.i')], - 1: ['pw-weak', t('instructions.password.strength.ii')], - 2: ['pw-average', t('instructions.password.strength.iii')], - 3: ['pw-good', t('instructions.password.strength.iv')], - 4: ['pw-great', t('instructions.password.strength.v')], -}; - -const snakeCase = (string) => string.replace(/[ -]/g, '_').replace(/\W/g, '').toLowerCase(); - -// fallback if zxcvbn lookup fails / field is empty -const fallback = ['pw-na', '...']; - -function getStrength(z) { - // override the strength value to 2 if the password is < 12 - if (z.password.length < 12 && z.score >= 3) { - z.score = 2; - } - return z && z.password.length ? scale[z.score] : fallback; -} - -export function getFeedback(z, minPasswordLength, forbiddenPasswords) { - if (!z || !z.password) { - return ' '; - } - - const { warning, suggestions } = z.feedback; - - if (forbiddenPasswords.includes(z.password)) { - return t('errors.attributes.password.avoid_using_phrases_that_are_easily_guessed'); - } - - if (!warning && !suggestions.length) { - if (z.password.length < minPasswordLength) { - return t('errors.attributes.password.too_short.other', { count: minPasswordLength }); - } - - return ' '; - } - - function lookup(str) { - // i18n-tasks-use t('zxcvbn.feedback.a_word_by_itself_is_easy_to_guess') - // i18n-tasks-use t('zxcvbn.feedback.add_another_word_or_two_uncommon_words_are_better') - // i18n-tasks-use t('zxcvbn.feedback.all_uppercase_is_almost_as_easy_to_guess_as_all_lowercase') - // i18n-tasks-use t('zxcvbn.feedback.avoid_dates_and_years_that_are_associated_with_you') - // i18n-tasks-use t('zxcvbn.feedback.avoid_recent_years') - // i18n-tasks-use t('zxcvbn.feedback.avoid_repeated_words_and_characters') - // i18n-tasks-use t('zxcvbn.feedback.avoid_sequences') - // i18n-tasks-use t('zxcvbn.feedback.avoid_years_that_are_associated_with_you') - // i18n-tasks-use t('zxcvbn.feedback.capitalization_doesnt_help_very_much') - // i18n-tasks-use t('zxcvbn.feedback.common_names_and_surnames_are_easy_to_guess') - // 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') - // i18n-tasks-use t('zxcvbn.feedback.repeats_like_abcabcabc_are_only_slightly_harder_to_guess_than_abc') - // i18n-tasks-use t('zxcvbn.feedback.reversed_words_arent_much_harder_to_guess') - // i18n-tasks-use t('zxcvbn.feedback.sequences_like_abc_or_6543_are_easy_to_guess') - // i18n-tasks-use t('zxcvbn.feedback.short_keyboard_patterns_are_easy_to_guess') - // i18n-tasks-use t('zxcvbn.feedback.straight_rows_of_keys_are_easy_to_guess') - // i18n-tasks-use t('zxcvbn.feedback.there_is_no_need_for_symbols_digits_or_uppercase_letters') - // i18n-tasks-use t('zxcvbn.feedback.this_is_a_top_100_common_password') - // 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)}`); - } - - if (warning) { - return lookup(warning); - } - - return `${suggestions.map((s) => lookup(s)).join('. ')}`; -} - -/** - * @param {HTMLElement?} element - * - * @return {string[]} - */ -export function getForbiddenPasswords(element) { - try { - return JSON.parse(element.dataset.forbidden); - } catch { - return []; - } -} - -function updatePasswordFeedback(cls, strength, feedback) { - const pwCntnr = document.getElementById('pw-strength-cntnr'); - const pwStrength = document.getElementById('pw-strength-txt'); - const pwFeedback = document.getElementById('pw-strength-feedback'); - - pwCntnr.className = cls; - pwStrength.innerHTML = strength; - pwFeedback.innerHTML = feedback; -} - -function validatePasswordField(score, input) { - if (score < 3) { - input.setCustomValidity(t('errors.messages.stronger_password')); - } else { - input.setCustomValidity(''); - } -} - -function checkPasswordStrength(password, minPasswordLength, forbiddenPasswords, input) { - const z = zxcvbn(password, forbiddenPasswords); - const [cls, strength] = getStrength(z); - const feedback = getFeedback(z, minPasswordLength, forbiddenPasswords); - - validatePasswordField(z.score, input); - updatePasswordFeedback(cls, strength, feedback); -} - -function analyzePw() { - const input = - document.querySelector('.password-toggle__input') || - document.querySelector('.password-confirmation__input'); - const forbiddenPasswordsElement = document.querySelector('[data-forbidden]'); - const forbiddenPasswords = getForbiddenPasswords(forbiddenPasswordsElement); - const minPasswordLength = document - .getElementById('pw-strength-cntnr') - .getAttribute('data-pw-min-length'); - - input.addEventListener('input', (e) => { - const password = e.target.value; - checkPasswordStrength(password, minPasswordLength, forbiddenPasswords, input); - }); -} - -document.addEventListener('DOMContentLoaded', analyzePw); diff --git a/app/views/devise/passwords/edit.html.erb b/app/views/devise/passwords/edit.html.erb index 2d17b9e9020..508af264d4b 100644 --- a/app/views/devise/passwords/edit.html.erb +++ b/app/views/devise/passwords/edit.html.erb @@ -17,16 +17,14 @@ <%= render PasswordConfirmationComponent.new( form: f, password_label: t('forms.passwords.edit.labels.password'), + forbidden_passwords: @forbidden_passwords, field_options: { input_html: { aria: { describedby: 'password-description' }, }, }, ) %> - <%= render 'devise/shared/password_strength', forbidden_passwords: @forbidden_passwords %> <%= f.submit t('forms.passwords.edit.buttons.submit'), class: 'display-block margin-y-5' %> <% end %> <%= render 'shared/password_accordion' %> - -<%= javascript_packs_tag_once 'pw-strength' %> diff --git a/app/views/devise/shared/_password_strength.html.erb b/app/views/devise/shared/_password_strength.html.erb deleted file mode 100644 index f1e3d3a77a5..00000000000 --- a/app/views/devise/shared/_password_strength.html.erb +++ /dev/null @@ -1,21 +0,0 @@ - diff --git a/app/views/event_disavowal/new.html.erb b/app/views/event_disavowal/new.html.erb index 137f751e5fa..38412a44d6b 100644 --- a/app/views/event_disavowal/new.html.erb +++ b/app/views/event_disavowal/new.html.erb @@ -16,14 +16,16 @@ label: t('forms.passwords.edit.labels.password'), required: true, input_html: { + id: 'new-password', autocomplete: 'new-password', }, }, ) %> - <%= render 'devise/shared/password_strength', forbidden_passwords: @forbidden_passwords %> + <%= render PasswordStrengthComponent.new( + input_id: 'new-password', + forbidden_passwords: @forbidden_passwords, + ) %> <%= f.submit t('forms.passwords.edit.buttons.submit'), class: 'margin-bottom-4' %> <% end %> <%= render 'shared/password_accordion' %> - -<%= javascript_packs_tag_once 'pw-strength' %> diff --git a/app/views/sign_up/passwords/new.html.erb b/app/views/sign_up/passwords/new.html.erb index 70b18b831c8..f0d3a80144b 100644 --- a/app/views/sign_up/passwords/new.html.erb +++ b/app/views/sign_up/passwords/new.html.erb @@ -14,11 +14,11 @@ <%= text_field_tag('username', @email_address.email, hidden: true, autocomplete: 'username') %> <%= render PasswordConfirmationComponent.new( form: f, + forbidden_passwords: @forbidden_passwords, field_options: { input_html: { aria: { describedby: 'password-description' } }, }, ) %> - <%= render 'devise/shared/password_strength', forbidden_passwords: @forbidden_passwords %> <%= hidden_field_tag :confirmation_token, @confirmation_token, id: 'confirmation_token' %> <%= f.submit t('forms.buttons.continue'), class: 'display-block margin-y-5' %> <% end %> @@ -26,5 +26,3 @@ <%= render 'shared/password_accordion' %> <%= render 'shared/cancel' %> - -<%= javascript_packs_tag_once 'pw-strength' %> diff --git a/app/views/users/passwords/edit.html.erb b/app/views/users/passwords/edit.html.erb index fd08abe5f5b..45a479f38f1 100644 --- a/app/views/users/passwords/edit.html.erb +++ b/app/views/users/passwords/edit.html.erb @@ -14,18 +14,16 @@ <%= render PasswordConfirmationComponent.new( form: f, password_label: t('forms.passwords.edit.labels.password'), + forbidden_passwords: @forbidden_passwords, field_options: { input_html: { aria: { describedby: 'password-description' }, }, }, ) %> - <%= render 'devise/shared/password_strength', forbidden_passwords: @forbidden_passwords %> <%= f.submit t('forms.buttons.submit.update'), class: 'display-block margin-top-5 margin-bottom-4' %> <% end %> <%= render 'shared/password_accordion' %> <%= render 'shared/cancel', link: account_path %> - -<%= javascript_packs_tag_once 'pw-strength' %> diff --git a/config/locales/instructions/en.yml b/config/locales/instructions/en.yml index 0783d62a70e..63edaca651b 100644 --- a/config/locales/instructions/en.yml +++ b/config/locales/instructions/en.yml @@ -88,12 +88,12 @@ en: you verified your identity with this account. If you don’t have it, you can still reset your password and then reverify your identity. strength: - i: Very weak - ii: Weak - iii: Average + 0: Very weak + 1: Weak + 2: Average + 3: Good + 4: Great intro: 'Password strength: ' - iv: Good - v: Great sp_handoff_bounced: Your sign in was successful, but %{sp_name} sent you back to %{app_name}. Please contact %{sp_link} for help. sp_handoff_bounced_with_no_sp: your service provider diff --git a/config/locales/instructions/es.yml b/config/locales/instructions/es.yml index 52014ed2233..37a4532fb86 100644 --- a/config/locales/instructions/es.yml +++ b/config/locales/instructions/es.yml @@ -93,12 +93,12 @@ es: con ella, de todos modos puede restablecer su contraseña y luego volver a verificar su identidad. strength: - i: Muy débil - ii: Débil - iii: Promedio + 0: Muy débil + 1: Débil + 2: Promedio + 3: Buena + 4: 'Muy buena' intro: 'Seguridad de la contraseña:' - iv: Buena - v: 'Muy buena' sp_handoff_bounced: Su inicio de sesión fue exitoso, pero %{sp_name} lo envió de regreso a %{app_name}. Póngase en contacto con %{sp_link} para obtener ayuda. diff --git a/config/locales/instructions/fr.yml b/config/locales/instructions/fr.yml index 0fdb4d98fe4..ec2e74cfe48 100644 --- a/config/locales/instructions/fr.yml +++ b/config/locales/instructions/fr.yml @@ -105,12 +105,12 @@ fr: avec ce compte. Si vous ne l’avez pas, vous pouvez toujours réinitialiser votre mot de passe et ensuite revérifier votre identité. strength: - i: Très faible - ii: Faible - iii: Moyen + 0: Très faible + 1: Faible + 2: Moyen + 3: Bonne + 4: Excellente intro: 'Force du mot de passe : ' - iv: Bonne - v: Excellente sp_handoff_bounced: Votre connexion a réussi, mais %{sp_name} vous a renvoyé à %{app_name}. Veuillez contacter %{sp_link} pour obtenir de l’aide. sp_handoff_bounced_with_no_sp: votre fournisseur de service diff --git a/package.json b/package.json index 16b3ef2886d..c1c8fb04671 100644 --- a/package.json +++ b/package.json @@ -40,8 +40,7 @@ "source-map-loader": "^4.0.0", "webpack": "^5.76.1", "webpack-assets-manifest": "^5.1.0", - "webpack-cli": "^4.10.0", - "zxcvbn": "4.4.2" + "webpack-cli": "^4.10.0" }, "devDependencies": { "@babel/cli": "^7.22.15", @@ -61,6 +60,7 @@ "@types/react-dom": "^17.0.11", "@types/sinon": "^10.0.13", "@types/sinon-chai": "^3.2.8", + "@types/zxcvbn": "^4.4.4", "@typescript-eslint/eslint-plugin": "^6.7.5", "@typescript-eslint/parser": "^6.7.5", "chai": "^4.3.10", diff --git a/spec/components/password_confirmation_component_spec.rb b/spec/components/password_confirmation_component_spec.rb index 0116d517731..f88d773359a 100644 --- a/spec/components/password_confirmation_component_spec.rb +++ b/spec/components/password_confirmation_component_spec.rb @@ -3,9 +3,10 @@ RSpec.describe PasswordConfirmationComponent, type: :component do let(:view_context) { vc_test_controller.view_context } let(:form) { SimpleForm::FormBuilder.new('', {}, view_context, {}) } + let(:options) { { form: } } subject(:rendered) do - render_inline PasswordConfirmationComponent.new(form:) + render_inline PasswordConfirmationComponent.new(**options) end it 'renders password fields with expected attributes' do @@ -22,15 +23,9 @@ end context 'with labels passed in' do - subject(:rendered) do - render_inline PasswordConfirmationComponent.new( - form:, - password_label: password_label, - confirmation_label: confirmation_label, - ) - end let(:password_label) { 'edited password label' } let(:confirmation_label) { 'edited password confirmation label' } + let(:options) { super().merge(password_label:, confirmation_label:) } it 'renders custom password label' do expect(rendered).to have_content(password_label) @@ -40,4 +35,17 @@ expect(rendered).to have_content(confirmation_label) end end + + context 'with forbidden passwords' do + let(:forbidden_passwords) { ['password'] } + let(:options) { super().merge(forbidden_passwords:) } + + it 'forwards forbidden passwords to rendered password strength component' do + expect(PasswordStrengthComponent).to receive(:new). + with(hash_including(forbidden_passwords:)). + and_call_original + + rendered + end + end end diff --git a/spec/components/password_strength_component_spec.rb b/spec/components/password_strength_component_spec.rb new file mode 100644 index 00000000000..1f2d7d6ba87 --- /dev/null +++ b/spec/components/password_strength_component_spec.rb @@ -0,0 +1,33 @@ +require 'rails_helper' + +RSpec.describe PasswordStrengthComponent, type: :component do + let(:input_id) { 'input' } + let(:options) { { input_id: } } + + subject(:rendered) { render_inline PasswordStrengthComponent.new(**options) } + + it 'renders with default attributes' do + element = rendered.at_css('lg-password-strength') + + expect(element.attr('input-id')).to eq('input') + expect(element.attr('minimum-length')).to eq('12') + expect(element.attr('forbidden-passwords')).to eq('[]') + expect(element.attr('class')).to eq('display-none') + end + + context 'with customized options' do + let(:minimum_length) { 10 } + let(:forbidden_passwords) { ['password'] } + let(:tag_options) { { class: 'example-class', data: { foo: 'bar' } } } + let(:options) { super().merge(minimum_length:, forbidden_passwords:, **tag_options) } + + it 'renders with customized option attributes' do + element = rendered.at_css('lg-password-strength') + + expect(element.attr('minimum-length')).to eq('10') + expect(element.attr('forbidden-passwords')).to eq('["password"]') + expect(element.attr('class').split(' ')).to match_array(['example-class', 'display-none']) + expect(element.attr('data-foo')).to eq('bar') + end + end +end diff --git a/spec/components/password_toggle_component_spec.rb b/spec/components/password_toggle_component_spec.rb index 726a9f8a27f..efdd3c434a5 100644 --- a/spec/components/password_toggle_component_spec.rb +++ b/spec/components/password_toggle_component_spec.rb @@ -64,6 +64,15 @@ expect(toggle_two.input_id).to be_present expect(toggle_one.input_id).not_to eq(toggle_two.input_id) end + + context 'with field_options customizing id' do + let(:options) { { field_options: { input_html: { id: 'custom' } } } } + + it 'respects customized id' do + expect(rendered).to have_css('#custom[type=password]') + expect(rendered).to have_css('[aria-controls=custom]') + end + end end context 'with tag options' do diff --git a/spec/components/previews/password_strength_component_preview.rb b/spec/components/previews/password_strength_component_preview.rb new file mode 100644 index 00000000000..aa1b954072c --- /dev/null +++ b/spec/components/previews/password_strength_component_preview.rb @@ -0,0 +1,31 @@ +class PasswordStrengthComponentPreview < BaseComponentPreview + # @after_render :inject_input_html + # @!group Preview + def default + render(PasswordStrengthComponent.new(input_id: 'preview-input')) + end + # @!endgroup + + # @after_render :inject_input_html + # @param minimum_length text + # @param forbidden_passwords text + def workbench(minimum_length: '12', forbidden_passwords: 'password') + render( + PasswordStrengthComponent.new( + input_id: 'preview-input', + minimum_length:, + forbidden_passwords: forbidden_passwords.split(','), + ), + ) + end + + private + + def inject_input_html(html, _context) + <<~HTML + + + #{html} + HTML + end +end diff --git a/spec/features/event_disavowal_spec.rb b/spec/features/event_disavowal_spec.rb index 661bedc7130..41fc36ea09c 100644 --- a/spec/features/event_disavowal_spec.rb +++ b/spec/features/event_disavowal_spec.rb @@ -131,6 +131,24 @@ expect(page.current_path).to eq(login_two_factor_path(otp_delivery_preference: :sms)) end + scenario 'disavowing an event with javascript enabled', :js do + perform_disavowable_password_reset + + open_last_email + click_email_link_matching(%r{events/disavow}) + + expect(page).to have_content(t('headings.passwords.change')) + + fill_in t('forms.passwords.edit.labels.password'), with: 'abc' + + expect(page).to have_content t('zxcvbn.feedback.sequences_like_abc_or_6543_are_easy_to_guess') + + fill_in t('forms.passwords.edit.labels.password'), with: 'NewVal!dPassw0rd' + click_button t('forms.passwords.edit.buttons.submit') + + expect(page).to have_content(t('devise.passwords.updated_not_active')) + end + def submit_prefilled_otp_code(user, delivery_preference) expect(current_path). to eq login_two_factor_path(otp_delivery_preference: delivery_preference) diff --git a/spec/features/users/user_profile_spec.rb b/spec/features/users/user_profile_spec.rb index 3c134230883..edc060d8b99 100644 --- a/spec/features/users/user_profile_spec.rb +++ b/spec/features/users/user_profile_spec.rb @@ -123,7 +123,7 @@ with: 'this is a great sentence' expect(page).to have_content(t('instructions.password.strength.intro')) - expect(page).to have_content t('instructions.password.strength.v') + expect(page).to have_content t('instructions.password.strength.4') check t('components.password_toggle.toggle_label') diff --git a/spec/features/visitors/set_password_spec.rb b/spec/features/visitors/set_password_spec.rb index 13556b8c812..6c7edfe9d99 100644 --- a/spec/features/visitors/set_password_spec.rb +++ b/spec/features/visitors/set_password_spec.rb @@ -28,7 +28,7 @@ create(:user, :unconfirmed) confirm_last_user - expect(page).to have_css('#pw-strength-cntnr.display-none') + expect(page).to have_css('lg-password-strength.display-none') end context 'password strength indicator when JS is on', js: true do @@ -42,13 +42,13 @@ fill_in t('forms.password'), with: 'password' expect(page).to have_content(t('instructions.password.strength.intro')) - expect(page).to have_content t('instructions.password.strength.i') + expect(page).to have_content t('instructions.password.strength.0') fill_in t('forms.password'), with: '123456789' expect(page).to have_content t('zxcvbn.feedback.this_is_a_top_10_common_password') fill_in t('forms.password'), with: 'this is a great sentence' - expect(page).to have_content t('instructions.password.strength.v') + expect(page).to have_content t('instructions.password.strength.4') fill_in t('forms.password'), with: ':b/}6tT#,' expect(page).to have_content t('errors.attributes.password.too_short.other', count: 12) diff --git a/spec/i18n_spec.rb b/spec/i18n_spec.rb index aea5b92134e..d5648c3cb64 100644 --- a/spec/i18n_spec.rb +++ b/spec/i18n_spec.rb @@ -174,14 +174,6 @@ def allowed_untranslated_key?(locale, key) bad_keys = keys.reject { |key| key =~ /^[a-z0-9_.]+$/ } expect(bad_keys).to be_empty end - - it 'has only has XML-safe identifiers (keys start with a letter)' do - keys = flattened_yaml_data.keys - - bad_keys = keys.select { |key| key.split('.').any? { |part| part =~ /^[0-9]/ } } - - expect(bad_keys).to be_empty - end end it 'has correctly-formatted interpolation values' do diff --git a/spec/javascript/packs/pw-strength-spec.js b/spec/javascript/packs/pw-strength-spec.js deleted file mode 100644 index b5b7700cbf0..00000000000 --- a/spec/javascript/packs/pw-strength-spec.js +++ /dev/null @@ -1,79 +0,0 @@ -import zxcvbn from 'zxcvbn'; -import { getForbiddenPasswords, getFeedback } from '../../../app/javascript/packs/pw-strength'; - -describe('pw-strength', () => { - describe('getForbiddenPasswords', () => { - it('returns empty array if given argument is null', () => { - const element = null; - const result = getForbiddenPasswords(element); - - expect(result).to.deep.equal([]); - }); - - it('returns empty array if element has absent dataset value', () => { - const element = document.createElement('span'); - const result = getForbiddenPasswords(element); - - expect(result).to.deep.equal([]); - }); - - it('returns empty array if element has invalid dataset value', () => { - const element = document.createElement('span'); - element.setAttribute('data-forbidden', 'nil'); - const result = getForbiddenPasswords(element); - - expect(result).to.deep.equal([]); - }); - - it('parsed array of forbidden passwords', () => { - const element = document.createElement('span'); - element.setAttribute('data-forbidden', '["foo","bar","baz"]'); - const result = getForbiddenPasswords(element); - - expect(result).to.be.deep.equal(['foo', 'bar', 'baz']); - }); - }); - - describe('getFeedback', () => { - const EMPTY_RESULT = ' '; - const MINIMUM_LENGTH = 12; - const FORBIDDEN_PASSWORDS = ['gsa', 'Login.gov']; - - it('returns an empty result for empty password', () => { - const z = zxcvbn(''); - - expect(getFeedback(z, MINIMUM_LENGTH, FORBIDDEN_PASSWORDS)).to.equal(EMPTY_RESULT); - }); - - it('returns an empty result for a strong password', () => { - const z = zxcvbn('!Juq2Uk2**RBEsA8'); - - expect(getFeedback(z, MINIMUM_LENGTH, FORBIDDEN_PASSWORDS)).to.equal(EMPTY_RESULT); - }); - - it('returns feedback for a weak password', () => { - const z = zxcvbn('password'); - - expect(getFeedback(z, MINIMUM_LENGTH, FORBIDDEN_PASSWORDS)).to.equal( - 'zxcvbn.feedback.this_is_a_top_10_common_password', - ); - }); - - it('shows feedback when a password is too short', () => { - const z = zxcvbn('_3G%JMyR"'); - - expect(getFeedback(z, MINIMUM_LENGTH, FORBIDDEN_PASSWORDS)).to.equal( - 'errors.attributes.password.too_short.other', - { count: MINIMUM_LENGTH }, - ); - }); - - it('shows feedback when a user enters a forbidden password', () => { - const z = zxcvbn('gsa'); - - expect(getFeedback(z, MINIMUM_LENGTH, FORBIDDEN_PASSWORDS)).to.equal( - 'errors.attributes.password.avoid_using_phrases_that_are_easily_guessed', - ); - }); - }); -}); diff --git a/spec/views/devise/shared/_password_strength.html.erb_spec.rb b/spec/views/devise/shared/_password_strength.html.erb_spec.rb deleted file mode 100644 index 7d04b321dc9..00000000000 --- a/spec/views/devise/shared/_password_strength.html.erb_spec.rb +++ /dev/null @@ -1,35 +0,0 @@ -require 'rails_helper' - -RSpec.describe 'devise/shared/_password_strength.html.erb' do - describe 'forbidden attributes' do - context 'when local is unassigned' do - before do - render - end - - it 'omits data-forbidden attribute from strength text tag' do - expect(rendered).to have_selector('#pw-strength-txt:not([data-forbidden])') - end - end - - context 'when local is nil' do - before do - render 'devise/shared/password_strength', forbidden_passwords: nil - end - - it 'omits data-forbidden attribute from strength text tag' do - expect(rendered).to have_selector('#pw-strength-txt:not([data-forbidden])') - end - end - - context 'when local is assigned' do - before do - render 'devise/shared/password_strength', forbidden_passwords: ['a', 'b', 'c'] - end - - it 'adds JSON-encoded data-forbidden to strength text tag' do - expect(rendered).to have_selector('#pw-strength-txt[data-forbidden="[\"a\",\"b\",\"c\"]"]') - end - end - end -end diff --git a/tsconfig.json b/tsconfig.json index 1c56906d480..e697d8bc798 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -25,10 +25,5 @@ "./*.js", "scripts" ], - "exclude": [ - "**/fixtures", - "**/*.spec.js", - "app/javascript/packs/pw-strength.js", - "app/javascript/packs/saml-post.js" - ] + "exclude": ["**/fixtures", "**/*.spec.js", "app/javascript/packs/saml-post.js"] } diff --git a/yarn.lock b/yarn.lock index 24a103d9ec2..68b1b76ee51 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1684,6 +1684,11 @@ dependencies: "@types/yargs-parser" "*" +"@types/zxcvbn@^4.4.4": + version "4.4.4" + resolved "https://registry.yarnpkg.com/@types/zxcvbn/-/zxcvbn-4.4.4.tgz#987f5fcd87e957097433c476c3a1c91a54f53131" + integrity sha512-Tuk4q7q0DnpzyJDI4aMeghGuFu2iS1QAdKpabn8JfbtfGmVDUgvZv1I7mEjP61Bvnp3ljKCC8BE6YYSTNxmvRQ== + "@typescript-eslint/eslint-plugin@^6.7.5": version "6.7.5" resolved "https://registry.yarnpkg.com/@typescript-eslint/eslint-plugin/-/eslint-plugin-6.7.5.tgz#f4024b9f63593d0c2b5bd6e4ca027e6f30934d4f" @@ -7315,7 +7320,7 @@ yocto-queue@^0.1.0: resolved "https://registry.yarnpkg.com/yocto-queue/-/yocto-queue-0.1.0.tgz#0294eb3dee05028d31ee1a5fa2c556a6aaf10a1b" integrity sha512-rVksvsnNCdJ/ohGc6xgPwyN8eheCxsiLM8mxuE/t/mOVqJewPuO1miLpTHQiRgTKCLexL4MeAFVagts7HmNZ2Q== -zxcvbn@4.4.2: +zxcvbn@^4.4.2: version "4.4.2" resolved "https://registry.yarnpkg.com/zxcvbn/-/zxcvbn-4.4.2.tgz#28ec17cf09743edcab056ddd8b1b06262cc73c30" integrity sha1-KOwXzwl0PtyrBW3dixsGJizHPDA=