fix: validate secret length for client_secret_jwt with no alg specified

BREAKING CHANGE: when a symmetrical endpoint authentication signing alg
is not specified the secret will be validated such that it can be used
with all available HS bit lengths

lib/helpers/client_schema.js

@@ -207,19 +207,32 @@ module.exports = function getSchema(provider) {
       }
 
       // CLIENT SECRET LENGTH
-      const hsLengths = SECRET_LENGTH_REQUIRED.map((prop) => {
+      const hsLengths = SECRET_LENGTH_REQUIRED.reduce((lengths, prop) => {
         if (this[prop] && this[prop].startsWith('HS')) {
-          return parseInt(this[prop].slice(-3) / 8, 10);
+          lengths.add(parseInt(this[prop].slice(-3), 10));
         }
 
-        return undefined;
-      });
+        return lengths;
+      }, new Set());
+
+      for (const endpoint of ['token', 'introspection', 'revocation']) { // eslint-disable-line no-restricted-syntax
+        if (
+          this[`${endpoint}_endpoint_auth_method`] === 'client_secret_jwt'
+          && this[`${endpoint}_endpoint_auth_signing_alg`] === undefined
+        ) {
+          const required = Math.max(...configuration[`${endpoint}EndpointAuthSigningAlgValues`]
+            .filter(alg => alg.startsWith('HS')).map(alg => parseInt(alg.slice(-3), 10)));
+
+          hsLengths.add(required);
+        }
+      }
 
-      const validateSecretLength = _.max(hsLengths);
+      const required = _.max(Array.from(hsLengths));
 
-      if (validateSecretLength) {
-        if (this.client_secret.length < validateSecretLength) {
-          invalidate('insufficient client_secret length');
+      if (required) {
+        const actual = this.client_secret.length * 8;
+        if (actual < required) {
+          invalidate(`insufficient client_secret length (need at least ${required} bits, got ${actual})`);
         }
       }
 

test/client_auth/client_auth.config.js

@@ -33,11 +33,10 @@ module.exports = {
   }, {
     token_endpoint_auth_method: 'client_secret_jwt',
     client_id: 'client-jwt-secret',
-    client_secret: 'atleast32byteslongforHS256mmkay?',
+    client_secret: 'its64bytes_____________________________________________________!',
     redirect_uris: ['https://client.example.com/cb'],
   }, {
     client_id: 'client-jwt-key',
-    client_secret: 'whateverwontbeusedanyway',
     redirect_uris: ['https://client.example.com/cb'],
     token_endpoint_auth_method: 'private_key_jwt',
     jwks: {

test/configuration/client_secrets.config.js

@@ -0,0 +1,9 @@
+const { clone } = require('lodash');
+
+const config = clone(require('../default.config'));
+
+config.features = { introspection: true, revocation: true, jwtIntrospection: true };
+
+module.exports = {
+  config,
+};

test/configuration/client_secrets.test.js

@@ -3,7 +3,7 @@ const { expect } = require('chai');
 const bootstrap = require('../test_helper');
 
 describe('Client#add', () => {
-  before(bootstrap(__dirname));
+  before(bootstrap(__dirname, { config: 'client_secrets' }));
 
   it('client secret is mandatory if even one of the authz needs it', function () {
     expect(this.provider.Client.needsSecret({
@@ -23,10 +23,31 @@ describe('Client#add', () => {
     })).to.be.true;
   });
 
+  ['token', 'introspection', 'revocation'].forEach((endpoint) => {
+    context(`configuring ${endpoint}_endpoint_auth_method=client_secret_jwt without ${endpoint}_endpoint_auth_signing_alg`, () => {
+      it('validates the secret is long enough to support the top available alg bitsize', function () {
+        return i(this.provider).clientAdd({
+          client_id: `${Math.random()}`,
+          client_secret: 'not64bytes_____________________________________________________',
+          redirect_uris: ['https://client.example.com/cb'],
+          [`${endpoint}_endpoint_auth_method`]: 'client_secret_jwt',
+        }).then((client) => {
+          expect(client).not.to.be.ok;
+        }, (err) => {
+          expect(err).to.be.ok;
+          expect(err).to.have.property('message', 'invalid_client_metadata');
+          expect(err).to.have.property('error_description', 'insufficient client_secret length (need at least 512 bits, got 504)');
+        });
+      });
+    });
+  });
+
   [
     'id_token_signed_response_alg',
     'request_object_signing_alg',
     'token_endpoint_auth_signing_alg',
+    'revocation_endpoint_auth_signing_alg',
+    'introspection_endpoint_auth_signing_alg',
     'userinfo_signed_response_alg',
     'introspection_signed_response_alg',
   ].forEach((metadata) => {
@@ -42,7 +63,7 @@ describe('Client#add', () => {
         }, (err) => {
           expect(err).to.be.ok;
           expect(err).to.have.property('message', 'invalid_client_metadata');
-          expect(err).to.have.property('error_description', 'insufficient client_secret length');
+          expect(err).to.have.property('error_description', 'insufficient client_secret length (need at least 256 bits, got 248)');
         });
       });
 
@@ -57,7 +78,7 @@ describe('Client#add', () => {
         }, (err) => {
           expect(err).to.be.ok;
           expect(err).to.have.property('message', 'invalid_client_metadata');
-          expect(err).to.have.property('error_description', 'insufficient client_secret length');
+          expect(err).to.have.property('error_description', 'insufficient client_secret length (need at least 384 bits, got 376)');
         });
       });
 
@@ -72,7 +93,7 @@ describe('Client#add', () => {
         }, (err) => {
           expect(err).to.be.ok;
           expect(err).to.have.property('message', 'invalid_client_metadata');
-          expect(err).to.have.property('error_description', 'insufficient client_secret length');
+          expect(err).to.have.property('error_description', 'insufficient client_secret length (need at least 512 bits, got 504)');
         });
       });
     });