Skip to content
Merged
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
1 change: 1 addition & 0 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
"indent": "off",
"max-classes-per-file": "off",
"newline-per-chained-call": "off",
"no-empty": ["error", { "allowEmptyCatch": true }],
"no-param-reassign": ["off", "never"],
"no-confusing-arrow": "off",
"no-plusplus": "off",
Expand Down
8 changes: 8 additions & 0 deletions app/controllers/event_disavowal_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ def new
extra: EventDisavowal::BuildDisavowedEventAnalyticsAttributes.call(disavowed_event),
).to_h,
)
@forbidden_passwords = forbidden_passwords
end

def create
Expand All @@ -20,12 +21,19 @@ def create
if result.success?
handle_successful_password_reset
else
@forbidden_passwords = forbidden_passwords
render :new
end
end

private

def forbidden_passwords
disavowed_event.user.email_addresses.flat_map do |email_address|
ForbiddenPasswords.new(email_address.email).call
end
end

def password_reset_from_disavowal_form
@password_reset_from_disavowal_form ||= EventDisavowal::PasswordResetFromDisavowalForm.new(
disavowed_event,
Expand Down
1 change: 1 addition & 0 deletions app/controllers/users/reset_passwords_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ def handle_unsuccessful_password_reset(result)
return
end

@forbidden_passwords = forbidden_passwords(resource.email_addresses)
render :edit
end

Expand Down
26 changes: 22 additions & 4 deletions app/javascript/packs/pw-strength.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,17 +96,35 @@ function disableSubmit(submitEl, length = 0, score = 0) {
}
}

/**
* @param {HTMLElement?} element
*
* @return {string[]}
*/
export function getForbiddenPasswords(element) {
try {
return JSON.parse(element.dataset.forbidden);
} catch {
return [];
}
}

