From 70785f9fd3155fa8ff356596cd4114d1d3d7c543 Mon Sep 17 00:00:00 2001
From: yuvipanda <yuvipanda@gmail.com>
Date: Fri, 27 Jul 2018 21:08:26 -0700
Subject: [PATCH 1/8] Add tljh-config command

We do not want users to hand-edit YAML files. This has been a
major source of bugs and confusion for users in z2jh. Doing so
in a terminal text editor makes it even worse.

This lets users type commands directly to modify config.yaml file
rather than edit files directly. This makes it a lot less error
prone and user friendly.

Advanced users can still edit config.yaml manually.

Fixes #38
---
 setup.py             |   7 +-
 tests/test_config.py |  78 ++++++++++++++++++++++
 tljh/config.py       | 150 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 234 insertions(+), 1 deletion(-)
 create mode 100644 tests/test_config.py
 create mode 100644 tljh/config.py

diff --git a/setup.py b/setup.py
index 269e37b..789d734 100644
--- a/setup.py
+++ b/setup.py
@@ -13,5 +13,10 @@ setup(
     install_requires=[
         'pyyaml==3.*',
         'ruamel.yaml==0.15.*'
-    ]
+    ],
+    entry_points={
+        'console_scripts': [
+            'tljh-config = tljh.config:main'
+        ]
+    }
 )
diff --git a/tests/test_config.py b/tests/test_config.py
new file mode 100644
index 0000000..4d8a591
--- /dev/null
+++ b/tests/test_config.py
@@ -0,0 +1,78 @@
+"""
+Test configuration commandline tools
+"""
+from tljh import config
+from contextlib import redirect_stdout
+import io
+import tempfile
+
+
+def test_set_no_mutate():
+    conf = {}
+
+    new_conf = config.set_item_in_config(conf, 'a.b', 'c')
+    assert new_conf['a']['b'] == 'c'
+    assert conf == {}
+
+def test_set_one_level():
+    conf = {}
+
+    new_conf = config.set_item_in_config(conf, 'a', 'b')
+    assert new_conf['a'] == 'b'
+
+def test_set_multi_level():
+    conf = {}
+
+    new_conf = config.set_item_in_config(conf, 'a.b', 'c')
+    new_conf = config.set_item_in_config(new_conf, 'a.d', 'e')
+    new_conf = config.set_item_in_config(new_conf, 'f', 'g')
+    assert new_conf == {
+        'a': {'b': 'c', 'd': 'e'},
+        'f': 'g'
+    }
+
+def test_set_overwrite():
+    """
+    We can overwrite already existing config items.
+
+    This might be surprising destructive behavior to some :D
+    """
+    conf = {
+        'a': 'b'
+    }
+
+    new_conf = config.set_item_in_config(conf, 'a', 'c')
+    assert new_conf == {'a': 'c'}
+
+    new_conf = config.set_item_in_config(new_conf, 'a.b', 'd')
+    assert new_conf == {'a': {'b': 'd'}}
+
+    new_conf = config.set_item_in_config(new_conf, 'a', 'hi')
+    assert new_conf == {'a': 'hi'}
+
+
+def test_show_config():
+    """
+    Test stdout output when showing config
+    """
+    conf = """
+# Just some test YAML
+a:
+  b:
+  - h
+  - 1
+    """.strip()
+
+
+    with tempfile.NamedTemporaryFile() as tmp:
+        tmp.write(conf.encode())
+        tmp.flush()
+
+        out = io.StringIO()
+        with redirect_stdout(out):
+            config.show_config(tmp.name)
+
+    assert out.getvalue().strip() == conf
+
+
+
diff --git a/tljh/config.py b/tljh/config.py
new file mode 100644
index 0000000..f2476aa
--- /dev/null
+++ b/tljh/config.py
@@ -0,0 +1,150 @@
+"""
+Commandline interface for setting config items in config.yaml.
+
+Used as:
+
+tljh-config set firstlevel.second_level something
+
+tljh-config show
+
+tljh-config show firstlevel
+
+tljh-config show firstlevel.second_level
+"""
+import sys
+import argparse
+from ruamel.yaml import YAML
+from copy import deepcopy
+from tljh import systemd
+
+
+yaml = YAML(typ='rt')
+
+
+def set_item_in_config(config, property_path, value):
+    """
+    Set key at property_path to value in config & return new config.
+
+    config is not mutated.
+
+    propert_path is a series of dot separated values. Any part of the path
+    that does not exist is created. 
+    """
+    path_components = property_path.split('.')
+
+    # Mutate a copy of the config, not config itself
+    cur_part = config_copy = deepcopy(config)
+    for i in range(len(path_components)):
+        cur_path = path_components[i]
+        if i == len(path_components) - 1:
+            # Final component
+            cur_part[cur_path] = value
+        else:
+            # If we are asked to create new non-leaf nodes, we will always make them dicts
+            # This means setting is *destructive* - will replace whatever is down there!
+            if cur_path not in cur_part or not isinstance(cur_part[cur_path], dict):
+                cur_part[cur_path] = {}
+            cur_part = cur_part[cur_path]
+
+    return config_copy
+
+
+def show_config(config_path):
+    """
+    Pretty print config from given config_path
+    """
+    try:
+        with open(config_path) as f:
+            config = yaml.load(f)
+    except FileNotFoundError:
+        config = {}
+    
+    yaml.dump(config, sys.stdout)
+
+
+def set_config_value(config_path, key_path, value):
+    """
+    Set key at key_path in config_path to value
+    """
+    # FIXME: Have a file lock here
+    # FIXME: Validate schema here
+    try:
+        with open(config_path) as f:
+            config = yaml.load(f)
+    except FileNotFoundError:
+        config = {}
+
+    config = set_item_in_config(config, key_path, value)
+
+    with open(config_path, 'w') as f:
+        yaml.dump(config, f)
+
+
+
+def reload_component(component):
+    """
+    Reload a TLJH component.
+
+    component can be 'hub' or 'proxy'.
+    """
+    if component == 'hub':
+        systemd.restart_service('jupyterhub')
+        # FIXME: Verify hub is back up?
+        print('Hub reload with new configuration complete')
+    elif component == 'proxy':
+        systemd.restart_service('configurable-http-proxy')
+        print('Proxy reload with new configuration complete')
+
+
+def main():
+    argparser = argparse.ArgumentParser()
+    argparser.add_argument(
+        '--config-path',
+        default='/opt/tljh/config.yaml',
+        help='Path to TLJH config.yaml file'
+    )
+    subparsers = argparser.add_subparsers(dest='action')
+
+    show_parser = subparsers.add_parser(
+        'show',
+        help='Show current configuration'
+    )
+
+    set_parser = subparsers.add_parser(
+        'set',
+        help='Set a configuration property'
+    )
+    set_parser.add_argument(
+        'key_path',
+        help='Dot separated path to configuration key to set'
+    )
+    set_parser.add_argument(
+        'value',
+        help='Value ot set the configuration key to'
+    )
+
+    reload_parser = subparsers.add_parser(
+        'reload',
+        help='Reload a component to apply configuration change'
+    )
+    reload_parser.add_argument(
+        'component',
+        choices=('hub', 'proxy'),
+        help='Which component to reload',
+        default='hub',
+        nargs='?'
+    )
+
+    args = argparser.parse_args()
+
+    if args.action == 'show':
+        show_config(args.config_path)
+    elif args.action == 'set':
+        set_config_value(args.config_path, args.key_path, args.value)
+    elif args.action == 'reload':
+        reload_config(args.component)
+    
+if __name__ == '__main__':
+    main()
+
+

