From ee42a5513e020424daa736962fee7fb69bd2373a Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 28 Jan 2020 11:02:55 +0000 Subject: [PATCH 1/4] Factor out a `copy_power_levels_contents` method I'm going to need another copy (hah!) of this. --- synapse/events/utils.py | 37 ++++++++++++++++++++++++++++++- synapse/handlers/room.py | 23 ++++++++++--------- tests/events/test_utils.py | 45 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 90 insertions(+), 15 deletions(-) diff --git a/synapse/events/utils.py b/synapse/events/utils.py index 07d1c5bcf..be57c6d9b 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -12,8 +12,9 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. - +import collections import re +from typing import Mapping, Union from six import string_types @@ -422,3 +423,37 @@ class EventClientSerializer(object): return yieldable_gather_results( self.serialize_event, events, time_now=time_now, **kwargs ) + + +def copy_power_levels_contents( + old_power_levels: Mapping[str, Union[int, Mapping[str, int]]] +): + """Copy the content of a power_levels event, unfreezing frozendicts along the way + + Raises: + TypeError if the input does not look like a valid power levels event content + """ + if not isinstance(old_power_levels, collections.Mapping): + raise TypeError("Not a valid power-levels content: %r" % (old_power_levels,)) + + power_levels = {} + for k, v in old_power_levels.items(): + + if isinstance(v, int): + power_levels[k] = v + continue + + if isinstance(v, collections.Mapping): + power_levels[k] = h = {} + for k1, v1 in v.items(): + # we should only have one level of nesting + if not isinstance(v1, int): + raise TypeError( + "Invalid power_levels value for %s.%s: %r" % (k, k1, v) + ) + h[k1] = v1 + continue + + raise TypeError("Invalid power_levels value for %s: %r" % (k, v)) + + return power_levels diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index a9490782b..532ee22fa 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -30,6 +30,7 @@ from twisted.internet import defer from synapse.api.constants import EventTypes, JoinRules, RoomCreationPreset from synapse.api.errors import AuthError, Codes, NotFoundError, StoreError, SynapseError from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersion +from synapse.events.utils import copy_power_levels_contents from synapse.http.endpoint import parse_and_validate_server_name from synapse.storage.state import StateFilter from synapse.types import ( @@ -367,6 +368,15 @@ class RoomCreationHandler(BaseHandler): if old_event: initial_state[k] = old_event.content + # deep-copy the power-levels event before we start modifying it + # note that if frozen_dicts are enabled, `power_levels` will be a frozen + # dict so we can't just copy.deepcopy it. + initial_state[ + (EventTypes.PowerLevels, "") + ] = power_levels = copy_power_levels_contents( + initial_state[(EventTypes.PowerLevels, "")] + ) + # Resolve the minimum power level required to send any state event # We will give the upgrading user this power level temporarily (if necessary) such that # they are able to copy all of the state events over, then revert them back to their @@ -375,8 +385,6 @@ class RoomCreationHandler(BaseHandler): # Copy over user power levels now as this will not be possible with >100PL users once # the room has been created - power_levels = initial_state[(EventTypes.PowerLevels, "")] - # Calculate the minimum power level needed to clone the room event_power_levels = power_levels.get("events", {}) state_default = power_levels.get("state_default", 0) @@ -386,16 +394,7 @@ class RoomCreationHandler(BaseHandler): # Raise the requester's power level in the new room if necessary current_power_level = power_levels["users"][user_id] if current_power_level < needed_power_level: - # make sure we copy the event content rather than overwriting it. - # note that if frozen_dicts are enabled, `power_levels` will be a frozen - # dict so we can't just copy.deepcopy it. - - new_power_levels = {k: v for k, v in power_levels.items() if k != "users"} - new_power_levels["users"] = { - k: v for k, v in power_levels.get("users", {}).items() if k != user_id - } - new_power_levels["users"][user_id] = needed_power_level - initial_state[(EventTypes.PowerLevels, "")] = new_power_levels + power_levels["users"][user_id] = needed_power_level yield self._send_events_for_new_room( requester, diff --git a/tests/events/test_utils.py b/tests/events/test_utils.py index 9e3d4d0f4..2b13980df 100644 --- a/tests/events/test_utils.py +++ b/tests/events/test_utils.py @@ -15,9 +15,14 @@ from synapse.events import FrozenEvent -from synapse.events.utils import prune_event, serialize_event +from synapse.events.utils import ( + copy_power_levels_contents, + prune_event, + serialize_event, +) +from synapse.util.frozenutils import freeze -from .. import unittest +from tests import unittest def MockEvent(**kwargs): @@ -241,3 +246,39 @@ class SerializeEventTestCase(unittest.TestCase): self.serialize( MockEvent(room_id="!foo:bar", content={"foo": "bar"}), ["room_id", 4] ) + + +class CopyPowerLevelsContentTestCase(unittest.TestCase): + def setUp(self) -> None: + self.test_content = { + "ban": 50, + "events": {"m.room.name": 100, "m.room.power_levels": 100}, + "events_default": 0, + "invite": 50, + "kick": 50, + "notifications": {"room": 20}, + "redact": 50, + "state_default": 50, + "users": {"@example:localhost": 100}, + "users_default": 0, + } + + def _test(self, input): + a = copy_power_levels_contents(input) + + self.assertEqual(a["ban"], 50) + self.assertEqual(a["events"]["m.room.name"], 100) + + # make sure that changing the copy changes the copy and not the orig + a["ban"] = 10 + a["events"]["m.room.power_levels"] = 20 + + self.assertEqual(input["ban"], 50) + self.assertEqual(input["events"]["m.room.power_levels"], 100) + + def test_unfrozen(self): + self._test(self.test_content) + + def test_frozen(self): + input = freeze(self.test_content) + self._test(input) From b36095ae5cd88d95802ecf01f8b0f541593ea773 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 28 Jan 2020 11:08:38 +0000 Subject: [PATCH 2/4] Set the PL for aliases events to 0. --- synapse/events/utils.py | 2 +- synapse/handlers/room.py | 17 +++++++++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/synapse/events/utils.py b/synapse/events/utils.py index be57c6d9b..f70f5032f 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -449,7 +449,7 @@ def copy_power_levels_contents( # we should only have one level of nesting if not isinstance(v1, int): raise TypeError( - "Invalid power_levels value for %s.%s: %r" % (k, k1, v) + "Invalid power_levels value for %s.%s: %r" % (k, k1, v1) ) h[k1] = v1 continue diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 532ee22fa..a95b45d79 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -287,7 +287,16 @@ class RoomCreationHandler(BaseHandler): except AuthError as e: logger.warning("Unable to update PLs in old room: %s", e) - logger.info("Setting correct PLs in new room to %s", old_room_pl_state.content) + new_pl_content = copy_power_levels_contents(old_room_pl_state.content) + + # pre-msc2260 rooms may not have the right setting for aliases. If no other + # value is set, set it now. + events_default = new_pl_content.get("events_default", 0) + new_pl_content.setdefault("events", {}).setdefault( + EventTypes.Aliases, events_default + ) + + logger.info("Setting correct PLs in new room to %s", new_pl_content) yield self.event_creation_handler.create_and_send_nonmember_event( requester, { @@ -295,7 +304,7 @@ class RoomCreationHandler(BaseHandler): "state_key": "", "room_id": new_room_id, "sender": requester.user.to_string(), - "content": old_room_pl_state.content, + "content": new_pl_content, }, ratelimit=False, ) @@ -812,6 +821,10 @@ class RoomCreationHandler(BaseHandler): EventTypes.RoomHistoryVisibility: 100, EventTypes.CanonicalAlias: 50, EventTypes.RoomAvatar: 50, + # MSC2260: Allow everybody to send alias events by default + # This will be reudundant on pre-MSC2260 rooms, since the + # aliases event is special-cased. + EventTypes.Aliases: 0, }, "events_default": 0, "state_default": 50, From dcd85b976dc93851d7f5246fc0684d5c5f7d7b5b Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 28 Jan 2020 17:46:34 +0000 Subject: [PATCH 3/4] Make /directory/room/ handle restrictive power levels Fixes a bug where the alias would be added, but `PUT /directory/room/` would return a 403. --- synapse/handlers/directory.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/directory.py b/synapse/handlers/directory.py index a07d2f1a1..8c5980cb0 100644 --- a/synapse/handlers/directory.py +++ b/synapse/handlers/directory.py @@ -151,7 +151,12 @@ class DirectoryHandler(BaseHandler): yield self._create_association(room_alias, room_id, servers, creator=user_id) if send_event: - yield self.send_room_alias_update_event(requester, room_id) + try: + yield self.send_room_alias_update_event(requester, room_id) + except AuthError as e: + # sending the aliases event may fail due to the user not having + # permission in the room; this is permitted. + logger.info("Skipping updating aliases event due to auth error %s", e) @defer.inlineCallbacks def delete_association(self, requester, room_alias, send_event=True): From 750d4d7599d1985bc262853494b21e9fee34c637 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 28 Jan 2020 11:09:14 +0000 Subject: [PATCH 4/4] changelog --- changelog.d/6790.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/6790.feature diff --git a/changelog.d/6790.feature b/changelog.d/6790.feature new file mode 100644 index 000000000..df9e4b77a --- /dev/null +++ b/changelog.d/6790.feature @@ -0,0 +1 @@ +Implement updated authorization rules for aliases events, from [MSC2260](https://github.com/matrix-org/matrix-doc/pull/2260).