Repository: cloudant/nodejs-cloudant (js)

Issue:
Retry 5xx/429s token service requests in IAM plugin.
- __Token Service (https://iam.cloud.ibm.com/identity/token)__
Retry `5xx` and `429`.

- __`/_iam_session`__
Retry `5xx` (endpoint will _not_ return 429 response).

Pull request:
Retry bad IAM token requests.
<!--
Thanks for your hard work, please ensure all items are complete before opening.
-->
## Checklist

- [x] Tick to sign-off your agreement to the [Developer Certificate of Origin (DCO) 1.1](../blob/master/DCO1.1.txt)
- [x] Added tests for code changes _or_ test/build only changes
- [x] Updated the change log file (`CHANGES.md`|`CHANGELOG.md`) _or_ test/build only changes
- [x] Completed the PR template below:

## Description
Retry bad IAM token requests.

Fixes #385.
<!--
Provide a short description; saving the detail for the `Approach` section

Also EITHER:
Link to issue this PR is resolving, use the Fixes #nnn form so that the
issue closes automatically when the PR merges e.g.:

Fixes #23

OR

For PRs without an associated issue and/or test/build issues

### 1. Steps to reproduce and the simplest code sample possible to demonstrate the issue
### 2. What you expected to happen
### 3. What actually happened
-->

## Approach
Catch `5xx` & `429` status codes and invoke retry mechanism (by _not_ setting `shouldApplyIAMAuth` to `false`).
<!--
Be brief: which component(s) of the code base does the fix focus on.

A place to note whether the part of the code base that is being worked is
particularly sensitive.
-->

## Schema & API Changes
No change.
<!--
EITHER:

- "No change"

OR

For public API (as opposed to internal) changes

- "Fixing bug in API, will change x in such-and-such way"
-->

## Security and Privacy
No change.
<!--
EITHER:

- "No change"

OR

"Making changes in e.g. auth|https|encryption|io
need to be careful about..."

-->

## Testing
Updated unit tests.
<!--
EITHER:

- Added new tests:
    - test x
    - test y
    - test z

OR

- Modified existing tests because ...

OR

- N/A build or packaging only changes

OR

In exceptional circumstances there may be a good reason we can't add automated
tests, for example if a specific device is required to reproduce a problem.

- No new tests because...
-->

## Monitoring and Logging
No change.
<!--
EITHER:

- "No change"

OR

- "Added new log line X..."
-->


Patch:
diff --git a/CHANGES.md b/CHANGES.md
index 41b5cb5..6d028a1 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -1,5 +1,7 @@
-# 4.1.2 (2019-07-29)
-- [FIXED] Plugins can now be loaded from outside of the 'plugins/' directory
+# UNRELEASED
+- [NEW] Added option to set new IAM API key.
+- [FIXED] Allow plugins to be loaded from outside the 'plugins/' directory.
+- [FIXED] Retry bad IAM token requests.
 
 # 4.1.1 (2019-06-17)
 - [FIXED] Remove unnecessary `npm-cli-login` dependency.
diff --git a/README.md b/README.md
index 7b77d2a..52066a1 100644
--- a/README.md
+++ b/README.md
@@ -355,7 +355,7 @@ All other configuration is plugin specific. It must be passed within an object
 to the `plugins` parameter in the client constructor. For example:
 
 ```js
-var cloudant = new Cloudant({ url: myurl, maxAttempt: 5, plugins: [ 'iamauth', { retry: { retryDelayMultiplier: 4 } } ]);
+var cloudant = new Cloudant({ url: myurl, maxAttempt: 5, plugins: [ { iamauth: { iamApiKey: 'abcxyz' } }, { retry: { retryDelayMultiplier: 4 } } ]);
 ```
 
 `maxAttempt` can _not_ be overridden by plugin specific configuration.
@@ -397,9 +397,31 @@ var cloudant = new Cloudant({ url: myurl, maxAttempt: 5, plugins: [ 'iamauth', {
    override this. To authenticate with the IAM token service set `iamClientId`
    and `iamClientSecret` in your plugin configuration.
 
+   The plugin will retry failed requests to the token service (specifically
+   `429` and `5xx` responses) until the number of retry requests reaches
+   `maxAttempt`. Be aware that retrying requests to the token service delays the
+   client request. It also increases the number of token exchange attempts and
+   therefore may result in rate limiting by the IAM token service.
+
+   The retry behavior can be configured using the following options:
+
+    - `retryDelayMultiplier`
+
+      The multiplication factor used for increasing the timeout after each
+      subsequent attempt _(default: 2)_.
+
+    - `retryInitialDelayMsecs`
+
+      The initial retry delay in milliseconds _(default: 500)_.
+
+   If the IAM token cannot be retrieved (either because the IAM token service is
+   down or the IAM API key is incorrect) then the request will continue without
+   IAM authentication allowing the database to return a `401` response to the
+   caller so that it may be handled appropriately.
+
    For example:
    ```js
-   var cloudant = new Cloudant({ url: 'https://examples.cloudant.com', plugins: { iamauth: { iamApiKey: 'xxxxxxxxxx' } } });
+   var cloudant = new Cloudant({ url: 'https://examples.cloudant.com', plugins: { iamauth: { iamApiKey: 'xxxxxxxxxx', retryDelayMultiplier: 4 } } });
    ```
 
    See [IBM Cloud Identity and Access Management](https://console.bluemix.net/docs/services/Cloudant/guides/iam.html#ibm-cloud-identity-and-access-management) for more information.
diff --git a/lib/client.js b/lib/client.js
index 750373f..182c6ca 100644
--- a/lib/client.js
+++ b/lib/client.js
@@ -1,4 +1,4 @@
-// Copyright © 2017, 2018 IBM Corp. All rights reserved.
+// Copyright © 2017, 2019 IBM Corp. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -233,6 +233,15 @@ class CloudantClient {
 
   // public
 
+  /**
+   * Get a client plugin instance.
+   *
+   * @param {string} pluginId
+   */
+  getPlugin(pluginId) {
+    return this._plugins[this._pluginIds.indexOf(pluginId)];
+  }
+
   /**
    * Perform a request using this Cloudant client.
    *
@@ -312,21 +321,19 @@ class CloudantClient {
       request.state.sending = false;
 
       debug(`Request attempt: ${request.state.attempt}`);
-
-      utils.runHooks('onRequest', request, request.options, function(err) {
-        utils.processState(request, function(stop) {
-          if (request.state.retry) {
-            debug('The onRequest hook issued retry.');
-            return done();
-          }
-          if (stop) {
-            debug(`The onRequest hook issued abort: ${stop}`);
-            return done(stop);
-          }
-
-          debug(`Delaying request for ${request.state.retryDelayMsecs} Msecs.`);
-
-          setTimeout(function() {
+      debug(`Delaying request for ${request.state.retryDelayMsecs} Msecs.`);
+
+      setTimeout(function() {
+        utils.runHooks('onRequest', request, request.options, function(err) {
+          utils.processState(request, function(stop) {
+            if (request.state.retry) {
+              debug('The onRequest hook issued retry.');
+              return done();
+            }
+            if (stop) {
+              debug(`The onRequest hook issued abort: ${stop}`);
+              return done(stop);
+            }
             if (request.abort) {
               debug('Client issued abort during plugin execution.');
               return done(new Error('Client issued abort'));
@@ -354,9 +361,9 @@ class CloudantClient {
                   .pipe(concatStream);
               }
             }
-          }, request.state.retryDelayMsecs);
+          });
         });
-      });
+      }, request.state.retryDelayMsecs);
     }, function(err) { debug(err.message); });
 
     return request.clientStream; // return stream to client
diff --git a/plugins/iamauth.js b/plugins/iamauth.js
index e53f98b..b87278f 100644
--- a/plugins/iamauth.js
+++ b/plugins/iamauth.js
@@ -25,33 +25,33 @@ const BasePlugin = require('./base.js');
  */
 class IAMPlugin extends BasePlugin {
   constructor(client, cfg) {
-    super(client, cfg);
+    if (typeof cfg.iamApiKey === 'undefined') {
+      throw new Error('Missing IAM API key from configuration');
+    }
 
-    var self = this;
-    self.iamApiKey = null;
-    self.baseUrl = cfg.baseUrl || null;
-    self.cookieJar = request.jar();
-    self.tokenUrl = cfg.iamTokenUrl || 'https://iam.cloud.ibm.com/identity/token';
+    // token service retry configuration
+    cfg = Object.assign({
+      retryDelayMultiplier: 2,
+      retryInitialDelayMsecs: 500
+    }, cfg);
 
-    // Specifies whether IAM authentication should be applied to the request being intercepted.
-    self.shouldApplyIAMAuth = true;
-    self.refreshRequired = true;
+    super(client, cfg);
 
-    if (typeof cfg.iamApiKey === 'undefined') {
-      debug('Missing IAM API key. Skipping IAM authentication.');
-      self.shouldApplyIAMAuth = false;
-    }
+    this.currentIamApiKey = null;
+    this.baseUrl = cfg.baseUrl || null;
+    this.cookieJar = null;
+    this.tokenUrl = cfg.iamTokenUrl || 'https://iam.cloud.ibm.com/identity/token';
+    this.shouldApplyIAMAuth = true;
+    this.refreshRequired = true;
   }
 
   onRequest(state, req, callback) {
     var self = this;
 
-    if (typeof self._cfg.iamApiKey === 'undefined') {
-      throw new Error('Missing IAM API key from configuration');
-    }
-
-    if (self._cfg.iamApiKey !== self.iamApiKey) {
-      debug('New credentials identified. Renewing session cookie...');
+    if (self._cfg.iamApiKey !== self.currentIamApiKey) {
+      debug('New IAM API key identified.');
+      this.cookieJar = request.jar(); // new jar
+      self.currentIamApiKey = self._cfg.iamApiKey;
       self.shouldApplyIAMAuth = self.refreshRequired = true;
     }
 
@@ -85,6 +85,13 @@ class IAMPlugin extends BasePlugin {
         debug(error.message);
         if (self.shouldApplyIAMAuth) {
           state.retry = true;
+          if (state.attempt === 1) {
+            state.retryDelayMsecs = self._cfg.retryInitialDelayMsecs;
+          } else {
+            state.retryDelayMsecs *= self._cfg.retryDelayMultiplier;
+          }
+        } else {
+          console.warn(`Disabling IAM authentication: ${error.message}`);
         }
       } else {
         req.jar = self.cookieJar; // add jar
@@ -153,8 +160,10 @@ class IAMPlugin extends BasePlugin {
                 callback(new Error('Invalid response from IAM token service'));
               }
             } else {
-              self.shouldApplyIAMAuth = false;
-              callback(new Error('Failed to access token'));
+              if (response.statusCode < 500 && response.statusCode !== 429) {
+                self.shouldApplyIAMAuth = false; // no retry
+              }
+              callback(new Error(`Failed to acquire access token. Status code: ${response.statusCode}`));
             }
           });
         },
@@ -170,13 +179,14 @@ class IAMPlugin extends BasePlugin {
             if (error) {
               callback(error);
             } else if (response.statusCode === 200) {
-              self.iamApiKey = cfg.iamApiKey;
               self.refreshRequired = false;
               debug('Successfully renewed IAM session.');
               callback();
             } else {
-              self.shouldApplyIAMAuth = false;
-              callback(new Error('Failed to exchange IAM token with Cloudant'));
+              if (response.statusCode < 500) {
+                self.shouldApplyIAMAuth = false; // no retry
+              }
+              callback(new Error(`Failed to exchange IAM token with Cloudant. Status code: ${response.statusCode}`));
             }
           });
         }
@@ -187,6 +197,11 @@ class IAMPlugin extends BasePlugin {
       });
     });
   }
