From 7544965145b8c15f793151a66d860453a41bf4ef Mon Sep 17 00:00:00 2001
From: 0mar <omar.richardson@kau.se>
Date: Thu, 15 Apr 2021 16:34:46 +0200
Subject: [PATCH 1/5] Fixed server model, removed some auth decorators

---
 jupyterhub/apihandlers/base.py     | 57 ++++++++++++++----------------
 jupyterhub/apihandlers/services.py | 22 ------------
 jupyterhub/apihandlers/users.py    | 24 ++-----------
 jupyterhub/roles.py                | 24 ++++++++-----
 jupyterhub/tests/test_scopes.py    | 10 ++----
 jupyterhub/utils.py                | 13 -------
 6 files changed, 46 insertions(+), 104 deletions(-)

diff --git a/jupyterhub/apihandlers/base.py b/jupyterhub/apihandlers/base.py
index 9832afc5..1659a001 100644
--- a/jupyterhub/apihandlers/base.py
+++ b/jupyterhub/apihandlers/base.py
@@ -87,29 +87,23 @@ class APIHandler(BaseHandler):
             """
             if sub_scope == scopes.Scope.ALL:
                 return True
-            else:
-                try:
-                    found_resource = orm_resource.name in sub_scope[kind]
-                except KeyError:
-                    found_resource = False
-            if not found_resource:  # Try group-based access
-                if kind == 'server' and 'user' in sub_scope:
-                    # First check if we have access to user info
-                    user_name = orm_resource.user.name
-                    found_resource = user_name in sub_scope['user']
-                    if not found_resource:
-                        # Now check for specific servers:
-                        server_format = f"{orm_resource.user / orm_resource.name}"
-                        found_resource = server_format in sub_scope[kind]
-                elif 'group' in sub_scope:
-                    group_names = set()
-                    if kind == 'user':
-                        group_names = {group.name for group in orm_resource.groups}
-                    elif kind == 'server':
-                        group_names = {group.name for group in orm_resource.user.groups}
-                    user_in_group = bool(group_names & set(sub_scope['group']))
-                    found_resource = user_in_group
-            return found_resource
+            elif orm_resource.name in sub_scope.get(kind, []):
+                return True
+            if kind == 'server':
+                server_format = f"{orm_resource.user.name}/{orm_resource.name}"
+                if server_format in sub_scope.get(kind, []):
+                    return True
+                # Fall back on checking if we have user access
+                if orm_resource.user.name in sub_scope.get('user', []):
+                    return True
+                # Fall back on checking if we have group access for this user
+                orm_resource = orm_resource.user
+            if 'group' in sub_scope:
+                group_names = {group.name for group in orm_resource.groups}
+                user_in_group = bool(group_names & set(sub_scope['group']))
+                if user_in_group:
+                    return True
+            return False
 
         return has_access_to
 
@@ -183,8 +177,8 @@ class APIHandler(BaseHandler):
         )
 
     def server_model(self, spawner):
-        """Get the JSON model for a Spawner"""
-        server_scope = 'read:users:servers'
+        """Get the JSON model for a Spawner
+        Assume server permission already granted"""
         server_state_scope = 'admin:users:server_state'
         model = {
             'name': spawner.name,
@@ -196,7 +190,6 @@ class APIHandler(BaseHandler):
             'user_options': spawner.user_options,
             'progress_url': spawner._progress_url,
         }
-        # First check users, then servers
         if server_state_scope in self.parsed_scopes:
             scope_filter = self.get_scope_filter(server_state_scope)
             if scope_filter(spawner, kind='server'):
@@ -260,7 +253,6 @@ class APIHandler(BaseHandler):
             'read:users:activity': {'kind', 'name', 'last_activity'},
             'read:users:servers': {'kind', 'name', 'servers'},
             'admin:users:auth_state': {'kind', 'name', 'auth_state'},
-            'admin:users:server_state': {'kind', 'name', 'servers', 'server_state'},
         }
         self.log.debug(
             "Asking for user model of %s with scopes [%s]",
@@ -277,13 +269,18 @@ class APIHandler(BaseHandler):
         if model:
             if '' in user.spawners and 'pending' in allowed_keys:
                 model['pending'] = user.spawners[''].pending
-            if 'servers' in allowed_keys:
-                servers = model['servers'] = {}
+
+            servers = model['servers'] = {}
+            scope = 'read:users:servers'
+            if scope in self.parsed_scopes:
+                scope_filter = self.get_scope_filter('read:users:servers')
                 for name, spawner in user.spawners.items():
                     # include 'active' servers, not just ready
                     # (this includes pending events)
-                    if spawner.active:
+                    if spawner.active and scope_filter(spawner, kind='server'):
                         servers[name] = self.server_model(spawner)
+            if not servers:
+                model.pop('servers')
         return model
 
     def group_model(self, group):
diff --git a/jupyterhub/apihandlers/services.py b/jupyterhub/apihandlers/services.py
index 4d7247d0..f0b87c30 100644
--- a/jupyterhub/apihandlers/services.py
+++ b/jupyterhub/apihandlers/services.py
@@ -40,28 +40,6 @@ class ServiceListAPIHandler(APIHandler):
         self.write(json.dumps(data))
 
 
-def admin_or_self(method):
-    """Decorator for restricting access to either the target service or admin"""
-    """***Deprecated in favor of RBAC. Use scope-based decorator***"""
-
-    def decorated_method(self, name):
-        current = self.current_user
-        if current is None:
-            raise web.HTTPError(403)
-        if not current.admin:
-            # not admin, maybe self
-            if not isinstance(current, orm.Service):
-                raise web.HTTPError(403)
-            if current.name != name:
-                raise web.HTTPError(403)
-        # raise 404 if not found
-        if name not in self.services:
-            raise web.HTTPError(404)
-        return method(self, name)
-
-    return decorated_method
-
-
 class ServiceAPIHandler(APIHandler):
     @needs_scope('read:services')
     def get(self, service_name):
diff --git a/jupyterhub/apihandlers/users.py b/jupyterhub/apihandlers/users.py
index 492a3a29..5010d1c7 100644
--- a/jupyterhub/apihandlers/users.py
+++ b/jupyterhub/apihandlers/users.py
@@ -169,24 +169,6 @@ class UserListAPIHandler(APIHandler):
         self.set_status(201)
 
 
-def admin_or_self(method):
-    """Decorator for restricting access to either the target user or admin"""
-
-    def m(self, name, *args, **kwargs):
-        current = self.current_user
-        if current is None:
-            raise web.HTTPError(403)
-        if not (current.name == name or current.admin):
-            raise web.HTTPError(403)
-
-        # raise 404 if not found
-        if not self.find_user(name):
-            raise web.HTTPError(404)
-        return method(self, name, *args, **kwargs)
-
-    return m
-
-
 class UserAPIHandler(APIHandler):
     @needs_scope(
         'read:users',
@@ -195,9 +177,7 @@ class UserAPIHandler(APIHandler):
         'read:users:groups',
         'read:users:activity',
     )
-    async def get(
-        self, user_name
-    ):  # Fixme: Does not work when only server filter is selected
+    async def get(self, user_name):
         user = self.find_user(user_name)
         model = self.user_model(user)
         # auth state will only be shown if the requester is an admin
@@ -268,7 +248,7 @@ class UserAPIHandler(APIHandler):
 
         self.set_status(204)
 
-    @needs_scope('admin:users')  # Todo: Change to `users`?
+    @needs_scope('admin:users')
     async def patch(self, user_name):
         user = self.find_user(user_name)
         if user is None:
diff --git a/jupyterhub/roles.py b/jupyterhub/roles.py
index e4bc2e1e..051b3c07 100644
--- a/jupyterhub/roles.py
+++ b/jupyterhub/roles.py
@@ -24,6 +24,7 @@ def get_default_roles():
             'scopes': [
                 'all',
                 'users',
+                'users:servers',
                 'users:tokens',
                 'admin:users',
                 'admin:users:servers',
@@ -51,7 +52,7 @@ def get_default_roles():
     return default_roles
 
 
-def expand_self_scope(name, read_only=False):
+def expand_self_scope(name):
     """
     Users have a metascope 'self' that should be expanded to standard user privileges.
     At the moment that is a user-filtered version (optional read) access to
