Skip to content
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

ClientRegistrations RestTemplate not configurable #14176

Closed
ZIRAKrezovic opened this issue Nov 21, 2023 · 6 comments
Closed

ClientRegistrations RestTemplate not configurable #14176

ZIRAKrezovic opened this issue Nov 21, 2023 · 6 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: bug A general bug

Comments

@ZIRAKrezovic
Copy link

Describe the bug

When ClientRegistrations is used to obtain meta data from issuer that has an invalid HTTPS certificate (in my case, self-signed), it fails with error

Caused by: java.lang.IllegalArgumentException: Unable to resolve Configuration with the provided Issuer of "https://redacted/keycloak/realms/redacted"
        at org.springframework.security.oauth2.client.registration.ClientRegistrations.getBuilder(ClientRegistrations.java:228)
        at org.springframework.security.oauth2.client.registration.ClientRegistrations.fromIssuerLocation(ClientRegistrations.java:152)
        at org.springframework.boot.autoconfigure.security.oauth2.client.OAuth2ClientPropertiesMapper.getBuilderFromIssuerIfPossible(OAuth2ClientPropertiesMapper.java:97)
Caused by: org.springframework.web.client.ResourceAccessException: I/O error on GET request for "https://redacted/keycloak/realms/redacted/.well-known/openid-configuration": PKIX path building failed: sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path t
o requested target
        at org.springframework.web.client.RestTemplate.createResourceAccessException(RestTemplate.java:905)
        at org.springframework.web.client.RestTemplate.doExecute(RestTemplate.java:885)
        at org.springframework.web.client.RestTemplate.exchange(RestTemplate.java:731)
        at org.springframework.security.oauth2.client.registration.ClientRegistrations.lambda$oidc$0(ClientRegistrations.java:163)
        at org.springframework.security.oauth2.client.registration.ClientRegistrations.getBuilder(ClientRegistrations.java:216)

To Reproduce

Minimal application.yml

spring:
  security:
    oauth2:
      client:
        registration:
          keycloak:
            client-id: secret
            client-secret: secret
        provider:
          keycloak:
            issuer-uri: https://redacted/keycloak/realms/realm
            user-name-attribute: preferred_username

Dependencies

<dependency>
	<groupId>org.springframework.boot</groupId>
	<artifactId>spring-boot-starter-security</artifactId>
</dependency>
<dependency>
	<groupId>org.springframework.security</groupId>
	<artifactId>spring-security-oauth2-client</artifactId>
</dependency>

Configuration bean

@Configuration
@EnableWebFluxSecurity
public class SecurityConfiguration {
    @Bean
    public SecurityWebFilterChain securityWebFilterChain(ServerHttpSecurity http) {
        return http.oauth2Login(withDefaults()).build();
    }
}

Spring Boot 3.2.0-RC2, Spring Security 6.2.0-RC2

Expected behavior

I can override WebClient instances for each service that calls my provider and provide a valid TLS certification path. But I cannot do it for ClientRegistrations.

Sample

A link to a GitHub repository with a minimal, reproducible sample.

Reports that include a sample will take priority over reports that do not.
At times, we may require a sample, so it is good to try and include a sample up front.

@ZIRAKrezovic ZIRAKrezovic added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Nov 21, 2023
@sjohnr
Copy link
Member

sjohnr commented Nov 28, 2023

@ZIRAKrezovic, thanks for reaching out!

In 6.2 (recently released), there is a new feature available (see gh-11783) which allows you to more easily configure a RestTemplate for OAuth2 Client components. See docs. This improvement only exists for servlet applications, with reactive support (gh-13763) planned for a future release.

However, regarding ClientRegistrations please see this comment:

ClientRegistrations is intended to be used as a utility/convenience class. It was designed to fulfill most use cases, however, it may not be suitable for certain use cases. For example, if the internal network traffic must be routed through a Proxy, you can bypass discovery by configuring the authorization-uri and token-uri property instead of the issuer-uri property.

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

I'm going to close this issue with the above explanation.

@sjohnr sjohnr closed this as completed Nov 28, 2023
@sjohnr sjohnr added status: declined A suggestion or change that we don't feel we should currently apply in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 28, 2023
@ZIRAKrezovic
Copy link
Author

Hi @sjohnr ... the problem here is without this "internal" class, there is no way to support RP initiated logout because the endsession_uri is configured only via the mentioned ClientRegistrations utility class.

@sjohnr
Copy link
Member

sjohnr commented Dec 4, 2023

