Skip to content

Commit 37a1fc4

Browse files
authored
fix: add cause error messages also on generic token parse failures MONGOSH-2884 (#234)
Otherwise, error messages are short and potentially not very helpful. We already implemented this for HTTP errors earlier in 4f69e53 and improved on it in cb07f51, but did not account for these specific types of very broad generic failures.
1 parent 455445b commit 37a1fc4

File tree

2 files changed

+44
-4
lines changed

2 files changed

+44
-4
lines changed

src/plugin.spec.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1704,6 +1704,36 @@ describe('OIDC plugin (mock OIDC provider)', function () {
17041704
expect(entry.error).to.include(selfSignedReason);
17051705
});
17061706

1707+
it('logs helpful error messages for OIDC token parse failures', async function () {
1708+
getTokenPayload = () => ({
1709+
expires_in: tokenPayload.expires_in,
1710+
payload: { ...tokenPayload.payload, nonce: undefined },
1711+
});
1712+
const plugin = createMongoDBOIDCPlugin({
1713+
openBrowserTimeout: 60_000,
1714+
openBrowser: fetchBrowser,
1715+
allowedFlows: ['auth-code'],
1716+
redirectURI: 'http://localhost:0/callback',
1717+
});
1718+
1719+
try {
1720+
await requestToken(plugin, {
1721+
issuer: provider.issuer,
1722+
clientId: 'mockclientid',
1723+
requestScopes: [],
1724+
});
1725+
expect.fail('missed exception');
1726+
} catch (err: any) {
1727+
expect(err.message).to.equal(
1728+
'invalid response encountered (caused by: JWT "nonce" (nonce) claim missing)'
1729+
);
1730+
expect(err.cause.message).to.equal('invalid response encountered');
1731+
expect(err.cause.cause.message).to.equal(
1732+
'JWT "nonce" (nonce) claim missing'
1733+
);
1734+
}
1735+
});
1736+
17071737
it('handles JSON failure responses from the IDP', async function () {
17081738
overrideRequestHandler = (url, req, res) => {
17091739
if (new URL(url).pathname.endsWith('/token')) {

src/util.ts

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -254,20 +254,30 @@ function getCause(err: unknown): Record<string, unknown> | undefined {
254254
}
255255
}
256256

257+
function wrapGenericError(err: unknown): MongoDBOIDCError {
258+
if (MongoDBOIDCError.isMongoDBOIDCError(err)) return err;
259+
return new MongoDBOIDCError(errorString(err), {
260+
codeName: 'GenericError',
261+
cause: err,
262+
});
263+
}
264+
257265
// openid-client@6.x has reduced error messages for HTTP errors significantly, reducing e.g.
258266
// an HTTP error to just a simple 'unexpect HTTP response status code' message, without
259267
// further diagnostic information. So if the `cause` of an `err` object is a fetch `Response`
260268
// object, we try to throw a more helpful error.
261269
export async function improveHTTPResponseBasedError<T>(
262270
err: T
263-
): Promise<T | MongoDBOIDCError> {
271+
): Promise<MongoDBOIDCError> {
264272
// Note: `err.cause` can either be an `Error` object itself, or a `Response`, or a JSON HTTP response body
265273
const cause = getCause(err);
266274
if (cause) {
267275
try {
268276
const statusObject =
269277
'status' in cause ? cause : (err as Record<string, unknown>);
270-
if (!statusObject.status) return err;
278+
if (!statusObject.status) {
279+
return wrapGenericError(err);
280+
}
271281

272282
let body = '';
273283
try {
@@ -306,10 +316,10 @@ export async function improveHTTPResponseBasedError<T>(
306316
{ codeName: 'HTTPResponseError', cause: err }
307317
);
308318
} catch {
309-
return err;
319+
return wrapGenericError(err);
310320
}
311321
}
312-
return err;
322+
return wrapGenericError(err);
313323
}
314324

315325
// Check whether converting a Node.js `Readable` stream to a web `ReadableStream`

0 commit comments

Comments
 (0)