fix: validate_scope allows any scope when client has no registered scope restriction#2403
Open
enjoykumawat wants to merge 7 commits intomodelcontextprotocol:mainfrom
Open
fix: validate_scope allows any scope when client has no registered scope restriction#2403enjoykumawat wants to merge 7 commits intomodelcontextprotocol:mainfrom
enjoykumawat wants to merge 7 commits intomodelcontextprotocol:mainfrom
Conversation
…ope restriction When OAuthClientMetadata.scope is None it means the client was registered without any scope restriction. The previous code built allowed_scopes as an empty list in that case, causing every requested scope to raise InvalidScopeError — the opposite of the intended behavior. Fix: return requested_scopes immediately when self.scope is None, bypassing the per-scope membership check. Also remove the now-unreachable pragma comments that suppressed coverage for the success path of the loop. Github-Issue: modelcontextprotocol#2216
Kludex
reviewed
Apr 8, 2026
src/mcp/shared/auth.py
Outdated
Comment on lines
+77
to
+79
| allowed_scopes = self.scope.split(" ") | ||
| for scope in requested_scopes: | ||
| if scope not in allowed_scopes: # pragma: no branch | ||
| if scope not in allowed_scopes: |
Member
There was a problem hiding this comment.
Since you are here, can we do this?
requested = set(requested_scopes)
allowed = set(self.scope.split())
if not requested.issubset(allowed):Replace the for-loop with set.issubset() for clarity and to report all invalid scopes at once rather than one per loop iteration. Github-Issue: modelcontextprotocol#2216
Collapses the multi-line raise into a single line now that the message fits within the line length limit.
When the SSE stream ends a second time without a response, the reconnect attempt counter was incorrectly reset to 0, causing infinite reconnect loops instead of properly backing off. Pass `attempt + 1` so the counter advances correctly.
Author
|
Refactored to use set operations as suggested — cleaner and avoids the explicit loop. Thanks for the tip! |
Author
|
Done! Updated to use requested = set(requested_scopes)
allowed = set(self.scope.split())
if not requested.issubset(allowed):
raise InvalidScopeError(...)Cleaner and handles duplicates correctly. Thanks for the suggestion! |
Kludex
reviewed
Apr 8, 2026
| # Stream ended again without response - reconnect again | ||
| logger.info("SSE stream disconnected, reconnecting...") | ||
| await self._handle_reconnection(ctx, reconnect_last_event_id, reconnect_retry_ms, 0) | ||
| await self._handle_reconnection(ctx, reconnect_last_event_id, reconnect_retry_ms, attempt + 1) |
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
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.
Problem
OAuthClientMetadata.validate_scopeincorrectly rejects all requested scopes when the client was registered without ascoperestriction (self.scope is None).The previous code built
allowed_scopes = []whenself.scope is None, so the membership checkscope not in allowed_scopeswas alwaysTrue, raisingInvalidScopeErrorfor every token request — even thoughNoneshould mean no restrictions.Fixes #2216
Fix
Return
requested_scopesimmediately whenself.scope is None, bypassing the per-scope membership check. This matches the intended semantics: a client registered without scope restrictions may request any scope.Also removed the
# pragma: no branchand# pragma: no covercomments that were suppressing coverage for code paths that are now reachable with correct behavior.Tests
Added four regression tests to
tests/shared/test_auth.py:validate_scopereturnsNonewhen no scope is requestedInvalidScopeError