function analyzePw() {
const { userAgent } = window.navigator;
const input = document.querySelector(
'#password_form_password, #reset_password_form_password, #update_user_password_form_password',
[
'#password_form_password',
'#event_disavowal_password_reset_from_disavowal_form_password',
'#reset_password_form_password',
'#update_user_password_form_password',
].join(','),
);
const pwCntnr = document.getElementById('pw-strength-cntnr');
const pwStrength = document.getElementById('pw-strength-txt');
const pwFeedback = document.getElementById('pw-strength-feedback');
const submit = document.querySelector('input[type="submit"]');
const forbiddenPasswordsElement = document.querySelector('[data-forbidden-passwords]');
const { forbiddenPasswords } = forbiddenPasswordsElement.dataset;
const forbiddenPasswordsElement = document.querySelector('[data-forbidden]');
const forbiddenPasswords = getForbiddenPasswords(forbiddenPasswordsElement);

disableSubmit(submit);

Expand All @@ -116,7 +134,7 @@ function analyzePw() {
pwCntnr.className = '';

function checkPasswordStrength(e) {
const z = zxcvbn(e.target.value, JSON.parse(forbiddenPasswords));
const z = zxcvbn(e.target.value, forbiddenPasswords);
const [cls, strength] = getStrength(z);
const feedback = getFeedback(z);
pwCntnr.className = cls;
Expand Down
2 changes: 1 addition & 1 deletion app/views/devise/passwords/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
<%= f.full_error :reset_password_token %>
<%= f.input :password, label: t('forms.passwords.edit.labels.password'), required: true,
input_html: { class: 'password-toggle' } %>
<%= render 'devise/shared/password_strength' %>
<%= render 'devise/shared/password_strength', forbidden_passwords: @forbidden_passwords %>
<%= f.button :submit, t('forms.passwords.edit.buttons.submit'), class: 'mb3' %>
<% end %>

Expand Down
6 changes: 3 additions & 3 deletions app/views/devise/shared/_password_strength.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@
<span class='h6'>
<%= t('instructions.password.strength.intro') %>
</span>
<span id='pw-strength-txt' class='bold' data-forbidden-passwords='<%= @forbidden_passwords %>'>
...
</span>
<%= tag.span '...', id: 'pw-strength-txt', class: 'bold', data: {
forbidden: local_assigns[:forbidden_passwords],
} %>
</div>
<div class='h6'>
<div id='pw-strength-feedback' class='italic'>
Expand Down
2 changes: 1 addition & 1 deletion app/views/event_disavowal/new.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
input_html: { value: @disavowal_token, name: :disavowal_token } %>
<%= f.input :password, label: t('forms.passwords.edit.labels.password'), required: true,
input_html: { aria: { invalid: false }, class: 'password-toggle' } %>
<%= render 'devise/shared/password_strength' %>
<%= render 'devise/shared/password_strength', forbidden_passwords: @forbidden_passwords %>
<%= f.button :submit, t('forms.passwords.edit.buttons.submit'), class: 'mb3' %>
<% end %>

Expand Down
2 changes: 1 addition & 1 deletion app/views/sign_up/passwords/new.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<%= f.input :password, required: true,
input_html: { aria: { invalid: false, describedby: 'password-description' },
class: 'password-toggle' } %>
<%= render 'devise/shared/password_strength' %>
<%= render 'devise/shared/password_strength', forbidden_passwords: @forbidden_passwords %>
<%= hidden_field_tag :confirmation_token, @confirmation_token, id: 'confirmation_token' %>
<%= f.input :request_id, as: :hidden, input_html: { value: params[:request_id] || request_id } %>
<div>
Expand Down
2 changes: 1 addition & 1 deletion app/views/users/passwords/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
<%= f.error_notification %>
<%= f.input :password, label: t('forms.passwords.edit.labels.password'), required: true,
input_html: { aria: { invalid: false, describedby: 'password-description' }, class: 'password-toggle' } %>
<%= render 'devise/shared/password_strength' %>
<%= render 'devise/shared/password_strength', forbidden_passwords: @forbidden_passwords %>
<%= f.button :submit, t('forms.buttons.submit.update'), class: 'mt2 mb3' %>
<% end %>

Expand Down
46 changes: 46 additions & 0 deletions spec/controllers/event_disavowal_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,17 @@

get :new, params: { disavowal_token: disavowal_token }
end

it 'assigns forbidden passwords' do
expect(@analytics).to receive(:track_event).with(
Analytics::EVENT_DISAVOWAL,
build_analytics_hash,
)

get :new, params: { disavowal_token: disavowal_token }

expect(assigns(:forbidden_passwords)).to all(be_a(String))
end
end

context 'with an invalid disavowal_token' do
Expand All @@ -39,6 +50,22 @@

get :new, params: { disavowal_token: disavowal_token }
end

it 'does not assign forbidden passwords' do
event.update!(disavowed_at: Time.zone.now)

expect(@analytics).to receive(:track_event).with(
Analytics::EVENT_DISAVOWAL_TOKEN_INVALID,
build_analytics_hash(
success: false,
errors: { event: [t('event_disavowals.errors.event_already_disavowed')] },
),
)

get :new, params: { disavowal_token: disavowal_token }

expect(assigns(:forbidden_passwords)).to be_nil
end
end
end

Expand Down Expand Up @@ -74,6 +101,25 @@

post :create, params: params
end

it 'assigns forbidden passwords' do
expect(@analytics).to receive(:track_event).with(
Analytics::EVENT_DISAVOWAL_PASSWORD_RESET,
build_analytics_hash(
success: false,
errors: { password: ['is too short (minimum is 12 characters)'] },
),
)

params = {
disavowal_token: disavowal_token,
event_disavowal_password_reset_from_disavowal_form: { password: 'too short' },
}

post :create, params: params

expect(assigns(:forbidden_passwords)).to all(be_a(String))
end
end

context 'with an invalid disavowal_token' do
Expand Down
1 change: 1 addition & 0 deletions spec/controllers/users/reset_passwords_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@

put :update, params: { reset_password_form: form_params }

expect(assigns(:forbidden_passwords)).to all(be_a(String))
expect(response).to render_template(:edit)
end
end
Expand Down
43 changes: 43 additions & 0 deletions spec/javascripts/packs/pw-strength-spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
describe('pw-strength', () => {
let getForbiddenPasswords;
before(async () => {
window.LoginGov = { I18n: { t() {} } };
({ getForbiddenPasswords } = await import('../../../app/javascript/packs/pw-strength'));
});

after(() => {
delete window.LoginGov;
});

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']);
});
});
});
35 changes: 35 additions & 0 deletions spec/views/devise/shared/_password_strength.html.erb_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
require 'rails_helper'

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