diff --git a/Cargo.lock b/Cargo.lock index 1078bbbda..450916dd3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3116,6 +3116,7 @@ dependencies = [ name = "mas-axum-utils" version = "0.15.0" dependencies = [ + "anyhow", "axum", "axum-extra", "base64ct", diff --git a/crates/axum-utils/Cargo.toml b/crates/axum-utils/Cargo.toml index c3fa87b6d..a112beb28 100644 --- a/crates/axum-utils/Cargo.toml +++ b/crates/axum-utils/Cargo.toml @@ -12,6 +12,7 @@ publish = false workspace = true [dependencies] +anyhow.workspace = true axum.workspace = true axum-extra.workspace = true base64ct.workspace = true diff --git a/crates/axum-utils/src/error_wrapper.rs b/crates/axum-utils/src/error_wrapper.rs index 1315803fe..2bfd448fc 100644 --- a/crates/axum-utils/src/error_wrapper.rs +++ b/crates/axum-utils/src/error_wrapper.rs @@ -5,9 +5,8 @@ // Please see LICENSE in the repository root for full details. use axum::response::{IntoResponse, Response}; -use http::StatusCode; -use crate::record_error; +use crate::InternalError; /// A simple wrapper around an error that implements [`IntoResponse`]. #[derive(Debug, thiserror::Error)] @@ -19,13 +18,6 @@ where T: std::error::Error + 'static, { fn into_response(self) -> Response { - // TODO: make this a bit more user friendly - let sentry_event_id = record_error!(self.0); - ( - StatusCode::INTERNAL_SERVER_ERROR, - sentry_event_id, - self.0.to_string(), - ) - .into_response() + InternalError::from(self.0).into_response() } } diff --git a/crates/axum-utils/src/fancy_error.rs b/crates/axum-utils/src/fancy_error.rs index 19b760ff5..98c2a3c51 100644 --- a/crates/axum-utils/src/fancy_error.rs +++ b/crates/axum-utils/src/fancy_error.rs @@ -15,55 +15,91 @@ use mas_templates::ErrorContext; use crate::sentry::SentryEventID; -pub struct FancyError { - context: ErrorContext, +fn build_context(mut err: &dyn std::error::Error) -> ErrorContext { + let description = err.to_string(); + let mut details = Vec::new(); + while let Some(source) = err.source() { + err = source; + details.push(err.to_string()); + } + + ErrorContext::new() + .with_description(description) + .with_details(details.join("\n")) } -impl FancyError { - #[must_use] - pub fn new(context: ErrorContext) -> Self { - Self { context } +pub struct GenericError { + error: Box, + code: StatusCode, +} + +impl IntoResponse for GenericError { + fn into_response(self) -> Response { + tracing::warn!(message = &*self.error); + let context = build_context(&*self.error); + let context_text = format!("{context}"); + + ( + self.code, + TypedHeader(ContentType::text()), + Extension(context), + context_text, + ) + .into_response() } } -impl std::fmt::Display for FancyError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let code = self.context.code().unwrap_or("Internal error"); - match (self.context.description(), self.context.details()) { - (Some(description), Some(details)) => { - write!(f, "{code}: {description} ({details})") - } - (Some(message), None) | (None, Some(message)) => { - write!(f, "{code}: {message}") - } - (None, None) => { - write!(f, "{code}") - } +impl GenericError { + pub fn new(code: StatusCode, err: impl std::error::Error + 'static) -> Self { + Self { + error: Box::new(err), + code, } } } -impl From for FancyError { - fn from(err: E) -> Self { - let context = ErrorContext::new() - .with_description(format!("{err}")) - .with_details(format!("{err:?}")); - FancyError { context } - } +pub struct InternalError { + error: Box, } -impl IntoResponse for FancyError { +impl IntoResponse for InternalError { fn into_response(self) -> Response { - tracing::error!(message = %self.context); - let error = format!("{}", self.context); + tracing::error!(message = &*self.error); let event_id = SentryEventID::for_last_event(); + let context = build_context(&*self.error); + let context_text = format!("{context}"); + ( StatusCode::INTERNAL_SERVER_ERROR, TypedHeader(ContentType::text()), event_id, - Extension(self.context), - error, + Extension(context), + context_text, ) .into_response() } } + +impl From for InternalError { + fn from(err: E) -> Self { + Self { + error: Box::new(err), + } + } +} + +impl InternalError { + /// Create a new error from a boxed error + #[must_use] + pub fn new(error: Box) -> Self { + Self { error } + } + + /// Create a new error from an [`anyhow::Error`] + #[must_use] + pub fn from_anyhow(err: anyhow::Error) -> Self { + Self { + error: err.into_boxed_dyn_error(), + } + } +} diff --git a/crates/axum-utils/src/lib.rs b/crates/axum-utils/src/lib.rs index aa6fad9e6..a3dc31cca 100644 --- a/crates/axum-utils/src/lib.rs +++ b/crates/axum-utils/src/lib.rs @@ -22,6 +22,6 @@ pub use axum; pub use self::{ error_wrapper::ErrorWrapper, - fancy_error::FancyError, + fancy_error::{GenericError, InternalError}, session::{SessionInfo, SessionInfoExt}, }; diff --git a/crates/handlers/src/admin/mod.rs b/crates/handlers/src/admin/mod.rs index 4c1305b13..3b041ed10 100644 --- a/crates/handlers/src/admin/mod.rs +++ b/crates/handlers/src/admin/mod.rs @@ -19,7 +19,7 @@ use axum::{ }; use hyper::header::{ACCEPT, AUTHORIZATION, CONTENT_TYPE}; use indexmap::IndexMap; -use mas_axum_utils::FancyError; +use mas_axum_utils::InternalError; use mas_http::CorsLayerExt; use mas_matrix::HomeserverConnection; use mas_policy::PolicyFactory; @@ -180,7 +180,7 @@ where async fn swagger( State(url_builder): State, State(templates): State, -) -> Result, FancyError> { +) -> Result, InternalError> { let ctx = ApiDocContext::from_url_builder(&url_builder); let res = templates.render_swagger(&ctx)?; Ok(Html(res)) @@ -189,7 +189,7 @@ async fn swagger( async fn swagger_callback( State(url_builder): State, State(templates): State, -) -> Result, FancyError> { +) -> Result, InternalError> { let ctx = ApiDocContext::from_url_builder(&url_builder); let res = templates.render_swagger_callback(&ctx)?; Ok(Html(res)) diff --git a/crates/handlers/src/compat/login_sso_complete.rs b/crates/handlers/src/compat/login_sso_complete.rs index 1a5d1504c..15da5fb4d 100644 --- a/crates/handlers/src/compat/login_sso_complete.rs +++ b/crates/handlers/src/compat/login_sso_complete.rs @@ -13,7 +13,7 @@ use axum::{ }; use chrono::Duration; use mas_axum_utils::{ - FancyError, + InternalError, cookies::CookieJar, csrf::{CsrfExt, ProtectedForm}, }; @@ -59,7 +59,7 @@ pub async fn get( cookie_jar: CookieJar, Path(id): Path, Query(params): Query, -) -> Result { +) -> Result { let (cookie_jar, maybe_session) = match load_session_or_fallback( cookie_jar, &clock, &mut rng, &templates, &locale, &mut repo, ) @@ -93,7 +93,8 @@ pub async fn get( .compat_sso_login() .lookup(id) .await? - .context("Could not find compat SSO login")?; + .context("Could not find compat SSO login") + .map_err(InternalError::from_anyhow)?; // Bail out if that login session is more than 30min old if clock.now() > login.created_at + Duration::microseconds(30 * 60 * 1000 * 1000) { @@ -132,7 +133,7 @@ pub async fn post( Path(id): Path, Query(params): Query, Form(form): Form>, -) -> Result { +) -> Result { let (cookie_jar, maybe_session) = match load_session_or_fallback( cookie_jar, &clock, &mut rng, &templates, &locale, &mut repo, ) @@ -166,7 +167,8 @@ pub async fn post( .compat_sso_login() .lookup(id) .await? - .context("Could not find compat SSO login")?; + .context("Could not find compat SSO login") + .map_err(InternalError::from_anyhow)?; // Bail out if that login session isn't pending, or is more than 30min old if !login.is_pending() diff --git a/crates/handlers/src/graphql/mod.rs b/crates/handlers/src/graphql/mod.rs index 4c854ed21..9428abe7b 100644 --- a/crates/handlers/src/graphql/mod.rs +++ b/crates/handlers/src/graphql/mod.rs @@ -26,7 +26,7 @@ use futures_util::TryStreamExt; use headers::{Authorization, ContentType, HeaderValue, authorization::Bearer}; use hyper::header::CACHE_CONTROL; use mas_axum_utils::{ - FancyError, SessionInfo, SessionInfoExt, cookies::CookieJar, sentry::SentryEventID, + InternalError, SessionInfo, SessionInfoExt, cookies::CookieJar, sentry::SentryEventID, }; use mas_data_model::{BrowserSession, Session, SiteConfig, User}; use mas_matrix::HomeserverConnection; @@ -383,7 +383,7 @@ pub async fn get( authorization: Option>>, user_agent: Option>, RawQuery(query): RawQuery, -) -> Result { +) -> Result { let token = authorization .as_ref() .map(|TypedHeader(Authorization(bearer))| bearer.token()); diff --git a/crates/handlers/src/health.rs b/crates/handlers/src/health.rs index f8a2672d7..b1dfbf993 100644 --- a/crates/handlers/src/health.rs +++ b/crates/handlers/src/health.rs @@ -5,11 +5,11 @@ // Please see LICENSE in the repository root for full details. use axum::{extract::State, response::IntoResponse}; -use mas_axum_utils::FancyError; +use mas_axum_utils::InternalError; use sqlx::PgPool; use tracing::{Instrument, info_span}; -pub async fn get(State(pool): State) -> Result { +pub async fn get(State(pool): State) -> Result { let mut conn = pool.acquire().await?; sqlx::query("SELECT $1") diff --git a/crates/handlers/src/lib.rs b/crates/handlers/src/lib.rs index 4d610482f..a934de6a9 100644 --- a/crates/handlers/src/lib.rs +++ b/crates/handlers/src/lib.rs @@ -35,7 +35,7 @@ use hyper::{ ACCEPT, ACCEPT_LANGUAGE, AUTHORIZATION, CONTENT_LANGUAGE, CONTENT_LENGTH, CONTENT_TYPE, }, }; -use mas_axum_utils::{FancyError, cookies::CookieJar}; +use mas_axum_utils::{InternalError, cookies::CookieJar}; use mas_data_model::SiteConfig; use mas_http::CorsLayerExt; use mas_keystore::{Encrypter, Keystore}; @@ -437,16 +437,14 @@ where ) .layer(AndThenLayer::new( async move |response: axum::response::Response| { - if response.status().is_server_error() { - // Error responses should have an ErrorContext attached to them - let ext = response.extensions().get::(); - if let Some(ctx) = ext { - if let Ok(res) = templates.render_error(ctx) { - let (mut parts, _original_body) = response.into_parts(); - parts.headers.remove(CONTENT_TYPE); - parts.headers.remove(CONTENT_LENGTH); - return Ok((parts, Html(res)).into_response()); - } + // Error responses should have an ErrorContext attached to them + let ext = response.extensions().get::(); + if let Some(ctx) = ext { + if let Ok(res) = templates.render_error(ctx) { + let (mut parts, _original_body) = response.into_parts(); + parts.headers.remove(CONTENT_TYPE); + parts.headers.remove(CONTENT_LENGTH); + return Ok((parts, Html(res)).into_response()); } } @@ -466,7 +464,7 @@ pub async fn fallback( method: Method, version: Version, PreferredLanguage(locale): PreferredLanguage, -) -> Result { +) -> Result { let ctx = NotFoundContext::new(&method, version, &uri).with_language(locale); // XXX: this should look at the Accept header and return JSON if requested diff --git a/crates/handlers/src/oauth2/device/consent.rs b/crates/handlers/src/oauth2/device/consent.rs index b84a9c9d9..05e1d502d 100644 --- a/crates/handlers/src/oauth2/device/consent.rs +++ b/crates/handlers/src/oauth2/device/consent.rs @@ -12,7 +12,7 @@ use axum::{ }; use axum_extra::TypedHeader; use mas_axum_utils::{ - FancyError, + InternalError, cookies::CookieJar, csrf::{CsrfExt, ProtectedForm}, }; @@ -54,7 +54,7 @@ pub(crate) async fn get( user_agent: Option>, cookie_jar: CookieJar, Path(grant_id): Path, -) -> Result { +) -> Result { let (cookie_jar, maybe_session) = match load_session_or_fallback( cookie_jar, &clock, &mut rng, &templates, &locale, &mut repo, ) @@ -86,17 +86,21 @@ pub(crate) async fn get( .oauth2_device_code_grant() .lookup(grant_id) .await? - .context("Device grant not found")?; + .context("Device grant not found") + .map_err(InternalError::from_anyhow)?; if grant.expires_at < clock.now() { - return Err(FancyError::from(anyhow::anyhow!("Grant is expired"))); + return Err(InternalError::from_anyhow(anyhow::anyhow!( + "Grant is expired" + ))); } let client = repo .oauth2_client() .lookup(grant.client_id) .await? - .context("Client not found")?; + .context("Client not found") + .map_err(InternalError::from_anyhow)?; // Evaluate the policy let res = policy @@ -132,7 +136,8 @@ pub(crate) async fn get( let rendered = templates .render_device_consent(&ctx) - .context("Failed to render template")?; + .context("Failed to render template") + .map_err(InternalError::from_anyhow)?; Ok((cookie_jar, Html(rendered)).into_response()) } @@ -151,7 +156,7 @@ pub(crate) async fn post( cookie_jar: CookieJar, Path(grant_id): Path, Form(form): Form>, -) -> Result { +) -> Result { let form = cookie_jar.verify_form(&clock, form)?; let (cookie_jar, maybe_session) = match load_session_or_fallback( cookie_jar, &clock, &mut rng, &templates, &locale, &mut repo, @@ -183,17 +188,21 @@ pub(crate) async fn post( .oauth2_device_code_grant() .lookup(grant_id) .await? - .context("Device grant not found")?; + .context("Device grant not found") + .map_err(InternalError::from_anyhow)?; if grant.expires_at < clock.now() { - return Err(FancyError::from(anyhow::anyhow!("Grant is expired"))); + return Err(InternalError::from_anyhow(anyhow::anyhow!( + "Grant is expired" + ))); } let client = repo .oauth2_client() .lookup(grant.client_id) .await? - .context("Client not found")?; + .context("Client not found") + .map_err(InternalError::from_anyhow)?; // Evaluate the policy let res = policy @@ -256,7 +265,8 @@ pub(crate) async fn post( let rendered = templates .render_device_consent(&ctx) - .context("Failed to render template")?; + .context("Failed to render template") + .map_err(InternalError::from_anyhow)?; Ok((cookie_jar, Html(rendered)).into_response()) } diff --git a/crates/handlers/src/oauth2/device/link.rs b/crates/handlers/src/oauth2/device/link.rs index f123563ec..0e3c8bd2c 100644 --- a/crates/handlers/src/oauth2/device/link.rs +++ b/crates/handlers/src/oauth2/device/link.rs @@ -8,7 +8,7 @@ use axum::{ extract::{Query, State}, response::{Html, IntoResponse}, }; -use mas_axum_utils::{FancyError, cookies::CookieJar}; +use mas_axum_utils::{InternalError, cookies::CookieJar}; use mas_router::UrlBuilder; use mas_storage::{BoxClock, BoxRepository}; use mas_templates::{ @@ -33,7 +33,7 @@ pub(crate) async fn get( State(url_builder): State, cookie_jar: CookieJar, Query(query): Query, -) -> Result { +) -> Result { let mut form_state = FormState::from_form(&query); // If we have a code in query, find it in the database diff --git a/crates/handlers/src/passwords.rs b/crates/handlers/src/passwords.rs index a0e36f8e5..eba028661 100644 --- a/crates/handlers/src/passwords.rs +++ b/crates/handlers/src/passwords.rs @@ -96,7 +96,10 @@ impl PasswordManager { /// # Errors /// /// Returns an error if the password manager is disabled - pub fn is_password_complex_enough(&self, password: &str) -> Result { + pub fn is_password_complex_enough( + &self, + password: &str, + ) -> Result { let inner = self.get_inner()?; let score = zxcvbn(password, &[]); Ok(u8::from(score.score()) >= inner.minimum_complexity) diff --git a/crates/handlers/src/upstream_oauth2/link.rs b/crates/handlers/src/upstream_oauth2/link.rs index 730576b61..0648bcfb4 100644 --- a/crates/handlers/src/upstream_oauth2/link.rs +++ b/crates/handlers/src/upstream_oauth2/link.rs @@ -14,7 +14,7 @@ use axum::{ use axum_extra::typed_header::TypedHeader; use hyper::StatusCode; use mas_axum_utils::{ - FancyError, SessionInfoExt, + GenericError, SessionInfoExt, cookies::CookieJar, csrf::{CsrfExt, ProtectedForm}, record_error, @@ -138,12 +138,13 @@ impl IntoResponse for RouteError { | Self::UserNotFound(_) | Self::HomeserverConnection(_) ); - let response = match self { - Self::LinkNotFound => (StatusCode::NOT_FOUND, "Link not found").into_response(), - Self::Internal(e) => FancyError::from(e).into_response(), - e => FancyError::from(e).into_response(), + + let status_code = match self { + Self::LinkNotFound => StatusCode::NOT_FOUND, + _ => StatusCode::INTERNAL_SERVER_ERROR, }; + let response = GenericError::new(status_code, self); (sentry_event_id, response).into_response() } } diff --git a/crates/handlers/src/views/app.rs b/crates/handlers/src/views/app.rs index b09fc4263..47b657436 100644 --- a/crates/handlers/src/views/app.rs +++ b/crates/handlers/src/views/app.rs @@ -8,7 +8,7 @@ use axum::{ extract::{Query, State}, response::{Html, IntoResponse}, }; -use mas_axum_utils::{FancyError, cookies::CookieJar}; +use mas_axum_utils::{InternalError, cookies::CookieJar}; use mas_router::{PostAuthAction, UrlBuilder}; use mas_storage::{BoxClock, BoxRepository, BoxRng}; use mas_templates::{AppContext, TemplateContext, Templates}; @@ -36,7 +36,7 @@ pub async fn get( clock: BoxClock, mut rng: BoxRng, cookie_jar: CookieJar, -) -> Result { +) -> Result { let (cookie_jar, maybe_session) = match load_session_or_fallback( cookie_jar, &clock, &mut rng, &templates, &locale, &mut repo, ) @@ -79,7 +79,7 @@ pub async fn get_anonymous( PreferredLanguage(locale): PreferredLanguage, State(templates): State, State(url_builder): State, -) -> Result { +) -> Result { let ctx = AppContext::from_url_builder(&url_builder).with_language(locale); let content = templates.render_app(&ctx)?; diff --git a/crates/handlers/src/views/index.rs b/crates/handlers/src/views/index.rs index e2fb7c194..c05f4e307 100644 --- a/crates/handlers/src/views/index.rs +++ b/crates/handlers/src/views/index.rs @@ -8,7 +8,7 @@ use axum::{ extract::State, response::{Html, IntoResponse, Response}, }; -use mas_axum_utils::{FancyError, cookies::CookieJar, csrf::CsrfExt}; +use mas_axum_utils::{InternalError, cookies::CookieJar, csrf::CsrfExt}; use mas_router::UrlBuilder; use mas_storage::{BoxClock, BoxRepository, BoxRng}; use mas_templates::{IndexContext, TemplateContext, Templates}; @@ -29,7 +29,7 @@ pub async fn get( mut repo: BoxRepository, cookie_jar: CookieJar, PreferredLanguage(locale): PreferredLanguage, -) -> Result { +) -> Result { let (cookie_jar, maybe_session) = match load_session_or_fallback( cookie_jar, &clock, &mut rng, &templates, &locale, &mut repo, ) diff --git a/crates/handlers/src/views/login.rs b/crates/handlers/src/views/login.rs index e22a077d0..bef1faf02 100644 --- a/crates/handlers/src/views/login.rs +++ b/crates/handlers/src/views/login.rs @@ -13,7 +13,7 @@ use axum::{ use axum_extra::typed_header::TypedHeader; use hyper::StatusCode; use mas_axum_utils::{ - FancyError, SessionInfoExt, + InternalError, SessionInfoExt, cookies::CookieJar, csrf::{CsrfExt, ProtectedForm}, }; @@ -74,7 +74,7 @@ pub(crate) async fn get( activity_tracker: BoundActivityTracker, Query(query): Query, cookie_jar: CookieJar, -) -> Result { +) -> Result { let (cookie_jar, maybe_session) = match load_session_or_fallback( cookie_jar, &clock, &mut rng, &templates, &locale, &mut repo, ) @@ -145,7 +145,7 @@ pub(crate) async fn post( cookie_jar: CookieJar, user_agent: Option>, Form(form): Form>, -) -> Result { +) -> Result { let user_agent = user_agent.map(|ua| ua.as_str().to_owned()); if !site_config.password_login_enabled { // XXX: is it necessary to have better errors here? @@ -338,11 +338,11 @@ pub(crate) async fn post( Ok((cookie_jar, reply).into_response()) } -async fn get_user_by_email_or_by_username( +async fn get_user_by_email_or_by_username( site_config: SiteConfig, - repo: &mut impl RepositoryAccess, + repo: &mut R, username_or_email: &str, -) -> Result, Box> { +) -> Result, R::Error> { if site_config.login_with_email_allowed && username_or_email.contains('@') { let maybe_user_email = repo.user_email().find_by_email(username_or_email).await?; @@ -393,7 +393,7 @@ async fn render( rng: impl Rng, templates: &Templates, homeserver: &dyn HomeserverConnection, -) -> Result { +) -> Result { let (csrf_token, cookie_jar) = cookie_jar.csrf_token(clock, rng); let providers = repo.upstream_oauth_provider().all_enabled().await?; @@ -401,7 +401,10 @@ async fn render( .with_form_state(form_state) .with_upstream_providers(providers); - let next = action.load_context(repo).await?; + let next = action + .load_context(repo) + .await + .map_err(InternalError::from_anyhow)?; let ctx = if let Some(next) = next { let ctx = handle_login_hint(ctx, &next, homeserver); ctx.with_post_action(next) diff --git a/crates/handlers/src/views/logout.rs b/crates/handlers/src/views/logout.rs index c315d7873..66bd28311 100644 --- a/crates/handlers/src/views/logout.rs +++ b/crates/handlers/src/views/logout.rs @@ -9,7 +9,7 @@ use axum::{ response::IntoResponse, }; use mas_axum_utils::{ - FancyError, SessionInfoExt, + InternalError, SessionInfoExt, cookies::CookieJar, csrf::{CsrfExt, ProtectedForm}, }; @@ -26,7 +26,7 @@ pub(crate) async fn post( State(url_builder): State, activity_tracker: BoundActivityTracker, Form(form): Form>>, -) -> Result { +) -> Result { let form = cookie_jar.verify_form(&clock, form)?; let (session_info, cookie_jar) = cookie_jar.session_info(); diff --git a/crates/handlers/src/views/recovery/progress.rs b/crates/handlers/src/views/recovery/progress.rs index eaabef134..ea56e6cb1 100644 --- a/crates/handlers/src/views/recovery/progress.rs +++ b/crates/handlers/src/views/recovery/progress.rs @@ -11,7 +11,7 @@ use axum::{ }; use hyper::StatusCode; use mas_axum_utils::{ - FancyError, SessionInfoExt, + InternalError, SessionInfoExt, cookies::CookieJar, csrf::{CsrfExt, ProtectedForm}, }; @@ -36,7 +36,7 @@ pub(crate) async fn get( PreferredLanguage(locale): PreferredLanguage, cookie_jar: CookieJar, Path(id): Path, -) -> Result { +) -> Result { if !site_config.account_recovery_allowed { let context = EmptyContext.with_language(locale); let rendered = templates.render_recovery_disabled(&context)?; @@ -90,7 +90,7 @@ pub(crate) async fn post( cookie_jar: CookieJar, Path(id): Path, Form(form): Form>, -) -> Result { +) -> Result { if !site_config.account_recovery_allowed { let context = EmptyContext.with_language(locale); let rendered = templates.render_recovery_disabled(&context)?; diff --git a/crates/handlers/src/views/recovery/start.rs b/crates/handlers/src/views/recovery/start.rs index ad87bdd17..72d0bc666 100644 --- a/crates/handlers/src/views/recovery/start.rs +++ b/crates/handlers/src/views/recovery/start.rs @@ -14,7 +14,7 @@ use axum::{ use axum_extra::typed_header::TypedHeader; use lettre::Address; use mas_axum_utils::{ - FancyError, SessionInfoExt, + InternalError, SessionInfoExt, cookies::CookieJar, csrf::{CsrfExt, ProtectedForm}, }; @@ -46,7 +46,7 @@ pub(crate) async fn get( State(url_builder): State, PreferredLanguage(locale): PreferredLanguage, cookie_jar: CookieJar, -) -> Result { +) -> Result { if !site_config.account_recovery_allowed { let context = EmptyContext.with_language(locale); let rendered = templates.render_recovery_disabled(&context)?; @@ -86,7 +86,7 @@ pub(crate) async fn post( PreferredLanguage(locale): PreferredLanguage, cookie_jar: CookieJar, Form(form): Form>, -) -> Result { +) -> Result { if !site_config.account_recovery_allowed { let context = EmptyContext.with_language(locale); let rendered = templates.render_recovery_disabled(&context)?; diff --git a/crates/handlers/src/views/register/mod.rs b/crates/handlers/src/views/register/mod.rs index 050f5d7de..3afe24573 100644 --- a/crates/handlers/src/views/register/mod.rs +++ b/crates/handlers/src/views/register/mod.rs @@ -7,7 +7,7 @@ use axum::{ extract::{Query, State}, response::{Html, IntoResponse, Response}, }; -use mas_axum_utils::{FancyError, SessionInfoExt, cookies::CookieJar, csrf::CsrfExt as _}; +use mas_axum_utils::{InternalError, SessionInfoExt, cookies::CookieJar, csrf::CsrfExt as _}; use mas_data_model::SiteConfig; use mas_router::{PasswordRegister, UpstreamOAuth2Authorize, UrlBuilder}; use mas_storage::{BoxClock, BoxRepository, BoxRng}; @@ -32,7 +32,7 @@ pub(crate) async fn get( activity_tracker: BoundActivityTracker, Query(query): Query, cookie_jar: CookieJar, -) -> Result { +) -> Result { let (csrf_token, cookie_jar) = cookie_jar.csrf_token(&clock, &mut rng); let (session_info, cookie_jar) = cookie_jar.session_info(); @@ -76,7 +76,10 @@ pub(crate) async fn get( } let mut ctx = RegisterContext::new(providers); - let post_action = query.load_context(&mut repo).await?; + let post_action = query + .load_context(&mut repo) + .await + .map_err(InternalError::from_anyhow)?; if let Some(action) = post_action { ctx = ctx.with_post_action(action); } diff --git a/crates/handlers/src/views/register/password.rs b/crates/handlers/src/views/register/password.rs index 92058970a..8c2925505 100644 --- a/crates/handlers/src/views/register/password.rs +++ b/crates/handlers/src/views/register/password.rs @@ -14,7 +14,7 @@ use axum_extra::typed_header::TypedHeader; use hyper::StatusCode; use lettre::Address; use mas_axum_utils::{ - FancyError, SessionInfoExt, + InternalError, SessionInfoExt, cookies::CookieJar, csrf::{CsrfExt, CsrfToken, ProtectedForm}, }; @@ -77,7 +77,7 @@ pub(crate) async fn get( mut repo: BoxRepository, Query(query): Query, cookie_jar: CookieJar, -) -> Result { +) -> Result { let (csrf_token, cookie_jar) = cookie_jar.csrf_token(&clock, &mut rng); let (session_info, cookie_jar) = cookie_jar.session_info(); @@ -140,7 +140,7 @@ pub(crate) async fn post( Query(query): Query, cookie_jar: CookieJar, Form(form): Form>, -) -> Result { +) -> Result { let user_agent = user_agent.map(|ua| ua.as_str().to_owned()); let ip_address = activity_tracker.ip(); @@ -179,7 +179,11 @@ pub(crate) async fn post( } else if repo.user().exists(&form.username).await? { // The user already exists in the database state.add_error_on_field(RegisterFormField::Username, FieldError::Exists); - } else if !homeserver.is_localpart_available(&form.username).await? { + } else if !homeserver + .is_localpart_available(&form.username) + .await + .map_err(InternalError::from_anyhow)? + { // The user already exists on the homeserver tracing::warn!( username = &form.username, @@ -361,7 +365,10 @@ pub(crate) async fn post( // Hash the password let password = Zeroizing::new(form.password.into_bytes()); - let (version, hashed_password) = password_manager.hash(&mut rng, password).await?; + let (version, hashed_password) = password_manager + .hash(&mut rng, password) + .await + .map_err(InternalError::from_anyhow)?; // Add the password to the registration let registration = repo @@ -390,8 +397,11 @@ async fn render( repo: &mut impl RepositoryAccess, templates: &Templates, captcha_config: Option, -) -> Result { - let next = action.load_context(repo).await?; +) -> Result { + let next = action + .load_context(repo) + .await + .map_err(InternalError::from_anyhow)?; let ctx = if let Some(next) = next { ctx.with_post_action(next) } else { diff --git a/crates/handlers/src/views/register/steps/display_name.rs b/crates/handlers/src/views/register/steps/display_name.rs index 34d9d300f..fa029475a 100644 --- a/crates/handlers/src/views/register/steps/display_name.rs +++ b/crates/handlers/src/views/register/steps/display_name.rs @@ -10,7 +10,7 @@ use axum::{ response::{Html, IntoResponse, Response}, }; use mas_axum_utils::{ - FancyError, + InternalError, cookies::CookieJar, csrf::{CsrfExt as _, ProtectedForm}, }; @@ -59,14 +59,15 @@ pub(crate) async fn get( mut repo: BoxRepository, Path(id): Path, cookie_jar: CookieJar, -) -> Result { +) -> Result { let (csrf_token, cookie_jar) = cookie_jar.csrf_token(&clock, &mut rng); let registration = repo .user_registration() .lookup(id) .await? - .context("Could not find user registration")?; + .context("Could not find user registration") + .map_err(InternalError::from_anyhow)?; // If the registration is completed, we can go to the registration destination // XXX: this might not be the right thing to do? Maybe an error page would be @@ -110,12 +111,13 @@ pub(crate) async fn post( Path(id): Path, cookie_jar: CookieJar, Form(form): Form>, -) -> Result { +) -> Result { let registration = repo .user_registration() .lookup(id) .await? - .context("Could not find user registration")?; + .context("Could not find user registration") + .map_err(InternalError::from_anyhow)?; // If the registration is completed, we can go to the registration destination // XXX: this might not be the right thing to do? Maybe an error page would be diff --git a/crates/handlers/src/views/register/steps/finish.rs b/crates/handlers/src/views/register/steps/finish.rs index afc05b65b..fd0736b29 100644 --- a/crates/handlers/src/views/register/steps/finish.rs +++ b/crates/handlers/src/views/register/steps/finish.rs @@ -12,7 +12,7 @@ use axum::{ }; use axum_extra::TypedHeader; use chrono::Duration; -use mas_axum_utils::{FancyError, SessionInfoExt as _, cookies::CookieJar}; +use mas_axum_utils::{InternalError, SessionInfoExt as _, cookies::CookieJar}; use mas_matrix::HomeserverConnection; use mas_router::{PostAuthAction, UrlBuilder}; use mas_storage::{ @@ -54,13 +54,14 @@ pub(crate) async fn get( PreferredLanguage(lang): PreferredLanguage, cookie_jar: CookieJar, Path(id): Path, -) -> Result { +) -> Result { let user_agent = user_agent.map(|ua| ua.as_str().to_owned()); let registration = repo .user_registration() .lookup(id) .await? - .context("User registration not found")?; + .context("User registration not found") + .map_err(InternalError::from_anyhow)?; // If the registration is completed, we can go to the registration destination // XXX: this might not be the right thing to do? Maybe an error page would be @@ -81,7 +82,7 @@ pub(crate) async fn get( // Make sure the registration session hasn't expired // XXX: this duration is hard-coded, could be configurable if clock.now() - registration.created_at > Duration::hours(1) { - return Err(FancyError::from(anyhow::anyhow!( + return Err(InternalError::from_anyhow(anyhow::anyhow!( "Registration session has expired" ))); } @@ -90,7 +91,7 @@ pub(crate) async fn get( let registrations = UserRegistrationSessions::load(&cookie_jar); if !registrations.contains(®istration) { // XXX: we should have a better error screen here - return Err(FancyError::from(anyhow::anyhow!( + return Err(InternalError::from_anyhow(anyhow::anyhow!( "Could not find the registration in the browser cookies" ))); } @@ -102,16 +103,17 @@ pub(crate) async fn get( if repo.user().exists(®istration.username).await? { // XXX: this could have a better error message, but as this is unlikely to // happen, we're fine with a vague message for now - return Err(FancyError::from(anyhow::anyhow!( + return Err(InternalError::from_anyhow(anyhow::anyhow!( "Username is already taken" ))); } if !homeserver .is_localpart_available(®istration.username) - .await? + .await + .map_err(InternalError::from_anyhow)? { - return Err(FancyError::from(anyhow::anyhow!( + return Err(InternalError::from_anyhow(anyhow::anyhow!( "Username is not available" ))); } @@ -120,12 +122,14 @@ pub(crate) async fn get( // change in the future let email_authentication_id = registration .email_authentication_id - .context("No email authentication started for this registration")?; + .context("No email authentication started for this registration") + .map_err(InternalError::from_anyhow)?; let email_authentication = repo .user_email() .lookup_authentication(email_authentication_id) .await? - .context("Could not load the email authentication")?; + .context("Could not load the email authentication") + .map_err(InternalError::from_anyhow)?; // Check that the email authentication has been completed if email_authentication.completed_at.is_none() { diff --git a/crates/handlers/src/views/register/steps/verify_email.rs b/crates/handlers/src/views/register/steps/verify_email.rs index 20c3e5cb2..bd291d5f0 100644 --- a/crates/handlers/src/views/register/steps/verify_email.rs +++ b/crates/handlers/src/views/register/steps/verify_email.rs @@ -9,7 +9,7 @@ use axum::{ response::{Html, IntoResponse, Response}, }; use mas_axum_utils::{ - FancyError, + InternalError, cookies::CookieJar, csrf::{CsrfExt, ProtectedForm}, }; @@ -47,14 +47,15 @@ pub(crate) async fn get( mut repo: BoxRepository, Path(id): Path, cookie_jar: CookieJar, -) -> Result { +) -> Result { let (csrf_token, cookie_jar) = cookie_jar.csrf_token(&clock, &mut rng); let registration = repo .user_registration() .lookup(id) .await? - .context("Could not find user registration")?; + .context("Could not find user registration") + .map_err(InternalError::from_anyhow)?; // If the registration is completed, we can go to the registration destination // XXX: this might not be the right thing to do? Maybe an error page would be @@ -76,16 +77,18 @@ pub(crate) async fn get( let email_authentication_id = registration .email_authentication_id - .context("No email authentication started for this registration")?; + .context("No email authentication started for this registration") + .map_err(InternalError::from_anyhow)?; let email_authentication = repo .user_email() .lookup_authentication(email_authentication_id) .await? - .context("Could not find email authentication")?; + .context("Could not find email authentication") + .map_err(InternalError::from_anyhow)?; if email_authentication.completed_at.is_some() { // XXX: display a better error here - return Err(FancyError::from(anyhow::anyhow!( + return Err(InternalError::from_anyhow(anyhow::anyhow!( "Email authentication already completed" ))); } @@ -115,14 +118,15 @@ pub(crate) async fn post( State(url_builder): State, Path(id): Path, Form(form): Form>, -) -> Result { +) -> Result { let form = cookie_jar.verify_form(&clock, form)?; let registration = repo .user_registration() .lookup(id) .await? - .context("Could not find user registration")?; + .context("Could not find user registration") + .map_err(InternalError::from_anyhow)?; // If the registration is completed, we can go to the registration destination // XXX: this might not be the right thing to do? Maybe an error page would be @@ -142,16 +146,18 @@ pub(crate) async fn post( let email_authentication_id = registration .email_authentication_id - .context("No email authentication started for this registration")?; + .context("No email authentication started for this registration") + .map_err(InternalError::from_anyhow)?; let email_authentication = repo .user_email() .lookup_authentication(email_authentication_id) .await? - .context("Could not find email authentication")?; + .context("Could not find email authentication") + .map_err(InternalError::from_anyhow)?; if email_authentication.completed_at.is_some() { // XXX: display a better error here - return Err(FancyError::from(anyhow::anyhow!( + return Err(InternalError::from_anyhow(anyhow::anyhow!( "Email authentication already completed" ))); }