@ZIRAKrezovic, I don't think I follow. You can configure RP-initiated logout as per the documentation.

there is no way to support RP initiated logout because the endsession_uri is configured only via the mentioned ClientRegistrations utility class.

I don't think this is correct, as the OidcClientInitiatedLogoutSuccessHandler simply accesses the end_session_endpoint via the following:

clientRegistration.getProviderDetails().getConfigurationMetadata().get("end_session_endpoint")

which can be specified via ClientRegistration.Builder#providerConfigurationMetadata(Map).

@ZIRAKrezovic
Copy link
Author

@sjohnr Forgot to add further context. I am using spring-boot + auto configuration. There is no way to do it with spring autoconfigured client registrations + repository. Thanks for the info, I may have to override a bit more than I'd like to.

@ZIRAKrezovic
Copy link
Author

It's somewhat a hack, but it works ... in case somebody else needs it

import lombok.extern.slf4j.Slf4j;

import org.springframework.beans.factory.InitializingBean;
import org.springframework.beans.factory.config.BeanPostProcessor;
import org.springframework.boot.context.properties.bind.BindResult;
import org.springframework.boot.context.properties.bind.Bindable;
import org.springframework.boot.context.properties.bind.Binder;
import org.springframework.context.EnvironmentAware;
import org.springframework.core.ResolvableType;
import org.springframework.core.env.Environment;
import org.springframework.security.oauth2.client.registration.ClientRegistration;
import org.springframework.security.oauth2.client.registration.InMemoryReactiveClientRegistrationRepository;

import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;

@Slf4j
public class OidcMetadataBeanPostProcessor
        implements BeanPostProcessor, InitializingBean, EnvironmentAware {
    private Environment environment;
    private Map<String, Map<String, String>> properties;

    @Override
    public Object postProcessAfterInitialization(Object bean, String beanName) {
        if (bean instanceof InMemoryReactiveClientRegistrationRepository repo) {
            return replace(repo);
        }

        return bean;
    }

    private InMemoryReactiveClientRegistrationRepository replace(
            InMemoryReactiveClientRegistrationRepository repo) {

        List<ClientRegistration> registrations = new ArrayList<>();
        List<ClientRegistration> modifiedRegistrations = new ArrayList<>();

        repo.forEach(registrations::add);

        Iterator<ClientRegistration> it = registrations.iterator();

        while (it.hasNext()) {
            var registration = it.next();
            if (properties.containsKey(registration.getRegistrationId())) {
                var details = registration.getProviderDetails();

                Map<String, Object> metadata = new HashMap<>(details.getConfigurationMetadata());
                metadata.putAll(properties.get(registration.getRegistrationId()));

                modifiedRegistrations.add(
                        ClientRegistration.withClientRegistration(registration)
                                .providerConfigurationMetadata(metadata)
                                .build());

                it.remove();
            }
        }

        registrations.addAll(modifiedRegistrations);

        if (modifiedRegistrations.isEmpty()) {
            return repo;
        } else {
            log.debug(
                    "Modified [{}] client registrations with metadata from 'security.oidc-meta-data'",
                    modifiedRegistrations.size());

            return new InMemoryReactiveClientRegistrationRepository(registrations);
        }
    }

    @Override
    public void afterPropertiesSet() {
        BindResult<Map<String, Map<String, String>>> bind =
                Binder.get(environment)
                        .bind(
                                "security.oidc-meta-data",
                                Bindable.of(
                                        ResolvableType.forClassWithGenerics(
                                                Map.class,
                                                ResolvableType.forClass(String.class),
                                                ResolvableType.forClassWithGenerics(
                                                        Map.class, String.class, String.class))));

        if (bind.isBound()) {
            properties = bind.get();
        } else {
            properties = Collections.emptyMap();
        }
    }

    @Override
    public void setEnvironment(Environment environment) {
        this.environment = environment;
    }
}

And application.yaml

security:
  oidc-meta-data:
    keycloak:
      first: second

@sjohnr
Copy link
Member

sjohnr commented Dec 5, 2023

@ZIRAKrezovic, I would also recommend you look into simply providing your ClientRegistration programmatically.

You can of course fetch details from the https://redacted/keycloak/realms/myrealm/.well-known/openid-configuration URL yourself if needed. Or you can simply read in those details from your own properties. Either way, you can specify your provider details like end_session_endpoint as part of that without needing to post-process an auto-configuration bean.

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: bug A general bug
Projects
None yet
Development

No branches or pull requests

2 participants