+
+  setIamApiKey(iamApiKey) {
+    debug('Setting new IAM API key.');
+    this._cfg.iamApiKey = iamApiKey;
+  }
 }
 
 IAMPlugin.id = 'iamauth';


Test patch:
diff --git a/test/plugins/iamauth.js b/test/plugins/iamauth.js
index b5ce0a4..c5f4062 100644
--- a/test/plugins/iamauth.js
+++ b/test/plugins/iamauth.js
@@ -353,6 +353,7 @@ describe('#db IAMAuth Plugin', function() {
         'response_type': 'cloud_iam',
         'apikey': IAM_API_KEY
       })
+      .times(3)
       .reply(500, 'Internal Error 500\nThe server encountered an unexpected condition which prevented it from fulfilling the request.');
 
     var cloudantMocks = nock(SERVER)
@@ -418,10 +419,12 @@ describe('#db IAMAuth Plugin', function() {
         'response_type': 'cloud_iam',
         'apikey': IAM_API_KEY
       })
+      .times(3)
       .reply(200, MOCK_IAM_TOKEN_RESPONSE);
 
     var cloudantMocks = nock(SERVER)
       .post('/_iam_session', {access_token: MOCK_ACCESS_TOKEN})
+      .times(3)
       .reply(500, {error: 'internal_server_error', reason: 'Internal Server Error'})
       .get(DBNAME)
       .reply(401, {error: 'unauthorized', reason: 'Unauthorized'});
