only admins can modify admins

- if not admin, cannot set admin=True anywhere
- if not admin, cannot modify any user where admin=True

jupyterhub/apihandlers/users.py

@@ -186,6 +186,8 @@ async def post(self):
         # admin is set for all users
         # to create admin and non-admin users requires at least two API requests
         admin = data.get('admin', False)
+        if admin and not self.current_user.admin:
+            raise web.HTTPError(403, "Only admins can grant admin permissions")
 
         to_create = []
         invalid_names = []
@@ -259,12 +261,16 @@ async def post(self, user_name):
         if user is not None:
             raise web.HTTPError(409, "User %s already exists" % user_name)
 
-        user = self.user_from_username(user_name)
         if data:
             self._check_user_model(data)
-            if 'admin' in data:
-                user.admin = data['admin']
-                assign_default_roles(self.db, entity=user)
+            if data.get('admin', False) and not self.current_user.admin:
+                raise web.HTTPError(403, "Only admins can grant admin permissions")
+
+        # create the user
+        user = self.user_from_username(user_name)
+        if data and data.get('admin', False):
+            user.admin = data['admin']
+            assign_default_roles(self.db, entity=user)
         self.db.commit()
 
         try:
@@ -322,7 +328,17 @@ async def patch(self, user_name):
                     400,
                     "User %s already exists, username must be unique" % data['name'],
                 )
+
+        if not self.current_user.admin:
+            if user.admin:
+                raise web.HTTPError(403, "Only admins can modify other admins")
+            if 'admin' in data and data['admin']:
+                raise web.HTTPError(403, "Only admins can grant admin permissions")
         for key, value in data.items():
+            value_s = "..." if key == "auth_state" else repr(value)
+            self.log.info(
+                f"{self.current_user.name} setting {key}={value_s} for {user.name}"
+            )
             if key == 'auth_state':
                 await user.save_auth_state(value)
             else:

jupyterhub/scopes.py

@@ -48,7 +48,7 @@
         'doc_description': 'Access the admin page. Permission to take actions via the admin page granted separately.',
     },
     'admin:users': {
-        'description': 'Read, write, create and delete users and their authentication state, not including their servers or tokens.',
+        'description': 'Read, modify, create, and delete users and their authentication state, not including their servers or tokens. This is an extremely privileged scope and should be considered tantamount to superuser.',
         'subscopes': ['admin:auth_state', 'users', 'read:roles:users', 'delete:users'],
     },
     'admin:auth_state': {'description': 'Read a user’s authentication state.'},
@@ -109,7 +109,7 @@
         'subscopes': ['groups', 'read:roles:groups', 'delete:groups'],
     },
     'groups': {
-        'description': 'Read and write group information, including adding/removing users to/from groups.',
+        'description': 'Read and write group information, including adding/removing users to/from groups. Note: adding users to groups is a way of granting permissions to users of the group,and may grant _access_ to users of other groups.',
         'subscopes': ['read:groups', 'list:groups'],
     },
     'list:groups': {

jupyterhub/tests/test_api.py

@@ -665,16 +665,25 @@ async def test_add_multi_user(app):
 
 @mark.user
 @mark.role
-async def test_add_multi_user_admin(app):
+@mark.parametrize("is_admin", [True, False])
+async def test_add_multi_user_admin(app, create_user_with_scopes, is_admin):
     db = app.db
+    requester = create_user_with_scopes("admin:users")
+    requester.admin = is_admin
+    db.commit()
     names = ['c', 'd']
     r = await api_request(
         app,
         'users',
         method='post',
         data=json.dumps({'usernames': names, 'admin': True}),
+        name=requester.name,
     )
-    assert r.status_code == 201
+    if is_admin:
+        assert r.status_code == 201
+    else:
+        assert r.status_code == 403
+        return
     reply = r.json()
     r_names = [user['name'] for user in reply]
     assert names == r_names
@@ -712,13 +721,26 @@ async def test_add_user_duplicate(app):
 
 @mark.user
 @mark.role
-async def test_add_admin(app):
+@mark.parametrize("is_admin", [True, False])
+async def test_add_admin(app, create_user_with_scopes, is_admin):
     db = app.db
     name = 'newadmin'
+    user = create_user_with_scopes("admin:users")
+    user.admin = is_admin
+    db.commit()
     r = await api_request(
-        app, 'users', name, method='post', data=json.dumps({'admin': True})
+        app,
+        'users',
+        name,
+        method='post',
+        data=json.dumps({'admin': True}),
+        name=user.name,
     )
-    assert r.status_code == 201
+    if is_admin:
+        assert r.status_code == 201
+    else:
+        assert r.status_code == 403
+        return
     user = find_user(db, name)
     assert user is not None
     assert user.name == name
@@ -738,9 +760,14 @@ async def test_delete_user(app):
 
 @mark.user
 @mark.role
-async def test_make_admin(app):
+@mark.parametrize("is_admin", [True, False])
+async def test_user_make_admin(app, create_user_with_scopes, is_admin):
     db = app.db
-    name = 'admin2'
+    requester = create_user_with_scopes('admin:users')
+    requester.admin = is_admin
+    db.commit()
+
+    name = new_username("make_admin")
     r = await api_request(app, 'users', name, method='post')
     assert r.status_code == 201
     user = find_user(db, name)
@@ -751,10 +778,18 @@ async def test_make_admin(app):
     assert orm.Role.find(db, 'admin') not in user.roles
 
     r = await api_request(
-        app, 'users', name, method='patch', data=json.dumps({'admin': True})
+        app,
+        'users',
+        name,
+        method='patch',
+        data=json.dumps({'admin': True}),
+        name=requester.name,
     )
-
-    assert r.status_code == 200
+    if is_admin:
+        assert r.status_code == 200
+    else:
+        assert r.status_code == 403
+        return
     user = find_user(db, name)
     assert user is not None
     assert user.name == name
@@ -763,6 +798,38 @@ async def test_make_admin(app):
     assert orm.Role.find(db, 'admin') in user.roles
 
 
+@mark.user
+@mark.parametrize("requester_is_admin", [True, False])
+@mark.parametrize("user_is_admin", [True, False])
+async def test_user_set_name(
+    app, user, create_user_with_scopes, requester_is_admin, user_is_admin
+):
+    db = app.db
+    requester = create_user_with_scopes('admin:users')
+    requester.admin = requester_is_admin
+    user.admin = user_is_admin
+    db.commit()
+    new_name = new_username()
+
+    r = await api_request(
+        app,
+        'users',
+        user.name,
+        method='patch',
+        data=json.dumps({'name': new_name}),
+        name=requester.name,
+    )
+    if requester_is_admin or not user_is_admin:
+        assert r.status_code == 200
+    else:
+        assert r.status_code == 403
+        return
+    renamed = find_user(db, new_name)
+    assert renamed is not None
+    assert renamed.name == new_name
+    assert renamed.id == user.id
+
+
 @mark.user
 async def test_set_auth_state(app, auth_state_enabled):
     auth_state = {'secret': 'hello'}