Merge branch 'rav/simplify_event_auth_interface' into develop

11 files changed, 239 insertions, 228 deletions

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.
diff --git a/synapse/event_auth.py b/synapse/event_auth.py
index 4c0b587a76..e23503c1e0 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
@@ -117,7 +113,6 @@ def validate_event_for_room_version(
 
 
 def check_auth_rules_for_event(
-    room_version_obj: RoomVersion,
     event: "EventBase",
     auth_events: Iterable["EventBase"],
 ) -> None:
@@ -136,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.
 
@@ -205,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")
@@ -225,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
 
@@ -247,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/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/event_auth.py b/synapse/handlers/event_auth.py
index 6bed464351..ed4149bd58 100644
--- a/synapse/handlers/event_auth.py
+++ b/synapse/handlers/event_auth.py
@@ -48,14 +48,13 @@ class EventAuthHandler:
 
     async def check_auth_rules_from_context(
         self,
-        room_version_obj: RoomVersion,
         event: EventBase,
         context: EventContext,
     ) -> None:
         """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.py b/synapse/handlers/federation.py
index 5e16139626..1e5694244a 100644
--- a/synapse/handlers/federation.py
+++ b/synapse/handlers/federation.py
@@ -800,9 +800,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(
@@ -973,9 +971,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
@@ -1034,9 +1030,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
@@ -1207,9 +1201,9 @@ 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
+                    event, context
                 )
             except AuthError as e:
                 logger.warning("Denying new third party invite %r because %s", event, e)
@@ -1259,10 +1253,8 @@ class FederationHandler:
         )
 
         try:
-            validate_event_for_room_version(room_version_obj, event)
-            await self._event_auth_handler.check_auth_rules_from_context(
-                room_version_obj, event, context
-            )
+            validate_event_for_room_version(event)
+            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/federation_event.py b/synapse/handlers/federation_event.py
index 9889d1cb44..6c9e6a00b5 100644
--- a/synapse/handlers/federation_event.py
+++ b/synapse/handlers/federation_event.py
@@ -1430,9 +1430,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 = []
@@ -1455,8 +1452,8 @@ class FederationEventHandler:
 
                 context = EventContext.for_outlier(self._storage_controllers)
                 try:
-                    validate_event_for_room_version(room_version_obj, event)
-                    check_auth_rules_for_event(room_version_obj, event, auth)
+                    validate_event_for_room_version(event)
+                    check_auth_rules_for_event(event, auth)
                 except AuthError as e:
                     logger.warning("Rejecting %r because %s", event, e)
                     context.rejected = RejectedReason.AUTH_ERROR
@@ -1499,11 +1496,8 @@ 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(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?
@@ -1521,7 +1515,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
@@ -1569,9 +1563,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
@@ -1671,7 +1663,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/handlers/message.py b/synapse/handlers/message.py
index 294217cc23..9b17939163 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
@@ -1274,23 +1274,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.
@@ -1298,9 +1281,9 @@ 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
+                    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 520663f172..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(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
+            tombstone_event, tombstone_context
         )
 
         # Upgrade the room
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 0e609114ef..6a16f38a15 100644
--- a/synapse/state/v2.py
+++ b/synapse/state/v2.py
@@ -574,7 +574,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 e2c506e5a4..229ecd84a6 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,38 +32,39 @@ 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)
 
         self.assertRaises(
             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 +76,14 @@ 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,
         )
 
@@ -89,8 +91,7 @@ class EventAuthTestCase(unittest.TestCase):
         self.assertRaises(
             AuthError,
             event_auth.check_auth_rules_for_event,
-            RoomVersions.V1,
-            _random_state_event(joiner),
+            _random_state_event(RoomVersions.V1, joiner),
             auth_events,
         )
 
@@ -104,28 +105,28 @@ 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
         self.assertRaises(
             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,37 +135,33 @@ 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,
         )
 
         # Reject an event with no state key.
         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,
             )
 
         # 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(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,38 +170,35 @@ 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,
         )
 
         # 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(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 +206,26 @@ 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(pl_event, auth_events)
+
+        else:
+            # But an MSC2209 room rejects this change.
+            with self.assertRaises(AuthError):
+                event_auth.check_auth_rules_for_event(pl_event, auth_events)
 
     def test_join_rules_public(self):
         """
@@ -243,58 +235,60 @@ 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(),
         )
 
         # A user cannot be force-joined to a room.
         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,64 +300,88 @@ 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(),
             )
 
         # A user cannot be force-joined to a room.
         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(
+                _join_event(RoomVersions.V6, pleb),
+                auth_events.values(),
+            )
+
+    def test_join_rules_msc3083_restricted(self) -> None:
         """
         Test joining a restricted room from MSC3083.
 
@@ -377,29 +395,25 @@ 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"
             },
         )
         event_auth.check_auth_rules_for_event(
-            RoomVersions.V8,
             authorised_join_event,
             auth_events.values(),
         )
@@ -408,14 +422,16 @@ 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"
@@ -427,20 +443,21 @@ 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(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"
@@ -453,8 +470,8 @@ 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,
                     "join",
                     sender=creator,
@@ -466,39 +483,41 @@ 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,
                 authorised_join_event,
                 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.V8, pleb, "leave"
+        )
         event_auth.check_auth_rules_for_event(
-            RoomVersions.V8,
             authorised_join_event,
             auth_events.values(),
         )
 
         # 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 +527,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 +554,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,)}