From 632fc540156a0e9e419715e028b8c727f9141ceb Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Fri, 22 Aug 2025 15:15:27 +0100 Subject: [PATCH 01/11] fix: atlas connectCluster defaults to readOnly DB roles --- src/tools/atlas/connect/connectCluster.ts | 50 +++++++++++++++-------- 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/src/tools/atlas/connect/connectCluster.ts b/src/tools/atlas/connect/connectCluster.ts index 1653c3f66..651dd033a 100644 --- a/src/tools/atlas/connect/connectCluster.ts +++ b/src/tools/atlas/connect/connectCluster.ts @@ -7,6 +7,7 @@ import { LogId } from "../../../common/logger.js"; import { inspectCluster } from "../../../common/atlas/cluster.js"; import { ensureCurrentIpInAccessList } from "../../../common/atlas/accessListUtils.js"; import { AtlasClusterConnectionInfo } from "../../../common/connectionManager.js"; +import { DatabaseUserRole } from "../../../common/atlas/openapi.js"; const EXPIRY_MS = 1000 * 60 * 60 * 12; // 12 hours @@ -72,16 +73,7 @@ export class ConnectClusterTool extends AtlasToolBase { const password = await generateSecurePassword(); const expiryDate = new Date(Date.now() + EXPIRY_MS); - - const readOnly = - this.config.readOnly || - (this.config.disabledTools?.includes("create") && - this.config.disabledTools?.includes("update") && - this.config.disabledTools?.includes("delete") && - !this.config.disabledTools?.includes("read") && - !this.config.disabledTools?.includes("metadata")); - - const roleName = readOnly ? "readAnyDatabase" : "readWriteAnyDatabase"; + const role = this.getRoleFromConfig(); await this.session.apiClient.createDatabaseUser({ params: { @@ -92,12 +84,7 @@ export class ConnectClusterTool extends AtlasToolBase { body: { databaseName: "admin", groupId: projectId, - roles: [ - { - roleName, - databaseName: "admin", - }, - ], + roles: [role], scopes: [{ type: "CLUSTER", name: clusterName }], username, password, @@ -258,4 +245,35 @@ export class ConnectClusterTool extends AtlasToolBase { ], }; } + + /** + * @description Get the role name for the database user based on the Atlas Admin API https://www.mongodb.com/docs/atlas/mongodb-users-roles-and-privileges/ + * @returns The role name for the database user + */ + private getRoleFromConfig(): DatabaseUserRole { + if (this.config.readOnly) { + return { + roleName: "readAnyDatabase", + databaseName: "admin", + }; + } + + // If all write tools are enabled, use readWriteAnyDatabase + if ( + !this.config.disabledTools?.includes("create") && + !this.config.disabledTools?.includes("update") && + !this.config.disabledTools?.includes("delete") && + !this.config.disabledTools?.includes("metadata") + ) { + return { + roleName: "readWriteAnyDatabase", + databaseName: "admin", + }; + } + + return { + roleName: "readAnyDatabase", + databaseName: "admin", + }; + } } From 439daab2bec0deb898135424add0f2e904c26821 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Fri, 22 Aug 2025 15:29:33 +0100 Subject: [PATCH 02/11] decouple and add unit test --- src/tools/atlas/connect/connectCluster.ts | 35 +------------- tests/unit/common/roles.test.ts | 56 +++++++++++++++++++++++ 2 files changed, 58 insertions(+), 33 deletions(-) create mode 100644 tests/unit/common/roles.test.ts diff --git a/src/tools/atlas/connect/connectCluster.ts b/src/tools/atlas/connect/connectCluster.ts index 651dd033a..ea910d1a2 100644 --- a/src/tools/atlas/connect/connectCluster.ts +++ b/src/tools/atlas/connect/connectCluster.ts @@ -7,7 +7,7 @@ import { LogId } from "../../../common/logger.js"; import { inspectCluster } from "../../../common/atlas/cluster.js"; import { ensureCurrentIpInAccessList } from "../../../common/atlas/accessListUtils.js"; import { AtlasClusterConnectionInfo } from "../../../common/connectionManager.js"; -import { DatabaseUserRole } from "../../../common/atlas/openapi.js"; +import { getDefaultRoleFromConfig } from "../../../common/atlas/roles.js"; const EXPIRY_MS = 1000 * 60 * 60 * 12; // 12 hours @@ -73,7 +73,7 @@ export class ConnectClusterTool extends AtlasToolBase { const password = await generateSecurePassword(); const expiryDate = new Date(Date.now() + EXPIRY_MS); - const role = this.getRoleFromConfig(); + const role = getDefaultRoleFromConfig(this.config); await this.session.apiClient.createDatabaseUser({ params: { @@ -245,35 +245,4 @@ export class ConnectClusterTool extends AtlasToolBase { ], }; } - - /** - * @description Get the role name for the database user based on the Atlas Admin API https://www.mongodb.com/docs/atlas/mongodb-users-roles-and-privileges/ - * @returns The role name for the database user - */ - private getRoleFromConfig(): DatabaseUserRole { - if (this.config.readOnly) { - return { - roleName: "readAnyDatabase", - databaseName: "admin", - }; - } - - // If all write tools are enabled, use readWriteAnyDatabase - if ( - !this.config.disabledTools?.includes("create") && - !this.config.disabledTools?.includes("update") && - !this.config.disabledTools?.includes("delete") && - !this.config.disabledTools?.includes("metadata") - ) { - return { - roleName: "readWriteAnyDatabase", - databaseName: "admin", - }; - } - - return { - roleName: "readAnyDatabase", - databaseName: "admin", - }; - } } diff --git a/tests/unit/common/roles.test.ts b/tests/unit/common/roles.test.ts new file mode 100644 index 000000000..b3dff5cc7 --- /dev/null +++ b/tests/unit/common/roles.test.ts @@ -0,0 +1,56 @@ +import { describe, it, expect } from "vitest"; +import { getDefaultRoleFromConfig } from "../../../src/common/atlas/roles.js"; +import { defaultUserConfig, UserConfig } from "../../../src/common/config.js"; + +describe("getDefaultRoleFromConfig", () => { + const defaultConfig: UserConfig = { + ...defaultUserConfig, + }; + + const readOnlyConfig: UserConfig = { + ...defaultConfig, + readOnly: true, + }; + + const readWriteConfig: UserConfig = { + ...defaultConfig, + readOnly: false, + disabledTools: [], + }; + + it("should return the correct role for a read-only config", () => { + const role = getDefaultRoleFromConfig(readOnlyConfig); + expect(role).toEqual({ + roleName: "readAnyDatabase", + databaseName: "admin", + }); + }); + + it("should return the correct role for a read-write config", () => { + const role = getDefaultRoleFromConfig(readWriteConfig); + expect(role).toEqual({ + roleName: "readWriteAnyDatabase", + databaseName: "admin", + }); + }); + + it("should return the correct role for a read-write config with all tools enabled", () => { + const role = getDefaultRoleFromConfig(readWriteConfig); + expect(role).toEqual({ + roleName: "readWriteAnyDatabase", + databaseName: "admin", + }); + }); + + // loop with each disabled tool + for (const tool of ["create", "update", "delete", "metadata"]) { + it(`should return the correct role for a read-write config with ${tool} disabled`, () => { + const config = { ...readWriteConfig, disabledTools: [tool] }; + const role = getDefaultRoleFromConfig(config); + expect(role).toEqual({ + roleName: "readAnyDatabase", + databaseName: "admin", + }); + }); + } +}); From c84caf1a7645e1b707ae54058ab2f82451deeb55 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Fri, 22 Aug 2025 15:33:42 +0100 Subject: [PATCH 03/11] add missing file --- src/common/atlas/roles.ts | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 src/common/atlas/roles.ts diff --git a/src/common/atlas/roles.ts b/src/common/atlas/roles.ts new file mode 100644 index 000000000..aa9f2c50b --- /dev/null +++ b/src/common/atlas/roles.ts @@ -0,0 +1,33 @@ +import { UserConfig } from "../config.js"; +import { DatabaseUserRole } from "./openapi.js"; + +/** + * Get the default role name for the database user based on the Atlas Admin API + * https://www.mongodb.com/docs/atlas/mongodb-users-roles-and-privileges/ + */ +export function getDefaultRoleFromConfig(config: UserConfig): DatabaseUserRole { + if (config.readOnly) { + return { + roleName: "readAnyDatabase", + databaseName: "admin", + }; + } + + // If all write tools are enabled, use readWriteAnyDatabase + if ( + !config.disabledTools?.includes("create") && + !config.disabledTools?.includes("update") && + !config.disabledTools?.includes("delete") && + !config.disabledTools?.includes("metadata") + ) { + return { + roleName: "readWriteAnyDatabase", + databaseName: "admin", + }; + } + + return { + roleName: "readAnyDatabase", + databaseName: "admin", + }; +} From 3c92f7c9729268b6c6c08f009cc3102c18951b83 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Fri, 22 Aug 2025 15:39:43 +0100 Subject: [PATCH 04/11] minor fix: rename test --- tests/unit/common/roles.test.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/unit/common/roles.test.ts b/tests/unit/common/roles.test.ts index b3dff5cc7..6d5f64193 100644 --- a/tests/unit/common/roles.test.ts +++ b/tests/unit/common/roles.test.ts @@ -42,10 +42,9 @@ describe("getDefaultRoleFromConfig", () => { }); }); - // loop with each disabled tool for (const tool of ["create", "update", "delete", "metadata"]) { - it(`should return the correct role for a read-write config with ${tool} disabled`, () => { - const config = { ...readWriteConfig, disabledTools: [tool] }; + it(`should return the correct role for a config with ${tool} disabled`, () => { + const config = { ...defaultConfig, disabledTools: [tool] }; const role = getDefaultRoleFromConfig(config); expect(role).toEqual({ roleName: "readAnyDatabase", From 34051b7a1d8eaeb2ac306df93f6d4b7e64dcea42 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Fri, 22 Aug 2025 16:42:56 +0100 Subject: [PATCH 05/11] address comment: add description to the DB User --- src/tools/atlas/connect/connectCluster.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/tools/atlas/connect/connectCluster.ts b/src/tools/atlas/connect/connectCluster.ts index ea910d1a2..8ee6abfa3 100644 --- a/src/tools/atlas/connect/connectCluster.ts +++ b/src/tools/atlas/connect/connectCluster.ts @@ -93,6 +93,8 @@ export class ConnectClusterTool extends AtlasToolBase { oidcAuthType: "NONE", x509Type: "NONE", deleteAfterDate: expiryDate.toISOString(), + description: + "This role is created by MongoDB MCP to connect to the cluster via the atlas-connect-cluster tool", }, }); From 271a678b2d6df440f6da4fb4ee5918cc59b84a5b Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Fri, 22 Aug 2025 16:44:05 +0100 Subject: [PATCH 06/11] address comment: keep readWrite if any write tool is enabled but not metadata --- src/common/atlas/roles.ts | 9 +++-- tests/unit/common/roles.test.ts | 63 ++++++++++++++++++++++++++++----- 2 files changed, 58 insertions(+), 14 deletions(-) diff --git a/src/common/atlas/roles.ts b/src/common/atlas/roles.ts index aa9f2c50b..829139d39 100644 --- a/src/common/atlas/roles.ts +++ b/src/common/atlas/roles.ts @@ -13,12 +13,11 @@ export function getDefaultRoleFromConfig(config: UserConfig): DatabaseUserRole { }; } - // If all write tools are enabled, use readWriteAnyDatabase + // If any of the write tools are enabled, use readWriteAnyDatabase if ( - !config.disabledTools?.includes("create") && - !config.disabledTools?.includes("update") && - !config.disabledTools?.includes("delete") && - !config.disabledTools?.includes("metadata") + !config.disabledTools?.includes("create") || + !config.disabledTools?.includes("update") || + !config.disabledTools?.includes("delete") ) { return { roleName: "readWriteAnyDatabase", diff --git a/tests/unit/common/roles.test.ts b/tests/unit/common/roles.test.ts index 6d5f64193..78326313f 100644 --- a/tests/unit/common/roles.test.ts +++ b/tests/unit/common/roles.test.ts @@ -18,6 +18,30 @@ describe("getDefaultRoleFromConfig", () => { disabledTools: [], }; + const readWriteConfigWithDeleteDisabled: UserConfig = { + ...defaultConfig, + readOnly: false, + disabledTools: ["delete"], + }; + + const readWriteConfigWithCreateDisabled: UserConfig = { + ...defaultConfig, + readOnly: false, + disabledTools: ["create"], + }; + + const readWriteConfigWithUpdateDisabled: UserConfig = { + ...defaultConfig, + readOnly: false, + disabledTools: ["update"], + }; + + const readWriteConfigWithAllToolsDisabled: UserConfig = { + ...defaultConfig, + readOnly: false, + disabledTools: ["create", "update", "delete"], + }; + it("should return the correct role for a read-only config", () => { const role = getDefaultRoleFromConfig(readOnlyConfig); expect(role).toEqual({ @@ -42,14 +66,35 @@ describe("getDefaultRoleFromConfig", () => { }); }); - for (const tool of ["create", "update", "delete", "metadata"]) { - it(`should return the correct role for a config with ${tool} disabled`, () => { - const config = { ...defaultConfig, disabledTools: [tool] }; - const role = getDefaultRoleFromConfig(config); - expect(role).toEqual({ - roleName: "readAnyDatabase", - databaseName: "admin", - }); + it("should return the correct role for a read-write config with delete disabled", () => { + const role = getDefaultRoleFromConfig(readWriteConfigWithDeleteDisabled); + expect(role).toEqual({ + roleName: "readWriteAnyDatabase", + databaseName: "admin", + }); + }); + + it("should return the correct role for a read-write config with create disabled", () => { + const role = getDefaultRoleFromConfig(readWriteConfigWithCreateDisabled); + expect(role).toEqual({ + roleName: "readWriteAnyDatabase", + databaseName: "admin", }); - } + }); + + it("should return the correct role for a read-write config with update disabled", () => { + const role = getDefaultRoleFromConfig(readWriteConfigWithUpdateDisabled); + expect(role).toEqual({ + roleName: "readAnyDatabase", + databaseName: "admin", + }); + }); + + it("should return the correct role for a read-write config with all tools disabled", () => { + const role = getDefaultRoleFromConfig(readWriteConfigWithAllToolsDisabled); + expect(role).toEqual({ + roleName: "readAnyDatabase", + databaseName: "admin", + }); + }); }); From 3ab7beb5d4dd2adf2d98fa064e48b614961538a7 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Fri, 22 Aug 2025 16:46:36 +0100 Subject: [PATCH 07/11] apply new lint --- src/common/atlas/roles.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/common/atlas/roles.ts b/src/common/atlas/roles.ts index 829139d39..30901ac7c 100644 --- a/src/common/atlas/roles.ts +++ b/src/common/atlas/roles.ts @@ -1,5 +1,5 @@ -import { UserConfig } from "../config.js"; -import { DatabaseUserRole } from "./openapi.js"; +import type { UserConfig } from "../config.js"; +import type { DatabaseUserRole } from "./openapi.js"; /** * Get the default role name for the database user based on the Atlas Admin API From fa45e6fcc11b9d289ee80d92bdbe5eb287d1de90 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Fri, 22 Aug 2025 16:50:21 +0100 Subject: [PATCH 08/11] fix new lint --- tests/unit/common/roles.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/common/roles.test.ts b/tests/unit/common/roles.test.ts index 78326313f..631060fb0 100644 --- a/tests/unit/common/roles.test.ts +++ b/tests/unit/common/roles.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect } from "vitest"; import { getDefaultRoleFromConfig } from "../../../src/common/atlas/roles.js"; -import { defaultUserConfig, UserConfig } from "../../../src/common/config.js"; +import { defaultUserConfig, type UserConfig } from "../../../src/common/config.js"; describe("getDefaultRoleFromConfig", () => { const defaultConfig: UserConfig = { From ac80a9c9ef2d3df0a1e420336ff1c5612b638e25 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Fri, 22 Aug 2025 16:52:30 +0100 Subject: [PATCH 09/11] correct unit test --- tests/unit/common/roles.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/common/roles.test.ts b/tests/unit/common/roles.test.ts index 631060fb0..058e605ab 100644 --- a/tests/unit/common/roles.test.ts +++ b/tests/unit/common/roles.test.ts @@ -85,7 +85,7 @@ describe("getDefaultRoleFromConfig", () => { it("should return the correct role for a read-write config with update disabled", () => { const role = getDefaultRoleFromConfig(readWriteConfigWithUpdateDisabled); expect(role).toEqual({ - roleName: "readAnyDatabase", + roleName: "readWriteAnyDatabase", databaseName: "admin", }); }); From 97c25561d9f6656f7736771609ba813b0eeab7c8 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Fri, 22 Aug 2025 18:01:51 +0100 Subject: [PATCH 10/11] address comments --- src/common/atlas/roles.ts | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/common/atlas/roles.ts b/src/common/atlas/roles.ts index 30901ac7c..5d776c754 100644 --- a/src/common/atlas/roles.ts +++ b/src/common/atlas/roles.ts @@ -1,32 +1,33 @@ import type { UserConfig } from "../config.js"; import type { DatabaseUserRole } from "./openapi.js"; +const readWriteRole: DatabaseUserRole = { + roleName: "readWriteAnyDatabase", + databaseName: "admin", +}; + +const readOnlyRole: DatabaseUserRole = { + roleName: "readAnyDatabase", + databaseName: "admin", +}; + /** * Get the default role name for the database user based on the Atlas Admin API * https://www.mongodb.com/docs/atlas/mongodb-users-roles-and-privileges/ */ export function getDefaultRoleFromConfig(config: UserConfig): DatabaseUserRole { if (config.readOnly) { - return { - roleName: "readAnyDatabase", - databaseName: "admin", - }; + return readOnlyRole; } // If any of the write tools are enabled, use readWriteAnyDatabase if ( - !config.disabledTools?.includes("create") || - !config.disabledTools?.includes("update") || - !config.disabledTools?.includes("delete") + !config.disabledTools.includes("create") || + !config.disabledTools.includes("update") || + !config.disabledTools.includes("delete") ) { - return { - roleName: "readWriteAnyDatabase", - databaseName: "admin", - }; + return readWriteRole; } - return { - roleName: "readAnyDatabase", - databaseName: "admin", - }; + return readOnlyRole; } From 903825dd412b347044d98d874d7514f96858a4b0 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Fri, 22 Aug 2025 18:03:17 +0100 Subject: [PATCH 11/11] address comment: description --- src/tools/atlas/connect/connectCluster.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tools/atlas/connect/connectCluster.ts b/src/tools/atlas/connect/connectCluster.ts index 9e4ca9e86..368303d47 100644 --- a/src/tools/atlas/connect/connectCluster.ts +++ b/src/tools/atlas/connect/connectCluster.ts @@ -93,8 +93,7 @@ export class ConnectClusterTool extends AtlasToolBase { oidcAuthType: "NONE", x509Type: "NONE", deleteAfterDate: expiryDate.toISOString(), - description: - "This role is created by MongoDB MCP to connect to the cluster via the atlas-connect-cluster tool", + description: "This temporary user is created by the MongoDB MCP Server to connect to the cluster.", }, });