Add + as an allowed character for Matrix IDs (MSC4009) (matrix-org#15911)

Author: clokep

changelog.d/15911.feature

@@ -0,0 +1 @@
+Allow `+` in Matrix IDs, per [MSC4009](https://github.com/matrix-org/matrix-spec-proposals/pull/4009).

synapse/config/experimental.py

@@ -395,9 +395,6 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
         # Check that none of the other config options conflict with MSC3861 when enabled
         self.msc3861.check_config_conflicts(self.root)
 
-        # MSC4009: E.164 Matrix IDs
-        self.msc4009_e164_mxids = experimental.get("msc4009_e164_mxids", False)
-
         # MSC4010: Do not allow setting m.push_rules account data.
         self.msc4010_push_rules_account_data = experimental.get(
             "msc4010_push_rules_account_data", False

synapse/handlers/register.py

@@ -143,15 +143,10 @@ async def check_username(
         assigned_user_id: Optional[str] = None,
         inhibit_user_in_use_error: bool = False,
     ) -> None:
-        if types.contains_invalid_mxid_characters(
-            localpart, self.hs.config.experimental.msc4009_e164_mxids
-        ):
-            extra_chars = (
-                "=_-./+" if self.hs.config.experimental.msc4009_e164_mxids else "=_-./"
-            )
+        if types.contains_invalid_mxid_characters(localpart):
             raise SynapseError(
                 400,
-                f"User ID can only contain characters a-z, 0-9, or '{extra_chars}'",
+                "User ID can only contain characters a-z, 0-9, or '=_-./+'",
                 Codes.INVALID_USERNAME,
             )
 

synapse/handlers/saml.py

@@ -27,9 +27,9 @@
 from synapse.http.site import SynapseRequest
 from synapse.module_api import ModuleApi
 from synapse.types import (
+    MXID_LOCALPART_ALLOWED_CHARACTERS,
     UserID,
     map_username_to_mxid_localpart,
-    mxid_localpart_allowed_characters,
 )
 from synapse.util.iterutils import chunk_seq
 
@@ -371,7 +371,7 @@ def expire_sessions(self) -> None:
 
 
 DOT_REPLACE_PATTERN = re.compile(
-    "[^%s]" % (re.escape("".join(mxid_localpart_allowed_characters)),)
+    "[^%s]" % (re.escape("".join(MXID_LOCALPART_ALLOWED_CHARACTERS)),)
 )
 
 

synapse/handlers/sso.py

@@ -225,8 +225,6 @@ def __init__(self, hs: "HomeServer"):
 
         self._consent_at_registration = hs.config.consent.user_consent_at_registration
 
-        self._e164_mxids = hs.config.experimental.msc4009_e164_mxids
-
     def register_identity_provider(self, p: SsoIdentityProvider) -> None:
         p_id = p.idp_id
         assert p_id not in self._identity_providers
@@ -713,7 +711,7 @@ async def _register_mapped_user(
         # Since the localpart is provided via a potentially untrusted module,
         # ensure the MXID is valid before registering.
         if not attributes.localpart or contains_invalid_mxid_characters(
-            attributes.localpart, self._e164_mxids
+            attributes.localpart
         ):
             raise MappingException("localpart is invalid: %s" % (attributes.localpart,))
 
@@ -946,7 +944,7 @@ async def check_username_availability(
             localpart,
         )
 
-        if contains_invalid_mxid_characters(localpart, self._e164_mxids):
+        if contains_invalid_mxid_characters(localpart):
             raise SynapseError(400, "localpart is invalid: %s" % (localpart,))
         user_id = UserID(localpart, self._server_name).to_string()
         user_infos = await self._store.get_users_by_id_case_insensitive(user_id)

synapse/types/__init__.py

@@ -348,22 +348,15 @@ class EventID(DomainSpecificString):
     SIGIL = "$"
 
 
-mxid_localpart_allowed_characters = set(
-    "_-./=" + string.ascii_lowercase + string.digits
+MXID_LOCALPART_ALLOWED_CHARACTERS = set(
+    "_-./=+" + string.ascii_lowercase + string.digits
 )
-# MSC4007 adds the + to the allowed characters.
-#
-# TODO If this was accepted, update the SSO code to support this, see the callers
-#      of map_username_to_mxid_localpart.
-extended_mxid_localpart_allowed_characters = mxid_localpart_allowed_characters | {"+"}
 
 # Guest user IDs are purely numeric.
 GUEST_USER_ID_PATTERN = re.compile(r"^\d+$")
 
 
-def contains_invalid_mxid_characters(
-    localpart: str, use_extended_character_set: bool
-) -> bool:
+def contains_invalid_mxid_characters(localpart: str) -> bool:
     """Check for characters not allowed in an mxid or groupid localpart
 
     Args:
@@ -374,12 +367,7 @@ def contains_invalid_mxid_characters(
     Returns:
         True if there are any naughty characters
     """
-    allowed_characters = (
-        extended_mxid_localpart_allowed_characters
-        if use_extended_character_set
-        else mxid_localpart_allowed_characters
-    )
-    return any(c not in allowed_characters for c in localpart)
+    return any(c not in MXID_LOCALPART_ALLOWED_CHARACTERS for c in localpart)
 
 
 UPPER_CASE_PATTERN = re.compile(b"[A-Z_]")
@@ -396,7 +384,7 @@ def contains_invalid_mxid_characters(
 #    bytes rather than strings
 #
 NON_MXID_CHARACTER_PATTERN = re.compile(
-    ("[^%s]" % (re.escape("".join(mxid_localpart_allowed_characters - {"="})),)).encode(
+    ("[^%s]" % (re.escape("".join(MXID_LOCALPART_ALLOWED_CHARACTERS - {"="})),)).encode(
         "ascii"
     )
 )

tests/handlers/test_register.py

@@ -588,17 +588,16 @@ def test_register_not_support_user(self) -> None:
         self.assertFalse(self.get_success(d))
 
     def test_invalid_user_id(self) -> None:
-        invalid_user_id = "+abcd"
+        invalid_user_id = "^abcd"
         self.get_failure(
             self.handler.register_user(localpart=invalid_user_id), SynapseError
         )
 
-    @override_config({"experimental_features": {"msc4009_e164_mxids": True}})
-    def text_extended_user_ids(self) -> None:
-        """+ should be allowed according to MSC4009."""
-        valid_user_id = "+1234"
+    def test_special_chars(self) -> None:
+        """Ensure that characters which are allowed in Matrix IDs work."""
+        valid_user_id = "a1234_-./=+"
         user_id = self.get_success(self.handler.register_user(localpart=valid_user_id))
-        self.assertEqual(user_id, valid_user_id)
+        self.assertEqual(user_id, f"@{valid_user_id}:test")
 
     def test_invalid_user_id_length(self) -> None:
         invalid_user_id = "x" * 256