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

m.push_rules, like m.fully_read, is a special account data type that cannot
be set using the normal /account_data endpoint. Return an error instead
of allowing data that will not be used to be stored.

2 files changed, 70 insertions, 22 deletions

diff --git a/synapse/rest/client/account_data.py b/synapse/rest/client/account_data.py
index 8eebb21c76..b1f9e9dc9b 100644
--- a/synapse/rest/client/account_data.py
+++ b/synapse/rest/client/account_data.py
@@ -13,9 +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 ReceiptTypes
+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
@@ -30,6 +30,23 @@ if TYPE_CHECKING:
 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
@@ -47,6 +64,7 @@ class AccountDataServlet(RestServlet):
         self.auth = hs.get_auth()
         self.store = hs.get_datastores().main
         self.handler = hs.get_account_data_handler()
+        self._push_rules_handler = hs.get_push_rules_handler()
 
     async def on_PUT(
         self, request: SynapseRequest, user_id: str, account_data_type: str
@@ -55,6 +73,10 @@ class AccountDataServlet(RestServlet):
         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
@@ -78,19 +100,28 @@ class AccountDataServlet(RestServlet):
         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._push_rules_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):
@@ -109,6 +140,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()
 
@@ -122,6 +154,10 @@ class UnstableAccountDataServlet(RestServlet):
         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, {}
@@ -165,9 +201,10 @@ class RoomAccountDataServlet(RestServlet):
                 Codes.INVALID_PARAM,
             )
 
-        body = parse_json_object_from_request(request)
-
-        if account_data_type == ReceiptTypes.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."
@@ -175,6 +212,8 @@ class RoomAccountDataServlet(RestServlet):
                 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.
@@ -209,19 +248,26 @@ class RoomAccountDataServlet(RestServlet):
                 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):
@@ -241,6 +287,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()
 
@@ -262,6 +309,10 @@ class UnstableRoomAccountDataServlet(RestServlet):
                 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
         )
diff --git a/synapse/rest/client/push_rule.py b/synapse/rest/client/push_rule.py
index 1147b6f8ec..5c9fece3ba 100644
--- a/synapse/rest/client/push_rule.py
+++ b/synapse/rest/client/push_rule.py
@@ -28,7 +28,6 @@ from synapse.http.servlet import (
     parse_string,
 )
 from synapse.http.site import SynapseRequest
-from synapse.push.clientformat import format_push_rules_for_user
 from synapse.push.rulekinds import PRIORITY_CLASS_MAP
 from synapse.rest.client._base import client_patterns
 from synapse.storage.push_rule import InconsistentRuleException, RuleNotFoundException
@@ -146,14 +145,12 @@ class PushRuleRestServlet(RestServlet):
 
     async def on_GET(self, request: SynapseRequest, path: str) -> Tuple[int, JsonDict]:
         requester = await self.auth.get_user_by_req(request)
-        user_id = requester.user.to_string()
+        requester.user.to_string()
 
         # we build up the full structure and then decide which bits of it
         # to send which means doing unnecessary work sometimes but is
         # is probably not going to make a whole lot of difference
-        rules_raw = await self.store.get_push_rules_for_user(user_id)
-
-        rules = format_push_rules_for_user(requester.user, rules_raw)
+        rules = await self._push_rules_handler.push_rules_for_user(requester.user)
 
         path_parts = path.split("/")[1:]