Removal: Remove support for experimental msc3886 (#17638)

changelog.d/17638.removal

@@ -0,0 +1 @@
+Remove support for closed [MSC3886](https://github.com/matrix-org/matrix-spec-proposals/pull/3886).

docs/upgrade.md

@@ -117,6 +117,17 @@ each upgrade are complete before moving on to the next upgrade, to avoid
 stacking them up. You can monitor the currently running background updates with
 [the Admin API](usage/administration/admin_api/background_updates.html#status).
 
+# Upgrading to v1.120.0
+
+## Removal of experimental MSC3886 feature
+
+[MSC3886](https://github.com/matrix-org/matrix-spec-proposals/pull/3886)
+has been closed (and will not enter the Matrix spec). As such, we are
+removing the experimental support for it in this release.
+
+The `experimental_features.msc3886_endpoint` configuration option has
+been removed.
+
 # Upgrading to v1.119.0
 
 ## Minimum supported Python version

synapse/config/experimental.py

@@ -365,11 +365,6 @@ class ExperimentalConfig(Config):
         # MSC3874: Filtering /messages with rel_types / not_rel_types.
         self.msc3874_enabled: bool = experimental.get("msc3874_enabled", False)
 
-        # MSC3886: Simple client rendezvous capability
-        self.msc3886_endpoint: Optional[str] = experimental.get(
-            "msc3886_endpoint", None
-        )
-
         # MSC3890: Remotely silence local notifications
         # Note: This option requires "experimental_features.msc3391_enabled" to be
         # set to "true", in order to communicate account data deletions to clients.

synapse/config/server.py

@@ -215,9 +215,6 @@ class HttpListenerConfig:
     additional_resources: Dict[str, dict] = attr.Factory(dict)
     tag: Optional[str] = None
     request_id_header: Optional[str] = None
-    # If true, the listener will return CORS response headers compatible with MSC3886:
-    # https://github.com/matrix-org/matrix-spec-proposals/pull/3886
-    experimental_cors_msc3886: bool = False
 
 
 @attr.s(slots=True, frozen=True, auto_attribs=True)
@@ -1004,7 +1001,6 @@ def parse_listener_def(num: int, listener: Any) -> ListenerConfig:
             additional_resources=listener.get("additional_resources", {}),
             tag=listener.get("tag"),
             request_id_header=listener.get("request_id_header"),
-            experimental_cors_msc3886=listener.get("experimental_cors_msc3886", False),
         )
 
     if socket_path:

synapse/http/server.py

@@ -921,15 +921,6 @@ def set_cors_headers(request: "SynapseRequest") -> None:
             b"Access-Control-Expose-Headers",
             b"Synapse-Trace-Id, Server, ETag",
         )
-    elif request.experimental_cors_msc3886:
-        request.setHeader(
-            b"Access-Control-Allow-Headers",
-            b"X-Requested-With, Content-Type, Authorization, Date, If-Match, If-None-Match",
-        )
-        request.setHeader(
-            b"Access-Control-Expose-Headers",
-            b"ETag, Location, X-Max-Bytes",
-        )
     else:
         request.setHeader(
             b"Access-Control-Allow-Headers",

synapse/http/site.py

@@ -94,7 +94,6 @@ class SynapseRequest(Request):
         self.reactor = site.reactor
         self._channel = channel  # this is used by the tests
         self.start_time = 0.0
-        self.experimental_cors_msc3886 = site.experimental_cors_msc3886
 
         # The requester, if authenticated. For federation requests this is the
         # server name, for client requests this is the Requester object.
@@ -666,10 +665,6 @@ class SynapseSite(ProxySite):
 
         request_id_header = config.http_options.request_id_header
 
-        self.experimental_cors_msc3886: bool = (
-            config.http_options.experimental_cors_msc3886
-        )
-
         def request_factory(channel: HTTPChannel, queued: bool) -> Request:
             return request_class(
                 channel,

synapse/rest/client/rendezvous.py

@@ -34,51 +34,6 @@ if TYPE_CHECKING:
 logger = logging.getLogger(__name__)
 
 
-# n.b [MSC3886](https://github.com/matrix-org/matrix-spec-proposals/pull/3886) has now been closed.
-# However, we want to keep this implementation around for some time.
-# TODO: define an end-of-life date for this implementation.
-class MSC3886RendezvousServlet(RestServlet):
-    """
-    This is a placeholder implementation of [MSC3886](https://github.com/matrix-org/matrix-spec-proposals/pull/3886)
-    simple client rendezvous capability that is used by the "Sign in with QR" functionality.
-
-    This implementation only serves as a 307 redirect to a configured server rather than being a full implementation.
-
-    A module that implements the full functionality is available at: https://pypi.org/project/matrix-http-rendezvous-synapse/.
-
-    Request:
-
-    POST /rendezvous HTTP/1.1
-    Content-Type: ...
-
-    ...
-
-    Response:
-
-    HTTP/1.1 307
-    Location: <configured endpoint>
-    """
-
-    PATTERNS = client_patterns(
-        "/org.matrix.msc3886/rendezvous$", releases=[], v1=False, unstable=True
-    )
-
-    def __init__(self, hs: "HomeServer"):
-        super().__init__()
-        redirection_target: Optional[str] = hs.config.experimental.msc3886_endpoint
-        assert (
-            redirection_target is not None
-        ), "Servlet is only registered if there is a redirection target"
-        self.endpoint = redirection_target.encode("utf-8")
-
-    async def on_POST(self, request: SynapseRequest) -> None:
-        respond_with_redirect(
-            request, self.endpoint, statusCode=TEMPORARY_REDIRECT, cors=True
-        )
-
-    # PUT, GET and DELETE are not implemented as they should be fulfilled by the redirect target.
-
-
 class MSC4108DelegationRendezvousServlet(RestServlet):
     PATTERNS = client_patterns(
         "/org.matrix.msc4108/rendezvous$", releases=[], v1=False, unstable=True
@@ -114,9 +69,6 @@ class MSC4108RendezvousServlet(RestServlet):
 
 
 def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None:
-    if hs.config.experimental.msc3886_endpoint is not None:
-        MSC3886RendezvousServlet(hs).register(http_server)
-
     if hs.config.experimental.msc4108_enabled:
         MSC4108RendezvousServlet(hs).register(http_server)
 

synapse/rest/client/versions.py

@@ -149,9 +149,6 @@ class VersionsRestServlet(RestServlet):
                     "org.matrix.msc3881": msc3881_enabled,
                     # Adds support for filtering /messages by event relation.
                     "org.matrix.msc3874": self.config.experimental.msc3874_enabled,
-                    # Adds support for simple HTTP rendezvous as per MSC3886
-                    "org.matrix.msc3886": self.config.experimental.msc3886_endpoint
-                    is not None,
                     # Adds support for relation-based redactions as per MSC3912.
                     "org.matrix.msc3912": self.config.experimental.msc3912_enabled,
                     # Whether recursively provide relations is supported.

tests/logging/test_terse_json.py

@@ -164,7 +164,6 @@ class TerseJsonTestCase(LoggerCleanupMixin, TestCase):
         site.site_tag = "test-site"
         site.server_version_string = "Server v1"
         site.reactor = Mock()
-        site.experimental_cors_msc3886 = False
         request = SynapseRequest(
             cast(HTTPChannel, FakeChannel(site, self.reactor)), site
         )

tests/rest/client/test_rendezvous.py

@@ -34,7 +34,6 @@ from tests import unittest
 from tests.unittest import override_config
 from tests.utils import HAS_AUTHLIB
 
-msc3886_endpoint = "/_matrix/client/unstable/org.matrix.msc3886/rendezvous"
 msc4108_endpoint = "/_matrix/client/unstable/org.matrix.msc4108/rendezvous"
 
 
@@ -54,17 +53,9 @@ class RendezvousServletTestCase(unittest.HomeserverTestCase):
         }
 
     def test_disabled(self) -> None:
-        channel = self.make_request("POST", msc3886_endpoint, {}, access_token=None)
-        self.assertEqual(channel.code, 404)
         channel = self.make_request("POST", msc4108_endpoint, {}, access_token=None)
         self.assertEqual(channel.code, 404)
 
-    @override_config({"experimental_features": {"msc3886_endpoint": "/asd"}})
-    def test_msc3886_redirect(self) -> None:
-        channel = self.make_request("POST", msc3886_endpoint, {}, access_token=None)
-        self.assertEqual(channel.code, 307)
-        self.assertEqual(channel.headers.getRawHeaders("Location"), ["/asd"])
-
     @unittest.skip_unless(HAS_AUTHLIB, "requires authlib")
     @override_config(
         {

tests/server.py

@@ -343,7 +343,6 @@ class FakeSite:
         self,
         resource: IResource,
         reactor: IReactorTime,
-        experimental_cors_msc3886: bool = False,
     ):
         """
 
@@ -352,7 +351,6 @@ class FakeSite:
         """
         self._resource = resource
         self.reactor = reactor
-        self.experimental_cors_msc3886 = experimental_cors_msc3886
 
     def getResourceFor(self, request: Request) -> IResource:
         return self._resource

tests/test_server.py

@@ -233,9 +233,7 @@ class OptionsResourceTests(unittest.TestCase):
         self.resource = OptionsResource()
         self.resource.putChild(b"res", DummyResource())
 
-    def _make_request(
+    def _make_request(self, method: bytes, path: bytes) -> FakeChannel:
-        self, method: bytes, path: bytes, experimental_cors_msc3886: bool = False
-    ) -> FakeChannel:
         """Create a request from the method/path and return a channel with the response."""
         # Create a site and query for the resource.
         site = SynapseSite(
@@ -246,7 +244,6 @@ class OptionsResourceTests(unittest.TestCase):
                 {
                     "type": "http",
                     "port": 0,
-                    "experimental_cors_msc3886": experimental_cors_msc3886,
                 },
             ),
             self.resource,
@@ -283,32 +280,6 @@ class OptionsResourceTests(unittest.TestCase):
             [b"Synapse-Trace-Id, Server"],
         )
 
-    def _check_cors_msc3886_headers(self, channel: FakeChannel) -> None:
-        # Ensure the correct CORS headers have been added
-        # as per https://github.com/matrix-org/matrix-spec-proposals/blob/hughns/simple-rendezvous-capability/proposals/3886-simple-rendezvous-capability.md#cors
-        self.assertEqual(
-            channel.headers.getRawHeaders(b"Access-Control-Allow-Origin"),
-            [b"*"],
-            "has correct CORS Origin header",
-        )
-        self.assertEqual(
-            channel.headers.getRawHeaders(b"Access-Control-Allow-Methods"),
-            [b"GET, HEAD, POST, PUT, DELETE, OPTIONS"],  # HEAD isn't in the spec
-            "has correct CORS Methods header",
-        )
-        self.assertEqual(
-            channel.headers.getRawHeaders(b"Access-Control-Allow-Headers"),
-            [
-                b"X-Requested-With, Content-Type, Authorization, Date, If-Match, If-None-Match"
-            ],
-            "has correct CORS Headers header",
-        )
-        self.assertEqual(
-            channel.headers.getRawHeaders(b"Access-Control-Expose-Headers"),
-            [b"ETag, Location, X-Max-Bytes"],
-            "has correct CORS Expose Headers header",
-        )
-
     def test_unknown_options_request(self) -> None:
         """An OPTIONS requests to an unknown URL still returns 204 No Content."""
         channel = self._make_request(b"OPTIONS", b"/foo/")
@@ -325,16 +296,6 @@ class OptionsResourceTests(unittest.TestCase):
 
         self._check_cors_standard_headers(channel)
 
-    def test_known_options_request_msc3886(self) -> None:
-        """An OPTIONS requests to an known URL still returns 204 No Content."""
-        channel = self._make_request(
-            b"OPTIONS", b"/res/", experimental_cors_msc3886=True
-        )
-        self.assertEqual(channel.code, 204)
-        self.assertNotIn("body", channel.result)
-
-        self._check_cors_msc3886_headers(channel)
-
     def test_unknown_request(self) -> None:
         """A non-OPTIONS request to an unknown URL should 404."""
         channel = self._make_request(b"GET", b"/foo/")