Skip to content

LG-11128: Allow skip to account #9410

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mdiarra3 merged 12 commits into from
Oct 24, 2023
+9 −1
Merged

LG-11128: Allow skip to account #9410

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
9b739d9
changelog: User-Facing Improvements, Authentication, In person proofi…
mdiarra3 Oct 18, 2023
bb9144d
remove account path check
mdiarra3 Oct 18, 2023
5a7d7ae
remove return
mdiarra3 Oct 18, 2023
7a55d5f
skip when in pending profile
mdiarra3 Oct 18, 2023
cba38fe
in person spec, fix verify profile concern fix
mdiarra3 Oct 19, 2023
7ba9609
Merge remote-tracking branch 'origin/main' into LG-11128-skip-to-acco…
mdiarra3 Oct 20, 2023
17a0a3e
update spec
mdiarra3 Oct 20, 2023
ba4fe96
in personccccccnucjfbjecbfguhbrdfbtvlvidgibetckftjthu
mdiarra3 Oct 20, 2023
0b8b89a
update spec
mdiarra3 Oct 20, 2023
c438f42
Merge remote-tracking branch 'origin/main' into LG-11128-skip-to-acco…
mdiarra3 Oct 23, 2023
e5aa1de
sign in user
mdiarra3 Oct 23, 2023
e0f002d
also 2fa
mdiarra3 Oct 23, 2023
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
2 changes: 1 addition & 1 deletion app/controllers/application_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ def after_sign_in_path_for(_user)
end

def signed_in_url
return url_for_pending_profile_reason if user_has_pending_profile?
return idv_verify_by_mail_enter_code_url if current_user.gpo_verification_pending_profile?
Copy link
Copy Markdown
Contributor

@soniaconnolly soniaconnolly Oct 19, 2023

Choose a reason for hiding this comment

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

url_for_pending_profile_reason also redirects for profiles that have fraud_review_pending or fraud_rejection status. I think we should change the behavior of that method, not remove it here. And add feature specs that confirm that users are redirected as expected - I'm surprised nothing failed.

I was going to run the in_person feature spec to get a look at idv_in_person_ready_to_verify_url and see if the user would still need to be able to get there, but my local setup won't run feature specs suddenly :-(. Do we have a banner on the account page to allow the user to access that page?

Copy link
Copy Markdown
Contributor

@aduth aduth Oct 19, 2023

Choose a reason for hiding this comment

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

@soniaconnolly Does the discussion at #9410 (comment) help contextualize the reason for the change? My fear with modifying url_for_pending_profile_reason is that method is used elsewhere like IdvController where we'd want the fraud review and fraud rejection screens to be shown, but we don't want those to be shown when signing in at IAL1.

Copy link
Copy Markdown
Contributor

@soniaconnolly soniaconnolly Oct 19, 2023

Choose a reason for hiding this comment

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

Oh, I see. That makes more sense, thanks.

return backup_code_reminder_url if user_needs_backup_code_reminder?
account_path
end
Expand Down
8 changes: 8 additions & 0 deletions spec/features/idv/in_person_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@
# signing in again before completing in-person proofing at a post office
Capybara.reset_session!
sign_in_live_with_2fa(user)
visit_idp_from_sp_with_ial2(:oidc)
expect(page).to have_current_path(idv_in_person_ready_to_verify_path)
end
end
Expand Down Expand Up @@ -232,6 +233,7 @@
# signing in again before completing in-person proofing at a post office
Capybara.reset_session!
sign_in_live_with_2fa(user)
visit_idp_from_sp_with_ial2(:oidc)
expect(page).to have_current_path(idv_in_person_ready_to_verify_path)

# confirm that user cannot visit other IdV pages before completing in-person proofing
Expand All @@ -241,6 +243,11 @@
expect(page).to have_current_path(idv_in_person_ready_to_verify_path)
visit idv_verify_info_url
expect(page).to have_current_path(idv_in_person_ready_to_verify_path)

# Confirms that user can visit account page even if not completing in person proofing
Capybara.reset_session!
sign_in_and_2fa_user(user)
expect(page).to have_current_path(account_path)
end

it 'allows the user to cancel and start over from the beginning', allow_browser_log: true do
Expand Down Expand Up @@ -900,6 +907,7 @@
# signing in again before completing in-person proofing at a post office
Capybara.reset_session!
sign_in_live_with_2fa(user)
visit_idp_from_sp_with_ial2(:oidc)
expect(page).to have_current_path(idv_in_person_ready_to_verify_path)
end
end
Expand Down