fix(v12): Create tombstone event on room upgrade

src/api/client/room/upgrade.rs

@@ -2,7 +2,7 @@ use std::cmp::max;
 
 use axum::extract::State;
 use conduwuit::{
-	Err, Error, Event, Result, debug, err, info,
+	Err, Error, Event, Result, RoomVersion, debug, err, info,
 	matrix::{StateKey, pdu::PduBuilder},
 };
 use futures::{FutureExt, StreamExt};
@@ -68,37 +68,76 @@ 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 replacement_room = RoomId::new(services.globals.server_name());
+	let room_features = RoomVersion::new(&body.new_version)?;
+	let replacement_room: Option<&RoomId> = if room_features.room_ids_as_hashes {
+		None
+	} else {
+		Some(&RoomId::new(services.globals.server_name()))
+	};
+	let replacement_room_tmp = match replacement_room {
+		| Some(v) => v,
+		| None => &RoomId::new(services.globals.server_name()),
+	};
 
 	let _short_id = services
 		.rooms
 		.short
-		.get_or_create_shortroomid(&replacement_room)
+		.get_or_create_shortroomid(replacement_room_tmp)
 		.await;
 
-	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
-	// 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(),
-			}),
-			sender_user,
-			Some(&body.room_id),
-			&state_lock,
-		)
-		.await?;
-
-	// Change lock to replacement room
-	drop(state_lock);
-	let state_lock = services.rooms.state.mutex.lock(&replacement_room).await;
+	// For pre-v12 rooms, send tombstone before creating replacement room
+	let tombstone_event_id = if !room_features.room_ids_as_hashes {
+		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
+		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.unwrap().to_owned(),
+				}),
+				sender_user,
+				Some(&body.room_id),
+				&state_lock,
+			)
+			.await?;
+		// 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;
 
 	// Get the old room creation event
 	let mut create_event_content: CanonicalJsonObject = services
@@ -111,7 +150,7 @@ pub(crate) async fn upgrade_room_route(
 	// Use the m.room.tombstone event as the predecessor
 	let predecessor = Some(ruma::events::room::create::PreviousRoom::new(
 		body.room_id.clone(),
-		Some(tombstone_event_id),
+		tombstone_event_id,
 	));
 
 	// Send a m.room.create event containing a predecessor field and the applicable
@@ -132,6 +171,7 @@ pub(crate) async fn upgrade_room_route(
 				// "creator" key no longer exists in V11 rooms
 				create_event_content.remove("creator");
 			},
+			// TODO(hydra): additional_creators
 		}
 	}
 
@@ -159,7 +199,7 @@ pub(crate) async fn upgrade_room_route(
 		return Err(Error::BadRequest(ErrorKind::BadJson, "Error forming creation event"));
 	}
 
-	services
+	let create_event_id = services
 		.rooms
 		.timeline
 		.build_and_append_pdu(
@@ -173,11 +213,18 @@ pub(crate) async fn upgrade_room_route(
 				timestamp: None,
 			},
 			sender_user,
-			Some(&replacement_room),
+			replacement_room,
 			&state_lock,
 		)
 		.boxed()
 		.await?;
+	let create_id = create_event_id.as_str().replace('$', "!");
+	let (replacement_room, state_lock) = if room_features.room_ids_as_hashes {
+		let parsed_room_id = RoomId::parse(&create_id)?;
+		(Some(parsed_room_id), services.rooms.state.mutex.lock(parsed_room_id).await)
+	} else {
+		(replacement_room, state_lock)
+	};
 
 	// Join the new room
 	services
@@ -204,7 +251,7 @@ pub(crate) async fn upgrade_room_route(
 				timestamp: None,
 			},
 			sender_user,
-			Some(&replacement_room),
+			replacement_room,
 			&state_lock,
 		)
 		.boxed()
@@ -243,7 +290,7 @@ pub(crate) async fn upgrade_room_route(
 						..Default::default()
 					},
 					sender_user,
-					Some(&replacement_room),
+					replacement_room,
 					&state_lock,
 				)
 				.boxed()
@@ -268,7 +315,7 @@ pub(crate) async fn upgrade_room_route(
 		services
 			.rooms
 			.alias
-			.set_alias(alias, &replacement_room, sender_user)?;
+			.set_alias(alias, replacement_room.unwrap(), sender_user)?;
 	}
 
 	// Get the old room power levels
@@ -310,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
@@ -334,8 +402,9 @@ pub(crate) async fn upgrade_room_route(
 			continue;
 		};
 		debug!(
-			"Updating space {space_id} child event for room {} to {replacement_room}",
-			&body.room_id
+			"Updating space {space_id} child event for room {} to {}",
+			&body.room_id,
+			replacement_room.unwrap()
 		);
 		// First, drop the space's child event
 		let state_lock = services.rooms.state.mutex.lock(space_id).await;
@@ -359,7 +428,10 @@ pub(crate) async fn upgrade_room_route(
 			.await
 			.ok();
 		// Now, add a new child event for the replacement room
-		debug!("Adding space child event for room {replacement_room} in space {space_id}");
+		debug!(
+			"Adding space child event for room {} in space {space_id}",
+			replacement_room.unwrap()
+		);
 		services
 			.rooms
 			.timeline
@@ -372,7 +444,7 @@ pub(crate) async fn upgrade_room_route(
 						suggested: child.suggested,
 					})
 					.expect("event is valid, we just created it"),
-					state_key: Some(replacement_room.as_str().into()),
+					state_key: Some(replacement_room.unwrap().as_str().into()),
 					..Default::default()
 				},
 				sender_user,
@@ -383,12 +455,15 @@ pub(crate) async fn upgrade_room_route(
 			.await
 			.ok();
 		debug!(
-			"Finished updating space {space_id} child event for room {} to {replacement_room}",
-			&body.room_id
+			"Finished updating space {space_id} child event for room {} to {}",
+			&body.room_id,
+			replacement_room.unwrap()
 		);
 		drop(state_lock);
 	}
 
 	// Return the replacement room id
-	Ok(upgrade_room::v3::Response { replacement_room })
+	Ok(upgrade_room::v3::Response {
+		replacement_room: replacement_room.unwrap().to_owned(),
+	})
 }

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",