feat: basic and post client auth methods are now interchangeable

Background: Many developers struggle to understand that this is a
configuration on the client and their SDKs rarely surface this, combined
with the fact that many SDKs default to POST while the defined default
should be basic this wastes developer's time.

This makes the two client auth methods interchangeable, so if the client
is configured with basic but post is provided, it works and vice versa.

lib/shared/token_auth.js

@@ -5,7 +5,6 @@ const instance = require('../helpers/weak_cache');
 const { 'x5t#S256': thumbprint } = require('../helpers/calculate_thumbprint');
 
 const rejectDupes = require('./reject_dupes');
-const loadClient = require('./load_client');
 const tokenCredentialAuth = require('./token_credential_auth');
 const getJWTAuthMiddleware = require('./token_jwt_auth');
 
@@ -83,15 +82,25 @@ module.exports = function tokenAuth(provider, endpoint, jwtAuthEndpointIdentifie
           } catch (err) {
             throw new InvalidRequest('client_id and client_secret in the authorization header are not properly encoded');
           }
-          ctx.oidc.authorization.methods = ['client_secret_basic'];
 
           if (clientId !== undefined && ctx.oidc.authorization.clientId !== clientId) {
             throw new InvalidRequest('mismatch in body and authorization client ids');
           }
-          if (clientSecret !== undefined) throw new InvalidRequest('client authentication must only be provided using one mechanism');
+
+          if (!ctx.oidc.authorization.clientSecret) {
+            throw new InvalidRequest('client_secret must be provided in the Authorization header');
+          }
+
+          if (clientSecret !== undefined) {
+            throw new InvalidRequest('client authentication must only be provided using one mechanism');
+          }
+
+          ctx.oidc.authorization.methods = ['client_secret_basic', 'client_secret_post'];
         } else if (clientId !== undefined) {
           ctx.oidc.authorization.clientId = clientId;
-          ctx.oidc.authorization.methods = ['client_secret_post', 'none', 'tls_client_auth', 'self_signed_tls_client_auth'];
+          ctx.oidc.authorization.methods = clientSecret
+            ? ['client_secret_basic', 'client_secret_post']
+            : ['none', 'tls_client_auth', 'self_signed_tls_client_auth'];
         }
 
         if (clientAssertion !== undefined) {
@@ -128,7 +137,17 @@ module.exports = function tokenAuth(provider, endpoint, jwtAuthEndpointIdentifie
 
         return next();
       },
-      loadClient(provider),
+      async function loadClient(ctx, next) {
+        const client = await provider.Client.find(ctx.oidc.authorization.clientId);
+
+        if (!client) {
+          throw new InvalidClientAuth('client not found');
+        }
+
+        ctx.oidc.entity('Client', client);
+
+        await next();
+      },
       async function auth(ctx, next) {
         const {
           params,
@@ -148,18 +167,16 @@ module.exports = function tokenAuth(provider, endpoint, jwtAuthEndpointIdentifie
 
         switch (clientMethod) { // eslint-disable-line default-case
           case 'none':
-            if (params.client_secret) {
-              throw new InvalidClientAuth(`unexpected client_secret provided for ${endpoint}_endpoint_auth_method=none client request`);
-            }
             break;
 
-          case 'client_secret_post':
-            if (!params.client_secret) {
-              throw new InvalidClientAuth('client_secret must be provided in the body');
-            }
-            tokenCredentialAuth(ctx, ctx.oidc.client.clientSecret, params.client_secret);
+          case 'client_secret_basic':
+          case 'client_secret_post': {
+            const expected = ctx.oidc.client.clientSecret;
+            const actual = params.client_secret || clientSecret;
+            tokenCredentialAuth(ctx, actual, expected);
 
             break;
+          }
 
           case 'client_secret_jwt':
             await tokenJwtAuth(
@@ -177,15 +194,6 @@ module.exports = function tokenAuth(provider, endpoint, jwtAuthEndpointIdentifie
 
             break;
 
-          case 'client_secret_basic':
-            if (!clientSecret) {
-              throw new InvalidClientAuth('client_secret must be provided in the Authorization header');
-            }
-
-            tokenCredentialAuth(ctx, ctx.oidc.client.clientSecret, clientSecret);
-
-            break;
-
           case 'tls_client_auth': {
             const { mTLS: { getCertificate, certificateAuthorized, certificateSubjectMatches } } = instance(provider).configuration('features');
 

lib/shared/token_credential_auth.js

@@ -1,7 +1,7 @@
 const constantEquals = require('../helpers/constant_equals');
 const { InvalidClientAuth } = require('../helpers/errors');
 
-module.exports = function tokenCredentialAuth(ctx, expected, actual) {
+module.exports = function tokenCredentialAuth(ctx, actual, expected) {
   if (!constantEquals(expected, actual, 1000)) {
     throw new InvalidClientAuth('invalid secret provided');
   }

test/client_auth/client_auth.test.js

@@ -189,7 +189,7 @@ describe('client authentication options', () => {
         .type('form')
         .expect(() => {
           expect(spy.calledOnce).to.be.true;
-          expect(errorDetail(spy)).to.equal('unexpected client_secret provided for token_endpoint_auth_method=none client request');
+          expect(errorDetail(spy)).to.equal('the registered client token_endpoint_auth_method does not match the provided auth mechanism');
         })
         .expect(401)
         .expect(tokenAuthRejected);
@@ -207,6 +207,16 @@ describe('client authentication options', () => {
         .expect(tokenAuthSucceeded);
     });
 
+    it('accepts the auth (but client configured with post)', function () {
+      return this.agent.post(route)
+        .send({
+          grant_type: 'implicit',
+        })
+        .type('form')
+        .auth('client-post', 'secret')
+        .expect(tokenAuthSucceeded);
+    });
+
     it('accepts the auth even with id in the body', function () {
       return this.agent.post(route)
         .send({
@@ -253,7 +263,7 @@ describe('client authentication options', () => {
         .expect(tokenAuthSucceeded);
     });
 
-    it('accepts the auth (https://tools.ietf.org/html/rfc6749#appendix-B)', function () {
+    it('rejects improperly encoded headers', function () {
       return this.agent.post(route)
         .send({
           grant_type: 'implicit',
@@ -357,20 +367,17 @@ describe('client authentication options', () => {
     });
 
     it('requires the client_secret to be sent', function () {
-      const spy = sinon.spy();
-      this.provider.once('grant.error', spy);
       return this.agent.post(route)
         .send({
           grant_type: 'implicit',
         })
         .type('form')
         .auth('client-basic', '')
-        .expect(() => {
-          expect(spy.calledOnce).to.be.true;
-          expect(errorDetail(spy)).to.equal('client_secret must be provided in the Authorization header');
-        })
-        .expect(401)
-        .expect(tokenAuthRejected);
+        .expect(400)
+        .expect({
+          error: 'invalid_request',
+          error_description: 'client_secret must be provided in the Authorization header',
+        });
     });
   });
 
@@ -386,6 +393,17 @@ describe('client authentication options', () => {
         .expect(tokenAuthSucceeded);
     });
 
+    it('accepts the auth (but client configured with basic)', function () {
+      return this.agent.post(route)
+        .send({
+          grant_type: 'implicit',
+          client_id: 'client-basic',
+          client_secret: 'secret',
+        })
+        .type('form')
+        .expect(tokenAuthSucceeded);
+    });
+
     it('rejects invalid secrets', function () {
       const spy = sinon.spy();
       this.provider.once('grant.error', spy);
@@ -416,7 +434,7 @@ describe('client authentication options', () => {
         .type('form')
         .expect(() => {
           expect(spy.calledOnce).to.be.true;
-          expect(errorDetail(spy)).to.equal('client_secret must be provided in the body');
+          expect(errorDetail(spy)).to.equal('the registered client token_endpoint_auth_method does not match the provided auth mechanism');
         })
         .expect(401)
         .expect(tokenAuthRejected);