Merge pull request #721 from consideRatio/pr/make-auth-config-more-general

Apply TLJH auth config with less assumptions

tests/test_configurer.py

@@ -11,7 +11,7 @@ from tljh import configurer
 
 class MockConfigurer:
     """
-    Mock a Traitlet Configurable object.
+    Mock a Traitlets Config class 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
@@ -24,6 +24,9 @@ class MockConfigurer:
         True
         >>> hasattr(c.FirstLevel, 'does_not_exist')
         False
+
+    The actual Config class implementation can be found at
+    https://github.com/ipython/traitlets/blob/34f596dd03b98434900a7d31c912fc168342bb80/traitlets/config/loader.py#L220
     """
 
     class _EmptyObject:
@@ -37,6 +40,13 @@ class MockConfigurer:
             self.__dict__[k] = MockConfigurer._EmptyObject()
         return self.__dict__[k]
 
+    def __getitem__(self, key):
+        """
+        To mimic the traitlets Config class instance we often access as "c", we
+        need to provide a subscript functionality that can be used as
+        c["Something"]. To do this, we provide a __getitem__ function.
+        """
+        return self.__getattr__(key)
 
 def test_mock_configurer():
     """
@@ -48,6 +58,7 @@ def test_mock_configurer():
 
     assert m.SomethingSomething == 'hi'
     assert m.FirstLevel.second_level == 'boo'
+    assert m["FirstLevel"].second_level == 'boo'
 
     assert not hasattr(m.FirstLevel, 'non_existent')
 

tljh/configurer.py

@@ -149,27 +149,55 @@ def update_base_url(c, config):
 
 def update_auth(c, config):
     """
-    Set auth related configuration from YAML config file
+    Set auth related configuration from YAML config file.
 
-    Use auth.type to determine authenticator to use. All parameters
+    As an example, this function should update the following TLJH auth
-    in the config under auth.{auth.type} will be passed straight to the
+    configuration:
-    authenticators themselves.
+
+    ```yaml
+    auth:
+      type: oauthenticator.github.GitHubOAuthenticator
+      GitHubOAuthenticator:
+        client_id: "..."
+        client_secret: "..."
+        oauth_callback_url: "..."
+      ClassName:
+        arbitrary_key: "..."
+        arbitrary_key_with_none_value:
+    ```
+
+    by applying the following configuration:
+
+    ```python
+    c.JupyterHub.authenticator_class = "oauthenticator.github.GitHubOAuthenticator"
+    c.GitHubOAuthenticator.client_id = "..."
+    c.GitHubOAuthenticator.client_secret = "..."
+    c.GitHubOAuthenticator.oauth_callback_url = "..."
+    c.ArbitraryKey.arbitrary_key = "..."
+    ```
+
+    Note that "auth.type" and "auth.ArbitraryKey.arbitrary_key_with_none_value"
+    are treated a bit differently. auth.type will always map to
+    c.JupyterHub.authenticator_class and any configured value being None won't
+    be set.
     """
-    auth = config.get('auth')
+    tljh_auth_config = config['auth']
 
     # FIXME: Make sure this is something importable.
-    # FIXME: SECURITY: Class must inherit from Authenticator, to prevent us being
+    # FIXME: SECURITY: Class must inherit from Authenticator, to prevent us
-    # used to set arbitrary properties on arbitrary types of objects!
+    #        being used to set arbitrary properties on arbitrary types of objects!
-    authenticator_class = auth['type']
+    c.JupyterHub.authenticator_class = tljh_auth_config['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_class.split('.')[-1])
 
-    for k, v in auth.get(authenticator_configname, {}).items():
+    for auth_key, auth_value in tljh_auth_config.items():
-        set_if_not_none(authenticator_parent, k, v)
+        if not (auth_key[0] == auth_key[0].upper() and isinstance(auth_value, dict)):
+            if auth_key == 'type':
+                continue
+            raise ValueError(f"Error: auth.{auth_key} was ignored, it didn't look like a valid configuration")
+        class_name = auth_key
+        class_config_to_set = auth_value
+        class_config = c[class_name]
+        for config_name, config_value in class_config_to_set.items():
+            set_if_not_none(class_config, config_name, config_value)
 
 
 def update_userlists(c, config):