From c1920eb566838b5245733ce2d93cf79e7cba0299 Mon Sep 17 00:00:00 2001 From: Pavel Djundik Date: Sun, 15 Dec 2019 16:56:17 +0200 Subject: [PATCH 1/6] When updating user file, write to temp file first --- src/clientManager.js | 9 ++++++++- src/command-line/users/reset.js | 15 ++++++++++++--- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/clientManager.js b/src/clientManager.js index 4e617519..4dc05385 100644 --- a/src/clientManager.js +++ b/src/clientManager.js @@ -195,8 +195,15 @@ ClientManager.prototype.updateUser = function(name, opts, callback) { return callback ? callback() : true; } + const pathReal = Helper.getUserConfigPath(name); + const pathTemp = pathReal + ".tmp"; + try { - fs.writeFileSync(Helper.getUserConfigPath(name), newUser); + // Write to a temp file first, in case the write fails + // we do not lose the original file (for example when disk is full) + fs.writeFileSync(pathTemp, newUser); + fs.renameSync(pathTemp, pathReal); + return callback ? callback() : true; } catch (e) { log.error(`Failed to update user ${colors.green(name)} (${e})`); diff --git a/src/command-line/users/reset.js b/src/command-line/users/reset.js index d81a36ff..ddb550fb 100644 --- a/src/command-line/users/reset.js +++ b/src/command-line/users/reset.js @@ -30,8 +30,10 @@ program return; } - const file = Helper.getUserConfigPath(name); - const user = require(file); + const pathReal = Helper.getUserConfigPath(name); + const pathTemp = pathReal + ".tmp"; + const user = JSON.parse(fs.readFileSync(pathReal, "utf-8")); + log.prompt( { text: "Enter new password:", @@ -44,7 +46,14 @@ program user.password = Helper.password.hash(password); user.sessions = {}; - fs.writeFileSync(file, JSON.stringify(user, null, "\t")); + + const newUser = JSON.stringify(user, null, "\t"); + + // Write to a temp file first, in case the write fails + // we do not lose the original file (for example when disk is full) + fs.writeFileSync(pathTemp, newUser); + fs.renameSync(pathTemp, pathReal); + log.info(`Successfully reset password for ${colors.bold(name)}.`); } ); From def56dc6942e5cdfe10cb920e2c4fd8ad419330a Mon Sep 17 00:00:00 2001 From: Pavel Djundik Date: Sun, 15 Dec 2019 17:07:17 +0200 Subject: [PATCH 2/6] Update user file once on auth --- src/client.js | 1 + src/clientManager.js | 2 ++ src/server.js | 19 +++++++------------ 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/client.js b/src/client.js index b7e69b10..f7dffa1f 100644 --- a/src/client.js +++ b/src/client.js @@ -316,6 +316,7 @@ Client.prototype.updateSession = function(token, ip, request) { }); client.manager.updateUser(client.name, { + browser: client.config.browser, sessions: client.config.sessions, }); }; diff --git a/src/clientManager.js b/src/clientManager.js index 4dc05385..e84ed37b 100644 --- a/src/clientManager.js +++ b/src/clientManager.js @@ -190,8 +190,10 @@ ClientManager.prototype.updateUser = function(name, opts, callback) { _.assign(user, opts); const newUser = JSON.stringify(user, null, "\t"); + // Do not touch the disk if object has not changed if (currentUser === newUser) { + console.log("same"); return callback ? callback() : true; } diff --git a/src/server.js b/src/server.js index 68d53d34..bf1185e4 100644 --- a/src/server.js +++ b/src/server.js @@ -664,15 +664,19 @@ function initializeClient(socket, client, token, lastMessage, openChannel) { socket.emit("commands", inputs.getCommands()); }; - if (!Helper.config.public && token === null) { + if (Helper.config.public) { + sendInitEvent(null); + } else if (token === null) { client.generateToken((newToken) => { - client.attachedClients[socket.id].token = token = client.calculateTokenHash(newToken); + token = client.calculateTokenHash(newToken); + client.attachedClients[socket.id].token = token; client.updateSession(token, getClientIp(socket), socket.request); sendInitEvent(newToken); }); } else { + client.updateSession(token, getClientIp(socket), socket.request); sendInitEvent(null); } } @@ -734,16 +738,9 @@ function performAuthentication(data) { let client; let token = null; - const finalInit = () => { + const finalInit = () => initializeClient(socket, client, token, data.lastMessage || -1, data.openChannel); - if (!Helper.config.public) { - client.manager.updateUser(client.name, { - browser: client.config.browser, - }); - } - }; - const initClient = () => { // Configuration does not change during runtime of TL, // and the client listens to this event only once @@ -827,8 +824,6 @@ function performAuthentication(data) { if (Object.prototype.hasOwnProperty.call(client.config.sessions, providedToken)) { token = providedToken; - client.updateSession(providedToken, getClientIp(socket), socket.request); - return authCallback(true); } } From f269ac3bee9faaea0b5134ab5407780bfdfbf6eb Mon Sep 17 00:00:00 2001 From: Pavel Djundik Date: Sun, 15 Dec 2019 17:26:18 +0200 Subject: [PATCH 3/6] Update user file without reading, debounce all saves --- src/client.js | 40 ++++++++++++++-------------------------- src/clientManager.js | 26 +++++++------------------- src/server.js | 8 ++------ 3 files changed, 23 insertions(+), 51 deletions(-) diff --git a/src/client.js b/src/client.js index f7dffa1f..8a9f19a3 100644 --- a/src/client.js +++ b/src/client.js @@ -315,29 +315,23 @@ Client.prototype.updateSession = function(token, ip, request) { agent: friendlyAgent, }); - client.manager.updateUser(client.name, { - browser: client.config.browser, - sessions: client.config.sessions, - }); + client.save(); }; Client.prototype.setPassword = function(hash, callback) { const client = this; - client.manager.updateUser( - client.name, - { - password: hash, - }, - function(err) { - if (err) { - return callback(false); - } - - client.config.password = hash; - return callback(true); + const oldHash = client.config.password; + client.config.password = hash; + client.manager.saveUser(client, function(err) { + if (err) { + // If user file fails to write, reset it back + client.config.password = oldHash; + return callback(false); } - ); + + return callback(true); + }); }; Client.prototype.input = function(data) { @@ -662,9 +656,7 @@ Client.prototype.registerPushSubscription = function(session, subscription, noSa session.pushSubscription = data; if (!noSave) { - this.manager.updateUser(this.name, { - sessions: this.config.sessions, - }); + this.save(); } return data; @@ -672,9 +664,7 @@ Client.prototype.registerPushSubscription = function(session, subscription, noSa Client.prototype.unregisterPushSubscription = function(token) { this.config.sessions[token].pushSubscription = null; - this.manager.updateUser(this.name, { - sessions: this.config.sessions, - }); + this.save(); }; Client.prototype.save = _.debounce( @@ -684,9 +674,7 @@ Client.prototype.save = _.debounce( } const client = this; - const json = {}; - json.networks = this.networks.map((n) => n.export()); - client.manager.updateUser(client.name, json); + client.manager.saveUser(client); }, 1000, {maxWait: 10000} diff --git a/src/clientManager.js b/src/clientManager.js index e84ed37b..a591554b 100644 --- a/src/clientManager.js +++ b/src/clientManager.js @@ -179,25 +179,13 @@ ClientManager.prototype.addUser = function(name, password, enableLog) { return true; }; -ClientManager.prototype.updateUser = function(name, opts, callback) { - const user = readUserConfig(name); +ClientManager.prototype.saveUser = function(client, callback) { + const json = Object.assign({}, client.config, { + networks: client.networks.map((n) => n.export()), + }); + const newUser = JSON.stringify(json, null, "\t"); - if (!user) { - return callback ? callback(true) : false; - } - - const currentUser = JSON.stringify(user, null, "\t"); - _.assign(user, opts); - const newUser = JSON.stringify(user, null, "\t"); - - - // Do not touch the disk if object has not changed - if (currentUser === newUser) { - console.log("same"); - return callback ? callback() : true; - } - - const pathReal = Helper.getUserConfigPath(name); + const pathReal = Helper.getUserConfigPath(client.name); const pathTemp = pathReal + ".tmp"; try { @@ -208,7 +196,7 @@ ClientManager.prototype.updateUser = function(name, opts, callback) { return callback ? callback() : true; } catch (e) { - log.error(`Failed to update user ${colors.green(name)} (${e})`); + log.error(`Failed to update user ${colors.green(client.name)} (${e})`); if (callback) { callback(e); diff --git a/src/server.js b/src/server.js index bf1185e4..da3248eb 100644 --- a/src/server.js +++ b/src/server.js @@ -591,9 +591,7 @@ function initializeClient(socket, client, token, lastMessage, openChannel) { value: newSetting.value, }); - client.manager.updateUser(client.name, { - clientSettings: client.config.clientSettings, - }); + client.save(); if (newSetting.name === "highlights") { client.compileCustomHighlights(); @@ -630,9 +628,7 @@ function initializeClient(socket, client, token, lastMessage, openChannel) { delete client.config.sessions[tokenToSignOut]; - client.manager.updateUser(client.name, { - sessions: client.config.sessions, - }); + client.save(); _.map(client.attachedClients, (attachedClient, socketId) => { if (attachedClient.token !== tokenToSignOut) { From 2365c9489eb3e15ca30ddd300ad7e35db417bf3d Mon Sep 17 00:00:00 2001 From: Pavel Djundik Date: Sun, 15 Dec 2019 17:29:39 +0200 Subject: [PATCH 4/6] Enforce user file types at runtime --- src/client.js | 3 +++ src/clientManager.js | 4 ---- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/client.js b/src/client.js index 8a9f19a3..909824f5 100644 --- a/src/client.js +++ b/src/client.js @@ -62,6 +62,9 @@ function Client(manager, name, config = {}) { const client = this; + client.config.log = Boolean(client.config.log); + client.config.password = String(client.config.password); + if (!Helper.config.public && client.config.log) { if (Helper.config.messageStorage.includes("sqlite")) { client.messageStorage.push(new MessageStorage(client)); diff --git a/src/clientManager.js b/src/clientManager.js index a591554b..dfe8a673 100644 --- a/src/clientManager.js +++ b/src/clientManager.js @@ -134,10 +134,6 @@ ClientManager.prototype.addUser = function(name, password, enableLog) { const user = { password: password || "", log: enableLog, - networks: [], - sessions: {}, - clientSettings: {}, - browser: {}, }; try { From 609151463044f95ef1c7d9f003ef62b24169183b Mon Sep 17 00:00:00 2001 From: Pavel Djundik Date: Wed, 18 Dec 2019 11:06:20 +0200 Subject: [PATCH 5/6] Do not write to disk if the json data hasn't actually changed --- src/client.js | 6 ++++-- src/clientManager.js | 18 +++++++++++++++++- test/tests/customhighlights.js | 6 ++++++ 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/client.js b/src/client.js index 909824f5..69a87c70 100644 --- a/src/client.js +++ b/src/client.js @@ -132,6 +132,8 @@ function Client(manager, name, config = {}) { delay += 1000 + Math.floor(Math.random() * 1000); }); + + client.fileHash = manager.getDataToSave(client).newHash; } } @@ -679,6 +681,6 @@ Client.prototype.save = _.debounce( const client = this; client.manager.saveUser(client); }, - 1000, - {maxWait: 10000} + 5000, + {maxWait: 20000} ); diff --git a/src/clientManager.js b/src/clientManager.js index dfe8a673..df774451 100644 --- a/src/clientManager.js +++ b/src/clientManager.js @@ -3,6 +3,7 @@ const _ = require("lodash"); const log = require("./log"); const colors = require("chalk"); +const crypto = require("crypto"); const fs = require("fs"); const path = require("path"); const Client = require("./client"); @@ -175,11 +176,26 @@ ClientManager.prototype.addUser = function(name, password, enableLog) { return true; }; -ClientManager.prototype.saveUser = function(client, callback) { +ClientManager.prototype.getDataToSave = function(client) { const json = Object.assign({}, client.config, { networks: client.networks.map((n) => n.export()), }); const newUser = JSON.stringify(json, null, "\t"); + const newHash = crypto + .createHash("sha256") + .update(newUser) + .digest("hex"); + + return {newUser, newHash}; +}; + +ClientManager.prototype.saveUser = function(client, callback) { + const {newUser, newHash} = this.getDataToSave(client); + + // Do not write to disk if the exported data hasn't actually changed + if (client.fileHash === newHash) { + return; + } const pathReal = Helper.getUserConfigPath(client.name); const pathTemp = pathReal + ".tmp"; diff --git a/test/tests/customhighlights.js b/test/tests/customhighlights.js index 9d4d37d8..a151fef8 100644 --- a/test/tests/customhighlights.js +++ b/test/tests/customhighlights.js @@ -13,6 +13,12 @@ describe("Custom highlights", function() { const client = new Client( { clients: [], + getDataToSave() { + return { + newUser: "", + newHash: "", + }; + }, }, "test", { From 0d7b980f909a9a19b2910b8d129e170be92f03e8 Mon Sep 17 00:00:00 2001 From: Pavel Djundik Date: Wed, 18 Dec 2019 11:22:11 +0200 Subject: [PATCH 6/6] Remove unnecessary client.sockets ref --- src/client.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/client.js b/src/client.js index 69a87c70..4ff2a001 100644 --- a/src/client.js +++ b/src/client.js @@ -54,7 +54,6 @@ function Client(manager, name, config = {}) { idMsg: 1, name: name, networks: [], - sockets: manager.sockets, manager: manager, messageStorage: [], highlightRegex: null, @@ -145,8 +144,8 @@ Client.prototype.createChannel = function(attr) { }; Client.prototype.emit = function(event, data) { - if (this.sockets !== null) { - this.sockets.in(this.id).emit(event, data); + if (this.manager !== null) { + this.manager.sockets.in(this.id).emit(event, data); } }; @@ -574,7 +573,7 @@ Client.prototype.names = function(data) { }; Client.prototype.quit = function(signOut) { - const sockets = this.sockets.sockets; + const sockets = this.manager.sockets.sockets; const room = sockets.adapter.rooms[this.id]; if (room && room.sockets) {