From 8214e84373bec58f30f6cb15ede2a407a1f80d4b Mon Sep 17 00:00:00 2001 From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> Date: Wed, 19 Jun 2024 13:30:00 +0100 Subject: [PATCH 01/14] Bump dev version [skip ci] --- cylc/uiserver/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cylc/uiserver/__init__.py b/cylc/uiserver/__init__.py index d84ff5ee..3e32c294 100644 --- a/cylc/uiserver/__init__.py +++ b/cylc/uiserver/__init__.py @@ -13,7 +13,7 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -__version__ = "1.5.1.dev" +__version__ = "1.6.0.dev" import os from typing import Dict From f6a05fc5212d7f3bdbd6d525f5660135fa198f75 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Wed, 19 Jun 2024 14:53:06 +0100 Subject: [PATCH 02/14] setup: bump cylc-flow version (#605) --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index c8d7cf5d..d5f3ccbc 100644 --- a/setup.cfg +++ b/setup.cfg @@ -49,7 +49,7 @@ install_requires = # bleeding-edge version. # NB: no graphene version specified; we only make light use of it in our # own code, so graphene-tornado's transitive version should do. - cylc-flow==8.3.* + cylc-flow==8.4.* ansimarkup>=1.0.0 graphene graphene-tornado==2.6.* From 8c60aa3b752dee4bf556d7f88457b37bddc7d380 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Thu, 4 Jul 2024 10:03:54 +0100 Subject: [PATCH 03/14] tidy: cylc/uiserver/authorise.py * Blacken code. * Put permissions in order. * Add comments. * Use Google-style docstrings. --- cylc/uiserver/authorise.py | 233 ++++++++++++++++---------- cylc/uiserver/tests/test_authorise.py | 82 ++++----- 2 files changed, 174 insertions(+), 141 deletions(-) diff --git a/cylc/uiserver/authorise.py b/cylc/uiserver/authorise.py index 858ca369..aec858e2 100644 --- a/cylc/uiserver/authorise.py +++ b/cylc/uiserver/authorise.py @@ -17,10 +17,10 @@ from functools import lru_cache 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 +from typing import List, Optional, Union, Any, Sequence, Set, Tuple import graphene from jupyter_server.auth import Authorizer @@ -118,13 +118,16 @@ def fget(): class Authorization: - """Authorization Information Class - One instance for the life of the UI Server. If authorization settings - change they will need to re-start the UI Server. + """Authorization configuration object. + + One instance of this class lives for the life of the UI Server. + + If authorization settings change the UI Server will need to be re-started + to pick them up. + Authorization has access groups: `READ`, `CONTROL`, `ALL` - along with their negations, `!READ`, `!CONTROL` and `!ALL` which indicate removal of the permission groups. - """ # config literals @@ -147,7 +150,6 @@ class Authorization: READ_OPERATION = "read" # Access group identifiers (used in config) - READ = "READ" CONTROL = "CONTROL" ALL = "ALL" @@ -156,23 +158,10 @@ class Authorization: NOT_ALL = "!ALL" # Access Groups - READ_OPS = {READ_OPERATION} ASYNC_OPS = {"query", "mutation"} READ_AUTH_OPS = {"query", "subscription"} - @staticmethod - @constant - def ALL_OPS() -> List[str]: - """ALL OPS constant, returns list of all mutations.""" - return get_list_of_mutations() - - @staticmethod - @constant - def CONTROL_OPS() -> List[str]: - """CONTROL OPS constant, returns list of all control mutations.""" - return get_list_of_mutations(control=True) - def __init__(self, owner, owner_auth_conf, site_auth_conf, log) -> None: self.owner = owner self.log = log @@ -189,6 +178,18 @@ def __init__(self, owner, owner_auth_conf, site_auth_conf, log) -> None: self._get_permitted_operations ) + @staticmethod + @constant + def ALL_OPS() -> List[str]: + """ALL OPS constant, returns list of all mutations.""" + return get_list_of_mutations() + + @staticmethod + @constant + def CONTROL_OPS() -> List[str]: + """CONTROL OPS constant, returns list of all control mutations.""" + return get_list_of_mutations(control=True) + @staticmethod def expand_and_process_access_groups(permission_set: set) -> set: """Process a permission set. @@ -200,36 +201,44 @@ def expand_and_process_access_groups(permission_set: set) -> set: permission_set: set of permissions Returns: - permission_set: processed permission set. + processed permission set. + """ + # Expand permission groups + # E.G. ALL -> ["read", "trigger", "broadcast", ...] for action_group, expansion in { + Authorization.READ: Authorization.READ_OPS, Authorization.CONTROL: Authorization.CONTROL_OPS.fget(), Authorization.ALL: Authorization.ALL_OPS.fget(), - Authorization.READ: Authorization.READ_OPS, }.items(): if action_group in permission_set: permission_set.remove(action_group) permission_set.update(expansion) - # Expand negated permissions + + # Expand negated permission groups + # E.G. !CONTROL -> ["!trigger", "!stop", "!pause", ...] for action_group, expansion in { - Authorization.NOT_CONTROL: [ - f"!{x}" for x in Authorization.CONTROL_OPS.fget()], - Authorization.NOT_ALL: [ - f"!{x}" for x in Authorization.ALL_OPS.fget()], - Authorization.NOT_READ: [ - f"!{x}" for x in Authorization.READ_OPS]}.items(): + Authorization.NOT_READ: [f"!{x}" for x in Authorization.READ_OPS], + Authorization.NOT_CONTROL: [ + f"!{x}" for x in Authorization.CONTROL_OPS.fget() + ], + Authorization.NOT_ALL: [ + f"!{x}" for x in Authorization.ALL_OPS.fget() + ], + }.items(): if action_group in permission_set: permission_set.remove(action_group) permission_set.update(expansion) + # Remove negated permissions remove = set() - for perm in permission_set: if perm.startswith("!"): remove.add(perm.lstrip("!")) remove.add(perm) permission_set.difference_update(remove) permission_set.discard("") + return permission_set @staticmethod @@ -241,29 +250,31 @@ def set_auth_conf(auth_conf: Union[LazyConfigValue, dict]) -> dict: Returns: Valid configuration dictionary + """ if isinstance(auth_conf, LazyConfigValue): return auth_conf.to_dict() return auth_conf def get_owner_site_limits_for_access_user( - self, access_user: Dict[str, Union[str, Sequence[Any]]] + self, access_user_name: str, access_user_groups: Sequence[Any] ) -> Set: """Returns limits owner can give to given access_user Args: - access_user: Dictionary containing info about access user and their - membership of system groups. + access_user_name: The username of the authenticated user. + access_user_groups: All groups the authenticated user belongs to. Returns: Set of limits that the uiserver owner is allowed to give away for given access user. + """ limits: Set[str] = set() if not self.owner_dict: return limits - items_to_check = ["*", access_user["access_username"]] - items_to_check.extend(access_user["access_user_groups"]) + items_to_check = ["*", access_user_name] + items_to_check.extend(access_user_groups) for item in items_to_check: permission: Union[str, List] = "" default = "" @@ -271,7 +282,8 @@ def get_owner_site_limits_for_access_user( default = self.owner_dict[item].get(Authorization.DEFAULT, "") with suppress(KeyError): permission = self.owner_dict[item].get( - Authorization.LIMIT, default) + Authorization.LIMIT, default + ) if permission == []: raise_auth_config_exception("site") if isinstance(permission, str): @@ -282,17 +294,18 @@ def get_owner_site_limits_for_access_user( return limits def get_access_user_permissions_from_owner_conf( - self, access_user: Dict[str, Union[str, Sequence[Any]]] + self, access_user_name: str, access_user_groups: Sequence[Any] ) -> set: """ Returns set of operations specific to access user from owner user conf. Args: - access_user: Dictionary containing info about access user and their - membership of system groups. Defaults to None. + access_user_name: The username of the authenticated user. + access_user_groups: All groups the authenticated user belongs to. + """ - items_to_check = ["*", access_user["access_username"]] - items_to_check.extend(access_user["access_user_groups"]) + items_to_check = ["*", access_user_name] + items_to_check.extend(access_user_groups) allowed_operations = set() for item in items_to_check: permission = self.owner_auth_conf.get(item, "") @@ -310,7 +323,8 @@ def get_access_user_permissions_from_owner_conf( def _get_permitted_operations(self, access_user: str): """Return permitted operations for given access_user. - Cached for efficiency. + This method is cached for efficiency. + Checks: - site config to ensure owner is permitted to give away permissions - user config for authorised operations related to access_user and @@ -322,39 +336,49 @@ def _get_permitted_operations(self, access_user: str): Returns: Set of operations permitted by given access user for this UI Server + """ - # For use in the ui, owner permissions (ALL operations) are set + # users have full access to their own server (ALL) if access_user == self.owner: return set(Authorization.ALL_OPS.fget()) - # Otherwise process permissions for (non-uiserver owner) access_user - access_user_dict = { - "access_username": access_user, - "access_user_groups": self._get_groups(access_user), - } + # all groups the authenticated user belongs to + access_user_groups = self._get_groups(access_user) + + # the maximum permissions the site permits the user to grant limits_owner_can_give = self.get_owner_site_limits_for_access_user( - access_user=access_user_dict) + access_user, access_user_groups + ) + + # the permissions the user wishes to grant user_conf_permitted_ops = ( self.get_access_user_permissions_from_owner_conf( - access_user=access_user_dict) + access_user, access_user_groups + ) ) - # If not explicit permissions for access user in owner conf then revert - # to site defaults + if len(user_conf_permitted_ops) == 0: + # the user has not specified the permissions they wish to grant + # -> fallback to the site defaults user_conf_permitted_ops = ( self.return_site_auth_defaults_for_access_user( - access_user=access_user_dict + access_user, access_user_groups ) ) + + # expand permission groups and remove negated permissions user_conf_permitted_ops = self.expand_and_process_access_groups( user_conf_permitted_ops ) limits_owner_can_give = self.expand_and_process_access_groups( limits_owner_can_give ) + + # subtract permissions that the site does not permit to be granted allowed_operations = limits_owner_can_give.intersection( user_conf_permitted_ops ) + self.log.info( f"User {access_user} authorized permissions: " f"{sorted(allowed_operations)}" @@ -371,6 +395,7 @@ def is_permitted(self, access_user: str, operation: str) -> bool: Returns: True if access_user permitted to action operation, otherwise, False. + """ if access_user == self.owner_user_info["user"]: return True @@ -386,8 +411,8 @@ def is_permitted(self, access_user: str, operation: str) -> bool: def build_owner_site_auth_conf(self): """Build UI Server owner permissions dictionary. - Creates a reduced site auth dictionary for the ui-server owner. + Creates a reduced site auth dictionary for the ui-server owner. """ owner_dict = {} items_to_check = ["*", self.owner_user_info["user"]] @@ -402,13 +427,17 @@ def build_owner_site_auth_conf(self): if existing_user_conf: # process limits and defaults and update dictionary existing_default = existing_user_conf.get( - Authorization.DEFAULT, '') + Authorization.DEFAULT, '' + ) existing_limit = existing_user_conf.get( - Authorization.LIMIT, existing_default) + Authorization.LIMIT, existing_default + ) new_default = acc_user_perms.get( - Authorization.DEFAULT, '') + Authorization.DEFAULT, '' + ) new_limit = acc_user_perms.get( - Authorization.LIMIT, new_default) + Authorization.LIMIT, new_default + ) set_defs = set() for conf in [existing_default, new_default]: if isinstance(conf, list): @@ -422,35 +451,35 @@ def build_owner_site_auth_conf(self): else: set_lims.add(conf) # update and continue - owner_dict[ - acc_user_conf][Authorization.LIMIT] = list( - set_lims) - owner_dict[ - acc_user_conf][Authorization.DEFAULT] = list( - set_defs) + owner_dict[acc_user_conf][Authorization.LIMIT] = list( + set_lims + ) + owner_dict[acc_user_conf][Authorization.DEFAULT] = ( + list(set_defs) + ) continue owner_dict.update(access_user_dict) # Now we have a reduced site auth dictionary for the current owner return owner_dict def return_site_auth_defaults_for_access_user( - self, access_user: Dict[str, Union[str, Sequence[Any]]] + self, access_user_name: str, access_user_groups: Sequence[Any] ) -> Set: """Return site authorization defaults for given access user. + Args: - access_user: access_user dictionary, in the form - {'access_username': username - 'access_user_group: [group1, group2,...]' - } + access_user_name: The username of the authenticated user. + access_user_groups: All groups the authenticated user belongs to. + Returns: - Set of default operations permitted + The set of default operations permitted. """ defaults: Set[str] = set() if not self.owner_dict: return defaults - items_to_check = ["*", access_user["access_username"]] - items_to_check.extend(access_user["access_user_groups"]) + items_to_check = ["*", access_user_name] + items_to_check.extend(access_user_groups) for item in items_to_check: permission: Union[str, List] = "" with suppress(KeyError): @@ -489,6 +518,7 @@ class AuthorizationMiddleware: Raises: web.HTTPError: Unauthorized requests. + """ auth = None @@ -502,8 +532,10 @@ def resolve(self, next_, root, info, **args): # It shouldn't get here but worth checking for zero trust if not op_name: self.auth_failed( - current_user, op_name, http_code=400, - msg="Operation not in schema." + current_user, + op_name, + http_code=400, + msg="Operation not in schema.", ) try: authorised = self.auth.is_permitted(current_user, op_name) @@ -512,15 +544,22 @@ def resolve(self, next_, root, info, **args): authorised = False if not authorised: self.auth_failed(current_user, op_name, http_code=403) - if (info.operation.operation in Authorization.ASYNC_OPS - or iscoroutinefunction(next_)): + if ( + info.operation.operation in Authorization.ASYNC_OPS + or iscoroutinefunction(next_) + ): return self.async_resolve(next_, root, info, **args) return next_(root, info, **args) - def auth_failed(self, current_user: str, op_name: str, - http_code: int, message: Optional[str] = None): - """ - Raise authorization error + def auth_failed( + self, + current_user: str, + op_name: str, + http_code: int, + message: Optional[str] = None, + ): + """Raise an authorization error. + Args: current_user: username accessing operation op_name: operation name @@ -529,31 +568,38 @@ def auth_failed(self, current_user: str, op_name: str, Raises: web.HTTPError + """ - log_message = (f"Authorization failed for {current_user}" - f":requested to {op_name}.") + log_message = ( + f"Authorization failed for {current_user}" + f":requested to {op_name}." + ) if message: log_message = log_message + " " + message raise web.HTTPError(http_code, reason=message) def get_op_name(self, field_name: str, operation: str) -> Optional[str]: - """ - Returns operation name required for authorization. + """Returns the operation name required for authorization. + Converts queries and subscriptions to read operations. + Args: field_name: Field name e.g. play operation: operation type Returns: - operation name + The operation name. + """ if operation in Authorization.READ_AUTH_OPS: return Authorization.READ_OPERATION else: # Check it is a mutation in our schema - if self.auth and re.sub( - r'(? Tuple[List[str], List[str]]: - """Return list of system groups for given user. + """Return a list of system groups for given user. Uses ``os.getgrouplist`` and ``os.NGROUPS_MAX`` to get system groups for a given user. ``grp.getgrgid`` then parses these to return a list of group @@ -573,7 +619,8 @@ def get_groups(username: str) -> Tuple[List[str], List[str]]: username: username used to check system groups. Returns: - list: system groups for username given + System groups for username given + """ groupmax = os.NGROUPS_MAX # type: ignore group_ids = os.getgrouplist(username, groupmax) @@ -589,8 +636,8 @@ def parse_group_ids(group_ids: List) -> Tuple[List[str], List[str]]: group_ids: List of users groups, in number format Returns: - List: List of users groups, in id format with group identifier - prepended. + List of users groups, in id format with group identifier prepended. + """ group_list = [] bad_group_list = [] @@ -609,7 +656,8 @@ def parse_group_ids(group_ids: List) -> Tuple[List[str], List[str]]: def get_list_of_mutations(control: bool = False) -> List[str]: """Gets list of mutations""" list_of_mutations = [ - attr for attr in dir(UISMutations) + attr + for attr in dir(UISMutations) if isinstance(getattr(UISMutations, attr), graphene.Field) ] if control: @@ -626,6 +674,7 @@ def raise_auth_config_exception(config_type: str): Args: config_type: Either site or user. + """ raise Exception( f'Error in {config_type} config: ' diff --git a/cylc/uiserver/tests/test_authorise.py b/cylc/uiserver/tests/test_authorise.py index 7122623d..df37331c 100644 --- a/cylc/uiserver/tests/test_authorise.py +++ b/cylc/uiserver/tests/test_authorise.py @@ -23,7 +23,7 @@ Authorization, AuthorizationMiddleware, get_list_of_mutations, - parse_group_ids + parse_group_ids, ) log = ExtensionApp().log @@ -86,9 +86,14 @@ }, }, "server_owner_1": { - "*": {"default": ["READ", "message"], - "limit": ["READ", "CONTROL"]}, - "user1": {"default": ["READ", "play", "pause"], "limit": ["ALL"]}, + "*": { + "default": ["READ", "message"], + "limit": ["READ", "CONTROL"], + }, + "user1": { + "default": ["READ", "play", "pause"], + "limit": ["ALL"], + }, }, "server_owner_2": { "user2": {"limit": "ALL"}, @@ -180,7 +185,7 @@ ["group:group2"], set(), id="owner only in group and *", - ) + ), ], ) @patch("cylc.uiserver.authorise.get_groups") @@ -193,9 +198,7 @@ def test_get_permitted_operations( user_groups, ): mocked_get_groups.side_effect = [(owner_groups, []), (user_groups, [])] - auth_obj = Authorization( - owner_name, FAKE_USER_CONF, FAKE_SITE_CONF, log - ) + auth_obj = Authorization(owner_name, FAKE_USER_CONF, FAKE_SITE_CONF, log) actual_operations = auth_obj.get_permitted_operations( access_user=user_name ) @@ -203,14 +206,13 @@ def test_get_permitted_operations( @pytest.mark.parametrize( - "expected_operations, access_user_dict, owner_auth_conf,", + 'expected_operations, access_user_name,' + ' access_user_groups, owner_auth_conf,', [ pytest.param( {"!kill", "READ", "kill", "!stop", "pause", "play"}, - { - "access_username": "access_user_1", - "access_user_groups": ["group:group1", "group:group2"], - }, + "access_user_1", + ["group:group1", "group:group2"], { "*": ["READ", "!kill"], "access_user_1": ["READ", "pause", "kill", "play", "!stop"], @@ -219,10 +221,8 @@ def test_get_permitted_operations( ), pytest.param( {"READ"}, - { - "access_username": "access_user_2", - "access_user_groups": ["group:group1", "group:group2"], - }, + "access_user_2", + ["group:group1", "group:group2"], { "*": ["READ"], "access_user_1": ["READ", "pause", "kill", "play", "!stop"], @@ -231,10 +231,8 @@ def test_get_permitted_operations( ), pytest.param( {"pause", "kill", "!stop", "READ", "CONTROL"}, - { - "access_username": "access_user_1", - "access_user_groups": ["group:group1", "group:group2"], - }, + "access_user_1", + ["group:group1", "group:group2"], { "*": ["READ"], "access_user_1": ["READ", "pause", "kill", "!stop"], @@ -246,7 +244,11 @@ def test_get_permitted_operations( ) @patch("cylc.uiserver.authorise.get_groups") def test_get_access_user_permissions_from_owner_conf( - mocked_get_groups, expected_operations, access_user_dict, owner_auth_conf + mocked_get_groups, + expected_operations, + access_user_name, + access_user_groups, + owner_auth_conf, ): """Test the un-processed permissions of owner conf.""" mocked_get_groups.return_value = (["group:blah"], []) @@ -254,7 +256,7 @@ def test_get_access_user_permissions_from_owner_conf( "some_user", owner_auth_conf, {"fake": "config"}, log ) permitted_operations = authobj.get_access_user_permissions_from_owner_conf( - access_user_dict + access_user_name, access_user_groups ) assert permitted_operations == expected_operations @@ -284,7 +286,7 @@ def test_expand_and_process_access_groups(permission_set, expected): "some_user", {"fake": "config"}, {"fake": "config"}, - log + log, ) actual = authobj.expand_and_process_access_groups(permission_set) assert actual == expected @@ -321,8 +323,7 @@ def test_expand_and_process_access_groups(permission_set, expected): ) def test_get_op_name(mut_field_name, operation, expected_op_name): mock_authobj = Authorization( - "some_user", {"fake": "config"}, - {"fake": "config"}, log + "some_user", {"fake": "config"}, {"fake": "config"}, log ) auth_middleware = AuthorizationMiddleware auth_middleware.auth = mock_authobj @@ -336,11 +337,7 @@ def test_get_op_name(mut_field_name, operation, expected_op_name): "owner_name, user_name, get_permitted_operations_is_called, expected", [ pytest.param( - "mel", - "mel", - False, - True, - id="Owner user always permitted" + "mel", "mel", False, True, id="Owner user always permitted" ), pytest.param( "mel", @@ -383,17 +380,9 @@ def test_get_list_of_mutations(control, expected): assert set(actual) == set(expected) -@pytest.mark.parametrize( - 'input_', - ( - [123], - [123, 456], - [100, 123] - ) -) +@pytest.mark.parametrize('input_', ([123], [123, 456], [100, 123])) def test_parse_group_ids(monkeypatch, input_): - """Returns a list of group ids or groups where ID's haven't worked - """ + """Returns a list of group ids or groups where ID's haven't worked""" mock_grid_db = { 123: 'foo', 456: 'bar', @@ -403,11 +392,6 @@ def test_parse_group_ids(monkeypatch, input_): ) result = parse_group_ids(input_) assert result == ( - [ - f'group:{mock_grid_db[i]}' - for i in input_ if i in mock_grid_db - ], - [ - i for i in input_ if i not in mock_grid_db - ], - ) + [f'group:{mock_grid_db[i]}' for i in input_ if i in mock_grid_db], + [i for i in input_ if i not in mock_grid_db], + ) From 697f340df275e912faef539b87c682d5c54a55ea Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Thu, 4 Jul 2024 11:18:07 +0100 Subject: [PATCH 04/14] auth: remove constant decorator * This was doing what @property does by itself. * Because attrs were being accessed via the class rather than the instance, it wasn't offering any safety. * Switched to @property and accessing it via the instance. --- cylc/uiserver/authorise.py | 37 +++++++++++-------------------------- 1 file changed, 11 insertions(+), 26 deletions(-) diff --git a/cylc/uiserver/authorise.py b/cylc/uiserver/authorise.py index aec858e2..901d90f6 100644 --- a/cylc/uiserver/authorise.py +++ b/cylc/uiserver/authorise.py @@ -105,18 +105,6 @@ def is_authorized(self, handler, user, action, resource) -> bool: return False -def constant(func): - """Decorator preventing reassignment""" - - def fset(self, value): - raise TypeError - - def fget(): - return func() - - return property(fget, fset) - - class Authorization: """Authorization configuration object. @@ -178,20 +166,17 @@ def __init__(self, owner, owner_auth_conf, site_auth_conf, log) -> None: self._get_permitted_operations ) - @staticmethod - @constant - def ALL_OPS() -> List[str]: + @property + def ALL_OPS(self) -> List[str]: """ALL OPS constant, returns list of all mutations.""" return get_list_of_mutations() - @staticmethod - @constant - def CONTROL_OPS() -> List[str]: + @property + def CONTROL_OPS(self) -> List[str]: """CONTROL OPS constant, returns list of all control mutations.""" return get_list_of_mutations(control=True) - @staticmethod - def expand_and_process_access_groups(permission_set: set) -> set: + def expand_and_process_access_groups(self, permission_set: set) -> set: """Process a permission set. Takes a permission set, e.g. limits, defaults. @@ -208,8 +193,8 @@ def expand_and_process_access_groups(permission_set: set) -> set: # E.G. ALL -> ["read", "trigger", "broadcast", ...] for action_group, expansion in { Authorization.READ: Authorization.READ_OPS, - Authorization.CONTROL: Authorization.CONTROL_OPS.fget(), - Authorization.ALL: Authorization.ALL_OPS.fget(), + Authorization.CONTROL: self.CONTROL_OPS, + Authorization.ALL: self.ALL_OPS, }.items(): if action_group in permission_set: permission_set.remove(action_group) @@ -220,10 +205,10 @@ def expand_and_process_access_groups(permission_set: set) -> set: for action_group, expansion in { Authorization.NOT_READ: [f"!{x}" for x in Authorization.READ_OPS], Authorization.NOT_CONTROL: [ - f"!{x}" for x in Authorization.CONTROL_OPS.fget() + f"!{x}" for x in self.CONTROL_OPS ], Authorization.NOT_ALL: [ - f"!{x}" for x in Authorization.ALL_OPS.fget() + f"!{x}" for x in self.ALL_OPS ], }.items(): if action_group in permission_set: @@ -340,7 +325,7 @@ def _get_permitted_operations(self, access_user: str): """ # users have full access to their own server (ALL) if access_user == self.owner: - return set(Authorization.ALL_OPS.fget()) + return set(self.ALL_OPS) # all groups the authenticated user belongs to access_user_groups = self._get_groups(access_user) @@ -598,7 +583,7 @@ def get_op_name(self, field_name: str, operation: str) -> Optional[str]: if ( self.auth and re.sub(r'(? Date: Thu, 4 Jul 2024 11:34:23 +0100 Subject: [PATCH 05/14] auth: fix type hints --- cylc/uiserver/authorise.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cylc/uiserver/authorise.py b/cylc/uiserver/authorise.py index 901d90f6..511ff81d 100644 --- a/cylc/uiserver/authorise.py +++ b/cylc/uiserver/authorise.py @@ -20,7 +20,7 @@ from inspect import iscoroutinefunction import os import re -from typing import List, Optional, Union, Any, Sequence, Set, Tuple +from typing import List, Optional, Union, Set, Tuple import graphene from jupyter_server.auth import Authorizer @@ -242,7 +242,7 @@ def set_auth_conf(auth_conf: Union[LazyConfigValue, dict]) -> dict: return auth_conf def get_owner_site_limits_for_access_user( - self, access_user_name: str, access_user_groups: Sequence[Any] + self, access_user_name: str, access_user_groups: List[str] ) -> Set: """Returns limits owner can give to given access_user @@ -279,7 +279,7 @@ def get_owner_site_limits_for_access_user( return limits def get_access_user_permissions_from_owner_conf( - self, access_user_name: str, access_user_groups: Sequence[Any] + self, access_user_name: str, access_user_groups: List[str] ) -> set: """ Returns set of operations specific to access user from owner user conf. @@ -448,7 +448,7 @@ def build_owner_site_auth_conf(self): return owner_dict def return_site_auth_defaults_for_access_user( - self, access_user_name: str, access_user_groups: Sequence[Any] + self, access_user_name: str, access_user_groups: List[str] ) -> Set: """Return site authorization defaults for given access user. @@ -480,7 +480,7 @@ def return_site_auth_defaults_for_access_user( defaults.discard("") return defaults - def _get_groups(self, user: str) -> List: + def _get_groups(self, user: str) -> List[str]: """Allows get groups to use self.logger if something goes wrong. Added to provide a single interface for get_groups to this class, to From 2ca62e4decfd4b92c2806e75aa8a077bd2438602 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Thu, 4 Jul 2024 13:26:21 +0100 Subject: [PATCH 06/14] auth: convert config to dict on __init__ * Rationalise the configuration object type earlier. * This avoids managing multiple data types throughout the code. --- cylc/uiserver/app.py | 8 ++++---- cylc/uiserver/authorise.py | 37 +++++++++++++++++-------------------- 2 files changed, 21 insertions(+), 24 deletions(-) diff --git a/cylc/uiserver/app.py b/cylc/uiserver/app.py index 8e9fb00c..a10665c1 100644 --- a/cylc/uiserver/app.py +++ b/cylc/uiserver/app.py @@ -542,15 +542,15 @@ def set_sub_server(self): auth=self.authobj, ) - def set_auth(self): + def set_auth(self) -> Authorization: """Create authorization object. One for the lifetime of the UIServer. """ return Authorization( getpass.getuser(), - self.config.CylcUIServer.user_authorization, - self.config.CylcUIServer.site_authorization, - self.log + self.config.CylcUIServer.user_authorization.to_dict(), + self.config.CylcUIServer.site_authorization.to_dict(), + self.log, ) def initialize_templates(self): diff --git a/cylc/uiserver/authorise.py b/cylc/uiserver/authorise.py index 511ff81d..af0b08d3 100644 --- a/cylc/uiserver/authorise.py +++ b/cylc/uiserver/authorise.py @@ -25,7 +25,6 @@ import graphene from jupyter_server.auth import Authorizer from tornado import web -from traitlets.config.loader import LazyConfigValue from cylc.uiserver.schema import UISMutations from cylc.uiserver.utils import is_bearer_token_authenticated @@ -116,6 +115,13 @@ class Authorization: Authorization has access groups: `READ`, `CONTROL`, `ALL` - along with their negations, `!READ`, `!CONTROL` and `!ALL` which indicate removal of the permission groups. + + Args: + owner: The server owner's user name. + owner_auth_conf: The server owner's authorization configuration. + site_auth_conf: The site's authorization configuration. + log: The application logger. + """ # config literals @@ -150,11 +156,17 @@ class Authorization: ASYNC_OPS = {"query", "mutation"} READ_AUTH_OPS = {"query", "subscription"} - def __init__(self, owner, owner_auth_conf, site_auth_conf, log) -> None: - self.owner = owner + def __init__( + self, + owner: str, + owner_auth_conf: dict, + site_auth_conf: dict, + log, + ): + self.owner: str = owner self.log = log - self.owner_auth_conf = self.set_auth_conf(owner_auth_conf) - self.site_auth_config = self.set_auth_conf(site_auth_conf) + self.owner_auth_conf: dict = owner_auth_conf + self.site_auth_config: dict = site_auth_conf self.owner_user_info = { "user": self.owner, "user_groups": self._get_groups(self.owner), @@ -226,21 +238,6 @@ def expand_and_process_access_groups(self, permission_set: set) -> set: return permission_set - @staticmethod - def set_auth_conf(auth_conf: Union[LazyConfigValue, dict]) -> dict: - """Resolve lazy config where empty - - Args: - auth_conf: Authorization configuration from a jupyter_config.py - - Returns: - Valid configuration dictionary - - """ - if isinstance(auth_conf, LazyConfigValue): - return auth_conf.to_dict() - return auth_conf - def get_owner_site_limits_for_access_user( self, access_user_name: str, access_user_groups: List[str] ) -> Set: From 26677dfa3d9acaa7e8a2d821fbb3735bced36c8e Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Thu, 4 Jul 2024 13:32:18 +0100 Subject: [PATCH 07/14] auth: turn a dictionary into attributes * Replace the `owner_user_info` dictionary (which contained two keys, one of which was duplicated) into variables. * This removes unnecessary dictionary lookups, the potential for key errors and makes typing easier. --- cylc/uiserver/authorise.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/cylc/uiserver/authorise.py b/cylc/uiserver/authorise.py index af0b08d3..cfcf74e8 100644 --- a/cylc/uiserver/authorise.py +++ b/cylc/uiserver/authorise.py @@ -158,19 +158,18 @@ class Authorization: def __init__( self, - owner: str, + owner_user_name: str, owner_auth_conf: dict, site_auth_conf: dict, log, ): - self.owner: str = owner + self.owner_user_name: str = owner_user_name + self.owner_user_groups: List[str] = self._get_groups( + self.owner_user_name + ) self.log = log self.owner_auth_conf: dict = owner_auth_conf self.site_auth_config: dict = site_auth_conf - self.owner_user_info = { - "user": self.owner, - "user_groups": self._get_groups(self.owner), - } self.owner_dict = self.build_owner_site_auth_conf() # lru_cache this method - see flake8-bugbear B019 @@ -321,7 +320,7 @@ def _get_permitted_operations(self, access_user: str): """ # users have full access to their own server (ALL) - if access_user == self.owner: + if access_user == self.owner_user_name: return set(self.ALL_OPS) # all groups the authenticated user belongs to @@ -379,7 +378,7 @@ def is_permitted(self, access_user: str, operation: str) -> bool: False. """ - if access_user == self.owner_user_info["user"]: + if access_user == self.owner_user_name: return True # re.sub needed for snake/camel case if re.sub( @@ -397,8 +396,8 @@ def build_owner_site_auth_conf(self): Creates a reduced site auth dictionary for the ui-server owner. """ owner_dict = {} - items_to_check = ["*", self.owner_user_info["user"]] - items_to_check.extend(self.owner_user_info["user_groups"]) + items_to_check = ["*", self.owner_user_name] + items_to_check.extend(self.owner_user_groups) # dict containing user info applying to the current ui_server owner for uis_owner_conf, access_user_dict in self.site_auth_config.items(): From 7b6455e1f17d8b343fa41dfce21563577feb5634 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Thu, 4 Jul 2024 14:06:40 +0100 Subject: [PATCH 08/14] auth: fill coverage holes --- cylc/uiserver/authorise.py | 2 +- cylc/uiserver/tests/test_authorise.py | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/cylc/uiserver/authorise.py b/cylc/uiserver/authorise.py index cfcf74e8..bebcbd87 100644 --- a/cylc/uiserver/authorise.py +++ b/cylc/uiserver/authorise.py @@ -239,7 +239,7 @@ def expand_and_process_access_groups(self, permission_set: set) -> set: def get_owner_site_limits_for_access_user( self, access_user_name: str, access_user_groups: List[str] - ) -> Set: + ) -> Set[str]: """Returns limits owner can give to given access_user Args: diff --git a/cylc/uiserver/tests/test_authorise.py b/cylc/uiserver/tests/test_authorise.py index df37331c..43e5e484 100644 --- a/cylc/uiserver/tests/test_authorise.py +++ b/cylc/uiserver/tests/test_authorise.py @@ -395,3 +395,17 @@ def test_parse_group_ids(monkeypatch, input_): [f'group:{mock_grid_db[i]}' for i in input_ if i in mock_grid_db], [i for i in input_ if i not in mock_grid_db], ) + + +def test_empty_configs(): + """Test the default permissions when no auth config is provided.""" + # blank site & user configs + auth_obj = Authorization('me', {}, {}, log) + # the owner_dict should be empty + assert auth_obj.owner_dict == {} + + # other users should not have any permissions + assert auth_obj._get_permitted_operations('someone-else') == set() + + # the owner should always have full permissions + assert auth_obj._get_permitted_operations('me') == set(ALL_OPS) From b4dce64445d70b2f762aba6389a22901afda28bd Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Thu, 4 Jul 2024 14:29:29 +0100 Subject: [PATCH 09/14] auth: switch to existing snake case conversion function * Also add tests covering the snake/camel case calling of the is_permitted interface. --- cylc/uiserver/authorise.py | 30 +++++++------- cylc/uiserver/tests/test_authorise.py | 56 +++++++++++++++++++++++---- 2 files changed, 64 insertions(+), 22 deletions(-) diff --git a/cylc/uiserver/authorise.py b/cylc/uiserver/authorise.py index bebcbd87..c45a1df5 100644 --- a/cylc/uiserver/authorise.py +++ b/cylc/uiserver/authorise.py @@ -19,7 +19,6 @@ import grp from inspect import iscoroutinefunction import os -import re from typing import List, Optional, Union, Set, Tuple import graphene @@ -29,6 +28,8 @@ from cylc.uiserver.schema import UISMutations from cylc.uiserver.utils import is_bearer_token_authenticated +from graphene.utils.str_converters import to_snake_case + class CylcAuthorizer(Authorizer): """Defines a safe default authorization policy for Jupyter Server. @@ -380,14 +381,15 @@ def is_permitted(self, access_user: str, operation: str) -> bool: """ if access_user == self.owner_user_name: return True - # re.sub needed for snake/camel case - if re.sub( - r'(? Optional[str]: """ if operation in Authorization.READ_AUTH_OPS: return Authorization.READ_OPERATION - else: - # Check it is a mutation in our schema - if ( - self.auth - and re.sub(r'(?. -from jupyter_server.extension.application import ExtensionApp +import logging from types import SimpleNamespace from unittest.mock import Mock, patch +from jupyter_server.extension.application import ExtensionApp + import pytest from cylc.uiserver.authorise import ( @@ -26,7 +28,7 @@ parse_group_ids, ) -log = ExtensionApp().log +LOG = ExtensionApp().log CONTROL_OPS = [ "clean", @@ -198,7 +200,7 @@ def test_get_permitted_operations( user_groups, ): mocked_get_groups.side_effect = [(owner_groups, []), (user_groups, [])] - auth_obj = Authorization(owner_name, FAKE_USER_CONF, FAKE_SITE_CONF, log) + auth_obj = Authorization(owner_name, FAKE_USER_CONF, FAKE_SITE_CONF, LOG) actual_operations = auth_obj.get_permitted_operations( access_user=user_name ) @@ -253,7 +255,7 @@ def test_get_access_user_permissions_from_owner_conf( """Test the un-processed permissions of owner conf.""" mocked_get_groups.return_value = (["group:blah"], []) authobj = Authorization( - "some_user", owner_auth_conf, {"fake": "config"}, log + "some_user", owner_auth_conf, {"fake": "config"}, LOG ) permitted_operations = authobj.get_access_user_permissions_from_owner_conf( access_user_name, access_user_groups @@ -286,7 +288,7 @@ def test_expand_and_process_access_groups(permission_set, expected): "some_user", {"fake": "config"}, {"fake": "config"}, - log, + LOG, ) actual = authobj.expand_and_process_access_groups(permission_set) assert actual == expected @@ -323,7 +325,7 @@ def test_expand_and_process_access_groups(permission_set, expected): ) def test_get_op_name(mut_field_name, operation, expected_op_name): mock_authobj = Authorization( - "some_user", {"fake": "config"}, {"fake": "config"}, log + "some_user", {"fake": "config"}, {"fake": "config"}, LOG ) auth_middleware = AuthorizationMiddleware auth_middleware.auth = mock_authobj @@ -357,7 +359,7 @@ def test_is_permitted( expected, ): mocked_get_groups.side_effect = [([""], []), ([""], [])] - auth_obj = Authorization(owner_name, FAKE_USER_CONF, FAKE_SITE_CONF, log) + auth_obj = Authorization(owner_name, FAKE_USER_CONF, FAKE_SITE_CONF, LOG) auth_obj.get_permitted_operations = Mock(return_value=["fake_operation"]) actual = auth_obj.is_permitted( access_user=user_name, operation="fake_operation" @@ -400,7 +402,7 @@ def test_parse_group_ids(monkeypatch, input_): def test_empty_configs(): """Test the default permissions when no auth config is provided.""" # blank site & user configs - auth_obj = Authorization('me', {}, {}, log) + auth_obj = Authorization('me', {}, {}, LOG) # the owner_dict should be empty assert auth_obj.owner_dict == {} @@ -409,3 +411,41 @@ def test_empty_configs(): # the owner should always have full permissions assert auth_obj._get_permitted_operations('me') == set(ALL_OPS) + + +def test_case_conversion(caplog): + """Test camel to snake case conversion of permission names.""" + my_log = logging.getLogger('test_case_conversion') + caplog.set_level(logging.INFO, logger=my_log.name) + + auth_obj = Authorization( + 'me', + {'other': ['CONTROL']}, + {'*': {'*': {'limit': ['ALL']}}}, + my_log, + ) + + # internal use case (perm provided in Python syntax) + assert auth_obj.is_permitted('other', 'release_hold_point') + assert auth_obj.is_permitted('other', 'set_graph_window_extent') + + # external use case (perm provided in GraphQL syntax) + assert auth_obj.is_permitted('other', 'ReleaseHoldPoint') + assert auth_obj.is_permitted('other', 'SetGraphWindowExtent') + + # invalid permission names + assert not auth_obj.is_permitted('other', 'no_such_permission') + assert not auth_obj.is_permitted('other', 'RELeaseHOLdPoint') + assert not auth_obj.is_permitted('other', 'SEtGRAPhWINDOwEXTENt') + assert not auth_obj.is_permitted('other', 'Release_Hold_Point') + + # calls should be logged + # (successfull call) + caplog.clear() + assert auth_obj.is_permitted('other', 'release_hold_point') + assert caplog.messages[-1] == 'other: authorized to release_hold_point' + + # (rejected call) + caplog.clear() + assert not auth_obj.is_permitted('other', 'broadcast') + assert caplog.messages[-1] == 'other: not authorized to broadcast' From 042f42e64101e674362e8d25ae038547f50b0de5 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Thu, 4 Jul 2024 15:14:33 +0100 Subject: [PATCH 10/14] auth: fill coverage hole --- cylc/uiserver/tests/test_authorise.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/cylc/uiserver/tests/test_authorise.py b/cylc/uiserver/tests/test_authorise.py index 6ed25dab..bb1da22b 100644 --- a/cylc/uiserver/tests/test_authorise.py +++ b/cylc/uiserver/tests/test_authorise.py @@ -449,3 +449,30 @@ def test_case_conversion(caplog): caplog.clear() assert not auth_obj.is_permitted('other', 'broadcast') assert caplog.messages[-1] == 'other: not authorized to broadcast' + + +def test_empty_list_in_config(): + """Test empty lists cause exceptions to be raised. + + Empty lists have an unclear meaning due to the inheritance of default + permissions so are not permitted. + + Note: + It would be nicer to validate this out at startup than forcing the + error at runtime. + """ + auth_obj = Authorization('me', {'other': []}, {}, LOG) + with pytest.raises(Exception, match='Error in user config'): + auth_obj.is_permitted('other', 'read') + + auth_obj = Authorization('me', {}, {'*': {'other': {'limit': []}}}, LOG) + with pytest.raises(Exception, match='Error in site config'): + auth_obj.is_permitted('other', 'read') + + auth_obj = Authorization('me', {}, {'*': {'other': {'default': []}}}, LOG) + with pytest.raises(Exception, match='Error in site config'): + auth_obj.is_permitted('other', 'read') + + auth_obj = Authorization('me', {}, {'*': {'me': {'default': []}}}, LOG) + with pytest.raises(Exception, match='Error in site config'): + auth_obj.return_site_auth_defaults_for_access_user('me', ['x']) From 199bbfce397f907ebef07753b4a09f21575add2c Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Mon, 29 Jul 2024 16:55:14 +0100 Subject: [PATCH 11/14] app: handle configs that are not LazyConfigValue objects (#616) --- cylc/uiserver/app.py | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/cylc/uiserver/app.py b/cylc/uiserver/app.py index c1fa3311..bf424d1d 100644 --- a/cylc/uiserver/app.py +++ b/cylc/uiserver/app.py @@ -53,14 +53,16 @@ the environment variable ``CYLC_SITE_CONF_PATH``. """ -from concurrent.futures import ProcessPoolExecutor import getpass import os -from pathlib import Path, PurePath import sys +from concurrent.futures import ProcessPoolExecutor +from pathlib import Path, PurePath from textwrap import dedent -from typing import List, Optional +from types import SimpleNamespace +from typing import List, Optional, Union +from jupyter_server.extension.application import ExtensionApp from pkg_resources import parse_version from tornado import ioloop from tornado.web import RedirectHandler @@ -76,9 +78,7 @@ default, validate, ) -from types import SimpleNamespace - -from jupyter_server.extension.application import ExtensionApp +from traitlets.config.loader import LazyConfigValue from cylc.flow.network.graphql import ( CylcGraphQLBackend, IgnoreFieldMiddleware @@ -109,6 +109,7 @@ from cylc.uiserver.websockets.tornado import TornadoSubscriptionServer from cylc.uiserver.workflows_mgr import WorkflowsManager + INFO_FILES_DIR = Path(USER_CONF_ROOT / "info_files") @@ -553,10 +554,21 @@ def set_auth(self) -> Authorization: """Create authorization object. One for the lifetime of the UIServer. """ + user_auth: Union[LazyConfigValue, dict] = ( + self.config.CylcUIServer.user_authorization + ) + site_auth: Union[LazyConfigValue, dict] = ( + self.config.CylcUIServer.site_authorization + ) + if isinstance(user_auth, LazyConfigValue): + user_auth = user_auth.to_dict() + if isinstance(site_auth, LazyConfigValue): + site_auth = site_auth.to_dict() + return Authorization( getpass.getuser(), - self.config.CylcUIServer.user_authorization.to_dict(), - self.config.CylcUIServer.site_authorization.to_dict(), + user_auth, + site_auth, self.log, ) From 8cb81dd07f75e873f845e98e4240378b026a5476 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Wed, 7 Aug 2024 15:38:30 +0100 Subject: [PATCH 12/14] shutdown: terminate subprocesses * We use a subprocess pool for running backgorund commands (e.g. `cylc clean`). * This creates subprocesses to run our commands in, however, it does not kill the subprocess once the command has completed, it keeps the subprocess open for future reuse (more efficient than creating and destroying them every time). * On shutdown we need to close the pool to mop up these subprocesses (this doesn't happen automatically). --- changes.d/619.fix.md | 1 + cylc/uiserver/app.py | 9 ++++++++- 2 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 changes.d/619.fix.md diff --git a/changes.d/619.fix.md b/changes.d/619.fix.md new file mode 100644 index 00000000..4b67746c --- /dev/null +++ b/changes.d/619.fix.md @@ -0,0 +1 @@ +Ensure that subprocesses created by Cylc UI Server are cleaned up correctly when the server shuts down. diff --git a/cylc/uiserver/app.py b/cylc/uiserver/app.py index c4caae78..3e41dc60 100644 --- a/cylc/uiserver/app.py +++ b/cylc/uiserver/app.py @@ -580,10 +580,17 @@ def launch_instance(cls, argv=None, workflow_id=None, **kwargs): async def stop_extension(self): # stop the async scan task await self.workflows_mgr.stop() + + # stop active subscriptions for sub in self.data_store_mgr.w_subs.values(): sub.stop() - # Shutdown the thread pool executor + + # Shutdown the thread pool executor (used for subscription processing) self.data_store_mgr.executor.shutdown(wait=False) + + # stop the process pool (used for background commands) + self.executor.shutdown() + # Destroy ZeroMQ context of all sockets self.workflows_mgr.context.destroy() self.profiler.stop() From f3f4b01bd9dab41b516f9109b42d6397d30cf531 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Thu, 8 Aug 2024 13:23:13 +0100 Subject: [PATCH 13/14] setup: types-pkg-resources -> types-setuptools (#620) --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index d5f3ccbc..32a35150 100644 --- a/setup.cfg +++ b/setup.cfg @@ -103,7 +103,7 @@ tests = # https://github.com/pytest-dev/pytest/issues/12263 pytest>=6,<8.2 towncrier>=23 - types-pkg_resources>=0.1.2 + types-setuptools types-requests>2 all = %(hub)s From dfcdf6c622d954d5071ddeece63dcd3c0630ed95 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 14 Oct 2024 11:07:26 +0100 Subject: [PATCH 14/14] build(deps): bump pypa/gh-action-pypi-publish from 1.9.0 to 1.10.3 (#631) [skip ci] --- .github/workflows/2_auto_publish_release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/2_auto_publish_release.yml b/.github/workflows/2_auto_publish_release.yml index e92168d4..bf5a6b8b 100644 --- a/.github/workflows/2_auto_publish_release.yml +++ b/.github/workflows/2_auto_publish_release.yml @@ -39,7 +39,7 @@ jobs: uses: cylc/release-actions/build-python-package@v1 - name: Publish distribution to PyPI - uses: pypa/gh-action-pypi-publish@v1.9.0 + uses: pypa/gh-action-pypi-publish@v1.10.3 with: user: __token__ # uses the API token feature of PyPI - least permissions possible password: ${{ secrets.PYPI_TOKEN }}