Skip to content

use client info scope #564

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
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dr3s
Copy link

@dr3s dr3s commented May 29, 2025

Fallback to client information scope when it is provided during client registration.

Motivation and Context

The supported scopes are being passed into DCR but not being used yet in the authorization flow. I believe this is the root cause of modelcontextprotocol/inspector#454

How Has This Been Tested?

Unit test and validated that it fixes the inspector oauth flow when connecting.

Breaking Changes

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

@dr3s dr3s marked this pull request as draft May 29, 2025 18:05
@leblancfg
Copy link

leblancfg commented May 30, 2025

I believe this would be a breaking change – IMO important to highlight as related to auth.

See also my comment in the linked inspector PR: modelcontextprotocol/inspector#455 (comment)

@dr3s
Copy link
Author

dr3s commented Jun 2, 2025

@leblancfg can you explain why it's a breaking change?

I guess I don't consider adding a fallback to be one. In addition, I was under the impression that client registration supports scopes on the input and that is somehow validated against the supported scopes and then returned in the client information upon successful registration.

@pcarleton
Copy link
Contributor

pcarleton commented Jun 3, 2025

Thanks for this change. I've gone back and forth on this, and after digging around a bit, I think we should not add this, and should also remove the client metadata fallback. If clients want to request all available scopes e.g. if a server is not OAuth spec compliant, the methods exist to do that. (see below)

Scopes come up in the MCP SDK’s in several different places:

  • Metadata
    • /.well-known/oauth-authorization-server
    • /.well-known/oauth-protected-resource
    • Both of ^ have a scopes_supported field.
      • In the AS, this means all scopes supported by the AS, some of which could be invalid for the client if the AS supports multiple RS’s that have non-overlapping scopes (see this issue in spring-boot, ty @leblancfg for the ref in the other issue)
  • Registration
    • An input to client registration is an optional set of scopes, both as a request (as in I want my client to be able to request scopes x, y z), and in the response (the server says this client is able to request scopes x, y).
    • If scope is not provided, a default value can optionally be set, or left blank.
  • Authorization
    • In the authorization request, scope is also an optional parameter, that the server can use, ignore, or set a default value. Notably, if it's omitted, the server is required to either use a default value or fail with invalid_scope.

In general, omitting scope should be valid according to the OAuth specs to both the register and the authorization endpoints, and the server should give you reasonable defaults. This means the it would still be "spec-correct" behavior to NOT make this change, and omit scope even if it's provided in the client registration response, and even not include the scopes included in client metadata request.

There are a few cases I'm considering:

  1. An AS trying to indicate scopes that the client should request by including it in the registration response. (It's preferable to use the response rather than the request data in that case)
  2. An AS returning scopes in a registration response that differ from it's preferred "default" scopes.
  3. An AS returning scopes that are invalid for a client, and would cause an error if submitted. (bad server behavior)
  4. An AS not having good default scope behavior (bad server behavior)

(4) was a motivator for originally automatically threading scopes through. The better fix for this issue was on the server, and if that wasn't available, fixing the scope passing outside of this function would be better, while leaving the default behavior to be omitting scope.

(2) seems useful to support to me, which is impossible with this change. A server can have a client that's capable of requesting many scopes, but the default auth flow will give it only the default scopes. If it needs more scopes, it can request them via the scope parameter.

Note: re the springboot issue, that was specifically about piping a scopes_supported result all the way from the metadata endpoint through to the authorization request, and the issue is that an AS could support multiple clients, some of which don't support the same scopes. In this case, we're talking about using the result from /register which is client-specific. That means if we get something back in that field from the server, then it should be valid for that client.

I'll follow up separately on modelcontextprotocol/inspector#455

I'm going to close this for now, but I'm open to reconsidering if we see that (1) and (4) being very common in practice.

@pcarleton pcarleton closed this Jun 3, 2025
@pcarleton
Copy link
Contributor

Caught up with @dr3s, and thought about this a bit more, so want to share the update on that.

Notably, if it's omitted, the server is required to either use a default value or fail with invalid_scope.

  1. An AS not having good default scope behavior (bad server behavior)

(4) is going to be common, and as I quoted it's valid to fail and return invalid_scope if you don't provide a scope value.

A nicer way to get a more specific scope is to get it via the scope= in a WWW-Authenticate, so we'll want to add that behavior, but in the meantime, I'll re-open this to consider the path forward. We'll either want to go this route or retry if we get invalid_scope for not providing it.

@pcarleton pcarleton reopened this Jun 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants