From f17a48cebe50f1ce9f76ac59169adcc8406b3db5 Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Fri, 14 Apr 2023 09:38:15 -0500 Subject: [PATCH 01/10] Remove unused configuration redis_throttle_alternate_url and redis_throttle_alternate_pool_write_enabled configurations (#8211) changelog: Internal, Configuration, Remove unused Redis migration configuration --- app/services/throttle.rb | 33 --------------------------------- config/application.yml.default | 2 -- config/initializers/01_redis.rb | 7 ------- lib/identity_config.rb | 2 -- 4 files changed, 44 deletions(-) diff --git a/app/services/throttle.rb b/app/services/throttle.rb index 6f898b96ac6..139c6a07d94 100644 --- a/app/services/throttle.rb +++ b/app/services/throttle.rb @@ -80,18 +80,6 @@ def increment! end end - if write_to_alternate? - REDIS_THROTTLE_ALTERNATE_POOL.with do |client| - _alternate_value, _success = client.multi do |multi| - multi.incr(key) - multi.expire( - key, - Throttle.attempt_window_in_minutes(throttle_type).minutes.seconds.to_i, - ) - end - end - end - @redis_attempts = value.to_i @redis_attempted_at = Time.zone.now @@ -133,12 +121,6 @@ def reset! client.del(key) end - if write_to_alternate? - REDIS_THROTTLE_ALTERNATE_POOL.with do |client| - client.del(key) - end - end - @redis_attempts = 0 @redis_attempted_at = nil end @@ -154,16 +136,6 @@ def increment_to_throttled! ) end - if write_to_alternate? - REDIS_THROTTLE_ALTERNATE_POOL.with do |client| - client.setex( - key, - Throttle.attempt_window_in_minutes(throttle_type).minutes.seconds.to_i, - value, - ) - end - end - @redis_attempts = value.to_i @redis_attempted_at = Time.zone.now @@ -178,11 +150,6 @@ def key end end - def write_to_alternate? - REDIS_THROTTLE_ALTERNATE_POOL && - IdentityConfig.store.redis_throttle_alternate_pool_write_enabled - end - def self.attempt_window_in_minutes(throttle_type) throttle_config.dig(throttle_type, :attempt_window) end diff --git a/config/application.yml.default b/config/application.yml.default index f73f2d5af4b..bc38d449f33 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -261,8 +261,6 @@ recaptcha_secret_key_v3: '' recovery_code_length: 4 redis_irs_attempt_api_url: redis://localhost:6379/2 redis_throttle_url: redis://localhost:6379/1 -redis_throttle_alternate_url: '' -redis_throttle_alternate_pool_write_enabled: false redis_url: redis://localhost:6379/0 redis_pool_size: 10 redis_session_pool_size: 10 diff --git a/config/initializers/01_redis.rb b/config/initializers/01_redis.rb index 467825aba79..1385069fa7d 100644 --- a/config/initializers/01_redis.rb +++ b/config/initializers/01_redis.rb @@ -7,10 +7,3 @@ REDIS_THROTTLE_POOL = ConnectionPool.new(size: IdentityConfig.store.redis_throttle_pool_size) do Redis.new(url: IdentityConfig.store.redis_throttle_url) end - -REDIS_THROTTLE_ALTERNATE_POOL = - if IdentityConfig.store.redis_throttle_alternate_url - ConnectionPool.new(size: IdentityConfig.store.redis_throttle_pool_size) do - Redis.new(url: IdentityConfig.store.redis_throttle_alternate_url) - end - end diff --git a/lib/identity_config.rb b/lib/identity_config.rb index 2cd46b4d018..3b7adc651ff 100644 --- a/lib/identity_config.rb +++ b/lib/identity_config.rb @@ -359,8 +359,6 @@ def self.build_store(config_map) config.add(:redis_irs_attempt_api_url) config.add(:redis_irs_attempt_api_pool_size, type: :integer) config.add(:redis_throttle_url) - config.add(:redis_throttle_alternate_url) - config.add(:redis_throttle_alternate_pool_write_enabled, type: :boolean) config.add(:redis_url) config.add(:redis_pool_size, type: :integer) config.add(:redis_session_pool_size, type: :integer) From 9c5ce327283b0ade1ec7cb76595a0fa70901beda Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Fri, 14 Apr 2023 08:16:27 -0700 Subject: [PATCH 02/10] Override tag style to leave case unchanged and make text bold (#8212) * Override tag style to leave case unchanged and make text bold Per Andrew Duthie the Login style for tags is Title case and bold, but the design system is still all caps, so override it for idp for now. changelog: User-facing Improvements, Design, change 'tag' style to be bold and preserve case --- app/assets/stylesheets/components/_tag.scss | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/assets/stylesheets/components/_tag.scss b/app/assets/stylesheets/components/_tag.scss index 2a9bd0f7fae..155568cfb7a 100644 --- a/app/assets/stylesheets/components/_tag.scss +++ b/app/assets/stylesheets/components/_tag.scss @@ -4,6 +4,8 @@ .usa-tag { display: inline-block; + text-transform: none; + font-weight: bold; } .usa-tag--informative { From 0ce4192f418fe0f3f309e8d88bd37aeba48b63da Mon Sep 17 00:00:00 2001 From: Jonathan Hooper Date: Fri, 14 Apr 2023 13:22:34 -0400 Subject: [PATCH 03/10] LG-9333 Preserve the SSN when returning to the SSN controller (#8197) The SSN controller allows a user to enter or update their SSN. Prior to this commit the previous value of the SSN was not maintained when the user was updating their SSN. This commit attempts to fix that by making use of the SSN form that is used in the controller. The template is changed to eventually read the SSN from the form. Additionally, the form is used in a pattern that better matches the pattern you expect a form object to be used in. This was not the case before because of constraints that the flow state machine placed on HTML forms. changelog: Improvements, Proofing workflow, The user's SSN is preserved for users who enter and SSN then continue to the verify step but return to the SSN in the unsupervised remote proofing flow. --- app/controllers/idv/ssn_controller.rb | 25 ++++++------------- app/forms/idv/ssn_format_form.rb | 9 +++++-- app/views/idv/in_person/ssn.html.erb | 2 +- app/views/idv/ssn/show.html.erb | 10 ++++---- app/views/shared/_ssn_field.html.erb | 2 +- spec/controllers/idv/ssn_controller_spec.rb | 5 ++-- .../idv/doc_auth/verify_info_step_spec.rb | 5 ++++ spec/forms/idv/ssn_format_form_spec.rb | 22 +++++++++++++++- 8 files changed, 51 insertions(+), 29 deletions(-) diff --git a/app/controllers/idv/ssn_controller.rb b/app/controllers/idv/ssn_controller.rb index 936b331d6f3..420e3e4b7a9 100644 --- a/app/controllers/idv/ssn_controller.rb +++ b/app/controllers/idv/ssn_controller.rb @@ -14,19 +14,22 @@ class SsnController < ApplicationController def show increment_step_counts - analytics.idv_doc_auth_redo_ssn_submitted(**analytics_arguments) if updating_ssn + @ssn_form = Idv::SsnFormatForm.new(current_user, flow_session) + analytics.idv_doc_auth_redo_ssn_submitted(**analytics_arguments) if @ssn_form.updating_ssn? analytics.idv_doc_auth_ssn_visited(**analytics_arguments) Funnel::DocAuth::RegisterStep.new(current_user.id, sp_session[:issuer]). call('ssn', :view, true) - render :show, locals: extra_view_variables + render :show, locals: threatmetrix_view_variables end def update @error_message = nil - form_response = form_submit + + @ssn_form = Idv::SsnFormatForm.new(current_user, flow_session) + form_response = @ssn_form.submit(params.require(:doc_auth).permit(:ssn)) analytics.idv_doc_auth_ssn_submitted( **analytics_arguments.merge(form_response.to_h), @@ -41,18 +44,10 @@ def update redirect_to next_url else @error_message = form_response.first_error_message - render :show, locals: extra_view_variables + render :show, locals: threatmetrix_view_variables end end - def extra_view_variables - { - updating_ssn: updating_ssn, - success_alert_enabled: !updating_ssn, - **threatmetrix_view_variables, - } - end - private def next_url @@ -83,12 +78,8 @@ def increment_step_counts current_flow_step_counts['Idv::Steps::SsnStep'] += 1 end - def form_submit - Idv::SsnFormatForm.new(current_user).submit(params.require(:doc_auth).permit(:ssn)) - end - def updating_ssn - flow_session.dig('pii_from_doc', :ssn).present? + @ssn_form.updating_ssn? end end end diff --git a/app/forms/idv/ssn_format_form.rb b/app/forms/idv/ssn_format_form.rb index 6354b763d83..8b5a6e6f4b3 100644 --- a/app/forms/idv/ssn_format_form.rb +++ b/app/forms/idv/ssn_format_form.rb @@ -8,11 +8,12 @@ class SsnFormatForm attr_accessor :ssn def self.model_name - ActiveModel::Name.new(self, nil, 'Ssn') + ActiveModel::Name.new(self, nil, 'doc_auth') end - def initialize(user) + def initialize(user, flow_session = {}) @user = user + @ssn = flow_session.dig('pii_from_doc', :ssn) end def submit(params) @@ -25,6 +26,10 @@ def submit(params) ) end + def updating_ssn? + ssn.present? + end + private def consume_params(params) diff --git a/app/views/idv/in_person/ssn.html.erb b/app/views/idv/in_person/ssn.html.erb index 5e6cd1d086d..195f167c9b6 100644 --- a/app/views/idv/in_person/ssn.html.erb +++ b/app/views/idv/in_person/ssn.html.erb @@ -50,7 +50,7 @@ locals: <% end %> <%= simple_form_for( - :doc_auth, + Idv::SsnFormatForm.new(current_user), url: url_for, method: :put, html: { autocomplete: 'off' }, diff --git a/app/views/idv/ssn/show.html.erb b/app/views/idv/ssn/show.html.erb index e7bc914d943..b5d6ba650fd 100644 --- a/app/views/idv/ssn/show.html.erb +++ b/app/views/idv/ssn/show.html.erb @@ -17,7 +17,7 @@ locals: <% title t('titles.doc_auth.ssn') %> -<% if success_alert_enabled %> +<% if !@ssn_form.updating_ssn? %> <%= render AlertComponent.new( type: :success, class: 'margin-bottom-4', @@ -26,7 +26,7 @@ locals: <% end %> <% end %> -<% if updating_ssn %> +<% if @ssn_form.updating_ssn? %> <%= render PageHeadingComponent.new.with_content(t('doc_auth.headings.ssn_update')) %> <% else %> <%= render PageHeadingComponent.new.with_content(t('doc_auth.headings.ssn')) %> @@ -66,7 +66,7 @@ locals: <% end %> <%= simple_form_for( - :doc_auth, + @ssn_form, url: idv_ssn_url, method: :put, html: { autocomplete: 'off' }, @@ -78,7 +78,7 @@ locals:

<%= @error_message %>

<%= f.submit class: 'display-block margin-y-5' do %> - <% if updating_ssn %> + <% if @ssn_form.updating_ssn? %> <%= t('forms.buttons.submit.update') %> <% else %> <%= t('forms.buttons.continue') %> @@ -86,7 +86,7 @@ locals: <% end %> <% end %> -<% if updating_ssn %> +<% if @ssn_form.updating_ssn? %> <%= render 'idv/shared/back', fallback_path: idv_verify_info_path %> <% else %> <%= render 'idv/doc_auth/cancel', step: 'ssn' %> diff --git a/app/views/shared/_ssn_field.html.erb b/app/views/shared/_ssn_field.html.erb index d559c10df0f..44afe95439f 100644 --- a/app/views/shared/_ssn_field.html.erb +++ b/app/views/shared/_ssn_field.html.erb @@ -15,7 +15,7 @@ locals: required: true, pattern: '^\d{3}-?\d{2}-?\d{4}$', maxlength: 11, - input_html: { class: 'ssn-toggle usa-input', value: '' }, + input_html: { class: 'ssn-toggle', value: f.object.ssn }, error_messages: { patternMismatch: t('idv.errors.pattern_mismatch.ssn') }, }, ) %> diff --git a/spec/controllers/idv/ssn_controller_spec.rb b/spec/controllers/idv/ssn_controller_spec.rb index 442d7eb48f9..7afed831c25 100644 --- a/spec/controllers/idv/ssn_controller_spec.rb +++ b/spec/controllers/idv/ssn_controller_spec.rb @@ -144,7 +144,8 @@ end it 'adds a threatmetrix session id to flow session' do - subject.extra_view_variables + put :update, params: params + subject.threatmetrix_view_variables expect(flow_session[:threatmetrix_session_id]).to_not eq(nil) end @@ -152,7 +153,7 @@ flow_session['pii_from_doc'][:ssn] = ssn put :update, params: params session_id = flow_session[:threatmetrix_session_id] - subject.extra_view_variables + subject.threatmetrix_view_variables expect(flow_session[:threatmetrix_session_id]).to eq(session_id) end end diff --git a/spec/features/idv/doc_auth/verify_info_step_spec.rb b/spec/features/idv/doc_auth/verify_info_step_spec.rb index ad24b0a7d27..abfcce0cb46 100644 --- a/spec/features/idv/doc_auth/verify_info_step_spec.rb +++ b/spec/features/idv/doc_auth/verify_info_step_spec.rb @@ -75,7 +75,12 @@ it 'allows the user to enter in a new ssn and displays updated info' do click_link t('idv.buttons.change_ssn_label') + expect(page).to have_current_path(idv_ssn_path) + expect( + find_field(t('idv.form.ssn_label_html')).value, + ).to eq(DocAuthHelper::GOOD_SSN.gsub(/\D/, '')) + fill_in t('idv.form.ssn_label_html'), with: '900456789' click_button t('forms.buttons.submit.update') diff --git a/spec/forms/idv/ssn_format_form_spec.rb b/spec/forms/idv/ssn_format_form_spec.rb index 2f791a7a307..f368e6e7f19 100644 --- a/spec/forms/idv/ssn_format_form_spec.rb +++ b/spec/forms/idv/ssn_format_form_spec.rb @@ -2,8 +2,10 @@ describe Idv::SsnFormatForm do let(:user) { create(:user) } - let(:subject) { Idv::SsnFormatForm.new(user) } let(:ssn) { '111-11-1111' } + let(:flow_session) { {} } + + subject { Idv::SsnFormatForm.new(user, flow_session) } describe '#submit' do context 'when the form is valid' do @@ -34,6 +36,24 @@ end end + describe '#updating_ssn' do + context 'when no flow_session value is provided' do + subject { Idv::SsnFormatForm.new(user) } + + it { expect(subject.updating_ssn?).to eq(false) } + end + + context 'when the pii_from_doc hash does not contain an SSN value' do + it { expect(subject.updating_ssn?).to eq(false) } + end + + context 'when there is an SSN in the pii_from_doc hash' do + let(:flow_session) { { 'pii_from_doc' => { ssn: '900-12-3456' } } } + + it { expect(subject.updating_ssn?).to eq(true) } + end + end + describe 'presence validations' do it 'is invalid when required attribute is not present' do subject.submit(ssn: nil) From e8584a416f33e20cc4e47484102f27e3c4125ce6 Mon Sep 17 00:00:00 2001 From: Eric Gade <105373963+eric-gade@users.noreply.github.com> Date: Fri, 14 Apr 2023 14:38:14 -0400 Subject: [PATCH 04/10] LG-8714 Remove UserDecorator (#8204) * Moving UserDecorator methods to User * Updating all references to user.decorate Also: - Move UserDecorator specs to User spec (with adjustments) - Delete old UserDecorator class and spe Co-authored-by: Zach Margolis --- .../accounts/connected_accounts_controller.rb | 2 +- .../accounts/history_controller.rb | 2 +- .../accounts/personal_keys_controller.rb | 2 +- .../two_factor_authentication_controller.rb | 2 +- app/controllers/accounts_controller.rb | 2 +- app/controllers/application_controller.rb | 10 +- .../concerns/account_reactivation_concern.rb | 2 +- .../concerns/ial2_profile_concern.rb | 2 +- .../concerns/idv/phone_otp_rate_limitable.rb | 10 +- .../concerns/idv/step_utilities_concern.rb | 2 +- app/controllers/concerns/idv_session.rb | 2 +- .../concerns/saml_idp_auth_concern.rb | 4 +- .../two_factor_authenticatable_methods.rb | 10 +- .../concerns/verify_profile_concern.rb | 5 +- app/controllers/events_controller.rb | 2 +- .../idv/come_back_later_controller.rb | 2 +- app/controllers/idv/doc_auth_controller.rb | 2 +- app/controllers/idv/gpo_controller.rb | 4 +- app/controllers/idv/gpo_verify_controller.rb | 2 +- .../idv/otp_verification_controller.rb | 2 +- .../idv/personal_key_controller.rb | 2 +- app/controllers/idv_controller.rb | 2 +- .../authorization_controller.rb | 6 +- app/controllers/saml_idp_controller.rb | 2 +- .../sign_up/completions_controller.rb | 2 +- .../backup_code_verification_controller.rb | 2 +- .../personal_key_verification_controller.rb | 6 +- app/controllers/users/passwords_controller.rb | 2 +- app/controllers/users/sessions_controller.rb | 4 +- .../users/totp_setup_controller.rb | 2 +- .../two_factor_authentication_controller.rb | 2 +- app/decorators/user_decorator.rb | 143 ------- app/forms/verify_password_form.rb | 2 +- app/forms/verify_personal_key_form.rb | 2 +- app/mailers/user_mailer.rb | 2 +- app/models/anonymous_user.rb | 4 + app/models/user.rb | 137 ++++++- app/presenters/account_show_presenter.rb | 30 +- app/presenters/idv/gpo_presenter.rb | 2 +- .../max_attempts_reached_presenter.rb | 6 +- app/services/attribute_asserter.rb | 2 +- app/services/flow/flow_state_machine.rb | 2 +- .../idv/send_phone_confirmation_otp.rb | 2 +- app/services/user_event_creator.rb | 2 +- app/views/accounts/_backup_codes.html.erb | 2 +- app/views/accounts/_emails.html.erb | 2 +- app/views/accounts/show.html.erb | 2 +- app/views/idv/doc_auth/welcome.html.erb | 2 +- .../_locked.html.erb | 2 +- app/views/users/delete/show.html.erb | 2 +- spec/controllers/accounts_controller_spec.rb | 4 +- .../idv/capture_doc_controller_spec.rb | 2 +- .../idv/come_back_later_controller_spec.rb | 4 +- spec/controllers/idv/gpo_controller_spec.rb | 4 +- .../idv/gpo_verify_controller_spec.rb | 4 +- .../users/totp_setup_controller_spec.rb | 8 +- spec/decorators/user_decorator_spec.rb | 386 ------------------ spec/features/saml/multiple_endpoints_spec.rb | 2 +- spec/models/user_spec.rb | 372 ++++++++++++++++- .../presenters/account_show_presenter_spec.rb | 26 +- .../max_attempts_reached_presenter_spec.rb | 10 +- spec/support/controller_helper.rb | 8 +- .../idv_examples/clearing_and_restarting.rb | 4 +- .../connected_accounts/show.html.erb_spec.rb | 4 +- .../accounts/history/show.html.erb_spec.rb | 4 +- spec/views/accounts/show.html.erb_spec.rb | 12 +- .../show.html.erb_spec.rb | 10 +- .../idv/doc_auth/welcome.html.erb_spec.rb | 8 +- .../mfa_confirmation/show.html.erb_spec.rb | 1 - spec/views/users/delete/show.html.erb_spec.rb | 12 +- 70 files changed, 637 insertions(+), 699 deletions(-) delete mode 100644 app/decorators/user_decorator.rb delete mode 100644 spec/decorators/user_decorator_spec.rb diff --git a/app/controllers/accounts/connected_accounts_controller.rb b/app/controllers/accounts/connected_accounts_controller.rb index 928effac2bb..da0e0940767 100644 --- a/app/controllers/accounts/connected_accounts_controller.rb +++ b/app/controllers/accounts/connected_accounts_controller.rb @@ -11,7 +11,7 @@ def show personal_key: flash[:personal_key], sp_session_request_url: sp_session_request_url_with_updated_params, sp_name: decorated_session.sp_name, - decorated_user: current_user.decorate, + user: current_user, locked_for_session: pii_locked_for_session?(current_user), ) end diff --git a/app/controllers/accounts/history_controller.rb b/app/controllers/accounts/history_controller.rb index 2cf9b6b5009..00b69f95789 100644 --- a/app/controllers/accounts/history_controller.rb +++ b/app/controllers/accounts/history_controller.rb @@ -11,7 +11,7 @@ def show personal_key: flash[:personal_key], sp_session_request_url: sp_session_request_url_with_updated_params, sp_name: decorated_session.sp_name, - decorated_user: current_user.decorate, + user: current_user, locked_for_session: pii_locked_for_session?(current_user), ) end diff --git a/app/controllers/accounts/personal_keys_controller.rb b/app/controllers/accounts/personal_keys_controller.rb index b58208a3d35..4014ab62a12 100644 --- a/app/controllers/accounts/personal_keys_controller.rb +++ b/app/controllers/accounts/personal_keys_controller.rb @@ -31,7 +31,7 @@ def prompt_for_password_if_pii_locked end def pii_locked? - UserDecorator.new(current_user).identity_verified? && + current_user.identity_verified? && !Pii::Cacher.new(current_user, user_session).exists_in_session? end diff --git a/app/controllers/accounts/two_factor_authentication_controller.rb b/app/controllers/accounts/two_factor_authentication_controller.rb index 53db8ca2afc..a17fa96ea0c 100644 --- a/app/controllers/accounts/two_factor_authentication_controller.rb +++ b/app/controllers/accounts/two_factor_authentication_controller.rb @@ -12,7 +12,7 @@ def show personal_key: flash[:personal_key], sp_session_request_url: sp_session_request_url_with_updated_params, sp_name: decorated_session.sp_name, - decorated_user: current_user.decorate, + user: current_user, locked_for_session: pii_locked_for_session?(current_user), ) end diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index c9087f81210..cd88ada8d9e 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -13,7 +13,7 @@ def show personal_key: flash[:personal_key], sp_session_request_url: sp_session_request_url_with_updated_params, sp_name: decorated_session.sp_name, - decorated_user: current_user.decorate, + user: current_user, locked_for_session: pii_locked_for_session?(current_user), ) end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 86812df4ac6..b6fdeb06011 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -252,16 +252,16 @@ def after_mfa_setup_path end def user_needs_to_reactivate_account? - return false if current_user.decorate.password_reset_profile.blank? + return false if current_user.password_reset_profile.blank? return false if pending_profile_newer_than_password_reset_profile? sp_session[:ial2] == true end def pending_profile_newer_than_password_reset_profile? - return false if current_user.decorate.pending_profile.blank? - return false if current_user.decorate.password_reset_profile.blank? - current_user.decorate.pending_profile.created_at > - current_user.decorate.password_reset_profile.updated_at + return false if current_user.pending_profile.blank? + return false if current_user.password_reset_profile.blank? + current_user.pending_profile.created_at > + current_user.password_reset_profile.updated_at end def reauthn_param diff --git a/app/controllers/concerns/account_reactivation_concern.rb b/app/controllers/concerns/account_reactivation_concern.rb index 0c057e15ff4..e751fe090db 100644 --- a/app/controllers/concerns/account_reactivation_concern.rb +++ b/app/controllers/concerns/account_reactivation_concern.rb @@ -2,7 +2,7 @@ module AccountReactivationConcern extend ActiveSupport::Concern def confirm_password_reset_profile - return if current_user.decorate.password_reset_profile + return if current_user.password_reset_profile redirect_to root_url end diff --git a/app/controllers/concerns/ial2_profile_concern.rb b/app/controllers/concerns/ial2_profile_concern.rb index a31c90b3958..c58908d4a32 100644 --- a/app/controllers/concerns/ial2_profile_concern.rb +++ b/app/controllers/concerns/ial2_profile_concern.rb @@ -3,7 +3,7 @@ module Ial2ProfileConcern def cache_active_profile(raw_password) cacher = Pii::Cacher.new(current_user, user_session) - profile = current_user.decorate.active_or_pending_profile + profile = current_user.active_or_pending_profile begin cacher.save(raw_password, profile) rescue Encryption::EncryptionError => err diff --git a/app/controllers/concerns/idv/phone_otp_rate_limitable.rb b/app/controllers/concerns/idv/phone_otp_rate_limitable.rb index 82a3952a382..598d0f74ad8 100644 --- a/app/controllers/concerns/idv/phone_otp_rate_limitable.rb +++ b/app/controllers/concerns/idv/phone_otp_rate_limitable.rb @@ -9,14 +9,14 @@ module PhoneOtpRateLimitable def handle_locked_out_user reset_attempt_count_if_user_no_longer_locked_out - return unless decorated_user.locked_out? + return unless current_user.locked_out? analytics.idv_phone_confirmation_otp_rate_limit_locked_out handle_too_many_otp_attempts false end def reset_attempt_count_if_user_no_longer_locked_out - return unless decorated_user.no_longer_locked_out? + return unless current_user.no_longer_locked_out? UpdateUser.new( user: current_user, @@ -41,13 +41,9 @@ def handle_too_many_otp_attempts def handle_max_attempts(type) presenter = TwoFactorAuthCode::MaxAttemptsReachedPresenter.new( type, - decorated_user, + current_user, ) render_full_width('two_factor_authentication/_locked', locals: { presenter: presenter }) end - - def decorated_user - current_user.decorate - end end end diff --git a/app/controllers/concerns/idv/step_utilities_concern.rb b/app/controllers/concerns/idv/step_utilities_concern.rb index cd065b15560..f349caf34df 100644 --- a/app/controllers/concerns/idv/step_utilities_concern.rb +++ b/app/controllers/concerns/idv/step_utilities_concern.rb @@ -17,7 +17,7 @@ def acuant_sdk_ab_test_analytics_args end def irs_reproofing? - effective_user&.decorate&.reproof_for_irs?( + effective_user&.reproof_for_irs?( service_provider: current_sp, ).present? end diff --git a/app/controllers/concerns/idv_session.rb b/app/controllers/concerns/idv_session.rb index 6329fd9ca87..ea613dd8653 100644 --- a/app/controllers/concerns/idv_session.rb +++ b/app/controllers/concerns/idv_session.rb @@ -10,7 +10,7 @@ module IdvSession def confirm_idv_needed return if effective_user.active_profile.blank? || decorated_session.requested_more_recent_verification? || - effective_user.decorate.reproof_for_irs?(service_provider: current_sp) + effective_user.reproof_for_irs?(service_provider: current_sp) redirect_to idv_activated_url end diff --git a/app/controllers/concerns/saml_idp_auth_concern.rb b/app/controllers/concerns/saml_idp_auth_concern.rb index 9ded7c8a9ef..c8ff4f74d33 100644 --- a/app/controllers/concerns/saml_idp_auth_concern.rb +++ b/app/controllers/concerns/saml_idp_auth_concern.rb @@ -117,8 +117,8 @@ def link_identity_from_session_data def identity_needs_verification? ial2_requested? && - (current_user.decorate.identity_not_verified? || - current_user.decorate.reproof_for_irs?(service_provider: current_sp)) + (current_user.identity_not_verified? || + current_user.reproof_for_irs?(service_provider: current_sp)) end def_delegators :ial_context, :ial2_requested? diff --git a/app/controllers/concerns/two_factor_authenticatable_methods.rb b/app/controllers/concerns/two_factor_authenticatable_methods.rb index c04e2d8cd70..1c62e4929d8 100644 --- a/app/controllers/concerns/two_factor_authenticatable_methods.rb +++ b/app/controllers/concerns/two_factor_authenticatable_methods.rb @@ -47,7 +47,7 @@ def handle_too_many_otp_sends(phone: nil, context: nil) def handle_max_attempts(type) presenter = TwoFactorAuthCode::MaxAttemptsReachedPresenter.new( type, - decorated_user, + current_user, ) sign_out render_full_width('two_factor_authentication/_locked', locals: { presenter: presenter }) @@ -79,7 +79,7 @@ def check_sp_required_mfa_bypass(auth_method:) end def reset_attempt_count_if_user_no_longer_locked_out - return unless decorated_user.no_longer_locked_out? + return unless current_user.no_longer_locked_out? UpdateUser.new( user: current_user, @@ -119,7 +119,7 @@ def handle_invalid_otp(type:, context: nil) flash.now[:error] = invalid_otp_error(type) - if decorated_user.locked_out? + if current_user.locked_out? handle_second_factor_locked_user(context: context, type: type) else render_show_after_invalid @@ -295,10 +295,6 @@ def display_phone_to_deliver_to end end - def decorated_user - current_user.decorate - end - def confirmation_for_add_phone? UserSessionContext.confirmation_context?(context) && user_fully_authenticated? end diff --git a/app/controllers/concerns/verify_profile_concern.rb b/app/controllers/concerns/verify_profile_concern.rb index 96c957f0ea0..92e0cdb3c43 100644 --- a/app/controllers/concerns/verify_profile_concern.rb +++ b/app/controllers/concerns/verify_profile_concern.rb @@ -17,14 +17,13 @@ def user_backup_codes_configured? end def user_last_signed_in_more_than_5_months_ago? - user = UserDecorator.new(current_user) - second_last_signed_in_at = user.second_last_signed_in_at + second_last_signed_in_at = current_user.second_last_signed_in_at second_last_signed_in_at && second_last_signed_in_at < 5.months.ago end def profile_needs_verification? return false if current_user.blank? - current_user.decorate.pending_profile_requires_verification? || + current_user.pending_profile_requires_verification? || user_needs_to_reactivate_account? end end diff --git a/app/controllers/events_controller.rb b/app/controllers/events_controller.rb index 2a8518c71a4..b05a2ebebcf 100644 --- a/app/controllers/events_controller.rb +++ b/app/controllers/events_controller.rb @@ -12,7 +12,7 @@ def show personal_key: nil, sp_session_request_url: sp_session_request_url_with_updated_params, sp_name: decorated_session.sp_name, - decorated_user: current_user.decorate, + user: current_user, locked_for_session: pii_locked_for_session?(current_user), ) device_and_events diff --git a/app/controllers/idv/come_back_later_controller.rb b/app/controllers/idv/come_back_later_controller.rb index f51840f782b..080783cae48 100644 --- a/app/controllers/idv/come_back_later_controller.rb +++ b/app/controllers/idv/come_back_later_controller.rb @@ -12,7 +12,7 @@ def show private def confirm_user_needs_gpo_confirmation - redirect_to account_url unless current_user.decorate.pending_profile_requires_verification? + redirect_to account_url unless current_user.pending_profile_requires_verification? end end end diff --git a/app/controllers/idv/doc_auth_controller.rb b/app/controllers/idv/doc_auth_controller.rb index a26e6431bf0..4f85ca3088c 100644 --- a/app/controllers/idv/doc_auth_controller.rb +++ b/app/controllers/idv/doc_auth_controller.rb @@ -33,7 +33,7 @@ def return_to_sp end def redirect_if_pending_profile - redirect_to idv_gpo_verify_url if current_user.decorate.pending_profile_requires_verification? + redirect_to idv_gpo_verify_url if current_user.pending_profile_requires_verification? end def redirect_if_flow_completed diff --git a/app/controllers/idv/gpo_controller.rb b/app/controllers/idv/gpo_controller.rb index 6bf55c01dda..4d243172618 100644 --- a/app/controllers/idv/gpo_controller.rb +++ b/app/controllers/idv/gpo_controller.rb @@ -57,7 +57,7 @@ def update_tracking end def resend_requested? - current_user.decorate.pending_profile_requires_verification? + current_user.pending_profile_requires_verification? end def confirm_mail_not_spammed @@ -68,7 +68,7 @@ def confirm_mail_not_spammed def confirm_user_completed_idv_profile_step # If the user has a pending profile, they may have completed idv in a # different session and need a letter resent now - return if current_user.decorate.pending_profile_requires_verification? + return if current_user.pending_profile_requires_verification? return if idv_session.verify_info_step_complete? redirect_to idv_doc_auth_url diff --git a/app/controllers/idv/gpo_verify_controller.rb b/app/controllers/idv/gpo_verify_controller.rb index 3a3e23ece7c..d39ad514cba 100644 --- a/app/controllers/idv/gpo_verify_controller.rb +++ b/app/controllers/idv/gpo_verify_controller.rb @@ -100,7 +100,7 @@ def params_otp end def confirm_verification_needed - return if current_user.decorate.pending_profile_requires_verification? + return if current_user.pending_profile_requires_verification? redirect_to account_url end diff --git a/app/controllers/idv/otp_verification_controller.rb b/app/controllers/idv/otp_verification_controller.rb index e3cd1fd9468..a1edb3c594a 100644 --- a/app/controllers/idv/otp_verification_controller.rb +++ b/app/controllers/idv/otp_verification_controller.rb @@ -61,7 +61,7 @@ def set_otp_verification_presenter end def handle_otp_confirmation_failure - if decorated_user.locked_out? + if current_user.locked_out? handle_too_many_otp_attempts else flash.now[:error] = t('two_factor_authentication.invalid_otp') diff --git a/app/controllers/idv/personal_key_controller.rb b/app/controllers/idv/personal_key_controller.rb index 6f94b3ba8c7..15255b35792 100644 --- a/app/controllers/idv/personal_key_controller.rb +++ b/app/controllers/idv/personal_key_controller.rb @@ -52,7 +52,7 @@ def confirm_profile_has_been_created end def confirm_user_not_pending_gpo_verificaiton - return unless current_user.decorate.pending_profile_requires_verification? + return unless current_user.pending_profile_requires_verification? redirect_to idv_come_back_later_url end diff --git a/app/controllers/idv_controller.rb b/app/controllers/idv_controller.rb index 984c268ccdf..ae73166a2cc 100644 --- a/app/controllers/idv_controller.rb +++ b/app/controllers/idv_controller.rb @@ -9,7 +9,7 @@ class IdvController < ApplicationController def index if decorated_session.requested_more_recent_verification? || - current_user.decorate.reproof_for_irs?(service_provider: current_sp) + current_user.reproof_for_irs?(service_provider: current_sp) verify_identity elsif active_profile? redirect_to idv_activated_url diff --git a/app/controllers/openid_connect/authorization_controller.rb b/app/controllers/openid_connect/authorization_controller.rb index e73b32eb287..fd65cd7718c 100644 --- a/app/controllers/openid_connect/authorization_controller.rb +++ b/app/controllers/openid_connect/authorization_controller.rb @@ -110,9 +110,9 @@ def track_authorize_analytics(result) def identity_needs_verification? (@authorize_form.ial2_requested? && - (current_user.decorate.identity_not_verified? || + (current_user.identity_not_verified? || decorated_session.requested_more_recent_verification?)) || - current_user.decorate.reproof_for_irs?(service_provider: current_sp) + current_user.reproof_for_irs?(service_provider: current_sp) end def build_authorize_form_from_params @@ -167,7 +167,7 @@ def store_request def pii_requested_but_locked? sp_session && sp_session_ial > 1 && - UserDecorator.new(current_user).identity_verified? && + current_user.identity_verified? && !Pii::Cacher.new(current_user, user_session).exists_in_session? end diff --git a/app/controllers/saml_idp_controller.rb b/app/controllers/saml_idp_controller.rb index 3c277eb5e70..876df1d76a9 100644 --- a/app/controllers/saml_idp_controller.rb +++ b/app/controllers/saml_idp_controller.rb @@ -112,7 +112,7 @@ def ialmax_requested_with_ial2_user? end def identity_needs_decryption? - UserDecorator.new(current_user).identity_verified? && + current_user.identity_verified? && !Pii::Cacher.new(current_user, user_session).exists_in_session? end diff --git a/app/controllers/sign_up/completions_controller.rb b/app/controllers/sign_up/completions_controller.rb index d8d47742c5c..69e00c2d1ea 100644 --- a/app/controllers/sign_up/completions_controller.rb +++ b/app/controllers/sign_up/completions_controller.rb @@ -34,7 +34,7 @@ def update private def verify_confirmed - redirect_to idv_url if current_user.decorate.identity_not_verified? + redirect_to idv_url if current_user.identity_not_verified? end def verify_needs_completions_screen diff --git a/app/controllers/two_factor_authentication/backup_code_verification_controller.rb b/app/controllers/two_factor_authentication/backup_code_verification_controller.rb index 61b369f7eae..ad325e0ec94 100644 --- a/app/controllers/two_factor_authentication/backup_code_verification_controller.rb +++ b/app/controllers/two_factor_authentication/backup_code_verification_controller.rb @@ -52,7 +52,7 @@ def handle_invalid_backup_code flash.now[:error] = t('two_factor_authentication.invalid_backup_code') - if decorated_user.locked_out? + if current_user.locked_out? handle_second_factor_locked_user(context: context, type: 'backup_code') else render_show_after_invalid diff --git a/app/controllers/two_factor_authentication/personal_key_verification_controller.rb b/app/controllers/two_factor_authentication/personal_key_verification_controller.rb index ecd816619f4..90084452fe1 100644 --- a/app/controllers/two_factor_authentication/personal_key_verification_controller.rb +++ b/app/controllers/two_factor_authentication/personal_key_verification_controller.rb @@ -52,7 +52,7 @@ def alert_user_about_personal_key_sign_in(disavowal_token) def generate_new_personal_key_for_verified_users_otherwise_retire_the_key_and_ensure_two_mfa if password_reset_profile.present? re_encrypt_profile_recovery_pii - elsif decorated_user.identity_verified? + elsif current_user.identity_verified? user_session[:personal_key] = PersonalKeyGenerator.new(current_user).create else remove_personal_key @@ -73,7 +73,7 @@ def re_encrypt_profile_recovery_pii end def password_reset_profile - @password_reset_profile ||= current_user.decorate.password_reset_profile + @password_reset_profile ||= current_user.password_reset_profile end def pii @@ -90,7 +90,7 @@ def normalized_personal_key def handle_valid_otp handle_valid_otp_for_authentication_context(auth_method: 'personal_key') - if decorated_user.identity_verified? || decorated_user.password_reset_profile.present? + if current_user.identity_verified? || current_user.password_reset_profile.present? redirect_to manage_personal_key_url elsif MfaPolicy.new(current_user).two_factor_enabled? redirect_to after_mfa_setup_path diff --git a/app/controllers/users/passwords_controller.rb b/app/controllers/users/passwords_controller.rb index e3fa59297c3..0554c7096a0 100644 --- a/app/controllers/users/passwords_controller.rb +++ b/app/controllers/users/passwords_controller.rb @@ -34,7 +34,7 @@ def update private def capture_password_if_pii_requested_but_locked - return unless current_user.decorate.identity_verified? && + return unless current_user.identity_verified? && !Pii::Cacher.new(current_user, user_session).exists_in_session? user_session[:stored_location] = request.url redirect_to capture_password_url diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index d845e4a7f58..6e34fe9d080 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -129,7 +129,7 @@ def auth_params def process_locked_out_user presenter = TwoFactorAuthCode::MaxAttemptsReachedPresenter.new( 'generic_login_attempts', - current_user.decorate, + current_user, ) sign_out render_full_width('two_factor_authentication/_locked', locals: { presenter: presenter }) @@ -189,7 +189,7 @@ def user_signed_in_and_not_locked_out?(user) end def user_locked_out?(user) - UserDecorator.new(user).locked_out? + user.locked_out? end def store_sp_metadata_in_session diff --git a/app/controllers/users/totp_setup_controller.rb b/app/controllers/users/totp_setup_controller.rb index 542b0b0a303..131b293c761 100644 --- a/app/controllers/users/totp_setup_controller.rb +++ b/app/controllers/users/totp_setup_controller.rb @@ -22,7 +22,7 @@ def new track_event @code = new_totp_secret - @qrcode = current_user.decorate.qrcode(new_totp_secret) + @qrcode = current_user.qrcode(new_totp_secret) end def confirm diff --git a/app/controllers/users/two_factor_authentication_controller.rb b/app/controllers/users/two_factor_authentication_controller.rb index b413a5bc881..765980cc1cd 100644 --- a/app/controllers/users/two_factor_authentication_controller.rb +++ b/app/controllers/users/two_factor_authentication_controller.rb @@ -176,7 +176,7 @@ def reauthn_param end def handle_valid_otp_params(otp_delivery_selection_result, method, default = nil) - otp_rate_limiter.reset_count_and_otp_last_sent_at if decorated_user.no_longer_locked_out? + otp_rate_limiter.reset_count_and_otp_last_sent_at if current_user.no_longer_locked_out? if exceeded_otp_send_limit? return handle_too_many_otp_sends( diff --git a/app/decorators/user_decorator.rb b/app/decorators/user_decorator.rb deleted file mode 100644 index e776884e154..00000000000 --- a/app/decorators/user_decorator.rb +++ /dev/null @@ -1,143 +0,0 @@ -class UserDecorator - include ActionView::Helpers::DateHelper - - delegate :pending_profile, to: :user - - attr_reader :user - - MAX_RECENT_EVENTS = 5 - MAX_RECENT_DEVICES = 5 - - def initialize(user) - @user = user - end - - def email_language_preference_description - if I18n.locale_available?(user.email_language) - # i18n-tasks-use t('account.email_language.name.en') - # i18n-tasks-use t('account.email_language.name.es') - # i18n-tasks-use t('account.email_language.name.fr') - I18n.t("account.email_language.name.#{user.email_language}") - else - I18n.t('account.email_language.name.en') - end - end - - def visible_email_addresses - user.email_addresses.filter do |email_address| - email_address.confirmed? || !email_address.confirmation_period_expired? - end - end - - def lockout_time_expiration - user.second_factor_locked_at + lockout_period - end - - def active_identity_for(service_provider) - user.active_identities.find_by(service_provider: service_provider.issuer) - end - - def active_or_pending_profile - user.active_profile || pending_profile - end - - def pending_profile_requires_verification? - return false if pending_profile.blank? - return true if identity_not_verified? - return false if active_profile_newer_than_pending_profile? - true - end - - def identity_not_verified? - !identity_verified? - end - - def identity_verified?(service_provider: nil) - user.active_profile.present? && !reproof_for_irs?(service_provider: service_provider) - end - - def reproof_for_irs?(service_provider:) - return false unless service_provider&.irs_attempts_api_enabled - return false unless user.active_profile.present? - !user.active_profile.initiating_service_provider&.irs_attempts_api_enabled - end - - def active_profile_newer_than_pending_profile? - user.active_profile.activated_at >= pending_profile.created_at - end - - # This user's most recently activated profile that has also been deactivated - # due to a password reset, or nil if there is no such profile - def password_reset_profile - profile = user.profiles.where.not(activated_at: nil).order(activated_at: :desc).first - profile if profile&.password_reset? - end - - def qrcode(otp_secret_key) - options = { - issuer: APP_NAME, - otp_secret_key: otp_secret_key, - digits: TwoFactorAuthenticatable::OTP_LENGTH, - interval: IdentityConfig.store.totp_code_interval, - } - url = ROTP::TOTP.new(otp_secret_key, options).provisioning_uri( - EmailContext.new(user).last_sign_in_email_address.email, - ) - qrcode = RQRCode::QRCode.new(url) - qrcode.as_png(size: 240).to_data_url - end - - def locked_out? - user.second_factor_locked_at.present? && !lockout_period_expired? - end - - def no_longer_locked_out? - user.second_factor_locked_at.present? && lockout_period_expired? - end - - def recent_events - events = Event.where(user_id: user.id).order('created_at DESC').limit(MAX_RECENT_EVENTS). - map(&:decorate) - (events + identity_events).sort_by(&:happened_at).reverse - end - - def identity_events - user.identities.includes(:service_provider_record).order('last_authenticated_at DESC') - end - - def recent_devices - @recent_devices ||= user.devices.order(last_used_at: :desc).limit(MAX_RECENT_DEVICES). - map(&:decorate) - end - - def devices? - !recent_devices.empty? - end - - def second_last_signed_in_at - user.events.where(event_type: 'sign_in_after_2fa'). - order(created_at: :desc).limit(2).pluck(:created_at).second - end - - def connected_apps - user.identities.not_deleted.includes(:service_provider_record).order('created_at DESC') - end - - def delete_account_bullet_key - if identity_verified? - I18n.t('users.delete.bullet_2_verified', app_name: APP_NAME) - else - I18n.t('users.delete.bullet_2_basic', app_name: APP_NAME) - end - end - - private - - def lockout_period - IdentityConfig.store.lockout_period_in_minutes.minutes - end - - def lockout_period_expired? - lockout_time_expiration < Time.zone.now - end -end diff --git a/app/forms/verify_password_form.rb b/app/forms/verify_password_form.rb index 8786a45cc2c..d894f187dc2 100644 --- a/app/forms/verify_password_form.rb +++ b/app/forms/verify_password_form.rb @@ -40,6 +40,6 @@ def reencrypt_pii end def profile - @profile ||= user.decorate.password_reset_profile + @profile ||= user.password_reset_profile end end diff --git a/app/forms/verify_personal_key_form.rb b/app/forms/verify_personal_key_form.rb index 8e6c8a4b867..1c4fdfdbfb5 100644 --- a/app/forms/verify_personal_key_form.rb +++ b/app/forms/verify_personal_key_form.rb @@ -30,7 +30,7 @@ def decrypted_pii private def password_reset_profile - user.decorate.password_reset_profile + user.password_reset_profile end def validate_personal_key diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 6ca3d9e78b2..419da9fe85e 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -90,7 +90,7 @@ def reset_password_instructions(token:, request_id:) @locale = locale_url_param @token = token @request_id = request_id - @pending_profile_requires_verification = user.decorate.pending_profile_requires_verification? + @pending_profile_requires_verification = user.pending_profile_requires_verification? @hide_title = @pending_profile_requires_verification mail(to: email_address.email, subject: t('user_mailer.reset_password_instructions.subject')) end diff --git a/app/models/anonymous_user.rb b/app/models/anonymous_user.rb index 7d40550945c..c53d6d720e2 100644 --- a/app/models/anonymous_user.rb +++ b/app/models/anonymous_user.rb @@ -44,4 +44,8 @@ def email_addresses def confirmed_at Time.zone.now end + + def locked_out? + second_factor_locked_at.present? && !lockout_period_expired? + end end diff --git a/app/models/user.rb b/app/models/user.rb index 021728c8381..9a5aec41fc1 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -2,6 +2,7 @@ class User < ApplicationRecord include NonNullUuid include ::NewRelic::Agent::MethodTracer + include ActionView::Helpers::DateHelper devise( :database_authenticatable, @@ -19,6 +20,9 @@ class User < ApplicationRecord include DeprecatedUserAttributes include UserOtpMethods + MAX_RECENT_EVENTS = 5 + MAX_RECENT_DEVICES = 5 + enum otp_delivery_preference: { sms: 0, voice: 1 } # rubocop:disable Rails/HasManyOrHasOneDependent @@ -216,10 +220,129 @@ def send_devise_notification(notification, *args) devise_mailer.send(notification, self, *args).deliver_now_or_later end - def decorate - UserDecorator.new(self) + # + # Decoration methods + # + def email_language_preference_description + if I18n.locale_available?(email_language) + # i18n-tasks-use t('account.email_language.name.en') + # i18n-tasks-use t('account.email_language.name.es') + # i18n-tasks-use t('account.email_language.name.fr') + I18n.t("account.email_language.name.#{email_language}") + else + I18n.t('account.email_language.name.en') + end + end + + def visible_email_addresses + email_addresses.filter do |email_address| + email_address.confirmed? || !email_address.confirmation_period_expired? + end + end + + def lockout_time_expiration + second_factor_locked_at + lockout_period + end + + def active_identity_for(service_provider) + active_identities.find_by(service_provider: service_provider.issuer) + end + + def active_or_pending_profile + active_profile || pending_profile end + def pending_profile_requires_verification? + return false if pending_profile.blank? + return true if identity_not_verified? + return false if active_profile_newer_than_pending_profile? + true + end + + def identity_not_verified? + !identity_verified? + end + + def identity_verified?(service_provider: nil) + active_profile.present? && !reproof_for_irs?(service_provider: service_provider) + end + + def reproof_for_irs?(service_provider:) + return false unless service_provider&.irs_attempts_api_enabled + return false unless active_profile.present? + !active_profile.initiating_service_provider&.irs_attempts_api_enabled + end + + def active_profile_newer_than_pending_profile? + active_profile.activated_at >= pending_profile.created_at + end + + # This user's most recently activated profile that has also been deactivated + # due to a password reset, or nil if there is no such profile + def password_reset_profile + profile = profiles.where.not(activated_at: nil).order(activated_at: :desc).first + profile if profile&.password_reset? + end + + def qrcode(otp_secret_key) + options = { + issuer: APP_NAME, + otp_secret_key: otp_secret_key, + digits: TwoFactorAuthenticatable::OTP_LENGTH, + interval: IdentityConfig.store.totp_code_interval, + } + url = ROTP::TOTP.new(otp_secret_key, options).provisioning_uri( + EmailContext.new(self).last_sign_in_email_address.email, + ) + qrcode = RQRCode::QRCode.new(url) + qrcode.as_png(size: 240).to_data_url + end + + def locked_out? + second_factor_locked_at.present? && !lockout_period_expired? + end + + def no_longer_locked_out? + second_factor_locked_at.present? && lockout_period_expired? + end + + def recent_events + events = Event.where(user_id: id).order('created_at DESC').limit(MAX_RECENT_EVENTS). + map(&:decorate) + (events + identity_events).sort_by(&:happened_at).reverse + end + + def identity_events + identities.includes(:service_provider_record).order('last_authenticated_at DESC') + end + + def recent_devices + @recent_devices ||= devices.order(last_used_at: :desc).limit(MAX_RECENT_DEVICES). + map(&:decorate) + end + + def recent_devices? + !recent_devices.empty? + end + + def second_last_signed_in_at + events.where(event_type: 'sign_in_after_2fa'). + order(created_at: :desc).limit(2).pluck(:created_at).second + end + + def connected_apps + identities.not_deleted.includes(:service_provider_record).order('created_at DESC') + end + + def delete_account_bullet_key + if identity_verified? + I18n.t('users.delete.bullet_2_verified', app_name: APP_NAME) + else + I18n.t('users.delete.bullet_2_basic', app_name: APP_NAME) + end + end + # End moved from UserDecorator + # Devise automatically downcases and strips any attribute defined in # config.case_insensitive_keys and config.strip_whitespace_keys via # before_validation callbacks. Email is included by default, which means that @@ -256,4 +379,14 @@ def send_confirmation_instructions end add_method_tracer :send_devise_notification, "Custom/#{name}/send_devise_notification" + + private + + def lockout_period + IdentityConfig.store.lockout_period_in_minutes.minutes + end + + def lockout_period_expired? + lockout_time_expiration < Time.zone.now + end end diff --git a/app/presenters/account_show_presenter.rb b/app/presenters/account_show_presenter.rb index 7258048ed24..082245ce551 100644 --- a/app/presenters/account_show_presenter.rb +++ b/app/presenters/account_show_presenter.rb @@ -1,12 +1,12 @@ class AccountShowPresenter - attr_reader :decorated_user, :decrypted_pii, :personal_key, :locked_for_session, :pii, + attr_reader :user, :decrypted_pii, :personal_key, :locked_for_session, :pii, :sp_session_request_url, :sp_name - def initialize(decrypted_pii:, personal_key:, sp_session_request_url:, sp_name:, decorated_user:, + def initialize(decrypted_pii:, personal_key:, sp_session_request_url:, sp_name:, user:, locked_for_session:) @decrypted_pii = decrypted_pii @personal_key = personal_key - @decorated_user = decorated_user + @user = user @sp_name = sp_name @sp_session_request_url = sp_session_request_url @locked_for_session = locked_for_session @@ -18,16 +18,16 @@ def show_personal_key_partial? end def show_password_reset_partial? - decorated_user.password_reset_profile.present? + user.password_reset_profile.present? end def show_pii_partial? - decrypted_pii.present? || decorated_user.identity_verified? + decrypted_pii.present? || user.identity_verified? end def show_manage_personal_key_partial? - decorated_user.user.encrypted_recovery_code_digest.present? && - decorated_user.password_reset_profile.blank? + user.encrypted_recovery_code_digest.present? && + user.password_reset_profile.blank? end def show_service_provider_continue_partial? @@ -35,7 +35,7 @@ def show_service_provider_continue_partial? end def show_gpo_partial? - decorated_user.pending_profile_requires_verification? + user.pending_profile_requires_verification? end def showing_any_partials? @@ -46,11 +46,11 @@ def showing_any_partials? end def show_unphishable_badge? - MfaPolicy.new(decorated_user.user).unphishable? + MfaPolicy.new(user).unphishable? end def show_verified_badge? - decorated_user.identity_verified? + user.identity_verified? end def showing_any_badges? @@ -58,28 +58,28 @@ def showing_any_badges? end def backup_codes_generated_at - decorated_user.user.backup_code_configurations.order(created_at: :asc).first&.created_at + user.backup_code_configurations.order(created_at: :asc).first&.created_at end def personal_key_generated_at - decorated_user.user.personal_key_generated_at + user.personal_key_generated_at end def header_personalization return decrypted_pii.first_name if decrypted_pii.present? - EmailContext.new(decorated_user.user).last_sign_in_email_address.email + EmailContext.new(user).last_sign_in_email_address.email end def totp_content - if TwoFactorAuthentication::AuthAppPolicy.new(decorated_user.user).enabled? + if TwoFactorAuthentication::AuthAppPolicy.new(user).enabled? I18n.t('account.index.auth_app_enabled') else I18n.t('account.index.auth_app_disabled') end end - delegate :recent_events, :recent_devices, :connected_apps, to: :decorated_user + delegate :recent_events, :recent_devices, :connected_apps, to: :user private diff --git a/app/presenters/idv/gpo_presenter.rb b/app/presenters/idv/gpo_presenter.rb index 62fd146b1c4..8953690c1cb 100644 --- a/app/presenters/idv/gpo_presenter.rb +++ b/app/presenters/idv/gpo_presenter.rb @@ -23,7 +23,7 @@ def fallback_back_path end def resend_requested? - current_user.decorate.pending_profile_requires_verification? + current_user.pending_profile_requires_verification? end def back_or_cancel_partial diff --git a/app/presenters/two_factor_auth_code/max_attempts_reached_presenter.rb b/app/presenters/two_factor_auth_code/max_attempts_reached_presenter.rb index e6e06fef4fe..0f638ab5719 100644 --- a/app/presenters/two_factor_auth_code/max_attempts_reached_presenter.rb +++ b/app/presenters/two_factor_auth_code/max_attempts_reached_presenter.rb @@ -2,11 +2,11 @@ module TwoFactorAuthCode class MaxAttemptsReachedPresenter include ActionView::Helpers::TranslationHelper - attr_reader :type, :decorated_user + attr_reader :type, :user - def initialize(type, decorated_user) + def initialize(type, user) @type = type - @decorated_user = decorated_user + @user = user end def locked_reason diff --git a/app/services/attribute_asserter.rb b/app/services/attribute_asserter.rb index e4b4737a01f..d55d0322c5a 100644 --- a/app/services/attribute_asserter.rb +++ b/app/services/attribute_asserter.rb @@ -146,7 +146,7 @@ def add_x509(attrs) def uuid_getter_function lambda do |principal| - identity = principal.decorate.active_identity_for(service_provider) + identity = principal.active_identity_for(service_provider) AgencyIdentityLinker.new(identity).link_identity.uuid end end diff --git a/app/services/flow/flow_state_machine.rb b/app/services/flow/flow_state_machine.rb index 5ffd4358c86..b443966a361 100644 --- a/app/services/flow/flow_state_machine.rb +++ b/app/services/flow/flow_state_machine.rb @@ -176,7 +176,7 @@ def analytics_properties step: current_step, step_count: current_flow_step_counts[current_step_name], analytics_id: @analytics_id, - irs_reproofing: effective_user&.decorate&.reproof_for_irs?( + irs_reproofing: effective_user&.reproof_for_irs?( service_provider: current_sp, ).present?, }.merge(flow.extra_analytics_properties) diff --git a/app/services/idv/send_phone_confirmation_otp.rb b/app/services/idv/send_phone_confirmation_otp.rb index 94226916c7f..e95bf536146 100644 --- a/app/services/idv/send_phone_confirmation_otp.rb +++ b/app/services/idv/send_phone_confirmation_otp.rb @@ -8,7 +8,7 @@ def initialize(user:, idv_session:) end def call - otp_rate_limiter.reset_count_and_otp_last_sent_at if user.decorate.no_longer_locked_out? + otp_rate_limiter.reset_count_and_otp_last_sent_at if user.no_longer_locked_out? return too_many_otp_sends_response if rate_limit_exceeded? otp_rate_limiter.increment diff --git a/app/services/user_event_creator.rb b/app/services/user_event_creator.rb index fc90b3ecf77..4408ae71b71 100644 --- a/app/services/user_event_creator.rb +++ b/app/services/user_event_creator.rb @@ -76,7 +76,7 @@ def build_disavowal_token # @return [Array(Event, String)] an (event, disavowal_token) tuple def create_event_for_new_device(event_type:, user:, disavowal_token:) - user_has_multiple_devices = UserDecorator.new(user).devices? + user_has_multiple_devices = user.recent_devices? if user_has_multiple_devices && disavowal_token.nil? device, event, disavowal_token = Device.transaction do diff --git a/app/views/accounts/_backup_codes.html.erb b/app/views/accounts/_backup_codes.html.erb index 88a3048b0b2..61dc0be4551 100644 --- a/app/views/accounts/_backup_codes.html.erb +++ b/app/views/accounts/_backup_codes.html.erb @@ -18,7 +18,7 @@ <% end %> -<% if TwoFactorAuthentication::BackupCodePolicy.new(@presenter.decorated_user.user).configured? %> +<% if TwoFactorAuthentication::BackupCodePolicy.new(@presenter.user).configured? %> <%= render 'accounts/actions/regenerate_backup_codes' %> <% else %> <%= render 'accounts/actions/generate_backup_codes' %> diff --git a/app/views/accounts/_emails.html.erb b/app/views/accounts/_emails.html.erb index 8652ef5211b..06288236f59 100644 --- a/app/views/accounts/_emails.html.erb +++ b/app/views/accounts/_emails.html.erb @@ -7,7 +7,7 @@ <%= render AlertComponent.new(type: :error, id: 'emails', class: 'margin-bottom-1', message: flash[:email_error]) %> <% end %> - <% @presenter.decorated_user.visible_email_addresses.each do |email| %> + <% @presenter.user.visible_email_addresses.each do |email| %>
diff --git a/app/views/accounts/show.html.erb b/app/views/accounts/show.html.erb index cab1f4abe69..77e94d34a4b 100644 --- a/app/views/accounts/show.html.erb +++ b/app/views/accounts/show.html.erb @@ -30,7 +30,7 @@

<%= t('i18n.language') %>

- <%= @presenter.decorated_user.email_language_preference_description %> + <%= @presenter.user.email_language_preference_description %>
<%= link_to(t('forms.buttons.edit'), account_email_language_path) %> diff --git a/app/views/idv/doc_auth/welcome.html.erb b/app/views/idv/doc_auth/welcome.html.erb index ecfaaa7df30..d426c5c7d99 100644 --- a/app/views/idv/doc_auth/welcome.html.erb +++ b/app/views/idv/doc_auth/welcome.html.erb @@ -8,7 +8,7 @@ intro: t('idv.welcome.no_js_intro', sp_name: decorated_session.sp_name || t('doc_auth.info.no_sp_name')), ) do %> - <% if @current_user&.decorate&.reproof_for_irs?(service_provider: @current_sp) %> + <% if @current_user&.reproof_for_irs?(service_provider: @current_sp) %> <%= render AlertComponent.new( type: :info, message: t('doc_auth.info.irs_reproofing_explanation'), diff --git a/app/views/two_factor_authentication/_locked.html.erb b/app/views/two_factor_authentication/_locked.html.erb index 9cd41b87ec3..0360cf507d2 100644 --- a/app/views/two_factor_authentication/_locked.html.erb +++ b/app/views/two_factor_authentication/_locked.html.erb @@ -9,7 +9,7 @@ <%= t( 'two_factor_authentication.please_try_again_html', countdown: render( - CountdownComponent.new(expiration: presenter.decorated_user.lockout_time_expiration), + CountdownComponent.new(expiration: presenter.user.lockout_time_expiration), ), ) %>

diff --git a/app/views/users/delete/show.html.erb b/app/views/users/delete/show.html.erb index 238265e1bb9..3e15de0aeda 100644 --- a/app/views/users/delete/show.html.erb +++ b/app/views/users/delete/show.html.erb @@ -8,7 +8,7 @@
  • <%= t('users.delete.bullet_1', app_name: APP_NAME) %>
  • -
  • <%= current_user.decorate.delete_account_bullet_key %>
  • +
  • <%= current_user.delete_account_bullet_key %>
  • <%= t('users.delete.bullet_3', app_name: APP_NAME) %>
  • <%= t('users.delete.bullet_4', app_name: APP_NAME) %>
diff --git a/spec/controllers/accounts_controller_spec.rb b/spec/controllers/accounts_controller_spec.rb index ff263a19a8c..716469bdeb6 100644 --- a/spec/controllers/accounts_controller_spec.rb +++ b/spec/controllers/accounts_controller_spec.rb @@ -46,7 +46,7 @@ personal_key: nil, sp_session_request_url: nil, sp_name: nil, - decorated_user: user.decorate, + user: user, locked_for_session: false, ) allow(subject).to receive(:presenter).and_return(presenter) @@ -90,7 +90,7 @@ personal_key: nil, sp_session_request_url: nil, sp_name: nil, - decorated_user: user.decorate, + user: user, locked_for_session: false, ) allow(subject).to receive(:presenter).and_return(presenter) diff --git a/spec/controllers/idv/capture_doc_controller_spec.rb b/spec/controllers/idv/capture_doc_controller_spec.rb index f1d61ab75e2..307866e3ffd 100644 --- a/spec/controllers/idv/capture_doc_controller_spec.rb +++ b/spec/controllers/idv/capture_doc_controller_spec.rb @@ -134,7 +134,7 @@ end it 'tracks expected events for irs reproofing' do - allow_any_instance_of(UserDecorator).to receive(:reproof_for_irs?).and_return(true) + allow_any_instance_of(User).to receive(:reproof_for_irs?).and_return(true) mock_next_step(:capture_complete) result = { step: 'capture_complete', diff --git a/spec/controllers/idv/come_back_later_controller_spec.rb b/spec/controllers/idv/come_back_later_controller_spec.rb index f38e60c3a85..c541d9d7466 100644 --- a/spec/controllers/idv/come_back_later_controller_spec.rb +++ b/spec/controllers/idv/come_back_later_controller_spec.rb @@ -5,10 +5,8 @@ let(:pending_profile_requires_verification) { true } before do - user_decorator = instance_double(UserDecorator) - allow(user_decorator).to receive(:pending_profile_requires_verification?). + allow(user).to receive(:pending_profile_requires_verification?). and_return(pending_profile_requires_verification) - allow(user).to receive(:decorate).and_return(user_decorator) stub_sign_in(user) end diff --git a/spec/controllers/idv/gpo_controller_spec.rb b/spec/controllers/idv/gpo_controller_spec.rb index 9dd60a0662e..2cef9dd43ef 100644 --- a/spec/controllers/idv/gpo_controller_spec.rb +++ b/spec/controllers/idv/gpo_controller_spec.rb @@ -135,8 +135,8 @@ before do stub_sign_in(user) - stub_decorated_user_with_pending_profile(user) - allow(user.decorate).to receive(:pending_profile_requires_verification?).and_return(true) + stub_user_with_pending_profile(user) + allow(user).to receive(:pending_profile_requires_verification?).and_return(true) end it 'calls the GpoConfirmationMaker to send another letter and redirects' do diff --git a/spec/controllers/idv/gpo_verify_controller_spec.rb b/spec/controllers/idv/gpo_verify_controller_spec.rb index 205a2cdfb75..620f4166726 100644 --- a/spec/controllers/idv/gpo_verify_controller_spec.rb +++ b/spec/controllers/idv/gpo_verify_controller_spec.rb @@ -21,13 +21,13 @@ stub_analytics stub_attempts_tracker stub_sign_in(user) - decorated_user = stub_decorated_user_with_pending_profile(user) + pending_user = stub_user_with_pending_profile(user) create( :gpo_confirmation_code, profile: pending_profile, otp_fingerprint: Pii::Fingerprinter.fingerprint(otp), ) - allow(decorated_user).to receive(:pending_profile_requires_verification?). + allow(pending_user).to receive(:pending_profile_requires_verification?). and_return(has_pending_profile) allow(IdentityConfig.store).to receive(:proofing_device_profiling). diff --git a/spec/controllers/users/totp_setup_controller_spec.rb b/spec/controllers/users/totp_setup_controller_spec.rb index 877cf55cfcf..d3ec68f9e10 100644 --- a/spec/controllers/users/totp_setup_controller_spec.rb +++ b/spec/controllers/users/totp_setup_controller_spec.rb @@ -31,9 +31,9 @@ expect(subject.user_session[:new_totp_secret]).not_to be_nil end - it 'can be used to generate a qrcode with UserDecorator#qrcode' do + it 'can be used to generate a qrcode with User#qrcode' do expect( - subject.current_user.decorate.qrcode(subject.user_session[:new_totp_secret]), + subject.current_user.qrcode(subject.user_session[:new_totp_secret]), ).not_to be_nil end @@ -66,9 +66,9 @@ expect(subject.user_session[:new_totp_secret]).not_to be_nil end - it 'can be used to generate a qrcode with UserDecorator#qrcode' do + it 'can be used to generate a qrcode with User#qrcode' do expect( - subject.current_user.decorate.qrcode(subject.user_session[:new_totp_secret]), + subject.current_user.qrcode(subject.user_session[:new_totp_secret]), ).not_to be_nil end diff --git a/spec/decorators/user_decorator_spec.rb b/spec/decorators/user_decorator_spec.rb deleted file mode 100644 index 914eb0ca7b9..00000000000 --- a/spec/decorators/user_decorator_spec.rb +++ /dev/null @@ -1,386 +0,0 @@ -require 'rails_helper' - -describe UserDecorator do - describe '#visible_email_addresses' do - let(:user) { create(:user) } - let(:confirmed_email_address) { user.email_addresses.detect(&:confirmed?) } - let!(:unconfirmed_expired_email_address) do - create( - :email_address, - user: user, - confirmed_at: nil, - confirmation_sent_at: 36.hours.ago, - ) - end - let!(:unconfirmed_unexpired_email_address) do - create( - :email_address, - user: user, - confirmed_at: nil, - confirmation_sent_at: 5.minutes.ago, - ) - end - - subject { described_class.new(user.reload) } - - it 'shows email addresses that have been confirmed' do - expect(subject.visible_email_addresses).to include(confirmed_email_address) - end - - it 'hides emails address that are unconfirmed and expired' do - expect(subject.visible_email_addresses).to_not include(unconfirmed_expired_email_address) - end - - it 'shows emails that are not confirmed and not expired' do - expect(subject.visible_email_addresses).to include(unconfirmed_unexpired_email_address) - end - end - - describe '#email_language_preference_description' do - let(:user) { build_stubbed(:user, email_language: email_language) } - - subject(:description) { UserDecorator.new(user).email_language_preference_description } - - context 'when the user has a supported email_language' do - let(:email_language) { 'es' } - - it 'is the that language' do - expect(description).to eq(I18n.t('account.email_language.name.es')) - end - end - - context 'when the user has a nil email_language' do - let(:email_language) { nil } - - it 'is the default language' do - expect(description).to eq(I18n.t('account.email_language.name.en')) - end - end - - context 'when the user has an unsupported email_language' do - let(:email_language) { 'zz' } - - it 'is the default language' do - expect(description).to eq(I18n.t('account.email_language.name.en')) - end - end - end - - describe '#lockout_time_expiration' do - it 'returns the time at which lockout will expire' do - freeze_time do - user = build_stubbed(:user, second_factor_locked_at: Time.zone.now - 180) - user_decorator = UserDecorator.new(user) - allow(IdentityConfig.store).to receive(:lockout_period_in_minutes).and_return(8) - - expect(user_decorator.lockout_time_expiration).to eq Time.zone.now + 300 - end - end - end - - describe '#active_identity_for' do - it 'returns Identity matching ServiceProvider' do - sp = create(:service_provider, issuer: 'http://sp.example.com') - user = create(:user) - user.identities << create( - :service_provider_identity, - service_provider: sp.issuer, - session_uuid: SecureRandom.uuid, - ) - - user_decorator = UserDecorator.new(user) - - expect(user_decorator.active_identity_for(sp)).to eq user.last_identity - end - end - - describe '#pending_profile_requires_verification?' do - it 'returns false when no pending profile exists' do - user = User.new - user_decorator = UserDecorator.new(user) - allow(user_decorator).to receive(:pending_profile).and_return(nil) - - expect(user_decorator.pending_profile_requires_verification?).to eq false - end - - it 'returns true when pending profile exists and identity is not verified' do - user = User.new - user_decorator = UserDecorator.new(user) - allow(user_decorator).to receive(:pending_profile).and_return('profile') - allow(user_decorator).to receive(:identity_not_verified?).and_return(true) - - expect(user_decorator.pending_profile_requires_verification?).to eq true - end - - it 'returns false when active profile is newer than pending profile' do - user = User.new - user_decorator = UserDecorator.new(user) - allow(user_decorator).to receive(:pending_profile).and_return('profile') - allow(user_decorator).to receive(:identity_not_verified?).and_return(false) - allow(user_decorator).to receive(:active_profile_newer_than_pending_profile?). - and_return(true) - - expect(user_decorator.pending_profile_requires_verification?).to eq false - end - - it 'returns true when pending profile is newer than active profile' do - user = User.new - user_decorator = UserDecorator.new(user) - allow(user_decorator).to receive(:pending_profile).and_return('profile') - allow(user_decorator).to receive(:identity_not_verified?).and_return(false) - allow(user_decorator).to receive(:active_profile_newer_than_pending_profile?). - and_return(false) - - expect(user_decorator.pending_profile_requires_verification?).to eq true - end - end - - describe '#identity_not_verified?' do - it 'returns true if identity_verified returns false' do - user = User.new - user_decorator = UserDecorator.new(user) - allow(user_decorator).to receive(:identity_verified?).and_return(false) - - expect(user_decorator.identity_not_verified?).to eq true - end - - it 'returns false if identity_verified returns true' do - user = User.new - user_decorator = UserDecorator.new(user) - allow(user_decorator).to receive(:identity_verified?).and_return(true) - - expect(user_decorator.identity_not_verified?).to eq false - end - end - - describe '#identity_verified?' do - it 'returns true if user has an active profile' do - user = User.new - user_decorator = UserDecorator.new(user) - allow(user).to receive(:active_profile).and_return(Profile.new) - - expect(user_decorator.identity_verified?).to eq true - end - - it 'returns false if user does not have an active profile' do - user = User.new - user_decorator = UserDecorator.new(user) - allow(user).to receive(:active_profile).and_return(nil) - - expect(user_decorator.identity_verified?).to eq false - end - end - - describe '#active_profile_newer_than_pending_profile?' do - it 'returns true if the active profile is newer than the pending profile' do - user = User.new - user_decorator = UserDecorator.new(user) - allow(user).to receive(:active_profile).and_return(Profile.new(activated_at: Time.zone.now)) - allow(user_decorator).to receive(:pending_profile). - and_return(Profile.new(created_at: 1.day.ago)) - - expect(user_decorator.active_profile_newer_than_pending_profile?).to eq true - end - - it 'returns false if the active profile is older than the pending profile' do - user = User.new - user_decorator = UserDecorator.new(user) - allow(user).to receive(:active_profile).and_return(Profile.new(activated_at: 1.day.ago)) - allow(user_decorator).to receive(:pending_profile). - and_return(Profile.new(created_at: Time.zone.now)) - - expect(user_decorator.active_profile_newer_than_pending_profile?).to eq false - end - end - - describe '#locked_out?' do - let(:locked_at) { nil } - let(:user) { User.new } - - before { allow(user).to receive(:second_factor_locked_at).and_return(locked_at) } - - around do |ex| - freeze_time { ex.run } - end - - subject(:locked_out?) { UserDecorator.new(user).locked_out? } - - it { expect(locked_out?).to eq(false) } - - context 'second factor locked out recently' do - let(:locked_at) { Time.zone.now } - - it { expect(locked_out?).to eq(true) } - end - - context 'second factor locked out a while ago' do - let(:locked_at) { IdentityConfig.store.lockout_period_in_minutes.minutes.ago - 1.second } - - it { expect(locked_out?).to eq(false) } - end - end - - describe '#no_longer_locked_out?' do - let(:locked_at) { nil } - let(:user) { User.new } - - before { allow(user).to receive(:second_factor_locked_at).and_return(locked_at) } - - around do |ex| - freeze_time { ex.run } - end - - subject(:no_longer_locked_out?) { UserDecorator.new(user).no_longer_locked_out? } - - it { expect(no_longer_locked_out?).to eq(false) } - - context 'second factor locked out recently' do - let(:locked_at) { Time.zone.now } - - it { expect(no_longer_locked_out?).to eq(false) } - end - - context 'second factor locked out a while ago' do - let(:locked_at) { IdentityConfig.store.lockout_period_in_minutes.minutes.ago - 1.second } - - it { expect(no_longer_locked_out?).to eq(true) } - end - end - - describe '#recent_events' do - let!(:user) { create(:user, :signed_up, created_at: Time.zone.now - 100.days) } - let(:decorated_user) { user.decorate } - let!(:event) { create(:event, user: user, created_at: Time.zone.now - 98.days) } - let!(:identity) do - create( - :service_provider_identity, - :active, - user: user, - last_authenticated_at: Time.zone.now - 60.days, - ) - end - let!(:another_event) do - create(:event, user: user, event_type: :email_changed, created_at: Time.zone.now - 30.days) - end - - it 'interleaves identities and events, decorates events, and sorts them in descending order' do - expect(decorated_user.recent_events). - to eq [another_event.decorate, identity, event.decorate] - end - end - - describe '#password_reset_profile' do - let(:user) { create(:user) } - subject(:decorated_user) { UserDecorator.new(user) } - - context 'with no profiles' do - it { expect(decorated_user.password_reset_profile).to be_nil } - end - - context 'with an active profile' do - let(:active_profile) do - build(:profile, :active, :verified, activated_at: 1.day.ago, pii: { first_name: 'Jane' }) - end - - before do - user.profiles << [ - active_profile, - build(:profile, :verified, activated_at: 5.days.ago, pii: { first_name: 'Susan' }), - ] - end - - it { expect(decorated_user.password_reset_profile).to be_nil } - - context 'when the active profile is deactivated due to password reset' do - before { active_profile.deactivate(:password_reset) } - - it { expect(decorated_user.password_reset_profile).to eq(active_profile) } - - context 'with a previously-cancelled pending profile' do - before do - user.profiles << build(:profile, :verification_cancelled) - end - - it { expect(decorated_user.password_reset_profile).to eq(active_profile) } - end - end - end - end - - describe '#delete_account_bullet_key' do - let(:user_decorator) { UserDecorator.new(build_stubbed(:user)) } - - it 'returns ial1 if identity is not verified' do - allow(user_decorator).to receive(:identity_verified?).and_return(false) - expect(user_decorator.delete_account_bullet_key). - to eq t('users.delete.bullet_2_basic', app_name: APP_NAME) - end - - it 'returns ial2 if identity is verified' do - allow(user_decorator).to receive(:identity_verified?).and_return(true) - expect(user_decorator.delete_account_bullet_key). - to eq t('users.delete.bullet_2_verified', app_name: APP_NAME) - end - end - - describe '#connected_apps' do - let(:user) { create(:user) } - let(:app) { create(:service_provider_identity, service_provider: 'aaa') } - let(:deleted_app) do - create(:service_provider_identity, service_provider: 'bbb', deleted_at: 5.days.ago) - end - - let(:user_decorator) { user.decorate } - - before { user.identities << app << deleted_app } - - it 'omits deleted apps' do - expect(user_decorator.connected_apps).to eq([app]) - end - end - - describe '#second_last_signed_in_at' do - it 'returns second most recent full authentication event' do - user = create(:user) - _event1 = create(:event, user: user, event_type: 'sign_in_after_2fa') - event2 = create(:event, user: user, event_type: 'sign_in_after_2fa') - _event3 = create(:event, user: user, event_type: 'sign_in_after_2fa') - - expect(user.decorate.second_last_signed_in_at).to eq(event2.reload.created_at) - end - end - - describe '#reproof_for_irs?' do - let(:service_provider) { create(:service_provider) } - - it 'returns false if the service provider is not an attempts API service provider' do - user = create(:user, :proofed) - - expect(user.decorate.reproof_for_irs?(service_provider: service_provider)).to be_falsy - end - - context 'an attempts API service provider' do - let(:service_provider) { create(:service_provider, :irs) } - - it 'returns false if the user has not proofed before' do - user = create(:user) - - expect(user.decorate.reproof_for_irs?(service_provider: service_provider)).to be_falsy - end - - it 'returns false if the active profile initiating SP was an attempts API SP' do - user = create(:user, :proofed) - - user.active_profile.update!(initiating_service_provider: service_provider) - - expect(user.decorate.reproof_for_irs?(service_provider: service_provider)).to be_falsy - end - - it 'returns true if the active profile initiating SP was not an attempts API SP' do - user = create(:user, :proofed) - - expect(user.decorate.reproof_for_irs?(service_provider: service_provider)).to be_truthy - end - end - end -end diff --git a/spec/features/saml/multiple_endpoints_spec.rb b/spec/features/saml/multiple_endpoints_spec.rb index 63ca9f0e2e7..fe0fb9fa67a 100644 --- a/spec/features/saml/multiple_endpoints_spec.rb +++ b/spec/features/saml/multiple_endpoints_spec.rb @@ -51,7 +51,7 @@ click_agree_and_continue service_provider = ServiceProvider.find_by(issuer: endpoint_saml_settings.issuer) - uuid = user.decorate.active_identity_for(service_provider).uuid + uuid = user.active_identity_for(service_provider).uuid endpoint_saml_settings = saml_settings endpoint_saml_settings.name_identifier_value = uuid diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index a02ba8da345..a76c6debfa6 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -335,14 +335,6 @@ end end - describe '#decorate' do - it 'returns a UserDecorator' do - user = build(:user) - - expect(user.decorate).to be_a(UserDecorator) - end - end - describe 'encrypted attributes' do context 'input is MixEd CaSe with whitespace' do it 'normalizes email' do @@ -891,4 +883,368 @@ def it_should_not_send_survey it { expect(user.broken_personal_key?).to eq(true) } end end + + describe '#visible_email_addresses' do + let(:user) { create(:user) } + let(:confirmed_email_address) { user.email_addresses.detect(&:confirmed?) } + let!(:unconfirmed_expired_email_address) do + create( + :email_address, + user: user, + confirmed_at: nil, + confirmation_sent_at: 36.hours.ago, + ) + end + let!(:unconfirmed_unexpired_email_address) do + create( + :email_address, + user: user, + confirmed_at: nil, + confirmation_sent_at: 5.minutes.ago, + ) + end + + it 'shows email addresses that have been confirmed' do + expect(user.visible_email_addresses).to include(confirmed_email_address) + end + + it 'hides emails address that are unconfirmed and expired' do + expect(user.visible_email_addresses).to_not include(unconfirmed_expired_email_address) + end + + it 'shows emails that are not confirmed and not expired' do + expect(user.visible_email_addresses).to include(unconfirmed_unexpired_email_address) + end + end + + describe '#email_language_preference_description' do + let(:user) { build_stubbed(:user, email_language: email_language) } + + subject(:description) { user.email_language_preference_description } + + context 'when the user has a supported email_language' do + let(:email_language) { 'es' } + + it 'is the that language' do + expect(description).to eq(I18n.t('account.email_language.name.es')) + end + end + + context 'when the user has a nil email_language' do + let(:email_language) { nil } + + it 'is the default language' do + expect(description).to eq(I18n.t('account.email_language.name.en')) + end + end + + context 'when the user has an unsupported email_language' do + let(:email_language) { 'zz' } + + it 'is the default language' do + expect(description).to eq(I18n.t('account.email_language.name.en')) + end + end + end + + describe '#lockout_time_expiration' do + it 'returns the time at which lockout will expire' do + freeze_time do + user = build_stubbed(:user, second_factor_locked_at: Time.zone.now - 180) + allow(IdentityConfig.store).to receive(:lockout_period_in_minutes).and_return(8) + + expect(user.lockout_time_expiration).to eq Time.zone.now + 300 + end + end + end + + describe '#active_identity_for' do + it 'returns Identity matching ServiceProvider' do + sp = create(:service_provider, issuer: 'http://sp.example.com') + user = create(:user) + user.identities << create( + :service_provider_identity, + service_provider: sp.issuer, + session_uuid: SecureRandom.uuid, + ) + + expect(user.active_identity_for(sp)).to eq user.last_identity + end + end + + describe '#pending_profile_requires_verification?' do + it 'returns false when no pending profile exists' do + user = User.new + allow(user).to receive(:pending_profile).and_return(nil) + + expect(user.pending_profile_requires_verification?).to eq false + end + + it 'returns true when pending profile exists and identity is not verified' do + user = User.new + allow(user).to receive(:pending_profile).and_return('profile') + allow(user).to receive(:identity_not_verified?).and_return(true) + + expect(user.pending_profile_requires_verification?).to eq true + end + + it 'returns false when active profile is newer than pending profile' do + user = User.new + allow(user).to receive(:pending_profile).and_return('profile') + allow(user).to receive(:identity_not_verified?).and_return(false) + allow(user).to receive(:active_profile_newer_than_pending_profile?). + and_return(true) + + expect(user.pending_profile_requires_verification?).to eq false + end + + it 'returns true when pending profile is newer than active profile' do + user = User.new + + allow(user).to receive(:pending_profile).and_return('profile') + allow(user).to receive(:identity_not_verified?).and_return(false) + allow(user).to receive(:active_profile_newer_than_pending_profile?). + and_return(false) + + expect(user.pending_profile_requires_verification?).to eq true + end + end + + describe '#identity_not_verified?' do + it 'returns true if identity_verified returns false' do + user = User.new + allow(user).to receive(:identity_verified?).and_return(false) + + expect(user.identity_not_verified?).to eq true + end + + it 'returns false if identity_verified returns true' do + user = User.new + allow(user).to receive(:identity_verified?).and_return(true) + + expect(user.identity_not_verified?).to eq false + end + end + + describe '#identity_verified?' do + it 'returns true if user has an active profile' do + user = User.new + allow(user).to receive(:active_profile).and_return(Profile.new) + + expect(user.identity_verified?).to eq true + end + + it 'returns false if user does not have an active profile' do + user = User.new + allow(user).to receive(:active_profile).and_return(nil) + + expect(user.identity_verified?).to eq false + end + end + + describe '#active_profile_newer_than_pending_profile?' do + it 'returns true if the active profile is newer than the pending profile' do + user = User.new + allow(user).to receive(:active_profile).and_return(Profile.new(activated_at: Time.zone.now)) + allow(user).to receive(:pending_profile). + and_return(Profile.new(created_at: 1.day.ago)) + + expect(user.active_profile_newer_than_pending_profile?).to eq true + end + + it 'returns false if the active profile is older than the pending profile' do + user = User.new + allow(user).to receive(:active_profile).and_return(Profile.new(activated_at: 1.day.ago)) + allow(user).to receive(:pending_profile). + and_return(Profile.new(created_at: Time.zone.now)) + + expect(user.active_profile_newer_than_pending_profile?).to eq false + end + end + + describe '#locked_out?' do + let(:locked_at) { nil } + let(:user) { User.new } + + before { allow(user).to receive(:second_factor_locked_at).and_return(locked_at) } + + around do |ex| + freeze_time { ex.run } + end + + it { expect(user.locked_out?).to eq(false) } + + context 'second factor locked out recently' do + let(:locked_at) { Time.zone.now } + + it { expect(user.locked_out?).to eq(true) } + end + + context 'second factor locked out a while ago' do + let(:locked_at) { IdentityConfig.store.lockout_period_in_minutes.minutes.ago - 1.second } + + it { expect(user.locked_out?).to eq(false) } + end + end + + describe '#no_longer_locked_out?' do + let(:locked_at) { nil } + let(:user) { User.new } + + before { allow(user).to receive(:second_factor_locked_at).and_return(locked_at) } + + around do |ex| + freeze_time { ex.run } + end + + subject(:no_longer_locked_out?) { user.no_longer_locked_out? } + + it { expect(no_longer_locked_out?).to eq(false) } + + context 'second factor locked out recently' do + let(:locked_at) { Time.zone.now } + + it { expect(no_longer_locked_out?).to eq(false) } + end + + context 'second factor locked out a while ago' do + let(:locked_at) { IdentityConfig.store.lockout_period_in_minutes.minutes.ago - 1.second } + + it { expect(no_longer_locked_out?).to eq(true) } + end + end + + describe '#recent_events' do + let!(:user) { create(:user, :signed_up, created_at: Time.zone.now - 100.days) } + + let!(:event) { create(:event, user: user, created_at: Time.zone.now - 98.days) } + let!(:identity) do + create( + :service_provider_identity, + :active, + user: user, + last_authenticated_at: Time.zone.now - 60.days, + ) + end + let!(:another_event) do + create(:event, user: user, event_type: :email_changed, created_at: Time.zone.now - 30.days) + end + + it 'interleaves identities and events, decorates events, and sorts them in descending order' do + expect(user.recent_events). + to eq [another_event.decorate, identity, event.decorate] + end + end + + describe '#password_reset_profile' do + let(:user) { create(:user) } + + context 'with no profiles' do + it { expect(user.password_reset_profile).to be_nil } + end + + context 'with an active profile' do + let(:active_profile) do + build(:profile, :active, :verified, activated_at: 1.day.ago, pii: { first_name: 'Jane' }) + end + + before do + user.profiles << [ + active_profile, + build(:profile, :verified, activated_at: 5.days.ago, pii: { first_name: 'Susan' }), + ] + end + + it { expect(user.password_reset_profile).to be_nil } + + context 'when the active profile is deactivated due to password reset' do + before { active_profile.deactivate(:password_reset) } + + it { expect(user.password_reset_profile).to eq(active_profile) } + + context 'with a previously-cancelled pending profile' do + before do + user.profiles << build(:profile, :verification_cancelled) + end + + it { expect(user.password_reset_profile).to eq(active_profile) } + end + end + end + end + + describe '#delete_account_bullet_key' do + let(:user) { build_stubbed(:user) } + + it 'returns ial1 if identity is not verified' do + allow(user).to receive(:identity_verified?).and_return(false) + expect(user.delete_account_bullet_key). + to eq t('users.delete.bullet_2_basic', app_name: APP_NAME) + end + + it 'returns ial2 if identity is verified' do + allow(user).to receive(:identity_verified?).and_return(true) + expect(user.delete_account_bullet_key). + to eq t('users.delete.bullet_2_verified', app_name: APP_NAME) + end + end + + describe '#connected_apps' do + let(:user) { create(:user) } + let(:app) { create(:service_provider_identity, service_provider: 'aaa') } + let(:deleted_app) do + create(:service_provider_identity, service_provider: 'bbb', deleted_at: 5.days.ago) + end + + before { user.identities << app << deleted_app } + + it 'omits deleted apps' do + expect(user.connected_apps).to eq([app]) + end + end + + describe '#second_last_signed_in_at' do + it 'returns second most recent full authentication event' do + user = create(:user) + _event1 = create(:event, user: user, event_type: 'sign_in_after_2fa') + event2 = create(:event, user: user, event_type: 'sign_in_after_2fa') + _event3 = create(:event, user: user, event_type: 'sign_in_after_2fa') + + expect(user.second_last_signed_in_at).to eq(event2.reload.created_at) + end + end + + describe '#reproof_for_irs?' do + let(:service_provider) { create(:service_provider) } + + it 'returns false if the service provider is not an attempts API service provider' do + user = create(:user, :proofed) + + expect(user.reproof_for_irs?(service_provider: service_provider)).to be_falsy + end + + context 'an attempts API service provider' do + let(:service_provider) { create(:service_provider, :irs) } + + it 'returns false if the user has not proofed before' do + user = create(:user) + + expect(user.reproof_for_irs?(service_provider: service_provider)).to be_falsy + end + + it 'returns false if the active profile initiating SP was an attempts API SP' do + user = create(:user, :proofed) + + user.active_profile.update!(initiating_service_provider: service_provider) + + expect(user.reproof_for_irs?(service_provider: service_provider)).to be_falsy + end + + it 'returns true if the active profile initiating SP was not an attempts API SP' do + user = create(:user, :proofed) + + expect(user.reproof_for_irs?(service_provider: service_provider)).to be_truthy + end + end + end end diff --git a/spec/presenters/account_show_presenter_spec.rb b/spec/presenters/account_show_presenter_spec.rb index a1ee856aae0..47a8bd28fad 100644 --- a/spec/presenters/account_show_presenter_spec.rb +++ b/spec/presenters/account_show_presenter_spec.rb @@ -13,7 +13,7 @@ dob: birthday ) profile_index = AccountShowPresenter.new( - decrypted_pii: decrypted_pii, personal_key: '', decorated_user: user.decorate, + decrypted_pii: decrypted_pii, personal_key: '', user: user, sp_session_request_url: nil, sp_name: nil, locked_for_session: false ) @@ -24,11 +24,11 @@ context 'AccountShowPresenter instance does not have decrypted_pii' do it 'returns the email the user used to sign in last' do - decorated_user = create(:user, :with_multiple_emails).decorate - email_address = decorated_user.user.reload.email_addresses.last + user = create(:user, :with_multiple_emails) + email_address = user.reload.email_addresses.last email_address.update!(last_sign_in_at: 1.minute.from_now) profile_index = AccountShowPresenter.new( - decrypted_pii: {}, personal_key: '', decorated_user: decorated_user, + decrypted_pii: {}, personal_key: '', user: user, sp_session_request_url: nil, sp_name: nil, locked_for_session: false ) @@ -47,7 +47,7 @@ ).to receive(:enabled?).and_return(true) profile_index = AccountShowPresenter.new( - decrypted_pii: {}, personal_key: '', decorated_user: user.decorate, + decrypted_pii: {}, personal_key: '', user: user, sp_session_request_url: nil, sp_name: nil, locked_for_session: false ) @@ -58,12 +58,12 @@ context 'user does not have an authenticator app enabled' do it 'returns localization for auth_app_disabled' do - user = User.new.decorate + user = User.new allow_any_instance_of( TwoFactorAuthentication::AuthAppPolicy, ).to receive(:enabled?).and_return(false) profile_index = AccountShowPresenter.new( - decrypted_pii: {}, personal_key: '', decorated_user: user, + decrypted_pii: {}, personal_key: '', user: user, sp_session_request_url: nil, sp_name: nil, locked_for_session: false ) @@ -84,7 +84,7 @@ personal_key: '', sp_session_request_url: nil, sp_name: nil, - decorated_user: user.reload.decorate, + user: user.reload, locked_for_session: false, ) @@ -103,7 +103,7 @@ personal_key: '', sp_session_request_url: nil, sp_name: nil, - decorated_user: user.reload.decorate, + user: user.reload, locked_for_session: false, ) @@ -123,7 +123,7 @@ personal_key: '', sp_session_request_url: nil, sp_name: nil, - decorated_user: user.decorate, + user: user, locked_for_session: false, ) end @@ -159,7 +159,7 @@ profile_index = AccountShowPresenter.new( decrypted_pii: {}, personal_key: '', - decorated_user: user.decorate, + user: user, sp_session_request_url: nil, sp_name: nil, locked_for_session: false, @@ -183,7 +183,7 @@ profile_index = AccountShowPresenter.new( decrypted_pii: {}, personal_key: '', - decorated_user: user.decorate, + user: user, sp_session_request_url: nil, sp_name: nil, locked_for_session: false, @@ -202,7 +202,7 @@ profile_index = AccountShowPresenter.new( decrypted_pii: {}, personal_key: '', - decorated_user: user.decorate, + user: user, sp_session_request_url: nil, sp_name: nil, locked_for_session: false, diff --git a/spec/presenters/max_attempts_reached_presenter_spec.rb b/spec/presenters/max_attempts_reached_presenter_spec.rb index dd360840f04..ab95a10c046 100644 --- a/spec/presenters/max_attempts_reached_presenter_spec.rb +++ b/spec/presenters/max_attempts_reached_presenter_spec.rb @@ -2,8 +2,8 @@ describe TwoFactorAuthCode::MaxAttemptsReachedPresenter do let(:type) { 'otp_requests' } - let(:decorated_user) { instance_double(UserDecorator) } - let(:presenter) { described_class.new(type, decorated_user) } + let(:user) { instance_double(User) } + let(:presenter) { described_class.new(type, user) } describe '#type' do subject { presenter.type } @@ -11,10 +11,10 @@ it { is_expected.to eq(type) } end - describe '#decorated_user' do - subject { presenter.decorated_user } + describe '#user' do + subject { presenter.user } - it { is_expected.to eq(decorated_user) } + it { is_expected.to eq(user) } end describe '#locked_reason' do diff --git a/spec/support/controller_helper.rb b/spec/support/controller_helper.rb index 2fb9be5bd67..644446babbd 100644 --- a/spec/support/controller_helper.rb +++ b/spec/support/controller_helper.rb @@ -87,14 +87,12 @@ def stub_user_with_applicant_data(user, applicant) allow(subject).to receive(:user_session).and_return(user_session) end - def stub_decorated_user_with_pending_profile(user) - decorated_user = instance_double(UserDecorator) - allow(user).to receive(:decorate).and_return(decorated_user) + def stub_user_with_pending_profile(user) allow(user).to receive(:pending_profile).and_return(pending_profile) - allow(decorated_user).to receive(:pending_profile_requires_verification?). + allow(user).to receive(:pending_profile_requires_verification?). and_return(has_pending_profile) allow(user).to receive(:fraud_review_pending?).and_return(false) - decorated_user + user end def stub_identity(user, params) diff --git a/spec/support/idv_examples/clearing_and_restarting.rb b/spec/support/idv_examples/clearing_and_restarting.rb index be194690714..ac51ba7e3af 100644 --- a/spec/support/idv_examples/clearing_and_restarting.rb +++ b/spec/support/idv_examples/clearing_and_restarting.rb @@ -10,7 +10,7 @@ acknowledge_and_confirm_personal_key expect(page).to have_current_path(sign_up_completed_path) - expect(user.reload.decorate.identity_verified?).to eq(true) + expect(user.reload.identity_verified?).to eq(true) end it 'allows the user to retry verification with gpo', js: true do @@ -32,7 +32,7 @@ expect(page).to have_content(t('idv.titles.come_back_later')) expect(page).to have_current_path(idv_come_back_later_path) - expect(user.reload.decorate.identity_verified?).to eq(false) + expect(user.reload.identity_verified?).to eq(false) expect(user.pending_profile?).to eq(true) expect(gpo_confirmation.entry[:address1]).to eq('1 FAKE RD') end diff --git a/spec/views/accounts/connected_accounts/show.html.erb_spec.rb b/spec/views/accounts/connected_accounts/show.html.erb_spec.rb index 9fafa1526eb..99bbbf4e793 100644 --- a/spec/views/accounts/connected_accounts/show.html.erb_spec.rb +++ b/spec/views/accounts/connected_accounts/show.html.erb_spec.rb @@ -1,15 +1,13 @@ require 'rails_helper' describe 'accounts/connected_accounts/show.html.erb' do let(:user) { create(:user, :signed_up, :with_personal_key) } - let(:decorated_user) { user.decorate } before do - allow(user).to receive(:decorate).and_return(decorated_user) allow(view).to receive(:current_user).and_return(user) assign( :presenter, AccountShowPresenter.new( - decrypted_pii: nil, personal_key: nil, decorated_user: decorated_user, + decrypted_pii: nil, personal_key: nil, user: user, sp_session_request_url: nil, sp_name: nil, locked_for_session: false ), diff --git a/spec/views/accounts/history/show.html.erb_spec.rb b/spec/views/accounts/history/show.html.erb_spec.rb index 6c39f832b6d..6b02052466f 100644 --- a/spec/views/accounts/history/show.html.erb_spec.rb +++ b/spec/views/accounts/history/show.html.erb_spec.rb @@ -2,15 +2,13 @@ describe 'accounts/history/show.html.erb' do let(:user) { create(:user, :signed_up, :with_personal_key) } - let(:decorated_user) { user.decorate } before do - allow(user).to receive(:decorate).and_return(decorated_user) allow(view).to receive(:current_user).and_return(user) assign( :presenter, AccountShowPresenter.new( - decrypted_pii: nil, personal_key: nil, decorated_user: decorated_user, + decrypted_pii: nil, personal_key: nil, user: user, sp_session_request_url: nil, sp_name: nil, locked_for_session: false ), diff --git a/spec/views/accounts/show.html.erb_spec.rb b/spec/views/accounts/show.html.erb_spec.rb index a07d8c01f7e..f779de7eca4 100644 --- a/spec/views/accounts/show.html.erb_spec.rb +++ b/spec/views/accounts/show.html.erb_spec.rb @@ -2,15 +2,13 @@ describe 'accounts/show.html.erb' do let(:user) { create(:user, :signed_up, :with_personal_key) } - let(:decorated_user) { user.decorate } before do - allow(user).to receive(:decorate).and_return(decorated_user) allow(view).to receive(:current_user).and_return(user) assign( :presenter, AccountShowPresenter.new( - decrypted_pii: nil, personal_key: nil, decorated_user: decorated_user, + decrypted_pii: nil, personal_key: nil, user: user, sp_session_request_url: nil, sp_name: nil, locked_for_session: false ), @@ -25,7 +23,7 @@ context 'when current user has password_reset_profile' do before do - allow(decorated_user).to receive(:password_reset_profile).and_return(true) + allow(user).to receive(:password_reset_profile).and_return(true) end it 'displays an alert with instructions to reactivate their profile' do @@ -46,7 +44,7 @@ context 'when the user does not have pending_profile' do before do - allow(decorated_user).to receive(:pending_profile).and_return(false) + allow(user).to receive(:pending_profile).and_return(false) end it 'lacks a pending profile section' do @@ -60,7 +58,7 @@ context 'when current user has pending_profile' do before do - allow(decorated_user).to receive(:pending_profile).and_return(build(:profile)) + allow(user).to receive(:pending_profile).and_return(build(:profile)) end it 'contains a link to activate profile' do @@ -169,7 +167,7 @@ assign( :presenter, AccountShowPresenter.new( - decrypted_pii: nil, personal_key: 'abc123', decorated_user: decorated_user, + decrypted_pii: nil, personal_key: 'abc123', user: user, sp_session_request_url: sp.return_to_sp_url, sp_name: sp.friendly_name, locked_for_session: false ), diff --git a/spec/views/accounts/two_factor_authentication/show.html.erb_spec.rb b/spec/views/accounts/two_factor_authentication/show.html.erb_spec.rb index 8b1acac7f31..8bb2b091999 100644 --- a/spec/views/accounts/two_factor_authentication/show.html.erb_spec.rb +++ b/spec/views/accounts/two_factor_authentication/show.html.erb_spec.rb @@ -2,15 +2,13 @@ describe 'accounts/two_factor_authentication/show.html.erb' do let(:user) { create(:user, :signed_up, :with_personal_key) } - let(:decorated_user) { user.decorate } before do - allow(user).to receive(:decorate).and_return(decorated_user) allow(view).to receive(:current_user).and_return(user) assign( :presenter, AccountShowPresenter.new( - decrypted_pii: nil, personal_key: nil, decorated_user: decorated_user, + decrypted_pii: nil, personal_key: nil, user: user, sp_session_request_url: nil, sp_name: nil, locked_for_session: false ), @@ -33,7 +31,7 @@ assign( :presenter, AccountShowPresenter.new( - decrypted_pii: nil, personal_key: nil, decorated_user: decorated_user, + decrypted_pii: nil, personal_key: nil, user: user, sp_session_request_url: nil, sp_name: nil, locked_for_session: false ), @@ -52,7 +50,7 @@ context 'when the user does not have password_reset_profile' do before do - allow(decorated_user).to receive(:password_reset_profile).and_return(false) + allow(user).to receive(:password_reset_profile).and_return(false) end it 'contains a personal key section' do @@ -68,7 +66,7 @@ context 'when current user has password_reset_profile' do before do - allow(decorated_user).to receive(:password_reset_profile).and_return(true) + allow(user).to receive(:password_reset_profile).and_return(true) end it 'lacks a personal key section' do diff --git a/spec/views/idv/doc_auth/welcome.html.erb_spec.rb b/spec/views/idv/doc_auth/welcome.html.erb_spec.rb index 41b33a6503a..bfc1ccd77d9 100644 --- a/spec/views/idv/doc_auth/welcome.html.erb_spec.rb +++ b/spec/views/idv/doc_auth/welcome.html.erb_spec.rb @@ -4,6 +4,7 @@ let(:flow_session) { {} } let(:user_fully_authenticated) { true } let(:sp_name) { nil } + let(:user) { create(:user) } before do @decorated_session = instance_double(ServiceProviderSessionDecorator) @@ -19,11 +20,8 @@ let(:need_irs_reproofing) { false } before do - user_decoration = instance_double('user decoration') - allow(user_decoration).to receive(:reproof_for_irs?).and_return(need_irs_reproofing) - fake_user = instance_double(User) - allow(fake_user).to receive(:decorate).and_return(user_decoration) - assign(:current_user, fake_user) + allow(user).to receive(:reproof_for_irs?).and_return(need_irs_reproofing) + assign(:current_user, user) render template: 'idv/doc_auth/welcome' end diff --git a/spec/views/mfa_confirmation/show.html.erb_spec.rb b/spec/views/mfa_confirmation/show.html.erb_spec.rb index 25db24e1c15..f4bd122637d 100644 --- a/spec/views/mfa_confirmation/show.html.erb_spec.rb +++ b/spec/views/mfa_confirmation/show.html.erb_spec.rb @@ -2,7 +2,6 @@ describe 'mfa_confirmation/show.html.erb' do let(:user) { create(:user, :signed_up, :with_personal_key) } - let(:decorated_user) { user.decorate } before do allow(view).to receive(:current_user).and_return(user) diff --git a/spec/views/users/delete/show.html.erb_spec.rb b/spec/views/users/delete/show.html.erb_spec.rb index 9fed803f45a..0781a1fff99 100644 --- a/spec/views/users/delete/show.html.erb_spec.rb +++ b/spec/views/users/delete/show.html.erb_spec.rb @@ -2,10 +2,8 @@ describe 'users/delete/show.html.erb' do let(:user) { build_stubbed(:user, :signed_up) } - let(:decorated_user) { user.decorate } before do - allow(user).to receive(:decorate).and_return(decorated_user) allow(view).to receive(:current_user).and_return(user) end @@ -20,20 +18,20 @@ render expect(rendered).to have_content(t('users.delete.bullet_1', app_name: APP_NAME)) - expect(rendered).to have_content(user.decorate.delete_account_bullet_key) + expect(rendered).to have_content(user.delete_account_bullet_key) expect(rendered).to have_content(t('users.delete.bullet_3', app_name: APP_NAME)) expect(rendered).to have_content(t('users.delete.bullet_4', app_name: APP_NAME)) end it 'displays bullets for loa1' do - allow(decorated_user).to receive(:identity_verified?).and_return(false) - expect(user.decorate.delete_account_bullet_key). + allow(user).to receive(:identity_verified?).and_return(false) + expect(user.delete_account_bullet_key). to eq t('users.delete.bullet_2_basic', app_name: APP_NAME) end it 'displays bullets for loa1' do - allow(decorated_user).to receive(:identity_verified?).and_return(true) - expect(user.decorate.delete_account_bullet_key). + allow(user).to receive(:identity_verified?).and_return(true) + expect(user.delete_account_bullet_key). to eq t('users.delete.bullet_2_verified', app_name: APP_NAME) end From b4157ef3cea0006ee64257a0c2fbdbe34c8f1962 Mon Sep 17 00:00:00 2001 From: John Skiles Skinner Date: Fri, 14 Apr 2023 15:52:44 -0700 Subject: [PATCH 05/10] Jskinne3 lg 9361 mobile local md (#8217) * Transfer mobile guide from old branch * Link to mobile docs from other docs * Check spelling * [skip changelog] * Auto-generate README * Soften USB hub wording becase it works for some people * Seperate mobile device and VM instructions --- README.md | 1 + docs/local-development.md | 10 +++-- docs/mobile.md | 80 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 3 deletions(-) create mode 100644 docs/mobile.md diff --git a/README.md b/README.md index 49941f75cf0..ff5a8b026f3 100644 --- a/README.md +++ b/README.md @@ -21,6 +21,7 @@ Refer to the [_Local Development_ documentation](./docs/local-development.md) to - [Docker](docs/Docker.md) - [Front-end Architecture](docs/frontend.md) - [Local Development](docs/local-development.md) +- [Mobile local development](docs/mobile.md) - [SAML Profile](docs/SAML_PROFILE.md) - [Security](docs/SECURITY.md) - [Troubleshooting Local Development](docs/troubleshooting.md) diff --git a/docs/local-development.md b/docs/local-development.md index 08510ab5cb3..1f33ac62437 100644 --- a/docs/local-development.md +++ b/docs/local-development.md @@ -154,9 +154,9 @@ The Geolite2-City database can be downloaded from MaxMind's site at [https://dev Download the GeoIP2 Binary and save it at `geo_data/GeoLite2-City.mmdb`. The app will start using that Geolite2 file for geolocation after restart. -### Testing on a mobile device or in a virtual machine +### Testing in a virtual machine -By default, the application binds to `localhost`. To test on a network device or within a virtual machine, you can bind to `0.0.0.0` instead, using the following instructions: +By default, the application binds to `localhost`. To test on a local network device or within a virtual machine, you can bind to `0.0.0.0` instead, using the following instructions: 1. Determine your computer's network IP address. On macOS, you can find this in the "Network" system settings, shown under the "Status: Connected" label. This often takes the format of `192.168.1.x` or `10.0.0.x`. 2. In `config/application.yml`, add `domain_name` and `mailer_domain_name` keys under `development`, like so: @@ -167,7 +167,11 @@ By default, the application binds to `localhost`. To test on a network device or ``` replacing `` with the address you found in Step 1 3. Start the server using the command `HOST=0.0.0.0 make run` -4. Assuming that your phone or virtual machine computer is connected on the same network, visit the application using the domain name configured in the second step (for example, `http://192.168.1.131:3000`). +4. From on the same network, visit the application using the domain name configured in the second step (for example, `http://192.168.1.131:3000`). + +### Testing on a mobile device + +[Moble device instructions are here](mobile.md) ### Testing the application over HTTPS diff --git a/docs/mobile.md b/docs/mobile.md new file mode 100644 index 00000000000..d51af5558dc --- /dev/null +++ b/docs/mobile.md @@ -0,0 +1,80 @@ +# Mobile local development + +Instructions to use an iPhone or Android mobile device for local app development, in two sections: +* [§ Use the app from a mobile device](#use-the-app-from-a-mobile-device) by running it over your home or office WiFi +* [§ Debugging with the desktop browser](#debugging-with-the-desktop-browser) by plugging your phone into your computer with a USB cable + +## Use the app from a mobile device + +These instructions will configure your local copy of the identity-idp app to serve web pages over your local computer network — the wifi in your home or office. You can broadcast the app to a mobile phone or tablet. Both your mobile device and your development computer (your laptop) must be connected to the same wifi network. + +By default, the application binds to `localhost`. These instructions bind to `0.0.0.0` instead. Some Android users report they can access `localhost:3000` directly on their phone, however. + +1. Find your Local Area Network IP address. On a MacBook, this is available at **System Preferences → Network**. The address may start with `192.168`. + +2. To your app's `application.yml` file, add the below. Fill in your actual LAN IP address. The final line creates a **confirm now** link in place of email confirmation. +```yaml +domain_name: 192.168.x.x:3000 +mailer_domain_name: 192.168.x.x:3000 +enable_load_testing_mode: true +``` + +3. In the Chrome web browser of your development computer, visit `chrome://inspect` + +4. Click on **Port forwarding**. For port `8234` enter `0.0.0.0:3000`. Check **Enable port forwarding** and click **Done**. These screenshots illustrates enabling port forwarding on a MacBook: + +Click on Port forwarding | Enter IP and enable +:-------------------------:|:-------------------------: +![port-forwarding-button](https://user-images.githubusercontent.com/546123/231608927-5f577e1a-bc82-47c6-b69a-d592c551a99f.png) | ![port-forwarding-settings](https://user-images.githubusercontent.com/546123/231608489-f09f281e-305d-4200-9f21-9d772773a113.png) + +5. Start your app's local web server with: +```bash +HOST=0.0.0.0 make run-https +``` + +6. In the Chrome browser on your phone, open a new incognito tab. In the address bar, type in `https://` (don't forget the `s`) followed by your LAN IP and port number (like `https://192.168.x.x:3000`). When you visit this page, you may see a **Your connection is not private** message. Click **Advanced** and **Proceed** to continue. You should then see the sign in screen of the identity-idp app. + +After you complete these steps, pages from the app are served from your development machine to your mobile device, where you may now use the identity-idp app. For front-end development, you may now want to turn on browser development tools per the next section of these instructions. + +## Debugging with the desktop browser + +After you have completed the [§ Use the app from a mobile device](#use-the-app-from-a-mobile-device) instructions above, you may further want to use your desktop browser's development and dubugging tools. + +To do this, you will plug your phone into your laptop. You will need a USB cable. It does not work via WiFi. + +### Android / Chrome + +These instructions will allow you to debug your phone browser with Chrome DevTools on your development machine. Also, they let you view and interact with your phone's browser screen on your laptop screen or development monitor, which lets you screenshare your development work with coworkers. + +1. In your Android phone, turn on USB debugging. This will allow your development computer to connect to your phone. + + **USB debugging** is a setting the **Developer options** menu. This menu may be hidden on your phone. It can be revealed with a ["magic tap"](https://developer.android.com/studio/debug/dev-options) on the phone **Build number** 5 times. + + +2. Plug your Android phone into your development computer with a USB cable. (A USB hub may or may not work.) If you see a message on your phone asking you to **Allow USB debugging** click to allow it. + +3. Visit `chrome://inspect` in the Chrome browser of your development computer. (It may already be open from the previous set of instructions.) You should see a listing of all the tabs open on your phone. Find the item on the list that represents the sign in screen of the identity-idp app. It should be at the top of the list. + +4. Click to **inspect** this tab. You should see browser DevTools and a representation of your phone's screen on your development computer, as in this illustration: + + inspect-androd-chrome-tab + +### iPhone / Safari + +These instructions work only if your development computer is an Apple product. You will need a USB cable with the appropriate "lightning" connector to plug into an iPhone. + +1. On your development Apple machine, open the Safari web browser. Go to menu items **Safari → Preferences → Advanced** and check **Show Develop menu in menu bar**. A screenshot: + + ![show_develop_menu](https://user-images.githubusercontent.com/546123/232129916-3c68d950-1145-4af6-9a1a-c8e7c3dea7a1.png) + +2. Take a glance at the newly-revealed **Develop** menu item in Safari. Seeing how the menu looks now may help you find your iPhone when it later appears in the menu. + +3. On your iPhone, go to **Settings → Safari → Advanced** and turn on Web Inspector. Make sure JavaScript is also on. + +4. Plug your iPhone into your development computer with a USB cable. (A USB hub may or may not work.) If you see a message on your phone asking you to **Trust This Computer?** click to trust it. + +5. Revisit the Safari **Develop** menu of your laptop. You'll see a menu item that wasn't there before: the name of your phone. + +6. Within that menu item, you'll find a list of Safari browser tabs open on your iPhone. To see them, the iPhone must be unlocked, and the Safari browser must be displayed on the phone's screen. + +7. Click on the tab you wish to inspect. You will see browser debugging tools like Elements, Console, and Layers. (Unlike the above Android instructions, you will not see a picture of your phone's browser screen on your laptop's screen.) From 35de604ef0e8aee4e5991ba90d53d98c6ee8463d Mon Sep 17 00:00:00 2001 From: Jonathan Hooper Date: Mon, 17 Apr 2023 09:41:21 -0400 Subject: [PATCH 06/10] Configure the DDP mock client to respond with a failed result on `no_result` (#8214) In a previous commit we changed the DDP proofer to respond with an exception result when the result from DDP included an unexpected status (ref: #8149). This includes when the result is nil. This commit changes the DDP mock's behavior to align with the DDP proofer's behavior. [skip changelog] --- app/services/proofing/mock/ddp_mock_client.rb | 9 +++++++++ spec/features/idv/threatmetrix_pending_spec.rb | 17 +++++++++++++++-- .../proofing/mock/ddp_mock_client_spec.rb | 6 ++++-- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/app/services/proofing/mock/ddp_mock_client.rb b/app/services/proofing/mock/ddp_mock_client.rb index 4e0fe62294c..167152f026b 100644 --- a/app/services/proofing/mock/ddp_mock_client.rb +++ b/app/services/proofing/mock/ddp_mock_client.rb @@ -50,6 +50,8 @@ def proof(applicant) response_body = response_body_json(review_status: review_status) request_result = response_body['request_result'] + return exception_result if review_status.nil? + result.review_status = review_status result.response_body = response_body @@ -77,6 +79,13 @@ def response_body_json(review_status:) json_body['review_status'] = review_status end end + + def exception_result + Proofing::DdpResult.new( + success: false, + exception: RuntimeError.new('Unexpected ThreatMetrix review_status value'), + ) + end end end end diff --git a/spec/features/idv/threatmetrix_pending_spec.rb b/spec/features/idv/threatmetrix_pending_spec.rb index 446b3a5eab0..5ddf176534c 100644 --- a/spec/features/idv/threatmetrix_pending_spec.rb +++ b/spec/features/idv/threatmetrix_pending_spec.rb @@ -86,9 +86,22 @@ end end - scenario 'users pending threatmetrix No Result, it logs idv_tmx_fraud_check event', :js do + scenario 'users pending threatmetrix No Result, it results in an error', :js do freeze_time do - expect_pending_failure_reason(threatmetrix: 'No Result') + user = create(:user, :signed_up) + visit_idp_from_ial1_oidc_sp( + client_id: service_provider.issuer, + irs_attempts_api_session_id: 'test-session-id', + ) + visit root_path + sign_in_and_2fa_user(user) + complete_doc_auth_steps_before_ssn_step + select 'No Result', from: :mock_profiling_result + complete_ssn_step + click_idv_continue + + expect(page).to have_content(t('idv.failure.sessions.exception')) + expect(page).to have_current_path(idv_session_errors_exception_path) end end diff --git a/spec/services/proofing/mock/ddp_mock_client_spec.rb b/spec/services/proofing/mock/ddp_mock_client_spec.rb index 346464fa89d..8ba1cf27d42 100644 --- a/spec/services/proofing/mock/ddp_mock_client_spec.rb +++ b/spec/services/proofing/mock/ddp_mock_client_spec.rb @@ -31,9 +31,11 @@ context 'with explicit no_result' do let(:redis_result) { 'no_result' } - it 'has a nil review status' do + it 'has an exception result' do expect(result.review_status).to be_nil - expect(result.response_body['review_status']).to be_nil + expect(result.response_body).to be_nil + expect(result.exception.inspect).to include('Unexpected ThreatMetrix review_status value') + expect(result.success?).to eq(false) end end From 241eccb3e49621b1166b2787beb85b2d03eaf090 Mon Sep 17 00:00:00 2001 From: Amir Reavis-Bey <1261794+amirbey@users.noreply.github.com> Date: Mon, 17 Apr 2023 13:58:45 -0400 Subject: [PATCH 07/10] LG-9034 Log TrueID decision product status (#8195) * LG-9034 Log TrueID decision product status Capture decision product status for TrueID responses Decision product status contains the result of TrueID after decisioning. Prior to this change, product status was only being captured for @productType='TrueID' and ignoring product status for @productType='TrueID_Decision' * [skip changelog] * successful if TrueID_Decision product does not exist * refactor obtaining decision_product for reuse * happy linting * Update spec/services/doc_auth/lexis_nexis/responses/true_id_response_spec.rb Co-authored-by: Sonia Connolly * remove unnecessary expect statements since the check is done a few lines below * remove line no longer used in test --------- Co-authored-by: AmirReavis-Bey Co-authored-by: Sonia Connolly --- .../lexis_nexis/responses/true_id_response.rb | 9 ++++ .../responses/true_id_response_spec.rb | 45 +++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/app/services/doc_auth/lexis_nexis/responses/true_id_response.rb b/app/services/doc_auth/lexis_nexis/responses/true_id_response.rb index 3c2f92ee622..87ea392b909 100644 --- a/app/services/doc_auth/lexis_nexis/responses/true_id_response.rb +++ b/app/services/doc_auth/lexis_nexis/responses/true_id_response.rb @@ -139,6 +139,7 @@ def create_response_info transaction_status: transaction_status, transaction_reason_code: transaction_reason_code, product_status: product_status, + decision_product_status: decision_product_status, doc_auth_result: doc_auth_result, processed_alerts: alerts, alert_failure_count: alerts[:failed]&.count.to_i, @@ -189,10 +190,18 @@ def product_status true_id_product&.dig(:ProductStatus) end + def decision_product_status + true_id_product_decision&.dig(:ProductStatus) + end + def true_id_product products[:TrueID] if products.present? end + def true_id_product_decision + products[:TrueID_Decision] if products.present? + end + def parsed_alerts return @new_alerts if defined?(@new_alerts) diff --git a/spec/services/doc_auth/lexis_nexis/responses/true_id_response_spec.rb b/spec/services/doc_auth/lexis_nexis/responses/true_id_response_spec.rb index 56e5d4ea0bf..2ce347432ea 100644 --- a/spec/services/doc_auth/lexis_nexis/responses/true_id_response_spec.rb +++ b/spec/services/doc_auth/lexis_nexis/responses/true_id_response_spec.rb @@ -99,6 +99,7 @@ transaction_status: 'passed', transaction_reason_code: 'trueid_pass', product_status: 'pass', + decision_product_status: 'pass', doc_auth_result: 'Passed', processed_alerts: a_hash_including(:failed), address_line2_present: true, @@ -145,6 +146,49 @@ end end + context 'when True_ID response does not contain a decision product status' do + let(:true_id_response_success_2) { JSON.parse(LexisNexisFixtures.true_id_response_success_2) } + describe 'when a True_ID Decision product is not present in the response' do + it 'excludes decision_product_status from logging' do + body_no_decision = true_id_response_success_2.tap do |json| + json['Products'].delete_if { |products| products['ProductType'] == 'TrueID_Decision' } + end.to_json + + decision_product = get_decision_product(true_id_response_success_2) + expect(decision_product).to be_nil + success_response_no_decision = instance_double( + Faraday::Response, status: 200, + body: body_no_decision + ) + response = described_class.new(success_response_no_decision, config) + + expect(response.to_h[:decision_product_status]).to be_nil + end + end + + describe 'when a True_ID_Decision does not contain a status' do + it 'excludes decision_product_status from logging' do + decision_product = get_decision_product(true_id_response_success_2) + body_no_decision_status = decision_product.tap do |json| + json.delete('ProductStatus') + end.to_json + + expect(decision_product['ProductStatus']).to be_nil + success_response_no_decision_status = instance_double( + Faraday::Response, status: 200, + body: body_no_decision_status + ) + response = described_class.new(success_response_no_decision_status, config) + + expect(response.to_h[:decision_product_status]).to be_nil + end + end + + def get_decision_product(resp) + resp['Products'].find { |product| product['ProductType'] == 'TrueID_Decision' } + end + end + context 'when the barcode can not be read' do let(:response) do described_class.new(attention_barcode_read, config) @@ -241,6 +285,7 @@ transaction_status: 'failed', transaction_reason_code: 'failed_true_id', product_status: 'pass', + decision_product_status: 'fail', doc_auth_result: 'Failed', processed_alerts: a_hash_including(:passed, :failed), address_line2_present: false, From 260c4bca7591823a2c2c09ace0adaed86bb638aa Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Mon, 17 Apr 2023 11:13:35 -0700 Subject: [PATCH 08/10] LG-9488: Entry controller for hybrid mobile document capture flow (#8209) * Add placeholder CaptureDocController * Add controller to handle entry into hybrid flow Controller looks at `document-capture-session` and does a "light login" to enable the user to start document capture. changelog: Internal, Flow state machine removal, Add controller for entry into hybrid mobile document capture * Refactor feature flag check out to HybridMobileConcern * Add tests around the 'Doc Auth' event * Send links to new hybrid mobile experience Going with /verify/documents?document-capture-session=xxxxx as a temporary url for now * CaptureDocController -> DocumentCaptureController * Tweak doc capture entry url naming & add comment * idv_hybrid_mobile_document_capture_entry_url -> idv_hybrid_mobile_entry_url * Refactor EntryController not to use before_action * Move HybridMobileConcern into Idv::HybridMobile namespace * WHOOPS need to see why this wasn't breaking any tests * Remove extra redirect Rails doesn't preserve the querystring when redirecting routes, and this is just an unnecessary extra step anyway * Add feature spec for mobile hybrid flow entry * Tweaks to capture complete step spec Ultimately we'll merge this with a doc capture step, presumably --- .../hybrid_mobile/hybrid_mobile_concern.rb | 15 ++ .../capture_complete_controller.rb | 7 +- .../document_capture_controller.rb | 11 ++ .../idv/hybrid_mobile/entry_controller.rb | 82 +++++++++ app/services/idv/steps/upload_step.rb | 21 ++- config/routes.rb | 4 + .../hybrid_mobile/entry_controller_spec.rb | 173 ++++++++++++++++++ .../hybrid_mobile/capture_complete_spec.rb | 10 +- spec/features/idv/hybrid_mobile/entry_spec.rb | 60 ++++++ spec/services/idv/steps/upload_step_spec.rb | 22 +++ 10 files changed, 387 insertions(+), 18 deletions(-) create mode 100644 app/controllers/concerns/idv/hybrid_mobile/hybrid_mobile_concern.rb create mode 100644 app/controllers/idv/hybrid_mobile/document_capture_controller.rb create mode 100644 app/controllers/idv/hybrid_mobile/entry_controller.rb create mode 100644 spec/controllers/idv/hybrid_mobile/entry_controller_spec.rb create mode 100644 spec/features/idv/hybrid_mobile/entry_spec.rb diff --git a/app/controllers/concerns/idv/hybrid_mobile/hybrid_mobile_concern.rb b/app/controllers/concerns/idv/hybrid_mobile/hybrid_mobile_concern.rb new file mode 100644 index 00000000000..fcab812b79c --- /dev/null +++ b/app/controllers/concerns/idv/hybrid_mobile/hybrid_mobile_concern.rb @@ -0,0 +1,15 @@ +module Idv + module HybridMobile + module HybridMobileConcern + extend ActiveSupport::Concern + + included do + before_action :render_404_if_hybrid_mobile_controllers_disabled + end + + def render_404_if_hybrid_mobile_controllers_disabled + render_not_found unless IdentityConfig.store.doc_auth_hybrid_mobile_controllers_enabled + end + end + end +end diff --git a/app/controllers/idv/hybrid_mobile/capture_complete_controller.rb b/app/controllers/idv/hybrid_mobile/capture_complete_controller.rb index 7697b607da8..19ae695e9c0 100644 --- a/app/controllers/idv/hybrid_mobile/capture_complete_controller.rb +++ b/app/controllers/idv/hybrid_mobile/capture_complete_controller.rb @@ -5,8 +5,7 @@ class CaptureCompleteController < ApplicationController include IdvStepConcern include StepIndicatorConcern include StepUtilitiesConcern - - before_action :render_404_if_hybrid_mobile_controllers_disabled + include HybridMobileConcern def show increment_step_counts @@ -21,10 +20,6 @@ def show private - def render_404_if_hybrid_mobile_controllers_disabled - render_not_found unless IdentityConfig.store.doc_auth_hybrid_mobile_controllers_enabled - end - def analytics_arguments { flow_path: 'hybrid', diff --git a/app/controllers/idv/hybrid_mobile/document_capture_controller.rb b/app/controllers/idv/hybrid_mobile/document_capture_controller.rb new file mode 100644 index 00000000000..7af9dc0a2c2 --- /dev/null +++ b/app/controllers/idv/hybrid_mobile/document_capture_controller.rb @@ -0,0 +1,11 @@ +module Idv + module HybridMobile + class DocumentCaptureController < ApplicationController + include HybridMobileConcern + + def show + # TODO + end + end + end +end diff --git a/app/controllers/idv/hybrid_mobile/entry_controller.rb b/app/controllers/idv/hybrid_mobile/entry_controller.rb new file mode 100644 index 00000000000..d1587eb0074 --- /dev/null +++ b/app/controllers/idv/hybrid_mobile/entry_controller.rb @@ -0,0 +1,82 @@ +module Idv + module HybridMobile + # Controller responsible for taking a `document-capture-session` UUID and configuring + # the user's Session to work when they're forwarded on to document capture. + class EntryController < ApplicationController + include HybridMobileConcern + + def show + track_document_capture_session_id_usage + + return handle_invalid_session if !validate_document_capture_session_id + + return handle_invalid_session if !validate_document_capture_user_id + + redirect_to idv_hybrid_mobile_document_capture_url + end + + private + + # This is the UUID present in the link sent to the user via SMS. + # It refers to a DocumentCaptureSession instance in the DB. + def document_capture_session_uuid + params['document-capture-session'] + end + + # This is the effective user for whom we are uploading documents. + def document_capture_user_id + session[:doc_capture_user_id] + end + + def handle_invalid_session + flash[:error] = t('errors.capture_doc.invalid_link') + redirect_to root_url + end + + def request_id + params.fetch(:request_id, '') + end + + def track_document_capture_session_id_usage + irs_attempts_api_tracker.idv_phone_upload_link_used + end + + def update_sp_session + return if sp_session[:issuer] || request_id.blank? + StoreSpMetadataInSession.new(session: session, request_id: request_id).call + end + + def validate_document_capture_session_id + if document_capture_session_uuid.blank? + # If we've already gotten a document capture user id previously, just continue + # processing and (eventually) redirect the user where they're supposed to be. + return true if document_capture_user_id + end + + result = Idv::DocumentCaptureSessionForm.new(document_capture_session_uuid).submit + + event_properties = result.to_h.tap do |properties| + # See LG-8890 for context + properties[:doc_capture_user_id?] = session[:doc_capture_user_id].present? + end + + analytics.track_event 'Doc Auth', event_properties + + if result.success? + reset_session + + session[:doc_capture_user_id] = result.extra[:for_user_id] + session[:document_capture_session_uuid] = document_capture_session_uuid + + update_sp_session + + true + end + end + + def validate_document_capture_user_id + !!document_capture_user_id + end + end + end +end diff --git a/app/services/idv/steps/upload_step.rb b/app/services/idv/steps/upload_step.rb index a6b272577e8..84d2556d669 100644 --- a/app/services/idv/steps/upload_step.rb +++ b/app/services/idv/steps/upload_step.rb @@ -34,6 +34,20 @@ def extra_view_variables { idv_phone_form: build_form } end + def link_for_send_link(session_uuid) + if IdentityConfig.store.doc_auth_hybrid_mobile_controllers_enabled + idv_hybrid_mobile_entry_url( + 'document-capture-session': session_uuid, + request_id: sp_session[:request_id], + ) + else + idv_capture_doc_dashes_url( + 'document-capture-session': session_uuid, + request_id: sp_session[:request_id], + ) + end + end + private def build_form @@ -137,13 +151,6 @@ def sp_or_app_name current_sp&.friendly_name.presence || APP_NAME end - def link_for_send_link(session_uuid) - idv_capture_doc_dashes_url( - 'document-capture-session': session_uuid, - request_id: sp_session[:request_id], - ) - end - def send_link session_uuid = flow_session[:document_capture_session_uuid] update_document_capture_session_requested_at(session_uuid) diff --git a/config/routes.rb b/config/routes.rb index 4c8cbf0a429..71ffdb4dfde 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -327,6 +327,10 @@ post '/forgot_password' => 'forgot_password#update' get '/document_capture' => 'document_capture#show' put '/document_capture' => 'document_capture#update' + # This route is included in SMS messages sent to users who start the IdV hybrid flow. It + # should be kept short, and should not include underscores ("_"). + get '/documents' => 'hybrid_mobile/entry#show', as: :hybrid_mobile_entry + get '/hybrid_mobile/document_capture' => 'hybrid_mobile/document_capture#show' get '/hybrid_mobile/capture_complete' => 'hybrid_mobile/capture_complete#show' get '/ssn' => 'ssn#show' put '/ssn' => 'ssn#update' diff --git a/spec/controllers/idv/hybrid_mobile/entry_controller_spec.rb b/spec/controllers/idv/hybrid_mobile/entry_controller_spec.rb new file mode 100644 index 00000000000..584f03242d9 --- /dev/null +++ b/spec/controllers/idv/hybrid_mobile/entry_controller_spec.rb @@ -0,0 +1,173 @@ +require 'rails_helper' + +describe Idv::HybridMobile::EntryController do + include IdvHelper + + describe '#show' do + let(:feature_flag_enabled) { true } + + let(:user) { create(:user) } + + let!(:document_capture_session) do + DocumentCaptureSession.create!( + user: user, + requested_at: Time.zone.now, + ) + end + + let(:session_uuid) { document_capture_session.uuid } + + before do + stub_analytics + stub_attempts_tracker + + allow(IdentityConfig.store).to receive(:doc_auth_hybrid_mobile_controllers_enabled). + and_return(feature_flag_enabled) + end + + context 'feature flag disabled' do + let(:feature_flag_enabled) { false } + + before do + get :show, params: { 'document-capture-session': session_uuid } + end + + it '404s' do + expect(response).to have_http_status(:not_found) + end + + it 'does not log that phone upload link was used' do + expect(@irs_attempts_api_tracker.events).not_to have_key(:idv_phone_upload_link_used) + end + end + + context 'with no session' do + before do + get :show + end + it 'logs that phone upload link was used' do + expect(@irs_attempts_api_tracker.events).to have_key(:idv_phone_upload_link_used) + end + it 'redirects to the root url' do + expect(response).to redirect_to root_url + end + end + + context 'with a bad session' do + before do + get :show, params: { 'document-capture-session': 'foo' } + end + it 'logs that phone upload link was used' do + expect(@irs_attempts_api_tracker.events).to have_key(:idv_phone_upload_link_used) + end + it 'logs an analytics event' do + expect(@analytics).to have_logged_event( + 'Doc Auth', + hash_including( + success: false, + errors: { session_uuid: ['invalid session'] }, + ), + ) + end + it 'redirects to the root url' do + expect(response).to redirect_to root_url + end + end + + context 'with an expired token' do + before do + travel_to(Time.zone.now + 1.day) do + get :show, params: { 'document-capture-session': session_uuid } + end + end + + it 'logs that phone upload link was used' do + expect(@irs_attempts_api_tracker.events).to have_key(:idv_phone_upload_link_used) + end + + it 'redirects to the root url' do + expect(response).to redirect_to root_url + end + end + + context 'with a good session uuid' do + let(:session) do + {} + end + + before do + allow(controller).to receive(:session).and_return(session) + get :show, params: { 'document-capture-session': session_uuid } + end + + it 'logs that phone upload link was used' do + expect(@irs_attempts_api_tracker.events).to have_key(:idv_phone_upload_link_used) + end + + it 'redirects to the first step' do + expect(response).to redirect_to idv_hybrid_mobile_document_capture_url + end + + it 'logs an analytics event' do + expect(@analytics).to have_logged_event( + 'Doc Auth', + hash_including( + success: true, + doc_capture_user_id?: false, + ), + ) + end + + context 'but we already had a session' do + let!(:different_document_capture_session) do + DocumentCaptureSession.create!( + user: user, + requested_at: Time.zone.now, + ) + end + + let(:session) do + { + doc_capture_user_id: user.id, + document_capture_session_uuid: different_document_capture_session.uuid, + } + end + + it 'assumes new document capture session' do + expect(controller.session).to include(document_capture_session_uuid: session_uuid) + end + + it 'logs an analytics event' do + expect(@analytics).to have_logged_event( + 'Doc Auth', + hash_including( + success: true, + doc_capture_user_id?: true, + ), + ) + end + + it 'redirects to the document capture screen' do + expect(response).to redirect_to idv_hybrid_mobile_document_capture_url + end + end + end + + context 'with a user id in session and no session uuid' do + let(:user) { create(:user) } + + before do + session[:doc_capture_user_id] = user.id + get :show + end + + it 'logs that phone upload link was used' do + expect(@irs_attempts_api_tracker.events).to have_key(:idv_phone_upload_link_used) + end + + it 'redirects to the first step' do + expect(response).to redirect_to idv_hybrid_mobile_document_capture_url + end + end + end +end diff --git a/spec/features/idv/hybrid_mobile/capture_complete_spec.rb b/spec/features/idv/hybrid_mobile/capture_complete_spec.rb index f091915118e..d5eadd6a137 100644 --- a/spec/features/idv/hybrid_mobile/capture_complete_spec.rb +++ b/spec/features/idv/hybrid_mobile/capture_complete_spec.rb @@ -5,14 +5,14 @@ include DocAuthHelper include DocCaptureHelper - let(:user) { user_with_2fa } - before do allow(IdentityConfig.store).to receive(:doc_auth_hybrid_mobile_controllers_enabled). and_return(true) - complete_doc_capture_steps_before_capture_complete_step(user) - allow_any_instance_of(Browser).to receive(:mobile?).and_return(true) - sign_in_and_2fa_user(user) + + sign_in_and_2fa_user + complete_doc_auth_steps_before_upload_step + click_send_link + visit(idv_hybrid_mobile_capture_complete_url) end diff --git a/spec/features/idv/hybrid_mobile/entry_spec.rb b/spec/features/idv/hybrid_mobile/entry_spec.rb new file mode 100644 index 00000000000..c363d45e0d3 --- /dev/null +++ b/spec/features/idv/hybrid_mobile/entry_spec.rb @@ -0,0 +1,60 @@ +require 'rails_helper' + +feature 'mobile hybrid flow entry', js: true do + include IdvStepHelper + + before do + allow(IdentityConfig.store).to receive(:doc_auth_hybrid_mobile_controllers_enabled). + and_return(true) + end + + let(:link_sent_via_sms) do + link = nil + + # Intercept the link being SMS'd to the user. + allow(Telephony).to receive(:send_doc_auth_link).and_wrap_original do |impl, config| + link = config[:link] + impl.call(**config) + end + + sign_in_and_2fa_user + complete_doc_auth_steps_before_upload_step + click_send_link + + link + end + + let(:link_to_visit) { link_sent_via_sms } + + context 'valid link' do + it 'puts the user on the document capture page' do + expect(link_to_visit).to be + + Capybara.using_session('mobile') do + visit link_to_visit + # Should have redirected to the actual doc capture url + expect(current_url).to eql(idv_hybrid_mobile_document_capture_url) + end + end + end + + context 'invalid link' do + let(:link_to_visit) do + # Put an invalid document-capture-session in the URL + uri = URI.parse(link_sent_via_sms) + query = Rack::Utils.parse_query(uri.query) + query['document-capture-session'] = SecureRandom.uuid + uri.query = Rack::Utils.build_query(query) + uri.to_s + end + + it 'redirects to the root' do + expect(link_to_visit).to be + + Capybara.using_session('mobile') do + visit link_to_visit + expect(current_url).to eql(root_url) + end + end + end +end diff --git a/spec/services/idv/steps/upload_step_spec.rb b/spec/services/idv/steps/upload_step_spec.rb index 881e6ee521c..f0e43c9d464 100644 --- a/spec/services/idv/steps/upload_step_spec.rb +++ b/spec/services/idv/steps/upload_step_spec.rb @@ -94,5 +94,27 @@ ) end end + + context 'hybrid mobile controllers enabled' do + before do + allow(IdentityConfig.store).to receive(:doc_auth_hybrid_mobile_controllers_enabled). + and_return(true) + end + + describe '#link_for_send_link' do + subject(:link) { step.link_for_send_link document_capture_session_uuid } + let(:document_capture_session_uuid) { SecureRandom.uuid } + + it 'generates a link to the hybrid mobile doc auth entry controller' do + expect(link).to eql( + Rails.application.routes.url_helpers.idv_hybrid_mobile_entry_url( + params: { + 'document-capture-session': document_capture_session_uuid, + }, + ), + ) + end + end + end end end From 712d962451422bfbeb5710ae6b40923e6750e252 Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Mon, 17 Apr 2023 15:01:35 -0500 Subject: [PATCH 09/10] Fix email confirmation 500 (#8224) * add failing spec * Use strong parameters in email confirmation controller to fix 500 error changelog: Bug Fixes, Email Confirmation, Use strong parameters in email confirmation controller to fix 500 error --- app/controllers/users/email_confirmations_controller.rb | 8 +++++++- .../users/email_confirmations_controller_spec.rb | 7 +++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/app/controllers/users/email_confirmations_controller.rb b/app/controllers/users/email_confirmations_controller.rb index 6f1ac0b81f2..862072cfae6 100644 --- a/app/controllers/users/email_confirmations_controller.rb +++ b/app/controllers/users/email_confirmations_controller.rb @@ -15,7 +15,9 @@ def create def email_address return @email_address if defined?(@email_address) - email_address = EmailAddress.find_with_confirmation_token(params[:confirmation_token]) + email_address = EmailAddress.find_with_confirmation_token( + confirmation_params[:confirmation_token], + ) if email_address&.user&.confirmed? @email_address = email_address else @@ -90,5 +92,9 @@ def email_address_already_confirmed_by_current_user? user_signed_in? && email_confirmation_token_validator.email_address_already_confirmed_by_user?(current_user) end + + def confirmation_params + params.permit(:confirmation_token) + end end end diff --git a/spec/controllers/users/email_confirmations_controller_spec.rb b/spec/controllers/users/email_confirmations_controller_spec.rb index 70f4e07bf8c..3da7fa06e62 100644 --- a/spec/controllers/users/email_confirmations_controller_spec.rb +++ b/spec/controllers/users/email_confirmations_controller_spec.rb @@ -60,5 +60,12 @@ expect(flash[:error]).to eq t('errors.messages.confirmation_invalid_token') end end + + describe 'Invalid email confirmation tokens' do + it 'rejects invalid parameters' do + get :create, params: { confirmation_token: { confirmation_token: 'abc' } } + expect(flash[:error]).to eq t('errors.messages.confirmation_invalid_token') + end + end end end From b78bb4c391c5d4411eec802253a08380140eaf6a Mon Sep 17 00:00:00 2001 From: gina-yamada <125507397+gina-yamada@users.noreply.github.com> Date: Mon, 17 Apr 2023 14:03:16 -0600 Subject: [PATCH 10/10] LG-9321: New field on Enrollment Outcomes for GetUspsProofingResultsJob Summary (#8216) * LG-9321 Add data to usps proof results job * LG-9321 Added one more test * LG-9321 Fix lint issues * changelog: Internal, In-person-proofing, new metric on enrollment outcomes summary for GetUspsProofingResultsJob * LG-9321 Add arg to round() for precision * LG-9321 Update round to have more precision Co-authored-by: Tim Bradley <90272033+NavaTim@users.noreply.github.com> * LG-9321 Remove initial unnecessary assignment Co-authored-by: Tim Bradley <90272033+NavaTim@users.noreply.github.com> --------- Co-authored-by: Tim Bradley <90272033+NavaTim@users.noreply.github.com> --- app/jobs/get_usps_proofing_results_job.rb | 10 ++++ .../get_usps_proofing_results_job_spec.rb | 57 ++++++++++++++++++- 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/app/jobs/get_usps_proofing_results_job.rb b/app/jobs/get_usps_proofing_results_job.rb index 467f21a2ec2..2697a2b4b61 100644 --- a/app/jobs/get_usps_proofing_results_job.rb +++ b/app/jobs/get_usps_proofing_results_job.rb @@ -39,9 +39,19 @@ def perform(_now) check_enrollments(enrollments) + percent_enrollments_errored = 0 + if enrollment_outcomes[:enrollments_checked] > 0 + percent_enrollments_errored = + (enrollment_outcomes[:enrollments_errored].fdiv( + enrollment_outcomes[:enrollments_checked], + ) * 100).round(2) + end + analytics.idv_in_person_usps_proofing_results_job_completed( **enrollment_outcomes, duration_seconds: (Time.zone.now - started_at).seconds.round(2), + # Calculate % of errored enrollments + percent_enrollments_errored:, ) true diff --git a/spec/jobs/get_usps_proofing_results_job_spec.rb b/spec/jobs/get_usps_proofing_results_job_spec.rb index 05ee63aee19..b80e547fbfd 100644 --- a/spec/jobs/get_usps_proofing_results_job_spec.rb +++ b/spec/jobs/get_usps_proofing_results_job_spec.rb @@ -302,7 +302,7 @@ ) end - it 'logs a message with counts of various outcomes when the job completes' do + it 'logs a message with counts of various outcomes when the job completes (errored > 0)' do allow(InPersonEnrollment).to receive(:needs_usps_status_check). and_return(pending_enrollments) stub_request_proofing_results_with_responses( @@ -324,6 +324,61 @@ enrollments_failed: 1, enrollments_in_progress: 1, enrollments_passed: 1, + percent_enrollments_errored: 20, + ) + + expect( + job_analytics.events['GetUspsProofingResultsJob: Job completed']. + first[:duration_seconds], + ).to be >= 0.0 + end + + it 'logs a message with counts of various outcomes when the job completes (errored = 0)' do + allow(InPersonEnrollment).to receive(:needs_usps_status_check). + and_return(pending_enrollments) + stub_request_proofing_results_with_responses( + request_passed_proofing_results_args, + ) + + job.perform(Time.zone.now) + + expect(job_analytics).to have_logged_event( + 'GetUspsProofingResultsJob: Job completed', + duration_seconds: anything, + enrollments_checked: 5, + enrollments_errored: 0, + enrollments_expired: 0, + enrollments_failed: 0, + enrollments_in_progress: 0, + enrollments_passed: 5, + percent_enrollments_errored: 0, + ) + + expect( + job_analytics.events['GetUspsProofingResultsJob: Job completed']. + first[:duration_seconds], + ).to be >= 0.0 + end + + it 'logs a message with counts of various outcomes when the job completes (no enrollments)' do + allow(InPersonEnrollment).to receive(:needs_usps_status_check). + and_return([]) + stub_request_proofing_results_with_responses( + request_passed_proofing_results_args, + ) + + job.perform(Time.zone.now) + + expect(job_analytics).to have_logged_event( + 'GetUspsProofingResultsJob: Job completed', + duration_seconds: anything, + enrollments_checked: 0, + enrollments_errored: 0, + enrollments_expired: 0, + enrollments_failed: 0, + enrollments_in_progress: 0, + enrollments_passed: 0, + percent_enrollments_errored: 0, ) expect(