Merge pull request #392 from cloudant/385-iam-error-on-auth-fail

Error when user cannot be authenticated using IAM.

Author: smithsz

CHANGES.md

@@ -1,3 +1,11 @@
+# UNRELEASED
+- [FIXED] Stopped disabling the IAM auth plugin after failed IAM
+  authentications. Subsequent requests will re-request authorization,
+  potentially failing again if the original authentication failure was not
+  temporary.
+- [FIXED] Ensure IAM API key can be correctly changed.
+- [FIXED] Callback with an error when a user cannot be authenticated using IAM.
+
 # 4.2.1 (2019-08-29)
 - [FIXED] Include all built-in plugin modules in webpack bundle.
 

README.md

@@ -414,16 +414,15 @@ var cloudant = new Cloudant({ url: myurl, maxAttempt: 5, plugins: [ { iamauth: {
 
       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', retryDelayMultiplier: 4 } } });
+   var cloudant = new Cloudant({ url: 'https://examples.cloudant.com', plugins: { iamauth: { iamApiKey: 'xxxxxxxxxx', retryDelayMultiplier: 4, retryInitialDelayMsecs: 100 } } });
    ```
 
+   If the IAM token cannot be retrieved after the configured number of retries
+   (either because the IAM token service is down or the IAM API key is
+   incorrect) then an error is returned to the client.
+
    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.
 
 3. `retry`

plugins/iamauth.js

@@ -41,7 +41,6 @@ class IAMPlugin extends BasePlugin {
     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;
   }
 
@@ -50,16 +49,9 @@ class IAMPlugin extends BasePlugin {
 
     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;
-    }
-
-    if (!self.shouldApplyIAMAuth) {
-      return callback(state);
-    }
-
-    if (!self.refreshRequired) {
+      state.stash.newApiKey = true;
+    } else if (!self.refreshRequired) {
       if (self.baseUrl && self.cookieJar.getCookies(self.baseUrl, {expire: true}).length === 0) {
         debug('There are no valid session cookies in the jar. Requesting IAM session refresh...');
       } else {
@@ -80,18 +72,18 @@ class IAMPlugin extends BasePlugin {
       });
     }
 
-    self.refreshCookie(self._cfg, function(error) {
+    self.refreshCookie(self._cfg, state, function(error) {
       if (error) {
         debug(error.message);
-        if (self.shouldApplyIAMAuth) {
+        if (state.attempt < state.maxAttempt) {
           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}`);
+          state.abortWithResponse = [ error ]; // return error to client
         }
       } else {
         req.jar = self.cookieJar; // add jar
@@ -101,7 +93,7 @@ class IAMPlugin extends BasePlugin {
   }
 
   onResponse(state, response, callback) {
-    if (this.shouldApplyIAMAuth && response.statusCode === 401) {
+    if (response.statusCode === 401) {
       debug('Requesting IAM session refresh for 401 response.');
       this.refreshRequired = true;
       state.retry = true;
@@ -110,7 +102,7 @@ class IAMPlugin extends BasePlugin {
   }
 
   // Perform IAM session request.
-  refreshCookie(cfg, callback) {
+  refreshCookie(cfg, state, callback) {
     var self = this;
 
     if (self.baseUrl === null) {
@@ -121,10 +113,11 @@ class IAMPlugin extends BasePlugin {
       stale: cfg.iamLockStaleMsecs || 2500, // 2.5 secs
       wait: cfg.iamLockWaitMsecs || 2000 // 2 secs
     }, function(error, done) {
-      if (!self.shouldApplyIAMAuth) {
-        return callback(new Error('Skipping IAM session authentication'));
-      }
-      if (!self.refreshRequired) {
+      if (state.stash.newApiKey) {
+        debug('Refreshing session with new IAM API key.');
+        state.stash.newApiKey = false;
+        self.cookieJar = request.jar(); // new jar
+      } else if (!self.refreshRequired) {
         debug('Session refresh no longer required.');
         return callback();
       }
@@ -160,9 +153,6 @@ class IAMPlugin extends BasePlugin {
                 callback(new Error('Invalid response from IAM token service'));
               }
             } else {
-              if (response.statusCode < 500 && response.statusCode !== 429) {
-                self.shouldApplyIAMAuth = false; // no retry
-              }
               callback(new Error(`Failed to acquire access token. Status code: ${response.statusCode}`));
             }
           });
@@ -183,9 +173,6 @@ class IAMPlugin extends BasePlugin {
               debug('Successfully renewed IAM session.');
               callback();
             } else {
-              if (response.statusCode < 500) {
-                self.shouldApplyIAMAuth = false; // no retry
-              }
               callback(new Error(`Failed to exchange IAM token with Cloudant. Status code: ${response.statusCode}`));
             }
           });

test/plugins/iamauth.js

@@ -342,7 +342,7 @@ describe('#db IAMAuth Plugin', function() {
     });
   });
 
-  it('skips IAM authentication if access token returns non-200 response', function(done) {
+  it('returns an error if access token returns non-200 response', function(done) {
     if (process.env.NOCK_OFF) {
       this.skip();
     }
@@ -356,19 +356,11 @@ describe('#db IAMAuth Plugin', function() {
       .times(3)
       .reply(500, 'Internal Error 500\nThe server encountered an unexpected condition which prevented it from fulfilling the request.');
 
-    var cloudantMocks = nock(SERVER)
-      .get(DBNAME)
-      .reply(401, {error: 'unauthorized', reason: 'Unauthorized'});
-
     var cloudantClient = new Client({ plugins: { iamauth: { iamApiKey: IAM_API_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);
+      assert.equal(err.message, 'Failed to acquire access token. Status code: 500');
       iamMocks.done();
-      cloudantMocks.done();
       done();
     });
   });
@@ -408,7 +400,7 @@ describe('#db IAMAuth Plugin', function() {
     });
   });
 
-  it('skips IAM authentication if IAM cookie login returns non-200 response', function(done) {
+  it('returns an error if IAM cookie login returns non-200 response', function(done) {
     if (process.env.NOCK_OFF) {
       this.skip();
     }
@@ -425,17 +417,12 @@ describe('#db IAMAuth Plugin', function() {
     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'});
+      .reply(500, {error: 'internal_server_error', reason: 'Internal Server Error'});
 
     var cloudantClient = new Client({ plugins: { iamauth: { iamApiKey: IAM_API_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);
+      assert.equal(err.message, 'Failed to exchange IAM token with Cloudant. Status code: 500');
       iamMocks.done();
       cloudantMocks.done();
       done();
@@ -479,39 +466,6 @@ describe('#db IAMAuth Plugin', function() {
     });
   });
 
-  it('skips authentication renewal on 401 response if previous attempts failed', 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
-      })
-      .reply(400, {
-        errorCode: 'BXNIM0415E',
-        errorMessage: 'Provided API key could not be found'
-      });
-
-    var cloudantMocks = nock(SERVER)
-      .get(DBNAME)
-      .reply(401, {error: 'unauthorized', reason: 'Unauthorized'});
-
-    var cloudantClient = new Client({ plugins: { iamauth: { iamApiKey: IAM_API_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);
-      iamMocks.done();
-      cloudantMocks.done();
-      done();
-    });
-  });
-
   it('throws error for unspecified IAM API key', function() {
     assert.throws(
       () => {
@@ -616,6 +570,7 @@ describe('#db IAMAuth Plugin', function() {
         'response_type': 'cloud_iam',
         'apikey': 'bad_key'
       })
+      .times(3)
       .reply(400, {
         errorCode: 'BXNIM0415E',
         errorMessage: 'Provided API key could not be found'
@@ -628,8 +583,6 @@ describe('#db IAMAuth Plugin', function() {
       .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)
@@ -638,10 +591,7 @@ describe('#db IAMAuth Plugin', function() {
     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);
+      assert.equal(err.message, 'Failed to acquire access token. Status code: 400');
 
       // update IAM API key
       cloudantClient.getPlugin('iamauth').setIamApiKey(IAM_API_KEY);