Removed database calls and made scope filter a callable

jupyterhub/apihandlers/base.py

@@ -2,6 +2,7 @@
 # Copyright (c) Jupyter Development Team.
 # Distributed under the terms of the Modified BSD License.
 import json
+import re
 from datetime import datetime
 from http.client import responses
 
@@ -9,6 +10,7 @@ from sqlalchemy.exc import SQLAlchemyError
 from tornado import web
 
 from .. import orm
+from .. import roles
 from ..handlers import BaseHandler
 from ..handlers import scopes
 from ..utils import isoformat
@@ -24,8 +26,27 @@ class APIHandler(BaseHandler):
     - strict referer checking for Cookie-authenticated requests
     - strict content-security-policy
     - methods for REST API models
+    - scope loading
     """
 
+    async def prepare(self):
+        await super().prepare()
+        self.raw_scopes = set()
+        self.parsed_scopes = {}
+        self._parse_scopes()
+
+    def _parse_scopes(self):
+        """Parse raw scope collection into a dict with filters that can be used to resolve API access"""
+        self.log.debug("Parsing scopes")
+        if self.current_user is not None:
+            self.raw_scopes = roles.get_subscopes(*self.current_user.roles)
+        oauth_token = self.get_current_user_oauth_token()
+        if oauth_token:
+            self.raw_scopes |= scopes.get_user_scopes(oauth_token.name)
+        if 'all' in self.raw_scopes:
+            self.raw_scopes |= scopes.get_user_scopes(self.current_user.name)
+        self.parsed_scopes = scopes.parse_scopes(self.raw_scopes)
+
     @property
     def content_security_policy(self):
         return '; '.join([super().content_security_policy, "default-src 'none'"])
@@ -64,36 +85,41 @@ class APIHandler(BaseHandler):
         return True
 
     def get_scope_filter(self, req_scope):
-        """Produce a filter for `*ListAPIHandlers* so that GET method knows which models to return"""
+        """Produce a filter for `*ListAPIHandlers* so that GET method knows which models to return.
-        scope_translator = {
+        Filter is a callable that takes a resource name and outputs true or false"""
-            'read:users': 'users',
+        kind_regex = re.compile(r':?(user|service|group)s:?')
-            'read:services': 'services',
+        try:
-            'read:groups': 'groups',
+            kind = re.search(kind_regex, req_scope).group(1)
-        }
+        except AttributeError:
-        if req_scope not in scope_translator:
+            self.log.warning(
-            raise AttributeError("Internal error: inconsistent scope situation")
+                "Regex error while processing scope %s, throwing 500", req_scope
-        kind = scope_translator[req_scope]
+            )
-        Resource = orm.get_class(kind)
+            raise web.HTTPError(
+                log_message="Unrecognized scope guard on method: %s" % req_scope
+            )
         try:
             sub_scope = self.parsed_scopes[req_scope]
         except AttributeError:
             raise web.HTTPError(
                 403,
-                "Resource scope %s (that was just accessed) not found in scopes anymore"
+                "Resource scope %s (that was just accessed) not found in parsed scope model"
                 % req_scope,
             )
-        if sub_scope == scopes.Scope.ALL:
+
-            return None  # Full access
+        def has_access(resource_name):
-        sub_scope_values = next(iter(sub_scope.values()))
+            if sub_scope == scopes.Scope.ALL:
-        query = self.db.query(Resource).filter(Resource.name.in_(sub_scope_values))
+                found_resource = True
-        scope_filter = {entry.name for entry in query.all()}
+            else:
-        if 'group' in sub_scope and kind == 'users':
+                found_resource = resource_name in sub_scope[kind]
-            groups = orm.Group.name.in_(sub_scope['group'])
+            if not found_resource:  # Try group-based access
-            users_in_groups = (
+                if 'groups' in sub_scope and kind == 'users':
-                self.db.query(orm.User).join(orm.Group.users).filter(groups)
+                    user = self.current_user()
-            )
+                    if user:
-            scope_filter |= {user.name for user in users_in_groups}
+                        user_in_group = bool(user.groups & sub_scope['groups'])
-        return scope_filter
+                        found_resource |= user_in_group
+            return found_resource
+
+        return has_access
 
     def get_current_user_cookie(self):
         """Override get_user_cookie to check Referer header"""
@@ -234,24 +260,12 @@ class APIHandler(BaseHandler):
             'last_activity': isoformat(user.last_activity),
         }
         access_map = {
-            'read:users': {
+            'read:users': set(model.keys()),  # All available components
-                'kind',
+            'read:users:names': {'kind', 'name'},
-                'name',
-                'admin',
-                'roles',
-                'groups',
-                'server',
-                'servers',
-                'pending',
-                'created',
-                'last_activity',
-            },
-            'read:users:name': {'kind', 'name'},
             'read:users:groups': {'kind', 'name', 'groups'},
             'read:users:activity': {'kind', 'name', 'last_activity'},
             'read:users:servers': {'kind', 'name', 'servers'},
         }
-        # Todo: Should 'name' be included in all access?
         self.log.debug(
             "Asking for user models with scopes [%s]" % ", ".join(self.raw_scopes)
         )
@@ -259,24 +273,23 @@ class APIHandler(BaseHandler):
         for scope in access_map:
             if scope in self.parsed_scopes:
                 scope_filter = self.get_scope_filter(scope)
-                if scope_filter is None or user.name in scope_filter:
+                if scope_filter(user.name):
                     allowed_keys |= access_map[scope]
         model = {key: model[key] for key in allowed_keys if key in model}
-        if not model:
+        if model:
-            return model  # No access to this user
+            if '' in user.spawners and 'pending' in allowed_keys:
-        if '' in user.spawners and 'pending' in allowed_keys:
+                model['pending'] = user.spawners[''].pending
-            model['pending'] = user.spawners[''].pending
+            if not (include_servers and 'servers' in allowed_keys):
-        if not (include_servers and 'servers' in allowed_keys):
+                model['servers'] = None
-            model['servers'] = None
+            else:
-        else:
+                servers = model['servers'] = {}
-            servers = model['servers'] = {}
+                for name, spawner in user.spawners.items():
-            for name, spawner in user.spawners.items():
+                    # include 'active' servers, not just ready
-                # include 'active' servers, not just ready
+                    # (this includes pending events)
-                # (this includes pending events)
+                    if spawner.active:
-                if spawner.active:
+                        servers[name] = self.server_model(
-                    servers[name] = self.server_model(
+                            spawner, include_state=include_state
-                        spawner, include_state=include_state
+                        )
-                    )
         return model
 
     def group_model(self, group):

jupyterhub/apihandlers/groups.py

@@ -38,10 +38,8 @@ class GroupListAPIHandler(_GroupAPIHandler):
     def get(self):
         """List groups"""
         groups = self.db.query(orm.Group)
-        scope_filter = self.get_scope_filter(self.db)
+        scope_filter = self.get_scope_filter('read:groups')
-        if scope_filter is not None:
+        data = [self.group_model(g) for g in groups if scope_filter(g)]
-            groups = groups.filter(orm.Group.name.in_(scope_filter))
-        data = [self.group_model(g) for g in groups]
         self.write(json.dumps(data))
 
     @needs_scope('admin:groups')

jupyterhub/handlers/base.py

@@ -81,8 +81,6 @@ class BaseHandler(RequestHandler):
         The current user (None if not logged in) may be accessed
         via the `self.current_user` property during the handling of any request.
         """
-        self.raw_scopes = set()
-        self.parsed_scopes = set()
         try:
             await self.get_current_user()
         except Exception:
@@ -431,16 +429,8 @@ class BaseHandler(RequestHandler):
                 # don't let errors here raise more than once
                 self._jupyterhub_user = None
                 self.log.exception("Error getting current user")
-        self._parse_scopes()
         return self._jupyterhub_user
 
-    def _parse_scopes(self):
-        if self._jupyterhub_user is not None or self.get_current_user_oauth_token():
-            self.raw_scopes = roles.get_subscopes(*self._jupyterhub_user.roles)
-            if 'all' in self.raw_scopes:
-                self.raw_scopes |= scopes.get_user_scopes(self.current_user.name)
-            self.parsed_scopes = scopes.parse_scopes(self.raw_scopes)
-
     @property
     def current_user(self):
         """Override .current_user accessor from tornado

jupyterhub/scopes.py

@@ -1,19 +1,18 @@
 import functools
 import inspect
-import re
 from enum import Enum
 
 from tornado import web
 from tornado.log import app_log
 
-from . import orm
+from . import roles
 
 
 class Scope(Enum):
     ALL = True
 
 
-def get_user_scopes(name):
+def get_user_scopes(name, read_only=False):
     """
     Scopes have a metascope 'all' that should be expanded to everything a user can do.
     At the moment that is a user-filtered version (optional read) access to
@@ -32,7 +31,11 @@ def get_user_scopes(name):
         'users:servers',
         'users:tokens',
     ]
