Problem with "Callback was already called" when loading plugin

sazulo

Hello everybody,

I'm currently trying to create an openid connect plugin for NodeBB, but I've got a "Callback was already called" exception at the loading of the plugin.

Here is my function :

/**
		* Creates the Passport Strategy to login with OpenID Connect.
		*
		* @param {*} strategies
		* @param {*} callback
		*/
OpenIDConnectPlugin.getStrategy = async function (strategies, callback) {
	try {
		if (!OpenIDConnectPlugin.config.enabled) {
			return callback(null, strategies);
		}
		OpenIDConnectPlugin.issuer = await Issuer.discover(OpenIDConnectPlugin.config.discoverUrl);

		const strategy = new Strategy({
			client: new OpenIDConnectPlugin.issuer.Client({
				client_id: OpenIDConnectPlugin.config.clientId,
				client_secret: OpenIDConnectPlugin.config.clientSecret,
				redirect_uris: [nconf.get('url') + AUTH_OIDC_CALLBACK_PATH],
			}),
			params: {
				// In OpenID Connect,
				// => issuer and subject in the 'openid' scope
				// => email in the 'email' scope
				// => username in the 'profile' scope ( as 'preferred_username' )
				scope: 'openid email profile',
				get nonce() {
					return uuid.v4();
				},
			},
		}, OpenIDConnectPlugin.verify);

		passport.use(strategy);

		strategies.push({
			name: strategy.name,
			url: AUTH_OIDC_LOGIN_PATH,
			callbackUrl: AUTH_OIDC_CALLBACK_PATH,
			icon: 'fa-openid',
			scope: 'user:username',
		});

		return callback(null, strategies);
	} catch (err) {
		console.log(err);
		winston.error(err);
		return callback(err);
	}
};

Here is the stacktrace of the console.log :

TypeError: Cannot read property 'length' of undefined
    at pathtoRegexp (/home/sazulo/Projects/tennotech/nodebb/nodebb/node_modules/path-to-regexp/index.js:63:49)
    at new Layer (/home/sazulo/Projects/tennotech/nodebb/nodebb/node_modules/express/lib/router/layer.js:45:17)
    at Function.route (/home/sazulo/Projects/tennotech/nodebb/nodebb/node_modules/express/lib/router/index.js:494:15)
    at Function.proto.<computed> [as get] (/home/sazulo/Projects/tennotech/nodebb/nodebb/node_modules/express/lib/router/index.js:509:22)
    at /home/sazulo/Projects/tennotech/nodebb/nodebb/src/routes/authentication.js:77:45
    at Array.forEach (<anonymous>)
    at /home/sazulo/Projects/tennotech/nodebb/nodebb/src/routes/authentication.js:65:20
    at nextTask (/home/sazulo/Projects/tennotech/nodebb/nodebb/node_modules/async/dist/async.js:5324:14)
    at next (/home/sazulo/Projects/tennotech/nodebb/nodebb/node_modules/async/dist/async.js:5331:9)
    at /home/sazulo/Projects/tennotech/nodebb/nodebb/node_modules/async/dist/async.js:969:16

Here is the UnhandledPromiseRejection :

2019-11-07T20:41:17.449Z [4567/61465] - error: Cannot read property 'length' of undefined
(node:61465) UnhandledPromiseRejectionWarning: Error: Callback was already called.
    at /home/sazulo/Projects/tennotech/nodebb/nodebb/node_modules/async/dist/async.js:966:32
    at /home/sazulo/Projects/tennotech/nodebb/nodebb/node_modules/async/dist/async.js:2518:13
    at Object.OpenIDConnectPlugin.getStrategy [as method] (/home/sazulo/Projects/tennotech/nodebb/nodebb/node_modules/nodebb-plugin-openidconnect/index.js:148:10)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)

The line 148 is the return callback(err); at the end.

<baris>

Hi @sazulo,

Since your plugin hook function is async you should not use the callback and just return strategies from the function.

Change your function to the below and it should work.

OpenIDConnectPlugin.getStrategy = async function (strategies) {
	try {
		if (!OpenIDConnectPlugin.config.enabled) {
			return strategies;
		}
		OpenIDConnectPlugin.issuer = await Issuer.discover(OpenIDConnectPlugin.config.discoverUrl);

		const strategy = new Strategy({
			client: new OpenIDConnectPlugin.issuer.Client({
				client_id: OpenIDConnectPlugin.config.clientId,
				client_secret: OpenIDConnectPlugin.config.clientSecret,
				redirect_uris: [nconf.get('url') + AUTH_OIDC_CALLBACK_PATH],
			}),
			params: {
				// In OpenID Connect,
				// => issuer and subject in the 'openid' scope
				// => email in the 'email' scope
				// => username in the 'profile' scope ( as 'preferred_username' )
				scope: 'openid email profile',
				get nonce() {
					return uuid.v4();
				},
			},
		}, OpenIDConnectPlugin.verify);

		passport.use(strategy);

		strategies.push({
			name: strategy.name,
			url: AUTH_OIDC_LOGIN_PATH,
			callbackUrl: AUTH_OIDC_CALLBACK_PATH,
			icon: 'fa-openid',
			scope: 'user:username',
		});

		return strategies;
	} catch (err) {
		console.log(err);
		winston.error(err);
		throw err;
	}
};

PitaJ

Also, you probably don't need the try catch wrapping everything since that logging is handled by NodeBB anyways.

sazulo

The real error was due to the fact that I wrote callbackUrl instead of callbackURL when pushing into the strategies.

And, as I'm developing with version 1.12.2, I can't just return the results as this version does not support promises.