Merged
Conversation
- This path is more in-line with other post-2fa steps for sign in - Bring back before_filter in accounts controller changelog: Internal, User suspension, Update suspended user check
- Expanding use so we can support a specific one-off request, but seemed easier to expand all reports at once - Update YARD params docs changelog: Internal, Reporting, Update funnel reports to accept multiple issuers
changelog: Internal, Analytics, Avoid duplicate analytics event for PIV/CAC login
* Update mobile docs * Make grammar consistent Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> * changelog: Internal, In-Person Proofing, add a small section to the mobile debugging docs that the IPP/doc auth teams use --------- Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
…cation OIDC request (LG-10936) (#9157) * Do not end session when switching languages during forced re-authentication OIDC request changelog: Bug Fixes, OpenID Connect, Do not end session when switching languages during forced re-authentication request * Update spec/controllers/openid_connect/authorization_controller_spec.rb Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com> --------- Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
* Fix error handler naming * Rerun CI jobs * Correct error handler name in tests * changelog: User-Facing Improvements, In-person proofing, Fix bug when camera permissions denied during document upload * Allow either onError or onFailure callback * Revert "Allow either onError or onFailure callback" This reverts commit d9000bf.
* LG-10681: Remove ArcGIS token job * LG-10681: Remove location search endpoint and ArcGIS geocoder service * LG-10681: Update location search tests and default full address entry to enabled * LG-10681: Remove text references to ArcGIS * changelog: Internal, In-Person Proofing, Remove ArcGIS API usage * LG-10681: Simplify test helper function names
* LG-10871: Direct "Learn more" Face/Touch link to help article changelog: Upcoming Features, Face or Touch Unlock, Update "Learn More" link to direct user to specific help page article * Update new.html.erb_spec.rb
* PostOfficeNoResult component and tests * Add new svg * changelog: Upcoming Features, In-person proofing, created a new component to display when no po search results are found * Added extra line at end of file * lint fix * Export new component and version change * optimize assest * fix spelling * Modify style * rename component * fix linter errors * more style changes * Add alt text translations for info-pin-map * updated test to be dynamic for translations * Use NoInPersonLocationsDisplay component in in-person-locations * modify style of no in person location display * modified grip gap
We have added code to start writing to profiles with a multi-region KMS key. This is in addition to the single-region key was were using previously. Records that were created before we introduced the multi-region key do not have a value encrypted with the multi-region KMS key. 5506d9e added a background job for backfilling the empty multi-region ciphertexts. This commit enables it after testing and timing it in lower environments and in production. changelog: Internal, Multi-region KMS migration, The MultiRegionKmsMigration::ProfileMigrationJob job which finds profiles with PII encrypted with the single-region KMS key and not the multi-region KMS key and decrypts the single-region KMS layer using the single region key and encrypts the result with the multi-region KMS key and saves the resulting ciphertext as the multi-region ciphertext was enabled.
…n KMS (#9164) \We are in the midst of a migration from a single-region KMS key to a multi-region capable KMS key. That migration involves the following steps: 1. Add new columns for multi-region ciphertexts 2. Start writing to the new columns with multi-region ciphertexts 3. Start opportunistically reading from the multi-region columns 4. Backfill the multi-region columns for old users 5. Stop using the old single-region columns This commit is part of step number 4. This commit adds a `UserMigrator` class which takes a user and migrates the single-region encrypted `encrypted_password_digest` and `encrypted_recovery_code_digest` to `encrypted_password_digest_multi_region` and `encrypted_recovery_code_digest_multi_region` respectively. This commit also includes a job to find users that require migration and invoke the `UserMigrator` against them. The background job is not currently on the background job schedule. This will allow us to do some testing with it in lower environments before we start using it in production. changelog: Internal, Multi-region KMS migration, A UserMigrator and UserMigration job was added for finding users that have an encrypted password digest or encrypted recovery code digest where the outer layer is encrypted with a single region KMS key and decrypting the outer layer using the single-region KMS key and then encrypting the results with a multi-region capable KMS key before saving the result of the multi-region capable KMS encryption in the columns that were added for multi-region password and recovery code digests in a previous commit.
…prove test speed (#9158) * Convert some simpler feature tests to controller and view tests changelog: Internal, Test Performance, Convert some simpler feature tests to controller and view tests * remove duplicative tests * Update spec/controllers/users/two_factor_authentication_controller_spec.rb Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com> * remove stub * Update spec/controllers/users/two_factor_authentication_controller_spec.rb Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com> * move unauthenticated account spec * move config tests to helper spec * update knapsack --------- Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
* add strings and contact number * update spec * fix line length * changelog: User-Facing Improvements, Send Proofing notification job, update sms text with contact number * update specs * update en translation
* standardize on :ssn in session_errors_controller * Also read idv_session.ssn in VerifyInfoConcern * Also read/write idv_session.ssn in SsnController * Also check rate limit for idv_session.ssn (and add specs for proof_ssn rate_limiter) * Also read idv_session.ssn in SessionErrorsController * Add ssn to idv_session * Also read from idv_session.ssn in #confirm_ssn_step_complete * Add incoming_ssn to SsnFormatForm#initialize, set to idv_session.ssn in new * changelog changelog: Internal, Identity Verification, move ssn to idv_session, part 1 of 2 * Change SsnFormatForm#initialize to take just the incoming ssn, no flow_session * Move ssn into idv_session for in_person flow And fix the initial conditions for a test on clearing idv_session.applicant
) * Consolidate signed_in_url with after_sign_in_path changelog: Internal, Code Quality, Consolidate route handling for after-sign-in behavior * Move confirm_user_is_not_suspended to AccountsController Not common logic * TEMPORARY: Test coverage for reactivate account * Revert "TEMPORARY: Test coverage for reactivate account" This reverts commit c2d627f. * Enhance feature spec coverage for expected profile reactivation * Restore backup code reminder logic to signed_in_url See: https://github.com/18F/identity-idp/pull/9159/files#r1319872936
changelog: Internal, Performance, Memoize options method on TwoFactorLoginOptionsPresenter
* changelog: Internal, In-Person Proofing, Extract FAS hook * Add simple test for hook
changelog: Internal, Performance, Do not query for PivCacConfiguration when x509_dn_uuid is blank
…ep is out of the FSM (#9176) * Don't need threatmetrix before action in in_person_controller anymore because the ssn step is outside the FSM in person flow now. [skip changelog] * Remove FSM version of threat_metrix csp before action and specs (no longer used) * rename override_csp_for_threat_metrix_no_fsm to override_csp_for_threat_metrix * Inline spec methods --------- Co-authored-by: Gina Yamada <gina.yamada@gsa.gov> Co-authored-by: Jessica Dembe <jessica.dembe@gsa.gov>
changelog: Upcoming Features, Face or Touch Unlock, Fix grammar for translated paragraph sentences
…are disabled (#9175) * when s3 reports disabled, Identity verification report to return immediately and not create a report to be uploaded to S3 * [skip changelog]
* Remove unused profile_step_params from idv_session [skip changelog] * Remove unused profile_confirmation from idv_session
* Move GPO controllers into by_mail and rename [skip changelog] * Move GPO controller specs into by_mail and rename * Get RequestLetterController spec passing * Get LetterEnqueuedController spec passing * Get EnterCodeController spec passing * come_back_later -> letter_enqueued * Update view specs * Analytics event naming * Rename some routes and add temporary redirects * 'come back later visited' -> 'letter enqueued visited' * idv_letter_enqueued -> idv_gpo_letter_enqueued * /by_mail -> /by_mail/enter_code * Fix GpoPresenter spec * 'USPS address visited' -> 'gpo request letter visited' * letter enqueued visited -> gpo letter enqueued visited' * `IdV: GPO verification visited` -> `IdV: gpo enter code visited` * enter code -> enter verify by mail code * 'GPO verification submitted' -> 'enter verify by mail code submitted' * Remove GPO from a couple of analytics event names * Clarify temporary redirects Match structure in the gpo_verification_enabled? check * Update app/services/analytics_events.rb Co-authored-by: Sonia Connolly <sonia.connolly@gsa.gov> * idv_gpo_url -> idv_request_letter_url * idv_gpo_letter_enqueued_url -> idv_letter_enqueued_url * idv_gpo_verify_url -> idv_enter_verify_by_mail_code_path * idv_gpo_url fixup * fixup idv_gpo_verify_url * Idv::GpoPresenter -> Idv::ByMail::RequestLetterPresenter * A couple of analytics methods * Fix test * idv_enter_verify_by_mail_code_url -> idv_verify_by_mail_enter_code_url * Don't change routes yet - Add handlers at the new route locations (for the GET routes--the others will PUT/POST to the old paths still) - Don't do any 302 redirecting yet. Actual route changes will come in a future deploy * Fix presenter spec * Update analytics method name * Add PUT/POST routes for new GPO locations --------- Co-authored-by: Sonia Connolly <sonia.connolly@gsa.gov>
* changelog: Big Fixes, Accessibility, Improve tappable area of footer links * slight adjustment to footer_links styles * refigure cascading order to prefer mobile * remove vertical padding on tablet * reset footer font size to original * reset footer icon size * remove unneeded size override
changelog: Bug Fixes, PIV CAC Sign-In, Fix issue preventing user from being redirected to partner after declined PIV/CAC setup from sign-in
* LG-10347 Make the key ID the session encryptor uses configurable We are currently migrating from a single-region KMS key to a multi-region capable KMS key. This commit modifies the SessionEncryptor and BackgroundArgsEncryptor to have to use a configurable key for their KMS client. These encryptors are used in 3 contexts: 1. Encryption of sessions (SessionEncryptor) 2. Encryption of GPO confirmation entries (SessionEncryptor) 3. Encryption of arguments to background jobs (BackgroundArgsEncryptor) The KMS client's for these encryptors will now use the configured key ID for encryptions. For decryption the client allows KMS to select the key. This means that decryption will not be affected by this change as long as KMS still has access to the keys referenced by the KeyID used for encryption. Since all of the encryption operations done with these encryptors produce ephemeral ciphertexts there is not need to worry about holding onto old keys after this has been deployed with the multi-region key configured for a while. changelog: Internal, Multi-region KMS migration, The SessionEncryptor and BackgroundArgsEncryptor were change to have a configurable KMS key ID that is used for encryption in order to facilitate a migration to a multi-region key ID.
…ch (#9172) Make it possible to display an info alert above the in person proofing location search results telling the user that they must enroll before visiting the Post Office. The alert will be shown when the user is searching for locations in the help center. changelog: User-Facing Improvements, In-person Proofing, Add info alert for users searching locations in help center
aduth
requested review from
NavaTim,
allthesignals,
amirbey,
charleyf,
gina-yamada,
jmdembe,
jmhooper,
kevinsmaster5,
mitchellhenke,
soniaconnolly,
svalexander,
tomas-nava and
zachmargolis
| @@ -1,7 +1,7 @@ | |||
| module Idv | |||
| class GpoController < ApplicationController | |||
| module Idv::ByMail | |||
Contributor
There was a problem hiding this comment.
I missed this in the original PR but I think we should use the nested namespace style, not the abbreviated style, ex:
module Idv
module ByMail
class RequestLetterController < ApplicationControllerIt's more clear which of the intermediate namespaces is a class vs a module and means the Rails autoloader has to do less guessing (because you can't redefine a class as a module and vice versa)
Contributor
Author
| }: InPersonLocationsProps) { | ||
| const isPilot = locations?.some((l) => l.isPilot); | ||
|
|
||
| if (locations?.length === 0) { |
Contributor
There was a problem hiding this comment.
also late to this, but I think this will be wrong if locations is ever null/undefined? because undefined === 0 is false
Contributor
Author
There was a problem hiding this comment.
Similarly expecting to be non-blocking. Also, not technically changing in this release.
While your reasoning is correct, I think it might be intentional? i.e. if locations is undefined, it might mean that they're being loaded, and we wouldn't want to show a "no results found" message?
cc @allthesignals re: #7468
Contributor
There was a problem hiding this comment.
The current usages of the InPersonLocations React component (in this release) check whether locations is truthy before attempting to render it:
Therefore this issue doesn't have practical implications for this release.
zachmargolis
approved these changes
Sep 11, 2023
mdiarra3
approved these changes
Sep 11, 2023
NavaTim
approved these changes
Sep 11, 2023
| }: InPersonLocationsProps) { | ||
| const isPilot = locations?.some((l) => l.isPilot); | ||
|
|
||
| if (locations?.length === 0) { |
Contributor
There was a problem hiding this comment.
The current usages of the InPersonLocations React component (in this release) check whether locations is truthy before attempting to render it:
Therefore this issue doesn't have practical implications for this release.
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
13 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