Need to populate an SPSession for each OIDC RP accessed
Description
Environment
has dependent
Activity
Henri MikkonenJune 14, 2022 at 7:42 AM
I verified that SAML2SPSessionCreationStrategy creates BasicSPSessions for OIDC RPs, as its code contains the following if-clause:
I fully agree that we should create OIDCSPSession once the logout is supported, but at this point I feel that BasicSPSession creation by default is satisfactory. However, a new property idp.oidc.SPSessionCreationStrategy enables injecting a custom session creation strategy already, but I kept the default at the SAML2 one that creates BasicSPSessions.
Once is implemented, it should contain creation of OIDCSPSessions, and by then we’ll also know what properties that class should carry.
Scott CantorApril 26, 2022 at 6:18 PM
I didn't bottom out the code but from memory, this is actually not quite right yet. That class creates a SAML2SPSession, not a BasicSPSession. Since we're looking at another patch anyway, we should just go ahead and create an OIDCSPSession class and adjust this accordingly.
Henri MikkonenMarch 2, 2022 at 3:54 PM
BasicSPSession is now put to the IdP session during the front-channel OIDC authentication sequence.
Scott CantorMarch 1, 2022 at 5:15 PM
Yes, for now that’s good enough, it gets them cataloged for those using the UI.
Henri MikkonenMarch 1, 2022 at 4:20 PM
It seems that the existing codebase for SAML is also able to create a BasicSPSession for the OIDC RPs.
If the authorize-flow evaluats the following bean (exactly the same definition in flows/saml/saml2/sso-abstract-beans.xml)
SAML2SPSessionCreationStrategy seems to create a BasicSPSession that contains client_id of RP as the id.
I also verified that OIDC RP appears in the logout page after this change.
Does this sound acceptable before a proper OIDC Logout support? Obviously we could also create a new OIDCRPSession as proposed in the description, but the contents at this phase would probably be equal or very close to BasicSPSession.
It appears the main flow for OIDC doesn't inject an SPSession into the IdPSession for the OIDC RP as a placeholder for logout. This is generally needed even when logout to those RPs isn't supported (e.g., SAML 1.1) so that the UI will include them as failed logouts.
We may or may not need to define an actual OIDCRPSession, but I suspect we'll want to as a placeholder if nothing else.
If we have some inkling of that would be necessary to know to actually support OIDC logout in some way, it could be stored in it of course.