fix(oauth): handle rate limiting in resource auto-detection #277
+778
−85
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When autoDetectResource() encounters a 429 rate limit response, retry with appropriate backoff instead of immediately falling back to the server URL. ## Changes - Add parseRateLimitWait() helper to extract wait duration from: - Retry-After header (seconds or HTTP-date per RFC 7231) - JSON body with reset_at field (Unix timestamp) - Rewrite autoDetectResource() with retry loop: - On 429: wait per hints (capped at 30s), retry up to 3 times - On other 4xx/5xx: fall back to server URL immediately - On success (401): extract resource from WWW-Authenticate - Add comprehensive unit tests for rate limit handling ## Context Some OAuth providers (e.g., Runlayer/Anysource) aggressively rate limit preflight requests, causing resource auto-detection to fail during OAuth login flows.
Add rate limit injection capabilities to the OAuth test server for testing autoDetectResource() retry behavior with 429 responses. ## Changes - Add MCPRateLimitCount, MCPRateLimitRetryAfter, MCPRateLimitUseResetAt to ErrorMode - Add rate limit check to oauthMiddleware (before auth check) - Add /.well-known/oauth-protected-resource endpoint (RFC 9728) - Add ProtectedResourceMetadata type - Add MCPURL and ProtectedResourceMetadataURL to ServerResult - Add ResetRateLimitCounter() helper for tests - Add tests for rate limiting and protected resource metadata
Fix staticcheck QF1012 warnings by using fmt.Fprintf(w, ...) directly instead of w.Write([]byte(fmt.Sprintf(...))). This is more idiomatic and avoids allocating an intermediate byte slice. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add context cancellation support to autoDetectResource so rate limit retries can be interrupted during shutdown or timeout scenarios. Changes: - Add ctx parameter to autoDetectResource and CreateOAuthConfigWithExtraParams - Use http.NewRequestWithContext for context-aware HTTP requests - Replace blocking time.Sleep with context-aware select for rate limit waits - Add parseRateLimitWaitWithSource to track wait time source for logging - Add TestAutoDetectResource_ContextCancellation to verify cancellation works - Update all 5 call sites in connection.go to pass context - Update test files to pass context.Background() Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
👋 Hi, I'm an automated AI code review bot. I ran some checks on this PR and found 1 point that might be worth attention (could be false positives, please use your judgment):
If you find these suggestions disruptive, you can reply "stop" , and I'll automatically skip this repository in the future. |
Add io.LimitReader to cap response body reads at 1MB to prevent potential memory exhaustion from unexpectedly large responses. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
When
autoDetectResource()encounters a 429 rate limit response, retry with appropriate backoff instead of immediately falling back to the server URL.Related #271
Problem
Some OAuth providers (e.g., Runlayer/Anysphere) aggressively rate limit preflight requests, causing resource auto-detection to fail during OAuth login flows. This results in the
resourceparameter being set to the server URL instead of the correct value from Protected Resource Metadata, which can cause authentication failures.Solution
Rate Limit Handling (
internal/oauth/config.go)parseRateLimitWait()helper to extract wait duration from:Retry-Afterheader (seconds or HTTP-date per RFC 7231)reset_atfield (Unix timestamp)autoDetectResource()with retry loop:Test Infrastructure (
tests/oauthserver/)ErrorMode:MCPRateLimitCount- return 429 N times before real responseMCPRateLimitRetryAfter- Retry-After header valueMCPRateLimitUseResetAt- use JSON body with reset_at instead/.well-known/oauth-protected-resourceendpoint (RFC 9728)MCPURLandProtectedResourceMetadataURLtoServerResultResetRateLimitCounter()helper for test isolationTest plan
parseRateLimitWait()(7 test cases)autoDetectResource()rate limit handling (4 test cases)🤖 Generated with Claude Code