fix(auth): treat null optional fields in token responses as absent#2461
Draft
claude[bot] wants to merge 3 commits into
Draft
fix(auth): treat null optional fields in token responses as absent#2461claude[bot] wants to merge 3 commits into
claude[bot] wants to merge 3 commits into
Conversation
Some OAuth authorization servers serialize absent optional members as JSON null (e.g. "refresh_token": null). RFC 6749 does not sanction null values, but such servers exist in the wild, and OAuthTokensSchema previously rejected these responses (null string fields failed validation) or mishandled them (expires_in: null coerced to 0 via Number(null), yielding an instantly-expired token). This broke token exchange and refresh in exchangeAuthorization/refreshAuthorization. Normalize null-valued optional members (id_token, expires_in, scope, refresh_token) to absent in a preprocess step before validation. The inferred OAuthTokens output type is unchanged, and null members are truly absent from the parsed output (not present as undefined), so spreads that preserve a previous refresh_token keep working. Related: #754 (same null-emitting-server pattern in the registration response schema). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KXnNnp3fxQYR5HF9BUVhYP
🦋 Changeset detectedLatest commit: 1018d9e The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
…n OAuthTokensSchema
Rework of the previous commit after review: wrapping the exported
OAuthTokensSchema in z.preprocess changed its class (ZodObject ->
ZodEffects/ZodPipe), breaking downstream .shape/.extend users on the
stable 1.x line.
- Revert OAuthTokensSchema to its original plain object definition
(byte-identical to v1.x; class, .shape, .extend, z.input and emitted
d.ts verified unchanged on zod 3.25.x and 4.x).
- Add OAuthTokenResponseSchema, a z.preprocess wrapper that normalizes
JSON null values in optional members to absent before validation
(RFC 6749 §5.1 does not sanction nulls; expires_in: null must never
reach z.coerce.number(), which would coerce it to 0). Optional keys
are derived from the schema shape, not hardcoded, so future optional
members are covered automatically.
- Use the new schema at the SDK-owned parse sites: executeTokenRequest
(client) and ProxyOAuthServerProvider token exchange/refresh.
- Harden refreshAuthorization's refresh-token preservation:
{ ...tokens, refresh_token: tokens.refresh_token ?? refreshToken }
so preservation holds regardless of key presence.
- Tests: strict key-absence assertions, string expires_in coercion,
null access_token / missing token_type rejection, a regression test
pinning OAuthTokensSchema's class and strictness, a drift guard over
every optional shape member, and e2e exchangeAuthorization /
refreshAuthorization coverage (including refresh_token: null
preserving the original refresh token).
- Add the missing changeset (patch).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KXnNnp3fxQYR5HF9BUVhYP
Stripping a null scope from a token response makes it indistinguishable from an omitted scope, which RFC 6749 §5.1 defines as an assertion that the granted scope is identical to the requested scope. Document that consumers must not infer the granted scope from its absence and should use token introspection for the authoritative grant.
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
1 participant
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.
Requested by Den Delimarsky · Slack thread
Before: connecting to an MCP server whose authorization server returns
"refresh_token": null(or"scope": null/"id_token": null) fails with a Zod validation error ("expected string, received null") insideexchangeAuthorization/refreshAuthorization, before any tokens are saved. Worse,"expires_in": nullpassed validation but silently coerced to0(Number(null) === 0), producing a token the client treats as already expired.After: those responses are accepted, and the null-valued fields are treated exactly as if they had been omitted.
expires_in: nullparses asundefined, not0.How: the exported
OAuthTokensSchemais left completely untouched (still a plainZodObject; class,.shape,.extend,z.input, and the emitted.d.tsare identical tov1.x, verified on zod 3.25.x and 4.x). Null normalization instead happens at the SDK's own response-parsing layer: a new exportedOAuthTokenResponseSchemainsrc/shared/auth.tswrapsOAuthTokensSchemain az.preprocessthat removes null-valued optional members before validation (matching the existingElicitResultnull-leniency idiom insrc/types.ts). The optional keys are derived from the schema's shape rather than hardcoded, so future optional members are covered automatically. The SDK's parse sites —executeTokenRequestinsrc/client/auth.tsand both token exchanges inProxyOAuthServerProvider— now use the new schema.access_token: nulland a missingtoken_typeare still rejected, andrefreshAuthorization's refresh-token preservation is hardened to{ ...tokens, refresh_token: tokens.refresh_token ?? refreshToken }so it holds regardless of how the key is serialized. A changeset (patch) is included.Note
Revised after an adversarial review: the previous approach wrapped
OAuthTokensSchemaitself in an object-levelz.preprocess, which changed the exported symbol's class (ZodObject→ZodEffects/ZodPipe), breaking downstream consumers that call.extend()or read.shape— unacceptable in a patch release on the stable 1.x line. It also hardcoded the optional-field list.Tests (
test/shared/auth.test.ts,test/client/auth.test.ts): per-field null-as-absent with strict key-absence assertions, all-optionals-nulltoStrictEqual,expires_in: nullnever becomes0, stringexpires_instill coerces, nullaccess_token/ missingtoken_typestill rejected, a regression test pinning thatOAuthTokensSchemakeeps rejecting nulls and remains usable with.shape/.extend, a drift guard walking every optional member of the shape, plus e2eexchangeAuthorization(all-null optionals) andrefreshAuthorization(server returnsrefresh_token: null, original refresh token preserved).Counterpart for the
main(v2) line: #2462. RFC 6749 doesn't sanctionnullfor absent members, but real-world servers emit it anyway — see #754 for the same null-emitting-server pattern (Ory Hydra) hitting the client registration response schema (that schema is intentionally left out of scope here).🤖 Generated with Claude Code
https://claude.ai/code/session_01KXnNnp3fxQYR5HF9BUVhYP
Generated by Claude Code