Skip to content

LG-13758: Bypass secondary id check for Enhanced IPP#10934

Merged
KeithNava merged 5 commits intomainfrom
keithw/LG-13758-by-secondary-id-check-when-eipp
Jul 23, 2024
Merged

LG-13758: Bypass secondary id check for Enhanced IPP#10934
KeithNava merged 5 commits intomainfrom
keithw/LG-13758-by-secondary-id-check-when-eipp

Conversation

@KeithNava
Copy link
Contributor

@KeithNava KeithNava commented Jul 15, 2024

🎫 Ticket

Link to the relevant ticket:
LG-13758

🛠 Summary of changes

As we await the new API contract from USPS with the expanded list of secondary ID options, we are temporarily bypassing the secondary check completely. This is to support the friends and family testing for Enhanced IPP

📜 Testing Plan

Please see the scenarios below for

  1. EIPP Enabled with Unsupported Secondary ID
  2. EIPP Enabled with Supported Secondary ID
  3. The current flow in production - EIPP Disabled (ID IPP) with Unsupported Secondary ID
  4. The current flow in production - EIPP Disabled (ID IPP) with Supported Secondary ID

Enhanced In Person Proofing Enabled

Note

Pre requisite for all of this is to spin up the OIDC-Sinatra app locally. You can find the setup steps here: https://docs.google.com/document/d/1FG9s8Cih9NGbrz7ag4WPLK_W10di-1xfDKy0Fjrg0fM/edit#heading=h.hepkwzlgvxf3. Once you have it running locally, you'll need to make sure you select the Enhanced IPP option from the dropdown. This is key since it sets the application up to enable Enhanced IPP.

eipp-sinatra2

Should PASS enrollment with UNSUPPORTED secondary ID #

Running the Job Manually 🧰

e = InPersonEnrollment.last
  • Next, in the same console terminal setup the mocked response
mock = UspsInPersonProofing::Mock::Fixtures.request_passed_proofing_secondary_id_type_results_response
res = JSON.parse(mock)
  • Create an instance of the job
job = GetUspsProofingResultsJob.new
  • Set the enrollments outcome instance variable
job.instance_variable_set('@enrollment_outcomes', { enrollments_passed: 0 })
  • Start the job
job.send(:process_enrollment_response, e, res)
  • Confirm the enrollment succeeded
e.status == 'passed'

Should PASS enrollment with SUPPORTED secondary ID #

Running the Job Manually 🧰

e = InPersonEnrollment.last
  • Next, in the same console terminal setup the mocked response
mock = UspsInPersonProofing::Mock::Fixtures.request_passed_proofing_supported_secondary_id_type_results_response
res = JSON.parse(mock)
  • Create an instance of the job
job = GetUspsProofingResultsJob.new
  • Set the enrollments outcome instance variable
job.instance_variable_set('@enrollment_outcomes', { enrollments_passed: 0 })
  • Start the job
job.send(:process_enrollment_response, e, res)
  • Confirm the enrollment succeeded
e.status == 'passed'

ID IPP (EIPP NOT Enabled)

Should FAIL enrollment with UNSUPPORTED secondary ID #

Running the Job Manually 🧰

e = InPersonEnrollment.last
  • Next, in the same console terminal setup the mocked response
mock = UspsInPersonProofing::Mock::Fixtures.request_passed_proofing_secondary_id_type_results_response
res = JSON.parse(mock)
  • Create an instance of the job
job = GetUspsProofingResultsJob.new
  • Set the enrollments outcome instance variable
job.instance_variable_set('@enrollment_outcomes', { enrollments_failed: 0 })
  • Start the job
job.send(:process_enrollment_response, e, res)
  • Confirm the enrollment failed
e.status == 'failed'

Should PASS enrollment with SUPPORTED secondary ID #

Running the Job Manually 🧰

e = InPersonEnrollment.last
  • Next, in the same console terminal setup the mocked response
mock = UspsInPersonProofing::Mock::Fixtures.request_passed_proofing_supported_secondary_id_type_results_response
res = JSON.parse(mock)
  • Create an instance of the job
job = GetUspsProofingResultsJob.new
  • Set the enrollments outcome instance variable
job.instance_variable_set('@enrollment_outcomes', { enrollments_passed: 0 })
  • Start the job
job.send(:process_enrollment_response, e, res)
  • Confirm the enrollment failed
e.status == 'failed'

