Merged
Conversation
* Update rexml gem changelog: Internal, Maintenance, Update rexml gem * fix spec
…1124) This commit makes changes to the way to `VerifyInfoController#show` logs events in the main IdV and in-person contexts. This controller action has 2 primary concerns: 1. Rendering the “Verify your information” page 2. Managing the results of the `ResolutionProofingJob` that runs when the “Verify your information” page is submitted Which concern is active for these actions is controlled by the `async_state` which is a `ProofingSessionAsyncResult`. This is a struct that includes the status of the `ResolutionProofingJob`. This status can have 4 values: - `NONE`: This means that no job has been submitted. In this state the controller should render the “Verify your information” page. - `IN_PROGRESS`: This means that the job is running. In this state the controller should render a loading state page that is detected by the `form-steps-wait` tooling on the frontend causing it to render a loading state. - `DONE`: The job has been completed. In this state the controller should process the results and redirect appropriately. - `MISSING`: The result is expected but not present. This indicates that the job has timed out. In this state the controller should render the “Verify your information” page with an error. In addition to these states the controller is aware of rate limits for the purpose of redirecting to a rate limit action when the user has been rate limited. Prior to this commit, these states had the following associated analytics actions: - `IdV: doc auth verify visited`: This event was previously logged in all cases. This event is the focus of this change. - `IdV: doc auth verify proofing results`: This event is logged when `async_state.status` is `DONE`. This event includes a large hash containing the results of running `ResolutionProofingJob`. - `IdV: proofing resolution result missing`: This event is logged when `async_state.status` is `MISSING`. - `Rate Limit Reached`: This event is logged by `RateLimitConcern` when a rate-limit is exceeded and the user is being redirected as a result. Since the `IdV: doc auth verify visited` was logged in all cases there was a lot of double counting of verify step visits. For example, if a user visited the page, submitted it, has 2 polling attempts, and then sees the job complete on the 3rd attempt then they will have 4 `IdV: doc auth verify visited` events in the log. This pattern has led to some confusion since this user has only visited this step once. This commit makes a change so that the `IdV: doc auth verify visited` is only logged when `async_state.status` is `NONE`, i.e. when the user is viewing the “Verify your information” screen to submit their info. All of the other values of `async_state.status` are covered by an existing analytics event except for `IN_PROGRESS`. This commit adds a new `idv_doc_auth_verify_polling_wait_visited` to ensure that this state is covered by logging. [skip changelog]
* Remove unnecessary use of DOMContentLoaded changelog: Internal, Performance, Remove unnecessary use of DOMContentLoaded * Revert mock-device-profiling changes
* changelog: internal, accuant capture, refactor variable name * Updated spec to use updated variable name * Updated events with updated clickSource variable * Updated test to follow updated logic detailed in AC * Updated params comments to fix lint * Added constinent variable names * Resolving lint issue
changelog: Internal, Reporting, further stagger the delay of reporting jobs so we don't overwhelm other systems
changelog: Internal, Reporting, Update APG Report with Current Month
* Add in-person proofing successes to the weekly drop-off report changelog: User-Facing Improvements, Reporting, IdV, Add in-person proofing completion count to the weekly report that lists IdV completion statistics * Update DropOffReport IPP queries based on PR feedback changelog: Internal, Reporting, IdV, Consolidate query for in-person proofing statistics
changelog: Internal, Source code, Update lint rules
* add token secret validation to socure webhook * add socure_webhook_secret_key config variable * fixup * use secure_compare for secret key Co-authored-by: Mitchell Henke <mitchell.henke@gsa.gov> * check for nil authorization header * add changelog changelog: Upcoming Features, Doc Auth, add secret validation for socure webhook * testing config variables in review app * formatting * remove gitlab-ci.yml overwrite * try out review app config * add a socure webhook key queue for older keys * fix identity_config spec * remove socure secret keys from docker default --------- Co-authored-by: Mitchell Henke <mitchell.henke@gsa.gov>
…11065) changelog: Bug Fixes, In-person proofing, addresses error that occurs when pii is nil in verify info controller
…doc auth vendor (#11150) * only provide mock_client when using mock doc auth vendor changelog: Bug Fixes, Document Authentication, Fix mock client metadata * add test en ensure mock-client-data is only sent when using mock client for doc auth * check only if mock-client metadata exists * happy linting
#11140) * changelog: Internal, Fraud prevention, include associated user_id in event disavowal * change user.id to use associated user_id * move user_id catching to the build_disavowal class * add user_id documentation to event * fix lint * pass user id as uuid * update specs to use uuid, add nil safe for disavowal error events * remove nil default * move not-optional keywords
…ions (#11145) We use ThreatMetrix as part of our proofing process. When we get a response from ThreatMetrix we log the response body along with some other metadata about the response. The response body contains sensitive PII that cannot be logged so before logging the response body is passed through the `ResponseRedacter` which redacts sensitive info. The `ResponseRedacter` gaurds against nil response bodies by returning returning a response with an error with the description `TMx response body was empty'. As a result, any time there is no response body to be logged we see that error in our logs. This can be confusing when other errors occur, for example network timeouts or connection failures. It makes it appear as though the issue is a missing response body. This commit modifies the `DdpResult` to only redact the response body when it is present. When it is absent a nil value is returned. This will cause the empty response body to quit being logged and appearing in cloudwatch when exceptions occur. [skip changelog]
mitchellhenke
approved these changes
Aug 27, 2024
Comment on lines
+101
to
+103
| unless user_session.dig('idv/in_person').present? | ||
| redirect_to idv_path | ||
| end |
Contributor
There was a problem hiding this comment.
oh I missed this in the original PR but I think it would better with a positive check, and maybe even fit on one line if we want
Suggested change
| unless user_session.dig('idv/in_person').present? | |
| redirect_to idv_path | |
| end | |
| redirect_to idv_path if user_session.dig('idv/in_person').blank? |
zachmargolis
approved these changes
Aug 27, 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
11 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
Upcoming Features