Skip to content

Commit 7416af0

Browse files
jblzclaude
andcommitted
feat(client): revoke OAuth tokens on Disconnect per RFC 7009
The Disconnect button wipes local OAuth state but never tells the authorization server. Tokens stay valid until natural expiry, leaving "tombstone" sessions for downstream services to clean up. Add a best-effort `revokeTokens` helper that POSTs to the AS's `revocation_endpoint` (RFC 8414) on disconnect, preferring the refresh token so the AS cascade-invalidates associated access tokens per RFC 7009 §2.1. A 3s timeout and swallowed errors keep an unresponsive AS from blocking the UI; if the AS doesn't advertise a `revocation_endpoint`, the helper no-ops. Routes through `createProxyFetch` when `connectionType === "proxy"`, matching how every other OAuth call in `useConnection` is made. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent f18775a commit 7416af0

4 files changed

Lines changed: 321 additions & 2 deletions

File tree

client/src/lib/__tests__/auth.test.ts

Lines changed: 182 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
import { discoverScopes } from "../auth";
1+
import { discoverScopes, revokeTokens } from "../auth";
22
import { discoverAuthorizationServerMetadata } from "@modelcontextprotocol/sdk/client/auth.js";
3+
import { SESSION_KEYS, getServerSpecificKey } from "../constants";
34

