From 9e5ab6dd581389271b817d256e2fca113614a080 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 15 Jun 2021 07:45:14 -0400 Subject: [PATCH] Remove the experimental flag for knocking and use stable prefixes / endpoints. (#10167) * Room version 7 for knocking. * Stable prefixes and endpoints (both client and federation) for knocking. * Removes the experimental configuration flag. --- changelog.d/10167.feature | 1 + synapse/api/constants.py | 4 +-- synapse/api/room_versions.py | 7 +++--- synapse/config/experimental.py | 7 ------ synapse/federation/federation_client.py | 9 ++----- synapse/federation/transport/client.py | 27 +++------------------ synapse/federation/transport/server.py | 22 ++--------------- synapse/handlers/federation.py | 6 ++--- synapse/handlers/room_member.py | 5 +--- synapse/rest/__init__.py | 5 +--- synapse/rest/client/v2_alpha/knock.py | 6 ++--- tests/federation/transport/test_knocking.py | 22 +++++++---------- tests/rest/client/v2_alpha/test_sync.py | 8 +++--- 13 files changed, 33 insertions(+), 96 deletions(-) create mode 100644 changelog.d/10167.feature diff --git a/changelog.d/10167.feature b/changelog.d/10167.feature new file mode 100644 index 000000000..9c4114019 --- /dev/null +++ b/changelog.d/10167.feature @@ -0,0 +1 @@ +Implement "room knocking" as per [MSC2403](https://github.com/matrix-org/matrix-doc/pull/2403). Contributed by Sorunome and anoa. \ No newline at end of file diff --git a/synapse/api/constants.py b/synapse/api/constants.py index 8d5b2177d..3940da5c8 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -41,7 +41,7 @@ class Membership: INVITE = "invite" JOIN = "join" - KNOCK = "xyz.amorgan.knock" + KNOCK = "knock" LEAVE = "leave" BAN = "ban" LIST = (INVITE, JOIN, KNOCK, LEAVE, BAN) @@ -58,7 +58,7 @@ class PresenceState: class JoinRules: PUBLIC = "public" - KNOCK = "xyz.amorgan.knock" + KNOCK = "knock" INVITE = "invite" PRIVATE = "private" # As defined for MSC3083. diff --git a/synapse/api/room_versions.py b/synapse/api/room_versions.py index 3349f399b..f6c1c97b4 100644 --- a/synapse/api/room_versions.py +++ b/synapse/api/room_versions.py @@ -180,9 +180,9 @@ class RoomVersions: msc3083_join_rules=True, msc2403_knocking=False, ) - MSC2403 = RoomVersion( - "xyz.amorgan.knock", - RoomDisposition.UNSTABLE, + V7 = RoomVersion( + "7", + RoomDisposition.STABLE, EventFormatVersions.V3, StateResolutionVersions.V2, enforce_key_validity=True, @@ -206,6 +206,7 @@ KNOWN_ROOM_VERSIONS = { RoomVersions.V6, RoomVersions.MSC2176, RoomVersions.MSC3083, + RoomVersions.V7, ) # Note that we do not include MSC2043 here unless it is enabled in the config. } # type: Dict[str, RoomVersion] diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index 37668079e..6ebce4b2f 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -12,7 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersions from synapse.config._base import Config from synapse.types import JsonDict @@ -30,9 +29,3 @@ class ExperimentalConfig(Config): # MSC3026 (busy presence state) self.msc3026_enabled = experimental.get("msc3026_enabled", False) # type: bool - - # MSC2403 (room knocking) - self.msc2403_enabled = experimental.get("msc2403_enabled", False) # type: bool - if self.msc2403_enabled: - # Enable the MSC2403 unstable room version - KNOWN_ROOM_VERSIONS[RoomVersions.MSC2403.identifier] = RoomVersions.MSC2403 diff --git a/synapse/federation/federation_client.py b/synapse/federation/federation_client.py index 03ec14ce8..ed09c6af1 100644 --- a/synapse/federation/federation_client.py +++ b/synapse/federation/federation_client.py @@ -90,7 +90,6 @@ class FederationClient(FederationBase): self._clock.looping_call(self._clear_tried_cache, 60 * 1000) self.state = hs.get_state_handler() self.transport_layer = hs.get_federation_transport_client() - self._msc2403_enabled = hs.config.experimental.msc2403_enabled self.hostname = hs.hostname self.signing_key = hs.signing_key @@ -621,11 +620,7 @@ class FederationClient(FederationBase): SynapseError: if the chosen remote server returns a 300/400 code, or no servers successfully handle the request. """ - valid_memberships = {Membership.JOIN, Membership.LEAVE} - - # Allow knocking if the feature is enabled - if self._msc2403_enabled: - valid_memberships.add(Membership.KNOCK) + valid_memberships = {Membership.JOIN, Membership.LEAVE, Membership.KNOCK} if membership not in valid_memberships: raise RuntimeError( @@ -989,7 +984,7 @@ class FederationClient(FederationBase): return await self._do_send_knock(destination, pdu) return await self._try_destination_list( - "xyz.amorgan.knock/send_knock", destinations, send_request + "send_knock", destinations, send_request ) async def _do_send_knock(self, destination: str, pdu: EventBase) -> JsonDict: diff --git a/synapse/federation/transport/client.py b/synapse/federation/transport/client.py index af0c679ed..c9e7c5746 100644 --- a/synapse/federation/transport/client.py +++ b/synapse/federation/transport/client.py @@ -47,7 +47,6 @@ class TransportLayerClient: def __init__(self, hs): self.server_name = hs.hostname self.client = hs.get_federation_http_client() - self._msc2403_enabled = hs.config.experimental.msc2403_enabled @log_function def get_room_state_ids(self, destination, room_id, event_id): @@ -221,29 +220,14 @@ class TransportLayerClient: Fails with ``FederationDeniedError`` if the remote destination is not in our federation whitelist """ - valid_memberships = {Membership.JOIN, Membership.LEAVE} - - # Allow knocking if the feature is enabled - if self._msc2403_enabled: - valid_memberships.add(Membership.KNOCK) + valid_memberships = {Membership.JOIN, Membership.LEAVE, Membership.KNOCK} if membership not in valid_memberships: raise RuntimeError( "make_membership_event called with membership='%s', must be one of %s" % (membership, ",".join(valid_memberships)) ) - - # Knock currently uses an unstable prefix - if membership == Membership.KNOCK: - # Create a path in the form of /unstable/xyz.amorgan.knock/make_knock/... - path = _create_path( - FEDERATION_UNSTABLE_PREFIX + "/xyz.amorgan.knock", - "/make_knock/%s/%s", - room_id, - user_id, - ) - else: - path = _create_v1_path("/make_%s/%s/%s", membership, room_id, user_id) + path = _create_v1_path("/make_%s/%s/%s", membership, room_id, user_id) ignore_backoff = False retry_on_dns_fail = False @@ -366,12 +350,7 @@ class TransportLayerClient: The list of state events may be empty. """ - path = _create_path( - FEDERATION_UNSTABLE_PREFIX + "/xyz.amorgan.knock", - "/send_knock/%s/%s", - room_id, - event_id, - ) + path = _create_v1_path("/send_knock/%s/%s", room_id, event_id) return await self.client.put_json( destination=destination, path=path, data=content diff --git a/synapse/federation/transport/server.py b/synapse/federation/transport/server.py index fe5fb6bee..16d740cf5 100644 --- a/synapse/federation/transport/server.py +++ b/synapse/federation/transport/server.py @@ -567,8 +567,6 @@ class FederationV2SendLeaveServlet(BaseFederationServerServlet): class FederationMakeKnockServlet(BaseFederationServerServlet): PATH = "/make_knock/(?P[^/]*)/(?P[^/]*)" - PREFIX = FEDERATION_UNSTABLE_PREFIX + "/xyz.amorgan.knock" - async def on_GET(self, origin, content, query, room_id, user_id): try: # Retrieve the room versions the remote homeserver claims to support @@ -585,8 +583,6 @@ class FederationMakeKnockServlet(BaseFederationServerServlet): class FederationV1SendKnockServlet(BaseFederationServerServlet): PATH = "/send_knock/(?P[^/]*)/(?P[^/]*)" - PREFIX = FEDERATION_UNSTABLE_PREFIX + "/xyz.amorgan.knock" - async def on_PUT(self, origin, content, query, room_id, event_id): content = await self.handler.on_send_knock_request(origin, content, room_id) return 200, content @@ -1610,6 +1606,8 @@ FEDERATION_SERVLET_CLASSES = ( FederationVersionServlet, RoomComplexityServlet, FederationSpaceSummaryServlet, + FederationV1SendKnockServlet, + FederationMakeKnockServlet, ) # type: Tuple[Type[BaseFederationServlet], ...] OPENID_SERVLET_CLASSES = ( @@ -1652,12 +1650,6 @@ GROUP_ATTESTATION_SERVLET_CLASSES = ( ) # type: Tuple[Type[BaseFederationServlet], ...] -MSC2403_SERVLET_CLASSES = ( - FederationV1SendKnockServlet, - FederationMakeKnockServlet, -) - - DEFAULT_SERVLET_GROUPS = ( "federation", "room_list", @@ -1700,16 +1692,6 @@ def register_servlets( server_name=hs.hostname, ).register(resource) - # Register msc2403 (knocking) servlets if the feature is enabled - if hs.config.experimental.msc2403_enabled: - for servletclass in MSC2403_SERVLET_CLASSES: - servletclass( - hs=hs, - authenticator=authenticator, - ratelimiter=ratelimiter, - server_name=hs.hostname, - ).register(resource) - if "openid" in servlet_groups: for servletclass in OPENID_SERVLET_CLASSES: servletclass( diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 664706348..b3a93212f 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -2009,8 +2009,7 @@ class FederationHandler(BaseHandler): """ if get_domain_from_id(user_id) != origin: logger.info( - "Get /xyz.amorgan.knock/make_knock request for user %r" - "from different origin %s, ignoring", + "Get /make_knock request for user %r from different origin %s, ignoring", user_id, origin, ) @@ -2077,8 +2076,7 @@ class FederationHandler(BaseHandler): if get_domain_from_id(event.sender) != origin: logger.info( - "Got /xyz.amorgan.knock/send_knock request for user %r " - "from different origin %s", + "Got /send_knock request for user %r from different origin %s", event.sender, origin, ) diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index c26963b1e..a49a61a34 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -707,10 +707,7 @@ class RoomMemberHandler(metaclass=abc.ABCMeta): knock.event_id, txn_id, requester, content ) - elif ( - self.config.experimental.msc2403_enabled - and effective_membership_state == Membership.KNOCK - ): + elif effective_membership_state == Membership.KNOCK: if not is_host_in_room: # The knock needs to be sent over federation instead remote_room_hosts.append(get_domain_from_id(room_id)) diff --git a/synapse/rest/__init__.py b/synapse/rest/__init__.py index 138411ad1..d29f2fea5 100644 --- a/synapse/rest/__init__.py +++ b/synapse/rest/__init__.py @@ -121,10 +121,7 @@ class ClientRestResource(JsonResource): account_validity.register_servlets(hs, client_resource) relations.register_servlets(hs, client_resource) password_policy.register_servlets(hs, client_resource) - - # Register msc2403 (knocking) servlets if the feature is enabled - if hs.config.experimental.msc2403_enabled: - knock.register_servlets(hs, client_resource) + knock.register_servlets(hs, client_resource) # moving to /_synapse/admin admin.register_servlets_for_client_rest_resource(hs, client_resource) diff --git a/synapse/rest/client/v2_alpha/knock.py b/synapse/rest/client/v2_alpha/knock.py index f046bf9cb..7d1bc4065 100644 --- a/synapse/rest/client/v2_alpha/knock.py +++ b/synapse/rest/client/v2_alpha/knock.py @@ -39,12 +39,10 @@ logger = logging.getLogger(__name__) class KnockRoomAliasServlet(RestServlet): """ - POST /xyz.amorgan.knock/{roomIdOrAlias} + POST /knock/{roomIdOrAlias} """ - PATTERNS = client_patterns( - "/xyz.amorgan.knock/(?P[^/]*)", releases=() - ) + PATTERNS = client_patterns("/knock/(?P[^/]*)") def __init__(self, hs: "HomeServer"): super().__init__() diff --git a/tests/federation/transport/test_knocking.py b/tests/federation/transport/test_knocking.py index 121aa88cf..8c215d50f 100644 --- a/tests/federation/transport/test_knocking.py +++ b/tests/federation/transport/test_knocking.py @@ -25,9 +25,6 @@ from synapse.types import RoomAlias from tests.test_utils import event_injection from tests.unittest import FederatingHomeserverTestCase, TestCase, override_config -# An identifier to use while MSC2304 is not in a stable release of the spec -KNOCK_UNSTABLE_IDENTIFIER = "xyz.amorgan.knock" - class KnockingStrippedStateEventHelperMixin(TestCase): def send_example_state_events_to_room( @@ -61,7 +58,7 @@ class KnockingStrippedStateEventHelperMixin(TestCase): self.get_success( event_injection.inject_event( hs, - room_version=RoomVersions.MSC2403.identifier, + room_version=RoomVersions.V7.identifier, room_id=room_id, sender=sender, type="com.example.secret", @@ -121,7 +118,7 @@ class KnockingStrippedStateEventHelperMixin(TestCase): self.get_success( event_injection.inject_event( hs, - room_version=RoomVersions.MSC2403.identifier, + room_version=RoomVersions.V7.identifier, room_id=room_id, sender=sender, type=event_type, @@ -135,7 +132,7 @@ class KnockingStrippedStateEventHelperMixin(TestCase): room_state[EventTypes.Create] = { "content": { "creator": sender, - "room_version": RoomVersions.MSC2403.identifier, + "room_version": RoomVersions.V7.identifier, }, "state_key": "", } @@ -232,7 +229,7 @@ class FederationKnockingTestCase( room_id = self.helper.create_room_as( "u1", is_public=False, - room_version=RoomVersions.MSC2403.identifier, + room_version=RoomVersions.V7.identifier, tok=user_token, ) @@ -243,14 +240,13 @@ class FederationKnockingTestCase( channel = self.make_request( "GET", - "/_matrix/federation/unstable/%s/make_knock/%s/%s?ver=%s" + "/_matrix/federation/v1/make_knock/%s/%s?ver=%s" % ( - KNOCK_UNSTABLE_IDENTIFIER, room_id, fake_knocking_user_id, # Inform the remote that we support the room version of the room we're # knocking on - RoomVersions.MSC2403.identifier, + RoomVersions.V7.identifier, ), ) self.assertEquals(200, channel.code, channel.result) @@ -275,7 +271,7 @@ class FederationKnockingTestCase( self.clock, self.hs.hostname, self.hs.signing_key, - room_version=RoomVersions.MSC2403, + room_version=RoomVersions.V7, event_dict=knock_event, ) @@ -287,8 +283,8 @@ class FederationKnockingTestCase( # Send the signed knock event into the room channel = self.make_request( "PUT", - "/_matrix/federation/unstable/%s/send_knock/%s/%s" - % (KNOCK_UNSTABLE_IDENTIFIER, room_id, signed_knock_event.event_id), + "/_matrix/federation/v1/send_knock/%s/%s" + % (room_id, signed_knock_event.event_id), signed_knock_event_json, ) self.assertEquals(200, channel.code, channel.result) diff --git a/tests/rest/client/v2_alpha/test_sync.py b/tests/rest/client/v2_alpha/test_sync.py index be5737e42..b52f78ba6 100644 --- a/tests/rest/client/v2_alpha/test_sync.py +++ b/tests/rest/client/v2_alpha/test_sync.py @@ -333,7 +333,7 @@ class SyncKnockTestCase( self.room_id = self.helper.create_room_as( self.user_id, is_public=False, - room_version="xyz.amorgan.knock", + room_version="7", tok=self.tok, ) @@ -363,7 +363,7 @@ class SyncKnockTestCase( # Knock on a room channel = self.make_request( "POST", - "/_matrix/client/unstable/xyz.amorgan.knock/%s" % (self.room_id,), + "/_matrix/client/r0/knock/%s" % (self.room_id,), b"{}", self.knocker_tok, ) @@ -371,7 +371,7 @@ class SyncKnockTestCase( # We expect to see the knock event in the stripped room state later self.expected_room_state[EventTypes.Member] = { - "content": {"membership": "xyz.amorgan.knock", "displayname": "knocker"}, + "content": {"membership": "knock", "displayname": "knocker"}, "state_key": "@knocker:test", } @@ -384,7 +384,7 @@ class SyncKnockTestCase( self.assertEqual(channel.code, 200, channel.json_body) # Extract the stripped room state events from /sync - knock_entry = channel.json_body["rooms"]["xyz.amorgan.knock"] + knock_entry = channel.json_body["rooms"]["knock"] room_state_events = knock_entry[self.room_id]["knock_state"]["events"] # Validate that the knock membership event came last