From d052f80d59c2a5ab6aa568cd47b7627595a92f01 Mon Sep 17 00:00:00 2001 From: "Gene M. Angelo, Jr" Date: Thu, 22 Sep 2022 03:39:25 -0400 Subject: [PATCH 01/23] Add Feature Flag to Conditionally Call VA Mock API (#7004) This change allows us to control whether or not we call the VA API mock proofer or a live VA API endpoint. changelog: Upcoming Features, Inherited Proofing, Add Feature Flag to Conditionally Call Mock API for VA User Data (LG-7623) --- config/application.yml.default | 3 +++ lib/identity_config.rb | 1 + 2 files changed, 4 insertions(+) diff --git a/config/application.yml.default b/config/application.yml.default index e75c698e042..9b1f7bdbaab 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -117,6 +117,7 @@ in_person_results_delay_in_hours: 1 include_slo_in_saml_metadata: false inherited_proofing_enabled: false inherited_proofing_va_base_url: 'https://staging-api.va.gov' +va_inherited_proofing_mock_enabled: true irs_attempt_api_audience: 'https://irs.gov' irs_attempt_api_auth_tokens: '' irs_attempt_api_csp_id: 'LOGIN.gov' @@ -334,6 +335,7 @@ development: identity_pki_local_dev: true in_person_proofing_enabled: true inherited_proofing_enabled: true + va_inherited_proofing_mock_enabled: true kantara_2fa_phone_restricted: true kantara_2fa_phone_existing_user_restriction: true kantara_restriction_enforcement_date: '2022-07-01' @@ -489,6 +491,7 @@ test: hmac_fingerprinter_key_queue: '["old-key-one", "old-key-two"]' identity_pki_disabled: true inherited_proofing_enabled: true + va_inherited_proofing_mock_enabled: false irs_attempt_api_auth_tokens: 'test-token-1,test-token-2' irs_attempt_api_public_key: MIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIICCgKCAgEAyut9Uio5XxsIUVrXARqCoHvcMVYT0p6WyU1BnbhxLRW4Q60p+4Bn32vVOt9nzeih7qvauYM5M0PZdKEmwOHflqPP+ABfKhL+6jxBhykN5P5UY375wTFBJZ20Fx8jOJbRhJD02oUQ49YKlDu3MG5Y0ApyD4ER4WKgxuB2OdyQKd9vg2ZZa+P2pw1HkFPEin0h8KBUFBeLGDZni8PIJdHBP6dA+xbayGBxSM/8xQC0JIg6KlGTcLql37QJIhP2oSv0nAJNb6idFPAz0uMCQDQWKKWV5FUDCsFVH7VuQz8xUCwnPn/SdaratB+29bwUpVhgHXrHdJ0i8vjBEX7smD7pI8CcFHuVgACt86NMlBnNCVkwumQgZNAAxe2mJoYcotEWOnhCuMc6MwSj985bj8XEdFlbf4ny9QO9rETd5aYcwXBiV/T6vd637uvHb0KenghNmlb1Tv9LMj2b9ZwNc9C6oeCnbN2YAfxSDrb8Ik+yq4hRewOvIK7f0CcpZYDXK25aHXnHm306Uu53KIwMGf1mha5T5LWTNaYy5XFoMWHJ9E+AnU/MUJSrwCAITH/S0JFcna5Oatn70aTE9pISATsqB5Iz1c46MvdrxD8hPoDjT7x6/EO316DZrxQfJhjbWsCB+R0QxYLkXPHczhB2Z0HPna9xB6RbJHzph7ifDizhZoMCAwEAAQ== irs_attempt_api_public_key_id: key1 diff --git a/lib/identity_config.rb b/lib/identity_config.rb index f3549883d93..81e47c3394d 100644 --- a/lib/identity_config.rb +++ b/lib/identity_config.rb @@ -196,6 +196,7 @@ def self.build_store(config_map) config.add(:include_slo_in_saml_metadata, type: :boolean) config.add(:inherited_proofing_enabled, type: :boolean) config.add(:inherited_proofing_va_base_url, type: :string) + config.add(:va_inherited_proofing_mock_enabled, type: :boolean) config.add(:irs_attempt_api_audience) config.add(:irs_attempt_api_auth_tokens, type: :comma_separated_string_list) config.add(:irs_attempt_api_csp_id) From c52b51bfacc7edee0070619d96068069d444f295 Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Thu, 22 Sep 2022 08:57:23 -0500 Subject: [PATCH 02/23] changelog: Internal, OpenID Connect Logout, Remove unused OpenID Connect Logout prevent_logout_redirect parameter (#7001) --- .../openid_connect/logout_controller.rb | 11 ++--- .../openid_connect/logout_controller_spec.rb | 26 +++++++++++ .../openid_connect/openid_connect_spec.rb | 43 ++----------------- 3 files changed, 33 insertions(+), 47 deletions(-) diff --git a/app/controllers/openid_connect/logout_controller.rb b/app/controllers/openid_connect/logout_controller.rb index c774e9785d1..830b9348260 100644 --- a/app/controllers/openid_connect/logout_controller.rb +++ b/app/controllers/openid_connect/logout_controller.rb @@ -19,12 +19,10 @@ def index if result.success? && (redirect_uri = result.extra[:redirect_uri]) sign_out - unless logout_params[:prevent_logout_redirect] == 'true' - redirect_to( - redirect_uri, - allow_other_host: true, - ) - end + redirect_to( + redirect_uri, + allow_other_host: true, + ) else render :error end @@ -35,7 +33,6 @@ def logout_params :id_token_hint, :post_logout_redirect_uri, :state, - :prevent_logout_redirect, ] if IdentityConfig.store.accept_client_id_in_oidc_logout || diff --git a/spec/controllers/openid_connect/logout_controller_spec.rb b/spec/controllers/openid_connect/logout_controller_spec.rb index 98a3c06f8bb..2978a80f9f8 100644 --- a/spec/controllers/openid_connect/logout_controller_spec.rb +++ b/spec/controllers/openid_connect/logout_controller_spec.rb @@ -60,6 +60,15 @@ expect(response).to redirect_to(/^#{post_logout_redirect_uri}/) end + it 'includes CSP headers' do + add_sp_session_request_url + action + + expect( + response.request.content_security_policy.directives['form-action'], + ).to eq(['\'self\'', 'gov.gsa.openidconnect.test:']) + end + it 'tracks events' do stub_analytics expect(@analytics).to receive(:track_event). @@ -580,4 +589,21 @@ end end end + + def add_sp_session_request_url + params = { + acr_values: Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF, + client_id: service_provider, + nonce: SecureRandom.hex, + redirect_uri: 'gov.gsa.openidconnect.test://result', + response_type: 'code', + scope: 'openid profile', + state: SecureRandom.hex, + } + session[:sp] = { + request_url: URI.parse( + "http://#{IdentityConfig.store.domain_name}?#{URI.encode_www_form(params)}", + ).to_s, + } + end end diff --git a/spec/features/openid_connect/openid_connect_spec.rb b/spec/features/openid_connect/openid_connect_spec.rb index 603f2a6d4bd..4c5a3dea282 100644 --- a/spec/features/openid_connect/openid_connect_spec.rb +++ b/spec/features/openid_connect/openid_connect_spec.rb @@ -143,7 +143,7 @@ end context 'when sending id_token_hint' do - it 'logout includes redirect_uris in CSP headers and destroys the session' do + it 'logout destroys the session' do id_token = sign_in_get_id_token state = SecureRandom.hex @@ -152,16 +152,6 @@ post_logout_redirect_uri: 'gov.gsa.openidconnect.test://result/signout', state: state, id_token_hint: id_token, - prevent_logout_redirect: true, - ) - - current_url_no_port = URI(current_url).tap { |uri| uri.port = nil }.to_s - expect(current_url_no_port).to include( - "http://www.example.com/openid_connect/logout?id_token_hint=#{id_token}", - ) - - expect(page.response_headers['Content-Security-Policy']).to include( - 'form-action \'self\' gov.gsa.openidconnect.test:', ) visit account_path @@ -177,7 +167,7 @@ end context 'when sending client_id' do - it 'logout includes redirect_uris in CSP headers and destroys the session' do + it 'logout destroys the session' do client_id = 'urn:gov:gsa:openidconnect:test' sign_in_get_id_token(client_id: client_id) @@ -187,16 +177,6 @@ client_id: client_id, post_logout_redirect_uri: 'gov.gsa.openidconnect.test://result/signout', state: state, - prevent_logout_redirect: true, - ) - - current_url_no_port = URI(current_url).tap { |uri| uri.port = nil }.to_s - expect(current_url_no_port).to include( - "http://www.example.com/openid_connect/logout?client_id=#{URI.encode_www_form_component(client_id)}", - ) - - expect(page.response_headers['Content-Security-Policy']).to include( - 'form-action \'self\' gov.gsa.openidconnect.test:', ) visit account_path @@ -223,7 +203,6 @@ client_id: client_id, post_logout_redirect_uri: 'gov.gsa.openidconnect.test://result/signout', state: state, - prevent_logout_redirect: true, ) current_url_no_port = URI(current_url).tap { |uri| uri.port = nil }.to_s @@ -242,7 +221,7 @@ and_return(true) end - it 'logout includes redirect_uris in CSP headers and destroys the session' do + it 'logout destroys the session' do client_id = 'urn:gov:gsa:openidconnect:test' sign_in_get_id_token(client_id: client_id) @@ -252,16 +231,6 @@ client_id: client_id, post_logout_redirect_uri: 'gov.gsa.openidconnect.test://result/signout', state: state, - prevent_logout_redirect: true, - ) - - current_url_no_port = URI(current_url).tap { |uri| uri.port = nil }.to_s - expect(current_url_no_port).to include( - "http://www.example.com/openid_connect/logout?client_id=#{URI.encode_www_form_component(client_id)}", - ) - - expect(page.response_headers['Content-Security-Policy']).to include( - 'form-action \'self\' gov.gsa.openidconnect.test:', ) visit account_path @@ -278,7 +247,6 @@ id_token_hint: id_token, post_logout_redirect_uri: 'gov.gsa.openidconnect.test://result/signout', state: state, - prevent_logout_redirect: true, ) current_url_no_port = URI(current_url).tap { |uri| uri.port = nil }.to_s @@ -693,11 +661,6 @@ id_token_hint: id_token, ) - current_url_no_port = URI(current_url).tap { |uri| uri.port = nil }.to_s - expect(current_url_no_port).to eq( - "gov.gsa.openidconnect.test://result/signout?state=#{state}", - ) - visit account_path expect(page).to_not have_content(t('headings.account.login_info')) expect(page).to have_content(t('headings.sign_in_without_sp')) From e46ffc649e489b2852148239d3a073bee82781eb Mon Sep 17 00:00:00 2001 From: olatifflexion <109746710+olatifflexion@users.noreply.github.com> Date: Thu, 22 Sep 2022 09:54:07 -0500 Subject: [PATCH 03/23] LG-7469 Standardize naming conventions (#7010) changelog: Internal, Attempts API, Standardize events name --- app/controllers/users/delete_controller.rb | 4 ++-- .../irs_attempts_api/tracker_events.rb | 18 +++++++++--------- .../users/delete_controller_spec.rb | 4 ++-- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/app/controllers/users/delete_controller.rb b/app/controllers/users/delete_controller.rb index 07a8a157e9f..c3c014c0db4 100644 --- a/app/controllers/users/delete_controller.rb +++ b/app/controllers/users/delete_controller.rb @@ -8,7 +8,7 @@ def show end def delete - irs_attempts_api_tracker.account_purged(success: true) + irs_attempts_api_tracker.logged_in_account_purged(success: true) send_push_notifications delete_user sign_out @@ -31,7 +31,7 @@ def confirm_current_password flash[:error] = t('idv.errors.incorrect_password') analytics.account_delete_submitted(success: false) - irs_attempts_api_tracker.account_purged(success: false) + irs_attempts_api_tracker.logged_in_account_purged(success: false) render :show end diff --git a/app/services/irs_attempts_api/tracker_events.rb b/app/services/irs_attempts_api/tracker_events.rb index e1d0ab6a908..5d2ec9c4237 100644 --- a/app/services/irs_attempts_api/tracker_events.rb +++ b/app/services/irs_attempts_api/tracker_events.rb @@ -3,15 +3,6 @@ # rubocop:disable Metrics/ModuleLength module IrsAttemptsApi module TrackerEvents - # @param [Boolean] success True if Account Successfully Deleted - # A User deletes their Login.gov account - def account_purged(success:) - track_event( - :account_purged, - success: success, - ) - end - # @param [Boolean] success True if Account Successfully Deleted # @param [Hash>] failure_reason displays why account deletion failed # A User confirms and deletes their Login.gov account after 24 hour period @@ -359,6 +350,15 @@ def idv_verification_submitted( ) end + # @param [Boolean] success True if Account Successfully Deleted + # A User deletes their Login.gov account + def logged_in_account_purged(success:) + track_event( + :logged_in_account_purged, + success: success, + ) + end + # @param [Boolean] success True if the password was successfully changed # @param [Hash>] failure_reason # A logged-in user has attempted to change their password diff --git a/spec/controllers/users/delete_controller_spec.rb b/spec/controllers/users/delete_controller_spec.rb index 7f3116829a5..2bd2c465769 100644 --- a/spec/controllers/users/delete_controller_spec.rb +++ b/spec/controllers/users/delete_controller_spec.rb @@ -43,7 +43,7 @@ expect(@analytics).to receive(:track_event). with('Account Delete submitted', success: false) expect(@irs_attempts_api_tracker).to receive(:track_event). - with(:account_purged, success: false) + with(:logged_in_account_purged, success: false) delete end @@ -70,7 +70,7 @@ expect(@analytics).to receive(:track_event). with('Account Delete submitted', success: true) expect(@irs_attempts_api_tracker).to receive(:track_event). - with(:account_purged, success: true) + with(:logged_in_account_purged, success: true) delete end From 6f861fe3ce2e3503d71734a8fb930cf1c831f7a3 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Thu, 22 Sep 2022 08:12:37 -0700 Subject: [PATCH 04/23] Speed up feature_management_spec.rb by removing use of browser (#7005) [skip changelog] * Add back in weight Co-authored-by: Andrew Duthie --- knapsack_rspec_report.json | 2 +- spec/lib/feature_management_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/knapsack_rspec_report.json b/knapsack_rspec_report.json index 97c09e94a54..6bbefd63972 100644 --- a/knapsack_rspec_report.json +++ b/knapsack_rspec_report.json @@ -27,7 +27,6 @@ "spec/presenters/setup_presenter_spec.rb": 0.05804999999963911, "spec/features/remember_device/session_expiration_spec.rb": 5.3716609999974025, "spec/requests/redirects_spec.rb": 0.12476699999388075, - "spec/lib/feature_management_spec.rb": 158.021550999998, "spec/forms/idv/doc_pii_form_spec.rb": 0.023514000000432134, "spec/controllers/users/personal_keys_controller_spec.rb": 0.22581699999864213, "spec/services/encryption/uak_password_verifier_spec.rb": 0.21799900000041816, @@ -45,6 +44,7 @@ "spec/forms/idv/ssn_form_spec.rb": 0.1248829999967711, "spec/components/vendor_outage_alert_component_spec.rb": 0.034957999996549916, "spec/views/idv/phone/new.html.erb_spec.rb": 0.06148799999937182, + "spec/lib/feature_management_spec.rb": 0.27332799999567214, "spec/lib/identity_job_log_subscriber_spec.rb": 0.16019000000233063, "spec/forms/gpo_verify_form_spec.rb": 0.25047999999515014, "spec/services/push_notification/email_changed_event_spec.rb": 0.022557999996934086, diff --git a/spec/lib/feature_management_spec.rb b/spec/lib/feature_management_spec.rb index 7a5db14502a..45fb182a27d 100644 --- a/spec/lib/feature_management_spec.rb +++ b/spec/lib/feature_management_spec.rb @@ -1,6 +1,6 @@ require 'rails_helper' -describe 'FeatureManagement', type: :feature do +describe 'FeatureManagement' do describe '#prefill_otp_codes?' do context 'when SMS sending is disabled' do before { allow(FeatureManagement).to receive(:telephony_test_adapter?).and_return(true) } From 90ac2c175dc27789ccccba9010a9b9c340853230 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 22 Sep 2022 12:06:12 -0400 Subject: [PATCH 05/23] Recompile Sass after exception when watching files (#7009) * Recompile Sass after exception when watching files **Why**: So that a developer has an opportunity to fix a compilation error, and so that they're not confused when their changes are not reflected in the running application process. changelog: Internal, Build Tooling, Recompile CSS after error in local development * Use "in" operator for Sass exception narrowing A bit clearer to interpret I think --- app/javascript/packages/build-sass/cli.js | 63 +++++++++++++++++++---- 1 file changed, 52 insertions(+), 11 deletions(-) diff --git a/app/javascript/packages/build-sass/cli.js b/app/javascript/packages/build-sass/cli.js index 6f347f127a9..08e649bfe20 100755 --- a/app/javascript/packages/build-sass/cli.js +++ b/app/javascript/packages/build-sass/cli.js @@ -7,13 +7,14 @@ import { fileURLToPath } from 'url'; import { buildFile } from './index.js'; /** @typedef {import('sass-embedded').Options<'sync'>} SyncSassOptions */ +/** @typedef {import('sass-embedded').Exception} SassException */ /** @typedef {import('./').BuildOptions} BuildOptions */ const env = process.env.NODE_ENV || process.env.RAILS_ENV || 'development'; const isProduction = env === 'production'; const args = process.argv.slice(2); -const files = args.filter((arg) => !arg.startsWith('-')); +const fileArgs = args.filter((arg) => !arg.startsWith('-')); const flags = args.filter((arg) => arg.startsWith('-')); const isWatching = flags.includes('--watch'); @@ -22,15 +23,55 @@ const outDir = flags.find((flag) => flag.startsWith('--out-dir='))?.slice(10); /** @type {BuildOptions & SyncSassOptions} */ const options = { outDir, optimize: isProduction }; -Promise.all( - files.map(async (file) => { - const { loadedUrls } = await buildFile(file, options); - if (isWatching) { - const loadedPaths = loadedUrls.map(fileURLToPath); - watch(loadedPaths).on('change', () => buildFile(file, options)); - } - }), -).catch((error) => { - console.error(error); +/** + * Watches given file path(s), triggering the callback on the first change. + * + * @param {string|string[]} paths Path(s) to watch. + * @param {() => void} callback Callback to invoke. + */ +function watchOnce(paths, callback) { + const watcher = watch(paths).once('change', () => { + watcher.close(); + callback(); + }); +} + +/** + * Returns true if the given error is a SassException, or false otherwise. + * + * @param {Error|SassException} error + * + * @return {error is SassException} + */ +const isSassException = (error) => 'span' in /** @type {SassException} */ (error); + +/** + * @param {string[]} files + * @return {Promise} + */ +function build(files) { + return Promise.all( + files.map(async (file) => { + const { loadedUrls } = await buildFile(file, options); + if (isWatching) { + const loadedPaths = loadedUrls.map(fileURLToPath); + watchOnce(loadedPaths, () => build([file])); + } + }), + ).catch( + /** @param {Error|SassException} error */ (error) => { + console.error(error); + + if (isWatching && isSassException(error) && error.span.url) { + const exceptionPath = fileURLToPath(error.span.url); + watchOnce(exceptionPath, () => build(files)); + } else { + throw error; + } + }, + ); +} + +build(fileArgs).catch(() => { process.exitCode = 1; }); From 063d350a42b763c909ae6cc4e3723bbf20e55c4d Mon Sep 17 00:00:00 2001 From: Tomas Apodaca Date: Thu, 22 Sep 2022 09:49:21 -0700 Subject: [PATCH 06/23] use deliver later for ipp results emails (#7007) [skip changelog] --- app/jobs/get_usps_proofing_results_job.rb | 8 ++++++-- spec/jobs/get_usps_proofing_results_job_spec.rb | 16 ++++++++-------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/app/jobs/get_usps_proofing_results_job.rb b/app/jobs/get_usps_proofing_results_job.rb index bea11b68d04..5695cc3fd37 100644 --- a/app/jobs/get_usps_proofing_results_job.rb +++ b/app/jobs/get_usps_proofing_results_job.rb @@ -230,21 +230,25 @@ def process_enrollment_response(enrollment, response) def send_verified_email(user, enrollment) user.confirmed_email_addresses.each do |email_address| + # rubocop:disable IdentityIdp/MailLaterLinter UserMailer.in_person_verified( user, email_address, enrollment: enrollment, - ).deliver_now_or_later(**mail_delivery_params) + ).deliver_later(**mail_delivery_params) + # rubocop:enable IdentityIdp/MailLaterLinter end end def send_failed_email(user, enrollment) user.confirmed_email_addresses.each do |email_address| + # rubocop:disable IdentityIdp/MailLaterLinter UserMailer.in_person_failed( user, email_address, enrollment: enrollment, - ).deliver_now_or_later(**mail_delivery_params) + ).deliver_later(**mail_delivery_params) + # rubocop:enable IdentityIdp/MailLaterLinter end end diff --git a/spec/jobs/get_usps_proofing_results_job_spec.rb b/spec/jobs/get_usps_proofing_results_job_spec.rb index 82a6390ffb2..6cf7bcd71af 100644 --- a/spec/jobs/get_usps_proofing_results_job_spec.rb +++ b/spec/jobs/get_usps_proofing_results_job_spec.rb @@ -239,11 +239,11 @@ it 'sends proofing failed email on response with failed status' do stub_request_failed_proofing_results - mailer = instance_double(ActionMailer::MessageDelivery, deliver_now_or_later: true) + mailer = instance_double(ActionMailer::MessageDelivery, deliver_later: true) user = pending_enrollment.user user.email_addresses.each do |email_address| # it sends with the default delay - expect(mailer).to receive(:deliver_now_or_later).with(wait: 1.hour) + expect(mailer).to receive(:deliver_later).with(wait: 1.hour) expect(UserMailer).to receive(:in_person_failed). with( user, @@ -259,11 +259,11 @@ it 'sends proofing verifed email on 2xx responses with valid JSON' do stub_request_passed_proofing_results - mailer = instance_double(ActionMailer::MessageDelivery, deliver_now_or_later: true) + mailer = instance_double(ActionMailer::MessageDelivery, deliver_later: true) user = pending_enrollment.user user.email_addresses.each do |email_address| # it sends with the default delay - expect(mailer).to receive(:deliver_now_or_later).with(wait: 1.hour) + expect(mailer).to receive(:deliver_later).with(wait: 1.hour) expect(UserMailer).to receive(:in_person_verified). with( user, @@ -283,10 +283,10 @@ allow(IdentityConfig.store). to(receive(:in_person_results_delay_in_hours).and_return(5)) - mailer = instance_double(ActionMailer::MessageDelivery, deliver_now_or_later: true) + mailer = instance_double(ActionMailer::MessageDelivery, deliver_later: true) user = pending_enrollment.user user.email_addresses.each do |email_address| - expect(mailer).to receive(:deliver_now_or_later).with(wait: 5.hours) + expect(mailer).to receive(:deliver_later).with(wait: 5.hours) expect(UserMailer).to receive(:in_person_verified).and_return(mailer) end @@ -301,10 +301,10 @@ allow(IdentityConfig.store). to(receive(:in_person_results_delay_in_hours).and_return(0)) - mailer = instance_double(ActionMailer::MessageDelivery, deliver_now_or_later: true) + mailer = instance_double(ActionMailer::MessageDelivery, deliver_later: true) user = pending_enrollment.user user.email_addresses.each do |email_address| - expect(mailer).to receive(:deliver_now_or_later).with(no_args) + expect(mailer).to receive(:deliver_later).with(no_args) expect(UserMailer).to receive(:in_person_verified).and_return(mailer) end From 6cb75899a45003ed31854a8aacf32f3ae35417e4 Mon Sep 17 00:00:00 2001 From: "Gene M. Angelo, Jr" Date: Thu, 22 Sep 2022 15:53:44 -0400 Subject: [PATCH 07/23] LG-7602 Pull VA IP Data and Store It In Session (#7008) https://cm-jira.usa.gov/browse/LG-7602 This is a supporting change for the LG-7602 feature for Inherited Proofing (IP) where user PII needs to be stored in session for IP flow steps and subsequent IDV workflow flow steps that need this information for (example) Lexis Phone Finder look ups. This change exposes a #user_pii method that retuns a Hash suitable to be stored in the idv_session and flow_session[:pii_from_user] (minus :uuid). The data mapped to the user PII returned from this method is Service Provider (SP)-specific in that the SP API returning the user PII may be returned having data keys that differ from what we need; consequently, create a mapper service for this, but that would disconnect what the SP service returns hash-wise. The Form knows about this, so it may be better suited "as is". changelog: Upcoming Features, Inherited Proofing, Pull VA IP Data and Store It In Session (LG-7602) --- app/forms/idv/inherited_proofing/base_form.rb | 139 +++++++++++ app/forms/idv/inherited_proofing/va/form.rb | 169 ++----------- .../idv/inherited_proofing/base_form_spec.rb | 231 ++++++++++++++++++ .../idv/inherited_proofing/va/form_spec.rb | 150 ++++++++---- 4 files changed, 489 insertions(+), 200 deletions(-) create mode 100644 app/forms/idv/inherited_proofing/base_form.rb create mode 100644 spec/forms/idv/inherited_proofing/base_form_spec.rb diff --git a/app/forms/idv/inherited_proofing/base_form.rb b/app/forms/idv/inherited_proofing/base_form.rb new file mode 100644 index 00000000000..681dd4b1e6f --- /dev/null +++ b/app/forms/idv/inherited_proofing/base_form.rb @@ -0,0 +1,139 @@ +module Idv + module InheritedProofing + class BaseForm + include ActiveModel::Model + + class << self + def model_name + ActiveModel::Name.new(self, nil, namespaced_model_name) + end + + def namespaced_model_name + self.to_s.gsub('::', '') + end + + def fields + @fields ||= required_fields + optional_fields + end + + def required_fields + raise NotImplementedError, + 'Override this method and return an Array of required field names as Symbols' + end + + def optional_fields + raise NotImplementedError, + 'Override this method and return an Array of optional field names as Symbols' + end + end + + private_class_method :namespaced_model_name, :required_fields, :optional_fields + + attr_reader :payload_hash + + def initialize(payload_hash:) + raise ArgumentError, 'payload_hash is blank?' if payload_hash.blank? + raise ArgumentError, 'payload_hash is not a Hash' unless payload_hash.is_a? Hash + + self.class.attr_accessor(*self.class.fields) + + @payload_hash = payload_hash.dup + + populate_field_data + end + + def submit + validate + + FormResponse.new( + success: valid?, + errors: errors, + extra: { + }, + ) + end + + # Perhaps overkill, but a mapper service of some kind, not bound to this class, + # that takes into consideration context, may be more suitable. In the meantime, + # simply return a Hash suitable to place into flow_session[:pii_from_user] in + # our inherited proofing flow steps. + def user_pii + raise NotImplementedError, 'Override this method and return a user PII Hash' + end + + private + + attr_writer :payload_hash + + # Populates our field data from the payload hash. + def populate_field_data + payload_field_info.each do |field_name, field_info| + # Ignore fields we're not interested in. + next unless respond_to? field_name + + value = payload_hash.dig( + *[field_info[:namespace], + field_info[:field_name]].flatten.compact, + ) + public_send("#{field_name}=", value) + end + end + + def payload_field_info + @payload_field_info ||= field_name_info_from payload_hash: payload_hash + end + + # This method simply navigates the payload hash received and creates qualified + # hash key names that can be used to verify/map to our field names in this model. + # This can be used to qualify nested hash fields and saves us some headaches + # if there are nested field names with the same name: + # + # given: + # + # payload_hash = { + # first_name: 'first_name', + # ... + # address: { + # street: '', + # ... + # } + # } + # + # field_name_info_from(payload_hash: payload_hash) #=> + # + # { + # :first_name=>{:field_name=>:first_name, :namespace=>[]}, + # ... + # :address_street=>{:field_name=>:street, :namespace=>[:address]}, + # ... + # } + # + # The generated, qualified field names expected to map to our model, because we named + # them as such. + # + # :field_name is the actual, unqualified field name found in the payload hash sent. + # :namespace is the hash key by which :field_name can be found in the payload hash + # if need be. + def field_name_info_from(payload_hash:, namespace: [], field_name_info: {}) + payload_hash.each do |key, value| + if value.is_a? Hash + field_name_info_from payload_hash: value, namespace: namespace << key, + field_name_info: field_name_info + namespace.pop + next + end + + namespace = namespace.dup + if namespace.blank? + field_name_info[key] = { field_name: key, namespace: namespace } + else + field_name_info["#{namespace.split.join('_')}_#{key}".to_sym] = + { field_name: key, namespace: namespace } + end + end + + field_name_info + end + end + end +end diff --git a/app/forms/idv/inherited_proofing/va/form.rb b/app/forms/idv/inherited_proofing/va/form.rb index 712d0b8c978..5f8a0874c47 100644 --- a/app/forms/idv/inherited_proofing/va/form.rb +++ b/app/forms/idv/inherited_proofing/va/form.rb @@ -1,163 +1,34 @@ module Idv module InheritedProofing module Va - class Form - include ActiveModel::Model - + class Form < Idv::InheritedProofing::BaseForm class << self - def model_name - ActiveModel::Name.new(self, nil, namespaced_model_name) - end - - def namespaced_model_name - self.to_s.gsub('::', '') - end - - # Returns the field names based on the validators we've set up. - def field_names - @field_names ||= fields.keys - end - - def fields - @fields ||= { - first_name: { required: true }, - last_name: { required: true }, - phone: { required: false }, - birth_date: { required: true }, - ssn: { required: true }, - address_street: { required: true }, - address_street2: { required: false }, - address_city: { required: false }, - address_state: { required: false }, - address_country: { required: false }, - address_zip: { required: true }, - } - end - def required_fields - @required_fields ||= fields.filter_map do |field_name, options| - field_name if options[:required] - end + @required_fields ||= %i[first_name last_name birth_date ssn address_street address_zip] end def optional_fields - @optional_fields ||= fields.filter_map do |field_name, options| - field_name unless options[:required] - end + @optional_fields ||= %i[phone address_street2 address_city address_state + address_country] end end - private_class_method :namespaced_model_name, :required_fields, :optional_fields - - attr_reader :payload_hash - - validate :validate_field_names - - required_fields.each { |required_field| validates(required_field, presence: true) } - - # This must be performed after our validators are defined. - attr_accessor(*self.field_names) - - def initialize(payload_hash:) - raise ArgumentError, 'payload_hash is blank?' if payload_hash.blank? - raise ArgumentError, 'payload_hash is not a Hash' unless payload_hash.is_a? Hash - - @payload_hash = payload_hash.dup - - populate_field_data - end - - def submit - validate - - FormResponse.new( - success: valid?, - errors: errors, - extra: { - }, - ) - end - - private - - attr_writer :payload_hash - - # Populates our field data from the payload hash. - def populate_field_data - payload_field_info.each do |field_name, field_info| - # Ignore fields we're not interested in. - next unless respond_to? field_name - - value = payload_hash.dig( - *[field_info[:namespace], - field_info[:field_name]].flatten.compact, - ) - public_send("#{field_name}=", value) - end - end - - # Validator for field names. All fields (not the presence of data) are required. - def validate_field_names - self.class.field_names.each do |field_name| - next if payload_field_info.key? field_name - errors.add(field_name, 'field is missing', type: :missing_required_field) - end - end - - def payload_field_info - @payload_field_info ||= field_name_info_from payload_hash: payload_hash - end - - # This method simply navigates the payload hash received and creates qualified - # hash key names that can be used to verify/map to our field names in this model. - # This can be used to qualify nested hash fields and saves us some headaches - # if there are nested field names with the same name: - # - # given: - # - # payload_hash = { - # first_name: 'first_name', - # ... - # address: { - # street: '', - # ... - # } - # } - # - # field_name_info_from(payload_hash: payload_hash) #=> - # - # { - # :first_name=>{:field_name=>:first_name, :namespace=>[]}, - # ... - # :address_street=>{:field_name=>:street, :namespace=>[:address]}, - # ... - # } - # - # The generated, qualified field names expected to map to our model, because we named - # them as such. - # - # :field_name is the actual, unqualified field name found in the payload hash sent. - # :namespace is the hash key by which :field_name can be found in the payload hash - # if need be. - def field_name_info_from(payload_hash:, namespace: [], field_name_info: {}) - payload_hash.each do |key, value| - if value.is_a? Hash - field_name_info_from payload_hash: value, namespace: namespace << key, - field_name_info: field_name_info - namespace.pop - next - end - - namespace = namespace.dup - if namespace.blank? - field_name_info[key] = { field_name: key, namespace: namespace } - else - field_name_info["#{namespace.split.join('_')}_#{key}".to_sym] = - { field_name: key, namespace: namespace } - end - end - - field_name_info + validates(*required_fields, presence: true) + + def user_pii + raise 'User PII is invalid' unless valid? + + user_pii = {} + user_pii[:first_name] = first_name + user_pii[:last_name] = last_name + user_pii[:dob] = birth_date + user_pii[:ssn] = ssn + user_pii[:phone] = phone + user_pii[:address1] = address_street + user_pii[:city] = address_city + user_pii[:state] = address_state + user_pii[:zipcode] = address_zip + user_pii end end end diff --git a/spec/forms/idv/inherited_proofing/base_form_spec.rb b/spec/forms/idv/inherited_proofing/base_form_spec.rb new file mode 100644 index 00000000000..80b640e1fd6 --- /dev/null +++ b/spec/forms/idv/inherited_proofing/base_form_spec.rb @@ -0,0 +1,231 @@ +require 'rails_helper' + +RSpec.shared_examples 'the hash is blank?' do + it 'raises an error' do + expect { subject }.to raise_error 'payload_hash is blank?' + end +end + +RSpec.describe Idv::InheritedProofing::BaseForm do + subject { form_object } + + let(:form_class) do + Class.new(Idv::InheritedProofing::BaseForm) do + class << self + def required_fields; [] end + + def optional_fields; [] end + end + + def user_pii; {} end + end + end + + let(:form_object) do + form_class.new(payload_hash: payload_hash) + end + + let(:payload_hash) do + { + first_name: 'Henry', + last_name: 'Ford', + phone: '12222222222', + birth_date: '2000-01-01', + ssn: '111223333', + address: { + street: '1234 Model Street', + street2: 'Suite A', + city: 'Detroit', + state: 'MI', + country: 'United States', + zip: '12345', + }, + } + end + + describe '#initialize' do + subject { form_class } + + context 'when .required_fields is not overridden' do + it 'raises an error' do + subject.singleton_class.send(:remove_method, :required_fields) + expected_error = 'Override this method and return ' \ + 'an Array of required field names as Symbols' + expect { subject.new(payload_hash: payload_hash) }.to raise_error(expected_error) + end + end + + context 'when .optional_fields is not overridden' do + it 'raises an error' do + subject.singleton_class.send(:remove_method, :optional_fields) + expected_error = 'Override this method and return ' \ + 'an Array of optional field names as Symbols' + expect { subject.new(payload_hash: payload_hash) }.to raise_error(expected_error) + end + end + + context 'when .user_pii is not overridden' do + subject do + Class.new(Idv::InheritedProofing::BaseForm) do + class << self + def required_fields; [] end + + def optional_fields; [] end + end + end + end + + it 'raises an error' do + expected_error = 'Override this method and return a user PII Hash' + expect { subject.new(payload_hash: payload_hash).user_pii }.to raise_error(expected_error) + end + end + end + + describe 'class methods' do + describe '.model_name' do + it 'returns the right model name' do + expect(described_class.model_name).to eq 'IdvInheritedProofingBaseForm' + end + end + + describe '.fields' do + subject do + Class.new(Idv::InheritedProofing::BaseForm) do + class << self + def required_fields; %i[required] end + + def optional_fields; %i[optional] end + end + + def user_pii; {} end + end + end + + let(:expected_field_names) do + [ + :required, + :optional, + ].sort + end + + it 'returns the right field names' do + expect(subject.fields).to match_array expected_field_names + end + end + end + + describe '#initialize' do + context 'when passing an invalid payload hash' do + context 'when not a Hash' do + let(:payload_hash) { :x } + + it 'raises an error' do + expect { subject }.to raise_error 'payload_hash is not a Hash' + end + end + + context 'when nil?' do + let(:payload_hash) { nil } + + it_behaves_like 'the hash is blank?' + end + + context 'when empty?' do + let(:payload_hash) { {} } + + it_behaves_like 'the hash is blank?' + end + end + + context 'when passing a valid payload hash' do + it 'raises no errors' do + expect { subject }.to_not raise_error + end + end + end + + describe '#validate' do + subject do + Class.new(Idv::InheritedProofing::BaseForm) do + class << self + def required_fields; %i[required] end + + def optional_fields; %i[optional] end + end + + def user_pii; {} end + end.new(payload_hash: payload_hash) + end + + let(:payload_hash) do + { + required: 'Required', + optional: 'Optional', + } + end + + context 'with valid payload data' do + it 'returns true' do + expect(subject.validate).to eq true + end + end + + context 'with invalid payload data' do + context 'when the payload has unrecognized fields' do + let(:payload_hash) do + { + xrequired: 'xRequired', + xoptional: 'xOptional', + } + end + + let(:expected_error_messages) do + [ + # Required field presence + 'Required field is missing', + 'Optional field is missing', + ] + end + + it 'returns true' do + expect(subject.validate).to eq true + end + end + + context 'when the payload has missing required field data' do + let(:payload_hash) do + { + required: nil, + optional: '', + } + end + + it 'returns true' do + expect(subject.validate).to eq true + end + + it 'returns no errors because no data validations take place by default' do + subject.validate + expect(subject.errors.full_messages).to eq [] + end + end + end + end + + describe '#submit' do + it 'returns a FormResponse object' do + expect(subject.submit).to be_kind_of FormResponse + end + + describe 'before returning' do + after do + subject.submit + end + + it 'calls #validate' do + expect(subject).to receive(:validate).once + end + end + end +end diff --git a/spec/forms/idv/inherited_proofing/va/form_spec.rb b/spec/forms/idv/inherited_proofing/va/form_spec.rb index ba74559849d..9a08fda4b8b 100644 --- a/spec/forms/idv/inherited_proofing/va/form_spec.rb +++ b/spec/forms/idv/inherited_proofing/va/form_spec.rb @@ -9,6 +9,9 @@ RSpec.describe Idv::InheritedProofing::Va::Form do subject(:form) { described_class.new payload_hash: payload_hash } + let(:required_fields) { %i[first_name last_name birth_date ssn address_street address_zip] } + let(:optional_fields) { %i[phone address_street2 address_city address_state address_country] } + let(:payload_hash) do { first_name: 'Henry', @@ -34,25 +37,21 @@ end end - describe '.field_names' do - let(:expected_field_names) do - [ - :address_city, - :address_country, - :address_state, - :address_street, - :address_street2, - :address_zip, - :birth_date, - :first_name, - :last_name, - :phone, - :ssn, - ].sort + describe '.fields' do + it 'returns all the fields' do + expect(described_class.fields).to match_array required_fields + optional_fields end + end - it 'returns the right model name' do - expect(described_class.field_names).to match_array expected_field_names + describe '.required_fields' do + it 'returns the required fields' do + expect(described_class.required_fields).to match_array required_fields + end + end + + describe '.optional_fields' do + it 'returns the optional fields' do + expect(described_class.optional_fields).to match_array optional_fields end end end @@ -116,18 +115,12 @@ let(:expected_error_messages) do [ - # Required field presence - 'First name field is missing', - 'Last name field is missing', - 'Phone field is missing', - 'Birth date field is missing', - 'Ssn field is missing', - 'Address street field is missing', - 'Address street2 field is missing', - 'Address city field is missing', - 'Address state field is missing', - 'Address country field is missing', - 'Address zip field is missing', + 'First name Please fill in this field.', + 'Last name Please fill in this field.', + 'Birth date Please fill in this field.', + 'Ssn Please fill in this field.', + 'Address street Please fill in this field.', + 'Address zip Please fill in this field.', ] end @@ -137,11 +130,7 @@ it 'adds the correct error messages for missing fields' do subject.validate - expect( - expected_error_messages.all? do |error_message| - subject.errors.full_messages.include? error_message - end, - ).to eq true + expect(subject.errors.full_messages).to match_array expected_error_messages end end @@ -185,32 +174,91 @@ expect(subject.errors.full_messages).to match_array expected_error_messages end end + + context 'when the payload has missing optional field data' do + let(:payload_hash) do + { + first_name: 'x', + last_name: 'x', + phone: nil, + birth_date: '01/01/2022', + ssn: '123456789', + address: { + street: 'x', + street2: nil, + city: '', + state: nil, + country: '', + zip: '12345', + }, + } + end + + it 'returns true' do + expect(subject.validate).to eq true + end + end end end describe '#submit' do - it 'returns a FormResponse object' do - expect(subject.submit).to be_kind_of FormResponse - end + context 'with an invalid payload' do + context 'when the payload has invalid field data' do + let(:payload_hash) do + { + first_name: nil, + last_name: '', + phone: nil, + birth_date: '', + ssn: nil, + address: { + street: '', + street2: nil, + city: '', + state: nil, + country: '', + zip: nil, + }, + } + end - describe 'before returning' do - after do - subject.submit - end + let(:expected_errors) do + { + # Required field data presence + first_name: ['Please fill in this field.'], + last_name: ['Please fill in this field.'], + birth_date: ['Please fill in this field.'], + ssn: ['Please fill in this field.'], + address_street: ['Please fill in this field.'], + address_zip: ['Please fill in this field.'], + } + end - it 'calls #validate' do - expect(subject).to receive(:validate).once + it 'returns a FormResponse indicating the correct errors and status' do + form_response = subject.submit + expect(form_response.success?).to eq false + expect(form_response.errors).to match_array expected_errors + end end end + end - context 'with an invalid payload' do - context 'when the payload has missing fields' do - it 'returns a FormResponse indicating errors' - end - - context 'when the payload has invalid field data' do - it 'returns a FormResponse indicating errors' - end + describe '#user_pii' do + let(:expected_user_pii) do + { + first_name: subject.first_name, + last_name: subject.last_name, + dob: subject.birth_date, + ssn: subject.ssn, + phone: subject.phone, + address1: subject.address_street, + city: subject.address_city, + state: subject.address_state, + zipcode: subject.address_zip, + } + end + it 'returns the correct user pii' do + expect(subject.user_pii).to eq expected_user_pii end end end From 6be4a5eacda129ac22b21852d9ef34cc088cd960 Mon Sep 17 00:00:00 2001 From: eileen-nava <80347702+eileen-nava@users.noreply.github.com> Date: Thu, 22 Sep 2022 15:55:09 -0400 Subject: [PATCH 08/23] LG-7275 Expand metrics for USPS proofing attempts (#6967) * expand metrics for USPS proofing attempts * LG-7275 Expand metrics for USPS proofing attempts changelog: Internal, In-person proofing, expand metrics for proofing attempts * enumerate params from analytics methods * refactor usps proofing tests to be DRYer * clean up usps proofing job and spec * tweak fixtures for expired id & unsupported status * clean up formatting and fixture --- app/jobs/get_usps_proofing_results_job.rb | 43 ++++++++---- app/services/analytics_events.rb | 51 ++++++++++++++ ...est_expired_proofing_results_response.json | 16 +---- .../get_usps_proofing_results_job_spec.rb | 69 ++++++++++++++----- spec/support/usps_ipp_helper.rb | 9 +-- 5 files changed, 139 insertions(+), 49 deletions(-) diff --git a/app/jobs/get_usps_proofing_results_job.rb b/app/jobs/get_usps_proofing_results_job.rb index 5695cc3fd37..6144e95d7d3 100644 --- a/app/jobs/get_usps_proofing_results_job.rb +++ b/app/jobs/get_usps_proofing_results_job.rb @@ -29,6 +29,24 @@ def enrollment_analytics_attributes(enrollment, complete:) } end + def response_analytics_attributes(response) + { + fraud_suspected: response['fraudSuspected'], + primary_id_type: response['primaryIdType'], + secondary_id_type: response['secondaryIdType'], + failure_reason: response['failureReason'], + transaction_end_date_time: response['transactionEndDateTime'], + transaction_start_date_time: response['transactionStartDateTime'], + status: response['status'], + assurance_level: response['assuranceLevel'], + proofing_post_office: response['proofingPostOffice'], + proofing_city: response['proofingCity'], + proofing_state: response['proofingState'], + scan_count: response['scanCount'], + response_message: response['responseMessage'], + } + end + def perform(_now) return true unless IdentityConfig.store.in_person_proofing_enabled @@ -113,10 +131,11 @@ def handle_bad_request_error(err, enrollment) # Customer has not been to post office for IPP enrollment_outcomes[:enrollments_in_progress] += 1 when IPP_EXPIRED_ERROR_MESSAGE - handle_expired_status_update(enrollment) + handle_expired_status_update(enrollment, err.response) else analytics(user: enrollment.user).idv_in_person_usps_proofing_results_job_exception( **enrollment_analytics_attributes(enrollment, complete: false), + **response_analytics_attributes(err.response), reason: 'Request exception', exception_class: err.class.to_s, exception_message: err.message, @@ -129,6 +148,7 @@ def handle_standard_error(err, enrollment) enrollment_outcomes[:enrollments_errored] += 1 analytics(user: enrollment.user).idv_in_person_usps_proofing_results_job_exception( **enrollment_analytics_attributes(enrollment, complete: false), + **response_analytics_attributes(err.response), reason: 'Request exception', exception_class: err.class.to_s, exception_message: err.message, @@ -143,12 +163,13 @@ def handle_response_is_not_a_hash(enrollment) ) end - def handle_unsupported_status(enrollment, status) + def handle_unsupported_status(enrollment, response) enrollment_outcomes[:enrollments_errored] += 1 analytics(user: enrollment.user).idv_in_person_usps_proofing_results_job_exception( **enrollment_analytics_attributes(enrollment, complete: false), + **response_analytics_attributes(response), reason: 'Unsupported status', - status: status, + status: response['status'], ) end @@ -156,6 +177,7 @@ def handle_unsupported_id_type(enrollment, response) enrollment_outcomes[:enrollments_failed] += 1 analytics(user: enrollment.user).idv_in_person_usps_proofing_results_job_enrollment_updated( **enrollment_analytics_attributes(enrollment, complete: true), + **response_analytics_attributes(response), fraud_suspected: response['fraudSuspected'], passed: false, primary_id_type: response['primaryIdType'], @@ -164,11 +186,11 @@ def handle_unsupported_id_type(enrollment, response) enrollment.update(status: :failed) end - def handle_expired_status_update(enrollment) + def handle_expired_status_update(enrollment, response) enrollment_outcomes[:enrollments_expired] += 1 analytics(user: enrollment.user).idv_in_person_usps_proofing_results_job_enrollment_updated( **enrollment_analytics_attributes(enrollment, complete: true), - fraud_suspected: nil, + **response_analytics_attributes(response[:body]), passed: false, reason: 'Enrollment has expired', ) @@ -179,15 +201,9 @@ def handle_failed_status(enrollment, response) enrollment_outcomes[:enrollments_failed] += 1 analytics(user: enrollment.user).idv_in_person_usps_proofing_results_job_enrollment_updated( **enrollment_analytics_attributes(enrollment, complete: true), - failure_reason: response['failureReason'], - fraud_suspected: response['fraudSuspected'], + **response_analytics_attributes(response), passed: false, - primary_id_type: response['primaryIdType'], - proofing_state: response['proofingState'], reason: 'Failed status', - secondary_id_type: response['secondaryIdType'], - transaction_end_date_time: response['transactionEndDateTime'], - transaction_start_date_time: response['transactionStartDateTime'], ) enrollment.update(status: :failed) @@ -198,6 +214,7 @@ def handle_successful_status_update(enrollment, response) enrollment_outcomes[:enrollments_passed] += 1 analytics(user: enrollment.user).idv_in_person_usps_proofing_results_job_enrollment_updated( **enrollment_analytics_attributes(enrollment, complete: true), + **response_analytics_attributes(**response), fraud_suspected: response['fraudSuspected'], passed: true, reason: 'Successful status update', @@ -224,7 +241,7 @@ def process_enrollment_response(enrollment, response) when IPP_STATUS_FAILED handle_failed_status(enrollment, response) else - handle_unsupported_status(enrollment, response['status']) + handle_unsupported_status(enrollment, response) end end diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index 96e2a5fe674..62627c65b99 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -2547,11 +2547,45 @@ def idv_in_person_usps_proofing_results_job_completed( # @param [String] enrollment_id # @param [String] exception_class # @param [String] exception_message + # @param [String] enrollment_code + # @param [Float] minutes_since_last_status_check + # @param [Float] minutes_since_last_status_update + # @param [Float] minutes_to_completion + # @param [Boolean] fraud_suspected + # @param [String] primary_id_type + # @param [String] secondary_id_type + # @param [String] failure_reason + # @param [String] transaction_end_date_time + # @param [String] transaction_start_date_time + # @param [String] status + # @param [String] assurance_level + # @param [String] proofing_post_office + # @param [String] proofing_city + # @param [String] proofing_state + # @param [String] scan_count + # @param [String] response_message def idv_in_person_usps_proofing_results_job_exception( reason:, enrollment_id:, exception_class: nil, exception_message: nil, + enrollment_code: nil, + minutes_since_last_status_check: nil, + minutes_since_last_status_update: nil, + minutes_to_completion: nil, + fraud_suspected: nil, + primary_id_type: nil, + secondary_id_type: nil, + failure_reason: nil, + transaction_end_date_time: nil, + transaction_start_date_time: nil, + status: nil, + assurance_level: nil, + proofing_post_office: nil, + proofing_city: nil, + proofing_state: nil, + scan_count: nil, + response_message: nil, **extra ) track_event( @@ -2560,6 +2594,23 @@ def idv_in_person_usps_proofing_results_job_exception( enrollment_id: enrollment_id, exception_class: exception_class, exception_message: exception_message, + enrollment_code: enrollment_code, + minutes_since_last_status_check: minutes_since_last_status_check, + minutes_since_last_status_update: minutes_since_last_status_update, + minutes_to_completion: minutes_to_completion, + fraud_suspected: fraud_suspected, + primary_id_type: primary_id_type, + secondary_id_type: secondary_id_type, + failure_reason: failure_reason, + transaction_end_date_time: transaction_end_date_time, + transaction_start_date_time: transaction_start_date_time, + status: status, + assurance_level: assurance_level, + proofing_post_office: proofing_post_office, + proofing_city: proofing_city, + proofing_state: proofing_state, + scan_count: scan_count, + response_message: response_message, **extra, ) end diff --git a/app/services/usps_in_person_proofing/mock/responses/request_expired_proofing_results_response.json b/app/services/usps_in_person_proofing/mock/responses/request_expired_proofing_results_response.json index c2622f1df7e..7f01924ad96 100644 --- a/app/services/usps_in_person_proofing/mock/responses/request_expired_proofing_results_response.json +++ b/app/services/usps_in_person_proofing/mock/responses/request_expired_proofing_results_response.json @@ -1,15 +1,3 @@ { - "status": "In-person expired", - "proofingPostOffice": "WILKES BARRE", - "proofingCity": "WILKES BARRE", - "proofingState": "PA", - "enrollmentCode": "2090002197604352", - "primaryIdType": "Uniformed Services identification card", - "transactionStartDateTime": "12/17/2020 033855", - "transactionEndDateTime": "12/17/2020 034055", - "secondaryIdType": "Deed of Trust", - "responseMessage": "More than 30 days have passed since opt-in to IPP", - "fraudSuspected": false, - "proofingConfirmationNumber": "350040248346707", - "ippAssuranceLevel": "1.5" -} \ No newline at end of file + "responseMessage": "More than 30 days have passed since opt-in to IPP" +} diff --git a/spec/jobs/get_usps_proofing_results_job_spec.rb b/spec/jobs/get_usps_proofing_results_job_spec.rb index 6cf7bcd71af..f9ae51e14eb 100644 --- a/spec/jobs/get_usps_proofing_results_job_spec.rb +++ b/spec/jobs/get_usps_proofing_results_job_spec.rb @@ -1,6 +1,6 @@ require 'rails_helper' -RSpec.shared_examples 'enrollment with a status update' do |passed:, status:| +RSpec.shared_examples 'enrollment with a status update' do |passed:, status:, response_json:| it 'logs a message with common attributes' do freeze_time do pending_enrollment.update( @@ -12,14 +12,28 @@ job.perform(Time.zone.now) end + response = JSON.parse(response_json) expect(job_analytics).to have_logged_event( 'GetUspsProofingResultsJob: Enrollment status updated', + assurance_level: response['assuranceLevel'], enrollment_code: pending_enrollment.enrollment_code, enrollment_id: pending_enrollment.id, + failure_reason: response['failureReason'], + fraud_suspected: response['fraudSuspected'], minutes_since_last_status_check: 15.0, minutes_since_last_status_update: 2.days.in_minutes, minutes_to_completion: 3.days.in_minutes, passed: passed, + primary_id_type: response['primaryIdType'], + proofing_city: response['proofingCity'], + proofing_post_office: response['proofingPostOffice'], + proofing_state: response['proofingState'], + response_message: response['responseMessage'], + scan_count: response['scanCount'], + secondary_id_type: response['secondaryIdType'], + status: response['status'], + transaction_end_date_time: response['transactionEndDateTime'], + transaction_start_date_time: response['transactionStartDateTime'], ) end @@ -318,14 +332,19 @@ stub_request_passed_proofing_results end - it_behaves_like('enrollment with a status update', passed: true, status: 'passed') + it_behaves_like( + 'enrollment with a status update', + passed: true, + status: 'passed', + response_json: UspsInPersonProofing::Mock::Fixtures. + request_passed_proofing_results_response, + ) it 'logs details about the success' do job.perform(Time.zone.now) expect(job_analytics).to have_logged_event( 'GetUspsProofingResultsJob: Enrollment status updated', - fraud_suspected: false, reason: 'Successful status update', ) end @@ -336,21 +355,19 @@ stub_request_failed_proofing_results end - it_behaves_like('enrollment with a status update', passed: false, status: 'failed') + it_behaves_like( + 'enrollment with a status update', + passed: false, + status: 'failed', + response_json: UspsInPersonProofing::Mock::Fixtures. + request_failed_proofing_results_response, + ) it 'logs failure details' do job.perform(Time.zone.now) expect(job_analytics).to have_logged_event( 'GetUspsProofingResultsJob: Enrollment status updated', - failure_reason: 'Clerk indicates that ID name or address does not match source data.', - fraud_suspected: false, - primary_id_type: 'Uniformed Services identification card', - proofing_state: 'PA', - reason: 'Failed status', - secondary_id_type: 'Deed of Trust', - transaction_end_date_time: '12/17/2020 034055', - transaction_start_date_time: '12/17/2020 033855', ) end end @@ -360,15 +377,19 @@ stub_request_passed_proofing_unsupported_id_results end - it_behaves_like('enrollment with a status update', passed: false, status: 'failed') + it_behaves_like( + 'enrollment with a status update', + passed: false, + status: 'failed', + response_json: UspsInPersonProofing::Mock::Fixtures. + request_passed_proofing_unsupported_id_results_response, + ) it 'logs a message about the unsupported ID' do job.perform Time.zone.now expect(job_analytics).to have_logged_event( 'GetUspsProofingResultsJob: Enrollment status updated', - fraud_suspected: false, - primary_id_type: 'Not supported', reason: 'Unsupported ID type', ) end @@ -379,7 +400,13 @@ stub_request_expired_proofing_results end - it_behaves_like('enrollment with a status update', passed: false, status: 'expired') + it_behaves_like( + 'enrollment with a status update', + passed: false, + status: 'expired', + response_json: UspsInPersonProofing::Mock::Fixtures. + request_expired_proofing_results_response, + ) it 'logs that the enrollment expired' do job.perform(Time.zone.now) @@ -396,7 +423,10 @@ stub_request_proofing_results_with_responses({}) end - it_behaves_like('enrollment encountering an exception', reason: 'Bad response structure') + it_behaves_like( + 'enrollment encountering an exception', + reason: 'Bad response structure', + ) end context 'when USPS returns an unexpected status' do @@ -404,7 +434,10 @@ stub_request_passed_proofing_unsupported_status_results end - it_behaves_like('enrollment encountering an exception', reason: 'Unsupported status') + it_behaves_like( + 'enrollment encountering an exception', + reason: 'Unsupported status', + ) it 'logs the status received' do job.perform(Time.zone.now) diff --git a/spec/support/usps_ipp_helper.rb b/spec/support/usps_ipp_helper.rb index 2a46b8a17d5..6dd73f8bdf1 100644 --- a/spec/support/usps_ipp_helper.rb +++ b/spec/support/usps_ipp_helper.rb @@ -62,16 +62,17 @@ def request_failed_proofing_results_args def stub_request_passed_proofing_unsupported_id_results stub_request(:post, %r{/ivs-ippaas-api/IPPRest/resources/rest/getProofingResults}).to_return( - status: 200, body: UspsInPersonProofing::Mock::Fixtures. - request_passed_proofing_unsupported_id_results_response + status: 200, + body: UspsInPersonProofing::Mock:: + Fixtures.request_passed_proofing_unsupported_id_results_response, ) end def stub_request_passed_proofing_unsupported_status_results stub_request(:post, %r{/ivs-ippaas-api/IPPRest/resources/rest/getProofingResults}).to_return( status: 200, - body: UspsInPersonProofing::Mock::Fixtures. - request_passed_proofing_unsupported_status_results_response, + body: UspsInPersonProofing::Mock:: + Fixtures.request_passed_proofing_unsupported_status_results_response, ) end From 0fa751631351dbe1d2ed3a537521550a6b5e3b7a Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Thu, 22 Sep 2022 18:04:50 -0500 Subject: [PATCH 09/23] changelog: Bug Fixes, Email Confirmation, Fix link in email when confirming new email address for existing account (#7019) --- app/views/user_mailer/add_email.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/user_mailer/add_email.html.erb b/app/views/user_mailer/add_email.html.erb index 2be3c84a3fb..6f125d4e35a 100644 --- a/app/views/user_mailer/add_email.html.erb +++ b/app/views/user_mailer/add_email.html.erb @@ -36,7 +36,7 @@

<%= link_to add_email_confirmation_url(_request_id: @request_id, confirmation_token: @token, locale: @locale), - sign_up_create_email_confirmation_url( + add_email_confirmation_url( _request_id: @request_id, confirmation_token: @token, locale: @locale, From d71f47b961922179c23e54e2cbee772a912dc774 Mon Sep 17 00:00:00 2001 From: "Gene M. Angelo, Jr" Date: Fri, 23 Sep 2022 08:57:33 -0400 Subject: [PATCH 10/23] LG-7602 Pull VA IP Data and Store It In Session (InheritedProofingConcern refactor) 2 of 3 (#7012) * https://cm-jira.usa.gov/browse/LG-7602 This is a supporting change for the LG-7602 feature for Inherited Proofing (IP). The user's PII needs to be obtained from the Service Provider in our Inherited Proofing process Flow Steps. Identifying whether or not an Inherited Proofing SP request is in progress, needs access to the decorated_session, which currently takes place in the InheritedProofingConcern. It makes sense (for how), to serve up the Service Provider Services and Forms in this concern, since we can easily identify the Service Provider here also; we can think about placing this code in a SP Service and Form factory or something similar later. changelog: Upcoming Features, Inherited Proofing, Pull VA IP Data and Store It In Session (LG-7602) * Update our scripts to recognize new param to server --- .../concerns/inherited_proofing_concern.rb | 34 ++++ .../inherited_proofing/va/mocks/service.rb | 4 +- .../idv/inherited_proofing/va/service.rb | 4 +- .../va/user_attributes/test_server.rb | 2 +- .../inherited_proofing_concern_spec.rb | 151 ++++++++++++++++++ .../va/mocks/service_spec.rb | 4 +- .../va/mocks/service_spec.rb | 2 +- .../idv/inherited_proofing/va/service_spec.rb | 2 +- 8 files changed, 195 insertions(+), 8 deletions(-) create mode 100644 spec/controllers/concerns/inherited_proofing_concern_spec.rb diff --git a/app/controllers/concerns/inherited_proofing_concern.rb b/app/controllers/concerns/inherited_proofing_concern.rb index 57b644e7ece..93a0add6f74 100644 --- a/app/controllers/concerns/inherited_proofing_concern.rb +++ b/app/controllers/concerns/inherited_proofing_concern.rb @@ -27,4 +27,38 @@ def va_inherited_proofing_auth_code def va_inherited_proofing_auth_code_params_key 'inherited_proofing_auth' end + + # Service Provider-agnostic members for now. + # Think about putting this in a factory(ies). + + def inherited_proofing_service + inherited_proofing_service_class.new inherited_proofing_service_provider_data + end + + def inherited_proofing_service_class + raise 'Inherited Proofing is not enabled' unless IdentityConfig.store.inherited_proofing_enabled + + if va_inherited_proofing? + if IdentityConfig.store.va_inherited_proofing_mock_enabled + return Idv::InheritedProofing::Va::Mocks::Service + end + return Idv::InheritedProofing::Va::Service + end + + raise 'Inherited proofing service class could not be identified' + end + + def inherited_proofing_form(payload_hash) + return Idv::InheritedProofing::Va::Form.new payload_hash: payload_hash if va_inherited_proofing? + + raise 'Inherited proofing form could not be identified' + end + + def inherited_proofing_service_provider_data + if va_inherited_proofing? + { auth_code: va_inherited_proofing_auth_code } + else + {} + end + end end diff --git a/app/services/idv/inherited_proofing/va/mocks/service.rb b/app/services/idv/inherited_proofing/va/mocks/service.rb index 0358b2c961f..0a580d60b2d 100644 --- a/app/services/idv/inherited_proofing/va/mocks/service.rb +++ b/app/services/idv/inherited_proofing/va/mocks/service.rb @@ -35,8 +35,8 @@ class Service }, }.freeze - def initialize(auth_code) - @auth_code = auth_code + def initialize(service_provider_data) + @auth_code = service_provider_data[:auth_code] end def execute diff --git a/app/services/idv/inherited_proofing/va/service.rb b/app/services/idv/inherited_proofing/va/service.rb index 094d9747ca2..fc7e70cd91e 100644 --- a/app/services/idv/inherited_proofing/va/service.rb +++ b/app/services/idv/inherited_proofing/va/service.rb @@ -9,8 +9,8 @@ class Service attr_reader :auth_code - def initialize(auth_code) - @auth_code = auth_code + def initialize(service_provider_data) + @auth_code = service_provider_data[:auth_code] end # Calls the endpoint and returns the decrypted response. diff --git a/scripts/inherited_proofing/va/user_attributes/test_server.rb b/scripts/inherited_proofing/va/user_attributes/test_server.rb index 894f68f5593..ce5a65b2e9b 100644 --- a/scripts/inherited_proofing/va/user_attributes/test_server.rb +++ b/scripts/inherited_proofing/va/user_attributes/test_server.rb @@ -10,7 +10,7 @@ class TestServer < Idv::InheritedProofing::Va::Service attr_reader :base_uri def initialize(auth_code:, private_key_file: nil, base_uri: nil) - super auth_code + super({ auth_code: auth_code }) if private_key_file.present? @private_key_file = force_tmp_private_key_file_name(private_key_file) diff --git a/spec/controllers/concerns/inherited_proofing_concern_spec.rb b/spec/controllers/concerns/inherited_proofing_concern_spec.rb new file mode 100644 index 00000000000..208d1159f80 --- /dev/null +++ b/spec/controllers/concerns/inherited_proofing_concern_spec.rb @@ -0,0 +1,151 @@ +require 'rails_helper' + +RSpec.describe InheritedProofingConcern do + subject do + Class.new do + include InheritedProofingConcern + end.new + end + + before do + allow(IdentityConfig.store).to receive(:inherited_proofing_enabled).and_return(true) + allow(subject).to receive(:va_inherited_proofing_auth_code).and_return auth_code + end + + let(:auth_code) { Idv::InheritedProofing::Va::Mocks::Service::VALID_AUTH_CODE } + let(:payload_hash) { Idv::InheritedProofing::Va::Mocks::Service::PAYLOAD_HASH } + + describe '#va_inherited_proofing?' do + context 'when the va auth code is present' do + it 'returns true' do + expect(subject.va_inherited_proofing?).to eq true + end + end + + context 'when the va auth code is not present' do + let(:auth_code) { nil } + + it 'returns false' do + expect(subject.va_inherited_proofing?).to eq false + end + end + end + + describe '#va_inherited_proofing_auth_code_params_key' do + it 'returns the correct va auth code url query param key' do + expect(subject.va_inherited_proofing_auth_code_params_key).to eq 'inherited_proofing_auth' + end + end + + describe '#inherited_proofing_service_class' do + context 'when va inherited proofing is disabled' do + before do + allow(IdentityConfig.store).to receive(:inherited_proofing_enabled).and_return(false) + end + + it 'raises an error' do + expect do + subject.inherited_proofing_service_class + end.to raise_error 'Inherited Proofing is not enabled' + end + end + + context 'when there is a va inherited proofing request' do + context 'when va mock proofing is turned on' do + before do + allow(IdentityConfig.store).to \ + receive(:va_inherited_proofing_mock_enabled).and_return(true) + end + + it 'returns the correct service provider service class' do + expect(subject.inherited_proofing_service_class).to \ + eq Idv::InheritedProofing::Va::Mocks::Service + end + end + + context 'when va mock proofing is turned off' do + before do + allow(IdentityConfig.store).to \ + receive(:va_inherited_proofing_mock_enabled).and_return(false) + end + + it 'returns the correct service provider service class' do + expect(subject.inherited_proofing_service_class).to eq Idv::InheritedProofing::Va::Service + end + end + end + + context 'when the inherited proofing request cannot be identified' do + let(:auth_code) { nil } + + it 'raises an error' do + expect do + subject.inherited_proofing_service_class + end.to raise_error 'Inherited proofing service class could not be identified' + end + end + end + + describe '#inherited_proofing_service' do + context 'when there is a va inherited proofing request' do + context 'when va mock proofing is turned on' do + before do + allow(IdentityConfig.store).to \ + receive(:va_inherited_proofing_mock_enabled).and_return(true) + end + + it 'returns the correct service provider service class' do + expect(subject.inherited_proofing_service).to \ + be_kind_of Idv::InheritedProofing::Va::Mocks::Service + end + end + + context 'when va mock proofing is turned off' do + before do + allow(IdentityConfig.store).to \ + receive(:va_inherited_proofing_mock_enabled).and_return(false) + end + + it 'returns the correct service provider service class' do + expect(subject.inherited_proofing_service).to \ + be_kind_of Idv::InheritedProofing::Va::Service + end + end + end + end + + describe '#inherited_proofing_form' do + context 'when there is a va inherited proofing request' do + it 'returns the correct form' do + expect(subject.inherited_proofing_form(payload_hash)).to \ + be_kind_of Idv::InheritedProofing::Va::Form + end + end + + context 'when the inherited proofing request cannot be identified' do + let(:auth_code) { nil } + + it 'raises an error' do + expect { subject.inherited_proofing_form(payload_hash) }.to \ + raise_error 'Inherited proofing form could not be identified' + end + end + end + + describe '#inherited_proofing_service_provider_data' do + context 'when there is a va inherited proofing request' do + it 'returns the correct service provider-specific data' do + expect(subject.inherited_proofing_service_provider_data).to \ + eq({ auth_code: auth_code }) + end + end + + context 'when the inherited proofing request cannot be identified' do + let(:auth_code) { nil } + + it 'returns an empty hash' do + expect(subject.inherited_proofing_service_provider_data).to eq({}) + end + end + end +end diff --git a/spec/features/services/idv/inherited_proofing/va/mocks/service_spec.rb b/spec/features/services/idv/inherited_proofing/va/mocks/service_spec.rb index 7e44e984fdb..88d8c7d429f 100644 --- a/spec/features/services/idv/inherited_proofing/va/mocks/service_spec.rb +++ b/spec/features/services/idv/inherited_proofing/va/mocks/service_spec.rb @@ -3,7 +3,9 @@ RSpec.describe 'Inherited Proofing VA API Proofer Service' do subject(:form) { Idv::InheritedProofing::Va::Form.new(payload_hash: proofer_results) } - let(:proofer_results) { Idv::InheritedProofing::Va::Mocks::Service.new(auth_code).execute } + let(:proofer_results) do + Idv::InheritedProofing::Va::Mocks::Service.new({ auth_code: auth_code }).execute + end let(:auth_code) { Idv::InheritedProofing::Va::Mocks::Service::VALID_AUTH_CODE } context 'when used with the VA Inherited Proofing Response Form' do diff --git a/spec/services/idv/inherited_proofing/va/mocks/service_spec.rb b/spec/services/idv/inherited_proofing/va/mocks/service_spec.rb index 83a88234723..8d0fcf5fc05 100644 --- a/spec/services/idv/inherited_proofing/va/mocks/service_spec.rb +++ b/spec/services/idv/inherited_proofing/va/mocks/service_spec.rb @@ -1,7 +1,7 @@ require 'rails_helper' RSpec.describe Idv::InheritedProofing::Va::Mocks::Service do - subject { described_class.new(auth_code) } + subject { described_class.new({ auth_code: auth_code }) } let(:auth_code) { described_class::VALID_AUTH_CODE } describe '#initialize' do diff --git a/spec/services/idv/inherited_proofing/va/service_spec.rb b/spec/services/idv/inherited_proofing/va/service_spec.rb index b53560ccfdf..81695a06e1e 100644 --- a/spec/services/idv/inherited_proofing/va/service_spec.rb +++ b/spec/services/idv/inherited_proofing/va/service_spec.rb @@ -10,7 +10,7 @@ include_context 'va_api_context' include_context 'va_user_context' - subject(:service) { described_class.new auth_code } + subject(:service) { described_class.new(auth_code: auth_code) } before do allow(service).to receive(:private_key).and_return(private_key) From 5d9ec6a7c426409ea57687fc4c47d57b1b45eea6 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 23 Sep 2022 11:49:08 -0400 Subject: [PATCH 11/23] Include U.S. territories in allowed countries for IdV phone (#7016) * Include U.S. territories in allowed countries for IdV phone changelog: Bug Fixes, Identity Verification, Allow U.S. territory numbers for IdV phone step * Use form instance for allowed countries * Only validate unsupported delivery methods if all unsupported * limit delivery options to phone number capabilities * Add regression coverage for "valid_phone_for_allowed_countries?" fix * Add test coverage for updated all-method-unsupported logic * Add test coverage for conditional field visibility * Check confirmed phones for PhoneNumberCapabilities initializer * Remove unnecessary memoization Conflicting variable name in "#new", not necessary anyways since we use it once in assignment, and helps collapse lines * Update spec expected error message for unsupported country * Fix OTP delivery method re-render on invalid create * Validate region regardless of number of supported countries We now support multiple. "valid country" should apply based on the selected country code regardless of how many countries are supported * Update server-side logic for U.S. number constraint error message * Fix empty errors value in spec assertion * Add phone analytics to phone finder * rename address proofing country codes constant and make it configurable * fix specs * JavaScript this.isConnected fix Co-authored-by: Mitchell Henke --- app/components/phone_input_component.rb | 1 - .../idv/otp_delivery_method_controller.rb | 21 ++++++- app/controllers/idv/phone_controller.rb | 3 +- app/forms/idv/phone_form.rb | 13 ++-- app/javascript/packages/phone-input/index.js | 60 +++++++++---------- .../packages/phone-input/index.spec.js | 19 +----- app/services/idv/phone_step.rb | 5 ++ app/services/phone_number_capabilities.rb | 2 + .../idv/otp_delivery_method/new.html.erb | 56 +++++++++-------- app/views/idv/phone/new.html.erb | 2 +- config/application.yml.default | 1 + config/locales/errors/en.yml | 3 - config/locales/errors/es.yml | 3 - config/locales/errors/fr.yml | 3 - lib/identity_config.rb | 1 + spec/controllers/idv/phone_controller_spec.rb | 18 ++++-- .../phone_otp_delivery_selection_step_spec.rb | 2 +- spec/forms/idv/phone_form_spec.rb | 43 ++++++++++++- spec/services/idv/phone_step_spec.rb | 8 +++ .../otp_delivery_method/new.html.erb_spec.rb | 32 ++++++++++ 20 files changed, 192 insertions(+), 104 deletions(-) diff --git a/app/components/phone_input_component.rb b/app/components/phone_input_component.rb index 0a512043747..c718ac00a40 100644 --- a/app/components/phone_input_component.rb +++ b/app/components/phone_input_component.rb @@ -57,7 +57,6 @@ def strings { country_code_label: t('components.phone_input.country_code_label'), invalid_phone: t('errors.messages.invalid_phone_number'), - country_constraint_usa: t('errors.messages.phone_country_constraint_usa'), unsupported_country: unsupported_country_string, } end diff --git a/app/controllers/idv/otp_delivery_method_controller.rb b/app/controllers/idv/otp_delivery_method_controller.rb index 19ab4f28c58..60a7cadb82e 100644 --- a/app/controllers/idv/otp_delivery_method_controller.rb +++ b/app/controllers/idv/otp_delivery_method_controller.rb @@ -12,7 +12,7 @@ class OtpDeliveryMethodController < ApplicationController def new analytics.idv_phone_otp_delivery_selection_visit - render :new, locals: { gpo_letter_available: gpo_letter_available } + render :new, locals: view_locals end def create @@ -25,6 +25,23 @@ def create private + attr_reader :idv_phone + + def view_locals + { + gpo_letter_available: gpo_letter_available, + phone_number_capabilities: phone_number_capabilities, + } + end + + def phone_number_capabilities + PhoneNumberCapabilities.new(idv_phone, phone_confirmed: user_phone?) + end + + def user_phone? + MfaContext.new(current_user).phone_configurations.any? { |config| config.phone == idv_phone } + end + def confirm_phone_step_complete redirect_to idv_phone_url if idv_session.vendor_phone_confirmation != true end @@ -44,7 +61,7 @@ def otp_delivery_selection_params def render_new_with_error_message flash[:error] = t('idv.errors.unsupported_otp_delivery_method') - render :new, locals: { gpo_letter_available: gpo_letter_available } + render :new, locals: view_locals end def send_phone_confirmation_otp_and_handle_result diff --git a/app/controllers/idv/phone_controller.rb b/app/controllers/idv/phone_controller.rb index 875b565576c..b413297c86c 100644 --- a/app/controllers/idv/phone_controller.rb +++ b/app/controllers/idv/phone_controller.rb @@ -100,7 +100,8 @@ def set_idv_form @idv_form = Idv::PhoneForm.new( user: current_user, previous_params: idv_session.previous_phone_step_params, - allowed_countries: ['US'], + allowed_countries: + PhoneNumberCapabilities::ADDRESS_IDENTITY_PROOFING_SUPPORTED_COUNTRY_CODES, ) end diff --git a/app/forms/idv/phone_form.rb b/app/forms/idv/phone_form.rb index 8d476d1a185..6cc3af2b40c 100644 --- a/app/forms/idv/phone_form.rb +++ b/app/forms/idv/phone_form.rb @@ -54,19 +54,16 @@ def initial_phone_value(input_phone) def validate_valid_phone_for_allowed_countries return if valid_phone_for_allowed_countries?(phone) - - if allowed_countries == ['US'] - errors.add(:phone, :must_have_us_country_code, type: :must_have_us_country_code) - else - errors.add(:phone, :improbable_phone, type: :improbable_phone) - end + errors.add(:phone, :improbable_phone, type: :improbable_phone) end def validate_phone_delivery_methods return unless valid_phone_for_allowed_countries?(phone) capabilities = PhoneNumberCapabilities.new(phone, phone_confirmed: user_phone?(phone)) - unsupported_delivery_methods(capabilities).each do |delivery_method| + unsupported_methods = unsupported_delivery_methods(capabilities) + return if unsupported_methods.count != delivery_methods.count + unsupported_methods.each do |delivery_method| errors.add( :phone, I18n.t( @@ -80,7 +77,7 @@ def validate_phone_delivery_methods def valid_phone_for_allowed_countries?(phone) if allowed_countries.present? - allowed_countries.all? { |country| Phonelib.valid_for_country?(phone, country) } + allowed_countries.any? { |country| Phonelib.valid_for_country?(phone, country) } else Phonelib.valid?(phone) end diff --git a/app/javascript/packages/phone-input/index.js b/app/javascript/packages/phone-input/index.js index e66726c0293..7fee5ce34ae 100644 --- a/app/javascript/packages/phone-input/index.js +++ b/app/javascript/packages/phone-input/index.js @@ -10,7 +10,6 @@ import { replaceVariables } from '@18f/identity-i18n'; * * @prop {string=} country_code_label * @prop {string=} invalid_phone - * @prop {string=} country_constraint_usa * @prop {string=} unsupported_country */ @@ -46,32 +45,34 @@ export class PhoneInput extends HTMLElement { countryCodePairs = {}; connectedCallback() { - /** @type {HTMLInputElement?} */ - this.textInput = this.querySelector('.phone-input__number'); - /** @type {HTMLSelectElement?} */ - this.codeInput = this.querySelector('.phone-input__international-code'); - this.codeWrapper = this.querySelector('.phone-input__international-code-wrapper'); - this.exampleText = this.querySelector('.phone-input__example'); - - try { - this.deliveryMethods = JSON.parse(this.dataset.deliveryMethods || ''); - this.countryCodePairs = JSON.parse(this.dataset.translatedCountryCodeNames || ''); - } catch {} - - if (!this.textInput || !this.codeInput) { - return; - } + if (this.isConnected) { + /** @type {HTMLInputElement?} */ + this.textInput = this.querySelector('.phone-input__number'); + /** @type {HTMLSelectElement?} */ + this.codeInput = this.querySelector('.phone-input__international-code'); + this.codeWrapper = this.querySelector('.phone-input__international-code-wrapper'); + this.exampleText = this.querySelector('.phone-input__example'); + + try { + this.deliveryMethods = JSON.parse(this.dataset.deliveryMethods || ''); + this.countryCodePairs = JSON.parse(this.dataset.translatedCountryCodeNames || ''); + } catch {} - this.iti = this.initializeIntlTelInput(); + if (!this.textInput || !this.codeInput) { + return; + } + + this.iti = this.initializeIntlTelInput(); - this.textInput.addEventListener('countrychange', () => this.syncCountryChangeToCodeInput()); - this.textInput.addEventListener('input', () => this.validate()); - this.codeInput.addEventListener('change', () => this.formatTextInput()); - this.codeInput.addEventListener('change', () => this.setExampleNumber()); - this.codeInput.addEventListener('change', () => this.validate()); + this.textInput.addEventListener('countrychange', () => this.syncCountryChangeToCodeInput()); + this.textInput.addEventListener('input', () => this.validate()); + this.codeInput.addEventListener('change', () => this.formatTextInput()); + this.codeInput.addEventListener('change', () => this.setExampleNumber()); + this.codeInput.addEventListener('change', () => this.validate()); - this.setExampleNumber(); - this.validate(); + this.setExampleNumber(); + this.validate(); + } } get selectedOption() { @@ -160,7 +161,7 @@ export class PhoneInput extends HTMLElement { } validate() { - const { textInput, codeInput, supportedCountryCodes, selectedOption } = this; + const { textInput, codeInput, selectedOption } = this; if (!textInput || !codeInput || !selectedOption) { return; } @@ -173,14 +174,9 @@ export class PhoneInput extends HTMLElement { return; } - const isInvalidCountry = - supportedCountryCodes?.length === 1 && !isValidNumberForRegion(phoneNumber, countryCode); + const isInvalidCountry = !isValidNumberForRegion(phoneNumber, countryCode); if (isInvalidCountry) { - if (countryCode === 'US') { - textInput.setCustomValidity(this.strings.country_constraint_usa || ''); - } else { - textInput.setCustomValidity(this.strings.invalid_phone || ''); - } + textInput.setCustomValidity(this.strings.invalid_phone || ''); } const isInvalidPhoneNumber = !isPhoneValid(phoneNumber, countryCode); diff --git a/app/javascript/packages/phone-input/index.spec.js b/app/javascript/packages/phone-input/index.spec.js index 86891e53940..e5731266e03 100644 --- a/app/javascript/packages/phone-input/index.spec.js +++ b/app/javascript/packages/phone-input/index.spec.js @@ -43,7 +43,6 @@ describe('PhoneInput', () => { { "country_code_label": "Country code", "invalid_phone": "Phone number is not valid", - "country_constraint_usa": "Must be a U.S. phone number", "unsupported_country": "We are unable to verify phone numbers from %{location}" } @@ -133,25 +132,13 @@ describe('PhoneInput', () => { }); it('validates phone from region', async () => { - const input = createAndConnectElement({ isSingleOption: true }); + const input = createAndConnectElement({ isNonUSSingleOption: true }); /** @type {HTMLInputElement} */ const phoneNumber = getByLabelText(input, 'Phone number'); - await userEvent.type(phoneNumber, '306-555-1234'); - expect(phoneNumber.validationMessage).to.equal('Must be a U.S. phone number'); - }); - - context('with non-U.S. single option', () => { - it('validates phone from region', async () => { - const input = createAndConnectElement({ isNonUSSingleOption: true }); - - /** @type {HTMLInputElement} */ - const phoneNumber = getByLabelText(input, 'Phone number'); - - await userEvent.type(phoneNumber, '513-555-1234'); - expect(phoneNumber.validationMessage).to.equal('Phone number is not valid'); - }); + await userEvent.type(phoneNumber, '513-555-1234'); + expect(phoneNumber.validationMessage).to.equal('Phone number is not valid'); }); }); diff --git a/app/services/idv/phone_step.rb b/app/services/idv/phone_step.rb index 521e8fbe6ff..b5c3a99e7d8 100644 --- a/app/services/idv/phone_step.rb +++ b/app/services/idv/phone_step.rb @@ -120,8 +120,13 @@ def start_phone_confirmation_session end def extra_analytics_attributes + parsed_phone = Phonelib.parse(applicant[:phone]) + { vendor: idv_result.except(:errors, :success), + area_code: parsed_phone.area_code, + country_code: parsed_phone.country, + phone_fingerprint: Pii::Fingerprinter.fingerprint(parsed_phone.e164), } end diff --git a/app/services/phone_number_capabilities.rb b/app/services/phone_number_capabilities.rb index cd63da50ab7..32a24b65faa 100644 --- a/app/services/phone_number_capabilities.rb +++ b/app/services/phone_number_capabilities.rb @@ -6,6 +6,8 @@ def self.load_config end INTERNATIONAL_CODES = load_config.freeze + ADDRESS_IDENTITY_PROOFING_SUPPORTED_COUNTRY_CODES = + IdentityConfig.store.address_identity_proofing_supported_country_codes attr_reader :phone, :phone_confirmed diff --git a/app/views/idv/otp_delivery_method/new.html.erb b/app/views/idv/otp_delivery_method/new.html.erb index a8bdd01f95c..955324501a1 100644 --- a/app/views/idv/otp_delivery_method/new.html.erb +++ b/app/views/idv/otp_delivery_method/new.html.erb @@ -24,33 +24,37 @@ ) do |f| %>

- <%= radio_button_tag( - 'otp_delivery_preference', - :sms, - false, - disabled: VendorStatus.new.vendor_outage?(:sms), - class: 'usa-radio__input usa-radio__input--tile', - ) %> - + <% if phone_number_capabilities.supports_sms? %> + <%= radio_button_tag( + 'otp_delivery_preference', + :sms, + false, + disabled: VendorStatus.new.vendor_outage?(:sms), + class: 'usa-radio__input usa-radio__input--tile', + ) %> + + <% end %> - <%= radio_button_tag( - 'otp_delivery_preference', - :voice, - false, - disabled: VendorStatus.new.vendor_outage?(:voice), - class: 'usa-radio__input usa-radio__input--tile', - ) %> - + <% if phone_number_capabilities.supports_voice? %> + <%= radio_button_tag( + 'otp_delivery_preference', + :voice, + false, + disabled: VendorStatus.new.vendor_outage?(:voice), + class: 'usa-radio__input usa-radio__input--tile', + ) %> + + <% end %>
<%= submit_tag(t('idv.buttons.send_confirmation_code'), class: 'usa-button usa-button--big usa-button--wide') %> diff --git a/app/views/idv/phone/new.html.erb b/app/views/idv/phone/new.html.erb index 14dcd6debf0..f65733d4750 100644 --- a/app/views/idv/phone/new.html.erb +++ b/app/views/idv/phone/new.html.erb @@ -56,7 +56,7 @@ ) do |f| %> <%= render PhoneInputComponent.new( form: f, - allowed_countries: ['US'], + allowed_countries: @idv_form.allowed_countries, required: true, class: 'margin-bottom-4', ) %> diff --git a/config/application.yml.default b/config/application.yml.default index 9b1f7bdbaab..5677d3533b8 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -41,6 +41,7 @@ acuant_upload_image_timeout: 1.0 acuant_get_results_timeout: 1.0 acuant_create_document_timeout: 1.0 add_email_link_valid_for_hours: 24 +address_identity_proofing_supported_country_codes: '["AS", "GU", "MP", "PR", "US", "VI"]' asset_host: '' async_wait_timeout_seconds: 60 async_stale_job_timeout_seconds: 300 diff --git a/config/locales/errors/en.yml b/config/locales/errors/en.yml index 1c887dbc1a7..a58141d4a61 100644 --- a/config/locales/errors/en.yml +++ b/config/locales/errors/en.yml @@ -67,15 +67,12 @@ en: invalid_voice_number: Invalid phone number. Check that you’ve entered the correct country code or area code. missing_field: Please fill in this field. - must_have_us_country_code: Invalid phone number. Please make sure you enter a - phone number with a U.S. country code. no_pending_profile: No profile is waiting for verification not_a_number: is not a number otp_failed: Your security code failed to send. password_incorrect: Incorrect password personal_key_incorrect: Incorrect personal key phone_confirmation_throttled: You tried too many times, please try again in %{timeout}. - phone_country_constraint_usa: Must be a U.S. phone number phone_duplicate: This account is already using the phone number you entered as an authenticator. Please use a different phone number. phone_required: Phone number is required diff --git a/config/locales/errors/es.yml b/config/locales/errors/es.yml index 4304186aa3d..40842a7d7f5 100644 --- a/config/locales/errors/es.yml +++ b/config/locales/errors/es.yml @@ -69,15 +69,12 @@ es: invalid_voice_number: Numero de telefono invalido. Verifique que haya ingresado el código de país o de área correcto. missing_field: Por favor, rellene este campo. - must_have_us_country_code: Número de teléfono no válido. Asegúrate de introducir - un número de teléfono con un código país de EE. UU. no_pending_profile: Ningún perfil está esperando verificación not_a_number: no es un número otp_failed: Se produjo un error al enviar el código de seguridad. password_incorrect: La contraseña es incorrecta personal_key_incorrect: La clave personal es incorrecta phone_confirmation_throttled: Lo intentaste muchas veces, vuelve a intentarlo en %{timeout}. - phone_country_constraint_usa: Debe ser un número telefónico de los Estados Unidos phone_duplicate: Esta cuenta ya está utilizando el número de teléfono que ingresó como autenticador. Por favor, use un número de teléfono diferente. diff --git a/config/locales/errors/fr.yml b/config/locales/errors/fr.yml index 62a2a1f873f..2a5a3597c3c 100644 --- a/config/locales/errors/fr.yml +++ b/config/locales/errors/fr.yml @@ -76,15 +76,12 @@ fr: invalid_voice_number: Numéro de téléphone invalide. Vérifiez que vous avez entré le bon indicatif international ou régional. missing_field: Veuillez remplir ce champ. - must_have_us_country_code: Numéro de téléphone non valide. Veuillez vous assurer - de saisir un numéro de téléphone avec un code pays des États-Unis. no_pending_profile: Aucun profil en attente de vérification not_a_number: N’est pas un nombre otp_failed: Échec de l’envoi de votre code de sécurité. password_incorrect: Mot de passe incorrect personal_key_incorrect: Clé personnelle incorrecte phone_confirmation_throttled: Vous avez essayé plusieurs fois, essayez à nouveau dans %{timeout}. - phone_country_constraint_usa: Dois être un numéro de téléphone américain phone_duplicate: Ce compte utilise déjà le numéro de téléphone que vous avez entré en tant qu’authentificateur. Veuillez utiliser un numéro de téléphone différent. diff --git a/lib/identity_config.rb b/lib/identity_config.rb index 81e47c3394d..7e92c9f946f 100644 --- a/lib/identity_config.rb +++ b/lib/identity_config.rb @@ -100,6 +100,7 @@ def self.build_store(config_map) config.add(:acuant_get_results_timeout, type: :float) config.add(:acuant_create_document_timeout, type: :float) config.add(:add_email_link_valid_for_hours, type: :integer) + config.add(:address_identity_proofing_supported_country_codes, type: :json) config.add(:asset_host, type: :string) config.add(:async_wait_timeout_seconds, type: :integer) config.add(:async_stale_job_timeout_seconds, type: :integer) diff --git a/spec/controllers/idv/phone_controller_spec.rb b/spec/controllers/idv/phone_controller_spec.rb index 7224b4a5b63..e54240baf9c 100644 --- a/spec/controllers/idv/phone_controller_spec.rb +++ b/spec/controllers/idv/phone_controller_spec.rb @@ -143,14 +143,14 @@ it 'renders #new' do put :create, params: { idv_phone_form: { phone: '703' } } - expect(flash[:error]).to eq t('errors.messages.must_have_us_country_code') + expect(flash[:error]).to eq t('errors.messages.improbable_phone') expect(response).to render_template(:new) end it 'disallows non-US numbers' do put :create, params: { idv_phone_form: { phone: international_phone } } - expect(flash[:error]).to eq t('errors.messages.must_have_us_country_code') + expect(flash[:error]).to eq t('errors.messages.improbable_phone') expect(response).to render_template(:new) end @@ -162,10 +162,10 @@ result = { success: false, errors: { - phone: [t('errors.messages.must_have_us_country_code')], + phone: [t('errors.messages.improbable_phone')], }, error_details: { - phone: [:must_have_us_country_code], + phone: [:improbable_phone], }, pii_like_keypaths: [[:errors, :phone], [:error_details, :phone]], country_code: nil, @@ -189,7 +189,7 @@ success: false, phone_number: '703', failure_reason: { - phone: [t('errors.messages.must_have_us_country_code')], + phone: [t('errors.messages.improbable_phone')], }, ) end @@ -314,6 +314,7 @@ end it 'tracks event with valid phone' do + proofing_phone = Phonelib.parse(good_phone) user = build(:user, with: { phone: '+1 (415) 555-0130', phone_confirmed_at: Time.zone.now }) stub_verify_steps_one_and_two(user) @@ -324,6 +325,9 @@ success: true, new_phone_added: true, errors: {}, + phone_fingerprint: Pii::Fingerprinter.fingerprint(proofing_phone.e164), + country_code: proofing_phone.country, + area_code: proofing_phone.area_code, pii_like_keypaths: [[:errors, :phone], [:context, :stages, :address]], vendor: { vendor_name: 'AddressMock', @@ -362,6 +366,7 @@ end it 'tracks event with invalid phone' do + proofing_phone = Phonelib.parse(bad_phone) user = build(:user, with: { phone: '+1 (415) 555-0130', phone_confirmed_at: Time.zone.now }) stub_verify_steps_one_and_two(user) @@ -371,6 +376,9 @@ result = { success: false, new_phone_added: true, + phone_fingerprint: Pii::Fingerprinter.fingerprint(proofing_phone.e164), + country_code: proofing_phone.country, + area_code: proofing_phone.area_code, errors: { phone: ['The phone number could not be verified.'], }, diff --git a/spec/features/idv/steps/phone_otp_delivery_selection_step_spec.rb b/spec/features/idv/steps/phone_otp_delivery_selection_step_spec.rb index d1ebb9ea0f5..09f9f942a63 100644 --- a/spec/features/idv/steps/phone_otp_delivery_selection_step_spec.rb +++ b/spec/features/idv/steps/phone_otp_delivery_selection_step_spec.rb @@ -71,7 +71,7 @@ it 'displays an error message' do expect(Telephony).to_not receive(:send_confirmation_otp) - expect(page).to have_content(t('errors.messages.phone_country_constraint_usa')) + expect(page).to have_content(t('errors.messages.invalid_phone_number')) expect(current_path).to eq(idv_phone_path) end end diff --git a/spec/forms/idv/phone_form_spec.rb b/spec/forms/idv/phone_form_spec.rb index ffdb92675c3..1b7fc5e6bd2 100644 --- a/spec/forms/idv/phone_form_spec.rb +++ b/spec/forms/idv/phone_form_spec.rb @@ -89,7 +89,7 @@ end context 'with specific allowed countries' do - let(:allowed_countries) { ['MP'] } + let(:allowed_countries) { ['MP', 'US'] } it 'validates to only allow numbers from permitted countries' do invalid_phones = ['+81 54 354 3643', '+12423270143'] @@ -116,7 +116,7 @@ result = subject.submit(phone: phone) expect(result.success?).to eq(false) - expect(result.errors[:phone]).to include(t('errors.messages.must_have_us_country_code')) + expect(result.errors[:phone]).to include(t('errors.messages.improbable_phone')) end valid_phones = ['7035551234'] valid_phones.each do |phone| @@ -154,5 +154,44 @@ end end end + + context 'with unsupported delivery method' do + let(:unsupported_delivery_methods) { [] } + let(:result) { subject.submit(params) } + + before do + allow(subject).to receive(:unsupported_delivery_methods). + and_return(unsupported_delivery_methods) + end + + context 'with one unsupported delivery method' do + let(:unsupported_delivery_methods) { [:sms] } + + it 'is valid' do + expect(result.success?).to eq(true) + expect(result.errors).to eq({}) + end + end + + context 'with all delivery methods unsupported' do + let(:unsupported_delivery_methods) { [:sms, :voice] } + + it 'is invalid' do + expect(result.success?).to eq(false) + expect(result.errors).to eq( + phone: [ + t( + 'two_factor_authentication.otp_delivery_preference.sms_unsupported', + location: 'United States', + ), + t( + 'two_factor_authentication.otp_delivery_preference.voice_unsupported', + location: 'United States', + ), + ], + ) + end + end + end end end diff --git a/spec/services/idv/phone_step_spec.rb b/spec/services/idv/phone_step_spec.rb index eb219503f70..42e004100b4 100644 --- a/spec/services/idv/phone_step_spec.rb +++ b/spec/services/idv/phone_step_spec.rb @@ -49,7 +49,11 @@ let(:throttle) { Throttle.new(throttle_type: :proof_address, user: user) } it 'succeeds with good params' do + proofing_phone = Phonelib.parse(good_phone) extra = { + phone_fingerprint: Pii::Fingerprinter.fingerprint(proofing_phone.e164), + country_code: proofing_phone.country, + area_code: proofing_phone.area_code, vendor: { vendor_name: 'AddressMock', exception: nil, @@ -78,7 +82,11 @@ end it 'fails with bad params' do + proofing_phone = Phonelib.parse(bad_phone) extra = { + phone_fingerprint: Pii::Fingerprinter.fingerprint(proofing_phone.e164), + country_code: proofing_phone.country, + area_code: proofing_phone.area_code, vendor: { vendor_name: 'AddressMock', exception: nil, diff --git a/spec/views/idv/otp_delivery_method/new.html.erb_spec.rb b/spec/views/idv/otp_delivery_method/new.html.erb_spec.rb index 97b0accbd72..46d4585739d 100644 --- a/spec/views/idv/otp_delivery_method/new.html.erb_spec.rb +++ b/spec/views/idv/otp_delivery_method/new.html.erb_spec.rb @@ -3,8 +3,17 @@ describe 'idv/otp_delivery_method/new.html.erb' do let(:gpo_letter_available) { false } let(:step_indicator_steps) { Idv::Flows::DocAuthFlow::STEP_INDICATOR_STEPS } + let(:supports_sms) { true } + let(:supports_voice) { true } before do + phone_number_capabilities = instance_double( + PhoneNumberCapabilities, + supports_sms?: supports_sms, + supports_voice?: supports_voice, + ) + + allow(view).to receive(:phone_number_capabilities).and_return(phone_number_capabilities) allow(view).to receive(:user_signing_up?).and_return(false) allow(view).to receive(:user_fully_authenticated?).and_return(true) allow(view).to receive(:gpo_letter_available).and_return(gpo_letter_available) @@ -46,4 +55,27 @@ expect(rendered).to have_field('otp_delivery_preference', with: :sms, disabled: true) end end + + it 'renders sms and voice options' do + expect(rendered).to have_field('otp_delivery_preference', with: :voice) + expect(rendered).to have_field('otp_delivery_preference', with: :sms) + end + + context 'without sms support' do + let(:supports_sms) { false } + + it 'renders voice option' do + expect(rendered).to have_field('otp_delivery_preference', with: :voice) + expect(rendered).not_to have_field('otp_delivery_preference', with: :sms) + end + end + + context 'without voice support' do + let(:supports_voice) { false } + + it 'renders sms option' do + expect(rendered).not_to have_field('otp_delivery_preference', with: :voice) + expect(rendered).to have_field('otp_delivery_preference', with: :sms) + end + end end From 52c19fcfb3fe71a3109e4e14c4bb2b26997135e1 Mon Sep 17 00:00:00 2001 From: eileen-nava <80347702+eileen-nava@users.noreply.github.com> Date: Fri, 23 Sep 2022 12:17:35 -0400 Subject: [PATCH 12/23] LG-7648: Create PR template (#7015) * create pr template * changelog: Internal, In-person proofing, create template for PRs * drop the reference to jira --- pull_request_template.md | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 pull_request_template.md diff --git a/pull_request_template.md b/pull_request_template.md new file mode 100644 index 00000000000..08199810137 --- /dev/null +++ b/pull_request_template.md @@ -0,0 +1,33 @@ +## 🎫 Ticket + +Link to the relevant ticket. + +## 🛠 Summary of changes + +Write a brief description of what you changed. + +## 📜 Testing Plan + +Provide a checklist of steps to confirm the changes. + +- [ ] Step 1 +- [ ] Step 2 +- [ ] Step 3 + +## 👀 Screenshots + +If relevant, include a screenshot or screen capture of the changes. + +
+Before: + +
+ +
+After: + +
+ +## 🚀 Notes for Deployment + +Include any special instructions for deployment. From 4b6ebbc77493c7d2dded92a92234e360cecfaff3 Mon Sep 17 00:00:00 2001 From: "Gene M. Angelo, Jr" Date: Fri, 23 Sep 2022 14:06:06 -0400 Subject: [PATCH 13/23] LG-7602 Pull VA IP Data and Store It In Session (Controller and Flow State) 3 of 3 (#7022) * Include mixins needed for Inherited Proofing flow Specifically, obtaining user PII from the service provider and flow state support, including session massaging in preparation for IDV unattended work fllow. changelog: Upcoming Features, Inherited Proofing, Pull VA IP Data and Store It In Session (LG-7602) * Set the initial session for our flow This sets up session for the subsequent Inherited Proofing (IP) work flow/flow steps. * Add mixin to obtain user pii needed In order to proof our Inherited Proofing user: NOTE: the use of InheritedProofingConcern #inherited_proofing_service, members via its inclusion in the InheritedProofingController. Add delegations needed for conveinient access to controller and idv_session from InheritedProofingBaseStep so that we have these in any Flow Step we need them. * Add a mixin to manage the user pii and flow session This mixin provides a simple means to store user pii into the flow_session[:pii_from_user] and manage idv_session session data in preparation for IDV workflow handoff. * Refactor the AgreementStep to save user pii To session. Consume previously added UserPiiManagable mixin and 1) Save the user pii to session in the #call method 2) Return the FormResponse from the Service Provider API call to obtain user pii, so that it can be processed by the base class e.g. merged in with the Flow Step Form FormResponse and passed to analytics properly/perform error handling.. * Refactor specs to accomodate changes The Inherited Proofing (IP) flow steps now need current_user to be populated as would be the case in a real-life scenario as the user would have created an account and logged in before being redirected to the IP work flow. This change sets up current_user to create a real-live scenario. Specs also test to make sure flow steps are working regardless of the IdentityConfig.store.va_inherited_proofing_mock_enabled setting, although this may be sufficiently covered by the spec/controllers/concerns/inherited_proofing_concern_spec.rb specs. --- .../idv/inherited_proofing_controller.rb | 2 + .../idv/flows/inherited_proofing_flow.rb | 5 +++ .../inherited_proofing/agreement_step.rb | 4 ++ .../inherited_proofing/user_pii_managable.rb | 37 ++++++++++++++++++ .../user_pii_retrievable.rb | 39 +++++++++++++++++++ .../idv/steps/inherited_proofing_base_step.rb | 2 + lib/session_encryptor.rb | 1 + .../idv/inherited_proofing_controller_spec.rb | 32 +++++++++++++-- .../inherited_proofing/agreement_step_spec.rb | 10 +++++ 9 files changed, 129 insertions(+), 3 deletions(-) create mode 100644 app/services/idv/steps/inherited_proofing/user_pii_managable.rb create mode 100644 app/services/idv/steps/inherited_proofing/user_pii_retrievable.rb diff --git a/app/controllers/idv/inherited_proofing_controller.rb b/app/controllers/idv/inherited_proofing_controller.rb index 398553acba4..90e9e79c9b4 100644 --- a/app/controllers/idv/inherited_proofing_controller.rb +++ b/app/controllers/idv/inherited_proofing_controller.rb @@ -1,6 +1,8 @@ module Idv class InheritedProofingController < ApplicationController include Flow::FlowStateMachine + include IdvSession + include InheritedProofingConcern before_action :render_404_if_disabled diff --git a/app/services/idv/flows/inherited_proofing_flow.rb b/app/services/idv/flows/inherited_proofing_flow.rb index b44b417fcac..b24858a9913 100644 --- a/app/services/idv/flows/inherited_proofing_flow.rb +++ b/app/services/idv/flows/inherited_proofing_flow.rb @@ -22,6 +22,11 @@ class InheritedProofingFlow < Flow::BaseFlow def initialize(controller, session, name) @idv_session = self.class.session_idv(session) super(controller, STEPS, ACTIONS, session[name]) + + @flow_session ||= {} + @flow_session[:pii_from_user] ||= { uuid: current_user.uuid } + applicant = @idv_session['applicant'] || {} + @flow_session[:pii_from_user] = @flow_session[:pii_from_user].to_h.merge(applicant) end def self.session_idv(session) diff --git a/app/services/idv/steps/inherited_proofing/agreement_step.rb b/app/services/idv/steps/inherited_proofing/agreement_step.rb index 052476b8e29..baef3e74130 100644 --- a/app/services/idv/steps/inherited_proofing/agreement_step.rb +++ b/app/services/idv/steps/inherited_proofing/agreement_step.rb @@ -2,9 +2,13 @@ module Idv module Steps module InheritedProofing class AgreementStep < InheritedProofingBaseStep + include UserPiiManagable + STEP_INDICATOR_STEP = :getting_started def call + inherited_proofing_save_user_pii_to_session! + inherited_proofing_form_response end def form_submit diff --git a/app/services/idv/steps/inherited_proofing/user_pii_managable.rb b/app/services/idv/steps/inherited_proofing/user_pii_managable.rb new file mode 100644 index 00000000000..544d92525c9 --- /dev/null +++ b/app/services/idv/steps/inherited_proofing/user_pii_managable.rb @@ -0,0 +1,37 @@ +module Idv + module Steps + module InheritedProofing + module UserPiiManagable + include UserPiiRetrievable + + def inherited_proofing_save_user_pii_to_session! + inherited_proofing_save_session! + inherited_proofing_skip_steps! + end + + private + + def inherited_proofing_save_session! + return unless inherited_proofing_form_response.success? + + flow_session[:pii_from_user] = + flow_session[:pii_from_user].to_h.merge(inherited_proofing_user_pii) + # This is unnecessary, but added for completeness. Any subsequent FLOWS we + # might splice into will pull from idv_session['applicant'] and merge into + # flow_session[:pii_from_user] anyhow in their #initialize(r); any subsequent + # STEP FLOWS we might splice into will populate idv_session['applicant'] and + # ultimately get merged in to flow_session[:pii_from_user] as well. + idv_session['applicant'] = flow_session[:pii_from_user] + end + + def inherited_proofing_skip_steps! + idv_session['profile_confirmation'] = true + idv_session['vendor_phone_confirmation'] = false + idv_session['user_phone_confirmation'] = false + idv_session['address_verification_mechanism'] = 'phone' + idv_session['resolution_successful'] = 'phone' + end + end + end + end +end diff --git a/app/services/idv/steps/inherited_proofing/user_pii_retrievable.rb b/app/services/idv/steps/inherited_proofing/user_pii_retrievable.rb new file mode 100644 index 00000000000..36323660088 --- /dev/null +++ b/app/services/idv/steps/inherited_proofing/user_pii_retrievable.rb @@ -0,0 +1,39 @@ +module Idv + module Steps + module InheritedProofing + module UserPiiRetrievable + def inherited_proofing_user_pii + inherited_proofing_info[0] + end + + def inherited_proofing_form_response + inherited_proofing_info[1] + end + + private + + # This needs error handling. + def inherited_proofing_info + return @inherited_proofing_info if defined? @inherited_proofing_info + + payload_hash = inherited_proofing_service.execute.dup + form = inherited_proofing_form(payload_hash) + form_response = form.submit + + user_pii = {} + user_pii = form.user_pii if form_response.success? + + @inherited_proofing_info = [user_pii, form_response] + end + + def inherited_proofing_service + controller.inherited_proofing_service + end + + def inherited_proofing_form(payload_hash) + controller.inherited_proofing_form payload_hash + end + end + end + end +end diff --git a/app/services/idv/steps/inherited_proofing_base_step.rb b/app/services/idv/steps/inherited_proofing_base_step.rb index 0611e901306..43c18f90889 100644 --- a/app/services/idv/steps/inherited_proofing_base_step.rb +++ b/app/services/idv/steps/inherited_proofing_base_step.rb @@ -1,6 +1,8 @@ module Idv module Steps class InheritedProofingBaseStep < Flow::BaseStep + delegate :controller, :idv_session, to: :@flow + def initialize(flow) super(flow, :inherited_proofing) end diff --git a/lib/session_encryptor.rb b/lib/session_encryptor.rb index 397b7e46a09..4ef683508b9 100644 --- a/lib/session_encryptor.rb +++ b/lib/session_encryptor.rb @@ -14,6 +14,7 @@ class SensitiveValueError < StandardError; end # personal keys are generated and stored in the session between requests, but are used # to decrypt PII bundles, so we treat them similarly to the PII itself. SENSITIVE_PATHS = [ + ['warden.user.user.session', 'idv/inherited_proofing'], ['warden.user.user.session', 'idv/doc_auth'], ['warden.user.user.session', 'idv/in_person'], ['warden.user.user.session', 'idv'], diff --git a/spec/controllers/idv/inherited_proofing_controller_spec.rb b/spec/controllers/idv/inherited_proofing_controller_spec.rb index d70a234c336..2fc7541ff39 100644 --- a/spec/controllers/idv/inherited_proofing_controller_spec.rb +++ b/spec/controllers/idv/inherited_proofing_controller_spec.rb @@ -1,6 +1,6 @@ require 'rails_helper' -describe Idv::InheritedProofingController do +shared_examples 'the flow steps work correctly' do describe '#index' do it 'redirects to the first step' do get :index @@ -50,8 +50,34 @@ expect(response).to redirect_to idv_inherited_proofing_step_url(step: :get_started) end end +end + +def mock_next_step(step) + allow_any_instance_of(Idv::Flows::InheritedProofingFlow).to receive(:next_step).and_return(step) +end + +describe Idv::InheritedProofingController do + let(:sp) { nil } + let(:user) { build(:user) } + + before do + allow(controller).to receive(:current_sp).and_return(sp) + stub_sign_in(user) + end + + context 'when VA inherited proofing mock is enabled' do + before do + allow(IdentityConfig.store).to receive(:va_inherited_proofing_mock_enabled).and_return(true) + end + + it_behaves_like 'the flow steps work correctly' + end + + context 'when VA inherited proofing mock is not enabled' do + before do + allow(IdentityConfig.store).to receive(:va_inherited_proofing_mock_enabled).and_return(false) + end - def mock_next_step(step) - allow_any_instance_of(Idv::Flows::InheritedProofingFlow).to receive(:next_step).and_return(step) + it_behaves_like 'the flow steps work correctly' end end diff --git a/spec/features/idv/inherited_proofing/agreement_step_spec.rb b/spec/features/idv/inherited_proofing/agreement_step_spec.rb index d98193f9730..d9aabf73d6c 100644 --- a/spec/features/idv/inherited_proofing/agreement_step_spec.rb +++ b/spec/features/idv/inherited_proofing/agreement_step_spec.rb @@ -4,6 +4,16 @@ include IdvHelper include DocAuthHelper + before do + allow(IdentityConfig.store).to receive(:va_inherited_proofing_mock_enabled).and_return true + allow_any_instance_of(Idv::InheritedProofingController).to \ + receive(:va_inherited_proofing?).and_return true + allow_any_instance_of(Idv::InheritedProofingController).to \ + receive(:va_inherited_proofing_auth_code).and_return auth_code + end + + let(:auth_code) { Idv::InheritedProofing::Va::Mocks::Service::VALID_AUTH_CODE } + def expect_ip_verify_info_step expect(page).to have_current_path(idv_ip_verify_info_step) end From b0b171dc013de2dde61114806beacaf1bb4e2435 Mon Sep 17 00:00:00 2001 From: Jessica Dembe Date: Fri, 23 Sep 2022 15:53:25 -0400 Subject: [PATCH 14/23] LG-6387: Make CTA button more prominent (#6986) * Change text * add text, rule, and styling for "First time using Login.gov? Add text, styling, and HTML for "First time using Login.gov?" * add translation for "First time using Login.gov?" * Add styling and adjust margin for buttons and text * delete unnecessary CSS and use USWDS classes Changelog: Improvements, User experience, Make CTA button more prominent [LG-6387] * standardize with use of em over px * Fix lint, replace login.gov with app name * add bg white to span using uswds class * Fix app name syntax * Use USWDS class for border-bottom * address code review comment * restructure text * changelog changelog: Improvements, UX experience, make CTA more prominent * fix margin between buttons and text Co-authored-by: Jack Cody Co-authored-by: Jessica Dembe --- .../stylesheets/utilities/_typography.scss | 26 +++++++++++++++++++ app/views/devise/sessions/new.html.erb | 7 +++-- config/locales/headings/en.yml | 1 + config/locales/headings/es.yml | 1 + config/locales/headings/fr.yml | 1 + 5 files changed, 34 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/utilities/_typography.scss b/app/assets/stylesheets/utilities/_typography.scss index 9b8678f5bc0..5117f2d2454 100644 --- a/app/assets/stylesheets/utilities/_typography.scss +++ b/app/assets/stylesheets/utilities/_typography.scss @@ -99,3 +99,29 @@ h6, .h6 { @extend %h6; } + +.separator-text { + display: flex; + align-items: center; + text-align: center; + font-size: 1.125rem; + margin-top: 32px; + margin-bottom: 16px; + + &::before, + &::after { + content: ''; + display: block; + border-bottom: 1px solid color('primary-light'); + flex-grow: 1; + min-width: 2rem; + } + + &::before { + margin-right: 1rem; + } + + &::after { + margin-left: 1rem; + } +} diff --git a/app/views/devise/sessions/new.html.erb b/app/views/devise/sessions/new.html.erb index 820be8f9529..ad1d516397c 100644 --- a/app/views/devise/sessions/new.html.erb +++ b/app/views/devise/sessions/new.html.erb @@ -34,11 +34,14 @@ ) %> <%= f.input :request_id, as: :hidden, input_html: { value: @request_id } %>
- <%= f.submit t('links.next'), full_width: true, big: false, wide: false, class: 'margin-bottom-2' %> + <%= f.submit t('links.next'), full_width: true, big: false, wide: false %> +

+ <%= t('headings.create_account_with_sp.cta', app_name: APP_NAME) %> +

<%= link_to( t('links.create_account'), sign_up_email_url(request_id: @request_id), - class: 'text-no-underline usa-button usa-button--outline usa-button--full-width', + class: 'text-no-underline usa-button usa-button--outline usa-button--full-width margin-top-1', ) %>
<% end %> diff --git a/config/locales/headings/en.yml b/config/locales/headings/en.yml index 67f5f57e499..c19a456178e 100644 --- a/config/locales/headings/en.yml +++ b/config/locales/headings/en.yml @@ -22,6 +22,7 @@ en: confirmations: new: Send another confirmation email create_account_with_sp: + cta: First time using %{app_name}? sp_text: is using %{app_name} to allow you to sign in to your account safely and securely. create_account_without_sp: Create a %{app_name} account diff --git a/config/locales/headings/es.yml b/config/locales/headings/es.yml index 74df805619c..01449e023fc 100644 --- a/config/locales/headings/es.yml +++ b/config/locales/headings/es.yml @@ -22,6 +22,7 @@ es: confirmations: new: Enviar otro email de confirmación create_account_with_sp: + cta: ¿Es la primera vez que utiliza %{app_name}? sp_text: está utilizando %{app_name} para permitirle iniciar sesión en su cuenta de forma segura. create_account_without_sp: Establezca una cuenta de %{app_name} diff --git a/config/locales/headings/fr.yml b/config/locales/headings/fr.yml index 788b6f180fb..de2c4e53066 100644 --- a/config/locales/headings/fr.yml +++ b/config/locales/headings/fr.yml @@ -22,6 +22,7 @@ fr: confirmations: new: Envoyer un autre courriel de confirmation create_account_with_sp: + cta: Première fois que vous utilisez %{app_name}? sp_text: utilise %{app_name} pour vous permettre de vous connecter à votre compte de façon sûre et sécurisée. create_account_without_sp: Créer un compte %{app_name} From 54cd883e97daf7ee0c765e5309ed59e2f56e2a68 Mon Sep 17 00:00:00 2001 From: "Luis H. Matos" Date: Sat, 24 Sep 2022 09:23:16 -0500 Subject: [PATCH 15/23] LG-7417 Standardize failure responses (#7006) * LG-7417 Standardize failure responses changelog: Internal, Attempts API, Standardize events --- .../delete_account_controller.rb | 2 +- .../concerns/unconfirmed_user_concern.rb | 2 +- app/controllers/idv/gpo_verify_controller.rb | 2 +- app/controllers/idv/phone_controller.rb | 2 +- .../sign_up/passwords_controller.rb | 2 +- .../sign_up/registrations_controller.rb | 2 +- .../piv_cac_verification_controller.rb | 2 +- app/controllers/users/passwords_controller.rb | 2 +- ...piv_cac_authentication_setup_controller.rb | 2 +- .../piv_cac_setup_from_sign_in_controller.rb | 2 +- .../users/reset_passwords_controller.rb | 6 +- .../users/verify_personal_key_controller.rb | 2 +- app/services/idv/steps/verify_base_step.rb | 2 +- app/services/irs_attempts_api/tracker.rb | 4 + .../delete_account_controller_spec.rb | 38 ++-- .../idv/gpo_verify_controller_spec.rb | 14 +- spec/controllers/idv/phone_controller_spec.rb | 53 +++--- .../sign_up/passwords_controller_spec.rb | 45 ++--- .../sign_up/registrations_controller_spec.rb | 20 +- .../piv_cac_verification_controller_spec.rb | 17 +- .../users/passwords_controller_spec.rb | 22 +-- .../users/reset_passwords_controller_spec.rb | 171 +++++++----------- .../verify_personal_key_controller_spec.rb | 18 +- spec/forms/idv/api_image_upload_form_spec.rb | 2 +- .../services/irs_attempts_api/tracker_spec.rb | 30 +++ spec/support/fake_attempts_tracker.rb | 4 + 26 files changed, 221 insertions(+), 247 deletions(-) diff --git a/app/controllers/account_reset/delete_account_controller.rb b/app/controllers/account_reset/delete_account_controller.rb index 6d1fbdcfe9f..a9d9d7625b8 100644 --- a/app/controllers/account_reset/delete_account_controller.rb +++ b/app/controllers/account_reset/delete_account_controller.rb @@ -20,7 +20,7 @@ def delete irs_attempts_api_tracker.account_reset_account_deleted( success: result.success?, - failure_reason: result.errors, + failure_reason: irs_attempts_api_tracker.parse_failure_reason(result), ) if result.success? handle_successful_deletion(result) diff --git a/app/controllers/concerns/unconfirmed_user_concern.rb b/app/controllers/concerns/unconfirmed_user_concern.rb index 96cf058fe4c..8cc637457c4 100644 --- a/app/controllers/concerns/unconfirmed_user_concern.rb +++ b/app/controllers/concerns/unconfirmed_user_concern.rb @@ -38,7 +38,7 @@ def stop_if_invalid_token irs_attempts_api_tracker.user_registration_email_confirmation( email: @email_address&.email, success: result.success?, - failure_reason: result.to_h[:error_details], + failure_reason: irs_attempts_api_tracker.parse_failure_reason(result), ) return if result.success? process_unsuccessful_confirmation diff --git a/app/controllers/idv/gpo_verify_controller.rb b/app/controllers/idv/gpo_verify_controller.rb index 37d5bb8559a..42445444156 100644 --- a/app/controllers/idv/gpo_verify_controller.rb +++ b/app/controllers/idv/gpo_verify_controller.rb @@ -35,7 +35,7 @@ def create analytics.idv_gpo_verification_submitted(**result.to_h) irs_attempts_api_tracker.idv_gpo_verification_submitted( success: result.success?, - failure_reason: result.errors.presence, + failure_reason: irs_attempts_api_tracker.parse_failure_reason(result), ) if result.success? diff --git a/app/controllers/idv/phone_controller.rb b/app/controllers/idv/phone_controller.rb index b413297c86c..a0b6a80e4b1 100644 --- a/app/controllers/idv/phone_controller.rb +++ b/app/controllers/idv/phone_controller.rb @@ -34,7 +34,7 @@ def create irs_attempts_api_tracker.idv_phone_submitted( phone_number: step_params[:phone], success: result.success?, - failure_reason: result.errors, + failure_reason: irs_attempts_api_tracker.parse_failure_reason(result), ) flash[:error] = result.first_error_message if !result.success? return render :new, locals: { gpo_letter_available: gpo_letter_available } if !result.success? diff --git a/app/controllers/sign_up/passwords_controller.rb b/app/controllers/sign_up/passwords_controller.rb index 6f8947118b4..a5c73987b96 100644 --- a/app/controllers/sign_up/passwords_controller.rb +++ b/app/controllers/sign_up/passwords_controller.rb @@ -16,7 +16,7 @@ def create analytics.password_creation(**result.to_h) irs_attempts_api_tracker.user_registration_password_submitted( success: result.success?, - failure_reason: result.errors, + failure_reason: irs_attempts_api_tracker.parse_failure_reason(result), ) store_sp_metadata_in_session unless sp_request_id.empty? diff --git a/app/controllers/sign_up/registrations_controller.rb b/app/controllers/sign_up/registrations_controller.rb index 522e6e26960..19204b5c997 100644 --- a/app/controllers/sign_up/registrations_controller.rb +++ b/app/controllers/sign_up/registrations_controller.rb @@ -30,7 +30,7 @@ def create irs_attempts_api_tracker.user_registration_email_submitted( email: permitted_params[:email], success: result.success?, - failure_reason: result.to_h[:error_details], + failure_reason: irs_attempts_api_tracker.parse_failure_reason(result), ) if result.success? diff --git a/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb b/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb index e36aae6a8b7..a01649d9b74 100644 --- a/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb +++ b/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb @@ -33,7 +33,7 @@ def process_token irs_attempts_api_tracker.mfa_login_piv_cac( success: result.success?, subject_dn: piv_cac_verification_form.x509_dn, - failure_reason: result.errors.presence, + failure_reason: irs_attempts_api_tracker.parse_failure_reason(result), ) if result.success? handle_valid_piv_cac diff --git a/app/controllers/users/passwords_controller.rb b/app/controllers/users/passwords_controller.rb index feb6314d61d..b9a482d2ca7 100644 --- a/app/controllers/users/passwords_controller.rb +++ b/app/controllers/users/passwords_controller.rb @@ -18,7 +18,7 @@ def update analytics.password_changed(**result.to_h) irs_attempts_api_tracker.logged_in_password_change( success: result.success?, - failure_reason: result.to_h[:error_details], + failure_reason: irs_attempts_api_tracker.parse_failure_reason(result), ) if result.success? diff --git a/app/controllers/users/piv_cac_authentication_setup_controller.rb b/app/controllers/users/piv_cac_authentication_setup_controller.rb index 78b92353cc9..d50d4ab2365 100644 --- a/app/controllers/users/piv_cac_authentication_setup_controller.rb +++ b/app/controllers/users/piv_cac_authentication_setup_controller.rb @@ -89,7 +89,7 @@ def process_piv_cac_setup irs_attempts_api_tracker.mfa_enroll_piv_cac( success: result.success?, subject_dn: user_piv_cac_form.x509_dn, - failure_reason: result.errors.presence, + failure_reason: irs_attempts_api_tracker.parse_failure_reason(result), ) if result.success? process_valid_submission diff --git a/app/controllers/users/piv_cac_setup_from_sign_in_controller.rb b/app/controllers/users/piv_cac_setup_from_sign_in_controller.rb index b5855e53caf..14c19fa1159 100644 --- a/app/controllers/users/piv_cac_setup_from_sign_in_controller.rb +++ b/app/controllers/users/piv_cac_setup_from_sign_in_controller.rb @@ -40,7 +40,7 @@ def process_piv_cac_setup irs_attempts_api_tracker.mfa_enroll_piv_cac( success: result.success?, subject_dn: user_piv_cac_form.x509_dn, - failure_reason: result.errors.presence, + failure_reason: irs_attempts_api_tracker.parse_failure_reason(result), ) if result.success? process_valid_submission diff --git a/app/controllers/users/reset_passwords_controller.rb b/app/controllers/users/reset_passwords_controller.rb index ee6bf002f0f..412863ef0ad 100644 --- a/app/controllers/users/reset_passwords_controller.rb +++ b/app/controllers/users/reset_passwords_controller.rb @@ -24,7 +24,7 @@ def edit analytics.password_reset_token(**result.to_h) irs_attempts_api_tracker.forgot_password_email_confirmed( success: result.success?, - failure_reason: result.errors, + failure_reason: irs_attempts_api_tracker.parse_failure_reason(result), ) if result.success? @@ -45,7 +45,7 @@ def update analytics.password_reset_password(**result.to_h) irs_attempts_api_tracker.forgot_password_new_password_submitted( success: result.success?, - failure_reason: result.errors, + failure_reason: irs_attempts_api_tracker.parse_failure_reason(result), ) if result.success? @@ -98,7 +98,7 @@ def create_account_if_email_not_found irs_attempts_api_tracker.user_registration_email_submitted( email: email, success: result.success?, - failure_reason: result.to_h[:error_details], + failure_reason: irs_attempts_api_tracker.parse_failure_reason(result), ) create_user_event(:account_created, user) end diff --git a/app/controllers/users/verify_personal_key_controller.rb b/app/controllers/users/verify_personal_key_controller.rb index 863c5258668..7eb42e04819 100644 --- a/app/controllers/users/verify_personal_key_controller.rb +++ b/app/controllers/users/verify_personal_key_controller.rb @@ -33,7 +33,7 @@ def create ) irs_attempts_api_tracker.personal_key_reactivation_submitted( success: result.success?, - failure_reason: result.errors, + failure_reason: irs_attempts_api_tracker.parse_failure_reason(result), ) if result.success? handle_success(decrypted_pii: personal_key_form.decrypted_pii) diff --git a/app/services/idv/steps/verify_base_step.rb b/app/services/idv/steps/verify_base_step.rb index 2c4fa2710de..370719ba534 100644 --- a/app/services/idv/steps/verify_base_step.rb +++ b/app/services/idv/steps/verify_base_step.rb @@ -241,7 +241,7 @@ def async_state_done(current_async_state) date_of_birth: pii_from_doc[:dob], address: pii_from_doc[:address1], ssn: pii_from_doc[:ssn], - failure_reason: form_response.errors&.presence, + failure_reason: @flow.irs_attempts_api_tracker.parse_failure_reason(form_response), ) if form_response.success? diff --git a/app/services/irs_attempts_api/tracker.rb b/app/services/irs_attempts_api/tracker.rb index 8c86c1d1de1..b0df686569e 100644 --- a/app/services/irs_attempts_api/tracker.rb +++ b/app/services/irs_attempts_api/tracker.rb @@ -60,6 +60,10 @@ def track_event(event_type, metadata = {}) end end + def parse_failure_reason(result) + return result.to_h[:error_details] || result.errors.presence + end + include TrackerEvents private diff --git a/spec/controllers/account_reset/delete_account_controller_spec.rb b/spec/controllers/account_reset/delete_account_controller_spec.rb index 4afa58afa03..8e0ca01ad78 100644 --- a/spec/controllers/account_reset/delete_account_controller_spec.rb +++ b/spec/controllers/account_reset/delete_account_controller_spec.rb @@ -3,6 +3,11 @@ describe AccountReset::DeleteAccountController do include AccountResetHelper + let(:invalid_token_message) do + t('errors.account_reset.granted_token_invalid', app_name: APP_NAME) + end + let(:invalid_token_error) { { token: [invalid_token_message] } } + describe '#delete' do it 'logs a good token to the analytics' do user = create(:user, :signed_up, :with_backup_code) @@ -41,7 +46,7 @@ expect(@irs_attempts_api_tracker).to receive(:account_reset_account_deleted).with( success: true, - failure_reason: {}, + failure_reason: nil, ) delete :delete @@ -55,10 +60,8 @@ properties = { user_id: 'anonymous-uuid', success: false, - errors: { token: [t('errors.account_reset.granted_token_invalid', app_name: APP_NAME)] }, - error_details: { - token: [t('errors.account_reset.granted_token_invalid', app_name: APP_NAME)], - }, + errors: invalid_token_error, + error_details: invalid_token_error, mfa_method_counts: {}, pii_like_keypaths: [[:mfa_method_counts, :phone]], account_age_in_days: 0, @@ -68,23 +71,16 @@ delete :delete expect(response).to redirect_to(root_url) - expect(flash[:error]).to eq( - t('errors.account_reset.granted_token_invalid', app_name: APP_NAME), - ) + expect(flash[:error]).to eq(invalid_token_message) end it 'logs an error in irs attempts tracker' do session[:granted_token] = 'foo' stub_attempts_tracker - properties = { - success: false, - failure_reason: { token: [t( - 'errors.account_reset.granted_token_invalid', - app_name: APP_NAME, - )] }, - } + expect(@irs_attempts_api_tracker).to receive(:account_reset_account_deleted).with( - properties, + success: false, + failure_reason: invalid_token_error, ) delete :delete @@ -149,10 +145,8 @@ properties = { user_id: 'anonymous-uuid', success: false, - errors: { token: [t('errors.account_reset.granted_token_invalid', app_name: APP_NAME)] }, - error_details: { - token: [t('errors.account_reset.granted_token_invalid', app_name: APP_NAME)], - }, + errors: invalid_token_error, + error_details: invalid_token_error, } expect(@analytics).to receive(:track_event). with('Account Reset: granted token validation', properties) @@ -160,9 +154,7 @@ get :show, params: { token: 'FOO' } expect(response).to redirect_to(root_url) - expect(flash[:error]).to eq( - t('errors.account_reset.granted_token_invalid', app_name: APP_NAME), - ) + expect(flash[:error]).to eq(invalid_token_message) end it 'displays a flash and redirects to root if the token is expired' do diff --git a/spec/controllers/idv/gpo_verify_controller_spec.rb b/spec/controllers/idv/gpo_verify_controller_spec.rb index 65d49284b39..42d8062205c 100644 --- a/spec/controllers/idv/gpo_verify_controller_spec.rb +++ b/spec/controllers/idv/gpo_verify_controller_spec.rb @@ -85,6 +85,9 @@ end describe '#create' do + let(:otp_code_incorrect) { { otp: [:confirmation_code_incorrect] } } + let(:success_properties) { { success: true, failure_reason: nil } } + subject(:action) do post( :create, @@ -109,7 +112,7 @@ pii_like_keypaths: [[:errors, :otp], [:error_details, :otp]], ) expect(@irs_attempts_api_tracker).to receive(:idv_gpo_verification_submitted). - with(success: true, failure_reason: nil) + with(success_properties) action @@ -146,7 +149,7 @@ pii_like_keypaths: [[:errors, :otp], [:error_details, :otp]], ) expect(@irs_attempts_api_tracker).to receive(:idv_gpo_verification_submitted). - with(success: true, failure_reason: nil) + with(success_properties) action @@ -171,12 +174,11 @@ errors: { otp: [t('errors.messages.confirmation_code_incorrect')] }, pending_in_person_enrollment: false, enqueued_at: nil, - error_details: { otp: [:confirmation_code_incorrect] }, + error_details: otp_code_incorrect, pii_like_keypaths: [[:errors, :otp], [:error_details, :otp]], ) - failure_reason = { otp: ['Incorrect code. Did you type it in correctly?'] } expect(@irs_attempts_api_tracker).to receive(:idv_gpo_verification_submitted). - with(success: false, failure_reason: failure_reason) + with(success: false, failure_reason: otp_code_incorrect) action @@ -202,7 +204,7 @@ errors: { otp: [t('errors.messages.confirmation_code_incorrect')] }, pending_in_person_enrollment: false, enqueued_at: nil, - error_details: { otp: [:confirmation_code_incorrect] }, + error_details: otp_code_incorrect, pii_like_keypaths: [[:errors, :otp], [:error_details, :otp]], ).exactly(max_attempts).times diff --git a/spec/controllers/idv/phone_controller_spec.rb b/spec/controllers/idv/phone_controller_spec.rb index e54240baf9c..2f0510d47b1 100644 --- a/spec/controllers/idv/phone_controller_spec.rb +++ b/spec/controllers/idv/phone_controller_spec.rb @@ -131,42 +131,49 @@ describe '#create' do context 'when form is invalid' do + let(:improbable_phone_error) { { phone: [:improbable_phone] } } + let(:improbable_phone_message) { t('errors.messages.improbable_phone') } + let(:improbable_phone_number) { '703' } + let(:improbable_phone_form) { { idv_phone_form: { phone: improbable_phone_number } } } before do user = build(:user, :with_phone, with: { phone: '+1 (415) 555-0130' }) stub_verify_steps_one_and_two(user) stub_analytics stub_attempts_tracker allow(@analytics).to receive(:track_event) - allow(@irs_attempts_api_tracker).to receive(:track_event) end it 'renders #new' do - put :create, params: { idv_phone_form: { phone: '703' } } + put :create, params: improbable_phone_form - expect(flash[:error]).to eq t('errors.messages.improbable_phone') + expect(flash[:error]).to eq improbable_phone_message expect(response).to render_template(:new) end it 'disallows non-US numbers' do put :create, params: { idv_phone_form: { phone: international_phone } } - expect(flash[:error]).to eq t('errors.messages.improbable_phone') + expect(flash[:error]).to eq improbable_phone_message expect(response).to render_template(:new) end - it 'tracks form error and does not make a vendor API call' do + it 'tracks form error events and does not make a vendor API call' do expect_any_instance_of(Idv::Agent).to_not receive(:proof_address) - put :create, params: { idv_phone_form: { phone: '703' } } + expect(@irs_attempts_api_tracker).to receive(:idv_phone_submitted).with( + success: false, + phone_number: improbable_phone_number, + failure_reason: improbable_phone_error, + ) + + put :create, params: improbable_phone_form result = { success: false, errors: { - phone: [t('errors.messages.improbable_phone')], - }, - error_details: { - phone: [:improbable_phone], + phone: [improbable_phone_message], }, + error_details: improbable_phone_error, pii_like_keypaths: [[:errors, :phone], [:error_details, :phone]], country_code: nil, area_code: nil, @@ -180,19 +187,6 @@ ) expect(subject.idv_session.vendor_phone_confirmation).to be_falsy end - - it 'tracks irs event idv_phone_submitted' do - put :create, params: { idv_phone_form: { phone: '703' } } - - expect(@irs_attempts_api_tracker).to have_received(:track_event).with( - :idv_phone_submitted, - success: false, - phone_number: '703', - failure_reason: { - phone: [t('errors.messages.improbable_phone')], - }, - ) - end end context 'when form is valid' do @@ -200,13 +194,18 @@ stub_analytics stub_attempts_tracker allow(@analytics).to receive(:track_event) - allow(@irs_attempts_api_tracker).to receive(:track_event) end it 'tracks events with valid phone' do user = build(:user, :with_phone, with: { phone: good_phone, confirmed_at: Time.zone.now }) stub_verify_steps_one_and_two(user) + expect(@irs_attempts_api_tracker).to receive(:idv_phone_submitted).with( + success: true, + phone_number: good_phone, + failure_reason: nil, + ) + put :create, params: { idv_phone_form: { phone: good_phone } } result = { @@ -223,12 +222,6 @@ expect(@analytics).to have_received(:track_event).with( 'IdV: phone confirmation form', result ) - expect(@irs_attempts_api_tracker).to have_received(:track_event).with( - :idv_phone_submitted, - success: true, - phone_number: good_phone, - failure_reason: {}, - ) end context 'when same as user phone' do diff --git a/spec/controllers/sign_up/passwords_controller_spec.rb b/spec/controllers/sign_up/passwords_controller_spec.rb index 4fa5333069e..35efcd64623 100644 --- a/spec/controllers/sign_up/passwords_controller_spec.rb +++ b/spec/controllers/sign_up/passwords_controller_spec.rb @@ -2,6 +2,7 @@ describe SignUp::PasswordsController do describe '#create' do + let(:success_properties) { { success: true, failure_reason: nil } } it 'tracks a valid password event' do token = 'new token' user = create(:user, :unconfirmed, confirmation_token: token) @@ -9,8 +10,6 @@ stub_analytics stub_attempts_tracker - allow(@irs_attempts_api_tracker).to receive(:track_event) - analytics_hash = { success: true, errors: {}, @@ -26,6 +25,15 @@ expect(@analytics).to receive(:track_event). with('Password Creation', analytics_hash) + expect(@irs_attempts_api_tracker).to receive(:user_registration_password_submitted).with( + success_properties, + ) + + expect(@irs_attempts_api_tracker).to receive(:user_registration_email_confirmation).with( + email: user.email_addresses.first.email, + **success_properties, + ) + post :create, params: { password_form: { password: 'NewVal!dPassw0rd' }, confirmation_token: token, @@ -33,17 +41,6 @@ user.reload - expect(@irs_attempts_api_tracker).to have_received(:track_event).with( - :user_registration_password_submitted, - success: true, - failure_reason: {}, - ) - expect(@irs_attempts_api_tracker).to have_received(:track_event).with( - :user_registration_email_confirmation, - email: user.email_addresses.first.email, - success: true, - failure_reason: nil, - ) expect(user.valid_password?('NewVal!dPassw0rd')).to eq true expect(user.confirmed?).to eq true end @@ -75,23 +72,20 @@ end it 'tracks an invalid password event' do + password_short_error = { password: [:too_short] } token = 'new token' user = create(:user, :unconfirmed, confirmation_token: token) stub_analytics stub_attempts_tracker - allow(@irs_attempts_api_tracker).to receive(:track_event) - analytics_hash = { success: false, errors: { password: ["This password is too short (minimum is #{Devise.password_length.first} characters)"], }, - error_details: { - password: [:too_short], - }, + error_details: password_short_error, user_id: user.uuid, request_id_present: false, } @@ -104,19 +98,16 @@ expect(@analytics).to receive(:track_event). with('Password Creation', analytics_hash) - post :create, params: { password_form: { password: 'NewVal' }, confirmation_token: token } - - expect(@irs_attempts_api_tracker).to have_received(:track_event).with( - :user_registration_password_submitted, + expect(@irs_attempts_api_tracker).to receive(:user_registration_password_submitted).with( success: false, - failure_reason: { password: ['This password is too short (minimum is 12 characters)'] }, + failure_reason: password_short_error, ) - expect(@irs_attempts_api_tracker).to have_received(:track_event).with( - :user_registration_email_confirmation, + expect(@irs_attempts_api_tracker).to receive(:user_registration_email_confirmation).with( email: user.email_addresses.first.email, - success: true, - failure_reason: nil, + **success_properties, ) + + post :create, params: { password_form: { password: 'NewVal' }, confirmation_token: token } end end diff --git a/spec/controllers/sign_up/registrations_controller_spec.rb b/spec/controllers/sign_up/registrations_controller_spec.rb index e47dbe4c46a..d24ee781dd7 100644 --- a/spec/controllers/sign_up/registrations_controller_spec.rb +++ b/spec/controllers/sign_up/registrations_controller_spec.rb @@ -30,15 +30,20 @@ end describe '#create' do + let(:success_properties) { { success: true, failure_reason: nil } } context 'when registering with a new email' do it 'tracks successful user registration' do stub_analytics stub_attempts_tracker allow(@analytics).to receive(:track_event) - allow(@irs_attempts_api_tracker).to receive(:track_event) allow(subject).to receive(:create_user_event) + expect(@irs_attempts_api_tracker).to receive(:user_registration_email_submitted).with( + email: 'new@example.com', + **success_properties, + ) + post :create, params: { user: { email: 'new@example.com', terms_accepted: '1' } } user = User.find_with_email('new@example.com') @@ -55,13 +60,6 @@ expect(@analytics).to have_received(:track_event). with('User Registration: Email Submitted', analytics_hash) - expect(@irs_attempts_api_tracker).to have_received(:track_event).with( - :user_registration_email_submitted, - email: 'new@example.com', - success: true, - failure_reason: nil, - ) - expect(subject).to have_received(:create_user_event).with(:account_created, user) end @@ -111,11 +109,9 @@ expect(@analytics).to receive(:track_event). with('User Registration: Email Submitted', analytics_hash) - expect(@irs_attempts_api_tracker).to receive(:track_event).with( - :user_registration_email_submitted, + expect(@irs_attempts_api_tracker).to receive(:user_registration_email_submitted).with( email: 'TEST@example.com ', - success: true, - failure_reason: nil, + **success_properties, ) expect(subject).to_not receive(:create_user_event) diff --git a/spec/controllers/two_factor_authentication/piv_cac_verification_controller_spec.rb b/spec/controllers/two_factor_authentication/piv_cac_verification_controller_spec.rb index ca9737f3e6a..c8b7c6f9226 100644 --- a/spec/controllers/two_factor_authentication/piv_cac_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/piv_cac_verification_controller_spec.rb @@ -14,6 +14,7 @@ let(:x509_issuer) do '/C=US/O=Entrust/OU=Certification Authorities/OU=Entrust Managed Services SSP CA' end + let(:bad_dn) { 'bad-dn' } before(:each) do session_info = { piv_cac_nonce: nonce } @@ -32,7 +33,7 @@ ) allow(PivCacService).to receive(:decode_token).with('bad-token').and_return( 'uuid' => 'bad-uuid', - 'subject' => 'bad-dn', + 'subject' => bad_dn, 'issuer' => x509_issuer, 'nonce' => nonce, ) @@ -111,8 +112,7 @@ expect(@analytics).to receive(:track_mfa_submit_event). with(submit_attributes) - expect(@irs_attempts_api_tracker).to receive(:track_event).with( - :mfa_login_piv_cac, + expect(@irs_attempts_api_tracker).to receive(:mfa_login_piv_cac).with( success: true, subject_dn: x509_subject, failure_reason: nil, @@ -195,9 +195,11 @@ expect(@irs_attempts_api_tracker).to receive(:mfa_login_rate_limited). with(mfa_device_type: 'piv_cac') + piv_cac_mismatch = { type: 'user.piv_cac_mismatch' } + submit_attributes = { success: false, - errors: { type: 'user.piv_cac_mismatch' }, + errors: piv_cac_mismatch, context: 'authentication', multi_factor_auth_method: 'piv_cac', key_id: nil, @@ -206,11 +208,10 @@ expect(@analytics).to receive(:track_mfa_submit_event). with(submit_attributes) - expect(@irs_attempts_api_tracker).to receive(:track_event).with( - :mfa_login_piv_cac, + expect(@irs_attempts_api_tracker).to receive(:mfa_login_piv_cac).with( success: false, - subject_dn: 'bad-dn', - failure_reason: { type: 'user.piv_cac_mismatch' }, + subject_dn: bad_dn, + failure_reason: piv_cac_mismatch, ) expect(@analytics).to receive(:track_event). diff --git a/spec/controllers/users/passwords_controller_spec.rb b/spec/controllers/users/passwords_controller_spec.rb index 0cca150a5de..4dde04e5cfa 100644 --- a/spec/controllers/users/passwords_controller_spec.rb +++ b/spec/controllers/users/passwords_controller_spec.rb @@ -10,15 +10,15 @@ stub_analytics stub_attempts_tracker allow(@analytics).to receive(:track_event) - allow(@irs_attempts_api_tracker).to receive(:track_event) + + expect(@irs_attempts_api_tracker).to receive(:logged_in_password_change). + with(failure_reason: nil, success: true) params = { password: 'salty new password' } patch :update, params: { update_user_password_form: params } expect(@analytics).to have_received(:track_event). with('Password Changed', success: true, errors: {}) - expect(@irs_attempts_api_tracker).to have_received(:track_event). - with(:logged_in_password_change, failure_reason: nil, success: true) expect(response).to redirect_to account_url expect(flash[:info]).to eq t('notices.password_changed') expect(flash[:personal_key]).to be_nil @@ -78,12 +78,17 @@ context 'form returns failure' do it 'renders edit' do + password_short_error = { password: [:too_short] } stub_sign_in stub_analytics stub_attempts_tracker allow(@analytics).to receive(:track_event) - allow(@irs_attempts_api_tracker).to receive(:track_event) + + expect(@irs_attempts_api_tracker).to receive(:logged_in_password_change).with( + success: false, + failure_reason: password_short_error, + ) params = { password: 'new' } patch :update, params: { update_user_password_form: params } @@ -96,14 +101,7 @@ t('errors.attributes.password.too_short.other', count: Devise.password_length.first), ], }, - error_details: { password: [:too_short] }, - ) - expect(@irs_attempts_api_tracker).to have_received(:track_event).with( - :logged_in_password_change, - success: false, - failure_reason: { - password: [:too_short], - }, + error_details: password_short_error, ) expect(response).to render_template(:edit) end diff --git a/spec/controllers/users/reset_passwords_controller_spec.rb b/spec/controllers/users/reset_passwords_controller_spec.rb index bd56e822017..ec678220a08 100644 --- a/spec/controllers/users/reset_passwords_controller_spec.rb +++ b/spec/controllers/users/reset_passwords_controller_spec.rb @@ -4,46 +4,48 @@ let(:password_error_message) do "This password is too short (minimum is #{Devise.password_length.first} characters)" end + let(:success_properties) { { success: true, failure_reason: nil } } + let(:token_expired_error) { 'token_expired' } describe '#edit' do before do stub_analytics stub_attempts_tracker allow(@analytics).to receive(:track_event) - allow(@irs_attempts_api_tracker).to receive(:track_event) end context 'no user matches token' do + let(:user_blank_error) { { user: [:blank] } } let(:analytics_hash) do { success: false, errors: { user: ['invalid_token'] }, - error_details: { user: [:blank] }, + error_details: user_blank_error, user_id: nil, } end it 'redirects to page where user enters email for password reset token' do + expect(@irs_attempts_api_tracker).to receive(:forgot_password_email_confirmed).with( + success: false, + failure_reason: user_blank_error, + ) + get :edit, params: { reset_password_token: 'foo' } expect(@analytics).to have_received(:track_event). with('Password Reset: Token Submitted', analytics_hash) - expect(@irs_attempts_api_tracker).to have_received(:track_event).with( - :forgot_password_email_confirmed, - success: false, - failure_reason: { user: ['invalid_token'] }, - ) - expect(response).to redirect_to new_user_password_path expect(flash[:error]).to eq t('devise.passwords.invalid_token') end end context 'token expired' do + let(:user_token_error) { { user: [token_expired_error] } } let(:analytics_hash) do { success: false, - errors: { user: ['token_expired'] }, - error_details: { user: ['token_expired'] }, + errors: user_token_error, + error_details: user_token_error, user_id: '123', } end @@ -55,15 +57,15 @@ end it 'redirects to page where user enters email for password reset token' do + expect(@irs_attempts_api_tracker).to receive(:forgot_password_email_confirmed).with( + success: false, + failure_reason: user_token_error, + ) + get :edit, params: { reset_password_token: 'foo' } expect(@analytics).to have_received(:track_event). with('Password Reset: Token Submitted', analytics_hash) - expect(@irs_attempts_api_tracker).to have_received(:track_event).with( - :forgot_password_email_confirmed, - success: false, - failure_reason: { user: ['token_expired'] }, - ) expect(response).to redirect_to new_user_password_path expect(flash[:error]).to eq t('devise.passwords.token_expired') end @@ -88,26 +90,27 @@ allow(ForbiddenPasswords).to receive(:new).with(email_address.email).and_return(forbidden) expect(forbidden).to receive(:call) + expect(@irs_attempts_api_tracker).to receive(:forgot_password_email_confirmed).with( + success_properties, + ) + get :edit, params: { reset_password_token: 'foo' } expect(response).to render_template :edit expect(flash.keys).to be_empty expect(response.body).to match('') - expect(@irs_attempts_api_tracker).to have_received(:track_event).with( - :forgot_password_email_confirmed, - success: true, - failure_reason: {}, - ) end end end describe '#update' do + let(:password_short_error) { { password: [:too_short] } } + let(:password_token_error) { { reset_password_token: [token_expired_error] } } context 'user submits new password after token expires' do - let(:irs_tracker_failure_reason) do + let(:reset_password_error_details) do { - password: [password_error_message], - reset_password_token: ['token_expired'], + **password_short_error, + **password_token_error, } end @@ -115,7 +118,11 @@ stub_analytics stub_attempts_tracker allow(@analytics).to receive(:track_event) - allow(@irs_attempts_api_tracker).to receive(:track_event) + + expect(@irs_attempts_api_tracker).to receive(:forgot_password_new_password_submitted).with( + success: false, + failure_reason: reset_password_error_details, + ) raw_reset_token, db_confirmation_token = Devise.token_generator.generate(User, :reset_password_token) @@ -135,39 +142,24 @@ success: false, errors: { password: [password_error_message], - reset_password_token: ['token_expired'], - }, - error_details: { - password: [:too_short], - reset_password_token: ['token_expired'], + **password_token_error, }, + error_details: reset_password_error_details, user_id: user.uuid, profile_deactivated: false, } expect(@analytics).to have_received(:track_event). with('Password Reset: Password Submitted', analytics_hash) - - expect(@irs_attempts_api_tracker).to have_received(:track_event).with( - :forgot_password_new_password_submitted, - success: false, - failure_reason: irs_tracker_failure_reason, - ) - expect(response).to redirect_to new_user_password_path expect(flash[:error]).to eq t('devise.passwords.token_expired') end end context 'user submits invalid new password' do - let(:irs_tracker_failure_reason) do - { password: [password_error_message] } - end - it 'renders edit' do stub_analytics stub_attempts_tracker - allow(@irs_attempts_api_tracker).to receive(:track_event) raw_reset_token, db_confirmation_token = Devise.token_generator.generate(User, :reset_password_token) @@ -183,25 +175,22 @@ errors: { password: [password_error_message], }, - error_details: { - password: [:too_short], - }, + error_details: password_short_error, user_id: user.uuid, profile_deactivated: false, } expect(@analytics).to receive(:track_event). with('Password Reset: Password Submitted', analytics_hash) + expect(@irs_attempts_api_tracker).to receive(:forgot_password_new_password_submitted).with( + success: false, + failure_reason: password_short_error, + ) put :update, params: { reset_password_form: form_params } expect(assigns(:forbidden_passwords)).to all(be_a(String)) expect(response).to render_template(:edit) - expect(@irs_attempts_api_tracker).to have_received(:track_event).with( - :forgot_password_new_password_submitted, - success: false, - failure_reason: irs_tracker_failure_reason, - ) end end @@ -230,7 +219,6 @@ stub_analytics stub_attempts_tracker allow(@analytics).to receive(:track_event) - allow(@irs_attempts_api_tracker).to receive(:track_event) raw_reset_token, db_confirmation_token = Devise.token_generator.generate(User, :reset_password_token) @@ -250,6 +238,10 @@ stub_user_mailer(user) + expect(@irs_attempts_api_tracker).to receive( + :forgot_password_new_password_submitted, + ).with(success_properties) + password = 'a really long passw0rd' params = { password: password, reset_password_token: raw_reset_token } @@ -265,11 +257,6 @@ expect(@analytics).to have_received(:track_event). with('Password Reset: Password Submitted', analytics_hash) - expect(@irs_attempts_api_tracker).to have_received(:track_event).with( - :forgot_password_new_password_submitted, - success: true, - failure_reason: {}, - ) expect(user.events.password_changed.size).to be 1 expect(response).to redirect_to new_user_session_path @@ -284,7 +271,6 @@ stub_analytics stub_attempts_tracker allow(@analytics).to receive(:track_event) - allow(@irs_attempts_api_tracker).to receive(:track_event) raw_reset_token, db_confirmation_token = Devise.token_generator.generate(User, :reset_password_token) @@ -300,6 +286,10 @@ stub_user_mailer(user) + expect(@irs_attempts_api_tracker).to receive(:forgot_password_new_password_submitted).with( + success_properties, + ) + get :edit, params: { reset_password_token: raw_reset_token } password = 'a really long passw0rd' params = { password: password, reset_password_token: raw_reset_token } @@ -315,14 +305,7 @@ expect(@analytics).to have_received(:track_event). with('Password Reset: Password Submitted', analytics_hash) - expect(@irs_attempts_api_tracker).to have_received(:track_event).with( - :forgot_password_new_password_submitted, - success: true, - failure_reason: {}, - ) - expect(user.active_profile.present?).to eq false - expect(response).to redirect_to new_user_session_path end end @@ -332,7 +315,6 @@ stub_analytics stub_attempts_tracker allow(@analytics).to receive(:track_event) - allow(@irs_attempts_api_tracker).to receive(:track_event) raw_reset_token, db_confirmation_token = Devise.token_generator.generate(User, :reset_password_token) @@ -349,6 +331,10 @@ stub_user_mailer(user) + expect(@irs_attempts_api_tracker).to receive(:forgot_password_new_password_submitted).with( + success_properties, + ) + password = 'a really long passw0rd' params = { password: password, reset_password_token: raw_reset_token } @@ -364,14 +350,7 @@ expect(@analytics).to have_received(:track_event). with('Password Reset: Password Submitted', analytics_hash) - expect(@irs_attempts_api_tracker).to have_received(:track_event).with( - :forgot_password_new_password_submitted, - success: true, - failure_reason: {}, - ) - expect(user.reload.confirmed?).to eq true - expect(response).to redirect_to new_user_session_path end end @@ -384,7 +363,11 @@ it 'send an email to tell the user they do not have an account yet' do stub_analytics stub_attempts_tracker - allow(@irs_attempts_api_tracker).to receive(:track_event) + + expect(@irs_attempts_api_tracker).to receive(:user_registration_email_submitted).with( + email: email, + **success_properties, + ) expect do put :create, params: { @@ -414,13 +397,6 @@ 'User Registration: Email Submitted', analytics_hash, ) - expect(@irs_attempts_api_tracker).to have_received(:track_event).with( - :user_registration_email_submitted, - email: email, - success: true, - failure_reason: nil, - ) - expect(response).to redirect_to forgot_password_path end end @@ -442,23 +418,20 @@ stub_analytics stub_attempts_tracker allow(@analytics).to receive(:track_event) - allow(@irs_attempts_api_tracker).to receive(:track_event) end it 'sends password reset email to user and tracks event' do + expect(@irs_attempts_api_tracker).to receive(:forgot_password_email_sent).with( + email: email, + success: true, + ) + expect do put :create, params: { password_reset_email_form: { email: email } } end.to change { ActionMailer::Base.deliveries.count }.by(1) expect(@analytics).to have_received(:track_event). with('Password Reset: Email Submitted', analytics_hash) - - expect(@irs_attempts_api_tracker).to have_received(:track_event).with( - :forgot_password_email_sent, - email: email, - success: true, - ) - expect(response).to redirect_to forgot_password_path end end @@ -486,10 +459,14 @@ stub_analytics stub_attempts_tracker allow(@analytics).to receive(:track_event) - allow(@irs_attempts_api_tracker).to receive(:track_event) end it 'sends password reset email to user and tracks event' do + expect(@irs_attempts_api_tracker).to receive(:forgot_password_email_sent).with( + email: user.email, + success: true, + ) + expect { put :create, params: params }. to change { ActionMailer::Base.deliveries.count }.by(1) @@ -498,13 +475,6 @@ expect(ActionMailer::Base.deliveries.last.subject). to eq t('user_mailer.reset_password_instructions.subject') - - expect(@irs_attempts_api_tracker).to have_received(:track_event).with( - :forgot_password_email_sent, - email: user.email, - success: true, - ) - expect(response).to redirect_to forgot_password_path end end @@ -513,7 +483,6 @@ it 'captures in analytics that the user was verified' do stub_analytics stub_attempts_tracker - allow(@irs_attempts_api_tracker).to receive(:track_event) user = create(:user, :signed_up) create(:profile, :active, :verified, user: user) @@ -528,15 +497,13 @@ expect(@analytics).to receive(:track_event). with('Password Reset: Email Submitted', analytics_hash) - - params = { password_reset_email_form: { email: user.email } } - put :create, params: params - - expect(@irs_attempts_api_tracker).to have_received(:track_event).with( - :forgot_password_email_sent, + expect(@irs_attempts_api_tracker).to receive(:forgot_password_email_sent).with( email: user.email, success: true, ) + + params = { password_reset_email_form: { email: user.email } } + put :create, params: params end end diff --git a/spec/controllers/users/verify_personal_key_controller_spec.rb b/spec/controllers/users/verify_personal_key_controller_spec.rb index 5425f57a323..a24a2b4cb1a 100644 --- a/spec/controllers/users/verify_personal_key_controller_spec.rb +++ b/spec/controllers/users/verify_personal_key_controller_spec.rb @@ -82,6 +82,7 @@ end let(:error_text) { 'Incorrect personal key' } let(:personal_key_error) { { personal_key: [error_text] } } + let(:failure_properties) { { success: false, failure_reason: personal_key_error } } let(:response_ok) { FormResponse.new(success: true, errors: {}) } let(:response_bad) { FormResponse.new(success: false, errors: personal_key_error, extra: {}) } @@ -113,9 +114,8 @@ it 'tracks irs attempts api for relevant users' do stub_attempts_tracker - expect(@irs_attempts_api_tracker).to receive(:track_event).with( - :personal_key_reactivation_submitted, - failure_reason: {}, + expect(@irs_attempts_api_tracker).to receive(:personal_key_reactivation_submitted).with( + failure_reason: nil, success: true, ).once @@ -142,10 +142,8 @@ it 'tracks irs attempts api for relevant users' do stub_attempts_tracker - expect(@irs_attempts_api_tracker).to receive(:track_event).with( - :personal_key_reactivation_submitted, - failure_reason: personal_key_error, - success: false, + expect(@irs_attempts_api_tracker).to receive(:personal_key_reactivation_submitted).with( + failure_properties, ).once allow_any_instance_of(VerifyPersonalKeyForm).to receive(:submit).and_return(response_bad) @@ -180,10 +178,8 @@ it 'tracks irs attempts api for relevant users' do stub_attempts_tracker - expect(@irs_attempts_api_tracker).to receive(:track_event).with( - :personal_key_reactivation_submitted, - failure_reason: personal_key_error, - success: false, + expect(@irs_attempts_api_tracker).to receive(:personal_key_reactivation_submitted).with( + failure_properties, ).once allow_any_instance_of(VerifyPersonalKeyForm).to receive(:submit).and_return(response_bad) diff --git a/spec/forms/idv/api_image_upload_form_spec.rb b/spec/forms/idv/api_image_upload_form_spec.rb index ac1de432b5f..e09b02bdfe9 100644 --- a/spec/forms/idv/api_image_upload_form_spec.rb +++ b/spec/forms/idv/api_image_upload_form_spec.rb @@ -93,7 +93,7 @@ end it 'is not valid' do - expect(irs_attempts_api_tracker).to receive(:idv_document_upload_rate_limited) + expect(irs_attempts_api_tracker).to receive(:idv_document_upload_rate_limited).with(no_args) expect(form.valid?).to eq(false) expect(form.errors[:limit]).to eq([I18n.t('errors.doc_auth.throttled_heading')]) end diff --git a/spec/services/irs_attempts_api/tracker_spec.rb b/spec/services/irs_attempts_api/tracker_spec.rb index 19ac7deb4e9..f7ba6492e1a 100644 --- a/spec/services/irs_attempts_api/tracker_spec.rb +++ b/spec/services/irs_attempts_api/tracker_spec.rb @@ -156,4 +156,34 @@ end end end + + describe '#parse_failure_reason' do + let(:mock_error_message) { 'failure_reason_from_error' } + let(:mock_error_details) { [{ mock_error: 'failure_reason_from_error_details' }] } + + it 'parses failure_reason from error_details' do + test_failure_reason = subject.parse_failure_reason( + { errors: mock_error_message, + error_details: mock_error_details }, + ) + + expect(test_failure_reason).to eq(mock_error_details) + end + + it 'parses failure_reason from errors when no error_details present' do + class MockFailureReason + def errors + 'failure_reason_from_error' + end + + def to_h + {} + end + end + + test_failure_reason = subject.parse_failure_reason(MockFailureReason.new) + + expect(test_failure_reason).to eq(mock_error_message) + end + end end diff --git a/spec/support/fake_attempts_tracker.rb b/spec/support/fake_attempts_tracker.rb index ae561bcf645..fa0add2a3cc 100644 --- a/spec/support/fake_attempts_tracker.rb +++ b/spec/support/fake_attempts_tracker.rb @@ -14,6 +14,10 @@ def track_event(event, attributes = {}) nil end + def parse_failure_reason(result) + return result.to_h[:error_details] || result.errors.presence + end + def track_mfa_submit_event(_attributes) # no-op end From 7d0115f113148cfec3327c77ea441ffe820608f2 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Mon, 26 Sep 2022 07:34:49 -0700 Subject: [PATCH 16/23] Update add_email to build its URL once and re-use it (#7021) - Follow-up to #7019 to prevent similar bugs in the future [skip changelog] --- app/mailers/user_mailer.rb | 6 ++++-- app/views/user_mailer/add_email.html.erb | 14 +++----------- spec/mailers/user_mailer_spec.rb | 15 +++++++++++++++ 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 6e0ef65b0a7..9762a97430d 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -173,8 +173,10 @@ def add_email(user, email, token) presenter = ConfirmationEmailPresenter.new(user, view_context) @first_sentence = presenter.first_sentence @confirmation_period = presenter.confirmation_period - @locale = locale_url_param - @token = token + @add_email_url = add_email_confirmation_url( + confirmation_token: token, + locale: locale_url_param, + ) mail(to: email, subject: t('user_mailer.add_email.subject')) end end diff --git a/app/views/user_mailer/add_email.html.erb b/app/views/user_mailer/add_email.html.erb index 6f125d4e35a..6b31c0f2fab 100644 --- a/app/views/user_mailer/add_email.html.erb +++ b/app/views/user_mailer/add_email.html.erb @@ -13,11 +13,7 @@
<%= link_to t('user_mailer.email_confirmation_instructions.link_text'), - add_email_confirmation_url( - _request_id: @request_id, - confirmation_token: @token, - locale: @locale, - ), + @add_email_url, target: '_blank', class: 'float-center', align: 'center', @@ -35,12 +31,8 @@

- <%= link_to add_email_confirmation_url(_request_id: @request_id, confirmation_token: @token, locale: @locale), - add_email_confirmation_url( - _request_id: @request_id, - confirmation_token: @token, - locale: @locale, - ), + <%= link_to @add_email_url, + @add_email_url, target: '_blank', rel: 'noopener' %>

diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index 4d4d69c3341..d3053408dae 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -5,6 +5,21 @@ let(:email_address) { user.email_addresses.first } let(:banned_email) { 'banned_email+123abc@gmail.com' } + describe '#add_email' do + let(:token) { SecureRandom.hex } + let(:mail) { UserMailer.add_email(user, email_address, token) } + + it_behaves_like 'a system email' + it_behaves_like 'an email that respects user email locale preference' + + it 'renders the add_email_confirmation_url' do + add_email_url = add_email_confirmation_url(confirmation_token: token) + + expect(mail.html_part.body).to have_content(add_email_url) + expect(mail.html_part.body).to_not have_content(sign_up_create_email_confirmation_url) + end + end + describe '#email_deleted' do let(:mail) { UserMailer.email_deleted(user, 'old@email.com') } From 41914fb600e30c1232b859a0486b9460fa89c8d0 Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Mon, 26 Sep 2022 10:00:45 -0500 Subject: [PATCH 17/23] Remove isConnected check in phone input JavaScript (#7026) [skip changelog] --- app/javascript/packages/phone-input/index.js | 48 ++++++++++---------- 1 file changed, 23 insertions(+), 25 deletions(-) diff --git a/app/javascript/packages/phone-input/index.js b/app/javascript/packages/phone-input/index.js index 7fee5ce34ae..77ecbe22d5f 100644 --- a/app/javascript/packages/phone-input/index.js +++ b/app/javascript/packages/phone-input/index.js @@ -45,34 +45,32 @@ export class PhoneInput extends HTMLElement { countryCodePairs = {}; connectedCallback() { - if (this.isConnected) { - /** @type {HTMLInputElement?} */ - this.textInput = this.querySelector('.phone-input__number'); - /** @type {HTMLSelectElement?} */ - this.codeInput = this.querySelector('.phone-input__international-code'); - this.codeWrapper = this.querySelector('.phone-input__international-code-wrapper'); - this.exampleText = this.querySelector('.phone-input__example'); - - try { - this.deliveryMethods = JSON.parse(this.dataset.deliveryMethods || ''); - this.countryCodePairs = JSON.parse(this.dataset.translatedCountryCodeNames || ''); - } catch {} - - if (!this.textInput || !this.codeInput) { - return; - } + /** @type {HTMLInputElement?} */ + this.textInput = this.querySelector('.phone-input__number'); + /** @type {HTMLSelectElement?} */ + this.codeInput = this.querySelector('.phone-input__international-code'); + this.codeWrapper = this.querySelector('.phone-input__international-code-wrapper'); + this.exampleText = this.querySelector('.phone-input__example'); + + try { + this.deliveryMethods = JSON.parse(this.dataset.deliveryMethods || ''); + this.countryCodePairs = JSON.parse(this.dataset.translatedCountryCodeNames || ''); + } catch {} + + if (!this.textInput || !this.codeInput) { + return; + } - this.iti = this.initializeIntlTelInput(); + this.iti = this.initializeIntlTelInput(); - this.textInput.addEventListener('countrychange', () => this.syncCountryChangeToCodeInput()); - this.textInput.addEventListener('input', () => this.validate()); - this.codeInput.addEventListener('change', () => this.formatTextInput()); - this.codeInput.addEventListener('change', () => this.setExampleNumber()); - this.codeInput.addEventListener('change', () => this.validate()); + this.textInput.addEventListener('countrychange', () => this.syncCountryChangeToCodeInput()); + this.textInput.addEventListener('input', () => this.validate()); + this.codeInput.addEventListener('change', () => this.formatTextInput()); + this.codeInput.addEventListener('change', () => this.setExampleNumber()); + this.codeInput.addEventListener('change', () => this.validate()); - this.setExampleNumber(); - this.validate(); - } + this.setExampleNumber(); + this.validate(); } get selectedOption() { From 5915efda64210d4cf32fb18d8f6a8744fa8454bc Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Mon, 26 Sep 2022 11:09:46 -0400 Subject: [PATCH 18/23] Remove feature specs related to FSMv2 (#7014) * Remove feature specs related to FSMv2 **Why**: The code for this feature is slated for removal (LG-7386). Until that can be prioritized, it'd be useful to remove feature specs since they prolong the runtime of continuous integration builds * Add changelog changelog: Internal, Automated Testing, Improve performance of automated tests --- knapsack_rspec_report.json | 10 +- spec/features/idv/analytics_spec.rb | 105 +++++++----------- .../idv/steps/confirmation_step_spec.rb | 32 ------ .../idv/steps/forgot_password_step_spec.rb | 41 ------- .../idv/steps/in_person/verify_step_spec.rb | 5 - spec/features/idv/steps/review_step_spec.rb | 34 ------ 6 files changed, 43 insertions(+), 184 deletions(-) diff --git a/knapsack_rspec_report.json b/knapsack_rspec_report.json index 6bbefd63972..54e6d980473 100644 --- a/knapsack_rspec_report.json +++ b/knapsack_rspec_report.json @@ -193,7 +193,7 @@ "spec/lib/analytics_events_documenter_spec.rb": 0.1543309999979101, "spec/services/proofing/aamva/response/verification_response_spec.rb": 0.1954249999980675, "spec/services/agreements/reports/agency_partner_accounts_report_spec.rb": 0.03952299999946263, - "spec/features/idv/analytics_spec.rb": 21.609241000005568, + "spec/features/idv/analytics_spec.rb": 38.083291000002646, "spec/presenters/two_factor_authentication/personal_key_selection_presenter_spec.rb": 0.004340999999840278, "spec/controllers/health/health_controller_spec.rb": 0.019761000003200024, "spec/features/idv/doc_capture/document_capture_step_spec.rb": 110.0928899999999, @@ -345,7 +345,7 @@ "spec/services/personal_key_generator_spec.rb": 0.3026769999996759, "spec/views/two_factor_authentication/sms_opt_in/new.html.erb_spec.rb": 0.06462499999906868, "spec/views/devise/shared/_password_strength.html.erb_spec.rb": 0.0479399999967427, - "spec/features/idv/steps/confirmation_step_spec.rb": 128.83288399999583, + "spec/features/idv/steps/confirmation_step_spec.rb": 155.54028299999482, "spec/services/db/deleted_user/create_spec.rb": 0.07442100000480423, "spec/controllers/test/telephony_controller_spec.rb": 0.025112000002991408, "spec/controllers/frontend_log_controller_spec.rb": 0.18439500000386033, @@ -406,7 +406,7 @@ "spec/features/idv/sp_handoff_spec.rb": 107.38864300000569, "spec/routing/gpo_verification_routing_spec.rb": 0.7558229999995092, "spec/features/visitors/js_disabled_spec.rb": 8.520698000000266, - "spec/features/idv/steps/review_step_spec.rb": 61.3108210000064, + "spec/features/idv/steps/review_step_spec.rb": 63.121782000001986, "spec/services/data_requests/create_email_addresses_report_spec.rb": 0.020833000002312474, "spec/forms/api/profile_creation_form_spec.rb": 0.7231019999962882, "spec/forms/new_phone_form_spec.rb": 0.7517710000029183, @@ -783,7 +783,7 @@ "spec/models/sp_return_log_spec.rb": 0.005828000001201872, "spec/views/idv/phone_errors/warning.html.erb_spec.rb": 0.047647999999753665, "spec/features/two_factor_authentication/change_factor_spec.rb": 23.08214399999997, - "spec/features/idv/steps/forgot_password_step_spec.rb": 46.1194669999968, + "spec/features/idv/steps/forgot_password_step_spec.rb": 33.54180800000904, "spec/lib/headers_filter_spec.rb": 0.007020999997621402, "spec/views/devise/sessions/new.html.erb_spec.rb": 0.2999290000007022, "spec/views/idv/doc_auth/welcome.html.erb_spec.rb": 0.11305600000196137, @@ -826,7 +826,7 @@ "spec/models/service_provider_spec.rb": 0.06330100000195671, "spec/controllers/openid_connect/user_info_controller_spec.rb": 0.12280399999872316, "spec/mailers/user_mailer_spec.rb": 1.9315589999969234, - "spec/features/idv/doc_auth/verify_step_spec.rb": 178.04718500000308, + "spec/features/idv/steps/in_person/verify_step_spec.rb": 21.102479999986826, "spec/models/event_spec.rb": 0.033781999998609535, "spec/forms/security_event_form_spec.rb": 2.691689999999653, "spec/forms/otp_delivery_selection_form_spec.rb": 0.10276400000293506, diff --git a/spec/features/idv/analytics_spec.rb b/spec/features/idv/analytics_spec.rb index 53e582d9cb4..7f818b956fd 100644 --- a/spec/features/idv/analytics_spec.rb +++ b/spec/features/idv/analytics_spec.rb @@ -8,7 +8,7 @@ let(:fake_analytics) { FakeAnalytics.new } # rubocop:disable Layout/LineLength let(:happy_path_events) do - common_events = { + { 'IdV: intro visited' => {}, 'IdV: doc auth welcome visited' => { flow_path: 'standard', step: 'welcome', step_count: 1 }, 'IdV: doc auth welcome submitted' => { success: true, errors: {}, flow_path: 'standard', step: 'welcome', step_count: 1 }, @@ -37,19 +37,11 @@ 'IdV: final resolution' => { success: true }, 'IdV: personal key visited' => {}, 'IdV: personal key submitted' => {}, - } - { - FSMv1: common_events.merge( - 'Frontend: IdV: show personal key modal' => {}, - ), - FSMv2: common_events.merge( - 'IdV: personal key confirm visited' => {}, - 'IdV: personal key confirm submitted' => {}, - ), + 'Frontend: IdV: show personal key modal' => {}, } end let(:gpo_path_events) do - common_events = { + { 'IdV: intro visited' => {}, 'IdV: doc auth welcome visited' => { flow_path: 'standard', step: 'welcome', step_count: 1 }, 'IdV: doc auth welcome submitted' => { success: true, errors: {}, flow_path: 'standard', step: 'welcome', step_count: 1 }, @@ -75,10 +67,6 @@ 'IdV: phone of record visited' => {}, 'IdV: USPS address letter requested' => { enqueued_at: Time.zone.now }, } - { - FSMv1: common_events, - FSMv2: common_events, - } end let(:in_person_path_events) do { @@ -141,62 +129,45 @@ and_return(fake_analytics) end - { - FSMv1: [], - FSMv2: %w[password_confirm personal_key personal_key_confirm], - }.each do |flow_version, steps_enabled| - context flow_version do - before do - allow(IdentityConfig.store).to receive(:idv_api_enabled_steps).and_return(steps_enabled) - WebMock.allow_net_connect!(net_http_connect_on_start: true) - end - - after do - webmock_allow_list = WebMock::Config.instance.allow - WebMock.disallow_net_connect!(net_http_connect_on_start: nil, allow: webmock_allow_list) - end - - context 'Happy path' do - before do - sign_in_and_2fa_user(user) - visit_idp_from_sp_with_ial2(:oidc) - complete_welcome_step - complete_agreement_step - complete_upload_step - complete_document_capture_step - complete_ssn_step - complete_verify_step - complete_phone_step(user) - complete_review_step(user) - acknowledge_and_confirm_personal_key - end + context 'Happy path' do + before do + sign_in_and_2fa_user(user) + visit_idp_from_sp_with_ial2(:oidc) + complete_welcome_step + complete_agreement_step + complete_upload_step + complete_document_capture_step + complete_ssn_step + complete_verify_step + complete_phone_step(user) + complete_review_step(user) + acknowledge_and_confirm_personal_key + end - it 'records all of the events' do - happy_path_events[flow_version].each do |event, _attributes| - expect(fake_analytics).to have_logged_event(event) - end - end + it 'records all of the events' do + happy_path_events.each do |event, _attributes| + expect(fake_analytics).to have_logged_event(event) end + end + end - context 'GPO path' do - before do - sign_in_and_2fa_user(user) - visit_idp_from_sp_with_ial2(:oidc) - complete_welcome_step - complete_agreement_step - complete_upload_step - complete_document_capture_step - complete_ssn_step - complete_verify_step - enter_gpo_flow - gpo_step - end + context 'GPO path' do + before do + sign_in_and_2fa_user(user) + visit_idp_from_sp_with_ial2(:oidc) + complete_welcome_step + complete_agreement_step + complete_upload_step + complete_document_capture_step + complete_ssn_step + complete_verify_step + enter_gpo_flow + gpo_step + end - it 'records all of the events' do - gpo_path_events[flow_version].each do |event, _attributes| - expect(fake_analytics).to have_logged_event(event) - end - end + it 'records all of the events' do + gpo_path_events.each do |event, _attributes| + expect(fake_analytics).to have_logged_event(event) end end end diff --git a/spec/features/idv/steps/confirmation_step_spec.rb b/spec/features/idv/steps/confirmation_step_spec.rb index fef0f1b2441..9dd13ed6c6a 100644 --- a/spec/features/idv/steps/confirmation_step_spec.rb +++ b/spec/features/idv/steps/confirmation_step_spec.rb @@ -3,13 +3,11 @@ feature 'idv confirmation step', js: true do include IdvStepHelper - let(:idv_api_enabled_steps) { [] } let(:idv_personal_key_confirmation_enabled) { true } let(:sp) { nil } let(:address_verification_mechanism) { :phone } before do - allow(IdentityConfig.store).to receive(:idv_api_enabled_steps).and_return(idv_api_enabled_steps) allow(IdentityConfig.store).to receive(:idv_personal_key_confirmation_enabled). and_return(idv_personal_key_confirmation_enabled) start_idv_from_sp(sp) @@ -34,36 +32,6 @@ expect(page).to have_content(t('headings.personal_key')) end - context 'with idv app feature enabled' do - let(:idv_api_enabled_steps) { ['password_confirm', 'personal_key', 'personal_key_confirm'] } - - it_behaves_like 'personal key page' - - it 'allows the user to refresh and still displays the personal key' do - # Visit the current path is the same as refreshing - visit current_path - expect(page).to have_content(t('headings.personal_key')) - - acknowledge_and_confirm_personal_key - expect(page).to have_current_path(account_path) - end - - context 'with personal key confirmation disabled' do - let(:idv_personal_key_confirmation_enabled) { false } - - before do - click_continue if javascript_enabled? - end - - it 'does not display modal content. and continues to the account page' do - expect(page).not_to have_content t('forms.personal_key.title') - expect(page).not_to have_content t('forms.personal_key.instructions') - expect(current_path).to eq(account_path) - expect(page).to have_content t('headings.account.verified_account') - end - end - end - context 'verifying by gpo' do let(:address_verification_mechanism) { :gpo } diff --git a/spec/features/idv/steps/forgot_password_step_spec.rb b/spec/features/idv/steps/forgot_password_step_spec.rb index b9a0fe3f811..38020c1aeac 100644 --- a/spec/features/idv/steps/forgot_password_step_spec.rb +++ b/spec/features/idv/steps/forgot_password_step_spec.rb @@ -36,45 +36,4 @@ expect(current_path).to eq edit_user_password_path end - - context 'with idv app feature enabled' do - before do - allow(IdentityConfig.store).to receive(:idv_api_enabled_steps). - and_return(['password_confirm', 'personal_key', 'personal_key_confirm']) - end - - it 'goes to the forgot password page from the review page' do - start_idv_from_sp - complete_idv_steps_before_review_step - - click_link t('idv.forgot_password.link_text') - - expect(page.current_path).to eq(idv_app_forgot_password_path) - end - - it 'goes back to the review page from the forgot password page' do - start_idv_from_sp - complete_idv_steps_before_review_step - - click_link t('idv.forgot_password.link_text') - click_link t('idv.forgot_password.try_again') - - expect(page.current_path).to eq idv_app_path(step: :password_confirm) - end - - it 'allows the user to reset their password' do - start_idv_from_sp - complete_idv_steps_before_review_step - - click_link t('idv.forgot_password.link_text') - click_button t('idv.forgot_password.reset_password') - - expect(page).to have_current_path(forgot_password_path, ignore_query: true, wait: 10) - - open_last_email - click_email_link_matching(/reset_password_token/) - - expect(current_path).to eq edit_user_password_path - end - end end diff --git a/spec/features/idv/steps/in_person/verify_step_spec.rb b/spec/features/idv/steps/in_person/verify_step_spec.rb index 571427b69d8..cf7c9433713 100644 --- a/spec/features/idv/steps/in_person/verify_step_spec.rb +++ b/spec/features/idv/steps/in_person/verify_step_spec.rb @@ -7,11 +7,6 @@ before do allow(IdentityConfig.store).to receive(:in_person_proofing_enabled).and_return(true) - allow(IdentityConfig.store).to receive(:idv_api_enabled_steps).and_return( - ['password_confirm', - 'personal_key', - 'personal_key_confirm'], - ) end it 'provides back buttons for address, state ID, and SSN that discard changes', diff --git a/spec/features/idv/steps/review_step_spec.rb b/spec/features/idv/steps/review_step_spec.rb index 0fc66b86eaa..a71fd81f9c1 100644 --- a/spec/features/idv/steps/review_step_spec.rb +++ b/spec/features/idv/steps/review_step_spec.rb @@ -111,38 +111,4 @@ end end end - - context 'with idv app feature enabled', js: true do - before do - allow(IdentityConfig.store).to receive(:idv_api_enabled_steps). - and_return(['password_confirm', 'personal_key', 'personal_key_confirm']) - end - - it 'redirects to personal key step after user enters their password', allow_browser_log: true do - start_idv_from_sp - complete_idv_steps_before_review_step - - click_on t('idv.messages.review.intro') - - expect(page).to have_content('FAKEY') - expect(page).to have_content('MCFAKERSON') - expect(page).to have_content('1 FAKE RD') - expect(page).to have_content('GREAT FALLS, MT 59010') - expect(page).to have_content('October 6, 1938') - expect(page).to have_content(DocAuthHelper::GOOD_SSN) - expect(page).to have_content('(202) 555-1212') - - fill_in t('components.password_toggle.label'), with: 'this is not the right password' - click_idv_continue - - expect(page).to have_content(t('idv.errors.incorrect_password')) - expect(page).to have_current_path(idv_app_path(step: :password_confirm)) - - fill_in t('components.password_toggle.label'), with: user_password - click_idv_continue - - expect(page).to have_content(t('headings.personal_key')) - expect(page).to have_current_path(idv_app_path(step: :personal_key)) - end - end end From 399e9bb925b59c8c042325d2880de136d476cd5d Mon Sep 17 00:00:00 2001 From: Jonathan Hooper Date: Mon, 26 Sep 2022 12:17:55 -0400 Subject: [PATCH 19/23] Send the address to AAMVA (#7011) * Send the address to AAMVA This commit starts sending an address to AAMVA to validate. The verification still succeeds even if the address fails to match. This is because MVAs do not reliably perform address verification. The address results are included in the hash returned from `Proofing::Aamva::Responses::VerificationResponse#verification_results`. These can be used downstream to allow the address to be used for validation when other proofers are unable to verify an address. [skip changelog] * Only get the 5 digit zipcode * unbreak things * no more #warns * get the proofing specs passing for real this time * add address line 2 to the request body * parse address line 2 from the response * more address line 2 cleanups --- app/services/proofing/aamva/applicant.rb | 5 +++ .../aamva/request/templates/verify.xml.erb | 6 ++++ .../aamva/request/verification_request.rb | 31 +++++++++++++++++ .../aamva/response/verification_response.rb | 5 +++ .../aamva/requests/verification_request.xml | 6 ++++ .../aamva/responses/verification_response.xml | 4 +++ ...rification_response_namespaced_success.xml | 5 ++- spec/services/proofing/aamva/proofing_spec.rb | 4 +-- .../request/verification_request_spec.rb | 33 +++++++++++++++++++ .../response/verification_response_spec.rb | 5 +++ 10 files changed, 101 insertions(+), 3 deletions(-) diff --git a/app/services/proofing/aamva/applicant.rb b/app/services/proofing/aamva/applicant.rb index db8ca90ccef..d521a6cd384 100644 --- a/app/services/proofing/aamva/applicant.rb +++ b/app/services/proofing/aamva/applicant.rb @@ -11,6 +11,11 @@ def self.from_proofer_applicant(applicant) last_name: applicant[:last_name], dob: format_dob(applicant[:dob]), state_id_data: format_state_id_data(applicant), + address1: applicant[:address1], + address2: applicant[:address2], + city: applicant[:city], + state: applicant[:state], + zipcode: applicant[:zipcode]&.slice(0..4), ) end diff --git a/app/services/proofing/aamva/request/templates/verify.xml.erb b/app/services/proofing/aamva/request/templates/verify.xml.erb index 2d6740e135a..c9ab21d99b6 100644 --- a/app/services/proofing/aamva/request/templates/verify.xml.erb +++ b/app/services/proofing/aamva/request/templates/verify.xml.erb @@ -25,6 +25,12 @@ + + + + + + diff --git a/app/services/proofing/aamva/request/verification_request.rb b/app/services/proofing/aamva/request/verification_request.rb index 3154e1745bf..99132c7856c 100644 --- a/app/services/proofing/aamva/request/verification_request.rb +++ b/app/services/proofing/aamva/request/verification_request.rb @@ -60,9 +60,36 @@ def add_user_provided_data_to_body user_provided_data_map.each do |xpath, data| REXML::XPath.first(document, xpath).add_text(data) end + add_street_address_line_2_to_rexml_document(document) if applicant.address2.present? @body = document.to_s end + def add_street_address_line_2_to_rexml_document(document) + old_address_node = document.delete_element('//ns1:Address') + new_address_node = old_address_node.clone + old_address_node.children.each do |child_node| + next unless child_node.node_type == :element + + new_element = child_node.clone + new_element.add_text(child_node.text) + new_address_node.add_element(new_element) + + if child_node.name == 'AddressDeliveryPointText' + new_address_node.add_element(address_line_2_element) + end + end + REXML::XPath.first( + document, + '//ns:verifyDriverLicenseDataRequest', + ).add_element(new_address_node) + end + + def address_line_2_element + element = REXML::Element.new('ns2:AddressDeliveryPointText') + element.add_text(applicant.address2) + element + end + def build_request_body renderer = ERB.new(request_body_template) @body = renderer.result(binding) @@ -106,6 +133,10 @@ def user_provided_data_map '//ns2:PersonGivenName' => applicant.first_name, '//ns2:PersonSurName' => applicant.last_name, '//ns1:PersonBirthDate' => applicant.dob, + '//ns2:AddressDeliveryPointText' => applicant.address1, + '//ns2:LocationCityName' => applicant.city, + '//ns2:LocationStateUsPostalServiceCode' => applicant.state, + '//ns2:LocationPostalCode' => applicant.zipcode, } end diff --git a/app/services/proofing/aamva/response/verification_response.rb b/app/services/proofing/aamva/response/verification_response.rb index 315855591ee..62938086233 100644 --- a/app/services/proofing/aamva/response/verification_response.rb +++ b/app/services/proofing/aamva/response/verification_response.rb @@ -11,6 +11,11 @@ class VerificationResponse 'PersonBirthDateMatchIndicator' => :dob, 'PersonLastNameExactMatchIndicator' => :last_name, 'PersonFirstNameExactMatchIndicator' => :first_name, + 'AddressLine1MatchIndicator' => :address1, + 'AddressLine2MatchIndicator' => :address2, + 'AddressCityMatchIndicator' => :city, + 'AddressStateCodeMatchIndicator' => :state, + 'AddressZIP5MatchIndicator' => :zipcode, }.freeze REQUIRED_VERIFICATION_ATTRIBUTES = %i[ diff --git a/spec/fixtures/proofing/aamva/requests/verification_request.xml b/spec/fixtures/proofing/aamva/requests/verification_request.xml index fcfa0155aa3..3e6068d01db 100644 --- a/spec/fixtures/proofing/aamva/requests/verification_request.xml +++ b/spec/fixtures/proofing/aamva/requests/verification_request.xml @@ -25,6 +25,12 @@ Testy McTesterson + + 123 Sunnyside way + Sterling + VA + 20176 + diff --git a/spec/fixtures/proofing/aamva/responses/verification_response.xml b/spec/fixtures/proofing/aamva/responses/verification_response.xml index cc797798d14..9cbf1ecc2ff 100644 --- a/spec/fixtures/proofing/aamva/responses/verification_response.xml +++ b/spec/fixtures/proofing/aamva/responses/verification_response.xml @@ -16,4 +16,8 @@ true true true + true + true + true + true diff --git a/spec/fixtures/proofing/aamva/responses/verification_response_namespaced_success.xml b/spec/fixtures/proofing/aamva/responses/verification_response_namespaced_success.xml index 6a67cedf76b..19d08f8624e 100644 --- a/spec/fixtures/proofing/aamva/responses/verification_response_namespaced_success.xml +++ b/spec/fixtures/proofing/aamva/responses/verification_response_namespaced_success.xml @@ -19,8 +19,11 @@ true true true + true + true + true + true - diff --git a/spec/services/proofing/aamva/proofing_spec.rb b/spec/services/proofing/aamva/proofing_spec.rb index 9a0f27a18c1..cd7ed626a22 100644 --- a/spec/services/proofing/aamva/proofing_spec.rb +++ b/spec/services/proofing/aamva/proofing_spec.rb @@ -69,7 +69,7 @@ subject.aamva_proof(state_id_data, result) expect(result.failed?).to eq(true) - expect(result.errors).to eq(dob: ['UNVERIFIED']) + expect(result.errors).to include(dob: ['UNVERIFIED']) end end @@ -82,7 +82,7 @@ subject.aamva_proof(state_id_data, result) expect(result.failed?).to eq(true) - expect(result.errors).to eq(dob: ['MISSING']) + expect(result.errors).to include(dob: ['MISSING']) end end end diff --git a/spec/services/proofing/aamva/request/verification_request_spec.rb b/spec/services/proofing/aamva/request/verification_request_spec.rb index 1f72d0809df..b9a9136cd65 100644 --- a/spec/services/proofing/aamva/request/verification_request_spec.rb +++ b/spec/services/proofing/aamva/request/verification_request_spec.rb @@ -7,6 +7,10 @@ first_name: 'Testy', last_name: 'McTesterson', dob: '10/29/1942', + address1: '123 Sunnyside way', + city: 'Sterling', + state: 'VA', + zipcode: '20176-1234', ) applicant.state_id_data.merge!( state_id_number: '123456789', @@ -39,6 +43,35 @@ expect(subject.body).to_not include('') expect(subject.body).to include('<foo></bar>') end + + it 'includes an address line 2 if one is present' do + applicant.address2 = 'Apt 1' + + document = REXML::Document.new(subject.body) + address_node = REXML::XPath.first(document, '//ns:verifyDriverLicenseDataRequest/ns1:Address') + + address_node_element_names = address_node.elements.map(&:name) + address_node_element_values = address_node.elements.map(&:text) + + expect(address_node_element_names).to eq( + [ + 'AddressDeliveryPointText', + 'AddressDeliveryPointText', + 'LocationCityName', + 'LocationStateUsPostalServiceCode', + 'LocationPostalCode', + ], + ) + expect(address_node_element_values).to eq( + [ + applicant.address1, + applicant.address2, + applicant.city, + applicant.state, + applicant.zipcode, + ], + ) + end end describe '#headers' do diff --git a/spec/services/proofing/aamva/response/verification_response_spec.rb b/spec/services/proofing/aamva/response/verification_response_spec.rb index a80757ef96b..2914a81357e 100644 --- a/spec/services/proofing/aamva/response/verification_response_spec.rb +++ b/spec/services/proofing/aamva/response/verification_response_spec.rb @@ -18,6 +18,11 @@ dob: true, last_name: true, first_name: true, + address1: true, + address2: nil, + city: true, + state: true, + zipcode: true, } end From f43f000bc5f0fb5df9df9281417bab8f7657fc31 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Mon, 26 Sep 2022 10:55:00 -0700 Subject: [PATCH 20/23] Add mock ThreatMetrix Javascript implementation (LG-7174) (#7018) * Add feature spec that makes sure mock JS works * Enable for all proofing in development by default * Use currentScript to avoid scraping DOM * Use react effects, hooks * Update ddp mock proofer - no more magic SSNs, only results from redis changelog: Internal, Device Profiling, Add ability to select mock device profiling response from UI Co-authored-by: Andrew Duthie --- .../test/device_profiling_controller.rb | 33 +++++ .../packs/mock-device-profiling.tsx | 127 ++++++++++++++++++ app/services/idv/steps/in_person/ssn_step.rb | 10 +- app/services/idv/steps/ssn_step.rb | 10 +- .../idv/steps/threat_metrix_step_helper.rb | 52 +++++++ app/services/proofing/mock/ddp_mock_client.rb | 39 +++--- .../proofing/mock/device_profiling_backend.rb | 28 ++++ app/views/idv/doc_auth/ssn.html.erb | 8 +- app/views/idv/in_person/ssn.html.erb | 8 +- app/views/idv/shared/_ssn.html.erb | 12 +- .../test/device_profiling/index.html.erb | 9 ++ config/application.yml.default | 3 +- config/routes.rb | 6 + .../test/device_profiling_controller_spec.rb | 35 +++++ spec/features/idv/doc_auth/ssn_step_spec.rb | 13 ++ spec/jobs/resolution_proofing_job_spec.rb | 8 ++ .../proofing/mock/ddp_mock_client_spec.rb | 47 ++++--- .../mock/device_profiling_backend_spec.rb | 27 ++++ spec/views/idv/shared/_ssn.html.erb_spec.rb | 2 + 19 files changed, 421 insertions(+), 56 deletions(-) create mode 100644 app/controllers/test/device_profiling_controller.rb create mode 100644 app/javascript/packs/mock-device-profiling.tsx create mode 100644 app/services/idv/steps/threat_metrix_step_helper.rb create mode 100644 app/services/proofing/mock/device_profiling_backend.rb create mode 100644 app/views/test/device_profiling/index.html.erb create mode 100644 spec/controllers/test/device_profiling_controller_spec.rb create mode 100644 spec/services/proofing/mock/device_profiling_backend_spec.rb diff --git a/app/controllers/test/device_profiling_controller.rb b/app/controllers/test/device_profiling_controller.rb new file mode 100644 index 00000000000..7b4c47abbc2 --- /dev/null +++ b/app/controllers/test/device_profiling_controller.rb @@ -0,0 +1,33 @@ +module Test + class DeviceProfilingController < ApplicationController + prepend_before_action :skip_session_load + prepend_before_action :skip_session_expiration + skip_before_action :verify_authenticity_token + + layout false + + # iframe fallback + def index + tmx_backend.record_profiling_result( + session_id: params[:session_id], + result: 'no_result', + ) + end + + # explicit JS POST + def create + tmx_backend.record_profiling_result( + session_id: params[:session_id], + result: params[:result], + ) + + head :ok + end + + private + + def tmx_backend + @tmx_backend ||= Proofing::Mock::DeviceProfilingBackend.new + end + end +end diff --git a/app/javascript/packs/mock-device-profiling.tsx b/app/javascript/packs/mock-device-profiling.tsx new file mode 100644 index 00000000000..aac43583608 --- /dev/null +++ b/app/javascript/packs/mock-device-profiling.tsx @@ -0,0 +1,127 @@ +import { render } from 'react-dom'; +import { useInstanceId } from '@18f/identity-react-hooks'; +import { ChangeEvent, useState, useEffect } from 'react'; + +const { currentScript } = document; + +function loadSessionId(): string | undefined { + if (currentScript instanceof HTMLScriptElement) { + return new URL(currentScript.src).searchParams.get('session_id') || undefined; + } +} + +function submitMockFraudResult({ result, sessionId }: { result: string; sessionId?: string }) { + window.fetch('/test/device_profiling', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + session_id: sessionId, + result, + }), + }); +} + +interface ChaosOption { + apply: () => void; + undo: () => void; +} + +const CHAOS_OPTIONS: ChaosOption[] = [ + { + apply: () => { + document.body.style.transform = 'scale(-1, 1)'; + }, + undo: () => { + document.body.style.transform = ''; + }, + }, + { + apply: () => { + document.body.style.transform = 'rotate(5deg)'; + }, + undo: () => { + document.body.style.transform = ''; + }, + }, + { + apply: () => { + document.body.style.filter = 'invert(100%)'; + }, + undo: () => { + document.body.style.filter = ''; + }, + }, + { + apply: () => { + document.body.style.fontFamily = 'Comic Sans MS'; + }, + undo: () => { + document.body.style.fontFamily = ''; + }, + }, +]; + +function MockDeviceProfilingOptions() { + const [selectedValue, setSelectedValue] = useState(''); + + useEffect(() => { + if (selectedValue === 'chaotic') { + const randomChaosOption = CHAOS_OPTIONS[Math.floor(Math.random() * CHAOS_OPTIONS.length)]; + randomChaosOption.apply(); + return randomChaosOption.undo; + } + if (selectedValue) { + submitMockFraudResult({ result: selectedValue, sessionId: loadSessionId() }); + } + }, [selectedValue]); + + const instanceId = useInstanceId(); + const inputId = `select-input-${instanceId}`; + + const options = [ + { value: 'no_result', title: 'No Result' }, + { value: 'pass', title: 'Pass' }, + { value: 'reject', title: 'Reject' }, + { value: 'review', title: 'Review' }, + { value: 'chaotic', title: 'Do something chaotic' }, + ]; + + return ( + <> + {/* eslint-disable-next-line jsx-a11y/label-has-associated-control */} + + + + ); +} + +document.addEventListener('DOMContentLoaded', () => { + const ssnInput = document.getElementsByName('doc_auth[ssn]')[0]; + + if (ssnInput) { + const passwordToggle = ssnInput.closest('lg-password-toggle'); + + const div = document.createElement('div'); + passwordToggle?.parentElement?.appendChild(div); + + render(, div); + } +}); diff --git a/app/services/idv/steps/in_person/ssn_step.rb b/app/services/idv/steps/in_person/ssn_step.rb index fb93befe803..6d2eb2238a9 100644 --- a/app/services/idv/steps/in_person/ssn_step.rb +++ b/app/services/idv/steps/in_person/ssn_step.rb @@ -4,6 +4,8 @@ module InPerson class SsnStep < DocAuthBaseStep STEP_INDICATOR_STEP = :verify_info + include ThreatMetrixStepHelper + def call flow_session[:pii_from_user][:ssn] = ssn @@ -18,7 +20,7 @@ def call def extra_view_variables { updating_ssn: updating_ssn, - threatmetrix_session_id: generate_threatmetrix_session_id, + **threatmetrix_view_variables, } end @@ -28,12 +30,6 @@ def form_submit Idv::SsnFormatForm.new(current_user).submit(permit(:ssn)) end - def generate_threatmetrix_session_id - return unless service_provider_device_profiling_enabled? - flow_session[:threatmetrix_session_id] = SecureRandom.uuid if !updating_ssn - flow_session[:threatmetrix_session_id] - end - def ssn flow_params[:ssn] end diff --git a/app/services/idv/steps/ssn_step.rb b/app/services/idv/steps/ssn_step.rb index e959254601b..a96927adb17 100644 --- a/app/services/idv/steps/ssn_step.rb +++ b/app/services/idv/steps/ssn_step.rb @@ -3,6 +3,8 @@ module Steps class SsnStep < DocAuthBaseStep STEP_INDICATOR_STEP = :verify_info + include ThreatMetrixStepHelper + def call return invalid_state_response if invalid_state? @@ -19,7 +21,7 @@ def call def extra_view_variables { updating_ssn: updating_ssn, - threatmetrix_session_id: generate_threatmetrix_session_id, + **threatmetrix_view_variables, } end @@ -34,12 +36,6 @@ def invalid_state_response FormResponse.new(success: false) end - def generate_threatmetrix_session_id - return unless service_provider_device_profiling_enabled? - flow_session[:threatmetrix_session_id] = SecureRandom.uuid if !updating_ssn - flow_session[:threatmetrix_session_id] - end - def ssn flow_params[:ssn] end diff --git a/app/services/idv/steps/threat_metrix_step_helper.rb b/app/services/idv/steps/threat_metrix_step_helper.rb new file mode 100644 index 00000000000..154af5df951 --- /dev/null +++ b/app/services/idv/steps/threat_metrix_step_helper.rb @@ -0,0 +1,52 @@ +module Idv + module Steps + module ThreatMetrixStepHelper + def threatmetrix_view_variables + session_id = generate_threatmetrix_session_id + + { + threatmetrix_session_id: session_id, + threatmetrix_javascript_urls: session_id && threatmetrix_javascript_urls(session_id), + threatmetrix_iframe_url: session_id && threatmetrix_iframe_url(session_id), + } + end + + def generate_threatmetrix_session_id + return unless service_provider_device_profiling_enabled? + flow_session[:threatmetrix_session_id] = SecureRandom.uuid if !updating_ssn + flow_session[:threatmetrix_session_id] + end + + # @return [Array] + def threatmetrix_javascript_urls(session_id) + sources = if IdentityConfig.store.lexisnexis_threatmetrix_mock_enabled + AssetSources.get_sources('mock-device-profiling') + else + ['https://h.online-metrix.net/fp/tags.js'] + end + + sources.map do |source| + UriService.add_params( + source, + org_id: IdentityConfig.store.lexisnexis_threatmetrix_org_id, + session_id: session_id, + ) + end + end + + def threatmetrix_iframe_url(session_id) + source = if IdentityConfig.store.lexisnexis_threatmetrix_mock_enabled + Rails.application.routes.url_helpers.test_device_profiling_iframe_url + else + 'https://h.online-metrix.net/fp/tags' + end + + UriService.add_params( + source, + org_id: IdentityConfig.store.lexisnexis_threatmetrix_org_id, + session_id: session_id, + ) + end + end + end +end diff --git a/app/services/proofing/mock/ddp_mock_client.rb b/app/services/proofing/mock/ddp_mock_client.rb index d6553fed8ef..9a5bd8c77e8 100644 --- a/app/services/proofing/mock/ddp_mock_client.rb +++ b/app/services/proofing/mock/ddp_mock_client.rb @@ -21,35 +21,32 @@ class DdpMockClient < Proofing::Base TRANSACTION_ID = 'ddp-mock-transaction-id-123' - # Trigger the "REJECT" status - REJECT_STATUS_SSN = '666-77-8888' - - # Trigger the "REVIEW" status - REVIEW_STATUS_SSN = '666-77-9999' - - # Trigger a nil status - NIL_STATUS_SSN = '666-77-0000' - proof do |applicant, result| result.transaction_id = TRANSACTION_ID + response_body = File.read( Rails.root.join( 'spec', 'fixtures', 'proofing', 'lexis_nexis', 'ddp', 'successful_response.json' ), ) - result.review_status = case SsnFormatter.format(applicant[:ssn]) - when REJECT_STATUS_SSN - 'reject' - when REVIEW_STATUS_SSN - 'review' - when NIL_STATUS_SSN - nil - else - 'pass' + + status = review_status(session_id: applicant[:threatmetrix_session_id]) + + result.review_status = status + result.response_body = JSON.parse(response_body).tap do |json_body| + json_body['review_status'] = status + end + end + + def review_status(session_id:) + device_status = DeviceProfilingBackend.new.profiling_result(session_id) + + case device_status + when 'no_result' + return nil + when 'reject', 'review', 'pass' + device_status end - result.response_body = JSON.parse( - response_body.gsub('REVIEW_STATUS', result.review_status.to_s), - ) end end end diff --git a/app/services/proofing/mock/device_profiling_backend.rb b/app/services/proofing/mock/device_profiling_backend.rb new file mode 100644 index 00000000000..e883f3800d8 --- /dev/null +++ b/app/services/proofing/mock/device_profiling_backend.rb @@ -0,0 +1,28 @@ +module Proofing + module Mock + class DeviceProfilingBackend + RESULTS = %w[ + no_result + pass + reject + review + ].to_set.freeze + + RESULT_TIMEOUT = 3600 + + def record_profiling_result(session_id:, result:) + raise ArgumentError, "unknown result=#{result}" if !RESULTS.include?(result) + + REDIS_POOL.with do |redis| + redis.setex("device_profiling:#{session_id}", RESULT_TIMEOUT, result) + end + end + + def profiling_result(session_id) + REDIS_POOL.with do |redis| + redis.get("device_profiling:#{session_id}") + end + end + end + end +end diff --git a/app/views/idv/doc_auth/ssn.html.erb b/app/views/idv/doc_auth/ssn.html.erb index 148aaedde4a..841d2909057 100644 --- a/app/views/idv/doc_auth/ssn.html.erb +++ b/app/views/idv/doc_auth/ssn.html.erb @@ -1 +1,7 @@ -<%= render 'idv/shared/ssn', flow_session: flow_session, success_alert_enabled: !updating_ssn, updating_ssn: updating_ssn, threatmetrix_session_id: threatmetrix_session_id %> +<%= render 'idv/shared/ssn', + flow_session: flow_session, + success_alert_enabled: !updating_ssn, + updating_ssn: updating_ssn, + threatmetrix_session_id: threatmetrix_session_id, + threatmetrix_javascript_urls: threatmetrix_javascript_urls, + threatmetrix_iframe_url: threatmetrix_iframe_url %> diff --git a/app/views/idv/in_person/ssn.html.erb b/app/views/idv/in_person/ssn.html.erb index 558e4ce0128..eb7df265545 100644 --- a/app/views/idv/in_person/ssn.html.erb +++ b/app/views/idv/in_person/ssn.html.erb @@ -1 +1,7 @@ -<%= render 'idv/shared/ssn', flow_session: flow_session, success_alert_enabled: false, updating_ssn: updating_ssn, threatmetrix_session_id: threatmetrix_session_id %> +<%= render 'idv/shared/ssn', + flow_session: flow_session, + success_alert_enabled: false, + updating_ssn: updating_ssn, + threatmetrix_session_id: threatmetrix_session_id, + threatmetrix_javascript_urls: threatmetrix_javascript_urls, + threatmetrix_iframe_url: threatmetrix_iframe_url %> diff --git a/app/views/idv/shared/_ssn.html.erb b/app/views/idv/shared/_ssn.html.erb index 45036b92334..0bc2e473493 100644 --- a/app/views/idv/shared/_ssn.html.erb +++ b/app/views/idv/shared/_ssn.html.erb @@ -31,10 +31,16 @@ locals: <% if IdentityConfig.store.proofing_device_profiling_collecting_enabled %> <% unless IdentityConfig.store.lexisnexis_threatmetrix_org_id.empty? %> <% if threatmetrix_session_id.present? %> - <%= javascript_include_tag "https://h.online-metrix.net/fp/tags.js?org_id=#{IdentityConfig.store.lexisnexis_threatmetrix_org_id}&session_id=#{threatmetrix_session_id}", nonce: true %> + <% threatmetrix_javascript_urls.each do |threatmetrix_javascript_url| %> + <%= javascript_include_tag threatmetrix_javascript_url, nonce: true %> + <% end %> <% end %> <% end %> diff --git a/app/views/test/device_profiling/index.html.erb b/app/views/test/device_profiling/index.html.erb new file mode 100644 index 00000000000..3ffcd2dc0f5 --- /dev/null +++ b/app/views/test/device_profiling/index.html.erb @@ -0,0 +1,9 @@ + + + + iframe fallback + + + hello from iframe + + diff --git a/config/application.yml.default b/config/application.yml.default index 5677d3533b8..50ee5c37deb 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -212,7 +212,7 @@ piv_cac_verify_token_url: https://localhost:8443/ platform_authentication_enabled: true poll_rate_for_verify_in_seconds: 3 proofer_mock_fallback: true -proofing_device_profiling_collecting_enabled: false +proofing_device_profiling_collecting_enabled: true proofing_device_profiling_decisioning_enabled: false proof_address_max_attempts: 5 proof_address_max_attempt_window_in_minutes: 360 @@ -351,6 +351,7 @@ development: newrelic_browser_app_id: '' newrelic_browser_key: '' newrelic_license_key: '' + no_sp_device_profiling_enabled: true nonessential_email_banlist: '["banned_email@gmail.com"]' otp_delivery_blocklist_findtime: 5 password_pepper: f22d4b2cafac9066fe2f4416f5b7a32c diff --git a/config/routes.rb b/config/routes.rb index dc69a717052..be30b0dcb1e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -150,6 +150,12 @@ end end + if IdentityConfig.store.lexisnexis_threatmetrix_mock_enabled + get '/test/device_profiling' => 'test/device_profiling#index', + as: :test_device_profiling_iframe + post '/test/device_profiling' => 'test/device_profiling#create' + end + get '/auth_method_confirmation' => 'mfa_confirmation#show' post '/auth_method_confirmation/skip' => 'mfa_confirmation#skip' diff --git a/spec/controllers/test/device_profiling_controller_spec.rb b/spec/controllers/test/device_profiling_controller_spec.rb new file mode 100644 index 00000000000..ecd09ab62b1 --- /dev/null +++ b/spec/controllers/test/device_profiling_controller_spec.rb @@ -0,0 +1,35 @@ +require 'rails_helper' + +RSpec.describe Test::DeviceProfilingController do + let(:session_id) { SecureRandom.uuid } + + around do |ex| + REDIS_POOL.with { |namespaced| namespaced.redis.flushdb } + ex.run + REDIS_POOL.with { |namespaced| namespaced.redis.flushdb } + end + + describe '#index' do + it 'sets no_result for the session_id' do + expect do + get :index, params: { session_id: session_id } + end.to( + change { Proofing::Mock::DeviceProfilingBackend.new.profiling_result(session_id) }. + from(nil).to('no_result'), + ) + end + end + + describe '#create' do + let(:result) { 'pass' } + + it 'sets the result in redis' do + expect do + post :create, params: { session_id: session_id, result: result } + end.to( + change { Proofing::Mock::DeviceProfilingBackend.new.profiling_result(session_id) }. + from(nil).to(result), + ) + end + end +end diff --git a/spec/features/idv/doc_auth/ssn_step_spec.rb b/spec/features/idv/doc_auth/ssn_step_spec.rb index c7209415a81..698eabf9b62 100644 --- a/spec/features/idv/doc_auth/ssn_step_spec.rb +++ b/spec/features/idv/doc_auth/ssn_step_spec.rb @@ -7,6 +7,9 @@ context 'desktop' do before do + allow(IdentityConfig.store). + to receive(:no_sp_device_profiling_enabled).and_return(true) + sign_in_and_2fa_user complete_doc_auth_steps_before_ssn_step end @@ -20,10 +23,20 @@ it 'proceeds to the next page with valid info' do fill_out_ssn_form_ok + + match = page.body.match(/session_id=(?[^"&]+)/) + session_id = match && match[:session_id] + expect(session_id).to be_present + + select 'Review', from: 'mock_profiling_result' + expect(page.find_field(t('idv.form.ssn_label_html'))['aria-invalid']).to eq('false') click_idv_continue expect(page).to have_current_path(idv_doc_auth_verify_step) + + profiling_result = Proofing::Mock::DeviceProfilingBackend.new.profiling_result(session_id) + expect(profiling_result).to eq('review') end it 'does not proceed to the next page with invalid info' do diff --git a/spec/jobs/resolution_proofing_job_spec.rb b/spec/jobs/resolution_proofing_job_spec.rb index 7a3d9f16ab9..5e81b0aa537 100644 --- a/spec/jobs/resolution_proofing_job_spec.rb +++ b/spec/jobs/resolution_proofing_job_spec.rb @@ -121,6 +121,10 @@ allow(state_id_proofer).to receive(:proof). and_return(Proofing::Result.new(transaction_id: aamva_transaction_id)) + Proofing::Mock::DeviceProfilingBackend.new.record_profiling_result( + session_id: threatmetrix_session_id, + result: 'pass', + ) end let(:lexisnexis_response) do @@ -479,6 +483,10 @@ and_return(Proofing::Result.new) expect(state_id_proofer).to receive(:proof). and_return(Proofing::Result.new) + Proofing::Mock::DeviceProfilingBackend.new.record_profiling_result( + session_id: threatmetrix_session_id, + result: 'pass', + ) end it 'logs the trace_id and timing info for ProofResolution and the Threatmetrix info' do diff --git a/spec/services/proofing/mock/ddp_mock_client_spec.rb b/spec/services/proofing/mock/ddp_mock_client_spec.rb index c5302a72dfa..7fce70e87e5 100644 --- a/spec/services/proofing/mock/ddp_mock_client_spec.rb +++ b/spec/services/proofing/mock/ddp_mock_client_spec.rb @@ -1,9 +1,17 @@ require 'rails_helper' RSpec.describe Proofing::Mock::DdpMockClient do + around do |ex| + REDIS_POOL.with { |namespaced| namespaced.redis.flushdb } + ex.run + REDIS_POOL.with { |namespaced| namespaced.redis.flushdb } + end + + let(:threatmetrix_session_id) { SecureRandom.uuid } + let(:applicant) do Idp::Constants::MOCK_IDV_APPLICANT_WITH_SSN.merge( - threatmetrix_session_id: 'ABCD-1234', + threatmetrix_session_id: threatmetrix_session_id, request_ip: Faker::Internet.ip_v4_address, ) end @@ -18,28 +26,37 @@ describe '#proof' do subject(:result) { instance.proof(applicant) } - it 'passes by default' do - expect(result.review_status).to eq('pass') + before do + Proofing::Mock::DeviceProfilingBackend.new.record_profiling_result( + result: redis_result, + session_id: threatmetrix_session_id, + ) end - context 'with magic "reject" SSN' do - let(:applicant) { super().merge(ssn: '666-77-8888') } - it 'fails' do - expect(result.review_status).to eq('reject') + context 'with explicit no_result' do + let(:redis_result) { 'no_result' } + + it 'has a nil review status' do + expect(result.review_status).to be_nil + expect(result.response_body['review_status']).to be_nil end end - context 'with magic "review" SSN' do - let(:applicant) { super().merge(ssn: '666-77-9999') } - it 'fails' do - expect(result.review_status).to eq('review') + context 'with reject' do + let(:redis_result) { 'reject' } + + it 'has a reject status' do + expect(result.review_status).to eq('reject') + expect(result.response_body['review_status']).to eq('reject') end end - context 'with magic "nil" SSN' do - let(:applicant) { super().merge(ssn: '666-77-0000') } - it 'fails' do - expect(result.review_status).to be_nil + context 'with pass' do + let(:redis_result) { 'pass' } + + it 'has a pass status' do + expect(result.review_status).to eq('pass') + expect(result.response_body['review_status']).to eq('pass') end end end diff --git a/spec/services/proofing/mock/device_profiling_backend_spec.rb b/spec/services/proofing/mock/device_profiling_backend_spec.rb new file mode 100644 index 00000000000..e318d80bed9 --- /dev/null +++ b/spec/services/proofing/mock/device_profiling_backend_spec.rb @@ -0,0 +1,27 @@ +require 'rails_helper' + +RSpec.describe Proofing::Mock::DeviceProfilingBackend do + around do |ex| + REDIS_POOL.with { |namespaced| namespaced.redis.flushdb } + ex.run + REDIS_POOL.with { |namespaced| namespaced.redis.flushdb } + end + + subject(:backend) { described_class.new } + let(:session_id) { SecureRandom.uuid } + + describe '#record_profiling_result' do + it 'raises with unknown result' do + expect { backend.record_profiling_result(session_id: session_id, result: 'aaa') }. + to raise_error(ArgumentError) + end + + it 'sets the value in redis' do + backend.record_profiling_result(session_id: session_id, result: 'reject') + + expect(backend.profiling_result(session_id)).to eq('reject') + + expect(backend.profiling_result('different_id')).to be_nil + end + end +end diff --git a/spec/views/idv/shared/_ssn.html.erb_spec.rb b/spec/views/idv/shared/_ssn.html.erb_spec.rb index 7f50ed52030..25829b40b13 100644 --- a/spec/views/idv/shared/_ssn.html.erb_spec.rb +++ b/spec/views/idv/shared/_ssn.html.erb_spec.rb @@ -31,6 +31,8 @@ success_alert_enabled: false, threatmetrix_session_id: session_id, updating_ssn: updating_ssn, + threatmetrix_javascript_urls: [tags_js_url], + threatmetrix_iframe_url: tags_iframe_url, } end From b79159c14f73d66c968ac4fc0dda917844c71bc0 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Mon, 26 Sep 2022 13:59:41 -0400 Subject: [PATCH 21/23] Improve changelog tool flexibility for commit messages (#7027) **Why**: To help avoid developer toil associated with crafting commit messages. Changelog:Internal,Changelog,Improve changelog tool flexibility for commit messages --- scripts/changelog_check.rb | 2 +- spec/fixtures/git_log_changelog.yml | 24 ++++++++++++++++++++++++ spec/scripts/changelog_check_spec.rb | 28 +++++++++++++++++++++++++++- 3 files changed, 52 insertions(+), 2 deletions(-) diff --git a/scripts/changelog_check.rb b/scripts/changelog_check.rb index 5af1e51f4f6..58c7f1e1a3c 100755 --- a/scripts/changelog_check.rb +++ b/scripts/changelog_check.rb @@ -3,7 +3,7 @@ require 'optparse' CHANGELOG_REGEX = - %r{^(?:\* )?changelog: (?[\w -/]{2,}), (?[\w -]{2,}), (?.+)$} + %r{^(?:\* )?[cC]hangelog: ?(?[\w -/]{2,}), ?(?[\w -]{2,}), ?(?.+)$} CATEGORIES = [ 'Improvements', 'Bug Fixes', diff --git a/spec/fixtures/git_log_changelog.yml b/spec/fixtures/git_log_changelog.yml index 7ca04ecc86c..7921ca0c420 100644 --- a/spec/fixtures/git_log_changelog.yml +++ b/spec/fixtures/git_log_changelog.yml @@ -79,3 +79,27 @@ squashed_commit_invalid: DELIMITER title: 'LG-5629 account buttons part 2 (#5945)' commit_messages: [] +commit_changelog_capitalized: + commit_log: | + title: Improve changelog tool flexibility for commit messages (#7000) + body:Changelog: Internal, Changelog, Improve changelog tool flexibility for commit messages + DELIMITER + title: Improve changelog tool flexibility for commit messages (#7000) + category: Internal + subcategory: Changelog + change: Improve changelog tool flexibility for commit messages + pr_number: '7000' + commit_messages: + - 'Changelog: Internal, Changelog, Improve changelog tool flexibility for commit messages' +commit_changelog_whitespace: + commit_log: | + title: Improve changelog tool flexibility for commit messages (#7000) + body:changelog:Internal,Changelog,Improve changelog tool flexibility for commit messages + DELIMITER + title: Improve changelog tool flexibility for commit messages (#7000) + category: Internal + subcategory: Changelog + change: Improve changelog tool flexibility for commit messages + pr_number: '7000' + commit_messages: + - 'changelog:Internal,Changelog,Improve changelog tool flexibility for commit messages' diff --git a/spec/scripts/changelog_check_spec.rb b/spec/scripts/changelog_check_spec.rb index 81dac6be249..29824a1e3fa 100644 --- a/spec/scripts/changelog_check_spec.rb +++ b/spec/scripts/changelog_check_spec.rb @@ -8,7 +8,7 @@ it 'builds a git log into structured changelog objects' do git_log = git_fixtures.values.pluck('commit_log').join("\n") changelog_entries = generate_changelog(git_log) - expect(changelog_entries.length).to eq 4 + expect(changelog_entries.length).to eq 6 fixture_and_changelog = git_fixtures.values.filter do |x| x['category'].present? end.zip(changelog_entries) @@ -35,6 +35,32 @@ expect(commits.first['pr_number']).to eq changelog.first.pr_number expect(commits.first['change']).to eq changelog.first.change end + + it 'detects changelog regardless of capitalization' do + commit = git_fixtures['commit_changelog_capitalized'] + git_log = commit['commit_log'] + + changelog = generate_changelog(git_log) + + expect(changelog).not_to be_empty + expect(commit['category']).to eq changelog.first.category + expect(commit['subcategory']).to eq changelog.first.subcategory + expect(commit['pr_number']).to eq changelog.first.pr_number + expect(commit['change']).to eq changelog.first.change + end + + it 'detects changelog regardless of whitespace' do + commit = git_fixtures['commit_changelog_whitespace'] + git_log = commit['commit_log'] + + changelog = generate_changelog(git_log) + + expect(changelog).not_to be_empty + expect(commit['category']).to eq changelog.first.category + expect(commit['subcategory']).to eq changelog.first.subcategory + expect(commit['pr_number']).to eq changelog.first.pr_number + expect(commit['change']).to eq changelog.first.change + end end describe '#build_structured_git_log' do From a49111ad2e235bf93276f6c714abf1f0a95b9a89 Mon Sep 17 00:00:00 2001 From: Shannon A <20867088+svalexander@users.noreply.github.com> Date: Mon, 26 Sep 2022 16:00:08 -0400 Subject: [PATCH 22/23] lg-7150 add login icon alt text (#7023) * update alt tag to login.gov logo * update user_mailer.html spec to include text for img alt text * [skip changelog] * use var * move alt text to i18l files * update spec * update untranslated keys --- app/views/layouts/user_mailer.html.erb | 1 + config/locales/mailer/en.yml | 1 + config/locales/mailer/es.yml | 1 + config/locales/mailer/fr.yml | 1 + spec/i18n_spec.rb | 1 + spec/views/layouts/user_mailer.html.erb_spec.rb | 4 ++++ 6 files changed, 9 insertions(+) diff --git a/app/views/layouts/user_mailer.html.erb b/app/views/layouts/user_mailer.html.erb index 243a7b28278..8ed95c823ec 100644 --- a/app/views/layouts/user_mailer.html.erb +++ b/app/views/layouts/user_mailer.html.erb @@ -46,6 +46,7 @@ attachments['logo.png'].url, size: '142x19', style: 'width: 142px; height: 19px;', + alt: t('mailer.logo', app_name: APP_NAME), ) %> diff --git a/config/locales/mailer/en.yml b/config/locales/mailer/en.yml index 879ffe1ffcc..7efbe725413 100644 --- a/config/locales/mailer/en.yml +++ b/config/locales/mailer/en.yml @@ -5,5 +5,6 @@ en: email_reuse_notice: subject: This email address is already associated with an account. help: If you need help, visit %{link} + logo: '%{app_name} logo' no_reply: Please do not reply to this message. privacy_policy: Privacy policy diff --git a/config/locales/mailer/es.yml b/config/locales/mailer/es.yml index a372efd30d7..e9cd9abe3d7 100644 --- a/config/locales/mailer/es.yml +++ b/config/locales/mailer/es.yml @@ -5,5 +5,6 @@ es: email_reuse_notice: subject: Este email ya está asociado a una cuenta. help: Si necesita ayuda, visite %{link} + logo: '%{app_name} logo' no_reply: Por favor, no responda a este mensaje. privacy_policy: Póliza de privacidad diff --git a/config/locales/mailer/fr.yml b/config/locales/mailer/fr.yml index 973fd7b1a4a..ebcad3e0227 100644 --- a/config/locales/mailer/fr.yml +++ b/config/locales/mailer/fr.yml @@ -5,5 +5,6 @@ fr: email_reuse_notice: subject: Cette adresse courriel est déjà associée à un compte. help: Si vous avez besoin d’aide, visitez le site %{link} + logo: '%{app_name} logo' no_reply: Veuillez ne pas répondre à ce message. privacy_policy: Politique de confidentialité diff --git a/spec/i18n_spec.rb b/spec/i18n_spec.rb index a19f6c95957..48f5da5c5b4 100644 --- a/spec/i18n_spec.rb +++ b/spec/i18n_spec.rb @@ -29,6 +29,7 @@ class BaseTask { key: 'time.pm' }, # "PM" is "PM" in French and Spanish { key: 'datetime.dotiw.minutes.one' }, # "minute is minute" in French and English { key: 'datetime.dotiw.minutes.other' }, # "minute is minute" in French and English + { key: 'mailer.logo' }, # "logo is logo" in English, French and Spanish ].freeze # rubocop:enable Layout/LineLength diff --git a/spec/views/layouts/user_mailer.html.erb_spec.rb b/spec/views/layouts/user_mailer.html.erb_spec.rb index b05277588c1..d2acbf298de 100644 --- a/spec/views/layouts/user_mailer.html.erb_spec.rb +++ b/spec/views/layouts/user_mailer.html.erb_spec.rb @@ -19,6 +19,10 @@ expect(rendered).to have_css("img[src*='.mail']") end + it 'includes alt text for app logo that reads Login.gov logo' do + expect(rendered).to have_css("img[alt='#{t('mailer.logo', app_name: APP_NAME)}']") + end + it 'includes the message subject in the body' do expect(rendered).to have_content @mail.subject end From 688a0d104f23296459a3316bb5282e691d24c360 Mon Sep 17 00:00:00 2001 From: Shannon A <20867088+svalexander@users.noreply.github.com> Date: Mon, 26 Sep 2022 16:23:02 -0400 Subject: [PATCH 23/23] LG-6896: Send 'fraud suspected' in-person proofing failed email (#7013) * string translations added * fraud email template * update string used in fraud erb file * update strings so hyperlink works * add check to send alternate failed email * add test for new email to proofing results job spec * add new failed email test to user mailer spec * remove first_name from email as it's not available at this point of the process * Improvements, GetUspsProofingResultsJob, add additonal failure email * changed hi to hello * remove erroneous first_name from user mailer spec * change deliver now or later to deliver later * change fraudSuspected to boolean and switched back to deliver_now_or_later * change back to deliver_later * lint changes * adding suggestions * new line * remove line * Improvement, User Mailer, send suspected fraud failure email * changelog:Improvement, GetUspsProofingResultsJob, send suspected fraud failure email * changelog: Improvement, GetUspsProofingResultsJob, send suspected fraud failure email * update login.gov to use const instead of str --- app/jobs/get_usps_proofing_results_job.rb | 18 +++++++++++++- app/mailers/user_mailer.rb | 13 ++++++++++ .../usps_in_person_proofing/mock/fixtures.rb | 4 ++++ ...ected_fraud_proofing_results_response.json | 15 ++++++++++++ .../in_person_failed_fraud.html.erb | 24 +++++++++++++++++++ config/locales/user_mailer/en.yml | 10 ++++++++ config/locales/user_mailer/es.yml | 10 ++++++++ config/locales/user_mailer/fr.yml | 10 ++++++++ .../get_usps_proofing_results_job_spec.rb | 20 ++++++++++++++++ spec/mailers/previews/user_mailer_preview.rb | 8 +++++++ spec/mailers/user_mailer_spec.rb | 22 +++++++++++++++++ spec/support/usps_ipp_helper.rb | 14 +++++++++++ 12 files changed, 167 insertions(+), 1 deletion(-) create mode 100644 app/services/usps_in_person_proofing/mock/responses/request_failed_suspected_fraud_proofing_results_response.json create mode 100644 app/views/user_mailer/in_person_failed_fraud.html.erb diff --git a/app/jobs/get_usps_proofing_results_job.rb b/app/jobs/get_usps_proofing_results_job.rb index 6144e95d7d3..ed71d3e79f2 100644 --- a/app/jobs/get_usps_proofing_results_job.rb +++ b/app/jobs/get_usps_proofing_results_job.rb @@ -207,7 +207,11 @@ def handle_failed_status(enrollment, response) ) enrollment.update(status: :failed) - send_failed_email(enrollment.user, enrollment) + if response['fraudSuspected'] + send_failed_fraud_email(enrollment.user, enrollment) + else + send_failed_email(enrollment.user, enrollment) + end end def handle_successful_status_update(enrollment, response) @@ -269,6 +273,18 @@ def send_failed_email(user, enrollment) end end + def send_failed_fraud_email(user, enrollment) + user.confirmed_email_addresses.each do |email_address| + # rubocop:disable IdentityIdp/MailLaterLinter + UserMailer.in_person_failed_fraud( + user, + email_address, + enrollment: enrollment, + ).deliver_later(**mail_delivery_params) + # rubocop:enable IdentityIdp/MailLaterLinter + end + end + def mail_delivery_params config_delay = IdentityConfig.store.in_person_results_delay_in_hours if config_delay > 0 diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 9762a97430d..cbd8e1e9336 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -274,6 +274,19 @@ def in_person_failed(user, email_address, enrollment:) end end + def in_person_failed_fraud(user, email_address, enrollment:) + with_user_locale(user) do + @presenter = Idv::InPerson::VerificationResultsEmailPresenter.new( + enrollment: enrollment, + url_options: url_options, + ) + mail( + to: email_address.email, + subject: t('user_mailer.in_person_failed_suspected_fraud.subject'), + ) + end + end + private def email_should_receive_nonessential_notifications?(email) diff --git a/app/services/usps_in_person_proofing/mock/fixtures.rb b/app/services/usps_in_person_proofing/mock/fixtures.rb index 95d80dacde3..515ee89451e 100644 --- a/app/services/usps_in_person_proofing/mock/fixtures.rb +++ b/app/services/usps_in_person_proofing/mock/fixtures.rb @@ -37,6 +37,10 @@ def self.request_failed_proofing_results_response load_response_fixture('request_failed_proofing_results_response.json') end + def self.request_failed_suspected_fraud_proofing_results_response + load_response_fixture('request_failed_suspected_fraud_proofing_results_response.json') + end + def self.request_passed_proofing_unsupported_id_results_response load_response_fixture('request_passed_proofing_unsupported_id_results_response.json') end diff --git a/app/services/usps_in_person_proofing/mock/responses/request_failed_suspected_fraud_proofing_results_response.json b/app/services/usps_in_person_proofing/mock/responses/request_failed_suspected_fraud_proofing_results_response.json new file mode 100644 index 00000000000..67a574919ed --- /dev/null +++ b/app/services/usps_in_person_proofing/mock/responses/request_failed_suspected_fraud_proofing_results_response.json @@ -0,0 +1,15 @@ +{ + "status": "In-person failed", + "proofingPostOffice": "WILKES BARRE", + "proofingCity": "WILKES BARRE", + "proofingState": "PA", + "enrollmentCode": "2090002197604352", + "primaryIdType": "Uniformed Services identification card", + "transactionStartDateTime": "12/17/2020 033855", + "transactionEndDateTime": "12/17/2020 034055", + "secondaryIdType": "Deed of Trust", + "failureReason": "Clerk indicates that ID name or address does not match source data.", + "fraudSuspected": true, + "proofingConfirmationNumber": "350040248346707", + "ippAssuranceLevel": "1.5" + } \ No newline at end of file diff --git a/app/views/user_mailer/in_person_failed_fraud.html.erb b/app/views/user_mailer/in_person_failed_fraud.html.erb new file mode 100644 index 00000000000..479a9f202a0 --- /dev/null +++ b/app/views/user_mailer/in_person_failed_fraud.html.erb @@ -0,0 +1,24 @@ +<%= t('user_mailer.in_person_failed_suspected_fraud.greeting') %>
+

+ <%= t( + 'user_mailer.in_person_failed_suspected_fraud.body.intro', + app_name: APP_NAME, + location: @presenter.location_name, + date: @presenter.formatted_verified_date, + ) %> +

+

+ <%= t( + 'user_mailer.in_person_verified.warning_contact_us_html', + contact_us_url: MarketingSite.contact_url, + sign_in_url: root_url, + ) + %> +

+

+ <%= t( + 'user_mailer.in_person_failed_suspected_fraud.body.help_center_html', + help_center_url: MarketingSite.help_url, + ) + %> +

diff --git a/config/locales/user_mailer/en.yml b/config/locales/user_mailer/en.yml index 69c0fd2346a..6299a7c0576 100644 --- a/config/locales/user_mailer/en.yml +++ b/config/locales/user_mailer/en.yml @@ -118,6 +118,16 @@ en: verifying_step_proof_of_address: 'If you try to verify your identity in person again, you need to bring a valid proof of address if your current address is different than the address on your ID.' + in_person_failed_suspected_fraud: + body: + help_center_html: If you need further help, you can visit our Help Center or reach out to + the agency you are trying to access. + intro: We understand that you were attempting to verify your identity through + %{app_name}, however your identity could not be verified at the + %{location} Post Office on %{date}. + greeting: Hello, + subject: Your identity could not be verified in person in_person_ready_to_verify: greeting: Hi %{name}, intro: Here are the details to verify your identity in person at a United States diff --git a/config/locales/user_mailer/es.yml b/config/locales/user_mailer/es.yml index b143a0fed07..89b1b74432e 100644 --- a/config/locales/user_mailer/es.yml +++ b/config/locales/user_mailer/es.yml @@ -126,6 +126,16 @@ es: persona, deberá llevar un comprobante de domicilio vigente si su dirección actual es distinta de la que aparece en su documento de identidad. + in_person_failed_suspected_fraud: + body: + help_center_html: Si necesita ayuda adicional, puede visitar nuestro Centro de Ayuda o + ponerse en contacto con la agencia a la que intenta acceder. + intro: Entendemos que estaba intentando verificar su identidad a través de + %{app_name}, sin embargo, su identidad no pudo ser verificada en la + oficina de correos de %{location} el %{date}. + greeting: Hola, + subject: No se pudo verificar su identidad en persona in_person_ready_to_verify: greeting: 'Hola, %{name}:' intro: Estos son los detalles para verificar su identidad en persona en una diff --git a/config/locales/user_mailer/fr.yml b/config/locales/user_mailer/fr.yml index 598a18098bd..4a2063b1ec8 100644 --- a/config/locales/user_mailer/fr.yml +++ b/config/locales/user_mailer/fr.yml @@ -130,6 +130,16 @@ fr: identité en personne, vous devez apporter un justificatif de domicile valable si votre adresse actuelle est différente de celle figurant sur votre pièce d’identité. + in_person_failed_suspected_fraud: + body: + help_center_html: Si vous avez besoin d’aide supplémentaire, vous pouvez consulter notre centre d’aide ou + contacter l’agence à laquelle vous essayez d’accéder. + intro: Nous comprenons que vous avez tenté de vérifier votre identité par le + biais de %{app_name}, mais votre identité n’a pas pu être vérifiée au + bureau de poste de %{location} le %{date}. + greeting: Bonjour, + subject: Votre identité n’a pas pu être vérifiée en personne in_person_ready_to_verify: greeting: Bonjour %{name}, intro: Voici les détails pour vérifier votre identité en personne dans un bureau diff --git a/spec/jobs/get_usps_proofing_results_job_spec.rb b/spec/jobs/get_usps_proofing_results_job_spec.rb index f9ae51e14eb..3e1d32c1254 100644 --- a/spec/jobs/get_usps_proofing_results_job_spec.rb +++ b/spec/jobs/get_usps_proofing_results_job_spec.rb @@ -270,6 +270,26 @@ job.perform(Time.zone.now) end + it 'sends failed email when fraudSuspected is true' do + stub_request_failed_suspected_fraud_proofing_results + + mailer = instance_double(ActionMailer::MessageDelivery, deliver_later: true) + user = pending_enrollment.user + user.email_addresses.each do |email_address| + # it sends with the default delay + expect(mailer).to receive(:deliver_later).with(wait: 1.hour) + expect(UserMailer).to receive(:in_person_failed_fraud). + with( + user, + email_address, + enrollment: instance_of(InPersonEnrollment), + ). + and_return(mailer) + end + + job.perform(Time.zone.now) + end + it 'sends proofing verifed email on 2xx responses with valid JSON' do stub_request_passed_proofing_results diff --git a/spec/mailers/previews/user_mailer_preview.rb b/spec/mailers/previews/user_mailer_preview.rb index ba78853d779..1a71d660d8c 100644 --- a/spec/mailers/previews/user_mailer_preview.rb +++ b/spec/mailers/previews/user_mailer_preview.rb @@ -159,6 +159,14 @@ def in_person_failed ) end + def in_person_failed_fraud + UserMailer.in_person_failed_fraud( + user, + email_address_record, + enrollment: in_person_enrollment, + ) + end + private def user diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index d3053408dae..eb1d454ac83 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -601,6 +601,28 @@ def expect_email_body_to_have_help_and_contact_links it_behaves_like 'an email that respects user email locale preference' end + describe '#in_person_failed_fraud' do + let(:enrollment) do + create( + :in_person_enrollment, + selected_location_details: { name: 'FRIENDSHIP' }, + status_updated_at: Time.zone.now - 2.hours, + ) + end + + let(:mail) do + p enrollment + UserMailer.in_person_failed_fraud( + user, + user.email_addresses.first, + enrollment: enrollment, + ) + end + + it_behaves_like 'a system email' + it_behaves_like 'an email that respects user email locale preference' + end + def strip_tags(str) ActionController::Base.helpers.strip_tags(str) end diff --git a/spec/support/usps_ipp_helper.rb b/spec/support/usps_ipp_helper.rb index 6dd73f8bdf1..22286669ee8 100644 --- a/spec/support/usps_ipp_helper.rb +++ b/spec/support/usps_ipp_helper.rb @@ -60,6 +60,20 @@ def request_failed_proofing_results_args body: UspsInPersonProofing::Mock::Fixtures.request_failed_proofing_results_response } end + def stub_request_failed_suspected_fraud_proofing_results + stub_request(:post, %r{/ivs-ippaas-api/IPPRest/resources/rest/getProofingResults}).to_return( + **request_failed_suspected_fraud_proofing_results_args, + ) + end + + def request_failed_suspected_fraud_proofing_results_args + { + status: 200, + body: UspsInPersonProofing::Mock:: + Fixtures.request_failed_suspected_fraud_proofing_results_response, + } + end + def stub_request_passed_proofing_unsupported_id_results stub_request(:post, %r{/ivs-ippaas-api/IPPRest/resources/rest/getProofingResults}).to_return( status: 200,