Skip to content

Expose property to configure configurationMetadata on an OAuth2 ClientRegistration #21375

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
nagaraj-pattar opened this issue May 10, 2020 · 18 comments
Labels
status: declined A suggestion or change that we don't feel we should currently apply

Comments

@nagaraj-pattar
Copy link

nagaraj-pattar commented May 10, 2020

The class org.springframework.boot.autoconfigure.security.oauth2.client.OAuth2ClientProperties.Provider is required to have the following

    private Map<String, Object> configurationMetadata;

    public Map<String, Object> getConfigurationMetadata() {
        return configurationMetadata;
    }

    public void setConfigurationMetadata(Map<String, Object> configurationMetadata) {
        this.configurationMetadata = configurationMetadata;
    }

Due to this missing code, we are unable to configure additional metadata configuration like

"end_session_endpoint".

Request you to make this fix so that client registration defined in the application.properties / application.yml file can be configured with additional metadata and OidcClientInitiatedLogoutSuccessHandler can consume this metadata.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label May 10, 2020
@mbhave mbhave added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels May 11, 2020
@mbhave mbhave added this to the 2.2.x milestone May 11, 2020
@mbhave mbhave changed the title OAuth2ClientProperties missing configuration metadata placeholders Expose property to configure configurationMetadata on an OAuth2 ClientRegistration May 11, 2020
@mbhave mbhave modified the milestones: 2.2.x, 2.2.8 May 12, 2020
@mbhave mbhave closed this as completed in 7b79029 May 12, 2020
@mbhave mbhave reopened this May 13, 2020
@mbhave
Copy link
Contributor

mbhave commented May 13, 2020

I am going to revert this change. After speaking with @jgrandja, it's become clear that configurationMetadata on the provider is not meant to be set by the application. It is part of OpenID Connect's Discovery Metadata. If a issueUri is configured, Spring Security will get the metadata from the issuerUri and set it on the ClientRegistration.

@nagaraj-pattar Our guess as to why you need to explicitly set the end_session_endpoint is that the OpenID Connect provider you're using doesn't implement OpenID Connect discovery or it doesn't provide a value for end_session_endpoint in the metadata. If you must set the configurationMetadata yourself, you can do so by creating a ClientRegistration yourself and providing a ClientRegistrationRepository with it.

@mbhave mbhave added status: declined A suggestion or change that we don't feel we should currently apply and removed type: enhancement A general enhancement labels May 13, 2020
@mbhave mbhave removed this from the 2.2.8 milestone May 13, 2020
mbhave added a commit that referenced this issue May 13, 2020
@mbhave mbhave closed this as completed May 13, 2020
@nagaraj-pattar
Copy link
Author

I faced few issues with issuer mismatch. If the issuer has mismatched, then those clients registrations were rejected by spring boot and application was not starting up. In such scenarios these properties will be useful to read client configuration from properties. There are already place holders for other URIs, hence, it is wise to expose these properties.

@mbhave
Copy link
Contributor

mbhave commented May 14, 2020

Issuer mismatch sounds like some sort of misconfiguration. Configuring the configuration metadata manually is not what a majority of users would do and we don't want to expose a property that would let them do that. Like I said, if you must configure it manually, you can create your own ClientRegistration using the Builder yourself.

If you have any more questions please ask them on Gitter. Spring Security's Gitter might be a better place to ask anything about issuer mismatch or configurationMetadata.

@dawi
Copy link

dawi commented Dec 13, 2020

Configuring the configuration metadata manually is not what a majority of users would do.

Manual metadata configuration has one advantage.
The application is able to start, even if the authorization server is not reachable.
I don't think that it is very likely, it's still an advantage though.

Actually this is the reason why I configured the metadata manually and stumbled over this issue.

@mbhave Is this decision final?

@mbhave
Copy link
Contributor

mbhave commented Dec 14, 2020

@dawi As mentioned in the comment above, if you must configure the metadata manually, you can do so by creating your own ClientRegistration using the Builder.

@adase11
Copy link

adase11 commented Nov 22, 2023

Coming back to this. Unless there is a strong reason for not allowing additional configuration after OpenID Connect discovery I think that Boot could offer the ability to set additional metadata properties that the provider doesn't offer configuration for. This would facilitate the case where the user (client) wants to be able to use discovery but is working with a provider that does not allow them to set the end_session_endpoint (and therefore in the current state is unable to fully rely on discovery).

