diff --git a/lib/galaxy/app_unittest_utils/galaxy_mock.py b/lib/galaxy/app_unittest_utils/galaxy_mock.py index 6aadf55cc64..117873b2c12 100644 --- a/lib/galaxy/app_unittest_utils/galaxy_mock.py +++ b/lib/galaxy/app_unittest_utils/galaxy_mock.py @@ -150,6 +150,9 @@ class MockAppConfig(GalaxyDataTestConfig, CommonConfigurationMixin): self.expose_dataset_path = True self.allow_user_dataset_purge = True + self.allow_user_creation = True + self.email_domain_allowlist_content = None + self.email_domain_blocklist_content = None self.enable_old_display_applications = True self.redact_username_in_logs = False self.auth_config_file = "config/auth_conf.xml.sample" diff --git a/lib/galaxy/managers/users.py b/lib/galaxy/managers/users.py index 159995bec4e..c206a95c0d5 100644 --- a/lib/galaxy/managers/users.py +++ b/lib/galaxy/managers/users.py @@ -82,17 +82,17 @@ class UserManager(base.ModelManager, deletable.PurgableManagerMixin): return None, message if not email or not username or not password or not confirm: return None, "Please provide email, username and password." - message = "\n".join( - ( - validate_email(trans, email), - validate_password(trans, password, confirm), - validate_publicname(trans, username), - ) - ).rstrip() + + email = util.restore_text(email).strip() + username = util.restore_text(username).strip() + # We could add a separate option here for enabling or disabling domain validation (DNS resolution test) + validate_domain = trans.app.config.user_activation_on + message = "\n".join((validate_email(trans, email, validate_domain=validate_domain), + validate_password(trans, password, confirm), + validate_publicname(trans, username))).rstrip() + if message: return None, message - email = util.restore_text(email) - username = util.restore_text(username) message, status = trans.app.auth_manager.check_registration_allowed(email, username, password) if message: return None, message diff --git a/lib/galaxy/security/validate_user_input.py b/lib/galaxy/security/validate_user_input.py index a672c23d321..5ad85f52272 100644 --- a/lib/galaxy/security/validate_user_input.py +++ b/lib/galaxy/security/validate_user_input.py @@ -6,13 +6,22 @@ user inputs - so these methods do not need to be escaped. """ import logging import re +import socket from sqlalchemy import func log = logging.getLogger(__name__) # Email validity parameters -VALID_EMAIL_RE = re.compile(r"[^@]+@[^@]+\.[^@]+") +# +# Many words (and regexes) have been written about validating email addresses and there is no perfect answer on how it +# should be done. We choose to use the HTML5 spec (and corresponding regex) that engages in a "willful violation" of RFC +# 5322 to provide a reasonably good validation. Additionally, we allow Unicode characters in both the user and domain +# parts of the email by using re's '\w' character. Note that \w includes "word" characters but appears to exclude emoji +# characters, which should in fact be valid. +# +# https://html.spec.whatwg.org/multipage/input.html#e-mail-state-(type%3Demail) +VALID_EMAIL_RE = re.compile(r"^[\w.!#$%&'*+\/=?^_`{|}~-]+@[\w](?:[\w-]{0,61}[\w])?(?:\.[\w](?:[\w-]{0,61}[\w])?)*$") EMAIL_MAX_LEN = 255 # Public name validity parameters @@ -53,7 +62,7 @@ def validate_publicname_str(publicname): return "" -def validate_email(trans, email, user=None, check_dup=True, allow_empty=False): +def validate_email(trans, email, user=None, check_dup=True, allow_empty=False, validate_domain=False): """ Validates the email format, also checks whether the domain is blocklisted in the disposable domains configuration. """ @@ -79,11 +88,23 @@ def validate_email(trans, email, user=None, check_dup=True, allow_empty=False): domain = extract_domain(email, base_only=True) if domain in trans.app.config.email_domain_blocklist_content: message = "Please enter your permanent email address." + if not message and validate_domain: + domain = email.split('@', 1)[1] + message = validate_domain(domain) + return message + + +def validate_domain(domain): + message = None + try: + socket.gethostbyname(domain) + except socket.gaierror: + message = "The email domain cannot be resolved." return message def extract_domain(email, base_only=False): - domain = email.split("@")[1] + domain = email.split('@', 1)[1] parts = domain.split(".") if len(parts) > 2 and base_only: return (".").join(parts[-2:]) diff --git a/test/unit/app/managers/test_UserManager.py b/test/unit/app/managers/test_UserManager.py index 3e3b42bed21..c95014d05d1 100644 --- a/test/unit/app/managers/test_UserManager.py +++ b/test/unit/app/managers/test_UserManager.py @@ -82,6 +82,18 @@ class UserManagerTestCase(BaseTestCase): **dict(email="user2a@user2.user2", username="user2", password=default_password), ) + def test_trimming(self): + self.log("emails must be trimmed") + user2b, message = self.user_manager.register(self.trans, email=' user2b@user2.user2 ', username='user2b', + password=default_password, confirm=default_password) + self.assertIsNone(message) + self.assertEqual(user2b.email, 'user2b@user2.user2') + self.log("usernames must be trimmed") + user2c, message = self.user_manager.register(self.trans, email='user2c@user2.user2', username=' user2c ', + password=default_password, confirm=default_password) + self.assertIsNone(message) + self.assertEqual(user2c.username, 'user2c') + def test_email_queries(self): user2 = self.user_manager.create(**user2_data) diff --git a/test/unit/data/security/test_validate_user_input.py b/test/unit/data/security/test_validate_user_input.py index e25be2a52cb..9f3d629420f 100644 --- a/test/unit/data/security/test_validate_user_input.py +++ b/test/unit/data/security/test_validate_user_input.py @@ -1,5 +1,6 @@ from galaxy.security.validate_user_input import ( extract_domain, + validate_domain, validate_email_str, validate_publicname_str, ) @@ -16,6 +17,11 @@ def test_extract_base_domain(): assert extract_domain("jack@foo.bar.com", base_only=True) == "bar.com" +def test_validate_domain(): + assert validate_domain("example.org") is None + assert validate_domain("this is an invalid domain!") is not None + + def test_validate_username(): assert validate_publicname_str("testuser") == "" assert validate_publicname_str("test.user") == "" @@ -27,7 +33,13 @@ def test_validate_username(): 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") != "" + assert validate_email_str("test-plus+user@foo.com") == "" + assert validate_email_str("test-ünicode-user@foo.com") == "" + assert validate_email_str("test@ünicode-domain.com") == "" + assert validate_email_str("test-missing-domain@") != "" + assert validate_email_str("@test-missing-local") != "" + assert validate_email_str("test-invalid-local\\character@foo.com") != "" + assert validate_email_str("test@invalid-domain-character!com") != "" + assert validate_email_str("test@newlines.in.address.are.invalid\n\n.com") != "" too_long_email = "N" * 255 + "@foo.com" assert validate_email_str(too_long_email) != ""