Update oidcUtil to allow a callback without userinfo [revisited] (#949)

* Update oidcUtil to allow a callback without userinfo

* replace undefined done reference with last argument

* fall back to fixed parameters number in verification callback

* add UT for passport strategy verification callback

* preserve conditional branch for response w/o token

* remove debug output

* remove extra blank line and place semicolons consistently

* bump oidc-middleware to 4.0.2 and update CHANGELOG

Co-authored-by: Bradley Cornford <[email protected]>

packages/oidc-middleware/CHANGELOG.md

@@ -1,3 +1,10 @@
+# 4.0.2
+
+### Bug Fixes
+
+- [#949](https://github.com/okta/okta-oidc-js/pull/949) 
+  - oidcUtil: support for passport strategy callback without userinfo
+
 # 4.0.1
 
 ### Bug Fixes

packages/oidc-middleware/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@okta/oidc-middleware",
-  "version": "4.0.1",
+  "version": "4.0.2",
   "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",

packages/oidc-middleware/src/oidcUtil.js

@@ -86,13 +86,29 @@ oidcUtil.bootstrapPassportStrategy = context => {
     },
     sessionKey: context.options.sessionKey,
     client: context.client
-  }, (tokenSet, userinfo, done) => {
-    return tokenSet && userinfo
-      ? done(null, {
-        userinfo: userinfo,
-        tokens: tokenSet
-      })
-      : done(null);
+  }, (tokenSet, callbackArg1, callbackArg2) => {
+    let done;
+    let userinfo;
+
+    if (typeof(callbackArg2) !== 'undefined') {
+      done = callbackArg2;
+      userinfo = callbackArg1;
+    } else {
+      done = callbackArg1;
+    }
+
+    if(tokenSet) {
+      return userinfo ?
+        done(null, {
+          userinfo,
+          tokens: tokenSet,
+        }) :
+        done(null, {
+          tokens: tokenSet
+        });
+    } else {
+      return done(null);
+    }
   });
 
   // bypass passport's serializers

packages/oidc-middleware/test/unit/oidcUtil.spec.js

@@ -0,0 +1,99 @@
+const passport = require('passport');
+const OpenIdClient = require('openid-client');
+const oidcUtil = require('../../src/oidcUtil.js');
+
+function createMockOpenIdClient(config={}) {
+  const Issuer = OpenIdClient.Issuer;
+
+  const defaultConfig = {
+    issuer: 'https://foo',
+    token_endpoint: 'https://foo/token',
+    userinfo_endpoint: 'https://foo/userinfo'
+  };
+  const issuer = new Issuer({
+    ...defaultConfig,
+    ...config
+  });
+  const client = new issuer.Client({
+    client_id: 'foo',
+    client_secret: 'foo',
+  });
+
+  client.callback = jest.fn(() => ({
+    access_token: 'token_value'
+  }));
+
+  client.userinfo = jest.fn(() => ({
+    cid: '123'
+  }));
+
+  return client;
+}
+
+function createMockRedirectRequest() {
+  const request = jest.mock();
+  request.method = 'GET';
+  request.url = 'http://foo/authorization-code/callback?code=foo';
+  request.session = {
+    'oidc:foo': {
+      response_type: 'code',
+    },
+  };
+  return request;
+}
+
+describe('oidcUtil', function () {
+  describe('Passport strategy setup', function () {
+    beforeEach(function () {
+      jest.clearAllMocks();
+    });
+
+    it('includes verification function which tolerates authentication repsonses w/o userInfo', function (next) {
+      const passportStrategySetter = jest.spyOn(passport, 'use').mockImplementation(() => {});
+
+      const context = {
+        options: {
+          scope: ['openid']
+        },
+        client: createMockOpenIdClient({
+          userinfo_endpoint: undefined
+        })
+      };
+      oidcUtil.bootstrapPassportStrategy(context);
+      const passportStrategy = passportStrategySetter.mock.calls[0][1];
+
+      passportStrategy.success = function (response) {
+        expect(response.userinfo).toBe(undefined);
+        expect(response.tokens).toEqual({access_token: 'token_value'});
+        next();
+      }
+      passportStrategy.error = function(error) {
+        expect(error).toEqual(undefined);
+      };
+      passportStrategy.authenticate(createMockRedirectRequest());
+    });
+
+    it('includes verification function which returns userinfo whenever it is available', function (next) {
+      const passportStrategySetter = jest.spyOn(passport, 'use').mockImplementation(() => {});
+      const context = {
+        options: {
+          scope: ['openid', 'profile']
+        },
+        client: createMockOpenIdClient()
+      };
+
+      oidcUtil.bootstrapPassportStrategy(context);
+      const passportStrategy = passportStrategySetter.mock.calls[0][1];
+
+      passportStrategy.success = function (response) {
+        expect(response.userinfo).toEqual({cid: '123'});
+        expect(response.tokens).toEqual({access_token: 'token_value'});
+        next();
+      };
+      passportStrategy.error = function(error) {
+        expect(error).toEqual(undefined);
+      };
+      passportStrategy.authenticate(createMockRedirectRequest());
+    })
+  })
+})