Refactored scope module. Implemented filter in *ListApiHandlers

jupyterhub/apihandlers/groups.py

@@ -6,8 +6,7 @@ import json
 from tornado import web
 
 from .. import orm
-from ..utils import admin_only
+from ..scopes import needs_scope
-from ..utils import needs_scope
 from .base import APIHandler
 
 

jupyterhub/apihandlers/hub.py

@@ -8,8 +8,7 @@ from tornado import web
 from tornado.ioloop import IOLoop
 
 from .._version import __version__
-from ..utils import admin_only
+from ..scopes import needs_scope
-from ..utils import needs_scope
 from .base import APIHandler
 
 

jupyterhub/apihandlers/proxy.py

@@ -5,8 +5,7 @@ import json
 
 from tornado import web
 
-from ..utils import admin_only
+from ..scopes import needs_scope
-from ..utils import needs_scope
 from .base import APIHandler
 
 

jupyterhub/apihandlers/services.py

@@ -9,8 +9,7 @@ import json
 from tornado import web
 
 from .. import orm
-from ..utils import admin_only
+from ..scopes import needs_scope
-from ..utils import needs_scope
 from .base import APIHandler
 
 

jupyterhub/apihandlers/users.py

@@ -14,13 +14,12 @@ from tornado import web
 from tornado.iostream import StreamClosedError
 
 from .. import orm
-from .. import roles
 from ..roles import update_roles
+from ..scopes import needs_scope
 from ..user import User
 from ..utils import isoformat
 from ..utils import iterate_until
 from ..utils import maybe_future
-from ..utils import needs_scope
 from ..utils import url_path_join
 from .base import APIHandler
 

jupyterhub/handlers/pages.py

@@ -17,7 +17,7 @@ from .. import orm
 from ..metrics import SERVER_POLL_DURATION_SECONDS
 from ..metrics import ServerPollStatus
 from ..pagination import Pagination
-from ..utils import admin_only
+from ..scopes import needs_scope
 from ..utils import maybe_future
 from ..utils import url_path_join
 from .base import BaseHandler
@@ -455,7 +455,7 @@ class AdminHandler(BaseHandler):
     """Render the admin page."""
 
     @web.authenticated
-    @admin_only
+    @needs_scope('admin:users')
     async def get(self):
         page, per_page, offset = Pagination(config=self.config).get_page_args(self)
 

jupyterhub/scopes.py

