Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
41 changes: 41 additions & 0 deletions app/components/password_confirmation_component.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<%= content_tag(:'lg-password-confirmation', **tag_options) do %>
<%= render ValidatedFieldComponent.new(
form: form,
name: :password,
type: :password,
label: default_label,
required: true,
**field_options,
input_html: field_options[:input_html].to_h.merge(
id: input_id,
class: ['password-confirmation__input1', *field_options.dig(:input_html, :class)],
),
) %>
<%= render ValidatedFieldComponent.new(
form: form,
name: :password_confirmation,
type: :password_confirmation,
Comment on lines +16 to +17
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this will need to be feature flagged in some manner since it will be backwards incompatible when running alongside a version that does not require password_confirmation.

label: confirmation_label,
required: true,
**field_options,
input_html: field_options[:input_html].to_h.merge(
id: input_confirmation_id,
class: ['password-confirmation__input2', *field_options.dig(:input_html, :class)],
),
error_messages: {
valueMissing: t('components.password_confirmation.errors.empty'),
}
) %>
<input
id="<%= toggle_id %>"
type="checkbox"
class="usa-checkbox__input password-toggle__toggle"
aria-controls="<%= input_id %>"
>
<label
for="<%= toggle_id %>"
class="usa-checkbox__label password-toggle__toggle-label"
>
<%= toggle_label %>
</label>
<% end %>
36 changes: 36 additions & 0 deletions app/components/password_confirmation_component.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
class PasswordConfirmationComponent < BaseComponent
attr_reader :form, :label, :toggle_label, :field_options, :tag_options

def initialize(
form:,
toggle_label: t('components.password_toggle.toggle_label'),
field_options: {},
**tag_options
)
@form = form
@label = label
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks like the assigned value here may not exist in scope. Should it be a constructor argument?

@toggle_label = toggle_label
@field_options = field_options
@tag_options = tag_options
end

def default_label
t('components.password_confirmation.label')
end

def confirmation_label
t('components.password_confirmation.confirm_label')
end

def toggle_id
"password-toggle-#{unique_id}"
end

def input_id
"password-input-#{unique_id}"
end

def input_confirmation_id
"password-confirmation-input-#{unique_id}"
end
end
1 change: 1 addition & 0 deletions app/components/password_confirmation_component.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import '@18f/identity-password-confirmation/password-confirmation-element';
2 changes: 1 addition & 1 deletion app/controllers/sign_up/passwords_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def render_page
end

def permitted_params
params.require(:password_form).permit(:confirmation_token, :password, :request_id)
params.require(:password_form).permit(:confirmation_token, :password, :password_confirmation, :request_id)
end

def process_successful_password_creation
Expand Down
5 changes: 2 additions & 3 deletions app/forms/password_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,10 @@ def initialize(user)
end

def submit(params)
submitted_password = params[:password]
@password = params[:password]
@password_confirmation = params[:password_confirmation]
@request_id = params.fetch(:request_id, '')

self.password = submitted_password

FormResponse.new(success: valid?, errors: errors, extra: extra_analytics_attributes)
end

Expand Down
13 changes: 13 additions & 0 deletions app/javascript/packages/password-confirmation/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"name": "@18f/identity-password-confirmation",
"private": true,
"version": "1.0.0",
"peerDependencies": {
"react": "^17.0.2"
},
"peerDependenciesMeta": {
"react": {
"optional": true
}
}
Comment on lines +4 to +12
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It doesn't appear that we're using React in the package.

Suggested change
"version": "1.0.0",
"peerDependencies": {
"react": "^17.0.2"
},
"peerDependenciesMeta": {
"react": {
"optional": true
}
}
"version": "1.0.0"

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import userEvent from '@testing-library/user-event';
import { getByLabelText } from '@testing-library/dom';
import { useSandbox } from '@18f/identity-test-helpers';
import * as analytics from '@18f/identity-analytics';
import './password-confirmation-element';
import type PasswordConfirmationElement from './password-confirmation-element';

