From f7ad1089a511da9cc425ee3dc5344d25b240056c Mon Sep 17 00:00:00 2001
From: James Valleroy <james.valleroy@gmail.com>
Date: Mon, 4 Nov 2013 00:30:21 +0000
Subject: [PATCH] Update tests for auth module, and fix some bugs discovered in
 auth module.

---
 modules/installed/lib/auth.py | 30 +++++++++++++-----
 tests/auth_test.py            | 58 +++++++++++++++++------------------
 2 files changed, 50 insertions(+), 38 deletions(-)

diff --git a/modules/installed/lib/auth.py b/modules/installed/lib/auth.py
index 23cd16e7a..cf683bd46 100644
--- a/modules/installed/lib/auth.py
+++ b/modules/installed/lib/auth.py
@@ -9,22 +9,30 @@
 import cherrypy
 import urllib
 from passlib.hash import bcrypt
+from passlib.exc import PasswordSizeError
 import cfg
 import random
 import time
+from model import User
 
 cfg.session_key = '_cp_username'
 
 def add_user(username, passphrase, name='', email='', expert=False):
+    """Add a new user with specified username and passphrase.
+    """
     error = None
     if not username: error = "Must specify a username!"
-    if not password: error = "Must specify a password!"
+    if not passphrase: error = "Must specify a passphrase!"
 
     if error is None:
         # hash the password whether the user exists, to foil timing
         # side-channel attacks
-        pass_hash = bcrypt.encrypt(password)
+        try:
+            pass_hash = bcrypt.encrypt(passphrase)
+        except PasswordSizeError:
+            error = "Password is too long."
 
+    if error is None:
         if username in cfg.users.get_all():
             error = "User already exists!"
         else:
@@ -38,7 +46,7 @@ def add_user(username, passphrase, name='', email='', expert=False):
                 'salt':pass_hash[7:29], # for bcrypt
             }
             new_user = User(di)
-            cfg.users.set(username,newuser)
+            cfg.users.set(username,new_user)
 
     if error:
         cfg.log(error)
@@ -55,13 +63,19 @@ def check_credentials(username, passphrase):
         cfg.log(error)
         return error
 
-    if username in cfg.users:
-        u = cfg.users[username]
-    else:
-        u = None
     # hash the password whether the user exists, to foil timing
     # side-channel attacks
-    pass_hash = bcrypt.encrypt(passphrase, salt=u['salt'])
+    try:
+        if username in cfg.users:
+            u = cfg.users[username]
+            pass_hash = bcrypt.encrypt(passphrase, salt=u['salt'])
+        else:
+            u = None
+            pass_hash = bcrypt.encrypt(passphrase)
+    except PasswordSizeError:
+        error = "Password is too long."
+        cfg.log(error)
+        return error
 
     if u is None or u['passphrase'] != pass_hash:
         error = "Bad user-name or password."
diff --git a/tests/auth_test.py b/tests/auth_test.py
index 861a51d59..589025987 100644
--- a/tests/auth_test.py
+++ b/tests/auth_test.py
@@ -26,45 +26,43 @@ class Auth(unittest.TestCase):
             cfg.users.remove(user[0])
         cfg.users.close()
 
-    def test_password_check(self):
-        self.add_user("test_user", "password")
+    def test_add_user(self):
+        self.assertIsNone(auth.add_user("test_user", "password"))
+        self.assertIsNotNone(auth.add_user(None, "password"))
+        self.assertIsNotNone(auth.add_user("test_user", None))
+        self.assertIsNotNone(auth.add_user("test_user", "password"))
 
-        # check_credentials returns None if there is no error, or returns error string
+    def test_password_check(self):
+        auth.add_user("test_user", "password")
+
+        # check_credentials returns None if there is no error,
+        # or returns error string
         self.assertIsNone(auth.check_credentials("test_user", "password"))
         self.assertIsNotNone(auth.check_credentials("test_user", "wrong"))
 
-    def test_salt_check(self):
-        test_user = self.add_user("test_user", "password", "abcdefghijklmnopqrstuv")
+    def test_nonexistent_user(self):
         self.assertIsNotNone(auth.check_credentials("test_user", "password"))
 
+    def test_password_too_long(self):
+        password = "x" * 4097
+        self.assertIsNotNone(auth.add_user("test_user", password))
+        self.assertIsNotNone(auth.check_credentials("test_user", password))
+
     def test_salt_is_random(self):
-        test_user1 = self.add_user("test_user1", "password")
-        test_user2 = self.add_user("test_user2", "password")
-        self.assertNotEqual(test_user1["salt"], test_user2["salt"])
+        auth.add_user("test_user1", "password")
+        auth.add_user("test_user2", "password")
+        self.assertNotEqual(
+            cfg.users["test_user1"]["salt"],
+            cfg.users["test_user2"]["salt"]
+            )
 
     def test_hash_is_random(self):
-        test_user1 = self.add_user("test_user1", "password")
-        test_user2 = self.add_user("test_user2", "password")
-        self.assertNotEqual(test_user1["passphrase"], test_user2["passphrase"])
-
-    # set fake_salt if you want to store an invalid salt
-    def add_user(self, test_username, passphrase='', fake_salt=None):
-        test_user = self.create_user(test_username, passphrase, fake_salt)
-        cfg.users.set(test_username,test_user)
-        return test_user
-
-    def create_user(self, username, passphrase='', fake_salt=None):
-        test_user = User()
-        test_user["username"] = username
-        pass_hash = bcrypt.encrypt(passphrase)
-        test_user["passphrase"] = pass_hash
-        if fake_salt:
-            test_user["salt"] = fake_salt
-        else:
-            # for bcrypt, the salt is output as part of the hash
-            test_user["salt"] = pass_hash[7:29]
-
-        return test_user
+        auth.add_user("test_user1", "password")
+        auth.add_user("test_user2", "password")
+        self.assertNotEqual(
+            cfg.users["test_user1"]["passphrase"],
+            cfg.users["test_user2"]["passphrase"]
+            )
 
 if __name__ == "__main__":
     unittest.main()