Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
b9ac0b4
changelog: Upcoming Features, Authentication, let users who's passwor…
mdiarra3 Jun 24, 2024
b9a05ed
add password compromised spec and edit objects
mdiarra3 Jun 25, 2024
d4e032f
sign in spec updated
mdiarra3 Jun 25, 2024
9b4d476
change to use user_password_params
mdiarra3 Jun 25, 2024
a858e41
Merge remote-tracking branch 'origin/main' into LG-13014-implement-pa…
mdiarra3 Jun 26, 2024
9e60712
address comments
mdiarra3 Jun 26, 2024
4b4fed0
Merge remote-tracking branch 'origin/main' into LG-13014-implement-pa…
mdiarra3 Jun 27, 2024
556a3d8
change up to use password controller
mdiarra3 Jun 28, 2024
ada95a2
password compromised updates
mdiarra3 Jul 1, 2024
f0df22e
fix lint and use global variable
mdiarra3 Jul 1, 2024
d9dfe06
update edit spec and global variable
mdiarra3 Jul 1, 2024
ffba90b
fix the sign in spec
mdiarra3 Jul 1, 2024
93911fe
fix form spec
mdiarra3 Jul 1, 2024
14fdbbe
address comments, update test to better redirect
mdiarra3 Jul 2, 2024
a5893a0
remove former check
mdiarra3 Jul 2, 2024
96a02e0
make sure invalid password method is correct
mdiarra3 Jul 2, 2024
13cfe98
Update to make aria described conditional
mdiarra3 Jul 3, 2024
0b27511
Merge remote-tracking branch 'origin/main' into LG-13014-implement-pa…
mdiarra3 Jul 3, 2024
6baa426
update specs and use hash format for attributes instead of positional…
mdiarra3 Jul 8, 2024
a5a68a3
fix reset password form spec
mdiarra3 Jul 8, 2024
db4182d
remove conditional for password strength description
mdiarra3 Jul 29, 2024
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
2 changes: 1 addition & 1 deletion app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ def fix_broken_personal_key_url
def after_sign_in_path_for(_user)
return rules_of_use_path if !current_user.accepted_rules_of_use_still_valid?
return user_please_call_url if current_user.suspended?
return user_password_compromised_url if session[:redirect_to_password_compromised].present?
return manage_password_url if session[:redirect_to_change_password].present?
return authentication_methods_setup_url if user_needs_sp_auth_method_setup?
return login_add_piv_cac_prompt_url if session[:needs_to_setup_piv_cac_after_sign_in].present?
return fix_broken_personal_key_url if current_user.broken_personal_key?
Expand Down
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 @@ -63,7 +63,7 @@ def process_successful_password_creation
end

def password_form
@password_form ||= PasswordForm.new(@user)
@password_form ||= PasswordForm.new(user: @user)
end

def process_unsuccessful_password_creation
Expand Down
19 changes: 0 additions & 19 deletions app/controllers/users/password_compromised_controller.rb

This file was deleted.

53 changes: 33 additions & 20 deletions app/controllers/users/passwords_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,22 @@ class PasswordsController < ApplicationController
before_action :confirm_recently_authenticated_2fa

def edit
analytics.edit_password_visit
@update_user_password_form = UpdateUserPasswordForm.new(current_user)
@forbidden_passwords = current_user.email_addresses.flat_map do |email_address|
ForbiddenPasswords.new(email_address.email).call
end
@update_password_presenter = UpdatePasswordPresenter.new(
user: current_user,
required_password_change: required_password_change?,
)
analytics.edit_password_visit(required_password_change: required_password_change?)
@update_user_password_form = UpdateUserPasswordForm.new(user: current_user)
end

def update
@update_user_password_form = UpdateUserPasswordForm.new(current_user, user_session)
@update_user_password_form = UpdateUserPasswordForm.new(
user: current_user,
user_session: user_session,
required_password_change: required_password_change?,
)

result = @update_user_password_form.submit(user_params)
result = @update_user_password_form.submit(user_password_params)

analytics.password_changed(**result.to_h)

Expand All @@ -39,8 +44,8 @@ def capture_password_if_pii_present_but_locked
redirect_to capture_password_url
end

def user_params
params.require(:update_user_password_form).permit(:password, :password_confirmation)
def required_password_change?
session[:redirect_to_change_password] == true
end

def handle_valid_password
Expand All @@ -54,11 +59,27 @@ def handle_valid_password
if @update_user_password_form.personal_key.present?
user_session[:personal_key] = @update_user_password_form.personal_key
redirect_to manage_personal_key_url
elsif required_password_change?
session.delete(:redirect_to_change_password)
redirect_to after_sign_in_path_for(current_user)
else
redirect_to account_url
redirect_to account_path
end
end

def handle_invalid_password
# If the form is submitted with a password that's too short (based on
# our Devise config) but that zxcvbn treats as strong enough, then we
# need to provide our custom forbidden passwords data that zxcvbn needs,
# otherwise the JS will throw an exception and the password strength
# meter will not appear.
@update_password_presenter = UpdatePasswordPresenter.new(
user: current_user,
required_password_change: required_password_change?,
)
render :edit
end

