Improvements to dynamic client registration #609

S-Luiten · 2025-07-10T13:49:20Z

Several improvements to dynamic client registration:

Added a delegate for handling the dynamic client registration response. This can be used to retrieve the client id (and client secret for confidential clients) allowing the MCP client to re-use the created client.
Added support for creating public clients.
Added support for authenticated dynamic client registration when the authorization server does not allow creating clients anonymously.

Motivation and Context

Exposing the dynamic client registration response is required to let the MCP client access the client id and client secret. Without these, the MCP client cannot re-use the client and must create a new one every time.
The keycloak realm I'm using is not configured to allow anonymous client registration, so an initial access token is required.

How Has This Been Tested?

This has been tested with an MCP server and MCP client both running locally, using keycloak as authorization server.

Breaking Changes

None

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation update

Checklist

I have read the MCP Documentation
My code follows the repository's style guidelines
New and existing tests pass locally
I have added appropriate error handling
I have added or updated documentation as needed

Additional context

The default OAuth client type has been set to confidential so it's not a breaking change, but public might make more sense as the default?

halter73

Thanks for the contribution. It's a little odd for me to see the Authorization Server set up to support dynamic client registration but still require a secret. If you can securely distribute a secret with your application, why not just pre-register the application with a known client ID and skip DCR altogether?

Nonetheless, I see that Keycloak does require an initial access token for DCR by default and RFC 7591 does talk about the possibility of the client registration endpoint requiring an initial access token, and I don't see the harm in supporting it as long as we move the option into a ClientOAuthOptions.DynamicClientRegistration sub property as not to confuse people.

Can you add some tests for this? You can look at AuthTests.cs and AuthEventTests.cs for examples.

@localden Do you have any thoughts on this?

halter73 · 2025-07-10T16:00:49Z

src/ModelContextProtocol.Core/Authentication/ClientOAuthOptions.cs

+    /// The implementation should save the client credentials securely for future use.
+    /// </para>
+    /// </remarks>
+    public DynamicClientRegistrationDelegate? DynamicClientRegistrationDelegate { get; set; }


Could we move all the dynamic client registration related configuration to a subproperty called DynamicClientRegistration (the type could be call DynamicClientRegistrationOptions). Then you'd have clientOAuthOptions.DynamicClientRegistration.ResponseDelegate, clientOAuthOptions.DynamicClientRegistration.ClientType, and clientOAuthOptions.DynamicClientRegistration.InitialAccessToken. I think it helps clarify these are only for dynamic client registration when dealing with ClientType and InitialAccessToken in particular.

Do you want the existing properties ClientName and ClientUri in this new class as well? It would be a breaking change though.

Yes. Good call. We're not too concerned about making these kinds of breaking changes this early.

halter73 · 2025-07-10T16:01:29Z

src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs

    private readonly string? _clientName;
    private readonly Uri? _clientUri;
+    private readonly OAuthClientType _clientType;


Suggested change

private readonly OAuthClientType _clientType;

private readonly OAuthClientType _dcrRequestedAuthMethod;

Maybe the enum itself should be changed to TokenEndpointAuthMethod as well, because client_secret_post and none are not the only options available. Or instead of an enum it can simply be a string.

halter73 · 2025-07-10T16:04:11Z

src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs

