From d6c3b7584fc46571e65226793304df35d7081534 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 29 Nov 2023 14:03:42 -0500 Subject: Request & follow redirects for /media/v3/download (#16701) Implement MSC3860 to follow redirects for federated media downloads. Note that the Client-Server API doesn't support this (yet) since the media repository in Synapse doesn't have a way of supporting redirects. --- tests/media/test_media_storage.py | 62 ++++++++++++++++++++++++++++-- tests/replication/test_multi_media_repo.py | 2 +- 2 files changed, 59 insertions(+), 5 deletions(-) (limited to 'tests') diff --git a/tests/media/test_media_storage.py b/tests/media/test_media_storage.py index f262304c3d..f981d1c0d8 100644 --- a/tests/media/test_media_storage.py +++ b/tests/media/test_media_storage.py @@ -27,10 +27,11 @@ from typing_extensions import Literal from twisted.internet import defer from twisted.internet.defer import Deferred +from twisted.python.failure import Failure from twisted.test.proto_helpers import MemoryReactor from twisted.web.resource import Resource -from synapse.api.errors import Codes +from synapse.api.errors import Codes, HttpResponseException from synapse.events import EventBase from synapse.http.types import QueryParams from synapse.logging.context import make_deferred_yieldable @@ -247,6 +248,7 @@ class MediaRepoTests(unittest.HomeserverTestCase): retry_on_dns_fail: bool = True, max_size: Optional[int] = None, ignore_backoff: bool = False, + follow_redirects: bool = False, ) -> "Deferred[Tuple[int, Dict[bytes, List[bytes]]]]": """A mock for MatrixFederationHttpClient.get_file.""" @@ -257,10 +259,15 @@ class MediaRepoTests(unittest.HomeserverTestCase): output_stream.write(data) return response + def write_err(f: Failure) -> Failure: + f.trap(HttpResponseException) + output_stream.write(f.value.response) + return f + d: Deferred[Tuple[bytes, Tuple[int, Dict[bytes, List[bytes]]]]] = Deferred() self.fetches.append((d, destination, path, args)) # Note that this callback changes the value held by d. - d_after_callback = d.addCallback(write_to) + d_after_callback = d.addCallbacks(write_to, write_err) return make_deferred_yieldable(d_after_callback) # Mock out the homeserver's MatrixFederationHttpClient @@ -316,10 +323,11 @@ class MediaRepoTests(unittest.HomeserverTestCase): self.assertEqual(len(self.fetches), 1) self.assertEqual(self.fetches[0][1], "example.com") self.assertEqual( - self.fetches[0][2], "/_matrix/media/r0/download/" + self.media_id + self.fetches[0][2], "/_matrix/media/v3/download/" + self.media_id ) self.assertEqual( - self.fetches[0][3], {"allow_remote": "false", "timeout_ms": "20000"} + self.fetches[0][3], + {"allow_remote": "false", "timeout_ms": "20000", "allow_redirect": "true"}, ) headers = { @@ -671,6 +679,52 @@ class MediaRepoTests(unittest.HomeserverTestCase): [b"cross-origin"], ) + def test_unknown_v3_endpoint(self) -> None: + """ + If the v3 endpoint fails, try the r0 one. + """ + channel = self.make_request( + "GET", + f"/_matrix/media/v3/download/{self.media_id}", + shorthand=False, + await_result=False, + ) + self.pump() + + # We've made one fetch, to example.com, using the media URL, and asking + # the other server not to do a remote fetch + self.assertEqual(len(self.fetches), 1) + self.assertEqual(self.fetches[0][1], "example.com") + self.assertEqual( + self.fetches[0][2], "/_matrix/media/v3/download/" + self.media_id + ) + + # The result which says the endpoint is unknown. + unknown_endpoint = b'{"errcode":"M_UNRECOGNIZED","error":"Unknown request"}' + self.fetches[0][0].errback( + HttpResponseException(404, "NOT FOUND", unknown_endpoint) + ) + + self.pump() + + # There should now be another request to the r0 URL. + self.assertEqual(len(self.fetches), 2) + self.assertEqual(self.fetches[1][1], "example.com") + self.assertEqual( + self.fetches[1][2], f"/_matrix/media/r0/download/{self.media_id}" + ) + + headers = { + b"Content-Length": [b"%d" % (len(self.test_image.data))], + } + + self.fetches[1][0].callback( + (self.test_image.data, (len(self.test_image.data), headers)) + ) + + self.pump() + self.assertEqual(channel.code, 200) + class TestSpamCheckerLegacy: """A spam checker module that rejects all media that includes the bytes diff --git a/tests/replication/test_multi_media_repo.py b/tests/replication/test_multi_media_repo.py index 1e9994cc0b..9a7b675f54 100644 --- a/tests/replication/test_multi_media_repo.py +++ b/tests/replication/test_multi_media_repo.py @@ -133,7 +133,7 @@ class MediaRepoShardTestCase(BaseMultiWorkerStreamTestCase): self.assertEqual(request.method, b"GET") self.assertEqual( request.path, - f"/_matrix/media/r0/download/{target}/{media_id}".encode(), + f"/_matrix/media/v3/download/{target}/{media_id}".encode(), ) self.assertEqual( request.requestHeaders.getRawHeaders(b"host"), [target.encode("utf-8")] -- cgit 1.5.1 From d6e194b2bc1b71c04d55617359b43f4dde5bc593 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 4 Dec 2023 04:36:12 -0700 Subject: Implement MSC4069: Inhibit profile propagation (#16636) MSC: https://github.com/matrix-org/matrix-spec-proposals/pull/4069 --- changelog.d/16636.feature | 1 + synapse/config/experimental.py | 4 + synapse/handlers/profile.py | 10 ++- synapse/rest/client/profile.py | 31 +++++++- synapse/rest/client/versions.py | 2 + tests/rest/client/test_profile.py | 160 ++++++++++++++++++++++++++++++++++++++ 6 files changed, 203 insertions(+), 5 deletions(-) create mode 100644 changelog.d/16636.feature (limited to 'tests') diff --git a/changelog.d/16636.feature b/changelog.d/16636.feature new file mode 100644 index 0000000000..a363eaafaf --- /dev/null +++ b/changelog.d/16636.feature @@ -0,0 +1 @@ +Support MSC4069: Inhibit profile propagation. \ No newline at end of file diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index 9f830e7094..6b9febe5a7 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -419,3 +419,7 @@ class ExperimentalConfig(Config): self.msc4028_push_encrypted_events = experimental.get( "msc4028_push_encrypted_events", False ) + + self.msc4069_profile_inhibit_propagation = experimental.get( + "msc4069_profile_inhibit_propagation", False + ) diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py index 1027fbfd28..e043fd5322 100644 --- a/synapse/handlers/profile.py +++ b/synapse/handlers/profile.py @@ -129,6 +129,7 @@ class ProfileHandler: new_displayname: str, by_admin: bool = False, deactivation: bool = False, + propagate: bool = True, ) -> None: """Set the displayname of a user @@ -138,6 +139,7 @@ class ProfileHandler: new_displayname: The displayname to give this user. by_admin: Whether this change was made by an administrator. deactivation: Whether this change was made while deactivating the user. + propagate: Whether this change also applies to the user's membership events. """ if not self.hs.is_mine(target_user): raise SynapseError(400, "User is not hosted on this homeserver") @@ -188,7 +190,8 @@ class ProfileHandler: target_user.to_string(), profile, by_admin, deactivation ) - await self._update_join_states(requester, target_user) + if propagate: + await self._update_join_states(requester, target_user) async def get_avatar_url(self, target_user: UserID) -> Optional[str]: if self.hs.is_mine(target_user): @@ -221,6 +224,7 @@ class ProfileHandler: new_avatar_url: str, by_admin: bool = False, deactivation: bool = False, + propagate: bool = True, ) -> None: """Set a new avatar URL for a user. @@ -230,6 +234,7 @@ class ProfileHandler: new_avatar_url: The avatar URL to give this user. by_admin: Whether this change was made by an administrator. deactivation: Whether this change was made while deactivating the user. + propagate: Whether this change also applies to the user's membership events. """ if not self.hs.is_mine(target_user): raise SynapseError(400, "User is not hosted on this homeserver") @@ -278,7 +283,8 @@ class ProfileHandler: target_user.to_string(), profile, by_admin, deactivation ) - await self._update_join_states(requester, target_user) + if propagate: + await self._update_join_states(requester, target_user) @cached() async def check_avatar_size_and_mime_type(self, mxc: str) -> bool: diff --git a/synapse/rest/client/profile.py b/synapse/rest/client/profile.py index 493e1acea0..12d3da1ced 100644 --- a/synapse/rest/client/profile.py +++ b/synapse/rest/client/profile.py @@ -13,12 +13,17 @@ # limitations under the License. """ This module contains REST servlets to do with profile: /profile/ """ + from http import HTTPStatus from typing import TYPE_CHECKING, Tuple from synapse.api.errors import Codes, SynapseError from synapse.http.server import HttpServer -from synapse.http.servlet import RestServlet, parse_json_object_from_request +from synapse.http.servlet import ( + RestServlet, + parse_boolean, + parse_json_object_from_request, +) from synapse.http.site import SynapseRequest from synapse.rest.client._base import client_patterns from synapse.types import JsonDict, UserID @@ -27,6 +32,20 @@ if TYPE_CHECKING: from synapse.server import HomeServer +def _read_propagate(hs: "HomeServer", request: SynapseRequest) -> bool: + # This will always be set by the time Twisted calls us. + assert request.args is not None + + propagate = True + if hs.config.experimental.msc4069_profile_inhibit_propagation: + do_propagate = request.args.get(b"org.matrix.msc4069.propagate") + if do_propagate is not None: + propagate = parse_boolean( + request, "org.matrix.msc4069.propagate", default=False + ) + return propagate + + class ProfileDisplaynameRestServlet(RestServlet): PATTERNS = client_patterns("/profile/(?P[^/]*)/displayname", v1=True) CATEGORY = "Event sending requests" @@ -80,7 +99,11 @@ class ProfileDisplaynameRestServlet(RestServlet): errcode=Codes.BAD_JSON, ) - await self.profile_handler.set_displayname(user, requester, new_name, is_admin) + propagate = _read_propagate(self.hs, request) + + await self.profile_handler.set_displayname( + user, requester, new_name, is_admin, propagate=propagate + ) return 200, {} @@ -135,8 +158,10 @@ class ProfileAvatarURLRestServlet(RestServlet): 400, "Missing key 'avatar_url'", errcode=Codes.MISSING_PARAM ) + propagate = _read_propagate(self.hs, request) + await self.profile_handler.set_avatar_url( - user, requester, new_avatar_url, is_admin + user, requester, new_avatar_url, is_admin, propagate=propagate ) return 200, {} diff --git a/synapse/rest/client/versions.py b/synapse/rest/client/versions.py index f4d19e0470..54c01bb739 100644 --- a/synapse/rest/client/versions.py +++ b/synapse/rest/client/versions.py @@ -129,6 +129,8 @@ class VersionsRestServlet(RestServlet): "org.matrix.msc3981": self.config.experimental.msc3981_recurse_relations, # Adds support for deleting account data. "org.matrix.msc3391": self.config.experimental.msc3391_enabled, + # Allows clients to inhibit profile update propagation. + "org.matrix.msc4069": self.config.experimental.msc4069_profile_inhibit_propagation, }, }, ) diff --git a/tests/rest/client/test_profile.py b/tests/rest/client/test_profile.py index 8f923fd40f..eb0fa00bb3 100644 --- a/tests/rest/client/test_profile.py +++ b/tests/rest/client/test_profile.py @@ -312,6 +312,166 @@ class ProfileTestCase(unittest.HomeserverTestCase): ) self.assertEqual(channel.code, 200, channel.result) + @unittest.override_config( + {"experimental_features": {"msc4069_profile_inhibit_propagation": True}} + ) + def test_msc4069_inhibit_propagation(self) -> None: + """Tests to ensure profile update propagation can be inhibited.""" + for prop in ["avatar_url", "displayname"]: + room_id = self.helper.create_room_as(tok=self.owner_tok) + + channel = self.make_request( + "PUT", + f"/rooms/{room_id}/state/m.room.member/{self.owner}", + content={"membership": "join", prop: "mxc://my.server/existing"}, + access_token=self.owner_tok, + ) + self.assertEqual(channel.code, 200, channel.result) + + channel = self.make_request( + "PUT", + f"/profile/{self.owner}/{prop}?org.matrix.msc4069.propagate=false", + content={prop: "http://my.server/pic.gif"}, + access_token=self.owner_tok, + ) + self.assertEqual(channel.code, 200, channel.result) + + res = ( + self._get_avatar_url() + if prop == "avatar_url" + else self._get_displayname() + ) + self.assertEqual(res, "http://my.server/pic.gif") + + channel = self.make_request( + "GET", + f"/rooms/{room_id}/state/m.room.member/{self.owner}", + access_token=self.owner_tok, + ) + self.assertEqual(channel.code, 200, channel.result) + self.assertEqual(channel.json_body.get(prop), "mxc://my.server/existing") + + def test_msc4069_inhibit_propagation_disabled(self) -> None: + """Tests to ensure profile update propagation inhibit flags are ignored when the + experimental flag is not enabled. + """ + for prop in ["avatar_url", "displayname"]: + room_id = self.helper.create_room_as(tok=self.owner_tok) + + channel = self.make_request( + "PUT", + f"/rooms/{room_id}/state/m.room.member/{self.owner}", + content={"membership": "join", prop: "mxc://my.server/existing"}, + access_token=self.owner_tok, + ) + self.assertEqual(channel.code, 200, channel.result) + + channel = self.make_request( + "PUT", + f"/profile/{self.owner}/{prop}?org.matrix.msc4069.propagate=false", + content={prop: "http://my.server/pic.gif"}, + access_token=self.owner_tok, + ) + self.assertEqual(channel.code, 200, channel.result) + + res = ( + self._get_avatar_url() + if prop == "avatar_url" + else self._get_displayname() + ) + self.assertEqual(res, "http://my.server/pic.gif") + + channel = self.make_request( + "GET", + f"/rooms/{room_id}/state/m.room.member/{self.owner}", + access_token=self.owner_tok, + ) + self.assertEqual(channel.code, 200, channel.result) + + # The ?propagate=false should be ignored by the server because the config flag + # isn't enabled. + self.assertEqual(channel.json_body.get(prop), "http://my.server/pic.gif") + + def test_msc4069_inhibit_propagation_default(self) -> None: + """Tests to ensure profile update propagation happens by default.""" + for prop in ["avatar_url", "displayname"]: + room_id = self.helper.create_room_as(tok=self.owner_tok) + + channel = self.make_request( + "PUT", + f"/rooms/{room_id}/state/m.room.member/{self.owner}", + content={"membership": "join", prop: "mxc://my.server/existing"}, + access_token=self.owner_tok, + ) + self.assertEqual(channel.code, 200, channel.result) + + channel = self.make_request( + "PUT", + f"/profile/{self.owner}/{prop}", + content={prop: "http://my.server/pic.gif"}, + access_token=self.owner_tok, + ) + self.assertEqual(channel.code, 200, channel.result) + + res = ( + self._get_avatar_url() + if prop == "avatar_url" + else self._get_displayname() + ) + self.assertEqual(res, "http://my.server/pic.gif") + + channel = self.make_request( + "GET", + f"/rooms/{room_id}/state/m.room.member/{self.owner}", + access_token=self.owner_tok, + ) + self.assertEqual(channel.code, 200, channel.result) + + # The ?propagate=false should be ignored by the server because the config flag + # isn't enabled. + self.assertEqual(channel.json_body.get(prop), "http://my.server/pic.gif") + + @unittest.override_config( + {"experimental_features": {"msc4069_profile_inhibit_propagation": True}} + ) + def test_msc4069_inhibit_propagation_like_default(self) -> None: + """Tests to ensure clients can request explicit profile propagation.""" + for prop in ["avatar_url", "displayname"]: + room_id = self.helper.create_room_as(tok=self.owner_tok) + + channel = self.make_request( + "PUT", + f"/rooms/{room_id}/state/m.room.member/{self.owner}", + content={"membership": "join", prop: "mxc://my.server/existing"}, + access_token=self.owner_tok, + ) + self.assertEqual(channel.code, 200, channel.result) + + channel = self.make_request( + "PUT", + f"/profile/{self.owner}/{prop}?org.matrix.msc4069.propagate=true", + content={prop: "http://my.server/pic.gif"}, + access_token=self.owner_tok, + ) + self.assertEqual(channel.code, 200, channel.result) + + res = ( + self._get_avatar_url() + if prop == "avatar_url" + else self._get_displayname() + ) + self.assertEqual(res, "http://my.server/pic.gif") + + channel = self.make_request( + "GET", + f"/rooms/{room_id}/state/m.room.member/{self.owner}", + access_token=self.owner_tok, + ) + self.assertEqual(channel.code, 200, channel.result) + + # The client requested ?propagate=true, so it should have happened. + self.assertEqual(channel.json_body.get(prop), "http://my.server/pic.gif") + def _setup_local_files(self, names_and_props: Dict[str, Dict[str, Any]]) -> None: """Stores metadata about files in the database. -- cgit 1.5.1 From 9e7f80037d08619cdd193831540d34e9ed5aacd7 Mon Sep 17 00:00:00 2001 From: Mathieu Velten Date: Mon, 4 Dec 2023 13:31:42 +0100 Subject: Server notices: add an autojoin setting for the notices room (#16699) Co-authored-by: Patrick Cloke --- changelog.d/16699.feature | 1 + docs/server_notices.md | 3 +++ docs/usage/configuration/config_documentation.md | 3 +++ synapse/config/server_notices.py | 2 ++ synapse/server_notices/server_notices_manager.py | 15 ++++++++++++- tests/rest/admin/test_server_notice.py | 27 ++++++++++++++++++++++++ 6 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 changelog.d/16699.feature (limited to 'tests') diff --git a/changelog.d/16699.feature b/changelog.d/16699.feature new file mode 100644 index 0000000000..7ede50f326 --- /dev/null +++ b/changelog.d/16699.feature @@ -0,0 +1 @@ +Add an autojoin setting for the notices room so users get joined directly instead of receiving an invite. diff --git a/docs/server_notices.md b/docs/server_notices.md index 339d10a0ab..aae25d23b8 100644 --- a/docs/server_notices.md +++ b/docs/server_notices.md @@ -46,6 +46,7 @@ server_notices: system_mxid_display_name: "Server Notices" system_mxid_avatar_url: "mxc://server.com/oumMVlgDnLYFaPVkExemNVVZ" room_name: "Server Notices" + auto_join: true ``` The only compulsory setting is `system_mxid_localpart`, which defines the user @@ -55,6 +56,8 @@ room which will be created. `system_mxid_display_name` and `system_mxid_avatar_url` can be used to set the displayname and avatar of the Server Notices user. +`auto_join` will autojoin users to the notices room instead of sending an invite. + ## Sending notices To send server notices to users you can use the diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 7c4e742cd5..812a7d429b 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -3815,6 +3815,8 @@ Sub-options for this setting include: * `system_mxid_display_name`: set the display name of the "notices" user * `system_mxid_avatar_url`: set the avatar for the "notices" user * `room_name`: set the room name of the server notices room +* `auto_join`: boolean. If true, the user will be automatically joined to the room instead of being invited. + Defaults to false. _Added in Synapse 1.98.0._ Example configuration: ```yaml @@ -3823,6 +3825,7 @@ server_notices: system_mxid_display_name: "Server Notices" system_mxid_avatar_url: "mxc://server.com/oumMVlgDnLYFaPVkExemNVVZ" room_name: "Server Notices" + auto_join: true ``` --- ### `enable_room_list_search` diff --git a/synapse/config/server_notices.py b/synapse/config/server_notices.py index ce041abe9b..a8badba0f8 100644 --- a/synapse/config/server_notices.py +++ b/synapse/config/server_notices.py @@ -48,6 +48,7 @@ class ServerNoticesConfig(Config): self.server_notices_mxid_display_name: Optional[str] = None self.server_notices_mxid_avatar_url: Optional[str] = None self.server_notices_room_name: Optional[str] = None + self.server_notices_auto_join: bool = False def read_config(self, config: JsonDict, **kwargs: Any) -> None: c = config.get("server_notices") @@ -62,3 +63,4 @@ class ServerNoticesConfig(Config): self.server_notices_mxid_avatar_url = c.get("system_mxid_avatar_url", None) # todo: i18n self.server_notices_room_name = c.get("room_name", "Server Notices") + self.server_notices_auto_join = c.get("auto_join", False) diff --git a/synapse/server_notices/server_notices_manager.py b/synapse/server_notices/server_notices_manager.py index 44b999677a..2353b5d47f 100644 --- a/synapse/server_notices/server_notices_manager.py +++ b/synapse/server_notices/server_notices_manager.py @@ -224,14 +224,27 @@ class ServerNoticesManager: if room.room_id == room_id: return + user_id_obj = UserID.from_string(user_id) await self._room_member_handler.update_membership( requester=requester, - target=UserID.from_string(user_id), + target=user_id_obj, room_id=room_id, action="invite", ratelimit=False, ) + if self._config.servernotices.server_notices_auto_join: + user_requester = create_requester( + user_id, authenticated_entity=self._server_name + ) + await self._room_member_handler.update_membership( + requester=user_requester, + target=user_id_obj, + room_id=room_id, + action="join", + ratelimit=False, + ) + async def _update_notice_user_profile_if_changed( self, requester: Requester, diff --git a/tests/rest/admin/test_server_notice.py b/tests/rest/admin/test_server_notice.py index dfd14f5751..2398bc503a 100644 --- a/tests/rest/admin/test_server_notice.py +++ b/tests/rest/admin/test_server_notice.py @@ -477,6 +477,33 @@ class ServerNoticeTestCase(unittest.HomeserverTestCase): # second room has new ID self.assertNotEqual(first_room_id, second_room_id) + @override_config( + {"server_notices": {"system_mxid_localpart": "notices", "auto_join": True}} + ) + def test_auto_join(self) -> None: + """ + Tests that the user get automatically joined to the notice room + when `auto_join` setting is used. + """ + # user has no room memberships + self._check_invite_and_join_status(self.other_user, 0, 0) + + # send server notice + server_notice_request_content = { + "user_id": self.other_user, + "content": {"msgtype": "m.text", "body": "test msg one"}, + } + + self.make_request( + "POST", + self.url, + access_token=self.admin_user_tok, + content=server_notice_request_content, + ) + + # user has joined the room + self._check_invite_and_join_status(self.other_user, 0, 1) + @override_config({"server_notices": {"system_mxid_localpart": "notices"}}) def test_update_notice_user_name_when_changed(self) -> None: """ -- cgit 1.5.1