From cf15c61a3c84641313dbe1a13f7c8f704ae58227 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Wed, 20 Mar 2024 08:43:57 -0400 Subject: [PATCH 01/36] interstitial --- .../gov_or_mil_email_detected_controller.rb | 25 +++++++++++++++++++ app/models/email_address.rb | 4 +++ app/models/user.rb | 4 +++ .../two_factor_options_presenter.rb | 4 +++ config/routes.rb | 1 + db/schema.rb | 2 +- 6 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 app/controllers/users/gov_or_mil_email_detected_controller.rb diff --git a/app/controllers/users/gov_or_mil_email_detected_controller.rb b/app/controllers/users/gov_or_mil_email_detected_controller.rb new file mode 100644 index 00000000000..e8f580d2c94 --- /dev/null +++ b/app/controllers/users/gov_or_mil_email_detected_controller.rb @@ -0,0 +1,25 @@ +module Users + class GovOrMilEmailDetectedController < ApplicationController + include TwoFactorAuthenticatableMethods + include MfaSetupConcern + include SecureHeadersConcern + include ReauthenticationRequiredConcern + + before_action :authenticate_user! + before_action :confirm_user_authenticated_for_2fa_setup + before_action :apply_secure_headers_override + before_action :user_email_is_gov_or_mil? + + helper_method :in_multi_mfa_selection_flow? + + def show + analytics.gov_or_mil_email_detected + end + + + def user_email_is_gov_or_mil? + redirect_to after_sign_in_path_for(current_user) unless current_user.has_gov_or_mil_email? + end + end + end + \ No newline at end of file diff --git a/app/models/email_address.rb b/app/models/email_address.rb index b451f4afe04..c5801c73c5f 100644 --- a/app/models/email_address.rb +++ b/app/models/email_address.rb @@ -17,6 +17,10 @@ def email=(email) self.email_fingerprint = email.present? ? encrypted_attributes[:email].fingerprint : '' end + def gov_or_mil? + self.email.end_with?('.gov', '.mil') + end + def confirmed? confirmed_at.present? end diff --git a/app/models/user.rb b/app/models/user.rb index 77a7d18dedd..3a9bf9fedef 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -74,6 +74,10 @@ def confirmed? email_addresses.where.not(confirmed_at: nil).any? end + def has_gov_or_mil_email? + confirmed_email_addresses.any?(&:gov_or_mil?) + end + def accepted_rules_of_use_still_valid? if self.accepted_terms_at.present? self.accepted_terms_at > IdentityConfig.store.rules_of_use_updated_at && diff --git a/app/presenters/two_factor_options_presenter.rb b/app/presenters/two_factor_options_presenter.rb index b7e5ae9748d..3ba0bdd11ac 100644 --- a/app/presenters/two_factor_options_presenter.rb +++ b/app/presenters/two_factor_options_presenter.rb @@ -149,4 +149,8 @@ def mfa_policy def user_has_dismissed_second_mfa_reminder? user.second_mfa_reminder_dismissed_at.present? end + + def user_has_gov_or_mil_email? + user.confirmed_email_addresses.any?(&:gov_or_mil?) + end end diff --git a/config/routes.rb b/config/routes.rb index 0e11ce3dcc0..67c2a60ba8e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -141,6 +141,7 @@ post 'login/add_piv_cac/prompt' => 'users/piv_cac_setup_from_sign_in#decline' get 'login/add_piv_cac/success' => 'users/piv_cac_setup_from_sign_in#success' post 'login/add_piv_cac/success' => 'users/piv_cac_setup_from_sign_in#next' + get '/login/gov_or_mil_email_detected' => 'users/gov_or_mil_email_detected#show' end if IdentityConfig.store.enable_test_routes diff --git a/db/schema.rb b/db/schema.rb index 4c3a718d4d5..ecaa39de52c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -653,7 +653,7 @@ add_foreign_key "iaa_gtcs", "partner_accounts" add_foreign_key "iaa_orders", "iaa_gtcs" add_foreign_key "in_person_enrollments", "profiles" - add_foreign_key "in_person_enrollments", "service_providers", column: "issuer", primary_key: "issuer" + add_foreign_key "in_person_enrollments", "service_providers", column: "issuer", primary_key: "issuer", validate: false add_foreign_key "in_person_enrollments", "users" add_foreign_key "integration_usages", "iaa_orders" add_foreign_key "integration_usages", "integrations" From b0b470a776f6c8a7126757b44f02de67a136f302 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Thu, 21 Mar 2024 08:00:21 -0400 Subject: [PATCH 02/36] add routes and migration --- .../gov_or_mil_email_detected_controller.rb | 15 ++++++++++++- app/services/analytics_events.rb | 4 ++++ .../gov_or_mil_email_detected/show.html.erb | 21 +++++++++++++++++++ config/locales/titles/en.yml | 1 + .../locales/two_factor_authentication/en.yml | 6 ++++++ config/routes.rb | 3 ++- 6 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 app/views/users/gov_or_mil_email_detected/show.html.erb diff --git a/app/controllers/users/gov_or_mil_email_detected_controller.rb b/app/controllers/users/gov_or_mil_email_detected_controller.rb index e8f580d2c94..4e1a3bee2f6 100644 --- a/app/controllers/users/gov_or_mil_email_detected_controller.rb +++ b/app/controllers/users/gov_or_mil_email_detected_controller.rb @@ -13,13 +13,26 @@ class GovOrMilEmailDetectedController < ApplicationController helper_method :in_multi_mfa_selection_flow? def show - analytics.gov_or_mil_email_detected + @email_type = email_type + analytics.gov_or_mil_email_detected_visited end + private + def user_email_is_gov_or_mil? redirect_to after_sign_in_path_for(current_user) unless current_user.has_gov_or_mil_email? end + + def email_type + address = current_user.confirmed_email_addresses.select {|address| address.gov_or_mil? } + case address.first.email.end_with?('.gov') + when true + '.gov' + else + '.mil' + end + end end end \ No newline at end of file diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index f62c191546d..2dcdf399c1d 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -3920,6 +3920,10 @@ def personal_key_reactivation_visited track_event('Personal key reactivation: Personal key form visited') end + def gov_or_mil_email_detected_visited + track_event('gov_or_mil_email_detected_visited') + end + # @param [Boolean] personal_key_present if personal key is present # Personal key viewed def personal_key_viewed(personal_key_present:, **extra) diff --git a/app/views/users/gov_or_mil_email_detected/show.html.erb b/app/views/users/gov_or_mil_email_detected/show.html.erb new file mode 100644 index 00000000000..f492791d260 --- /dev/null +++ b/app/views/users/gov_or_mil_email_detected/show.html.erb @@ -0,0 +1,21 @@ +<% self.title = t('titles.piv_cac_setup.upsell') %> + +<%= render AlertIconComponent.new(icon_name: :info_question) %> + +<%= render PageHeadingComponent.new.with_content(t('titles.piv_cac_setup.upsell')) %> + +

+ <%= t('two_factor_authentication.piv_cac_upsell.info', email_type: @email_type) %> +

+ +<%= form_tag(login_add_piv_cac_for_gov_email_path, method: :confirm, class: 'margin-top-5') do %> + <%= button_tag t('two_factor_authentication.piv_cac_upsell.add_piv'), type: 'submit', + class: 'usa-button usa-button--big usa-button--wide margin-bottom-2' %> +<% end %> + +<% if MfaPolicy.new(current_user).two_factor_enabled? %> + <%= link_to t('links.cancel'), account_path %> +<% else %> + <%= link_to t('two_factor_authentication.choose_another_option'), authentication_methods_setup_path %> +<% end %> + diff --git a/config/locales/titles/en.yml b/config/locales/titles/en.yml index 1a58bbca528..6a3f897285a 100644 --- a/config/locales/titles/en.yml +++ b/config/locales/titles/en.yml @@ -59,6 +59,7 @@ en: new: Use your PIV/CAC to sign in to your account piv_cac_setup: new: Use your PIV/CAC card to secure your account + upsell: Enhance your account security with a government employee ID present_piv_cac: Present your PIV/CAC present_webauthn: Connect your hardware security key reactivate_account: Reactivate your account diff --git a/config/locales/two_factor_authentication/en.yml b/config/locales/two_factor_authentication/en.yml index 4ef615402a5..a3528ffafef 100644 --- a/config/locales/two_factor_authentication/en.yml +++ b/config/locales/two_factor_authentication/en.yml @@ -146,6 +146,12 @@ en: nickname: Nickname renamed: Successfully renamed your PIV/CAC method piv_cac_header_text: Present your PIV/CAC + piv_cac_upsell: + add_piv: Add PIV/CAC card + choose_other_method: Choose other methods instead + info: Because you are using a %{email_type} email, we recommend you add your government + employee ID as one of your authentication methods. This adds another + layer of security to your account. please_try_again_html: Please try again in %{countdown}. read_about_two_factor_authentication: Read about two-factor authentication recaptcha: diff --git a/config/routes.rb b/config/routes.rb index 67c2a60ba8e..8970c23b165 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -141,7 +141,8 @@ post 'login/add_piv_cac/prompt' => 'users/piv_cac_setup_from_sign_in#decline' get 'login/add_piv_cac/success' => 'users/piv_cac_setup_from_sign_in#success' post 'login/add_piv_cac/success' => 'users/piv_cac_setup_from_sign_in#next' - get '/login/gov_or_mil_email_detected' => 'users/gov_or_mil_email_detected#show' + get 'login/gov_or_mil_email_detected' => 'users/gov_or_mil_email_detected#show' + post 'login/add_piv_cac_for_gov_email' => 'users/gov_or_mil_email_detected#confirm' end if IdentityConfig.store.enable_test_routes From 49f9501cb3af691c29494c420651fca81efd4c87 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Thu, 21 Mar 2024 11:16:36 -0400 Subject: [PATCH 03/36] changelog: User-Facing Improvements, PIV/CAC, Add Piv interstitial page for gov emails --- app/controllers/concerns/piv_cac_concern.rb | 2 +- ...cted_controller.rb => possible_piv_user_controller.rb} | 2 +- .../users/two_factor_authentication_setup_controller.rb | 8 ++++++++ config/routes.rb | 4 ++-- .../20240321121322_add_pivcac_notice_on_user.rb | 5 +++++ db/schema.rb | 3 ++- 6 files changed, 19 insertions(+), 5 deletions(-) rename app/controllers/users/{gov_or_mil_email_detected_controller.rb => possible_piv_user_controller.rb} (93%) create mode 100644 db/primary_migrate/20240321121322_add_pivcac_notice_on_user.rb diff --git a/app/controllers/concerns/piv_cac_concern.rb b/app/controllers/concerns/piv_cac_concern.rb index a305c4d42a9..ef2838d66ec 100644 --- a/app/controllers/concerns/piv_cac_concern.rb +++ b/app/controllers/concerns/piv_cac_concern.rb @@ -44,4 +44,4 @@ def piv_cac_setup_csp_form_action_uris end [piv_cac_uri] + csp_uris end -end +end diff --git a/app/controllers/users/gov_or_mil_email_detected_controller.rb b/app/controllers/users/possible_piv_user_controller.rb similarity index 93% rename from app/controllers/users/gov_or_mil_email_detected_controller.rb rename to app/controllers/users/possible_piv_user_controller.rb index 4e1a3bee2f6..d60f76a0315 100644 --- a/app/controllers/users/gov_or_mil_email_detected_controller.rb +++ b/app/controllers/users/possible_piv_user_controller.rb @@ -1,5 +1,5 @@ module Users - class GovOrMilEmailDetectedController < ApplicationController + class PossiblePivUserController < ApplicationController include TwoFactorAuthenticatableMethods include MfaSetupConcern include SecureHeadersConcern diff --git a/app/controllers/users/two_factor_authentication_setup_controller.rb b/app/controllers/users/two_factor_authentication_setup_controller.rb index 8437d30b05a..dd9a70494dd 100644 --- a/app/controllers/users/two_factor_authentication_setup_controller.rb +++ b/app/controllers/users/two_factor_authentication_setup_controller.rb @@ -2,9 +2,11 @@ module Users class TwoFactorAuthenticationSetupController < ApplicationController include UserAuthenticator include MfaSetupConcern + include PivCacConcern before_action :authenticate_user before_action :confirm_user_authenticated_for_2fa_setup + before_action :check_if_possible_piv_user delegate :enabled_mfa_methods_count, to: :mfa_context @@ -77,5 +79,11 @@ def two_factor_options_form_params rescue ActionController::ParameterMissing ActionController::Parameters.new(selection: []) end + + def check_if_possible_piv_user + if current_user.has_gov_or_mil_email? && !current_user.piv_cac_recommended_dismissed + redirect_to login_possible_piv_user_path + end + end end end diff --git a/config/routes.rb b/config/routes.rb index 8970c23b165..7b4741e525d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -141,8 +141,8 @@ post 'login/add_piv_cac/prompt' => 'users/piv_cac_setup_from_sign_in#decline' get 'login/add_piv_cac/success' => 'users/piv_cac_setup_from_sign_in#success' post 'login/add_piv_cac/success' => 'users/piv_cac_setup_from_sign_in#next' - get 'login/gov_or_mil_email_detected' => 'users/gov_or_mil_email_detected#show' - post 'login/add_piv_cac_for_gov_email' => 'users/gov_or_mil_email_detected#confirm' + get 'login/possible_piv_user' => 'users/possible_piv_user#show' + post 'login/add_piv_for_user' => 'users/possible_piv_user#confirm' end if IdentityConfig.store.enable_test_routes diff --git a/db/primary_migrate/20240321121322_add_pivcac_notice_on_user.rb b/db/primary_migrate/20240321121322_add_pivcac_notice_on_user.rb new file mode 100644 index 00000000000..5ef21dfc6d1 --- /dev/null +++ b/db/primary_migrate/20240321121322_add_pivcac_notice_on_user.rb @@ -0,0 +1,5 @@ +class AddPivcacNoticeOnUser < ActiveRecord::Migration[7.1] + def change + add_column :users, :piv_cac_recommended_dismissed, :boolean + end +end diff --git a/db/schema.rb b/db/schema.rb index ecaa39de52c..2a8bfaae5c9 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2024_02_16_184124) do +ActiveRecord::Schema[7.1].define(version: 2024_03_21_121322) do # These are extensions that must be enabled in order to support this database enable_extension "citext" enable_extension "pg_stat_statements" @@ -612,6 +612,7 @@ t.string "encrypted_password_digest_multi_region" t.string "encrypted_recovery_code_digest_multi_region" t.datetime "second_mfa_reminder_dismissed_at" + t.boolean "piv_cac_recommended_dismissed" t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true t.index ["uuid"], name: "index_users_on_uuid", unique: true end From f3b53d7020fc2992a3b6a6fa3b84330f56a5d4ec Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Thu, 21 Mar 2024 16:49:14 -0400 Subject: [PATCH 04/36] possible piv user --- .../users/possible_piv_user_controller.rb | 5 ++++ .../gov_or_mil_email_detected/show.html.erb | 21 -------------- .../users/possible_piv_user/show.html.erb | 28 +++++++++++++++++++ ...0240321121322_add_pivcac_notice_on_user.rb | 2 +- db/schema.rb | 2 +- 5 files changed, 35 insertions(+), 23 deletions(-) delete mode 100644 app/views/users/gov_or_mil_email_detected/show.html.erb create mode 100644 app/views/users/possible_piv_user/show.html.erb diff --git a/app/controllers/users/possible_piv_user_controller.rb b/app/controllers/users/possible_piv_user_controller.rb index d60f76a0315..ea85b4751f0 100644 --- a/app/controllers/users/possible_piv_user_controller.rb +++ b/app/controllers/users/possible_piv_user_controller.rb @@ -17,6 +17,11 @@ def show analytics.gov_or_mil_email_detected_visited end + def confirm + # UpdateUser.new(user: current_user, attributes: { piv_cac_recommended_dismissed: true }).call + redirect_to confirmation_path('piv_cac') + end + private diff --git a/app/views/users/gov_or_mil_email_detected/show.html.erb b/app/views/users/gov_or_mil_email_detected/show.html.erb deleted file mode 100644 index f492791d260..00000000000 --- a/app/views/users/gov_or_mil_email_detected/show.html.erb +++ /dev/null @@ -1,21 +0,0 @@ -<% self.title = t('titles.piv_cac_setup.upsell') %> - -<%= render AlertIconComponent.new(icon_name: :info_question) %> - -<%= render PageHeadingComponent.new.with_content(t('titles.piv_cac_setup.upsell')) %> - -

