From d48523583b5ffa8c2b6020f3ca0a36cc40abeaa1 Mon Sep 17 00:00:00 2001 From: mdecimus Date: Thu, 18 Jul 2024 20:04:09 +0200 Subject: [PATCH] Improved error handling (part 4) --- crates/common/src/lib.rs | 2 - crates/directory/src/core/dispatch.rs | 12 +- crates/directory/src/core/secret.rs | 10 +- crates/imap-proto/src/protocol/mod.rs | 17 +- crates/imap-proto/src/receiver.rs | 21 +- crates/imap/src/core/client.rs | 380 ++++++++++-------- crates/imap/src/core/message.rs | 2 +- crates/imap/src/core/session.rs | 28 +- crates/imap/src/op/authenticate.rs | 79 ++-- crates/imap/src/op/copy_move.rs | 2 +- crates/imap/src/op/create.rs | 38 +- crates/imap/src/op/delete.rs | 32 +- crates/imap/src/op/fetch.rs | 6 +- crates/imap/src/op/noop.rs | 2 +- crates/imap/src/op/store.rs | 2 +- crates/jmap-proto/src/error/method.rs | 215 ++++------ crates/jmap-proto/src/error/request.rs | 24 +- crates/jmap-proto/src/method/get.rs | 5 +- crates/jmap-proto/src/method/set.rs | 9 +- crates/jmap-proto/src/object/blob.rs | 6 +- crates/jmap-proto/src/object/email.rs | 12 +- .../jmap-proto/src/object/email_submission.rs | 6 +- crates/jmap-proto/src/object/mailbox.rs | 12 +- crates/jmap-proto/src/object/sieve.rs | 6 +- crates/jmap-proto/src/request/mod.rs | 6 +- crates/jmap-proto/src/request/websocket.rs | 12 +- crates/jmap-proto/src/response/references.rs | 108 +++-- crates/jmap-proto/src/types/value.rs | 5 +- crates/jmap/src/api/http.rs | 122 +++++- crates/jmap/src/api/management/mod.rs | 27 +- crates/jmap/src/api/management/principal.rs | 51 +-- crates/jmap/src/api/request.rs | 19 +- crates/jmap/src/auth/mod.rs | 17 +- crates/jmap/src/auth/oauth/token.rs | 65 ++- crates/jmap/src/auth/rate_limit.rs | 2 - crates/jmap/src/blob/get.rs | 8 +- crates/jmap/src/blob/upload.rs | 4 +- crates/jmap/src/changes/get.rs | 3 +- crates/jmap/src/changes/query.rs | 17 +- crates/jmap/src/changes/state.rs | 7 +- crates/jmap/src/email/copy.rs | 9 +- crates/jmap/src/email/get.rs | 8 +- crates/jmap/src/email/parse.rs | 10 +- crates/jmap/src/email/query.rs | 30 +- crates/jmap/src/email/snippet.rs | 3 +- crates/jmap/src/lib.rs | 3 +- crates/jmap/src/mailbox/query.rs | 13 +- crates/jmap/src/principal/query.rs | 7 +- crates/jmap/src/push/get.rs | 6 +- crates/jmap/src/quota/query.rs | 4 +- crates/jmap/src/services/ingest.rs | 68 ++-- crates/jmap/src/services/state.rs | 6 +- crates/jmap/src/sieve/query.rs | 13 +- crates/jmap/src/submission/query.rs | 13 +- crates/jmap/src/vacation/set.rs | 12 +- crates/jmap/src/websocket/stream.rs | 7 +- crates/managesieve/src/core/client.rs | 19 +- crates/managesieve/src/core/mod.rs | 32 +- crates/managesieve/src/op/authenticate.rs | 60 ++- crates/pop3/src/client.rs | 93 +++-- crates/pop3/src/op/authenticate.rs | 64 ++- crates/pop3/src/protocol/response.rs | 4 +- crates/pop3/src/session.rs | 13 +- crates/store/src/dispatch/fts.rs | 8 +- crates/trc/src/imple.rs | 128 ++++++ crates/trc/src/lib.rs | 37 +- crates/trc/src/macros.rs | 4 +- tests/src/directory/smtp.rs | 8 +- tests/src/jmap/mod.rs | 2 +- tests/src/jmap/webhooks.rs | 2 +- 70 files changed, 1114 insertions(+), 973 deletions(-) diff --git a/crates/common/src/lib.rs b/crates/common/src/lib.rs index 26e901dc..626f6aa3 100644 --- a/crates/common/src/lib.rs +++ b/crates/common/src/lib.rs @@ -228,8 +228,6 @@ impl Core { protocol: ServerProtocol, return_member_of: bool, ) -> trc::Result> { - let c = "use trc::Error and implement OAUTH"; - // First try to authenticate the user against the default directory let result = match directory .query(QueryBy::Credentials(credentials), return_member_of) diff --git a/crates/directory/src/core/dispatch.rs b/crates/directory/src/core/dispatch.rs index dfc6d799..f907b32f 100644 --- a/crates/directory/src/core/dispatch.rs +++ b/crates/directory/src/core/dispatch.rs @@ -24,7 +24,7 @@ impl Directory { DirectoryInner::Smtp(store) => store.query(by).await, DirectoryInner::Memory(store) => store.query(by).await, } - .caused_by( trc::location!()) + .caused_by(trc::location!()) } pub async fn email_to_ids(&self, email: &str) -> trc::Result> { @@ -36,7 +36,7 @@ impl Directory { DirectoryInner::Smtp(store) => store.email_to_ids(email).await, DirectoryInner::Memory(store) => store.email_to_ids(email).await, } - .caused_by( trc::location!()) + .caused_by(trc::location!()) } pub async fn is_local_domain(&self, domain: &str) -> trc::Result { @@ -55,7 +55,7 @@ impl Directory { DirectoryInner::Smtp(store) => store.is_local_domain(domain).await, DirectoryInner::Memory(store) => store.is_local_domain(domain).await, } - .caused_by( trc::location!())?; + .caused_by(trc::location!())?; // Update cache if let Some(cache) = &self.cache { @@ -81,7 +81,7 @@ impl Directory { DirectoryInner::Smtp(store) => store.rcpt(email).await, DirectoryInner::Memory(store) => store.rcpt(email).await, } - .caused_by( trc::location!())?; + .caused_by(trc::location!())?; // Update cache if let Some(cache) = &self.cache { @@ -100,7 +100,7 @@ impl Directory { DirectoryInner::Smtp(store) => store.vrfy(address).await, DirectoryInner::Memory(store) => store.vrfy(address).await, } - .caused_by( trc::location!()) + .caused_by(trc::location!()) } pub async fn expn(&self, address: &str) -> trc::Result> { @@ -112,6 +112,6 @@ impl Directory { DirectoryInner::Smtp(store) => store.expn(address).await, DirectoryInner::Memory(store) => store.expn(address).await, } - .caused_by( trc::location!()) + .caused_by(trc::location!()) } } diff --git a/crates/directory/src/core/secret.rs b/crates/directory/src/core/secret.rs index 136f1657..f6b18955 100644 --- a/crates/directory/src/core/secret.rs +++ b/crates/directory/src/core/secret.rs @@ -59,7 +59,7 @@ impl Principal { // Token needs to validate with at least one of the TOTP secrets is_totp_verified = TOTP::from_url(secret) .map_err(|err| { - trc::AuthCause::Invalid + trc::AuthCause::Error .reason(err) .details(secret.to_string()) })? @@ -128,7 +128,7 @@ async fn verify_hash_prefix(hashed_secret: &str, secret: &str) -> trc::Result { - tx.send(Err(trc::AuthCause::Invalid + tx.send(Err(trc::AuthCause::Error .reason(err) .details(hashed_secret))) .ok(); @@ -155,7 +155,7 @@ async fn verify_hash_prefix(hashed_secret: &str, secret: &str) -> trc::Result trc::Resul } } "PLAIN" | "plain" | "CLEAR" | "clear" => Ok(hashed_secret == secret), - _ => Err(trc::AuthCause::Invalid + _ => Err(trc::AuthCause::Error .ctx(trc::Key::Reason, "Unsupported algorithm") .details(hashed_secret.to_string())), } } else { - Err(trc::AuthCause::Invalid + Err(trc::AuthCause::Error .into_err() .details(hashed_secret.to_string())) } diff --git a/crates/imap-proto/src/protocol/mod.rs b/crates/imap-proto/src/protocol/mod.rs index 64b1e306..a3d60591 100644 --- a/crates/imap-proto/src/protocol/mod.rs +++ b/crates/imap-proto/src/protocol/mod.rs @@ -494,15 +494,26 @@ impl SerializeResponse for trc::Error { buf.push(b' '); buf.extend_from_slice(self.value_as_str(trc::Key::Type).unwrap_or("NO").as_bytes()); buf.push(b' '); - if let Some(code) = self.value_as_str(trc::Key::Code) { + if let Some(code) = self + .value_as_str(trc::Key::Code) + .or_else(|| match self.as_ref() { + trc::Cause::Store(trc::StoreCause::NotFound) => { + Some(ResponseCode::NonExistent.as_str()) + } + trc::Cause::Store(_) => Some(ResponseCode::ContactAdmin.as_str()), + trc::Cause::Limit(trc::LimitCause::Quota) => Some(ResponseCode::OverQuota.as_str()), + trc::Cause::Limit(_) => Some(ResponseCode::Limit.as_str()), + trc::Cause::Auth(_) => Some(ResponseCode::AuthenticationFailed.as_str()), + _ => None, + }) + { buf.push(b'['); buf.extend_from_slice(code.as_bytes()); buf.extend_from_slice(b"] "); } buf.extend_from_slice( self.value_as_str(trc::Key::Details) - .or_else(|| self.value_as_str(trc::Key::Reason)) - .unwrap_or("Internal server error") + .unwrap_or_else(|| self.as_ref().message()) .as_bytes(), ); buf.extend_from_slice(b"\r\n"); diff --git a/crates/imap-proto/src/receiver.rs b/crates/imap-proto/src/receiver.rs index ce245ce0..3d4d58b1 100644 --- a/crates/imap-proto/src/receiver.rs +++ b/crates/imap-proto/src/receiver.rs @@ -4,15 +4,15 @@ * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-SEL */ -use std::{borrow::Cow, fmt::Display}; +use std::fmt::Display; -use super::{ResponseCode, ResponseType, StatusResponse}; +use super::{ResponseCode, ResponseType}; #[derive(Debug, Clone)] pub enum Error { NeedsMoreData, NeedsLiteral { size: u32 }, - Error { response: StatusResponse }, + Error { response: trc::Error }, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -92,7 +92,7 @@ impl Receiver { } } - pub fn error_reset(&mut self, message: impl Into>) -> Error { + pub fn error_reset(&mut self, message: impl Into) -> Error { let request = std::mem::take(&mut self.request); let err = Error::err( if !request.tag.is_empty() { @@ -462,14 +462,13 @@ impl Display for Token { } impl Error { - pub fn err(tag: Option, message: impl Into>) -> Self { + pub fn err(tag: Option, message: impl Into) -> Self { Error::Error { - response: StatusResponse { - tag, - code: ResponseCode::Parse.into(), - message: message.into(), - rtype: ResponseType::Bad, - }, + response: trc::Cause::Imap + .ctx(trc::Key::Details, message) + .ctx_opt(trc::Key::Id, tag) + .ctx(trc::Key::Type, ResponseType::Bad) + .code(ResponseCode::Parse), } } } diff --git a/crates/imap/src/core/client.rs b/crates/imap/src/core/client.rs index 8a6828df..175dc986 100644 --- a/crates/imap/src/core/client.rs +++ b/crates/imap/src/core/client.rs @@ -6,17 +6,17 @@ use std::{iter::Peekable, sync::Arc, vec::IntoIter}; -use common::listener::{limiter::ConcurrencyLimiter, SessionStream}; +use common::listener::{limiter::ConcurrencyLimiter, SessionResult, SessionStream}; use imap_proto::{ receiver::{self, Request}, - Command, ResponseCode, StatusResponse, + Command, ResponseType, StatusResponse, }; use jmap::auth::rate_limit::ConcurrencyLimiters; use super::{SelectedMailbox, Session, SessionData, State}; impl Session { - pub async fn ingest(&mut self, bytes: &[u8]) -> trc::Result { + pub async fn ingest(&mut self, bytes: &[u8]) -> SessionResult { /*for line in String::from_utf8_lossy(bytes).split("\r\n") { let c = println!("{}", line); }*/ @@ -36,8 +36,10 @@ impl Session { Ok(request) => { requests.push(request); } - Err(response) => { - self.write_bytes(response.into_bytes()).await?; + Err(err) => { + if !self.write_error(err).await { + return SessionResult::Close; + } } }, Err(receiver::Error::NeedsMoreData) => { @@ -48,7 +50,9 @@ impl Session { break; } Err(receiver::Error::Error { response }) => { - self.write_bytes(response.into_bytes()).await?; + if !self.write_error(response).await { + return SessionResult::Close; + } break; } } @@ -56,135 +60,179 @@ impl Session { let mut requests = requests.into_iter().peekable(); while let Some(request) = requests.next() { - match request.command { - Command::List | Command::Lsub => { - self.handle_list(request).await?; - } - Command::Select | Command::Examine => { - self.handle_select(request).await?; - } - Command::Create => { - self.handle_create(group_requests(&mut requests, vec![request])) - .await?; - } - Command::Delete => { - self.handle_delete(group_requests(&mut requests, vec![request])) - .await?; - } - Command::Rename => { - self.handle_rename(request).await?; - } - Command::Status => { - self.handle_status(request).await?; - } - Command::Append => { - self.handle_append(request).await?; - } - Command::Close => { - self.handle_close(request).await?; - } - Command::Unselect => { - self.handle_unselect(request).await?; - } - Command::Expunge(is_uid) => { - self.handle_expunge(request, is_uid).await?; - } - Command::Search(is_uid) => { - self.handle_search(request, false, is_uid).await?; - } - Command::Fetch(is_uid) => { - self.handle_fetch(request, is_uid).await?; - } - Command::Store(is_uid) => { - self.handle_store(request, is_uid).await?; - } - Command::Copy(is_uid) => { - self.handle_copy_move(request, false, is_uid).await?; - } - Command::Move(is_uid) => { - self.handle_copy_move(request, true, is_uid).await?; - } - Command::Sort(is_uid) => { - self.handle_search(request, true, is_uid).await?; - } - Command::Thread(is_uid) => { - self.handle_thread(request, is_uid).await?; - } - Command::Idle => { - self.handle_idle(request).await?; - } - Command::Subscribe => { - self.handle_subscribe(request, true).await?; - } - Command::Unsubscribe => { - self.handle_subscribe(request, false).await?; - } - Command::Namespace => { - self.handle_namespace(request).await?; - } - Command::Authenticate => { - self.handle_authenticate(request).await?; - } - Command::Login => { - self.handle_login(request).await?; - } - Command::Capability => { - self.handle_capability(request).await?; - } - Command::Enable => { - self.handle_enable(request).await?; - } - Command::StartTls => { - return self - .write_bytes( - StatusResponse::ok("Begin TLS negotiation now") - .with_tag(request.tag) - .into_bytes(), - ) - .await - .map(|_| true); - } - Command::Noop => { - self.handle_noop(request).await?; - } - Command::Check => { - self.handle_noop(request).await?; - } - Command::Logout => { - self.handle_logout(request).await?; - let todo = "disconnect"; - //return Err(()); - } - Command::SetAcl => { - self.handle_set_acl(request).await?; - } - Command::DeleteAcl => { - self.handle_set_acl(request).await?; - } - Command::GetAcl => { - self.handle_get_acl(request).await?; - } - Command::ListRights => { - self.handle_list_rights(request).await?; - } - Command::MyRights => { - self.handle_my_rights(request).await?; - } - Command::Unauthenticate => { - self.handle_unauthenticate(request).await?; - } - Command::Id => { - self.handle_id(request).await?; + let result = match request.command { + Command::List | Command::Lsub => self + .handle_list(request) + .await + .map(|_| SessionResult::Continue), + Command::Select | Command::Examine => self + .handle_select(request) + .await + .map(|_| SessionResult::Continue), + Command::Create => self + .handle_create(group_requests(&mut requests, vec![request])) + .await + .map(|_| SessionResult::Continue), + Command::Delete => self + .handle_delete(group_requests(&mut requests, vec![request])) + .await + .map(|_| SessionResult::Continue), + Command::Rename => self + .handle_rename(request) + .await + .map(|_| SessionResult::Continue), + Command::Status => self + .handle_status(request) + .await + .map(|_| SessionResult::Continue), + Command::Append => self + .handle_append(request) + .await + .map(|_| SessionResult::Continue), + Command::Close => self + .handle_close(request) + .await + .map(|_| SessionResult::Continue), + Command::Unselect => self + .handle_unselect(request) + .await + .map(|_| SessionResult::Continue), + Command::Expunge(is_uid) => self + .handle_expunge(request, is_uid) + .await + .map(|_| SessionResult::Continue), + Command::Search(is_uid) => self + .handle_search(request, false, is_uid) + .await + .map(|_| SessionResult::Continue), + Command::Fetch(is_uid) => self + .handle_fetch(request, is_uid) + .await + .map(|_| SessionResult::Continue), + Command::Store(is_uid) => self + .handle_store(request, is_uid) + .await + .map(|_| SessionResult::Continue), + Command::Copy(is_uid) => self + .handle_copy_move(request, false, is_uid) + .await + .map(|_| SessionResult::Continue), + Command::Move(is_uid) => self + .handle_copy_move(request, true, is_uid) + .await + .map(|_| SessionResult::Continue), + Command::Sort(is_uid) => self + .handle_search(request, true, is_uid) + .await + .map(|_| SessionResult::Continue), + Command::Thread(is_uid) => self + .handle_thread(request, is_uid) + .await + .map(|_| SessionResult::Continue), + Command::Idle => self + .handle_idle(request) + .await + .map(|_| SessionResult::Continue), + Command::Subscribe => self + .handle_subscribe(request, true) + .await + .map(|_| SessionResult::Continue), + Command::Unsubscribe => self + .handle_subscribe(request, false) + .await + .map(|_| SessionResult::Continue), + Command::Namespace => self + .handle_namespace(request) + .await + .map(|_| SessionResult::Continue), + Command::Authenticate => self + .handle_authenticate(request) + .await + .map(|_| SessionResult::Continue), + Command::Login => self + .handle_login(request) + .await + .map(|_| SessionResult::Continue), + Command::Capability => self + .handle_capability(request) + .await + .map(|_| SessionResult::Continue), + Command::Enable => self + .handle_enable(request) + .await + .map(|_| SessionResult::Continue), + Command::StartTls => self + .write_bytes( + StatusResponse::ok("Begin TLS negotiation now") + .with_tag(request.tag) + .into_bytes(), + ) + .await + .map(|_| SessionResult::UpgradeTls), + Command::Noop => self + .handle_noop(request) + .await + .map(|_| SessionResult::Continue), + Command::Check => self + .handle_noop(request) + .await + .map(|_| SessionResult::Continue), + Command::Logout => self + .handle_logout(request) + .await + .map(|_| SessionResult::Close), + Command::SetAcl => self + .handle_set_acl(request) + .await + .map(|_| SessionResult::Continue), + Command::DeleteAcl => self + .handle_set_acl(request) + .await + .map(|_| SessionResult::Continue), + Command::GetAcl => self + .handle_get_acl(request) + .await + .map(|_| SessionResult::Continue), + Command::ListRights => self + .handle_list_rights(request) + .await + .map(|_| SessionResult::Continue), + Command::MyRights => self + .handle_my_rights(request) + .await + .map(|_| SessionResult::Continue), + Command::Unauthenticate => self + .handle_unauthenticate(request) + .await + .map(|_| SessionResult::Continue), + Command::Id => self + .handle_id(request) + .await + .map(|_| SessionResult::Continue), + }; + + match result { + Ok(SessionResult::Continue) => (), + Ok(result) => return result, + Err(err) => { + if !self.write_error(err).await { + return SessionResult::Close; + } } } } if let Some(needs_literal) = needs_literal { - self.write_bytes(format!("+ Ready for {} bytes.\r\n", needs_literal).into_bytes()) - .await?; + if let Err(err) = self + .write_bytes(format!("+ Ready for {} bytes.\r\n", needs_literal).into_bytes()) + .await + { + self.write_error(err).await; + return SessionResult::Close; + } } - Ok(false) + SessionResult::Continue } } @@ -205,33 +253,21 @@ pub fn group_requests( } impl Session { - async fn is_allowed( - &self, - request: Request, - ) -> Result, StatusResponse> { + async fn is_allowed(&self, request: Request) -> trc::Result> { let state = &self.state; // Rate limit request if let State::Authenticated { data } | State::Selected { data, .. } = state { if let Some(rate) = &self.jmap.core.imap.rate_requests { - match data + if data .jmap .core .storage .lookup .is_rate_allowed(format!("ireq:{}", data.account_id).as_bytes(), rate, true) - .await + .await? + .is_some() { - Ok(None) => {} - Ok(Some(_)) => { - return Err(StatusResponse::no("Too many requests") - .with_tag(request.tag) - .with_code(ResponseCode::Limit)); - } - Err(_) => { - return Err(StatusResponse::no("Internal server error") - .with_tag(request.tag) - .with_code(ResponseCode::ContactAdmin)); - } + return Err(trc::LimitCause::TooManyRequests.into_err()); } } } @@ -243,17 +279,26 @@ impl Session { if self.instance.acceptor.is_tls() { Ok(request) } else { - Err(StatusResponse::no("TLS is not available.").with_tag(request.tag)) + Err(trc::Cause::Imap + .into_err() + .details("TLS is not available.") + .id(request.tag)) } } else { - Err(StatusResponse::no("Already in TLS mode.").with_tag(request.tag)) + Err(trc::Cause::Imap + .into_err() + .details("Already in TLS mode.") + .id(request.tag)) } } Command::Authenticate => { if let State::NotAuthenticated { .. } = state { Ok(request) } else { - Err(StatusResponse::no("Already authenticated.").with_tag(request.tag)) + Err(trc::Cause::Imap + .into_err() + .details("Already authenticated.") + .id(request.tag)) } } Command::Login => { @@ -261,13 +306,16 @@ impl Session { if self.is_tls || self.jmap.core.imap.allow_plain_auth { Ok(request) } else { - Err( - StatusResponse::no("LOGIN is disabled on the clear-text port.") - .with_tag(request.tag), - ) + Err(trc::Cause::Imap + .into_err() + .details("LOGIN is disabled on the clear-text port.") + .id(request.tag)) } } else { - Err(StatusResponse::no("Already authenticated.").with_tag(request.tag)) + Err(trc::Cause::Imap + .into_err() + .details("Already authenticated.") + .id(request.tag)) } } Command::Enable @@ -293,7 +341,10 @@ impl Session { if let State::Authenticated { .. } | State::Selected { .. } = state { Ok(request) } else { - Err(StatusResponse::no("Not authenticated.").with_tag(request.tag)) + Err(trc::Cause::Imap + .into_err() + .details("Not authenticated.") + .id(request.tag)) } } Command::Close @@ -316,16 +367,21 @@ impl Session { { Ok(request) } else { - Err(StatusResponse::no("Not permitted in EXAMINE state.") - .with_tag(request.tag)) + Err(trc::Cause::Imap + .into_err() + .details("Not permitted in EXAMINE state.") + .id(request.tag)) } } - State::Authenticated { .. } => { - Err(StatusResponse::bad("No mailbox is selected.").with_tag(request.tag)) - } - State::NotAuthenticated { .. } => { - Err(StatusResponse::no("Not authenticated.").with_tag(request.tag)) - } + State::Authenticated { .. } => Err(trc::Cause::Imap + .into_err() + .details("No mailbox is selected.") + .ctx(trc::Key::Type, ResponseType::Bad) + .id(request.tag)), + State::NotAuthenticated { .. } => Err(trc::Cause::Imap + .into_err() + .details("Not authenticated.") + .id(request.tag)), }, } } diff --git a/crates/imap/src/core/message.rs b/crates/imap/src/core/message.rs index 44081a65..b42592ba 100644 --- a/crates/imap/src/core/message.rs +++ b/crates/imap/src/core/message.rs @@ -197,7 +197,7 @@ impl SessionData { } } if !buf.is_empty() { - self.write_bytes(buf).await; + self.write_bytes(buf).await?; } Ok(modseq) diff --git a/crates/imap/src/core/session.rs b/crates/imap/src/core/session.rs index 9f9ebb28..ca13778f 100644 --- a/crates/imap/src/core/session.rs +++ b/crates/imap/src/core/session.rs @@ -6,7 +6,7 @@ use std::sync::Arc; -use common::listener::{stream::NullIo, SessionData, SessionManager, SessionStream}; +use common::listener::{stream::NullIo, SessionData, SessionManager, SessionResult, SessionStream}; use imap_proto::{ protocol::{ProtocolVersion, SerializeResponse}, receiver::Receiver, @@ -58,12 +58,11 @@ impl Session { Ok(Ok(bytes_read)) => { if bytes_read > 0 { match self.ingest(&buf[..bytes_read]).await { - Ok(false) => (), - Ok(true) => { + SessionResult::Continue => (), + SessionResult::UpgradeTls => { return true; } - Err(_) => { - tracing::debug!(parent: &self.span, event = "disconnect", "Disconnecting client."); + SessionResult::Close => { break; } } @@ -203,10 +202,21 @@ impl Session { } } - pub async fn write_error(&self, err: trc::Error) -> trc::Result<()> { - let todo = "log"; + pub async fn write_error(&self, err: trc::Error) -> bool { + tracing::warn!(parent: &self.span, event = "error", reason = %err, "IMAP error."); - self.write_bytes(err.serialize()).await + if !err.matches(trc::Cause::Network) { + let disconnect = err.must_disconnect(); + + if let Err(err) = self.write_bytes(err.serialize()).await { + tracing::debug!(parent: &self.span, event = "error", reason = %err, "Failed to write error."); + false + } else { + !disconnect + } + } else { + false + } } } @@ -235,7 +245,7 @@ impl super::SessionData { } pub async fn write_error(&self, err: trc::Error) -> trc::Result<()> { - let todo = "log"; + tracing::warn!(parent: &self.span, event = "error", reason = %err, "IMAP error."); if !err.matches(trc::Cause::Network) { self.write_bytes(err.serialize()).await diff --git a/crates/imap/src/op/authenticate.rs b/crates/imap/src/op/authenticate.rs index 9096146b..26a43468 100644 --- a/crates/imap/src/op/authenticate.rs +++ b/crates/imap/src/op/authenticate.rs @@ -69,24 +69,43 @@ impl Session { tag: String, ) -> trc::Result<()> { // Throttle authentication requests - self.jmap.is_auth_allowed_soft(&self.remote_addr).await?; + self.jmap + .is_auth_allowed_soft(&self.remote_addr) + .await + .map_err(|err| err.id(tag.clone()))?; // Authenticate let access_token = match credentials { Credentials::Plain { username, secret } | Credentials::XOauth2 { username, secret } => { self.jmap .authenticate_plain(&username, &secret, self.remote_addr, ServerProtocol::Imap) - .await? + .await } Credentials::OAuthBearer { token } => { - let (account_id, _, _) = self + match self .jmap .validate_access_token("access_token", &token) - .await?; - - self.jmap.get_access_token(account_id).await? + .await + { + Ok((account_id, _, _)) => self.jmap.get_access_token(account_id).await, + Err(err) => Err(err), + } } - }; + } + .map_err(|err| { + if err.matches(trc::Cause::Auth(trc::AuthCause::Failed)) { + let auth_failures = self.state.auth_failures(); + if auth_failures < self.jmap.core.imap.max_auth_failures { + self.state = State::NotAuthenticated { + auth_failures: auth_failures + 1, + }; + } else { + return trc::AuthCause::TooManyAttempts.into_err().caused_by(err); + } + } + + err.id(tag.clone()) + })?; // Enforce concurrency limits let in_flight = match self @@ -96,7 +115,9 @@ impl Session { Some(Some(limiter)) => Some(limiter), None => None, Some(None) => { - return Err(trc::LimitCause::ConcurrentRequest.into_err()); + return Err(trc::LimitCause::ConcurrentRequest + .into_err() + .id(tag.clone())); } }; @@ -105,9 +126,12 @@ impl Session { self.jmap.cache_access_token(access_token.clone()); // Create session - let todo = "handle auth errors"; self.state = State::Authenticated { - data: Arc::new(SessionData::new(self, &access_token, in_flight).await?), + data: Arc::new( + SessionData::new(self, &access_token, in_flight) + .await + .map_err(|err| err.id(tag.clone()))?, + ), }; self.write_bytes( StatusResponse::ok("Authentication successful") @@ -118,41 +142,6 @@ impl Session { .into_bytes(), ) .await - - /*if let Some(access_token) = access_token { - - } else { - self.write_bytes( - StatusResponse::no(if is_totp_error { - "Missing TOTP code, try with 'secret$totp_code'." - } else { - "Authentication failed." - }) - .with_tag(tag) - .with_code(ResponseCode::AuthenticationFailed) - .into_bytes(), - ) - .await?; - - let auth_failures = self.state.auth_failures(); - if auth_failures < self.jmap.core.imap.max_auth_failures { - self.state = State::NotAuthenticated { - auth_failures: auth_failures + 1, - }; - Ok(()) - } else { - self.write_bytes( - StatusResponse::bye("Too many authentication failures").into_bytes(), - ) - .await?; - tracing::debug!( - parent: &self.span, - event = "disconnect", - "Too many authentication failures, disconnecting.", - ); - Err(()) - } - }*/ } pub async fn handle_unauthenticate(&mut self, request: Request) -> trc::Result<()> { diff --git a/crates/imap/src/op/copy_move.rs b/crates/imap/src/op/copy_move.rs index a5d15ad3..8f51159a 100644 --- a/crates/imap/src/op/copy_move.rs +++ b/crates/imap/src/op/copy_move.rs @@ -363,7 +363,7 @@ impl SessionData { }) .into_bytes(), ) - .await; + .await?; if did_move { // Resynchronize source mailbox on a successful move diff --git a/crates/imap/src/op/create.rs b/crates/imap/src/op/create.rs index dd254d20..f026e567 100644 --- a/crates/imap/src/op/create.rs +++ b/crates/imap/src/op/create.rs @@ -28,27 +28,21 @@ use trc::AddContext; impl Session { pub async fn handle_create(&mut self, requests: Vec>) -> trc::Result<()> { - let mut arguments = Vec::with_capacity(requests.len()); - - for request in requests { - match request.parse_create(self.version) { - Ok(argument) => { - arguments.push(argument); - } - Err(err) => self.write_error(err).await?, - } - } - let data = self.state.session_data(); + let version = self.version; + spawn_op!(data, { - for argument in arguments { - match data.create_folder(argument).await { - Ok(response) => { - data.write_bytes(response.into_bytes()).await; - } - Err(error) => { - data.write_error(error).await; - } + for request in requests { + match request.parse_create(version) { + Ok(argument) => match data.create_folder(argument).await { + Ok(response) => { + data.write_bytes(response.into_bytes()).await?; + } + Err(error) => { + data.write_error(error).await?; + } + }, + Err(err) => data.write_error(err).await?, } } @@ -129,8 +123,10 @@ impl SessionData { .await; // Add created mailboxes to session - self.add_created_mailboxes(&mut params, change_id, create_ids) - .imap_ctx(&arguments.tag, trc::location!())?; + std::mem::drop( + self.add_created_mailboxes(&mut params, change_id, create_ids) + .imap_ctx(&arguments.tag, trc::location!())?, + ); // Build response Ok(StatusResponse::ok("Mailbox created.") diff --git a/crates/imap/src/op/delete.rs b/crates/imap/src/op/delete.rs index 7f3fc078..7d2f41e0 100644 --- a/crates/imap/src/op/delete.rs +++ b/crates/imap/src/op/delete.rs @@ -19,27 +19,21 @@ use super::ImapContext; impl Session { pub async fn handle_delete(&mut self, requests: Vec>) -> trc::Result<()> { - let mut arguments = Vec::with_capacity(requests.len()); - - for request in requests { - match request.parse_delete(self.version) { - Ok(argument) => { - arguments.push(argument); - } - Err(response) => self.write_error(response).await?, - } - } - let data = self.state.session_data(); + let version = self.version; + spawn_op!(data, { - for argument in arguments { - match data.delete_folder(argument).await { - Ok(response) => { - data.write_bytes(response.into_bytes()).await; - } - Err(error) => { - data.write_error(error).await; - } + for request in requests { + match request.parse_delete(version) { + Ok(argument) => match data.delete_folder(argument).await { + Ok(response) => { + data.write_bytes(response.into_bytes()).await?; + } + Err(error) => { + data.write_error(error).await?; + } + }, + Err(response) => data.write_error(response).await?, } } diff --git a/crates/imap/src/op/fetch.rs b/crates/imap/src/op/fetch.rs index 634b45fe..70603f7a 100644 --- a/crates/imap/src/op/fetch.rs +++ b/crates/imap/src/op/fetch.rs @@ -163,7 +163,7 @@ impl SessionData { ids: vanished, } .serialize(&mut buf); - self.write_bytes(buf).await; + self.write_bytes(buf).await?; } } @@ -176,7 +176,7 @@ impl SessionData { .with_code(ResponseCode::highest_modseq(modseq)) .into_bytes(), ) - .await; + .await?; } return Ok( StatusResponse::completed(Command::Fetch(is_uid)).with_tag(arguments.tag) @@ -545,7 +545,7 @@ impl SessionData { .with_code(ResponseCode::highest_modseq(modseq)) .into_bytes(), ) - .await; + .await?; } Ok(StatusResponse::completed(Command::Fetch(is_uid)).with_tag(arguments.tag)) diff --git a/crates/imap/src/op/noop.rs b/crates/imap/src/op/noop.rs index b7001410..536dce7b 100644 --- a/crates/imap/src/op/noop.rs +++ b/crates/imap/src/op/noop.rs @@ -18,7 +18,7 @@ impl Session { self.is_qresync, self.version.is_rev2(), ) - .await; + .await?; } self.write_bytes( diff --git a/crates/imap/src/op/store.rs b/crates/imap/src/op/store.rs index 608c2c5d..a07647fa 100644 --- a/crates/imap/src/op/store.rs +++ b/crates/imap/src/op/store.rs @@ -317,7 +317,7 @@ impl SessionData { .jmap .commit_changes(account_id, changelog) .await - .imap_ctx(&response.tag.as_ref().unwrap(), trc::location!())?; + .imap_ctx(response.tag.as_ref().unwrap(), trc::location!())?; self.jmap .broadcast_state_change(if !changed_mailboxes.is_empty() { StateChange::new(account_id) diff --git a/crates/jmap-proto/src/error/method.rs b/crates/jmap-proto/src/error/method.rs index b94c4789..18c1fd1e 100644 --- a/crates/jmap-proto/src/error/method.rs +++ b/crates/jmap-proto/src/error/method.rs @@ -34,116 +34,6 @@ pub enum MethodError { #[derive(Debug)] pub struct MethodErrorWrapper(trc::Error); -impl From for trc::Error { - fn from(value: MethodError) -> Self { - let (typ, description): (&'static str, trc::Value) = match value { - MethodError::InvalidArguments(description) => ("invalidArguments", description.into()), - MethodError::RequestTooLarge => ( - "requestTooLarge", - concat!( - "The number of ids requested by the client exceeds the maximum number ", - "the server is willing to process in a single method call." - ) - .into(), - ), - MethodError::StateMismatch => ( - "stateMismatch", - concat!( - "An \"ifInState\" argument was supplied, but ", - "it does not match the current state." - ) - .into(), - ), - MethodError::AnchorNotFound => ( - "anchorNotFound", - concat!( - "An anchor argument was supplied, but it ", - "cannot be found in the results of the query." - ) - .into(), - ), - MethodError::UnsupportedFilter(description) => { - ("unsupportedFilter", description.into()) - } - MethodError::UnsupportedSort(description) => ("unsupportedSort", description.into()), - MethodError::ServerFail(_) => ("serverFail", { - concat!( - "An unexpected error occurred while processing ", - "this call, please contact the system administrator." - ) - .into() - }), - MethodError::NotFound => ("serverPartialFail", { - concat!( - "One or more items are no longer available on the ", - "server, please try again." - ) - .into() - }), - MethodError::UnknownMethod(description) => ("unknownMethod", description.into()), - MethodError::ServerUnavailable => ( - "serverUnavailable", - concat!( - "This server is temporarily unavailable. ", - "Attempting this same operation later may succeed." - ) - .into(), - ), - MethodError::ServerPartialFail => ( - "serverPartialFail", - concat!( - "Some, but not all, expected changes described by the method ", - "occurred. Please resynchronize to determine server state." - ) - .into(), - ), - MethodError::InvalidResultReference(description) => { - ("invalidResultReference", description.into()) - } - MethodError::Forbidden(description) => ("forbidden", description.into()), - MethodError::AccountNotFound => ( - "accountNotFound", - "The accountId does not correspond to a valid account".into(), - ), - MethodError::AccountNotSupportedByMethod => ( - "accountNotSupportedByMethod", - concat!( - "The accountId given corresponds to a valid account, ", - "but the account does not support this method or data type." - ) - .into(), - ), - MethodError::AccountReadOnly => ( - "accountReadOnly", - "This method modifies state, but the account is read-only.".into(), - ), - MethodError::UnknownDataType => ( - "unknownDataType", - concat!( - "The server does not recognise this data type, ", - "or the capability to enable it is not present ", - "in the current Request Object." - ) - .into(), - ), - MethodError::CannotCalculateChanges => ( - "cannotCalculateChanges", - concat!( - "The server cannot calculate the changes ", - "between the old and new states." - ) - .into(), - ), - }; - - let todo = "fix"; - - trc::JmapCause::RequestTooLarge - .ctx(trc::Key::Type, typ) - .ctx(trc::Key::Details, description) - } -} - impl From for MethodErrorWrapper { fn from(value: trc::Error) -> Self { MethodErrorWrapper(value) @@ -186,29 +76,98 @@ impl Serialize for MethodErrorWrapper { { let mut map = serializer.serialize_map(2.into())?; - let todo = "fix"; - let (error_type, description) = if self - .0 - .matches(trc::Cause::Jmap(trc::JmapCause::RequestTooLarge)) - { - ( - self.0 - .value(trc::Key::Type) - .and_then(|v| v.as_str()) - .unwrap(), - self.0 - .value(trc::Key::Details) - .and_then(|v| v.as_str()) - .unwrap(), - ) - } else { - ( + let description = self.0.value(trc::Key::Details).and_then(|v| v.as_str()); + + let (error_type, description) = match self.0.as_ref() { + trc::Cause::Jmap(cause) => match cause { + trc::JmapCause::InvalidArguments => { + ("invalidArguments", description.unwrap_or_default()) + } + trc::JmapCause::RequestTooLarge => ( + "requestTooLarge", + concat!( + "The number of ids requested by the client exceeds the maximum number ", + "the server is willing to process in a single method call." + ), + ), + trc::JmapCause::StateMismatch => ( + "stateMismatch", + concat!( + "An \"ifInState\" argument was supplied, but ", + "it does not match the current state." + ), + ), + trc::JmapCause::AnchorNotFound => ( + "anchorNotFound", + concat!( + "An anchor argument was supplied, but it ", + "cannot be found in the results of the query." + ), + ), + trc::JmapCause::UnsupportedFilter => { + ("unsupportedFilter", description.unwrap_or_default()) + } + trc::JmapCause::UnsupportedSort => { + ("unsupportedSort", description.unwrap_or_default()) + } + trc::JmapCause::NotFound => ("serverPartialFail", { + concat!( + "One or more items are no longer available on the ", + "server, please try again." + ) + }), + trc::JmapCause::UnknownMethod => ("unknownMethod", description.unwrap_or_default()), + trc::JmapCause::InvalidResultReference => { + ("invalidResultReference", description.unwrap_or_default()) + } + trc::JmapCause::Forbidden => ("forbidden", description.unwrap_or_default()), + trc::JmapCause::AccountNotFound => ( + "accountNotFound", + "The accountId does not correspond to a valid account", + ), + trc::JmapCause::AccountNotSupportedByMethod => ( + "accountNotSupportedByMethod", + concat!( + "The accountId given corresponds to a valid account, ", + "but the account does not support this method or data type." + ), + ), + trc::JmapCause::AccountReadOnly => ( + "accountReadOnly", + "This method modifies state, but the account is read-only.", + ), + trc::JmapCause::UnknownDataType => ( + "unknownDataType", + concat!( + "The server does not recognise this data type, ", + "or the capability to enable it is not present ", + "in the current Request Object." + ), + ), + trc::JmapCause::CannotCalculateChanges => ( + "cannotCalculateChanges", + concat!( + "The server cannot calculate the changes ", + "between the old and new states." + ), + ), + trc::JmapCause::UnknownCapability + | trc::JmapCause::NotJSON + | trc::JmapCause::NotRequest => ( + "serverUnavailable", + concat!( + "This server is temporarily unavailable. ", + "Attempting this same operation later may succeed." + ), + ), + }, + _ => ( "serverUnavailable", concat!( "This server is temporarily unavailable. ", "Attempting this same operation later may succeed." ), - ) + ), }; map.serialize_entry("type", error_type)?; diff --git a/crates/jmap-proto/src/error/request.rs b/crates/jmap-proto/src/error/request.rs index 82e7a1f0..4ce38735 100644 --- a/crates/jmap-proto/src/error/request.rs +++ b/crates/jmap-proto/src/error/request.rs @@ -35,22 +35,22 @@ pub enum RequestErrorType { } #[derive(Debug, serde::Serialize, serde::Deserialize)] -pub struct RequestError { +pub struct RequestError<'x> { #[serde(rename = "type")] pub p_type: RequestErrorType, pub status: u16, #[serde(skip_serializing_if = "Option::is_none")] - pub title: Option>, - pub detail: Cow<'static, str>, + pub title: Option>, + pub detail: Cow<'x, str>, #[serde(skip_serializing_if = "Option::is_none")] pub limit: Option, } -impl RequestError { +impl<'x> RequestError<'x> { pub fn blank( status: u16, - title: impl Into>, - detail: impl Into>, + title: impl Into>, + detail: impl Into>, ) -> Self { RequestError { p_type: RequestErrorType::Other, @@ -110,6 +110,14 @@ impl RequestError { ) } + pub fn over_quota() -> Self { + RequestError::blank( + 403, + "Quota exceeded", + "You have exceeded your account quota.", + ) + } + pub fn too_many_requests() -> Self { RequestError::blank( 429, @@ -198,7 +206,7 @@ impl RequestError { } } - pub fn not_request(detail: impl Into>) -> RequestError { + pub fn not_request(detail: impl Into>) -> RequestError<'x> { RequestError { p_type: RequestErrorType::NotRequest, limit: None, @@ -209,7 +217,7 @@ impl RequestError { } } -impl Display for RequestError { +impl Display for RequestError<'_> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.write_str(&self.detail) } diff --git a/crates/jmap-proto/src/method/get.rs b/crates/jmap-proto/src/method/get.rs index 5a9c9905..c05a83b9 100644 --- a/crates/jmap-proto/src/method/get.rs +++ b/crates/jmap-proto/src/method/get.rs @@ -5,7 +5,6 @@ */ use crate::{ - error::method::MethodError, object::{blob, email, Object}, parser::{json::Parser, JsonObjectParser, Token}, request::{ @@ -176,7 +175,7 @@ impl GetRequest { .collect::>(), )) } else { - Err(MethodError::RequestTooLarge.into()) + Err(trc::JmapCause::RequestTooLarge.into_err()) } } else { Ok(None) @@ -196,7 +195,7 @@ impl GetRequest { .collect::>(), )) } else { - Err(MethodError::RequestTooLarge.into()) + Err(trc::JmapCause::RequestTooLarge.into_err()) } } else { Ok(None) diff --git a/crates/jmap-proto/src/method/set.rs b/crates/jmap-proto/src/method/set.rs index 4efa7bbd..3fa4b803 100644 --- a/crates/jmap-proto/src/method/set.rs +++ b/crates/jmap-proto/src/method/set.rs @@ -8,10 +8,7 @@ use ahash::AHashMap; use utils::map::{bitmap::Bitmap, vec_map::VecMap}; use crate::{ - error::{ - method::MethodError, - set::{InvalidProperty, SetError}, - }, + error::set::{InvalidProperty, SetError}, object::{email_submission, mailbox, sieve, Object}, parser::{json::Parser, JsonObjectParser, Token}, request::{ @@ -407,7 +404,7 @@ impl SetRequest { }) > max_objects_in_set { - Err(MethodError::RequestTooLarge.into()) + Err(trc::JmapCause::RequestTooLarge.into_err()) } else { Ok(()) } @@ -483,7 +480,7 @@ impl SetResponse { state_change: None, }) } else { - Err(MethodError::RequestTooLarge.into()) + Err(trc::JmapCause::RequestTooLarge.into_err()) } } diff --git a/crates/jmap-proto/src/object/blob.rs b/crates/jmap-proto/src/object/blob.rs index d38b0664..999f57cf 100644 --- a/crates/jmap-proto/src/object/blob.rs +++ b/crates/jmap-proto/src/object/blob.rs @@ -16,11 +16,7 @@ pub struct GetArguments { } impl RequestPropertyParser for GetArguments { - fn parse( - &mut self, - parser: &mut Parser, - property: RequestProperty, - ) -> trc::Result { + fn parse(&mut self, parser: &mut Parser, property: RequestProperty) -> trc::Result { match &property.hash[0] { 0x7465_7366_666f => { self.offset = parser diff --git a/crates/jmap-proto/src/object/email.rs b/crates/jmap-proto/src/object/email.rs index 28876fcd..66d02319 100644 --- a/crates/jmap-proto/src/object/email.rs +++ b/crates/jmap-proto/src/object/email.rs @@ -25,11 +25,7 @@ pub struct QueryArguments { } impl RequestPropertyParser for GetArguments { - fn parse( - &mut self, - parser: &mut Parser, - property: RequestProperty, - ) -> trc::Result { + fn parse(&mut self, parser: &mut Parser, property: RequestProperty) -> trc::Result { match (&property.hash[0], &property.hash[1]) { (0x7365_6974_7265_706f_7250_7964_6f62, _) => { self.body_properties = >>::parse(parser)?; @@ -62,11 +58,7 @@ impl RequestPropertyParser for GetArguments { } impl RequestPropertyParser for QueryArguments { - fn parse( - &mut self, - parser: &mut Parser, - property: RequestProperty, - ) -> trc::Result { + fn parse(&mut self, parser: &mut Parser, property: RequestProperty) -> trc::Result { if property.hash[0] == 0x0073_6461_6572_6854_6573_7061_6c6c_6f63 { self.collapse_threads = parser .next_token::()? diff --git a/crates/jmap-proto/src/object/email_submission.rs b/crates/jmap-proto/src/object/email_submission.rs index 2f429748..121feff1 100644 --- a/crates/jmap-proto/src/object/email_submission.rs +++ b/crates/jmap-proto/src/object/email_submission.rs @@ -21,11 +21,7 @@ pub struct SetArguments { } impl RequestPropertyParser for SetArguments { - fn parse( - &mut self, - parser: &mut Parser, - property: RequestProperty, - ) -> trc::Result { + fn parse(&mut self, parser: &mut Parser, property: RequestProperty) -> trc::Result { if property.hash[0] == 0x4565_7461_6470_5573_7365_6363_7553_6e6f && property.hash[1] == 0x6c69_616d { diff --git a/crates/jmap-proto/src/object/mailbox.rs b/crates/jmap-proto/src/object/mailbox.rs index fd34573a..5cc3225a 100644 --- a/crates/jmap-proto/src/object/mailbox.rs +++ b/crates/jmap-proto/src/object/mailbox.rs @@ -21,11 +21,7 @@ pub struct QueryArguments { } impl RequestPropertyParser for SetArguments { - fn parse( - &mut self, - parser: &mut Parser, - property: RequestProperty, - ) -> trc::Result { + fn parse(&mut self, parser: &mut Parser, property: RequestProperty) -> trc::Result { if property.hash[0] == 0x4565_766f_6d65_5279_6f72_7473_6544_6e6f && property.hash[1] == 0x0073_6c69_616d { @@ -40,11 +36,7 @@ impl RequestPropertyParser for SetArguments { } impl RequestPropertyParser for QueryArguments { - fn parse( - &mut self, - parser: &mut Parser, - property: RequestProperty, - ) -> trc::Result { + fn parse(&mut self, parser: &mut Parser, property: RequestProperty) -> trc::Result { match &property.hash[0] { 0x6565_7254_7341_7472_6f73 => { self.sort_as_tree = parser diff --git a/crates/jmap-proto/src/object/sieve.rs b/crates/jmap-proto/src/object/sieve.rs index bca31205..8d92774d 100644 --- a/crates/jmap-proto/src/object/sieve.rs +++ b/crates/jmap-proto/src/object/sieve.rs @@ -17,11 +17,7 @@ pub struct SetArguments { } impl RequestPropertyParser for SetArguments { - fn parse( - &mut self, - parser: &mut Parser, - property: RequestProperty, - ) -> trc::Result { + fn parse(&mut self, parser: &mut Parser, property: RequestProperty) -> trc::Result { if property.hash[0] == 0x7461_7669_7463_4173_7365_6363_7553_6e6f && property.hash[1] == 0x0074_7069_7263_5365 { diff --git a/crates/jmap-proto/src/request/mod.rs b/crates/jmap-proto/src/request/mod.rs index a223b528..6e12d42a 100644 --- a/crates/jmap-proto/src/request/mod.rs +++ b/crates/jmap-proto/src/request/mod.rs @@ -112,9 +112,5 @@ impl Display for RequestProperty { } pub trait RequestPropertyParser { - fn parse( - &mut self, - parser: &mut Parser, - property: RequestProperty, - ) -> trc::Result; + fn parse(&mut self, parser: &mut Parser, property: RequestProperty) -> trc::Result; } diff --git a/crates/jmap-proto/src/request/websocket.rs b/crates/jmap-proto/src/request/websocket.rs index b1fcbe4b..92c50a94 100644 --- a/crates/jmap-proto/src/request/websocket.rs +++ b/crates/jmap-proto/src/request/websocket.rs @@ -78,7 +78,7 @@ pub struct WebSocketStateChange { } #[derive(Debug, serde::Serialize)] -pub struct WebSocketRequestError { +pub struct WebSocketRequestError<'x> { #[serde(rename = "@type")] pub type_: WebSocketRequestErrorType, @@ -88,7 +88,7 @@ pub struct WebSocketRequestError { #[serde(skip_serializing_if = "Option::is_none")] limit: Option, status: u16, - detail: Cow<'static, str>, + detail: Cow<'x, str>, #[serde(rename = "requestId")] #[serde(skip_serializing_if = "Option::is_none")] @@ -180,8 +180,8 @@ impl WebSocketMessage { } } -impl WebSocketRequestError { - pub fn from_error(error: RequestError, request_id: Option) -> Self { +impl<'x> WebSocketRequestError<'x> { + pub fn from_error(error: RequestError<'x>, request_id: Option) -> Self { Self { type_: WebSocketRequestErrorType::RequestError, p_type: error.p_type, @@ -197,8 +197,8 @@ impl WebSocketRequestError { } } -impl From for WebSocketRequestError { - fn from(value: RequestError) -> Self { +impl<'x> From> for WebSocketRequestError<'x> { + fn from(value: RequestError<'x>) -> Self { Self::from_error(value, None) } } diff --git a/crates/jmap-proto/src/response/references.rs b/crates/jmap-proto/src/response/references.rs index 1222ecb8..7769b019 100644 --- a/crates/jmap-proto/src/response/references.rs +++ b/crates/jmap-proto/src/response/references.rs @@ -9,7 +9,7 @@ use std::collections::HashMap; use utils::map::vec_map::VecMap; use crate::{ - error::{method::MethodError, set::SetError}, + error::set::SetError, method::{copy::CopyResponse, set::SetResponse, upload::DataSourceObject}, object::Object, request::{ @@ -50,10 +50,11 @@ impl Response { if let Some(resolved_id) = self.created_ids.get(reference) { *id = MaybeReference::Value(resolved_id.clone()); } else { - return Err(MethodError::InvalidResultReference(format!( - "Id reference {reference:?} does not exist." - )) - .into()); + return Err(trc::JmapCause::InvalidResultReference + .into_err() + .details(format!( + "Id reference {reference:?} does not exist." + ))); } } } @@ -150,10 +151,11 @@ impl Response { *id = MaybeReference::Value(blob_id.clone()); } Some(_) => { - return Err(MethodError::InvalidResultReference(format!( + return Err(trc::JmapCause::InvalidResultReference + .into_err() + .details(format!( "Id reference {parent_id:?} points to invalid type." - )) - .into()); + ))); } None => { graph @@ -252,10 +254,9 @@ impl Response { if let Some(AnyId::Id(id)) = self.created_ids.get(ir) { Ok(*id) } else { - Err( - MethodError::InvalidResultReference(format!("Id reference {ir:?} not found.")) - .into(), - ) + Err(trc::JmapCause::InvalidResultReference + .into_err() + .details(format!("Id reference {ir:?} not found."))) } } @@ -275,10 +276,9 @@ impl Response { .or_insert_with(Vec::new) .push(parent_id.to_string()); } else { - return Err(MethodError::InvalidResultReference(format!( - "Id reference {parent_id:?} not found." - )) - .into()); + return Err(trc::JmapCause::InvalidResultReference + .into_err() + .details(format!("Id reference {parent_id:?} not found."))); } } SetValue::IdReferences(id_refs) => { @@ -292,10 +292,9 @@ impl Response { .or_insert_with(Vec::new) .push(parent_id.to_string()); } else { - return Err(MethodError::InvalidResultReference(format!( - "Id reference {parent_id:?} not found." - )) - .into()); + return Err(trc::JmapCause::InvalidResultReference + .into_err() + .details(format!("Id reference {parent_id:?} not found."))); } } } @@ -320,10 +319,11 @@ fn topological_sort( for (from_id, to_ids) in graph.iter() { for to_id in to_ids { if !create.contains_key(to_id) { - return Err(MethodError::InvalidResultReference(format!( - "Invalid reference to non-existing object {to_id:?} from {from_id:?}" - )) - .into()); + return Err(trc::JmapCause::InvalidResultReference + .into_err() + .details(format!( + "Invalid reference to non-existing object {to_id:?} from {from_id:?}" + ))); } } } @@ -338,10 +338,9 @@ fn topological_sort( if let Some(to_ids) = graph.get(from_id) { it_stack.push((it, from_id)); if it_stack.len() > 1000 { - return Err(MethodError::InvalidArguments( - "Cyclical references are not allowed.".to_string(), - ) - .into()); + return Err(trc::JmapCause::InvalidArguments + .into_err() + .details("Cyclical references are not allowed.".to_string())); } it = to_ids.iter(); continue; @@ -455,28 +454,27 @@ impl EvalResult { match value { Value::Id(id) => ids.push(id), _ => { - return Err(MethodError::InvalidResultReference(format!( - "Failed to evaluate {rr} result reference." - )) - .into()); + return Err(trc::JmapCause::InvalidResultReference + .into_err() + .details(format!( + "Failed to evaluate {rr} result reference." + ))); } } } } _ => { - return Err(MethodError::InvalidResultReference(format!( - "Failed to evaluate {rr} result reference." - )) - .into()) + return Err(trc::JmapCause::InvalidResultReference + .into_err() + .details(format!("Failed to evaluate {rr} result reference."))) } } } Ok(ids) } else { - Err(MethodError::InvalidResultReference(format!( - "Failed to evaluate {rr} result reference." - )) - .into()) + Err(trc::JmapCause::InvalidResultReference + .into_err() + .details(format!("Failed to evaluate {rr} result reference."))) } } @@ -498,28 +496,27 @@ impl EvalResult { ids.push(MaybeReference::Value(blob_id.into())) } _ => { - return Err(MethodError::InvalidResultReference(format!( - "Failed to evaluate {rr} result reference." - )) - .into()); + return Err(trc::JmapCause::InvalidResultReference + .into_err() + .details(format!( + "Failed to evaluate {rr} result reference." + ))); } } } } _ => { - return Err(MethodError::InvalidResultReference(format!( - "Failed to evaluate {rr} result reference." - )) - .into()) + return Err(trc::JmapCause::InvalidResultReference + .into_err() + .details(format!("Failed to evaluate {rr} result reference."))) } } } Ok(ids) } else { - Err(MethodError::InvalidResultReference(format!( - "Failed to evaluate {rr} result reference." - )) - .into()) + Err(trc::JmapCause::InvalidResultReference + .into_err() + .details(format!("Failed to evaluate {rr} result reference."))) } } @@ -527,10 +524,9 @@ impl EvalResult { if let EvalResult::Properties(properties) = self { Ok(properties) } else { - Err(MethodError::InvalidResultReference(format!( - "Failed to evaluate {rr} result reference." - )) - .into()) + Err(trc::JmapCause::InvalidResultReference + .into_err() + .details(format!("Failed to evaluate {rr} result reference."))) } } } diff --git a/crates/jmap-proto/src/types/value.rs b/crates/jmap-proto/src/types/value.rs index 65c83d88..80373df4 100644 --- a/crates/jmap-proto/src/types/value.rs +++ b/crates/jmap-proto/src/types/value.rs @@ -111,10 +111,7 @@ impl Value { }) } - pub fn from_property( - parser: &mut Parser<'_>, - property: &Property, - ) -> trc::Result { + pub fn from_property(parser: &mut Parser<'_>, property: &Property) -> trc::Result { match &property { Property::BlobId => Ok(parser .next_token::()? diff --git a/crates/jmap/src/api/http.rs b/crates/jmap/src/api/http.rs index de1c94dd..9a54406d 100644 --- a/crates/jmap/src/api/http.rs +++ b/crates/jmap/src/api/http.rs @@ -22,7 +22,7 @@ use hyper::{ }; use hyper_util::rt::TokioIo; use jmap_proto::{ - error::request::RequestError, + error::request::{RequestError, RequestLimitError}, request::{capability::Session, Request}, response::Response, types::{blob::BlobId, id::Id}, @@ -35,7 +35,10 @@ use crate::{ JmapInstance, JMAP, }; -use super::{HtmlResponse, HttpRequest, HttpResponse, JmapSessionManager, JsonResponse}; +use super::{ + management::ManagementApiError, HtmlResponse, HttpRequest, HttpResponse, JmapSessionManager, + JsonResponse, +}; pub struct HttpSessionData { pub instance: Arc, @@ -513,30 +516,103 @@ impl ToHttpResponse for JsonResponse { impl ToHttpResponse for trc::Error { fn into_http_response(self) -> HttpResponse { - tracing::error!(context = "store", error = %self, "Database error"); + match self.as_ref() { + trc::Cause::Manage(cause) => { + let details_or_reason = self + .value(trc::Key::Details) + .or_else(|| self.value(trc::Key::Reason)) + .and_then(|v| v.as_str()); - RequestError::internal_server_error().into_http_response() + match cause { + trc::ManageCause::MissingParameter => ManagementApiError::FieldMissing { + field: self.value_as_str(trc::Key::Key).unwrap_or_default(), + }, + trc::ManageCause::AlreadyExists => ManagementApiError::FieldAlreadyExists { + field: self.value_as_str(trc::Key::Key).unwrap_or_default(), + value: self.value_as_str(trc::Key::Value).unwrap_or_default(), + }, + trc::ManageCause::NotFound => ManagementApiError::NotFound { + item: self.value_as_str(trc::Key::Key).unwrap_or_default(), + }, + trc::ManageCause::NotSupported => ManagementApiError::Unsupported { + details: details_or_reason.unwrap_or("Requested action is unsupported"), + }, + trc::ManageCause::AssertFailed => ManagementApiError::AssertFailed, + trc::ManageCause::Error => ManagementApiError::Other { + details: details_or_reason.unwrap_or("An error occurred."), + }, + } + } + .into_http_response(), + + _ => self.to_request_error().into_http_response(), + } } } -/*impl ToHttpResponse for std::io::Error { - fn into_http_response(self) -> HttpResponse { - tracing::error!(context = "i/o", error = %self, "I/O error"); - - RequestError::internal_server_error().into_http_response() - } +pub trait ToRequestError { + fn to_request_error(&self) -> RequestError<'_>; } -impl ToHttpResponse for serde_json::Error { - fn into_http_response(self) -> HttpResponse { - RequestError::blank( - StatusCode::BAD_REQUEST.as_u16(), - "Invalid parameters", - format!("Failed to deserialize JSON: {self}"), - ) - .into_http_response() +impl ToRequestError for trc::Error { + fn to_request_error(&self) -> RequestError<'_> { + let details_or_reason = self + .value(trc::Key::Details) + .or_else(|| self.value(trc::Key::Reason)) + .and_then(|v| v.as_str()); + let details = details_or_reason.unwrap_or_else(|| self.as_ref().message()); + + match self.as_ref() { + trc::Cause::Jmap(cause) => match cause { + trc::JmapCause::UnknownCapability => RequestError::unknown_capability(details), + trc::JmapCause::NotJSON => RequestError::not_json(details), + trc::JmapCause::NotRequest => RequestError::not_request(details), + _ => RequestError::invalid_parameters(), + }, + trc::Cause::Limit(cause) => match cause { + trc::LimitCause::SizeRequest => RequestError::limit(RequestLimitError::SizeRequest), + trc::LimitCause::SizeUpload => RequestError::limit(RequestLimitError::SizeUpload), + trc::LimitCause::CallsIn => RequestError::limit(RequestLimitError::CallsIn), + trc::LimitCause::ConcurrentRequest => { + RequestError::limit(RequestLimitError::ConcurrentRequest) + } + trc::LimitCause::ConcurrentUpload => { + RequestError::limit(RequestLimitError::ConcurrentUpload) + } + trc::LimitCause::Quota => RequestError::over_quota(), + trc::LimitCause::BlobQuota => RequestError::over_blob_quota( + self.value(trc::Key::Total) + .and_then(|v| v.to_uint()) + .unwrap_or_default() as usize, + self.value(trc::Key::Size) + .and_then(|v| v.to_uint()) + .unwrap_or_default() as usize, + ), + trc::LimitCause::TooManyRequests => RequestError::too_many_requests(), + }, + trc::Cause::Auth(cause) => match cause { + trc::AuthCause::Failed => RequestError::unauthorized(), + trc::AuthCause::MissingTotp => { + RequestError::blank(403, "TOTP code required", cause.message()) + } + trc::AuthCause::TooManyAttempts | trc::AuthCause::Banned => { + RequestError::too_many_auth_attempts() + } + trc::AuthCause::Error => RequestError::internal_server_error(), + }, + trc::Cause::Resource(cause) => match cause { + trc::ResourceCause::NotFound => RequestError::not_found(), + trc::ResourceCause::BadParameters => RequestError::blank( + StatusCode::BAD_REQUEST.as_u16(), + "Invalid parameters", + details_or_reason.unwrap_or("One or multiple parameters could not be parsed."), + ), + trc::ResourceCause::Error => RequestError::internal_server_error(), + }, + _ => RequestError::internal_server_error(), + } } -}*/ +} impl JsonResponse { pub fn new(inner: T) -> Self { @@ -578,6 +654,12 @@ impl ToHttpResponse for Session { } } +impl ToHttpResponse for ManagementApiError<'_> { + fn into_http_response(self) -> super::HttpResponse { + JsonResponse::new(self).into_http_response() + } +} + impl ToHttpResponse for DownloadResponse { fn into_http_response(self) -> HttpResponse { hyper::Response::builder() @@ -623,7 +705,7 @@ impl ToHttpResponse for UploadResponse { } } -impl ToHttpResponse for RequestError { +impl ToHttpResponse for RequestError<'_> { fn into_http_response(self) -> HttpResponse { hyper::Response::builder() .status(StatusCode::from_u16(self.status).unwrap()) diff --git a/crates/jmap/src/api/management/mod.rs b/crates/jmap/src/api/management/mod.rs index a3e8d9fe..455f081b 100644 --- a/crates/jmap/src/api/management/mod.rs +++ b/crates/jmap/src/api/management/mod.rs @@ -28,27 +28,14 @@ use crate::{auth::AccessToken, JMAP}; #[derive(Serialize)] #[serde(tag = "error")] -pub enum ManagementApiError { - FieldAlreadyExists { - field: Cow<'static, str>, - value: Cow<'static, str>, - }, - FieldMissing { - field: Cow<'static, str>, - }, - NotFound { - item: Cow<'static, str>, - }, - Unsupported { - details: Cow<'static, str>, - }, +#[serde(rename_all = "camelCase")] +pub enum ManagementApiError<'x> { + FieldAlreadyExists { field: &'x str, value: &'x str }, + FieldMissing { field: &'x str }, + NotFound { item: &'x str }, + Unsupported { details: &'x str }, AssertFailed, - Other { - details: Cow<'static, str>, - }, - UnsupportedDirectoryOperation { - class: Cow<'static, str>, - }, + Other { details: &'x str }, } impl JMAP { diff --git a/crates/jmap/src/api/management/principal.rs b/crates/jmap/src/api/management/principal.rs index e197c017..a2a08520 100644 --- a/crates/jmap/src/api/management/principal.rs +++ b/crates/jmap/src/api/management/principal.rs @@ -189,7 +189,7 @@ impl JMAP { } Method::DELETE => { // Remove FTS index - self.core.storage.fts.remove_all(account_id).await; + self.core.storage.fts.remove_all(account_id).await?; // Delete account self.core @@ -408,8 +408,6 @@ impl JMAP { } pub fn assert_supported_directory(&self) -> trc::Result<()> { - let todo = "update webadmin"; - let class = match &self.core.storage.directory.store { DirectoryInner::Internal(_) => return Ok(()), DirectoryInner::Ldap(_) => "LDAP", @@ -441,50 +439,3 @@ impl From> for PrincipalResponse { } } } - -/* -fn into_directory_response(mut error: trc::Error) -> trc::Result { - let response = match error.as_ref() { - trc::Cause::MissingParameter => ManagementApiError::FieldMissing { - field: error - .take_value(trc::Key::Key) - .and_then(|v| v.into_string()) - .unwrap_or_default(), - }, - trc::Cause::AlreadyExists => ManagementApiError::FieldAlreadyExists { - field: error - .take_value(trc::Key::Key) - .and_then(|v| v.into_string()) - .unwrap_or_default(), - value: error - .take_value(trc::Key::Value) - .and_then(|v| v.into_string()) - .unwrap_or_default(), - }, - trc::StoreCause::NotFound.into_err() => ManagementApiError::NotFound { - item: error - .take_value(trc::Key::Key) - .and_then(|v| v.into_string()) - .unwrap_or_default(), - }, - trc::Cause::Unsupported => { - return JsonResponse::new(ManagementApiError::Unsupported { - details: "Requested action is unsupported".into(), - }) - .into_http_response(); - } - _ => { - tracing::warn!( - context = "directory", - event = "error", - reason = ?error, - "Directory error" - ); - - return RequestError::internal_server_error().into_http_response(); - } - }; - - JsonResponse::new(response).into_http_response() -} -*/ diff --git a/crates/jmap/src/api/request.rs b/crates/jmap/src/api/request.rs index 58a9afc0..89d264b4 100644 --- a/crates/jmap/src/api/request.rs +++ b/crates/jmap/src/api/request.rs @@ -8,7 +8,6 @@ use std::sync::Arc; use common::listener::ServerInstance; use jmap_proto::{ - error::method::MethodError, method::{ get, query, set::{self}, @@ -37,6 +36,8 @@ impl JMAP { for mut call in request.method_calls { // Resolve result and id references if let Err(method_error) = response.resolve_references(&mut call.method) { + tracing::error!(error = ?method_error, "Error handling method call"); + response.push_response(call.id, MethodName::error(), method_error); continue; } @@ -85,6 +86,8 @@ impl JMAP { response.push_response(call.id, call.name, method_response); } Err(err) => { + tracing::error!(error = ?err, "Error handling method call"); + response.push_error(call.id, err); } } @@ -160,10 +163,9 @@ impl JMAP { if self.core.jmap.principal_allow_lookups || access_token.is_super_user() { self.principal_get(req).await?.into() } else { - return Err(MethodError::Forbidden( - "Principal lookups are disabled".to_string(), - ) - .into()); + return Err(trc::JmapCause::Forbidden + .into_err() + .details("Principal lookups are disabled".to_string())); } } get::RequestArguments::Quota => { @@ -208,10 +210,9 @@ impl JMAP { if self.core.jmap.principal_allow_lookups || access_token.is_super_user() { self.principal_query(req).await?.into() } else { - return Err(MethodError::Forbidden( - "Principal lookups are disabled".to_string(), - ) - .into()); + return Err(trc::JmapCause::Forbidden + .into_err() + .details("Principal lookups are disabled".to_string())); } } query::RequestArguments::Quota => { diff --git a/crates/jmap/src/auth/mod.rs b/crates/jmap/src/auth/mod.rs index 0358a4b5..69b0cd98 100644 --- a/crates/jmap/src/auth/mod.rs +++ b/crates/jmap/src/auth/mod.rs @@ -15,10 +15,7 @@ use aes_gcm_siv::{ }; use directory::{Principal, Type}; -use jmap_proto::{ - error::method::MethodError, - types::{collection::Collection, id::Id}, -}; +use jmap_proto::types::{collection::Collection, id::Id}; use store::blake3; use utils::map::bitmap::Bitmap; @@ -118,11 +115,10 @@ impl AccessToken { if self.has_access(to_account_id.document_id(), to_collection) { Ok(self) } else { - Err(MethodError::Forbidden(format!( + Err(trc::JmapCause::Forbidden.into_err().details(format!( "You do not have access to account {}", to_account_id - )) - .into()) + ))) } } @@ -130,10 +126,9 @@ impl AccessToken { if self.is_member(account_id.document_id()) { Ok(self) } else { - Err( - MethodError::Forbidden(format!("You are not an owner of account {}", account_id)) - .into(), - ) + Err(trc::JmapCause::Forbidden + .into_err() + .details(format!("You are not an owner of account {}", account_id))) } } } diff --git a/crates/jmap/src/auth/oauth/token.rs b/crates/jmap/src/auth/oauth/token.rs index de44353e..88512a30 100644 --- a/crates/jmap/src/auth/oauth/token.rs +++ b/crates/jmap/src/auth/oauth/token.rs @@ -89,48 +89,45 @@ impl JMAP { (params.get("device_code"), params.get("client_id")) { // Obtain code - match self + if let Some(auth_code) = self .core .storage .lookup .key_get::>(format!("oauth:{device_code}").into_bytes()) .await? { - Some(auth_code) => { - let oauth = auth_code.inner; - response = if oauth.client_id != client_id { - TokenResponse::error(ErrorType::InvalidClient) - } else { - match oauth.status { - OAuthStatus::Authorized => { - // Mark this token as issued - self.core - .storage - .lookup - .key_delete(format!("oauth:{device_code}").into_bytes()) - .await?; + let oauth = auth_code.inner; + response = if oauth.client_id != client_id { + TokenResponse::error(ErrorType::InvalidClient) + } else { + match oauth.status { + OAuthStatus::Authorized => { + // Mark this token as issued + self.core + .storage + .lookup + .key_delete(format!("oauth:{device_code}").into_bytes()) + .await?; - // Issue token - self.issue_token(oauth.account_id, &oauth.client_id, true) - .await - .map(TokenResponse::Granted) - .map_err(|err| { - trc::AuthCause::Error - .into_err() - .details(err) - .caused_by(trc::location!()) - })? - } - OAuthStatus::Pending => { - TokenResponse::error(ErrorType::AuthorizationPending) - } - OAuthStatus::TokenIssued => { - TokenResponse::error(ErrorType::ExpiredToken) - } + // Issue token + self.issue_token(oauth.account_id, &oauth.client_id, true) + .await + .map(TokenResponse::Granted) + .map_err(|err| { + trc::AuthCause::Error + .into_err() + .details(err) + .caused_by(trc::location!()) + })? } - }; - } - None => (), + OAuthStatus::Pending => { + TokenResponse::error(ErrorType::AuthorizationPending) + } + OAuthStatus::TokenIssued => { + TokenResponse::error(ErrorType::ExpiredToken) + } + } + }; } } } else if grant_type.eq_ignore_ascii_case("refresh_token") { diff --git a/crates/jmap/src/auth/rate_limit.rs b/crates/jmap/src/auth/rate_limit.rs index 3462b5cf..3e0492f4 100644 --- a/crates/jmap/src/auth/rate_limit.rs +++ b/crates/jmap/src/auth/rate_limit.rs @@ -105,8 +105,6 @@ impl JMAP { } pub async fn is_auth_allowed_soft(&self, addr: &IpAddr) -> trc::Result<()> { - let todo = "convert into request errors"; - if let Some(rate) = &self.core.jmap.rate_authenticate_req { if self .core diff --git a/crates/jmap/src/blob/get.rs b/crates/jmap/src/blob/get.rs index 1dc976e5..d7dee222 100644 --- a/crates/jmap/src/blob/get.rs +++ b/crates/jmap/src/blob/get.rs @@ -5,7 +5,6 @@ */ use jmap_proto::{ - error::method::MethodError, method::{ get::{GetRequest, GetResponse}, lookup::{BlobInfo, BlobLookupRequest, BlobLookupResponse}, @@ -148,10 +147,7 @@ impl JMAP { Ok(response) } - pub async fn blob_lookup( - &self, - request: BlobLookupRequest, - ) -> trc::Result { + pub async fn blob_lookup(&self, request: BlobLookupRequest) -> trc::Result { let mut include_email = false; let mut include_mailbox = false; let mut include_thread = false; @@ -176,7 +172,7 @@ impl JMAP { Ok(value) } - MaybeUnparsable::ParseError(_) => Err(MethodError::UnknownDataType), + MaybeUnparsable::ParseError(_) => Err(trc::JmapCause::UnknownDataType.into_err()), }) .collect::, _>>()?; let req_account_id = request.account_id.document_id(); diff --git a/crates/jmap/src/blob/upload.rs b/crates/jmap/src/blob/upload.rs index 13d1cf2b..54b0c881 100644 --- a/crates/jmap/src/blob/upload.rs +++ b/crates/jmap/src/blob/upload.rs @@ -7,7 +7,7 @@ use std::sync::Arc; use jmap_proto::{ - error::{method::MethodError, set::SetError}, + error::set::SetError, method::upload::{ BlobUploadRequest, BlobUploadResponse, BlobUploadResponseObject, DataSourceObject, }, @@ -43,7 +43,7 @@ impl JMAP { let account_id = request.account_id.document_id(); if request.create.len() > self.core.jmap.set_max_objects { - return Err(MethodError::RequestTooLarge.into()); + return Err(trc::JmapCause::RequestTooLarge.into_err()); } 'outer: for (create_id, upload_object) in request.create { diff --git a/crates/jmap/src/changes/get.rs b/crates/jmap/src/changes/get.rs index d2a97928..a863180b 100644 --- a/crates/jmap/src/changes/get.rs +++ b/crates/jmap/src/changes/get.rs @@ -5,7 +5,6 @@ */ use jmap_proto::{ - error::method::MethodError, method::changes::{ChangesRequest, ChangesResponse, RequestArguments}, types::{collection::Collection, property::Property, state::State}, }; @@ -49,7 +48,7 @@ impl JMAP { RequestArguments::Quota => { access_token.assert_is_member(request.account_id)?; - return Err(MethodError::CannotCalculateChanges.into()); + return Err(trc::JmapCause::CannotCalculateChanges.into_err()); } }; diff --git a/crates/jmap/src/changes/query.rs b/crates/jmap/src/changes/query.rs index 60314afd..aa321618 100644 --- a/crates/jmap/src/changes/query.rs +++ b/crates/jmap/src/changes/query.rs @@ -4,13 +4,10 @@ * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-SEL */ -use jmap_proto::{ - error::method::MethodError, - method::{ - changes::{self, ChangesRequest}, - query::{self, QueryRequest}, - query_changes::{AddedItem, QueryChangesRequest, QueryChangesResponse}, - }, +use jmap_proto::method::{ + changes::{self, ChangesRequest}, + query::{self, QueryRequest}, + query_changes::{AddedItem, QueryChangesRequest, QueryChangesResponse}, }; use crate::{auth::AccessToken, JMAP}; @@ -36,9 +33,9 @@ impl JMAP { } query::RequestArguments::Quota => changes::RequestArguments::Quota, _ => { - return Err( - MethodError::UnknownMethod("Unknown method".to_string()).into() - ) + return Err(trc::JmapCause::UnknownMethod + .into_err() + .details("Unknown method")) } }, }, diff --git a/crates/jmap/src/changes/state.rs b/crates/jmap/src/changes/state.rs index 313b4ab4..955fa0dc 100644 --- a/crates/jmap/src/changes/state.rs +++ b/crates/jmap/src/changes/state.rs @@ -4,10 +4,7 @@ * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-SEL */ -use jmap_proto::{ - error::method::MethodError, - types::{collection::Collection, state::State}, -}; +use jmap_proto::types::{collection::Collection, state::State}; use trc::AddContext; use crate::JMAP; @@ -37,7 +34,7 @@ impl JMAP { let old_state: State = self.get_state(account_id, collection).await?; if let Some(if_in_state) = if_in_state { if &old_state != if_in_state { - return Err(MethodError::StateMismatch.into()); + return Err(trc::JmapCause::StateMismatch.into_err()); } } diff --git a/crates/jmap/src/email/copy.rs b/crates/jmap/src/email/copy.rs index a590320b..d514ac0c 100644 --- a/crates/jmap/src/email/copy.rs +++ b/crates/jmap/src/email/copy.rs @@ -5,7 +5,7 @@ */ use jmap_proto::{ - error::{method::MethodError, set::SetError}, + error::set::SetError, method::{ copy::{CopyRequest, CopyResponse, RequestArguments}, set::{self, SetRequest}, @@ -60,10 +60,9 @@ impl JMAP { let from_account_id = request.from_account_id.document_id(); if account_id == from_account_id { - return Err(MethodError::InvalidArguments( - "From accountId is equal to fromAccountId".to_string(), - ) - .into()); + return Err(trc::JmapCause::InvalidArguments + .into_err() + .details("From accountId is equal to fromAccountId")); } let old_state = self .assert_state(account_id, Collection::Email, &request.if_in_state) diff --git a/crates/jmap/src/email/get.rs b/crates/jmap/src/email/get.rs index 13c48d49..32b03894 100644 --- a/crates/jmap/src/email/get.rs +++ b/crates/jmap/src/email/get.rs @@ -5,7 +5,6 @@ */ use jmap_proto::{ - error::method::MethodError, method::get::{GetRequest, GetResponse}, object::{email::GetArguments, Object}, types::{ @@ -400,10 +399,9 @@ impl JMAP { } _ => { - return Err(MethodError::InvalidArguments(format!( - "Invalid property {property:?}" - )) - .into()); + return Err(trc::JmapCause::InvalidArguments + .into_err() + .details(format!("Invalid property {property:?}"))); } } } diff --git a/crates/jmap/src/email/parse.rs b/crates/jmap/src/email/parse.rs index 63319c8c..55a01096 100644 --- a/crates/jmap/src/email/parse.rs +++ b/crates/jmap/src/email/parse.rs @@ -5,7 +5,6 @@ */ use jmap_proto::{ - error::method::MethodError, method::parse::{ParseEmailRequest, ParseEmailResponse}, object::Object, types::{property::Property, value::Value}, @@ -30,7 +29,7 @@ impl JMAP { access_token: &AccessToken, ) -> trc::Result { if request.blob_ids.len() > self.core.jmap.mail_parse_max_items { - return Err(MethodError::RequestTooLarge.into()); + return Err(trc::JmapCause::RequestTooLarge.into_err()); } let properties = request.properties.unwrap_or_else(|| { vec![ @@ -235,10 +234,9 @@ impl JMAP { } _ => { - return Err(MethodError::InvalidArguments(format!( - "Invalid property {property:?}" - )) - .into()); + return Err(trc::JmapCause::InvalidArguments + .into_err() + .details(format!("Invalid property {property:?}"))); } } } diff --git a/crates/jmap/src/email/query.rs b/crates/jmap/src/email/query.rs index 2b93b28b..ca44445d 100644 --- a/crates/jmap/src/email/query.rs +++ b/crates/jmap/src/email/query.rs @@ -5,7 +5,6 @@ */ use jmap_proto::{ - error::method::MethodError, method::query::{Comparator, Filter, QueryRequest, QueryResponse, SortProperty}, object::email::QueryArguments, types::{acl::Acl, collection::Collection, keyword::Keyword, property::Property}, @@ -109,17 +108,18 @@ impl JMAP { Filter::Header(header) => { let mut header = header.into_iter(); let header_name = header.next().ok_or_else(|| { - MethodError::InvalidArguments( - "Header name is missing.".to_string(), - ) + trc::JmapCause::InvalidArguments + .into_err() + .details("Header name is missing.".to_string()) })?; match HeaderName::parse(header_name) { Some(HeaderName::Other(header_name)) => { - return Err(MethodError::InvalidArguments(format!( - "Querying header '{header_name}' is not supported.", - )) - .into()); + return Err(trc::JmapCause::InvalidArguments + .into_err() + .details(format!( + "Querying header '{header_name}' is not supported.", + ))); } Some(header_name) => { if let Some(header_value) = header.next() { @@ -155,7 +155,9 @@ impl JMAP { fts_filters.push(cond.into()); } other => { - return Err(MethodError::UnsupportedFilter(other.to_string()).into()) + return Err(trc::JmapCause::UnsupportedFilter + .into_err() + .details(other.to_string())) } } } @@ -252,7 +254,9 @@ impl JMAP { } other => { - return Err(MethodError::UnsupportedFilter(other.to_string()).into()) + return Err(trc::JmapCause::UnsupportedFilter + .into_err() + .details(other.to_string())) } } } @@ -329,7 +333,11 @@ impl JMAP { query::Comparator::field(Property::Cc, comparator.is_ascending) } - other => return Err(MethodError::UnsupportedSort(other.to_string()).into()), + other => { + return Err(trc::JmapCause::UnsupportedSort + .into_err() + .details(other.to_string())) + } }); } diff --git a/crates/jmap/src/email/snippet.rs b/crates/jmap/src/email/snippet.rs index 61516e6e..ae70afd2 100644 --- a/crates/jmap/src/email/snippet.rs +++ b/crates/jmap/src/email/snippet.rs @@ -5,7 +5,6 @@ */ use jmap_proto::{ - error::method::MethodError, method::{ query::Filter, search_snippet::{GetSearchSnippetRequest, GetSearchSnippetResponse, SearchSnippet}, @@ -83,7 +82,7 @@ impl JMAP { }; if email_ids.len() > self.core.jmap.snippet_max_results { - return Err(MethodError::RequestTooLarge.into()); + return Err(trc::JmapCause::RequestTooLarge.into_err()); } for email_id in email_ids { diff --git a/crates/jmap/src/lib.rs b/crates/jmap/src/lib.rs index 87296122..a14779d9 100644 --- a/crates/jmap/src/lib.rs +++ b/crates/jmap/src/lib.rs @@ -21,7 +21,6 @@ use dashmap::DashMap; use directory::QueryBy; use email::cache::Threads; use jmap_proto::{ - error::method::MethodError, method::{ query::{QueryRequest, QueryResponse}, set::{SetRequest, SetResponse}, @@ -540,7 +539,7 @@ impl UpdateResults for QueryResponse { .collect::>(); Ok(()) } else { - Err(MethodError::AnchorNotFound.into()) + Err(trc::JmapCause::AnchorNotFound.into_err()) } } } diff --git a/crates/jmap/src/mailbox/query.rs b/crates/jmap/src/mailbox/query.rs index 86d6ad65..16bcaa66 100644 --- a/crates/jmap/src/mailbox/query.rs +++ b/crates/jmap/src/mailbox/query.rs @@ -5,7 +5,6 @@ */ use jmap_proto::{ - error::method::MethodError, method::query::{Comparator, Filter, QueryRequest, QueryResponse, SortProperty}, object::{mailbox::QueryArguments, Object}, types::{acl::Acl, collection::Collection, property::Property, value::Value}, @@ -80,7 +79,11 @@ impl JMAP { filters.push(cond.into()); } - other => return Err(MethodError::UnsupportedFilter(other.to_string()).into()), + other => { + return Err(trc::JmapCause::UnsupportedFilter + .into_err() + .details(other.to_string())) + } } } @@ -182,7 +185,11 @@ impl JMAP { query::Comparator::field(Property::ParentId, comparator.is_ascending) } - other => return Err(MethodError::UnsupportedSort(other.to_string()).into()), + other => { + return Err(trc::JmapCause::UnsupportedSort + .into_err() + .details(other.to_string())) + } }); } diff --git a/crates/jmap/src/principal/query.rs b/crates/jmap/src/principal/query.rs index 9e9674b3..f2e862ce 100644 --- a/crates/jmap/src/principal/query.rs +++ b/crates/jmap/src/principal/query.rs @@ -6,7 +6,6 @@ use directory::QueryBy; use jmap_proto::{ - error::method::MethodError, method::query::{Filter, QueryRequest, QueryResponse, RequestArguments}, types::collection::Collection, }; @@ -65,7 +64,11 @@ impl JMAP { } } Filter::Type(_) => {} - other => return Err(MethodError::UnsupportedFilter(other.to_string()).into()), + other => { + return Err(trc::JmapCause::UnsupportedFilter + .into_err() + .details(other.to_string())) + } } } diff --git a/crates/jmap/src/push/get.rs b/crates/jmap/src/push/get.rs index c3d35eaa..888516d2 100644 --- a/crates/jmap/src/push/get.rs +++ b/crates/jmap/src/push/get.rs @@ -6,7 +6,6 @@ use base64::{engine::general_purpose, Engine}; use jmap_proto::{ - error::method::MethodError, method::get::{GetRequest, GetResponse, RequestArguments}, object::Object, types::{collection::Collection, property::Property, type_state::DataType, value::Value}, @@ -84,10 +83,9 @@ impl JMAP { result.append(Property::Id, Value::Id(id)); } Property::Url | Property::Keys | Property::Value => { - return Err(MethodError::Forbidden( + return Err(trc::JmapCause::Forbidden.into_err().details( "The 'url' and 'keys' properties are not readable".to_string(), - ) - .into()); + )); } property => { result.append(property.clone(), push.remove(property)); diff --git a/crates/jmap/src/quota/query.rs b/crates/jmap/src/quota/query.rs index fe3abbb6..4511250d 100644 --- a/crates/jmap/src/quota/query.rs +++ b/crates/jmap/src/quota/query.rs @@ -62,7 +62,7 @@ impl JMAP { Filter::And | Filter::Or | Filter::Not | Filter::Close => { filters.push(cond.into()); } - other => return Err(MethodError::UnsupportedFilter(other.to_string())), + other => return Err(trc::JmapCause::UnsupportedFilter.into_err().details(other.to_string())), } } @@ -87,7 +87,7 @@ impl JMAP { SortProperty::Used => { query::Comparator::field(Property::Used, comparator.is_ascending) } - other => return Err(MethodError::UnsupportedSort(other.to_string())), + other => return Err(trc::JmapCause::UnsupportedSort.into_err().details(other.to_string())), }); } diff --git a/crates/jmap/src/services/ingest.rs b/crates/jmap/src/services/ingest.rs index 8df7aea0..bbf12f84 100644 --- a/crates/jmap/src/services/ingest.rs +++ b/crates/jmap/src/services/ingest.rs @@ -18,8 +18,6 @@ use crate::{ impl JMAP { pub async fn deliver_message(&self, message: IngestMessage) -> Vec { - let todo = "trace all errors"; - // Read message let raw_message = match self .core @@ -117,7 +115,14 @@ impl JMAP { }) .await } - Err(_) => { + Err(err) => { + tracing::error!( + context = "ingest", + error = ?err, + rcpt = rcpt, + "Failed to ingest message" + ); + *status = DeliveryResult::TemporaryFailure { reason: "Transient server failure.".into(), }; @@ -139,31 +144,42 @@ impl JMAP { .await; } } - Err(mut err) => match err.as_ref() { - trc::Cause::Limit(trc::LimitCause::Quota) => { - *status = DeliveryResult::TemporaryFailure { - reason: "Mailbox over quota.".into(), + Err(mut err) => { + tracing::error!( + context = "ingest", + error = ?err, + rcpt = rcpt, + "Failed to ingest message" + ); + + match err.as_ref() { + trc::Cause::Limit(trc::LimitCause::Quota) => { + *status = DeliveryResult::TemporaryFailure { + reason: "Mailbox over quota.".into(), + } + } + trc::Cause::Ingest => { + *status = DeliveryResult::PermanentFailure { + code: err + .value(trc::Key::Reason) + .and_then(|v| v.to_uint()) + .map(|n| { + [(n / 100) as u8, ((n % 100) / 10) as u8, (n % 10) as u8] + }) + .unwrap(), + reason: err + .take_value(trc::Key::Reason) + .and_then(|v| v.into_string()) + .unwrap(), + } + } + _ => { + *status = DeliveryResult::TemporaryFailure { + reason: "Transient server failure.".into(), + } } } - trc::Cause::Ingest => { - *status = DeliveryResult::PermanentFailure { - code: err - .value(trc::Key::Reason) - .and_then(|v| v.to_uint()) - .map(|n| [(n / 100) as u8, ((n % 100) / 10) as u8, (n % 10) as u8]) - .unwrap(), - reason: err - .take_value(trc::Key::Reason) - .and_then(|v| v.into_string()) - .unwrap(), - } - } - _ => { - *status = DeliveryResult::TemporaryFailure { - reason: "Transient server failure.".into(), - } - } - }, + } } } diff --git a/crates/jmap/src/services/state.rs b/crates/jmap/src/services/state.rs index 1ddc574f..bd3ac621 100644 --- a/crates/jmap/src/services/state.rs +++ b/crates/jmap/src/services/state.rs @@ -99,7 +99,11 @@ pub fn spawn_state_manager(core: JmapInstance, mut change_rx: mpsc::Receiver result, Err(err) => { - let todo = "log me"; + tracing::error!( + context = "ingest", + error = ?err, + "Failed to obtain access token" + ); continue; } }; diff --git a/crates/jmap/src/sieve/query.rs b/crates/jmap/src/sieve/query.rs index 9a32c4b1..b14d0260 100644 --- a/crates/jmap/src/sieve/query.rs +++ b/crates/jmap/src/sieve/query.rs @@ -5,7 +5,6 @@ */ use jmap_proto::{ - error::method::MethodError, method::query::{ Comparator, Filter, QueryRequest, QueryResponse, RequestArguments, SortProperty, }, @@ -32,7 +31,11 @@ impl JMAP { Filter::And | Filter::Or | Filter::Not | Filter::Close => { filters.push(cond.into()); } - other => return Err(MethodError::UnsupportedFilter(other.to_string()).into()), + other => { + return Err(trc::JmapCause::UnsupportedFilter + .into_err() + .details(other.to_string())) + } } } @@ -57,7 +60,11 @@ impl JMAP { SortProperty::IsActive => { query::Comparator::field(Property::IsActive, comparator.is_ascending) } - other => return Err(MethodError::UnsupportedSort(other.to_string()).into()), + other => { + return Err(trc::JmapCause::UnsupportedSort + .into_err() + .details(other.to_string())) + } }); } diff --git a/crates/jmap/src/submission/query.rs b/crates/jmap/src/submission/query.rs index 7f8aad44..df43ef92 100644 --- a/crates/jmap/src/submission/query.rs +++ b/crates/jmap/src/submission/query.rs @@ -5,7 +5,6 @@ */ use jmap_proto::{ - error::method::MethodError, method::query::{ Comparator, Filter, QueryRequest, QueryResponse, RequestArguments, SortProperty, }, @@ -60,7 +59,11 @@ impl JMAP { Filter::And | Filter::Or | Filter::Not | Filter::Close => { filters.push(cond.into()); } - other => return Err(MethodError::UnsupportedFilter(other.to_string()).into()), + other => { + return Err(trc::JmapCause::UnsupportedFilter + .into_err() + .details(other.to_string())) + } } } @@ -88,7 +91,11 @@ impl JMAP { SortProperty::SentAt => { query::Comparator::field(Property::SendAt, comparator.is_ascending) } - other => return Err(MethodError::UnsupportedSort(other.to_string()).into()), + other => { + return Err(trc::JmapCause::UnsupportedSort + .into_err() + .details(other.to_string())) + } }); } diff --git a/crates/jmap/src/vacation/set.rs b/crates/jmap/src/vacation/set.rs index 791e39f1..76bc46ae 100644 --- a/crates/jmap/src/vacation/set.rs +++ b/crates/jmap/src/vacation/set.rs @@ -7,10 +7,7 @@ use std::borrow::Cow; use jmap_proto::{ - error::{ - method::MethodError, - set::{SetError, SetErrorType}, - }, + error::set::{SetError, SetErrorType}, method::set::{RequestArguments, SetRequest, SetResponse}, object::{index::ObjectIndexBuilder, Object}, response::references::EvalObjectReferences, @@ -51,10 +48,9 @@ impl JMAP { let mut changes = None; match (request.create, request.update) { (Some(create), Some(update)) if !create.is_empty() && !update.is_empty() => { - return Err(MethodError::InvalidArguments( - "Creating and updating on the same request is not allowed.".into(), - ) - .into()); + return Err(trc::JmapCause::InvalidArguments + .into_err() + .details("Creating and updating on the same request is not allowed.")); } (Some(create), _) if !create.is_empty() => { for (id, obj) in create { diff --git a/crates/jmap/src/websocket/stream.rs b/crates/jmap/src/websocket/stream.rs index 2a138425..d836b241 100644 --- a/crates/jmap/src/websocket/stream.rs +++ b/crates/jmap/src/websocket/stream.rs @@ -21,7 +21,7 @@ use tokio_tungstenite::WebSocketStream; use tungstenite::Message; use utils::map::bitmap::Bitmap; -use crate::{auth::AccessToken, JMAP}; +use crate::{api::http::ToRequestError, auth::AccessToken, JMAP}; impl JMAP { pub async fn handle_websocket_stream( @@ -99,9 +99,8 @@ impl JMAP { continue; } Err(err) => { - let todo = "fix"; - //err.to_json() - todo!() + tracing::debug!(parent: &span, error = ?err, "Failed to parse WebSocket message"); + WebSocketRequestError::from(err.to_request_error()).to_json() }, }; if let Err(err) = stream.send(Message::Text(response)).await { diff --git a/crates/managesieve/src/core/client.rs b/crates/managesieve/src/core/client.rs index 71e33d5b..d00fb22d 100644 --- a/crates/managesieve/src/core/client.rs +++ b/crates/managesieve/src/core/client.rs @@ -11,7 +11,7 @@ use store::query::Filter; use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt}; use trc::AddContext; -use super::{Command, ResponseCode, SerializeResponse, Session, State, StatusResponse}; +use super::{Command, ResponseCode, SerializeResponse, Session, State}; impl Session { pub async fn ingest(&mut self, bytes: &[u8]) -> SessionResult { @@ -31,8 +31,14 @@ impl Session { requests.push(request); } Err(err) => { + let mut disconnect = err.must_disconnect(); + if let Err(err) = self.write_error(err).await { tracing::error!(parent: &self.span, event = "error", error = ?err); + disconnect = true; + } + + if disconnect { return SessionResult::Close; } } @@ -45,10 +51,7 @@ impl Session { break; } Err(receiver::Error::Error { response }) => { - if let Err(err) = self - .write(&StatusResponse::no(response.message).into_bytes()) - .await - { + if let Err(err) = self.write_error(response).await { tracing::error!(parent: &self.span, event = "error", error = ?err); return SessionResult::Close; } @@ -88,8 +91,14 @@ impl Session { } } Err(err) => { + let mut disconnect = err.must_disconnect(); + if let Err(err) = self.write_error(err).await { tracing::error!(parent: &self.span, event = "error", error = ?err); + disconnect = true; + } + + if disconnect { return SessionResult::Close; } } diff --git a/crates/managesieve/src/core/mod.rs b/crates/managesieve/src/core/mod.rs index db30c63c..077453c0 100644 --- a/crates/managesieve/src/core/mod.rs +++ b/crates/managesieve/src/core/mod.rs @@ -264,27 +264,35 @@ pub trait SerializeResponse { impl SerializeResponse for trc::Error { fn serialize(&self) -> Vec { - let todo = "serialize messages properly in all protocols"; let mut buf = Vec::with_capacity(64); buf.extend_from_slice(self.value_as_str(trc::Key::Type).unwrap_or("NO").as_bytes()); - if let Some(code) = self.value_as_str(trc::Key::Code) { + if let Some(code) = self + .value_as_str(trc::Key::Code) + .or_else(|| match self.as_ref() { + trc::Cause::Store(trc::StoreCause::NotFound) => { + Some(ResponseCode::NonExistent.as_str()) + } + trc::Cause::Store(_) => Some(ResponseCode::TryLater.as_str()), + trc::Cause::Limit(trc::LimitCause::Quota) => Some(ResponseCode::Quota.as_str()), + trc::Cause::Limit(_) => Some(ResponseCode::TryLater.as_str()), + _ => None, + }) + { buf.extend_from_slice(b" ("); buf.extend_from_slice(code.as_bytes()); buf.push(b')'); } - if let Some(message) = self + let message = self .value_as_str(trc::Key::Details) - .or_else(|| self.value_as_str(trc::Key::Reason)) - { - buf.extend_from_slice(b" \""); - for ch in message.as_bytes() { - if [b'\"', b'\\'].contains(ch) { - buf.push(b'\\'); - } - buf.push(*ch); + .unwrap_or_else(|| self.as_ref().message()); + buf.extend_from_slice(b" \""); + for ch in message.as_bytes() { + if [b'\"', b'\\'].contains(ch) { + buf.push(b'\\'); } - buf.push(b'\"'); + buf.push(*ch); } + buf.push(b'\"'); buf.extend_from_slice(b"\r\n"); buf } diff --git a/crates/managesieve/src/op/authenticate.rs b/crates/managesieve/src/op/authenticate.rs index aa9f10bf..82d57d16 100644 --- a/crates/managesieve/src/op/authenticate.rs +++ b/crates/managesieve/src/op/authenticate.rs @@ -71,7 +71,6 @@ impl Session { self.jmap.is_auth_allowed_soft(&self.remote_addr).await?; // Authenticate - let mut is_totp_error = false; let access_token = match credentials { Credentials::Plain { username, secret } | Credentials::XOauth2 { username, secret } => { self.jmap @@ -81,16 +80,37 @@ impl Session { self.remote_addr, ServerProtocol::ManageSieve, ) - .await? + .await } Credentials::OAuthBearer { token } => { - let (account_id, _, _) = self + match self .jmap .validate_access_token("access_token", &token) - .await?; - self.jmap.get_access_token(account_id).await? + .await + { + Ok((account_id, _, _)) => self.jmap.get_access_token(account_id).await, + Err(err) => Err(err), + } } - }; + } + .map_err(|err| { + if err.matches(trc::Cause::Auth(trc::AuthCause::Failed)) { + match &self.state { + State::NotAuthenticated { auth_failures } + if *auth_failures < self.jmap.core.imap.max_auth_failures => + { + self.state = State::NotAuthenticated { + auth_failures: auth_failures + 1, + }; + } + _ => { + return trc::AuthCause::TooManyAttempts.into_err().caused_by(err); + } + } + } + + err + })?; // Enforce concurrency limits let in_flight = match self @@ -114,35 +134,7 @@ impl Session { in_flight, }; - let todo = "implement this"; - Ok(StatusResponse::ok("Authentication successful").into_bytes()) - /*} else { - match &self.state { - State::NotAuthenticated { auth_failures } - if *auth_failures < self.jmap.core.imap.max_auth_failures => - { - self.state = State::NotAuthenticated { - auth_failures: auth_failures + 1, - }; - Err(trc::Cause::Authentication - .into_err() - .details(if is_totp_error { - "Missing TOTP code, try with 'secret$totp_code'." - } else { - "Authentication failed." - })) - } - _ => { - tracing::debug!( - parent: &self.span, - event = "disconnect", - "Too many authentication failures, disconnecting.", - ); - Err(StatusResponse::bye("Too many authentication failures")) - } - } - }*/ } pub async fn handle_unauthenticate(&mut self) -> trc::Result> { diff --git a/crates/pop3/src/client.rs b/crates/pop3/src/client.rs index 1bf7f5b5..09556513 100644 --- a/crates/pop3/src/client.rs +++ b/crates/pop3/src/client.rs @@ -53,15 +53,16 @@ impl Session { } for request in requests { - let mut result = None; - let maybe_err = match request { + let result = match request { Ok(command) => match self.validate_request(command).await { Ok(command) => match command { Command::User { name } => { if let State::NotAuthenticated { username, .. } = &mut self.state { let response = format!("{name} is a valid mailbox"); *username = Some(name); - self.write_ok(response).await + self.write_ok(response) + .await + .map(|_| SessionResult::Continue) } else { unreachable!(); } @@ -78,20 +79,36 @@ impl Session { secret: string, }) .await + .map(|_| SessionResult::Continue) } - Command::Quit => { - result = SessionResult::Close.into(); - self.handle_quit().await + Command::Quit => self.handle_quit().await.map(|_| SessionResult::Close), + Command::Stat => self.handle_stat().await.map(|_| SessionResult::Continue), + Command::List { msg } => { + self.handle_list(msg).await.map(|_| SessionResult::Continue) } - Command::Stat => self.handle_stat().await, - Command::List { msg } => self.handle_list(msg).await, - Command::Retr { msg } => self.handle_fetch(msg, None).await, - Command::Dele { msg } => self.handle_dele(vec![msg]).await, - Command::DeleMany { msgs } => self.handle_dele(msgs).await, - Command::Top { msg, n } => self.handle_fetch(msg, n.into()).await, - Command::Uidl { msg } => self.handle_uidl(msg).await, - Command::Noop => self.write_ok("NOOP").await, - Command::Rset => self.handle_rset().await, + Command::Retr { msg } => self + .handle_fetch(msg, None) + .await + .map(|_| SessionResult::Continue), + Command::Dele { msg } => self + .handle_dele(vec![msg]) + .await + .map(|_| SessionResult::Continue), + Command::DeleMany { msgs } => self + .handle_dele(msgs) + .await + .map(|_| SessionResult::Continue), + Command::Top { msg, n } => self + .handle_fetch(msg, n.into()) + .await + .map(|_| SessionResult::Continue), + Command::Uidl { msg } => { + self.handle_uidl(msg).await.map(|_| SessionResult::Continue) + } + Command::Noop => { + self.write_ok("NOOP").await.map(|_| SessionResult::Continue) + } + Command::Rset => self.handle_rset().await.map(|_| SessionResult::Continue), Command::Capa => { let mechanisms = if self.stream.is_tls() || self.jmap.core.imap.allow_plain_auth { @@ -108,37 +125,37 @@ impl Session { .serialize(), ) .await + .map(|_| SessionResult::Continue) } - Command::Stls => { - result = SessionResult::UpgradeTls.into(); - self.write_ok("Begin TLS negotiation now").await - } - Command::Utf8 => self.write_ok("UTF8 enabled").await, - Command::Auth { mechanism, params } => { - self.handle_sasl(mechanism, params).await - } - Command::Apop { .. } => { - self.write_err( - trc::Cause::Pop3.into_err().details("APOP not supported."), - ) + Command::Stls => self + .write_ok("Begin TLS negotiation now") .await + .map(|_| SessionResult::UpgradeTls), + Command::Utf8 => self + .write_ok("UTF8 enabled") + .await + .map(|_| SessionResult::Continue), + Command::Auth { mechanism, params } => self + .handle_sasl(mechanism, params) + .await + .map(|_| SessionResult::Continue), + Command::Apop { .. } => { + Err(trc::Cause::Pop3.into_err().details("APOP not supported.")) } }, - Err(err) => self.write_err(err).await, + Err(err) => Err(err), }, - Err(err) => self.write_err(err).await, + Err(err) => Err(err), }; - if let Err(err) = maybe_err { - tracing::error!(parent: &self.span, "Error: {:?}", err); - if err.matches(trc::Cause::Network) { - return SessionResult::Close; - } else if let Err(err) = self.write_err(err).await { - tracing::error!(parent: &self.span, "Error: {:?}", err); - return SessionResult::Close; + match result { + Ok(SessionResult::Continue) => (), + Ok(result) => return result, + Err(err) => { + if !self.write_err(err).await { + return SessionResult::Close; + } } - } else if let Some(result) = result { - return result; } } diff --git a/crates/pop3/src/op/authenticate.rs b/crates/pop3/src/op/authenticate.rs index 93bc2422..0d7fd203 100644 --- a/crates/pop3/src/op/authenticate.rs +++ b/crates/pop3/src/op/authenticate.rs @@ -62,25 +62,46 @@ impl Session { pub async fn handle_auth(&mut self, credentials: Credentials) -> trc::Result<()> { // Throttle authentication requests - let todo = "disconnect in all protocols when this error is returned"; self.jmap.is_auth_allowed_soft(&self.remote_addr).await?; // Authenticate - let mut is_totp_error = false; let access_token = match credentials { Credentials::Plain { username, secret } | Credentials::XOauth2 { username, secret } => { self.jmap .authenticate_plain(&username, &secret, self.remote_addr, ServerProtocol::Pop3) - .await? + .await } Credentials::OAuthBearer { token } => { - let (account_id, _, _) = self + match self .jmap .validate_access_token("access_token", &token) - .await?; - self.jmap.get_access_token(account_id).await? + .await + { + Ok((account_id, _, _)) => self.jmap.get_access_token(account_id).await, + Err(err) => Err(err), + } } - }; + } + .map_err(|err| { + if err.matches(trc::Cause::Auth(trc::AuthCause::Failed)) { + match &self.state { + State::NotAuthenticated { + auth_failures, + username, + } if *auth_failures < self.jmap.core.imap.max_auth_failures => { + self.state = State::NotAuthenticated { + auth_failures: auth_failures + 1, + username: username.clone(), + }; + } + _ => { + return trc::AuthCause::TooManyAttempts.into_err().caused_by(err); + } + } + } + + err + })?; // Enforce concurrency limits let in_flight = match self @@ -102,37 +123,8 @@ impl Session { let mailbox = self.fetch_mailbox(access_token.primary_id()).await?; // Create session - let todo = "fix below"; self.state = State::Authenticated { in_flight, mailbox }; self.write_ok("Authentication successful").await - /*} else { - match &self.state { - State::NotAuthenticated { - auth_failures, - username, - } if *auth_failures < self.jmap.core.imap.max_auth_failures => { - self.state = State::NotAuthenticated { - auth_failures: auth_failures + 1, - username: username.clone(), - }; - self.write_err(if is_totp_error { - "Missing TOTP code, try with 'secret$totp_code'." - } else { - "Authentication failed." - }) - .await - } - _ => { - tracing::debug!( - parent: &self.span, - event = "disconnect", - "Too many authentication failures, disconnecting.", - ); - self.write_err("Too many authentication failures").await?; - Err(()) - } - } - }*/ } pub fn get_concurrency_limiter(&self, account_id: u32) -> Option> { diff --git a/crates/pop3/src/protocol/response.rs b/crates/pop3/src/protocol/response.rs index 2b026a8f..425a5a2b 100644 --- a/crates/pop3/src/protocol/response.rs +++ b/crates/pop3/src/protocol/response.rs @@ -152,11 +152,9 @@ pub trait SerializeResponse { impl SerializeResponse for trc::Error { fn serialize(&self) -> Vec { - let todo = "serialize messages properly in all protocols"; let message = self .value_as_str(trc::Key::Details) - .or_else(|| self.value_as_str(trc::Key::Reason)) - .unwrap_or("Internal Server Error"); + .unwrap_or_else(|| self.as_ref().message()); let mut buf = Vec::with_capacity(message.len() + 6); buf.extend_from_slice(b"-ERR "); buf.extend_from_slice(message.as_bytes()); diff --git a/crates/pop3/src/session.rs b/crates/pop3/src/session.rs index 9db33f56..6837cd68 100644 --- a/crates/pop3/src/session.rs +++ b/crates/pop3/src/session.rs @@ -159,8 +159,17 @@ impl Session { .await } - pub async fn write_err(&mut self, err: trc::Error) -> trc::Result<()> { + pub async fn write_err(&mut self, err: trc::Error) -> bool { tracing::error!(parent: &self.span, "POP3 error: {}", err); - self.write_bytes(err.serialize()).await + let disconnect = err.must_disconnect(); + + if !err.matches(trc::Cause::Network) { + if let Err(err) = self.write_bytes(err.serialize()).await { + tracing::debug!(parent: &self.span, "Failed to write error: {}", err); + return false; + } + } + + !disconnect } } diff --git a/crates/store/src/dispatch/fts.rs b/crates/store/src/dispatch/fts.rs index 57c47fcf..fd56451f 100644 --- a/crates/store/src/dispatch/fts.rs +++ b/crates/store/src/dispatch/fts.rs @@ -26,7 +26,7 @@ impl FtsStore { #[cfg(feature = "elastic")] FtsStore::ElasticSearch(store) => store.fts_index(document).await, } - .caused_by( trc::location!()) + .caused_by(trc::location!()) } pub async fn query + Display + Clone + std::fmt::Debug>( @@ -42,7 +42,7 @@ impl FtsStore { store.fts_query(account_id, collection, filters).await } } - .caused_by( trc::location!()) + .caused_by(trc::location!()) } pub async fn remove( @@ -58,7 +58,7 @@ impl FtsStore { store.fts_remove(account_id, collection, document_ids).await } } - .caused_by( trc::location!()) + .caused_by(trc::location!()) } pub async fn remove_all(&self, account_id: u32) -> trc::Result<()> { @@ -67,6 +67,6 @@ impl FtsStore { #[cfg(feature = "elastic")] FtsStore::ElasticSearch(store) => store.fts_remove_all(account_id).await, } - .caused_by( trc::location!()) + .caused_by(trc::location!()) } } diff --git a/crates/trc/src/imple.rs b/crates/trc/src/imple.rs index 2596d35a..3dffa825 100644 --- a/crates/trc/src/imple.rs +++ b/crates/trc/src/imple.rs @@ -156,6 +156,29 @@ impl Cause { pub fn into_err(self) -> Error { Error::new(self) } + + pub fn message(&self) -> &'static str { + match self { + Self::Store(cause) => cause.message(), + Self::Jmap(cause) => cause.message(), + Self::Imap => "IMAP error", + Self::ManageSieve => "ManageSieve error", + Self::Pop3 => "POP3 error", + Self::Smtp => "SMTP error", + Self::Thread => "Thread error", + Self::Fetch => "Fetch error", + Self::Acme => "ACME error", + Self::Dns => "DNS error", + Self::Ingest => "Message Ingest error", + Self::Network => "Network error", + Self::Limit(cause) => cause.message(), + Self::Manage(cause) => cause.message(), + Self::Auth(cause) => cause.message(), + Self::Purge => "Purge error", + Self::Configuration => "Configuration error", + Self::Resource(cause) => cause.message(), + } + } } impl StoreCause { @@ -178,6 +201,32 @@ impl StoreCause { pub fn into_err(self) -> Error { Error::new(Cause::Store(self)) } + + pub fn message(&self) -> &'static str { + match self { + Self::AssertValue => "Another process has modified the value", + Self::BlobMissingMarker => "Blob is missing marker", + Self::FoundationDB => "FoundationDB error", + Self::MySQL => "MySQL error", + Self::PostgreSQL => "PostgreSQL error", + Self::RocksDB => "RocksDB error", + Self::SQLite => "SQLite error", + Self::Ldap => "LDAP error", + Self::ElasticSearch => "ElasticSearch error", + Self::Redis => "Redis error", + Self::S3 => "S3 error", + Self::Filesystem => "Filesystem error", + Self::Pool => "Connection pool error", + Self::DataCorruption => "Data corruption", + Self::Decompress => "Decompression error", + Self::Deserialize => "Deserialization error", + Self::NotFound => "Not found", + Self::NotConfigured => "Not configured", + Self::NotSupported => "Operation not supported", + Self::Unexpected => "Unexpected error", + Self::Crypto => "Crypto error", + } + } } impl AuthCause { @@ -200,6 +249,19 @@ impl AuthCause { pub fn into_err(self) -> Error { Error::new(Cause::Auth(self)) } + + pub fn message(&self) -> &'static str { + match self { + Self::Failed => "Authentication failed", + Self::MissingTotp => concat!( + "A TOTP code is required to authenticate this account. ", + "Try authenticating again using 'secret$totp_token'." + ), + Self::TooManyAttempts => "Too many authentication attempts", + Self::Banned => "Banned", + Self::Error => "Authentication error", + } + } } impl ManageCause { @@ -222,6 +284,17 @@ impl ManageCause { pub fn into_err(self) -> Error { Error::new(Cause::Manage(self)) } + + pub fn message(&self) -> &'static str { + match self { + Self::MissingParameter => "Missing parameter", + Self::AlreadyExists => "Already exists", + Self::AssertFailed => "Assertion failed", + Self::NotFound => "Not found", + Self::NotSupported => "Operation not supported", + Self::Error => "Management API Error", + } + } } impl JmapCause { @@ -244,6 +317,29 @@ impl JmapCause { pub fn into_err(self) -> Error { Error::new(Cause::Jmap(self)) } + + pub fn message(&self) -> &'static str { + match self { + Self::InvalidArguments => "Invalid arguments", + Self::RequestTooLarge => "Request too large", + Self::StateMismatch => "State mismatch", + Self::AnchorNotFound => "Anchor not found", + Self::UnsupportedFilter => "Unsupported filter", + Self::UnsupportedSort => "Unsupported sort", + Self::UnknownMethod => "Unknown method", + Self::InvalidResultReference => "Invalid result reference", + Self::Forbidden => "Forbidden", + Self::AccountNotFound => "Account not found", + Self::AccountNotSupportedByMethod => "Account not supported by method", + Self::AccountReadOnly => "Account read-only", + Self::NotFound => "Not found", + Self::CannotCalculateChanges => "Cannot calculate changes", + Self::UnknownDataType => "Unknown data type", + Self::UnknownCapability => "Unknown capability", + Self::NotJSON => "Not JSON", + Self::NotRequest => "Not a request", + } + } } impl LimitCause { @@ -266,6 +362,19 @@ impl LimitCause { pub fn into_err(self) -> Error { Error::new(Cause::Limit(self)) } + + pub fn message(&self) -> &'static str { + match self { + Self::SizeRequest => "Request too large", + Self::SizeUpload => "Upload too large", + Self::CallsIn => "Too many calls in", + Self::ConcurrentRequest => "Too many concurrent requests", + Self::ConcurrentUpload => "Too many concurrent uploads", + Self::Quota => "Quota exceeded", + Self::BlobQuota => "Blob quota exceeded", + Self::TooManyRequests => "Too many requests", + } + } } impl ResourceCause { @@ -288,6 +397,14 @@ impl ResourceCause { pub fn into_err(self) -> Error { Error::new(Cause::Resource(self)) } + + pub fn message(&self) -> &'static str { + match self { + Self::NotFound => "Not found", + Self::BadParameters => "Bad parameters", + Self::Error => "Resource error", + } + } } impl Error { @@ -301,12 +418,23 @@ impl Error { self.inner == Cause::Store(StoreCause::AssertValue) } + #[inline(always)] pub fn is_jmap_method_error(&self) -> bool { !matches!( self.inner, Cause::Jmap(JmapCause::UnknownCapability | JmapCause::NotJSON | JmapCause::NotRequest) ) } + + #[inline(always)] + pub fn must_disconnect(&self) -> bool { + matches!( + self.inner, + Cause::Network + | Cause::Auth(AuthCause::TooManyAttempts | AuthCause::Banned) + | Cause::Limit(LimitCause::ConcurrentRequest | LimitCause::TooManyRequests) + ) + } } impl Value { diff --git a/crates/trc/src/lib.rs b/crates/trc/src/lib.rs index 351960ba..1a5979b5 100644 --- a/crates/trc/src/lib.rs +++ b/crates/trc/src/lib.rs @@ -95,16 +95,6 @@ pub enum Cause { Resource(ResourceCause), } -/* - - Http, - Crypto, - Timeout, - Configuration, - Unknown, - -*/ - #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum StoreCause { AssertValue, @@ -160,17 +150,16 @@ pub enum LimitCause { SizeRequest, SizeUpload, CallsIn, - ConcurrentRequest, //RequestError::limit(RequestLimitError::ConcurrentRequest) StatusResponse::bye("Too many concurrent IMAP connections.").into_bytes(), - ConcurrentUpload, //RequestError::limit(RequestLimitError::ConcurrentUpload) + ConcurrentRequest, + ConcurrentUpload, Quota, - BlobQuota, //RequestError::over_blob_quota - TooManyRequests, //RequestError::too_many_requests() + disconnect imap StatusResponse::bye("Too many authentication requests from this IP address.") + BlobQuota, + TooManyRequests, } #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ManageCause { MissingParameter, - Invalid, AlreadyExists, AssertFailed, NotFound, @@ -182,9 +171,8 @@ pub enum ManageCause { pub enum AuthCause { Failed, MissingTotp, - TooManyAttempts, //RequestError::too_many_auth_attempts() + disconnect imap + TooManyAttempts, Banned, - Invalid, Error, } @@ -195,21 +183,6 @@ pub enum ResourceCause { Error, } -/* - -RequestError::unauthorized().into_http_response() - -RequestError::blank( - 403, - "TOTP code required", - concat!( - "A TOTP code is required to authenticate this account. ", - "Try authenticating again using 'secret$totp_token'." - ), - ) - -*/ - #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Protocol { Jmap, diff --git a/crates/trc/src/macros.rs b/crates/trc/src/macros.rs index 3b668548..cd9c2b8a 100644 --- a/crates/trc/src/macros.rs +++ b/crates/trc/src/macros.rs @@ -13,7 +13,7 @@ macro_rules! trace { .ctx($crate::Key::$key, $crate::Value::from($value)) )* ; - eprintln!("{}", event); + //eprintln!("{}", event); } }; } @@ -27,7 +27,7 @@ macro_rules! error { .ctx($crate::Key::$key, $crate::Value::from($value)) )* ; - eprintln!("{}", event); + //eprintln!("{}", event); }}; } diff --git a/tests/src/directory/smtp.rs b/tests/src/directory/smtp.rs index 87aa5c37..abc30fb9 100644 --- a/tests/src/directory/smtp.rs +++ b/tests/src/directory/smtp.rs @@ -88,7 +88,7 @@ async fn lmtp_directory() { Item::Verify(v) => match core.vrfy(&handle, v).await { Ok(v) => v.into(), Err(e) => { - if e.matches(trc::StoreCause::NotSupported) { + if e.matches(trc::Cause::Store(trc::StoreCause::NotSupported)) { LookupResult::False } else { panic!("Unexpected error: {e:?}") @@ -98,7 +98,7 @@ async fn lmtp_directory() { Item::Expand(v) => match core.expn(&handle, v).await { Ok(v) => v.into(), Err(e) => { - if e.matches(trc::StoreCause::NotSupported) { + if e.matches(trc::Cause::Store(trc::StoreCause::NotSupported)) { LookupResult::False } else { panic!("Unexpected error: {e:?}") @@ -132,7 +132,7 @@ async fn lmtp_directory() { Item::Verify(v) => match core.vrfy(&handle, v).await { Ok(v) => v.into(), Err(e) => { - if e.matches(trc::StoreCause::NotSupported) { + if e.matches(trc::Cause::Store(trc::StoreCause::NotSupported)) { LookupResult::False } else { panic!("Unexpected error: {e:?}") @@ -142,7 +142,7 @@ async fn lmtp_directory() { Item::Expand(v) => match core.expn(&handle, v).await { Ok(v) => v.into(), Err(e) => { - if e.matches(trc::StoreCause::NotSupported) { + if e.matches(trc::Cause::Store(trc::StoreCause::NotSupported)) { LookupResult::False } else { panic!("Unexpected error: {e:?}") diff --git a/tests/src/jmap/mod.rs b/tests/src/jmap/mod.rs index 72eac659..73f53f12 100644 --- a/tests/src/jmap/mod.rs +++ b/tests/src/jmap/mod.rs @@ -689,7 +689,7 @@ pub async fn test_account_login(login: &str, secret: &str) -> Client { #[derive(Deserialize)] #[serde(untagged)] pub enum Response { - RequestError(RequestError), + RequestError(RequestError<'static>), Error { error: String, details: String }, Data { data: T }, } diff --git a/tests/src/jmap/webhooks.rs b/tests/src/jmap/webhooks.rs index 7ac11622..bffcf98d 100644 --- a/tests/src/jmap/webhooks.rs +++ b/tests/src/jmap/webhooks.rs @@ -140,7 +140,7 @@ pub fn spawn_mock_webhook_endpoint() -> Arc { //let c = print!("rejected webhook: {}", serde_json::to_string_pretty(&request).unwrap()); Ok::<_, hyper::Error>( - Err(trc::ResourceCause::NotFound.into_err()) + RequestError::not_found().into_http_response() ) }