Add 'expireEarly' option. Document 'maxClockSkew' option (#220)

* Document 'maxClockSkew' option

improve test

* make clock / offset internal

* expireEarly option

* util.extend, update docs, test

* util.extend again

* expireEarlySeconds

* util.extend should behave as Object.assign

* fix extend test

README.md

@@ -146,7 +146,6 @@ tokenManager: {
 }
 ```
 
-
 By default, the `tokenManager` will attempt to renew expired tokens. When an expired token is requested by the `tokenManager.get()` method, a renewal request is executed to update the token. If you wish to manually control token renewal, set `autoRenew` to false to disable this feature. You can listen to  [`expired`](#tokenmanageronevent-callback-context) events to know when the token has expired.
 
 ```javascript
@@ -155,6 +154,16 @@ tokenManager: {
 }
 ```
 
+Renewing tokens slightly early helps ensure a stable user experience. By default, the `expired` event will fire 30 seconds before actual expiration time. If `autoRenew` is set to true, tokens will be renewed within 30 seconds of expiration, if accessed with `tokenManager.get()`. You can customize this value by setting the `expireEarlySeconds` option. The value should be large enough to account for network latency between the client and Okta's servers.
+
+```javascript
+// Emit expired event 2 minutes before expiration
+// Tokens accessed with tokenManager.get() will auto-renew within 2 minutes of expiration
+tokenManager: {
+  expireEarlySeconds: 120
+}
+```
+
 #### Additional Options
 
 | Option | Description |
@@ -168,6 +177,7 @@ tokenManager: {
 | `tokenUrl`  | Specify a custom tokenUrl. Defaults to the issuer plus "/v1/token". |
 | `ignoreSignature` | ID token signatures are validated by default when `token.getWithoutPrompt`, `token.getWithPopup`,  `token.getWithRedirect`, and `token.verify` are called. To disable ID token signature validation for these methods, set this value to `true`. |
 | | This option should be used only for browser support and testing purposes. |
+| `maxClockSkew` | Defaults to 300 (five minutes). This is the maximum difference allowed between a client's clock and Okta's, in seconds, when validating tokens. Setting this to 0 is not recommended, because it increases the likelihood that valid tokens will fail validation.
 
 ##### Example Client
 

lib/TokenManager.js

@@ -19,6 +19,23 @@ var Q = require('q');
 var Emitter = require('tiny-emitter');
 var config = require('./config');
 var storageBuilder = require('./storageBuilder');
+var SdkClock = require('./clock');
+
+var DEFAULT_OPTIONS = {
+  autoRenew: true,
+  storage: 'localStorage',
+  expireEarlySeconds: 30
+};
+
+function getExpireTime(tokenMgmtRef, token) {
+  var expireTime = token.expiresAt - tokenMgmtRef.options.expireEarlySeconds;
+  return expireTime;
+}
+
+function hasExpired(tokenMgmtRef, token) {
+  var expireTime = getExpireTime(tokenMgmtRef, token);
+  return expireTime <= tokenMgmtRef.clock.now();
+}
 
 function emitExpired(tokenMgmtRef, key, token) {
   tokenMgmtRef.emitter.emit('expired', key, token);
@@ -47,7 +64,8 @@ function clearExpireEventTimeoutAll(tokenMgmtRef) {
 }
 
 function setExpireEventTimeout(sdk, tokenMgmtRef, key, token) {
-  var expireEventWait = Math.max(token.expiresAt - sdk.clock.now(), 0) * 1000;
+  var expireTime = getExpireTime(tokenMgmtRef, token);
+  var expireEventWait = Math.max(expireTime - tokenMgmtRef.clock.now(), 0) * 1000;
 
   // Clear any existing timeout
   clearExpireEventTimeout(tokenMgmtRef, key);
@@ -100,11 +118,11 @@ function get(storage, key) {
 function getAsync(sdk, tokenMgmtRef, storage, key) {
   return Q.Promise(function(resolve) {
     var token = get(storage, key);
-    if (!token || token.expiresAt > sdk.clock.now()) {
+    if (!token || !hasExpired(tokenMgmtRef, token)) {
       return resolve(token);
     }
 
-    var tokenPromise = tokenMgmtRef.autoRenew
+    var tokenPromise = tokenMgmtRef.options.autoRenew
       ? renew(sdk, tokenMgmtRef, storage, key)
       : remove(tokenMgmtRef, storage, key);
 
@@ -179,11 +197,7 @@ function clear(tokenMgmtRef, storage) {
 }
 
 function TokenManager(sdk, options) {
-  options = options || {};
-  options.storage = options.storage || 'localStorage';
-  if (!options.autoRenew && options.autoRenew !== false) {
-    options.autoRenew = true;
-  }
+  options = util.extend({}, DEFAULT_OPTIONS, util.removeNils(options));
 
   if (options.storage === 'localStorage' && !storageUtil.browserHasLocalStorage()) {
     util.warn('This browser doesn\'t support localStorage. Switching to sessionStorage.');
@@ -210,9 +224,11 @@ function TokenManager(sdk, options) {
       throw new AuthSdkError('Unrecognized storage option');
   }
 
+  var clock = SdkClock.create(sdk, options);
   var tokenMgmtRef = {
+    clock: clock,
+    options: options,
     emitter: new Emitter(),
-    autoRenew: options.autoRenew,
     expireTimeouts: {},
     renewPromise: {}
   };

lib/browser/browser.js

@@ -25,7 +25,6 @@ var session           = require('../session');
 var token             = require('../token');
 var TokenManager      = require('../TokenManager');
 var tx                = require('../tx');
-var clock             = require('../clock');
 var util              = require('../util');
 
 function OktaAuthBuilder(args) {
@@ -66,12 +65,6 @@ function OktaAuthBuilder(args) {
     this.options.maxClockSkew = args.maxClockSkew;
   }
 
-  // Calculated local clock offset from server time (in milliseconds). Can be positive or negative.
-  this.options.localClockOffset = args.localClockOffset || 0;
-  sdk.clock = {
-    now: util.bind(clock.getLocalAdjustedTime, null, sdk)
-  };
-
   // Give the developer the ability to disable token signature
   // validation.
   this.options.ignoreSignature = !!args.ignoreSignature;

lib/clock.js

@@ -1,9 +1,23 @@
-function getLocalAdjustedTime(sdk) {
-  var localClockOffset = parseInt(sdk.options.localClockOffset || 0);
-  var now = (Date.now() + localClockOffset) / 1000;
-  return now;
+var util = require('./util');
+
+function SdkClock(localOffset) {
+  // Calculated local clock offset from server time (in milliseconds). Can be positive or negative.
+  this.localOffset = parseInt(localOffset || 0);
 }
 
-module.exports = {
-  getLocalAdjustedTime: getLocalAdjustedTime
+util.extend(SdkClock.prototype, {
+  // Return the current time (in seconds)
+  now: function() {
+    var now = (Date.now() + this.localOffset) / 1000;
+    return now;
+  }
+});
+
+// factory method. Create an instance of a clock from current context.
+SdkClock.create = function(/* sdk, options */) {
+  // TODO: calculate localOffset
+  var localOffset = 0;
+  return new SdkClock(localOffset);
 };
+
+module.exports = SdkClock;

lib/util.js

@@ -127,15 +127,19 @@ util.genRandomString = function(length) {
 };
 
 util.extend = function() {
+  // First object will be modified!
   var obj1 = arguments[0];
+  // Properties from other objects will be copied over
   var objArray = [].slice.call(arguments, 1);
   objArray.forEach(function(obj) {
     for (var prop in obj) {
-      if (obj.hasOwnProperty(prop)) {
+      // copy over all properties with defined values
+      if (obj.hasOwnProperty(prop) && obj[prop] !== undefined) {
         obj1[prop] = obj[prop];
       }
     }
   });
+  return obj1; // return the modified object
 };
 
 util.removeNils = function(obj) {

test/spec/clock.js

@@ -1,77 +1,62 @@
-var clock = require('../../lib/clock');
+var SdkClock = require('../../lib/clock');
 
 describe('clock', function() {
-
-  describe('getLocalAdjustedTime', function() {
-    it('returns the local time / 1000', function() {
-      var fakeDate = 4200;
-      jest.spyOn(Date, 'now').mockReturnValue(fakeDate);
-      var sdk = {
-        options: {
-          localClockOffset: 0
-        }
-      };
-      expect(clock.getLocalAdjustedTime(sdk)).toBe(fakeDate / 1000);
-    });
-
-    it('can have a positive offset', function() {
-      var fakeDate = 4200;
-      jest.spyOn(Date, 'now').mockReturnValue(fakeDate);
-      var offset = 2300;
-      var sdk = {
-        options: {
-          localClockOffset: offset
-        }
-      };
-      expect(clock.getLocalAdjustedTime(sdk)).toBe((fakeDate + offset) / 1000);
-    });
-
-    it('can have a negative offset', function() {
-      var fakeDate = 4200;
-      jest.spyOn(Date, 'now').mockReturnValue(fakeDate);
-      var offset = -2300;
-      var sdk = {
-        options: {
-          localClockOffset: offset
-        }
-      };
-      expect(clock.getLocalAdjustedTime(sdk)).toBe((fakeDate + offset) / 1000);
-    });
-
-    it('returns a valid number even if offset is a string', function() {
-      var fakeDate = 4200;
-      jest.spyOn(Date, 'now').mockReturnValue(fakeDate);
-      var sdk = {
-        options: {
-          localClockOffset: "0"
-        }
-      };
-      expect(clock.getLocalAdjustedTime(sdk)).toBe(fakeDate / 1000);
+  describe('create', function() {
+    it('returns an instance of SdkClock', function() {
+      expect(SdkClock.create() instanceof SdkClock).toBe(true);
     });
+  });
+
+  describe('SdkClock', function() {
+    describe('now', function() {
+      it('returns the local time / 1000', function() {
+        var fakeDate = 4200;
+        jest.spyOn(Date, 'now').mockReturnValue(fakeDate);
+        var offset = 0;
+        var clock = new SdkClock(offset);
+        expect(clock.now()).toBe(fakeDate / 1000);
+      });
+
+      it('can have a positive offset', function() {
+        var fakeDate = 4200;
+        jest.spyOn(Date, 'now').mockReturnValue(fakeDate);
+        var offset = 2300;
+        var clock = new SdkClock(offset);
+        expect(clock.now()).toBe((fakeDate + offset) / 1000);
+      });
+
+      it('can have a negative offset', function() {
+        var fakeDate = 4200;
+        jest.spyOn(Date, 'now').mockReturnValue(fakeDate);
+        var offset = -2300;
+        var clock = new SdkClock(offset);
+        expect(clock.now()).toBe((fakeDate + offset) / 1000);
+      });
+
+      it('returns a valid number even if offset is a string', function() {
+        var fakeDate = 4200;
+        jest.spyOn(Date, 'now').mockReturnValue(fakeDate);
+        var offset = "0";
+        var clock = new SdkClock(offset);
+        expect(clock.now()).toBe(fakeDate / 1000);
+      });
+
+      it('returns a valid number even if offset is not set', function() {
+        var fakeDate = 4200;
+        jest.spyOn(Date, 'now').mockReturnValue(fakeDate);
+        var offset = null;
+        var clock = new SdkClock(offset);
+        expect(clock.now()).toBe(fakeDate / 1000);
+      });
+
+      it('will be NaN if offset is NaN', function() {
+        var fakeDate = 4200;
+        jest.spyOn(Date, 'now').mockReturnValue(fakeDate);
+        var offset = "definitelyNotanumber";
+        var clock = new SdkClock(offset);
+        expect(clock.now()).toBe(Number.NaN);
+      });
 
-
-    it('returns a valid number even if offset is not set', function() {
-      var fakeDate = 4200;
-      jest.spyOn(Date, 'now').mockReturnValue(fakeDate);
-      var sdk = {
-        options: {
-
-        }
-      };
-      expect(clock.getLocalAdjustedTime(sdk)).toBe(fakeDate / 1000);
-    });
-
-
-    it('will be NaN if offset is NaN', function() {
-      var fakeDate = 4200;
-      jest.spyOn(Date, 'now').mockReturnValue(fakeDate);
-      var sdk = {
-        options: {
-          localClockOffset: "definitelyNotanumber"
-        }
-      };
-      expect(clock.getLocalAdjustedTime(sdk)).toBe(Number.NaN);
     });
-
   });
 })