Skip to content

Global IdV feature flag / new "IdV Unavailable" page#7970

Closed
matthinz wants to merge 33 commits intomainfrom
matthinz/8710-who-needs-idv
Closed

Global IdV feature flag / new "IdV Unavailable" page#7970
matthinz wants to merge 33 commits intomainfrom
matthinz/8710-who-needs-idv

Conversation

@matthinz
Copy link
Contributor

🎫 Ticket

LG-8710

🛠 Summary of changes

Provide a way to disable IdV independent of vendor status flags

There was a desire to be able to keep our existing VendorStatus infrastructure, but also provide a means for on-call engineers to completely disable identity verification independent of that.

So now, in addition to marking a required vendor as :full_outage, setting idv_available: false will disable IdV completely.

In code, FeatureManagement.idv_available? should be the preferred way of establishing whether identity verification is available to end users.

Add a new page explaining that Idv is down

The new page (see screenshots below) includes language related to identity verification, and so is not appropriate to be used as a generic error page. There are instances where the old vendor outage page were being used by non-IdV code—these are unmodified.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

  • Set idv_available: false in your config and restart your server
  • Try to verify
  • Get an error message

👀 Screenshots

With Service Provider:
English

sp-en

Spanish

sp-es

French

sp-fr

Without Service Provider:
English

no-sp-en

Spanish

no-sp-es

French

no-sp-fr

@matthinz matthinz requested a review from a team March 10, 2023 20:33
@matthinz
Copy link
Contributor Author

@aduth I’m leaning toward this approach to avoid adding code to ApplicationController:

  • Add a route (/errors/idv_unavailable) that non-IdV code (e.g. registration) can redirect to if needed.
  • Wire up routes under /verify to show the error screen (and return a 503) when idv is unavailable (as this PR already does)

This way users can hit refresh during IdV to 🤞 try and fix their issue, but we still have flexibility to redirect the user to a friendly "idv unavailable" from other parts of the app.

@mitchellhenke mitchellhenke force-pushed the matthinz/8710-who-needs-idv branch from e990ee3 to b24a680 Compare March 14, 2023 17:50
@matthinz matthinz force-pushed the matthinz/8710-who-needs-idv branch 2 times, most recently from 9917262 to 62ec030 Compare March 14, 2023 23:43
matthinz added 17 commits March 15, 2023 10:01
Tie feature to vendor availability as well as config.
Return a 503 service unavailable whenever IdV is unavailable.
The unused i18n detector was getting tripped up by the ternary conditional in here.
There's not really a route to this action by default.
Did not end up implementing this
Rather than bolting more functionality onto ApplicationController:

- Add a new route at `/errors/idv_unavailable` for non-IdV parts of the app to redirect to
- Take over `/verify/*` and show the unavailable message when it is.
When IdV becomes available again, try and send the user back where they were.
matthinz added 11 commits March 15, 2023 10:01
… more helpful and detailed error message when identity verification is unavailable.
We're not longer using this bit of VendorStatus.
Keep `redirect_from` analytics param for this case.

(503 responses will log with the event `path` set to the currnet location)
Use `show` for the action that shows the error, and move view to the more conventional `idv/unavailable/show.html.erb`.
For both use cases (fixed path at /errors/idv_unavailable AND catchall for /verify/* when idv is down), just use the "show" action.

Also, only return 503 Service Unavailable if idv is actually unavailable.
@matthinz matthinz force-pushed the matthinz/8710-who-needs-idv branch from 62ec030 to e116e8e Compare March 15, 2023 18:16
5XX errors will trigger nuisance alerts in monitoring tools. An error message that we have purposefully put in place should be regarded as "OK".
@aduth
Copy link
Contributor

aduth commented Mar 20, 2023

It appears that the spec failing in the build is relevant to the changes.

This was leftover from an earlier version of this PR.
@matthinz
Copy link
Contributor Author

I'm gonna mark this as a "work in progress" for now, as there is some overlap here with #8011 that I'd like to iron out after that lands.

@matthinz
Copy link
Contributor Author

I'm going to close this in favor of a new PR, since this has gone stale w/r/t main.

@matthinz matthinz closed this Mar 30, 2023
@matthinz matthinz deleted the matthinz/8710-who-needs-idv branch March 30, 2023 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants