Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Add support for room version 10 #13220

Merged
merged 9 commits into from
Jul 13, 2022
Merged

Add support for room version 10 #13220

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
169fcbc
add check for non-int powerlevels if supported
H-Shay Jul 7, 2022
3e7242d
add tests to see that events in room v10 fail with stringy powerlevels
H-Shay Jul 7, 2022
e2f3113
newsfragement
H-Shay Jul 7, 2022
017cf51
fix newsfragment number
H-Shay Jul 7, 2022
808dd94
requested changes
H-Shay Jul 12, 2022
b071842
tweaks to test to accomodate suggested changes
H-Shay Jul 12, 2022
da2d7a2
requested changes
H-Shay Jul 13, 2022
65fd49e
suggested changes
H-Shay Jul 13, 2022
77a1a57
lint
H-Shay Jul 13, 2022
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/13220.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add support for room version 10.
33 changes: 33 additions & 0 deletions synapse/api/room_versions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ class RoomVersion:
# MSC3787: Adds support for a `knock_restricted` join rule, mixing concepts of
# knocks and restricted join rules into the same join condition.
msc3787_knock_restricted_join_rule: bool
# MSC3667: Enforce integer power levels
msc3667_int_only_power_levels: bool


class RoomVersions:
Expand All @@ -103,6 +105,7 @@ class RoomVersions:
msc2716_historical=False,
msc2716_redactions=False,
msc3787_knock_restricted_join_rule=False,
msc3667_int_only_power_levels=False,
)
V2 = RoomVersion(
"2",
Expand All @@ -120,6 +123,7 @@ class RoomVersions:
msc2716_historical=False,
msc2716_redactions=False,
msc3787_knock_restricted_join_rule=False,
msc3667_int_only_power_levels=False,
)
V3 = RoomVersion(
"3",
Expand All @@ -137,6 +141,7 @@ class RoomVersions:
msc2716_historical=False,
msc2716_redactions=False,
msc3787_knock_restricted_join_rule=False,
msc3667_int_only_power_levels=False,
)
V4 = RoomVersion(
"4",
Expand All @@ -154,6 +159,7 @@ class RoomVersions:
msc2716_historical=False,
msc2716_redactions=False,
msc3787_knock_restricted_join_rule=False,
msc3667_int_only_power_levels=False,
)
V5 = RoomVersion(
"5",
Expand All @@ -171,6 +177,7 @@ class RoomVersions:
msc2716_historical=False,
msc2716_redactions=False,
msc3787_knock_restricted_join_rule=False,
msc3667_int_only_power_levels=False,
)
V6 = RoomVersion(
"6",
Expand All @@ -188,6 +195,7 @@ class RoomVersions:
msc2716_historical=False,
msc2716_redactions=False,
msc3787_knock_restricted_join_rule=False,
msc3667_int_only_power_levels=False,
)
MSC2176 = RoomVersion(
"org.matrix.msc2176",
Expand All @@ -205,6 +213,7 @@ class RoomVersions:
msc2716_historical=False,
msc2716_redactions=False,
msc3787_knock_restricted_join_rule=False,
msc3667_int_only_power_levels=False,
)
V7 = RoomVersion(
"7",
Expand All @@ -222,6 +231,7 @@ class RoomVersions:
msc2716_historical=False,
msc2716_redactions=False,
msc3787_knock_restricted_join_rule=False,
msc3667_int_only_power_levels=False,
)
V8 = RoomVersion(
"8",
Expand All @@ -239,6 +249,7 @@ class RoomVersions:
msc2716_historical=False,
msc2716_redactions=False,
msc3787_knock_restricted_join_rule=False,
msc3667_int_only_power_levels=False,
)
V9 = RoomVersion(
"9",
Expand All @@ -256,6 +267,7 @@ class RoomVersions:
msc2716_historical=False,
msc2716_redactions=False,
msc3787_knock_restricted_join_rule=False,
msc3667_int_only_power_levels=False,
)
MSC2716v3 = RoomVersion(
"org.matrix.msc2716v3",
Expand All @@ -273,6 +285,7 @@ class RoomVersions:
msc2716_historical=True,
msc2716_redactions=True,
msc3787_knock_restricted_join_rule=False,
msc3667_int_only_power_levels=False,
)
MSC3787 = RoomVersion(
"org.matrix.msc3787",
Expand All @@ -290,6 +303,25 @@ class RoomVersions:
msc2716_historical=False,
msc2716_redactions=False,
msc3787_knock_restricted_join_rule=True,
msc3667_int_only_power_levels=False,
)
V10 = RoomVersion(
"10",
RoomDisposition.STABLE,
EventFormatVersions.V3,
StateResolutionVersions.V2,
enforce_key_validity=True,
special_case_aliases_auth=False,
strict_canonicaljson=True,
limit_notifications_power_levels=True,
msc2176_redaction_rules=False,
msc3083_join_rules=True,
msc3375_redaction_rules=True,
msc2403_knocking=True,
msc2716_historical=False,
msc2716_redactions=False,
msc3787_knock_restricted_join_rule=True,
msc3667_int_only_power_levels=True,
)


Expand All @@ -308,6 +340,7 @@ class RoomVersions:
RoomVersions.V9,
RoomVersions.MSC2716v3,
RoomVersions.MSC3787,
RoomVersions.V10,
)
}

Expand Down
25 changes: 25 additions & 0 deletions synapse/event_auth.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,31 @@ def validate_event_for_room_version(event: "EventBase") -> None:
Raises:
SynapseError if there is a problem with the event
"""
# Reject events with stringy power levels if required by room version
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the check here because the spec seems to indicate that the power level check failure should be "rejected under the authorization rules " and this was an auth check function relating to room versions, so it seemed the appropriate place. If it's not I am happy to move it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move this to _check_power_levels, where we apply the rest of the auth rules under item 10. in the spec (https://spec.matrix.org/latest/rooms/v10/#authorization-rules) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, done in 808dd94.

if (
event.type == EventTypes.PowerLevels
and event.room_version.msc3667_int_only_power_levels
):
for key, value in event.content.items():
if key in [
squahtx marked this conversation as resolved.
Show resolved Hide resolved
"users_default",
"events_default",
"state_default",
"ban",
"reject",
H-Shay marked this conversation as resolved.
Show resolved Hide resolved
"kick",
"invite",
]:
if not isinstance(value, int):
raise AuthError(403, f"{value} must be an integer.")
if key in ["events", "notifications"]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this not also need users?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to include users too. The spec wording is:

  1. [New in this version] If either of the keys events or notifications in content are present and not a dictionary with values that are integers, reject.
  2. If users key in content is not a dictionary with keys that are valid user IDs with values that are integers, reject.

and 2. doesn't mention users because it's covered in 3. We currently validate a weaker form of 3. in _check_power_levels, where we check if the value is parseable as an integer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing this out, I added users and removed the weaker check in _check_power_levels as it is superseded by this change.

if not isinstance(value, dict) or not all(
isinstance(v, int) for v in value.values()
):
raise AuthError(
403, f"{value} must be a dict with integers as values."
)

squahtx marked this conversation as resolved.
Show resolved Hide resolved
_check_size_limits(event)

if not hasattr(event, "room_id"):
Expand Down
35 changes: 35 additions & 0 deletions tests/test_event_auth.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,41 @@ def test_join_rules_msc3083_restricted(self) -> None:
auth_events.values(),
)

def test_room_v10_rejects_string_power_levels(self) -> None:
pl_event_content = {"users_default": "42"}
pl_event = make_event_from_dict(
H-Shay marked this conversation as resolved.
Show resolved Hide resolved
{
"room_id": TEST_ROOM_ID,
**_maybe_get_event_id_dict_for_room_version(RoomVersions.V10),
"type": "m.room.power_levels",
"sender": "@test:test.com",
"state_key": "",
"content": pl_event_content,
"signatures": {"test.com": {"ed25519:0": "some9signature"}},
},
room_version=RoomVersions.V10,
)

with self.assertRaises(AuthError):
event_auth.validate_event_for_room_version(pl_event)

pl_event2_content = {"events": {"m.room.name": "42", "m.room.power_levels": 42}}
pl_event2 = make_event_from_dict(
{
"room_id": TEST_ROOM_ID,
**_maybe_get_event_id_dict_for_room_version(RoomVersions.V10),
"type": "m.room.power_levels",
"sender": "@test:test.com",
"state_key": "",
"content": pl_event2_content,
"signatures": {"test.com": {"ed25519:0": "some9signature"}},
},
room_version=RoomVersions.V10,
)

with self.assertRaises(AuthError):
event_auth.validate_event_for_room_version(pl_event2)


# helpers for making events
TEST_DOMAIN = "example.com"
Expand Down