diff --git a/Cargo.lock b/Cargo.lock index d33da32..b0c343b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6094,7 +6094,7 @@ dependencies = [ [[package]] name = "tranquil-api" -version = "0.4.3" +version = "0.4.4" dependencies = [ "anyhow", "axum", @@ -6142,7 +6142,7 @@ dependencies = [ [[package]] name = "tranquil-auth" -version = "0.4.3" +version = "0.4.4" dependencies = [ "anyhow", "base32", @@ -6165,7 +6165,7 @@ dependencies = [ [[package]] name = "tranquil-cache" -version = "0.4.3" +version = "0.4.4" dependencies = [ "async-trait", "base64 0.22.1", @@ -6179,7 +6179,7 @@ dependencies = [ [[package]] name = "tranquil-comms" -version = "0.4.3" +version = "0.4.4" dependencies = [ "async-trait", "base64 0.22.1", @@ -6194,7 +6194,7 @@ dependencies = [ [[package]] name = "tranquil-config" -version = "0.4.3" +version = "0.4.4" dependencies = [ "confique", "serde", @@ -6202,7 +6202,7 @@ dependencies = [ [[package]] name = "tranquil-crypto" -version = "0.4.3" +version = "0.4.4" dependencies = [ "aes-gcm", "base64 0.22.1", @@ -6218,7 +6218,7 @@ dependencies = [ [[package]] name = "tranquil-db" -version = "0.4.3" +version = "0.4.4" dependencies = [ "async-trait", "chrono", @@ -6235,7 +6235,7 @@ dependencies = [ [[package]] name = "tranquil-db-traits" -version = "0.4.3" +version = "0.4.4" dependencies = [ "async-trait", "base64 0.22.1", @@ -6251,7 +6251,7 @@ dependencies = [ [[package]] name = "tranquil-infra" -version = "0.4.3" +version = "0.4.4" dependencies = [ "async-trait", "bytes", @@ -6262,7 +6262,7 @@ dependencies = [ [[package]] name = "tranquil-lexicon" -version = "0.4.3" +version = "0.4.4" dependencies = [ "chrono", "hickory-resolver", @@ -6280,7 +6280,7 @@ dependencies = [ [[package]] name = "tranquil-oauth" -version = "0.4.3" +version = "0.4.4" dependencies = [ "anyhow", "axum", @@ -6303,7 +6303,7 @@ dependencies = [ [[package]] name = "tranquil-oauth-server" -version = "0.4.3" +version = "0.4.4" dependencies = [ "axum", "base64 0.22.1", @@ -6336,7 +6336,7 @@ dependencies = [ [[package]] name = "tranquil-pds" -version = "0.4.3" +version = "0.4.4" dependencies = [ "aes-gcm", "anyhow", @@ -6424,7 +6424,7 @@ dependencies = [ [[package]] name = "tranquil-repo" -version = "0.4.3" +version = "0.4.4" dependencies = [ "bytes", "cid", @@ -6436,7 +6436,7 @@ dependencies = [ [[package]] name = "tranquil-ripple" -version = "0.4.3" +version = "0.4.4" dependencies = [ "async-trait", "backon", @@ -6461,7 +6461,7 @@ dependencies = [ [[package]] name = "tranquil-scopes" -version = "0.4.3" +version = "0.4.4" dependencies = [ "axum", "futures", @@ -6477,7 +6477,7 @@ dependencies = [ [[package]] name = "tranquil-server" -version = "0.4.3" +version = "0.4.4" dependencies = [ "axum", "clap", @@ -6497,7 +6497,7 @@ dependencies = [ [[package]] name = "tranquil-storage" -version = "0.4.3" +version = "0.4.4" dependencies = [ "async-trait", "aws-config", @@ -6514,7 +6514,7 @@ dependencies = [ [[package]] name = "tranquil-sync" -version = "0.4.3" +version = "0.4.4" dependencies = [ "anyhow", "axum", @@ -6536,7 +6536,7 @@ dependencies = [ [[package]] name = "tranquil-types" -version = "0.4.3" +version = "0.4.4" dependencies = [ "chrono", "cid", diff --git a/crates/tranquil-pds/src/api/error.rs b/crates/tranquil-pds/src/api/error.rs index 2a7d03e..73e908d 100644 --- a/crates/tranquil-pds/src/api/error.rs +++ b/crates/tranquil-pds/src/api/error.rs @@ -9,8 +9,7 @@ use std::borrow::Cow; #[derive(Debug, Serialize)] struct ErrorBody<'a> { error: Cow<'a, str>, - #[serde(skip_serializing_if = "Option::is_none")] - message: Option, + message: String, } #[derive(Debug)] @@ -214,7 +213,7 @@ impl ApiError { } fn error_name(&self) -> Cow<'static, str> { match self { - Self::InternalError(_) | Self::DatabaseError => Cow::Borrowed("InternalError"), + Self::InternalError(_) | Self::DatabaseError => Cow::Borrowed("InternalServerError"), Self::UpstreamFailure | Self::UpstreamUnavailable(_) | Self::UpstreamErrorMsg(_) => { Cow::Borrowed("UpstreamError") } @@ -313,22 +312,47 @@ impl ApiError { Self::LegacyLoginBlocked => Cow::Borrowed("MfaRequired"), } } - fn message(&self) -> Option { + fn message(&self) -> String { match self { - Self::InternalError(msg) - | Self::AuthenticationFailed(msg) - | Self::InvalidToken(msg) - | Self::ExpiredToken(msg) - | Self::OAuthExpiredToken(msg) - | Self::RepoNotFound(msg) - | Self::BlobNotFound(msg) - | Self::InvalidHandle(msg) - | Self::HandleNotAvailable(msg) - | Self::InvalidSwap(msg) - | Self::InsufficientScope(msg) - | Self::InvalidCode(msg) - | Self::RateLimitExceeded(msg) - | Self::ServiceUnavailable(msg) => msg.clone(), + Self::InternalError(msg) => msg + .clone() + .unwrap_or_else(|| "Internal Server Error".into()), + Self::AuthenticationFailed(msg) => msg + .clone() + .unwrap_or_else(|| "Authentication failed".into()), + Self::InvalidToken(msg) => { + msg.clone().unwrap_or_else(|| "Invalid token".into()) + } + Self::ExpiredToken(msg) | Self::OAuthExpiredToken(msg) => { + msg.clone().unwrap_or_else(|| "Token has expired".into()) + } + Self::RepoNotFound(msg) => msg + .clone() + .unwrap_or_else(|| "Repository not found".into()), + Self::BlobNotFound(msg) => { + msg.clone().unwrap_or_else(|| "Blob not found".into()) + } + Self::InvalidHandle(msg) => { + msg.clone().unwrap_or_else(|| "Invalid handle".into()) + } + Self::HandleNotAvailable(msg) => msg + .clone() + .unwrap_or_else(|| "Handle not available".into()), + Self::InvalidSwap(msg) => { + msg.clone().unwrap_or_else(|| "Invalid swap".into()) + } + Self::InsufficientScope(msg) => msg + .clone() + .unwrap_or_else(|| "Insufficient scope".into()), + Self::InvalidCode(msg) => { + msg.clone().unwrap_or_else(|| "Invalid code".into()) + } + Self::RateLimitExceeded(msg) => msg + .clone() + .unwrap_or_else(|| "Rate limit exceeded".into()), + Self::ServiceUnavailable(msg) => msg + .clone() + .unwrap_or_else(|| "Service temporarily unavailable".into()), Self::InvalidRequest(msg) | Self::UpstreamUnavailable(msg) | Self::InvalidPassword(msg) @@ -336,109 +360,117 @@ impl ApiError { | Self::InvalidRecord(msg) | Self::NotFoundMsg(msg) | Self::UpstreamErrorMsg(msg) - | Self::PayloadTooLarge(msg) => Some(msg.clone()), - Self::AccountMigrated => Some( - "Account has been migrated to another PDS. Repo operations are not allowed." - .to_string(), - ), - Self::AccountNotVerified => Some( - "You must verify at least one notification channel before creating records" - .to_string(), - ), - Self::NoPasskeys => { - Some("No passkeys registered for this account".to_string()) + | Self::PayloadTooLarge(msg) + | Self::InvalidScopes(msg) + | Self::InvalidDelegation(msg) + | Self::AuthorizationError(msg) + | Self::InvalidDid(msg) => msg.clone(), + Self::UpstreamError { message, .. } => message + .clone() + .unwrap_or_else(|| "Upstream error".into()), + Self::DatabaseError => "Internal Server Error".into(), + Self::AuthenticationRequired => "Authentication required".into(), + Self::TokenRequired => "Authentication token required".into(), + Self::AccountDeactivated => "Account is deactivated".into(), + Self::AccountTakedown => "Account has been taken down".into(), + Self::AccountNotFound => "Account not found".into(), + Self::RecordNotFound => "Record not found".into(), + Self::Forbidden => "Forbidden".into(), + Self::InvitesDisabled => "Invite codes are disabled on this server".into(), + Self::InvalidCollection => "Invalid collection".into(), + Self::InvalidChannel => "Invalid notification channel".into(), + Self::TotpAlreadyEnabled => "TOTP is already enabled".into(), + Self::TotpNotEnabled => "TOTP is not enabled".into(), + Self::DuplicateAppPassword => "An app password with this name already exists".into(), + Self::AppPasswordNotFound => "App password not found".into(), + Self::SessionNotFound => "Session not found".into(), + Self::UpstreamFailure => "Upstream service failed".into(), + Self::RepoTakendown => "Repository has been taken down".into(), + Self::RepoDeactivated => "Repository is deactivated".into(), + Self::AccountMigrated => { + "Account has been migrated to another PDS. Repo operations are not allowed.".into() } - Self::NoChallengeInProgress => Some( - "No passkey authentication in progress or challenge expired".to_string(), - ), - Self::InvalidCredential => Some("Failed to parse credential response".to_string()), - Self::NoRegistrationInProgress => Some( - "No registration in progress. Call startPasskeyRegistration first.".to_string(), - ), - Self::RegistrationFailed => { - Some("Failed to verify passkey registration".to_string()) + Self::AccountNotVerified => { + "You must verify at least one notification channel before creating records".into() } - Self::PasskeyNotFound => Some("Passkey not found".to_string()), - Self::InvalidId => Some("Invalid ID format".to_string()), - Self::InvalidScopes(msg) | Self::InvalidDelegation(msg) => Some(msg.clone()), - Self::ControllerNotFound => Some("Controller account not found".to_string()), + Self::NoPasskeys => "No passkeys registered for this account".into(), + Self::NoChallengeInProgress => { + "No passkey authentication in progress or challenge expired".into() + } + Self::InvalidCredential => "Failed to parse credential response".into(), + Self::NoRegistrationInProgress => { + "No registration in progress. Call startPasskeyRegistration first.".into() + } + Self::RegistrationFailed => "Failed to verify passkey registration".into(), + Self::PasskeyNotFound => "Passkey not found".into(), + Self::InvalidId => "Invalid ID format".into(), + Self::ControllerNotFound => "Controller account not found".into(), Self::DelegationNotFound => { - Some("No active delegation found for this controller".to_string()) + "No active delegation found for this controller".into() } Self::InviteCodeRequired => { - Some("An invite code is required to create an account".to_string()) + "An invite code is required to create an account".into() } - Self::RepoNotReady => Some("Repository not ready".to_string()), - Self::PasskeyCounterAnomaly => Some( - "Authentication failed: security key counter anomaly detected. This may indicate a cloned key.".to_string(), - ), - Self::MfaVerificationRequired => Some( - "This sensitive operation requires MFA verification".to_string(), - ), - Self::DeviceNotFound => Some("Device not found".to_string()), - Self::NoEmail => Some("Recipient has no email address".to_string()), - Self::AuthorizationError(msg) | Self::InvalidDid(msg) => Some(msg.clone()), + Self::RepoNotReady => "Repository not ready".into(), + Self::PasskeyCounterAnomaly => { + "Authentication failed: security key counter anomaly detected. This may indicate a cloned key.".into() + } + Self::MfaVerificationRequired => { + "This sensitive operation requires MFA verification".into() + } + Self::DeviceNotFound => "Device not found".into(), + Self::NoEmail => "Recipient has no email address".into(), Self::InvalidSigningKey => { - Some("Signing key not found, already used, or expired".to_string()) - } - Self::SetupExpired => { - Some("Setup has already been completed or expired".to_string()) - } - Self::InvalidAccount => { - Some("This account is not a passkey-only account".to_string()) - } - Self::InvalidRecoveryLink => Some("Invalid recovery link".to_string()), - Self::RecoveryLinkExpired => Some("Recovery link has expired".to_string()), - Self::MissingEmail => { - Some("Email is required when using email verification".to_string()) + "Signing key not found, already used, or expired".into() } + Self::SetupExpired => "Setup has already been completed or expired".into(), + Self::InvalidAccount => "This account is not a passkey-only account".into(), + Self::InvalidRecoveryLink => "Invalid recovery link".into(), + Self::RecoveryLinkExpired => "Recovery link has expired".into(), + Self::MissingEmail => "Email is required when using email verification".into(), Self::MissingDiscordId => { - Some("Discord ID is required when using Discord verification".to_string()) + "Discord ID is required when using Discord verification".into() } Self::MissingTelegramUsername => { - Some("Telegram username is required when using Telegram verification".to_string()) + "Telegram username is required when using Telegram verification".into() } Self::MissingSignalNumber => { - Some("Signal username is required when using Signal verification".to_string()) + "Signal username is required when using Signal verification".into() } - Self::InvalidVerificationChannel => Some("Invalid verification channel".to_string()), + Self::InvalidVerificationChannel => "Invalid verification channel".into(), Self::SelfHostedDidWebDisabled => { - Some("Self-hosted did:web accounts are disabled on this server".to_string()) - } - Self::AccountAlreadyExists => Some("Account already exists".to_string()), - Self::HandleNotFound => Some("Unable to resolve handle".to_string()), - Self::SubjectNotFound => Some("Subject not found".to_string()), - Self::SsoProviderNotFound => Some("Unknown SSO provider".to_string()), - Self::SsoProviderNotEnabled => Some("SSO provider is not enabled".to_string()), - Self::SsoInvalidAction => { - Some("Action must be login, link, or register".to_string()) + "Self-hosted did:web accounts are disabled on this server".into() } + Self::AccountAlreadyExists => "Account already exists".into(), + Self::HandleNotFound => "Unable to resolve handle".into(), + Self::SubjectNotFound => "Subject not found".into(), + Self::SsoProviderNotFound => "Unknown SSO provider".into(), + Self::SsoProviderNotEnabled => "SSO provider is not enabled".into(), + Self::SsoInvalidAction => "Action must be login, link, or register".into(), Self::SsoNotAuthenticated => { - Some("Must be authenticated to link SSO account".to_string()) + "Must be authenticated to link SSO account".into() } - Self::SsoSessionExpired => Some("SSO session expired or invalid".to_string()), + Self::SsoSessionExpired => "SSO session expired or invalid".into(), Self::SsoAlreadyLinked => { - Some("This SSO account is already linked to a different user".to_string()) + "This SSO account is already linked to a different user".into() } - Self::SsoLinkNotFound => Some("Linked account not found".to_string()), + Self::SsoLinkNotFound => "Linked account not found".into(), Self::IdentifierMismatch => { - Some("The identifier does not match the verification token".to_string()) + "The identifier does not match the verification token".into() + } + Self::UpstreamTimeout => "Upstream service timed out".into(), + Self::AdminRequired => "This action requires admin privileges".into(), + Self::EmailTaken => "This email address is already registered".into(), + Self::HandleTaken => "This handle is already taken".into(), + Self::InvalidEmail => "Please provide a valid email address".into(), + Self::InvalidInviteCode => "The invite code provided is invalid".into(), + Self::DuplicateCreate => "Account creation failed: duplicate request".into(), + Self::LegacyLoginBlocked => { + "This account requires MFA. Please use an OAuth client that supports TOTP verification.".into() } - Self::UpstreamError { message, .. } => message.clone(), - Self::UpstreamTimeout => Some("Upstream service timed out".to_string()), - Self::AdminRequired => Some("This action requires admin privileges".to_string()), - Self::EmailTaken => Some("This email address is already registered".to_string()), - Self::HandleTaken => Some("This handle is already taken".to_string()), - Self::InvalidEmail => Some("Please provide a valid email address".to_string()), - Self::InvalidInviteCode => Some("The invite code provided is invalid".to_string()), - Self::DuplicateCreate => Some("Account creation failed: duplicate request".to_string()), - Self::LegacyLoginBlocked => Some( - "This account requires MFA. Please use an OAuth client that supports TOTP verification.".to_string(), - ), Self::AuthFactorTokenRequired => { - Some("A sign in code has been sent to your email address".to_string()) + "A sign-in code has been sent to your email address".into() } - _ => None, } } pub fn from_upstream_response(status: StatusCode, body: &[u8]) -> Self { diff --git a/crates/tranquil-pds/src/api/proxy.rs b/crates/tranquil-pds/src/api/proxy.rs index 7a63b9a..90e090c 100644 --- a/crates/tranquil-pds/src/api/proxy.rs +++ b/crates/tranquil-pds/src/api/proxy.rs @@ -10,7 +10,7 @@ use axum::{ body::Bytes, extract::{RawQuery, Request, State}, handler::Handler, - http::{HeaderMap, Method, StatusCode}, + http::{HeaderMap, Method}, response::{IntoResponse, Response}, }; use futures_util::future::Either; @@ -335,7 +335,7 @@ async fn proxy_handler( Ok(b) => b, Err(e) => { error!("Error reading proxy response body: {:?}", e); - return (StatusCode::BAD_GATEWAY, "Error reading upstream response") + return ApiError::UpstreamUnavailable("Error reading upstream response".into()) .into_response(); } }; @@ -350,16 +350,16 @@ async fn proxy_handler( Ok(r) => r, Err(e) => { error!("Error building proxy response: {:?}", e); - (StatusCode::INTERNAL_SERVER_ERROR, "Internal Server Error").into_response() + ApiError::InternalError(None).into_response() } } } Err(e) => { error!("Error sending proxy request: {:?}", e); if e.is_timeout() { - (StatusCode::GATEWAY_TIMEOUT, "Upstream Timeout").into_response() + ApiError::UpstreamTimeout.into_response() } else { - (StatusCode::BAD_GATEWAY, "Upstream Error").into_response() + ApiError::UpstreamFailure.into_response() } } } diff --git a/crates/tranquil-pds/src/lib.rs b/crates/tranquil-pds/src/lib.rs index d9fe982..8aac451 100644 --- a/crates/tranquil-pds/src/lib.rs +++ b/crates/tranquil-pds/src/lib.rs @@ -100,7 +100,7 @@ pub fn app_with_routes(state: AppState, external: ExternalRoutes) -> Router { .layer(DefaultBodyLimit::max( tranquil_config::get().server.max_blob_size as usize, )) - .layer(axum::middleware::map_response(rewrite_422_to_400)) + .layer(axum::middleware::map_response(rewrite_extractor_errors)) .layer(middleware::from_fn(metrics::metrics_middleware)) .layer( CorsLayer::new() @@ -150,8 +150,24 @@ pub fn app_with_routes(state: AppState, external: ExternalRoutes) -> Router { router } -async fn rewrite_422_to_400(response: axum::response::Response) -> axum::response::Response { - if response.status() != StatusCode::UNPROCESSABLE_ENTITY { +fn is_plain_text(headers: &http::HeaderMap) -> bool { + headers + .get(http::header::CONTENT_TYPE) + .and_then(|v| v.to_str().ok()) + .is_some_and(|ct| ct.starts_with("text/plain")) +} + +fn should_rewrite_to_xrpc_error(response: &axum::response::Response) -> bool { + match response.status() { + StatusCode::UNPROCESSABLE_ENTITY => true, + StatusCode::BAD_REQUEST => is_plain_text(response.headers()), + StatusCode::UNSUPPORTED_MEDIA_TYPE => is_plain_text(response.headers()), + _ => false, + } +} + +async fn rewrite_extractor_errors(response: axum::response::Response) -> axum::response::Response { + if !should_rewrite_to_xrpc_error(&response) { return response; } let (mut parts, body) = response.into_parts(); @@ -173,11 +189,11 @@ async fn rewrite_422_to_400(response: axum::response::Response) -> axum::respons .unwrap_or_else(|| { String::from_utf8(bytes.to_vec()).unwrap_or_else(|_| "Invalid request body".into()) }); - let message = humanize_json_error(&raw); + let message = humanize_extraction_error(&raw); parts.status = StatusCode::BAD_REQUEST; parts.headers.remove(http::header::CONTENT_LENGTH); - let error_name = classify_deserialization_error(&raw); + let error_name = classify_extraction_error(&raw); let new_body = json!({ "error": error_name, "message": message @@ -188,7 +204,7 @@ async fn rewrite_422_to_400(response: axum::response::Response) -> axum::respons ) } -fn humanize_json_error(raw: &str) -> String { +fn humanize_extraction_error(raw: &str) -> String { if raw.contains("missing field") { raw.split("missing field `") .nth(1) @@ -201,14 +217,21 @@ fn humanize_json_error(raw: &str) -> String { "Invalid JSON syntax".to_string() } else if raw.contains("Content-Type") || raw.contains("content type") { "Content-Type must be application/json".to_string() + } else if raw.contains("Failed to parse") || raw.contains("expected ident") { + "Invalid JSON in request body".to_string() + } else if raw.contains("Failed to deserialize query string") { + raw.strip_prefix("Failed to deserialize query string: ") + .map(|rest| format!("Invalid query parameter: {}", rest)) + .unwrap_or_else(|| "Invalid query parameters".into()) } else { raw.to_string() } } -fn classify_deserialization_error(raw: &str) -> &'static str { +fn classify_extraction_error(raw: &str) -> &'static str { match raw { s if s.contains("invalid handle") => "InvalidHandle", + s if s.contains("invalid CID") || s.contains("invalid cid") => "InvalidRequest", _ => "InvalidRequest", } } diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index f934b27..f474f3e 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -86,6 +86,18 @@ import type { const API_BASE = "/xrpc"; +const STATUS_FALLBACK_MESSAGE: Record = { + 400: "Bad request", + 401: "Authentication required", + 403: "Forbidden", + 404: "Not found", + 429: "Rate limit exceeded", + 500: "Internal server error", + 502: "Bad gateway", + 503: "Service unavailable", + 504: "Gateway timeout", +}; + export class ApiError extends Error { public did?: Did; public reauthMethods?: string[]; @@ -96,7 +108,7 @@ export class ApiError extends Error { did?: string, reauthMethods?: string[], ) { - super(message); + super(message ?? STATUS_FALLBACK_MESSAGE[status] ?? "Request failed"); this.name = "ApiError"; this.did = did ? unsafeAsDid(did) : undefined; this.reauthMethods = reauthMethods;