cli: stored-auth injection honors refresh_token, not just access_token (#1665)
Closes #1665 `--use-stored-auth` previously injected the stored access token blindly, so an expired token failed even with a valid refresh_token on disk. Now, when a `refresh_token` is stored, the CLI runs the OAuth refresh grant and injects the fresh access token, persisting the rotation. - New exported `refreshStoredAuthToken(serverUrl, statePath, deps?)`: reuses the SDK's `refreshAuthorization` (not a hand-rolled token request) with the stored `clientInformation` + `serverMetadata` (discovered via core `getAuthorizationServerUrl` + SDK `discoverAuthorizationServerMetadata` when the metadata wasn't persisted). Rotated tokens are written back through `serializeOAuthPersistBlob` under the same `{servers,idpSessions}` key, so web and CLI stay consistent. Missing refresh_token/clientInformation or a failed grant throw `CliExitCodeError(AUTH_REQUIRED)` with a clear message + envelope code (`no_stored_token` / `refresh_failed`). - `--use-stored-auth` prefers refresh when a refresh_token is present (covers both absent and expired access tokens — the stored blob has no expiry, so the refresh token is the durable credential); falls back to injecting the stored access token otherwise (unchanged). - `deps` are injectable so the refresh/discovery are unit-tested without a live token endpoint; an e2e test drives the real SDK path against a mock token endpoint and asserts the refreshed token is injected + the rotation persisted. - CLI README updated (removed the "blind injection" caveat). CLI coverage gate green (cli.ts 100/92/100/100). Part of the #1579 decomposition (Wave 4, CLI lane). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw
This commit is contained in:
cliffhall
parent
a029e42a19
commit
cdf124106e
1 changed files with 365 additions and 34 deletions
|
|
@ -1,10 +1,11 @@
|
|||
import { describe, it, expect, beforeAll, afterAll } from "vitest";
|
||||
import { mkdtempSync, writeFileSync, rmSync } from "node:fs";
|
||||
import { describe, it, expect, beforeAll, afterAll, vi } from "vitest";
|
||||
import { mkdtempSync, writeFileSync, readFileSync, rmSync } from "node:fs";
|
||||
import { createServer, type Server } from "node:http";
|
||||
import { join, dirname } from "node:path";
|
||||
import { tmpdir } from "node:os";
|
||||
import { runCli } from "./helpers/cli-runner.js";
|
||||
import { expectCliFailure, expectCliSuccess } from "./helpers/assertions.js";
|
||||
import { normalizeServerUrl } from "../src/cli.js";
|
||||
import { normalizeServerUrl, refreshStoredAuthToken } from "../src/cli.js";
|
||||
import {
|
||||
createTestServerHttp,
|
||||
createEchoTool,
|
||||
|
|
@ -36,6 +37,137 @@ describe("normalizeServerUrl", () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe("refreshStoredAuthToken", () => {
|
||||
const SERVER = "https://api.example/mcp";
|
||||
const freshTokens = {
|
||||
access_token: "refreshed-access-token",
|
||||
token_type: "Bearer",
|
||||
refresh_token: "rotated-refresh-token",
|
||||
expires_in: 3600,
|
||||
};
|
||||
|
||||
it("runs the refresh grant, injects the fresh token, and persists the rotation", async () => {
|
||||
const path = writeOAuthFixture({
|
||||
[SERVER]: {
|
||||
tokens: { refresh_token: "old-refresh", token_type: "Bearer" },
|
||||
clientInformation: { client_id: "cid", client_secret: "sec" },
|
||||
serverMetadata: {
|
||||
issuer: "https://auth.example",
|
||||
token_endpoint: "https://auth.example/token",
|
||||
},
|
||||
},
|
||||
});
|
||||
try {
|
||||
const refresh = vi.fn().mockResolvedValue(freshTokens);
|
||||
const discover = vi.fn();
|
||||
const token = await refreshStoredAuthToken(SERVER, path, {
|
||||
refresh,
|
||||
discover,
|
||||
});
|
||||
expect(token).toBe("refreshed-access-token");
|
||||
// Stored metadata present → no discovery needed.
|
||||
expect(discover).not.toHaveBeenCalled();
|
||||
// Refresh called with the stored refresh token + client information.
|
||||
expect(refresh).toHaveBeenCalledTimes(1);
|
||||
const [, opts] = refresh.mock.calls[0]!;
|
||||
expect(opts.refreshToken).toBe("old-refresh");
|
||||
expect(opts.clientInformation).toEqual({
|
||||
client_id: "cid",
|
||||
client_secret: "sec",
|
||||
});
|
||||
// Rotation persisted back under the same key.
|
||||
const persisted = JSON.parse(readFileSync(path, "utf8")) as {
|
||||
servers: Record<string, { tokens?: { refresh_token?: string } }>;
|
||||
};
|
||||
expect(persisted.servers[SERVER]?.tokens?.refresh_token).toBe(
|
||||
"rotated-refresh-token",
|
||||
);
|
||||
} finally {
|
||||
rmSync(path, { force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("discovers the auth-server metadata when it was not persisted", async () => {
|
||||
const path = writeOAuthFixture({
|
||||
[SERVER]: {
|
||||
tokens: { refresh_token: "old-refresh", token_type: "Bearer" },
|
||||
clientInformation: { client_id: "cid" },
|
||||
},
|
||||
});
|
||||
try {
|
||||
const refresh = vi.fn().mockResolvedValue(freshTokens);
|
||||
const discover = vi.fn().mockResolvedValue({
|
||||
issuer: "https://api.example",
|
||||
token_endpoint: "https://api.example/token",
|
||||
});
|
||||
const token = await refreshStoredAuthToken(SERVER, path, {
|
||||
refresh,
|
||||
discover,
|
||||
});
|
||||
expect(token).toBe("refreshed-access-token");
|
||||
expect(discover).toHaveBeenCalledTimes(1);
|
||||
} finally {
|
||||
rmSync(path, { force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("throws AUTH_REQUIRED when no refresh token is stored", async () => {
|
||||
const path = writeOAuthFixture({
|
||||
[SERVER]: {
|
||||
tokens: { access_token: "only-access", token_type: "Bearer" },
|
||||
},
|
||||
});
|
||||
try {
|
||||
await expect(
|
||||
refreshStoredAuthToken(SERVER, path, { refresh: vi.fn() }),
|
||||
).rejects.toMatchObject({ exitCode: 3 });
|
||||
} finally {
|
||||
rmSync(path, { force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("throws AUTH_REQUIRED when a refresh token is present but client information is missing", async () => {
|
||||
const path = writeOAuthFixture({
|
||||
[SERVER]: {
|
||||
tokens: { refresh_token: "old-refresh", token_type: "Bearer" },
|
||||
},
|
||||
});
|
||||
try {
|
||||
await expect(
|
||||
refreshStoredAuthToken(SERVER, path, { refresh: vi.fn() }),
|
||||
).rejects.toMatchObject({ exitCode: 3 });
|
||||
} finally {
|
||||
rmSync(path, { force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("surfaces a refresh-grant failure as AUTH_REQUIRED (refresh_failed)", async () => {
|
||||
const path = writeOAuthFixture({
|
||||
[SERVER]: {
|
||||
tokens: { refresh_token: "old-refresh", token_type: "Bearer" },
|
||||
clientInformation: { client_id: "cid" },
|
||||
serverMetadata: {
|
||||
issuer: "https://auth.example",
|
||||
token_endpoint: "https://auth.example/token",
|
||||
},
|
||||
},
|
||||
});
|
||||
try {
|
||||
const refresh = vi
|
||||
.fn()
|
||||
.mockRejectedValue(new Error("invalid_grant: token revoked"));
|
||||
await expect(
|
||||
refreshStoredAuthToken(SERVER, path, { refresh }),
|
||||
).rejects.toMatchObject({
|
||||
exitCode: 3,
|
||||
envelope: { code: "refresh_failed" },
|
||||
});
|
||||
} finally {
|
||||
rmSync(path, { force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("--use-stored-auth", () => {
|
||||
let server: ReturnType<typeof createTestServerHttp>;
|
||||
let serverUrl: string;
|
||||
|
|
@ -197,6 +329,71 @@ describe("--use-stored-auth", () => {
|
|||
const last = server.getRecordedRequests().at(-1)!;
|
||||
expect(last.headers?.authorization).toBe(`Bearer ${TOKEN}`);
|
||||
});
|
||||
|
||||
it("refreshes a stored refresh_token end-to-end and injects the fresh access token (#1665)", async () => {
|
||||
// Minimal OAuth token endpoint: honors the refresh_token grant with a
|
||||
// rotated token pair, so the CLI's real SDK refresh path has something to
|
||||
// talk to (the MCP test server accepts any bearer, so a successful connect
|
||||
// proves the refreshed token was injected).
|
||||
let tokenRequests = 0;
|
||||
const tokenServer: Server = createServer((req, res) => {
|
||||
tokenRequests += 1;
|
||||
res.writeHead(200, { "content-type": "application/json" });
|
||||
res.end(
|
||||
JSON.stringify({
|
||||
access_token: "refreshed-access-token",
|
||||
token_type: "Bearer",
|
||||
refresh_token: "rotated-refresh-token",
|
||||
expires_in: 3600,
|
||||
}),
|
||||
);
|
||||
});
|
||||
await new Promise<void>((resolve) => tokenServer.listen(0, resolve));
|
||||
const addr = tokenServer.address();
|
||||
const tokenBase =
|
||||
typeof addr === "object" && addr ? `http://127.0.0.1:${addr.port}` : "";
|
||||
const fixture = writeOAuthFixture({
|
||||
[serverUrl]: {
|
||||
tokens: { refresh_token: "old-refresh", token_type: "Bearer" },
|
||||
clientInformation: { client_id: "cid", client_secret: "sec" },
|
||||
serverMetadata: {
|
||||
issuer: tokenBase,
|
||||
token_endpoint: `${tokenBase}/token`,
|
||||
response_types_supported: ["code"],
|
||||
grant_types_supported: ["authorization_code", "refresh_token"],
|
||||
token_endpoint_auth_methods_supported: ["client_secret_post"],
|
||||
},
|
||||
},
|
||||
});
|
||||
try {
|
||||
const result = await runCli(
|
||||
[
|
||||
"--transport",
|
||||
"http",
|
||||
"--server-url",
|
||||
serverUrl,
|
||||
"--use-stored-auth",
|
||||
"--method",
|
||||
"tools/list",
|
||||
],
|
||||
{ env: { MCP_INSPECTOR_OAUTH_STATE_PATH: fixture } },
|
||||
);
|
||||
expectCliSuccess(result);
|
||||
expect(tokenRequests).toBeGreaterThan(0);
|
||||
const last = server.getRecordedRequests().at(-1)!;
|
||||
expect(last.headers?.authorization).toBe("Bearer refreshed-access-token");
|
||||
// Rotation persisted so a subsequent run reuses the new refresh token.
|
||||
const persisted = JSON.parse(readFileSync(fixture, "utf8")) as {
|
||||
servers: Record<string, { tokens?: { refresh_token?: string } }>;
|
||||
};
|
||||
expect(persisted.servers[serverUrl]?.tokens?.refresh_token).toBe(
|
||||
"rotated-refresh-token",
|
||||
);
|
||||
} finally {
|
||||
rmSync(fixture, { force: true });
|
||||
await new Promise<void>((resolve) => tokenServer.close(() => resolve()));
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("--list-stored-auth", () => {
|
||||
|
|
|
|||
Loading…
Reference in a new issue