From 9bf4a64826ab9627b899a7b443a4fe900e6d45b7 Mon Sep 17 00:00:00 2001 From: yuvipanda Date: Sun, 15 Jul 2018 13:40:17 -0700 Subject: [PATCH 1/6] Dynamically set authenticator properties from YAML We don't wanna explicitly map keys from the YAML to traitlet config for the authentication - that's a lot of busywork for no gain. We instead switch to using snake_case everywhere, and dynamically set traitlet config from YAML config! --- tljh/configurer.py | 50 ++++++++++++++++++++++----------------- tljh/jupyterhub_config.py | 10 +++++++- 2 files changed, 37 insertions(+), 23 deletions(-) diff --git a/tljh/configurer.py b/tljh/configurer.py index c2b8b80..7a34b41 100644 --- a/tljh/configurer.py +++ b/tljh/configurer.py @@ -7,10 +7,6 @@ be called many times per lifetime of a jupyterhub. Traitlets that modify the startup of JupyterHub should not be here. FIXME: A strong feeling that JSON Schema should be involved somehow. """ -import copy -import os -import yaml - # Default configuration for tljh # User provided config is merged into this default = { @@ -18,7 +14,7 @@ default = { 'type': 'firstuse', 'dummy': {}, 'firstuse': { - 'createUsers': False + 'create_users': False } }, 'users': { @@ -30,20 +26,18 @@ default = { 'memory': '1G', 'cpu': None }, - 'userEnvironment': { - 'defaultApp': 'classic' + 'user_environment': { + 'default_app': 'classic' } } -def apply_yaml_config(path, c): - if os.path.exists(path): - with open(path) as f: - # FIXME: Figure out correct order of merging here - tljh_config = _merge_dictionaries(dict(default), yaml.safe_load(f)) - else: - tljh_config = copy.deepcopy(default) +def apply_config(config_overrides, c): + """ + Merge config_overrides with config defaults & apply to JupyterHub config c + """ + tljh_config = _merge_dictionaries(dict(default), config_overrides) update_auth(c, tljh_config) update_userlists(c, tljh_config) @@ -52,21 +46,33 @@ def apply_yaml_config(path, c): update_user_account_config(c, tljh_config) +def set_if_not_none(parent, key, value): + """ + Set attribute 'key' on parent if value is not None + """ + if value is not None: + setattr(parent, key, value) + + def update_auth(c, config): """ Set auth related configuration from YAML config file + + Use auth.type to determine authenticator to use. All parameters + in the config under auth.{auth.type} will be passed straight to the + authenticators themselves. """ auth = config.get('auth') if auth['type'] == 'dummy': c.JupyterHub.authenticator_class = 'dummyauthenticator.DummyAuthenticator' - password = auth['dummy'].get('password') - if password is not None: - c.DummyAuthenticator.password = password - return + authenticator_parent = c.DummyAuthenticator elif auth['type'] == 'firstuse': c.JupyterHub.authenticator_class = 'firstuseauthenticator.FirstUseAuthenticator' - c.FirstUseAuthenticator.create_users = auth['firstuse']['createUsers'] + authenticator_parent = c.FirstUseAuthenticator + + for k, v in auth[auth['type']].items(): + set_if_not_none(authenticator_parent, k, v) def update_userlists(c, config): @@ -94,12 +100,12 @@ def update_user_environment(c, config): """ Set user environment configuration """ - user_env = config['userEnvironment'] + user_env = config['user_environment'] # Set default application users are launched into - if user_env['defaultApp'] == 'jupyterlab': + if user_env['default_app'] == 'jupyterlab': c.Spawner.default_url = '/lab' - elif user_env['defaultApp'] == 'nteract': + elif user_env['default_app'] == 'nteract': c.Spawner.default_url = '/nteract' diff --git a/tljh/jupyterhub_config.py b/tljh/jupyterhub_config.py index 515993d..f556a0b 100644 --- a/tljh/jupyterhub_config.py +++ b/tljh/jupyterhub_config.py @@ -4,6 +4,8 @@ JupyterHub config for the littlest jupyterhub. import os from systemdspawner import SystemdSpawner from tljh import user, configurer +import yaml +import copy INSTALL_PREFIX = os.environ.get('TLJH_INSTALL_PREFIX') USER_ENV_PREFIX = os.path.join(INSTALL_PREFIX, 'user') @@ -40,4 +42,10 @@ c.SystemdSpawner.default_shell = '/bin/bash' # Drop the '-singleuser' suffix present in the default template c.SystemdSpawner.unit_name_template = 'jupyter-{USERNAME}' -configurer.apply_yaml_config(os.path.join(INSTALL_PREFIX, 'config.yaml'), c) +config_overrides_path = os.path.join(INSTALL_PREFIX, 'config.yaml') +if os.path.exists(config_overrides_path): + with open(config_overrides_path) as f: + config_overrides = yaml.safe_load(f) +else: + config_overrides = {} +configurer.apply_config(config_overrides, c) From 395b6543da42509ce52f5fb0dca3045dc54a97c1 Mon Sep 17 00:00:00 2001 From: yuvipanda Date: Sun, 15 Jul 2018 21:52:25 -0700 Subject: [PATCH 2/6] Add tests for configurer.py --- tests/test_configurer.py | 127 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 tests/test_configurer.py diff --git a/tests/test_configurer.py b/tests/test_configurer.py new file mode 100644 index 0000000..cb4f2b7 --- /dev/null +++ b/tests/test_configurer.py @@ -0,0 +1,127 @@ +""" +Test +""" + +from tljh import configurer + + +class MockConfigurer: + """ + Mock a Traitlet Configurable object. + + Equivalent to the `c` in `c.JupyterHub.some_property` method of setting + traitlet properties. If an accessed attribute doesn't exist, a new instance + of EmtpyObject is returned. This lets us set arbitrary attributes two + levels deep. + + >>> c = MockConfigurer() + >>> c.FirstLevel.second_level = 'hi' + >>> c.FirstLevel.second_level == 'hi' + True + >>> hasattr(c.FirstLevel, 'does_not_exist') + False + """ + + class _EmptyObject: + """ + Empty class for putting attributes in. + """ + pass + + def __getattr__(self, k): + if k not in self.__dict__: + self.__dict__[k] = MockConfigurer._EmptyObject() + return self.__dict__[k] + + +def test_mock_configurer(): + """ + Test the MockConfigurer's mocking ability + """ + m = MockConfigurer() + m.SomethingSomething = 'hi' + m.FirstLevel.second_level = 'boo' + + assert m.SomethingSomething == 'hi' + assert m.FirstLevel.second_level == 'boo' + + assert not hasattr(m.FirstLevel, 'non_existent') + + +def apply_mock_config(overrides): + """ + Configure a mock configurer with given overrides. + + overrides should be a dict that matches what you parse from a config.yaml + """ + c = MockConfigurer() + configurer.apply_config(overrides, c) + return c + + +def test_app_default(): + """ + Test default application with no config overrides. + """ + c = apply_mock_config({}) + # default_url is not set, so JupyterHub will pick default. + assert not hasattr(c.Spawner, 'default_url') + + +def test_app_jupyterlab(): + """ + Test setting JupyterLab as default application + """ + c = apply_mock_config({'user_environment': {'default_app': 'jupyterlab'}}) + assert c.Spawner.default_url == '/lab' + + +def test_app_nteract(): + """ + Test setting nteract as default application + """ + c = apply_mock_config({'user_environment': {'default_app': 'nteract'}}) + assert c.Spawner.default_url == '/nteract' + + +def test_auth_default(): + """ + Test default authentication settings with no overrides + """ + c = apply_mock_config({}) + + assert c.JupyterHub.authenticator_class == 'firstuseauthenticator.FirstUseAuthenticator' + # Do not auto create users who haven't been manually added by default + assert not c.FirstUseAuthenticator.create_users + + +def test_auth_dummy(): + """ + Test setting Dummy Authenticator & password + """ + c = apply_mock_config({ + 'auth': { + 'type': 'dummy', + 'dummy': { + 'password': 'test' + } + } + }) + assert c.JupyterHub.authenticator_class == 'dummyauthenticator.DummyAuthenticator' + assert c.DummyAuthenticator.password == 'test' + + +def test_auth_firstuse(): + """ + Test setting FirstUse Authenticator options + """ + c = apply_mock_config({ + 'auth': { + 'type': 'firstuse', + 'firstuse': { + 'create_users': True + } + } + }) + assert c.JupyterHub.authenticator_class == 'firstuseauthenticator.FirstUseAuthenticator' + assert c.FirstUseAuthenticator.create_users From 66de7bb03878fcb9866e1d9f8239308512f0f59a Mon Sep 17 00:00:00 2001 From: yuvipanda Date: Mon, 16 Jul 2018 01:19:24 -0700 Subject: [PATCH 3/6] Make authenticator class assigning code more generic --- tljh/configurer.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/tljh/configurer.py b/tljh/configurer.py index 7a34b41..0626132 100644 --- a/tljh/configurer.py +++ b/tljh/configurer.py @@ -64,12 +64,17 @@ def update_auth(c, config): """ auth = config.get('auth') - if auth['type'] == 'dummy': - c.JupyterHub.authenticator_class = 'dummyauthenticator.DummyAuthenticator' - authenticator_parent = c.DummyAuthenticator - elif auth['type'] == 'firstuse': - c.JupyterHub.authenticator_class = 'firstuseauthenticator.FirstUseAuthenticator' - authenticator_parent = c.FirstUseAuthenticator + # Map value of 'auth.type' in config to a fully qualified name + authenticator_class_map = { + 'dummy': 'dummyauthenticator.DummyAuthenticator', + 'firstuse': 'firstuseauthenticator.FirstUseAuthenticator' + } + + authenticator_classname = authenticator_class_map[auth['type']] + c.JupyterHub.authenticator_class = authenticator_classname + # Use just class name when setting config. If authenticator is dummyauthenticator.DummyAuthenticator, + # its config will be set under c.DummyAuthenticator + authenticator_parent = getattr(c, authenticator_classname.split('.')[-1]) for k, v in auth[auth['type']].items(): set_if_not_none(authenticator_parent, k, v) From 2c7f99b57c9e75a007ab8c5b6e8cb4ff2bc62325 Mon Sep 17 00:00:00 2001 From: yuvipanda Date: Mon, 16 Jul 2018 12:03:45 -0700 Subject: [PATCH 4/6] Support arbitrary authenticators - Removes all need for special casing authenticators. - Install them in hub environment, directly start using them. - Consider if we should special case any *at all* --- tests/test_configurer.py | 18 ++++++++++++++++++ tljh/configurer.py | 17 +++++++++++++---- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/tests/test_configurer.py b/tests/test_configurer.py index cb4f2b7..5f7ad2b 100644 --- a/tests/test_configurer.py +++ b/tests/test_configurer.py @@ -125,3 +125,21 @@ def test_auth_firstuse(): }) assert c.JupyterHub.authenticator_class == 'firstuseauthenticator.FirstUseAuthenticator' assert c.FirstUseAuthenticator.create_users + + +def test_auth_github(): + """ + Test using GitHub authenticator, which is not explicitly special cased. + """ + c = apply_mock_config({ + 'auth': { + 'type': 'oauthenticator.github.GitHubOAuthenticator', + 'GitHubOAuthenticator': { + 'client_id': 'something', + 'client_secret': 'something-else' + } + } + }) + assert c.JupyterHub.authenticator_class == 'oauthenticator.github.GitHubOAuthenticator' + assert c.GitHubOAuthenticator.client_id == 'something' + assert c.GitHubOAuthenticator.client_secret == 'something-else' diff --git a/tljh/configurer.py b/tljh/configurer.py index 0626132..c4bd2f2 100644 --- a/tljh/configurer.py +++ b/tljh/configurer.py @@ -70,13 +70,22 @@ def update_auth(c, config): 'firstuse': 'firstuseauthenticator.FirstUseAuthenticator' } - authenticator_classname = authenticator_class_map[auth['type']] - c.JupyterHub.authenticator_class = authenticator_classname + if auth['type'] in authenticator_class_map: + authenticator_class = authenticator_class_map[auth['type']] + authenticator_configname = auth['type'] + else: + # FIXME: Make sure this is something importable. + # FIXME: SECURITY: Class must inherit from Authenticator, to prevent us being + # used to set arbitrary properties on arbitrary types of objects! + authenticator_class = auth['type'] + # When specifying fully qualified name, use classname as key for config + authenticator_configname = authenticator_class.split('.')[-1] + c.JupyterHub.authenticator_class = authenticator_class # Use just class name when setting config. If authenticator is dummyauthenticator.DummyAuthenticator, # its config will be set under c.DummyAuthenticator - authenticator_parent = getattr(c, authenticator_classname.split('.')[-1]) + authenticator_parent = getattr(c, authenticator_class.split('.')[-1]) - for k, v in auth[auth['type']].items(): + for k, v in auth.get(authenticator_configname, {}).items(): set_if_not_none(authenticator_parent, k, v) From cc5622995dfd281bb6474b7d471fdb891afa2b11 Mon Sep 17 00:00:00 2001 From: yuvipanda Date: Mon, 16 Jul 2018 16:05:44 -0700 Subject: [PATCH 5/6] Do not special case *any* authenticator No two ways to set authenticator options - just one way. It's slightly 'ugly' because of the mixing of camel case & snake case, but is worth the massive reductions in complexity! --- tests/test_configurer.py | 10 +++++----- tljh/configurer.py | 27 ++++++++------------------- 2 files changed, 13 insertions(+), 24 deletions(-) diff --git a/tests/test_configurer.py b/tests/test_configurer.py index 5f7ad2b..df4b69f 100644 --- a/tests/test_configurer.py +++ b/tests/test_configurer.py @@ -101,8 +101,8 @@ def test_auth_dummy(): """ c = apply_mock_config({ 'auth': { - 'type': 'dummy', - 'dummy': { + 'type': 'dummyauthenticator.DummyAuthenticator', + 'DummyAuthenticator': { 'password': 'test' } } @@ -117,8 +117,8 @@ def test_auth_firstuse(): """ c = apply_mock_config({ 'auth': { - 'type': 'firstuse', - 'firstuse': { + 'type': 'firstuseauthenticator.FirstUseAuthenticator', + 'FirstUseAuthenticator': { 'create_users': True } } @@ -129,7 +129,7 @@ def test_auth_firstuse(): def test_auth_github(): """ - Test using GitHub authenticator, which is not explicitly special cased. + Test using GitHub authenticator """ c = apply_mock_config({ 'auth': { diff --git a/tljh/configurer.py b/tljh/configurer.py index c4bd2f2..4863204 100644 --- a/tljh/configurer.py +++ b/tljh/configurer.py @@ -11,9 +11,8 @@ FIXME: A strong feeling that JSON Schema should be involved somehow. # User provided config is merged into this default = { 'auth': { - 'type': 'firstuse', - 'dummy': {}, - 'firstuse': { + 'type': 'firstuseauthenticator.FirstUseAuthenticator', + 'FirstUseAuthenticator': { 'create_users': False } }, @@ -64,22 +63,12 @@ def update_auth(c, config): """ auth = config.get('auth') - # Map value of 'auth.type' in config to a fully qualified name - authenticator_class_map = { - 'dummy': 'dummyauthenticator.DummyAuthenticator', - 'firstuse': 'firstuseauthenticator.FirstUseAuthenticator' - } - - if auth['type'] in authenticator_class_map: - authenticator_class = authenticator_class_map[auth['type']] - authenticator_configname = auth['type'] - else: - # FIXME: Make sure this is something importable. - # FIXME: SECURITY: Class must inherit from Authenticator, to prevent us being - # used to set arbitrary properties on arbitrary types of objects! - authenticator_class = auth['type'] - # When specifying fully qualified name, use classname as key for config - authenticator_configname = authenticator_class.split('.')[-1] + # FIXME: Make sure this is something importable. + # FIXME: SECURITY: Class must inherit from Authenticator, to prevent us being + # used to set arbitrary properties on arbitrary types of objects! + authenticator_class = auth['type'] + # When specifying fully qualified name, use classname as key for config + authenticator_configname = authenticator_class.split('.')[-1] c.JupyterHub.authenticator_class = authenticator_class # Use just class name when setting config. If authenticator is dummyauthenticator.DummyAuthenticator, # its config will be set under c.DummyAuthenticator From 0385f8e5f742b8d79d82b8678a25cc25280542b7 Mon Sep 17 00:00:00 2001 From: yuvipanda Date: Mon, 16 Jul 2018 16:19:16 -0700 Subject: [PATCH 6/6] Add LDAP & OAuthenticator to initial install --- tljh/installer.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tljh/installer.py b/tljh/installer.py index 42c3b56..39e7fb7 100644 --- a/tljh/installer.py +++ b/tljh/installer.py @@ -73,7 +73,9 @@ def ensure_jupyterhub_package(prefix): 'jupyterhub==0.9.0', 'jupyterhub-dummyauthenticator==0.3.1', 'jupyterhub-systemdspawner==0.11', - 'jupyterhub-firstuseauthenticator==0.10' + 'jupyterhub-firstuseauthenticator==0.10', + 'jupyterhub-ldapauthenticator==1.2.2', + 'oauthenticator==0.7.3', ])