Skip to content

feat: Allow configuration of temporary user timeout when connecting to atlas cluster #544

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Sep 12, 2025
Merged

feat: Allow configuration of temporary user timeout when connecting to atlas cluster #544

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
83c9639
feat: Allow configuration of temporary user when connecting to atlas …
jeroenvervaeke Sep 11, 2025
4a9d879
renamed temporaryDatabaseUserLifetimeSeconds -> atlasTemporaryDatabas…
jeroenvervaeke Sep 11, 2025
9581337
split options
jeroenvervaeke Sep 11, 2025
ce905bd
fixed shallow merge issue
jeroenvervaeke Sep 11, 2025
90123d3
fixed typing issue not caught by typescript
jeroenvervaeke Sep 11, 2025
b7e0d1b
updated the readme
jeroenvervaeke Sep 11, 2025
b0e8b94
converted duration numbers to multiplications
jeroenvervaeke Sep 11, 2025
17c15bb
added mergeOptions and options interface
jeroenvervaeke Sep 11, 2025
5f60f73
Merge branch 'main' into MCP-199
jeroenvervaeke Sep 12, 2025
6700878
undo requested PR change requests
jeroenvervaeke Sep 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion src/common/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import type { Secret } from "./keychain.js";
import levenshtein from "ts-levenshtein";

// From: https://github.com/mongodb-js/mongosh/blob/main/packages/cli-repl/src/arg-parser.ts
const OPTIONS = {
const MONGOSH_OPTIONS = {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is a problem. The options that you see here are not all MONGOSH options. They contain MCP related options as well.

I don't have strong opinion on fixing this right now but consider that this is only a partial solution. We still need to address it entirely, perhaps with https://jira.mongodb.org/browse/MCP-202

string: [
"apiBaseUrl",
"apiClientId",
Expand Down Expand Up @@ -92,6 +92,18 @@ const OPTIONS = {
},
} as const;

const MCP_SERVER_OPTIONS = {
string: ["atlasTemporaryDatabaseUserLifetimeMs"],
} as const;

const OPTIONS = {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is fine. alternative idea for consideration but non-blocking:

function mergeOptions(...optionSources: Array<Record<string, any>>) {
    return {
        string: optionSources.flatMap(opts => opts.string ?? []),
        boolean: optionSources.flatMap(opts => opts.boolean ?? []),
        array: optionSources.flatMap(opts => opts.array ?? []),
        alias: Object.assign({}, ...optionSources.map(opts => opts.alias ?? {})),
        configuration: Object.assign({}, ...optionSources.map(opts => opts.configuration ?? {})),
    } as const;
}

const OPTIONS = mergeOptions(MONGOSH_OPTIONS, MCP_SERVER_OPTIONS);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have accepted your suggestion.

I had to introduce a new Options interface to make the linter happy.

string: [...MONGOSH_OPTIONS.string, ...MCP_SERVER_OPTIONS.string],
boolean: [...MONGOSH_OPTIONS.boolean],
array: [...MONGOSH_OPTIONS.array],
alias: { ...MONGOSH_OPTIONS.alias },
configuration: { ...MONGOSH_OPTIONS.configuration },
} as const;

const ALL_CONFIG_KEYS = new Set(
(OPTIONS.string as readonly string[])
.concat(OPTIONS.array)
Expand Down Expand Up @@ -161,6 +173,7 @@ export interface UserConfig extends CliOptions {
loggers: Array<"stderr" | "disk" | "mcp">;
idleTimeoutMs: number;
notificationTimeoutMs: number;
atlasTemporaryDatabaseUserLifetimeMs: number;
}

export const defaultUserConfig: UserConfig = {
Expand All @@ -180,6 +193,7 @@ export const defaultUserConfig: UserConfig = {
idleTimeoutMs: 600000, // 10 minutes
notificationTimeoutMs: 540000, // 9 minutes
httpHeaders: {},
atlasTemporaryDatabaseUserLifetimeMs: 14400000, // 4 hours
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When representing time, I find it easier to reason about the numbers if they're expressed as multiplication of units of time - i.e. 1000 * 60 * 60 * 4 rather than the result - it's much easier to spot mistakes than making the calculation in your head.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback! I chose to use the plain number format here to maintain consistency with the other time values already defined in this struct, which all follow the same pattern.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't mind changing the others to match as a drive-by as well

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've update the numbers

};

export const config = setupUserConfig({
Expand Down
3 changes: 1 addition & 2 deletions src/tools/atlas/connect/connectCluster.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { ensureCurrentIpInAccessList } from "../../../common/atlas/accessListUti
import type { AtlasClusterConnectionInfo } from "../../../common/connectionManager.js";
import { getDefaultRoleFromConfig } from "../../../common/atlas/roles.js";

const EXPIRY_MS = 1000 * 60 * 60 * 12; // 12 hours
const addedIpAccessListMessage =
"Note: Your current IP address has been added to the Atlas project's IP access list to enable secure connection.";

Expand Down Expand Up @@ -77,7 +76,7 @@ export class ConnectClusterTool extends AtlasToolBase {
const username = `mcpUser${Math.floor(Math.random() * 100000)}`;
const password = await generateSecurePassword();

const expiryDate = new Date(Date.now() + EXPIRY_MS);
const expiryDate = new Date(Date.now() + this.config.atlasTemporaryDatabaseUserLifetimeMs);
const role = getDefaultRoleFromConfig(this.config);

await this.session.apiClient.createDatabaseUser({
Expand Down
9 changes: 9 additions & 0 deletions tests/unit/common/config.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ describe("config", () => {
{ envVar: "MDB_MCP_HTTP_HOST", property: "httpHost", value: "localhost" },
{ envVar: "MDB_MCP_IDLE_TIMEOUT_MS", property: "idleTimeoutMs", value: 5000 },
{ envVar: "MDB_MCP_NOTIFICATION_TIMEOUT_MS", property: "notificationTimeoutMs", value: 5000 },
{
envVar: "MDB_MCP_ATLAS_TEMPORARY_DATABASE_USER_LIFETIME_MS",
property: "atlasTemporaryDatabaseUserLifetimeMs",
value: 12345,
},
] as const;

for (const { envVar, property, value } of testCases) {
Expand Down Expand Up @@ -129,6 +134,10 @@ describe("config", () => {
cli: ["--notificationTimeoutMs", "42"],
expected: { notificationTimeoutMs: "42" },
},
{
cli: ["--atlasTemporaryDatabaseUserLifetimeMs", "12345"],
expected: { atlasTemporaryDatabaseUserLifetimeMs: "12345" },
},
{
cli: ["--telemetry", "enabled"],
expected: { telemetry: "enabled" },
Expand Down
Loading