Repository: cloudant/nodejs-cloudant (js)

Issue:
cookieauth plugin with autoRenew renews token constantly
<!-- Issues will be CLOSED IMMEDIATELY if the following template is not completed. -->

## Bug Description
The cookieauth plugin with `autoRenew=true` can get the `TokenManager` into a state where it constantly renews the token. This happens if the `Max-Age` cookie header field is missing.

```
  cloudant:tokens:tokenmanager Auto renewing token now... +1ms
  cloudant:tokens:tokenmanager Starting token renewal. +0ms
  cloudant:tokens:tokenmanager Finished token renewal. +466ms
  cloudant:tokens:tokenmanager Renewing token in NaN milliseconds. +0ms
  cloudant:tokens:tokenmanager Auto renewing token now... +1ms
  cloudant:tokens:tokenmanager Starting token renewal. +0ms
  cloudant:tokens:tokenmanager Finished token renewal. +555ms
  cloudant:tokens:tokenmanager Renewing token in NaN milliseconds. +0ms
```

### 1. Steps to reproduce and the simplest code sample possible to demonstrate the issue
<!--
Outline the steps you take to make the problem happen.
Provide the simplest code sample you can, in context, that reproduces the issue.
-->
Connect using the cookieauth plugin and get a `Set-Cookie` HTTP header without the `Max-Age` field.

### 2. What you expected to happen
Cookie to be renewed after the `Max-Age` from the `Set-Cookie` header, or the `defaultMaxAgeSecs` if not present.

### 3. What actually happened
Cookie is renewed constantly

## Environment details
- Version(s) that are affected by this issue.
    > 4.2.2 and above
- Node.js version
    > any


Pull request description:
Fix constant token renewal in the absence of a cookie Max-Age
<!--
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
The TokenManager supports auto renewal of tokens. It uses the Max-Age field of the cookie header or a default value to schedule a callback for the renewal.

Fixes #426

<!--
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

<!--
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.
-->

The problem is that the default value is only used for the first renewal. For subsequent renewals the default is undefined so the renewal is scheduled instantly unless a Max-Age field is found in the header:

```
  _autoRenew(defaultMaxAgeSecs) {
    debug('Auto renewing token now...');
    this._renew().then((response) => {
      let maxAgeSecs = cookie.parse(response.headers['set-cookie'][0])['Max-Age'] || defaultMaxAgeSecs;
      let delayMSecs = maxAgeSecs / 2 * 1000;
      debug(`Renewing token in ${delayMSecs} milliseconds.`);
      setTimeout(this._autoRenew.bind(this), delayMSecs).unref(); <-- defaultMaxAgeSecs not passed on
    }).catch((error) => {
      debug(`Failed to auto renew token - ${error}. Retrying in 60 seconds.`);
      setTimeout(this._autoRenew.bind(this), 60000).unref();
    });
  }
```

The fix is to pass the `defaultMaxAgeSecs` on to the setTimeout handler.

## 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
Added new test `correctly auto renews token in the absence of a cookie Max-Age` in `test/tokens/TokenManager.js`
<!--
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 81e9285..2abebb4 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -1,3 +1,7 @@
+# Unreleased
+- [FIXED] Stopped constant token auto renewal in the absence of a cookie
+  header Max-Age
+
 # 4.2.4 (2020-03-02)
 - [FIXED] Pinned Nano to version 8.1 to resolve issue with extending upstream
   TypeScript changes in Nano version 8.2.0.
diff --git a/lib/tokens/TokenManager.js b/lib/tokens/TokenManager.js
index 2ea32e8..c21500f 100644
--- a/lib/tokens/TokenManager.js
+++ b/lib/tokens/TokenManager.js
@@ -35,7 +35,7 @@ class TokenManager {
       let maxAgeSecs = cookie.parse(response.headers['set-cookie'][0])['Max-Age'] || defaultMaxAgeSecs;
       let delayMSecs = maxAgeSecs / 2 * 1000;
       debug(`Renewing token in ${delayMSecs} milliseconds.`);
-      setTimeout(this._autoRenew.bind(this), delayMSecs).unref();
+      setTimeout(this._autoRenew.bind(this, defaultMaxAgeSecs), delayMSecs).unref();
     }).catch((error) => {
       debug(`Failed to auto renew token - ${error}. Retrying in 60 seconds.`);
       setTimeout(this._autoRenew.bind(this), 60000).unref();
diff --git a/package-lock.json b/package-lock.json
index 6ac31c4..bf979e1 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1553,9 +1553,9 @@
       "integrity": "sha1-Jjj8OaAzHpysGgS3F5mTHJxQ33k="
     },
     "lodash": {
-      "version": "4.17.15",
-      "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.15.tgz",
-      "integrity": "sha512-8xOcRHvCjnocdS5cpwXQXVzmmh5e5+saE2QGoeQmbKmRS6J3VQppPOIt0MnmE+4xlZoumy0GPG0D0MVIQbNA1A=="
+      "version": "4.17.19",
+      "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.19.tgz",
+      "integrity": "sha512-JNvd8XER9GQX0v2qJgsaN/mzFCNA5BRe/j8JN9d+tWyGLSodKQHKFicdwNYzWwI3wjRnaKPsGj1XkBjx/F96DQ=="
     },
     "log-symbols": {
       "version": "3.0.0",


Test patch:
diff --git a/test/tokens/TokenManager.js b/test/tokens/TokenManager.js
index edb8257..b2965bf 100644
--- a/test/tokens/TokenManager.js
+++ b/test/tokens/TokenManager.js
@@ -23,12 +23,13 @@ class TokenManagerRenewSuccess extends TokenManager {
   constructor() {
     super();
     this._getTokenCallCount = 0;
+    this._cookieHeader = 'Max-Age=1';
   }
 
   _getToken(callback) {
     this._getTokenCallCount += 1;
     setTimeout(() => {
-      callback(null, { headers: { 'set-cookie': [ 'Max-Age=1' ] } });
+      callback(null, { headers: { 'set-cookie': [ this._cookieHeader ] } });
     }, 100);
   }
 
@@ -36,6 +37,14 @@ class TokenManagerRenewSuccess extends TokenManager {
   get getTokenCallCount() {
     return this._getTokenCallCount;
   }
+
+  get cookieHeader() {
+    return this._cookieHeader;
+  }
+
+  set cookieHeader(cookieHeader) {
+    this._cookieHeader = cookieHeader;
+  }
 }
 
 class TokenManagerRenewFailure extends TokenManager {
@@ -90,6 +99,17 @@ describe('Token Manger', (done) => {
     }, 2000);
   });
 
+  it('correctly auto renews token in the absence of a cookie Max-Age', (done) => {
+    let t = new TokenManagerRenewSuccess();
+    t.cookieHeader = '';
+    t.startAutoRenew(2);
+    setTimeout(() => {
+      // one renew every 1 seconds
+      assert.equal(t.getTokenCallCount, 2);
+      done();
+    }, 2000);
+  });
+
   it('only makes one renewal request', (done) => {
     let t = new TokenManagerRenewSuccess();
     let renewalCount = 0;