def send_password_reset_risc_event
event = PushNotification::PasswordResetEvent.new(user: current_user)
PushNotification::HttpPush.deliver(event)
Expand All @@ -69,16 +90,8 @@ def create_event_and_notify_user_about_password_change
UserAlerts::AlertUserAboutPasswordChange.call(current_user, disavowal_token)
end

def handle_invalid_password
# If the form is submitted with a password that's too short (based on
# our Devise config) but that zxcvbn treats as strong enough, then we
# need to provide our custom forbidden passwords data that zxcvbn needs,
# otherwise the JS will throw an exception and the password strength
# meter will not appear.
@forbidden_passwords = current_user.email_addresses.flat_map do |email_address|
ForbiddenPasswords.new(email_address.email).call
end
render :edit
def user_password_params
params.require(:update_user_password_form).permit(:password, :password_confirmation)
end
end
end
4 changes: 2 additions & 2 deletions app/controllers/users/reset_passwords_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def edit

analytics.password_reset_token(**result.to_h)
if result.success?
@reset_password_form = ResetPasswordForm.new(build_user)
@reset_password_form = ResetPasswordForm.new(user: build_user)
@forbidden_passwords = forbidden_passwords(token_user.email_addresses)
else
handle_invalid_or_expired_token(result)
Expand All @@ -43,7 +43,7 @@ def edit
# PUT /resource/password
def update
self.resource = user_matching_token(user_params[:reset_password_token])
@reset_password_form = ResetPasswordForm.new(resource)
@reset_password_form = ResetPasswordForm.new(user: resource)

result = @reset_password_form.submit(user_params)

Expand Down
4 changes: 2 additions & 2 deletions app/controllers/users/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,9 @@ def handle_valid_authentication
user_id: current_user.id,
email: auth_params[:email],
)
check_password_compromised
user_session[:platform_authenticator_available] =
params[:platform_authenticator_available] == 'true'
check_password_compromised
redirect_to next_url_after_valid_authentication
end

Expand Down Expand Up @@ -243,7 +243,7 @@ def check_password_compromised
return if current_user.password_compromised_checked_at.present? ||
!eligible_for_password_lookup?

session[:redirect_to_password_compromised] =
session[:redirect_to_change_password] =
PwnedPasswords::LookupPassword.call(auth_params[:password])
update_user_password_compromised_checked_at
end
Expand Down
2 changes: 1 addition & 1 deletion app/forms/password_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class PasswordForm
include ActiveModel::Model
include FormPasswordValidator

def initialize(user)
def initialize(user:)
@user = user
@validate_confirmation = true
end
Expand Down
2 changes: 1 addition & 1 deletion app/forms/reset_password_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class ResetPasswordForm

validate :valid_token

def initialize(user)
def initialize(user:)
@user = user
@reset_password_token = @user.reset_password_token
@validate_confirmation = true
Expand Down
6 changes: 4 additions & 2 deletions app/forms/update_user_password_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ class UpdateUserPasswordForm

delegate :personal_key, to: :encryptor

def initialize(user, user_session = nil)
def initialize(user:, user_session: nil, required_password_change: false)
@user = user
@user_session = user_session
@required_password_change = required_password_change
@validate_confirmation = true
end

Expand All @@ -22,7 +23,7 @@ def submit(params)

private

attr_reader :user, :user_session
attr_reader :user, :user_session, :required_password_change

def process_valid_submission
user.update!(password: password)
Expand All @@ -48,6 +49,7 @@ def extra_analytics_attributes
active_profile_present: user.active_profile.present?,
pending_profile_present: user.pending_profile.present?,
user_id: user.uuid,
required_password_change: required_password_change,
}
end
end
34 changes: 34 additions & 0 deletions app/presenters/update_password_presenter.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# frozen_string_literal: true

class UpdatePasswordPresenter
attr_reader :user, :required_password_change

alias_method :required_password_change?, :required_password_change
def initialize(user:, required_password_change: false)
@user = user
@required_password_change = required_password_change
end

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

def aria_described_by_if_eligible
return {} if required_password_change?
Comment on lines +18 to +19
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.

Since we render the description for all views, do we need this condition anymore, or should we just always apply the aria-describedby ?

{
input_html: {
aria: { describedby: 'password-description' },
},
}
end

def submit_text
if required_password_change?
I18n.t('forms.passwords.edit.buttons.submit')
else
I18n.t('forms.buttons.submit.update')
end
end
end
9 changes: 7 additions & 2 deletions app/services/analytics_events.rb
Original file line number Diff line number Diff line change
Expand Up @@ -392,9 +392,14 @@ def doc_auth_warning(message: nil, **extra)
)
end

# @param [Boolean] required_password_change if user forced to change password
# When a user views the edit password page
def edit_password_visit
track_event('Edit Password Page Visited')
def edit_password_visit(required_password_change: false, **extra)
track_event(
'Edit Password Page Visited',
required_password_change: required_password_change,
**extra,
)
end

