From c388162bd0eaf7ae20d8e4209fcde263081e029b Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Mon, 27 Mar 2023 12:48:37 -0700 Subject: [PATCH 01/13] Remove realistic-looking example s3 bucket name (#8078) changelog: Internal, Source code, Update example bucket name --- spec/lib/deploy/activate_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/lib/deploy/activate_spec.rb b/spec/lib/deploy/activate_spec.rb index 9ee41e15865..5040626c424 100644 --- a/spec/lib/deploy/activate_spec.rb +++ b/spec/lib/deploy/activate_spec.rb @@ -78,12 +78,12 @@ subject.run expect(s3_client).to have_received(:get_object).with( - bucket: 'login-gov.secrets.12345-us-west-1', + bucket: kind_of(String), key: 'common/GeoIP2-City.mmdb', response_target: kind_of(String), ) expect(s3_client).to have_received(:get_object).with( - bucket: 'login-gov.secrets.12345-us-west-1', + bucket: kind_of(String), key: 'common/pwned-passwords.txt', response_target: kind_of(String), ) From fd702725ef5efe376e21c676a26d6462c035f901 Mon Sep 17 00:00:00 2001 From: Doug Price Date: Mon, 27 Mar 2023 16:28:31 -0400 Subject: [PATCH 02/13] LG-8868: Allow admin to designate LexisNexis Phone Finder outage. (#8073) Adds the vendor_status_lexisnexis_phone_finder flag, which when set to full_outage will force the user through the mail-only flow. The user is shown the mail only warning page prior to starting IdV. The hybrid flow is still available. changelog: User-Facing Improvements, Vendor outage, Phone Finder outage flag Co-authored-by: Eric Gade --- .../concerns/idv/verify_info_concern.rb | 2 +- app/presenters/idv/gpo_presenter.rb | 4 +-- app/services/outage_status.rb | 12 +++----- config/application.yml.default | 1 + lib/feature_management.rb | 4 ++- lib/identity_config.rb | 11 +++---- spec/features/idv/outage_spec.rb | 30 +++++++++++++++++++ 7 files changed, 47 insertions(+), 17 deletions(-) diff --git a/app/controllers/concerns/idv/verify_info_concern.rb b/app/controllers/concerns/idv/verify_info_concern.rb index 4f3dd6402a7..6fa9a7a28e8 100644 --- a/app/controllers/concerns/idv/verify_info_concern.rb +++ b/app/controllers/concerns/idv/verify_info_concern.rb @@ -123,7 +123,7 @@ def async_state_done(current_async_state) end def next_step_url - return idv_gpo_url if OutageStatus.new.gpo_only? + return idv_gpo_url if FeatureManagement.idv_gpo_only? idv_phone_url end diff --git a/app/presenters/idv/gpo_presenter.rb b/app/presenters/idv/gpo_presenter.rb index 9c7b210bb3e..62fd146b1c4 100644 --- a/app/presenters/idv/gpo_presenter.rb +++ b/app/presenters/idv/gpo_presenter.rb @@ -27,7 +27,7 @@ def resend_requested? end def back_or_cancel_partial - if OutageStatus.new.gpo_only? + if FeatureManagement.idv_gpo_only? 'idv/doc_auth/cancel' else 'idv/shared/back' @@ -35,7 +35,7 @@ def back_or_cancel_partial end def back_or_cancel_parameters - if OutageStatus.new.gpo_only? + if FeatureManagement.idv_gpo_only? { step: 'gpo' } else { fallback_path: fallback_back_path } diff --git a/app/services/outage_status.rb b/app/services/outage_status.rb index 33a7950ba54..290dbde4acc 100644 --- a/app/services/outage_status.rb +++ b/app/services/outage_status.rb @@ -19,6 +19,8 @@ def vendor_outage?(vendor) IdentityConfig.store.vendor_status_lexisnexis_instant_verify when :lexisnexis_trueid IdentityConfig.store.vendor_status_lexisnexis_trueid + when :lexisnexis_phone_finder + IdentityConfig.store.vendor_status_lexisnexis_phone_finder when :sms IdentityConfig.store.vendor_status_sms when :voice @@ -49,14 +51,8 @@ def all_phone_vendor_outage? all_vendor_outage?(PHONE_VENDORS) end - def gpo_only? - IdentityConfig.store.feature_idv_force_gpo_verification_enabled || - any_phone_vendor_outage? - end - - def allow_hybrid_flow? - IdentityConfig.store.feature_idv_hybrid_flow_enabled && - !any_phone_vendor_outage? + def phone_finder_outage? + all_vendor_outage?([:lexisnexis_phone_finder]) end def from_idv? diff --git a/config/application.yml.default b/config/application.yml.default index 10b639988ef..d068766dfc2 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -315,6 +315,7 @@ usps_upload_sftp_timeout: 5 valid_authn_contexts: '["http://idmanagement.gov/ns/assurance/loa/1", "http://idmanagement.gov/ns/assurance/loa/3", "http://idmanagement.gov/ns/assurance/ial/1", "http://idmanagement.gov/ns/assurance/ial/2", "http://idmanagement.gov/ns/assurance/ial/0", "http://idmanagement.gov/ns/assurance/ial/2?strict=true", "urn:gov:gsa:ac:classes:sp:PasswordProtectedTransport:duo", "http://idmanagement.gov/ns/assurance/aal/2", "http://idmanagement.gov/ns/assurance/aal/3", "http://idmanagement.gov/ns/assurance/aal/3?hspd12=true","http://idmanagement.gov/ns/assurance/aal/2?phishing_resistant=true","http://idmanagement.gov/ns/assurance/aal/2?hspd12=true"]' vendor_status_acuant: 'operational' vendor_status_lexisnexis_instant_verify: 'operational' +vendor_status_lexisnexis_phone_finder: 'operational' vendor_status_lexisnexis_trueid: 'operational' vendor_status_sms: 'operational' vendor_status_voice: 'operational' diff --git a/lib/feature_management.rb b/lib/feature_management.rb index edf08866ea5..4ab08c5fe96 100644 --- a/lib/feature_management.rb +++ b/lib/feature_management.rb @@ -158,7 +158,9 @@ def self.idv_allow_hybrid_flow? end def self.idv_gpo_only? + outage_status = OutageStatus.new IdentityConfig.store.feature_idv_force_gpo_verification_enabled || - OutageStatus.new.any_phone_vendor_outage? + outage_status.any_phone_vendor_outage? || + outage_status.phone_finder_outage? end end diff --git a/lib/identity_config.rb b/lib/identity_config.rb index 3a6a097a19b..2621a28db84 100644 --- a/lib/identity_config.rb +++ b/lib/identity_config.rb @@ -296,11 +296,6 @@ def self.build_store(config_map) config.add(:otps_per_ip_limit, type: :integer) config.add(:otps_per_ip_period, type: :integer) config.add(:otps_per_ip_track_only_mode, type: :boolean) - config.add(:vendor_status_acuant, type: :symbol, enum: VENDOR_STATUS_OPTIONS) - config.add(:vendor_status_lexisnexis_instant_verify, type: :symbol, enum: VENDOR_STATUS_OPTIONS) - config.add(:vendor_status_lexisnexis_trueid, type: :symbol, enum: VENDOR_STATUS_OPTIONS) - config.add(:vendor_status_sms, type: :symbol, enum: VENDOR_STATUS_OPTIONS) - config.add(:vendor_status_voice, type: :symbol, enum: VENDOR_STATUS_OPTIONS) config.add(:outbound_connection_check_retry_count, type: :integer) config.add(:outbound_connection_check_timeout, type: :integer) config.add(:outbound_connection_check_url) @@ -441,6 +436,12 @@ def self.build_store(config_map) config.add(:usps_upload_sftp_timeout, type: :integer) config.add(:usps_upload_sftp_username, type: :string) config.add(:valid_authn_contexts, type: :json) + config.add(:vendor_status_acuant, type: :symbol, enum: VENDOR_STATUS_OPTIONS) + config.add(:vendor_status_lexisnexis_instant_verify, type: :symbol, enum: VENDOR_STATUS_OPTIONS) + config.add(:vendor_status_lexisnexis_phone_finder, type: :symbol, enum: VENDOR_STATUS_OPTIONS) + config.add(:vendor_status_lexisnexis_trueid, type: :symbol, enum: VENDOR_STATUS_OPTIONS) + config.add(:vendor_status_sms, type: :symbol, enum: VENDOR_STATUS_OPTIONS) + config.add(:vendor_status_voice, type: :symbol, enum: VENDOR_STATUS_OPTIONS) config.add(:verification_errors_report_configs, type: :json) config.add(:verify_gpo_key_attempt_window_in_minutes, type: :integer) config.add(:verify_gpo_key_max_attempts, type: :integer) diff --git a/spec/features/idv/outage_spec.rb b/spec/features/idv/outage_spec.rb index 41a74c81ae3..e52db17f486 100644 --- a/spec/features/idv/outage_spec.rb +++ b/spec/features/idv/outage_spec.rb @@ -20,6 +20,36 @@ def sign_in_with_idv_required(user:, sms_or_totp: :sms) let(:new_password) { 'some really awesome new password' } let(:pii) { { ssn: '666-66-1234', dob: '1920-01-01', first_name: 'alice' } } + context 'vendor_status_lexisnexis_phone_finder set to full_outage', js: true do + before do + allow(IdentityConfig.store).to receive(:vendor_status_lexisnexis_phone_finder). + and_return(:full_outage) + end + + it 'takes the user through the mail only flow, allowing hybrid' do + sign_in_with_idv_required(user: user) + + expect(current_path).to eq idv_mail_only_warning_path + + click_idv_continue + + expect(current_path).to eq idv_doc_auth_step_path(step: :welcome) + + complete_welcome_step + complete_agreement_step + + # Still offer the option for hybrid flow + expect(current_path).to eq idv_doc_auth_step_path(step: :upload) + + complete_upload_step + complete_document_capture_step + complete_ssn_step + complete_verify_step + + expect(current_path).to eq idv_gpo_path + end + end + context 'GPO only enabled, but user starts over', js: true do before do allow(IdentityConfig.store).to receive(:feature_idv_force_gpo_verification_enabled). From 3346dc18d63de9ba2e76053707b2c00ce60ec159 Mon Sep 17 00:00:00 2001 From: Matt Wagner Date: Mon, 27 Mar 2023 21:35:27 -0400 Subject: [PATCH 03/13] LG-8664 | Log Attempt Event for fraud account ops (#7951) changelog: Internal, Attempts API, Log events for fraud rejection (LG-8664) --------- Co-authored-by: Jonathan Hooper Co-authored-by: Zach Margolis --- .../concerns/idv/verify_info_concern.rb | 2 +- app/models/fraud_review_request.rb | 5 + app/models/phone_number_opt_out.rb | 2 +- app/models/profile.rb | 34 +++++ app/models/user.rb | 1 + app/services/cloud_front_header_parser.rb | 1 + .../idv/steps/threat_metrix_step_helper.rb | 22 +-- app/services/irs_attempts_api/tracker.rb | 2 +- .../irs_attempts_api/tracker_events.rb | 13 ++ config/application.yml.default | 1 + ...0322000756_create_fraud_review_requests.rb | 14 ++ db/schema.rb | 12 +- lib/identity_config.rb | 1 + spec/models/profile_spec.rb | 130 ++++++++++++++++++ .../cloud_front_header_parser_spec.rb | 18 ++- 15 files changed, 245 insertions(+), 13 deletions(-) create mode 100644 app/models/fraud_review_request.rb create mode 100644 db/primary_migrate/20230322000756_create_fraud_review_requests.rb diff --git a/app/controllers/concerns/idv/verify_info_concern.rb b/app/controllers/concerns/idv/verify_info_concern.rb index 6fa9a7a28e8..2cbfd63dd82 100644 --- a/app/controllers/concerns/idv/verify_info_concern.rb +++ b/app/controllers/concerns/idv/verify_info_concern.rb @@ -221,7 +221,7 @@ def add_proofing_costs(results) elsif stage == :threatmetrix # transaction_id comes from request_id tmx_id = hash[:transaction_id] - log_irs_tmx_fraud_check_event(hash) if tmx_id + log_irs_tmx_fraud_check_event(hash, current_user) if tmx_id add_cost(:threatmetrix, transaction_id: tmx_id) if tmx_id end end diff --git a/app/models/fraud_review_request.rb b/app/models/fraud_review_request.rb new file mode 100644 index 00000000000..d4f8a39325a --- /dev/null +++ b/app/models/fraud_review_request.rb @@ -0,0 +1,5 @@ +class FraudReviewRequest < ApplicationRecord + include NonNullUuid + + belongs_to :user +end diff --git a/app/models/phone_number_opt_out.rb b/app/models/phone_number_opt_out.rb index 5f85cd2bff8..868e26c5781 100644 --- a/app/models/phone_number_opt_out.rb +++ b/app/models/phone_number_opt_out.rb @@ -1,4 +1,4 @@ -# Represents a record of a phone number that has beed opted out of SMS in AWS Pinpoint +# Represents a record of a phone number that has been opted out of SMS in AWS Pinpoint # AWS maintains separate opt-out lists per region, so this helps us keep track across regions class PhoneNumberOptOut < ApplicationRecord include NonNullUuid diff --git a/app/models/profile.rb b/app/models/profile.rb index 09c5b62a085..66ac064e4ad 100644 --- a/app/models/profile.rb +++ b/app/models/profile.rb @@ -49,6 +49,7 @@ def activate def activate_after_passing_review update!(fraud_review_pending: false, fraud_rejection: false) + track_fraud_review_adjudication(decision: 'pass') activate end @@ -62,6 +63,9 @@ def deactivate_for_fraud_review def reject_for_fraud(notify_user:) update!(active: false, fraud_review_pending: false, fraud_rejection: true) + track_fraud_review_adjudication( + decision: notify_user ? 'manual_reject' : 'automatic_reject', + ) UserAlerts::AlertUserAboutAccountRejected.call(user) if notify_user end @@ -120,8 +124,38 @@ def has_proofed_before? Profile.where(user_id: user_id).where.not(activated_at: nil).where.not(id: self.id).exists? end + def irs_attempts_api_tracker + @irs_attempts_api_tracker ||= IrsAttemptsApi::Tracker.new( + session_id: nil, + request: nil, + user: user, + sp: initiating_service_provider, + cookie_device_uuid: nil, + sp_request_uri: nil, + enabled_for_session: initiating_service_provider&.irs_attempts_api_enabled?, + analytics: Analytics.new( + user: user, + request: nil, + sp: initiating_service_provider&.issuer, + session: {}, + ahoy: nil, + ), + ) + end + private + def track_fraud_review_adjudication(decision:) + if IdentityConfig.store.irs_attempt_api_track_idv_fraud_review + fraud_review_request = user.fraud_review_requests.last + irs_attempts_api_tracker.fraud_review_adjudicated( + decision: decision, + cached_irs_session_id: fraud_review_request&.irs_session_id, + cached_login_session_id: fraud_review_request&.login_session_id, + ) + end + end + def personal_key_generator @personal_key_generator ||= PersonalKeyGenerator.new(user) end diff --git a/app/models/user.rb b/app/models/user.rb index fb50ed5bd1e..740db97ce74 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -44,6 +44,7 @@ class User < ApplicationRecord source: :service_provider_record has_many :sign_in_restrictions, dependent: :destroy has_many :in_person_enrollments, dependent: :destroy + has_many :fraud_review_requests, dependent: :destroy has_one :pending_in_person_enrollment, -> { where(status: :pending).order(created_at: :desc) }, diff --git a/app/services/cloud_front_header_parser.rb b/app/services/cloud_front_header_parser.rb index 11392f4dd13..f350ae2d38a 100644 --- a/app/services/cloud_front_header_parser.rb +++ b/app/services/cloud_front_header_parser.rb @@ -10,6 +10,7 @@ def client_port # Source IP and port for client connection to CloudFront def viewer_address + return nil unless @request&.headers @request.headers['CloudFront-Viewer-Address'] end end diff --git a/app/services/idv/steps/threat_metrix_step_helper.rb b/app/services/idv/steps/threat_metrix_step_helper.rb index 62eae9de24a..143067ed286 100644 --- a/app/services/idv/steps/threat_metrix_step_helper.rb +++ b/app/services/idv/steps/threat_metrix_step_helper.rb @@ -47,18 +47,24 @@ def threatmetrix_iframe_url(session_id) ) end - def log_irs_tmx_fraud_check_event(result) + def log_irs_tmx_fraud_check_event(result, user) return unless IdentityConfig.store.irs_attempt_api_track_tmx_fraud_check_event return unless FeatureManagement.proofing_device_profiling_collecting_enabled? + success = result[:review_status] == 'pass' - if !success && (tmx_summary_reason_code = result.dig( - :response_body, - :tmx_summary_reason_code, - )) - failure_reason = { - tmx_summary_reason_code: tmx_summary_reason_code, - } + unless success + FraudReviewRequest.create( + user: user, + irs_session_id: irs_attempts_api_session_id, + login_session_id: Digest::SHA1.hexdigest(user.unique_session_id.to_s), + ) + + if (tmx_summary_reason_code = result.dig(:response_body, :tmx_summary_reason_code)) + failure_reason = { + tmx_summary_reason_code: tmx_summary_reason_code, + } + end end irs_attempts_api_tracker.idv_tmx_fraud_check( diff --git a/app/services/irs_attempts_api/tracker.rb b/app/services/irs_attempts_api/tracker.rb index a4d64d4a07c..2067cf4cbda 100644 --- a/app/services/irs_attempts_api/tracker.rb +++ b/app/services/irs_attempts_api/tracker.rb @@ -16,7 +16,7 @@ def initialize(session_id:, request:, user:, sp:, cookie_device_uuid:, end def track_event(event_type, metadata = {}) - return if !enabled? + return unless enabled? if metadata.has_key?(:failure_reason) && (metadata[:failure_reason].blank? || diff --git a/app/services/irs_attempts_api/tracker_events.rb b/app/services/irs_attempts_api/tracker_events.rb index 4338a16b002..1f4eb829fa6 100644 --- a/app/services/irs_attempts_api/tracker_events.rb +++ b/app/services/irs_attempts_api/tracker_events.rb @@ -67,6 +67,19 @@ def forgot_password_new_password_submitted(success:, failure_reason: nil) ) end + # @param [String] decision One of 'pass', 'manual_reject', or 'automated_reject' + # @param [String] cached_irs_session_id The IRS session id ('tid') the user had when flagged + # @param [String] cached_login_session_id The Login.gov session id the user had when flagged + # A profile offlined for review has been approved or rejected. + def fraud_review_adjudicated(decision:, cached_irs_session_id:, cached_login_session_id:) + track_event( + :fraud_review_adjudicated, + decision: decision, + cached_irs_session_id: cached_irs_session_id, + cached_login_session_id: cached_login_session_id, + ) + end + # @param ["mobile", "desktop"] upload_method method chosen for uploading id verification # A user has selected id document upload method def idv_document_upload_method_selected(upload_method:) diff --git a/config/application.yml.default b/config/application.yml.default index d068766dfc2..dc234f1d352 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -154,6 +154,7 @@ irs_attempt_api_event_ttl_seconds: 86400 irs_attempt_api_event_count_default: 1000 irs_attempt_api_event_count_max: 10000 irs_attempt_api_payload_size_logging_enabled: true +irs_attempt_api_track_idv_fraud_review: false irs_attempt_api_track_tmx_fraud_check_event: false key_pair_generation_percent: 0 logins_per_ip_track_only_mode: false diff --git a/db/primary_migrate/20230322000756_create_fraud_review_requests.rb b/db/primary_migrate/20230322000756_create_fraud_review_requests.rb new file mode 100644 index 00000000000..24116a3e74b --- /dev/null +++ b/db/primary_migrate/20230322000756_create_fraud_review_requests.rb @@ -0,0 +1,14 @@ +class CreateFraudReviewRequests < ActiveRecord::Migration[7.0] + def change + create_table :fraud_review_requests do |t| + t.integer :user_id + t.string :uuid + t.string :irs_session_id + t.string :login_session_id + + t.timestamps + + t.index :user_id, unique: false + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 823c35bdb60..5dce1006baa 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2023_03_20_235607) do +ActiveRecord::Schema[7.0].define(version: 2023_03_22_000756) do # These are extensions that must be enabled in order to support this database enable_extension "pg_stat_statements" enable_extension "pgcrypto" @@ -226,6 +226,16 @@ t.index ["user_id", "created_at"], name: "index_events_on_user_id_and_created_at" end + create_table "fraud_review_requests", force: :cascade do |t| + t.integer "user_id" + t.string "uuid" + t.string "irs_session_id" + t.string "login_session_id" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["user_id"], name: "index_fraud_review_requests_on_user_id" + end + create_table "iaa_gtcs", force: :cascade do |t| t.string "gtc_number", null: false t.integer "mod_number", default: 0, null: false diff --git a/lib/identity_config.rb b/lib/identity_config.rb index 2621a28db84..0d72855d181 100644 --- a/lib/identity_config.rb +++ b/lib/identity_config.rb @@ -234,6 +234,7 @@ def self.build_store(config_map) config.add(:irs_attempt_api_event_count_default, type: :integer) config.add(:irs_attempt_api_event_count_max, type: :integer) config.add(:irs_attempt_api_payload_size_logging_enabled, type: :boolean) + config.add(:irs_attempt_api_track_idv_fraud_review, type: :boolean) config.add(:irs_attempt_api_track_tmx_fraud_check_event, type: :boolean) config.add(:irs_attempt_api_public_key) config.add(:irs_attempt_api_public_key_id) diff --git a/spec/models/profile_spec.rb b/spec/models/profile_spec.rb index b79ceaf2990..f701a0c7b99 100644 --- a/spec/models/profile_spec.rb +++ b/spec/models/profile_spec.rb @@ -299,6 +299,69 @@ expect(profile).to be_active end + + context 'when the initiating_sp is the IRS' do + let(:sp) { create(:service_provider, :irs) } + let(:profile) do + create( + :profile, + user: user, + active: false, + fraud_review_pending: true, + initiating_service_provider: sp, + ) + end + + context 'when the feature flag is enabled' do + before do + allow(IdentityConfig.store).to receive(:irs_attempt_api_track_idv_fraud_review). + and_return(true) + end + + it 'logs an attempt event' do + allow(IdentityConfig.store).to receive(:irs_attempt_api_enabled).and_return(true) + expect(profile.initiating_service_provider.irs_attempts_api_enabled?).to be_truthy + + expect(profile.irs_attempts_api_tracker).to receive(:fraud_review_adjudicated). + with( + hash_including(decision: 'pass'), + ) + profile.activate_after_passing_review + end + end + + context 'when the feature flag is disabled' do + before do + allow(IdentityConfig.store).to receive(:irs_attempt_api_track_idv_fraud_review). + and_return(false) + end + + it 'does not log an attempt event' do + allow(IdentityConfig.store).to receive(:irs_attempt_api_enabled).and_return(true) + expect(profile.initiating_service_provider.irs_attempts_api_enabled?).to be_truthy + + expect(profile.irs_attempts_api_tracker).not_to receive(:fraud_review_adjudicated) + profile.activate_after_passing_review + end + end + end + + context 'when the initiating_sp is not the IRS' do + it 'does not log an attempt event' do + sp = create(:service_provider) + profile = create( + :profile, + user: user, + active: false, + fraud_review_pending: true, + initiating_service_provider: sp, + ) + expect(profile.initiating_service_provider.irs_attempts_api_enabled?).to be_falsey + + expect(profile.irs_attempts_api_tracker).not_to receive(:fraud_review_adjudicated) + profile.activate_after_passing_review + end + end end describe '#deactivate_for_fraud_review' do @@ -353,6 +416,73 @@ expect { profile }.to change(ActionMailer::Base.deliveries, :count).by(0) end end + + context 'when the SP is the IRS' do + let(:sp) { create(:service_provider, :irs) } + let(:profile) do + create( + :profile, + user: user, + active: false, + fraud_review_pending: true, + initiating_service_provider: sp, + ) + end + + context 'and notify_user is true' do + it 'logs an event with manual_reject' do + allow(IdentityConfig.store).to receive(:irs_attempt_api_enabled).and_return(true) + allow(IdentityConfig.store).to receive(:irs_attempt_api_track_idv_fraud_review). + and_return(true) + + expect(profile.initiating_service_provider.irs_attempts_api_enabled?).to be_truthy + + expect(profile.irs_attempts_api_tracker).to receive(:fraud_review_adjudicated). + with( + hash_including(decision: 'manual_reject'), + ) + + profile.reject_for_fraud(notify_user: true) + end + end + + context 'and notify_user is false' do + it 'logs an event with automatic_reject' do + allow(IdentityConfig.store).to receive(:irs_attempt_api_enabled).and_return(true) + allow(IdentityConfig.store).to receive(:irs_attempt_api_track_idv_fraud_review). + and_return(true) + + expect(profile.initiating_service_provider.irs_attempts_api_enabled?).to be_truthy + + expect(profile.irs_attempts_api_tracker).to receive(:fraud_review_adjudicated). + with( + hash_including(decision: 'automatic_reject'), + ) + + profile.reject_for_fraud(notify_user: false) + end + end + end + + context 'when the SP is not the IRS' do + it 'does not log an event' do + sp = create(:service_provider) + profile = create( + :profile, + user: user, + active: false, + fraud_review_pending: true, + initiating_service_provider: sp, + ) + allow(IdentityConfig.store).to receive(:irs_attempt_api_enabled).and_return(true) + + expect(profile.initiating_service_provider.irs_attempts_api_enabled?).to be_falsey + + expect(profile.irs_attempts_api_tracker).not_to receive(:fraud_review_adjudicated) + + profile.reject_for_fraud(notify_user: true) + end + end end describe 'scopes' do diff --git a/spec/services/cloud_front_header_parser_spec.rb b/spec/services/cloud_front_header_parser_spec.rb index 159bd2df848..927c084fee7 100644 --- a/spec/services/cloud_front_header_parser_spec.rb +++ b/spec/services/cloud_front_header_parser_spec.rb @@ -39,7 +39,23 @@ describe '#client_port' do it 'returns nil' do - expect(subject.client_port).to eq nil + expect(subject.client_port).to be nil + end + end + end + + context 'with no request included' do + let(:req) { nil } + + describe '#viewer_address' do + it 'returns nil' do + expect(subject.viewer_address).to be nil + end + end + + describe '#client_port' do + it 'returns nil' do + expect(subject.client_port).to be nil end end end From 3cef5891c83bca8517c5aa7bf6099f9d4bb4442e Mon Sep 17 00:00:00 2001 From: jc-gsa <104452882+jc-gsa@users.noreply.github.com> Date: Tue, 28 Mar 2023 07:01:39 -0500 Subject: [PATCH 04/13] LG-9136: Add sign in attempt count to analytics (#8036) * Add bad_password_count to analytics event email_and_password_auth [skip changelog] --- app/controllers/users/sessions_controller.rb | 4 +-- app/services/analytics_events.rb | 3 ++ .../users/sessions_controller_spec.rb | 33 +++++++++++++++++++ 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index f10861b521c..5d3971a1854 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -32,8 +32,6 @@ def new end def create - track_authentication_attempt(auth_params[:email]) - return process_locked_out_session if session_bad_password_count_max_exceeded? return process_locked_out_user if current_user && user_locked_out?(current_user) @@ -42,6 +40,7 @@ def create handle_valid_authentication ensure increment_session_bad_password_count if throttle_password_failure && !current_user + track_authentication_attempt(auth_params[:email]) end def destroy @@ -175,6 +174,7 @@ def track_authentication_attempt(email) success: success, user_id: user.uuid, user_locked_out: user_locked_out?(user), + bad_password_count: session[:bad_password_count].to_i, stored_location: session['user_return_to'], sp_request_url_present: sp_session[:request_url].present?, remember_device: remember_device_cookie.present?, diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index 64a89d83671..af285402965 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -240,6 +240,7 @@ def doc_auth_warning(message: nil, **extra) # @param [Boolean] success # @param [String] user_id # @param [Boolean] user_locked_out if the user is currently locked out of their second factor + # @param [String] bad_password_count represents number of prior login failures # @param [String] stored_location the URL to return to after signing in # @param [Boolean] sp_request_url_present if was an SP request URL in the session # @param [Boolean] remember_device if the remember device cookie was present @@ -248,6 +249,7 @@ def email_and_password_auth( success:, user_id:, user_locked_out:, + bad_password_count:, stored_location:, sp_request_url_present:, remember_device:, @@ -258,6 +260,7 @@ def email_and_password_auth( success: success, user_id: user_id, user_locked_out: user_locked_out, + bad_password_count: bad_password_count, stored_location: stored_location, sp_request_url_present: sp_request_url_present, remember_device: remember_device, diff --git a/spec/controllers/users/sessions_controller_spec.rb b/spec/controllers/users/sessions_controller_spec.rb index c090c1d0832..0aa82d80071 100644 --- a/spec/controllers/users/sessions_controller_spec.rb +++ b/spec/controllers/users/sessions_controller_spec.rb @@ -193,6 +193,7 @@ describe 'POST /' do include AccountResetHelper + it 'tracks the successful authentication for existing user' do user = create(:user, :signed_up) subject.session['user_return_to'] = mock_valid_site @@ -203,6 +204,7 @@ success: true, user_id: user.uuid, user_locked_out: false, + bad_password_count: 0, stored_location: mock_valid_site, sp_request_url_present: false, remember_device: false, @@ -225,6 +227,7 @@ success: false, user_id: user.uuid, user_locked_out: false, + bad_password_count: 1, stored_location: nil, sp_request_url_present: false, remember_device: false, @@ -243,6 +246,7 @@ success: false, user_id: 'anonymous-uuid', user_locked_out: false, + bad_password_count: 1, stored_location: nil, sp_request_url_present: false, remember_device: false, @@ -281,6 +285,7 @@ success: false, user_id: user.uuid, user_locked_out: true, + bad_password_count: 0, stored_location: nil, sp_request_url_present: false, remember_device: false, @@ -292,6 +297,30 @@ post :create, params: { user: { email: user.email.upcase, password: user.password } } end + it 'tracks count of multiple unsuccessful authentication attempts' do + user = create( + :user, + :signed_up, + ) + + stub_analytics + + analytics_hash = { + success: false, + user_id: user.uuid, + user_locked_out: false, + bad_password_count: 2, + stored_location: nil, + sp_request_url_present: false, + remember_device: false, + } + + post :create, params: { user: { email: user.email.upcase, password: 'invalid' } } + expect(@analytics).to receive(:track_event). + with('Email and Password Authentication', analytics_hash) + post :create, params: { user: { email: user.email.upcase, password: 'invalid' } } + end + it 'tracks the presence of SP request_url in session' do subject.session[:sp] = { request_url: mock_valid_site } stub_analytics @@ -299,6 +328,7 @@ success: false, user_id: 'anonymous-uuid', user_locked_out: false, + bad_password_count: 1, stored_location: nil, sp_request_url_present: true, remember_device: false, @@ -368,6 +398,7 @@ success: true, user_id: user.uuid, user_locked_out: false, + bad_password_count: 0, stored_location: nil, sp_request_url_present: false, remember_device: false, @@ -440,6 +471,7 @@ success: true, user_id: user.uuid, user_locked_out: false, + bad_password_count: 0, stored_location: nil, sp_request_url_present: false, remember_device: true, @@ -465,6 +497,7 @@ success: true, user_id: user.uuid, user_locked_out: false, + bad_password_count: 0, stored_location: nil, sp_request_url_present: false, remember_device: true, From fef6075503df6675f86ce148bec12895959209e2 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 28 Mar 2023 09:15:28 -0400 Subject: [PATCH 05/13] Update document capture to use request utility (#8072) * Add read option to request module to allow raw response * Update document capture to use request utility changelog: Bug Fixes, Form Submission, Refresh form authenticity token when user's session is renewed with "Keep me signed in" * Remove unused union type From earlier iteration * Update upload-spec.jsx * Update upload-spec.js * Document capture is not JSON --- .../document-capture/context/upload.tsx | 19 +-- .../document-capture/services/upload.ts | 14 +- app/javascript/packages/request/index.spec.ts | 31 ++++ app/javascript/packages/request/index.ts | 31 +++- app/javascript/packs/document-capture.tsx | 9 +- .../components/document-capture-spec.jsx | 35 ++-- .../document-capture/context/upload-spec.jsx | 11 +- .../document-capture/services/upload-spec.js | 149 +++++------------- 8 files changed, 126 insertions(+), 173 deletions(-) diff --git a/app/javascript/packages/document-capture/context/upload.tsx b/app/javascript/packages/document-capture/context/upload.tsx index 030309694d6..9f30da741d2 100644 --- a/app/javascript/packages/document-capture/context/upload.tsx +++ b/app/javascript/packages/document-capture/context/upload.tsx @@ -12,7 +12,6 @@ const UploadContext = createContext({ backgroundUploadURLs: {} as Record, backgroundUploadEncryptKey: undefined as CryptoKey | undefined, flowPath: 'standard' as FlowPath, - csrf: null as string | null, formData: {} as Record, }); @@ -45,11 +44,6 @@ interface UploadOptions { * Endpoint to which payload should be sent. */ endpoint: string; - - /** - * CSRF token to send as parameter to upload implementation. - */ - csrf: string | null; } export interface UploadSuccessResponse { @@ -142,11 +136,6 @@ interface UploadContextProviderProps { */ statusPollInterval?: number; - /** - * CSRF token to send as parameter to upload implementation. - */ - csrf: string | null; - /** * Extra form data to merge into the payload before uploading */ @@ -177,27 +166,25 @@ function UploadContextProvider({ endpoint, statusEndpoint, statusPollInterval, - csrf, formData = DEFAULT_FORM_DATA, flowPath, children, }: UploadContextProviderProps) { - const uploadWithCSRF = (payload) => upload({ ...payload, ...formData }, { endpoint, csrf }); + const uploadWithFormData = (payload) => upload({ ...payload, ...formData }, { endpoint }); const getStatus = () => statusEndpoint - ? upload({ ...formData }, { endpoint: statusEndpoint, method: 'PUT', csrf }) + ? upload({ ...formData }, { endpoint: statusEndpoint, method: 'PUT' }) : Promise.reject(); const value = useObjectMemo({ - upload: uploadWithCSRF, + upload: uploadWithFormData, getStatus, statusPollInterval, backgroundUploadURLs, backgroundUploadEncryptKey, isMockClient, flowPath, - csrf, formData, }); diff --git a/app/javascript/packages/document-capture/services/upload.ts b/app/javascript/packages/document-capture/services/upload.ts index 84b98fd1935..9384286cd61 100644 --- a/app/javascript/packages/document-capture/services/upload.ts +++ b/app/javascript/packages/document-capture/services/upload.ts @@ -1,5 +1,6 @@ import { FormError } from '@18f/identity-form-steps'; import { forceRedirect } from '@18f/identity-url'; +import { request } from '@18f/identity-request'; import type { UploadSuccessResponse, UploadErrorResponse, @@ -67,12 +68,13 @@ export function toFormEntryError(uploadFieldError: UploadFieldError): UploadForm return formEntryError; } -const upload: UploadImplementation = async function (payload, { method = 'POST', endpoint, csrf }) { - const headers: HeadersInit = {}; - if (csrf) { - headers['X-CSRF-Token'] = csrf; - } - const response = await window.fetch(endpoint, { method, headers, body: toFormData(payload) }); +const upload: UploadImplementation = async function (payload, { method = 'POST', endpoint }) { + const response = await request(endpoint, { + method, + body: toFormData(payload), + json: false, + read: false, + }); if (!response.ok && !response.status.toString().startsWith('4')) { // 4xx is an expected error state, handled after JSON deserialization. Anything else not OK diff --git a/app/javascript/packages/request/index.spec.ts b/app/javascript/packages/request/index.spec.ts index 7daf28d5500..40eecb5cfcb 100644 --- a/app/javascript/packages/request/index.spec.ts +++ b/app/javascript/packages/request/index.spec.ts @@ -148,6 +148,37 @@ describe('request', () => { }); }); + context('with read=false option', () => { + it('returns the raw response', async () => { + sandbox.stub(window, 'fetch').resolves(new Response(JSON.stringify({}))); + const response = await request('https://example.com', { read: false }); + expect(response.status).to.equal(200); + }); + }); + + context('with unsuccessful response', () => { + beforeEach(() => { + sandbox.stub(window, 'fetch').resolves(new Response(JSON.stringify({}), { status: 400 })); + }); + + it('throws an error', async () => { + await request('https://example.com', { read: false }) + .then(() => { + throw new Error('Unexpected promise resolution'); + }) + .catch((error) => { + expect(error).to.exist(); + }); + }); + + context('with read=false option', () => { + it('returns the raw response', async () => { + const response = await request('https://example.com', { read: false }); + expect(response.status).to.equal(400); + }); + }); + }); + context('with response including csrf token', () => { beforeEach(() => { sandbox.stub(window, 'fetch').callsFake(() => diff --git a/app/javascript/packages/request/index.ts b/app/javascript/packages/request/index.ts index c7b518152e1..e577ec79606 100644 --- a/app/javascript/packages/request/index.ts +++ b/app/javascript/packages/request/index.ts @@ -7,9 +7,15 @@ interface RequestOptions extends RequestInit { json?: object | boolean; /** - * Whether to include CSRF token in the request. Defaults to true. + * Whether to include the default CSRF token in the request, or use a custom implementation to + * retrieve a CSRF token. Defaults to true. */ csrf?: boolean | CSRFGetter; + + /** + * Whether to automatically read the response as JSON or text. Defaults to true. + */ + read?: boolean; } class CSRF { @@ -49,10 +55,15 @@ class CSRF { } export async function request( - url: string, - options: Partial = {}, -): Promise { - const { csrf = true, json = true, ...fetchOptions } = options; + url, + options?: Partial & { read?: true }, +): Promise; +export async function request( + url, + options?: Partial & { read?: false }, +): Promise; +export async function request(url: string, options: Partial = {}) { + const { csrf = true, json = true, read = true, ...fetchOptions } = options; let { body, headers } = fetchOptions; headers = new Headers(headers); @@ -76,9 +87,13 @@ export async function request( const response = await window.fetch(url, { ...fetchOptions, headers, body }); CSRF.token = response.headers.get('X-CSRF-Token'); - if (!response.ok) { - throw new Error(); + if (read) { + if (!response.ok) { + throw new Error(); + } + + return json ? response.json() : response.text(); } - return json ? response.json() : response.text(); + return response; } diff --git a/app/javascript/packs/document-capture.tsx b/app/javascript/packs/document-capture.tsx index 53396309284..13d4d9655ed 100644 --- a/app/javascript/packs/document-capture.tsx +++ b/app/javascript/packs/document-capture.tsx @@ -14,6 +14,7 @@ import { import { isCameraCapableMobile } from '@18f/identity-device'; import { FlowContext } from '@18f/identity-verify-flow'; import { trackEvent as baseTrackEvent } from '@18f/identity-analytics'; +import { request } from '@18f/identity-request'; import type { FlowPath, DeviceContextValue } from '@18f/identity-document-capture'; /** @@ -81,7 +82,6 @@ const trackEvent: typeof baseTrackEvent = (event, payload) => { (async () => { const backgroundUploadURLs = getBackgroundUploadURLs(); const isAsyncForm = Object.keys(backgroundUploadURLs).length > 0; - const csrf = getMetaContent('csrf-token'); const formData: Record = { document_capture_session_uuid: appRoot.getAttribute('data-document-capture-session-uuid'), @@ -104,11 +104,7 @@ const trackEvent: typeof baseTrackEvent = (event, payload) => { formData.step = 'verify_document'; } - const keepAlive = () => - window.fetch(keepAliveEndpoint, { - method: 'POST', - headers: [csrf && ['X-CSRF-Token', csrf]].filter(Boolean) as [string, string][], - }); + const keepAlive = () => request(keepAliveEndpoint, { method: 'POST' }); const { helpCenterRedirectUrl: helpCenterRedirectURL, @@ -155,7 +151,6 @@ const trackEvent: typeof baseTrackEvent = (event, payload) => { endpoint: String(appRoot.getAttribute('data-endpoint')), statusEndpoint: String(appRoot.getAttribute('data-status-endpoint')), statusPollInterval: Number(appRoot.getAttribute('data-status-poll-interval-ms')), - csrf, isMockClient, backgroundUploadURLs, backgroundUploadEncryptKey, diff --git a/spec/javascripts/packages/document-capture/components/document-capture-spec.jsx b/spec/javascripts/packages/document-capture/components/document-capture-spec.jsx index 452a7d163a9..9d4e419ab29 100644 --- a/spec/javascripts/packages/document-capture/components/document-capture-spec.jsx +++ b/spec/javascripts/packages/document-capture/components/document-capture-spec.jsx @@ -275,15 +275,12 @@ describe('document-capture/components/document-capture', () => { sandbox .stub(window, 'fetch') .withArgs(endpoint) - .resolves({ - ok: false, - status: 418, - url: endpoint, - json: () => - Promise.resolve({ - redirect: '#teapot', - }), - }); + .resolves( + new Response(JSON.stringify({ redirect: '#teapot' }), { + status: 418, + url: endpoint, + }), + ); const frontImage = getByLabelText('doc_auth.headings.document_capture_front'); const backImage = getByLabelText('doc_auth.headings.document_capture_back'); @@ -305,7 +302,7 @@ describe('document-capture/components/document-capture', () => { it('renders async upload pending progress', async () => { const statusChecks = 3; let remainingStatusChecks = statusChecks; - sandbox.stub(window, 'fetch').resolves({ ok: true, headers: new window.Headers() }); + sandbox.stub(window, 'fetch').resolves(new Response()); const upload = sinon.stub().callsFake((payload, { endpoint }) => { switch (endpoint) { case 'about:blank#upload': @@ -393,13 +390,11 @@ describe('document-capture/components/document-capture', () => { sandbox.stub(window, 'fetch'); window.fetch.withArgs('about:blank#front').returns( new Promise((resolve, reject) => { - completeUploadAsSuccess = () => resolve({ ok: true, headers: new window.Headers() }); + completeUploadAsSuccess = () => resolve(new Response()); completeUploadAsFailure = () => reject(new Error()); }), ); - window.fetch - .withArgs('about:blank#back') - .resolves({ ok: true, headers: new window.Headers() }); + window.fetch.withArgs('about:blank#back').resolves(new Response()); upload = sinon.stub().resolves({ success: true, isPending: false }); const key = await window.crypto.subtle.generateKey( { @@ -514,12 +509,12 @@ describe('document-capture/components/document-capture', () => { sandbox .stub(window, 'fetch') .withArgs(endpoint) - .resolves({ - ok: false, - status: 400, - url: endpoint, - json: () => ({ success: false, remaining_attempts: 1, errors: [{}] }), - }); + .resolves( + new Response(JSON.stringify({ success: false, remaining_attempts: 1, errors: [{}] }), { + status: 400, + url: endpoint, + }), + ); expect(queryByText('idv.troubleshooting.options.verify_in_person')).not.to.exist(); await userEvent.click(getByText('forms.buttons.submit.default')); diff --git a/spec/javascripts/packages/document-capture/context/upload-spec.jsx b/spec/javascripts/packages/document-capture/context/upload-spec.jsx index f840e6c41db..40e22c82b50 100644 --- a/spec/javascripts/packages/document-capture/context/upload-spec.jsx +++ b/spec/javascripts/packages/document-capture/context/upload-spec.jsx @@ -21,7 +21,6 @@ describe('document-capture/context/upload', () => { 'backgroundUploadEncryptKey', 'flowPath', 'formData', - 'csrf', ]); expect(result.current.upload).to.equal(defaultUpload); @@ -30,7 +29,6 @@ describe('document-capture/context/upload', () => { expect(result.current.isMockClient).to.be.false(); expect(result.current.backgroundUploadURLs).to.deep.equal({}); expect(result.current.backgroundUploadEncryptKey).to.be.undefined(); - expect(result.current.csrf).to.be.null(); expect(await result.current.getStatus()).to.deep.equal({}); }); @@ -72,7 +70,7 @@ describe('document-capture/context/upload', () => { sandbox .stub(window, 'fetch') .withArgs(statusEndpoint) - .resolves({ ok: true, url: statusEndpoint, json: () => Promise.resolve({ success: true }) }); + .resolves(new Response(JSON.stringify({ success: true }), { url: statusEndpoint })); await result.current.getStatus(); expect(result.current.statusPollInterval).to.equal(1000); @@ -109,18 +107,16 @@ describe('document-capture/context/upload', () => { expect(result.current.backgroundUploadEncryptKey).to.equal(key); }); - it('can provide endpoint and csrf to make available to uploader', async () => { + it('provides endpoint to custom uploader', async () => { const { result } = renderHook(() => useContext(UploadContext), { wrapper: ({ children }) => ( + upload={(payload, { endpoint }) => Promise.resolve({ ...payload, receivedEndpoint: endpoint, - receivedCSRF: csrf, }) } - csrf="example" endpoint="https://example.com" > {children} @@ -132,7 +128,6 @@ describe('document-capture/context/upload', () => { expect(uploadResult).to.deep.equal({ sent: true, receivedEndpoint: 'https://example.com', - receivedCSRF: 'example', }); }); diff --git a/spec/javascripts/packages/document-capture/services/upload-spec.js b/spec/javascripts/packages/document-capture/services/upload-spec.js index 38eb6ab24a5..e0da05c434c 100644 --- a/spec/javascripts/packages/document-capture/services/upload-spec.js +++ b/spec/javascripts/packages/document-capture/services/upload-spec.js @@ -39,70 +39,22 @@ describe('document-capture/services/upload', () => { sandbox.stub(window, 'fetch').callsFake((url, init) => { expect(url).to.equal(endpoint); - expect(init.headers).to.be.empty(); expect(init.body).to.be.instanceOf(window.FormData); expect(init.body.get('foo')).to.equal('bar'); - return Promise.resolve( - /** @type {Partial} */ ({ - ok: true, - status: 200, - url: endpoint, - json: () => - Promise.resolve({ - success: true, - }), - }), - ); + return Promise.resolve(new Response(JSON.stringify({ success: true }), { url: endpoint })); }); - const result = await upload({ foo: 'bar' }, { endpoint, csrf: null }); + const result = await upload({ foo: 'bar' }, { endpoint }); expect(result).to.deep.equal({ success: true, isPending: false }); }); - context('with csrf token', () => { - it('submits payload to endpoint successfully', async () => { - const endpoint = 'https://example.com'; - const csrf = 'TYsqyyQ66Y'; - - sandbox.stub(window, 'fetch').callsFake((url, init) => { - expect(url).to.equal(endpoint); - expect(init.headers['X-CSRF-Token']).to.equal(csrf); - expect(init.body).to.be.instanceOf(window.FormData); - expect(init.body.get('foo')).to.equal('bar'); - - return Promise.resolve( - /** @type {Partial} */ ({ - ok: true, - status: 200, - url: endpoint, - json: () => - Promise.resolve({ - success: true, - }), - }), - ); - }); - - const result = await upload({ foo: 'bar' }, { endpoint, csrf }); - expect(result).to.deep.equal({ success: true, isPending: false }); - }); - }); - it('handles redirect', async () => { const endpoint = 'https://example.com'; - const csrf = 'TYsqyyQ66Y'; - sandbox.stub(window, 'fetch').callsFake(() => - Promise.resolve( - /** @type {Partial} */ ({ - ok: true, - status: 200, - url: '#teapot', - text: () => Promise.resolve(''), - }), - ), - ); + sandbox + .stub(window, 'fetch') + .callsFake(() => Promise.resolve(new Response('', { url: '#teapot' }))); let assertOnHashChange; @@ -116,7 +68,7 @@ describe('document-capture/services/upload', () => { window.addEventListener('hashchange', assertOnHashChange); }), - upload({}, { endpoint, csrf }).then(() => { + upload({}, { endpoint }).then(() => { throw new Error('Unexpected upload resolution during redirect.'); }), ]); @@ -124,30 +76,25 @@ describe('document-capture/services/upload', () => { window.removeEventListener('hashchange', assertOnHashChange); }); - it('handles pending success success', async () => { + it('handles pending success', async () => { const endpoint = 'https://example.com'; - const csrf = 'TYsqyyQ66Y'; sandbox.stub(window, 'fetch').callsFake((url, init) => { expect(url).to.equal(endpoint); - expect(init.headers['X-CSRF-Token']).to.equal(csrf); expect(init.body).to.be.instanceOf(window.FormData); expect(init.body.get('foo')).to.equal('bar'); return Promise.resolve( - /** @type {Partial} */ ({ - ok: true, - status: 202, - url: endpoint, - json: () => - Promise.resolve({ - success: true, - }), - }), + new Response( + JSON.stringify({ + success: true, + }), + { status: 202, url: endpoint }, + ), ); }); - const result = await upload({ foo: 'bar' }, { endpoint, csrf }); + const result = await upload({ foo: 'bar' }, { endpoint }); expect(result).to.deep.equal({ success: true, isPending: true }); }); @@ -156,28 +103,25 @@ describe('document-capture/services/upload', () => { sandbox.stub(window, 'fetch').callsFake(() => Promise.resolve( - /** @type {Partial} */ ({ - ok: false, - status: 400, - url: endpoint, - json: () => - Promise.resolve({ - success: false, - errors: [ - { field: 'front', message: 'Please fill in this field' }, - { field: 'back', message: 'Please fill in this field' }, - ], - remaining_attempts: 3, - hints: true, - result_failed: true, - ocr_pii: { first_name: 'Fakey', last_name: 'McFakerson', dob: '1938-10-06' }, - }), - }), + new Response( + JSON.stringify({ + success: false, + errors: [ + { field: 'front', message: 'Please fill in this field' }, + { field: 'back', message: 'Please fill in this field' }, + ], + remaining_attempts: 3, + hints: true, + result_failed: true, + ocr_pii: { first_name: 'Fakey', last_name: 'McFakerson', dob: '1938-10-06' }, + }), + { status: 400, url: endpoint }, + ), ), ); try { - await upload({}, { endpoint, csrf: 'TYsqyyQ66Y' }); + await upload({}, { endpoint }); throw new Error('This is a safeguard and should never be reached, since upload should error'); } catch (error) { expect(error).to.be.instanceOf(UploadFormEntriesError); @@ -203,17 +147,13 @@ describe('document-capture/services/upload', () => { sandbox.stub(window, 'fetch').callsFake(() => Promise.resolve( - /** @type {Partial} */ ({ - ok: false, - status: 418, - statusText: "I'm a teapot", - url: endpoint, - json: () => - Promise.resolve({ - success: false, - redirect: '#teapot', - }), - }), + new Response( + JSON.stringify({ + success: false, + redirect: '#teapot', + }), + { status: 418, url: endpoint }, + ), ), ); @@ -229,7 +169,7 @@ describe('document-capture/services/upload', () => { window.addEventListener('hashchange', assertOnHashChange); }), - upload({}, { endpoint, csrf: 'TYsqyyQ66Y' }).then(() => { + upload({}, { endpoint }).then(() => { throw new Error('Unexpected upload resolution during redirect.'); }), ]); @@ -240,19 +180,12 @@ describe('document-capture/services/upload', () => { it('throws unhandled response', async () => { const endpoint = 'https://example.com'; - sandbox.stub(window, 'fetch').callsFake(() => - Promise.resolve( - /** @type {Partial} */ ({ - ok: false, - status: 500, - statusText: 'Server error', - url: endpoint, - }), - ), - ); + sandbox + .stub(window, 'fetch') + .resolves(new Response('', { status: 500, url: endpoint, statusText: 'Server error' })); try { - await upload({}, { endpoint, csrf: 'TYsqyyQ66Y' }); + await upload({}, { endpoint }); } catch (error) { expect(error).to.be.instanceof(Error); expect(error.message).to.equal('Server error'); From c3e590fbc0f55bca1d091dbab173243ae87b054a Mon Sep 17 00:00:00 2001 From: jc-gsa <104452882+jc-gsa@users.noreply.github.com> Date: Tue, 28 Mar 2023 08:43:28 -0500 Subject: [PATCH 06/13] LG-5847: Update confirmation email language (#8079) * Update user language preference on registration changelog: User-Facing Improvements, Registration, Update user confirmation email language --- app/forms/register_user_email_form.rb | 7 +++ spec/forms/register_user_email_form_spec.rb | 69 +++++++++++++-------- 2 files changed, 50 insertions(+), 26 deletions(-) diff --git a/app/forms/register_user_email_form.rb b/app/forms/register_user_email_form.rb index 0d1b4926239..a84a3f9a3a6 100644 --- a/app/forms/register_user_email_form.rb +++ b/app/forms/register_user_email_form.rb @@ -84,6 +84,7 @@ def process_successful_submission(request_id, instructions) # To prevent discovery of existing emails, we check to see if the email is # already taken and if so, we act as if the user registration was successful. if email_taken? && user_unconfirmed? + update_user_language_preference send_sign_up_unconfirmed_email(request_id) elsif email_taken? send_sign_up_confirmed_email @@ -98,6 +99,12 @@ def process_successful_submission(request_id, instructions) end end + def update_user_language_preference + if existing_user.email_language != email_language + existing_user.update(email_language: email_language) + end + end + def extra_analytics_attributes { email_already_exists: email_taken?, diff --git a/spec/forms/register_user_email_form_spec.rb b/spec/forms/register_user_email_form_spec.rb index 607ac3fd51a..31be43af7f4 100644 --- a/spec/forms/register_user_email_form_spec.rb +++ b/spec/forms/register_user_email_form_spec.rb @@ -9,22 +9,24 @@ describe '#submit' do context 'when email is already taken' do - it 'sets success to true to prevent revealing account existence' do - existing_user = create(:user, :signed_up, email: 'taken@gmail.com') - - extra = { + let(:email_address) { 'taken@gmail.com' } + let!(:existing_user) { create(:user, :signed_up, email: email_address) } + let(:extra_params) do + { email_already_exists: true, throttled: false, user_id: existing_user.uuid, domain_name: 'gmail.com', } + end + it 'sets success to true to prevent revealing account existence' do expect(subject.submit(email: 'TAKEN@gmail.com', terms_accepted: '1').to_h).to eq( success: true, errors: {}, - **extra, + **extra_params, ) - expect(subject.email).to eq 'taken@gmail.com' + expect(subject.email).to eq email_address expect_delivered_email_count(1) expect_delivered_email( to: [subject.email], @@ -36,13 +38,11 @@ expect(attempts_tracker).to receive( :user_registration_email_submission_rate_limited, ).with( - email: 'taken@example.com', email_already_registered: true, + email: email_address, email_already_registered: true, ) - create(:user, :signed_up, email: 'taken@example.com') - IdentityConfig.store.reg_confirmed_email_max_attempts.times do - subject.submit(email: 'TAKEN@example.com', terms_accepted: '1') + subject.submit(email: 'TAKEN@gmail.com', terms_accepted: '1') end expect(analytics).to have_logged_event( @@ -53,40 +53,57 @@ end context 'when email is already taken and existing user is unconfirmed' do - it 'sends confirmation instructions to existing user' do - user = create(:user, email: 'existing@test.com', confirmed_at: nil, uuid: '123') - allow(User).to receive(:find_with_email).with(user.email).and_return(user) + let(:email_address) { 'existing@test.com' } + let!(:existing_user) do + create(:user, email: email_address, email_language: 'en', confirmed_at: nil, uuid: '123') + end + let(:params) do + { + email: 'EXISTING@test.com', + email_language: 'fr', + terms_accepted: '1', + } + end + let(:extra_params) do + { + email_already_exists: true, + throttled: false, + user_id: existing_user.uuid, + domain_name: 'test.com', + } + end + let(:send_sign_up_email_confirmation) { instance_double(SendSignUpEmailConfirmation) } - send_sign_up_email_confirmation = instance_double(SendSignUpEmailConfirmation) + it 'sends confirmation instructions to existing user' do expect(send_sign_up_email_confirmation).to receive(:call) expect(SendSignUpEmailConfirmation).to receive(:new).with( - user, + existing_user, ).and_return(send_sign_up_email_confirmation) - extra = { - email_already_exists: true, - throttled: false, - user_id: user.uuid, - domain_name: 'test.com', - } + result = subject.submit(params).to_h - expect(subject.submit(email: user.email, terms_accepted: '1').to_h).to eq( + expect(result).to eq( success: true, errors: {}, - **extra, + **extra_params, ) end + it 'updates users language preference' do + expect do + subject.submit(params) + end.to change { existing_user.reload.email_language }.from('en').to('fr') + end + it 'creates throttle events after reaching throttle limit' do expect(attempts_tracker).to receive( :user_registration_email_submission_rate_limited, ).with( - email: 'test@example.com', email_already_registered: false, + email: email_address, email_already_registered: false, ) - create(:user, email: 'test@example.com', confirmed_at: nil, uuid: '123') IdentityConfig.store.reg_unconfirmed_email_max_attempts.times do - subject.submit(email: 'test@example.com', terms_accepted: '1') + subject.submit(email: email_address, terms_accepted: '1') end expect(analytics).to have_logged_event( From c140ca729a48dbfdfa04d8f1cec1e46223d1a1dd Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 28 Mar 2023 10:18:07 -0400 Subject: [PATCH 07/13] Remove unused styles related to IdV selfie (#8077) changelog: Internal, Performance, Reduce size of stylesheet assets --- .../document-capture/components/review-issues-step.scss | 3 --- app/javascript/packages/document-capture/styles.scss | 1 - 2 files changed, 4 deletions(-) delete mode 100644 app/javascript/packages/document-capture/components/review-issues-step.scss diff --git a/app/javascript/packages/document-capture/components/review-issues-step.scss b/app/javascript/packages/document-capture/components/review-issues-step.scss deleted file mode 100644 index a5743641bf6..00000000000 --- a/app/javascript/packages/document-capture/components/review-issues-step.scss +++ /dev/null @@ -1,3 +0,0 @@ -.document-capture-review-issues-step__input:not(.document-capture-review-issues-step__input--unconstrained-width) { - max-width: 375px; -} diff --git a/app/javascript/packages/document-capture/styles.scss b/app/javascript/packages/document-capture/styles.scss index 8256b76c59d..7fad52a07e8 100644 --- a/app/javascript/packages/document-capture/styles.scss +++ b/app/javascript/packages/document-capture/styles.scss @@ -2,4 +2,3 @@ @import './components/acuant-capture'; @import './components/acuant-capture-canvas'; @import './components/location-collection-item'; -@import './components/review-issues-step'; From 8ca4ec6c26a3b29438de72ea5783f3fc82f5885d Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Tue, 28 Mar 2023 11:25:41 -0700 Subject: [PATCH 08/13] LG-9099 IdvStepConcern updates (#8082) * Move confirm_profile_not_already_confirmed to IdvStepConcern In a post-FSM world we want all the logic about redirects between steps to be in IdvStepConcern. Include this method in the pattern, and include IdvStepConcern in SsnController. Also remove redundant calls to `confirm_two_factor_authenticated` before action since it is included by IdvStepConcern. * Rename confirm_profile_not_already_confirmed to confirm_verify_info_step_needed Note that in_person/verify_info_controller has its own implementation which was also renamed. * Move confirm_pii_from_doc to IdvStepConcern Note that AddressController has its own slightly different implementation of this method. * Rename `confirm_pii_from_doc` to `confirm_document_capture_complete` including in AddressController changelog: Internal, Identity Verification, Refactor code that moves between steps --- .../concerns/idv/step_utilities_concern.rb | 13 ------------- app/controllers/concerns/idv_step_concern.rb | 13 +++++++++++++ app/controllers/idv/address_controller.rb | 4 ++-- .../idv/in_person/verify_info_controller.rb | 4 ++-- app/controllers/idv/ssn_controller.rb | 6 +++--- app/controllers/idv/verify_info_controller.rb | 3 +-- .../idv/in_person/verify_info_controller_spec.rb | 2 +- spec/controllers/idv/ssn_controller_spec.rb | 2 +- 8 files changed, 23 insertions(+), 24 deletions(-) diff --git a/app/controllers/concerns/idv/step_utilities_concern.rb b/app/controllers/concerns/idv/step_utilities_concern.rb index 626abf69c62..59e03150f23 100644 --- a/app/controllers/concerns/idv/step_utilities_concern.rb +++ b/app/controllers/concerns/idv/step_utilities_concern.rb @@ -11,19 +11,6 @@ def flow_path flow_session[:flow_path] end - def confirm_pii_from_doc - @pii = flow_session&.[]('pii_from_doc') # hash with indifferent access - return if @pii.present? - - flow_session&.delete('Idv::Steps::DocumentCaptureStep') - redirect_to idv_doc_auth_url - end - - def confirm_profile_not_already_confirmed - return unless idv_session.verify_info_step_complete? - redirect_to idv_review_url - end - # Copied from capture_doc_flow.rb # and from doc_auth_flow.rb def acuant_sdk_ab_test_analytics_args diff --git a/app/controllers/concerns/idv_step_concern.rb b/app/controllers/concerns/idv_step_concern.rb index 0067dc4c46d..dc5f3742356 100644 --- a/app/controllers/concerns/idv_step_concern.rb +++ b/app/controllers/concerns/idv_step_concern.rb @@ -8,6 +8,14 @@ module IdvStepConcern before_action :confirm_idv_needed end + def confirm_document_capture_complete + @pii = flow_session&.[]('pii_from_doc') # hash with indifferent access + return if @pii.present? + + flow_session&.delete('Idv::Steps::DocumentCaptureStep') + redirect_to idv_doc_auth_url + end + def confirm_verify_info_step_complete return if idv_session.verify_info_step_complete? @@ -18,6 +26,11 @@ def confirm_verify_info_step_complete end end + def confirm_verify_info_step_needed + return unless idv_session.verify_info_step_complete? + redirect_to idv_review_url + end + def confirm_address_step_complete return if idv_session.address_step_complete? diff --git a/app/controllers/idv/address_controller.rb b/app/controllers/idv/address_controller.rb index a52f40c116b..9492de49ae9 100644 --- a/app/controllers/idv/address_controller.rb +++ b/app/controllers/idv/address_controller.rb @@ -3,7 +3,7 @@ class AddressController < ApplicationController include IdvSession before_action :confirm_two_factor_authenticated - before_action :confirm_pii_from_doc + before_action :confirm_document_capture_complete def new analytics.idv_address_visit @@ -24,7 +24,7 @@ def update private - def confirm_pii_from_doc + def confirm_document_capture_complete @pii = user_session.dig('idv/doc_auth', 'pii_from_doc') return if @pii.present? redirect_to idv_doc_auth_url diff --git a/app/controllers/idv/in_person/verify_info_controller.rb b/app/controllers/idv/in_person/verify_info_controller.rb index 21747e62fb5..3fef4d4b638 100644 --- a/app/controllers/idv/in_person/verify_info_controller.rb +++ b/app/controllers/idv/in_person/verify_info_controller.rb @@ -9,7 +9,7 @@ class VerifyInfoController < ApplicationController before_action :renders_404_if_flag_not_set before_action :confirm_two_factor_authenticated before_action :confirm_ssn_step_complete - before_action :confirm_profile_not_already_confirmed + before_action :confirm_verify_info_step_needed def show @in_person_proofing = true @@ -107,7 +107,7 @@ def confirm_ssn_step_complete redirect_to idv_in_person_url end - def confirm_profile_not_already_confirmed + def confirm_verify_info_step_needed # todo: should this instead be like so? # return unless idv_session.resolution_successful == true return unless idv_session.verify_info_step_complete? diff --git a/app/controllers/idv/ssn_controller.rb b/app/controllers/idv/ssn_controller.rb index 76ddf64111a..25945d0373f 100644 --- a/app/controllers/idv/ssn_controller.rb +++ b/app/controllers/idv/ssn_controller.rb @@ -1,13 +1,13 @@ module Idv class SsnController < ApplicationController include IdvSession + include IdvStepConcern include StepIndicatorConcern include StepUtilitiesConcern include Steps::ThreatMetrixStepHelper - before_action :confirm_two_factor_authenticated - before_action :confirm_profile_not_already_confirmed - before_action :confirm_pii_from_doc + before_action :confirm_verify_info_step_needed + before_action :confirm_document_capture_complete attr_accessor :error_message diff --git a/app/controllers/idv/verify_info_controller.rb b/app/controllers/idv/verify_info_controller.rb index 2601be0f70e..9ea99d5ae5b 100644 --- a/app/controllers/idv/verify_info_controller.rb +++ b/app/controllers/idv/verify_info_controller.rb @@ -6,9 +6,8 @@ class VerifyInfoController < ApplicationController include VerifyInfoConcern include Steps::ThreatMetrixStepHelper - before_action :confirm_two_factor_authenticated before_action :confirm_ssn_step_complete - before_action :confirm_profile_not_already_confirmed + before_action :confirm_verify_info_step_needed def show @in_person_proofing = false diff --git a/spec/controllers/idv/in_person/verify_info_controller_spec.rb b/spec/controllers/idv/in_person/verify_info_controller_spec.rb index d3a0cf80a5d..83ea59e23d9 100644 --- a/spec/controllers/idv/in_person/verify_info_controller_spec.rb +++ b/spec/controllers/idv/in_person/verify_info_controller_spec.rb @@ -35,7 +35,7 @@ it 'confirms verify step not already complete' do expect(subject).to have_actions( :before, - :confirm_profile_not_already_confirmed, + :confirm_verify_info_step_needed, ) end diff --git a/spec/controllers/idv/ssn_controller_spec.rb b/spec/controllers/idv/ssn_controller_spec.rb index 19a9a49da96..af91a94147f 100644 --- a/spec/controllers/idv/ssn_controller_spec.rb +++ b/spec/controllers/idv/ssn_controller_spec.rb @@ -31,7 +31,7 @@ it 'checks that the previous step is complete' do expect(subject).to have_actions( :before, - :confirm_pii_from_doc, + :confirm_document_capture_complete, ) end end From fe1d70a73a7b50b0016dff7daa65b63e03ef9bff Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Tue, 28 Mar 2023 15:02:17 -0700 Subject: [PATCH 09/13] Remove final references to send_link step (#8083) Note that Funnel::DocAuth::RegisterStep still refers to send_link and the doc_auth_logs table still has columns for send_link_view_at and send_link_view_count changelog: Internal, Identity Verification, Remove final references to SendLinkStep --- app/services/analytics_events.rb | 2 +- app/services/idv/actions/redo_document_capture_action.rb | 1 - app/services/idv/steps/link_sent_step.rb | 2 +- app/services/idv/steps/upload_step.rb | 2 -- 4 files changed, 2 insertions(+), 5 deletions(-) diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index af285402965..07089cf5f2d 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -917,7 +917,7 @@ def idv_doc_auth_submitted_pii_validation( # The "hybrid handoff" step: Desktop user has submitted their choice to # either continue via desktop ("document_capture" destination) or switch # to mobile phone ("send_link" destination) to perform document upload. - # Mobile users sill log this event but with skip_upload_step = true + # Mobile users still log this event but with skip_upload_step = true def idv_doc_auth_upload_submitted(**extra) track_event('IdV: doc auth upload submitted', **extra) end diff --git a/app/services/idv/actions/redo_document_capture_action.rb b/app/services/idv/actions/redo_document_capture_action.rb index 2e0fa2a7fd9..d17513cb837 100644 --- a/app/services/idv/actions/redo_document_capture_action.rb +++ b/app/services/idv/actions/redo_document_capture_action.rb @@ -10,7 +10,6 @@ def call unless flow_session[:skip_upload_step] mark_step_incomplete(:email_sent) mark_step_incomplete(:link_sent) - mark_step_incomplete(:send_link) mark_step_incomplete(:upload) end end diff --git a/app/services/idv/steps/link_sent_step.rb b/app/services/idv/steps/link_sent_step.rb index d4bd0e779e7..28d551cd5da 100644 --- a/app/services/idv/steps/link_sent_step.rb +++ b/app/services/idv/steps/link_sent_step.rb @@ -3,7 +3,7 @@ module Steps class LinkSentStep < DocAuthBaseStep STEP_INDICATOR_STEP = :verify_id - HYBRID_FLOW_STEPS = %i[upload send_link link_sent email_sent document_capture] + HYBRID_FLOW_STEPS = %i[upload link_sent email_sent document_capture] def self.analytics_visited_event :idv_doc_auth_link_sent_visited diff --git a/app/services/idv/steps/upload_step.rb b/app/services/idv/steps/upload_step.rb index 718a145b0ca..b5edaec3bd9 100644 --- a/app/services/idv/steps/upload_step.rb +++ b/app/services/idv/steps/upload_step.rb @@ -72,7 +72,6 @@ def handle_phone_submission failure_reason: failure_reason, ) - mark_step_complete(:send_link) mark_step_complete(:email_sent) build_telephony_form_response(telephony_result) @@ -91,7 +90,6 @@ def application end def send_user_to_email_sent_step - mark_step_complete(:send_link) mark_step_complete(:link_sent) UserMailer.with( user: current_user, email_address: current_user.confirmed_email_addresses.first, From fa59166f78fa9d7bb2eff0ead04c8cbc0fd0d663 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 29 Mar 2023 09:18:27 -0400 Subject: [PATCH 10/13] Remove redundant "remaining" value from session timeout API (Part 1) (#8084) * Remove redundant "remaining" value from timeout API changelog: Internal, Session Timeout, Remove redundant timeout value from session timeout API * Revert session API remaining removal Temporary for first deploy, see https://github.com/18F/identity-idp/pull/8084/files#r1150918192 * Optimistically hide modal on session continue 1. Fixes specs 2. Better user experience, don't have to wait for request to finish before modal is closed, thus making app feel more responsive 3. The previous result handling with `success` would cause a second polling interval to begin * Bring over rest of spec changes from #7966 The CSRF won't be updated until after the request finishes, even if the modal will be disappeared immediately --- app/javascript/packs/session-timeout-ping.ts | 18 ++++++------------ spec/features/users/sign_in_spec.rb | 17 ++++++++--------- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/app/javascript/packs/session-timeout-ping.ts b/app/javascript/packs/session-timeout-ping.ts index 882e2f2b5aa..2f25db66270 100644 --- a/app/javascript/packs/session-timeout-ping.ts +++ b/app/javascript/packs/session-timeout-ping.ts @@ -23,11 +23,6 @@ interface PingResponse { */ live: boolean; - /** - * Time remaining in active session, in seconds. - */ - remaining: number; - /** * ISO8601-formatted date string for session timeout. */ @@ -64,7 +59,7 @@ function handleTimeout(redirectURL: string) { } function success(data: PingResponse) { - let timeRemaining = data.remaining * 1000; + let timeRemaining = new Date(data.timeout).valueOf() - Date.now(); const showWarning = timeRemaining < warning; if (!data.live) { @@ -80,9 +75,6 @@ function success(data: PingResponse) { countdownEl.expiration = new Date(data.timeout); countdownEl.start(); }); - } else { - modal.hide(); - countdownEls.forEach((countdownEl) => countdownEl.stop()); } if (timeRemaining < frequency) { @@ -102,9 +94,11 @@ function ping() { } function keepalive() { - request('/sessions/keepalive', { method: 'POST' }) - .then(success) - .catch((error) => notifyNewRelic(error, 'keepalive')); + modal.hide(); + countdownEls.forEach((countdownEl) => countdownEl.stop()); + request('/sessions/keepalive', { method: 'POST' }).catch((error) => { + notifyNewRelic(error, 'keepalive'); + }); } keepaliveEl?.addEventListener('click', keepalive, false); diff --git a/spec/features/users/sign_in_spec.rb b/spec/features/users/sign_in_spec.rb index c3a7b1d0aa5..0ce969f8a61 100644 --- a/spec/features/users/sign_in_spec.rb +++ b/spec/features/users/sign_in_spec.rb @@ -273,15 +273,14 @@ end scenario 'user can continue browsing with refreshed CSRF token' do - expect do - click_button t('notices.timeout_warning.signed_in.continue') - expect(page).not_to have_css('.usa-js-modal--active', wait: 5) - end.to change { find('[name=authenticity_token]', visible: false).value } - - expect(current_path).to eq forget_all_browsers_path - - click_button t('forms.buttons.confirm') - expect(current_path).to eq account_path + token = find('[name=authenticity_token]', visible: false).value + click_button t('notices.timeout_warning.signed_in.continue') + expect(page).not_to have_css('.usa-js-modal--active') + expect(page).to have_css( + "[name=authenticity_token]:not([value='#{token}'])", + visible: false, + wait: 5, + ) end scenario 'user has option to sign out' do From bec18733a2182dd73a3413ab37658dfa045b762e Mon Sep 17 00:00:00 2001 From: Alex Bradley Date: Wed, 29 Mar 2023 12:03:07 -0400 Subject: [PATCH 11/13] LG-8770 TMX rejected users page (#8055) * add redirect for users with rejected profiles * add test in auth controller for fraud review pending redirect * add controller and view for verify errors * add fraud_rejection redirects * create verify failure page * add test for verify_errors_controller * add analytics event for verify errors visited * add tests to check redirects * add changelong changelog: User-Facing Improvements, IdV, New fraud rejection page for users * lints * fraud before actions placed in concern * move verify_errors to not_verified * remove external link and Exit Login.gov * add handle fraud before action * check for fraud in errors controllers * remove before actions --- .../concerns/fraud_review_concern.rb | 18 +++++++++++++++ app/controllers/idv/doc_auth_controller.rb | 2 +- .../idv/not_verified_controller.rb | 9 ++++++++ app/controllers/idv_controller.rb | 2 +- .../authorization_controller.rb | 6 +++++ app/controllers/saml_idp_controller.rb | 1 + app/services/analytics_events.rb | 5 ++++ app/views/idv/not_verified/show.html.erb | 23 +++++++++++++++++++ config/locales/idv/en.yml | 4 ++++ config/locales/idv/es.yml | 4 ++++ config/locales/idv/fr.yml | 4 ++++ config/routes.rb | 1 + .../idv/not_verified_controller_spec.rb | 21 +++++++++++++++++ spec/controllers/idv_controller_spec.rb | 20 ++++++++++++++++ .../authorization_controller_spec.rb | 18 +++++++++++++++ 15 files changed, 136 insertions(+), 2 deletions(-) create mode 100644 app/controllers/idv/not_verified_controller.rb create mode 100644 app/views/idv/not_verified/show.html.erb create mode 100644 spec/controllers/idv/not_verified_controller_spec.rb diff --git a/app/controllers/concerns/fraud_review_concern.rb b/app/controllers/concerns/fraud_review_concern.rb index f21407835e0..28cfdea80aa 100644 --- a/app/controllers/concerns/fraud_review_concern.rb +++ b/app/controllers/concerns/fraud_review_concern.rb @@ -1,16 +1,34 @@ module FraudReviewConcern extend ActiveSupport::Concern + def handle_fraud + handle_pending_fraud_review + handle_fraud_rejection + end + def handle_pending_fraud_review redirect_to_fraud_review if fraud_review_pending? end + def handle_fraud_rejection + redirect_to_fraud_rejection if fraud_rejection? + end + def redirect_to_fraud_review redirect_to idv_setup_errors_url end + def redirect_to_fraud_rejection + redirect_to idv_not_verified_url + end + def fraud_review_pending? return false unless user_fully_authenticated? current_user.fraud_review_pending? end + + def fraud_rejection? + return false unless user_fully_authenticated? + current_user.fraud_rejection? + end end diff --git a/app/controllers/idv/doc_auth_controller.rb b/app/controllers/idv/doc_auth_controller.rb index b2e55231a30..b7ac16c4417 100644 --- a/app/controllers/idv/doc_auth_controller.rb +++ b/app/controllers/idv/doc_auth_controller.rb @@ -2,7 +2,6 @@ module Idv class DocAuthController < ApplicationController before_action :confirm_two_factor_authenticated before_action :redirect_if_pending_in_person_enrollment - before_action :handle_pending_fraud_review before_action :redirect_if_pending_profile before_action :extend_timeout_using_meta_refresh_for_select_paths @@ -13,6 +12,7 @@ class DocAuthController < ApplicationController include FraudReviewConcern before_action :redirect_if_flow_completed + before_action :handle_fraud before_action :override_document_capture_step_csp before_action :update_if_skipping_upload # rubocop:disable Rails/LexicallyScopedActionFilter diff --git a/app/controllers/idv/not_verified_controller.rb b/app/controllers/idv/not_verified_controller.rb new file mode 100644 index 00000000000..7065a293856 --- /dev/null +++ b/app/controllers/idv/not_verified_controller.rb @@ -0,0 +1,9 @@ +module Idv + class NotVerifiedController < ApplicationController + before_action :confirm_two_factor_authenticated + + def show + analytics.idv_not_verified_visited + end + end +end diff --git a/app/controllers/idv_controller.rb b/app/controllers/idv_controller.rb index 905043cbd51..984c268ccdf 100644 --- a/app/controllers/idv_controller.rb +++ b/app/controllers/idv_controller.rb @@ -4,8 +4,8 @@ class IdvController < ApplicationController include FraudReviewConcern before_action :confirm_two_factor_authenticated - before_action :handle_pending_fraud_review before_action :profile_needs_reactivation?, only: [:index] + before_action :handle_fraud def index if decorated_session.requested_more_recent_verification? || diff --git a/app/controllers/openid_connect/authorization_controller.rb b/app/controllers/openid_connect/authorization_controller.rb index 4a1e52f4cbe..e73b32eb287 100644 --- a/app/controllers/openid_connect/authorization_controller.rb +++ b/app/controllers/openid_connect/authorization_controller.rb @@ -21,6 +21,7 @@ class AuthorizationController < ApplicationController def index return redirect_to_fraud_review if fraud_review_pending_for_ial2_request? + return redirect_to_fraud_rejection if fraud_rejection_for_ial2_request? return redirect_to_account_or_verify_profile_url if profile_or_identity_needs_verification? return redirect_to(sign_up_completed_url) if needs_completion_screen_reason link_identity_to_service_provider @@ -90,6 +91,11 @@ def fraud_review_pending_for_ial2_request? fraud_review_pending? end + def fraud_rejection_for_ial2_request? + return false unless @authorize_form.ial2_or_greater? + fraud_rejection? + end + def profile_or_identity_needs_verification? return false unless @authorize_form.ial2_or_greater? profile_needs_verification? || identity_needs_verification? diff --git a/app/controllers/saml_idp_controller.rb b/app/controllers/saml_idp_controller.rb index d695da00adc..ebce55c5d95 100644 --- a/app/controllers/saml_idp_controller.rb +++ b/app/controllers/saml_idp_controller.rb @@ -25,6 +25,7 @@ class SamlIdpController < ApplicationController def auth capture_analytics return redirect_to_fraud_review if fraud_review_pending? && ial2_requested? + return redirect_to_fraud_rejection if fraud_rejection? && ial2_requested? return redirect_to_verification_url if profile_or_identity_needs_verification_or_decryption? return redirect_to(sign_up_completed_url) if needs_completion_screen_reason if auth_count == 1 && first_visit_for_sp? diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index 07089cf5f2d..164c4b025ce 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -3496,6 +3496,11 @@ def idv_setup_errors_visited(proofing_components: nil, **extra) ) end + # Tracks when user reaches verify errors due to being rejected due to fraud + def idv_not_verified_visited + track_event('IdV: Not verified visited') + end + # @param [String] redirect_url URL user was directed to # @param [String, nil] step which step # @param [String, nil] location which part of a step, if applicable diff --git a/app/views/idv/not_verified/show.html.erb b/app/views/idv/not_verified/show.html.erb new file mode 100644 index 00000000000..1721694124c --- /dev/null +++ b/app/views/idv/not_verified/show.html.erb @@ -0,0 +1,23 @@ +<%= render( + 'idv/shared/error', + title: t('titles.failure.information_not_verified'), + heading: t('idv.failure.verify.heading'), + ) do %> +

+ <% if decorated_session.sp_name.present? %> + <%= link_to( + t('idv.failure.verify.fail_link_html', sp_name: decorated_session.sp_name), + return_to_sp_failure_to_proof_path( + step: 'verify_info', + location: request.params[:action], + ), + ) %> + <% else %> + <%= link_to( + t('idv.failure.verify.fail_link_html', sp_name: APP_NAME), + account_path, + ) %> + <% end %> + <%= t('idv.failure.verify.fail_text') %> +

+<% end %> diff --git a/config/locales/idv/en.yml b/config/locales/idv/en.yml index beb9ef0918f..9302117bbf6 100644 --- a/config/locales/idv/en.yml +++ b/config/locales/idv/en.yml @@ -99,6 +99,10 @@ en: heading: Please give us a call timeout: We are experiencing higher than usual wait time processing your request. Please try again. + verify: + fail_link_html: Get help at %{sp_name} + fail_text: to access services. + heading: We couldn’t verify your identity forgot_password: link_text: Forgot password? modal_header: Are you sure you can’t remember your password? diff --git a/config/locales/idv/es.yml b/config/locales/idv/es.yml index 03fa169711f..939c2b443cd 100644 --- a/config/locales/idv/es.yml +++ b/config/locales/idv/es.yml @@ -109,6 +109,10 @@ es: heading: Llámenos timeout: Estamos experimentando un tiempo de espera superior al habitual al procesar su solicitud. Inténtalo de nuevo. + verify: + fail_link_html: Obtenga ayuda en %{sp_name} + fail_text: para acceder a los servicios. + heading: No hemos podido verificar su identidad forgot_password: link_text: '¿Se te olvidó tu contraseña?' modal_header: '¿Estás seguro de que no puedes recordar tu contraseña?' diff --git a/config/locales/idv/fr.yml b/config/locales/idv/fr.yml index 6d3cee55b8f..ef4ca40cea0 100644 --- a/config/locales/idv/fr.yml +++ b/config/locales/idv/fr.yml @@ -113,6 +113,10 @@ fr: heading: S’il vous plaît, appelez-nous timeout: Le temps d’attente pour le traitement de votre demande est plus long que d’habitude Veuillez réessayer. + verify: + fail_link_html: Obtenez de l’aide auprès de %{sp_name} + fail_text: pour accéder aux services. + heading: Nous n’avons pas pu vérifier votre identité forgot_password: link_text: Mot de passe oublié? modal_header: Êtes-vous sûr de ne pas pouvoir vous souvenir de votre mot de passe? diff --git a/config/routes.rb b/config/routes.rb index 58912bce1ca..b16e9df174b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -331,6 +331,7 @@ get '/session/errors/exception' => 'session_errors#exception' get '/session/errors/throttled' => 'session_errors#throttled' get '/setup_errors' => 'setup_errors#show' + get '/not_verified' => 'not_verified#show' delete '/session' => 'sessions#destroy' get '/cancel/' => 'cancellations#new', as: :cancel put '/cancel' => 'cancellations#update' diff --git a/spec/controllers/idv/not_verified_controller_spec.rb b/spec/controllers/idv/not_verified_controller_spec.rb new file mode 100644 index 00000000000..64557e4dbc8 --- /dev/null +++ b/spec/controllers/idv/not_verified_controller_spec.rb @@ -0,0 +1,21 @@ +require 'rails_helper' + +describe Idv::NotVerifiedController do + let(:user) { build_stubbed(:user, :signed_up) } + + before do + stub_sign_in(user) + end + + it 'renders the show template' do + stub_analytics + + expect(@analytics).to receive(:track_event).with( + 'IdV: Not verified visited', + ) + + get :show + + expect(response).to render_template :show + end +end diff --git a/spec/controllers/idv_controller_spec.rb b/spec/controllers/idv_controller_spec.rb index d7f379b73a9..681fe63ab47 100644 --- a/spec/controllers/idv_controller_spec.rb +++ b/spec/controllers/idv_controller_spec.rb @@ -27,6 +27,26 @@ get :index end + it 'redirects to sad face page if fraud review is pending' do + profile = create(:profile, fraud_review_pending: true) + + stub_sign_in(profile.user) + + get :index + + expect(response).to redirect_to(idv_setup_errors_url) + end + + it 'redirects to fraud rejection page if profile is rejected' do + profile = create(:profile, fraud_rejection: true) + + stub_sign_in(profile.user) + + get :index + + expect(response).to redirect_to(idv_not_verified_url) + end + context 'if number of attempts has been exceeded' do before do user = create(:user) diff --git a/spec/controllers/openid_connect/authorization_controller_spec.rb b/spec/controllers/openid_connect/authorization_controller_spec.rb index d0156405b7d..7dcf6a434f1 100644 --- a/spec/controllers/openid_connect/authorization_controller_spec.rb +++ b/spec/controllers/openid_connect/authorization_controller_spec.rb @@ -143,6 +143,24 @@ action expect(controller).to redirect_to(idv_url) end + + context 'user is under fraud review' do + let(:user) { create(:profile, fraud_review_pending: true).user } + + it 'redirects to fraud review page if fraud review is pending' do + action + expect(controller).to redirect_to(idv_setup_errors_url) + end + end + + context 'user is rejected due to fraud' do + let(:user) { create(:profile, fraud_rejection: true).user } + + it 'redirects to fraud rejection page if user is fraud rejected ' do + action + expect(controller).to redirect_to(idv_not_verified_url) + end + end end context 'profile is reset' do From 2fe1b638aab3a3e1b5b5f89a428c1375429521ab Mon Sep 17 00:00:00 2001 From: Shannon A <20867088+svalexander@users.noreply.github.com> Date: Wed, 29 Mar 2023 12:32:04 -0400 Subject: [PATCH 12/13] Shannon/lg 9005 update verify info pg (#8056) * add strings and pull address into separate file * update headings i18n * add heading for remote and remove conditionals for headings * remove address component as this is out of scope * only show changes for enrollments where capture_secondary_id_enabled is true * specs for verify step * fix verify spec * fix lint and doc auth spec failure * changelog: User-Facing Improvements, Verify your info, update sections * remove div and nil check * lint fix * add padding to titles and update --- .../idv/steps/in_person/verify_step.rb | 5 + app/views/idv/in_person/verify.html.erb | 2 +- app/views/idv/shared/_verify.html.erb | 133 +++++++---- config/locales/headings/en.yml | 4 + config/locales/headings/es.yml | 4 + config/locales/headings/fr.yml | 4 + lib/idp/constants.rb | 7 + .../idv/steps/in_person/verify_step_spec.rb | 216 +++++++++++++----- spec/support/features/in_person_helper.rb | 46 ++-- spec/support/features/verify_step_helper.rb | 19 ++ 10 files changed, 324 insertions(+), 116 deletions(-) create mode 100644 spec/support/features/verify_step_helper.rb diff --git a/app/services/idv/steps/in_person/verify_step.rb b/app/services/idv/steps/in_person/verify_step.rb index 3203dd4846d..43a80d1d2eb 100644 --- a/app/services/idv/steps/in_person/verify_step.rb +++ b/app/services/idv/steps/in_person/verify_step.rb @@ -20,6 +20,7 @@ def call def extra_view_variables { + capture_secondary_id_enabled: capture_secondary_id_enabled?, pii: pii, step_url: method(:idv_in_person_step_url), } @@ -33,6 +34,10 @@ def add_proofing_component update(document_check: Idp::Constants::Vendors::USPS) end + def capture_secondary_id_enabled? + current_user.establishing_in_person_enrollment.capture_secondary_id_enabled + end + def pii flow_session[:pii_from_user] end diff --git a/app/views/idv/in_person/verify.html.erb b/app/views/idv/in_person/verify.html.erb index 28f3686a616..9b9b7a94dcd 100644 --- a/app/views/idv/in_person/verify.html.erb +++ b/app/views/idv/in_person/verify.html.erb @@ -1 +1 @@ -<%= render 'idv/shared/verify', pii: pii, step_url: step_url, remote_identity_proofing: false %> +<%= render 'idv/shared/verify', pii: pii, step_url: step_url, remote_identity_proofing: false, capture_secondary_id_enabled: capture_secondary_id_enabled %> diff --git a/app/views/idv/shared/_verify.html.erb b/app/views/idv/shared/_verify.html.erb index efdd21aa577..5f20a1d332f 100644 --- a/app/views/idv/shared/_verify.html.erb +++ b/app/views/idv/shared/_verify.html.erb @@ -7,9 +7,14 @@ locals: <% title t('titles.idv.verify_info') %> <%= render PageHeadingComponent.new.with_content(t('headings.verify')) %> -
-
+
+
+ <% if capture_secondary_id_enabled %> +
+ <%= t('headings.state_id') %> +
+ <% end %>
<%= t('idv.form.first_name') %>:
<%= pii[:first_name] %>
@@ -30,72 +35,104 @@ locals:
<%= pii[:state_id_number] %>
<% end %> + <% if !remote_identity_proofing && capture_secondary_id_enabled %> +
+
<%= t('idv.form.address1') %>:
+
<%= pii[:state_id_address1] %>
+
+
+
<%= t('idv.form.address2') %>:
+
<%= pii[:state_id_address2] %>
+
+
+
<%= t('idv.form.city') %>:
+
<%= pii[:state_id_city] %>
+
+
+
<%= t('idv.form.state') %>:
+
<%= pii[:state_id_jurisdiction] %>
+
+
+
<%= t('idv.form.zipcode') %>:
+
<%= pii[:state_id_zipcode] %>
+
+ <% end %>
<% if !remote_identity_proofing %>
<%= button_to( step_url.call(step: :redo_state_id), method: :put, - class: 'usa-button usa-button--unstyled', + class: 'usa-button usa-button--unstyled padding-y-1', 'aria-label': t('idv.buttons.change_state_id_label'), ) { t('idv.buttons.change_label') } %>
<% end %>
-
-
-
-
<%= t('idv.form.address1') %>:
-
<%= pii[:address1] %>
-
- <% if pii[:address2].present? %> -
-
<%= t('idv.form.address2') %>:
-
<%= pii[:address2] %>
+
+
+ <% if capture_secondary_id_enabled %> +
+ <%= remote_identity_proofing ? t('headings.address') : t('headings.residential_address') %> +
+ <% end %> +
+
<%= t('idv.form.address1') %>:
+
<%= pii[:address1] %>
+
+
+
<%= t('idv.form.address2') %>:
+
<%= pii[:address2] %>
+
+
+
<%= t('idv.form.city') %>:
+
<%= pii[:city] %>
+
+
+
<%= t('idv.form.state') %>:
+
<%= pii[:state] %>
+
+
+
<%= t('idv.form.zipcode') %>:
+
<%= pii[:zipcode] %>
+
+
+
+ <%= button_to( + step_url.call(step: :redo_address), + method: :put, + class: 'usa-button usa-button--unstyled padding-y-1', + 'aria-label': t('idv.buttons.change_address_label'), + ) { t('idv.buttons.change_label') } %>
- <% end %> -
-
<%= t('idv.form.city') %>:
-
<%= pii[:city] %>
-
-
-
<%= t('idv.form.state') %>:
-
<%= pii[:state] %>
-
-
-
<%= t('idv.form.zipcode') %>:
-
<%= pii[:zipcode] %>
-
-
-
- <%= button_to( - step_url.call(step: :redo_address), - method: :put, - class: 'usa-button usa-button--unstyled', - 'aria-label': t('idv.buttons.change_address_label'), - ) { t('idv.buttons.change_label') } %>
-
- <%= t('idv.form.ssn') %>: - <%= render( - 'shared/masked_text', - text: SsnFormatter.format(pii[:ssn]), - masked_text: SsnFormatter.format_masked(pii[:ssn]), - accessible_masked_text: t( - 'idv.accessible_labels.masked_ssn', - first_number: pii[:ssn][0], - last_number: pii[:ssn][-1], - ), - toggle_label: t('forms.ssn.show'), - ) %> + <% if capture_secondary_id_enabled %> +
+ <%= t('headings.ssn') %> +
+ <% end %> +
+ <%= t('idv.form.ssn') %> : + <%= render( + 'shared/masked_text', + text: SsnFormatter.format(pii[:ssn]), + masked_text: SsnFormatter.format_masked(pii[:ssn]), + accessible_masked_text: t( + 'idv.accessible_labels.masked_ssn', + first_number: pii[:ssn][0], + last_number: pii[:ssn][-1], + ), + toggle_label: t('forms.ssn.show'), + ) %> +
<%= button_to( step_url.call(step: :redo_ssn), method: :put, - class: 'usa-button usa-button--unstyled', + class: 'usa-button usa-button--unstyled padding-y-1', 'aria-label': t('idv.buttons.change_ssn_label'), ) { t('idv.buttons.change_label') } %>
diff --git a/config/locales/headings/en.yml b/config/locales/headings/en.yml index 59447949d86..fedd4b45093 100644 --- a/config/locales/headings/en.yml +++ b/config/locales/headings/en.yml @@ -17,6 +17,7 @@ en: add_email: Add a new email address add_info: phone: Add a phone number + address: Address cancellations: prompt: Are you sure you want to cancel? confirmations: @@ -58,10 +59,13 @@ en: piv_cac_setup: already_associated: The PIV/CAC you presented is associated with another user. new: Use your PIV/CAC card to secure your account + residential_address: Current residential address session_timeout_warning: Need more time? sign_in_with_sp: Sign in to continue to %{sp} sign_in_without_sp: Sign in sp_handoff_bounced: There was a problem connecting to %{sp_name} + ssn: Social Security Number + state_id: State-issued ID totp_setup: new: Add an authentication app verify: Verify your information diff --git a/config/locales/headings/es.yml b/config/locales/headings/es.yml index 8688a74d1a1..4a4e75dae54 100644 --- a/config/locales/headings/es.yml +++ b/config/locales/headings/es.yml @@ -17,6 +17,7 @@ es: add_email: Añadir una nueva dirección de correo electrónico add_info: phone: Agregar un número de teléfono + address: Dirección cancellations: prompt: '¿Estas seguro que quieres cancelar?' confirmations: @@ -58,10 +59,13 @@ es: piv_cac_setup: already_associated: La PIV/CAC que has presentado está asociada a otro usuario. new: Use su tarjeta PIV/CAC para asegurar su cuenta + residential_address: Dirección residencial actual session_timeout_warning: '¿Necesita más tiempo?' sign_in_with_sp: Iniciar sesión para continuar con %{sp} sign_in_without_sp: Iniciar sesión sp_handoff_bounced: Hubo un problema al conectarse a %{sp_name} + ssn: Número de seguro social + state_id: Documento de identidad emitido por el estado totp_setup: new: Agregar una aplicación de autenticación verify: Verifica tus datos diff --git a/config/locales/headings/fr.yml b/config/locales/headings/fr.yml index b90b46370e9..5c2e4561aae 100644 --- a/config/locales/headings/fr.yml +++ b/config/locales/headings/fr.yml @@ -17,6 +17,7 @@ fr: add_email: Ajouter une nouvelle adresse e-mail add_info: phone: Ajouter un numéro de téléphone + address: Adresse cancellations: prompt: Es-tu sûre de vouloir annuler? confirmations: @@ -61,10 +62,13 @@ fr: already_associated: La carte PIV/CAC que vous avez présentée est associée à un autre utilisateur. new: Utilisez votre carte PIV/CAC pour sécuriser votre compte + residential_address: Adresse de résidence actuelle session_timeout_warning: Vous avez besoin de plus de temps? sign_in_with_sp: Connectez-vous pour continuer à %{sp} sign_in_without_sp: Connexion sp_handoff_bounced: Un problème est survenu lors de la connexion à %{sp_name} + ssn: Numéro de sécurité sociale + state_id: Carte d’identité délivrée par l’État totp_setup: new: Ajouter une application d’authentification verify: Vérifier votre information diff --git a/lib/idp/constants.rb b/lib/idp/constants.rb index 760795e4831..fcf730f4796 100644 --- a/lib/idp/constants.rb +++ b/lib/idp/constants.rb @@ -100,6 +100,13 @@ module Vendors phone: nil, }.freeze + MOCK_IDV_APPLICANT_STATE_ID_ADDRESS = MOCK_IDV_APPLICANT.merge( + state_id_address1: '123 Way St', + state_id_address2: '2nd Address Line', + state_id_city: 'Best City', + state_id_zipcode: '12345', + ).freeze + MOCK_IDV_APPLICANT_WITH_SSN = MOCK_IDV_APPLICANT.merge(ssn: '900-66-1234').freeze MOCK_IDV_APPLICANT_WITH_PHONE = MOCK_IDV_APPLICANT_WITH_SSN.merge(phone: '12025551212').freeze 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 a2a5d0342a0..a8288f3c411 100644 --- a/spec/features/idv/steps/in_person/verify_step_spec.rb +++ b/spec/features/idv/steps/in_person/verify_step_spec.rb @@ -4,64 +4,172 @@ RSpec.describe 'doc auth IPP Verify Step', js: true do include IdvStepHelper include InPersonHelper + include VerifyStepHelper + + let(:remote_identity_proofing) { false } before do allow(IdentityConfig.store).to receive(:in_person_proofing_enabled).and_return(true) end - it 'provides back buttons for address, state ID, and SSN that discard changes', - allow_browser_log: true do - user = user_with_2fa - - sign_in_and_2fa_user(user) - begin_in_person_proofing(user) - complete_location_step(user) - complete_prepare_step(user) - complete_state_id_step(user) - complete_address_step(user) - complete_ssn_step(user) - - # verify page - expect(page).to have_content(t('headings.verify')) - expect(page).to have_text(InPersonHelper::GOOD_FIRST_NAME) - expect(page).to have_text(InPersonHelper::GOOD_LAST_NAME) - expect(page).to have_text(InPersonHelper::GOOD_DOB_FORMATTED_EVENT) - expect(page).to have_text(InPersonHelper::GOOD_STATE_ID_NUMBER) - expect(page).to have_text(InPersonHelper::GOOD_ADDRESS1) - expect(page).to have_text(InPersonHelper::GOOD_CITY) - expect(page).to have_text(InPersonHelper::GOOD_ZIPCODE) - expect(page).to have_text(Idp::Constants::MOCK_IDV_APPLICANT[:state]) - expect(page).to have_text('9**-**-***4') - - # click update state ID button - click_button t('idv.buttons.change_state_id_label') - expect(page).to have_content(t('in_person_proofing.headings.update_state_id')) - fill_in t('in_person_proofing.form.state_id.first_name'), with: 'bad first name' - click_doc_auth_back_link - expect(page).to have_content(t('headings.verify')) - expect(page).to have_text(InPersonHelper::GOOD_FIRST_NAME) - expect(page).not_to have_text('bad first name') - - # click update address button - click_button t('idv.buttons.change_address_label') - expect(page).to have_content(t('in_person_proofing.headings.update_address')) - fill_in t('idv.form.address1'), with: 'bad address' - click_doc_auth_back_link - expect(page).to have_content(t('headings.verify')) - expect(page).to have_text(InPersonHelper::GOOD_ADDRESS1) - expect(page).not_to have_text('bad address') - - # click update ssn button - click_button t('idv.buttons.change_ssn_label') - expect(page).to have_content(t('doc_auth.headings.ssn_update')) - fill_out_ssn_form_fail - click_doc_auth_back_link - expect(page).to have_content(t('headings.verify')) - expect(page).to have_text('9**-**-***4') - - complete_verify_step(user) - - # phone page - expect(page).to have_content(t('idv.titles.session.phone')) + context 'capture secondary id is not enabled' do + let(:capture_secondary_id_enabled) { false } + let(:double_address_verification) { false } + + before do + allow(IdentityConfig.store).to receive(:in_person_capture_secondary_id_enabled). + and_return(false) + end + + it 'provides back buttons for address, state ID, and SSN that discard changes', + allow_browser_log: true do + user = user_with_2fa + + sign_in_and_2fa_user(user) + begin_in_person_proofing(user) + complete_location_step(user) + complete_prepare_step(user) + complete_state_id_step(user, double_address_verification: double_address_verification) + complete_address_step(user, double_address_verification: double_address_verification) + complete_ssn_step(user) + + # verify page + expect(page).to have_content(t('headings.verify')) + expect(page).to have_text(InPersonHelper::GOOD_FIRST_NAME) + expect(page).to have_text(InPersonHelper::GOOD_LAST_NAME) + expect(page).to have_text(InPersonHelper::GOOD_DOB_FORMATTED_EVENT) + expect(page).to have_text(InPersonHelper::GOOD_STATE_ID_NUMBER) + expect_good_address + expect(page).to have_text('9**-**-***4') + + # click update state ID button + click_button t('idv.buttons.change_state_id_label') + expect(page).to have_content(t('in_person_proofing.headings.update_state_id')) + fill_in t('in_person_proofing.form.state_id.first_name'), with: 'bad first name' + click_doc_auth_back_link + expect(page).to have_content(t('headings.verify')) + expect(page).to have_text(InPersonHelper::GOOD_FIRST_NAME) + expect(page).not_to have_text('bad first name') + + # click update address button + click_button t('idv.buttons.change_address_label') + expect(page).to have_content(t('in_person_proofing.headings.update_address')) + fill_in t('idv.form.address1'), with: 'bad address' + click_doc_auth_back_link + expect(page).to have_content(t('headings.verify')) + expect(page).to have_text(InPersonHelper::GOOD_ADDRESS1) + expect(page).not_to have_text('bad address') + + # click update ssn button + click_button t('idv.buttons.change_ssn_label') + expect(page).to have_content(t('doc_auth.headings.ssn_update')) + fill_out_ssn_form_fail + click_doc_auth_back_link + expect(page).to have_content(t('headings.verify')) + expect(page).to have_text('9**-**-***4') + + complete_verify_step(user) + + # phone page + expect(page).to have_content(t('idv.titles.session.phone')) + end + end + + context 'with secondary capture enabled and user has different address' do + let(:capture_secondary_id_enabled) { true } + let(:enrollment) { InPersonEnrollment.new(capture_secondary_id_enabled:) } + let(:user) { user_with_2fa } + let(:same_address_as_id) { false } + let(:double_address_verification) { true } + + before do + allow(IdentityConfig.store).to receive(:in_person_capture_secondary_id_enabled). + and_return(true) + allow(user).to receive(:establishing_in_person_enrollment). + and_return(enrollment) + allow(subject).to receive(:remote_identity_proofing).and_return(false) + end + + it 'shows two addresses when current address is different from address on id', + allow_browser_log: true do + sign_in_and_2fa_user(user) + begin_in_person_proofing(user) + complete_location_step(user) + complete_prepare_step(user) + complete_state_id_step( + user, same_address_as_id: same_address_as_id, + double_address_verification: double_address_verification + ) + complete_address_step(user, double_address_verification: double_address_verification) + complete_ssn_step(user) + + # verify page + expect(page).to have_current_path(idv_in_person_step_url(step: :verify)) + expect(page).to have_content(t('headings.verify')) + expect(page).to have_content(t('headings.state_id')) + expect(page).to have_text(InPersonHelper::GOOD_FIRST_NAME) + expect(page).to have_text(InPersonHelper::GOOD_LAST_NAME) + expect(page).to have_text(InPersonHelper::GOOD_DOB_FORMATTED_EVENT) + expect(page).to have_text(InPersonHelper::GOOD_STATE_ID_NUMBER) + expect_good_state_id_address + expect(page).to have_content(t('headings.residential_address')) + expect_good_address + expect(page).to have_content(t('headings.ssn')) + expect(page).to have_text('9**-**-***4') + + complete_verify_step(user) + end + end + + context 'with secondary capture enabled and user has same address' do + let(:capture_secondary_id_enabled) { true } + let(:enrollment) { InPersonEnrollment.new(capture_secondary_id_enabled:) } + let(:user) { user_with_2fa } + let(:same_address_as_id) { true } + let(:double_address_verification) { true } + + before do + allow(IdentityConfig.store).to receive(:in_person_capture_secondary_id_enabled). + and_return(true) + allow(user).to receive(:establishing_in_person_enrollment). + and_return(enrollment) + end + + it 'shows same address in state id and current residential sections', + allow_browser_log: true do + sign_in_and_2fa_user(user) + begin_in_person_proofing(user) + complete_location_step(user) + complete_prepare_step(user) + complete_state_id_step( + user, same_address_as_id: same_address_as_id, + double_address_verification: double_address_verification + ) + fill_out_address_form_ok( + same_address_as_id: same_address_as_id, + double_address_verification: double_address_verification, + ) + click_idv_continue + complete_ssn_step(user) + + # verify page + expect(page).to have_current_path(idv_in_person_step_url(step: :verify)) + expect(page).to have_content(t('headings.verify')) + expect(page).to have_content(t('headings.state_id')) + expect(page).to have_text(InPersonHelper::GOOD_FIRST_NAME) + expect(page).to have_text(InPersonHelper::GOOD_LAST_NAME) + expect(page).to have_text(InPersonHelper::GOOD_DOB_FORMATTED_EVENT) + expect(page).to have_text(InPersonHelper::GOOD_STATE_ID_NUMBER) + expect(page).to have_text(InPersonHelper::GOOD_STATE_ID_ADDRESS1).twice + expect(page).to have_text(InPersonHelper::GOOD_STATE_ID_ADDRESS2).twice + expect(page).to have_text(InPersonHelper::GOOD_STATE_ID_CITY).twice + expect(page).to have_text(Idp::Constants::MOCK_IDV_APPLICANT[:state_id_jurisdiction]).twice + expect(page).to have_text(InPersonHelper::GOOD_STATE_ID_ZIPCODE).twice + expect(page).to have_content(t('headings.residential_address')) + expect(page).to have_content(t('headings.ssn')) + expect(page).to have_text('9**-**-***4') + + complete_verify_step(user) + end end end diff --git a/spec/support/features/in_person_helper.rb b/spec/support/features/in_person_helper.rb index 8ca97184adb..fa52c358d19 100644 --- a/spec/support/features/in_person_helper.rb +++ b/spec/support/features/in_person_helper.rb @@ -21,8 +21,12 @@ module InPersonHelper GOOD_CITY = Idp::Constants::MOCK_IDV_APPLICANT[:city] GOOD_ZIPCODE = Idp::Constants::MOCK_IDV_APPLICANT[:zipcode] GOOD_STATE = Idp::Constants::MOCK_IDV_APPLICANT_FULL_STATE + GOOD_STATE_ID_ADDRESS1 = Idp::Constants::MOCK_IDV_APPLICANT_STATE_ID_ADDRESS[:state_id_address1] + GOOD_STATE_ID_ADDRESS2 = Idp::Constants::MOCK_IDV_APPLICANT_STATE_ID_ADDRESS[:state_id_address2] + GOOD_STATE_ID_CITY = Idp::Constants::MOCK_IDV_APPLICANT_STATE_ID_ADDRESS[:city] + GOOD_STATE_ID_ZIPCODE = Idp::Constants::MOCK_IDV_APPLICANT_STATE_ID_ADDRESS[:zipcode] - def fill_out_state_id_form_ok(double_address_verification: false) + def fill_out_state_id_form_ok(double_address_verification: false, same_address_as_id: false) fill_in t('in_person_proofing.form.state_id.first_name'), with: GOOD_FIRST_NAME fill_in t('in_person_proofing.form.state_id.last_name'), with: GOOD_LAST_NAME year, month, day = GOOD_DOB.split('-') @@ -34,20 +38,32 @@ def fill_out_state_id_form_ok(double_address_verification: false) fill_in t('in_person_proofing.form.state_id.state_id_number'), with: GOOD_STATE_ID_NUMBER if double_address_verification - fill_in t('in_person_proofing.form.state_id.address1'), with: GOOD_ADDRESS1 - fill_in t('in_person_proofing.form.state_id.address2'), with: GOOD_ADDRESS2 - fill_in t('in_person_proofing.form.state_id.city'), with: GOOD_CITY - fill_in t('in_person_proofing.form.state_id.zipcode'), with: GOOD_ZIPCODE - choose t('in_person_proofing.form.state_id.same_address_as_id_no') + fill_in t('in_person_proofing.form.state_id.address1'), with: GOOD_STATE_ID_ADDRESS1 + fill_in t('in_person_proofing.form.state_id.address2'), with: GOOD_STATE_ID_ADDRESS2 + fill_in t('in_person_proofing.form.state_id.city'), with: GOOD_STATE_ID_CITY + fill_in t('in_person_proofing.form.state_id.zipcode'), with: GOOD_STATE_ID_ZIPCODE + if same_address_as_id + choose t('in_person_proofing.form.state_id.same_address_as_id_yes') + else + choose t('in_person_proofing.form.state_id.same_address_as_id_no') + end end end - def fill_out_address_form_ok(double_address_verification: false) - fill_in t('idv.form.address1'), with: GOOD_ADDRESS1 + def fill_out_address_form_ok(double_address_verification: false, same_address_as_id: false) + fill_in t('idv.form.address1'), + with: same_address_as_id ? GOOD_STATE_ID_ADDRESS1 : GOOD_ADDRESS1 fill_in t('idv.form.address2_optional'), with: GOOD_ADDRESS2 unless double_address_verification - fill_in t('idv.form.city'), with: GOOD_CITY - fill_in t('idv.form.zipcode'), with: GOOD_ZIPCODE - select GOOD_STATE, from: t('idv.form.state') + fill_in t('idv.form.address2'), + with: same_address_as_id ? GOOD_STATE_ID_ADDRESS2 : GOOD_ADDRESS2 + fill_in t('idv.form.city'), with: same_address_as_id ? GOOD_STATE_ID_CITY : GOOD_CITY + fill_in t('idv.form.zipcode'), with: same_address_as_id ? GOOD_STATE_ID_ZIPCODE : GOOD_ZIPCODE + if same_address_as_id + select GOOD_STATE_ID_JURISDICTION, + from: t('in_person_proofing.form.state_id.state_id_jurisdiction') + else + select GOOD_STATE, from: t('idv.form.state') + end unless double_address_verification choose t('in_person_proofing.form.address.same_address_choice_yes') end @@ -76,6 +92,7 @@ def complete_location_step(_user = nil) end def complete_prepare_step(_user = nil) + expect(page).to have_text(t('forms.buttons.continue'), wait: 10) click_spinner_button_and_wait t('forms.buttons.continue') end @@ -83,7 +100,10 @@ def complete_state_id_step(_user = nil, same_address_as_id: true, double_address_verification: false) # Wait for page to load before attempting to fill out form expect(page).to have_current_path(idv_in_person_step_path(step: :state_id), wait: 10) - fill_out_state_id_form_ok(double_address_verification: double_address_verification) + fill_out_state_id_form_ok( + double_address_verification: double_address_verification, + same_address_as_id: same_address_as_id, + ) click_idv_continue unless double_address_verification && same_address_as_id expect(page).to have_current_path(idv_in_person_step_path(step: :address), wait: 10) @@ -92,7 +112,7 @@ def complete_state_id_step(_user = nil, same_address_as_id: true, end def complete_address_step(_user = nil, double_address_verification: false) - fill_out_address_form_ok(double_address_verification:) + fill_out_address_form_ok(double_address_verification: double_address_verification) click_idv_continue end diff --git a/spec/support/features/verify_step_helper.rb b/spec/support/features/verify_step_helper.rb new file mode 100644 index 00000000000..953ebd29c8b --- /dev/null +++ b/spec/support/features/verify_step_helper.rb @@ -0,0 +1,19 @@ +module VerifyStepHelper + include InPersonHelper + + def expect_good_state_id_address + expect(page).to have_text(InPersonHelper::GOOD_STATE_ID_ADDRESS1) + expect(page).to have_text(InPersonHelper::GOOD_STATE_ID_ADDRESS2) + expect(page).to have_text(InPersonHelper::GOOD_STATE_ID_CITY) + expect(page).to have_text(Idp::Constants::MOCK_IDV_APPLICANT[:state_id_jurisdiction]) + expect(page).to have_text(InPersonHelper::GOOD_STATE_ID_ZIPCODE) + end + + def expect_good_address + expect(page).to have_text(InPersonHelper::GOOD_ADDRESS1) + expect(page).to have_content(t('idv.form.address2')) + expect(page).to have_text(InPersonHelper::GOOD_CITY) + expect(page).to have_text(Idp::Constants::MOCK_IDV_APPLICANT[:state]) + expect(page).to have_text(InPersonHelper::GOOD_ZIPCODE) + end +end From 43ea3c1af864402082235d3a16b6b58213b5076e Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Wed, 29 Mar 2023 16:03:17 -0700 Subject: [PATCH 13/13] Update fraud review rake tasks to handle nil data (LG-9271) (#8096) * Update users:review:pass task to handle nil/missing data (LG-9271) * Update users:review:reject to handle nil/missing data changelog: Internal, Fraud review, Update fraud review tasks to handle nil/missing data --------- Co-authored-by: Amir Reavis-Bey Co-authored-by: Sonia Connolly Co-authored-by: Sonia Connolly --- app/models/proofing_component.rb | 2 +- lib/tasks/review_profile.rake | 62 ++++++++++++++++---------- spec/lib/tasks/review_profile_spec.rb | 60 ++++++++++++++++++++++++- spec/models/proofing_component_spec.rb | 27 +++++++++++ 4 files changed, 126 insertions(+), 25 deletions(-) create mode 100644 spec/models/proofing_component_spec.rb diff --git a/app/models/proofing_component.rb b/app/models/proofing_component.rb index 59e45dd725b..0c6c38a5e55 100644 --- a/app/models/proofing_component.rb +++ b/app/models/proofing_component.rb @@ -2,6 +2,6 @@ class ProofingComponent < ApplicationRecord belongs_to :user def review_eligible? - verified_at.after?(30.days.ago) + verified_at&.after?(30.days.ago) end end diff --git a/lib/tasks/review_profile.rake b/lib/tasks/review_profile.rake index 66f92778099..c7ac1568e0b 100644 --- a/lib/tasks/review_profile.rake +++ b/lib/tasks/review_profile.rake @@ -5,18 +5,28 @@ namespace :users do desc 'Pass a user that has a pending review' task pass: :environment do |_task, args| STDOUT.sync = true - print 'Enter the name of the investigator: ' + STDOUT.print 'Enter the name of the investigator: ' investigator_name = STDIN.gets.chomp - print 'Enter the issue/ticket/reason for the investigation: ' + STDOUT.print 'Enter the issue/ticket/reason for the investigation: ' investigation_number = STDIN.gets.chomp - print 'Enter the User UUID to pass: ' + STDOUT.print 'Enter the User UUID to pass: ' user_uuid = STDIN.gets.chomp - puts "investigator name: #{investigator_name}" - puts "investigation reason: #{investigation_number}" - puts "uuid: #{user_uuid}" + STDOUT.puts "investigator name: #{investigator_name}" + STDOUT.puts "investigation reason: #{investigation_number}" + STDOUT.puts "uuid: #{user_uuid}" user = User.find_by(uuid: user_uuid) - if user.fraud_review_pending? && user.proofing_component.review_eligible? + if !user + STDOUT.puts 'Error: Could not find user with that UUID' + next + end + + if !user.fraud_review_pending? + STDOUT.puts 'Error: User does not have a pending fraud review' + next + end + + if user.proofing_component.review_eligible? profile = user.fraud_review_pending_profile profile.activate_after_passing_review @@ -31,40 +41,46 @@ namespace :users do disavowal_token: disavowal_token, ) - puts "User's profile has been activated and the user has been emailed." + STDOUT.puts "User's profile has been activated and the user has been emailed." else - puts "There was an error activating the user's profile please try again" + STDOUT.puts "There was an error activating the user's profile. Please try again" end - elsif !user.proofing_component.review_eligible? - puts 'User is past the 30 day review eligibility' else - puts 'User was not found pending a review' + STDOUT.puts 'User is past the 30 day review eligibility' end end desc 'Reject a user that has a pending review' task reject: :environment do |_task, args| STDOUT.sync = true - print 'Enter the name of the investigator: ' + STDOUT.print 'Enter the name of the investigator: ' investigator_name = STDIN.gets.chomp - print 'Enter the issue/ticket/reason for the investigation: ' + STDOUT.print 'Enter the issue/ticket/reason for the investigation: ' investigation_number = STDIN.gets.chomp - print 'Enter the User UUID to reject: ' + STDOUT.print 'Enter the User UUID to reject: ' user_uuid = STDIN.gets.chomp - puts "investigator name: #{investigator_name}" - puts "investigation reason: #{investigation_number}" - puts "uuid: #{user_uuid}" + STDOUT.puts "investigator name: #{investigator_name}" + STDOUT.puts "investigation reason: #{investigation_number}" + STDOUT.puts "uuid: #{user_uuid}" user = User.find_by(uuid: user_uuid) - if user.fraud_review_pending? && user.proofing_component.review_eligible? + if !user + STDOUT.puts 'Error: Could not find user with that UUID' + next + end + + if !user.fraud_review_pending? + STDOUT.puts 'Error: User does not have a pending fraud review' + next + end + + if user.proofing_component.review_eligible? profile = user.fraud_review_pending_profile profile.reject_for_fraud(notify_user: true) - puts "User's profile has been deactivated due to fraud rejection." - elsif !user.proofing_component.review_eligible? - puts 'User is past the 30 day review eligibility' + STDOUT.puts "User's profile has been deactivated due to fraud rejection." else - puts 'User was not found pending a review' + STDOUT.puts 'User is past the 30 day review eligibility' end end end diff --git a/spec/lib/tasks/review_profile_spec.rb b/spec/lib/tasks/review_profile_spec.rb index b12b5d98c2f..e6e6fec3d08 100644 --- a/spec/lib/tasks/review_profile_spec.rb +++ b/spec/lib/tasks/review_profile_spec.rb @@ -3,6 +3,7 @@ describe 'review_profile' do let(:user) { create(:user, :deactivated_threatmetrix_profile) } + let(:uuid) { user.uuid } let(:task_name) { nil } subject(:invoke_task) do @@ -10,14 +11,17 @@ Rake::Task[task_name].invoke end + let(:stdout) { StringIO.new } + before do Rake.application.rake_require('lib/tasks/review_profile', [Rails.root.to_s]) Rake::Task.define_task(:environment) allow(STDIN).to receive(:gets).and_return( "John Doe\n", "Rspec Test\n", - user.uuid, + uuid, ) + stub_const('STDOUT', stdout) end describe 'users:review:pass' do @@ -39,6 +43,33 @@ invoke_task end end + + context 'when the user does not exist' do + let(:user) { nil } + let(:uuid) { 'not-a-real-uuid' } + + it 'prints an error' do + invoke_task + + expect(stdout.string).to include('Error: Could not find user with that UUID') + end + end + + context 'when the user profile has a nil verified_at' do + let(:user) do + create( + :user, + :with_pending_in_person_enrollment, + proofing_component: build(:proofing_component), + ) + end + + it 'prints an error' do + invoke_task + + expect(stdout.string).to include('Error: User does not have a pending fraud review') + end + end end describe 'users:review:reject' do @@ -53,5 +84,32 @@ it 'sends the user an email about their account deactivation' do expect { invoke_task }.to change(ActionMailer::Base.deliveries, :count).by(1) end + + context 'when the user does not exist' do + let(:user) { nil } + let(:uuid) { 'not-a-real-uuid' } + + it 'prints an error' do + invoke_task + + expect(stdout.string).to include('Error: Could not find user with that UUID') + end + end + + context 'when the user profile has a nil verified_at' do + let(:user) do + create( + :user, + :with_pending_in_person_enrollment, + proofing_component: build(:proofing_component), + ) + end + + it 'prints an error' do + invoke_task + + expect(stdout.string).to include('Error: User does not have a pending fraud review') + end + end end end diff --git a/spec/models/proofing_component_spec.rb b/spec/models/proofing_component_spec.rb new file mode 100644 index 00000000000..f0ba6887f40 --- /dev/null +++ b/spec/models/proofing_component_spec.rb @@ -0,0 +1,27 @@ +require 'rails_helper' + +RSpec.describe ProofingComponent do + describe '#review_eligible?' do + subject(:review_eligible?) do + build(:proofing_component, verified_at: verified_at).review_eligible? + end + + context 'when verified_at is nil' do + let(:verified_at) { nil } + + it { is_expected.to be_falsey } + end + + context 'when verified_at is within 30 days' do + let(:verified_at) { 15.days.ago } + + it { is_expected.to be_truthy } + end + + context 'when verified_at is older than 30 days' do + let(:verified_at) { 45.days.ago } + + it { is_expected.to be_falsey } + end + end +end