Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support MSC3757: Restricting who can overwrite a state event #17513

Merged
merged 29 commits into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
7855b97
Support MSC3757
AndrewFerr Jun 13, 2024
6ec215b
Add changelog
AndrewFerr Aug 1, 2024
6a69213
Run power level tests on MSC3757 rooms
AndrewFerr Aug 7, 2024
e481238
Check if @-prefixed state key is not a user ID
AndrewFerr Aug 8, 2024
248025d
Test MSC3757 auth rules
AndrewFerr Aug 8, 2024
a12238d
Support MSC3757 for room versions 9 and 11
AndrewFerr Aug 8, 2024
663da8d
Skip redundant characters for some string searches
AndrewFerr Aug 12, 2024
c4cc4d8
Add explicit field to room version for MSC3757
AndrewFerr Aug 15, 2024
5657863
Parametrize fewer power level unit tests
AndrewFerr Aug 15, 2024
e9d22ba
Lint
AndrewFerr Aug 15, 2024
228d6b4
Return 400 for @-prefixed, non-userID state key
AndrewFerr Sep 4, 2024
e175871
Define test event type string once
AndrewFerr Sep 4, 2024
a841921
Lint
AndrewFerr Sep 4, 2024
772d35a
Refactor state key parsing
AndrewFerr Sep 5, 2024
ba2ce94
Split & comment allow conditions
AndrewFerr Sep 5, 2024
77af387
Drop support for room version 9
AndrewFerr Sep 5, 2024
6f429b6
Refactor unit tests
AndrewFerr Sep 5, 2024
defeb6e
Use one expression for commented allow conditions
AndrewFerr Sep 5, 2024
426f52b
Narrow caught error type
AndrewFerr Sep 9, 2024
8c203af
Remove license header from new test file
AndrewFerr Sep 11, 2024
0d901f7
Run Complement tests
AndrewFerr Sep 16, 2024
b16fa10
Make suffix test actually use suffix
AndrewFerr Sep 23, 2024
77fc9d2
Test suffixed key without separator
AndrewFerr Sep 23, 2024
c0dceaf
Lock non-owned-state tests to non-MSC room version
AndrewFerr Sep 23, 2024
e1fa3ef
Test user ID in middle of state key
AndrewFerr Sep 23, 2024
7bcd7a0
Rename admin to "room creator"
AndrewFerr Sep 25, 2024
d28c17b
Test state with key of a user ID not in the room
AndrewFerr Sep 25, 2024
792b065
Always check the errcode in failure responses
AndrewFerr Sep 25, 2024
89b20d4
Merge branch 'develop' into af/msc3757
sandhose Sep 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/17513.feature
Original file line number Diff line number Diff line change
@@ -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).
80 changes: 80 additions & 0 deletions synapse/api/room_versions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -280,6 +290,28 @@ class RoomVersions:
knock_restricted_join_rule=False,
enforce_int_power_levels=False,
msc3931_push_features=(),
msc3757_enabled=False,
)
MSC3757v9 = RoomVersion(
# MSC3757 (Restricting who can overwrite a state event) based on room version "9"
"org.matrix.msc3757.9",
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=False,
enforce_int_power_levels=False,
msc3931_push_features=(),
msc3757_enabled=True,
)
V10 = RoomVersion(
"10",
Expand All @@ -299,6 +331,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"
Expand All @@ -319,6 +352,28 @@ class RoomVersions:
knock_restricted_join_rule=True,
enforce_int_power_levels=True,
msc3931_push_features=(PushRuleRoomFlag.EXTENSIBLE_EVENTS,),
msc3757_enabled=False,
)
MSC3757v10 = RoomVersion(
sandhose marked this conversation as resolved.
Show resolved Hide resolved
# 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",
Expand All @@ -338,6 +393,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,
)


Expand All @@ -355,6 +432,9 @@ class RoomVersions:
RoomVersions.V9,
RoomVersions.V10,
RoomVersions.V11,
RoomVersions.MSC3757v9,
RoomVersions.MSC3757v10,
RoomVersions.MSC3757v11,
AndrewFerr marked this conversation as resolved.
Show resolved Hide resolved
)
}

Expand Down
34 changes: 28 additions & 6 deletions synapse/event_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -386,8 +386,10 @@ def check_state_dependent_auth_rules(
RoomVersions.V7,
RoomVersions.V8,
RoomVersions.V9,
RoomVersions.MSC3757v9,
RoomVersions.V10,
RoomVersions.MSC1767v10,
RoomVersions.MSC3757v10,
}


