fix(v12): Create tombstone event on room upgrade

src/api/client/room/upgrade.rs

@@ -68,6 +68,33 @@ pub(crate) async fn upgrade_room_route(
 		return Err!(Request(UserSuspended("You cannot perform this action while suspended.")));
 	}
 
+	// First, check if the user has permission to upgrade the room (send tombstone
+	// event)
+	let old_room_state_lock = services.rooms.state.mutex.lock(&body.room_id).await;
+
+	// Check tombstone permission by attempting to create (but not send) the event
+	// Note that this does internally call the policy server with a fake room ID,
+	// which may not be good?
+	let tombstone_test_result = services
+		.rooms
+		.timeline
+		.create_hash_and_sign_event(
+			PduBuilder::state(StateKey::new(), &RoomTombstoneEventContent {
+				body: "This room has been replaced".to_owned(),
+				replacement_room: RoomId::new(services.globals.server_name()),
+			}),
+			sender_user,
+			Some(&body.room_id),
+			&old_room_state_lock,
+		)
+		.await;
+
+	if let Err(_e) = tombstone_test_result {
+		return Err!(Request(Forbidden("User does not have permission to upgrade this room.")));
+	}
+
+	drop(old_room_state_lock);
+
 	// Create a replacement room
 	let room_features = RoomVersion::new(&body.new_version)?;
 	let replacement_room: Option<&RoomId> = if room_features.room_ids_as_hashes {
@@ -86,20 +113,18 @@ pub(crate) async fn upgrade_room_route(
 		.get_or_create_shortroomid(replacement_room_tmp)
 		.await;
 
-	let tombstone_event_id = if room_features.room_ids_as_hashes {
+	// For pre-v12 rooms, send tombstone before creating replacement room
-		None
+	let tombstone_event_id = if !room_features.room_ids_as_hashes {
-	} else {
 		let state_lock = services.rooms.state.mutex.lock(&body.room_id).await;
 		// Send a m.room.tombstone event to the old room to indicate that it is not
-		// intended to be used any further Fail if the sender does not have the required
+		// intended to be used any further
-		// permissions
 		let tombstone_event_id = services
 			.rooms
 			.timeline
 			.build_and_append_pdu(
 				PduBuilder::state(StateKey::new(), &RoomTombstoneEventContent {
 					body: "This room has been replaced".to_owned(),
-					replacement_room: replacement_room.clone().unwrap().to_owned(),
+					replacement_room: replacement_room.unwrap().to_owned(),
 				}),
 				sender_user,
 				Some(&body.room_id),
@@ -109,6 +134,8 @@ pub(crate) async fn upgrade_room_route(
 		// Change lock to replacement room
 		drop(state_lock);
 		Some(tombstone_event_id)
+	} else {
+		None
 	};
 	let state_lock = services.rooms.state.mutex.lock(replacement_room_tmp).await;
 
@@ -330,6 +357,27 @@ pub(crate) async fn upgrade_room_route(
 
 	drop(state_lock);
 
+	// For v12 rooms, send tombstone AFTER creating replacement room
+	if room_features.room_ids_as_hashes {
+		let old_room_state_lock = services.rooms.state.mutex.lock(&body.room_id).await;
+		// For v12 rooms, no event reference in predecessor due to cyclic dependency -
+		// could best effort one maybe?
+		services
+			.rooms
+			.timeline
+			.build_and_append_pdu(
+				PduBuilder::state(StateKey::new(), &RoomTombstoneEventContent {
+					body: "This room has been replaced".to_owned(),
+					replacement_room: replacement_room.unwrap().to_owned(),
+				}),
+				sender_user,
+				Some(&body.room_id),
+				&old_room_state_lock,
+			)
+			.await?;
+		drop(old_room_state_lock);
+	}
+
 	// Check if the old room has a space parent, and if so, whether we should update
 	// it (m.space.parent, room_id)
 	let parents = services

src/service/rooms/timeline/create.rs

@@ -274,8 +274,6 @@ pub async fn create_hash_and_sign_event(
 	pdu_json.insert("event_id".into(), CanonicalJsonValue::String(pdu.event_id.clone().into()));
 
 	// Check with the policy server
-	// TODO(hydra): Skip this check for create events (why didnt we do this
-	// already?)
 	if room_id.is_some() {
 		trace!(
 			"Checking event {} in room {} with policy server",