Skip to content

LG-5184: error page on Acuant or LexisNexis full outage#5551

Merged
solipet merged 25 commits intomainfrom
dprice-lg-5184-full-outage-page
Nov 1, 2021
Merged

LG-5184: error page on Acuant or LexisNexis full outage#5551
solipet merged 25 commits intomainfrom
dprice-lg-5184-full-outage-page

Conversation

@solipet
Copy link
Contributor

@solipet solipet commented Oct 27, 2021

Why: As a user, I expect that if I'm unable to complete identity verification within Login.gov due to an outage of one or more vendors, I am prevented from starting the flow and shown a message which explains the situation.

If the idp is configured to explicitly acknowledge a full outage of the Acuant, LexisNexis Instant Verify, or LexisNexis TrueID services, the user is blocked from proceeding with Identity Proofing. The three cases for this are when a user comes from an SP requiring IAL2 and:

  • they attempt to create a new account,
  • they sign in to their existing IAL1 account and would normally go into the proofing flow, or
  • they forget/reset their password, and need to re-verify without their personal key.

@solipet solipet marked this pull request as ready for review October 29, 2021 17:54
@solipet solipet requested review from aduth and zachmargolis October 29, 2021 17:54

before_action :override_document_capture_step_csp
before_action :update_if_skipping_upload
# rubocop:disable Rails/LexicallyScopedActionFilter
Copy link
Contributor Author

Choose a reason for hiding this comment

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

had to disable this cop since :show is not explicitly defined on this controller but via magic in the FSM

Comment on lines +55 to +57
if enum && !enum.include?(converted_value)
raise "unexpected #{key}: #{value}, expected one of #{enum}"
end
Copy link
Contributor

Choose a reason for hiding this comment

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

codeclimate is warning this raise is not covered, can we add some tests in identity_config_spec that exercise this new enum logic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Erm... I don't see identity_config_spec.rb?! 😕

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh dang I thought we had one 😭

solipet and others added 2 commits October 29, 2021 15:16
Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
solipet and others added 2 commits October 29, 2021 16:06
Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
solipet and others added 2 commits October 29, 2021 16:52
Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
@solipet solipet force-pushed the dprice-lg-5184-full-outage-page branch from 4f8891a to d1c1cb6 Compare October 29, 2021 21:28
Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

Looks good 👍

return unless ial2_requested?

if VendorStatus.new.any_ial2_vendor_outage?
session[:vendor_outage_redirect] = CREATE_ACCOUNT
Copy link
Contributor

Choose a reason for hiding this comment

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

For future reference, how should a developer plan to give a name to this constant? Is it the flow that the user was trying to do? I'm wondering if maybe it could be something we either automate (e.g. via referrer), or require less thought on the part of the developer (e.g. using the class name self.class.name), or omit altogether.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, I didn't love this - originally the the messaging was slightly different if the user came from creating an account, but then that messaging became the default. I think I'll merge it as is, but we can revisit as we work through expanded vendor outage handling.

@solipet solipet force-pushed the dprice-lg-5184-full-outage-page branch from d1e63f0 to bb76f75 Compare November 1, 2021 15:50
@solipet solipet merged commit 2cc3d28 into main Nov 1, 2021
@solipet solipet deleted the dprice-lg-5184-full-outage-page branch November 1, 2021 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants