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

Support MSC3667 in a new room version #11885

Closed
wants to merge 8 commits into from
Closed

Support MSC3667 in a new room version #11885

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0f6ab42
Support MSC3667 in a new room version
turt2live Feb 2, 2022
0e55160
Changelog
turt2live Feb 2, 2022
f1d5890
lint
turt2live Feb 2, 2022
f5e2cde
more lint
turt2live Feb 2, 2022
72b19cb
Merge branch 'develop' into travis/msc3667
turt2live May 20, 2022
99ef23d
Remove defunct opt2 room version
turt2live May 20, 2022
cf72994
Populate missing room version fields (post-merge)
turt2live May 20, 2022
3f310c9
Address the easy parts of code review
turt2live May 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/11885.feature
View file
Open in desktop
@@ -0,0 +1 @@
Support [MSC3667](https://github.com/matrix-org/matrix-doc/pull/3667) in a new room version.
34 changes: 34 additions & 0 deletions synapse/api/room_versions.py
View file
Open in desktop
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: Treat string format power levels as invalid, thus denied.
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,26 @@ class RoomVersions:
msc2716_historical=False,
msc2716_redactions=False,
msc3787_knock_restricted_join_rule=True,
msc3667_int_only_power_levels=False,
)
MSC3667 = RoomVersion(
# v7 + MSC3667
"org.matrix.msc3667",
RoomDisposition.UNSTABLE,
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=False,
msc3375_redaction_rules=False,
msc2403_knocking=True,
msc2716_historical=False,
msc2716_redactions=False,
msc3787_knock_restricted_join_rule=False,
msc3667_int_only_power_levels=True,
)


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

Expand Down
16 changes: 16 additions & 0 deletions synapse/event_auth.py
View file
Open in desktop
Expand Up @@ -143,6 +143,22 @@ def check_auth_rules_for_event(
Raises:
AuthError if the checks fail
"""
# Before we get too far into event auth, validate that the event is even
# valid enough to be used
if event.type == EventTypes.PowerLevels:
Copy link
Member Author

Choose a reason for hiding this comment

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

this might be in the wrong place, but it's highly convenient to put it here

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like for similar code we usually do it in EventValidator.validate_new + event_from_pdu_json, but I'm unsure if that makes sense or not. (I was looking at where we call validate_canonicaljson)

Copy link
Member

Choose a reason for hiding this comment

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

We should really add a function to the Validator for inbound federation events, I guess

# If applicable, validate that the known power levels are integers
if room_version_obj.msc3667_int_only_power_levels:
for k, v in event.content.items():
if k in ["events", "notifications", "users"]:
Copy link
Member

Choose a reason for hiding this comment

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

I am somewhat tempted to just recursively check that all values are either dicts or ints? That way we don't need to remember to update this set for new fields. That would break experimental non-int fields, so maybe we could only do that on the redacted form of the power level?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm a little bit worried that'd treat ban: {} as valid :(

if type(v) is not dict:
Copy link
Member

Choose a reason for hiding this comment

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

One usually does isinstance(v, dict), but it doesn't really matter in this case (isinstance correctly handles subclasses and the like)

raise AuthError(403, "Not a valid object: %s" % (k,))
for v2 in v.values():
Copy link
Member Author

Choose a reason for hiding this comment

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

this check won't work because of how the auth rules are written: we have to check the keys explicitly.

if type(v2) is not int:
raise AuthError(403, "Not a valid power level: %s" % (v2,))
else:
if type(v) is not int:
raise AuthError(403, "Not a valid power level: %s" % (v,))

# We need to ensure that the auth events are actually for the same room, to
# stop people from using powers they've been granted in other rooms for
# example.
Expand Down