From cb07f5171218f698cfdecde4d731761f4a847a7f Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Wed, 13 Aug 2025 20:56:32 +0200 Subject: [PATCH 1/2] fix: improve error message for generic openid-client error MONGOSH-2487 (#230) * fix: improve error message for generic openid-client error MONGOSH-2487 Similar to previous commits, this change ensures that diagnostic information is not lost through recent changes to openid-client's error system, where currently a number of errors are grouped together under "something went wrong" (https://github.com/panva/openid-client/blob/a1e4a4fd88fcdff12d88e3ff73f3e27fe5df4252/src/index.ts#L975) and the library expects consumers to consistently expose the `cause` property of the error object. This also unifies the two existing `messageFromError` and `errorString` implementations, which happened to duplicate each other. * fixup: electron compat * fixup: do not add "[object Response]" to errors * fixup: add anchors to regex --- src/plugin.spec.ts | 76 ++++++++++++++++++++++++++++++++++++++- src/plugin.ts | 5 ++- src/util.ts | 35 ++++++++++-------- test/self-signed-cert.pem | 21 +++++++++++ test/self-signed-key.pem | 27 ++++++++++++++ 5 files changed, 145 insertions(+), 19 deletions(-) create mode 100644 test/self-signed-cert.pem create mode 100644 test/self-signed-key.pem diff --git a/src/plugin.spec.ts b/src/plugin.spec.ts index 1392b5a..e7097e4 100644 --- a/src/plugin.spec.ts +++ b/src/plugin.spec.ts @@ -40,6 +40,8 @@ import type { TokenMetadata, } from '@mongodb-js/oidc-mock-provider'; import { OIDCMockProvider } from '@mongodb-js/oidc-mock-provider'; +import type { Server as HTTPSServer } from 'https'; +import { createServer as createHTTPSServer } from 'https'; // node-fetch@3 is ESM-only... // eslint-disable-next-line @typescript-eslint/consistent-type-imports @@ -1417,6 +1419,36 @@ describe('OIDC plugin (mock OIDC provider)', function () { }); context('HTTP request handling', function () { + let badHTTPSServer: HTTPSServer; + + before(async function () { + badHTTPSServer = createHTTPSServer( + { + key: await fs.readFile( + // https://github.com/nodejs/node/blob/becb55aac3f7eb93b03223744c35c6194f11e3e9/test/fixtures/keys/agent2-key.pem + path.resolve(__dirname, '..', 'test', 'self-signed-key.pem') + ), + cert: await fs.readFile( + // https://github.com/nodejs/node/blob/becb55aac3f7eb93b03223744c35c6194f11e3e9/test/fixtures/keys/agent2-cert.pem + path.resolve(__dirname, '..', 'test', 'self-signed-cert.pem') + ), + }, + (req, res) => { + res.writeHead(200, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify({ message: 'Hello World' })); + } + ); + badHTTPSServer.listen(0, 'localhost'); + await once(badHTTPSServer, 'listening'); + }); + + after(async function () { + if (badHTTPSServer) { + badHTTPSServer.close(); + await once(badHTTPSServer, 'close'); + } + }); + it('will track all outgoing HTTP requests', async function () { const pluginHttpRequests: string[] = []; const localServerHttpRequests: string[] = []; @@ -1555,7 +1587,7 @@ describe('OIDC plugin (mock OIDC provider)', function () { expect(customFetch).to.have.been.called; }); - it('logs helpful error messages', async function () { + it('logs helpful error messages for HTTP failures', async function () { const outboundHTTPRequestsCompleted: any[] = []; getTokenPayload = () => Promise.reject(new Error('test failure')); @@ -1595,6 +1627,48 @@ describe('OIDC plugin (mock OIDC provider)', function () { }); }); + it('logs helpful error messages for TLS failures', async function () { + const outboundHTTPRequestsFailed: any[] = []; + const port = (badHTTPSServer.address() as AddressInfo).port; + + const plugin = createMongoDBOIDCPlugin({ + openBrowserTimeout: 60_000, + openBrowser: fetchBrowser, + allowedFlows: ['auth-code'], + redirectURI: 'http://localhost:0/callback', + }); + + plugin.logger.on( + 'mongodb-oidc-plugin:outbound-http-request-failed', + (ev) => outboundHTTPRequestsFailed.push(ev) + ); + + let selfSignedReason = 'self-signed certificate'; + try { + await requestToken(plugin, { + issuer: `https://localhost:${port}/`, + clientId: 'mockclientid', + requestScopes: [], + }); + expect.fail('missed exception'); + } catch (err: any) { + // Electron and Node.js provide different error messages + selfSignedReason = err.message.includes('self-signed certificate') + ? 'self-signed certificate' + : 'self signed certificate'; + expect(err.message).to.include( + `Unable to fetch issuer metadata for "https://localhost:${port}/": something went wrong (caused by: request to https://localhost:${port}/.well-known/openid-configuration failed, reason: ${selfSignedReason}` + ); + } + const entry = outboundHTTPRequestsFailed.find( + (ev) => + ev.url === + `https://localhost:${port}/.well-known/openid-configuration` + ); + expect(entry).to.exist; + expect(entry.error).to.include(selfSignedReason); + }); + it('handles JSON failure responses from the IDP', async function () { overrideRequestHandler = (url, req, res) => { if (new URL(url).pathname.endsWith('/token')) { diff --git a/src/plugin.ts b/src/plugin.ts index 533532d..54246db 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -11,7 +11,6 @@ import { errorString, getRefreshTokenId, improveHTTPResponseBasedError, - messageFromError, nodeFetchCompat, normalizeObject, throwIfAborted, @@ -525,7 +524,7 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin { throw new MongoDBOIDCError( `Unable to fetch issuer metadata for ${JSON.stringify( serverMetadata.issuer - )}: ${messageFromError(err)}`, + )}: ${errorString(err)}`, { cause: err, codeName: 'IssuerMetadataDiscoveryFailed', @@ -844,7 +843,7 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin { browserHandle?.once('error', (err) => reject( new MongoDBOIDCError( - `Opening browser failed with '${messageFromError( + `Opening browser failed with '${errorString( err )}'${extraErrorInfo()}`, { cause: err, codeName: 'BrowserOpenFailedSpawnError' } diff --git a/src/util.ts b/src/util.ts index 5f5986a..1dd313c 100644 --- a/src/util.ts +++ b/src/util.ts @@ -47,12 +47,6 @@ export async function withAbortCheck< } } -export function errorString(err: unknown): string { - return String( - typeof err === 'object' && err && 'message' in err ? err.message : err - ); -} - // AbortSignal.timeout, but consistently .unref()ed export function timeoutSignal(ms: number): AbortSignal { const controller = new AbortController(); @@ -127,15 +121,26 @@ export function validateSecureHTTPUrl( } } -export function messageFromError(err: unknown): string { - return String( - err && - typeof err === 'object' && - 'message' in err && - typeof err.message === 'string' - ? err.message - : err - ); +export function errorString(err: unknown): string { + if ( + !err || + typeof err !== 'object' || + !('message' in err) || + typeof err.message !== 'string' + ) { + return String(err); + } + const cause = getCause(err); + let { message } = err; + if (cause) { + const causeMessage = errorString(cause); + if ( + !message.includes(causeMessage) && + !causeMessage.match(/^\[object.+\]$/i) + ) + message += ` (caused by: ${causeMessage})`; + } + return message; } const salt = randomBytes(16); diff --git a/test/self-signed-cert.pem b/test/self-signed-cert.pem new file mode 100644 index 0000000..7a43e1a --- /dev/null +++ b/test/self-signed-cert.pem @@ -0,0 +1,21 @@ +-----BEGIN CERTIFICATE----- +MIIDgzCCAmsCFF3cqhUsBYLNh3bCVatENxZcoY7rMA0GCSqGSIb3DQEBCwUAMH0x +CzAJBgNVBAYTAlVTMQswCQYDVQQIDAJDQTELMAkGA1UEBwwCU0YxDzANBgNVBAoM +BkpveWVudDEQMA4GA1UECwwHTm9kZS5qczEPMA0GA1UEAwwGYWdlbnQyMSAwHgYJ +KoZIhvcNAQkBFhFyeUB0aW55Y2xvdWRzLm9yZzAgFw0yMjA5MDMxNDQ2NTFaGA8y +Mjk2MDYxNzE0NDY1MVowfTELMAkGA1UEBhMCVVMxCzAJBgNVBAgMAkNBMQswCQYD +VQQHDAJTRjEPMA0GA1UECgwGSm95ZW50MRAwDgYDVQQLDAdOb2RlLmpzMQ8wDQYD +VQQDDAZhZ2VudDIxIDAeBgkqhkiG9w0BCQEWEXJ5QHRpbnljbG91ZHMub3JnMIIB +IjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAvqt/4KDehQLLDH+I2KXOxg4G +WfNBISWmKlExPBfz9i1LY/rwoRwryv3Lpr40M05Dx+Rt4LMC+If7NGvrV8hdNSOz +jW7P7R6upVdNXpeZDmHvhq+G/xv+x/Hdv3+Sdm/JC8TD2HRYcHSSWsirRbfA9eJe +L0ADh1mJGNpWS+9FNXtbR3LRWsRwNjP1Lb39tXIsfHiWrJ/F6yAhWOU+ZZvvjazp +bZX7Kes0lxVtyWCzLFpnzYa/gajGLdGJwTrfKXsSz2wk6szKlbO0mzX0aHviPRPT +ftUVs91qORJ8tkAU4u78bpV0eCM8tVJh/N/oSm7ysLUjxhJrfNxHmmkGyaRL/QID +AQABMA0GCSqGSIb3DQEBCwUAA4IBAQA+94z0pI0JEU1dX4bHGkhP6hwmv5tu7KlA +R0hK33pF+boiagbySHrXW/y119VLp+o1FjuOlS4ETgAjcIjN2dDmJc0JEj6jnXyc +4IYhRMDg4INAnmXX9bdCmpYuvhw/73cuxkdkMxH8p4O7v5HSqfpwjTEX8tWtpeMI +IZ4+H/ddOKyvF3SO8lfrYJ7TXyypWfxzEiBuJnhZgpMG7zpZMGIzTkcN9VFTCv8d +DCW0Lr2Ix/GY7nf/R9zDFnEZTW6IIkRp9UsUdOrgqgfSxp/C48foFv7gqMO/9PD8 +E8uE8986AFd5cK67imYPspHXv5UycySifwsSixi0hI9lDZqUIoWH +-----END CERTIFICATE----- diff --git a/test/self-signed-key.pem b/test/self-signed-key.pem new file mode 100644 index 0000000..0ef31aa --- /dev/null +++ b/test/self-signed-key.pem @@ -0,0 +1,27 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEpAIBAAKCAQEAvqt/4KDehQLLDH+I2KXOxg4GWfNBISWmKlExPBfz9i1LY/rw +oRwryv3Lpr40M05Dx+Rt4LMC+If7NGvrV8hdNSOzjW7P7R6upVdNXpeZDmHvhq+G +/xv+x/Hdv3+Sdm/JC8TD2HRYcHSSWsirRbfA9eJeL0ADh1mJGNpWS+9FNXtbR3LR +WsRwNjP1Lb39tXIsfHiWrJ/F6yAhWOU+ZZvvjazpbZX7Kes0lxVtyWCzLFpnzYa/ +gajGLdGJwTrfKXsSz2wk6szKlbO0mzX0aHviPRPTftUVs91qORJ8tkAU4u78bpV0 +eCM8tVJh/N/oSm7ysLUjxhJrfNxHmmkGyaRL/QIDAQABAoIBAQCPnrT7KZGTVTBH +IMWekv52ltfX53BWnHpWg8P3RP+hniqci8e3Q3YFODivR7QgNUK/DeRqDc0eEad5 +rBSgka8LuPGlhiOes67PojwIFV7Xw5Ndu1ePT7IRP7FNbrWO+tLQR41RvQlk45ne +Qison6n8TF+vbaN6z0mCa+v21KsoBYxQHM7IJ6pgMxg24aNW0WTk7PXdJp1LWRiJ +uixlXjOcKWQXaP+HxiQuXGj7isvv0O6xH2cl3GfgQ5rx8mG/APvLIz+dc/QBGQAr +v6IVlDtd3AiYS7YeB7/5OvY+0emJ7U15ZJLNnCzlrqNDjxCN+cbXAeTdlKRJp21F +rpjiZdfhAoGBAPw7EbWMq9ooluQ0bs+v6tvNCvXBfd/VAvG3w1/z3MvhVVYLx1Ag +zleZom3YUXRv24WW3qHXFEGgyz2Sd3mJ4AuR9TDhvij6rHO6E0C8shB0oBlLoNo0 +4Ve28VQfaI77AKd7BYIoCWsCA5oTV+34AYlApMYkkaplRwSs9X5wIZ8ZAoGBAMGE +7I1ASqMnQqdjzpBpGom+XpSPXGdBiH9mNPUajb0sPFvnnhpTSa3/k9m036Q9vQNH +PEOeyjFbF7c/QKsPZLUbFl4uXdEmN4BUab5qQMSQVB9SlQOUB5G/qk/M90TgSbBm +hFrpJrlf0Vsgnm1EMMOhoGdXbkB147AFnOcIekSFAoGAa31c0arOPd1YWI5Dvvxw +MRWTmyHHW9EyPQKcH1MUgEpaDJ5eZTZl2Q0fHIK4S8+zlJ2z6PJ4rnMwyd+WTNRG +B4g/HoLFgD87qOHefJMtqzeYVs9VEEjC05eiBsCP1YcAQ194/HvFb7XfBRVDPqWX +Of+zeMFy1lPszQBMaoKswVkCgYAElrRNPSMH71xjP7icMAHTFlKDz0pvoFwuOSw0 +S6bkv3HG9B0JnsP2fkLxPJq4+EXNGBlTuSYuOWy8iaFs7PaEXNoQ7aSH2xIh1t6T +B0312z5DZ9/kr9PmHtdZAREz7uWQaz3kMfcbGiyKrqFTEfTeDq0RBj+1A5aci+WG +jOrpSQKBgQCvf/R/le3m8EsMe5AmNMl1mvpZ5wWn0yVS0vnjJRbS4TCUGK1lSf74 +tIZ+PEMp9CRaS2eTGtsWwQvuORlWlgYuYvJfxwvvnbLjln66SuE7pZHG2UILw4vZ +5xkxTmL93VXFWRaH418mifGDiLIYr14+yzbW366r9L72BuN1dZJNzg== +-----END RSA PRIVATE KEY----- From 24f095e822d45cab75c74f237bc023a839eb9ae3 Mon Sep 17 00:00:00 2001 From: "mongodb-devtools-bot[bot]" <189715634+mongodb-devtools-bot[bot]@users.noreply.github.com> Date: Wed, 13 Aug 2025 19:52:00 +0000 Subject: [PATCH 2/2] chore: bump version to v2.0.3 (#231) Co-authored-by: addaleax <899444+addaleax@users.noreply.github.com> --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 1cc04d0..fce1409 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@mongodb-js/oidc-plugin", - "version": "2.0.2", + "version": "2.0.3", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@mongodb-js/oidc-plugin", - "version": "2.0.2", + "version": "2.0.3", "license": "Apache-2.0", "dependencies": { "express": "^5.1.0", diff --git a/package.json b/package.json index 1ad8258..8791fe4 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "email": "compass@mongodb.com" }, "homepage": "https://github.com/mongodb-js/oidc-plugin", - "version": "2.0.2", + "version": "2.0.3", "repository": { "type": "git", "url": "https://github.com/mongodb-js/oidc-plugin.git"