@@ -510,10 +513,10 @@ describe('#db IAMAuth Plugin', function() {
   });
 
   it('throws error for unspecified IAM API key', function() {
-    var cloudantClient = new Client({ plugins: 'iamauth' });
     assert.throws(
       () => {
-        cloudantClient.request({ url: SERVER + DBNAME });
+        /* eslint-disable no-new */
+        new Client({ plugins: 'iamauth' });
       },
       /Missing IAM API key from configuration/,
       'did not throw with expected message'
@@ -554,4 +557,105 @@ describe('#db IAMAuth Plugin', function() {
       assert.fail(`Unexpected reject: ${err}`);
     });
   });
+
+  it('successfully retries request on 500 IAM token service response and returns 200 response', function(done) {
+    if (process.env.NOCK_OFF) {
+      this.skip();
+    }
+
+    var iamMocks = nock(TOKEN_SERVER)
+      .post('/identity/token', {
+        'grant_type': 'urn:ibm:params:oauth:grant-type:apikey',
+        'response_type': 'cloud_iam',
+        'apikey': IAM_API_KEY
+      })
+      .times(2)
+      .reply(500, {error: 'internal_server_error', reason: 'Internal Server Error'})
+      .post('/identity/token', {
+        'grant_type': 'urn:ibm:params:oauth:grant-type:apikey',
+        'response_type': 'cloud_iam',
+        'apikey': IAM_API_KEY
+      })
+      .reply(200, MOCK_IAM_TOKEN_RESPONSE);
+
+    var cloudantMocks = nock(SERVER)
+      .post('/_iam_session', {access_token: MOCK_ACCESS_TOKEN})
+      .reply(200, {ok: true}, MOCK_SET_IAM_SESSION_HEADER)
+      .get(DBNAME)
+      .reply(200, {doc_count: 0});
+
+    var cloudantClient = new Client({ maxAttempt: 3, plugins: { iamauth: { iamApiKey: IAM_API_KEY } } });
+    var req = { url: SERVER + DBNAME, method: 'GET' };
+
+    var startTs = (new Date()).getTime();
+
+    cloudantClient.request(req, function(err, resp, data) {
+      assert.equal(err, null);
+      assert.equal(resp.request.headers.cookie, MOCK_IAM_SESSION);
+      assert.equal(resp.statusCode, 200);
+      assert.ok(data.indexOf('"doc_count":0') > -1);
+
+      // validate retry delay
+      var now = (new Date()).getTime();
+      assert.ok(now - startTs > (500 + 1000));
+
+      iamMocks.done();
+      cloudantMocks.done();
+      done();
+    });
+  });
+
+  it('supports changing the IAM API key', function(done) {
+    if (process.env.NOCK_OFF) {
+      this.skip();
+    }
+
+    var iamMocks = nock(TOKEN_SERVER)
+      .post('/identity/token', {
+        'grant_type': 'urn:ibm:params:oauth:grant-type:apikey',
+        'response_type': 'cloud_iam',
+        'apikey': 'bad_key'
+      })
+      .reply(400, {
+        errorCode: 'BXNIM0415E',
+        errorMessage: 'Provided API key could not be found'
+      })
+      .post('/identity/token', {
+        'grant_type': 'urn:ibm:params:oauth:grant-type:apikey',
+        'response_type': 'cloud_iam',
+        'apikey': IAM_API_KEY
+      })
+      .reply(200, MOCK_IAM_TOKEN_RESPONSE);
+
+    var cloudantMocks = nock(SERVER)
+      .get(DBNAME)
+      .reply(401, {error: 'unauthorized', reason: 'Unauthorized'})
+      .post('/_iam_session', {access_token: MOCK_ACCESS_TOKEN})
+      .reply(200, {ok: true}, MOCK_SET_IAM_SESSION_HEADER)
+      .get(DBNAME)
+      .reply(200, {doc_count: 0});
+
+    var cloudantClient = new Client({ plugins: { iamauth: { iamApiKey: 'bad_key' } } });
+    var req = { url: SERVER + DBNAME, method: 'GET' };
+    cloudantClient.request(req, function(err, resp, data) {
+      assert.equal(err, null);
+      assert.equal(resp.request.headers.cookie, null);
+      assert.equal(resp.statusCode, 401);
+      assert.ok(data.indexOf('"error":"unauthorized"') > -1);
+
+      // update IAM API key
+      cloudantClient.getPlugin('iamauth').setIamApiKey(IAM_API_KEY);
+
+      cloudantClient.request(req, function(err, resp, data) {
+        assert.equal(err, null);
+        assert.equal(resp.request.headers.cookie, MOCK_IAM_SESSION);
+        assert.equal(resp.statusCode, 200);
+        assert.ok(data.indexOf('"doc_count":0') > -1);
+
+        iamMocks.done();
+        cloudantMocks.done();
+        done();
+      });
+    });
+  });
 });
diff --git a/test/readmeexamples.js b/test/readmeexamples.js
index 8d0bd74..ce90751 100644
--- a/test/readmeexamples.js
+++ b/test/readmeexamples.js
@@ -1,4 +1,4 @@
-// Copyright © 2018 IBM Corp. All rights reserved.
+// Copyright © 2018, 2019 IBM Corp. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -260,10 +260,11 @@ describe('#db README Examples', function() {
       var cloudant = new Cloudant({
         url: `https://${ME}.cloudant.com`,
         maxAttempt: 5,
-        plugins: [ 'iamauth', { retry: { retryDelayMultiplier: 4 } } ]
+        plugins: [ { iamauth: { iamApiKey: 'abcxyz' } }, { retry: { retryDelayMultiplier: 4 } } ]
       });
       assert.equal(cloudant.cc._plugins.length, 2);
       assert.equal(cloudant.cc._plugins[0].constructor.name, 'IAMPlugin');
+      assert.equal(cloudant.cc._plugins[0]._cfg.iamApiKey, 'abcxyz');
       assert.equal(cloudant.cc._plugins[1].constructor.name, 'RetryPlugin');
       assert.equal(cloudant.cc._plugins[1]._cfg.retryDelayMultiplier, 4);
     });