fix(client): let OAuth-derived Authorization override caller-supplied header (#2475)
Signed-off-by: sanjibani <18418553+sanjibani@users.noreply.github.com> Co-authored-by: sanjibani <18418553+sanjibani@users.noreply.github.com> Co-authored-by: Konstantin Konstantinov <KKonstantinov@users.noreply.github.com> Co-authored-by: Konstantin Konstantinov <konstantin@mach5technology.com>
This commit is contained in:
parent
5ecc791d81
commit
b65426158e
2 changed files with 438 additions and 28 deletions
|
|
@ -303,6 +303,26 @@ describe('SSEClientTransport', () => {
|
|||
expect(lastServerRequest.headers.authorization).toBe(authToken);
|
||||
});
|
||||
|
||||
it('tolerates requestInit.headers set to null by a JavaScript caller', async () => {
|
||||
// The TS type excludes null, but a JS caller or a JSON config forwarded verbatim can
|
||||
// pass it. `new Headers(null)` throws, so the transport must map falsy to undefined.
|
||||
transport = new SSEClientTransport(resourceBaseUrl, {
|
||||
requestInit: { headers: null as unknown as RequestInit['headers'] }
|
||||
});
|
||||
|
||||
await transport.start();
|
||||
expect(lastServerRequest.headers.accept).toBe('text/event-stream');
|
||||
|
||||
const message: JSONRPCMessage = {
|
||||
jsonrpc: '2.0',
|
||||
id: '1',
|
||||
method: 'test',
|
||||
params: {}
|
||||
};
|
||||
await transport.send(message);
|
||||
expect(lastServerRequest.headers['content-type']).toBe('application/json');
|
||||
});
|
||||
|
||||
it('passes custom headers to fetch requests', async () => {
|
||||
const customHeaders = {
|
||||
Authorization: 'Bearer test-token',
|
||||
|
|
@ -660,6 +680,123 @@ describe('SSEClientTransport', () => {
|
|||
expect(lastServerRequest.headers['x-custom-header']).toBe('custom-value');
|
||||
});
|
||||
|
||||
it('lets the auth provider token override a stale caller-supplied Authorization header', async () => {
|
||||
// Regression test for #2208: transport-managed headers are merged on top of
|
||||
// requestInit.headers, so a static Authorization placeholder (e.g. an env-var
|
||||
// API key) gives way to the provider's token on both the SSE GET and POSTs.
|
||||
mockAuthProvider.tokens.mockResolvedValue({
|
||||
access_token: 'fresh-token',
|
||||
token_type: 'Bearer'
|
||||
});
|
||||
|
||||
transport = new SSEClientTransport(resourceBaseUrl, {
|
||||
authProvider: mockAuthProvider,
|
||||
requestInit: {
|
||||
headers: {
|
||||
Authorization: 'Bearer stale-placeholder',
|
||||
'X-Custom-Header': 'custom-value'
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
await transport.start();
|
||||
|
||||
// SSE GET
|
||||
expect(lastServerRequest.headers.authorization).toBe('Bearer fresh-token');
|
||||
expect(lastServerRequest.headers['x-custom-header']).toBe('custom-value');
|
||||
|
||||
const message: JSONRPCMessage = {
|
||||
jsonrpc: '2.0',
|
||||
id: '1',
|
||||
method: 'test',
|
||||
params: {}
|
||||
};
|
||||
|
||||
await transport.send(message);
|
||||
|
||||
// POST
|
||||
expect(lastServerRequest.headers.authorization).toBe('Bearer fresh-token');
|
||||
expect(lastServerRequest.headers['x-custom-header']).toBe('custom-value');
|
||||
});
|
||||
|
||||
it('replaces a caller-supplied Authorization header regardless of name casing (Headers instance)', async () => {
|
||||
// #2208 follow-up: `Headers` normalizes names to lowercase; the transport must
|
||||
// still send exactly its own token, not a combined two-token value.
|
||||
mockAuthProvider.tokens.mockResolvedValue({
|
||||
access_token: 'fresh-token',
|
||||
token_type: 'Bearer'
|
||||
});
|
||||
|
||||
transport = new SSEClientTransport(resourceBaseUrl, {
|
||||
authProvider: mockAuthProvider,
|
||||
requestInit: {
|
||||
headers: new Headers({
|
||||
authorization: 'Bearer stale-placeholder',
|
||||
'x-custom-header': 'custom-value'
|
||||
})
|
||||
}
|
||||
});
|
||||
|
||||
await transport.start();
|
||||
|
||||
expect(lastServerRequest.headers.authorization).toBe('Bearer fresh-token');
|
||||
expect(lastServerRequest.headers['x-custom-header']).toBe('custom-value');
|
||||
|
||||
const message: JSONRPCMessage = {
|
||||
jsonrpc: '2.0',
|
||||
id: '1',
|
||||
method: 'test',
|
||||
params: {}
|
||||
};
|
||||
|
||||
await transport.send(message);
|
||||
|
||||
expect(lastServerRequest.headers.authorization).toBe('Bearer fresh-token');
|
||||
expect(lastServerRequest.headers['x-custom-header']).toBe('custom-value');
|
||||
});
|
||||
|
||||
it('keeps Fetch Headers combine semantics for a repeated name in a tuple array', async () => {
|
||||
// A repeated name in a tuple array is the one `HeadersInit` form that can express a
|
||||
// multi-valued header, and `fetch(url, { headers: [['x', 'a'], ['x', 'b']] })` sends
|
||||
// "x: a, b". The transport builds its headers with the same `Headers` constructor, so
|
||||
// a caller-supplied repeated name is combined exactly as a direct `fetch` would, while
|
||||
// a repeated *transport-managed* name is still replaced outright by the transport's
|
||||
// own value rather than combined with it.
|
||||
mockAuthProvider.tokens.mockResolvedValue({
|
||||
access_token: 'fresh-token',
|
||||
token_type: 'Bearer'
|
||||
});
|
||||
|
||||
transport = new SSEClientTransport(resourceBaseUrl, {
|
||||
authProvider: mockAuthProvider,
|
||||
requestInit: {
|
||||
headers: [
|
||||
['Authorization', 'Bearer stale-1'],
|
||||
['Authorization', 'Bearer stale-2'],
|
||||
['x-multi', 'a'],
|
||||
['x-multi', 'b']
|
||||
]
|
||||
}
|
||||
});
|
||||
|
||||
await transport.start();
|
||||
|
||||
expect(lastServerRequest.headers.authorization).toBe('Bearer fresh-token');
|
||||
expect(lastServerRequest.headers['x-multi']).toBe('a, b');
|
||||
|
||||
const message: JSONRPCMessage = {
|
||||
jsonrpc: '2.0',
|
||||
id: '1',
|
||||
method: 'test',
|
||||
params: {}
|
||||
};
|
||||
|
||||
await transport.send(message);
|
||||
|
||||
expect(lastServerRequest.headers.authorization).toBe('Bearer fresh-token');
|
||||
expect(lastServerRequest.headers['x-multi']).toBe('a, b');
|
||||
});
|
||||
|
||||
it('refreshes expired token during SSE connection', async () => {
|
||||
// Mock tokens() to return expired token until saveTokens is called
|
||||
let currentTokens: OAuthTokens = {
|
||||
|
|
|
|||
|
|
@ -702,6 +702,169 @@ describe('StreamableHTTPClientTransport', () => {
|
|||
expect(globalThis.fetch).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('OAuth-derived Authorization overrides a stale caller-supplied Authorization', async () => {
|
||||
// Regression test for #2208: the SDK-derived common headers must be
|
||||
// merged on top of caller-supplied headers, so OAuth-derived tokens
|
||||
// (or any SDK-computed value) win over a stale placeholder the
|
||||
// caller might have set in `requestInit.headers` (e.g. an env-var
|
||||
// API key that's no longer valid).
|
||||
const tokens: OAuthTokens = {
|
||||
access_token: 'oauth-access-token',
|
||||
token_type: 'Bearer'
|
||||
};
|
||||
mockAuthProvider.tokens.mockResolvedValue(tokens);
|
||||
const requestInit = {
|
||||
headers: {
|
||||
Authorization: 'Bearer stale-placeholder',
|
||||
'mcp-protocol-version': 'caller-supplied',
|
||||
'X-Caller-Header': 'preserved'
|
||||
}
|
||||
};
|
||||
transport = new StreamableHTTPClientTransport(new URL('http://localhost:1234/mcp'), {
|
||||
requestInit,
|
||||
authProvider: mockAuthProvider
|
||||
});
|
||||
transport.setProtocolVersion('2025-03-26');
|
||||
|
||||
let actualReqInit: RequestInit = {};
|
||||
(globalThis.fetch as Mock).mockImplementation(async (_url, reqInit) => {
|
||||
actualReqInit = reqInit;
|
||||
return new Response(null, { status: 200, headers: { 'content-type': 'text/event-stream' } });
|
||||
});
|
||||
|
||||
await transport.start();
|
||||
await transport['_startOrAuthSse']({});
|
||||
// On the SSE GET: the OAuth-derived token wins over the stale placeholder, and so
|
||||
// does the transport-managed protocol version.
|
||||
expect((actualReqInit.headers as Headers).get('authorization')).toBe('Bearer oauth-access-token');
|
||||
expect((actualReqInit.headers as Headers).get('mcp-protocol-version')).toBe('2025-03-26');
|
||||
// Caller-supplied non-auth headers still pass through.
|
||||
expect((actualReqInit.headers as Headers).get('x-caller-header')).toBe('preserved');
|
||||
|
||||
// Same precedence on POST.
|
||||
await transport.send({ jsonrpc: '2.0', method: 'test', params: {} } as JSONRPCMessage);
|
||||
expect((actualReqInit.headers as Headers).get('authorization')).toBe('Bearer oauth-access-token');
|
||||
expect((actualReqInit.headers as Headers).get('mcp-protocol-version')).toBe('2025-03-26');
|
||||
expect((actualReqInit.headers as Headers).get('x-caller-header')).toBe('preserved');
|
||||
expect(globalThis.fetch).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it.each([
|
||||
[
|
||||
'a Headers instance',
|
||||
(): NonNullable<RequestInit['headers']> =>
|
||||
new Headers({
|
||||
authorization: 'Bearer stale-placeholder',
|
||||
'MCP-Protocol-Version': 'caller-supplied',
|
||||
'x-caller-header': 'preserved'
|
||||
})
|
||||
],
|
||||
[
|
||||
'a lowercase plain object',
|
||||
(): NonNullable<RequestInit['headers']> => ({
|
||||
authorization: 'Bearer stale-placeholder',
|
||||
'MCP-Protocol-Version': 'caller-supplied',
|
||||
'x-caller-header': 'preserved'
|
||||
})
|
||||
],
|
||||
[
|
||||
'a tuple array',
|
||||
(): NonNullable<RequestInit['headers']> => [
|
||||
['authorization', 'Bearer stale-placeholder'],
|
||||
['MCP-Protocol-Version', 'caller-supplied'],
|
||||
['x-caller-header', 'preserved']
|
||||
]
|
||||
]
|
||||
])('transport-managed headers replace caller-supplied ones regardless of name casing (%s)', async (_label, makeHeaders) => {
|
||||
// #2208 follow-up: header names compare case-insensitively. A `Headers` instance
|
||||
// normalizes names to lowercase and a caller may spell them any way; a key-exact
|
||||
// object merge would keep both spellings and the Fetch `Headers` constructor would
|
||||
// then combine them into one two-token value. The transport must send exactly its
|
||||
// own value.
|
||||
mockAuthProvider.tokens.mockResolvedValue({ access_token: 'oauth-access-token', token_type: 'Bearer' });
|
||||
transport = new StreamableHTTPClientTransport(new URL('http://localhost:1234/mcp'), {
|
||||
requestInit: { headers: makeHeaders() },
|
||||
authProvider: mockAuthProvider
|
||||
});
|
||||
transport.setProtocolVersion('2025-03-26');
|
||||
|
||||
let actualReqInit: RequestInit = {};
|
||||
(globalThis.fetch as Mock).mockImplementation(async (_url, reqInit) => {
|
||||
actualReqInit = reqInit;
|
||||
return new Response(null, { status: 200, headers: { 'content-type': 'text/event-stream' } });
|
||||
});
|
||||
|
||||
await transport.start();
|
||||
await transport['_startOrAuthSse']({});
|
||||
expect((actualReqInit.headers as Headers).get('authorization')).toBe('Bearer oauth-access-token');
|
||||
expect((actualReqInit.headers as Headers).get('mcp-protocol-version')).toBe('2025-03-26');
|
||||
expect((actualReqInit.headers as Headers).get('x-caller-header')).toBe('preserved');
|
||||
|
||||
await transport.send({ jsonrpc: '2.0', method: 'test', params: {} } as JSONRPCMessage);
|
||||
expect((actualReqInit.headers as Headers).get('authorization')).toBe('Bearer oauth-access-token');
|
||||
expect((actualReqInit.headers as Headers).get('mcp-protocol-version')).toBe('2025-03-26');
|
||||
expect((actualReqInit.headers as Headers).get('x-caller-header')).toBe('preserved');
|
||||
});
|
||||
|
||||
it('keeps Fetch Headers combine semantics for a repeated name in a tuple array', async () => {
|
||||
// A repeated name in a tuple array is the one `HeadersInit` form that can express a
|
||||
// multi-valued header, and `fetch(url, { headers: [['x', 'a'], ['x', 'b']] })` sends
|
||||
// "x: a, b". The transport builds its headers with the same `Headers` constructor, so
|
||||
// a caller-supplied repeated name is combined exactly as a direct `fetch` would, while
|
||||
// a repeated *transport-managed* name is still replaced outright by the transport's
|
||||
// own value rather than combined with it.
|
||||
mockAuthProvider.tokens.mockResolvedValue({ access_token: 'oauth-access-token', token_type: 'Bearer' });
|
||||
transport = new StreamableHTTPClientTransport(new URL('http://localhost:1234/mcp'), {
|
||||
requestInit: {
|
||||
headers: [
|
||||
['Authorization', 'Bearer stale-1'],
|
||||
['Authorization', 'Bearer stale-2'],
|
||||
['x-multi', 'a'],
|
||||
['x-multi', 'b']
|
||||
]
|
||||
},
|
||||
authProvider: mockAuthProvider
|
||||
});
|
||||
|
||||
let actualReqInit: RequestInit = {};
|
||||
(globalThis.fetch as Mock).mockImplementation(async (_url, reqInit) => {
|
||||
actualReqInit = reqInit;
|
||||
return new Response(null, { status: 200, headers: { 'content-type': 'text/event-stream' } });
|
||||
});
|
||||
|
||||
await transport.start();
|
||||
await transport['_startOrAuthSse']({});
|
||||
expect((actualReqInit.headers as Headers).get('authorization')).toBe('Bearer oauth-access-token');
|
||||
expect((actualReqInit.headers as Headers).get('x-multi')).toBe('a, b');
|
||||
|
||||
await transport.send({ jsonrpc: '2.0', method: 'test', params: {} } as JSONRPCMessage);
|
||||
expect((actualReqInit.headers as Headers).get('authorization')).toBe('Bearer oauth-access-token');
|
||||
expect((actualReqInit.headers as Headers).get('x-multi')).toBe('a, b');
|
||||
});
|
||||
|
||||
it('tolerates requestInit.headers set to null by a JavaScript caller', async () => {
|
||||
// The TS type excludes null, but a JS caller or a JSON config forwarded verbatim can
|
||||
// pass it. `new Headers(null)` throws, so the transport must map falsy to undefined.
|
||||
transport = new StreamableHTTPClientTransport(new URL('http://localhost:1234/mcp'), {
|
||||
requestInit: { headers: null as unknown as RequestInit['headers'] }
|
||||
});
|
||||
|
||||
let actualReqInit: RequestInit = {};
|
||||
(globalThis.fetch as Mock).mockImplementation(async (_url, reqInit) => {
|
||||
actualReqInit = reqInit;
|
||||
return new Response(null, { status: 200, headers: { 'content-type': 'text/event-stream' } });
|
||||
});
|
||||
|
||||
await transport.start();
|
||||
await transport['_startOrAuthSse']({});
|
||||
expect(actualReqInit.headers).toBeInstanceOf(Headers);
|
||||
expect((actualReqInit.headers as Headers).get('accept')).toContain('text/event-stream');
|
||||
|
||||
await transport.send({ jsonrpc: '2.0', method: 'test', params: {} } as JSONRPCMessage);
|
||||
expect(actualReqInit.headers).toBeInstanceOf(Headers);
|
||||
expect(globalThis.fetch).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('should always send specified custom headers (Headers class)', async () => {
|
||||
const requestInit = {
|
||||
headers: new Headers({
|
||||
|
|
|
|||
Loading…
Reference in a new issue