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

Implement MSC2175 to stop adding the creator field. #15394

Merged
merged 13 commits into from
Apr 6, 2023
1 change: 1 addition & 0 deletions changelog.d/15394.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Implement [MSC2175](https://github.com/matrix-org/matrix-doc/pull/2175) to stop adding `creator` to create events.
2 changes: 2 additions & 0 deletions synapse/api/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,8 @@ class EventContentFields:
FEDERATE: Final = "m.federate"

# The creator of the room, as used in `m.room.create` events.
#
# This is deprecated in MSC2175.
ROOM_CREATOR: Final = "creator"

# Used in m.room.guest_access events.
Expand Down
16 changes: 16 additions & 0 deletions synapse/api/room_versions.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ class RoomVersion:
# MSC2209: Check 'notifications' key while verifying
# m.room.power_levels auth rules.
limit_notifications_power_levels: bool
# MSC2175: No longer include the creator in m.room.create events.
msc2175_implicit_room_creator: bool
# MSC2174/MSC2176: Apply updated redaction rules algorithm.
msc2176_redaction_rules: bool
# MSC3083: Support the 'restricted' join_rule.
Expand Down Expand Up @@ -116,6 +118,7 @@ class RoomVersions:
special_case_aliases_auth=True,
strict_canonicaljson=False,
limit_notifications_power_levels=False,
msc2175_implicit_room_creator=False,
msc2176_redaction_rules=False,
msc3083_join_rules=False,
msc3375_redaction_rules=False,
Expand All @@ -135,6 +138,7 @@ class RoomVersions:
special_case_aliases_auth=True,
strict_canonicaljson=False,
limit_notifications_power_levels=False,
msc2175_implicit_room_creator=False,
msc2176_redaction_rules=False,
msc3083_join_rules=False,
msc3375_redaction_rules=False,
Expand All @@ -154,6 +158,7 @@ class RoomVersions:
special_case_aliases_auth=True,
strict_canonicaljson=False,
limit_notifications_power_levels=False,
msc2175_implicit_room_creator=False,
msc2176_redaction_rules=False,
msc3083_join_rules=False,
msc3375_redaction_rules=False,
Expand All @@ -173,6 +178,7 @@ class RoomVersions:
special_case_aliases_auth=True,
strict_canonicaljson=False,
limit_notifications_power_levels=False,
msc2175_implicit_room_creator=False,
msc2176_redaction_rules=False,
msc3083_join_rules=False,
msc3375_redaction_rules=False,
Expand All @@ -192,6 +198,7 @@ class RoomVersions:
special_case_aliases_auth=True,
strict_canonicaljson=False,
limit_notifications_power_levels=False,
msc2175_implicit_room_creator=False,
msc2176_redaction_rules=False,
msc3083_join_rules=False,
msc3375_redaction_rules=False,
Expand All @@ -211,6 +218,7 @@ class RoomVersions:
special_case_aliases_auth=False,
strict_canonicaljson=True,
limit_notifications_power_levels=True,
msc2175_implicit_room_creator=False,
msc2176_redaction_rules=False,
msc3083_join_rules=False,
msc3375_redaction_rules=False,
Expand All @@ -230,6 +238,7 @@ class RoomVersions:
special_case_aliases_auth=False,
strict_canonicaljson=True,
limit_notifications_power_levels=True,
msc2175_implicit_room_creator=False,
msc2176_redaction_rules=True,
msc3083_join_rules=False,
msc3375_redaction_rules=False,
Expand All @@ -249,6 +258,7 @@ class RoomVersions:
special_case_aliases_auth=False,
strict_canonicaljson=True,
limit_notifications_power_levels=True,
msc2175_implicit_room_creator=False,
msc2176_redaction_rules=False,
msc3083_join_rules=False,
msc3375_redaction_rules=False,
Expand All @@ -268,6 +278,7 @@ class RoomVersions:
special_case_aliases_auth=False,
strict_canonicaljson=True,
limit_notifications_power_levels=True,
msc2175_implicit_room_creator=False,
msc2176_redaction_rules=False,
msc3083_join_rules=True,
msc3375_redaction_rules=False,
Expand All @@ -287,6 +298,7 @@ class RoomVersions:
special_case_aliases_auth=False,
strict_canonicaljson=True,
limit_notifications_power_levels=True,
msc2175_implicit_room_creator=False,
msc2176_redaction_rules=False,
msc3083_join_rules=True,
msc3375_redaction_rules=True,
Expand All @@ -306,6 +318,7 @@ class RoomVersions:
special_case_aliases_auth=False,
strict_canonicaljson=True,
limit_notifications_power_levels=True,
msc2175_implicit_room_creator=False,
msc2176_redaction_rules=False,
msc3083_join_rules=True,
msc3375_redaction_rules=True,
Expand All @@ -325,6 +338,7 @@ class RoomVersions:
special_case_aliases_auth=False,
strict_canonicaljson=True,
limit_notifications_power_levels=True,
msc2175_implicit_room_creator=False,
msc2176_redaction_rules=False,
msc3083_join_rules=True,
msc3375_redaction_rules=True,
Expand All @@ -344,6 +358,7 @@ class RoomVersions:
special_case_aliases_auth=False,
strict_canonicaljson=True,
limit_notifications_power_levels=True,
msc2175_implicit_room_creator=False,
msc2176_redaction_rules=False,
msc3083_join_rules=False,
msc3375_redaction_rules=False,
Expand All @@ -364,6 +379,7 @@ class RoomVersions:
special_case_aliases_auth=False,
strict_canonicaljson=True,
limit_notifications_power_levels=True,
msc2175_implicit_room_creator=False,
msc2176_redaction_rules=False,
msc3083_join_rules=True,
msc3375_redaction_rules=True,
Expand Down
19 changes: 15 additions & 4 deletions synapse/event_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -455,8 +455,11 @@ def _check_create(event: "EventBase") -> None:
"room appears to have unsupported version %s" % (room_version_prop,),
)

