mcp-go icon indicating copy to clipboard operation
mcp-go copied to clipboard

chore(oauth): OAuth handler to support functional configuration options

Open appleboy opened this issue 6 months ago • 3 comments

Description

  • Refactored the NewOAuthHandler function to use the functional options pattern, making configuration more flexible and extensible.
  • Added support for injecting a custom HTTP client through the WithOAuthHTTPClient option, allowing advanced control over authentication requests (e.g., timeout, proxy).

fix https://github.com/mark3labs/mcp-go/issues/440

Motivation

  • This change addresses issue #440 by enabling developers to customize the OAuth Handler more easily and improve testability. The previous implementation did not allow for easy HTTP client customization, which limited its flexibility.

Changes

  • NewOAuthHandler now accepts functional options.
  • Introduced WithOAuthHTTPClient for injecting a custom http.Client.
  • Removed the hardcoded HTTP client instantiation to enhance flexibility and testability.

Testing

  • Manually verified that the OAuth flow works as expected with both the default and custom HTTP clients.
  • Automated tests have not been added yet—feedback on specific test cases is welcome.

Notes

  • This is a non-breaking change and should not affect existing usage.
  • If you have additional customization needs or use cases, please share them.

Type of Change

  • [X] Bug fix (non-breaking change that fixes an issue)
  • [ ] New feature (non-breaking change that adds functionality)
  • [ ] MCP spec compatibility implementation
  • [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [ ] Documentation update
  • [ ] Code refactoring (no functional changes)
  • [ ] Performance improvement
  • [ ] Tests only (no functional changes)
  • [ ] Other (please describe):

Checklist

  • [X] My code follows the code style of this project
  • [X] I have performed a self-review of my own code
  • [ ] I have added tests that prove my fix is effective or that my feature works
  • [ ] I have updated the documentation accordingly

MCP Spec Compliance

  • [ ] This PR implements a feature defined in the MCP specification
  • [ ] Link to relevant spec section: Link text
  • [ ] Implementation follows the specification exactly

Additional Information

Summary by CodeRabbit

  • New Features

    • Added support for customizing the OAuth HTTP client, allowing injection of a custom HTTP client for authentication requests.
    • OAuth handler construction now accepts optional initialization options for more flexible setup.
  • Tests

    • Added tests verifying custom HTTP client behavior, fallback to defaults, and request handling with custom clients.

appleboy avatar Jun 24 '25 10:06 appleboy

[!WARNING]

Rate limit exceeded

@appleboy has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 14 minutes and 2 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 7e90843f66b62061bfe4a0b505ff890864d7bec7 and de9ea9ecd075d8ac53a38ab89b673210b7bdbfc0.

📒 Files selected for processing (2)
  • client/transport/oauth.go (1 hunks)
  • client/transport/oauth_test.go (1 hunks)

Walkthrough

New functional options support was added to OAuthHandler: an OAuthHandlerOption type and WithOAuthHTTPClient allow injecting a custom http.Client. NewOAuthHandler signature changed to accept variadic options and applies them; a default 30s client remains unless overridden. (39 words)

Changes

Cohort / File(s) Change Summary
OAuth handler construction
client/transport/oauth.go
Added OAuthHandlerOption type and WithOAuthHTTPClient option; changed NewOAuthHandler(config OAuthConfig)NewOAuthHandler(config OAuthConfig, opts ...OAuthHandlerOption) and apply options to the constructed OAuthHandler, allowing a custom http.Client (default 30s kept if none provided).
Tests for HTTP client option
client/transport/oauth_test.go
Added TestWithOAuthHTTPClient with subtests validating setting a custom HTTP client, ignoring nil client, preserving custom Transport, using the provided client for metadata fetch, and falling back to default client when not provided.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The description provides detailed sections but does not follow the required template’s “Fixes #<issue_number>” syntax and leaves the Additional Information section with only placeholder comments instead of removing or populating it. Please update the Description section to use “Fixes #440” per the template and remove or fill the Additional Information section to align with repository guidelines.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly summarizes the main change by indicating that the OAuth handler now supports functional configuration options and accurately reflects the scope of the PR without extraneous details.
Linked Issues Check ✅ Passed The PR implements the requested direct override for the HTTP client by introducing WithOAuthHTTPClient and updating NewOAuthHandler to accept it, fully satisfying the issue’s requirement to support custom http.Client injection.
Out of Scope Changes Check ✅ Passed All changes are scoped to the OAuth handler and its tests to support the functional options pattern and custom HTTP client injection, with no unrelated modifications present.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Jun 24 '25 10:06 coderabbitai[bot]

@ezynda3 , any feedback?

appleboy avatar Jul 12 '25 01:07 appleboy

ping @ezynda3

appleboy avatar Oct 09 '25 02:10 appleboy