Repository: jupyterhub/nativeauthenticator

Issue:
Add username sanitization

Discussion:

Patch:
diff --git a/docs/index.rst b/docs/index.rst
index 3d2a5de..2c2d480 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -46,7 +46,9 @@ Then, you must create the configuration file for JupyterHub:
 
 And change the default Authenticator class for our Native Authenticator class:
 
-`c.JupyterHub.authenticator_class = 'nativeauthenticator.NativeAuthenticator'`
+.. code-block:: python
+
+    c.JupyterHub.authenticator_class = 'nativeauthenticator.NativeAuthenticator'
 
 
 Run your JupyterHub normally, and the authenticator will be running with it.
@@ -66,10 +68,16 @@ If you are and admin, be sure that your username is listed on the `admin_users`
 If you create a new user that is listed as an admin on the config file, it will automatically have access to the system just after the signup. 
 
 
+Usernames restrictions
+----------------------
+
+Usernames can't contain commas, whitespaces, slashes or be empty. If any of these are in the username on signup, the user won't be able to do the signup. 
+
+
 Authorize new users
 -------------------
 
-To authorize new users to enter the system or to manage those that already have access to the system you can go to `<ip:port>/hub/authorize`. 
+To authorize new users to enter the system or to manage those that already have access to the system you can go to `/hub/authorize`. 
 
 
 Change password
diff --git a/nativeauthenticator/handlers.py b/nativeauthenticator/handlers.py
index 49325e8..9798b31 100644
--- a/nativeauthenticator/handlers.py
+++ b/nativeauthenticator/handlers.py
@@ -43,9 +43,17 @@ class SignUpHandler(LocalBase):
                        'home page and log in the system')
         if not user:
             alert = 'alert-danger'
-            message = ('Something went wrong. Be sure your password has at '
-                       f'least {self.authenticator.minimum_password_length} '
-                       'characters and is not too common.')
+            pw_len = self.authenticator.minimum_password_length
+
+            if pw_len:
+                message = ("Something went wrong. Be sure your password has "
+                           f"at least {pw_len} characters, doesn't have "
+                           "spaces or commas and is not too common.")
+            else:
+                message = ("Something went wrong. Be sure your password "
+                           " doesn't have spaces or commas and is not too "
+                           "common.")
+
         return alert, message
 
     async def post(self):
diff --git a/nativeauthenticator/nativeauthenticator.py b/nativeauthenticator/nativeauthenticator.py
index 6c8b934..3cf27b9 100644
--- a/nativeauthenticator/nativeauthenticator.py
+++ b/nativeauthenticator/nativeauthenticator.py
@@ -133,7 +133,8 @@ class NativeAuthenticator(Authenticator):
         if user:
             return user
 
-        if not self.is_password_strong(pw):
+        if not self.is_password_strong(pw) or \
+           not self.validate_username(username):
             return
 
         encoded_pw = bcrypt.hashpw(pw.encode(), bcrypt.gensalt())
@@ -150,6 +151,12 @@ class NativeAuthenticator(Authenticator):
         user.password = bcrypt.hashpw(new_password.encode(), bcrypt.gensalt())
         self.db.commit()
 
+    def validate_username(self, username):
+        invalid_chars = [',', ' ']
+        if any((char in username) for char in invalid_chars):
+            return False
+        return super().validate_username(username)
+
     def get_handlers(self, app):
         native_handlers = [
             (r'/signup', SignUpHandler),


Test patch:
diff --git a/nativeauthenticator/tests/test_authenticator.py b/nativeauthenticator/tests/test_authenticator.py
index 67aeb39..c5f55ce 100644
--- a/nativeauthenticator/tests/test_authenticator.py
+++ b/nativeauthenticator/tests/test_authenticator.py
@@ -45,6 +45,13 @@ async def test_create_user(is_admin, open_signup, expected_authorization,
     assert user.is_authorized == expected_authorization
 
 
+async def test_create_user_bas_characters(tmpcwd, app):
+    '''Test method get_or_create_user with bad characters on username'''
+    auth = NativeAuthenticator(db=app.db)
+    assert not auth.get_or_create_user('john snow', 'password')
+    assert not auth.get_or_create_user('john,snow', 'password')
+
+
 @pytest.mark.parametrize("password,min_len,expected", [
     ("qwerty", 1, False),
     ("agameofthrones", 1, True),
@@ -57,24 +64,24 @@ async def test_create_user_with_strong_passwords(password, min_len, expected,
     auth = NativeAuthenticator(db=app.db)
     auth.check_common_password = True
     auth.minimum_password_length = min_len
-    user = auth.get_or_create_user('John Snow', password)
+    user = auth.get_or_create_user('johnsnow', password)
     assert bool(user) == expected
 
 
 @pytest.mark.parametrize("username,password,authorized,expected", [
     ("name", '123', False, False),
-    ("John Snow", '123', True, False),
+    ("johnsnow", '123', True, False),
     ("Snow", 'password', True, False),
-    ("John Snow", 'password', False, False),
-    ("John Snow", 'password', True, True),
+    ("johnsnow", 'password', False, False),
+    ("johnsnow", 'password', True, True),
 ])
 async def test_authentication(username, password, authorized, expected,
                               tmpcwd, app):
     '''Test if authentication fails with a unexistent user'''
     auth = NativeAuthenticator(db=app.db)
-    auth.get_or_create_user('John Snow', 'password')
+    auth.get_or_create_user('johnsnow', 'password')
     if authorized:
-        UserInfo.change_authorization(app.db, 'John Snow')
+        UserInfo.change_authorization(app.db, 'johnsnow')
     response = await auth.authenticate(app, {'username': username,
                                              'password': password})
     assert bool(response) == expected
@@ -123,9 +130,9 @@ async def test_authentication_with_exceed_atempts_of_login(tmpcwd, app):
     auth.allowed_failed_logins = 3
     auth.secs_before_next_try = 10
 
-    infos = {'username': 'John Snow', 'password': 'wrongpassword'}
+    infos = {'username': 'johnsnow', 'password': 'wrongpassword'}
     auth.get_or_create_user(infos['username'], 'password')
-    UserInfo.change_authorization(app.db, 'John Snow')
+    UserInfo.change_authorization(app.db, 'johnsnow')
 
     for i in range(3):
         response = await auth.authenticate(app, infos)