ClientRegistrations#fromIssuerLocation should have shorter (or configurable) Http Connect/Read timeouts

[Kehrlann]

Expected Behavior

When using ClientRegistrations.fromIssuerLocation("http://192.0.2.0") or any non-responding address[1], the http connect timeout should be "reasonable", in seconds so that users get quick feedback when a issuerUri just doesn't respond. Reasonable may be somewhere between 5 and 10s.

Or the ClientRegistrations.rest RestTemplate could be configurable.

Current Behavior

The current timeout is whatever the default timeout is for RestTemplate / underlying HTTP client implementation. In a "default" Boot application, that timeout is 75s. This means the application will hang for 75 seconds, and Tomcat won't even start up.

That is too long in certain contexts, e.g. a Boot app deployed to Kubernetes. The app would take 75 seconds to respond, which could be longer than the user's readiness probe - the pod would get thrashed without giving useful information to the user.

Context

We have seen this issue in k8s deployments, where our pods would crash because a DNS record was (incorrectly) changed and an OIDC issuer did not respond anymore.

Sample

A Boot app with OAuth2 login enabled, and the following config:

spring:
  security:
    oauth2:
      client:
        registration:
          default:
            client-id: "foo"
            client-secret: "bar"
            provider: my-provider
        provider:
          my-provider:
            issuer-uri: https://192.0.2.0 # or any other non-responding IP

This can be reproduced programmatically by calling ClientRegistrations.fromIssuerLocation("http://192.0.2.0");

Happy to contribute a PR.

Footnotes

[1] Used 192.0.2.0 as RFC5737 - IPv4 Address Blocks Reserved for Documentation, works as an example for MacOS but I haven't tried on other systems.

[sjohnr]

Hi @Kehrlann! I think this is closely related to gh-8882, which aims to solve this for a variety of components in the OAuth 2.0 landscape.

See ClientRegistrations section, which states:

NOTE: The underlying HTTP Client used in ClientRegistrations was purposely encapsulated and there is no plan to expose it.

So it does seem that while there are a number of components that may require customizing the RestTemplate, in this particular case it has been decided it would not be addressed. There are a lot of issues (many closed as duplicate or for other reasons) that could be researched to put together the consensus on this, but I think a fair summary would be:

ClientRegistrations.fromIssuerLocation is a convenience around configuring a handful of dynamic properties explicitly. If timeouts and other network/infrastructure concerns make it impractical to rely on for configuration, manual configuration is still fairly practical.

Based on a quick check the code, it seems there are only something like 5 dynamic properties: clientAuthenticationMethod, authorizationUri, tokenUri, jwkSetUri and sometimes userInfoUri. (The issuerUri is obviously set by the value you provide already.) Is it infeasible in your environment to set these properties in a more reliable way that doesn't rely on this utility method? For example, do the values tend to change from host to host often enough that a static utility method based on the issuer you already provide falls apart quickly? Any other ideas for a workaround?

[Kehrlann]

Not exposing the underlying http client makes sense 👌

In the general case, I think the k8s-based case I mentioned above can be a real problem for users, as something that works on your local dev machine may have different network config in your target k8s cluster, and so you may end up with a difficult to debug situation.

Spring Boot uses that utility class for autoconfig, and so I think the general UX would be better (for edge cases) with smaller timeouts. spring-security can decide what a reasonable timeout should be (I don't have an informed opinion other than "75s is too long").

---

For example, do the values tend to change from host to host often enough that a static utility method based on the issuer you already provide falls apart quickly? Any other ideas for a workaround?

We use spring-security + spring-authorization-server to build a federated authorization server, that users can configure on their own upstream identity-providers. It is packaged as a k8s deployment, that users supply config to. So we can't know those properties in advance.

We do want to provide it as a configuration mechanism, rather than asking our users to spell out all of the data:

• our users, who may not be very oauth2-savvy, only have to find the URL that ends with /.well-known/openid-configuration
• it makes the integration guides we may write smaller (e.g., to integrate with Google do this and put issuer-uri: accounts.google.com/.well-known/openid-configuration, for okta do that and put issuer-uri: https://YOUR-TENANT.okta.com/oauth2/default/.well-known/openid-configuration)

Our solutions with the current setup would be to get inspiration from the OIDC auto-config in spring-security and reimplement ClientRegistrations#oidc.

[sjohnr]

Thanks @Kehrlann, I see your point. I only worry about how to manage a decision like this among many other parts of the code base that utilize RestTemplate or WebClient. Would we need to apply this on a wider scale?

If you want to take a crack at a PR to adjust the timeouts I don’t think it can hurt to see what it looks like. Do you want to try your hand at it?

[Kehrlann]

@sjohnr here's the PR - #11232 . I have not added tests - I am unsure where they would go, and I'm unsure how to tweak the values in the test so that they fail faster than 10s.

---

I've looked into the 75s timeout - the actual timeout declared in Java code is 0, "infinite" - I guess the 75s timeout I experienced was an OS-level socket timeout, see:

$ curl 192.0.2.0
curl: (28) Failed to connect to 192.0.2.0 port 80 after 75005 ms: Operation timed out
$ sysctl net.inet.tcp.keepinit # ha-ha, here it is
net.inet.tcp.keepinit: 75000

Note: since the default rest template uses URLConnection under the hood, timeouts can be controlled through the sun.net.client.defaultConnectTimeout system property but I am unsure how wide-ranging the implications would be on a simple app. I've validated it works with -Dsun.net.client.defaultConnectTimeout=5000.

[jgrandja]

@Kehrlann Please see this comment as this may help your scenario. Any feedback would be greatly appreciated.

[Kehrlann]

This works perfectly for my use-case, thanks! Closing this in favor of possible changes outlined in #8882 .