Skip to content

fix(client): preserve resource_metadata URL across non-Bearer WWW-Authenticate challenges #1951

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Zelys-DFKH wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(client): preserve resource_metadata URL across non-Bearer WWW-Authenticate challenges #1951

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions .changeset/fix-oauth-non-bearer-www-authenticate-fallback.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@modelcontextprotocol/client': patch
---

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.
12 changes: 11 additions & 1 deletion packages/client/src/client/auth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ export interface UnauthorizedContext {
serverUrl: URL;
/** Fetch function configured with the transport's `requestInit`, for making auth requests. */
fetchFn: FetchLike;
/**
* Resource metadata URL previously established from an earlier Bearer challenge, if any.
* Used as a fallback when the current `WWW-Authenticate` header does not carry a
* `resource_metadata` parameter (e.g. a non-Bearer scheme such as `Negotiate`).
*/
resourceMetadataUrl?: URL;
}

/**
Expand Down Expand Up @@ -101,7 +107,11 @@ export function isOAuthClientProvider(provider: AuthProvider | OAuthClientProvid
* Used by {@linkcode adaptOAuthProvider} to bridge `OAuthClientProvider` to `AuthProvider`.
*/
export async function handleOAuthUnauthorized(provider: OAuthClientProvider, ctx: UnauthorizedContext): Promise<void> {
const { resourceMetadataUrl, scope } = extractWWWAuthenticateParams(ctx.response);
const { resourceMetadataUrl: extractedUrl, scope } = extractWWWAuthenticateParams(ctx.response);
// When the current challenge carries no resource_metadata (e.g. a non-Bearer scheme
// like Negotiate), fall back to the URL the transport stored from an earlier Bearer
// challenge so that PRM discovery can still run with the right starting point.
const resourceMetadataUrl = extractedUrl ?? ctx.resourceMetadataUrl;
const result = await auth(provider, {
serverUrl: ctx.serverUrl,
resourceMetadataUrl,
Expand Down
50 changes: 36 additions & 14 deletions packages/client/src/client/sse.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,15 @@ export class SSEClientTransport implements Transport {
this._last401Response = response;
if (response.headers.has('www-authenticate')) {
const { resourceMetadataUrl, scope } = extractWWWAuthenticateParams(response);
this._resourceMetadataUrl = resourceMetadataUrl;
this._scope = scope;
// Only update when the Bearer challenge advertises a URL; a non-Bearer
// scheme (e.g. Negotiate) must not clear a previously-stored value so
// the well-known fallback can still run with the right starting point.
if (resourceMetadataUrl !== undefined) {
this._resourceMetadataUrl = resourceMetadataUrl;
}
if (scope !== undefined) {
this._scope = scope;
}
}
}

Expand All @@ -151,15 +158,22 @@ export class SSEClientTransport implements Transport {
const response = this._last401Response;
this._last401Response = undefined;
this._eventSource?.close();
this._authProvider.onUnauthorized({ response, serverUrl: this._url, fetchFn: this._fetchWithInit }).then(
// onUnauthorized succeeded → retry fresh. Its onerror handles its own onerror?.() + reject.
() => this._startOrAuth().then(resolve, reject),
// onUnauthorized failed → not yet reported.
error => {
this.onerror?.(error);
reject(error);
}
);
this._authProvider
.onUnauthorized({
response,
serverUrl: this._url,
fetchFn: this._fetchWithInit,
resourceMetadataUrl: this._resourceMetadataUrl
})
.then(
// onUnauthorized succeeded → retry fresh. Its onerror handles its own onerror?.() + reject.
() => this._startOrAuth().then(resolve, reject),
// onUnauthorized failed → not yet reported.
error => {
this.onerror?.(error);
reject(error);
}
);
return;
}
const error = new UnauthorizedError();
Expand Down Expand Up @@ -270,15 +284,23 @@ export class SSEClientTransport implements Transport {
if (response.status === 401 && this._authProvider) {
if (response.headers.has('www-authenticate')) {
const { resourceMetadataUrl, scope } = extractWWWAuthenticateParams(response);
this._resourceMetadataUrl = resourceMetadataUrl;
this._scope = scope;
// Only update when the Bearer challenge advertises a URL; a non-Bearer
// scheme (e.g. Negotiate) must not clear a previously-stored value so
// the well-known fallback can still run with the right starting point.
if (resourceMetadataUrl !== undefined) {
this._resourceMetadataUrl = resourceMetadataUrl;
}
if (scope !== undefined) {
this._scope = scope;
}
}

if (this._authProvider.onUnauthorized && !isAuthRetry) {
await this._authProvider.onUnauthorized({
response,
serverUrl: this._url,
fetchFn: this._fetchWithInit
fetchFn: this._fetchWithInit,
resourceMetadataUrl: this._resourceMetadataUrl
});
await response.text?.().catch(() => {});
// Purposely _not_ awaited, so we don't call onerror twice
Expand Down
28 changes: 22 additions & 6 deletions packages/client/src/client/streamableHttp.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,15 +257,23 @@ export class StreamableHTTPClientTransport implements Transport {
if (response.status === 401 && this._authProvider) {
if (response.headers.has('www-authenticate')) {
const { resourceMetadataUrl, scope } = extractWWWAuthenticateParams(response);
this._resourceMetadataUrl = resourceMetadataUrl;
this._scope = scope;
// Only update when the Bearer challenge advertises a URL; a non-Bearer
// scheme (e.g. Negotiate) must not clear a previously-stored value so
// the well-known fallback can still run with the right starting point.
if (resourceMetadataUrl !== undefined) {
this._resourceMetadataUrl = resourceMetadataUrl;
}
if (scope !== undefined) {
this._scope = scope;
}
}

if (this._authProvider.onUnauthorized && !isAuthRetry) {
await this._authProvider.onUnauthorized({
response,
serverUrl: this._url,
fetchFn: this._fetchWithInit
fetchFn: this._fetchWithInit,
resourceMetadataUrl: this._resourceMetadataUrl
});
await response.text?.().catch(() => {});
// Purposely _not_ awaited, so we don't call onerror twice
Expand Down Expand Up @@ -565,15 +573,23 @@ export class StreamableHTTPClientTransport implements Transport {
// Store WWW-Authenticate params for interactive finishAuth() path
if (response.headers.has('www-authenticate')) {
const { resourceMetadataUrl, scope } = extractWWWAuthenticateParams(response);
this._resourceMetadataUrl = resourceMetadataUrl;
this._scope = scope;
// Only update when the Bearer challenge advertises a URL; a non-Bearer
// scheme (e.g. Negotiate) must not clear a previously-stored value so
// the well-known fallback can still run with the right starting point.
if (resourceMetadataUrl !== undefined) {
this._resourceMetadataUrl = resourceMetadataUrl;
}
if (scope !== undefined) {
this._scope = scope;
}
}

if (this._authProvider.onUnauthorized && !isAuthRetry) {
await this._authProvider.onUnauthorized({
response,
serverUrl: this._url,
fetchFn: this._fetchWithInit
fetchFn: this._fetchWithInit,
resourceMetadataUrl: this._resourceMetadataUrl
});
await response.text?.().catch(() => {});
// Purposely _not_ awaited, so we don't call onerror twice
Expand Down
87 changes: 86 additions & 1 deletion packages/client/test/client/streamableHttp.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { JSONRPCMessage, JSONRPCRequest } from '@modelcontextprotocol/core'
import { OAuthError, OAuthErrorCode, SdkError, SdkErrorCode } from '@modelcontextprotocol/core';
import type { Mock, Mocked } from 'vitest';

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

it('falls back to well-known discovery on 401 with non-Bearer WWW-Authenticate (e.g. Negotiate)', async () => {
const message: JSONRPCMessage = {
jsonrpc: '2.0',
method: 'test',
params: {},
id: 'test-id'
};

(globalThis.fetch as Mock)
.mockResolvedValueOnce({
ok: false,
status: 401,
statusText: 'Unauthorized',
headers: new Headers({ 'WWW-Authenticate': 'Negotiate' }),
text: async () => 'Authentication required.'
})
.mockResolvedValue({
ok: false,
status: 404,
text: async () => {
throw 'dont read my body';
}
});

await expect(transport.send(message)).rejects.toThrow(UnauthorizedError);
expect(mockAuthProvider.redirectToAuthorization.mock.calls).toHaveLength(1);
});

it('passes stored resource metadata URL to onUnauthorized context after non-Bearer 401', async () => {
// Regression test: a non-Bearer WWW-Authenticate (e.g. Negotiate) must not wipe
// out a Bearer resource_metadata URL stored from an earlier Bearer challenge.
// The stored URL must reach onUnauthorized via ctx.resourceMetadataUrl so PRM
// discovery can use the right starting point instead of falling back to the root
// well-known URI.
//
// Uses a plain AuthProvider (not OAuthClientProvider) so onUnauthorized is called
// directly by the transport — no adaptOAuthProvider wrapper whose internal closure
// would hide the context from a spy.
const message: JSONRPCMessage = {
jsonrpc: '2.0',
method: 'test',
params: {},
id: 'test-id'
};
const resourceMetadataUrl = 'http://example.com/.well-known/oauth-protected-resource';

const onUnauthorized = vi.fn<AuthProvider['onUnauthorized'] & object>().mockResolvedValue(undefined);
const plainTransport = new StreamableHTTPClientTransport(new URL('http://localhost:1234/mcp'), {
authProvider: { token: async () => 'test-token', onUnauthorized }
});

try {
// First request: Bearer 401 → stores _resourceMetadataUrl; onUnauthorized resolves, retry succeeds
(globalThis.fetch as Mock)
.mockResolvedValueOnce({
ok: false,
status: 401,
headers: new Headers({ 'WWW-Authenticate': `Bearer resource_metadata="${resourceMetadataUrl}"` }),
text: async () => ''
})
.mockResolvedValueOnce({ ok: true, status: 202, headers: new Headers() });

await plainTransport.send(message);
onUnauthorized.mockClear();

// Second request: Negotiate 401 → must NOT clear _resourceMetadataUrl; retry succeeds
(globalThis.fetch as Mock)
.mockResolvedValueOnce({
ok: false,
status: 401,
headers: new Headers({ 'WWW-Authenticate': 'Negotiate' }),
text: async () => ''
})
.mockResolvedValueOnce({ ok: true, status: 202, headers: new Headers() });

await plainTransport.send(message);

// onUnauthorized must receive the stored Bearer URL in ctx.resourceMetadataUrl,
// not undefined, so PRM discovery uses the correct endpoint.
expect(onUnauthorized).toHaveBeenCalledWith(expect.objectContaining({ resourceMetadataUrl: new URL(resourceMetadataUrl) }));
} finally {
await plainTransport.close().catch(() => {});
}
});

it('attempts upscoping on 403 with WWW-Authenticate header', async () => {
const message: JSONRPCMessage = {
jsonrpc: '2.0',
Expand Down
Loading