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
16 changes: 16 additions & 0 deletions app/assets/stylesheets/components/_btn.scss
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,19 @@
border-color: $gray;
color: $gray;
}

.btn-account-action {
border: 0;
color: $blue;
font-size: .8125rem;
font-weight: normal;
margin-bottom: -3px;
margin-top: -3px;
padding: .5rem;
padding-bottom: .125rem;
padding-top: .125rem;

a {
text-decoration: none;
}
}
28 changes: 28 additions & 0 deletions app/controllers/users/webauthn_setup_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,36 @@ def confirm
end
end

def delete
if current_user.total_mfa_options_enabled > 1
handle_successful_delete
else
handle_failed_delete
end
redirect_to account_url
end

private

def handle_successful_delete
WebauthnConfiguration.where(user_id: current_user.id, id: params[:id]).destroy_all
flash[:success] = t('notices.webauthn_deleted')
track_delete(true)
end

def handle_failed_delete
flash[:error] = t('errors.webauthn_setup.delete_last')
track_delete(false)
end

def track_delete(success)
analytics.track_event(
Analytics::WEBAUTHN_DELETED,
success: success,
mfa_options_enabled: current_user.total_mfa_options_enabled
)
end

def save_challenge_in_session
credential_creation_options = ::WebAuthn.credential_creation_options
user_session[:webauthn_challenge] = credential_creation_options[:challenge].bytes.to_a
Expand Down
12 changes: 11 additions & 1 deletion app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ def need_two_factor_authentication?(_request)
end

def two_factor_enabled?
phone_configurations.any?(&:mfa_enabled?) || totp_enabled? || piv_cac_enabled?
phone_configurations.any?(&:mfa_enabled?) || totp_enabled? || piv_cac_enabled? ||
webauthn_configurations.any?
end

def send_two_factor_authentication_code(_code)
Expand Down Expand Up @@ -159,5 +160,14 @@ def send_custom_confirmation_instructions(id = nil, instructions = nil)
send_devise_notification(:confirmation_instructions,
@raw_confirmation_token, opts)
end

def total_mfa_options_enabled
total = [phone_mfa_enabled?, piv_cac_enabled?, totp_enabled?].count { |tf| tf }
total + webauthn_configurations.size
end

def phone_mfa_enabled?
phone_configurations.any?(&:mfa_enabled?)
end
end
# rubocop:enable Rails/HasManyOrHasOneDependent
2 changes: 1 addition & 1 deletion app/models/webauthn_configuration.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
class WebauthnConfiguration < ApplicationRecord
belongs_to :user, inverse_of: :webauthn_configuration
belongs_to :user
validates :user_id, presence: true
validates :name, presence: true
validates :credential_id, presence: true
Expand Down
1 change: 1 addition & 0 deletions app/services/analytics.rb
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ def browser
USER_REGISTRATION_PIV_CAC_DISABLED = 'User Registration: piv cac disabled'.freeze
USER_REGISTRATION_PIV_CAC_ENABLED = 'User Registration: piv cac enabled'.freeze
USER_REGISTRATION_PIV_CAC_SETUP_VISIT = 'User Registration: piv cac setup visited'.freeze
WEBAUTHN_DELETED = 'WebAuthn Deleted'.freeze
WEBAUTHN_SETUP_VISIT = 'WebAuthn Setup Visited'.freeze
WEBAUTHN_SETUP_SUBMITTED = 'WebAuthn Setup Submitted'.freeze
# rubocop:enable Metrics/LineLength
Expand Down
4 changes: 0 additions & 4 deletions app/view_models/account_show.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,6 @@ def piv_cac_partial
end
end

def webauthn_partial
'accounts/actions/add_webauthn'
end

def manage_personal_key_partial
yield if decorated_user.password_reset_profile.blank?
end
Expand Down
15 changes: 15 additions & 0 deletions app/views/accounts/_webauthn.html.slim
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
.clearfix.border-top.border-blue-light
.p2.col.col-12
.col.col-6.bold
= t('account.index.webauthn')
.right-align.col.col-6
.btn.btn-account-action.rounded-lg.bg-light-blue
= link_to t('account.index.webauthn_add'), webauthn_setup_url
- current_user.webauthn_configurations.each do |cfg|
.p2.col.col-12.border-top.border-blue-light
.col.col-8.sm-6.truncate
= cfg.name
.col.col-4.sm-6.right-align
= button_to(t('account.index.webauthn_delete'), webauthn_setup_path(id: cfg.id),
method: :delete, class: 'btn btn-link')
.clearfix
3 changes: 0 additions & 3 deletions app/views/accounts/actions/_add_webauthn.html.slim

This file was deleted.

5 changes: 1 addition & 4 deletions app/views/accounts/show.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,7 @@ h1.hide = t('titles.account')
action: @view_model.totp_partial

- if FeatureManagement.webauthn_enabled?
= render 'account_item',
name: t('account.index.webauthn'),
content: content_tag(:em, @view_model.totp_content),
action: @view_model.webauthn_partial
= render 'webauthn'