-    scope_list.extend(['read:' + scope for scope in scope_list])
+    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)
     return {"{}!user={}".format(scope, name) for scope in scope_list}
 
 
@@ -55,7 +58,7 @@ def _check_user_in_expanded_scope(handler, user_name, scope_group_names):
     user = handler.find_user(user_name)
     if user is None:
         raise web.HTTPError(404, "No access to resources or resources not found")
-    group_names = {group.name for group in user.groups}  # SQL query faster?
+    group_names = {group.name for group in user.groups}
     return bool(set(scope_group_names) & group_names)
 
 
@@ -67,7 +70,7 @@ def _check_scope(api_handler, req_scope, **kwargs):
     # Parse user name and server name together
     try:
         api_name = api_handler.request.path
-    except:
+    except AttributeError:
         api_name = type(api_handler).__name__
     if 'user' in kwargs and 'server' in kwargs:
         kwargs['server'] = "{}/{}".format(kwargs['user'], kwargs['server'])
@@ -147,6 +150,14 @@ def needs_scope(*scopes):
             bound_sig = sig.bind(self, *args, **kwargs)
             bound_sig.apply_defaults()
             s_kwargs = {}
+            # current_user = self.current_user()
+            # if current_user is not None or self.get_current_user_oauth_token():
+            #     self.raw_scopes = roles.get_subscopes(*current_user.roles)
+            #     if 'all' in self.raw_scopes:
+            #         self.raw_scopes |= get_user_scopes(self.current_user.name)
+            #     self.parsed_scopes = parse_scopes(self.raw_scopes)
+            # else:
+            #     app_log.warning("No user found in access checking, so no scopes loaded")
             for resource in {'user', 'server', 'group', 'service'}:
                 resource_name = resource + '_name'
                 if resource_name in bound_sig.arguments:

