diff --git a/lib/galaxy/managers/base.py b/lib/galaxy/managers/base.py index bac9e9198ab..5ec7dc4c413 100644 --- a/lib/galaxy/managers/base.py +++ b/lib/galaxy/managers/base.py @@ -94,13 +94,9 @@ def get_class(class_name): def decode_id(app, id): - try: - # note: use str - occasionally a fully numeric id will be placed in post body and parsed as int via JSON - # resulting in error for valid id - return app.security.decode_id(str(id)) - except (ValueError, TypeError): - msg = "Malformed id ( %s ) specified, unable to decode" % (str(id)) - raise exceptions.MalformedId(msg, id=str(id)) + # note: use str - occasionally a fully numeric id will be placed in post body and parsed as int via JSON + # resulting in error for valid id + return app.security.decode_id(str(id)) def get_object(trans, id, class_name, check_ownership=False, check_accessible=False, deleted=None): diff --git a/lib/galaxy/managers/cloudauthzs.py b/lib/galaxy/managers/cloudauthzs.py index b206f7cc755..e568fe62924 100644 --- a/lib/galaxy/managers/cloudauthzs.py +++ b/lib/galaxy/managers/cloudauthzs.py @@ -7,7 +7,6 @@ import logging from galaxy import model from galaxy.exceptions import ( InternalServerError, - MalformedId ) from galaxy.managers import base from galaxy.managers import sharable @@ -103,11 +102,7 @@ class CloudAuthzsDeserializer(base.ModelDeserializer): :return: decoded authentication ID. """ - try: - decoded_authn_id = self.app.security.decode_id(val) - except Exception: - log.debug("cannot decode authz_id `" + str(val) + "`") - raise MalformedId(f"Invalid `authz_id` {val}!") + decoded_authn_id = self.app.security.decode_id(val, object_name='authz') trans = context.get("trans") if trans is None: diff --git a/lib/galaxy/managers/folders.py b/lib/galaxy/managers/folders.py index b88a2979002..f0930bd3ead 100644 --- a/lib/galaxy/managers/folders.py +++ b/lib/galaxy/managers/folders.py @@ -295,11 +295,7 @@ class FolderManager: :raises: MalformedId """ - try: - decoded_id = trans.security.decode_id(encoded_folder_id) - except ValueError: - raise MalformedId("Malformed folder id ( %s ) specified, unable to decode" % (str(encoded_folder_id))) - return decoded_id + return trans.security.decode_id(encoded_folder_id, object_name="folder") def cut_and_decode(self, trans, encoded_folder_id): """ @@ -454,7 +450,7 @@ class FoldersService: valid_add_roles = [] invalid_add_roles_names = [] for role_id in new_add_roles_ids: - role = self.role_manager.get(trans, self.__decode_id(trans, role_id, 'role')) + role = self.role_manager.get(trans, trans.security.decode_id(role_id, object_name='role')) # Check whether role is in the set of allowed roles valid_roles, total_roles = trans.app.security_agent.get_valid_roles(trans, folder) if role in valid_roles: @@ -468,7 +464,7 @@ class FoldersService: valid_manage_roles = [] invalid_manage_roles_names = [] for role_id in new_manage_roles_ids: - role = self.role_manager.get(trans, self.__decode_id(trans, role_id, 'role')) + role = self.role_manager.get(trans, trans.security.decode_id(role_id, object_name='role')) # Check whether role is in the set of allowed roles valid_roles, total_roles = trans.app.security_agent.get_valid_roles(trans, folder) if role in valid_roles: @@ -482,7 +478,7 @@ class FoldersService: valid_modify_roles = [] invalid_modify_roles_names = [] for role_id in new_modify_roles_ids: - role = self.role_manager.get(trans, self.__decode_id(trans, role_id, 'role')) + role = self.role_manager.get(trans, trans.security.decode_id(role_id, object_name='role')) # Check whether role is in the set of allowed roles valid_roles, total_roles = trans.app.security_agent.get_valid_roles(trans, folder) if role in valid_roles: @@ -555,17 +551,3 @@ class FoldersService: updated_folder = self.folder_manager.update(trans, folder, name, description) folder_dict = self.folder_manager.get_folder_dict(trans, updated_folder) return folder_dict - - def __decode_id(self, trans, encoded_id, object_name=None): - """ - Try to decode the id. - - :param object_name: Name of the object the id belongs to. (optional) - :type object_name: str - """ - try: - return trans.security.decode_id(encoded_id) - except TypeError: - raise MalformedId('Malformed %s id specified, unable to decode.' % object_name if object_name is not None else '') - except ValueError: - raise MalformedId('Wrong %s id specified, unable to decode.' % object_name if object_name is not None else '') diff --git a/lib/galaxy/managers/libraries.py b/lib/galaxy/managers/libraries.py index 7934ee33367..49bbd341d18 100644 --- a/lib/galaxy/managers/libraries.py +++ b/lib/galaxy/managers/libraries.py @@ -368,7 +368,7 @@ class LibrariesManager: :raises: MalformedId, ObjectNotFound """ - library = self.library_manager.get(trans, self.__decode_id(trans, id, 'library')) + library = self.library_manager.get(trans, trans.security.decode_id(id, object_name='library')) library_dict = self.library_manager.get_library_dict(trans, library) return library_dict @@ -419,7 +419,7 @@ class LibrariesManager: :rtype: dict :raises: RequestParameterMissingException """ - library = self.library_manager.get(trans, self.__decode_id(trans, id, 'library')) + library = self.library_manager.get(trans, trans.security.decode_id(id, object_name='library')) name = payload.get('name', None) if name == '': raise exceptions.RequestParameterMissingException("Parameter 'name' of library is required. You cannot remove it.") @@ -445,7 +445,7 @@ class LibrariesManager: .. seealso:: :attr:`galaxy.model.Library.dict_element_visible_keys` """ - library = self.library_manager.get(trans, self.__decode_id(trans, id, 'library')) + library = self.library_manager.get(trans, trans.security.decode_id(id, object_name='library')) library = self.library_manager.delete(trans, library, undelete) library_dict = self.library_manager.get_library_dict(trans, library) return library_dict @@ -478,7 +478,7 @@ class LibrariesManager: """ current_user_roles = trans.get_current_user_roles() is_admin = trans.user_is_admin - library = self.library_manager.get(trans, self.__decode_id(trans, id, 'library')) + library = self.library_manager.get(trans, trans.security.decode_id(id, object_name='library')) if not (is_admin or trans.app.security_agent.can_manage_library_item(current_user_roles, library)): raise exceptions.InsufficientPermissionsException('You do not have proper permission to access permissions of this library.') @@ -525,7 +525,7 @@ class LibrariesManager: """ is_admin = trans.user_is_admin current_user_roles = trans.get_current_user_roles() - library = self.library_manager.get(trans, self.__decode_id(trans, id, 'library')) + library = self.library_manager.get(trans, trans.security.decode_id(id, object_name='library')) if not (is_admin or trans.app.security_agent.can_manage_library_item(current_user_roles, library)): raise exceptions.InsufficientPermissionsException('You do not have proper permission to modify permissions of this library.') @@ -551,7 +551,7 @@ class LibrariesManager: valid_access_roles = [] invalid_access_roles_names = [] for role_id in new_access_roles_ids: - role = self.role_manager.get(trans, self.__decode_id(trans, role_id, 'role')) + role = self.role_manager.get(trans, trans.security.decode_id(role_id, object_name='role')) valid_roles, total_roles = trans.app.security_agent.get_valid_roles(trans, library, is_library_access=True) if role in valid_roles: valid_access_roles.append(role) @@ -564,7 +564,7 @@ class LibrariesManager: valid_add_roles = [] invalid_add_roles_names = [] for role_id in new_add_roles_ids: - role = self.role_manager.get(trans, self.__decode_id(trans, role_id, 'role')) + role = self.role_manager.get(trans, trans.security.decode_id(role_id, object_name='role')) valid_roles, total_roles = trans.app.security_agent.get_valid_roles(trans, library) if role in valid_roles: valid_add_roles.append(role) @@ -577,7 +577,7 @@ class LibrariesManager: valid_manage_roles = [] invalid_manage_roles_names = [] for role_id in new_manage_roles_ids: - role = self.role_manager.get(trans, self.__decode_id(trans, role_id, 'role')) + role = self.role_manager.get(trans, trans.security.decode_id(role_id, object_name='role')) valid_roles, total_roles = trans.app.security_agent.get_valid_roles(trans, library) if role in valid_roles: valid_manage_roles.append(role) @@ -590,7 +590,7 @@ class LibrariesManager: valid_modify_roles = [] invalid_modify_roles_names = [] for role_id in new_modify_roles_ids: - role = self.role_manager.get(trans, self.__decode_id(trans, role_id, 'role')) + role = self.role_manager.get(trans, trans.security.decode_id(role_id, object_name='role')) valid_roles, total_roles = trans.app.security_agent.get_valid_roles(trans, library) if role in valid_roles: valid_modify_roles.append(role) @@ -632,22 +632,3 @@ class LibrariesManager: trans.app.security_agent.copy_library_permissions(trans, library, library.root_folder) item = library.to_dict(view='element', value_mapper={'id': trans.security.encode_id, 'root_folder_id': trans.security.encode_id}) return item - - def __decode_id( - self, - trans: ProvidesAppContext, - encoded_id, - object_name: Optional[str] = None, - ): - """ - Try to decode the id. - - :param object_name: Name of the object the id belongs to. (optional) - :type object_name: str - """ - try: - return trans.security.decode_id(encoded_id) - except TypeError: - raise exceptions.MalformedId(f"Malformed {object_name if object_name is not None else ''} id specified, unable to decode.") - except ValueError: - raise exceptions.MalformedId(f"Wrong {object_name if object_name is not None else ''} id specified, unable to decode.") diff --git a/lib/galaxy/managers/markdown_util.py b/lib/galaxy/managers/markdown_util.py index bbf3333bb7f..5ccb5f28e37 100644 --- a/lib/galaxy/managers/markdown_util.py +++ b/lib/galaxy/managers/markdown_util.py @@ -28,7 +28,6 @@ except Exception: from galaxy.exceptions import ( MalformedContents, - MalformedId, MessageException, ) from galaxy.managers.hdcas import HDCASerializer @@ -69,10 +68,7 @@ def ready_galaxy_markdown_for_import(trans, external_galaxy_markdown): object_id = None if id_match: object_id = id_match.group(2) - try: - decoded_id = trans.security.decode_id(object_id) - except Exception: - raise MalformedId("Invalid encoded ID %s" % object_id) + decoded_id = trans.security.decode_id(object_id) line = line.replace(id_match.group(), "%s=%d" % (id_match.group(1), decoded_id)) return (line, False) diff --git a/lib/galaxy/security/idencoding.py b/lib/galaxy/security/idencoding.py index 0065dfd8962..9a51f5e0a97 100644 --- a/lib/galaxy/security/idencoding.py +++ b/lib/galaxy/security/idencoding.py @@ -1,6 +1,7 @@ import codecs import collections import logging +from typing import Optional from Crypto.Cipher import Blowfish from Crypto.Random import get_random_bytes @@ -79,9 +80,14 @@ class IdEncodingHelper: rval[k] = [self.encode_all_ids(el, True) for el in v] return rval - def decode_id(self, obj_id, kind=None): - id_cipher = self.__id_cipher(kind) - return int(unicodify(id_cipher.decrypt(codecs.decode(obj_id, 'hex'))).lstrip("!")) + def decode_id(self, obj_id, kind=None, object_name: Optional[str] = None): + try: + id_cipher = self.__id_cipher(kind) + return int(unicodify(id_cipher.decrypt(codecs.decode(obj_id, 'hex'))).lstrip("!")) + except TypeError: + raise galaxy.exceptions.MalformedId(f"Malformed {object_name if object_name is not None else ''} id ( {obj_id} ) specified, unable to decode.") + except ValueError: + raise galaxy.exceptions.MalformedId(f"Wrong {object_name if object_name is not None else ''} id ( {obj_id} ) specified, unable to decode.") def encode_guid(self, session_key): # Session keys are strings diff --git a/lib/galaxy/visualization/plugins/resource_parser.py b/lib/galaxy/visualization/plugins/resource_parser.py index bdc0cca9d00..0b6576479a2 100644 --- a/lib/galaxy/visualization/plugins/resource_parser.py +++ b/lib/galaxy/visualization/plugins/resource_parser.py @@ -197,11 +197,11 @@ class ResourceParser: # db models elif param_type == 'visualization': # ?: is this even used anymore/anywhere? - decoded_visualization_id = self._decode_id(query_param) + decoded_visualization_id = trans.security.decode_id(query_param, object_name=param_type) parsed_param = self.managers.visualization.get_accessible(decoded_visualization_id, trans.user) elif param_type == 'dataset': - decoded_dataset_id = self._decode_id(query_param) + decoded_dataset_id = trans.security.decode_id(query_param, object_name=param_type) parsed_param = self.managers.hda.get_accessible(decoded_dataset_id, trans.user) elif param_type == 'hda_or_ldda': @@ -209,7 +209,7 @@ class ResourceParser: # needs info from another param... hda_ldda = param_modifiers.get('hda_ldda') if hda_ldda == 'hda': - decoded_dataset_id = self._decode_id(encoded_dataset_id) + decoded_dataset_id = trans.security.decode_id(query_param, object_name="dataset") parsed_param = self.managers.hda.get_accessible(decoded_dataset_id, trans.user) else: parsed_param = self.managers.ldda.get(trans, encoded_dataset_id) @@ -220,12 +220,3 @@ class ResourceParser: parsed_param = galaxy.util.sanitize_html.sanitize_html(dbkey) return parsed_param - - def _decode_id(self, id): - try: - return self.app().security.decode_id(str(id)) - except (ValueError, TypeError): - raise galaxy.exceptions.MalformedId( - "Malformed id ( %s ) specified, unable to decode" % (str(id)), - id=str(id) - ) diff --git a/lib/galaxy/webapps/galaxy/api/cloudauthz.py b/lib/galaxy/webapps/galaxy/api/cloudauthz.py index 95e7e723a56..3617de5fb2b 100644 --- a/lib/galaxy/webapps/galaxy/api/cloudauthz.py +++ b/lib/galaxy/webapps/galaxy/api/cloudauthz.py @@ -122,13 +122,13 @@ class CloudAuthzController(BaseGalaxyAPIController): 'but received `{}`.'.format(type(config))) if authn_id: try: - authn_id = self.decode_id(authn_id) - except Exception: - log.debug(msg_template.format("cannot decode authn_id `" + str(authn_id) + "`")) - raise MalformedId('Invalid `authn_id`!') + decoded_authn_id = self.decode_id(authn_id) + except MalformedId as e: + log.debug(msg_template.format(f"cannot decode authz_id `{authn_id}`")) + raise e try: - trans.app.authnz_manager.can_user_assume_authn(trans, authn_id) + trans.app.authnz_manager.can_user_assume_authn(trans, decoded_authn_id) except Exception as e: raise e @@ -175,9 +175,9 @@ class CloudAuthzController(BaseGalaxyAPIController): msg_template = "Rejected user `" + str(trans.user.id) + "`'s request to delete cloudauthz config because of {}." try: authz_id = self.decode_id(encoded_authz_id) - except Exception: - log.debug(msg_template.format("cannot decode authz_id `" + str(encoded_authz_id) + "`")) - raise MalformedId('Invalid `authz_id`!') + except MalformedId as e: + log.debug(msg_template.format(f"cannot decode authz_id `{encoded_authz_id}`")) + raise e try: cloudauthz = trans.app.authnz_manager.try_get_authz_config(trans.sa_session, trans.user.id, authz_id) @@ -239,9 +239,9 @@ class CloudAuthzController(BaseGalaxyAPIController): msg_template = "Rejected user `" + str(trans.user.id) + "`'s request to delete cloudauthz config because of {}." try: authz_id = self.decode_id(encoded_authz_id) - except Exception: - log.debug(msg_template.format("cannot decode authz_id `" + str(encoded_authz_id) + "`")) - raise MalformedId('Invalid `authz_id`!') + except MalformedId as e: + log.debug(msg_template.format(f"cannot decode authz_id `{encoded_authz_id}`")) + raise e try: cloudauthz_to_update = trans.app.authnz_manager.try_get_authz_config(trans.sa_session, trans.user.id, authz_id) diff --git a/lib/galaxy/webapps/galaxy/api/library_contents.py b/lib/galaxy/webapps/galaxy/api/library_contents.py index 6e62e4096a2..d2c8c4f871d 100644 --- a/lib/galaxy/webapps/galaxy/api/library_contents.py +++ b/lib/galaxy/webapps/galaxy/api/library_contents.py @@ -92,10 +92,8 @@ class LibraryContentsController(BaseGalaxyAPIController, UsesLibraryMixinItems, ld.api_type = 'file' rval.append(ld) return rval - try: - decoded_library_id = self.decode_id(library_id) - except Exception: - raise exceptions.MalformedId('Malformed library id ( %s ) specified, unable to decode.' % library_id) + + decoded_library_id = self.decode_id(library_id) try: library = trans.sa_session.query(trans.app.model.Library).filter(trans.app.model.Library.table.c.id == decoded_library_id).one() except MultipleResultsFound: diff --git a/lib/galaxy/workflow/run_request.py b/lib/galaxy/workflow/run_request.py index 1feeef2df60..8dd6f7fc920 100644 --- a/lib/galaxy/workflow/run_request.py +++ b/lib/galaxy/workflow/run_request.py @@ -498,11 +498,3 @@ def workflow_request_to_run_config(work_request_context, workflow_invocation): resource_params=resource_params, ) return workflow_run_config - - -def __decode_id(trans, workflow_id, model_type="workflow"): - try: - return trans.security.decode_id(workflow_id) - except Exception: - message = f"Malformed {model_type} id ( {workflow_id} ) specified, unable to decode" - raise exceptions.MalformedId(message) diff --git a/lib/tool_shed/webapp/api/repositories.py b/lib/tool_shed/webapp/api/repositories.py index 00a4a84e1ff..82a24d2e22d 100644 --- a/lib/tool_shed/webapp/api/repositories.py +++ b/lib/tool_shed/webapp/api/repositories.py @@ -17,7 +17,6 @@ from galaxy.exceptions import ( ActionInputError, ConfigDoesNotAllowException, InsufficientPermissionsException, - MalformedId, ObjectNotFound, RequestParameterInvalidException, RequestParameterMissingException @@ -684,11 +683,6 @@ class RepositoriesController(BaseAPIController): :raises: ObjectNotFound, MalformedId """ - try: - trans.security.decode_id(id) - except Exception: - raise MalformedId('The given id is invalid.') - repository = repository_util.get_repository_in_tool_shed(self.app, id) if repository is None: raise ObjectNotFound('Unable to locate repository for the given id.') @@ -806,10 +800,6 @@ class RepositoriesController(BaseAPIController): :not found: Empty dictionary. """ - try: - trans.security.decode_id(id) - except Exception: - raise MalformedId('The given id is invalid.') recursive = util.asbool(kwd.get('recursive', 'True')) all_metadata = {} repository = repository_util.get_repository_in_tool_shed(self.app, id, eagerload_columns=['downloadable_revisions']) diff --git a/test/unit/managers/test_HistoryManager.py b/test/unit/managers/test_HistoryManager.py index d2c38e2de88..1a151c85a9b 100644 --- a/test/unit/managers/test_HistoryManager.py +++ b/test/unit/managers/test_HistoryManager.py @@ -667,7 +667,7 @@ class HistoryDeserializerTestCase(BaseTestCase): self.assertEqual(len(user_shares), 0) self.log('adding a bad user id should error') - self.assertRaises(TypeError, + self.assertRaises(exceptions.MalformedId, deserializer.deserialize, item, {'users_shared_with': [None]}, user=user2) self.log('adding a non-existing user id should do nothing')