Repository: auth0/auth0.js
Pull request #535 (2017-10-27 01:11:08)

Only generate nonce when needed
nonce was being generated and stored even though responseType wasn't requesting for an id_token, causing `nonce does not match` error. This PR fixes this behavior and only generates and stores a nonce when it actually needs to (responseType includes id_token).


Patch:
diff --git a/src/web-auth/transaction-manager.js b/src/web-auth/transaction-manager.js
index 78d6ff9..cd77a2d 100644
--- a/src/web-auth/transaction-manager.js
+++ b/src/web-auth/transaction-manager.js
@@ -13,12 +13,18 @@ TransactionManager.prototype.process = function(options) {
   if (!options.responseType) {
     throw new Error('responseType is required');
   }
-  var transaction = this.generateTransaction(options.appState, options.state, options.nonce);
+  var responseTypeIncludesIdToken = options.responseType.indexOf('id_token') !== -1;
+
+  var transaction = this.generateTransaction(
+    options.appState,
+    options.state,
+    options.nonce,
+    responseTypeIncludesIdToken
+  );
   if (!options.state) {
     options.state = transaction.state;
   }
 
-  var responseTypeIncludesIdToken = options.responseType.indexOf('id_token') !== -1;
   if (responseTypeIncludesIdToken && !options.nonce) {
     options.nonce = transaction.nonce;
   }
@@ -26,9 +32,9 @@ TransactionManager.prototype.process = function(options) {
   return options;
 };
 
-TransactionManager.prototype.generateTransaction = function(appState, state, nonce) {
+TransactionManager.prototype.generateTransaction = function(appState, state, nonce, generateNonce) {
   state = state || random.randomString(this.keyLength);
-  nonce = nonce || random.randomString(this.keyLength);
+  nonce = nonce || (generateNonce ? random.randomString(this.keyLength) : null);
 
   storage.setItem(this.namespace + state, {
     nonce: nonce,


Test patch:
diff --git a/test/web-auth/transaction-manager.test.js b/test/web-auth/transaction-manager.test.js
index 6027cb9..678935c 100644
--- a/test/web-auth/transaction-manager.test.js
+++ b/test/web-auth/transaction-manager.test.js
@@ -1,22 +1,28 @@
 var expect = require('expect.js');
 var stub = require('sinon').stub;
+var spy = require('sinon').spy;
 
 var TransactionManager = require('../../src/web-auth/transaction-manager');
 var objectHelper = require('../../src/helper/object');
+var random = require('../../src/helper/random');
+var storage = require('../../src/helper/storage');
 
 context('TransactionManager', function() {
-  before(function() {
-    stub(TransactionManager.prototype, 'generateTransaction', function(appState, state, nonce) {
-      return { state: state || 'randomState', nonce: nonce || 'randomNonce' };
-    });
-  });
-  after(function() {
-    TransactionManager.prototype.generateTransaction.restore();
-  });
   beforeEach(function() {
     this.tm = new TransactionManager();
   });
   context('process', function() {
+    beforeEach(function() {
+      stub(TransactionManager.prototype, 'generateTransaction', function(appState, state, nonce) {
+        return {
+          state: state || 'randomState',
+          nonce: nonce || 'randomNonce'
+        };
+      });
+    });
+    afterEach(function() {
+      TransactionManager.prototype.generateTransaction.restore();
+    });
     it('throws when responseType is not available', function() {
       var _this = this;
       expect(function() {
@@ -30,6 +36,20 @@ context('TransactionManager', function() {
       });
     });
     it('generates a nonce when responseType contains id_token and a nonce is not provided', function() {
+      TransactionManager.prototype.generateTransaction.restore();
+      stub(TransactionManager.prototype, 'generateTransaction', function(
+        appState,
+        state,
+        nonce,
+        generateNonce
+      ) {
+        expect(generateNonce).to.be(true);
+        console.log('asdlkhalsdhalskhd');
+        return {
+          state: state || 'randomState',
+          nonce: nonce || 'randomNonce'
+        };
+      });
       expect(this.tm.process({ responseType: 'code id_token', state: 'some-state' })).to.be.eql({
         responseType: 'code id_token',
         state: 'some-state',
@@ -46,4 +66,49 @@ context('TransactionManager', function() {
       });
     });
   });
+  context('generateTransaction', function() {
+    beforeEach(function() {
+      stub(random, 'randomString', function() {
+        return 'randomString';
+      });
+      stub(storage, 'setItem');
+    });
+    afterEach(function() {
+      random.randomString.restore();
+      storage.setItem.restore();
+    });
+    it('always stores random state', function() {
+      var result = this.tm.generateTransaction('appState', null, null, false);
+      expect(result).to.be.eql({ state: 'randomString', nonce: null });
+      expect(storage.setItem.calledOnce).to.be(true);
+      expect(storage.setItem.lastCall.args[0]).to.be('com.auth0.auth.randomString');
+      expect(storage.setItem.lastCall.args[1]).to.be.eql({
+        nonce: null,
+        appState: 'appState',
+        state: 'randomString'
+      });
+    });
+    it('only stores random nonce when generateNonce is true', function() {
+      var result = this.tm.generateTransaction('appState', null, null, true);
+      expect(result).to.be.eql({ state: 'randomString', nonce: 'randomString' });
+      expect(storage.setItem.calledOnce).to.be(true);
+      expect(storage.setItem.lastCall.args[0]).to.be('com.auth0.auth.randomString');
+      expect(storage.setItem.lastCall.args[1]).to.be.eql({
+        nonce: 'randomString',
+        appState: 'appState',
+        state: 'randomString'
+      });
+    });
+    it('uses nonce/state when provided', function() {
+      var result = this.tm.generateTransaction('appState', 'state', 'nonce');
+      expect(result).to.be.eql({ state: 'state', nonce: 'nonce' });
+      expect(storage.setItem.calledOnce).to.be(true);
+      expect(storage.setItem.lastCall.args[0]).to.be('com.auth0.auth.state');
+      expect(storage.setItem.lastCall.args[1]).to.be.eql({
+        nonce: 'nonce',
+        appState: 'appState',
+        state: 'state'
+      });
+    });
+  });
 });