Added tests and removed model flags

jupyterhub/apihandlers/base.py

@@ -88,9 +88,12 @@ class APIHandler(BaseHandler):
             if sub_scope == scopes.Scope.ALL:
                 return True
             else:
-                found_resource = orm_resource.name in sub_scope[kind]
+                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':
+                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']

jupyterhub/apihandlers/users.py

@@ -188,11 +188,13 @@ class UserAPIHandler(APIHandler):
     @needs_scope(
         'read:users',
         'read:users:name',
-        'reda:users:servers',
+        'read:users:servers',
         'read:users:groups',
         'read:users:activity',
     )
-    async def get(self, user_name):
+    async def get(
+        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
@@ -263,7 +265,7 @@ class UserAPIHandler(APIHandler):
 
         self.set_status(204)
 
-    @needs_scope('admin:users')
+    @needs_scope('admin:users')  # Todo: Change to `users`?
     async def patch(self, user_name):
         user = self.find_user(user_name)
         if user is None:
@@ -326,6 +328,7 @@ class UserTokenListAPIHandler(APIHandler):
             oauth_tokens.append(self.token_model(token))
         self.write(json.dumps({'api_tokens': api_tokens, 'oauth_tokens': oauth_tokens}))
 
+    # Todo: Set to @needs_scope('users:tokens')
     async def post(self, user_name):
         body = self.get_json_body() or {}
         if not isinstance(body, dict):
@@ -765,7 +768,7 @@ class ActivityAPIHandler(APIHandler):
             )
         return servers
 
-    @needs_scope('users')
+    @needs_scope('users:activity')
     def post(self, user_name):
         user = self.find_user(user_name)
         if user is None:

jupyterhub/app.py

@@ -1377,7 +1377,7 @@ class JupyterHub(Application):
         Can be a Unicode string (e.g. '/hub/home') or a callable based on the handler object:
 
         ::
-        
+
             def default_url_fn(handler):
                 user = handler.current_user
                 if user and user.admin:

jupyterhub/roles.py

@@ -90,8 +90,8 @@ def _get_scope_hierarchy():
             'read:users:servers',
         ],
         'users:tokens': ['read:users:tokens'],
-        'admin:users': ['admin:users:auth_state', 'admin:users:server_state'],
-        'admin:users:servers': None,
+        'admin:users': ['admin:users:auth_state'],
+        'admin:users:servers': ['admin:users:server_state'],
         'groups': ['read:groups'],
         'admin:groups': None,
         'read:services': None,
@@ -236,7 +236,7 @@ def assign_default_roles(db, entity):
     # tokens can have only 'token' role as default
     # assign the default only for tokens
     if isinstance(entity, orm.APIToken):
-        if not entity.roles and entity.user is not None:
+        if not entity.roles and (entity.user or entity.service) is not None:
             default_token_role.tokens.append(entity)
         db.commit()
     # users and services can have 'user' or 'admin' roles as default

jupyterhub/scopes.py

@@ -64,7 +64,7 @@ def _check_user_in_expanded_scope(handler, user_name, scope_group_names):
 
 def _check_scope(api_handler, req_scope, **kwargs):
     """Check if scopes satisfy requirements
-    Returns True for (restricted) access, False for refused access
+    Returns True for (potentially restricted) access, False for refused access
     """
     # Parse user name and server name together
     try:
@@ -74,24 +74,23 @@ def _check_scope(api_handler, req_scope, **kwargs):
     if 'user' in kwargs and 'server' in kwargs:
         kwargs['server'] = "{}/{}".format(kwargs['user'], kwargs['server'])
     if req_scope not in api_handler.parsed_scopes:
-        app_log.debug("No scopes present to access %s" % api_name)
+        app_log.debug("No access to %s via %s", api_name, req_scope)
         return False
     if api_handler.parsed_scopes[req_scope] == Scope.ALL:
-        app_log.debug("Unrestricted access to %s call", api_name)
+        app_log.debug("Unrestricted access to %s via %s", api_name, req_scope)
         return True
     # Apply filters
     sub_scope = api_handler.parsed_scopes[req_scope]
     if not kwargs:
         app_log.debug(
-            "Client has restricted access to %s. Internal filtering may apply"
-            % api_name
+            "Client has restricted access to %s via %s. Internal filtering may apply",
+            api_name,
+            req_scope,
         )
         return True
     for (filter_, filter_value) in kwargs.items():
         if filter_ in sub_scope and filter_value in sub_scope[filter_]:
