feat: check client_id and client_secret ABNF syntax

BREAKING CHANGE: `client_id` and `client_secret` values are now checked to conform to their ABNF syntax (%x20-7E).
panva · Dec 15, 2020 · 3d0d078 · 3d0d078
1 parent 33223ff
commit 3d0d078
Show file tree

Hide file tree

Showing 5 changed files with 23 additions and 15 deletions.
diff --git a/lib/consts/client_attributes.js b/lib/consts/client_attributes.js
@@ -159,6 +159,15 @@ const ENUM = {
   application_type: () => ['native', 'web'],
 };
 
+const noVSCHAR = /[^\x20-\x7E]/;
+// const noNQCHAR = /[^\x21\x23-\x5B\x5D-\x7E]/;
+// const noNQSCHAR = /[^\x20-\x21\x23-\x5B\x5D-\x7E]/;
+
+const SYNTAX = {
+  client_id: noVSCHAR,
+  client_secret: noVSCHAR,
+};
+
 module.exports = {
   ARYS,
   BOOL,
@@ -169,6 +178,7 @@ module.exports = {
   RECOGNIZED_METADATA,
   REQUIRED,
   STRING,
+  SYNTAX,
   WEB_URI,
   WHEN,
 };
diff --git a/lib/helpers/client_schema.js b/lib/helpers/client_schema.js
@@ -11,6 +11,7 @@ const {
     RECOGNIZED_METADATA: RECOGNIZED,
     REQUIRED,
     STRING,
+    SYNTAX,
     WEB_URI,
     WHEN,
   },
@@ -256,6 +257,7 @@ module.exports = function getSchema(provider) {
       this.whens();
       this.arrays();
       this.strings();
+      this.syntax();
       this.normalizeResponseTypes();
       this.enums();
       this.webUris();
@@ -643,6 +645,15 @@ module.exports = function getSchema(provider) {
         this.scope = [...parsed].join(' ');
       }
     }
+
+    syntax() {
+      // eslint-disable-next-line no-restricted-syntax
+      for (const [prop, regexp] of Object.entries(SYNTAX)) {
+        if (regexp.exec(this[prop])) {
+          this.invalidate(`invalid ${prop} value`);
+        }
+      }
+    }
   }
 
   return Schema;

diff --git a/test/client_auth/client_auth.config.js b/test/client_auth/client_auth.config.js
@@ -59,11 +59,6 @@ module.exports = {
     client_id: 'an:identifier',
     client_secret: 'some secure & non-standard secret',
     redirect_uris: ['https://client.example.com/cb'],
-  }, {
-    token_endpoint_auth_method: 'client_secret_basic',
-    client_id: ' %&+£€',
-    client_secret: ' %&+£€',
-    redirect_uris: ['https://client.example.com/cb'],
   }, {
     token_endpoint_auth_method: 'client_secret_post',
     client_id: 'client-post',

diff --git a/test/client_auth/client_auth.test.js b/test/client_auth/client_auth.test.js
@@ -280,16 +280,6 @@ 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: 'implicit',
-        })
-        .type('form')
-        .auth('+%25%26%2B%C2%A3%E2%82%AC', '+%25%26%2B%C2%A3%E2%82%AC')
-        .expect(tokenAuthSucceeded);
-    });
-
     it('accepts the auth (https://tools.ietf.org/html/rfc6749#appendix-B again)', function () {
       return this.agent.post(route)
         .send({

diff --git a/test/configuration/client_metadata.test.js b/test/configuration/client_metadata.test.js
@@ -198,6 +198,7 @@ describe('Client metadata validation', () => {
     mustBeString(this.title, [123, {}, [], true]);
 
     allows(this.title, 'whatever client id');
+    rejects(this.title, '£', 'invalid client_id value');
   });
 
   context('client_name', function () {
@@ -209,6 +210,7 @@ describe('Client metadata validation', () => {
     isRequired(this.title);
     mustBeString(this.title, [123, {}, [], true]);
     allows(this.title, 'whatever client secret');
+    rejects(this.title, '£', 'invalid client_secret value');
     // must of certain length => GOTO: client_secrets.test.js
   });