From 9330ed99f01ad8a6890f2b8cd5c49000f46de0ca Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Tue, 3 Jan 2023 09:47:50 -0800 Subject: [PATCH 01/18] LG-8392: Email user when OK'ing their ThreatMetrix status (#7555) * Email user when OK'ing their ThreatMetrix status When operators manually OK a user's ThreatMetrix status, send the "You verified your identity" email. changelog: Internal, ThreatMetrix, Email when OK'ing user's ThreatMetrix status For LG-8392 * s/disavowal_token/build_disavowal_token/ * linty mclinterson --- app/services/user_event_creator.rb | 18 ++++++++++--- lib/tasks/review_profile.rake | 21 ++++++++++++--- spec/lib/tasks/review_profile_spec.rb | 37 +++++++++++++++++++-------- 3 files changed, 59 insertions(+), 17 deletions(-) diff --git a/app/services/user_event_creator.rb b/app/services/user_event_creator.rb index c51e80f4973..830fb4fe78d 100644 --- a/app/services/user_event_creator.rb +++ b/app/services/user_event_creator.rb @@ -31,16 +31,24 @@ def create_out_of_band_user_event(event_type) create_event_for_device(event_type: event_type, user: current_user, device: nil) end + def create_out_of_band_user_event_with_disavowal(event_type) + create_event_for_device( + event_type: event_type, + user: current_user, + device: nil, + disavowal_token: build_disavowal_token, + ) + end + # @return [Array(Event, String)] an (event, disavowal_token) tuple def create_user_event_with_disavowal(event_type, user = current_user, device = nil) - disavowal_token = SecureRandom.urlsafe_base64(32) if device create_event_for_existing_device( event_type: event_type, user: user, device: device, - disavowal_token: disavowal_token + disavowal_token: build_disavowal_token ) else - create_user_event(event_type, user, disavowal_token) + create_user_event(event_type, user, build_disavowal_token) end end @@ -57,6 +65,10 @@ def create_event_for_existing_device(event_type:, user:, device:, disavowal_toke ) end + def build_disavowal_token + SecureRandom.urlsafe_base64(32) + end + # @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? diff --git a/lib/tasks/review_profile.rake b/lib/tasks/review_profile.rake index a296aab4d64..dfb301372a6 100644 --- a/lib/tasks/review_profile.rake +++ b/lib/tasks/review_profile.rake @@ -10,11 +10,24 @@ namespace :users do if user.decorate.threatmetrix_review_pending? && user.proofing_component.review_eligible? result = ProofingComponent.find_by(user: user) result.update(threatmetrix_review_status: 'pass') - user.profiles. + + profile = user.profiles. where(deactivation_reason: 'threatmetrix_review_pending'). - first. - activate - puts "User's review state has been updated to #{result.threatmetrix_review_status}." + first + profile.activate + + event, disavowal_token = UserEventCreator.new(current_user: user). + create_out_of_band_user_event_with_disavowal(:account_verified) + + UserAlerts::AlertUserAboutAccountVerified.call( + user: user, + date_time: event.created_at, + sp_name: nil, + disavowal_token: disavowal_token, + ) + + status = result.threatmetrix_review_status + puts "User's review state has been updated to #{status} and the user has been emailed." elsif !user.proofing_component.review_eligible? puts 'User is past the 30 day review eligibility' else diff --git a/spec/lib/tasks/review_profile_spec.rb b/spec/lib/tasks/review_profile_spec.rb index 9af26481232..f1d3b3ad5ce 100644 --- a/spec/lib/tasks/review_profile_spec.rb +++ b/spec/lib/tasks/review_profile_spec.rb @@ -2,30 +2,47 @@ require 'rake' describe 'review_profile' do + let(:user) { create(:user, :deactivated_threatmetrix_profile) } + let(:task_name) { nil } + + subject(:invoke_task) do + Rake::Task[task_name].reenable + Rake::Task[task_name].invoke + end + before do Rake.application.rake_require('lib/tasks/review_profile', [Rails.root.to_s]) Rake::Task.define_task(:environment) + allow(STDIN).to receive(:getpass) { user.uuid } end describe 'users:review:pass' do - it 'sets threatmetrix_review_status to pass' do - user = create(:user, :deactivated_threatmetrix_profile) - - allow(STDIN).to receive(:getpass) { user.uuid } + let(:task_name) { 'users:review:pass' } - Rake::Task['users:review:pass'].invoke + it 'sets threatmetrix_review_status to pass' do + invoke_task expect(user.reload.proofing_component.threatmetrix_review_status).to eq('pass') expect(user.reload.profiles.first.active).to eq(true) end + + it 'dispatches account verified alert' do + freeze_time do + expect(UserAlerts::AlertUserAboutAccountVerified).to receive(:call).with( + user: user, + date_time: Time.zone.now, + disavowal_token: kind_of(String), + sp_name: nil, + ) + invoke_task + end + end end describe 'users:review:reject' do - it 'sets threatmetrix_review_status to reject' do - user = create(:user, :deactivated_threatmetrix_profile) + let(:task_name) { 'users:review:reject' } - allow(STDIN).to receive(:getpass) { user.uuid } - - Rake::Task['users:review:reject'].invoke + it 'sets threatmetrix_review_status to reject' do + invoke_task expect(user.reload.proofing_component.threatmetrix_review_status).to eq('reject') expect(user.reload.profiles.first.deactivation_reason).to eq('threatmetrix_review_rejected') end From 76e725e867711ac616710747f79a299553a551df Mon Sep 17 00:00:00 2001 From: Jonathan Hooper Date: Tue, 3 Jan 2023 12:32:35 -0600 Subject: [PATCH 02/18] LG-8520 Do not disable profile on GPO OTP verification if threatmetrix is not required to verify (#7567) When Threatmetrix is not required for verification we should not require it for verification after success GPO OTP submisison. This commit fixes a bug where a ThreatMetrix pass was required to receive an active profile after a successful GPO code entry regardless of whether the Threatmetrix requirement was active. [skip changelog] --- app/forms/gpo_verify_form.rb | 6 +++ .../steps/gpo_otp_verification_step_spec.rb | 44 ++++++++++++------- spec/forms/gpo_verify_form_spec.rb | 31 +++++++++++++ 3 files changed, 65 insertions(+), 16 deletions(-) diff --git a/app/forms/gpo_verify_form.rb b/app/forms/gpo_verify_form.rb index 356c0c83f4c..70f080de79a 100644 --- a/app/forms/gpo_verify_form.rb +++ b/app/forms/gpo_verify_form.rb @@ -21,6 +21,8 @@ def submit if pending_in_person_enrollment? UspsInPersonProofing::EnrollmentHelper.schedule_in_person_enrollment(user, pii) pending_profile&.deactivate(:in_person_verification_pending) + elsif threatmetrix_check_failed? && threatmetrix_enabled? + pending_profile&.deactivate(:threatmetrix_review_pending) else activate_profile end @@ -78,6 +80,10 @@ def pending_in_person_enrollment? pending_profile&.proofing_components&.[]('document_check') == Idp::Constants::Vendors::USPS end + def threatmetrix_enabled? + IdentityConfig.store.lexisnexis_threatmetrix_required_to_verify + end + def threatmetrix_check_failed? status = pending_profile&.proofing_components&.[]('threatmetrix_review_status') !status.nil? && status != 'pass' diff --git a/spec/features/idv/steps/gpo_otp_verification_step_spec.rb b/spec/features/idv/steps/gpo_otp_verification_step_spec.rb index 9faacde0edb..3839c2e690f 100644 --- a/spec/features/idv/steps/gpo_otp_verification_step_spec.rb +++ b/spec/features/idv/steps/gpo_otp_verification_step_spec.rb @@ -24,6 +24,7 @@ end let(:user) { profile.user } let(:threatmetrix_enabled) { false } + let(:threatmetrix_required_to_verify) { false } let(:threatmetrix_review_status) { nil } let(:redirect_after_verification) { nil } let(:profile_should_be_active) { true } @@ -33,39 +34,50 @@ allow(IdentityConfig.store).to receive(:lexisnexis_threatmetrix_enabled). and_return(threatmetrix_enabled) allow(IdentityConfig.store).to receive(:lexisnexis_threatmetrix_required_to_verify). - and_return(threatmetrix_enabled) + and_return(threatmetrix_required_to_verify) end it_behaves_like 'gpo otp verification' context 'ThreatMetrix enabled' do let(:threatmetrix_enabled) { true } + let(:threatmetrix_required_to_verify) { true } context 'ThreatMetrix says "pass"' do let(:threatmetrix_review_status) { 'pass' } it_behaves_like 'gpo otp verification' end - # context 'ThreatMetrix says "review"' do - # let(:threatmetrix_review_status) { 'review' } - # let(:redirect_after_verification) { idv_setup_errors_path } - # let(:profile_should_be_active) { false } - # let(:expected_deactivation_reason) { 'threatmetrix_review_pending' } - # it_behaves_like 'gpo otp verification' - # end - - # context 'ThreatMetrix says "reject"' do - # let(:threatmetrix_review_status) { 'reject' } - # let(:redirect_after_verification) { idv_setup_errors_path } - # let(:profile_should_be_active) { false } - # let(:expected_deactivation_reason) { 'threatmetrix_review_pending' } - # it_behaves_like 'gpo otp verification' - # end + context 'ThreatMetrix says "review"' do + let(:threatmetrix_review_status) { 'review' } + let(:redirect_after_verification) { idv_setup_errors_path } + let(:profile_should_be_active) { false } + let(:expected_deactivation_reason) { 'threatmetrix_review_pending' } + it_behaves_like 'gpo otp verification' + end + + context 'ThreatMetrix says "reject"' do + let(:threatmetrix_review_status) { 'reject' } + let(:redirect_after_verification) { idv_setup_errors_path } + let(:profile_should_be_active) { false } + let(:expected_deactivation_reason) { 'threatmetrix_review_pending' } + it_behaves_like 'gpo otp verification' + end context 'No ThreatMetrix result on proofing component' do let(:threatmetrix_review_status) { nil } it_behaves_like 'gpo otp verification' end + + context 'without verification requirement enabled creates active profile' do + let(:threatmetrix_required_to_verify) { false } + + let(:threatmetrix_review_status) { 'review' } + let(:redirect_after_verification) { account_path } # TODO + let(:profile_should_be_active) { true } + let(:expected_deactivation_reason) { nil } + it_behaves_like 'gpo otp verification' + end end context 'with gpo feature disabled' do diff --git a/spec/forms/gpo_verify_form_spec.rb b/spec/forms/gpo_verify_form_spec.rb index 63e0b406199..fa8a6c79c23 100644 --- a/spec/forms/gpo_verify_form_spec.rb +++ b/spec/forms/gpo_verify_form_spec.rb @@ -170,10 +170,41 @@ expect(result.success?).to eq true end + it 'does not activate the users profile' do + subject.submit + profile = user.profiles.first + expect(profile.active).to eq(false) + expect(profile.deactivation_reason).to eq('threatmetrix_review_pending') + end + it 'notes that threatmetrix failed' do result = subject.submit expect(result.extra).to include(threatmetrix_check_failed: true) end + + context 'threatmetrix is not required for verification' do + before do + allow(IdentityConfig.store).to receive(:lexisnexis_threatmetrix_required_to_verify). + and_return(false) + end + + it 'returns true' do + result = subject.submit + expect(result.success?).to eq true + end + + it 'does activate the users profile' do + subject.submit + profile = user.profiles.first + expect(profile.active).to eq(true) + expect(profile.deactivation_reason).to eq(nil) + end + + it 'notes that threatmetrix failed' do + result = subject.submit + expect(result.extra).to include(threatmetrix_check_failed: true) + end + end end end From 0fb088beba4850abe7679a7672c227c79bad4b56 Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Tue, 3 Jan 2023 13:09:04 -0600 Subject: [PATCH 03/18] Update rubocop, brakeman, rspec, erb_lint, bullet, bootsnap gems (#7569) * Update Rubocop [skip changelog] * update rspec * Rubocop updates * update erb_lint * update bullet * update bootsnap * add brakeman ignore * fix spec --- .erb-lint.yml | 2 + .rubocop.yml | 4 +- Gemfile | 8 +-- Gemfile.lock | 67 +++++++++---------- app/controllers/users/sessions_controller.rb | 2 +- config/brakeman.ignore | 36 +++++++++- .../encryptors/pii_encryptor_spec.rb | 2 +- 7 files changed, 77 insertions(+), 44 deletions(-) diff --git a/.erb-lint.yml b/.erb-lint.yml index 78dd764abf1..5ff53cbe0b2 100644 --- a/.erb-lint.yml +++ b/.erb-lint.yml @@ -23,3 +23,5 @@ linters: suggestion: 'Rename classes that are known to be hidden by the Hush plugin' SpaceAroundErbTag: enabled: true + CommentSyntax: + enabled: true diff --git a/.rubocop.yml b/.rubocop.yml index 67cecbea01c..f35456ac4f0 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -636,7 +636,7 @@ Metrics/BlockLength: CountComments: false Enabled: true Max: 25 - IgnoredMethods: + AllowedMethods: - Struct.new - RedactedStruct.new Exclude: @@ -1080,7 +1080,7 @@ Style/LineEndConcatenation: Style/MethodCallWithoutArgsParentheses: Enabled: true - IgnoredMethods: [] + AllowedMethods: [] Style/MethodDefParentheses: Enabled: true diff --git a/Gemfile b/Gemfile index ef0dfb4b035..ee122388156 100644 --- a/Gemfile +++ b/Gemfile @@ -93,7 +93,7 @@ group :development, :test do gem 'bullet', '~> 7.0' gem 'capybara-webmock', git: 'https://github.com/hashrocket/capybara-webmock.git', ref: '63d790a0' gem 'data_uri', require: false - gem 'erb_lint', '~> 0.1.0', require: false + gem 'erb_lint', '~> 0.3.0', require: false gem 'i18n-tasks', '>= 0.9.31' gem 'knapsack' gem 'nokogiri', '~> 1.13.10' @@ -104,9 +104,9 @@ group :development, :test do gem 'pry-rails' gem 'psych' gem 'puma' - gem 'rspec-rails', '6.0.0.rc1' - gem 'rubocop', '~> 1.29.1', require: false - gem 'rubocop-performance', '~> 1.14.0', require: false + gem 'rspec-rails', '~> 6.0' + gem 'rubocop', '~> 1.42.0', require: false + gem 'rubocop-performance', '~> 1.15.0', require: false gem 'rubocop-rails', '>= 2.5.2', require: false end diff --git a/Gemfile.lock b/Gemfile.lock index bc0ac93cc05..926b669ed3b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -177,23 +177,22 @@ GEM coderay (>= 1.0.0) erubi (>= 1.0.0) rack (>= 0.9.0) - better_html (1.0.16) - actionview (>= 4.0) - activesupport (>= 4.0) + better_html (2.0.1) + actionview (>= 6.0) + activesupport (>= 6.0) ast (~> 2.0) erubi (~> 1.4) - html_tokenizer (~> 0.0.6) parser (>= 2.4) smart_properties bindata (2.4.10) binding_of_caller (1.0.0) debug_inspector (>= 0.0.1) - bootsnap (1.9.3) + bootsnap (1.9.4) msgpack (~> 1.0) - brakeman (5.2.1) + brakeman (5.4.0) browser (5.3.1) builder (3.2.4) - bullet (7.0.1) + bullet (7.0.7) activesupport (>= 3.0.0) uniform_notifier (~> 1.11) bundler-audit (0.9.0.1) @@ -260,16 +259,15 @@ GEM htmlentities (~> 4.3.3) launchy (~> 2.1) mail (~> 2.7) - erb_lint (0.1.1) + erb_lint (0.3.1) activesupport - better_html (~> 1.0.7) - html_tokenizer + better_html (>= 2.0.1) parser (>= 2.7.1.4) rainbow rubocop smart_properties errbase (0.2.1) - erubi (1.11.0) + erubi (1.12.0) et-orbi (1.2.7) tzinfo execjs (2.8.1) @@ -328,7 +326,6 @@ GEM heapy (0.2.0) thor highline (2.0.3) - html_tokenizer (0.0.7) htmlbeautifier (1.4.2) htmlentities (4.3.4) http_accept_language (2.1.1) @@ -352,6 +349,7 @@ GEM jmespath (1.6.1) jsbundling-rails (1.0.0) railties (>= 6.0.0) + json (2.6.3) jwe (0.4.0) jwt (2.4.1) knapsack (4.0.0) @@ -395,9 +393,9 @@ GEM method_source (1.0.0) mini_histogram (0.3.1) mini_mime (1.1.2) - mini_portile2 (2.8.0) - minitest (5.16.3) - msgpack (1.4.2) + mini_portile2 (2.8.1) + minitest (5.17.0) + msgpack (1.6.0) multiset (0.5.3) nenv (0.3.0) net-imap (0.2.3) @@ -436,7 +434,7 @@ GEM parallel (1.22.1) parallel_tests (3.7.3) parallel - parser (3.1.2.1) + parser (3.2.0.0) ast (~> 2.4.1) pg (1.4.5) pg_query (2.2.0) @@ -467,8 +465,8 @@ GEM puma (5.6.4) nio4r (~> 2.0) raabro (1.4.0) - racc (1.6.1) - rack (2.2.4) + racc (1.6.2) + rack (2.2.5) rack-attack (6.5.0) rack (>= 1.0, < 3) rack-cors (1.1.1) @@ -535,7 +533,7 @@ GEM redis-session-store (0.11.4) actionpack (>= 3, < 8) redis (>= 3, < 5) - regexp_parser (2.6.0) + regexp_parser (2.6.1) reline (0.2.7) io-console (~> 0.5) request_store (1.5.0) @@ -557,13 +555,13 @@ GEM rspec-mocks (~> 3.11.0) rspec-core (3.11.0) rspec-support (~> 3.11.0) - rspec-expectations (3.11.0) + rspec-expectations (3.11.1) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.11.0) - rspec-mocks (3.11.1) + rspec-mocks (3.11.2) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.11.0) - rspec-rails (6.0.0.rc1) + rspec-rails (6.0.1) actionpack (>= 6.1) activesupport (>= 6.1) railties (>= 6.1) @@ -573,21 +571,22 @@ GEM rspec-support (~> 3.11) rspec-retry (0.6.2) rspec-core (> 3.3) - rspec-support (3.11.0) + rspec-support (3.11.1) rspec_junit_formatter (0.6.0) rspec-core (>= 2, < 4, != 2.12.0) - rubocop (1.29.1) + rubocop (1.42.0) + json (~> 2.3) parallel (~> 1.10) - parser (>= 3.1.0.0) + parser (>= 3.1.2.1) rainbow (>= 2.2.2, < 4.0) regexp_parser (>= 1.8, < 3.0) rexml (>= 3.2.5, < 4.0) - rubocop-ast (>= 1.17.0, < 2.0) + rubocop-ast (>= 1.24.1, < 2.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 1.4.0, < 3.0) - rubocop-ast (1.21.0) + rubocop-ast (1.24.1) parser (>= 3.1.1.0) - rubocop-performance (1.14.3) + rubocop-performance (1.15.2) rubocop (>= 1.7.0, < 2.0) rubocop-ast (>= 0.4.0) rubocop-rails (2.12.4) @@ -664,8 +663,8 @@ GEM unf (0.1.4) unf_ext unf_ext (0.0.8) - unicode-display_width (2.3.0) - uniform_notifier (1.14.2) + unicode-display_width (2.4.0) + uniform_notifier (1.16.0) uuid (2.3.9) macaddr (~> 1.0) valid_email (0.1.4) @@ -747,7 +746,7 @@ DEPENDENCIES devise (~> 4.8) dotiw (>= 4.0.1) email_spec - erb_lint (~> 0.1.0) + erb_lint (~> 0.3.0) factory_bot_rails (>= 6.2.0) faker faraday (~> 2) @@ -804,11 +803,11 @@ DEPENDENCIES retries rotp (~> 6.1) rqrcode - rspec-rails (= 6.0.0.rc1) + rspec-rails (~> 6.0) rspec-retry rspec_junit_formatter - rubocop (~> 1.29.1) - rubocop-performance (~> 1.14.0) + rubocop (~> 1.42.0) + rubocop-performance (~> 1.15.0) rubocop-rails (>= 2.5.2) ruby-progressbar ruby-saml diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index 9819ac4b3e3..2fbe9f03162 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -221,7 +221,7 @@ def pending_account_reset_request ).call end - LETTERS_AND_DASHES = /\A[a-z0-9\-]+\Z/i + LETTERS_AND_DASHES = /\A[a-z0-9-]+\Z/i def request_id_if_valid request_id = (params[:request_id] || sp_session[:request_id]).to_s diff --git a/config/brakeman.ignore b/config/brakeman.ignore index 5e8d40af348..7bfec532fb1 100644 --- a/config/brakeman.ignore +++ b/config/brakeman.ignore @@ -29,6 +29,9 @@ }, "user_input": "params[:step]", "confidence": "Weak", + "cwe_id": [ + 22 + ], "note": "" }, { @@ -60,8 +63,34 @@ }, "user_input": "params[:step]", "confidence": "Weak", + "cwe_id": [ + 22 + ], "note": "" }, + { + "warning_type": "Weak Cryptography", + "warning_code": 126, + "fingerprint": "62a8c37ff0f723d2ebbbbf64c443a21632a2dcdc87fd20e6f61c2cec323482d2", + "check_name": "WeakRSAKey", + "message": "Use of padding mode PKCS1 (default if not specified), which is known to be insecure. Use OAEP instead", + "file": "app/services/irs_attempts_api/envelope_encryptor.rb", + "line": 19, + "link": "https://brakemanscanner.org/docs/warning_types/weak_cryptography/", + "code": "OpenSSL::PKey::RSA.new(Base64.strict_decode64(public_key_str)).public_encrypt(OpenSSL::Cipher.new(\"aes-256-cbc\").random_key)", + "render_path": null, + "location": { + "type": "method", + "class": "IrsAttemptsApi::EnvelopeEncryptor", + "method": "s(:self).encrypt" + }, + "user_input": null, + "confidence": "High", + "cwe_id": [ + 780 + ], + "note": "This is necessary due to the parameters of the IRS systems that we integrate with." + }, { "warning_type": "Dynamic Render Path", "warning_code": 15, @@ -91,9 +120,12 @@ }, "user_input": "params[:step]", "confidence": "Weak", + "cwe_id": [ + 22 + ], "note": "" } ], - "updated": "2022-07-05 11:19:47 -0400", - "brakeman_version": "5.2.0" + "updated": "2023-01-03 12:29:54 -0600", + "brakeman_version": "5.4.0" } diff --git a/spec/services/encryption/encryptors/pii_encryptor_spec.rb b/spec/services/encryption/encryptors/pii_encryptor_spec.rb index 1c68f366901..8cc2491e0b8 100644 --- a/spec/services/encryption/encryptors/pii_encryptor_spec.rb +++ b/spec/services/encryption/encryptors/pii_encryptor_spec.rb @@ -90,7 +90,7 @@ kms_client = instance_double(Encryption::KmsClient) expect(Encryption::KmsClient).to receive(:new).and_return(kms_client) expect(kms_client).to receive(:decrypt). - with('kms_ciphertext', 'context' => 'pii-encryption', 'user_uuid' => 'uuid-123-abc'). + with('kms_ciphertext', { 'context' => 'pii-encryption', 'user_uuid' => 'uuid-123-abc' }). and_return('aes_ciphertext') cipher = instance_double(Encryption::AesCipher) From 4fb2f3e0ccabba006171479a1d715e0a8efe3ae1 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Tue, 3 Jan 2023 11:24:28 -0800 Subject: [PATCH 04/18] Remove some indirection in IdentityLinker (#7420) - Remove more intermediate methods - Inline a bunch - Spec cleanup * Fix regression in verified_attributes * Remove method name that was a lie and inline [skip changelog] --- app/services/identity_linker.rb | 72 ++++++++++++--------------- spec/services/identity_linker_spec.rb | 30 +++++++++-- 2 files changed, 57 insertions(+), 45 deletions(-) diff --git a/app/services/identity_linker.rb b/app/services/identity_linker.rb index 665297f01c9..d8d8a06c07f 100644 --- a/app/services/identity_linker.rb +++ b/app/services/identity_linker.rb @@ -7,19 +7,41 @@ def initialize(user, service_provider) @ial = nil end - def link_identity(**extra_attrs) + def link_identity( + code_challenge: nil, + ial: nil, + nonce: nil, + rails_session_id: nil, + scope: nil, + verified_attributes: nil, + last_consented_at: nil, + clear_deleted_at: nil + ) return unless user && service_provider.present? - process_ial(extra_attrs) - attributes = merged_attributes(extra_attrs) - identity.update!(attributes) + process_ial(ial) + + identity.update!( + identity_attributes.merge( + code_challenge: code_challenge, + ial: ial, + nonce: nonce, + rails_session_id: rails_session_id, + scope: scope, + verified_attributes: combined_verified_attributes(verified_attributes), + ).tap do |hash| + hash[:last_consented_at] = last_consented_at if last_consented_at + hash[:deleted_at] = nil if clear_deleted_at + end, + ) + AgencyIdentityLinker.new(identity).link_identity identity end private - def process_ial(extra_attrs) - @ial = extra_attrs[:ial] + def process_ial(ial) + @ial = ial now = Time.zone.now process_ial_at(now) process_verified_at(now) @@ -39,15 +61,7 @@ def process_verified_at(now) end def identity - @identity ||= find_or_create_identity_with_costing - end - - def find_or_create_identity_with_costing - user.identities.create_or_find_by(service_provider: service_provider.issuer) - end - - def merged_attributes(extra_attrs) - identity_attributes.merge(optional_attributes(**extra_attrs)) + @identity ||= user.identities.create_or_find_by(service_provider: service_provider.issuer) end def identity_attributes @@ -58,31 +72,7 @@ def identity_attributes } end - def optional_attributes( - code_challenge: nil, - ial: nil, - nonce: nil, - rails_session_id: nil, - scope: nil, - verified_attributes: nil, - last_consented_at: nil, - clear_deleted_at: nil - ) - { - code_challenge: code_challenge, - ial: ial, - nonce: nonce, - rails_session_id: rails_session_id, - scope: scope, - verified_attributes: merge_attributes(verified_attributes), - }.tap do |hash| - hash[:last_consented_at] = last_consented_at if last_consented_at - hash[:deleted_at] = nil if clear_deleted_at - end - end - - def merge_attributes(verified_attributes) - verified_attributes = verified_attributes.to_a.map(&:to_s) - (identity.verified_attributes.to_a + verified_attributes).uniq.sort + def combined_verified_attributes(verified_attributes) + [*identity.verified_attributes, *verified_attributes.to_a.map(&:to_s)].uniq.sort end end diff --git a/spec/services/identity_linker_spec.rb b/spec/services/identity_linker_spec.rb index 346b73cb011..53e05b3b33e 100644 --- a/spec/services/identity_linker_spec.rb +++ b/spec/services/identity_linker_spec.rb @@ -32,6 +32,7 @@ ial = 3 scope = 'openid profile email' code_challenge = SecureRandom.hex + verified_attributes = %w[address email] IdentityLinker.new(user, service_provider).link_identity( rails_session_id: rails_session_id, @@ -39,6 +40,7 @@ ial: ial, scope: scope, code_challenge: code_challenge, + verified_attributes: verified_attributes.map(&:to_sym), ) user.reload @@ -48,13 +50,14 @@ expect(last_identity.ial).to eq(ial) expect(last_identity.scope).to eq(scope) expect(last_identity.code_challenge).to eq(code_challenge) + expect(last_identity.verified_attributes).to eq(verified_attributes) end context 'identity.last_consented_at' do let(:now) { Time.zone.now } let(:six_months_ago) { 6.months.ago } - it 'does override a previous last_consented_at by default' do + it 'does not override a previous last_consented_at by default' do IdentityLinker.new(user, service_provider). link_identity(last_consented_at: six_months_ago) last_identity = user.reload.last_identity @@ -74,7 +77,26 @@ end end - context 'clear_deleted_at' do + context 'identity.verified_attributes' do + before do + IdentityLinker.new(user, service_provider).link_identity( + verified_attributes: %i[address email], + ) + end + + it 'adds to verified_attributes' do + expect do + IdentityLinker.new(user, service_provider).link_identity( + verified_attributes: %i[all_emails verified_at], + ) + end.to( + change { user.identities.last.verified_attributes }. + to(%w[address all_emails email verified_at]), + ) + end + end + + context ':clear_deleted_at' do let(:yesterday) { 1.day.ago } before do @@ -88,7 +110,7 @@ link_identity(clear_deleted_at: clear_deleted_at) end - context 'clear_deleted_at is nil' do + context ':clear_deleted_at is nil' do let(:clear_deleted_at) { nil } it 'nulls out deleted_at' do @@ -98,7 +120,7 @@ end end - context 'clear_deleted_at is true' do + context ':clear_deleted_at is true' do let(:clear_deleted_at) { true } it 'nulls out deleted_at' do From e89aa29bb6f9189e50d72edf747b8ed636fc25eb Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 3 Jan 2023 15:18:07 -0500 Subject: [PATCH 05/18] Remove fetch stubbing in AddressSearch spec (#7564) * Remove fetch stubbing in AddressSearch spec See: https://github.com/18F/identity-idp/pull/7538/files#r1060682580 [skip changelog] * Another one --- .../document-capture/components/address-search.spec.tsx | 3 --- .../components/in-person-location-step.spec.tsx | 3 --- 2 files changed, 6 deletions(-) diff --git a/app/javascript/packages/document-capture/components/address-search.spec.tsx b/app/javascript/packages/document-capture/components/address-search.spec.tsx index af85cf815eb..936e5609c6d 100644 --- a/app/javascript/packages/document-capture/components/address-search.spec.tsx +++ b/app/javascript/packages/document-capture/components/address-search.spec.tsx @@ -1,7 +1,6 @@ import { render } from '@testing-library/react'; import { useSandbox } from '@18f/identity-test-helpers'; import userEvent from '@testing-library/user-event'; -import { fetch } from 'whatwg-fetch'; import { setupServer } from 'msw/node'; import { rest } from 'msw'; import type { SetupServerApi } from 'msw/node'; @@ -26,7 +25,6 @@ describe('AddressSearch', () => { let server: SetupServerApi; before(() => { - global.window.fetch = fetch; server = setupServer( rest.post(LOCATIONS_URL, (_req, res, ctx) => res(ctx.json([{ name: 'Baltimore' }]))), rest.post(ADDRESS_SEARCH_URL, (_req, res, ctx) => res(ctx.json(DEFAULT_RESPONSE))), @@ -36,7 +34,6 @@ describe('AddressSearch', () => { after(() => { server.close(); - global.window.fetch = () => Promise.reject(new Error('Fetch must be stubbed')); }); it('fires the callback with correct input', async () => { diff --git a/app/javascript/packages/document-capture/components/in-person-location-step.spec.tsx b/app/javascript/packages/document-capture/components/in-person-location-step.spec.tsx index b0370f8a693..e2a1b696d3b 100644 --- a/app/javascript/packages/document-capture/components/in-person-location-step.spec.tsx +++ b/app/javascript/packages/document-capture/components/in-person-location-step.spec.tsx @@ -3,7 +3,6 @@ import { useContext } from 'react'; import { render } from '@testing-library/react'; import { getAllByRole } from '@testing-library/dom'; import userEvent from '@testing-library/user-event'; -import { fetch } from 'whatwg-fetch'; import { setupServer } from 'msw/node'; import { rest } from 'msw'; import type { SetupServerApi } from 'msw/node'; @@ -37,7 +36,6 @@ const DEFAULT_PROPS = { describe('InPersonLocationStep', () => { let server: SetupServerApi; before(() => { - global.window.fetch = fetch; server = setupServer( rest.post(LOCATIONS_URL, (_req, res, ctx) => res(ctx.json([{ name: 'Baltimore' }]))), rest.post(ADDRESS_SEARCH_URL, (_req, res, ctx) => res(ctx.json(DEFAULT_RESPONSE))), @@ -48,7 +46,6 @@ describe('InPersonLocationStep', () => { after(() => { server.close(); - global.window.fetch = () => Promise.reject(new Error('Fetch must be stubbed')); }); it('logs step submission with selected location', async () => { From 788c5ec49cf0f994953ccc973d4b40d5cea75b93 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 3 Jan 2023 15:22:45 -0500 Subject: [PATCH 06/18] Add component preview for One Time Code input (#7523) changelog: Internal, Components, Add One Time Code Input component to local development previews --- .../one_time_code_input_component_preview.rb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 spec/components/previews/one_time_code_input_component_preview.rb diff --git a/spec/components/previews/one_time_code_input_component_preview.rb b/spec/components/previews/one_time_code_input_component_preview.rb new file mode 100644 index 00000000000..7b055fcf224 --- /dev/null +++ b/spec/components/previews/one_time_code_input_component_preview.rb @@ -0,0 +1,18 @@ +class OneTimeCodeInputComponentPreview < BaseComponentPreview + # @!group Preview + # @display form true + def numeric + render(OneTimeCodeInputComponent.new(form: form_builder)) + end + + def alphanumeric + render(OneTimeCodeInputComponent.new(form: form_builder, numeric: false)) + end + # @!endgroup + + # @display form true + # @param numeric toggle + def workbench(numeric: false) + render(OneTimeCodeInputComponent.new(form: form_builder, numeric: numeric)) + end +end From 5291892743fd0982017be9b42bb298df22ac2c32 Mon Sep 17 00:00:00 2001 From: Matt Gardner Date: Tue, 3 Jan 2023 15:40:58 -0500 Subject: [PATCH 07/18] LG-8514: PO Search: Handle unhandled timeout errors (#7559) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * changelog: Upcoming Features, In-Person Proofing, return all pilot locations if there’s a failure * Flag locations if they are static pilot locations; use Rails.logger * Remove check on distance as this is no longer accurate --- .../in_person/usps_locations_controller.rb | 16 ++++----- .../components/address-search.tsx | 2 ++ .../components/in-person-locations.tsx | 11 +++--- .../usps_in_person_proofing/post_office.rb | 1 + .../usps_in_person_proofing/proofer.rb | 1 + config/ipp_pilot_usps_facilities.json | 35 +++++++++++-------- .../usps_locations_controller_spec.rb | 9 +++-- .../usps_in_person_proofing/proofer_spec.rb | 1 - 8 files changed, 46 insertions(+), 30 deletions(-) diff --git a/app/controllers/idv/in_person/usps_locations_controller.rb b/app/controllers/idv/in_person/usps_locations_controller.rb index ab84ab0b216..5135df024d0 100644 --- a/app/controllers/idv/in_person/usps_locations_controller.rb +++ b/app/controllers/idv/in_person/usps_locations_controller.rb @@ -13,7 +13,8 @@ class UspsLocationsController < ApplicationController # retrieve the list of nearby IPP Post Office locations with a POST request def index - usps_response = [] + response = [] + begin if IdentityConfig.store.arcgis_search_enabled candidate = UspsInPersonProofing::Applicant.new( @@ -21,17 +22,16 @@ def index city: search_params['city'], state: search_params['state'], zip_code: search_params['zip_code'] ) - usps_response = proofer.request_facilities(candidate) + response = proofer.request_facilities(candidate) else - usps_response = proofer.request_pilot_facilities + response = proofer.request_pilot_facilities end - rescue ActionController::ParameterMissing - usps_response = proofer.request_pilot_facilities - rescue Faraday::ConnectionFailed => _error - nil + rescue => err + Rails.logger.warn(err) + response = proofer.request_pilot_facilities end - render json: usps_response.to_json + render json: response.to_json end def proofer diff --git a/app/javascript/packages/document-capture/components/address-search.tsx b/app/javascript/packages/document-capture/components/address-search.tsx index 2c06f22dfe4..3f1bf38e985 100644 --- a/app/javascript/packages/document-capture/components/address-search.tsx +++ b/app/javascript/packages/document-capture/components/address-search.tsx @@ -23,6 +23,7 @@ export interface PostOffice { weekday_hours: string; zip_code_4: string; zip_code_5: string; + is_pilot: boolean; } export interface LocationQuery { @@ -55,6 +56,7 @@ const formatLocation = (postOffices: PostOffice[]) => { sundayHours: po.sunday_hours, tty: po.tty, weekdayHours: po.weekday_hours, + isPilot: !!po.is_pilot, } as FormattedLocation; formattedLocations.push(location); }); diff --git a/app/javascript/packages/document-capture/components/in-person-locations.tsx b/app/javascript/packages/document-capture/components/in-person-locations.tsx index e354de6fafa..d0727730200 100644 --- a/app/javascript/packages/document-capture/components/in-person-locations.tsx +++ b/app/javascript/packages/document-capture/components/in-person-locations.tsx @@ -12,6 +12,7 @@ export interface FormattedLocation { streetAddress: string; sundayHours: string; weekdayHours: string; + isPilot: boolean; } interface InPersonLocationsProps { @@ -22,6 +23,7 @@ interface InPersonLocationsProps { function InPersonLocations({ locations, onSelect, address }: InPersonLocationsProps) { const { t } = useI18n(); + const isPilot = locations?.some((l) => l.isPilot); if (locations?.length === 0) { return ( @@ -37,10 +39,11 @@ function InPersonLocations({ locations, onSelect, address }: InPersonLocationsPr return ( <>

