launchpad-reviewers team mailing list archive

[Guruprasad <mp+459228@xxxxxxxxxxxxxxxxxx>]

Guruprasad has proposed merging ~lgp171188/launchpad:fix-homeserver-domain-validation into launchpad:master.

Commit message:
Fix the matrix homeserver domain validation regex

The previous regex did not allow some valid TLDs like the
2-character-long ones.


Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~lgp171188/launchpad/+git/launchpad/+merge/459228
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~lgp171188/launchpad:fix-homeserver-domain-validation into launchpad:master.

diff --git a/lib/lp/registry/interfaces/socialaccount.py b/lib/lp/registry/interfaces/socialaccount.py
index 94e8d78..4bff5f6 100644
--- a/lib/lp/registry/interfaces/socialaccount.py
+++ b/lib/lp/registry/interfaces/socialaccount.py
@@ -8,6 +8,7 @@ __all__ = [
     "ISocialAccountSet",
     "MatrixPlatform",
     "SocialPlatformType",
+    "SOCIAL_PLATFORM_TYPES_MAP",
     "SocialAccountIdentityError",
     "validate_social_account_identity",
 ]
@@ -138,13 +139,16 @@ class MatrixPlatform(SocialPlatform):
             raise SocialAccountIdentityError("Username must be a string.")
         # Matrix username can contain a-z, 0-9, ., _, =, -, and /
         # ref: https://spec.matrix.org/v1.1/appendices/#user-identifiers
-        username_patter = r"^[A-z0-9-=_./]+"
-        if not re.match(username_patter, identity["username"]):
+        username_regex = r"^[A-z0-9-=_./]+"
+        if not re.match(username_regex, identity["username"]):
             raise SocialAccountIdentityError("Username must be valid.")
-        hs_pattern = r"^[A-z0-9][A-z0-9-]*(\.[A-z0-9]([A-z0-9-][A-z0-9])*)+$"
+        homeserver_regex = (
+            r"^(?:[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?\.)+"
+            "[a-z0-9][a-z0-9-]{0,61}[a-z0-9]$"
+        )
         if not isinstance(identity["homeserver"], str):
             raise SocialAccountIdentityError("Homeserver must be a string.")
-        if not re.match(hs_pattern, identity["homeserver"]):
+        if not re.match(homeserver_regex, identity["homeserver"]):
             raise SocialAccountIdentityError(
                 "Homeserver must be a valid domain."
             )
diff --git a/lib/lp/registry/tests/test_socialaccount.py b/lib/lp/registry/tests/test_socialaccount.py
index 6e45017..10dba86 100644
--- a/lib/lp/registry/tests/test_socialaccount.py
+++ b/lib/lp/registry/tests/test_socialaccount.py
@@ -1,3 +1,4 @@
+from testtools.matchers import MatchesStructure
 from zope.component import getUtility
 from zope.interface.verify import verifyObject
 
@@ -71,6 +72,31 @@ class TestSocialAccount(TestCaseWithFactory):
             social_account.identity["username"], "test-n/ic.kn=am_e"
         )
 
+    def test_different_length_tlds_homeserver_domain(self):
+        user = self.factory.makePerson()
+        tlds = ("sh", "com", "wiki", "online", "co.uk")
+
+        for tld in tlds:
+            homeserver_domain = f"example.{tld}"
+            attributes = {
+                "homeserver": homeserver_domain,
+                "username": "user",
+            }
+            social_account = getUtility(ISocialAccountSet).new(
+                user, SocialPlatformType.MATRIX, attributes
+            )
+            self.assertEqual(len(user.social_accounts), 1)
+            self.assertThat(
+                social_account,
+                MatchesStructure.byEquality(
+                    platform=SocialPlatformType.MATRIX,
+                    identity={
+                        "homeserver": homeserver_domain,
+                        "username": "user",
+                    },
+                ),
+            )
+
     def test_malformed_identity_matrix_account(self):
         # Matrix Identity must contain homeserver and username
         user = self.factory.makePerson()