From 302534c3487b7f3a4be6963d628f919d61bc22a6 Mon Sep 17 00:00:00 2001 From: Andrew Ferrazzutti Date: Thu, 26 Sep 2024 09:25:05 -0400 Subject: [PATCH] Support MSC3757: Restricting who can overwrite a state event (#17513) Link to the MSC: https://github.com/matrix-org/matrix-spec-proposals/pull/3757 --------- Co-authored-by: Quentin Gliech --- changelog.d/17513.feature | 1 + scripts-dev/complement.sh | 1 + synapse/api/room_versions.py | 58 +++++ synapse/event_auth.py | 37 +++- tests/rest/client/test_owned_state.py | 308 ++++++++++++++++++++++++++ 5 files changed, 399 insertions(+), 6 deletions(-) create mode 100644 changelog.d/17513.feature create mode 100644 tests/rest/client/test_owned_state.py diff --git a/changelog.d/17513.feature b/changelog.d/17513.feature new file mode 100644 index 0000000000..21441c7211 --- /dev/null +++ b/changelog.d/17513.feature @@ -0,0 +1 @@ +Add implementation of restricting who can overwrite a state event as proposed by [MSC3757](https://github.com/matrix-org/matrix-spec-proposals/pull/3757). diff --git a/scripts-dev/complement.sh b/scripts-dev/complement.sh index 8fef1ae022..b6dcb96e2c 100755 --- a/scripts-dev/complement.sh +++ b/scripts-dev/complement.sh @@ -220,6 +220,7 @@ test_packages=( ./tests/msc3874 ./tests/msc3890 ./tests/msc3391 + ./tests/msc3757 ./tests/msc3930 ./tests/msc3902 ./tests/msc3967 diff --git a/synapse/api/room_versions.py b/synapse/api/room_versions.py index fbc1d58ecb..4bde385f78 100644 --- a/synapse/api/room_versions.py +++ b/synapse/api/room_versions.py @@ -107,6 +107,8 @@ class RoomVersion: # support the flag. Unknown flags are ignored by the evaluator, making conditions # fail if used. msc3931_push_features: Tuple[str, ...] # values from PushRuleRoomFlag + # MSC3757: Restricting who can overwrite a state event + msc3757_enabled: bool class RoomVersions: @@ -128,6 +130,7 @@ class RoomVersions: knock_restricted_join_rule=False, enforce_int_power_levels=False, msc3931_push_features=(), + msc3757_enabled=False, ) V2 = RoomVersion( "2", @@ -147,6 +150,7 @@ class RoomVersions: knock_restricted_join_rule=False, enforce_int_power_levels=False, msc3931_push_features=(), + msc3757_enabled=False, ) V3 = RoomVersion( "3", @@ -166,6 +170,7 @@ class RoomVersions: knock_restricted_join_rule=False, enforce_int_power_levels=False, msc3931_push_features=(), + msc3757_enabled=False, ) V4 = RoomVersion( "4", @@ -185,6 +190,7 @@ class RoomVersions: knock_restricted_join_rule=False, enforce_int_power_levels=False, msc3931_push_features=(), + msc3757_enabled=False, ) V5 = RoomVersion( "5", @@ -204,6 +210,7 @@ class RoomVersions: knock_restricted_join_rule=False, enforce_int_power_levels=False, msc3931_push_features=(), + msc3757_enabled=False, ) V6 = RoomVersion( "6", @@ -223,6 +230,7 @@ class RoomVersions: knock_restricted_join_rule=False, enforce_int_power_levels=False, msc3931_push_features=(), + msc3757_enabled=False, ) V7 = RoomVersion( "7", @@ -242,6 +250,7 @@ class RoomVersions: knock_restricted_join_rule=False, enforce_int_power_levels=False, msc3931_push_features=(), + msc3757_enabled=False, ) V8 = RoomVersion( "8", @@ -261,6 +270,7 @@ class RoomVersions: knock_restricted_join_rule=False, enforce_int_power_levels=False, msc3931_push_features=(), + msc3757_enabled=False, ) V9 = RoomVersion( "9", @@ -280,6 +290,7 @@ class RoomVersions: knock_restricted_join_rule=False, enforce_int_power_levels=False, msc3931_push_features=(), + msc3757_enabled=False, ) V10 = RoomVersion( "10", @@ -299,6 +310,7 @@ class RoomVersions: knock_restricted_join_rule=True, enforce_int_power_levels=True, msc3931_push_features=(), + msc3757_enabled=False, ) MSC1767v10 = RoomVersion( # MSC1767 (Extensible Events) based on room version "10" @@ -319,6 +331,28 @@ class RoomVersions: knock_restricted_join_rule=True, enforce_int_power_levels=True, msc3931_push_features=(PushRuleRoomFlag.EXTENSIBLE_EVENTS,), + msc3757_enabled=False, + ) + MSC3757v10 = RoomVersion( + # MSC3757 (Restricting who can overwrite a state event) based on room version "10" + "org.matrix.msc3757.10", + RoomDisposition.UNSTABLE, + EventFormatVersions.ROOM_V4_PLUS, + StateResolutionVersions.V2, + enforce_key_validity=True, + special_case_aliases_auth=False, + strict_canonicaljson=True, + limit_notifications_power_levels=True, + implicit_room_creator=False, + updated_redaction_rules=False, + restricted_join_rule=True, + restricted_join_rule_fix=True, + knock_join_rule=True, + msc3389_relation_redactions=False, + knock_restricted_join_rule=True, + enforce_int_power_levels=True, + msc3931_push_features=(), + msc3757_enabled=True, ) V11 = RoomVersion( "11", @@ -338,6 +372,28 @@ class RoomVersions: knock_restricted_join_rule=True, enforce_int_power_levels=True, msc3931_push_features=(), + msc3757_enabled=False, + ) + MSC3757v11 = RoomVersion( + # MSC3757 (Restricting who can overwrite a state event) based on room version "11" + "org.matrix.msc3757.11", + RoomDisposition.UNSTABLE, + EventFormatVersions.ROOM_V4_PLUS, + StateResolutionVersions.V2, + enforce_key_validity=True, + special_case_aliases_auth=False, + strict_canonicaljson=True, + limit_notifications_power_levels=True, + implicit_room_creator=True, # Used by MSC3820 + updated_redaction_rules=True, # Used by MSC3820 + restricted_join_rule=True, + restricted_join_rule_fix=True, + knock_join_rule=True, + msc3389_relation_redactions=False, + knock_restricted_join_rule=True, + enforce_int_power_levels=True, + msc3931_push_features=(), + msc3757_enabled=True, ) @@ -355,6 +411,8 @@ class RoomVersions: RoomVersions.V9, RoomVersions.V10, RoomVersions.V11, + RoomVersions.MSC3757v10, + RoomVersions.MSC3757v11, ) } diff --git a/synapse/event_auth.py b/synapse/event_auth.py index b834547d11..c208b900c5 100644 --- a/synapse/event_auth.py +++ b/synapse/event_auth.py @@ -388,6 +388,7 @@ def check_state_dependent_auth_rules( RoomVersions.V9, RoomVersions.V10, RoomVersions.MSC1767v10, + RoomVersions.MSC3757v10, } @@ -790,9 +791,10 @@ def get_send_level( def _can_send_event(event: "EventBase", auth_events: StateMap["EventBase"]) -> bool: + state_key = event.get_state_key() power_levels_event = get_power_level_event(auth_events) - send_level = get_send_level(event.type, event.get("state_key"), power_levels_event) + send_level = get_send_level(event.type, state_key, power_levels_event) user_level = get_user_power_level(event.user_id, auth_events) if user_level < send_level: @@ -803,11 +805,34 @@ def _can_send_event(event: "EventBase", auth_events: StateMap["EventBase"]) -> b errcode=Codes.INSUFFICIENT_POWER, ) - # Check state_key - if hasattr(event, "state_key"): - if event.state_key.startswith("@"): - if event.state_key != event.user_id: - raise AuthError(403, "You are not allowed to set others state") + if ( + state_key is not None + and state_key.startswith("@") + and state_key != event.user_id + ): + if event.room_version.msc3757_enabled: + try: + colon_idx = state_key.index(":", 1) + suffix_idx = state_key.find("_", colon_idx + 1) + state_key_user_id = ( + state_key[:suffix_idx] if suffix_idx != -1 else state_key + ) + if not UserID.is_valid(state_key_user_id): + raise ValueError + except ValueError: + raise SynapseError( + 400, + "State key neither equals a valid user ID, nor starts with one plus an underscore", + errcode=Codes.BAD_JSON, + ) + if ( + # sender is owner of the state key + state_key_user_id == event.user_id + # sender has higher PL than the owner of the state key + or user_level > get_user_power_level(state_key_user_id, auth_events) + ): + return True + raise AuthError(403, "You are not allowed to set others state") return True diff --git a/tests/rest/client/test_owned_state.py b/tests/rest/client/test_owned_state.py new file mode 100644 index 0000000000..5fb5767676 --- /dev/null +++ b/tests/rest/client/test_owned_state.py @@ -0,0 +1,308 @@ +from http import HTTPStatus + +from parameterized import parameterized_class + +from twisted.test.proto_helpers import MemoryReactor + +from synapse.api.errors import Codes +from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersions +from synapse.rest import admin +from synapse.rest.client import login, room +from synapse.server import HomeServer +from synapse.types import JsonDict +from synapse.util import Clock + +from tests.unittest import HomeserverTestCase + +_STATE_EVENT_TEST_TYPE = "com.example.test" + +# To stress-test parsing, include separator & sigil characters +_STATE_KEY_SUFFIX = "_state_key_suffix:!@#$123" + + +class OwnedStateBase(HomeserverTestCase): + servlets = [ + admin.register_servlets, + room.register_servlets, + login.register_servlets, + ] + + def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: + self.creator_user_id = self.register_user("creator", "pass") + self.creator_access_token = self.login("creator", "pass") + self.user1_user_id = self.register_user("user1", "pass") + self.user1_access_token = self.login("user1", "pass") + + self.room_id = self.helper.create_room_as( + self.creator_user_id, + tok=self.creator_access_token, + is_public=True, + extra_content={ + "power_level_content_override": { + "events": { + _STATE_EVENT_TEST_TYPE: 0, + }, + }, + }, + ) + + self.helper.join( + room=self.room_id, user=self.user1_user_id, tok=self.user1_access_token + ) + + +class WithoutOwnedStateTestCase(OwnedStateBase): + def default_config(self) -> JsonDict: + config = super().default_config() + config["default_room_version"] = RoomVersions.V10.identifier + return config + + def test_user_can_set_state_with_own_userid_key(self) -> None: + self.helper.send_state( + self.room_id, + _STATE_EVENT_TEST_TYPE, + {}, + state_key=f"{self.user1_user_id}", + tok=self.user1_access_token, + expect_code=HTTPStatus.OK, + ) + + def test_room_creator_cannot_set_state_with_own_suffixed_key(self) -> None: + body = self.helper.send_state( + self.room_id, + _STATE_EVENT_TEST_TYPE, + {}, + state_key=f"{self.creator_user_id}{_STATE_KEY_SUFFIX}", + tok=self.creator_access_token, + expect_code=HTTPStatus.FORBIDDEN, + ) + + self.assertEqual( + body["errcode"], + Codes.FORBIDDEN, + body, + ) + + def test_room_creator_cannot_set_state_with_other_userid_key(self) -> None: + body = self.helper.send_state( + self.room_id, + _STATE_EVENT_TEST_TYPE, + {}, + state_key=f"{self.user1_user_id}", + tok=self.creator_access_token, + expect_code=HTTPStatus.FORBIDDEN, + ) + + self.assertEqual( + body["errcode"], + Codes.FORBIDDEN, + body, + ) + + def test_room_creator_cannot_set_state_with_other_suffixed_key(self) -> None: + body = self.helper.send_state( + self.room_id, + _STATE_EVENT_TEST_TYPE, + {}, + state_key=f"{self.user1_user_id}{_STATE_KEY_SUFFIX}", + tok=self.creator_access_token, + expect_code=HTTPStatus.FORBIDDEN, + ) + + self.assertEqual( + body["errcode"], + Codes.FORBIDDEN, + body, + ) + + def test_room_creator_cannot_set_state_with_nonmember_userid_key(self) -> None: + body = self.helper.send_state( + self.room_id, + _STATE_EVENT_TEST_TYPE, + {}, + state_key="@notinroom:hs2", + tok=self.creator_access_token, + expect_code=HTTPStatus.FORBIDDEN, + ) + + self.assertEqual( + body["errcode"], + Codes.FORBIDDEN, + body, + ) + + def test_room_creator_cannot_set_state_with_malformed_userid_key(self) -> None: + body = self.helper.send_state( + self.room_id, + _STATE_EVENT_TEST_TYPE, + {}, + state_key="@oops", + tok=self.creator_access_token, + expect_code=HTTPStatus.FORBIDDEN, + ) + + self.assertEqual( + body["errcode"], + Codes.FORBIDDEN, + body, + ) + + +@parameterized_class( + ("room_version",), + [(i,) for i, v in KNOWN_ROOM_VERSIONS.items() if v.msc3757_enabled], +) +class MSC3757OwnedStateTestCase(OwnedStateBase): + room_version: str + + def default_config(self) -> JsonDict: + config = super().default_config() + config["default_room_version"] = self.room_version + return config + + def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: + super().prepare(reactor, clock, hs) + + self.user2_user_id = self.register_user("user2", "pass") + self.user2_access_token = self.login("user2", "pass") + + self.helper.join( + room=self.room_id, user=self.user2_user_id, tok=self.user2_access_token + ) + + def test_user_can_set_state_with_own_suffixed_key(self) -> None: + self.helper.send_state( + self.room_id, + _STATE_EVENT_TEST_TYPE, + {}, + state_key=f"{self.user1_user_id}{_STATE_KEY_SUFFIX}", + tok=self.user1_access_token, + expect_code=HTTPStatus.OK, + ) + + def test_room_creator_can_set_state_with_other_userid_key(self) -> None: + self.helper.send_state( + self.room_id, + _STATE_EVENT_TEST_TYPE, + {}, + state_key=f"{self.user1_user_id}", + tok=self.creator_access_token, + expect_code=HTTPStatus.OK, + ) + + def test_room_creator_can_set_state_with_other_suffixed_key(self) -> None: + self.helper.send_state( + self.room_id, + _STATE_EVENT_TEST_TYPE, + {}, + state_key=f"{self.user1_user_id}{_STATE_KEY_SUFFIX}", + tok=self.creator_access_token, + expect_code=HTTPStatus.OK, + ) + + def test_user_cannot_set_state_with_other_userid_key(self) -> None: + body = self.helper.send_state( + self.room_id, + _STATE_EVENT_TEST_TYPE, + {}, + state_key=f"{self.user2_user_id}", + tok=self.user1_access_token, + expect_code=HTTPStatus.FORBIDDEN, + ) + + self.assertEqual( + body["errcode"], + Codes.FORBIDDEN, + body, + ) + + def test_user_cannot_set_state_with_other_suffixed_key(self) -> None: + body = self.helper.send_state( + self.room_id, + _STATE_EVENT_TEST_TYPE, + {}, + state_key=f"{self.user2_user_id}{_STATE_KEY_SUFFIX}", + tok=self.user1_access_token, + expect_code=HTTPStatus.FORBIDDEN, + ) + + self.assertEqual( + body["errcode"], + Codes.FORBIDDEN, + body, + ) + + def test_user_cannot_set_state_with_unseparated_suffixed_key(self) -> None: + body = self.helper.send_state( + self.room_id, + _STATE_EVENT_TEST_TYPE, + {}, + state_key=f"{self.user1_user_id}{_STATE_KEY_SUFFIX[1:]}", + tok=self.user1_access_token, + expect_code=HTTPStatus.FORBIDDEN, + ) + + self.assertEqual( + body["errcode"], + Codes.FORBIDDEN, + body, + ) + + def test_user_cannot_set_state_with_misplaced_userid_in_key(self) -> None: + body = self.helper.send_state( + self.room_id, + _STATE_EVENT_TEST_TYPE, + {}, + # Still put @ at start of state key, because without it, there is no write protection at all + state_key=f"@prefix_{self.user1_user_id}{_STATE_KEY_SUFFIX}", + tok=self.user1_access_token, + expect_code=HTTPStatus.FORBIDDEN, + ) + + self.assertEqual( + body["errcode"], + Codes.FORBIDDEN, + body, + ) + + def test_room_creator_can_set_state_with_nonmember_userid_key(self) -> None: + self.helper.send_state( + self.room_id, + _STATE_EVENT_TEST_TYPE, + {}, + state_key="@notinroom:hs2", + tok=self.creator_access_token, + expect_code=HTTPStatus.OK, + ) + + def test_room_creator_cannot_set_state_with_malformed_userid_key(self) -> None: + body = self.helper.send_state( + self.room_id, + _STATE_EVENT_TEST_TYPE, + {}, + state_key="@oops", + tok=self.creator_access_token, + expect_code=HTTPStatus.BAD_REQUEST, + ) + + self.assertEqual( + body["errcode"], + Codes.BAD_JSON, + body, + ) + + def test_room_creator_cannot_set_state_with_improperly_suffixed_key(self) -> None: + body = self.helper.send_state( + self.room_id, + _STATE_EVENT_TEST_TYPE, + {}, + state_key=f"{self.creator_user_id}@{_STATE_KEY_SUFFIX[1:]}", + tok=self.creator_access_token, + expect_code=HTTPStatus.BAD_REQUEST, + ) + + self.assertEqual( + body["errcode"], + Codes.BAD_JSON, + body, + )