diff --git a/changelog.d/20298.feature b/changelog.d/20298.feature new file mode 100644 index 0000000000..cf2479f571 --- /dev/null +++ b/changelog.d/20298.feature @@ -0,0 +1 @@ +Add the `m.preview_url` capability (stabilizes support for MSC4452). diff --git a/synapse/rest/client/capabilities.py b/synapse/rest/client/capabilities.py index 4ddaaeda74..c38e9dd930 100644 --- a/synapse/rest/client/capabilities.py +++ b/synapse/rest/client/capabilities.py @@ -74,9 +74,12 @@ class CapabilitiesRestServlet(RestServlet): "m.get_login_token": { "enabled": self.config.auth.login_via_existing_enabled, }, + "m.preview_url": { + "enabled": self.config.media.url_preview_enabled, + }, } } - + # Keep around while clients migrate away from MSC4452 if self.config.experimental.msc4452_enabled: response["capabilities"]["io.element.msc4452.preview_url"] = { "enabled": self.config.media.url_preview_enabled, diff --git a/synapse/rest/client/media.py b/synapse/rest/client/media.py index c2e30b687b..906db81783 100644 --- a/synapse/rest/client/media.py +++ b/synapse/rest/client/media.py @@ -26,7 +26,6 @@ import re from synapse.api.errors import ( Codes, SynapseError, - UnrecognizedRequestError, cs_error, ) from synapse.http.server import ( @@ -90,16 +89,12 @@ class PreviewURLServlet(RestServlet): self.media_repo = media_repo self.media_storage = media_storage self.url_previewer = self.media_repo.url_previewer - self.can_respond_403 = hs.config.experimental.msc4452_enabled async def on_GET(self, request: SynapseRequest) -> None: requester = await self.auth.get_user_by_req(request) if self.url_previewer is None: # If we have no url_previewer then it has been disabled by the server. - if self.can_respond_403: - raise SynapseError(403, "URL Previews are disabled", Codes.FORBIDDEN) - else: - raise UnrecognizedRequestError(code=404) + raise SynapseError(403, "URL Previews are disabled", Codes.FORBIDDEN) url = parse_string(request, "url", required=True) ts = parse_integer(request, "ts") if ts is None: diff --git a/synapse/rest/media/preview_url_resource.py b/synapse/rest/media/preview_url_resource.py index 5a7bdf38f0..7f3e736b05 100644 --- a/synapse/rest/media/preview_url_resource.py +++ b/synapse/rest/media/preview_url_resource.py @@ -23,7 +23,7 @@ import re from typing import TYPE_CHECKING -from synapse.api.errors import Codes, SynapseError, UnrecognizedRequestError +from synapse.api.errors import Codes, SynapseError from synapse.http.server import respond_with_json_bytes from synapse.http.servlet import RestServlet, parse_integer, parse_string from synapse.http.site import SynapseRequest @@ -67,16 +67,12 @@ class PreviewUrlResource(RestServlet): self.media_repo = media_repo self.media_storage = media_storage self.url_previewer = self.media_repo.url_previewer - self.can_respond_403 = hs.config.experimental.msc4452_enabled async def on_GET(self, request: SynapseRequest) -> None: requester = await self.auth.get_user_by_req(request) if self.url_previewer is None: # If we have no url_previewer then it has been disabled by the server. - if self.can_respond_403: - raise SynapseError(403, "URL Previews are disabled", Codes.FORBIDDEN) - else: - raise UnrecognizedRequestError(code=404) + raise SynapseError(403, "URL Previews are disabled", Codes.FORBIDDEN) url = parse_string(request, "url", required=True) ts = parse_integer(request, "ts", default=self.clock.time_msec()) og = await self.url_previewer.preview(url, requester.user, ts) diff --git a/tests/rest/client/test_capabilities.py b/tests/rest/client/test_capabilities.py index 42926c4359..4e64407788 100644 --- a/tests/rest/client/test_capabilities.py +++ b/tests/rest/client/test_capabilities.py @@ -323,10 +323,44 @@ class CapabilitiesTestCase(unittest.HomeserverTestCase): @override_config( { "url_preview_enabled": False, - "experimental_features": {"msc4452_enabled": True}, } ) def test_url_previews_disabled(self) -> None: + access_token = self.get_success( + self.auth_handler.create_access_token_for_user_id( + self.user, device_id=None, valid_until_ms=None + ) + ) + channel = self.make_request("GET", self.url, access_token=access_token) + capabilities = channel.json_body["capabilities"] + self.assertEqual(channel.code, HTTPStatus.OK) + self.assertFalse(capabilities["m.preview_url"]["enabled"]) + + @skip_unless(lxml is not None, "Requires lxml") + @override_config( + { + "url_preview_enabled": True, + "url_preview_ip_range_blacklist": ["127.0.0.1"], + }, + ) + def test_url_previewsenabled(self) -> None: + access_token = self.get_success( + self.auth_handler.create_access_token_for_user_id( + self.user, device_id=None, valid_until_ms=None + ) + ) + channel = self.make_request("GET", self.url, access_token=access_token) + capabilities = channel.json_body["capabilities"] + self.assertEqual(channel.code, HTTPStatus.OK) + self.assertTrue(capabilities["m.preview_url"]["enabled"]) + + @override_config( + { + "url_preview_enabled": False, + "experimental_features": {"msc4452_enabled": True}, + } + ) + def test_url_previews_experimental_disabled(self) -> None: access_token = self.get_success( self.auth_handler.create_access_token_for_user_id( self.user, device_id=None, valid_until_ms=None @@ -336,6 +370,7 @@ class CapabilitiesTestCase(unittest.HomeserverTestCase): capabilities = channel.json_body["capabilities"] self.assertEqual(channel.code, HTTPStatus.OK) self.assertFalse(capabilities["io.element.msc4452.preview_url"]["enabled"]) + self.assertFalse(capabilities["m.preview_url"]["enabled"]) @skip_unless(lxml is not None, "Requires lxml") @override_config( @@ -345,7 +380,7 @@ class CapabilitiesTestCase(unittest.HomeserverTestCase): "experimental_features": {"msc4452_enabled": True}, }, ) - def test_url_previews_enabled(self) -> None: + def test_url_previews_experimental_enabled(self) -> None: access_token = self.get_success( self.auth_handler.create_access_token_for_user_id( self.user, device_id=None, valid_until_ms=None @@ -355,3 +390,4 @@ class CapabilitiesTestCase(unittest.HomeserverTestCase): capabilities = channel.json_body["capabilities"] self.assertEqual(channel.code, HTTPStatus.OK) self.assertTrue(capabilities["io.element.msc4452.preview_url"]["enabled"]) + self.assertTrue(capabilities["m.preview_url"]["enabled"]) diff --git a/tests/rest/client/test_media.py b/tests/rest/client/test_media.py index 1b926c613d..fa1841e039 100644 --- a/tests/rest/client/test_media.py +++ b/tests/rest/client/test_media.py @@ -1625,26 +1625,12 @@ class URLPreviewDisabledTests(unittest.HomeserverTestCase): "experimental_features": {"msc4452_enabled": False}, } ) - def test_disabled_previews(self) -> None: - """Tests that disabling URL previews gives back a sane response.""" - channel = self.make_request( - "GET", - "/_matrix/client/v1/media/preview_url?url=" + quote("http://example.com"), - access_token=self.tok, - ) - self.assertEqual(channel.code, 404, channel.result) - self.assertEqual( - channel.json_body, - {"errcode": "M_UNRECOGNIZED", "error": "Unrecognized request"}, - ) - @override_config( { "url_preview_enabled": False, - "experimental_features": {"msc4452_enabled": True}, } ) - def test_disabled_previews_with_msc4452(self) -> None: + def test_disabled_previews(self) -> None: """Tests that disabling URL previews gives back a sane response.""" channel = self.make_request( "GET",