fix: Remove non-compliant and non-functional non-authoritative directory queries

chore: Add news frag

changelog.d/1393.bugfix

@@ -0,0 +1 @@
+Removed non-compliant nor functional room alias lookups over federation. Contributed by @nex

src/admin/room/moderation.rs

@@ -89,13 +89,7 @@ async fn ban_room(&self, room: OwnedRoomOrAliasId) -> Result {
 			 locally, if not using get_alias_helper to fetch room ID remotely"
 		);
 
-		match self
+		match self.services.rooms.alias.resolve_alias(room_alias).await {
-			.services
-			.rooms
-			.alias
-			.resolve_alias(room_alias, None)
-			.await
-		{
 			| Ok((room_id, servers)) => {
 				debug!(
 					%room_id,
@@ -235,7 +229,7 @@ async fn ban_list_of_rooms(&self) -> Result {
 										.services
 										.rooms
 										.alias
-										.resolve_alias(room_alias, None)
+										.resolve_alias(room_alias)
 										.await
 									{
 										| Ok((room_id, servers)) => {
@@ -388,13 +382,7 @@ async fn unban_room(&self, room: OwnedRoomOrAliasId) -> Result {
 					 room ID over federation"
 				);
 
-				match self
+				match self.services.rooms.alias.resolve_alias(room_alias).await {
-					.services
-					.rooms
-					.alias
-					.resolve_alias(room_alias, None)
-					.await
-				{
 					| Ok((room_id, servers)) => {
 						debug!(
 							%room_id,

src/api/client/alias.rs

@@ -1,12 +1,6 @@
 use axum::extract::State;
-use conduwuit::{Err, Result, debug};
+use conduwuit::{Err, Result};
-use conduwuit_service::Services;
+use ruma::api::client::alias::{create_alias, delete_alias, get_alias};
-use futures::StreamExt;
-use rand::seq::SliceRandom;
-use ruma::{
-	OwnedServerName, RoomAliasId, RoomId,
-	api::client::alias::{create_alias, delete_alias, get_alias},
-};
 
 use crate::Ruma;
 
@@ -96,65 +90,9 @@ pub(crate) async fn get_alias_route(
 ) -> Result<get_alias::v3::Response> {
 	let room_alias = body.body.room_alias;
 
-	let Ok((room_id, servers)) = services.rooms.alias.resolve_alias(&room_alias, None).await
+	let Ok((room_id, servers)) = services.rooms.alias.resolve_alias(&room_alias).await else {
-	else {
 		return Err!(Request(NotFound("Room with alias not found.")));
 	};
 
-	let servers = room_available_servers(&services, &room_id, &room_alias, servers).await;
-	debug!(%room_alias, %room_id, "available servers: {servers:?}");
-
 	Ok(get_alias::v3::Response::new(room_id, servers))
 }
-
-async fn room_available_servers(
-	services: &Services,
-	room_id: &RoomId,
-	room_alias: &RoomAliasId,
-	pre_servers: Vec<OwnedServerName>,
-) -> Vec<OwnedServerName> {
-	// find active servers in room state cache to suggest
-	let mut servers: Vec<OwnedServerName> = services
-		.rooms
-		.state_cache
-		.room_servers(room_id)
-		.map(ToOwned::to_owned)
-		.collect()
-		.await;
-
-	// push any servers we want in the list already (e.g. responded remote alias
-	// servers, room alias server itself)
-	servers.extend(pre_servers);
-
-	servers.sort_unstable();
-	servers.dedup();
-
-	// shuffle list of servers randomly after sort and dedupe
-	servers.shuffle(&mut rand::thread_rng());
-
-	// insert our server as the very first choice if in list, else check if we can
-	// prefer the room alias server first
-	match servers
-		.iter()
-		.position(|server_name| services.globals.server_is_ours(server_name))
-	{
-		| Some(server_index) => {
-			servers.swap_remove(server_index);
-			servers.insert(0, services.globals.server_name().to_owned());
-		},
-		| _ => {
-			match servers
-				.iter()
-				.position(|server| server == room_alias.server_name())
-			{
-				| Some(alias_server_index) => {
-					servers.swap_remove(alias_server_index);
-					servers.insert(0, room_alias.server_name().into());
-				},
-				| _ => {},
-			}
-		},
-	}
-
-	servers
-}

src/api/client/membership/join.rs

@@ -198,11 +198,7 @@ pub(crate) async fn join_room_by_id_or_alias_route(
 			(servers, room_id)
 		},
 		| Err(room_alias) => {
-			let (room_id, mut servers) = services
+			let (room_id, mut servers) = services.rooms.alias.resolve_alias(&room_alias).await?;
-				.rooms
-				.alias
-				.resolve_alias(&room_alias, Some(body.via.clone()))
-				.await?;
 
 			banned_room_check(
 				&services,

src/api/client/membership/knock.rs

@@ -102,11 +102,7 @@ pub(crate) async fn knock_room_route(
 			(servers, room_id)
 		},
 		| Err(room_alias) => {
-			let (room_id, mut servers) = services
+			let (room_id, mut servers) = services.rooms.alias.resolve_alias(&room_alias).await?;
-				.rooms
-				.alias
-				.resolve_alias(&room_alias, Some(body.via.clone()))
-				.await?;
 
 			banned_room_check(
 				&services,

src/api/client/state.rs

@@ -342,10 +342,10 @@ async fn allowed_to_send_state_event(
 					}
 
 					for alias in aliases {
-						let (alias_room_id, _servers) = services
+						let (alias_room_id, _) = services
 							.rooms
 							.alias
-							.resolve_alias(&alias, None)
+							.resolve_alias(&alias)
 							.await
 							.map_err(|e| {
 								err!(Request(Unknown("Failed resolving alias \"{alias}\": {e}")))

src/database/engine/open.rs

@@ -35,8 +35,7 @@ pub(crate) async fn open(ctx: Arc<Context>, desc: &[Descriptor]) -> Result<Arc<S
 	}
 
 	debug!("Opening database...");
-	let db = Db::open_cf_descriptors(&db_opts, path, cfds)
+	let db = Db::open_cf_descriptors(&db_opts, path, cfds).or_else(or_else)?;
-	.or_else(or_else)?;
 
 	info!(
 		columns = num_cfds,

src/service/rooms/alias/mod.rs

@@ -3,7 +3,7 @@ mod remote;
 use std::sync::Arc;
 
 use conduwuit::{
-	Err, Event, Result, Server, err,
+	Err, Event, Result, err,
 	utils::{ReadyExt, stream::TryIgnore},
 };
 use database::{Deserialized, Ignore, Interfix, Map};
@@ -30,12 +30,12 @@ struct Data {
 }
 
 struct Services {
-	server: Arc<Server>,
 	admin: Dep<admin::Service>,
 	appservice: Dep<appservice::Service>,
 	globals: Dep<globals::Service>,
 	sending: Dep<sending::Service>,
 	state_accessor: Dep<rooms::state_accessor::Service>,
+	state_cache: Dep<rooms::state_cache::Service>,
 }
 
 impl crate::Service for Service {
@@ -47,13 +47,13 @@ impl crate::Service for Service {
 				aliasid_alias: args.db["aliasid_alias"].clone(),
 			},
 			services: Services {
-				server: args.server.clone(),
 				admin: args.depend::<admin::Service>("admin"),
 				appservice: args.depend::<appservice::Service>("appservice"),
 				globals: args.depend::<globals::Service>("globals"),
 				sending: args.depend::<sending::Service>("sending"),
 				state_accessor: args
 					.depend::<rooms::state_accessor::Service>("rooms::state_accessor"),
+				state_cache: args.depend::<rooms::state_cache::Service>("rooms::state_cache"),
 			},
 		}))
 	}
@@ -117,6 +117,9 @@ impl Service {
 		Ok(())
 	}
 
+	/// Resolves the given room ID or alias, returning the resolved room ID.
+	/// Unlike resolve_with_servers (the underlying call), potential resident
+	/// servers are not returned
 	#[inline]
 	pub async fn resolve(&self, room: &RoomOrAliasId) -> Result<OwnedRoomId> {
 		self.resolve_with_servers(room, None)
@@ -124,6 +127,14 @@ impl Service {
 			.map(|(room_id, _)| room_id)
 	}
 
+	/// Resolves the given room ID or alias, returning the resolved room ID, and
+	/// any servers that might be able to assist in fetching room data.
+	///
+	/// If the input is a room ID, this simply returns it and <servers>.
+	/// If the input is an alias, this attempts to resolve it locally, then via
+	/// appservices, and finally remotely if the alias is not local.
+	/// If the alias is successfully resolved, the room ID and an empty list of
+	/// servers is returned.
 	pub async fn resolve_with_servers(
 		&self,
 		room: &RoomOrAliasId,
@@ -134,28 +145,26 @@ impl Service {
 			Ok((room_id.to_owned(), servers.unwrap_or_default()))
 		} else {
 			let alias: &RoomAliasId = room.try_into().expect("valid RoomAliasId");
-			self.resolve_alias(alias, servers).await
+			self.resolve_alias(alias).await
 		}
 	}
 
+	/// Resolves the given room alias, returning the resolved room ID and any
+	/// servers that might be in the room.
 	#[tracing::instrument(skip(self), name = "resolve")]
 	pub async fn resolve_alias(
 		&self,
 		room_alias: &RoomAliasId,
-		servers: Option<Vec<OwnedServerName>>,
 	) -> Result<(OwnedRoomId, Vec<OwnedServerName>)> {
-		let server_name = room_alias.server_name();
+		let server_is_ours = self
-		let server_is_ours = self.services.globals.server_is_ours(server_name);
+			.services
-		let servers_contains_ours = || {
+			.globals
-			servers
+			.server_is_ours(room_alias.server_name());
-				.as_ref()
-				.is_some_and(|servers| servers.contains(&self.services.server.name))
-		};
 
-		if !server_is_ours && !servers_contains_ours() {
+		if !server_is_ours {
-			return self
+			// TODO: The spec advises servers may cache remote room aliases temporarily.
-				.remote_resolve(room_alias, servers.unwrap_or_default())
+			// We might want to look at doing that.
-				.await;
+			return self.remote_resolve(room_alias).await;
 		}
 
 		let room_id = match self.resolve_local_alias(room_alias).await {
@@ -163,10 +172,18 @@ impl Service {
 			| Err(_) => self.resolve_appservice_alias(room_alias).await?,
 		};
 
-		room_id.map_or_else(
+		if let Some(room_id) = room_id {
-			|| Err!(Request(NotFound("Room with alias not found."))),
+			let servers: Vec<OwnedServerName> = self
-			|room_id| Ok((room_id, Vec::new())),
+				.services
-		)
+				.state_cache
+				.room_servers(&room_id)
+				.map(ToOwned::to_owned)
+				.collect()
+				.await;
+			return Ok((room_id, servers));
+		}
+
+		Err!(Request(NotFound("Alias does not exist.")))
 	}
 
 	#[tracing::instrument(skip(self), level = "debug")]
@@ -206,12 +223,12 @@ impl Service {
 
 		// The creator of an alias can remove it
 		if self
-            .who_created_alias(alias).await
+			.who_created_alias(alias).await
-            .is_ok_and(|user| user == user_id)
+			.is_ok_and(|user| user == user_id)
-            // Server admins can remove any local alias
+			// Server admins can remove any local alias
-            || self.services.admin.user_is_admin(user_id).await
+			|| self.services.admin.user_is_admin(user_id).await
-            // Always allow the server service account to remove the alias, since there may not be an admin room
+			// Always allow the server service account to remove the alias, since there may not be an admin room
-            || server_user == user_id
+			|| server_user == user_id
 		{
 			return Ok(true);
 		}

src/service/rooms/alias/remote.rs

@@ -1,6 +1,4 @@
-use std::iter::once;
+use conduwuit::{Result, debug, error, implement};
-
-use conduwuit::{Result, debug, debug_error, err, implement};
 use federation::query::get_room_information::v1::Response;
 use ruma::{OwnedRoomId, OwnedServerName, RoomAliasId, ServerName, api::federation};
 
@@ -8,40 +6,21 @@ use ruma::{OwnedRoomId, OwnedServerName, RoomAliasId, ServerName, api::federatio
 pub(super) async fn remote_resolve(
 	&self,
 	room_alias: &RoomAliasId,
-	servers: Vec<OwnedServerName>,
 ) -> Result<(OwnedRoomId, Vec<OwnedServerName>)> {
-	debug!(?room_alias, servers = ?servers, "resolve");
+	debug!("Asking {} to resolve {room_alias:?}", room_alias.server_name());
-	let servers = once(room_alias.server_name())
+	match self
-		.map(ToOwned::to_owned)
+		.remote_request(room_alias, room_alias.server_name())
-		.chain(servers.into_iter());
+		.await
-
+	{
-	let mut resolved_servers = Vec::new();
+		| Err(e) => {
-	let mut resolved_room_id: Option<OwnedRoomId> = None;
+			error!("Unable to resolve remote room alias {}: {e}", room_alias);
-	for server in servers {
+			Err(e)
-		match self.remote_request(room_alias, &server).await {
+		},
-			| Err(e) => debug_error!("Failed to query for {room_alias:?} from {server}: {e}"),
+		| Ok(Response { room_id, servers }) => {
-			| Ok(Response { room_id, servers }) => {
+			debug!("Remote resolved {room_alias:?} to {room_id:?} with servers {servers:?}");
-				debug!(
+			Ok((room_id, servers))
-					"Server {server} answered with {room_id:?} for {room_alias:?} servers: \
+		},
-					 {servers:?}"
-				);
-
-				resolved_room_id.get_or_insert(room_id);
-				add_server(&mut resolved_servers, server);
-
-				if !servers.is_empty() {
-					add_servers(&mut resolved_servers, servers);
-					break;
-				}
-			},
-		}
 	}
-
-	resolved_room_id
-		.map(|room_id| (room_id, resolved_servers))
-		.ok_or_else(|| {
-			err!(Request(NotFound("No servers could assist in resolving the room alias")))
-		})
 }
 
 #[implement(super::Service)]
@@ -59,15 +38,3 @@ async fn remote_request(
 		.send_federation_request(server, request)
 		.await
 }
-
-fn add_servers(servers: &mut Vec<OwnedServerName>, new: Vec<OwnedServerName>) {
-	for server in new {
-		add_server(servers, server);
-	}
-}
-
-fn add_server(servers: &mut Vec<OwnedServerName>, server: OwnedServerName) {
-	if !servers.contains(&server) {
-		servers.push(server);
-	}
-}