Skip to content

Commit 3295106

Browse files
Zelys-DFKHclaude
andcommitted
fix(client): preserve resource_metadata URL across non-Bearer WWW-Authenticate challenges
When WWW-Authenticate carries a non-Bearer scheme (e.g. Negotiate), extractWWWAuthenticateParams returns undefined for resourceMetadataUrl. Both transports were unconditionally writing that undefined back to _resourceMetadataUrl, wiping out any URL stored from an earlier Bearer challenge and breaking PRM discovery for multi-challenge sequences. - Add optional resourceMetadataUrl to UnauthorizedContext so transports can pass the stored Bearer URL into the onUnauthorized callback - handleOAuthUnauthorized now uses extractedUrl ?? ctx.resourceMetadataUrl, falling back to the stored URL when the current challenge has none - Both StreamableHTTPClientTransport and SSEClientTransport guard _resourceMetadataUrl and _scope updates to only overwrite when the extracted value is non-undefined, and include this._resourceMetadataUrl in every onUnauthorized context Fixes #1946 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent bdfd7f0 commit 3295106

5 files changed

Lines changed: 160 additions & 22 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@modelcontextprotocol/client': patch
3+
---
4+
5+
Fixed OAuth discovery fall back to well-known URI when WWW-Authenticate carries a non-Bearer scheme (e.g. Negotiate). Previously a non-Bearer challenge could clear the `resource_metadata` URL stored from an earlier Bearer challenge, causing PRM discovery to start from the wrong endpoint. The stored URL is now preserved across non-Bearer 401s and passed through to the authorization handler via context.

packages/client/src/client/auth.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,12 @@ export interface UnauthorizedContext {
4646
serverUrl: URL;
4747
/** Fetch function configured with the transport's `requestInit`, for making auth requests. */
4848
fetchFn: FetchLike;
49+
/**
50+
* Resource metadata URL previously established from an earlier Bearer challenge, if any.
51+
* Used as a fallback when the current `WWW-Authenticate` header does not carry a
52+
* `resource_metadata` parameter (e.g. a non-Bearer scheme such as `Negotiate`).
53+
*/
54+
resourceMetadataUrl?: URL;
4955
}
5056