For example, after working with a provider that does not offer the configuration of the end_session_endpoint property I used a variation of the solution in spring-projects/spring-security#7695 to get things working. This works fine but it seems to me that this could be something that Spring Boot provides out of the box.

I'd propose that Spring Boot could offer a way, via properties, to specify additional configurationMetadata properties and have those specified values be added to any already provided by .well-known/openid-configuration discovery for the particular ClientRegistration.

This seems related to the following two previous issues that I came across in my search for a solution
spring-projects/spring-security#7695
spring-projects/spring-security#10059

Also, the other way I've solved for this is to set up a LogoutSuccessHandler that calls the end_session_endpoint. This felt like more of a "hack" to me than an appropriate solution but I'm open to hearing other opinions on that.

@wilkinsona
Copy link
Member

What's your take on this please, @jgrandja?

@daniel-cues
Copy link

daniel-cues commented Dec 7, 2023

@mbhave I disagree with Issuer mismatch always being a sort of misconfiguration. There are justified cases in which the issuer might mismatch. For example, I might use subdomains for each application Identity manager and this leads to a mismatch:

{
"issuer":"https://identity.mydomain.com/",
"authorization_endpoint":"https://identity-1238751009726.identity.mydomain.com/oauth2/v1/authorize",
"token_endpoint":"https://identity-1238751009726.identity.mydomain.com/oauth2/v1/token",
...
}

This setup works on other frameworks, I can't find a reason why it shouldn't on spring. I think there should be either an option to allow mismatches or at least an option to load all metadata manually.

#21375 (comment) @adase11 That sounds definitely like a hack. The fact is, OidcClientInitiatedLogoutSuccessHandler exists specifically for that exact purpose but it cannot be used if end_session_endpoint is not set, which can happen for a myriad of reasons. I don't see the point on supporting this many property mappings, if they're not complete and actively make functionality unavailable.

@jgrandja
Copy link

jgrandja commented Dec 7, 2023

@adase11

I think that Boot could offer the ability to set additional metadata properties that the provider doesn't offer configuration for

I'm curious, why doesn't the provider return the specific configuration metadata? It looks like you are referring to end_session_endpoint? If this is not being returned in the provider configuration response, then the provider does not provide this capability, which happens to be the Logout Endpoint.

However, since you're looking to set this manually, it appears that the provider does in fact support the Logout Endpoint? If this is the case, then the Provider should be configured correctly to return end_session_endpoint. This is a configuration that should be applied at the provider. Allowing the flexibility to manually set provider configuration metadata at the client application (via Boot properties) does not make sense to me, especially for this particular case.

What provider are you using? Are you able to configure it or is this managed by another team?

@jgrandja
Copy link

jgrandja commented Dec 7, 2023

@daniel-cues

There are justified cases in which the issuer might mismatch. For example, I might use subdomains for each application Identity manager and this leads to a mismatch

Please review the spec at 4.3. OpenID Provider Configuration Validation:

The issuer value returned MUST be identical to the Issuer URL that was directly used to retrieve the configuration information. This MUST also be identical to the iss Claim value in ID Tokens issued from this Issuer.

FYI, this validation check lives in Spring Security (ClientRegistrations) not Spring Boot. And as mentioned above, this validation is implemented to spec.

I understand your environment is using subdomains for each issuer (very common), however, this is still an environment related issue and should be addressed there.

@adase11
Copy link

adase11 commented Dec 7, 2023

@jgrandja Here's Google's documentation around OIDC discovery which points to their .well-known/openid-configuration from accounts.google.com that doesn't contain the end_session_endpoint. I cant say definitively if there isn't some other reason that discovery file excludes the end_session_endpoint.

Another one I've seen more details on is Auth0. Auth0 doesn't allow users to configure the end_session_endpoint like they do for all the other OAuth endpoints. There, now is more complex ways to configure this with Auth0 (there are a few mentions of this on their message boards here here). What they've set up is that clients can contact support to get the endpoint added to the configuration (see docs) however I figured that if they treat this differently than others might as well.

For what it's worth, I definitely hear the argument that the Boot shouldn't have to account for providers that are non compliant to the spec

@pelepelin
Copy link

you can do so by creating a ClientRegistration yourself and providing a ClientRegistrationRepository with it

Missing ability to override a couple of additional properties requires copying a very big chunk of code which handles all the configuration properties under spring.security.oauth2.client.