@@ -0,0 +1,216 @@
+import functools
+import inspect
+from enum import Enum
+
+from tornado import web
+from tornado.log import app_log
+
+from . import orm
+
+
+class Scope(Enum):
+    ALL = True
+
+
+def get_user_scopes(name):
+    """
+    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
+    users
+    users:name
+    users:groups
+    users:activity
+    users:servers
+    users:tokens
+
+    """
+    scope_list = [
+        'users',
+        'users:name',
+        'users:groups',
+        'users:activity',
+        'users:servers',
+        'users:tokens',
+    ]
+    scope_list.extend(['read:' + scope for scope in scope_list])
+    return {"{}!user={}".format(scope, name) for scope in scope_list}
+
+
+def _needs_scope_expansion(filter_, filter_value, sub_scope):
+    """
+    Check if there is a requirements to expand the `group` scope to individual `user` scopes.
+    Assumptions:
+    filter_ != Scope.ALL
+    """
+    if not (filter_ == 'user' and 'group' in sub_scope):
+        return False
+    if 'user' in sub_scope:
+        return filter_value not in sub_scope['user']
+    else:
+        return True
+
+
+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 such user found')
+    group_names = {group.name for group in user.groups}
+    return bool(set(scope_group_names) & group_names)
+
+
+def _flatten_groups(groups):
+    user_set = set()
+    for group in groups:
+        user_set |= {
+            user.name for user in group.users
+        }  # todo: I think this could be one query, no for loop
+    return user_set
+
+
+def get_orm_class(kind):
+    class_dict = {
+        'users': orm.User,
+        'services': orm.Service,
+        'tokens': orm.APIToken,
+        'groups': orm.Group,
+    }
+    if kind not in class_dict:
+        raise ValueError(
+            "Kind must be one of %s, not %s" % (", ".join(class_dict), kind)
+        )
+    return class_dict[kind]
+
+
+def _get_scope_filter(db, req_scope, sub_scope):
+    # Rough draft
+    scope_translator = {
+        'read:users': 'users',
+        'read:services': 'services',
+        'read:groups': 'groups',
+    }
+    if req_scope not in scope_translator:
+        raise AttributeError("Scope not found; scope filter not constructed")
+    kind = scope_translator[req_scope]
+    Class = get_orm_class(kind)
+    sub_scope_values = next(iter(sub_scope.values()))
+    query = db.query(Class).filter(Class.name.in_(sub_scope_values))
+    scope_filter = [entry.name for entry in query.all()]
+    if 'group' in sub_scope and kind == 'users':
+        groups = db.query(orm.Group).filter(orm.Group.name.in_(sub_scope['group']))
+        scope_filter += _flatten_groups(groups)
+    return set(scope_filter)
+
+
+def _check_scope(api_handler, req_scope, scopes, **kwargs):
+    # Parse user name and server name together
+    if 'user' in kwargs and 'server' in kwargs:
+        kwargs['server'] = "{}/{}".format(kwargs['user'], kwargs['server'])
+    if req_scope not in scopes:
+        return False
+    if scopes[req_scope] == Scope.ALL:
+        return True
+    # Apply filters
+    sub_scope = scopes[req_scope]
+    if 'scope_filter' in kwargs:
+        scope_filter = _get_scope_filter(api_handler.db, req_scope, sub_scope)
+        return scope_filter
+    else:
+        # Interface change: Now can have multiple filters
+        for (filter_, filter_value) in kwargs.items():
+            if filter_ in sub_scope and filter_value in sub_scope[filter_]:
+                return True
+            if _needs_scope_expansion(filter_, filter_value, sub_scope):
+                group_names = sub_scope['group']
+                if _check_user_in_expanded_scope(
+                    api_handler, filter_value, group_names
+                ):
+                    return True
+        return False
+
+
+def _parse_scopes(scope_list):
+    """
+    Parses scopes and filters in something akin to JSON style
+
+    For instance, scope list ["users", "groups!group=foo", "users:servers!server=bar", "users:servers!server=baz"]
+    would lead to scope model
+    {
+       "users":scope.ALL,
+       "users:admin":{
+          "user":[
+             "alice"
+          ]
+       },
+       "users:servers":{
+          "server":[
+             "bar",
+             "baz"
+          ]
+       }
+    }
+    """
+    parsed_scopes = {}
+    for scope in scope_list:
+        base_scope, _, filter_ = scope.partition('!')
+        if not filter_:
+            parsed_scopes[base_scope] = Scope.ALL
+        elif base_scope not in parsed_scopes:
+            parsed_scopes[base_scope] = {}
+        if parsed_scopes[base_scope] != Scope.ALL:
+            key, _, val = filter_.partition('=')
+            if key not in parsed_scopes[base_scope]:
+                parsed_scopes[base_scope][key] = []
+            parsed_scopes[base_scope][key].append(val)
+    return parsed_scopes
+
+
+def needs_scope(scope):
+    """Decorator to restrict access to users or services with the required scope"""
+
+    def scope_decorator(func):
+        @functools.wraps(func)
+        def _auth_func(self, *args, **kwargs):
+            sig = inspect.signature(func)
+            bound_sig = sig.bind(self, *args, **kwargs)
+            bound_sig.apply_defaults()
+            s_kwargs = {}
+            for resource in {'user', 'server', 'group', 'service'}:
+                resource_name = resource + '_name'
+                if resource_name in bound_sig.arguments:
+                    resource_value = bound_sig.arguments[resource_name]
+                    s_kwargs[resource] = resource_value
+            if 'scope_filter' in bound_sig.arguments:
+                s_kwargs['scope_filter'] = None
+            if 'all' in self.scopes and self.current_user:
+                # todo: What if no user is found? See test_api/test_referer_check
+                self.scopes |= get_user_scopes(self.current_user.name)
+            parsed_scopes = _parse_scopes(self.scopes)
+            scope_filter = _check_scope(self, scope, parsed_scopes, **s_kwargs)
+            if (
+                scope_filter
+            ):  # Todo: This checks if True or set of resource names. Not very nice yet
+                if isinstance(scope_filter, set):
+                    kwargs['scope_filter'] = scope_filter
+                return func(self, *args, **kwargs)
+            else:
+                # catching attr error occurring for older_requirements test
+                # could be done more ellegantly?
+                try:
+                    request_path = self.request.path
+                except AttributeError:
+                    request_path = 'the requested API'
+                app_log.warning(
+                    "Not authorizing access to {}. Requires scope {}, not derived from scopes {}".format(
+                        request_path, scope, ", ".join(self.scopes)
+                    )
+                )
+                raise web.HTTPError(
+                    403,
+                    "Action is not authorized with current scopes; requires {}".format(
+                        scope
+                    ),
+                )
+
+        return _auth_func
+
+    return scope_decorator

jupyterhub/tests/test_api.py

@@ -14,7 +14,6 @@ from pytest import mark
 
 import jupyterhub
 from .. import orm
-from .. import roles
 from ..objects import Server
 from ..utils import url_path_join as ujoin
 from ..utils import utcnow
@@ -167,21 +166,27 @@ TIMESTAMP = normalize_timestamp(datetime.now().isoformat() + 'Z')
 
 @mark.user
 @mark.role
-async def test_get_users(app):
+async def test_get_users(app):  # todo: Sync with scope tests
     db = app.db
     r = await api_request(app, 'users', headers=auth_header(db, 'admin'))
     assert r.status_code == 200
 
     users = sorted(r.json(), key=lambda d: d['name'])
     users = [normalize_user(u) for u in users]
+    user_model = {
+        'name': 'user',
+        'admin': False,
+        'roles': ['user'],
+        'last_activity': None,
+    }
     assert users == [
         fill_user({'name': 'admin', 'admin': True, 'roles': ['admin']}),
-        fill_user(
+        fill_user(user_model),
-            {'name': 'user', 'admin': False, 'roles': ['user'], 'last_activity': None}
-        ),
     ]
     r = await api_request(app, 'users', headers=auth_header(db, 'user'))
-    assert r.status_code == 403
+    assert r.status_code == 200
+    r_user_model = json.loads(r.text)[0]
+    assert r_user_model['name'] == user_model['name']
 
 
 @mark.user

jupyterhub/tests/test_scopes.py

@@ -3,17 +3,16 @@ import json
 from unittest import mock
 
 import pytest
-import tornado
 from pytest import mark
 from tornado import web
 from tornado.httputil import HTTPServerRequest
 
 from .. import orm
 from .. import roles
-from ..utils import check_scope
+from ..scopes import _check_scope
-from ..utils import needs_scope
+from ..scopes import _parse_scopes
-from ..utils import parse_scopes
+from ..scopes import needs_scope
-from ..utils import Scope
+from ..scopes import Scope
 from .utils import add_user
 from .utils import api_request
 from .utils import auth_header
@@ -27,7 +26,7 @@ def test_scope_constructor():
         'read:users!user={}'.format(user1),
         'read:users!user={}'.format(user2),
     ]
-    parsed_scopes = parse_scopes(scope_list)
+    parsed_scopes = _parse_scopes(scope_list)
 
     assert 'read:users' in parsed_scopes
     assert parsed_scopes['users']
@@ -36,25 +35,25 @@ def test_scope_constructor():
 
 def test_scope_precendence():
     scope_list = ['read:users!user=maeby', 'read:users']
-    parsed_scopes = parse_scopes(scope_list)
+    parsed_scopes = _parse_scopes(scope_list)
     assert parsed_scopes['read:users'] == Scope.ALL
 
 
 def test_scope_check_present():
     handler = None
     scope_list = ['read:users']
-    parsed_scopes = parse_scopes(scope_list)
+    parsed_scopes = _parse_scopes(scope_list)
-    assert check_scope(handler, 'read:users', parsed_scopes)
+    assert _check_scope(handler, 'read:users', parsed_scopes)
-    assert check_scope(handler, 'read:users', parsed_scopes, user='maeby')
+    assert _check_scope(handler, 'read:users', parsed_scopes, user='maeby')
 
 
 def test_scope_check_not_present():
     handler = None
     scope_list = ['read:users!user=maeby']
