diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 1cddeba6..70704a2b 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -18,10 +18,10 @@ jobs: strategy: matrix: os: ['ubuntu-latest'] - python: ['3.7', '3.8', '3.9'] + python: ['3.8', '3.9'] include: - os: 'macos-latest' - python: '3.7' + python: '3.8' steps: - name: Checkout uses: actions/checkout@v3 diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index b22e25fb..7df5d2d5 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -13,10 +13,10 @@ jobs: strategy: matrix: os: ['ubuntu-latest'] - python: ['3.7', '3.8', '3.9'] + python: ['3.8', '3.9'] include: - os: 'macos-latest' - python: '3.7' + python: '3.8' env: PYTEST_ADDOPTS: --cov --color=yes diff --git a/CHANGES.md b/CHANGES.md index 52019d35..58414ad4 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -11,6 +11,16 @@ creating a new release entry be sure to copy & paste the span tag with the updated. Only the first match gets replaced, so it's fine to leave the old ones in. --> +------------------------------------------------------------------------------- +## __cylc-uiserver-1.4.0 (Awaiting Release)__ + +### Enhancements + +[#450](https://github.com/cylc/cylc-uiserver/pull/450) - +Upgraded to Jupyter Server 2.7+ and Jupyter Hub 4.0+. Note cylc-uiserver +1.3 remains supported and compatible with cylc-flow 8.2 for those not ready +to make the jump just yet. + ------------------------------------------------------------------------------- ## __cylc-uiserver-1.3.0 (Released 2023-07-21)__ diff --git a/cylc/uiserver/app.py b/cylc/uiserver/app.py index fe363b75..e821a6f2 100644 --- a/cylc/uiserver/app.py +++ b/cylc/uiserver/app.py @@ -152,6 +152,7 @@ class CylcUIServer(ExtensionApp): config_file_paths.insert(0, str(Path(uis_pkg).parent)) # packaged config config_file_paths.reverse() # TODO: Add a link to the access group table mappings in cylc documentation + # https://github.com/cylc/cylc-uiserver/issues/466 AUTH_DESCRIPTION = ''' Authorization can be granted at operation (mutation) level, i.e. specifically grant user access to execute Cylc commands, e.g. @@ -172,7 +173,7 @@ class CylcUIServer(ExtensionApp): applied to all workflows. For more information, including the access group mappings, see - :ref:`Authorization`. + :ref:`cylc.uiserver.multi-user`. ''' site_authorization = Dict( diff --git a/cylc/uiserver/authorise.py b/cylc/uiserver/authorise.py index d3401f84..415913b7 100644 --- a/cylc/uiserver/authorise.py +++ b/cylc/uiserver/authorise.py @@ -15,18 +15,85 @@ from contextlib import suppress from functools import lru_cache -import graphene +from getpass import getuser import grp from typing import List, Dict, Optional, Union, Any, Sequence, Set, Tuple from inspect import iscoroutinefunction import os import re + +import graphene +from jupyter_server.auth import Authorizer from tornado import web from traitlets.config.loader import LazyConfigValue from cylc.uiserver.schema import UISMutations +class CylcAuthorizer(Authorizer): + """Defines a safe default authorization policy for Jupyter Server. + + `Jupyter Server`_ provides an authorisation layer which gives full + permissions to any user who has been granted permission to the Jupyter Hub + ``access:servers`` scope + (see :ref:`JupyterHub scopes reference `). This allows + the execution of arbitrary code under another user account. + + To prevent this you must define an authorisation policy using + :py:attr:`c.ServerApp.authorizer_class + `. + + This class defines a policy which blocks all API calls to another user's + server, apart from calls to Cylc interfaces explicitly defined in the + :ref:`Cylc authorisation configuration `. + + This class is configured as the default authoriser for all Jupyter Server + instances spawned via the ``cylc hubapp`` command. This is the default if + you started `Jupyter Hub`_ using the ``cylc hub`` command. To see where + this default is set, see this file for the appropriate release of + cylc-uiserver: + https://github.com/cylc/cylc-uiserver/blob/master/cylc/uiserver/jupyter_config.py + + If you are launching Jupyter Hub via another command (e.g. ``jupyterhub``) + or are overriding :py:attr:`jupyterhub.app.JupyterHub.spawner_class`, then + you will need to configure a safe authorisation policy e.g: + + .. code-block:: python + + from cylc.uiserver.authorise import CylcAuthorizer + c.ServerApp.authorizer_class = CylcAuthorizer + + .. note:: + + It is possible to provide read-only access to Jupyter Server extensions + such as Jupyter Lab, however, this isn't advisable as Jupyter Lab does + not apply file-system permissions to what another user is allowed to + see. + + If you wish to grant users access to other user's Jupyter Lab servers, + override this configuration with due care over what you choose to + expose. + + """ + + def is_authorized(self, handler, user, action, resource) -> bool: + """Allow a user to access their own server. + + Note that Cylc uses its own authorization system (which is locked-down + by default) and is not affected by this policy. + """ + # the username of the user running this server + # (used for authorzation purposes) + me = getuser() + + if user.username == me: + # give the user full permissions to their own server + return True + + # block access to everyone else + return False + + def constant(func): """Decorator preventing reassignment""" diff --git a/cylc/uiserver/handlers.py b/cylc/uiserver/handlers.py index aecef402..0ec9341e 100644 --- a/cylc/uiserver/handlers.py +++ b/cylc/uiserver/handlers.py @@ -18,13 +18,17 @@ import json import getpass import os -import socket -from typing import TYPE_CHECKING, Callable, Union, Dict +from typing import TYPE_CHECKING, Callable, Dict from graphene_tornado.tornado_graphql_handler import TornadoGraphQLHandler from graphql import get_default_backend from graphql_ws.constants import GRAPHQL_WS from jupyter_server.base.handlers import JupyterHandler +from jupyter_server.auth.identity import ( + User as JPSUser, + IdentityProvider as JPSIdentityProvider, + PasswordIdentityProvider, +) from tornado import web, websocket from tornado.ioloop import IOLoop @@ -61,60 +65,42 @@ def _inner( **kwargs, ): nonlocal fun - user: Union[ - None, # unauthenticated - dict, # hub auth - str, # token auth or anonymous - - ] = handler.get_current_user() - if user is None or user == 'anonymous': - # user is not authenticated - calls should not get this far - # but the extra safety doesn't hurt - # NOTE: Auth tests will hit this line unless mocked authentication - # is provided. - raise web.HTTPError(403, reason='Forbidden') - if not ( - isinstance(user, str) # token authenticated - or ( - isinstance(user, dict) - and _authorise(handler, user['name']) - ) - ): + user: JPSUser = handler.current_user + + if not user or not user.username: + # the user is only truthy if they have authenticated successfully + raise web.HTTPError(403, reason='authorization insufficient') + + if not handler.identity_provider.auth_enabled: + # if authentication is turned off we don't want to work with this raise web.HTTPError(403, reason='authorization insufficient') + + if is_token_authenticated(handler): + # token or password authenticated, the bearer of the token or + # password has full control + pass + + elif not _authorise(handler, user.username): + # other authentication (e.g. JupyterHub auth), check the user has + # read permissions for Cylc + raise web.HTTPError(403, reason='authorization insufficient') + return fun(handler, *args, **kwargs) return _inner -def is_token_authenticated( - handler: JupyterHandler, - user: Union[bytes, dict, str], -) -> bool: - """Returns True if the UIS is running "standalone". +def is_token_authenticated(handler: 'CylcAppHandler') -> bool: + """Returns True if this request is bearer token authenticated. - At present we cannot use handler.is_token_authenticated because it - returns False when the token is cached in a cookie. + E.G. The default single-user token-based authenticated. - https://github.com/jupyter-server/jupyter_server/pull/562 + In these cases the bearer of the token is awarded full privileges. """ - if isinstance(user, bytes): # noqa: SIM114 - # Cookie authentication: - # * The URL token is added to a secure cookie, it can then be - # removed from the URL for subsequent requests, the cookie is - # used in its place. - # * If the token was used token_authenticated is True. - # * If the cookie was used it is False (despite the cookie auth - # being derived from token auth). - # * Due to a bug in jupyter_server the user is returned as bytes - # when cookie auth is used so at present we can use this to - # tell. - # https://github.com/jupyter-server/jupyter_server/pull/562 - # TODO: this hack is obviously not suitable for production! - return True - elif handler.token_authenticated: - # standalone UIS, the bearer of the token is the owner - # (no multi-user functionality so no further auth required) - return True - return False + identity_provider: JPSIdentityProvider = ( + handler.serverapp.identity_provider + ) + return identity_provider.__class__ == PasswordIdentityProvider + # NOTE: not using isinstance to narrow this down to just the one class def _authorise( @@ -136,28 +122,24 @@ def _authorise( return False -def parse_current_user(current_user): - """Standardises and returns the current user.""" - if isinstance(current_user, dict): - # the server is running with authentication services provided - # by a hub - current_user = dict(current_user) # make a copy for safety - return current_user +def get_username(handler: 'CylcAppHandler'): + """Return the username for the authenticated user. + + If the handler is token authenticated, then we return the username of the + account that this server instance is running under. + """ + if is_token_authenticated(handler): + # the bearer of the token has full privileges + return ME else: - # the server is running using a token - # authentication is provided by jupyter server - return { - 'kind': 'user', - 'name': ME, - 'server': socket.gethostname() - } + return handler.current_user.username class CylcAppHandler(JupyterHandler): """Base handler for Cylc endpoints. This handler adds the Cylc authorisation layer which is triggered by - calling CylcAppHandler.get_current_user which is called by + accessing CylcAppHandler.current_user which is called by web.authenticated. When running as a Hub application the make_singleuser_app method patches @@ -171,11 +153,7 @@ class CylcAppHandler(JupyterHandler): def initialize(self, auth): self.auth = auth - - # Without this, there is no xsrf token from the GET which causes a 403 and - # a missing _xsrf argument error on the first POST. - def prepare(self): - _ = self.xsrf_token + super().initialize() @property def hub_users(self): @@ -266,11 +244,11 @@ def set_default_headers(self) -> None: self.set_header("Content-Type", 'application/json') @web.authenticated - @authorised + # @authorised TODO: I can't think why we would want to authorise this def get(self): - user_info = self.get_current_user() - - user_info = parse_current_user(user_info) + user_info = { + 'name': get_username(self) + } # add an entry for the workflow owner # NOTE: when running behind a hub this may be different from the @@ -283,6 +261,7 @@ def get(self): self.auth.get_permitted_operations(user_info['name'])) ] # Pass the gui mode to the ui + # (used for functionality not security) if not os.environ.get("JUPYTERHUB_SINGLEUSER_APP"): user_info['mode'] = 'single user' else: @@ -337,15 +316,9 @@ def context(self): 'graphql_params': self.graphql_params, 'request': self.request, 'resolvers': self.resolvers, - 'current_user': parse_current_user( - self.get_current_user() - ).get('name'), + 'current_user': get_username(self), } - @web.authenticated - def prepare(self): - super().prepare() - @web.authenticated # type: ignore[arg-type] async def execute(self, *args, **kwargs) -> 'ExecutionResult': # Use own backend, and TornadoGraphQLHandler already does validation. @@ -411,9 +384,7 @@ def context(self): return { 'request': self.request, 'resolvers': self.resolvers, - 'current_user': parse_current_user( - self.get_current_user() - ).get('name'), + 'current_user': get_username(self), 'ops_queue': {}, 'sub_statuses': self.sub_statuses } diff --git a/cylc/uiserver/jupyter_config.py b/cylc/uiserver/jupyter_config.py index 1f9eae18..3caf105e 100644 --- a/cylc/uiserver/jupyter_config.py +++ b/cylc/uiserver/jupyter_config.py @@ -15,7 +15,6 @@ # Configuration file for jupyterhub. -import os from pathlib import Path import pkg_resources @@ -23,6 +22,7 @@ __file__ as uis_pkg, getenv) from cylc.uiserver.app import USER_CONF_ROOT +from cylc.uiserver.authorise import CylcAuthorizer # the command the hub should spawn (i.e. the cylc uiserver itself) @@ -94,3 +94,10 @@ } }, } + + +# Define the authorization-policy for Jupyter Server. +# This prevents users being granted full access to extensions such as Jupyter +# Lab as a result of being granted the ``access:servers`` permission in Jupyter +# Hub. +c.ServerApp.authorizer_class = CylcAuthorizer diff --git a/cylc/uiserver/tests/conftest.py b/cylc/uiserver/tests/conftest.py index fc9f7da9..7b3f8692 100644 --- a/cylc/uiserver/tests/conftest.py +++ b/cylc/uiserver/tests/conftest.py @@ -29,6 +29,8 @@ from traitlets.config import Config import zmq +from jupyter_server.auth.identity import User + from cylc.flow.cfgspec.glbl_cfg import glbl_cfg from cylc.flow.id import Tokens from cylc.flow.data_messages_pb2 import ( # type: ignore @@ -200,60 +202,16 @@ def authorisation_false(monkeypatch): @pytest.fixture -def mock_authentication(monkeypatch: pytest.MonkeyPatch): - - def _mock_authentication(user=None, server=None, none=False): - ret = { - 'name': user or getuser(), - 'server': server or gethostname() - } - if none: - ret = None - monkeypatch.setattr( - 'cylc.uiserver.handlers.parse_current_user', - lambda x: { - 'kind': 'user', - 'name': None, - 'server': 'some_server' - } - ) - - def mock_redirect(*args): - # normally tornado would attempt to redirect us to the login - # page - for testing purposes we will skip this and raise - # a 403 with an explanatory reason - raise HTTPError( - 403, - reason='login redirect replaced by 403 for test purposes' - ) - - monkeypatch.setattr( - 'cylc.uiserver.handlers.CylcAppHandler.redirect', - mock_redirect - ) - - monkeypatch.setattr( - 'cylc.uiserver.handlers.CylcAppHandler.get_current_user', - lambda x: ret - ) - monkeypatch.setattr( - 'cylc.uiserver.handlers.CylcAppHandler.get_login_url', - lambda x: "http://cylc" - ) - - _mock_authentication() - - return _mock_authentication - - -@pytest.fixture -def mock_authentication_yossarian(mock_authentication): - mock_authentication(user='yossarian') - - -@pytest.fixture -def mock_authentication_none(mock_authentication): - mock_authentication(none=True) +def mock_authentication_yossarian(monkeypatch): + user = User('yossarian') + monkeypatch.setattr( + 'cylc.uiserver.handlers.CylcAppHandler.current_user', + user, + ) + monkeypatch.setattr( + 'cylc.uiserver.handlers.is_token_authenticated', + lambda x: True, + ) @pytest.fixture diff --git a/cylc/uiserver/tests/test_auth.py b/cylc/uiserver/tests/test_auth.py index 5cb9e3ea..b9bb4e1f 100644 --- a/cylc/uiserver/tests/test_auth.py +++ b/cylc/uiserver/tests/test_auth.py @@ -22,7 +22,7 @@ @pytest.mark.integration -@pytest.mark.usefixtures("mock_authentication") +@pytest.mark.usefixtures("mock_authentication_yossarian") async def test_cylc_handler(patch_conf_files, jp_fetch): """The Cylc endpoints have been added and work.""" resp = await jp_fetch( @@ -32,7 +32,7 @@ async def test_cylc_handler(patch_conf_files, jp_fetch): @pytest.mark.integration -@pytest.mark.usefixtures("mock_authentication") +@pytest.mark.usefixtures("mock_authentication_yossarian") @pytest.mark.parametrize( 'endpoint,code,message,body', [ @@ -70,45 +70,6 @@ async def test_authorised_and_authenticated( await _test(jp_fetch, endpoint, code, message, body) -@pytest.mark.integration -@pytest.mark.usefixtures("mock_authentication_none") -@pytest.mark.parametrize( - 'endpoint,code,message,body', - [ - pytest.param( - ('cylc', 'graphql'), - 403, - 'login redirect replaced by 403 for test purposes', - None, - id='cylc/graphql', - ), - pytest.param( - ('cylc', 'subscriptions'), - 403, - 'Forbidden', - None, - id='cylc/subscriptions', - ), - pytest.param( - ('cylc', 'userprofile'), - 403, - 'login redirect replaced by 403 for test purposes', - None, - id='cylc/userprofile', - ) - ] -) -async def test_unauthenticated( - patch_conf_files, - jp_fetch, - endpoint, - code, - message, - body -): - await _test(jp_fetch, endpoint, code, message, body) - - @pytest.mark.integration @pytest.mark.usefixtures("mock_authentication_yossarian") @pytest.mark.parametrize( @@ -124,19 +85,13 @@ async def test_unauthenticated( ), pytest.param( # should pass through authentication but fail as there is no query + # (authorisation is performed in the grahpql layer) ('cylc', 'subscriptions'), 400, 'Bad Request', None, id='cylc/subscriptions', ), - pytest.param( - ('cylc', 'userprofile'), - 403, - 'authorization insufficient', - None, - id='cylc/userprofile', - ) ] ) async def test_unauthorised( diff --git a/cylc/uiserver/tests/test_handlers.py b/cylc/uiserver/tests/test_handlers.py index cf79dd05..5f10676d 100644 --- a/cylc/uiserver/tests/test_handlers.py +++ b/cylc/uiserver/tests/test_handlers.py @@ -56,15 +56,15 @@ def _create_handler(self, logged_in=True): handler.get _current_user = lambda: {'name': getuser()} else: - handler.get_current_user = lambda: None + handler.current_user = None return handler - @pytest.mark.usefixtures("mock_authentication") + @pytest.mark.usefixtures("mock_authentication_yossarian") def test_websockets_subprotocol(self): handler = self._create_handler() assert handler.select_subprotocol(subprotocols=[]) == GRAPHQL_WS - @pytest.mark.usefixtures("mock_authentication") + @pytest.mark.usefixtures("mock_authentication_yossarian") def test_websockets_check_origin_accepts_same_origin(self): """A request that includes the Host header must use the same value as the server host, or an error is raised. @@ -79,7 +79,7 @@ def test_websockets_check_origin_accepts_same_origin(self): handler.request.headers['Host'] = host_header assert handler.check_origin(f'http://{host_header}') - @pytest.mark.usefixtures("mock_authentication") + @pytest.mark.usefixtures("mock_authentication_yossarian") def test_websockets_check_origin_rejects_different_origin(self): """A request from a different Host MUST be blocked to prevent CORS attacks. @@ -93,13 +93,13 @@ def test_websockets_check_origin_rejects_different_origin(self): handler.request.headers['Origin'] = 'ui.notcylc' assert not handler.check_origin('http://ui.notcylc') - @pytest.mark.usefixtures("mock_authentication") + @pytest.mark.usefixtures("mock_authentication_yossarian") def test_websockets_context(self): handler = self._create_handler() assert handler.request == handler.context['request'] assert 'resolvers' in handler.context - @pytest.mark.usefixtures("mock_authentication") + @pytest.mark.usefixtures("mock_authentication_yossarian") def test_websockets_queue(self): handler = self._create_handler() message = '{"message":"a message"}' @@ -111,7 +111,7 @@ def test_websockets_queue(self): get_async_test_timeout()) assert handler.queue.empty() - @pytest.mark.usefixtures("mock_authentication") + @pytest.mark.usefixtures("mock_authentication_yossarian") def test_assert_callback_handler_gets_called(self): handler = self._create_handler() handler.subscription_server = MagicMock() diff --git a/setup.cfg b/setup.cfg index eee4d1f0..92505753 100644 --- a/setup.cfg +++ b/setup.cfg @@ -33,7 +33,6 @@ classifiers = License :: OSI Approved :: GNU General Public License v3 (GPLv3) Operating System :: POSIX Programming Language :: Python - Programming Language :: Python :: 3.7 Programming Language :: Python :: 3.8 Programming Language :: Python :: 3.9 Programming Language :: Python :: 3 :: Only @@ -41,7 +40,7 @@ classifiers = [options] packages = find_namespace: -python_requires = >=3.7 +python_requires = >=3.8 include_package_data = True install_requires = # NB: We have cylc-flow at the top to force it to install its transitive @@ -55,7 +54,7 @@ install_requires = graphene graphene-tornado==2.6.* graphql-ws==0.4.4 - jupyter_server>=1.10.2, <2.0 + jupyter_server>=2.7 requests tornado>=6.1.0 # matches jupyter_server value traitlets>=5.2.1 # required for logging_config (5.2.0 had bugs) @@ -84,7 +83,7 @@ cylc.command = [options.extras_require] hub = - jupyterhub>=1.4.0 + jupyterhub>=4 tests = coverage>=5.0.0 flake8-broken-line>=0.3.0 @@ -95,6 +94,7 @@ tests = flake8-mutable>=1.2.0 flake8-simplify>=0.14.0 flake8>=3.0.0 + jupyter_server[test] mypy>=0.900 pytest-cov>=2.8.0 pytest-tornasync>=0.5.0