Repository: auth0/node-auth0
Pull request #374 (2019-07-08 17:50:10)

Encode id param on GET
Fix https://github.com/auth0/node-auth0/issues/296

### Changes

When hitting the management API in endpoints like `https://example.auth0.com/api/v2/resource/:id`, some of the IDs might need to be encoded, like user ids etc. This PR is a general solution for this issue and it will encode all `id` params sent to GET requests. To make sure we're backwards compatible and we don't encode something that is already encoded, we're doing a check before encoding the value. This means that:
- if the `id` param is encoded, we'll do nothing
- if the `id` param is not encoded, we'll encode it

### References

https://github.com/auth0/node-auth0/issues/296

### Testing

- [x] This change adds unit test coverage


Patch:
diff --git a/src/Auth0RestClient.js b/src/Auth0RestClient.js
index d0224a5e7..48ee840a1 100644
--- a/src/Auth0RestClient.js
+++ b/src/Auth0RestClient.js
@@ -2,6 +2,8 @@ var RestClient = require('rest-facade').Client;
 var Promise = require('bluebird');
 var ArgumentError = require('rest-facade').ArgumentError;
 
+var utils = require('./utils');
+
 var Auth0RestClient = function(resourceUrl, options, provider) {
   if (resourceUrl === null || resourceUrl === undefined) {
     throw new ArgumentError('Must provide a Resource Url');
@@ -45,12 +47,15 @@ var Auth0RestClient = function(resourceUrl, options, provider) {
   };
 };
 
-Auth0RestClient.prototype.getAll = function(/* [params], [callback] */) {
+Auth0RestClient.prototype.getAll = function(params, callback) {
   return this.wrappedProvider('getAll', arguments);
 };
 
-Auth0RestClient.prototype.get = function(/* [params], [callback] */) {
-  return this.wrappedProvider('get', arguments);
+Auth0RestClient.prototype.get = function(params, callback) {
+  if (typeof params === 'object' && params.id) {
+    params.id = utils.maybeDecode(`${params.id}`);
+  }
+  return this.wrappedProvider('get', [...[params, callback].filter(Boolean)]);
 };
 
 Auth0RestClient.prototype.create = function(/* [params], [callback] */) {
diff --git a/src/utils.js b/src/utils.js
index 59ecb09b2..79f30821b 100644
--- a/src/utils.js
+++ b/src/utils.js
@@ -90,3 +90,15 @@ utils.getRequestPromise = function(settings) {
     );
   });
 };
+
+utils.containsUnsafeChars = s => {
+  const safeChars = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ$-_.+!*'(),%";
+  return !!s.split('').find(c => !safeChars.includes(c));
+};
+
+utils.maybeDecode = url => {
+  if (utils.containsUnsafeChars(url)) {
+    return encodeURIComponent(url);
+  }
+  return url;
+};


Test patch:
diff --git a/test/auth0-rest-client.tests.js b/test/auth0-rest-client.tests.js
index 96844d6c7..d9adc0cc7 100644
--- a/test/auth0-rest-client.tests.js
+++ b/test/auth0-rest-client.tests.js
@@ -30,19 +30,67 @@ describe('Auth0RestClient', function() {
     expect(client).to.throw(ArgumentError, 'Must provide options');
   });
 
-  it('should accept a callback', function(done) {
-    nock(API_URL)
-      .get('/some-resource')
-      .reply(200, { data: 'value' });
-
-    var options = {
-      headers: {}
-    };
-    var client = new Auth0RestClient(API_URL + '/some-resource', options, this.providerMock);
-    client.getAll(function(err, data) {
-      expect(data).to.deep.equal({ data: 'value' });
-      done();
-      nock.cleanAll();
+  describe('`get`', () => {
+    it('should encode params.id on `get` requests', function(done) {
+      nock(API_URL)
+        .get('/some-resource/auth0%7C1234')
+        .reply(200, { data: 'value' });
+
+      var options = {
+        headers: {}
+      };
+      var client = new Auth0RestClient(API_URL + '/some-resource/:id', options, this.providerMock);
+      client.get({ id: 'auth0|1234' }, function(err, data) {
+        expect(data).to.deep.equal({ data: 'value' });
+        done();
+        nock.cleanAll();
+      });
+    });
+    it('should not encode other params on `get` requests', function(done) {
+      nock(API_URL)
+        .get('/some-resource/1234?otherEncoded=auth0%7C6789&other=foobar')
+        .reply(200, { data: 'value' });
+
+      var options = {
+        headers: {}
+      };
+      var client = new Auth0RestClient(API_URL + '/some-resource/:id', options, this.providerMock);
+      client.get({ id: '1234', other: 'foobar', otherEncoded: 'auth0|6789' }, function(err, data) {
+        expect(err).to.be.null;
+        expect(data).to.deep.equal({ data: 'value' });
+        done();
+        nock.cleanAll();
+      });
+    });
+    it('should accept only a callback', function(done) {
+      nock(API_URL)
+        .get('/some-resource')
+        .reply(200, { data: 'value' });
+
+      var options = {
+        headers: {}
+      };
+      var client = new Auth0RestClient(API_URL + '/some-resource', options, this.providerMock);
+      client.get({}, function(err, data) {
+        expect(data).to.deep.equal({ data: 'value' });
+        done();
+        nock.cleanAll();
+      });
+    });
+    it('should return a promise if no callback is provided', function(done) {
+      nock(API_URL)
+        .get('/some-resource')
+        .reply(200, { data: 'value' });
+
+      var options = {
+        headers: {}
+      };
+      var client = new Auth0RestClient(API_URL + '/some-resource', options, this.providerMock);
+      client.get().then(data => {
+        expect(data).to.deep.equal({ data: 'value' });
+        done();
+        nock.cleanAll();
+      });
     });
   });
 
diff --git a/test/utils.tests.js b/test/utils.tests.js
index 919b8ae01..5fcbaf17f 100644
--- a/test/utils.tests.js
+++ b/test/utils.tests.js
@@ -13,4 +13,22 @@ describe('Utils', function() {
       });
     });
   });
+  describe('maybe decode', () => {
+    it('encodes regular user_id', () => {
+      const normalId = 'auth0|1234';
+      expect(utils.maybeDecode(normalId)).to.be.equal('auth0%7C1234');
+    });
+    it('encodes a malicious user_id', () => {
+      const maliciousId = 'anotherUserId/secret';
+      expect(utils.maybeDecode(maliciousId)).to.be.equal('anotherUserId%2Fsecret');
+    });
+    it('encodes a malicious user_id with a percentage', () => {
+      const maliciousId = 'anotherUserId/secret%23';
+      expect(utils.maybeDecode(maliciousId)).to.be.equal('anotherUserId%2Fsecret%2523');
+    });
+    it('does not encode an already encoded user_id', () => {
+      const maliciousId = 'auth0%7C1234';
+      expect(utils.maybeDecode(maliciousId)).to.be.equal('auth0%7C1234');
+    });
+  });
 });