Improper Access Control in jupyterhub-firstuseauthenticator

@@ -5,8 +5,11 @@
 password for that account. It is hashed with bcrypt & stored
 locally in a dbm file, and checked next time they log in.
 """
-import dbm
 import os
+import shutil
+
+import bcrypt
+import dbm
 from jinja2 import ChoiceLoader, FileSystemLoader
 from jupyterhub.auth import Authenticator
 from jupyterhub.handlers import BaseHandler
@@ -16,8 +19,6 @@
 from tornado import web
 from traitlets.traitlets import Unicode, Bool, Integer
 
-import bcrypt
-
 
 TEMPLATE_DIR = os.path.join(os.path.dirname(__file__), 'templates')
 
@@ -45,7 +46,6 @@ def __init__(self, *args, **kwargs):
         self._loaded = False
         super().__init__(*args, **kwargs)
 
-
     def _register_template_path(self):
         if self._loaded:
             return
@@ -59,14 +59,12 @@ def _register_template_path(self):
 
         self._loaded = True
 
-
     @web.authenticated
     async def get(self):
         self._register_template_path()
         html = await self.render_template('reset.html')
         self.finish(html)
 
-
     @web.authenticated
     async def post(self):
         user = self.current_user
@@ -120,6 +118,164 @@ class FirstUseAuthenticator(Authenticator):
         """
     )
 