- <%= t('two_factor_authentication.piv_cac_upsell.info', email_type: @email_type) %> -

- -<%= form_tag(login_add_piv_cac_for_gov_email_path, method: :confirm, class: 'margin-top-5') do %> - <%= button_tag t('two_factor_authentication.piv_cac_upsell.add_piv'), type: 'submit', - class: 'usa-button usa-button--big usa-button--wide margin-bottom-2' %> -<% end %> - -<% if MfaPolicy.new(current_user).two_factor_enabled? %> - <%= link_to t('links.cancel'), account_path %> -<% else %> - <%= link_to t('two_factor_authentication.choose_another_option'), authentication_methods_setup_path %> -<% end %> - diff --git a/app/views/users/possible_piv_user/show.html.erb b/app/views/users/possible_piv_user/show.html.erb new file mode 100644 index 00000000000..073c0c300e3 --- /dev/null +++ b/app/views/users/possible_piv_user/show.html.erb @@ -0,0 +1,28 @@ +<% self.title = t('titles.piv_cac_setup.upsell') %> + +<%= render AlertIconComponent.new(icon_name: :info_question) %> + +<%= render PageHeadingComponent.new.with_content(t('titles.piv_cac_setup.upsell')) %> + +

+ <%= t('two_factor_authentication.piv_cac_upsell.info', email_type: @email_type) %> +

