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

Add support for stable prefixes for MSC2285: private read receipts #13273

Merged
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
83b7af1
Switch to stable prefixes for MSC2285
SimonBrandner Jul 14, 2022
01fbb6d
Add changelog
SimonBrandner Jul 14, 2022
cafe4be
Avoid leaking unstable private read receipts
SimonBrandner Jul 14, 2022
967f8ed
Delint
SimonBrandner Jul 14, 2022
a9f2329
Fix code
SimonBrandner Jul 14, 2022
08d874d
Merge remote-tracking branch 'upstream/develop' into SimonBrandner/fe…
SimonBrandner Jul 23, 2022
ba20652
Use a const
SimonBrandner Jul 23, 2022
d18a485
Convert list to tuple.
clokep Jul 25, 2022
8323358
Merge remote-tracking branch 'upstream/develop' into SimonBrandner/fe…
SimonBrandner Jul 28, 2022
404459f
Add background update to remove unstable private read receipts
SimonBrandner Jul 29, 2022
57251c8
Remove handling for unstable private read receipts
SimonBrandner Jul 29, 2022
e12706c
Don't bump `SCHEMA_VERSION`
SimonBrandner Jul 29, 2022
c8cf10e
Remove `READ_PRIVATE_UNSTABLE` from `ReceiptTypes`
SimonBrandner Jul 29, 2022
84299f6
Assume return value
SimonBrandner Jul 29, 2022
208e97a
Remove non-relevant test
SimonBrandner Jul 29, 2022
a7c3a1c
Don't bother with a migration script
SimonBrandner Jul 29, 2022
252146d
Remove non-schema thing
SimonBrandner Jul 29, 2022
3cbd80b
Only use single `'`
SimonBrandner Jul 29, 2022
8854e35
Use `!=`
SimonBrandner Jul 29, 2022
baa9e84
Merge pull request #1 from SimonBrandner/SimonBrandner/feat/db-rr
SimonBrandner Jul 29, 2022
4455232
Changelog
SimonBrandner Aug 4, 2022
de9005f
Merge remote-tracking branch 'upstream/develop' into SimonBrandner/fe…
SimonBrandner Aug 4, 2022
c0b3c90
Support both stable and unstable private RRs
SimonBrandner Aug 4, 2022
3612ceb
Test both stable and unstable private RRs
SimonBrandner Aug 4, 2022
a37a17d
Advertise unstable prefix
SimonBrandner Aug 4, 2022
14b36a3
Check for all receipt types
SimonBrandner Aug 4, 2022
746351c
Delint
SimonBrandner Aug 4, 2022
50bffe4
Further dlint
SimonBrandner Aug 4, 2022
1b66814
Remove `(`
SimonBrandner Aug 4, 2022
bcdbb5d
This won't work...
SimonBrandner Aug 4, 2022
0f498e0
Reformat...
SimonBrandner Aug 4, 2022
660c776
Get tests passing
SimonBrandner Aug 4, 2022
b1ddf1a
Merge remote-tracking branch 'origin/develop' into SimonBrandner/feat…
clokep Aug 4, 2022
3094b39
Fix type hints.
clokep Aug 4, 2022
011d6d2
Put experimental flag back in place
SimonBrandner Aug 5, 2022
ee6512f
Only advertise `org.matrix.msc2285` if enabled
SimonBrandner Aug 5, 2022
2cd29bd
Fix tests
SimonBrandner Aug 5, 2022
89d5bd4
Merge remote-tracking branch 'origin/SimonBrandner/feat/disable-rr' i…
SimonBrandner Aug 5, 2022
f0c531d
Fix tests
SimonBrandner Aug 5, 2022
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/13273.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Switch to stable prefixes for [MSC2285 (private read receipts)](https://github.com/matrix-org/matrix-spec-proposals/pull/2285).
2 changes: 1 addition & 1 deletion synapse/api/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ class GuestAccess:

class ReceiptTypes:
READ: Final = "m.read"
READ_PRIVATE: Final = "org.matrix.msc2285.read.private"
READ_PRIVATE: Final = "m.read.private"
FULLY_READ: Final = "m.fully_read"


Expand Down
3 changes: 0 additions & 3 deletions synapse/config/experimental.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,6 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
# MSC2716 (importing historical messages)
self.msc2716_enabled: bool = experimental.get("msc2716_enabled", False)

# MSC2285 (private read receipts)
self.msc2285_enabled: bool = experimental.get("msc2285_enabled", False)

# MSC3244 (room version capabilities)
self.msc3244_enabled: bool = experimental.get("msc3244_enabled", True)

Expand Down
11 changes: 4 additions & 7 deletions synapse/handlers/initial_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,8 @@ async def _snapshot_all_rooms(
joined_rooms,
to_key=int(now_token.receipt_key),
)
if self.hs.config.experimental.msc2285_enabled:
receipt = ReceiptEventSource.filter_out_private_receipts(receipt, user_id)

receipt = ReceiptEventSource.filter_out_private_receipts(receipt, user_id)

tags_by_room = await self.store.get_tags_for_user(user_id)

Expand Down Expand Up @@ -456,11 +456,8 @@ async def get_receipts() -> List[JsonDict]:
)
if not receipts:
return []
if self.hs.config.experimental.msc2285_enabled:
receipts = ReceiptEventSource.filter_out_private_receipts(
receipts, user_id
)
return receipts

