Merged
Conversation
changelog: Internal, Reporting, fix the date range nil check for both billing reports
* Refactor `OpenidConnectAuthorizeForm` to remove `IalContext` The `OpenidConnectAuthorizeForm` class is a form object that validates an OpenID Connect authorization request. This includes validating things like whether the scopes and ACR values requested by the service provider are allowed. For example, if an SP that is not allowed to request identity proofing requests scopes that required identity proofing this form responds with an error. The `IalContext` class provides a number of helper methods helper methods for determining the IAL of a request by consuming either the integer IAL or the ACR values from the request and considering that along with SP defaults to determine a requests IAL. The `IALContext` can optionally consider the user context to make determinations about IAL2 requests. Since this is optional it leads to inconsistent results. For example, in the `OpenidConnectAuthorizeForm` the user context is not provided so the `IALContext` will never return true from `ial2_or_greater?` for an IALMax request even if the user has identity proofed. The `IALContext` is being replaced with the `AuthnContextResolver` to address this. The `OpenidConnectAuthorizeForm` does not necessarily need to be concerned with the state of the user and rather needs to be concerned with what was requested in totality and whether it is allowed. This will be more important when we begin accepting multiple vectors of trust to support an IALMax-like feature for service providers using that feature. This commit changes the `OpenidConnectAuthorizeForm` to remove the `IALContext`. [skip changelog] * remove the user ial context
changelog: Internal, Maintenance, Update rexml gem
* Use configured locales when building path-based rate limits changelog: Internal, Rate Limiting, Use configured locales when building path-based rate limits * add spec
* Update translations for Chinese (Simplified) in language pickers changelog: Internal, Translations, Update translations for language pickers * update allowed untranslated keys
…0626) Previous commits retired the following fields on the `ServiceProviderRequest` model: - ial - aal - biometric_comparison_required These fields were left behind because removing them leads to issues during deploys when old and new versions of the code are running. The `ServiceProviderRequest` model has default values for these fields. This means that we can stop creating records with these fields. However, we use the hash that is written to redis to create `ServiceProviderRequest`s. This means that if we remove the keyword arguments for these fields we will encounter a `ArgumentError` when we try to create a record on a new host with a hash written by an old host. [skip changelog]
#10611) * Changelog: User facing improvements,GPO verify,Improved letter enqueued language Overhauled the letter enqueued page language per UX recommendations and added simplified Chinese translations. Co-authored by: Andrew Duthie <andrew.duthie@gsa.gov> Co-authored by: Jonathon Hooper <jonathon.hooper@gsa.gov>
changelog: Internal, Automated Testing, Remove unused spec user factory traits
…10643) Input field error message should be the width of the app, not the width of the input field. changelog: User-Facing Improvements, Layout, Error message on IdV code entry now is full width. Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
The `IdentityLinker` seems to be setting the following attributes on `ServiceProviderIdentities` for reporting purposes: - `last_ial2_authenticated_at` - `last_ial1_authenticated_at` - `verified_at` These fields seems to be queried for a number of reports and aren't used by any business logic in the IdP. This behavior is currently untested. You could remove this entire method and the test suite would still be green. This commit tests the current behavior. The current behavior does not appear to account for a number of edge cases e.g. users who sign in with IALMax or users who are signing into multiple service providers. This commit addresses none of those edge cases because I do not understand the ramifications of addressing those edge cases for our reporting. [skip changelog]
…r` (#10649) The `SamlRequestPresenter` was initialized with the service provider and with the SAML request. It implemented a single public method: `requested_attributes`. This method considered the attributes requested in the SAML request, the service provider defaults, and the IAL context of the request. This was used by the `FederatedProtocols::Saml` class to determine what attributes were requested which ultimately found their way into a column the `identities` table. Much of the work to consider the IAL context, namely what the service provider requested and service provider defaults, is already handled by the caller. I refactored this class to take the `ial` as an argument instead of determining it from the request object and service provider. I also moved the `SamlRequestParser` call that found the requested attributes up into the caller and passed that as an argument. That makes the testing approach for this much easier. You no longer need to pass a parsable request object or mock the request parser to test the behavior here. [skip changelog]
* remove Get help at CTA from doc capture components * remove Get Help at SP CTA from phone_errors screens * remove Get Help from SP CTA for session_errors screens * remove get_help_at_sp translation key * add changelog changelog: User-Facing Improvements, IdV, Remove Get Help CTA from non fraud screens * remove compact from session_errors/exception * remove getFailureToProofUrl * remove unused prop
changelog: Upcoming Features, Chinese Language, Fix content for Chinese verified text
changelog: Internal, Security, Add subresource integrity for design system initializer script
changelog: Internal, Build Tooling, Use SHA256 for JavaScript subresource integrity
changelog: Internal, Internationalization, Fix language picker when displaying in Chinese (Simplified)
* Add and update specs to reflect doc + add ymls This adds specs for 1. when there are multiple doc auth errors on front and back 2. when there are multiple front doc auth errors 3. when there are multiple back doc auth errors It also updates the previously existing tests to check for the rate limit message, as when I was working on the rate limit message change I found sometimes I had the message showing twice or the wrong message. So I wanted to make sure that the correct message was showing in those tests as well. * Restore previous default error message This reverts a change I made in #10450 that turned out to be the wrong change. * Show subheading whenever IPP and !failedResult Our previous logic for the subheading was too strict and didn't show when selfie had errors. But really we want this h2 to show whever there is a corresponding h2 in the ipp section to balance out the html structure and make it more accessible. * Simplify rate limit message logic After discussing this in the error doc, we found that we: - always want the rate limit message to show - do not know the logic behind showing one message versus the other, and would like to opt for the one that is more clear To that end, I moved the logic for the rate limit message up a level and out of so many conditionals. This ended up being a bigger change than I expected as so many tests relied on the place where it was previously being called, but I think it simplifies the code. * Update test helper for mobile ipp selfie flow Recently this text was changed, and when trying to use the same test helper as in previous specs, it wouldn't work. This change updates the helper to be able to navigate the features enabled. * Rm unusued string from translations Because we decided to consolidate and only use one of the rate limit messages, we no longer use this string. * changelog: User-Facing Improvements, Doc Auth, Fix error messaging for case of multiple errors --------- Co-authored-by: Amir Reavis-Bey <amir.reavis-bey@gsa.gov>
These arguments are confusing for the following reasons: 1. We never actually pass in a user. We use the user to initialize an `IalContext` (see below) 2. We use the IAL to initialize an `IalContext`. We use this to call `bill_for_ial_1_or_2` which is intended to consider billing impacts of IALMax if the user is proofed. We always pass a nil user and we always pass an `ial` argument of 2 so this code always returns `2`. We can shortcut all of this by hardcoding `2` for the `ial` attribute. [skip changelog]
matthinz
requested review from
a team,
jmax-gsa,
samathad2023,
solipet,
stephencshelton and
theabrad
mitchellhenke
approved these changes
May 21, 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
9 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
Internal
Upcoming Features