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

Remove support for unstable private read receipts #13653

Merged
merged 2 commits into from
Sep 1, 2022
Merged

Remove support for unstable private read receipts #13653

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2687a9a
Remove support for unstable private read receipts
SimonBrandner Aug 27, 2022
157e1b7
Add changelog
SimonBrandner Aug 27, 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/13653.removal
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Remove support for unstable [private read receipts](https://github.com/matrix-org/matrix-spec-proposals/pull/2285).
1 change: 0 additions & 1 deletion synapse/api/constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,6 @@ class GuestAccess:
class ReceiptTypes:
READ: Final = "m.read"
READ_PRIVATE: Final = "m.read.private"
UNSTABLE_READ_PRIVATE: Final = "org.matrix.msc2285.read.private"
FULLY_READ: Final = "m.fully_read"


Expand Down
3 changes: 0 additions & 3 deletions synapse/config/experimental.py
View file
Open in desktop
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 (unstable 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
29 changes: 6 additions & 23 deletions synapse/handlers/receipts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,10 +163,7 @@ async def received_client_receipt(
if not is_new:
return

if self.federation_sender and receipt_type not in (
ReceiptTypes.READ_PRIVATE,
ReceiptTypes.UNSTABLE_READ_PRIVATE,
):
if self.federation_sender and receipt_type != ReceiptTypes.READ_PRIVATE:
await self.federation_sender.send_read_receipt(receipt)


Expand Down Expand Up @@ -206,38 +203,24 @@ 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
or ReceiptTypes.UNSTABLE_READ_PRIVATE in event_content
):
Comment on lines -209 to -212
Copy link
Member

@clokep clokep Sep 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this means that we'll now leak any previously stored unstable private read receipts, which is a huge privacy leak.

I think we should nuke them from the database using a delta file.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #13692.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, sorry, I managed to forget about it again...

if ReceiptTypes.READ_PRIVATE in event_content:
# 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
not in (
ReceiptTypes.READ_PRIVATE,
ReceiptTypes.UNSTABLE_READ_PRIVATE,
)
if receipt_type != ReceiptTypes.READ_PRIVATE
}

# Copy the current user's private read receipt from the
# original content, if it exists.
user_private_read_receipt = orig_event_content.get(
ReceiptTypes.READ_PRIVATE, {}
).get(user_id, None)
user_private_read_receipt = orig_event_content[
ReceiptTypes.READ_PRIVATE
].get(user_id, None)
if user_private_read_receipt:
event_content[ReceiptTypes.READ_PRIVATE] = {
user_id: user_private_read_receipt
}
user_unstable_private_read_receipt = orig_event_content.get(
ReceiptTypes.UNSTABLE_READ_PRIVATE, {}
).get(user_id, None)
if user_unstable_private_read_receipt:
event_content[ReceiptTypes.UNSTABLE_READ_PRIVATE] = {
user_id: user_unstable_private_read_receipt
}

# Include the event if there is at least one non-private read
# receipt or the current user has a private read receipt.
Expand Down
5 changes: 1 addition & 4 deletions synapse/replication/tcp/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,10 +416,7 @@ async def _on_new_receipts(
if not self._is_mine_id(receipt.user_id):
continue
# Private read receipts never get sent over federation.
if receipt.receipt_type in (
ReceiptTypes.READ_PRIVATE,
ReceiptTypes.UNSTABLE_READ_PRIVATE,
):
if receipt.receipt_type == ReceiptTypes.READ_PRIVATE:
continue
receipt_info = ReadReceipt(
receipt.room_id,
Expand Down
1 change: 0 additions & 1 deletion synapse/rest/client/notifications.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
[
ReceiptTypes.READ,
ReceiptTypes.READ_PRIVATE,
ReceiptTypes.UNSTABLE_READ_PRIVATE,
],
)

Expand Down
2 changes: 0 additions & 2 deletions synapse/rest/client/read_marker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,6 @@ def __init__(self, hs: "HomeServer"):
ReceiptTypes.FULLY_READ,
ReceiptTypes.READ_PRIVATE,
}
if hs.config.experimental.msc2285_enabled:
self._known_receipt_types.add(ReceiptTypes.UNSTABLE_READ_PRIVATE)

async def on_POST(
self, request: SynapseRequest, room_id: str
Expand Down
2 changes: 0 additions & 2 deletions synapse/rest/client/receipts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,6 @@ def __init__(self, hs: "HomeServer"):
ReceiptTypes.READ_PRIVATE,
ReceiptTypes.FULLY_READ,
}
if hs.config.experimental.msc2285_enabled:
self._known_receipt_types.add(ReceiptTypes.UNSTABLE_READ_PRIVATE)

async def on_POST(
self, request: SynapseRequest, room_id: str, receipt_type: str, event_id: str
Expand Down
1 change: 0 additions & 1 deletion synapse/rest/client/versions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ def on_GET(self, request: Request) -> Tuple[int, JsonDict]:
"org.matrix.msc3026.busy_presence": self.config.experimental.msc3026_enabled,
# Supports receiving private read receipts as per MSC2285
"org.matrix.msc2285.stable": True, # TODO: Remove when MSC2285 becomes a part of the spec
"org.matrix.msc2285": self.config.experimental.msc2285_enabled,
# Supports filtering of /publicRooms by room type as per MSC3827
"org.matrix.msc3827.stable": True,
# Adds support for importing historical messages as per MSC2716
Expand Down
2 changes: 0 additions & 2 deletions synapse/storage/databases/main/event_push_actions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,6 @@ def _get_unread_counts_by_receipt_txn(
receipt_types=(
ReceiptTypes.READ,
ReceiptTypes.READ_PRIVATE,
ReceiptTypes.UNSTABLE_READ_PRIVATE,
),
)

Expand Down Expand Up @@ -468,7 +467,6 @@ def _get_receipts_by_room_txn(
(
ReceiptTypes.READ,
ReceiptTypes.READ_PRIVATE,
ReceiptTypes.UNSTABLE_READ_PRIVATE,
),
)

Expand Down
48 changes: 13 additions & 35 deletions tests/handlers/test_receipts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
from copy import deepcopy
from typing import List

from parameterized import parameterized

from synapse.api.constants import EduTypes, ReceiptTypes
from synapse.types import JsonDict

Expand All @@ -27,16 +25,13 @@ class ReceiptsTestCase(unittest.HomeserverTestCase):
def prepare(self, reactor, clock, hs):
self.event_source = hs.get_event_sources().sources.receipt

@parameterized.expand(
[ReceiptTypes.READ_PRIVATE, ReceiptTypes.UNSTABLE_READ_PRIVATE]
)
def test_filters_out_private_receipt(self, receipt_type: str) -> None:
def test_filters_out_private_receipt(self) -> None:
self._test_filters_private(
[
{
"content": {
"$1435641916114394fHBLK:matrix.org": {
receipt_type: {
ReceiptTypes.READ_PRIVATE: {
"@rikj:jki.re": {
"ts": 1436451550453,
}
Expand All @@ -50,18 +45,13 @@ def test_filters_out_private_receipt(self, receipt_type: str) -> None:
[],
)

@parameterized.expand(
[ReceiptTypes.READ_PRIVATE, ReceiptTypes.UNSTABLE_READ_PRIVATE]
)
def test_filters_out_private_receipt_and_ignores_rest(
self, receipt_type: str
) -> None:
def test_filters_out_private_receipt_and_ignores_rest(self) -> None:
self._test_filters_private(
[
{
"content": {
"$1dgdgrd5641916114394fHBLK:matrix.org": {
receipt_type: {
ReceiptTypes.READ_PRIVATE: {
"@rikj:jki.re": {
"ts": 1436451550453,
},
Expand Down Expand Up @@ -94,18 +84,15 @@ def test_filters_out_private_receipt_and_ignores_rest(
],
)

@parameterized.expand(
[ReceiptTypes.READ_PRIVATE, ReceiptTypes.UNSTABLE_READ_PRIVATE]
)
def test_filters_out_event_with_only_private_receipts_and_ignores_the_rest(
self, receipt_type: str
self,
) -> None:
self._test_filters_private(
[
{
"content": {
"$14356419edgd14394fHBLK:matrix.org": {
receipt_type: {
ReceiptTypes.READ_PRIVATE: {
"@rikj:jki.re": {
"ts": 1436451550453,
},
Expand Down Expand Up @@ -175,18 +162,15 @@ def test_handles_empty_event(self) -> None:
],
)

@parameterized.expand(
[ReceiptTypes.READ_PRIVATE, ReceiptTypes.UNSTABLE_READ_PRIVATE]
)
def test_filters_out_receipt_event_with_only_private_receipt_and_ignores_rest(
self, receipt_type: str
self,
) -> None:
self._test_filters_private(
[
{
"content": {
"$14356419edgd14394fHBLK:matrix.org": {
receipt_type: {
ReceiptTypes.READ_PRIVATE: {
"@rikj:jki.re": {
"ts": 1436451550453,
},
Expand Down Expand Up @@ -262,16 +246,13 @@ def test_handles_string_data(self) -> None:
],
)

@parameterized.expand(
[ReceiptTypes.READ_PRIVATE, ReceiptTypes.UNSTABLE_READ_PRIVATE]
)
def test_leaves_our_private_and_their_public(self, receipt_type: str) -> None:
def test_leaves_our_private_and_their_public(self) -> None:
self._test_filters_private(
[
{
"content": {
"$1dgdgrd5641916114394fHBLK:matrix.org": {
receipt_type: {
ReceiptTypes.READ_PRIVATE: {
"@me:server.org": {
"ts": 1436451550453,
},
Expand All @@ -296,7 +277,7 @@ def test_leaves_our_private_and_their_public(self, receipt_type: str) -> None:
{
"content": {
"$1dgdgrd5641916114394fHBLK:matrix.org": {
receipt_type: {
ReceiptTypes.READ_PRIVATE: {
"@me:server.org": {
"ts": 1436451550453,
},
Expand All @@ -319,16 +300,13 @@ def test_leaves_our_private_and_their_public(self, receipt_type: str) -> None:
],
)

@parameterized.expand(
[ReceiptTypes.READ_PRIVATE, ReceiptTypes.UNSTABLE_READ_PRIVATE]
)
def test_we_do_not_mutate(self, receipt_type: str) -> None:
def test_we_do_not_mutate(self) -> None:
"""Ensure the input values are not modified."""
events = [
{
"content": {
"$1435641916114394fHBLK:matrix.org": {
receipt_type: {
ReceiptTypes.READ_PRIVATE: {
"@rikj:jki.re": {
"ts": 1436451550453,
}
Expand Down
Loading