return ReceiptEventSource.filter_out_private_receipts(receipts, user_id)

presence, receipts, (messages, token) = await make_deferred_yieldable(
gather_results(
Expand Down
24 changes: 15 additions & 9 deletions synapse/handlers/receipts.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,20 +203,27 @@ def filter_out_private_receipts(
for event_id, orig_event_content in room.get("content", {}).items():
event_content = orig_event_content
# If there are private read receipts, additional logic is necessary.
if ReceiptTypes.READ_PRIVATE in event_content:
if (
ReceiptTypes.READ_PRIVATE in event_content
or "org.matrix.msc2285.read.private" in event_content
SimonBrandner marked this conversation as resolved.
Show resolved Hide resolved
):
# Make a copy without private read receipts to avoid leaking
# other user's private read receipts..
event_content = {
receipt_type: receipt_value
for receipt_type, receipt_value in event_content.items()
if receipt_type != ReceiptTypes.READ_PRIVATE
if receipt_type
not in [
ReceiptTypes.READ_PRIVATE,
"org.matrix.msc2285.read.private",
]
}

# Copy the current user's private read receipt from the
# original content, if it exists.
user_private_read_receipt = orig_event_content[
ReceiptTypes.READ_PRIVATE
].get(user_id, None)
user_private_read_receipt = orig_event_content.get(
ReceiptTypes.READ_PRIVATE, {}
).get(user_id, None)
SimonBrandner marked this conversation as resolved.
Show resolved Hide resolved
if user_private_read_receipt:
event_content[ReceiptTypes.READ_PRIVATE] = {
user_id: user_private_read_receipt
Expand Down Expand Up @@ -256,10 +263,9 @@ async def get_new_events(
room_ids, from_key=from_key, to_key=to_key
)

if self.config.experimental.msc2285_enabled:
events = ReceiptEventSource.filter_out_private_receipts(
events, user.to_string()
)
events = ReceiptEventSource.filter_out_private_receipts(
events, user.to_string()
)

return events, to_key

Expand Down
8 changes: 5 additions & 3 deletions synapse/rest/client/read_marker.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,11 @@ def __init__(self, hs: "HomeServer"):
self.read_marker_handler = hs.get_read_marker_handler()
self.presence_handler = hs.get_presence_handler()

self._known_receipt_types = {ReceiptTypes.READ, ReceiptTypes.FULLY_READ}
if hs.config.experimental.msc2285_enabled:
self._known_receipt_types.add(ReceiptTypes.READ_PRIVATE)
SimonBrandner marked this conversation as resolved.
Show resolved Hide resolved
self._known_receipt_types = {
ReceiptTypes.READ,
ReceiptTypes.FULLY_READ,
ReceiptTypes.READ_PRIVATE,
}
SimonBrandner marked this conversation as resolved.
Show resolved Hide resolved

async def on_POST(
self, request: SynapseRequest, room_id: str
Expand Down
10 changes: 5 additions & 5 deletions synapse/rest/client/receipts.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,11 @@ def __init__(self, hs: "HomeServer"):
self.read_marker_handler = hs.get_read_marker_handler()
self.presence_handler = hs.get_presence_handler()

self._known_receipt_types = {ReceiptTypes.READ}
if hs.config.experimental.msc2285_enabled:
self._known_receipt_types.update(
(ReceiptTypes.READ_PRIVATE, ReceiptTypes.FULLY_READ)
)
self._known_receipt_types = {
ReceiptTypes.READ,
ReceiptTypes.READ_PRIVATE,
ReceiptTypes.FULLY_READ,
}

async def on_POST(
self, request: SynapseRequest, room_id: str, receipt_type: str, event_id: str
Expand Down
2 changes: 1 addition & 1 deletion synapse/rest/client/versions.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ def on_GET(self, request: Request) -> Tuple[int, JsonDict]:
# Supports the busy presence state described in MSC3026.
"org.matrix.msc3026.busy_presence": self.config.experimental.msc3026_enabled,
# Supports receiving private read receipts as per MSC2285
"org.matrix.msc2285": self.config.experimental.msc2285_enabled,
"org.matrix.msc2285.stable": True, # TODO: Remove when MSC2285 becomes a part of the spec
# Supports filtering of /publicRooms by room type MSC3827
"org.matrix.msc3827": self.config.experimental.msc3827_enabled,
# Adds support for importing historical messages as per MSC2716
Expand Down
20 changes: 20 additions & 0 deletions tests/handlers/test_receipts.py
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,26 @@ def test_leaves_our_private_and_their_public(self):
],
)

def test_filters_out_unstable_private_receipt(self):
self._test_filters_private(
[
{
"content": {
"$1435641916114394fHBLK:matrix.org": {
"org.matrix.msc2285.read.private": {
"@rikj:jki.re": {
"ts": 1436451550453,
}
}
}
},
"room_id": "!jEsUZKDJdhlrceRyVU:example.org",
"type": EduTypes.RECEIPT,
}
],
[],
)

def test_we_do_not_mutate(self):
"""Ensure the input values are not modified."""
events = [
Expand Down
17 changes: 6 additions & 11 deletions tests/rest/client/test_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
KnockingStrippedStateEventHelperMixin,
)
from tests.server import TimedOutException
from tests.unittest import override_config


class FilterTestCase(unittest.HomeserverTestCase):
Expand Down Expand Up @@ -408,15 +407,14 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
# Join the second user
self.helper.join(room=self.room_id, user=self.user2, tok=self.tok2)

@override_config({"experimental_features": {"msc2285_enabled": True}})
def test_private_read_receipts(self) -> None:
# Send a message as the first user
res = self.helper.send(self.room_id, body="hello", tok=self.tok)

# Send a private read receipt to tell the server the first user's message was read
channel = self.make_request(
"POST",
f"/rooms/{self.room_id}/receipt/org.matrix.msc2285.read.private/{res['event_id']}",
f"/rooms/{self.room_id}/receipt/m.read.private/{res['event_id']}",
{},
access_token=self.tok2,
)
Expand All @@ -425,7 +423,6 @@ def test_private_read_receipts(self) -> None:
# Test that the first user can't see the other user's private read receipt
self.assertIsNone(self._get_read_receipt())

@override_config({"experimental_features": {"msc2285_enabled": True}})
def test_public_receipt_can_override_private(self) -> None:
"""
Sending a public read receipt to the same event which has a private read
Expand Down Expand Up @@ -456,7 +453,6 @@ def test_public_receipt_can_override_private(self) -> None:
# Test that we did override the private read receipt
self.assertNotEqual(self._get_read_receipt(), None)

@override_config({"experimental_features": {"msc2285_enabled": True}})
def test_private_receipt_cannot_override_public(self) -> None:
"""
Sending a private read receipt to the same event which has a public read
Expand Down Expand Up @@ -543,7 +539,6 @@ def default_config(self) -> JsonDict:
config = super().default_config()
config["experimental_features"] = {
"msc2654_enabled": True,
"msc2285_enabled": True,
}
return config

Expand Down Expand Up @@ -625,7 +620,7 @@ def test_unread_counts(self) -> None:
# Send a read receipt to tell the server we've read the latest event.
channel = self.make_request(
"POST",
f"/rooms/{self.room_id}/receipt/org.matrix.msc2285.read.private/{res['event_id']}",
f"/rooms/{self.room_id}/receipt/m.read.private/{res['event_id']}",
{},
access_token=self.tok,
)
Expand Down Expand Up @@ -701,7 +696,7 @@ def test_unread_counts(self) -> None:
self._check_unread_count(5)
res2 = self.helper.send(self.room_id, "hello", tok=self.tok2)

# Make sure both m.read and org.matrix.msc2285.read.private advance
# Make sure both m.read and m.read.private advance
channel = self.make_request(
"POST",
f"/rooms/{self.room_id}/receipt/m.read/{res1['event_id']}",
Expand All @@ -713,7 +708,7 @@ def test_unread_counts(self) -> None:

channel = self.make_request(
"POST",
f"/rooms/{self.room_id}/receipt/org.matrix.msc2285.read.private/{res2['event_id']}",
f"/rooms/{self.room_id}/receipt/m.read.private/{res2['event_id']}",
{},
access_token=self.tok,
)
Expand All @@ -740,11 +735,11 @@ def test_read_receipts_only_go_down(self, receipt_type: ReceiptTypes) -> None:
self.assertEqual(channel.code, 200, channel.json_body)
self._check_unread_count(0)

# Make sure neither m.read nor org.matrix.msc2285.read.private make the
# Make sure neither m.read nor m.read.private make the
# read receipt go up to an older event
channel = self.make_request(
"POST",
f"/rooms/{self.room_id}/receipt/org.matrix.msc2285.read.private/{res1['event_id']}",
f"/rooms/{self.room_id}/receipt/m.read.private/{res1['event_id']}",
{},
access_token=self.tok,
)
Expand Down