refactor: test decoded basic auth tokens for their VSCHAR pattern

panva · Jan 5, 2024 · 3f86cc0 · 3f86cc0
1 parent 391885c
commit 3f86cc0
Show file tree

Hide file tree

Showing 5 changed files with 73 additions and 4 deletions.
diff --git a/lib/consts/client_attributes.js b/lib/consts/client_attributes.js
@@ -164,7 +164,7 @@ const ENUM = {
   application_type: () => ['native', 'web'],
 };
 
-const noVSCHAR = /[^\x20-\x7E]/;
+export const noVSCHAR = /[^\x20-\x7E]/;
 // const noNQCHAR = /[^\x21\x23-\x5B\x5D-\x7E]/;
 // const noNQSCHAR = /[^\x20-\x21\x23-\x5B\x5D-\x7E]/;
 

diff --git a/lib/shared/token_auth.js b/lib/shared/token_auth.js
@@ -3,6 +3,7 @@ import setWWWAuthenticate from '../helpers/set_www_authenticate.js';
 import * as JWT from '../helpers/jwt.js';
 import instance from '../helpers/weak_cache.js';
 import certificateThumbprint from '../helpers/certificate_thumbprint.js';
+import { noVSCHAR } from '../consts/client_attributes.js';
 
 import rejectDupes from './reject_dupes.js';
 import getJWTAuthMiddleware from './token_jwt_auth.js';
@@ -11,7 +12,15 @@ const assertionType = 'urn:ietf:params:oauth:client-assertion-type:jwt-bearer';
 
 // see https://tools.ietf.org/html/rfc6749#appendix-B
 function decodeAuthToken(token) {
-  return decodeURIComponent(token.replace(/\+/g, '%20'));
+  // TODO: in v9.x consider enabling stricter encoding check
+  // if (token.match(/[^a-zA-Z0-9%+]/)) {
+  //   throw new Error();
+  // }
+  const authToken = decodeURIComponent(token.replace(/\+/g, '%20'));
+  if (noVSCHAR.test(authToken)) {
+    throw new Error('invalid character found');
+  }
+  return authToken;
 }
 
 export default function tokenAuth(provider) {

diff --git a/test/client_auth/client_auth.config.js b/test/client_auth/client_auth.config.js
@@ -145,5 +145,14 @@ export default {
     response_types: [],
     redirect_uris: [],
     client_secret_expires_at: 1,
+  },
+  // Appendix B
+  {
+    token_endpoint_auth_method: 'client_secret_basic',
+    client_id: ' %&+',
+    client_secret: ' %&+',
+    grant_types: ['foo'],
+    response_types: [],
+    redirect_uris: [],
   }],
 };
diff --git a/test/client_auth/client_auth.test.js b/test/client_auth/client_auth.test.js
@@ -281,13 +281,24 @@ describe('client authentication options', () => {
         });
     });
 
+    it('accepts the auth (https://tools.ietf.org/html/rfc6749#appendix-B)', function () {
+      return this.agent.post(route)
+        .send({
+          grant_type: 'foo',
+        })
+        .type('form')
+        .auth(' %&+', ' %&+')
+        .expect(200)
+        .expect(tokenAuthSucceeded);
+    });
+
     it('accepts the auth (https://tools.ietf.org/html/rfc6749#appendix-B again)', function () {
       return this.agent.post(route)
         .send({
           grant_type: 'foo',
         })
         .type('form')
-        .auth('an%3Aidentifier', 'some+secure+%26+non-standard+secret')
+        .auth('an:identifier', 'some secure & non-standard secret')
         .expect(200)
         .expect(tokenAuthSucceeded);
     });
@@ -298,7 +309,7 @@ describe('client authentication options', () => {
           grant_type: 'foo',
         })
         .type('form')
-        .auth('foo with %', 'foo with $')
+        .set('Authorization', `Basic ${btoa('foo with %:foo with $')}`)
         .expect({
           error: 'invalid_request',
           error_description: 'client_id and client_secret in the authorization header are not properly encoded',

diff --git a/test/test_helper.js b/test/test_helper.js
@@ -9,6 +9,7 @@ import { once } from 'node:events';
 import sinon from 'sinon';
 import { dirname } from 'desm';
 import flatten from 'lodash/flatten.js';
+import { Request } from 'superagent'; // eslint-disable-line import/no-extraneous-dependencies
 import { agent as supertest } from 'supertest';
 import { expect } from 'chai';
 import koaMount from 'koa-mount';
@@ -27,6 +28,45 @@ import instance from '../lib/helpers/weak_cache.js';
 import { Account, TestAdapter } from './models.js';
 import keys from './keys.js';
 
+const { _auth } = Request.prototype;
+
+function encodeToken(token) {
+  return encodeURIComponent(token).replace(/(?:[-_.!~*'()]|%20)/g, (substring) => {
+    switch (substring) {
+      case '-':
+        return '%2D';
+      case '_':
+        return '%5F';
+      case '.':
+        return '%2E';
+      case '!':
+        return '%21';
+      case '~':
+        return '%7E';
+      case '*':
+        return '%2A';
+      case "'":
+        return '%27';
+      case '(':
+        return '%28';
+      case ')':
+        return '%29';
+      case '%20':
+        return '+';
+      default:
+        throw new Error();
+    }
+  });
+}
+
+Request.prototype._auth = function (user, pass, options, encoder) {
+  if (options?.type === 'basic') {
+    return _auth.call(this, encodeToken(user), encodeToken(pass), options, encoder);
+  }
+
+  return _auth.call(this, user, pass, options, encoder);
+};
+
 process.env.NODE_ENV = process.env.NODE_ENV || 'test';
 
 global.i = instance;