From 82f227b35fd7c8f09f85c845074b600c2638b95a Mon Sep 17 00:00:00 2001 From: Till Faelligen <2353100+S7evinK@users.noreply.github.com> Date: Thu, 11 Apr 2024 12:48:07 +0200 Subject: [PATCH 1/9] Redact membership events to remove displayname and avatar_url --- synapse/handlers/deactivate_account.py | 41 +++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/deactivate_account.py b/synapse/handlers/deactivate_account.py index b13c4b6cb93..e301bb41667 100644 --- a/synapse/handlers/deactivate_account.py +++ b/synapse/handlers/deactivate_account.py @@ -22,11 +22,14 @@ import logging from typing import TYPE_CHECKING, Optional -from synapse.api.constants import Membership +from synapse.api.constants import Membership, EventTypes, Direction from synapse.api.errors import SynapseError from synapse.handlers.device import DeviceHandler from synapse.metrics.background_process_metrics import run_as_background_process from synapse.types import Codes, Requester, UserID, create_requester +from synapse.api.filtering import Filter +from immutabledict import immutabledict +from synapse.streams.config import PaginationConfig if TYPE_CHECKING: from synapse.server import HomeServer @@ -48,6 +51,9 @@ def __init__(self, hs: "HomeServer"): self.user_directory_handler = hs.get_user_directory_handler() self._server_name = hs.hostname self._third_party_rules = hs.get_module_api_callbacks().third_party_event_rules + # Used to get + self._pagination_handler = hs.get_pagination_handler() + self._event_creation_handler = hs.get_event_creation_handler() # Flag that indicates whether the process to part users from rooms is running self._user_parter_running = False @@ -260,9 +266,42 @@ async def _part_user(self, user_id: str) -> None: """Causes the given user_id to leave all the rooms they're joined to""" user = UserID.from_string(user_id) + # Get all membership events for this user rooms_for_user = await self.store.get_rooms_for_user(user_id) + + filter = Filter(self.hs, {}) + filter.senders = [user_id] + filter.types = [EventTypes.Member] + filter.limit = 1000 + + pagination = PaginationConfig(direction=Direction.FORWARDS, limit=1000, from_token=None, to_token=None) + + logger.info("EventFilter: %r", filter) + requester = create_requester(user_id) + for room_id in rooms_for_user: logger.info("User parter parting %r from %r", user_id, room_id) + + filter.rooms = [room_id] + # TODO: Use something else to get the membership events + events = await self._pagination_handler.get_messages(requester=requester, room_id=room_id, event_filter=filter, use_admin_priviledge=False, pagin_config=pagination) + + # TODO: Check if there are more events + for event in events["chunk"]: + has_profile = "displayname" in event["content"] or "avatar_url" in event["content"] + if has_profile and "redacted_because" not in event["unsigned"]: + event_dict = { + "type": EventTypes.Redaction, + "content": { + "redacts": event["event_id"], # room version 11 + }, + "redacts": event["event_id"], # earlier room version + "room_id": event["room_id"], + "sender": requester.user.to_string(), + } + logger.info("sending redaction event") + await self._event_creation_handler.create_event(requester, event_dict) + try: await self._room_member_handler.update_membership( create_requester(user, authenticated_entity=self._server_name), From e2300cf60ac8da04b31ed46793c46a483df74aaf Mon Sep 17 00:00:00 2001 From: Till Faelligen <2353100+S7evinK@users.noreply.github.com> Date: Thu, 11 Apr 2024 15:54:54 +0200 Subject: [PATCH 2/9] Add query to get all membership events for a given user and room --- synapse/storage/databases/main/roommember.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 5d515025950..d28fa27a857 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -1234,6 +1234,26 @@ async def get_rooms_user_has_been_in(self, user_id: str) -> Set[str]: return set(room_ids) + async def get_membership_event_ids_for_user(self, user_id: str, room_id: str) -> Set[str]: + """Get all event_ids for the given user and room. + + Args: + user_id: The user ID to get the event IDs for. + room_id: The room ID to look up events for. + + Returns: + Set of event IDs + """ + + event_ids = await self.db_pool.simple_select_onecol( + table="room_memberships", + keyvalues={"user_id": user_id, "room_id": room_id}, + retcol="event_id", + desc="get_membership_event_ids_for_user" + ) + + return set(event_ids) + @cached(max_entries=5000) async def _get_membership_from_event_id( self, member_event_id: str From 6f8077dce02785353c6c9e01561d485584bb67e9 Mon Sep 17 00:00:00 2001 From: Till Faelligen <2353100+S7evinK@users.noreply.github.com> Date: Thu, 11 Apr 2024 15:55:26 +0200 Subject: [PATCH 3/9] Don't use get_messages to get the events to redact --- synapse/handlers/deactivate_account.py | 66 ++++++++++---------------- 1 file changed, 26 insertions(+), 40 deletions(-) diff --git a/synapse/handlers/deactivate_account.py b/synapse/handlers/deactivate_account.py index e301bb41667..9261f5e0bdf 100644 --- a/synapse/handlers/deactivate_account.py +++ b/synapse/handlers/deactivate_account.py @@ -22,14 +22,12 @@ import logging from typing import TYPE_CHECKING, Optional -from synapse.api.constants import Membership, EventTypes, Direction +from synapse.api.constants import Membership, EventTypes from synapse.api.errors import SynapseError from synapse.handlers.device import DeviceHandler from synapse.metrics.background_process_metrics import run_as_background_process from synapse.types import Codes, Requester, UserID, create_requester -from synapse.api.filtering import Filter -from immutabledict import immutabledict -from synapse.streams.config import PaginationConfig +from synapse.events import EventBase if TYPE_CHECKING: from synapse.server import HomeServer @@ -51,8 +49,6 @@ def __init__(self, hs: "HomeServer"): self.user_directory_handler = hs.get_user_directory_handler() self._server_name = hs.hostname self._third_party_rules = hs.get_module_api_callbacks().third_party_event_rules - # Used to get - self._pagination_handler = hs.get_pagination_handler() self._event_creation_handler = hs.get_event_creation_handler() # Flag that indicates whether the process to part users from rooms is running @@ -266,45 +262,35 @@ async def _part_user(self, user_id: str) -> None: """Causes the given user_id to leave all the rooms they're joined to""" user = UserID.from_string(user_id) - # Get all membership events for this user rooms_for_user = await self.store.get_rooms_for_user(user_id) - - filter = Filter(self.hs, {}) - filter.senders = [user_id] - filter.types = [EventTypes.Member] - filter.limit = 1000 - - pagination = PaginationConfig(direction=Direction.FORWARDS, limit=1000, from_token=None, to_token=None) - - logger.info("EventFilter: %r", filter) - requester = create_requester(user_id) + requester = create_requester(user, authenticated_entity=self._server_name) + should_erase = await self.store.is_user_erased(user_id) for room_id in rooms_for_user: - logger.info("User parter parting %r from %r", user_id, room_id) - - filter.rooms = [room_id] - # TODO: Use something else to get the membership events - events = await self._pagination_handler.get_messages(requester=requester, room_id=room_id, event_filter=filter, use_admin_priviledge=False, pagin_config=pagination) - - # TODO: Check if there are more events - for event in events["chunk"]: - has_profile = "displayname" in event["content"] or "avatar_url" in event["content"] - if has_profile and "redacted_because" not in event["unsigned"]: - event_dict = { - "type": EventTypes.Redaction, - "content": { - "redacts": event["event_id"], # room version 11 - }, - "redacts": event["event_id"], # earlier room version - "room_id": event["room_id"], - "sender": requester.user.to_string(), - } - logger.info("sending redaction event") - await self._event_creation_handler.create_event(requester, event_dict) - try: + # Before parting the user, redact all membership events if requested + if should_erase: + event_ids = await self.store.get_membership_event_ids_for_user( + user_id, room_id) + events: list[EventBase] = await self.store.get_events_as_list( + event_ids) + for event in events: + has_profile = "displayname" in event.content or "avatar_url" in event.content + if has_profile and "redacted_because" not in event.unsigned: + event_dict = { + "type": EventTypes.Redaction, + "room_id": event["room_id"], + "sender": requester.user.to_string(), + } + if event.room_version.updated_redaction_rules: + event_dict["content"]["redacts"] = event.event_id + else: + event_dict["redacts"] = event.event_id + await self._event_creation_handler.create_and_send_nonmember_event( + requester, event_dict, ratelimit=False) + await self._room_member_handler.update_membership( - create_requester(user, authenticated_entity=self._server_name), + requester, user, room_id, "leave", From 54d17baad85f71cd19081466c62adc5531aeb5da Mon Sep 17 00:00:00 2001 From: Till Faelligen <2353100+S7evinK@users.noreply.github.com> Date: Fri, 12 Apr 2024 11:26:38 +0200 Subject: [PATCH 4/9] Run the linters --- synapse/handlers/deactivate_account.py | 20 +++++++++++++------- synapse/storage/databases/main/roommember.py | 6 ++++-- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/synapse/handlers/deactivate_account.py b/synapse/handlers/deactivate_account.py index 9261f5e0bdf..575b7ebd76c 100644 --- a/synapse/handlers/deactivate_account.py +++ b/synapse/handlers/deactivate_account.py @@ -22,12 +22,12 @@ import logging from typing import TYPE_CHECKING, Optional -from synapse.api.constants import Membership, EventTypes +from synapse.api.constants import EventTypes, Membership from synapse.api.errors import SynapseError +from synapse.events import EventBase from synapse.handlers.device import DeviceHandler from synapse.metrics.background_process_metrics import run_as_background_process from synapse.types import Codes, Requester, UserID, create_requester -from synapse.events import EventBase if TYPE_CHECKING: from synapse.server import HomeServer @@ -271,11 +271,16 @@ async def _part_user(self, user_id: str) -> None: # Before parting the user, redact all membership events if requested if should_erase: event_ids = await self.store.get_membership_event_ids_for_user( - user_id, room_id) + user_id, room_id + ) events: list[EventBase] = await self.store.get_events_as_list( - event_ids) + event_ids + ) for event in events: - has_profile = "displayname" in event.content or "avatar_url" in event.content + has_profile = ( + "displayname" in event.content + or "avatar_url" in event.content + ) if has_profile and "redacted_because" not in event.unsigned: event_dict = { "type": EventTypes.Redaction, @@ -286,8 +291,9 @@ async def _part_user(self, user_id: str) -> None: event_dict["content"]["redacts"] = event.event_id else: event_dict["redacts"] = event.event_id - await self._event_creation_handler.create_and_send_nonmember_event( - requester, event_dict, ratelimit=False) + await self._event_creation_handler.create_event( + requester, event_dict, ratelimit=False + ) await self._room_member_handler.update_membership( requester, diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index d28fa27a857..9fddbb2caf4 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -1234,7 +1234,9 @@ async def get_rooms_user_has_been_in(self, user_id: str) -> Set[str]: return set(room_ids) - async def get_membership_event_ids_for_user(self, user_id: str, room_id: str) -> Set[str]: + async def get_membership_event_ids_for_user( + self, user_id: str, room_id: str + ) -> Set[str]: """Get all event_ids for the given user and room. Args: @@ -1249,7 +1251,7 @@ async def get_membership_event_ids_for_user(self, user_id: str, room_id: str) -> table="room_memberships", keyvalues={"user_id": user_id, "room_id": room_id}, retcol="event_id", - desc="get_membership_event_ids_for_user" + desc="get_membership_event_ids_for_user", ) return set(event_ids) From bd8ab2f06e16c970e96581364ab7b983e27601e3 Mon Sep 17 00:00:00 2001 From: Till Faelligen <2353100+S7evinK@users.noreply.github.com> Date: Fri, 12 Apr 2024 11:40:32 +0200 Subject: [PATCH 5/9] Make mypy happy --- synapse/handlers/deactivate_account.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/synapse/handlers/deactivate_account.py b/synapse/handlers/deactivate_account.py index 575b7ebd76c..b1184a7174b 100644 --- a/synapse/handlers/deactivate_account.py +++ b/synapse/handlers/deactivate_account.py @@ -24,7 +24,6 @@ from synapse.api.constants import EventTypes, Membership from synapse.api.errors import SynapseError -from synapse.events import EventBase from synapse.handlers.device import DeviceHandler from synapse.metrics.background_process_metrics import run_as_background_process from synapse.types import Codes, Requester, UserID, create_requester @@ -273,9 +272,7 @@ async def _part_user(self, user_id: str) -> None: event_ids = await self.store.get_membership_event_ids_for_user( user_id, room_id ) - events: list[EventBase] = await self.store.get_events_as_list( - event_ids - ) + events = await self.store.get_events_as_list(event_ids) for event in events: has_profile = ( "displayname" in event.content @@ -288,10 +285,16 @@ async def _part_user(self, user_id: str) -> None: "sender": requester.user.to_string(), } if event.room_version.updated_redaction_rules: - event_dict["content"]["redacts"] = event.event_id + event_dict.update( + { + "content": { + "redacts": event.event_id, + } + } + ) else: event_dict["redacts"] = event.event_id - await self._event_creation_handler.create_event( + await self._event_creation_handler.create_and_send_nonmember_event( requester, event_dict, ratelimit=False ) From bdcdae419efd62d35f7c10f6facaf8712c341232 Mon Sep 17 00:00:00 2001 From: Till Faelligen <2353100+S7evinK@users.noreply.github.com> Date: Fri, 12 Apr 2024 11:40:50 +0200 Subject: [PATCH 6/9] Add newsfile --- changelog.d/17076.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/17076.bugfix diff --git a/changelog.d/17076.bugfix b/changelog.d/17076.bugfix new file mode 100644 index 00000000000..a111ea2b886 --- /dev/null +++ b/changelog.d/17076.bugfix @@ -0,0 +1 @@ +Redact membership events if the user requested erasure upon deactivating. \ No newline at end of file From b13b36a9da0307d219dc2b93c8630bc301489d2c Mon Sep 17 00:00:00 2001 From: Till Faelligen <2353100+S7evinK@users.noreply.github.com> Date: Fri, 12 Apr 2024 14:11:30 +0200 Subject: [PATCH 7/9] Simplify redaction the event --- synapse/handlers/deactivate_account.py | 29 +++----------------------- 1 file changed, 3 insertions(+), 26 deletions(-) diff --git a/synapse/handlers/deactivate_account.py b/synapse/handlers/deactivate_account.py index b1184a7174b..a8be6931031 100644 --- a/synapse/handlers/deactivate_account.py +++ b/synapse/handlers/deactivate_account.py @@ -22,7 +22,7 @@ import logging from typing import TYPE_CHECKING, Optional -from synapse.api.constants import EventTypes, Membership +from synapse.api.constants import Membership from synapse.api.errors import SynapseError from synapse.handlers.device import DeviceHandler from synapse.metrics.background_process_metrics import run_as_background_process @@ -272,31 +272,8 @@ async def _part_user(self, user_id: str) -> None: event_ids = await self.store.get_membership_event_ids_for_user( user_id, room_id ) - events = await self.store.get_events_as_list(event_ids) - for event in events: - has_profile = ( - "displayname" in event.content - or "avatar_url" in event.content - ) - if has_profile and "redacted_because" not in event.unsigned: - event_dict = { - "type": EventTypes.Redaction, - "room_id": event["room_id"], - "sender": requester.user.to_string(), - } - if event.room_version.updated_redaction_rules: - event_dict.update( - { - "content": { - "redacts": event.event_id, - } - } - ) - else: - event_dict["redacts"] = event.event_id - await self._event_creation_handler.create_and_send_nonmember_event( - requester, event_dict, ratelimit=False - ) + for event_id in event_ids: + await self.store.expire_event(event_id) await self._room_member_handler.update_membership( requester, From a5da1e4b80cead150fc318c0f71955d7a9662716 Mon Sep 17 00:00:00 2001 From: Till Faelligen <2353100+S7evinK@users.noreply.github.com> Date: Wed, 17 Apr 2024 09:34:53 +0200 Subject: [PATCH 8/9] Restore log message, remove unused event creation handler --- synapse/handlers/deactivate_account.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/deactivate_account.py b/synapse/handlers/deactivate_account.py index a8be6931031..11ac3776803 100644 --- a/synapse/handlers/deactivate_account.py +++ b/synapse/handlers/deactivate_account.py @@ -48,7 +48,6 @@ def __init__(self, hs: "HomeServer"): self.user_directory_handler = hs.get_user_directory_handler() self._server_name = hs.hostname self._third_party_rules = hs.get_module_api_callbacks().third_party_event_rules - self._event_creation_handler = hs.get_event_creation_handler() # Flag that indicates whether the process to part users from rooms is running self._user_parter_running = False @@ -266,6 +265,7 @@ async def _part_user(self, user_id: str) -> None: should_erase = await self.store.is_user_erased(user_id) for room_id in rooms_for_user: + logger.info("User parter parting %r from %r", user_id, room_id) try: # Before parting the user, redact all membership events if requested if should_erase: From 6d2053d883799c258c7720deff6675d33cf17c46 Mon Sep 17 00:00:00 2001 From: Till Faelligen <2353100+S7evinK@users.noreply.github.com> Date: Wed, 17 Apr 2024 09:43:05 +0200 Subject: [PATCH 9/9] Add test --- tests/handlers/test_deactivate_account.py | 37 +++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/handlers/test_deactivate_account.py b/tests/handlers/test_deactivate_account.py index b3f9e50f0f3..c698771a063 100644 --- a/tests/handlers/test_deactivate_account.py +++ b/tests/handlers/test_deactivate_account.py @@ -424,3 +424,40 @@ def test_deactivate_account_rejects_knocks(self) -> None: self._store.get_knocked_at_rooms_for_local_user(self.user) ) self.assertEqual(len(after_deactivate_knocks), 0) + + def test_membership_is_redacted_upon_deactivation(self) -> None: + """ + Tests that room membership events are redacted if erasure is requested. + """ + # Create a room + room_id = self.helper.create_room_as( + self.user, + is_public=True, + tok=self.token, + ) + + # Change the displayname + membership_event, _ = self.get_success( + self.handler.update_membership( + requester=create_requester(self.user), + target=UserID.from_string(self.user), + room_id=room_id, + action=Membership.JOIN, + content={"displayname": "Hello World!"}, + ) + ) + + # Deactivate the account + self._deactivate_my_account() + + # Get the all membership event IDs + membership_event_ids = self.get_success( + self._store.get_membership_event_ids_for_user(self.user, room_id=room_id) + ) + + # Get the events incl. JSON + events = self.get_success(self._store.get_events_as_list(membership_event_ids)) + + # Validate that there is no displayname in any of the events + for event in events: + self.assertTrue("displayname" not in event.content)