Fixed vertical filtering in user models, but does not work for OAuth yet

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
 
@@ -64,15 +65,19 @@ 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"""
-        scope_translator = {
-            'read:users': 'users',
-            'read:services': 'services',
-            'read:groups': 'groups',
-        }
-        if req_scope not in scope_translator:
-            raise AttributeError("Internal error: inconsistent scope situation")
-        kind = scope_translator[req_scope]
+        """Produce a filter for `*ListAPIHandlers* so that GET method knows which models to return
+        If the APIHandler has unrestricted access
+        """
+        kind_regex = re.compile(r':?(users|services|groups):?')
+        try:
+            kind = re.search(kind_regex, req_scope).group(1)
+        except AttributeError:
+            self.log.warning(
+                "Regex error while processing scope %s, throwing 500", req_scope
+            )
+            raise web.HTTPError(
+                log_message="Unrecognized scope guard on method: %s" % req_scope
+            )
         Resource = orm.get_class(kind)
         try:
             sub_scope = self.parsed_scopes[req_scope]
@@ -246,7 +251,7 @@ class APIHandler(BaseHandler):
                 'created',
                 'last_activity',
             },
-            'read:users:name': {'kind', 'name'},
+            'read:users:names': {'kind', 'name'},
             'read:users:groups': {'kind', 'name', 'groups'},
             'read:users:activity': {'kind', 'name', 'last_activity'},
             'read:users:servers': {'kind', 'name', 'servers'},
@@ -255,28 +260,32 @@ class APIHandler(BaseHandler):
         self.log.debug(
             "Asking for user models with scopes [%s]" % ", ".join(self.raw_scopes)
         )
+        self.log.debug(
+            "Current requests have db loaded scopes [%s]" % self.current_user
+        )
         allowed_keys = set()
+        # if not self.parsed_scopes:
+        #     self.parse_scopes()
         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:
                     allowed_keys |= access_map[scope]
         model = {key: model[key] for key in allowed_keys if key in model}
-        if not model:
-            return model  # No access to this user
-        if '' in user.spawners and 'pending' in allowed_keys:
-            model['pending'] = user.spawners[''].pending
-        if not (include_servers and 'servers' in allowed_keys):
-            model['servers'] = None
-        else:
-            servers = model['servers'] = {}
-            for name, spawner in user.spawners.items():
-                # include 'active' servers, not just ready
-                # (this includes pending events)
-                if spawner.active:
-                    servers[name] = self.server_model(
-                        spawner, include_state=include_state
-                    )
+        if model:
+            if '' in user.spawners and 'pending' in allowed_keys:
+                model['pending'] = user.spawners[''].pending
+            if (
+                include_servers and 'servers' in allowed_keys
+            ):  # Todo: Log breaking change: now no server component in user model if no access
+                servers = model['servers'] = {}
+                for name, spawner in user.spawners.items():
+                    # include 'active' servers, not just ready
+                    # (this includes pending events)
+                    if spawner.active:
+                        servers[name] = self.server_model(
+                            spawner, include_state=include_state
+                        )
         return model
 
     def group_model(self, group):

jupyterhub/apihandlers/groups.py

@@ -38,7 +38,7 @@ 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:
             groups = groups.filter(orm.Group.name.in_(scope_filter))
         data = [self.group_model(g) for g in groups]

jupyterhub/handlers/base.py

@@ -427,16 +427,17 @@ class BaseHandler(RequestHandler):
                 if user and isinstance(user, User):
                     user = await self.refresh_auth(user)
                 self._jupyterhub_user = user
+                self._parse_scopes()
             except Exception:
                 # 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 self.current_user:  # Todo: Deal with oauth tokens
+                self.raw_scopes = roles.get_subscopes(*self.current_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)

jupyterhub/scopes.py

@@ -143,6 +143,7 @@ def needs_scope(*scopes):
     def scope_decorator(func):
         @functools.wraps(func)
         def _auth_func(self, *args, **kwargs):
+            self.parse_scopes()  # Todo: Check most practical locations for parsing scopes
             sig = inspect.signature(func)
             bound_sig = sig.bind(self, *args, **kwargs)
             bound_sig.apply_defaults()

jupyterhub/tests/test_api.py

@@ -260,7 +260,9 @@ async def test_get_self(app):
     db = app.db
 
     # basic get self
-    r = await api_request(app, 'user')
+    r = await api_request(
+        app, 'user', headers=auth_header(db, 'user')
+    )  # Todo: check after dealing with oauth
     r.raise_for_status()
     assert r.json()['kind'] == 'user'
 

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', ('maeby',), False),
+        (['users!user=gob'], 'other_thing', ('gob',), True),
+        (['users!user=gob'], 'other_thing', ('maeby',), True),
     ],
 )
 def test_scope_method_access(scopes, method, arguments, is_allowed):
@@ -403,7 +404,8 @@ 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):