describe('PasswordConfirmationElement', () => {
let idCounter = 0;
const sandbox = useSandbox();

function createElement() {
const element = document.createElement(
'lg-password-confirmation',
) as PasswordConfirmationElement;
const idSuffix = ++idCounter;
element.innerHTML = `
<label for="input-${idSuffix}">Password</label>
<input id="input-${idSuffix}" class="password-confirmation__input1">
<label for="input-${idSuffix}b">Confirm password</label>
<input id="input-${idSuffix}b" class="password-confirmation__input2">
<div class="password-toggle__toggle-wrapper">
<input
id="toggle-${idSuffix}"
type="checkbox"
class="password-toggle__toggle"
aria-controls="input-${idSuffix}"
>
<label for="toggle-${idSuffix}" class="usa-checkbox__label password-toggle__toggle-label">
Show password
</label>
</div>`;
document.body.appendChild(element);
return element;
}

it('initializes input type', () => {
const element = createElement();

const input = getByLabelText(element, 'Password') as HTMLInputElement;

expect(input.type).to.equal('password');
});

it('changes input type on toggle', async () => {
const element = createElement();

const input = getByLabelText(element, 'Password') as HTMLInputElement;
const toggle = getByLabelText(element, 'Show password') as HTMLInputElement;

await userEvent.click(toggle);

expect(input.type).to.equal('text');
});

it('logs an event when clicking the Show Password button', async () => {
sandbox.stub(analytics, 'trackEvent');
const element = createElement();
const toggle = getByLabelText(element, 'Show password') as HTMLInputElement;

await userEvent.click(toggle);

expect(analytics.trackEvent).to.have.been.calledWith('Show Password button clicked', {
path: window.location.pathname,
});
});

it('should validate password confirmation', async () => {
const element = createElement();
const input1 = getByLabelText(element, 'Password') as HTMLInputElement;
const input2 = getByLabelText(element, 'Confirm password') as HTMLInputElement;

await userEvent.type(input1, 'different_password1');
await userEvent.type(input2, 'different_password2');
expect(input2.validity.customError).to.be.true;

await userEvent.type(input1, 'matching_password!');
await userEvent.type(input2, 'matching_password!');
expect(input2.validity.customError).to.be.false;
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import { trackEvent } from '@18f/identity-analytics';
import { t } from '@18f/identity-i18n';

class PasswordConfirmationElement extends HTMLElement {
connectedCallback() {
this.toggle.addEventListener('change', () => this.setInputType());
this.toggle.addEventListener('click', () => this.trackToggleEvent());
this.input_confirmation.addEventListener('change', () => this.validatePassword());
this.setInputType();
}

/**
* Checkbox toggle for visibility.
*/
get toggle(): HTMLInputElement {
return this.querySelector('.password-toggle__toggle')! as HTMLInputElement;
}

/**
* Text or password input.
*/
get input(): HTMLInputElement {
return this.querySelector('.password-confirmation__input1')! as HTMLInputElement;
}

/**
* Text or password confirmation input.
*/
get input_confirmation(): HTMLInputElement {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should use camel-case in JavaScript (typically I'd expect the linter to flag this, but I don't know if it applies to class function names).

Suggested change
get input_confirmation(): HTMLInputElement {
get inputConfirmation(): HTMLInputElement {

return this.querySelector('.password-confirmation__input2')! as HTMLInputElement;
Comment on lines +23 to +30
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If we're calling these "input" and "input confirmation" in the code, maybe we could represent the class names similarly for consistency?

Suggested change
return this.querySelector('.password-confirmation__input1')! as HTMLInputElement;
}
/**
* Text or password confirmation input.
*/
get input_confirmation(): HTMLInputElement {
return this.querySelector('.password-confirmation__input2')! as HTMLInputElement;
return this.querySelector('.password-confirmation__input')! as HTMLInputElement;
}
/**
* Text or password confirmation input.
*/
get input_confirmation(): HTMLInputElement {
return this.querySelector('.password-confirmation__input-confirmation')! as HTMLInputElement;

}

setInputType() {
const checked = this.toggle.checked ? 'text' : 'password';
this.input.type = checked;
this.input_confirmation.type = checked;
}

trackToggleEvent() {
trackEvent('Show Password button clicked', { path: window.location.pathname });
}

validatePassword() {
const password = this.input.value;
const confirmation = this.input_confirmation.value;

if (password && password !== confirmation) {
const errorMsg = t('components.password_confirmation.errors.mismatch');
this.input_confirmation.setCustomValidity(errorMsg);
} else {
this.input_confirmation.setCustomValidity('');
}
}
}

declare global {
interface HTMLElementTagNameMap {
'lg-password-confirmation': PasswordConfirmationElement;
}
}

if (!customElements.get('lg-password-confirmation')) {
customElements.define('lg-password-confirmation', PasswordConfirmationElement);
}

export default PasswordConfirmationElement;
55 changes: 29 additions & 26 deletions app/javascript/packs/pw-strength.js
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I expect we're going to hit some conflicts in this file between this and #8085, just a heads-up between you and @jmdembe .

Original file line number Diff line number Diff line change
Expand Up @@ -89,44 +89,47 @@ export function getForbiddenPasswords(element) {
}
}

function analyzePw() {
const input = document.querySelector('.password-toggle__input');
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, forbiddenPasswords, input) {
const z = zxcvbn(password, forbiddenPasswords);
const [cls, strength] = getStrength(z);
const feedback = getFeedback(z);

validatePasswordField(z.score, input);
updatePasswordFeedback(cls, strength, feedback);
}

function analyzePw() {
const input = document.querySelector('.password-toggle__input') || document.querySelector('.password-confirmation__input1');
const forbiddenPasswordsElement = document.querySelector('[data-forbidden]');
const forbiddenPasswords = getForbiddenPasswords(forbiddenPasswordsElement);

// the pw strength module is hidden by default ("display-none" CSS class)
// (so that javascript disabled browsers won't see it)
// thus, first step is unhiding it
const pwCntnr = document.getElementById('pw-strength-cntnr');
pwCntnr.className = '';

function updatePasswordFeedback(cls, strength, feedback) {
pwCntnr.className = cls;
pwStrength.innerHTML = strength;
pwFeedback.innerHTML = feedback;
}

function validatePasswordField(score) {
if (score < 3) {
input.setCustomValidity(t('errors.messages.stronger_password'));
} else {
input.setCustomValidity('');
}
}

function checkPasswordStrength(password) {
const z = zxcvbn(password, forbiddenPasswords);
const [cls, strength] = getStrength(z);
const feedback = getFeedback(z);

validatePasswordField(z.score);
updatePasswordFeedback(cls, strength, feedback);
}

input.addEventListener('input', (e) => {
checkPasswordStrength(e.target.value);
const password = e.target.value;
checkPasswordStrength(password, forbiddenPasswords, input);
});
}

Expand Down
12 changes: 11 additions & 1 deletion app/validators/form_password_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,19 @@ module FormPasswordValidator
extend ActiveSupport::Concern

included do
attr_accessor :password
attr_accessor :password, :password_confirmation
attr_reader :user

validates :password,
presence: true,
length: { in: Devise.password_length }
validates :password_confirmation,
presence: true,
length: { in: Devise.password_length }
validate :password_graphemes_length
validate :strong_password
validate :not_pwned
validate :passwords_match
end

private
Expand Down Expand Up @@ -50,6 +54,12 @@ def not_pwned
errors.add :password, :pwned_password, type: :pwned_password
end

def passwords_match
if password != password_confirmation
errors.add(:password_confirmation, :confirmation, type: :mismatch)
end
end

def min_password_score
IdentityConfig.store.min_password_score
end
Expand Down
7 changes: 1 addition & 6 deletions app/views/sign_up/passwords/new.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,8 @@
method: :post,
html: { autocomplete: 'off' },
) do |f| %>
<%= render PasswordToggleComponent.new(
<%= render PasswordConfirmationComponent.new(
form: f,
field_options: {
label: t('forms.password'),
required: true,
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' %>
Expand Down
Loading