test(cli): cover cliOAuth.ts step-up/recovery paths to the ≥90% gate
Bring clients/cli/src/cliOAuth.ts from 75% to ≥90% on all four coverage
dimensions (now 100/96.42/100/100) by exercising the previously-untested
interactive step-up and auth-recovery paths:
- confirmStepUpFromStdin (the default stdin confirmer): drive it via the
default `confirmStepUp` parameter of handleCliAuthRecoveryRequired with
node:readline/promises mocked — covers the y / "yes" (trimmed,
case-insensitive) accept paths and the n decline path (which throws).
- connectInspectorWithOAuth's AuthRecoveryRequiredError branch: both the
storage-already-satisfies resume and the interactive-recovery sub-branches,
plus the isUnauthorizedError branch (with a rejecting disconnect to hit the
`.catch(() => {})` guard), the non-OAuth-capable-config rethrow, and the
non-OAuth-error rethrow.
Part of #1610 — v2/main mid-session-auth code below the per-file coverage
gate that v2/main's CI does not run but the Wave-1 rollup (#1550) does.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX
This commit is contained in:
parent
d825af3781
commit
01cc79c61b
1 changed files with 227 additions and 0 deletions
|
|
@ -3,15 +3,39 @@ import { AuthRecoveryRequiredError } from "@inspector/core/auth/challenge.js";
|
||||||
import { MutableRedirectUrlProvider } from "@inspector/core/auth/index.js";
|
import { MutableRedirectUrlProvider } from "@inspector/core/auth/index.js";
|
||||||
import * as runnerInteractive from "@inspector/core/auth/node/runner-interactive-oauth.js";
|
import * as runnerInteractive from "@inspector/core/auth/node/runner-interactive-oauth.js";
|
||||||
import {
|
import {
|
||||||
|
connectInspectorWithOAuth,
|
||||||
handleCliAuthRecoveryRequired,
|
handleCliAuthRecoveryRequired,
|
||||||
isStandardOAuthStepUp,
|
isStandardOAuthStepUp,
|
||||||
runCliInteractiveOAuth,
|
runCliInteractiveOAuth,
|
||||||
withCliAuthRecoveryRetry,
|
withCliAuthRecoveryRetry,
|
||||||
} from "../src/cliOAuth.js";
|
} from "../src/cliOAuth.js";
|
||||||
|
import type { MCPServerConfig } from "@inspector/core/mcp/types.js";
|
||||||
|
|
||||||
|
// `confirmStepUpFromStdin` (the default step-up confirmer) reads a line from
|
||||||
|
// stdin via node:readline/promises. Mock the module so the default path can be
|
||||||
|
// exercised deterministically without real TTY input.
|
||||||
|
const { mockQuestion, mockClose } = vi.hoisted(() => ({
|
||||||
|
mockQuestion: vi.fn(),
|
||||||
|
mockClose: vi.fn(),
|
||||||
|
}));
|
||||||
|
vi.mock("node:readline/promises", () => ({
|
||||||
|
createInterface: vi.fn(() => ({
|
||||||
|
question: mockQuestion,
|
||||||
|
close: mockClose,
|
||||||
|
})),
|
||||||
|
}));
|
||||||
|
|
||||||
|
const CALLBACK_URL_CONFIG = {
|
||||||
|
hostname: "127.0.0.1",
|
||||||
|
port: 6276,
|
||||||
|
pathname: "/oauth/callback",
|
||||||
|
};
|
||||||
|
|
||||||
describe("cliOAuth", () => {
|
describe("cliOAuth", () => {
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
vi.restoreAllMocks();
|
vi.restoreAllMocks();
|
||||||
|
mockQuestion.mockReset();
|
||||||
|
mockClose.mockReset();
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("isStandardOAuthStepUp", () => {
|
describe("isStandardOAuthStepUp", () => {
|
||||||
|
|
@ -227,4 +251,207 @@ describe("cliOAuth", () => {
|
||||||
expect(result).toBe("ok");
|
expect(result).toBe("ok");
|
||||||
expect(fn).toHaveBeenCalledTimes(2);
|
expect(fn).toHaveBeenCalledTimes(2);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("confirmStepUpFromStdin (default stdin confirmer)", () => {
|
||||||
|
const standardStepUpError = () =>
|
||||||
|
new AuthRecoveryRequiredError(new URL("https://as.example/authorize"), {
|
||||||
|
reason: "insufficient_scope",
|
||||||
|
requiredScopes: ["weather:read"],
|
||||||
|
});
|
||||||
|
|
||||||
|
const clientNeedingStepUp = () => ({
|
||||||
|
authenticate: vi.fn(),
|
||||||
|
beginInteractiveAuthorization: vi.fn(),
|
||||||
|
completeOAuthFlow: vi.fn(),
|
||||||
|
checkAuthChallengeSatisfied: vi.fn().mockResolvedValue(false),
|
||||||
|
});
|
||||||
|
|
||||||
|
it("proceeds with OAuth when the user answers y (no confirmStepUp arg)", async () => {
|
||||||
|
mockQuestion.mockResolvedValue("y");
|
||||||
|
const runSpy = vi
|
||||||
|
.spyOn(runnerInteractive, "runRunnerInteractiveOAuth")
|
||||||
|
.mockResolvedValue({ kind: "success" });
|
||||||
|
|
||||||
|
// Omitting the confirmStepUp argument exercises the default
|
||||||
|
// confirmStepUpFromStdin, which reads from the mocked readline interface.
|
||||||
|
await handleCliAuthRecoveryRequired(
|
||||||
|
clientNeedingStepUp(),
|
||||||
|
standardStepUpError(),
|
||||||
|
new MutableRedirectUrlProvider(),
|
||||||
|
CALLBACK_URL_CONFIG,
|
||||||
|
{},
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(mockQuestion).toHaveBeenCalled();
|
||||||
|
expect(mockClose).toHaveBeenCalled();
|
||||||
|
expect(runSpy).toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("accepts a whitespace-padded, upper-case 'YES'", async () => {
|
||||||
|
mockQuestion.mockResolvedValue(" YES ");
|
||||||
|
const runSpy = vi
|
||||||
|
.spyOn(runnerInteractive, "runRunnerInteractiveOAuth")
|
||||||
|
.mockResolvedValue({ kind: "success" });
|
||||||
|
|
||||||
|
await handleCliAuthRecoveryRequired(
|
||||||
|
clientNeedingStepUp(),
|
||||||
|
standardStepUpError(),
|
||||||
|
new MutableRedirectUrlProvider(),
|
||||||
|
CALLBACK_URL_CONFIG,
|
||||||
|
{},
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(runSpy).toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("declines (throws) when the user answers n", async () => {
|
||||||
|
mockQuestion.mockResolvedValue("n");
|
||||||
|
const runSpy = vi.spyOn(runnerInteractive, "runRunnerInteractiveOAuth");
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
handleCliAuthRecoveryRequired(
|
||||||
|
clientNeedingStepUp(),
|
||||||
|
standardStepUpError(),
|
||||||
|
new MutableRedirectUrlProvider(),
|
||||||
|
CALLBACK_URL_CONFIG,
|
||||||
|
{},
|
||||||
|
),
|
||||||
|
).rejects.toThrow("Step-up authorization declined.");
|
||||||
|
|
||||||
|
expect(mockClose).toHaveBeenCalled();
|
||||||
|
expect(runSpy).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("connectInspectorWithOAuth recovery branch", () => {
|
||||||
|
const oauthServerConfig = {
|
||||||
|
type: "streamable-http",
|
||||||
|
url: "https://as.example/mcp",
|
||||||
|
} as MCPServerConfig;
|
||||||
|
|
||||||
|
it("resumes without re-auth when storage already satisfies the challenge", async () => {
|
||||||
|
const runSpy = vi.spyOn(runnerInteractive, "runRunnerInteractiveOAuth");
|
||||||
|
const connect = vi
|
||||||
|
.fn()
|
||||||
|
.mockRejectedValueOnce(
|
||||||
|
new AuthRecoveryRequiredError(
|
||||||
|
new URL("https://as.example/authorize"),
|
||||||
|
{ reason: "insufficient_scope", requiredScopes: ["weather:read"] },
|
||||||
|
),
|
||||||
|
)
|
||||||
|
.mockResolvedValueOnce(undefined);
|
||||||
|
const client = {
|
||||||
|
connect,
|
||||||
|
disconnect: vi.fn(),
|
||||||
|
checkAuthChallengeSatisfied: vi.fn().mockResolvedValue(true),
|
||||||
|
};
|
||||||
|
|
||||||
|
await connectInspectorWithOAuth(
|
||||||
|
client,
|
||||||
|
oauthServerConfig,
|
||||||
|
new MutableRedirectUrlProvider(),
|
||||||
|
CALLBACK_URL_CONFIG,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(connect).toHaveBeenCalledTimes(2);
|
||||||
|
expect(runSpy).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("runs interactive recovery when storage does not satisfy the challenge", async () => {
|
||||||
|
const runSpy = vi
|
||||||
|
.spyOn(runnerInteractive, "runRunnerInteractiveOAuth")
|
||||||
|
.mockResolvedValue({ kind: "success" });
|
||||||
|
const connect = vi
|
||||||
|
.fn()
|
||||||
|
.mockRejectedValueOnce(
|
||||||
|
new AuthRecoveryRequiredError(
|
||||||
|
new URL("https://as.example/authorize"),
|
||||||
|
{ reason: "token_expired" },
|
||||||
|
),
|
||||||
|
)
|
||||||
|
.mockResolvedValueOnce(undefined);
|
||||||
|
const client = {
|
||||||
|
connect,
|
||||||
|
disconnect: vi.fn(),
|
||||||
|
checkAuthChallengeSatisfied: vi.fn().mockResolvedValue(false),
|
||||||
|
};
|
||||||
|
|
||||||
|
await connectInspectorWithOAuth(
|
||||||
|
client,
|
||||||
|
oauthServerConfig,
|
||||||
|
new MutableRedirectUrlProvider(),
|
||||||
|
CALLBACK_URL_CONFIG,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(connect).toHaveBeenCalledTimes(2);
|
||||||
|
expect(runSpy).toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("runs interactive OAuth on a plain unauthorized error (disconnect failure is swallowed)", async () => {
|
||||||
|
const runSpy = vi
|
||||||
|
.spyOn(runnerInteractive, "runRunnerInteractiveOAuth")
|
||||||
|
.mockResolvedValue({ kind: "success" });
|
||||||
|
const connect = vi
|
||||||
|
.fn()
|
||||||
|
.mockRejectedValueOnce(new Error("Connection failed for server (401)"))
|
||||||
|
.mockResolvedValueOnce(undefined);
|
||||||
|
// A rejecting disconnect exercises the `.catch(() => {})` guard.
|
||||||
|
const client = {
|
||||||
|
connect,
|
||||||
|
disconnect: vi.fn().mockRejectedValue(new Error("disconnect failed")),
|
||||||
|
checkAuthChallengeSatisfied: vi.fn(),
|
||||||
|
};
|
||||||
|
|
||||||
|
await connectInspectorWithOAuth(
|
||||||
|
client,
|
||||||
|
oauthServerConfig,
|
||||||
|
new MutableRedirectUrlProvider(),
|
||||||
|
CALLBACK_URL_CONFIG,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(client.disconnect).toHaveBeenCalled();
|
||||||
|
expect(runSpy).toHaveBeenCalled();
|
||||||
|
expect(connect).toHaveBeenCalledTimes(2);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("rethrows a non-OAuth error unchanged", async () => {
|
||||||
|
const runSpy = vi.spyOn(runnerInteractive, "runRunnerInteractiveOAuth");
|
||||||
|
const connect = vi
|
||||||
|
.fn()
|
||||||
|
.mockRejectedValue(new Error("some unrelated failure"));
|
||||||
|
const client = {
|
||||||
|
connect,
|
||||||
|
disconnect: vi.fn(),
|
||||||
|
checkAuthChallengeSatisfied: vi.fn(),
|
||||||
|
};
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
connectInspectorWithOAuth(
|
||||||
|
client,
|
||||||
|
oauthServerConfig,
|
||||||
|
new MutableRedirectUrlProvider(),
|
||||||
|
CALLBACK_URL_CONFIG,
|
||||||
|
),
|
||||||
|
).rejects.toThrow("some unrelated failure");
|
||||||
|
expect(runSpy).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("rethrows when the server config is not OAuth-capable", async () => {
|
||||||
|
const connect = vi.fn().mockRejectedValue(new Error("nope (401)"));
|
||||||
|
const client = {
|
||||||
|
connect,
|
||||||
|
disconnect: vi.fn(),
|
||||||
|
checkAuthChallengeSatisfied: vi.fn(),
|
||||||
|
};
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
connectInspectorWithOAuth(
|
||||||
|
client,
|
||||||
|
{ type: "stdio", command: "x" } as MCPServerConfig,
|
||||||
|
new MutableRedirectUrlProvider(),
|
||||||
|
CALLBACK_URL_CONFIG,
|
||||||
|
),
|
||||||
|
).rejects.toThrow("nope (401)");
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue