Hugging Face Auth Integration - #4385
Conversation
This comment has been minimized.
This comment has been minimized.
|
Note to maintainers: this also needs to be opened against main once merged to release/3.x |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 783dd3db08
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| upstream_authorization_endpoint=HUGGINGFACE_AUTHORIZATION_ENDPOINT, | ||
| upstream_token_endpoint=HUGGINGFACE_TOKEN_ENDPOINT, | ||
| upstream_client_id=client_id, | ||
| upstream_client_secret=client_secret, |
There was a problem hiding this comment.
Treat empty client secrets as omitted
When client_secret is an empty string (for example from env-based config), this still passes a non-None secret into OAuthProxy, so its jwt_signing_key requirement for public clients is skipped, while the token auth method below treats the same value as no secret. That lets a public-client deployment start with FastMCP JWT and storage keys derived from "" instead of forcing an explicit signing key; normalize empty strings to None before calling super().__init__.
Useful? React with 👍 / 👎.
* Add OAuthProxy issuer response parameter * Cover OAuthProxy issuer error redirects * Relax host origin guard defaults (#4439) * Use exact issuer in authorize errors * Restore HTTP host guard compatibility (#4472) * Hugging Face Auth Integration (#4385) Co-authored-by: Jeremiah Lowin <153965+jlowin@users.noreply.github.com> * Docs: add v3.4.4 changelog entries (#4473) * Explain unnormalized issuer; cover consent-denial path base_url * Revert "Merge remote-tracking branch 'origin/release/3.x' into codex/oauth-proxy-rfc9207-issuer" This reverts commit 9e34b16, reversing changes made to 640dc60. * Preserve callback query bytes when appending iss/code/state params add_query_params previously decoded the existing query with parse_qsl and re-encoded it, mutating opaque or signed query strings (a valueless ?flag became ?flag=, non-UTF-8 percent-encoded bytes got replaced). Append the newly-encoded params to the existing query string instead of round-tripping it through parse/encode. Also fixes a stray bare `httpx` reference in a test that should use httpx2 following the SDK v2 migration. * Attach RFC 9207 iss to authorize() success redirects too AuthorizationHandler only added iss to error redirects from the SDK's base handler, not to code redirects returned directly by authorize() overrides that bypass consent/upstream (as GitHub's mocked test does). Since metadata now unconditionally advertises authorization_response_iss_parameter_supported, any client-facing redirect missing iss hard-fails RFC 9207-aware clients. Also fixes HeadlessOAuth, which parsed code/state from the redirect but silently dropped iss, so the same regression would have masked itself across every other provider integration test too. * Carry RFC 9207 iss through the production OAuth callback path OAuthProxy advertises authorization_response_iss_parameter_supported and sends iss on every authorization redirect, but the client's production callback chain (CallbackResponse -> OAuthCallbackResult -> OAuth.callback_handler) had no iss field, so it was silently dropped and the SDK's validate_authorization_response_iss rejected the callback. HeadlessOAuth already carried iss through, which is why CI stayed green while real clients failed. Add iss to CallbackResponse and OAuthCallbackResult, thread it through store_result_once for both success and error branches, and pass it into AuthorizationCodeResult in OAuth.callback_handler. * Don't duplicate iss when a provider redirect already carries one * Consolidate RFC 9207 iss handling into a single redirect helper Every client-facing authorization redirect must carry exactly one iss. That invariant was being enforced by hand at five separate call sites, each building its own params dict -- which is how the success-redirect path shipped without iss in the first place, and how a registered redirect_uri that already carries its own iss could end up duplicated. Route all five sites through build_client_redirect(), which owns the idempotent replace-or-append behavior so no caller can get it wrong. --------- Co-authored-by: shaun smith <1936278+evalstate@users.noreply.github.com>
Description
Add Hugging Face auth integration, example and docs. Hugging Face OAuth supports public and private apps, with PKCE, DCR and CIMD.
Closes #4384
Contribution type
Checklist
uv run prek run --all-filesand all checks pass