Repository: auth0/nextjs-auth0
Pull request #236 (2021-01-12 13:40:53)

AggregateError#message from `Issuer.discover` includes stack trace
### Description

One of the errors thrown by `Issuer.discover` is an `AggregateError`, which encapsulates all the errors thrown when the `openid-client` tries to find a `well-known` endpoint.

The `error.message` of the `AggregateError` includes a `\n` separated string of a list of the entire errors (`String(error)`) - stack trace included (See https://github.com/sindresorhus/aggregate-error/blob/v3.1.0/index.js#L26-L31)

In production we don't want to show the stack trace, so the error message in this case needs to be amended. Instead of trying to edit the error message, we're opting for showing the first error in the aggregate, on the assumption that, if all the discovery endpoints fail, the error will be similar for all.

In the long term we should try and fix this in the downstream dependency `openid-client` > `p-any` > `p-some` > `aggregate-error`

### References

https://github.com/sindresorhus/aggregate-error/issues/4#issuecomment-488356468

### Testing

- Point `issuerBaseURL` at an https server with invalid certs
- Visit Login

#### Before

![image](https://user-images.githubusercontent.com/1299658/104321398-7906ec80-54db-11eb-85f8-210db65e4fca.png)

#### After

![image](https://user-images.githubusercontent.com/1299658/104321438-845a1800-54db-11eb-86c2-42abbb36bc5d.png)

- [x] This change adds test coverage for new/changed/fixed functionality

### Checklist

- [x] I have added documentation for new/changed functionality in this PR or in auth0.com/docs
- [x] All active GitHub checks for tests, formatting, and security are passing
- [x] The correct base branch is being used, if not `master`



Patch:
diff --git a/src/auth0-session/client.ts b/src/auth0-session/client.ts
index 753c531a..7658503d 100644
--- a/src/auth0-session/client.ts
+++ b/src/auth0-session/client.ts
@@ -19,6 +19,17 @@ function sortSpaceDelimitedString(str: string): string {
   return str.split(' ').sort().join(' ');
 }
 
+// Issuer.discover throws an `AggregateError` in some cases, this error includes the stack trace in the
+// message which causes the stack to be exposed when reporting the error in production. Am using the non standard
+// `_errors` property to identify the polyfilled `AggregateError`
+// See https://github.com/sindresorhus/aggregate-error/issues/4#issuecomment-488356468
+function normalizeAggregateError(e: Error | (Error & { _errors: Error[] })): Error {
+  if ('_errors' in e) {
+    return e._errors[0];
+  }
+  return e;
+}
+
 export default function get(config: Config, { name, version }: Telemetry): ClientFactory {
   let client: Client | null = null;
 
@@ -54,7 +65,12 @@ export default function get(config: Config, { name, version }: Telemetry): Clien
     };
 
     applyHttpOptionsCustom(Issuer);
-    const issuer = await Issuer.discover(config.issuerBaseURL);
+    let issuer: Issuer<Client>;
+    try {
+      issuer = await Issuer.discover(config.issuerBaseURL);
+    } catch (e) {
+      throw normalizeAggregateError(e);
+    }
     applyHttpOptionsCustom(issuer);
 
     const issuerTokenAlgs = Array.isArray(issuer.id_token_signing_alg_values_supported)


Test patch:
diff --git a/tests/auth0-session/client.test.ts b/tests/auth0-session/client.test.ts
index ab803e61..de4ea489 100644
--- a/tests/auth0-session/client.test.ts
+++ b/tests/auth0-session/client.test.ts
@@ -1,5 +1,5 @@
 import nock from 'nock';
-import { Client } from 'openid-client';
+import { Client, Issuer } from 'openid-client';
 import { getConfig, clientFactory, ConfigParameters } from '../../src/auth0-session';
 import { jwks } from './fixtures/cert';
 import pkg from '../../package.json';
@@ -132,4 +132,16 @@ describe('clientFactory', function () {
       })
     ).resolves.not.toThrow();
   });
+
+  it('should not disclose stack trace in AggregateError message when discovery fails', async () => {
+    nock.cleanAll();
+    nock('https://op.example.com').get('/.well-known/oauth-authorization-server').reply(500);
+    nock('https://op.example.com').get('/.well-known/openid-configuration').reply(500);
+    await expect(getClient()).rejects.toThrowError(new Error('expected 200 OK, got: 500 Internal Server Error'));
+  });
+
+  it('should not normalize individual errors from discovery', async () => {
+    jest.spyOn(Issuer, 'discover').mockRejectedValue(new Error('foo'));
+    await expect(getClient()).rejects.toThrowError(new Error('foo'));
+  });
 });