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

.changeset/oauth-tokens-null-optional-fields.md

@@ -0,0 +1,38 @@
+---
+'@modelcontextprotocol/core': minor
+'@modelcontextprotocol/core-internal': patch
+'@modelcontextprotocol/client': patch
+'@modelcontextprotocol/server-legacy': patch
+'@modelcontextprotocol/codemod': 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` (defined in
+`@modelcontextprotocol/core`'s auth schema module and forwarded through
+`core-internal`'s re-export shim) 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, with its `.shape`/`.extend` and input types intact: it
+rejects `null` for its string-typed optional members, though `expires_in: null`
+still coerces to `0` there (use `OAuthTokenResponseSchema` for raw wire
+input). `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.
+
+`OAuthTokenResponseSchema` is also a public export from
+`@modelcontextprotocol/core`'s root, since the sibling v1 release exports it
+from `@modelcontextprotocol/sdk/shared/auth.js` and migrating code needs a v2
+home for it. The v1-to-v2 codemod's auth schema allowlist now includes the
+name, so `import { OAuthTokenResponseSchema } from
+'@modelcontextprotocol/sdk/shared/auth.js'` rewrites to
+`@modelcontextprotocol/core` alongside the other auth schema constants.

docs/advanced/wire-schemas.md

@@ -4,7 +4,7 @@ shape: how-to
 
 # Wire schemas
 