+ +<%= render ButtonComponent.new( + action: ->(**tag_options, &block) do + button_to( + login_add_piv_for_user_path, + **tag_options, + &block + ) + end, + big: true, + class: 'margin-bottom-2', + ).with_content(t('two_factor_authentication.piv_cac_upsell.add_piv')) %> + +<% if MfaPolicy.new(current_user).two_factor_enabled? %> + <%= link_to t('links.cancel'), account_path %> +<% else %> + <%= link_to t('two_factor_authentication.piv_cac_upsell.choose_other_method'), authentication_methods_setup_path %> +<% end %> + diff --git a/db/primary_migrate/20240321121322_add_pivcac_notice_on_user.rb b/db/primary_migrate/20240321121322_add_pivcac_notice_on_user.rb index 5ef21dfc6d1..cb26fd1aafd 100644 --- a/db/primary_migrate/20240321121322_add_pivcac_notice_on_user.rb +++ b/db/primary_migrate/20240321121322_add_pivcac_notice_on_user.rb @@ -1,5 +1,5 @@ class AddPivcacNoticeOnUser < ActiveRecord::Migration[7.1] def change - add_column :users, :piv_cac_recommended_dismissed, :boolean + add_column :users, :piv_cac_recommended_dismissed, :boolean, default: false end end diff --git a/db/schema.rb b/db/schema.rb index 2a8bfaae5c9..da76613e4ef 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -612,7 +612,7 @@ t.string "encrypted_password_digest_multi_region" t.string "encrypted_recovery_code_digest_multi_region" t.datetime "second_mfa_reminder_dismissed_at" - t.boolean "piv_cac_recommended_dismissed" + t.boolean "piv_cac_recommended_dismissed", default: false t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true t.index ["uuid"], name: "index_users_on_uuid", unique: true end From fc237df3b60f9067aa18df30bce75cffa8ef2b37 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Thu, 21 Mar 2024 18:51:44 -0400 Subject: [PATCH 05/36] possible piv user --- app/controllers/users/possible_piv_user_controller.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/controllers/users/possible_piv_user_controller.rb b/app/controllers/users/possible_piv_user_controller.rb index ea85b4751f0..daf678de874 100644 --- a/app/controllers/users/possible_piv_user_controller.rb +++ b/app/controllers/users/possible_piv_user_controller.rb @@ -18,8 +18,9 @@ def show end def confirm - # UpdateUser.new(user: current_user, attributes: { piv_cac_recommended_dismissed: true }).call - redirect_to confirmation_path('piv_cac') + UpdateUser.new(user: current_user, attributes: { piv_cac_recommended_dismissed: true }).call + user_session[:mfa_selections] = ['piv_cac'] + redirect_to confirmation_path(user_session[:mfa_selections].first) end From bdc91f49e9530c297cf738d693c78abf68fc86a5 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Fri, 22 Mar 2024 10:20:37 -0400 Subject: [PATCH 06/36] possible piv user built --- .../users/possible_piv_user_controller.rb | 4 ++++ .../users/possible_piv_user/show.html.erb | 12 ++++++++++- .../possible_piv_user_controller_spec.rb | 21 +++++++++++++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 spec/controllers/users/possible_piv_user_controller_spec.rb diff --git a/app/controllers/users/possible_piv_user_controller.rb b/app/controllers/users/possible_piv_user_controller.rb index daf678de874..e7a9f33fd69 100644 --- a/app/controllers/users/possible_piv_user_controller.rb +++ b/app/controllers/users/possible_piv_user_controller.rb @@ -23,6 +23,10 @@ def confirm redirect_to confirmation_path(user_session[:mfa_selections].first) end + def skip + UpdateUser.new(user: current_user, attributes: { piv_cac_recommended_dismissed: true }).call + end + private diff --git a/app/views/users/possible_piv_user/show.html.erb b/app/views/users/possible_piv_user/show.html.erb index 073c0c300e3..5ee316799d1 100644 --- a/app/views/users/possible_piv_user/show.html.erb +++ b/app/views/users/possible_piv_user/show.html.erb @@ -23,6 +23,16 @@ <% if MfaPolicy.new(current_user).two_factor_enabled? %> <%= link_to t('links.cancel'), account_path %> <% else %> - <%= link_to t('two_factor_authentication.piv_cac_upsell.choose_other_method'), authentication_methods_setup_path %> + <%= render ButtonComponent.new( + action: ->(**tag_options, &block) do + button_to( + login_add_piv_for_user_path, + **tag_options, + &block + ) + end, + big: true, + class: 'margin-bottom-2', + ).with_content(t('two_factor_authentication.piv_cac_upsell.choose_other_method')) %> <% end %> diff --git a/spec/controllers/users/possible_piv_user_controller_spec.rb b/spec/controllers/users/possible_piv_user_controller_spec.rb new file mode 100644 index 00000000000..cc78b0f7449 --- /dev/null +++ b/spec/controllers/users/possible_piv_user_controller_spec.rb @@ -0,0 +1,21 @@ +require 'rails_helper' + +RSpec.describe Users::PleaseCallController do + let(:user) { create(:user, :suspended) } + + before do + stub_sign_in(user) + end + + it 'renders the show template' do + stub_analytics + + expect(@analytics).to receive(:track_event).with( + 'User Suspension: Please call visited', + ) + + get :show + + expect(response).to render_template :show + end +end From f430663925fbd2be8161b7637f09542eb2bd7125 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Fri, 22 Mar 2024 11:24:31 -0400 Subject: [PATCH 07/36] rename controller to something more cohesive, add spec for controller --- ...r.rb => piv_cac_recommended_controller.rb} | 7 +++-- app/services/analytics_events.rb | 16 ++++++++--- .../show.html.erb | 28 ++++++++----------- config/routes.rb | 5 ++-- .../piv_cac_recommended_controller_spec.rb | 21 ++++++++++++++ .../possible_piv_user_controller_spec.rb | 21 -------------- 6 files changed, 53 insertions(+), 45 deletions(-) rename app/controllers/users/{possible_piv_user_controller.rb => piv_cac_recommended_controller.rb} (83%) rename app/views/users/{possible_piv_user => piv_cac_recommended}/show.html.erb (52%) create mode 100644 spec/controllers/users/piv_cac_recommended_controller_spec.rb delete mode 100644 spec/controllers/users/possible_piv_user_controller_spec.rb diff --git a/app/controllers/users/possible_piv_user_controller.rb b/app/controllers/users/piv_cac_recommended_controller.rb similarity index 83% rename from app/controllers/users/possible_piv_user_controller.rb rename to app/controllers/users/piv_cac_recommended_controller.rb index e7a9f33fd69..194b805d701 100644 --- a/app/controllers/users/possible_piv_user_controller.rb +++ b/app/controllers/users/piv_cac_recommended_controller.rb @@ -1,5 +1,5 @@ module Users - class PossiblePivUserController < ApplicationController + class PivCacRecommendedController < ApplicationController include TwoFactorAuthenticatableMethods include MfaSetupConcern include SecureHeadersConcern @@ -14,17 +14,20 @@ class PossiblePivUserController < ApplicationController def show @email_type = email_type - analytics.gov_or_mil_email_detected_visited + analytics.piv_cac_recommended_page_visited end def confirm UpdateUser.new(user: current_user, attributes: { piv_cac_recommended_dismissed: true }).call user_session[:mfa_selections] = ['piv_cac'] + analytics.piv_cac_recommended_accepted redirect_to confirmation_path(user_session[:mfa_selections].first) end def skip UpdateUser.new(user: current_user, attributes: { piv_cac_recommended_dismissed: true }).call + analytics.piv_cac_recommended_skipped + redirect_to after_sign_in_path_for(current_user) end diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index 7785e312584..6504b6ec447 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -3986,10 +3986,6 @@ def personal_key_reactivation_visited track_event('Personal key reactivation: Personal key form visited') end - def gov_or_mil_email_detected_visited - track_event('gov_or_mil_email_detected_visited') - end - # @param [Boolean] personal_key_present if personal key is present # Personal key viewed def personal_key_viewed(personal_key_present:, **extra) @@ -4120,6 +4116,18 @@ def piv_cac_update_name_submitted( ) end + def piv_cac_recommended_page_visited + track_event('piv_cac_recommended_page_visited') + end + + def piv_cac_recommended_accepted + track_event('piv_cac_recommended_accepted') + end + + def piv_cac_recommended_skipped + track_event('piv_cac_recommended_skipped') + 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/app/views/users/possible_piv_user/show.html.erb b/app/views/users/piv_cac_recommended/show.html.erb similarity index 52% rename from app/views/users/possible_piv_user/show.html.erb rename to app/views/users/piv_cac_recommended/show.html.erb index 5ee316799d1..55da7aab4e6 100644 --- a/app/views/users/possible_piv_user/show.html.erb +++ b/app/views/users/piv_cac_recommended/show.html.erb @@ -11,7 +11,7 @@ <%= render ButtonComponent.new( action: ->(**tag_options, &block) do button_to( - login_add_piv_for_user_path, + login_piv_cac_recommended_add_path, **tag_options, &block ) @@ -20,19 +20,15 @@ class: 'margin-bottom-2', ).with_content(t('two_factor_authentication.piv_cac_upsell.add_piv')) %> -<% if MfaPolicy.new(current_user).two_factor_enabled? %> - <%= link_to t('links.cancel'), account_path %> -<% else %> - <%= render ButtonComponent.new( - action: ->(**tag_options, &block) do - button_to( - login_add_piv_for_user_path, - **tag_options, - &block - ) - end, - big: true, - class: 'margin-bottom-2', - ).with_content(t('two_factor_authentication.piv_cac_upsell.choose_other_method')) %> -<% end %> +<%= render ButtonComponent.new( + action: ->(**tag_options, &block) do + button_to( + login_piv_cac_recommended_skip_path, + **tag_options, + &block + ) + end, + big: true, + class: 'margin-bottom-2', + ).with_content(t('two_factor_authentication.piv_cac_upsell.choose_other_method')) %> diff --git a/config/routes.rb b/config/routes.rb index 7b4741e525d..a4fb85de2e0 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -141,8 +141,9 @@ post 'login/add_piv_cac/prompt' => 'users/piv_cac_setup_from_sign_in#decline' get 'login/add_piv_cac/success' => 'users/piv_cac_setup_from_sign_in#success' post 'login/add_piv_cac/success' => 'users/piv_cac_setup_from_sign_in#next' - get 'login/possible_piv_user' => 'users/possible_piv_user#show' - post 'login/add_piv_for_user' => 'users/possible_piv_user#confirm' + get 'login/piv_cac_recommeded' => 'users/piv_cac_recommeded#show' + post 'login/piv_cac_recommended/add' => 'users/piv_cac_recommeded#confirm' + post 'login/piv_cac_recommended/skip' => 'users/piv_cac_recommeded#skip' end if IdentityConfig.store.enable_test_routes diff --git a/spec/controllers/users/piv_cac_recommended_controller_spec.rb b/spec/controllers/users/piv_cac_recommended_controller_spec.rb new file mode 100644 index 00000000000..978c0c6643f --- /dev/null +++ b/spec/controllers/users/piv_cac_recommended_controller_spec.rb @@ -0,0 +1,21 @@ +require 'rails_helper' + +RSpec.describe Users::PivCacRecommendedController do + before do + user = build(:user) + stub_sign_in_before_2fa(user) + stub_analytics + end + + context '#show' do + + end + + context '#confirm' do + + end + + context '#skip' do + + end +end diff --git a/spec/controllers/users/possible_piv_user_controller_spec.rb b/spec/controllers/users/possible_piv_user_controller_spec.rb deleted file mode 100644 index cc78b0f7449..00000000000 --- a/spec/controllers/users/possible_piv_user_controller_spec.rb +++ /dev/null @@ -1,21 +0,0 @@ -require 'rails_helper' - -RSpec.describe Users::PleaseCallController do - let(:user) { create(:user, :suspended) } - - before do - stub_sign_in(user) - end - - it 'renders the show template' do - stub_analytics - - expect(@analytics).to receive(:track_event).with( - 'User Suspension: Please call visited', - ) - - get :show - - expect(response).to render_template :show - end -end From 3a9865d69312ce10b7e81c2532f29705fb3bfe70 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Fri, 22 Mar 2024 11:38:08 -0400 Subject: [PATCH 08/36] rubocop fixes --- app/controllers/concerns/piv_cac_concern.rb | 2 +- .../users/piv_cac_recommended_controller.rb | 80 +++++++++---------- ..._factor_authentication_setup_controller.rb | 2 +- app/models/email_address.rb | 4 - app/services/analytics_events.rb | 6 +- .../piv_cac_recommended_controller_spec.rb | 3 - 6 files changed, 44 insertions(+), 53 deletions(-) diff --git a/app/controllers/concerns/piv_cac_concern.rb b/app/controllers/concerns/piv_cac_concern.rb index ef2838d66ec..a305c4d42a9 100644 --- a/app/controllers/concerns/piv_cac_concern.rb +++ b/app/controllers/concerns/piv_cac_concern.rb @@ -44,4 +44,4 @@ def piv_cac_setup_csp_form_action_uris end [piv_cac_uri] + csp_uris end -end +end diff --git a/app/controllers/users/piv_cac_recommended_controller.rb b/app/controllers/users/piv_cac_recommended_controller.rb index 194b805d701..dbf2cc0bc8b 100644 --- a/app/controllers/users/piv_cac_recommended_controller.rb +++ b/app/controllers/users/piv_cac_recommended_controller.rb @@ -1,51 +1,49 @@ module Users - class PivCacRecommendedController < ApplicationController - include TwoFactorAuthenticatableMethods - include MfaSetupConcern - include SecureHeadersConcern - include ReauthenticationRequiredConcern - - before_action :authenticate_user! - before_action :confirm_user_authenticated_for_2fa_setup - before_action :apply_secure_headers_override - before_action :user_email_is_gov_or_mil? - - helper_method :in_multi_mfa_selection_flow? - - def show - @email_type = email_type - analytics.piv_cac_recommended_page_visited - end + class PivCacRecommendedController < ApplicationController + include TwoFactorAuthenticatableMethods + include MfaSetupConcern + include SecureHeadersConcern + include ReauthenticationRequiredConcern - def confirm - UpdateUser.new(user: current_user, attributes: { piv_cac_recommended_dismissed: true }).call - user_session[:mfa_selections] = ['piv_cac'] - analytics.piv_cac_recommended_accepted - redirect_to confirmation_path(user_session[:mfa_selections].first) - end + before_action :authenticate_user! + before_action :confirm_user_authenticated_for_2fa_setup + before_action :apply_secure_headers_override + before_action :user_email_is_gov_or_mil? - def skip - UpdateUser.new(user: current_user, attributes: { piv_cac_recommended_dismissed: true }).call - analytics.piv_cac_recommended_skipped - redirect_to after_sign_in_path_for(current_user) - end + helper_method :in_multi_mfa_selection_flow? + def show + @email_type = email_type + analytics.piv_cac_recommended_page_visited + end - private + def confirm + UpdateUser.new(user: current_user, attributes: { piv_cac_recommended_dismissed: true }).call + user_session[:mfa_selections] = ['piv_cac'] + analytics.piv_cac_recommended_accepted + redirect_to confirmation_path(user_session[:mfa_selections].first) + end - def user_email_is_gov_or_mil? - redirect_to after_sign_in_path_for(current_user) unless current_user.has_gov_or_mil_email? - end + def skip + UpdateUser.new(user: current_user, attributes: { piv_cac_recommended_dismissed: true }).call + analytics.piv_cac_recommended_skipped + redirect_to after_sign_in_path_for(current_user) + end + + private - def email_type - address = current_user.confirmed_email_addresses.select {|address| address.gov_or_mil? } - case address.first.email.end_with?('.gov') - when true - '.gov' - else - '.mil' - end + def user_email_is_gov_or_mil? + redirect_to after_sign_in_path_for(current_user) unless current_user.has_gov_or_mil_email? + end + + def email_type + address = current_user.confirmed_email_addresses.select { |address| address.gov_or_mil? } + case address.first.email.end_with?('.gov') + when true + '.gov' + else + '.mil' end end end - \ No newline at end of file + end diff --git a/app/controllers/users/two_factor_authentication_setup_controller.rb b/app/controllers/users/two_factor_authentication_setup_controller.rb index 36d00e11029..e4ad737cb15 100644 --- a/app/controllers/users/two_factor_authentication_setup_controller.rb +++ b/app/controllers/users/two_factor_authentication_setup_controller.rb @@ -89,7 +89,7 @@ def two_factor_options_form_params def check_if_possible_piv_user if current_user.has_gov_or_mil_email? && !current_user.piv_cac_recommended_dismissed - redirect_to login_possible_piv_user_path + redirect_to login_possible_piv_user_path end end end diff --git a/app/models/email_address.rb b/app/models/email_address.rb index dea2430921f..b02f68b965f 100644 --- a/app/models/email_address.rb +++ b/app/models/email_address.rb @@ -17,10 +17,6 @@ def email=(email) self.email_fingerprint = email.present? ? encrypted_attributes[:email].fingerprint : '' end - def gov_or_mil? - self.email.end_with?('.gov', '.mil') - end - def confirmed? confirmed_at.present? end diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index 6504b6ec447..58d031e41a9 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -4117,15 +4117,15 @@ def piv_cac_update_name_submitted( end def piv_cac_recommended_page_visited - track_event('piv_cac_recommended_page_visited') + track_event(:piv_cac_recommended_page_visited) end def piv_cac_recommended_accepted - track_event('piv_cac_recommended_accepted') + track_event(:piv_cac_recommended_accepted) end def piv_cac_recommended_skipped - track_event('piv_cac_recommended_skipped') + track_event(:piv_cac_recommended_skipped) end # @param [String] redirect_url URL user was directed to diff --git a/spec/controllers/users/piv_cac_recommended_controller_spec.rb b/spec/controllers/users/piv_cac_recommended_controller_spec.rb index 978c0c6643f..08445b007f3 100644 --- a/spec/controllers/users/piv_cac_recommended_controller_spec.rb +++ b/spec/controllers/users/piv_cac_recommended_controller_spec.rb @@ -8,14 +8,11 @@ end context '#show' do - end context '#confirm' do - end context '#skip' do - end end From 1af7c198521c9946a39852025e32970552bb89a1 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Fri, 22 Mar 2024 12:47:00 -0400 Subject: [PATCH 09/36] concern for pivcac --- app/controllers/application_controller.rb | 5 +++++ app/controllers/concerns/mfa_setup_concern.rb | 7 +++++++ .../users/piv_cac_recommended_controller.rb | 11 ++++++++++- .../two_factor_authentication_setup_controller.rb | 6 ------ app/views/users/piv_cac_recommended/show.html.erb | 4 ++-- 5 files changed, 24 insertions(+), 9 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index a982d7b95f8..1c41ba61ec6 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -234,6 +234,7 @@ def after_sign_in_path_for(_user) return fix_broken_personal_key_url if current_user.broken_personal_key? return user_session.delete(:stored_location) if user_session.key?(:stored_location) return reactivate_account_url if user_needs_to_reactivate_account? + return piv_cac_recommended_path if user_recommended_for_piv_cac? return second_mfa_reminder_url if user_needs_second_mfa_reminder? return sp_session_request_url_with_updated_params if sp_session.key?(:request_url) signed_in_url @@ -261,6 +262,10 @@ def user_needs_to_reactivate_account? resolved_authn_context_result.identity_proofing? end + def user_recommended_for_piv_cac? + current_user.has_gov_or_mil_email? && !current_user.piv_cac_recommended_dismissed + end + def pending_profile_newer_than_password_reset_profile? return false if current_user.pending_profile.blank? return false if current_user.password_reset_profile.blank? diff --git a/app/controllers/concerns/mfa_setup_concern.rb b/app/controllers/concerns/mfa_setup_concern.rb index 6120cd979f9..788223493c6 100644 --- a/app/controllers/concerns/mfa_setup_concern.rb +++ b/app/controllers/concerns/mfa_setup_concern.rb @@ -71,6 +71,13 @@ def show_skip_additional_mfa_link? mfa_context.webauthn_platform_configurations.count == 1) end + + def check_if_possible_piv_user + if current_user.has_gov_or_mil_email? && !current_user.piv_cac_recommended_dismissed + redirect_to login_possible_piv_user_path + end + end + private def track_user_registration_mfa_setup_complete_event diff --git a/app/controllers/users/piv_cac_recommended_controller.rb b/app/controllers/users/piv_cac_recommended_controller.rb index dbf2cc0bc8b..4119a63faea 100644 --- a/app/controllers/users/piv_cac_recommended_controller.rb +++ b/app/controllers/users/piv_cac_recommended_controller.rb @@ -14,6 +14,7 @@ class PivCacRecommendedController < ApplicationController def show @email_type = email_type + @skip_text = skip_text analytics.piv_cac_recommended_page_visited end @@ -45,5 +46,13 @@ def email_type '.mil' end end + + def skip_text + if MfaPolicy.new(current_user).two_factor_enabled? + t('mfa.skip') + else + t('two_factor_authentication.piv_cac_upsell.choose_other_method') + end + end end - end +end diff --git a/app/controllers/users/two_factor_authentication_setup_controller.rb b/app/controllers/users/two_factor_authentication_setup_controller.rb index e4ad737cb15..33c3bae7bb5 100644 --- a/app/controllers/users/two_factor_authentication_setup_controller.rb +++ b/app/controllers/users/two_factor_authentication_setup_controller.rb @@ -86,11 +86,5 @@ def two_factor_options_form_params rescue ActionController::ParameterMissing ActionController::Parameters.new(selection: []) end - - def check_if_possible_piv_user - if current_user.has_gov_or_mil_email? && !current_user.piv_cac_recommended_dismissed - redirect_to login_possible_piv_user_path - end - end end end diff --git a/app/views/users/piv_cac_recommended/show.html.erb b/app/views/users/piv_cac_recommended/show.html.erb index 55da7aab4e6..7c446728cbc 100644 --- a/app/views/users/piv_cac_recommended/show.html.erb +++ b/app/views/users/piv_cac_recommended/show.html.erb @@ -17,7 +17,7 @@ ) end, big: true, - class: 'margin-bottom-2', + class: 'margin-bottom-3', ).with_content(t('two_factor_authentication.piv_cac_upsell.add_piv')) %> <%= render ButtonComponent.new( @@ -30,5 +30,5 @@ end, big: true, class: 'margin-bottom-2', - ).with_content(t('two_factor_authentication.piv_cac_upsell.choose_other_method')) %> + ).with_content(@skip_text) %> From 47cd73411c2d236954963b53d1303a7bd09c4177 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Mon, 25 Mar 2024 10:11:43 -0400 Subject: [PATCH 10/36] MFA setup redirect to piv for proper emails --- app/controllers/concerns/mfa_setup_concern.rb | 3 +- config/routes.rb | 6 ++-- spec/features/users/sign_up_spec.rb | 28 +++++++++++++++++++ 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/app/controllers/concerns/mfa_setup_concern.rb b/app/controllers/concerns/mfa_setup_concern.rb index 788223493c6..2f5e631c3bc 100644 --- a/app/controllers/concerns/mfa_setup_concern.rb +++ b/app/controllers/concerns/mfa_setup_concern.rb @@ -71,10 +71,9 @@ def show_skip_additional_mfa_link? mfa_context.webauthn_platform_configurations.count == 1) end - def check_if_possible_piv_user if current_user.has_gov_or_mil_email? && !current_user.piv_cac_recommended_dismissed - redirect_to login_possible_piv_user_path + redirect_to login_piv_cac_recommended_path end end diff --git a/config/routes.rb b/config/routes.rb index a4fb85de2e0..871f6f6ee0d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -141,9 +141,9 @@ post 'login/add_piv_cac/prompt' => 'users/piv_cac_setup_from_sign_in#decline' get 'login/add_piv_cac/success' => 'users/piv_cac_setup_from_sign_in#success' post 'login/add_piv_cac/success' => 'users/piv_cac_setup_from_sign_in#next' - get 'login/piv_cac_recommeded' => 'users/piv_cac_recommeded#show' - post 'login/piv_cac_recommended/add' => 'users/piv_cac_recommeded#confirm' - post 'login/piv_cac_recommended/skip' => 'users/piv_cac_recommeded#skip' + get 'login/piv_cac_recommended' => 'users/piv_cac_recommended#show' + post 'login/piv_cac_recommended/add' => 'users/piv_cac_recommended#confirm' + post 'login/piv_cac_recommended/skip' => 'users/piv_cac_recommended#skip' end if IdentityConfig.store.enable_test_routes diff --git a/spec/features/users/sign_up_spec.rb b/spec/features/users/sign_up_spec.rb index a5810561a03..a20e10805fa 100644 --- a/spec/features/users/sign_up_spec.rb +++ b/spec/features/users/sign_up_spec.rb @@ -516,6 +516,34 @@ def clipboard_text end end + describe 'mil or gov email account' do + before do + confirm_email('test@test.gov') + submit_form_with_valid_password + end + it 'should land user on piv cac suggestion page' do + expect(current_path).to eq login_piv_cac_recommended_path + end + + context 'user can skip piv cac prompt' do + it 'should skip piv cac prompt and land of mfa screen' do + expect(current_path).to eq login_piv_cac_recommended_path + click_button t('two_factor_authentication.piv_cac_upsell.choose_other_method') + + expect(current_path).to eq authentication_methods_setup_path + end + end + + context 'user who selects to add piv is directed to piv screen' do + it 'should be directed straight to piv add screen' do + expect(current_path).to eq login_piv_cac_recommended_path + click_button t('two_factor_authentication.piv_cac_upsell.add_piv') + + expect(current_path).to eq setup_piv_cac_path + end + end + end + def click_2fa_option(option) find("label[for='two_factor_options_form_selection_#{option}']").click end From 222921ee80d47f1f18a65ed163fe3c4fd81dde36 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Mon, 25 Mar 2024 12:14:41 -0400 Subject: [PATCH 11/36] piv cac --- .../users/piv_cac_recommended_controller.rb | 1 + .../piv_cac_recommended_controller_spec.rb | 65 ++++++++++++++++--- 2 files changed, 57 insertions(+), 9 deletions(-) diff --git a/app/controllers/users/piv_cac_recommended_controller.rb b/app/controllers/users/piv_cac_recommended_controller.rb index 4119a63faea..fd6aefe92ba 100644 --- a/app/controllers/users/piv_cac_recommended_controller.rb +++ b/app/controllers/users/piv_cac_recommended_controller.rb @@ -34,6 +34,7 @@ def skip private def user_email_is_gov_or_mil? + binding.pry redirect_to after_sign_in_path_for(current_user) unless current_user.has_gov_or_mil_email? end diff --git a/spec/controllers/users/piv_cac_recommended_controller_spec.rb b/spec/controllers/users/piv_cac_recommended_controller_spec.rb index 08445b007f3..33d4d24a359 100644 --- a/spec/controllers/users/piv_cac_recommended_controller_spec.rb +++ b/spec/controllers/users/piv_cac_recommended_controller_spec.rb @@ -1,18 +1,65 @@ require 'rails_helper' RSpec.describe Users::PivCacRecommendedController do - before do - user = build(:user) - stub_sign_in_before_2fa(user) - stub_analytics - end + + describe 'New user' do + let(:user) { create(:user, :fully_registered) } + before do + stub_sign_in_before_2fa(user) + stub_analytics + end - context '#show' do - end + context '#show' do + context 'with user with gov email' do + it 'should render with .gov content ' do + get :show + + expect(assigns(:email_type)).to eq('.gov') + expect(response.status).to eq 200 + end + end + + context 'with user with mil email' do + let(:user) { build(:user, email: 'test@test.mil') } + it 'should render with .gov content ' do + get :show + + expect(assigns(:email_type)).to eq('.mil') + expect(response.status).to eq 200 + end + end + + context 'with user without proper email' do + let(:user) { build(:user, email: 'test@test.com') } + + it 'redirects back to sign in page' do + expect(response).to redirect_to(authentication_methods_setup_path) + end + end + end + + context '#confirm' do + end + + context '#skip' do + end - context '#confirm' do end - context '#skip' do + describe 'Sign in flow' do + before do + stub_analytics + user = create(:user, :fully_registered, :with_phone, with: { phone: '703-555-1212' }) + stub_sign_in(user) + end + + context '#show' do + end + + context '#confirm' do + end + + context '#skip' do + end end end From 8fe6af8643be70b010670200de60c8efbb7fb080 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Tue, 26 Mar 2024 10:49:11 -0400 Subject: [PATCH 12/36] Add piv cac recommended presenter and data --- app/controllers/application_controller.rb | 2 +- app/controllers/concerns/mfa_setup_concern.rb | 7 +- .../users/piv_cac_recommended_controller.rb | 43 +++----- .../piv_cac_recommended_presenter.rb | 32 ++++++ app/services/analytics_events.rb | 15 ++- .../users/piv_cac_recommended/show.html.erb | 4 +- config/locales/titles/es.yml | 1 + config/locales/titles/fr.yml | 1 + .../locales/two_factor_authentication/en.yml | 9 +- .../locales/two_factor_authentication/fr.yml | 9 ++ ...0240321121322_add_pivcac_notice_on_user.rb | 2 +- db/schema.rb | 2 +- .../piv_cac_recommended_controller_spec.rb | 101 ++++++++++++------ .../piv_cac_recommended_presenter_spec.rb | 93 ++++++++++++++++ 14 files changed, 244 insertions(+), 77 deletions(-) create mode 100644 app/presenters/piv_cac_recommended_presenter.rb create mode 100644 spec/presenters/piv_cac_recommended_presenter_spec.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 1c41ba61ec6..ae72e462969 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -263,7 +263,7 @@ def user_needs_to_reactivate_account? end def user_recommended_for_piv_cac? - current_user.has_gov_or_mil_email? && !current_user.piv_cac_recommended_dismissed + current_user.has_gov_or_mil_email? && current_user.piv_cac_recommended_visited_at.nil? end def pending_profile_newer_than_password_reset_profile? diff --git a/app/controllers/concerns/mfa_setup_concern.rb b/app/controllers/concerns/mfa_setup_concern.rb index 2f5e631c3bc..18e57877001 100644 --- a/app/controllers/concerns/mfa_setup_concern.rb +++ b/app/controllers/concerns/mfa_setup_concern.rb @@ -66,13 +66,18 @@ def mfa_selection_index user_session[:mfa_selection_index] || 0 end + def set_piv_cac_as_option_and_redirect + user_session[:mfa_selections] = ['piv_cac'] + redirect_to confirmation_path(user_session[:mfa_selections].first) + end + def show_skip_additional_mfa_link? !(mfa_context.enabled_mfa_methods_count == 1 && mfa_context.webauthn_platform_configurations.count == 1) end def check_if_possible_piv_user - if current_user.has_gov_or_mil_email? && !current_user.piv_cac_recommended_dismissed + if current_user.has_gov_or_mil_email? && current_user.piv_cac_recommended_visited_at.nil? redirect_to login_piv_cac_recommended_path end end diff --git a/app/controllers/users/piv_cac_recommended_controller.rb b/app/controllers/users/piv_cac_recommended_controller.rb index fd6aefe92ba..f464d257836 100644 --- a/app/controllers/users/piv_cac_recommended_controller.rb +++ b/app/controllers/users/piv_cac_recommended_controller.rb @@ -10,50 +10,33 @@ class PivCacRecommendedController < ApplicationController before_action :apply_secure_headers_override before_action :user_email_is_gov_or_mil? - helper_method :in_multi_mfa_selection_flow? - def show - @email_type = email_type - @skip_text = skip_text - analytics.piv_cac_recommended_page_visited + @recommended_presenter = PivCacRecommendedPresenter.new(current_user) + analytics.piv_cac_recommended_visited end def confirm - UpdateUser.new(user: current_user, attributes: { piv_cac_recommended_dismissed: true }).call - user_session[:mfa_selections] = ['piv_cac'] - analytics.piv_cac_recommended_accepted - redirect_to confirmation_path(user_session[:mfa_selections].first) + UpdateUser.new( + user: current_user, + attributes: { piv_cac_recommended_visited_at: Time.zone.now }, + ).call + analytics.piv_cac_recommended(action: :accepted) + set_piv_cac_as_option_and_redirect end def skip - UpdateUser.new(user: current_user, attributes: { piv_cac_recommended_dismissed: true }).call - analytics.piv_cac_recommended_skipped + UpdateUser.new( + user: current_user, + attributes: { piv_cac_recommended_visited_at: Time.zone.now }, + ).call + analytics.piv_cac_recommended(action: :skipped) redirect_to after_sign_in_path_for(current_user) end private def user_email_is_gov_or_mil? - binding.pry redirect_to after_sign_in_path_for(current_user) unless current_user.has_gov_or_mil_email? end - - def email_type - address = current_user.confirmed_email_addresses.select { |address| address.gov_or_mil? } - case address.first.email.end_with?('.gov') - when true - '.gov' - else - '.mil' - end - end - - def skip_text - if MfaPolicy.new(current_user).two_factor_enabled? - t('mfa.skip') - else - t('two_factor_authentication.piv_cac_upsell.choose_other_method') - end - end end end diff --git a/app/presenters/piv_cac_recommended_presenter.rb b/app/presenters/piv_cac_recommended_presenter.rb new file mode 100644 index 00000000000..7a75e481378 --- /dev/null +++ b/app/presenters/piv_cac_recommended_presenter.rb @@ -0,0 +1,32 @@ +class PivCacRecommendedPresenter + attr_accessor :user + def initialize(user) + @user = user + end + + def info + if MfaPolicy.new(user).two_factor_enabled? + I18n.t('two_factor_authentication.piv_cac_upsell.existing_user_info', email_type: email_type) + else + I18n.t('two_factor_authentication.piv_cac_upsell.new_user_info', email_type: email_type) + end + end + + def email_type + address = user.confirmed_email_addresses.select { |address| address.gov_or_mil? } + case address.first.email.end_with?('.gov') + when true + '.gov' + else + '.mil' + end + end + + def skip_text + if MfaPolicy.new(user).two_factor_enabled? + I18n.t('mfa.skip') + else + I18n.t('two_factor_authentication.piv_cac_upsell.choose_other_method') + end + end + end diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index 58d031e41a9..7823728e997 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -4116,16 +4116,15 @@ def piv_cac_update_name_submitted( ) end - def piv_cac_recommended_page_visited - track_event(:piv_cac_recommended_page_visited) + # Tracks when user visits piv cac recommended + def piv_cac_recommended_visited + track_event(:piv_cac_recommended_visited) end - def piv_cac_recommended_accepted - track_event(:piv_cac_recommended_accepted) - end - - def piv_cac_recommended_skipped - track_event(:piv_cac_recommended_skipped) + # @param [String] action what action user made + # Tracks when user submits an action on Piv Cac recommended page + def piv_cac_recommended(action: nil) + track_event(:piv_cac_recommended, action: action) end # @param [String] redirect_url URL user was directed to diff --git a/app/views/users/piv_cac_recommended/show.html.erb b/app/views/users/piv_cac_recommended/show.html.erb index 7c446728cbc..2e1f2b73c44 100644 --- a/app/views/users/piv_cac_recommended/show.html.erb +++ b/app/views/users/piv_cac_recommended/show.html.erb @@ -5,7 +5,7 @@ <%= render PageHeadingComponent.new.with_content(t('titles.piv_cac_setup.upsell')) %>

