diff --git a/lib/galaxy/webapps/galaxy/api/users.py b/lib/galaxy/webapps/galaxy/api/users.py index c48381ffbdd..8dd4a781cd4 100644 --- a/lib/galaxy/webapps/galaxy/api/users.py +++ b/lib/galaxy/webapps/galaxy/api/users.py @@ -416,7 +416,7 @@ class UserAPIController(BaseAPIController, UsesTagsMixin, BaseUIController, Uses # Update email if 'email' in payload: email = payload.get('email') - message = self._validate_email(email) or validate_email(trans, email, user) + message = validate_email(trans, email, user) if message: raise exceptions.RequestParameterInvalidException(message) if user.email != email: @@ -441,7 +441,7 @@ class UserAPIController(BaseAPIController, UsesTagsMixin, BaseUIController, Uses # Update public name if 'username' in payload: username = payload.get('username') - message = self._validate_publicname(username) or validate_publicname(trans, username, user) + message = validate_publicname(trans, username, user) if message: raise exceptions.RequestParameterInvalidException(message) if user.username != username: @@ -581,19 +581,6 @@ class UserAPIController(BaseAPIController, UsesTagsMixin, BaseUIController, Uses else: raise exceptions.ObjectAttributeInvalidException("This type is not supported. Given object_type: %s" % object_type) - def _validate_email(self, email): - ''' Validate email and username using regex ''' - if email == '' or not isinstance(email, str): - return 'Please provide your email address.' - if not re.match(r'^(([^<>()[\]\.,;:\s@"]+(\.[^<>()[\]\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$', email): - return 'Please provide your valid email address.' - if len(email) > 255: - return 'Email cannot be more than 255 characters in length.' - - def _validate_publicname(self, username): - if not re.match(r'^[a-z0-9\-]{3,255}$', username): - return 'Public name must contain only lowercase letters, numbers and "-". It also has to be shorter than 255 characters but longer than 2.' - @expose_api def get_password(self, trans, id, payload=None, **kwd): """ diff --git a/lib/galaxy/webapps/galaxy/buildapp.py b/lib/galaxy/webapps/galaxy/buildapp.py index 6e2e75d1ade..be1479f2c20 100644 --- a/lib/galaxy/webapps/galaxy/buildapp.py +++ b/lib/galaxy/webapps/galaxy/buildapp.py @@ -16,6 +16,7 @@ import galaxy.model.mapping import galaxy.web.framework import galaxy.webapps.base.webapp from galaxy import util +from galaxy.security.validate_user_input import VALID_PUBLICNAME_SUB from galaxy.util import asbool from galaxy.util.properties import load_app_properties from galaxy.web.framework.middleware.batch import BatchMiddleware @@ -90,12 +91,14 @@ def app_factory(global_conf, load_app_kwds=None, **kwargs): webapp.add_route('/display_application/{dataset_id}/{app_name}/{link_name}/{user_id}/{app_action}/{action_param}/{action_param_extra:.+?}', controller='dataset', action='display_application', dataset_id=None, user_id=None, app_name=None, link_name=None, app_action=None, action_param=None, action_param_extra=None) - webapp.add_route('/u/{username}/d/{slug}/{filename}', controller='dataset', action='display_by_username_and_slug', filename=None) - webapp.add_route('/u/{username}/p/{slug}', controller='page', action='display_by_username_and_slug') - webapp.add_route('/u/{username}/h/{slug}', controller='history', action='display_by_username_and_slug') - webapp.add_route('/u/{username}/w/{slug}', controller='workflow', action='display_by_username_and_slug') - webapp.add_route('/u/{username}/w/{slug}/{format}', controller='workflow', action='display_by_username_and_slug') - webapp.add_route('/u/{username}/v/{slug}', controller='visualization', action='display_by_username_and_slug') + + USERNAME_REQS = {'username': VALID_PUBLICNAME_SUB.pattern} + webapp.add_route('/u/{username}/d/{slug}/{filename}', controller='dataset', action='display_by_username_and_slug', filename=None, requirements=USERNAME_REQS) + webapp.add_route('/u/{username}/p/{slug}', controller='page', action='display_by_username_and_slug', requirements=USERNAME_REQS) + webapp.add_route('/u/{username}/h/{slug}', controller='history', action='display_by_username_and_slug', requirements=USERNAME_REQS) + webapp.add_route('/u/{username}/w/{slug}', controller='workflow', action='display_by_username_and_slug', requirements=USERNAME_REQS) + webapp.add_route('/u/{username}/w/{slug}/{format}', controller='workflow', action='display_by_username_and_slug', requirements=USERNAME_REQS) + webapp.add_route('/u/{username}/v/{slug}', controller='visualization', action='display_by_username_and_slug', requirements=USERNAME_REQS) # TODO: Refactor above routes into external method to allow testing in # isolation as well. diff --git a/test/unit/data/security/test_validate_user_input.py b/test/unit/data/security/test_validate_user_input.py index 63f9b829625..e25be2a52cb 100644 --- a/test/unit/data/security/test_validate_user_input.py +++ b/test/unit/data/security/test_validate_user_input.py @@ -1,12 +1,33 @@ -from galaxy.security.validate_user_input import extract_domain +from galaxy.security.validate_user_input import ( + extract_domain, + validate_email_str, + validate_publicname_str, +) def test_extract_full_domain(): - assert extract_domain('jack@foo.com') == 'foo.com' - assert extract_domain('jack@foo.bar.com') == 'foo.bar.com' + assert extract_domain("jack@foo.com") == "foo.com" + assert extract_domain("jack@foo.bar.com") == "foo.bar.com" def test_extract_base_domain(): # Use case: ignore subdomains to filter out disposable email addresses - assert extract_domain('jack@foo.com', base_only=True) == 'foo.com' - assert extract_domain('jack@foo.bar.com', base_only=True) == 'bar.com' + assert extract_domain("jack@foo.com", base_only=True) == "foo.com" + assert extract_domain("jack@foo.bar.com", base_only=True) == "bar.com" + + +def test_validate_username(): + assert validate_publicname_str("testuser") == "" + assert validate_publicname_str("test.user") == "" + assert validate_publicname_str("test-user") == "" + assert validate_publicname_str("test@user") != "" + assert validate_publicname_str("test user") != "" + + +def test_validate_email(): + assert validate_email_str("test@foo.com") == "" + assert validate_email_str("test-dot.user@foo.com") == "" + assert validate_email_str("test@com") != "" + assert validate_email_str("@not-a-domain") != "" + too_long_email = "N" * 255 + "@foo.com" + assert validate_email_str(too_long_email) != ""