5157
/**
@@ -101,7 +107,11 @@ export function isOAuthClientProvider(provider: AuthProvider | OAuthClientProvid
101107
* Used by {@linkcode adaptOAuthProvider} to bridge `OAuthClientProvider` to `AuthProvider`.
102108
*/
103109
export async function handleOAuthUnauthorized(provider: OAuthClientProvider, ctx: UnauthorizedContext): Promise<void> {
104-
const { resourceMetadataUrl, scope } = extractWWWAuthenticateParams(ctx.response);
110+
const { resourceMetadataUrl: extractedUrl, scope } = extractWWWAuthenticateParams(ctx.response);
111+
// When the current challenge carries no resource_metadata (e.g. a non-Bearer scheme
112+
// like Negotiate), fall back to the URL the transport stored from an earlier Bearer
113+
// challenge so that PRM discovery can still run with the right starting point.
114+
const resourceMetadataUrl = extractedUrl ?? ctx.resourceMetadataUrl;
105115
const result = await auth(provider, {
106116
serverUrl: ctx.serverUrl,
107117
resourceMetadataUrl,

packages/client/src/client/sse.ts

Lines changed: 36 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,15 @@ export class SSEClientTransport implements Transport {
135135
this._last401Response = response;
136136
if (response.headers.has('www-authenticate')) {
137137
const { resourceMetadataUrl, scope } = extractWWWAuthenticateParams(response);
138-
this._resourceMetadataUrl = resourceMetadataUrl;
139-
this._scope = scope;
138+
// Only update when the Bearer challenge advertises a URL; a non-Bearer
139+
// scheme (e.g. Negotiate) must not clear a previously-stored value so
140+
// the well-known fallback can still run with the right starting point.
141+
if (resourceMetadataUrl !== undefined) {
142+
this._resourceMetadataUrl = resourceMetadataUrl;
143+
}
144+
if (scope !== undefined) {
145+
this._scope = scope;
146+
}
140147
}
141148
}
142149

@@ -151,15 +158,22 @@ export class SSEClientTransport implements Transport {
151158
const response = this._last401Response;
152159
this._last401Response = undefined;
153160
this._eventSource?.close();
154-
this._authProvider.onUnauthorized({ response, serverUrl: this._url, fetchFn: this._fetchWithInit }).then(
155-
// onUnauthorized succeeded → retry fresh. Its onerror handles its own onerror?.() + reject.
156-
() => this._startOrAuth().then(resolve, reject),
157-
// onUnauthorized failed → not yet reported.
158-
error => {
159-
this.onerror?.(error);
160-
reject(error);
161-
}
162-
);
161+
this._authProvider
162+
.onUnauthorized({
163+
response,
164+
serverUrl: this._url,
165+
fetchFn: this._fetchWithInit,
166+
resourceMetadataUrl: this._resourceMetadataUrl
167+
})
168+
.then(
169+
// onUnauthorized succeeded → retry fresh. Its onerror handles its own onerror?.() + reject.
170+
() => this._startOrAuth().then(resolve, reject),
171+
// onUnauthorized failed → not yet reported.
172+
error => {
173+
this.onerror?.(error);
174+
reject(error);
175+
}
176+
);
163177
return;
164178
}
165179
const error = new UnauthorizedError();
@@ -270,15 +284,23 @@ export class SSEClientTransport implements Transport {
270284
if (response.status === 401 && this._authProvider) {
271285
if (response.headers.has('www-authenticate')) {
272286
const { resourceMetadataUrl, scope } = extractWWWAuthenticateParams(response);
273-
this._resourceMetadataUrl = resourceMetadataUrl;
274-
this._scope = scope;
287+
// Only update when the Bearer challenge advertises a URL; a non-Bearer
288+
// scheme (e.g. Negotiate) must not clear a previously-stored value so
289+
// the well-known fallback can still run with the right starting point.
290+
if (resourceMetadataUrl !== undefined) {
291+
this._resourceMetadataUrl = resourceMetadataUrl;
292+
}
293+
if (scope !== undefined) {
294+
this._scope = scope;
295+
}
275296
}
276297

277298
if (this._authProvider.onUnauthorized && !isAuthRetry) {
278299
await this._authProvider.onUnauthorized({
279300
response,
280301
serverUrl: this._url,
281-
fetchFn: this._fetchWithInit
302+
fetchFn: this._fetchWithInit,
303+
resourceMetadataUrl: this._resourceMetadataUrl
282304
});
283305
await response.text?.().catch(() => {});
284306
// Purposely _not_ awaited, so we don't call onerror twice

packages/client/src/client/streamableHttp.ts

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -257,15 +257,23 @@ export class StreamableHTTPClientTransport implements Transport {
257257
if (response.status === 401 && this._authProvider) {
258258
if (response.headers.has('www-authenticate')) {
259259
const { resourceMetadataUrl, scope } = extractWWWAuthenticateParams(response);
260-
this._resourceMetadataUrl = resourceMetadataUrl;
261-
this._scope = scope;
260+
// Only update when the Bearer challenge advertises a URL; a non-Bearer
261+
// scheme (e.g. Negotiate) must not clear a previously-stored value so
262+
// the well-known fallback can still run with the right starting point.
263+
if (resourceMetadataUrl !== undefined) {
264+
this._resourceMetadataUrl = resourceMetadataUrl;
265+
}
266+
if (scope !== undefined) {
267+
this._scope = scope;
268+
}
262269
}
263270

264271
if (this._authProvider.onUnauthorized && !isAuthRetry) {
265272
await this._authProvider.onUnauthorized({
266273
response,
267274
serverUrl: this._url,
268-
fetchFn: this._fetchWithInit
275+
fetchFn: this._fetchWithInit,
276+
resourceMetadataUrl: this._resourceMetadataUrl
269277
});
270278
await response.text?.().catch(() => {});
271279
// Purposely _not_ awaited, so we don't call onerror twice
@@ -565,15 +573,23 @@ export class StreamableHTTPClientTransport implements Transport {
565573
// Store WWW-Authenticate params for interactive finishAuth() path
566574
if (response.headers.has('www-authenticate')) {
567575
const { resourceMetadataUrl, scope } = extractWWWAuthenticateParams(response);
568-
this._resourceMetadataUrl = resourceMetadataUrl;
569-
this._scope = scope;
576+
// Only update when the Bearer challenge advertises a URL; a non-Bearer
577+
// scheme (e.g. Negotiate) must not clear a previously-stored value so
578+
// the well-known fallback can still run with the right starting point.
579+
if (resourceMetadataUrl !== undefined) {
580+
this._resourceMetadataUrl = resourceMetadataUrl;
581+
}
582+
if (scope !== undefined) {
583+
this._scope = scope;
584+
}
570585
}
571586

572587
if (this._authProvider.onUnauthorized && !isAuthRetry) {
573588
await this._authProvider.onUnauthorized({
574589
response,
575590
serverUrl: this._url,
576-
fetchFn: this._fetchWithInit
591+
fetchFn: this._fetchWithInit,
592+
resourceMetadataUrl: this._resourceMetadataUrl
577593
});
578594
await response.text?.().catch(() => {});
579595
// Purposely _not_ awaited, so we don't call onerror twice

packages/client/test/client/streamableHttp.test.ts

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type { JSONRPCMessage, JSONRPCRequest } from '@modelcontextprotocol/core'
22
import { OAuthError, OAuthErrorCode, SdkError, SdkErrorCode } from '@modelcontextprotocol/core';
33
import type { Mock, Mocked } from 'vitest';
44

5-
import type { OAuthClientProvider } from '../../src/client/auth.js';
5+
import type { AuthProvider, OAuthClientProvider } from '../../src/client/auth.js';
66
import { UnauthorizedError } from '../../src/client/auth.js';
77
import type { ReconnectionScheduler, StartSSEOptions, StreamableHTTPReconnectionOptions } from '../../src/client/streamableHttp.js';
88
import { StreamableHTTPClientTransport } from '../../src/client/streamableHttp.js';
@@ -783,6 +783,91 @@ describe('StreamableHTTPClientTransport', () => {
783783
expect(mockAuthProvider.redirectToAuthorization.mock.calls).toHaveLength(1);
784784
});
785785

786+
it('falls back to well-known discovery on 401 with non-Bearer WWW-Authenticate (e.g. Negotiate)', async () => {
787+
const message: JSONRPCMessage = {
788+
jsonrpc: '2.0',
789+
method: 'test',
790+
params: {},
791+
id: 'test-id'
792+
};
793+
794+
(globalThis.fetch as Mock)
795+
.mockResolvedValueOnce({
796+
ok: false,
797+
status: 401,
798+
statusText: 'Unauthorized',
799+
headers: new Headers({ 'WWW-Authenticate': 'Negotiate' }),
800+
text: async () => 'Authentication required.'
801+
})
802+
.mockResolvedValue({
803+
ok: false,
804+
status: 404,
805+
text: async () => {
806+
throw 'dont read my body';
807+
}
808+
});
809+
810+
await expect(transport.send(message)).rejects.toThrow(UnauthorizedError);
811+
expect(mockAuthProvider.redirectToAuthorization.mock.calls).toHaveLength(1);
812+
});
813+
814+
it('passes stored resource metadata URL to onUnauthorized context after non-Bearer 401', async () => {
815+
// Regression test: a non-Bearer WWW-Authenticate (e.g. Negotiate) must not wipe
816+
// out a Bearer resource_metadata URL stored from an earlier Bearer challenge.
817+
// The stored URL must reach onUnauthorized via ctx.resourceMetadataUrl so PRM
818+
// discovery can use the right starting point instead of falling back to the root
819+
// well-known URI.
820+
//
821+
// Uses a plain AuthProvider (not OAuthClientProvider) so onUnauthorized is called
822+
// directly by the transport — no adaptOAuthProvider wrapper whose internal closure
823+
// would hide the context from a spy.
824+
const message: JSONRPCMessage = {
825+
jsonrpc: '2.0',
826+
method: 'test',
827+
params: {},
828+
id: 'test-id'
829+
};
830+
const resourceMetadataUrl = 'http://example.com/.well-known/oauth-protected-resource';
831+
832+
const onUnauthorized = vi.fn<AuthProvider['onUnauthorized'] & object>().mockResolvedValue(undefined);
833+
const plainTransport = new StreamableHTTPClientTransport(new URL('http://localhost:1234/mcp'), {
834+
authProvider: { token: async () => 'test-token', onUnauthorized }
835+
});
836+
837+
try {
838+
// First request: Bearer 401 → stores _resourceMetadataUrl; onUnauthorized resolves, retry succeeds
839+
(globalThis.fetch as Mock)
840+
.mockResolvedValueOnce({
841+
ok: false,
842+
status: 401,
843+
headers: new Headers({ 'WWW-Authenticate': `Bearer resource_metadata="${resourceMetadataUrl}"` }),
844+
text: async () => ''
845+
})
846+
.mockResolvedValueOnce({ ok: true, status: 202, headers: new Headers() });
847+
848+
await plainTransport.send(message);
849+
onUnauthorized.mockClear();
850+
851+
// Second request: Negotiate 401 → must NOT clear _resourceMetadataUrl; retry succeeds
852+
(globalThis.fetch as Mock)
853+
.mockResolvedValueOnce({
854+
ok: false,
855+
status: 401,
856+
headers: new Headers({ 'WWW-Authenticate': 'Negotiate' }),
857+
text: async () => ''
858+
})
859+
.mockResolvedValueOnce({ ok: true, status: 202, headers: new Headers() });
860+
861+
await plainTransport.send(message);
862+
863+
// onUnauthorized must receive the stored Bearer URL in ctx.resourceMetadataUrl,
864+
// not undefined, so PRM discovery uses the correct endpoint.
865+
expect(onUnauthorized).toHaveBeenCalledWith(expect.objectContaining({ resourceMetadataUrl: new URL(resourceMetadataUrl) }));
866+
} finally {
867+
await plainTransport.close().catch(() => {});
868+
}
869+
});
870+
786871
it('attempts upscoping on 403 with WWW-Authenticate header', async () => {
787872
const message: JSONRPCMessage = {
788873
jsonrpc: '2.0',

0 commit comments

Comments
 (0)