From 2959184a42398277ff916206235b844a8f7be5d7 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 10 Jun 2022 10:44:19 +0100 Subject: [PATCH 1/5] EventAuthTestCase: build events for the right room version In practice, when we run the auth rules, all of the events have the right room version. Let's stop building Room V1 events for these tests and use the right version. --- tests/test_event_auth.py | 324 +++++++++++++++++++++++++-------------- 1 file changed, 205 insertions(+), 119 deletions(-) diff --git a/tests/test_event_auth.py b/tests/test_event_auth.py index e2c506e5a4..1e11fb5dac 100644 --- a/tests/test_event_auth.py +++ b/tests/test_event_auth.py @@ -15,10 +15,12 @@ import unittest from typing import Optional +from parameterized import parameterized + from synapse import event_auth from synapse.api.constants import EventContentFields from synapse.api.errors import AuthError -from synapse.api.room_versions import RoomVersions +from synapse.api.room_versions import EventFormatVersions, RoomVersion, RoomVersions from synapse.events import EventBase, make_event_from_dict from synapse.types import JsonDict, get_domain_from_id @@ -30,19 +32,23 @@ class EventAuthTestCase(unittest.TestCase): """ creator = "@creator:example.com" auth_events = [ - _create_event(creator), - _join_event(creator), + _create_event(RoomVersions.V9, creator), + _join_event(RoomVersions.V9, creator), ] # creator should be able to send state event_auth.check_auth_rules_for_event( RoomVersions.V9, - _random_state_event(creator), + _random_state_event(RoomVersions.V9, creator), auth_events, ) # ... but a rejected join_rules event should cause it to be rejected - rejected_join_rules = _join_rules_event(creator, "public") + rejected_join_rules = _join_rules_event( + RoomVersions.V9, + creator, + "public", + ) rejected_join_rules.rejected_reason = "stinky" auth_events.append(rejected_join_rules) @@ -50,18 +56,18 @@ class EventAuthTestCase(unittest.TestCase): AuthError, event_auth.check_auth_rules_for_event, RoomVersions.V9, - _random_state_event(creator), + _random_state_event(RoomVersions.V9, creator), auth_events, ) # ... even if there is *also* a good join rules - auth_events.append(_join_rules_event(creator, "public")) + auth_events.append(_join_rules_event(RoomVersions.V9, creator, "public")) self.assertRaises( AuthError, event_auth.check_auth_rules_for_event, RoomVersions.V9, - _random_state_event(creator), + _random_state_event(RoomVersions.V9, creator), auth_events, ) @@ -73,15 +79,15 @@ class EventAuthTestCase(unittest.TestCase): creator = "@creator:example.com" joiner = "@joiner:example.com" auth_events = [ - _create_event(creator), - _join_event(creator), - _join_event(joiner), + _create_event(RoomVersions.V1, creator), + _join_event(RoomVersions.V1, creator), + _join_event(RoomVersions.V1, joiner), ] # creator should be able to send state event_auth.check_auth_rules_for_event( RoomVersions.V1, - _random_state_event(creator), + _random_state_event(RoomVersions.V1, creator), auth_events, ) @@ -90,7 +96,7 @@ class EventAuthTestCase(unittest.TestCase): AuthError, event_auth.check_auth_rules_for_event, RoomVersions.V1, - _random_state_event(joiner), + _random_state_event(RoomVersions.V1, joiner), auth_events, ) @@ -104,13 +110,15 @@ class EventAuthTestCase(unittest.TestCase): king = "@joiner2:example.com" auth_events = [ - _create_event(creator), - _join_event(creator), + _create_event(RoomVersions.V1, creator), + _join_event(RoomVersions.V1, creator), _power_levels_event( - creator, {"state_default": "30", "users": {pleb: "29", king: "30"}} + RoomVersions.V1, + creator, + {"state_default": "30", "users": {pleb: "29", king: "30"}}, ), - _join_event(pleb), - _join_event(king), + _join_event(RoomVersions.V1, pleb), + _join_event(RoomVersions.V1, king), ] # pleb should not be able to send state @@ -118,14 +126,14 @@ class EventAuthTestCase(unittest.TestCase): AuthError, event_auth.check_auth_rules_for_event, RoomVersions.V1, - _random_state_event(pleb), + _random_state_event(RoomVersions.V1, pleb), auth_events, ), # king should be able to send state event_auth.check_auth_rules_for_event( RoomVersions.V1, - _random_state_event(king), + _random_state_event(RoomVersions.V1, king), auth_events, ) @@ -134,14 +142,14 @@ class EventAuthTestCase(unittest.TestCase): creator = "@creator:example.com" other = "@other:example.com" auth_events = [ - _create_event(creator), - _join_event(creator), + _create_event(RoomVersions.V1, creator), + _join_event(RoomVersions.V1, creator), ] # creator should be able to send aliases event_auth.check_auth_rules_for_event( RoomVersions.V1, - _alias_event(creator), + _alias_event(RoomVersions.V1, creator), auth_events, ) @@ -149,7 +157,7 @@ class EventAuthTestCase(unittest.TestCase): with self.assertRaises(AuthError): event_auth.check_auth_rules_for_event( RoomVersions.V1, - _alias_event(creator, state_key=""), + _alias_event(RoomVersions.V1, creator, state_key=""), auth_events, ) @@ -157,14 +165,14 @@ class EventAuthTestCase(unittest.TestCase): with self.assertRaises(AuthError): event_auth.check_auth_rules_for_event( RoomVersions.V1, - _alias_event(creator, state_key="test.com"), + _alias_event(RoomVersions.V1, creator, state_key="test.com"), auth_events, ) # Note that the member does *not* need to be in the room. event_auth.check_auth_rules_for_event( RoomVersions.V1, - _alias_event(other), + _alias_event(RoomVersions.V1, other), auth_events, ) @@ -173,26 +181,26 @@ class EventAuthTestCase(unittest.TestCase): creator = "@creator:example.com" other = "@other:example.com" auth_events = [ - _create_event(creator), - _join_event(creator), + _create_event(RoomVersions.V6, creator), + _join_event(RoomVersions.V6, creator), ] # creator should be able to send aliases event_auth.check_auth_rules_for_event( RoomVersions.V6, - _alias_event(creator), + _alias_event(RoomVersions.V6, creator), auth_events, ) # No particular checks are done on the state key. event_auth.check_auth_rules_for_event( RoomVersions.V6, - _alias_event(creator, state_key=""), + _alias_event(RoomVersions.V6, creator, state_key=""), auth_events, ) event_auth.check_auth_rules_for_event( RoomVersions.V6, - _alias_event(creator, state_key="test.com"), + _alias_event(RoomVersions.V6, creator, state_key="test.com"), auth_events, ) @@ -200,11 +208,12 @@ class EventAuthTestCase(unittest.TestCase): with self.assertRaises(AuthError): event_auth.check_auth_rules_for_event( RoomVersions.V6, - _alias_event(other), + _alias_event(RoomVersions.V6, other), auth_events, ) - def test_msc2209(self): + @parameterized.expand([(RoomVersions.V1, True), (RoomVersions.V6, False)]) + def test_notifications(self, room_version: RoomVersion, allow_modification: bool): """ Notifications power levels get checked due to MSC2209. """ @@ -212,28 +221,28 @@ class EventAuthTestCase(unittest.TestCase): pleb = "@joiner:example.com" auth_events = [ - _create_event(creator), - _join_event(creator), + _create_event(room_version, creator), + _join_event(room_version, creator), _power_levels_event( - creator, {"state_default": "30", "users": {pleb: "30"}} + room_version, creator, {"state_default": "30", "users": {pleb: "30"}} ), - _join_event(pleb), + _join_event(room_version, pleb), ] - # pleb should be able to modify the notifications power level. - event_auth.check_auth_rules_for_event( - RoomVersions.V1, - _power_levels_event(pleb, {"notifications": {"room": 100}}), - auth_events, + pl_event = _power_levels_event( + room_version, pleb, {"notifications": {"room": 100}} ) - # But an MSC2209 room rejects this change. - with self.assertRaises(AuthError): - event_auth.check_auth_rules_for_event( - RoomVersions.V6, - _power_levels_event(pleb, {"notifications": {"room": 100}}), - auth_events, - ) + # on room V1, pleb should be able to modify the notifications power level. + if allow_modification: + event_auth.check_auth_rules_for_event(room_version, pl_event, auth_events) + + else: + # But an MSC2209 room rejects this change. + with self.assertRaises(AuthError): + event_auth.check_auth_rules_for_event( + room_version, pl_event, auth_events + ) def test_join_rules_public(self): """ @@ -243,15 +252,17 @@ class EventAuthTestCase(unittest.TestCase): pleb = "@joiner:example.com" auth_events = { - ("m.room.create", ""): _create_event(creator), - ("m.room.member", creator): _join_event(creator), - ("m.room.join_rules", ""): _join_rules_event(creator, "public"), + ("m.room.create", ""): _create_event(RoomVersions.V6, creator), + ("m.room.member", creator): _join_event(RoomVersions.V6, creator), + ("m.room.join_rules", ""): _join_rules_event( + RoomVersions.V6, creator, "public" + ), } # Check join. event_auth.check_auth_rules_for_event( RoomVersions.V6, - _join_event(pleb), + _join_event(RoomVersions.V6, pleb), auth_events.values(), ) @@ -259,42 +270,48 @@ class EventAuthTestCase(unittest.TestCase): with self.assertRaises(AuthError): event_auth.check_auth_rules_for_event( RoomVersions.V6, - _member_event(pleb, "join", sender=creator), + _member_event(RoomVersions.V6, pleb, "join", sender=creator), auth_events.values(), ) # Banned should be rejected. - auth_events[("m.room.member", pleb)] = _member_event(pleb, "ban") + auth_events[("m.room.member", pleb)] = _member_event( + RoomVersions.V6, pleb, "ban" + ) with self.assertRaises(AuthError): event_auth.check_auth_rules_for_event( RoomVersions.V6, - _join_event(pleb), + _join_event(RoomVersions.V6, pleb), auth_events.values(), ) # A user who left can re-join. - auth_events[("m.room.member", pleb)] = _member_event(pleb, "leave") + auth_events[("m.room.member", pleb)] = _member_event( + RoomVersions.V6, pleb, "leave" + ) event_auth.check_auth_rules_for_event( RoomVersions.V6, - _join_event(pleb), + _join_event(RoomVersions.V6, pleb), auth_events.values(), ) # A user can send a join if they're in the room. - auth_events[("m.room.member", pleb)] = _member_event(pleb, "join") + auth_events[("m.room.member", pleb)] = _member_event( + RoomVersions.V6, pleb, "join" + ) event_auth.check_auth_rules_for_event( RoomVersions.V6, - _join_event(pleb), + _join_event(RoomVersions.V6, pleb), auth_events.values(), ) # A user can accept an invite. auth_events[("m.room.member", pleb)] = _member_event( - pleb, "invite", sender=creator + RoomVersions.V6, pleb, "invite", sender=creator ) event_auth.check_auth_rules_for_event( RoomVersions.V6, - _join_event(pleb), + _join_event(RoomVersions.V6, pleb), auth_events.values(), ) @@ -306,16 +323,18 @@ class EventAuthTestCase(unittest.TestCase): pleb = "@joiner:example.com" auth_events = { - ("m.room.create", ""): _create_event(creator), - ("m.room.member", creator): _join_event(creator), - ("m.room.join_rules", ""): _join_rules_event(creator, "invite"), + ("m.room.create", ""): _create_event(RoomVersions.V6, creator), + ("m.room.member", creator): _join_event(RoomVersions.V6, creator), + ("m.room.join_rules", ""): _join_rules_event( + RoomVersions.V6, creator, "invite" + ), } # A join without an invite is rejected. with self.assertRaises(AuthError): event_auth.check_auth_rules_for_event( RoomVersions.V6, - _join_event(pleb), + _join_event(RoomVersions.V6, pleb), auth_events.values(), ) @@ -323,47 +342,76 @@ class EventAuthTestCase(unittest.TestCase): with self.assertRaises(AuthError): event_auth.check_auth_rules_for_event( RoomVersions.V6, - _member_event(pleb, "join", sender=creator), + _member_event(RoomVersions.V6, pleb, "join", sender=creator), auth_events.values(), ) # Banned should be rejected. - auth_events[("m.room.member", pleb)] = _member_event(pleb, "ban") + auth_events[("m.room.member", pleb)] = _member_event( + RoomVersions.V6, pleb, "ban" + ) with self.assertRaises(AuthError): event_auth.check_auth_rules_for_event( RoomVersions.V6, - _join_event(pleb), + _join_event(RoomVersions.V6, pleb), auth_events.values(), ) # A user who left cannot re-join. - auth_events[("m.room.member", pleb)] = _member_event(pleb, "leave") + auth_events[("m.room.member", pleb)] = _member_event( + RoomVersions.V6, pleb, "leave" + ) with self.assertRaises(AuthError): event_auth.check_auth_rules_for_event( RoomVersions.V6, - _join_event(pleb), + _join_event(RoomVersions.V6, pleb), auth_events.values(), ) # A user can send a join if they're in the room. - auth_events[("m.room.member", pleb)] = _member_event(pleb, "join") + auth_events[("m.room.member", pleb)] = _member_event( + RoomVersions.V6, pleb, "join" + ) event_auth.check_auth_rules_for_event( RoomVersions.V6, - _join_event(pleb), + _join_event(RoomVersions.V6, pleb), auth_events.values(), ) # A user can accept an invite. auth_events[("m.room.member", pleb)] = _member_event( - pleb, "invite", sender=creator + RoomVersions.V6, pleb, "invite", sender=creator ) event_auth.check_auth_rules_for_event( RoomVersions.V6, - _join_event(pleb), + _join_event(RoomVersions.V6, pleb), auth_events.values(), ) - def test_join_rules_msc3083_restricted(self): + def test_join_rules_restricted_old_room(self) -> None: + """Old room versions should reject joins to restricted rooms""" + creator = "@creator:example.com" + pleb = "@joiner:example.com" + + auth_events = { + ("m.room.create", ""): _create_event(RoomVersions.V6, creator), + ("m.room.member", creator): _join_event(RoomVersions.V6, creator), + ("m.room.power_levels", ""): _power_levels_event( + RoomVersions.V6, creator, {"invite": 0} + ), + ("m.room.join_rules", ""): _join_rules_event( + RoomVersions.V6, creator, "restricted" + ), + } + + with self.assertRaises(AuthError): + event_auth.check_auth_rules_for_event( + RoomVersions.V6, + _join_event(RoomVersions.V6, pleb), + auth_events.values(), + ) + + def test_join_rules_msc3083_restricted(self) -> None: """ Test joining a restricted room from MSC3083. @@ -377,22 +425,19 @@ class EventAuthTestCase(unittest.TestCase): pleb = "@joiner:example.com" auth_events = { - ("m.room.create", ""): _create_event(creator), - ("m.room.member", creator): _join_event(creator), - ("m.room.power_levels", ""): _power_levels_event(creator, {"invite": 0}), - ("m.room.join_rules", ""): _join_rules_event(creator, "restricted"), + ("m.room.create", ""): _create_event(RoomVersions.V8, creator), + ("m.room.member", creator): _join_event(RoomVersions.V8, creator), + ("m.room.power_levels", ""): _power_levels_event( + RoomVersions.V8, creator, {"invite": 0} + ), + ("m.room.join_rules", ""): _join_rules_event( + RoomVersions.V8, creator, "restricted" + ), } - # Older room versions don't understand this join rule - with self.assertRaises(AuthError): - event_auth.check_auth_rules_for_event( - RoomVersions.V6, - _join_event(pleb), - auth_events.values(), - ) - # A properly formatted join event should work. authorised_join_event = _join_event( + RoomVersions.V8, pleb, additional_content={ EventContentFields.AUTHORISING_USER: "@creator:example.com" @@ -408,14 +453,17 @@ class EventAuthTestCase(unittest.TestCase): # are done properly). pl_auth_events = auth_events.copy() pl_auth_events[("m.room.power_levels", "")] = _power_levels_event( - creator, {"invite": 100, "users": {"@inviter:foo.test": 150}} + RoomVersions.V8, + creator, + {"invite": 100, "users": {"@inviter:foo.test": 150}}, ) pl_auth_events[("m.room.member", "@inviter:foo.test")] = _join_event( - "@inviter:foo.test" + RoomVersions.V8, "@inviter:foo.test" ) event_auth.check_auth_rules_for_event( RoomVersions.V8, _join_event( + RoomVersions.V8, pleb, additional_content={ EventContentFields.AUTHORISING_USER: "@inviter:foo.test" @@ -428,19 +476,22 @@ class EventAuthTestCase(unittest.TestCase): with self.assertRaises(AuthError): event_auth.check_auth_rules_for_event( RoomVersions.V8, - _join_event(pleb), + _join_event(RoomVersions.V8, pleb), auth_events.values(), ) # An join authorised by a user who is not in the room is rejected. pl_auth_events = auth_events.copy() pl_auth_events[("m.room.power_levels", "")] = _power_levels_event( - creator, {"invite": 100, "users": {"@other:example.com": 150}} + RoomVersions.V8, + creator, + {"invite": 100, "users": {"@other:example.com": 150}}, ) with self.assertRaises(AuthError): event_auth.check_auth_rules_for_event( RoomVersions.V8, _join_event( + RoomVersions.V8, pleb, additional_content={ EventContentFields.AUTHORISING_USER: "@other:example.com" @@ -455,6 +506,7 @@ class EventAuthTestCase(unittest.TestCase): event_auth.check_auth_rules_for_event( RoomVersions.V8, _member_event( + RoomVersions.V8, pleb, "join", sender=creator, @@ -466,7 +518,9 @@ class EventAuthTestCase(unittest.TestCase): ) # Banned should be rejected. - auth_events[("m.room.member", pleb)] = _member_event(pleb, "ban") + auth_events[("m.room.member", pleb)] = _member_event( + RoomVersions.V8, pleb, "ban" + ) with self.assertRaises(AuthError): event_auth.check_auth_rules_for_event( RoomVersions.V8, @@ -475,7 +529,9 @@ class EventAuthTestCase(unittest.TestCase): ) # A user who left can re-join. - auth_events[("m.room.member", pleb)] = _member_event(pleb, "leave") + auth_events[("m.room.member", pleb)] = _member_event( + RoomVersions.V8, pleb, "leave" + ) event_auth.check_auth_rules_for_event( RoomVersions.V8, authorised_join_event, @@ -484,21 +540,23 @@ class EventAuthTestCase(unittest.TestCase): # A user can send a join if they're in the room. (This doesn't need to # be authorised since the user is already joined.) - auth_events[("m.room.member", pleb)] = _member_event(pleb, "join") + auth_events[("m.room.member", pleb)] = _member_event( + RoomVersions.V8, pleb, "join" + ) event_auth.check_auth_rules_for_event( RoomVersions.V8, - _join_event(pleb), + _join_event(RoomVersions.V8, pleb), auth_events.values(), ) # A user can accept an invite. (This doesn't need to be authorised since # the user was invited.) auth_events[("m.room.member", pleb)] = _member_event( - pleb, "invite", sender=creator + RoomVersions.V8, pleb, "invite", sender=creator ) event_auth.check_auth_rules_for_event( RoomVersions.V8, - _join_event(pleb), + _join_event(RoomVersions.V8, pleb), auth_events.values(), ) @@ -508,20 +566,25 @@ class EventAuthTestCase(unittest.TestCase): TEST_ROOM_ID = "!test:room" -def _create_event(user_id: str) -> EventBase: +def _create_event( + room_version: RoomVersion, + user_id: str, +) -> EventBase: return make_event_from_dict( { "room_id": TEST_ROOM_ID, - "event_id": _get_event_id(), + **_maybe_get_event_id_dict_for_room_version(room_version), "type": "m.room.create", "state_key": "", "sender": user_id, "content": {"creator": user_id}, - } + }, + room_version=room_version, ) def _member_event( + room_version: RoomVersion, user_id: str, membership: str, sender: Optional[str] = None, @@ -530,79 +593,102 @@ def _member_event( return make_event_from_dict( { "room_id": TEST_ROOM_ID, - "event_id": _get_event_id(), + **_maybe_get_event_id_dict_for_room_version(room_version), "type": "m.room.member", "sender": sender or user_id, "state_key": user_id, "content": {"membership": membership, **(additional_content or {})}, "prev_events": [], - } + }, + room_version=room_version, ) -def _join_event(user_id: str, additional_content: Optional[dict] = None) -> EventBase: - return _member_event(user_id, "join", additional_content=additional_content) +def _join_event( + room_version: RoomVersion, + user_id: str, + additional_content: Optional[dict] = None, +) -> EventBase: + return _member_event( + room_version, + user_id, + "join", + additional_content=additional_content, + ) -def _power_levels_event(sender: str, content: JsonDict) -> EventBase: +def _power_levels_event( + room_version: RoomVersion, + sender: str, + content: JsonDict, +) -> EventBase: return make_event_from_dict( { "room_id": TEST_ROOM_ID, - "event_id": _get_event_id(), + **_maybe_get_event_id_dict_for_room_version(room_version), "type": "m.room.power_levels", "sender": sender, "state_key": "", "content": content, - } + }, + room_version=room_version, ) -def _alias_event(sender: str, **kwargs) -> EventBase: +def _alias_event(room_version: RoomVersion, sender: str, **kwargs) -> EventBase: data = { "room_id": TEST_ROOM_ID, - "event_id": _get_event_id(), + **_maybe_get_event_id_dict_for_room_version(room_version), "type": "m.room.aliases", "sender": sender, "state_key": get_domain_from_id(sender), "content": {"aliases": []}, } data.update(**kwargs) - return make_event_from_dict(data) + return make_event_from_dict(data, room_version=room_version) -def _random_state_event(sender: str) -> EventBase: +def _random_state_event(room_version: RoomVersion, sender: str) -> EventBase: return make_event_from_dict( { "room_id": TEST_ROOM_ID, - "event_id": _get_event_id(), + **_maybe_get_event_id_dict_for_room_version(room_version), "type": "test.state", "sender": sender, "state_key": "", "content": {"membership": "join"}, - } + }, + room_version=room_version, ) -def _join_rules_event(sender: str, join_rule: str) -> EventBase: +def _join_rules_event( + room_version: RoomVersion, sender: str, join_rule: str +) -> EventBase: return make_event_from_dict( { "room_id": TEST_ROOM_ID, - "event_id": _get_event_id(), + **_maybe_get_event_id_dict_for_room_version(room_version), "type": "m.room.join_rules", "sender": sender, "state_key": "", "content": { "join_rule": join_rule, }, - } + }, + room_version=room_version, ) event_count = 0 -def _get_event_id() -> str: +def _maybe_get_event_id_dict_for_room_version(room_version: RoomVersion) -> dict: + """If this room version needs it, generate an event id""" + if room_version.event_format != EventFormatVersions.V1: + return {} + global event_count c = event_count event_count += 1 - return "!%i:example.com" % (c,) + return {"event_id": "!%i:example.com" % (c,)} From 68be42f6b6433e93c7dccc0eae70177500ca60bc Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 9 Jun 2022 15:51:34 +0100 Subject: [PATCH 2/5] Remove `room_version` param from `validate_event_for_room_version` Instead, use the `room_version` property of the event we're validating. The `room_version` was originally added as a parameter somewhere around #4482, but really it's been redundant since #6875 added a `room_version` field to `EventBase`. --- synapse/event_auth.py | 12 ++++-------- synapse/events/validator.py | 4 ++++ synapse/handlers/federation.py | 4 ++-- synapse/handlers/federation_event.py | 4 ++-- synapse/handlers/message.py | 2 +- synapse/handlers/room.py | 2 +- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/synapse/event_auth.py b/synapse/event_auth.py index 4c0b587a76..77f90558d8 100644 --- a/synapse/event_auth.py +++ b/synapse/event_auth.py @@ -45,9 +45,7 @@ if typing.TYPE_CHECKING: logger = logging.getLogger(__name__) -def validate_event_for_room_version( - room_version_obj: RoomVersion, event: "EventBase" -) -> None: +def validate_event_for_room_version(event: "EventBase") -> None: """Ensure that the event complies with the limits, and has the right signatures NB: does not *validate* the signatures - it assumes that any signatures present @@ -60,12 +58,10 @@ def validate_event_for_room_version( NB: This is used to check events that have been received over federation. As such, it can only enforce the checks specified in the relevant room version, to avoid a split-brain situation where some servers accept such events, and others reject - them. - - TODO: consider moving this into EventValidator + them. See also EventValidator, which contains extra checks which are applied only to + locally-generated events. Args: - room_version_obj: the version of the room which contains this event event: the event to be checked Raises: @@ -103,7 +99,7 @@ def validate_event_for_room_version( raise AuthError(403, "Event not signed by sending server") is_invite_via_allow_rule = ( - room_version_obj.msc3083_join_rules + event.room_version.msc3083_join_rules and event.type == EventTypes.Member and event.membership == Membership.JOIN and EventContentFields.AUTHORISING_USER in event.content diff --git a/synapse/events/validator.py b/synapse/events/validator.py index 29fa9b3880..27c8beba25 100644 --- a/synapse/events/validator.py +++ b/synapse/events/validator.py @@ -35,6 +35,10 @@ class EventValidator: def validate_new(self, event: EventBase, config: HomeServerConfig) -> None: """Validates the event has roughly the right format + Suitable for checking a locally-created event. It has stricter checks than + is appropriate for an event received over federation (for which, see + event_auth.validate_event_for_room_version) + Args: event: The event to validate. config: The homeserver's configuration. diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 6a143440d3..b596417769 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -1206,7 +1206,7 @@ class FederationHandler: event.internal_metadata.send_on_behalf_of = self.hs.hostname try: - validate_event_for_room_version(room_version_obj, event) + validate_event_for_room_version(event) await self._event_auth_handler.check_auth_rules_from_context( room_version_obj, event, context ) @@ -1258,7 +1258,7 @@ class FederationHandler: ) try: - validate_event_for_room_version(room_version_obj, event) + validate_event_for_room_version(event) await self._event_auth_handler.check_auth_rules_from_context( room_version_obj, event, context ) diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index 87a0608359..420ad8b969 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -1453,7 +1453,7 @@ class FederationEventHandler: context = EventContext.for_outlier(self._storage_controllers) try: - validate_event_for_room_version(room_version_obj, event) + validate_event_for_room_version(event) check_auth_rules_for_event(room_version_obj, event, auth) except AuthError as e: logger.warning("Rejecting %r because %s", event, e) @@ -1501,7 +1501,7 @@ class FederationEventHandler: room_version_obj = KNOWN_ROOM_VERSIONS[room_version] try: - validate_event_for_room_version(room_version_obj, event) + validate_event_for_room_version(event) except AuthError as e: logger.warning("While validating received event %r: %s", event, e) # TODO: use a different rejected reason here? diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index f455158a2c..b078e2424f 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -1297,7 +1297,7 @@ class EventCreationHandler: assert event.content["membership"] == Membership.LEAVE else: try: - validate_event_for_room_version(room_version_obj, event) + validate_event_for_room_version(event) await self._event_auth_handler.check_auth_rules_from_context( room_version_obj, event, context ) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 520663f172..44d9784077 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -227,7 +227,7 @@ class RoomCreationHandler: }, ) old_room_version = await self.store.get_room_version(old_room_id) - validate_event_for_room_version(old_room_version, tombstone_event) + validate_event_for_room_version(tombstone_event) await self._event_auth_handler.check_auth_rules_from_context( old_room_version, tombstone_event, tombstone_context ) From 0d9d36b15c3cd2851f1cf3452c096480f568e6cb Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 10 Jun 2022 10:48:25 +0100 Subject: [PATCH 3/5] Remove `room_version` param from `check_auth_rules_for_event` Instead, use the `room_version` property of the event we're checking. The `room_version` was originally added as a parameter somewhere around #4482, but really it's been redundant since #6875 added a `room_version` field to `EventBase`. --- synapse/event_auth.py | 15 +++++----- synapse/handlers/event_auth.py | 2 +- synapse/handlers/federation_event.py | 16 +++-------- synapse/state/v1.py | 4 +-- synapse/state/v2.py | 1 - tests/test_event_auth.py | 43 ++-------------------------- 6 files changed, 16 insertions(+), 65 deletions(-) diff --git a/synapse/event_auth.py b/synapse/event_auth.py index 77f90558d8..e23503c1e0 100644 --- a/synapse/event_auth.py +++ b/synapse/event_auth.py @@ -113,7 +113,6 @@ def validate_event_for_room_version(event: "EventBase") -> None: def check_auth_rules_for_event( - room_version_obj: RoomVersion, event: "EventBase", auth_events: Iterable["EventBase"], ) -> None: @@ -132,7 +131,6 @@ def check_auth_rules_for_event( a bunch of other tests. Args: - room_version_obj: the version of the room event: the event being checked. auth_events: the room state to check the events against. @@ -201,7 +199,10 @@ def check_auth_rules_for_event( raise AuthError(403, "This room has been marked as unfederatable.") # 4. If type is m.room.aliases - if event.type == EventTypes.Aliases and room_version_obj.special_case_aliases_auth: + if ( + event.type == EventTypes.Aliases + and event.room_version.special_case_aliases_auth + ): # 4a. If event has no state_key, reject if not event.is_state(): raise AuthError(403, "Alias event must be a state event") @@ -221,7 +222,7 @@ def check_auth_rules_for_event( # 5. If type is m.room.membership if event.type == EventTypes.Member: - _is_membership_change_allowed(room_version_obj, event, auth_dict) + _is_membership_change_allowed(event.room_version, event, auth_dict) logger.debug("Allowing! %s", event) return @@ -243,17 +244,17 @@ def check_auth_rules_for_event( _can_send_event(event, auth_dict) if event.type == EventTypes.PowerLevels: - _check_power_levels(room_version_obj, event, auth_dict) + _check_power_levels(event.room_version, event, auth_dict) if event.type == EventTypes.Redaction: - check_redaction(room_version_obj, event, auth_dict) + check_redaction(event.room_version, event, auth_dict) if ( event.type == EventTypes.MSC2716_INSERTION or event.type == EventTypes.MSC2716_BATCH or event.type == EventTypes.MSC2716_MARKER ): - check_historical(room_version_obj, event, auth_dict) + check_historical(event.room_version, event, auth_dict) logger.debug("Allowing! %s", event) diff --git a/synapse/handlers/event_auth.py b/synapse/handlers/event_auth.py index 6bed464351..7bbb833f30 100644 --- a/synapse/handlers/event_auth.py +++ b/synapse/handlers/event_auth.py @@ -55,7 +55,7 @@ class EventAuthHandler: """Check an event passes the auth rules at its own auth events""" auth_event_ids = event.auth_event_ids() auth_events_by_id = await self._store.get_events(auth_event_ids) - check_auth_rules_for_event(room_version_obj, event, auth_events_by_id.values()) + check_auth_rules_for_event(event, auth_events_by_id.values()) def compute_auth_events( self, diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index 420ad8b969..9488fef297 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -1428,9 +1428,6 @@ class FederationEventHandler: allow_rejected=True, ) - room_version = await self._store.get_room_version_id(room_id) - room_version_obj = KNOWN_ROOM_VERSIONS[room_version] - def prep(event: EventBase) -> Optional[Tuple[EventBase, EventContext]]: with nested_logging_context(suffix=event.event_id): auth = [] @@ -1454,7 +1451,7 @@ class FederationEventHandler: context = EventContext.for_outlier(self._storage_controllers) try: validate_event_for_room_version(event) - check_auth_rules_for_event(room_version_obj, event, auth) + check_auth_rules_for_event(event, auth) except AuthError as e: logger.warning("Rejecting %r because %s", event, e) context.rejected = RejectedReason.AUTH_ERROR @@ -1497,9 +1494,6 @@ class FederationEventHandler: assert not event.internal_metadata.outlier # first of all, check that the event itself is valid. - room_version = await self._store.get_room_version_id(event.room_id) - room_version_obj = KNOWN_ROOM_VERSIONS[room_version] - try: validate_event_for_room_version(event) except AuthError as e: @@ -1519,7 +1513,7 @@ class FederationEventHandler: # ... and check that the event passes auth at those auth events. try: - check_auth_rules_for_event(room_version_obj, event, claimed_auth_events) + check_auth_rules_for_event(event, claimed_auth_events) except AuthError as e: logger.warning( "While checking auth of %r against auth_events: %s", event, e @@ -1567,9 +1561,7 @@ class FederationEventHandler: auth_events_for_auth = calculated_auth_event_map try: - check_auth_rules_for_event( - room_version_obj, event, auth_events_for_auth.values() - ) + check_auth_rules_for_event(event, auth_events_for_auth.values()) except AuthError as e: logger.warning("Failed auth resolution for %r because %s", event, e) context.rejected = RejectedReason.AUTH_ERROR @@ -1669,7 +1661,7 @@ class FederationEventHandler: ) try: - check_auth_rules_for_event(room_version_obj, event, current_auth_events) + check_auth_rules_for_event(event, current_auth_events) except AuthError as e: logger.warning( "Soft-failing %r (from %s) because %s", diff --git a/synapse/state/v1.py b/synapse/state/v1.py index 499a328201..8bbb4ce41c 100644 --- a/synapse/state/v1.py +++ b/synapse/state/v1.py @@ -30,7 +30,7 @@ from typing import ( from synapse import event_auth from synapse.api.constants import EventTypes from synapse.api.errors import AuthError -from synapse.api.room_versions import RoomVersion, RoomVersions +from synapse.api.room_versions import RoomVersion from synapse.events import EventBase from synapse.types import MutableStateMap, StateMap @@ -331,7 +331,6 @@ def _resolve_auth_events( try: # The signatures have already been checked at this point event_auth.check_auth_rules_for_event( - RoomVersions.V1, event, auth_events.values(), ) @@ -349,7 +348,6 @@ def _resolve_normal_events( try: # The signatures have already been checked at this point event_auth.check_auth_rules_for_event( - RoomVersions.V1, event, auth_events.values(), ) diff --git a/synapse/state/v2.py b/synapse/state/v2.py index c618df2fde..041ccac59e 100644 --- a/synapse/state/v2.py +++ b/synapse/state/v2.py @@ -547,7 +547,6 @@ async def _iterative_auth_checks( try: event_auth.check_auth_rules_for_event( - room_version, event, auth_events.values(), ) diff --git a/tests/test_event_auth.py b/tests/test_event_auth.py index 1e11fb5dac..229ecd84a6 100644 --- a/tests/test_event_auth.py +++ b/tests/test_event_auth.py @@ -38,7 +38,6 @@ class EventAuthTestCase(unittest.TestCase): # creator should be able to send state event_auth.check_auth_rules_for_event( - RoomVersions.V9, _random_state_event(RoomVersions.V9, creator), auth_events, ) @@ -55,7 +54,6 @@ class EventAuthTestCase(unittest.TestCase): self.assertRaises( AuthError, event_auth.check_auth_rules_for_event, - RoomVersions.V9, _random_state_event(RoomVersions.V9, creator), auth_events, ) @@ -66,7 +64,6 @@ class EventAuthTestCase(unittest.TestCase): self.assertRaises( AuthError, event_auth.check_auth_rules_for_event, - RoomVersions.V9, _random_state_event(RoomVersions.V9, creator), auth_events, ) @@ -86,7 +83,6 @@ class EventAuthTestCase(unittest.TestCase): # creator should be able to send state event_auth.check_auth_rules_for_event( - RoomVersions.V1, _random_state_event(RoomVersions.V1, creator), auth_events, ) @@ -95,7 +91,6 @@ class EventAuthTestCase(unittest.TestCase): self.assertRaises( AuthError, event_auth.check_auth_rules_for_event, - RoomVersions.V1, _random_state_event(RoomVersions.V1, joiner), auth_events, ) @@ -125,14 +120,12 @@ class EventAuthTestCase(unittest.TestCase): self.assertRaises( AuthError, event_auth.check_auth_rules_for_event, - RoomVersions.V1, _random_state_event(RoomVersions.V1, pleb), auth_events, ), # king should be able to send state event_auth.check_auth_rules_for_event( - RoomVersions.V1, _random_state_event(RoomVersions.V1, king), auth_events, ) @@ -148,7 +141,6 @@ class EventAuthTestCase(unittest.TestCase): # creator should be able to send aliases event_auth.check_auth_rules_for_event( - RoomVersions.V1, _alias_event(RoomVersions.V1, creator), auth_events, ) @@ -156,7 +148,6 @@ class EventAuthTestCase(unittest.TestCase): # Reject an event with no state key. with self.assertRaises(AuthError): event_auth.check_auth_rules_for_event( - RoomVersions.V1, _alias_event(RoomVersions.V1, creator, state_key=""), auth_events, ) @@ -164,14 +155,12 @@ class EventAuthTestCase(unittest.TestCase): # If the domain of the sender does not match the state key, reject. with self.assertRaises(AuthError): event_auth.check_auth_rules_for_event( - RoomVersions.V1, _alias_event(RoomVersions.V1, creator, state_key="test.com"), auth_events, ) # Note that the member does *not* need to be in the room. event_auth.check_auth_rules_for_event( - RoomVersions.V1, _alias_event(RoomVersions.V1, other), auth_events, ) @@ -187,19 +176,16 @@ class EventAuthTestCase(unittest.TestCase): # creator should be able to send aliases event_auth.check_auth_rules_for_event( - RoomVersions.V6, _alias_event(RoomVersions.V6, creator), auth_events, ) # No particular checks are done on the state key. event_auth.check_auth_rules_for_event( - RoomVersions.V6, _alias_event(RoomVersions.V6, creator, state_key=""), auth_events, ) event_auth.check_auth_rules_for_event( - RoomVersions.V6, _alias_event(RoomVersions.V6, creator, state_key="test.com"), auth_events, ) @@ -207,7 +193,6 @@ class EventAuthTestCase(unittest.TestCase): # Per standard auth rules, the member must be in the room. with self.assertRaises(AuthError): event_auth.check_auth_rules_for_event( - RoomVersions.V6, _alias_event(RoomVersions.V6, other), auth_events, ) @@ -235,14 +220,12 @@ class EventAuthTestCase(unittest.TestCase): # on room V1, pleb should be able to modify the notifications power level. if allow_modification: - event_auth.check_auth_rules_for_event(room_version, pl_event, auth_events) + event_auth.check_auth_rules_for_event(pl_event, auth_events) else: # But an MSC2209 room rejects this change. with self.assertRaises(AuthError): - event_auth.check_auth_rules_for_event( - room_version, pl_event, auth_events - ) + event_auth.check_auth_rules_for_event(pl_event, auth_events) def test_join_rules_public(self): """ @@ -261,7 +244,6 @@ class EventAuthTestCase(unittest.TestCase): # Check join. event_auth.check_auth_rules_for_event( - RoomVersions.V6, _join_event(RoomVersions.V6, pleb), auth_events.values(), ) @@ -269,7 +251,6 @@ class EventAuthTestCase(unittest.TestCase): # A user cannot be force-joined to a room. with self.assertRaises(AuthError): event_auth.check_auth_rules_for_event( - RoomVersions.V6, _member_event(RoomVersions.V6, pleb, "join", sender=creator), auth_events.values(), ) @@ -280,7 +261,6 @@ class EventAuthTestCase(unittest.TestCase): ) with self.assertRaises(AuthError): event_auth.check_auth_rules_for_event( - RoomVersions.V6, _join_event(RoomVersions.V6, pleb), auth_events.values(), ) @@ -290,7 +270,6 @@ class EventAuthTestCase(unittest.TestCase): RoomVersions.V6, pleb, "leave" ) event_auth.check_auth_rules_for_event( - RoomVersions.V6, _join_event(RoomVersions.V6, pleb), auth_events.values(), ) @@ -300,7 +279,6 @@ class EventAuthTestCase(unittest.TestCase): RoomVersions.V6, pleb, "join" ) event_auth.check_auth_rules_for_event( - RoomVersions.V6, _join_event(RoomVersions.V6, pleb), auth_events.values(), ) @@ -310,7 +288,6 @@ class EventAuthTestCase(unittest.TestCase): RoomVersions.V6, pleb, "invite", sender=creator ) event_auth.check_auth_rules_for_event( - RoomVersions.V6, _join_event(RoomVersions.V6, pleb), auth_events.values(), ) @@ -333,7 +310,6 @@ class EventAuthTestCase(unittest.TestCase): # A join without an invite is rejected. with self.assertRaises(AuthError): event_auth.check_auth_rules_for_event( - RoomVersions.V6, _join_event(RoomVersions.V6, pleb), auth_events.values(), ) @@ -341,7 +317,6 @@ class EventAuthTestCase(unittest.TestCase): # A user cannot be force-joined to a room. with self.assertRaises(AuthError): event_auth.check_auth_rules_for_event( - RoomVersions.V6, _member_event(RoomVersions.V6, pleb, "join", sender=creator), auth_events.values(), ) @@ -352,7 +327,6 @@ class EventAuthTestCase(unittest.TestCase): ) with self.assertRaises(AuthError): event_auth.check_auth_rules_for_event( - RoomVersions.V6, _join_event(RoomVersions.V6, pleb), auth_events.values(), ) @@ -363,7 +337,6 @@ class EventAuthTestCase(unittest.TestCase): ) with self.assertRaises(AuthError): event_auth.check_auth_rules_for_event( - RoomVersions.V6, _join_event(RoomVersions.V6, pleb), auth_events.values(), ) @@ -373,7 +346,6 @@ class EventAuthTestCase(unittest.TestCase): RoomVersions.V6, pleb, "join" ) event_auth.check_auth_rules_for_event( - RoomVersions.V6, _join_event(RoomVersions.V6, pleb), auth_events.values(), ) @@ -383,7 +355,6 @@ class EventAuthTestCase(unittest.TestCase): RoomVersions.V6, pleb, "invite", sender=creator ) event_auth.check_auth_rules_for_event( - RoomVersions.V6, _join_event(RoomVersions.V6, pleb), auth_events.values(), ) @@ -406,7 +377,6 @@ class EventAuthTestCase(unittest.TestCase): with self.assertRaises(AuthError): event_auth.check_auth_rules_for_event( - RoomVersions.V6, _join_event(RoomVersions.V6, pleb), auth_events.values(), ) @@ -444,7 +414,6 @@ class EventAuthTestCase(unittest.TestCase): }, ) event_auth.check_auth_rules_for_event( - RoomVersions.V8, authorised_join_event, auth_events.values(), ) @@ -461,7 +430,6 @@ class EventAuthTestCase(unittest.TestCase): RoomVersions.V8, "@inviter:foo.test" ) event_auth.check_auth_rules_for_event( - RoomVersions.V8, _join_event( RoomVersions.V8, pleb, @@ -475,7 +443,6 @@ class EventAuthTestCase(unittest.TestCase): # A join which is missing an authorised server is rejected. with self.assertRaises(AuthError): event_auth.check_auth_rules_for_event( - RoomVersions.V8, _join_event(RoomVersions.V8, pleb), auth_events.values(), ) @@ -489,7 +456,6 @@ class EventAuthTestCase(unittest.TestCase): ) with self.assertRaises(AuthError): event_auth.check_auth_rules_for_event( - RoomVersions.V8, _join_event( RoomVersions.V8, pleb, @@ -504,7 +470,6 @@ class EventAuthTestCase(unittest.TestCase): # *would* be valid, but is sent be a different user.) with self.assertRaises(AuthError): event_auth.check_auth_rules_for_event( - RoomVersions.V8, _member_event( RoomVersions.V8, pleb, @@ -523,7 +488,6 @@ class EventAuthTestCase(unittest.TestCase): ) with self.assertRaises(AuthError): event_auth.check_auth_rules_for_event( - RoomVersions.V8, authorised_join_event, auth_events.values(), ) @@ -533,7 +497,6 @@ class EventAuthTestCase(unittest.TestCase): RoomVersions.V8, pleb, "leave" ) event_auth.check_auth_rules_for_event( - RoomVersions.V8, authorised_join_event, auth_events.values(), ) @@ -544,7 +507,6 @@ class EventAuthTestCase(unittest.TestCase): RoomVersions.V8, pleb, "join" ) event_auth.check_auth_rules_for_event( - RoomVersions.V8, _join_event(RoomVersions.V8, pleb), auth_events.values(), ) @@ -555,7 +517,6 @@ class EventAuthTestCase(unittest.TestCase): RoomVersions.V8, pleb, "invite", sender=creator ) event_auth.check_auth_rules_for_event( - RoomVersions.V8, _join_event(RoomVersions.V8, pleb), auth_events.values(), ) From c1b28b8842849a2b3e62025d378997861c041932 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 10 Jun 2022 11:01:55 +0100 Subject: [PATCH 4/5] Remove redundant `room_version` param from `check_auth_rules_from_context` It's now implied by the room_version property on the event. --- synapse/handlers/event_auth.py | 1 - synapse/handlers/federation.py | 18 +++++------------- synapse/handlers/message.py | 21 ++------------------- synapse/handlers/room.py | 3 +-- 4 files changed, 8 insertions(+), 35 deletions(-) diff --git a/synapse/handlers/event_auth.py b/synapse/handlers/event_auth.py index 7bbb833f30..ed4149bd58 100644 --- a/synapse/handlers/event_auth.py +++ b/synapse/handlers/event_auth.py @@ -48,7 +48,6 @@ class EventAuthHandler: async def check_auth_rules_from_context( self, - room_version_obj: RoomVersion, event: EventBase, context: EventContext, ) -> None: diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index b596417769..6310f0ef27 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -799,9 +799,7 @@ class FederationHandler: # The remote hasn't signed it yet, obviously. We'll do the full checks # when we get the event back in `on_send_join_request` - await self._event_auth_handler.check_auth_rules_from_context( - room_version, event, context - ) + await self._event_auth_handler.check_auth_rules_from_context(event, context) return event async def on_invite_request( @@ -972,9 +970,7 @@ class FederationHandler: try: # The remote hasn't signed it yet, obviously. We'll do the full checks # when we get the event back in `on_send_leave_request` - await self._event_auth_handler.check_auth_rules_from_context( - room_version_obj, event, context - ) + await self._event_auth_handler.check_auth_rules_from_context(event, context) except AuthError as e: logger.warning("Failed to create new leave %r because %s", event, e) raise e @@ -1033,9 +1029,7 @@ class FederationHandler: try: # The remote hasn't signed it yet, obviously. We'll do the full checks # when we get the event back in `on_send_knock_request` - await self._event_auth_handler.check_auth_rules_from_context( - room_version_obj, event, context - ) + await self._event_auth_handler.check_auth_rules_from_context(event, context) except AuthError as e: logger.warning("Failed to create new knock %r because %s", event, e) raise e @@ -1208,7 +1202,7 @@ class FederationHandler: try: validate_event_for_room_version(event) await self._event_auth_handler.check_auth_rules_from_context( - room_version_obj, event, context + event, context ) except AuthError as e: logger.warning("Denying new third party invite %r because %s", event, e) @@ -1259,9 +1253,7 @@ class FederationHandler: try: validate_event_for_room_version(event) - await self._event_auth_handler.check_auth_rules_from_context( - room_version_obj, event, context - ) + await self._event_auth_handler.check_auth_rules_from_context(event, context) except AuthError as e: logger.warning("Denying third party invite %r because %s", event, e) raise e diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index b078e2424f..c8bbcfd8c2 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -42,7 +42,7 @@ from synapse.api.errors import ( SynapseError, UnsupportedRoomVersionError, ) -from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersions +from synapse.api.room_versions import KNOWN_ROOM_VERSIONS from synapse.api.urls import ConsentURIBuilder from synapse.event_auth import validate_event_for_room_version from synapse.events import EventBase, relation_from_event @@ -1273,23 +1273,6 @@ class EventCreationHandler: ) return prev_event - if event.is_state() and (event.type, event.state_key) == ( - EventTypes.Create, - "", - ): - room_version_id = event.content.get( - "room_version", RoomVersions.V1.identifier - ) - maybe_room_version_obj = KNOWN_ROOM_VERSIONS.get(room_version_id) - if not maybe_room_version_obj: - raise UnsupportedRoomVersionError( - "Attempt to create a room with unsupported room version %s" - % (room_version_id,) - ) - room_version_obj = maybe_room_version_obj - else: - room_version_obj = await self.store.get_room_version(event.room_id) - if event.internal_metadata.is_out_of_band_membership(): # the only sort of out-of-band-membership events we expect to see here are # invite rejections and rescinded knocks that we have generated ourselves. @@ -1299,7 +1282,7 @@ class EventCreationHandler: try: validate_event_for_room_version(event) await self._event_auth_handler.check_auth_rules_from_context( - room_version_obj, event, context + event, context ) except AuthError as err: logger.warning("Denying new event %r because %s", event, err) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 44d9784077..d8918ee1aa 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -226,10 +226,9 @@ class RoomCreationHandler: }, }, ) - old_room_version = await self.store.get_room_version(old_room_id) validate_event_for_room_version(tombstone_event) await self._event_auth_handler.check_auth_rules_from_context( - old_room_version, tombstone_event, tombstone_context + tombstone_event, tombstone_context ) # Upgrade the room From a6173a16fe308291054b5fb507e6c160161ed0a5 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 10 Jun 2022 11:08:01 +0100 Subject: [PATCH 5/5] changelog --- changelog.d/13017.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/13017.misc diff --git a/changelog.d/13017.misc b/changelog.d/13017.misc new file mode 100644 index 0000000000..b314687f9c --- /dev/null +++ b/changelog.d/13017.misc @@ -0,0 +1 @@ +Remove redundant `room_version` parameters from event auth functions.