fix(jwe): pass non-JWE tokens through and constrain the decryption alg pin

Address review feedback on JWE:
- decryptAccessToken now returns non-JWE tokens (not five compact parts)
  unchanged instead of throwing. Toggling Token Encryption off, a mid-rollout
  mismatch, or an opaque-token audience no longer turns every login and refresh
  into a hard 500.
- Constrain accessTokenDecryptionAlg to the strong-algorithm allowlist at the
  config layer (defense in depth), mirroring the runtime allowlist.
- Add tests: an off-allowlist alg (A128KW) is rejected, non-JWE and opaque tokens
  pass through, and the clear-error path now uses a genuine JWE with a wrong key.

Author: jd3vi1

lib/client.js

@@ -341,14 +341,23 @@ const ALLOWED_ACCESS_TOKEN_JWE_ALGS = [
  * Decrypts a JWE-encrypted access token.
  * Co-located here so it can call the internal importPrivateKey helper directly.
  *
- * The key-management algorithm is taken from the JWE protected header (so the tenant
- * can encrypt with e.g. RSA-OAEP-256 or RSA-OAEP-512 without the client pre-declaring
- * it) but ONLY after jose validates it against an allowlist of strong algorithms.
- * This prevents algorithm-confusion / downgrade attacks (notably RSA1_5). When the
- * developer sets `accessTokenDecryptionAlg`, the allowlist is narrowed to that one
- * value, turning it into a hard pin.
+ * A JWE in compact serialization has exactly five dot-separated parts. Anything
+ * else (a plaintext 3-part JWT, an opaque token, etc.) is returned unchanged, so
+ * that toggling Token Encryption off, a mid-rollout mismatch, or an audience that
+ * yields an opaque token does not turn every login and refresh into a hard error.
+ *
+ * For genuine JWEs the key-management algorithm is taken from the protected header
+ * (so the tenant can encrypt with e.g. RSA-OAEP-256 or RSA-OAEP-512 without the
+ * client pre-declaring it) but ONLY after jose validates it against an allowlist of
+ * strong algorithms. This prevents algorithm-confusion / downgrade attacks (notably
+ * RSA1_5). When the developer sets `accessTokenDecryptionAlg`, the allowlist is
+ * narrowed to that one value, turning it into a hard pin.
  */
 async function decryptAccessToken(token, keyData, alg) {
+  // Not a compact JWE (five parts) — pass through untouched.
+  if (typeof token !== 'string' || token.split('.').length !== 5) {
+    return token;
+  }
   const keyManagementAlgorithms = alg ? [alg] : ALLOWED_ACCESS_TOKEN_JWE_ALGS;
   const { plaintext } = await compactDecrypt(
     token,

lib/config.js

@@ -50,6 +50,20 @@ const ASYMMETRIC_SIGNING_ALGS = [
   'Ed25519',
 ];
 
+// Strong JWE key-management algorithms accepted for access-token decryption.
+// Mirrors ALLOWED_ACCESS_TOKEN_JWE_ALGS in lib/client.js; kept here so an explicit
+// accessTokenDecryptionAlg pin is validated at config time (defense in depth).
+const ACCESS_TOKEN_JWE_ALGS = [
+  'RSA-OAEP',
+  'RSA-OAEP-256',
+  'RSA-OAEP-384',
+  'RSA-OAEP-512',
+  'ECDH-ES',
+  'ECDH-ES+A128KW',
+  'ECDH-ES+A192KW',
+  'ECDH-ES+A256KW',
+];
+
 const paramsSchema = Joi.object({
   secret: Joi.alternatives([
     Joi.string().min(8),
@@ -390,8 +404,11 @@ const paramsSchema = Joi.object({
     ),
   // Optional pin. When omitted, the key-management alg is read from the JWE
   // protected header, so decryption works with whatever alg the tenant uses
-  // (e.g. RSA-OAEP-256 or RSA-OAEP-512). When set, the token's alg must match.
-  accessTokenDecryptionAlg: Joi.string().optional(),
+  // (e.g. RSA-OAEP-256 or RSA-OAEP-512). When set, the token's alg must match,
+  // and the pin itself is constrained to the strong-algorithm allowlist.
+  accessTokenDecryptionAlg: Joi.string()
+    .valid(...ACCESS_TOKEN_JWE_ALGS)
+    .optional(),
   discoveryCacheMaxAge: Joi.number()
     .optional()
     .min(0)

test/callback.tests.js

@@ -1862,16 +1862,25 @@ describe('callback response_mode: form_post', () => {
       assert.equal(tokens.accessToken.access_token, plainAccessToken);
     });
 
-    it('should surface a clear error when decryption fails (wrong key/token)', async () => {
-      const notAJwe = '__test_access_token__';
+    it('should surface a clear error when decryption fails (wrong key)', async () => {
+      // A genuine JWE (5 parts) encrypted to the fixture public key, but the SDK is
+      // configured with a different, non-matching private key -> decryption fails.
+      const jwe = await encryptToken(
+        'eyJhbGciOiJSUzI1NiJ9.body.sig',
+        'RSA-OAEP-256',
+      );
+      const { generateKeyPairSync } = require('crypto');
+      const wrongKey = generateKeyPairSync('rsa', {
+        modulusLength: 2048,
+      }).privateKey.export({ type: 'pkcs8', format: 'pem' });
       const validToken = makeIdToken({ c_hash: '77QmUPtjPfzWtF2AnpK9RQ' });
 
       const { response } = await setup({
-        accessToken: notAJwe,
+        accessToken: jwe,
         authOpts: {
           ...defaultConfig,
           clientSecret: '__test_client_secret__',
-          accessTokenDecryptionKey: privatePEM,
+          accessTokenDecryptionKey: wrongKey,
           authorizationParams: { response_type: 'code id_token' },
         },
         cookies: generateCookies({
@@ -1892,6 +1901,32 @@ describe('callback response_mode: form_post', () => {
       );
     });
 
+    it('should pass a plaintext (non-JWE) access token through unchanged', async () => {
+      const validToken = makeIdToken({ c_hash: '77QmUPtjPfzWtF2AnpK9RQ' });
+
+      const { tokens } = await setup({
+        accessToken: '__test_access_token__', // opaque, not a JWE
+        authOpts: {
+          ...defaultConfig,
+          clientSecret: '__test_client_secret__',
+          accessTokenDecryptionKey: privatePEM,
+          authorizationParams: { response_type: 'code id_token' },
+        },
+        cookies: generateCookies({
+          state: expectedDefaultState,
+          nonce: '__test_nonce__',
+        }),
+        body: {
+          state: expectedDefaultState,
+          id_token: validToken,
+          code: 'jHkWEdUXMU1BwAsC4vtUsZwnNvTIxEl0z9K3vx5KF0Y',
+        },
+      });
+
+      // No hard 500; the opaque token is stored unchanged.
+      assert.equal(tokens.accessToken.access_token, '__test_access_token__');
+    });
+
     it('should decrypt the refreshed access token (refresh grant, not just callback)', async () => {
       const loginPlain = 'eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.login.sig';
       const refreshedPlain =

test/client.tests.js

@@ -907,29 +907,26 @@ describe('client initialization', function () {
       assert.equal(decrypted, plaintext);
     });
 
-    it('should throw when given a non-JWE string', async function () {
+    it('should pass a 3-part JWT (non-JWE) through unchanged', async function () {
       const plainJwt = 'eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.test.signature';
 
-      await assert.isRejected(
-        decryptAccessToken(
-          plainJwt,
-          require('../end-to-end/fixture/jwk').privatePEM,
-          'RSA-OAEP-256',
-        ),
+      const result = await decryptAccessToken(
+        plainJwt,
+        require('../end-to-end/fixture/jwk').privatePEM,
+        'RSA-OAEP-256',
       );
+      assert.equal(result, plainJwt);
     });
 
-    it('should throw when decryption fails with invalid JWE', async function () {
-      const plainJwt = 'eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9';
+    it('should pass a non-JWE opaque string through unchanged', async function () {
+      const opaque = 'eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9';
 
-      // Try to decrypt something that's not a valid JWE
-      await assert.isRejected(
-        decryptAccessToken(
-          plainJwt,
-          require('../end-to-end/fixture/jwk').privatePEM,
-          'RSA-OAEP-256',
-        ),
+      const result = await decryptAccessToken(
+        opaque,
+        require('../end-to-end/fixture/jwk').privatePEM,
+        'RSA-OAEP-256',
       );
+      assert.equal(result, opaque);
     });
 
     // The alg is read from the JWE header (within a strong-alg allowlist) when no
@@ -969,5 +966,27 @@ describe('client initialization', function () {
         decryptAccessToken(jwe, privatePEM, 'RSA-OAEP-256'),
       );
     });
+
+    it('should reject a JWE whose alg is not on the allowlist', async function () {
+      // A128KW is a valid JWE alg but deliberately excluded from the allowlist.
+      // The key resolver (importPrivateKey) must never run for a rejected alg.
+      const symKey = new Uint8Array(16); // 128-bit key for A128KW
+      const jwe = await new CompactEncrypt(new TextEncoder().encode(plaintext))
+        .setProtectedHeader({ alg: 'A128KW', enc: 'A128CBC-HS256' })
+        .encrypt(symKey);
+      await assert.isRejected(decryptAccessToken(jwe, privatePEM, undefined));
+    });
+
+    it('should pass a non-JWE (plaintext JWT) token through unchanged', async function () {
+      const jwt = 'eyJhbGciOiJSUzI1NiJ9.eyJzdWIiOiJ4In0.sig'; // 3 parts, not a JWE
+      const result = await decryptAccessToken(jwt, privatePEM, undefined);
+      assert.equal(result, jwt);
+    });
+
+    it('should pass an opaque access token through unchanged', async function () {
+      const opaque = 'opaque-access-token-value';
+      const result = await decryptAccessToken(opaque, privatePEM, undefined);
+      assert.equal(result, opaque);
+    });
   });
 });