Fixed server model, removed some auth decorators

jupyterhub/apihandlers/base.py

@@ -87,29 +87,23 @@ class APIHandler(BaseHandler):
             """
             if sub_scope == scopes.Scope.ALL:
                 return True
-            else:
+            elif orm_resource.name in sub_scope.get(kind, []):
-                try:
+                return True
-                    found_resource = orm_resource.name in sub_scope[kind]
+            if kind == 'server':
-                except KeyError:
+                server_format = f"{orm_resource.user.name}/{orm_resource.name}"
-                    found_resource = False
+                if server_format in sub_scope.get(kind, []):
-            if not found_resource:  # Try group-based access
+                    return True
-                if kind == 'server' and 'user' in sub_scope:
+                # Fall back on checking if we have user access
-                    # First check if we have access to user info
+                if orm_resource.user.name in sub_scope.get('user', []):
-                    user_name = orm_resource.user.name
+                    return True
-                    found_resource = user_name in sub_scope['user']
+                # Fall back on checking if we have group access for this user
-                    if not found_resource:
+                orm_resource = orm_resource.user
-                        # Now check for specific servers:
+            if 'group' in sub_scope:
-                        server_format = f"{orm_resource.user / orm_resource.name}"
+                group_names = {group.name for group in orm_resource.groups}
-                        found_resource = server_format in sub_scope[kind]
+                user_in_group = bool(group_names & set(sub_scope['group']))
-                elif 'group' in sub_scope:
+                if user_in_group:
-                    group_names = set()
+                    return True
-                    if kind == 'user':
+            return False
-                        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
 
         return has_access_to
 
@@ -183,8 +177,8 @@ class APIHandler(BaseHandler):
         )
 
     def server_model(self, spawner):
-        """Get the JSON model for a Spawner"""
+        """Get the JSON model for a Spawner
-        server_scope = 'read:users:servers'
+        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):

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):

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(
+    async def get(self, user_name):
-        self, user_name
-    ):  # Fixme: Does not work when only server filter is selected
         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:

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.extend(read_scope_list)
-        scope_list = read_scope_list
-    else:
-        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
+        'all': None,
-        'users': ['read:users', 'users:activity', 'users:servers'],
+        '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

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'

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"""