From 02d58c72ea681cd1359f3193eca19967fc53390a Mon Sep 17 00:00:00 2001 From: Kevin Mas Ruiz Date: Mon, 28 Jul 2025 20:33:44 +0200 Subject: [PATCH 01/10] chore: [broken] Add a test to verify that the http-proxy is called. Right now the code is broken because there is some issue with the promise returned by the agent. I'm still investigating it. --- package-lock.json | 63 +++++++++++++++++++++++++++++ package.json | 7 +++- src/common/atlas/apiClient.ts | 18 +++++++++ tests/unit/common/apiClient.test.ts | 39 ++++++++++++++++++ 4 files changed, 125 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index 71889e3ee..f6dfe0584 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,6 +12,7 @@ "@modelcontextprotocol/sdk": "^1.15.0", "@mongodb-js/device-id": "^0.3.1", "@mongodb-js/devtools-connect": "^3.9.2", + "@mongodb-js/devtools-proxy-support": "^0.5.1", "@mongosh/service-provider-node-driver": "^3.10.2", "@vitest/eslint-plugin": "^1.3.4", "bson": "^6.10.4", @@ -39,6 +40,7 @@ "@modelcontextprotocol/inspector": "^0.16.0", "@redocly/cli": "^1.34.4", "@types/express": "^5.0.1", + "@types/http-proxy": "^1.17.16", "@types/node": "^24.0.12", "@types/proper-lockfile": "^4.1.4", "@types/simple-oauth2": "^5.0.7", @@ -49,6 +51,7 @@ "eslint-config-prettier": "^10.1.5", "eslint-plugin-prettier": "^5.5.1", "globals": "^16.3.0", + "http-proxy": "^1.18.1", "mongodb-runner": "^5.9.2", "ollama-ai-provider": "^1.2.0", "openapi-types": "^12.1.3", @@ -5384,6 +5387,16 @@ "dev": true, "license": "MIT" }, + "node_modules/@types/http-proxy": { + "version": "1.17.16", + "resolved": "https://registry.npmjs.org/@types/http-proxy/-/http-proxy-1.17.16.tgz", + "integrity": "sha512-sdWoUajOB1cd0A8cRRQ1cfyWNbmFKLAqBB89Y8x5iYyG/mkJHc0YUH8pdWBy2omi9qtCpiIgGjuwO0dQST2l5w==", + "dev": true, + "license": "MIT", + "dependencies": { + "@types/node": "*" + } + }, "node_modules/@types/json-schema": { "version": "7.0.15", "resolved": "https://registry.npmjs.org/@types/json-schema/-/json-schema-7.0.15.tgz", @@ -8249,6 +8262,27 @@ "integrity": "sha512-GX+ysw4PBCz0PzosHDepZGANEuFCMLrnRTiEy9McGjmkCQYwRq4A/X786G/fjM/+OjsWSU1ZrY5qyARZmO/uwg==", "license": "ISC" }, + "node_modules/follow-redirects": { + "version": "1.15.9", + "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.15.9.tgz", + "integrity": "sha512-gew4GsXizNgdoRyqmyfMHyAmXsZDk6mHkSxZFCzW9gwlbtOW44CDtYavM+y+72qD/Vq2l550kMF52DT8fOLJqQ==", + "dev": true, + "funding": [ + { + "type": "individual", + "url": "https://github.com/sponsors/RubenVerborgh" + } + ], + "license": "MIT", + "engines": { + "node": ">=4.0" + }, + "peerDependenciesMeta": { + "debug": { + "optional": true + } + } + }, "node_modules/for-each": { "version": "0.3.5", "resolved": "https://registry.npmjs.org/for-each/-/for-each-0.3.5.tgz", @@ -8779,6 +8813,21 @@ "node": ">= 0.8" } }, + "node_modules/http-proxy": { + "version": "1.18.1", + "resolved": "https://registry.npmjs.org/http-proxy/-/http-proxy-1.18.1.tgz", + "integrity": "sha512-7mz/721AbnJwIVbnaSv1Cz3Am0ZLT/UBwkC92VlxhXv/k/BBQfM2fXElQNC27BVGr0uwUpplYPQM9LnaBMR5NQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "eventemitter3": "^4.0.0", + "follow-redirects": "^1.0.0", + "requires-port": "^1.0.0" + }, + "engines": { + "node": ">=8.0.0" + } + }, "node_modules/http-proxy-agent": { "version": "7.0.2", "resolved": "https://registry.npmjs.org/http-proxy-agent/-/http-proxy-agent-7.0.2.tgz", @@ -8792,6 +8841,13 @@ "node": ">= 14" } }, + "node_modules/http-proxy/node_modules/eventemitter3": { + "version": "4.0.7", + "resolved": "https://registry.npmjs.org/eventemitter3/-/eventemitter3-4.0.7.tgz", + "integrity": "sha512-8guHBZCwKnFhYdHr2ysuRWErTwhoN2X8XELRlrRwpmfeY2jjuUN4taQMsULKUVo1K4DvZl+0pgfyoysHxvmvEw==", + "dev": true, + "license": "MIT" + }, "node_modules/http2-client": { "version": "1.3.5", "resolved": "https://registry.npmjs.org/http2-client/-/http2-client-1.3.5.tgz", @@ -11699,6 +11755,13 @@ "node": ">=0.10.0" } }, + "node_modules/requires-port": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/requires-port/-/requires-port-1.0.0.tgz", + "integrity": "sha512-KigOCHcocU3XODJxsu8i/j8T9tzT4adHiecwORRQ0ZZFcp7ahwXuRU1m+yuO90C5ZUyGeGfocHDI14M3L3yDAQ==", + "dev": true, + "license": "MIT" + }, "node_modules/reservoir": { "version": "0.1.2", "resolved": "https://registry.npmjs.org/reservoir/-/reservoir-0.1.2.tgz", diff --git a/package.json b/package.json index 7b87e0be3..b2ce71ad5 100644 --- a/package.json +++ b/package.json @@ -16,7 +16,7 @@ }, "type": "module", "scripts": { - "start": "node dist/index.js --transport http --loggers stderr mcp", + "start": "node dist/index.js --transport stdio --loggers stderr mcp", "prepare": "npm run build", "build:clean": "rm -rf dist", "build:compile": "tsc --project tsconfig.build.json", @@ -43,6 +43,7 @@ "@modelcontextprotocol/inspector": "^0.16.0", "@redocly/cli": "^1.34.4", "@types/express": "^5.0.1", + "@types/http-proxy": "^1.17.16", "@types/node": "^24.0.12", "@types/proper-lockfile": "^4.1.4", "@types/simple-oauth2": "^5.0.7", @@ -53,6 +54,7 @@ "eslint-config-prettier": "^10.1.5", "eslint-plugin-prettier": "^5.5.1", "globals": "^16.3.0", + "http-proxy": "^1.18.1", "mongodb-runner": "^5.9.2", "ollama-ai-provider": "^1.2.0", "openapi-types": "^12.1.3", @@ -63,14 +65,15 @@ "tsx": "^4.20.3", "typescript": "^5.8.3", "typescript-eslint": "^8.36.0", - "vitest": "^3.2.4", "uuid": "^11.1.0", + "vitest": "^3.2.4", "yaml": "^2.8.0" }, "dependencies": { "@modelcontextprotocol/sdk": "^1.15.0", "@mongodb-js/device-id": "^0.3.1", "@mongodb-js/devtools-connect": "^3.9.2", + "@mongodb-js/devtools-proxy-support": "^0.5.1", "@mongosh/service-provider-node-driver": "^3.10.2", "@vitest/eslint-plugin": "^1.3.4", "bson": "^6.10.4", diff --git a/src/common/atlas/apiClient.ts b/src/common/atlas/apiClient.ts index a587d04ac..ff18118dd 100644 --- a/src/common/atlas/apiClient.ts +++ b/src/common/atlas/apiClient.ts @@ -6,6 +6,8 @@ import { paths, operations } from "./openapi.js"; import { CommonProperties, TelemetryEvent } from "../../telemetry/types.js"; import { packageInfo } from "../packageInfo.js"; import logger, { LogId } from "../logger.js"; +import { createFetch, useOrCreateAgent } from "@mongodb-js/devtools-proxy-support"; +import HTTPS from "https"; const ATLAS_API_VERSION = "2025-03-12"; @@ -29,11 +31,25 @@ export class ApiClient { clientSecret: string; }; }; + + private static customFetch = createFetch({ + useEnvironmentVariableProxies: true, + }) as unknown as typeof fetch; + + private static customAgent = useOrCreateAgent({ + useEnvironmentVariableProxies: true, + }); + private client: Client; private oauth2Client?: ClientCredentials; private accessToken?: AccessToken; + private ensureAgentIsInitialized = async () => { + await ApiClient.customAgent?.initialize?.(); + }; + private getAccessToken = async () => { + // await this.ensureAgentIsInitialized(); if (this.oauth2Client && (!this.accessToken || this.accessToken.expired())) { this.accessToken = await this.oauth2Client.getToken({}); } @@ -72,6 +88,7 @@ export class ApiClient { "User-Agent": this.options.userAgent, Accept: `application/vnd.atlas.${ATLAS_API_VERSION}+json`, }, + fetch: ApiClient.customFetch, }); if (this.options.credentials?.clientId && this.options.credentials?.clientSecret) { this.oauth2Client = new ClientCredentials({ @@ -88,6 +105,7 @@ export class ApiClient { headers: { "User-Agent": this.options.userAgent, }, + agent: ApiClient.customAgent, }, }); this.client.use(this.authMiddleware); diff --git a/tests/unit/common/apiClient.test.ts b/tests/unit/common/apiClient.test.ts index 0c93f2191..4b6afc11f 100644 --- a/tests/unit/common/apiClient.test.ts +++ b/tests/unit/common/apiClient.test.ts @@ -1,6 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { ApiClient } from "../../../src/common/atlas/apiClient.js"; import { CommonProperties, TelemetryEvent, TelemetryResult } from "../../../src/telemetry/types.js"; +import httpProxy from "http-proxy"; describe("ApiClient", () => { let apiClient: ApiClient; @@ -94,6 +95,44 @@ describe("ApiClient", () => { }); }); + describe("oauth authentication proxy", () => { + let apiClient: ApiClient; + let proxyServer: httpProxy; + let requests: unknown[]; + + beforeEach(() => { + process.env.HTTP_PROXY = "localhost:8888"; + apiClient = new ApiClient({ + baseUrl: "https://httpbin.org", + credentials: { + clientId: "test-client-id", + clientSecret: "test-client-secret", + }, + userAgent: "test-user-agent", + }); + + proxyServer = httpProxy.createProxyServer({ target: "https://api.test.com" }); + proxyServer.on("proxyReq", (proxyReq, req, res, options) => { + requests.push(req); + }); + + requests = []; + }); + + afterEach(async () => { + delete process.env.HTTP_PROXY; + + await proxyServer.close(); + //await apiClient.close(); + }); + + it("should send the oauth request through a proxy if configured", async () => { + await apiClient.validateAccessToken(); + console.log(requests); + expect(true).toBeFalsy(); + }); + }); + describe("sendEvents", () => { it("should send events to authenticated endpoint when token is available and valid", async () => { const mockFetch = vi.spyOn(global, "fetch"); From 517511744ed4c89d4613efe8a58fab11dec4553e Mon Sep 17 00:00:00 2001 From: Kevin Mas Ruiz Date: Tue, 29 Jul 2025 15:04:11 +0200 Subject: [PATCH 02/10] chore: Change to use oauth4webapi instead of simple-oauth2 This adds support for a custom fetch that can use proxies configured with environment variables. --- package-lock.json | 114 +-------------------------- package.json | 2 +- src/common/atlas/apiClient.ts | 142 ++++++++++++++++++++++++---------- 3 files changed, 106 insertions(+), 152 deletions(-) diff --git a/package-lock.json b/package-lock.json index f6dfe0584..fbb7a0673 100644 --- a/package-lock.json +++ b/package-lock.json @@ -24,8 +24,8 @@ "mongodb-redact": "^1.1.8", "mongodb-schema": "^12.6.2", "node-machine-id": "1.1.12", + "oauth4webapi": "^3.6.0", "openapi-fetch": "^0.14.0", - "simple-oauth2": "^5.1.0", "yargs-parser": "^22.0.0", "zod": "^3.25.76" }, @@ -1711,53 +1711,6 @@ "dev": true, "license": "MIT" }, - "node_modules/@hapi/boom": { - "version": "10.0.1", - "resolved": "https://registry.npmjs.org/@hapi/boom/-/boom-10.0.1.tgz", - "integrity": "sha512-ERcCZaEjdH3OgSJlyjVk8pHIFeus91CjKP3v+MpgBNp5IvGzP2l/bRiD78nqYcKPaZdbKkK5vDBVPd2ohHBlsA==", - "license": "BSD-3-Clause", - "dependencies": { - "@hapi/hoek": "^11.0.2" - } - }, - "node_modules/@hapi/bourne": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/@hapi/bourne/-/bourne-3.0.0.tgz", - "integrity": "sha512-Waj1cwPXJDucOib4a3bAISsKJVb15MKi9IvmTI/7ssVEm6sywXGjVJDhl6/umt1pK1ZS7PacXU3A1PmFKHEZ2w==", - "license": "BSD-3-Clause" - }, - "node_modules/@hapi/hoek": { - "version": "11.0.7", - "resolved": "https://registry.npmjs.org/@hapi/hoek/-/hoek-11.0.7.tgz", - "integrity": "sha512-HV5undWkKzcB4RZUusqOpcgxOaq6VOAH7zhhIr2g3G8NF/MlFO75SjOr2NfuSx0Mh40+1FqCkagKLJRykUWoFQ==", - "license": "BSD-3-Clause" - }, - "node_modules/@hapi/topo": { - "version": "5.1.0", - "resolved": "https://registry.npmjs.org/@hapi/topo/-/topo-5.1.0.tgz", - "integrity": "sha512-foQZKJig7Ob0BMAYBfcJk8d77QtOe7Wo4ox7ff1lQYoNNAb6jwcY1ncdoy2e9wQZzvNy7ODZCYJkK8kzmcAnAg==", - "license": "BSD-3-Clause", - "dependencies": { - "@hapi/hoek": "^9.0.0" - } - }, - "node_modules/@hapi/topo/node_modules/@hapi/hoek": { - "version": "9.3.0", - "resolved": "https://registry.npmjs.org/@hapi/hoek/-/hoek-9.3.0.tgz", - "integrity": "sha512-/c6rf4UJlmHlC9b5BaNvzAcFv7HZ2QHaV0D4/HNlBdvFnvQq8RI4kYdhyPCl7Xj+oWvTWQ8ujhqS53LIgAe6KQ==", - "license": "BSD-3-Clause" - }, - "node_modules/@hapi/wreck": { - "version": "18.1.0", - "resolved": "https://registry.npmjs.org/@hapi/wreck/-/wreck-18.1.0.tgz", - "integrity": "sha512-0z6ZRCmFEfV/MQqkQomJ7sl/hyxvcZM7LtuVqN3vdAO4vM9eBbowl0kaqQj9EJJQab+3Uuh1GxbGIBFy4NfJ4w==", - "license": "BSD-3-Clause", - "dependencies": { - "@hapi/boom": "^10.0.1", - "@hapi/bourne": "^3.0.0", - "@hapi/hoek": "^11.0.2" - } - }, "node_modules/@humanfs/core": { "version": "0.19.1", "resolved": "https://registry.npmjs.org/@humanfs/core/-/core-0.19.1.tgz", @@ -4656,33 +4609,6 @@ "win32" ] }, - "node_modules/@sideway/address": { - "version": "4.1.5", - "resolved": "https://registry.npmjs.org/@sideway/address/-/address-4.1.5.tgz", - "integrity": "sha512-IqO/DUQHUkPeixNQ8n0JA6102hT9CmaljNTPmQ1u8MEhBo/R4Q8eKLN/vGZxuebwOroDB4cbpjheD4+/sKFK4Q==", - "license": "BSD-3-Clause", - "dependencies": { - "@hapi/hoek": "^9.0.0" - } - }, - "node_modules/@sideway/address/node_modules/@hapi/hoek": { - "version": "9.3.0", - "resolved": "https://registry.npmjs.org/@hapi/hoek/-/hoek-9.3.0.tgz", - "integrity": "sha512-/c6rf4UJlmHlC9b5BaNvzAcFv7HZ2QHaV0D4/HNlBdvFnvQq8RI4kYdhyPCl7Xj+oWvTWQ8ujhqS53LIgAe6KQ==", - "license": "BSD-3-Clause" - }, - "node_modules/@sideway/formula": { - "version": "3.0.1", - "resolved": "https://registry.npmjs.org/@sideway/formula/-/formula-3.0.1.tgz", - "integrity": "sha512-/poHZJJVjx3L+zVD6g9KgHfYnb443oi7wLu/XKojDviHy6HOEOA6z1Trk5aR1dGcmPenJEgb2sK2I80LeS3MIg==", - "license": "BSD-3-Clause" - }, - "node_modules/@sideway/pinpoint": { - "version": "2.0.0", - "resolved": "https://registry.npmjs.org/@sideway/pinpoint/-/pinpoint-2.0.0.tgz", - "integrity": "sha512-RNiOoTPkptFtSVzQevY/yWtZwf/RxyVnPy/OcA9HBM3MlGDnBEYL5B41H0MTn0Uec8Hi+2qUtTfG2WWZBmMejQ==", - "license": "BSD-3-Clause" - }, "node_modules/@sinclair/typebox": { "version": "0.27.8", "resolved": "https://registry.npmjs.org/@sinclair/typebox/-/typebox-0.27.8.tgz", @@ -9296,25 +9222,6 @@ "node": "^14.15.0 || ^16.10.0 || >=18.0.0" } }, - "node_modules/joi": { - "version": "17.13.3", - "resolved": "https://registry.npmjs.org/joi/-/joi-17.13.3.tgz", - "integrity": "sha512-otDA4ldcIx+ZXsKHWmp0YizCweVRZG96J10b0FevjfuncLO1oX59THoAmHkNubYJ+9gWsYsp5k8v4ib6oDv1fA==", - "license": "BSD-3-Clause", - "dependencies": { - "@hapi/hoek": "^9.3.0", - "@hapi/topo": "^5.1.0", - "@sideway/address": "^4.1.5", - "@sideway/formula": "^3.0.1", - "@sideway/pinpoint": "^2.0.0" - } - }, - "node_modules/joi/node_modules/@hapi/hoek": { - "version": "9.3.0", - "resolved": "https://registry.npmjs.org/@hapi/hoek/-/hoek-9.3.0.tgz", - "integrity": "sha512-/c6rf4UJlmHlC9b5BaNvzAcFv7HZ2QHaV0D4/HNlBdvFnvQq8RI4kYdhyPCl7Xj+oWvTWQ8ujhqS53LIgAe6KQ==", - "license": "BSD-3-Clause" - }, "node_modules/jose": { "version": "6.0.11", "resolved": "https://registry.npmjs.org/jose/-/jose-6.0.11.tgz", @@ -10575,11 +10482,10 @@ } }, "node_modules/oauth4webapi": { - "version": "3.5.5", - "resolved": "https://registry.npmjs.org/oauth4webapi/-/oauth4webapi-3.5.5.tgz", - "integrity": "sha512-1K88D2GiAydGblHo39NBro5TebGXa+7tYoyIbxvqv3+haDDry7CBE1eSYuNbOSsYCCU6y0gdynVZAkm4YPw4hg==", + "version": "3.6.0", + "resolved": "https://registry.npmjs.org/oauth4webapi/-/oauth4webapi-3.6.0.tgz", + "integrity": "sha512-OwXPTXjKPOldTpAa19oksrX9TYHA0rt+VcUFTkJ7QKwgmevPpNm9Cn5vFZUtIo96FiU6AfPuUUGzoXqgOzibWg==", "license": "MIT", - "peer": true, "funding": { "url": "https://github.com/sponsors/panva" } @@ -12414,18 +12320,6 @@ "url": "https://github.com/steveukx/git-js?sponsor=1" } }, - "node_modules/simple-oauth2": { - "version": "5.1.0", - "resolved": "https://registry.npmjs.org/simple-oauth2/-/simple-oauth2-5.1.0.tgz", - "integrity": "sha512-gWDa38Ccm4MwlG5U7AlcJxPv3lvr80dU7ARJWrGdgvOKyzSj1gr3GBPN1rABTedAYvC/LsGYoFuFxwDBPtGEbw==", - "license": "Apache-2.0", - "dependencies": { - "@hapi/hoek": "^11.0.4", - "@hapi/wreck": "^18.0.0", - "debug": "^4.3.4", - "joi": "^17.6.4" - } - }, "node_modules/simple-websocket": { "version": "9.1.0", "resolved": "https://registry.npmjs.org/simple-websocket/-/simple-websocket-9.1.0.tgz", diff --git a/package.json b/package.json index b2ce71ad5..9a75795f6 100644 --- a/package.json +++ b/package.json @@ -85,8 +85,8 @@ "mongodb-redact": "^1.1.8", "mongodb-schema": "^12.6.2", "node-machine-id": "1.1.12", + "oauth4webapi": "^3.6.0", "openapi-fetch": "^0.14.0", - "simple-oauth2": "^5.1.0", "yargs-parser": "^22.0.0", "zod": "^3.25.76" }, diff --git a/src/common/atlas/apiClient.ts b/src/common/atlas/apiClient.ts index ff18118dd..e8abb5b1a 100644 --- a/src/common/atlas/apiClient.ts +++ b/src/common/atlas/apiClient.ts @@ -1,13 +1,12 @@ import createClient, { Client, Middleware } from "openapi-fetch"; import type { FetchOptions } from "openapi-fetch"; -import { AccessToken, ClientCredentials } from "simple-oauth2"; import { ApiClientError } from "./apiClientError.js"; import { paths, operations } from "./openapi.js"; import { CommonProperties, TelemetryEvent } from "../../telemetry/types.js"; import { packageInfo } from "../packageInfo.js"; import logger, { LogId } from "../logger.js"; -import { createFetch, useOrCreateAgent } from "@mongodb-js/devtools-proxy-support"; -import HTTPS from "https"; +import { createFetch } from "@mongodb-js/devtools-proxy-support"; +import * as oauth from "oauth4webapi"; const ATLAS_API_VERSION = "2025-03-12"; @@ -22,6 +21,11 @@ export interface ApiClientOptions { userAgent?: string; } +interface AccessToken { + access_token: string; + expires_at?: number; +} + export class ApiClient { private options: { baseUrl: string; @@ -36,24 +40,33 @@ export class ApiClient { useEnvironmentVariableProxies: true, }) as unknown as typeof fetch; - private static customAgent = useOrCreateAgent({ - useEnvironmentVariableProxies: true, - }); - private client: Client; - private oauth2Client?: ClientCredentials; + + private oauth2Client?: oauth.Client; + private oauth2Issuer?: oauth.AuthorizationServer; private accessToken?: AccessToken; - private ensureAgentIsInitialized = async () => { - await ApiClient.customAgent?.initialize?.(); - }; + public hasCredentials(): boolean { + return !!this.oauth2Client && !!this.oauth2Issuer; + } + + private isAccessTokenValid(): boolean { + return !!( + this.accessToken && + (this.accessToken.expires_at == undefined || this.accessToken.expires_at > Date.now() / 1000) + ); + } private getAccessToken = async () => { - // await this.ensureAgentIsInitialized(); - if (this.oauth2Client && (!this.accessToken || this.accessToken.expired())) { - this.accessToken = await this.oauth2Client.getToken({}); + if (!this.hasCredentials()) { + return undefined; } - return this.accessToken?.token.access_token as string | undefined; + + if (!this.isAccessTokenValid()) { + this.accessToken = await this.getNewAccessToken(); + } + + return this.accessToken?.access_token; }; private authMiddleware: Middleware = { @@ -90,30 +103,76 @@ export class ApiClient { }, fetch: ApiClient.customFetch, }); + if (this.options.credentials?.clientId && this.options.credentials?.clientSecret) { - this.oauth2Client = new ClientCredentials({ - client: { - id: this.options.credentials.clientId, - secret: this.options.credentials.clientSecret, - }, - auth: { - tokenHost: this.options.baseUrl, - tokenPath: "/api/oauth/token", - revokePath: "/api/oauth/revoke", - }, - http: { - headers: { - "User-Agent": this.options.userAgent, - }, - agent: ApiClient.customAgent, - }, - }); + this.oauth2Issuer = { + issuer: this.options.baseUrl, + token_endpoint: new URL("/api/oauth/token", this.options.baseUrl).toString(), + revocation_endpoint: new URL("/api/oauth/revoke", this.options.baseUrl).toString(), + token_endpoint_auth_methods_supported: ["client_secret_basic"], + grant_types_supported: ["client_credentials"], + }; + + this.oauth2Client = { + client_id: this.options.credentials.clientId, + client_secret: this.options.credentials.clientSecret, + }; + this.client.use(this.authMiddleware); } } - public hasCredentials(): boolean { - return !!this.oauth2Client; + private getOauthClientAuth(): { client: oauth.Client | undefined; clientAuth: oauth.ClientAuth | undefined } { + if (this.options.credentials?.clientId && this.options.credentials.clientSecret) { + const clientSecret = this.options.credentials.clientSecret; + const clientId = this.options.credentials.clientId; + + // We are using our own ClientAuth because ClientSecretBasic URL encodes wrongly + // the username and password (for example, encodes `_` which is wrong). + return { + client: { client_id: clientId }, + clientAuth: (_as, client, _body, headers) => { + const credentials = Buffer.from(`${clientId}:${clientSecret}`).toString("base64"); + headers.set("Authorization", `Basic ${credentials}`); + }, + }; + } + + return { client: undefined, clientAuth: undefined }; + } + + private async getNewAccessToken(): Promise { + if (!this.hasCredentials() || !this.oauth2Issuer) { + return undefined; + } + + const { client, clientAuth } = this.getOauthClientAuth(); + if (client && clientAuth) { + try { + const response = await oauth.clientCredentialsGrantRequest( + this.oauth2Issuer, + client, + clientAuth, + new URLSearchParams(), + { + [oauth.customFetch]: ApiClient.customFetch, + headers: { + "User-Agent": this.options.userAgent, + }, + } + ); + + const result = await oauth.processClientCredentialsResponse(this.oauth2Issuer, client, response); + this.accessToken = { + access_token: result.access_token, + expires_at: Date.now() / 1000 + (result.expires_in ?? 0), + }; + } catch (error: unknown) { + const err = error instanceof Error ? error : new Error(String(error)); + logger.error(LogId.atlasConnectFailure, "apiClient", `Failed to request access token: ${err.message}`); + } + return this.accessToken; + } } public async validateAccessToken(): Promise { @@ -121,15 +180,16 @@ export class ApiClient { } public async close(): Promise { - if (this.accessToken) { - try { - await this.accessToken.revoke("access_token"); - } catch (error: unknown) { - const err = error instanceof Error ? error : new Error(String(error)); - logger.error(LogId.atlasApiRevokeFailure, "apiClient", `Failed to revoke access token: ${err.message}`); + const { client, clientAuth } = this.getOauthClientAuth(); + try { + if (this.oauth2Issuer && this.accessToken && client && clientAuth) { + await oauth.revocationRequest(this.oauth2Issuer, client, clientAuth, this.accessToken.access_token); } - this.accessToken = undefined; + } catch (error: unknown) { + const err = error instanceof Error ? error : new Error(String(error)); + logger.error(LogId.atlasApiRevokeFailure, "apiClient", `Failed to revoke access token: ${err.message}`); } + this.accessToken = undefined; } public async getIpInfo(): Promise<{ From a39df442886055bb2a56ce6a344d3836cb05e08d Mon Sep 17 00:00:00 2001 From: Kevin Mas Ruiz Date: Tue, 29 Jul 2025 19:08:03 +0200 Subject: [PATCH 03/10] chore: Add integration tests for proxy support in the Atlas API Uses a variant devtools-shared/devtools-proxy-support http proxy server for testing. --- package-lock.json | 124 ++++++++++------- package.json | 2 +- src/common/atlas/apiClient.ts | 2 +- tests/integration/common/apiClient.test.ts | 77 +++++++++++ .../fixtures/httpsServerProxyTest.ts | 126 ++++++++++++++++++ tests/integration/fixtures/server.key | 28 ++++ tests/integration/fixtures/server.pem | 18 +++ tests/unit/common/apiClient.test.ts | 39 ------ 8 files changed, 324 insertions(+), 92 deletions(-) create mode 100644 tests/integration/common/apiClient.test.ts create mode 100644 tests/integration/fixtures/httpsServerProxyTest.ts create mode 100644 tests/integration/fixtures/server.key create mode 100644 tests/integration/fixtures/server.pem diff --git a/package-lock.json b/package-lock.json index fbb7a0673..8e00371ac 100644 --- a/package-lock.json +++ b/package-lock.json @@ -47,11 +47,11 @@ "@types/yargs-parser": "^21.0.3", "@vitest/coverage-v8": "^3.2.4", "ai": "^4.3.17", + "duplexpair": "^1.0.2", "eslint": "^9.30.1", "eslint-config-prettier": "^10.1.5", "eslint-plugin-prettier": "^5.5.1", "globals": "^16.3.0", - "http-proxy": "^1.18.1", "mongodb-runner": "^5.9.2", "ollama-ai-provider": "^1.2.0", "openapi-types": "^12.1.3", @@ -7375,6 +7375,58 @@ "node": ">= 0.4" } }, + "node_modules/duplexpair": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/duplexpair/-/duplexpair-1.0.2.tgz", + "integrity": "sha512-6DHuWdEGHNcuSqrn926rWJcRsTDrb+ugw0hx/trAxCH48z9WlFqDtwtbiEMq/KGFYQWzLs1VA0I6KUkuIgCoXw==", + "dev": true, + "license": "MIT", + "dependencies": { + "readable-stream": "^4.5.2" + } + }, + "node_modules/duplexpair/node_modules/buffer": { + "version": "6.0.3", + "resolved": "https://registry.npmjs.org/buffer/-/buffer-6.0.3.tgz", + "integrity": "sha512-FTiCpNxtwiZZHEZbcbTIcZjERVICn9yq/pDFkTl95/AxzD1naBctN7YO68riM/gLSDY7sdrMby8hofADYuuqOA==", + "dev": true, + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/feross" + }, + { + "type": "patreon", + "url": "https://www.patreon.com/feross" + }, + { + "type": "consulting", + "url": "https://feross.org/support" + } + ], + "license": "MIT", + "dependencies": { + "base64-js": "^1.3.1", + "ieee754": "^1.2.1" + } + }, + "node_modules/duplexpair/node_modules/readable-stream": { + "version": "4.7.0", + "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-4.7.0.tgz", + "integrity": "sha512-oIGGmcpTLwPga8Bn6/Z75SVaH1z5dUut2ibSyAMVhmUggWpmDn2dapB0n7f8nwaSiRtepAsfJyfXIO5DCVAODg==", + "dev": true, + "license": "MIT", + "dependencies": { + "abort-controller": "^3.0.0", + "buffer": "^6.0.3", + "events": "^3.3.0", + "process": "^0.11.10", + "string_decoder": "^1.3.0" + }, + "engines": { + "node": "^12.22.0 || ^14.17.0 || >=16.0.0" + } + }, "node_modules/eastasianwidth": { "version": "0.2.0", "resolved": "https://registry.npmjs.org/eastasianwidth/-/eastasianwidth-0.2.0.tgz", @@ -7842,6 +7894,16 @@ "dev": true, "license": "MIT" }, + "node_modules/events": { + "version": "3.3.0", + "resolved": "https://registry.npmjs.org/events/-/events-3.3.0.tgz", + "integrity": "sha512-mQw+2fkQbALzQ7V0MY0IqdnXNOeTtP4r0lN9z7AAawCXgqea7bDii20AYrIBrFd/Hx0M2Ocz6S111CaFkUcb0Q==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=0.8.x" + } + }, "node_modules/eventsource": { "version": "3.0.7", "resolved": "https://registry.npmjs.org/eventsource/-/eventsource-3.0.7.tgz", @@ -8188,27 +8250,6 @@ "integrity": "sha512-GX+ysw4PBCz0PzosHDepZGANEuFCMLrnRTiEy9McGjmkCQYwRq4A/X786G/fjM/+OjsWSU1ZrY5qyARZmO/uwg==", "license": "ISC" }, - "node_modules/follow-redirects": { - "version": "1.15.9", - "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.15.9.tgz", - "integrity": "sha512-gew4GsXizNgdoRyqmyfMHyAmXsZDk6mHkSxZFCzW9gwlbtOW44CDtYavM+y+72qD/Vq2l550kMF52DT8fOLJqQ==", - "dev": true, - "funding": [ - { - "type": "individual", - "url": "https://github.com/sponsors/RubenVerborgh" - } - ], - "license": "MIT", - "engines": { - "node": ">=4.0" - }, - "peerDependenciesMeta": { - "debug": { - "optional": true - } - } - }, "node_modules/for-each": { "version": "0.3.5", "resolved": "https://registry.npmjs.org/for-each/-/for-each-0.3.5.tgz", @@ -8739,21 +8780,6 @@ "node": ">= 0.8" } }, - "node_modules/http-proxy": { - "version": "1.18.1", - "resolved": "https://registry.npmjs.org/http-proxy/-/http-proxy-1.18.1.tgz", - "integrity": "sha512-7mz/721AbnJwIVbnaSv1Cz3Am0ZLT/UBwkC92VlxhXv/k/BBQfM2fXElQNC27BVGr0uwUpplYPQM9LnaBMR5NQ==", - "dev": true, - "license": "MIT", - "dependencies": { - "eventemitter3": "^4.0.0", - "follow-redirects": "^1.0.0", - "requires-port": "^1.0.0" - }, - "engines": { - "node": ">=8.0.0" - } - }, "node_modules/http-proxy-agent": { "version": "7.0.2", "resolved": "https://registry.npmjs.org/http-proxy-agent/-/http-proxy-agent-7.0.2.tgz", @@ -8767,13 +8793,6 @@ "node": ">= 14" } }, - "node_modules/http-proxy/node_modules/eventemitter3": { - "version": "4.0.7", - "resolved": "https://registry.npmjs.org/eventemitter3/-/eventemitter3-4.0.7.tgz", - "integrity": "sha512-8guHBZCwKnFhYdHr2ysuRWErTwhoN2X8XELRlrRwpmfeY2jjuUN4taQMsULKUVo1K4DvZl+0pgfyoysHxvmvEw==", - "dev": true, - "license": "MIT" - }, "node_modules/http2-client": { "version": "1.3.5", "resolved": "https://registry.npmjs.org/http2-client/-/http2-client-1.3.5.tgz", @@ -11230,6 +11249,16 @@ "node": ">=6" } }, + "node_modules/process": { + "version": "0.11.10", + "resolved": "https://registry.npmjs.org/process/-/process-0.11.10.tgz", + "integrity": "sha512-cdGef/drWFoydD1JsMzuFf8100nZl+GT+yacc2bEced5f9Rjk4z+WtFUTBu9PhOi9j/jfmBPu0mMEY4wIdAF8A==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">= 0.6.0" + } + }, "node_modules/process-nextick-args": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/process-nextick-args/-/process-nextick-args-2.0.1.tgz", @@ -11661,13 +11690,6 @@ "node": ">=0.10.0" } }, - "node_modules/requires-port": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/requires-port/-/requires-port-1.0.0.tgz", - "integrity": "sha512-KigOCHcocU3XODJxsu8i/j8T9tzT4adHiecwORRQ0ZZFcp7ahwXuRU1m+yuO90C5ZUyGeGfocHDI14M3L3yDAQ==", - "dev": true, - "license": "MIT" - }, "node_modules/reservoir": { "version": "0.1.2", "resolved": "https://registry.npmjs.org/reservoir/-/reservoir-0.1.2.tgz", diff --git a/package.json b/package.json index 9a75795f6..6a28c71b1 100644 --- a/package.json +++ b/package.json @@ -50,11 +50,11 @@ "@types/yargs-parser": "^21.0.3", "@vitest/coverage-v8": "^3.2.4", "ai": "^4.3.17", + "duplexpair": "^1.0.2", "eslint": "^9.30.1", "eslint-config-prettier": "^10.1.5", "eslint-plugin-prettier": "^5.5.1", "globals": "^16.3.0", - "http-proxy": "^1.18.1", "mongodb-runner": "^5.9.2", "ollama-ai-provider": "^1.2.0", "openapi-types": "^12.1.3", diff --git a/src/common/atlas/apiClient.ts b/src/common/atlas/apiClient.ts index e8abb5b1a..1151e9f45 100644 --- a/src/common/atlas/apiClient.ts +++ b/src/common/atlas/apiClient.ts @@ -21,7 +21,7 @@ export interface ApiClientOptions { userAgent?: string; } -interface AccessToken { +export interface AccessToken { access_token: string; expires_at?: number; } diff --git a/tests/integration/common/apiClient.test.ts b/tests/integration/common/apiClient.test.ts new file mode 100644 index 000000000..49ea03714 --- /dev/null +++ b/tests/integration/common/apiClient.test.ts @@ -0,0 +1,77 @@ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import type { AccessToken } from "../../../src/common/atlas/apiClient.js"; +import { ApiClient } from "../../../src/common/atlas/apiClient.js"; +import { HTTPServerProxyTestSetup } from "../fixtures/httpsServerProxyTest.js"; + +describe("ApiClient integration test", () => { + describe("oauth authentication proxy", () => { + let apiClient: ApiClient; + let proxyTestSetup: HTTPServerProxyTestSetup; + + beforeEach(async () => { + process.env.NODE_TLS_REJECT_UNAUTHORIZED = "0"; + proxyTestSetup = new HTTPServerProxyTestSetup(); + await proxyTestSetup.listen(); + + process.env.HTTP_PROXY = `https://localhost:${proxyTestSetup.httpsProxyPort}/`; + apiClient = new ApiClient({ + baseUrl: `https://localhost:${proxyTestSetup.httpsServerPort}/`, + credentials: { + clientId: "test-client-id", + clientSecret: "test-client-secret", + }, + userAgent: "test-user-agent", + }); + }); + + function withToken(accessToken: string, expired: boolean) { + const apiClientMut = apiClient as unknown as { accessToken: AccessToken }; + const expireAt = expired ? Date.now() - 100000 : Date.now() + 10000; + + apiClientMut.accessToken = { + access_token: accessToken, + expires_at: expireAt, + }; + } + + afterEach(async () => { + delete process.env.NODE_TLS_REJECT_UNAUTHORIZED; + delete process.env.HTTP_PROXY; + + await apiClient.close(); + await proxyTestSetup.teardown(); + }); + + it("should send the oauth request through a proxy if configured", async () => { + await apiClient.validateAccessToken(); + expect(proxyTestSetup.getRequestedUrls()).toEqual([ + `http://localhost:${proxyTestSetup.httpsServerPort}/api/oauth/token`, + ]); + }); + + it("should send the oauth revoke request through a proxy if configured", async () => { + withToken("my non expired token", false); + await apiClient.close(); + expect(proxyTestSetup.getRequestedUrls()).toEqual([ + `http://localhost:${proxyTestSetup.httpsServerPort}/api/oauth/revoke`, + ]); + }); + + it("should make an atlas call when the token is not expired", async () => { + withToken("my not expired", false); + await apiClient.listOrganizations(); + expect(proxyTestSetup.getRequestedUrls()).toEqual([ + `http://localhost:${proxyTestSetup.httpsServerPort}/api/atlas/v2/orgs`, + ]); + }); + + it("should request a new token and an atlas call when the token is expired", async () => { + withToken("my expired", true); + await apiClient.listOrganizations(); + expect(proxyTestSetup.getRequestedUrls()).toEqual([ + `http://localhost:${proxyTestSetup.httpsServerPort}/api/oauth/token`, + `http://localhost:${proxyTestSetup.httpsServerPort}/api/atlas/v2/orgs`, + ]); + }); + }); +}); diff --git a/tests/integration/fixtures/httpsServerProxyTest.ts b/tests/integration/fixtures/httpsServerProxyTest.ts new file mode 100644 index 000000000..11fb326ee --- /dev/null +++ b/tests/integration/fixtures/httpsServerProxyTest.ts @@ -0,0 +1,126 @@ +import { once } from "events"; +import { readFileSync } from "fs"; +import type { Server as HTTPServer, IncomingMessage, RequestListener } from "http"; +import type { Server as HTTPSServer } from "https"; +import { createServer as createHTTPSServer } from "https"; +import type { AddressInfo } from "net"; +import path from "path"; +import { createServer as createHTTPServer, get as httpGet } from "http"; +import DuplexPair from "duplexpair"; +import { promisify } from "util"; +import type { Duplex } from "stream"; + +function parseHTTPAuthHeader(header: string | undefined): [string, string] { + if (!header?.startsWith("Basic ")) return ["", ""]; + const [username = "", pw = ""] = Buffer.from(header.split(" ")[1], "base64").toString().split(":"); + return [username, pw]; +} + +export class HTTPServerProxyTestSetup { + // Target servers: These actually handle requests. + readonly httpServer: HTTPServer; + readonly httpsServer: HTTPSServer; + // Proxy servers: + readonly httpProxyServer: HTTPServer; + readonly httpsProxyServer: HTTPServer; + readonly connections: Duplex[] = []; + canTunnel: () => boolean = () => true; + authHandler: undefined | ((username: string, password: string) => boolean); + + get httpServerPort(): number { + return (this.httpServer.address() as AddressInfo).port; + } + get httpsServerPort(): number { + return (this.httpsServer.address() as AddressInfo).port; + } + get httpProxyPort(): number { + return (this.httpProxyServer.address() as AddressInfo).port; + } + get httpsProxyPort(): number { + return (this.httpsProxyServer.address() as AddressInfo).port; + } + + requests: IncomingMessage[]; + tlsOptions = Object.freeze({ + key: readFileSync(path.resolve(__dirname, "server.key")), + cert: readFileSync(path.resolve(__dirname, "server.pem")), + }); + + constructor() { + this.requests = []; + const handler: RequestListener = (req, res) => { + this.requests.push(req); + res.writeHead(200); + res.end(`OK ${req.url ?? ""}`); + }; + this.httpServer = createHTTPServer(handler); + this.httpsServer = createHTTPSServer({ ...this.tlsOptions }, handler); + + const onconnect = (server: HTTPServer) => (req: IncomingMessage, socket: Duplex, head: Buffer) => { + const [username, pw] = parseHTTPAuthHeader(req.headers["proxy-authorization"]); + if (this.authHandler?.(username, pw) === false) { + socket.end("HTTP/1.0 407 Proxy Authentication Required\r\n\r\n"); + return; + } + if (req.url === "127.0.0.1:1") { + socket.end("HTTP/1.0 502 Bad Gateway\r\n\r\n"); + return; + } + socket.unshift(head); + server.emit("connection", socket); + socket.write("HTTP/1.0 200 OK\r\n\r\n"); + }; + + this.httpProxyServer = createHTTPServer((req, res) => { + const [username, pw] = parseHTTPAuthHeader(req.headers["proxy-authorization"]); + if (this.authHandler?.(username, pw) === false) { + res.writeHead(407); + res.end(); + return; + } + httpGet( + req.url!, + { + createConnection: () => { + const { socket1, socket2 } = new DuplexPair(); + this.httpServer.emit("connection", socket2); + return socket1; + }, + }, + (proxyRes) => proxyRes.pipe(res) + ); + }).on("connect", onconnect(this.httpServer)); + + this.httpsProxyServer = createHTTPServer(() => { + throw new Error("should not use normal req/res handler"); + }).on("connect", onconnect(this.httpsServer)); + } + + async listen(): Promise { + await Promise.all( + [this.httpServer, this.httpsServer, this.httpProxyServer, this.httpsProxyServer].map(async (server) => { + await promisify(server.listen.bind(server))(0); + server.on("connection", (conn) => this.connections.push(conn)); + }) + ); + } + + getRequestedUrls(): string[] { + return this.requests.map((r) => + Object.assign(new URL(`http://_`), { + pathname: r.url, + host: r.headers.host, + }).toString() + ); + } + + async teardown(): Promise { + for (const conn of this.connections) if (!conn.destroyed) conn.destroy?.(); + const closePromises: Promise[] = []; + for (const server of [this.httpServer, this.httpsServer, this.httpProxyServer, this.httpsProxyServer]) { + server.close(); + closePromises.push(once(server, "close")); + } + await Promise.all(closePromises); + } +} diff --git a/tests/integration/fixtures/server.key b/tests/integration/fixtures/server.key new file mode 100644 index 000000000..e6a734252 --- /dev/null +++ b/tests/integration/fixtures/server.key @@ -0,0 +1,28 @@ +-----BEGIN PRIVATE KEY----- +MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCqZ+7n/RUBp0JT +CJZpkctjRsz9SuNSD70+SrN9WMY8UWpC7u9A+yUFmZ9/lz4B8aZ5DVM4uHijlgQS +Asfxo5MaIqAVgeEYGk7rRZuh1FTCde8MTLob+HXoHCVzUdipy5uohTpk5NwvAiEJ +2mOyJrjG3DJsvE8QFyOinM2YcnxZrv01mpTnl3hEBWYJC+OAuU5xsxONHMenk441 +cmetT2v1ojUwxVlmOnMIZwlOLBbbBiTd8TuojCRtKJt51485jlNBPUWYY4E+PCqo +p8G7wM7DLJoAkNRviP3h/iwgG85Z+QBVaQ+Z1NjCBrhLfyPrl5Lwp4jH6+K3kxAT +LDNgYjg/AgMBAAECggEABHfmOVoXD2yJ3jCG9Sy4BxnnrSqmDFRSU4xU6ZAG3rUJ +0sh+KJeNUHjHSGq4WwexpFH3oTChJTT9VVWSVaFC7bgDt5yowN+Luzqfip5NPK4n +/wwSA0LAIL6AMuZuBoHKyp/3uIaRyX/GSwJZg+XlCX3jqptDfXoF2rE+6OTzosxb +Xj1IcfkFa9hScHOgGP8oKoqRR7kP3TRCkpqejJw1R2tHimG6rumPJnzhSRwiu4bC +VkzX4m5nVDL7X17HUiKYtxTVJSF0j3ChkGmc2o/I9jTOb/Ne4Lqgeitfx+H9bpiy +xcKbqk16yDnWcu/kfQ8JEKYq8w3zN9ltdtAZT46sgQKBgQDoaUMIeWRalnUWWOX5 +ipjdUML1fmVqBTvZV4O8Q4bc3HN7lZUf7oHPelfnwGdpwkw5mOE2s5EChf9nyOpY +MBd73oPCVhVGfBsn+V+iHOyU6F7V7BPWkdPm7W+kF38GI5ietMA/9VAbWvbXaFqX +u77lm8/JrpZ8q9Qu2P2jfKXZvwKBgQC7s5dHa6wX0E+xEmHdp0kC3QxyoKcwmz+I +WJQAERwSUcgEqV/iYBLnU/UYd+G/i1uRlS4VTWyuXRi6+ayvCvdZCPcAnsyZhFqC +3UlGjKJknAx1r15BaGUjWZQgBC107UnoJqxSrCsyrv7LsWlOmKQl0X5Z/d03TNKS +PSbfMPpBgQKBgQDJ/cpb0B1vOfrzfDoMQvAO0cVP1hXQKlJU2GHPOyU4SYU48M2V +3hYGO/+wlSGL4mmbWYrLnw82ET3kdtNt6AZRCxiay3RcOTrk6DC81cSsurTJ2g93 +2nA/8TapeB5XOJLJxLCeJdgEnm+Q0cqCu5LzPhM+5zU1j6WvPbpb39bJQwKBgEH8 +JX9fE7WfbpSCMNNaHqmaCek2HvBQc2o8MXNAkIzEITu6S1HqklquQihi5IKQvBUW +y4eDm2REqA/6+8DhawjqxOJ78NM7GxKMNllN0TzrOtoYV1tJFtzxfcgvj8deL7Ak +AEpj6h+8MyhqaunNcU82MWPzgdQR9qigRM0Li76BAoGAYdpnXvYFkZ2dxbRXADOg +iaxjcb6LP2pkYzdkbi+BsrgFd9Tkwi4K31e6MRxHcIrt/worRaf93E8KCmoo1cm/ +X9OXdhlrLB4lxIGAET6zDnab19AIMLzqCFvf7qIqKGGEgOzpGuMC+KgQ9JxF3LuJ +X8J4gcAAZd+S/GTNEJjnjDQ= +-----END PRIVATE KEY----- diff --git a/tests/integration/fixtures/server.pem b/tests/integration/fixtures/server.pem new file mode 100644 index 000000000..22002cf78 --- /dev/null +++ b/tests/integration/fixtures/server.pem @@ -0,0 +1,18 @@ +-----BEGIN CERTIFICATE----- +MIIC9TCCAd2gAwIBAgIUQbeKnE/uHZxOfdARDstiUBRUHjQwDQYJKoZIhvcNAQEL +BQAwEzERMA8GA1UEAwwIVkFMSEFMTEEwHhcNMjQwOTA3MjA1MDAzWhcNMzQwOTA1 +MjA1MDAzWjATMREwDwYDVQQDDAhWQUxIQUxMQTCCASIwDQYJKoZIhvcNAQEBBQAD +ggEPADCCAQoCggEBAKpn7uf9FQGnQlMIlmmRy2NGzP1K41IPvT5Ks31YxjxRakLu +70D7JQWZn3+XPgHxpnkNUzi4eKOWBBICx/GjkxoioBWB4RgaTutFm6HUVMJ17wxM +uhv4degcJXNR2KnLm6iFOmTk3C8CIQnaY7ImuMbcMmy8TxAXI6KczZhyfFmu/TWa +lOeXeEQFZgkL44C5TnGzE40cx6eTjjVyZ61Pa/WiNTDFWWY6cwhnCU4sFtsGJN3x +O6iMJG0om3nXjzmOU0E9RZhjgT48KqinwbvAzsMsmgCQ1G+I/eH+LCAbzln5AFVp +D5nU2MIGuEt/I+uXkvCniMfr4reTEBMsM2BiOD8CAwEAAaNBMD8wCQYDVR0TBAIw +ADATBgNVHREEDDAKgghWQUxIQUxMQTAdBgNVHQ4EFgQUD09yi8JCbltXSxd8TO2q +sXhpP/kwDQYJKoZIhvcNAQELBQADggEBACNk0Te9CQK99Zpeqt3nD8fuwPBZmlmL +fzlFi6octO/NIEW+qXB/ZPhirb3J7uwwHt9sDz19aRxWXOei8Alnr7h3gLuXqZJi +VGrORnWRZYr0IlHEbPgKOxL/u3vKkYvXHIGm2juCsrz1m7E3ltRyjlskPT73k6s4 +qkEASM3WkDw9G8aRW3V8n7Da8ADgIgmjFyRQlBIBylvcDsVXq4FoOh5O/pExFPlT +t2a32iodhOcHD0fDO1TP1771o/KIFMiwk2B7alr7t0EsbKk7RqaFHMRjlw4tGVtE +is5slmN7TfmpG8ocqFOMPfpW4fOaz9ezaAHBlkeprl85luprpNWttg0= +-----END CERTIFICATE----- diff --git a/tests/unit/common/apiClient.test.ts b/tests/unit/common/apiClient.test.ts index 4b6afc11f..0c93f2191 100644 --- a/tests/unit/common/apiClient.test.ts +++ b/tests/unit/common/apiClient.test.ts @@ -1,7 +1,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { ApiClient } from "../../../src/common/atlas/apiClient.js"; import { CommonProperties, TelemetryEvent, TelemetryResult } from "../../../src/telemetry/types.js"; -import httpProxy from "http-proxy"; describe("ApiClient", () => { let apiClient: ApiClient; @@ -95,44 +94,6 @@ describe("ApiClient", () => { }); }); - describe("oauth authentication proxy", () => { - let apiClient: ApiClient; - let proxyServer: httpProxy; - let requests: unknown[]; - - beforeEach(() => { - process.env.HTTP_PROXY = "localhost:8888"; - apiClient = new ApiClient({ - baseUrl: "https://httpbin.org", - credentials: { - clientId: "test-client-id", - clientSecret: "test-client-secret", - }, - userAgent: "test-user-agent", - }); - - proxyServer = httpProxy.createProxyServer({ target: "https://api.test.com" }); - proxyServer.on("proxyReq", (proxyReq, req, res, options) => { - requests.push(req); - }); - - requests = []; - }); - - afterEach(async () => { - delete process.env.HTTP_PROXY; - - await proxyServer.close(); - //await apiClient.close(); - }); - - it("should send the oauth request through a proxy if configured", async () => { - await apiClient.validateAccessToken(); - console.log(requests); - expect(true).toBeFalsy(); - }); - }); - describe("sendEvents", () => { it("should send events to authenticated endpoint when token is available and valid", async () => { const mockFetch = vi.spyOn(global, "fetch"); From b966ca6cdce086ac3b19a37df470c031e99ecb80 Mon Sep 17 00:00:00 2001 From: Kevin Mas Ruiz Date: Tue, 29 Jul 2025 19:10:41 +0200 Subject: [PATCH 04/10] chore: Add scripts to start stdio and http transports --- package.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 6a28c71b1..7bfab0f90 100644 --- a/package.json +++ b/package.json @@ -16,7 +16,8 @@ }, "type": "module", "scripts": { - "start": "node dist/index.js --transport stdio --loggers stderr mcp", + "start": "node dist/index.js --transport http --loggers stderr mcp", + "start:stdio": "node dist/index.js --transport stdio --loggers stderr mcp", "prepare": "npm run build", "build:clean": "rm -rf dist", "build:compile": "tsc --project tsconfig.build.json", From 5e81707398c6d2bd7a5364156deb67a38204d75a Mon Sep 17 00:00:00 2001 From: Kevin Mas Ruiz Date: Wed, 30 Jul 2025 11:52:22 +0200 Subject: [PATCH 05/10] chore: Use node-fetch request as a custom request node-fetch requires an internal symbol to identify if a request is an object or a string. Because openapi-fetch does not provide this symbol, node-fetch misunderstands the request. --- package-lock.json | 1 + package.json | 1 + src/common/atlas/apiClient.ts | 21 +++++++++--- tests/integration/common/apiClient.test.ts | 37 +++++++++++++++------- 4 files changed, 44 insertions(+), 16 deletions(-) diff --git a/package-lock.json b/package-lock.json index 8e00371ac..992e8dad2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -23,6 +23,7 @@ "mongodb-log-writer": "^2.4.1", "mongodb-redact": "^1.1.8", "mongodb-schema": "^12.6.2", + "node-fetch": "^3.3.2", "node-machine-id": "1.1.12", "oauth4webapi": "^3.6.0", "openapi-fetch": "^0.14.0", diff --git a/package.json b/package.json index 7bfab0f90..9f8d08478 100644 --- a/package.json +++ b/package.json @@ -85,6 +85,7 @@ "mongodb-log-writer": "^2.4.1", "mongodb-redact": "^1.1.8", "mongodb-schema": "^12.6.2", + "node-fetch": "^3.3.2", "node-machine-id": "1.1.12", "oauth4webapi": "^3.6.0", "openapi-fetch": "^0.14.0", diff --git a/src/common/atlas/apiClient.ts b/src/common/atlas/apiClient.ts index 1151e9f45..d71c48e01 100644 --- a/src/common/atlas/apiClient.ts +++ b/src/common/atlas/apiClient.ts @@ -7,6 +7,7 @@ import { packageInfo } from "../packageInfo.js"; import logger, { LogId } from "../logger.js"; import { createFetch } from "@mongodb-js/devtools-proxy-support"; import * as oauth from "oauth4webapi"; +import { Request } from "node-fetch"; const ATLAS_API_VERSION = "2025-03-12"; @@ -36,7 +37,7 @@ export class ApiClient { }; }; - private static customFetch = createFetch({ + private static customFetch: typeof fetch = createFetch({ useEnvironmentVariableProxies: true, }) as unknown as typeof fetch; @@ -53,7 +54,8 @@ export class ApiClient { private isAccessTokenValid(): boolean { return !!( this.accessToken && - (this.accessToken.expires_at == undefined || this.accessToken.expires_at > Date.now() / 1000) + this.accessToken.expires_at != undefined && + this.accessToken.expires_at > Date.now() ); } @@ -102,6 +104,7 @@ export class ApiClient { Accept: `application/vnd.atlas.${ATLAS_API_VERSION}+json`, }, fetch: ApiClient.customFetch, + Request: Request, }); if (this.options.credentials?.clientId && this.options.credentials?.clientSecret) { @@ -128,7 +131,7 @@ export class ApiClient { const clientId = this.options.credentials.clientId; // We are using our own ClientAuth because ClientSecretBasic URL encodes wrongly - // the username and password (for example, encodes `_` which is wrong). + // the username and password (for example, encodes `_` to %5F, which is wrong). return { client: { client_id: clientId }, clientAuth: (_as, client, _body, headers) => { @@ -165,7 +168,7 @@ export class ApiClient { const result = await oauth.processClientCredentialsResponse(this.oauth2Issuer, client, response); this.accessToken = { access_token: result.access_token, - expires_at: Date.now() / 1000 + (result.expires_in ?? 0), + expires_at: Date.now() + (result.expires_in ?? 0) * 1000, }; } catch (error: unknown) { const err = error instanceof Error ? error : new Error(String(error)); @@ -465,10 +468,18 @@ export class ApiClient { } async listOrganizations(options?: FetchOptions) { - const { data, error, response } = await this.client.GET("/api/atlas/v2/orgs", options); + console.log(">> listOrg1 "); + try { + const { data, error, response } = await this.client.GET("/api/atlas/v2/orgs", options); + } catch (ex) { + console.error(ex); + } + console.log(">> listOrg2 "); if (error) { + console.log(">> listOrg3 "); throw ApiClientError.fromError(response, error); } + console.log(">> listOrg4 "); return data; } diff --git a/tests/integration/common/apiClient.test.ts b/tests/integration/common/apiClient.test.ts index 49ea03714..71a9346ea 100644 --- a/tests/integration/common/apiClient.test.ts +++ b/tests/integration/common/apiClient.test.ts @@ -4,7 +4,7 @@ import { ApiClient } from "../../../src/common/atlas/apiClient.js"; import { HTTPServerProxyTestSetup } from "../fixtures/httpsServerProxyTest.js"; describe("ApiClient integration test", () => { - describe("oauth authentication proxy", () => { + describe(`atlas API proxy integration`, () => { let apiClient: ApiClient; let proxyTestSetup: HTTPServerProxyTestSetup; @@ -26,24 +26,36 @@ describe("ApiClient integration test", () => { function withToken(accessToken: string, expired: boolean) { const apiClientMut = apiClient as unknown as { accessToken: AccessToken }; - const expireAt = expired ? Date.now() - 100000 : Date.now() + 10000; + const diff = 10_000; + const now = Date.now(); apiClientMut.accessToken = { access_token: accessToken, - expires_at: expireAt, + expires_at: expired ? now - diff : now + diff, }; } + async function ignoringResult(fn: () => Promise): Promise { + try { + await fn(); + } catch (error: unknown) { + // we are ignoring the error because we know that + // the type safe client will fail. It will fail + // because we are returning an empty 200, and it expects + // a specific format not relevant for these tests. + } + } + afterEach(async () => { delete process.env.NODE_TLS_REJECT_UNAUTHORIZED; delete process.env.HTTP_PROXY; - await apiClient.close(); + await ignoringResult(() => apiClient.close()); await proxyTestSetup.teardown(); }); it("should send the oauth request through a proxy if configured", async () => { - await apiClient.validateAccessToken(); + await ignoringResult(() => apiClient.validateAccessToken()); expect(proxyTestSetup.getRequestedUrls()).toEqual([ `http://localhost:${proxyTestSetup.httpsServerPort}/api/oauth/token`, ]); @@ -51,23 +63,26 @@ describe("ApiClient integration test", () => { it("should send the oauth revoke request through a proxy if configured", async () => { withToken("my non expired token", false); - await apiClient.close(); + await ignoringResult(() => apiClient.close()); expect(proxyTestSetup.getRequestedUrls()).toEqual([ `http://localhost:${proxyTestSetup.httpsServerPort}/api/oauth/revoke`, ]); }); it("should make an atlas call when the token is not expired", async () => { - withToken("my not expired", false); - await apiClient.listOrganizations(); + withToken("my non expired token", false); + await ignoringResult(() => apiClient.listOrganizations()); expect(proxyTestSetup.getRequestedUrls()).toEqual([ `http://localhost:${proxyTestSetup.httpsServerPort}/api/atlas/v2/orgs`, ]); }); - it("should request a new token and an atlas call when the token is expired", async () => { - withToken("my expired", true); - await apiClient.listOrganizations(); + it("should request a new token and make an atlas call when the token is expired", async () => { + withToken("my expired token", true); + await ignoringResult(() => apiClient.validateAccessToken()); + withToken("my non expired token", false); + await ignoringResult(() => apiClient.listOrganizations()); + expect(proxyTestSetup.getRequestedUrls()).toEqual([ `http://localhost:${proxyTestSetup.httpsServerPort}/api/oauth/token`, `http://localhost:${proxyTestSetup.httpsServerPort}/api/atlas/v2/orgs`, From 66839d7a8ccba502152606c9c00910cd06a256ec Mon Sep 17 00:00:00 2001 From: Kevin Mas Ruiz Date: Wed, 30 Jul 2025 11:57:36 +0200 Subject: [PATCH 06/10] chore: Fix compilation errors and make more explicit required types --- src/common/atlas/apiClient.ts | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/src/common/atlas/apiClient.ts b/src/common/atlas/apiClient.ts index d71c48e01..15d239098 100644 --- a/src/common/atlas/apiClient.ts +++ b/src/common/atlas/apiClient.ts @@ -1,5 +1,5 @@ import createClient, { Client, Middleware } from "openapi-fetch"; -import type { FetchOptions } from "openapi-fetch"; +import type { ClientOptions, FetchOptions } from "openapi-fetch"; import { ApiClientError } from "./apiClientError.js"; import { paths, operations } from "./openapi.js"; import { CommonProperties, TelemetryEvent } from "../../telemetry/types.js"; @@ -7,7 +7,7 @@ import { packageInfo } from "../packageInfo.js"; import logger, { LogId } from "../logger.js"; import { createFetch } from "@mongodb-js/devtools-proxy-support"; import * as oauth from "oauth4webapi"; -import { Request } from "node-fetch"; +import { Request as NodeFetchRequest } from "node-fetch"; const ATLAS_API_VERSION = "2025-03-12"; @@ -104,7 +104,7 @@ export class ApiClient { Accept: `application/vnd.atlas.${ATLAS_API_VERSION}+json`, }, fetch: ApiClient.customFetch, - Request: Request, + Request: NodeFetchRequest as unknown as ClientOptions["Request"], }); if (this.options.credentials?.clientId && this.options.credentials?.clientSecret) { @@ -468,18 +468,10 @@ export class ApiClient { } async listOrganizations(options?: FetchOptions) { - console.log(">> listOrg1 "); - try { - const { data, error, response } = await this.client.GET("/api/atlas/v2/orgs", options); - } catch (ex) { - console.error(ex); - } - console.log(">> listOrg2 "); + const { data, error, response } = await this.client.GET("/api/atlas/v2/orgs", options); if (error) { - console.log(">> listOrg3 "); throw ApiClientError.fromError(response, error); } - console.log(">> listOrg4 "); return data; } From b8623a307e7ed25c3d6492abf6b5e3c06a181d5c Mon Sep 17 00:00:00 2001 From: Kevin Mas Ruiz Date: Wed, 30 Jul 2025 12:01:37 +0200 Subject: [PATCH 07/10] chore: If we can't split, just assume empty, it's for testing purposes --- tests/integration/fixtures/httpsServerProxyTest.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/integration/fixtures/httpsServerProxyTest.ts b/tests/integration/fixtures/httpsServerProxyTest.ts index 11fb326ee..39859e025 100644 --- a/tests/integration/fixtures/httpsServerProxyTest.ts +++ b/tests/integration/fixtures/httpsServerProxyTest.ts @@ -12,7 +12,9 @@ import type { Duplex } from "stream"; function parseHTTPAuthHeader(header: string | undefined): [string, string] { if (!header?.startsWith("Basic ")) return ["", ""]; - const [username = "", pw = ""] = Buffer.from(header.split(" ")[1], "base64").toString().split(":"); + const [username = "", pw = ""] = Buffer.from(header.split(" ")[1] ?? "", "base64") + .toString() + .split(":"); return [username, pw]; } From 44a1308ef52da357cca96ea2679b49e9e925b197 Mon Sep 17 00:00:00 2001 From: Kevin Mas Ruiz Date: Wed, 30 Jul 2025 12:08:22 +0200 Subject: [PATCH 08/10] chore: fix promisify, it seems the contract is different --- tests/integration/fixtures/httpsServerProxyTest.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/fixtures/httpsServerProxyTest.ts b/tests/integration/fixtures/httpsServerProxyTest.ts index 39859e025..7e4e67668 100644 --- a/tests/integration/fixtures/httpsServerProxyTest.ts +++ b/tests/integration/fixtures/httpsServerProxyTest.ts @@ -101,7 +101,7 @@ export class HTTPServerProxyTestSetup { async listen(): Promise { await Promise.all( [this.httpServer, this.httpsServer, this.httpProxyServer, this.httpsProxyServer].map(async (server) => { - await promisify(server.listen.bind(server))(0); + await promisify(server.listen.bind(server, 0))(); server.on("connection", (conn) => this.connections.push(conn)); }) ); From 56ad2dd55612f90b5331288a4c0c6ca5dc7918a2 Mon Sep 17 00:00:00 2001 From: Kevin Mas Ruiz Date: Wed, 30 Jul 2025 12:20:57 +0200 Subject: [PATCH 09/10] chore: Fix linter warnings --- tests/integration/common/apiClient.test.ts | 3 ++- tests/integration/fixtures/httpsServerProxyTest.ts | 11 ++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/integration/common/apiClient.test.ts b/tests/integration/common/apiClient.test.ts index 71a9346ea..54bb040dc 100644 --- a/tests/integration/common/apiClient.test.ts +++ b/tests/integration/common/apiClient.test.ts @@ -38,7 +38,8 @@ describe("ApiClient integration test", () => { async function ignoringResult(fn: () => Promise): Promise { try { await fn(); - } catch (error: unknown) { + } catch (_error: unknown) { + void _error; // we are ignoring the error because we know that // the type safe client will fail. It will fail // because we are returning an empty 200, and it expects diff --git a/tests/integration/fixtures/httpsServerProxyTest.ts b/tests/integration/fixtures/httpsServerProxyTest.ts index 7e4e67668..d1d53f8bc 100644 --- a/tests/integration/fixtures/httpsServerProxyTest.ts +++ b/tests/integration/fixtures/httpsServerProxyTest.ts @@ -81,12 +81,13 @@ export class HTTPServerProxyTestSetup { return; } httpGet( - req.url!, + req.url ?? "", { createConnection: () => { - const { socket1, socket2 } = new DuplexPair(); - this.httpServer.emit("connection", socket2); - return socket1; + const sockets = new DuplexPair(); + this.httpServer.emit("connection", sockets.socket2); + // eslint-disable-next-line @typescript-eslint/no-unsafe-return + return sockets.socket1; }, }, (proxyRes) => proxyRes.pipe(res) @@ -102,7 +103,7 @@ export class HTTPServerProxyTestSetup { await Promise.all( [this.httpServer, this.httpsServer, this.httpProxyServer, this.httpsProxyServer].map(async (server) => { await promisify(server.listen.bind(server, 0))(); - server.on("connection", (conn) => this.connections.push(conn)); + server.on("connection", (conn: Duplex) => this.connections.push(conn)); }) ); } From 0ce7935f6b666716b476b3f49fd468bb142c605e Mon Sep 17 00:00:00 2001 From: Kevin Mas Ruiz Date: Wed, 30 Jul 2025 17:04:01 +0200 Subject: [PATCH 10/10] chore: Add clarifying comments on castings --- src/common/atlas/apiClient.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/common/atlas/apiClient.ts b/src/common/atlas/apiClient.ts index 15d239098..66f9ada19 100644 --- a/src/common/atlas/apiClient.ts +++ b/src/common/atlas/apiClient.ts @@ -37,6 +37,10 @@ export class ApiClient { }; }; + // createFetch assumes that the first parameter of fetch is always a string + // with the URL. However, fetch can also receive a Request object. While + // the typechecking complains, createFetch does passthrough the parameters + // so it works fine. private static customFetch: typeof fetch = createFetch({ useEnvironmentVariableProxies: true, }) as unknown as typeof fetch; @@ -104,6 +108,9 @@ export class ApiClient { Accept: `application/vnd.atlas.${ATLAS_API_VERSION}+json`, }, fetch: ApiClient.customFetch, + // NodeFetchRequest has more overloadings than the native Request + // so it complains here. However, the interfaces are actually compatible + // so it's not a real problem, just a type checking problem. Request: NodeFetchRequest as unknown as ClientOptions["Request"], });