In my case OIDC provider is compliant but it is served via a reverse proxy which is not ready at the moment of Spring app configuration and it makes impossible to fetch from issuer-uri. And that's ridiculous that it's not possible to provide the data from .well-known/openid-configuration the other way than via hardcoded RestTemplate in a final class ClientRegistrations.

@wilkinsona
Copy link
Member

@pelepelin can you please describe your use case in some more detail? Specifically, it would be useful to know exactly what additional configuration you would like to be able to provide and why that's necessary when working with a compliant OIDC provider. Also, where does the reverse proxy fit in and how does that adversely affect things?

@pelepelin
Copy link

pelepelin commented Mar 13, 2024

@wilkinsona Generally it's similar to the last linked ticket spring-projects/spring-security#14257

  • When an application and OIDC provider are in the same private network, it could be desirable (secure, reliable) to load OIDC configuration via private hostname but it fails because OIDC is configured to return issuer matching externally accessible domain.
  • When a spring-cloud-gateway is used as a front reverse-proxy to OIDC provider and it exposes OAuth login at the same time, OAuth just can't be configured by issuer-uri for the same reason, since gateway endpoints are not yet served at the configuration phase.
  • If OAuth login is configured with other properties under spring.security.oauth2.client.provider instead of issuer-uri, most things work, except of "RP-initiated logout", for which end_session_endpoint is required.

So in the end I followed https://docs.spring.io/spring-security/reference/reactive/oauth2/login/core.html#webflux-oauth2-login-register-reactiveclientregistrationrepository-bean and it's not so much code but I was disappointed that I can't use spring-boot configuration properties provided out of the box (and might miss something configuring the client with my code).

@wilkinsona
Copy link
Member

Thanks, @pelepelin.

WDYT of this use case, @jgrandja?

@alexist
Copy link

alexist commented Jul 11, 2024

In my case : We have lot of OIDC providers, some are managed by our organization, and other are provided by our customer. Configuring issuer uri work, but : if only one provider is down, Spring application will not start.
It should be great if i WE Can possible configure everything
(The application should start in degraded mode and try to resolve later Dynamic URL configuration)

@jgrandja
Copy link

@pelepelin

Missing ability to override a couple of additional properties requires copying a very big chunk of code which handles all the configuration properties under spring.security.oauth2.client

I'd like to see the code duplication your application is requiring. Can you please put together a minimal sample demonstrating where you are needing to copy code and we'll see what we can do to help reduce it. Feel free to use the Spring Authorization Server Demo Sample as a starting point.

@pelepelin
Copy link

pelepelin commented Jul 24, 2024

@jgrandja I'm afraid a self-containing sample for my use case would involve a docker-compose, gateway service and keycloak as an identity provider. However, for my sentence the configuration bean below is enough, companion custom properties class is obvious.

This is effectively a specialized version of OAuth2ClientPropertiesMapper with the difference that it sets some more properties, notably (up: fixed) it sets issuerUri without network validation, and it sets end_session_endpoint for which there is no setter in spring-boot.

@Configuration
@AllArgsConstructor
public class OAuth2Config {

    @NonNull
    SecurityProperties securityProperties;

    @Bean
    public ReactiveClientRegistrationRepository clientRegistrationRepository() {
        return new InMemoryReactiveClientRegistrationRepository(this.clientRegistration());
    }

    private ClientRegistration clientRegistration() {
        OAuth2Client props = securityProperties.oauth2Client();
        return ClientRegistration.withRegistrationId(props.registrationId())
            .clientId(props.clientId())
            .clientSecret(props.clientSecret())
            .clientName(props.clientName())
            .clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC)
            .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE)
            .redirectUri("{baseUrl}" + securityProperties.authorizationCallbackPath())
            .scope(props.scope())
            .userNameAttributeName(props.userNameAttribute())
            .issuerUri(props.issuer().toASCIIString())
            .authorizationUri(props.authorizationEndpoint().toASCIIString())
            .tokenUri(props.tokenEndpoint().toASCIIString())
            .userInfoUri(props.userinfoEndpoint().toASCIIString())
            .jwkSetUri(props.jwksUri().toASCIIString())
            .providerConfigurationMetadata(Map.of(
                "end_session_endpoint", props.endSessionEndpoint().toASCIIString()
            ))
            .build();
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

10 participants