A few minor cleanups

changelog

@@ -1,3 +1,7 @@
+Thu Sep 26 13:17 2013 CDT
+    * lib/user.js: A few minor cleanups to login functions
+    * lib/api.js: Pass the login failure reason to the action log
+
 Tue Sep 24 15:18 2013 CDT
     * www/assets/js/callbacks.js: Double fix search result buttons because
       the paginator was being duplicated

lib/api.js

@@ -164,7 +164,7 @@ module.exports = function (Server) {
         db.userLogin(name, pw, session, function (err, row) {
             if(err) {
                 if(err !== "Session expired")
-                    ActionLog.record(getIP(req), name, "login-failure");
+                    ActionLog.record(getIP(req), name, "login-failure", err);
                 res.jsonp({
                     success: false,
                     error: err

lib/user.js

@@ -577,83 +577,93 @@ User.prototype.initCallbacks = function() {
 }
 
 var lastguestlogin = {};
+User.prototype.guestLogin = function (name) {
+    var self = this;
+
+    if (self.ip in lastguestlogin) {
+        var diff = (Date.now() - lastguestlogin[self.ip])/1000;
+        if (diff < self.server.cfg["guest-login-delay"]) {
+            self.socket.emit("login", {
+                success: false,
+                error: "Guest logins are restricted to one per IP address "+
+                       "per " + self.server.cfg["guest-login-delay"] +
+                       " seconds.",
+            });
+            return false;
+        }
+    }
+
+    if(!$util.isValidUserName(name)) {
+        self.socket.emit("login", {
+            success: false,
+            error: "Invalid username.  Usernames must be 1-20 characters "+
+                   "long and consist only of characters a-z, A-Z, 0-9, -, "+
+                   "and _"
+        });
+        return;
+    }
+
+    // Set the loggingIn flag to avoid race conditions with the callback
+    self.loggingIn = true;
+    self.server.db.isUsernameTaken(name, function (err, taken) {
+        self.loggingIn = false;
+        if(err) {
+            self.socket.emit("login", {
+                success: false,
+                error: "Internal error: " + err
+            });
+            return;
+        }
+
+        if(taken) {
+            self.socket.emit("login", {
+                success: false,
+                error: "That username is registered and protected."
+            });
+            return;
+        }
+
+        if(self.inChannel()) {
+            for(var i = 0; i < self.channel.users.length; i++) {
+                if(self.channel.users[i].name == name) {
+                    self.socket.emit("login", {
+                        success: false,
+                        error: "That name is already in use on this channel"
+                    });
+                    return;
+                }
+            }
+        }
+        lastguestlogin[self.ip] = Date.now();
+        self.rank = 0;
+        Logger.syslog.log(self.ip + " signed in as " + name);
+        self.server.db.recordVisit(self.ip, name);
+        self.name = name;
+        self.loggedIn = false;
+        self.socket.emit("login", {
+            success: true,
+            name: name
+        });
+        self.socket.emit("rank", self.rank);
+        if(self.inChannel()) {
+            self.channel.logger.log(self.ip + " signed in as " + name);
+            self.channel.broadcastNewUser(self);
+        }
+    });
+}
 // Attempt to login
 User.prototype.login = function(name, pw, session) {
     var self = this;
     // No password => try guest login
     if(pw == "" && session == "") {
-        if(self.ip in lastguestlogin) {
+        this.guestLogin(name);
-            var diff = (Date.now() - lastguestlogin[self.ip])/1000;
-            if(diff < self.server.cfg["guest-login-delay"]) {
-                self.socket.emit("login", {
-                    success: false,
-                    error: ["Guest logins are restricted to one per ",
-                            self.server.cfg["guest-login-delay"]
-                            + " seconds per IP.  ",
-                            "This restriction does not apply to registered users."
-                            ].join("")
-                });
-                return false;
-            }
-        }
-        if(!$util.isValidUserName(name)) {
-            self.socket.emit("login", {
-                success: false,
-                error: "Invalid username.  Usernames must be 1-20 characters long and consist only of alphanumeric characters and underscores"
-            });
-            return;
-        }
-
-        self.server.db.isUsernameTaken(name, function (err, taken) {
-            if(err) {
-                self.socket.emit("login", {
-                    success: false,
-                    error: "Internal error: " + err
-                });
-                return;
-            }
-
-            if(taken) {
-                self.socket.emit("login", {
-                    success: false,
-                    error: "That username is taken"
-                });
-                return;
-            }
-
-            if(self.inChannel()) {
-                for(var i = 0; i < self.channel.users.length; i++) {
-                    if(self.channel.users[i].name == name) {
-                        self.socket.emit("login", {
-                            success: false,
-                            error: "That name is already taken on self channel"
-                        });
-                        return;
-                    }
-                }
-            }
-            lastguestlogin[self.ip] = Date.now();
-            self.rank = Rank.Guest;
-            Logger.syslog.log(self.ip + " signed in as " + name);
-            self.server.db.recordVisit(self.ip, name);
-            self.name = name;
-            self.loggedIn = false;
-            self.socket.emit("login", {
-                success: true,
-                name: name
-            });
-            self.socket.emit("rank", self.rank);
-            if(self.inChannel()) {
-                self.channel.logger.log(self.ip + " signed in as " + name);
-                self.channel.broadcastNewUser(self);
-            }
-        });
     } else {
         self.loggingIn = true;
         self.server.db.userLogin(name, pw, session, function (err, row) {
             if(err) {
                 self.loggingIn = false;
-                self.server.actionlog.record(self.ip, name, "login-failure");
+                self.server.actionlog.record(self.ip, name, "login-failure",
+                                             err);
                 self.socket.emit("login", {
                     success: false,
                     error: err
@@ -661,17 +671,20 @@ User.prototype.login = function(name, pw, session) {
                 return;
             }
             if(self.inChannel()) {
+                var n = name.toLowerCase();
                 for(var i = 0; i < self.channel.users.length; i++) {
-                    if(self.channel.users[i].name.toLowerCase() == name.toLowerCase()) {
+                    if(self.channel.users[i].name.toLowerCase() === n) {
-                        if (self.channel.users[i] == self) {
+                        if (self.channel.users[i] === self) {
                             Logger.errlog.log("Wat: user.login() but user "+
                                               "already logged in on channel");
                             break;
                         }
-                        self.channel.kick(self.channel.users[i], "Duplicate login");
+                        self.channel.kick(self.channel.users[i], 
+                                          "Duplicate login");
                     }
                 }
             }
+            // Record logins for administrator accounts
             if(self.global_rank >= 255)
                 self.server.actionlog.record(self.ip, name, "login-success");
             self.loggedIn = true;
@@ -703,6 +716,8 @@ User.prototype.login = function(name, pw, session) {
                         self.saverank = true;
                         self.rank = rank;
                     } else {
+                        // If there was an error in retrieving the rank,
+                        // don't overwrite it with a bad value
                         self.saverank = false;
                         self.rank = self.global_rank;
                     }