Closes #36777 This PR includes:

  • Adding the ConnectionDetails abstraction for OAuth2 Registration + Provider
  • Docker-Compose Support for the keycloak/keycloak Image

Open Questions: * Since the Registration has a lot fields that cant simply be extracted from any Keycloak env. variables, I added Docker service labels to set e.g. the scope of the client, or the client secret. Is this the best way to handle this?

I also thought about not ignoring the PropertiesOAuth2ClientConnectionDetails if the Docker-Compose support already created an OAuth2ClientConnectionDetails Bean, but rather merge them together, so that the registration-id, provider are extracted from the Keycloak env. variables and the other settings (scope, client-secret, etc.) could be set in the application.properties. I am not sure which to prefer.

When the open questions are answered, I would also add the TestContainer-Support for Keycloak.

Comment From: PhilKes

Any opinions on this @philwebb , @mhalbritter? 😀

Comment From: mhalbritter

We're currently preparing the Spring Boot 3.3 release. This PR is planned as an enhancement for 3.4 and we'll look into it in detail when 3.3 is out.

Comment From: wilkinsona

Thanks for the PR, @PhilKes. Can you please describe how you identified the properties that should be part of OAuth2ClientConnectionDetails? My first impression is that there are too many of them but I may be overlooking something.

The four different labels that are being used to supply some of the values feels like a smell to me as it tells me that these settings (probably) aren't coming from the Docker Compose-managed service but from user configuration. Typically, we'd want the user's standard configuration to be reused (so that their use of Docker Compose matches as closely as possible to running their application "properly") and where test-specific configuration is needed, it can be provided through application properties rather than Docker Compose labels.

Comment From: PhilKes

Thanks for the PR, @PhilKes. Can you please describe how you identified the properties that should be part of OAuth2ClientConnectionDetails? My first impression is that there are too many of them but I may be overlooking something.

The four different labels that are being used to supply some of the values feels like a smell to me as it tells me that these settings (probably) aren't coming from the Docker Compose-managed service but from user configuration. Typically, we'd want the user's standard configuration to be reused (so that their use of Docker Compose matches as closely as possible to running their application "properly") and where test-specific configuration is needed, it can be provided through application properties rather than Docker Compose labels.

Thanks for your remarks @wilkinsona I wasnt really sure what the minimum amount of properties were, since it's not as trivial as e.g. host + user + password, so I simply copied the attributes from OAuth2ClientProperties for a first draft. Now I suppose the bare minimum for OAuth2ConnectionDetails would be: - Registration: - clientId - clientSecret - redirectUri - provider - Provider: - issuerUri?

All other properties should then only be set via OAuth2ClientProperties?

About the labels, the clientId and clientSecret are set for a Registration which has a specified name. If the user defined a Registration in the application.properties, how do we match that Registration to the Provider in e.g. KeycloakDockerComposeConnectionDetailsFactory? In theory there could be more than 1 Registration or Provider, so how do we know which Provider and Registation name should be used in KeycloakDockerComposeConnectionDetailsFactory?

Thank you for your feedback.

Comment From: wilkinsona

I think the breadth of the connection details is going to be quite hard to get right.

I'm not sure that it makes sense to be able to override a registration's provider when using Docker Compose or Testcontainers. If you're using one of the common providers, would we want to encourage overriding it to use a "test" provider? I'm not sure that we would as what you're testing would be quite far removed from what will happen at runtime.

I think I'll take this to #36777 as that's probably a better place to figure it out.

Comment From: wilkinsona

Thanks again for the PR, @PhilKes. Unfortunately, it's become clear from https://github.com/spring-projects/spring-boot/issues/36777 that I don't think we're ready to start implementing something and it looks like this may not be heading in the right direction. I'm going to close this one so that we can take a step back and figure out exactly what we want to do.