fix: revert missing mTLS cert errors to invalid_grant

This reverts commit c4a62e9. I'm
changing the errors back to invalid_grant so that the error_description
does not give away details to the attacker. Deployments should still
offer an out of bands feed of the error_detail error property to
developers so that they can know what's up.

lib/actions/grants/authorization_code.js

@@ -1,7 +1,7 @@
 const { get } = require('lodash');
 const uidToGrantId = require('debug')('oidc-provider:uid');
 
-const { InvalidGrant, InvalidRequest } = require('../../helpers/errors');
+const { InvalidGrant } = require('../../helpers/errors');
 const presence = require('../../helpers/validate_presence');
 const instance = require('../../helpers/weak_cache');
 const checkPKCE = require('../../helpers/pkce');
@@ -48,7 +48,7 @@ module.exports.handler = async function authorizationCodeHandler(ctx, next) {
   if (ctx.oidc.client.tlsClientCertificateBoundAccessTokens) {
     cert = getCertificate(ctx);
     if (!cert) {
-      throw new InvalidRequest('mutual TLS client certificate not provided');
+      throw new InvalidGrant('mutual TLS client certificate not provided');
     }
   }
 

lib/actions/grants/client_credentials.js

@@ -1,5 +1,5 @@
 const instance = require('../../helpers/weak_cache');
-const { InvalidRequest, InvalidGrant } = require('../../helpers/errors');
+const { InvalidGrant } = require('../../helpers/errors');
 
 module.exports.handler = async function clientCredentialsHandler(ctx, next) {
   const { ClientCredentials, ReplayDetection } = ctx.oidc.provider;
@@ -33,7 +33,7 @@ module.exports.handler = async function clientCredentialsHandler(ctx, next) {
     const cert = getCertificate(ctx);
 
     if (!cert) {
-      throw new InvalidRequest('mutual TLS client certificate not provided');
+      throw new InvalidGrant('mutual TLS client certificate not provided');
     }
     token.setThumbprint('x5t', cert);
   }

lib/actions/grants/device_code.js

@@ -5,9 +5,7 @@ const errors = require('../../helpers/errors');
 const presence = require('../../helpers/validate_presence');
 const instance = require('../../helpers/weak_cache');
 
-const {
-  InvalidGrant, ExpiredToken, AuthorizationPending, InvalidRequest,
-} = errors;
+const { InvalidGrant, ExpiredToken, AuthorizationPending } = errors;
 
 const gty = 'device_code';
 
@@ -36,7 +34,7 @@ module.exports.handler = async function deviceCodeHandler(ctx, next) {
   if (ctx.oidc.client.tlsClientCertificateBoundAccessTokens) {
     cert = getCertificate(ctx);
     if (!cert) {
-      throw new InvalidRequest('mutual TLS client certificate not provided');
+      throw new InvalidGrant('mutual TLS client certificate not provided');
     }
   }
 

lib/actions/grants/refresh_token.js

@@ -1,7 +1,7 @@
 const _ = require('lodash');
 const uidToGrantId = require('debug')('oidc-provider:uid');
 
-const { InvalidRequest, InvalidGrant, InvalidScope } = require('../../helpers/errors');
+const { InvalidGrant, InvalidScope } = require('../../helpers/errors');
 const presence = require('../../helpers/validate_presence');
 const instance = require('../../helpers/weak_cache');
 const revokeGrant = require('../../helpers/revoke_grant');
@@ -39,7 +39,7 @@ module.exports.handler = async function refreshTokenHandler(ctx, next) {
   if (ctx.oidc.client.tlsClientCertificateBoundAccessTokens || refreshToken['x5t#S256']) {
     cert = getCertificate(ctx);
     if (!cert) {
-      throw new InvalidRequest('mutual TLS client certificate not provided');
+      throw new InvalidGrant('mutual TLS client certificate not provided');
     }
   }
 

test/certificate_bound_access_tokens/certificate_bound_access_tokens.test.js

@@ -110,16 +110,22 @@ describe('features.mTLS.certificateBoundAccessTokens', () => {
       expect(RefreshToken).not.to.have.property('x5t#S256');
     });
 
-    it('verifies the request made with mutual-TLS', function () {
-      return this.agent.post('/token')
+    it('verifies the request made with mutual-TLS', async function () {
+      const spy = sinon.spy();
+      this.provider.once('grant.error', spy);
+
+      await this.agent.post('/token')
         .auth('client', 'secret')
         .send({
           grant_type: 'urn:ietf:params:oauth:grant-type:device_code',
           device_code: this.dc,
         })
         .type('form')
         .expect(400)
-        .expect({ error: 'invalid_request', error_description: 'mutual TLS client certificate not provided' });
+        .expect({ error: 'invalid_grant', error_description: 'grant request is invalid' });
+
+      expect(spy).to.have.property('calledOnce', true);
+      expect(spy.args[0][1]).to.have.property('error_detail', 'mutual TLS client certificate not provided');
     });
 
     it('binds the refresh token to the certificate for public clients', async function () {
@@ -190,8 +196,11 @@ describe('features.mTLS.certificateBoundAccessTokens', () => {
         expect(RefreshToken).not.to.have.property('x5t#S256');
       });
 
-      it('verifies the request made with mutual-TLS', function () {
-        return this.agent.post('/token')
+      it('verifies the request made with mutual-TLS', async function () {
+        const spy = sinon.spy();
+        this.provider.once('grant.error', spy);
+
+        await this.agent.post('/token')
           .auth('client', 'secret')
           .send({
             grant_type: 'authorization_code',
@@ -200,7 +209,10 @@ describe('features.mTLS.certificateBoundAccessTokens', () => {
           })
           .type('form')
           .expect(400)
-          .expect({ error: 'invalid_request', error_description: 'mutual TLS client certificate not provided' });
+          .expect({ error: 'invalid_grant', error_description: 'grant request is invalid' });
+
+        expect(spy).to.have.property('calledOnce', true);
+        expect(spy.args[0][1]).to.have.property('error_detail', 'mutual TLS client certificate not provided');
       });
     });
 
@@ -240,16 +252,22 @@ describe('features.mTLS.certificateBoundAccessTokens', () => {
         expect(RefreshToken['x5t#S256']).to.be.undefined;
       });
 
-      it('verifies the request made with mutual-TLS', function () {
-        return this.agent.post('/token')
+      it('verifies the request made with mutual-TLS', async function () {
+        const spy = sinon.spy();
+        this.provider.once('grant.error', spy);
+
+        await this.agent.post('/token')
           .auth('client', 'secret')
           .send({
             grant_type: 'refresh_token',
             refresh_token: this.rt,
           })
           .type('form')
           .expect(400)
-          .expect({ error: 'invalid_request', error_description: 'mutual TLS client certificate not provided' });
+          .expect({ error: 'invalid_grant', error_description: 'grant request is invalid' });
+
+        expect(spy).to.have.property('calledOnce', true);
+        expect(spy.args[0][1]).to.have.property('error_detail', 'mutual TLS client certificate not provided');
       });
     });
   });
@@ -297,8 +315,11 @@ describe('features.mTLS.certificateBoundAccessTokens', () => {
         expect(RefreshToken).to.have.property('x5t#S256', expectedS256);
       });
 
-      it('verifies the request made with mutual-TLS', function () {
-        return this.agent.post('/token')
+      it('verifies the request made with mutual-TLS', async function () {
+        const spy = sinon.spy();
+        this.provider.once('grant.error', spy);
+
+        await this.agent.post('/token')
           .send({
             client_id: 'client-none',
             grant_type: 'authorization_code',
@@ -307,7 +328,10 @@ describe('features.mTLS.certificateBoundAccessTokens', () => {
           })
           .type('form')
           .expect(400)
-          .expect({ error: 'invalid_request', error_description: 'mutual TLS client certificate not provided' });
+          .expect({ error: 'invalid_grant', error_description: 'grant request is invalid' });
+
+        expect(spy).to.have.property('calledOnce', true);
+        expect(spy.args[0][1]).to.have.property('error_detail', 'mutual TLS client certificate not provided');
       });
     });
 
@@ -347,16 +371,22 @@ describe('features.mTLS.certificateBoundAccessTokens', () => {
         expect(RefreshToken).to.have.property('x5t#S256', expectedS256);
       });
 
-      it('verifies the request made with mutual-TLS', function () {
-        return this.agent.post('/token')
+      it('verifies the request made with mutual-TLS', async function () {
+        const spy = sinon.spy();
+        this.provider.once('grant.error', spy);
+
+        await this.agent.post('/token')
           .auth('client', 'secret')
           .send({
             grant_type: 'refresh_token',
             refresh_token: this.rt,
           })
           .type('form')
           .expect(400)
-          .expect({ error: 'invalid_request', error_description: 'mutual TLS client certificate not provided' });
+          .expect({ error: 'invalid_grant', error_description: 'grant request is invalid' });
+
+        expect(spy).to.have.property('calledOnce', true);
+        expect(spy.args[0][1]).to.have.property('error_detail', 'mutual TLS client certificate not provided');
       });
 
       it('verifies the request made with mutual-TLS using the same cert', async function () {
@@ -397,13 +427,19 @@ describe('features.mTLS.certificateBoundAccessTokens', () => {
       expect(ClientCredentials).to.have.property('x5t#S256', expectedS256);
     });
 
-    it('verifies the request was made with mutual-TLS', function () {
-      return this.agent.post('/token')
+    it('verifies the request was made with mutual-TLS', async function () {
+      const spy = sinon.spy();
+      this.provider.once('grant.error', spy);
+
+      await this.agent.post('/token')
         .auth('client', 'secret')
         .send({ grant_type: 'client_credentials' })
         .type('form')
         .expect(400)
-        .expect({ error: 'invalid_request', error_description: 'mutual TLS client certificate not provided' });
+        .expect({ error: 'invalid_grant', error_description: 'grant request is invalid' });
+
+      expect(spy).to.have.property('calledOnce', true);
+      expect(spy.args[0][1]).to.have.property('error_detail', 'mutual TLS client certificate not provided');
     });
   });
 });