Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions .changeset/oauth-tokens-null-optional-fields.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
---
'@modelcontextprotocol/core-internal': patch
'@modelcontextprotocol/client': patch
'@modelcontextprotocol/core': patch
'@modelcontextprotocol/server-legacy': patch
---

OAuth token responses with null-valued optional members no longer fail
validation. Some authorization servers serialize absent optional members as
JSON `null` (nonconformant with RFC 6749 §5.1, but common in the wild);
previously `refresh_token`, `scope`, or `id_token` set to `null` failed token
exchange and refresh, and `expires_in: null` silently coerced to `0`, yielding
an instantly-expired token. The SDK's own token-response parse sites (client
token exchange/refresh, JWT-grant cross-app exchange, and the server-legacy
proxy provider) now validate with a new `OAuthTokenResponseSchema` that removes
null-valued optional members before validation, so they are strictly absent
from the parsed output. The exported `OAuthTokensSchema` is unchanged — still a
plain object schema that rejects nulls, with its `.shape`/`.extend` and input
types intact. `refreshAuthorization` additionally hardens its merge with the
previously-stored refresh token, so an explicitly `undefined` `refresh_token`
in a parsed response can never clobber the preserved token. Note that a
stripped null `scope` is thereafter indistinguishable from an omitted `scope`
— which RFC 6749 §5.1 defines as an assertion that the granted scope is
identical to the requested scope — so consumers should not infer the granted
scope from its absence.
6 changes: 3 additions & 3 deletions packages/client/src/client/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
OAuthErrorResponseSchema,
OAuthMetadataSchema,
OAuthProtectedResourceMetadataSchema,
OAuthTokensSchema,
OAuthTokenResponseSchema,
OpenIdProviderDiscoveryMetadataSchema,
resourceUrlFromServerUrl,
stampErrorBrands
Expand Down Expand Up @@ -2100,7 +2100,7 @@ export async function executeTokenRequest(
const json: unknown = await response.json();

try {
return OAuthTokensSchema.parse(json);
return OAuthTokenResponseSchema.parse(json);
} catch (parseError) {
// Some OAuth servers (e.g., GitHub) return error responses with HTTP 200 status.
// Check for error field only if token parsing failed.
Expand Down Expand Up @@ -2215,7 +2215,7 @@ export async function refreshAuthorization(
});

// Preserve original refresh token if server didn't return a new one
return { refresh_token: refreshToken, ...tokens };
return { ...tokens, refresh_token: tokens.refresh_token ?? refreshToken };
}

/**
Expand Down
4 changes: 2 additions & 2 deletions packages/client/src/client/crossAppAccess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
*/

import type { FetchLike } from '@modelcontextprotocol/core-internal';
import { IdJagTokenExchangeResponseSchema, OAuthErrorResponseSchema, OAuthTokensSchema } from '@modelcontextprotocol/core-internal';
import { IdJagTokenExchangeResponseSchema, OAuthErrorResponseSchema, OAuthTokenResponseSchema } from '@modelcontextprotocol/core-internal';