# @param [Boolean] success
Expand Down
15 changes: 0 additions & 15 deletions app/views/users/password_compromised/show.html.erb

This file was deleted.

24 changes: 16 additions & 8 deletions app/views/users/passwords/edit.html.erb
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
<% self.title = t('titles.edit_info.password') %>

<% if @update_password_presenter.required_password_change? %>
<%= render AlertComponent.new(
type: :warning,
class: 'margin-bottom-4',
) do
t('users.password_compromised.warning', app_name: APP_NAME)
end
%>
<% end %>

<%= render PageHeadingComponent.new.with_content(t('headings.edit_info.password')) %>

<p id="password-description">
Expand All @@ -11,16 +21,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' },
},
},
forbidden_passwords: @update_password_presenter.forbidden_passwords,
field_options: @update_password_presenter.aria_described_by_if_eligible,
) %>
<%= f.submit t('forms.buttons.submit.update'), class: 'display-block margin-top-5 margin-bottom-4' %>
<%= f.submit @update_password_presenter.submit_text, class: 'display-block margin-top-5 margin-bottom-4' %>
<% end %>

<%= render 'shared/password_accordion' %>

<%= render 'shared/cancel', link: account_path %>
<% unless @update_password_presenter.required_password_change? %>
<%= render 'shared/cancel', link: account_path %>
<% end %>
1 change: 1 addition & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1910,6 +1910,7 @@ users.delete.bullet_4: We will notify the agencies you access with %{app_name} t
users.delete.heading: Are you sure you want to delete your account?
users.delete.instructions: Enter your password to confirm that you want to delete your account.
users.delete.subheading: 'If you delete your account:'
users.password_compromised.warning: We found your password in a data breach on another site or app. %{app_name} requires you to change your password to protect your account.
users.personal_key.accessible_labels.code_example: A personal key example with 16 characters
users.personal_key.accessible_labels.preview: Personal key preview
users.personal_key.confirmation_error: You’ve entered an incorrect personal key.
Expand Down
1 change: 1 addition & 0 deletions config/locales/es.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1921,6 +1921,7 @@ users.delete.bullet_4: Notificaremos a las agencias a las que acceda con %{app_n
users.delete.heading: '¿Está seguro de que desea eliminar su cuenta?'
users.delete.instructions: Ingrese su contraseña para confirmar que desea eliminar su cuenta.
users.delete.subheading: 'Si elimina su cuenta:'
users.password_compromised.warning: Hemos encontrado su contraseña en una filtración de datos en otro sitio o aplicación. %{app_name} requiere que cambie su contraseña para proteger su cuenta.
users.personal_key.accessible_labels.code_example: Un ejemplo de clave personal con 16 caracteres
users.personal_key.accessible_labels.preview: Vista previa de la clave personal
users.personal_key.confirmation_error: Ingresó una clave personal incorrecta.
Expand Down
1 change: 1 addition & 0 deletions config/locales/fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1910,6 +1910,7 @@ users.delete.bullet_4: Nous informerons les organismes auxquels vous accédez av
users.delete.heading: Êtes-vous sûr de vouloir supprimer votre compte ?
users.delete.instructions: Saisissez votre mot de passe pour confirmer que vous souhaitez supprimer votre compte.
users.delete.subheading: 'Si vous supprimez votre compte :'
users.password_compromised.warning: Nous avons trouvé votre mot de passe dans le cadre d’une fuite de données sur un autre site Web ou une application. %{app_name} vous demande de modifier votre mot de passe pour protéger votre compte.
users.personal_key.accessible_labels.code_example: Un exemple de clé personnelle à 16 caractères
users.personal_key.accessible_labels.preview: Aperçu de clé personnelle
users.personal_key.confirmation_error: Vous avez saisi une clé personnelle erronée.
Expand Down
1 change: 1 addition & 0 deletions config/locales/zh.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1926,6 +1926,7 @@ users.delete.bullet_4: 我们将通知你使用 %{app_name} 访问的政府机
users.delete.heading: 你确定要删除账户吗?
users.delete.instructions: 输入密码来确认你要删除账户。
users.delete.subheading: 如果你删除自己的账户:
users.password_compromised.warning: 另一个网站或应用程序的数据泄露中我们发现有你的密码。%{app_name} 要求你更改密码来保护你的账户。
users.personal_key.accessible_labels.code_example: 有 16 个字符的个人密钥示例
users.personal_key.accessible_labels.preview: 个人密钥预览
users.personal_key.confirmation_error: 你输入的个人密钥不对
Expand Down
1 change: 0 additions & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,6 @@
get '/confirm_backup_codes' => 'users/backup_code_setup#confirm_backup_codes'

get '/user_please_call' => 'users/please_call#show'
get '/user_password_compromised' => 'users/password_compromised#show'

post '/sign_up/create_password' => 'sign_up/passwords#create', as: :sign_up_create_password
get '/sign_up/email/confirm' => 'sign_up/email_confirmations#create',
Expand Down
Loading