From 95a0359d32d8eccd6e03c7ecd44fd1ea661e7e27 Mon Sep 17 00:00:00 2001
From: yuvipanda <yuvipanda@gmail.com>
Date: Fri, 27 Jul 2018 22:22:45 -0700
Subject: [PATCH 2/8] Add howto doc on dummy authenticator

---
 docs/howto/auth/dummy.rst | 43 +++++++++++++++++++++++++++++++++++++++
 docs/index.rst            |  8 ++++++++
 2 files changed, 51 insertions(+)
 create mode 100644 docs/howto/auth/dummy.rst

diff --git a/docs/howto/auth/dummy.rst b/docs/howto/auth/dummy.rst
new file mode 100644
index 0000000..dc62893
--- /dev/null
+++ b/docs/howto/auth/dummy.rst
@@ -0,0 +1,43 @@
+.. _howto/auth/dummy:
+
+=====================================================
+Authenticate *any* user with a single shared password
+=====================================================
+
+The **Dummy Authenticator** lets *any* user log in with the given password.
+This authenticator is **extremely insecure**, so do not use it if you can
+avoid it.
+
+Enabling the authenticator
+==========================
+
+1. Always use DummyAuthenticator with a password. You can communicate this 
+   password to all your users via an out of band mechanism (like writing on
+   a whiteboard). Once you have selected a password, configure TLJH to use
+   the password by executing the following from an admin console.
+
+.. code-block:: bash
+   
+   tljh-config set auth.DummyAuthenticator.password <password>
+
+   Remember to replace ``<password>`` with the password you choose.
+
+2. Enable the authenticator and reload config to apply configuration:
+
+   tljh-config set auth.type dummyauthenticator.DummyAuthenticator
+   tljh-config reload
+
+Users who are currently logged in will continue to be logged in. When they 
+log out and try to log back in, they will be asked to provide a username and
+password.
+
+Changing the password
+=====================
+
+The password used by DummyAuthenticator can be changed with the following
+commands:
+
+.. code-block:: bash
+
+   tljh-config set auth.DummyAuthenticator.password <new-password>
+   tljh-config reload
\ No newline at end of file
diff --git a/docs/index.rst b/docs/index.rst
index b3ad167..6fdf76c 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -57,6 +57,14 @@ How-To guides answer the question 'How do I...?' for a lot of topics.
    howto/notebook-interfaces
    howto/resource-estimation
 
