Conversation
jmhooper
force-pushed
the
jmhooper-add-authn-context-comparison-recognition-to-saml-protocol
branch
from
ff5416f to
c2e78d8
Compare
SAML SPs can request IALMax by using an IAL authn context that does not require proofing and including a Authn Context Comparison value of "minimum". This essentially says that the "IAL1" Authn context is the minimum acceptable context so "IAL2" is also acceptable. This is problematic for the new `AuthnContextResolver` since it does not have visibility into these attributes on the SAML request when resolving the authn context. This commit addresses the issue by returning the IALMax Authentication Context Reference from the `Saml#ial` in this case. This way it is picked up downstream by the `AuthnContextResolver`. [skip changelog]
jmhooper
force-pushed
the
jmhooper-add-authn-context-comparison-recognition-to-saml-protocol
branch
from
27cf0cc to
1b4dea1
Compare
| @ial_context ||= IalContext.new( | ||
| ial: requested_ial_authn_context, | ||
| ial: resolved_authn_context_int_ial, |
There was a problem hiding this comment.
WDYT of renaming the attribute in the IalContext constructor to be int_ial for clarity?
There was a problem hiding this comment.
The IalContext actually takes an integer or a string.
There was a problem hiding this comment.
I am making sure that we pass the integer value here because that will properly compute IALMax. In some IALMax cases the value is the string value for IAL1 with the minimum context comparison.
Sgtpluck
left a comment
Sgtpluck
left a comment
There was a problem hiding this comment.
Just a small, non-blocking suggestion. Thanks for plumbing this through.
|
|
||
| def pii_requested_but_locked? | ||
| if (sp_session && sp_session_ial > 1) || ial_context.ialmax_requested? | ||
| if resolved_authn_context_result.identity_proofing? || resolved_authn_context_result.ialmax? |
There was a problem hiding this comment.
[question] since both parts of this conditional are now on the resolved_authn_context_result object, does it make sense to have a method in that object that's like resolved_authn_context_result.pii_requested?
There was a problem hiding this comment.
I hesitate a little because pii_requested sounds like we are doing proofing if you don't have the context about ialmax (which is not hidden pretty deep in the Vot::Parser. We could always add something like identity_proofing_or_ialmax??
There was a problem hiding this comment.
yeah i think that sounds good!
jmhooper
deleted the
jmhooper-add-authn-context-comparison-recognition-to-saml-protocol
branch
4 participants
SAML SPs can request IALMax by using an IAL authn context that does not require proofing and including a Authn Context Comparison value of "minimum". This essentially says that the "IAL1" Authn context is the minimum acceptable context so "IAL2" is also acceptable.
This is problematic for the new
AuthnContextResolversince it does not have visibility into these attributes on the SAML request when resolving the authn context.This commit addresses the issue by returning the IALMax Authentication Context Reference from
Saml#ialin this case. This way the IALMax value is picked up downstream by theAuthnContextResolver.This type of request has a few differences from an IALMax request with the IALMax authn context reference: