Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 1 addition & 15 deletions app/services/idv/phone_step.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ def update_idv_session
idv_session.address_verification_mechanism = :phone
idv_session.applicant = applicant
idv_session.vendor_phone_confirmation = true
idv_session.user_phone_confirmation = phone_matches_user_phone?
idv_session.user_phone_confirmation = false
Copy link
Copy Markdown
Contributor Author

@mitchellhenke mitchellhenke Jul 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the crux and only thing needed in this PR to change the behavior, everything else is to support changing the specs

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if there are other things we could simplify if we can always assume OTP confirmation will be required. At first I wondered if we could get rid of this variable altogether, though it appears it's necessary to push people to the review step if they're already done with this one.

Maybe we could still clean up some of the OTP-related redirect logic?

e.g.

if phone_confirmation_required?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially? I'm not sure though, the current flow is:

  1. PhoneFinder
  2. Confirm Phone via OTP if needed

so the stuff we can delete in the app might be more limited. The tests are a different story though.


ProofingComponent.create_or_find_by(user: idv_session.current_user).
update(address_check: 'lexis_nexis_address')
Expand All @@ -119,20 +119,6 @@ def start_phone_confirmation_session
)
end

def phone_matches_user_phone?
applicant_phone = PhoneFormatter.format(applicant[:phone])
return false if applicant_phone.blank?
user_phones.include?(applicant_phone)
end

def user_phones
MfaContext.new(
idv_session.current_user,
).phone_configurations.map do |phone_configuration|
PhoneFormatter.format(phone_configuration.phone)
end.compact
end

