inspector icon indicating copy to clipboard operation
inspector copied to clipboard

[OAuth] Request all supported scopes when not using the OAuth debugger

Open leblancfg opened this issue 7 months ago • 2 comments

Adds a "scope" section to the Authorization URL in the OAuth flow, matching the OAuth debugger behaviour.

Motivation and Context

When using the OAuth debugger, it adds the scope search parameter with the ones provided in the scopes_supported key.

However, the main "connect" button does not pass them to the MCP SDK and they are missing from the call. Some OAuth providers and setups will require you to have them and the OAuth flow fails in these cases. This change fixes that.

Closes #454

How Has This Been Tested?

Local tests for a streamable HTTP MCP server were successful when using the OAuth debugger. The URL generated in the debugger is (scroll to see the end):

https://oauth.example.com/oauth/authorize?response_type=code&client_id=$CLIENT_ID&code_challenge=$CODE_CHALLENGE&code_challenge_method=S256&redirect_uri=http%3A%2F%2F127.0.0.1%3A6274%2Foauth%2Fcallback%2Fdebug&scope=read+write

but the "Connect" button currently returns:

https://oauth.example.com/oauth/authorize?response_type=code&client_id=$CLIENT_ID&code_challenge=$CODE_CHALLENGE&code_challenge_method=S256&redirect_uri=http%3A%2F%2F127.0.0.1%3A6274%2Foauth%2Fcallback%2Fdebug

This PR makes it so scopes are passed if the scopes_supported key is present in the authorization server metadata.

Breaking Changes

MCP servers that rely on implicitly using default scopes will now have them explicitly set, and those scopes might differ.

Types of changes

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

Checklist

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

leblancfg avatar May 27 '25 21:05 leblancfg

opened https://github.com/modelcontextprotocol/typescript-sdk/pull/564 because it seems all clients that I tested need this.

dr3s avatar May 29 '25 18:05 dr3s

it seems all clients that I tested need this

This would indeed be simpler.

However, I was under the impression scopes_supported is not meant to indicate that an OAuth client should request all scopes in the list. I think this is an OK assumption to make in the inspector (meant for debugging), but I didn't think it was an OK assumption to make at the SDK layer.

Related issue in Spring Boot, where this used to be the default and was changed to i.e.

if metadata.getScopes() contains openid then
default to openid, profile, email
else
default to openid

maybe this is a better default, even in the inspector?

leblancfg avatar May 30 '25 01:05 leblancfg

unfortunately, there's no way for the Client to know the correct scope to request without using the oauth server or registered client metadata. MS Entra requires the scope to be passed and the minimal scopes suggested won't be sufficient until we have some way of triggering consent for additional scopes when privileges need to be escalated.

dr3s avatar Jun 02 '25 17:06 dr3s

unfortunately, there's no way for the Client to know the correct scope to request without using the oauth server or registered client metadata. MS Entra requires the scope to be passed and the minimal scopes suggested won't be sufficient until we have some way of triggering consent for additional scopes when privileges need to be escalated.

In the Spring Boot example, it was a convincing argument that defaulting to all scopes is maybe not as good as defaulting to none. I have to agree with @leblancfg that in the Inspector, it would be less problematic than in the SDK.

cliffhall avatar Jun 02 '25 20:06 cliffhall

@cliffhall I see the "waiting on submitter" tag, anything in particular you'd like me to clarify?

leblancfg avatar Jun 03 '25 19:06 leblancfg

@cliffhall I see the "waiting on submitter" tag, anything in particular you'd like me to clarify?

Really waiting to hear @pcarleton's take. When closing the counterpart PR in the SDK, he said he'd follow up here.

I think it's probably ok to request all scopes in the Inspector, but maybe the another solution is a configuration panel field that lets you put in the scopes you want to request, and if nothing is there, we request none.

cliffhall avatar Jun 03 '25 22:06 cliffhall

@cliffhall a configurable UI panel may be ok for inspector but I can't see it being suitable for other MCP clients. The configuration would be too technical for most users. Even for inspector, users wouldn't understand some scope unless they were also the MCP server dev.

dr3s avatar Jun 04 '25 15:06 dr3s