-    parsed_scopes = parse_scopes(scope_list)
+    parsed_scopes = _parse_scopes(scope_list)
-    assert not check_scope(handler, 'read:users', parsed_scopes)
+    assert not _check_scope(handler, 'read:users', parsed_scopes)
-    assert not check_scope(handler, 'read:users', parsed_scopes, user='gob')
+    assert not _check_scope(handler, 'read:users', parsed_scopes, user='gob')
-    assert not check_scope(
+    assert not _check_scope(
         handler, 'read:users', parsed_scopes, user='gob', server='server'
     )
 
@@ -62,17 +61,17 @@ def test_scope_check_not_present():
 def test_scope_filters():
     handler = None
     scope_list = ['read:users', 'read:users!group=bluths', 'read:users!user=maeby']
-    parsed_scopes = parse_scopes(scope_list)
+    parsed_scopes = _parse_scopes(scope_list)
-    assert check_scope(handler, 'read:users', parsed_scopes, group='bluth')
+    assert _check_scope(handler, 'read:users', parsed_scopes, group='bluth')
-    assert check_scope(handler, 'read:users', parsed_scopes, user='maeby')
+    assert _check_scope(handler, 'read:users', parsed_scopes, user='maeby')
 
 
 def test_scope_multiple_filters():
     handler = None
-    assert check_scope(
+    assert _check_scope(
         handler,
         'read:users',
-        parse_scopes(['read:users!user=george_michael']),
+        _parse_scopes(['read:users!user=george_michael']),
         user='george_michael',
         group='bluths',
     )
@@ -81,8 +80,8 @@ def test_scope_multiple_filters():
 def test_scope_parse_server_name():
     handler = None
     scope_list = ['users:servers!server=maeby/server1', 'read:users!user=maeby']
-    parsed_scopes = parse_scopes(scope_list)
+    parsed_scopes = _parse_scopes(scope_list)
-    assert check_scope(
+    assert _check_scope(
         handler, 'users:servers', parsed_scopes, user='maeby', server='server1'
     )
 

jupyterhub/utils.py

@@ -4,7 +4,6 @@
 import asyncio
 import concurrent.futures
 import errno
-import functools
 import hashlib
 import inspect
 import os
@@ -18,7 +17,6 @@ import warnings
 from binascii import b2a_hex
 from datetime import datetime
 from datetime import timezone
-from enum import Enum
 from hmac import compare_digest
 from operator import itemgetter
 
@@ -30,8 +28,6 @@ from tornado.httpclient import HTTPError
 from tornado.log import app_log
 from tornado.platform.asyncio import to_asyncio_future
 
-from . import orm  # todo: only necessary for scopes, move later
-
 
 def random_port():
     """Get a single random port."""
@@ -283,8 +279,13 @@ def authenticated_403(self):
 
 @auth_decorator
 def admin_only(self):
-    """Decorator for restricting access to admin users"""
+    """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)
 
@@ -297,214 +298,6 @@ def metrics_authentication(self):
         raise web.HTTPError(403)
 
 
-# Todo: Move all scope-related methods to scope module
-class Scope(Enum):
-    ALL = True
-
-
-def get_user_scopes(name):
-    """
-    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
-    users
-    users:name
-    users:groups
-    users:activity
-    users:servers
-    users:tokens
-
-    """
-    scope_list = [
-        'users',
-        'users:name',
-        'users:groups',
-        'users:activity',
-        'users:servers',
-        'users:tokens',
-    ]
-    scope_list.extend(['read:' + scope for scope in scope_list])
-    return {"{}!user={}".format(scope, name) for scope in scope_list}
-
-
-def needs_scope_expansion(filter_, filter_value, sub_scope):
-    """
-    Check if there is a requirements to expand the `group` scope to individual `user` scopes.
-    Assumptions:
-    filter_ != Scope.ALL
-    """
-    if not (filter_ == 'user' and 'group' in sub_scope):
-        return False
-    if 'user' in sub_scope:
-        return filter_value not in sub_scope['user']
-    else:
-        return True
-
-
-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 such user found')
-    group_names = {group.name for group in user.groups}
-    return bool(set(scope_group_names) & group_names)
-
-
-def _flatten_groups(groups):
-    user_set = set()
-    for group in groups:
-        user_set |= {
-            user.name for user in group.users
-        }  # todo: I think this could be one query, no for loop
-    return user_set
-
-
-def get_orm_class(kind):
-    class_dict = {
-        'users': orm.User,
-        'services': orm.Service,
-        'tokens': orm.APIToken,
-        'groups': orm.Group,
-    }
-    if kind not in class_dict:
-        raise ValueError(
-            "Kind must be one of %s, not %s" % (", ".join(class_dict), kind)
-        )
-    return class_dict[kind]
-
-
-def _get_scope_filter(db, req_scope, sub_scope):
-    # Rough draft
-    scope_translator = {
-        'read:users': 'users',
-        'read:services': 'services',
-        'read:groups': 'groups',
-    }
-    if req_scope not in scope_translator:
-        raise AttributeError("Scope not found; scope filter not constructed")
-    kind = scope_translator[req_scope]
-    Class = get_orm_class(kind)
-    sub_scope_values = next(iter(sub_scope.values()))
-    query = db.query(Class).filter(Class.name.in_(sub_scope_values))
-    scope_filter = [entry.name for entry in query.all()]
-    if 'group' in sub_scope and kind == 'users':
-        groups = db.query(orm.Group).filter(orm.Group.name.in_(sub_scope['group']))
-        scope_filter += _flatten_groups(groups)
-    return set(scope_filter)
-
-
-def check_scope(api_handler, req_scope, scopes, **kwargs):
-    # Parse user name and server name together
-    if 'user' in kwargs and 'server' in kwargs:
-        kwargs['server'] = "{}/{}".format(kwargs['user'], kwargs['server'])
-    if req_scope not in scopes:
-        return False
-    if scopes[req_scope] == Scope.ALL:
-        return True
-    # Apply filters
-    sub_scope = scopes[req_scope]
-    if 'scope_filter' in kwargs:
-        scope_filter = _get_scope_filter(api_handler.db, req_scope, sub_scope)
-        return scope_filter
-    else:
-        # Interface change: Now can have multiple filters
-        for (filter_, filter_value) in kwargs.items():
-            if filter_ in sub_scope and filter_value in sub_scope[filter_]:
-                return True
-            if needs_scope_expansion(filter_, filter_value, sub_scope):
-                group_names = sub_scope['group']
-                if check_user_in_expanded_scope(api_handler, filter_value, group_names):
-                    return True
-        return False
-
-
-# Todo: make some methods private
-
-
-def parse_scopes(scope_list):
-    """
-    Parses scopes and filters in something akin to JSON style
-
-    For instance, scope list ["users", "groups!group=foo", "users:servers!server=bar", "users:servers!server=baz"]
-    would lead to scope model
-    {
-       "users":scope.ALL,
-       "users:admin":{
-          "user":[
-             "alice"
-          ]
-       },
-       "users:servers":{
-          "server":[
-             "bar",
-             "baz"
-          ]
-       }
-    }
-    """
-    parsed_scopes = {}
-    for scope in scope_list:
-        base_scope, _, filter_ = scope.partition('!')
-        if not filter_:
-            parsed_scopes[base_scope] = Scope.ALL
-        elif base_scope not in parsed_scopes:
-            parsed_scopes[base_scope] = {}
-        if parsed_scopes[base_scope] != Scope.ALL:
-            key, _, val = filter_.partition('=')
-            if key not in parsed_scopes[base_scope]:
-                parsed_scopes[base_scope][key] = []
-            parsed_scopes[base_scope][key].append(val)
-    return parsed_scopes
-
-
-def needs_scope(scope):
-    """Decorator to restrict access to users or services with the required scope"""
-
-    def scope_decorator(func):
-        @functools.wraps(func)
-        def _auth_func(self, *args, **kwargs):
-            sig = inspect.signature(func)
-            bound_sig = sig.bind(self, *args, **kwargs)
-            bound_sig.apply_defaults()
-            s_kwargs = {}
-            for resource in {'user', 'server', 'group', 'service'}:
-                resource_name = resource + '_name'
-                if resource_name in bound_sig.arguments:
-                    resource_value = bound_sig.arguments[resource_name]
-                    s_kwargs[resource] = resource_value
-            if 'scope_filter' in bound_sig.arguments:
-                s_kwargs['scope_filter'] = None
-            if 'all' in self.scopes and self.current_user:
-                # todo: What if no user is found? See test_api/test_referer_check
-                self.scopes |= get_user_scopes(self.current_user.name)
-            parsed_scopes = parse_scopes(self.scopes)
-            scope_filter = check_scope(self, scope, parsed_scopes, **s_kwargs)
-            if scope_filter:
-                if isinstance(scope_filter, set):
-                    kwargs['scope_filter'] = scope_filter
-                return func(self, *args, **kwargs)
-            else:
-                # catching attr error occurring for older_requirements test
-                # could be done more ellegantly?
-                try:
-                    request_path = self.request.path
-                except AttributeError:
-                    request_path = 'the requested API'
-                app_log.warning(
-                    "Not authorizing access to {}. Requires scope {}, not derived from scopes {}".format(
-                        request_path, scope, ", ".join(self.scopes)
-                    )
-                )
-                raise web.HTTPError(
-                    403,
-                    "Action is not authorized with current scopes; requires {}".format(
-                        scope
-                    ),
-                )
-
-        return _auth_func
-
-    return scope_decorator
-
-
 # Token utilities