Expand Down Expand Up @@ -790,9 +792,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:
Expand All @@ -803,11 +806,30 @@ 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:
colon_idx = state_key.find(":", 1)
if colon_idx != -1:
suffix_idx = state_key.find("_", colon_idx + 1)
AndrewFerr marked this conversation as resolved.
Show resolved Hide resolved
state_key_user_id = (
state_key[:suffix_idx] if suffix_idx != -1 else state_key
AndrewFerr marked this conversation as resolved.
Show resolved Hide resolved
)
if not UserID.is_valid(state_key_user_id):
raise UnstableSpecAuthError(
403,
"State key is not a valid user ID",
errcode=Codes.BAD_JSON,
)
if (
state_key_user_id == event.user_id
or user_level > get_user_power_level(state_key_user_id, auth_events)
):
return True
AndrewFerr marked this conversation as resolved.
Show resolved Hide resolved
raise AuthError(403, "You are not allowed to set others state")

return True

Expand Down
79 changes: 75 additions & 4 deletions tests/rest/client/test_power_levels.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,14 @@
#
#
from http import HTTPStatus
from typing import Optional

from parameterized import parameterized_class

from twisted.test.proto_helpers import MemoryReactor

from synapse.api.errors import Codes
from synapse.api.room_versions import RoomVersions
from synapse.events.utils import CANONICALJSON_MAX_INT, CANONICALJSON_MIN_INT
from synapse.rest import admin
from synapse.rest.client import login, room, sync
Expand All @@ -32,9 +36,10 @@
from tests.unittest import HomeserverTestCase


class PowerLevelsTestCase(HomeserverTestCase):
class BasePowerLevelsTestCase(HomeserverTestCase):
"""Tests that power levels are enforced in various situations"""

room_version: Optional[str] = None
servlets = [
admin.register_servlets,
room.register_servlets,
Expand All @@ -58,7 +63,9 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:

# Create a room
self.room_id = self.helper.create_room_as(
self.admin_user_id, tok=self.admin_access_token
self.admin_user_id,
tok=self.admin_access_token,
room_version=self.room_version,
)

# Invite the other users
Expand Down Expand Up @@ -100,6 +107,8 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
tok=self.admin_access_token,
)


class PowerLevelsTestCase(BasePowerLevelsTestCase):
def test_non_admins_cannot_enable_room_encryption(self) -> None:
# have the mod try to enable room encryption
self.helper.send_state(
Expand Down Expand Up @@ -149,7 +158,9 @@ def test_non_admins_cannot_send_server_acl(self) -> None:
def test_non_admins_cannot_tombstone_room(self) -> None:
# Create another room that will serve as our "upgraded room"
self.upgraded_room_id = self.helper.create_room_as(
self.admin_user_id, tok=self.admin_access_token
self.admin_user_id,
tok=self.admin_access_token,
room_version=self.room_version,
)

# have the mod try to send a tombstone event
Expand Down Expand Up @@ -203,7 +214,9 @@ def test_admins_can_send_server_acl(self) -> None:
def test_admins_can_tombstone_room(self) -> None:
# Create another room that will serve as our "upgraded room"
self.upgraded_room_id = self.helper.create_room_as(
self.admin_user_id, tok=self.admin_access_token
self.admin_user_id,
tok=self.admin_access_token,
room_version=self.room_version,
)

# have the admin try to send a tombstone event
Expand Down Expand Up @@ -293,3 +306,61 @@ def test_cannot_set_unsafe_small_power_levels(self) -> None:
Codes.BAD_JSON,
body,
)


@parameterized_class(
("room_version", "allows_owned_state"),
[
(rv.identifier, rv.msc3757_enabled)
for rv in [
RoomVersions.V10,
RoomVersions.MSC3757v10,
AndrewFerr marked this conversation as resolved.
Show resolved Hide resolved
]
],
)
class MSC3757PowerLevelsTestCase(BasePowerLevelsTestCase):
allows_owned_state: bool
AndrewFerr marked this conversation as resolved.
Show resolved Hide resolved

def test_can_set_own_state(self) -> None:
self.helper.send_state(
self.room_id,
"org.matrix.msc3757.test",
{},
state_key=f"{self.mod_user_id}_suffix",
tok=self.mod_access_token,
expect_code=(
HTTPStatus.OK if self.allows_owned_state else HTTPStatus.FORBIDDEN
),
)

def test_admins_can_set_others_state(self) -> None:
self.helper.send_state(
self.room_id,
"org.matrix.msc3757.test",
{},
state_key=f"{self.mod_user_id}_suffix",
tok=self.admin_access_token,
expect_code=(
HTTPStatus.OK if self.allows_owned_state else HTTPStatus.FORBIDDEN
),
)

def test_non_admins_cannot_set_others_state(self) -> None:
self.helper.send_state(
self.room_id,
"org.matrix.msc3757.test",
{},
state_key=f"{self.admin_user_id}_suffix",
tok=self.mod_access_token,
expect_code=HTTPStatus.FORBIDDEN,
)

def test_cannot_set_state_with_non_user_id_key(self) -> None:
self.helper.send_state(
self.room_id,
"org.matrix.msc3757.test",
{},
state_key=f"{self.admin_user_id}@suffix",
tok=self.admin_access_token,
expect_code=HTTPStatus.FORBIDDEN,
)
Loading