# 1.4 If content has no creator field, reject.
if EventContentFields.ROOM_CREATOR not in event.content:
# 1.4 If content has no creator field, reject if the room version requires it.
if (
not event.room_version.msc2175_implicit_room_creator
and EventContentFields.ROOM_CREATOR not in event.content
clokep marked this conversation as resolved.
Show resolved Hide resolved
):
raise AuthError(403, "Create event lacks a 'creator' property")


Expand Down Expand Up @@ -491,7 +494,11 @@ def _is_membership_change_allowed(
key = (EventTypes.Create, "")
create = auth_events.get(key)
if create and event.prev_event_ids()[0] == create.event_id:
if create.content["creator"] == event.state_key:
if room_version.msc2175_implicit_room_creator:
creator = create.sender
else:
creator = create.content[EventContentFields.ROOM_CREATOR]
if creator == event.state_key:
return

target_user_id = event.state_key
Expand Down Expand Up @@ -1004,7 +1011,11 @@ def get_user_power_level(user_id: str, auth_events: StateMap["EventBase"]) -> in
# that.
key = (EventTypes.Create, "")
create_event = auth_events.get(key)
if create_event is not None and create_event.content["creator"] == user_id:
if create_event.room_version.msc2175_implicit_room_creator:
creator = create_event.sender
else:
creator = create_event.content[EventContentFields.ROOM_CREATOR]
if create_event is not None and creator == user_id:
return 100
else:
return 0
Expand Down
5 changes: 4 additions & 1 deletion synapse/handlers/federation_event.py
Original file line number Diff line number Diff line change
Expand Up @@ -1515,7 +1515,10 @@ async def _handle_marker_event(self, origin: str, marker_event: EventBase) -> No
# support it or the event is not from the room creator.
room_version = await self._store.get_room_version(marker_event.room_id)
create_event = await self._store.get_create_event_for_room(marker_event.room_id)
room_creator = create_event.content.get(EventContentFields.ROOM_CREATOR)
if room_version.msc2175_implicit_room_creator:
room_creator = create_event.sender
else:
room_creator = create_event.content.get(EventContentFields.ROOM_CREATOR)
if not room_version.msc2716_historical and (
not self._config.experimental.msc2716_enabled
or marker_event.sender != room_creator
Expand Down
7 changes: 6 additions & 1 deletion synapse/handlers/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -1909,7 +1909,12 @@ async def persist_and_notify_client_events(
room_version_obj = KNOWN_ROOM_VERSIONS[room_version]

create_event = await self.store.get_create_event_for_room(event.room_id)
room_creator = create_event.content.get(EventContentFields.ROOM_CREATOR)
if room_version_obj.msc2175_implicit_room_creator:
room_creator = create_event.sender
else:
room_creator = create_event.content.get(
EventContentFields.ROOM_CREATOR
)

# Only check an insertion event if the room version
# supports it or the event is from the room creator.
Expand Down
22 changes: 12 additions & 10 deletions synapse/handlers/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,7 @@ async def clone_existing_room(
await self._send_events_for_new_room(
requester,
new_room_id,
new_room_version,
# we expect to override all the presets with initial_state, so this is
# somewhat arbitrary.
room_config={"preset": RoomCreationPreset.PRIVATE_CHAT},
Expand Down Expand Up @@ -922,6 +923,7 @@ async def create_room(
) = await self._send_events_for_new_room(
requester,
room_id,
room_version,
room_config=config,
invite_list=invite_list,
initial_state=initial_state,
Expand Down Expand Up @@ -998,6 +1000,7 @@ async def _send_events_for_new_room(
self,
creator: Requester,
room_id: str,
room_version: RoomVersion,
room_config: JsonDict,
invite_list: List[str],
initial_state: MutableStateMap,
Expand All @@ -1020,6 +1023,8 @@ async def _send_events_for_new_room(
the user requesting the room creation
room_id:
room id for the room being created
room_version:
The room version of the new room.
room_config:
A dict of configuration options. This will be the body of
a /createRoom request; see
Expand Down Expand Up @@ -1053,14 +1058,6 @@ async def _send_events_for_new_room(
# (as this info can't be pulled from the db)
state_map: MutableStateMap[str] = {}

def create_event_dict(etype: str, content: JsonDict, **kwargs: Any) -> JsonDict:
e = {"type": etype, "content": content}

e.update(event_keys)
e.update(kwargs)

return e

async def create_event(
etype: str,
content: JsonDict,
Expand All @@ -1083,7 +1080,10 @@ async def create_event(
nonlocal depth
nonlocal prev_event

event_dict = create_event_dict(etype, content, **kwargs)
# Create the event dictionary.
event_dict = {"type": etype, "content": content}
event_dict.update(event_keys)
event_dict.update(kwargs)

(
new_event,
Expand Down Expand Up @@ -1120,7 +1120,9 @@ async def create_event(
400, f"'{preset_config}' is not a valid preset", errcode=Codes.BAD_JSON
)

creation_content.update({"creator": creator_id})
# MSC2175 removes the creator field from the create event.
if not room_version.msc2175_implicit_room_creator:
creation_content.update({"creator": creator_id})
clokep marked this conversation as resolved.
Show resolved Hide resolved
creation_event, unpersisted_creation_context = await create_event(
EventTypes.Create, creation_content, False
)
Expand Down
17 changes: 12 additions & 5 deletions synapse/storage/databases/main/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -1998,6 +1998,9 @@ def _background_populate_rooms_creator_column_txn(
for room_id, event_json in room_id_to_create_event_results:
event_dict = db_to_json(event_json)

# The creator property might not exist in newer room versions, but
# for those versions the creator column should be properly populate
# during room creation.
creator = event_dict.get("content").get(EventContentFields.ROOM_CREATOR)
Comment on lines +2001 to 2004
Copy link
Contributor

Choose a reason for hiding this comment

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

Aside: Is the rooms.creator column useful? I guess it avoids looking up the create event and from the state and event_json tables (though I'd bet we're already looking in those tables anyway).

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 fairly certain that we do use it, but it seems like it has been around forever.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, maybe not -- it was added in #10697.

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems to be mentioned on #10737 too.


self.db_pool.simple_update_txn(
Expand Down Expand Up @@ -2132,12 +2135,16 @@ async def upsert_room_on_join(
# invalid, and it would fail auth checks anyway.
raise StoreError(400, "No create event in state")

room_creator = create_event.content.get(EventContentFields.ROOM_CREATOR)
# Before MSC2175, the room creator was a separate field.
if not room_version.msc2175_implicit_room_creator:
room_creator = create_event.content.get(EventContentFields.ROOM_CREATOR)

if not isinstance(room_creator, str):
# If the create event does not have a creator then the room is
# invalid, and it would fail auth checks anyway.
raise StoreError(400, "No creator defined on the create event")
if not isinstance(room_creator, str):
# If the create event does not have a creator then the room is
# invalid, and it would fail auth checks anyway.
raise StoreError(400, "No creator defined on the create event")
else:
room_creator = create_event.sender

await self.db_pool.simple_upsert(
desc="upsert_room_on_join",
Expand Down