Trim spaces from around email addresses and usernames when registering.

Additionally, include a stricter email address validation.

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"

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

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:])

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)
 

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) != ""