Tracking AAL attribute value differences #11817
Conversation
| def analytics | ||
| Analytics.new(user: identity.user, request: nil, session: {}, sp: nil) | ||
| end |
There was a problem hiding this comment.
I feel like our typical pattern is to log analytics from the controller level. is there a way that we can have this class expose an analytics_attributes or something that we can leverage from the controller? then we can attribute the right request, SP, session info as needed as well
There was a problem hiding this comment.
i found some examples of this pattern, so thought that it would be okay in this scenario, especially since it's a short-term event that will be removed when it's safe to implement the actual fix.
but i don't feel that strongly about it, and can try to figure out how to expose the needed data at the controller level -- just means it will have to wait until after i'm back!
There was a problem hiding this comment.
I think some of the context such as SP is valuable, maybe as another option we could popular sp: from that data here
There was a problem hiding this comment.
i agree that we could populate sp data -- i'm not sure what "as another option" means exactly here? do you mean keeping this event in the presenter with the included data?
There was a problem hiding this comment.
I think the suggestion is to use identity and pass in sp: identity.service_provider or similar. In this case, it is probably less useful since we pass in client_id: service_provider.issuer, to the event itself.
There was a problem hiding this comment.
i know that the Analytics object adds some extra details if the actually, when i look a little closer, i think that's only the case if there's also a sp value is there, so i can add it in, in case any of those details end up being useful!session object. so maybe it's not that useful
| def analytics | ||
| Analytics.new(user:, request: nil, session: {}, sp: nil) | ||
| end |
There was a problem hiding this comment.
ditto about logging from the controller layer if possible! SAML idp controller is a mess but is there a way?
There was a problem hiding this comment.
this is actually way messier than the OIDC code path, since the AttributeAsserter object is used in private methods in the SamlIdpAuthConcern and is comingled with methods in the SamlIdp gem so i think it's not worth it here, especially for a temporary event.
|
Note: I created a ticket to analyze the data in a few weeks https://gitlab.login.gov/lg-teams/Melba/protocols-backlog/-/issues/147 |
🎫 Ticket
Link to the relevant ticket:
Add tracking for AAL resolution
🛠 Summary of changes
This change:
Note: In the OIDC code flow, this attribute is set in the authorization_form class. My hope is that once this bug is fixed, we will be able to remove that value and associated code.