Skip to content

Additional attributes for ClientRegistration and ProviderDetails #9669

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

Open
philsttr opened this issue Apr 21, 2021 · 9 comments
Open

Additional attributes for ClientRegistration and ProviderDetails #9669

philsttr opened this issue Apr 21, 2021 · 9 comments
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: enhancement A general enhancement

Comments

@philsttr
Copy link
Contributor

philsttr commented Apr 21, 2021

New Behavior

I'd like the ability to attach additional attributes/properties to ClientRegistrations and ProviderDetails, so that it can be utilized by ReactiveOAuth2AccessTokenResponseClients and OAuth2AccessTokenResponseClients when requesting tokens.

For example, when configuring ClientRegistrations or Providers via spring-boot auto configuration, I'd like the ability to do something like this:

spring:
  security:
    oauth2:
      client:
        provider:
          my-provider:
            issuer-uri: https://my-issuer
            # New Feature: Custom Attributes
            attributes:
              my-custom-provider-attribute: my-custom-provider-attribute-value
        registration:
          my-client:
            provider: my-provider
            client-id: my-client-id
            client-secret: my-client-secret
            client-authentication-method: basic
            authorization-grant-type: client_credentials
            scope:
              - my.scope.1
              - my.scope.2
            # New Feature: Custom Attributes
            attributes:
              my-custom-registration-attribute: my-custom-registration-attribute-value

And ClientRegistration and ProviderDetails would each have an additional Map<String, Object> attributes property.

Current Behavior

ClientRegistrations and ProviderDetails only support the standard set of attributes/metadata/properties that are utilized by the standard ReactiveOAuth2AccessTokenResponseClients and OAuth2AccessTokenResponseClients

Context

We're utilizing an OAuth 2.0 Authorization Server that supports some additional, non-standard, inputs when requesting tokens. One example is that it supports an input parameter that determines the type of token to return... opaque token or JWT.

In order to support this input, I currently customize the WebClient used by ReactiveOAuth2AccessTokenResponseClient with a custom filter that adds the additional form inputs statically, applying the same values for every request for all client registrations. I'd like for the filter to be able to inspect additional attributes/properties defined on each ClientRegistration or ProviderDetails in order to determine what inputs to add. I know I could create a separate, parallel, set of configuration properties for each client registration and provider, but that gets pretty messy from a configuration property standpoint, and from a spring-boot auto-configuration standpoint. Having first-class support for custom attributes on the client registrations and providers would simplify configuration, and be more intuitive.

@philsttr philsttr added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels Apr 21, 2021
@jgrandja
Copy link
Contributor

jgrandja commented Apr 22, 2021

Hey @philsttr. A way to store custom attributes does seem valuable.

Can you provide a couple more examples so I can better understand the type of attributes you need to store.

I'm wondering if you can leverage the existing ClientRegistration.ProviderDetails.configurationMetadata and we can add ClientRegistration.configurationMetadata, which will be needed when we implement OIDC Dynamic Client Registration.

@jgrandja jgrandja self-assigned this Apr 22, 2021
@jgrandja jgrandja added 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 Apr 22, 2021
@philsttr
Copy link
Contributor Author

philsttr commented Apr 22, 2021

The main use case is for specifying the token type/format... "opaque" or "jwt" for now, but possibly others in the future. Ideally this attribute could be specified at the provider OR client registration level, with the client registration level value taking precedence (when examined by our customized WebClient filter).

A smaller specialized use case is for retrieving a token with all scopes available to the client, without having to enumerate all the scopes to request in the client registration. We know this is not ideal, and are trying to get rid of this specialized use case, but we still need to support it for now. This is currently just a boolean value.

Regarding utilizing configurationMetadata... This sounds like a good approach, but some changes would be needed before we can use it. Specifically, we're using openid, and the ProviderDetails.configurationMetadata is currently populated from the openid .well-known/openid-configuration endpoint, without the possibility to add to it (unless I missed something). So, the ability to add to the metadata returned by the .well-known/openid-configuration endpoint would be needed before we could utilize it.

In total, the following modifications would be needed:

  1. Allow to add additional properties to ProviderDetails.configurationMetadata (in addition to the metadata returned by the .well-known/openid-configuration endpoint)
  2. Create ClientRegistration.configurationMetadata, and allow adding properties to it.

@jgrandja
Copy link
Contributor

@philsttr I agree with the 2 changes you mentioned.

We should also account for the use case where ClientRegistration.configurationMetadata could be updated without the use of Dynamic Client Registration, as well, ClientRegistration.ProviderDetails.configurationMetadata could also be updated without the use of Provider Configuration discovery.

Would you be interested in submitting a PR for this? FYI, there is no rush as this won't be able to go into 5.5.

@jgrandja jgrandja added this to the 5.6.0-M1 milestone Apr 27, 2021
@philsttr
Copy link
Contributor Author

Another engineer on my team, @eurythmia, would like to work on this and submit a PR. Might be a while before we can get to it though.

@jgrandja
Copy link
Contributor

No worries @philsttr. There is no rush for this. I have it scheduled for 5.6.0-M1, which is at least 2-3 months out.

@philsttr
Copy link
Contributor Author

We've had some changes in priorities on our side and will not be able to get to this for a while longer.

If anyone else wants to pick this up in the meantime, please feel free.

@klinck
Copy link

klinck commented Nov 1, 2022

In regards to "configurationMetadata", we think there is a bug in the deserializer where its not able to deserialize the UnmodifiableMap class specified in the json string. We are using Azure as the Oauth2 client credentials provider in this use case.

I was able to solve it temporarily by changing the Jackson Security class handling the deserialization by adding UnmodifiableMap to the allow list.

I thought I would post a comment here just because there is some discussion around using the "configurationMetadata" attribute under ProviderDetails and this is the attribute the deserializer fails on.

Any advice on where I should go from here? This fails in the latest release versions of all dependencies related to Spring security I believe.

The advice given in the exception I am not sure is something we should be fixing anywhere in our code. Seems like it would be a fix for a broader audience.


java.lang.IllegalArgumentException: The class with java.util.Collections$UnmodifiableMap and name of java.util.Collections$UnmodifiableMap is not in the allowlist. If you believe this class is safe to deserialize, please provide an explicit mapping using Jackson annotations or by providing a Mixin. If the serialization is only done by a trusted source, you can also enable default typing. See #4370 for details
at org.springframework.security.jackson2.SecurityJackson2Modules$AllowlistTypeIdResolver.typeFromId(SecurityJackson2Modules.java:253) ~[spring-security-core-5.6.7.jar:5.6.7]

@buckett
Copy link

buckett commented Nov 10, 2022

@klinck I wonder if you're seeing the same problem as I was (reported in #12190) of not registering all the Jackson Modules for Spring Security.

@Ch4ni
Copy link

Ch4ni commented Jul 3, 2024

apologies for resurrecting an old issue. Just tagging myself so that I don't lose sight of this issue since my username has changed from @eurythmia to @Ch4ni.

I intend to start re-loading context on this within the coming weeks for fun and profit.

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) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

8 participants