From 4b953411f5cea7f712760c7746661fa21a9fc93d Mon Sep 17 00:00:00 2001 From: Aikins Laryea Date: Fri, 30 Oct 2026 00:39:16 +0000 Subject: [PATCH] fix(cli): revoke the sign-in that a new login replaces --- packages/cli/README.md | 2 +- packages/cli/src/auth.ts | 90 ++++++++++++++++++++-------------- packages/cli/tests/cli.test.ts | 18 ++++++- 3 files changed, 71 insertions(+), 39 deletions(-) diff --git a/packages/cli/README.md b/packages/cli/README.md index 167f434..07df7f7 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -34,7 +34,7 @@ In a shell with no terminal to paste into, such as an agent's, `telemetry login telemetry login --finish 'http://127.0.0.1:53124/callback?code=…' ``` -A sign-in covers one workspace. To read another workspace, run `telemetry login` again and choose it. +A sign-in covers one workspace. To read another workspace, run `telemetry login` again and choose it. The new sign-in replaces the old one, and the CLI asks telemetry.dev to revoke the old one. ## Use diff --git a/packages/cli/src/auth.ts b/packages/cli/src/auth.ts index 2015322..f4cbf4c 100644 --- a/packages/cli/src/auth.ts +++ b/packages/cli/src/auth.ts @@ -466,8 +466,21 @@ const redeem = async (provider: SessionAuthProvider, params: URLSearchParams) => const email = await fetchEmail(provider.session); const { pending: _finished, ...signedIn } = provider.session; + // Read under the same lock as the write: the server treats revoking a refresh token + // that another command already rotated as reuse, and revokes every token the client + // holds for the person, this new sign-in included. + let replaced: Session = { server: signedIn.server }; + const saved = await updateSession(signedIn.server, (current) => { + replaced = current; + + return { ...signedIn, ...(email ? { email } : {}) }; + }); + + // The sign-in this one replaces would otherwise stay valid at the server until it + // expires. Its revocation is best effort: the new sign-in is already saved. + if (tokenKey(replaced.tokens) !== tokenKey(saved.tokens)) await revokeRefreshToken(replaced); - return updateSession(signedIn.server, () => ({ ...signedIn, ...(email ? { email } : {}) })); + return saved; }; /** @@ -595,47 +608,52 @@ export const login = async ({ } }; +/** + * Revokes `session`'s refresh token at the server (RFC 7009) with the client it was + * issued to. Returns whether the server confirmed it; nothing here throws. + */ +const revokeRefreshToken = async (session: Session) => { + const refresh = session.tokens?.refresh_token; + + if (!refresh || !session.client) return false; + + try { + const { discoverOAuthServerInfo } = await import("@modelcontextprotocol/client"); + const info = await discoverOAuthServerInfo(mcpUrl(session.server), { fetchFn: oauthFetch }); + + // RFC 7009 revocation is in the OAuth metadata, outside the SDK's type. + const metadata = info.authorizationServerMetadata as + | { revocation_endpoint?: unknown } + | undefined; + const endpoint = + typeof metadata?.revocation_endpoint === "string" ? metadata.revocation_endpoint : undefined; + + if (!endpoint) return false; + + const response = await oauthFetch(endpoint, { + method: "POST", + redirect: "error", + headers: { "Content-Type": "application/x-www-form-urlencoded" }, + body: new URLSearchParams({ + token: refresh, + token_type_hint: "refresh_token", + client_id: session.client.client_id, + }), + }); + + return response.ok; + } catch { + return false; + } +}; + /** * Revokes the refresh token at the server, then forgets the tokens. Keeps the client * registration so the next sign-in reuses it. A revocation that fails still signs out * locally, and reports it so the person can disconnect the CLI in account settings. */ export const logout = async (session: Session) => { - const refresh = session.tokens?.refresh_token; - let revoked = false; - - if (refresh && session.client) { - try { - const { discoverOAuthServerInfo } = await import("@modelcontextprotocol/client"); - const info = await discoverOAuthServerInfo(mcpUrl(session.server), { fetchFn: oauthFetch }); - - // RFC 7009 revocation is in the OAuth metadata, outside the SDK's type. - const metadata = info.authorizationServerMetadata as - | { revocation_endpoint?: unknown } - | undefined; - const endpoint = - typeof metadata?.revocation_endpoint === "string" - ? metadata.revocation_endpoint - : undefined; - - if (endpoint) { - const response = await oauthFetch(endpoint, { - method: "POST", - redirect: "error", - headers: { "Content-Type": "application/x-www-form-urlencoded" }, - body: new URLSearchParams({ - token: refresh, - token_type_hint: "refresh_token", - client_id: session.client.client_id, - }), - }); - - revoked = response.ok; - } - } catch { - revoked = false; - } - } + const revoked = await revokeRefreshToken(session); await updateSession(session.server, (saved) => ({ server: saved.server, diff --git a/packages/cli/tests/cli.test.ts b/packages/cli/tests/cli.test.ts index f0a339c..71a80e6 100644 --- a/packages/cli/tests/cli.test.ts +++ b/packages/cli/tests/cli.test.ts @@ -194,8 +194,16 @@ test("input that isn't valid exits 2 before calling telemetry.dev, even from a s const savedSession = async () => JSON.parse(await readFile(sessionPath(), "utf8")); -test("a shell without a terminal signs in with --no-browser, then --finish, keeping the old sign-in until then", async () => { - await signIn(PROJECTS[0]); +test("a shell without a terminal signs in with --no-browser, then --finish, keeping the old sign-in until then and revoking it after", async () => { + await writeFile( + sessionPath(), + JSON.stringify({ + server: fixture.url, + client: { client_id: "fixture-client" }, + tokens: { access_token: TOKEN, token_type: "Bearer", refresh_token: "previous-refresh" }, + }), + { mode: 0o600 }, + ); const started = await run("login", "--no-browser"); expect(started.code).toBe(0); @@ -216,6 +224,7 @@ test("a shell without a terminal signs in with --no-browser, then --finish, keep expect(refused.code).toBe(4); expect(refused.stderr).toContain("telemetry login"); expect(refused.stderr).not.toContain(" at "); + expect(calls.some((call) => call.tool === "oauth:revoke")).toBe(false); expect(await savedSession()).toMatchObject({ tokens: { access_token: TOKEN }, pending: {} }); const finished = await run("login", "--finish", address); @@ -227,6 +236,11 @@ test("a shell without a terminal signs in with --no-browser, then --finish, keep expect(session).toMatchObject({ email: EMAIL, tokens: { refresh_token: REFRESH_TOKEN } }); expect(session.pending).toBeUndefined(); + // The sign-in it replaced no longer works at the server. + expect(calls.filter((call) => call.tool === "oauth:revoke").map((call) => call.args)).toEqual([ + expect.objectContaining({ token: "previous-refresh", client_id: "fixture-client" }), + ]); + // The fixture rejects a code without the verifier its challenge came from. expect(calls.some((call) => call.tool === "oauth:authorization_code")).toBe(true);