+We have a special set of How-To Guides on using various forms of authentication
+with your JupyterHub.
+
+.. toctree::
+   :titlesonly:
+
+   howto/auth/dummy
+
 Topic Guides
 ============
 

From 0d0dbc828cb03198483566ef939e2206a99d64ee Mon Sep 17 00:00:00 2001
From: yuvipanda <yuvipanda@gmail.com>
Date: Fri, 27 Jul 2018 22:57:19 -0700
Subject: [PATCH 3/8] Fix typo in tljh-config

---
 tljh/config.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tljh/config.py b/tljh/config.py
index f2476aa..9f1c38f 100644
--- a/tljh/config.py
+++ b/tljh/config.py
@@ -142,7 +142,7 @@ def main():
     elif args.action == 'set':
         set_config_value(args.config_path, args.key_path, args.value)
     elif args.action == 'reload':
-        reload_config(args.component)
+        reload_component(args.component)
     
 if __name__ == '__main__':
     main()

From fad3e70116038f9ec30f6fa26ba0b1c502aa1483 Mon Sep 17 00:00:00 2001
From: yuvipanda <yuvipanda@gmail.com>
Date: Fri, 27 Jul 2018 22:57:59 -0700
Subject: [PATCH 4/8] Add topic guide on tljh-config

---
 docs/howto/auth/dummy.rst  |  6 +--
 docs/index.rst             |  1 +
 docs/topic/tljh-config.rst | 81 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 85 insertions(+), 3 deletions(-)
 create mode 100644 docs/topic/tljh-config.rst

diff --git a/docs/howto/auth/dummy.rst b/docs/howto/auth/dummy.rst
index dc62893..8810867 100644
--- a/docs/howto/auth/dummy.rst
+++ b/docs/howto/auth/dummy.rst
@@ -18,14 +18,14 @@ Enabling the authenticator
 
 .. code-block:: bash
    
-   tljh-config set auth.DummyAuthenticator.password <password>
+   sudo -E tljh-config set auth.DummyAuthenticator.password <password>
 
    Remember to replace ``<password>`` with the password you choose.
 
 2. Enable the authenticator and reload config to apply configuration:
 
-   tljh-config set auth.type dummyauthenticator.DummyAuthenticator
-   tljh-config reload
+   sudo -E tljh-config set auth.type dummyauthenticator.DummyAuthenticator
+   sudo -E tljh-config reload
 
 Users who are currently logged in will continue to be logged in. When they 
 log out and try to log back in, they will be asked to provide a username and
diff --git a/docs/index.rst b/docs/index.rst
index 6fdf76c..93c8529 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -77,6 +77,7 @@ Topic guides provide in-depth explanations of specific topics.
    guides/admin
    topic/security
    topic/customizing-installer
+   topic/tljh-config
 
 
 Troubleshooting
