Skip to content

Add state param to OAuth provider #529

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

Merged
merged 2 commits into from
May 22, 2025

Conversation

jescalan
Copy link
Contributor

Adds the ability for a state parameter to be provided by the OAuth client provider.

Motivation and Context

The current auth implementation does not allow for a state parameter to be passed into the authorize url. It's fairly common for OAuth implementations to mandate the inclusion of a state parameter for security purposes. For example, Ory, a popular OAuth server implementation, will reject requests without a state parameter. In order to maximize compatibility with OAuth servers, adding the option to include a state parameter I think would be useful.

In cases where the developer does not have the means to or does not desire to pass a state parameter, defining this is optional, so it will still work as previously if state is not present as a function on the OAuth client provider.

How Has This Been Tested?

I added passing tests for the state param being present and absent into the auth test suite. I also took this patch and applied it to a project I am working on that utilizes an OAuth server that requires the state parameter to be present, and verified that with this change, the state parameter was added and the OAuth flow was able to be completed.

Breaking Changes

This should not be breaking at all.

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

I have several more improvements to the auth handling code in my personal queue - I would love to establish a line of communication with whoever is maintaining this area of the codebase currently in order to be able to discuss proposed changed before submitting PRs to make sure that I'm directionally aligned. If anyone is willing to connect with me that would be wonderful! In the meantime, I'll continue working on getting PRs submitted 🚀

@ihrpr ihrpr added this to the HPR milestone May 22, 2025
Copy link
Contributor

@pcarleton pcarleton left a comment

Choose a reason for hiding this comment

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

Thanks for adding this!

For line of communication, I recommend the community working group discord, the Auth WG would be relevant:
https://discord.com/channels/1358869848138059966/1360835991749001368

More info on that discord / link to join is here: https://github.com/modelcontextprotocol-community/working-groups

@pcarleton pcarleton merged commit 0516f98 into modelcontextprotocol:main May 22, 2025
2 checks passed
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