From 353dc6c62c2813f24b63d7f6caec90efe4c948de Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 21 Oct 2021 14:32:28 +0100 Subject: [PATCH 01/15] Add a ModuleApi method to update a user's membership in a room --- synapse/module_api/__init__.py | 57 +++++++++++++++++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index ab7ef8f950bd..6d99bf161ce4 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -33,6 +33,7 @@ from twisted.internet import defer from twisted.web.resource import IResource +from synapse.api.constants import Membership from synapse.events import EventBase from synapse.events.presence_router import PresenceRouter from synapse.http.client import SimpleHttpClient @@ -560,8 +561,62 @@ def get_state_events_in_room( state = yield defer.ensureDeferred(self._store.get_events(state_ids.values())) return state.values() + async def update_room_membership( + self, + sender: str, + target: str, + room_id: str, + new_membership: str, + content: Optional[dict] = None, + ) -> EventBase: + """Updates the membership of a user to the given value. + + Added in Synapse v1.46.0. + + Args: + sender: The user performing the membership change. + target: The user whose membership is changing. This is often the same value + as `sender`, but it might differ in some cases (e.g. when kicking a user, + the `sender` is the user performing the kick and the `target` is the user + being kicked). + room_id: The room in which to change the membership. + new_membership: The new membership state of `target` after this operation. See + https://spec.matrix.org/unstable/client-server-api/#mroommember for the + list of allowed values. + content: Additional values to include in the resulting event's content. + + Returns: + The newly created membership event. + + Raises: + RuntimeError if the resulting event could not be found after performing the + membership event. + ShadowBanError if a shadow-banned requester attempts to send an invite. + SynapseError if the module attempts to send a membership event that isn't + allowed, either by the server's configuration (e.g. trying to set a + per-room display name that's too long) or by the validation rules around + membership updates (e.g. the `membership` value is invalid). + """ + event_id, _ = await self._hs.get_room_member_handler().update_membership( + requester=create_requester(sender), + target=UserID.from_string(target), + room_id=room_id, + action=new_membership, + content=content, + ) + + # Try to retrieve the resulting event. + event = await self._hs.get_datastore().get_event(event_id) + if event is None: + raise RuntimeError("Could not find resulting membership event %s" % event_id) + + return event + async def create_and_send_event_into_room(self, event_dict: JsonDict) -> EventBase: - """Create and send an event into a room. Membership events are currently not supported. + """Create and send an event into a room. + + Membership events are not supported by this method. To update a user's membership + in a room, please use the `update_room_membership` method instead. Args: event_dict: A dictionary representing the event to send. From 8915ed316324fd5c47bad02aff027399202b1b65 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 21 Oct 2021 14:36:04 +0100 Subject: [PATCH 02/15] Changelog --- changelog.d/11147.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/11147.feature diff --git a/changelog.d/11147.feature b/changelog.d/11147.feature new file mode 100644 index 000000000000..1669ad218588 --- /dev/null +++ b/changelog.d/11147.feature @@ -0,0 +1 @@ +Add a ModuleApi method to update a user's membership in a room. From 0500af2f95ed5d3964540e46eb5907c5c6fd4b69 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 21 Oct 2021 14:46:48 +0100 Subject: [PATCH 03/15] Lint --- synapse/module_api/__init__.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 6d99bf161ce4..c2601bcf5eba 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -33,7 +33,6 @@ from twisted.internet import defer from twisted.web.resource import IResource -from synapse.api.constants import Membership from synapse.events import EventBase from synapse.events.presence_router import PresenceRouter from synapse.http.client import SimpleHttpClient @@ -608,7 +607,9 @@ async def update_room_membership( # Try to retrieve the resulting event. event = await self._hs.get_datastore().get_event(event_id) if event is None: - raise RuntimeError("Could not find resulting membership event %s" % event_id) + raise RuntimeError( + "Could not find resulting membership event %s" % event_id + ) return event From 9bda37ed032c1d419ab0a6b798e58d82589ca6de Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 21 Oct 2021 14:58:40 +0100 Subject: [PATCH 04/15] Add a test --- tests/module_api/test_api.py | 47 ++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tests/module_api/test_api.py b/tests/module_api/test_api.py index e915dd5c7cd2..4b390bd37ef8 100644 --- a/tests/module_api/test_api.py +++ b/tests/module_api/test_api.py @@ -385,6 +385,53 @@ def test_send_local_online_presence_to_federation(self): self.assertTrue(found_update) + def test_update_membership(self): + """Tests that the module API can update the membership of a user in a room.""" + peter = self.register_user("peter", "hackme") + lesley = self.register_user("lesley", "hackme") + tok = self.login("peter", "hackme") + + # Make peter create a public room. + room_id = self.helper.create_room_as( + room_creator=peter, + is_public=True, + tok=tok + ) + + # Make lesley join it. + self.get_success( + defer.ensureDeferred( + self.module_api.update_room_membership(lesley, lesley, room_id, "join") + ) + ) + + # Check that the membership of lesley in the room is "join". + res = self.helper.get_state( + room_id=room_id, + event_type="m.room.member", + state_key=lesley, + tok=tok, + ) + + self.assertEqual(res["membership"], "join") + + # Make peter kick lesley from the room. + self.get_success( + defer.ensureDeferred( + self.module_api.update_room_membership(peter, lesley, room_id, "leave") + ) + ) + + # Check that the membership of lesley in the room is "leave". + res = self.helper.get_state( + room_id=room_id, + event_type="m.room.member", + state_key=lesley, + tok=tok, + ) + + self.assertEqual(res["membership"], "leave") + class ModuleApiWorkerTestCase(BaseMultiWorkerStreamTestCase): """For testing ModuleApi functionality in a multi-worker setup""" From 81885024e3a2b1fef89a31348d5b97456c88188d Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 21 Oct 2021 15:03:55 +0100 Subject: [PATCH 05/15] Only allow local users to send membership updates --- synapse/module_api/__init__.py | 12 +++++++++--- tests/module_api/test_api.py | 9 +++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index c2601bcf5eba..1a0643dd3b39 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -573,7 +573,8 @@ async def update_room_membership( Added in Synapse v1.46.0. Args: - sender: The user performing the membership change. + sender: The user performing the membership change. Must be a user local to + this homeserver. target: The user whose membership is changing. This is often the same value as `sender`, but it might differ in some cases (e.g. when kicking a user, the `sender` is the user performing the kick and the `target` is the user @@ -588,14 +589,19 @@ async def update_room_membership( The newly created membership event. Raises: - RuntimeError if the resulting event could not be found after performing the - membership event. + RuntimeError if the `sender` isn't a local user, or the resulting event could + not be found after performing the membership event. ShadowBanError if a shadow-banned requester attempts to send an invite. SynapseError if the module attempts to send a membership event that isn't allowed, either by the server's configuration (e.g. trying to set a per-room display name that's too long) or by the validation rules around membership updates (e.g. the `membership` value is invalid). """ + if not self.is_mine(sender): + raise RuntimeError( + "Tried to send an event as a user that isn't local to this homeserver", + ) + event_id, _ = await self._hs.get_room_member_handler().update_membership( requester=create_requester(sender), target=UserID.from_string(target), diff --git a/tests/module_api/test_api.py b/tests/module_api/test_api.py index 4b390bd37ef8..4d16f398a7cd 100644 --- a/tests/module_api/test_api.py +++ b/tests/module_api/test_api.py @@ -432,6 +432,15 @@ def test_update_membership(self): self.assertEqual(res["membership"], "leave") + # Try to send a membership update from a non-local user and check that it fails. + d = defer.ensureDeferred( + self.module_api.update_room_membership( + "@nicolas:otherserver.com", lesley, room_id, "invite", + ) + ) + + self.get_failure(d, RuntimeError) + class ModuleApiWorkerTestCase(BaseMultiWorkerStreamTestCase): """For testing ModuleApi functionality in a multi-worker setup""" From ff02a1b0e554b7f3806abaa7151e4a46e9a81dfa Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 21 Oct 2021 15:06:07 +0100 Subject: [PATCH 06/15] Lint --- tests/module_api/test_api.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/module_api/test_api.py b/tests/module_api/test_api.py index 4d16f398a7cd..58717d74563c 100644 --- a/tests/module_api/test_api.py +++ b/tests/module_api/test_api.py @@ -393,9 +393,7 @@ def test_update_membership(self): # Make peter create a public room. room_id = self.helper.create_room_as( - room_creator=peter, - is_public=True, - tok=tok + room_creator=peter, is_public=True, tok=tok ) # Make lesley join it. @@ -435,7 +433,10 @@ def test_update_membership(self): # Try to send a membership update from a non-local user and check that it fails. d = defer.ensureDeferred( self.module_api.update_room_membership( - "@nicolas:otherserver.com", lesley, room_id, "invite", + "@nicolas:otherserver.com", + lesley, + room_id, + "invite", ) ) From 32a74a338377439ac559c0c10049d0f64127ea2c Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 22 Oct 2021 17:28:52 +0200 Subject: [PATCH 07/15] Update changelog.d/11147.feature Co-authored-by: reivilibre --- changelog.d/11147.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/11147.feature b/changelog.d/11147.feature index 1669ad218588..af72d85c2059 100644 --- a/changelog.d/11147.feature +++ b/changelog.d/11147.feature @@ -1 +1 @@ -Add a ModuleApi method to update a user's membership in a room. +Add a module API method to update a user's membership in a room. From 943e22a5a8cde5bc802bdc120689db747e1f9df3 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 22 Oct 2021 17:17:10 +0100 Subject: [PATCH 08/15] Incorporate review --- synapse/module_api/__init__.py | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 1a0643dd3b39..b4d092fdd596 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -566,7 +566,7 @@ async def update_room_membership( target: str, room_id: str, new_membership: str, - content: Optional[dict] = None, + content: Optional[JsonDict] = None, ) -> EventBase: """Updates the membership of a user to the given value. @@ -589,8 +589,7 @@ async def update_room_membership( The newly created membership event. Raises: - RuntimeError if the `sender` isn't a local user, or the resulting event could - not be found after performing the membership event. + RuntimeError if the `sender` isn't a local user. ShadowBanError if a shadow-banned requester attempts to send an invite. SynapseError if the module attempts to send a membership event that isn't allowed, either by the server's configuration (e.g. trying to set a @@ -602,6 +601,18 @@ async def update_room_membership( "Tried to send an event as a user that isn't local to this homeserver", ) + if content is None: + content = {} + + # Set the profile if not already done by the module. + if "avatar_url" not in content: + content["avatar_url"] = self._hs.get_profile_handler().get_avatar_url(target) + + if "displayname" not in content: + content["displayname"] = self._hs.get_profile_handler().get_displayname( + target, + ) + event_id, _ = await self._hs.get_room_member_handler().update_membership( requester=create_requester(sender), target=UserID.from_string(target), @@ -612,10 +623,10 @@ async def update_room_membership( # Try to retrieve the resulting event. event = await self._hs.get_datastore().get_event(event_id) - if event is None: - raise RuntimeError( - "Could not find resulting membership event %s" % event_id - ) + + # update_membership is supposed to always return after the event has been + # successfully persisted. + assert event is not None return event From 025b11ff77905fb69a07743205b744239e64396a Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 22 Oct 2021 17:22:11 +0100 Subject: [PATCH 09/15] Lint --- synapse/module_api/__init__.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index b4d092fdd596..f88f85e2c2d9 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -601,21 +601,26 @@ async def update_room_membership( "Tried to send an event as a user that isn't local to this homeserver", ) + requester = create_requester(sender) + target_user_id = UserID.from_string(target) + if content is None: content = {} # Set the profile if not already done by the module. if "avatar_url" not in content: - content["avatar_url"] = self._hs.get_profile_handler().get_avatar_url(target) + content["avatar_url"] = self._hs.get_profile_handler().get_avatar_url( + requester.user, + ) if "displayname" not in content: content["displayname"] = self._hs.get_profile_handler().get_displayname( - target, + target_user_id, ) event_id, _ = await self._hs.get_room_member_handler().update_membership( - requester=create_requester(sender), - target=UserID.from_string(target), + requester=requester, + target=target_user_id, room_id=room_id, action=new_membership, content=content, From d2e6f3d7b8f783de47ad29b6e826011cdd7419d9 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 22 Oct 2021 17:40:48 +0100 Subject: [PATCH 10/15] Await coroutines --- synapse/module_api/__init__.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index f88f85e2c2d9..88a11c61e73f 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -609,12 +609,14 @@ async def update_room_membership( # Set the profile if not already done by the module. if "avatar_url" not in content: - content["avatar_url"] = self._hs.get_profile_handler().get_avatar_url( + content["avatar_url"] = await self._hs.get_profile_handler().get_avatar_url( requester.user, ) if "displayname" not in content: - content["displayname"] = self._hs.get_profile_handler().get_displayname( + content[ + "displayname" + ] = await self._hs.get_profile_handler().get_displayname( target_user_id, ) From ce559d8d91fcd6cc5ffb4a1bca236ceb44762ecf Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 25 Oct 2021 14:50:18 +0100 Subject: [PATCH 11/15] Test that the module API is pre-filling the profile --- tests/module_api/test_api.py | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/tests/module_api/test_api.py b/tests/module_api/test_api.py index 58717d74563c..b794b01970b2 100644 --- a/tests/module_api/test_api.py +++ b/tests/module_api/test_api.py @@ -20,7 +20,7 @@ from synapse.federation.units import Transaction from synapse.handlers.presence import UserPresenceState from synapse.rest import admin -from synapse.rest.client import login, presence, room +from synapse.rest.client import login, presence, profile, room from synapse.types import create_requester from tests.events.test_presence_router import send_presence_update, sync_presence @@ -37,6 +37,7 @@ class ModuleApiTestCase(HomeserverTestCase): login.register_servlets, room.register_servlets, presence.register_servlets, + profile.register_servlets, ] def prepare(self, reactor, clock, homeserver): @@ -390,12 +391,32 @@ def test_update_membership(self): peter = self.register_user("peter", "hackme") lesley = self.register_user("lesley", "hackme") tok = self.login("peter", "hackme") + lesley_tok = self.login("lesley", "hackme") # Make peter create a public room. room_id = self.helper.create_room_as( room_creator=peter, is_public=True, tok=tok ) + # Set a profile for lesley. + channel = self.make_request( + method="PUT", + path="/_matrix/client/r0/profile/%s/displayname" % lesley, + content={"displayname": "Lesley May"}, + access_token=lesley_tok, + ) + + self.assertEqual(channel.code, 200, channel.result) + + channel = self.make_request( + method="PUT", + path="/_matrix/client/r0/profile/%s/avatar_url" % lesley, + content={"avatar_url": "some_url"}, + access_token=lesley_tok, + ) + + self.assertEqual(channel.code, 200, channel.result) + # Make lesley join it. self.get_success( defer.ensureDeferred( @@ -413,6 +434,10 @@ def test_update_membership(self): self.assertEqual(res["membership"], "join") + # Also check that the profile was correctly filled out. + self.assertEqual(res["displayname"], "Lesley May") + self.assertEqual(res["avatar_url"], "some_url") + # Make peter kick lesley from the room. self.get_success( defer.ensureDeferred( From d47fff202738e3c774265c0805d8be465743b9b2 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 28 Oct 2021 13:25:58 +0100 Subject: [PATCH 12/15] Fetch whole profile, and the right one --- synapse/module_api/__init__.py | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 6745d80c4a26..fa4e811ef7d7 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -33,6 +33,7 @@ from twisted.internet import defer from twisted.web.resource import IResource +from synapse.api.errors import SynapseError from synapse.events import EventBase from synapse.events.presence_router import PresenceRouter from synapse.http.client import SimpleHttpClient @@ -673,17 +674,28 @@ async def update_room_membership( content = {} # Set the profile if not already done by the module. - if "avatar_url" not in content: - content["avatar_url"] = await self._hs.get_profile_handler().get_avatar_url( - requester.user, - ) - - if "displayname" not in content: - content[ - "displayname" - ] = await self._hs.get_profile_handler().get_displayname( - target_user_id, - ) + if "avatar_url" not in content or "displayname" not in content: + try: + # Try to fetch the user's profile. + profile = await self._hs.get_profile_handler().get_profile( + target_user_id.to_string(), + ) + except SynapseError as e: + if e.code == 404: + # If the profile couldn't be found, use default values. + profile = { + "displayname": target_user_id.localpart, + "avatar_url": None, + } + else: + raise + + # Set the profile where it needs to be set. + if "avatar_url" not in content: + content["avatar_url"] = profile["avatar_url"] + + if "displayname" not in content: + content["displayname"] = profile["displayname"] event_id, _ = await self._hs.get_room_member_handler().update_membership( requester=requester, From 555d59581223dd3ef74878c871d68bf2e93da7fe Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 28 Oct 2021 13:26:17 +0100 Subject: [PATCH 13/15] Test invites, and users with no profile --- tests/module_api/test_api.py | 42 ++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/module_api/test_api.py b/tests/module_api/test_api.py index b794b01970b2..37852852a82e 100644 --- a/tests/module_api/test_api.py +++ b/tests/module_api/test_api.py @@ -417,6 +417,28 @@ def test_update_membership(self): self.assertEqual(channel.code, 200, channel.result) + # Make Peter invite Lesley to the room. + self.get_success( + defer.ensureDeferred( + self.module_api.update_room_membership(peter, lesley, room_id, "invite") + ) + ) + + res = self.helper.get_state( + room_id=room_id, + event_type="m.room.member", + state_key=lesley, + tok=tok, + ) + + # Check the membership is correct. + self.assertEqual(res["membership"], "invite") + + # Also check that the profile was correctly filled out, and that it's not + # Peter's. + self.assertEqual(res["displayname"], "Lesley May") + self.assertEqual(res["avatar_url"], "some_url") + # Make lesley join it. self.get_success( defer.ensureDeferred( @@ -467,6 +489,26 @@ def test_update_membership(self): self.get_failure(d, RuntimeError) + # Check that inviting a user that doesn't have a profile falls back to using a + # default (localpart + no avatar) profile. + simone = "@simone:" + self.hs.config.server.server_name + self.get_success( + defer.ensureDeferred( + self.module_api.update_room_membership(peter, simone, room_id, "invite") + ) + ) + + res = self.helper.get_state( + room_id=room_id, + event_type="m.room.member", + state_key=simone, + tok=tok, + ) + + self.assertEqual(res["membership"], "invite") + self.assertEqual(res["displayname"], "simone") + self.assertIsNone(res["avatar_url"]) + class ModuleApiWorkerTestCase(BaseMultiWorkerStreamTestCase): """For testing ModuleApi functionality in a multi-worker setup""" From f1d72eee2ee61a532a5535628d218aa0a2453763 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 28 Oct 2021 14:28:09 +0100 Subject: [PATCH 14/15] Incorporate review --- synapse/module_api/__init__.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index fa4e811ef7d7..4d8da0ad55cb 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -681,14 +681,20 @@ async def update_room_membership( target_user_id.to_string(), ) except SynapseError as e: - if e.code == 404: - # If the profile couldn't be found, use default values. - profile = { - "displayname": target_user_id.localpart, - "avatar_url": None, - } - else: - raise + # If the profile couldn't be found, use default values. + profile = { + "displayname": target_user_id.localpart, + "avatar_url": None, + } + + if e.code != 404: + # If the error isn't 404, it means we tried to fetch the profile over + # federation but the remote server responded with a non-standard + # status code. + logger.error( + "Got non-404 error status when fetching profile for %s" + % target_user_id.to_string(), + ) # Set the profile where it needs to be set. if "avatar_url" not in content: From 215e6fa970cbdbc0187788104dff4ae5e8155a0a Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 28 Oct 2021 17:49:33 +0200 Subject: [PATCH 15/15] Update synapse/module_api/__init__.py Co-authored-by: reivilibre --- synapse/module_api/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 4d8da0ad55cb..36042ed2e05c 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -692,8 +692,8 @@ async def update_room_membership( # federation but the remote server responded with a non-standard # status code. logger.error( - "Got non-404 error status when fetching profile for %s" - % target_user_id.to_string(), + "Got non-404 error status when fetching profile for %s", + target_user_id.to_string(), ) # Set the profile where it needs to be set.