LG-12674 Ensure requested VTR is logged in SAML and OIDC endpoints#10311
Merged
LG-12674 Ensure requested VTR is logged in SAML and OIDC endpoints#10311
Conversation
This commit adds logging to ensure we have visibility into the VTR param that is sent by a service provider using SAML or OIDC. When the param is sent it is expanded to include implied components. For example, “Pb” will be expanded into “C1.C2.P1.Pb”. For debugging purposes we will want visibility into what the service provider actually requests. Additionally, when looking at SP redirects that are served it may be helpful to see the VTR and ACR values that were operated on. This addresses this concern on the following events: - OpenID Connect: authorization request - SAML Auth - SAML Auth Request - SP redirect initiated This commit also updates the SAML request logging params in `analytics_events` to be inclusive of what is actually logged. ### OpenID Connect: authorization request This request already has a `vtr` value that is logged. However this is the parsed VTR value. If the VTR cannot be parsed this value is nil. This commit adds a new `vtr_param` value that includes the raw, unparsed `vtr` param. ### SAML Auth This event logs the result from `SamlRequestValidator#call` which includes the AuthnContext as an extra analytic attribute. This is where the raw VTR is read from. No changes were necessary here besides better documentation of the params. ### SAML Auth Request This commit added logging of the AuthnContext here to include both the raw ACR values and VTR param that are requested. ### SP redirect initiated This commit added logging of the `vtr` and `acr_values` that are present in `sp_session`. This will allow us to see the ACR and VTR values that were operated on. [skip changelog]
Contributor
Author
|
This is a draft because I expect many tests to fail due to changed analytics attributes. I'll mark it ready for review when I have addressed those. |
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.
This commit adds logging to ensure we have visibility into the VTR param that is sent by a service provider using SAML or OIDC.
When the param is sent it is expanded to include implied components. For example, “Pb” will be expanded into “C1.C2.P1.Pb”. For debugging purposes we will want visibility into what the service provider actually requests.
Additionally, when looking at SP redirects that are served it may be helpful to see the VTR and ACR values that were operated on.
This addresses this concern on the following events:
This commit also updates the SAML request logging params in
analytics_eventsto be inclusive of what is actually logged.OpenID Connect: authorization request
This request already has a
vtrvalue that is logged. However this is the parsed VTR value. If the VTR cannot be parsed this value is nil. This commit adds a newvtr_paramvalue that includes the raw, unparsedvtrparam.SAML Auth
This event logs the result from
SamlRequestValidator#callwhich includes the AuthnContext as an extra analytic attribute. This is where the raw VTR is read from. No changes were necessary here besides better documentation of the params.SAML Auth Request
This commit added logging of the AuthnContext here to include both the raw ACR values and VTR param that are requested.
SP redirect initiated
This commit added logging of the
vtrandacr_valuesthat are present insp_session. This will allow us to see the ACR and VTR values that were operated on.