Skip to content

Conversation

dwreeves
Copy link

@dwreeves dwreeves commented Jun 18, 2025

Resolves #977

Motivation and Context

I am attempting to authorize to the ChatGPT web client using Okta OAuth. I got it working, but with significant rewrites and patching to the internals of the MCP Python SDK.

I'm not comfortable proposing support for everything I needed to do as many of those are client side issues with OpenAI's implementation, including lack of PKCE support. However, one of the things that tripped me up seems to be a minor issue on the MCP Python SDK's side, which is that the ChatGPT client was requesting &scope=. I'm not 100% sure what's going on as I give the client a default scope when it registers that it in turn is not requesting.

In any case, if None is a valid value for the scopes, then it seems that [] should also be, and furthermore that &scope= in the query params should correspond to either None or []. However, right now the MCP SDK only sets the scopes to None if the query parameter is not defined at all, whereas the ChatGPT web client sends &scope=, which parses as "", which in turn fails validation unless "" is defined as a valid scope in the ClientRegistrationOptions.

How Has This Been Tested?

I have a patched version of this running and connecting to ChatGPT's web client.

Breaking Changes

Should be no 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 (pre-commit run -a)
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed (n/a)

Additional context

@felixweinberger felixweinberger requested review from a team and ochafik September 17, 2025 16:33
Copy link
Contributor

@felixweinberger felixweinberger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @dwreeves thank you for this contribution! And apologies for the time it took to get back to you on this.

I'm assuming this is still a problem you're running into when using the OpenAI SDK? This change seems reasonable to me to handle idiosyncratic ways of requesting scopes.

One could debate whether &scope= should be interpreted as None or [], but given there are 2 different ways to declare it, it makes sense to me to have to different outcomes so I'm OK with this.

However, this change seems to break some existing tests:

ERROR at setup of TestProtectedResourceMetadata.test_state_parameter_validation_uses_constant_time 

Would you have bandwidth to take a look at that? Happy to come back to this quickly!

@felixweinberger felixweinberger added the needs more work Not ready to be merged yet, needs additional changes. label Sep 17, 2025
@dwreeves
Copy link
Author

dwreeves commented Sep 17, 2025

@felixweinberger I'm not actually if this is still an issue with OpenAI's implementation of OAuth-secured MCP! They've changed it quite a bit since I wrote this PR, including adding PKCE support.

We got it working exactly two times under very specific situations, but I gave up on it after too many issues, all of which seemed to be on their end (including random 500s) since we had no issues on Anthropic or running locally.

I still think it is a sensible behavior to not interpret scope= as "" but it is unclear to me today if it fixes any extant issues with OpenAI.

I think I can get this working on Friday and see why the PR is failing; I'm busy today and tomorrow.

@felixweinberger
Copy link
Contributor

@felixweinberger I'm not actually if this is still an issue with OpenAI's implementation of OAuth-secured MCP! They've changed it quite a bit since I wrote this PR, including adding PKCE support.

We got it working exactly two times under very specific situations, but I gave up on it after too many issues, all of which seemed to be on their end (including random 500s) since we had no issues on Anthropic or running locally.

I still think it is a sensible behavior to not interpret scope= as "" but it is unclear to me today if it fixes any extant issues with OpenAI.

I think I can get this working on Friday and see why the PR is failing; I'm busy today and tomorrow.

Gotcha that would be great thank you - agreed that it still makes sense to handle this case explicitly.

@dwreeves
Copy link
Author

Actually, my months of intermittent testing is all coming back to me. OpenAI does now support user scopes in their OAuth implementation, so they will return e.g. &scope=user and not just an empty param. So, this is definitely far less necessary.

@felixweinberger
Copy link
Contributor

Actually, my months of intermittent testing is all coming back to me. OpenAI does now support user scopes in their OAuth implementation, so they will return e.g. &scope=user and not just an empty param. So, this is definitely far less necessary.

Coming back to this, sounds like this is no longer a problem? To manage our inbound load I'm closing this one for now, but feel free to ping here if it's something you'd want to come back to.

Thanks again for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs more work Not ready to be merged yet, needs additional changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Empty but defined scope query param ("&scope=") validates as [""] and not [] during authorization.

2 participants