@cliffhall a configurable UI panel may be ok for inspector but I can't see it being suitable for other MCP clients. The configuration would be too technical for most users. Even for inspector, users wouldn't understand some scope unless they were also the MCP server dev.

@dr3s You're right, for other MCP clients a scopes field would not be appropriate at all. But this tool is primarily used by MCP server devs to test their own servers. If you wanted to ensure that your server only authorized and allowed interaction with the requested scopes, this would probably be the easiest way to do it.

cliffhall avatar Jun 04 '25 16:06 cliffhall

Hey sorry for the delay!

Here's the issue we run into, imagine an example like this:

  • supported_scopes: [read, write, admin]
  • implicit required default scope read that the client doesn't know about unless they read the server's docs
  • user attempts auth with scope omitted, gets invalid_scope error
  • user attempts auth again with read write admin as scope parameter... more privilege than it needs. (problem is the client doesn't know what to pick here.)

The best we can do with the current spec is to rely on the WWW-Authenticate header to provide a scope. So the order of preference would be:

  1. Use the scope included during the initial 401 WWW-Authenticate header response
  2. Omit scope otherwise, and let the server give defaults
  3. include all of supported_scopes as a final fallback after ^ we get invalid_scope

This change jumps straight to (3), but also (1) isn't supported in the SDK yet.

I think we could either: a. Merge this, so it's consistent with the debugger, and then fix them both later once the SDK supports (1) b. Not merge this (and then also "undo" the debugger scope thing), and wait to fix them until SDK supports (1) c. Not merge this, and require manually inputting scopes if you don't want "default".

I'm slightly in favor of (a) since it should reduce confusion and increase compatibility. The biggest downside would be having it be in the inspector and having that be "contagious" where others start using the pattern and it becomes the norm to request all scopes.

pcarleton avatar Jun 09 '25 10:06 pcarleton

The best we can do with the current spec is to rely on the WWW-Authenticate header to provide a scope. So the order of preference would be:

  1. Use the scope included during the initial 401 WWW-Authenticate header response
  2. Omit scope otherwise, and let the server give defaults
  3. include all of supported_scopes as a final fallback after ^ we get invalid_scope

This change jumps straight to (3), but also (1) isn't supported in the SDK yet.

I think we could either: a. Merge this, so it's consistent with the debugger, and then fix them both later once the SDK supports (1) b. Not merge this (and then also "undo" the debugger scope thing), and wait to fix them until SDK supports (1) c. Not merge this, and require manually inputting scopes if you don't want "default".

I'm slightly in favor of (a) since it should reduce confusion and increase compatibility. The biggest downside would be having it be in the inspector and having that be "contagious" where others start using the pattern and it becomes the norm to request all scopes.

Thanks for weighing in, @pcarleton. I agree the that getting all scopes now but moving quickly to amend when we have a better option sounds like the right thing.

cliffhall avatar Jun 09 '25 15:06 cliffhall

@leblancfg this needs a prettier-fix run to pass CI.

cliffhall avatar Jun 09 '25 15:06 cliffhall

I tried the simpleStreamableHttp example in the typescript-sdk using:

npx tsx src/examples/server/simpleStreamableHttp.ts --oauth

but it's difficult to interpret the results.

Not using the debugger first, just clicking 'Connect'

The authorize call has scope query param

Screenshot 2025-06-10 at 5 54 10 PM

Error reporting that the mcp:tools scope was invalid

Screenshot 2025-06-10 at 5 52 52 PM

Presumably this is correct behavior, since you don't already have a token.

Using the debugger flow first

Metadata shows mcp:tools scope in scopes_supported

Screenshot 2025-06-10 at 5 56 05 PM

Received token has mcp:tools scope

Screenshot 2025-06-10 at 5 57 38 PM

Clicking connect successful but authorize call isn't made

Screenshot 2025-06-10 at 5 58 55 PM

None of the calls made after clicking connect have the scope query param, and authorize is not among the calls.

Without the PR

Using debugger, authorize call has scope param

Screenshot 2025-06-10 at 6 14 18 PM

Clicking connect is successful

Screenshot 2025-06-10 at 6 15 57 PM

Conclusion

I don't see a difference in the behavior with or without this PR. I can't say for certain if the server is different from yours or if I'm missing some subtlety.

cliffhall avatar Jun 10 '25 22:06 cliffhall