Add kid to JWT header (#3394)

* Add kid to JWT header * Consolidate utils imports * Stub out importJWTPublicKey * Use `--bail` flag with tap tests (brings more clarity to test failures)
boxyhq · Dec 3, 2024 · df9c48b · df9c48b
1 parent 649c0b0
commit df9c48b
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 4 deletions.
diff --git a/npm/package.json b/npm/package.json
@@ -34,7 +34,7 @@
     "db:migration:run:mssql": "cross-env DB_TYPE=mssql DB_URL='sqlserver://localhost:1433;database=master;username=sa;password=123ABabc!' ts-node --transpile-only ../node_modules/typeorm/cli.js migration:run -d typeorm.ts",
     "db:migration:run:sqlite": "cross-env DB_TYPE=sqlite DB_URL='file:///tmp/migration-sqlite.db' ts-node --transpile-only ../node_modules/typeorm/cli.js migration:run -d typeorm.ts",
     "prepublishOnly": "npm run build",
-    "test": "cross-env BOXYHQ_NO_ANALYTICS=1 NODE_OPTIONS='--experimental-require-module' tap --timeout=0 --allow-incomplete-coverage --allow-empty-coverage test/**/*.test.ts",
+    "test": "cross-env BOXYHQ_NO_ANALYTICS=1 NODE_OPTIONS='--experimental-require-module' tap --timeout=0 --allow-incomplete-coverage --allow-empty-coverage --bail test/**/*.test.ts",
     "sort": "npx sort-package-json"
   },
   "tap": {

diff --git a/npm/src/controller/oauth.ts b/npm/src/controller/oauth.ts
@@ -4,8 +4,6 @@ import { promisify } from 'util';
 import { deflateRaw } from 'zlib';
 import saml from '@boxyhq/saml20';
 import { SAMLProfile } from '@boxyhq/saml20/dist/typings';
-import { AuthorizationCodeGrantResult, clientIDFederatedPrefix, clientIDOIDCPrefix } from './utils';
-
 import type {
   IOAuthController,
   JacksonOption,
@@ -23,11 +21,15 @@ import type {
   IdentityFederationApp,
 } from '../typings';
 import {
+  AuthorizationCodeGrantResult,
+  clientIDFederatedPrefix,
+  clientIDOIDCPrefix,
   relayStatePrefix,
   IndexNames,
   OAuthErrorResponse,
   getErrorMessage,
   loadJWSPrivateKey,
+  computeKid,
   isJWSKeyPairLoaded,
   extractOIDCUserProfile,
   getScopeValues,
@@ -1180,8 +1182,9 @@ export class OAuthController implements IOAuthController {
         groups: codeVal.profile.claims.groups,
       };
       const signingKey = await loadJWSPrivateKey(jwtSigningKeys.private, jwsAlg!);
+      const kid = await computeKid(jwtSigningKeys.public, jwsAlg!);
       const id_token = await new jose.SignJWT(claims)
-        .setProtectedHeader({ alg: jwsAlg! })
+        .setProtectedHeader({ alg: jwsAlg!, kid })
         .setIssuedAt()
         .setIssuer(this.opts.externalUrl)
         .setSubject(codeVal.profile.claims.id)

diff --git a/npm/src/controller/utils.ts b/npm/src/controller/utils.ts
@@ -121,6 +121,12 @@ export const generateJwkThumbprint = async (jwk: jose.JWK): Promise<string> => {
   return thumbprint;
 };
 
+export const computeKid = async (key: string, jwsAlg: string): Promise<string> => {
+  const importedPublicKey = await importJWTPublicKey(key, jwsAlg!);
+  const publicKeyJWK = await exportPublicKeyJWK(importedPublicKey);
+  return await generateJwkThumbprint(publicKeyJWK);
+};
+
 export const validateSSOConnection = (
   body:
     | SAMLSSOConnectionWithRawMetadata

diff --git a/npm/test/sso/saml_idp_oauth.test.ts b/npm/test/sso/saml_idp_oauth.test.ts
@@ -563,6 +563,7 @@ tap.test('token()', async (t) => {
         };
 
         const stubLoadJWSPrivateKey = sinon.stub(utils, 'loadJWSPrivateKey').resolves(keyPair.privateKey);
+        const stubimportJWTPublicKey = sinon.stub(utils, 'importJWTPublicKey').resolves(keyPair.publicKey);
         const stubValidate = sinon.stub(saml, 'validate').resolves({
           audience: '',
           claims: { id: 'id', firstName: 'john', lastName: 'doe', email: '[email protected]' },
@@ -612,6 +613,7 @@ tap.test('token()', async (t) => {
         stubRandomBytes.restore();
         stubValidate.restore();
         stubLoadJWSPrivateKey.restore();
+        stubimportJWTPublicKey.restore();
       });
 
       t.test('PKCE check', async (t) => {