Resolve merge conflicts

ci/init-db.sh

@@ -20,7 +20,7 @@ fi
 
 # Configure a set of databases in the database server for upgrade tests
 set -x
-for SUFFIX in '' _upgrade_072 _upgrade_081 _upgrade_094; do
+for SUFFIX in '' _upgrade_100 _upgrade_122 _upgrade_130; do
     $SQL_CLIENT "DROP DATABASE jupyterhub${SUFFIX};" 2>/dev/null || true
     $SQL_CLIENT "CREATE DATABASE jupyterhub${SUFFIX} ${EXTRA_CREATE_DATABASE_ARGS:-};"
 done

docs/source/getting-started/authenticators-users-basics.md

@@ -91,6 +91,7 @@ JupyterHub's [OAuthenticator][] currently supports the following
 popular services:
 
 - Auth0
+- Azure AD
 - Bitbucket
 - CILogon
 - GitHub

docs/source/installation-guide-hard.rst

@@ -3,4 +3,4 @@
 JupyterHub the hard way
 =======================
 
-This guide has moved to https://github.com/manics/jupyterhub-the-hard-way/blob/jupyterhub-alternative-doc/docs/installation-guide-hard.md
+This guide has moved to https://github.com/jupyterhub/jupyterhub-the-hard-way/blob/master/docs/installation-guide-hard.md

examples/service-fastapi/Dockerfile

@@ -0,0 +1,13 @@
+FROM jupyterhub/jupyterhub
+
+# Create test user (PAM auth) and install single-user Jupyter 
+RUN useradd testuser --create-home --shell /bin/bash
+RUN echo 'testuser:passwd' | chpasswd
+RUN pip install jupyter 
+
+COPY app ./app
+COPY jupyterhub_config.py .
+COPY requirements.txt /tmp/requirements.txt
+RUN pip install -r /tmp/requirements.txt
+
+CMD ["jupyterhub", "--ip", "0.0.0.0"]

examples/service-fastapi/README.md

