Skip to content

Commit 2941dde

Browse files
blvafmenezes
andauthored
chore: Improve access list and connect experienece [MCP-5] (#372)
Co-authored-by: Filipe Constantinov Menezes <[email protected]>
1 parent 84e63f6 commit 2941dde

File tree

10 files changed

+155
-12
lines changed

10 files changed

+155
-12
lines changed

src/common/atlas/accessListUtils.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import { ApiClient } from "./apiClient.js";
2+
import logger, { LogId } from "../logger.js";
3+
import { ApiClientError } from "./apiClientError.js";
4+
5+
export const DEFAULT_ACCESS_LIST_COMMENT = "Added by MongoDB MCP Server to enable tool access";
6+
7+
export async function makeCurrentIpAccessListEntry(
8+
apiClient: ApiClient,
9+
projectId: string,
10+
comment: string = DEFAULT_ACCESS_LIST_COMMENT
11+
) {
12+
const { currentIpv4Address } = await apiClient.getIpInfo();
13+
return {
14+
groupId: projectId,
15+
ipAddress: currentIpv4Address,
16+
comment,
17+
};
18+
}
19+
20+
/**
21+
* Ensures the current public IP is in the access list for the given Atlas project.
22+
* If the IP is already present, this is a no-op.
23+
* @param apiClient The Atlas API client instance
24+
* @param projectId The Atlas project ID
25+
*/
26+
export async function ensureCurrentIpInAccessList(apiClient: ApiClient, projectId: string): Promise<void> {
27+
const entry = await makeCurrentIpAccessListEntry(apiClient, projectId, DEFAULT_ACCESS_LIST_COMMENT);
28+
try {
29+
await apiClient.createProjectIpAccessList({
30+
params: { path: { groupId: projectId } },
31+
body: [entry],
32+
});
33+
logger.debug(
34+
LogId.atlasIpAccessListAdded,
35+
"accessListUtils",
36+
`IP access list created: ${JSON.stringify(entry)}`
37+
);
38+
} catch (err) {
39+
if (err instanceof ApiClientError && err.response?.status === 409) {
40+
// 409 Conflict: entry already exists, log info
41+
logger.debug(
42+
LogId.atlasIpAccessListAdded,
43+
"accessListUtils",
44+
`IP address ${entry.ipAddress} is already present in the access list for project ${projectId}.`
45+
);
46+
return;
47+
}
48+
logger.warning(
49+
LogId.atlasIpAccessListAddFailure,
50+
"accessListUtils",
51+
`Error adding IP access list: ${err instanceof Error ? err.message : String(err)}`
52+
);
53+
}
54+
}

src/common/logger.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ export const LogId = {
2020
atlasConnectAttempt: mongoLogId(1_001_005),
2121
atlasConnectSucceeded: mongoLogId(1_001_006),
2222
atlasApiRevokeFailure: mongoLogId(1_001_007),
23+
atlasIpAccessListAdded: mongoLogId(1_001_008),
24+
atlasIpAccessListAddFailure: mongoLogId(1_001_009),
2325

2426
telemetryDisabled: mongoLogId(1_002_001),
2527
telemetryEmitFailure: mongoLogId(1_002_002),

src/tools/atlas/connect/connectCluster.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { ToolArgs, OperationType } from "../../tool.js";
55
import { generateSecurePassword } from "../../../helpers/generatePassword.js";
66
import logger, { LogId } from "../../../common/logger.js";
77
import { inspectCluster } from "../../../common/atlas/cluster.js";
8+
import { ensureCurrentIpInAccessList } from "../../../common/atlas/accessListUtils.js";
89

910
const EXPIRY_MS = 1000 * 60 * 60 * 12; // 12 hours
1011

@@ -198,6 +199,7 @@ export class ConnectClusterTool extends AtlasToolBase {
198199
}
199200

200201
protected async execute({ projectId, clusterName }: ToolArgs<typeof this.argsShape>): Promise<CallToolResult> {
202+
await ensureCurrentIpInAccessList(this.session.apiClient, projectId);
201203
for (let i = 0; i < 60; i++) {
202204
const state = await this.queryConnection(projectId, clusterName);
203205
switch (state) {

src/tools/atlas/create/createAccessList.ts

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@ import { z } from "zod";
22
import { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
33
import { AtlasToolBase } from "../atlasTool.js";
44
import { ToolArgs, OperationType } from "../../tool.js";
5-
6-
const DEFAULT_COMMENT = "Added by Atlas MCP";
5+
import { makeCurrentIpAccessListEntry, DEFAULT_ACCESS_LIST_COMMENT } from "../../../common/atlas/accessListUtils.js";
76

87
export class CreateAccessListTool extends AtlasToolBase {
98
public name = "atlas-create-access-list";
@@ -17,7 +16,11 @@ export class CreateAccessListTool extends AtlasToolBase {
1716
.optional(),
1817
cidrBlocks: z.array(z.string().cidr()).describe("CIDR blocks to allow access from").optional(),
1918
currentIpAddress: z.boolean().describe("Add the current IP address").default(false),
20-
comment: z.string().describe("Comment for the access list entries").default(DEFAULT_COMMENT).optional(),
19+
comment: z
20+
.string()
21+
.describe("Comment for the access list entries")
22+
.default(DEFAULT_ACCESS_LIST_COMMENT)
23+
.optional(),
2124
};
2225

2326
protected async execute({
@@ -34,23 +37,22 @@ export class CreateAccessListTool extends AtlasToolBase {
3437
const ipInputs = (ipAddresses || []).map((ipAddress) => ({
3538
groupId: projectId,
3639
ipAddress,
37-
comment: comment || DEFAULT_COMMENT,
40+
comment: comment || DEFAULT_ACCESS_LIST_COMMENT,
3841
}));
3942

4043
if (currentIpAddress) {
41-
const currentIp = await this.session.apiClient.getIpInfo();
42-
const input = {
43-
groupId: projectId,
44-
ipAddress: currentIp.currentIpv4Address,
45-
comment: comment || DEFAULT_COMMENT,
46-
};
44+
const input = await makeCurrentIpAccessListEntry(
45+
this.session.apiClient,
46+
projectId,
47+
comment || DEFAULT_ACCESS_LIST_COMMENT
48+
);
4749
ipInputs.push(input);
4850
}
4951

5052
const cidrInputs = (cidrBlocks || []).map((cidrBlock) => ({
5153
groupId: projectId,
5254
cidrBlock,
53-
comment: comment || DEFAULT_COMMENT,
55+
comment: comment || DEFAULT_ACCESS_LIST_COMMENT,
5456
}));
5557

5658
const inputs = [...ipInputs, ...cidrInputs];

src/tools/atlas/create/createDBUser.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { AtlasToolBase } from "../atlasTool.js";
44
import { ToolArgs, OperationType } from "../../tool.js";
55
import { CloudDatabaseUser, DatabaseUserRole } from "../../../common/atlas/openapi.js";
66
import { generateSecurePassword } from "../../../helpers/generatePassword.js";
7+
import { ensureCurrentIpInAccessList } from "../../../common/atlas/accessListUtils.js";
78

89
export class CreateDBUserTool extends AtlasToolBase {
910
public name = "atlas-create-db-user";
@@ -44,6 +45,7 @@ export class CreateDBUserTool extends AtlasToolBase {
4445
roles,
4546
clusters,
4647
}: ToolArgs<typeof this.argsShape>): Promise<CallToolResult> {
48+
await ensureCurrentIpInAccessList(this.session.apiClient, projectId);
4749
const shouldGeneratePassword = !password;
4850
if (shouldGeneratePassword) {
4951
password = await generateSecurePassword();

src/tools/atlas/create/createFreeCluster.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
33
import { AtlasToolBase } from "../atlasTool.js";
44
import { ToolArgs, OperationType } from "../../tool.js";
55
import { ClusterDescription20240805 } from "../../../common/atlas/openapi.js";
6+
import { ensureCurrentIpInAccessList } from "../../../common/atlas/accessListUtils.js";
67

78
export class CreateFreeClusterTool extends AtlasToolBase {
89
public name = "atlas-create-free-cluster";
@@ -37,6 +38,7 @@ export class CreateFreeClusterTool extends AtlasToolBase {
3738
terminationProtectionEnabled: false,
3839
} as unknown as ClusterDescription20240805;
3940

41+
await ensureCurrentIpInAccessList(this.session.apiClient, projectId);
4042
await this.session.apiClient.createCluster({
4143
params: {
4244
path: {

tests/integration/tools/atlas/accessLists.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
22
import { describeWithAtlas, withProject } from "./atlasHelpers.js";
33
import { expectDefined } from "../../helpers.js";
44
import { afterAll, beforeAll, describe, expect, it } from "vitest";
5+
import { ensureCurrentIpInAccessList } from "../../../../src/common/atlas/accessListUtils.js";
56

67
function generateRandomIp() {
78
const randomIp: number[] = [192];
@@ -95,5 +96,23 @@ describeWithAtlas("ip access lists", (integration) => {
9596
}
9697
});
9798
});
99+
100+
describe("ensureCurrentIpInAccessList helper", () => {
101+
it("should add the current IP to the access list and be idempotent", async () => {
102+
const apiClient = integration.mcpServer().session.apiClient;
103+
const projectId = getProjectId();
104+
const ipInfo = await apiClient.getIpInfo();
105+
// First call should add the IP
106+
await expect(ensureCurrentIpInAccessList(apiClient, projectId)).resolves.not.toThrow();
107+
// Second call should be a no-op (idempotent)
108+
await expect(ensureCurrentIpInAccessList(apiClient, projectId)).resolves.not.toThrow();
109+
// Check that the IP is present in the access list
110+
const accessList = await apiClient.listProjectIpAccessLists({
111+
params: { path: { groupId: projectId } },
112+
});
113+
const found = accessList.results?.some((entry) => entry.ipAddress === ipInfo.currentIpv4Address);
114+
expect(found).toBe(true);
115+
});
116+
});
98117
});
99118
});

tests/integration/tools/atlas/clusters.test.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,10 @@ describeWithAtlas("clusters", (integration) => {
8282
expect(createFreeCluster.inputSchema.properties).toHaveProperty("region");
8383
});
8484

85-
it("should create a free cluster", async () => {
85+
it("should create a free cluster and add current IP to access list", async () => {
8686
const projectId = getProjectId();
87+
const session = integration.mcpServer().session;
88+
const ipInfo = await session.apiClient.getIpInfo();
8789

8890
const response = (await integration.mcpClient().callTool({
8991
name: "atlas-create-free-cluster",
@@ -96,6 +98,13 @@ describeWithAtlas("clusters", (integration) => {
9698
expect(response.content).toBeInstanceOf(Array);
9799
expect(response.content).toHaveLength(2);
98100
expect(response.content[0]?.text).toContain("has been created");
101+
102+
// Check that the current IP is present in the access list
103+
const accessList = await session.apiClient.listProjectIpAccessLists({
104+
params: { path: { groupId: projectId } },
105+
});
106+
const found = accessList.results?.some((entry) => entry.ipAddress === ipInfo.currentIpv4Address);
107+
expect(found).toBe(true);
99108
});
100109
});
101110

tests/integration/tools/atlas/dbUsers.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,18 @@ describeWithAtlas("db users", (integration) => {
7979
expect(elements[0]?.text).toContain(userName);
8080
expect(elements[0]?.text).toContain("with password: `");
8181
});
82+
83+
it("should add current IP to access list when creating a database user", async () => {
84+
const projectId = getProjectId();
85+
const session = integration.mcpServer().session;
86+
const ipInfo = await session.apiClient.getIpInfo();
87+
await createUserWithMCP();
88+
const accessList = await session.apiClient.listProjectIpAccessLists({
89+
params: { path: { groupId: projectId } },
90+
});
91+
const found = accessList.results?.some((entry) => entry.ipAddress === ipInfo.currentIpv4Address);
92+
expect(found).toBe(true);
93+
});
8294
});
8395
describe("atlas-list-db-users", () => {
8496
it("should have correct metadata", async () => {

tests/unit/accessListUtils.test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import { describe, it, expect, vi } from "vitest";
2+
import { ApiClient } from "../../src/common/atlas/apiClient.js";
3+
import { ensureCurrentIpInAccessList, DEFAULT_ACCESS_LIST_COMMENT } from "../../src/common/atlas/accessListUtils.js";
4+
import { ApiClientError } from "../../src/common/atlas/apiClientError.js";
5+
6+
describe("accessListUtils", () => {
7+
it("should add the current IP to the access list", async () => {
8+
const apiClient = {
9+
getIpInfo: vi.fn().mockResolvedValue({ currentIpv4Address: "127.0.0.1" } as never),
10+
createProjectIpAccessList: vi.fn().mockResolvedValue(undefined as never),
11+
} as unknown as ApiClient;
12+
await ensureCurrentIpInAccessList(apiClient, "projectId");
13+
// eslint-disable-next-line @typescript-eslint/unbound-method
14+
expect(apiClient.createProjectIpAccessList).toHaveBeenCalledWith({
15+
params: { path: { groupId: "projectId" } },
16+
body: [{ groupId: "projectId", ipAddress: "127.0.0.1", comment: DEFAULT_ACCESS_LIST_COMMENT }],
17+
});
18+
});
19+
20+
it("should not fail if the current IP is already in the access list", async () => {
21+
const apiClient = {
22+
getIpInfo: vi.fn().mockResolvedValue({ currentIpv4Address: "127.0.0.1" } as never),
23+
createProjectIpAccessList: vi
24+
.fn()
25+
.mockRejectedValue(
26+
ApiClientError.fromError(
27+
{ status: 409, statusText: "Conflict" } as Response,
28+
{ message: "Conflict" } as never
29+
) as never
30+
),
31+
} as unknown as ApiClient;
32+
await ensureCurrentIpInAccessList(apiClient, "projectId");
33+
// eslint-disable-next-line @typescript-eslint/unbound-method
34+
expect(apiClient.createProjectIpAccessList).toHaveBeenCalledWith({
35+
params: { path: { groupId: "projectId" } },
36+
body: [{ groupId: "projectId", ipAddress: "127.0.0.1", comment: DEFAULT_ACCESS_LIST_COMMENT }],
37+
});
38+
});
39+
});

0 commit comments

Comments
 (0)