From f56670515bc402e13ee0bb2dd99ceb6e2ea8ba7d Mon Sep 17 00:00:00 2001 From: Kegan Dougal <7190048+kegsay@users.noreply.github.com> Date: Fri, 13 Jun 2025 17:32:24 +0100 Subject: [PATCH] bugfix: assert we always pass the create event to get_user_power_level (#18545) The create event is required if there is no PL event, in which case the creator gets PL100. ### Pull Request Checklist * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) --------- Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> --- changelog.d/18545.bugfix | 1 + synapse/api/auth/base.py | 22 +++++++++------- synapse/event_auth.py | 33 ++++++++++++++---------- synapse/push/bulk_push_rule_evaluator.py | 23 ++++++++++++++++- synapse/rest/client/room.py | 17 ++++++------ synapse/state/__init__.py | 1 + 6 files changed, 66 insertions(+), 31 deletions(-) create mode 100644 changelog.d/18545.bugfix diff --git a/changelog.d/18545.bugfix b/changelog.d/18545.bugfix new file mode 100644 index 000000000..3ca6b4d62 --- /dev/null +++ b/changelog.d/18545.bugfix @@ -0,0 +1 @@ +Fix an issue where Synapse could calculate the wrong power level for the creator of the room if there was no power levels event. diff --git a/synapse/api/auth/base.py b/synapse/api/auth/base.py index fc1c0cc90..3126bc9f2 100644 --- a/synapse/api/auth/base.py +++ b/synapse/api/auth/base.py @@ -37,7 +37,9 @@ from synapse.appservice import ApplicationService from synapse.http import get_request_user_agent from synapse.http.site import SynapseRequest from synapse.logging.opentracing import trace +from synapse.state import CREATE_KEY, POWER_KEY from synapse.types import Requester, create_requester +from synapse.types.state import StateFilter from synapse.util.cancellation import cancellable if TYPE_CHECKING: @@ -216,18 +218,20 @@ class BaseAuth: # by checking if they would (theoretically) be able to change the # m.room.canonical_alias events - power_level_event = ( - await self._storage_controllers.state.get_current_state_event( - room_id, EventTypes.PowerLevels, "" - ) + auth_events = await self._storage_controllers.state.get_current_state( + room_id, + StateFilter.from_types( + [ + POWER_KEY, + CREATE_KEY, + ] + ), ) - auth_events = {} - if power_level_event: - auth_events[(EventTypes.PowerLevels, "")] = power_level_event - send_level = event_auth.get_send_level( - EventTypes.CanonicalAlias, "", power_level_event + EventTypes.CanonicalAlias, + "", + auth_events.get(POWER_KEY), ) user_level = event_auth.get_user_power_level( requester.user.to_string(), auth_events diff --git a/synapse/event_auth.py b/synapse/event_auth.py index 5999c264d..35d02c829 100644 --- a/synapse/event_auth.py +++ b/synapse/event_auth.py @@ -64,6 +64,7 @@ from synapse.api.room_versions import ( RoomVersion, RoomVersions, ) +from synapse.state import CREATE_KEY from synapse.storage.databases.main.events_worker import EventRedactBehaviour from synapse.types import ( MutableStateMap, @@ -308,6 +309,13 @@ def check_state_dependent_auth_rules( auth_dict = {(e.type, e.state_key): e for e in auth_events} + # Later code relies on there being a create event e.g _can_federate, _is_membership_change_allowed + # so produce a more intelligible error if we don't have one. + if auth_dict.get(CREATE_KEY) is None: + raise AuthError( + 403, f"Event {event.event_id} is missing a create event in auth_events." + ) + # additional check for m.federate creating_domain = get_domain_from_id(event.room_id) originating_domain = get_domain_from_id(event.sender) @@ -1010,11 +1018,16 @@ def get_user_power_level(user_id: str, auth_events: StateMap["EventBase"]) -> in user_id: user's id to look up in power_levels auth_events: state in force at this point in the room (or rather, a subset of - it including at least the create event and power levels event. + it including at least the create event, and possibly a power levels event). Returns: the user's power level in this room. """ + create_event = auth_events.get(CREATE_KEY) + assert create_event is not None, ( + "A create event in the auth events chain is required to calculate user power level correctly," + " but was not found. This indicates a bug" + ) power_level_event = get_power_level_event(auth_events) if power_level_event: level = power_level_event.content.get("users", {}).get(user_id) @@ -1028,18 +1041,12 @@ def get_user_power_level(user_id: str, auth_events: StateMap["EventBase"]) -> in else: # if there is no power levels event, the creator gets 100 and everyone # else gets 0. - - # some things which call this don't pass the create event: hack around - # that. - key = (EventTypes.Create, "") - create_event = auth_events.get(key) - if create_event is not None: - if create_event.room_version.implicit_room_creator: - creator = create_event.sender - else: - creator = create_event.content[EventContentFields.ROOM_CREATOR] - if creator == user_id: - return 100 + if create_event.room_version.implicit_room_creator: + creator = create_event.sender + else: + creator = create_event.content[EventContentFields.ROOM_CREATOR] + if creator == user_id: + return 100 return 0 diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index 8249d5e84..1f4f5b90c 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -50,7 +50,7 @@ from synapse.event_auth import auth_types_for_event, get_user_power_level from synapse.events import EventBase, relation_from_event from synapse.events.snapshot import EventContext from synapse.logging.context import make_deferred_yieldable, run_in_background -from synapse.state import POWER_KEY +from synapse.state import CREATE_KEY, POWER_KEY from synapse.storage.databases.main.roommember import EventIdMembership from synapse.storage.invite_rule import InviteRule from synapse.storage.roommember import ProfileInfo @@ -246,6 +246,7 @@ class BulkPushRuleEvaluator: StateFilter.from_types(event_types) ) pl_event_id = prev_state_ids.get(POWER_KEY) + create_event_id = prev_state_ids.get(CREATE_KEY) # fastpath: if there's a power level event, that's all we need, and # not having a power level event is an extreme edge case @@ -268,6 +269,26 @@ class BulkPushRuleEvaluator: if auth_event: auth_events_dict[auth_event_id] = auth_event auth_events = {(e.type, e.state_key): e for e in auth_events_dict.values()} + if auth_events.get(CREATE_KEY) is None: + # if the event being checked is the create event, use its own permissions + if event.type == EventTypes.Create and event.get_state_key() == "": + auth_events[CREATE_KEY] = event + else: + auth_events[ + CREATE_KEY + ] = await self.store.get_create_event_for_room(event.room_id) + + # if we are evaluating the create event, then use itself to determine power levels. + if event.type == EventTypes.Create and event.get_state_key() == "": + auth_events[CREATE_KEY] = event + else: + # if we aren't processing the create event, create_event_id should always be set + assert create_event_id is not None + create_event = event_id_to_event.get(create_event_id) + if create_event: + auth_events[CREATE_KEY] = create_event + else: + auth_events[CREATE_KEY] = await self.store.get_event(create_event_id) sender_level = get_user_power_level(event.sender, auth_events) diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py index bb41575d4..c64ce1f9c 100644 --- a/synapse/rest/client/room.py +++ b/synapse/rest/client/room.py @@ -64,6 +64,7 @@ from synapse.logging.opentracing import set_tag from synapse.metrics.background_process_metrics import run_as_background_process from synapse.rest.client._base import client_patterns from synapse.rest.client.transactions import HttpTransactionCache +from synapse.state import CREATE_KEY, POWER_KEY from synapse.streams.config import PaginationConfig from synapse.types import JsonDict, Requester, StreamToken, ThirdPartyInstanceID, UserID from synapse.types.state import StateFilter @@ -924,16 +925,16 @@ class RoomEventServlet(RestServlet): if include_unredacted_content and not await self.auth.is_server_admin( requester ): - power_level_event = ( - await self._storage_controllers.state.get_current_state_event( - room_id, EventTypes.PowerLevels, "" - ) + auth_events = await self._storage_controllers.state.get_current_state( + room_id, + StateFilter.from_types( + [ + POWER_KEY, + CREATE_KEY, + ] + ), ) - auth_events = {} - if power_level_event: - auth_events[(EventTypes.PowerLevels, "")] = power_level_event - redact_level = event_auth.get_named_level(auth_events, "redact", 50) user_level = event_auth.get_user_power_level( requester.user.to_string(), auth_events diff --git a/synapse/state/__init__.py b/synapse/state/__init__.py index 9e48e0927..1c3e5d00a 100644 --- a/synapse/state/__init__.py +++ b/synapse/state/__init__.py @@ -83,6 +83,7 @@ EVICTION_TIMEOUT_SECONDS = 60 * 60 _NEXT_STATE_ID = 1 +CREATE_KEY = (EventTypes.Create, "") POWER_KEY = (EventTypes.PowerLevels, "")