Merge pull request #11856 from davelopez/unify_security_decode_id_exceptions

Unify security decode id exceptions
This commit is contained in:
Marius van den Beek
2021-04-19 09:15:37 +02:00
committed by GitHub
12 changed files with 44 additions and 117 deletions
+3 -7
View File
@@ -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):
+1 -6
View File
@@ -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:
+4 -22
View File
@@ -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 '')
+9 -28
View File
@@ -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.")
+1 -5
View File
@@ -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)
+9 -3
View File
@@ -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
@@ -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)
)
+11 -11
View File
@@ -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)
@@ -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:
-8
View File
@@ -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)
-10
View File
@@ -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'])
+1 -1
View File
@@ -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')