@KeithNava KeithNava force-pushed the keithw/LG-13758-by-secondary-id-check-when-eipp branch from fd29cf9 to a5dbac0 Compare July 16, 2024 19:51
@KeithNava KeithNava marked this pull request as ready for review July 16, 2024 19:53
@KeithNava KeithNava requested review from a team and shanechesnutt-ft July 17, 2024 10:56
@gina-yamada
Copy link
Contributor

gina-yamada commented Jul 17, 2024

Does this response (UspsInPersonProofing::Mock::Fixtures.request_passed_proofing_secondary_id_type_results_response) usually cause a failure for IPP (not EIPP)?

Copy link
Contributor

@shanechesnutt-ft shanechesnutt-ft Jul 17, 2024

Choose a reason for hiding this comment

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

I am wondering if it makes sense to create a new method for creating separation between EIPP checks and ID IPP checks

def has_unsupported_secondary_id_type?(response)
  response['secondaryIdType'].present? && 
    SUPPORTED_SECONDARY_ID_TYPES.exclude(response['secondaryIdType'])
end

Then the passed_with_unsupported_secondary_id_type? would look like this

def passed_with_unsupported_secondary_id_type?(enrollment, response)
  !enrollment.enhanced_ipp? && has_unsupported_secondary_id_type?(response)
end

Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally it would be nice if we had a wrapper for the proofing response then we could have a method in the response wrapper that can make the check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, that's a good thought, I'm not sure. Since this is a temporary change I would expect the final solution to either not have this check at all or expand the list of supported secondary ids based on being in the enhanced_ipp flow. I'm just not sure where this isolation would be.

@KeithNava
Copy link
Contributor Author

Does this response (UspsInPersonProofing::Mock::Fixtures.request_passed_proofing_secondary_id_type_results_response) usually cause a failure for IPP (not EIPP)?

@gina-yamada Yes - it does for me locally. Sorry, I've been updating the testing plan but it's going slow since I'm having some trouble on my local machine.

@jennyverdeyen jennyverdeyen self-requested a review July 17, 2024 21:18
Copy link
Contributor

@jennyverdeyen jennyverdeyen left a comment

Choose a reason for hiding this comment

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

Test plan looks good after working out some hiccups with @KeithNava! I ran through it and both test cases behave as expected. LGTM 👍

Copy link
Contributor

@shanechesnutt-ft shanechesnutt-ft left a comment

Choose a reason for hiding this comment

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

I ran through the tests cases yesterday. Everything looked good.

@KeithNava KeithNava requested review from a team and eileen-nava July 18, 2024 13:52
Copy link
Contributor

Choose a reason for hiding this comment

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

(I'm commenting on a file so that we can thread responses to this comment and resolve the conversation when we're done chatting.)

I noticed that the testing plan recommends using UspsInPersonProofing::Mock::Fixtures.request_passed_proofing_secondary_id_type_results_response. When I look at this mock file, it has "ippAssuranceLevel": "1.5". Since this is EIPP, I think we'd want to use a file with "ippAssuranceLevel": "2.0".

I don't know that this would affect behavior for the testing plan, but I wanted to flag it because that line jumped out at me.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would recommend using fixtures that have the correct assurance level in the specs. Thoughts?

@KeithNava KeithNava force-pushed the keithw/LG-13758-by-secondary-id-check-when-eipp branch 2 times, most recently from cd1ea4c to 99e178a Compare July 23, 2024 14:42
Copy link
Contributor

Choose a reason for hiding this comment

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

👏🏻 for the updated ippAssuranceLevel

Copy link
Contributor

Choose a reason for hiding this comment

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

Non-blocking nit: I think Bypasses should be one word here

Copy link
Contributor

Choose a reason for hiding this comment

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

👏🏻

Copy link
Contributor

@eileen-nava eileen-nava left a comment

Choose a reason for hiding this comment

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

Thanks for refactoring! Looks good. Approved.

@KeithNava KeithNava force-pushed the keithw/LG-13758-by-secondary-id-check-when-eipp branch from 99e178a to 88a0258 Compare July 23, 2024 17:49
@KeithNava
Copy link
Contributor Author

Thanks everyone for your reviews and feedback!

@KeithNava KeithNava merged commit 93e1194 into main Jul 23, 2024
@KeithNava KeithNava deleted the keithw/LG-13758-by-secondary-id-check-when-eipp branch July 23, 2024 18:15
mitchellhenke pushed a commit that referenced this pull request Jul 31, 2024
* feat: bypass secondary id check for enhanced ipp

* feat: update tests for reusability

* changelog: Upcoming Features, Enhanced In-person proofing, Bypass secondary id check for EIPP

* feat: add new IAL2 fixtures

* feat: add guard clause for eipp under unsupported secondary id check
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.

5 participants