diff --git a/crates/adapters/http-axum/src/errors.rs b/crates/adapters/http-axum/src/errors.rs index a212053a..d4ed68ce 100644 --- a/crates/adapters/http-axum/src/errors.rs +++ b/crates/adapters/http-axum/src/errors.rs @@ -7,7 +7,6 @@ use domain::errors::DomainError; pub const NOT_FOUND: &str = "NOT_FOUND"; pub const INVALID_INPUT: &str = "INVALID_INPUT"; -pub const CONFLICT: &str = "CONFLICT"; pub const UNAUTHORIZED: &str = "UNAUTHORIZED"; pub const FORBIDDEN: &str = "FORBIDDEN"; pub const RATE_LIMITED: &str = "RATE_LIMITED"; diff --git a/crates/adapters/http-axum/src/extractors/json.rs b/crates/adapters/http-axum/src/extractors/json.rs index 9974ec8d..27c8d994 100644 --- a/crates/adapters/http-axum/src/extractors/json.rs +++ b/crates/adapters/http-axum/src/extractors/json.rs @@ -11,10 +11,12 @@ use axum::{ }; use serde::de::DeserializeOwned; -use crate::errors::{INVALID_INPUT, refuse}; +use crate::errors::{INTERNAL, INVALID_INPUT, refuse}; /// Malformed request (bad JSON, query or path), refused as `INVALID_INPUT`. /// Status follows axum: 400 syntax, 415 content type, 422 data errors, 413 size. +/// A rejection with a 5xx status is a server-side fault (e.g. a route that +/// declares no path parameter), refused as `INTERNAL`. pub struct MalformedRequest { status: StatusCode, message: String, @@ -22,7 +24,12 @@ pub struct MalformedRequest { impl IntoResponse for MalformedRequest { fn into_response(self) -> Response { - refuse(self.status, INVALID_INPUT, self.message) + let code = if self.status.is_server_error() { + INTERNAL + } else { + INVALID_INPUT + }; + refuse(self.status, code, self.message) } } diff --git a/crates/adapters/sqlite/src/event_queue/mod.rs b/crates/adapters/sqlite/src/event_queue/mod.rs index 8479ac8a..18f260df 100644 --- a/crates/adapters/sqlite/src/event_queue/mod.rs +++ b/crates/adapters/sqlite/src/event_queue/mod.rs @@ -38,18 +38,18 @@ pub struct SqliteEventQueue { } impl SqliteEventQueue { - pub async fn create(pool: SqlitePool, config: DbEventQueueConfig) -> anyhow::Result { - Ok(Self { + pub fn create(pool: SqlitePool, config: DbEventQueueConfig) -> Self { + Self { pool, config: Arc::new(config), - }) + } } pub async fn create_publisher( pool: SqlitePool, config: &config::EventsConfig, ) -> anyhow::Result> { - let q = Self::create(pool, DbEventQueueConfig::from_config(config)).await?; + let q = Self::create(pool, DbEventQueueConfig::from_config(config)); Ok(Arc::new(q)) } @@ -57,7 +57,7 @@ impl SqliteEventQueue { pool: SqlitePool, config: &config::EventsConfig, ) -> anyhow::Result<(Arc, Arc)> { - let q = Self::create(pool, DbEventQueueConfig::from_config(config)).await?; + let q = Self::create(pool, DbEventQueueConfig::from_config(config)); Ok((Arc::new(q.clone()), Arc::new(q))) } } diff --git a/crates/application/tests/diary/edit_review_test.rs b/crates/application/tests/diary/edit_review_test.rs index da78fd4c..09b0d1b1 100644 --- a/crates/application/tests/diary/edit_review_test.rs +++ b/crates/application/tests/diary/edit_review_test.rs @@ -131,7 +131,7 @@ async fn edit_sets_watch_medium() { } #[tokio::test] -async fn editing_a_review_fails_when_the_store_refuses() { +async fn editing_a_review_fails_when_the_store_refuses_to_load_it() { let (_, events, review_id, user_id) = setup().await; let deps = EditReviewDeps { review: Arc::new(domain::testing::RefusingReviewRepository), diff --git a/crates/application/tests/users.rs b/crates/application/tests/users.rs index dfcc27c1..59c516c3 100644 --- a/crates/application/tests/users.rs +++ b/crates/application/tests/users.rs @@ -2,6 +2,8 @@ mod support; #[path = "users/authorize_admin_test.rs"] mod authorize_admin_test; +#[path = "users/delete_account_test.rs"] +mod delete_account_test; #[path = "users/get_current_profile_test.rs"] mod get_current_profile_test; #[path = "users/get_federated_profile_stats_test.rs"] diff --git a/crates/application/tests/users/delete_account_test.rs b/crates/application/tests/users/delete_account_test.rs new file mode 100644 index 00000000..d65e7e44 --- /dev/null +++ b/crates/application/tests/users/delete_account_test.rs @@ -0,0 +1,87 @@ +use crate::support::{TestContextBuilder, register_command}; +use std::sync::Arc; + +use domain::{ + errors::DomainError, + events::DomainEvent, + ports::UserRepository, + testing::{InMemoryStore, NoopEventPublisher, RefusingEventPublisher}, + value_objects::{Email, UserId}, +}; +use uuid::Uuid; + +use application::{ + auth::{deps::RegisterDeps, register}, + users::{delete_account, deps::DeleteAccountDeps}, +}; + +async fn register_user(b: &TestContextBuilder, users: &Arc) -> Uuid { + let reg_deps = RegisterDeps { + user: b.user_repo.clone(), + password_hasher: b.password_hasher.clone(), + config: b.config.clone(), + }; + register::execute(®_deps, register_command("alice@example.com", "alice")) + .await + .unwrap(); + users + .find_by_email(&Email::new("alice@example.com".into()).unwrap()) + .await + .unwrap() + .unwrap() + .id() + .value() +} + +#[tokio::test] +async fn deleting_an_unknown_user_is_not_found() { + let events = NoopEventPublisher::new(); + let b = TestContextBuilder::new(); + let deps = DeleteAccountDeps { + user: b.user_repo.clone(), + event_publisher: Arc::clone(&events) as _, + }; + + let err = delete_account::execute(&deps, Uuid::new_v4()) + .await + .unwrap_err(); + + assert!(matches!(err, DomainError::NotFound(_))); + assert!(events.published().is_empty()); +} + +#[tokio::test] +async fn deleting_an_existing_user_publishes_user_deleted() { + let users = InMemoryStore::new(); + let events = NoopEventPublisher::new(); + let b = TestContextBuilder::new().with_users(Arc::clone(&users) as _); + let deps = DeleteAccountDeps { + user: b.user_repo.clone(), + event_publisher: Arc::clone(&events) as _, + }; + let uid = register_user(&b, &users).await; + + delete_account::execute(&deps, uid).await.unwrap(); + + let published = events.published(); + assert!( + matches!( + published.as_slice(), + [DomainEvent::UserDeleted { user_id }] if *user_id == UserId::from_uuid(uid) + ), + "published: {published:?}" + ); +} + +#[tokio::test] +async fn deleting_fails_when_the_publisher_refuses() { + let users = InMemoryStore::new(); + let b = TestContextBuilder::new().with_users(Arc::clone(&users) as _); + let deps = DeleteAccountDeps { + user: b.user_repo.clone(), + event_publisher: Arc::new(RefusingEventPublisher), + }; + let uid = register_user(&b, &users).await; + + assert!(delete_account::execute(&deps, uid).await.is_err()); +} diff --git a/crates/bootstrap/Cargo.toml b/crates/bootstrap/Cargo.toml index 35303588..f580ea8e 100644 --- a/crates/bootstrap/Cargo.toml +++ b/crates/bootstrap/Cargo.toml @@ -17,7 +17,7 @@ federation = [ config = { workspace = true } domain = { workspace = true } application = { workspace = true } -http-axum = { path = "../adapters/http-axum", default-features = false } +http-axum = { workspace = true, default-features = false } auth = { workspace = true } metadata = { workspace = true } poster-fetcher = { workspace = true } diff --git a/crates/bootstrap/src/build.rs b/crates/bootstrap/src/build.rs index b4ee6116..4fdde3d9 100644 --- a/crates/bootstrap/src/build.rs +++ b/crates/bootstrap/src/build.rs @@ -44,7 +44,7 @@ use application::wrapup::deps::{ }; use domain::value_objects::InstanceIdentity; -use crate::{DatabaseOutput, Repositories}; +use crate::{Repositories, database::DatabaseOutput}; /// The composition root's single assembly point: every one of the server-facing /// deps structs, built once from the already-constructed `Repositories` and diff --git a/crates/bootstrap/src/database.rs b/crates/bootstrap/src/database.rs index bad5da5e..89ef2cce 100644 --- a/crates/bootstrap/src/database.rs +++ b/crates/bootstrap/src/database.rs @@ -2,16 +2,11 @@ use std::sync::Arc; use anyhow::Context; use domain::ports::{ - AuthService, ImageRefCommand, ImageRefQuery, LocalApContentQuery, MetadataClient, - MovieDeduplicator, ObjectStorage, PasswordHasher, PosterFetcherClient, - RefreshSessionRepository, UserProfileFieldsRepository, WatchEventCommand, WatchEventQuery, - WebhookTokenRepository, + AuthService, ImageRefCommand, ImageRefQuery, MetadataClient, MovieDeduplicator, ObjectStorage, + PasswordHasher, PosterFetcherClient, RefreshSessionRepository, UserProfileFieldsRepository, + WatchEventCommand, WatchEventQuery, WebhookTokenRepository, }; -pub enum DbPool { - Sqlite(sqlx::SqlitePool), -} - pub struct DatabaseOutput { pub movie_command: Arc, pub movie_query: Arc, @@ -31,13 +26,15 @@ pub struct DatabaseOutput { pub search_port: Arc, pub search_command: Arc, pub profile_fields: Arc, - pub ap_content: Arc, + #[cfg(feature = "federation")] + pub ap_content: Arc, pub wrapup_stats: Arc, pub wrapup_repo: Arc, pub insights: Arc, pub goal_command: Arc, pub goal_query: Arc, pub user_settings: Arc, + #[cfg(feature = "federation")] pub federation_settings: std::sync::Arc, pub remote_goal: Arc, pub refresh_session: Arc, @@ -46,7 +43,7 @@ pub struct DatabaseOutput { pub image_ref_query: Arc, pub follow_command: Arc, pub follow_query: Arc, - pub db_pool: DbPool, + pub db_pool: sqlx::SqlitePool, } pub async fn build_database_adapters( @@ -105,6 +102,7 @@ fn assemble( search_port: sp, search_command: sc, profile_fields: pf, + #[cfg(feature = "federation")] ap_content: w.ap_content, wrapup_stats: w.wrapup_stats, wrapup_repo: w.wrapup_repo, @@ -112,6 +110,7 @@ fn assemble( goal_command: w.goal_command, goal_query: w.goal_query, user_settings: w.user_settings, + #[cfg(feature = "federation")] federation_settings: w.federation_settings, remote_goal: w.remote_goal, refresh_session: Arc::new(sqlite::SqliteRefreshSessionAdapter::new(w.pool.clone())) as _, @@ -120,7 +119,7 @@ fn assemble( image_ref_query, follow_command: Arc::clone(&social) as _, follow_query: social as _, - db_pool: DbPool::Sqlite(w.pool), + db_pool: w.pool, }) } @@ -148,14 +147,6 @@ pub fn build_object_storage( object_storage::create(config) } -pub fn build_profile_fields_repo( - pool: &DbPool, -) -> anyhow::Result> { - match pool { - DbPool::Sqlite(pool) => Ok(sqlite::create_profile_fields_repo(pool.clone())), - } -} - pub fn warn_if_database_backend_set() { if let Ok(v) = std::env::var("DATABASE_BACKEND") && v != "sqlite" diff --git a/crates/bootstrap/src/event_bus.rs b/crates/bootstrap/src/event_bus.rs index b4ace78a..222832d2 100644 --- a/crates/bootstrap/src/event_bus.rs +++ b/crates/bootstrap/src/event_bus.rs @@ -4,8 +4,6 @@ use std::sync::Arc; use anyhow::Context; use domain::ports::{EventConsumer, EventPublisher}; -use crate::database::DbPool; - #[derive(Clone, Copy)] pub enum EventBusBackend { Db, @@ -29,17 +27,13 @@ impl EventBusBackend { } pub async fn create_publisher( - db_pool: &DbPool, + db_pool: &sqlx::SqlitePool, config: &config::AppConfig, ) -> anyhow::Result> { match EventBusBackend::from_config(&config.events)? { EventBusBackend::Db => { tracing::info!("event bus: DB queue"); - Ok(match db_pool { - DbPool::Sqlite(pool) => { - sqlite::SqliteEventQueue::create_publisher(pool.clone(), &config.events).await? - } - }) + Ok(sqlite::SqliteEventQueue::create_publisher(db_pool.clone(), &config.events).await?) } #[cfg(feature = "nats")] EventBusBackend::Nats => { @@ -52,19 +46,13 @@ pub async fn create_publisher( } pub async fn create_channel( - db_pool: &DbPool, + db_pool: &sqlx::SqlitePool, config: &config::AppConfig, ) -> anyhow::Result<(Arc, Arc)> { match EventBusBackend::from_config(&config.events)? { EventBusBackend::Db => { tracing::info!("event bus: DB queue"); - match db_pool { - DbPool::Sqlite(pool) => Ok(sqlite::SqliteEventQueue::create_channel( - pool.clone(), - &config.events, - ) - .await?), - } + Ok(sqlite::SqliteEventQueue::create_channel(db_pool.clone(), &config.events).await?) } #[cfg(feature = "nats")] EventBusBackend::Nats => { diff --git a/crates/bootstrap/src/federation.rs b/crates/bootstrap/src/federation.rs index 941b9c9d..7517a38f 100644 --- a/crates/bootstrap/src/federation.rs +++ b/crates/bootstrap/src/federation.rs @@ -64,8 +64,7 @@ pub async fn wire_server_social( config: &config::AppConfig, event_publisher: &Arc, ) -> anyhow::Result { - let crate::database::DbPool::Sqlite(pool) = &db.db_pool; - let fed_repos = sqlite_federation::wire(pool.clone(), instance.clone()); + let fed_repos = sqlite_federation::wire(db.db_pool.clone(), instance.clone()); let ap = activitypub::wire(activitypub::ActivityPubDeps { activity_repo: fed_repos.activity, @@ -117,7 +116,7 @@ pub async fn wire_server_social( remote_watchlist: fed_repos.remote_watchlist, federation_admin: fed_repos.admin_query, federated_profile: Some(sqlite::create_federated_profile_query( - pool.clone(), + db.db_pool.clone(), instance.clone(), )), }) @@ -132,8 +131,7 @@ pub async fn wire_worker_handlers( config: &config::AppConfig, event_publisher: &Arc, ) -> anyhow::Result>> { - let crate::database::DbPool::Sqlite(pool) = &db.db_pool; - let fed_repos = sqlite_federation::wire(pool.clone(), instance.clone()); + let fed_repos = sqlite_federation::wire(db.db_pool.clone(), instance.clone()); let ap_wire = activitypub::wire(activitypub::ActivityPubDeps { activity_repo: fed_repos.activity, diff --git a/crates/bootstrap/src/lib.rs b/crates/bootstrap/src/lib.rs index 5ac674ef..58711843 100644 --- a/crates/bootstrap/src/lib.rs +++ b/crates/bootstrap/src/lib.rs @@ -4,8 +4,8 @@ //! [`build_worker`]; everything between configuration and a running service is //! assembled here (ADR-0005, ADR-0013). -pub mod build; -pub mod database; +mod build; +mod database; mod event_bus; mod federation; #[cfg(feature = "federation")] @@ -15,9 +15,7 @@ mod server; mod tracing_setup; mod worker; -pub use build::{build_deps, build_worker_deps}; -pub use database::{DatabaseOutput, DbPool}; -pub use event_bus::EventBusBackend; +pub use build::build_deps; pub use repositories::Repositories; pub use server::{ServerApp, build_server, build_server_on}; pub use tracing_setup::init_tracing; diff --git a/crates/bootstrap/src/worker.rs b/crates/bootstrap/src/worker.rs index 7313feda..1c49dce9 100644 --- a/crates/bootstrap/src/worker.rs +++ b/crates/bootstrap/src/worker.rs @@ -66,7 +66,7 @@ async fn assemble( event_publisher: Arc::clone(&event_publisher_arc), person_enrichment, }; - let worker_deps = crate::build_worker_deps(&db, &worker_services); + let worker_deps = crate::build::build_worker_deps(&db, &worker_services); let image_ref_command = Arc::clone(&db.image_ref_command); let image_ref_query = Arc::clone(&db.image_ref_query); diff --git a/crates/bootstrap/tests/config_propagation_test.rs b/crates/bootstrap/tests/config_propagation_test.rs new file mode 100644 index 00000000..f7a4af4f --- /dev/null +++ b/crates/bootstrap/tests/config_propagation_test.rs @@ -0,0 +1,103 @@ +//! Configuration values reach the running app through the composition root. + +mod support; + +use axum::{ + body::Body, + http::{Method, Request, header}, +}; + +async fn server_with(config: config::AppConfig) -> axum::Router { + bootstrap::build_server_on(&config, support::memory_pool().await) + .await + .expect("server builds") +} + +fn set_cookies(headers: &axum::http::HeaderMap) -> Vec { + headers + .get_all(header::SET_COOKIE) + .iter() + .map(|v| v.to_str().unwrap().to_string()) + .collect() +} + +#[tokio::test] +async fn registration_is_refused_when_allow_registration_is_false() { + let mut config = support::test_config(); + config.server.allow_registration = false; + let router = server_with(config).await; + + let form = Request::builder() + .method(Method::POST) + .uri("/register") + .header(header::CONTENT_TYPE, "application/x-www-form-urlencoded") + .body(Body::from(format!( + "email=a%40example.com&username=alice&password={}&_csrf=x", + support::PASSWORD + ))) + .unwrap(); + let (status, headers, _) = support::send(&router, form).await; + assert_eq!(status, 303, "classic form redirects away"); + assert_eq!(headers.get(header::LOCATION).unwrap(), "/"); + assert!( + support::cookie_value(&headers, "token").is_none(), + "no session issued" + ); + + let (status, _, _) = support::send( + &router, + support::json_request( + Method::POST, + "/api/v1/auth/register", + None, + serde_json::json!({ + "email": "a@example.com", + "username": "alice", + "password": support::PASSWORD, + }), + ), + ) + .await; + assert_eq!(status, 401, "JSON register is refused"); +} + +#[tokio::test] +async fn secure_cookies_setting_marks_cookies_secure() { + for secure in [true, false] { + let mut config = support::test_config(); + config.server.secure_cookies = secure; + let router = server_with(config).await; + + let (status, headers, _) = support::send(&router, support::get("/register", None)).await; + assert_eq!(status, 200); + let cookies = set_cookies(&headers); + assert!(!cookies.is_empty(), "csrf cookie is set"); + assert!( + cookies.iter().all(|c| c.contains("Secure") == secure), + "secure_cookies={secure}: {cookies:?}" + ); + + let csrf = support::cookie_value(&headers, "csrf").expect("csrf cookie"); + let form = Request::builder() + .method(Method::POST) + .uri("/register") + .header(header::CONTENT_TYPE, "application/x-www-form-urlencoded") + .header(header::COOKIE, format!("csrf={csrf}")) + .body(Body::from(format!( + "email=a%40example.com&username=alice&password={}&_csrf={csrf}", + support::PASSWORD + ))) + .unwrap(); + let (status, headers, _) = support::send(&router, form).await; + assert_eq!(status, 303); + let token_cookie = set_cookies(&headers) + .into_iter() + .find(|c| c.starts_with("token=")) + .expect("session cookie"); + assert_eq!( + token_cookie.contains("Secure"), + secure, + "session cookie, secure_cookies={secure}: {token_cookie}" + ); + } +} diff --git a/crates/bootstrap/tests/http_handlers/extractors_test.rs b/crates/bootstrap/tests/http_handlers/extractors_test.rs index e49fd9a0..d2d727e5 100644 --- a/crates/bootstrap/tests/http_handlers/extractors_test.rs +++ b/crates/bootstrap/tests/http_handlers/extractors_test.rs @@ -359,3 +359,49 @@ async fn admin_user_surfaces_a_repository_error_as_500() { .unwrap(); assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR); } + +async fn path_id_handler( + http_axum::extractors::PathId(id): http_axum::extractors::PathId, +) -> String { + id.to_string() +} + +async fn refusal_code(resp: axum::response::Response) -> String { + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + let v: serde_json::Value = serde_json::from_slice(&bytes).expect("refusal body is JSON"); + v["code"].as_str().unwrap().to_string() +} + +#[tokio::test] +async fn path_extractor_on_a_route_without_params_is_refused_as_internal() { + let app = Router::new().route("/no-params", get(path_id_handler)); + let resp = app + .oneshot( + Request::builder() + .uri("/no-params") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR); + assert_eq!(refusal_code(resp).await, "INTERNAL"); +} + +#[tokio::test] +async fn path_extractor_with_an_unparseable_param_is_refused_as_invalid_input() { + let app = Router::new().route("/items/{id}", get(path_id_handler)); + let resp = app + .oneshot( + Request::builder() + .uri("/items/abc") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); + assert_eq!(refusal_code(resp).await, "INVALID_INPUT"); +} diff --git a/docs/adr/0014-http-adapter-and-one-refusal-shape.md b/docs/adr/0014-http-adapter-and-one-refusal-shape.md index fefc0ca4..88928e42 100644 --- a/docs/adr/0014-http-adapter-and-one-refusal-shape.md +++ b/docs/adr/0014-http-adapter-and-one-refusal-shape.md @@ -16,7 +16,7 @@ rate limiter's own 429 text. The refusals were documented without a body schema. **One body.** `api_types::ErrorResponse { error, code }`. `error` keeps its name and meaning (the SPA and existing clients read it); `code` is an additive, stable machine-readable field: -`NOT_FOUND`, `INVALID_INPUT`, `CONFLICT`, `UNAUTHORIZED`, `FORBIDDEN`, `RATE_LIMITED`, `INTERNAL`. +`NOT_FOUND`, `INVALID_INPUT`, `UNAUTHORIZED`, `FORBIDDEN`, `RATE_LIMITED`, `INTERNAL`. **One constructor.** `errors::refuse(status, code, message) -> Response` is the only place a JSON error body is built. `ApiError` maps every `DomainError` variant to it with the status mapping that