+    check_passwords_on_startup = Bool(
+        True,
+        config=True,
+        help="""
+        Check for non-normalized-username passwords on startup.
+
+        Prior to 1.0, multiple passwords could be set for the same username,
+        without normalization.
+
+        When True, duplicate usernames will be detected and removed,
+        and ensure all usernames are normalized.
+
+        If any duplicates are found, a backup of the original is created,
+        which can be inspected manually.
+
+        Typically, this will only need to run once.
+        """,
+    )
+
+    def __init__(self, **kwargs):
+        super().__init__(**kwargs)
+        if self.check_passwords_on_startup:
+            self._check_passwords()
+
+    def _check_passwords(self):
+        """Validation checks on the password database at startup
+
+        Mainly checks for the presence of passwords for non-normalized usernames
+
+        If a username is present only in one non-normalized form,
+        it will be renamed to the normalized form.
+
+        If multiple forms of the same normalized username are present,
+        ensure that at least the normalized form is also present.
+        It will continue to produce warnings until manual intervention removes the non-normalized entries.
+
+        Non-normalized entries will never be used during login.
+        """
+
+        # it's nontrival to check for db existence, because there are so many extensions
+        # and you don't give dbm a path, you give it a *base* name,
+        # which may point to one or more paths.
+        # There's no way to retrieve the actual path(s) for a db
+        dbm_extensions = ("", ".db", ".pag", ".dir", ".dat", ".bak")
+        dbm_files = list(
+            filter(os.path.isfile, (self.dbm_path + ext for ext in dbm_extensions))
+        )
+        if not dbm_files:
+            # no database, nothing to do
+            return
+
+        backup_path = self.dbm_path + "-backup"
+        backup_files = list(
+            filter(os.path.isfile, (backup_path + ext for ext in dbm_extensions))
+        )
+
+        collision_warning = (
+            f"Duplicate password entries have been found, and stored in {backup_path!r}."
+            f" Duplicate entries have been removed from {self.dbm_path!r}."
+            f" If you are happy with the solution, you can delete the backup file(s): {' '.join(backup_files)}."
+            " Or you can inspect the backup database with:\n"
+            "    import dbm\n"
+            f"    with dbm.open({backup_path!r}, 'r') as db:\n"
+            "        for username in db.keys():\n"
+            "            print(username, db[username])\n"
+        )
+
+        if backup_files:
+            self.log.warning(collision_warning)
+            return
+
+        # create a temporary backup of the passwords db
+        # to be retained only if collisions are detected
+        # or deleted if no collisions are detected
+        backup_files = []
+        for path in dbm_files:
+            base, ext = os.path.splitext(path)
+            if ext not in dbm_extensions:
+                # catch weird names with '.' and no .db extension
+                base = path
+                ext = ""
+            backup = f"{base}-backup{ext}"
+            shutil.copyfile(path, backup)
+            backup_files.append(backup)
+
+        collision_found = False
+
+        with dbm.open(self.dbm_path, "w") as db:
+            # load the username:hashed_password dict
+            passwords = {}
+            for key in db.keys():
+                passwords[key.decode("utf8")] = db[key]
+
+            # normalization map
+            # compute the full map before checking in case two non-normalized forms are used
+            # keys are normalized usernames,
+            # values are lists of all names present in the db
+            # which normalize to the same user
+            normalized_usernames = {}
+            for username in passwords:
+                normalized_username = self.normalize_username(username)
+                normalized_usernames.setdefault(normalized_username, []).append(
+                    username
+                )
+
+            # check if any non-normalized usernames are in the db
+            for normalized_username, usernames in normalized_usernames.items():
+                # case 1. only one form, make sure it's stored in the normalized username
+                if len(usernames) == 1:
+                    username = usernames[0]
+                    # case 1.a only normalized form, nothing to do
+                    if username == normalized_username:
+                        continue
+                    # 1.b only one form, not normalized. Unambiguous to fix.
+                    # move password from non-normalized to normalized.
+                    self.log.warning(
+                        f"Normalizing username in password db {username}->{normalized_username}"
+                    )
+                    db[normalized_username.encode("utf8")] = passwords[username]
+                    del db[username]
+                else:
+                    # collision! Multiple passwords for the same Hub user with different normalization
+                    # do not clear these automatically because the 'right' answer is ambiguous,
+                    # but make sure the normalized_username is set,
+                    # so that after upgrade, there is always a password set
+                    # the non-normalized username passwords will never be used
+                    # after jupyterhub-firstuseauthenticator 1.0
+                    self.log.warning(
+                        f"{len(usernames)} variations of the username {normalized_username} present in password database: {usernames}."
+                        f" Only the password stored for the normalized {normalized_username} will be used."
+                    )
+                    collision_found = True
+                    if normalized_username not in passwords:
+                        # we choose usernames[0] as most likely to be the first entry
+                        # this isn't guaranteed, but it's the best information we have
+                        username = usernames[0]
+                        self.log.warning(
+                            f"Normalizing username in password db {username}->{normalized_username}"
+                        )
+                        db[normalized_username.encode("utf8")] = passwords[username]
+                    for username in usernames:
+                        if username != normalized_username:
+                            self.log.warning(
+                                f"Removing un-normalized username from password db {username}"
+                            )
+                            del db[username]
+
+        if collision_found:
+            self.log.warning(collision_warning)
+        else:
+            # remove backup files, if we didn't find anything to backup
+            self.log.debug(f"No collisions found, removing backup files {backup_files}")
+            for path in backup_files:
+                try:
+                    os.remove(path)
+                except FileNotFoundError:
+                    pass
+
     def _user_exists(self, username):
         """
         Return true if given user already exists.
@@ -141,19 +297,19 @@ def validate_username(self, name):
         return super().validate_username(name)
 
     async def authenticate(self, handler, data):
-        username = self.normalize_username(data['username'])
-        password = data['password']
+        username = self.normalize_username(data["username"])
+        password = data["password"]
 
         if not self.create_users:
             if not self._user_exists(username):
                 return None
 
         with dbm.open(self.dbm_path, 'c', 0o600) as db:
-            stored_pw = db.get(username.encode(), None)
+            stored_pw = db.get(username.encode("utf8"), None)
 
             if stored_pw is not None:
                 # for existing passwords: ensure password hash match
-                if bcrypt.hashpw(password.encode(), stored_pw) != stored_pw:
+                if bcrypt.hashpw(password.encode("utf8"), stored_pw) != stored_pw:
                     return None
             else:
                 # for new users: ensure password validity and store password hash
@@ -164,7 +320,7 @@ async def authenticate(self, handler, data):
                     )
                     self.log.error(handler.custom_login_error)
                     return None
-                db[username] = bcrypt.hashpw(password.encode(), bcrypt.gensalt())
+                db[username] = bcrypt.hashpw(password.encode("utf8"), bcrypt.gensalt())
 
         return username
 
@@ -181,7 +337,6 @@ def delete_user(self, user):
         except KeyError:
             pass
 
-
     def reset_password(self, username, new_password):
         """
         This allows changing the password of a logged user.
@@ -194,12 +349,14 @@ def reset_password(self, username, new_password):
             self.log.error(login_err)
             # Resetting the password will fail if the new password is too short.
             return login_err
-        with dbm.open(self.dbm_path, 'c', 0o600) as db:
-            db[username] = bcrypt.hashpw(new_password.encode(), bcrypt.gensalt())
+        with dbm.open(self.dbm_path, "c", 0o600) as db:
+            db[username] = bcrypt.hashpw(new_password.encode("utf8"), bcrypt.gensalt())
         login_msg = "Your password has been changed successfully!"
         self.log.info(login_msg)
         return login_msg
 
