Simplify dynamic client registration with custom metadata

[dsukhoroslov]

Expected Behavior

• RegisteredClient/OidcClientRegistration classes should provide access any Client Metadata parameters as per OIDC DCR spec
• RegisteredClientRepository should be able to store and retrieve any custom Client Metadata parameters.

Current Behavior

• Current DCR implementation supports only limited set of Client Metadata parameters

Context
in our project we need to register clients with some additional parameters which used in communication with downstream systems. It should be possible as per OIDC DCR spec, but the current implementation supports only limited set of Client Metadata parameters (even not custom, but specified in the spec). Could you consider to extend the current DCR implementation a bit? The most flexible way would be to store initial Client Registration request body as raw JSON string in DB and also provide access to it in RegistreredClient/OidcClientRegistration classes with generic Map<String, Object> interface, probably. Some CustomClientMetadataValidator component could be also useful to be able to customise Client Metadata validation.

[jgrandja]

@dsukhoroslov It's already possible to store custom client metadata during dynamic client registration and access it from RegisteredClient. I've added the following test to demonstrate usage:

spring-authorization-server/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationTests.java

Line 412 in b6ff06d

public void requestWhenClientRegistersWithCustomMetadataThenSavedToRegisteredClient() throws Exception {

You will notice some code duplication in the test so we'll look at improving it. If there is anything else you would like to suggest for improvement please let us know.

[dsukhoroslov]

@jgrandja thank you so much for the hint, using it I was able to store custom client metadata in RegisteredClient. if Converter would be public and not final then it should be possible just to extend it and not duplicate its code, probably..

[jgrandja]

@dsukhoroslov I'm glad it worked out 👍

I'm going to re-open this issue as we will likely make the Converter public to allow for better reuse.

[dsukhoroslov]

ah, ok 👍

[ddubson]

@jgrandja - may I request assignment on this issue?

[ddubson]

@jgrandja - as part of my research on this enhancement, I realized I can easily address #647 as well as it is directly related. Thoughts on that?

[jgrandja]

@ddubson That would be great. But let's keep them separate and submit as separate PR's. Thanks.

[ddubson]

@jgrandja -- here is the breakdown of this task as per my understanding. Please let me know if this makes sense or if you have any revisions.

Issue
when an end-user attempts to dynamically register a new client via /connect/register and they would like to include custom metadata fields along with the supported DCR fields, they are unable to do so -- the custom fields in the request body, once the client is created, are simply ignored. The response body at creation time does not contain the custom fields, and likewise for the response body of a client read operation (i.e. GET /connect/register?client_id=XYZ).

Proposed solution
The auth server should be configurable to accept custom metadata that is pre-registered ahead of time. What this means it that an auth server developer shall register specific custom metadata fields, and then, at client registration time, if that specific field is part of the request, that field shall be saved as a client registration claim and returned back to the user as part of the registration response body and as part of client read response body. Any custom metadata field that is not registered shall be ignored.

Example workflow
An end-user wants to ensure that as a client gets registered, field example_metadata shall be included as part of client registration.

1. The authorization server must register this custom metadata field, like so:

    http.getConfigurer(OAuth2AuthorizationServerConfigurer.class)
        .oidc(oidc -> oidc.clientRegistrationEndpoint(customizer -> {
		  customizer.acceptedCustomClaims(claims -> claims.add("example_metadata"));
         }));

2. At request POST /connect/register, the end-user may now include field example_metadata in the request body. The response body shall return this field and its value.
3. Once registered, at request GET /connect/register?client_id=XYZ, the end user may observe field example_metadata in the response body with its proper value.

[jgrandja]

@ddubson I'm not sure we need to allow for pre-registered custom claims. Please take a look at this comment as well as gh-1044.

I'm thinking the solution is as follows:

• Make OidcClientRegistrationAuthenticationProvider.OidcClientRegistrationRegisteredClientConverter public to allow for reuse and extend it to process the custom metadata in the request
• Expose setClientRegistrationConverter(Converter<RegisteredClient, OidcClientRegistration>) as this will allow to customize the Client Registration Response to include the custom metadata
• Make RegisteredClientOidcClientRegistrationConverter public to allow for reuse and extend it to process the custom metadata for the response

Makes sense?

[ddubson]

Yes this makes sense to me, I understand the approach you provided -- it seems simpler to implement than what I put forward. I'll start on the implementation and will let you know if I have any follow up questions.

[minuth]

@dsukhoroslov It's already possible to store custom client metadata during dynamic client registration and access it from RegisteredClient. I've added the following test to demonstrate usage:

spring-authorization-server/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationTests.java

Line 412 in b6ff06d

public void requestWhenClientRegistersWithCustomMetadataThenSavedToRegisteredClient() throws Exception {

You will notice some code duplication in the test so we'll look at improving it. If there is anything else you would like to suggest for improvement please let us know.

@jgrandja Would you consider adopting a design similar to Spring Security's UserDetails and UserDetailsService, where key components are based on interfaces rather than concrete classes? For instance, defining an interface for RegisteredClient could allow for greater flexibility and support for custom implementations, since the current design are hard to extend and tightly coupled to specific classes:

1. RegisteredClient relies heavily on the builder pattern, which enforces immutability but limits extensibility for custom attributes or behaviors.
2. RegisteredClientRepository directly depends on the RegisteredClient class, making it challenging to provide custom implementations without re-implementing or wrapping the repository.

[jgrandja]

@minuth

defining an interface for RegisteredClient could allow for greater flexibility and support for custom implementations, since the current design are hard to extend and tightly coupled to specific classes

Can you provide more details on the type of custom implementation you are trying to create. What new attributes are you introducing? Could you not use RegisteredClient.clientSettings for your custom attributes?

[minuth]

@minuth

defining an interface for RegisteredClient could allow for greater flexibility and support for custom implementations, since the current design are hard to extend and tightly coupled to specific classes

Can you provide more details on the type of custom implementation you are trying to create. What new attributes are you introducing? Could you not use RegisteredClient.clientSettings for your custom attributes?

Of course, we can put all custom attributes in RegisteredClient.clientSettings, but it seems limited and less flexible for extending the class.