Fixes for remaining issues
This commit is contained in:
parent
8f1ed7588d
commit
a066873e37
6 changed files with 415 additions and 43 deletions
|
|
@ -559,16 +559,23 @@ function App({
|
|||
);
|
||||
|
||||
// Connect — on 401 or mid-session auth recovery, run OAuth then retry.
|
||||
type TuiOAuthRunResult =
|
||||
| "success"
|
||||
| "already_authorized"
|
||||
| "insufficient_scope"
|
||||
| "skipped"
|
||||
| "unsupported";
|
||||
|
||||
const runOAuthAuthentication = useCallback(
|
||||
async (options?: {
|
||||
challenge?: AuthChallenge;
|
||||
authorizationUrl?: URL;
|
||||
/** When set, run OAuth for this server (may differ from the selected server). */
|
||||
serverName?: string;
|
||||
}) => {
|
||||
}): Promise<TuiOAuthRunResult> => {
|
||||
const serverName = options?.serverName ?? selectedServer;
|
||||
if (!serverName) {
|
||||
return;
|
||||
return "unsupported";
|
||||
}
|
||||
const client = inspectorClientsRef.current[serverName];
|
||||
const serverEntry = mcpServersRef.current[serverName];
|
||||
|
|
@ -578,9 +585,11 @@ function App({
|
|||
!serverConfig ||
|
||||
!isOAuthCapableServerConfig(serverConfig)
|
||||
) {
|
||||
return;
|
||||
return "unsupported";
|
||||
}
|
||||
if (oauthInProgressRef.current) {
|
||||
return "skipped";
|
||||
}
|
||||
if (oauthInProgressRef.current) return;
|
||||
oauthInProgressRef.current = true;
|
||||
getTuiLogger().info(
|
||||
{ server: serverName },
|
||||
|
|
@ -594,7 +603,7 @@ function App({
|
|||
const redirectUrlProvider = redirectUrlProvidersRef.current[serverName];
|
||||
if (!redirectUrlProvider) {
|
||||
oauthInProgressRef.current = false;
|
||||
return;
|
||||
return "unsupported";
|
||||
}
|
||||
try {
|
||||
const result = await runRunnerInteractiveOAuth({
|
||||
|
|
@ -612,11 +621,13 @@ function App({
|
|||
if (result.kind === "insufficient_scope") {
|
||||
setOauthStatus("error");
|
||||
setOauthMessage(stepUpInsufficientScopeMessage(result.challenge));
|
||||
return;
|
||||
return "insufficient_scope";
|
||||
}
|
||||
if (result.kind === "success" || result.kind === "already_authorized") {
|
||||
setOauthRevision((n) => n + 1);
|
||||
return result.kind;
|
||||
}
|
||||
return "unsupported";
|
||||
} finally {
|
||||
oauthInProgressRef.current = false;
|
||||
callbackServerRef.current = null;
|
||||
|
|
@ -710,13 +721,21 @@ function App({
|
|||
}
|
||||
setOauthStatus("authenticating");
|
||||
try {
|
||||
await runOAuthAuthentication({
|
||||
const oauthResult = await runOAuthAuthentication({
|
||||
challenge: error.authChallenge,
|
||||
authorizationUrl: error.authorizationUrl,
|
||||
serverName,
|
||||
});
|
||||
setOauthStatus("idle");
|
||||
setOauthMessage("Authorization updated. Retry your action.");
|
||||
if (
|
||||
oauthResult === "success" ||
|
||||
oauthResult === "already_authorized"
|
||||
) {
|
||||
setOauthStatus("idle");
|
||||
setOauthMessage("Authorization updated. Retry your action.");
|
||||
} else if (oauthResult === "skipped") {
|
||||
setOauthStatus("idle");
|
||||
setOauthMessage("OAuth already in progress.");
|
||||
}
|
||||
} catch (authErr) {
|
||||
const authMsg =
|
||||
authErr instanceof Error ? authErr.message : String(authErr);
|
||||
|
|
@ -770,8 +789,16 @@ function App({
|
|||
setOauthStatus("authenticating");
|
||||
setOauthMessage(null);
|
||||
await disconnectInspector();
|
||||
await runOAuthAuthentication();
|
||||
await finishConnect();
|
||||
const oauthResult = await runOAuthAuthentication();
|
||||
if (
|
||||
oauthResult === "success" ||
|
||||
oauthResult === "already_authorized"
|
||||
) {
|
||||
await finishConnect();
|
||||
} else if (oauthResult === "skipped") {
|
||||
setOauthStatus("idle");
|
||||
setOauthMessage("OAuth already in progress.");
|
||||
}
|
||||
} catch (authErr) {
|
||||
if (authErr instanceof AuthRecoveryRequiredError) {
|
||||
handleAuthRecoveryRequired(selectedServer, authErr);
|
||||
|
|
@ -1299,8 +1326,15 @@ function App({
|
|||
);
|
||||
if (tabAccelerators[input.toLowerCase()]) {
|
||||
const nextTab = tabAccelerators[input.toLowerCase()]!;
|
||||
setActiveTab(nextTab);
|
||||
setFocus(nextTab === "auth" ? "tabContentList" : "tabs");
|
||||
const authStepUpAccelerator =
|
||||
input.toLowerCase() === "a" &&
|
||||
nextTab === "auth" &&
|
||||
activeTab === "auth" &&
|
||||
pendingStepUp?.serverName === selectedServer;
|
||||
if (!authStepUpAccelerator) {
|
||||
setActiveTab(nextTab);
|
||||
setFocus(nextTab === "auth" ? "tabContentList" : "tabs");
|
||||
}
|
||||
} else if (key.tab && !key.shift) {
|
||||
// Flat focus order: servers -> tabs -> list -> details -> wrap to servers
|
||||
const focusOrder: FocusArea[] =
|
||||
|
|
@ -1689,14 +1723,22 @@ function App({
|
|||
return;
|
||||
}
|
||||
if (outcome.kind === "interactive") {
|
||||
await runOAuthAuthentication({
|
||||
const oauthResult = await runOAuthAuthentication({
|
||||
challenge: outcome.challenge,
|
||||
authorizationUrl: outcome.authorizationUrl,
|
||||
});
|
||||
setOauthStatus("idle");
|
||||
setOauthMessage(
|
||||
"Step-up authorization succeeded. Retry your action.",
|
||||
);
|
||||
if (
|
||||
oauthResult === "success" ||
|
||||
oauthResult === "already_authorized"
|
||||
) {
|
||||
setOauthStatus("idle");
|
||||
setOauthMessage(
|
||||
"Step-up authorization succeeded. Retry your action.",
|
||||
);
|
||||
} else if (oauthResult === "skipped") {
|
||||
setOauthStatus("idle");
|
||||
setOauthMessage("OAuth already in progress.");
|
||||
}
|
||||
return;
|
||||
}
|
||||
if (outcome.kind === "failed") {
|
||||
|
|
@ -1712,14 +1754,22 @@ function App({
|
|||
);
|
||||
return;
|
||||
}
|
||||
await runOAuthAuthentication({
|
||||
const oauthResult = await runOAuthAuthentication({
|
||||
challenge,
|
||||
authorizationUrl,
|
||||
});
|
||||
setOauthStatus("idle");
|
||||
setOauthMessage(
|
||||
"Step-up authorization succeeded. Retry your action.",
|
||||
);
|
||||
if (
|
||||
oauthResult === "success" ||
|
||||
oauthResult === "already_authorized"
|
||||
) {
|
||||
setOauthStatus("idle");
|
||||
setOauthMessage(
|
||||
"Step-up authorization succeeded. Retry your action.",
|
||||
);
|
||||
} else if (oauthResult === "skipped") {
|
||||
setOauthStatus("idle");
|
||||
setOauthMessage("OAuth already in progress.");
|
||||
}
|
||||
} catch (authErr) {
|
||||
const authMsg =
|
||||
authErr instanceof Error
|
||||
|
|
|
|||
|
|
@ -2,6 +2,8 @@ import { describe, it, expect } from "vitest";
|
|||
import {
|
||||
computeScopeUnion,
|
||||
isStrictScopeSuperset,
|
||||
resolveEffectiveGrantedScope,
|
||||
resolvePersistedScopeAfterGrant,
|
||||
} from "@inspector/core/auth/scopes.js";
|
||||
|
||||
describe("scopes", () => {
|
||||
|
|
@ -34,4 +36,32 @@ describe("scopes", () => {
|
|||
expect(isStrictScopeSuperset("mcp weather:read", undefined)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolvePersistedScopeAfterGrant", () => {
|
||||
it("prefers explicit granted scope over requested", () => {
|
||||
expect(resolvePersistedScopeAfterGrant("mcp", "mcp weather:read")).toBe(
|
||||
"mcp",
|
||||
);
|
||||
});
|
||||
|
||||
it("falls back to requested scope when grant omits scope", () => {
|
||||
expect(
|
||||
resolvePersistedScopeAfterGrant(undefined, "mcp weather:read"),
|
||||
).toBe("mcp weather:read");
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolveEffectiveGrantedScope", () => {
|
||||
it("uses token scope when present even if storage overstates grant", () => {
|
||||
expect(resolveEffectiveGrantedScope("mcp weather:read", "mcp")).toBe(
|
||||
"mcp",
|
||||
);
|
||||
});
|
||||
|
||||
it("falls back to stored scope when token omits scope", () => {
|
||||
expect(resolveEffectiveGrantedScope("mcp tools:read", undefined)).toBe(
|
||||
"mcp tools:read",
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -0,0 +1,50 @@
|
|||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { AuthChallengeError } from "@inspector/core/auth/challenge.js";
|
||||
import { InspectorClient } from "@inspector/core/mcp/inspectorClient.js";
|
||||
|
||||
describe("InspectorClient direct auth recovery retry bound", () => {
|
||||
beforeEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("retries an operation once after satisfied recovery, then gives up", async () => {
|
||||
const client = Object.create(InspectorClient.prototype) as InspectorClient;
|
||||
const internals = client as unknown as {
|
||||
directAuthRecovery: boolean;
|
||||
directAuthRecoveryActive: boolean;
|
||||
reconnectAfterAuthRecovery: ReturnType<typeof vi.fn>;
|
||||
withDirectAuthRecovery: <T>(
|
||||
operation: () => Promise<T>,
|
||||
context?: { method?: string; toolName?: string },
|
||||
attempt?: number,
|
||||
) => Promise<T>;
|
||||
};
|
||||
internals.directAuthRecovery = true;
|
||||
internals.directAuthRecoveryActive = true;
|
||||
internals.reconnectAfterAuthRecovery = vi.fn().mockResolvedValue(undefined);
|
||||
|
||||
const challenge = {
|
||||
reason: "insufficient_scope" as const,
|
||||
requiredScopes: ["weather:read"],
|
||||
};
|
||||
const operation = vi
|
||||
.fn()
|
||||
.mockRejectedValue(new AuthChallengeError(challenge, 403));
|
||||
|
||||
vi.spyOn(client, "handleAuthChallenge").mockResolvedValue({
|
||||
kind: "satisfied",
|
||||
});
|
||||
vi.spyOn(client, "dispatchTypedEvent").mockImplementation(() => {});
|
||||
|
||||
await expect(
|
||||
internals.withDirectAuthRecovery.call(client, operation, {
|
||||
method: "tools/call",
|
||||
toolName: "get_temp",
|
||||
}),
|
||||
).rejects.toBeInstanceOf(AuthChallengeError);
|
||||
|
||||
expect(operation).toHaveBeenCalledTimes(2);
|
||||
expect(client.handleAuthChallenge).toHaveBeenCalledTimes(1);
|
||||
expect(internals.reconnectAfterAuthRecovery).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
|
@ -606,6 +606,54 @@ describe("OAuthManager", () => {
|
|||
);
|
||||
captureSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("persists granted scope when AS down-scopes the token response", async () => {
|
||||
mockedMcpAuth.mockResolvedValue("AUTHORIZED");
|
||||
const params = createMockParams();
|
||||
storageOf(params).getScope.mockReturnValue("mcp");
|
||||
storageOf(params).getTokens.mockResolvedValue({
|
||||
access_token: "access",
|
||||
token_type: "Bearer",
|
||||
scope: "mcp",
|
||||
});
|
||||
storageOf(params).getClientInformation.mockResolvedValue({
|
||||
client_id: "cid",
|
||||
});
|
||||
const manager = new OAuthManager(params);
|
||||
(
|
||||
manager as unknown as { pendingAuthorizationScope: string | undefined }
|
||||
).pendingAuthorizationScope = "mcp weather:read";
|
||||
|
||||
await manager.completeOAuthFlow("code");
|
||||
|
||||
expect(storageOf(params).saveScope).toHaveBeenCalledWith(
|
||||
SERVER_URL,
|
||||
"mcp",
|
||||
);
|
||||
});
|
||||
|
||||
it("persists requested scope when the token response omits scope", async () => {
|
||||
mockedMcpAuth.mockResolvedValue("AUTHORIZED");
|
||||
const params = createMockParams();
|
||||
storageOf(params).getTokens.mockResolvedValue({
|
||||
access_token: "access",
|
||||
token_type: "Bearer",
|
||||
});
|
||||
storageOf(params).getClientInformation.mockResolvedValue({
|
||||
client_id: "cid",
|
||||
});
|
||||
const manager = new OAuthManager(params);
|
||||
(
|
||||
manager as unknown as { pendingAuthorizationScope: string | undefined }
|
||||
).pendingAuthorizationScope = "mcp weather:read";
|
||||
|
||||
await manager.completeOAuthFlow("code");
|
||||
|
||||
expect(storageOf(params).saveScope).toHaveBeenCalledWith(
|
||||
SERVER_URL,
|
||||
"mcp weather:read",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("completeOAuthFlow (EMA)", () => {
|
||||
|
|
@ -823,6 +871,34 @@ describe("OAuthManager", () => {
|
|||
).toBe(true);
|
||||
});
|
||||
|
||||
it("returns false for invalid_token even when a locally valid token exists", async () => {
|
||||
const params = createMockParams();
|
||||
storageOf(params).getTokens.mockResolvedValue({
|
||||
access_token: "tok",
|
||||
token_type: "Bearer",
|
||||
expires_in: 3600,
|
||||
});
|
||||
const manager = new OAuthManager(params);
|
||||
|
||||
expect(
|
||||
await manager.checkAuthChallengeSatisfied({ reason: "invalid_token" }),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("returns false for unauthorized even when a locally valid token exists", async () => {
|
||||
const params = createMockParams();
|
||||
storageOf(params).getTokens.mockResolvedValue({
|
||||
access_token: "tok",
|
||||
token_type: "Bearer",
|
||||
expires_in: 3600,
|
||||
});
|
||||
const manager = new OAuthManager(params);
|
||||
|
||||
expect(
|
||||
await manager.checkAuthChallengeSatisfied({ reason: "unauthorized" }),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("returns true when stored scope covers step-up union", async () => {
|
||||
const params = createMockParams();
|
||||
storageOf(params).getTokens.mockResolvedValue({
|
||||
|
|
@ -859,6 +935,24 @@ describe("OAuthManager", () => {
|
|||
).toBe(false);
|
||||
});
|
||||
|
||||
it("ignores inflated stored scope when token scope is explicit", async () => {
|
||||
const params = createMockParams();
|
||||
storageOf(params).getTokens.mockResolvedValue({
|
||||
access_token: "tok",
|
||||
token_type: "Bearer",
|
||||
scope: "mcp",
|
||||
});
|
||||
storageOf(params).getScope.mockReturnValue("mcp weather:read");
|
||||
const manager = new OAuthManager(params);
|
||||
|
||||
expect(
|
||||
await manager.checkAuthChallengeSatisfied({
|
||||
reason: "insufficient_scope",
|
||||
requiredScopes: ["weather:read"],
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("returns false for insufficient_scope with no scopes in the challenge", async () => {
|
||||
const params = createMockParams();
|
||||
storageOf(params).getTokens.mockResolvedValue({
|
||||
|
|
|
|||
|
|
@ -121,6 +121,50 @@ describe("test server scope requirements", () => {
|
|||
expect(result.status).toBe(200);
|
||||
});
|
||||
|
||||
it("enforces requiredScopes on resource templates for resources/read", () => {
|
||||
const registry = buildScopeRequirementRegistry({
|
||||
...createMinimalConfig({ tools: [] }),
|
||||
resourceTemplates: [
|
||||
{
|
||||
name: "files",
|
||||
uriTemplate: "file:///{path}",
|
||||
requiredScopes: ["files:read"],
|
||||
handler: async () => ({
|
||||
contents: [{ uri: "file:///tmp/x", text: "ok" }],
|
||||
}),
|
||||
},
|
||||
],
|
||||
});
|
||||
expect(registry.resourceTemplates.get("file:///{path}")).toEqual([
|
||||
"files:read",
|
||||
]);
|
||||
|
||||
const denied = invokeScopeMiddleware(
|
||||
registry,
|
||||
{
|
||||
jsonrpc: "2.0",
|
||||
id: 2,
|
||||
method: "resources/read",
|
||||
params: { uri: "file:///tmp/example.txt" },
|
||||
},
|
||||
mintTestAccessToken("mcp"),
|
||||
);
|
||||
expect(denied.next).toBe(false);
|
||||
expect(denied.status).toBe(403);
|
||||
|
||||
const allowed = invokeScopeMiddleware(
|
||||
registry,
|
||||
{
|
||||
jsonrpc: "2.0",
|
||||
id: 3,
|
||||
method: "resources/read",
|
||||
params: { uri: "file:///tmp/example.txt" },
|
||||
},
|
||||
mintTestAccessToken("mcp files:read"),
|
||||
);
|
||||
expect(allowed.next).toBe(true);
|
||||
});
|
||||
|
||||
it("uses oauthTokenScopes attached by bearer middleware (external JWT path)", () => {
|
||||
const registry = buildScopeRequirementRegistry({
|
||||
...createMinimalConfig(),
|
||||
|
|
|
|||
|
|
@ -267,6 +267,18 @@ describe("RemoteSession", () => {
|
|||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("cancelRequestWait clears the timeout for a pending wait", async () => {
|
||||
vi.useFakeTimers();
|
||||
const session = new RemoteSession("s-cancel");
|
||||
const wait = session.waitForRequestResponse(42, 5000);
|
||||
const rejection = expect(wait).rejects.toThrow(/cancelled/);
|
||||
session.cancelRequestWait(42);
|
||||
await rejection;
|
||||
expect(vi.getTimerCount()).toBe(0);
|
||||
await vi.advanceTimersByTimeAsync(5000);
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("setAuthState updates the session auth provider", async () => {
|
||||
const { createRemoteAuthProvider } =
|
||||
await import("@inspector/core/mcp/remote/node/tokenAuthProvider.js");
|
||||
|
|
|
|||
Loading…
Reference in a new issue