From ffeea6bfb739077821c72c950dbc99c5208fdd1b Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Thu, 8 May 2025 10:29:55 +0100 Subject: [PATCH 1/6] fix: validate creds --- src/common/atlas/apiClient.ts | 42 +++++++- src/server.ts | 19 +++- tests/integration/helpers.ts | 10 ++ tests/integration/server.test.ts | 1 + tests/unit/apiClient.test.ts | 176 +++++++++++++++++++++++++++++++ 5 files changed, 243 insertions(+), 5 deletions(-) create mode 100644 tests/unit/apiClient.test.ts diff --git a/src/common/atlas/apiClient.ts b/src/common/atlas/apiClient.ts index 0287f721..78bd688d 100644 --- a/src/common/atlas/apiClient.ts +++ b/src/common/atlas/apiClient.ts @@ -89,6 +89,11 @@ export class ApiClient { return !!(this.oauth2Client && this.accessToken); } + public async hasValidAccessToken(): Promise { + const accessToken = await this.getAccessToken(); + return accessToken !== undefined; + } + public async getIpInfo(): Promise<{ currentIpv4Address: string; }> { @@ -115,7 +120,6 @@ export class ApiClient { } async sendEvents(events: TelemetryEvent[]): Promise { - let endpoint = "api/private/unauth/telemetry/events"; const headers: Record = { Accept: "application/json", "Content-Type": "application/json", @@ -124,12 +128,41 @@ export class ApiClient { const accessToken = await this.getAccessToken(); if (accessToken) { - endpoint = "api/private/v1.0/telemetry/events"; + const authUrl = new URL("api/private/v1.0/telemetry/events", this.options.baseUrl); headers["Authorization"] = `Bearer ${accessToken}`; + + try { + const response = await fetch(authUrl, { + method: "POST", + headers, + body: JSON.stringify(events), + }); + + if (response.ok) { + return; + } + + // If anything other than 401, throw the error + if (response.status !== 401) { + throw await ApiClientError.fromResponse(response); + } + + // For 401, fall through to unauthenticated endpoint + delete headers["Authorization"]; + } catch (error) { + // If the error is not a 401, rethrow it + if (!(error instanceof ApiClientError) || error.response.status !== 401) { + throw error; + } + + // For 401 errors, fall through to unauthenticated endpoint + delete headers["Authorization"]; + } } - const url = new URL(endpoint, this.options.baseUrl); - const response = await fetch(url, { + // Send to unauthenticated endpoint (either as fallback from 401 or direct if no token) + const unauthUrl = new URL("api/private/unauth/telemetry/events", this.options.baseUrl); + const response = await fetch(unauthUrl, { method: "POST", headers, body: JSON.stringify(events), @@ -237,6 +270,7 @@ export class ApiClient { "/api/atlas/v2/groups/{groupId}/clusters/{clusterName}", options ); + if (error) { throw ApiClientError.fromError(response, error); } diff --git a/src/server.ts b/src/server.ts index 4d2df644..091ebd79 100644 --- a/src/server.ts +++ b/src/server.ts @@ -104,7 +104,7 @@ export class Server { * @param command - The server command (e.g., "start", "stop", "register", "deregister") * @param additionalProperties - Additional properties specific to the event */ - emitServerEvent(command: ServerCommand, commandDuration: number, error?: Error) { + private emitServerEvent(command: ServerCommand, commandDuration: number, error?: Error) { const event: ServerEvent = { timestamp: new Date().toISOString(), source: "mdbmcp", @@ -185,5 +185,22 @@ export class Server { throw new Error("Failed to connect to MongoDB instance using the connection string from the config"); } } + + if (this.userConfig.apiClientId && this.userConfig.apiClientSecret) { + try { + await this.session.apiClient.hasValidAccessToken(); + } catch (error) { + if (this.userConfig.connectionString === undefined) { + console.error("Failed to validate MongoDB Atlas the credentials from the config: ", error); + + throw new Error( + "Failed to connect to MongoDB Atlas instance using the credentials from the config" + ); + } + console.error( + "Failed to validate MongoDB Atlas the credentials from the config, but validated the connection string." + ); + } + } } } diff --git a/tests/integration/helpers.ts b/tests/integration/helpers.ts index b5c31b9b..d9122af7 100644 --- a/tests/integration/helpers.ts +++ b/tests/integration/helpers.ts @@ -7,6 +7,7 @@ import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { Session } from "../../src/session.js"; import { Telemetry } from "../../src/telemetry/telemetry.js"; import { config } from "../../src/config.js"; +import { jest } from "@jest/globals"; interface ParameterInfo { name: string; @@ -57,6 +58,12 @@ export function setupIntegrationTest(getUserConfig: () => UserConfig): Integrati apiClientSecret: userConfig.apiClientSecret, }); + // Mock hasValidAccessToken for tests + if (userConfig.apiClientId && userConfig.apiClientSecret) { + const mockFn = jest.fn<() => Promise>().mockResolvedValue(true); + session.apiClient.hasValidAccessToken = mockFn; + } + userConfig.telemetry = "disabled"; const telemetry = Telemetry.create(session, userConfig); @@ -70,6 +77,9 @@ export function setupIntegrationTest(getUserConfig: () => UserConfig): Integrati version: "5.2.3", }), }); + + // mock validation + await mcpServer.connect(serverTransport); await mcpClient.connect(clientTransport); }); diff --git a/tests/integration/server.test.ts b/tests/integration/server.test.ts index 3b4c1858..6a932209 100644 --- a/tests/integration/server.test.ts +++ b/tests/integration/server.test.ts @@ -1,3 +1,4 @@ +import { beforeAll } from "@jest/globals"; import { defaultTestConfig, expectDefined, setupIntegrationTest } from "./helpers.js"; import { describeWithMongoDB } from "./tools/mongodb/mongodbHelpers.js"; diff --git a/tests/unit/apiClient.test.ts b/tests/unit/apiClient.test.ts new file mode 100644 index 00000000..a6b04ef0 --- /dev/null +++ b/tests/unit/apiClient.test.ts @@ -0,0 +1,176 @@ +import { jest } from "@jest/globals"; +import { ApiClient } from "../../src/common/atlas/apiClient.js"; +import { + CommonProperties, + TelemetryEvent, + CommonStaticProperties, + TelemetryResult, +} from "../../src/telemetry/types.js"; + +describe("ApiClient", () => { + let apiClient: ApiClient; + + const mockEvents: TelemetryEvent[] = [ + { + timestamp: new Date().toISOString(), + source: "mdbmcp", + properties: { + mcp_client_version: "1.0.0", + mcp_client_name: "test-client", + mcp_server_version: "1.0.0", + mcp_server_name: "test-server", + platform: "test-platform", + arch: "test-arch", + os_type: "test-os", + component: "test-component", + duration_ms: 100, + result: "success" as TelemetryResult, + category: "test-category", + }, + }, + ]; + + beforeEach(() => { + apiClient = new ApiClient({ + baseUrl: "https://api.test.com", + credentials: { + clientId: "test-client-id", + clientSecret: "test-client-secret", + }, + }); + + // @ts-ignore - accessing private property for testing + apiClient.getAccessToken = jest.fn().mockResolvedValue("mockToken"); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + describe("constructor", () => { + it("should create a client with the correct configuration", () => { + expect(apiClient).toBeDefined(); + expect(apiClient.hasCredentials()).toBeDefined(); + }); + }); + + describe("listProjects", () => { + it("should return a list of projects", async () => { + const mockProjects = { + results: [ + { id: "1", name: "Project 1" }, + { id: "2", name: "Project 2" }, + ], + totalCount: 2, + }; + + const mockGet = jest.fn().mockImplementation(async () => ({ + data: mockProjects, + error: null, + response: new Response(), + })); + + // @ts-ignore - accessing private property for testing + apiClient.client.GET = mockGet; + + const result = await apiClient.listProjects(); + + expect(mockGet).toHaveBeenCalledWith("/api/atlas/v2/groups", undefined); + expect(result).toEqual(mockProjects); + }); + + it("should throw an error when the API call fails", async () => { + const mockError = { + reason: "Test error", + detail: "Something went wrong", + }; + + const mockGet = jest.fn().mockImplementation(async () => ({ + data: null, + error: mockError, + response: new Response(), + })); + + // @ts-ignore - accessing private property for testing + apiClient.client.GET = mockGet; + + await expect(apiClient.listProjects()).rejects.toThrow(); + }); + }); + + describe("sendEvents", () => { + it("should send events to authenticated endpoint when token is available", async () => { + const mockFetch = jest.spyOn(global, "fetch"); + mockFetch.mockResolvedValueOnce(new Response(null, { status: 200 })); + + await apiClient.sendEvents(mockEvents); + + const url = new URL("api/private/v1.0/telemetry/events", "https://api.test.com"); + expect(mockFetch).toHaveBeenCalledWith(url, { + method: "POST", + headers: { + "Content-Type": "application/json", + Authorization: expect.stringContaining("Bearer"), + Accept: "application/json", + "User-Agent": expect.stringContaining("AtlasMCP"), + }, + body: JSON.stringify(mockEvents), + }); + }); + + it("should fall back to unauthenticated endpoint when token is not available", async () => { + const mockFetch = jest.spyOn(global, "fetch"); + mockFetch.mockResolvedValueOnce(new Response(null, { status: 200 })); + + // @ts-ignore - accessing private property for testing + apiClient.getAccessToken = jest.fn().mockResolvedValue(undefined); + + await apiClient.sendEvents(mockEvents); + + const url = new URL("api/private/unauth/telemetry/events", "https://api.test.com"); + expect(mockFetch).toHaveBeenCalledWith(url, { + method: "POST", + headers: { + "Content-Type": "application/json", + Accept: "application/json", + "User-Agent": expect.stringContaining("AtlasMCP"), + }, + body: JSON.stringify(mockEvents), + }); + }); + + it("should fall back to unauthenticated endpoint on 401 error", async () => { + const mockFetch = jest.spyOn(global, "fetch"); + mockFetch + .mockResolvedValueOnce(new Response(null, { status: 401 })) + .mockResolvedValueOnce(new Response(null, { status: 200 })); + + await apiClient.sendEvents(mockEvents); + + const url = new URL("api/private/unauth/telemetry/events", "https://api.test.com"); + expect(mockFetch).toHaveBeenCalledTimes(2); + expect(mockFetch).toHaveBeenLastCalledWith(url, { + method: "POST", + headers: { + "Content-Type": "application/json", + Accept: "application/json", + "User-Agent": expect.stringContaining("AtlasMCP"), + }, + body: JSON.stringify(mockEvents), + }); + }); + + it("should throw error when both authenticated and unauthenticated requests fail", async () => { + const mockFetch = jest.spyOn(global, "fetch"); + mockFetch + .mockResolvedValueOnce(new Response(null, { status: 401 })) + .mockResolvedValueOnce(new Response(null, { status: 500 })); + + const mockToken = "test-token"; + // @ts-ignore - accessing private property for testing + apiClient.getAccessToken = jest.fn().mockResolvedValue(mockToken); + + await expect(apiClient.sendEvents(mockEvents)).rejects.toThrow(); + }); + }); +}); From ef8356960bf2904ef3b826dc25f985700deead37 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Thu, 8 May 2025 10:56:45 +0100 Subject: [PATCH 2/6] lint --- tests/integration/server.test.ts | 1 - tests/unit/apiClient.test.ts | 7 +------ 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/tests/integration/server.test.ts b/tests/integration/server.test.ts index 6a932209..3b4c1858 100644 --- a/tests/integration/server.test.ts +++ b/tests/integration/server.test.ts @@ -1,4 +1,3 @@ -import { beforeAll } from "@jest/globals"; import { defaultTestConfig, expectDefined, setupIntegrationTest } from "./helpers.js"; import { describeWithMongoDB } from "./tools/mongodb/mongodbHelpers.js"; diff --git a/tests/unit/apiClient.test.ts b/tests/unit/apiClient.test.ts index a6b04ef0..f0d95484 100644 --- a/tests/unit/apiClient.test.ts +++ b/tests/unit/apiClient.test.ts @@ -1,11 +1,6 @@ import { jest } from "@jest/globals"; import { ApiClient } from "../../src/common/atlas/apiClient.js"; -import { - CommonProperties, - TelemetryEvent, - CommonStaticProperties, - TelemetryResult, -} from "../../src/telemetry/types.js"; +import { CommonProperties, TelemetryEvent, TelemetryResult } from "../../src/telemetry/types.js"; describe("ApiClient", () => { let apiClient: ApiClient; From 7139a2c1886911937b83afa99ef7d0c095822667 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Thu, 8 May 2025 10:58:55 +0100 Subject: [PATCH 3/6] lint --- tests/unit/apiClient.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/unit/apiClient.test.ts b/tests/unit/apiClient.test.ts index f0d95484..97328e99 100644 --- a/tests/unit/apiClient.test.ts +++ b/tests/unit/apiClient.test.ts @@ -34,7 +34,7 @@ describe("ApiClient", () => { }, }); - // @ts-ignore - accessing private property for testing + // @ts-expect-error accessing private property for testing apiClient.getAccessToken = jest.fn().mockResolvedValue("mockToken"); }); @@ -65,7 +65,7 @@ describe("ApiClient", () => { response: new Response(), })); - // @ts-ignore - accessing private property for testing + // @ts-expect-error accessing private property for testing apiClient.client.GET = mockGet; const result = await apiClient.listProjects(); @@ -86,7 +86,7 @@ describe("ApiClient", () => { response: new Response(), })); - // @ts-ignore - accessing private property for testing + // @ts-expect-error accessing private property for testing apiClient.client.GET = mockGet; await expect(apiClient.listProjects()).rejects.toThrow(); @@ -117,7 +117,7 @@ describe("ApiClient", () => { const mockFetch = jest.spyOn(global, "fetch"); mockFetch.mockResolvedValueOnce(new Response(null, { status: 200 })); - // @ts-ignore - accessing private property for testing + // @ts-expect-error accessing private property for testing apiClient.getAccessToken = jest.fn().mockResolvedValue(undefined); await apiClient.sendEvents(mockEvents); @@ -162,7 +162,7 @@ describe("ApiClient", () => { .mockResolvedValueOnce(new Response(null, { status: 500 })); const mockToken = "test-token"; - // @ts-ignore - accessing private property for testing + // @ts-expect-error accessing private property for testing apiClient.getAccessToken = jest.fn().mockResolvedValue(mockToken); await expect(apiClient.sendEvents(mockEvents)).rejects.toThrow(); From 8ebc22d5bea699fd0c7bec57370809b515cb73f3 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Thu, 8 May 2025 11:02:42 +0100 Subject: [PATCH 4/6] lint --- tests/unit/apiClient.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/apiClient.test.ts b/tests/unit/apiClient.test.ts index 97328e99..21cc07fd 100644 --- a/tests/unit/apiClient.test.ts +++ b/tests/unit/apiClient.test.ts @@ -59,7 +59,7 @@ describe("ApiClient", () => { totalCount: 2, }; - const mockGet = jest.fn().mockImplementation(async () => ({ + const mockGet = jest.fn().mockImplementation(() => ({ data: mockProjects, error: null, response: new Response(), @@ -80,7 +80,7 @@ describe("ApiClient", () => { detail: "Something went wrong", }; - const mockGet = jest.fn().mockImplementation(async () => ({ + const mockGet = jest.fn().mockImplementation(() => ({ data: null, error: mockError, response: new Response(), From 24692fef55e64397e7cadf559031de4d3f0306a0 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Thu, 8 May 2025 11:23:56 +0100 Subject: [PATCH 5/6] lint --- tests/unit/apiClient.test.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/unit/apiClient.test.ts b/tests/unit/apiClient.test.ts index 21cc07fd..a704e6b7 100644 --- a/tests/unit/apiClient.test.ts +++ b/tests/unit/apiClient.test.ts @@ -32,6 +32,7 @@ describe("ApiClient", () => { clientId: "test-client-id", clientSecret: "test-client-secret", }, + userAgent: "test-user-agent", }); // @ts-expect-error accessing private property for testing @@ -105,9 +106,9 @@ describe("ApiClient", () => { method: "POST", headers: { "Content-Type": "application/json", - Authorization: expect.stringContaining("Bearer"), + Authorization: "Bearer mockToken", Accept: "application/json", - "User-Agent": expect.stringContaining("AtlasMCP"), + "User-Agent": "test-user-agent", }, body: JSON.stringify(mockEvents), }); @@ -128,7 +129,7 @@ describe("ApiClient", () => { headers: { "Content-Type": "application/json", Accept: "application/json", - "User-Agent": expect.stringContaining("AtlasMCP"), + "User-Agent": "test-user-agent", }, body: JSON.stringify(mockEvents), }); @@ -149,7 +150,7 @@ describe("ApiClient", () => { headers: { "Content-Type": "application/json", Accept: "application/json", - "User-Agent": expect.stringContaining("AtlasMCP"), + "User-Agent": "test-user-agent", }, body: JSON.stringify(mockEvents), }); From 93402e34e5535978a45b79a8191d92a4a9ecaba5 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Thu, 8 May 2025 11:24:34 +0100 Subject: [PATCH 6/6] clean up --- tests/integration/helpers.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/integration/helpers.ts b/tests/integration/helpers.ts index d9122af7..308c9a3d 100644 --- a/tests/integration/helpers.ts +++ b/tests/integration/helpers.ts @@ -78,8 +78,6 @@ export function setupIntegrationTest(getUserConfig: () => UserConfig): Integrati }), }); - // mock validation - await mcpServer.connect(serverTransport); await mcpClient.connect(clientTransport); });