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.
This commit is contained in:
Patrick Cloke 2023-05-09 10:34:10 -04:00 committed by GitHub
parent 2bfe3f0b81
commit 4b4e0dc3ce
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 95 additions and 35 deletions

1
changelog.d/15554.bugfix Normal file
View File

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

View File

@ -1 +0,0 @@
Use account data constants in more places.

1
changelog.d/15555.bugfix Normal file
View File

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

View File

@ -202,3 +202,8 @@ class ExperimentalConfig(Config):
# 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
)

View File

@ -11,14 +11,15 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
from typing import TYPE_CHECKING, List, Optional, Union
from typing import TYPE_CHECKING, Any, Dict, List, Optional, Union
import attr
from synapse.api.errors import SynapseError, UnrecognizedRequestError
from synapse.push.clientformat import format_push_rules_for_user
from synapse.storage.push_rule import RuleNotFoundException
from synapse.synapse_rust.push import get_base_rule_ids
from synapse.types import JsonDict
from synapse.types import JsonDict, UserID
if TYPE_CHECKING:
from synapse.server import HomeServer
@ -115,6 +116,17 @@ class PushRulesHandler:
stream_id = self._main_store.get_max_push_rules_stream_id()
self._notifier.on_new_event("push_rules_key", stream_id, users=[user_id])
async def push_rules_for_user(
self, user: UserID
) -> Dict[str, Dict[str, List[Dict[str, Any]]]]:
"""
Push rules aren't really account data, but get formatted as such for /sync.
"""
user_id = user.to_string()
rules_raw = await self._main_store.get_push_rules_for_user(user_id)
rules = format_push_rules_for_user(user, rules_raw)
return rules
def check_actions(actions: List[Union[str, JsonDict]]) -> None:
"""Check if the given actions are spec compliant.

View File

@ -50,7 +50,6 @@ from synapse.logging.opentracing import (
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 @@ class SyncHandler:
self.notifier = hs.get_notifier()
self.presence_handler = hs.get_presence_handler()
self._relations_handler = hs.get_relations_handler()
self._push_rules_handler = hs.get_push_rules_handler()
self.event_sources = hs.get_event_sources()
self.clock = hs.get_clock()
self.state = hs.get_state_handler()
@ -428,12 +428,6 @@ class SyncHandler:
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",
@ -1779,7 +1773,7 @@ class SyncHandler:
global_account_data = dict(global_account_data)
global_account_data[
AccountDataTypes.PUSH_RULES
] = await self.push_rules_for_user(sync_config.user)
] = await self._push_rules_handler.push_rules_for_user(sync_config.user)
else:
all_global_account_data = await self.store.get_global_account_data_for_user(
user_id
@ -1788,7 +1782,7 @@ class SyncHandler:
global_account_data = dict(all_global_account_data)
global_account_data[
AccountDataTypes.PUSH_RULES
] = await self.push_rules_for_user(sync_config.user)
] = await self._push_rules_handler.push_rules_for_user(sync_config.user)
account_data_for_user = (
await sync_config.filter_collection.filter_global_account_data(

View File

@ -22,7 +22,7 @@ from synapse.types import UserID
def format_push_rules_for_user(
user: UserID, ruleslist: FilteredPushRules
) -> Dict[str, Dict[str, list]]:
) -> Dict[str, Dict[str, List[Dict[str, Any]]]]:
"""Converts a list of rawrules and a enabled map into nested dictionaries
to match the Matrix client-server format for push rules"""

View File

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

View File

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