refactor: extract OAuth helper functions and simplify provider state #1586
Conversation
maxisbey
force-pushed
the
maxisbey/oauth-helpers
branch
2 times, most recently
from
040a546 to
81c470f
Compare
maxisbey
changed the title
| raise OAuthRegistrationError(f"Invalid registration response: {e}") | ||
|
|
||
|
|
||
| async def handle_token_response_scopes( |
There was a problem hiding this comment.
based on the discussion, we’ll want to remove that check from the SDK
maxisbey
force-pushed
the
maxisbey/oauth-helpers
branch
from
6780800 to
fd612f2
Compare
pcarleton
left a comment
pcarleton
left a comment
There was a problem hiding this comment.
looks good, a few minor comments
| Validated OAuthToken model | ||
| Raises: | ||
| OAuthTokenError: If response JSON is invalid or contains unauthorized scopes |
There was a problem hiding this comment.
| OAuthTokenError: If response JSON is invalid or contains unauthorized scopes | |
| OAuthTokenError: If response JSON is invalid |
| return requested_path.startswith(configured_path) | ||
|
|
||
|
|
||
| def generate_pkce_parameters(verifier_length: int = 128) -> tuple[str, str]: |
There was a problem hiding this comment.
do we ever need to support different verifier lengths? seems like we can just always do 128, and cut out some complexity here
| # Generate code_verifier using unreserved characters per RFC 7636 Section 4.1 | ||
| # unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~" | ||
| code_verifier = "".join( | ||
| secrets.choice(string.ascii_letters + string.digits + "-._~") for _ in range(verifier_length) |
There was a problem hiding this comment.
optional: can use this:
code_verifier = secrets.token_urlsafe(96)
(borrowed from : https://github.com/RomeoDespres/pkce/blob/master/pkce/__init__.py#L40C5-L40C55)
| oauth_path = f"/.well-known/oauth-authorization-server{parsed.path.rstrip('/')}" | ||
| urls.append(urljoin(base_url, oauth_path)) | ||
|
|
||
| # OAuth root fallback |
There was a problem hiding this comment.
jfyi: we need to change this, but do it carefully (see modelcontextprotocol/typescript-sdk#1103) likely better in a follow-up, but wanted to flag as you're rolling this out.
tl;dr having this root-based above path-based OIDC means we'll get the root-level metadata when there's a path-based one we should use.
Refactors OAuth 2.0 authentication implementation to expose reusable utilities.
Motivation and Context
The OAuth authentication logic was tightly coupled to the httpx auth provider, making it difficult for external tools (like MCP proxies or custom clients) to reuse core OAuth functionality. This refactor extracts the OAuth primitives into standalone utilities that can be used independently.
Key improvements:
How Has This Been Tested?
Existing test suite updated to reflect new structure. All tests pass.
Breaking Changes
None - this is a pure refactor with no API changes.
Types of changes
Checklist
Additional context
New utilities added:
client/auth/utils.py: OAuth protocol helpers (discovery URLs, WWW-Authenticate parsing, scope selection, response handlers)shared/auth_utils.py: PKCE generation and token expiry calculationThe main OAuth2Auth provider (
client/auth/oauth2.py) now delegates to these utilities, reducing from 221 lines to 93 lines of core logic.