Refactor OpenidConnectAuthorizeForm to remove IalContext#10637
Merged
Refactor OpenidConnectAuthorizeForm to remove IalContext#10637
OpenidConnectAuthorizeForm to remove IalContext#10637Conversation
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]
Sgtpluck
approved these changes
May 16, 2024
Contributor
Sgtpluck
left a comment
Sgtpluck
left a comment
There was a problem hiding this comment.
this feels really clear and easy to read to me. what an improvement, thank you 🎊
| end | ||
| end | ||
|
|
||
| def identity_proofing_requested_or_default? |
Contributor
There was a problem hiding this comment.
i LOVE all these little conditional methods, it makes things much clearer to me, thank you.
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
2 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.
The
OpenidConnectAuthorizeFormclass 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
IalContextclass 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. TheIALContextcan optionally consider the user context to make determinations about IAL2 requests. Since this is optional it leads to inconsistent results. For example, in theOpenidConnectAuthorizeFormthe user context is not provided so theIALContextwill never return true fromial2_or_greater?for an IALMax request even if the user has identity proofed. TheIALContextis being replaced with theAuthnContextResolverto address this.The
OpenidConnectAuthorizeFormdoes 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
OpenidConnectAuthorizeFormto remove theIALContext.