Skip to content

Jmax/lg 12260 use authn context resolver to compute legacy sp session values#10030

Merged
jmax-gsa merged 32 commits intomainfrom
jmax/LG-12260-use-authn-context-resolver-to-compute-legacy-sp-session-values
Feb 12, 2024
Merged

Jmax/lg 12260 use authn context resolver to compute legacy sp session values#10030
jmax-gsa merged 32 commits intomainfrom
jmax/LG-12260-use-authn-context-resolver-to-compute-legacy-sp-session-values

Conversation

@jmax-gsa
Copy link
Contributor

@jmax-gsa jmax-gsa commented Feb 5, 2024

🎫 Ticket

LG-12260

🛠 Summary of changes

Use the new AuthnContextResolver to translate vector of trust style SP requests into legacy values in the SP session.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

  • Ensure that the new specs in spec/services/store_sp_metadata_in_session_spec.rb all pass.
  • Ensure that all existing specs pass

Extracted `request_id`
Extracted `app_session`
Extracted `instance`
Renamed `app_session_hash` to `expected_sp_values`
Extracted `requested_attributes`
Renamed `url` to `request_url`
@jmax-gsa jmax-gsa requested review from a team and jmhooper February 5, 2024 16:37
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like request_acr never gets set anywhere. It should be the ial and aal strings separated by a space ( ) when ACR values are used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added specs for this.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need this conditional.

The VOT Parser can handle ACR values. We should be able to pass both the ACR and VOT value into the parser and have it work.

I think the feature flag should prevent us from consuming and storing the VTR param rather than from using it here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I just integrated the feature flag into the form where the VTR is consumed here: 1cb5ed4

changelog: Upcoming Features,VOT,Handle vector of trust in SP requests
@jmax-gsa jmax-gsa force-pushed the jmax/LG-12260-use-authn-context-resolver-to-compute-legacy-sp-session-values branch from dedcecd to 69da264 Compare February 5, 2024 18:16
end

def biometric_comparison_required_value
parsed_vot.biometric_comparison?
Copy link
Contributor

Choose a reason for hiding this comment

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

Here I think we want to look at parsed_vot.biometric_comparison? || sp_request.biometric_comparison_required. Since the current biometric comparison API is experimental and exists outside of the acr_values and vtr params we'll need to look at what is on the SP request until we update the sample OIDC SP.

@jmax-gsa jmax-gsa merged commit ccc2b18 into main Feb 12, 2024
@jmax-gsa jmax-gsa deleted the jmax/LG-12260-use-authn-context-resolver-to-compute-legacy-sp-session-values branch February 12, 2024 19:20
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.

2 participants