@@ -69,6 +72,7 @@ public ClientOAuthProvider(
        _redirectUri = options.RedirectUri ?? throw new ArgumentException("ClientOAuthOptions.RedirectUri must configured.");
        _clientName = options.ClientName;
        _clientUri = options.ClientUri;
+        _clientType = options.ClientType ?? OAuthClientType.Confidential;


I think it would be clearer if the default value for clientOAuthOptions.DynamicClientRegistration.ClientType was OAuthClientType.Confidential rather than null if that's how we always interpret it.

I'm curious why you added the option for a public client though if you're going through the effort of dynamic client registration? Why not just use a secret if the server is willing to give you one?

If you're not doing dynamic client registration, you can already specify a client ID without a client secret if you want to act as a public client.

I'm curious why you added the option for a public client though if you're going through the effort of dynamic client registration? Why not just use a secret if the server is willing to give you one?

If you're not doing dynamic client registration, you can already specify a client ID without a client secret if you want to act as a public client.

I'm only using DCR because the MCP client I was trying to get this working with does not support specifying the client ID. If you create a new client every time it doesn't really matter if it's public or confidential, but I also included the DynamicClientRegistrationDelegate so that the client can be re-used, and in that case the choice for public or confidential does matter.

I'm still not convinced that the choice of public or confidential matters unless there's some authorization server that supports DCR but doesn't support client secrets which I have not seen yet.

I can see how the DynamicClientRegistrationDelegate is useful, but if you've retained tokens from a previous session using that delegate and use those, you're no longer doing DCR for the subsequent sessions. At that point you're just a normal confidential client like any other time you manually configure ClientOAuthOptions.ClientSecret.

I think we should remove ClientType/OAuthClientType entirely, and just add InitialAccessToken and the DynamicClientRegistrationDelegate to the DynamicClientRegistrationOptions.

halter73 · 2025-07-10T16:04:50Z

src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs

+
+        if (options.InitialAccessToken is not null)
+        {
+            _token = new()


I think we should use a new field for the dynamic client registration initial access token, so we don't conflate it with a normal access token.

I agree, I thought it would make sense but in hindsight this approach only overcomplicates things.

src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs

halter73 · 2025-07-10T16:25:49Z

src/ModelContextProtocol.Core/Authentication/OAuthClientType.cs

+    Confidential,
+
+    /// <summary>
+    /// A public client, typically a client-side application that cannot securely store credentials.


If not for dynamic client registration, this would be true. But given that we do support DCR, why shouldn't we always use a confidential flow given we can just store the secret in memory? It's not like a typically client-side OAuth application without DCR that would need to ship the credential with the executable.

This is what make it weird to me why we'd even give the option to request we do no token endpoint authentication when using DCR. Why even add the option? Does Keycloak not support using client secrets for the token exchange when using DCR?

S-Luiten · 2025-07-11T09:15:09Z

Thanks for the contribution. It's a little odd for me to see the Authorization Server set up to support dynamic client registration but still require a secret. If you can securely distribute a secret with your application, why not just pre-register the application with a known client ID and skip DCR altogether?

Nonetheless, I see that Keycloak does require an initial access token for DCR by default and RFC 7591 does talk about the possibility of the client registration endpoint requiring an initial access token, and I don't see the harm in supporting it as long as we move the option into a ClientOAuthOptions.DynamicClientRegistration sub property as not to confuse people.

Can you add some tests for this? You can look at AuthTests.cs and AuthEventTests.cs for examples.

@localden Do you have any thoughts on this?

@halter73 Thanks for the review, I'm not too familiar with dynamic client registration, but like you say it's the default configuration in Keycloak, so I tried to get it working in that configuration. I agree that pre-registering a client makes a lot more sense than DCR and is also easier, but the problem is that not all MCP clients support this. For example vscode has built-in support for microsoft and github authentication, but only supports DCR for third party authorization servers, so there's no way for me to pass the client id of the pre-configured client. I'm not sure if this has already been adressed in the 1.102 update, because when I tried testing it I ran into this issue: microsoft/vscode#255275

halter73 · 2025-07-11T17:22:18Z

I agree that pre-registering a client makes a lot more sense than DCR and is also easier, but the problem is that not all MCP clients support this. For example vscode has built-in support for microsoft and github authentication, but only supports DCR for third party authorization servers, so there's no way for me to pass the client id of the pre-configured client.

The problem is that clients like VS Code are likely not going to support DCR with an initial access token for the same reason that they do not support using a preregistered client ID and client secret which is that it requires additional configuration. If VS Code ever adds the ability to specify the DCR initial access token via config, why wouldn't they also add support for specifying a client ID and client secret for your preregistered client?

…ions.

S-Luiten · 2025-07-15T14:41:16Z

@halter73 I think I've addressed all your feedback now, let me know if there's anything else you'd like to see changed.

For me this change doesn't have much priority anymore since pre-registering a client and passing its client id is a better solution than DCR. This has also been made possible in the latest vscode insiders build and I finally got it working that way now.

Steven Luiten added 3 commits July 10, 2025 15:16

Add a delegate for handling the dynamic client registration response.

5496590

Support creating a public client in dynamic client registration.

febcbf1

Support passing an initial access token for dynamic client registration.

5700cbc

halter73 reviewed Jul 10, 2025

View reviewed changes

Steven Luiten added 2 commits July 11, 2025 11:33

Do not re-use _token for DCR's initial access token.

0ec0e61

Rename.

210b6f2

Steven Luiten added 5 commits July 14, 2025 09:36

Move DCR-related properties to new class DynamicClientRegistrationOpt…

cf15212

…ions.

Remove OAuthClientType.

b84a954

Revert usings order.

6a301fe

Test DynamicClientRegistrationDelegate.

5816a68

Add paramName to exceptions.

d9fb5cb

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Improvements to dynamic client registration #609

Improvements to dynamic client registration #609

S-Luiten commented Jul 10, 2025

Uh oh!

halter73 left a comment •

edited

Loading

Uh oh!

halter73 Jul 10, 2025

Uh oh!

S-Luiten Jul 11, 2025

Uh oh!

halter73 Jul 11, 2025

Uh oh!

halter73 Jul 10, 2025

Uh oh!

S-Luiten Jul 11, 2025

Uh oh!

halter73 Jul 10, 2025

Uh oh!

S-Luiten Jul 11, 2025

Uh oh!

halter73 Jul 11, 2025

Uh oh!

halter73 Jul 10, 2025

Uh oh!

S-Luiten Jul 11, 2025

Uh oh!

Uh oh!

halter73 Jul 10, 2025

Uh oh!

S-Luiten commented Jul 11, 2025

Uh oh!

halter73 commented Jul 11, 2025

Uh oh!

S-Luiten commented Jul 15, 2025

Uh oh!

Uh oh!

	private readonly OAuthClientType _clientType;
	private readonly OAuthClientType _dcrRequestedAuthMethod;

Improvements to dynamic client registration #609

Are you sure you want to change the base?

Improvements to dynamic client registration #609

Conversation

S-Luiten commented Jul 10, 2025

Motivation and Context

How Has This Been Tested?

Breaking Changes

Types of changes

Checklist

Additional context

Uh oh!

halter73 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

S-Luiten commented Jul 11, 2025

Uh oh!

halter73 commented Jul 11, 2025

Uh oh!

S-Luiten commented Jul 15, 2025

Uh oh!

Uh oh!

halter73 left a comment •

edited

Loading