diff --git a/src/plugin.spec.ts b/src/plugin.spec.ts index cbf9eda..1750d1f 100644 --- a/src/plugin.spec.ts +++ b/src/plugin.spec.ts @@ -28,7 +28,11 @@ import { verifySuccessfulAuthCodeFlowLog } from '../test/log-hook-verification-h import { automaticRefreshTimeoutMS, MongoDBOIDCPluginImpl } from './plugin'; import sinon from 'sinon'; import { publicPluginToInternalPluginMap_DoNotUseOutsideOfTests } from './api'; -import type { Server as HTTPServer } from 'http'; +import type { + Server as HTTPServer, + ServerResponse, + IncomingMessage, +} from 'http'; import { createServer as createHTTPServer } from 'http'; import type { AddressInfo } from 'net'; import type { @@ -1245,6 +1249,7 @@ describe('OIDC plugin (local OIDC provider)', function () { describe('OIDC plugin (mock OIDC provider)', function () { let provider: OIDCMockProvider; let getTokenPayload: OIDCMockProviderConfig['getTokenPayload']; + let overrideRequestHandler: OIDCMockProviderConfig['overrideRequestHandler']; let additionalIssuerMetadata: OIDCMockProviderConfig['additionalIssuerMetadata']; let receivedHttpRequests: string[] = []; const tokenPayload = { @@ -1270,8 +1275,13 @@ describe('OIDC plugin (mock OIDC provider)', function () { additionalIssuerMetadata() { return additionalIssuerMetadata?.() ?? {}; }, - overrideRequestHandler(url: string) { + overrideRequestHandler( + url: string, + req: IncomingMessage, + res: ServerResponse + ) { receivedHttpRequests.push(url); + return overrideRequestHandler?.(url, req, res); }, }); }); @@ -1283,6 +1293,7 @@ describe('OIDC plugin (mock OIDC provider)', function () { beforeEach(function () { receivedHttpRequests = []; getTokenPayload = () => tokenPayload; + overrideRequestHandler = undefined; additionalIssuerMetadata = undefined; }); @@ -1538,5 +1549,39 @@ describe('OIDC plugin (mock OIDC provider)', function () { statusText: 'Internal Server Error', }); }); + + it('handles JSON failure responses from the IDP', async function () { + overrideRequestHandler = (url, req, res) => { + if (new URL(url).pathname.endsWith('/token')) { + res.writeHead(400, { 'Content-Type': 'application/json' }); + res.end( + JSON.stringify({ + error: 'jsonfailure', + error_description: 'failure text', + }) + ); + } + }; + const plugin = createMongoDBOIDCPlugin({ + openBrowserTimeout: 60_000, + openBrowser: fetchBrowser, + allowedFlows: ['auth-code'], + redirectURI: 'http://localhost:0/callback', + customFetch: sinon.stub().callsFake(fetch), + }); + + try { + await requestToken(plugin, { + issuer: provider.issuer, + clientId: 'mockclientid', + requestScopes: [], + }); + expect.fail('missed exception'); + } catch (err: any) { + expect(err.message).to.equal( + 'server responded with an error in the response body: caused by HTTP response 400 : jsonfailure, failure text' + ); + } + }); }); }); diff --git a/src/plugin.ts b/src/plugin.ts index c6187cb..d19d533 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -12,6 +12,7 @@ import { getRefreshTokenId, improveHTTPResponseBasedError, messageFromError, + nodeFetchCompat, normalizeObject, throwIfAborted, TokenSet, @@ -405,7 +406,7 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin { this.logger.emit('mongodb-oidc-plugin:outbound-http-request', { url }); try { - const response = await this.doFetch(url, init); + const response = nodeFetchCompat(await this.doFetch(url, init)); this.logger.emit('mongodb-oidc-plugin:outbound-http-request-completed', { url, status: response.status, diff --git a/src/util.ts b/src/util.ts index abde002..5f5986a 100644 --- a/src/util.ts +++ b/src/util.ts @@ -6,6 +6,7 @@ import type { } from 'openid-client'; import { MongoDBOIDCError, type OIDCAbortSignal } from './types'; import { createHash, randomBytes } from 'crypto'; +import type { Readable } from 'stream'; class AbortError extends Error { constructor() { @@ -236,6 +237,18 @@ export class TokenSet { } } +function getCause(err: unknown): Record | undefined { + if ( + err && + typeof err === 'object' && + 'cause' in err && + err.cause && + typeof err.cause === 'object' + ) { + return err.cause as Record; + } +} + // openid-client@6.x has reduced error messages for HTTP errors significantly, reducing e.g. // an HTTP error to just a simple 'unexpect HTTP response status code' message, without // further diagnostic information. So if the `cause` of an `err` object is a fetch `Response` @@ -243,37 +256,48 @@ export class TokenSet { export async function improveHTTPResponseBasedError( err: T ): Promise { - if ( - err && - typeof err === 'object' && - 'cause' in err && - err.cause && - typeof err.cause === 'object' && - 'status' in err.cause && - 'statusText' in err.cause && - 'text' in err.cause && - typeof err.cause.text === 'function' - ) { + // Note: `err.cause` can either be an `Error` object itself, or a `Response`, or a JSON HTTP response body + const cause = getCause(err); + if (cause) { try { + const statusObject = + 'status' in cause ? cause : (err as Record); + if (!statusObject.status) return err; + let body = ''; try { - body = await err.cause.text(); + if ('text' in cause && typeof cause.text === 'function') + body = await cause.text(); // Handle the `Response` case } catch { // ignore } let errorMessageFromBody = ''; try { - const parsed = JSON.parse(body); + let parsed: Record = cause; + try { + parsed = JSON.parse(body); + } catch { + // ignore, and maybe `parsed` already contains the parsed JSON body anyway + } errorMessageFromBody = - ': ' + String(parsed.error_description || parsed.error || ''); + ': ' + + [parsed.error, parsed.error_description] + .filter(Boolean) + .map(String) + .join(', '); } catch { // ignore } if (!errorMessageFromBody) errorMessageFromBody = `: ${body}`; + + const statusTextInsert = + 'statusText' in statusObject + ? `(${String(statusObject.statusText)})` + : ''; return new MongoDBOIDCError( `${errorString(err)}: caused by HTTP response ${String( - err.cause.status - )} (${String(err.cause.statusText)})${errorMessageFromBody}`, + statusObject.status + )} ${statusTextInsert}${errorMessageFromBody}`, { codeName: 'HTTPResponseError', cause: err } ); } catch { @@ -282,3 +306,76 @@ export async function improveHTTPResponseBasedError( } return err; } + +// Check whether converting a Node.js `Readable` stream to a web `ReadableStream` +// is possible. We use this for compatibility with fetch() implementations that +// return Node.js `Readable` streams like node-fetch. +export function streamIsNodeReadable(stream: unknown): stream is Readable { + return !!( + stream && + typeof stream === 'object' && + 'pipe' in stream && + typeof stream.pipe === 'function' && + (!('cancel' in stream) || !stream.cancel) + ); +} + +export function nodeFetchCompat( + response: Response & { body: Readable | ReadableStream | null } +): Response { + const notImplemented = (method: string) => + new MongoDBOIDCError(`Not implemented: body.${method}`, { + codeName: 'HTTPBodyShimNotImplemented', + }); + const { body, clone } = response; + if (streamIsNodeReadable(body)) { + let webStream: ReadableStream | undefined; + const toWeb = () => + webStream ?? (body.constructor as typeof Readable).toWeb?.(body); + // Provide ReadableStream methods that may be used by openid-client + Object.assign( + body, + { + locked: false, + cancel() { + if (webStream) return webStream.cancel(); + body.resume(); + }, + getReader(...args: Parameters) { + if ((webStream = toWeb())) return webStream.getReader(...args); + + throw notImplemented('getReader'); + }, + pipeThrough(...args: Parameters) { + if ((webStream = toWeb())) return webStream.pipeThrough(...args); + throw notImplemented('pipeThrough'); + }, + pipeTo(...args: Parameters) { + if ((webStream = toWeb())) return webStream.pipeTo(...args); + + throw notImplemented('pipeTo'); + }, + tee(...args: Parameters) { + if ((webStream = toWeb())) return webStream.tee(...args); + throw notImplemented('tee'); + }, + values(...args: Parameters) { + if ((webStream = toWeb())) return webStream.values(...args); + throw notImplemented('values'); + }, + }, + body + ); + Object.assign(response, { + clone: function (this: Response): Response { + // node-fetch replaces `.body` on `.clone()` on *both* + // the original and the cloned Response objects + const cloned = clone.call(this); + nodeFetchCompat(this); + return nodeFetchCompat(cloned); + }, + }); + } + + return response; +}