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

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

@@ -0,0 +1,38 @@
+---
+'@modelcontextprotocol/core': patch
+'@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

@@ -22,7 +22,7 @@ import {
     OAuthErrorResponseSchema,
     OAuthMetadataSchema,
     OAuthProtectedResourceMetadataSchema,
-    OAuthTokensSchema,
+    OAuthTokenResponseSchema,
     OpenIdProviderDiscoveryMetadataSchema,
     resourceUrlFromServerUrl,
     stampErrorBrands
@@ -1314,7 +1314,14 @@ async function authInternal(
                 fetchFn
             });
 
-            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';
         } catch (error) {
             // A non-TLS token endpoint is a configuration error — re-authorizing cannot
@@ -2100,7 +2107,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.
@@ -2215,7 +2222,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

@@ -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';
@@ -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}`);
     }

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

@@ -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,
@@ -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,
@@ -3035,6 +3088,188 @@ describe('OAuth Authorization', () => {
             expect(body.get('refresh_token')).toBe('refresh123');
         });
 
+        // 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>) => {
+            mockFetch.mockImplementation(url => {
+                const urlString = url.toString();
+
+                if (urlString.includes('/.well-known/oauth-protected-resource')) {
+                    return Promise.resolve({
+                        ok: true,
+                        status: 200,
+                        json: async () => ({
+                            resource: 'https://api.example.com/mcp-server',
+                            authorization_servers: ['https://auth.example.com']
+                        })
+                    });
+                } else if (urlString.includes('/.well-known/oauth-authorization-server')) {
+                    return Promise.resolve({
+                        ok: true,
+                        status: 200,
+                        json: async () => ({
+                            issuer: 'https://auth.example.com',
+                            authorization_endpoint: 'https://auth.example.com/authorize',
+                            token_endpoint: 'https://auth.example.com/token',
+                            response_types_supported: ['code'],
+                            code_challenge_methods_supported: ['S256']
+                        })
+                    });
+                } else if (urlString.includes('/token')) {
+                    return Promise.resolve({
+                        ok: true,
+                        status: 200,
+                        json: async () => tokenResponse
+                    });
+                }
+
+                return Promise.resolve({ ok: false, status: 404 });
+            });
+        };
+
+        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'
+            });
+        });
+
         it('skips default PRM resource validation when custom validateResourceURL is provided', async () => {
             const mockValidateResourceURL = vi.fn().mockResolvedValue(undefined);
             const providerWithCustomValidation = {

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',