diff --git a/src/common/atlas/generatePassword.ts b/src/common/atlas/generatePassword.ts new file mode 100644 index 00000000..9e07267c --- /dev/null +++ b/src/common/atlas/generatePassword.ts @@ -0,0 +1,10 @@ +import { randomBytes } from "crypto"; +import { promisify } from "util"; + +const randomBytesAsync = promisify(randomBytes); + +export async function generateSecurePassword(): Promise { + const buf = await randomBytesAsync(16); + const pass = buf.toString("base64url"); + return pass; +} diff --git a/src/tools/atlas/create/createDBUser.ts b/src/tools/atlas/create/createDBUser.ts index a477862b..a8266a0a 100644 --- a/src/tools/atlas/create/createDBUser.ts +++ b/src/tools/atlas/create/createDBUser.ts @@ -3,6 +3,7 @@ import { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { AtlasToolBase } from "../atlasTool.js"; import { ToolArgs, OperationType } from "../../tool.js"; import { CloudDatabaseUser, DatabaseUserRole } from "../../../common/atlas/openapi.js"; +import { generateSecurePassword } from "../../../common/atlas/generatePassword.js"; export class CreateDBUserTool extends AtlasToolBase { protected name = "atlas-create-db-user"; @@ -11,7 +12,16 @@ export class CreateDBUserTool extends AtlasToolBase { protected argsShape = { projectId: z.string().describe("Atlas project ID"), username: z.string().describe("Username for the new user"), - password: z.string().describe("Password for the new user"), + // Models will generate overly simplistic passwords like SecurePassword123 or + // AtlasPassword123, which are easily guessable and exploitable. We're instructing + // the model not to try and generate anything and instead leave the field unset. + password: z + .string() + .optional() + .nullable() + .describe( + "Password for the new user. If the user hasn't supplied an explicit password, leave it unset and under no circumstances try to generate a random one. A secure password will be generated by the MCP server if necessary." + ), roles: z .array( z.object({ @@ -34,6 +44,11 @@ export class CreateDBUserTool extends AtlasToolBase { roles, clusters, }: ToolArgs): Promise { + const shouldGeneratePassword = !password; + if (shouldGeneratePassword) { + password = await generateSecurePassword(); + } + const input = { groupId: projectId, awsIAMType: "NONE", @@ -62,7 +77,12 @@ export class CreateDBUserTool extends AtlasToolBase { }); return { - content: [{ type: "text", text: `User "${username}" created sucessfully.` }], + content: [ + { + type: "text", + text: `User "${username}" created successfully${shouldGeneratePassword ? ` with password: \`${password}\`` : ""}.`, + }, + ], }; } } diff --git a/src/tools/atlas/metadata/connectCluster.ts b/src/tools/atlas/metadata/connectCluster.ts index 1bed7179..8280406a 100644 --- a/src/tools/atlas/metadata/connectCluster.ts +++ b/src/tools/atlas/metadata/connectCluster.ts @@ -2,24 +2,14 @@ import { z } from "zod"; import { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { AtlasToolBase } from "../atlasTool.js"; import { ToolArgs, OperationType } from "../../tool.js"; -import { randomBytes } from "crypto"; -import { promisify } from "util"; +import { generateSecurePassword } from "../../../common/atlas/generatePassword.js"; import logger, { LogId } from "../../../logger.js"; const EXPIRY_MS = 1000 * 60 * 60 * 12; // 12 hours -const randomBytesAsync = promisify(randomBytes); - -async function generateSecurePassword(): Promise { - const buf = await randomBytesAsync(16); - const pass = buf.toString("base64url"); - return pass; -} - function sleep(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)); } - export class ConnectClusterTool extends AtlasToolBase { protected name = "atlas-connect-cluster"; protected description = "Connect to MongoDB Atlas cluster"; diff --git a/tests/integration/helpers.ts b/tests/integration/helpers.ts index bacc89b9..c4187318 100644 --- a/tests/integration/helpers.ts +++ b/tests/integration/helpers.ts @@ -117,7 +117,7 @@ export function getResponseElements(content: unknown | { content: unknown }): { content = (content as { content: unknown }).content; } - expect(Array.isArray(content)).toBe(true); + expect(content).toBeArray(); const response = content as { type: string; text: string }[]; for (const item of response) { diff --git a/tests/integration/tools/atlas/dbUsers.test.ts b/tests/integration/tools/atlas/dbUsers.test.ts index 892bb89e..2bcb95fa 100644 --- a/tests/integration/tools/atlas/dbUsers.test.ts +++ b/tests/integration/tools/atlas/dbUsers.test.ts @@ -1,24 +1,49 @@ import { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; -import { Session } from "../../../../src/session.js"; import { describeWithAtlas, withProject, randomId } from "./atlasHelpers.js"; -import { expectDefined } from "../../helpers.js"; +import { expectDefined, getResponseElements } from "../../helpers.js"; +import { ApiClientError } from "../../../../src/common/atlas/apiClientError.js"; describeWithAtlas("db users", (integration) => { - const userName = "testuser-" + randomId; withProject(integration, ({ getProjectId }) => { - afterAll(async () => { - const projectId = getProjectId(); + let userName: string; + beforeEach(() => { + userName = "testuser-" + randomId; + }); - const session: Session = integration.mcpServer().session; - await session.apiClient.deleteDatabaseUser({ - params: { - path: { - groupId: projectId, - username: userName, - databaseName: "admin", - }, + const createUserWithMCP = async (password?: string): Promise => { + return await integration.mcpClient().callTool({ + name: "atlas-create-db-user", + arguments: { + projectId: getProjectId(), + username: userName, + password, + roles: [ + { + roleName: "readWrite", + databaseName: "admin", + }, + ], }, }); + }; + + afterEach(async () => { + try { + await integration.mcpServer().session.apiClient.deleteDatabaseUser({ + params: { + path: { + groupId: getProjectId(), + username: userName, + databaseName: "admin", + }, + }, + }); + } catch (error) { + // Ignore 404 errors when deleting the user + if (!(error instanceof ApiClientError) || error.response?.status !== 404) { + throw error; + } + } }); describe("atlas-create-db-user", () => { @@ -34,26 +59,24 @@ describeWithAtlas("db users", (integration) => { expect(createDbUser.inputSchema.properties).toHaveProperty("roles"); expect(createDbUser.inputSchema.properties).toHaveProperty("clusters"); }); - it("should create a database user", async () => { - const projectId = getProjectId(); - const response = (await integration.mcpClient().callTool({ - name: "atlas-create-db-user", - arguments: { - projectId, - username: userName, - password: "testpassword", - roles: [ - { - roleName: "readWrite", - databaseName: "admin", - }, - ], - }, - })) as CallToolResult; - expect(response.content).toBeArray(); - expect(response.content).toHaveLength(1); - expect(response.content[0].text).toContain("created sucessfully"); + it("should create a database user with supplied password", async () => { + const response = await createUserWithMCP("testpassword"); + + const elements = getResponseElements(response); + expect(elements).toHaveLength(1); + expect(elements[0].text).toContain("created successfully"); + expect(elements[0].text).toContain(userName); + expect(elements[0].text).not.toContain("testpassword"); + }); + + it("should create a database user with generated password", async () => { + const response = await createUserWithMCP(); + const elements = getResponseElements(response); + expect(elements).toHaveLength(1); + expect(elements[0].text).toContain("created successfully"); + expect(elements[0].text).toContain(userName); + expect(elements[0].text).toContain("with password: `"); }); }); describe("atlas-list-db-users", () => { @@ -68,6 +91,8 @@ describeWithAtlas("db users", (integration) => { it("returns database users by project", async () => { const projectId = getProjectId(); + await createUserWithMCP(); + const response = (await integration .mcpClient() .callTool({ name: "atlas-list-db-users", arguments: { projectId } })) as CallToolResult;