Fix JWT retry logic (#461)

Author: sujaygarlanka

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## Upcoming Release
 
 - Added a getter method (getNextMarker()) for the next marker to PagingIterator
+- Fixed JWT retry logic so the a new JTI claim is generated on each retry
 
 ## 1.30.0 [2019-11-21]
 

lib/api-request.js

@@ -15,7 +15,8 @@ var assert = require('assert'),
 	request = require('request'),
 	EventEmitter = require('events').EventEmitter,
 	Config = require('./util/config'),
-	httpStatusCodes = require('http-status');
+	httpStatusCodes = require('http-status'),
+	getRetryTimeout = require('./util/exponential-backoff');
 
 // ------------------------------------------------------------------------------
 // Typedefs and Callbacks
@@ -94,9 +95,6 @@ var retryableStatusCodes = {};
 retryableStatusCodes[httpStatusCodes.REQUEST_TIMEOUT] = true;
 retryableStatusCodes[httpStatusCodes.TOO_MANY_REQUESTS] = true;
 
-// Retry intervals are between 50% and 150% of the exponentially increasing base amount
-const RETRY_RANDOMIZATION_FACTOR = 0.5;
-
 /**
  * Returns true if the response info indicates a temporary/transient error.
  *
@@ -157,21 +155,6 @@ function cleanSensitiveHeaders(requestObj) {
 	}
 }
 
-/**
- * Calculate the exponential backoff time with randomized jitter
- * @param {int} numRetries Which retry number this one will be
- * @param {int} baseInterval The base retry interval set in config
- * @returns {int} The number of milliseconds after which to retry
- */
-function getRetryTimeout(numRetries, baseInterval) {
-
-	var minRandomization = 1 - RETRY_RANDOMIZATION_FACTOR;
-	var maxRandomization = 1 + RETRY_RANDOMIZATION_FACTOR;
-	var randomization = (Math.random() * (maxRandomization - minRandomization)) + minRandomization;
-	var exponential = Math.pow(2, numRetries - 1);
-	return Math.ceil(exponential * baseInterval * randomization);
-}
-
 // ------------------------------------------------------------------------------
 // Public
 // ------------------------------------------------------------------------------
@@ -264,8 +247,12 @@ APIRequest.prototype._handleResponse = function(err, response) {
 		// Have the SDK emit the error response
 		this.eventBus.emit('response', err);
 
-		// If our APIRequest instance is retryable, attempt a retry. Otherwise, finish and propagate the error.
-		if (this.isRetryable) {
+		var isJWT = false;
+		if (this.config.request.hasOwnProperty('form') && this.config.request.form.hasOwnProperty('grant_type') && this.config.request.form.grant_type === 'urn:ietf:params:oauth:grant-type:jwt-bearer') {
+			isJWT = true;
+		}
+		// If our APIRequest instance is retryable, attempt a retry. Otherwise, finish and propagate the error. Doesn't retry when the request is for JWT authentication, since that is handled in retryJWTGrant.
+		if (this.isRetryable && !isJWT) {
 			this._retry(err);
 		} else {
 			this._finish(err);
@@ -318,6 +305,8 @@ APIRequest.prototype._retry = function(err) {
 				this._finish(err);
 				return;
 			}
+		} else if (err.response.hasOwnProperty('headers') && err.response.headers.hasOwnProperty('retry-after')) {
+			retryTimeout = err.response.headers['retry-after'] * 1000;
 		} else {
 			retryTimeout = getRetryTimeout(this.numRetries, this.config.retryIntervalMS);
 		}

lib/token-manager.js

@@ -53,12 +53,12 @@
   */
 function isJWTAuthErrorRetryable(err) {
 
-	return err.authExpired
-		&& err.response.headers.date
-		&& (
-			err.response.body.error_description.indexOf('exp') > -1
-				|| err.response.body.error_description.indexOf('jti') > -1
-		);
+	if (err.authExpired && err.response.headers.date && (err.response.body.error_description.indexOf('exp') > -1 || err.response.body.error_description.indexOf('jti') > -1)) {
+		return true;
+	} else if (err.statusCode === 429 || err.statusCode >= 500) {
+		return true;
+	}
+	return false;
 }
 
 // ------------------------------------------------------------------------------
@@ -68,7 +68,8 @@ var errors = require('./util/errors'),
 	jwt = require('jsonwebtoken'),
 	uuid = require('uuid'),
 	httpStatusCodes = require('http-status'),
-	Promise = require('bluebird');
+	Promise = require('bluebird'),
+	getRetryTimeout = require('./util/exponential-backoff');
 
 // ------------------------------------------------------------------------------
 // Constants
@@ -100,6 +101,9 @@ var tokenPaths = {
 	REVOKE: '/revoke'
 };
 
+// Timer used to track elapsed time starting with executing an async request and ending with emitting the response.
+var asyncRequestTimer;
+
 // The XFF header label - Used to give the API better information for uploads, rate-limiting, etc.
 const HEADER_XFF = 'X-Forwarded-For';
 const ACCESS_TOKEN_TYPE = 'urn:ietf:params:oauth:token-type:access_token';
@@ -345,37 +349,92 @@ TokenManager.prototype = {
 		try {
 			assertion = jwt.sign(claims, keyParams, jwtOptions);
 		} catch (jwtErr) {
-
 			return Promise.reject(jwtErr);
 		}
 
 		var params = {
 			grant_type: grantTypes.JWT,
 			assertion
 		};
+		// Start the request timer immediately before executing the async request
+		asyncRequestTimer = process.hrtime();
 		return this.getTokens(params, options)
-			.catch(err => {
-
-				// When a client's clock is out of sync with Box API servers, they'll get an error about the exp claim
-				// In these cases, we can attempt to retry the grant request with a new exp claim calculated frem the
-				// Date header sent by the server
-				if (isJWTAuthErrorRetryable(err)) {
-
-					var serverTime = Math.floor(Date.parse(err.response.headers.date) / 1000);
-					claims.exp = serverTime + this.config.appAuth.expirationTime;
-					jwtOptions.jwtid = uuid.v4();
+			.catch(err => this.retryJWTGrant(claims, jwtOptions, keyParams, params, options, err, 0));
+	},
 
-					try {
-						params.assertion = jwt.sign(claims, keyParams, jwtOptions);
-					} catch (jwtErr) {
-						throw jwtErr;
+	/**
+	 * Attempt a retry if possible and create a new JTI claim. If the request hasn't exceeded it's maximum number of retries,
+	 * re-execute the request (after the retry interval). Otherwise, propagate a new error.
+	 *
+	 * @param {Object} claims - JTI claims object
+	 * @param {Object} [jwtOptions] - JWT options for the signature
+	 * @param {Object} keyParams - Key JWT parameters object that contains the private key and the passphrase
+	 * @param {Object} params - Should contain all params expected by Box OAuth2 token endpoint
+	 * @param {TokenRequestOptions} [options] - Sets optional behavior for the token grant
+	 * @param {Error} error - Error from the previous JWT request
+	 * @param {int} numRetries - Number of retries attempted
+	 * @returns {Promise<TokenInfo>} Promise resolving to the token info
+	 */
+	// eslint-disable-next-line max-params
+	retryJWTGrant(claims, jwtOptions, keyParams, params, options, error, numRetries) {
+		if (numRetries < this.config.numMaxRetries && isJWTAuthErrorRetryable(error)) {
+			var retryTimeout;
+			numRetries += 1;
+			// If the retry strategy is defined, then use it to determine the time (in ms) until the next retry or to
+			// propagate an error to the user.
+			if (this.config.retryStrategy) {
+				// Get the total elapsed time so far since the request was executed
+				var totalElapsedTime = process.hrtime(asyncRequestTimer);
+				var totalElapsedTimeMS = (totalElapsedTime[0] * 1000) + (totalElapsedTime[1] / 1000000);
+				var retryOptions = {
+					error,
+					numRetryAttempts: numRetries,
+					numMaxRetries: this.config.numMaxRetries,
+					retryIntervalMS: this.config.retryIntervalMS,
+					totalElapsedTimeMS
+				};
+
+				retryTimeout = this.config.retryStrategy(retryOptions);
+
+				// If the retry strategy doesn't return a number/time in ms, then propagate the response error to the user.
+				// However, if the retry strategy returns its own error, this will be propagated to the user instead.
+				if (typeof retryTimeout !== 'number') {
+					if (retryTimeout instanceof Error) {
+						error = retryTimeout;
 					}
-
-					return this.getTokens(params, options);
+					throw error;
 				}
+			} else if (error.response.headers.hasOwnProperty('retry-after')) {
+				retryTimeout = error.response.headers['retry-after'] * 1000;
+			} else {
+				retryTimeout = getRetryTimeout(numRetries, this.config.retryIntervalMS);
+			}
+
+			var time = Math.floor(Date.now() / 1000);
+			if (error.response.headers.date) {
+				time = Math.floor(Date.parse(error.response.headers.date) / 1000);
+			}
+			// Add length of retry timeout to current expiration time to calculate the expiration time for the JTI claim.
+			claims.exp = time + this.config.appAuth.expirationTime + (retryTimeout / 1000);
+			jwtOptions.jwtid = uuid.v4();
 
-				throw err;
+			try {
+				params.assertion = jwt.sign(claims, keyParams, jwtOptions);
+			} catch (jwtErr) {
+				throw jwtErr;
+			}
+
+			return Promise.delay(retryTimeout).then(() => {
+				// Start the request timer immediately before executing the async request
+				asyncRequestTimer = process.hrtime();
+				return this.getTokens(params, options)
+					.catch(err => this.retryJWTGrant(claims, jwtOptions, keyParams, params, options, err, numRetries));
 			});
+		} else if (numRetries >= this.config.numMaxRetries) {
+			error.maxRetriesExceeded = true;
+		}
+
+		throw error;
 	},
 
 	/**

lib/util/exponential-backoff.js

@@ -0,0 +1,29 @@
+/**
+ * @fileoverview Calculate exponential backoff time
+ */
+
+'use strict';
+
+// ------------------------------------------------------------------------------
+// Private
+// ------------------------------------------------------------------------------
+
+// Retry intervals are between 50% and 150% of the exponentially increasing base amount
+const RETRY_RANDOMIZATION_FACTOR = 0.5;
+
+/**
+ * Calculate the exponential backoff time with randomized jitter
+ * @param {int} numRetries Which retry number this one will be
+ * @param {int} baseInterval The base retry interval set in config
+ * @returns {int} The number of milliseconds after which to retry
+ */
+function getRetryTimeout(numRetries, baseInterval) {
+
+	var minRandomization = 1 - RETRY_RANDOMIZATION_FACTOR;
+	var maxRandomization = 1 + RETRY_RANDOMIZATION_FACTOR;
+	var randomization = (Math.random() * (maxRandomization - minRandomization)) + minRandomization;
+	var exponential = Math.pow(2, numRetries - 1);
+	return Math.ceil(exponential * baseInterval * randomization);
+}
+
+module.exports = getRetryTimeout;

tests/lib/api-request-manager-test.js

@@ -98,6 +98,7 @@ describe('APIRequestManager', function() {
 		});
 
 	});
+
 	describe('makeRequest()', function() {
 
 		it('should pass the given options to the APIRequest constructor when called', function() {