@@ -0,0 +1,107 @@
+# Fastapi
+
+[FastAPI](https://fastapi.tiangolo.com/) is a popular new web framework attractive for its type hinting, async support, automatic doc generation (Swagger), and more. Their [Feature highlights](https://fastapi.tiangolo.com/features/) sum it up nicely.
+
+# Swagger UI with OAuth demo
+
+![Fastapi Service Example](./fastapi_example.gif)
+
+# Try it out locally
+
+1. Install `fastapi` and other dependencies, then launch Jupyterhub
+
+```
+pip install -r requirements.txt
+jupyterhub --ip=127.0.0.1
+```
+
+2. Visit http://127.0.0.1:8000/services/fastapi or http://127.0.0.1:8000/services/fastapi/docs
+
+3. Try interacting programmatically. If you create a new token in your control panel or pull out the `JUPYTERHUB_API_TOKEN` in the single user environment, you can skip the third step here.
+
+```
+$ curl -X GET http://127.0.0.1:8000/services/fastapi/
+{"Hello":"World"}
+
+$ curl -X GET http://127.0.0.1:8000/services/fastapi/me
+{"detail":"Must login with token parameter, cookie, or header"}
+
+$ curl -X POST http://127.0.0.1:8000/hub/api/authorizations/token \
+       -d '{"username": "myname", "password": "mypasswd!"}' \
+       | jq '.token'
+"3fee13ce6d2845da9bd5f2c2170d3428"
+
+$ curl -X GET http://127.0.0.1:8000/services/fastapi/me \
+       -H "Authorization: Bearer 3fee13ce6d2845da9bd5f2c2170d3428" \
+       | jq .
+{
+  "name": "myname",
+  "admin": false,
+  "groups": [],
+  "server": null,
+  "pending": null,
+  "last_activity": "2021-04-07T18:05:11.587638+00:00",
+  "servers": null
+}
+```
+
+# Try it out in Docker
+
+1. Build and run the Docker image locally
+
+```bash
+sudo docker build . -t service-fastapi
+sudo docker run -it -p 8000:8000 service-fastapi
+```
+
+2. Visit http://127.0.0.1:8000/services/fastapi/docs. When going through the OAuth flow or getting a token from the control panel, you can log in with `testuser` / `passwd`.
+
+# PUBLIC_HOST
+
+If you are running your service behind a proxy, or on a Docker / Kubernetes infrastructure, you might run into an error during OAuth that says `Mismatching redirect URI`. In the Jupterhub logs, there will be a warning along the lines of: `[W 2021-04-06 23:40:06.707 JupyterHub provider:498] Redirect uri https://jupyterhub.my.cloud/services/fastapi/oauth_callback != /services/fastapi/oauth_callback`. This happens because Swagger UI adds the request host, as seen in the browser, to the Authorization URL.
+
+To solve that problem, the `oauth_redirect_uri` value in the service initialization needs to match what Swagger will auto-generate and what the service will use when POST'ing to `/oauth2/token`. In this example, setting the `PUBLIC_HOST` environment variable to your public-facing Hub domain (e.g. `https://jupyterhub.my.cloud`) should make it work.
+
+# Notes on security.py
+
+FastAPI has a concept of a [dependency injection](https://fastapi.tiangolo.com/tutorial/dependencies) using a `Depends` object (and a subclass `Security`) that is automatically instantiated/executed when it is a parameter for your endpoint routes. You can utilize a `Depends` object for re-useable common parameters or authentication mechanisms like the [`get_user`](https://fastapi.tiangolo.com/tutorial/security/get-current-user) pattern.
+
+JupyterHub OAuth has three ways to authenticate: a `token` url parameter; a `Authorization: Bearer <token>` header; and a (deprecated) `jupyterhub-services` cookie. FastAPI has helper functions that let us create `Security` (dependency injection) objects for each of those. When you need to allow multiple / optional authentication dependencies (`Security` objects), then you can use the argument `auto_error=False` and it will return `None` instead of raising an `HTTPException`.
+
+Endpoints that need authentication (`/me` and `/debug` in this example) can leverage the `get_user` pattern and effectively pull the user model from the Hub API when a request has authenticated with cookie / token / header all using the simple syntax,
+
+```python
+from .security import get_current_user
+from .models import User
+
+@router.get("/new_endpoint")
+async def new_endpoint(user: User = Depends(get_current_user)):
+    "Function that needs to work with an authenticated user"
+    return {"Hello": user.name}
+```
+
+# Notes on client.py
+
+FastAPI is designed to be an asynchronous web server, so the interactions with the Hub API should be made asynchronously as well. Instead of using `requests` to get user information from a token/cookie, this example uses [`httpx`](https://www.python-httpx.org/). `client.py` defines a small function that creates a `Client` (equivalent of `requests.Session`) with the Hub API url as it's `base_url` and adding the `JUPYTERHUB_API_TOKEN` to every header.
+
+Consider this a very minimal alternative to using `jupyterhub.services.auth.HubOAuth`
+
+```python
+# client.py
+import os
+
+def get_client():
+    base_url = os.environ["JUPYTERHUB_API_URL"]
+    token = os.environ["JUPYTERHUB_API_TOKEN"]
+    headers = {"Authorization": "Bearer %s" % token}
+    return httpx.AsyncClient(base_url=base_url, headers=headers)
+```
+
+```python
+# other modules
+from .client import get_client
+
+async with get_client() as client:
+    resp = await client.get('/endpoint')
+    ...
+```

examples/service-fastapi/app/__init__.py

@@ -0,0 +1 @@
+from .app import app

examples/service-fastapi/app/app.py

@@ -0,0 +1,25 @@
+import os
+
+from fastapi import FastAPI
+
+from .service import router
+
+### When managed by Jupyterhub, the actual endpoints
+### will be served out prefixed by /services/:name.
+### One way to handle this with FastAPI is to use an APIRouter.
+### All routes are defined in service.py
+
+app = FastAPI(
+    title="Example FastAPI Service",
+    version="0.1",
+    ### Serve out Swagger from the service prefix (<hub>/services/:name/docs)
+    openapi_url=router.prefix + "/openapi.json",
+    docs_url=router.prefix + "/docs",
+    redoc_url=router.prefix + "/redoc",
+    ### Add our service client id to the /docs Authorize form automatically
+    swagger_ui_init_oauth={"clientId": os.environ["JUPYTERHUB_CLIENT_ID"]},
+    ### Default /docs/oauth2 redirect will cause Hub
+    ### to raise oauth2 redirect uri mismatch errors
+    swagger_ui_oauth2_redirect_url=os.environ["JUPYTERHUB_OAUTH_CALLBACK_URL"],
+)
+app.include_router(router)

examples/service-fastapi/app/client.py

@@ -0,0 +1,11 @@
+import os
+
+import httpx
+
+
+# a minimal alternative to using HubOAuth class
+def get_client():
+    base_url = os.environ["JUPYTERHUB_API_URL"]
+    token = os.environ["JUPYTERHUB_API_TOKEN"]
+    headers = {"Authorization": "Bearer %s" % token}
+    return httpx.AsyncClient(base_url=base_url, headers=headers)

examples/service-fastapi/app/models.py

@@ -0,0 +1,46 @@
+from datetime import datetime
+from typing import Any
+from typing import List
+from typing import Optional
+
+from pydantic import BaseModel
+
+
+# https://jupyterhub.readthedocs.io/en/stable/_static/rest-api/index.html
+class Server(BaseModel):
+    name: str
+    ready: bool
+    pending: Optional[str]
+    url: str
+    progress_url: str
+    started: datetime
+    last_activity: datetime
+    state: Optional[Any]
+    user_options: Optional[Any]
+
+
+class User(BaseModel):
+    name: str
+    admin: bool
+    groups: List[str]
+    server: Optional[str]
+    pending: Optional[str]
+    last_activity: datetime
+    servers: Optional[List[Server]]
+
+
+# https://stackoverflow.com/questions/64501193/fastapi-how-to-use-httpexception-in-responses
+class AuthorizationError(BaseModel):
+    detail: str
+
+
+class HubResponse(BaseModel):
+    msg: str
+    request_url: str
+    token: str
+    response_code: int
+    hub_response: dict
+
+
+class HubApiError(BaseModel):
+    detail: HubResponse

examples/service-fastapi/app/security.py

@@ -0,0 +1,61 @@
+import os
+
+from fastapi import HTTPException
+from fastapi import Security
+from fastapi import status
+from fastapi.security import OAuth2AuthorizationCodeBearer
+from fastapi.security.api_key import APIKeyQuery
+
+from .client import get_client
+from .models import User
+
+### Endpoints can require authentication using Depends(get_current_user)
+### get_current_user will look for a token in url params or
+### Authorization: bearer token (header).
+### Hub technically supports cookie auth too, but it is deprecated so
+### not being included here.
+auth_by_param = APIKeyQuery(name="token", auto_error=False)
+
+auth_url = os.environ["PUBLIC_HOST"] + "/hub/api/oauth2/authorize"
+auth_by_header = OAuth2AuthorizationCodeBearer(
+    authorizationUrl=auth_url, tokenUrl="get_token", auto_error=False
+)
+### ^^ The flow for OAuth2 in Swagger is that the "authorize" button
+### will redirect user (browser) to "auth_url", which is the Hub login page.
+### After logging in, the browser will POST to our internal /get_token endpoint
+### with the auth code.  That endpoint POST's to Hub /oauth2/token with
+### our client_secret (JUPYTERHUB_API_TOKEN) and that code to get an
+### access_token, which it returns to browser, which places in Authorization header.
+
+### For consideration: optimize performance with a cache instead of
+### always hitting the Hub api?
+async def get_current_user(
+    auth_by_param: str = Security(auth_by_param),
+    auth_by_header: str = Security(auth_by_header),
+):
+    token = auth_by_param or auth_by_header
+    if token is None:
+        raise HTTPException(
+            status.HTTP_401_UNAUTHORIZED,
+            detail="Must login with token parameter or Authorization bearer header",
+        )
+
+    async with get_client() as client:
+        endpoint = "/user"
+        # normally we auth to Hub API with service api token,
+        # but this time auth as the user token to get user model
+        headers = {"Authorization": f"Bearer {token}"}
+        resp = await client.get(endpoint, headers=headers)
+        if resp.is_error:
+            raise HTTPException(
+                status.HTTP_400_BAD_REQUEST,
+                detail={
+                    "msg": "Error getting user info from token",
+                    "request_url": str(resp.request.url),
+                    "token": token,
+                    "response_code": resp.status_code,
+                    "hub_response": resp.json(),
+                },
+            )
+    user = User(**resp.json())
+    return user

examples/service-fastapi/app/service.py

@@ -0,0 +1,70 @@
+import os
+
+from fastapi import APIRouter
+from fastapi import Depends
+from fastapi import Form
+from fastapi import Request
+
+from .client import get_client
+from .models import AuthorizationError
+from .models import HubApiError
+from .models import User
+from .security import get_current_user
+
+# APIRouter prefix cannot end in /
+service_prefix = os.getenv("JUPYTERHUB_SERVICE_PREFIX", "").rstrip("/")
+router = APIRouter(prefix=service_prefix)
+
+
+@router.post("/get_token", include_in_schema=False)
+async def get_token(code: str = Form(...)):
+    "Callback function for OAuth2AuthorizationCodeBearer scheme"
+    # The only thing we need in this form post is the code
+    # Everything else we can hardcode / pull from env
+    async with get_client() as client:
+        redirect_uri = (
+            os.environ["PUBLIC_HOST"] + os.environ["JUPYTERHUB_OAUTH_CALLBACK_URL"],
+        )
+        data = {
+            "client_id": os.environ["JUPYTERHUB_CLIENT_ID"],
+            "client_secret": os.environ["JUPYTERHUB_API_TOKEN"],
+            "grant_type": "authorization_code",
+            "code": code,
+            "redirect_uri": redirect_uri,
+        }
+        resp = await client.post("/oauth2/token", data=data)
+    ### resp.json() is {'access_token': <token>, 'token_type': 'Bearer'}
+    return resp.json()
+
+
+@router.get("/")
+async def index():
+    "Non-authenticated function that returns {'Hello': 'World'}"
+    return {"Hello": "World"}
+
+
+# response_model and responses dict translate to OpenAPI (Swagger) hints
+# compare and contrast what the /me endpoint looks like in Swagger vs /debug
+@router.get(
+    "/me",
+    response_model=User,
+    responses={401: {'model': AuthorizationError}, 400: {'model': HubApiError}},
+)
+async def me(user: User = Depends(get_current_user)):
+    "Authenticated function that returns the User model"
+    return user
+
+
+@router.get("/debug")
+async def index(request: Request, user: User = Depends(get_current_user)):
+    """
+    Authenticated function that returns a few pieces of debug
+     * Environ of the service process
+     * Request headers
+     * User model
+    """
+    return {
+        "env": dict(os.environ),
+        "headers": dict(request.headers),
+        "user": user,
+    }

examples/service-fastapi/jupyterhub_config.py

@@ -0,0 +1,31 @@
+import os
+import warnings
+
+# When Swagger performs OAuth2 in the browser, it will set
+# the request host + relative path as the redirect uri, causing a
+# uri mismatch if the oauth_redirect_uri is just the relative path
+# is set in the c.JupyterHub.services entry (as per default).
+# Therefore need to know the request host ahead of time.
+if "PUBLIC_HOST" not in os.environ:
+    msg = (
+        "env PUBLIC_HOST is not set, defaulting to http://127.0.0.1:8000.  "
+        "This can cause problems with OAuth.  "
+        "Set PUBLIC_HOST to your public (browser accessible) host."
+    )
+    warnings.warn(msg)
+    public_host = "http://127.0.0.1:8000"
+else:
+    public_host = os.environ["PUBLIC_HOST"].rstrip('/')
+service_name = "fastapi"
+oauth_redirect_uri = f"{public_host}/services/{service_name}/oauth_callback"
+
+c.JupyterHub.services = [
+    {
+        "name": service_name,
+        "url": "http://127.0.0.1:10202",
+        "command": ["uvicorn", "app:app", "--port", "10202"],
+        "admin": True,
+        "oauth_redirect_uri": oauth_redirect_uri,
+        "environment": {"PUBLIC_HOST": public_host},
+    }
+]

examples/service-fastapi/requirements.txt

@@ -0,0 +1,4 @@
+fastapi
+httpx
+python-multipart
+uvicorn

jupyterhub/_version.py

@@ -3,8 +3,8 @@
 # Distributed under the terms of the Modified BSD License.
 
 version_info = (
-    1,
-    4,
+    2,
+    0,
     0,
     "",  # release (b1, rc1, or "" for final or dev)
     "dev",  # dev or nothing for beta/rc/stable releases

jupyterhub/alembic/versions/1cebaf56856c_session_id.py

@@ -23,7 +23,7 @@ tables = ('oauth_access_tokens', 'oauth_codes')
 
 def add_column_if_table_exists(table, column):
     engine = op.get_bind().engine
-    if table not in engine.table_names():
+    if table not in sa.inspect(engine).get_table_names():
         # table doesn't exist, no need to upgrade
         # because jupyterhub will create it on launch
         logger.warning("Skipping upgrade of absent table: %s", table)

jupyterhub/alembic/versions/4dc2d5a8c53c_user_options.py

@@ -17,7 +17,8 @@ from jupyterhub.orm import JSONDict
 
 
 def upgrade():
-    tables = op.get_bind().engine.table_names()
+    engine = op.get_bind().engine
+    tables = sa.inspect(engine).get_table_names()
     if 'spawners' in tables:
         op.add_column('spawners', sa.Column('user_options', JSONDict()))
 

jupyterhub/alembic/versions/56cc5a70207e_token_tracking.py

@@ -20,7 +20,8 @@ logger = logging.getLogger('alembic')
 
 
 def upgrade():
-    tables = op.get_bind().engine.table_names()
+    engine = op.get_bind().engine
+    tables = sa.inspect(engine).get_table_names()
     op.add_column('api_tokens', sa.Column('created', sa.DateTime(), nullable=True))
     op.add_column(
         'api_tokens', sa.Column('last_activity', sa.DateTime(), nullable=True)

jupyterhub/alembic/versions/833da8570507_rbac.py

@@ -0,0 +1,49 @@
+"""rbac
+
+Revision ID: 833da8570507
+Revises: 4dc2d5a8c53c
+Create Date: 2021-02-17 15:03:04.360368
+
+"""
+# revision identifiers, used by Alembic.
+revision = '833da8570507'
+down_revision = '4dc2d5a8c53c'
+branch_labels = None
+depends_on = None
+
+from alembic import op
+import sqlalchemy as sa
+
+
+def upgrade():
+    # FIXME, maybe: currently drops all api tokens and forces recreation!
+    # this ensures a consistent database, but requires:
+    # 1. all servers to be stopped for upgrade (maybe unavoidable anyway)
+    # 2. any manually issued/stored tokens to be re-issued
+
+    # tokens loaded via configuration will be recreated on launch and unaffected
+    op.drop_table('api_tokens')
+    op.drop_table('oauth_access_tokens')
+    return
+    # TODO: explore in-place migration. This seems hard!
+    # 1. add new columns in api tokens
+    # 2. fill default fields (client_id='jupyterhub') for all api tokens
+    # 3. copy oauth tokens into api tokens
+    # 4. give oauth tokens 'identify' scopes
+
+
+def downgrade():
+    # delete OAuth tokens for non-jupyterhub clients
+    # drop new columns from api tokens
+    op.drop_constraint(None, 'api_tokens', type_='foreignkey')
+    op.drop_column('api_tokens', 'session_id')
+    op.drop_column('api_tokens', 'client_id')
+
+    # FIXME: only drop tokens whose client id is not 'jupyterhub'
+    # until then, drop all tokens
+    op.drop_table("api_tokens")
+
+    op.drop_table('api_token_role_map')
+    op.drop_table('service_role_map')
+    op.drop_table('user_role_map')
+    op.drop_table('roles')

jupyterhub/alembic/versions/99a28a4418e1_user_created.py

@@ -31,7 +31,7 @@ def upgrade():
         % (now,)
     )
 
-    tables = c.engine.table_names()
+    tables = sa.inspect(c.engine).get_table_names()
 
     if 'spawners' in tables:
         op.add_column('spawners', sa.Column('started', sa.DateTime, nullable=True))

jupyterhub/alembic/versions/d68c98b66cd4_client_description.py

@@ -16,7 +16,8 @@ import sqlalchemy as sa
 
 
 def upgrade():
-    tables = op.get_bind().engine.table_names()
+    engine = op.get_bind().engine
+    tables = sa.inspect(engine).get_table_names()
     if 'oauth_clients' in tables:
         op.add_column(
             'oauth_clients', sa.Column('description', sa.Unicode(length=1023))

jupyterhub/apihandlers/auth.py

@@ -29,8 +29,6 @@ class TokenAPIHandler(APIHandler):
             "/authorizations/token/:token endpoint is deprecated in JupyterHub 2.0. Use /api/user"
         )
         orm_token = orm.APIToken.find(self.db, token)
-        if orm_token is None:
-            orm_token = orm.OAuthAccessToken.find(self.db, token)
         if orm_token is None:
             raise web.HTTPError(404)
 

jupyterhub/apihandlers/base.py

@@ -79,23 +79,39 @@ class APIHandler(BaseHandler):
                 % req_scope,
             )
 
-        def has_access(orm_resource, kind):
+        def has_access_to(orm_resource, kind):
             """
-            param orm_resource: User or Service or Group
-            param kind: 'users' or 'services' or 'groups'
+            param orm_resource: User or Service or Group or spawner
+            param kind: 'user' or 'service' or 'group' or 'server'.
+            `kind` could probably be derived from `orm_resource`, problem is Jupyterhub.users.User
             """
             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 'group' in sub_scope and kind == 'user':
-                    group_names = {group.name for group in orm_resource.groups}
+                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']
+                    if not found_resource:
+                        # Now check for specific servers:
+                        server_format = f"{orm_resource.user / orm_resource.name}"
+                        found_resource = server_format in sub_scope[kind]
+                elif 'group' in sub_scope:
+                    group_names = set()
+                    if kind == 'user':
+                        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
+        return has_access_to
 
     def get_current_user_cookie(self):
         """Override get_user_cookie to check Referer header"""
@@ -166,39 +182,29 @@ class APIHandler(BaseHandler):
             json.dumps({'status': status_code, 'message': message or status_message})
         )
 
-    def server_model(self, spawner, include_state=False):
+    def server_model(self, spawner):
         """Get the JSON model for a Spawner"""
-        return {
+        server_scope = 'read:users:servers'
+        server_state_scope = 'admin:users:server_state'
+        model = {
             'name': spawner.name,
             'last_activity': isoformat(spawner.orm_spawner.last_activity),
             'started': isoformat(spawner.orm_spawner.started),
             'pending': spawner.pending,
             'ready': spawner.ready,
-            'state': spawner.get_state() if include_state else None,
             'url': url_path_join(spawner.user.url, spawner.name, '/'),
             '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'):
+                model['state'] = spawner.get_state()
+        return model
 
     def token_model(self, token):
         """Get the JSON model for an APIToken"""
-        expires_at = None
-        if isinstance(token, orm.APIToken):
-            kind = 'api_token'
-            roles = [r.name for r in token.roles]
-            extra = {'note': token.note}
-            expires_at = token.expires_at
-        elif isinstance(token, orm.OAuthAccessToken):
-            kind = 'oauth'
-            # oauth tokens do not bear roles
-            roles = []
-            extra = {'oauth_client': token.client.description or token.client.client_id}
-            if token.expires_at:
-                expires_at = datetime.fromtimestamp(token.expires_at)
-        else:
-            raise TypeError(
-                "token must be an APIToken or OAuthAccessToken, not %s" % type(token)
-            )
 
         if token.user:
             owner_key = 'user'
@@ -211,16 +217,17 @@ class APIHandler(BaseHandler):
         model = {
             owner_key: owner,
             'id': token.api_id,
-            'kind': kind,
-            'roles': [role for role in roles],
+            'kind': 'api_token',
+            'roles': [r.name for r in token.roles],
             'created': isoformat(token.created),
             'last_activity': isoformat(token.last_activity),
-            'expires_at': isoformat(expires_at),
+            'expires_at': isoformat(token.expires_at),
+            'note': token.note,
+            'oauth_client': token.client.description or token.client.client_id,
         }
-        model.update(extra)
         return model
 
-    def user_model(self, user, include_servers=False, include_state=False):
+    def user_model(self, user):
         """Get the JSON model for a User object"""
         if isinstance(user, orm.User):
             user = self.users[user.id]
@@ -234,13 +241,26 @@ class APIHandler(BaseHandler):
             'pending': None,
             'created': isoformat(user.created),
             'last_activity': isoformat(user.last_activity),
+            'auth_state': None,  # placeholder, filled in later
         }
         access_map = {
-            'read:users': set(model.keys()),  # All available components
+            'read:users': {
+                'kind',
+                'name',
+                'admin',
+                'roles',
+                'groups',
+                'server',
+                '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'},
+            '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]",
@@ -257,35 +277,45 @@ class APIHandler(BaseHandler):
         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: Replace include_state with scope (read|admin):users:auth_state
+            if 'servers' in allowed_keys:
                 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
-                        )
+                        servers[name] = self.server_model(spawner)
         return model
 
-    def group_model(self, group):  # Todo: make consistent to do scope checking here
+    def group_model(self, group):
         """Get the JSON model for a Group object"""
-        return {
-            'kind': 'group',
-            'name': group.name,
-            'users': [u.name for u in group.users],
-            'roles': [r.name for r in group.roles],
-        }
+        model = {}
+        req_scope = 'read:groups'
+        if req_scope in self.parsed_scopes:
+            scope_filter = self.get_scope_filter(req_scope)
+            if scope_filter(group, kind='group'):
+                model = {
+                    'kind': 'group',
+                    'name': group.name,
+                    'roles': [r.name for r in group.roles],
+                    'users': [u.name for u in group.users],
+                }
+        return model
 
-    def service_model(self, service):  # Todo: make consistent to do scope checking here
+    def service_model(self, service):
         """Get the JSON model for a Service object"""
-        return {
-            'kind': 'service',
-            'name': service.name,
-            'admin': service.admin,
-            'roles': [r.name for r in service.roles],
-        }
+        model = {}
+        req_scope = 'read:services'
+        if req_scope in self.parsed_scopes:
+            scope_filter = self.get_scope_filter(req_scope)
+            if scope_filter(service, kind='service'):
+                model = {
+                    'kind': 'service',
+                    'name': service.name,
+                    'roles': [r.name for r in service.roles],
+                    'admin': service.admin,
+                }
+                # todo: Remove once we replace admin flag with role check
+        return model
 
     _user_model_types = {
         'name': str,

jupyterhub/apihandlers/users.py

@@ -14,7 +14,8 @@ from tornado import web
 from tornado.iostream import StreamClosedError
 
 from .. import orm
-from ..roles import update_roles
+from .. import scopes
+from ..roles import assign_default_roles
 from ..scopes import needs_scope
 from ..user import User
 from ..utils import isoformat
@@ -32,14 +33,16 @@ class SelfAPIHandler(APIHandler):
 
     async def get(self):
         user = self.current_user
-        if user is None:
-            # whoami can be accessed via oauth token
-            user = self.get_current_user_oauth_token()
         if user is None:
             raise web.HTTPError(403)
         if isinstance(user, orm.Service):
+            # ensure we have the minimal 'identify' scopes for the token owner
+            self.raw_scopes.update(scopes.identify_scopes(user))
+            self.parsed_scopes = scopes.parse_scopes(self.raw_scopes)
             model = self.service_model(user)
         else:
+            self.raw_scopes.update(scopes.identify_scopes(user.orm_user))
+            self.parsed_scopes = scopes.parse_scopes(self.raw_scopes)
             model = self.user_model(user)
         self.write(json.dumps(model))
 
@@ -56,7 +59,7 @@ class UserListAPIHandler(APIHandler):
     @needs_scope(
         'read:users',
         'read:users:name',
-        'reda:users:servers',
+        'read:users:servers',
         'read:users:groups',
         'read:users:activity',
     )
@@ -104,9 +107,7 @@ class UserListAPIHandler(APIHandler):
         data = []
         for u in query:
             if post_filter is None or post_filter(u):
-                user_model = self.user_model(
-                    u, include_servers=True, include_state=True
-                )
+                user_model = self.user_model(u)
                 if user_model:
                     data.append(user_model)
         self.write(json.dumps(data))
@@ -151,7 +152,7 @@ class UserListAPIHandler(APIHandler):
             user = self.user_from_username(name)
             if admin:
                 user.admin = True
-            update_roles(self.db, obj=user, kind='users')
+            assign_default_roles(self.db, entity=user)
             self.db.commit()
             try:
                 await maybe_future(self.authenticator.add_user(user))
@@ -187,18 +188,23 @@ def admin_or_self(method):
 
 
 class UserAPIHandler(APIHandler):
-    @needs_scope('read:users')
-    async def get(self, user_name):
+    @needs_scope(
+        'read:users',
+        'read:users:name',
+        'read:users:servers',
+        'read:users:groups',
+        'read:users:activity',
+    )
+    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, include_servers=True, include_state=self.current_user.admin
-        )
+        model = self.user_model(user)
         # auth state will only be shown if the requester is an admin
         # this means users can't see their own auth state unless they
         # are admins, Hub admins often are also marked as admins so they
         # will see their auth state but normal users won't
-        requester = self.current_user
-        if requester.admin:
+        if 'auth_state' in model:
             model['auth_state'] = await user.get_auth_state()
         self.write(json.dumps(model))
 
@@ -214,7 +220,7 @@ class UserAPIHandler(APIHandler):
             self._check_user_model(data)
             if 'admin' in data:
                 user.admin = data['admin']
-                update_roles(self.db, obj=user, kind='users')
+                assign_default_roles(self.db, entity=user)
         self.db.commit()
 
         try:
@@ -262,7 +268,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:
@@ -282,7 +288,7 @@ class UserAPIHandler(APIHandler):
             else:
                 setattr(user, key, value)
                 if key == 'admin':
-                    update_roles(self.db, obj=user, kind='users')
+                    assign_default_roles(self.db, entity=user)
         self.db.commit()
         user_ = self.user_model(user)
         user_['auth_state'] = await user.get_auth_state()
@@ -313,18 +319,9 @@ class UserTokenListAPIHandler(APIHandler):
                 continue
             api_tokens.append(self.token_model(token))
 
-        oauth_tokens = []
-        # OAuth tokens use integer timestamps
-        now_timestamp = now.timestamp()
-        for token in sorted(user.oauth_tokens, key=sort_key):
-            if token.expires_at and token.expires_at < now_timestamp:
-                # exclude expired tokens
-                self.db.delete(token)
-                self.db.commit()
-                continue
-            oauth_tokens.append(self.token_model(token))
-        self.write(json.dumps({'api_tokens': api_tokens, 'oauth_tokens': oauth_tokens}))
+        self.write(json.dumps({'api_tokens': api_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):
@@ -406,19 +403,15 @@ class UserTokenAPIHandler(APIHandler):
         (e.g. wrong owner, invalid key format, etc.)
         """
         not_found = "No such token %s for user %s" % (token_id, user.name)
-        prefix, id_ = token_id[0], token_id[1:]
-        if prefix == 'a':
-            Token = orm.APIToken
-        elif prefix == 'o':
-            Token = orm.OAuthAccessToken
-        else:
+        prefix, id_ = token_id[:1], token_id[1:]
+        if prefix != 'a':
             raise web.HTTPError(404, not_found)
         try:
             id_ = int(id_)
         except ValueError:
             raise web.HTTPError(404, not_found)
 
-        orm_token = self.db.query(Token).filter(Token.id == id_).first()
+        orm_token = self.db.query(orm.APIToken).filter_by(id=id_).first()
         if orm_token is None or orm_token.user is not user.orm_user:
             raise web.HTTPError(404, "Token not found %s", orm_token)
         return orm_token
@@ -440,10 +433,10 @@ class UserTokenAPIHandler(APIHandler):
             raise web.HTTPError(404, "No such user: %s" % user_name)
         token = self.find_token_by_id(user, token_id)
         # deleting an oauth token deletes *all* oauth tokens for that client
-        if isinstance(token, orm.OAuthAccessToken):
-            client_id = token.client_id
+        client_id = token.client_id
+        if token.client_id != "jupyterhub":
             tokens = [
-                token for token in user.oauth_tokens if token.client_id == client_id
+                token for token in user.api_tokens if token.client_id == client_id
             ]
         else:
             tokens = [token]
@@ -764,7 +757,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

@@ -9,6 +9,7 @@ import json
 import logging
 import os
 import re
+import secrets
 import signal
 import socket
 import sys
@@ -382,6 +383,42 @@ class JupyterHub(Application):
         Default is two weeks.
         """,
     ).tag(config=True)
+
+    oauth_token_expires_in = Integer(
+        help="""Expiry (in seconds) of OAuth access tokens.
+
+        The default is to expire when the cookie storing them expires,
+        according to `cookie_max_age_days` config.
+
+        These are the tokens stored in cookies when you visit
+        a single-user server or service.
+        When they expire, you must re-authenticate with the Hub,
+        even if your Hub authentication is still valid.
+        If your Hub authentication is valid,
+        logging in may be a transparent redirect as you refresh the page.
+        
+        This does not affect JupyterHub API tokens in general,
+        which do not expire by default.
+        Only tokens issued during the oauth flow
+        accessing services and single-user servers are affected.
+
+        .. versionadded:: 1.4
+            OAuth token expires_in was not previously configurable.
+        .. versionchanged:: 1.4
+            Default now uses cookie_max_age_days so that oauth tokens
+            which are generally stored in cookies,
+            expire when the cookies storing them expire.
+            Previously, it was one hour.
+        """,
+        config=True,
+    )
+
+    @default("oauth_token_expires_in")
+    def _cookie_max_age_seconds(self):
+        """default to cookie max age, where these tokens are stored"""
+        # convert cookie max age days to seconds
+        return int(self.cookie_max_age_days * 24 * 3600)
+
     redirect_to_server = Bool(
         True, help="Redirect user to server (if running), instead of control panel."
     ).tag(config=True)
@@ -1502,7 +1539,7 @@ class JupyterHub(Application):
         if not secret:
             secret_from = 'new'
             self.log.debug("Generating new %s", trait_name)
-            secret = os.urandom(COOKIE_SECRET_BYTES)
+            secret = secrets.token_bytes(COOKIE_SECRET_BYTES)
 
         if secret_file and secret_from == 'new':
             # if we generated a new secret, store it in the secret_file
@@ -1655,6 +1692,26 @@ class JupyterHub(Application):
         except orm.DatabaseSchemaMismatch as e:
             self.exit(e)
 
+        # ensure the default oauth client exists
+        if (
+            not self.db.query(orm.OAuthClient)
+            .filter_by(identifier="jupyterhub")
+            .one_or_none()
+        ):
+            # create the oauth client for jupyterhub itself
+            # this allows us to distinguish between orphaned tokens
+            # (failed cascade deletion) and tokens issued by the hub
+            # it has no client_secret, which means it cannot be used
+            # to make requests
+            client = orm.OAuthClient(
+                identifier="jupyterhub",
+                secret="",
+                redirect_uri="",
+                description="JupyterHub",
+            )
+            self.db.add(client)
+            self.db.commit()
+
     def init_hub(self):
         """Load the Hub URL config"""
         hub_args = dict(
@@ -1860,12 +1917,12 @@ class JupyterHub(Application):
         self.log.debug('Loading default roles to database')
         default_roles = roles.get_default_roles()
         for role in default_roles:
-            roles.add_role(db, role)
+            roles.create_role(db, role)
 
         # load predefined roles from config file
         self.log.debug('Loading predefined roles from config file to database')
         for predef_role in self.load_roles:
-            roles.add_role(db, predef_role)
+            roles.create_role(db, predef_role)
             # add users, services, and/or groups,
             # tokens need to be checked for permissions
             for bearer in role_bearers:
@@ -1882,19 +1939,26 @@ class JupyterHub(Application):
                                     "Username %r is not in Authenticator.allowed_users"
                                     % bname
                                 )
-                        roles.add_obj(
-                            db, objname=bname, kind=bearer, rolename=predef_role['name']
+                        Class = orm.get_class(bearer)
+                        orm_obj = Class.find(db, bname)
+                        roles.grant_role(
+                            db, entity=orm_obj, rolename=predef_role['name']
                         )
+        # make sure that on no admin situation, all roles are reset
+        admin_role = orm.Role.find(db, name='admin')
+        if not admin_role.users:
+            app_log.warning(
+                "No admin users found; assuming hub upgrade. Initializing default roles for all entities"
+            )
+            # make sure all users, services and tokens have at least one role (update with default)
+            for bearer in role_bearers:
+                roles.check_for_default_roles(db, bearer)
 
-        # make sure role bearers have at least a default role
-        for bearer in role_bearers:
-            roles.check_for_default_roles(db, bearer)
+            # now add roles to tokens if their owner's permissions allow
+            roles.add_predef_roles_tokens(db, self.load_roles)
 
-        # now add roles to tokens if their owner's permissions allow
-        roles.add_predef_roles_tokens(db, self.load_roles)
-
-        # check tokens for default roles
-        roles.check_for_default_roles(db, bearer='tokens')
+            # check tokens for default roles
+            roles.check_for_default_roles(db, bearer='tokens')
 
     async def _add_tokens(self, token_dict, kind):
         """Add tokens for users or services to the database"""
@@ -1935,6 +1999,13 @@ class JupyterHub(Application):
                     db.add(obj)
                     db.commit()
                 self.log.info("Adding API token for %s: %s", kind, name)
+                # If we have roles in the configuration file, they will be added later
+                # Todo: works but ugly
+                config_roles = None
+                for config_role in self.load_roles:
+                    if 'tokens' in config_role and token in config_role['tokens']:
+                        config_roles = []
+                        break
                 try:
                     # set generated=False to ensure that user-provided tokens
                     # get extra hashing (don't trust entropy of user-provided tokens)
@@ -1942,6 +2013,7 @@ class JupyterHub(Application):
                         token,
                         note="from config",
                         generated=self.trust_user_provided_tokens,
+                        roles=config_roles,
                     )
                 except Exception:
                     if created:
@@ -1962,12 +2034,13 @@ class JupyterHub(Application):
         run periodically
         """
         # this should be all the subclasses of Expiring
-        for cls in (orm.APIToken, orm.OAuthAccessToken, orm.OAuthCode):
+        for cls in (orm.APIToken, orm.OAuthCode):
             self.log.debug("Purging expired {name}s".format(name=cls.__name__))
             cls.purge_expired(self.db)
 
     async def init_api_tokens(self):
         """Load predefined API tokens (for services) into database"""
+
         await self._add_tokens(self.service_tokens, kind='service')
         await self._add_tokens(self.api_tokens, kind='user')
 
@@ -1998,6 +2071,8 @@ class JupyterHub(Application):
             if orm_service is None:
                 # not found, create a new one
                 orm_service = orm.Service(name=name)
+                if spec.get('admin', False):
+                    roles.update_roles(self.db, entity=orm_service, roles=['admin'])
                 self.db.add(orm_service)
             orm_service.admin = spec.get('admin', False)
             self.db.commit()
@@ -2236,6 +2311,7 @@ class JupyterHub(Application):
             lambda: self.db,
             url_prefix=url_path_join(base_url, 'api/oauth2'),
             login_url=url_path_join(base_url, 'login'),
+            token_expires_in=self.oauth_token_expires_in,
         )
 
     def cleanup_oauth_clients(self):
@@ -2243,7 +2319,7 @@ class JupyterHub(Application):
 
         This should mainly be services that have been removed from configuration or renamed.
         """
-        oauth_client_ids = set()
+        oauth_client_ids = {"jupyterhub"}
         for service in self._service_map.values():
             if service.oauth_available:
                 oauth_client_ids.add(service.oauth_client_id)

jupyterhub/handlers/base.py

@@ -247,26 +247,6 @@ class BaseHandler(RequestHandler):
             return None
         return match.group(1)
 
-    def get_current_user_oauth_token(self):
-        """Get the current user identified by OAuth access token
-
-        Separate from API token because OAuth access tokens
-        can only be used for identifying users,
-        not using the API.
-        """
-        token = self.get_auth_token()
-        if token is None:
-            return None
-        orm_token = orm.OAuthAccessToken.find(self.db, token)
-        if orm_token is None:
-            return None
-
-        now = datetime.utcnow()
-        recorded = self._record_activity(orm_token, now)
-        if self._record_activity(orm_token.user, now) or recorded:
-            self.db.commit()
-        return self._user_from_orm(orm_token.user)
-
     def _record_activity(self, obj, timestamp=None):
         """record activity on an ORM object
 
@@ -373,7 +353,7 @@ class BaseHandler(RequestHandler):
             # FIXME: scopes should give us better control than this
             # don't consider API requests originating from a server
             # to be activity from the user
-            if not orm_token.note.startswith("Server at "):
+            if not orm_token.note or not orm_token.note.startswith("Server at "):
                 recorded = self._record_activity(orm_token.user, now) or recorded
         if recorded:
             self.db.commit()
@@ -439,17 +419,10 @@ class BaseHandler(RequestHandler):
     def _resolve_scopes(self):
         self.raw_scopes = set()
         app_log.debug("Loading and parsing scopes")
-        if not self.current_user:
-            # check for oauth tokens as long as #3380 not merged
-            user_from_oauth = self.get_current_user_oauth_token()
-            if user_from_oauth is not None:
-                self.raw_scopes = {f'read:users!user={user_from_oauth.name}'}
-            else:
-                app_log.debug("No user found, no scopes loaded")
-        else:
-            api_token = self.get_token()
-            if api_token:
-                self.raw_scopes = scopes.get_scopes_for(api_token)
+        if self.current_user:
+            orm_token = self.get_token()
+            if orm_token:
+                self.raw_scopes = scopes.get_scopes_for(orm_token)
             else:
                 self.raw_scopes = scopes.get_scopes_for(self.current_user)
         self.parsed_scopes = scopes.parse_scopes(self.raw_scopes)
@@ -480,7 +453,7 @@ class BaseHandler(RequestHandler):
             # not found, create and register user
             u = orm.User(name=username)
             self.db.add(u)
-            roles.update_roles(self.db, obj=u, kind='users')
+            roles.assign_default_roles(self.db, entity=u)
             TOTAL_USERS.inc()
             self.db.commit()
             user = self._user_from_orm(u)
@@ -501,10 +474,8 @@ class BaseHandler(RequestHandler):
                 # don't clear session tokens if not logged in,
                 # because that could be a malicious logout request!
                 count = 0
-                for access_token in (
-                    self.db.query(orm.OAuthAccessToken)
-                    .filter(orm.OAuthAccessToken.user_id == user.id)
-                    .filter(orm.OAuthAccessToken.session_id == session_id)
+                for access_token in self.db.query(orm.APIToken).filter_by(
+                    user_id=user.id, session_id=session_id
                 ):
                     self.db.delete(access_token)
                     count += 1
@@ -765,7 +736,7 @@ class BaseHandler(RequestHandler):
         # Only set `admin` if the authenticator returned an explicit value.
         if admin is not None and admin != user.admin:
             user.admin = admin
-            roles.update_roles(self.db, obj=user, kind='users')
+            roles.assign_default_roles(self.db, entity=user)
             self.db.commit()
         # always set auth_state and commit,
         # because there could be key-rotation or clearing of previous values

jupyterhub/handlers/pages.py

@@ -552,36 +552,32 @@ class TokenPageHandler(BaseHandler):
             return (token.last_activity or never, token.created or never)
 
         now = datetime.utcnow()
-        api_tokens = []
-        for token in sorted(user.api_tokens, key=sort_key, reverse=True):
-            if token.expires_at and token.expires_at < now:
-                self.db.delete(token)
-                self.db.commit()
-                continue
-            api_tokens.append(token)
 
         # group oauth client tokens by client id
-        # AccessTokens have expires_at as an integer timestamp
-        now_timestamp = now.timestamp()
-        oauth_tokens = defaultdict(list)
-        for token in user.oauth_tokens:
-            if token.expires_at and token.expires_at < now_timestamp:
-                self.log.warning("Deleting expired token")
+        all_tokens = defaultdict(list)
+        for token in sorted(user.api_tokens, key=sort_key, reverse=True):
+            if token.expires_at and token.expires_at < now:
+                self.log.warning(f"Deleting expired token {token}")
                 self.db.delete(token)
                 self.db.commit()
                 continue
             if not token.client_id:
                 # token should have been deleted when client was deleted
-                self.log.warning("Deleting stale oauth token for %s", user.name)
+                self.log.warning("Deleting stale oauth token {token}")
                 self.db.delete(token)
                 self.db.commit()
                 continue
-            oauth_tokens[token.client_id].append(token)
+            all_tokens[token.client_id].append(token)
 
+        # individually list tokens issued by jupyterhub itself
+        api_tokens = all_tokens.pop("jupyterhub", [])
+
+        # group all other tokens issued under their owners
         # get the earliest created and latest last_activity
         # timestamp for a given oauth client
         oauth_clients = []
-        for client_id, tokens in oauth_tokens.items():
+
+        for client_id, tokens in all_tokens.items():
             created = tokens[0].created
             last_activity = tokens[0].last_activity
             for token in tokens[1:]:

jupyterhub/oauth/provider.py

@@ -2,18 +2,18 @@
 
 implements https://oauthlib.readthedocs.io/en/latest/oauth2/server.html
 """
+from datetime import timedelta
+
 from oauthlib import uri_validate
 from oauthlib.oauth2 import RequestValidator
 from oauthlib.oauth2 import WebApplicationServer
 from oauthlib.oauth2.rfc6749.grant_types import authorization_code
 from oauthlib.oauth2.rfc6749.grant_types import base
-from tornado.escape import url_escape
 from tornado.log import app_log
 
 from .. import orm
 from ..utils import compare_token
 from ..utils import hash_token
-from ..utils import url_path_join
 
 # patch absolute-uri check
 # because we want to allow relative uri oauth
@@ -60,6 +60,9 @@ class JupyterHubRequestValidator(RequestValidator):
         )
         if oauth_client is None:
             return False
+        if not client_secret or not oauth_client.secret:
+            # disallow authentication with no secret
+            return False
         if not compare_token(oauth_client.secret, client_secret):
             app_log.warning("Client secret mismatch for %s", client_id)
             return False
@@ -339,19 +342,22 @@ class JupyterHubRequestValidator(RequestValidator):
             .filter_by(identifier=request.client.client_id)
             .first()
         )
-        orm_access_token = orm.OAuthAccessToken(
-            client=client,
-            grant_type=orm.GrantType.authorization_code,
-            expires_at=orm.OAuthAccessToken.now() + token['expires_in'],
-            refresh_token=token['refresh_token'],
-            # TODO: save scopes,
-            # scopes=scopes,
+        # FIXME: pick a role
+        # this will be empty for now
+        roles = list(self.db.query(orm.Role).filter_by(name='identify'))
+        # FIXME: support refresh tokens
+        # These should be in a new table
+        token.pop("refresh_token", None)
+
+        # APIToken.new commits the token to the db
+        orm.APIToken.new(
+            client_id=client.identifier,
+            expires_in=token['expires_in'],
+            roles=roles,
             token=token['access_token'],
             session_id=request.session_id,
             user=request.user,
         )
-        self.db.add(orm_access_token)
-        self.db.commit()
         return client.redirect_uri
 
     def validate_bearer_token(self, token, scopes, request):
@@ -412,6 +418,8 @@ class JupyterHubRequestValidator(RequestValidator):
         )
         if orm_client is None:
             return False
+        if not orm_client.secret:
+            return False
         request.client = orm_client
         return True
 
@@ -558,30 +566,37 @@ class JupyterHubOAuthServer(WebApplicationServer):
 
         hash its client_secret before putting it in the database.
         """
-        # clear existing clients with same ID
-        for orm_client in self.db.query(orm.OAuthClient).filter_by(
-            identifier=client_id
-        ):
-            self.db.delete(orm_client)
-        self.db.commit()
-
-        orm_client = orm.OAuthClient(
-            identifier=client_id,
-            secret=hash_token(client_secret),
-            redirect_uri=redirect_uri,
-            description=description,
+        # Update client if it already exists, else create it
+        # Sqlalchemy doesn't have a good db agnostic UPSERT,
+        # so we do this manually. It's protected inside a
+        # transaction, so should fail if there are multiple
+        # rows with the same identifier.
+        orm_client = (
+            self.db.query(orm.OAuthClient).filter_by(identifier=client_id).one_or_none()
         )
-        self.db.add(orm_client)
+        if orm_client is None:
+            orm_client = orm.OAuthClient(
+                identifier=client_id,
+            )
+            self.db.add(orm_client)
+            app_log.info(f'Creating oauth client {client_id}')
+        else:
+            app_log.info(f'Updating oauth client {client_id}')
+        orm_client.secret = hash_token(client_secret) if client_secret else ""
+        orm_client.redirect_uri = redirect_uri
+        orm_client.description = description
         self.db.commit()
 
     def fetch_by_client_id(self, client_id):
         """Find a client by its id"""
-        return self.db.query(orm.OAuthClient).filter_by(identifier=client_id).first()
+        client = self.db.query(orm.OAuthClient).filter_by(identifier=client_id).first()
+        if client and client.secret:
+            return client
 
 
-def make_provider(session_factory, url_prefix, login_url):
+def make_provider(session_factory, url_prefix, login_url, **oauth_server_kwargs):
     """Make an OAuth provider"""
     db = session_factory()
     validator = JupyterHubRequestValidator(db)
-    server = JupyterHubOAuthServer(db, validator)
+    server = JupyterHubOAuthServer(db, validator, **oauth_server_kwargs)
     return server

jupyterhub/orm.py

@@ -39,7 +39,8 @@ from sqlalchemy.types import Text
 from sqlalchemy.types import TypeDecorator
 from tornado.log import app_log
 
-from .roles import add_role
+from .roles import assign_default_roles
+from .roles import create_role
 from .roles import get_default_roles
 from .roles import update_roles
 from .utils import compare_token
@@ -276,9 +277,6 @@ class User(Base):
     last_activity = Column(DateTime, nullable=True)
 
     api_tokens = relationship("APIToken", backref="user", cascade="all, delete-orphan")
-    oauth_tokens = relationship(
-        "OAuthAccessToken", backref="user", cascade="all, delete-orphan"
-    )
     oauth_codes = relationship(
         "OAuthCode", backref="user", cascade="all, delete-orphan"
     )
@@ -484,7 +482,9 @@ class Hashed(Expiring):
     @classmethod
     def check_token(cls, db, token):
         """Check if a token is acceptable"""
+        print("checking", cls, token, len(token), cls.min_length)
         if len(token) < cls.min_length:
+            print("raising")
             raise ValueError(
                 "Tokens must be at least %i characters, got %r"
                 % (cls.min_length, token)
@@ -529,14 +529,34 @@ class Hashed(Expiring):
                 return orm_token
 
 
+# ------------------------------------
+# OAuth tables
+# ------------------------------------
+
+
+class GrantType(enum.Enum):
+    # we only use authorization_code for now
+    authorization_code = 'authorization_code'
+    implicit = 'implicit'
+    password = 'password'
+    client_credentials = 'client_credentials'
+    refresh_token = 'refresh_token'
+
+
 class APIToken(Hashed, Base):
     """An API token"""
 
     __tablename__ = 'api_tokens'
 
-    user_id = Column(Integer, ForeignKey('users.id', ondelete="CASCADE"), nullable=True)
+    user_id = Column(
+        Integer,
+        ForeignKey('users.id', ondelete="CASCADE"),
+        nullable=True,
+    )
     service_id = Column(
-        Integer, ForeignKey('services.id', ondelete="CASCADE"), nullable=True
+        Integer,
+        ForeignKey('services.id', ondelete="CASCADE"),
+        nullable=True,
     )
 
     id = Column(Integer, primary_key=True)
@@ -547,6 +567,26 @@ class APIToken(Hashed, Base):
     def api_id(self):
         return 'a%i' % self.id
 
+    # added in 2.0
+    client_id = Column(
+        Unicode(255),
+        ForeignKey(
+            'oauth_clients.identifier',
+            ondelete='CASCADE',
+        ),
+    )
+    # FIXME: refresh_tokens not implemented
+    # should be a relation to another token table
+    # refresh_token = Column(
+    #     Integer,
+    #     ForeignKey('refresh_tokens.id', ondelete="CASCADE"),
+    #     nullable=True,
+    # )
+
+    # the browser session id associated with a given token,
+    # if issued during oauth to be stored in a cookie
+    session_id = Column(Unicode(255), nullable=True)
+
     # token metadata for bookkeeping
     now = datetime.utcnow  # for expiry
     created = Column(DateTime, default=datetime.utcnow)
@@ -565,8 +605,12 @@ class APIToken(Hashed, Base):
             # this shouldn't happen
             kind = 'owner'
             name = 'unknown'
-        return "<{cls}('{pre}...', {kind}='{name}')>".format(
-            cls=self.__class__.__name__, pre=self.prefix, kind=kind, name=name
+        return "<{cls}('{pre}...', {kind}='{name}', client_id={client_id!r})>".format(
+            cls=self.__class__.__name__,
+            pre=self.prefix,
+            kind=kind,
+            name=name,
+            client_id=self.client_id,
         )
 
     @classmethod
@@ -587,6 +631,14 @@ class APIToken(Hashed, Base):
             raise ValueError("kind must be 'user', 'service', or None, not %r" % kind)
         for orm_token in prefix_match:
             if orm_token.match(token):
+                if not orm_token.client_id:
+                    app_log.warning(
+                        "Deleting stale oauth token for %s with no client",
+                        orm_token.user and orm_token.user.name,
+                    )
+                    db.delete(orm_token)
+                    db.commit()
+                    return
                 return orm_token
 
     @classmethod
@@ -598,7 +650,10 @@ class APIToken(Hashed, Base):
         roles=None,
         note='',
         generated=True,
+        session_id=None,
         expires_in=None,
+        client_id='jupyterhub',
+        return_orm=False,
     ):
         """Generate a new API token for a user or service"""
         assert user or service
@@ -613,7 +668,12 @@ class APIToken(Hashed, Base):
             cls.check_token(db, token)
         # two stages to ensure orm_token.generated has been set
         # before token setter is called
-        orm_token = cls(generated=generated, note=note or '')
+        orm_token = cls(
+            generated=generated,
+            note=note or '',
+            client_id=client_id,
+            session_id=session_id,
+        )
         orm_token.token = token
         if user:
             assert user.id is not None
@@ -630,82 +690,16 @@ class APIToken(Hashed, Base):
         if not token_role:
             default_roles = get_default_roles()
             for role in default_roles:
-                add_role(db, role)
-        update_roles(db, obj=orm_token, kind='tokens', roles=roles)
+                create_role(db, role)
+        if roles is not None:
+            update_roles(db, entity=orm_token, roles=roles)
+        else:
+            assign_default_roles(db, entity=orm_token)
+
         db.commit()
         return token
 
 
-# ------------------------------------
-# OAuth tables
-# ------------------------------------
-
-
-class GrantType(enum.Enum):
-    # we only use authorization_code for now
-    authorization_code = 'authorization_code'
-    implicit = 'implicit'
-    password = 'password'
-    client_credentials = 'client_credentials'
-    refresh_token = 'refresh_token'
-
-
-class OAuthAccessToken(Hashed, Base):
-    __tablename__ = 'oauth_access_tokens'
-    id = Column(Integer, primary_key=True, autoincrement=True)
-
-    @staticmethod
-    def now():
-        return datetime.utcnow().timestamp()
-
-    @property
-    def api_id(self):
-        return 'o%i' % self.id
-
-    client_id = Column(
-        Unicode(255), ForeignKey('oauth_clients.identifier', ondelete='CASCADE')
-    )
-    grant_type = Column(Enum(GrantType), nullable=False)
-    expires_at = Column(Integer)
-    refresh_token = Column(Unicode(255))
-    # TODO: drop refresh_expires_at. Refresh tokens shouldn't expire
-    refresh_expires_at = Column(Integer)
-    user_id = Column(Integer, ForeignKey('users.id', ondelete='CASCADE'))
-    service = None  # for API-equivalence with APIToken
-
-    # the browser session id associated with a given token
-    session_id = Column(Unicode(255))
-
-    # from Hashed
-    hashed = Column(Unicode(255), unique=True)
-    prefix = Column(Unicode(16), index=True)
-
-    created = Column(DateTime, default=datetime.utcnow)
-    last_activity = Column(DateTime, nullable=True)
-
-    def __repr__(self):
-        return "<{cls}('{prefix}...', client_id={client_id!r}, user={user!r}, expires_in={expires_in}>".format(
-            cls=self.__class__.__name__,
-            client_id=self.client_id,
-            user=self.user and self.user.name,
-            prefix=self.prefix,
-            expires_in=self.expires_in,
-        )
-
-    @classmethod
-    def find(cls, db, token):
-        orm_token = super().find(db, token)
-        if orm_token and not orm_token.client_id:
-            app_log.warning(
-                "Deleting stale oauth token for %s with no client",
-                orm_token.user and orm_token.user.name,
-            )
-            db.delete(orm_token)
-            db.commit()
-            return
-        return orm_token
-
-
 class OAuthCode(Expiring, Base):
     __tablename__ = 'oauth_codes'
 
@@ -747,7 +741,7 @@ class OAuthClient(Base):
         return self.identifier
 
     access_tokens = relationship(
-        OAuthAccessToken, backref='client', cascade='all, delete-orphan'
+        APIToken, backref='client', cascade='all, delete-orphan'
     )
     codes = relationship(OAuthCode, backref='client', cascade='all, delete-orphan')
 
@@ -868,7 +862,7 @@ def check_db_revision(engine):
     - Empty databases are tagged with the current revision
     """
     # Check database schema version
-    current_table_names = set(engine.table_names())
+    current_table_names = set(inspect(engine).get_table_names())
     my_table_names = set(Base.metadata.tables.keys())
 
     from .dbutil import _temp_alembic_ini

jupyterhub/roles.py

@@ -47,11 +47,6 @@ def get_default_roles():
             'description': 'Token with same rights as token owner',
             'scopes': ['all'],
         },
-        {
-            'name': 'service',
-            'description': 'Temporary no scope role for services',
-            'scopes': [],
-        },
     ]
     return default_roles
 
@@ -83,7 +78,7 @@ def expand_self_scope(name, read_only=False):
     return {"{}!user={}".format(scope, name) for scope in scope_list}
 
 
-def get_scope_hierarchy():
+def _get_scope_hierarchy():
     """
     Returns a dictionary of scopes:
     scopes.keys() = scopes of highest level and scopes that have their own subscopes
@@ -101,8 +96,8 @@ def get_scope_hierarchy():
             'read:users:servers',
         ],
         'users:tokens': ['read:users:tokens'],
-        'admin:users': None,
-        '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,
@@ -133,7 +128,7 @@ def horizontal_filter(func):
 def _expand_scope(scopename):
     """Returns a set of all subscopes"""
 
-    scopes = get_scope_hierarchy()
+    scopes = _get_scope_hierarchy()
     subscopes = [scopename]
 
     def _expand_subscopes(index):
@@ -165,23 +160,18 @@ def expand_roles_to_scopes(orm_object):
     pass_roles = orm_object.roles
     if isinstance(orm_object, orm.User):
         groups_roles = []
-        # groups_roles = [role for group.role in orm_object.groups for role in group.roles]
         for group in orm_object.groups:
             groups_roles.extend(group.roles)
         pass_roles.extend(groups_roles)
-    # scopes = get_subscopes(*orm_object.roles)
-    scopes = get_subscopes(*pass_roles)
+    scopes = _get_subscopes(*pass_roles)
     if 'self' in scopes:
-        if not (isinstance(orm_object, orm.User) or hasattr(orm_object, 'orm_user')):
-            raise ValueError(
-                "Metascope 'self' only valid for Users, got %s" % orm_object
-            )
         scopes.remove('self')
-        scopes |= expand_self_scope(orm_object.name)
+        if isinstance(orm_object, orm.User) or hasattr(orm_object, 'orm_user'):
+            scopes |= expand_self_scope(orm_object.name)
     return scopes
 
 
-def get_subscopes(*args):
+def _get_subscopes(*args):
     """Returns a set of all available subscopes for a specified role or list of roles"""
 
     scope_list = []
@@ -197,7 +187,7 @@ def get_subscopes(*args):
 def _check_scopes(*args):
     """Check if provided scopes exist"""
 
-    allowed_scopes = get_scope_hierarchy()
+    allowed_scopes = _get_scope_hierarchy()
     allowed_filters = ['!user=', '!service=', '!group=', '!server=']
     subscopes = set(
         chain.from_iterable([x for x in allowed_scopes.values() if x is not None])
@@ -229,11 +219,14 @@ def _overwrite_role(role, role_dict):
                     'admin role description or scopes cannot be overwritten'
                 )
             else:
-                setattr(role, attr, role_dict[attr])
-                app_log.info('Role %r %r attribute has been changed', role.name, attr)
+                if role_dict[attr] != getattr(role, attr):
+                    setattr(role, attr, role_dict[attr])
+                    app_log.info(
+                        'Role %r %r attribute has been changed', role.name, attr
+                    )
 
 
-def add_role(db, role_dict):
+def create_role(db, role_dict):
     """Adds a new role to database or modifies an existing one"""
 
     default_roles = get_default_roles()
@@ -265,7 +258,7 @@ def add_role(db, role_dict):
     db.commit()
 
 
-def remove_role(db, rolename):
+def delete_role(db, rolename):
     """Removes a role from database"""
 
     # default roles are not removable
@@ -285,76 +278,73 @@ def remove_role(db, rolename):
 def existing_only(func):
     """Decorator for checking if objects and roles exist"""
 
-    def _check_existence(db, objname, kind, rolename):
-
-        Class = orm.get_class(kind)
-        obj = Class.find(db, objname)
+    def _check_existence(db, entity, rolename):
         role = orm.Role.find(db, rolename)
-
-        if obj is None:
-            raise ValueError("%r of kind %r does not exist" % (objname, kind))
+        if entity is None:
+            raise ValueError(
+                "%r of kind %r does not exist" % (entity, type(entity).__name__)
+            )
         elif role is None:
             raise ValueError("Role %r does not exist" % rolename)
         else:
-            func(db, obj, kind, role)
+            func(db, entity, role)
 
     return _check_existence
 
 
 @existing_only
-def add_obj(db, objname, kind, rolename):
-    """Adds a role for users, services, tokens or groups"""
-
-    if kind == 'tokens':
-        log_objname = objname
+def grant_role(db, entity, rolename):
+    """Adds a role for users, services or tokens"""
+    if isinstance(entity, orm.APIToken):
+        entity_repr = entity
     else:
-        log_objname = objname.name
+        entity_repr = entity.name
 
-    if rolename not in objname.roles:
-        objname.roles.append(rolename)
-        db.commit()
-        app_log.info('Adding role %s for %s: %s', rolename.name, kind[:-1], log_objname)
-
-
-@existing_only
-def remove_obj(db, objname, kind, rolename):
-    """Removes a role for users, services or tokens"""
-
-    if kind == 'tokens':
-        log_objname = objname
-    else:
-        log_objname = objname.name
-
-    if rolename in objname.roles:
-        objname.roles.remove(rolename)
+    if rolename not in entity.roles:
+        entity.roles.append(rolename)
         db.commit()
         app_log.info(
-            'Removing role %s for %s: %s', rolename.name, kind[:-1], log_objname
+            'Adding role %s for %s: %s',
+            rolename.name,
+            type(entity).__name__,
+            entity_repr,
         )
 
 
-def _switch_default_role(db, obj, kind, admin):
-    """Switch between default user and admin roles for users/services"""
+@existing_only
+def strip_role(db, entity, rolename):
+    """Removes a role for users, services or tokens"""
+    if isinstance(entity, orm.APIToken):
+        entity_repr = entity
+    else:
+        entity_repr = entity.name
+    if rolename in entity.roles:
+        entity.roles.remove(rolename)
+        db.commit()
+        app_log.info(
+            'Removing role %s for %s: %s',
+            rolename.name,
+            type(entity).__name__,
+            entity_repr,
+        )
 
+
+def _switch_default_role(db, obj, admin):
+    """Switch between default user/service and admin roles for users/services"""
     user_role = orm.Role.find(db, 'user')
-    # temporary fix of default service role
-    if kind == 'services':
-        user_role = orm.Role.find(db, 'service')
-
     admin_role = orm.Role.find(db, 'admin')
 
-    def _add_and_remove(db, obj, kind, current_role, new_role):
-
+    def add_and_remove(db, obj, current_role, new_role):
         if current_role in obj.roles:
-            remove_obj(db, objname=obj.name, kind=kind, rolename=current_role.name)
+            strip_role(db, entity=obj, rolename=current_role.name)
         # only add new default role if the user has no other roles
         if len(obj.roles) < 1:
-            add_obj(db, objname=obj.name, kind=kind, rolename=new_role.name)
+            grant_role(db, entity=obj, rolename=new_role.name)
 
     if admin:
-        _add_and_remove(db, obj, kind, user_role, admin_role)
+        add_and_remove(db, obj, user_role, admin_role)
     else:
-        _add_and_remove(db, obj, kind, admin_role, user_role)
+        add_and_remove(db, obj, admin_role, user_role)
 
 
 def _token_allowed_role(db, token, role):
@@ -364,7 +354,7 @@ def _token_allowed_role(db, token, role):
 
     standard_permissions = {'all', 'read:all'}
 
-    token_scopes = get_subscopes(role)
+    token_scopes = _get_subscopes(role)
     extra_scopes = token_scopes - standard_permissions
     # ignore horizontal filters
     raw_extra_scopes = {
@@ -382,7 +372,7 @@ def _token_allowed_role(db, token, role):
         raw_owner_scopes = {
             scope.split('!', 1)[0] if '!' in scope else scope for scope in owner_scopes
         }
-        if (raw_extra_scopes).issubset(raw_owner_scopes):
+        if raw_extra_scopes.issubset(raw_owner_scopes):
             return True
         else:
             return False
@@ -390,51 +380,50 @@ def _token_allowed_role(db, token, role):
         raise ValueError('Owner the token %r not found', token)
 
 
-def update_roles(db, obj, kind, roles=None):
-    """Updates object's roles if specified,
-    assigns default if no roles specified"""
-
-    Class = orm.get_class(kind)
+def assign_default_roles(db, entity):
+    """Assigns the default roles to an entity:
+    users and services get 'user' role, or admin role if they have admin flag
+    Tokens get 'token' role"""
     default_token_role = orm.Role.find(db, 'token')
-    if roles:
-        for rolename in roles:
-            if Class == orm.APIToken:
-                role = orm.Role.find(db, rolename)
-                if role:
-                    app_log.debug(
-                        'Checking token permissions against requested role %s', rolename
-                    )
-                    if _token_allowed_role(db, obj, role):
-                        role.tokens.append(obj)
-                        app_log.info(
-                            'Adding role %s for %s: %s', role.name, kind[:-1], obj
-                        )
-                    else:
-                        raise ValueError(
-                            'Requested token role %r of %r has more permissions than the token owner',
-                            rolename,
-                            obj,
-                        )
-                else:
-                    raise NameError('Requested role %r does not exist' % rolename)
-            else:
-                add_obj(db, objname=obj.name, kind=kind, rolename=rolename)
+    if isinstance(entity, orm.Group):
+        pass
+    elif isinstance(entity, orm.APIToken):
+        app_log.debug('Assigning default roles to tokens')
+        if not entity.roles and (entity.user or entity.service) is not None:
+            default_token_role.tokens.append(entity)
+            app_log.info('Added role %s to token %s', default_token_role.name, entity)
+        db.commit()
+    # users and services can have 'user' or 'admin' roles as default
     else:
-        # groups can be without a role
-        if Class == orm.Group:
-            pass
-        # tokens can have only 'token' role as default
-        # assign the default only for tokens
-        elif Class == orm.APIToken:
-            app_log.debug('Assigning default roles to tokens')
-            if not obj.roles and obj.user is not None:
-                default_token_role.tokens.append(obj)
-                app_log.info('Added role %s to token %s', default_token_role.name, obj)
-            db.commit()
-        # users and services can have 'user' or 'admin' roles as default
+        # todo: when we deprecate admin flag: replace with role check
+        app_log.debug('Assigning default roles to %s', type(entity).__name__)
+        _switch_default_role(db, entity, entity.admin)
+
+
+def update_roles(db, entity, roles):
+    """Updates object's roles"""
+    standard_permissions = {'all', 'read:all'}
+    for rolename in roles:
+        if isinstance(entity, orm.APIToken):
+            role = orm.Role.find(db, rolename)
+            if role:
+                app_log.debug(
+                    'Checking token permissions against requested role %s', rolename
+                )
+                if _token_allowed_role(db, entity, role):
+                    role.tokens.append(entity)
+                    app_log.info('Adding role %s for token: %s', role.name, entity)
+                else:
+                    raise ValueError(
+                        'Requested token role %r of %r has more permissions than the token owner',
+                        rolename,
+                        entity,
+                    )
+            else:
+                raise NameError('Role %r does not exist' % rolename)
         else:
-            app_log.debug('Assigning default roles to %s', kind)
-            _switch_default_role(db, obj, kind, obj.admin)
+            app_log.debug('Assigning default roles to %s', type(entity).__name__)
+            grant_role(db, entity=entity, rolename=rolename)
 
 
 def add_predef_roles_tokens(db, predef_roles):
@@ -453,7 +442,7 @@ def add_predef_roles_tokens(db, predef_roles):
                         % (token_name, token_role.name)
                     )
                 else:
-                    update_roles(db, obj=token, kind='tokens', roles=[token_role.name])
+                    update_roles(db, token, roles=[token_role.name])
 
 
 def check_for_default_roles(db, bearer):
@@ -471,7 +460,7 @@ def check_for_default_roles(db, bearer):
             .group_by(Class.id)
             .having(func.count(orm.Role.id) == 0)
         ):
-            update_roles(db, obj=obj, kind=bearer)
+            assign_default_roles(db, obj)
     db.commit()
 
 
@@ -481,6 +470,6 @@ def mock_roles(app, name, kind):
     obj = Class.find(app.db, name=name)
     default_roles = get_default_roles()
     for role in default_roles:
-        add_role(app.db, role)
+        create_role(app.db, role)
     app_log.info('Assigning default roles to mocked %s: %s', kind[:-1], name)
-    update_roles(db=app.db, obj=obj, kind=kind)
+    assign_default_roles(db=app.db, entity=obj)

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/services/service.py

@@ -51,6 +51,7 @@ from traitlets import Dict
 from traitlets import HasTraits
 from traitlets import Instance
 from traitlets import Unicode
+from traitlets import validate
 from traitlets.config import LoggingConfigurable
 
 from .. import orm
@@ -284,6 +285,15 @@ class Service(LoggingConfigurable):
     def _default_client_id(self):
         return 'service-%s' % self.name
 
+    @validate("oauth_client_id")
+    def _validate_client_id(self, proposal):
+        if not proposal.value.startswith("service-"):
+            raise ValueError(
+                f"service {self.name} has oauth_client_id='{proposal.value}'."
+                " Service oauth client ids must start with 'service-'"
+            )
+        return proposal.value
+
     oauth_redirect_uri = Unicode(
         help="""OAuth redirect URI for this service.
 

jupyterhub/singleuser/mixins.py

@@ -12,7 +12,9 @@ import asyncio
 import json
 import os
 import random
+import secrets
 import warnings
+from datetime import datetime
 from datetime import timezone
 from textwrap import dedent
 from urllib.parse import urlparse
@@ -251,7 +253,7 @@ class SingleUserNotebookAppMixin(Configurable):
     cookie_secret = Bytes()
 
     def _cookie_secret_default(self):
-        return os.urandom(32)
+        return secrets.token_bytes(32)
 
     user = CUnicode().tag(config=True)
     group = CUnicode().tag(config=True)

jupyterhub/tests/conftest.py

@@ -125,7 +125,11 @@ def db():
     """Get a db session"""
     global _db
     if _db is None:
-        _db = orm.new_session_factory('sqlite:///:memory:')()
+        # make sure some initial db contents are filled out
+        # specifically, the 'default' jupyterhub oauth client
+        app = MockHub(db_url='sqlite:///:memory:')
+        app.init_db()
+        _db = app.db
         user = orm.User(name=getuser())
         _db.add(user)
         _db.commit()
@@ -164,9 +168,14 @@ def cleanup_after(request, io_loop):
     allows cleanup of servers between tests
     without having to launch a whole new app
     """
+
     try:
         yield
     finally:
+        if _db is not None:
+            # cleanup after failed transactions
+            _db.rollback()
+
         if not MockHub.initialized():
             return
         app = MockHub.instance()
@@ -251,7 +260,7 @@ def _mockservice(request, app, url=False):
         assert name in app._service_map
         service = app._service_map[name]
         token = service.orm.api_tokens[0]
-        update_roles(app.db, token, 'tokens', roles=['token'])
+        update_roles(app.db, token, roles=['token'])
 
         async def start():
             # wait for proxy to be updated before starting the service

jupyterhub/tests/mocking.py

@@ -342,7 +342,7 @@ class MockHub(JupyterHub):
             self.db.add(user)
             self.db.commit()
             metrics.TOTAL_USERS.inc()
-        roles.update_roles(self.db, obj=user, kind='users')
+        roles.assign_default_roles(self.db, entity=user)
         self.db.commit()
 
     def stop(self):

jupyterhub/tests/populate_db.py

@@ -6,6 +6,7 @@ used in test_db.py
 """
 import os
 from datetime import datetime
+from functools import partial
 
 import jupyterhub
 from jupyterhub import orm
@@ -62,32 +63,35 @@ def populate_db(url):
         db.commit()
 
     # create some oauth objects
-    if jupyterhub.version_info >= (0, 8):
-        # create oauth client
-        client = orm.OAuthClient(identifier='oauth-client')
-        db.add(client)
-        db.commit()
-        code = orm.OAuthCode(client_id=client.identifier)
-        db.add(code)
-        db.commit()
-        access_token = orm.OAuthAccessToken(
-            client_id=client.identifier,
-            user_id=user.id,
+    client = orm.OAuthClient(identifier='oauth-client')
+    db.add(client)
+    db.commit()
+    code = orm.OAuthCode(client_id=client.identifier)
+    db.add(code)
+    db.commit()
+    if jupyterhub.version_info < (2, 0):
+        Token = partial(
+            orm.OAuthAccessToken,
             grant_type=orm.GrantType.authorization_code,
         )
-        db.add(access_token)
-        db.commit()
+    else:
+        Token = orm.APIToken
+    access_token = Token(
+        client_id=client.identifier,
+        user_id=user.id,
+    )
+    db.add(access_token)
+    db.commit()
 
     # set some timestamps added in 0.9
-    if jupyterhub.version_info >= (0, 9):
-        assert user.created
-        assert admin.created
-        # set last_activity
-        user.last_activity = datetime.utcnow()
-        spawner = user.orm_spawners['']
-        spawner.started = datetime.utcnow()
-        spawner.last_activity = datetime.utcnow()
-        db.commit()
+    assert user.created
+    assert admin.created
+    # set last_activity
+    user.last_activity = datetime.utcnow()
+    spawner = user.orm_spawners['']
+    spawner.started = datetime.utcnow()
+    spawner.last_activity = datetime.utcnow()
+    db.commit()
 
 
 if __name__ == '__main__':

jupyterhub/tests/test_api.py

@@ -179,9 +179,12 @@ async def test_get_users(app):
         'admin': False,
         'roles': ['user'],
         'last_activity': None,
+        'auth_state': None,
     }
     assert users == [
-        fill_user({'name': 'admin', 'admin': True, 'roles': ['admin']}),
+        fill_user(
+            {'name': 'admin', 'admin': True, 'roles': ['admin'], 'auth_state': None}
+        ),
         fill_user(user_model),
     ]
     r = await api_request(app, 'users', headers=auth_header(db, 'user'))
@@ -270,11 +273,10 @@ async def test_get_self(app):
     oauth_client = orm.OAuthClient(identifier='eurydice')
     db.add(oauth_client)
     db.commit()
-    oauth_token = orm.OAuthAccessToken(
+    oauth_token = orm.APIToken(
         user=u.orm_user,
         client=oauth_client,
         token=token,
-        grant_type=orm.GrantType.authorization_code,
     )
     db.add(oauth_token)
     db.commit()
@@ -1420,12 +1422,11 @@ async def test_token_list(app, as_user, for_user, status):
     if status != 200:
         return
     reply = r.json()
-    assert sorted(reply) == ['api_tokens', 'oauth_tokens']
+    assert sorted(reply) == ['api_tokens']
     assert len(reply['api_tokens']) == len(for_user_obj.api_tokens)
     assert all(token['user'] == for_user for token in reply['api_tokens'])
-    assert all(token['user'] == for_user for token in reply['oauth_tokens'])
     # validate individual token ids
-    for token in reply['api_tokens'] + reply['oauth_tokens']:
+    for token in reply['api_tokens']:
         r = await api_request(
             app, 'users', for_user, 'tokens', token['id'], headers=headers
         )

jupyterhub/tests/test_app.py

@@ -50,7 +50,7 @@ def test_raise_error_on_missing_specified_config():
     process = Popen(
         [sys.executable, '-m', 'jupyterhub', '--config', 'not-available.py']
     )
-    # wait inpatiently for the process to exit like we want it to
+    # wait impatiently for the process to exit like we want it to
     for i in range(100):
         time.sleep(0.1)
         returncode = process.poll()

jupyterhub/tests/test_db.py

@@ -36,7 +36,7 @@ def generate_old_db(env_dir, hub_version, db_url):
     check_call([env_py, populate_db, db_url])
 
 
-@pytest.mark.parametrize('hub_version', ['0.7.2', '0.8.1', '0.9.4'])
+@pytest.mark.parametrize('hub_version', ['1.0.0', "1.2.2", "1.3.0"])
 async def test_upgrade(tmpdir, hub_version):
     db_url = os.getenv('JUPYTERHUB_TEST_DB_URL')
     if db_url:

jupyterhub/tests/test_named_servers.py

@@ -52,7 +52,6 @@ async def test_default_server(app, named_servers):
     r.raise_for_status()
 
     user_model = normalize_user(r.json())
-    print(user_model)
     assert user_model == fill_user(
         {
             'name': username,

jupyterhub/tests/test_orm.py

@@ -355,8 +355,9 @@ def test_user_delete_cascade(db):
     spawner.server = server = orm.Server()
     oauth_code = orm.OAuthCode(client=oauth_client, user=user)
     db.add(oauth_code)
-    oauth_token = orm.OAuthAccessToken(
-        client=oauth_client, user=user, grant_type=orm.GrantType.authorization_code
+    oauth_token = orm.APIToken(
+        client=oauth_client,
+        user=user,
     )
     db.add(oauth_token)
     db.commit()
@@ -377,7 +378,7 @@ def test_user_delete_cascade(db):
     assert_not_found(db, orm.Spawner, spawner_id)
     assert_not_found(db, orm.Server, server_id)
     assert_not_found(db, orm.OAuthCode, oauth_code_id)
-    assert_not_found(db, orm.OAuthAccessToken, oauth_token_id)
+    assert_not_found(db, orm.APIToken, oauth_token_id)
 
 
 def test_oauth_client_delete_cascade(db):
@@ -391,12 +392,13 @@ def test_oauth_client_delete_cascade(db):
     # these should all be deleted automatically when the user goes away
     oauth_code = orm.OAuthCode(client=oauth_client, user=user)
     db.add(oauth_code)
-    oauth_token = orm.OAuthAccessToken(
-        client=oauth_client, user=user, grant_type=orm.GrantType.authorization_code
+    oauth_token = orm.APIToken(
+        client=oauth_client,
+        user=user,
     )
     db.add(oauth_token)
     db.commit()
-    assert user.oauth_tokens == [oauth_token]
+    assert user.api_tokens == [oauth_token]
 
     # record all of the ids
     oauth_code_id = oauth_code.id
@@ -408,8 +410,8 @@ def test_oauth_client_delete_cascade(db):
 
     # verify that everything gets deleted
     assert_not_found(db, orm.OAuthCode, oauth_code_id)
-    assert_not_found(db, orm.OAuthAccessToken, oauth_token_id)
-    assert user.oauth_tokens == []
+    assert_not_found(db, orm.APIToken, oauth_token_id)
+    assert user.api_tokens == []
     assert user.oauth_codes == []
 
 
@@ -510,32 +512,31 @@ def test_expiring_api_token(app, user):
 def test_expiring_oauth_token(app, user):
     db = app.db
     token = "abc123"
-    now = orm.OAuthAccessToken.now
+    now = orm.APIToken.now
     client = orm.OAuthClient(identifier="xxx", secret="yyy")
     db.add(client)
-    orm_token = orm.OAuthAccessToken(
+    orm_token = orm.APIToken(
         token=token,
-        grant_type=orm.GrantType.authorization_code,
         client=client,
         user=user,
-        expires_at=now() + 30,
+        expires_at=now() + timedelta(seconds=30),
     )
     db.add(orm_token)
     db.commit()
 
-    found = orm.OAuthAccessToken.find(db, token)
+    found = orm.APIToken.find(db, token)
     assert found is orm_token
     # purge_expired doesn't delete non-expired
-    orm.OAuthAccessToken.purge_expired(db)
-    found = orm.OAuthAccessToken.find(db, token)
+    orm.APIToken.purge_expired(db)
+    found = orm.APIToken.find(db, token)
     assert found is orm_token
 
-    with mock.patch.object(orm.OAuthAccessToken, 'now', lambda: now() + 60):
-        found = orm.OAuthAccessToken.find(db, token)
+    with mock.patch.object(orm.APIToken, 'now', lambda: now() + timedelta(seconds=60)):
+        found = orm.APIToken.find(db, token)
         assert found is None
-        assert orm_token in db.query(orm.OAuthAccessToken)
-        orm.OAuthAccessToken.purge_expired(db)
-        assert orm_token not in db.query(orm.OAuthAccessToken)
+        assert orm_token in db.query(orm.APIToken)
+        orm.APIToken.purge_expired(db)
+        assert orm_token not in db.query(orm.APIToken)
 
 
 def test_expiring_oauth_code(app, user):

jupyterhub/tests/test_pages.py

@@ -869,8 +869,9 @@ async def test_oauth_token_page(app):
     user = app.users[orm.User.find(app.db, name)]
     client = orm.OAuthClient(identifier='token')
     app.db.add(client)
-    oauth_token = orm.OAuthAccessToken(
-        client=client, user=user, grant_type=orm.GrantType.authorization_code
+    oauth_token = orm.APIToken(
+        client=client,
+        user=user,
     )
     app.db.add(oauth_token)
     app.db.commit()

jupyterhub/tests/test_roles.py

@@ -10,6 +10,7 @@ from tornado.log import app_log
 
 from .. import orm
 from .. import roles
+from ..scopes import get_scopes_for
 from ..utils import maybe_future
 from .mocking import MockHub
 from .utils import add_user
@@ -207,9 +208,9 @@ def test_orm_roles_delete_cascade(db):
 )
 def test_get_subscopes(db, scopes, subscopes):
     """Test role scopes expansion into their subscopes"""
-    roles.add_role(db, {'name': 'testing_scopes', 'scopes': scopes})
+    roles.create_role(db, {'name': 'testing_scopes', 'scopes': scopes})
     role = orm.Role.find(db, name='testing_scopes')
-    response = roles.get_subscopes(role)
+    response = roles._get_subscopes(role)
     assert response == subscopes
     db.delete(role)
 
@@ -245,6 +246,16 @@ async def test_load_default_roles(tmpdir, request):
             'info',
             app_log.info('Role new-role added to database'),
         ),
+        (
+            'the-same-role',
+            {
+                'name': 'new-role',
+                'description': 'Some description',
+                'scopes': ['groups'],
+            },
+            'no-log',
+            None,
+        ),
         ('no_name', {'scopes': ['users']}, 'error', KeyError),
         (
             'no_scopes',
@@ -270,28 +281,28 @@ async def test_load_default_roles(tmpdir, request):
             'info',
             app_log.info('Role user scopes attribute has been changed'),
         ),
+        # rewrite the user role back to 'default'
+        (
+            'user',
+            {'name': 'user', 'scopes': ['self']},
+            'info',
+            app_log.info('Role user scopes attribute has been changed'),
+        ),
     ],
 )
-async def test_adding_new_roles(
-    tmpdir, request, role, role_def, response_type, response
-):
-    """Test raising errors and warnings when creating new roles"""
+async def test_creating_roles(app, role, role_def, response_type, response):
+    """Test raising errors and warnings when creating/modifying roles"""
 
-    kwargs = {'load_roles': [role_def]}
-    ssl_enabled = getattr(request.module, "ssl_enabled", False)
-    if ssl_enabled:
-        kwargs['internal_certs_location'] = str(tmpdir)
-    hub = MockHub(**kwargs)
-    hub.init_db()
-    db = hub.db
+    db = app.db
 
     if response_type == 'error':
         with pytest.raises(response):
-            await hub.init_roles()
+            roles.create_role(db, role_def)
 
     elif response_type == 'warning' or response_type == 'info':
         with pytest.warns(response):
-            await hub.init_roles()
+            roles.create_role(db, role_def)
+        # check the role has been created/modified
         role = orm.Role.find(db, role_def['name'])
         assert role is not None
         if 'description' in role_def.keys():
@@ -299,6 +310,14 @@ async def test_adding_new_roles(
         if 'scopes' in role_def.keys():
             assert role.scopes == role_def['scopes']
 
+    # make sure no warnings/info logged when the role exists and its definition hasn't been changed
+    elif response_type == 'no-log':
+        with pytest.warns(response) as record:
+            roles.create_role(db, role_def)
+        assert not record.list
+        role = orm.Role.find(db, role_def['name'])
+        assert role is not None
+
 
 @mark.role
 @mark.parametrize(
@@ -326,13 +345,13 @@ async def test_delete_roles(db, role_type, rolename, response_type, response):
         assert check_role is not None
         # check the role is deleted and info raised
         with pytest.warns(response):
-            roles.remove_role(db, rolename)
+            roles.delete_role(db, rolename)
         check_role = orm.Role.find(db, rolename)
         assert check_role is None
 
     elif response_type == 'error':
         with pytest.raises(response):
-            roles.remove_role(db, rolename)
+            roles.delete_role(db, rolename)
 
 
 @mark.role
@@ -367,20 +386,20 @@ async def test_scope_existence(tmpdir, request, role, response):
     db = hub.db
 
     if response == 'existing':
-        roles.add_role(db, role)
+        roles.create_role(db, role)
         added_role = orm.Role.find(db, role['name'])
         assert added_role is not None
         assert added_role.scopes == role['scopes']
 
     elif response == NameError:
         with pytest.raises(response):
-            roles.add_role(db, role)
+            roles.create_role(db, role)
         added_role = orm.Role.find(db, role['name'])
         assert added_role is None
 
     # delete the tested roles
     if added_role:
-        roles.remove_role(db, added_role.name)
+        roles.delete_role(db, added_role.name)
 
 
 @mark.role
@@ -427,7 +446,7 @@ async def test_load_roles_users(tmpdir, request):
 
     # delete the test roles
     for role in roles_to_load:
-        roles.remove_role(db, role['name'])
+        roles.delete_role(db, role['name'])
 
 
 @mark.role
@@ -476,13 +495,11 @@ async def test_load_roles_services(tmpdir, request):
     # test if every service has a role (and no duplicates)
     admin_role = orm.Role.find(db, name='admin')
     user_role = orm.Role.find(db, name='user')
-    service_role = orm.Role.find(db, name='service')
 
     # test if predefined roles loaded and assigned
     culler_role = orm.Role.find(db, name='idle-culler')
     culler_service = orm.Service.find(db, name='idle-culler')
     assert culler_role in culler_service.roles
-    assert service_role not in culler_service.roles
 
     # test if every service has a role (and no duplicates)
     for service in db.query(orm.Service):
@@ -492,13 +509,10 @@ async def test_load_roles_services(tmpdir, request):
         # test default role assignment
         if service.admin:
             assert admin_role in service.roles
-            assert service_role not in service.roles
-        elif culler_role not in service.roles:
-            assert service_role in service.roles
-            assert service_role.scopes == []
-            assert admin_role not in service.roles
-            # make sure 'user' role not assigned to service
             assert user_role not in service.roles
+        elif culler_role not in service.roles:
+            assert user_role in service.roles
+            assert admin_role not in service.roles
 
     # delete the test services
     for service in db.query(orm.Service):
@@ -512,7 +526,7 @@ async def test_load_roles_services(tmpdir, request):
 
     # delete the test roles
     for role in roles_to_load:
-        roles.remove_role(db, role['name'])
+        roles.delete_role(db, role['name'])
 
 
 @mark.role
@@ -561,7 +575,7 @@ async def test_load_roles_groups(tmpdir, request):
 
     # delete the test roles
     for role in roles_to_load:
-        roles.remove_role(db, role['name'])
+        roles.delete_role(db, role['name'])
 
 
 @mark.role
@@ -614,7 +628,7 @@ async def test_load_roles_user_tokens(tmpdir, request):
 
     # delete the test roles
     for role in roles_to_load:
-        roles.remove_role(db, role['name'])
+        roles.delete_role(db, role['name'])
 
 
 @mark.role
@@ -657,12 +671,14 @@ async def test_load_roles_user_tokens_not_allowed(tmpdir, request):
 
     # delete the test roles
     for role in roles_to_load:
-        roles.remove_role(db, role['name'])
+        roles.delete_role(db, role['name'])
 
 
 @mark.role
 async def test_load_roles_service_tokens(tmpdir, request):
-    services = [{'name': 'idle-culler', 'api_token': 'another-secret-token'}]
+    services = [
+        {'name': 'idle-culler', 'api_token': 'another-secret-token'},
+    ]
     service_tokens = {
         'another-secret-token': 'idle-culler',
     }
@@ -713,7 +729,7 @@ async def test_load_roles_service_tokens(tmpdir, request):
 
     # delete the test roles
     for role in roles_to_load:
-        roles.remove_role(db, role['name'])
+        roles.delete_role(db, role['name'])
 
 
 @mark.role
@@ -769,7 +785,7 @@ async def test_load_roles_service_tokens_not_allowed(tmpdir, request):
 
     # delete the test roles
     for role in roles_to_load:
-        roles.remove_role(db, role['name'])
+        roles.delete_role(db, role['name'])
 
 
 @mark.role
@@ -793,10 +809,10 @@ async def test_get_new_token_via_api(app, headers, rolename, scopes, status):
 
     user = add_user(app.db, app, name='user')
     if rolename and rolename != 'non-existing':
-        roles.add_role(app.db, {'name': rolename, 'scopes': scopes})
+        roles.create_role(app.db, {'name': rolename, 'scopes': scopes})
         if rolename == 'groups-reader':
             # add role for a group
-            roles.add_role(app.db, {'name': 'group-role', 'scopes': ['groups']})
+            roles.create_role(app.db, {'name': 'group-role', 'scopes': ['groups']})
             # create a group and add the user and group_role
             group = orm.Group.find(app.db, 'test-group')
             if not group:
@@ -833,3 +849,32 @@ async def test_get_new_token_via_api(app, headers, rolename, scopes, status):
     # verify deletion
     r = await api_request(app, 'users/user/tokens', token_id)
     assert r.status_code == 404
+
+
+@mark.role
+@mark.parametrize(
+    "kind, has_user_scopes",
+    [
+        ('users', True),
+        ('services', False),
+    ],
+)
+async def test_self_expansion(app, kind, has_user_scopes):
+    Class = orm.get_class(kind)
+    orm_obj = Class(name=f'test_{kind}')
+    app.db.add(orm_obj)
+    app.db.commit()
+    test_role = orm.Role(name='test_role', scopes=['self'])
+    orm_obj.roles.append(test_role)
+    # test expansion of user/service scopes
+    scopes = roles.expand_roles_to_scopes(orm_obj)
+    assert bool(scopes) == has_user_scopes
+
+    # test expansion of token scopes
+    orm_obj.new_api_token()
+    print(orm_obj.api_tokens[0])
+    token_scopes = get_scopes_for(orm_obj.api_tokens[0])
+    print(token_scopes)
+    assert bool(token_scopes) == has_user_scopes
+    app.db.delete(orm_obj)
+    app.db.delete(test_role)

jupyterhub/tests/test_scopes.py

@@ -11,6 +11,7 @@ from .. import orm
 from .. import roles
 from ..handlers import BaseHandler
 from ..scopes import _check_scope
+from ..scopes import get_scopes_for
 from ..scopes import needs_scope
 from ..scopes import parse_scopes
 from ..scopes import Scope
@@ -88,6 +89,10 @@ class MockAPIHandler:
         self.request = mock.Mock(spec=HTTPServerRequest)
         self.request.path = '/path'
 
+    def set_scopes(self, *scopes):
+        self.raw_scopes = set(scopes)
+        self.parsed_scopes = parse_scopes(self.raw_scopes)
+
     @needs_scope('users')
     def user_thing(self, user_name):
         return True
@@ -115,6 +120,12 @@ class MockAPIHandler:
         return True
 
 
+@pytest.fixture
+def mock_handler():
+    obj = MockAPIHandler()
+    return obj
+
+
 @mark.parametrize(
     "scopes, method, arguments, is_allowed",
     [
@@ -168,12 +179,10 @@ class MockAPIHandler:
         (['users!user=gob'], 'other_thing', ('maeby',), True),
     ],
 )
-def test_scope_method_access(scopes, method, arguments, is_allowed):
-    obj = MockAPIHandler()
-    obj.current_user = mock.Mock(name=arguments[0])
-    obj.raw_scopes = set(scopes)
-    obj.parsed_scopes = parse_scopes(obj.raw_scopes)
-    api_call = getattr(obj, method)
+def test_scope_method_access(mock_handler, scopes, method, arguments, is_allowed):
+    mock_handler.current_user = mock.Mock(name=arguments[0])
+    mock_handler.set_scopes(*scopes)
+    api_call = getattr(mock_handler, method)
     if is_allowed:
         assert api_call(*arguments)
     else:
@@ -181,31 +190,18 @@ def test_scope_method_access(scopes, method, arguments, is_allowed):
             api_call(*arguments)
 
 
-def test_double_scoped_method_succeeds():
-    obj = MockAPIHandler()
-    obj.current_user = mock.Mock(name='lucille')
-    obj.raw_scopes = {'users', 'read:services'}
-    obj.parsed_scopes = parse_scopes(obj.raw_scopes)
-    assert obj.secret_thing()
+def test_double_scoped_method_succeeds(mock_handler):
+    mock_handler.current_user = mock.Mock(name='lucille')
+    mock_handler.set_scopes('users', 'read:services')
+    mock_handler.parsed_scopes = parse_scopes(mock_handler.raw_scopes)
+    assert mock_handler.secret_thing()
 
 
-def test_double_scoped_method_denials():
-    obj = MockAPIHandler()
-    obj.current_user = mock.Mock(name='lucille2')
-    obj.raw_scopes = {'users', 'read:groups'}
-    obj.parsed_scopes = parse_scopes(obj.raw_scopes)
+def test_double_scoped_method_denials(mock_handler):
+    mock_handler.current_user = mock.Mock(name='lucille2')
+    mock_handler.set_scopes('users', 'read:groups')
     with pytest.raises(web.HTTPError):
-        obj.secret_thing()
-
-
-def generate_test_role(user_name, scopes, role_name='test'):
-    role = {
-        'name': role_name,
-        'description': '',
-        'users': [user_name],
-        'scopes': scopes,
-    }
-    return role
+        mock_handler.secret_thing()
 
 
 @mark.parametrize(
@@ -229,7 +225,7 @@ async def test_expand_groups(app, user_name, in_group, status_code):
             'read:groups',
         ],
     }
-    roles.add_role(app.db, test_role)
+    roles.create_role(app.db, test_role)
     user = add_user(app.db, name=user_name)
     group_name = 'bluth'
     group = orm.Group.find(app.db, name=group_name)
@@ -238,14 +234,15 @@ async def test_expand_groups(app, user_name, in_group, status_code):
         app.db.add(group)
     if in_group and user not in group.users:
         group.users.append(user)
-    kind = 'users'
-    roles.update_roles(app.db, user, kind, roles=['test'])
-    roles.remove_obj(app.db, user_name, kind, 'user')
+    roles.update_roles(app.db, user, roles=['test'])
+    roles.strip_role(app.db, user, 'user')
     app.db.commit()
     r = await api_request(
         app, 'users', user_name, headers=auth_header(app.db, user_name)
     )
     assert r.status_code == status_code
+    app.db.delete(group)
+    app.db.commit()
 
 
 async def test_by_fake_user(app):
@@ -261,81 +258,127 @@ async def test_by_fake_user(app):
 err_message = "No access to resources or resources not found"
 
 
-async def test_request_fake_user(app):
-    user_name = 'buster'
-    fake_user = 'annyong'
-    add_user(app.db, name=user_name)
-    test_role = generate_test_role(user_name, ['read:users!group=stuff'])
-    roles.add_role(app.db, test_role)
-    roles.add_obj(app.db, objname=user_name, kind='users', rolename='test')
+@pytest.fixture
+def create_temp_role(app):
+    """Generate a temporary role with certain scopes.
+    Convenience function that provides setup, database handling and teardown"""
+    temp_roles = []
+    index = [1]
+
+    def temp_role_creator(scopes, role_name=None):
+        if not role_name:
+            role_name = f'temp_role_{index[0]}'
+            index[0] += 1
+        temp_role = orm.Role(name=role_name, scopes=list(scopes))
+        temp_roles.append(temp_role)
+        app.db.add(temp_role)
+        app.db.commit()
+        return temp_role
+
+    yield temp_role_creator
+    for role in temp_roles:
+        app.db.delete(role)
     app.db.commit()
+
+
+@pytest.fixture
+def create_user_with_scopes(app, create_temp_role):
+    """Generate a temporary user with specific scopes.
+    Convenience function that provides setup, database handling and teardown"""
+    temp_users = []
+    counter = 0
+    get_role = create_temp_role
+
+    def temp_user_creator(*scopes):
+        nonlocal counter
+        counter += 1
+        name = f"temp_user_{counter}"
+        role = get_role(scopes)
+        orm_user = orm.User(name=name)
+        app.db.add(orm_user)
+        app.db.commit()
+        temp_users.append(orm_user)
+        roles.update_roles(app.db, orm_user, roles=[role.name])
+        return app.users[orm_user.id]
+
+    yield temp_user_creator
+    for user in temp_users:
+        app.users.delete(user)
+
+
+@pytest.fixture
+def create_service_with_scopes(app, create_temp_role):
+    """Generate a temporary service with specific scopes.
+    Convenience function that provides setup, database handling and teardown"""
+    temp_service = []
+    counter = 0
+    role_function = create_temp_role
+
+    def temp_service_creator(*scopes):
+        nonlocal counter
+        counter += 1
+        name = f"temp_service_{counter}"
+        role = role_function(scopes)
+        app.services.append({'name': name})
+        app.init_services()
+        orm_service = orm.Service.find(app.db, name)
+        app.db.commit()
+        roles.update_roles(app.db, orm_service, roles=[role.name])
+        return orm_service
+
+    yield temp_service_creator
+    for service in temp_service:
+        app.db.delete(service)
+    app.db.commit()
+
+
+async def test_request_fake_user(app, create_user_with_scopes):
+    fake_user = 'annyong'
+    user = create_user_with_scopes('read:users!group=stuff')
     r = await api_request(
-        app, 'users', fake_user, headers=auth_header(app.db, user_name)
+        app, 'users', fake_user, headers=auth_header(app.db, user.name)
     )
     assert r.status_code == 404
     # Consistency between no user and user not accessible
     assert r.json()['message'] == err_message
 
 
-async def test_refuse_exceeding_token_permissions(app):
-    user_name = 'abed'
-    user = add_user(app.db, name=user_name)
-    add_user(app.db, name='user')
-    api_token = user.new_api_token()
-    exceeding_role = generate_test_role(user_name, ['read:users'], 'exceeding_role')
-    roles.add_role(app.db, exceeding_role)
-    roles.add_obj(app.db, objname=api_token, kind='tokens', rolename='exceeding_role')
-    app.db.commit()
-    headers = {'Authorization': 'token %s' % api_token}
-    r = await api_request(app, 'users', headers=headers)
-    assert r.status_code == 200
-    result_names = {user['name'] for user in r.json()}
-    assert result_names == {user_name}
+async def test_refuse_exceeding_token_permissions(
+    app, create_user_with_scopes, create_temp_role
+):
+    user = create_user_with_scopes('self')
+    user.new_api_token()
+    create_temp_role(['admin:users'], 'exceeding_role')
+    with pytest.raises(ValueError):
+        roles.update_roles(app.db, entity=user.api_tokens[0], roles=['exceeding_role'])
 
 
-async def test_exceeding_user_permissions(app):
-    user_name = 'abed'
-    user = add_user(app.db, name=user_name)
-    add_user(app.db, name='user')
+async def test_exceeding_user_permissions(
+    app, create_user_with_scopes, create_temp_role
+):
+    user = create_user_with_scopes('read:users:groups')
     api_token = user.new_api_token()
     orm_api_token = orm.APIToken.find(app.db, token=api_token)
-    reader_role = generate_test_role(user_name, ['read:users'], 'reader_role')
-    subreader_role = generate_test_role(
-        user_name, ['read:users:groups'], 'subreader_role'
-    )
-    roles.add_role(app.db, reader_role)
-    roles.add_role(app.db, subreader_role)
-    app.db.commit()
-    roles.update_roles(app.db, user, kind='users', roles=['reader_role'])
-    roles.update_roles(app.db, orm_api_token, kind='tokens', roles=['subreader_role'])
-    orm_api_token.roles.remove(orm.Role.find(app.db, name='token'))
-    app.db.commit()
-
+    create_temp_role(['read:users'], 'reader_role')
+    roles.grant_role(app.db, orm_api_token, rolename='reader_role')
     headers = {'Authorization': 'token %s' % api_token}
     r = await api_request(app, 'users', headers=headers)
     assert r.status_code == 200
     keys = {key for user in r.json() for key in user.keys()}
     assert 'groups' in keys
     assert 'last_activity' not in keys
-    roles.remove_obj(app.db, user_name, 'users', 'reader_role')
 
 
-async def test_user_service_separation(app, mockservice_url):
+async def test_user_service_separation(app, mockservice_url, create_temp_role):
     name = mockservice_url.name
     user = add_user(app.db, name=name)
 
-    reader_role = generate_test_role(name, ['read:users'], 'reader_role')
-    subreader_role = generate_test_role(name, ['read:users:groups'], 'subreader_role')
-    roles.add_role(app.db, reader_role)
-    roles.add_role(app.db, subreader_role)
-    app.db.commit()
-    roles.update_roles(app.db, user, kind='users', roles=['subreader_role'])
-    roles.update_roles(
-        app.db, mockservice_url.orm, kind='services', roles=['reader_role']
-    )
+    create_temp_role(['read:users'], 'reader_role')
+    create_temp_role(['read:users:groups'], 'subreader_role')
+    roles.update_roles(app.db, user, roles=['subreader_role'])
+    roles.update_roles(app.db, mockservice_url.orm, roles=['reader_role'])
     user.roles.remove(orm.Role.find(app.db, name='user'))
     api_token = user.new_api_token()
-    app.db.commit()
     headers = {'Authorization': 'token %s' % api_token}
     r = await api_request(app, 'users', headers=headers)
     assert r.status_code == 200
@@ -344,33 +387,22 @@ async def test_user_service_separation(app, mockservice_url):
     assert 'last_activity' not in keys
 
 
-async def test_request_user_outside_group(app):
-    user_name = 'buster'
-    fake_user = 'hello'
-    add_user(app.db, name=user_name)
-    add_user(app.db, name=fake_user)
-    test_role = generate_test_role(user_name, ['read:users!group=stuff'])
-    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()
+async def test_request_user_outside_group(app, create_user_with_scopes):
+    outside_user = 'hello'
+    user = create_user_with_scopes('read:users!group=stuff')
+    add_user(app.db, name=outside_user)
     r = await api_request(
-        app, 'users', fake_user, headers=auth_header(app.db, user_name)
+        app, 'users', outside_user, headers=auth_header(app.db, user.name)
     )
     assert r.status_code == 404
     # Consistency between no user and user not accessible
     assert r.json()['message'] == err_message
 
 
-async def test_user_filter(app):
-    user_name = 'rita'
-    user = add_user(app.db, name=user_name)
-    app.db.commit()
-    scopes = ['read:users!user=lindsay', 'read:users!user=gob', 'read:users!user=oscar']
-    test_role = generate_test_role(user, scopes)
-    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')
+async def test_user_filter(app, create_user_with_scopes):
+    user = create_user_with_scopes(
+        'read:users!user=lindsay', 'read:users!user=gob', 'read:users!user=oscar'
+    )
     name_in_scope = {'lindsay', 'oscar', 'gob'}
     outside_scope = {'maeby', 'marta'}
     group_name = 'bluth'
@@ -379,17 +411,19 @@ async def test_user_filter(app):
         group = orm.Group(name=group_name)
         app.db.add(group)
     for name in name_in_scope | outside_scope:
-        user = add_user(app.db, name=name)
+        group_user = add_user(app.db, name=name)
         if name not in group.users:
-            group.users.append(user)
+            group.users.append(group_user)
     app.db.commit()
-    r = await api_request(app, 'users', headers=auth_header(app.db, user_name))
+    r = await api_request(app, 'users', headers=auth_header(app.db, user.name))
     assert r.status_code == 200
     result_names = {user['name'] for user in r.json()}
     assert result_names == name_in_scope
+    app.db.delete(group)
+    app.db.commit()
 
 
-async def test_service_filter(app):
+async def test_service_filter(app, create_user_with_scopes):
     services = [
         {'name': 'cull_idle', 'api_token': 'some-token'},
         {'name': 'user_service', 'api_token': 'some-other-token'},
@@ -397,120 +431,210 @@ async def test_service_filter(app):
     for service in services:
         app.services.append(service)
     app.init_services()
-    user_name = 'buster'
-    user = add_user(app.db, name=user_name)
-    app.db.commit()
-    test_role = generate_test_role(user, ['read:services!service=cull_idle'])
-    roles.add_role(app.db, test_role)
-    roles.add_obj(app.db, objname=user_name, kind='users', rolename='test')
-    r = await api_request(app, 'services', headers=auth_header(app.db, user_name))
+    user = create_user_with_scopes('read:services!service=cull_idle')
+    r = await api_request(app, 'services', headers=auth_header(app.db, user.name))
     assert r.status_code == 200
     service_names = set(r.json().keys())
     assert service_names == {'cull_idle'}
 
 
-async def test_user_filter_with_group(app):
-    # Move role setup to setup method?
-    user_name = 'sally'
-    add_user(app.db, name=user_name)
-    external_user_name = 'britta'
-    add_user(app.db, name=external_user_name)
-    test_role = generate_test_role(user_name, ['read:users!group=sitwell'])
-    roles.add_role(app.db, test_role)
-    roles.add_obj(app.db, objname=user_name, kind='users', rolename='test')
-
-    name_set = {'sally', 'stan'}
+async def test_user_filter_with_group(app, create_user_with_scopes):
     group_name = 'sitwell'
+    user1 = create_user_with_scopes(f'read:users!group={group_name}')
+    user2 = create_user_with_scopes('self')
+    external_user = create_user_with_scopes('self')
+    name_set = {user1.name, user2.name}
     group = orm.Group.find(app.db, name=group_name)
     if not group:
         group = orm.Group(name=group_name)
         app.db.add(group)
-    for name in name_set:
-        user = add_user(app.db, name=name)
-        if name not in group.users:
-            group.users.append(user)
+    for user in {user1, user2}:
+        group.users.append(user)
     app.db.commit()
 
-    r = await api_request(app, 'users', headers=auth_header(app.db, user_name))
+    r = await api_request(app, 'users', headers=auth_header(app.db, user1.name))
     assert r.status_code == 200
     result_names = {user['name'] for user in r.json()}
     assert result_names == name_set
-    assert external_user_name not in result_names
+    assert external_user.name not in result_names
+    app.db.delete(group)
+    app.db.commit()
 
 
-async def test_group_scope_filter(app):
-    user_name = 'rollerblade'
-    add_user(app.db, name=user_name)
-    scopes = ['read:groups!group=sitwell', 'read:groups!group=bluth']
-    test_role = generate_test_role(user_name, scopes)
-    roles.add_role(app.db, test_role)
-    roles.add_obj(app.db, objname=user_name, kind='users', rolename='test')
-
-    group_set = {'sitwell', 'bluth', 'austero'}
-    for group_name in group_set:
+async def test_group_scope_filter(app, create_user_with_scopes):
+    in_groups = {'sitwell', 'bluth'}
+    out_groups = {'austero'}
+    user = create_user_with_scopes(
+        *(f'read:groups!group={group}' for group in in_groups)
+    )
+    for group_name in in_groups | out_groups:
         group = orm.Group.find(app.db, name=group_name)
         if not group:
             group = orm.Group(name=group_name)
             app.db.add(group)
     app.db.commit()
-    r = await api_request(app, 'groups', headers=auth_header(app.db, user_name))
+    r = await api_request(app, 'groups', headers=auth_header(app.db, user.name))
     assert r.status_code == 200
     result_names = {user['name'] for user in r.json()}
-    assert result_names == {'sitwell', 'bluth'}
-
-
-async def test_vertical_filter(app):
-    user_name = 'lindsey'
-    add_user(app.db, name=user_name)
-    test_role = generate_test_role(user_name, ['read:users:name'])
-    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')
+    assert result_names == in_groups
+    for group_name in in_groups | out_groups:
+        group = orm.Group.find(app.db, name=group_name)
+        app.db.delete(group)
     app.db.commit()
 
-    r = await api_request(app, 'users', headers=auth_header(app.db, user_name))
+
+async def test_vertical_filter(app, create_user_with_scopes):
+    user = create_user_with_scopes('read:users:name')
+    r = await api_request(app, 'users', headers=auth_header(app.db, user.name))
     assert r.status_code == 200
     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):
-    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))
+async def test_stacked_vertical_filter(app, create_user_with_scopes):
+    user = create_user_with_scopes('read:users:activity', 'read:users:servers')
+    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):
-    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()
+async def test_cross_filter(app, create_user_with_scopes):
+    user = create_user_with_scopes('read:users:activity', 'self')
     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))
+    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
+    for model_user in r.json():
+        if model_user['name'] == user.name:
+            assert key_in_full_model in model_user
         else:
-            assert set(user.keys()) == restricted_keys
+            assert set(model_user.keys()) == restricted_keys
+
+
+@mark.parametrize(
+    "kind, has_user_scopes",
+    [
+        ('users', True),
+        ('services', False),
+    ],
+)
+async def test_metascope_self_expansion(
+    app, kind, has_user_scopes, create_user_with_scopes, create_service_with_scopes
+):
+    if kind == 'users':
+        orm_obj = create_user_with_scopes('self')
+    else:
+        orm_obj = create_service_with_scopes('self')
+    # test expansion of user/service scopes
+    scopes = roles.expand_roles_to_scopes(orm_obj)
+    assert bool(scopes) == has_user_scopes
+
+    # test expansion of token scopes
+    orm_obj.new_api_token()
+    token_scopes = get_scopes_for(orm_obj.api_tokens[0])
+    assert bool(token_scopes) == has_user_scopes
+
+
+async def test_metascope_all_expansion(app, create_user_with_scopes):
+    user = create_user_with_scopes('self')
+    user.new_api_token()
+    token = user.api_tokens[0]
+    # Check 'all' expansion
+    token_scope_set = get_scopes_for(token)
+    user_scope_set = get_scopes_for(user)
+    assert user_scope_set == token_scope_set
+
+    # Check no roles means no permissions
+    token.roles.clear()
+    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'}),
+        (['read:users:servers!group=nuts'], False, 2, {'name'}, {'state'}),
+        (
+            ['admin:users:server_state', 'read:users:servers'],
+            True,  # Todo: test for server stop
+            2,
+            {'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
+            {'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=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'
+        group = orm.Group.find(app.db, name=group_name)
+        if not group:
+            group = orm.Group(name=group_name)
+            app.db.add(group)
+        group.users.append(user)
+        app.db.commit()
+        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()
+            await 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.delete(group)
+            app.db.commit()

jupyterhub/tests/test_services.py

@@ -96,7 +96,7 @@ async def test_external_service(app):
 
         service = app._service_map[name]
         api_token = service.orm.api_tokens[0]
-        update_roles(app.db, api_token, 'tokens', roles=['token'])
+        update_roles(app.db, api_token, roles=['token'])
         url = public_url(app, service) + '/api/users'
         r = await async_requests.get(url, allow_redirects=False)
         r.raise_for_status()

jupyterhub/tests/test_services_auth.py

@@ -444,11 +444,7 @@ async def test_oauth_logout(app, mockservice_url):
 
     def auth_tokens():
         """Return list of OAuth access tokens for the user"""
-        return list(
-            app.db.query(orm.OAuthAccessToken).filter(
-                orm.OAuthAccessToken.user_id == app_user.id
-            )
-        )
+        return list(app.db.query(orm.APIToken).filter_by(user_id=app_user.id))
 
     # ensure we start empty
     assert auth_tokens() == []
@@ -475,6 +471,10 @@ async def test_oauth_logout(app, mockservice_url):
     session_id = s.cookies['jupyterhub-session-id']
 
     assert len(auth_tokens()) == 1
+    token = auth_tokens()[0]
+    assert token.expires_in is not None
+    # verify that oauth_token_expires_in has its desired effect
+    assert abs(app.oauth_token_expires_in - token.expires_in) < 30
 
     # hit hub logout URL
     r = await s.get(public_url(app, path='hub/logout'))

jupyterhub/tests/utils.py

@@ -9,6 +9,7 @@ from certipy import Certipy
 from jupyterhub import metrics
 from jupyterhub import orm
 from jupyterhub.objects import Server
+from jupyterhub.roles import assign_default_roles
 from jupyterhub.roles import update_roles
 from jupyterhub.utils import url_path_join as ujoin
 
@@ -113,7 +114,10 @@ def add_user(db, app=None, **kwargs):
             setattr(orm_user, attr, value)
     db.commit()
     requested_roles = kwargs.get('roles')
-    update_roles(db, obj=orm_user, kind='users', roles=requested_roles)
+    if requested_roles:
+        update_roles(db, entity=orm_user, roles=requested_roles)
+    else:
+        assign_default_roles(db, entity=orm_user)
     if app:
         return app.users[orm_user.id]
     else:

jupyterhub/user.py

@@ -826,10 +826,7 @@ class User:
             try:
                 await maybe_future(spawner.run_post_stop_hook())
             except:
-                spawner.clear_state()
-                spawner.orm_spawner.state = spawner.get_state()
-                self.db.commit()
-                raise
+                self.log.exception("Error in Spawner.post_stop_hook for %s", self)
             spawner.clear_state()
             spawner.orm_spawner.state = spawner.get_state()
             self.db.commit()

jupyterhub/utils.py

@@ -8,6 +8,7 @@ import hashlib
 import inspect
 import os
 import random
+import secrets
 import socket
 import ssl
 import sys
@@ -325,7 +326,7 @@ def hash_token(token, salt=8, rounds=16384, algorithm='sha512'):
     """
     h = hashlib.new(algorithm)
     if isinstance(salt, int):
-        salt = b2a_hex(os.urandom(salt))
+        salt = b2a_hex(secrets.token_bytes(salt))
     if isinstance(salt, bytes):
         bsalt = salt
         salt = salt.decode('utf8')