diff --git a/jupyterhub/app.py b/jupyterhub/app.py index cdc8e13f..c2abbab5 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -813,13 +813,22 @@ class JupyterHub(Application): orm_token = orm.APIToken.find(db, token) if orm_token is None: user = orm.User.find(db, username) + user_created = False if user is None: + user_created = True self.log.debug("Adding user %r to database", username) user = orm.User(name=username) db.add(user) db.commit() self.log.info("Adding API token for %s", username) - user.new_api_token(token) + try: + user.new_api_token(token) + except Exception: + if user_created: + # don't allow bad tokens to create users + db.delete(user) + db.commit() + raise else: self.log.debug("Not duplicating token %s", orm_token) db.commit() diff --git a/jupyterhub/orm.py b/jupyterhub/orm.py index 1e7b7390..2f780459 100644 --- a/jupyterhub/orm.py +++ b/jupyterhub/orm.py @@ -313,6 +313,8 @@ class User(Base): if token is None: token = new_token() else: + if len(token) < 8: + raise ValueError("Tokens must be at least 8 characters, got %r" % token) found = APIToken.find(db, token) if found: raise ValueError("Collision on token: %s..." % token[:4]) diff --git a/jupyterhub/tests/test_app.py b/jupyterhub/tests/test_app.py index 832b91ac..e0e9dcd1 100644 --- a/jupyterhub/tests/test_app.py +++ b/jupyterhub/tests/test_app.py @@ -5,6 +5,9 @@ import re import sys from subprocess import check_output, Popen, PIPE from tempfile import NamedTemporaryFile, TemporaryDirectory + +import pytest + from .mocking import MockHub from .. import orm @@ -50,6 +53,7 @@ def test_generate_config(): assert 'Spawner.cmd' in cfg_text assert 'Authenticator.whitelist' in cfg_text + def test_init_tokens(): with TemporaryDirectory() as td: db_file = os.path.join(td, 'jupyterhub.sqlite') @@ -76,3 +80,10 @@ def test_init_tokens(): assert api_token is not None user = api_token.user assert user.name == username + + # don't allow failed token insertion to create users: + tokens['short'] = 'gman' + app = MockHub(db_file=db_file, api_tokens=tokens) + # with pytest.raises(ValueError): + app.initialize([]) + assert orm.User.find(app.db, 'gman') is None