- <%= t('two_factor_authentication.piv_cac_upsell.info', email_type: @email_type) %> + <%= @recommended_presenter.info %>

<%= render ButtonComponent.new( @@ -30,5 +30,5 @@ end, big: true, class: 'margin-bottom-2', - ).with_content(@skip_text) %> + ).with_content(@recommended_presenter.skip_text) %> diff --git a/config/locales/titles/es.yml b/config/locales/titles/es.yml index 6d2268a9d0c..63626cf7b93 100644 --- a/config/locales/titles/es.yml +++ b/config/locales/titles/es.yml @@ -59,6 +59,7 @@ es: new: Use su PIV / CAC para iniciar sesión en su cuenta piv_cac_setup: new: Use su tarjeta PIV/CAC para asegurar su cuenta + upsell: Aumente la seguridad de su cuenta con una identificación de empleado del gobierno. present_piv_cac: Presenta tu PIV/CAC present_webauthn: Conecte su clave de seguridad de hardware reactivate_account: Reactive su cuenta diff --git a/config/locales/titles/fr.yml b/config/locales/titles/fr.yml index c79bac9bf3b..667b37cddd0 100644 --- a/config/locales/titles/fr.yml +++ b/config/locales/titles/fr.yml @@ -59,6 +59,7 @@ fr: new: Utilisez votre PIV / CAC pour vous connecter à votre compte piv_cac_setup: new: Utilisez votre carte PIV/CAC pour sécuriser votre compte + upsell: Renforcer la sécurité de votre compte avec une carte d’employé fédéral present_piv_cac: Veuillez présenter votre carte PIV/CAC present_webauthn: Branchez votre clé de sécurité physique reactivate_account: Réactiver le profil diff --git a/config/locales/two_factor_authentication/en.yml b/config/locales/two_factor_authentication/en.yml index 54f9d13f416..886f346e456 100644 --- a/config/locales/two_factor_authentication/en.yml +++ b/config/locales/two_factor_authentication/en.yml @@ -149,9 +149,12 @@ en: piv_cac_upsell: add_piv: Add PIV/CAC card choose_other_method: Choose other methods instead - info: Because you are using a %{email_type} email, we recommend you add your government - employee ID as one of your authentication methods. This adds another - layer of security to your account. + existing_user_info: Because you are using a %{email_type} email, we recommend + you add your government employee ID as one of your authentication + methods. This will greatly enhance your account security. + new_user_info: Because you are using a %{email_type} email, we recommend you add + your government employee ID as one of your authentication methods. This + adds another layer of security to your account. please_try_again_html: Please try again in %{countdown}. read_about_two_factor_authentication: Read about two-factor authentication recaptcha: diff --git a/config/locales/two_factor_authentication/fr.yml b/config/locales/two_factor_authentication/fr.yml index 2c6de307f79..07c273c022f 100644 --- a/config/locales/two_factor_authentication/fr.yml +++ b/config/locales/two_factor_authentication/fr.yml @@ -161,6 +161,15 @@ fr: 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 + piv_cac_upsell: + add_piv: Add PIV/CAC card + choose_other_method: Choisir plutôt d’autres méthodes + existing_user_info: Because you are using a %{email_type} email, we recommend + you add your government employee ID as one of your authentication + methods. This will greatly enhance your account security. + new_user_info: Because you are using a %{email_type} email, we recommend you add + your government employee ID as one of your authentication methods. This + adds another layer of security to your account. please_try_again_html: Veuillez essayer de nouveau dans %{countdown}. read_about_two_factor_authentication: En savoir plus sur l’authentification à deux facteurs recaptcha: diff --git a/db/primary_migrate/20240321121322_add_pivcac_notice_on_user.rb b/db/primary_migrate/20240321121322_add_pivcac_notice_on_user.rb index cb26fd1aafd..3870749c052 100644 --- a/db/primary_migrate/20240321121322_add_pivcac_notice_on_user.rb +++ b/db/primary_migrate/20240321121322_add_pivcac_notice_on_user.rb @@ -1,5 +1,5 @@ class AddPivcacNoticeOnUser < ActiveRecord::Migration[7.1] def change - add_column :users, :piv_cac_recommended_dismissed, :boolean, default: false + add_column :users, :piv_cac_recommended_visited_at, :datetime, default: nil end end diff --git a/db/schema.rb b/db/schema.rb index da76613e4ef..42565bd663e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -612,7 +612,7 @@ t.string "encrypted_password_digest_multi_region" t.string "encrypted_recovery_code_digest_multi_region" t.datetime "second_mfa_reminder_dismissed_at" - t.boolean "piv_cac_recommended_dismissed", default: false + t.datetime "piv_cac_recommended_visited_at" t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true t.index ["uuid"], name: "index_users_on_uuid", unique: true end diff --git a/spec/controllers/users/piv_cac_recommended_controller_spec.rb b/spec/controllers/users/piv_cac_recommended_controller_spec.rb index 33d4d24a359..f326f998e96 100644 --- a/spec/controllers/users/piv_cac_recommended_controller_spec.rb +++ b/spec/controllers/users/piv_cac_recommended_controller_spec.rb @@ -1,65 +1,106 @@ require 'rails_helper' RSpec.describe Users::PivCacRecommendedController do - describe 'New user' do - let(:user) { create(:user, :fully_registered) } + let(:user) { create(:user, email: 'example@example.gov') } before do stub_sign_in_before_2fa(user) stub_analytics + controller.user_session[:in_account_creation_flow] = true end context '#show' do - context 'with user with gov email' do - it 'should render with .gov content ' do - get :show + context 'with user without proper email' do + let(:user) { create(:user, email: 'example@example.com') } - expect(assigns(:email_type)).to eq('.gov') - expect(response.status).to eq 200 + it 'redirects back to sign in path page' do + get :show + expect(response).to redirect_to(account_path) end end + end - context 'with user with mil email' do - let(:user) { build(:user, email: 'test@test.mil') } - it 'should render with .gov content ' do - get :show + it 'logs analytic event' do + get :show - expect(assigns(:email_type)).to eq('.mil') - expect(response.status).to eq 200 - end - end + expect(@analytics).to have_logged_event(:piv_cac_recommended_visited) + end + end + + describe 'Sign in flow' do + let(:user) { create(:user, :with_phone, { email: 'example@example.gov' }) } + before do + stub_analytics + + stub_sign_in(user) + user.reload + end + context '#show' do context 'with user without proper email' do - let(:user) { build(:user, email: 'test@test.com') } + let(:user) { create(:user, :with_phone, { email: 'example@example.com' }) } - it 'redirects back to sign in page' do - expect(response).to redirect_to(authentication_methods_setup_path) + it 'redirects back to account page' do + get :show + expect(response).to redirect_to(account_path) end end end - - context '#confirm' do + end + + context '#confirm' do + let(:user) { create(:user, email: 'example@example.gov') } + before do + stub_sign_in_before_2fa(user) + stub_analytics + controller.user_session[:in_account_creation_flow] = true + end + + it 'directs user to piv cac page' do + post :confirm + + expect(response).to redirect_to(setup_piv_cac_path) end - - context '#skip' do + + it 'sets piv_cac recommended as set' do + post :confirm + + user.reload + expect(user.piv_cac_recommended_visited_at).to be_truthy end + it 'logs analytics' do + post :confirm + + expect(@analytics).to have_logged_event(:piv_cac_recommended, action: :accepted) + end end - describe 'Sign in flow' do + context '#skip' do + let(:user) { create(:user, email: 'example@example.gov') } before do + stub_sign_in_before_2fa(user) stub_analytics - user = create(:user, :fully_registered, :with_phone, with: { phone: '703-555-1212' }) - stub_sign_in(user) + controller.user_session[:in_account_creation_flow] = true end - context '#show' do + it 'directs user to after set up page' do + post :skip + + expect(response).to redirect_to(account_path) end - - context '#confirm' do + + it 'sets piv_cac recommended as set' do + post :skip + + user.reload + expect(user.piv_cac_recommended_visited_at).to be_truthy end - - context '#skip' do + + it 'logs analytics' do + post :skip + + expect(@analytics).to have_logged_event(:piv_cac_recommended, action: :skipped) end end end diff --git a/spec/presenters/piv_cac_recommended_presenter_spec.rb b/spec/presenters/piv_cac_recommended_presenter_spec.rb new file mode 100644 index 00000000000..88e26f55f78 --- /dev/null +++ b/spec/presenters/piv_cac_recommended_presenter_spec.rb @@ -0,0 +1,93 @@ +require 'rails_helper' + +RSpec.describe PivCacRecommendedPresenter do + let(:user) { create(:user, email: 'example@example.gov') } + let(:presenter) { described_class.new(user) } + + describe '#info' do + context 'when is a gov email' do + context 'when existing user' do + let(:user) { create(:user, :with_phone, { email: 'example@example.gov' }) } + it 'should match existing user .gov text' do + expect(presenter.info).to eq( + t( + 'two_factor_authentication.piv_cac_upsell.existing_user_info', + email_type: '.gov', + ), + ) + end + end + + context 'when user has no mfa methods' do + let(:user) { create(:user, email: 'example@example.gov') } + it 'should match new user .gov text' do + expect(presenter.info).to eq( + t( + 'two_factor_authentication.piv_cac_upsell.new_user_info', + email_type: '.gov', + ), + ) + end + end + end + + context 'when user has a .mil email' do + context 'when existing user' do + let(:user) { create(:user, :with_phone, { email: 'example@example.mil' }) } + it 'should match existing user .mil text' do + expect(presenter.info).to eq( + t( + 'two_factor_authentication.piv_cac_upsell.existing_user_info', + email_type: '.mil', + ), + ) + end + end + + context 'when user has no mfa methods yet' do + let(:user) { create(:user, email: 'example@example.mil') } + it 'should match new user .mil text' do + expect(presenter.info).to eq( + t( + 'two_factor_authentication.piv_cac_upsell.new_user_info', + email_type: '.mil', + ), + ) + end + end + end + end + + describe '#email_type' do + context 'when is gov email' do + it 'should return .gov' do + expect(presenter.email_type).to eq('.gov') + end + end + + context 'when .mil email' do + let(:user) { create(:user, email: 'example@example.mil') } + it 'should return .mil' do + expect(presenter.email_type).to eq('.mil') + end + end + end + + describe '#skip_type' do + context 'when existing user' do + let(:user) { create(:user, :with_phone, { email: 'example@example.mil' }) } + it 'should return skip text' do + expect(presenter.skip_text).to eq(t('mfa.skip')) + end + end + + context 'when user has no mfa methods yet' do + let(:user) { create(:user, email: 'example@example.mil') } + it 'should return choose another method text' do + expect(presenter.skip_text).to eq( + t('two_factor_authentication.piv_cac_upsell.choose_other_method'), + ) + end + end + end +end From 2c4bf1e975038872e33dbe69d93c54d7e505f266 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Tue, 26 Mar 2024 12:09:19 -0400 Subject: [PATCH 13/36] update translations --- config/locales/titles/es.yml | 3 ++- config/locales/two_factor_authentication/es.yml | 11 +++++++++++ config/locales/two_factor_authentication/fr.yml | 2 +- spec/features/users/sign_in_spec.rb | 10 ++++++++++ 4 files changed, 24 insertions(+), 2 deletions(-) diff --git a/config/locales/titles/es.yml b/config/locales/titles/es.yml index 63626cf7b93..0b1b30547b6 100644 --- a/config/locales/titles/es.yml +++ b/config/locales/titles/es.yml @@ -59,7 +59,8 @@ es: new: Use su PIV / CAC para iniciar sesión en su cuenta piv_cac_setup: new: Use su tarjeta PIV/CAC para asegurar su cuenta - upsell: Aumente la seguridad de su cuenta con una identificación de empleado del gobierno. + upsell: Aumente la seguridad de su cuenta con una identificación de empleado del + gobierno. present_piv_cac: Presenta tu PIV/CAC present_webauthn: Conecte su clave de seguridad de hardware reactivate_account: Reactive su cuenta diff --git a/config/locales/two_factor_authentication/es.yml b/config/locales/two_factor_authentication/es.yml index 6b9e368f708..12ed4036d75 100644 --- a/config/locales/two_factor_authentication/es.yml +++ b/config/locales/two_factor_authentication/es.yml @@ -153,6 +153,17 @@ es: nickname: Apodo renamed: Se ha cambiado correctamente el nombre de su método PIV/CAC piv_cac_header_text: Presenta tu PIV/CAC + piv_cac_upsell: + add_piv: Agregar tarjeta PIV/CAC + choose_other_method: Elegir otros métodos + existing_user_info: Dado que utiliza un correo electrónico %{email_type}, le + recomendamos que añada su identificación de empleado del gobierno como + uno de sus métodos de autenticación. Esto aumentará en gran medida la + seguridad de su cuenta. + new_user_info: Dado que utiliza un correo electrónico %{email_type}, le + recomendamos que añada su identificación de empleado del gobierno como + uno de sus métodos de autenticación. Esto agrega un nivel adicional de + seguridad a su cuenta. please_try_again_html: Inténtelo de nuevo en %{countdown}. read_about_two_factor_authentication: Conozca la autenticación de dos factores recaptcha: diff --git a/config/locales/two_factor_authentication/fr.yml b/config/locales/two_factor_authentication/fr.yml index 07c273c022f..de06b46ad4a 100644 --- a/config/locales/two_factor_authentication/fr.yml +++ b/config/locales/two_factor_authentication/fr.yml @@ -162,7 +162,7 @@ fr: renamed: Votre méthode PIV/CAC a été renommée avec succès piv_cac_header_text: Veuillez présenter votre carte PIV/CAC piv_cac_upsell: - add_piv: Add PIV/CAC card + add_piv: Ajouter une carte PIV/CAC choose_other_method: Choisir plutôt d’autres méthodes existing_user_info: Because you are using a %{email_type} email, we recommend you add your government employee ID as one of your authentication diff --git a/spec/features/users/sign_in_spec.rb b/spec/features/users/sign_in_spec.rb index c5c8251ecae..ac729553d4b 100644 --- a/spec/features/users/sign_in_spec.rb +++ b/spec/features/users/sign_in_spec.rb @@ -162,6 +162,16 @@ to(include(expected_form_action)) end + scenario 'User with gov/mil email and is signing in without having visited piv recommended page ever' do + + + end + + scenario 'User has Piv/Cac' do + + + end + scenario 'user attempts sign in with piv/cac with no account then creates account' do visit_idp_from_sp_with_ial1(:oidc) click_on t('account.login.piv_cac') From c68154796aaec33facaedc3602ae3bf28c5ada22 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Tue, 26 Mar 2024 13:44:10 -0400 Subject: [PATCH 14/36] add spec and stub piv cac --- app/controllers/application_controller.rb | 9 ++++- .../piv_cac_recommended_presenter.rb | 2 +- .../users/piv_cac_recommended/show.html.erb | 40 +++++++++---------- .../locales/two_factor_authentication/en.yml | 1 + .../locales/two_factor_authentication/es.yml | 1 + .../locales/two_factor_authentication/fr.yml | 1 + ...or_authentication_setup_controller_spec.rb | 2 +- spec/features/users/sign_in_spec.rb | 24 ++++++++--- spec/features/users/sign_up_spec.rb | 2 +- .../piv_cac_recommended_presenter_spec.rb | 4 +- 10 files changed, 54 insertions(+), 32 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index ae72e462969..c1053c6a7e0 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -234,7 +234,7 @@ def after_sign_in_path_for(_user) return fix_broken_personal_key_url if current_user.broken_personal_key? return user_session.delete(:stored_location) if user_session.key?(:stored_location) return reactivate_account_url if user_needs_to_reactivate_account? - return piv_cac_recommended_path if user_recommended_for_piv_cac? + return login_piv_cac_recommended_path if user_recommended_for_piv_cac? return second_mfa_reminder_url if user_needs_second_mfa_reminder? return sp_session_request_url_with_updated_params if sp_session.key?(:request_url) signed_in_url @@ -263,7 +263,12 @@ def user_needs_to_reactivate_account? end def user_recommended_for_piv_cac? - current_user.has_gov_or_mil_email? && current_user.piv_cac_recommended_visited_at.nil? + current_user.has_gov_or_mil_email? && current_user.piv_cac_recommended_visited_at.nil? && + !user_already_has_piv? + end + + def user_already_has_piv? + MfaContext.new(current_user).piv_cac_configurations.present? end def pending_profile_newer_than_password_reset_profile? diff --git a/app/presenters/piv_cac_recommended_presenter.rb b/app/presenters/piv_cac_recommended_presenter.rb index 7a75e481378..5ccc0278963 100644 --- a/app/presenters/piv_cac_recommended_presenter.rb +++ b/app/presenters/piv_cac_recommended_presenter.rb @@ -24,7 +24,7 @@ def email_type def skip_text if MfaPolicy.new(user).two_factor_enabled? - I18n.t('mfa.skip') + I18n.t('two_factor_authentication.piv_cac_upsell.skip') else I18n.t('two_factor_authentication.piv_cac_upsell.choose_other_method') end diff --git a/app/views/users/piv_cac_recommended/show.html.erb b/app/views/users/piv_cac_recommended/show.html.erb index 2e1f2b73c44..abd2a85d40e 100644 --- a/app/views/users/piv_cac_recommended/show.html.erb +++ b/app/views/users/piv_cac_recommended/show.html.erb @@ -9,26 +9,26 @@

