Streamlined modules, server tweaks, better feedback

19 files changed, 204 insertions, 114 deletions

diff --git a/server/src/commands/admin/addmod.js b/server/src/commands/admin/addmod.js
index dba5aba..e9dde2c 100644
--- a/server/src/commands/admin/addmod.js
+++ b/server/src/commands/admin/addmod.js
@@ -1,5 +1,5 @@
 /*
-
+  Description: Adds the target trip to the mod list then elevates the uType
 */
 
 'use strict';
@@ -16,14 +16,16 @@ exports.run = async (core, server, socket, data) => {
 
   core.config.mods.push(mod); // purposely not using `config.set()` to avoid auto-save
 
-  for (let client of server.clients) {
-    if (typeof client.trip !== 'undefined' && client.trip === data.trip) {
-      client.uType = 'mod';
+  let newMod = server.findSockets({ trip: data.trip });
+
+  if (newMod.length !== 0) {
+    for (let i = 0, l = newMod.length; i < l; i++) {
+      newMod[i].uType = 'mod';
 
-      server.reply({
+      server.send({
         cmd: 'info',
         text: 'You are now a mod.'
-      }, client);
+      }, newMod[i]);
     }
   }
 
diff --git a/server/src/commands/admin/listusers.js b/server/src/commands/admin/listusers.js
index 1ddb16b..226b000 100644
--- a/server/src/commands/admin/listusers.js
+++ b/server/src/commands/admin/listusers.js
@@ -1,5 +1,5 @@
 /*
-
+  Description: Outputs all current channels and their user nicks
 */
 
 'use strict';
diff --git a/server/src/commands/admin/reload.js b/server/src/commands/admin/reload.js
index 88e40c7..387ae97 100644
--- a/server/src/commands/admin/reload.js
+++ b/server/src/commands/admin/reload.js
@@ -1,5 +1,5 @@
 /*
-
+  Description: Clears and resets the command modules, outputting any errors
 */
 
 'use strict';
@@ -14,7 +14,9 @@ exports.run = async (core, server, socket, data) => {
   loadResult += core.commands.loadCommands();
 
   if (loadResult == '') {
-    loadResult = 'Commands reloaded without errors!';
+    loadResult = `Loaded ${core.commands._commands.length} commands, 0 errors`;
+  } else {
+    loadResult = `Loaded ${core.commands._commands.length} commands, error(s): ${loadResult}`;
   }
 
   server.reply({
diff --git a/server/src/commands/admin/saveconfig.js b/server/src/commands/admin/saveconfig.js
index 4c6dff8..0c4e0c9 100644
--- a/server/src/commands/admin/saveconfig.js
+++ b/server/src/commands/admin/saveconfig.js
@@ -1,5 +1,5 @@
 /*
-
+  Description: Writes any changes to the config to the disk
 */
 
 'use strict';
diff --git a/server/src/commands/admin/shout.js b/server/src/commands/admin/shout.js
index c3cfded..736134a 100644
--- a/server/src/commands/admin/shout.js
+++ b/server/src/commands/admin/shout.js
@@ -1,5 +1,5 @@
 /*
-
+  Description: Emmits a server-wide message as `info`
 */
 
 'use strict';
diff --git a/server/src/commands/core/chat.js b/server/src/commands/core/chat.js
index c086077..ee45425 100644
--- a/server/src/commands/core/chat.js
+++ b/server/src/commands/core/chat.js
@@ -1,10 +1,10 @@
 /*
-
+  Description: Rebroadcasts any `text` to all clients in a `channel`
 */
 
 'use strict';
 
-function parseText(text) {
+const parseText = (text) => {
   if (typeof text !== 'string') {
     return false;
   }
@@ -15,7 +15,7 @@ function parseText(text) {
   text = text.replace(/\n{3,}/g, "\n\n");
 
   return text;
-}
+};
 
 exports.run = async (core, server, socket, data) => {
   let text = parseText(data.text);
diff --git a/server/src/commands/core/help.js b/server/src/commands/core/help.js
index 7b5237c..71cc745 100644
--- a/server/src/commands/core/help.js
+++ b/server/src/commands/core/help.js
@@ -1,5 +1,5 @@
 /*
-
+  Description: Outputs the current command module list or command categories
 */
 
 'use strict';
@@ -44,9 +44,8 @@ exports.run = async (core, server, socket, data) => {
   }, socket);
 };
 
-// optional parameters are marked, all others are required
 exports.info = {
-  name: 'help', // actual command name
+  name: 'help',
   usage: 'help ([ type:categories] | [category:<category name> | command:<command name> ])',
   description: 'Outputs information about the servers current protocol'
 };
diff --git a/server/src/commands/core/invite.js b/server/src/commands/core/invite.js
index 7d162d6..5889562 100644
--- a/server/src/commands/core/invite.js
+++ b/server/src/commands/core/invite.js
@@ -1,12 +1,12 @@
 /*
-
+  Description: Generates a semi-unique channel name then broadcasts it to each client
 */
 
 'use strict';
 
-function verifyNickname(nick) {
+const verifyNickname = (nick) => {
   return /^[a-zA-Z0-9_]{1,24}$/.test(nick);
-}
+};
 
 exports.run = async (core, server, socket, data) => {
   if (typeof data.nick !== 'string') {
@@ -19,7 +19,7 @@ exports.run = async (core, server, socket, data) => {
   }
 
   if (data.nick == socket.nick) {
-    // TODO: reply with something witty? They invited themself
+    // They invited themself
     return;
   }
 
diff --git a/server/src/commands/core/join.js b/server/src/commands/core/join.js
index 609e39d..e896361 100644
--- a/server/src/commands/core/join.js
+++ b/server/src/commands/core/join.js
@@ -1,20 +1,20 @@
 /*
-
+  Description: Initial entry point, applies `channel` and `nick` to the calling socket
 */
 
 'use strict';
 
 const crypto = require('crypto');
 
-function hash(password) {
-  var sha = crypto.createHash('sha256');
+const hash = (password) => {
+  let sha = crypto.createHash('sha256');
   sha.update(password);
   return sha.digest('base64').substr(0, 6);
-}
+};
 
-function verifyNickname(nick) {
+const verifyNickname = (nick) => {
   return /^[a-zA-Z0-9_]{1,24}$/.test(nick);
-}
+};
 
 exports.run = async (core, server, socket, data) => {
   if (server._police.frisk(socket.remoteAddress, 3)) {
@@ -53,7 +53,7 @@ exports.run = async (core, server, socket, data) => {
       text: 'Nickname must consist of up to 24 letters, numbers, and underscores'
     }, socket);
 
-    return
+    return;
   }
 
   for (let client of server.clients) {
@@ -99,7 +99,8 @@ exports.run = async (core, server, socket, data) => {
   server.broadcast({
     cmd: 'onlineAdd',
     nick: nick,
-    trip: trip || 'null'
+    trip: trip || 'null',
+    hash: server.getSocketHash(socket)
   }, { channel: channel });
 
   socket.uType = uType;
diff --git a/server/src/commands/core/morestats.js b/server/src/commands/core/morestats.js
new file mode 100644
index 0000000..887d663
--- /dev/null
+++ b/server/src/commands/core/morestats.js
@@ -0,0 +1,54 @@
+/*
+  Description: Outputs more info than the legacy stats command
+*/
+
+'use strict';
+
+const stripIndents = require('common-tags').stripIndents;
+
+const formatTime = (time) => {
+  let seconds = time[0] + time[1] / 1e9;
+
+  let minutes = Math.floor(seconds / 60);
+  seconds = seconds % 60;
+
+  let hours = Math.floor(minutes / 60);
+  minutes = minutes % 60;
+  return `${hours.toFixed(0)}h ${minutes.toFixed(0)}m ${seconds.toFixed(0)}s`;
+};
+
+exports.run = async (core, server, socket, data) => {
+  let ips = {};
+  let channels = {};
+  for (let client of server.clients) {
+    if (client.channel) {
+      channels[client.channel] = true;
+      ips[client.remoteAddress] = true;
+    }
+  }
+
+  let uniqueClientCount = Object.keys(ips).length;
+  let uniqueChannels = Object.keys(channels).length;
+
+  ips = null;
+  channels = null;
+
+  server.reply({
+    cmd: 'info',
+    text: stripIndents`current-connections: ${uniqueClientCount}
+                       current-channels: ${uniqueChannels}
+                       users-joined: ${(core.managers.stats.get('users-joined') || 0)}
+                       invites-sent: ${(core.managers.stats.get('invites-sent') || 0)}
+                       messages-sent: ${(core.managers.stats.get('messages-sent') || 0)}
+                       users-banned: ${(core.managers.stats.get('users-banned') || 0)}
+                       stats-requested: ${(core.managers.stats.get('stats-requested') || 0)}
+                       server-uptime: ${formatTime(process.hrtime(core.managers.stats.get('start-time')))}`
+  }, socket);
+
+  core.managers.stats.increment('stats-requested');
+};
+
+exports.info = {
+  name: 'morestats',
+  description: 'Sends back current server stats to the calling client'
+};
diff --git a/server/src/commands/core/showcase.js b/server/src/commands/core/showcase.js
index 5d15ea2..5eca4a2 100644
--- a/server/src/commands/core/showcase.js
+++ b/server/src/commands/core/showcase.js
@@ -1,5 +1,5 @@
 /*
-
+  Description: This is a template module that should not be on prod
 */
 
 'use strict';
@@ -14,21 +14,26 @@ const createReply = (echoInput) => {
   return `You want me to echo: ${echoInput}?`
 };
 
-// `exports.run()` is required and will always be passed (core, server, socket, data)
-// be sure it's asyn too
-// this is the main function
+/*
+  `exports.run()` is required and will always be passed (core, server, socket, data)
+
+  be sure it's async too
+  this is the main function that will run when called
+*/
 exports.run = async (core, server, socket, data) => {
 
   server.reply({
     cmd: 'info',
-    text: `SHOWCASE MODULE: ${core.showcase} - ${this.createReply(data.echo)}`
+    text: `SHOWCASE MODULE: ${core.showcase} - ${createReply(data.echo)}`
   }, socket);
 
 };
 
-// `exports.init()` is optional, and will only be run when the module is loaded into memory
-// it will always be passed a reference to the global core class
-// note: this will fire again if a reload is issued, keep that in mind
+/*
+  `exports.init()` is optional, and will only be run when the module is loaded into memory
+  it will always be passed a reference to the global core class
+  note: this will fire again if a reload is issued, keep that in mind
+*/
 exports.init = (core) => {
   if (typeof core.showcase === 'undefined') {
     core.showcase = 'init is a handy place to put global data by assigning it to `core`';
diff --git a/server/src/commands/core/stats.js b/server/src/commands/core/stats.js
index ba28319..a1abddb 100644
--- a/server/src/commands/core/stats.js
+++ b/server/src/commands/core/stats.js
@@ -1,22 +1,9 @@
 /*
-
+  Description: Legacy stats output, kept for compatibility, outputs user and channel count
 */
 
 'use strict';
 
-const stripIndents = require('common-tags').stripIndents;
-
-const formatTime = (time) => {
-  let seconds = time[0] + time[1] / 1e9;
-
-  let minutes = Math.floor(seconds / 60);
-  seconds = seconds % 60;
-
-  let hours = Math.floor(minutes / 60);
-  minutes = minutes % 60;
-  return `${hours.toFixed(0)}h ${minutes.toFixed(0)}m ${seconds.toFixed(0)}s`;
-};
-
 exports.run = async (core, server, socket, data) => {
   let ips = {};
   let channels = {};
@@ -35,14 +22,7 @@ exports.run = async (core, server, socket, data) => {
 
   server.reply({
     cmd: 'info',
-    text: stripIndents`current-connections: ${uniqueClientCount}
-                       current-channels: ${uniqueChannels}
-                       users-joined: ${(core.managers.stats.get('users-joined') || 0)}
-                       invites-sent: ${(core.managers.stats.get('invites-sent') || 0)}
-                       messages-sent: ${(core.managers.stats.get('messages-sent') || 0)}
-                       users-banned: ${(core.managers.stats.get('users-banned') || 0)}
-                       stats-requested: ${(core.managers.stats.get('stats-requested') || 0)}
-                       server-uptime: ${formatTime(process.hrtime(core.managers.stats.get('start-time')))}`
+    text: `${uniqueClientCount} unique IPs in ${uniqueChannels} channels`
   }, socket);
 
   core.managers.stats.increment('stats-requested');
@@ -50,5 +30,5 @@ exports.run = async (core, server, socket, data) => {
 
 exports.info = {
   name: 'stats',
-  description: 'Sends back current server stats to the calling client'
+  description: 'Sends back legacy server stats to the calling client'
 };
diff --git a/server/src/commands/mod/ban.js b/server/src/commands/mod/ban.js
index 1880ef3..5ee77b6 100644
--- a/server/src/commands/mod/ban.js
+++ b/server/src/commands/mod/ban.js
@@ -1,5 +1,5 @@
 /*
-
+  Description: Adds the target socket's ip to the ratelimiter
 */
 
 'use strict';
@@ -15,16 +15,9 @@ exports.run = async (core, server, socket, data) => {
   }
 
   let targetNick = data.nick;
-  let badClient = null;
-  for (let client of server.clients) {
-    // Find badClient's socket
-    if (client.channel == socket.channel && client.nick == targetNick) {
-      badClient = client;
-      break;
-    }
-  }
+  let badClient = server.findSockets({ channel: socket.channel, nick: targetNick });
 
-  if (!badClient) {
+  if (badClient.length === 0) {
     server.reply({
       cmd: 'warn',
       text: 'Could not find user in channel'
@@ -33,6 +26,8 @@ exports.run = async (core, server, socket, data) => {
     return;
   }
 
+  badClient = badClient[0];
+
   if (badClient.uType !== 'user') {
     server.reply({
       cmd: 'warn',
@@ -42,16 +37,21 @@ exports.run = async (core, server, socket, data) => {
     return;
   }
 
-  // TODO: add reference to banned users nick or unban by nick cmd
+  // TODO unban by hash
   server._police.arrest(badClient.remoteAddress);
-  // TODO: add event to log?
 
   console.log(`${socket.nick} [${socket.trip}] banned ${targetNick} in ${socket.channel}`);
 
   server.broadcast({
     cmd: 'info',
     text: `Banned ${targetNick}`
-  }, { channel: socket.channel });
+  }, { channel: socket.channel, uType: 'user' });
+
+  server.broadcast({
+    cmd: 'info',
+    text: `${socket.nick} banned ${targetNick} in ${socket.channel}, userhash: ${server.getSocketHash(badClient)}`
+  }, { uType: 'mod' });
+
   badClient.close();
 
   core.managers.stats.increment('users-banned');
diff --git a/server/src/commands/mod/kick.js b/server/src/commands/mod/kick.js
index a730caf..f51d576 100644
--- a/server/src/commands/mod/kick.js
+++ b/server/src/commands/mod/kick.js
@@ -1,5 +1,5 @@
 /*
-
+  Description: Forces a change on the target socket's channel, then broadcasts event
 */
 
 'use strict';
@@ -15,16 +15,9 @@ exports.run = async (core, server, socket, data) => {
   }
 
   let targetNick = data.nick;
-  let badClient = null;
-  for (let client of server.clients) {
-    // Find badClient's socket
-    if (client.channel == socket.channel && client.nick == targetNick) {
-      badClient = client;
-      break;
-    }
-  }
+  let badClient = server.findSockets({ channel: socket.channel, nick: targetNick });
 
-  if (!badClient) {
+  if (badClient.length === 0) {
     server.reply({
       cmd: 'warn',
       text: 'Could not find user in channel'
@@ -33,6 +26,8 @@ exports.run = async (core, server, socket, data) => {
     return;
   }
 
+  badClient = badClient[0];
+
   if (badClient.uType !== 'user') {
     server.reply({
       cmd: 'warn',
@@ -42,7 +37,6 @@ exports.run = async (core, server, socket, data) => {
     return;
   }
 
-  // TODO: add event to log?
   let newChannel = Math.random().toString(36).substr(2, 8);
   badClient.channel = newChannel;
 
@@ -54,7 +48,7 @@ exports.run = async (core, server, socket, data) => {
     nick: targetNick
   }, { channel: socket.channel });
 
-  // publicly broadcast event (TODO: should this be supressed?)
+  // publicly broadcast event
   server.broadcast({
     cmd: 'info',
     text: `Kicked ${targetNick}`
diff --git a/server/src/commands/mod/unban.js b/server/src/commands/mod/unban.js
index 193b614..ee028d2 100644
--- a/server/src/commands/mod/unban.js
+++ b/server/src/commands/mod/unban.js
@@ -1,5 +1,5 @@
 /*
-
+  Description: Removes a target ip from the ratelimiter
 */
 
 'use strict';
@@ -15,17 +15,32 @@ exports.run = async (core, server, socket, data) => {
   }
 
   let ip = data.ip;
-  let nick = data.nick; // for future upgrade
+  let hash = data.hash; // TODO unban by hash
+
+  // TODO unban by hash
+  let recordFound = server._police.pardon(data.ip);
+
+  if (!recordFound) {
+    server.reply({
+      cmd: 'warn',
+      text: 'Could not find target in records'
+    }, socket);
+
+    return;
+  }
 
-  // TODO: support remove by nick future upgrade
-  server._police.pardon(badClient.remoteAddress);
-  console.log(`${socket.nick} [${socket.trip}] unbanned ${/*nick || */ip} in ${socket.channel}`);
+  console.log(`${socket.nick} [${socket.trip}] unbanned ${/*hash || */ip} in ${socket.channel}`);
 
   server.reply({
     cmd: 'info',
-    text: `Unbanned ${/*nick || */ip}`
+    text: `${socket.nick} unbanned a userhash: ${server.getSocketHash(ip)}`
   }, socket);
 
+  server.broadcast({
+    cmd: 'info',
+    text: `${socket.nick} unbanned a userhash: ${server.getSocketHash(ip)}`
+  }, { uType: 'mod' });
+
   core.managers.stats.decrement('users-banned');
 };
 
diff --git a/server/src/core/rateLimiter.js b/server/src/core/rateLimiter.js
index 0f45239..f5c739b 100644
--- a/server/src/core/rateLimiter.js
+++ b/server/src/core/rateLimiter.js
@@ -97,7 +97,10 @@ class Police {
 
     if (record) {
       record.arrested = false;
+      return true;
     }
+
+    return false;
   }
 }
 
diff --git a/server/src/core/server.js b/server/src/core/server.js
index 98763af..753ed70 100644
--- a/server/src/core/server.js
+++ b/server/src/core/server.js
@@ -10,13 +10,16 @@
 'use strict';
 
 const wsServer = require('ws').Server;
+const socketReady = require('ws').OPEN;
+const crypto = require('crypto');
+const ipSalt = (Math.random().toString(36).substring(2, 16) + Math.random().toString(36).substring(2, (Math.random() * 16))).repeat(16);
 const Police = require('./rateLimiter');
 
 class server extends wsServer {
   /**
    * Create a HackChat server instance.
    *
-   * @param {Object} core Reference to the core server object
+   * @param {Object} core Reference to the global core object
    */
   constructor (core) {
     super({ port: core.config.websocketPort });
@@ -78,6 +81,7 @@ class server extends wsServer {
       return;
     }
 
+    // Start sent data verification
     var args = null;
     try {
       args = JSON.parse(data);
@@ -106,6 +110,7 @@ class server extends wsServer {
       return;
     }
 
+    // Finished verification, pass to command modules
     this._core.commands.handleCommand(this, socket, args);
   }
 
@@ -122,8 +127,8 @@ class server extends wsServer {
           nick: socket.nick
         }, { channel: socket.channel });
       }
-    } catch (e) {
-      // TODO: Should this be added to the error log?
+    } catch (err) {
+      console.log(`Server, handle close event error: ${err}`);
     }
   }
 
@@ -134,9 +139,7 @@ class server extends wsServer {
     * @param {String} err The sad stuff
     */
   handleError (socket, err) {
-    // Meh, yolo
-    // I mean;
-    // TODO: Should this be added to the error log?
+    console.log(`Server error: ${err}`);
   }
 
   /**
@@ -150,7 +153,7 @@ class server extends wsServer {
     data.time = Date.now();
 
     try {
-      if (socket.readyState == 1) { // Who says statically checking port status is bad practice? Everyone? Damnit. #TODO
+      if (socket.readyState === socketReady) {
         socket.send(JSON.stringify(data));
       }
     } catch (e) { }
@@ -170,16 +173,36 @@ class server extends wsServer {
     * Finds sockets/clients that meet the filter requirements, then passes the data to them
     *
     * @param {Object} data Object to convert to json for transmission
+    * @param {Object} filter see `this.findSockets()`
+    */
+  broadcast (data, filter) {
+    let targetSockets = this.findSockets(filter);
+
+    if (targetSockets.length === 0) {
+      return false;
+    }
+
+    for (let i = 0, l = targetSockets.length; i < l; i++) {
+      this.send(data, targetSockets[i]);
+    }
+
+    return true;
+  }
+
+  /**
+    * Finds sockets/clients that meet the filter requirements, returns result as array
+    *
+    * @param {Object} data Object to convert to json for transmission
     * @param {Object} filter The socket must of equal or greater attribs matching `filter`
     *                        = {} // matches all
     *                        = { channel: 'programming' } // matches any socket where (`socket.channel` === 'programming')
     *                        = { channel: 'programming', nick: 'Marzavec' } // matches any socket where (`socket.channel` === 'programming' && `socket.nick` === 'Marzavec')
     */
-  broadcast (data, filter) {
+  findSockets (filter) {
     let filterAttribs = Object.keys(filter);
     let reqCount = filterAttribs.length;
     let curMatch;
-    let sent = false;
+    let matches = [];
     for ( let socket of this.clients ) {
       curMatch = 0;
 
@@ -189,12 +212,29 @@ class server extends wsServer {
       }
 
       if (curMatch === reqCount) {
-        this.send(data, socket);
-        sent = true;
+        matches.push(socket);
       }
     }
 
-    return sent;
+    return matches;
+  }
+
+  /**
+    * Encrypts target socket's remote address using non-static variable length salt
+    * encodes and shortens the output, returns that value
+    *
+    * @param {Object||String} target Either the target socket or ip as string
+    */
+  getSocketHash (target) {
+    let sha = crypto.createHash('sha256');
+
+    if (typeof target === 'string') {
+      sha.update(target + ipSalt);
+    } else {
+      sha.update(target.remoteAddress + ipSalt);
+    }
+
+    return sha.digest('base64').substr(0, 15);
   }
 }
 
diff --git a/server/src/managers/commands.js b/server/src/managers/commands.js
index 95d0f1b..569206d 100644
--- a/server/src/managers/commands.js
+++ b/server/src/managers/commands.js
@@ -17,7 +17,7 @@ class CommandManager {
   /**
     * Create a `CommandManager` instance for handling commands/protocol
     *
-    * @param {Object} core reference to the global core object
+    * @param {Object} core Reference to the global core object
     */
   constructor (core) {
     this.core = core;
@@ -57,8 +57,7 @@ class CommandManager {
     let error = this._validateCommand(command);
 
     if (error) {
-      // TODO: Add to logger?
-      let errText = `Failed to load '${name}': ${error}\n\n`;
+      let errText = `Failed to load '${name}': ${error}`;
       console.log(errText);
       return errText;
     }
@@ -82,8 +81,7 @@ class CommandManager {
       try {
         command.init(this.core);
       } catch (err) {
-        // TODO: Add to logger?
-        let errText = `Failed to initialize '${name}': ${err}\n\n`;
+        let errText = `Failed to initialize '${name}': ${err}`;
         console.log(errText);
         return errText;
       }
@@ -227,8 +225,7 @@ class CommandManager {
     try {
       return await command.run(this.core, server, socket, data);
     } catch (err) {
-      // TODO: Add to logger?
-      let errText = `Failed to execute '${command.info.name}': ${err}\n\n`;
+      let errText = `Failed to execute '${command.info.name}': ${err}`;
       console.log(errText);
 
       server.reply({
diff --git a/server/src/managers/config.js b/server/src/managers/config.js
index 2ff4a88..1848cac 100644
--- a/server/src/managers/config.js
+++ b/server/src/managers/config.js
@@ -202,10 +202,8 @@ class ConfigManager {
       fse.removeSync(backupPath);
 
       return true;
-    } catch (e) {
-      // TODO: restore backup
-      // TODO: output to logging engine?
-      console.log('Failed to save config file!');
+    } catch (err) {
+      console.log(`Failed to save config file: ${err}`);
 
       return false;
     }