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

changelog.d/15554.bugfix

@@ -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.

synapse/api/constants.py

@@ -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:

synapse/config/experimental.py

@@ -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
+        )

synapse/handlers/account_data.py

@@ -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,
@@ -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]]:
+        """
+        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"):

synapse/handlers/read_marker.py

@@ -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:
@@ -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
@@ -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
                 )

synapse/handlers/sync.py

@@ -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
@@ -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()
@@ -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",
@@ -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:
@@ -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(

synapse/rest/client/account_data.py

@@ -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
@@ -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
@@ -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
@@ -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):
@@ -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()
 
@@ -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, {}
@@ -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.
@@ -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):
@@ -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()
 
@@ -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
         )