<%= render ButtonComponent.new( - action: ->(**tag_options, &block) do - button_to( - login_piv_cac_recommended_add_path, - **tag_options, - &block - ) - end, - big: true, - class: 'margin-bottom-3', - ).with_content(t('two_factor_authentication.piv_cac_upsell.add_piv')) %> + action: ->(**tag_options, &block) do + button_to( + login_piv_cac_recommended_add_path, + **tag_options, + &block + ) + end, + big: true, + class: 'margin-bottom-3', +).with_content(t('two_factor_authentication.piv_cac_upsell.add_piv')) %> <%= render ButtonComponent.new( - action: ->(**tag_options, &block) do - button_to( - login_piv_cac_recommended_skip_path, - **tag_options, - &block - ) - end, - big: true, - class: 'margin-bottom-2', - ).with_content(@recommended_presenter.skip_text) %> + action: ->(**tag_options, &block) do + button_to( + login_piv_cac_recommended_skip_path, + **tag_options, + &block + ) + end, + outline: true, + class: 'usa-button usa-button--unstyled', +).with_content(@recommended_presenter.skip_text) %> diff --git a/config/locales/two_factor_authentication/en.yml b/config/locales/two_factor_authentication/en.yml index 886f346e456..ed1588b554d 100644 --- a/config/locales/two_factor_authentication/en.yml +++ b/config/locales/two_factor_authentication/en.yml @@ -155,6 +155,7 @@ en: new_user_info: Because you are using a %{email_type} email, we recommend you add your government employee ID as one of your authentication methods. This adds another layer of security to your account. + skip: Skip please_try_again_html: Please try again in %{countdown}. read_about_two_factor_authentication: Read about two-factor authentication recaptcha: diff --git a/config/locales/two_factor_authentication/es.yml b/config/locales/two_factor_authentication/es.yml index 12ed4036d75..ffd4f5d0c6b 100644 --- a/config/locales/two_factor_authentication/es.yml +++ b/config/locales/two_factor_authentication/es.yml @@ -164,6 +164,7 @@ es: recomendamos que añada su identificación de empleado del gobierno como uno de sus métodos de autenticación. Esto agrega un nivel adicional de seguridad a su cuenta. + skip: Omitir please_try_again_html: Inténtelo de nuevo en %{countdown}. read_about_two_factor_authentication: Conozca la autenticación de dos factores recaptcha: diff --git a/config/locales/two_factor_authentication/fr.yml b/config/locales/two_factor_authentication/fr.yml index de06b46ad4a..db925200487 100644 --- a/config/locales/two_factor_authentication/fr.yml +++ b/config/locales/two_factor_authentication/fr.yml @@ -170,6 +170,7 @@ fr: new_user_info: Because you are using a %{email_type} email, we recommend you add your government employee ID as one of your authentication methods. This adds another layer of security to your account. + skip: Ignorer please_try_again_html: Veuillez essayer de nouveau dans %{countdown}. read_about_two_factor_authentication: En savoir plus sur l’authentification à deux facteurs recaptcha: diff --git a/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb b/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb index 7d4fc7a4971..15411b1af07 100644 --- a/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb +++ b/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb @@ -23,7 +23,7 @@ end context 'with user having gov or mil email' do - let(:user) { create(:user, email: 'example@example.gov') } + let(:user) { create(:user, email: 'example@example.gov', piv_cac_recommended_visited_at: Time.zone.now) } it 'tracks the visit in analytics' do get :index diff --git a/spec/features/users/sign_in_spec.rb b/spec/features/users/sign_in_spec.rb index ac729553d4b..5324e2614a3 100644 --- a/spec/features/users/sign_in_spec.rb +++ b/spec/features/users/sign_in_spec.rb @@ -162,14 +162,28 @@ to(include(expected_form_action)) end - scenario 'User with gov/mil email and is signing in without having visited piv recommended page ever' do - + scenario 'User with gov/mil email directed to recommended PIV page' do + user = create(:user, :with_phone, { email: 'example@example.gov' }) + visit new_user_session_path + fill_in_credentials_and_submit(user.email, user.password) + fill_in_code_with_last_phone_otp + click_submit_default + expect(page).to have_current_path(login_piv_cac_recommended_path) + click_button(t('two_factor_authentication.piv_cac_upsell.add_piv')) + expect(page).to have_current_path(setup_piv_cac_path) end - scenario 'User has Piv/Cac' do - - + scenario 'User with gov/mil email and skips recommendation page' do + user = create(:user, :with_phone, { email: 'example@example.gov' }) + + visit new_user_session_path + fill_in_credentials_and_submit(user.email, user.password) + fill_in_code_with_last_phone_otp + click_submit_default + expect(page).to have_current_path(login_piv_cac_recommended_path) + click_button(t('mfa.skip')) + expect(page).to have_current_path(account_path) end scenario 'user attempts sign in with piv/cac with no account then creates account' do diff --git a/spec/features/users/sign_up_spec.rb b/spec/features/users/sign_up_spec.rb index a20e10805fa..a645727b255 100644 --- a/spec/features/users/sign_up_spec.rb +++ b/spec/features/users/sign_up_spec.rb @@ -526,7 +526,7 @@ def clipboard_text end context 'user can skip piv cac prompt' do - it 'should skip piv cac prompt and land of mfa screen' do + it 'should skip piv cac prompt and land on mfa screen' do expect(current_path).to eq login_piv_cac_recommended_path click_button t('two_factor_authentication.piv_cac_upsell.choose_other_method') diff --git a/spec/presenters/piv_cac_recommended_presenter_spec.rb b/spec/presenters/piv_cac_recommended_presenter_spec.rb index 88e26f55f78..645f0d5badc 100644 --- a/spec/presenters/piv_cac_recommended_presenter_spec.rb +++ b/spec/presenters/piv_cac_recommended_presenter_spec.rb @@ -66,7 +66,7 @@ end context 'when .mil email' do - let(:user) { create(:user, email: 'example@example.mil') } + let(:user) { create(:user, email: 'example@example.mil') } it 'should return .mil' do expect(presenter.email_type).to eq('.mil') end @@ -77,7 +77,7 @@ context 'when existing user' do let(:user) { create(:user, :with_phone, { email: 'example@example.mil' }) } it 'should return skip text' do - expect(presenter.skip_text).to eq(t('mfa.skip')) + expect(presenter.skip_text).to eq(t('two_factor_authentication.piv_cac_upsell.skip')) end end From 15320466f88b0d821a35af67b61bbfceceb0c867 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Tue, 26 Mar 2024 13:48:36 -0400 Subject: [PATCH 15/36] piv cac recommended show --- .../users/piv_cac_recommended/show.html.erb | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/app/views/users/piv_cac_recommended/show.html.erb b/app/views/users/piv_cac_recommended/show.html.erb index abd2a85d40e..34c7f19cf64 100644 --- a/app/views/users/piv_cac_recommended/show.html.erb +++ b/app/views/users/piv_cac_recommended/show.html.erb @@ -9,26 +9,26 @@

