Skip to content

Commit e1bb086

Browse files
BobDickinsonBobDickinson
committed
Implemented OAuthManager for InspectorClient
1 parent 2968a6f commit e1bb086

4 files changed

Lines changed: 683 additions & 363 deletions

File tree

‎core/__tests__/mcp/oauthManager.test.ts‎

Lines changed: 224 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,224 @@
1+
/**
2+
* OAuthManager unit tests. Uses mocked getServerUrl, fetch, storage, and
3+
* dispatch callbacks to verify config merge, callback invocation, clearOAuthTokens,
4+
* error propagation, and getOAuthState/getOAuthStep after beginGuidedAuth.
5+
*/
6+
import { describe, it, expect, vi } from "vitest";
7+
import {
8+
OAuthManager,
9+
type OAuthManagerConfig,
10+
type OAuthManagerParams,
11+
} from "../../mcp/oauthManager.js";
12+
13+
const SERVER_URL = "https://example.com/mcp";
14+
15+
function createMockParams(
16+
overrides?: Partial<OAuthManagerParams>,
17+
): OAuthManagerParams {
18+
const dispatchOAuthStepChange = vi.fn();
19+
const dispatchOAuthComplete = vi.fn();
20+
const dispatchOAuthAuthorizationRequired = vi.fn();
21+
const dispatchOAuthError = vi.fn();
22+
23+
const storage = {
24+
getScope: vi.fn().mockResolvedValue(undefined),
25+
getClientInformation: vi.fn().mockResolvedValue(undefined),
26+
saveClientInformation: vi.fn().mockResolvedValue(undefined),
27+
savePreregisteredClientInformation: vi.fn().mockResolvedValue(undefined),
28+
saveScope: vi.fn().mockResolvedValue(undefined),
29+
getTokens: vi.fn().mockResolvedValue(undefined),
30+
saveTokens: vi.fn().mockResolvedValue(undefined),
31+
getCodeVerifier: vi.fn().mockReturnValue("verifier"),
32+
saveCodeVerifier: vi.fn().mockResolvedValue(undefined),
33+
clear: vi.fn(),
34+
clearClientInformation: vi.fn(),
35+
clearTokens: vi.fn(),
36+
clearCodeVerifier: vi.fn(),
37+
clearScope: vi.fn(),
38+
clearServerMetadata: vi.fn(),
39+
getServerMetadata: vi.fn().mockReturnValue(null),
40+
saveServerMetadata: vi.fn().mockResolvedValue(undefined),
41+
};
42+
43+
const redirectUrlProvider = {
44+
getRedirectUrl: vi.fn().mockReturnValue("http://localhost/callback"),
45+
};
46+
47+
const navigation = {
48+
navigateToAuthorization: vi.fn(),
49+
};
50+
51+
const initialConfig: OAuthManagerConfig = {
52+
storage,
53+
redirectUrlProvider,
54+
navigation,
55+
clientId: "test-client",
56+
clientSecret: "test-secret",
57+
};
58+
59+
return {
60+
getServerUrl: vi.fn().mockReturnValue(SERVER_URL),
61+
effectiveAuthFetch: vi.fn().mockResolvedValue(new Response("{}")),
62+
getEventTarget: vi.fn().mockReturnValue(new EventTarget()),
63+
initialConfig,
64+
dispatchOAuthStepChange,
65+
dispatchOAuthComplete,
66+
dispatchOAuthAuthorizationRequired,
67+
dispatchOAuthError,
68+
...overrides,
69+
};
70+
}
71+
72+
describe("OAuthManager", () => {
73+
describe("setOAuthConfig", () => {
74+
it("merges config without throwing", () => {
75+
const params = createMockParams();
76+
const manager = new OAuthManager(params);
77+
expect(() => {
78+
manager.setOAuthConfig({ scope: "read write" });
79+
manager.setOAuthConfig({ clientId: "new-id" });
80+
}).not.toThrow();
81+
});
82+
});
83+
84+
describe("getServerUrl propagation", () => {
85+
it("createOAuthProviderForTransport throws when getServerUrl throws", async () => {
86+
const params = createMockParams({
87+
getServerUrl: vi.fn().mockImplementation(() => {
88+
throw new Error("OAuth is only supported for HTTP-based transports");
89+
}),
90+
});
91+
const manager = new OAuthManager(params);
92+
await expect(manager.createOAuthProviderForTransport()).rejects.toThrow(
93+
"OAuth is only supported for HTTP-based transports",
94+
);
95+
});
96+
});
97+
98+
describe("clearOAuthTokens", () => {
99+
it("calls storage.clear(serverUrl) when storage is configured", () => {
100+
const params = createMockParams();
101+
const manager = new OAuthManager(params);
102+
manager.clearOAuthTokens();
103+
expect(params.initialConfig.storage!.clear).toHaveBeenCalledWith(
104+
SERVER_URL,
105+
);
106+
expect(manager.getOAuthState()).toBeUndefined();
107+
expect(manager.getOAuthStep()).toBeUndefined();
108+
});
109+
110+
it("no-ops when storage is not configured", () => {
111+
const params = createMockParams({
112+
initialConfig: {
113+
redirectUrlProvider: {
114+
getRedirectUrl: vi.fn().mockReturnValue("http://localhost"),
115+
},
116+
navigation: { navigateToAuthorization: vi.fn() },
117+
} as OAuthManagerConfig,
118+
});
119+
const manager = new OAuthManager(params);
120+
manager.clearOAuthTokens();
121+
expect(params.getServerUrl).not.toHaveBeenCalled();
122+
});
123+
});
124+
125+
describe("getOAuthState / getOAuthStep", () => {
126+
it("returns undefined before any flow", () => {
127+
const params = createMockParams();
128+
const manager = new OAuthManager(params);
129+
expect(manager.getOAuthState()).toBeUndefined();
130+
expect(manager.getOAuthStep()).toBeUndefined();
131+
});
132+
});
133+
134+
describe("dispatch callbacks", () => {
135+
it("completeOAuthFlow calls dispatchOAuthError when normal path throws", async () => {
136+
const params = createMockParams();
137+
const manager = new OAuthManager(params);
138+
// Normal path (no guided state): auth() will run and fail (no real server), so catch calls dispatchOAuthError
139+
await expect(manager.completeOAuthFlow("bad-code")).rejects.toThrow();
140+
expect(params.dispatchOAuthError).toHaveBeenCalledWith(
141+
expect.objectContaining({
142+
error: expect.any(Error),
143+
}),
144+
);
145+
});
146+
});
147+
148+
describe("getOAuthTokens", () => {
149+
it("returns undefined when not authorized", async () => {
150+
const params = createMockParams();
151+
(
152+
params.initialConfig.storage as unknown as {
153+
getTokens: ReturnType<typeof vi.fn>;
154+
}
155+
).getTokens.mockResolvedValue(undefined);
156+
const manager = new OAuthManager(params);
157+
const tokens = await manager.getOAuthTokens();
158+
expect(tokens).toBeUndefined();
159+
});
160+
161+
it("returns tokens from storage when no in-memory state", async () => {
162+
const params = createMockParams();
163+
const storedTokens = {
164+
access_token: "stored-token",
165+
token_type: "Bearer",
166+
};
167+
(
168+
params.initialConfig.storage as unknown as {
169+
getTokens: ReturnType<typeof vi.fn>;
170+
}
171+
).getTokens.mockResolvedValue(storedTokens);
172+
const manager = new OAuthManager(params);
173+
const tokens = await manager.getOAuthTokens();
174+
expect(tokens).toEqual(storedTokens);
175+
});
176+
});
177+
178+
describe("isOAuthAuthorized", () => {
179+
it("returns false when getOAuthTokens returns undefined", async () => {
180+
const params = createMockParams();
181+
(
182+
params.initialConfig.storage as unknown as {
183+
getTokens: ReturnType<typeof vi.fn>;
184+
}
185+
).getTokens.mockResolvedValue(undefined);
186+
const manager = new OAuthManager(params);
187+
expect(await manager.isOAuthAuthorized()).toBe(false);
188+
});
189+
190+
it("returns true when getOAuthTokens returns tokens", async () => {
191+
const params = createMockParams();
192+
(
193+
params.initialConfig.storage as unknown as {
194+
getTokens: ReturnType<typeof vi.fn>;
195+
}
196+
).getTokens.mockResolvedValue({
197+
access_token: "x",
198+
token_type: "Bearer",
199+
});
200+
const manager = new OAuthManager(params);
201+
expect(await manager.isOAuthAuthorized()).toBe(true);
202+
});
203+
});
204+
205+
describe("setGuidedAuthorizationCode", () => {
206+
it("throws when not in guided flow", async () => {
207+
const params = createMockParams();
208+
const manager = new OAuthManager(params);
209+
await expect(
210+
manager.setGuidedAuthorizationCode("code", true),
211+
).rejects.toThrow("Not in guided OAuth flow");
212+
});
213+
});
214+
215+
describe("proceedOAuthStep", () => {
216+
it("throws when not in guided flow", async () => {
217+
const params = createMockParams();
218+
const manager = new OAuthManager(params);
219+
await expect(manager.proceedOAuthStep()).rejects.toThrow(
220+
"Not in guided OAuth flow",
221+
);
222+
});
223+
});
224+
});

