Skip to content

OidcClientInitiatedLogoutSuccessHandler should allow to set Logout Endpoint via other mechanisms that OP's Discovery #10059

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
lmagnien opened this issue Jul 9, 2021 · 4 comments
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement

Comments

@lmagnien
Copy link

lmagnien commented Jul 9, 2021

Expected Behavior

It should be possible to set end_session_endpoint used by OidcClientInitiatedLogoutSuccessHandler via other mechanisms than by configuring the ClientRegistration with the issuer-uri.

Current Behavior

OidcClientInitiatedLogoutSuccessHandler determine end_session_endpoint using ClientRegistration.providerDetails.configurationMetadata obtained during Discovery from URI pointed by ClientRegistration.issuer-uri

	private URI endSessionEndpoint(ClientRegistration clientRegistration) {
		if (clientRegistration != null) {
			ProviderDetails providerDetails = clientRegistration.getProviderDetails();
			Object endSessionEndpoint = providerDetails.getConfigurationMetadata().get("end_session_endpoint");
			if (endSessionEndpoint != null) {
				return URI.create(endSessionEndpoint.toString());
			}
		}
		return null;
	}

Context

According to RP-Initiated Logout documentation, the OP's Logout Endpoint "is normally obtained via the end_session_endpoint element of the OP's Discovery response or may be learned via other mechanisms"

In case of an Oidc Provider which do not expose Discovery endpoint, but support RP-Initiated Logout, it is not possible to use OidcClientInitiatedLogoutSuccessHandler as is.

I did not find better workaround than copy/paste the code of OidcClientInitiatedLogoutSuccessHandler and rewrite endSessionEndpoint method as it is proposed in this stackoverflow response.

If there is no better workaround, it would be interresting to avoid copy/paste OidcClientInitiatedLogoutSuccessHandler. Either by exposing a public setter to configure end_session_endpoint or by creating a new ClientRegistration.endSessionEndpoint attribute and use it in endSessionEndpoint method. Another option would be to make OidcClientInitiatedLogoutSuccessHandler non final.

Maybe a setter with the possibility to add some custom attributes in ClientRegistration as proposed in #9669 would be sufficient.

@lmagnien lmagnien added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels Jul 9, 2021
@jzheaux jzheaux self-assigned this Jul 12, 2021
@jzheaux
Copy link
Contributor

jzheaux commented Jul 12, 2021

I agree that #9669 seems the most sensible way to do this since it would allow applications to specify the endpoint in a custom way.

@jzheaux jzheaux added in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: blocked An issue that's blocked on an external project change and removed status: waiting-for-triage An issue we've not yet triaged status: blocked An issue that's blocked on an external project change labels Jul 12, 2021
@jzheaux
Copy link
Contributor

jzheaux commented Dec 10, 2021

@lmagnien, I think there may be a simpler way. Can you see if this works for you when you are constructing your ClientRegistration instance?

ClientRegistration registration = ClientRegistration.withRegistrationId("id")
    // ...
    .providerConfigurationMetadata(Collections.singletonMap("end_session_endpoint", computedEndpoint()))
    .build();

@jzheaux jzheaux added the status: waiting-for-feedback We need additional information before we can continue label Dec 10, 2021
@lmagnien
Copy link
Author

Yes, you are right. It should work.

I was wrong in my description of the issue. Actually, I should have written that springboot, via autoconfiguration (OAuth2ClientAutoConfiguration), does not allow to set the end_session_endpoint used by OidcClientInitiatedLogoutSuccessHandler via other mechanisms than setting the spring.security.oauth2.client.provider.[providerId].issuer-uri property.

Maybe, I didn't think about your option, which seems to be the right one, because I would have had to give up springboot’s autoconfiguration for the entire ClientRegistration configuration. Which is not very convenient.

Finally, a possible evolution, if one had to be done, would be to make the Springboot autoconfiguration allow setting the ClientRegistration.ProviderDetails.configurationMetadata. As I understand it, that is what is proposed here : #9669 (comment).

Thank you very much for your time and help with this issue.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Dec 14, 2021
@jzheaux
Copy link
Contributor

jzheaux commented Dec 14, 2021

It may be reasonable to add the Spring Boot end_session_endpoint property. If you wish, you can log an issue there, and ping jgrandja for his thoughts.

As far as Spring Security is concerned, it looks like there's nothing more to do for now, so I'll close this issue.

@jzheaux jzheaux closed this as completed Dec 14, 2021
@jzheaux jzheaux added status: declined A suggestion or change that we don't feel we should currently apply and removed status: feedback-provided Feedback has been provided labels Dec 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants