Skip to content

Commit ab7176f

Browse files
committed
Coordinate MCP authorization recovery per connection
1 parent 12d5c45 commit ab7176f

3 files changed

Lines changed: 344 additions & 83 deletions

File tree

‎src/mcp/client-auth-policy.test.ts‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ let blockTokenExchange = false;
1717
let tokenExchangeSignals: (AbortSignal | null | undefined)[] = [];
1818
let tokenExchangeAborts = 0;
1919
let lastTransportAuth: (() => Promise<void>) | undefined;
20+
let lastTransportRedirect: (() => Promise<void>) | undefined;
2021
let tokenRefreshSignals: (AbortSignal | null | undefined)[] = [];
2122
let tokenRefreshAborts = 0;
2223

@@ -35,9 +36,10 @@ function hangUntilAbort(
3536
});
3637
}
3738

39+
const redirectToAuthorization = () => undefined;
3840
const authProvider = {
3941
resetAuthorization: async () => undefined,
40-
redirectToAuthorization: () => undefined,
42+
redirectToAuthorization,
4143
};
4244

4345
await withMockedModule(
@@ -46,7 +48,10 @@ await withMockedModule(
4648
...real,
4749
Client: class {
4850
async connect(): Promise<void> {
49-
if (clientConnectError !== undefined) throw clientConnectError;
51+
if (clientConnectError !== undefined) {
52+
if (clientConnectError instanceof UnauthorizedError) await lastTransportRedirect?.();
53+
throw clientConnectError;
54+
}
5055
}
5156
async listTools(
5257
_params?: unknown,
@@ -86,12 +91,18 @@ await withMockedModule(
8691
constructor(
8792
_url: URL,
8893
private readonly options?: {
94+
authProvider?: { redirectToAuthorization?: (url: URL) => void | Promise<void> };
8995
requestInit?: RequestInit;
9096
fetch?: (url: string | URL, init?: RequestInit) => Promise<Response>;
9197
},
9298
) {
9399
transportOptions.push(options);
94100
lastTransportAuth = () => this.auth();
101+
lastTransportRedirect = async () => {
102+
await this.options?.authProvider?.redirectToAuthorization?.(
103+
new URL("https://auth.test/authorize"),
104+
);
105+
};
95106
}
96107
async finishAuth(): Promise<void> {
97108
const signal = this.options?.requestInit?.signal;
@@ -177,8 +188,10 @@ describe("HTTP MCP auth policy", () => {
177188
tokenExchangeSignals = [];
178189
tokenExchangeAborts = 0;
179190
lastTransportAuth = undefined;
191+
lastTransportRedirect = undefined;
180192
tokenRefreshSignals = [];
181193
tokenRefreshAborts = 0;
194+
authProvider.redirectToAuthorization = redirectToAuthorization;
182195
});
183196

184197
test("built-in anonymous Exa treats 401 as a normal failure without OAuth machinery", async () => {

‎src/mcp/client-auth-reauth-cap.test.ts‎

Lines changed: 163 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,23 @@ import { withMockedModule } from "../../tests/helpers/mock-module.js";
55
let finishAuthCalls = 0;
66
let finishAuthError: Error | undefined = new Error("finishAuth exploded");
77
let connectFailuresLeft = 0;
8+
let listFailuresLeft = 0;
89
let callFailuresLeft = 0;
10+
let callToolCalls = 0;
911
let redirectsPerFailure = 1;
12+
let redirectOnListFailure = false;
1013
let redirectConcurrently = false;
1114
let providerCreates = 0;
1215
let refreshCalls = 0;
1316
let refreshSucceeds = false;
1417
let authEvents: string[] = [];
1518
let authURLCount = 0;
19+
let authorizedCount = 0;
20+
let waitForCodeCalls = 0;
21+
let refreshGate: Promise<void> | undefined;
22+
let releaseRefresh: (() => void) | undefined;
23+
let callbackGate: Promise<void> | undefined;
24+
let releaseCallback: (() => void) | undefined;
1625
let liveProvider: { redirectToAuthorization?: (url: URL) => void | Promise<void> } | undefined;
1726

1827
const fakeProvider = {
@@ -21,6 +30,7 @@ const fakeProvider = {
2130
refreshToken: async () => {
2231
refreshCalls += 1;
2332
authEvents.push("refresh");
33+
await refreshGate;
2434
if (!refreshSucceeds) throw new UnauthorizedError("refresh rejected");
2535
return { access_token: "fresh", refresh_token: "refresh-me" };
2636
},
@@ -38,6 +48,22 @@ async function emitRedirects(
3848
}
3949
}
4050

51+
function waitForGate(signal: AbortSignal): Promise<void> {
52+
if (callbackGate === undefined) return Promise.resolve();
53+
if (signal.aborted) return Promise.reject(signal.reason);
54+
return new Promise((resolve, reject) => {
55+
const onAbort = () => {
56+
signal.removeEventListener("abort", onAbort);
57+
reject(signal.reason);
58+
};
59+
signal.addEventListener("abort", onAbort, { once: true });
60+
void callbackGate?.then(() => {
61+
signal.removeEventListener("abort", onAbort);
62+
resolve();
63+
});
64+
});
65+
}
66+
4167
await withMockedModule(
4268
import.meta.resolve("@modelcontextprotocol/sdk/client/index.js"),
4369
(real: typeof import("@modelcontextprotocol/sdk/client/index.js")) => ({
@@ -56,9 +82,17 @@ await withMockedModule(
5682
}
5783
}
5884
async listTools(): Promise<{ tools: [] }> {
85+
if (listFailuresLeft > 0) {
86+
listFailuresLeft -= 1;
87+
if (redirectOnListFailure) {
88+
await liveProvider?.redirectToAuthorization?.(new URL("https://auth.test/authorize"));
89+
}
90+
throw new UnauthorizedError("authorization required");
91+
}
5992
return { tools: [] };
6093
}
6194
async callTool(): Promise<{ content: [] }> {
95+
callToolCalls += 1;
6296
if (callFailuresLeft > 0) {
6397
callFailuresLeft -= 1;
6498
await emitRedirects(liveProvider);
@@ -107,7 +141,11 @@ await withMockedModule(
107141
startCallbackServer: async () => ({
108142
redirectUrl: "http://127.0.0.1:12345/callback",
109143
expectState: () => undefined,
110-
waitForCode: async () => "code",
144+
waitForCode: async (signal: AbortSignal) => {
145+
waitForCodeCalls += 1;
146+
await waitForGate(signal);
147+
return "code";
148+
},
111149
close: () => undefined,
112150
}),
113151
}),
@@ -154,8 +192,11 @@ async function connectWithAuthPrompt(): Promise<{ ok: boolean; error?: string }>
154192
describe("HTTP MCP re-auth loop prevention", () => {
155193
beforeEach(() => {
156194
connectFailuresLeft = 0;
195+
listFailuresLeft = 0;
157196
callFailuresLeft = 0;
197+
callToolCalls = 0;
158198
redirectsPerFailure = 1;
199+
redirectOnListFailure = false;
159200
redirectConcurrently = false;
160201
finishAuthCalls = 0;
161202
finishAuthError = new Error("finishAuth exploded");
@@ -165,6 +206,12 @@ describe("HTTP MCP re-auth loop prevention", () => {
165206
refreshSucceeds = false;
166207
authEvents = [];
167208
authURLCount = 0;
209+
authorizedCount = 0;
210+
waitForCodeCalls = 0;
211+
refreshGate = undefined;
212+
releaseRefresh = undefined;
213+
callbackGate = undefined;
214+
releaseCallback = undefined;
168215
resetBrowserAuthState();
169216
setSystemTime();
170217
});
@@ -187,6 +234,121 @@ describe("HTTP MCP re-auth loop prevention", () => {
187234
expect(authURLCount).toBe(0);
188235
});
189236

237+
test("rejects promptly when refresh and an auth probe fail without emitting a URL", async () => {
238+
const connected = await connectMCPServer(config, {
239+
onAuthURL: () => (authURLCount += 1),
240+
onAuthorized: () => (authorizedCount += 1),
241+
});
242+
expect(connected.ok).toBe(true);
243+
if (!connected.ok) return;
244+
redirectsPerFailure = 0;
245+
callFailuresLeft = 1;
246+
listFailuresLeft = 1;
247+
248+
await expect(connected.client.call("ping", {}, new AbortController().signal)).rejects.toThrow(
249+
"authorization required",
250+
);
251+
252+
expect(refreshCalls).toBe(1);
253+
expect(authURLCount).toBe(0);
254+
expect(waitForCodeCalls).toBe(0);
255+
expect(authorizedCount).toBe(0);
256+
});
257+
258+
test("shares live-call recovery across concurrent unauthorized calls", async () => {
259+
finishAuthError = undefined;
260+
const connected = await connectMCPServer(config, {
261+
onAuthURL: () => (authURLCount += 1),
262+
onAuthorized: () => (authorizedCount += 1),
263+
});
264+
expect(connected.ok).toBe(true);
265+
if (!connected.ok) return;
266+
redirectsPerFailure = 0;
267+
callFailuresLeft = 2;
268+
listFailuresLeft = 1;
269+
redirectOnListFailure = true;
270+
271+
const calls = [
272+
connected.client.call("first", { value: 1 }, new AbortController().signal),
273+
connected.client.call("second", { value: 2 }, new AbortController().signal),
274+
];
275+
276+
await expect(Promise.all(calls)).resolves.toEqual(["", ""]);
277+
expect(refreshCalls).toBe(1);
278+
expect(authURLCount).toBe(1);
279+
expect(waitForCodeCalls).toBe(1);
280+
expect(finishAuthCalls).toBe(1);
281+
expect(callToolCalls).toBe(4);
282+
expect(authorizedCount).toBe(1);
283+
});
284+
285+
test("does not start browser fallback while shared refresh is pending", async () => {
286+
const connected = await connectMCPServer(config, { onAuthURL: () => (authURLCount += 1) });
287+
expect(connected.ok).toBe(true);
288+
if (!connected.ok) return;
289+
refreshGate = new Promise((resolve) => {
290+
releaseRefresh = resolve;
291+
});
292+
redirectsPerFailure = 0;
293+
callFailuresLeft = 1;
294+
const first = connected.client.call("first", {}, new AbortController().signal);
295+
while (refreshCalls === 0) await Promise.resolve();
296+
297+
redirectsPerFailure = 1;
298+
callFailuresLeft = 1;
299+
const second = connected.client.call("second", {}, new AbortController().signal);
300+
await Promise.resolve();
301+
expect(authURLCount).toBe(0);
302+
303+
releaseRefresh?.();
304+
await expect(Promise.all([first, second])).rejects.toThrow("finishAuth exploded");
305+
expect(refreshCalls).toBe(1);
306+
expect(authURLCount).toBe(1);
307+
expect(waitForCodeCalls).toBe(1);
308+
});
309+
310+
test("caller abort does not cancel shared recovery for another call", async () => {
311+
finishAuthError = undefined;
312+
callbackGate = new Promise((resolve) => {
313+
releaseCallback = resolve;
314+
});
315+
const connected = await connectMCPServer(config, { onAuthURL: () => (authURLCount += 1) });
316+
expect(connected.ok).toBe(true);
317+
if (!connected.ok) return;
318+
callFailuresLeft = 2;
319+
const firstAbort = new AbortController();
320+
const first = connected.client.call("first", {}, firstAbort.signal);
321+
const second = connected.client.call("second", {}, new AbortController().signal);
322+
while (waitForCodeCalls === 0) await Promise.resolve();
323+
324+
firstAbort.abort(new Error("caller stopped"));
325+
await expect(first).rejects.toThrow("caller stopped");
326+
releaseCallback?.();
327+
328+
await expect(second).resolves.toBe("");
329+
expect(waitForCodeCalls).toBe(1);
330+
expect(finishAuthCalls).toBe(1);
331+
expect(authURLCount).toBe(1);
332+
});
333+
334+
test("client close aborts the shared callback waiter", async () => {
335+
finishAuthError = undefined;
336+
callbackGate = new Promise((resolve) => {
337+
releaseCallback = resolve;
338+
});
339+
const connected = await connectMCPServer(config, { onAuthURL: () => (authURLCount += 1) });
340+
expect(connected.ok).toBe(true);
341+
if (!connected.ok) return;
342+
callFailuresLeft = 1;
343+
const call = connected.client.call("ping", {}, new AbortController().signal);
344+
while (waitForCodeCalls === 0) await Promise.resolve();
345+
346+
await connected.client.close();
347+
348+
await expect(call).rejects.toHaveProperty("name", "AbortError");
349+
expect(finishAuthCalls).toBe(0);
350+
});
351+
190352
test("does not repeat the SDK refresh after redirecting to authorization", async () => {
191353
finishAuthError = undefined;
192354
connectFailuresLeft = 1;

0 commit comments

Comments
 (0)