Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix reject knocks on deactivating account #17010

Merged
merged 4 commits into from
Mar 21, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/17010.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix bug which did not reject pending knocks at rooms on deactivating account. Contributed by @hanadi92.
reivilibre marked this conversation as resolved.
Show resolved Hide resolved
25 changes: 15 additions & 10 deletions synapse/handlers/deactivate_account.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -168,9 +170,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:
Expand All @@ -194,34 +196,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 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,
)
Expand Down
16 changes: 16 additions & 0 deletions synapse/storage/databases/main/roommember.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]:
Expand Down
87 changes: 85 additions & 2 deletions tests/handlers/test_deactivate_account.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -37,13 +38,15 @@ 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:
self._store = hs.get_datastores().main

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:
"""
Expand Down Expand Up @@ -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)
Loading