Merge pull request #1636 from thelounge/xpaw/cleanup-clientmanager

Cleanup client manager functions

src/clientManager.js

@@ -1,10 +1,11 @@
 "use strict";
 
-var _ = require("lodash");
+const _ = require("lodash");
-var colors = require("colors/safe");
+const colors = require("colors/safe");
-var fs = require("fs");
+const fs = require("fs");
-var Client = require("./client");
+const path = require("path");
-var Helper = require("./helper");
+const Client = require("./client");
+const Helper = require("./helper");
 const WebPush = require("./plugins/webpush");
 
 module.exports = ClientManager;
@@ -19,6 +20,7 @@ ClientManager.prototype.init = function(identHandler, sockets) {
 	this.webPush = new WebPush();
 
 	if (!Helper.config.public && !Helper.config.ldap.enable) {
+		// TODO: Remove deprecated warning in v3.0.0
 		if ("autoload" in Helper.config) {
 			log.warn(`Autoloading users is now always enabled. Please remove the ${colors.yellow("autoload")} option from your configuration file.`);
 		}
@@ -35,12 +37,6 @@ ClientManager.prototype.autoloadUsers = function() {
 	const users = this.getUsers();
 	const noUsersWarning = `There are currently no users. Create one with ${colors.bold("lounge add <name>")}.`;
 
-	// There was an error, already logged, but we have to crash the server as
-	// user directory could not be accessed
-	if (users === undefined) {
-		process.exit(1);
-	}
-
 	if (users.length === 0) {
 		log.info(noUsersWarning);
 	}
@@ -64,84 +60,77 @@ ClientManager.prototype.autoloadUsers = function() {
 			if (client) {
 				client.quit(true);
 				this.clients = _.without(this.clients, client);
-				log.info(`User ${colors.bold(name)} disconnected and removed`);
+				log.info(`User ${colors.bold(name)} disconnected and removed.`);
 			}
 		});
 	}, 1000, {maxWait: 10000}));
 };
 
 ClientManager.prototype.loadUser = function(name) {
-	let json;
+	const user = readUserConfig(name);
-	try {
+
-		json = this.readUserConfig(name);
+	if (!user) {
-	} catch (e) {
-		log.error("Failed to read user config", e);
 		return;
 	}
-	if (!this.findClient(name)) {
+
-		this.clients.push(new Client(
+	let client = this.findClient(name);
-			this,
+
-			name,
+	if (client) {
-			json
+		log.warn(`Tried to load user ${colors.bold(name)}, which is already loaded.`);
-		));
+		return client;
 	}
+
+	client = new Client(this, name, user);
+	this.clients.push(client);
+
+	return client;
 };
 
 ClientManager.prototype.getUsers = function() {
-	var users = [];
+	return fs
-	try {
+		.readdirSync(Helper.USERS_PATH)
-		var files = fs.readdirSync(Helper.USERS_PATH);
+		.filter((file) => file.endsWith(".json"))
-		files.forEach((file) => {
+		.map((file) => file.slice(0, -5));
-			if (file.indexOf(".json") !== -1) {
-				users.push(file.replace(".json", ""));
-			}
-		});
-	} catch (e) {
-		log.error(`Failed to get users (${e})`);
-		return;
-	}
-	return users;
 };
 
 ClientManager.prototype.addUser = function(name, password, enableLog) {
-	var users = this.getUsers();
+	if (path.basename(name) !== name) {
-	if (users.indexOf(name) !== -1) {
+		throw new Error(`${name} is an invalid username.`);
-		return false;
-	}
-	try {
-		if (require("path").basename(name) !== name) {
-			throw new Error(name + " is an invalid username.");
 	}
 
-		var user = {
+	const userPath = Helper.getUserConfigPath(name);
-			user: name,
+
+	if (fs.existsSync(userPath)) {
+		log.error(`User ${colors.green(name)} already exists.`);
+		return false;
+	}
+
+	const user = {
 		password: password || "",
-			log: enableLog,
+		log: enableLog || false,
 		awayMessage: "",
 		networks: [],
 		sessions: {},
 	};
-		fs.writeFileSync(
+
-			Helper.getUserConfigPath(name),
+	try {
-			JSON.stringify(user, null, "\t")
+		fs.writeFileSync(userPath, JSON.stringify(user, null, "\t"));
-		);
 	} catch (e) {
-		log.error("Failed to add user " + name, e);
+		log.error(`Failed to create user ${colors.green(name)} (${e})`);
 		throw e;
 	}
+
 	return true;
 };
 
 ClientManager.prototype.updateUser = function(name, opts, callback) {
-	const users = this.getUsers();
+	const user = readUserConfig(name);
-	if (users.indexOf(name) === -1) {
+
-		return false;
+	if (!user) {
-	}
+		log.error(`Tried to update invalid user ${colors.green(name)}. This is most likely a bug.`);
-	if (typeof opts === "undefined") {
 		return false;
 	}
 
-	const user = this.readUserConfig(name);
 	const currentUser = JSON.stringify(user, null, "\t");
 	_.assign(user, opts);
 	const newUser = JSON.stringify(user, null, "\t");
@@ -153,7 +142,7 @@ ClientManager.prototype.updateUser = function(name, opts, callback) {
 
 	fs.writeFile(Helper.getUserConfigPath(name), newUser, (err) => {
 		if (err) {
-			log.error(`Failed to update user ${colors.green(name)} (${err})`);
+			log.error(`Failed to update user ${colors.green(name)}. (${err})`);
 		}
 
 		if (callback) {
@@ -162,24 +151,27 @@ ClientManager.prototype.updateUser = function(name, opts, callback) {
 	});
 };
 
-ClientManager.prototype.readUserConfig = function(name) {
-	var users = this.getUsers();
-	if (users.indexOf(name) === -1) {
-		return false;
-	}
-	var data = fs.readFileSync(Helper.getUserConfigPath(name), "utf-8");
-	return JSON.parse(data);
-};
-
 ClientManager.prototype.removeUser = function(name) {
-	var users = this.getUsers();
+	const userPath = Helper.getUserConfigPath(name);
-	if (users.indexOf(name) === -1) {
+
+	if (!fs.existsSync(userPath)) {
+		log.error(`Tried to remove non-existing user ${colors.green(name)}.`);
 		return false;
 	}
-	try {
+
-		fs.unlinkSync(Helper.getUserConfigPath(name));
+	fs.unlinkSync(userPath);
-	} catch (e) {
+
-		throw e;
-	}
 	return true;
 };
+
+function readUserConfig(name) {
+	const userPath = Helper.getUserConfigPath(name);
+
+	if (!fs.existsSync(userPath)) {
+		log.error(`Tried to read non-existing user ${colors.green(name)}`);
+		return false;
+	}
+
+	const data = fs.readFileSync(userPath, "utf-8");
+	return JSON.parse(data);
+}

src/server.js

@@ -512,8 +512,7 @@ function performAuthentication(data) {
 		// If authorization succeeded but there is no loaded user,
 		// load it and find the user again (this happens with LDAP)
 		if (!client) {
-			manager.loadUser(data.user);
+			client = manager.loadUser(data.user);
-			client = manager.findClient(data.user);
 		}
 
 		initClient();