Hi,

Let me start by thanking you for your service to the Java community. I have been using Spring + Spring security for more than 15 years. So, I owe the Spring team a debt of gratitude.

Here, I am going to explain why the assertion on the issuer URI in ClientRegistrations#withProviderConfiguration is preventing a very legitimate use case. While adding very little value.

Use case: Backchannel communication with the Identity provider (IDP)

What I mean with Backchannel communication with IDP is that, sometimes you need/want to use a private URI (Within a private network) to communicate with the IDP to retrieve certs and fetch tokens.

No. you can do it already using "authorization-uri", "user-info-uri", "token-uri" and "jwk-set-uri". Not Exactly!

Probably you are thinking right now that one can already do that using "authorization-uri", "user-info-uri", "token-uri" and "jwk-set-uri" settings. Although using above mentioned setting kind of works. But it has some shortcomings for example there is no way to set "end_session_endpoint" and therefore single logout functionality would not work. Also, there are another 50+ different settings in the openid-configuration that you can not set e.g. "response_types_supported", "id_token_encryption_alg_values_supported", etc. Therefore I believe having the possibility of configuring a private URI for the issuer is useful.

Why backchannel communication with IDP is useful?

Let me start with an example I have an application that is running on Kubernetes and I use Keycloak as IDP which is running on the same K8S cluster. I would like to configure the issuer URI to be the URI of the Keycloak service within my k8s rather than Its public address because:

  • having external dependency is not a good thing: Let's say a DNS service failure should not prevent the application from starting or scaling up. In fact, this issue came to my attention during a recent regional disruption of Cloudflare service. While Cloudflare was working well in most of the world. The application could not scale up because regionally the DNS services were disrupted.
  • The local network is safer, faster, and more robust than the public network

I am not the only one who thinks that backchannel communication with IDP is useful. In fact, Keycloak already has a setting for it. if you set the variable "hostname-url" and then you look up "/realms//.well-known/openid-configuration" you will see that all the hostnames except in "token_endpoint", "introspection_endpoint", "userinfo_endpoint", "backchannel_authentication_endpoint" and "jwks_uri" are using the hostname you are you have configureed but the abovementioned fields are using the hostname you used to fetch the openid-configuration. These are exactly the fields that you want to use in the backchannel communication with your IDP. In fact, you can switch off this behavior using the flag "hostname-strict-backchannel=true" (Please notice that hostname-strict-backchannel is by default off) I guess the point that I am trying to make is backchannel communication is not some crazy corner case but a quite common use case.

What is the added value of this assertion?

I can think of two possible purposes for this assertion

  • Security
  • Alerting the developer to miss configuration (Failfast approach)

Security: In theory, if the IDP/DNS is compromised the attacker might want to point the application to a different IDP. But to be frank, If the IDP's DNS is compromised the attackers can run their own IDP with the same hostname therefore this extra check would not prevent them from breaking into the application. Another scenario is that your IDP is compromised and the attacker can change the values of the "/.well-known/openid-configuration" at will. But in that case, I argue that they don't need to send your application to a different IDP provider because they already have pretty much full control of your IDP. So I really don't see how this assertion can make the application more secure. In fact, it makes it less secure by fetching the issuer and certs via the public network.

Alerting the developer to miss configuration: That is the only case in which I see some added value for this asset. Especially in case, some rookie uses the HTTP version of the URI instead of HTTPS.

But in general, I believe this assertion does more harm than good.

Alternative solutions

Removing this assertion is not the only solution to the problem. Another approach would be to have a separate configuration for private issuer URI let's say we add a setting like "private-issuer-uri". When present it is used to fetch the issuer config instead of "issuer-uri" but we can use the value of "issuer-uri" for the assertion. So "issuer-uri" would be the public issuer URI and "private-issuer-uri" would be the private issuer URI. Another approach would be to make the assertion switch off using a flag like "backchannel-communication: true" so that anybody who would like to use an internal network to fetch the issuer can switch off this assertion.

Best regards, Ebrahim Aharpour

Comment From: sjohnr

@aharpour thanks for your detailed writeup! Before we discuss the proposal here, I have a few questions to start.

for example there is no way to set "end_session_endpoint" and therefore single logout functionality would not work.

I am struggling to understand the statement "no way to set." It sounds like you're saying it is impossible, but of course it is possible via ClientRegistration.Builder#providerConfigurationMetadata. Can you please explain more what you are not able to do?

Also, there are another 50+ different settings in the openid-configuration that you can not set e.g. "response_types_supported", "id_token_encryption_alg_values_supported", etc.

Same question. Can you please explain why you feel you cannot set these values?

Comment From: aharpour

@sjohnr Sorry, I should have been more clear. In both cases I meant using Spring boot configuration, there is no way to set providerConfigurationMetadata and by extension "end_session_endpoint". Of course, one can provide their own implementation of ClientRegistrationRepository and take full control of client registration. Actually, That is what I have done at the moment in my project. But I am hoping that by providing a more official way of pointing to the issuer via a private URI. I can get rid of those customizations because I feel I am reaching too deep into the Spring security code and that leaves me vulnerable to potential changes in every upgrade, while my case is quite mainstream and does justify such customization.

Comment From: jgrandja

@aharpour @sjohnr This issue seems related to https://github.com/spring-projects/spring-boot/issues/21375#issuecomment-1845620916

Comment From: sjohnr

@aharpour thanks for the clarification!

Actually, providing a ClientRegistrationRepository that overrides the one provided by Spring Boot is the intended path for users (see tip at the end of OAuth2 Client docs), so you are doing the right thing. You need not worry that you are reaching too deeply into Spring Security. The ClientRegistrations class is provided for convenience, but is not the required (or even recommended) way to always create a ClientRegistration. Since creating your own ClientRegistrationRepository is intended, you will continue to benefit from upgrades as we strive to not break backwards compatibility whenever possible.

There are a few possible ways to provide the metadata you are wanting to provide. Of course, you can call the private URL yourself and create your own instance of ClientRegistration.Builder to do what you need or just create your own instance with hard-coded properties. It sounds like you are creating your own instance programmatically at the moment, so you are good to go there.

Regarding the assertion on the issuer URI, it is there because it is mentioned in the spec. I understand your point and it could be argued that this validation is unnecessary. However, I always find implicit assumptions that aren't checked in the code to allow for subtle invisible bugs and problems, regardless of their security impact, and I have had this particular assertion catch bugs in my configuration in the past, so I believe it is valuable. For that reason, I do not believe we would simply want to remove it.

Your use case should be achieved through building a custom ClientRegistration.Builder as discussed above, and you can/should fetch the provider details using your own HTTP client if needed. Keep in mind that the ClientRegistrationRepository actually intends to be backed by a database in which case you wouldn't even be fetching/specifying the details on startup, but at application deployment time.

I'm going to close this issue and corresponding PR with the above explanation. Please let me know if you feel I have misunderstood you and we can discuss it further.

Comment From: aharpour

@sjohnr Thank you very much for the clear explanation. In particular, the fact that I was by passing ClientRegistrations class was giving me an uneasy feeling. Your explanation put my mind at ease. Thank you!

Comment From: sjohnr

@aharpour @sjohnr This issue seems related to spring-projects/spring-boot#21375 (comment)

Thanks @jgrandja! I actually didn't see your comment before replying, but I think we're on the same page here.

Comment From: daniel-cues

@aharpour How did you code your custom ClientRegistration? Do you have code we can use as reference?

Comment From: sjohnr

@aharpour How did you code your custom ClientRegistration? Do you have code we can use as reference?

@daniel-cues please check the docs regarding ClientRegistrationRepository.