From f7a3ebe44d24e18c79847104f103a07f4a575a5f Mon Sep 17 00:00:00 2001 From: Hanadi Date: Thu, 21 Mar 2024 21:05:54 +0300 Subject: [PATCH] Fix reject knocks on deactivating account (#17010) --- changelog.d/17010.bugfix | 1 + synapse/handlers/deactivate_account.py | 25 +++--- synapse/storage/databases/main/roommember.py | 16 ++++ tests/handlers/test_deactivate_account.py | 87 +++++++++++++++++++- 4 files changed, 117 insertions(+), 12 deletions(-) create mode 100644 changelog.d/17010.bugfix diff --git a/changelog.d/17010.bugfix b/changelog.d/17010.bugfix new file mode 100644 index 0000000000..0e1495f744 --- /dev/null +++ b/changelog.d/17010.bugfix @@ -0,0 +1 @@ +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 diff --git a/synapse/handlers/deactivate_account.py b/synapse/handlers/deactivate_account.py index de5ed5d5b6..b13c4b6cb9 100644 --- a/synapse/handlers/deactivate_account.py +++ b/synapse/handlers/deactivate_account.py @@ -18,9 +18,11 @@ # [This file includes modifications made by New Vector Limited] # # +import itertools 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 +170,9 @@ class DeactivateAccountHandler: # 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 +196,37 @@ class DeactivateAccountHandler: 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 itertools.chain(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 5b0daffa46..5d51502595 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -369,6 +369,22 @@ class RoomMemberWorkerStore(EventsWorkerStore, CacheInvalidationWorkerStore): 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 25ac68e6c5..b3f9e50f0f 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 @@ class DeactivateAccountTestCase(HomeserverTestCase): 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 @@ class DeactivateAccountTestCase(HomeserverTestCase): 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)