@@ -71,10 +72,7 @@ def expand_self_scope(name, read_only=False):
         'users:tokens',
     ]
     read_scope_list = ['read:' + scope for scope in scope_list]
-    if read_only:
-        scope_list = read_scope_list
-    else:
-        scope_list.extend(read_scope_list)
+    scope_list.extend(read_scope_list)
     return {"{}!user={}".format(scope, name) for scope in scope_list}
 
 
@@ -87,18 +85,18 @@ def _get_scope_hierarchy():
 
     scopes = {
         'self': None,
-        'all': None,  # Optional 'read:all' as subscope, not implemented at this stage
-        'users': ['read:users', 'users:activity', 'users:servers'],
+        'all': None,
+        'users': ['read:users', 'users:groups', 'users:activity'],
         'read:users': [
             'read:users:name',
             'read:users:groups',
             'read:users:activity',
-            'read:users:servers',
         ],
         'users:tokens': ['read:users:tokens'],
         'admin:users': ['admin:users:auth_state'],
         'admin:users:servers': ['admin:users:server_state'],
         'groups': ['read:groups'],
+        'users:servers': ['read:users:servers'],
         'admin:groups': None,
         'read:services': None,
         'read:hub': None,
@@ -112,13 +110,21 @@ def _get_scope_hierarchy():
 def horizontal_filter(func):
     """Decorator to account for horizontal filtering in scope syntax"""
 
+    def expand_server_filter(hor_filter):
+        resource, mark, value = hor_filter.partition('=')
+        if resource == 'server':
+            user, mark, server = value.partition('/')
+            return f'read:users:name!user={user}'
+
     def ignore(scopename):
         # temporarily remove horizontal filtering if present
         scopename, mark, hor_filter = scopename.partition('!')
         expanded_scope = func(scopename)
         # add the filter back
         full_expanded_scope = {scope + mark + hor_filter for scope in expanded_scope}
-
+        server_filter = expand_server_filter(hor_filter)
+        if server_filter:
+            full_expanded_scope.add(server_filter)
         return full_expanded_scope
 
     return ignore
diff --git a/jupyterhub/tests/test_scopes.py b/jupyterhub/tests/test_scopes.py
index c0f4269d..e16e6f5f 100644
--- a/jupyterhub/tests/test_scopes.py
+++ b/jupyterhub/tests/test_scopes.py
@@ -561,6 +561,7 @@ async def test_metascope_all_expansion(app, create_user_with_scopes):
     "scopes, can_stop ,num_servers, keys_in, keys_out",
     [
         (['read:users:servers!user=almond'], False, 2, {'name'}, {'state'}),
+        (['admin:users', 'read:users'], False, 0, set(), set()),
         (['read:users:servers!group=nuts'], False, 2, {'name'}, {'state'}),
         (
             ['admin:users:server_state', 'read:users:servers'],
@@ -569,15 +570,13 @@ async def test_metascope_all_expansion(app, create_user_with_scopes):
             {'name', 'state'},
             set(),
         ),
-        (['users:servers', 'read:users:name'], True, 0, set(), set()),
         (
             [
-                'read:users:name!user=almond',
                 'read:users:servers!server=almond/bianca',
                 'admin:users:server_state!server=almond/bianca',
             ],
             False,
-            0,  # fixme: server-scope not working yet
+            1,
             {'name', 'state'},
             set(),
         ),
@@ -590,11 +589,6 @@ async def test_server_state_access(
         app.tornado_settings,
         {'allow_named_servers': True, 'named_server_limit_per_user': 2},
     ):
-        ## 1. Test a user can access all servers without auth_state
-        ## 2. Test a service with admin:user but no admin:users:servers gets no access to any server data
-        ## 3. Test a service with admin:user:server_state gets access to auth_state
-        ## 4. Test a service with user:servers!server=x gives access to one server, and the correct server.
-        ## 5. Test a service with users:servers!group=x gives access to both servers
         username = 'almond'
         user = add_user(app.db, app, name=username)
         group_name = 'nuts'
diff --git a/jupyterhub/utils.py b/jupyterhub/utils.py
index d420c13c..73905ea2 100644
--- a/jupyterhub/utils.py
+++ b/jupyterhub/utils.py
@@ -287,19 +287,6 @@ def authenticated_403(self):
         raise web.HTTPError(403)
 
 
-@auth_decorator
-def admin_only(self):
-    """Decorator for restricting access to admin users
-    Deprecated in favor of scopes.need_scope()
-    """
-    user = self.current_user
-    app_log.warning(
-        "Admin decorator is deprecated and will be removed soon. Use scope-based decorator instead"
-    )
-    if user is None or not user.admin:
-        raise web.HTTPError(403)
-
-
 @auth_decorator
 def metrics_authentication(self):
     """Decorator for restricting access to metrics"""

From cb104ffe42e8ba28537e065d7c99140c4ec05864 Mon Sep 17 00:00:00 2001
From: 0mar <omar.richardson@kau.se>
Date: Thu, 15 Apr 2021 17:30:13 +0200
Subject: [PATCH 2/5] Fixed tests

---
 jupyterhub/apihandlers/base.py         | 17 ++++++++---------
 jupyterhub/tests/test_api.py           |  2 +-
 jupyterhub/tests/test_named_servers.py |  4 ++--
 jupyterhub/tests/test_roles.py         |  4 +---
 jupyterhub/tests/test_scopes.py        |  4 ++--
 5 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/jupyterhub/apihandlers/base.py b/jupyterhub/apihandlers/base.py
index 1659a001..2522ee3b 100644
--- a/jupyterhub/apihandlers/base.py
+++ b/jupyterhub/apihandlers/base.py
@@ -70,14 +70,12 @@ class APIHandler(BaseHandler):
         """Produce a filter for `*ListAPIHandlers* so that GET method knows which models to return.
         Filter is a callable that takes a resource name and outputs true or false"""
 
-        try:
-            sub_scope = self.parsed_scopes[req_scope]
-        except AttributeError:
-            raise web.HTTPError(
-                403,
-                "Resource scope %s (that was just accessed) not found in parsed scope model"
-                % req_scope,
-            )
+        def has_no_access(orm_resource, kind):
+            return False
+
+        if req_scope not in self.parsed_scopes:
+            return has_no_access
+        sub_scope = self.parsed_scopes[req_scope]
 
         def has_access_to(orm_resource, kind):
             """
@@ -98,7 +96,8 @@ class APIHandler(BaseHandler):
                     return True
                 # Fall back on checking if we have group access for this user
                 orm_resource = orm_resource.user
-            if 'group' in sub_scope:
+                kind = 'user'
+            if kind == 'user' and 'group' in sub_scope:
                 group_names = {group.name for group in orm_resource.groups}
                 user_in_group = bool(group_names & set(sub_scope['group']))
                 if user_in_group:
diff --git a/jupyterhub/tests/test_api.py b/jupyterhub/tests/test_api.py
index 2881a69c..6597e587 100644
--- a/jupyterhub/tests/test_api.py
+++ b/jupyterhub/tests/test_api.py
@@ -158,7 +158,7 @@ def fill_user(model):
     model.setdefault('pending', None)
     model.setdefault('created', TIMESTAMP)
     model.setdefault('last_activity', TIMESTAMP)
-    model.setdefault('servers', {})
+    # model.setdefault('servers', {})
     return model
 
 
diff --git a/jupyterhub/tests/test_named_servers.py b/jupyterhub/tests/test_named_servers.py
index 88a48d6d..e07f405f 100644
--- a/jupyterhub/tests/test_named_servers.py
+++ b/jupyterhub/tests/test_named_servers.py
@@ -86,7 +86,7 @@ async def test_default_server(app, named_servers):
 
     user_model = normalize_user(r.json())
     assert user_model == fill_user(
-        {'name': username, 'roles': ['user'], 'servers': {}, 'auth_state': None}
+        {'name': username, 'roles': ['user'], 'auth_state': None}
     )
 
 
@@ -160,7 +160,7 @@ async def test_delete_named_server(app, named_servers):
 
     user_model = normalize_user(r.json())
     assert user_model == fill_user(
-        {'name': username, 'roles': ['user'], 'auth_state': None, 'servers': {}}
+        {'name': username, 'roles': ['user'], 'auth_state': None}
     )
     # wrapper Spawner is gone
     assert servername not in user.spawners
diff --git a/jupyterhub/tests/test_roles.py b/jupyterhub/tests/test_roles.py
index 5192ccf9..279324cf 100644
--- a/jupyterhub/tests/test_roles.py
+++ b/jupyterhub/tests/test_roles.py
@@ -181,11 +181,10 @@ def test_orm_roles_delete_cascade(db):
                 'users',
                 'read:users',
                 'users:activity',
-                'users:servers',
+                'users:groups',
                 'read:users:name',
                 'read:users:groups',
                 'read:users:activity',
-                'read:users:servers',
             },
         ),
         (
@@ -195,7 +194,6 @@ def test_orm_roles_delete_cascade(db):
                 'read:users:name',
                 'read:users:groups',
                 'read:users:activity',
-                'read:users:servers',
             },
         ),
         (['read:users:servers'], {'read:users:servers'}),
diff --git a/jupyterhub/tests/test_scopes.py b/jupyterhub/tests/test_scopes.py
index e16e6f5f..e9190bcc 100644
--- a/jupyterhub/tests/test_scopes.py
+++ b/jupyterhub/tests/test_scopes.py
@@ -492,10 +492,10 @@ async def test_vertical_filter(app, create_user_with_scopes):
 
 
 async def test_stacked_vertical_filter(app, create_user_with_scopes):
-    user = create_user_with_scopes('read:users:activity', 'read:users:servers')
+    user = create_user_with_scopes('read:users:activity', 'read:users:groups')
     r = await api_request(app, 'users', headers=auth_header(app.db, user.name))
     assert r.status_code == 200
-    allowed_keys = {'name', 'kind', 'servers', 'last_activity'}
+    allowed_keys = {'name', 'kind', 'groups', 'last_activity'}
     result_model = set([key for user in r.json() for key in user.keys()])
     assert result_model == allowed_keys
 

From b23385902803861c3b7206464030a0d43846eacd Mon Sep 17 00:00:00 2001
From: 0mar <omar.richardson@kau.se>
Date: Fri, 16 Apr 2021 14:03:31 +0200
Subject: [PATCH 3/5] Refactored scope_filter

---
 jupyterhub/apihandlers/base.py | 73 +++++++++++++++-------------------
 1 file changed, 32 insertions(+), 41 deletions(-)

diff --git a/jupyterhub/apihandlers/base.py b/jupyterhub/apihandlers/base.py
index 2522ee3b..f630e350 100644
--- a/jupyterhub/apihandlers/base.py
+++ b/jupyterhub/apihandlers/base.py
@@ -13,6 +13,7 @@ from tornado import web
 from .. import orm
 from .. import scopes
 from ..handlers import BaseHandler
+from ..user import User
 from ..utils import isoformat
 from ..utils import url_path_join
 
@@ -70,18 +71,17 @@ class APIHandler(BaseHandler):
         """Produce a filter for `*ListAPIHandlers* so that GET method knows which models to return.
         Filter is a callable that takes a resource name and outputs true or false"""
 
-        def has_no_access(orm_resource, kind):
+        def no_access(orm_resource, kind):
             return False
 
         if req_scope not in self.parsed_scopes:
-            return has_no_access
+            return no_access
         sub_scope = self.parsed_scopes[req_scope]
 
         def has_access_to(orm_resource, kind):
             """
             param orm_resource: User or Service or Group or spawner
             param kind: 'user' or 'service' or 'group' or 'server'.
-            `kind` could probably be derived from `orm_resource`, problem is Jupyterhub.users.User
             """
             if sub_scope == scopes.Scope.ALL:
                 return True
@@ -178,7 +178,6 @@ class APIHandler(BaseHandler):
     def server_model(self, spawner):
         """Get the JSON model for a Spawner
         Assume server permission already granted"""
-        server_state_scope = 'admin:users:server_state'
         model = {
             'name': spawner.name,
             'last_activity': isoformat(spawner.orm_spawner.last_activity),
@@ -189,10 +188,9 @@ class APIHandler(BaseHandler):
             'user_options': spawner.user_options,
             'progress_url': spawner._progress_url,
         }
-        if server_state_scope in self.parsed_scopes:
-            scope_filter = self.get_scope_filter(server_state_scope)
-            if scope_filter(spawner, kind='server'):
-                model['state'] = spawner.get_state()
+        scope_filter = self.get_scope_filter('admin:users:server_state')
+        if scope_filter(spawner, kind='server'):
+            model['state'] = spawner.get_state()
         return model
 
     def token_model(self, token):
@@ -260,24 +258,21 @@ class APIHandler(BaseHandler):
         )
         allowed_keys = set()
         for scope in access_map:
-            if scope in self.parsed_scopes:
-                scope_filter = self.get_scope_filter(scope)
-                if scope_filter(user, kind='user'):
-                    allowed_keys |= access_map[scope]
+            scope_filter = self.get_scope_filter(scope)
+            if scope_filter(user, kind='user'):
+                allowed_keys |= access_map[scope]
         model = {key: model[key] for key in allowed_keys if key in model}
         if model:
             if '' in user.spawners and 'pending' in allowed_keys:
                 model['pending'] = user.spawners[''].pending
 
             servers = model['servers'] = {}
-            scope = 'read:users:servers'
-            if scope in self.parsed_scopes:
-                scope_filter = self.get_scope_filter('read:users:servers')
-                for name, spawner in user.spawners.items():
-                    # include 'active' servers, not just ready
-                    # (this includes pending events)
-                    if spawner.active and scope_filter(spawner, kind='server'):
-                        servers[name] = self.server_model(spawner)
+            scope_filter = self.get_scope_filter('read:users:servers')
+            for name, spawner in user.spawners.items():
+                # include 'active' servers, not just ready
+                # (this includes pending events)
+                if spawner.active and scope_filter(spawner, kind='server'):
+                    servers[name] = self.server_model(spawner)
             if not servers:
                 model.pop('servers')
         return model
@@ -285,32 +280,28 @@ class APIHandler(BaseHandler):
     def group_model(self, group):
         """Get the JSON model for a Group object"""
         model = {}
-        req_scope = 'read:groups'
-        if req_scope in self.parsed_scopes:
-            scope_filter = self.get_scope_filter(req_scope)
-            if scope_filter(group, kind='group'):
-                model = {
-                    'kind': 'group',
-                    'name': group.name,
-                    'roles': [r.name for r in group.roles],
-                    'users': [u.name for u in group.users],
-                }
+        scope_filter = self.get_scope_filter('read:groups')
+        if scope_filter(group, kind='group'):
+            model = {
+                'kind': 'group',
+                'name': group.name,
+                'roles': [r.name for r in group.roles],
+                'users': [u.name for u in group.users],
+            }
         return model
 
     def service_model(self, service):
         """Get the JSON model for a Service object"""
         model = {}
-        req_scope = 'read:services'
-        if req_scope in self.parsed_scopes:
-            scope_filter = self.get_scope_filter(req_scope)
-            if scope_filter(service, kind='service'):
-                model = {
-                    'kind': 'service',
-                    'name': service.name,
-                    'roles': [r.name for r in service.roles],
-                    'admin': service.admin,
-                }
-                # todo: Remove once we replace admin flag with role check
+        scope_filter = self.get_scope_filter('read:services')
+        if scope_filter(service, kind='service'):
+            model = {
+                'kind': 'service',
+                'name': service.name,
+                'roles': [r.name for r in service.roles],
+                'admin': service.admin,
+            }
+            # todo: Remove once we replace admin flag with role check
         return model
 
     _user_model_types = {

From 46e2f72fa65372a072a175047642837b70155a08 Mon Sep 17 00:00:00 2001
From: 0mar <omar.richardson@kau.se>
Date: Fri, 16 Apr 2021 14:54:04 +0200
Subject: [PATCH 4/5] Test server start/stop

---
 jupyterhub/apihandlers/users.py |   2 +-
 jupyterhub/tests/test_scopes.py | 104 ++++++++++++++++++--------------
 2 files changed, 61 insertions(+), 45 deletions(-)

diff --git a/jupyterhub/apihandlers/users.py b/jupyterhub/apihandlers/users.py
index 5010d1c7..23ac0027 100644
--- a/jupyterhub/apihandlers/users.py
+++ b/jupyterhub/apihandlers/users.py
@@ -301,7 +301,7 @@ class UserTokenListAPIHandler(APIHandler):
 
         self.write(json.dumps({'api_tokens': api_tokens}))
 
-    # Todo: Set to @needs_scope('users:tokens')
+    @needs_scope('users:tokens')
     async def post(self, user_name):
         body = self.get_json_body() or {}
         if not isinstance(body, dict):
diff --git a/jupyterhub/tests/test_scopes.py b/jupyterhub/tests/test_scopes.py
index e9190bcc..98a5dc93 100644
--- a/jupyterhub/tests/test_scopes.py
+++ b/jupyterhub/tests/test_scopes.py
@@ -289,10 +289,11 @@ def create_user_with_scopes(app, create_temp_role):
     counter = 0
     get_role = create_temp_role
 
-    def temp_user_creator(*scopes):
+    def temp_user_creator(*scopes, name=None):
         nonlocal counter
-        counter += 1
-        name = f"temp_user_{counter}"
+        if name is None:
+            counter += 1
+            name = f"temp_user_{counter}"
         role = get_role(scopes)
         orm_user = orm.User(name=name)
         app.db.add(orm_user)
@@ -314,10 +315,11 @@ def create_service_with_scopes(app, create_temp_role):
     counter = 0
     role_function = create_temp_role
 
-    def temp_service_creator(*scopes):
+    def temp_service_creator(*scopes, name=None):
         nonlocal counter
-        counter += 1
-        name = f"temp_service_{counter}"
+        if name is None:
+            counter += 1
+            name = f"temp_service_{counter}"
         role = role_function(scopes)
         app.services.append({'name': name})
         app.init_services()
@@ -562,10 +564,16 @@ async def test_metascope_all_expansion(app, create_user_with_scopes):
     [
         (['read:users:servers!user=almond'], False, 2, {'name'}, {'state'}),
         (['admin:users', 'read:users'], False, 0, set(), set()),
-        (['read:users:servers!group=nuts'], False, 2, {'name'}, {'state'}),
+        (
+            ['read:users:servers!group=nuts', 'users:servers'],
+            True,
+            2,
+            {'name'},
+            {'state'},
+        ),
         (
             ['admin:users:server_state', 'read:users:servers'],
-            True,  # Todo: test for server stop
+            False,
             2,
             {'name', 'state'},
             set(),
@@ -583,14 +591,20 @@ async def test_metascope_all_expansion(app, create_user_with_scopes):
     ],
 )
 async def test_server_state_access(
-    app, scopes, can_stop, num_servers, keys_in, keys_out
+    app,
+    create_user_with_scopes,
+    create_service_with_scopes,
+    scopes,
+    can_stop,
+    num_servers,
+    keys_in,
+    keys_out,
 ):
     with mock.patch.dict(
         app.tornado_settings,
         {'allow_named_servers': True, 'named_server_limit_per_user': 2},
     ):
-        username = 'almond'
-        user = add_user(app.db, app, name=username)
+        user = create_user_with_scopes('self', name='almond')
         group_name = 'nuts'
         group = orm.Group.find(app.db, name=group_name)
         if not group:
@@ -599,36 +613,38 @@ async def test_server_state_access(
         group.users.append(user)
         app.db.commit()
         server_names = ['bianca', 'terry']
-        try:
-            for server_name in server_names:
-                await api_request(
-                    app, 'users', username, 'servers', server_name, method='post'
-                )
-            role = orm.Role(name=f"{username}-role", scopes=scopes)
-            app.db.add(role)
-            app.db.commit()
-            service_name = 'server_accessor'
-            service = orm.Service(name=service_name)
-            app.db.add(service)
-            service.roles.append(role)
-            app.db.commit()
-            api_token = service.new_api_token()
-            await app.init_roles()
-            headers = {'Authorization': 'token %s' % api_token}
-            r = await api_request(app, 'users', username, headers=headers)
-            r.raise_for_status()
-            user_model = r.json()
-            if num_servers:
-                assert 'servers' in user_model
-                server_models = user_model['servers']
-                assert len(server_models) == num_servers
-                for server, server_model in server_models.items():
-                    assert keys_in.issubset(server_model)
-                    assert keys_out.isdisjoint(server_model)
-            else:
-                assert 'servers' not in user_model
-        finally:
-            app.db.delete(role)
-            app.db.delete(service)
-            app.db.delete(group)
-            app.db.commit()
+        for server_name in server_names:
+            await api_request(
+                app, 'users', user.name, 'servers', server_name, method='post'
+            )
+        service = create_service_with_scopes(*scopes)
+        api_token = service.new_api_token()
+        await app.init_roles()
+        headers = {'Authorization': 'token %s' % api_token}
+        r = await api_request(app, 'users', user.name, headers=headers)
+        r.raise_for_status()
+        user_model = r.json()
+        if num_servers:
+            assert 'servers' in user_model
+            server_models = user_model['servers']
+            assert len(server_models) == num_servers
+            for server, server_model in server_models.items():
+                assert keys_in.issubset(server_model)
+                assert keys_out.isdisjoint(server_model)
+        else:
+            assert 'servers' not in user_model
+        r = await api_request(
+            app,
+            'users',
+            user.name,
+            'servers',
+            server_names[0],
+            method='delete',
+            headers=headers,
+        )
+        if can_stop:
+            assert r.status_code == 204
+        else:
+            assert r.status_code == 403
+        app.db.delete(group)
+        app.db.commit()

From ef1351b441ad0374f3d5929b86e671a449cf3adc Mon Sep 17 00:00:00 2001
From: 0mar <omar.richardson@kau.se>
Date: Tue, 20 Apr 2021 11:04:04 +0200
Subject: [PATCH 5/5] Added todo for future PR

---
 jupyterhub/apihandlers/users.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/jupyterhub/apihandlers/users.py b/jupyterhub/apihandlers/users.py
index 23ac0027..b245fbc4 100644
--- a/jupyterhub/apihandlers/users.py
+++ b/jupyterhub/apihandlers/users.py
@@ -301,7 +301,7 @@ class UserTokenListAPIHandler(APIHandler):
 
         self.write(json.dumps({'api_tokens': api_tokens}))
 
-    @needs_scope('users:tokens')
+    # @needs_scope('users:tokens') #Todo: needs internal scope checking
     async def post(self, user_name):
         body = self.get_json_body() or {}
         if not isinstance(body, dict):