Skip to content

Implement support for client_credentials and RFC 7523 #1020

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

Conversation

LucaButBoring
Copy link
Contributor

@LucaButBoring LucaButBoring commented Jun 24, 2025

Implements support for the client_credentials flow over client_secret_basic/client_secret_post. Requires the client to explicitly declare support for the client_credentials grant type, as demonstrated in the example code.

Motivation and Context

Enables machine-to-machine auth and using IdPs that don't support a code grant flow.

How Has This Been Tested?

Added an example that integrates with Discord using this flow. The example does not use Dynamic Client Registration because the OAuthProvider boilerplate that offers that has the authorization code grant flow baked into it right now. Instead, a dummy client ID and client secret are hardcoded in the TokenStorage implementation.

Breaking Changes

  • Possible breaking change? OAuthClientInformationFull.client_id is now optional as described in the OAuth 2.1 specification, section 3.2.2.

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

#709

@LucaButBoring LucaButBoring marked this pull request as ready for review June 25, 2025 20:30
@ihrpr ihrpr added this to the auth-spec milestone Jun 30, 2025
@SoldierSacha
Copy link

I also implemented this, along with the token exchange grant type in addition to client credentials

#882

@LucaButBoring LucaButBoring requested review from a team as code owners July 24, 2025 20:06
@LucaButBoring
Copy link
Contributor Author

@pcarleton @dsp-ant Let me know if I should close this in favor of #882, or if the credential grants should be PR'd separately (whether that looks like #882 being split or scoped down to token_exchange). There is also now an SEP that would benefit from either PR to satisfy the implementation example requirement.

@LucaButBoring LucaButBoring changed the title Implement support for client_credentials Implement support for client_credentials and RFC 7523 Jul 29, 2025
@LucaButBoring
Copy link
Contributor Author

Added a client implementation of RFC 7523 to support SEP-1046. I'll add a server implementation within the next few days.

yannj-fr and others added 3 commits August 1, 2025 09:12
@LucaButBoring
Copy link
Contributor Author

LucaButBoring commented Aug 1, 2025

Changes:

  1. @yannj-fr added support for the RFC 7523 S2.2 flow (client_credentials with an assertion)
  2. I've temporarily removed the example code as it was using Discord as an example, and as of Remove github from auth examples #1011 I believe we do not want to use external integrations as examples anymore

Unsure about if we should have a server implementation now, reviewing the last core maintainer meeting notes: modelcontextprotocol/modelcontextprotocol#1061

Authorization Server Implementation in SDKs Paul Carleton questioned whether SDKs should provide a fully functional authorization server, leaning towards not providing one due to maintenance overhead. Nick agreed, stating that client-side OAuth implementation is crucial, but implementing an OAuth server is less important as it is often handled by external technology. Den Delimarsky also cautioned against implementing an authorization server within the SDK due to security complexities and potential breach risks.

As a practical matter, that guidance complicates testing, however.

edit: Discussing alternatives on Discord.

@pcarleton pcarleton requested review from ochafik and removed request for pcarleton August 4, 2025 10:51
@ochafik
Copy link

ochafik commented Aug 5, 2025

Hi @LucaButBoring, thanks for your PR!

After chatting w/ the team it would seem indeed that something along the lines of #882 would reduce the maintenance complexity (changes encapsulated in httpx.Auth), although it's still TBD whether we do want to onboard these features (pending discussion / decision on modelcontextprotocol/modelcontextprotocol#1046).

Closing this for now (thanks again!), but do let me know if you feel strongly otherwise ✌️

@ochafik ochafik closed this Aug 5, 2025
@LucaButBoring
Copy link
Contributor Author

Sounds good, I believe in the future we want to start using authlib or similar for adding new OAuth support anyways, so a future iteration of this PR (JWT-specific) will likely be rewritten with that anyhow.

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.

5 participants