-            app_log.debug(
-                "Restricted client access supported by endpoint %s" % api_name
-            )
+            app_log.debug("Argument-based access to %s via %s", api_name, req_scope)
             return True
         if _needs_scope_expansion(filter_, filter_value, sub_scope):
             group_names = sub_scope['group']
@@ -160,27 +159,26 @@ def needs_scope(*scopes):
                 if resource_name in bound_sig.arguments:
                     resource_value = bound_sig.arguments[resource_name]
                     s_kwargs[resource] = resource_value
-            has_access = False
             for scope in scopes:
-                has_access |= _check_scope(self, scope, **s_kwargs)
-            if has_access:
-                return func(self, *args, **kwargs)
-            else:
-                try:
-                    end_point = self.request.path
-                except AttributeError:
-                    end_point = self.__name__
-                app_log.warning(
-                    "Not authorizing access to {}. Requires any of [{}], not derived from scopes [{}]".format(
-                        end_point, ", ".join(scopes), ", ".join(self.raw_scopes)
-                    )
-                )
-                raise web.HTTPError(
-                    403,
-                    "Action is not authorized with current scopes; requires any of [{}]".format(
-                        ", ".join(scopes)
-                    ),
+                app_log.debug("Checking access via scope %s", scope)
+                has_access = _check_scope(self, scope, **s_kwargs)
+                if has_access:
+                    return func(self, *args, **kwargs)
+            try:
+                end_point = self.request.path
+            except AttributeError:
+                end_point = self.__name__
+            app_log.warning(
+                "Not authorizing access to {}. Requires any of [{}], not derived from scopes [{}]".format(
+                    end_point, ", ".join(scopes), ", ".join(self.raw_scopes)
                 )
+            )
+            raise web.HTTPError(
+                403,
+                "Action is not authorized with current scopes; requires any of [{}]".format(
+                    ", ".join(scopes)
+                ),
+            )
 
         return _auth_func
 

jupyterhub/tests/test_roles.py

@@ -340,6 +340,12 @@ async def test_load_roles_tokens(tmpdir, request):
             'scopes': ['users:servers', 'admin:servers'],
             'tokens': ['another-secret-token'],
         },
+        {
+            'name': 'admin',
+            'description': 'Admin access',
+            'scopes': ['a lot'],
+            'users': ['admin'],
+        },
     ]
     kwargs = {
         'load_roles': roles_to_load,

jupyterhub/tests/test_scopes.py

@@ -540,6 +540,7 @@ async def test_metascope_self_expansion(app, kind, has_user_scopes):
     assert bool(token_scopes) == has_user_scopes
     app.db.delete(orm_obj)
     app.db.delete(test_role)
+    app.db.commit()
 
 
 async def test_metascope_all_expansion(app):
@@ -557,3 +558,77 @@ async def test_metascope_all_expansion(app):
     app.db.commit()
     token_scope_set = get_scopes_for(token)
     assert not token_scope_set
+
+
+@mark.parametrize(
+    "scopes, can_stop ,num_servers, keys_in, keys_out",
+    [
+        (['read:users:servers!user=almond'], False, 2, {'name'}, {'state'}),
+        (
+            ['admin:users:server_state', 'read:users:servers'],
+            True,
+            2,
+            {'name', 'state'},
+            set(),
+        ),
+        (['users:servers', 'read:users:name'], True, 0, set(), set()),
+        (
+            [
+                'read:users:name!user=almond',
+                'read:users:servers!server=almond/bianca',  # fixme: server-scope not working yet
+                'admin:users:server_state!server=almond/bianca',
+            ],
+            False,
+            1,
+            {'name', 'state'},
+            set(),
+        ),
+    ],
+)
+async def test_server_state_access(
+    app, 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},
+    ):
+        ## 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 gives access to one server, and the correct server.
+        username = 'almond'
+        user = add_user(app.db, app, name=username)
+
+        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()
+            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.commit()