- if current_user.piv_cac_available?
= render 'account_item',
Expand Down
2 changes: 1 addition & 1 deletion app/views/users/webauthn_setup/new.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ ul.list-reset
= hidden_field_tag :webauthn_public_key, '', id: 'webauthn_public_key'
= hidden_field_tag :attestation_object, '', id: 'attestation_object'
= hidden_field_tag :client_data_json, '', id: 'client_data_json'
= text_field_tag :name, '', required: true, pattern: '[A-Za-z0-9]*', id: 'name',
= text_field_tag :name, '', required: true, id: 'name',
class: 'block col-12 field monospace', size: 16, maxlength: 20,
'aria-labelledby': 'totp-label'
.col.col-6.sm-col-5.px1
Expand Down
2 changes: 2 additions & 0 deletions config/locales/account/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ en:
reactivate_button: Enter the code you received via US mail
success: Your account has been verified.
webauthn: Hardware security key
webauthn_add: "+ Add hardware security key"
webauthn_delete: Remove
items:
delete_your_account: Delete your account
personal_key: Personal key
Expand Down
2 changes: 2 additions & 0 deletions config/locales/account/es.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ es:
reactivate_button: Ingrese el código que recibió por correo postal.
success: Su cuenta ha sido verificada.
webauthn: Clave de seguridad de hardware
webauthn_add: "+ Agregar clave de seguridad de hardware"
webauthn_delete: Retirar
items:
delete_your_account: Eliminar su cuenta
personal_key: Clave personal
Expand Down
2 changes: 2 additions & 0 deletions config/locales/account/fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ fr:
reactivate_button: Entrez le code que vous avez reçu par la poste
success: Votre compte a été vérifié.
webauthn: Clé de sécurité matérielle
webauthn_add: "+ Ajouter une clé de sécurité matérielle"
webauthn_delete: Retirer
items:
delete_your_account: Supprimer votre compte
personal_key: Clé personnelle
Expand Down
1 change: 1 addition & 0 deletions config/locales/errors/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ en:
letter for a new code.
weak_password: Your password is not strong enough. %{feedback}
webauthn_setup:
delete_last: Sorry, you can not remove your last MFA option.
general_error: There was an error adding your hardward security key. Please
try again.
unique_name: That name is already taken. Please choose a different name.
1 change: 1 addition & 0 deletions config/locales/errors/es.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ es:
usps_otp_expired: NOT TRANSLATED YET
weak_password: Su contraseña no es suficientemente segura. %{feedback}
webauthn_setup:
delete_last: Lo sentimos, no puedes eliminar tu última opción de MFA.
general_error: Hubo un error al agregar su clave de seguridad de hardware. Inténtalo
de nuevo.
unique_name: El nombre ya fue escogido. Por favor, elija un nombre diferente.
1 change: 1 addition & 0 deletions config/locales/errors/fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ fr:
usps_otp_expired: NOT TRANSLATED YET
weak_password: Votre mot de passe n'est pas assez fort. %{feedback}
webauthn_setup:
delete_last: Désolé, vous ne pouvez pas supprimer votre dernière option MFA
general_error: Une erreur s'est produite lors de l'ajout de votre clé de sécurité
matérielle. Veuillez réessayer.
unique_name: Ce nom est déjà pris. Veuillez choisir un autre nom.
1 change: 1 addition & 0 deletions config/locales/notices/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,6 @@ en:
link: use a different email address
text_html: Or, %{link}
webauthn_added: You added a hardware security key.
webauthn_deleted: You deleted a hardware security key.
session_timedout: We signed you out. For your security, %{app} ends your session
when you haven’t moved to a new page for %{minutes} minutes.
1 change: 1 addition & 0 deletions config/locales/notices/es.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,6 @@ es:
link: use un email diferente
text_html: O %{link}
webauthn_added: Agregaste una clave de seguridad de hardware.
webauthn_deleted: Has eliminado una clave de seguridad de hardware.
session_timedout: Hemos terminado su sesión. Para su seguridad, %{app} cierra su
sesión cuando usted no pasa a una nueva página durante %{minutes} minutos.
1 change: 1 addition & 0 deletions config/locales/notices/fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ fr:
link: utilisez une adresse courriel différente
text_html: Or, %{link}
webauthn_added: Vous avez ajouté une clé de sécurité matérielle.
webauthn_deleted: Vous avez supprimé une clé de sécurité matérielle.
session_timedout: Nous vous avons déconnecté. Pour votre sécurité, %{app} désactive
votre session lorsque vous demeurez sur une page sans vous déplacer pendant %{minutes}
minutes.
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@
if FeatureManagement.webauthn_enabled?
get '/webauthn_setup' => 'users/webauthn_setup#new', as: :webauthn_setup
patch '/webauthn_setup' => 'users/webauthn_setup#confirm'
delete '/webauthn_setup' => 'users/webauthn_setup#delete'
end

delete '/authenticator_setup' => 'users/totp_setup#disable', as: :disable_totp
Expand Down
31 changes: 31 additions & 0 deletions spec/controllers/users/webauthn_setup_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,5 +82,36 @@
patch :confirm, params: params
end
end

