From d8d36eb1fae673b5eade839cec4d89cc9c85dedd Mon Sep 17 00:00:00 2001 From: Jack Cody Date: Wed, 27 Dec 2023 16:52:48 -0800 Subject: [PATCH 1/6] Add WIP --- .../piv_cac_controller.rb | 58 ++++++++++++++++ .../piv_cac_delete_form.rb | 57 ++++++++++++++++ .../piv_cac_update_form.rb | 68 +++++++++++++++++++ app/views/accounts/_piv_cac.html.erb | 27 ++++---- config/routes.rb | 2 + 5 files changed, 199 insertions(+), 13 deletions(-) create mode 100644 app/controllers/api/internal/two_factor_authentication/piv_cac_controller.rb create mode 100644 app/forms/two_factor_authentication/piv_cac_delete_form.rb create mode 100644 app/forms/two_factor_authentication/piv_cac_update_form.rb diff --git a/app/controllers/api/internal/two_factor_authentication/piv_cac_controller.rb b/app/controllers/api/internal/two_factor_authentication/piv_cac_controller.rb new file mode 100644 index 00000000000..a43a4a92c4e --- /dev/null +++ b/app/controllers/api/internal/two_factor_authentication/piv_cac_controller.rb @@ -0,0 +1,58 @@ +module Api + module Internal + module TwoFactorAuthentication + class PivCacController < ApplicationController + include CsrfTokenConcern + include ReauthenticationRequiredConcern + + before_action :render_unauthorized, unless: :recently_authenticated_2fa? + + after_action :add_csrf_token_header_to_response + + respond_to :json + + def update + result = ::TwoFactorAuthentication::PivCacUpdateForm.new( + user: current_user, + configuration_id: params[:id], + ).submit(name: params[:name]) + + # TODO: Change the name of the following + # analytics.webauthn_update_name_submitted(**result.to_h) + + if result.success? + render json: { success: true } + else + render json: { success: false, error: result.first_error_message }, status: :bad_request + end + end + + def destroy + result = ::TwoFactorAuthentication::PivCacDeleteForm.new( + user: current_user, + configuration_id: params[:id], + ).submit + + # TODO: Change the name of the following + # analytics.webauthn_delete_submitted(**result.to_h) + + if result.success? + create_user_event(:piv_cac_disabled) + revoke_remember_device(current_user) + event = PushNotification::RecoveryInformationChangedEvent.new(user: current_user) + PushNotification::HttpPush.deliver(event) + render json: { success: true } + else + render json: { success: false, error: result.first_error_message }, status: :bad_request + end + end + + private + + def render_unauthorized + render json: { error: 'Unauthorized' }, status: :unauthorized + end + end + end + end +end diff --git a/app/forms/two_factor_authentication/piv_cac_delete_form.rb b/app/forms/two_factor_authentication/piv_cac_delete_form.rb new file mode 100644 index 00000000000..18e519b8e03 --- /dev/null +++ b/app/forms/two_factor_authentication/piv_cac_delete_form.rb @@ -0,0 +1,57 @@ +module TwoFactorAuthentication + class PivCacDeleteForm + include ActiveModel::Model + include ActionView::Helpers::TranslationHelper + + attr_reader :user, :configuration_id + + validate :validate_configuration_exists + validate :validate_has_multiple_mfa + + def initialize(user:, configuration_id:) + @user = user + @configuration_id = configuration_id + end + + def submit + success = valid? + + configuration.destroy if success + + FormResponse.new( + success:, + errors:, + extra: extra_analytics_attributes, + serialize_error_details_only: true, + ) + end + + def configuration + @configuration ||= user.piv_cac_configurations.find_by(id: configuration_id) + end + + private + + def validate_configuration_exists + return if configuration.present? + errors.add( + :configuration_id, + :configuration_not_found, + message: t('errors.manage_authenticator.internal_error'), + ) + end + + def validate_has_multiple_mfa + return if !configuration || MfaPolicy.new(user).multiple_factors_enabled? + errors.add( + :configuration_id, + :only_method, + message: t('errors.manage_authenticator.remove_only_method_error'), + ) + end + + def extra_analytics_attributes + { configuration_id: } + end + end +end diff --git a/app/forms/two_factor_authentication/piv_cac_update_form.rb b/app/forms/two_factor_authentication/piv_cac_update_form.rb new file mode 100644 index 00000000000..3df080833ea --- /dev/null +++ b/app/forms/two_factor_authentication/piv_cac_update_form.rb @@ -0,0 +1,68 @@ +module TwoFactorAuthentication + class PivCacUpdateForm + include ActiveModel::Model + include ActionView::Helpers::TranslationHelper + + attr_reader :user, :configuration_id + + validate :validate_configuration_exists + validate :validate_unique_name + + def initialize(user:, configuration_id:) + @user = user + @configuration_id = configuration_id + end + + def submit(name:) + @name = name + + success = valid? + if valid? + configuration.name = name + success = configuration.valid? + errors.merge!(configuration.errors) + configuration.save if success + end + + FormResponse.new( + success:, + errors:, + extra: extra_analytics_attributes, + serialize_error_details_only: true, + ) + end + + def name + return @name if defined?(@name) + @name = configuration&.name + end + + def configuration + @configuration ||= user.piv_cac_configurations.find_by(id: configuration_id) + end + + private + + def validate_configuration_exists + return if configuration.present? + errors.add( + :configuration_id, + :configuration_not_found, + message: t('errors.manage_authenticator.internal_error'), + ) + end + + def validate_unique_name + return unless user.piv_cac_configurations.where.not(id: configuration_id).find_by(name:) + errors.add( + :name, + :duplicate, + message: t('errors.manage_authenticator.unique_name_error'), + ) + end + + def extra_analytics_attributes + { configuration_id: } + end + end +end diff --git a/app/views/accounts/_piv_cac.html.erb b/app/views/accounts/_piv_cac.html.erb index 0008e84835d..f7d60aa8bb2 100644 --- a/app/views/accounts/_piv_cac.html.erb +++ b/app/views/accounts/_piv_cac.html.erb @@ -3,19 +3,20 @@
- <% MfaContext.new(current_user).piv_cac_configurations.each do |piv_cac_configuration| %> -
-
-
- <%= piv_cac_configuration.name %> -
-
- <% if MfaPolicy.new(current_user).multiple_factors_enabled? %> -
- <%= render 'accounts/actions/disable_piv_cac', id: piv_cac_configuration.id %> -
- <% end %> -
+ <% MfaContext.new(current_user).piv_cac_configurations.each do |configuration| %> + <%= render ManageableAuthenticatorComponent.new( + configuration:, + user_session:, + manage_url: edit_webauthn_path(id: configuration.id), + manage_api_url: api_internal_two_factor_authentication_piv_cac_path(id: configuration.id), + custom_strings: { + # TODO: Change + deleted: 'deleted', # Change + renamed: 'renamed', # Change + manage_accessible_label: 'some_label', # Change + }, + role: 'list-item', + ) %> <% end %>
diff --git a/config/routes.rb b/config/routes.rb index e806fcbeaa0..055202f2c14 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -21,6 +21,8 @@ put '/sessions' => 'sessions#update' namespace :two_factor_authentication do + put '/piv_cac/:id' => 'piv_cac#update', as: :piv_cac + delete '/piv_cac/:id' => 'piv_cac#destroy', as: nil put '/webauthn/:id' => 'webauthn#update', as: :webauthn delete '/webauthn/:id' => 'webauthn#destroy', as: nil put '/auth_app/:id' => 'auth_app#update', as: :auth_app From 1c0f3ed5f682e8ad067cd96eceb6ce2ad69ecb5a Mon Sep 17 00:00:00 2001 From: Jack Cody Date: Thu, 4 Jan 2024 16:27:08 -0600 Subject: [PATCH 2/6] Add WIP --- .../piv_cac_controller.rb | 16 +++-- app/controllers/users/piv_cac_controller.rb | 69 +++++++++++++++++++ .../piv_cac_delete_form.rb | 2 +- .../piv_cac_update_form.rb | 2 +- .../piv_cac_edit_presenter.rb | 37 ++++++++++ app/views/accounts/_piv_cac.html.erb | 14 ++-- app/views/users/piv_cac/edit.html.erb | 39 +++++++++++ .../locales/two_factor_authentication/en.yml | 5 ++ config/routes.rb | 5 +- 9 files changed, 173 insertions(+), 16 deletions(-) create mode 100644 app/controllers/users/piv_cac_controller.rb create mode 100644 app/presenters/two_factor_authentication/piv_cac_edit_presenter.rb create mode 100644 app/views/users/piv_cac/edit.html.erb diff --git a/app/controllers/api/internal/two_factor_authentication/piv_cac_controller.rb b/app/controllers/api/internal/two_factor_authentication/piv_cac_controller.rb index a43a4a92c4e..9545fe46819 100644 --- a/app/controllers/api/internal/two_factor_authentication/piv_cac_controller.rb +++ b/app/controllers/api/internal/two_factor_authentication/piv_cac_controller.rb @@ -17,8 +17,8 @@ def update configuration_id: params[:id], ).submit(name: params[:name]) - # TODO: Change the name of the following - # analytics.webauthn_update_name_submitted(**result.to_h) + # TODO: Update the following + # analytics.piv_cac_update_name_submitted(**result.to_h) if result.success? render json: { success: true } @@ -33,14 +33,13 @@ def destroy configuration_id: params[:id], ).submit - # TODO: Change the name of the following - # analytics.webauthn_delete_submitted(**result.to_h) + # TODO: Update the following + # analytics.piv_cac_delete_submitted(**result.to_h) if result.success? create_user_event(:piv_cac_disabled) revoke_remember_device(current_user) - event = PushNotification::RecoveryInformationChangedEvent.new(user: current_user) - PushNotification::HttpPush.deliver(event) + deliver_push_notification render json: { success: true } else render json: { success: false, error: result.first_error_message }, status: :bad_request @@ -49,6 +48,11 @@ def destroy private + def deliver_push_notification + event = PushNotification::RecoveryInformationChangedEvent.new(user: current_user) + PushNotification::HttpPush.deliver(event) + end + def render_unauthorized render json: { error: 'Unauthorized' }, status: :unauthorized end diff --git a/app/controllers/users/piv_cac_controller.rb b/app/controllers/users/piv_cac_controller.rb new file mode 100644 index 00000000000..22ddd12946c --- /dev/null +++ b/app/controllers/users/piv_cac_controller.rb @@ -0,0 +1,69 @@ +module Users + class PivCacController < ApplicationController + include ReauthenticationRequiredConcern + + before_action :confirm_two_factor_authenticated + before_action :confirm_recently_authenticated_2fa + before_action :set_form + before_action :validate_configuration_exists + before_action :set_presenter + + def edit; end + + def update + result = form.submit(name: params.dig(:form, :name)) + + # TODO: Verify analytics + # analytics.piv_cac_update_name_submitted(**result.to_h) + + if result.success? + flash[:success] = presenter.rename_success_alert_text + redirect_to account_path + else + flash.now[:error] = result.first_error_message + render :edit + end + end + + def destroy + result = form.submit + + # TODO: Verify analytics + # analytics.piv_cac_delete_submitted(**result.to_h) + + if result.success? + flash[:success] = presenter.delete_success_alert_text + redirect_to account_path + else + flash[:error] = result.first_error_message + redirect_to edit_piv_cac_path(id: params[:id]) + end + end + + private + + alias_method :set_form, :form + alias_method :set_presenter, :presenter + + def form + @form ||= form_class.new(user: current_user, configuration_id: params[:id]) + end + + def presenter + @presenter ||= TwoFactorAuthentication::PivCacEditPresenter.new + end + + def form_class + case action_name + when 'edit', 'update' + TwoFactorAuthentication::PivCacUpdateForm + when 'destroy' + TwoFactorAuthentication::PivCacDeleteForm + end + end + + def validate_configuration_exists + render_not_found if configuration.blank? + end + end +end diff --git a/app/forms/two_factor_authentication/piv_cac_delete_form.rb b/app/forms/two_factor_authentication/piv_cac_delete_form.rb index 18e519b8e03..5b4a9da898f 100644 --- a/app/forms/two_factor_authentication/piv_cac_delete_form.rb +++ b/app/forms/two_factor_authentication/piv_cac_delete_form.rb @@ -51,7 +51,7 @@ def validate_has_multiple_mfa end def extra_analytics_attributes - { configuration_id: } + { configuration_id: configuration_id } end end end diff --git a/app/forms/two_factor_authentication/piv_cac_update_form.rb b/app/forms/two_factor_authentication/piv_cac_update_form.rb index 3df080833ea..8c16615ad64 100644 --- a/app/forms/two_factor_authentication/piv_cac_update_form.rb +++ b/app/forms/two_factor_authentication/piv_cac_update_form.rb @@ -62,7 +62,7 @@ def validate_unique_name end def extra_analytics_attributes - { configuration_id: } + { configuration_id: configuration_id } end end end diff --git a/app/presenters/two_factor_authentication/piv_cac_edit_presenter.rb b/app/presenters/two_factor_authentication/piv_cac_edit_presenter.rb new file mode 100644 index 00000000000..9a48f5c8901 --- /dev/null +++ b/app/presenters/two_factor_authentication/piv_cac_edit_presenter.rb @@ -0,0 +1,37 @@ +module TwoFactorAuthentication + class PivCacEditPresenter + include ActionView::Helpers::TranslationHelper + + def initialize; end + + def heading + t('two_factor_authentication.piv_cac.edit_heading') + end + + def nickname_field_label + # missing translation key + # t('two_factor_authentication.piv_cac.nickname') + 'Nickname' + end + + def rename_button_label + # missing translation key + # t('two_factor_authentication.piv_cac.change_nickname') + 'Rename' + end + + def delete_button_label + # missing translation key + # t('two_factor_authentication.piv_cac.delete') + 'Delete' + end + + def rename_success_alert_text + t('two_factor_authentication.piv_cac.renamed') + end + + def delete_success_alert_text + t('two_factor_authentication.piv_cac.deleted') + end + end +end diff --git a/app/views/accounts/_piv_cac.html.erb b/app/views/accounts/_piv_cac.html.erb index f7d60aa8bb2..820ab08457e 100644 --- a/app/views/accounts/_piv_cac.html.erb +++ b/app/views/accounts/_piv_cac.html.erb @@ -2,18 +2,17 @@ <%= t('headings.account.federal_employee_id') %> -
+
<% MfaContext.new(current_user).piv_cac_configurations.each do |configuration| %> <%= render ManageableAuthenticatorComponent.new( configuration:, user_session:, - manage_url: edit_webauthn_path(id: configuration.id), + manage_url: edit_piv_cac_path(id: configuration.id), manage_api_url: api_internal_two_factor_authentication_piv_cac_path(id: configuration.id), custom_strings: { - # TODO: Change - deleted: 'deleted', # Change - renamed: 'renamed', # Change - manage_accessible_label: 'some_label', # Change + deleted: t('two_factor_authentication.piv_cac.deleted'), + renamed: t('two_factor_authentication.piv_cac.renamed'), + manage_accessible_label: t('two_factor_authentication.piv_cac.manage_accessible_label'), }, role: 'list-item', ) %> @@ -26,6 +25,7 @@ link_to(setup_piv_cac_url, **tag_options, &block) end, icon: :add, - class: 'usa-button usa-button--outline margin-top-2', + outline: true, + class: 'margin-top-2', ).with_content(t('account.index.piv_cac_add')) %> <% end %> diff --git a/app/views/users/piv_cac/edit.html.erb b/app/views/users/piv_cac/edit.html.erb new file mode 100644 index 00000000000..7995216d56c --- /dev/null +++ b/app/views/users/piv_cac/edit.html.erb @@ -0,0 +1,39 @@ +<% self.title = @presenter.heading %> + +<%= render PageHeadingComponent.new.with_content(@presenter.heading) %> + +<%= simple_form_for( + @form, + as: :form, + method: :put, + html: { autocomplete: 'off' }, + url: piv_cac_path(id: @form.configuration.id), + ) do |f| %> + <%= render ValidatedFieldComponent.new( + form: f, + name: :name, + label: @presenter.nickname_field_label, + ) %> + <%= f.submit( + @presenter.rename_button_label, + class: 'display-block margin-top-5', + ) %> +<% end %> + +<%= render ButtonComponent.new( + action: ->(**tag_options, &block) do + button_to( + piv_cac_path(id: @form.configuration.id), + form: { aria: { label: @presenter.delete_button_label } }, + **tag_options, + &block + ) + end, + method: :delete, + big: true, + wide: true, + danger: true, + class: 'display-block margin-top-2', + ).with_content(@presenter.delete_button_label) %> + +<%= render 'shared/cancel', link: account_path %> \ No newline at end of file diff --git a/config/locales/two_factor_authentication/en.yml b/config/locales/two_factor_authentication/en.yml index e55ae0021a5..7288b58d16e 100644 --- a/config/locales/two_factor_authentication/en.yml +++ b/config/locales/two_factor_authentication/en.yml @@ -140,6 +140,11 @@ en: troubleshooting: change_number: Use another phone number code_not_received: I didn’t receive my one-time code + piv_cac: + deleted: Successfully deleted a PIV/CAC method + edit_heading: Manage your PIV/CAC settings + manage_accessible_label: Manage PIV/CAC + renamed: Successfully renamed your PIV/CAC method piv_cac_header_text: Present your PIV/CAC please_try_again_html: Please try again in %{countdown}. read_about_two_factor_authentication: Read about two-factor authentication diff --git a/config/routes.rb b/config/routes.rb index 055202f2c14..ed8509775e4 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -22,7 +22,7 @@ namespace :two_factor_authentication do put '/piv_cac/:id' => 'piv_cac#update', as: :piv_cac - delete '/piv_cac/:id' => 'piv_cac#destroy', as: nil + delete '/piv_cac/:id' => 'piv_cac#destroy', as: nil put '/webauthn/:id' => 'webauthn#update', as: :webauthn delete '/webauthn/:id' => 'webauthn#destroy', as: nil put '/auth_app/:id' => 'auth_app#update', as: :auth_app @@ -263,6 +263,9 @@ delete '/manage/phone/:id' => 'users/edit_phone#destroy' get '/manage/personal_key' => 'users/personal_keys#show', as: :manage_personal_key post '/manage/personal_key' => 'users/personal_keys#update' + get '/manage/piv_cac/:id' => 'users/piv_cac#edit', as: :edit_piv_cac + put '/manage/piv_cac/:id' => 'users/piv_cac#update', as: :piv_cac + delete '/manage/piv_cac/:id' => 'users/piv_cac#destroy', as: nil get '/manage/webauthn/:id' => 'users/webauthn#edit', as: :edit_webauthn put '/manage/webauthn/:id' => 'users/webauthn#update', as: :webauthn delete '/manage/webauthn/:id' => 'users/webauthn#destroy', as: nil From d18ed7785c85fe25925ed846954fc8f64e528784 Mon Sep 17 00:00:00 2001 From: Jack Cody Date: Fri, 5 Jan 2024 07:35:01 -0600 Subject: [PATCH 3/6] Add tests --- .../piv_cac_controller.rb | 6 - app/controllers/users/piv_cac_controller.rb | 14 +- .../piv_cac_update_form.rb | 2 + .../piv_cac_edit_presenter.rb | 12 +- .../locales/two_factor_authentication/en.yml | 3 + .../locales/two_factor_authentication/es.yml | 8 + .../locales/two_factor_authentication/fr.yml | 8 + .../users/piv_cac_controller_spec.rb | 182 ++++++++++++++++++ .../piv_cac_delete_form_spec.rb | 91 +++++++++ .../piv_cac_update_form_spec.rb | 142 ++++++++++++++ .../piv_cac_edit_presenter_spec.rb | 44 +++++ spec/views/accounts/_piv_cac.html.erb_spec.rb | 29 +++ 12 files changed, 516 insertions(+), 25 deletions(-) create mode 100644 spec/controllers/users/piv_cac_controller_spec.rb create mode 100644 spec/forms/two_factor_authentication/piv_cac_delete_form_spec.rb create mode 100644 spec/forms/two_factor_authentication/piv_cac_update_form_spec.rb create mode 100644 spec/presenters/two_factor_authentication/piv_cac_edit_presenter_spec.rb create mode 100644 spec/views/accounts/_piv_cac.html.erb_spec.rb diff --git a/app/controllers/api/internal/two_factor_authentication/piv_cac_controller.rb b/app/controllers/api/internal/two_factor_authentication/piv_cac_controller.rb index 9545fe46819..0f8f192ce5b 100644 --- a/app/controllers/api/internal/two_factor_authentication/piv_cac_controller.rb +++ b/app/controllers/api/internal/two_factor_authentication/piv_cac_controller.rb @@ -17,9 +17,6 @@ def update configuration_id: params[:id], ).submit(name: params[:name]) - # TODO: Update the following - # analytics.piv_cac_update_name_submitted(**result.to_h) - if result.success? render json: { success: true } else @@ -33,9 +30,6 @@ def destroy configuration_id: params[:id], ).submit - # TODO: Update the following - # analytics.piv_cac_delete_submitted(**result.to_h) - if result.success? create_user_event(:piv_cac_disabled) revoke_remember_device(current_user) diff --git a/app/controllers/users/piv_cac_controller.rb b/app/controllers/users/piv_cac_controller.rb index 22ddd12946c..c339f5bb781 100644 --- a/app/controllers/users/piv_cac_controller.rb +++ b/app/controllers/users/piv_cac_controller.rb @@ -13,9 +13,6 @@ def edit; end def update result = form.submit(name: params.dig(:form, :name)) - # TODO: Verify analytics - # analytics.piv_cac_update_name_submitted(**result.to_h) - if result.success? flash[:success] = presenter.rename_success_alert_text redirect_to account_path @@ -28,9 +25,6 @@ def update def destroy result = form.submit - # TODO: Verify analytics - # analytics.piv_cac_delete_submitted(**result.to_h) - if result.success? flash[:success] = presenter.delete_success_alert_text redirect_to account_path @@ -42,9 +36,6 @@ def destroy private - alias_method :set_form, :form - alias_method :set_presenter, :presenter - def form @form ||= form_class.new(user: current_user, configuration_id: params[:id]) end @@ -53,6 +44,9 @@ def presenter @presenter ||= TwoFactorAuthentication::PivCacEditPresenter.new end + alias_method :set_form, :form + alias_method :set_presenter, :presenter + def form_class case action_name when 'edit', 'update' @@ -63,7 +57,7 @@ def form_class end def validate_configuration_exists - render_not_found if configuration.blank? + render_not_found if form.configuration.blank? end end end diff --git a/app/forms/two_factor_authentication/piv_cac_update_form.rb b/app/forms/two_factor_authentication/piv_cac_update_form.rb index 8c16615ad64..2c8144f6072 100644 --- a/app/forms/two_factor_authentication/piv_cac_update_form.rb +++ b/app/forms/two_factor_authentication/piv_cac_update_form.rb @@ -45,6 +45,7 @@ def configuration def validate_configuration_exists return if configuration.present? + errors.add( :configuration_id, :configuration_not_found, @@ -54,6 +55,7 @@ def validate_configuration_exists def validate_unique_name return unless user.piv_cac_configurations.where.not(id: configuration_id).find_by(name:) + errors.add( :name, :duplicate, diff --git a/app/presenters/two_factor_authentication/piv_cac_edit_presenter.rb b/app/presenters/two_factor_authentication/piv_cac_edit_presenter.rb index 9a48f5c8901..d643ea4ab48 100644 --- a/app/presenters/two_factor_authentication/piv_cac_edit_presenter.rb +++ b/app/presenters/two_factor_authentication/piv_cac_edit_presenter.rb @@ -9,21 +9,15 @@ def heading end def nickname_field_label - # missing translation key - # t('two_factor_authentication.piv_cac.nickname') - 'Nickname' + t('two_factor_authentication.piv_cac.nickname') end def rename_button_label - # missing translation key - # t('two_factor_authentication.piv_cac.change_nickname') - 'Rename' + t('two_factor_authentication.piv_cac.change_nickname') end def delete_button_label - # missing translation key - # t('two_factor_authentication.piv_cac.delete') - 'Delete' + t('two_factor_authentication.piv_cac.delete') end def rename_success_alert_text diff --git a/config/locales/two_factor_authentication/en.yml b/config/locales/two_factor_authentication/en.yml index 7288b58d16e..ab5e7db2a51 100644 --- a/config/locales/two_factor_authentication/en.yml +++ b/config/locales/two_factor_authentication/en.yml @@ -141,9 +141,12 @@ en: change_number: Use another phone number code_not_received: I didn’t receive my one-time code piv_cac: + change_nickname: Change nickname + delete: Delete this method deleted: Successfully deleted a PIV/CAC method edit_heading: Manage your PIV/CAC settings manage_accessible_label: Manage PIV/CAC + nickname: Nickname renamed: Successfully renamed your PIV/CAC method piv_cac_header_text: Present your PIV/CAC please_try_again_html: Please try again in %{countdown}. diff --git a/config/locales/two_factor_authentication/es.yml b/config/locales/two_factor_authentication/es.yml index 5f094a6efae..393fc824356 100644 --- a/config/locales/two_factor_authentication/es.yml +++ b/config/locales/two_factor_authentication/es.yml @@ -147,6 +147,14 @@ es: troubleshooting: change_number: Utilice otro número de teléfono. code_not_received: No recibí mi código de un solo uso. + piv_cac: + change_nickname: TODO UPDATE TRANSLATION + delete: TODO UPDATE TRANSLATION + deleted: TODO UPDATE TRANSLATION + edit_heading: TODO UPDATE TRANSLATION + manage_accessible_label: TODO UPDATE TRANSLATION + nickname: TODO UPDATE TRANSLATION + renamed: TODO UPDATE TRANSLATION piv_cac_header_text: Presenta tu PIV/CAC please_try_again_html: Inténtelo de nuevo en %{countdown}. read_about_two_factor_authentication: Conozca la autenticación de dos factores diff --git a/config/locales/two_factor_authentication/fr.yml b/config/locales/two_factor_authentication/fr.yml index 01465df3f19..8e274c65f55 100644 --- a/config/locales/two_factor_authentication/fr.yml +++ b/config/locales/two_factor_authentication/fr.yml @@ -155,6 +155,14 @@ fr: troubleshooting: change_number: Utilisez un autre numéro de téléphone code_not_received: Je n’ai pas reçu mon code à usage unique + piv_cac: + change_nickname: TODO UPDATE TRANSLATION + delete: TODO UPDATE TRANSLATION + deleted: TODO UPDATE TRANSLATION + edit_heading: TODO UPDATE TRANSLATION + manage_accessible_label: TODO UPDATE TRANSLATION + nickname: TODO UPDATE TRANSLATION + renamed: TODO UPDATE TRANSLATION piv_cac_header_text: Veuillez présenter votre carte PIV/CAC please_try_again_html: Veuillez essayer de nouveau dans %{countdown}. read_about_two_factor_authentication: En savoir plus sur l’authentification à deux facteurs diff --git a/spec/controllers/users/piv_cac_controller_spec.rb b/spec/controllers/users/piv_cac_controller_spec.rb new file mode 100644 index 00000000000..c017d01da46 --- /dev/null +++ b/spec/controllers/users/piv_cac_controller_spec.rb @@ -0,0 +1,182 @@ +require 'rails_helper' + +RSpec.describe Users::PivCacController do + let(:user) { create(:user, :with_phone) } + let(:configuration) { create(:piv_cac_configuration, user: user) } + let(:presenter) { TwoFactorAuthentication::PivCacEditPresenter.new } + + before do + stub_analytics + stub_sign_in(user) if user + end + + describe '#edit' do + let(:params) { { id: configuration.id } } + let(:response) { get :edit, params: params } + + it 'assigns the form instance' do + response + + expect(assigns(:form)).to be_kind_of(TwoFactorAuthentication::PivCacUpdateForm) + expect(assigns(:form).configuration).to eq(configuration) + end + + context 'signed out' do + let(:user) { nil } + let(:configuration) { create(:webauthn_configuration) } + + it 'redirects to sign-in page' do + expect(response).to redirect_to(new_user_session_url) + end + end + + context 'not recently authenticated' do + before do + allow(controller).to receive(:recently_authenticated_2fa?).and_return(false) + end + + it 'redirects to reauthenticate' do + expect(response).to redirect_to(login_two_factor_options_path) + end + end + + context 'editing a configuration that does not exist' do + let(:params) { { id: 0 } } + + it 'renders not found' do + expect(response).to be_not_found + end + end + + context 'editing a configuration that does not belong to the user' do + let(:configuration) { create(:piv_cac_configuration) } + + it 'renders not found' do + expect(response).to be_not_found + end + end + end + + describe '#update' do + let(:name) { 'example' } + let(:params) { { id: configuration.id, form: { name: name } } } + let(:response) { put :update, params: params } + + it 'redirects to account page with success message' do + expect(response).to redirect_to(account_path) + expect(flash[:success]).to eq(presenter.rename_success_alert_text) + end + + it 'assigns the form instance' do + response + + expect(assigns(:form)).to be_kind_of(TwoFactorAuthentication::PivCacUpdateForm) + expect(assigns(:form).configuration).to eq(configuration) + end + + context 'signed out' do + let(:user) { nil } + let(:configuration) { create(:piv_cac_configuration) } + + it 'redirects to sign-in page' do + expect(response).to redirect_to(new_user_session_url) + end + end + + context 'with invalid submission' do + let(:name) { '' } + + it 'renders edit template with error' do + expect(response).to render_template(:edit) + expect(flash.now[:error]).to eq(t('errors.messages.blank')) + end + end + + context 'not recently authenticated' do + before do + allow(controller).to receive(:recently_authenticated_2fa?).and_return(false) + end + + it 'redirects to reauthenticate' do + expect(response).to redirect_to(login_two_factor_options_path) + end + end + + context 'with a configuration that does not exist' do + let(:params) { { id: 0 } } + + it 'renders not found' do + expect(response).to be_not_found + end + end + + context 'with a configuration that does not belong to the user' do + let(:configuration) { create(:piv_cac_configuration) } + + it 'renders not found' do + expect(response).to be_not_found + end + end + end + + describe '#destroy' do + let(:params) { { id: configuration.id } } + let(:response) { delete :destroy, params: params } + + it 'responds with successful result' do + expect(response).to redirect_to(account_path) + expect(flash[:success]).to eq(presenter.delete_success_alert_text) + end + + it 'assigns the form instance' do + response + + expect(assigns(:form)).to be_kind_of(TwoFactorAuthentication::PivCacDeleteForm) + expect(assigns(:form).configuration).to eq(configuration) + end + + context 'signed out' do + let(:user) { nil } + let(:configuration) { create(:piv_cac_configuration) } + + it 'redirects to sign-in page' do + expect(response).to redirect_to(new_user_session_url) + end + end + + context 'with invalid submission' do + let(:user) { create(:user) } + + it 'redirects to edit with unsuccessful result' do + expect(response).to redirect_to(edit_piv_cac_path(id: configuration.id)) + expect(flash[:error]).to eq(t('errors.manage_authenticator.remove_only_method_error')) + end + end + + context 'not recently authenticated' do + before do + allow(controller).to receive(:recently_authenticated_2fa?).and_return(false) + end + + it 'redirects to reauthenticate' do + expect(response).to redirect_to(login_two_factor_options_path) + end + end + + context 'with a configuration that does not exist' do + let(:params) { { id: 0 } } + + it 'renders not found' do + expect(response).to be_not_found + end + end + + context 'with a configuration that does not belong to the user' do + let(:configuration) { create(:piv_cac_configuration) } + + it 'renders not found' do + expect(response).to be_not_found + end + end + end +end diff --git a/spec/forms/two_factor_authentication/piv_cac_delete_form_spec.rb b/spec/forms/two_factor_authentication/piv_cac_delete_form_spec.rb new file mode 100644 index 00000000000..ef6e43b4fd4 --- /dev/null +++ b/spec/forms/two_factor_authentication/piv_cac_delete_form_spec.rb @@ -0,0 +1,91 @@ +require 'rails_helper' + +RSpec.describe TwoFactorAuthentication::PivCacDeleteForm do + let(:user) { create(:user) } + let(:configuration) { create(:piv_cac_configuration, user: user) } + let(:configuration_id) { configuration&.id } + let(:form) { described_class.new(user: user, configuration_id: configuration_id) } + + describe '#submit' do + let(:result) { form.submit } + + context 'when the user has another mfa enabled' do + before do + create(:phone_configuration, user: user) + end + + it 'returns a successful result' do + expect(result.success?).to eq(true) + expect(result.to_h).to eq(success: true, configuration_id: configuration_id) + end + + context 'with blank configuration' do + let(:configuration) { nil } + + it 'returns an unsuccessful result' do + expect(result.success?).to eq(false) + expect(result.to_h).to eq( + success: false, + error_details: { + configuration_id: { configuration_not_found: true }, + }, + configuration_id: configuration_id, + ) + end + end + + context 'with a non-existent configuration_id' do + let(:configuration_id) { 'does-not-exist' } + + it 'returns an unsuccessful result' do + expect(result.success?).to eq(false) + expect(result.to_h).to eq( + success: false, + error_details: { + configuration_id: { configuration_not_found: true }, + }, + configuration_id: configuration_id, + ) + end + end + + context 'with configuration not belonging to the user' do + let(:configuration) { create(:piv_cac_configuration) } + + it 'returns an unsuccessful result' do + expect(result.success?).to eq(false) + expect(result.to_h).to eq( + success: false, + error_details: { + configuration_id: { configuration_not_found: true }, + }, + configuration_id: configuration_id, + ) + end + end + end + + context 'when the user does not have another mfa enabled' do + let(:user) { create(:user) } + + it 'returns an unsuccessful result' do + expect(result.success?).to eq(false) + expect(result.to_h).to eq( + success: false, + error_details: { + configuration_id: { only_method: true }, + }, + configuration_id: configuration_id, + ) + end + end + end + + describe '#configuration' do + subject(:form_configuration) { form.configuration } + + it 'returns configuration' do + expect(form_configuration).to eq(configuration) + end + end +end diff --git a/spec/forms/two_factor_authentication/piv_cac_update_form_spec.rb b/spec/forms/two_factor_authentication/piv_cac_update_form_spec.rb new file mode 100644 index 00000000000..2d6eb087185 --- /dev/null +++ b/spec/forms/two_factor_authentication/piv_cac_update_form_spec.rb @@ -0,0 +1,142 @@ +require 'rails_helper' + +RSpec.describe TwoFactorAuthentication::PivCacUpdateForm do + let(:user) { create(:user) } + let(:original_name) { 'original-name' } + let(:configuration) { create(:piv_cac_configuration, user: user, name: original_name) } + let(:configuration_id) { configuration&.id } + let(:form) { described_class.new(user:, configuration_id:) } + + describe '#submit' do + let(:name) { 'new-name' } + let(:result) { form.submit(name: name) } + + it 'returns a successful result' do + expect(result.success?).to eq(true) + expect(result.to_h).to eq(success: true, configuration_id: configuration_id) + end + + it 'saves the new name' do + result + + expect(configuration.reload.name).to eq(name) + end + + context 'with blank configuration' do + let(:configuration) { nil } + + it 'returns an unsuccessful result' do + expect(result.success?).to eq(false) + expect(result.to_h).to eq( + success: false, + error_details: { + configuration_id: { configuration_not_found: true }, + }, + configuration_id: configuration_id, + ) + end + end + + context 'with configuration that does not exist' do + let(:configuration_id) { 'does-not-exist' } + + it 'returns an unsuccessful result' do + expect(result.success?).to eq(false) + expect(result.to_h).to eq( + success: false, + error_details: { + configuration_id: { configuration_not_found: true }, + }, + configuration_id: configuration_id, + ) + end + end + + context 'with configuration not belonging to the user' do + let(:configuration) { create(:piv_cac_configuration, name: original_name) } + + it 'returns an unsuccessful result' do + expect(result.success?).to eq(false) + expect(result.to_h).to eq( + success: false, + error_details: { + configuration_id: { configuration_not_found: true }, + }, + configuration_id: configuration_id, + ) + end + + it 'does not save the new name' do + expect(configuration).not_to receive(:save) + + result + + expect(configuration.reload.name).to eq(original_name) + end + end + + context 'with blank name' do + let(:name) { '' } + + it 'returns an unsuccessful result' do + expect(result.success?).to eq(false) + expect(result.to_h).to eq( + success: false, + error_details: { + name: { blank: true }, + }, + configuration_id:, + ) + end + + it 'does not save the new name' do + expect(configuration).not_to receive(:save) + + result + + expect(configuration.reload.name).to eq(original_name) + end + end + + context 'with duplicate name' do + before do + create(:piv_cac_configuration, user:, name:, x509_dn_uuid: 'unique-1') + end + + it 'returns an unsuccessful result' do + expect(result.success?).to eq(false) + expect(result.to_h).to eq( + success: false, + error_details: { + name: { duplicate: true }, + }, + configuration_id:, + ) + end + + it 'does not save the new name' do + expect(configuration).not_to receive(:save) + + result + + expect(configuration.reload.name).to eq(original_name) + end + end + end + + describe '#name' do + subject(:name) { form.name } + + it 'returns configuration name' do + expect(name).to eq(configuration.name) + end + end + + describe '#configuration' do + subject(:form_configuration) { form.configuration } + + it 'returns configuration' do + expect(form_configuration).to eq(configuration) + end + end +end diff --git a/spec/presenters/two_factor_authentication/piv_cac_edit_presenter_spec.rb b/spec/presenters/two_factor_authentication/piv_cac_edit_presenter_spec.rb new file mode 100644 index 00000000000..23aa801617c --- /dev/null +++ b/spec/presenters/two_factor_authentication/piv_cac_edit_presenter_spec.rb @@ -0,0 +1,44 @@ +require 'rails_helper' + +RSpec.describe TwoFactorAuthentication::PivCacEditPresenter do + subject(:presenter) { described_class.new } + + describe '#heading' do + it 'returns heading text' do + expect(presenter.heading).to eq(t('two_factor_authentication.piv_cac.edit_heading')) + end + end + + describe '#nickname_field_label' do + it 'returns nickname field label' do + expect(presenter.nickname_field_label).to eq(t('two_factor_authentication.piv_cac.nickname')) + end + end + + describe '#rename_button_label' do + it 'returns rename button label' do + expect(presenter.rename_button_label). + to eq(t('two_factor_authentication.piv_cac.change_nickname')) + end + end + + describe '#delete_button_label' do + it 'returns delete button label' do + expect(presenter.delete_button_label).to eq(t('two_factor_authentication.piv_cac.delete')) + end + end + + describe '#rename_success_alert_text' do + it 'returns rename success alert text' do + expect(presenter.rename_success_alert_text). + to eq(t('two_factor_authentication.piv_cac.renamed')) + end + end + + describe '#delete_success_alert_text' do + it 'returns delete success alert text' do + expect(presenter.delete_success_alert_text). + to eq(t('two_factor_authentication.piv_cac.deleted')) + end + end +end diff --git a/spec/views/accounts/_piv_cac.html.erb_spec.rb b/spec/views/accounts/_piv_cac.html.erb_spec.rb new file mode 100644 index 00000000000..3efaa9961aa --- /dev/null +++ b/spec/views/accounts/_piv_cac.html.erb_spec.rb @@ -0,0 +1,29 @@ +require 'rails_helper' + +RSpec.describe 'accounts/_piv_cac.html.erb' do + let(:user) do + user = create(:user) + 2.times do |n| + create( + :piv_cac_configuration, + user: user, + name: "Configuration #{n}", + x509_dn_uuid: "unique-uuid-#{n}", + ) + end + user + end + + let(:user_session) { { auth_events: [] } } + + subject(:rendered) { render partial: 'accounts/piv_cac' } + + before do + allow(view).to receive(:current_user).and_return(user) + allow(view).to receive(:user_session).and_return(user_session) + end + + it 'renders a list of piv cac configurations' do + expect(rendered).to have_selector('[role="list"] [role="list-item"]', count: 2) + end +end From 80e9c7fd22ce9c937a4139738cc50a90a49a25ce Mon Sep 17 00:00:00 2001 From: Jack Cody Date: Wed, 17 Jan 2024 16:00:43 -0600 Subject: [PATCH 4/6] Add translations --- config/locales/two_factor_authentication/es.yml | 14 +++++++------- config/locales/two_factor_authentication/fr.yml | 14 +++++++------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/config/locales/two_factor_authentication/es.yml b/config/locales/two_factor_authentication/es.yml index 393fc824356..9ef52718a16 100644 --- a/config/locales/two_factor_authentication/es.yml +++ b/config/locales/two_factor_authentication/es.yml @@ -148,13 +148,13 @@ es: change_number: Utilice otro número de teléfono. code_not_received: No recibí mi código de un solo uso. piv_cac: - change_nickname: TODO UPDATE TRANSLATION - delete: TODO UPDATE TRANSLATION - deleted: TODO UPDATE TRANSLATION - edit_heading: TODO UPDATE TRANSLATION - manage_accessible_label: TODO UPDATE TRANSLATION - nickname: TODO UPDATE TRANSLATION - renamed: TODO UPDATE TRANSLATION + change_nickname: Cambiar apodo + delete: Eliminar este método + deleted: Se ha eliminado correctamente un método PIV/CAC + edit_heading: Gestionar la configuración de PIV/CAC + manage_accessible_label: Gestionar PIV/CAC + nickname: Apodo + renamed: Se ha cambiado correctamente el nombre de su método PIV/CAC piv_cac_header_text: Presenta tu PIV/CAC please_try_again_html: Inténtelo de nuevo en %{countdown}. read_about_two_factor_authentication: Conozca la autenticación de dos factores diff --git a/config/locales/two_factor_authentication/fr.yml b/config/locales/two_factor_authentication/fr.yml index 8e274c65f55..6d7a86738f7 100644 --- a/config/locales/two_factor_authentication/fr.yml +++ b/config/locales/two_factor_authentication/fr.yml @@ -156,13 +156,13 @@ fr: change_number: Utilisez un autre numéro de téléphone code_not_received: Je n’ai pas reçu mon code à usage unique piv_cac: - change_nickname: TODO UPDATE TRANSLATION - delete: TODO UPDATE TRANSLATION - deleted: TODO UPDATE TRANSLATION - edit_heading: TODO UPDATE TRANSLATION - manage_accessible_label: TODO UPDATE TRANSLATION - nickname: TODO UPDATE TRANSLATION - renamed: TODO UPDATE TRANSLATION + change_nickname: Changer de pseudo + delete: Supprimer cette méthode + deleted: Suppression réussie d’une méthode PIV/CAC + edit_heading: Gérer les paramètres de PIV/CAC + manage_accessible_label: Gérer la carte PIV/CAC + nickname: Pseudo + renamed: Votre méthode PIV/CAC a été renommée avec succès piv_cac_header_text: Veuillez présenter votre carte PIV/CAC please_try_again_html: Veuillez essayer de nouveau dans %{countdown}. read_about_two_factor_authentication: En savoir plus sur l’authentification à deux facteurs From 2ea0366f7f3bc57004c84c5bfc2488f654e12d03 Mon Sep 17 00:00:00 2001 From: Jack Cody Date: Wed, 17 Jan 2024 16:01:38 -0600 Subject: [PATCH 5/6] Add analytics --- .../piv_cac_controller.rb | 4 ++ app/controllers/users/piv_cac_controller.rb | 13 +++++++ app/services/analytics_events.rb | 38 +++++++++++++++++++ 3 files changed, 55 insertions(+) diff --git a/app/controllers/api/internal/two_factor_authentication/piv_cac_controller.rb b/app/controllers/api/internal/two_factor_authentication/piv_cac_controller.rb index 0f8f192ce5b..4a08b0d74a5 100644 --- a/app/controllers/api/internal/two_factor_authentication/piv_cac_controller.rb +++ b/app/controllers/api/internal/two_factor_authentication/piv_cac_controller.rb @@ -17,6 +17,8 @@ def update configuration_id: params[:id], ).submit(name: params[:name]) + analytics.piv_cac_update_name_submitted(**result.to_h) + if result.success? render json: { success: true } else @@ -30,6 +32,8 @@ def destroy configuration_id: params[:id], ).submit + analytics.piv_cac_delete_submitted(**result.to_h) + if result.success? create_user_event(:piv_cac_disabled) revoke_remember_device(current_user) diff --git a/app/controllers/users/piv_cac_controller.rb b/app/controllers/users/piv_cac_controller.rb index c339f5bb781..a675022ee94 100644 --- a/app/controllers/users/piv_cac_controller.rb +++ b/app/controllers/users/piv_cac_controller.rb @@ -13,6 +13,8 @@ def edit; end def update result = form.submit(name: params.dig(:form, :name)) + analytics.piv_cac_update_name_submitted(**result.to_h) + if result.success? flash[:success] = presenter.rename_success_alert_text redirect_to account_path @@ -25,7 +27,13 @@ def update def destroy result = form.submit + analytics.piv_cac_delete_submitted(**result.to_h) + if result.success? + create_user_event(:piv_cac_disabled) + revoke_remember_device(current_user) + deliver_push_notification + flash[:success] = presenter.delete_success_alert_text redirect_to account_path else @@ -36,6 +44,11 @@ def destroy private + def deliver_push_notification + event = PushNotification::RecoveryInformationChangedEvent.new(user: current_user) + PushNotification::HttpPush.deliver(event) + end + def form @form ||= form_class.new(user: current_user, configuration_id: params[:id]) end diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index 0fbe31c784b..1f7b8b8c42f 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -3830,6 +3830,25 @@ def phone_input_country_changed(country_code:, **extra) track_event(:phone_input_country_changed, country_code:, **extra) end + # @param [Boolean] success + # @param [Hash] error_details + # @param [Integer] configuration_id + # Tracks when user attempts to delete a PIV/CAC configuraton + def piv_cac_delete_submitted( + success:, + configuration_id:, + error_details: nil, + **extra + ) + track_event( + :piv_cac_delete_submitted, + success:, + error_details:, + configuration_id:, + **extra, + ) + end + # @identity.idp.previous_event_name User Registration: piv cac disabled # @identity.idp.previous_event_name PIV CAC disabled # Tracks when user's piv cac is disabled @@ -3850,6 +3869,25 @@ def piv_cac_login(success:, errors:, **extra) ) end + # @param [Boolean] success + # @param [Hash] error_details + # @param [Integer] configuration_id + # Tracks when user submits a name change for a PIV/CAC configuraton + def piv_cac_update_name_submitted( + success:, + configuration_id:, + error_details: nil, + **extra + ) + track_event( + :piv_cac_update_name_submitted, + success:, + error_details:, + configuration_id:, + **extra, + ) + end + def piv_cac_login_visited track_event(:piv_cac_login_visited) end From 92ad152b932e8383cf6a0aef9d7ef4a920fdd150 Mon Sep 17 00:00:00 2001 From: Jack Cody Date: Wed, 17 Jan 2024 16:02:13 -0600 Subject: [PATCH 6/6] Add tests changelog: User-facing Improvements, MFA, Add inline device editing for PIV CAC --- app/services/analytics_events.rb | 32 +-- .../piv_cac_controller_spec.rb | 217 ++++++++++++++++++ .../users/piv_cac_controller_spec.rb | 22 ++ .../features/users/piv_cac_management_spec.rb | 22 +- spec/views/accounts/show.html.erb_spec.rb | 5 + .../views/users/piv_cac/edit.html.erb_spec.rb | 46 ++++ 6 files changed, 320 insertions(+), 24 deletions(-) create mode 100644 spec/controllers/api/internal/two_factor_authentication/piv_cac_controller_spec.rb create mode 100644 spec/views/users/piv_cac/edit.html.erb_spec.rb diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index 1f7b8b8c42f..cd08bdb0123 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -3869,6 +3869,22 @@ def piv_cac_login(success:, errors:, **extra) ) end + def piv_cac_login_visited + track_event(:piv_cac_login_visited) + end + + # @identity.idp.previous_event_name User Registration: piv cac setup visited + # @identity.idp.previous_event_name PIV CAC setup visited + # Tracks when user's piv cac setup + # @param [Boolean] in_account_creation_flow + def piv_cac_setup_visited(in_account_creation_flow:, **extra) + track_event( + :piv_cac_setup_visited, + in_account_creation_flow:, + **extra, + ) + end + # @param [Boolean] success # @param [Hash] error_details # @param [Integer] configuration_id @@ -3888,22 +3904,6 @@ def piv_cac_update_name_submitted( ) end - def piv_cac_login_visited - track_event(:piv_cac_login_visited) - end - - # @identity.idp.previous_event_name User Registration: piv cac setup visited - # @identity.idp.previous_event_name PIV CAC setup visited - # Tracks when user's piv cac setup - # @param [Boolean] in_account_creation_flow - def piv_cac_setup_visited(in_account_creation_flow:, **extra) - track_event( - :piv_cac_setup_visited, - in_account_creation_flow:, - **extra, - ) - end - # @param [String] redirect_url URL user was directed to # @param [String, nil] step which step # @param [String, nil] location which part of a step, if applicable diff --git a/spec/controllers/api/internal/two_factor_authentication/piv_cac_controller_spec.rb b/spec/controllers/api/internal/two_factor_authentication/piv_cac_controller_spec.rb new file mode 100644 index 00000000000..91c238740e4 --- /dev/null +++ b/spec/controllers/api/internal/two_factor_authentication/piv_cac_controller_spec.rb @@ -0,0 +1,217 @@ +require 'rails_helper' + +RSpec.describe Api::Internal::TwoFactorAuthentication::PivCacController do + let(:user) { create(:user, :with_phone) } + let(:configuration) { create(:piv_cac_configuration, user: user) } + + before do + stub_analytics + stub_sign_in(user) if user + end + + describe '#update' do + let(:name) { 'example' } + let(:params) { { id: configuration.id, name: } } + let(:response) { put :update, params: params } + subject(:response_body) { JSON.parse(response.body, symbolize_names: true) } + + it 'responds with successful result' do + expect(response_body).to eq(success: true) + expect(response.status).to eq(200) + end + + it 'logs the submission attempt' do + response + + expect(@analytics).to have_logged_event( + :piv_cac_update_name_submitted, + success: true, + configuration_id: configuration.id.to_s, + error_details: nil, + ) + end + + it 'includes csrf token in the response headers' do + expect(response.headers['X-CSRF-Token']).to be_kind_of(String) + end + + context 'signed out' do + let(:user) { nil } + let(:configuration) { create(:webauthn_configuration) } + + it 'responds with unauthorized response' do + expect(response_body).to eq(error: 'Unauthorized') + expect(response.status).to eq(401) + end + end + + context 'with invalid submission' do + let(:name) { '' } + + it 'responds with unsuccessful result' do + expect(response_body).to eq(success: false, error: t('errors.messages.blank')) + expect(response.status).to eq(400) + end + + it 'logs the submission attempt' do + response + + expect(@analytics).to have_logged_event( + :piv_cac_update_name_submitted, + success: false, + configuration_id: configuration.id.to_s, + error_details: { name: { blank: true } }, + ) + end + end + + context 'not recently authenticated' do + before do + allow(controller).to receive(:recently_authenticated_2fa?).and_return(false) + end + + it 'responds with unauthorized response' do + expect(response_body).to eq(error: 'Unauthorized') + expect(response.status).to eq(401) + end + end + + context 'with a configuration that does not exist' do + let(:params) { { id: 0 } } + + it 'responds with unsuccessful result' do + expect(response_body).to eq( + success: false, + error: t('errors.manage_authenticator.internal_error'), + ) + expect(response.status).to eq(400) + end + end + + context 'with a configuration that does not belong to the user' do + let(:configuration) { create(:webauthn_configuration) } + + it 'responds with unsuccessful result' do + expect(response_body).to eq( + success: false, + error: t('errors.manage_authenticator.internal_error'), + ) + expect(response.status).to eq(400) + end + end + end + + describe '#destroy' do + let(:params) { { id: configuration.id } } + let(:response) { delete :destroy, params: params } + subject(:response_body) { JSON.parse(response.body, symbolize_names: true) } + + it 'responds with successful result' do + expect(response_body).to eq(success: true) + expect(response.status).to eq(200) + end + + it 'logs the submission attempt' do + response + + expect(@analytics).to have_logged_event( + :piv_cac_delete_submitted, + success: true, + configuration_id: configuration.id.to_s, + error_details: nil, + ) + end + + it 'includes csrf token in the response headers' do + expect(response.headers['X-CSRF-Token']).to be_kind_of(String) + end + + it 'sends a recovery information changed event' do + expect(PushNotification::HttpPush).to receive(:deliver). + with(PushNotification::RecoveryInformationChangedEvent.new(user: user)) + + response + end + + it 'revokes remembered device' do + expect(user.remember_device_revoked_at).to eq nil + + freeze_time do + response + expect(user.reload.remember_device_revoked_at).to eq Time.zone.now + end + end + + it 'logs a user event for the removed credential' do + expect { response }.to change { user.events.piv_cac_disabled.size }.by 1 + end + + context 'signed out' do + let(:user) { nil } + let(:configuration) { create(:webauthn_configuration) } + + it 'responds with unauthorized response' do + expect(response_body).to eq(error: 'Unauthorized') + expect(response.status).to eq(401) + end + end + + context 'with invalid submission' do + let(:user) { create(:user) } + + it 'responds with unsuccessful result' do + expect(response_body).to eq( + success: false, + error: t('errors.manage_authenticator.remove_only_method_error'), + ) + expect(response.status).to eq(400) + end + + it 'logs the submission attempt' do + response + + expect(@analytics).to have_logged_event( + :piv_cac_delete_submitted, + success: false, + configuration_id: configuration.id.to_s, + error_details: { configuration_id: { only_method: true } }, + ) + end + end + + context 'not recently authenticated' do + before do + allow(controller).to receive(:recently_authenticated_2fa?).and_return(false) + end + + it 'responds with unauthorized response' do + expect(response_body).to eq(error: 'Unauthorized') + expect(response.status).to eq(401) + end + end + + context 'with a configuration that does not exist' do + let(:params) { { id: 0 } } + + it 'responds with unsuccessful result' do + expect(response_body).to eq( + success: false, + error: t('errors.manage_authenticator.internal_error'), + ) + expect(response.status).to eq(400) + end + end + + context 'with a configuration that does not belong to the user' do + let(:configuration) { create(:webauthn_configuration) } + + it 'responds with unsuccessful result' do + expect(response_body).to eq( + success: false, + error: t('errors.manage_authenticator.internal_error'), + ) + expect(response.status).to eq(400) + end + end + end +end diff --git a/spec/controllers/users/piv_cac_controller_spec.rb b/spec/controllers/users/piv_cac_controller_spec.rb index c017d01da46..f407d211e1f 100644 --- a/spec/controllers/users/piv_cac_controller_spec.rb +++ b/spec/controllers/users/piv_cac_controller_spec.rb @@ -67,6 +67,17 @@ expect(flash[:success]).to eq(presenter.rename_success_alert_text) end + it 'logs the submission attempt' do + response + + expect(@analytics).to have_logged_event( + :piv_cac_update_name_submitted, + success: true, + error_details: nil, + configuration_id: configuration.id.to_s, + ) + end + it 'assigns the form instance' do response @@ -128,6 +139,17 @@ expect(flash[:success]).to eq(presenter.delete_success_alert_text) end + it 'logs the submission attempt' do + response + + expect(@analytics).to have_logged_event( + :piv_cac_delete_submitted, + success: true, + configuration_id: configuration.id.to_s, + error_details: nil, + ) + end + it 'assigns the form instance' do response diff --git a/spec/features/users/piv_cac_management_spec.rb b/spec/features/users/piv_cac_management_spec.rb index 5b6f67e76f3..26ea4315e19 100644 --- a/spec/features/users/piv_cac_management_spec.rb +++ b/spec/features/users/piv_cac_management_spec.rb @@ -35,9 +35,9 @@ def find_form(page, attributes) expect(current_path).to eq account_path visit account_two_factor_authentication_path - expect(page.find('.remove-piv')).to_not be_nil - user.reload + expect(page).to have_link(href: edit_piv_cac_path(id: user.piv_cac_configurations.first.id)) + expect(user.piv_cac_configurations.first.x509_dn_uuid).to eq uuid expect(user.events.order(created_at: :desc).last.event_type).to eq('piv_cac_enabled') end @@ -160,6 +160,7 @@ def find_form(page, attributes) with: { phone: '+1 202-555-1212' }, ) end + let(:piv_name) { 'My PIV Card' } scenario 'does allow association of another piv/cac with the account' do stub_piv_cac_service @@ -175,15 +176,20 @@ def find_form(page, attributes) sign_in_and_2fa_user(user) visit account_two_factor_authentication_path - expect(page.find('.remove-piv')).to_not be_nil - page.find('.remove-piv').click + expect(page).to have_content(piv_name) - expect(current_path).to eq piv_cac_delete_path - click_on t('account.index.piv_cac_confirm_delete') + click_link( + format( + '%s: %s', + t('two_factor_authentication.piv_cac.manage_accessible_label'), + piv_name, + ), + ) - expect(current_path).to eq account_two_factor_authentication_path + expect(current_path).to eq(edit_piv_cac_path(id: user.piv_cac_configurations.first.id)) + click_button t('two_factor_authentication.piv_cac.delete') - expect(page).to have_link(t('account.index.piv_cac_add'), href: setup_piv_cac_url) + expect(page).to have_content(t('two_factor_authentication.piv_cac.deleted')) user.reload expect(user.piv_cac_configurations.first&.x509_dn_uuid).to be_nil diff --git a/spec/views/accounts/show.html.erb_spec.rb b/spec/views/accounts/show.html.erb_spec.rb index 22e8364a511..58ddbac7c7b 100644 --- a/spec/views/accounts/show.html.erb_spec.rb +++ b/spec/views/accounts/show.html.erb_spec.rb @@ -120,6 +120,11 @@ context 'user has a piv/cac' do let(:user) { create(:user, :fully_registered, :with_piv_or_cac) } + + before do + allow(view).to receive(:user_session).and_return({}) + end + it 'renders the piv/cac section' do render diff --git a/spec/views/users/piv_cac/edit.html.erb_spec.rb b/spec/views/users/piv_cac/edit.html.erb_spec.rb new file mode 100644 index 00000000000..75e97e6e3dd --- /dev/null +++ b/spec/views/users/piv_cac/edit.html.erb_spec.rb @@ -0,0 +1,46 @@ +require 'rails_helper' + +RSpec.describe 'users/piv_cac/edit.html.erb' do + include Devise::Test::ControllerHelpers + + let(:nickname) { 'Example' } + let(:configuration) { create(:piv_cac_configuration, name: nickname) } + let(:user) { create(:user, piv_cac_configurations: [configuration]) } + let(:form) do + TwoFactorAuthentication::PivCacUpdateForm.new( + user:, + configuration_id: configuration.id, + ) + end + let(:presenter) do + TwoFactorAuthentication::PivCacEditPresenter.new + end + + subject(:rendered) { render } + + before do + @form = form + @presenter = presenter + end + + it 'renders form to update configuration' do + expect(rendered).to have_selector( + "form[action='#{piv_cac_path(id: configuration.id)}'] input[name='_method'][value='put']", + visible: false, + ) + end + + it 'initializes form with configuration values' do + expect(rendered).to have_field( + t('two_factor_authentication.piv_cac.nickname'), + with: nickname, + ) + end + + it 'has labelled form with button to delete configuration' do + expect(rendered).to have_button_to_with_accessibility( + t('two_factor_authentication.piv_cac.delete'), + piv_cac_path(id: configuration.id), + ) + end +end