From b223908bd8d8ba9a52f2cff3a9193c30bcdb4105 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 2 Feb 2022 10:17:53 -0500 Subject: [PATCH 1/3] Remove support for the webclient listener. --- changelog.d/11895.removal | 1 + docs/upgrade.md | 13 +++++ synapse/api/urls.py | 1 - synapse/app/homeserver.py | 34 +---------- synapse/config/server.py | 48 ++++------------ tests/http/test_webclient.py | 108 ----------------------------------- 6 files changed, 29 insertions(+), 176 deletions(-) create mode 100644 changelog.d/11895.removal delete mode 100644 tests/http/test_webclient.py diff --git a/changelog.d/11895.removal b/changelog.d/11895.removal new file mode 100644 index 000000000000..5accb1aef776 --- /dev/null +++ b/changelog.d/11895.removal @@ -0,0 +1 @@ +Drop support for `webclient` listeners, they are now ignored. The `web_client_location` configuration must point to a HTTP(S) URL, if configured. diff --git a/docs/upgrade.md b/docs/upgrade.md index f455d257babf..3606dc4b027b 100644 --- a/docs/upgrade.md +++ b/docs/upgrade.md @@ -85,6 +85,19 @@ process, for example: dpkg -i matrix-synapse-py3_1.3.0+stretch1_amd64.deb ``` +# Upgrading to v1.53.0 + +## Dropping support for `webclient` listeners and non-HTTP(S) `web_client_location` + +Per the deprecation notice in Synapse v1.51.0, listeners of type `webclient` +are no longer supported and configuring them is a now a configuration error. + +Configuring a non-HTTP(S) `web_client_location` configuration is is now a +configuration error. Since the `webclient` listener is no longer supported, this +setting only applies to the root path `/` of the Synapse install and no longer +the `/_matrix/client/` path. + + # Upgrading to v1.51.0 ## Deprecation of `webclient` listeners and non-HTTP(S) `web_client_location` diff --git a/synapse/api/urls.py b/synapse/api/urls.py index f9f9467dc1d7..bd49fa6a5f03 100644 --- a/synapse/api/urls.py +++ b/synapse/api/urls.py @@ -28,7 +28,6 @@ FEDERATION_V2_PREFIX = FEDERATION_PREFIX + "/v2" FEDERATION_UNSTABLE_PREFIX = FEDERATION_PREFIX + "/unstable" STATIC_PREFIX = "/_matrix/static" -WEB_CLIENT_PREFIX = "/_matrix/client" SERVER_KEY_V2_PREFIX = "/_matrix/key/v2" MEDIA_R0_PREFIX = "/_matrix/media/r0" MEDIA_V3_PREFIX = "/_matrix/media/v3" diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index efedcc88894b..24d55b049488 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -21,7 +21,6 @@ from twisted.internet.tcp import Port from twisted.web.resource import EncodingResourceWrapper, Resource from twisted.web.server import GzipEncoderFactory -from twisted.web.static import File import synapse import synapse.config.logger @@ -33,7 +32,6 @@ MEDIA_V3_PREFIX, SERVER_KEY_V2_PREFIX, STATIC_PREFIX, - WEB_CLIENT_PREFIX, ) from synapse.app import _base from synapse.app._base import ( @@ -53,7 +51,6 @@ from synapse.http.server import ( OptionsResource, RootOptionsRedirectResource, - RootRedirect, StaticResource, ) from synapse.http.site import SynapseSite @@ -134,15 +131,12 @@ def _listener_http( # Try to find something useful to serve at '/': # # 1. Redirect to the web client if it is an HTTP(S) URL. - # 2. Redirect to the web client served via Synapse. - # 3. Redirect to the static "Synapse is running" page. - # 4. Do not redirect and use a blank resource. - if self.config.server.web_client_location_is_redirect: + # 2. Redirect to the static "Synapse is running" page. + # 3. Do not redirect and use a blank resource. + if self.config.server.web_client_location: root_resource: Resource = RootOptionsRedirectResource( self.config.server.web_client_location ) - elif WEB_CLIENT_PREFIX in resources: - root_resource = RootOptionsRedirectResource(WEB_CLIENT_PREFIX) elif STATIC_PREFIX in resources: root_resource = RootOptionsRedirectResource(STATIC_PREFIX) else: @@ -270,28 +264,6 @@ def _configure_named_resource( if name in ["keys", "federation"]: resources[SERVER_KEY_V2_PREFIX] = KeyApiV2Resource(self) - if name == "webclient": - # webclient listeners are deprecated as of Synapse v1.51.0, remove it - # in > v1.53.0. - webclient_loc = self.config.server.web_client_location - - if webclient_loc is None: - logger.warning( - "Not enabling webclient resource, as web_client_location is unset." - ) - elif self.config.server.web_client_location_is_redirect: - resources[WEB_CLIENT_PREFIX] = RootRedirect(webclient_loc) - else: - logger.warning( - "Running webclient on the same domain is not recommended: " - "https://github.com/matrix-org/synapse#security-note - " - "after you move webclient to different host you can set " - "web_client_location to its full URL to enable redirection." - ) - # GZip is disabled here due to - # https://twistedmatrix.com/trac/ticket/7678 - resources[WEB_CLIENT_PREFIX] = File(webclient_loc) - if name == "metrics" and self.config.metrics.enable_metrics: resources[METRICS_PREFIX] = MetricsResource(RegistryProxy) diff --git a/synapse/config/server.py b/synapse/config/server.py index a0a00a9798da..7bc962454684 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -179,7 +179,6 @@ def generate_ip_set( "openid", "replication", "static", - "webclient", } @@ -519,16 +518,12 @@ def read_config(self, config, **kwargs): self.listeners = l2 self.web_client_location = config.get("web_client_location", None) - self.web_client_location_is_redirect = self.web_client_location and ( + # Non-HTTP(S) web client location is not supported. + if self.web_client_location and not ( self.web_client_location.startswith("http://") or self.web_client_location.startswith("https://") - ) - # A non-HTTP(S) web client location is deprecated. - if self.web_client_location and not self.web_client_location_is_redirect: - logger.warning(NO_MORE_NONE_HTTP_WEB_CLIENT_LOCATION_WARNING) - - # Warn if webclient is configured for a worker. - _warn_if_webclient_configured(self.listeners) + ): + raise ConfigError("web_client_location must point to a HTTP(S) URL.") self.gc_thresholds = read_gc_thresholds(config.get("gc_thresholds", None)) self.gc_seconds = self.read_gc_intervals(config.get("gc_min_interval", None)) @@ -1351,11 +1346,16 @@ def parse_listener_def(listener: Any) -> ListenerConfig: http_config = None if listener_type == "http": + try: + resources = [ + HttpResourceConfig(**res) for res in listener.get("resources", []) + ] + except ValueError as e: + raise ConfigError("Unknown listener resource") from e + http_config = HttpListenerConfig( x_forwarded=listener.get("x_forwarded", False), - resources=[ - HttpResourceConfig(**res) for res in listener.get("resources", []) - ], + resources=resources, additional_resources=listener.get("additional_resources", {}), tag=listener.get("tag"), ) @@ -1363,30 +1363,6 @@ def parse_listener_def(listener: Any) -> ListenerConfig: return ListenerConfig(port, bind_addresses, listener_type, tls, http_config) -NO_MORE_NONE_HTTP_WEB_CLIENT_LOCATION_WARNING = """ -Synapse no longer supports serving a web client. To remove this warning, -configure 'web_client_location' with an HTTP(S) URL. -""" - - -NO_MORE_WEB_CLIENT_WARNING = """ -Synapse no longer includes a web client. To redirect the root resource to a web client, configure -'web_client_location'. To remove this warning, remove 'webclient' from the 'listeners' -configuration. -""" - - -def _warn_if_webclient_configured(listeners: Iterable[ListenerConfig]) -> None: - for listener in listeners: - if not listener.http_options: - continue - for res in listener.http_options.resources: - for name in res.names: - if name == "webclient": - logger.warning(NO_MORE_WEB_CLIENT_WARNING) - return - - _MANHOLE_SETTINGS_SCHEMA = { "type": "object", "properties": { diff --git a/tests/http/test_webclient.py b/tests/http/test_webclient.py deleted file mode 100644 index ee5cf299f64c..000000000000 --- a/tests/http/test_webclient.py +++ /dev/null @@ -1,108 +0,0 @@ -# Copyright 2022 The Matrix.org Foundation C.I.C. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -from http import HTTPStatus -from typing import Dict - -from twisted.web.resource import Resource - -from synapse.app.homeserver import SynapseHomeServer -from synapse.config.server import HttpListenerConfig, HttpResourceConfig, ListenerConfig -from synapse.http.site import SynapseSite - -from tests.server import make_request -from tests.unittest import HomeserverTestCase, create_resource_tree, override_config - - -class WebClientTests(HomeserverTestCase): - @override_config( - { - "web_client_location": "https://example.org", - } - ) - def test_webclient_resolves_with_client_resource(self): - """ - Tests that both client and webclient resources can be accessed simultaneously. - - This is a regression test created in response to https://github.com/matrix-org/synapse/issues/11763. - """ - for resource_name_order_list in [ - ["webclient", "client"], - ["client", "webclient"], - ]: - # Create a dictionary from path regex -> resource - resource_dict: Dict[str, Resource] = {} - - for resource_name in resource_name_order_list: - resource_dict.update( - SynapseHomeServer._configure_named_resource(self.hs, resource_name) - ) - - # Create a root resource which ties the above resources together into one - root_resource = Resource() - create_resource_tree(resource_dict, root_resource) - - # Create a site configured with this resource to make HTTP requests against - listener_config = ListenerConfig( - port=8008, - bind_addresses=["127.0.0.1"], - type="http", - http_options=HttpListenerConfig( - resources=[HttpResourceConfig(names=resource_name_order_list)] - ), - ) - test_site = SynapseSite( - logger_name="synapse.access.http.fake", - site_tag=self.hs.config.server.server_name, - config=listener_config, - resource=root_resource, - server_version_string="1", - max_request_body_size=1234, - reactor=self.reactor, - ) - - # Attempt to make requests to endpoints on both the webclient and client resources - # on test_site. - self._request_client_and_webclient_resources(test_site) - - def _request_client_and_webclient_resources(self, test_site: SynapseSite) -> None: - """Make a request to an endpoint on both the webclient and client-server resources - of the given SynapseSite. - - Args: - test_site: The SynapseSite object to make requests against. - """ - - # Ensure that the *webclient* resource is behaving as expected (we get redirected to - # the configured web_client_location) - channel = make_request( - self.reactor, - site=test_site, - method="GET", - path="/_matrix/client", - ) - # Check that we are being redirected to the webclient location URI. - self.assertEqual(channel.code, HTTPStatus.FOUND) - self.assertEqual( - channel.headers.getRawHeaders("Location"), ["https://example.org"] - ) - - # Ensure that a request to the *client* resource works. - channel = make_request( - self.reactor, - site=test_site, - method="GET", - path="/_matrix/client/v3/login", - ) - self.assertEqual(channel.code, HTTPStatus.OK) - self.assertIn("flows", channel.json_body) From 04057fc0bb241c496fc4092fcbb779d43d01eba8 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 3 Feb 2022 13:06:09 -0500 Subject: [PATCH 2/3] Clarify upgrade notes. Co-authored-by: Brendan Abolivier --- docs/upgrade.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/upgrade.md b/docs/upgrade.md index 3606dc4b027b..7d582af0a794 100644 --- a/docs/upgrade.md +++ b/docs/upgrade.md @@ -94,7 +94,7 @@ are no longer supported and configuring them is a now a configuration error. Configuring a non-HTTP(S) `web_client_location` configuration is is now a configuration error. Since the `webclient` listener is no longer supported, this -setting only applies to the root path `/` of the Synapse install and no longer +setting only applies to the root path `/` of Synapse's web server and no longer the `/_matrix/client/` path. From 715b7adab85376360f74afe1f9ac0b3226becb89 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 3 Feb 2022 13:07:27 -0500 Subject: [PATCH 3/3] Update changelog. --- changelog.d/11895.removal | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/11895.removal b/changelog.d/11895.removal index 5accb1aef776..5973d96a3340 100644 --- a/changelog.d/11895.removal +++ b/changelog.d/11895.removal @@ -1 +1 @@ -Drop support for `webclient` listeners, they are now ignored. The `web_client_location` configuration must point to a HTTP(S) URL, if configured. +Drop support for `webclient` listeners and configuring `web_client_location` to a non-HTTP(S) URL. Deprecated configurations are a configuration error.