Alternative implementation to logout with id_token_hint #3053
Conversation
strehle
changed the title
| @@ -155,6 +157,10 @@ public Authentication authenticate(Authentication request) throws Authentication | |||
| uaaAuthenticationDetails = UaaAuthenticationDetails.UNKNOWN; | |||
| } | |||
| UaaAuthentication success = new UaaAuthentication(new UaaPrincipal(user), user.getAuthorities(), uaaAuthenticationDetails); | |||
| success.setSamlMessageContext(new SAMLMessageContext()); | |||
There was a problem hiding this comment.
Did you mean to add this? Not sure how it is related to the change.
There was a problem hiding this comment.
Not Saml, but you see here is the set of the SAML credential which can be re-retrieved after login
|
Looks good to me ! okay to merge after addressing comment from Mike. |
| @@ -155,6 +157,10 @@ public Authentication authenticate(Authentication request) throws Authentication | |||
| uaaAuthenticationDetails = UaaAuthenticationDetails.UNKNOWN; | |||
| } | |||
| UaaAuthentication success = new UaaAuthentication(new UaaPrincipal(user), user.getAuthorities(), uaaAuthenticationDetails); | |||
| success.setSamlMessageContext(new SAMLMessageContext()); | |||
| if (authenticationData instanceof ExternalOAuthAuthenticationManager.AuthenticationData authenticationInternal) { | |||
| success.setIdpIdToken(authenticationInternal.getIdToken()); | |||
There was a problem hiding this comment.
I meant that the better place for id_toiken_hint is here, because here is the SAML .
The class https://github.com/cloudfoundry/uaa/blob/develop/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/UaaLoginHint.java is for OAuth-Token-Requests, e.g. https://devforum.okta.com/t/how-to-pass-login-hint-into-authorization-url/26905
There was a problem hiding this comment.
You added line 160 as part of this PL so I'm not sure of your point. Ideally this ExternalLoginAuthenticationManager wouldn't have SAML or OIDC specific code in it. That's why my PL keeps all the id_token_hint changes in the OIDC classes.
There was a problem hiding this comment.
no it not the check, but the fact that UaaLoginHint is used to parse the login_hint parameter.
My request is: use either UaaAuthentication because here are all session relevant information.
In case of SAML the credential is also stored here.
The other class is used to parse requests and therefore you had to jsonIgore
There was a problem hiding this comment.
See my new commit on PL 3049. I'm storing the id_token in the UaaAuthentication now. I think the only significant difference is that I use the existing ExternalOAuthCodeToken to carry the id_token over to where it can be set in the UaaAuthentication, and all of my changes are contained in the OAuth packages.
Sonar: https://sonarcloud.io/summary/new_code?id=cloudfoundry-identity-parent&pullRequest=3053