From a0d10989ad6f6317aaa11cb34d45a6ba532970bc Mon Sep 17 00:00:00 2001 From: Jonathan Sambrook Date: Wed, 22 Apr 2020 14:33:35 +0100 Subject: [PATCH 1/2] Tidy up the auth plugin API mechanism to hide implementation details The caller doesn't care which plugin is being used, so this commit consolidates implementation details within auth.js The motivation for this work is to prepare for extending the auth API (to allow "advanced" LDAP to query user entry ontological state at start up), by tidying up rather than duplicating the existing mechanism. --- src/plugins/auth.js | 44 +++++++++++++++++++++++++++++++++++++++ src/plugins/auth/ldap.js | 1 + src/plugins/auth/local.js | 1 + src/server.js | 18 ++-------------- 4 files changed, 48 insertions(+), 16 deletions(-) create mode 100644 src/plugins/auth.js diff --git a/src/plugins/auth.js b/src/plugins/auth.js new file mode 100644 index 00000000..e53fb3ee --- /dev/null +++ b/src/plugins/auth.js @@ -0,0 +1,44 @@ +"use strict"; + +const log = require("../log"); +const colors = require("chalk"); + +// The order defines priority: the first available plugin is used. +// Always keep 'local' auth plugin at the end of the list; it should always be enabled. +const plugins = [require("./auth/ldap"), require("./auth/local")]; + +function unimplemented(funcName) { + log.debug( + `Auth module ${colors.bold( + module.exports.moduleName + )} doesn't implement function ${colors.bold(funcName)}` + ); +} + +// Default API implementations +module.exports = { + moduleName: "", + + // Must override: implements authentication mechanism + auth: () => unimplemented("auth"), +}; + +// local auth should always be enabled, but check here to verify +let somethingEnabled = false; + +// Override default API stubs with exports from first enabled plugin found +for (const plugin of plugins) { + if (plugin.isEnabled()) { + somethingEnabled = true; + + for (const name in plugin) { + module.exports[name] = plugin[name]; + } + + break; + } +} + +if (!somethingEnabled) { + log.error("None of the auth plugins is enabled"); +} diff --git a/src/plugins/auth/ldap.js b/src/plugins/auth/ldap.js index 18441e5e..091c892e 100644 --- a/src/plugins/auth/ldap.js +++ b/src/plugins/auth/ldap.js @@ -144,6 +144,7 @@ function isLdapEnabled() { } module.exports = { + moduleName: "ldap", auth: ldapAuth, isEnabled: isLdapEnabled, }; diff --git a/src/plugins/auth/local.js b/src/plugins/auth/local.js index e5ff367c..1b062f31 100644 --- a/src/plugins/auth/local.js +++ b/src/plugins/auth/local.js @@ -46,6 +46,7 @@ function localAuth(manager, client, user, password, callback) { } module.exports = { + moduleName: "local", auth: localAuth, isEnabled: () => true, }; diff --git a/src/server.js b/src/server.js index a5a08c06..59f7e33b 100644 --- a/src/server.js +++ b/src/server.js @@ -17,16 +17,13 @@ const net = require("net"); const Identification = require("./identification"); const changelog = require("./plugins/changelog"); const inputs = require("./plugins/inputs"); +const Auth = require("./plugins/auth"); const themes = require("./plugins/packages/themes"); themes.loadLocalThemes(); const packages = require("./plugins/packages/index"); -// The order defined the priority: the first available plugin is used -// ALways keep local auth in the end, which should always be enabled. -const authPlugins = [require("./plugins/auth/ldap"), require("./plugins/auth/local")]; - // A random number that will force clients to reload the page if it differs const serverHash = Math.floor(Date.now() * Math.random()); @@ -854,18 +851,7 @@ function performAuthentication(data) { } // Perform password checking - let auth = () => { - log.error("None of the auth plugins is enabled"); - }; - - for (let i = 0; i < authPlugins.length; ++i) { - if (authPlugins[i].isEnabled()) { - auth = authPlugins[i].auth; - break; - } - } - - auth(manager, client, data.user, data.password, authCallback); + Auth.auth(manager, client, data.user, data.password, authCallback); } function reverseDnsLookup(ip, callback) { From 878ac0d19294f05c2b63f29cffda36d50ec8913e Mon Sep 17 00:00:00 2001 From: Jonathan Sambrook Date: Wed, 22 Apr 2020 14:33:09 +0100 Subject: [PATCH 2/2] Filter user loading at startup for "advanced" LDAP Users are loaded at startup. Currently when using "advanced" LDAP authentication this is true even if they no longer have a valid entry in the LDAP server. This commit uses the existing LDAP filter (specified in config.js's searchDN used by the "advanced" LDAP mechanism) to weed out any users that no longer have the relevant LDAP entry. Local and "simple" LDAP auth mechanisms continue to use the existing load all users approach. In the "simple" LDAP case this is because we only have access to the hashed password, and so can't bind to LDAP. --- src/clientManager.js | 11 ++- src/plugins/auth.js | 6 ++ src/plugins/auth/ldap.js | 148 ++++++++++++++++++++++++++++++--------- 3 files changed, 132 insertions(+), 33 deletions(-) diff --git a/src/clientManager.js b/src/clientManager.js index 7aa8315d..ca1f55b6 100644 --- a/src/clientManager.js +++ b/src/clientManager.js @@ -6,6 +6,7 @@ const colors = require("chalk"); const crypto = require("crypto"); const fs = require("fs"); const path = require("path"); +const Auth = require("./plugins/auth"); const Client = require("./client"); const Helper = require("./helper"); const WebPush = require("./plugins/webpush"); @@ -45,7 +46,15 @@ ClientManager.prototype.loadUsers = function () { ); } - users.forEach((name) => this.loadUser(name)); + // This callback is used by Auth plugins to load users they deem acceptable + const callbackLoadUser = (user) => { + this.loadUser(user); + }; + + if (!Auth.loadUsers(users, callbackLoadUser)) { + // Fallback to loading all users + users.forEach((name) => this.loadUser(name)); + } }; ClientManager.prototype.autoloadUsers = function () { diff --git a/src/plugins/auth.js b/src/plugins/auth.js index e53fb3ee..6e951010 100644 --- a/src/plugins/auth.js +++ b/src/plugins/auth.js @@ -21,6 +21,12 @@ module.exports = { // Must override: implements authentication mechanism auth: () => unimplemented("auth"), + + // Optional to override: implements filter for loading users at start up + // This allows an auth plugin to check if a user is still acceptable, if the plugin + // can do so without access to the user's unhashed password. + // Returning 'false' triggers fallback to default behaviour of loading all users + loadUsers: () => false, }; // local auth should always be enabled, but check here to verify diff --git a/src/plugins/auth/ldap.js b/src/plugins/auth/ldap.js index 091c892e..1c93419f 100644 --- a/src/plugins/auth/ldap.js +++ b/src/plugins/auth/ldap.js @@ -3,6 +3,7 @@ const log = require("../../log"); const Helper = require("../../helper"); const ldap = require("ldapjs"); +const colors = require("chalk"); function ldapAuthCommon(user, bindDN, password, callback) { const config = Helper.config; @@ -77,41 +78,42 @@ function advancedLdapAuth(user, password, callback) { log.error("Invalid LDAP root credentials"); ldapclient.unbind(); callback(false); - } else { - ldapclient.search(base, searchOptions, function (err2, res) { - if (err2) { - log.warn(`LDAP User not found: ${userDN}`); - ldapclient.unbind(); + return; + } + + ldapclient.search(base, searchOptions, function (err2, res) { + if (err2) { + log.warn(`LDAP User not found: ${userDN}`); + ldapclient.unbind(); + callback(false); + return; + } + + let found = false; + + res.on("searchEntry", function (entry) { + found = true; + const bindDN = entry.objectName; + log.info(`Auth against LDAP ${config.ldap.url} with found bindDN ${bindDN}`); + ldapclient.unbind(); + + ldapAuthCommon(user, bindDN, password, callback); + }); + + res.on("error", function (err3) { + log.error(`LDAP error: ${err3}`); + callback(false); + }); + + res.on("end", function (result) { + ldapclient.unbind(); + + if (!found) { + log.warn(`LDAP Search did not find anything for: ${userDN} (${result.status})`); callback(false); - } else { - let found = false; - res.on("searchEntry", function (entry) { - found = true; - const bindDN = entry.objectName; - log.info( - `Auth against LDAP ${config.ldap.url} with found bindDN ${bindDN}` - ); - ldapclient.unbind(); - - ldapAuthCommon(user, bindDN, password, callback); - }); - res.on("error", function (err3) { - log.error(`LDAP error: ${err3}`); - callback(false); - }); - res.on("end", function (result) { - ldapclient.unbind(); - - if (!found) { - log.warn( - `LDAP Search did not find anything for: ${userDN} (${result.status})` - ); - callback(false); - } - }); } }); - } + }); }); } @@ -139,6 +141,87 @@ function ldapAuth(manager, client, user, password, callback) { return auth(user, password, callbackWrapper); } +/** + * Use the LDAP filter from config to check that users still exist before loading them + * via the supplied callback function. + */ + +function advancedLdapLoadUsers(users, callbackLoadUser) { + const config = Helper.config; + + const ldapclient = ldap.createClient({ + url: config.ldap.url, + tlsOptions: config.ldap.tlsOptions, + }); + + const base = config.ldap.searchDN.base; + + ldapclient.on("error", function (err) { + log.error(`Unable to connect to LDAP server: ${err}`); + }); + + ldapclient.bind(config.ldap.searchDN.rootDN, config.ldap.searchDN.rootPassword, function (err) { + if (err) { + log.error("Invalid LDAP root credentials"); + return true; + } + + const remainingUsers = new Set(users); + + const searchOptions = { + scope: config.ldap.searchDN.scope, + filter: `${config.ldap.searchDN.filter}`, + attributes: [config.ldap.primaryKey], + paged: true, + }; + + ldapclient.search(base, searchOptions, function (err2, res) { + if (err2) { + log.error(`LDAP search error: ${err2}`); + return true; + } + + res.on("searchEntry", function (entry) { + const user = entry.attributes[0]._vals[0].toString(); + + if (remainingUsers.has(user)) { + remainingUsers.delete(user); + callbackLoadUser(user); + } + }); + + res.on("error", function (err3) { + log.error(`LDAP error: ${err3}`); + }); + + res.on("end", function () { + remainingUsers.forEach((user) => { + log.warn( + `No account info in LDAP for ${colors.bold( + user + )} but user config file exists` + ); + }); + }); + }); + + ldapclient.unbind(); + }); + + return true; +} + +function ldapLoadUsers(users, callbackLoadUser) { + if ("baseDN" in Helper.config.ldap) { + // simple LDAP case can't test for user existence without access to the + // user's unhashed password, so indicate need to fallback to default + // loadUser behaviour by returning false + return false; + } + + return advancedLdapLoadUsers(users, callbackLoadUser); +} + function isLdapEnabled() { return !Helper.config.public && Helper.config.ldap.enable; } @@ -147,4 +230,5 @@ module.exports = { moduleName: "ldap", auth: ldapAuth, isEnabled: isLdapEnabled, + loadUsers: ldapLoadUsers, };