fix(auth): treat null optional fields in token responses as absent

.changeset/oauth-token-response-null-optional-fields.md

@@ -0,0 +1,8 @@
+---
+'@modelcontextprotocol/sdk': patch
+---
+
+The client (and the proxy server provider) now normalize JSON `null` values in optional members of OAuth token responses to absent before validation, via a new exported `OAuthTokenResponseSchema` used at the SDK's own parse sites. Some authorization servers serialize absent
+optional members as `null` (nonconformant with RFC 6749 §5.1); previously such responses failed validation (`refresh_token`, `scope`, `id_token`) or coerced `expires_in: null` to `0`, an instantly-expired token. The exported `OAuthTokensSchema` is unchanged. 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. `refreshAuthorization` now also
+preserves the previous refresh token whenever the response does not carry a new one, regardless of how the `refresh_token` key is serialized.

src/client/auth.ts

@@ -16,7 +16,7 @@ import {
     OAuthClientInformationFullSchema,
     OAuthMetadataSchema,
     OAuthProtectedResourceMetadataSchema,
-    OAuthTokensSchema
+    OAuthTokenResponseSchema
 } from '../shared/auth.js';
 import { checkResourceAllowed, resourceUrlFromServerUrl } from '../shared/auth-utils.js';
 import {
@@ -434,8 +434,8 @@ function discardIfIssuerMismatch<T extends { issuer?: string }>(stored: T | null
     return issuersMatch(stored.issuer, issuer) ? stored : undefined;
 }
 
-// `issuer` is added by the client when it stores a value; authorization server responses are parsed without it.
-const TokenResponseSchema = OAuthTokensSchema.omit({ issuer: true });
+// `issuer` is added by the client when it stores a value; authorization server responses are parsed without it
+// (token responses go through OAuthTokenResponseSchema, which omits `issuer` the same way).
 const RegistrationResponseSchema = OAuthClientInformationFullSchema.omit({ issuer: true });
 
 function boundElsewhereError(stored: { issuer?: string }, issuer: string): Error {
@@ -1363,7 +1363,7 @@ async function executeTokenRequest(
         throw await parseErrorResponse(response);
     }
 
-    return TokenResponseSchema.parse(await response.json());
+    return OAuthTokenResponseSchema.parse(await response.json());
 }
 
 /**
@@ -1457,7 +1457,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 };
 }
 
 /**

src/server/auth/providers/proxyProvider.ts

@@ -5,15 +5,15 @@ import {
     OAuthClientInformationFullSchema,
     OAuthTokenRevocationRequest,
     OAuthTokens,
-    OAuthTokensSchema
+    OAuthTokenResponseSchema
 } from '../../../shared/auth.js';
 import { AuthInfo } from '../types.js';
 import { AuthorizationParams, OAuthServerProvider } from '../provider.js';
 import { ServerError } from '../errors.js';
 import { FetchLike } from '../../../shared/transport.js';
 
-// `issuer` is added by a client when it stores a value; upstream responses are parsed without it.
-const TokenResponseSchema = OAuthTokensSchema.omit({ issuer: true });
+// `issuer` is added by a client when it stores a value; upstream responses are parsed without it
+// (token responses go through OAuthTokenResponseSchema, which omits `issuer` the same way).
 const RegistrationResponseSchema = OAuthClientInformationFullSchema.omit({ issuer: true });
 
 export type ProxyEndpoints = {
@@ -192,7 +192,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider {
         }
 
         const data = await response.json();
-        return TokenResponseSchema.parse(data);
+        return OAuthTokenResponseSchema.parse(data);
     }
 
     async exchangeRefreshToken(
@@ -233,7 +233,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider {
         }
 
         const data = await response.json();
-        return TokenResponseSchema.parse(data);
+        return OAuthTokenResponseSchema.parse(data);
     }
 
     async verifyAccessToken(token: string): Promise<AuthInfo> {

src/shared/auth.ts

@@ -143,6 +143,55 @@ export const OAuthTokensSchema = z
     })
     .strip();
 
+/** {@link OAuthTokensSchema} as it appears on the wire: without the client-only `issuer` stamp. */
+const OAuthTokensWireSchema = OAuthTokensSchema.omit({ issuer: true });
+
+/**
+ * Schema for parsing OAuth 2.1 token responses received from an authorization
+ * server.
+ *
+ * Some authorization servers serialize absent optional members as JSON null
+ * (e.g. `"refresh_token": null`), which is nonconformant with RFC 6749 §5.1.
+ * We normalize null to undefined for leniency: null values in optional
+ * members are normalized to absent before validation, so that (a) otherwise
+ * valid responses from such servers parse, and (b) `expires_in: null` never
+ * reaches `z.coerce.number()`, which would coerce it to 0 — an
+ * instantly-expired token. Null values in required members are still
+ * rejected, non-object input is passed through unchanged, and the strict
+ * {@link OAuthTokensSchema} is unaffected.
+ *
+ * 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.
+ *
+ * The client-only `issuer` stamp is not part of the wire format: like the other
+ * response parse paths, this schema validates against {@link OAuthTokensSchema}
+ * without its `issuer` member, so an authorization server cannot inject the
+ * value the client binds stored credentials to.
+ */
+export const OAuthTokenResponseSchema = z.preprocess(
+    data => {
+        if (data === null || typeof data !== 'object' || Array.isArray(data)) {
+            return data;
+        }
+        const normalized: Record<string, unknown> = { ...data };
+        for (const [key, fieldSchema] of Object.entries(OAuthTokensWireSchema.shape)) {
+            if (normalized[key] === null && fieldSchema.safeParse(undefined).success) {
+                delete normalized[key];
+            }
+        }
+        return normalized;
+    },
+    // `issuer` is added by the client when it stores a value; authorization server
+    // responses are parsed without it (and `.strip()` then drops an incoming key).
+    OAuthTokensWireSchema
+);
+
 /**
  * OAuth 2.1 error response
  */

test/client/auth.test.ts

@@ -1602,6 +1602,36 @@ 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('accepts a token response with all optional fields null', async () => {
+            mockFetch.mockResolvedValueOnce({
+                ok: true,
+                status: 200,
+                json: async () => ({
+                    access_token: 'access123',
+                    token_type: 'Bearer',
+                    id_token: null,
+                    expires_in: null,
+                    scope: null,
+                    refresh_token: null
+                })
+            });
+
+            const tokens = await exchangeAuthorization('https://auth.example.com', {
+                clientInformation: validClientInfo,
+                authorizationCode: 'code123',
+                codeVerifier: 'verifier123',
+                redirectUri: 'http://localhost:3000/callback'
+            });
+
+            // The null members must be truly absent from the result, not
+            // present with an undefined value.
+            expect(tokens).toStrictEqual({
+                access_token: 'access123',
+                token_type: 'Bearer'
+            });
+        });
+
         it('exchanges code for tokens with auth', async () => {
             mockFetch.mockResolvedValueOnce({
                 ok: true,
@@ -1852,6 +1882,30 @@ describe('OAuth Authorization', () => {
             expect(tokens).toEqual({ refresh_token: refreshToken, ...validTokens });
         });
 
+        it('keeps the existing refresh token if the server returns a null refresh_token', async () => {
+            mockFetch.mockResolvedValueOnce({
+                ok: true,
+                status: 200,
+                json: async () => ({
+                    access_token: 'newaccess123',
+                    token_type: 'Bearer',
+                    refresh_token: null
+                })
+            });
+
+            const refreshToken = 'refresh123';
+            const tokens = await refreshAuthorization('https://auth.example.com', {
+                clientInformation: validClientInfo,
+                refreshToken
+            });
+
+            expect(tokens).toStrictEqual({
+                access_token: 'newaccess123',
+                token_type: 'Bearer',
+                refresh_token: refreshToken
+            });
+        });
+
         it('validates token response schema', async () => {
             mockFetch.mockResolvedValueOnce({
                 ok: true,

test/server/auth/providers/proxyProvider.test.ts

@@ -226,6 +226,56 @@ describe('Proxy OAuth Server Provider', () => {
             expect(tokens).toEqual(mockTokenResponse);
         });
 
+        describe('null optional fields in upstream response', () => {
+            // Some authorization servers (e.g. AWS Cognito) serialize absent optional
+            // members as JSON null instead of omitting them
+            const nullFieldTokenResponse = {
+                access_token: 'new-access-token',
+                token_type: 'Bearer',
+                expires_in: null,
+                refresh_token: null,
+                scope: null,
+                id_token: null
+            };
+
+            beforeEach(() => {
+                (global.fetch as Mock).mockImplementation(() =>
+                    Promise.resolve({
+                        ok: true,
+                        json: () => Promise.resolve(nullFieldTokenResponse)
+                    })
+                );
+            });
+
+            it('exchanges authorization code and treats null fields as absent', async () => {
+                const tokens = await provider.exchangeAuthorizationCode(validClient, 'test-code', 'test-verifier');
+
+                expect(tokens).toStrictEqual({
+                    access_token: 'new-access-token',
+                    token_type: 'Bearer'
+                });
+                expect('refresh_token' in tokens).toBe(false);
+                expect('expires_in' in tokens).toBe(false);
+                expect('scope' in tokens).toBe(false);
+                expect('id_token' in tokens).toBe(false);
+                expect(tokens.expires_in).toBeUndefined();
+            });
+
+            it('exchanges refresh token and treats null fields as absent', async () => {
+                const tokens = await provider.exchangeRefreshToken(validClient, 'test-refresh-token', ['read', 'write']);
+
+                expect(tokens).toStrictEqual({
+                    access_token: 'new-access-token',
+                    token_type: 'Bearer'
+                });
+                expect('refresh_token' in tokens).toBe(false);
+                expect('expires_in' in tokens).toBe(false);
+                expect('scope' in tokens).toBe(false);
+                expect('id_token' in tokens).toBe(false);
+                expect(tokens.expires_in).toBeUndefined();
+            });
+        });
+
         it.each(['https://upstream.example.com', 42])('does not forward an issuer of %j from the upstream token response', async issuer => {
             (global.fetch as Mock).mockImplementation(() =>
                 Promise.resolve({