Merged
Conversation
…0481) * LG-13139: Fix race condition with verifier expired validity changelog: Bug Fixes, OpenID Connect, Fix occasional errors resulting from race condition on expiring token * Return identity as tuple pair with submission response See: #10481 (comment) * Add return type documentation for submit method Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com> * Ensure validation performed independent of identity references --------- Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
[skip changelog]
…sponse#pii_from_doc` (#10478) The `pii_from_doc` method in `DocAuth::Mock::ResultsResponse` is intended to mock the behavior of the `pii_from_doc` method on the LexisNexis TrueID client. That method uses [`DocAuth::LexisNexis::DocPiiReader`](https://github.com/18F/identity-idp/blob/a2b3cfcf78b127f6d8a7d8b5fbd27141add52159/app/services/doc_auth/lexis_nexis/doc_pii_reader.rb) to construct its response. As a result the response will always contain the same keys. The `pii_from_doc` method in the mock client returns a set of default PII described by [`Idp::Constants::MOCK_IDV_APPLICANT`](https://github.com/18F/identity-idp/blob/a2b3cfcf78b127f6d8a7d8b5fbd27141add52159/lib/idp/constants.rb#L94) if an image is uploaded. If a user uploads a YAML file, however, the YAML can dictate what PII is returned. This commit makes changes to the behavior when a YAML file is uploaded. Previously when a YAML file had a `document` key the hash under that `document` key would be returned from `pii_from_doc`. This leads to issues with missing or extra values being present in this hash. This is especially problematic when the YAML file is used for testing since it does not properly simulate the behavior of a deployed environment. Now, when a `document` key is provided the values from the hash under that `document` key are merged into the default values. This addresses the issue of missing keys. Extraneous keys will be addressed in a follow-up. ### Returning a nil value from `pii_from_doc` It should be noted that prior to this commit when no `document` key was present the `pii_from_doc` method returned `nil`. This allowed testing error conditions where there would be no PII. For example, if a document cannot be read then `pii_from_doc` is expected to return `nil` since the PII from the document could not be read. This commit preservers that behavior. ### Returning a nil value for specific attributes There are testing scenarios where it is desirable for a specific PII field to be nil. For example, if we are testing the validation that a zipcode is included we need to simulate a scenario where `pii_from_doc` returns a nil zipcode. Previously those scenarios could be tested by simply not providing a zipcode. Now a default value will be in the place of the absent zipcode. This testing scenario can be covered by explicitly providing a nil value in the YAML file e.g. ```yaml document: zipcode: null ``` [skip changelog]
- Only used in one place, easy to check response status changelog: Internal, Source code, Remove dependency on subprocess gem
changelog: Internal, Code Quality, Remove unused code related to disabled survey feature
changelog: Internal, Automated Testing, Add test to enforce TypeScript conventions
changelog: Internal, Data Reporting, Fix Drop Off Report
* LG-11981: add event idv_sdk_selfie_image_capture_initialized. changelog: Internal, Doc Auth, Analytics event for selfie capture readiness. * LG-11981: fix prop name. * LG-11981: add test. * LG-11981: linter fix. * LG-11981: linter fix.
This method is extensively tested, but I can’t find anywhere it is used. [This part](https://github.com/18F/identity-idp/blob/5b1b15e8ff5b3a8f406557eacb8a501684e2d9e0/app/services/flow/base_step.rb#L52) of the tests for the old flow state machine makes me suspect it was used to build a response for the `#call` method on FSM steps. [skip changelog]
changelog: Internal, Code Quality, Remove deprecated routes
* Disable retries gem sleep in all specs - Why: so that the test suite runs faster changelog: Internal, Source code, Speed up test runs by disabling retry sleeps
- Switch Proofing::Aamva::Applicant to be a Struct * Switch to RedactedStruct * Fix specs that relied on Hashie::Mash#merge! changelog: Internal, Source code, Remove dependency on hashie gem
changelog: Internal, CORS, Allow Gitlab pages sites to access via CORS
* Publish @18f/identity-build-sass v3.1.0 NPM package changelog: Internal, JavaScript Packages, Publish @18f/identity-build-sass v3.1.0 NPM package * Apply fixes from npm pkg fix
Ensure the user doesn't have an active profile before we try to send them a reminder email. changelog: Bug Fixes, Identity verification, Don't send GPO reminder emails to users with active profiles
#10450) * Test h1 and body copy logic for doc auth + selfie This change: - moves some test setup to before all / after all - this is so we can reuse the user and other vars we do not change in this file and do not care about - the one drawback is that currently if you ctrl + c twice out of this test, the `after(:all)` block will not run and you will end up with a user hanging out in the db that will not be deleted - we discussed fixing this in this test, but decided it wasn't a good time. If folks agree we can create a ticket to come back to this work - there is no issue if you let the test finish on its own, or if you only ctrl + c once, letting it clean up and run the `after` block - pressing it twice skips the `after` block, which is how it ends up in this broken state - adds a lot of test fixture files - we needed a lot of different setup files in order to test the various combinations Ideally, instead of having each one of these tests be described a comment, it would have its own `it` block. Unfortunately right now the setup would demand running through all the setup steps if we were to do that, making it very slow. Instead it is harder to read, but a nice future refactor would let us pull out the setup in something like a `before(:context)`, allowing us have proper `it` blocks and change what we need without repetition. * Fix h1 error issues on review issues page Previous to this change we did not have this logic and relied on the `else` case to display the heading for doc auth errors. This meant if we had both a doc auth error and a selfie error, it would catch the selfie error first and display the selfie error `h1` instead. This change adds a new `result_code_invalid?` method to the `image_upload_response_presenter`. Passing this through to the front end allows us to use its logic to determine which `h1` to display in the `document-capture-warning` component when we have a doc auth error. * Fix body copy error issues on review issues page Previous to this change we did not have this logic and relied on the `else` case to display the body copy for doc auth errors. This meant if we had both a doc auth error and a selfie error, it would catch the selfie error first and display the selfie error body copy instead. This change: - updates the error_generator logic to take doc auth errors into account - previously we were letting the doc auth errors fall all the way through to the `AlertErrorHandler`, that was then handling doc auth, pii, and unknown errors - we introduced a new `DocAuthErrorHandler` that we could call before selfie errors and took much of the logic out of `AlertErrorHandler` - we renamed `AlertErrorHandler` to `UnknownErrorHandler` to better reflect what it does - I also needed to update the default error messages to display what we expect [as represented in the figma](https://www.figma.com/file/YejFLjUrlzZNjjM8McoudH/MVP%3A-Selfie-UX?type=design&node-id=802-5999&mode=design&t=IgxmyFvrKsLZDqZl-0) - it was tricky getting the attention without barcode case right - this resulted in some of the changes in the next commit, about the misc other changes needed for this work. * Misc changes needed to fix up errors displayed This change: - updates the mock proofer (and spec) to not delete the attention with barcode message - having the mock proofer delete the attention with barcode error message made it impossible to test - update TrueID specs - these began failing with the change to the default liveness general error - fixes an empty space in one of the yml files - this was a strange one! [See this Slack comment for why](https://gsa-tts.slack.com/archives/C05HSH9RQ57/p1713286668833529). * changelog: User-Facing Improvements, Doc Auth, Fix order of errors displayed when there are multiple errors --------- Co-authored-by: Brittany Greaner <brittany.greaner@gsa.gov> Co-authored-by: Eileen McFarland <eileen.mcfarland@gsa.gov> Co-authored-by: Dawei Wang <dawei.wang@gsa.gov> Co-authored-by: Amir Reavis-Bey <amir.reavis-bey@gsa.gov>
aduth
approved these changes
Apr 25, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User-Facing Improvements
Bug Fixes
Internal