Add application_type to the Dynamic Client Registration request#1613
Open
jayaraman-venkatesan wants to merge 1 commit into
Open
Conversation
application_type to the Dynamic Client Registration requestapplication_type to the Dynamic Client Registration request
5063493 to
cd9ba60
Compare
Contributor
Author
|
@mikekistler - can you please help reviewing this PR |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1545
Implements SEP-837 — adds
application_typeto the Dynamic Client Registration request when the C# SDK registers against an OIDC-flavored authorization server.Summary
Without
application_type, an OIDC AS applies its spec-mandated default. This PR makes the SDK supply the correct value.What happens in the changes
When the caller does set
DynamicClientRegistrationOptions.ApplicationTypeexplicitly, the value is cross-checked against the inferred type atClientOAuthProviderconstruction time. A conflict (for example"web"paired with a localhost redirect URI) throwsArgumentExceptionsynchronously, before any HTTP request is sentWhat changed
src/ModelContextProtocol.Core/Authentication/DynamicClientRegistrationRequest.cs— adds theapplication_typefield on the wire DTO.DynamicClientRegistrationOptions.cs— adds theApplicationTypesetter so callers can opt out of inference and force a specific value.ClientOAuthProvider.cs— resolution happens once, in the constructor:InferApplicationType(Uri redirectUri)helper (mirrors Go sdk's inferApplicationType`).ArgumentExceptionon conflict; otherwise stores the resolved value and writes it back ontooptions.DynamicClientRegistration.ApplicationTypeso callers can observe what was selected.tests/ModelContextProtocol.Tests/Authentication/ClientOAuthProviderApplicationTypeTests.cs(new)127.0.0.1,[::1], custom scheme, https remote.(Parameter 'options').Contract changes
DynamicClientRegistrationOptions:ApplicationType { get; set; }. Backwards-compatible — existing callers default tonulland now get an inferred value where they previously got nothing on the wire.ClientOAuthProvider(constructed viaHttpClientTransport) now throwsArgumentExceptionwhen an explicitApplicationTypeconflicts with the redirect URI shape.ApplicationTypeback tooptions.DynamicClientRegistration.ApplicationType. This is documented in the xmldoc and mirrors Go's behavior.Test plan
dotnet build src/ModelContextProtocol.Core— 0 warnings, 0 errors.dotnet test tests/ModelContextProtocol.Tests(net10.0) — 1969 passed, 0 failed, 5 skippedClientOAuthProviderApplicationTypeTests— 9/9 pass, including exact-message assertion on the conflict throw.