Skip to content

[auth]: support oauth client_secret_basic / none / custom methods #720

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 25 commits into
base: main
Choose a base branch
from

Conversation

ochafik
Copy link
Contributor

@ochafik ochafik commented Jul 1, 2025

This is an attempt to merge #531 and #552

Motivation and Context

This merges two sets of OAuth changes:

How Has This Been Tested?

WIP: testing in inspector

Breaking Changes

n/a

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

  • Main changes from McpError: MCP error -32601: Method not found #415:
    • Renamed callback to addClientAuthentication
    • Passed callback explicitly to refreshAuthorization & exchangeAuthorization, to avoid the awkwardness of them accepting a provider + other redundant parameters (codeVerifier, redirectUri)
jaredhanson and others added 19 commits May 21, 2025 19:14
… refreshAuthorization to maintain compatibility.
The applyBasicAuth function was incorrectly trying to set headers using array notation on a Headers object. Fixed by using the proper Headers.set() method instead of treating it as a plain object. This ensures that HTTP Basic authentication works correctly when client_secret_basic is the selected authentication method. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
@jaredhanson
Copy link

Thanks @ochafik ! This is looking pretty good. I'm experimenting with the branch right now, and have a suggested modification that I'll propose later tonight or tomorrow.

@jaredhanson
Copy link

Hi @ochafik - I opened #723, with my proposed changes. Feel free to pull commits from that branch into this one, if you find them acceptable. Thanks for taking up these changes - really looking forward to them landing in a release. Let me know if I can do anything else.

Copy link

@SightStudio SightStudio left a comment

Choose a reason for hiding this comment

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

I know my feedback came a bit late, but it looks good to me.
I don't have any major comments to add.
Thank you for reviewing it positively!

@ochafik ochafik marked this pull request as ready for review July 7, 2025 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants