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

Error if attempting to set m.push_rules account data, per MSC4010. #15555

Merged
merged 10 commits into from
May 9, 2023
1 change: 1 addition & 0 deletions changelog.d/15554.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Experimental support for [MSC4010](https://github.com/matrix-org/matrix-spec-proposals/pull/4010) which rejects setting the `"m.push_rules"` via account data.
1 change: 1 addition & 0 deletions synapse/api/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ class AccountDataTypes:
DIRECT: Final = "m.direct"
IGNORED_USER_LIST: Final = "m.ignored_user_list"
TAG: Final = "m.tag"
PUSH_RULES: Final = "m.push_rules"


class HistoryVisibility:
Expand Down
5 changes: 5 additions & 0 deletions synapse/config/experimental.py
Original file line number Diff line number Diff line change
Expand Up @@ -202,3 +202,8 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:

# MSC4009: E.164 Matrix IDs
self.msc4009_e164_mxids = experimental.get("msc4009_e164_mxids", False)

# MSC4010: Do not allow setting m.push_rules account data.
self.msc4010_push_rules_account_data = experimental.get(
"msc4010_push_rules_account_data", False
)
12 changes: 11 additions & 1 deletion synapse/handlers/account_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@
# limitations under the License.
import logging
import random
from typing import TYPE_CHECKING, Awaitable, Callable, List, Optional, Tuple
from typing import TYPE_CHECKING, Awaitable, Callable, Dict, List, Optional, Tuple

from synapse.api.constants import AccountDataTypes
from synapse.push.clientformat import format_push_rules_for_user
from synapse.replication.http.account_data import (
ReplicationAddRoomAccountDataRestServlet,
ReplicationAddTagRestServlet,
Expand Down Expand Up @@ -302,6 +303,15 @@ async def remove_tag_from_room(self, user_id: str, room_id: str, tag: str) -> in
)
return response["max_stream_id"]

async def push_rules_for_user(self, user: UserID) -> Dict[str, Dict[str, list]]:
clokep marked this conversation as resolved.
Show resolved Hide resolved
"""
Push rules aren't really account data, but get formatted as such for /sync.
"""
user_id = user.to_string()
rules_raw = await self._store.get_push_rules_for_user(user_id)
rules = format_push_rules_for_user(user, rules_raw)
return rules


class AccountDataEventSource(EventSource[int, JsonDict]):
def __init__(self, hs: "HomeServer"):
Expand Down
5 changes: 3 additions & 2 deletions synapse/handlers/read_marker.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import logging
from typing import TYPE_CHECKING

from synapse.api.constants import ReceiptTypes
from synapse.util.async_helpers import Linearizer

if TYPE_CHECKING:
Expand Down Expand Up @@ -42,7 +43,7 @@ async def received_client_read_marker(

async with self.read_marker_linearizer.queue((room_id, user_id)):
existing_read_marker = await self.store.get_account_data_for_room_and_type(
user_id, room_id, "m.fully_read"
user_id, room_id, ReceiptTypes.FULLY_READ
)

should_update = True
Expand All @@ -56,5 +57,5 @@ async def received_client_read_marker(
if should_update:
content = {"event_id": event_id}
await self.account_data_handler.add_account_data_to_room(
user_id, room_id, "m.fully_read", content
user_id, room_id, ReceiptTypes.FULLY_READ, content
)
18 changes: 7 additions & 11 deletions synapse/handlers/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@
start_active_span,
trace,
)
from synapse.push.clientformat import format_push_rules_for_user
from synapse.storage.databases.main.event_push_actions import RoomNotifCounts
from synapse.storage.databases.main.roommember import extract_heroes_from_room_summary
from synapse.storage.roommember import MemberSummary
Expand Down Expand Up @@ -261,6 +260,7 @@ def __init__(self, hs: "HomeServer"):
self.notifier = hs.get_notifier()
self.presence_handler = hs.get_presence_handler()
self._relations_handler = hs.get_relations_handler()
self._account_data_handler = hs.get_account_data_handler()
self.event_sources = hs.get_event_sources()
self.clock = hs.get_clock()
self.state = hs.get_state_handler()
Expand Down Expand Up @@ -428,12 +428,6 @@ async def current_sync_for_user(
set_tag(SynapseTags.SYNC_RESULT, bool(sync_result))
return sync_result

async def push_rules_for_user(self, user: UserID) -> Dict[str, Dict[str, list]]:
user_id = user.to_string()
rules_raw = await self.store.get_push_rules_for_user(user_id)
rules = format_push_rules_for_user(user, rules_raw)
return rules

async def ephemeral_by_room(
self,
sync_result_builder: "SyncResultBuilder",
Expand Down Expand Up @@ -1777,7 +1771,9 @@ async def _generate_sync_entry_for_account_data(

if push_rules_changed:
global_account_data = dict(global_account_data)
global_account_data["m.push_rules"] = await self.push_rules_for_user(
global_account_data[
AccountDataTypes.PUSH_RULES
] = await self._account_data_handler.push_rules_for_user(
sync_config.user
)
else:
Expand All @@ -1786,9 +1782,9 @@ async def _generate_sync_entry_for_account_data(
)

global_account_data = dict(all_global_account_data)
global_account_data["m.push_rules"] = await self.push_rules_for_user(
sync_config.user
)
global_account_data[
AccountDataTypes.PUSH_RULES
] = await self._account_data_handler.push_rules_for_user(sync_config.user)

account_data_for_user = (
await sync_config.filter_collection.filter_global_account_data(
Expand Down
83 changes: 67 additions & 16 deletions synapse/rest/client/account_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@
# limitations under the License.

import logging
from typing import TYPE_CHECKING, Tuple
from typing import TYPE_CHECKING, Optional, Tuple

from synapse.api.constants import AccountDataTypes, ReceiptTypes
from synapse.api.errors import AuthError, Codes, NotFoundError, SynapseError
from synapse.http.server import HttpServer
from synapse.http.servlet import RestServlet, parse_json_object_from_request
Expand All @@ -29,6 +30,23 @@
logger = logging.getLogger(__name__)


def _check_can_set_account_data_type(account_data_type: str) -> None:
"""The fully read marker and push rules cannot be directly set via /account_data."""
if account_data_type == ReceiptTypes.FULLY_READ:
raise SynapseError(
405,
"Cannot set m.fully_read through this API."
" Use /rooms/!roomId:server.name/read_markers",
Codes.BAD_JSON,
)
elif account_data_type == AccountDataTypes.PUSH_RULES:
raise SynapseError(
405,
"Cannot set m.push_rules through this API. Use /pushrules",
Codes.BAD_JSON,
)


class AccountDataServlet(RestServlet):
"""
PUT /user/{user_id}/account_data/{account_dataType} HTTP/1.1
Expand All @@ -54,6 +72,10 @@ async def on_PUT(
if user_id != requester.user.to_string():
raise AuthError(403, "Cannot add account data for other users.")

# Raise an error if the account data type cannot be set directly.
if self._hs.config.experimental.msc4010_push_rules_account_data:
_check_can_set_account_data_type(account_data_type)

body = parse_json_object_from_request(request)

# If experimental support for MSC3391 is enabled, then providing an empty dict
Expand All @@ -77,19 +99,28 @@ async def on_GET(
if user_id != requester.user.to_string():
raise AuthError(403, "Cannot get account data for other users.")

event = await self.store.get_global_account_data_by_type_for_user(
user_id, account_data_type
)
# Push rules are stored in a separate table and must be queried separately.
if (
self._hs.config.experimental.msc4010_push_rules_account_data
and account_data_type == AccountDataTypes.PUSH_RULES
):
account_data: Optional[JsonDict] = await self.handler.push_rules_for_user(
requester.user
)
else:
account_data = await self.store.get_global_account_data_by_type_for_user(
user_id, account_data_type
)

if event is None:
if account_data is None:
raise NotFoundError("Account data not found")

# If experimental support for MSC3391 is enabled, then this endpoint should
# return a 404 if the content for an account data type is an empty dict.
if self._hs.config.experimental.msc3391_enabled and event == {}:
if self._hs.config.experimental.msc3391_enabled and account_data == {}:
raise NotFoundError("Account data not found")

return 200, event
return 200, account_data


class UnstableAccountDataServlet(RestServlet):
Expand All @@ -108,6 +139,7 @@ class UnstableAccountDataServlet(RestServlet):

def __init__(self, hs: "HomeServer"):
super().__init__()
self._hs = hs
self.auth = hs.get_auth()
self.handler = hs.get_account_data_handler()

Expand All @@ -121,6 +153,10 @@ async def on_DELETE(
if user_id != requester.user.to_string():
raise AuthError(403, "Cannot delete account data for other users.")

# Raise an error if the account data type cannot be set directly.
if self._hs.config.experimental.msc4010_push_rules_account_data:
_check_can_set_account_data_type(account_data_type)

await self.handler.remove_account_data_for_user(user_id, account_data_type)

return 200, {}
Expand Down Expand Up @@ -164,16 +200,19 @@ async def on_PUT(
Codes.INVALID_PARAM,
)

body = parse_json_object_from_request(request)

if account_data_type == "m.fully_read":
# Raise an error if the account data type cannot be set directly.
if self._hs.config.experimental.msc4010_push_rules_account_data:
_check_can_set_account_data_type(account_data_type)
elif account_data_type == ReceiptTypes.FULLY_READ:
raise SynapseError(
405,
"Cannot set m.fully_read through this API."
" Use /rooms/!roomId:server.name/read_markers",
Codes.BAD_JSON,
)

body = parse_json_object_from_request(request)

# If experimental support for MSC3391 is enabled, then providing an empty dict
# as the value for an account data type should be functionally equivalent to
# calling the DELETE method on the same type.
Expand Down Expand Up @@ -208,19 +247,26 @@ async def on_GET(
Codes.INVALID_PARAM,
)

event = await self.store.get_account_data_for_room_and_type(
user_id, room_id, account_data_type
)
# Room-specific push rules are not currently supported.
if (
self._hs.config.experimental.msc4010_push_rules_account_data
and account_data_type == AccountDataTypes.PUSH_RULES
):
account_data: Optional[JsonDict] = {}
else:
account_data = await self.store.get_account_data_for_room_and_type(
user_id, room_id, account_data_type
)

if event is None:
if account_data is None:
raise NotFoundError("Room account data not found")

# If experimental support for MSC3391 is enabled, then this endpoint should
# return a 404 if the content for an account data type is an empty dict.
if self._hs.config.experimental.msc3391_enabled and event == {}:
if self._hs.config.experimental.msc3391_enabled and account_data == {}:
raise NotFoundError("Room account data not found")

return 200, event
return 200, account_data


class UnstableRoomAccountDataServlet(RestServlet):
Expand All @@ -240,6 +286,7 @@ class UnstableRoomAccountDataServlet(RestServlet):

def __init__(self, hs: "HomeServer"):
super().__init__()
self._hs = hs
self.auth = hs.get_auth()
self.handler = hs.get_account_data_handler()

Expand All @@ -261,6 +308,10 @@ async def on_DELETE(
Codes.INVALID_PARAM,
)

# Raise an error if the account data type cannot be set directly.
if self._hs.config.experimental.msc4010_push_rules_account_data:
_check_can_set_account_data_type(account_data_type)

await self.handler.remove_account_data_for_room(
user_id, room_id, account_data_type
)
Expand Down