From a4243183f0b500f9f30f2d24af19f30a99f65f63 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 11 Jul 2023 12:21:00 -0400 Subject: [PATCH] Add + as an allowed character for Matrix IDs (MSC4009) (#15911) --- changelog.d/15911.feature | 1 + synapse/config/experimental.py | 3 --- synapse/handlers/register.py | 9 ++------- synapse/handlers/saml.py | 4 ++-- synapse/handlers/sso.py | 6 ++---- synapse/types/__init__.py | 22 +++++----------------- tests/handlers/test_register.py | 11 +++++------ 7 files changed, 17 insertions(+), 39 deletions(-) create mode 100644 changelog.d/15911.feature diff --git a/changelog.d/15911.feature b/changelog.d/15911.feature new file mode 100644 index 0000000000..b24077c6c3 --- /dev/null +++ b/changelog.d/15911.feature @@ -0,0 +1 @@ +Allow `+` in Matrix IDs, per [MSC4009](https://github.com/matrix-org/matrix-spec-proposals/pull/4009). diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index 8e0f5356b4..0970f22a75 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -382,9 +382,6 @@ class ExperimentalConfig(Config): # 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 diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py index a2d3f03061..3a55056df5 100644 --- a/synapse/handlers/register.py +++ b/synapse/handlers/register.py @@ -143,15 +143,10 @@ class RegistrationHandler: 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, ) diff --git a/synapse/handlers/saml.py b/synapse/handlers/saml.py index 874860d461..6083c9f4b5 100644 --- a/synapse/handlers/saml.py +++ b/synapse/handlers/saml.py @@ -27,9 +27,9 @@ from synapse.http.servlet import parse_string 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 @@ class SamlHandler: DOT_REPLACE_PATTERN = re.compile( - "[^%s]" % (re.escape("".join(mxid_localpart_allowed_characters)),) + "[^%s]" % (re.escape("".join(MXID_LOCALPART_ALLOWED_CHARACTERS)),) ) diff --git a/synapse/handlers/sso.py b/synapse/handlers/sso.py index c3a51722bd..4d29328a74 100644 --- a/synapse/handlers/sso.py +++ b/synapse/handlers/sso.py @@ -225,8 +225,6 @@ class SsoHandler: 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 @@ class SsoHandler: # 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 @@ class SsoHandler: 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) diff --git a/synapse/types/__init__.py b/synapse/types/__init__.py index 095be070e0..fdfd465c8d 100644 --- a/synapse/types/__init__.py +++ b/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 @@ UPPER_CASE_PATTERN = re.compile(b"[A-Z_]") # 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" ) ) diff --git a/tests/handlers/test_register.py b/tests/handlers/test_register.py index 8d8584609b..54eeec228e 100644 --- a/tests/handlers/test_register.py +++ b/tests/handlers/test_register.py @@ -587,17 +587,16 @@ class RegistrationTestCase(unittest.HomeserverTestCase): 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