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.

13 files changed, 33 insertions, 96 deletions

diff --git a/changelog.d/10167.feature b/changelog.d/10167.feature
new file mode 100644
index 0000000000..9c41140194
--- /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 8d5b2177d2..3940da5c88 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 3349f399ba..f6c1c97b40 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 37668079e7..6ebce4b2f7 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 03ec14ce87..ed09c6af1f 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 af0c679ed9..c9e7c57461 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 fe5fb6bee7..16d740cf58 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<room_id>[^/]*)/(?P<user_id>[^/]*)"
 
-    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<room_id>[^/]*)/(?P<event_id>[^/]*)"
 
-    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 6647063485..b3a93212f1 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 c26963b1e1..a49a61a34c 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 138411ad19..d29f2fea5e 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 f046bf9cb3..7d1bc40658 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<room_identifier>[^/]*)", releases=()
-    )
+    PATTERNS = client_patterns("/knock/(?P<room_identifier>[^/]*)")
 
     def __init__(self, hs: "HomeServer"):
         super().__init__()
diff --git a/tests/federation/transport/test_knocking.py b/tests/federation/transport/test_knocking.py
index 121aa88cfa..8c215d50f2 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 be5737e420..b52f78ba69 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