Skip to content

LG-13313 Add multiple vector of trust support for SAML#10680

Merged
jmhooper merged 2 commits intomainfrom
jmhooper-saml-user-aware-authn-context-resolver
May 22, 2024
Merged

LG-13313 Add multiple vector of trust support for SAML#10680
jmhooper merged 2 commits intomainfrom
jmhooper-saml-user-aware-authn-context-resolver

Conversation

@jmhooper
Copy link
Contributor

We added support for sending multiple vectors of trust using the OIDC interface in #10517. This commit adds the same feature to the SAML interace.

To send multiple vectors using SAML partners can include multiple AuthnContextNodes with a vector in each. This was enabled in 18F/saml_idp#100.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend hiding whitespace to review this file:

image

@jmhooper jmhooper force-pushed the jmhooper-saml-user-aware-authn-context-resolver branch from a5e36a7 to bb46e1f Compare May 22, 2024 15:10
@jmhooper jmhooper force-pushed the jmhooper-user-aware-authn-context-resolver branch from a1f4cd5 to 3f7492e Compare May 22, 2024 15:12
@jmhooper jmhooper force-pushed the jmhooper-saml-user-aware-authn-context-resolver branch from bb46e1f to 088eff9 Compare May 22, 2024 15:12
Base automatically changed from jmhooper-user-aware-authn-context-resolver to main May 22, 2024 17:45
We added support for sending multiple vectors of trust using the OIDC interface in #10517. This commit adds the same feature to the SAML interace.

To send multiple vectors using SAML partners can include multiple AuthnContextNodes with a vector in each. This was enabled in 18F/saml_idp#100.

[skip changelog]
@jmhooper jmhooper force-pushed the jmhooper-saml-user-aware-authn-context-resolver branch from 607d1c1 to 5291f41 Compare May 22, 2024 17:48
@jmhooper jmhooper marked this pull request as ready for review May 22, 2024 17:48
@jmhooper jmhooper requested review from matthinz and n1zyy May 22, 2024 17:49
@jmhooper jmhooper changed the title Add multiple vector of trust support for SAML LG-13313 Add multiple vector of trust support for SAML May 22, 2024
return @parsed_vectors_of_trust if defined?(@parsed_vectors_of_trust)

@parsed_vectors_of_trust = begin
if vtr.is_a?(Array) && !vtr.empty?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Array class check is confusing to me, what happens if it is not an array?
I'd expect something like Array(vtr).map { ... } which would seamlessly handle arrays and single items

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I merged before I saw this comment. I think this could be cleared up with a simple #blank? check. We want to fall back to ACR values if it is an empty array or nil. I'll follow up in a new PR

@jmhooper jmhooper merged commit d2505df into main May 22, 2024
@jmhooper jmhooper deleted the jmhooper-saml-user-aware-authn-context-resolver branch May 22, 2024 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants