From 148257de12586d739d22be2db25767d6817affce Mon Sep 17 00:00:00 2001 From: Min RK Date: Fri, 23 Apr 2021 14:12:46 +0200 Subject: [PATCH 01/45] DOC: details of oauth in jupyterhub --- docs/source/reference/oauth.md | 214 +++++++++++++++++++++++++++++++++ 1 file changed, 214 insertions(+) create mode 100644 docs/source/reference/oauth.md diff --git a/docs/source/reference/oauth.md b/docs/source/reference/oauth.md new file mode 100644 index 00000000..a98c9293 --- /dev/null +++ b/docs/source/reference/oauth.md @@ -0,0 +1,214 @@ +# JupyterHub and OAuth + +JupyterHub uses OAuth 2 internally as a mechanism for authenticating users. +As such, JupyterHub itself always functions as an OAuth **provider**. +More on what that means below. + +Additionally, JupyterHub is _often_ deployed with [oauthenticator](https://oauthenticator.readthedocs.io), +where an external identity provider, such as GitHub or KeyCloak, is used to authenticate users. +When this is the case, there are \*two nested + +This means that when you are using JupyterHub, there is always _at least one_ and often two layers of OAuth involved in a user logging in and accessing their server. + +Some relevant points: + +- Single-user servers _never_ need to communicate with or be aware of the upstream provider. + As far as they are concerned, only JupyterHub is an OAuth provider, + and how users authenticate with the Hub itself is irrelevant. +- When talking to a single-user server, + there are ~always two tokens: + a token issued to the server itself to communicate with the Hub API, + and a second per-user token in the browser to represent the completed login process and authorized permissions. + More on this later. + +### Key OAuth terms + +- **provider** the entity responsible for managing. + JupyterHub is _always_ an oauth provider for its components. + When OAuthenticator is used, an external service, such as GitHub or KeyCloak, is also an oauth provider. +- **client** An entity that requests OAuth tokens on a user's behalf. + JupyterHub _services_ or single-user _servers_ are OAuth clients of the JupyterHub _provider_. + When OAuthenticator is used, JupyterHub is itself also an OAuth _client_ for the external oauth _provider_, e.g. GitHub. +- **browser** A user's web browser, which makes requests and stores things like cookies to +- **token** The secret value used to represent a user's authorization. This is the final product of the OAuth process + +### The oauth flow + +OAuth flow is what we call the sequence of HTTP requests involved in authenticating a user and issuing a token. + +It generally goes like this: + +1. A _browser_ makes an HTTP request to an oauth _client_. +2. There are no credentials, so the client _redirects_ the browser to an "authorize" page on the oauth _provider_ with some extra information: + - the oauth **client id** of the client itself + - the **redirect uri** to be redirected back to after completion + - the **scopes** requested, which the user should be presented with to confirm. + This is the "X would like to be able to Y on your behalf. Allow this?" page you see on all the "Login with ..." pages around the Internet. +3. During this authorize step, + the browser must be _authenticated_ with the provider. + This is often already stored in a cookie, + but if not the provider webapp must begin its _own_ authentication process before serving the authorization page. +4. After the user tells the provider that they want to proceed with the authorization, + the provider records this authorization in a short-lived record called an **oauth code**. +5. Finally, + the oauth provider redirects the browser _back_ to the oauth client's "redirect uri" + (or "oauth callback uri"), + with the oauth code in a url parameter. + +At this point: + +- The browser is authenticated with the _provider_ +- The user's authorized permissions are recorded in an _oauth code_ +- The _provider_ knows that the given oauth client's requested permissions have been granted, but the client doesn't know this yet. +- All requests so far have been made directly by the browser. + No requests have originated at the client or provider. + +Now we get to finish the OAuth process. +Let's dig into what the oauth client does when it handles +the oauth callback request with the + +- The OAuth client receives the _code_ and makes an API request to the _provider_ to exchange the code for a real _token_. + This is the first direct request between the OAuth _client_ and the _provider_. +- Once the token is retrieved, the client _usually_ + makes a second API request to the _provider_ + to retrieve information about the owner of the token (the user) +- Finally, the oauth client stores its own record that the user is authorized in a cookie. + This could be the token itself, or any other appropriate representation of successful authentication. + +_phew_ + +So that's _one_ OAuth process. + +## Full sequence of OAuth in JupyterHub + +Let's go through the above oauth process in Jupyter, +with specific examples of each HTTP request and what information is contained. + +Our starting point: + +- a user's single-user server is running. Let's call them `danez` +- jupyterhub is running with GitHub as an oauth provider, +- Danez has a fresh browser session with no cookies yet + +First request: + +- browser->single-user server +- `GET /user/danez/notebooks/mynotebook.ipynb` +- no credentials, so client starts oauth process with JupyterHub +- response: 302 redirect -> `/hub/api/oauth2/authorize` + with: + - client-id=`jupyterhub-user-danez` + - redirect-uri=`/user/danez/oauth_callback` (we'll come back later!) + +Second request, following redirect: + +- browser->jupyterhub +- `GET /hub/api/oauth2/authorize` +- no credentials, so jupyterhub starts oauth process _with GitHub_ +- response: 302 redirect -> `/hub/api/oauth2/authorize` + with: + - client-id=`jupyterhub-client-uuid` + - redirect-uri=`/hub/oauth_callback` (we'll come back later!) + +Third request, following redirect: + +- browser->GitHub +- `GET https://github.com/login/oauth/authorize` + +Prompts for login and asks for confirmation of authorization. + +After successful authorization +(either by looking up a pre-existing authorization, +or recording it via form submission) +GitHub issues oauth code and redirects to `/hub/oauth_callback?code=github-code` + +Next request: + +- browser->JupyterHub +- `GET /hub/oauth_callback?code=github-code` + +Inside the callback handler, JupyterHub makes two API requests: + +The first: + +- JupyterHub->GitHub +- `POST https://github.com/login/oauth/access_token` +- request made with oauth code from url parameter +- response includes an Access token + +The second: + +- JupyterHub->GitHub +- `GET https://api.github.com/user` +- request made with access token in the `Authorization` header +- response is the user model, including username, email, etc. + +Now the oauth callback request completes with: + +- set cookie on `/hub/` recording jupyterhub authentication so we don't need to do oauth with github again for a while +- redirect -> `/hub/api/oauth2/authorize` + +Now, we get our first repeated request: + +- browser->jupyterhub +- `GET /hub/api/oauth2/authorize` +- this time with credentials, + so jupyterhub either + 1. serves the authorization confirmation page, or + 2. automatically accepts authorization (shortcut taken when a user is visiting their own server) +- redirect -> `/user/danez/oauth_callback?code=jupyterhub-code` + +Here, we start the same oauth callback process as before, but at Danez's single-user server + +- browser->single-user server +- `GET /user/danez/oauth_callback` + +(in handler) + +Inside the callback handler, Danez's server makes two API requests to JupyterHub: + +The first: + +- single-user server->JupyterHub +- `POST /hub/api/oauth2/token` +- request made with oauth code from url parameter +- response includes an API token + +The second: + +- single-user server->JupyterHub +- `GET /hub/api/user` +- request made with token in the `Authorization` header +- response is the user model, including username, groups, etc. + +Finally completing `GET /user/danez/oauth_callback`: + +- response sets cookie, storing encrypted access token +- _finally_ redirects back to the original `/user/danez/notebooks/mynotebook.ipynb` + +Final request: + +- browser -> single-user server +- `GET /user/danez/notebooks/mynotebook.ipynb` +- encrypted jupyterhub token in cookie + +To authenticate this request, the single token stored in the encrypted cookie is passed to the Hub for verification: + +- single-user server -> Hub +- `GET /hub/api/user` +- browser's token in Authorization header +- response: user model with name, groups, etc. + +If the user model matches who should be allowed (e.g. Danez), +then the request is allowed. + +_the end_ + +## A tale of two tokens + +**TODO**: discuss API token issued to server at startup and oauth-issued token in cookie, and some details of how JupyterLab currently deals with that. +` + +## Notes + +- I omitted some information about the distinction between tokens issued to the server, due to RBAC changes. But they are different! From c61b8e60c2ca2ae7dbfabc6f389f81de1e6d3925 Mon Sep 17 00:00:00 2001 From: 0mar Date: Fri, 30 Apr 2021 17:27:26 +0200 Subject: [PATCH 02/45] Removed configuration options to assign roles to tokens --- jupyterhub/app.py | 12 --- jupyterhub/roles.py | 19 ---- jupyterhub/tests/test_roles.py | 163 --------------------------------- 3 files changed, 194 deletions(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index d7ab38f2..9249ce75 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -333,7 +333,6 @@ class JupyterHub(Application): 'scopes': ['users', 'groups'], 'users': ['cyclops', 'gandalf'], 'services': [], - 'tokens': [], 'groups': [] } ] @@ -2013,9 +2012,6 @@ class JupyterHub(Application): 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) - # check tokens for default roles roles.check_for_default_roles(db, bearer='tokens') @@ -2058,13 +2054,6 @@ 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) @@ -2072,7 +2061,6 @@ class JupyterHub(Application): token, note="from config", generated=self.trust_user_provided_tokens, - roles=config_roles, ) except Exception: if created: diff --git a/jupyterhub/roles.py b/jupyterhub/roles.py index 7199b30f..b79941ed 100644 --- a/jupyterhub/roles.py +++ b/jupyterhub/roles.py @@ -479,25 +479,6 @@ def update_roles(db, entity, roles): grant_role(db, entity=entity, rolename=rolename) -def add_predef_roles_tokens(db, predef_roles): - - """Adds tokens to predefined roles in config file - if their permissions allow""" - - for predef_role in predef_roles: - if 'tokens' in predef_role.keys(): - token_role = orm.Role.find(db, name=predef_role['name']) - for token_name in predef_role['tokens']: - token = orm.APIToken.find(db, token_name) - if token is None: - raise ValueError( - "Token %r does not exist and cannot assign it to role %r" - % (token_name, token_role.name) - ) - else: - update_roles(db, token, roles=[token_role.name]) - - def check_for_default_roles(db, bearer): """Checks that role bearers have at least one role (default if none). diff --git a/jupyterhub/tests/test_roles.py b/jupyterhub/tests/test_roles.py index 41d5ed12..3ef14a0d 100644 --- a/jupyterhub/tests/test_roles.py +++ b/jupyterhub/tests/test_roles.py @@ -589,7 +589,6 @@ async def test_load_roles_user_tokens(tmpdir, request): 'name': 'reader', 'description': 'Read all users models', 'scopes': ['read:users'], - 'tokens': ['super-secret-token'], }, ] kwargs = { @@ -608,11 +607,6 @@ async def test_load_roles_user_tokens(tmpdir, request): await hub.init_api_tokens() await hub.init_roles() - # test if gandalf's token has the 'reader' role - reader_role = orm.Role.find(db, 'reader') - token = orm.APIToken.find(db, 'super-secret-token') - assert reader_role in token.roles - # test if all other tokens have default 'user' role token_role = orm.Role.find(db, 'token') secret_token = orm.APIToken.find(db, 'secret-token') @@ -630,163 +624,6 @@ async def test_load_roles_user_tokens(tmpdir, request): roles.delete_role(db, role['name']) -@mark.role -async def test_load_roles_user_tokens_not_allowed(tmpdir, request): - user_tokens = { - 'secret-token': 'bilbo', - } - roles_to_load = [ - { - 'name': 'user-creator', - 'description': 'Creates/deletes any user', - 'scopes': ['admin:users'], - 'tokens': ['secret-token'], - }, - ] - kwargs = { - 'load_roles': roles_to_load, - 'api_tokens': user_tokens, - } - 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 - hub.authenticator.allowed_users = ['bilbo'] - await hub.init_users() - await hub.init_api_tokens() - - response = 'allowed' - # bilbo has only default 'user' role - # while bilbo's token is requesting role with higher permissions - with pytest.raises(ValueError): - await hub.init_roles() - - # delete the test tokens - for token in db.query(orm.APIToken): - db.delete(token) - db.commit() - - # delete the test roles - for role in roles_to_load: - 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'}, - ] - service_tokens = { - 'another-secret-token': 'idle-culler', - } - roles_to_load = [ - { - 'name': 'idle-culler', - 'description': 'Cull idle servers', - 'scopes': [ - 'read:users:name', - 'read:users:activity', - 'read:users:servers', - 'users:servers', - ], - 'services': ['idle-culler'], - 'tokens': ['another-secret-token'], - }, - ] - kwargs = { - 'load_roles': roles_to_load, - 'services': services, - 'service_tokens': service_tokens, - } - 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 - await hub.init_api_tokens() - await hub.init_roles() - - # test if another-secret-token has idle-culler role - service = orm.Service.find(db, 'idle-culler') - culler_role = orm.Role.find(db, 'idle-culler') - token = orm.APIToken.find(db, 'another-secret-token') - assert len(token.roles) == 1 - assert culler_role in token.roles - - # delete the test services - for service in db.query(orm.Service): - db.delete(service) - db.commit() - - # delete the test tokens - for token in db.query(orm.APIToken): - db.delete(token) - db.commit() - - # delete the test roles - for role in roles_to_load: - roles.delete_role(db, role['name']) - - -@mark.role -async def test_load_roles_service_tokens_not_allowed(tmpdir, request): - services = [{'name': 'some-service', 'api_token': 'secret-token'}] - service_tokens = { - 'secret-token': 'some-service', - } - roles_to_load = [ - { - 'name': 'user-reader', - 'description': 'Read-only user models', - 'scopes': ['read:users'], - 'services': ['some-service'], - }, - # 'idle-culler' role has higher permissions that the token's owner 'some-service' - { - 'name': 'idle-culler', - 'description': 'Cull idle servers', - 'scopes': [ - 'read:users:name', - 'read:users:activity', - 'read:users:servers', - 'users:servers', - ], - 'tokens': ['secret-token'], - }, - ] - kwargs = { - 'load_roles': roles_to_load, - 'services': services, - 'service_tokens': service_tokens, - } - 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 - await hub.init_api_tokens() - with pytest.raises(ValueError): - await hub.init_roles() - - # delete the test services - for service in db.query(orm.Service): - db.delete(service) - db.commit() - - # delete the test tokens - for token in db.query(orm.APIToken): - db.delete(token) - db.commit() - - # delete the test roles - for role in roles_to_load: - roles.delete_role(db, role['name']) - - @mark.role @mark.parametrize( "headers, rolename, scopes, status", From 0eb5e3b6ce586e91c5c70f9446780bec0d558626 Mon Sep 17 00:00:00 2001 From: 0mar Date: Fri, 7 May 2021 15:31:03 +0200 Subject: [PATCH 03/45] Split role creation and role assignment --- jupyterhub/app.py | 29 +++++++++++++++++------------ jupyterhub/tests/test_roles.py | 17 +++++++++-------- jupyterhub/tests/test_scopes.py | 2 +- jupyterhub/tests/test_services.py | 4 ++-- 4 files changed, 29 insertions(+), 23 deletions(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index 9249ce75..d47085f6 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -329,7 +329,7 @@ class JupyterHub(Application): load_roles = [ { 'name': 'teacher', - 'description': 'Access users information, servers and groups without create/delete privileges', + 'description': 'Access to users' information, servers and groups', 'scopes': ['users', 'groups'], 'users': ['cyclops', 'gandalf'], 'services': [], @@ -1965,25 +1965,27 @@ class JupyterHub(Application): group.users.append(user) db.commit() - async def init_roles(self): + async def init_role_creation(self): """Load default and predefined roles into the database""" - db = self.db - # tokens are added separately - role_bearers = ['users', 'services', 'groups'] - - # load default roles self.log.debug('Loading default roles to database') default_roles = roles.get_default_roles() for role in default_roles: - roles.create_role(db, role) + roles.create_role(self.db, role) + async def init_role_assignment(self): + # tokens are added separately + role_bearers = ['users', 'services', 'groups'] + + db = self.db # 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.create_role(db, predef_role) + predef_role_obj = orm.Role.find(db, name=predef_role['name']) # add users, services, and/or groups, # tokens need to be checked for permissions for bearer in role_bearers: + orm_role_bearers = [] if bearer in predef_role.keys(): for bname in predef_role[bearer]: if bearer == 'users': @@ -1999,9 +2001,11 @@ class JupyterHub(Application): ) Class = orm.get_class(bearer) orm_obj = Class.find(db, bname) - roles.grant_role( - db, entity=orm_obj, rolename=predef_role['name'] - ) + orm_role_bearers.append(orm_obj) + # roles.grant_role( + # db, entity=orm_obj, rolename=predef_role['name'] + # ) + setattr(predef_role_obj, bearer, orm_role_bearers) # make sure that on no admin situation, all roles are reset admin_role = orm.Role.find(db, name='admin') if not admin_role.users: @@ -2606,11 +2610,12 @@ class JupyterHub(Application): self.init_hub() self.init_proxy() self.init_oauth() + await self.init_role_creation() await self.init_users() await self.init_groups() self.init_services() await self.init_api_tokens() - await self.init_roles() + await self.init_role_assignment() self.init_tornado_settings() self.init_handlers() self.init_tornado_application() diff --git a/jupyterhub/tests/test_roles.py b/jupyterhub/tests/test_roles.py index f7b644fb..ba664b8f 100644 --- a/jupyterhub/tests/test_roles.py +++ b/jupyterhub/tests/test_roles.py @@ -237,7 +237,7 @@ async def test_load_default_roles(tmpdir, request): hub = MockHub(**kwargs) hub.init_db() db = hub.db - await hub.init_roles() + await hub.init_role_creation() # test default roles loaded to database default_roles = roles.get_default_roles() for role in default_roles: @@ -435,8 +435,8 @@ async def test_load_roles_users(tmpdir, request): hub.authenticator.admin_users = ['admin'] hub.authenticator.allowed_users = ['cyclops', 'gandalf', 'bilbo', 'gargamel'] await hub.init_users() - await hub.init_roles() - + await hub.init_role_creation() + await hub.init_role_assignment() admin_role = orm.Role.find(db, 'admin') user_role = orm.Role.find(db, 'user') # test if every user has a role (and no duplicates) @@ -502,8 +502,8 @@ async def test_load_roles_services(tmpdir, request): admin_service = orm.Service.find(db, 'admin_service') admin_service.admin = True db.commit() - await hub.init_roles() - + await hub.init_role_creation() + await hub.init_role_assignment() # 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') @@ -571,7 +571,8 @@ async def test_load_roles_groups(tmpdir, request): hub.init_db() db = hub.db await hub.init_groups() - await hub.init_roles() + await hub.init_role_creation() + await hub.init_role_assignment() assist_role = orm.Role.find(db, name='assistant') head_role = orm.Role.find(db, name='head') @@ -618,8 +619,8 @@ async def test_load_roles_user_tokens(tmpdir, request): hub.authenticator.allowed_users = ['cyclops', 'gandalf'] await hub.init_users() await hub.init_api_tokens() - await hub.init_roles() - + await hub.init_role_creation() + await hub.init_role_assignment() # test if all other tokens have default 'user' role token_role = orm.Role.find(db, 'token') secret_token = orm.APIToken.find(db, 'secret-token') diff --git a/jupyterhub/tests/test_scopes.py b/jupyterhub/tests/test_scopes.py index 9d8b421e..acd9ab8e 100644 --- a/jupyterhub/tests/test_scopes.py +++ b/jupyterhub/tests/test_scopes.py @@ -619,7 +619,7 @@ async def test_server_state_access( ) service = create_service_with_scopes(*scopes) api_token = service.new_api_token() - await app.init_roles() + await app.init_role_creation() headers = {'Authorization': 'token %s' % api_token} r = await api_request(app, 'users', user.name, headers=headers) r.raise_for_status() diff --git a/jupyterhub/tests/test_services.py b/jupyterhub/tests/test_services.py index 0ac0421c..266f4a07 100644 --- a/jupyterhub/tests/test_services.py +++ b/jupyterhub/tests/test_services.py @@ -89,10 +89,10 @@ async def test_external_service(app): } ] await maybe_future(app.init_services()) - await maybe_future(app.init_roles()) + await maybe_future(app.init_role_creation()) await app.init_api_tokens() await app.proxy.add_all_services(app._service_map) - await app.init_roles() + await app.init_role_creation() service = app._service_map[name] api_token = service.orm.api_tokens[0] From 988bc376ac7254072e7db32cd37c7b17c5dc7189 Mon Sep 17 00:00:00 2001 From: 0mar Date: Fri, 7 May 2021 16:20:16 +0200 Subject: [PATCH 04/45] Added tests for user role configuration --- jupyterhub/tests/test_roles.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/jupyterhub/tests/test_roles.py b/jupyterhub/tests/test_roles.py index ba664b8f..eff091dc 100644 --- a/jupyterhub/tests/test_roles.py +++ b/jupyterhub/tests/test_roles.py @@ -884,3 +884,34 @@ async def test_oauth_allowed_roles(app, create_temp_role): app_service = app.services[0] assert app_service['name'] == 'oas1' assert set(app_service['oauth_roles']) == set(allowed_roles) + + +async def test_config_role_assignment(): + role_name = 'painter' + user_name = 'benny' + other_users = ['agnetha', 'bjorn', 'anni-frid'] + roles_to_load = [ + { + 'name': role_name, + 'description': 'painting with colors', + 'scopes': ['users', 'groups'], + 'users': other_users, + }, + ] + for user_in_config in [False, True]: + if user_in_config: + roles_to_load[0]['users'].append(user_name) + kwargs = {'load_roles': roles_to_load} + hub = MockHub(**kwargs, clean_db=False) + hub.init_db() + hub.authenticator.admin_users = ['admin'] + hub.authenticator.allowed_users = other_users + [user_name] + await hub.init_users() + await hub.init_role_creation() + await hub.init_role_assignment() + user = orm.User.find(hub.db, name=user_name) + role = orm.Role.find(hub.db, name=role_name) + if user_in_config: + assert role in user.roles + else: + assert role not in user.roles From 915fa4bfcc8d41c18ce5c6826b3a81c492593bdf Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 12 May 2021 11:05:47 +0200 Subject: [PATCH 05/45] Apply suggestions from code review thanks Carol! Co-authored-by: Carol Willing --- docs/source/reference/oauth.md | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/docs/source/reference/oauth.md b/docs/source/reference/oauth.md index a98c9293..185239cd 100644 --- a/docs/source/reference/oauth.md +++ b/docs/source/reference/oauth.md @@ -24,20 +24,21 @@ Some relevant points: ### Key OAuth terms - **provider** the entity responsible for managing. - JupyterHub is _always_ an oauth provider for its components. + JupyterHub is _always_ an oauth provider for JupyterHub's components. When OAuthenticator is used, an external service, such as GitHub or KeyCloak, is also an oauth provider. - **client** An entity that requests OAuth tokens on a user's behalf. JupyterHub _services_ or single-user _servers_ are OAuth clients of the JupyterHub _provider_. When OAuthenticator is used, JupyterHub is itself also an OAuth _client_ for the external oauth _provider_, e.g. GitHub. -- **browser** A user's web browser, which makes requests and stores things like cookies to -- **token** The secret value used to represent a user's authorization. This is the final product of the OAuth process +- **browser** A user's web browser, which makes requests and stores things like cookies +- **token** The secret value used to represent a user's authorization. This is the final product of the OAuth process. ### The oauth flow -OAuth flow is what we call the sequence of HTTP requests involved in authenticating a user and issuing a token. +OAuth flow is what we call the sequence of HTTP requests involved in authenticating a user and issuing a token, ultimately used for authorized access to a service or single-user server. It generally goes like this: +#### Oauth request and redirect 1. A _browser_ makes an HTTP request to an oauth _client_. 2. There are no credentials, so the client _redirects_ the browser to an "authorize" page on the oauth _provider_ with some extra information: - the oauth **client id** of the client itself @@ -55,6 +56,7 @@ It generally goes like this: (or "oauth callback uri"), with the oauth code in a url parameter. +#### State after redirect At this point: - The browser is authenticated with the _provider_ @@ -63,6 +65,7 @@ At this point: - All requests so far have been made directly by the browser. No requests have originated at the client or provider. +#### OAuth Client Handles Callback Request Now we get to finish the OAuth process. Let's dig into what the oauth client does when it handles the oauth callback request with the @@ -92,7 +95,7 @@ Our starting point: First request: -- browser->single-user server +- browser->single-user server running JupyterLab or Jupyter Classic - `GET /user/danez/notebooks/mynotebook.ipynb` - no credentials, so client starts oauth process with JupyterHub - response: 302 redirect -> `/hub/api/oauth2/authorize` @@ -134,7 +137,7 @@ The first: - JupyterHub->GitHub - `POST https://github.com/login/oauth/access_token` - request made with oauth code from url parameter -- response includes an Access token +- response includes an access token The second: From 65f3933da4ce7b332bef1c451d2b21b34f8a0529 Mon Sep 17 00:00:00 2001 From: IvanaH8 Date: Thu, 20 May 2021 14:36:21 +0200 Subject: [PATCH 06/45] Create scope dictionary --- docs/source/rbac/scopedict.py | 97 +++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 docs/source/rbac/scopedict.py diff --git a/docs/source/rbac/scopedict.py b/docs/source/rbac/scopedict.py new file mode 100644 index 00000000..25c3eec9 --- /dev/null +++ b/docs/source/rbac/scopedict.py @@ -0,0 +1,97 @@ +"""Scope definitions""" + + +def get_scope_dict(): + """Returns a nested dictionary of all available scopes: + + {scopename: {'description': description, + 'subscopes': [immediate subscopes]}, + } + + without 'subscopes' key if the scope has no subscopes. + """ + scope_dict = { + '(no_scope)': {'description': 'Allows for only identifying the owning entity.'}, + 'self': { + 'description': 'Metascope, grants access to user’s own resources only; resolves to (no_scope) for services.' + }, + 'all': { + 'description': 'Metascope, valid for tokens only. Grants access to everything that the token’s owning entity can access.' + }, + 'admin:users': { + 'description': 'Grants read, write, create and delete access to users and their authentication state but not their servers or tokens.', + 'subscopes': ['admin:users:auth_state', 'users'], + }, + 'admin:users:auth_state': { + 'description': 'Grants access to users’ authentication state only.' + }, + 'users': { + 'description': 'Grants read and write permissions to users’ models apart from servers, tokens and authentication state.', + 'subscopes': ['read:users', 'users:activity'], + }, + 'read:users': { + 'description': 'Read-only access to users’ models apart from servers, tokens and authentication state.', + 'subscopes': [ + 'read:users:name', + 'read:users:groups', + 'read:users:activity', + 'read:users:roles', + ], + }, + 'read:users:name': {'description': 'Read-only access to users’ names.'}, + 'read:users:groups': {'description': 'Read-only access to users’ group names.'}, + 'read:users:roles': {'description': 'Read-only access to users’ role names.'}, + 'read:users:activity': { + 'description': 'Read-only access to users’ last activity.' + }, + 'users:activity': { + 'description': 'Grants access to read and post users’ last activity only.', + 'subscopes': ['read:users:activity'], + }, + 'admin:users:servers': { + 'description': 'Grants read, start/stop, create and delete permissions to users’ servers and their state.', + 'subscopes': ['admin:users:server_state', 'users:servers'], + }, + 'admin:users:server_state': { + 'description': 'Grants access to servers’ state only.' + }, + 'users:servers': { + 'description': 'Allows for starting/stopping users’ servers in addition to read access to their models. Does not include the server state.', + 'subscopes': ['read:users:servers'], + }, + 'read:users:servers': { + 'description': 'Read-only access to users’ names and their server models. Does not include the server state.', + 'subscopes': ['read:users:name'], + }, + 'users:tokens': { + 'description': 'Grants read, write, create and delete permissions to users’ tokens.', + 'subscopes': ['read:users:tokens'], + }, + 'read:users:tokens': {'description': 'Read-only access to users’ tokens.'}, + 'admin:groups': { + 'description': 'Grants read, write, create and delete access to groups.', + 'subscopes': ['groups'], + }, + 'groups': { + 'description': 'Grants read and write permissions to groups, including adding/removing users to/from groups.', + 'subscopes': ['read:groups'], + }, + 'read:groups': {'description': 'Read-only access to groups’ models.'}, + 'read:services': { + 'description': 'Read-only access to service models.', + 'subscopes': ['read:services:name', 'read:services:roles'], + }, + 'read:services:name': {'description': 'Read-only access to service names.'}, + 'read:services:roles': { + 'description': 'Read-only access to service role names.' + }, + 'read:hub': { + 'description': 'Read-only access to detailed information about the Hub.' + }, + 'proxy': { + 'description': 'Allows for obtaining information about the proxy’s routing table, for syncing the Hub with proxy and notifying the Hub about a new proxy.' + }, + 'shutdown': {'description': 'Grants access to shutdown the hub.'}, + } + + return scope_dict From 948179ee0e360a1f643d99b62e8b4856f8e09f6c Mon Sep 17 00:00:00 2001 From: IvanaH8 Date: Thu, 20 May 2021 14:49:28 +0200 Subject: [PATCH 07/45] Generate scope table in separate markdown file --- docs/source/rbac/generate-scope-table.py | 95 ++++++++++++++++++++++++ 1 file changed, 95 insertions(+) create mode 100644 docs/source/rbac/generate-scope-table.py diff --git a/docs/source/rbac/generate-scope-table.py b/docs/source/rbac/generate-scope-table.py new file mode 100644 index 00000000..5024eb48 --- /dev/null +++ b/docs/source/rbac/generate-scope-table.py @@ -0,0 +1,95 @@ +import os +from collections import defaultdict + +from pytablewriter import MarkdownTableWriter +from scopedict import get_scope_dict + +HERE = os.path.abspath(os.path.dirname(__file__)) + + +class ScopeTableGenerator: + def __init__(self): + self.scopes = get_scope_dict() + + @classmethod + def create_writer(cls, table_name, headers, values): + writer = MarkdownTableWriter() + writer.table_name = table_name + writer.headers = headers + writer.value_matrix = values + writer.margin = 1 + return writer + + def _get_scope_relationships(self): + """Returns a tuple of dictionary of all scope-subscope pairs and a list of just subscopes: + + ({scope: subscope}, [subscopes]) + + used for creating hierarchical scope table in _parse_scopes() + """ + pairs = [] + for scope in self.scopes.keys(): + if self.scopes[scope].get('subscopes'): + for subscope in self.scopes[scope]['subscopes']: + pairs.append((scope, subscope)) + else: + pairs.append((scope, None)) + subscopes = [pair[1] for pair in pairs] + pairs_dict = defaultdict(list) + for scope, subscope in pairs: + pairs_dict[scope].append(subscope) + return pairs_dict, subscopes + + def _get_top_scopes(self, subscopes): + """Returns a list of highest level scopes + (not a subscope of any other scopes)""" + top_scopes = [] + for scope in self.scopes.keys(): + if scope not in subscopes: + top_scopes.append(scope) + return top_scopes + + def _parse_scopes(self): + """Returns a list of table rows where row: + [indented scopename string, scope description string]""" + scope_pairs, subscopes = self._get_scope_relationships() + top_scopes = self._get_top_scopes(subscopes) + + table_rows = [] + md_indent = "   " + + def _add_subscopes(table_rows, scopename, depth=0): + description = self.scopes[scopename]['description'] + table_row = [f"{md_indent*depth}`{scopename}`", description] + table_rows.append(table_row) + for subscope in scope_pairs[scopename]: + if subscope: + _add_subscopes(table_rows, subscope, depth + 1) + + for scope in top_scopes: + _add_subscopes(table_rows, scope) + + return table_rows + + def write_table(self): + """Generates the scope table in markdown format and writes it into scope-table.md file""" + filename = f"{HERE}/scope-table.md" + table_name = "" + headers = ["Scope", "Description"] + values = self._parse_scopes() + writer = self.create_writer(table_name, headers, values) + + title = "Table 1. Available scopes and their hierarchy" + content = f"{title}\n{writer.dumps()}" + with open(filename, 'w') as f: + f.write(content) + print(f"Generated {filename}.") + + +def main(): + table_generator = ScopeTableGenerator() + table_generator.write_table() + + +if __name__ == "__main__": + main() From 7914c010998f1cd222be8f4d892f389de0a809e7 Mon Sep 17 00:00:00 2001 From: IvanaH8 Date: Thu, 20 May 2021 14:52:28 +0200 Subject: [PATCH 08/45] Call scope table generation in makefile and include in scopes.md --- docs/Makefile | 7 ++++++- docs/source/rbac/scopes.md | 35 ++++------------------------------- 2 files changed, 10 insertions(+), 32 deletions(-) diff --git a/docs/Makefile b/docs/Makefile index 5e61789e..f36da011 100644 --- a/docs/Makefile +++ b/docs/Makefile @@ -66,7 +66,12 @@ metrics: source/reference/metrics.rst source/reference/metrics.rst: generate-metrics.py python3 generate-metrics.py -html: rest-api metrics +scopes: source/rbac/scope-table.md + +source/rbac/scope-table.md: source/rbac/generate-scope-table.py source/rbac/scopedict.py + python3 source/rbac/generate-scope-table.py + +html: rest-api metrics scopes $(SPHINXBUILD) -b html $(ALLSPHINXOPTS) $(BUILDDIR)/html @echo @echo "Build finished. The HTML pages are in $(BUILDDIR)/html." diff --git a/docs/source/rbac/scopes.md b/docs/source/rbac/scopes.md index 9478525d..6827f56c 100644 --- a/docs/source/rbac/scopes.md +++ b/docs/source/rbac/scopes.md @@ -81,37 +81,10 @@ The payload of an API call can be filtered both horizontally and vertically simu ## Available scopes Table below lists all available scopes and illustrates their hierarchy. Indented scopes indicate subscopes of the scope(s) above them. -% TODO: Automatically generate this table from code when creating docs -Table 1. Available scopes and their hierarchy -| Scope name | Description | -| :--------- | :---------- | -| (no scope) | Allows for only identifying the owning entity. | -| `self` | Metascope, grants access to user's own resources; resolves to (no scope) for services. | -| `all` | Metascope, valid for tokens only. Grants access to everything that the token's owning entity can do. | -| `admin:users` | Grants read, write, create and delete access to users and their authentication state _but not their servers or tokens._ | -|    `admin:users:auth_state` | Grants access to users' authentication state only. | -|    `users` | Grants read and write permissions to users' models _apart from servers, tokens and authentication state_. | -|       `users:activity` | Grants access to read and post users' activity only. | -|       `read:users` | Read-only access to users' models _apart from servers, tokens and authentication state_. | -|          `read:users:name` | Read-only access to users' names. | -|          `read:users:roles` | Read-only access to a list of users' roles names. | -|          `read:users:groups` | Read-only access to a list of users' group names. | -|          `read:users:activity` | Read-only access to users' activity. | -| `admin:users:servers` | Grants read, start/stop, create and delete permissions to users' servers and their state. | -|    `admin:users:server_state` | Grants access to servers' state only. | -|    `users:servers` | Allows for starting/stopping users' servers in addition to read access to their models. _Does not include the server state_. | -|       `read:users:servers` | Read-only access to users' server models. _Does not include the server state_. | -| `users:tokens` | Grants read, write, create and delete permissions to users' tokens. | -|    `read:users:tokens` | Read-only access to users' tokens. | -| `admin:groups` | Grants read, write, create and delete access to groups. | -|    `groups` | Grants read and write permissions to groups, including adding/removing users to/from groups. | -|       `read:groups` | Read-only access to groups. | -| `read:services` | Read-only access to service models. | -|    `read:services:name` | Read-only access to service names. | -|    `read:services:roles` | Read-only access to a list of service roles names. | -| `read:hub` | Read-only access to detailed information about the Hub. | -| `proxy` | Allows for obtaining information about the proxy's routing table, for syncing the Hub with proxy and notifying the Hub about a new proxy. | -| `shutdown` | Grants access to shutdown the hub. | + +```{include} scope-table.md + +``` ```{Caution} Note that only the {ref}`horizontal filtering ` can be added to scopes to customize them. \ From e61cacf5e8b9838ee74168cec44b01e300c87b5b Mon Sep 17 00:00:00 2001 From: IvanaH8 Date: Thu, 20 May 2021 14:59:49 +0200 Subject: [PATCH 09/45] Add message to run make clean before make html --- docs/source/rbac/generate-scope-table.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/source/rbac/generate-scope-table.py b/docs/source/rbac/generate-scope-table.py index 5024eb48..2934f058 100644 --- a/docs/source/rbac/generate-scope-table.py +++ b/docs/source/rbac/generate-scope-table.py @@ -84,6 +84,9 @@ class ScopeTableGenerator: with open(filename, 'w') as f: f.write(content) print(f"Generated {filename}.") + print( + "Run 'make clean' before 'make html' to ensure the built scopes.html contains latest scope table changes." + ) def main(): From 3fde458c0725988f40f4c987eacad93ca3b68f64 Mon Sep 17 00:00:00 2001 From: Min RK Date: Fri, 21 May 2021 16:43:51 +0200 Subject: [PATCH 10/45] fix appending group roles to user roles ensure we are using a fresh list before calling extend otherwise, we are extending the user's own roles --- jupyterhub/roles.py | 7 ++-- jupyterhub/tests/test_roles.py | 68 ++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 4 deletions(-) diff --git a/jupyterhub/roles.py b/jupyterhub/roles.py index dbd2e75a..b51bb84d 100644 --- a/jupyterhub/roles.py +++ b/jupyterhub/roles.py @@ -164,13 +164,12 @@ def expand_roles_to_scopes(orm_object): if not isinstance(orm_object, orm.Base): raise TypeError(f"Only orm objects allowed, got {orm_object}") - pass_roles = orm_object.roles + pass_roles = [] + pass_roles.extend(orm_object.roles) if isinstance(orm_object, orm.User): - groups_roles = [] for group in orm_object.groups: - groups_roles.extend(group.roles) - pass_roles.extend(groups_roles) + pass_roles.extend(group.roles) scopes = _get_subscopes(*pass_roles, owner=orm_object) diff --git a/jupyterhub/tests/test_roles.py b/jupyterhub/tests/test_roles.py index 4fcf806a..661af326 100644 --- a/jupyterhub/tests/test_roles.py +++ b/jupyterhub/tests/test_roles.py @@ -1046,3 +1046,71 @@ async def test_oauth_allowed_roles(app, create_temp_role): app_service = app.services[0] assert app_service['name'] == 'oas1' assert set(app_service['oauth_roles']) == set(allowed_roles) + + +async def test_user_group_roles(app, create_temp_role): + user = add_user(app.db, app, name='jack') + another_user = add_user(app.db, app, name='jill') + + group = orm.Group.find(app.db, name='A') + if not group: + group = orm.Group(name='A') + app.db.add(group) + app.db.commit() + + if group not in user.groups: + user.groups.append(group) + app.db.commit() + + if group not in another_user.groups: + another_user.groups.append(group) + app.db.commit() + + group_role = orm.Role.find(app.db, 'student-a') + if not group_role: + create_temp_role(['read:groups!group=A'], 'student-a') + roles.grant_role(app.db, group, rolename='student-a') + group_role = orm.Role.find(app.db, 'student-a') + + # repeat check to ensure group roles don't get added to the user more than once + # regression test for #3472 + roles_before = list(user.roles) + for i in range(3): + roles.expand_roles_to_scopes(user.orm_user) + user_roles = list(user.roles) + assert user_roles == roles_before + + # jack's API token + token = user.new_api_token() + + headers = {'Authorization': 'token %s' % token} + r = await api_request(app, 'users', method='get', headers=headers) + assert r.status_code == 200 + r.raise_for_status() + reply = r.json() + + print(reply) + + assert len(reply[0]['roles']) == 1 + assert reply[0]['name'] == 'jack' + assert group_role.name not in reply[0]['roles'] + + headers = {'Authorization': 'token %s' % token} + r = await api_request(app, 'groups', method='get', headers=headers) + assert r.status_code == 200 + r.raise_for_status() + reply = r.json() + + print(reply) + + headers = {'Authorization': 'token %s' % token} + r = await api_request(app, 'users', method='get', headers=headers) + assert r.status_code == 200 + r.raise_for_status() + reply = r.json() + + print(reply) + + assert len(reply[0]['roles']) == 1 + assert reply[0]['name'] == 'jack' + assert group_role.name not in reply[0]['roles'] From 4a1459195e59de33d54a6026c121651d3f8761ab Mon Sep 17 00:00:00 2001 From: IvanaH8 Date: Fri, 21 May 2021 16:58:45 +0200 Subject: [PATCH 11/45] Move scope_definitions dict to jupyterhub/scopes.py --- docs/Makefile | 2 +- docs/source/rbac/generate-scope-table.py | 5 +- docs/source/rbac/scopedict.py | 97 ------------------------ jupyterhub/scopes.py | 81 ++++++++++++++++++++ 4 files changed, 85 insertions(+), 100 deletions(-) delete mode 100644 docs/source/rbac/scopedict.py diff --git a/docs/Makefile b/docs/Makefile index f36da011..694d5a35 100644 --- a/docs/Makefile +++ b/docs/Makefile @@ -68,7 +68,7 @@ source/reference/metrics.rst: generate-metrics.py scopes: source/rbac/scope-table.md -source/rbac/scope-table.md: source/rbac/generate-scope-table.py source/rbac/scopedict.py +source/rbac/scope-table.md: source/rbac/generate-scope-table.py python3 source/rbac/generate-scope-table.py html: rest-api metrics scopes diff --git a/docs/source/rbac/generate-scope-table.py b/docs/source/rbac/generate-scope-table.py index 2934f058..b282e642 100644 --- a/docs/source/rbac/generate-scope-table.py +++ b/docs/source/rbac/generate-scope-table.py @@ -2,14 +2,15 @@ import os from collections import defaultdict from pytablewriter import MarkdownTableWriter -from scopedict import get_scope_dict + +from jupyterhub.scopes import scope_definitions HERE = os.path.abspath(os.path.dirname(__file__)) class ScopeTableGenerator: def __init__(self): - self.scopes = get_scope_dict() + self.scopes = scope_definitions @classmethod def create_writer(cls, table_name, headers, values): diff --git a/docs/source/rbac/scopedict.py b/docs/source/rbac/scopedict.py deleted file mode 100644 index 25c3eec9..00000000 --- a/docs/source/rbac/scopedict.py +++ /dev/null @@ -1,97 +0,0 @@ -"""Scope definitions""" - - -def get_scope_dict(): - """Returns a nested dictionary of all available scopes: - - {scopename: {'description': description, - 'subscopes': [immediate subscopes]}, - } - - without 'subscopes' key if the scope has no subscopes. - """ - scope_dict = { - '(no_scope)': {'description': 'Allows for only identifying the owning entity.'}, - 'self': { - 'description': 'Metascope, grants access to user’s own resources only; resolves to (no_scope) for services.' - }, - 'all': { - 'description': 'Metascope, valid for tokens only. Grants access to everything that the token’s owning entity can access.' - }, - 'admin:users': { - 'description': 'Grants read, write, create and delete access to users and their authentication state but not their servers or tokens.', - 'subscopes': ['admin:users:auth_state', 'users'], - }, - 'admin:users:auth_state': { - 'description': 'Grants access to users’ authentication state only.' - }, - 'users': { - 'description': 'Grants read and write permissions to users’ models apart from servers, tokens and authentication state.', - 'subscopes': ['read:users', 'users:activity'], - }, - 'read:users': { - 'description': 'Read-only access to users’ models apart from servers, tokens and authentication state.', - 'subscopes': [ - 'read:users:name', - 'read:users:groups', - 'read:users:activity', - 'read:users:roles', - ], - }, - 'read:users:name': {'description': 'Read-only access to users’ names.'}, - 'read:users:groups': {'description': 'Read-only access to users’ group names.'}, - 'read:users:roles': {'description': 'Read-only access to users’ role names.'}, - 'read:users:activity': { - 'description': 'Read-only access to users’ last activity.' - }, - 'users:activity': { - 'description': 'Grants access to read and post users’ last activity only.', - 'subscopes': ['read:users:activity'], - }, - 'admin:users:servers': { - 'description': 'Grants read, start/stop, create and delete permissions to users’ servers and their state.', - 'subscopes': ['admin:users:server_state', 'users:servers'], - }, - 'admin:users:server_state': { - 'description': 'Grants access to servers’ state only.' - }, - 'users:servers': { - 'description': 'Allows for starting/stopping users’ servers in addition to read access to their models. Does not include the server state.', - 'subscopes': ['read:users:servers'], - }, - 'read:users:servers': { - 'description': 'Read-only access to users’ names and their server models. Does not include the server state.', - 'subscopes': ['read:users:name'], - }, - 'users:tokens': { - 'description': 'Grants read, write, create and delete permissions to users’ tokens.', - 'subscopes': ['read:users:tokens'], - }, - 'read:users:tokens': {'description': 'Read-only access to users’ tokens.'}, - 'admin:groups': { - 'description': 'Grants read, write, create and delete access to groups.', - 'subscopes': ['groups'], - }, - 'groups': { - 'description': 'Grants read and write permissions to groups, including adding/removing users to/from groups.', - 'subscopes': ['read:groups'], - }, - 'read:groups': {'description': 'Read-only access to groups’ models.'}, - 'read:services': { - 'description': 'Read-only access to service models.', - 'subscopes': ['read:services:name', 'read:services:roles'], - }, - 'read:services:name': {'description': 'Read-only access to service names.'}, - 'read:services:roles': { - 'description': 'Read-only access to service role names.' - }, - 'read:hub': { - 'description': 'Read-only access to detailed information about the Hub.' - }, - 'proxy': { - 'description': 'Allows for obtaining information about the proxy’s routing table, for syncing the Hub with proxy and notifying the Hub about a new proxy.' - }, - 'shutdown': {'description': 'Grants access to shutdown the hub.'}, - } - - return scope_dict diff --git a/jupyterhub/scopes.py b/jupyterhub/scopes.py index 859f6958..21058416 100644 --- a/jupyterhub/scopes.py +++ b/jupyterhub/scopes.py @@ -11,6 +11,87 @@ from . import orm from . import roles +scope_definitions = { + '(no_scope)': {'description': 'Allows for only identifying the owning entity.'}, + 'self': { + 'description': 'Metascope, grants access to user’s own resources only; resolves to (no_scope) for services.' + }, + 'all': { + 'description': 'Metascope, valid for tokens only. Grants access to everything that the token’s owning entity can access.' + }, + 'admin:users': { + 'description': 'Grants read, write, create and delete access to users and their authentication state but not their servers or tokens.', + 'subscopes': ['admin:users:auth_state', 'users'], + }, + 'admin:users:auth_state': { + 'description': 'Grants access to users’ authentication state only.' + }, + 'users': { + 'description': 'Grants read and write permissions to users’ models apart from servers, tokens and authentication state.', + 'subscopes': ['read:users', 'users:activity'], + }, + 'read:users': { + 'description': 'Read-only access to users’ models apart from servers, tokens and authentication state.', + 'subscopes': [ + 'read:users:name', + 'read:users:groups', + 'read:users:activity', + 'read:users:roles', + ], + }, + 'read:users:name': {'description': 'Read-only access to users’ names.'}, + 'read:users:groups': {'description': 'Read-only access to users’ group names.'}, + 'read:users:roles': {'description': 'Read-only access to users’ role names.'}, + 'read:users:activity': {'description': 'Read-only access to users’ last activity.'}, + 'users:activity': { + 'description': 'Grants access to read and post users’ last activity only.', + 'subscopes': ['read:users:activity'], + }, + 'admin:users:servers': { + 'description': 'Grants read, start/stop, create and delete permissions to users’ servers and their state.', + 'subscopes': ['admin:users:server_state', 'users:servers'], + }, + 'admin:users:server_state': { + 'description': 'Grants access to servers’ state only.' + }, + 'users:servers': { + 'description': 'Allows for starting/stopping users’ servers in addition to read access to their models. Does not include the server state.', + 'subscopes': ['read:users:servers'], + }, + 'read:users:servers': { + 'description': 'Read-only access to users’ names and their server models. Does not include the server state.', + 'subscopes': ['read:users:name'], + }, + 'users:tokens': { + 'description': 'Grants read, write, create and delete permissions to users’ tokens.', + 'subscopes': ['read:users:tokens'], + }, + 'read:users:tokens': {'description': 'Read-only access to users’ tokens.'}, + 'admin:groups': { + 'description': 'Grants read, write, create and delete access to groups.', + 'subscopes': ['groups'], + }, + 'groups': { + 'description': 'Grants read and write permissions to groups, including adding/removing users to/from groups.', + 'subscopes': ['read:groups'], + }, + 'read:groups': {'description': 'Read-only access to groups’ models.'}, + 'read:services': { + 'description': 'Read-only access to service models.', + 'subscopes': ['read:services:name', 'read:services:roles'], + }, + 'read:services:name': {'description': 'Read-only access to service names.'}, + 'read:services:roles': {'description': 'Read-only access to service role names.'}, + 'read:hub': { + 'description': 'Read-only access to detailed information about the Hub.' + }, + 'proxy': { + 'description': 'Allows for obtaining information about the proxy’s routing table, for syncing the Hub with proxy and notifying the Hub about a new proxy.' + }, + 'shutdown': {'description': 'Grants access to shutdown the hub.'}, +} + + class Scope(Enum): ALL = True From 800f3cf79f547e4e0044473e872c3ea0da9a1d2a Mon Sep 17 00:00:00 2001 From: IvanaH8 Date: Fri, 21 May 2021 17:03:24 +0200 Subject: [PATCH 12/45] Add trigger to conf.py to call generate-scope-table --- docs/source/conf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/source/conf.py b/docs/source/conf.py index a53fbbfc..3da73605 100644 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -212,7 +212,7 @@ if on_rtd: # build both metrics and rest-api, since RTD doesn't run make from subprocess import check_call as sh - sh(['make', 'metrics', 'rest-api'], cwd=docs) + sh(['make', 'metrics', 'rest-api', 'scopes'], cwd=docs) # -- Spell checking ------------------------------------------------------- From e0439bc3105433e74266864e78b15cf76b1876f0 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Sun, 23 May 2021 11:38:53 +0200 Subject: [PATCH 13/45] Apply suggestions from code review Co-authored-by: Ivana --- jupyterhub/tests/test_roles.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jupyterhub/tests/test_roles.py b/jupyterhub/tests/test_roles.py index 661af326..9ea326ea 100644 --- a/jupyterhub/tests/test_roles.py +++ b/jupyterhub/tests/test_roles.py @@ -1072,7 +1072,7 @@ async def test_user_group_roles(app, create_temp_role): roles.grant_role(app.db, group, rolename='student-a') group_role = orm.Role.find(app.db, 'student-a') - # repeat check to ensure group roles don't get added to the user more than once + # repeat check to ensure group roles don't get added to the user at all # regression test for #3472 roles_before = list(user.roles) for i in range(3): From 915fee2734796fe3bb46f038a7c138248cc18594 Mon Sep 17 00:00:00 2001 From: 0mar Date: Mon, 24 May 2021 13:36:59 +0200 Subject: [PATCH 14/45] Added strict admin check to role assignment --- jupyterhub/app.py | 26 +++++++++++++++++++------- jupyterhub/roles.py | 1 - 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index a9985c08..c33e8910 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -329,7 +329,7 @@ class JupyterHub(Application): load_roles = [ { 'name': 'teacher', - 'description': 'Access to users' information, servers and groups', + 'description': 'Access to users' information and group membership', 'scopes': ['users', 'groups'], 'users': ['cyclops', 'gandalf'], 'services': [], @@ -1978,7 +1978,15 @@ class JupyterHub(Application): """Load default and predefined roles into the database""" self.log.debug('Loading default roles to database') default_roles = roles.get_default_roles() - for role in default_roles: + default_role_names = [r['name'] for r in default_roles] + init_roles = default_roles + self.load_roles + if not orm.Role.find(self.db, name='admin'): + self._rbac_upgrade = True + for role in self.db.query(orm.Role): + if role.name not in default_role_names: + self.db.delete(role) + self.db.commit() + for role in init_roles: roles.create_role(self.db, role) async def init_role_assignment(self): @@ -1987,9 +1995,13 @@ class JupyterHub(Application): db = self.db # load predefined roles from config file + for entity in self.users + self.services: + if entity.admin: + roles.grant_role(db, entity, 'admin') + else: + roles.strip_role(db, entity, 'admin') self.log.debug('Loading predefined roles from config file to database') for predef_role in self.load_roles: - roles.create_role(db, predef_role) predef_role_obj = orm.Role.find(db, name=predef_role['name']) # add users, services, and/or groups, # tokens need to be checked for permissions @@ -2015,11 +2027,11 @@ class JupyterHub(Application): # db, entity=orm_obj, rolename=predef_role['name'] # ) setattr(predef_role_obj, bearer, orm_role_bearers) - # make sure that on no admin situation, all roles are reset - admin_role = orm.Role.find(db, name='admin') - if not admin_role.users: + # make sure that on hub upgrade, all roles are reset + + if not getattr(self, '_rbac_upgrade', False): app_log.warning( - "No admin users found; assuming hub upgrade. Initializing default roles for all entities" + "No admin role 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: diff --git a/jupyterhub/roles.py b/jupyterhub/roles.py index 27c2c30c..2dc10177 100644 --- a/jupyterhub/roles.py +++ b/jupyterhub/roles.py @@ -446,7 +446,6 @@ def assign_default_roles(db, entity): db.commit() # users and services can have 'user' or 'admin' roles as default else: - # 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) From 5a5cdb418e8691188dd0cedb5c434cae1583ada2 Mon Sep 17 00:00:00 2001 From: 0mar Date: Mon, 24 May 2021 14:53:20 +0200 Subject: [PATCH 15/45] (wip): update role init process --- jupyterhub/app.py | 19 +++++--- jupyterhub/tests/test_roles.py | 84 ++++++++++++++++++++++++---------- 2 files changed, 74 insertions(+), 29 deletions(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index c33e8910..7ee594ca 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -1894,7 +1894,7 @@ class JupyterHub(Application): ) # add allowed users to the db - for name in allowed_users: + for name in allowed_users: # fixme: Do we add standard roles here? user = orm.User.find(db, name) if user is None: user = orm.User(name=name) @@ -1995,11 +1995,6 @@ class JupyterHub(Application): db = self.db # load predefined roles from config file - for entity in self.users + self.services: - if entity.admin: - roles.grant_role(db, entity, 'admin') - else: - roles.strip_role(db, entity, 'admin') self.log.debug('Loading predefined roles from config file to database') for predef_role in self.load_roles: predef_role_obj = orm.Role.find(db, name=predef_role['name']) @@ -2027,6 +2022,18 @@ class JupyterHub(Application): # db, entity=orm_obj, rolename=predef_role['name'] # ) setattr(predef_role_obj, bearer, orm_role_bearers) + for entity in db.query(orm.Service): + if entity.admin: + roles.grant_role(db, entity, 'admin') + else: + roles.assign_default_roles(db, entity) + for entity in db.query( + orm.User + ): # fixme: why can't I combine these expressions? + if entity.admin: + roles.grant_role(db, entity, 'admin') + else: + roles.assign_default_roles(db, entity) # make sure that on hub upgrade, all roles are reset if not getattr(self, '_rbac_upgrade', False): diff --git a/jupyterhub/tests/test_roles.py b/jupyterhub/tests/test_roles.py index eff091dc..d3d66365 100644 --- a/jupyterhub/tests/test_roles.py +++ b/jupyterhub/tests/test_roles.py @@ -434,8 +434,8 @@ async def test_load_roles_users(tmpdir, request): db = hub.db hub.authenticator.admin_users = ['admin'] hub.authenticator.allowed_users = ['cyclops', 'gandalf', 'bilbo', 'gargamel'] - await hub.init_users() await hub.init_role_creation() + await hub.init_users() await hub.init_role_assignment() admin_role = orm.Role.find(db, 'admin') user_role = orm.Role.find(db, 'user') @@ -570,8 +570,8 @@ async def test_load_roles_groups(tmpdir, request): hub = MockHub(**kwargs) hub.init_db() db = hub.db - await hub.init_groups() await hub.init_role_creation() + await hub.init_groups() await hub.init_role_assignment() assist_role = orm.Role.find(db, name='assistant') @@ -617,9 +617,9 @@ async def test_load_roles_user_tokens(tmpdir, request): db = hub.db hub.authenticator.admin_users = ['admin'] hub.authenticator.allowed_users = ['cyclops', 'gandalf'] + await hub.init_role_creation() await hub.init_users() await hub.init_api_tokens() - await hub.init_role_creation() await hub.init_role_assignment() # test if all other tokens have default 'user' role token_role = orm.Role.find(db, 'token') @@ -886,32 +886,70 @@ async def test_oauth_allowed_roles(app, create_temp_role): assert set(app_service['oauth_roles']) == set(allowed_roles) -async def test_config_role_assignment(): +async def test_config_role_list(): + roles_to_load = [ + { + 'name': 'elephant', + 'description': 'pacing about', + 'scopes': ['read:hub'], + }, + { + 'name': 'tiger', + 'description': 'pouncing stuff', + 'scopes': ['shutdown'], + }, + ] + kwargs = {'load_roles': roles_to_load} + hub = MockHub(**kwargs) + hub.init_db() + hub.authenticator.admin_users = ['admin'] + await hub.init_role_creation() + for role_conf in roles_to_load: + assert orm.Role.find(hub.db, name=role_conf['name']) + # Now reload and see if user is removed from role list + roles_to_load.pop(0) + hub.load_roles = roles_to_load + await hub.init_role_creation() + print(hub.db.query(orm.Role).all()) + assert orm.Role.find(hub.db, name='tiger') + assert not orm.Role.find(hub.db, name='elephant') + + +async def test_config_role_users(): role_name = 'painter' user_name = 'benny' - other_users = ['agnetha', 'bjorn', 'anni-frid'] + user_names = ['agnetha', 'bjorn', 'anni-frid', user_name] roles_to_load = [ { 'name': role_name, 'description': 'painting with colors', 'scopes': ['users', 'groups'], - 'users': other_users, + 'users': user_names, + }, + { + 'name': role_name, + 'description': 'painting with colors', + 'scopes': ['users', 'groups'], + 'users': user_names, }, ] - for user_in_config in [False, True]: - if user_in_config: - roles_to_load[0]['users'].append(user_name) - kwargs = {'load_roles': roles_to_load} - hub = MockHub(**kwargs, clean_db=False) - hub.init_db() - hub.authenticator.admin_users = ['admin'] - hub.authenticator.allowed_users = other_users + [user_name] - await hub.init_users() - await hub.init_role_creation() - await hub.init_role_assignment() - user = orm.User.find(hub.db, name=user_name) - role = orm.Role.find(hub.db, name=role_name) - if user_in_config: - assert role in user.roles - else: - assert role not in user.roles + kwargs = {'load_roles': roles_to_load} + hub = MockHub(**kwargs) + hub.init_db() + hub.authenticator.admin_users = ['admin'] + hub.authenticator.allowed_users = user_names + await hub.init_role_creation() + await hub.init_users() + await hub.init_role_assignment() + user = orm.User.find(hub.db, name=user_name) + role = orm.Role.find(hub.db, name=role_name) + assert role in user.roles + # Now reload and see if user is removed from role list + roles_to_load[0]['users'].remove(user_name) + hub.load_roles = roles_to_load + await hub.init_role_creation() + await hub.init_users() + await hub.init_role_assignment() + user = orm.User.find(hub.db, name=user_name) + role = orm.Role.find(hub.db, name=role_name) + assert role not in user.roles From 1a01302e27ea2d2498f535122f6f1eeee2d49472 Mon Sep 17 00:00:00 2001 From: 0mar Date: Tue, 25 May 2021 11:16:15 +0200 Subject: [PATCH 16/45] Fixed bug in scope test fixture teardown --- jupyterhub/tests/test_scopes.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/jupyterhub/tests/test_scopes.py b/jupyterhub/tests/test_scopes.py index 43fe9f79..5bd3825e 100644 --- a/jupyterhub/tests/test_scopes.py +++ b/jupyterhub/tests/test_scopes.py @@ -311,7 +311,7 @@ def create_user_with_scopes(app, create_temp_role): 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 = [] + temp_services = [] counter = 0 role_function = create_temp_role @@ -325,11 +325,12 @@ def create_service_with_scopes(app, create_temp_role): app.init_services() orm_service = orm.Service.find(app.db, name) app.db.commit() + temp_services.append(orm_service) roles.update_roles(app.db, orm_service, roles=[role.name]) return orm_service yield temp_service_creator - for service in temp_service: + for service in temp_services: app.db.delete(service) app.db.commit() From c13ad804fe3b5278df022b947c2588a7734b948a Mon Sep 17 00:00:00 2001 From: 0mar Date: Tue, 25 May 2021 13:51:43 +0200 Subject: [PATCH 17/45] Added default roles for users and unified admin check --- jupyterhub/app.py | 44 +++++++++++++++++++-------------- jupyterhub/auth.py | 2 +- jupyterhub/tests/test_roles.py | 8 +++++- jupyterhub/tests/test_scopes.py | 1 - 4 files changed, 34 insertions(+), 21 deletions(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index 7ee594ca..816dd06c 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -1894,7 +1894,7 @@ class JupyterHub(Application): ) # add allowed users to the db - for name in allowed_users: # fixme: Do we add standard roles here? + for name in allowed_users: user = orm.User.find(db, name) if user is None: user = orm.User(name=name) @@ -1995,6 +1995,14 @@ class JupyterHub(Application): db = self.db # load predefined roles from config file + for role_spec in self.load_roles: + if role_spec['name'] == 'admin' and self.admin_users: + app_log.info( + "Extending admin role assignment with config admin users: %s", + str(self.admin_users), + ) + role_spec['users'].extend(self.admin_users) + role_spec['users'] = set(role_spec['users']) self.log.debug('Loading predefined roles from config file to database') for predef_role in self.load_roles: predef_role_obj = orm.Role.find(db, name=predef_role['name']) @@ -2018,24 +2026,22 @@ class JupyterHub(Application): Class = orm.get_class(bearer) orm_obj = Class.find(db, bname) orm_role_bearers.append(orm_obj) - # roles.grant_role( - # db, entity=orm_obj, rolename=predef_role['name'] - # ) + # Ensure all with admin role have admin flag + if predef_role['name'] == 'admin': + orm_obj.admin = True setattr(predef_role_obj, bearer, orm_role_bearers) - for entity in db.query(orm.Service): - if entity.admin: - roles.grant_role(db, entity, 'admin') - else: - roles.assign_default_roles(db, entity) - for entity in db.query( - orm.User - ): # fixme: why can't I combine these expressions? - if entity.admin: - roles.grant_role(db, entity, 'admin') - else: - roles.assign_default_roles(db, entity) - # make sure that on hub upgrade, all roles are reset + db.commit() + allowed_users = db.query(orm.User).filter( + orm.User.name.in_(self.authenticator.allowed_users) + ) + for user in allowed_users: + roles.grant_role(db, user, 'user') + for admin_user in db.query(orm.User).filter_by(admin=True): + roles.grant_role(db, admin_user, 'admin') + for admin_service in db.query(orm.Service).filter_by(admin=True): + roles.grant_role(db, admin_service, 'admin') + # make sure that on hub upgrade, all roles are reset if not getattr(self, '_rbac_upgrade', False): app_log.warning( "No admin role found; assuming hub upgrade. Initializing default roles for all entities" @@ -2150,7 +2156,9 @@ 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): + if spec.get( + 'admin', False + ): # Todo: fix double assignment of admin roles roles.update_roles(self.db, entity=orm_service, roles=['admin']) self.db.add(orm_service) orm_service.admin = spec.get('admin', False) diff --git a/jupyterhub/auth.py b/jupyterhub/auth.py index c4151d18..a62c1103 100644 --- a/jupyterhub/auth.py +++ b/jupyterhub/auth.py @@ -112,7 +112,7 @@ class Authenticator(LoggingConfigurable): Use this with supported authenticators to restrict which users can log in. This is an additional list that further restricts users, beyond whatever restrictions the - authenticator has in place. + authenticator has in place. Any user in this list is granted the 'user' role on hub startup. If empty, does not perform any additional restriction. diff --git a/jupyterhub/tests/test_roles.py b/jupyterhub/tests/test_roles.py index d3d66365..93071a40 100644 --- a/jupyterhub/tests/test_roles.py +++ b/jupyterhub/tests/test_roles.py @@ -446,7 +446,7 @@ async def test_load_roles_users(tmpdir, request): assert len(user.roles) == len(set(user.roles)) if user.admin: assert admin_role in user.roles - assert user_role not in user.roles + assert user_role in user.roles # test if predefined roles loaded and assigned teacher_role = orm.Role.find(db, name='teacher') @@ -953,3 +953,9 @@ async def test_config_role_users(): user = orm.User.find(hub.db, name=user_name) role = orm.Role.find(hub.db, name=role_name) assert role not in user.roles + + +# todo: test admin flag -> admin role and other way around +# todo: test custom user role reset on startup +# todo: test removal from config -> removal from database +# todo: test customizing user scopes -/> membership changes diff --git a/jupyterhub/tests/test_scopes.py b/jupyterhub/tests/test_scopes.py index 5bd3825e..1d501a17 100644 --- a/jupyterhub/tests/test_scopes.py +++ b/jupyterhub/tests/test_scopes.py @@ -620,7 +620,6 @@ async def test_server_state_access( ) service = create_service_with_scopes(*scopes) api_token = service.new_api_token() - await app.init_role_creation() headers = {'Authorization': 'token %s' % api_token} r = await api_request(app, 'users', user.name, headers=headers) r.raise_for_status() From d39673eea23238757f1360a53246db9b6bc90e47 Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 26 May 2021 12:28:59 +0200 Subject: [PATCH 18/45] Flesh out oauth details doc adress review, add emoji, expand details, examlpes, and add discussion of caching and revocation. --- docs/source/rbac/scopes.md | 2 +- docs/source/reference/index.rst | 1 + docs/source/reference/oauth.md | 238 ++++++++++++++++++++++++++------ 3 files changed, 199 insertions(+), 42 deletions(-) diff --git a/docs/source/rbac/scopes.md b/docs/source/rbac/scopes.md index 9478525d..74ee6d30 100644 --- a/docs/source/rbac/scopes.md +++ b/docs/source/rbac/scopes.md @@ -1,4 +1,4 @@ -# Scopes +# Scopes in JupyterHub A scope has a syntax-based design that reveals which resources it provides access to. Resources are objects with a type, associated data, relationships to other resources, and a set of methods that operate on them (see [RESTful API](https://restful-api-design.readthedocs.io/en/latest/resources.html) documentation for more information). diff --git a/docs/source/reference/index.rst b/docs/source/reference/index.rst index ed478fa1..76177072 100644 --- a/docs/source/reference/index.rst +++ b/docs/source/reference/index.rst @@ -26,3 +26,4 @@ what happens under-the-hood when you deploy and configure your JupyterHub. config-proxy config-sudo config-reference + oauth diff --git a/docs/source/reference/oauth.md b/docs/source/reference/oauth.md index 185239cd..a658eb90 100644 --- a/docs/source/reference/oauth.md +++ b/docs/source/reference/oauth.md @@ -2,45 +2,60 @@ JupyterHub uses OAuth 2 internally as a mechanism for authenticating users. As such, JupyterHub itself always functions as an OAuth **provider**. -More on what that means below. +More on what that means [below](oauth-terms). Additionally, JupyterHub is _often_ deployed with [oauthenticator](https://oauthenticator.readthedocs.io), where an external identity provider, such as GitHub or KeyCloak, is used to authenticate users. -When this is the case, there are \*two nested +When this is the case, there are _two_ nested oauth flows: +an _internal_ oauth flow where JupyterHub is the **provider**, +and and _external_ oauth flow, where JupyterHub is a **client**. This means that when you are using JupyterHub, there is always _at least one_ and often two layers of OAuth involved in a user logging in and accessing their server. Some relevant points: -- Single-user servers _never_ need to communicate with or be aware of the upstream provider. +- Single-user servers _never_ need to communicate with or be aware of the upstream provider configured in your Authenticator. As far as they are concerned, only JupyterHub is an OAuth provider, and how users authenticate with the Hub itself is irrelevant. - When talking to a single-user server, there are ~always two tokens: a token issued to the server itself to communicate with the Hub API, and a second per-user token in the browser to represent the completed login process and authorized permissions. - More on this later. + More on this [later](two-tokens). -### Key OAuth terms +(oauth-terms)= -- **provider** the entity responsible for managing. +## Key OAuth terms + +Here are some key definitions to keep in mind when we are talking about OAuth. +You can also read more detail [here](https://www.oauth.com/oauth2-servers/definitions/). + +- **provider** the entity responsible for managing identity and authorization, + always a web server. JupyterHub is _always_ an oauth provider for JupyterHub's components. When OAuthenticator is used, an external service, such as GitHub or KeyCloak, is also an oauth provider. -- **client** An entity that requests OAuth tokens on a user's behalf. - JupyterHub _services_ or single-user _servers_ are OAuth clients of the JupyterHub _provider_. - When OAuthenticator is used, JupyterHub is itself also an OAuth _client_ for the external oauth _provider_, e.g. GitHub. +- **client** An entity that requests OAuth **tokens** on a user's behalf, + generally a web server of some kind. + OAuth **clients** are services that _delegate_ authentication and/or authorization + to an OAuth **provider**. + JupyterHub _services_ or single-user _servers_ are OAuth **clients** of the JupyterHub **provider**. + When OAuthenticator is used, JupyterHub is itself _also_ an OAuth **client** for the external oauth **provider**, e.g. GitHub. - **browser** A user's web browser, which makes requests and stores things like cookies - **token** The secret value used to represent a user's authorization. This is the final product of the OAuth process. +- **code** A short-lived temporary secret that the **client** exchanges + for a **token** at the conclusion of oauth, + in what's generally called the "oauth callback handler." -### The oauth flow +## One oauth flow -OAuth flow is what we call the sequence of HTTP requests involved in authenticating a user and issuing a token, ultimately used for authorized access to a service or single-user server. +OAuth **flow** is what we call the sequence of HTTP requests involved in authenticating a user and issuing a token, ultimately used for authorized access to a service or single-user server. -It generally goes like this: +A single oauth flow generally goes like this: -#### Oauth request and redirect -1. A _browser_ makes an HTTP request to an oauth _client_. -2. There are no credentials, so the client _redirects_ the browser to an "authorize" page on the oauth _provider_ with some extra information: +### OAuth request and redirect + +1. A **browser** makes an HTTP request to an oauth **client**. +2. There are no credentials, so the client _redirects_ the browser to an "authorize" page on the oauth **provider** with some extra information: - the oauth **client id** of the client itself - the **redirect uri** to be redirected back to after completion - the **scopes** requested, which the user should be presented with to confirm. @@ -49,14 +64,17 @@ It generally goes like this: the browser must be _authenticated_ with the provider. This is often already stored in a cookie, but if not the provider webapp must begin its _own_ authentication process before serving the authorization page. + This _may_ even begin another oauth flow! 4. After the user tells the provider that they want to proceed with the authorization, the provider records this authorization in a short-lived record called an **oauth code**. -5. Finally, - the oauth provider redirects the browser _back_ to the oauth client's "redirect uri" +5. Finally, the oauth provider redirects the browser _back_ to the oauth client's "redirect uri" (or "oauth callback uri"), with the oauth code in a url parameter. -#### State after redirect +That's the end of the requests made between the **browser** and the **provider**. + +### State after redirect + At this point: - The browser is authenticated with the _provider_ @@ -65,7 +83,8 @@ At this point: - All requests so far have been made directly by the browser. No requests have originated at the client or provider. -#### OAuth Client Handles Callback Request +### OAuth Client Handles Callback Request + Now we get to finish the OAuth process. Let's dig into what the oauth client does when it handles the oauth callback request with the @@ -74,30 +93,44 @@ the oauth callback request with the This is the first direct request between the OAuth _client_ and the _provider_. - Once the token is retrieved, the client _usually_ makes a second API request to the _provider_ - to retrieve information about the owner of the token (the user) + to retrieve information about the owner of the token (the user). + This is the step where behavior diverges for different OAuth providers. + Up to this point, all oauth providers are the same, following the oauth specification. + However, oauth does not define a standard for exchanging tokens for information about their owner or permissions ([OpenID Connect](https://openid.net/connect/) does that), + so this step may be different for each OAuth provider. - Finally, the oauth client stores its own record that the user is authorized in a cookie. This could be the token itself, or any other appropriate representation of successful authentication. +- Last of all, now that credentials have been established, + the browser can be redirected to the _original_ URL where it started, + to try the request again. + If the client wasn't able to keep track of the original URL all this time + (not always easy!), + you might end up back at a default landing page instead of where you started the login process. This is frustrating! -_phew_ +😮‍💨 _phew_. So that's _one_ OAuth process. ## Full sequence of OAuth in JupyterHub -Let's go through the above oauth process in Jupyter, +Let's go through the above oauth process in JupyterHub, with specific examples of each HTTP request and what information is contained. +For bonus points, we are using the double-oauth example of JupyterHub configured with GitHubOAuthenticator. + +To disambiguate, we will call the OAuth process where JupyterHub is the **provider** "internal oauth," +and the one with JupyterHub as a **client** "external oauth." Our starting point: - a user's single-user server is running. Let's call them `danez` -- jupyterhub is running with GitHub as an oauth provider, +- jupyterhub is running with GitHub as an oauth provider (this means two full instances of oauth), - Danez has a fresh browser session with no cookies yet First request: - browser->single-user server running JupyterLab or Jupyter Classic - `GET /user/danez/notebooks/mynotebook.ipynb` -- no credentials, so client starts oauth process with JupyterHub +- no credentials, so single-user server (as an oauth **client**) starts internal oauth process with JupyterHub (the **provider**) - response: 302 redirect -> `/hub/api/oauth2/authorize` with: - client-id=`jupyterhub-user-danez` @@ -107,23 +140,38 @@ Second request, following redirect: - browser->jupyterhub - `GET /hub/api/oauth2/authorize` -- no credentials, so jupyterhub starts oauth process _with GitHub_ -- response: 302 redirect -> `/hub/api/oauth2/authorize` +- no credentials, so jupyterhub starts external oauth process _with GitHub_ +- response: 302 redirect -> `https://github.com/login/oauth/authorize` with: - client-id=`jupyterhub-client-uuid` - redirect-uri=`/hub/oauth_callback` (we'll come back later!) +_pause_ This is where JupyterHub configuration comes into play. +Recall, in this case JupyterHub is using: + +```python +c.JupyterHub.authenticator_class = 'github' +``` + +That means authenticating a request to the Hub itself starts +a _second_, external oauth process with GitHub as a provider. +This external oauth process is optional, though. +If you were using the default username+password PAMAuthenticator, +this redirect would have been to `/hub/login` instead, to present the user +with a login form. + Third request, following redirect: - browser->GitHub - `GET https://github.com/login/oauth/authorize` -Prompts for login and asks for confirmation of authorization. +Here, GitHub prompts for login and asks for confirmation of authorization +(more redirects if you aren't logged in to GitHub yet, but ultimately back to this `/authorize` URL). After successful authorization (either by looking up a pre-existing authorization, or recording it via form submission) -GitHub issues oauth code and redirects to `/hub/oauth_callback?code=github-code` +GitHub issues an **oauth code** and redirects to `/hub/oauth_callback?code=github-code` Next request: @@ -136,39 +184,42 @@ The first: - JupyterHub->GitHub - `POST https://github.com/login/oauth/access_token` -- request made with oauth code from url parameter -- response includes an access token +- request made with oauth **code** from url parameter +- response includes an access **token** The second: - JupyterHub->GitHub - `GET https://api.github.com/user` -- request made with access token in the `Authorization` header +- request made with access **token** in the `Authorization` header - response is the user model, including username, email, etc. -Now the oauth callback request completes with: +Now the external oauth callback request completes with: -- set cookie on `/hub/` recording jupyterhub authentication so we don't need to do oauth with github again for a while +- set cookie on `/hub/` path, recording jupyterhub authentication so we don't need to do external oauth with GitHub again for a while - redirect -> `/hub/api/oauth2/authorize` +🎉 At this point, we have completed our first OAuth flow! 🎉 + Now, we get our first repeated request: - browser->jupyterhub - `GET /hub/api/oauth2/authorize` - this time with credentials, so jupyterhub either - 1. serves the authorization confirmation page, or + 1. serves the internal authorization confirmation page, or 2. automatically accepts authorization (shortcut taken when a user is visiting their own server) - redirect -> `/user/danez/oauth_callback?code=jupyterhub-code` -Here, we start the same oauth callback process as before, but at Danez's single-user server +Here, we start the same oauth callback process as before, but at Danez's single-user server for the _internal_ oauth - browser->single-user server - `GET /user/danez/oauth_callback` (in handler) -Inside the callback handler, Danez's server makes two API requests to JupyterHub: +Inside the internal oauth callback handler, +Danez's server makes two API requests to JupyterHub: The first: @@ -204,14 +255,119 @@ To authenticate this request, the single token stored in the encrypted cookie is If the user model matches who should be allowed (e.g. Danez), then the request is allowed. +See {doc}`../rbac/scopes` for how JupyterHub uses scopes to determine authorized access to servers and services. _the end_ -## A tale of two tokens +## Token caches and expiry -**TODO**: discuss API token issued to server at startup and oauth-issued token in cookie, and some details of how JupyterLab currently deals with that. -` +Because tokens represent information from an external source, +they can become 'stale,' +or the information they represent may no longer be accurate. +For example: a user's GitHub account may no longer be authorized to use JupyterHub, +that should ultimately propagate to revoking access and force logging in again. -## Notes +To handle this, OAuth tokens and the various places they are stored can _expire_, +which should have the same effect as no credentials, +and trigger the authorization process again. -- I omitted some information about the distinction between tokens issued to the server, due to RBAC changes. But they are different! +In JupyterHub's internal oauth, we have these layers of information that can go stale: + +- The oauth client has a **cache** of Hub responses for tokens, + so it doesn't need to make API requests to the Hub for every request it receives. + This cache has an expiry of five minutes by default, + and is governed by the configuration `HubAuth.cache_max_age` in the single-user server. +- The internal oauth token is stored in a cookie, which has its own expiry (default: 14 days), + governed by `JupyterHub.cookie_max_age_days`. +- The internal oauth token can also itself expire, + which is by default the same as the cookie expiry, + since it makes sense for the token itself and the place it is stored to expire at the same time. + This is governed by `JupyterHub.cookie_max_age_days` first, + or can overridden by `JupyterHub.oauth_token_expires_in`. + +That's all for _internal_ auth storage, +but the information from the _external_ authentication provider +(could be PAM or GitHub OAuth, etc.) can also expire. +Authenticator configuration governs when JupyterHub needs to ask again, +triggering the external login process anew before letting a user proceed. + +- `jupyterhub-hub-login` cookie stores that a browser is authenticated with the Hub. + This expires according to `JupyterHub.cookie_max_age_days` configuration, + with a default of 14 days. + The `jupyterhub-hub-login` cookie is encrypted with `JupyterHub.cookie_secret` + configuration. +- {meth}`.Authenticator.refresh_user` is a method to refresh a user's auth info. + By default, it does nothing, but it can return an updated user model if a user's information has changed, + or force a full login process again if needed. +- {attr}`.Authenticator.auth_refresh_age` configuration governs how often + `refresh_user()` will be called to check if a user must login again (default: 300 seconds). +- {attr}`.Authenticator.refresh_pre_spawn` configuration governs whether + `refresh_user()` should be called prior to spawning a server, + to force fresh auth info when a server is launched (default: False). + This can be useful when Authenticators pass access tokens to spawner environments, to ensure they aren't getting a stale token that's about to expire. + +**So what happens when these things expire or get stale?** + +- If the HubAuth **token response cache** expires, + when a request is made with a token, + the Hub is asked for the latest information about the token. + This usually has no visible effect, since it is just refreshing a cache. + If it turns out that the token itself has expired or been revoked, + the request will be denied. +- If the token has expired, but is still in the cookie: + when the token response cache expires, + the next time the server asks the hub about the token, + no user will be identified and the internal oauth process begins again. +- If the token _cookie_ expires, the next browser request will be made with no credentials, + and the internal oauth process will begin again. + This will usually have the form of a transparent redirect browsers won't notice. + However, if this occurs on an API request in a long-lived page visit + such as a JupyterLab session, the API request may fail and require + a page refresh to get renewed credentials. +- If the _JupyterHub_ cookie expires, the next time the browser makes a request to the Hub, + the Hub's authorization process must begin again (e.g. login with GitHub). + Hub cookie expiry on its own **does not** mean that a user can no longer access their single-user server! +- If credentials from the upstream provider (e.g. GitHub) become stale or outdated, + these will not be refreshed until/unless `refresh_user` is called + _and_ `refresh_user()` on the given Authenticator is implemented to perform such a check. + At this point, few Authenticators implement `refresh_user` to support this feature. + If your Authenticator does not or cannot implement `refresh_user`, + the only way to force a check is to reset the `JupyterHub.cookie_secret` encryption key, + which invalidates the `jupyterhub-hub-login` cookie for all users. + +### Logging out + +Logging out of JupyterHub means clearing and revoking many of these credentials: + +- The `jupyterhub-hub-login` cookie is revoked, meaning the next request to the Hub itself will require a new login. +- The token stored in the `jupyterhub-user-username` cookie for the single-user server + will be revoked, based on its associaton with `jupyterhub-session-id`, but the _cookie itself cannot be cleared at this point_ +- The shared `jupyterhub-session-id` is cleared, which ensures that the HubAuth **token response cache** will not be used, + and the next request with the expired token will ask the Hub, which will inform the single-user server that the token has expired + +## Extra bits + +(two-tokens)= + +### A tale of two tokens + +**TODO**: discuss API token issued to server at startup ($JUPYTERHUB_API_TOKEN) +and oauth-issued token in the cookie, +and some details of how JupyterLab currently deals with that. +They are different, and JupyterLab should be making requests using the token from the cookie, +not the token from the server, +but that is not currently the case. + +### Redirect loops + +In general, an authenticated web endpoint has this behavior, +based on the authentication/authorization state of the browser: + +- If authorized, allow the request to happen +- If authenticated (I know who you are) but not authorized (you are not allowed), fail with a 403 permission denied error +- If not authenticated, start a redirect process to establish authorization, + which should end in a redirect back to the original URL to try again. + **This is why problems in authentication result in redirect loops!** + If the second request fails to detect the authentication that should have been established during the redirect, + it will start the authentication redirect process over again, + and keep redirecting in a loop until the browser balks. From 3ba8e11553f614bc8da083efe918f54dccdb60a0 Mon Sep 17 00:00:00 2001 From: 0mar Date: Wed, 26 May 2021 15:39:45 +0200 Subject: [PATCH 19/45] Added tests and fixed bugs --- jupyterhub/app.py | 48 ++++++++---- jupyterhub/tests/test_roles.py | 129 ++++++++++++++++++++++++++++++--- 2 files changed, 152 insertions(+), 25 deletions(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index 816dd06c..5fbc2f36 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -21,6 +21,7 @@ from datetime import timezone from functools import partial from getpass import getuser from glob import glob +from itertools import chain from operator import itemgetter from textwrap import dedent from urllib.parse import unquote @@ -1978,13 +1979,15 @@ class JupyterHub(Application): """Load default and predefined roles into the database""" self.log.debug('Loading default roles to database') default_roles = roles.get_default_roles() - default_role_names = [r['name'] for r in default_roles] - init_roles = default_roles + self.load_roles + config_role_names = [r['name'] for r in self.load_roles] + init_roles = self.load_roles.copy() + for role_spec in default_roles: + if role_spec['name'] not in config_role_names: + init_roles.append(role_spec) if not orm.Role.find(self.db, name='admin'): self._rbac_upgrade = True - for role in self.db.query(orm.Role): - if role.name not in default_role_names: - self.db.delete(role) + self.db.query(orm.Role).delete() + app_log.info("Deleting all roles in database") self.db.commit() for role in init_roles: roles.create_role(self.db, role) @@ -1992,20 +1995,23 @@ class JupyterHub(Application): async def init_role_assignment(self): # tokens are added separately role_bearers = ['users', 'services', 'groups'] - + config_admin_users = set(self.admin_users) | self.authenticator.admin_users db = self.db # load predefined roles from config file for role_spec in self.load_roles: - if role_spec['name'] == 'admin' and self.admin_users: + if role_spec['name'] == 'admin' and config_admin_users: app_log.info( "Extending admin role assignment with config admin users: %s", - str(self.admin_users), + str(config_admin_users), ) - role_spec['users'].extend(self.admin_users) - role_spec['users'] = set(role_spec['users']) + role_spec['users'] = set(role_spec.get('users', [])) + role_spec['users'] |= config_admin_users self.log.debug('Loading predefined roles from config file to database') + has_admin_role_spec = False for predef_role in self.load_roles: predef_role_obj = orm.Role.find(db, name=predef_role['name']) + if predef_role['name'] == 'admin': + has_admin_role_spec = bool(predef_role.get('users', False)) # add users, services, and/or groups, # tokens need to be checked for permissions for bearer in role_bearers: @@ -2036,11 +2042,23 @@ class JupyterHub(Application): ) for user in allowed_users: roles.grant_role(db, user, 'user') - for admin_user in db.query(orm.User).filter_by(admin=True): - roles.grant_role(db, admin_user, 'admin') - for admin_service in db.query(orm.Service).filter_by(admin=True): - roles.grant_role(db, admin_service, 'admin') - + admin_role = orm.Role.find(db, 'admin') + admin_objs = chain( + db.query(orm.User).filter_by(admin=True), + db.query(orm.Service).filter_by(admin=True), + ) + for admin_obj in admin_objs: + if has_admin_role_spec: + app_log.debug( + "Admin users explicitly specified in config, so previous db state is ignored" + ) + admin_obj.admin = admin_role in admin_obj.roles + else: + app_log.debug( + "Admin users not specified in config, elevate to admin based on previous db state" + ) + roles.grant_role(db, admin_obj, 'admin') + db.commit() # make sure that on hub upgrade, all roles are reset if not getattr(self, '_rbac_upgrade', False): app_log.warning( diff --git a/jupyterhub/tests/test_roles.py b/jupyterhub/tests/test_roles.py index e2883500..a3d41830 100644 --- a/jupyterhub/tests/test_roles.py +++ b/jupyterhub/tests/test_roles.py @@ -967,18 +967,16 @@ async def test_config_role_list(): 'scopes': ['shutdown'], }, ] - kwargs = {'load_roles': roles_to_load} - hub = MockHub(**kwargs) + hub = MockHub(load_roles=roles_to_load) hub.init_db() hub.authenticator.admin_users = ['admin'] await hub.init_role_creation() for role_conf in roles_to_load: assert orm.Role.find(hub.db, name=role_conf['name']) - # Now reload and see if user is removed from role list + # Now remove elephant from config and see if it is removed from database roles_to_load.pop(0) hub.load_roles = roles_to_load await hub.init_role_creation() - print(hub.db.query(orm.Role).all()) assert orm.Role.find(hub.db, name='tiger') assert not orm.Role.find(hub.db, name='elephant') @@ -1001,8 +999,7 @@ async def test_config_role_users(): 'users': user_names, }, ] - kwargs = {'load_roles': roles_to_load} - hub = MockHub(**kwargs) + hub = MockHub(load_roles=roles_to_load) hub.init_db() hub.authenticator.admin_users = ['admin'] hub.authenticator.allowed_users = user_names @@ -1023,7 +1020,119 @@ async def test_config_role_users(): assert role not in user.roles -# todo: test admin flag -> admin role and other way around -# todo: test custom user role reset on startup -# todo: test removal from config -> removal from database -# todo: test customizing user scopes -/> membership changes +async def test_admin_role_and_flag(): + admin_role_spec = [ + { + 'name': 'admin', + 'users': ['eddy'], + } + ] + hub = MockHub(load_roles=admin_role_spec) + hub.init_db() + hub.authenticator.admin_users = ['admin'] + hub.authenticator.allowed_users = ['eddy'] + await hub.init_role_creation() + await hub.init_users() + await hub.init_role_assignment() + admin_role = orm.Role.find(hub.db, name='admin') + for user_name in ['eddy', 'admin']: + user = orm.User.find(hub.db, name=user_name) + assert user.admin + assert admin_role in user.roles + admin_role_spec[0]['users'].remove('eddy') + hub.load_roles = admin_role_spec + await hub.init_users() + await hub.init_role_assignment() + user = orm.User.find(hub.db, name='eddy') + assert not user.admin + assert admin_role not in user.roles + + +async def test_custom_role_reset(): + user_role_spec = [ + { + 'name': 'user', + 'scopes': ['self', 'shutdown'], + 'users': ['eddy'], + } + ] + hub = MockHub(load_roles=user_role_spec) + hub.init_db() + hub.authenticator.allowed_users = ['eddy'] + await hub.init_role_creation() + await hub.init_users() + await hub.init_role_assignment() + user_role = orm.Role.find(hub.db, name='user') + user = orm.User.find(hub.db, name='eddy') + assert user_role in user.roles + assert 'shutdown' in user_role.scopes + hub.load_roles = [] + await hub.init_role_creation() + await hub.init_users() + await hub.init_role_assignment() + user_role = orm.Role.find(hub.db, name='user') + user = orm.User.find(hub.db, name='eddy') + assert user_role in user.roles + assert 'shutdown' not in user_role.scopes + + +async def test_removal_config_to_db(): + role_spec = [ + { + 'name': 'user', + 'scopes': ['self', 'shutdown'], + }, + { + 'name': 'wizard', + 'scopes': ['self', 'read:groups'], + }, + ] + hub = MockHub(load_roles=role_spec) + hub.init_db() + await hub.init_role_creation() + assert orm.Role.find(hub.db, 'user') + assert orm.Role.find(hub.db, 'wizard') + hub.load_roles = [] + await hub.init_role_creation() + assert orm.Role.find(hub.db, 'user') + assert not orm.Role.find(hub.db, 'wizard') + + +async def test_user_config_respects_memberships(): + role_spec = [ + { + 'name': 'user', + 'scopes': ['self', 'shutdown'], + } + ] + user_names = ['eddy', 'carol'] + hub = MockHub(load_roles=role_spec) + hub.init_db() + hub.authenticator.allowed_users = user_names + await hub.init_role_creation() + await hub.init_users() + await hub.init_role_assignment() + user_role = orm.Role.find(hub.db, 'user') + for user_name in user_names: + user = orm.User.find(hub.db, user_name) + assert user in user_role.users + + +async def test_admin_role_respects_config(): + role_spec = [ + { + 'name': 'admin', + 'scopes': ['self', 'shutdown'], + } + ] + admin_users = ['eddy', 'carol'] + hub = MockHub(load_roles=role_spec) + hub.init_db() + hub.authenticator.admin_users = admin_users + await hub.init_role_creation() + await hub.init_users() + await hub.init_role_assignment() + admin_role = orm.Role.find(hub.db, 'admin') + for user_name in admin_users: + user = orm.User.find(hub.db, user_name) + assert user in admin_role.users From b399158060517eabadc968313f0ee0ee7d384337 Mon Sep 17 00:00:00 2001 From: IvanaH8 Date: Wed, 26 May 2021 16:45:53 +0200 Subject: [PATCH 20/45] Create scope_hierarchy dict automatically from scope_definitions --- jupyterhub/roles.py | 48 ++++++++++++++++++++------------------------ jupyterhub/scopes.py | 10 +++++---- 2 files changed, 28 insertions(+), 30 deletions(-) diff --git a/jupyterhub/roles.py b/jupyterhub/roles.py index b51bb84d..e446582b 100644 --- a/jupyterhub/roles.py +++ b/jupyterhub/roles.py @@ -8,6 +8,7 @@ from sqlalchemy import func from tornado.log import app_log from . import orm +from . import scopes def get_default_roles(): @@ -75,35 +76,30 @@ def expand_self_scope(name): def _get_scope_hierarchy(): """ - Returns a dictionary of scopes: - scopes.keys() = scopes of highest level and scopes that have their own subscopes - scopes.values() = a list of first level subscopes or None + Returns: + scope hierarchy (dict): dictionary of available scopes and their hierarchy where + scopes.keys() = top level scopes and scopes that have their own subscopes + scopes.values() = list of immediate subscopes or None """ + subscope_lists = [ + value['subscopes'] + for value in scopes.scope_definitions.values() + if 'subscopes' in value + ] - scopes = { - 'self': None, - 'all': None, - 'admin:users': ['admin:users:auth_state', 'users'], - 'users': ['read:users', 'users:activity'], - 'read:users': [ - 'read:users:name', - 'read:users:groups', - 'read:users:activity', - ], - 'users:activity': ['read:users:activity'], - 'users:tokens': ['read:users:tokens'], - 'admin:users:servers': ['admin:users:server_state', 'users:servers'], - 'users:servers': ['read:users:servers'], - 'read:users:servers': ['read:users:name'], - 'admin:groups': ['groups'], - 'groups': ['read:groups'], - 'read:services': None, - 'read:hub': None, - 'proxy': None, - 'shutdown': None, - } + scope_hierarchy = {} + for scope in scopes.scope_definitions.keys(): - return scopes + has_subscopes = scopes.scope_definitions[scope].get('subscopes') + is_subscope = any(scope in subscope_list for subscope_list in subscope_lists) + + if has_subscopes: + scope_hierarchy[scope] = scopes.scope_definitions[scope]['subscopes'] + else: + if not is_subscope: + scope_hierarchy[scope] = None + + return scope_hierarchy def horizontal_filter(func): diff --git a/jupyterhub/scopes.py b/jupyterhub/scopes.py index 21058416..eaf86273 100644 --- a/jupyterhub/scopes.py +++ b/jupyterhub/scopes.py @@ -36,13 +36,13 @@ scope_definitions = { 'read:users:name', 'read:users:groups', 'read:users:activity', - 'read:users:roles', ], + # TODO: add read:users:admin and read:users:roles as subscopes here once implemented }, 'read:users:name': {'description': 'Read-only access to users’ names.'}, 'read:users:groups': {'description': 'Read-only access to users’ group names.'}, - 'read:users:roles': {'description': 'Read-only access to users’ role names.'}, 'read:users:activity': {'description': 'Read-only access to users’ last activity.'}, + # TODO: add read:users:admin and read:users:roles once implemented 'users:activity': { 'description': 'Grants access to read and post users’ last activity only.', 'subscopes': ['read:users:activity'], @@ -78,10 +78,12 @@ scope_definitions = { 'read:groups': {'description': 'Read-only access to groups’ models.'}, 'read:services': { 'description': 'Read-only access to service models.', - 'subscopes': ['read:services:name', 'read:services:roles'], + 'subscopes': ['read:services:name'], + # TODO: add read:services:admin and read:services:roles as subscopes here once implemented }, 'read:services:name': {'description': 'Read-only access to service names.'}, - 'read:services:roles': {'description': 'Read-only access to service role names.'}, + # TODO: add read:services:admin and read:services:roles once implemented + #'read:services:roles': {'description': 'Read-only access to service role names.'}, 'read:hub': { 'description': 'Read-only access to detailed information about the Hub.' }, From 290a697df2ab5565e51b2b385c22a8f3f13870b1 Mon Sep 17 00:00:00 2001 From: 0mar Date: Wed, 26 May 2021 16:55:20 +0200 Subject: [PATCH 21/45] Fixed service admin declaration --- jupyterhub/app.py | 65 ++++++++++++++++++++++++++--------------------- 1 file changed, 36 insertions(+), 29 deletions(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index 5fbc2f36..3cf2c1ae 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -1995,23 +1995,34 @@ class JupyterHub(Application): async def init_role_assignment(self): # tokens are added separately role_bearers = ['users', 'services', 'groups'] + admin_role_objects = ['users', 'services'] config_admin_users = set(self.admin_users) | self.authenticator.admin_users db = self.db # load predefined roles from config file - for role_spec in self.load_roles: - if role_spec['name'] == 'admin' and config_admin_users: - app_log.info( - "Extending admin role assignment with config admin users: %s", - str(config_admin_users), - ) - role_spec['users'] = set(role_spec.get('users', [])) - role_spec['users'] |= config_admin_users + if config_admin_users: + for role_spec in self.load_roles: + if role_spec['name'] == 'admin': + app_log.info( + "Extending admin role assignment with config admin users: %s", + str(config_admin_users), + ) + role_spec['users'] = set(role_spec.get('users', [])) + role_spec['users'] |= config_admin_users self.log.debug('Loading predefined roles from config file to database') - has_admin_role_spec = False + has_admin_role_spec = {role_bearer: False for role_bearer in admin_role_objects} for predef_role in self.load_roles: predef_role_obj = orm.Role.find(db, name=predef_role['name']) - if predef_role['name'] == 'admin': - has_admin_role_spec = bool(predef_role.get('users', False)) + for bearer in admin_role_objects: + if predef_role['name'] == 'admin': + has_admin_role_spec[bearer] = bool(predef_role.get(bearer, False)) + if has_admin_role_spec[bearer]: + app_log.debug( + f"Admin {bearer} explicitly specified in config, so previous db state is ignored" + ) + else: + app_log.debug( + f"Admin {bearer} not specified in config, elevate to admin based on previous db state" + ) # add users, services, and/or groups, # tokens need to be checked for permissions for bearer in role_bearers: @@ -2043,21 +2054,13 @@ class JupyterHub(Application): for user in allowed_users: roles.grant_role(db, user, 'user') admin_role = orm.Role.find(db, 'admin') - admin_objs = chain( - db.query(orm.User).filter_by(admin=True), - db.query(orm.Service).filter_by(admin=True), - ) - for admin_obj in admin_objs: - if has_admin_role_spec: - app_log.debug( - "Admin users explicitly specified in config, so previous db state is ignored" - ) - admin_obj.admin = admin_role in admin_obj.roles - else: - app_log.debug( - "Admin users not specified in config, elevate to admin based on previous db state" - ) - roles.grant_role(db, admin_obj, 'admin') + for bearer in admin_role_objects: + Class = orm.get_class(bearer) + for admin_obj in db.query(Class).filter_by(admin=True): + if has_admin_role_spec[bearer]: + admin_obj.admin = admin_role in admin_obj.roles + else: + roles.grant_role(db, admin_obj, 'admin') db.commit() # make sure that on hub upgrade, all roles are reset if not getattr(self, '_rbac_upgrade', False): @@ -2174,9 +2177,13 @@ 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 - ): # Todo: fix double assignment of admin roles + if spec.get('admin', False): + self.log.warning( + f"Service {name} sets `admin: True`, which is deprecated in JupyterHub 2.0." + " You can assign now assign roles via `JupyterHub.load_roles` configuration." + " If you specify services in the admin role configuration, " + "the Service admin flag will be ignored." + ) roles.update_roles(self.db, entity=orm_service, roles=['admin']) self.db.add(orm_service) orm_service.admin = spec.get('admin', False) From 587ea28581e63bd3d8e538b473d252785947319b Mon Sep 17 00:00:00 2001 From: 0mar Date: Thu, 27 May 2021 10:36:23 +0200 Subject: [PATCH 22/45] Added error for duplicate roles --- jupyterhub/app.py | 5 +++++ jupyterhub/tests/test_roles.py | 30 ++++++++++++++++++++++++------ 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index 3cf2c1ae..04de33f3 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -1981,6 +1981,11 @@ class JupyterHub(Application): default_roles = roles.get_default_roles() config_role_names = [r['name'] for r in self.load_roles] init_roles = self.load_roles.copy() + for role_name in config_role_names: + if config_role_names.count(role_name) > 1: + raise AttributeError( + f"Role {role_name} multiply defined. Please check the `load_roles` configuration" + ) for role_spec in default_roles: if role_spec['name'] not in config_role_names: init_roles.append(role_spec) diff --git a/jupyterhub/tests/test_roles.py b/jupyterhub/tests/test_roles.py index a3d41830..f2e370f4 100644 --- a/jupyterhub/tests/test_roles.py +++ b/jupyterhub/tests/test_roles.py @@ -992,12 +992,6 @@ async def test_config_role_users(): 'scopes': ['users', 'groups'], 'users': user_names, }, - { - 'name': role_name, - 'description': 'painting with colors', - 'scopes': ['users', 'groups'], - 'users': user_names, - }, ] hub = MockHub(load_roles=roles_to_load) hub.init_db() @@ -1020,6 +1014,30 @@ async def test_config_role_users(): assert role not in user.roles +async def test_duplicate_role_users(): + role_name = 'painter' + user_name = 'benny' + user_names = ['agnetha', 'bjorn', 'anni-frid', user_name] + roles_to_load = [ + { + 'name': role_name, + 'description': 'painting with colors', + 'scopes': ['users', 'groups'], + 'users': user_names, + }, + { + 'name': role_name, + 'description': 'painting with colors', + 'scopes': ['users', 'groups'], + 'users': user_names, + }, + ] + hub = MockHub(load_roles=roles_to_load) + hub.init_db() + with pytest.raises(AttributeError): + await hub.init_role_creation() + + async def test_admin_role_and_flag(): admin_role_spec = [ { From 320ad75b123a924a7c9bd15f846aa57152e05335 Mon Sep 17 00:00:00 2001 From: Ivana Date: Thu, 27 May 2021 11:04:46 +0200 Subject: [PATCH 23/45] Update jupyterhub/roles.py Co-authored-by: Min RK --- jupyterhub/roles.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jupyterhub/roles.py b/jupyterhub/roles.py index e446582b..b9f36a45 100644 --- a/jupyterhub/roles.py +++ b/jupyterhub/roles.py @@ -88,7 +88,7 @@ def _get_scope_hierarchy(): ] scope_hierarchy = {} - for scope in scopes.scope_definitions.keys(): + for scope, definition in scopes.scope_definitions.items(): has_subscopes = scopes.scope_definitions[scope].get('subscopes') is_subscope = any(scope in subscope_list for subscope_list in subscope_lists) From 05f6892e37b027719c74579380be69a815734806 Mon Sep 17 00:00:00 2001 From: IvanaH8 Date: Thu, 27 May 2021 18:11:33 +0200 Subject: [PATCH 24/45] Get subscopes directly from scopes.scope_definitions no need for _get_scope_hierarchy() --- jupyterhub/roles.py | 88 +++++++++------------------------- jupyterhub/scopes.py | 8 ++-- jupyterhub/tests/test_roles.py | 10 ++++ 3 files changed, 37 insertions(+), 69 deletions(-) diff --git a/jupyterhub/roles.py b/jupyterhub/roles.py index b9f36a45..7229790c 100644 --- a/jupyterhub/roles.py +++ b/jupyterhub/roles.py @@ -74,34 +74,6 @@ def expand_self_scope(name): return {"{}!user={}".format(scope, name) for scope in scope_list} -def _get_scope_hierarchy(): - """ - Returns: - scope hierarchy (dict): dictionary of available scopes and their hierarchy where - scopes.keys() = top level scopes and scopes that have their own subscopes - scopes.values() = list of immediate subscopes or None - """ - subscope_lists = [ - value['subscopes'] - for value in scopes.scope_definitions.values() - if 'subscopes' in value - ] - - scope_hierarchy = {} - for scope, definition in scopes.scope_definitions.items(): - - has_subscopes = scopes.scope_definitions[scope].get('subscopes') - is_subscope = any(scope in subscope_list for subscope_list in subscope_lists) - - if has_subscopes: - scope_hierarchy[scope] = scopes.scope_definitions[scope]['subscopes'] - else: - if not is_subscope: - scope_hierarchy[scope] = None - - return scope_hierarchy - - def horizontal_filter(func): """Decorator to account for horizontal filtering in scope syntax""" @@ -129,29 +101,17 @@ def horizontal_filter(func): def _expand_scope(scopename): """Returns a set of all subscopes""" - scopes = _get_scope_hierarchy() - subscopes = [scopename] + expanded_scope = [] - def _expand_subscopes(index): + def _add_subscopes(scopename): + expanded_scope.append(scopename) + if scopes.scope_definitions[scopename].get('subscopes'): + for subscope in scopes.scope_definitions[scopename].get('subscopes'): + _add_subscopes(subscope) - more_subscopes = list( - filter(lambda scope: scope in scopes.keys(), subscopes[index:]) - ) - for scope in more_subscopes: - subscopes.extend(scopes[scope]) + _add_subscopes(scopename) - if scopename in scopes.keys() and scopes[scopename] is not None: - subscopes.extend(scopes[scopename]) - # record the index from where it should check for "subscopes of sub-subscopes" - index_for_sssc = len(subscopes) - # check for "subscopes of subscopes" - _expand_subscopes(index=1) - # check for "subscopes of sub-subscopes" - _expand_subscopes(index=index_for_sssc) - - expanded_scope = set(subscopes) - - return expanded_scope + return set(expanded_scope) def expand_roles_to_scopes(orm_object): @@ -205,29 +165,27 @@ def _get_subscopes(*args, owner=None): return scopes -def _check_scopes(*args): +def _check_scopes(*args, rolename=None): """Check if provided scopes exist""" - allowed_scopes = _get_scope_hierarchy() + allowed_scopes = set(scopes.scope_definitions.keys()) allowed_filters = ['!user=', '!service=', '!group=', '!server=', '!user'] - subscopes = set( - chain.from_iterable([x for x in allowed_scopes.values() if x is not None]) - ) + + if rolename: + log_role = f"for role {rolename}" + else: + log_role = "" for scope in args: - # check the ! filters - if '!' in scope: - if any(filter in scope for filter in allowed_filters): - scope = scope.split('!', 1)[0] - else: + scopename, _, filter_ = scope.partition('!') + if scopename not in allowed_scopes: + raise NameError(f"Scope '{scope}' {log_role} does not exist") + if filter_: + full_filter = f"!{filter_}" + if not any(full_filter in scope for full_filter in allowed_filters): raise NameError( - 'Scope filter %r in scope %r does not exist', - scope.split('!', 1)[1], - scope, + f"Scope filter '{full_filter}' in scope '{scope}' {log_role} does not exist" ) - # check if the actual scope syntax exists - if scope not in allowed_scopes.keys() and scope not in subscopes: - raise NameError('Scope %r does not exist', scope) def _overwrite_role(role, role_dict): @@ -284,7 +242,7 @@ def create_role(db, role_dict): # check if the provided scopes exist if scopes: - _check_scopes(*scopes) + _check_scopes(*scopes, rolename=role_dict['name']) if role is None: if not scopes: diff --git a/jupyterhub/scopes.py b/jupyterhub/scopes.py index eaf86273..80679107 100644 --- a/jupyterhub/scopes.py +++ b/jupyterhub/scopes.py @@ -37,12 +37,12 @@ scope_definitions = { 'read:users:groups', 'read:users:activity', ], - # TODO: add read:users:admin and read:users:roles as subscopes here once implemented + # TODO: add read:users:roles as subscopes here once implemented }, 'read:users:name': {'description': 'Read-only access to users’ names.'}, 'read:users:groups': {'description': 'Read-only access to users’ group names.'}, 'read:users:activity': {'description': 'Read-only access to users’ last activity.'}, - # TODO: add read:users:admin and read:users:roles once implemented + # TODO: add read:users:roles once implemented 'users:activity': { 'description': 'Grants access to read and post users’ last activity only.', 'subscopes': ['read:users:activity'], @@ -79,10 +79,10 @@ scope_definitions = { 'read:services': { 'description': 'Read-only access to service models.', 'subscopes': ['read:services:name'], - # TODO: add read:services:admin and read:services:roles as subscopes here once implemented + # TODO: add read:services:roles as subscopes here once implemented }, 'read:services:name': {'description': 'Read-only access to service names.'}, - # TODO: add read:services:admin and read:services:roles once implemented + # TODO: add read:services:roles once implemented #'read:services:roles': {'description': 'Read-only access to service role names.'}, 'read:hub': { 'description': 'Read-only access to detailed information about the Hub.' diff --git a/jupyterhub/tests/test_roles.py b/jupyterhub/tests/test_roles.py index 9ea326ea..949d3e28 100644 --- a/jupyterhub/tests/test_roles.py +++ b/jupyterhub/tests/test_roles.py @@ -212,6 +212,16 @@ def test_orm_roles_delete_cascade(db): ), (['read:users:servers'], {'read:users:servers', 'read:users:name'}), (['admin:groups'], {'admin:groups', 'groups', 'read:groups'}), + ( + ['admin:groups', 'read:users:servers'], + { + 'admin:groups', + 'groups', + 'read:groups', + 'read:users:servers', + 'read:users:name', + }, + ), ( ['users:tokens!group=hobbits'], {'users:tokens!group=hobbits', 'read:users:tokens!group=hobbits'}, From 03f968fea027b1f4c4f785de865cc8d9eee2d604 Mon Sep 17 00:00:00 2001 From: 0mar Date: Tue, 1 Jun 2021 12:41:29 +0200 Subject: [PATCH 25/45] wip: fixing errors and suggestions --- jupyterhub/app.py | 62 ++++++++++++++++++------------- jupyterhub/roles.py | 12 ++++-- jupyterhub/tests/test_roles.py | 18 +++++++-- jupyterhub/tests/test_services.py | 2 +- 4 files changed, 60 insertions(+), 34 deletions(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index 04de33f3..d536a85b 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -1980,19 +1980,21 @@ class JupyterHub(Application): self.log.debug('Loading default roles to database') default_roles = roles.get_default_roles() config_role_names = [r['name'] for r in self.load_roles] - init_roles = self.load_roles.copy() + init_roles = default_roles for role_name in config_role_names: if config_role_names.count(role_name) > 1: - raise AttributeError( + raise ValueError( f"Role {role_name} multiply defined. Please check the `load_roles` configuration" ) - for role_spec in default_roles: - if role_spec['name'] not in config_role_names: - init_roles.append(role_spec) + for role_spec in self.load_roles: + init_roles.append(role_spec) if not orm.Role.find(self.db, name='admin'): self._rbac_upgrade = True - self.db.query(orm.Role).delete() - app_log.info("Deleting all roles in database") + for role in self.db.query(orm.Role).filter( + orm.Role.name.in_(config_role_names) + ): + app_log.info(f"Deleting role {role.name}") + role.delete() self.db.commit() for role in init_roles: roles.create_role(self.db, role) @@ -2001,15 +2003,18 @@ class JupyterHub(Application): # tokens are added separately role_bearers = ['users', 'services', 'groups'] admin_role_objects = ['users', 'services'] - config_admin_users = set(self.admin_users) | self.authenticator.admin_users + config_admin_users = set(self.authenticator.admin_users) db = self.db # load predefined roles from config file if config_admin_users: for role_spec in self.load_roles: if role_spec['name'] == 'admin': + app_log.warning( + "Configuration specifies both admin_users and users in the admin role specification" + "If admin role is present in config, it should contain all admin users." + ) app_log.info( - "Extending admin role assignment with config admin users: %s", - str(config_admin_users), + "Merging admin_users set with users list in admin role" ) role_spec['users'] = set(role_spec.get('users', [])) role_spec['users'] |= config_admin_users @@ -2017,16 +2022,14 @@ class JupyterHub(Application): has_admin_role_spec = {role_bearer: False for role_bearer in admin_role_objects} for predef_role in self.load_roles: predef_role_obj = orm.Role.find(db, name=predef_role['name']) - for bearer in admin_role_objects: - if predef_role['name'] == 'admin': - has_admin_role_spec[bearer] = bool(predef_role.get(bearer, False)) + if predef_role['name'] == 'admin': + for bearer in admin_role_objects: + has_admin_role_spec[bearer] = bearer in predef_role if has_admin_role_spec[bearer]: - app_log.debug( - f"Admin {bearer} explicitly specified in config, so previous db state is ignored" - ) + app_log.info(f"Admin role specifies static {bearer} list") else: - app_log.debug( - f"Admin {bearer} not specified in config, elevate to admin based on previous db state" + app_log.info( + f"Admin role does not specify {bearer}, preserving admin membership in database" ) # add users, services, and/or groups, # tokens need to be checked for permissions @@ -2045,14 +2048,21 @@ class JupyterHub(Application): "Username %r is not in Authenticator.allowed_users" % bname ) - Class = orm.get_class(bearer) - orm_obj = Class.find(db, bname) + Class = orm.get_class(bearer) + orm_obj = Class.find(db, bname) + if orm_obj: orm_role_bearers.append(orm_obj) - # Ensure all with admin role have admin flag - if predef_role['name'] == 'admin': - orm_obj.admin = True - setattr(predef_role_obj, bearer, orm_role_bearers) - db.commit() + else: + app_log.warning( + f"User {bname} not added, only found in role definition" + ) + # Todo: Add users to allowed_users + # Ensure all with admin role have admin flag + if predef_role['name'] == 'admin': + orm_obj.admin = True + setattr(predef_role_obj, bearer, orm_role_bearers) + + db.commit() allowed_users = db.query(orm.User).filter( orm.User.name.in_(self.authenticator.allowed_users) ) @@ -2068,7 +2078,7 @@ class JupyterHub(Application): roles.grant_role(db, admin_obj, 'admin') db.commit() # make sure that on hub upgrade, all roles are reset - if not getattr(self, '_rbac_upgrade', False): + if getattr(self, '_rbac_upgrade', False): app_log.warning( "No admin role found; assuming hub upgrade. Initializing default roles for all entities" ) diff --git a/jupyterhub/roles.py b/jupyterhub/roles.py index 1f9ad65c..fad31c6f 100644 --- a/jupyterhub/roles.py +++ b/jupyterhub/roles.py @@ -239,10 +239,14 @@ def _overwrite_role(role, role_dict): for attr in role_dict.keys(): if attr == 'description' or attr == 'scopes': - if role.name == 'admin' and role_dict[attr] != getattr(role, attr): - raise ValueError( - 'admin role description or scopes cannot be overwritten' - ) + if role.name == 'admin': + admin_role_spec = [ + r for r in get_default_roles() if r['name'] == 'admin' + ][0] + if role_dict[attr] != admin_role_spec[attr]: + raise ValueError( + 'admin role description or scopes cannot be overwritten' + ) else: if role_dict[attr] != getattr(role, attr): setattr(role, attr, role_dict[attr]) diff --git a/jupyterhub/tests/test_roles.py b/jupyterhub/tests/test_roles.py index f2e370f4..0eeb6a1c 100644 --- a/jupyterhub/tests/test_roles.py +++ b/jupyterhub/tests/test_roles.py @@ -497,12 +497,12 @@ async def test_load_roles_services(tmpdir, request): hub = MockHub(**kwargs) hub.init_db() db = hub.db + await hub.init_role_creation() await hub.init_api_tokens() # make 'admin_service' admin admin_service = orm.Service.find(db, 'admin_service') admin_service.admin = True db.commit() - await hub.init_role_creation() await hub.init_role_assignment() # test if every service has a role (and no duplicates) admin_role = orm.Role.find(db, name='admin') @@ -1034,7 +1034,7 @@ async def test_duplicate_role_users(): ] hub = MockHub(load_roles=roles_to_load) hub.init_db() - with pytest.raises(AttributeError): + with pytest.raises(ValueError): await hub.init_role_creation() @@ -1116,6 +1116,14 @@ async def test_removal_config_to_db(): assert not orm.Role.find(hub.db, 'wizard') +async def test_no_admin_role_change(): + role_spec = [{'name': 'admin', 'scopes': ['shutdown']}] + hub = MockHub(load_roles=role_spec) + hub.init_db() + with pytest.raises(ValueError): + await hub.init_role_creation() + + async def test_user_config_respects_memberships(): role_spec = [ { @@ -1140,7 +1148,6 @@ async def test_admin_role_respects_config(): role_spec = [ { 'name': 'admin', - 'scopes': ['self', 'shutdown'], } ] admin_users = ['eddy', 'carol'] @@ -1154,3 +1161,8 @@ async def test_admin_role_respects_config(): for user_name in admin_users: user = orm.User.find(hub.db, user_name) assert user in admin_role.users + + +# todo: add test for empty user list and for no user list present +# Todo: Add test for rbac_upgrade behaviour flag +# Todo: test token roles are not reassigned if they are deleted diff --git a/jupyterhub/tests/test_services.py b/jupyterhub/tests/test_services.py index 266f4a07..fd922273 100644 --- a/jupyterhub/tests/test_services.py +++ b/jupyterhub/tests/test_services.py @@ -92,7 +92,7 @@ async def test_external_service(app): await maybe_future(app.init_role_creation()) await app.init_api_tokens() await app.proxy.add_all_services(app._service_map) - await app.init_role_creation() + await app.init_role_assignment() service = app._service_map[name] api_token = service.orm.api_tokens[0] From 2bf8e57e2c8617d7f434a68de96937032d40301c Mon Sep 17 00:00:00 2001 From: 0mar Date: Tue, 1 Jun 2021 13:27:49 +0200 Subject: [PATCH 26/45] Fixed whitespace bug --- jupyterhub/app.py | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index d536a85b..dd07b2c8 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -1991,10 +1991,10 @@ class JupyterHub(Application): if not orm.Role.find(self.db, name='admin'): self._rbac_upgrade = True for role in self.db.query(orm.Role).filter( - orm.Role.name.in_(config_role_names) + orm.Role.name.notin_(config_role_names) ): app_log.info(f"Deleting role {role.name}") - role.delete() + self.db.delete(role) self.db.commit() for role in init_roles: roles.create_role(self.db, role) @@ -2010,7 +2010,7 @@ class JupyterHub(Application): for role_spec in self.load_roles: if role_spec['name'] == 'admin': app_log.warning( - "Configuration specifies both admin_users and users in the admin role specification" + "Configuration specifies both admin_users and users in the admin role specification. " "If admin role is present in config, it should contain all admin users." ) app_log.info( @@ -2048,20 +2048,19 @@ class JupyterHub(Application): "Username %r is not in Authenticator.allowed_users" % bname ) - Class = orm.get_class(bearer) - orm_obj = Class.find(db, bname) - if orm_obj: - orm_role_bearers.append(orm_obj) - else: - app_log.warning( - f"User {bname} not added, only found in role definition" - ) - # Todo: Add users to allowed_users - # Ensure all with admin role have admin flag - if predef_role['name'] == 'admin': - orm_obj.admin = True - setattr(predef_role_obj, bearer, orm_role_bearers) - + Class = orm.get_class(bearer) + orm_obj = Class.find(db, bname) + if orm_obj: + orm_role_bearers.append(orm_obj) + else: + app_log.warning( + f"User {bname} not added, only found in role definition" + ) + # Todo: Add users to allowed_users + # Ensure all with admin role have admin flag + if predef_role['name'] == 'admin': + orm_obj.admin = True + setattr(predef_role_obj, bearer, orm_role_bearers) db.commit() allowed_users = db.query(orm.User).filter( orm.User.name.in_(self.authenticator.allowed_users) From 246ce6797cd9db4aee669222ea45e53601815ec0 Mon Sep 17 00:00:00 2001 From: 0mar Date: Tue, 1 Jun 2021 15:35:04 +0200 Subject: [PATCH 27/45] Fixed some bugs and implemented suggestions, save one weird test case --- jupyterhub/app.py | 10 +++-- jupyterhub/tests/test_roles.py | 77 ++++++++++++++++++++++++++++++++-- 2 files changed, 80 insertions(+), 7 deletions(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index dd07b2c8..ad591b57 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -1980,6 +1980,7 @@ class JupyterHub(Application): self.log.debug('Loading default roles to database') default_roles = roles.get_default_roles() config_role_names = [r['name'] for r in self.load_roles] + init_roles = default_roles for role_name in config_role_names: if config_role_names.count(role_name) > 1: @@ -1988,10 +1989,11 @@ class JupyterHub(Application): ) for role_spec in self.load_roles: init_roles.append(role_spec) + init_role_names = [r['name'] for r in init_roles] if not orm.Role.find(self.db, name='admin'): self._rbac_upgrade = True for role in self.db.query(orm.Role).filter( - orm.Role.name.notin_(config_role_names) + orm.Role.name.notin_(init_role_names) ): app_log.info(f"Deleting role {role.name}") self.db.delete(role) @@ -2011,7 +2013,7 @@ class JupyterHub(Application): if role_spec['name'] == 'admin': app_log.warning( "Configuration specifies both admin_users and users in the admin role specification. " - "If admin role is present in config, it should contain all admin users." + "If admin role is present in config, c.authenticator.admin_users should not be used." ) app_log.info( "Merging admin_users set with users list in admin role" @@ -2076,17 +2078,17 @@ class JupyterHub(Application): else: roles.grant_role(db, admin_obj, 'admin') db.commit() - # make sure that on hub upgrade, all roles are reset + # make sure that on hub upgrade, all users, services and tokens have at least one role (update with default) if getattr(self, '_rbac_upgrade', False): app_log.warning( "No admin role 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) # check tokens for default roles roles.check_for_default_roles(db, bearer='tokens') + self._rbac_upgrade = False async def _add_tokens(self, token_dict, kind): """Add tokens for users or services to the database""" diff --git a/jupyterhub/tests/test_roles.py b/jupyterhub/tests/test_roles.py index 07c4ef33..56b3d12d 100644 --- a/jupyterhub/tests/test_roles.py +++ b/jupyterhub/tests/test_roles.py @@ -1173,6 +1173,77 @@ async def test_admin_role_respects_config(): assert user in admin_role.users -# todo: add test for empty user list and for no user list present -# Todo: Add test for rbac_upgrade behaviour flag -# Todo: test token roles are not reassigned if they are deleted +async def test_empty_admin_spec(): + role_spec = [{'name': 'admin', 'users': []}] + hub = MockHub(load_roles=role_spec) + hub.init_db() + hub.authenticator.admin_users = [] + await hub.init_role_creation() + await hub.init_users() + await hub.init_role_assignment() + admin_role = orm.Role.find(hub.db, 'admin') + assert not admin_role.users + + +async def test_hub_upgrade_detection(): + role_spec = [{'name': 'admin', 'users': []}] + service = {'name': 'sheep_counter', 'api_token': 'some-token'} + hub = MockHub(load_roles=role_spec) + hub.init_db() + await hub.init_role_creation() + await hub.init_users() + await hub.init_api_tokens() + + assert hub._rbac_upgrade + await hub.init_role_assignment() + orm_service = orm.Service.find(hub.db, 'sheep_counter') + user_role = orm.Role.find(hub.db, 'user') + assert user_role in orm_service.roles + # Restart hub, now we are no longer in upgrade mode + hub = MockHub(load_roles=role_spec, services=[service]) + hub.test_clean_db = False + hub.init_db() + await hub.init_role_creation() + assert not getattr(hub, '_rbac_upgrade', False) + hub.db.delete(orm_service) + hub.db.commit() + + +async def test_token_keep_roles_on_restart(): + role_spec = [ + { + 'name': 'bloop', + 'scopes': ['read:users'], + } + ] + + hub = MockHub(load_roles=role_spec) + hub.init_db() + hub.authenticator.admin_users = ['ben'] + await hub.init_role_creation() + await hub.init_users() + await hub.init_role_assignment() + user = orm.User.find(hub.db, name='ben') + for _ in range(3): + user.new_api_token() + happy_token, content_token, sad_token = user.api_tokens + roles.grant_role(hub.db, happy_token, 'bloop') + roles.strip_role(hub.db, sad_token, 'token') + assert len(happy_token.roles) == 2 + assert len(content_token.roles) == 1 + assert len(sad_token.roles) == 0 + # Restart hub and see if roles are as expected + hub.load_roles = [] + await hub.init_role_creation() + await hub.init_users() + await hub.init_api_tokens() + await hub.init_role_assignment() + user = orm.User.find(hub.db, name='ben') + happy_token, content_token, sad_token = user.api_tokens + assert len(happy_token.roles) == 1 + assert len(content_token.roles) == 1 + print(sad_token.roles) + assert len(sad_token.roles) == 0 + for token in user.api_tokens: + hub.db.delete(token) + hub.db.commit() From 8f2bbd4d1184bbd95cb7792987bf984a784a6465 Mon Sep 17 00:00:00 2001 From: 0mar Date: Tue, 1 Jun 2021 23:42:50 +0200 Subject: [PATCH 28/45] Test still fails, issue with emulating hub restart --- jupyterhub/tests/test_roles.py | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/jupyterhub/tests/test_roles.py b/jupyterhub/tests/test_roles.py index 56b3d12d..5b9d166f 100644 --- a/jupyterhub/tests/test_roles.py +++ b/jupyterhub/tests/test_roles.py @@ -1188,23 +1188,20 @@ async def test_empty_admin_spec(): async def test_hub_upgrade_detection(): role_spec = [{'name': 'admin', 'users': []}] service = {'name': 'sheep_counter', 'api_token': 'some-token'} - hub = MockHub(load_roles=role_spec) - hub.init_db() - await hub.init_role_creation() - await hub.init_users() - await hub.init_api_tokens() - - assert hub._rbac_upgrade - await hub.init_role_assignment() + hub = MockHub(load_roles=role_spec, services=[service]) + await hub.initialize() orm_service = orm.Service.find(hub.db, 'sheep_counter') user_role = orm.Role.find(hub.db, 'user') assert user_role in orm_service.roles + orm_service.roles = [] + hub.db.commit() # Restart hub, now we are no longer in upgrade mode - hub = MockHub(load_roles=role_spec, services=[service]) - hub.test_clean_db = False - hub.init_db() - await hub.init_role_creation() + hub = MockHub(load_roles=role_spec, services=[service], db=hub.db) + await hub.initialize() + # Fixme: How to respect db state? assert not getattr(hub, '_rbac_upgrade', False) + orm_service = orm.Service.find(hub.db, 'sheep_counter') + assert not orm_service.roles hub.db.delete(orm_service) hub.db.commit() From d6bb1e631879e980454dbfc9e5b8d3a28f88a98b Mon Sep 17 00:00:00 2001 From: 0mar Date: Thu, 3 Jun 2021 13:26:06 +0200 Subject: [PATCH 29/45] Fixed upgrade test --- jupyterhub/app.py | 3 +- jupyterhub/tests/test_roles.py | 60 ++++++++++++++++++++++++++-------- 2 files changed, 48 insertions(+), 15 deletions(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index ad591b57..d10d0065 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -1992,6 +1992,8 @@ class JupyterHub(Application): init_role_names = [r['name'] for r in init_roles] if not orm.Role.find(self.db, name='admin'): self._rbac_upgrade = True + else: + self._rbac_upgrade = False for role in self.db.query(orm.Role).filter( orm.Role.name.notin_(init_role_names) ): @@ -2088,7 +2090,6 @@ class JupyterHub(Application): # check tokens for default roles roles.check_for_default_roles(db, bearer='tokens') - self._rbac_upgrade = False async def _add_tokens(self, token_dict, kind): """Add tokens for users or services to the database""" diff --git a/jupyterhub/tests/test_roles.py b/jupyterhub/tests/test_roles.py index 5b9d166f..07efdc4b 100644 --- a/jupyterhub/tests/test_roles.py +++ b/jupyterhub/tests/test_roles.py @@ -2,11 +2,13 @@ # Copyright (c) Jupyter Development Team. # Distributed under the terms of the Modified BSD License. import json +import os from itertools import chain import pytest from pytest import mark from tornado.log import app_log +from traitlets.config import Config from .. import orm from .. import roles @@ -1185,26 +1187,56 @@ async def test_empty_admin_spec(): assert not admin_role.users -async def test_hub_upgrade_detection(): - role_spec = [{'name': 'admin', 'users': []}] - service = {'name': 'sheep_counter', 'api_token': 'some-token'} - hub = MockHub(load_roles=role_spec, services=[service]) +# Todo: Test that services don't get default roles on any startup + + +async def test_hub_upgrade_detection(tmpdir): + db_url = f"sqlite:///{tmpdir.join('jupyterhub.sqlite')}" + os.environ['JUPYTERHUB_TEST_DB_URL'] = db_url + # Create hub with users and tokens + hub = MockHub(db_url=db_url) await hub.initialize() - orm_service = orm.Service.find(hub.db, 'sheep_counter') + user_names = ['patricia', 'quentin'] user_role = orm.Role.find(hub.db, 'user') - assert user_role in orm_service.roles - orm_service.roles = [] + for name in user_names: + user = add_user(hub.db, name=name) + user.new_api_token() + assert user_role in user.roles + for role in hub.db.query(orm.Role): + hub.db.delete(role) hub.db.commit() - # Restart hub, now we are no longer in upgrade mode - hub = MockHub(load_roles=role_spec, services=[service], db=hub.db) + # Restart hub in emulated upgrade mode: default roles for all entities + hub.test_clean_db = False await hub.initialize() - # Fixme: How to respect db state? - assert not getattr(hub, '_rbac_upgrade', False) - orm_service = orm.Service.find(hub.db, 'sheep_counter') - assert not orm_service.roles - hub.db.delete(orm_service) + assert getattr(hub, '_rbac_upgrade', False) + user_role = orm.Role.find(hub.db, 'user') + token_role = orm.Role.find(hub.db, 'token') + for name in user_names: + user = orm.User.find(hub.db, name) + assert user_role in user.roles + assert token_role in user.api_tokens[0].roles + # Strip all roles and see if it sticks + user_role.users = [] + token_role.tokens = [] hub.db.commit() + hub.init_db() + hub.init_hub() + await hub.init_role_creation() + await hub.init_users() + hub.authenticator.allowed_users = ['patricia'] + await hub.init_api_tokens() + await hub.init_role_assignment() + assert not getattr(hub, '_rbac_upgrade', False) + user_role = orm.Role.find(hub.db, 'user') + token_role = orm.Role.find(hub.db, 'token') + allowed_user = orm.User.find(hub.db, 'patricia') + rem_user = orm.User.find(hub.db, 'quentin') + assert user_role in allowed_user.roles + assert token_role not in allowed_user.api_tokens[0].roles + assert user_role not in rem_user.roles + assert token_role not in rem_user.roles + async def test_token_keep_roles_on_restart(): role_spec = [ From 2ab6c61e9aebecbcdb9f62114657e38fbde66ca8 Mon Sep 17 00:00:00 2001 From: IvanaH8 Date: Tue, 25 May 2021 09:50:01 +0200 Subject: [PATCH 30/45] Synchronize scope variable nomenclature and docstrings across rbac utils --- jupyterhub/roles.py | 106 +++++++++++++++++++++++++++++-------------- jupyterhub/scopes.py | 58 +++++++++++++++-------- 2 files changed, 113 insertions(+), 51 deletions(-) diff --git a/jupyterhub/roles.py b/jupyterhub/roles.py index 316448e9..781fd65c 100644 --- a/jupyterhub/roles.py +++ b/jupyterhub/roles.py @@ -12,8 +12,14 @@ from . import scopes def get_default_roles(): - """Returns a list of default role dictionaries""" - + """Returns: + default roles (list): default role definitions as dictionaries: + { + 'name': role name, + 'description': role description, + 'scopes': list of scopes, + } + """ default_roles = [ { 'name': 'user', @@ -37,9 +43,7 @@ def get_default_roles(): { 'name': 'server', 'description': 'Post activity only', - 'scopes': [ - 'users:activity!user' - ], # TO DO - fix scope to refer to only self once implemented + 'scopes': ['users:activity!user'], }, { 'name': 'token', @@ -60,6 +64,12 @@ def expand_self_scope(name): users:activity users:servers users:tokens + + Arguments: + name (str): user name + + Returns: + expanded scopes (set): set of expanded scopes covering standard user privileges """ scope_list = [ 'users', @@ -99,8 +109,13 @@ def horizontal_filter(func): @horizontal_filter def _expand_scope(scopename): - """Returns a set of all subscopes""" + """Returns a set of all subscopes + Arguments: + scopename (str): name of the scope to expand + Returns: + expanded scope (set): set of all scope's subscopes including the scope itself + """ expanded_scope = [] def _add_subscopes(scopename): @@ -116,7 +131,14 @@ def _expand_scope(scopename): def expand_roles_to_scopes(orm_object): """Get the scopes listed in the roles of the User/Service/Group/Token - If User, take into account the user's groups roles as well""" + If User, take into account the user's groups roles as well + + Arguments: + orm_object: orm.User, orm.Service, orm.Group or orm.APIToken + + Returns: + expanded scopes (set): set of all expanded scopes for the orm object + """ if not isinstance(orm_object, orm.Base): raise TypeError(f"Only orm objects allowed, got {orm_object}") @@ -127,26 +149,35 @@ def expand_roles_to_scopes(orm_object): for group in orm_object.groups: pass_roles.extend(group.roles) - scopes = _get_subscopes(*pass_roles, owner=orm_object) + expanded_scopes = _get_subscopes(*pass_roles, owner=orm_object) - return scopes + return expanded_scopes def _get_subscopes(*args, owner=None): - """Returns a set of all available subscopes for a specified role or list of roles""" + """Returns a set of all available subscopes for a specified role or list of roles + Arguments: + role (obj): orm.Role + or + roles (list): list of orm.Roles + owner (obj, optional): orm.User or orm.Service as owner of orm.APIToken + + Returns: + expanded scopes (set): set of all expanded scopes for the role(s) + """ scope_list = [] for role in args: scope_list.extend(role.scopes) - scopes = set(chain.from_iterable(list(map(_expand_scope, scope_list)))) + expanded_scopes = set(chain.from_iterable(list(map(_expand_scope, scope_list)))) # transform !user filter to !user=ownername - for scope in scopes: + for scope in expanded_scopes: base_scope, _, filter = scope.partition('!') if filter == 'user': - scopes.remove(scope) + expanded_scopes.remove(scope) if isinstance(owner, orm.APIToken): token_owner = owner.user if token_owner is None: @@ -155,18 +186,26 @@ def _get_subscopes(*args, owner=None): else: name = owner.name trans_scope = f'{base_scope}!user={name}' - scopes.add(trans_scope) + expanded_scopes.add(trans_scope) - if 'self' in scopes: - scopes.remove('self') + if 'self' in expanded_scopes: + expanded_scopes.remove('self') if owner and isinstance(owner, orm.User): - scopes |= expand_self_scope(owner.name) + expanded_scopes |= expand_self_scope(owner.name) - return scopes + return expanded_scopes def _check_scopes(*args, rolename=None): - """Check if provided scopes exist""" + """Check if provided scopes exist + + Arguments: + scope (str): name of the scope to check + or + scopes (list): list of scopes to check + + Raises NameError if scope does not exist + """ allowed_scopes = set(scopes.scope_definitions.keys()) allowed_filters = ['!user=', '!service=', '!group=', '!server=', '!user'] @@ -190,7 +229,6 @@ def _check_scopes(*args, rolename=None): def _overwrite_role(role, role_dict): """Overwrites role's description and/or scopes with role_dict if role not 'admin'""" - for attr in role_dict.keys(): if attr == 'description' or attr == 'scopes': if role.name == 'admin': @@ -231,7 +269,6 @@ def _validate_role_name(name): def create_role(db, role_dict): """Adds a new role to database or modifies an existing one""" - default_roles = get_default_roles() if 'name' not in role_dict.keys(): @@ -264,7 +301,6 @@ def create_role(db, role_dict): def delete_role(db, rolename): """Removes a role from database""" - # default roles are not removable default_roles = get_default_roles() if any(role['name'] == rolename for role in default_roles): @@ -298,7 +334,7 @@ def existing_only(func): @existing_only def grant_role(db, entity, rolename): - """Adds a role for users, services or tokens""" + """Adds a role for users, services, groups or tokens""" if isinstance(entity, orm.APIToken): entity_repr = entity else: @@ -317,7 +353,7 @@ def grant_role(db, entity, rolename): @existing_only def strip_role(db, entity, rolename): - """Removes a role for users, services or tokens""" + """Removes a role for users, services, groups or tokens""" if isinstance(entity, orm.APIToken): entity_repr = entity else: @@ -352,10 +388,12 @@ def _switch_default_role(db, obj, admin): def _token_allowed_role(db, token, role): + """Checks if requested role for token does not grant the token + higher permissions than the token's owner has - """Returns True if token allowed to have requested role through - comparing the requested scopes with the set of token's owner scopes""" - + Returns: + True if requested permissions are within the owner's permissions, False otherwise + """ owner = token.user if owner is None: owner = token.service @@ -389,9 +427,10 @@ def _token_allowed_role(db, token, role): def assign_default_roles(db, entity): - """Assigns the default roles to an entity: + """Assigns default role to an entity: users and services get 'user' role, or admin role if they have admin flag - Tokens get 'token' role""" + tokens get 'token' role + """ if isinstance(entity, orm.Group): pass elif isinstance(entity, orm.APIToken): @@ -408,7 +447,9 @@ def assign_default_roles(db, entity): def update_roles(db, entity, roles): - """Updates object's roles""" + """Updates object's roles checking for requested permissions + if object is orm.APIToken + """ standard_permissions = {'all', 'read:all'} for rolename in roles: if isinstance(entity, orm.APIToken): @@ -432,10 +473,9 @@ def update_roles(db, entity, roles): def check_for_default_roles(db, bearer): - """Checks that role bearers have at least one role (default if none). - Groups can be without a role""" - + Groups can be without a role + """ Class = orm.get_class(bearer) if Class == orm.Group: pass diff --git a/jupyterhub/scopes.py b/jupyterhub/scopes.py index 80679107..47b6ff29 100644 --- a/jupyterhub/scopes.py +++ b/jupyterhub/scopes.py @@ -1,4 +1,14 @@ -"""General scope definitions and utilities""" +""" +General scope definitions and utilities + +Scope variable nomenclature +--------------------------- +scopes: list of scopes with abbreviations (e.g., in role definition) +expanded scopes: set of expanded scopes without abbreviations (i.e., resolved metascopes, filters and subscopes) +parsed scopes: dictionary JSON like format of expanded scopes +intersection : set of expanded scopes as intersection of 2 expanded scope sets +identify scopes: set of expanded scopes needed for identify (whoami) endpoints +""" import functools import inspect import warnings @@ -99,11 +109,13 @@ class Scope(Enum): def _intersect_scopes(scopes_a, scopes_b): - """Intersect two sets of scopes + """Intersect two sets of expanded scopes by comparing their permissions - Compares the permissions of two sets of scopes, - including horizontal filters, - and returns the intersected scopes. + Arguments: + scopes_a, scopes_b: sets of expanded scopes + + Returns: + intersection: set of expanded scopes as intersection of the arguments Note: Intersects correctly with ALL and exact filter matches (i.e. users!user=x & read:users:name -> read:users:name!user=x) @@ -180,10 +192,19 @@ def _intersect_scopes(scopes_a, scopes_b): def get_scopes_for(orm_object): - """Find scopes for a given user or token and resolve permissions""" - scopes = set() + """Find scopes for a given user or token and resolve permissions + + Arguments: + orm_object: orm object or User wrapper + + Returns: + expanded scopes (set) for the orm object + or + intersection (set) if orm_object == orm.APIToken + """ + expanded_scopes = set() if orm_object is None: - return scopes + return expanded_scopes if not isinstance(orm_object, orm.Base): from .user import User @@ -204,8 +225,8 @@ def get_scopes_for(orm_object): token_scopes.remove('all') token_scopes |= owner_scopes - scopes = _intersect_scopes(token_scopes, owner_scopes) - discarded_token_scopes = token_scopes - scopes + intersection = _intersect_scopes(token_scopes, owner_scopes) + discarded_token_scopes = token_scopes - intersection # Not taking symmetric difference here because token owner can naturally have more scopes than token if discarded_token_scopes: @@ -213,9 +234,10 @@ def get_scopes_for(orm_object): "discarding scopes [%s], not present in owner roles" % ", ".join(discarded_token_scopes) ) + expanded_scopes = intersection else: - scopes = roles.expand_roles_to_scopes(orm_object) - return scopes + expanded_scopes = roles.expand_roles_to_scopes(orm_object) + return expanded_scopes def _needs_scope_expansion(filter_, filter_value, sub_scope): @@ -319,16 +341,16 @@ def parse_scopes(scope_list): def unparse_scopes(parsed_scopes): - """Turn a parsed_scopes dictionary back into a scopes set""" - scopes = set() + """Turn a parsed_scopes dictionary back into a expanded scopes set""" + expanded_scopes = set() for base, filters in parsed_scopes.items(): if filters == Scope.ALL: - scopes.add(base) + expanded_scopes.add(base) else: for entity, names_list in filters.items(): for name in names_list: - scopes.add(f'{base}!{entity}={name}') - return scopes + expanded_scopes.add(f'{base}!{entity}={name}') + return expanded_scopes def needs_scope(*scopes): @@ -384,7 +406,7 @@ def identify_scopes(obj): obj: orm.User or orm.Service Returns: - scopes (set): set of scopes needed for 'identify' endpoints + identify scopes (set): set of scopes needed for 'identify' endpoints """ if isinstance(obj, orm.User): return { From e6845a68f5bbd9978146afe3a2840f9e63426513 Mon Sep 17 00:00:00 2001 From: IvanaH8 Date: Wed, 26 May 2021 12:04:21 +0200 Subject: [PATCH 31/45] Clarify some function names in rbac utils --- jupyterhub/apihandlers/users.py | 2 +- jupyterhub/handlers/base.py | 4 ++-- jupyterhub/scopes.py | 10 +++++----- jupyterhub/tests/test_scopes.py | 28 +++++++++++++++------------- 4 files changed, 23 insertions(+), 21 deletions(-) diff --git a/jupyterhub/apihandlers/users.py b/jupyterhub/apihandlers/users.py index a0dfbe2f..210a1f70 100644 --- a/jupyterhub/apihandlers/users.py +++ b/jupyterhub/apihandlers/users.py @@ -336,7 +336,7 @@ class UserTokenListAPIHandler(APIHandler): # couldn't identify requester raise web.HTTPError(403) self._jupyterhub_user = requester - self._resolve_scopes() + self._resolve_roles_and_scopes() user = self.find_user(user_name) kind = 'user' if isinstance(requester, User) else 'service' scope_filter = self.get_scope_filter('users:tokens') diff --git a/jupyterhub/handlers/base.py b/jupyterhub/handlers/base.py index 3c49577c..59c2ae57 100644 --- a/jupyterhub/handlers/base.py +++ b/jupyterhub/handlers/base.py @@ -87,7 +87,7 @@ class BaseHandler(RequestHandler): except Exception: self.log.exception("Failed to get current user") self._jupyterhub_user = None - self._resolve_scopes() + self._resolve_roles_and_scopes() return await maybe_future(super().prepare()) @property @@ -416,7 +416,7 @@ class BaseHandler(RequestHandler): self.log.exception("Error getting current user") return self._jupyterhub_user - def _resolve_scopes(self): + def _resolve_roles_and_scopes(self): self.raw_scopes = set() app_log.debug("Loading and parsing scopes") if self.current_user: diff --git a/jupyterhub/scopes.py b/jupyterhub/scopes.py index 47b6ff29..370b601f 100644 --- a/jupyterhub/scopes.py +++ b/jupyterhub/scopes.py @@ -108,7 +108,7 @@ class Scope(Enum): ALL = True -def _intersect_scopes(scopes_a, scopes_b): +def _intersect_expanded_scopes(scopes_a, scopes_b): """Intersect two sets of expanded scopes by comparing their permissions Arguments: @@ -192,7 +192,7 @@ def _intersect_scopes(scopes_a, scopes_b): def get_scopes_for(orm_object): - """Find scopes for a given user or token and resolve permissions + """Find scopes for a given user or token from their roles and resolve permissions Arguments: orm_object: orm object or User wrapper @@ -225,7 +225,7 @@ def get_scopes_for(orm_object): token_scopes.remove('all') token_scopes |= owner_scopes - intersection = _intersect_scopes(token_scopes, owner_scopes) + intersection = _intersect_expanded_scopes(token_scopes, owner_scopes) discarded_token_scopes = token_scopes - intersection # Not taking symmetric difference here because token owner can naturally have more scopes than token @@ -263,7 +263,7 @@ def _check_user_in_expanded_scope(handler, user_name, scope_group_names): return bool(set(scope_group_names) & group_names) -def _check_scope(api_handler, req_scope, **kwargs): +def _check_scope_access(api_handler, req_scope, **kwargs): """Check if scopes satisfy requirements Returns True for (potentially restricted) access, False for refused access """ @@ -375,7 +375,7 @@ def needs_scope(*scopes): s_kwargs[resource] = resource_value for scope in scopes: app_log.debug("Checking access via scope %s", scope) - has_access = _check_scope(self, scope, **s_kwargs) + has_access = _check_scope_access(self, scope, **s_kwargs) if has_access: return func(self, *args, **kwargs) try: diff --git a/jupyterhub/tests/test_scopes.py b/jupyterhub/tests/test_scopes.py index 1d501a17..69f5e38c 100644 --- a/jupyterhub/tests/test_scopes.py +++ b/jupyterhub/tests/test_scopes.py @@ -9,8 +9,8 @@ from tornado.httputil import HTTPServerRequest from .. import orm from .. import roles from ..handlers import BaseHandler -from ..scopes import _check_scope -from ..scopes import _intersect_scopes +from ..scopes import _check_scope_access +from ..scopes import _intersect_expanded_scopes from ..scopes import get_scopes_for from ..scopes import needs_scope from ..scopes import parse_scopes @@ -49,37 +49,39 @@ def test_scope_precendence(): def test_scope_check_present(): handler = get_handler_with_scopes(['read:users']) - assert _check_scope(handler, 'read:users') - assert _check_scope(handler, 'read:users', user='maeby') + assert _check_scope_access(handler, 'read:users') + assert _check_scope_access(handler, 'read:users', user='maeby') def test_scope_check_not_present(): handler = get_handler_with_scopes(['read:users!user=maeby']) - assert _check_scope(handler, 'read:users') + assert _check_scope_access(handler, 'read:users') with pytest.raises(web.HTTPError): - _check_scope(handler, 'read:users', user='gob') + _check_scope_access(handler, 'read:users', user='gob') with pytest.raises(web.HTTPError): - _check_scope(handler, 'read:users', user='gob', server='server') + _check_scope_access(handler, 'read:users', user='gob', server='server') def test_scope_filters(): handler = get_handler_with_scopes( ['read:users', 'read:users!group=bluths', 'read:users!user=maeby'] ) - assert _check_scope(handler, 'read:users', group='bluth') - assert _check_scope(handler, 'read:users', user='maeby') + assert _check_scope_access(handler, 'read:users', group='bluth') + assert _check_scope_access(handler, 'read:users', user='maeby') def test_scope_multiple_filters(): handler = get_handler_with_scopes(['read:users!user=george_michael']) - assert _check_scope(handler, 'read:users', user='george_michael', group='bluths') + assert _check_scope_access( + handler, 'read:users', user='george_michael', group='bluths' + ) def test_scope_parse_server_name(): handler = get_handler_with_scopes( ['users:servers!server=maeby/server1', 'read:users!user=maeby'] ) - assert _check_scope(handler, 'users:servers', user='maeby', server='server1') + assert _check_scope_access(handler, 'users:servers', user='maeby', server='server1') class MockAPIHandler: @@ -828,10 +830,10 @@ async def test_resolve_token_permissions( ), ], ) -def test_intersect_scopes(left, right, expected, should_warn, recwarn): +def test_intersect_expanded_scopes(left, right, expected, should_warn, recwarn): # run every test in both directions, to ensure symmetry of the inputs for a, b in [(left, right), (right, left)]: - intersection = _intersect_scopes(set(left), set(right)) + intersection = _intersect_expanded_scopes(set(left), set(right)) assert intersection == set(expected) if should_warn: From 335320fd14e71f5c60040c3a0c8ee6a5905f103a Mon Sep 17 00:00:00 2001 From: IvanaH8 Date: Fri, 28 May 2021 11:08:34 +0200 Subject: [PATCH 32/45] Rename raw_scopes attr for base handler to expanded_scopes --- jupyterhub/apihandlers/auth.py | 4 ++-- jupyterhub/apihandlers/base.py | 2 +- jupyterhub/apihandlers/users.py | 9 +++++---- jupyterhub/handlers/base.py | 12 ++++++------ jupyterhub/roles.py | 15 ++++++++------- jupyterhub/scopes.py | 6 +++--- jupyterhub/tests/test_scopes.py | 8 ++++---- 7 files changed, 29 insertions(+), 27 deletions(-) diff --git a/jupyterhub/apihandlers/auth.py b/jupyterhub/apihandlers/auth.py index ed548add..4e7673f2 100644 --- a/jupyterhub/apihandlers/auth.py +++ b/jupyterhub/apihandlers/auth.py @@ -35,8 +35,8 @@ class TokenAPIHandler(APIHandler): if owner: # having a token means we should be able to read the owner's model # (this is the only thing this handler is for) - self.raw_scopes.update(scopes.identify_scopes(owner)) - self.parsed_scopes = scopes.parse_scopes(self.raw_scopes) + self.expanded_scopes.update(scopes.identify_scopes(owner)) + self.parsed_scopes = scopes.parse_scopes(self.expanded_scopes) # record activity whenever we see a token now = orm_token.last_activity = datetime.utcnow() diff --git a/jupyterhub/apihandlers/base.py b/jupyterhub/apihandlers/base.py index 3ead420f..3dc8073b 100644 --- a/jupyterhub/apihandlers/base.py +++ b/jupyterhub/apihandlers/base.py @@ -254,7 +254,7 @@ class APIHandler(BaseHandler): self.log.debug( "Asking for user model of %s with scopes [%s]", user.name, - ", ".join(self.raw_scopes), + ", ".join(self.expanded_scopes), ) allowed_keys = set() for scope in access_map: diff --git a/jupyterhub/apihandlers/users.py b/jupyterhub/apihandlers/users.py index 210a1f70..a33bef20 100644 --- a/jupyterhub/apihandlers/users.py +++ b/jupyterhub/apihandlers/users.py @@ -37,12 +37,13 @@ class SelfAPIHandler(APIHandler): 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) + self.expanded_scopes.update(scopes.identify_scopes(user)) + self.parsed_scopes = scopes.parse_scopes(self.expanded_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) + self.expanded_scopes.update(scopes.identify_scopes(user.orm_user)) + print('Expanded scopes in selfapihandler') + self.parsed_scopes = scopes.parse_scopes(self.expanded_scopes) model = self.user_model(user) # validate return, should have at least kind and name, # otherwise our filters did something wrong diff --git a/jupyterhub/handlers/base.py b/jupyterhub/handlers/base.py index 59c2ae57..3b8ea3f2 100644 --- a/jupyterhub/handlers/base.py +++ b/jupyterhub/handlers/base.py @@ -81,7 +81,7 @@ class BaseHandler(RequestHandler): The current user (None if not logged in) may be accessed via the `self.current_user` property during the handling of any request. """ - self.raw_scopes = set() + self.expanded_scopes = set() try: await self.get_current_user() except Exception: @@ -417,16 +417,16 @@ class BaseHandler(RequestHandler): return self._jupyterhub_user def _resolve_roles_and_scopes(self): - self.raw_scopes = set() + self.expanded_scopes = set() app_log.debug("Loading and parsing scopes") if self.current_user: orm_token = self.get_token() if orm_token: - self.raw_scopes = scopes.get_scopes_for(orm_token) + self.expanded_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) - app_log.debug("Found scopes [%s]", ",".join(self.raw_scopes)) + self.expanded_scopes = scopes.get_scopes_for(self.current_user) + self.parsed_scopes = scopes.parse_scopes(self.expanded_scopes) + app_log.debug("Found scopes [%s]", ",".join(self.expanded_scopes)) @property def current_user(self): diff --git a/jupyterhub/roles.py b/jupyterhub/roles.py index 781fd65c..73c171dd 100644 --- a/jupyterhub/roles.py +++ b/jupyterhub/roles.py @@ -401,21 +401,22 @@ def _token_allowed_role(db, token, role): if owner is None: raise ValueError(f"Owner not found for {token}") - token_scopes = _get_subscopes(role, owner=owner) + expanded_scopes = _get_subscopes(role, owner=owner) implicit_permissions = {'all', 'read:all'} - explicit_scopes = token_scopes - implicit_permissions + explicit_scopes = expanded_scopes - implicit_permissions # ignore horizontal filters - raw_scopes = { + no_filter_scopes = { scope.split('!', 1)[0] if '!' in scope else scope for scope in explicit_scopes } # find the owner's scopes - owner_scopes = expand_roles_to_scopes(owner) + expanded_owner_scopes = expand_roles_to_scopes(owner) # ignore horizontal filters - raw_owner_scopes = { - scope.split('!', 1)[0] if '!' in scope else scope for scope in owner_scopes + no_filter_owner_scopes = { + scope.split('!', 1)[0] if '!' in scope else scope + for scope in expanded_owner_scopes } - disallowed_scopes = raw_scopes.difference(raw_owner_scopes) + disallowed_scopes = no_filter_scopes.difference(no_filter_owner_scopes) if not disallowed_scopes: # no scopes requested outside owner's own scopes return True diff --git a/jupyterhub/scopes.py b/jupyterhub/scopes.py index 370b601f..a0c94d8a 100644 --- a/jupyterhub/scopes.py +++ b/jupyterhub/scopes.py @@ -363,8 +363,8 @@ def needs_scope(*scopes): bound_sig = sig.bind(self, *args, **kwargs) bound_sig.apply_defaults() # Load scopes in case they haven't been loaded yet - if not hasattr(self, 'raw_scopes'): - self.raw_scopes = {} + if not hasattr(self, 'expanded_scopes'): + self.expanded_scopes = {} self.parsed_scopes = {} s_kwargs = {} @@ -384,7 +384,7 @@ def needs_scope(*scopes): 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) + end_point, ", ".join(scopes), ", ".join(self.expanded_scopes) ) ) raise web.HTTPError( diff --git a/jupyterhub/tests/test_scopes.py b/jupyterhub/tests/test_scopes.py index 69f5e38c..6652e0be 100644 --- a/jupyterhub/tests/test_scopes.py +++ b/jupyterhub/tests/test_scopes.py @@ -86,14 +86,14 @@ def test_scope_parse_server_name(): class MockAPIHandler: def __init__(self): - self.raw_scopes = {'users'} + self.expanded_scopes = {'users'} self.parsed_scopes = {} 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) + self.expanded_scopes = set(scopes) + self.parsed_scopes = parse_scopes(self.expanded_scopes) @needs_scope('users') def user_thing(self, user_name): @@ -195,7 +195,7 @@ def test_scope_method_access(mock_handler, scopes, method, arguments, is_allowed 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) + mock_handler.parsed_scopes = parse_scopes(mock_handler.expanded_scopes) assert mock_handler.secret_thing() From 563146445f8ecb4689e5e588ca71a4843af35bd4 Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 12 May 2021 14:46:15 +0200 Subject: [PATCH 33/45] add scopes.check_scope_filter Extracted from APIHandler.get_scope_filter for easier re-use and mve get_scope_filter to BaseHandler from APIHandler since it will be needed on oauth --- jupyterhub/apihandlers/base.py | 48 ++-------------------------------- jupyterhub/handlers/base.py | 19 ++++++++++++++ jupyterhub/scopes.py | 40 +++++++++++++++++++++++++--- 3 files changed, 58 insertions(+), 49 deletions(-) diff --git a/jupyterhub/apihandlers/base.py b/jupyterhub/apihandlers/base.py index 3dc8073b..8e55fb4b 100644 --- a/jupyterhub/apihandlers/base.py +++ b/jupyterhub/apihandlers/base.py @@ -1,19 +1,14 @@ """Base API handlers""" # Copyright (c) Jupyter Development Team. # Distributed under the terms of the Modified BSD License. -import functools import json -import re -from datetime import datetime from http.client import responses from sqlalchemy.exc import SQLAlchemyError from tornado import web from .. import orm -from .. import scopes from ..handlers import BaseHandler -from ..user import User from ..utils import isoformat from ..utils import url_path_join @@ -66,46 +61,6 @@ class APIHandler(BaseHandler): return False return True - @functools.lru_cache() - def get_scope_filter(self, req_scope): - """Produce a filter for `*ListAPIHandlers* so that GET method knows which models to return. - Filter is a callable that takes a resource name and outputs true or false""" - - def no_access(orm_resource, kind): - return False - - if req_scope not in self.parsed_scopes: - return no_access - sub_scope = self.parsed_scopes[req_scope] - - def has_access_to(orm_resource, kind): - """ - param orm_resource: User or Service or Group or spawner - param kind: 'user' or 'service' or 'group' or 'server'. - """ - if sub_scope == scopes.Scope.ALL: - return True - elif orm_resource.name in sub_scope.get(kind, []): - return True - if kind == 'server': - server_format = f"{orm_resource.user.name}/{orm_resource.name}" - if server_format in sub_scope.get(kind, []): - return True - # Fall back on checking if we have user access - if orm_resource.user.name in sub_scope.get('user', []): - return True - # Fall back on checking if we have group access for this user - orm_resource = orm_resource.user - kind = 'user' - if kind == 'user' and 'group' in sub_scope: - group_names = {group.name for group in orm_resource.groups} - user_in_group = bool(group_names & set(sub_scope['group'])) - if user_in_group: - return True - return False - - return has_access_to - def get_current_user_cookie(self): """Override get_user_cookie to check Referer header""" cookie_user = super().get_current_user_cookie() @@ -213,7 +168,8 @@ class APIHandler(BaseHandler): 'last_activity': isoformat(token.last_activity), 'expires_at': isoformat(token.expires_at), 'note': token.note, - 'oauth_client': token.client.description or token.client.client_id, + 'oauth_client': token.oauth_client.description + or token.oauth_client.identifier, } return model diff --git a/jupyterhub/handlers/base.py b/jupyterhub/handlers/base.py index 3b8ea3f2..30bd81ac 100644 --- a/jupyterhub/handlers/base.py +++ b/jupyterhub/handlers/base.py @@ -2,6 +2,7 @@ # Copyright (c) Jupyter Development Team. # Distributed under the terms of the Modified BSD License. import asyncio +import functools import json import math import random @@ -428,6 +429,24 @@ class BaseHandler(RequestHandler): self.parsed_scopes = scopes.parse_scopes(self.expanded_scopes) app_log.debug("Found scopes [%s]", ",".join(self.expanded_scopes)) + @functools.lru_cache() + def get_scope_filter(self, req_scope): + """Produce a filter function for req_scope on resources + + Returns `has_access_to(orm_resource, kind)` which returns True or False + for whether the current request has access to req_scope on the given resource. + """ + + def no_access(orm_resource, kind): + return False + + if req_scope not in self.parsed_scopes: + return no_access + + sub_scope = self.parsed_scopes[req_scope] + + return functools.partial(scopes.check_scope_filter, sub_scope) + @property def current_user(self): """Override .current_user accessor from tornado diff --git a/jupyterhub/scopes.py b/jupyterhub/scopes.py index a0c94d8a..1c73108e 100644 --- a/jupyterhub/scopes.py +++ b/jupyterhub/scopes.py @@ -332,11 +332,13 @@ def parse_scopes(scope_list): parsed_scopes[base_scope] = Scope.ALL elif base_scope not in parsed_scopes: parsed_scopes[base_scope] = {} + if parsed_scopes[base_scope] != Scope.ALL: - key, _, val = filter_.partition('=') + key, _, value = filter_.partition('=') if key not in parsed_scopes[base_scope]: - parsed_scopes[base_scope][key] = [] - parsed_scopes[base_scope][key].append(val) + parsed_scopes[base_scope][key] = set([value]) + else: + parsed_scopes[base_scope][key].add(value) return parsed_scopes @@ -422,3 +424,35 @@ def identify_scopes(obj): } else: raise TypeError(f"Expected orm.User or orm.Service, got {obj!r}") + + +def check_scope_filter(sub_scope, orm_resource, kind): + """Return whether a sub_scope filter applies to a given resource. + + param sub_scope: parsed_scopes filter (i.e. dict or Scope.ALL) + param orm_resource: User or Service or Group or Spawner + param kind: 'user' or 'service' or 'group' or 'server'. + + Returns True or False + """ + if sub_scope is Scope.ALL: + return True + elif kind in sub_scope and orm_resource.name in sub_scope[kind]: + return True + + if kind == 'server': + server_format = f"{orm_resource.user.name}/{orm_resource.name}" + if server_format in sub_scope.get(kind, []): + return True + # Fall back on checking if we have user access + if 'user' in sub_scope and orm_resource.user.name in sub_scope['user']: + return True + # Fall back on checking if we have group access for this user + orm_resource = orm_resource.user + kind = 'user' + elif kind == 'user' and 'group' in sub_scope: + group_names = {group.name for group in orm_resource.groups} + user_in_group = bool(group_names & set(sub_scope['group'])) + if user_in_group: + return True + return False From 7e46d5d0fc64ea11707d327b95fad7e715ef1085 Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 12 May 2021 14:48:16 +0200 Subject: [PATCH 34/45] store relationship between oauth client and service/spawner so that we can look up the spawner/service from the oauth client and vice versa --- .../alembic/versions/833da8570507_rbac.py | 32 +++++++++++++++++- jupyterhub/app.py | 15 ++++++--- jupyterhub/handlers/pages.py | 6 ++-- jupyterhub/oauth/provider.py | 2 -- jupyterhub/orm.py | 33 ++++++++++++++++++- jupyterhub/tests/test_api.py | 2 +- jupyterhub/tests/test_orm.py | 6 ++-- jupyterhub/tests/test_pages.py | 2 +- jupyterhub/user.py | 9 ++--- 9 files changed, 85 insertions(+), 22 deletions(-) diff --git a/jupyterhub/alembic/versions/833da8570507_rbac.py b/jupyterhub/alembic/versions/833da8570507_rbac.py index b76fc707..dbf6fc0b 100644 --- a/jupyterhub/alembic/versions/833da8570507_rbac.py +++ b/jupyterhub/alembic/versions/833da8570507_rbac.py @@ -1,4 +1,4 @@ -"""rbac +"""RBAC Revision ID: 833da8570507 Revises: 4dc2d5a8c53c @@ -16,6 +16,30 @@ import sqlalchemy as sa def upgrade(): + # associate spawners and services with their oauth clients + op.add_column( + 'services', sa.Column('oauth_client_id', sa.Unicode(length=255), nullable=True) + ) + op.create_foreign_key( + None, + 'services', + 'oauth_clients', + ['oauth_client_id'], + ['identifier'], + ondelete='SET NULL', + ) + op.add_column( + 'spawners', sa.Column('oauth_client_id', sa.Unicode(length=255), nullable=True) + ) + op.create_foreign_key( + None, + 'spawners', + 'oauth_clients', + ['oauth_client_id'], + ['identifier'], + ondelete='SET NULL', + ) + # 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) @@ -33,6 +57,12 @@ def upgrade(): def downgrade(): + + op.drop_constraint(None, 'spawners', type_='foreignkey') + op.drop_column('spawners', 'oauth_client_id') + op.drop_constraint(None, 'services', type_='foreignkey') + op.drop_column('services', 'oauth_client_id') + # delete OAuth tokens for non-jupyterhub clients # drop new columns from api tokens op.drop_constraint(None, 'api_tokens', type_='foreignkey') diff --git a/jupyterhub/app.py b/jupyterhub/app.py index d10d0065..7fb3a775 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -394,7 +394,7 @@ class JupyterHub(Application): 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 @@ -887,7 +887,7 @@ class JupyterHub(Application): "/", help=""" The routing prefix for the Hub itself. - + Override to send only a subset of traffic to the Hub. Default is to use the Hub as the default route for all requests. @@ -899,7 +899,7 @@ class JupyterHub(Application): may want to handle these events themselves, in which case they can register their own default target with the proxy and set e.g. `hub_routespec = /hub/` to serve only the hub's own pages, or even `/hub/api/` for api-only operation. - + Note: hub_routespec must include the base_url, if any. .. versionadded:: 1.4 @@ -1484,7 +1484,7 @@ class JupyterHub(Application): Can be a Unicode string (e.g. '/hub/home') or a callable based on the handler object: :: - + def default_url_fn(handler): user = handler.current_user if user and user.admin: @@ -1956,6 +1956,7 @@ class JupyterHub(Application): for name, usernames in self.load_groups.items(): group = orm.Group.find(db, name) if group is None: + self.log.info(f"Creating group {name}") group = orm.Group(name=name) db.add(group) for username in usernames: @@ -1970,8 +1971,10 @@ class JupyterHub(Application): if user is None: if not self.authenticator.validate_username(username): raise ValueError("Group username %r is not valid" % username) + self.log.info(f"Creating user {username} for group {name}") user = orm.User(name=username) db.add(user) + self.log.debug(f"Adding user {username} to group {name}") group.users.append(user) db.commit() @@ -2264,6 +2267,10 @@ class JupyterHub(Application): allowed_roles=service.oauth_roles, description="JupyterHub service %s" % service.name, ) + service.orm.oauth_client_id = service.oauth_client_id + else: + if service.oauth_client: + self.db.delete(service.oauth_client) self._service_map[name] = service diff --git a/jupyterhub/handlers/pages.py b/jupyterhub/handlers/pages.py index 3d2b03f6..78d847cf 100644 --- a/jupyterhub/handlers/pages.py +++ b/jupyterhub/handlers/pages.py @@ -10,7 +10,6 @@ from http.client import responses from jinja2 import TemplateNotFound from tornado import web from tornado.httputil import url_concat -from tornado.httputil import urlparse from .. import __version__ from .. import orm @@ -590,8 +589,9 @@ class TokenPageHandler(BaseHandler): token = tokens[0] oauth_clients.append( { - 'client': token.client, - 'description': token.client.description or token.client.identifier, + 'client': token.oauth_client, + 'description': token.oauth_client.description + or token.oauth_client.identifier, 'created': created, 'last_activity': last_activity, 'tokens': tokens, diff --git a/jupyterhub/oauth/provider.py b/jupyterhub/oauth/provider.py index 80e8140e..8f21b3bb 100644 --- a/jupyterhub/oauth/provider.py +++ b/jupyterhub/oauth/provider.py @@ -2,8 +2,6 @@ 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 diff --git a/jupyterhub/orm.py b/jupyterhub/orm.py index b16926ac..342abfae 100644 --- a/jupyterhub/orm.py +++ b/jupyterhub/orm.py @@ -326,6 +326,21 @@ class Spawner(Base): last_activity = Column(DateTime, nullable=True) user_options = Column(JSONDict) + # added in 2.0 + oauth_client_id = Column( + Unicode(255), + ForeignKey( + 'oauth_clients.identifier', + ondelete='SET NULL', + ), + ) + oauth_client = relationship( + 'OAuthClient', + backref=backref("spawner", uselist=False), + cascade="all, delete-orphan", + single_parent=True, + ) + # properties on the spawner wrapper # some APIs get these low-level objects # when the spawner isn't running, @@ -377,6 +392,21 @@ class Service(Base): ) pid = Column(Integer) + # added in 2.0 + oauth_client_id = Column( + Unicode(255), + ForeignKey( + 'oauth_clients.identifier', + ondelete='SET NULL', + ), + ) + oauth_client = relationship( + 'OAuthClient', + backref=backref("service", uselist=False), + cascade="all, delete-orphan", + single_parent=True, + ) + def new_api_token(self, token=None, **kwargs): """Create a new API token If `token` is given, load that token. @@ -567,6 +597,7 @@ class APIToken(Hashed, Base): ondelete='CASCADE', ), ) + # FIXME: refresh_tokens not implemented # should be a relation to another token table # refresh_token = Column( @@ -746,7 +777,7 @@ class OAuthClient(Base): return self.identifier access_tokens = relationship( - APIToken, backref='client', cascade='all, delete-orphan' + APIToken, backref='oauth_client', cascade='all, delete-orphan' ) codes = relationship(OAuthCode, backref='client', cascade='all, delete-orphan') diff --git a/jupyterhub/tests/test_api.py b/jupyterhub/tests/test_api.py index 03f45bc6..0b6b8014 100644 --- a/jupyterhub/tests/test_api.py +++ b/jupyterhub/tests/test_api.py @@ -307,7 +307,7 @@ async def test_get_self(app): db.commit() oauth_token = orm.APIToken( user=u.orm_user, - client=oauth_client, + oauth_client=oauth_client, token=token, ) db.add(oauth_token) diff --git a/jupyterhub/tests/test_orm.py b/jupyterhub/tests/test_orm.py index 389137b5..6c1be117 100644 --- a/jupyterhub/tests/test_orm.py +++ b/jupyterhub/tests/test_orm.py @@ -364,7 +364,7 @@ def test_user_delete_cascade(db): oauth_code = orm.OAuthCode(client=oauth_client, user=user) db.add(oauth_code) oauth_token = orm.APIToken( - client=oauth_client, + oauth_client=oauth_client, user=user, ) db.add(oauth_token) @@ -401,7 +401,7 @@ def test_oauth_client_delete_cascade(db): oauth_code = orm.OAuthCode(client=oauth_client, user=user) db.add(oauth_code) oauth_token = orm.APIToken( - client=oauth_client, + oauth_client=oauth_client, user=user, ) db.add(oauth_token) @@ -525,7 +525,7 @@ def test_expiring_oauth_token(app, user): db.add(client) orm_token = orm.APIToken( token=token, - client=client, + oauth_client=client, user=user, expires_at=now() + timedelta(seconds=30), ) diff --git a/jupyterhub/tests/test_pages.py b/jupyterhub/tests/test_pages.py index 1bba9926..4fe1dfb9 100644 --- a/jupyterhub/tests/test_pages.py +++ b/jupyterhub/tests/test_pages.py @@ -870,7 +870,7 @@ async def test_oauth_token_page(app): client = orm.OAuthClient(identifier='token') app.db.add(client) oauth_token = orm.APIToken( - client=client, + oauth_client=client, user=user, ) app.db.add(oauth_token) diff --git a/jupyterhub/user.py b/jupyterhub/user.py index 85e1b88b..5f10f54e 100644 --- a/jupyterhub/user.py +++ b/jupyterhub/user.py @@ -590,15 +590,11 @@ class User: client_id = spawner.oauth_client_id oauth_provider = self.settings.get('oauth_provider') if oauth_provider: - oauth_client = oauth_provider.fetch_by_client_id(client_id) - # create a new OAuth client + secret on every launch - # containers that resume will be updated below - allowed_roles = spawner.oauth_roles if callable(allowed_roles): allowed_roles = allowed_roles(spawner) - oauth_provider.add_client( + oauth_client = oauth_provider.add_client( client_id, api_token, url_path_join(self.url, server_name, 'oauth_callback'), @@ -606,6 +602,7 @@ class User: description="Server at %s" % (url_path_join(self.base_url, server_name) + '/'), ) + spawner.orm_spawner.oauth_client = oauth_client db.commit() # trigger pre-spawn hook on authenticator @@ -614,7 +611,7 @@ class User: spawner._start_pending = True if authenticator: - # pre_spawn_start can thow errors that can lead to a redirect loop + # pre_spawn_start can throw errors that can lead to a redirect loop # if left uncaught (see https://github.com/jupyterhub/jupyterhub/issues/2683) await maybe_future(authenticator.pre_spawn_start(self, spawner)) From 57f4c084929f69b76c4e5d9a05ae3971c0a8699d Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 12 May 2021 16:41:56 +0200 Subject: [PATCH 35/45] get upgrade working on sqlite with foreign key naming convention --- .../alembic/versions/833da8570507_rbac.py | 85 ++++++++++++------- jupyterhub/orm.py | 14 ++- 2 files changed, 67 insertions(+), 32 deletions(-) diff --git a/jupyterhub/alembic/versions/833da8570507_rbac.py b/jupyterhub/alembic/versions/833da8570507_rbac.py index dbf6fc0b..2abecea9 100644 --- a/jupyterhub/alembic/versions/833da8570507_rbac.py +++ b/jupyterhub/alembic/versions/833da8570507_rbac.py @@ -1,4 +1,5 @@ -"""RBAC +""" +rbac changes for jupyterhub 2.0 Revision ID: 833da8570507 Revises: 4dc2d5a8c53c @@ -14,31 +15,40 @@ depends_on = None from alembic import op import sqlalchemy as sa +from jupyterhub import orm + + +naming_convention = orm.meta.naming_convention + def upgrade(): # associate spawners and services with their oauth clients - op.add_column( - 'services', sa.Column('oauth_client_id', sa.Unicode(length=255), nullable=True) - ) - op.create_foreign_key( - None, - 'services', - 'oauth_clients', - ['oauth_client_id'], - ['identifier'], - ondelete='SET NULL', - ) - op.add_column( - 'spawners', sa.Column('oauth_client_id', sa.Unicode(length=255), nullable=True) - ) - op.create_foreign_key( - None, - 'spawners', - 'oauth_clients', - ['oauth_client_id'], - ['identifier'], - ondelete='SET NULL', - ) + # op.add_column( + # 'services', sa.Column('oauth_client_id', sa.Unicode(length=255), nullable=True) + # ) + for table_name in ('services', 'spawners'): + column_name = "oauth_client_id" + target_table = "oauth_clients" + target_column = "identifier" + with op.batch_alter_table( + table_name, + schema=None, + ) as batch_op: + batch_op.add_column( + sa.Column('oauth_client_id', sa.Unicode(length=255), nullable=True), + ) + batch_op.create_foreign_key( + naming_convention["fk"] + % dict( + table_name=table_name, + column_0_name=column_name, + referred_table_name=target_table, + ), + target_table, + [column_name], + [target_column], + ondelete='SET NULL', + ) # FIXME, maybe: currently drops all api tokens and forces recreation! # this ensures a consistent database, but requires: @@ -57,17 +67,32 @@ def upgrade(): def downgrade(): + for table_name in ('services', 'spawners'): + column_name = "oauth_client_id" + target_table = "oauth_clients" + target_column = "identifier" - op.drop_constraint(None, 'spawners', type_='foreignkey') - op.drop_column('spawners', 'oauth_client_id') - op.drop_constraint(None, 'services', type_='foreignkey') - op.drop_column('services', 'oauth_client_id') + with op.batch_alter_table( + table_name, + schema=None, + naming_convention=orm.meta.naming_convention, + ) as batch_op: + batch_op.drop_constraint( + naming_convention["fk"] + % dict( + table_name=table_name, + column_0_name=column_name, + referred_table_name=target_table, + ), + type_='foreignkey', + ) + batch_op.drop_column(column_name) # 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') + # 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 diff --git a/jupyterhub/orm.py b/jupyterhub/orm.py index 342abfae..8f6e2833 100644 --- a/jupyterhub/orm.py +++ b/jupyterhub/orm.py @@ -16,12 +16,12 @@ from sqlalchemy import Boolean from sqlalchemy import Column from sqlalchemy import create_engine from sqlalchemy import DateTime -from sqlalchemy import Enum from sqlalchemy import event from sqlalchemy import exc from sqlalchemy import ForeignKey from sqlalchemy import inspect from sqlalchemy import Integer +from sqlalchemy import MetaData from sqlalchemy import or_ from sqlalchemy import select from sqlalchemy import Table @@ -115,7 +115,17 @@ class JSONList(JSONDict): return value -Base = declarative_base() +meta = MetaData( + naming_convention={ + "ix": "ix_%(column_0_label)s", + "uq": "uq_%(table_name)s_%(column_0_name)s", + "ck": "ck_%(table_name)s_%(constraint_name)s", + "fk": "fk_%(table_name)s_%(column_0_name)s_%(referred_table_name)s", + "pk": "pk_%(table_name)s", + } +) + +Base = declarative_base(metadata=meta) Base.log = app_log From e5198b403976f2737f70b410b7ff35520402a347 Mon Sep 17 00:00:00 2001 From: Min RK Date: Fri, 21 May 2021 12:51:03 +0200 Subject: [PATCH 36/45] create boolean columns with create_constraint=False matches new default behavior in sqlalchemy 1.4 --- jupyterhub/orm.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jupyterhub/orm.py b/jupyterhub/orm.py index 8f6e2833..26219c72 100644 --- a/jupyterhub/orm.py +++ b/jupyterhub/orm.py @@ -274,7 +274,7 @@ class User(Base): def orm_spawners(self): return {s.name: s for s in self._orm_spawners} - admin = Column(Boolean, default=False) + admin = Column(Boolean(create_constraint=False), default=False) created = Column(DateTime, default=datetime.utcnow) last_activity = Column(DateTime, nullable=True) @@ -386,7 +386,7 @@ class Service(Base): # common user interface: name = Column(Unicode(255), unique=True) - admin = Column(Boolean, default=False) + admin = Column(Boolean(create_constraint=False), default=False) api_tokens = relationship( "APIToken", backref="service", cascade="all, delete-orphan" From e2076e6c91d6bc8dd939b86dbf60ab44e8ee1276 Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 12 May 2021 14:49:06 +0200 Subject: [PATCH 37/45] implement access scopes - access:services for services - access:users:servers for servers - tokens automatically have access to their issuing client (if their owner does, too) - Check access scope in HubAuth integration --- .../external/jupyterhub_config.py | 24 ++++- .../external/shared-notebook-service | 8 +- jupyterhub/apihandlers/auth.py | 25 +++++ jupyterhub/apihandlers/users.py | 32 ++++--- jupyterhub/roles.py | 16 +++- jupyterhub/scopes.py | 23 +++++ jupyterhub/services/auth.py | 91 ++++++++++++++++++- jupyterhub/services/service.py | 13 +++ jupyterhub/spawner.py | 12 +++ jupyterhub/tests/mockservice.py | 5 +- jupyterhub/tests/test_services_auth.py | 91 ++++++++++++++++--- 11 files changed, 304 insertions(+), 36 deletions(-) diff --git a/examples/service-notebook/external/jupyterhub_config.py b/examples/service-notebook/external/jupyterhub_config.py index 3b2ef52e..c6157253 100644 --- a/examples/service-notebook/external/jupyterhub_config.py +++ b/examples/service-notebook/external/jupyterhub_config.py @@ -1,15 +1,33 @@ # our user list -c.Authenticator.whitelist = ['minrk', 'ellisonbg', 'willingc'] +c.Authenticator.allowed_users = ['minrk', 'ellisonbg', 'willingc'] # ellisonbg and willingc have access to a shared server: -c.JupyterHub.load_groups = {'shared': ['ellisonbg', 'willingc']} +c.JupyterHub.load_groups = {'shared-notebook-grp': ['ellisonbg', 'willingc']} + +c.JupyterHub.load_roles = [ + { + "name": "shared-notebook", + "groups": ["shared-notebook-grp"], + "scopes": ["access:services!service=shared-notebook"], + }, + # by default, the user role has access to all services + # we want to limit that, so give users only access to 'self' + { + "name": "user", + "scopes": ["self"], + }, +] # start the notebook server as a service c.JupyterHub.services = [ { 'name': 'shared-notebook', 'url': 'http://127.0.0.1:9999', - 'api_token': 'super-secret', + 'api_token': 'c3a29e5d386fd7c9aa1e8fe9d41c282ec8b', } ] + +# dummy spawner and authenticator for testing, don't actually use these! +c.JupyterHub.authenticator_class = 'dummy' +c.JupyterHub.spawner_class = 'simple' diff --git a/examples/service-notebook/external/shared-notebook-service b/examples/service-notebook/external/shared-notebook-service index 20206e92..3510c0a6 100755 --- a/examples/service-notebook/external/shared-notebook-service +++ b/examples/service-notebook/external/shared-notebook-service @@ -1,9 +1,11 @@ #!/bin/bash -l set -e -export JUPYTERHUB_API_TOKEN=super-secret +# these must match the values in jupyterhub_config.py +export JUPYTERHUB_API_TOKEN=c3a29e5d386fd7c9aa1e8fe9d41c282ec8b export JUPYTERHUB_SERVICE_URL=http://127.0.0.1:9999 export JUPYTERHUB_SERVICE_NAME=shared-notebook +export JUPYTERHUB_SERVICE_PREFIX="/services/${JUPYTERHUB_SERVICE_NAME}/" +export JUPYTERHUB_CLIENT_ID="service-${JUPYTERHUB_SERVICE_NAME}" -jupyterhub-singleuser \ - --group='shared' +jupyterhub-singleuser diff --git a/jupyterhub/apihandlers/auth.py b/jupyterhub/apihandlers/auth.py index 4e7673f2..7506dd40 100644 --- a/jupyterhub/apihandlers/auth.py +++ b/jupyterhub/apihandlers/auth.py @@ -216,6 +216,31 @@ class OAuthAuthorizeHandler(OAuthHandler, BaseHandler): ) credentials = self.add_credentials(credentials) client = self.oauth_provider.fetch_by_client_id(credentials['client_id']) + allowed = False + + # check for access to target resource + if client.spawner: + scope_filter = self.get_scope_filter("access:users:servers") + allowed = scope_filter(client.spawner, kind='server') + elif client.service: + scope_filter = self.get_scope_filter("access:services") + allowed = scope_filter(client.service, kind='service') + else: + # client is not associated with a service or spawner. + # This shouldn't happen, but it might if this is a stale or forged request + # from a service or spawner that's since been deleted + self.log.error( + f"OAuth client {client} has no service or spawner, cannot resolve scopes." + ) + raise web.HTTPError(500, "OAuth configuration error") + + if not allowed: + self.log.error( + f"User {self.current_user} not allowed to access {client.description}" + ) + raise web.HTTPError( + 403, f"You do not have permission to access {client.description}" + ) if not self.needs_oauth_confirm(self.current_user, client): self.log.debug( "Skipping oauth confirmation for %s accessing %s", diff --git a/jupyterhub/apihandlers/users.py b/jupyterhub/apihandlers/users.py index a33bef20..9bb379ed 100644 --- a/jupyterhub/apihandlers/users.py +++ b/jupyterhub/apihandlers/users.py @@ -35,21 +35,31 @@ class SelfAPIHandler(APIHandler): user = self.current_user if user is None: raise web.HTTPError(403) + + _added_scopes = set() if isinstance(user, orm.Service): # ensure we have the minimal 'identify' scopes for the token owner - self.expanded_scopes.update(scopes.identify_scopes(user)) - self.parsed_scopes = scopes.parse_scopes(self.expanded_scopes) - model = self.service_model(user) + identify_scopes = scopes.identify_scopes(user) + get_model = self.service_model else: - self.expanded_scopes.update(scopes.identify_scopes(user.orm_user)) - print('Expanded scopes in selfapihandler') + identify_scopes = scopes.identify_scopes(user.orm_user) + get_model = self.user_model + + # ensure we have permission to identify ourselves + # all tokens can do this on this endpoint + for scope in identify_scopes: + if scope not in self.expanded_scopes: + _added_scopes.add(scope) + self.expanded_scopes.add(scope) + if _added_scopes: + # re-parse with new scopes self.parsed_scopes = scopes.parse_scopes(self.expanded_scopes) - model = self.user_model(user) - # validate return, should have at least kind and name, - # otherwise our filters did something wrong - for key in ("kind", "name"): - if key not in model: - raise ValueError(f"Missing identify model for {user}: {model}") + + model = get_model(user) + + # add scopes to identify model, + # but not the scopes we added to ensure we could read our own model + model["scopes"] = sorted(self.expanded_scopes.difference(_added_scopes)) self.write(json.dumps(model)) diff --git a/jupyterhub/roles.py b/jupyterhub/roles.py index 73c171dd..6417a3e6 100644 --- a/jupyterhub/roles.py +++ b/jupyterhub/roles.py @@ -24,11 +24,13 @@ def get_default_roles(): { 'name': 'user', 'description': 'Standard user privileges', - 'scopes': ['self'], + 'scopes': [ + 'self', + ], }, { 'name': 'admin', - 'description': 'Admin privileges (currently can do everything)', + 'description': 'Admin privileges (can do everything)', 'scopes': [ 'admin:users', 'admin:users:servers', @@ -38,12 +40,17 @@ def get_default_roles(): 'read:hub', 'proxy', 'shutdown', + 'access:services', + 'access:users:servers', ], }, { 'name': 'server', 'description': 'Post activity only', - 'scopes': ['users:activity!user'], + 'scopes': [ + 'users:activity!user', + 'access:users:servers!user', + ], }, { 'name': 'token', @@ -64,6 +71,7 @@ def expand_self_scope(name): users:activity users:servers users:tokens + access:users:servers Arguments: name (str): user name @@ -80,6 +88,8 @@ def expand_self_scope(name): 'users:tokens', ] read_scope_list = ['read:' + scope for scope in scope_list] + # access doesn't want the 'read:' prefix + scope_list.append('access:users:servers') scope_list.extend(read_scope_list) return {"{}!user={}".format(scope, name) for scope in scope_list} diff --git a/jupyterhub/scopes.py b/jupyterhub/scopes.py index 1c73108e..afce5974 100644 --- a/jupyterhub/scopes.py +++ b/jupyterhub/scopes.py @@ -97,6 +97,12 @@ scope_definitions = { 'read:hub': { 'description': 'Read-only access to detailed information about the Hub.' }, + 'access:users:servers': { + 'description': 'Access user servers via API or browser.', + }, + 'access:services': { + 'description': 'Access services via API or browser.', + }, 'proxy': { 'description': 'Allows for obtaining information about the proxy’s routing table, for syncing the Hub with proxy and notifying the Hub about a new proxy.' }, @@ -220,6 +226,23 @@ def get_scopes_for(orm_object): app_log.warning(f"Authenticated with token {orm_object}") owner = orm_object.user or orm_object.service token_scopes = roles.expand_roles_to_scopes(orm_object) + if orm_object.client_id != "jupyterhub": + # oauth tokens can be used to access the service issuing the token, + # assuming the owner itself still has permission to do so + spawner = orm_object.oauth_client.spawner + if spawner: + token_scopes.add( + f"access:users:servers!server={spawner.user.name}/{spawner.name}" + ) + else: + service = orm_object.oauth_client.service + if service: + token_scopes.add(f"access:services!service={service.name}") + else: + app_log.warning( + f"Token {orm_object} has no associated service or spawner!" + ) + owner_scopes = roles.expand_roles_to_scopes(owner) if 'all' in token_scopes: token_scopes.remove('all') diff --git a/jupyterhub/services/auth.py b/jupyterhub/services/auth.py index 4f8e0524..1066a9d7 100644 --- a/jupyterhub/services/auth.py +++ b/jupyterhub/services/auth.py @@ -33,13 +33,50 @@ from traitlets import Dict from traitlets import Instance from traitlets import Integer from traitlets import observe +from traitlets import Set from traitlets import Unicode from traitlets import validate from traitlets.config import SingletonConfigurable +from ..scopes import _intersect_scopes from ..utils import url_path_join +def check_scopes(required_scopes, scopes): + """Check that required_scope(s) are in scopes + + Returns the subset of scopes matching required_scopes, + which is truthy if any scopes match any required scopes. + + Correctly resolves scope filters *except* for groups -> user, + e.g. require: access:server!user=x, have: access:server!group=y + will not grant access to user x even if user x is in group y. + + Parameters + ---------- + + required_scopes: set + The set of scopes required. + scopes: set + The set (or list) of scopes to check against required_scopes + + Returns + ------- + relevant_scopes: set + The set of scopes in required_scopes that are present in scopes, + which is truthy if any required scopes are present, + and falsy otherwise. + """ + if isinstance(required_scopes, str): + required_scopes = {required_scopes} + + intersection = _intersect_scopes(required_scopes, scopes) + # re-intersect with required_scopes in case the intersection + # applies stricter filters than required_scopes declares + # e.g. required_scopes = {'read:users'} and intersection has only {'read:users!user=x'} + return set(required_scopes) & intersection + + class _ExpiringDict(dict): """Dict-like cache for Hub API requests @@ -285,6 +322,24 @@ class HubAuth(SingletonConfigurable): def _default_cache(self): return _ExpiringDict(self.cache_max_age) + oauth_scopes = Set( + Unicode(), + help="""OAuth scopes to use for allowing access. + + Get from $JUPYTERHUB_OAUTH_SCOPES by default. + """, + ).tag(config=True) + + @default('oauth_scopes') + def _default_scopes(self): + env_scopes = os.getenv('JUPYTERHUB_OAUTH_SCOPES') + if env_scopes: + return set(json.loads(env_scopes)) + service_name = os.getenv("JUPYTERHUB_SERVICE_NAME") + if service_name: + return {f'access:services!service={service_name}'} + return set() + def _check_hub_authorization(self, url, api_token, cache_key=None, use_cache=True): """Identify a user with the Hub @@ -495,6 +550,10 @@ class HubAuth(SingletonConfigurable): app_log.debug("No user identified") return user_model + def check_scopes(self, required_scopes, user): + """Check whether the user has required scope(s)""" + return check_scopes(required_scopes, set(user["scopes"])) + class HubOAuth(HubAuth): """HubAuth using OAuth for login instead of cookies set by the Hub. @@ -771,6 +830,7 @@ class HubAuthenticated(object): A handler that mixes this in must have the following attributes/properties: - .hub_auth: A HubAuth instance + - .hub_scopes: A set of JupyterHub 2.0 OAuth scopes to allow. - .hub_users: A set of usernames to allow. If left unspecified or None, username will not be checked. - .hub_groups: A set of group names to allow. @@ -795,13 +855,19 @@ class HubAuthenticated(object): hub_groups = None # set of allowed groups allow_admin = False # allow any admin user access + @property + def hub_scopes(self): + """Set of allowed scopes (use hub_auth.oauth_scopes by default)""" + return self.hub_auth.oauth_scopes or None + @property def allow_all(self): """Property indicating that all successfully identified user or service should be allowed. """ return ( - self.hub_services is None + self.hub_scopes is None + and self.hub_services is None and self.hub_users is None and self.hub_groups is None ) @@ -842,22 +908,41 @@ class HubAuthenticated(object): Returns the input if the user should be allowed, None otherwise. - Override if you want to check anything other than the username's presence in hub_users list. + Override for custom logic in authenticating users. Args: - model (dict): the user or service model returned from :class:`HubAuth` + user_model (dict): the user or service model returned from :class:`HubAuth` Returns: user_model (dict): The user model if the user should be allowed, None otherwise. """ name = model['name'] kind = model.setdefault('kind', 'user') + if self.allow_all: app_log.debug( "Allowing Hub %s %s (all Hub users and services allowed)", kind, name ) return model + if self.hub_scopes: + scopes = self.hub_auth.check_scopes(self.hub_scopes, model) + if scopes: + app_log.debug( + f"Allowing Hub {kind} {name} based on oauth scopes {scopes}" + ) + return model + else: + app_log.warning( + f"Not allowing Hub {kind} {name}: missing required scopes" + ) + app_log.debug( + f"Hub {kind} {name} needs scope(s) {self.hub_scopes}, has scope(s) {model['scopes']}" + ) + # if hub_scopes are used, *only* hub_scopes are used + # note: this means successful authentication, but insufficient permission + raise UserNotAllowed(model) + if self.allow_admin and model.get('admin', False): app_log.debug("Allowing Hub admin %s", name) return model diff --git a/jupyterhub/services/service.py b/jupyterhub/services/service.py index f556b6da..24d19592 100644 --- a/jupyterhub/services/service.py +++ b/jupyterhub/services/service.py @@ -98,6 +98,14 @@ class _ServiceSpawner(LocalProcessSpawner): cwd = Unicode() cmd = Command(minlen=0) + _service_name = Unicode() + + @default("oauth_scopes") + def _default_oauth_scopes(self): + return [ + "access:services", + f"access:services!service={self._service_name}", + ] def make_preexec_fn(self, name): if not name: @@ -330,6 +338,10 @@ class Service(LoggingConfigurable): """ return bool(self.server is not None or self.oauth_redirect_uri) + @property + def oauth_client(self): + return self.orm.oauth_client + @property def server(self): if self.orm.server: @@ -384,6 +396,7 @@ class Service(LoggingConfigurable): environment=env, api_token=self.api_token, oauth_client_id=self.oauth_client_id, + _service_name=self.name, cookie_options=self.cookie_options, cwd=self.cwd, hub=self.hub, diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index fd18386c..39b50133 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -216,6 +216,16 @@ class Spawner(LoggingConfigurable): admin_access = Bool(False) api_token = Unicode() oauth_client_id = Unicode() + + oauth_scopes = List(Unicode()) + + @default("oauth_scopes") + def _default_oauth_scopes(self): + return [ + f"access:users:server!server={self.user.name}/{self.name}", + f"access:users:server!user={self.user.name}", + ] + handler = Any() oauth_roles = Union( @@ -803,6 +813,8 @@ class Spawner(LoggingConfigurable): self.user.url, self.name, 'oauth_callback' ) + env['JUPYTERHUB_OAUTH_SCOPES'] = json.dumps(self.oauth_scopes) + # Info previously passed on args env['JUPYTERHUB_USER'] = self.user.name env['JUPYTERHUB_SERVER_NAME'] = self.name diff --git a/jupyterhub/tests/mockservice.py b/jupyterhub/tests/mockservice.py index 415f512f..082aa65d 100644 --- a/jupyterhub/tests/mockservice.py +++ b/jupyterhub/tests/mockservice.py @@ -21,6 +21,7 @@ from urllib.parse import urlparse import requests from tornado import httpserver from tornado import ioloop +from tornado import log from tornado import web from jupyterhub.services.auth import HubAuthenticated @@ -114,7 +115,9 @@ def main(): if __name__ == '__main__': - from tornado.options import parse_command_line + from tornado.options import parse_command_line, options parse_command_line() + options.logging = 'debug' + log.enable_pretty_logging() main() diff --git a/jupyterhub/tests/test_services_auth.py b/jupyterhub/tests/test_services_auth.py index 3ed5d628..158bdbf9 100644 --- a/jupyterhub/tests/test_services_auth.py +++ b/jupyterhub/tests/test_services_auth.py @@ -25,6 +25,7 @@ from tornado.web import HTTPError from tornado.web import RequestHandler from .. import orm +from .. import roles from ..services.auth import _ExpiringDict from ..services.auth import HubOAuth from ..services.auth import HubOAuthenticated @@ -87,6 +88,7 @@ async def test_hubauth_token(app, mockservice_url): public_url(app, mockservice_url) + '/whoami/', headers={'Authorization': 'token %s' % token}, ) + r.raise_for_status() reply = r.json() sub_reply = {key: reply.get(key, 'missing') for key in ['name', 'admin']} assert sub_reply == {'name': 'river', 'admin': False} @@ -111,36 +113,101 @@ async def test_hubauth_token(app, mockservice_url): assert path.endswith('/hub/login') -async def test_hubauth_service_token(app, mockservice_url): +@pytest.mark.parametrize( + "scopes, allowed", + [ + ( + [ + "access:services", + ], + True, + ), + ( + [ + "access:services!service=$service", + ], + True, + ), + ( + [ + "access:services!service=other-service", + ], + False, + ), + ( + [ + "access:users:servers!user=$service", + ], + False, + ), + ], +) +async def test_hubauth_service_token(request, app, mockservice_url, scopes, allowed): """Test HubAuthenticated service with service API tokens""" + scopes = [scope.replace('$service', mockservice_url.name) for scope in scopes] + token = hexlify(os.urandom(5)).decode('utf8') name = 'test-api-service' app.service_tokens[token] = name await app.init_api_tokens() + orm_service = app.db.query(orm.Service).filter_by(name=name).one() + role_name = "test-hubauth-service-token" + + roles.create_role( + app.db, + { + "name": role_name, + "description": "role for test", + "scopes": scopes, + }, + ) + request.addfinalizer(lambda: roles.delete_role(app.db, role_name)) + roles.grant_role(app.db, orm_service, role_name) + # token in Authorization header r = await async_requests.get( - public_url(app, mockservice_url) + '/whoami/', + public_url(app, mockservice_url) + 'whoami/', headers={'Authorization': 'token %s' % token}, allow_redirects=False, ) - r.raise_for_status() - assert r.status_code == 200 - reply = r.json() - assert reply == {'kind': 'service', 'name': name, 'admin': False, 'roles': []} - assert not r.cookies + if allowed: + r.raise_for_status() + assert r.status_code == 200 + reply = r.json() + assert reply == { + 'kind': 'service', + 'name': name, + 'admin': False, + 'roles': [role_name], + 'scopes': scopes, + } + assert not r.cookies + else: + assert r.status_code == 403 # token in ?token parameter r = await async_requests.get( - public_url(app, mockservice_url) + '/whoami/?token=%s' % token + public_url(app, mockservice_url) + 'whoami/?token=%s' % token ) - r.raise_for_status() - reply = r.json() - assert reply == {'kind': 'service', 'name': name, 'admin': False, 'roles': []} + if allowed: + r.raise_for_status() + assert r.status_code == 200 + reply = r.json() + assert reply == { + 'kind': 'service', + 'name': name, + 'admin': False, + 'roles': [role_name], + 'scopes': scopes, + } + assert not r.cookies + else: + assert r.status_code == 403 r = await async_requests.get( - public_url(app, mockservice_url) + '/whoami/?token=no-such-token', + public_url(app, mockservice_url) + 'whoami/?token=no-such-token', allow_redirects=False, ) assert r.status_code == 302 From 72b1dd22046b799b79a6ea1727b85115b4f8c51f Mon Sep 17 00:00:00 2001 From: Min RK Date: Thu, 20 May 2021 09:45:22 +0200 Subject: [PATCH 38/45] oauth: use client_id for description if empty that way description can never be empty on retrieval --- jupyterhub/oauth/provider.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jupyterhub/oauth/provider.py b/jupyterhub/oauth/provider.py index 8f21b3bb..8bcfe0cb 100644 --- a/jupyterhub/oauth/provider.py +++ b/jupyterhub/oauth/provider.py @@ -611,7 +611,7 @@ class JupyterHubOAuthServer(WebApplicationServer): allowed_roles = [] orm_client.secret = hash_token(client_secret) if client_secret else "" orm_client.redirect_uri = redirect_uri - orm_client.description = description + orm_client.description = description or client_id orm_client.allowed_roles = allowed_roles self.db.commit() From 0ba222b28835ad53a3ea8565abd35bda306a153b Mon Sep 17 00:00:00 2001 From: Min RK Date: Thu, 20 May 2021 10:21:33 +0200 Subject: [PATCH 39/45] move role/scope fixtures to conftest so they can be more easily reused --- jupyterhub/tests/conftest.py | 78 ++++++++++++++++++++++++++++++++- jupyterhub/tests/test_roles.py | 2 - jupyterhub/tests/test_scopes.py | 77 -------------------------------- 3 files changed, 76 insertions(+), 81 deletions(-) diff --git a/jupyterhub/tests/conftest.py b/jupyterhub/tests/conftest.py index f439bfb0..a51bf858 100644 --- a/jupyterhub/tests/conftest.py +++ b/jupyterhub/tests/conftest.py @@ -27,7 +27,6 @@ Fixtures to add functionality or spawning behavior # Distributed under the terms of the Modified BSD License. import asyncio import inspect -import logging import os import sys from getpass import getuser @@ -50,7 +49,6 @@ from ..utils import random_port from .mocking import MockHub from .test_services import mockservice_cmd from .utils import add_user -from .utils import ssl_setup # global db session object _db = None @@ -339,3 +337,79 @@ def slow_bad_spawn(app): app.tornado_settings, {'spawner_class': mocking.SlowBadSpawner} ): yield + + +@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() + + +@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, name=None): + nonlocal counter + if name is None: + 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) + 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) + + +@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, name=None): + nonlocal counter + if name is None: + 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() + 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() diff --git a/jupyterhub/tests/test_roles.py b/jupyterhub/tests/test_roles.py index 07efdc4b..72a68476 100644 --- a/jupyterhub/tests/test_roles.py +++ b/jupyterhub/tests/test_roles.py @@ -13,10 +13,8 @@ from traitlets.config import Config from .. import orm from .. import roles from ..scopes import get_scopes_for -from ..utils import maybe_future from ..utils import utcnow from .mocking import MockHub -from .test_scopes import create_temp_role from .utils import add_user from .utils import api_request diff --git a/jupyterhub/tests/test_scopes.py b/jupyterhub/tests/test_scopes.py index 6652e0be..81642cba 100644 --- a/jupyterhub/tests/test_scopes.py +++ b/jupyterhub/tests/test_scopes.py @@ -260,83 +260,6 @@ async def test_by_fake_user(app): err_message = "No access to resources or resources not found" -@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, name=None): - nonlocal counter - if name is None: - 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_services = [] - counter = 0 - role_function = create_temp_role - - def temp_service_creator(*scopes, name=None): - nonlocal counter - if name is None: - 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() - temp_services.append(orm_service) - roles.update_roles(app.db, orm_service, roles=[role.name]) - return orm_service - - yield temp_service_creator - for service in temp_services: - 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') From 69d2839ba300f2bfa960af7755a004e5efcac4e2 Mon Sep 17 00:00:00 2001 From: Min RK Date: Thu, 20 May 2021 11:28:27 +0200 Subject: [PATCH 40/45] test access scopes in authorize handler - provider.add_client returns the client - fix Spawner access scopes - debug logging in mock spawners - Assign service access scopes --- jupyterhub/oauth/provider.py | 1 + jupyterhub/orm.py | 10 ++- jupyterhub/spawner.py | 4 +- jupyterhub/tests/mocking.py | 5 ++ jupyterhub/tests/test_services_auth.py | 119 +++++++++++++++++++------ jupyterhub/tests/test_singleuser.py | 70 +++++++++++++-- 6 files changed, 168 insertions(+), 41 deletions(-) diff --git a/jupyterhub/oauth/provider.py b/jupyterhub/oauth/provider.py index 8bcfe0cb..7594131a 100644 --- a/jupyterhub/oauth/provider.py +++ b/jupyterhub/oauth/provider.py @@ -614,6 +614,7 @@ class JupyterHubOAuthServer(WebApplicationServer): orm_client.description = description or client_id orm_client.allowed_roles = allowed_roles self.db.commit() + return orm_client def fetch_by_client_id(self, client_id): """Find a client by its id""" diff --git a/jupyterhub/orm.py b/jupyterhub/orm.py index 26219c72..c38feff7 100644 --- a/jupyterhub/orm.py +++ b/jupyterhub/orm.py @@ -514,9 +514,7 @@ 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) @@ -773,6 +771,11 @@ class OAuthCode(Expiring, Base): .first() ) + def __repr__(self): + return ( + f"<{self.__class__.__name__}(id={self.id}, client_id={self.client_id!r})>" + ) + class OAuthClient(Base): __tablename__ = 'oauth_clients' @@ -795,6 +798,9 @@ class OAuthClient(Base): # *not* the roles of the client itself allowed_roles = relationship('Role', secondary='oauth_client_role_map') + def __repr__(self): + return f"<{self.__class__.__name__}(identifier={self.identifier!r})>" + # General database utilities diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index 39b50133..22fbe451 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -222,8 +222,8 @@ class Spawner(LoggingConfigurable): @default("oauth_scopes") def _default_oauth_scopes(self): return [ - f"access:users:server!server={self.user.name}/{self.name}", - f"access:users:server!user={self.user.name}", + f"access:users:servers!server={self.user.name}/{self.name}", + f"access:users:servers!user={self.user.name}", ] handler = Any() diff --git a/jupyterhub/tests/mocking.py b/jupyterhub/tests/mocking.py index 5acb039a..723e7bd1 100644 --- a/jupyterhub/tests/mocking.py +++ b/jupyterhub/tests/mocking.py @@ -388,6 +388,10 @@ class MockSingleUserServer(SingleUserNotebookApp): def init_signal(self): pass + @default("log_level") + def _default_log_level(self): + return 10 + class StubSingleUserSpawner(MockSpawner): """Spawner that starts a MockSingleUserServer in a thread.""" @@ -425,6 +429,7 @@ class StubSingleUserSpawner(MockSpawner): app.initialize(args) assert app.hub_auth.oauth_client_id assert app.hub_auth.api_token + assert app.hub_auth.oauth_scopes app.start() self._thread = threading.Thread(target=_run) diff --git a/jupyterhub/tests/test_services_auth.py b/jupyterhub/tests/test_services_auth.py index 158bdbf9..165a7ad2 100644 --- a/jupyterhub/tests/test_services_auth.py +++ b/jupyterhub/tests/test_services_auth.py @@ -1,36 +1,20 @@ """Tests for service authentication""" -import asyncio import copy -import json import os import sys from binascii import hexlify -from functools import partial -from queue import Queue -from threading import Thread from unittest import mock from urllib.parse import parse_qs from urllib.parse import urlparse import pytest -import requests -import requests_mock from pytest import raises -from tornado.httpserver import HTTPServer from tornado.httputil import url_concat -from tornado.ioloop import IOLoop -from tornado.web import Application -from tornado.web import authenticated -from tornado.web import HTTPError -from tornado.web import RequestHandler from .. import orm from .. import roles from ..services.auth import _ExpiringDict -from ..services.auth import HubOAuth -from ..services.auth import HubOAuthenticated from ..utils import url_path_join -from .mocking import public_host from .mocking import public_url from .test_api import add_user from .utils import async_requests @@ -77,21 +61,29 @@ def test_expiring_dict(): assert cache.get('key', 'default') == 'cached value' -async def test_hubauth_token(app, mockservice_url): +async def test_hubauth_token(app, mockservice_url, create_user_with_scopes): """Test HubAuthenticated service with user API tokens""" - u = add_user(app.db, name='river') + u = create_user_with_scopes("access:services") token = u.new_api_token() + no_access_token = u.new_api_token(roles=[]) app.db.commit() + # token without sufficient permission in Authorization header + r = await async_requests.get( + public_url(app, mockservice_url) + '/whoami/', + headers={'Authorization': f'token {no_access_token}'}, + ) + assert r.status_code == 403 + # token in Authorization header r = await async_requests.get( public_url(app, mockservice_url) + '/whoami/', - headers={'Authorization': 'token %s' % token}, + headers={'Authorization': f'token {token}'}, ) r.raise_for_status() reply = r.json() sub_reply = {key: reply.get(key, 'missing') for key in ['name', 'admin']} - assert sub_reply == {'name': 'river', 'admin': False} + assert sub_reply == {'name': u.name, 'admin': 'missing'} # token in ?token parameter r = await async_requests.get( @@ -100,7 +92,7 @@ async def test_hubauth_token(app, mockservice_url): r.raise_for_status() reply = r.json() sub_reply = {key: reply.get(key, 'missing') for key in ['name', 'admin']} - assert sub_reply == {'name': 'river', 'admin': False} + assert sub_reply == {'name': u.name, 'admin': 'missing'} r = await async_requests.get( public_url(app, mockservice_url) + '/whoami/?token=no-such-token', @@ -237,14 +229,15 @@ async def test_hubauth_service_token(request, app, mockservice_url, scopes, allo (["token", "user"], ["identify"], []), # any item outside the list isn't allowed (["token", "user"], ["token", "server"], None), - # reuesting subset + # requesting subset (["admin", "user"], ["user"], ["user"]), (["user", "token", "server"], ["token", "user"], ["token", "user"]), ], ) -async def test_oauth_service( +async def test_oauth_service_roles( app, mockservice_url, + create_user_with_scopes, client_allowed_roles, request_roles, expected_roles, @@ -262,7 +255,9 @@ async def test_oauth_service( url = url_path_join(public_url(app, mockservice_url) + 'owhoami/?arg=x') # first request is only going to login and get us to the oauth form page s = AsyncSession() - name = 'link' + user = create_user_with_scopes("access:services") + roles.grant_role(app.db, user, "user") + name = user.name s.cookies = await app.login_user(name) r = await s.get(url) @@ -298,7 +293,7 @@ async def test_oauth_service( assert r.status_code == 200 reply = r.json() sub_reply = {key: reply.get(key, 'missing') for key in ('kind', 'name')} - assert sub_reply == {'name': 'link', 'kind': 'user'} + assert sub_reply == {'name': user.name, 'kind': 'user'} # token-authenticated request to HubOAuth token = app.users[name].new_api_token() @@ -318,12 +313,78 @@ async def test_oauth_service( assert reply['name'] == name -async def test_oauth_cookie_collision(app, mockservice_url): +@pytest.mark.parametrize( + "access_scopes, expect_success", + [ + (["access:services"], True), + (["access:services!service=$service"], True), + (["access:services!service=other-service"], False), + (["self"], False), + ([], False), + ], +) +async def test_oauth_access_scopes( + app, + mockservice_url, + create_user_with_scopes, + access_scopes, + expect_success, +): + """Check that oauth/authorize validates access scopes""" + service = mockservice_url + access_scopes = [s.replace("$service", service.name) for s in access_scopes] + url = url_path_join(public_url(app, mockservice_url) + 'owhoami/?arg=x') + # first request is only going to login and get us to the oauth form page + s = AsyncSession() + user = create_user_with_scopes(*access_scopes) + name = user.name + s.cookies = await app.login_user(name) + + r = await s.get(url) + if not expect_success: + assert r.status_code == 403 + return + r.raise_for_status() + # we should be looking at the oauth confirmation page + assert urlparse(r.url).path == app.base_url + 'hub/api/oauth2/authorize' + # verify oauth state cookie was set at some point + assert set(r.history[0].cookies.keys()) == {'service-%s-oauth-state' % service.name} + + # submit the oauth form to complete authorization + r = await s.post(r.url, headers={'Referer': r.url}) + r.raise_for_status() + assert r.url == url + # verify oauth cookie is set + assert 'service-%s' % service.name in set(s.cookies.keys()) + # verify oauth state cookie has been consumed + assert 'service-%s-oauth-state' % service.name not in set(s.cookies.keys()) + + # second request should be authenticated, which means no redirects + r = await s.get(url, allow_redirects=False) + r.raise_for_status() + assert r.status_code == 200 + reply = r.json() + sub_reply = {key: reply.get(key, 'missing') for key in ('kind', 'name')} + assert sub_reply == {'name': name, 'kind': 'user'} + + # revoke user access, should result in 403 + user.roles = [] + app.db.commit() + + # reset session id to avoid cached response + s.cookies.pop('jupyterhub-session-id') + + r = await s.get(url, allow_redirects=False) + assert r.status_code == 403 + + +async def test_oauth_cookie_collision(app, mockservice_url, create_user_with_scopes): service = mockservice_url url = url_path_join(public_url(app, mockservice_url), 'owhoami/') print(url) s = AsyncSession() name = 'mypha' + user = create_user_with_scopes("access:services", name=name) s.cookies = await app.login_user(name) state_cookie_name = 'service-%s-oauth-state' % service.name service_cookie_name = 'service-%s' % service.name @@ -376,7 +437,7 @@ async def test_oauth_cookie_collision(app, mockservice_url): assert state_cookies == [] -async def test_oauth_logout(app, mockservice_url): +async def test_oauth_logout(app, mockservice_url, create_user_with_scopes): """Verify that logout via the Hub triggers logout for oauth services 1. clears session id cookie @@ -390,11 +451,11 @@ async def test_oauth_logout(app, mockservice_url): # first request is only going to set login cookie s = AsyncSession() name = 'propha' - app_user = add_user(app.db, app=app, name=name) + user = create_user_with_scopes("access:services", name=name) def auth_tokens(): """Return list of OAuth access tokens for the user""" - return list(app.db.query(orm.APIToken).filter_by(user_id=app_user.id)) + return list(app.db.query(orm.APIToken).filter_by(user_id=user.id)) # ensure we start empty assert auth_tokens() == [] diff --git a/jupyterhub/tests/test_singleuser.py b/jupyterhub/tests/test_singleuser.py index ddf080f0..0b0eef5a 100644 --- a/jupyterhub/tests/test_singleuser.py +++ b/jupyterhub/tests/test_singleuser.py @@ -3,6 +3,8 @@ import sys from subprocess import check_output from urllib.parse import urlparse +import pytest + import jupyterhub from ..utils import url_path_join from .mocking import public_url @@ -11,7 +13,32 @@ from .utils import async_requests from .utils import AsyncSession -async def test_singleuser_auth(app): +@pytest.mark.parametrize( + "access_scopes, server_name, expect_success", + [ + (["access:users:servers"], "", True), + (["access:users:servers"], "named", True), + (["access:users:servers!user=$user"], "", True), + (["access:users:servers!user=$user"], "named", True), + (["access:users:servers!server=$server"], "", True), + (["access:users:servers!server=$server"], "named-server", True), + (["access:users:servers!server=$user/other"], "", False), + (["access:users:servers!server=$user/other"], "some-name", False), + (["access:users:servers!user=$other"], "", False), + (["access:users:servers!user=$other"], "named", False), + (["access:services"], "", False), + (["self"], "named", False), + (["admin"], "", False), + ([], "", False), + ], +) +async def test_singleuser_auth( + app, + create_user_with_scopes, + access_scopes, + server_name, + expect_success, +): # use StubSingleUserSpawner to launch a single-user app in a thread app.spawner_class = StubSingleUserSpawner app.tornado_settings['spawner_class'] = StubSingleUserSpawner @@ -19,10 +46,11 @@ async def test_singleuser_auth(app): # login, start the server cookies = await app.login_user('nandy') user = app.users['nandy'] - if not user.running: - await user.spawn() - await app.proxy.add_user(user) - url = public_url(app, user) + if server_name not in user.spawners or not user.spawners[server_name].active: + await user.spawn(server_name) + await app.proxy.add_user(user, server_name) + spawner = user.spawners[server_name] + url = url_path_join(public_url(app, user), server_name) # no cookies, redirects to login page r = await async_requests.get(url) @@ -40,7 +68,11 @@ async def test_singleuser_auth(app): assert ( urlparse(r.url) .path.rstrip('/') - .endswith(url_path_join('/user/nandy', user.spawner.default_url or "/tree")) + .endswith( + url_path_join( + f'/user/{user.name}', spawner.name, spawner.default_url or "/tree" + ) + ) ) assert r.status_code == 200 @@ -49,17 +81,39 @@ async def test_singleuser_auth(app): assert len(r.cookies) == 0 # accessing another user's server hits the oauth confirmation page + access_scopes = [s.replace("$user", user.name) for s in access_scopes] + access_scopes = [ + s.replace("$server", f"{user.name}/{server_name}") for s in access_scopes + ] + other_user = create_user_with_scopes(*access_scopes, name="burgess") + cookies = await app.login_user('burgess') s = AsyncSession() s.cookies = cookies r = await s.get(url) assert urlparse(r.url).path.endswith('/oauth2/authorize') + if not expect_success: + # user isn't authorized, should raise 403 + assert r.status_code == 403 + return + r.raise_for_status() # submit the oauth form to complete authorization r = await s.post(r.url, data={'scopes': ['identify']}, headers={'Referer': r.url}) final_url = urlparse(r.url).path.rstrip('/') - final_path = url_path_join('/user/nandy', user.spawner.default_url or "/tree") + final_path = url_path_join( + '/user/', user.name, spawner.name, spawner.default_url or "/tree" + ) assert final_url.endswith(final_path) - # user isn't authorized, should raise 403 + r.raise_for_status() + + # revoke user access, should result in 403 + other_user.roles = [] + app.db.commit() + + # reset session id to avoid cached response + s.cookies.pop('jupyterhub-session-id') + + r = await s.get(r.url, allow_redirects=False) assert r.status_code == 403 assert 'burgess' in r.text From 40de16e0e1e493156dbe58123e77749bd6590820 Mon Sep 17 00:00:00 2001 From: Min RK Date: Fri, 21 May 2021 10:17:57 +0200 Subject: [PATCH 41/45] Update service examples and documentation with access scopes and roles --- docs/source/reference/services.md | 114 +++++++++--------- examples/postgres/hub/jupyterhub_config.py | 4 +- examples/service-announcement/README.md | 44 ++++--- examples/service-announcement/announcement.py | 3 - .../service-announcement/jupyterhub_config.py | 27 ++++- examples/service-fastapi/README.md | 18 ++- examples/service-fastapi/app/models.py | 6 +- examples/service-fastapi/app/security.py | 20 ++- examples/service-fastapi/jupyterhub_config.py | 15 ++- .../external/jupyterhub_config.py | 36 +++--- .../managed/jupyterhub_config.py | 28 ++++- examples/service-whoami/README.md | 62 +++++++++- examples/service-whoami/jupyterhub_config.py | 15 ++- examples/service-whoami/whoami-oauth.py | 9 +- examples/service-whoami/whoami.py | 11 +- jupyterhub/services/auth.py | 17 ++- 16 files changed, 291 insertions(+), 138 deletions(-) diff --git a/docs/source/reference/services.md b/docs/source/reference/services.md index 408b396a..ab7d8da0 100644 --- a/docs/source/reference/services.md +++ b/docs/source/reference/services.md @@ -86,10 +86,18 @@ Hub-Managed Service would include: This example would be configured as follows in `jupyterhub_config.py`: ```python +c.JupyterHub.load_roles = [ + { + "name": "idle-culler", + "scopes": [ + "users:servers", + # also 'admin:users' if culling idle users as well + ] + } + c.JupyterHub.services = [ { 'name': 'idle-culler', - 'admin': True, 'command': [sys.executable, '-m', 'jupyterhub_idle_culler', '--timeout=3600'] } ] @@ -114,6 +122,7 @@ JUPYTERHUB_BASE_URL: Base URL of the Hub (https://mydomain[:port]/) JUPYTERHUB_SERVICE_PREFIX: URL path prefix of this service (/services/:service-name/) JUPYTERHUB_SERVICE_URL: Local URL where the service is expected to be listening. Only for proxied web services. +JUPYTERHUB_OAUTH_SCOPES: JSON-serialized list of scopes to use for allowing access to the service. ``` For the previous 'cull idle' Service example, these environment variables @@ -231,50 +240,8 @@ service. See the `service-whoami-flask` example in the [JupyterHub GitHub repo](https://github.com/jupyterhub/jupyterhub/tree/HEAD/examples/service-whoami-flask) for more details. -```python -from functools import wraps -import json -import os -from urllib.parse import quote - -from flask import Flask, redirect, request, Response - -from jupyterhub.services.auth import HubOAuth - -prefix = os.environ.get('JUPYTERHUB_SERVICE_PREFIX', '/') - -auth = HubOAuth( - api_token=os.environ['JUPYTERHUB_API_TOKEN'], - cache_max_age=60, -) - -app = Flask(__name__) - - -def authenticated(f): - """Decorator for authenticating with the Hub""" - @wraps(f) - def decorated(*args, **kwargs): - token = request.headers.get(auth.auth_header_name) - if token: - user = auth.user_for_token(token) - else: - user = None - if user: - return f(user, *args, **kwargs) - else: - # redirect to login url on failed auth - return redirect(auth.login_url + '?next=%s' % quote(request.path)) - return decorated - - -@app.route(prefix) -@authenticated -def whoami(user): - return Response( - json.dumps(user, indent=1, sort_keys=True), - mimetype='application/json', - ) +```{literalinclude} ../../../examples/service-whoami-flask/whoami-flask.py +:language: python ``` ### Authenticating tornado services with JupyterHub @@ -315,25 +282,38 @@ undefined, then any user will be allowed. If you don't want to use the reference implementation (e.g. you find the implementation a poor fit for your Flask app), you can implement authentication via the Hub yourself. -We recommend looking at the [`HubAuth`][hubauth] class implementation for reference, +JupyterHub is a standard OAuth2 provider, +so you can any OAuth 2 implementation appropriate for +See the [FastAPI example][] for an example of using JupyterHub as an OAuth provider with fastapi, +without using any code imported from JupyterHub. + +On completion of OAuth, you will have an access token for JupyterHub, +which can be used to identify the user and the permissions (scopes) +the user has authorized for your service. + +You will only get to this stage if the user has the required `access:services!service=$service-name` scope. + +To retrieve the user model for the token, make a request to `GET /hub/api/user` with the token in the Authorization header. +For example, using flask: + +```{literal-include} ../../../examples/service-whoami-flask/whoami-flask.py +:language: python +``` + +We recommend looking at the [`HubOAuth`][huboauth] class implementation for reference, and taking note of the following process: -1. retrieve the cookie `jupyterhub-services` from the request. -2. Make an API request `GET /hub/api/authorizations/cookie/jupyterhub-services/cookie-value`, - where cookie-value is the url-encoded value of the `jupyterhub-services` cookie. - This request must be authenticated with a Hub API token in the `Authorization` header, - for example using the `api_token` from your [external service's configuration](#externally-managed-services). +1. retrieve the token from the request. +2. Make an API request `GET /hub/api/user`, + with the token in the `Authorization` header. For example, with [requests][]: ```python r = requests.get( - '/'.join(["http://127.0.0.1:8081/hub/api", - "authorizations/cookie/jupyterhub-services", - quote(encrypted_cookie, safe=''), - ]), + "http://127.0.0.1:8081/hub/api/user", headers = { - 'Authorization' : 'token %s' % api_token, + 'Authorization' : f'token {api_token}', }, ) r.raise_for_status() @@ -342,13 +322,27 @@ and taking note of the following process: 3. On success, the reply will be a JSON model describing the user: - ```json + ```python { "name": "inara", - "groups": ["serenity", "guild"] + # groups may be omitted, depending on permissions + "groups": ["serenity", "guild"], + # scopes is new in JupyterHub 2.0 + "scopes": [ + "access:services", + "read:users:name", + "read:users!user=inara", + "..." + ] } ``` +The `scopes` field can be used to manage access. +Note: a user will have access to a service to complete oauth access to the service for the first time. +Individual permissions may be revoked at any later point without revoking the token, +in which case the `scopes` field in this model should be checked on each access. +The default required scopes for access are available from `hub_auth.oauth_scopes` or `$JUPYTERHUB_OAUTH_SCOPES`. + An example of using an Externally-Managed Service and authentication is in [nbviewer README][nbviewer example] section on securing the notebook viewer, and an example of its configuration is found [here](https://github.com/jupyter/nbviewer/blob/ed942b10a52b6259099e2dd687930871dc8aac22/nbviewer/providers/base.py#L95). @@ -357,9 +351,9 @@ section on securing the notebook viewer. [requests]: http://docs.python-requests.org/en/master/ [services_auth]: ../api/services.auth.html -[hubauth]: ../api/services.auth.html#jupyterhub.services.auth.HubAuth -[hubauth.user_for_cookie]: ../api/services.auth.html#jupyterhub.services.auth.HubAuth.user_for_cookie +[huboauth]: ../api/services.auth.html#jupyterhub.services.auth.HubOAuth [hubauth.user_for_token]: ../api/services.auth.html#jupyterhub.services.auth.HubAuth.user_for_token [hubauthenticated]: ../api/services.auth.html#jupyterhub.services.auth.HubAuthenticated [nbviewer example]: https://github.com/jupyter/nbviewer#securing-the-notebook-viewer +[fastapi example]: https://github.com/jupyterhub/jupyterhub/tree/HEAD/examples/service-fastapi [jupyterhub_idle_culler]: https://github.com/jupyterhub/jupyterhub-idle-culler diff --git a/examples/postgres/hub/jupyterhub_config.py b/examples/postgres/hub/jupyterhub_config.py index 9482657b..4a8efaef 100644 --- a/examples/postgres/hub/jupyterhub_config.py +++ b/examples/postgres/hub/jupyterhub_config.py @@ -1,10 +1,10 @@ # Configuration file for jupyterhub (postgres example). -c = get_config() +c = get_config() # noqa # Add some users. c.JupyterHub.admin_users = {'rhea'} -c.Authenticator.whitelist = {'ganymede', 'io', 'rhea'} +c.Authenticator.allowed_users = {'ganymede', 'io', 'rhea'} # These environment variables are automatically supplied by the linked postgres # container. diff --git a/examples/service-announcement/README.md b/examples/service-announcement/README.md index cff87ec3..4be75b57 100644 --- a/examples/service-announcement/README.md +++ b/examples/service-announcement/README.md @@ -6,15 +6,17 @@ that appear when JupyterHub renders pages. To run the service as a hub-managed service simply include in your JupyterHub configuration file something like: - c.JupyterHub.services = [ - { - 'name': 'announcement', - 'url': 'http://127.0.0.1:8888', - 'command': [sys.executable, "-m", "announcement"], - } - ] +```python +c.JupyterHub.services = [ + { + 'name': 'announcement', + 'url': 'http://127.0.0.1:8888', + 'command': [sys.executable, "-m", "announcement", "--port", "8888"], + } +] +``` -This starts the announcements service up at `/services/announcement` when +This starts the announcements service up at `/services/announcement/` when JupyterHub launches. By default the announcement text is empty. The `announcement` module has a configurable port (default 8888) and an API @@ -23,15 +25,28 @@ that environment variable is set or `/` if it is not. ## Managing the Announcement -Admin users can set the announcement text with an API token: +Users with permission can set the announcement text with an API token: $ curl -X POST -H "Authorization: token " \ -d '{"announcement":"JupyterHub will be upgraded on August 14!"}' \ - https://.../services/announcement + https://.../services/announcement/ + +To grant permission, add a role (JupyterHub 2.0) with access to the announcement service: + +```python +# grant the 'announcer' permission to access the announcement service +c.JupyterHub.load_roles = [ + { + "name": "announcers", + "users": ["announcer"], # or groups + "scopes": ["access:services!service=announcement"], + } +] +``` Anyone can read the announcement: - $ curl https://.../services/announcement | python -m json.tool + $ curl https://.../services/announcement/ | python -m json.tool { announcement: "JupyterHub will be upgraded on August 14!", timestamp: "...", @@ -41,10 +56,11 @@ Anyone can read the announcement: The time the announcement was posted is recorded in the `timestamp` field and the user who posted the announcement is recorded in the `user` field. -To clear the announcement text, just DELETE. Only admin users can do this. +To clear the announcement text, send a DELETE request. +This has the same permissionOnly admin users can do this. - $ curl -X POST -H "Authorization: token " \ - https://.../services/announcement + $ curl -X DELETE -H "Authorization: token " \ + https://.../services/announcement/ ## Seeing the Announcement in JupyterHub diff --git a/examples/service-announcement/announcement.py b/examples/service-announcement/announcement.py index a0cf5964..6a50c33d 100644 --- a/examples/service-announcement/announcement.py +++ b/examples/service-announcement/announcement.py @@ -13,9 +13,6 @@ from jupyterhub.services.auth import HubAuthenticated class AnnouncementRequestHandler(HubAuthenticated, web.RequestHandler): """Dynamically manage page announcements""" - hub_users = [] - allow_admin = True - def initialize(self, storage): """Create storage for announcement text""" self.storage = storage diff --git a/examples/service-announcement/jupyterhub_config.py b/examples/service-announcement/jupyterhub_config.py index 069a76d7..9bca9809 100644 --- a/examples/service-announcement/jupyterhub_config.py +++ b/examples/service-announcement/jupyterhub_config.py @@ -2,11 +2,18 @@ import sys # To run the announcement service managed by the hub, add this. +port = 9999 c.JupyterHub.services = [ { 'name': 'announcement', - 'url': 'http://127.0.0.1:8888', - 'command': [sys.executable, "-m", "announcement"], + 'url': f'http://127.0.0.1:{port}', + 'command': [ + sys.executable, + "-m", + "announcement", + '--port', + str(port), + ], } ] @@ -14,3 +21,19 @@ c.JupyterHub.services = [ # for an example of how to do this. c.JupyterHub.template_paths = ["templates"] + +c.Authenticator.allowed_users = {"announcer", "otheruser"} + +# grant the 'announcer' permission to access the announcement service +c.JupyterHub.load_roles = [ + { + "name": "announcers", + "users": ["announcer"], + "scopes": ["access:services!service=announcement"], + } +] + +# dummy spawner and authenticator for testing, don't actually use these! +c.JupyterHub.authenticator_class = 'dummy' +c.JupyterHub.spawner_class = 'simple' +c.JupyterHub.ip = '127.0.0.1' # let's just run on localhost while dummy auth is enabled diff --git a/examples/service-fastapi/README.md b/examples/service-fastapi/README.md index b2167228..26fb6a7b 100644 --- a/examples/service-fastapi/README.md +++ b/examples/service-fastapi/README.md @@ -16,6 +16,7 @@ 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 + Login with username 'test-user' and any password. 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. @@ -24,10 +25,10 @@ $ 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"} +{"detail":"Must login with token parameter, or Authorization bearer header"} -$ curl -X POST http://127.0.0.1:8000/hub/api/authorizations/token \ - -d '{"username": "myname", "password": "mypasswd!"}' \ +$ curl -X POST http://127.0.0.1:8000/hub/api/users/test-user/tokens \ + -d '{"auth": {"username": "test-user", "password": "mypasswd!"}}' \ | jq '.token' "3fee13ce6d2845da9bd5f2c2170d3428" @@ -35,13 +36,18 @@ $ curl -X GET http://127.0.0.1:8000/services/fastapi/me \ -H "Authorization: Bearer 3fee13ce6d2845da9bd5f2c2170d3428" \ | jq . { - "name": "myname", + "name": "test-user", "admin": false, "groups": [], "server": null, "pending": null, - "last_activity": "2021-04-07T18:05:11.587638+00:00", - "servers": null + "last_activity": "2021-05-21T09:13:00.514309+00:00", + "servers": null, + "scopes": [ + "access:services", + "access:users:servers!user=test-user", + "...", + ] } ``` diff --git a/examples/service-fastapi/app/models.py b/examples/service-fastapi/app/models.py index c3751053..33fdab68 100644 --- a/examples/service-fastapi/app/models.py +++ b/examples/service-fastapi/app/models.py @@ -1,5 +1,6 @@ from datetime import datetime from typing import Any +from typing import Dict from typing import List from typing import Optional @@ -22,11 +23,12 @@ class Server(BaseModel): class User(BaseModel): name: str admin: bool - groups: List[str] + groups: Optional[List[str]] server: Optional[str] pending: Optional[str] last_activity: datetime - servers: Optional[List[Server]] + servers: Optional[Dict[str, Server]] + scopes: List[str] # https://stackoverflow.com/questions/64501193/fastapi-how-to-use-httpexception-in-responses diff --git a/examples/service-fastapi/app/security.py b/examples/service-fastapi/app/security.py index 0f1c79bd..62b2ea5e 100644 --- a/examples/service-fastapi/app/security.py +++ b/examples/service-fastapi/app/security.py @@ -1,3 +1,4 @@ +import json import os from fastapi import HTTPException @@ -27,6 +28,12 @@ auth_by_header = OAuth2AuthorizationCodeBearer( ### our client_secret (JUPYTERHUB_API_TOKEN) and that code to get an ### access_token, which it returns to browser, which places in Authorization header. +if os.environ.get("JUPYTERHUB_OAUTH_SCOPES"): + # typically ["access:services", "access:services!service=$service_name"] + access_scopes = json.loads(os.environ["JUPYTERHUB_OAUTH_SCOPES"]) +else: + access_scopes = ["access:services"] + ### For consideration: optimize performance with a cache instead of ### always hitting the Hub api? async def get_current_user( @@ -58,4 +65,15 @@ async def get_current_user( }, ) user = User(**resp.json()) - return user + if any(scope in user.scopes for scope in access_scopes): + return user + else: + raise HTTPException( + status.HTTP_403_FORBIDDEN, + detail={ + "msg": f"User not authorized: {user.name}", + "request_url": str(resp.request.url), + "token": token, + "user": resp.json(), + }, + ) diff --git a/examples/service-fastapi/jupyterhub_config.py b/examples/service-fastapi/jupyterhub_config.py index 9fb054ec..f8f9e864 100644 --- a/examples/service-fastapi/jupyterhub_config.py +++ b/examples/service-fastapi/jupyterhub_config.py @@ -24,8 +24,21 @@ 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}, } ] + +c.JupyterHub.load_roles = [ + { + "name": "user", + # grant all users access to services + "scopes": ["self", "access:services"], + }, +] + + +# dummy for testing, create test-user +c.Authenticator.allowed_users = {"test-user"} +c.JupyterHub.authenticator_class = "dummy" +c.JupyterHub.spawner_class = "simple" diff --git a/examples/service-notebook/external/jupyterhub_config.py b/examples/service-notebook/external/jupyterhub_config.py index c6157253..21911fc8 100644 --- a/examples/service-notebook/external/jupyterhub_config.py +++ b/examples/service-notebook/external/jupyterhub_config.py @@ -1,33 +1,35 @@ # our user list c.Authenticator.allowed_users = ['minrk', 'ellisonbg', 'willingc'] -# ellisonbg and willingc have access to a shared server: +service_name = 'shared-notebook' +service_port = 9999 +group_name = 'shared' -c.JupyterHub.load_groups = {'shared-notebook-grp': ['ellisonbg', 'willingc']} +# ellisonbg and willingc are in a group that will access the shared server: -c.JupyterHub.load_roles = [ - { - "name": "shared-notebook", - "groups": ["shared-notebook-grp"], - "scopes": ["access:services!service=shared-notebook"], - }, - # by default, the user role has access to all services - # we want to limit that, so give users only access to 'self' - { - "name": "user", - "scopes": ["self"], - }, -] +c.JupyterHub.load_groups = {group_name: ['ellisonbg', 'willingc']} # start the notebook server as a service c.JupyterHub.services = [ { - 'name': 'shared-notebook', - 'url': 'http://127.0.0.1:9999', + 'name': service_name, + 'url': 'http://127.0.0.1:{}'.format(service_port), 'api_token': 'c3a29e5d386fd7c9aa1e8fe9d41c282ec8b', } ] +# This "role assignment" is what grants members of the group +# access to the service +c.JupyterHub.load_roles = [ + { + "name": "shared-notebook", + "groups": [group_name], + "scopes": [f"access:services!service={service_name}"], + }, +] + + # dummy spawner and authenticator for testing, don't actually use these! c.JupyterHub.authenticator_class = 'dummy' c.JupyterHub.spawner_class = 'simple' +c.JupyterHub.ip = '127.0.0.1' # let's just run on localhost while dummy auth is enabled diff --git a/examples/service-notebook/managed/jupyterhub_config.py b/examples/service-notebook/managed/jupyterhub_config.py index f9ef721f..10b95ef0 100644 --- a/examples/service-notebook/managed/jupyterhub_config.py +++ b/examples/service-notebook/managed/jupyterhub_config.py @@ -1,19 +1,35 @@ # our user list -c.Authenticator.whitelist = ['minrk', 'ellisonbg', 'willingc'] - -# ellisonbg and willingc have access to a shared server: - -c.JupyterHub.load_groups = {'shared': ['ellisonbg', 'willingc']} +c.Authenticator.allowed_users = ['minrk', 'ellisonbg', 'willingc'] service_name = 'shared-notebook' service_port = 9999 group_name = 'shared' +# ellisonbg and willingc have access to a shared server: + +c.JupyterHub.load_groups = {group_name: ['ellisonbg', 'willingc']} + # start the notebook server as a service c.JupyterHub.services = [ { 'name': service_name, 'url': 'http://127.0.0.1:{}'.format(service_port), - 'command': ['jupyterhub-singleuser', '--group=shared', '--debug'], + 'command': ['jupyterhub-singleuser', '--debug'], } ] + +# This "role assignment" is what grants members of the group +# access to the service +c.JupyterHub.load_roles = [ + { + "name": "shared-notebook", + "groups": [group_name], + "scopes": [f"access:services!service={service_name}"], + }, +] + + +# dummy spawner and authenticator for testing, don't actually use these! +c.JupyterHub.authenticator_class = 'dummy' +c.JupyterHub.spawner_class = 'simple' +c.JupyterHub.ip = '127.0.0.1' # let's just run on localhost while dummy auth is enabled diff --git a/examples/service-whoami/README.md b/examples/service-whoami/README.md index 2a94385e..2fb59f02 100644 --- a/examples/service-whoami/README.md +++ b/examples/service-whoami/README.md @@ -2,15 +2,15 @@ Uses `jupyterhub.services.HubAuthenticated` to authenticate requests with the Hub. -There is an implementation each of cookie-based `HubAuthenticated` and OAuth-based `HubOAuthenticated`. +There is an implementation each of api-token-based `HubAuthenticated` and OAuth-based `HubOAuthenticated`. ## Run -1. Launch JupyterHub and the `whoami service` with +1. Launch JupyterHub and the `whoami` services with jupyterhub --ip=127.0.0.1 -2. Visit http://127.0.0.1:8000/services/whoami or http://127.0.0.1:8000/services/whoami-oauth +2. Visit http://127.0.0.1:8000/services/whoami-oauth After logging in with your local-system credentials, you should see a JSON dump of your user info: @@ -24,15 +24,65 @@ After logging in with your local-system credentials, you should see a JSON dump } ``` +The `whoami-api` service powered by the base `HubAuthenticated` class only supports token-authenticated API requests, +not browser visits, because it does not implement OAuth. Visit it by requesting an api token from the tokens page, +and making a direct request: + +```bash +$ curl -H "Authorization: token 8630bbd8ef064c48b22c7f122f0cd8ad" http://127.0.0.1:8000/services/whoami-api/ | jq . +{ + "admin": false, + "created": "2021-05-21T09:47:41.299400Z", + "groups": [], + "kind": "user", + "last_activity": "2021-05-21T09:49:08.290745Z", + "name": "test", + "pending": null, + "roles": [ + "user" + ], + "scopes": [ + "access:services", + "access:users:servers!user=test", + "read:users!user=test", + "read:users:activity!user=test", + "read:users:groups!user=test", + "read:users:name!user=test", + "read:users:servers!user=test", + "read:users:tokens!user=test", + "users!user=test", + "users:activity!user=test", + "users:groups!user=test", + "users:name!user=test", + "users:servers!user=test", + "users:tokens!user=test" + ], + "server": null +} +``` + This relies on the Hub starting the whoami services, via config (see [jupyterhub_config.py](./jupyterhub_config.py)). -You may set the `hub_users` configuration in the service script -to restrict access to the service to a whitelist of allowed users. -By default, any authenticated user is allowed. +To govern access to the services, create **roles** with the scope `access:services!service=$service-name`, +and assign users to the scope. + +The jupyterhub_config.py grants access for all users to all services via the default 'user' role, with: + +```python +c.JupyterHub.load_roles = [ + { + "name": "user", + # grant all users access to all services + "scopes": ["access:services", "self"], + } +] +``` A similar service could be run externally, by setting the JupyterHub service environment variables: JUPYTERHUB_API_TOKEN JUPYTERHUB_SERVICE_PREFIX + JUPYTERHUB_OAUTH_SCOPES + JUPYTERHUB_CLIENT_ID # for whoami-oauth only or instantiating and configuring a HubAuth object yourself, and attaching it as `self.hub_auth` in your HubAuthenticated handlers. diff --git a/examples/service-whoami/jupyterhub_config.py b/examples/service-whoami/jupyterhub_config.py index 1d1f9435..c1363e62 100644 --- a/examples/service-whoami/jupyterhub_config.py +++ b/examples/service-whoami/jupyterhub_config.py @@ -2,7 +2,7 @@ import sys c.JupyterHub.services = [ { - 'name': 'whoami', + 'name': 'whoami-api', 'url': 'http://127.0.0.1:10101', 'command': [sys.executable, './whoami.py'], }, @@ -12,3 +12,16 @@ c.JupyterHub.services = [ 'command': [sys.executable, './whoami-oauth.py'], }, ] + +c.JupyterHub.load_roles = [ + { + "name": "user", + # grant all users access to all services + "scopes": ["access:services", "self"], + } +] + +# dummy spawner and authenticator for testing, don't actually use these! +c.JupyterHub.authenticator_class = 'dummy' +c.JupyterHub.spawner_class = 'simple' +c.JupyterHub.ip = '127.0.0.1' # let's just run on localhost while dummy auth is enabled diff --git a/examples/service-whoami/whoami-oauth.py b/examples/service-whoami/whoami-oauth.py index 72c97dda..b2158e51 100644 --- a/examples/service-whoami/whoami-oauth.py +++ b/examples/service-whoami/whoami-oauth.py @@ -1,6 +1,6 @@ """An example service authenticating with the Hub. -This example service serves `/services/whoami/`, +This example service serves `/services/whoami-oauth/`, authenticated with the Hub, showing the user their own info. """ @@ -20,13 +20,6 @@ from jupyterhub.utils import url_path_join class WhoAmIHandler(HubOAuthenticated, RequestHandler): - # hub_users can be a set of users who are allowed to access the service - # `getuser()` here would mean only the user who started the service - # can access the service: - - # from getpass import getuser - # hub_users = {getuser()} - @authenticated def get(self): user_model = self.get_current_user() diff --git a/examples/service-whoami/whoami.py b/examples/service-whoami/whoami.py index 2a5a3373..435ff329 100644 --- a/examples/service-whoami/whoami.py +++ b/examples/service-whoami/whoami.py @@ -1,6 +1,8 @@ """An example service authenticating with the Hub. -This serves `/services/whoami/`, authenticated with the Hub, showing the user their own info. +This serves `/services/whoami-api/`, authenticated with the Hub, showing the user their own info. + +HubAuthenticated only supports token-based access. """ import json import os @@ -16,13 +18,6 @@ from jupyterhub.services.auth import HubAuthenticated class WhoAmIHandler(HubAuthenticated, RequestHandler): - # hub_users can be a set of users who are allowed to access the service - # `getuser()` here would mean only the user who started the service - # can access the service: - - # from getpass import getuser - # hub_users = {getuser()} - @authenticated def get(self): user_model = self.get_current_user() diff --git a/jupyterhub/services/auth.py b/jupyterhub/services/auth.py index 1066a9d7..0127883e 100644 --- a/jupyterhub/services/auth.py +++ b/jupyterhub/services/auth.py @@ -824,13 +824,26 @@ class UserNotAllowed(Exception): ) -class HubAuthenticated(object): +class HubAuthenticated: """Mixin for tornado handlers that are authenticated with JupyterHub A handler that mixes this in must have the following attributes/properties: - .hub_auth: A HubAuth instance - .hub_scopes: A set of JupyterHub 2.0 OAuth scopes to allow. + Default comes from .hub_auth.oauth_scopes, + which in turn is set by $JUPYTERHUB_OAUTH_SCOPES + Default values include: + - 'access:services', 'access:services!service={service_name}' for services + - 'access:users:servers', 'access:users:servers!user={user}', + 'access:users:servers!server={user}/{server_name}' + for single-user servers + + If hub_scopes is not used (e.g. JupyterHub 1.x), + these additional properties can be used: + + - .allow_admin: If True, allow any admin user. + Default: False. - .hub_users: A set of usernames to allow. If left unspecified or None, username will not be checked. - .hub_groups: A set of group names to allow. @@ -943,6 +956,8 @@ class HubAuthenticated(object): # note: this means successful authentication, but insufficient permission raise UserNotAllowed(model) + # proceed with the pre-2.0 way if hub_scopes is not set + if self.allow_admin and model.get('admin', False): app_log.debug("Allowing Hub admin %s", name) return model From fbea31d00a0c1f5f72d119def72322c1246dd54a Mon Sep 17 00:00:00 2001 From: Min RK Date: Fri, 21 May 2021 15:23:31 +0200 Subject: [PATCH 42/45] support groups in _intersect_scopes Requires db resolution --- jupyterhub/scopes.py | 119 +++++++++++++++++++++------- jupyterhub/services/auth.py | 4 +- jupyterhub/tests/test_scopes.py | 78 ++++++++++++++++++ jupyterhub/tests/test_singleuser.py | 14 ++++ 4 files changed, 184 insertions(+), 31 deletions(-) diff --git a/jupyterhub/scopes.py b/jupyterhub/scopes.py index afce5974..32276a20 100644 --- a/jupyterhub/scopes.py +++ b/jupyterhub/scopes.py @@ -13,7 +13,9 @@ import functools import inspect import warnings from enum import Enum +from functools import lru_cache +import sqlalchemy as sa from tornado import web from tornado.log import app_log @@ -114,21 +116,38 @@ class Scope(Enum): ALL = True -def _intersect_expanded_scopes(scopes_a, scopes_b): - """Intersect two sets of expanded scopes by comparing their permissions +def _intersect_expanded_scopes(scopes_a, scopes_b, db=None): + """Intersect two sets of scopes by comparing their permissions Arguments: scopes_a, scopes_b: sets of expanded scopes + db (optional): db connection for resolving group membership Returns: intersection: set of expanded scopes as intersection of the arguments - Note: Intersects correctly with ALL and exact filter matches - (i.e. users!user=x & read:users:name -> read:users:name!user=x) - - Does not currently intersect with containing filters - (i.e. users!group=x & users!user=y even if user y is in group x) + If db is given, group membership will be accounted for in intersections, + Otherwise, it can result in lower than intended permissions, + (i.e. users!group=x & users!user=y will be empty, even if user y is in group x.) """ + empty_set = frozenset() + + # cached lookups for group membership of users and servers + @lru_cache() + def groups_for_user(username): + """Get set of group names for a given username""" + user = db.query(orm.User).filter_by(name=username).first() + if user is None: + return empty_set + else: + return {group.name for group in user.groups} + + @lru_cache() + def groups_for_server(server): + """Get set of group names for a given server""" + username, _, servername = server.partition("/") + return groups_for_user(username) + parsed_scopes_a = parse_scopes(scopes_a) parsed_scopes_b = parse_scopes(scopes_b) @@ -144,11 +163,14 @@ def _intersect_expanded_scopes(scopes_a, scopes_b): elif filters_b == Scope.ALL: common_filters[base] = filters_a else: - # warn *if* there are non-overlapping user= and group= filters common_entities = filters_a.keys() & filters_b.keys() all_entities = filters_a.keys() | filters_b.keys() + + # if we don't have a db session, we can't check group membership + # warn *if* there are non-overlapping user= and group= filters that we can't check if ( - not warned + db is None + and not warned and 'group' in all_entities and ('user' in all_entities or 'server' in all_entities) ): @@ -160,39 +182,67 @@ def _intersect_expanded_scopes(scopes_a, scopes_b): not warned and "group" in a and b_key in b - and set(a["group"]).difference(b.get("group", [])) - and set(b[b_key]).difference(a.get(b_key, [])) + and a["group"].difference(b.get("group", [])) + and b[b_key].difference(a.get(b_key, [])) ): warnings.warn( f"{base}[!{b_key}={b[b_key]}, !group={a['group']}] combinations of filters present," - " intersection between not considered. May result in lower than intended permissions.", + " without db access. Intersection between not considered." + " May result in lower than intended permissions.", UserWarning, ) warned = True common_filters[base] = { - entity: set(parsed_scopes_a[base][entity]) - & set(parsed_scopes_b[base][entity]) + entity: filters_a[entity] & filters_b[entity] for entity in common_entities } - if 'server' in all_entities and 'user' in all_entities: - if filters_a.get('server') == filters_b.get('server'): - continue + # resolve hierarchies (group/user/server) in both directions + common_servers = common_filters[base].get("server", set()) + common_users = common_filters[base].get("user", set()) - additional_servers = set() - # resolve user/server hierarchy in both directions - for a, b in [(filters_a, filters_b), (filters_b, filters_a)]: - if 'server' in a and 'user' in b: - for server in a['server']: + for a, b in [(filters_a, filters_b), (filters_b, filters_a)]: + if 'server' in a and b.get('server') != a['server']: + # skip already-added servers (includes overlapping servers) + servers = a['server'].difference(common_servers) + + # resolve user/server hierarchy + if servers and 'user' in b: + for server in servers: username, _, servername = server.partition("/") if username in b['user']: - additional_servers.add(server) + common_servers.add(server) - if additional_servers: - if "server" not in common_filters[base]: - common_filters[base]["server"] = set() - common_filters[base]["server"].update(additional_servers) + # resolve group/server hierarchy if db available + servers = servers.difference(common_servers) + if db is not None and servers and 'group' in b: + for server in servers: + server_groups = groups_for_server(server) + if server_groups & b['group']: + common_servers.add(server) + + # resolve group/user hierarchy if db available and user sets aren't identical + if ( + db is not None + and 'user' in a + and 'group' in b + and b.get('user') != a['user'] + ): + # skip already-added users (includes overlapping users) + users = a['user'].difference(common_users) + for username in users: + groups = groups_for_user(username) + if groups & b["group"]: + common_users.add(username) + + # add server filter if there wasn't one before + if common_servers and "server" not in common_filters[base]: + common_filters[base]["server"] = common_servers + + # add user filter if it's non-empty and there wasn't one before + if common_users and "user" not in common_filters[base]: + common_filters[base]["user"] = common_users return unparse_scopes(common_filters) @@ -244,11 +294,21 @@ def get_scopes_for(orm_object): ) owner_scopes = roles.expand_roles_to_scopes(owner) + + if token_scopes == {'all'}: + # token_scopes is only 'all', return owner scopes as-is + # short-circuit common case where we don't need to compute an intersection + return owner_scopes + if 'all' in token_scopes: token_scopes.remove('all') token_scopes |= owner_scopes - intersection = _intersect_expanded_scopes(token_scopes, owner_scopes) + intersection = _intersect_expanded_scopes( + token_scopes, + owner_scopes, + db=sa.inspect(orm_object).session, + ) discarded_token_scopes = token_scopes - intersection # Not taking symmetric difference here because token owner can naturally have more scopes than token @@ -473,7 +533,8 @@ def check_scope_filter(sub_scope, orm_resource, kind): # Fall back on checking if we have group access for this user orm_resource = orm_resource.user kind = 'user' - elif kind == 'user' and 'group' in sub_scope: + + if kind == 'user' and 'group' in sub_scope: group_names = {group.name for group in orm_resource.groups} user_in_group = bool(group_names & set(sub_scope['group'])) if user_in_group: diff --git a/jupyterhub/services/auth.py b/jupyterhub/services/auth.py index 0127883e..64080723 100644 --- a/jupyterhub/services/auth.py +++ b/jupyterhub/services/auth.py @@ -38,7 +38,7 @@ from traitlets import Unicode from traitlets import validate from traitlets.config import SingletonConfigurable -from ..scopes import _intersect_scopes +from ..scopes import _intersect_expanded_scopes from ..utils import url_path_join @@ -70,7 +70,7 @@ def check_scopes(required_scopes, scopes): if isinstance(required_scopes, str): required_scopes = {required_scopes} - intersection = _intersect_scopes(required_scopes, scopes) + intersection = _intersect_expanded_scopes(required_scopes, scopes) # re-intersect with required_scopes in case the intersection # applies stricter filters than required_scopes declares # e.g. required_scopes = {'read:users'} and intersection has only {'read:users!user=x'} diff --git a/jupyterhub/tests/test_scopes.py b/jupyterhub/tests/test_scopes.py index 81642cba..305112f7 100644 --- a/jupyterhub/tests/test_scopes.py +++ b/jupyterhub/tests/test_scopes.py @@ -763,3 +763,81 @@ def test_intersect_expanded_scopes(left, right, expected, should_warn, recwarn): assert len(recwarn) == 1 else: assert len(recwarn) == 0 + + +@pytest.mark.parametrize( + "left, right, expected, groups", + [ + ( + ["users!group=gx"], + ["users!user=ux"], + ["users!user=ux"], + {"gx": ["ux"]}, + ), + ( + ["read:users!group=gx"], + ["read:users!user=nosuchuser"], + [], + {}, + ), + ( + ["read:users!group=gx"], + ["read:users!server=nosuchuser/server"], + [], + {}, + ), + ( + ["read:users!group=gx"], + ["read:users!server=ux/server"], + ["read:users!server=ux/server"], + {"gx": ["ux"]}, + ), + ( + ["read:users!group=gx"], + ["read:users!server=ux/server", "read:users!user=uy"], + ["read:users!server=ux/server"], + {"gx": ["ux"], "gy": ["uy"]}, + ), + ( + ["read:users!group=gy"], + ["read:users!server=ux/server", "read:users!user=uy"], + ["read:users!user=uy"], + {"gx": ["ux"], "gy": ["uy"]}, + ), + ], +) +def test_intersect_groups(request, db, left, right, expected, groups): + if isinstance(left, str): + left = set([left]) + if isinstance(right, str): + right = set([right]) + + # if we have a db connection, we can actually resolve + created = [] + for groupname, members in groups.items(): + group = orm.Group.find(db, name=groupname) + if not group: + group = orm.Group(name=groupname) + db.add(group) + created.append(group) + db.commit() + for username in members: + user = orm.User.find(db, name=username) + if user is None: + user = orm.User(name=username) + db.add(user) + created.append(user) + user.groups.append(group) + db.commit() + + def _cleanup(): + for obj in created: + db.delete(obj) + db.commit() + + request.addfinalizer(_cleanup) + + # run every test in both directions, to ensure symmetry of the inputs + for a, b in [(left, right), (right, left)]: + intersection = _intersect_expanded_scopes(set(left), set(right), db) + assert intersection == set(expected) diff --git a/jupyterhub/tests/test_singleuser.py b/jupyterhub/tests/test_singleuser.py index 0b0eef5a..d2fe3657 100644 --- a/jupyterhub/tests/test_singleuser.py +++ b/jupyterhub/tests/test_singleuser.py @@ -6,6 +6,7 @@ from urllib.parse import urlparse import pytest import jupyterhub +from .. import orm from ..utils import url_path_join from .mocking import public_url from .mocking import StubSingleUserSpawner @@ -16,6 +17,8 @@ from .utils import AsyncSession @pytest.mark.parametrize( "access_scopes, server_name, expect_success", [ + (["access:users:servers!group=$group"], "", True), + (["access:users:servers!group=other-group"], "", False), (["access:users:servers"], "", True), (["access:users:servers"], "named", True), (["access:users:servers!user=$user"], "", True), @@ -46,6 +49,16 @@ async def test_singleuser_auth( # login, start the server cookies = await app.login_user('nandy') user = app.users['nandy'] + + group = orm.Group.find(app.db, name="visitors") + if group is None: + group = orm.Group(name="visitors") + app.db.add(group) + app.db.commit() + if group not in user.groups: + user.groups.append(group) + app.db.commit() + if server_name not in user.spawners or not user.spawners[server_name].active: await user.spawn(server_name) await app.proxy.add_user(user, server_name) @@ -85,6 +98,7 @@ async def test_singleuser_auth( access_scopes = [ s.replace("$server", f"{user.name}/{server_name}") for s in access_scopes ] + access_scopes = [s.replace("$group", f"{group.name}") for s in access_scopes] other_user = create_user_with_scopes(*access_scopes, name="burgess") cookies = await app.login_user('burgess') From 3270bc76afe4ae36b916c774910da8851dc644b7 Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 26 May 2021 07:18:18 +0000 Subject: [PATCH 43/45] readme typo Co-authored-by: Ivana --- examples/service-announcement/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/service-announcement/README.md b/examples/service-announcement/README.md index 4be75b57..0582facc 100644 --- a/examples/service-announcement/README.md +++ b/examples/service-announcement/README.md @@ -57,7 +57,7 @@ The time the announcement was posted is recorded in the `timestamp` field and the user who posted the announcement is recorded in the `user` field. To clear the announcement text, send a DELETE request. -This has the same permissionOnly admin users can do this. +This has the same permission requirement. $ curl -X DELETE -H "Authorization: token " \ https://.../services/announcement/ From 4a0fed1a5b7c8640e83e6138bcd033409b3e53a9 Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 26 May 2021 09:19:02 +0200 Subject: [PATCH 44/45] address review in services doc --- docs/source/reference/services.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/source/reference/services.md b/docs/source/reference/services.md index ab7d8da0..a78e8707 100644 --- a/docs/source/reference/services.md +++ b/docs/source/reference/services.md @@ -90,8 +90,9 @@ c.JupyterHub.load_roles = [ { "name": "idle-culler", "scopes": [ - "users:servers", - # also 'admin:users' if culling idle users as well + "read:users:activity", # read user last_activity + "users:servers", # start and stop servers + # 'admin:users' # needed if culling idle users as well ] } @@ -283,8 +284,8 @@ If you don't want to use the reference implementation (e.g. you find the implementation a poor fit for your Flask app), you can implement authentication via the Hub yourself. JupyterHub is a standard OAuth2 provider, -so you can any OAuth 2 implementation appropriate for -See the [FastAPI example][] for an example of using JupyterHub as an OAuth provider with fastapi, +so you can use any OAuth 2 client implementation appropriate for your toolkit. +See the [FastAPI example][] for an example of using JupyterHub as an OAuth provider with [FastAPI][], without using any code imported from JupyterHub. On completion of OAuth, you will have an access token for JupyterHub, @@ -356,4 +357,5 @@ section on securing the notebook viewer. [hubauthenticated]: ../api/services.auth.html#jupyterhub.services.auth.HubAuthenticated [nbviewer example]: https://github.com/jupyter/nbviewer#securing-the-notebook-viewer [fastapi example]: https://github.com/jupyterhub/jupyterhub/tree/HEAD/examples/service-fastapi +[fastapi]: https://fastapi.tiangolo.com [jupyterhub_idle_culler]: https://github.com/jupyterhub/jupyterhub-idle-culler From 1dab57af6fc492b24b85052827ca88169fd9fdf8 Mon Sep 17 00:00:00 2001 From: Min RK Date: Tue, 8 Jun 2021 09:48:11 +0200 Subject: [PATCH 45/45] remove invalid access scope test --- jupyterhub/tests/test_singleuser.py | 1 - 1 file changed, 1 deletion(-) diff --git a/jupyterhub/tests/test_singleuser.py b/jupyterhub/tests/test_singleuser.py index d2fe3657..bf3b0905 100644 --- a/jupyterhub/tests/test_singleuser.py +++ b/jupyterhub/tests/test_singleuser.py @@ -31,7 +31,6 @@ from .utils import AsyncSession (["access:users:servers!user=$other"], "named", False), (["access:services"], "", False), (["self"], "named", False), - (["admin"], "", False), ([], "", False), ], )