LG-362 LOA3 fail states by davemcorwin · Pull Request #2259 · 18F/identity-idp

davemcorwin · 2018-06-19T22:36:11Z

Apology

This PR got a little out of hand...

Description

Addresses part of LG362, specifically all of the failure screens a user may see during the verification (LOA3) process.

Implement redesign of failure screens (removing modals)
Refactor i18n names to match removal of modal
Refactor to using Presenters consistent with all new failure screens and remove IDV view_model
Refactor handling of failures and flow in during verification
Added data (first name = 'Time') that can trigger a timeout in the Mock Resolution Proofer

Notes:

For the most part the failure screens now have unique urls which makes it easier to reproduce them AND for users to communicate where they are with customer support.
Removing the modals makes the app a bit easier to maintain since we only have to worry about one flow, there are still modals elsewhere.

Screenshots

Hi! Before submitting your PR for review, and/or before merging it, please
go through the following checklist:

For DB changes, check for missing indexes, check to see if the changes
affect other apps (such as the dashboard), make sure the DB columns in the
various environments are properly populated, coordinate with devops, plan
migrations in separate steps.
For route changes, make sure GET requests don't change state or result in
destructive behavior. GET requests should only result in information being
read, not written.
For encryption changes, make sure it is compatible with data that was
encrypted with the old code.
For secrets changes, make sure to update the S3 secrets bucket with the
new configs in all environments.
Do not disable Rubocop or Reek offenses unless you are absolutely sure
they are false positives. If you're not sure how to fix the offense, please
ask a teammate.
When reading data, write tests for nil values, empty strings,
and invalid formats.
When calling redirect_to in a controller, use _url, not _path.
When adding user data to the session, use the user_session helper
instead of the session helper so the data does not persist beyond the user's
session.
When adding a new controller that requires the user to be fully
authenticated, make sure to add before_action :confirm_two_factor_authenticated.

jgsmith-usds · 2018-06-28T16:36:13Z

app/controllers/idv/jurisdiction_controller.rb

    end
+
+    def failure_url(reason)
+      idv_jurisdiction_failure_url(reason)


Doesn't look like we have cases that need a failure_url?

This is used by a few of the IdV concerns:

IdvSessionConcern

IdvStepConcern

jgsmith-usds · 2018-06-28T16:37:10Z

app/controllers/idv/phone_controller.rb


    def create
-      result = idv_form.submit(step_params)
+      @idv_form = idv_form


Why the use of @idv_form and @_idv_form (line 76)? I'd just use idv_form everywhere.

jgsmith-usds · 2018-06-28T16:39:30Z

app/controllers/idv/sessions_controller.rb


    def create
-      result = idv_form.submit(profile_params)
+      @idv_form = idv_form


Similar comment as above. idv_form (line 99) uses an instance variable @_idv_form to store this. If you do need @idv_form, then perhaps change @_idv_form to @idv_form and then use idv_form here to make sure it's defined (in case you want it in a view).

jgsmith-usds · 2018-06-28T16:40:06Z

app/controllers/idv/sessions_controller.rb

-    def process_success
-      redirect_to idv_session_success_url
+    def process_form_failure
+      redirect_to idv_session_failure_url(:dupe_ssn) and return if @idv_form.duplicate_ssn?


Just use idv_form to get the form. No need for the instance variable.

jgsmith-usds · 2018-06-28T16:41:57Z

app/decorators/service_provider_session_decorator.rb


  def sp_alert_name
-    sp_alert? ? SP_ALERTS[sp_name][:i18n_name] : nil
+    SP_ALERTS.dig(sp_name, :i18n_name) || nil


If SP_ALERTS[sp_name][:i18n_name] doesn't exist, it'll return nil anyway, so || nil isn't really necessary.

jgsmith-usds · 2018-06-28T16:45:53Z

app/views/idv/sessions/new.html.slim

  'https://login.gov/help/privacy-and-security/how-does-logingov-protect-my-data/'

