Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Commit

Permalink
Fix that user cannot /forget rooms after the last member has left (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
dklimpel authored Aug 30, 2022
1 parent 51d732d commit 682dfcf
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 6 deletions.
1 change: 1 addition & 0 deletions changelog.d/13546.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix bug that user cannot `/forget` rooms after the last member has left the room.
7 changes: 5 additions & 2 deletions synapse/handlers/room_member.py
Original file line number Diff line number Diff line change
Expand Up @@ -1925,8 +1925,11 @@ async def forget(self, user: UserID, room_id: str) -> None:
]:
raise SynapseError(400, "User %s in room %s" % (user_id, room_id))

if membership:
await self.store.forget(user_id, room_id)
# In normal case this call is only required if `membership` is not `None`.
# But: After the last member had left the room, the background update
# `_background_remove_left_rooms` is deleting rows related to this room from
# the table `current_state_events` and `get_current_state_events` is `None`.
await self.store.forget(user_id, room_id)


def get_users_which_can_issue_invite(auth_events: StateMap[EventBase]) -> List[str]:
Expand Down
93 changes: 91 additions & 2 deletions tests/handlers/test_room_member.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import synapse.rest.client.login
import synapse.rest.client.room
from synapse.api.constants import EventTypes, Membership
from synapse.api.errors import LimitExceededError
from synapse.api.errors import LimitExceededError, SynapseError
from synapse.crypto.event_signing import add_hashes_and_signatures
from synapse.events import FrozenEventV3
from synapse.federation.federation_client import SendJoinResult
Expand All @@ -17,7 +17,11 @@
from tests.replication._base import BaseMultiWorkerStreamTestCase
from tests.server import make_request
from tests.test_utils import make_awaitable
from tests.unittest import FederatingHomeserverTestCase, override_config
from tests.unittest import (
FederatingHomeserverTestCase,
HomeserverTestCase,
override_config,
)


class TestJoinsLimitedByPerRoomRateLimiter(FederatingHomeserverTestCase):
Expand Down Expand Up @@ -287,3 +291,88 @@ def test_local_users_joining_on_another_worker_contribute_to_rate_limit(
),
LimitExceededError,
)


class RoomMemberMasterHandlerTestCase(HomeserverTestCase):
servlets = [
synapse.rest.admin.register_servlets,
synapse.rest.client.login.register_servlets,
synapse.rest.client.room.register_servlets,
]

def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
self.handler = hs.get_room_member_handler()
self.store = hs.get_datastores().main

# Create two users.
self.alice = self.register_user("alice", "pass")
self.alice_ID = UserID.from_string(self.alice)
self.alice_token = self.login("alice", "pass")
self.bob = self.register_user("bob", "pass")
self.bob_ID = UserID.from_string(self.bob)
self.bob_token = self.login("bob", "pass")

# Create a room on this homeserver.
self.room_id = self.helper.create_room_as(self.alice, tok=self.alice_token)

def test_leave_and_forget(self) -> None:
"""Tests that forget a room is successfully. The test is performed with two users,
as forgetting by the last user respectively after all users had left the
is a special edge case."""
self.helper.join(self.room_id, user=self.bob, tok=self.bob_token)

# alice is not the last room member that leaves and forgets the room
self.helper.leave(self.room_id, user=self.alice, tok=self.alice_token)
self.get_success(self.handler.forget(self.alice_ID, self.room_id))
self.assertTrue(
self.get_success(self.store.did_forget(self.alice, self.room_id))
)

# the server has not forgotten the room
self.assertFalse(
self.get_success(self.store.is_locally_forgotten_room(self.room_id))
)

def test_leave_and_forget_last_user(self) -> None:
"""Tests that forget a room is successfully when the last user has left the room."""

# alice is the last room member that leaves and forgets the room
self.helper.leave(self.room_id, user=self.alice, tok=self.alice_token)
self.get_success(self.handler.forget(self.alice_ID, self.room_id))
self.assertTrue(
self.get_success(self.store.did_forget(self.alice, self.room_id))
)

# the server has forgotten the room
self.assertTrue(
self.get_success(self.store.is_locally_forgotten_room(self.room_id))
)

def test_forget_when_not_left(self) -> None:
"""Tests that a user cannot not forgets a room that has not left."""
self.get_failure(self.handler.forget(self.alice_ID, self.room_id), SynapseError)

def test_rejoin_forgotten_by_user(self) -> None:
"""Test that a user that has forgotten a room can do a re-join.
The room was not forgotten from the local server.
One local user is still member of the room."""
self.helper.join(self.room_id, user=self.bob, tok=self.bob_token)

self.helper.leave(self.room_id, user=self.alice, tok=self.alice_token)
self.get_success(self.handler.forget(self.alice_ID, self.room_id))
self.assertTrue(
self.get_success(self.store.did_forget(self.alice, self.room_id))
)

# the server has not forgotten the room
self.assertFalse(
self.get_success(self.store.is_locally_forgotten_room(self.room_id))
)

self.helper.join(self.room_id, user=self.alice, tok=self.alice_token)
# TODO: A join to a room does not invalidate the forgotten cache
# see https://github.com/matrix-org/synapse/issues/13262
self.store.did_forget.invalidate_all()
self.assertFalse(
self.get_success(self.store.did_forget(self.alice, self.room_id))
)
4 changes: 2 additions & 2 deletions tests/storage/test_roommember.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ def test__null_byte_in_display_name_properly_handled(self) -> None:
# Check that alice's display name is now None
self.assertEqual(row[0]["display_name"], None)

def test_room_is_locally_forgotten(self):
def test_room_is_locally_forgotten(self) -> None:
"""Test that when the last local user has forgotten a room it is known as forgotten."""
# join two local and one remote user
self.room = self.helper.create_room_as(self.u_alice, tok=self.t_alice)
Expand Down Expand Up @@ -199,7 +199,7 @@ def test_room_is_locally_forgotten(self):
self.get_success(self.store.is_locally_forgotten_room(self.room))
)

def test_join_locally_forgotten_room(self):
def test_join_locally_forgotten_room(self) -> None:
"""Tests if a user joins a forgotten room the room is not forgotten anymore."""
self.room = self.helper.create_room_as(self.u_alice, tok=self.t_alice)
self.assertFalse(
Expand Down

0 comments on commit 682dfcf

Please sign in to comment.