Add m.preview_url capability (Stabilise MSC4452) (#20298)

For https://github.com/element-hq/synapse/issues/19719

MSC4452 is now merged, so the stable capability can be made visible.
I've kept in the unstable flag in order to transition away at a later
date.

### Pull Request Checklist

<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->

* [ ] Pull request is based on the develop branch
* [ ] Pull request includes a [changelog
file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog).
The entry should:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
  - Use markdown where necessary, mostly for `code blocks`.
  - End with either a period (.) or an exclamation mark (!).
  - Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [ ] [Code
style](https://element-hq.github.io/synapse/latest/code_style.html) is
correct (run the
[linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))

changelog.d/20298.feature

@@ -0,0 +1 @@
+Add the `m.preview_url` capability (stabilizes support for MSC4452).

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,

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:

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)

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"])

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",