diff --git a/Cargo.lock b/Cargo.lock index c22225dc..02d9d074 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3384,6 +3384,7 @@ dependencies = [ "mas-http", "mas-matrix", "serde", + "serde_json", "tower", "tracing", "url", diff --git a/crates/axum-utils/src/language_detection.rs b/crates/axum-utils/src/language_detection.rs index f0e25092..802d4465 100644 --- a/crates/axum-utils/src/language_detection.rs +++ b/crates/axum-utils/src/language_detection.rs @@ -109,7 +109,7 @@ impl Header for AcceptLanguage { let quality = std::str::from_utf8(quality).map_err(|_e| Error::invalid())?; let quality = quality.parse::().map_err(|_e| Error::invalid())?; // Bound the quality between 0 and 1 - let quality = quality.min(1_f64).max(0_f64); + let quality = quality.clamp(0_f64, 1_f64); // Make sure the iterator is empty if it.next().is_some() { diff --git a/crates/http/src/ext.rs b/crates/http/src/ext.rs index d4bc8aa6..850f5902 100644 --- a/crates/http/src/ext.rs +++ b/crates/http/src/ext.rs @@ -14,7 +14,7 @@ use std::{ops::RangeBounds, sync::OnceLock}; -use http::{header::HeaderName, Request, StatusCode}; +use http::{header::HeaderName, Request, Response, StatusCode}; use tower::Service; use tower_http::cors::CorsLayer; @@ -70,6 +70,9 @@ pub trait ServiceExt: Sized { BytesToBodyRequest::new(self) } + /// Adds a layer which collects all the response body into a contiguous + /// byte buffer. + /// This makes the response type `Response`. fn response_body_to_bytes(self) -> BodyToBytesResponse { BodyToBytesResponse::new(self) } @@ -86,20 +89,40 @@ pub trait ServiceExt: Sized { FormUrlencodedRequest::new(self) } - fn catch_http_code(self, status_code: StatusCode, mapper: M) -> CatchHttpCodes + /// Catches responses with the given status code and then maps those + /// responses to an error type using the provided `mapper` function. + fn catch_http_code( + self, + status_code: StatusCode, + mapper: M, + ) -> CatchHttpCodes where - M: Clone, + M: Fn(Response) -> E + Send + Clone + 'static, { self.catch_http_codes(status_code..=status_code, mapper) } - fn catch_http_codes(self, bounds: B, mapper: M) -> CatchHttpCodes + /// Catches responses with the given status codes and then maps those + /// responses to an error type using the provided `mapper` function. + fn catch_http_codes(self, bounds: B, mapper: M) -> CatchHttpCodes where B: RangeBounds, - M: Clone, + M: Fn(Response) -> E + Send + Clone + 'static, { CatchHttpCodes::new(self, bounds, mapper) } + + /// Shorthand for [`Self::catch_http_codes`] which catches all client errors + /// (4xx) and server errors (5xx). + fn catch_http_errors(self, mapper: M) -> CatchHttpCodes + where + M: Fn(Response) -> E + Send + Clone + 'static, + { + self.catch_http_codes( + StatusCode::from_u16(400).unwrap()..StatusCode::from_u16(600).unwrap(), + mapper, + ) + } } impl ServiceExt for S where S: Service> {} diff --git a/crates/http/src/layers/catch_http_codes.rs b/crates/http/src/layers/catch_http_codes.rs index aaeb0a46..4e73984c 100644 --- a/crates/http/src/layers/catch_http_codes.rs +++ b/crates/http/src/layers/catch_http_codes.rs @@ -24,12 +24,8 @@ pub enum Error { #[error(transparent)] Service { inner: S }, - #[error("request failed with status {status_code}")] - HttpError { - status_code: StatusCode, - #[source] - inner: E, - }, + #[error("request failed with status {status_code}: {inner}")] + HttpError { status_code: StatusCode, inner: E }, } impl Error { @@ -45,10 +41,16 @@ impl Error { } } +/// A layer that catches responses with the HTTP status codes lying within +/// `bounds` and then maps the requests into a custom error type using `mapper`. #[derive(Clone)] pub struct CatchHttpCodes { + /// The inner service inner: S, + /// Which HTTP status codes to catch bounds: (Bound, Bound), + /// The function used to convert errors, which must be + /// `Fn(Response) -> E + Send + Clone + 'static`. mapper: M, } diff --git a/crates/http/src/layers/json_response.rs b/crates/http/src/layers/json_response.rs index 1dc90391..4813b8e5 100644 --- a/crates/http/src/layers/json_response.rs +++ b/crates/http/src/layers/json_response.rs @@ -23,6 +23,7 @@ use tower::{Layer, Service}; #[derive(Debug, Error)] pub enum Error { + /// An error from the inner service. #[error(transparent)] Service { inner: Service }, diff --git a/crates/matrix-synapse/Cargo.toml b/crates/matrix-synapse/Cargo.toml index 98852fb0..5339dae5 100644 --- a/crates/matrix-synapse/Cargo.toml +++ b/crates/matrix-synapse/Cargo.toml @@ -16,6 +16,7 @@ anyhow.workspace = true async-trait.workspace = true http.workspace = true serde.workspace = true +serde_json.workspace = true tower.workspace = true tracing.workspace = true url.workspace = true diff --git a/crates/matrix-synapse/src/error.rs b/crates/matrix-synapse/src/error.rs new file mode 100644 index 00000000..4926a8c9 --- /dev/null +++ b/crates/matrix-synapse/src/error.rs @@ -0,0 +1,64 @@ +use std::{error::Error, fmt::Display}; + +use http::Response; +use mas_axum_utils::axum::body::Bytes; +use serde::Deserialize; +use tracing::debug; + +/// Represents a Matrix error +/// Ref: +#[derive(Debug, Deserialize)] +struct MatrixError { + errcode: String, + error: String, +} + +/// Represents an error received from the homeserver. +/// Where possible, we capture the Matrix error from the JSON response body. +/// +/// Note that the `CatchHttpCodes` layer already captures the `StatusCode` for +/// us; we don't need to do that twice. +#[derive(Debug)] +pub(crate) struct HomeserverError { + matrix_error: Option, +} + +impl Display for HomeserverError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + if let Some(matrix_error) = &self.matrix_error { + write!(f, "{matrix_error}") + } else { + write!(f, "(no specific error)") + } + } +} + +impl Error for HomeserverError {} + +impl HomeserverError { + /// Return the error code (`errcode`) + pub fn errcode(&self) -> Option<&str> { + self.matrix_error.as_ref().map(|me| me.errcode.as_str()) + } +} + +/// Parses a JSON-encoded Matrix error from the response body +/// Spec reference: +#[allow(clippy::needless_pass_by_value)] +pub(crate) fn catch_homeserver_error(response: Response) -> HomeserverError { + let matrix_error: Option = match serde_json::from_slice(response.body().as_ref()) { + Ok(body) => Some(body), + Err(err) => { + debug!("failed to deserialise expected homeserver error: {err:?}"); + None + } + }; + HomeserverError { matrix_error } +} + +impl Display for MatrixError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let MatrixError { errcode, error } = &self; + write!(f, "{errcode}: {error}") + } +} diff --git a/crates/matrix-synapse/src/lib.rs b/crates/matrix-synapse/src/lib.rs index 3e77e63f..46be1b2e 100644 --- a/crates/matrix-synapse/src/lib.rs +++ b/crates/matrix-synapse/src/lib.rs @@ -14,16 +14,29 @@ #![allow(clippy::blocks_in_conditions)] +use anyhow::{bail, Context}; use http::{header::AUTHORIZATION, request::Builder, Method, Request, StatusCode}; use mas_axum_utils::http_client_factory::HttpClientFactory; -use mas_http::{EmptyBody, HttpServiceExt}; +use mas_http::{catch_http_codes, json_response, EmptyBody, HttpServiceExt}; use mas_matrix::{HomeserverConnection, MatrixUser, ProvisionRequest}; use serde::{Deserialize, Serialize}; use tower::{Service, ServiceExt}; +use tracing::debug; use url::Url; +use self::error::catch_homeserver_error; + static SYNAPSE_AUTH_PROVIDER: &str = "oauth-delegated"; +/// Encountered when trying to register a user ID which has been taken. +/// — +const M_USER_IN_USE: &str = "M_USER_IN_USE"; +/// Encountered when trying to register a user ID which is not valid. +/// — +const M_INVALID_USERNAME: &str = "M_INVALID_USERNAME"; + +mod error; + #[derive(Clone)] pub struct SynapseConnection { homeserver: String, @@ -136,6 +149,13 @@ struct SynapseDeactivateUserRequest { #[derive(Serialize)] struct SynapseAllowCrossSigningResetRequest {} +/// Response body of +/// `/_synapse/admin/v1/username_available?username={localpart}` +#[derive(Deserialize)] +struct UsernameAvailableResponse { + available: bool, +} + #[async_trait::async_trait] impl HomeserverConnection for SynapseConnection { type Error = anyhow::Error; @@ -151,7 +171,7 @@ impl HomeserverConnection for SynapseConnection { matrix.homeserver = self.homeserver, matrix.mxid = mxid, ), - err(Display), + err(Debug), )] async fn query_user(&self, mxid: &str) -> Result { let mxid = urlencoding::encode(mxid); @@ -159,13 +179,19 @@ impl HomeserverConnection for SynapseConnection { .http_client_factory .client("homeserver.query_user") .response_body_to_bytes() + .catch_http_errors(catch_homeserver_error) .json_response(); let request = self .get(&format!("_synapse/admin/v2/users/{mxid}")) .body(EmptyBody::new())?; - let response = client.ready().await?.call(request).await?; + let response = client + .ready() + .await? + .call(request) + .await + .context("Failed to query user from Synapse")?; if response.status() != StatusCode::OK { return Err(anyhow::anyhow!("Failed to query user from Synapse")); @@ -186,13 +212,16 @@ impl HomeserverConnection for SynapseConnection { matrix.homeserver = self.homeserver, matrix.localpart = localpart, ), - err(Display), + err(Debug), )] async fn is_localpart_available(&self, localpart: &str) -> Result { let localpart = urlencoding::encode(localpart); let mut client = self .http_client_factory - .client("homeserver.is_localpart_available"); + .client("homeserver.is_localpart_available") + .response_body_to_bytes() + .catch_http_errors(catch_homeserver_error) + .json_response::(); let request = self .get(&format!( @@ -200,14 +229,39 @@ impl HomeserverConnection for SynapseConnection { )) .body(EmptyBody::new())?; - let response = client.ready().await?.call(request).await?; + let response = client.ready().await?.call(request).await; - match response.status() { - StatusCode::OK => Ok(true), - StatusCode::BAD_REQUEST => Ok(false), - _ => Err(anyhow::anyhow!( - "Failed to query localpart availability from Synapse" - )), + match response { + Ok(resp) => { + if !resp.status().is_success() { + // We should have already handled 4xx and 5xx errors by this point + // so anything not 2xx is fairly weird + bail!( + "unexpected response from /username_available: {}", + resp.status() + ); + } + Ok(resp.into_body().available) + } + Err(err) => match err { + // Convoluted as... but we want to handle some of the 400 Bad Request responses + // ourselves + json_response::Error::Service { + inner: + catch_http_codes::Error::HttpError { + status_code: StatusCode::BAD_REQUEST, + inner: homeserver_error, + }, + } if homeserver_error.errcode() == Some(M_INVALID_USERNAME) + || homeserver_error.errcode() == Some(M_USER_IN_USE) => + { + debug!("Username not available: {homeserver_error}"); + Ok(false) + } + + other_err => Err(anyhow::Error::new(other_err) + .context("Failed to query localpart availability from Synapse")), + }, } } @@ -219,7 +273,7 @@ impl HomeserverConnection for SynapseConnection { matrix.mxid = request.mxid(), user.id = request.sub(), ), - err(Display), + err(Debug), )] async fn provision_user(&self, request: &ProvisionRequest) -> Result { let mut body = SynapseUser { @@ -254,14 +308,21 @@ impl HomeserverConnection for SynapseConnection { .http_client_factory .client("homeserver.provision_user") .request_bytes_to_body() - .json_request(); + .json_request() + .response_body_to_bytes() + .catch_http_errors(catch_homeserver_error); let mxid = urlencoding::encode(request.mxid()); let request = self .put(&format!("_synapse/admin/v2/users/{mxid}")) .body(body)?; - let response = client.ready().await?.call(request).await?; + let response = client + .ready() + .await? + .call(request) + .await + .context("Failed to provision user in Synapse")?; match response.status() { StatusCode::CREATED => Ok(true), @@ -281,7 +342,7 @@ impl HomeserverConnection for SynapseConnection { matrix.mxid = mxid, matrix.device_id = device_id, ), - err(Display), + err(Debug), )] async fn create_device(&self, mxid: &str, device_id: &str) -> Result<(), Self::Error> { let mxid = urlencoding::encode(mxid); @@ -289,13 +350,20 @@ impl HomeserverConnection for SynapseConnection { .http_client_factory .client("homeserver.create_device") .request_bytes_to_body() - .json_request(); + .json_request() + .response_body_to_bytes() + .catch_http_errors(catch_homeserver_error); let request = self .post(&format!("_synapse/admin/v2/users/{mxid}/devices")) .body(SynapseDevice { device_id })?; - let response = client.ready().await?.call(request).await?; + let response = client + .ready() + .await? + .call(request) + .await + .context("Failed to create device in Synapse")?; if response.status() != StatusCode::CREATED { return Err(anyhow::anyhow!("Failed to create device in Synapse")); @@ -312,12 +380,16 @@ impl HomeserverConnection for SynapseConnection { matrix.mxid = mxid, matrix.device_id = device_id, ), - err(Display), + err(Debug), )] async fn delete_device(&self, mxid: &str, device_id: &str) -> Result<(), Self::Error> { let mxid = urlencoding::encode(mxid); let device_id = urlencoding::encode(device_id); - let mut client = self.http_client_factory.client("homeserver.delete_device"); + let mut client = self + .http_client_factory + .client("homeserver.delete_device") + .response_body_to_bytes() + .catch_http_errors(catch_homeserver_error); let request = self .delete(&format!( @@ -325,7 +397,12 @@ impl HomeserverConnection for SynapseConnection { )) .body(EmptyBody::new())?; - let response = client.ready().await?.call(request).await?; + let response = client + .ready() + .await? + .call(request) + .await + .context("Failed to delete device in Synapse")?; if response.status() != StatusCode::OK { return Err(anyhow::anyhow!("Failed to delete device in Synapse")); @@ -342,7 +419,7 @@ impl HomeserverConnection for SynapseConnection { matrix.mxid = mxid, erase = erase, ), - err(Display), + err(Debug), )] async fn delete_user(&self, mxid: &str, erase: bool) -> Result<(), Self::Error> { let mxid = urlencoding::encode(mxid); @@ -350,13 +427,20 @@ impl HomeserverConnection for SynapseConnection { .http_client_factory .client("homeserver.delete_user") .request_bytes_to_body() - .json_request(); + .json_request() + .response_body_to_bytes() + .catch_http_errors(catch_homeserver_error); let request = self .post(&format!("_synapse/admin/v1/deactivate/{mxid}")) .body(SynapseDeactivateUserRequest { erase })?; - let response = client.ready().await?.call(request).await?; + let response = client + .ready() + .await? + .call(request) + .await + .context("Failed to delete user in Synapse")?; if response.status() != StatusCode::OK { return Err(anyhow::anyhow!("Failed to delete user in Synapse")); @@ -373,7 +457,7 @@ impl HomeserverConnection for SynapseConnection { matrix.mxid = mxid, matrix.displayname = displayname, ), - err(Display), + err(Debug), )] async fn set_displayname(&self, mxid: &str, displayname: &str) -> Result<(), Self::Error> { let mxid = urlencoding::encode(mxid); @@ -381,13 +465,20 @@ impl HomeserverConnection for SynapseConnection { .http_client_factory .client("homeserver.set_displayname") .request_bytes_to_body() - .json_request(); + .json_request() + .response_body_to_bytes() + .catch_http_errors(catch_homeserver_error); let request = self .put(&format!("_matrix/client/v3/profile/{mxid}/displayname")) .body(SetDisplayNameRequest { displayname })?; - let response = client.ready().await?.call(request).await?; + let response = client + .ready() + .await? + .call(request) + .await + .context("Failed to set displayname in Synapse")?; if response.status() != StatusCode::OK { return Err(anyhow::anyhow!("Failed to set displayname in Synapse")); @@ -416,7 +507,7 @@ impl HomeserverConnection for SynapseConnection { matrix.homeserver = self.homeserver, matrix.mxid = mxid, ), - err(Display), + err(Debug), )] async fn allow_cross_signing_reset(&self, mxid: &str) -> Result<(), Self::Error> { let mxid = urlencoding::encode(mxid); @@ -424,7 +515,9 @@ impl HomeserverConnection for SynapseConnection { .http_client_factory .client("homeserver.allow_cross_signing_reset") .request_bytes_to_body() - .json_request(); + .json_request() + .response_body_to_bytes() + .catch_http_errors(catch_homeserver_error); let request = self .post(&format!( @@ -432,11 +525,17 @@ impl HomeserverConnection for SynapseConnection { )) .body(SynapseAllowCrossSigningResetRequest {})?; - let response = client.ready().await?.call(request).await?; + let response = client + .ready() + .await? + .call(request) + .await + .context("Failed to allow cross-signing reset in Synapse")?; if response.status() != StatusCode::OK { return Err(anyhow::anyhow!( - "Failed to allow cross signing reset in Synapse" + "Failed to allow cross signing reset in Synapse: {}", + response.status() )); } diff --git a/templates/pages/error.html b/templates/pages/error.html index f56cccb5..c5fc0def 100644 --- a/templates/pages/error.html +++ b/templates/pages/error.html @@ -43,9 +43,8 @@ limitations under the License. {% if details %}
- 
-        {{ details }}
-
+ {# caution: do not introduce whitespace between 
 and  #}
+      
{{ details }}

     {% endif %}
   
 {% endblock %}