<%= render ButtonComponent.new( - action: ->(**tag_options, &block) do - button_to( - login_piv_cac_recommended_add_path, - **tag_options, - &block - ) - end, - big: true, - class: 'margin-bottom-3', -).with_content(t('two_factor_authentication.piv_cac_upsell.add_piv')) %> + action: ->(**tag_options, &block) do + button_to( + login_piv_cac_recommended_add_path, + **tag_options, + &block + ) + end, + big: true, + class: 'margin-bottom-3', + ).with_content(t('two_factor_authentication.piv_cac_upsell.add_piv')) %> <%= render ButtonComponent.new( - action: ->(**tag_options, &block) do - button_to( - login_piv_cac_recommended_skip_path, - **tag_options, - &block - ) - end, - outline: true, - class: 'usa-button usa-button--unstyled', -).with_content(@recommended_presenter.skip_text) %> + action: ->(**tag_options, &block) do + button_to( + login_piv_cac_recommended_skip_path, + **tag_options, + &block + ) + end, + outline: true, + class: 'usa-button usa-button--unstyled', + ).with_content(@recommended_presenter.skip_text) %> From 0788dbf0548f13f81e38b70f6808bdc67d940a34 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Tue, 26 Mar 2024 13:52:11 -0400 Subject: [PATCH 16/36] refactor mfa selection concern --- app/controllers/concerns/mfa_setup_concern.rb | 5 ++--- app/controllers/users/piv_cac_recommended_controller.rb | 3 ++- .../users/two_factor_authentication_setup_controller_spec.rb | 4 +++- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/app/controllers/concerns/mfa_setup_concern.rb b/app/controllers/concerns/mfa_setup_concern.rb index 18e57877001..5747fde02b8 100644 --- a/app/controllers/concerns/mfa_setup_concern.rb +++ b/app/controllers/concerns/mfa_setup_concern.rb @@ -66,9 +66,8 @@ def mfa_selection_index user_session[:mfa_selection_index] || 0 end - def set_piv_cac_as_option_and_redirect - user_session[:mfa_selections] = ['piv_cac'] - redirect_to confirmation_path(user_session[:mfa_selections].first) + def set_mfa_selections(selections) + user_session[:mfa_selections] = selections end def show_skip_additional_mfa_link? diff --git a/app/controllers/users/piv_cac_recommended_controller.rb b/app/controllers/users/piv_cac_recommended_controller.rb index f464d257836..516ac030a41 100644 --- a/app/controllers/users/piv_cac_recommended_controller.rb +++ b/app/controllers/users/piv_cac_recommended_controller.rb @@ -21,7 +21,8 @@ def confirm attributes: { piv_cac_recommended_visited_at: Time.zone.now }, ).call analytics.piv_cac_recommended(action: :accepted) - set_piv_cac_as_option_and_redirect + set_mfa_selections(['piv_cac']) + redirect_to confirmation_path(user_session[:mfa_selections].first) end def skip diff --git a/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb b/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb index 15411b1af07..aed8116e9d3 100644 --- a/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb +++ b/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb @@ -23,7 +23,9 @@ end context 'with user having gov or mil email' do - let(:user) { create(:user, email: 'example@example.gov', piv_cac_recommended_visited_at: Time.zone.now) } + let(:user) do + create(:user, email: 'example@example.gov', piv_cac_recommended_visited_at: Time.zone.now) + end it 'tracks the visit in analytics' do get :index From 4512e24afdf27d3cd5083dc54a9bf7b17732517f Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Tue, 26 Mar 2024 14:19:03 -0400 Subject: [PATCH 17/36] User-Facing Improvements, Piv/Cac, add Migration to add piv visited at column --- ...0326181600_add_piv_cac_recommended_visited_at_on_user.rb | 5 +++++ db/schema.rb | 6 +++--- 2 files changed, 8 insertions(+), 3 deletions(-) create mode 100644 db/primary_migrate/20240326181600_add_piv_cac_recommended_visited_at_on_user.rb diff --git a/db/primary_migrate/20240326181600_add_piv_cac_recommended_visited_at_on_user.rb b/db/primary_migrate/20240326181600_add_piv_cac_recommended_visited_at_on_user.rb new file mode 100644 index 00000000000..96007bc5475 --- /dev/null +++ b/db/primary_migrate/20240326181600_add_piv_cac_recommended_visited_at_on_user.rb @@ -0,0 +1,5 @@ +class AddPivCacRecommendedVisitedAtOnUser < ActiveRecord::Migration[7.1] + def change + add_column :users, :piv_cac_recommended_visited_at, :datetime, default: nil + end +end diff --git a/db/schema.rb b/db/schema.rb index 4c3a718d4d5..2a10a2a8285 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,11 +10,10 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2024_02_16_184124) do +ActiveRecord::Schema[7.1].define(version: 2024_03_26_181600) do # These are extensions that must be enabled in order to support this database enable_extension "citext" enable_extension "pg_stat_statements" - enable_extension "pgcrypto" enable_extension "plpgsql" enable_extension "postgis" @@ -612,6 +611,7 @@ t.string "encrypted_password_digest_multi_region" t.string "encrypted_recovery_code_digest_multi_region" t.datetime "second_mfa_reminder_dismissed_at" + t.datetime "piv_cac_recommended_visited_at" t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true t.index ["uuid"], name: "index_users_on_uuid", unique: true end @@ -653,7 +653,7 @@ add_foreign_key "iaa_gtcs", "partner_accounts" add_foreign_key "iaa_orders", "iaa_gtcs" add_foreign_key "in_person_enrollments", "profiles" - add_foreign_key "in_person_enrollments", "service_providers", column: "issuer", primary_key: "issuer" + add_foreign_key "in_person_enrollments", "service_providers", column: "issuer", primary_key: "issuer", validate: false add_foreign_key "in_person_enrollments", "users" add_foreign_key "integration_usages", "iaa_orders" add_foreign_key "integration_usages", "integrations" From 962a3b9a69d9b5585d4842b87ef696a41bf21189 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Tue, 26 Mar 2024 14:21:13 -0400 Subject: [PATCH 18/36] update schema --- db/schema.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/schema.rb b/db/schema.rb index 2a10a2a8285..3c7369f7814 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -653,7 +653,7 @@ add_foreign_key "iaa_gtcs", "partner_accounts" add_foreign_key "iaa_orders", "iaa_gtcs" add_foreign_key "in_person_enrollments", "profiles" - add_foreign_key "in_person_enrollments", "service_providers", column: "issuer", primary_key: "issuer", validate: false + add_foreign_key "in_person_enrollments", "service_providers", column: "issuer", primary_key: "issuer" add_foreign_key "in_person_enrollments", "users" add_foreign_key "integration_usages", "iaa_orders" add_foreign_key "integration_usages", "integrations" From 13146a66fcb461f32a15fa3b489d07a1821fffc6 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Tue, 26 Mar 2024 14:24:19 -0400 Subject: [PATCH 19/36] put back pgcrypto --- db/schema.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/db/schema.rb b/db/schema.rb index 3c7369f7814..58ec0d6650c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -14,6 +14,7 @@ # These are extensions that must be enabled in order to support this database enable_extension "citext" enable_extension "pg_stat_statements" + enable_extension "pgcrypto" enable_extension "plpgsql" enable_extension "postgis" From 1fa72f91804a937697fea75323fbc07488d0f865 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Tue, 26 Mar 2024 14:31:46 -0400 Subject: [PATCH 20/36] rubocop and analytic events --- app/services/analytics_events.rb | 7 +++++-- config/locales/two_factor_authentication/fr.yml | 14 ++++++++------ spec/features/users/sign_in_spec.rb | 2 +- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index 7823728e997..61f2987cfc5 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -4123,8 +4123,11 @@ def piv_cac_recommended_visited # @param [String] action what action user made # Tracks when user submits an action on Piv Cac recommended page - def piv_cac_recommended(action: nil) - track_event(:piv_cac_recommended, action: action) + def piv_cac_recommended(action: nil, **_exra) + track_event( + :piv_cac_recommended, + action: action, + ) end # @param [String] redirect_url URL user was directed to diff --git a/config/locales/two_factor_authentication/fr.yml b/config/locales/two_factor_authentication/fr.yml index db925200487..b4876271f24 100644 --- a/config/locales/two_factor_authentication/fr.yml +++ b/config/locales/two_factor_authentication/fr.yml @@ -164,12 +164,14 @@ fr: piv_cac_upsell: add_piv: Ajouter une carte PIV/CAC choose_other_method: Choisir plutôt d’autres méthodes - existing_user_info: Because you are using a %{email_type} email, we recommend - you add your government employee ID as one of your authentication - methods. This will greatly enhance your account security. - new_user_info: Because you are using a %{email_type} email, we recommend you add - your government employee ID as one of your authentication methods. This - adds another layer of security to your account. + existing_user_info: Comme vous utilisez une adresse e-mail avec le suffixe + %{email_type}, nous vous recommandons d’ajouter votre carte d’employé + fédéral parmi vos méthodes d’authentification. Ceci renforcera + considérablement la sécurité de votre compte. + new_user_info: Comme vous utilisez une adresse e-mail avec le suffixe + %{email_type}, nous vous recommandons d’ajouter votre carte d’employé + fédéral parmi vos méthodes d’authentification. Ceci permet d’ajouter une + couche supplémentaire de sécurité à votre compte. skip: Ignorer 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/features/users/sign_in_spec.rb b/spec/features/users/sign_in_spec.rb index 5324e2614a3..f7ff848b63d 100644 --- a/spec/features/users/sign_in_spec.rb +++ b/spec/features/users/sign_in_spec.rb @@ -182,7 +182,7 @@ fill_in_code_with_last_phone_otp click_submit_default expect(page).to have_current_path(login_piv_cac_recommended_path) - click_button(t('mfa.skip')) + click_button(t('two_factor_authentication.piv_cac_upsell.skip')) expect(page).to have_current_path(account_path) end From d234f0b1cc521039c168e8f1fd4fd8c333a50cb5 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Wed, 27 Mar 2024 08:04:38 -0400 Subject: [PATCH 21/36] changelog: User-Facing Improvements, PIV/CAC, Piv Migration for added check on user --- .../20240321121322_add_pivcac_notice_on_user.rb | 5 +++++ db/schema.rb | 3 +-- 2 files changed, 6 insertions(+), 2 deletions(-) create mode 100644 db/primary_migrate/20240321121322_add_pivcac_notice_on_user.rb diff --git a/db/primary_migrate/20240321121322_add_pivcac_notice_on_user.rb b/db/primary_migrate/20240321121322_add_pivcac_notice_on_user.rb new file mode 100644 index 00000000000..70bc48f22d7 --- /dev/null +++ b/db/primary_migrate/20240321121322_add_pivcac_notice_on_user.rb @@ -0,0 +1,5 @@ +class AddPivcacNoticeOnUser < ActiveRecord::Migration[7.1] + def change + add_column :users, :piv_cac_recommended_dismissed_at, :datetime, default: nil + end +end diff --git a/db/schema.rb b/db/schema.rb index 58ec0d6650c..2a10a2a8285 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -14,7 +14,6 @@ # These are extensions that must be enabled in order to support this database enable_extension "citext" enable_extension "pg_stat_statements" - enable_extension "pgcrypto" enable_extension "plpgsql" enable_extension "postgis" @@ -654,7 +653,7 @@ add_foreign_key "iaa_gtcs", "partner_accounts" add_foreign_key "iaa_orders", "iaa_gtcs" add_foreign_key "in_person_enrollments", "profiles" - add_foreign_key "in_person_enrollments", "service_providers", column: "issuer", primary_key: "issuer" + add_foreign_key "in_person_enrollments", "service_providers", column: "issuer", primary_key: "issuer", validate: false add_foreign_key "in_person_enrollments", "users" add_foreign_key "integration_usages", "iaa_orders" add_foreign_key "integration_usages", "integrations" From 9dd3665e9c042caa0c1bb9cb39817576da82d3d4 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Wed, 27 Mar 2024 08:11:56 -0400 Subject: [PATCH 22/36] update schema and change to dismissed at --- ...326181600_add_piv_cac_recommended_dismissed_at_on_user.rb | 5 +++++ ...40326181600_add_piv_cac_recommended_visited_at_on_user.rb | 5 ----- db/schema.rb | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) create mode 100644 db/primary_migrate/20240326181600_add_piv_cac_recommended_dismissed_at_on_user.rb delete mode 100644 db/primary_migrate/20240326181600_add_piv_cac_recommended_visited_at_on_user.rb diff --git a/db/primary_migrate/20240326181600_add_piv_cac_recommended_dismissed_at_on_user.rb b/db/primary_migrate/20240326181600_add_piv_cac_recommended_dismissed_at_on_user.rb new file mode 100644 index 00000000000..66bb8d2e264 --- /dev/null +++ b/db/primary_migrate/20240326181600_add_piv_cac_recommended_dismissed_at_on_user.rb @@ -0,0 +1,5 @@ +class AddPivCacRecommendedDismissedAtOnUser < ActiveRecord::Migration[7.1] + def change + add_column :users, :piv_cac_recommended_dismissed_at, :datetime, default: nil + end +end diff --git a/db/primary_migrate/20240326181600_add_piv_cac_recommended_visited_at_on_user.rb b/db/primary_migrate/20240326181600_add_piv_cac_recommended_visited_at_on_user.rb deleted file mode 100644 index 96007bc5475..00000000000 --- a/db/primary_migrate/20240326181600_add_piv_cac_recommended_visited_at_on_user.rb +++ /dev/null @@ -1,5 +0,0 @@ -class AddPivCacRecommendedVisitedAtOnUser < ActiveRecord::Migration[7.1] - def change - add_column :users, :piv_cac_recommended_visited_at, :datetime, default: nil - end -end diff --git a/db/schema.rb b/db/schema.rb index 2a10a2a8285..ed41b209fa0 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -611,7 +611,7 @@ t.string "encrypted_password_digest_multi_region" t.string "encrypted_recovery_code_digest_multi_region" t.datetime "second_mfa_reminder_dismissed_at" - t.datetime "piv_cac_recommended_visited_at" + t.datetime "piv_cac_recommended_dismissed_at" t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true t.index ["uuid"], name: "index_users_on_uuid", unique: true end From 762d86309f2612ccb593bc8c2bdedabf229df536 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Wed, 27 Mar 2024 08:12:47 -0400 Subject: [PATCH 23/36] remove unneeded migration --- .../20240321121322_add_pivcac_notice_on_user.rb | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 db/primary_migrate/20240321121322_add_pivcac_notice_on_user.rb diff --git a/db/primary_migrate/20240321121322_add_pivcac_notice_on_user.rb b/db/primary_migrate/20240321121322_add_pivcac_notice_on_user.rb deleted file mode 100644 index 70bc48f22d7..00000000000 --- a/db/primary_migrate/20240321121322_add_pivcac_notice_on_user.rb +++ /dev/null @@ -1,5 +0,0 @@ -class AddPivcacNoticeOnUser < ActiveRecord::Migration[7.1] - def change - add_column :users, :piv_cac_recommended_dismissed_at, :datetime, default: nil - end -end From 723e9edce813afb850d19c021bc215194f319529 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Wed, 27 Mar 2024 08:13:42 -0400 Subject: [PATCH 24/36] fix schema --- db/schema.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/db/schema.rb b/db/schema.rb index ed41b209fa0..bf030067ebe 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -14,6 +14,7 @@ # These are extensions that must be enabled in order to support this database enable_extension "citext" enable_extension "pg_stat_statements" + enable_extension "pgcrypto" enable_extension "plpgsql" enable_extension "postgis" @@ -653,7 +654,7 @@ add_foreign_key "iaa_gtcs", "partner_accounts" add_foreign_key "iaa_orders", "iaa_gtcs" add_foreign_key "in_person_enrollments", "profiles" - add_foreign_key "in_person_enrollments", "service_providers", column: "issuer", primary_key: "issuer", validate: false + add_foreign_key "in_person_enrollments", "service_providers", column: "issuer", primary_key: "issuer" add_foreign_key "in_person_enrollments", "users" add_foreign_key "integration_usages", "iaa_orders" add_foreign_key "integration_usages", "integrations" From d1690eaccc641683738a0a6d005acdf9b550c0dd Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Wed, 27 Mar 2024 08:29:27 -0400 Subject: [PATCH 25/36] address comments, piv cac recommended controller fix and reorder analytic events --- app/controllers/application_controller.rb | 2 +- app/controllers/concerns/mfa_setup_concern.rb | 2 +- .../users/piv_cac_recommended_controller.rb | 10 +++---- ..._factor_authentication_setup_controller.rb | 1 - .../piv_cac_recommended_presenter.rb | 2 +- app/services/analytics_events.rb | 29 ++++++++++--------- .../piv_cac_recommended_controller_spec.rb | 4 +-- ...or_authentication_setup_controller_spec.rb | 2 +- 8 files changed, 25 insertions(+), 27 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index c1053c6a7e0..c374c2d6d17 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -263,7 +263,7 @@ def user_needs_to_reactivate_account? end def user_recommended_for_piv_cac? - current_user.has_gov_or_mil_email? && current_user.piv_cac_recommended_visited_at.nil? && + current_user.has_gov_or_mil_email? && current_user.piv_cac_recommended_dismissed_at.nil? && !user_already_has_piv? end diff --git a/app/controllers/concerns/mfa_setup_concern.rb b/app/controllers/concerns/mfa_setup_concern.rb index 5747fde02b8..25d8a78a249 100644 --- a/app/controllers/concerns/mfa_setup_concern.rb +++ b/app/controllers/concerns/mfa_setup_concern.rb @@ -76,7 +76,7 @@ def show_skip_additional_mfa_link? end def check_if_possible_piv_user - if current_user.has_gov_or_mil_email? && current_user.piv_cac_recommended_visited_at.nil? + if current_user.has_gov_or_mil_email? && current_user.piv_cac_recommended_dismissed_at.nil? redirect_to login_piv_cac_recommended_path end end diff --git a/app/controllers/users/piv_cac_recommended_controller.rb b/app/controllers/users/piv_cac_recommended_controller.rb index 516ac030a41..3442a62da8f 100644 --- a/app/controllers/users/piv_cac_recommended_controller.rb +++ b/app/controllers/users/piv_cac_recommended_controller.rb @@ -3,10 +3,8 @@ class PivCacRecommendedController < ApplicationController include TwoFactorAuthenticatableMethods include MfaSetupConcern include SecureHeadersConcern - include ReauthenticationRequiredConcern before_action :authenticate_user! - before_action :confirm_user_authenticated_for_2fa_setup before_action :apply_secure_headers_override before_action :user_email_is_gov_or_mil? @@ -18,17 +16,17 @@ def show def confirm UpdateUser.new( user: current_user, - attributes: { piv_cac_recommended_visited_at: Time.zone.now }, + attributes: { piv_cac_recommended_dismissed_at: Time.zone.now }, ).call analytics.piv_cac_recommended(action: :accepted) set_mfa_selections(['piv_cac']) - redirect_to confirmation_path(user_session[:mfa_selections].first) + redirect_to next_setup_path end def skip UpdateUser.new( user: current_user, - attributes: { piv_cac_recommended_visited_at: Time.zone.now }, + attributes: { piv_cac_recommended_dismissed_at: Time.zone.now }, ).call analytics.piv_cac_recommended(action: :skipped) redirect_to after_sign_in_path_for(current_user) @@ -36,7 +34,7 @@ def skip private - def user_email_is_gov_or_mil? + def user_email_is_gov_or_mil redirect_to after_sign_in_path_for(current_user) unless current_user.has_gov_or_mil_email? end end diff --git a/app/controllers/users/two_factor_authentication_setup_controller.rb b/app/controllers/users/two_factor_authentication_setup_controller.rb index 33c3bae7bb5..287579d1bab 100644 --- a/app/controllers/users/two_factor_authentication_setup_controller.rb +++ b/app/controllers/users/two_factor_authentication_setup_controller.rb @@ -2,7 +2,6 @@ module Users class TwoFactorAuthenticationSetupController < ApplicationController include UserAuthenticator include MfaSetupConcern - include PivCacConcern before_action :authenticate_user before_action :confirm_user_authenticated_for_2fa_setup diff --git a/app/presenters/piv_cac_recommended_presenter.rb b/app/presenters/piv_cac_recommended_presenter.rb index 5ccc0278963..330d3f27a13 100644 --- a/app/presenters/piv_cac_recommended_presenter.rb +++ b/app/presenters/piv_cac_recommended_presenter.rb @@ -29,4 +29,4 @@ def skip_text I18n.t('two_factor_authentication.piv_cac_upsell.choose_other_method') end end - end +end diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index f5405c14d7a..d2cf8be9cf5 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -4127,6 +4127,21 @@ def piv_cac_login_visited track_event(:piv_cac_login_visited) end + # @param [String] action what action user made + # Tracks when user submits an action on Piv Cac recommended page + def piv_cac_recommended(action: nil, **extra) + track_event( + :piv_cac_recommended, + action: action, + **extra, + ) + end + + # Tracks when user visits piv cac recommended + def piv_cac_recommended_visited + track_event(:piv_cac_recommended_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 @@ -4158,20 +4173,6 @@ def piv_cac_update_name_submitted( ) end - # Tracks when user visits piv cac recommended - def piv_cac_recommended_visited - track_event(:piv_cac_recommended_visited) - end - - # @param [String] action what action user made - # Tracks when user submits an action on Piv Cac recommended page - def piv_cac_recommended(action: nil, **_exra) - track_event( - :piv_cac_recommended, - action: action, - ) - 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/users/piv_cac_recommended_controller_spec.rb b/spec/controllers/users/piv_cac_recommended_controller_spec.rb index f326f998e96..a9bd996aa9f 100644 --- a/spec/controllers/users/piv_cac_recommended_controller_spec.rb +++ b/spec/controllers/users/piv_cac_recommended_controller_spec.rb @@ -66,7 +66,7 @@ post :confirm user.reload - expect(user.piv_cac_recommended_visited_at).to be_truthy + expect(user.piv_cac_recommended_dismissed_at).to be_truthy end it 'logs analytics' do @@ -94,7 +94,7 @@ post :skip user.reload - expect(user.piv_cac_recommended_visited_at).to be_truthy + expect(user.piv_cac_recommended_dismissed_at).to be_truthy end it 'logs analytics' do diff --git a/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb b/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb index aed8116e9d3..ad71c86a732 100644 --- a/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb +++ b/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb @@ -24,7 +24,7 @@ context 'with user having gov or mil email' do let(:user) do - create(:user, email: 'example@example.gov', piv_cac_recommended_visited_at: Time.zone.now) + create(:user, email: 'example@example.gov', piv_cac_recommended_dismissed_at: Time.zone.now) end it 'tracks the visit in analytics' do From a985060b206e087b4378d5055c5f6ff42c5176f6 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Wed, 27 Mar 2024 08:32:44 -0400 Subject: [PATCH 26/36] change method name to be clearer --- app/controllers/users/piv_cac_recommended_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/users/piv_cac_recommended_controller.rb b/app/controllers/users/piv_cac_recommended_controller.rb index 3442a62da8f..052b14e3f20 100644 --- a/app/controllers/users/piv_cac_recommended_controller.rb +++ b/app/controllers/users/piv_cac_recommended_controller.rb @@ -6,7 +6,7 @@ class PivCacRecommendedController < ApplicationController before_action :authenticate_user! before_action :apply_secure_headers_override - before_action :user_email_is_gov_or_mil? + before_action :redirect_unless_user_email_is_gov_or_mil def show @recommended_presenter = PivCacRecommendedPresenter.new(current_user) @@ -34,7 +34,7 @@ def skip private - def user_email_is_gov_or_mil + def redirect_unless_user_email_is_gov_or_mil redirect_to after_sign_in_path_for(current_user) unless current_user.has_gov_or_mil_email? end end From 023ac7b3d5b865db9a8f6f85b5c3f3d3e84e0471 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Wed, 27 Mar 2024 09:41:11 -0400 Subject: [PATCH 27/36] change back to confirmation check --- app/controllers/users/piv_cac_recommended_controller.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/users/piv_cac_recommended_controller.rb b/app/controllers/users/piv_cac_recommended_controller.rb index 052b14e3f20..2cba299fd6a 100644 --- a/app/controllers/users/piv_cac_recommended_controller.rb +++ b/app/controllers/users/piv_cac_recommended_controller.rb @@ -3,8 +3,8 @@ class PivCacRecommendedController < ApplicationController include TwoFactorAuthenticatableMethods include MfaSetupConcern include SecureHeadersConcern - - before_action :authenticate_user! + + before_action :confirm_user_authenticated_for_2fa_setup before_action :apply_secure_headers_override before_action :redirect_unless_user_email_is_gov_or_mil @@ -20,7 +20,7 @@ def confirm ).call analytics.piv_cac_recommended(action: :accepted) set_mfa_selections(['piv_cac']) - redirect_to next_setup_path + redirect_to confirmation_path(user_session[:mfa_selections].first) end def skip From 48276b101ff5d6cd9e54a960450381d24400bc69 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Wed, 27 Mar 2024 10:45:36 -0400 Subject: [PATCH 28/36] rubocop fix --- app/controllers/users/piv_cac_recommended_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/users/piv_cac_recommended_controller.rb b/app/controllers/users/piv_cac_recommended_controller.rb index 2cba299fd6a..d25aa8f9149 100644 --- a/app/controllers/users/piv_cac_recommended_controller.rb +++ b/app/controllers/users/piv_cac_recommended_controller.rb @@ -3,7 +3,7 @@ class PivCacRecommendedController < ApplicationController include TwoFactorAuthenticatableMethods include MfaSetupConcern include SecureHeadersConcern - + before_action :confirm_user_authenticated_for_2fa_setup before_action :apply_secure_headers_override before_action :redirect_unless_user_email_is_gov_or_mil From 26132608963716ca7922d6f8d02e68f00276dfab Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Wed, 27 Mar 2024 13:54:57 -0400 Subject: [PATCH 29/36] rubocop and mfa concern --- app/controllers/concerns/mfa_setup_concern.rb | 4 ++++ app/controllers/users/piv_cac_recommended_controller.rb | 2 +- .../users/two_factor_authentication_setup_controller.rb | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/controllers/concerns/mfa_setup_concern.rb b/app/controllers/concerns/mfa_setup_concern.rb index 25d8a78a249..293a46c0a9d 100644 --- a/app/controllers/concerns/mfa_setup_concern.rb +++ b/app/controllers/concerns/mfa_setup_concern.rb @@ -54,6 +54,10 @@ def suggest_second_mfa? mfa_selection_count < 2 && mfa_context.enabled_mfa_methods_count < 2 end + def firs_mfa_selection_path + confirmation_path(user_session[:mfa_selections].first) + end + def in_account_creation_flow? user_session[:in_account_creation_flow] || false end diff --git a/app/controllers/users/piv_cac_recommended_controller.rb b/app/controllers/users/piv_cac_recommended_controller.rb index d25aa8f9149..4c5cf238dff 100644 --- a/app/controllers/users/piv_cac_recommended_controller.rb +++ b/app/controllers/users/piv_cac_recommended_controller.rb @@ -20,7 +20,7 @@ def confirm ).call analytics.piv_cac_recommended(action: :accepted) set_mfa_selections(['piv_cac']) - redirect_to confirmation_path(user_session[:mfa_selections].first) + redirect_to firs_mfa_selection_path end def skip diff --git a/app/controllers/users/two_factor_authentication_setup_controller.rb b/app/controllers/users/two_factor_authentication_setup_controller.rb index 287579d1bab..2826a130865 100644 --- a/app/controllers/users/two_factor_authentication_setup_controller.rb +++ b/app/controllers/users/two_factor_authentication_setup_controller.rb @@ -74,7 +74,7 @@ def process_valid_form user_session[:mfa_selections] = @two_factor_options_form.selection if user_session[:mfa_selections].first.present? - redirect_to confirmation_path(user_session[:mfa_selections].first) + redirect_to firs_mfa_selection_path else redirect_to after_mfa_setup_path end From 8d05c2577a90d3a81ddef61722b52c2c51144b7b Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Wed, 3 Apr 2024 09:01:33 -0400 Subject: [PATCH 30/36] address comments, delete obsolete migration --- app/controllers/concerns/mfa_setup_concern.rb | 2 +- app/controllers/users/piv_cac_recommended_controller.rb | 2 ++ .../users/two_factor_authentication_setup_controller.rb | 7 +------ app/presenters/piv_cac_recommended_presenter.rb | 2 ++ app/presenters/two_factor_options_presenter.rb | 5 +---- ...6181600_add_piv_cac_recommended_dismissed_at_on_user.rb | 5 ----- spec/presenters/piv_cac_recommended_presenter_spec.rb | 2 +- 7 files changed, 8 insertions(+), 17 deletions(-) delete mode 100644 db/primary_migrate/20240326181600_add_piv_cac_recommended_dismissed_at_on_user.rb diff --git a/app/controllers/concerns/mfa_setup_concern.rb b/app/controllers/concerns/mfa_setup_concern.rb index 4e7cb23f78c..e999c4c1376 100644 --- a/app/controllers/concerns/mfa_setup_concern.rb +++ b/app/controllers/concerns/mfa_setup_concern.rb @@ -56,7 +56,7 @@ def suggest_second_mfa? mfa_selection_count < 2 && mfa_context.enabled_mfa_methods_count < 2 end - def firs_mfa_selection_path + def first_mfa_selection_path confirmation_path(user_session[:mfa_selections].first) end diff --git a/app/controllers/users/piv_cac_recommended_controller.rb b/app/controllers/users/piv_cac_recommended_controller.rb index 4c5cf238dff..dc090081ec6 100644 --- a/app/controllers/users/piv_cac_recommended_controller.rb +++ b/app/controllers/users/piv_cac_recommended_controller.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Users class PivCacRecommendedController < ApplicationController include TwoFactorAuthenticatableMethods diff --git a/app/controllers/users/two_factor_authentication_setup_controller.rb b/app/controllers/users/two_factor_authentication_setup_controller.rb index 227b0e31b9a..492acb4bcee 100644 --- a/app/controllers/users/two_factor_authentication_setup_controller.rb +++ b/app/controllers/users/two_factor_authentication_setup_controller.rb @@ -74,12 +74,7 @@ def two_factor_options_presenter def process_valid_form user_session[:mfa_selections] = @two_factor_options_form.selection - - if user_session[:mfa_selections].first.present? - redirect_to firs_mfa_selection_path - else - redirect_to after_mfa_setup_path - end + redirect_to(first_mfa_selection_path || after_mfa_setup_path) end def two_factor_options_form_params diff --git a/app/presenters/piv_cac_recommended_presenter.rb b/app/presenters/piv_cac_recommended_presenter.rb index 330d3f27a13..37c5b2d937d 100644 --- a/app/presenters/piv_cac_recommended_presenter.rb +++ b/app/presenters/piv_cac_recommended_presenter.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class PivCacRecommendedPresenter attr_accessor :user def initialize(user) diff --git a/app/presenters/two_factor_options_presenter.rb b/app/presenters/two_factor_options_presenter.rb index 4b20a7c57e3..83a0d4434f7 100644 --- a/app/presenters/two_factor_options_presenter.rb +++ b/app/presenters/two_factor_options_presenter.rb @@ -11,6 +11,7 @@ class TwoFactorOptionsPresenter :user_agent delegate :two_factor_enabled?, to: :mfa_policy + delegate :has_gov_or_mil_email?, to: :user, prefix: :user def initialize( user_agent:, @@ -120,8 +121,4 @@ def mfa_policy def user_has_dismissed_second_mfa_reminder? user.second_mfa_reminder_dismissed_at.present? end - - def user_has_gov_or_mil_email? - user.confirmed_email_addresses.any?(&:gov_or_mil?) - end end diff --git a/db/primary_migrate/20240326181600_add_piv_cac_recommended_dismissed_at_on_user.rb b/db/primary_migrate/20240326181600_add_piv_cac_recommended_dismissed_at_on_user.rb deleted file mode 100644 index 66bb8d2e264..00000000000 --- a/db/primary_migrate/20240326181600_add_piv_cac_recommended_dismissed_at_on_user.rb +++ /dev/null @@ -1,5 +0,0 @@ -class AddPivCacRecommendedDismissedAtOnUser < ActiveRecord::Migration[7.1] - def change - add_column :users, :piv_cac_recommended_dismissed_at, :datetime, default: nil - end -end diff --git a/spec/presenters/piv_cac_recommended_presenter_spec.rb b/spec/presenters/piv_cac_recommended_presenter_spec.rb index 645f0d5badc..0044df6adfa 100644 --- a/spec/presenters/piv_cac_recommended_presenter_spec.rb +++ b/spec/presenters/piv_cac_recommended_presenter_spec.rb @@ -73,7 +73,7 @@ end end - describe '#skip_type' do + describe '#skip_text' do context 'when existing user' do let(:user) { create(:user, :with_phone, { email: 'example@example.mil' }) } it 'should return skip text' do From a137b3005470ad904cb03778c2db1460907646c9 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Wed, 3 Apr 2024 10:01:49 -0400 Subject: [PATCH 31/36] make sure to check controller for proper redirecting --- .../piv_cac_recommended_presenter.rb | 4 +-- ...or_authentication_setup_controller_spec.rb | 28 ++++++++++++++----- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/app/presenters/piv_cac_recommended_presenter.rb b/app/presenters/piv_cac_recommended_presenter.rb index 37c5b2d937d..612a91afd2c 100644 --- a/app/presenters/piv_cac_recommended_presenter.rb +++ b/app/presenters/piv_cac_recommended_presenter.rb @@ -15,8 +15,8 @@ def info end def email_type - address = user.confirmed_email_addresses.select { |address| address.gov_or_mil? } - case address.first.email.end_with?('.gov') + address = user.confirmed_email_addresses.find { |address| address.gov_or_mil? } + case address.email.end_with?('.gov') when true '.gov' else diff --git a/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb b/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb index ad71c86a732..ca2f800cb33 100644 --- a/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb +++ b/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb @@ -26,16 +26,30 @@ let(:user) do create(:user, email: 'example@example.gov', piv_cac_recommended_dismissed_at: Time.zone.now) end + context 'having already visited the PIV interstitial page' do + it 'tracks the visit in analytics' do + get :index + + expect(@analytics).to have_logged_event( + 'User Registration: 2FA Setup visited', + enabled_mfa_methods_count: 0, + gov_or_mil_email: true, + ) + end + end - it 'tracks the visit in analytics' do - get :index + context 'directed to page without having visited PIV interstitial page' do + let(:user) do + create(:user, email: 'example@example.gov') + end - expect(@analytics).to have_logged_event( - 'User Registration: 2FA Setup visited', - enabled_mfa_methods_count: 0, - gov_or_mil_email: true, - ) + it 'redirects user to piv_recommended_path' do + get :index + + expect(response).to redirect_to(login_piv_cac_recommended_url) + end end + end context 'when signed out' do From 42456bae472067938eca7bd64bea1b7b5d0a3027 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Wed, 3 Apr 2024 12:10:15 -0400 Subject: [PATCH 32/36] make unstyled --- app/views/users/piv_cac_recommended/show.html.erb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/views/users/piv_cac_recommended/show.html.erb b/app/views/users/piv_cac_recommended/show.html.erb index 34c7f19cf64..f099ec02a11 100644 --- a/app/views/users/piv_cac_recommended/show.html.erb +++ b/app/views/users/piv_cac_recommended/show.html.erb @@ -28,7 +28,6 @@ &block ) end, - outline: true, class: 'usa-button usa-button--unstyled', ).with_content(@recommended_presenter.skip_text) %> From 232505749812aec01ea68e4a1e3539aebe6328f1 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Wed, 3 Apr 2024 16:37:41 -0400 Subject: [PATCH 33/36] rubocop and fix method --- app/controllers/users/piv_cac_recommended_controller.rb | 2 +- .../users/two_factor_authentication_setup_controller_spec.rb | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/app/controllers/users/piv_cac_recommended_controller.rb b/app/controllers/users/piv_cac_recommended_controller.rb index dc090081ec6..c395283f734 100644 --- a/app/controllers/users/piv_cac_recommended_controller.rb +++ b/app/controllers/users/piv_cac_recommended_controller.rb @@ -22,7 +22,7 @@ def confirm ).call analytics.piv_cac_recommended(action: :accepted) set_mfa_selections(['piv_cac']) - redirect_to firs_mfa_selection_path + redirect_to first_mfa_selection_path end def skip diff --git a/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb b/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb index ca2f800cb33..ce9e9021ec1 100644 --- a/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb +++ b/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb @@ -29,7 +29,7 @@ context 'having already visited the PIV interstitial page' do it 'tracks the visit in analytics' do get :index - + expect(@analytics).to have_logged_event( 'User Registration: 2FA Setup visited', enabled_mfa_methods_count: 0, @@ -45,11 +45,10 @@ it 'redirects user to piv_recommended_path' do get :index - + expect(response).to redirect_to(login_piv_cac_recommended_url) end end - end context 'when signed out' do From dd40dc06fbd9e33e7e68f1f48b4f531b33d52e94 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Fri, 5 Apr 2024 08:32:08 -0400 Subject: [PATCH 34/36] commit option --- db/schema.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/db/schema.rb b/db/schema.rb index 1af808446ec..7a369a4b148 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -613,6 +613,7 @@ t.string "encrypted_recovery_code_digest_multi_region" t.datetime "second_mfa_reminder_dismissed_at" t.datetime "piv_cac_recommended_dismissed_at" + t.datetime "check_password_compromised_at" t.datetime "sign_in_new_device_at" t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true t.index ["sign_in_new_device_at"], name: "index_users_on_sign_in_new_device_at" From 7da27a527179361625bda447abe3ff5b3c208274 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Fri, 5 Apr 2024 09:15:23 -0400 Subject: [PATCH 35/36] remove unneeded schema change --- db/schema.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/db/schema.rb b/db/schema.rb index 7a369a4b148..1af808446ec 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -613,7 +613,6 @@ t.string "encrypted_recovery_code_digest_multi_region" t.datetime "second_mfa_reminder_dismissed_at" t.datetime "piv_cac_recommended_dismissed_at" - t.datetime "check_password_compromised_at" t.datetime "sign_in_new_device_at" t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true t.index ["sign_in_new_device_at"], name: "index_users_on_sign_in_new_device_at" From 55ab8ac08673886988f8463a5de0c797ec6e8dae Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Fri, 5 Apr 2024 09:47:46 -0400 Subject: [PATCH 36/36] address comments --- app/controllers/application_controller.rb | 2 +- app/presenters/piv_cac_recommended_presenter.rb | 2 +- app/views/users/piv_cac_recommended/show.html.erb | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index c374c2d6d17..e16f565cf4c 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -263,7 +263,7 @@ def user_needs_to_reactivate_account? end def user_recommended_for_piv_cac? - current_user.has_gov_or_mil_email? && current_user.piv_cac_recommended_dismissed_at.nil? && + current_user.piv_cac_recommended_dismissed_at.nil? && current_user.has_gov_or_mil_email? && !user_already_has_piv? end diff --git a/app/presenters/piv_cac_recommended_presenter.rb b/app/presenters/piv_cac_recommended_presenter.rb index 612a91afd2c..882730987e4 100644 --- a/app/presenters/piv_cac_recommended_presenter.rb +++ b/app/presenters/piv_cac_recommended_presenter.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class PivCacRecommendedPresenter - attr_accessor :user + attr_reader :user def initialize(user) @user = user end diff --git a/app/views/users/piv_cac_recommended/show.html.erb b/app/views/users/piv_cac_recommended/show.html.erb index f099ec02a11..3e7ef1e0167 100644 --- a/app/views/users/piv_cac_recommended/show.html.erb +++ b/app/views/users/piv_cac_recommended/show.html.erb @@ -1,6 +1,6 @@ <% self.title = t('titles.piv_cac_setup.upsell') %> -<%= render AlertIconComponent.new(icon_name: :info_question) %> +<%= render AlertIconComponent.new(icon_name: :info_question, class: 'margin-bottom-4') %> <%= render PageHeadingComponent.new.with_content(t('titles.piv_cac_setup.upsell')) %> @@ -17,7 +17,7 @@ ) end, big: true, - class: 'margin-bottom-3', + class: 'margin-top-5 margin-bottom-2', ).with_content(t('two_factor_authentication.piv_cac_upsell.add_piv')) %> <%= render ButtonComponent.new( @@ -28,6 +28,6 @@ &block ) end, - class: 'usa-button usa-button--unstyled', + unstyled: true, ).with_content(@recommended_presenter.skip_text) %>