[oidc-middleware] upgrade deps for compatibility with node 12 (#661)

okta · Feb 4, 2020 · 0f6e67c · 0f6e67c
1 parent 0f08683
commit 0f6e67c
Show file tree

Hide file tree

Showing 9 changed files with 1,705 additions and 2,080 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -1,6 +1,6 @@
 language: node_js
 node_js:
-- '8.10.0'
+- '12.4.0'
 
 addons:
   chrome: stable

diff --git a/packages/oidc-middleware/.eslintrc.json b/packages/oidc-middleware/.eslintrc.json
@@ -1,9 +1,13 @@
 {
   "extends": [
     "eslint:recommended",
-    "plugin:promise/recommended"
+    "plugin:promise/recommended",
+    "plugin:jasmine/recommended"
+  ],
+  "plugins": [
+    "promise",
+    "jasmine"
   ],
-  "plugins": ["promise"],
   "env": {
     "browser": false,
     "commonjs": true
@@ -13,5 +17,8 @@
   },
   "globals": {
     "Promise": "readonly"
+  },
+  "rules": {
+    "jasmine/new-line-before-expect": 0
   }
 }
diff --git a/packages/oidc-middleware/CHANGELOG.md b/packages/oidc-middleware/CHANGELOG.md
@@ -1,3 +1,12 @@
+# 4.0.0
+
+### Breaking Changes
+
+- [#661](https://github.com/okta/okta-oidc-js/pull/661) Requires Node >= 10.13.0. Add support for Node 12. Update production dependencies:
+  - `[email protected]` (was 2.5.0)
+  - `[email protected]` (was 0.3.2)
+  - `@okta/[email protected]` (was 0.2.0)
+
 # 3.0.0
 
 ### Breaking Changes 

diff --git a/packages/oidc-middleware/package.json b/packages/oidc-middleware/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@okta/oidc-middleware",
-  "version": "3.0.0",
+  "version": "4.0.0",
   "description": "OpenId Connect middleware for authorization code flows",
   "repository": "https://github.com/okta/okta-oidc-js",
   "homepage": "https://github.com/okta/okta-oidc-js/tree/master/packages/oidc-middleware",
@@ -17,7 +17,7 @@
   "main": "index.js",
   "scripts": {
     "lint": "eslint .",
-    "test": "yarn test:unit && yarn test:e2e && yarn test:integration",
+    "test": "yarn lint && yarn test:unit && yarn test:e2e && yarn test:integration",
     "pretest:e2e": "../../scripts/updateSeDrivers.sh",
     "test:e2e": "protractor test/e2e/protractor.conf.js",
     "test:integration": "../../scripts/tck.sh 0.4.0-SNAPSHOT",
@@ -26,40 +26,40 @@
     "start:custom": "node test/e2e/harness/start-custom-login-server.js"
   },
   "engines": {
-    "node": ">=6"
+    "node": "^10.13.0 || >=12.0.0"
   },
   "jest": {
     "testEnvironment": "jsdom"
   },
   "license": "Apache-2.0",
   "dependencies": {
-    "@okta/configuration-validation": "^0.2.0",
+    "@okta/configuration-validation": "^0.4.1",
     "body-parser": "^1.18.2",
     "connect-ensure-login": "^0.1.1",
     "csurf": "^1.9.0",
     "express": "^4.16.3",
     "lodash": "^4.17.5",
     "negotiator": "^0.6.1",
     "node-fetch": "^2.3.0",
-    "openid-client": "2.5.0",
-    "passport": "^0.3.2",
+    "openid-client": "3.12.2",
+    "passport": "^0.4.1",
     "uuid": "^3.1.0"
   },
   "devDependencies": {
     "babel-eslint": "^10.0.3",
-    "cross-env": "^5.1.1",
-    "ejs": "^2.5.7",
+    "cross-env": "^7.0.0",
+    "ejs": "^3.0.1",
     "eslint": "^6.6.0",
-    "eslint-plugin-jasmine": "^2.10.1",
+    "eslint-plugin-jasmine": "^4.1.0",
     "eslint-plugin-jest": "^23.0.3",
     "eslint-plugin-promise": "^4.2.1",
     "eslint-plugin-protractor": "^2.1.1",
     "express-session": "^1.15.5",
     "jasmine-protractor-browser-log-reporter": "^1.0.0",
     "jasmine-reporters": "^2.2.0",
     "jasmine-spec-reporter": "^4.1.0",
-    "jest": "^23.6.0",
-    "nock": "^9.1.6",
+    "jest": "^25.1.0",
+    "nock": "^11.7.2",
     "protractor": "^5.4.2",
     "read-package-tree": "^5.1.6",
     "server-destroy": "^1.0.1"

diff --git a/packages/oidc-middleware/src/oidcUtil.js b/packages/oidc-middleware/src/oidcUtil.js
@@ -11,33 +11,42 @@
  */
 
 const passport = require('passport');
-const OpenIdClientStrategy = require('openid-client').Strategy;
-const Issuer = require('openid-client').Issuer;
+const OpenIdClient = require('openid-client');
 const ensureLoggedIn = require('connect-ensure-login').ensureLoggedIn;
 const Negotiator = require('negotiator');
 const os = require('os');
 
 const pkg = require('../package.json');
 
-/**
- * Parse out the default user agent for the openid-client library, which currently looks like:
- *
- * openid-client/1.15.0 (https://github.com/panva/node-openid-client)
- *
- * We strip off the github link because it's not necessary.
- */
-let clientUserAgent = Issuer.defaultHttpOptions.headers['User-Agent'];
-if (typeof clientUserAgent === 'string' && clientUserAgent) {
-  clientUserAgent = ' ' + clientUserAgent.split(' ')[0]
-} else {
-  clientUserAgent = '';
-}
+const OpenIdClientStrategy = OpenIdClient.Strategy;
+const Issuer = OpenIdClient.Issuer;
+const custom = OpenIdClient.custom;
+
+const oidcUtil = module.exports;
 
-const userAgent = `${pkg.name}/${pkg.version}${clientUserAgent} node/${process.versions.node} ${os.platform()}/${os.release()}`;
+function customizeUserAgent(options) {
+  /**
+   * Parse out the default user agent for the openid-client library, which currently looks like:
+   *
+   * openid-client/1.15.0 (https://github.com/panva/node-openid-client)
+   *
+   * We strip off the github link because it's not necessary.
+   */
+  options = options || {};
+  const headers = options.headers || {};
+  let clientUserAgent = headers['User-Agent'];
+  if (typeof clientUserAgent === 'string') {
+    clientUserAgent = ' ' + clientUserAgent.split(' ')[0]
+  } else {
+    clientUserAgent = '';
+  }
 
-Issuer.defaultHttpOptions.headers['User-Agent'] = userAgent;
+  const userAgent = `${pkg.name}/${pkg.version}${clientUserAgent} node/${process.versions.node} ${os.platform()}/${os.release()}`;
+  headers['User-Agent'] = userAgent;
 
-const oidcUtil = module.exports;
+  options.headers = headers;
+  return options;
+}
 
 oidcUtil.createClient = context => {
   const {
@@ -49,7 +58,11 @@ oidcUtil.createClient = context => {
     timeout
   } = context.options;
 
-  Issuer.defaultHttpOptions.timeout = timeout || 10000;
+  Issuer[custom.http_options] = function(options) {
+    options = customizeUserAgent(options);
+    options.timeout = timeout || 10000;
+    return options;
+  };
 
   return Issuer.discover(issuer +  '/.well-known/openid-configuration')
   .then(iss => {
@@ -60,8 +73,8 @@ oidcUtil.createClient = context => {
         redirect_uri
       ]
     });
-
-    client.CLOCK_TOLERANCE = maxClockSkew;
+    client[custom.http_options] = customizeUserAgent;
+    client[custom.clock_tolerance] = maxClockSkew;
 
     return client;
   });

diff --git a/packages/oidc-middleware/test/unit/.eslintrc.json b/packages/oidc-middleware/test/unit/.eslintrc.json
@@ -5,5 +5,8 @@
   "env": {
     "es6": true,
     "node": true
+  },
+  "rules": {
+    "jest/no-test-callback": 0
   }
 }
diff --git a/packages/oidc-middleware/test/unit/constructor.spec.js b/packages/oidc-middleware/test/unit/constructor.spec.js
@@ -1,4 +1,4 @@
-const Issuer = require('openid-client').Issuer;
+const OpenIdClient = require('openid-client');
 const nock = require('nock');
 const os = require('os');
 const path = require('path');
@@ -8,6 +8,9 @@ const { ExpressOIDC } = require('../../index.js');
 const pkg = require('../../package.json');
 const modulesRoot = path.resolve(__dirname, '../../');
 
+const Issuer = OpenIdClient.Issuer;
+const custom = OpenIdClient.custom;
+
 describe('new ExpressOIDC()', () => {
   const findDomainMessage = 'You can copy your domain from the Okta Developer ' +
     'Console. Follow these instructions to find it: https://bit.ly/finding-okta-domain';
@@ -23,6 +26,23 @@ describe('new ExpressOIDC()', () => {
     appBaseUrl: 'https://app.foo'
   };
 
+  function mockWellKnown(issuer) {
+    issuer = issuer || 'https://foo'
+    nock(issuer)
+    .get('/.well-known/openid-configuration')
+    .reply(200, {
+      issuer
+    })
+  }
+
+  afterEach(function() {
+    if(!nock.isDone()) {
+      nock.cleanAll();
+      throw new Error('Not all nock interceptors were used!');
+    }
+  });
+
+
   it('should throw if no issuer is provided', () => {
     function createInstance() {
       new ExpressOIDC({
@@ -38,27 +58,30 @@ describe('new ExpressOIDC()', () => {
     function createInstance() {
       new ExpressOIDC({
         ...minimumConfig, 
-        issuer: 'http://foo.com' 
+        issuer: 'http://foo' 
       });
     }
-    const errorMsg = `Your Okta URL must start with https. Current value: http://foo.com. ${findDomainMessage}`;
+    const errorMsg = `Your Okta URL must start with https. Current value: http://foo. ${findDomainMessage}`;
     expect(createInstance).toThrow(errorMsg);
   });
 
-  it('should not throw if https issuer validation is skipped', () => {
+  it('should not throw if https issuer validation is skipped', done => {
     jest.spyOn(console, 'warn').mockImplementation(() => {}); // silence for testing
-    function createInstance() {
-      new ExpressOIDC({
-        ...minimumConfig,
-        issuer: 'http://foo.com',
-        testing: {
-          disableHttpsCheck: true
-        }
-      }).on('error', () => {}); // prevent warning about unhandled error on this intentional error
-    }
-    const errorMsg = `Your Okta URL must start with https. Current value: http://foo.com. ${findDomainMessage}`;
-    expect(createInstance).not.toThrow(errorMsg);
-    expect(console.warn).toBeCalledWith('Warning: HTTPS check is disabled. This allows for insecure configurations and is NOT recommended for production use.');
+    mockWellKnown('http://foo');
+    new ExpressOIDC({
+      ...minimumConfig,
+      issuer: 'http://foo',
+      testing: {
+        disableHttpsCheck: true
+      }
+    })
+    .on('error', () => {
+      expect(false).toBe(true);
+    })
+    .on('ready', () => {
+      expect(console.warn).toBeCalledWith('Warning: HTTPS check is disabled. This allows for insecure configurations and is NOT recommended for production use.');
+      done();
+    });
   });
 
   it('should throw if an issuer matching {yourOktaDomain} is provided', () => {
@@ -163,7 +186,7 @@ describe('new ExpressOIDC()', () => {
       });
     }
     const errorMsg = `Your client secret is missing. ${findCredentialsMessage}`;
-  expect(createInstance).toThrow(errorMsg);
+    expect(createInstance).toThrow(errorMsg);
   });
 
   it('should throw if a client_id matching {clientId} is provided', () => {
@@ -232,36 +255,42 @@ describe('new ExpressOIDC()', () => {
     expect(createInstance).toThrow(errorMsg);
   });
 
-  it('should set the HTTP timeout to 10 seconds', () => {
+  it('should set the HTTP timeout to 10 seconds', done => {
+    mockWellKnown();
     new ExpressOIDC({
       ...minimumConfig
-    }).on('error', () => {
-      // Ignore errors caused by mock configuration data
+    })
+    .on('ready', () => {
+      expect(Issuer[custom.http_options]().timeout).toBe(10000);
+      done();
     });
-    expect(Issuer.defaultHttpOptions.timeout).toBe(10000);
   });
 
-  it('should allow me to change the HTTP timeout', () => {
+  it('should allow me to change the HTTP timeout', done => {
+    mockWellKnown();
     new ExpressOIDC({
       ...minimumConfig,
-      timeout: 1
-    }).on('error', () => {
-      // Ignore errors caused by mock configuration data
+      timeout: 2000
+    })
+    .on('ready', () => {
+      expect(Issuer[custom.http_options]().timeout).toBe(2000);
+      done();
     });
-    expect(Issuer.defaultHttpOptions.timeout).toBe(1);
   });
 
   // eslint-disable-next-line jest/no-test-callback
   it('should throw ETIMEOUT if the timeout is reached', (done) => {
     nock('https://foo')
     .get('/.well-known/openid-configuration')
+    .delay(1000)
     .reply(200, function cb() {
       // dont reply, we want to timeout
     });
     new ExpressOIDC({
       ...minimumConfig,
       timeout: 1
     }).on('error', (e) => {
+      nock.abortPendingRequests();
       expect(e.code).toBe('ETIMEDOUT');
       done();
     });
@@ -273,20 +302,23 @@ describe('new ExpressOIDC()', () => {
       return kidName.includes('openid');
     }, function (er, data) {
       const openIdPkg = data.children[0].package;
+
+      const expectedAgent = `${pkg.name}/${pkg.version} ${openIdPkg.name}/${openIdPkg.version} node/${process.versions.node} ${os.platform()}/${os.release()}`;
+      let userAgent;
+
       nock('https://foo')
       .get('/.well-known/openid-configuration')
       .reply(200, function cb() {
-        const userAgent = this.req.headers['user-agent'];
-        const expectedAgent = `${pkg.name}/${pkg.version} ${openIdPkg.name}/${openIdPkg.version} node/${process.versions.node} ${os.platform()}/${os.release()}`;
-        expect(userAgent).toBe(expectedAgent);
-        done();
+        userAgent = this.req.headers['user-agent'];
+        return JSON.stringify({ issuer: 'https://foo' });
       });
       new ExpressOIDC({
         ...minimumConfig
-      }).on('error', () => {
-        // Because we're mocking and not fulfilling the real response, the client will error
-        // Ignore this because we're only asserting what we see on the request
-      });
+      })
+      .on('ready', () => {
+        expect(userAgent).toBe(expectedAgent);
+        done();
+      })
     });
   })
 });
diff --git a/packages/oidc-middleware/test/unit/logout.spec.js b/packages/oidc-middleware/test/unit/logout.spec.js
@@ -164,7 +164,7 @@ describe('logout', () => {
       describe('session', () => {
         it('calls req.logout()', async () => {
           await logout(req, res);
-          expect(req.logout).toHaveBeenCalled();
+          expect(req.logout).toHaveBeenCalledWith();
         })
       })
     });