fix: prevent callback from being called twice on payload claim conflict

When jwt.sign() was called with a callback and the payload had a
claim that conflicted with an option (e.g., payload.iss + options.issuer),
the callback was invoked twice: once with the error and once with
a signed token.

This happened because the options-to-payload mapping used forEach(),
where 'return failure()' only returned from the forEach callback,
not from the outer function. Execution continued to jws.createSign().

Replaced forEach with a for loop so 'return failure()' properly
exits the sign function.

Fixes #1000

Author: cobyfrombrooklyn-bot

sign.js

@@ -214,15 +214,17 @@ module.exports = function (payload, secretOrPrivateKey, options, callback) {
     }
   }
 
-  Object.keys(options_to_payload).forEach(function (key) {
+  const optionKeys = Object.keys(options_to_payload);
+  for (let i = 0; i < optionKeys.length; i++) {
+    const key = optionKeys[i];
     const claim = options_to_payload[key];
     if (typeof options[key] !== 'undefined') {
       if (typeof payload[claim] !== 'undefined') {
         return failure(new Error('Bad "options.' + key + '" option. The payload already has an "' + claim + '" property.'));
       }
       payload[claim] = options[key];
     }
-  });
+  }
 
   const encoding = options.encoding || 'utf8';
 

test/async_sign.tests.js

@@ -147,4 +147,31 @@ describe('signing a token asynchronously', function() {
       });
     });
   });
+
+  // Regression test for https://github.com/auth0/node-jsonwebtoken/issues/1000
+  describe('when payload has a claim that conflicts with options', function () {
+    it('should call the callback only once with an error', function (done) {
+      var callCount = 0;
+      jwt.sign(
+        { iss: 'bar', iat: Math.floor(Date.now() / 1000) },
+        'secret',
+        { algorithm: 'HS256', issuer: 'foo' },
+        function (err, token) {
+          callCount++;
+          if (callCount === 1) {
+            expect(err).to.be.an.instanceof(Error);
+            expect(err.message).to.match(/payload already has an "iss" property/);
+            expect(token).to.be.undefined;
+            // Wait a tick to ensure callback isn't called again
+            setTimeout(function () {
+              expect(callCount).to.equal(1);
+              done();
+            }, 10);
+          } else {
+            done(new Error('Callback was called ' + callCount + ' times, expected once'));
+          }
+        }
+      );
+    });
+  });
 });