[jwt] fix unauthorized being thrown in onRequest (#3149)

* [jwt] fix unauthorized being thrown in `onRequest` * changeset * fix tests
dotansimha · Dec 22, 2023 · b9d2afc · b9d2afc
1 parent ab81a2e
commit b9d2afc
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 25 deletions.
diff --git a/.changeset/thirty-seas-attack.md b/.changeset/thirty-seas-attack.md
@@ -0,0 +1,6 @@
+---
+'@graphql-yoga/plugin-jwt': patch
+---
+
+Fix unauthorized error resulting in an response with 500 status or in a server crash (depending on
+actual HTTP server implementation used).
diff --git a/packages/plugins/jwt/src/__tests__/jwt.spec.ts b/packages/plugins/jwt/src/__tests__/jwt.spec.ts
@@ -21,45 +21,70 @@ describe('jwt', () => {
   it('should throw on unsupported header type', async () => {
     const server = createTestServer();
 
-    await expect(server.queryWithAuth('Basic 123')).rejects.toMatchObject({
-      message: 'Unsupported token type provided: "Basic"',
-      extensions: { http: { status: 401 } },
+    const response = await server.queryWithAuth('Basic 123');
+    expect(response.status).toBe(401);
+    expect(await response.json()).toMatchObject({
+      errors: [
+        {
+          message: 'Unsupported token type provided: "Basic"',
+        },
+      ],
     });
   });
 
   it('should throw on invalid token', async () => {
     const server = createTestServer();
 
-    await expect(server.queryWithAuth('Bearer abcd')).rejects.toMatchObject({
-      message: 'Failed to decode authentication token',
-      extensions: { http: { status: 401 } },
+    const response = await server.queryWithAuth('Bearer abcd');
+    expect(response.status).toBe(401);
+    expect(await response.json()).toMatchObject({
+      errors: [
+        {
+          message: 'Failed to decode authentication token. Verification failed.',
+        },
+      ],
     });
   });
 
   it('should not accept token without algorithm', async () => {
     const server = createTestServer();
 
-    await expect(server.queryWithAuth(buildJWTWithoutAlg())).rejects.toMatchObject({
-      message: 'Failed to decode authentication token',
-      extensions: { http: { status: 401 } },
+    const response = await server.queryWithAuth(buildJWTWithoutAlg());
+    expect(response.status).toBe(401);
+    expect(await response.json()).toMatchObject({
+      errors: [
+        {
+          message: 'Failed to decode authentication token. Verification failed.',
+        },
+      ],
     });
   });
 
   it('should not allow non matching issuer', async () => {
     const server = createTestServer();
 
-    await expect(server.queryWithAuth(buildJWT({ iss: 'test' }))).rejects.toMatchObject({
-      message: 'Failed to decode authentication token',
-      extensions: { http: { status: 401 } },
+    const response = await server.queryWithAuth(buildJWT({ iss: 'test' }));
+    expect(response.status).toBe(401);
+    expect(await response.json()).toMatchObject({
+      errors: [
+        {
+          message: 'Failed to decode authentication token. Verification failed.',
+        },
+      ],
     });
   });
 
   it('should not allow non matching audience', async () => {
     const server = createTestServer({ audience: 'test' });
 
-    await expect(server.queryWithAuth(buildJWT({ aud: 'wrong' }))).rejects.toMatchObject({
-      message: 'Failed to decode authentication token',
-      extensions: { http: { status: 401 } },
+    const response = await server.queryWithAuth(buildJWT({ aud: 'wrong' }));
+    expect(response.status).toBe(401);
+    expect(await response.json()).toMatchObject({
+      errors: [
+        {
+          message: 'Failed to decode authentication token. Verification failed.',
+        },
+      ],
     });
   });
 
@@ -74,9 +99,14 @@ describe('jwt', () => {
   it('should not allow unknown key id', async () => {
     const server = createTestServer({ signingKey: undefined, jwksUri: 'test' });
 
-    await expect(server.queryWithAuth(buildJWT({}, { keyid: 'unknown' }))).rejects.toMatchObject({
-      message: 'Failed to decode authentication token. Unknown key id.',
-      extensions: { http: { status: 401 } },
+    const response = await server.queryWithAuth(buildJWT({}, { keyid: 'unknown' }));
+    expect(response.status).toBe(401);
+    expect(await response.json()).toMatchObject({
+      errors: [
+        {
+          message: 'Failed to decode authentication token. Unknown key id.',
+        },
+      ],
     });
   });
 

diff --git a/packages/plugins/jwt/src/index.ts b/packages/plugins/jwt/src/index.ts
@@ -80,7 +80,7 @@ export function useJWT(options: JwtPluginOptions): Plugin {
   }
 
   return {
-    async onRequest({ request, serverContext, url }) {
+    async onRequestParse({ request, serverContext, url }) {
       const token = await getToken({ request, serverContext, url });
       if (token != null) {
         const signingKey = options.signingKey ?? (await fetchKey(jwksClient, token));
@@ -133,12 +133,7 @@ function verify(
       { ...options, algorithms: options?.algorithms ?? ['RS256'] },
       (err, result) => {
         if (err) {
-          // Should we expose the error message? Perhaps only in development mode?
-          reject(
-            unauthorizedError('Failed to decode authentication token', {
-              originalError: err,
-            }),
-          );
+          reject(unauthorizedError('Failed to decode authentication token. Verification failed.'));
         } else {
           resolve(result as JwtPayload);
         }