import type { ClientAuthMethod } from './auth';
import { applyClientAuthentication, assertSecureTokenEndpoint, discoverAuthorizationServerMetadata } from './auth';
Expand Down Expand Up @@ -298,7 +298,7 @@ export async function exchangeJwtAuthGrant(options: {
const responseBody = await response.json();

// Validate response using core schema
const parseResult = OAuthTokensSchema.safeParse(responseBody);
const parseResult = OAuthTokenResponseSchema.safeParse(responseBody);
if (!parseResult.success) {
throw new Error(`Invalid token response: ${parseResult.error.message}`);
}
Expand Down
53 changes: 53 additions & 0 deletions packages/client/test/client/auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2005,6 +2005,39 @@ describe('OAuth Authorization', () => {
expect(body.get('redirect_uri')).toBe('http://localhost:3000/callback');
expect(body.get('resource')).toBe('https://api.example.com/mcp-server');
});
it('treats null optional fields as absent (some auth servers serialize absent members as null)', async () => {
mockFetch.mockResolvedValueOnce({
ok: true,
status: 200,
json: async () => ({
access_token: 'access123',
token_type: 'Bearer',
expires_in: null,
scope: null,
refresh_token: null,
id_token: null
})
});

const tokens = await exchangeAuthorization('https://auth.example.com', {
clientInformation: validClientInfo,
authorizationCode: 'code123',
codeVerifier: 'verifier123',
redirectUri: 'http://localhost:3000/callback',
resource: new URL('https://api.example.com/mcp-server')
});

// toStrictEqual pins the null-valued members as strictly ABSENT keys, not
// present-but-undefined: refreshAuthorization spreads the parsed response
// when merging with stored tokens, so a present `refresh_token: undefined`
// key would clobber the preserved refresh token. It also pins that
// expires_in: null did not coerce to 0 (an instantly-expired token).
expect(tokens).toStrictEqual({
access_token: 'access123',
token_type: 'Bearer'
});
});

it('exchanges code for tokens with auth', async () => {
mockFetch.mockResolvedValueOnce({
ok: true,
Expand Down Expand Up @@ -2257,6 +2290,26 @@ describe('OAuth Authorization', () => {
expect(tokens).toEqual({ refresh_token: refreshToken, ...validTokens });
});

it('keeps the existing refresh token when the server returns refresh_token: null', async () => {
mockFetch.mockResolvedValueOnce({
ok: true,
status: 200,
json: async () => ({ ...validTokens, refresh_token: null })
});

const refreshToken = 'refresh123';
const tokens = await refreshAuthorization('https://auth.example.com', {
clientInformation: validClientInfo,
refreshToken
});

// The null must not clobber the preserved token when the parsed response
// is merged with the previously-stored tokens (it would then be persisted
// via saveTokens, silently destroying the stored refresh token).
expect(tokens.refresh_token).toBe(refreshToken);
expect(tokens).toStrictEqual({ ...validTokens, refresh_token: refreshToken });
});

it('validates token response schema', async () => {
mockFetch.mockResolvedValueOnce({
ok: true,
Expand Down
36 changes: 36 additions & 0 deletions packages/core-internal/src/shared/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,42 @@ export const OAuthTokensSchema = z
})
.strip();

/**
* OAuth 2.1 token response as received over the wire from an authorization server.
*
* Some authorization servers serialize absent optional members as JSON `null`
* (nonconformant with RFC 6749 §5.1, but common in the wild). Null-valued
* optional members are normalized to absent — the key is removed, mirroring
* ElicitResult's `content` null normalization — before {@linkcode OAuthTokensSchema}
* validates, so valid-but-sloppy responses parse and `expires_in: null` never
* reaches `z.coerce.number()` (`Number(null) === 0` would yield an
* instantly-expired token). Removing the key (rather than mapping it to
* `undefined`) matters: callers such as `refreshAuthorization` spread the parsed
* response when merging with previously-stored tokens, and a present-but-undefined
* `refresh_token` key would clobber the preserved value.
*
* Per RFC 6749 §5.1, an absent `scope` member is a positive assertion that
* the granted scope is identical to the scope the client requested, so
* stripping `scope: null` converts a response with undefined semantics into
* that assertion. This has no bearing on enforcement — the SDK never uses
* `tokens.scope` for authorization decisions, and the resource server remains
* authoritative — but consumers must not derive granted-scope conclusions
* from the member's absence. Consumers that need the authoritative grant
* should use token introspection instead.
*/
export const OAuthTokenResponseSchema = z.preprocess(value => {
if (value === null || typeof value !== 'object' || Array.isArray(value)) {
return value;
}
const normalized: Record<string, unknown> = { ...value };
for (const [key, memberSchema] of Object.entries(OAuthTokensSchema.shape)) {
if (normalized[key] === null && memberSchema.safeParse(undefined).success) {
delete normalized[key];
}
}
return normalized;
}, OAuthTokensSchema);

/**
* RFC 8693 §2.2.1 Token Exchange response for ID-JAG tokens.
*
Expand Down
177 changes: 177 additions & 0 deletions packages/core-internal/test/shared/auth.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import * as z from 'zod/v4';

import {
OAuthClientMetadataSchema,
OAuthMetadataSchema,
OAuthTokenResponseSchema,
OAuthTokensSchema,
OpenIdProviderMetadataSchema,
OptionalSafeUrlSchema,
SafeUrlSchema
Expand Down Expand Up @@ -100,6 +104,179 @@ describe('OpenIdProviderMetadataSchema', () => {
});
});

describe('OAuthTokensSchema', () => {
it('parses a fully-populated token response unchanged', () => {
const tokens = OAuthTokensSchema.parse({
access_token: 'access123',
id_token: 'id123',
token_type: 'Bearer',
expires_in: 3600,
scope: 'read write',
refresh_token: 'refresh123'
});

expect(tokens).toEqual({
access_token: 'access123',
id_token: 'id123',
token_type: 'Bearer',
expires_in: 3600,
scope: 'read write',
refresh_token: 'refresh123'
});
});

it('parses a token response with optional fields absent', () => {
const tokens = OAuthTokensSchema.parse({
access_token: 'access123',
token_type: 'Bearer'
});

expect(tokens.access_token).toBe('access123');
expect(tokens.token_type).toBe('Bearer');
expect(tokens.id_token).toBeUndefined();
expect(tokens.expires_in).toBeUndefined();
expect(tokens.scope).toBeUndefined();
expect(tokens.refresh_token).toBeUndefined();
});

it('rejects null-valued members (null normalization lives in OAuthTokenResponseSchema)', () => {
expect(
OAuthTokensSchema.safeParse({
access_token: 'access123',
token_type: 'Bearer',
refresh_token: null
}).success
).toBe(false);
expect(
OAuthTokensSchema.safeParse({
access_token: null,
token_type: 'Bearer'
}).success
).toBe(false);
});

it('remains a plain ZodObject with an introspectable shape (regression pin: consumers use .shape/.extend)', () => {
expect(OAuthTokensSchema).toBeInstanceOf(z.ZodObject);
expect(Object.keys(OAuthTokensSchema.shape).sort()).toEqual([
'access_token',
'expires_in',
'id_token',
'refresh_token',
'scope',
'token_type'
]);

const extended = OAuthTokensSchema.extend({ example_extension: z.string() });
expect(
extended.parse({
access_token: 'access123',
token_type: 'Bearer',
example_extension: 'ext'
}).example_extension
).toBe('ext');

// Optional members must stay optional in z.input — a compile-time pin
// (a schema-level preprocess would degrade these keys to required `unknown`).
const minimalInput: z.input<typeof OAuthTokensSchema> = {
access_token: 'access123',
token_type: 'Bearer'
};
expect(OAuthTokensSchema.parse(minimalInput)).toStrictEqual({
access_token: 'access123',
token_type: 'Bearer'
});
});
});

describe('OAuthTokenResponseSchema', () => {
it.each(['refresh_token', 'scope', 'id_token'] as const)(
'treats a null %s as absent (some auth servers serialize absent members as null)',
field => {
const tokens = OAuthTokenResponseSchema.parse({
access_token: 'access123',
token_type: 'Bearer',
[field]: null
});

expect(field in tokens).toBe(false);
}
);

it('strips all null-valued optional members so the keys are strictly absent', () => {
const tokens = OAuthTokenResponseSchema.parse({
access_token: 'access123',
token_type: 'Bearer',
expires_in: null,
scope: null,
refresh_token: null,
id_token: null
});

// toStrictEqual distinguishes absent keys from present-but-undefined keys.
// Key absence is load-bearing: refreshAuthorization spreads the parsed
// response when merging with previously-stored tokens, and a present
// `refresh_token: undefined` key would clobber the preserved refresh token.
expect(tokens).toStrictEqual({
access_token: 'access123',
token_type: 'Bearer'
});
});

it('treats a null expires_in as absent rather than coercing it to 0 (an instantly-expired token)', () => {
const tokens = OAuthTokenResponseSchema.parse({
access_token: 'access123',
token_type: 'Bearer',
expires_in: null
});

expect('expires_in' in tokens).toBe(false);
expect(tokens.expires_in).not.toBe(0);
});

it('still coerces string expires_in values to numbers', () => {
const tokens = OAuthTokenResponseSchema.parse({
access_token: 'access123',
token_type: 'Bearer',
expires_in: '3600'
});

expect(tokens.expires_in).toBe(3600);
});

it('still rejects a null access_token (only optional members are normalized)', () => {
expect(
OAuthTokenResponseSchema.safeParse({
access_token: null,
token_type: 'Bearer'
}).success
).toBe(false);
});

it('still rejects a missing token_type', () => {
expect(
OAuthTokenResponseSchema.safeParse({
access_token: 'access123'
}).success
).toBe(false);
});

it('normalizes a null in every optional member of OAuthTokensSchema (drift guard for future members)', () => {
for (const [key, memberSchema] of Object.entries(OAuthTokensSchema.shape)) {
if (!memberSchema.safeParse(undefined).success) {
continue; // Required member — nulls must keep failing, covered above.
}

const tokens = OAuthTokenResponseSchema.parse({
access_token: 'access123',
token_type: 'Bearer',
[key]: null
});

expect(key in tokens).toBe(false);
}
});
});

describe('OAuthClientMetadataSchema', () => {
it('validates client metadata with safe URLs', () => {
const metadata = {
Expand Down
6 changes: 3 additions & 3 deletions packages/server-legacy/src/auth/providers/proxyProvider.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { FetchLike, OAuthClientInformationFull, OAuthTokenRevocationRequest, OAuthTokens } from '@modelcontextprotocol/core-internal';
import { OAuthClientInformationFullSchema, OAuthTokensSchema } from '@modelcontextprotocol/core-internal';
import { OAuthClientInformationFullSchema, OAuthTokenResponseSchema } from '@modelcontextprotocol/core-internal';
import type { Response } from 'express';

import type { OAuthRegisteredClientsStore } from '../clients';
Expand Down Expand Up @@ -194,7 +194,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider {
}

const data = await response.json();
return OAuthTokensSchema.parse(data);
return OAuthTokenResponseSchema.parse(data);
}

async exchangeRefreshToken(
Expand Down Expand Up @@ -235,7 +235,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider {
}

const data = await response.json();
return OAuthTokensSchema.parse(data);
return OAuthTokenResponseSchema.parse(data);
}

async verifyAccessToken(token: string): Promise<AuthInfo> {
Expand Down
Loading