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
11 changes: 11 additions & 0 deletions app/assets/stylesheets/components/_btn.scss
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,17 @@
}
}

.btn-danger {
background-color: $red;
border: 1px solid $red;
border-radius: $border-radius-lg;
color: $white;

&:hover {
background-color: $red;
}
}

.btn-wide {
box-sizing: border-box;
min-width: 220px;
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/concerns/two_factor_authenticatable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -269,15 +269,15 @@ def decorated_user

def reenter_phone_number_path
locale = LinkLocaleResolver.locale
if MfaContext.new(current_user).phone_configurations.any?
if MfaPolicy.new(current_user).two_factor_enabled?
manage_phone_path(locale: locale)
else
phone_setup_path(locale: locale)
end
end

def confirmation_for_phone_change?
confirmation_context? && MfaContext.new(current_user).phone_configurations.any?
confirmation_context? && MfaPolicy.new(current_user).two_factor_enabled?
end

def presenter_for_two_factor_authentication_method
Expand Down
14 changes: 14 additions & 0 deletions app/controllers/users/phones_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,20 @@ def update
end
end

def delete
result = TwoFactorAuthentication::PhoneDeletionForm.new(
current_user, phone_configuration
).submit
analytics.track_event(Analytics::PHONE_DELETION_REQUESTED, result.to_h)
if result.success?
flash[:success] = t('two_factor_authentication.phone.delete.success')
else
flash[:error] = t('two_factor_authentication.phone.delete.failure')
end

redirect_to account_url
end

private

# we only allow editing of the first configuration since we'll eventually be
Expand Down
2 changes: 0 additions & 2 deletions app/controllers/users/reset_passwords_controller.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
# rubocop:disable Metrics/ClassLength
module Users
class ResetPasswordsController < Devise::PasswordsController
include RecaptchaConcern
Expand Down Expand Up @@ -132,4 +131,3 @@ def assert_reset_token_passed
end
end
end
# rubocop:enable Metrics/ClassLength
48 changes: 48 additions & 0 deletions app/forms/two_factor_authentication/phone_deletion_form.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
module TwoFactorAuthentication
class PhoneDeletionForm
include ActiveModel::Model

attr_reader :user, :configuration

validates :user, multiple_mfa_options: true
validates :configuration, allow_nil: true, owned_by_user: true

def initialize(user, configuration)
@user = user
@configuration = configuration
end

def submit
success = configuration.blank? || valid? && configuration_destroyed

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

private

def extra_analytics_attributes
{
configuration_present: configuration.present?,
configuration_id: configuration&.id,
configuration_owner: configuration&.user&.uuid,
mfa_method_counts: MfaContext.new(user.reload).enabled_two_factor_configuration_counts_hash,
}
end

def configuration_destroyed
if configuration.destroy != false
user.phone_configurations.reload
update_remember_device_revoked_at
true
else
errors.add(:configuration, :not_destroyed, message: 'cannot delete phone')
false
end
end

def update_remember_device_revoked_at
attributes = { remember_device_revoked_at: Time.zone.now }
UpdateUser.new(user: user, attributes: attributes).call
end
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.

What is the attack vector here? The user and configuration that are passed in to the form are based on the current user. In what scenario would the configuration not be owned by the user?

end
end
4 changes: 4 additions & 0 deletions app/models/auth_app_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,8 @@ def selection_presenters
def friendly_name
:auth_app
end

def self.selection_presenters(set)
set.flat_map(&:selection_presenters)
end
end
4 changes: 4 additions & 0 deletions app/models/personal_key_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,8 @@ def selection_presenters
[]
end
end

def self.selection_presenters(set)
set.flat_map(&:selection_presenters)
end
end
4 changes: 4 additions & 0 deletions app/models/phone_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,8 @@ def selection_presenters
def friendly_name
:phone
end

def self.selection_presenters(set)
set.flat_map(&:selection_presenters)
end
end
4 changes: 4 additions & 0 deletions app/models/piv_cac_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,8 @@ def selection_presenters
def friendly_name
:piv_cac
end

def self.selection_presenters(set)
set.flat_map(&:selection_presenters)
end
end
8 changes: 8 additions & 0 deletions app/models/webauthn_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,12 @@ def selection_presenters
def friendly_name
:webauthn
end

def self.selection_presenters(set)
if set.any?
set.first.selection_presenters
else
[]
end
end
end
23 changes: 1 addition & 22 deletions app/presenters/two_factor_login_options_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def options
# webauthn keys and phones. However, we only want to show one of each option
# during login, except for phones, where we want to allow the user to choose
# which MFA-enabled phone they want to use.
all_sms_and_voice_options_plus_one_of_each_remaining_option(configurations)
configurations.group_by(&:class).flat_map { |klass, set| klass.selection_presenters(set) }
end

def should_display_account_reset_or_cancel_link?
Expand All @@ -53,27 +53,6 @@ def account_reset_or_cancel_link

private

def all_sms_and_voice_options_plus_one_of_each_remaining_option(configurations)
presenters = configurations.flat_map(&:selection_presenters)
presenters_for_phone_options(presenters) + presenters_for_options_that_are_not_phone(presenters)
end

def presenters_for_options_that_are_not_phone(presenters)
presenters.select { |presenter| !phone_presenter_class?(presenter.class) }.uniq(&:class)
end

def presenters_for_phone_options(presenters)
presenters.select { |presenter| phone_presenter_class?(presenter.class) }
end

def phone_presenter_class?(class_name)
phone_classes = [
TwoFactorAuthentication::SmsSelectionPresenter,
TwoFactorAuthentication::VoiceSelectionPresenter,
]
phone_classes.include?(class_name)
end

def account_reset_link
t('two_factor_authentication.account_reset.text_html',
link: @view.link_to(
Expand Down
1 change: 1 addition & 0 deletions app/services/analytics.rb
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ def browser
PASSWORD_RESET_VISIT = 'Password Reset: Email Form Visited'.freeze
PERSONAL_KEY_VIEWED = 'Personal Key Viewed'.freeze
PHONE_CHANGE_REQUESTED = 'Phone Number Change: requested'.freeze
PHONE_DELETION_REQUESTED = 'Phone Number Deletion: requested'.freeze
PROFILE_ENCRYPTION_INVALID = 'Profile Encryption: Invalid'.freeze
PROFILE_PERSONAL_KEY_CREATE = 'Profile: Created new personal key'.freeze
RATE_LIMIT_TRIGGERED = 'Rate Limit Triggered'.freeze
Expand Down
7 changes: 7 additions & 0 deletions app/validators/multiple_mfa_options_validator.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
class MultipleMfaOptionsValidator < ActiveModel::EachValidator
# :reek:UtilityFunction
def validate_each(record, attribute, value)
return if MfaPolicy.new(value).multiple_factors_enabled?
record.errors.add attribute, 'must have multiple MFA configurations'
end
end
7 changes: 7 additions & 0 deletions app/validators/owned_by_user_validator.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
class OwnedByUserValidator < ActiveModel::EachValidator
# :reek:UtilityFunction
def validate_each(record, attribute, value)
return if value.user&.id == record.user&.id
record.errors.add attribute, 'must be owned by the user'
end
end
4 changes: 4 additions & 0 deletions app/view_models/account_show.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ def edit_action_partial
'accounts/actions/edit_action_button'
end

def manage_action_partial
'accounts/actions/manage_action_button'
end

def pii_partial
if decrypted_pii.present?
'accounts/pii'
Expand Down
2 changes: 1 addition & 1 deletion app/views/accounts/_phone.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@
.col.col-8.sm-6.truncate
= phone_configuration.phone
.col.col-4.sm-6.right-align
= render @view_model.edit_action_partial,
= render @view_model.manage_action_partial,
path: manage_phone_url,
name: t('account.index.phone')
4 changes: 4 additions & 0 deletions app/views/accounts/actions/_manage_action_button.html.slim
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
= link_to path do
span.hide
= name
= t('forms.buttons.manage')
8 changes: 7 additions & 1 deletion app/views/users/phones/edit.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,13 @@ h1.h3.my0 = t('headings.edit_info.phone')
= f.input :phone, as: :tel, label: false, required: true,
input_html: { class: 'phone col-8 mb4' }
= render 'users/shared/otp_delivery_preference_selection'
= f.button :submit, t('forms.buttons.submit.confirm_change')
= f.button :submit, t('forms.buttons.submit.confirm_change'), class: 'btn-wide'
- if MfaPolicy.new(current_user).multiple_factors_enabled? && @user_phone_form.phone.present?
br
.sm-col-8.mb3
= button_to t('forms.phone.buttons.delete'), manage_phone_url, \
class: 'btn btn-danger btn-wide', \
method: :delete
= render 'shared/cancel', link: account_path

= stylesheet_link_tag 'intl-tel-number/intlTelInput'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,3 @@
class: :otp_delivery_preference_voice
span.indicator
= t('two_factor_authentication.otp_delivery_preference.voice')
p.mb0.mt1
= link_to t('links.two_factor_authentication.app_option'), authenticator_setup_path
4 changes: 4 additions & 0 deletions config/locales/forms/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ en:
disable: Disable
edit: Edit
enable: Enable
manage: Manage
resend_confirmation: Resend confirmation instructions
send_security_code: Send code
submit:
Expand All @@ -34,6 +35,9 @@ en:
instructions: Please confirm you have a copy of your personal key by entering
it below.
title: Enter your personal key
phone:
buttons:
delete: Remove Phone
piv_cac_mfa:
submit: Present PIV/CAC card
piv_cac_setup:
Expand Down
4 changes: 4 additions & 0 deletions config/locales/forms/es.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ es:
disable: Suspender
edit: Editar
enable: Permitir
manage: Administrar
resend_confirmation: Reenviar instrucciones de confirmación
send_security_code: Enviar código
submit:
Expand All @@ -34,6 +35,9 @@ es:
instructions: Confirme que tiene una copia de su clave personal ingresándola
a continuación.
title: Ingrese su clave personal
phone:
buttons:
delete: Eliminar el teléfono
piv_cac_mfa:
submit: Presentar tarjeta PIV/CAC
piv_cac_setup:
Expand Down
4 changes: 4 additions & 0 deletions config/locales/forms/fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ fr:
disable: Désactiver
edit: Modifier
enable: Activer
manage: Administrer
resend_confirmation: Envoyer les instructions de confirmation de nouveau
send_security_code: Envoyer le code
submit:
Expand All @@ -34,6 +35,9 @@ fr:
instructions: Veuillez confirmer que vous avez une copie de votre clé personnelle
en l'entrant ci-dessous.
title: Entrez votre clé personnelle
phone:
buttons:
delete: Supprimer le numéro de teléfono
piv_cac_mfa:
submit: Veuillez présenter une carte PIV/CAC
piv_cac_setup:
Expand Down
2 changes: 1 addition & 1 deletion config/locales/headings/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ en:
edit_info:
email: Change your email
password: Change your password
phone: Enter your new phone number
phone: Manage your phone number
lock_failure: Here's what you can do
passwords:
change: Change your password
Expand Down
2 changes: 1 addition & 1 deletion config/locales/headings/es.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ es:
edit_info:
email: Cambie su email
password: Cambie su contraseña
phone: Ingrese su nuevo número de teléfono
phone: Administrar su número de teléfono
lock_failure: Esto es lo que puedes hacer
passwords:
change: Cambie su contraseña
Expand Down
2 changes: 1 addition & 1 deletion config/locales/headings/fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ fr:
edit_info:
email: Changez votre courriel
password: Changez votre mot de passe
phone: Entrez votre nouveau numéro de téléphone
phone: Administrer votre numéro de téléphone
lock_failure: Voici ce que vous pouvez faire
passwords:
change: Changez votre mot de passe
Expand Down
1 change: 0 additions & 1 deletion config/locales/links/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ en:
sign_in: Sign in
sign_out: Sign out
two_factor_authentication:
app_option: Use an authentication application instead.
get_another_code: Get another code
what_is_totp: What is an authentication app?
what_is_webauthn: What is a hardware security key?
1 change: 0 additions & 1 deletion config/locales/links/es.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ es:
sign_in: Iniciar sesión
sign_out: Cerrar sesión
two_factor_authentication:
app_option: Use una aplicación de autenticación en su lugar.
get_another_code: Obtener otro código
what_is_totp: "¿Qué es una app de autenticación?"
what_is_webauthn: "¿Qué es una clave de seguridad de hardware?"
1 change: 0 additions & 1 deletion config/locales/links/fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ fr:
sign_in: Connexion
sign_out: Déconnexion
two_factor_authentication:
app_option: Utilisez une application d'authentification à la place.
get_another_code: Obtenir un autre code
what_is_totp: Qu'est-ce qu'une application d'authentification?
what_is_webauthn: Qu'est-ce qu'une clé de sécurité physique?
4 changes: 4 additions & 0 deletions config/locales/two_factor_authentication/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ en:
personal_key_header_text: Enter your personal key
personal_key_prompt: You can use this personal key once. After you enter it, you'll
be provided a new key.
phone:
delete:
success: Your phone has been removed.
failure: Unable to remove your phone.
phone_fallback:
question: Don't have access to your phone right now?
phone_sms_info_html: We'll text a security code <strong>each time you sign in</strong>.
Expand Down
4 changes: 4 additions & 0 deletions config/locales/two_factor_authentication/es.yml
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ es:
personal_key_header_text: Ingrese su clave personal
personal_key_prompt: Puede usar esta clave personal una vez. Después de ingresarlo,
se le dará una nueva clave.
phone:
delete:
success: Su teléfono ha sido eliminado.
failure: No se puede eliminar el teléfono.
phone_fallback:
question: "¿No tiene acceso a su teléfono ahora mismo?"
phone_sms_info_html: Le enviaremos un mensaje de texto con un código de seguridad
Expand Down
Loading