‎core/__tests__/storage-adapters.test.ts‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,13 @@ describe("Storage adapters", () => {
7474
tokens: { access_token: "test-token", token_type: "Bearer" },
7575
});
7676

77-
// Wait for persistence
78-
await new Promise((resolve) => setTimeout(resolve, 100));
79-
80-
// Verify file exists and contains state
81-
expect(existsSync(filePath)).toBe(true);
77+
// Wait for persistence (Zustand persist is async; poll for file so we don't race with cleanup)
78+
await vi.waitFor(
79+
() => {
80+
expect(existsSync(filePath)).toBe(true);
81+
},
82+
{ timeout: 2000, interval: 20 },
83+
);
8284
const fileContent = readFileSync(filePath, "utf-8");
8385
const parsed = JSON.parse(fileContent);
8486
expect(parsed.state.servers["https://example.com"].tokens).toEqual({
@@ -89,19 +91,20 @@ describe("Storage adapters", () => {
8991

9092
it("loads persisted state on initialization", async () => {
9193
tempDir = mkdtempSync(join(tmpdir(), "inspector-storage-test-"));
92-
const filePath = join(
93-
tmpdir(),
94-
"inspector-storage-test-",
95-
"test-store.json",
96-
);
94+
const filePath = join(tempDir!, "test-store.json");
9795

9896
// Create initial store and persist
9997
const storage1 = createFileStorageAdapter({ filePath });
10098
const store1 = createOAuthStore(storage1);
10199
store1.getState().setServerState("https://example.com", {
102100
tokens: { access_token: "initial-token", token_type: "Bearer" },
103101
});
104-
await new Promise((resolve) => setTimeout(resolve, 100));
102+
await vi.waitFor(
103+
() => {
104+
expect(existsSync(filePath)).toBe(true);
105+
},
106+
{ timeout: 2000, interval: 20 },
107+
);
105108

106109
// Create new store instance (should load persisted state)
107110
const storage2 = createFileStorageAdapter({ filePath });

0 commit comments

Comments
 (0)