Add server-side identity assertion (SEP-990 ID-JAG) - #4483
Conversation
marvin-context-protocol
Bot
added
auth
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3589350560
ℹ️ 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".
| # client-controlled `params.scopes`. | ||
| granted_scopes = _assertion_granted_scopes(claims) | ||
| if not granted_scopes: | ||
| granted_scopes = list(params.scopes or []) |
There was a problem hiding this comment.
When an ID-JAG omits scope/scp, this falls back to the token request's scope parameter, which is entirely client-controlled and not covered by the signed assertion. In deployments that use scopes for tool/resource authorization, a client with a valid identity assertion can request scope=admin (or any other scope) and have it embedded in the FastMCP JWT, satisfying downstream scope checks even though the IdP did not grant that permission. The issued scopes should come from the assertion or explicit server policy, or the request scopes need to be strictly validated against such policy.
Useful? React with 👍 / 👎.
| tokens = await self.provider.exchange_identity_assertion( | ||
| client_info, params | ||
| ) |
There was a problem hiding this comment.
This manual ID-JAG dispatch bypasses the SDK token handler's grant_type not in client_info.grant_types check. With identity assertion enabled, a client registered only for the default authorization_code/refresh_token grants can still post the jwt-bearer grant and reach exchange_identity_assertion, so the client's registered grant types no longer constrain the token endpoint. Please reject the request unless JWT_BEARER_GRANT_TYPE is present in client_info.grant_types before exchanging the assertion.
Useful? React with 👍 / 👎.
Scopes for the issued token now derive only from the signed assertion (or server policy when it omits scopes); the client-supplied request scope may narrow but never widen them. Enforce the registered grant-type constraint the SDK check bypassed, and have proxy DCR add the jwt-bearer grant to registered clients when identity assertion is enabled.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd64aae03c
ℹ️ 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".
| # 3. Verify signature, iss, aud, and exp via JWTVerifier. | ||
| verifier = await self._get_verifier(iss) | ||
| access_token = await verifier.load_access_token(assertion) |
There was a problem hiding this comment.
Reject a signed assertion when its optional nbf claim is later than the current time. JWTVerifier.load_access_token() only checks exp, issuer, audience, signature, and scopes, and this additional validation checks iat but never nbf, so an ID-JAG that the IdP has explicitly marked as not yet valid is accepted immediately and exchanged for an access token.
Useful? React with 👍 / 👎.
| unverified_claims = _decode_unverified_claims(assertion) | ||
| except (ValueError, KeyError, IndexError) as e: | ||
| raise IdentityAssertionError(f"Malformed assertion payload: {e}") from e | ||
| iss = unverified_claims.get("iss") |
There was a problem hiding this comment.
Handle a decoded JWT payload that is valid JSON but not an object. For example, a request with the required typ header and a payload of [] makes _decode_unverified_claims() return a list, after which this .get() raises AttributeError outside the caught validation errors; because this happens before signature verification, any client reaching the enabled token endpoint can turn malformed assertions into HTTP 500 responses instead of invalid_grant.
Useful? React with 👍 / 👎.
| # "https://server.com" matches a base URL rendered as "https://server.com/". | ||
| self.audience = config.audience or audience.rstrip("/") | ||
|
|
||
| self._jti_cache: dict[str, float] = {} |
There was a problem hiding this comment.
Store consumed JTIs in shared storage rather than this validator-local dictionary. In a deployment with multiple workers or replicas, each OAuthProxy instance has an independent cache, so the same signed assertion can be successfully exchanged once per instance depending on load-balancer routing, defeating the advertised replay rejection in normal horizontally scaled production environments.
Useful? React with 👍 / 👎.
| if len(self._jti_cache) > self._jti_cache_max_size: | ||
| self._cleanup_expired_jtis() | ||
| if len(self._jti_cache) > self._jti_cache_max_size: | ||
| logger.warning("ID-JAG jti cache at capacity, possible attack") | ||
| raise IdentityAssertionError("Server overloaded, please retry") |
There was a problem hiding this comment.
Check capacity before inserting, or remove the newly inserted JTI when rejecting it. Once 10,000 still-valid assertions are cached, the next assertion is inserted and then rejected, but its entry remains; every subsequent valid assertion repeats this and grows the dictionary despite receiving an overload error, allowing a user capable of obtaining fresh signed assertions to cause sustained memory growth until entries expire.
Useful? React with 👍 / 👎.
| if params.scopes: | ||
| requested = set(params.scopes) | ||
| granted_scopes = [s for s in authoritative_scopes if s in requested] |
There was a problem hiding this comment.
Prevent the request scope from removing configured required_scopes. With required_scopes=["admin"], an assertion granting admin read passes validation, but a request containing scope=read reaches this intersection and receives a token without admin, contradicting the configuration contract that these scopes must be present on the issued access token; reject such a request or preserve the mandatory scopes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9e7603717
ℹ️ 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".
| except (ValueError, KeyError, IndexError) as e: | ||
| raise IdentityAssertionError(f"Malformed assertion header: {e}") from e | ||
| if header.get("typ") != ID_JAG_TYP: | ||
| raise IdentityAssertionError( |
There was a problem hiding this comment.
When an assertion's header is valid JSON but not an object, such as [], decode_jwt_header() returns that value and the subsequent header.get(...) raises AttributeError outside the caught exceptions. Any client reaching the enabled token endpoint can therefore turn an unsigned malformed assertion into an HTTP 500 instead of the expected invalid_grant; validate that header is a dictionary before accessing it.
Useful? React with 👍 / 👎.
| access_token = self.jwt_issuer.issue_access_token( | ||
| client_id=client.client_id or "", | ||
| scopes=granted_scopes, | ||
| jti=access_jti, |
There was a problem hiding this comment.
When an ID-JAG token request supplies resource, params.resource is never inspected and this always issues a token for the proxy's configured resource. A request targeting another server—or another tenant when the resource URL contains a tenant query parameter—therefore succeeds with a token for this server instead of returning invalid_target; this bypasses the resource-mismatch invariant already enforced by authorize() in this file at lines 975–1011.
Useful? React with 👍 / 👎.
| verifier = _JWTVerifier( | ||
| jwks_uri=jwks_uri, | ||
| issuer=issuer, | ||
| audience=self.audience, | ||
| ) |
There was a problem hiding this comment.
When a trusted IdP signs ID-JAGs with ES256, PS256, or another supported asymmetric algorithm, this verifier is constructed without an algorithm, so JWTVerifier silently defaults to RS256 and rejects every assertion even when the configured JWKS contains the correct key. Since IdentityAssertion exposes no signing-algorithm setting, such issuers cannot use the feature; pass a configured algorithm or derive and validate an allowed algorithm policy.
Useful? React with 👍 / 👎.
…hm config - Honor RFC 8707 resource on the jwt-bearer grant (invalid_target on mismatch), mirroring authorize()'s invariant incl. skip-when-unconfigured - Reject JSON-array JOSE headers with invalid_grant instead of a 500 - Add IdentityAssertion.algorithm so ES256/PS256 issuers can be verified (JWTVerifier otherwise defaults to RS256)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68acafaa6d
ℹ️ 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".
| logger.info("ID-JAG rejected: %s", e) | ||
| raise TokenError("invalid_grant", "Invalid identity assertion") from e | ||
|
|
||
| subject = str(claims["sub"]) |
There was a problem hiding this comment.
When an IdP issues an ID-JAG for client A, registered client B can present it because the validated claims are never required to contain a matching signed client_id, and this path immediately mints the token for the presenting client.client_id. The exchange_identity_assertion contract in the MCP SDK explicitly requires the assertion's client_id to match the authenticated client; without that binding, a leaked or intercepted assertion crosses client boundaries, especially because proxy DCR clients are public. Reject assertions with a missing or mismatched client_id before issuing the token.
Useful? React with 👍 / 👎.
| access_token = self.jwt_issuer.issue_access_token( | ||
| client_id=client.client_id or "", | ||
| scopes=granted_scopes, | ||
| jti=access_jti, | ||
| expires_in=expires_in, |
There was a problem hiding this comment.
When one authorization-server issuer is used for multiple MCP resources, an assertion whose signed resource names server A is accepted by server B: only the client-controlled params.resource is checked, claims["resource"] is ignored, and this call issues a token with B's configured audience. The MCP SDK's ID-JAG provider contract specifically requires the issued token to be restricted using the signed assertion resource rather than merely the request indicator, so require and validate that claim against this proxy's resource before issuance.
Useful? React with 👍 / 👎.
SEP-990: the IdP signs which client and which resource the assertion was minted for. With public proxy clients the presented client_id is self-asserted, so the signed binding is what stops client B redeeming client A's leaked assertion — and the signed resource claim stops an assertion for server A being redeemed at server B behind the same IdP.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5bc23d73bd
ℹ️ 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".
| if ( | ||
| self._identity_assertion is not None | ||
| and JWT_BEARER_GRANT_TYPE not in registered_grant_types | ||
| ): | ||
| registered_grant_types.append(JWT_BEARER_GRANT_TYPE) |
There was a problem hiding this comment.
When identity assertion is enabled, this silently grants the JWT-bearer flow to every dynamically registered client; the client is then stored immediately below with token_endpoint_auth_method="none" and no secret. SEP-990 §5.1 and the MCP SDK's ID-JAG handler require an out-of-band provisioned confidential client, so an ordinary self-registered, unauthenticated client can reach the assertion-exchange hook, bypassing the client-authentication half of the flow. Keep this grant out of DCR and require stored confidential credentials before dispatching it.
Useful? React with 👍 / 👎.
| if len(self._jti_cache) >= self._jti_cache_max_size: | ||
| logger.warning("ID-JAG jti cache at capacity, possible attack") | ||
| raise IdentityAssertionError("Server overloaded, please retry") | ||
| self._jti_cache[jti] = exp |
There was a problem hiding this comment.
A cryptographically valid assertion is marked consumed here before exchange_identity_assertion() checks its signed client_id and resource. If someone possessing a copied ID-JAG submits it using a different public client ID or to the wrong resource first, that request is rejected but the intended client subsequently receives a replay error, allowing the assertion to be burned without ever issuing a token. Delay committing the JTI until the proxy-level bindings have passed, or include those checks in the validator before this insertion.
Useful? React with 👍 / 👎.
| f"OIDC discovery for issuer {issuer!r} failed: {e}" | ||
| ) from e | ||
|
|
||
| jwks_uri = body.get("jwks_uri") |
There was a problem hiding this comment.
When a configured issuer's discovery endpoint returns successful, valid JSON that is not an object—for example [] or a JSON error string—response.json() succeeds but this .get() raises an uncaught AttributeError. The first token exchange then becomes an HTTP 500 instead of the expected invalid_grant, and every request continues failing until the issuer response changes; validate that body is a mapping or translate this failure to IdentityAssertionError.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 027569d796
ℹ️ 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".
| verifier = _JWTVerifier( | ||
| jwks_uri=jwks_uri, | ||
| issuer=issuer, | ||
| audience=self.audience, | ||
| algorithm=self.config.algorithm, |
There was a problem hiding this comment.
When IdentityAssertion.algorithm is unsupported (for example, EdDSA) or incompatible with JWKS verification (such as an HS* algorithm), configuration succeeds and the first exchange raises ValueError while constructing JWTVerifier. That exception is not translated to IdentityAssertionError, so every token request returns HTTP 500 rather than rejecting the configuration at startup or returning a controlled OAuth error.
Useful? React with 👍 / 👎.
| if nbf is not None and nbf > now + self.CLOCK_SKEW_SECONDS: | ||
| raise IdentityAssertionError("Assertion is not yet valid (nbf in future)") | ||
| if iat is not None: | ||
| if iat > now + self.CLOCK_SKEW_SECONDS: | ||
| raise IdentityAssertionError("Assertion iat is in the future") |
There was a problem hiding this comment.
When a correctly signed assertion from a trusted issuer contains a malformed string or object for nbf or iat, signature verification succeeds but these comparisons raise TypeError, which escapes the validator and turns the token exchange into HTTP 500. Validate the NumericDate claim types and raise IdentityAssertionError so malformed assertions consistently produce invalid_grant.
Useful? React with 👍 / 👎.
…e temporal claims, algorithm, and discovery body - Move the client_id/resource binding checks into the validator itself, before jti is recorded as consumed, so an assertion presented with the wrong binding is rejected without burning replay protection for whoever it actually belongs to - Reject non-numeric exp/iat/nbf with invalid_grant instead of a 500 - Validate IdentityAssertion.algorithm at config time (must be an asymmetric JWS algorithm verifiable via JWKS) - Reject a non-object OIDC discovery body with invalid_grant instead of a 500 - Centralize the resource-URL comparison helpers used by both the validator and OAuthProxy.authorize()
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f565c28731
ℹ️ 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".
| # SEP-990: ID-JAG tokens are self-contained — the asserted subject | ||
| # is carried in the token itself, and there is no upstream token to | ||
| # swap for. Return directly from the verified claims. | ||
| if payload.get("fastmcp_grant") == _ID_JAG_GRANT_MARKER: |
There was a problem hiding this comment.
When upstream_revocation_endpoint is configured, the proxy advertises a revocation endpoint, but revoking an ID-JAG access token cannot invalidate it: load_access_token() always accepts the signed self-contained token here, while revoke_token() only deletes refresh-token metadata and forwards the FastMCP token to the upstream provider, which did not issue it. The revocation call therefore returns success but the token remains usable until expiry, including when access_token_expiry_seconds is configured above the five-minute default; record revoked access-token JTIs and reject them in this branch.
Useful? React with 👍 / 👎.
| jti = claims.get("jti") | ||
| if not jti: | ||
| raise IdentityAssertionError("Assertion must include jti claim") | ||
| cached_exp = self._jti_cache.get(jti) |
There was a problem hiding this comment.
If a correctly signed assertion from a trusted issuer contains an array or object for jti, this dictionary lookup raises an uncaught TypeError because those values are unhashable. The resulting token request returns HTTP 500 rather than the controlled invalid_grant response used for malformed assertions; validate that jti is a non-empty string before using it as a cache key.
Useful? React with 👍 / 👎.
| if v is not None and not v.startswith(("RS", "PS", "ES")): | ||
| raise ValueError( | ||
| f"Unsupported algorithm {v!r} for identity assertion: trusted " | ||
| "issuers are verified via JWKS, so algorithm must be an " | ||
| "asymmetric JWS algorithm (RS*, PS*, or ES*), e.g. 'ES256'" | ||
| ) |
There was a problem hiding this comment.
The current prefix-based validation still accepts unsupported values such as RS999, PS999, and ES999; fresh evidence in the current code is that JWTVerifier only accepts the explicit 256/384/512 variants and raises ValueError for these values. Because verifier construction happens during the first exchange and that exception is not translated to IdentityAssertionError, such accepted configuration makes every ID-JAG request return HTTP 500 instead of failing at startup.
Useful? React with 👍 / 👎.
- Migrate identity assertion + tests to httpx2 and the local httpx2_mock (legacy httpx is now banned; pytest-httpx no longer intercepts) - Add is_optional to the shared httpx2_mock, mirroring pytest-httpx - Tighten the algorithm allowlist to JWTVerifier's exact supported set (prefix check accepted typos like RS999 -> 500 on first exchange) - Reject non-string jti before the cache lookup (unhashable -> 500) - Track revocation for self-contained ID-JAG tokens: revoke_token records the jti and load_access_token rejects it until natural expiry - Dedupe resource-URL helpers: proxy now imports the shared normalize_resource_url/server_url_has_query from identity_assertion
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 63acb77d8d
ℹ️ 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".
| "private_key_jwt", | ||
| "none", | ||
| ] | ||
| if self._cimd_manager is not None: |
There was a problem hiding this comment.
When identity assertion is enabled with enable_cimd=False, this condition skips adding none to token_endpoint_auth_methods_supported, although register_client() stores every DCR client with token_endpoint_auth_method="none". The resulting metadata advertises the JWT-bearer grant but only lists client_secret_post and client_secret_basic, so clients that choose authentication from discovery cannot select the only method accepted for these registrations; add none whenever ID-JAG/public proxy clients are supported and keep only private_key_jwt conditional on CIMD.
Useful? React with 👍 / 👎.
…thout CIMD DCR clients are public, so metadata consumers must see 'none' to use the advertised jwt-bearer grant; previously only the CIMD path added it.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e6d6da9e3
ℹ️ 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".
| restrict_tag, | ||
| run_auth_checks, | ||
| ) | ||
| from .identity_assertion import IdentityAssertion |
There was a problem hiding this comment.
Importing this specialized feature eagerly makes every fastmcp.server.auth import load identity_assertion and its httpx2 dependency, bypassing the lazy-import boundary documented immediately below. This directly conflicts with the repository's AGENTS.md rule that package initializers must not import heavy or specialized integrations; preserve the public API through TYPE_CHECKING plus __getattr__, as is already done for OAuthProxy and other optional auth features.
Useful? React with 👍 / 👎.
| jwks_uri = (self.config.jwks_uris or {}).get(issuer) | ||
| if not jwks_uri: | ||
| jwks_uri = await self._discover_jwks_uri(issuer) |
There was a problem hiding this comment.
When an enabled issuer has no explicit JWKS URI, a client can submit an assertion with the trusted iss value and trigger discovery before any signature verification. Until the first discovery succeeds—and indefinitely while that endpoint is unavailable—every request starts a separate HTTP call with a 10-second timeout because failures and in-flight lookups are not cached; concurrent malformed requests can therefore exhaust token-endpoint workers and hammer the configured IdP. Cache discovery failures with backoff or coalesce concurrent discovery per issuer.
Useful? React with 👍 / 👎.
SEP-990 identity assertion leads: the SDK provides the wire contract and provider hook; FastMCP provides the complete server-side implementation. Inventories the full modern-era capability set for v4.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea70c1c49b
ℹ️ 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".
| self.config = config | ||
| # Normalize away any trailing slash so an ID-JAG `aud` of | ||
| # "https://server.com" matches a base URL rendered as "https://server.com/". | ||
| self.audience = config.audience or audience.rstrip("/") |
There was a problem hiding this comment.
When base_url has no path, AnyHttpUrl renders it with a trailing slash and the metadata endpoint advertises that exact value as issuer (for example, https://myserver.com/), but this strips the slash before configuring JWTVerifier. An IdP that correctly uses the advertised issuer as the ID-JAG aud will therefore have every assertion rejected for an audience mismatch unless the operator manually duplicates the advertised value in IdentityAssertion.audience; normalize both sides or retain the advertised issuer unchanged.
Useful? React with 👍 / 👎.
| verifier = _JWTVerifier( | ||
| jwks_uri=jwks_uri, | ||
| issuer=issuer, | ||
| audience=self.audience, | ||
| algorithm=self.config.algorithm, | ||
| ) |
There was a problem hiding this comment.
When trusted_issuers contains IdPs using different signing algorithms, every issuer-specific verifier receives the same single config.algorithm; for example, configuring ES256 for one issuer makes an RS256 issuer unusable, while leaving the default makes the ES256 issuer unusable. Since the API explicitly supports multiple issuers and already configures JWKS URIs per issuer, the algorithm policy also needs to be per issuer or an allowed set that can safely accommodate all configured issuers.
Useful? React with 👍 / 👎.
… algorithms, discovery backoff
- IdentityAssertion re-exported lazily from server.auth (the eager import
bypassed the package's documented lazy-import boundary)
- Accept the ID-JAG aud both with and without base_url's trailing slash;
metadata advertises the slashed form, so IdPs echoing it verbatim work
- algorithms={issuer: alg} per-issuer override, mirroring jwks_uris
- OIDC discovery serializes per-issuer and backs off 30s after a failure
(discovery runs pre-signature, so garbage could amplify into HTTP floods)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0b8d3e565
ℹ️ 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".
| if isinstance(value, bool) or not isinstance(value, (int, float)): | ||
| raise IdentityAssertionError(f"Assertion {name} claim must be a number") | ||
| return float(value) |
There was a problem hiding this comment.
A correctly signed assertion containing NaN for exp, iat, or nbf passes this numeric check because Python treats NaN as a float. In particular, exp=NaN bypasses the verifier's expiry and lifetime comparisons, then stores NaN in the JTI cache; since NaN > now is always false, the same assertion can be exchanged repeatedly and its cache entry never expires. Require math.isfinite(value) in addition to the current type check.
Useful? React with 👍 / 👎.
| lock = self._discovery_locks.setdefault(issuer, asyncio.Lock()) | ||
| async with lock: | ||
| failed_at = self._discovery_failures.get(issuer) | ||
| if ( | ||
| failed_at is not None | ||
| and time.monotonic() - failed_at < self._discovery_failure_cooldown | ||
| ): | ||
| raise IdentityAssertionError( | ||
| f"OIDC discovery for issuer {issuer!r} recently failed; backing off" | ||
| ) | ||
| return await self._fetch_discovery(issuer) |
There was a problem hiding this comment.
When multiple first exchanges for one issuer arrive concurrently, every caller observes the verifier as absent before entering this method and queues on the lock. Fresh evidence after the discovery-hardening change is that each queued caller unconditionally executes _fetch_discovery() after acquiring the lock, even though the first caller has already discovered the URI and cached a verifier outside the lock; a startup burst therefore serializes one outbound discovery request per exchange and can accumulate N times the discovery latency or timeout. Cache the discovered URI or recheck the verifier/discovery result after lock acquisition.
Useful? React with 👍 / 👎.
| # 4. sub is mandatory (RFC 7523 §3) — it identifies the end user. | ||
| sub = claims.get("sub") | ||
| if not sub: | ||
| raise IdentityAssertionError("Assertion must include sub claim") |
There was a problem hiding this comment.
A signed but malformed non-string sub such as 123 or true passes this truthiness check and is later coerced with str(claims["sub"]). That makes it indistinguishable in the FastMCP auth context from a legitimate string subject such as "123" or "True", potentially applying that user's subject-based authorization to the wrong identity when an issuer emits the wrong claim type. RFC JWT subjects are strings, so reject non-string or empty values here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 532471eabd
ℹ️ 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".
| if not isinstance(body, dict): | ||
| # Valid JSON that isn't an object (e.g. `[]` or a bare string) — | ||
| # guard before .get() so a misbehaving discovery endpoint maps to | ||
| # invalid_grant, not a 500 on every subsequent exchange. | ||
| raise IdentityAssertionError( | ||
| f"OIDC discovery document for issuer {issuer!r} is not a JSON object" |
There was a problem hiding this comment.
When a trusted issuer repeatedly returns valid JSON that is not an object, this branch raises without recording _discovery_failures, so every assertion triggers another outbound discovery request instead of observing the configured cooldown. Fresh evidence in the current code is that failure timestamps are set only inside the preceding HTTP/JSON exception handler; the same omission also affects object responses with a missing or non-string jwks_uri. Record these validation failures before raising so a misconfigured issuer cannot be hammered on every token request.
Useful? React with 👍 / 👎.
| if isinstance(scp, list): | ||
| return [str(s) for s in scp] |
There was a problem hiding this comment.
When a signed assertion contains a malformed scp array with integers, booleans, or objects, coercing every entry with str() can turn it into a configured required scope and ultimately grant that scope—for example, scp: [123] satisfies required_scopes=["123"]. JWT scope arrays must contain strings, so reject the assertion when any entry is not a non-empty string rather than converting values into authorization identifiers.
Useful? React with 👍 / 👎.
| from fastmcp import FastMCP | ||
| from fastmcp.server.auth import OAuthProxy, IdentityAssertion | ||
|
|
||
| auth = OAuthProxy( |
There was a problem hiding this comment.
This documentation block cannot run as written because OAuthProxy.__init__ requires the keyword-only token_verifier argument, but the example omits it; users following the new feature guide receive TypeError: missing required keyword-only argument: 'token_verifier' before identity assertion is configured. The repository's AGENTS.md also requires documentation code blocks to be fully runnable, so instantiate and pass an appropriate verifier in this example.
Useful? React with 👍 / 👎.
jlowin
added
the
feature
1 participant
Enterprises running FastMCP internally have had no way to let the corporate IdP decide access: every employee's agent had to walk a browser OAuth consent flow, and revocation lived in FastMCP's own client registrations rather than the IdP. This implements the server half of SEP-990 identity assertion (ID-JAG): the IdP issues a signed assertion of the employee's identity (
typ: oauth-id-jag+jwt), the agent presents it at the token endpoint via the RFC 7523jwt-bearergrant, and FastMCP validates it (trusted issuer, audience,typ, expiry,jtireplay) and mints a short-lived access token — no browser, no consent screen, no refresh token (clients re-exchange), revocation at the IdP.Nearly everything is reuse: signature/issuer/audience/expiry checks ride
JWTVerifier(with OIDC discovery of the issuer's JWKS), replay protection mirrors the CIMDjticache, and issuance goes through the existingJWTIssuer— so the asserted subject flows intoget_access_token()like any other token. AS metadata advertises thejwt-bearergrant andid-jagprofile only when configured; the grant isunsupported_grant_typeotherwise.OIDCProxyinherits the whole mechanism via a threaded constructor param.One deliberate divergence from the SDK reference, flagged for review: the SDK's token handler requires a confidential client (stored secret) before honoring the grant — but FastMCP's proxy DCR clients are always public (
token_endpoint_auth_method="none"), so under that rule no proxy client could ever use ID-JAG. FastMCP's handler dispatches the grant without the confidential precondition, on the reasoning that the validated assertion from a trusted issuer is the authoritative grant (the SDK's own docstring warns against deriving authorization fromclient_idalone). If we'd rather gate this harder (e.g. allow-listing client ids or requiring CIMD-attested clients), that's a small follow-up.Client-side wrapper (FastMCP-as-client presenting assertions) is a separate later phase, as agreed.
Label: features