-`@modelcontextprotocol/core` exports the **wire schemas** — the exact Zod constants the SDK validates protocol and OAuth payloads against — for code that holds raw JSON instead of SDK objects.
+`@modelcontextprotocol/core` exports the **wire schemas** — the exact Zod constants the SDK validates protocol and OAuth payloads against — for code that holds raw JSON instead of SDK objects. (One qualification: at the SDK's own token-response parse sites, `OAuthTokensSchema` is applied through the null-tolerant `OAuthTokenResponseSchema` wrapper — see the note in the OAuth section below.)
 
 ## Validate a wire payload
 
@@ -129,7 +129,7 @@ A document missing a required endpoint fails the parse; a valid one comes back t
 https://auth.example.com/token
 ```
 
-The group follows the same naming convention: `OAuthTokensSchema` for token responses, `OAuthProtectedResourceMetadataSchema` for protected-resource metadata, `OpenIdProviderDiscoveryMetadataSchema` for OpenID provider discovery.
+The group follows the same naming convention: `OAuthTokensSchema` for token responses, `OAuthProtectedResourceMetadataSchema` for protected-resource metadata, `OpenIdProviderDiscoveryMetadataSchema` for OpenID provider discovery. Note that the SDK's own token-response parse sites validate with `OAuthTokenResponseSchema` — also exported from `@modelcontextprotocol/core` — a null-tolerant wrapper around `OAuthTokensSchema` that drops null-valued optional members before validating. The plain `OAuthTokensSchema` rejects `null` for its string-typed members, but `expires_in: null` coerces to `0` there, so prefer `OAuthTokenResponseSchema` when validating raw wire input.
 
 ## Get the TypeScript types, guards and errors from the SDK packages
 

packages/client/src/client/auth.ts

@@ -23,7 +23,7 @@ import {
     OAuthErrorResponseSchema,
     OAuthMetadataSchema,
     OAuthProtectedResourceMetadataSchema,
-    OAuthTokensSchema,
+    OAuthTokenResponseSchema,
     OpenIdProviderDiscoveryMetadataSchema,
     resourceUrlFromServerUrl,
     stampErrorBrands,
@@ -1426,7 +1426,14 @@ async function authInternal(
         // token, so silently dropping the new tokens would leave the client
         // with credentials that are already invalid server-side.
         if (newTokens) {
-            await provider.saveTokens({ ...newTokens, issuer }, infoCtx);
+            // RFC 6749 §5.1/§6: the SDK sends no `scope` parameter on refresh, so a
+            // response without `scope` (including a null-stripped one) asserts the grant
+            // is unchanged — preserve the stored scope instead of erasing the recorded
+            // grant (the 403 insufficient_scope step-up unions it to avoid losing
+            // previously-granted permissions). The conditional spread keeps `scope`
+            // strictly absent when neither side has one — never present-but-undefined.
+            const preservedScope = newTokens.scope ?? tokens.scope;
+            await provider.saveTokens({ ...newTokens, ...(preservedScope !== undefined && { scope: preservedScope }), issuer }, infoCtx);
             return 'AUTHORIZED';
         }
     }
@@ -2261,7 +2268,7 @@ export async function executeTokenRequest(
     const json: unknown = await response.json();
 
     try {
-        return OAuthTokensSchema.parse(withoutIssuer(json));
+        return OAuthTokenResponseSchema.parse(withoutIssuer(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.
@@ -2384,7 +2391,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 };
 }
 
 /**

packages/client/src/client/crossAppAccess.ts

@@ -13,7 +13,7 @@ import {
     fetchWithinOrigin,
     IdJagTokenExchangeResponseSchema,
     OAuthErrorResponseSchema,
-    OAuthTokensSchema,
+    OAuthTokenResponseSchema,
     withoutIssuer
 } from '@modelcontextprotocol/core-internal';
 
@@ -304,7 +304,7 @@ export async function exchangeJwtAuthGrant(options: {
     const responseBody = await response.json();
 
     // Validate response using core schema
-    const parseResult = OAuthTokensSchema.safeParse(withoutIssuer(responseBody));
+    const parseResult = OAuthTokenResponseSchema.safeParse(withoutIssuer(responseBody));
     if (!parseResult.success) {
         throw new Error(`Invalid token response: ${parseResult.error.message}`);
     }

packages/client/test/client/auth.test.ts

@@ -2026,6 +2026,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,
@@ -2278,6 +2311,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,
@@ -3061,7 +3114,7 @@ describe('OAuth Authorization', () => {
             expect(body.get('refresh_token')).toBe('refresh123');
         });
 
-        // The #2034 tests below differ only in how the token endpoint answers, so the
+        // The tests below differ only in how the token endpoint answers, so the
         // discovery fixture is shared. `tokenResponse` is invoked per POST to /token.
         const mockDiscoveryWithTokenEndpoint = (tokenResponse: () => unknown): void => {
             mockFetch.mockImplementation(url => {
@@ -3096,6 +3149,158 @@ describe('OAuth Authorization', () => {
             });
         };
 
+        // RFC 6749 §5.1/§6: the SDK sends no `scope` parameter on refresh, so a response
+        // without `scope` (or with `scope: null`, stripped by OAuthTokenResponseSchema)
+        // asserts the grant is unchanged. The stored scope must survive the refresh —
+        // otherwise, after a restart, the insufficient_scope step-up would compute its
+        // scope union without the original grant and force interactive re-authorization.
+        const mockRefreshFetchWithTokenResponse = (tokenResponse: Record<string, unknown>): void =>
+            mockDiscoveryWithTokenEndpoint(() => ({
+                ok: true,
+                status: 200,
+                json: async () => tokenResponse
+            }));
+
+        it('keeps the stored scope when the refresh response returns scope: null', async () => {
+            mockRefreshFetchWithTokenResponse({
+                access_token: 'new-access123',
+                token_type: 'Bearer',
+                expires_in: 3600,
+                scope: null
+            });
+
+            (mockProvider.clientInformation as Mock).mockResolvedValue({
+                client_id: 'test-client',
+                client_secret: 'test-secret'
+            });
+            (mockProvider.tokens as Mock).mockResolvedValue({
+                access_token: 'old-access',
+                refresh_token: 'refresh123',
+                scope: 'read write'
+            });
+            (mockProvider.saveTokens as Mock).mockResolvedValue(undefined);
+
+            const result = await auth(mockProvider, {
+                serverUrl: 'https://api.example.com/mcp-server'
+            });
+
+            expect(result).toBe('AUTHORIZED');
+            expect(mockProvider.saveTokens).toHaveBeenCalledWith(
+                expect.objectContaining({
+                    access_token: 'new-access123',
+                    refresh_token: 'refresh123',
+                    scope: 'read write'
+                }),
+                expect.anything()
+            );
+        });
+
+        it('keeps the stored scope when the refresh response omits scope', async () => {
+            mockRefreshFetchWithTokenResponse({
+                access_token: 'new-access123',
+                token_type: 'Bearer',
+                expires_in: 3600
+            });
+
+            (mockProvider.clientInformation as Mock).mockResolvedValue({
+                client_id: 'test-client',
+                client_secret: 'test-secret'
+            });
+            (mockProvider.tokens as Mock).mockResolvedValue({
+                access_token: 'old-access',
+                refresh_token: 'refresh123',
+                scope: 'read write'
+            });
+            (mockProvider.saveTokens as Mock).mockResolvedValue(undefined);
+
+            const result = await auth(mockProvider, {
+                serverUrl: 'https://api.example.com/mcp-server'
+            });
+
+            expect(result).toBe('AUTHORIZED');
+            expect(mockProvider.saveTokens).toHaveBeenCalledWith(
+                expect.objectContaining({
+                    access_token: 'new-access123',
+                    refresh_token: 'refresh123',
+                    scope: 'read write'
+                }),
+                expect.anything()
+            );
+        });
+
+        it('stores the scope the refresh response grants when one is present', async () => {
+            mockRefreshFetchWithTokenResponse({
+                access_token: 'new-access123',
+                token_type: 'Bearer',
+                expires_in: 3600,
+                scope: 'read'
+            });
+
+            (mockProvider.clientInformation as Mock).mockResolvedValue({
+                client_id: 'test-client',
+                client_secret: 'test-secret'
+            });
+            (mockProvider.tokens as Mock).mockResolvedValue({
+                access_token: 'old-access',
+                refresh_token: 'refresh123',
+                scope: 'read write'
+            });
+            (mockProvider.saveTokens as Mock).mockResolvedValue(undefined);
+
+            const result = await auth(mockProvider, {
+                serverUrl: 'https://api.example.com/mcp-server'
+            });
+
+            expect(result).toBe('AUTHORIZED');
+            // A scope the server does return is authoritative (a refresh may narrow the
+            // grant, RFC 6749 §6) — preservation only applies to an absent member.
+            expect(mockProvider.saveTokens).toHaveBeenCalledWith(
+                expect.objectContaining({
+                    access_token: 'new-access123',
+                    scope: 'read'
+                }),
+                expect.anything()
+            );
+        });
+
+        it('saves a payload without a scope key when neither the response nor the stored tokens have one', async () => {
+            mockRefreshFetchWithTokenResponse({
+                access_token: 'new-access123',
+                token_type: 'Bearer',
+                expires_in: 3600
+            });
+
+            (mockProvider.clientInformation as Mock).mockResolvedValue({
+                client_id: 'test-client',
+                client_secret: 'test-secret'
+            });
+            (mockProvider.tokens as Mock).mockResolvedValue({
+                access_token: 'old-access',
+                refresh_token: 'refresh123'
+            });
+            (mockProvider.saveTokens as Mock).mockResolvedValue(undefined);
+
+            const result = await auth(mockProvider, {
+                serverUrl: 'https://api.example.com/mcp-server'
+            });
+
+            expect(result).toBe('AUTHORIZED');
+            // Scope preservation must not manufacture a present-but-undefined `scope`
+            // key — the exact key shape this PR's normalization exists to prevent.
+            // toStrictEqual distinguishes an absent key from one set to undefined.
+            // The last saveTokens call is the refresh save (the first is the
+            // SEP-2352 issuer back-stamp of the legacy unstamped token set).
+            const savedTokens = (mockProvider.saveTokens as Mock).mock.lastCall![0];
+            expect('scope' in savedTokens).toBe(false);
+            expect(savedTokens).toStrictEqual({
+                access_token: 'new-access123',
+                token_type: 'Bearer',
+                expires_in: 3600,
+                refresh_token: 'refresh123',
+                issuer: 'https://auth.example.com'
+            });
+        });
+
         // A real Response: parseErrorResponse() reads the body via .text(), which a plain
         // object mock cannot satisfy.
         const oauthErrorResponse = (code: OAuthErrorCode, message: string): Response =>

packages/codemod/src/migrations/v1-to-v2/mappings/authSchemaNames.ts

@@ -4,7 +4,9 @@
 // set (the corresponding TYPES, e.g. OAuthTokens, resolve by context to @modelcontextprotocol/client |
 // /server). This is the v1 auth-schema set — a SUBSET of core's auth exports. v2-only auth schemas
 // (e.g. IdJagTokenExchangeResponseSchema) are exported by core but NOT listed here: v1 never had
-// them, so there is nothing to migrate. test/v1-to-v2/authSchemaNames.test.ts asserts every name here is
+// them, so there is nothing to migrate. OAuthTokenResponseSchema, the null-tolerant token-response
+// wrapper, IS listed: v1 exports it from sdk/shared/auth.js and core exports it publicly, so its
+// imports route to core like the rest. test/v1-to-v2/authSchemaNames.test.ts asserts every name here is
 // exported by core (so the rewritten import resolves). Keep alphabetized.
 export const AUTH_SCHEMA_NAMES: ReadonlySet<string> = new Set([
     'OAuthClientInformationFullSchema',
@@ -14,6 +16,7 @@ export const AUTH_SCHEMA_NAMES: ReadonlySet<string> = new Set([
     'OAuthErrorResponseSchema',
     'OAuthMetadataSchema',
     'OAuthProtectedResourceMetadataSchema',
+    'OAuthTokenResponseSchema',
     'OAuthTokenRevocationRequestSchema',
     'OAuthTokensSchema',
     'OpenIdProviderDiscoveryMetadataSchema',

packages/codemod/test/v1-to-v2/authSchemaNames.test.ts

@@ -21,8 +21,11 @@ describe('AUTH_SCHEMA_NAMES (codemod auth schema-routing allowlist)', () => {
 
         const notExportedByCore = [...AUTH_SCHEMA_NAMES].filter(name => !coreAuthExports.has(name));
         expect(notExportedByCore).toEqual([]);
-        // The v1 auth-schema set is frozen; pin its size so an accidental add/remove is caught.
-        expect(AUTH_SCHEMA_NAMES.size).toBe(11);
+        // Pin the v1 auth-schema set's size so an accidental add/remove is caught. 12 = the 11
+        // original v1 exports plus OAuthTokenResponseSchema (the null-tolerant token-response
+        // wrapper, public in v1 and exported from core's barrel).
+        expect(AUTH_SCHEMA_NAMES.size).toBe(12);
+        expect(AUTH_SCHEMA_NAMES.has('OAuthTokenResponseSchema')).toBe(true);
     });
 
     it('keeps the no-v2-home auth schemas OUT of the routing allowlist', () => {

packages/core-internal/src/shared/auth.ts

@@ -30,6 +30,7 @@ export {
     OAuthErrorResponseSchema,
     OAuthMetadataSchema,
     OAuthProtectedResourceMetadataSchema,
+    OAuthTokenResponseSchema,
     OAuthTokenRevocationRequestSchema,
     OAuthTokensSchema,
     OpenIdProviderDiscoveryMetadataSchema,