- {t('in_person_proofing.body.location.po_search.results_description', { - address, - count: locations?.length, - })} + {!isPilot && + t('in_person_proofing.body.location.po_search.results_description', { + address, + count: locations?.length, + })}

{t('in_person_proofing.body.location.po_search.results_instructions')}

diff --git a/app/services/usps_in_person_proofing/post_office.rb b/app/services/usps_in_person_proofing/post_office.rb index 3febd3fccad..bdb987c8b65 100644 --- a/app/services/usps_in_person_proofing/post_office.rb +++ b/app/services/usps_in_person_proofing/post_office.rb @@ -11,6 +11,7 @@ module UspsInPersonProofing :weekday_hours, :zip_code_4, :zip_code_5, + :is_pilot, keyword_init: true, ) end diff --git a/app/services/usps_in_person_proofing/proofer.rb b/app/services/usps_in_person_proofing/proofer.rb index 4210627b77e..2c4f366353c 100644 --- a/app/services/usps_in_person_proofing/proofer.rb +++ b/app/services/usps_in_person_proofing/proofer.rb @@ -208,6 +208,7 @@ def parse_facilities(facilities) weekday_hours: hours['weekdayHours'], zip_code_4: post_office['zip4'], zip_code_5: post_office['zip5'], + is_pilot: post_office['isPilot'], ) end end diff --git a/config/ipp_pilot_usps_facilities.json b/config/ipp_pilot_usps_facilities.json index 168d83b2f9c..0654787c472 100644 --- a/config/ipp_pilot_usps_facilities.json +++ b/config/ipp_pilot_usps_facilities.json @@ -13,14 +13,15 @@ "sundayHours": "Closed" } ], - "distance": "1.0 mi", + "distance": null, "streetAddress": "900 E FAYETTE ST RM 118", "city": "BALTIMORE", "phone": "410-347-4202", "name": "BALTIMORE", "zip4": "9715", "state": "MD", - "zip5": "21233" + "zip5": "21233", + "isPilot": true }, { "parking": "Street", @@ -35,14 +36,15 @@ "sundayHours": "Closed" } ], - "distance": "0.1 mi", + "distance": null, "streetAddress": "6900 WISCONSIN AVE STE 100", "city": "CHEVY CHASE", "phone": "301-941-2670", "name": "BETHESDA", "zip4": "9996", "state": "MD", - "zip5": "20815" + "zip5": "20815", + "isPilot": true }, { "parking": "Lot", @@ -57,14 +59,15 @@ "sundayHours": "10:00 AM - 4:00 PM" } ], - "distance": "0.0 mi", + "distance": null, "streetAddress": "4005 WISCONSIN AVE NW", "city": "WASHINGTON", "phone": "202-842-3332", "name": "FRIENDSHIP", "zip4": "9997", "state": "DC", - "zip5": "20016" + "zip5": "20016", + "isPilot": true }, { "parking": "Lot", @@ -79,14 +82,15 @@ "sundayHours": "Closed" } ], - "distance": "2.1 mi", + "distance": null, "streetAddress": "900 BRENTWOOD RD NE", "city": "WASHINGTON", "phone": "202-636-1259", "name": "WASHINGTON", "zip4": "9998", "state": "DC", - "zip5": "20066" + "zip5": "20066", + "isPilot": true }, { "parking": "Street", @@ -101,14 +105,15 @@ "sundayHours": "Closed" } ], - "distance": "0.0 mi", + "distance": null, "streetAddress": "3118 WASHINGTON BLVD", "city": "ARLINGTON", "phone": "703-993-0072", "name": "ARLINGTON", "zip4": "9998", "state": "VA", - "zip5": "22201" + "zip5": "22201", + "isPilot": true }, { "parking": "Lot", @@ -123,14 +128,15 @@ "sundayHours": "Closed" } ], - "distance": "0.0 mi", + "distance": null, "streetAddress": "44715 PRENTICE DR", "city": "DULLES", "phone": "703-406-6291", "name": "ASHBURN", "zip4": "9996", "state": "VA", - "zip5": "20101" + "zip5": "20101", + "isPilot": true }, { "parking": "Lot", @@ -145,14 +151,15 @@ "sundayHours": "9:00 AM - 5:00 PM" } ], - "distance": "0.0 mi", + "distance": null, "streetAddress": "8409 LEE HWY", "city": "MERRIFIELD", "phone": "703-698-6377", "name": "MERRIFIELD", "zip4": "9998", "state": "VA", - "zip5": "22116" + "zip5": "22116", + "isPilot": true } ] } diff --git a/spec/controllers/idv/in_person/usps_locations_controller_spec.rb b/spec/controllers/idv/in_person/usps_locations_controller_spec.rb index 7be3c281e15..4382f76307d 100644 --- a/spec/controllers/idv/in_person/usps_locations_controller_spec.rb +++ b/spec/controllers/idv/in_person/usps_locations_controller_spec.rb @@ -131,12 +131,15 @@ end context 'with unsuccessful fetch' do + let(:exception) { Faraday::ConnectionFailed } + before do - exception = Faraday::ConnectionFailed.new('error') allow(proofer).to receive(:request_facilities).with(fake_address).and_raise(exception) + allow(proofer).to receive(:request_pilot_facilities).and_return(pilot_locations) end - it 'gets an empty response' do + it 'returns all pilot locations' do + expect(Rails.logger).to receive(:warn) response = post :index, params: { address: { street_address: '742 Evergreen Terrace', city: 'Springfield', @@ -144,7 +147,7 @@ zip_code: '89011' } } json = response.body facilities = JSON.parse(json) - expect(facilities.length).to eq 0 + expect(facilities.length).to eq 4 end end end diff --git a/spec/services/usps_in_person_proofing/proofer_spec.rb b/spec/services/usps_in_person_proofing/proofer_spec.rb index d906eca641e..5c78d2b2ab6 100644 --- a/spec/services/usps_in_person_proofing/proofer_spec.rb +++ b/spec/services/usps_in_person_proofing/proofer_spec.rb @@ -55,7 +55,6 @@ def check_facility(facility) expect(facility.address).to be_present expect(facility.city).to be_present - expect(facility.distance).to be_present expect(facility.name).to be_present expect(facility.phone).to be_present expect(facility.saturday_hours).to be_present From bfac162dbc1342da37629703a31a2d4c62c5b64c Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Tue, 3 Jan 2023 15:09:39 -0600 Subject: [PATCH 08/18] Fix 500 error when submitting invalid email domain in reset password form (#7570) * add failing spec * Include translation helper in FormEmailValidator changelog: Bug Fixes, Reset Password, Fix 500 error when translating invalid domain error message --- app/validators/form_email_validator.rb | 1 + spec/forms/password_reset_email_form_spec.rb | 14 ++++++++++++++ 2 files changed, 15 insertions(+) diff --git a/app/validators/form_email_validator.rb b/app/validators/form_email_validator.rb index 9b822eff5ef..fc37679a33b 100644 --- a/app/validators/form_email_validator.rb +++ b/app/validators/form_email_validator.rb @@ -1,5 +1,6 @@ module FormEmailValidator extend ActiveSupport::Concern + include ActionView::Helpers::TranslationHelper included do include ActiveModel::Validations::Callbacks diff --git a/spec/forms/password_reset_email_form_spec.rb b/spec/forms/password_reset_email_form_spec.rb index 887e0829a33..80f8c4ca6e0 100644 --- a/spec/forms/password_reset_email_form_spec.rb +++ b/spec/forms/password_reset_email_form_spec.rb @@ -50,6 +50,20 @@ active_profile: false, ) end + + it 'returns false and adds errors to the form object when domain is invalid' do + subject = PasswordResetEmailForm.new('test@çà.com') + errors = { email: [t('valid_email.validations.email.invalid')] } + + expect(subject.submit.to_h).to include( + success: false, + errors: errors, + error_details: hash_including(*errors.keys), + user_id: 'nonexistent-uuid', + confirmed: false, + active_profile: false, + ) + end end end end From 869886f6e2e9b670d6217198a728bc993772e8ca Mon Sep 17 00:00:00 2001 From: jc-gsa <104452882+jc-gsa@users.noreply.github.com> Date: Tue, 3 Jan 2023 16:22:05 -0600 Subject: [PATCH 09/18] LG-8069: Inform user about OTP SMS send to unsupported phone type (#7506) Add notice for unsupported phone type changelog: Improvements, Multi-Factor Authentication, Inform user about unsupported phone type for SMS --- .../concerns/phone_confirmation.rb | 4 +- .../otp_verification_controller.rb | 5 +++ .../users/phone_setup_controller.rb | 10 ++++- app/controllers/users/phones_controller.rb | 5 ++- app/forms/new_phone_form.rb | 45 ++++++++++--------- .../phone_delivery_presenter.rb | 18 ++++++++ app/services/phone_number_capabilities.rb | 1 + .../otp_verification/show.html.erb | 6 +++ .../locales/two_factor_authentication/en.yml | 3 ++ .../locales/two_factor_authentication/es.yml | 4 ++ .../locales/two_factor_authentication/fr.yml | 4 ++ lib/telephony/test/sms_sender.rb | 16 ++++++- .../users/phone_setup_controller_spec.rb | 2 +- spec/forms/new_phone_form_spec.rb | 8 ++++ .../phone_delivery_presenter_spec.rb | 25 +++++++++++ .../otp_verification/show.html.erb_spec.rb | 34 +++++++++++--- 16 files changed, 157 insertions(+), 33 deletions(-) diff --git a/app/controllers/concerns/phone_confirmation.rb b/app/controllers/concerns/phone_confirmation.rb index 43cb4c4ef88..2b014db6cdc 100644 --- a/app/controllers/concerns/phone_confirmation.rb +++ b/app/controllers/concerns/phone_confirmation.rb @@ -1,8 +1,10 @@ module PhoneConfirmation def prompt_to_confirm_phone(id:, phone:, selected_delivery_method: nil, - selected_default_number: nil) + selected_default_number: nil, phone_type: nil) + user_session[:unconfirmed_phone] = phone user_session[:context] = 'confirmation' + user_session[:phone_type] = phone_type.to_s redirect_to otp_send_url( otp_delivery_selection_form: { diff --git a/app/controllers/two_factor_authentication/otp_verification_controller.rb b/app/controllers/two_factor_authentication/otp_verification_controller.rb index 9b834a58292..5c55f13f757 100644 --- a/app/controllers/two_factor_authentication/otp_verification_controller.rb +++ b/app/controllers/two_factor_authentication/otp_verification_controller.rb @@ -13,6 +13,7 @@ class OtpVerificationController < ApplicationController def show analytics.multi_factor_auth_enter_otp_visit(**analytics_properties) + @landline_alert = landline_warning? @presenter = presenter_for_two_factor_authentication_method end @@ -56,6 +57,10 @@ def phone_enabled? TwoFactorAuthentication::PhonePolicy.new(current_user).enabled? end + def landline_warning? + user_session[:phone_type] == 'landline' && two_factor_authentication_method == 'sms' + end + def confirm_voice_capability return if two_factor_authentication_method == 'sms' diff --git a/app/controllers/users/phone_setup_controller.rb b/app/controllers/users/phone_setup_controller.rb index 1bf7140f7a7..449b836048c 100644 --- a/app/controllers/users/phone_setup_controller.rb +++ b/app/controllers/users/phone_setup_controller.rb @@ -11,7 +11,10 @@ class PhoneSetupController < ApplicationController helper_method :in_multi_mfa_selection_flow? def index - @new_phone_form = NewPhoneForm.new(current_user) + @new_phone_form = NewPhoneForm.new( + current_user, + setup_voice_preference: setup_voice_preference?, + ) track_phone_setup_visit end @@ -45,6 +48,10 @@ def set_setup_presenter ) end + def setup_voice_preference? + params[:otp_delivery_preference].to_s == 'voice' + end + def user_opted_remember_device_cookie cookies.encrypted[:user_opted_remember_device_preference] end @@ -55,6 +62,7 @@ def handle_create_success(phone) id: nil, phone: @new_phone_form.phone, selected_delivery_method: @new_phone_form.otp_delivery_preference, + phone_type: @new_phone_form.phone_info&.type, ) else flash[:error] = t('errors.messages.phone_duplicate') diff --git a/app/controllers/users/phones_controller.rb b/app/controllers/users/phones_controller.rb index 5fd3aa909e1..c166c0fd3cd 100644 --- a/app/controllers/users/phones_controller.rb +++ b/app/controllers/users/phones_controller.rb @@ -39,9 +39,10 @@ def user_params def confirm_phone flash[:info] = t('devise.registrations.phone_update_needs_confirmation') prompt_to_confirm_phone( - id: user_session[:phone_id], phone: @new_phone_form.phone, + id: user_session[:phone_id], + phone: @new_phone_form.phone, selected_delivery_method: @new_phone_form.otp_delivery_preference, - selected_default_number: @new_phone_form.otp_make_default_number + selected_default_number: @new_phone_form.otp_make_default_number, ) end diff --git a/app/forms/new_phone_form.rb b/app/forms/new_phone_form.rb index af5228822c3..75ec6a21b2f 100644 --- a/app/forms/new_phone_form.rb +++ b/app/forms/new_phone_form.rb @@ -16,12 +16,15 @@ class NewPhoneForm validate :validate_allowed_carrier attr_accessor :phone, :international_code, :otp_delivery_preference, - :otp_make_default_number + :otp_make_default_number, :setup_voice_preference - def initialize(user) + alias_method :setup_voice_preference?, :setup_voice_preference + + def initialize(user, setup_voice_preference: false) self.user = user self.otp_delivery_preference = user.otp_delivery_preference self.otp_make_default_number = false + self.setup_voice_preference = setup_voice_preference end def submit(params) @@ -38,7 +41,25 @@ def delivery_preference_sms? end def delivery_preference_voice? - VendorStatus.new.vendor_outage?(:sms) + VendorStatus.new.vendor_outage?(:sms) || setup_voice_preference? + end + + # @return [Telephony::PhoneNumberInfo, nil] + def phone_info + return @phone_info if defined?(@phone_info) + + if phone.blank? || !IdentityConfig.store.voip_check + @phone_info = nil + else + @phone_info = Telephony.phone_info(phone) + end + rescue Aws::Pinpoint::Errors::TooManyRequestsException + @warning_message = 'AWS pinpoint phone info rate limit' + @phone_info = Telephony::PhoneNumberInfo.new(type: :unknown) + rescue Aws::Pinpoint::Errors::BadRequestException + errors.add(:phone, :improbable_phone, type: :improbable_phone) + @redacted_phone = redact(phone) + @phone_info = Telephony::PhoneNumberInfo.new(type: :unknown) end private @@ -104,24 +125,6 @@ def validate_not_premium_rate end end - # @return [Telephony::PhoneNumberInfo, nil] - def phone_info - return @phone_info if defined?(@phone_info) - - if phone.blank? || !IdentityConfig.store.voip_check - @phone_info = nil - else - @phone_info = Telephony.phone_info(phone) - end - rescue Aws::Pinpoint::Errors::TooManyRequestsException - @warning_message = 'AWS pinpoint phone info rate limit' - @phone_info = Telephony::PhoneNumberInfo.new(type: :unknown) - rescue Aws::Pinpoint::Errors::BadRequestException - errors.add(:phone, :improbable_phone, type: :improbable_phone) - @redacted_phone = redact(phone) - @phone_info = Telephony::PhoneNumberInfo.new(type: :unknown) - end - def parsed_phone @parsed_phone ||= Phonelib.parse(phone) end diff --git a/app/presenters/two_factor_auth_code/phone_delivery_presenter.rb b/app/presenters/two_factor_auth_code/phone_delivery_presenter.rb index c4b37da22db..d69595a65bd 100644 --- a/app/presenters/two_factor_auth_code/phone_delivery_presenter.rb +++ b/app/presenters/two_factor_auth_code/phone_delivery_presenter.rb @@ -1,5 +1,9 @@ module TwoFactorAuthCode class PhoneDeliveryPresenter < TwoFactorAuthCode::GenericDeliveryPresenter + include ActionView::Helpers::UrlHelper + include ActionView::Helpers::TagHelper + include ActionView::Helpers::TranslationHelper + attr_reader :otp_delivery_preference, :otp_make_default_number, :unconfirmed_phone, @@ -19,6 +23,20 @@ def phone_number_message ) end + def landline_warning + t( + 'two_factor_authentication.otp_delivery_preference.landline_warning_html', + phone_setup_path: link_to( + phone_call_text, + phone_setup_path(otp_delivery_preference: 'voice'), + ), + ) + end + + def phone_call_text + t('two_factor_authentication.otp_delivery_preference.phone_call') + end + def fallback_question t('two_factor_authentication.phone_fallback.question') end diff --git a/app/services/phone_number_capabilities.rb b/app/services/phone_number_capabilities.rb index 32a24b65faa..04b84984bd6 100644 --- a/app/services/phone_number_capabilities.rb +++ b/app/services/phone_number_capabilities.rb @@ -53,6 +53,7 @@ def sms_only? def supports_sms? return false if country_code_data.nil? + supports_sms = country_code_data['supports_sms'] supports_sms_unconfirmed = country_code_data.fetch( diff --git a/app/views/two_factor_authentication/otp_verification/show.html.erb b/app/views/two_factor_authentication/otp_verification/show.html.erb index f76e0d445e5..71925e45e2b 100644 --- a/app/views/two_factor_authentication/otp_verification/show.html.erb +++ b/app/views/two_factor_authentication/otp_verification/show.html.erb @@ -1,5 +1,11 @@ <% title t('titles.enter_2fa_code.one_time_code') %> +<% if @landline_alert %> + <%= render AlertComponent.new(type: :warning, class: 'margin-bottom-2') do %> + <%= @presenter.landline_warning %> + <% end %> +<% end %> + <% if @presenter.otp_expiration.present? %> <%= render CountdownAlertComponent.new( show_at_remaining: IdentityConfig.store.otp_expiration_warning_seconds.seconds, diff --git a/config/locales/two_factor_authentication/en.yml b/config/locales/two_factor_authentication/en.yml index b91bfcd162c..71299564b62 100644 --- a/config/locales/two_factor_authentication/en.yml +++ b/config/locales/two_factor_authentication/en.yml @@ -83,7 +83,10 @@ en: otp_delivery_preference: instruction: You can change this selection the next time you sign in. If you entered a landline, please select “Phone call” below. + landline_warning_html: The phone number entered appears to be a landline + phone. Request a one-time code by %{phone_setup_path} instead. no_supported_options: We are unable to verify phone numbers from %{location} + phone_call: phone call sms: Text message (SMS) sms_unsupported: We are unable to send text messages to phone numbers in %{location}. title: How should we send you a code? diff --git a/config/locales/two_factor_authentication/es.yml b/config/locales/two_factor_authentication/es.yml index b8c8a9554bc..0f82520f03f 100644 --- a/config/locales/two_factor_authentication/es.yml +++ b/config/locales/two_factor_authentication/es.yml @@ -90,7 +90,11 @@ es: seguridad para ese número de teléfono. otp_delivery_preference: instruction: Puede cambiar esta selección la próxima vez que inicie sesión. + landline_warning_html: Al parecer el número ingresado pertenece a un teléfono + fijo. Mejor solicita un código de un solo uso por + %{phone_setup_path}. no_supported_options: No podemos verificar los números de teléfono de %{location} + phone_call: llamada telefónica sms: Mensaje de texto (SMS, sigla en inglés) sms_unsupported: No podemos enviar mensajes de texto a números de teléfono de %{location}. title: '¿Cómo deberíamos enviarle un código?' diff --git a/config/locales/two_factor_authentication/fr.yml b/config/locales/two_factor_authentication/fr.yml index 79b8ab3f19a..be0fa64a14b 100644 --- a/config/locales/two_factor_authentication/fr.yml +++ b/config/locales/two_factor_authentication/fr.yml @@ -92,8 +92,12 @@ fr: de sécurité à ce numéro de téléphone. otp_delivery_preference: instruction: Vous pouvez modifier cette sélection lors de votre prochaine connexion. + landline_warning_html: Le numéro de téléphone saisi semble être un téléphone + fixe. Demandez plutôt un code à usage unique par + %{phone_setup_path}. no_supported_options: Nous ne sommes pas en mesure de vérifier les numéros de téléphone de %{location} + phone_call: appel téléphonique sms: Message texte (SMS) sms_unsupported: Il est impossible d’envoyer des messages texte à des numéros de téléphone dans %{location}. diff --git a/lib/telephony/test/sms_sender.rb b/lib/telephony/test/sms_sender.rb index a34cecca3d5..f365ef74202 100644 --- a/lib/telephony/test/sms_sender.rb +++ b/lib/telephony/test/sms_sender.rb @@ -1,6 +1,8 @@ module Telephony module Test class SmsSender + LANDLINE_PHONE_NUMBER = '+1 225-555-3000' + # rubocop:disable Lint/UnusedMethodArgument def send(message:, to:, country_code:, otp: nil) error = ErrorSimulator.new.error_for_number(to) @@ -36,13 +38,23 @@ def phone_info(phone_number) error: error, ) else + type = phone_type(phone_number) + PhoneNumberInfo.new( - type: :mobile, - carrier: 'Test Mobile Carrier', + type: type, + carrier: "Test #{type.to_s.capitalize} Carrier", ) end end + def phone_type(phone_number) + if phone_number == LANDLINE_PHONE_NUMBER + :landline + else + :mobile + end + end + def success_response Response.new( success: true, diff --git a/spec/controllers/users/phone_setup_controller_spec.rb b/spec/controllers/users/phone_setup_controller_spec.rb index a3f15baf9ec..fa6742024db 100644 --- a/spec/controllers/users/phone_setup_controller_spec.rb +++ b/spec/controllers/users/phone_setup_controller_spec.rb @@ -23,7 +23,7 @@ expect(@analytics).to receive(:track_event). with('User Registration: phone setup visited', enabled_mfa_methods_count: 0) - expect(NewPhoneForm).to receive(:new).with(user) + expect(NewPhoneForm).to receive(:new).with(user, setup_voice_preference: false) get :index diff --git a/spec/forms/new_phone_form_spec.rb b/spec/forms/new_phone_form_spec.rb index 28d8e351075..ea8ea60cb66 100644 --- a/spec/forms/new_phone_form_spec.rb +++ b/spec/forms/new_phone_form_spec.rb @@ -373,6 +373,14 @@ expect(form.delivery_preference_voice?).to eq(true) end end + + context 'with setup_voice_preference present' do + subject(:form) { NewPhoneForm.new(user, setup_voice_preference: true) } + + it 'is true' do + expect(form.delivery_preference_voice?).to eq(true) + end + end end describe '#redact' do diff --git a/spec/presenters/two_factor_auth_code/phone_delivery_presenter_spec.rb b/spec/presenters/two_factor_auth_code/phone_delivery_presenter_spec.rb index a0068522f47..d835610e658 100644 --- a/spec/presenters/two_factor_auth_code/phone_delivery_presenter_spec.rb +++ b/spec/presenters/two_factor_auth_code/phone_delivery_presenter_spec.rb @@ -2,6 +2,7 @@ describe TwoFactorAuthCode::PhoneDeliveryPresenter do include Rails.application.routes.url_helpers + include ActionView::Helpers::UrlHelper let(:view) { ActionController::Base.new.view_context } let(:data) do @@ -57,6 +58,30 @@ end end + describe '#landline_warning' do + let(:landline_html) do + t( + 'two_factor_authentication.otp_delivery_preference.landline_warning_html', + phone_setup_path: link_to( + presenter.phone_call_text, + phone_setup_path(otp_delivery_preference: 'voice'), + ), + ) + end + + it 'returns translated landline warning html' do + expect(presenter.landline_warning).to eq landline_html + end + end + + describe '#phone_call_text' do + let(:phone_call) { t('two_factor_authentication.otp_delivery_preference.phone_call') } + + it 'returns translation for word phone call' do + expect(presenter.phone_call_text).to eq phone_call + end + end + def account_reset_cancel_link(account_reset_token) I18n.t( 'two_factor_authentication.account_reset.pending_html', cancel_link: diff --git a/spec/views/two_factor_authentication/otp_verification/show.html.erb_spec.rb b/spec/views/two_factor_authentication/otp_verification/show.html.erb_spec.rb index 9ec25747d9b..7237bce56da 100644 --- a/spec/views/two_factor_authentication/otp_verification/show.html.erb_spec.rb +++ b/spec/views/two_factor_authentication/otp_verification/show.html.erb_spec.rb @@ -26,6 +26,20 @@ allow(@presenter).to receive(:reauthn).and_return(false) end + it 'allow user to return to two factor options screen' do + render + expect(rendered).to have_link(t('two_factor_authentication.choose_another_option')) + end + + it 'does not show a landline setup warning' do + render + + expect(rendered).not_to have_link( + 'phone call', + href: phone_setup_path(otp_delivery_preference: 'voice'), + ) + end + context 'common OTP delivery screen behavior' do it 'has a localized title' do expect(view).to receive(:title).with(t('titles.enter_2fa_code.one_time_code')) @@ -40,11 +54,6 @@ end end - it 'allow user to return to two factor options screen' do - render - expect(rendered).to have_link(t('two_factor_authentication.choose_another_option')) - end - context 'OTP copy' do let(:help_text) do t( @@ -253,5 +262,20 @@ expect(rendered).to have_link(t('forms.two_factor.try_again'), href: phone_setup_path) end end + + context 'with landline setup warning' do + before do + assign(:landline_alert, true) + end + + it 'shows landline warning' do + render + + expect(rendered).to have_link( + 'phone call', + href: phone_setup_path(otp_delivery_preference: 'voice'), + ) + end + end end end From 78feec8b97002508fb3aacd82754ba168d3fa827 Mon Sep 17 00:00:00 2001 From: Jonathan Hooper Date: Wed, 4 Jan 2023 10:02:01 -0600 Subject: [PATCH 10/18] Remove unused `issuer` arg from `ResolutionProofingJob` (#7574) This arg used to be used to determine if ThreatMetrix should be used. This was in place because ThreatMetrix was only active for some service providers. A recent change made Threatmetrix active for all service providers. This argument was left in place with a `nil` default to prevent ArgumentErrors when the workers were running old and new code during deployment. Now that the code that used this argument is not running in any deployed environment the argument can be safely removed. [skip changelog] --- app/jobs/resolution_proofing_job.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/jobs/resolution_proofing_job.rb b/app/jobs/resolution_proofing_job.rb index 8a405c4a500..4e81098f6c5 100644 --- a/app/jobs/resolution_proofing_job.rb +++ b/app/jobs/resolution_proofing_job.rb @@ -19,8 +19,7 @@ def perform( should_proof_state_id:, user_id: nil, threatmetrix_session_id: nil, - request_ip: nil, - issuer: nil # rubocop:disable Lint:UnusedMethodArgument + request_ip: nil ) timer = JobHelpers::Timer.new From 83c0a1f7a63bf854e4ae5d65942b0e1aec556c18 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 4 Jan 2023 11:09:54 -0500 Subject: [PATCH 11/18] LG-8489: Improve YAML normalization error tolerance (#7573) * LG-8489: Improve YAML normalization error tolerance changelog: Internal, Build Tooling, Improve YAML normalization error tolerance * Use top-level await * Add YAML normalization to lintfix Make target * Restore parallelization See: https://github.com/18F/identity-idp/pull/7573/files#r1061615298 --- Makefile | 4 +++- app/javascript/packages/normalize-yaml/cli.js | 16 ++++++++++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index f2c86cbc1e8..ad5a27c3ad5 100644 --- a/Makefile +++ b/Makefile @@ -113,7 +113,7 @@ lint_yarn_lock: package.json yarn.lock lint_lockfiles: lint_gemfile_lock lint_yarn_lock ## Lints to ensure lockfiles are in sync -lintfix: ## Try to automatically fix any ruby, ERB, javascript, or CSS lint errors +lintfix: ## Try to automatically fix any Ruby, ERB, JavaScript, YAML, or CSS lint errors @echo "--- rubocop fix ---" bundle exec rubocop -a @echo "--- erblint fix ---" @@ -122,6 +122,8 @@ lintfix: ## Try to automatically fix any ruby, ERB, javascript, or CSS lint erro yarn lint --fix @echo "--- stylelint fix ---" yarn lint:css --fix + @echo "--- normalize yaml ---" + make normalize_yaml brakeman: ## Runs brakeman bundle exec brakeman diff --git a/app/javascript/packages/normalize-yaml/cli.js b/app/javascript/packages/normalize-yaml/cli.js index 1c417bb67a8..905d56f7d6f 100755 --- a/app/javascript/packages/normalize-yaml/cli.js +++ b/app/javascript/packages/normalize-yaml/cli.js @@ -1,4 +1,7 @@ #!/usr/bin/env node + +/* eslint-disable no-console */ + import { promises as fsPromises } from 'fs'; import { join } from 'path'; import prettier from 'prettier'; @@ -24,10 +27,19 @@ const options = { ), }; -Promise.all( +let exitCode = 0; + +await Promise.all( files.map(async (relativePath) => { const absolutePath = join(process.cwd(), relativePath); const content = await readFile(absolutePath, 'utf8'); - await writeFile(absolutePath, normalize(content, options)); + try { + await writeFile(absolutePath, normalize(content, options)); + } catch (error) { + console.error(`Error normalizing ${relativePath}: ${error.message}`); + exitCode = 1; + } }), ); + +process.exit(exitCode); From 5c9c7eb51d62632bc4fa721821e2456c41c1ed7a Mon Sep 17 00:00:00 2001 From: Matt Gardner Date: Wed, 4 Jan 2023 11:30:49 -0500 Subject: [PATCH 12/18] changelog: Internal, In-Person Proofing, Use map instead of iterating with forEach (#7571) --- .../components/address-search.tsx | 36 ++++++++----------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/app/javascript/packages/document-capture/components/address-search.tsx b/app/javascript/packages/document-capture/components/address-search.tsx index 3f1bf38e985..1ff1b5e2e96 100644 --- a/app/javascript/packages/document-capture/components/address-search.tsx +++ b/app/javascript/packages/document-capture/components/address-search.tsx @@ -42,26 +42,20 @@ interface Location { address: string; } -const formatLocation = (postOffices: PostOffice[]) => { - const formattedLocations = [] as FormattedLocation[]; - postOffices.forEach((po: PostOffice, index) => { - const location = { - formattedCityStateZip: `${po.city}, ${po.state}, ${po.zip_code_5}-${po.zip_code_4}`, - id: index, - distance: po.distance, - name: po.name, - phone: po.phone, - saturdayHours: po.saturday_hours, - streetAddress: po.address, - sundayHours: po.sunday_hours, - tty: po.tty, - weekdayHours: po.weekday_hours, - isPilot: !!po.is_pilot, - } as FormattedLocation; - formattedLocations.push(location); - }); - return formattedLocations; -}; +const formatLocations = (postOffices: PostOffice[]): FormattedLocation[] => + postOffices.map((po: PostOffice, index) => ({ + formattedCityStateZip: `${po.city}, ${po.state}, ${po.zip_code_5}-${po.zip_code_4}`, + id: index, + distance: po.distance, + name: po.name, + phone: po.phone, + saturdayHours: po.saturday_hours, + streetAddress: po.address, + sundayHours: po.sunday_hours, + tty: po.tty, + weekdayHours: po.weekday_hours, + isPilot: !!po.is_pilot, + })); export const snakeCase = (value: string) => value @@ -85,7 +79,7 @@ const requestUspsLocations = async (address: LocationQuery): Promise Date: Wed, 4 Jan 2023 09:36:47 -0800 Subject: [PATCH 13/18] Link to the handbook from README and CONTRIBUTING files (#7572) * Link to the handbook from README and CONTRIBUTING files * [skip changelog] --- CONTRIBUTING.md | 2 +- README.md | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 27c5745c7aa..bccbcba27d2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -4,7 +4,7 @@ We’re so glad you’re thinking about contributing to a Technology Transformat TTS is committed to building a safe, welcoming, harassment-free culture for everyone. We expect everyone on the TTS team and everyone within TTS spaces, including contributors to our projects, to follow the [TTS Code of Conduct](https://github.com/18F/code-of-conduct/blob/master/code-of-conduct.md). -We encourage you to read this project’s CONTRIBUTING policy (you are here), its [LICENSE](LICENSE.md), [README](README.md) +We encourage you to read this project’s CONTRIBUTING policy (you are here), its [LICENSE](LICENSE.md), and its [README](README.md). When you are ready to make a pull request, read our [pull request process](https://handbook.login.gov/articles/pull-request-review.html), which is a part of [the Login.gov Handbook](https://handbook.login.gov/). If you have any questions or want to read more, check out the [18F Open Source Policy GitHub repository]( https://github.com/18f/open-source-policy), or [send us an email](mailto:18f@gsa.gov). diff --git a/README.md b/README.md index 7df4c26f09a..8b8e695bb04 100644 --- a/README.md +++ b/README.md @@ -9,6 +9,8 @@ This repository contains the core code base and documentation for the identity m Thank you for your interest in contributing to the Login.gov IdP! For complete instructions on how to contribute code, please read through our [CONTRIBUTING.md](CONTRIBUTING.md) documentation. +You may also want to read the [the Login.gov team Handbook](https://handbook.login.gov/). + ## Creating your local development environment ### Installing on your local machine From 89ade749898a2df2c4ff0b8bc79208f72bd87e89 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Wed, 4 Jan 2023 13:14:18 -0500 Subject: [PATCH 14/18] LG-8389 - Log deactivation reason (#7540) * LG-8389 - Log deactivation reason [skip changelog] * Lint and broken spec. * Add deactivation_reason to personal key submitted event * Add deactivation_reason to docs For idv_review_complete and idv_final * Use have_logged_event matcher * Log review complete + final resolution events w/ deactivation_reason * Update specs Co-authored-by: Matt Hinz --- .../idv/personal_key_controller.rb | 5 +- app/controllers/idv/review_controller.rb | 10 +- app/services/analytics_events.rb | 11 ++- .../idv/personal_key_controller_spec.rb | 99 ++++++++++++++++--- .../controllers/idv/review_controller_spec.rb | 18 +++- spec/features/idv/analytics_spec.rb | 18 ++-- 6 files changed, 135 insertions(+), 26 deletions(-) diff --git a/app/controllers/idv/personal_key_controller.rb b/app/controllers/idv/personal_key_controller.rb index 5fe20e43966..580cd0ba3ee 100644 --- a/app/controllers/idv/personal_key_controller.rb +++ b/app/controllers/idv/personal_key_controller.rb @@ -19,7 +19,10 @@ def show def update user_session[:need_personal_key_confirmation] = false - analytics.idv_personal_key_submitted(address_verification_method: address_verification_method) + analytics.idv_personal_key_submitted( + address_verification_method: address_verification_method, + deactivation_reason: idv_session.profile&.deactivation_reason, + ) redirect_to next_step end diff --git a/app/controllers/idv/review_controller.rb b/app/controllers/idv/review_controller.rb index f5c3055eecc..6857fa40b10 100644 --- a/app/controllers/idv/review_controller.rb +++ b/app/controllers/idv/review_controller.rb @@ -58,8 +58,14 @@ def create redirect_to next_step - analytics.idv_review_complete(success: true) - analytics.idv_final(success: true) + analytics.idv_review_complete( + success: true, + deactivation_reason: idv_session.profile.deactivation_reason, + ) + analytics.idv_final( + success: true, + deactivation_reason: idv_session.profile.deactivation_reason, + ) return unless FeatureManagement.reveal_gpo_code? session[:last_gpo_confirmation_code] = idv_session.gpo_otp diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index cd4b517b227..8820890d808 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -1050,16 +1050,19 @@ def idv_intro_visit end # @param [Boolean] success + # @param [String, nil] deactivation_reason Reason user's profile was deactivated, if any. # @param [Idv::ProofingComponentsLogging] proofing_components User's current proofing components # Tracks the last step of IDV, indicates the user successfully prooved def idv_final( success:, + deactivation_reason: nil, proofing_components: nil, **extra ) track_event( 'IdV: final resolution', success: success, + deactivation_reason: deactivation_reason, proofing_components: proofing_components, **extra, ) @@ -1076,10 +1079,12 @@ def idv_personal_key_visited(proofing_components: nil, **extra) end # @param [Idv::ProofingComponentsLogging] proofing_components User's current proofing components + # @param [String, nil] deactivation_reason Reason profile was deactivated. # User submitted IDV personal key page - def idv_personal_key_submitted(proofing_components: nil, **extra) + def idv_personal_key_submitted(proofing_components: nil, deactivation_reason: nil, **extra) track_event( 'IdV: personal key submitted', + deactivation_reason: deactivation_reason, proofing_components: proofing_components, **extra, ) @@ -1382,10 +1387,12 @@ def idv_proofing_resolution_result_missing(proofing_components: nil, **extra) # User submitted IDV password confirm page # @param [Boolean] success # @param [Idv::ProofingComponentsLogging] proofing_components User's current proofing components - def idv_review_complete(success:, proofing_components: nil, **extra) + # @param [String, nil] deactivation_reason Reason user's profile was deactivated, if any. + def idv_review_complete(success:, deactivation_reason: nil, proofing_components: nil, **extra) track_event( 'IdV: review complete', success: success, + deactivation_reason: deactivation_reason, proofing_components: proofing_components, **extra, ) diff --git a/spec/controllers/idv/personal_key_controller_spec.rb b/spec/controllers/idv/personal_key_controller_spec.rb index b0475cdc7aa..02a468d1b2c 100644 --- a/spec/controllers/idv/personal_key_controller_spec.rb +++ b/spec/controllers/idv/personal_key_controller_spec.rb @@ -32,6 +32,10 @@ def stub_idv_session ) end + before do + stub_analytics + end + describe 'before_actions' do it 'includes before_actions from AccountStateChecker' do expect(subject).to have_actions( @@ -155,6 +159,16 @@ def index expect(subject.user_session[:need_personal_key_confirmation]).to eq(false) end + + it 'logs key submitted event' do + patch :update + + expect(@analytics).to have_logged_event( + 'IdV: personal key submitted', address_verification_method: nil, + deactivation_reason: nil, + proofing_components: nil + ) + end end context 'user selected gpo verification' do @@ -181,6 +195,16 @@ def index expect(response).to redirect_to idv_come_back_later_path end end + it 'logs key submitted event' do + patch :update + + expect(@analytics).to have_logged_event( + 'IdV: personal key submitted', + address_verification_method: nil, + deactivation_reason: 'gpo_verification_pending', + proofing_components: nil, + ) + end end context 'with in person profile' do @@ -196,6 +220,17 @@ def index expect(response).to redirect_to idv_in_person_ready_to_verify_url end + + it 'logs key submitted event' do + patch :update + + expect(@analytics).to have_logged_event( + 'IdV: personal key submitted', + address_verification_method: nil, + deactivation_reason: nil, + proofing_components: nil, + ) + end end context 'with device profiling decisioning enabled' do @@ -205,24 +240,66 @@ def index to receive(:lexisnexis_threatmetrix_required_to_verify).and_return(true) end - it 'redirects to account path when threatmetrix review status is nil' do - patch :update + context 'threatmetrix review status is nil' do + it 'redirects to account path' do + patch :update - expect(response).to redirect_to account_path + expect(response).to redirect_to account_path + end + it 'logs key submitted event' do + patch :update + + expect(@analytics).to have_logged_event( + 'IdV: personal key submitted', + address_verification_method: nil, + deactivation_reason: nil, + proofing_components: nil, + ) + end end - it 'redirects to account path when device profiling passes' do - ProofingComponent.find_by(user: user).update(threatmetrix_review_status: 'pass') - patch :update + context 'device profiling passes' do + before do + ProofingComponent.find_by(user: user).update(threatmetrix_review_status: 'pass') + end + it 'redirects to account path' do + patch :update - expect(response).to redirect_to account_path + expect(response).to redirect_to account_path + end + it 'logs key submitted event' do + patch :update + + expect(@analytics).to have_logged_event( + 'IdV: personal key submitted', + address_verification_method: nil, + deactivation_reason: nil, + proofing_components: nil, + ) + end end - it 'redirects to come back later path when device profiling fails' do - ProofingComponent.find_by(user: user).update(threatmetrix_review_status: 'fail') - patch :update + context 'device profiling fails' do + before do + ProofingComponent.find_by(user: user).update(threatmetrix_review_status: 'reject') + profile.deactivation_reason = :threatmetrix_review_pending + end + + it 'redirects to come back later path' do + patch :update + expect(response).to redirect_to idv_setup_errors_path + end - expect(response).to redirect_to idv_setup_errors_path + it 'logs key submitted event' do + patch :update + + expect(@analytics).to have_logged_event( + 'IdV: personal key submitted', + address_verification_method: nil, + deactivation_reason: 'threatmetrix_review_pending', + proofing_components: nil, + ) + end end end end diff --git a/spec/controllers/idv/review_controller_spec.rb b/spec/controllers/idv/review_controller_spec.rb index b09e12ee322..82d1289c759 100644 --- a/spec/controllers/idv/review_controller_spec.rb +++ b/spec/controllers/idv/review_controller_spec.rb @@ -277,6 +277,7 @@ def show 'IdV: review complete', success: false, proofing_components: nil, + deactivation_reason: nil, ) end end @@ -293,7 +294,8 @@ def show expect(@analytics).to have_logged_event( 'IdV: review complete', success: true, - proofing_components: nil + proofing_components: nil, + deactivation_reason: anything ) expect(@analytics).to have_logged_event( 'IdV: final resolution', @@ -609,6 +611,20 @@ def show expect(user.profiles.last.deactivation_reason).to eq('threatmetrix_review_pending') end + + it 'logs events' do + put :create, params: { user: { password: ControllerHelper::VALID_PASSWORD } } + expect(@analytics).to have_logged_event( + 'IdV: review complete', success: true, + proofing_components: nil, + deactivation_reason: 'threatmetrix_review_pending' + ) + expect(@analytics).to have_logged_event( + 'IdV: final resolution', success: true, + proofing_components: nil, + deactivation_reason: 'threatmetrix_review_pending' + ) + end end end diff --git a/spec/features/idv/analytics_spec.rb b/spec/features/idv/analytics_spec.rb index 54ecaa525ce..26eee59f8f0 100644 --- a/spec/features/idv/analytics_spec.rb +++ b/spec/features/idv/analytics_spec.rb @@ -35,11 +35,11 @@ 'IdV: phone confirmation otp visited' => { proofing_components: { document_check: 'mock', document_type: 'state_id', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'lexis_nexis_address' } }, 'IdV: phone confirmation otp submitted' => { success: true, code_expired: false, code_matches: true, second_factor_attempts_count: 0, second_factor_locked_at: nil, proofing_components: { document_check: 'mock', document_type: 'state_id', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'lexis_nexis_address' }, errors: {} }, 'IdV: review info visited' => { address_verification_method: 'phone', proofing_components: { document_check: 'mock', document_type: 'state_id', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'lexis_nexis_address' } }, - 'IdV: review complete' => { success: true, proofing_components: { document_check: 'mock', document_type: 'state_id', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'lexis_nexis_address' } }, - 'IdV: final resolution' => { success: true, proofing_components: { document_check: 'mock', document_type: 'state_id', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'lexis_nexis_address' } }, + 'IdV: review complete' => { success: true, proofing_components: { document_check: 'mock', document_type: 'state_id', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'lexis_nexis_address' }, deactivation_reason: nil }, + 'IdV: final resolution' => { success: true, proofing_components: { document_check: 'mock', document_type: 'state_id', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'lexis_nexis_address' }, deactivation_reason: nil }, 'IdV: personal key visited' => { address_verification_method: 'phone', proofing_components: { document_check: 'mock', document_type: 'state_id', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'lexis_nexis_address' } }, 'IdV: personal key acknowledgment toggled' => { checked: true, proofing_components: { document_check: 'mock', document_type: 'state_id', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'lexis_nexis_address' } }, - 'IdV: personal key submitted' => { address_verification_method: 'phone', proofing_components: { document_check: 'mock', document_type: 'state_id', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'lexis_nexis_address' } }, + 'IdV: personal key submitted' => { address_verification_method: 'phone', proofing_components: { document_check: 'mock', document_type: 'state_id', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'lexis_nexis_address' }, deactivation_reason: nil }, } end @@ -68,11 +68,11 @@ 'IdV: USPS address letter requested' => { resend: false, proofing_components: { document_check: 'mock', document_type: 'state_id', source_check: 'aamva', resolution_check: 'lexis_nexis' } }, 'IdV: review info visited' => { address_verification_method: 'gpo', proofing_components: { document_check: 'mock', document_type: 'state_id', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'gpo_letter' } }, 'IdV: USPS address letter enqueued' => { enqueued_at: Time.zone.now.utc, resend: false, proofing_components: { document_check: 'mock', document_type: 'state_id', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'gpo_letter' } }, - 'IdV: review complete' => { success: true, proofing_components: { document_check: 'mock', document_type: 'state_id', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'gpo_letter' } }, - 'IdV: final resolution' => { success: true, proofing_components: { document_check: 'mock', document_type: 'state_id', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'gpo_letter' } }, + 'IdV: review complete' => { success: true, proofing_components: { document_check: 'mock', document_type: 'state_id', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'gpo_letter' }, deactivation_reason: 'gpo_verification_pending' }, + 'IdV: final resolution' => { success: true, proofing_components: { document_check: 'mock', document_type: 'state_id', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'gpo_letter' }, deactivation_reason: 'gpo_verification_pending' }, 'IdV: personal key visited' => { address_verification_method: 'gpo', proofing_components: { document_check: 'mock', document_type: 'state_id', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'gpo_letter' } }, 'IdV: personal key acknowledgment toggled' => { checked: true, proofing_components: { document_check: 'mock', document_type: 'state_id', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'gpo_letter' } }, - 'IdV: personal key submitted' => { address_verification_method: 'gpo', proofing_components: { document_check: 'mock', document_type: 'state_id', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'gpo_letter' } }, + 'IdV: personal key submitted' => { address_verification_method: 'gpo', proofing_components: { document_check: 'mock', document_type: 'state_id', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'gpo_letter' }, deactivation_reason: 'gpo_verification_pending' }, 'IdV: come back later visited' => { proofing_components: { document_check: 'mock', document_type: 'state_id', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'gpo_letter' } }, } end @@ -110,11 +110,11 @@ 'IdV: phone confirmation otp visited' => { proofing_components: { address_check: 'lexis_nexis_address', document_check: 'usps', resolution_check: 'lexis_nexis', source_check: 'aamva' } }, 'IdV: phone confirmation otp submitted' => { success: true, code_expired: false, code_matches: true, second_factor_attempts_count: 0, second_factor_locked_at: nil, proofing_components: { document_check: 'usps', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'lexis_nexis_address' }, errors: {} }, 'IdV: review info visited' => { proofing_components: { document_check: 'usps', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'lexis_nexis_address' }, address_verification_method: 'phone' }, - 'IdV: review complete' => { success: true, proofing_components: { document_check: 'usps', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'lexis_nexis_address' } }, - 'IdV: final resolution' => { success: true, proofing_components: { document_check: 'usps', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'lexis_nexis_address' } }, + 'IdV: review complete' => { success: true, proofing_components: { document_check: 'usps', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'lexis_nexis_address' }, deactivation_reason: 'in_person_verification_pending' }, + 'IdV: final resolution' => { success: true, proofing_components: { document_check: 'usps', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'lexis_nexis_address' }, deactivation_reason: 'in_person_verification_pending' }, 'IdV: personal key visited' => { proofing_components: { document_check: 'usps', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'lexis_nexis_address' }, address_verification_method: 'phone' }, 'IdV: personal key acknowledgment toggled' => { checked: true, proofing_components: { document_check: 'usps', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'lexis_nexis_address' } }, - 'IdV: personal key submitted' => { proofing_components: { document_check: 'usps', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'lexis_nexis_address' }, address_verification_method: 'phone' }, + 'IdV: personal key submitted' => { proofing_components: { document_check: 'usps', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'lexis_nexis_address' }, address_verification_method: 'phone', deactivation_reason: 'in_person_verification_pending' }, 'IdV: in person ready to verify visited' => { proofing_components: { document_check: 'usps', source_check: 'aamva', resolution_check: 'lexis_nexis', address_check: 'lexis_nexis_address' } }, } end From e7b8fb56d73380c1550bd3952eb3f6d789fd194e Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Wed, 4 Jan 2023 14:58:39 -0600 Subject: [PATCH 15/18] Disable automatically generating hints, placeholders and labels for simple_form (#7539) * Disable automatically generating hints, placeholders and labels for simple_form changelog: Internal, Forms, Disable automatically generating hints and placeholders and labels for simple_form * additional optional label configuration --- config/initializers/simple_form.rb | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/config/initializers/simple_form.rb b/config/initializers/simple_form.rb index 2ee030df322..7649f642f9e 100644 --- a/config/initializers/simple_form.rb +++ b/config/initializers/simple_form.rb @@ -14,6 +14,9 @@ } config.wrappers :base do |b| + b.optional :hint + b.optional :label + b.optional :placeholder b.use :html5 b.use :input, class: 'field' end @@ -22,13 +25,13 @@ tag: 'div', class: 'margin-bottom-4' do |b| b.use :html5 - b.use :placeholder + b.optional :placeholder b.optional :maxlength b.optional :pattern b.optional :min_max b.optional :readonly - b.use :label, class: 'usa-label' - b.use :hint, wrap_with: { tag: 'div', class: 'usa-hint' } + b.optional :label, class: 'usa-label' + b.optional :hint, wrap_with: { tag: 'div', class: 'usa-hint' } b.use :input, class: 'display-block width-full field', error_class: 'usa-input--error' b.use :error, wrap_with: { tag: 'div', class: 'usa-error-message' } end @@ -39,9 +42,9 @@ config.wrappers :uswds_checkbox do |b| b.use :html5 - b.use :hint, wrap_with: { tag: 'div', class: 'usa-hint' } + b.optional :hint, wrap_with: { tag: 'div', class: 'usa-hint' } b.use :input, class: 'usa-checkbox__input', error_class: 'usa-input--error' - b.use :label, class: 'usa-checkbox__label' + b.optional :label, class: 'usa-checkbox__label' b.use :error, wrap_with: { tag: 'div', class: 'usa-error-message' } end @@ -59,9 +62,9 @@ item_label_class: item_label_class do |b| b.use :html5 b.wrapper :legend, tag: 'legend', class: legend_class do |ba| - ba.use :label_text + ba.optional :label_text end - b.use :hint, wrap_with: { tag: 'div', class: 'usa-hint margin-bottom-05' } + b.optional :hint, wrap_with: { tag: 'div', class: 'usa-hint margin-bottom-05' } b.wrapper :grid_row, tag: :div, class: 'grid-row margin-bottom-neg-1' do |gr| gr.wrapper :grid_column_radios, tag: :div, class: 'grid-col-fill' do |gc| gc.wrapper :column_wrapper, tag: :div, class: 'display-inline-block minw-full' do |cr| From 02651c29afcb9102d394a9606641ddf067dfd566 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Wed, 4 Jan 2023 13:27:53 -0800 Subject: [PATCH 16/18] Make sure analytics event params all have types (#7575) **Why**: breaks documentation [skip changelog] --- app/services/analytics_events.rb | 2 +- lib/analytics_events_documenter.rb | 6 ++++++ spec/lib/analytics_events_documenter_spec.rb | 16 ++++++++++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index 8820890d808..8e24980d28e 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -1400,7 +1400,7 @@ def idv_review_complete(success:, deactivation_reason: nil, proofing_components: # @param [Idv::ProofingComponentsLogging] proofing_components User's # current proofing components - # @param address_verification_method The method (phone or gpo) being + # @param [String] address_verification_method The method (phone or gpo) being # used to verify the user's identity # User visited IDV password confirm page def idv_review_info_visited(proofing_components: nil, diff --git a/lib/analytics_events_documenter.rb b/lib/analytics_events_documenter.rb index c0d88a7dedf..6198a865825 100644 --- a/lib/analytics_events_documenter.rb +++ b/lib/analytics_events_documenter.rb @@ -84,6 +84,7 @@ def require_extra_params? !!@require_extra_params end + # rubocop:disable Metrics/BlockLength # Checks for params that are missing documentation, and returns a list of # @return [Array] def missing_documentation @@ -118,9 +119,14 @@ def missing_documentation errors << "#{error_prefix} don't use * as an argument, remove all args or name args" end + method_object.tags('param').each do |tag| + errors << "#{error_prefix} #{tag.name} missing types" if !tag.types + end + errors end end + # rubocop:enable Metrics/BlockLength # @return [{ events: Array}] def as_json diff --git a/spec/lib/analytics_events_documenter_spec.rb b/spec/lib/analytics_events_documenter_spec.rb index 32a4e93d9af..16f2abd7043 100644 --- a/spec/lib/analytics_events_documenter_spec.rb +++ b/spec/lib/analytics_events_documenter_spec.rb @@ -148,6 +148,22 @@ def some_event(pii_like_keypaths:, **extra) end end + context 'when a method documents a param but leaves out types' do + let(:source_code) { <<~RUBY } + class AnalyticsEvents + # @param success + def some_event(success:, **extra) + track_event('Some Event') + end + end + RUBY + + it 'has an error documentation to be missing' do + expect(documenter.missing_documentation.first). + to include('some_event success missing types') + end + end + context 'when a method does not have a **extra param' do let(:require_extra_params) { true } From dc0c75227d109c7c5e10f4d7ad20a7b0f20b13be Mon Sep 17 00:00:00 2001 From: Osman Latif <109746710+olatifflexion@users.noreply.github.com> Date: Wed, 4 Jan 2023 15:49:55 -0600 Subject: [PATCH 17/18] LG-8185 IDV verification submitted event when timeout (#7579) * LG-8185 IDV verification submitted event when timeout changelog: Internal, Attempts API, Track event when timeout --- app/services/idv/steps/verify_base_step.rb | 33 ++++++++++++------- .../features/idv/doc_auth/verify_step_spec.rb | 26 +++++++++++++++ 2 files changed, 48 insertions(+), 11 deletions(-) diff --git a/app/services/idv/steps/verify_base_step.rb b/app/services/idv/steps/verify_base_step.rb index 4da2143059c..9425e82ad52 100644 --- a/app/services/idv/steps/verify_base_step.rb +++ b/app/services/idv/steps/verify_base_step.rb @@ -226,6 +226,10 @@ def process_async_state(current_async_state) delete_async mark_step_incomplete(:verify) @flow.analytics.idv_proofing_resolution_result_missing + log_idv_verification_submitted_event( + success: false, + failure_reason: { idv_verification: [:timeout] }, + ) elsif current_async_state.done? async_state_done(current_async_state) end @@ -244,18 +248,8 @@ def async_state_done(current_async_state) pii_like_keypaths: [[:errors, :ssn], [:response_body, :first_name]], }, ) - pii_from_doc = pii || {} - @flow.irs_attempts_api_tracker.idv_verification_submitted( + log_idv_verification_submitted_event( success: form_response.success?, - document_state: pii_from_doc[:state], - document_number: pii_from_doc[:state_id_number], - document_issued: pii_from_doc[:state_id_issued], - document_expiration: pii_from_doc[:state_id_expiration], - first_name: pii_from_doc[:first_name], - last_name: pii_from_doc[:last_name], - date_of_birth: pii_from_doc[:dob], - address: pii_from_doc[:address1], - ssn: pii_from_doc[:ssn], failure_reason: @flow.irs_attempts_api_tracker.parse_failure_reason(form_response), ) @@ -314,6 +308,23 @@ def idv_result_to_form_response( def redact(text) text.gsub(/[a-z]/i, 'X').gsub(/\d/i, '#') end + + def log_idv_verification_submitted_event(success: false, failure_reason: nil) + pii_from_doc = pii || {} + @flow.irs_attempts_api_tracker.idv_verification_submitted( + success: success, + document_state: pii_from_doc[:state], + document_number: pii_from_doc[:state_id_number], + document_issued: pii_from_doc[:state_id_issued], + document_expiration: pii_from_doc[:state_id_expiration], + first_name: pii_from_doc[:first_name], + last_name: pii_from_doc[:last_name], + date_of_birth: pii_from_doc[:dob], + address: pii_from_doc[:address1], + ssn: pii_from_doc[:ssn], + failure_reason: failure_reason, + ) + end end end end diff --git a/spec/features/idv/doc_auth/verify_step_spec.rb b/spec/features/idv/doc_auth/verify_step_spec.rb index 7872dcaee42..36c124e5406 100644 --- a/spec/features/idv/doc_auth/verify_step_spec.rb +++ b/spec/features/idv/doc_auth/verify_step_spec.rb @@ -303,6 +303,32 @@ click_idv_continue expect(page).to have_current_path(idv_phone_path) end + + it 'tracks attempts tracker event with failure reason' do + expect(fake_attempts_tracker).to receive(:idv_verification_submitted).with( + success: false, + failure_reason: { idv_verification: [:timeout] }, + document_state: 'MT', + document_number: '1111111111111', + document_issued: '2019-12-31', + document_expiration: '2099-12-31', + first_name: 'FAKEY', + last_name: 'MCFAKERSON', + date_of_birth: '1938-10-06', + address: '1 FAKE RD', + ssn: '900-66-1234', + ) + sign_in_and_2fa_user + complete_doc_auth_steps_before_verify_step + + allow(DocumentCaptureSession).to receive(:find_by). + and_return(nil) + + click_idv_continue + expect(page).to have_content(t('idv.failure.timeout')) + expect(page).to have_current_path(idv_doc_auth_verify_step) + allow(DocumentCaptureSession).to receive(:find_by).and_call_original + end end context 'async timed out' do From a7393b19707d8b188bba829e84a8e9a3cbef169c Mon Sep 17 00:00:00 2001 From: Alex Bradley Date: Thu, 5 Jan 2023 13:31:56 -0500 Subject: [PATCH 18/18] Removed ProofingComponent updates for TMX in rake (#7578) [skip changelog] Removed the updates to ProofingComponent in the rake task for activating or deactivating a user's profile who is under threatmetrix review. --- lib/tasks/review_profile.rake | 22 +++++++++++----------- spec/lib/tasks/review_profile_spec.rb | 6 ++---- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/lib/tasks/review_profile.rake b/lib/tasks/review_profile.rake index dfb301372a6..27d96d47365 100644 --- a/lib/tasks/review_profile.rake +++ b/lib/tasks/review_profile.rake @@ -8,9 +8,6 @@ namespace :users do user = User.find_by(uuid: user_uuid) if user.decorate.threatmetrix_review_pending? && user.proofing_component.review_eligible? - result = ProofingComponent.find_by(user: user) - result.update(threatmetrix_review_status: 'pass') - profile = user.profiles. where(deactivation_reason: 'threatmetrix_review_pending'). first @@ -26,8 +23,11 @@ namespace :users do disavowal_token: disavowal_token, ) - status = result.threatmetrix_review_status - puts "User's review state has been updated to #{status} and the user has been emailed." + if profile.active? + puts "User's profile has been activated and the user has been emailed." + else + puts "There was an error activating the user's profile please try again" + end elsif !user.proofing_component.review_eligible? puts 'User is past the 30 day review eligibility' else @@ -41,13 +41,13 @@ namespace :users do user = User.find_by(uuid: user_uuid) if user.decorate.threatmetrix_review_pending? && user.proofing_component.review_eligible? - result = ProofingComponent.find_by(user: user) - result.update(threatmetrix_review_status: 'reject') - user.profiles. + profile = user.profiles. where(deactivation_reason: 'threatmetrix_review_pending'). - first. - deactivate(:threatmetrix_review_rejected) - puts "User's review state has been updated to #{result.threatmetrix_review_status}." + first + + profile.deactivate(:threatmetrix_review_rejected) + + puts "User's profile has been deactivated with reason #{profile.deactivation_reason}" elsif !user.proofing_component.review_eligible? puts 'User is past the 30 day review eligibility' else diff --git a/spec/lib/tasks/review_profile_spec.rb b/spec/lib/tasks/review_profile_spec.rb index f1d3b3ad5ce..3c7a6296199 100644 --- a/spec/lib/tasks/review_profile_spec.rb +++ b/spec/lib/tasks/review_profile_spec.rb @@ -19,9 +19,8 @@ describe 'users:review:pass' do let(:task_name) { 'users:review:pass' } - it 'sets threatmetrix_review_status to pass' do + it 'activates the users profile' do invoke_task - expect(user.reload.proofing_component.threatmetrix_review_status).to eq('pass') expect(user.reload.profiles.first.active).to eq(true) end @@ -41,9 +40,8 @@ describe 'users:review:reject' do let(:task_name) { 'users:review:reject' } - it 'sets threatmetrix_review_status to reject' do + it 'deactivates the users profile with reason threatmetrix_review_rejected' do invoke_task - expect(user.reload.proofing_component.threatmetrix_review_status).to eq('reject') expect(user.reload.profiles.first.deactivation_reason).to eq('threatmetrix_review_rejected') end end