diff --git a/docs/topic/tljh-config.rst b/docs/topic/tljh-config.rst
new file mode 100644
index 0000000..3e22e5f
--- /dev/null
+++ b/docs/topic/tljh-config.rst
@@ -0,0 +1,81 @@
+.. _topic/tljh-config:
+
+=====================================
+Configuring TLJH with ``tljh-config``
+=====================================
+
+``tljh-config`` is the commandline program used to make configuration
+changes to TLJH. 
+
+Running ``tljh-config``
+======================`
+
+You can run ``tljh-config`` in two ways:
+
+#. From inside a terminal in JupyterHub while logged in as an admin user.
+   This method is **recommended**.
+
+#. By directly calling ``/opt/tljh/hub/bin/tljh-config`` as root when
+   logged in to the server via other means (such as SSH). This is an
+   advanced use case, and not covered much in this guide.
+
+Set a configuration property
+============================
+
+TLJH's configuration is organized in a nested tree structure. You can
+set a particular property with the following command:
+
+.. code-block:: bash
+
+   sudo -E tljh-config set <property-path> <value>
+
+
+where:
+
+#. ``<property-path>`` is a dot-separated path to the property you want
+   to set.
+#. ``<value>`` is the value you want to set the property to.
+
+For example, to set the password for the DummyAuthenticator, you
+need to set the ``auth.DummyAuthenticator.password`` property. You would
+do so with the following:
+
+.. code-block:: bash
+
+   sudo -E tljh-config set auth.DummyAuthenticator.password mypassword
+
+
+This can only set string and numerical properties, not lists.
+
+View current configuration
+==========================
+
+To see the current configuration, you can run the following command:
+
+.. code-block:: bash
+
+   sudo -E tljh-config show
+
+This will print the current configuration of your TLJH. This is very
+useful when asking for support!
+
+Reloading JupyterHub to apply configuration
+===========================================
+
+After modifying the configuration, you need to reload JupyterHub for
+it to take effect. You can do so with:
+
+.. code-block:: bash
+
+   sudo -E tljh-config reload
+
+This should not affect any running users. The JupyterHub will be
+restarted and loaded with the new configuration.
+
+Advanced: ``config.yaml``
+=========================
+
+``tljh-config`` is a simple program that modifies the contents of the 
+``config.yaml`` file located at ``/opt/tljh/config.yaml``. ``tljh-config`` 
+is the recommended method of editing / viewing configuration since editing
+YAML by hand in a terminal text editor is a large source of errors.
\ No newline at end of file

From 6f99da5d95f8d0d35b313657948eb6f8c4a34739 Mon Sep 17 00:00:00 2001
From: yuvipanda <yuvipanda@gmail.com>
Date: Sat, 28 Jul 2018 00:52:02 -0700
Subject: [PATCH 5/8] Add full integration test with hubtraf

- Fully simulates what a user would be doing
- Also tests tljh-config set and reload functionality
---
 .circleci/config.yml               |  7 +++++++
 integration-tests/requirements.txt |  3 ++-
 integration-tests/test_hub.py      | 27 +++++++++++++++++++++++++++
 3 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/.circleci/config.yml b/.circleci/config.yml
index 31e5b60..fd4b98a 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -77,6 +77,13 @@ jobs:
                     python3 .circleci/integration-test.py copy . /srv/src
                     python3 .circleci/integration-test.py run 'python3 /srv/src/bootstrap/bootstrap.py'
 
+
+            - run:
+                name: switch to dummyauthenticator
+                command: |
+                    python3 .circleci/integration-test.py run '/opt/tljh/hub/bin/tljh-config set auth.type dummyauthenticator.DummyAuthenticator'
+                    python3 .circleci/integration-test.py run '/opt/tljh/hub/bin/tljh-config reload'
+
             - run:
                 name: print systemd status + logs
                 command: |
diff --git a/integration-tests/requirements.txt b/integration-tests/requirements.txt
index 547de5c..271c563 100644
--- a/integration-tests/requirements.txt
+++ b/integration-tests/requirements.txt
@@ -1,2 +1,3 @@
 pytest
-requests
+pytest-asyncio
+git+https://github.com/yuvipanda/hubtraf.git
\ No newline at end of file
diff --git a/integration-tests/test_hub.py b/integration-tests/test_hub.py
index d5d6cb0..ddb1980 100644
--- a/integration-tests/test_hub.py
+++ b/integration-tests/test_hub.py
@@ -1,5 +1,32 @@
 import requests
+from hubtraf.user import User
+from hubtraf.auth.dummy import login_dummy
+import secrets
+import pytest
+from functools import partial
+import pwd
+
 
 def test_hub_up():
     r = requests.get('http://127.0.0.1')
     r.raise_for_status()
+
+
+@pytest.mark.asyncio
+async def test_user_code_execute():
+    """
+    User logs in, starts a server & executes code
+    """
+    # This *must* be localhost, not an IP
+    # aiohttp throws away cookies if we are connecting to an IP!
+    hub_url = 'http://localhost'
+    username = secrets.token_hex(8)
+
+    async with User(username, hub_url, partial(login_dummy, password='')) as u:
+            await u.login()
+            await u.ensure_server()
+            await u.start_kernel()
+            await u.assert_code_output("5 * 4", "20", 5, 5)
+
+            # Assert that the user exists
+            assert pwd.getpwnam(f'jupyter-{username}') is not None
\ No newline at end of file

From f0c944aeb8147596bd8171e053f9d466a8730fe0 Mon Sep 17 00:00:00 2001
From: yuvipanda <yuvipanda@gmail.com>
Date: Sat, 28 Jul 2018 11:05:29 -0700
Subject: [PATCH 6/8] Add code + tests for adding items to a list

---
 integration-tests/test_hub.py | 36 ++++++++++++++++++-
 tests/test_config.py          | 30 ++++++++++++++++
 tljh/config.py                | 67 ++++++++++++++++++++++++++++++++---
 3 files changed, 127 insertions(+), 6 deletions(-)

diff --git a/integration-tests/test_hub.py b/integration-tests/test_hub.py
index ddb1980..bb5897d 100644
--- a/integration-tests/test_hub.py
+++ b/integration-tests/test_hub.py
@@ -4,7 +4,10 @@ from hubtraf.auth.dummy import login_dummy
 import secrets
 import pytest
 from functools import partial
+import asyncio
 import pwd
+import grp
+import sys
 
 
 def test_hub_up():
@@ -29,4 +32,35 @@ async def test_user_code_execute():
             await u.assert_code_output("5 * 4", "20", 5, 5)
 
             # Assert that the user exists
-            assert pwd.getpwnam(f'jupyter-{username}') is not None
\ No newline at end of file
+            assert pwd.getpwnam(f'jupyter-{username}') is not None
+
+
+@pytest.mark.asyncio
+async def test_user_admin_code():
+    """
+    User logs in, starts a server & executes code
+    """
+    # This *must* be localhost, not an IP
+    # aiohttp throws away cookies if we are connecting to an IP!
+    hub_url = 'http://localhost'
+    username = secrets.token_hex(8)
+
+    tljh_config_path = [sys.executable, '-m', 'tljh.config']
+
+    assert 0 == await (await asyncio.create_subprocess_exec(*tljh_config_path, 'add-item', 'users.admin', username)).wait()
+    assert 0 == await (await asyncio.create_subprocess_exec(*tljh_config_path, 'reload')).wait()
+
+    # FIXME: wait for reload to finish & hub to come up
+    # Should be part of tljh-config reload
+    await asyncio.sleep(1)
+    async with User(username, hub_url, partial(login_dummy, password='')) as u:
+            await u.login()
+            await u.ensure_server()
+            await u.start_kernel()
+            await u.assert_code_output("5 * 4", "20", 5, 5)
+
+            # Assert that the user exists
+            assert pwd.getpwnam(f'jupyter-{username}') is not None
+
+            # Assert that the user has admin rights
+            assert f'jupyter-{username}' in grp.getgrnam('jupyterhub-admins').gr_mem
\ No newline at end of file
diff --git a/tests/test_config.py b/tests/test_config.py
index 4d8a591..c0abdce 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -51,6 +51,36 @@ def test_set_overwrite():
     assert new_conf == {'a': 'hi'}
 
 
+def test_add_to_config_one_level():
+    conf = {}
+
+    new_conf = config.add_item_to_config(conf, 'a.b', 'c')
+    assert new_conf == {
+        'a': {'b': ['c']}
+    }
+
+
+def test_add_to_config_zero_level():
+    conf = {}
+
+    new_conf = config.add_item_to_config(conf, 'a', 'b')
+    assert new_conf == {
+        'a': ['b']
+    }
+
+def test_add_to_config_multiple():
+    conf = {}
+
+    new_conf = config.add_item_to_config(conf, 'a.b.c', 'd')
+    assert new_conf == {
+        'a': {'b': {'c': ['d']}}
+    }
+
+    new_conf = config.add_item_to_config(new_conf, 'a.b.c', 'e')
+    assert new_conf == {
+        'a': {'b': {'c': ['d', 'e']}}
+    }
+
 def test_show_config():
     """
     Test stdout output when showing config
diff --git a/tljh/config.py b/tljh/config.py
index 9f1c38f..770896e 100644
--- a/tljh/config.py
+++ b/tljh/config.py
@@ -27,14 +27,14 @@ def set_item_in_config(config, property_path, value):
 
     config is not mutated.
 
-    propert_path is a series of dot separated values. Any part of the path
+    property_path is a series of dot separated values. Any part of the path
     that does not exist is created. 
     """
     path_components = property_path.split('.')
 
     # Mutate a copy of the config, not config itself
     cur_part = config_copy = deepcopy(config)
-    for i in range(len(path_components)):
+    for i, cur_path in enumerate(path_components):
         cur_path = path_components[i]
         if i == len(path_components) - 1:
             # Final component
@@ -49,6 +49,34 @@ def set_item_in_config(config, property_path, value):
     return config_copy
 
 
+def add_item_to_config(config, property_path, value):
+    """
+    Add an item to a list in config.
+    """
+    path_components = property_path.split('.')
+
+    # Mutate a copy of the config, not config itself
+    cur_part = config_copy = deepcopy(config)
+    for i, cur_path in enumerate(path_components):
+        if i == len(path_components) - 1:
+            # Final component, it must be a list and we append to it
+            print('l', cur_path, cur_part)
+            if cur_path not in cur_part or not isinstance(cur_part[cur_path], list):
+                cur_part[cur_path] = []
+            cur_part = cur_part[cur_path]
+
+            cur_part.append(value)
+        else:
+            # If we are asked to create new non-leaf nodes, we will always make them dicts
+            # This means setting is *destructive* - will replace whatever is down there!
+            print('p', cur_path, cur_part)
+            if cur_path not in cur_part or not isinstance(cur_part[cur_path], dict):
+                cur_part[cur_path] = {}
+            cur_part = cur_part[cur_path]
+
+    return config_copy
+
+
 def show_config(config_path):
     """
     Pretty print config from given config_path
@@ -80,6 +108,22 @@ def set_config_value(config_path, key_path, value):
         yaml.dump(config, f)
 
 
+def add_config_value(config_path, key_path, value):
+    """
+    Add value to list at key_path
+    """
+    # FIXME: Have a file lock here
+    # FIXME: Validate schema here
+    try:
+        with open(config_path) as f:
+            config = yaml.load(f)
+    except FileNotFoundError:
+        config = {}
+
+    config = add_item_to_config(config, key_path, value)
+
+    with open(config_path, 'w') as f:
+        yaml.dump(config, f)
 
 def reload_component(component):
     """
@@ -123,6 +167,19 @@ def main():
         help='Value ot set the configuration key to'
     )
 
+    add_item_parser = subparsers.add_parser(
+        'add-item',
+        help='Add a value to a list for a configuration property'
+    )
+    add_item_parser.add_argument(
+        'key_path',
+        help='Dot separated path to configuration key to set'
+    )
+    add_item_parser.add_argument(
+        'value',
+        help='Value ot set the configuration key to'
+    )
+
     reload_parser = subparsers.add_parser(
         'reload',
         help='Reload a component to apply configuration change'
@@ -141,10 +198,10 @@ def main():
         show_config(args.config_path)
     elif args.action == 'set':
         set_config_value(args.config_path, args.key_path, args.value)
+    elif args.action == 'add-item':
+        add_config_value(args.config_path, args.key_path, args.value)
     elif args.action == 'reload':
         reload_component(args.component)
     
 if __name__ == '__main__':
-    main()
-
-
+    main()
\ No newline at end of file

From 7e9e2d375cef81daa87b1c7191a4aab0611a6137 Mon Sep 17 00:00:00 2001
From: yuvipanda <yuvipanda@gmail.com>
Date: Sat, 28 Jul 2018 11:57:11 -0700
Subject: [PATCH 7/8] Add tljh-config remove-item to remove an item from a list

---
 integration-tests/test_hub.py | 48 ++++++++++++++++++++++++++++++++---
 tests/test_config.py          | 27 ++++++++++++++++++++
 tljh/config.py                | 43 ++++++++++++++++++++++++++++---
 3 files changed, 110 insertions(+), 8 deletions(-)

diff --git a/integration-tests/test_hub.py b/integration-tests/test_hub.py
index bb5897d..12efad9 100644
--- a/integration-tests/test_hub.py
+++ b/integration-tests/test_hub.py
@@ -36,9 +36,9 @@ async def test_user_code_execute():
 
 
 @pytest.mark.asyncio
-async def test_user_admin_code():
+async def test_user_admin_add():
     """
-    User logs in, starts a server & executes code
+    User is made an admin, logs in and we check if they are in admin group
     """
     # This *must* be localhost, not an IP
     # aiohttp throws away cookies if we are connecting to an IP!
@@ -56,11 +56,51 @@ async def test_user_admin_code():
     async with User(username, hub_url, partial(login_dummy, password='')) as u:
             await u.login()
             await u.ensure_server()
-            await u.start_kernel()
-            await u.assert_code_output("5 * 4", "20", 5, 5)
 
             # Assert that the user exists
             assert pwd.getpwnam(f'jupyter-{username}') is not None
 
             # Assert that the user has admin rights
+            assert f'jupyter-{username}' in grp.getgrnam('jupyterhub-admins').gr_mem
+
+
+@pytest.mark.asyncio
+async def test_user_admin_remove():
+    """
+    User is made an admin, logs in and we check if they are in admin group.
+
+    Then we remove them from admin group, and check they *aren't* in admin group :D
+    """
+    # This *must* be localhost, not an IP
+    # aiohttp throws away cookies if we are connecting to an IP!
+    hub_url = 'http://localhost'
+    username = secrets.token_hex(8)
+
+    tljh_config_path = [sys.executable, '-m', 'tljh.config']
+
+    assert 0 == await (await asyncio.create_subprocess_exec(*tljh_config_path, 'add-item', 'users.admin', username)).wait()
+    assert 0 == await (await asyncio.create_subprocess_exec(*tljh_config_path, 'reload')).wait()
+
+    # FIXME: wait for reload to finish & hub to come up
+    # Should be part of tljh-config reload
+    await asyncio.sleep(1)
+    async with User(username, hub_url, partial(login_dummy, password='')) as u:
+            await u.login()
+            await u.ensure_server()
+
+            # Assert that the user exists
+            assert pwd.getpwnam(f'jupyter-{username}') is not None
+
+            # Assert that the user has admin rights
+            assert f'jupyter-{username}' in grp.getgrnam('jupyterhub-admins').gr_mem
+
+
+            assert 0 == await (await asyncio.create_subprocess_exec(*tljh_config_path, 'remove-item', 'users.admin', username)).wait()
+            assert 0 == await (await asyncio.create_subprocess_exec(*tljh_config_path, 'reload')).wait()
+            await asyncio.sleep(1)
+
+            await u.stop_server()
+            await u.ensure_server()
+
+            # Assert that the user does *not* have admin rights
             assert f'jupyter-{username}' in grp.getgrnam('jupyterhub-admins').gr_mem
\ No newline at end of file
diff --git a/tests/test_config.py b/tests/test_config.py
index c0abdce..271ae79 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -4,6 +4,7 @@ Test configuration commandline tools
 from tljh import config
 from contextlib import redirect_stdout
 import io
+import pytest
 import tempfile
 
 
@@ -81,6 +82,32 @@ def test_add_to_config_multiple():
         'a': {'b': {'c': ['d', 'e']}}
     }
 
+
+def test_remove_from_config():
+    conf = {}
+
+    new_conf = config.add_item_to_config(conf, 'a.b.c', 'd')
+    new_conf = config.add_item_to_config(new_conf, 'a.b.c', 'e')
+    assert new_conf == {
+        'a': {'b': {'c': ['d', 'e']}}
+    }
+
+    new_conf = config.remove_item_from_config(new_conf, 'a.b.c', 'e')
+    assert new_conf == {
+        'a': {'b': {'c': ['d']}}
+    }
+
+def test_remove_from_config_error():
+    with pytest.raises(ValueError):
+        config.remove_item_from_config({}, 'a.b.c', 'e')
+
+    with pytest.raises(ValueError):
+        config.remove_item_from_config({'a': 'b'}, 'a.b', 'e')
+
+    with pytest.raises(ValueError):
+        config.remove_item_from_config({'a': ['b']}, 'a', 'e')
+
+
 def test_show_config():
     """
     Test stdout output when showing config
diff --git a/tljh/config.py b/tljh/config.py
index 770896e..8398465 100644
--- a/tljh/config.py
+++ b/tljh/config.py
@@ -60,7 +60,6 @@ def add_item_to_config(config, property_path, value):
     for i, cur_path in enumerate(path_components):
         if i == len(path_components) - 1:
             # Final component, it must be a list and we append to it
-            print('l', cur_path, cur_part)
             if cur_path not in cur_part or not isinstance(cur_part[cur_path], list):
                 cur_part[cur_path] = []
             cur_part = cur_part[cur_path]
@@ -69,13 +68,34 @@ def add_item_to_config(config, property_path, value):
         else:
             # If we are asked to create new non-leaf nodes, we will always make them dicts
             # This means setting is *destructive* - will replace whatever is down there!
-            print('p', cur_path, cur_part)
             if cur_path not in cur_part or not isinstance(cur_part[cur_path], dict):
                 cur_part[cur_path] = {}
             cur_part = cur_part[cur_path]
 
     return config_copy
 
+def remove_item_from_config(config, property_path, value):
+    """
+    Add an item to a list in config.
+    """
+    path_components = property_path.split('.')
+
+    # Mutate a copy of the config, not config itself
+    cur_part = config_copy = deepcopy(config)
+    for i, cur_path in enumerate(path_components):
+        if i == len(path_components) - 1:
+            # Final component, it must be a list and we append to it
+            if cur_path not in cur_part or not isinstance(cur_part[cur_path], list):
+                raise ValueError(f'{property_path} is not a list')
+            cur_part = cur_part[cur_path]
+            cur_part.remove(value)
+        else:
+            if cur_path not in cur_part or not isinstance(cur_part[cur_path], dict):
+                raise ValueError(f'{property_path} does not exist in config!')
+            cur_part = cur_part[cur_path]
+
+    return config_copy
+
 
 def show_config(config_path):
     """
@@ -173,11 +193,24 @@ def main():
     )
     add_item_parser.add_argument(
         'key_path',
-        help='Dot separated path to configuration key to set'
+        help='Dot separated path to configuration key to add value to'
     )
     add_item_parser.add_argument(
         'value',
-        help='Value ot set the configuration key to'
+        help='Value to add to the configuration key'
+    )
+
+    remove_item_parser = subparsers.add_parser(
+        'remove-item',
+        help='Remove a value from a list for a configuration property'
+    )
+    remove_item_parser.add_argument(
+        'key_path',
+        help='Dot separated path to configuration key to remove value from'
+    )
+    remove_item_parser.add_argument(
+        'value',
+        help='Value to remove from key_path'
     )
 
     reload_parser = subparsers.add_parser(
@@ -200,6 +233,8 @@ def main():
         set_config_value(args.config_path, args.key_path, args.value)
     elif args.action == 'add-item':
         add_config_value(args.config_path, args.key_path, args.value)
+    elif args.action == 'remove-item':
+        add_config_value(args.config_path, args.key_path, args.value)
     elif args.action == 'reload':
         reload_component(args.component)
     

From 7acd331e3858e297cfa141668713a4251b163130 Mon Sep 17 00:00:00 2001
From: yuvipanda <yuvipanda@gmail.com>
Date: Sat, 28 Jul 2018 13:07:01 -0700
Subject: [PATCH 8/8] Add documentation on adding / removing admin users

---
 docs/guides/admin.rst                | 57 ----------------------------
 docs/howto/admin-users.rst           | 55 +++++++++++++++++++++++++++
 docs/index.rst                       |  2 +-
 docs/topic/customizing-installer.rst |  2 +
 4 files changed, 58 insertions(+), 58 deletions(-)
 delete mode 100644 docs/guides/admin.rst
 create mode 100644 docs/howto/admin-users.rst

diff --git a/docs/guides/admin.rst b/docs/guides/admin.rst
deleted file mode 100644
index f3078aa..0000000
--- a/docs/guides/admin.rst
+++ /dev/null
@@ -1,57 +0,0 @@
-.. _admin_access:
-
-=====================
-Administrative Access
-=====================
-
-In The Littlest JupyterHub, we try to allow users to do as many administrative
-tasks as possible within JupyterHub itself. Admin users can:
-
-1. Have full root access with passwordless ``sudo``
-2. Install system-wide packages with ``apt``
-3. Install ``conda`` / ``pip`` packages for all JupyterHub users
-4. Change the amount of RAM / CPU available to each user, and more!
-
-By default, there are no admin users. You should add some after installation.
-
-Adding admin users
-==================
-
-Admin users are specified in the `YAML <https://en.wikipedia.org/wiki/YAML>`_
-config file at ``/opt/tljh/config.yaml``. This file is created upon installing
-tljh.
-
-1. Open the ``config.yaml`` file for editing.
-
-   .. code-block:: bash
-
-      sudo nano /opt/tljh/config.yaml
-
-2. Add usernames that should have admin access.
-
-   .. code-block:: yaml
-
-      users:
-        admin:
-          - user1
-          - user2
-
-   Be careful around the syntax - indentation matters, and you should be using
-   spaces and not tabs.
-
-   When you are done, save the file and exit. In ``nano``, you can do this with
-   ``Ctrl+X`` key.
-
-3. When you are sure the format is ok, restart JupyterHub to let the config take
-   effect.
-
-   .. code-block:: bash
-
-      sudo systemctl restart jupyterhub
-
-This should give you admin access from JupyterHub! You can verify this by:
-
-1. Opening a Terminal in your JupyterHub and checking if ``sudo`` works
-2. Opening your JupyterHub ``Control Panel`` and checking for the **Admin** tab
-
-From now on, you can use the JupyterHub to do most configuration changes.
diff --git a/docs/howto/admin-users.rst b/docs/howto/admin-users.rst
new file mode 100644
index 0000000..4b72fb3
--- /dev/null
+++ b/docs/howto/admin-users.rst
@@ -0,0 +1,55 @@
+.. _howto/admin-users:
+
+========================
+Add / Remove admin users
+========================
+
+Admin users in TLJH have the following powers:
+
+#. Full root access to the server with passwordless ``sudo``.
+   This lets them do literally whatever they want in the server
+#. Access servers / home directories of all other users
+#. Install new packages for everyone with ``conda``, ``pip`` or ``apt``
+#. Change configuration of TLJH
+
+This is a lot of power, so make sure you know who you are giving it
+to. Admin users should have decent passwords / secure logni mechanisms,
+so attackers can not easily gain control of the system.
+
+Make sure an admin user is present
+==================================
+
+You should make sure an admin user is present when you **install** TLJH
+the very first time. The ``:ref:`--admin <topic/customizing-installer/admin>```
+flag passed to the installer does this. If you had forgotten to do so, the
+easiest way to fix this is to run the installer again.
+
+Adding new admin users
+======================
+
+New admin users can be added by executing the following commands on an
+admin terminal:
+
+.. code-block:: bash
+
+   sudo -E tljh-config add-item users.admin <username>
+   sudo -E tljh-config reload
+
+If the user is already using the JupyterHub, they might have to stop and
+start their server from the control panel to gain new powers.
+
+Removing admin users
+====================
+
+You can remove an existing admin user by executing the following commands in
+an admin terminal:
+
+.. code-block:: bash
+
+   sudo -E tljh-config remove-item users.admin <username>
+   sudo -E tljh-config reload
+
+If the user is already using the JupyterHub, they will continue to have
+some of their admin powers until their server is stopped. Another admin
+can force their server to stop by clicking 'Stop Server' in the admin
+panel.
diff --git a/docs/index.rst b/docs/index.rst
index 93c8529..1c8e5b5 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -54,6 +54,7 @@ How-To guides answer the question 'How do I...?' for a lot of topics.
    :titlesonly:
 
    howto/user-environment
+   howto/admin-users
    howto/notebook-interfaces
    howto/resource-estimation
 
@@ -74,7 +75,6 @@ Topic guides provide in-depth explanations of specific topics.
    :titlesonly:
 
    topic/requirements
-   guides/admin
    topic/security
    topic/customizing-installer
    topic/tljh-config
diff --git a/docs/topic/customizing-installer.rst b/docs/topic/customizing-installer.rst
index 567e596..7a68fe8 100644
--- a/docs/topic/customizing-installer.rst
+++ b/docs/topic/customizing-installer.rst
@@ -13,6 +13,8 @@ is executed as:
 
 This page documents the various options you can pass as commandline parameters to the installer.
 
+.. _topic/customizing-installer/admin:
+
 Adding admin users
 ===================