jupyterhub/tests/test_scopes.py

@@ -54,7 +54,7 @@ def test_scope_check_present():
 
 def test_scope_check_not_present():
     handler = get_handler_with_scopes(['read:users!user=maeby'])
-    assert not _check_scope(handler, 'read:users')
+    assert _check_scope(handler, 'read:users')
     with pytest.raises(web.HTTPError):
         _check_scope(handler, 'read:users', user='gob')
     with pytest.raises(web.HTTPError):
@@ -103,7 +103,8 @@ class MockAPIHandler:
         return True
 
     @needs_scope('users')
-    def other_thing(self, other_name):
+    def other_thing(self, non_filter_argument):
+        # Rely on inner vertical filtering
         return True
 
     @needs_scope('users')
@@ -161,8 +162,8 @@ class MockAPIHandler:
         ),
         (['users'], 'other_thing', ('gob',), True),
         (['read:users'], 'other_thing', ('gob',), False),
-        (['users!user=gob'], 'other_thing', ('gob',), False),
+        (['users!user=gob'], 'other_thing', ('gob',), True),
-        (['users!user=gob'], 'other_thing', ('maeby',), False),
+        (['users!user=gob'], 'other_thing', ('maeby',), True),
     ],
 )
 def test_scope_method_access(scopes, method, arguments, is_allowed):
@@ -403,12 +404,47 @@ async def test_vertical_filter(app):
 
     r = await api_request(app, 'users', headers=auth_header(app.db, user_name))
     assert r.status_code == 200
-    assert set(r.json().keys()) == {'names'}
+    allowed_keys = {'name', 'kind'}
+    assert set([key for user in r.json() for key in user.keys()]) == allowed_keys
 
 
 async def test_stacked_vertical_filter(app):
-    pass
+    user_name = 'user'
+    test_role = generate_test_role(
+        user_name, ['read:users:activity', 'read:users:servers']
+    )
+    roles.add_role(app.db, test_role)
+    roles.add_obj(app.db, objname=user_name, kind='users', rolename='test')
+    roles.remove_obj(app.db, objname=user_name, kind='users', rolename='user')
+    app.db.commit()
+
+    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'}
+    result_model = set([key for user in r.json() for key in user.keys()])
+    assert result_model == allowed_keys
 
 
 async def test_cross_filter(app):
-    pass
+    user_name = 'abed'
+    add_user(app.db, name=user_name)
+    test_role = generate_test_role(
+        user_name, ['read:users:activity', 'read:users!user=abed']
+    )
+    roles.add_role(app.db, test_role)
+    roles.add_obj(app.db, objname=user_name, kind='users', rolename='test')
+    roles.remove_obj(app.db, objname=user_name, kind='users', rolename='user')
+    app.db.commit()
+    new_users = {'britta', 'jeff', 'annie'}
+    for new_user_name in new_users:
+        add_user(app.db, name=new_user_name)
+    app.db.commit()
+    r = await api_request(app, 'users', headers=auth_header(app.db, user_name))
+    assert r.status_code == 200
+    restricted_keys = {'name', 'kind', 'last_activity'}
+    key_in_full_model = 'created'
+    for user in r.json():
+        if user['name'] == user_name:
+            assert key_in_full_model in user
+        else:
+            assert set(user.keys()) == restricted_keys