45
jest.mock("@modelcontextprotocol/sdk/client/auth.js", () => ({
56
discoverAuthorizationServerMetadata: jest.fn(),
@@ -156,3 +157,183 @@ describe("discoverScopes", () => {
156157
},
157158
);
158159
});
160+
161+
describe("revokeTokens", () => {
162+
const serverUrl = "https://example.com";
163+
const revocationEndpoint = "https://test.com/revoke";
164+
const metadataWithRevocation = {
165+
...baseMetadata,
166+
revocation_endpoint: revocationEndpoint,
167+
};
168+
169+
const seedTokens = (tokens: {
170+
access_token: string;
171+
token_type?: string;
172+
refresh_token?: string;
173+
}) => {
174+
sessionStorage.setItem(
175+
getServerSpecificKey(SESSION_KEYS.TOKENS, serverUrl),
176+
JSON.stringify({ token_type: "Bearer", ...tokens }),
177+
);
178+
};
179+
180+
const seedClientInfo = (
181+
client_id: string,
182+
{ isPreregistered = false } = {},
183+
) => {
184+
const key = getServerSpecificKey(
185+
isPreregistered
186+
? SESSION_KEYS.PREREGISTERED_CLIENT_INFORMATION
187+
: SESSION_KEYS.CLIENT_INFORMATION,
188+
serverUrl,
189+
);
190+
sessionStorage.setItem(key, JSON.stringify({ client_id }));
191+
};
192+
193+
const parseRevokeBody = (call: [unknown, RequestInit | undefined]) => {
194+
const init = call[1];
195+
return new URLSearchParams(init?.body as string);
196+
};
197+
198+
let warnSpy: jest.SpyInstance;
199+
let debugSpy: jest.SpyInstance;
200+
201+
beforeEach(() => {
202+
jest.clearAllMocks();
203+
sessionStorage.clear();
204+
warnSpy = jest.spyOn(console, "warn").mockImplementation(() => {});
205+
debugSpy = jest.spyOn(console, "debug").mockImplementation(() => {});
206+
});
207+
208+
afterEach(() => {
209+
warnSpy.mockRestore();
210+
debugSpy.mockRestore();
211+
});
212+
213+
it("posts refresh_token to revocation_endpoint when available, includes client_id", async () => {
214+
mockDiscoverAuth.mockResolvedValue(metadataWithRevocation);
215+
seedTokens({ access_token: "at-123", refresh_token: "rt-456" });
216+
seedClientInfo("client-xyz");
217+
const fetchFn = jest
218+
.fn<Promise<Response>, [RequestInfo | URL, RequestInit?]>()
219+
.mockResolvedValue(new Response(null, { status: 200 }));
220+
221+
await revokeTokens({ serverUrl, fetchFn });
222+
223+
expect(fetchFn).toHaveBeenCalledTimes(1);
224+
const [url, init] = fetchFn.mock.calls[0];
225+
expect(url).toBe(revocationEndpoint);
226+
expect(init?.method).toBe("POST");
227+
expect((init?.headers as Record<string, string>)["Content-Type"]).toBe(
228+
"application/x-www-form-urlencoded",
229+
);
230+
const body = parseRevokeBody(fetchFn.mock.calls[0]);
231+
expect(body.get("token")).toBe("rt-456");
232+
expect(body.get("token_type_hint")).toBe("refresh_token");
233+
expect(body.get("client_id")).toBe("client-xyz");
234+
});
235+
236+
it("prefers preregistered client_id over dynamic", async () => {
237+
mockDiscoverAuth.mockResolvedValue(metadataWithRevocation);
238+
seedTokens({ access_token: "at-123", refresh_token: "rt-456" });
239+
seedClientInfo("dynamic-client");
240+
seedClientInfo("preregistered-client", { isPreregistered: true });
241+
const fetchFn = jest
242+
.fn<Promise<Response>, [RequestInfo | URL, RequestInit?]>()
243+
.mockResolvedValue(new Response(null, { status: 200 }));
244+
245+
await revokeTokens({ serverUrl, fetchFn });
246+
247+
const body = parseRevokeBody(fetchFn.mock.calls[0]);
248+
expect(body.get("client_id")).toBe("preregistered-client");
249+
});
250+
251+
it("falls back to access_token when no refresh_token is present", async () => {
252+
mockDiscoverAuth.mockResolvedValue(metadataWithRevocation);
253+
seedTokens({ access_token: "at-only" });
254+
const fetchFn = jest
255+
.fn<Promise<Response>, [RequestInfo | URL, RequestInit?]>()
256+
.mockResolvedValue(new Response(null, { status: 200 }));
257+
258+
await revokeTokens({ serverUrl, fetchFn });
259+
260+
expect(fetchFn).toHaveBeenCalledTimes(1);
261+
const body = parseRevokeBody(fetchFn.mock.calls[0]);
262+
expect(body.get("token")).toBe("at-only");
263+
expect(body.get("token_type_hint")).toBe("access_token");
264+
expect(body.get("client_id")).toBeNull();
265+
});
266+
267+
it("no-ops when no tokens are stored", async () => {
268+
const fetchFn = jest.fn<
269+
Promise<Response>,
270+
[RequestInfo | URL, RequestInit?]
271+
>();
272+
273+
await revokeTokens({ serverUrl, fetchFn });
274+
275+
expect(fetchFn).not.toHaveBeenCalled();
276+
expect(mockDiscoverAuth).not.toHaveBeenCalled();
277+
});
278+
279+
it("no-ops when AS metadata has no revocation_endpoint", async () => {
280+
mockDiscoverAuth.mockResolvedValue(baseMetadata);
281+
seedTokens({ access_token: "at-123", refresh_token: "rt-456" });
282+
const fetchFn = jest.fn<
283+
Promise<Response>,
284+
[RequestInfo | URL, RequestInit?]
285+
>();
286+
287+
await revokeTokens({ serverUrl, fetchFn });
288+
289+
expect(fetchFn).not.toHaveBeenCalled();
290+
});
291+
292+
it("swallows fetch rejection and logs a warning", async () => {
293+
mockDiscoverAuth.mockResolvedValue(metadataWithRevocation);
294+
seedTokens({ access_token: "at-123", refresh_token: "rt-456" });
295+
const fetchFn = jest
296+
.fn<Promise<Response>, [RequestInfo | URL, RequestInit?]>()
297+
.mockRejectedValue(new Error("network down"));
298+
299+
await expect(revokeTokens({ serverUrl, fetchFn })).resolves.toBeUndefined();
300+
expect(warnSpy).toHaveBeenCalledWith(
301+
"Token revocation failed (best-effort):",
302+
expect.any(Error),
303+
);
304+
});
305+
306+
it("treats non-2xx response as a soft failure without throwing", async () => {
307+
mockDiscoverAuth.mockResolvedValue(metadataWithRevocation);
308+
seedTokens({ access_token: "at-123", refresh_token: "rt-456" });
309+
const fetchFn = jest
310+
.fn<Promise<Response>, [RequestInfo | URL, RequestInit?]>()
311+
.mockResolvedValue(
312+
new Response("nope", { status: 400, statusText: "Bad Request" }),
313+
);
314+
315+
await expect(revokeTokens({ serverUrl, fetchFn })).resolves.toBeUndefined();
316+
expect(warnSpy).toHaveBeenCalledWith(
317+
expect.stringContaining("Token revocation responded 400"),
318+
);
319+
});
320+
321+
it("uses the provided fetchFn, not the global fetch", async () => {
322+
mockDiscoverAuth.mockResolvedValue(metadataWithRevocation);
323+
seedTokens({ access_token: "at-123", refresh_token: "rt-456" });
324+
const globalFetchSpy = jest
325+
.spyOn(globalThis, "fetch")
326+
.mockResolvedValue(new Response(null, { status: 200 }));
327+
const fetchFn = jest
328+
.fn<Promise<Response>, [RequestInfo | URL, RequestInit?]>()
329+
.mockResolvedValue(new Response(null, { status: 200 }));
330+
331+
try {
332+
await revokeTokens({ serverUrl, fetchFn });
333+
expect(fetchFn).toHaveBeenCalledTimes(1);
334+
expect(globalFetchSpy).not.toHaveBeenCalled();
335+
} finally {
336+
globalFetchSpy.mockRestore();
337+
}
338+
});
339+
});

client/src/lib/auth.ts

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,92 @@ export const clearScopeFromSessionStorage = (serverUrl: string) => {
129129
sessionStorage.removeItem(key);
130130
};
131131

132+
/**
133+
* Best-effort RFC 7009 token revocation. Called on user-initiated disconnect so
134+
* the authorization server can invalidate the access/refresh token rather than
135+
* waiting for natural expiry. Per RFC 7009 §2.1, revoking a refresh token also
136+
* invalidates associated access tokens, so we prefer the refresh token when
137+
* present and fall back to the access token otherwise.
138+
*
139+
* Never throws: if there are no saved tokens, no advertised revocation_endpoint,
140+
* or the POST fails for any reason, this resolves quietly. A 3s timeout keeps a
141+
* slow AS from blocking the UI.
142+
*/
143+
export const revokeTokens = async ({
144+
serverUrl,
145+
fetchFn,
146+
}: {
147+
serverUrl: string;
148+
fetchFn?: typeof fetch;
149+
}): Promise<void> => {
150+
try {
151+
const tokensRaw = sessionStorage.getItem(
152+
getServerSpecificKey(SESSION_KEYS.TOKENS, serverUrl),
153+
);
154+
if (!tokensRaw) {
155+
return;
156+
}
157+
const tokens = await OAuthTokensSchema.parseAsync(JSON.parse(tokensRaw));
158+
159+
const metadata = await discoverAuthorizationServerMetadata(
160+
new URL("/", serverUrl),
161+
{ fetchFn },
162+
);
163+
// `revocation_endpoint` is declared on OAuthMetadata but not on the OIDC
164+
// branch of the union, even though OIDC providers may advertise it at
165+
// runtime per RFC 8414. Narrow with `in` so we read it from either shape.
166+
const revocationEndpoint =
167+
metadata && "revocation_endpoint" in metadata
168+
? (metadata as { revocation_endpoint?: string }).revocation_endpoint
169+
: undefined;
170+
if (!revocationEndpoint) {
171+
return;
172+
}
173+
174+
const token = tokens.refresh_token ?? tokens.access_token;
175+
const tokenTypeHint = tokens.refresh_token
176+
? "refresh_token"
177+
: "access_token";
178+
179+
const body = new URLSearchParams();
180+
body.set("token", token);
181+
body.set("token_type_hint", tokenTypeHint);
182+
183+
// Public-client convention: include client_id in the form body. Try
184+
// preregistered first, then dynamically registered (same priority as
185+
// InspectorOAuthClientProvider.clientInformation()).
186+
const clientInfo =
187+
(await getClientInformationFromSessionStorage({
188+
serverUrl,
189+
isPreregistered: true,
190+
})) ??
191+
(await getClientInformationFromSessionStorage({
192+
serverUrl,
193+
isPreregistered: false,
194+
}));
195+
if (clientInfo?.client_id) {
196+
body.set("client_id", clientInfo.client_id);
197+
}
198+
199+
const response = await (fetchFn ?? fetch)(revocationEndpoint, {
200+
method: "POST",
201+
headers: { "Content-Type": "application/x-www-form-urlencoded" },
202+
body: body.toString(),
203+
signal: AbortSignal.timeout(3000),
204+
});
205+
206+
if (!response.ok) {
207+
console.warn(
208+
`Token revocation responded ${response.status} ${response.statusText} (best-effort, continuing)`,
209+
);
210+
return;
211+
}
212+
console.debug("Token revocation succeeded");
213+
} catch (error) {
214+
console.warn("Token revocation failed (best-effort):", error);
215+
}
216+
};
217+
132218
export class InspectorOAuthClientProvider implements OAuthClientProvider {
133219
constructor(protected serverUrl: string) {
134220
// Save the server URL to session storage

client/src/lib/hooks/__tests__/useConnection.test.tsx

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import {
2525
ElicitRequest,
2626
} from "@modelcontextprotocol/sdk/types.js";
2727
import { auth } from "@modelcontextprotocol/sdk/client/auth.js";
28-
import { discoverScopes } from "../../auth";
28+
import { discoverScopes, revokeTokens } from "../../auth";
2929
import { CustomHeaders } from "../../types/customHeaders";
3030

3131
// Mock fetch
@@ -145,18 +145,23 @@ jest.mock("../../auth", () => ({
145145
InspectorOAuthClientProvider: jest.fn().mockImplementation(() => ({
146146
tokens: jest.fn().mockResolvedValue({ access_token: "mock-token" }),
147147
redirectUrl: "http://localhost:3000/oauth/callback",
148+
clear: jest.fn(),
148149
})),
149150
clearClientInformationFromSessionStorage: jest.fn(),
150151
saveClientInformationToSessionStorage: jest.fn(),
151152
saveScopeToSessionStorage: jest.fn(),
152153
clearScopeFromSessionStorage: jest.fn(),
153154
discoverScopes: jest.fn(),
155+
revokeTokens: jest.fn().mockResolvedValue(undefined),
154156
}));
155157

156158
const mockAuth = auth as jest.MockedFunction<typeof auth>;
157159
const mockDiscoverScopes = discoverScopes as jest.MockedFunction<
158160
typeof discoverScopes
159161
>;
162+
const mockRevokeTokens = revokeTokens as jest.MockedFunction<
163+
typeof revokeTokens
164+
>;
160165

161166
describe("useConnection", () => {
162167
const defaultProps: Parameters<typeof useConnection>[0] = {
@@ -1791,4 +1796,45 @@ describe("useConnection", () => {
17911796
).toBeNull();
17921797
});
17931798
});
1799+
1800+
describe("disconnect", () => {
1801+
beforeEach(() => {
1802+
jest.clearAllMocks();
1803+
});
1804+
1805+
test("revokes tokens (RFC 7009) before clearing local OAuth state", async () => {
1806+
const { InspectorOAuthClientProvider } = jest.requireMock("../../auth");
1807+
const providerCtor = InspectorOAuthClientProvider as jest.Mock;
1808+
providerCtor.mockClear();
1809+
1810+
const { result } = renderHook(() => useConnection(defaultProps));
1811+
await act(async () => {
1812+
await result.current.connect();
1813+
});
1814+
1815+
await act(async () => {
1816+
await result.current.disconnect();
1817+
});
1818+
1819+
expect(mockRevokeTokens).toHaveBeenCalledTimes(1);
1820+
expect(mockRevokeTokens).toHaveBeenCalledWith({
1821+
serverUrl: defaultProps.sseUrl,
1822+
fetchFn: expect.any(Function),
1823+
});
1824+
1825+
// disconnect constructs an InspectorOAuthClientProvider just for the
1826+
// local-clear step; that instance's clear() must have been invoked.
1827+
const disconnectInstance = providerCtor.mock.results[
1828+
providerCtor.mock.results.length - 1
1829+
].value as { clear: jest.Mock };
1830+
expect(disconnectInstance.clear).toHaveBeenCalledTimes(1);
1831+
1832+
// Revocation must precede the local clear() so we have tokens to send.
1833+
expect(mockRevokeTokens.mock.invocationCallOrder[0]).toBeLessThan(
1834+
disconnectInstance.clear.mock.invocationCallOrder[0],
1835+
);
1836+
1837+
expect(result.current.connectionStatus).toBe("disconnected");
1838+
});
1839+
});
17941840
});

client/src/lib/hooks/useConnection.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ import {
6262
saveScopeToSessionStorage,
6363
clearScopeFromSessionStorage,
6464
discoverScopes,
65+
revokeTokens,
6566
} from "../auth";
6667
import { createProxyFetch } from "../proxyFetch";
6768
import {
@@ -1181,6 +1182,11 @@ export function useConnection({
11811182
clientTransport as StreamableHTTPClientTransport
11821183
).terminateSession();
11831184
await mcpClient?.close();
1185+
// RFC 7009: revoke tokens at the AS before wiping local state, so the
1186+
// server doesn't keep a still-valid token around as a tombstone.
1187+
const fetchFn =
1188+
connectionType === "proxy" ? createProxyFetch(config) : undefined;
1189+
await revokeTokens({ serverUrl: sseUrl, fetchFn });
11841190
const authProvider = new InspectorOAuthClientProvider(sseUrl);
11851191
authProvider.clear();
11861192
setMcpClient(null);

0 commit comments

Comments
 (0)