Do not consider events by ignored users for relations (#12285)

Filter the events returned from `/relations` for the requester's ignored users
in a similar way to `/messages` (and `/sync`).
rei/12281_reproduce
Patrick Cloke 2022-03-24 10:25:42 -04:00 committed by GitHub
parent 5436b014f4
commit 4df10d3214
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 90 additions and 4 deletions

1
changelog.d/12227.bugfix Normal file
View File

@ -0,0 +1 @@
Fix a long-standing bug where events from ignored users were still considered for relations.

View File

@ -1 +0,0 @@
Refactor the relations endpoints to add a `RelationsHandler`.

1
changelog.d/12232.bugfix Normal file
View File

@ -0,0 +1 @@
Fix a long-standing bug where events from ignored users were still considered for relations.

View File

@ -1 +0,0 @@
Refactor relations tests to improve code re-use.

1
changelog.d/12285.bugfix Normal file
View File

@ -0,0 +1 @@
Fix a long-standing bug where events from ignored users were still considered for relations.

View File

@ -21,6 +21,7 @@ from synapse.api.constants import RelationTypes
from synapse.api.errors import SynapseError from synapse.api.errors import SynapseError
from synapse.events import EventBase from synapse.events import EventBase
from synapse.types import JsonDict, Requester, StreamToken from synapse.types import JsonDict, Requester, StreamToken
from synapse.visibility import filter_events_for_client
if TYPE_CHECKING: if TYPE_CHECKING:
from synapse.server import HomeServer from synapse.server import HomeServer
@ -62,6 +63,7 @@ class BundledAggregations:
class RelationsHandler: class RelationsHandler:
def __init__(self, hs: "HomeServer"): def __init__(self, hs: "HomeServer"):
self._main_store = hs.get_datastores().main self._main_store = hs.get_datastores().main
self._storage = hs.get_storage()
self._auth = hs.get_auth() self._auth = hs.get_auth()
self._clock = hs.get_clock() self._clock = hs.get_clock()
self._event_handler = hs.get_event_handler() self._event_handler = hs.get_event_handler()
@ -103,7 +105,8 @@ class RelationsHandler:
user_id = requester.user.to_string() user_id = requester.user.to_string()
await self._auth.check_user_in_room_or_world_readable( # TODO Properly handle a user leaving a room.
(_, member_event_id) = await self._auth.check_user_in_room_or_world_readable(
room_id, user_id, allow_departed_users=True room_id, user_id, allow_departed_users=True
) )
@ -130,6 +133,10 @@ class RelationsHandler:
[c["event_id"] for c in pagination_chunk.chunk] [c["event_id"] for c in pagination_chunk.chunk]
) )
events = await filter_events_for_client(
self._storage, user_id, events, is_peeking=(member_event_id is None)
)
now = self._clock.time_msec() now = self._clock.time_msec()
# Do not bundle aggregations when retrieving the original event because # Do not bundle aggregations when retrieving the original event because
# we want the content before relations are applied to it. # we want the content before relations are applied to it.

View File

@ -20,7 +20,7 @@ from unittest.mock import patch
from twisted.test.proto_helpers import MemoryReactor from twisted.test.proto_helpers import MemoryReactor
from synapse.api.constants import EventTypes, RelationTypes from synapse.api.constants import AccountDataTypes, EventTypes, RelationTypes
from synapse.rest import admin from synapse.rest import admin
from synapse.rest.client import login, register, relations, room, sync from synapse.rest.client import login, register, relations, room, sync
from synapse.server import HomeServer from synapse.server import HomeServer
@ -1324,6 +1324,84 @@ class BundledAggregationsTestCase(BaseRelationsTestCase):
self.assertIn("m.relations", parent_event["unsigned"]) self.assertIn("m.relations", parent_event["unsigned"])
class RelationIgnoredUserTestCase(BaseRelationsTestCase):
"""Relations sent from an ignored user should be ignored."""
def _test_ignored_user(
self, allowed_event_ids: List[str], ignored_event_ids: List[str]
) -> None:
"""
Fetch the relations and ensure they're all there, then ignore user2, and
repeat.
"""
# Get the relations.
event_ids = self._get_related_events()
self.assertCountEqual(event_ids, allowed_event_ids + ignored_event_ids)
# Ignore user2 and re-do the requests.
self.get_success(
self.store.add_account_data_for_user(
self.user_id,
AccountDataTypes.IGNORED_USER_LIST,
{"ignored_users": {self.user2_id: {}}},
)
)
# Get the relations.
event_ids = self._get_related_events()
self.assertCountEqual(event_ids, allowed_event_ids)
def test_annotation(self) -> None:
"""Annotations should ignore"""
# Send 2 from us, 2 from the to be ignored user.
allowed_event_ids = []
ignored_event_ids = []
channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", key="a")
allowed_event_ids.append(channel.json_body["event_id"])
channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", key="b")
allowed_event_ids.append(channel.json_body["event_id"])
channel = self._send_relation(
RelationTypes.ANNOTATION,
"m.reaction",
key="a",
access_token=self.user2_token,
)
ignored_event_ids.append(channel.json_body["event_id"])
channel = self._send_relation(
RelationTypes.ANNOTATION,
"m.reaction",
key="c",
access_token=self.user2_token,
)
ignored_event_ids.append(channel.json_body["event_id"])
self._test_ignored_user(allowed_event_ids, ignored_event_ids)
def test_reference(self) -> None:
"""Annotations should ignore"""
channel = self._send_relation(RelationTypes.REFERENCE, "m.room.test")
allowed_event_ids = [channel.json_body["event_id"]]
channel = self._send_relation(
RelationTypes.REFERENCE, "m.room.test", access_token=self.user2_token
)
ignored_event_ids = [channel.json_body["event_id"]]
self._test_ignored_user(allowed_event_ids, ignored_event_ids)
def test_thread(self) -> None:
"""Annotations should ignore"""
channel = self._send_relation(RelationTypes.THREAD, "m.room.test")
allowed_event_ids = [channel.json_body["event_id"]]
channel = self._send_relation(
RelationTypes.THREAD, "m.room.test", access_token=self.user2_token
)
ignored_event_ids = [channel.json_body["event_id"]]
self._test_ignored_user(allowed_event_ids, ignored_event_ids)
class RelationRedactionTestCase(BaseRelationsTestCase): class RelationRedactionTestCase(BaseRelationsTestCase):
""" """
Test the behaviour of relations when the parent or child event is redacted. Test the behaviour of relations when the parent or child event is redacted.