diff --git a/book/changelog.md b/book/changelog.md index 5850e9ed..eeb9fdf6 100644 --- a/book/changelog.md +++ b/book/changelog.md @@ -311,3 +311,5 @@ This will be the first release of Grapevine since it was forked from Conduit ([!121](https://gitlab.computer.surgery/matrix/grapevine/-/merge_requests/121)) 25. Add configuration options to tune the value of each cache individually. ([!124](https://gitlab.computer.surgery/matrix/grapevine/-/merge_requests/124)) +26. Allow adding canonical aliases from remote servers. + ([!158](https://gitlab.computer.surgery/matrix/grapevine/-/merge_requests/158)) diff --git a/src/api/client_server/alias.rs b/src/api/client_server/alias.rs index c885d5e2..b2fbcf65 100644 --- a/src/api/client_server/alias.rs +++ b/src/api/client_server/alias.rs @@ -107,6 +107,8 @@ pub(crate) async fn get_alias_route( get_alias_helper(body.body.room_alias).await.map(Ra) } +// Can't use `services().rooms.alias.resolve_alias` because we also need the set +// of servers from the remote get_room_information request. pub(crate) async fn get_alias_helper( room_alias: OwnedRoomAliasId, ) -> Result { diff --git a/src/api/client_server/state.rs b/src/api/client_server/state.rs index cfa48ef0..2e6f3b13 100644 --- a/src/api/client_server/state.rs +++ b/src/api/client_server/state.rs @@ -212,7 +212,7 @@ async fn send_state_event_for_key_helper( let sender_user = sender; if event_type == &StateEventType::RoomCanonicalAlias { - validate_canonical_alias_event(room_id, json.cast_ref())?; + validate_canonical_alias_event(room_id, json.cast_ref()).await?; } let room_token = services() @@ -253,7 +253,7 @@ async fn send_state_event_for_key_helper( /// > are already present in the state event. /// /// [spec]: https://spec.matrix.org/v1.13/client-server-api/#put_matrixclientv3roomsroomidstateeventtypestatekey -fn validate_canonical_alias_event( +async fn validate_canonical_alias_event( room_id: &RoomId, json: &Raw, ) -> Result<()> { @@ -311,13 +311,11 @@ fn validate_canonical_alias_event( ) })?; - if alias.server_name() != services().globals.server_name() - || services() - .rooms - .alias - .resolve_local_alias(&alias)? - .filter(|room| room == room_id) - .is_none() + // The spec doesn't say explicitly that we should allow adding new + // remote canonical aliases, but it's reasonable behavior and what + // synapse does. + if services().rooms.alias.resolve_alias(&alias).await?.as_deref() + != Some(room_id) { return Err(Error::BadRequest( ErrorKind::BadAlias, diff --git a/src/service/rooms/alias.rs b/src/service/rooms/alias.rs index 0b934fe7..638d4d92 100644 --- a/src/service/rooms/alias.rs +++ b/src/service/rooms/alias.rs @@ -1,6 +1,7 @@ +use http::StatusCode; use ruma::{ - api::client::error::ErrorKind, OwnedRoomAliasId, OwnedRoomId, RoomAliasId, - RoomId, UserId, + api::{client::error::ErrorKind, federation}, + OwnedRoomAliasId, OwnedRoomId, RoomAliasId, RoomId, UserId, }; use crate::{services, Error, Result}; @@ -60,7 +61,7 @@ impl Service { self.db.remove_alias(alias) } - /// Looks up the roomid for the given alias. + /// Looks up the roomid for the given local alias. pub(crate) fn resolve_local_alias( &self, alias: &RoomAliasId, @@ -75,4 +76,46 @@ impl Service { ) -> Box> + 'a> { self.db.local_aliases_for_room(room_id) } + + /// Looks up the roomid for the given alias, fetching over federation if + /// remote. + pub(crate) async fn resolve_alias( + &self, + alias: &RoomAliasId, + ) -> Result> { + if alias.server_name() == services().globals.server_name() { + self.resolve_local_alias(alias) + } else { + self.resolve_remote_alias(alias).await + } + } + + /// Look up an alias on a remote server over federation. + async fn resolve_remote_alias( + &self, + alias: &RoomAliasId, + ) -> Result> { + let result = services() + .sending + .send_federation_request( + alias.server_name(), + federation::query::get_room_information::v1::Request { + room_alias: alias.to_owned(), + }, + ) + .await; + + match result { + Ok(response) => Ok(Some(response.room_id)), + // The spec only names the 404 status code explicitly, but matching + // on M_NOT_FOUND as well seems reasonable. + Err(Error::Federation(_, error)) + if error.status_code == StatusCode::NOT_FOUND + || error.error_kind() == Some(&ErrorKind::NotFound) => + { + Ok(None) + } + Err(error) => Err(error), + } + } }