def extra_analytics_attributes
{
vendor: idv_result.except(:errors, :success),
Expand Down
12 changes: 6 additions & 6 deletions spec/controllers/idv/phone_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@
expect(response).to render_template :new
put :create, params: { idv_phone_form: { phone: good_phone } }
get :new
expect(response).to redirect_to idv_review_path
expect(response).to redirect_to idv_otp_delivery_method_path
end

it 'shows waiting interstitial if async process is in progress' do
Expand Down Expand Up @@ -218,12 +218,12 @@
stub_verify_steps_one_and_two(user)
end

it 'redirects to review page and sets phone_confirmed_at' do
it 'redirects to otp delivery page' do
put :create, params: { idv_phone_form: { phone: good_phone } }

expect(response).to redirect_to idv_phone_path
get :new
expect(response).to redirect_to idv_review_path
expect(response).to redirect_to idv_otp_delivery_method_path

expected_applicant = {
'first_name' => 'Some',
Expand All @@ -234,7 +234,7 @@

expect(subject.idv_session.applicant).to eq expected_applicant
expect(subject.idv_session.vendor_phone_confirmation).to eq true
expect(subject.idv_session.user_phone_confirmation).to eq true
expect(subject.idv_session.user_phone_confirmation).to eq false
end

context 'with full vendor outage' do
Expand All @@ -243,12 +243,12 @@
and_return(true)
end

it 'redirects to review page' do
it 'redirects to vendor outage page' do
put :create, params: { idv_phone_form: { phone: good_phone } }

expect(response).to redirect_to idv_phone_path
get :new
expect(response).to redirect_to idv_review_path
expect(response).to redirect_to vendor_outage_path(from: :idv_phone)
end
end
end
Expand Down
12 changes: 4 additions & 8 deletions spec/features/accessibility/idv_pages_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@
scenario 'review page' do
sign_in_and_2fa_user
visit idv_path
complete_all_doc_auth_steps
click_idv_continue
complete_all_doc_auth_steps_before_password_step

expect(page).to have_current_path(idv_review_path)
expect(page).to be_axe_clean.according_to :section508, :"best-practice", :wcag21aa
Expand All @@ -52,8 +51,7 @@
scenario 'personal key / confirmation page' do
sign_in_and_2fa_user
visit idv_path
complete_all_doc_auth_steps
click_idv_continue
complete_all_doc_auth_steps_before_password_step
fill_in t('idv.form.password'), with: Features::SessionHelper::VALID_PASSWORD
click_continue

Expand All @@ -66,8 +64,7 @@
scenario 'doc auth steps accessibility' do
sign_in_and_2fa_user
visit idv_path
complete_all_doc_auth_steps(expect_accessible: true)
click_idv_continue
complete_all_doc_auth_steps_before_password_step(expect_accessible: true)
fill_in t('idv.form.password'), with: Features::SessionHelper::VALID_PASSWORD
click_continue

Expand All @@ -80,8 +77,7 @@
scenario 'doc auth steps accessibility on mobile', driver: :headless_chrome_mobile do
sign_in_and_2fa_user
visit idv_path
complete_all_doc_auth_steps(expect_accessible: true)
click_idv_continue
complete_all_doc_auth_steps_before_password_step(expect_accessible: true)
fill_in t('idv.form.password'), with: Features::SessionHelper::VALID_PASSWORD
click_continue

Expand Down
21 changes: 0 additions & 21 deletions spec/features/idv/doc_auth/finished_spec.rb

This file was deleted.

3 changes: 2 additions & 1 deletion spec/features/idv/hybrid_flow_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,9 @@
expect(page).to have_content(t('headings.verify'))
click_idv_continue

fill_out_phone_form_mfa_phone(user)
fill_out_phone_form_ok
click_idv_continue
verify_phone_otp

fill_in t('idv.form.password'), with: Features::SessionHelper::VALID_PASSWORD
click_idv_continue
Expand Down
1 change: 1 addition & 0 deletions spec/features/idv/in_person_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@
expect(page).to have_content(t('idv.titles.session.phone'))
fill_out_phone_form_mfa_phone(user)
click_idv_continue
verify_phone_otp

# password confirm page
expect(page).to have_content(
Expand Down
7 changes: 3 additions & 4 deletions spec/features/idv/proofing_components_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@

expect(current_path).to eq idv_doc_auth_step_path(step: :welcome)

complete_all_doc_auth_steps
click_idv_continue
complete_all_doc_auth_steps_before_password_step
fill_in 'Password', with: Features::SessionHelper::VALID_PASSWORD
click_continue
acknowledge_and_confirm_personal_key
Expand Down Expand Up @@ -52,7 +51,7 @@

it 'clears liveness enabled proofing component when user re-proofs without liveness', js: true do
allow(IdentityConfig.store).to receive(:liveness_checking_enabled).and_return(true)
user = user_with_2fa
user = user_with_totp_2fa
sign_in_and_2fa_user(user)
visit_idp_from_oidc_sp_with_ial2_strict
complete_proofing_steps
Expand All @@ -64,7 +63,7 @@

trigger_reset_password_and_click_email_link(user.email)
reset_password_and_sign_back_in(user, user.password)
fill_in_code_with_last_phone_otp
fill_in_code_with_last_totp(user)
click_submit_default

expect(user.reload.profiles.where(active: true)).to be_empty
Expand Down
25 changes: 12 additions & 13 deletions spec/features/idv/steps/phone_step_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,26 @@
include IdvHelper

context 'with valid information' do
it 'allows the user to continue to the phone otp delivery selection step' do
start_idv_from_sp
complete_idv_steps_before_phone_step
fill_out_phone_form_ok
click_idv_continue
# it 'allows the user to continue to the phone otp delivery selection step' do
# start_idv_from_sp
# complete_idv_steps_before_phone_step
# fill_out_phone_form_ok
# click_idv_continue

expect(page).to have_content(t('idv.titles.otp_delivery_method'))
expect(page).to have_current_path(idv_otp_delivery_method_path)
end
# expect(page).to have_content(t('idv.titles.otp_delivery_method'))
# expect(page).to have_current_path(idv_otp_delivery_method_path)
# end

it 'redirects to the confirmation step when the phone matches the 2fa phone number', js: true do
it 'redirects to the otp delivery step when the phone matches the 2fa phone number', js: true do
user = user_with_2fa
start_idv_from_sp
complete_idv_steps_before_phone_step(user)
fill_out_phone_form_ok(MfaContext.new(user).phone_configurations.first.phone)

click_idv_continue

expect(page).to have_content(t('idv.titles.session.review', app_name: APP_NAME))
expect(page).to have_current_path(idv_review_path)
expect(page).to have_content(t('idv.titles.otp_delivery_method', app_name: APP_NAME))
expect(page).to have_current_path(idv_otp_delivery_method_path)
end

it 'allows a user without a phone number to continue' do
Expand Down Expand Up @@ -124,14 +124,13 @@
complete_idv_steps_before_phone_step(user)

allow(DocumentCaptureSession).to receive(:find_by).and_return(nil)

fill_out_phone_form_ok(MfaContext.new(user).phone_configurations.first.phone)
click_idv_continue
expect(page).to have_content(t('idv.failure.timeout'))
expect(page).to have_current_path(idv_phone_path)
allow(DocumentCaptureSession).to receive(:find_by).and_call_original
click_idv_continue
expect(page).to have_current_path(idv_review_path)
expect(page).to have_current_path(idv_otp_delivery_method_path)
end
end

Expand Down
6 changes: 2 additions & 4 deletions spec/features/idv/strict_ial2/upgrade_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@

expect(page.current_path).to eq(idv_doc_auth_welcome_step)

complete_all_doc_auth_steps
click_idv_continue
complete_all_doc_auth_steps_before_password_step
fill_in 'Password', with: user.password
click_continue
acknowledge_and_confirm_personal_key
Expand Down Expand Up @@ -53,8 +52,7 @@

expect(page.current_path).to eq(idv_doc_auth_welcome_step)

complete_all_doc_auth_steps
click_idv_continue
complete_all_doc_auth_steps_before_password_step
fill_in 'Password', with: user.password
click_continue
acknowledge_and_confirm_personal_key
Expand Down
3 changes: 1 addition & 2 deletions spec/features/idv/strict_ial2/usps_upload_disallowed_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@
# Directed to the start of the proofing flow instead of GPO code verification
expect(current_path).to eq(idv_doc_auth_step_path(step: :welcome))

complete_all_doc_auth_steps
click_idv_continue
complete_all_doc_auth_steps_before_password_step
fill_in 'Password', with: user.password
click_continue
acknowledge_and_confirm_personal_key
Expand Down
6 changes: 1 addition & 5 deletions spec/features/openid_connect/openid_connect_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -262,11 +262,7 @@
scope: 'openid email profile',
verified_within: '30d',
proofing_steps: proc do
complete_all_doc_auth_steps

fill_out_phone_form_mfa_phone(user)
click_idv_continue

complete_all_doc_auth_steps_before_password_step
fill_in t('idv.form.password'), with: Features::SessionHelper::VALID_PASSWORD
click_continue

Expand Down
5 changes: 2 additions & 3 deletions spec/features/sp_cost_tracking_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,9 @@

visit_idp_from_sp_with_ial2(:oidc, verified_within: '45d')
fill_in_credentials_and_submit(user.confirmed_email_addresses.first.email, password)
fill_in_code_with_last_phone_otp
fill_in_code_with_last_totp(user)
click_submit_default
complete_all_doc_auth_steps
click_idv_continue
complete_all_doc_auth_steps_before_password_step
fill_in 'Password', with: password
click_continue
acknowledge_and_confirm_personal_key
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@
click_link t('account.index.reactivation.link')
click_on t('links.account.reactivate.without_key')
click_on t('forms.buttons.continue')
complete_all_doc_auth_steps
click_idv_continue
complete_all_doc_auth_steps_before_password_step
fill_in 'Password', with: new_password
click_idv_continue
acknowledge_and_confirm_personal_key
Expand Down
3 changes: 1 addition & 2 deletions spec/features/users/user_profile_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,7 @@
fill_in_code_with_last_phone_otp
click_submit_default
click_on t('links.account.reactivate.without_key')
complete_all_doc_auth_steps
click_idv_continue
complete_all_doc_auth_steps_before_password_step
fill_in 'Password', with: user_password
click_idv_continue
acknowledge_and_confirm_personal_key
Expand Down
4 changes: 2 additions & 2 deletions spec/services/idv/phone_step_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@
expect { subject.submit(phone: fail_phone) }.to_not change { throttle.attempts }
end

it 'marks the phone as confirmed if it matches 2FA phone' do
it 'marks the phone as unconfirmed if it matches 2FA phone' do
user.phone_configurations = [build(:phone_configuration, user: user, phone: good_phone)]

subject.submit(phone: good_phone)
Expand All @@ -120,7 +120,7 @@

expect(result.success?).to eq(true)
expect(idv_session.vendor_phone_confirmation).to eq(true)
expect(idv_session.user_phone_confirmation).to eq(true)
expect(idv_session.user_phone_confirmation).to eq(false)
end

it 'does not mark the phone as confirmed if it does not match 2FA phone' do
Expand Down
24 changes: 22 additions & 2 deletions spec/support/features/doc_auth_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,13 @@ def complete_doc_auth_steps_before_email_sent_step
click_on t('doc_auth.info.upload_computer_link')
end

def complete_doc_auth_steps_before_phone_otp_step(expect_accessible: false)
complete_doc_auth_steps_before_verify_step(expect_accessible: expect_accessible)
click_idv_continue
expect(page).to be_axe_clean.according_to :section508, :"best-practice" if expect_accessible
click_idv_continue
end

def mobile_device
Browser.new(
'Mozilla/5.0 (iPhone; CPU iPhone OS 11_0 like Mac OS X) \
Expand Down Expand Up @@ -175,10 +182,17 @@ def complete_all_doc_auth_steps(expect_accessible: false)
expect(page).to be_axe_clean.according_to :section508, :"best-practice" if expect_accessible
end

def complete_proofing_steps
complete_all_doc_auth_steps
def complete_all_doc_auth_steps_before_password_step(expect_accessible: false)
complete_all_doc_auth_steps(expect_accessible: expect_accessible)
fill_out_phone_form_ok if find('#idv_phone_form_phone').value.blank?
click_continue
verify_phone_otp
expect(page).to have_current_path(idv_review_path, wait: 10)
expect(page).to be_axe_clean.according_to :section508, :"best-practice" if expect_accessible
end

def complete_proofing_steps
complete_all_doc_auth_steps_before_password_step
fill_in 'Password', with: RequestHelper::VALID_PASSWORD
click_continue
acknowledge_and_confirm_personal_key
Expand Down Expand Up @@ -253,6 +267,12 @@ def set_up_document_capture_result(uuid:, idv_result:)
end
end

def verify_phone_otp
choose_idv_otp_delivery_method_sms
fill_in_code_with_last_phone_otp
click_submit_default
end

def fill_out_address_form_ok
fill_in 'idv_form_address1', with: '123 Main St'
fill_in 'idv_form_city', with: 'Nowhere'
Expand Down
Loading