From 3d079c6a3aeaec11f9d3762fcb9c66017c694058 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Mon, 14 Jul 2025 17:57:17 +0100 Subject: [PATCH 01/13] WIP --- README.md | 2 + src/common/atlas/ensureAccessList.ts | 29 +++++++++++ src/tools/atlas/connect/connectCluster.ts | 2 + src/tools/atlas/create/createDBUser.ts | 2 + src/tools/atlas/create/createFreeCluster.ts | 2 + .../tools/atlas/accessLists.test.ts | 19 ++++++++ .../integration/tools/atlas/clusters.test.ts | 14 ++++-- tests/integration/tools/atlas/dbUsers.test.ts | 12 +++++ tests/unit/ensureAccessList.test.ts | 48 +++++++++++++++++++ 9 files changed, 127 insertions(+), 3 deletions(-) create mode 100644 src/common/atlas/ensureAccessList.ts create mode 100644 tests/unit/ensureAccessList.test.ts diff --git a/README.md b/README.md index 8a9a6a0d..4edbc318 100644 --- a/README.md +++ b/README.md @@ -374,10 +374,12 @@ To use the Atlas API tools, you'll need to create a service account in MongoDB A To learn more about Service Accounts, check the [MongoDB Atlas documentation](https://www.mongodb.com/docs/atlas/api/service-accounts-overview/). 2. **Save Client Credentials:** + - After creation, you'll be shown the Client ID and Client Secret - **Important:** Copy and save the Client Secret immediately as it won't be displayed again 3. **Add Access List Entry:** + - Add your IP address to the API access list 4. **Configure the MCP Server:** diff --git a/src/common/atlas/ensureAccessList.ts b/src/common/atlas/ensureAccessList.ts new file mode 100644 index 00000000..784a31a9 --- /dev/null +++ b/src/common/atlas/ensureAccessList.ts @@ -0,0 +1,29 @@ +import { ApiClientError } from "./apiClientError.js"; + +/** + * Ensures the current public IP is in the access list for the given Atlas project. + * If the IP is already present, this is a no-op. + * @param apiClient The Atlas API client instance + * @param projectId The Atlas project ID + */ +export async function ensureCurrentIpInAccessList(apiClient: any, projectId: string): Promise { + // Get the current public IP + const { currentIpv4Address } = await apiClient.getIpInfo(); + const entry = { + groupId: projectId, + ipAddress: currentIpv4Address, + comment: "Added by MCP pre-run access list helper", + }; + try { + await apiClient.createProjectIpAccessList({ + params: { path: { groupId: projectId } }, + body: [entry], + }); + } catch (err) { + if (err instanceof ApiClientError && err.response?.status === 409) { + // 409 Conflict: entry already exists, ignore + return; + } + throw err; + } +} diff --git a/src/tools/atlas/connect/connectCluster.ts b/src/tools/atlas/connect/connectCluster.ts index d8cda77d..40a352d8 100644 --- a/src/tools/atlas/connect/connectCluster.ts +++ b/src/tools/atlas/connect/connectCluster.ts @@ -5,6 +5,7 @@ import { ToolArgs, OperationType } from "../../tool.js"; import { generateSecurePassword } from "../../../helpers/generatePassword.js"; import logger, { LogId } from "../../../common/logger.js"; import { inspectCluster } from "../../../common/atlas/cluster.js"; +import { ensureCurrentIpInAccessList } from "../../../common/atlas/ensureAccessList.js"; const EXPIRY_MS = 1000 * 60 * 60 * 12; // 12 hours @@ -198,6 +199,7 @@ export class ConnectClusterTool extends AtlasToolBase { } protected async execute({ projectId, clusterName }: ToolArgs): Promise { + await ensureCurrentIpInAccessList(this.session.apiClient, projectId); for (let i = 0; i < 60; i++) { const state = await this.queryConnection(projectId, clusterName); switch (state) { diff --git a/src/tools/atlas/create/createDBUser.ts b/src/tools/atlas/create/createDBUser.ts index d2133a04..81f1d609 100644 --- a/src/tools/atlas/create/createDBUser.ts +++ b/src/tools/atlas/create/createDBUser.ts @@ -4,6 +4,7 @@ import { AtlasToolBase } from "../atlasTool.js"; import { ToolArgs, OperationType } from "../../tool.js"; import { CloudDatabaseUser, DatabaseUserRole } from "../../../common/atlas/openapi.js"; import { generateSecurePassword } from "../../../helpers/generatePassword.js"; +import { ensureCurrentIpInAccessList } from "../../../common/atlas/ensureAccessList.js"; export class CreateDBUserTool extends AtlasToolBase { public name = "atlas-create-db-user"; @@ -44,6 +45,7 @@ export class CreateDBUserTool extends AtlasToolBase { roles, clusters, }: ToolArgs): Promise { + await ensureCurrentIpInAccessList(this.session.apiClient, projectId); const shouldGeneratePassword = !password; if (shouldGeneratePassword) { password = await generateSecurePassword(); diff --git a/src/tools/atlas/create/createFreeCluster.ts b/src/tools/atlas/create/createFreeCluster.ts index ed04409b..74bf2295 100644 --- a/src/tools/atlas/create/createFreeCluster.ts +++ b/src/tools/atlas/create/createFreeCluster.ts @@ -3,6 +3,7 @@ import { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { AtlasToolBase } from "../atlasTool.js"; import { ToolArgs, OperationType } from "../../tool.js"; import { ClusterDescription20240805 } from "../../../common/atlas/openapi.js"; +import { ensureCurrentIpInAccessList } from "../../../common/atlas/ensureAccessList.js"; export class CreateFreeClusterTool extends AtlasToolBase { public name = "atlas-create-free-cluster"; @@ -37,6 +38,7 @@ export class CreateFreeClusterTool extends AtlasToolBase { terminationProtectionEnabled: false, } as unknown as ClusterDescription20240805; + await ensureCurrentIpInAccessList(this.session.apiClient, projectId); await this.session.apiClient.createCluster({ params: { path: { diff --git a/tests/integration/tools/atlas/accessLists.test.ts b/tests/integration/tools/atlas/accessLists.test.ts index d2dc219c..75a5b4ca 100644 --- a/tests/integration/tools/atlas/accessLists.test.ts +++ b/tests/integration/tools/atlas/accessLists.test.ts @@ -1,6 +1,7 @@ import { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { describeWithAtlas, withProject } from "./atlasHelpers.js"; import { expectDefined } from "../../helpers.js"; +import { ensureCurrentIpInAccessList } from "../../../../src/common/atlas/ensureAccessList.js"; function generateRandomIp() { const randomIp: number[] = [192]; @@ -94,5 +95,23 @@ describeWithAtlas("ip access lists", (integration) => { } }); }); + + describe("ensureCurrentIpInAccessList helper", () => { + it("should add the current IP to the access list and be idempotent", async () => { + const apiClient = integration.mcpServer().session.apiClient; + const projectId = getProjectId(); + const ipInfo = await apiClient.getIpInfo(); + // First call should add the IP + await expect(ensureCurrentIpInAccessList(apiClient, projectId)).resolves.not.toThrow(); + // Second call should be a no-op (idempotent) + await expect(ensureCurrentIpInAccessList(apiClient, projectId)).resolves.not.toThrow(); + // Check that the IP is present in the access list + const accessList = await apiClient.listProjectIpAccessLists({ + params: { path: { groupId: projectId } }, + }); + const found = accessList.results?.some((entry) => entry.ipAddress === ipInfo.currentIpv4Address); + expect(found).toBe(true); + }); + }); }); }); diff --git a/tests/integration/tools/atlas/clusters.test.ts b/tests/integration/tools/atlas/clusters.test.ts index 3916e511..48f11288 100644 --- a/tests/integration/tools/atlas/clusters.test.ts +++ b/tests/integration/tools/atlas/clusters.test.ts @@ -81,20 +81,28 @@ describeWithAtlas("clusters", (integration) => { expect(createFreeCluster.inputSchema.properties).toHaveProperty("region"); }); - it("should create a free cluster", async () => { + it("should create a free cluster and add current IP to access list", async () => { const projectId = getProjectId(); + const session = integration.mcpServer().session; + const ipInfo = await session.apiClient.getIpInfo(); const response = (await integration.mcpClient().callTool({ name: "atlas-create-free-cluster", arguments: { projectId, - name: clusterName, + name: clusterName + "-iptest", region: "US_EAST_1", }, })) as CallToolResult; expect(response.content).toBeArray(); - expect(response.content).toHaveLength(2); expect(response.content[0]?.text).toContain("has been created"); + + // Check that the current IP is present in the access list + const accessList = await session.apiClient.listProjectIpAccessLists({ + params: { path: { groupId: projectId } }, + }); + const found = accessList.results?.some((entry) => entry.ipAddress === ipInfo.currentIpv4Address); + expect(found).toBe(true); }); }); diff --git a/tests/integration/tools/atlas/dbUsers.test.ts b/tests/integration/tools/atlas/dbUsers.test.ts index 3bfb979e..5ac02cb1 100644 --- a/tests/integration/tools/atlas/dbUsers.test.ts +++ b/tests/integration/tools/atlas/dbUsers.test.ts @@ -78,6 +78,18 @@ describeWithAtlas("db users", (integration) => { expect(elements[0]?.text).toContain(userName); expect(elements[0]?.text).toContain("with password: `"); }); + + it("should add current IP to access list when creating a database user", async () => { + const projectId = getProjectId(); + const session = integration.mcpServer().session; + const ipInfo = await session.apiClient.getIpInfo(); + await createUserWithMCP(); + const accessList = await session.apiClient.listProjectIpAccessLists({ + params: { path: { groupId: projectId } }, + }); + const found = accessList.results?.some((entry) => entry.ipAddress === ipInfo.currentIpv4Address); + expect(found).toBe(true); + }); }); describe("atlas-list-db-users", () => { it("should have correct metadata", async () => { diff --git a/tests/unit/ensureAccessList.test.ts b/tests/unit/ensureAccessList.test.ts new file mode 100644 index 00000000..e5c6dd01 --- /dev/null +++ b/tests/unit/ensureAccessList.test.ts @@ -0,0 +1,48 @@ +import { ensureCurrentIpInAccessList } from "../../src/common/atlas/ensureAccessList.js"; +import { ApiClientError } from "../../src/common/atlas/apiClientError.js"; + +describe("ensureCurrentIpInAccessList", () => { + const projectId = "test-project-id"; + const ip = "1.2.3.4"; + let apiClient: any; + + beforeEach(() => { + apiClient = { + getIpInfo: jest.fn().mockResolvedValue({ currentIpv4Address: ip }), + createProjectIpAccessList: jest.fn().mockResolvedValue(undefined), + }; + }); + + it("adds the current IP to the access list", async () => { + await expect(ensureCurrentIpInAccessList(apiClient, projectId)).resolves.not.toThrow(); + expect(apiClient.getIpInfo).toHaveBeenCalled(); + expect(apiClient.createProjectIpAccessList).toHaveBeenCalledWith({ + params: { path: { groupId: projectId } }, + body: [ + { + groupId: projectId, + ipAddress: ip, + comment: expect.any(String), + }, + ], + }); + }); + + it("is idempotent if the IP is already present (409 error)", async () => { + apiClient.createProjectIpAccessList.mockRejectedValueOnce( + Object.assign(new ApiClientError("Conflict", { status: 409, statusText: "Conflict" } as any), { + response: { status: 409 }, + }) + ); + await expect(ensureCurrentIpInAccessList(apiClient, projectId)).resolves.not.toThrow(); + }); + + it("throws for other errors", async () => { + apiClient.createProjectIpAccessList.mockRejectedValueOnce( + Object.assign(new ApiClientError("Other", { status: 500, statusText: "Server Error" } as any), { + response: { status: 500 }, + }) + ); + await expect(ensureCurrentIpInAccessList(apiClient, projectId)).rejects.toThrow(); + }); +}); From 43cf255a38afec30f0027e6fac31c0f64d1f2db2 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Tue, 15 Jul 2025 13:38:32 +0100 Subject: [PATCH 02/13] MCP-5: Improve accesslist experience --- README.md | 2 - src/common/atlas/accessListUtils.ts | 44 +++++++++++++++++ src/common/atlas/ensureAccessList.ts | 30 +----------- src/common/logger.ts | 1 + src/tools/atlas/connect/connectCluster.ts | 15 +++++- src/tools/atlas/create/createAccessList.ts | 12 ++--- src/tools/atlas/create/createDBUser.ts | 2 +- src/tools/atlas/create/createFreeCluster.ts | 2 +- .../tools/atlas/accessLists.test.ts | 2 +- tests/unit/accessListUtils.test.ts | 30 ++++++++++++ tests/unit/ensureAccessList.test.ts | 48 ------------------- 11 files changed, 99 insertions(+), 89 deletions(-) create mode 100644 src/common/atlas/accessListUtils.ts create mode 100644 tests/unit/accessListUtils.test.ts delete mode 100644 tests/unit/ensureAccessList.test.ts diff --git a/README.md b/README.md index 4edbc318..8a9a6a0d 100644 --- a/README.md +++ b/README.md @@ -374,12 +374,10 @@ To use the Atlas API tools, you'll need to create a service account in MongoDB A To learn more about Service Accounts, check the [MongoDB Atlas documentation](https://www.mongodb.com/docs/atlas/api/service-accounts-overview/). 2. **Save Client Credentials:** - - After creation, you'll be shown the Client ID and Client Secret - **Important:** Copy and save the Client Secret immediately as it won't be displayed again 3. **Add Access List Entry:** - - Add your IP address to the API access list 4. **Configure the MCP Server:** diff --git a/src/common/atlas/accessListUtils.ts b/src/common/atlas/accessListUtils.ts new file mode 100644 index 00000000..e26e3edb --- /dev/null +++ b/src/common/atlas/accessListUtils.ts @@ -0,0 +1,44 @@ +import { ApiClient } from "./apiClient.js"; +import logger, { LogId } from "../logger.js"; +import { ApiClientError } from "./apiClientError.js"; + +export async function makeCurrentIpAccessListEntry( + apiClient: ApiClient, + projectId: string, + comment: string = "Added by Atlas MCP" +) { + const { currentIpv4Address } = await apiClient.getIpInfo(); + return { + groupId: projectId, + ipAddress: currentIpv4Address, + comment, + }; +} + +/** + * Ensures the current public IP is in the access list for the given Atlas project. + * If the IP is already present, this is a no-op. + * @param apiClient The Atlas API client instance + * @param projectId The Atlas project ID + */ +export async function ensureCurrentIpInAccessList(apiClient: ApiClient, projectId: string): Promise { + // Get the current public IP + const entry = await makeCurrentIpAccessListEntry(apiClient, projectId, "Added by MCP pre-run access list helper"); + try { + await apiClient.createProjectIpAccessList({ + params: { path: { groupId: projectId } }, + body: [entry], + }); + logger.debug( + LogId.atlasIpAccessListAdded, + "atlas-connect-cluster", + `IP access list created: ${JSON.stringify(entry)}` + ); + } catch (err) { + if (err instanceof ApiClientError && err.response?.status === 409) { + // 409 Conflict: entry already exists, ignore + return; + } + throw err; + } +} diff --git a/src/common/atlas/ensureAccessList.ts b/src/common/atlas/ensureAccessList.ts index 784a31a9..ae17dd2c 100644 --- a/src/common/atlas/ensureAccessList.ts +++ b/src/common/atlas/ensureAccessList.ts @@ -1,29 +1 @@ -import { ApiClientError } from "./apiClientError.js"; - -/** - * Ensures the current public IP is in the access list for the given Atlas project. - * If the IP is already present, this is a no-op. - * @param apiClient The Atlas API client instance - * @param projectId The Atlas project ID - */ -export async function ensureCurrentIpInAccessList(apiClient: any, projectId: string): Promise { - // Get the current public IP - const { currentIpv4Address } = await apiClient.getIpInfo(); - const entry = { - groupId: projectId, - ipAddress: currentIpv4Address, - comment: "Added by MCP pre-run access list helper", - }; - try { - await apiClient.createProjectIpAccessList({ - params: { path: { groupId: projectId } }, - body: [entry], - }); - } catch (err) { - if (err instanceof ApiClientError && err.response?.status === 409) { - // 409 Conflict: entry already exists, ignore - return; - } - throw err; - } -} +// This file has been migrated. ensureCurrentIpInAccessList is now in accessListUtils.ts diff --git a/src/common/logger.ts b/src/common/logger.ts index b1fb78a9..b1c82818 100644 --- a/src/common/logger.ts +++ b/src/common/logger.ts @@ -20,6 +20,7 @@ export const LogId = { atlasConnectAttempt: mongoLogId(1_001_005), atlasConnectSucceeded: mongoLogId(1_001_006), atlasApiRevokeFailure: mongoLogId(1_001_007), + atlasIpAccessListAdded: mongoLogId(1_001_008), telemetryDisabled: mongoLogId(1_002_001), telemetryEmitFailure: mongoLogId(1_002_002), diff --git a/src/tools/atlas/connect/connectCluster.ts b/src/tools/atlas/connect/connectCluster.ts index 40a352d8..4843aa30 100644 --- a/src/tools/atlas/connect/connectCluster.ts +++ b/src/tools/atlas/connect/connectCluster.ts @@ -5,7 +5,7 @@ import { ToolArgs, OperationType } from "../../tool.js"; import { generateSecurePassword } from "../../../helpers/generatePassword.js"; import logger, { LogId } from "../../../common/logger.js"; import { inspectCluster } from "../../../common/atlas/cluster.js"; -import { ensureCurrentIpInAccessList } from "../../../common/atlas/ensureAccessList.js"; +import { ensureCurrentIpInAccessList } from "../../../common/atlas/accessListUtils.js"; const EXPIRY_MS = 1000 * 60 * 60 * 12; // 12 hours @@ -57,6 +57,19 @@ export class ConnectClusterTool extends AtlasToolBase { "atlas-connect-cluster", `error querying cluster: ${error.message}` ); + + // sometimes the error can be "error querying cluster: bad auth : Authentication failed." + // which just means it's still propagating permissions to the cluster + // in that case, we want to classify this as connecting. + if (error.message.includes("Authentication failed")) { + logger.debug( + LogId.atlasConnectFailure, + "atlas-connect-cluster", + `assuming connecting to cluster: ${this.session.connectedAtlasCluster?.clusterName}` + ); + return "connecting"; + } + return "unknown"; } } diff --git a/src/tools/atlas/create/createAccessList.ts b/src/tools/atlas/create/createAccessList.ts index 4941b1e8..b9935d83 100644 --- a/src/tools/atlas/create/createAccessList.ts +++ b/src/tools/atlas/create/createAccessList.ts @@ -2,6 +2,7 @@ import { z } from "zod"; import { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { AtlasToolBase } from "../atlasTool.js"; import { ToolArgs, OperationType } from "../../tool.js"; +import { makeCurrentIpAccessListEntry } from "../../../common/atlas/accessListUtils.js"; const DEFAULT_COMMENT = "Added by Atlas MCP"; @@ -38,12 +39,11 @@ export class CreateAccessListTool extends AtlasToolBase { })); if (currentIpAddress) { - const currentIp = await this.session.apiClient.getIpInfo(); - const input = { - groupId: projectId, - ipAddress: currentIp.currentIpv4Address, - comment: comment || DEFAULT_COMMENT, - }; + const input = await makeCurrentIpAccessListEntry( + this.session.apiClient, + projectId, + comment || DEFAULT_COMMENT + ); ipInputs.push(input); } diff --git a/src/tools/atlas/create/createDBUser.ts b/src/tools/atlas/create/createDBUser.ts index 81f1d609..9541c281 100644 --- a/src/tools/atlas/create/createDBUser.ts +++ b/src/tools/atlas/create/createDBUser.ts @@ -4,7 +4,7 @@ import { AtlasToolBase } from "../atlasTool.js"; import { ToolArgs, OperationType } from "../../tool.js"; import { CloudDatabaseUser, DatabaseUserRole } from "../../../common/atlas/openapi.js"; import { generateSecurePassword } from "../../../helpers/generatePassword.js"; -import { ensureCurrentIpInAccessList } from "../../../common/atlas/ensureAccessList.js"; +import { ensureCurrentIpInAccessList } from "../../../common/atlas/accessListUtils.js"; export class CreateDBUserTool extends AtlasToolBase { public name = "atlas-create-db-user"; diff --git a/src/tools/atlas/create/createFreeCluster.ts b/src/tools/atlas/create/createFreeCluster.ts index 74bf2295..0a8dda09 100644 --- a/src/tools/atlas/create/createFreeCluster.ts +++ b/src/tools/atlas/create/createFreeCluster.ts @@ -3,7 +3,7 @@ import { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { AtlasToolBase } from "../atlasTool.js"; import { ToolArgs, OperationType } from "../../tool.js"; import { ClusterDescription20240805 } from "../../../common/atlas/openapi.js"; -import { ensureCurrentIpInAccessList } from "../../../common/atlas/ensureAccessList.js"; +import { ensureCurrentIpInAccessList } from "../../../common/atlas/accessListUtils.js"; export class CreateFreeClusterTool extends AtlasToolBase { public name = "atlas-create-free-cluster"; diff --git a/tests/integration/tools/atlas/accessLists.test.ts b/tests/integration/tools/atlas/accessLists.test.ts index 75a5b4ca..c79896b9 100644 --- a/tests/integration/tools/atlas/accessLists.test.ts +++ b/tests/integration/tools/atlas/accessLists.test.ts @@ -1,7 +1,7 @@ import { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { describeWithAtlas, withProject } from "./atlasHelpers.js"; import { expectDefined } from "../../helpers.js"; -import { ensureCurrentIpInAccessList } from "../../../../src/common/atlas/ensureAccessList.js"; +import { ensureCurrentIpInAccessList } from "../../../../src/common/atlas/accessListUtils.js"; function generateRandomIp() { const randomIp: number[] = [192]; diff --git a/tests/unit/accessListUtils.test.ts b/tests/unit/accessListUtils.test.ts new file mode 100644 index 00000000..568564a2 --- /dev/null +++ b/tests/unit/accessListUtils.test.ts @@ -0,0 +1,30 @@ +/* global jest */ +import { ApiClient } from "../../src/common/atlas/apiClient.js"; +import { ensureCurrentIpInAccessList } from "../../src/common/atlas/accessListUtils.js"; +import { jest } from "@jest/globals"; + +describe("ensureCurrentIpInAccessList", () => { + it("should add the current IP to the access list", async () => { + const apiClient = { + getIpInfo: jest.fn().mockResolvedValue({ currentIpv4Address: "127.0.0.1" } as never), + createProjectIpAccessList: jest.fn().mockResolvedValue(undefined as never), + } as unknown as ApiClient; + await ensureCurrentIpInAccessList(apiClient, "projectId"); + expect(apiClient.createProjectIpAccessList).toHaveBeenCalledWith({ + params: { path: { groupId: "projectId" } }, + body: [{ groupId: "projectId", ipAddress: "127.0.0.1", comment: "Added by MCP pre-run access list helper" }], + }); + }); + + it("should not fail if the current IP is already in the access list", async () => { + const apiClient = { + getIpInfo: jest.fn().mockResolvedValue({ currentIpv4Address: "127.0.0.1" } as never), + createProjectIpAccessList: jest.fn().mockRejectedValue({ response: { status: 409 } } as never), + } as unknown as ApiClient; + await ensureCurrentIpInAccessList(apiClient, "projectId"); + expect(apiClient.createProjectIpAccessList).toHaveBeenCalledWith({ + params: { path: { groupId: "projectId" } }, + body: [{ groupId: "projectId", ipAddress: "127.0.0.1", comment: "Added by MCP pre-run access list helper" }], + }); + }); +}); diff --git a/tests/unit/ensureAccessList.test.ts b/tests/unit/ensureAccessList.test.ts deleted file mode 100644 index e5c6dd01..00000000 --- a/tests/unit/ensureAccessList.test.ts +++ /dev/null @@ -1,48 +0,0 @@ -import { ensureCurrentIpInAccessList } from "../../src/common/atlas/ensureAccessList.js"; -import { ApiClientError } from "../../src/common/atlas/apiClientError.js"; - -describe("ensureCurrentIpInAccessList", () => { - const projectId = "test-project-id"; - const ip = "1.2.3.4"; - let apiClient: any; - - beforeEach(() => { - apiClient = { - getIpInfo: jest.fn().mockResolvedValue({ currentIpv4Address: ip }), - createProjectIpAccessList: jest.fn().mockResolvedValue(undefined), - }; - }); - - it("adds the current IP to the access list", async () => { - await expect(ensureCurrentIpInAccessList(apiClient, projectId)).resolves.not.toThrow(); - expect(apiClient.getIpInfo).toHaveBeenCalled(); - expect(apiClient.createProjectIpAccessList).toHaveBeenCalledWith({ - params: { path: { groupId: projectId } }, - body: [ - { - groupId: projectId, - ipAddress: ip, - comment: expect.any(String), - }, - ], - }); - }); - - it("is idempotent if the IP is already present (409 error)", async () => { - apiClient.createProjectIpAccessList.mockRejectedValueOnce( - Object.assign(new ApiClientError("Conflict", { status: 409, statusText: "Conflict" } as any), { - response: { status: 409 }, - }) - ); - await expect(ensureCurrentIpInAccessList(apiClient, projectId)).resolves.not.toThrow(); - }); - - it("throws for other errors", async () => { - apiClient.createProjectIpAccessList.mockRejectedValueOnce( - Object.assign(new ApiClientError("Other", { status: 500, statusText: "Server Error" } as any), { - response: { status: 500 }, - }) - ); - await expect(ensureCurrentIpInAccessList(apiClient, projectId)).rejects.toThrow(); - }); -}); From a34de8634236db887f5c7d96b05199e8102454b5 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Tue, 15 Jul 2025 13:39:31 +0100 Subject: [PATCH 03/13] remove file --- src/common/atlas/ensureAccessList.ts | 1 - 1 file changed, 1 deletion(-) delete mode 100644 src/common/atlas/ensureAccessList.ts diff --git a/src/common/atlas/ensureAccessList.ts b/src/common/atlas/ensureAccessList.ts deleted file mode 100644 index ae17dd2c..00000000 --- a/src/common/atlas/ensureAccessList.ts +++ /dev/null @@ -1 +0,0 @@ -// This file has been migrated. ensureCurrentIpInAccessList is now in accessListUtils.ts From 46e2e5fb36c55a9cd7c1afde5da6e79dda38c820 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Tue, 15 Jul 2025 13:47:41 +0100 Subject: [PATCH 04/13] fix test --- tests/unit/accessListUtils.test.ts | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/tests/unit/accessListUtils.test.ts b/tests/unit/accessListUtils.test.ts index 568564a2..b437b1c1 100644 --- a/tests/unit/accessListUtils.test.ts +++ b/tests/unit/accessListUtils.test.ts @@ -2,8 +2,9 @@ import { ApiClient } from "../../src/common/atlas/apiClient.js"; import { ensureCurrentIpInAccessList } from "../../src/common/atlas/accessListUtils.js"; import { jest } from "@jest/globals"; +import { ApiClientError } from "../../src/common/atlas/apiClientError.js"; -describe("ensureCurrentIpInAccessList", () => { +describe("accessListUtils", () => { it("should add the current IP to the access list", async () => { const apiClient = { getIpInfo: jest.fn().mockResolvedValue({ currentIpv4Address: "127.0.0.1" } as never), @@ -12,19 +13,30 @@ describe("ensureCurrentIpInAccessList", () => { await ensureCurrentIpInAccessList(apiClient, "projectId"); expect(apiClient.createProjectIpAccessList).toHaveBeenCalledWith({ params: { path: { groupId: "projectId" } }, - body: [{ groupId: "projectId", ipAddress: "127.0.0.1", comment: "Added by MCP pre-run access list helper" }], + body: [ + { groupId: "projectId", ipAddress: "127.0.0.1", comment: "Added by MCP pre-run access list helper" }, + ], }); }); it("should not fail if the current IP is already in the access list", async () => { const apiClient = { getIpInfo: jest.fn().mockResolvedValue({ currentIpv4Address: "127.0.0.1" } as never), - createProjectIpAccessList: jest.fn().mockRejectedValue({ response: { status: 409 } } as never), + createProjectIpAccessList: jest + .fn() + .mockRejectedValue( + ApiClientError.fromError( + { status: 409, statusText: "Conflict" } as Response, + { message: "Conflict" } as never + ) as never + ), } as unknown as ApiClient; await ensureCurrentIpInAccessList(apiClient, "projectId"); expect(apiClient.createProjectIpAccessList).toHaveBeenCalledWith({ params: { path: { groupId: "projectId" } }, - body: [{ groupId: "projectId", ipAddress: "127.0.0.1", comment: "Added by MCP pre-run access list helper" }], + body: [ + { groupId: "projectId", ipAddress: "127.0.0.1", comment: "Added by MCP pre-run access list helper" }, + ], }); }); }); From 39cc667be919e5498f6afea2f6c23dd2fc5b0f27 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Tue, 15 Jul 2025 13:54:47 +0100 Subject: [PATCH 05/13] update test --- tests/unit/accessListUtils.test.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/unit/accessListUtils.test.ts b/tests/unit/accessListUtils.test.ts index b437b1c1..c12bb6f6 100644 --- a/tests/unit/accessListUtils.test.ts +++ b/tests/unit/accessListUtils.test.ts @@ -1,14 +1,13 @@ -/* global jest */ +import { describe, it, expect, vi } from "vitest"; import { ApiClient } from "../../src/common/atlas/apiClient.js"; import { ensureCurrentIpInAccessList } from "../../src/common/atlas/accessListUtils.js"; -import { jest } from "@jest/globals"; import { ApiClientError } from "../../src/common/atlas/apiClientError.js"; describe("accessListUtils", () => { it("should add the current IP to the access list", async () => { const apiClient = { - getIpInfo: jest.fn().mockResolvedValue({ currentIpv4Address: "127.0.0.1" } as never), - createProjectIpAccessList: jest.fn().mockResolvedValue(undefined as never), + getIpInfo: vi.fn().mockResolvedValue({ currentIpv4Address: "127.0.0.1" } as never), + createProjectIpAccessList: vi.fn().mockResolvedValue(undefined as never), } as unknown as ApiClient; await ensureCurrentIpInAccessList(apiClient, "projectId"); expect(apiClient.createProjectIpAccessList).toHaveBeenCalledWith({ @@ -21,8 +20,8 @@ describe("accessListUtils", () => { it("should not fail if the current IP is already in the access list", async () => { const apiClient = { - getIpInfo: jest.fn().mockResolvedValue({ currentIpv4Address: "127.0.0.1" } as never), - createProjectIpAccessList: jest + getIpInfo: vi.fn().mockResolvedValue({ currentIpv4Address: "127.0.0.1" } as never), + createProjectIpAccessList: vi .fn() .mockRejectedValue( ApiClientError.fromError( From ccb61acc3cdac8d5ef1c650f3503f22a8af30377 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Tue, 15 Jul 2025 14:27:01 +0100 Subject: [PATCH 06/13] Revert change --- src/tools/atlas/connect/connectCluster.ts | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/tools/atlas/connect/connectCluster.ts b/src/tools/atlas/connect/connectCluster.ts index 4843aa30..c2e5dba5 100644 --- a/src/tools/atlas/connect/connectCluster.ts +++ b/src/tools/atlas/connect/connectCluster.ts @@ -57,19 +57,6 @@ export class ConnectClusterTool extends AtlasToolBase { "atlas-connect-cluster", `error querying cluster: ${error.message}` ); - - // sometimes the error can be "error querying cluster: bad auth : Authentication failed." - // which just means it's still propagating permissions to the cluster - // in that case, we want to classify this as connecting. - if (error.message.includes("Authentication failed")) { - logger.debug( - LogId.atlasConnectFailure, - "atlas-connect-cluster", - `assuming connecting to cluster: ${this.session.connectedAtlasCluster?.clusterName}` - ); - return "connecting"; - } - return "unknown"; } } From 1dda709cd370f119bb2d3effda1faedec49ad2fb Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Tue, 15 Jul 2025 14:27:34 +0100 Subject: [PATCH 07/13] update --- tests/integration/tools/atlas/accessLists.test.ts | 1 + tests/unit/accessListUtils.test.ts | 2 ++ 2 files changed, 3 insertions(+) diff --git a/tests/integration/tools/atlas/accessLists.test.ts b/tests/integration/tools/atlas/accessLists.test.ts index 671ff438..8274de18 100644 --- a/tests/integration/tools/atlas/accessLists.test.ts +++ b/tests/integration/tools/atlas/accessLists.test.ts @@ -2,6 +2,7 @@ import { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { describeWithAtlas, withProject } from "./atlasHelpers.js"; import { expectDefined } from "../../helpers.js"; import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { ensureCurrentIpInAccessList } from "../../../../src/common/atlas/accessListUtils.js"; function generateRandomIp() { const randomIp: number[] = [192]; diff --git a/tests/unit/accessListUtils.test.ts b/tests/unit/accessListUtils.test.ts index c12bb6f6..8070d088 100644 --- a/tests/unit/accessListUtils.test.ts +++ b/tests/unit/accessListUtils.test.ts @@ -10,6 +10,7 @@ describe("accessListUtils", () => { createProjectIpAccessList: vi.fn().mockResolvedValue(undefined as never), } as unknown as ApiClient; await ensureCurrentIpInAccessList(apiClient, "projectId"); + // eslint-disable-next-line @typescript-eslint/unbound-method expect(apiClient.createProjectIpAccessList).toHaveBeenCalledWith({ params: { path: { groupId: "projectId" } }, body: [ @@ -31,6 +32,7 @@ describe("accessListUtils", () => { ), } as unknown as ApiClient; await ensureCurrentIpInAccessList(apiClient, "projectId"); + // eslint-disable-next-line @typescript-eslint/unbound-method expect(apiClient.createProjectIpAccessList).toHaveBeenCalledWith({ params: { path: { groupId: "projectId" } }, body: [ From cdb37b9313d1b7b241ce5f7aa478ab62562fa7ca Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Tue, 15 Jul 2025 14:35:02 +0100 Subject: [PATCH 08/13] update test --- tests/integration/tools/atlas/clusters.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/tools/atlas/clusters.test.ts b/tests/integration/tools/atlas/clusters.test.ts index 7cf0c34d..5cc3c1f6 100644 --- a/tests/integration/tools/atlas/clusters.test.ts +++ b/tests/integration/tools/atlas/clusters.test.ts @@ -91,7 +91,7 @@ describeWithAtlas("clusters", (integration) => { name: "atlas-create-free-cluster", arguments: { projectId, - name: clusterName + "-iptest", + name: clusterName, region: "US_EAST_1", }, })) as CallToolResult; From 0f5cb6b6b0227481685edadd319832e74576a1d7 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Tue, 15 Jul 2025 14:38:01 +0100 Subject: [PATCH 09/13] update logs --- src/common/atlas/accessListUtils.ts | 4 ++-- src/common/logger.ts | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/common/atlas/accessListUtils.ts b/src/common/atlas/accessListUtils.ts index e26e3edb..8a9f2245 100644 --- a/src/common/atlas/accessListUtils.ts +++ b/src/common/atlas/accessListUtils.ts @@ -31,7 +31,7 @@ export async function ensureCurrentIpInAccessList(apiClient: ApiClient, projectI }); logger.debug( LogId.atlasIpAccessListAdded, - "atlas-connect-cluster", + "accessListUtils", `IP access list created: ${JSON.stringify(entry)}` ); } catch (err) { @@ -39,6 +39,6 @@ export async function ensureCurrentIpInAccessList(apiClient: ApiClient, projectI // 409 Conflict: entry already exists, ignore return; } - throw err; + logger.debug(LogId.atlasIpAccessListAddFailure, "accessListUtils", `Error adding IP access list: ${err}`); } } diff --git a/src/common/logger.ts b/src/common/logger.ts index b1c82818..fc89f6bd 100644 --- a/src/common/logger.ts +++ b/src/common/logger.ts @@ -21,6 +21,7 @@ export const LogId = { atlasConnectSucceeded: mongoLogId(1_001_006), atlasApiRevokeFailure: mongoLogId(1_001_007), atlasIpAccessListAdded: mongoLogId(1_001_008), + atlasIpAccessListAddFailure: mongoLogId(1_001_009), telemetryDisabled: mongoLogId(1_002_001), telemetryEmitFailure: mongoLogId(1_002_002), From 4389e6ce9818a57a574f613b291f4f92b145db6c Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Tue, 15 Jul 2025 14:40:56 +0100 Subject: [PATCH 10/13] lint --- src/common/atlas/accessListUtils.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/common/atlas/accessListUtils.ts b/src/common/atlas/accessListUtils.ts index 8a9f2245..e65d7c9d 100644 --- a/src/common/atlas/accessListUtils.ts +++ b/src/common/atlas/accessListUtils.ts @@ -39,6 +39,10 @@ export async function ensureCurrentIpInAccessList(apiClient: ApiClient, projectI // 409 Conflict: entry already exists, ignore return; } - logger.debug(LogId.atlasIpAccessListAddFailure, "accessListUtils", `Error adding IP access list: ${err}`); + logger.debug( + LogId.atlasIpAccessListAddFailure, + "accessListUtils", + `Error adding IP access list: ${err instanceof Error ? err.message : String(err)}` + ); } } From 66ec84f77e9146cb0d2a1f139bdac8313a56becd Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Tue, 15 Jul 2025 15:46:11 +0100 Subject: [PATCH 11/13] address comments --- src/common/atlas/accessListUtils.ts | 13 ++++++++++--- src/tools/atlas/create/createAccessList.ts | 16 +++++++++------- tests/unit/accessListUtils.test.ts | 10 +++------- 3 files changed, 22 insertions(+), 17 deletions(-) diff --git a/src/common/atlas/accessListUtils.ts b/src/common/atlas/accessListUtils.ts index e65d7c9d..786063ef 100644 --- a/src/common/atlas/accessListUtils.ts +++ b/src/common/atlas/accessListUtils.ts @@ -2,10 +2,12 @@ import { ApiClient } from "./apiClient.js"; import logger, { LogId } from "../logger.js"; import { ApiClientError } from "./apiClientError.js"; +export const DEFAULT_ACCESS_LIST_COMMENT = "Added by MongoDB MCP Server to enable tool access"; + export async function makeCurrentIpAccessListEntry( apiClient: ApiClient, projectId: string, - comment: string = "Added by Atlas MCP" + comment: string = DEFAULT_ACCESS_LIST_COMMENT ) { const { currentIpv4Address } = await apiClient.getIpInfo(); return { @@ -23,7 +25,7 @@ export async function makeCurrentIpAccessListEntry( */ export async function ensureCurrentIpInAccessList(apiClient: ApiClient, projectId: string): Promise { // Get the current public IP - const entry = await makeCurrentIpAccessListEntry(apiClient, projectId, "Added by MCP pre-run access list helper"); + const entry = await makeCurrentIpAccessListEntry(apiClient, projectId, DEFAULT_ACCESS_LIST_COMMENT); try { await apiClient.createProjectIpAccessList({ params: { path: { groupId: projectId } }, @@ -36,7 +38,12 @@ export async function ensureCurrentIpInAccessList(apiClient: ApiClient, projectI ); } catch (err) { if (err instanceof ApiClientError && err.response?.status === 409) { - // 409 Conflict: entry already exists, ignore + // 409 Conflict: entry already exists, log info + logger.debug( + LogId.atlasIpAccessListAdded, + "accessListUtils", + `IP address ${entry.ipAddress} is already present in the access list for project ${projectId}.` + ); return; } logger.debug( diff --git a/src/tools/atlas/create/createAccessList.ts b/src/tools/atlas/create/createAccessList.ts index b9935d83..3a2c1a22 100644 --- a/src/tools/atlas/create/createAccessList.ts +++ b/src/tools/atlas/create/createAccessList.ts @@ -2,9 +2,7 @@ import { z } from "zod"; import { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { AtlasToolBase } from "../atlasTool.js"; import { ToolArgs, OperationType } from "../../tool.js"; -import { makeCurrentIpAccessListEntry } from "../../../common/atlas/accessListUtils.js"; - -const DEFAULT_COMMENT = "Added by Atlas MCP"; +import { makeCurrentIpAccessListEntry, DEFAULT_ACCESS_LIST_COMMENT } from "../../../common/atlas/accessListUtils.js"; export class CreateAccessListTool extends AtlasToolBase { public name = "atlas-create-access-list"; @@ -18,7 +16,11 @@ export class CreateAccessListTool extends AtlasToolBase { .optional(), cidrBlocks: z.array(z.string().cidr()).describe("CIDR blocks to allow access from").optional(), currentIpAddress: z.boolean().describe("Add the current IP address").default(false), - comment: z.string().describe("Comment for the access list entries").default(DEFAULT_COMMENT).optional(), + comment: z + .string() + .describe("Comment for the access list entries") + .default(DEFAULT_ACCESS_LIST_COMMENT) + .optional(), }; protected async execute({ @@ -35,14 +37,14 @@ export class CreateAccessListTool extends AtlasToolBase { const ipInputs = (ipAddresses || []).map((ipAddress) => ({ groupId: projectId, ipAddress, - comment: comment || DEFAULT_COMMENT, + comment: comment || DEFAULT_ACCESS_LIST_COMMENT, })); if (currentIpAddress) { const input = await makeCurrentIpAccessListEntry( this.session.apiClient, projectId, - comment || DEFAULT_COMMENT + comment || DEFAULT_ACCESS_LIST_COMMENT ); ipInputs.push(input); } @@ -50,7 +52,7 @@ export class CreateAccessListTool extends AtlasToolBase { const cidrInputs = (cidrBlocks || []).map((cidrBlock) => ({ groupId: projectId, cidrBlock, - comment: comment || DEFAULT_COMMENT, + comment: comment || DEFAULT_ACCESS_LIST_COMMENT, })); const inputs = [...ipInputs, ...cidrInputs]; diff --git a/tests/unit/accessListUtils.test.ts b/tests/unit/accessListUtils.test.ts index 8070d088..6dc62b65 100644 --- a/tests/unit/accessListUtils.test.ts +++ b/tests/unit/accessListUtils.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, vi } from "vitest"; import { ApiClient } from "../../src/common/atlas/apiClient.js"; -import { ensureCurrentIpInAccessList } from "../../src/common/atlas/accessListUtils.js"; +import { ensureCurrentIpInAccessList, DEFAULT_ACCESS_LIST_COMMENT } from "../../src/common/atlas/accessListUtils.js"; import { ApiClientError } from "../../src/common/atlas/apiClientError.js"; describe("accessListUtils", () => { @@ -13,9 +13,7 @@ describe("accessListUtils", () => { // eslint-disable-next-line @typescript-eslint/unbound-method expect(apiClient.createProjectIpAccessList).toHaveBeenCalledWith({ params: { path: { groupId: "projectId" } }, - body: [ - { groupId: "projectId", ipAddress: "127.0.0.1", comment: "Added by MCP pre-run access list helper" }, - ], + body: [{ groupId: "projectId", ipAddress: "127.0.0.1", comment: DEFAULT_ACCESS_LIST_COMMENT }], }); }); @@ -35,9 +33,7 @@ describe("accessListUtils", () => { // eslint-disable-next-line @typescript-eslint/unbound-method expect(apiClient.createProjectIpAccessList).toHaveBeenCalledWith({ params: { path: { groupId: "projectId" } }, - body: [ - { groupId: "projectId", ipAddress: "127.0.0.1", comment: "Added by MCP pre-run access list helper" }, - ], + body: [{ groupId: "projectId", ipAddress: "127.0.0.1", comment: DEFAULT_ACCESS_LIST_COMMENT }], }); }); }); From e2f0d2106cce1afbd2b0dafa52a1dca68e3c59de Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Tue, 15 Jul 2025 15:47:20 +0100 Subject: [PATCH 12/13] update --- src/common/atlas/accessListUtils.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/common/atlas/accessListUtils.ts b/src/common/atlas/accessListUtils.ts index 786063ef..0418cd7e 100644 --- a/src/common/atlas/accessListUtils.ts +++ b/src/common/atlas/accessListUtils.ts @@ -24,7 +24,6 @@ export async function makeCurrentIpAccessListEntry( * @param projectId The Atlas project ID */ export async function ensureCurrentIpInAccessList(apiClient: ApiClient, projectId: string): Promise { - // Get the current public IP const entry = await makeCurrentIpAccessListEntry(apiClient, projectId, DEFAULT_ACCESS_LIST_COMMENT); try { await apiClient.createProjectIpAccessList({ From 25e2b58dd0bc900a941497e3552de7b04985c422 Mon Sep 17 00:00:00 2001 From: Filipe Constantinov Menezes Date: Thu, 17 Jul 2025 17:04:52 +0100 Subject: [PATCH 13/13] fix: address comment --- src/common/atlas/accessListUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/atlas/accessListUtils.ts b/src/common/atlas/accessListUtils.ts index 0418cd7e..dddb8605 100644 --- a/src/common/atlas/accessListUtils.ts +++ b/src/common/atlas/accessListUtils.ts @@ -45,7 +45,7 @@ export async function ensureCurrentIpInAccessList(apiClient: ApiClient, projectI ); return; } - logger.debug( + logger.warning( LogId.atlasIpAccessListAddFailure, "accessListUtils", `Error adding IP access list: ${err instanceof Error ? err.message : String(err)}`