From 4cc464da46b631d5d064c884c6d5054514ee0b9d Mon Sep 17 00:00:00 2001 From: hanadi92 Date: Mon, 18 Mar 2024 16:51:57 +0300 Subject: [PATCH 1/4] fix: reject pending knocks on deactivating account --- synapse/handlers/deactivate_account.py | 24 +++--- synapse/storage/databases/main/roommember.py | 16 ++++ tests/handlers/test_deactivate_account.py | 87 +++++++++++++++++++- 3 files changed, 115 insertions(+), 12 deletions(-) diff --git a/synapse/handlers/deactivate_account.py b/synapse/handlers/deactivate_account.py index de5ed5d5b63..c09c7ebe6e4 100644 --- a/synapse/handlers/deactivate_account.py +++ b/synapse/handlers/deactivate_account.py @@ -21,6 +21,7 @@ import logging from typing import TYPE_CHECKING, Optional +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 @@ -168,9 +169,9 @@ async def deactivate_account( # parts users from rooms (if it isn't already running) self._start_user_parting() - # Reject all pending invites for the user, so that the user doesn't show up in the - # "invited" section of rooms' members list. - await self._reject_pending_invites_for_user(user_id) + # Reject all pending invites and knocks for the user, so that the + # user doesn't show up in the "invited" section of rooms' members list. + await self._reject_pending_invites_and_knocks_for_user(user_id) # Remove all information on the user from the account_validity table. if self._account_validity_enabled: @@ -194,34 +195,37 @@ async def deactivate_account( return identity_server_supports_unbinding - async def _reject_pending_invites_for_user(self, user_id: str) -> None: - """Reject pending invites addressed to a given user ID. + async def _reject_pending_invites_and_knocks_for_user(self, user_id: str) -> None: + """Reject pending invites and knocks addressed to a given user ID. Args: - user_id: The user ID to reject pending invites for. + user_id: The user ID to reject pending invites and knocks for. """ user = UserID.from_string(user_id) pending_invites = await self.store.get_invited_rooms_for_local_user(user_id) + pending_knocks = await self.store.get_knocked_at_rooms_for_local_user(user_id) - for room in pending_invites: + for room in pending_invites + pending_knocks: try: await self._room_member_handler.update_membership( create_requester(user, authenticated_entity=self._server_name), user, room.room_id, - "leave", + Membership.LEAVE, ratelimit=False, require_consent=False, ) logger.info( - "Rejected invite for deactivated user %r in room %r", + "Rejected %r for deactivated user %r in room %r", + room.membership, user_id, room.room_id, ) except Exception: logger.exception( - "Failed to reject invite for user %r in room %r:" + "Failed to reject %r for user %r in room %r:" " ignoring and continuing", + room.membership, user_id, room.room_id, ) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 5b0daffa46b..5d515025950 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -369,6 +369,22 @@ async def get_invited_rooms_for_local_user( user_id, [Membership.INVITE] ) + async def get_knocked_at_rooms_for_local_user( + self, user_id: str + ) -> Sequence[RoomsForUser]: + """Get all the rooms the *local* user has knocked at. + + Args: + user_id: The user ID. + + Returns: + A list of RoomsForUser. + """ + + return await self.get_rooms_for_local_user_where_membership_is( + user_id, [Membership.KNOCK] + ) + async def get_invite_for_local_user_in_room( self, user_id: str, room_id: str ) -> Optional[RoomsForUser]: diff --git a/tests/handlers/test_deactivate_account.py b/tests/handlers/test_deactivate_account.py index 25ac68e6c5d..b3f9e50f0f3 100644 --- a/tests/handlers/test_deactivate_account.py +++ b/tests/handlers/test_deactivate_account.py @@ -21,12 +21,13 @@ from twisted.test.proto_helpers import MemoryReactor -from synapse.api.constants import AccountDataTypes +from synapse.api.constants import AccountDataTypes, EventTypes, JoinRules, Membership from synapse.push.rulekinds import PRIORITY_CLASS_MAP from synapse.rest import admin -from synapse.rest.client import account, login +from synapse.rest.client import account, login, room from synapse.server import HomeServer from synapse.synapse_rust.push import PushRule +from synapse.types import UserID, create_requester from synapse.util import Clock from tests.unittest import HomeserverTestCase @@ -37,6 +38,7 @@ class DeactivateAccountTestCase(HomeserverTestCase): login.register_servlets, admin.register_servlets, account.register_servlets, + room.register_servlets, ] def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: @@ -44,6 +46,7 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.user = self.register_user("user", "pass") self.token = self.login("user", "pass") + self.handler = self.hs.get_room_member_handler() def _deactivate_my_account(self) -> None: """ @@ -341,3 +344,83 @@ def test_deactivate_account_needs_auth(self) -> None: self.assertEqual(req.code, 401, req) self.assertEqual(req.json_body["flows"], [{"stages": ["m.login.password"]}]) + + def test_deactivate_account_rejects_invites(self) -> None: + """ + Tests that deactivating an account rejects its invite memberships + """ + # Create another user and room just for the invitation + another_user = self.register_user("another_user", "pass") + token = self.login("another_user", "pass") + room_id = self.helper.create_room_as(another_user, is_public=False, tok=token) + + # Invite user to the created room + invite_event, _ = self.get_success( + self.handler.update_membership( + requester=create_requester(another_user), + target=UserID.from_string(self.user), + room_id=room_id, + action=Membership.INVITE, + ) + ) + + # Check that the invite exists + invite = self.get_success( + self._store.get_invited_rooms_for_local_user(self.user) + ) + self.assertEqual(invite[0].event_id, invite_event) + + # Deactivate the user + self._deactivate_my_account() + + # Check that the deactivated user has no invites in the room + after_deactivate_invite = self.get_success( + self._store.get_invited_rooms_for_local_user(self.user) + ) + self.assertEqual(len(after_deactivate_invite), 0) + + def test_deactivate_account_rejects_knocks(self) -> None: + """ + Tests that deactivating an account rejects its knock memberships + """ + # Create another user and room just for the invitation + another_user = self.register_user("another_user", "pass") + token = self.login("another_user", "pass") + room_id = self.helper.create_room_as( + another_user, + is_public=False, + tok=token, + ) + + # Allow room to be knocked at + self.helper.send_state( + room_id, + EventTypes.JoinRules, + {"join_rule": JoinRules.KNOCK}, + tok=token, + ) + + # Knock user at the created room + knock_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.KNOCK, + ) + ) + + # Check that the knock exists + knocks = self.get_success( + self._store.get_knocked_at_rooms_for_local_user(self.user) + ) + self.assertEqual(knocks[0].event_id, knock_event) + + # Deactivate the user + self._deactivate_my_account() + + # Check that the deactivated user has no knocks + after_deactivate_knocks = self.get_success( + self._store.get_knocked_at_rooms_for_local_user(self.user) + ) + self.assertEqual(len(after_deactivate_knocks), 0) From 37cf281e626d4cab8f284eccaf5dd4bd91dcb916 Mon Sep 17 00:00:00 2001 From: hanadi92 Date: Mon, 18 Mar 2024 16:59:38 +0300 Subject: [PATCH 2/4] docs: add changelog --- changelog.d/17010.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/17010.bugfix diff --git a/changelog.d/17010.bugfix b/changelog.d/17010.bugfix new file mode 100644 index 00000000000..8cd50930945 --- /dev/null +++ b/changelog.d/17010.bugfix @@ -0,0 +1 @@ +Fix bug which did not reject pending knocks at rooms on deactivating account. Contributed by @hanadi92. \ No newline at end of file From 586410610920917ea70ca540128d1c6109680d22 Mon Sep 17 00:00:00 2001 From: hanadi92 Date: Mon, 18 Mar 2024 17:32:50 +0300 Subject: [PATCH 3/4] fix: use itertools chain to iterate over two sequences --- synapse/handlers/deactivate_account.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/deactivate_account.py b/synapse/handlers/deactivate_account.py index c09c7ebe6e4..b13c4b6cb93 100644 --- a/synapse/handlers/deactivate_account.py +++ b/synapse/handlers/deactivate_account.py @@ -18,6 +18,7 @@ # [This file includes modifications made by New Vector Limited] # # +import itertools import logging from typing import TYPE_CHECKING, Optional @@ -205,7 +206,7 @@ async def _reject_pending_invites_and_knocks_for_user(self, user_id: str) -> Non pending_invites = await self.store.get_invited_rooms_for_local_user(user_id) pending_knocks = await self.store.get_knocked_at_rooms_for_local_user(user_id) - for room in pending_invites + pending_knocks: + for room in itertools.chain(pending_invites, pending_knocks): try: await self._room_member_handler.update_membership( create_requester(user, authenticated_entity=self._server_name), From bff220692fc86464ee1a65cab0474147b9175b98 Mon Sep 17 00:00:00 2001 From: reivilibre Date: Thu, 21 Mar 2024 17:41:58 +0000 Subject: [PATCH 4/4] Update changelog.d/17010.bugfix --- changelog.d/17010.bugfix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/17010.bugfix b/changelog.d/17010.bugfix index 8cd50930945..0e1495f744f 100644 --- a/changelog.d/17010.bugfix +++ b/changelog.d/17010.bugfix @@ -1 +1 @@ -Fix bug which did not reject pending knocks at rooms on deactivating account. Contributed by @hanadi92. \ No newline at end of file +Fix bug which did not retract a user's pending knocks at rooms when their account was deactivated. Contributed by @hanadi92. \ No newline at end of file