describe 'delete' do
before do
allow(controller.current_user).to receive(:total_mfa_options_enabled).and_return(2)
end

it 'deletes a webauthn configuration' do
cfg = create_webauthn_configuration(controller.current_user, 'key1', 'id1', 'foo1')
delete :delete, params: { id: cfg.id }

expect(response).to redirect_to(account_url)
expect(flash.now[:success]).to eq t('notices.webauthn_deleted')
expect(WebauthnConfiguration.count).to eq(0)
end

it 'tracks the delete' do
cfg = create_webauthn_configuration(controller.current_user, 'key1', 'id1', 'foo1')

result = { success: true, mfa_options_enabled: 2 }
expect(@analytics).to receive(:track_event).with(Analytics::WEBAUTHN_DELETED, result)

delete :delete, params: { id: cfg.id }
end
end
end

def create_webauthn_configuration(user, name, id, key)
WebauthnConfiguration.create(user_id: user.id,
credential_public_key: key,
credential_id: id,
name: name)
end
end
70 changes: 63 additions & 7 deletions spec/features/users/webauthn_management_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,16 @@
feature 'Webauthn Management' do
include WebauthnHelper

context 'with no webauthn associated yet' do
let(:user) { create(:user, :signed_up, with: { phone: '+1 202-555-1212' }) }
let(:user) { create(:user, :signed_up, with: { phone: '+1 202-555-1212' }) }

context 'with no webauthn associated yet' do
it 'allows user to add a webauthn configuration' do
mock_challenge
sign_in_and_2fa_user(user)
visit account_path
expect(current_path).to eq account_path

click_link t('forms.buttons.enable'), href: webauthn_setup_url
click_link t('account.index.webauthn_add'), href: webauthn_setup_url
expect(current_path).to eq webauthn_setup_path

mock_press_button_on_hardware_key
Expand All @@ -27,7 +27,7 @@
visit account_path
expect(current_path).to eq account_path

click_link t('forms.buttons.enable'), href: webauthn_setup_url
click_link t('account.index.webauthn_add'), href: webauthn_setup_url
expect(current_path).to eq webauthn_setup_path

mock_press_button_on_hardware_key
Expand All @@ -43,7 +43,7 @@
visit account_path
expect(current_path).to eq account_path

click_link t('forms.buttons.enable'), href: webauthn_setup_url
click_link t('account.index.webauthn_add'), href: webauthn_setup_url
expect(current_path).to eq webauthn_setup_path

click_submit_default
Expand All @@ -59,7 +59,7 @@
visit account_path
expect(current_path).to eq account_path

click_link t('forms.buttons.enable'), href: webauthn_setup_url
click_link t('account.index.webauthn_add'), href: webauthn_setup_url
expect(current_path).to eq webauthn_setup_path

mock_press_button_on_hardware_key
Expand All @@ -68,7 +68,7 @@
expect(current_path).to eq account_path
expect(page).to have_content t('notices.webauthn_added')

click_link t('forms.buttons.enable'), href: webauthn_setup_url
click_link t('account.index.webauthn_add'), href: webauthn_setup_url
expect(current_path).to eq webauthn_setup_path

mock_press_button_on_hardware_key
Expand All @@ -77,6 +77,55 @@
expect(current_path).to eq webauthn_setup_path
expect(page).to have_content t('errors.webauthn_setup.unique_name')
end

it 'displays a link to add a hardware security key' do
sign_in_and_2fa_user(user)

visit account_path
expect(page).to have_link(t('account.index.webauthn_add'), href: webauthn_setup_url)
end
end

context 'with webauthn associations' do
it 'displays the user supplied names of the webauthn keys' do
create_webauthn_configuration(user, 'key1', '1', 'foo1')
create_webauthn_configuration(user, 'key2', '2', 'bar2')

sign_in_and_2fa_user(user)
visit account_path

expect(page).to have_content 'key1'
expect(page).to have_content 'key2'
end

it 'allows the user to delete the webauthn key' do
create_webauthn_configuration(user, 'key1', '1', 'foo1')

sign_in_and_2fa_user(user)
visit account_path

expect(page).to have_content 'key1'

click_button t('account.index.webauthn_delete')

expect(page).to_not have_content 'key1'
expect(page).to have_content t('notices.webauthn_deleted')
end

it 'prevents a user from deleting the last key' do
create_webauthn_configuration(user, 'key1', '1', 'foo1')

sign_in_and_2fa_user(user)
PhoneConfiguration.first.update(mfa_enabled: false)
visit account_path

expect(page).to have_content 'key1'

click_button t('account.index.webauthn_delete')

expect(page).to have_content 'key1'
expect(page).to have_content t('errors.webauthn_setup.delete_last')
end
end

def mock_challenge
Expand All @@ -97,4 +146,11 @@ def mock_press_button_on_hardware_key
def set_hidden_field(id, value)
first("input##{id}", visible: false).set(value)
end

def create_webauthn_configuration(user, name, id, key)
WebauthnConfiguration.create(user_id: user.id,
credential_public_key: key,
credential_id: id,
name: name)
end
end