-
     def get_handlers(self, app):
-        return [(r'/login', CustomLoginHandler), (r'/auth/change-password', ResetPasswordHandler)]
+        return [
+            (r"/login", CustomLoginHandler),
+            (r"/auth/change-password", ResetPasswordHandler),
+        ]

@@ -1,9 +1,10 @@
 """tests for first-use authenticator"""
 
-import pytest
-import logging
 from unittest import mock
 
+import dbm
+import pytest
+
 from firstuseauthenticator import FirstUseAuthenticator
 
 
@@ -74,3 +75,120 @@ def user_exists(username):
                     'Password too short! Please choose a password at least %d characters long.'
                     % auth.min_password_length
                 )
+
+
+async def test_normalized_check(caplog, tmpcwd):
+    # cases:
+    # 1.a - normalized
+    # 1.b not normalized, no collision
+    # 2.a normalized present, collision
+    # 2.b normalized not present, collision
+    # disable normalization, populate db with duplicates
+    to_load = [
+        "onlynormalized",
+        "onlyNotNormalized",
+        "collisionnormalized",
+        "collisionNormalized",
+        "collisionNotNormalized",
+        "collisionNotnormalized",
+    ]
+
+    # load passwords
+    auth1 = FirstUseAuthenticator()
+    with mock.patch.object(auth1, "normalize_username", lambda x: x):
+        for username in to_load:
+            assert await auth1.authenticate(
+                mock.Mock(),
+                {
+                    "username": username,
+                    "password": username,
+                },
+            )
+
+    # first make sure normalization was skipped
+    with dbm.open(auth1.dbm_path) as db:
+        for username in to_load:
+            assert db.get(username.encode("utf8"))
+    # at startup, normalization is checked
+    auth2 = FirstUseAuthenticator()
+    with dbm.open(auth1.dbm_path) as db:
+        passwords = {key.decode("utf8"): db[key].decode("utf8") for key in db.keys()}
+    in_db = set(passwords)
+    # 1.a no-op
+    assert "onlynormalized" in in_db
+    # 1.b renamed
+    assert "onlynotnormalized" in in_db
+    assert "onlyNotNormalized" not in in_db
+    # 2.a collision, preserve normalized
+    assert "collisionnormalized" in in_db
+    assert "collisionNormalized" not in in_db
+    # 2.b collision, preserve and add normalized
+    assert "collisionnotnormalized" in in_db
+    assert "collisionNotNormalized" not in in_db
+    assert "collisionNotnormalized" not in in_db
+
+    # check the backup
+    with dbm.open(auth1.dbm_path + "-backup") as db:
+        backup_passwords = {
+            key.decode("utf8"): db[key].decode("utf8") for key in db.keys()
+        }
+
+    for name in to_load:
+        assert name in backup_passwords
+
+    # now verify logins
+    m = mock.Mock()
+    for username, password in (
+        ("onlynormalized", "onlynormalized"),
+        ("onlyNormalized", "onlynormalized"),
+        ("onlynotnormalized", "onlyNotNormalized"),
+        ("onlyNotNormalized", "onlyNotNormalized"),
+        ("collisionnormalized", "collisionnormalized"),
+        ("collisionNormalized", "collisionnormalized"),
+        ("collisionnotnormalized", "collisionNotNormalized"),
+        ("collisionNotNormalized", "collisionNotNormalized"),
+    ):
+        # normalized form, doesn't reset password
+        authenticated = await auth2.authenticate(
+            m,
+            {
+                "username": username,
+                "password": "firstuse",
+            },
+        )
+        assert authenticated is None
+
+        # non-normalized form, doesn't reset password
+        authenticated = await auth2.authenticate(
+            m,
+            {
+                "username": username.upper(),
+                "password": "firstuse",
+            },
+        )
+        assert authenticated is None
+
+        # normalized form, accepts correct password
+        authenticated = await auth2.authenticate(
+            m,
+            {
+                "username": username,
+                "password": password,
+            },
+        )
+        assert authenticated
+        assert authenticated == auth2.normalize_username(username)
+
+        # non-normalized form, accepts correct password
+        authenticated = await auth2.authenticate(
+            m,
+            {
+                "username": username.upper(),
+                "password": password,
+            },
+        )
+        assert authenticated
+        assert authenticated == auth2.normalize_username(username)
+
+    # load again, should skip the
+    auth3 = FirstUseAuthenticator()