-= simple_form_for(@view_model.idv_form, url: idv_session_path,
+= simple_form_for(@idv_form, url: idv_session_path,


One of the patterns I'm playing with in the 2fa modularization effort is making the form a property of the presenter object. This is helpful especially when parts of the presenter might depend on information in the form, such as error states. But in that case, I'd make things like the dob (line 22 below) a property of the presenter, and have the presenter figure out how to get it from the form.

Since we're focusing on adding fail states to the IdV flow here, I'd call refactoring the form / presenter object relationship for this kind of out of scope. We can come back and investigate though.

jmhooper · 2018-07-11T19:24:03Z

@jgsmith-usds: worked through this w/ @mryenq, wanna take another look?

jmhooper · 2018-07-11T20:28:51Z

app/views/idv/shared/other_failure.html.slim

@@ -0,0 +1,5 @@
+= render 'shared/failure', presenter: presenter


One change we want to make before merging is rename this idv_failure since this view is specific to idv failures

davemcorwin

@jmhooper Your changes LGTM, just made one small syntax change.

jmhooper

LGTM2

**Why**: #2259 replaced modal dialogs with dedicated alert screens, and this code is now unused.

* Notify via email when resetting password from RISC (LG-3120) (#4177) * Combine reset password classes * Remove rake task and script * Update email copy * Remove docker branch builds, leave "build-release-container" (#4181) * convert idv cac success template to erb * convert backup code setup confirm delete template to erb * convert backup code setup confirm edit template to erb * convert piv cac auth setup error template to erb * Add more randomness to Agency factories (#4186) **Why**: We have a unique index on agency name * fix: Gemfile & Gemfile.lock to reduce vulnerabilities (#4187) The following vulnerabilities are fixed with an upgrade: - https://snyk.io/vuln/SNYK-RUBY-ACTIONVIEW-632514 * refactor account page a bit * placeholder * lint * rubocop * add redirect * fix backup codes spec * add translations to mobile menu * move two factor to own controller * move connected accounts to own controller * move history to own controller * fix indent * fix menu not showing on tablets * fix some specs * fix some specs * rubocop * fix greeting and header nav * fix more specs * double quotes in html attributes * update redirects for piv/cac setup * use USWDS breakpoints * convert navigation class method to being instantiated * rubocop * csscop * fix piv cac spec * remove semi-circle from user-access image * internationalization for new account banner * normalize yaml * update events page to align with new account page design * review changes * LG-3118 Fix bugs discovered while testing AAL3 flows (#4182) This commit fixes 2 bugs: 1.) The WebAuthn setup was not marking the session as AAL3, so the user was needed to interact with their token twice 2.) The "Don't have your <method>" text was appearing on the PIV/WebAuthn verification screen when the user did not have an option to change methods * LG-3334: Support failure testing for document capture in development environments (#4173) * Allow AcuantCapture to receive non-image files in non-production environments **Why**: As a developer, I expect that I can upload files which simulate expected failures which can occur during document capture, so that I can reliably test non-happy-path flows. * Return error from YAML in non-production image upload * rubocop * Fix client test specs * Pass mock client as detail to root upload context * Pass through non-image files to client implementation See: https://github.com/18F/identity-idp/pull/4173/files#r485970570 * Remove unused i18n keys * update user access svg to hide bottom corners * LG-3118 Update the webauthn verification headers (#4190) This was discovered during LG-3118. The headers on the WebAuthn verification screen are supposed to ask you to connect instead of present your key. * Separate links to Privacy Act Statement from Practices (LG-3467) (#4188) * update banner to for mobile design changes * LG-873 Add x509_issuer to piv/cac response data for OIDC (#4174) * Change "Sign in" to "Continue" on user authorization confirmation (#4189) This was found as part of LG-3118. The "Sign in" button on this page should say "Continue". While I was in there I noticed the rest of the text on the page was not translated so I went ahead and took care of that. * LG-3342: Rate limit image uploads during doc auth API response (#4185) * LG-3342: Rate limit image uploads during doc auth API response **Why:** As a legitimate user doing doc auth, I want the image uploads in the react flow to be uploaded, so that users attempting to steal an identity have a harder time proofing. * Pass image form status as argument to render_form_response * Memoize image upload form throttle increment to avoid duplicate counting * Increment throttle by submission See: #4185 (comment) * Assign fallback status at time of use See: #4185 (comment) Co-Authored-By: Zach Margolis <zbmargolis@gmail.com> * Create ImageUploadResponsePresenter to handle image upload response * Add test spec for ImageUploadResponsePresenter Co-authored-by: Zach Margolis <zbmargolis@gmail.com> * do not show account banner on mobile * Forget remembered browsers after RISC password reset (LG-3120) (#4195) * Update CSS style guide image path (#4198) * LG-3294 Replace raw NET::HTTP with Faraday equivalent. (#4197) LG-3294 Replace raw NET::HTTP with Faraday equivalent for the piv_cac_service. * Add maintenance window notification for Acuant (LG-3451) (#4202) Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> * LG-3320 Prevent uploading selfie (#4199) * Use image-path CSS helper function to cache asset (#4203) * LG-3463: Rate limit image uploads during doc auth: Client response (#4193) * LG-3463: Rate limit image uploads during doc auth: Client response **Why**: As a user who has exceeded the allowable limit of doc auth attempts, I expect to be presented with a message explaining why I'm no longer able to submit my documents. With these changes, when the client detects there are no more attempts, the user is redirected to the error state. See: #4185 (LG-3342) * Return redirect URL in 429 response * Update test spec * Use ref callback to initialize FullScreen modal focus trap (#4200) **Why**: This may resolve an issue where focus trap's event handling is not removed after a FullScreen dialog is hidden, under the assumption that the previous implementation could leave potential for desync between modalRef and useEffect, or between the useEffect's unsubscribe behavior and trapRef.current.deactivate. * LG-3487: Remove file size validation for doc auth images (#4205) **Why**: As a user, I expect that even if my mobile device captures photos with low quality, I will be allowed to submit them in case Acuant will still select them, so that I am not blocked from proofing. * LG-3473: Log analytics for the image upload API (#4201) * LG-3473: Log analytics for the image upload API **Why**: As a user, I want login.gov to log analytics for the doc auth image upload API, so that if I am having an issue the login.gov team can debug it * rubocop * Log analytics using analytics.track_event * Rename doc auth image upload analytics constants **Why**: Clarity * Omit braces in last hash arguments **Why**: See #4201 (comment) * Tweak PIV/CAC and WebAuthn verification controllers (#4191) This was a discovery during LG-3118. This commit has 2 fixes: 1. The presenters renders the help text for AAL3 only when the user could only use the current method. They should render the help text if an AAL3 sign in is required 2. Remove duplicate paragraph from the PIV/CAC screen * Remove redundant "Connect your key" header (#4204) **Why**: We changed the context of the header on the page, making this header duplicative * Remove unused "verify-modal" JavaScript pack (#4210) **Why**: #2259 replaced modal dialogs with dedicated alert screens, and this code is now unused. * LexisNexis doc_auth client now available for use (LG-3263) (#4184) * LexisNexis doc_auth client now available for use (LG-3263) Co-authored-by: amathews <amathews@fearsol.com> * LG-3476: Upgrade focus-trap from ^2.3.0 (2.4.6) to 6.0.1 (#4211) * Upgrade focus-trap from ^2.3.0 (2.4.6) to 6.0.1 * Simplify components index file **Why**: - Exports unused - TrapModal variable as pass-through is unnecessary, since direct access to Modal import is available * Remove proxyquire (#4212) **Why**: While occasionally useful to mock out the behavior of underlying modules, it can give a false sense of security that the behavior of a mocked implementation will match the experience and expectations of the user-facing behavior. * Fix mobile nav rendering (#4209) * make sure nav is rendered in a valid place * render mobile nav as child of body * Update app/views/layouts/base.html.erb Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> * add usa-overlay div * add title to new account pages Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> * Remove unused "if checked" JavaScript form validation (#4214) * Remove unused submit auto-enable form validation (#4216) **Why**: While some forms implement custom auto-enable behavior for submit buttons, they do so by bypassing the default auto-enable behavior. There does not appear to be any instances of forms which rely on this core behavior. * LG-3339: Extract submit-with-spinner pack from form-validation.js (#4219) * Extract submit-with-spinner pack from form-validation.js * Add comment for custom event dispatching * Define element-closest as dependency of polyfill package * Convert a few more templates to erb (#4220) * convert edit password templates to erb * convert event disavowal new template to erb * convert pii_review template to erb * convert idv review new template to erb * Update app/views/shared/_pii_review.html.erb Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com> * Update app/views/shared/_pii_review.html.erb Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com> * Update app/views/shared/_pii_review.html.erb Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com> * Update app/views/shared/_pii_review.html.erb Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com> * remove extra space * fix specs Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com> * update rubocop to 0.91.0 (#4218) * Convert some more slim templates (#4222) * remove unused code in layout * convert idv come back later template to erb * convert verify_personal_key template to erb * convert devise passwords new template to erb * convert page took too long template to erb * convert in person find usps template to erb * fix unnecessary space * Use Acuant SDK for selfie capture (#4224) * Use AcuantPassiveLiveness for mobile selfie capture * Cache image data blob * Fix test specs * LG-3339: Make functionality in form-validation.js opt-in (#4221) * LG-3339: Make functionality in form-validation.js opt-in **Why**: As a developer, I want the behavior of forms to be predictable and enable automatic validations only when intended, so that I'm not caught in a situation where automatic validations occur unexpectedly and break user behavior. * Pass through simple_form_for arguments as given * Rename idp_simple_form_for as validated_form_for * Print form validation script once with data attribute * Explicitly include Webpacker helper * Print javascript_pack_tag using variadic arguments * Add test spec for form-validation script * Add test spec for script helper * Rename print_javascript_pack_once_tags to render_javascript_pack_once_tags * Use validated_form_for globally * Preserve this reference in I18n calls * Substitute classlist.js with classlist-polyfill (#4225) * Pass fields used to check if liveness is enabled to the hybrid flow (LG-3471) (#4223) Pass fields used to check if liveness is enabled to the hybrid flow (LG-3471) * LG-3543 Update doc captures to avoid showing the selfie in the hybrid… (#4230) The selfie step currently appears in the hybrid flow even if SPs don't request it. This commit backports the changes from #4223 to the old doc captures table so this bug is fixed for the old liveness flow. Co-authored-by: Doug Price <dprice@fearless.tech> Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com> Co-authored-by: Mitchell Henke <mitchell.henke@gsa.gov> Co-authored-by: Snyk bot <snyk-bot@snyk.io> Co-authored-by: Jonathan Hooper <jonathan.hooper@gsa.gov> Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> Co-authored-by: Steve Urciuoli <steve.urciuoli@gsa.gov> Co-authored-by: Zach Margolis <zbmargolis@gmail.com> Co-authored-by: Shade L. Jenifer <shade.jenifer@gsa.gov> Co-authored-by: Alex Mathews <44584810+amathews-fs@users.noreply.github.com>

davemcorwin added 11 commits June 14, 2018 13:24

Make the generic failure template a partial

390d185

Update jurisdiction fail screens

22c5862

Fix translations;

a4a746f

Explicitly set translations as used

02d7622

Lint fixes

377dacc

Merge branch 'master' into LG-362-idv-attempts-fail-screen

901e994

Update profile attempts fail screen

c27fdf9

Update specs

541a955

Merge branch 'master' into LG-362-idv-attempts-fail-screen

041db2d

WIP

7ad119a

A lot of cleanup

530e8f4

davemcorwin added the status - work in progress label Jun 19, 2018

davemcorwin changed the title ~~Lg 362 idv attempts fail screen~~ Lg 362 idv attempts fail screens Jun 19, 2018

davemcorwin added 13 commits June 19, 2018 15:37

Fix specs

9f0d483

Merge branch 'master' into LG-362-idv-attempts-fail-screen

5c857ee

Rubocop fixes

b9dff27

Rubocop fix

556762d

Add presenter specs

eef8367

Update spec, i18n path

3173d54

More presenter specs

fd88f8c

Merge branch 'master' into LG-362-idv-attempts-fail-screen

cdd535d

Add failure specs to sessions controller spec

11539dd

Fix SP return urls

551d914

Add timeout example to moc resolution proofer

66a287c

Fix spec

4b388b2

Fix spec again

429211c

davemcorwin added status - ready for review and removed status - work in progress labels Jun 21, 2018

davemcorwin added 2 commits June 25, 2018 12:59

Exclude proofer mocks from test coverage

aa4f705

Merge branch 'master' into LG-362-idv-attempts-fail-screen

800c5e7

jgsmith-usds reviewed Jun 28, 2018

View reviewed changes

jmhooper added 4 commits July 11, 2018 10:38

Merge branch 'master' into LG-362-idv-attempts-fail-screen

ea0a130

clean up failure urls and add form before actions

da4ba0e

fix some tests

081f08b

reduce cognition

b6c1384

jmhooper reviewed Jul 11, 2018

View reviewed changes

rename other failure to verification failure

9c79ef1

mryenq changed the title ~~Lg 362 idv attempts fail screens~~ LG-362 LOA3 fail states Jul 12, 2018

Use the accessor for idv_form not the instance variable

d8d26cb

davemcorwin commented Jul 16, 2018

View reviewed changes

jmhooper approved these changes Jul 17, 2018

View reviewed changes

jmhooper merged commit 8542f03 into master Jul 17, 2018

aduth added a commit that referenced this pull request Sep 16, 2020

Remove unused "verify-modal" JavaScript pack

b4c80d1

**Why**: #2259 replaced modal dialogs with dedicated alert screens, and this code is now unused.

This was referenced Sep 16, 2020

Remove unused "verify-modal" JavaScript pack #4210

Merged

LG-3476: Upgrade focus-trap from ^2.3.0 (2.4.6) to 6.0.1 #4211

Merged

aduth added a commit that referenced this pull request Sep 16, 2020

Remove unused "verify-modal" JavaScript pack (#4210)

1f35efc

**Why**: #2259 replaced modal dialogs with dedicated alert screens, and this code is now unused.

aduth mentioned this pull request Nov 4, 2020

Use i18n-tasks to audit used JS locale strings #4378

Merged

amathews-fs deleted the LG-362-idv-attempts-fail-screen branch January 7, 2021 18:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LG-362 LOA3 fail states#2259

LG-362 LOA3 fail states#2259
jmhooper merged 32 commits intomasterfrom
LG-362-idv-attempts-fail-screen

davemcorwin commented Jun 19, 2018 •

edited

Loading

Uh oh!

jgsmith-usds Jun 28, 2018

Uh oh!

jmhooper Jul 11, 2018 •

edited

Loading

Uh oh!

jgsmith-usds Jun 28, 2018

Uh oh!

jgsmith-usds Jun 28, 2018

Uh oh!

jgsmith-usds Jun 28, 2018

Uh oh!

jgsmith-usds Jun 28, 2018

Uh oh!

jgsmith-usds Jun 28, 2018

Uh oh!

jmhooper Jul 11, 2018

Uh oh!

jmhooper commented Jul 11, 2018

Uh oh!

jmhooper Jul 11, 2018 •

edited

Loading

Uh oh!

davemcorwin left a comment

Uh oh!

jmhooper left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@@ -0,0 +1,5 @@
		= render 'shared/failure', presenter: presenter

Conversation

davemcorwin commented Jun 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Apology

Description

Notes:

Screenshots

Uh oh!

jgsmith-usds Jun 28, 2018

Choose a reason for hiding this comment

Uh oh!

jmhooper Jul 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jgsmith-usds Jun 28, 2018

Choose a reason for hiding this comment

Uh oh!

jgsmith-usds Jun 28, 2018

Choose a reason for hiding this comment

Uh oh!

jgsmith-usds Jun 28, 2018

Choose a reason for hiding this comment

Uh oh!

jgsmith-usds Jun 28, 2018

Choose a reason for hiding this comment

Uh oh!

jgsmith-usds Jun 28, 2018

Choose a reason for hiding this comment

Uh oh!

jmhooper Jul 11, 2018

Choose a reason for hiding this comment

Uh oh!

jmhooper commented Jul 11, 2018

Uh oh!

jmhooper Jul 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davemcorwin left a comment

Choose a reason for hiding this comment

Uh oh!

jmhooper left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

davemcorwin commented Jun 19, 2018 •

edited

Loading

jmhooper Jul 11, 2018 •

edited

Loading

jmhooper Jul 11, 2018 •

edited

Loading