diff --git a/crates/adapters/http-axum/src/errors.rs b/crates/adapters/http-axum/src/errors.rs index d4ed68ce..16f9fa29 100644 --- a/crates/adapters/http-axum/src/errors.rs +++ b/crates/adapters/http-axum/src/errors.rs @@ -9,6 +9,7 @@ pub const NOT_FOUND: &str = "NOT_FOUND"; pub const INVALID_INPUT: &str = "INVALID_INPUT"; pub const UNAUTHORIZED: &str = "UNAUTHORIZED"; pub const FORBIDDEN: &str = "FORBIDDEN"; +pub const CONFLICT: &str = "CONFLICT"; pub const RATE_LIMITED: &str = "RATE_LIMITED"; pub const INTERNAL: &str = "INTERNAL"; @@ -32,6 +33,7 @@ pub fn domain_error_status(e: &DomainError) -> StatusCode { DomainError::NotFound(_) => StatusCode::NOT_FOUND, DomainError::Unauthorized(_) => StatusCode::UNAUTHORIZED, DomainError::Forbidden(_) => StatusCode::FORBIDDEN, + DomainError::Conflict(_) => StatusCode::CONFLICT, DomainError::InfrastructureError(_) => StatusCode::INTERNAL_SERVER_ERROR, } } @@ -64,6 +66,7 @@ impl IntoResponse for ApiError { DomainError::NotFound(_) => refuse(status, NOT_FOUND, self.0.to_string()), DomainError::Unauthorized(_) => refuse(status, UNAUTHORIZED, self.0.to_string()), DomainError::Forbidden(_) => refuse(status, FORBIDDEN, self.0.to_string()), + DomainError::Conflict(_) => refuse(status, CONFLICT, self.0.to_string()), DomainError::InfrastructureError(_) => { tracing::error!("Internal error: {:?}", self.0); refuse(status, INTERNAL, "internal server error") diff --git a/crates/adapters/http-axum/src/extractors/mod.rs b/crates/adapters/http-axum/src/extractors/mod.rs index 539c7222..3d01efce 100644 --- a/crates/adapters/http-axum/src/extractors/mod.rs +++ b/crates/adapters/http-axum/src/extractors/mod.rs @@ -8,6 +8,11 @@ use axum::{ }; use domain::{errors::DomainError, value_objects::UserId}; +async fn ensure_active(state: &AppState, user_id: &UserId) -> Result<(), DomainError> { + application::users::ensure_active::execute(&state.app_ctx.deps.users.ensure_active, user_id) + .await +} + use crate::{errors::ApiError, state::AppState}; pub struct AuthenticatedUser(pub UserId); @@ -37,6 +42,7 @@ where .auth .validate_token(token) .await?; + ensure_active(&app_state, &user_id).await?; Ok(AuthenticatedUser(user_id)) } } @@ -75,6 +81,10 @@ where .validate_token(&token) .await .ok(); + let user_id = match user_id { + Some(id) if ensure_active(&app_state, &id).await.is_ok() => Some(id), + _ => None, + }; Ok(OptionalCookieUser(user_id)) } } @@ -97,6 +107,14 @@ where .validate_token(&token) .await .map_err(|_| Redirect::to("/login").into_response())?; + ensure_active(&app_state, &user_id) + .await + .map_err(|e| match e { + DomainError::InfrastructureError(_) => { + StatusCode::INTERNAL_SERVER_ERROR.into_response() + } + _ => Redirect::to("/login").into_response(), + })?; Ok(RequiredCookieUser(user_id)) } } diff --git a/crates/adapters/sqlite/src/repositories/image_ref/mod.rs b/crates/adapters/sqlite/src/repositories/image_ref/mod.rs index 0a1397c5..5601b667 100644 --- a/crates/adapters/sqlite/src/repositories/image_ref/mod.rs +++ b/crates/adapters/sqlite/src/repositories/image_ref/mod.rs @@ -26,6 +26,8 @@ pub fn create_image_ref(pool: SqlitePool) -> (Arc, Arc Result<(), DomainError> { let mut tx = self .pool @@ -52,6 +54,8 @@ impl ImageRefCommand for SqliteImageRefAdapter { #[async_trait] impl ImageRefQuery for SqliteImageRefAdapter { + // Tombstoned users are intentionally visible here until the purge removes + // the row, so their avatar is not garbage-collected early. async fn list_keys(&self) -> Result, DomainError> { let rows: Vec<(String,)> = sqlx::query_as( "SELECT avatar_path FROM users WHERE avatar_path IS NOT NULL diff --git a/crates/adapters/sqlite/src/repositories/users/mod.rs b/crates/adapters/sqlite/src/repositories/users/mod.rs index 4abb6cf7..3ffd2d10 100644 --- a/crates/adapters/sqlite/src/repositories/users/mod.rs +++ b/crates/adapters/sqlite/src/repositories/users/mod.rs @@ -171,7 +171,7 @@ impl UserRepository for SqliteUserRepository { .map_err(crate::shared::map_sqlx_error)?; let stamped = sqlx::query("UPDATE users SET deleted_at = ? WHERE id = ? AND deleted_at IS NULL") - .bind(Utc::now().to_rfc3339()) + .bind(Utc::now().format("%Y-%m-%dT%H:%M:%SZ").to_string()) .bind(&id_str) .execute(&mut *tx) .await diff --git a/crates/adapters/sqlite/tests/repositories/social_repository_test.rs b/crates/adapters/sqlite/tests/repositories/social_repository_test.rs index b27ba033..6e805266 100644 --- a/crates/adapters/sqlite/tests/repositories/social_repository_test.rs +++ b/crates/adapters/sqlite/tests/repositories/social_repository_test.rs @@ -288,3 +288,40 @@ async fn follow_lists_omit_deleted_local_users() { |a| a.identity == SocialIdentity::Local(domain::value_objects::UserId::from_uuid(gone)) )); } + +#[tokio::test] +async fn follower_lists_omit_deleted_local_users() { + let pool = test_pool().await; + let viewer = uuid::Uuid::new_v4(); + let gone = uuid::Uuid::new_v4(); + let live = uuid::Uuid::new_v4(); + sqlx::query( + "INSERT INTO users (id, username, deleted_at) VALUES (?1, 'gone', '2026-01-01T00:00:00Z'), (?2, 'live', NULL)", + ) + .bind(gone.to_string()) + .bind(live.to_string()) + .execute(&pool) + .await + .unwrap(); + sqlx::query( + "INSERT INTO ap_followers (local_user_id, remote_actor_url, follow_activity_id, status) + VALUES (?1, 'https://md.example/users/' || ?2, '', 'accepted'), + (?1, 'https://md.example/users/' || ?3, '', 'accepted'), + (?1, 'https://other.example/users/remote', '', 'accepted')", + ) + .bind(viewer.to_string()) + .bind(gone.to_string()) + .bind(live.to_string()) + .execute(&pool) + .await + .unwrap(); + + let actors = FollowQuery::get_followers(&repo(pool), viewer) + .await + .unwrap(); + + assert_eq!(actors.len(), 2, "deleted local follower must not be listed"); + assert!(!actors.iter().any( + |a| a.identity == SocialIdentity::Local(domain::value_objects::UserId::from_uuid(gone)) + )); +} diff --git a/crates/application/src/auth/register.rs b/crates/application/src/auth/register.rs index b19ae607..8cf3cb1b 100644 --- a/crates/application/src/auth/register.rs +++ b/crates/application/src/auth/register.rs @@ -21,7 +21,9 @@ pub async fn execute(deps: &RegisterDeps, cmd: RegisterCommand) -> Result<(), Do )); } - if deps.user.find_by_username(&username).await?.is_some() { + if deps.user.find_by_username(&username).await?.is_some() + || deps.user.is_username_retired(&username).await? + { return Err(DomainError::ValidationError( "Username already taken".into(), )); diff --git a/crates/application/src/deps.rs b/crates/application/src/deps.rs index d5233cf2..cb305d6c 100644 --- a/crates/application/src/deps.rs +++ b/crates/application/src/deps.rs @@ -26,10 +26,10 @@ use crate::person::deps::{EnrichPersonDeps, GetPersonDeps}; use crate::search::deps::SearchDeps; use crate::social::deps::{SocialCommandDeps, SocialQueryDeps}; use crate::users::deps::{ - AuthorizeAdminDeps, DeleteAccountDeps, GetCurrentProfileDeps, GetFederatedProfileDeps, + AuthorizeAdminDeps, EnsureActiveDeps, GetCurrentProfileDeps, GetFederatedProfileDeps, GetFederatedProfileStatsDeps, GetLocalProfileDeps, GetPageViewerDeps, GetProfileSettingsDeps, - GetSettingsDeps, GetUsersListDeps, ResolveUsernameDeps, UpdateProfileDeps, - UpdateProfileFieldsDeps, UpdateSettingsDeps, + GetSettingsDeps, GetUsersListDeps, RequestAccountDeletionDeps, ResolveUsernameDeps, + UpdateProfileDeps, UpdateProfileFieldsDeps, UpdateSettingsDeps, }; use crate::watchlist::deps::{ GetWatchlistDeps, GetWatchlistForOwnerDeps, IsOnWatchlistDeps, RemoveFromWatchlistDeps, @@ -124,8 +124,8 @@ pub struct UsersGroup { pub get_profile_settings: GetProfileSettingsDeps, pub get_users_list: GetUsersListDeps, pub update_profile: UpdateProfileDeps, - /// Not reachable from the server binary; see the `Deps`-level note above. Pre-existing dead use case: `users::delete_account::execute` has zero callers anywhere in the workspace, worker included. - pub delete_account: DeleteAccountDeps, + pub request_account_deletion: RequestAccountDeletionDeps, + pub ensure_active: EnsureActiveDeps, pub get_current_profile: GetCurrentProfileDeps, pub update_profile_fields: UpdateProfileFieldsDeps, pub get_settings: GetSettingsDeps, diff --git a/crates/application/src/users/delete_account.rs b/crates/application/src/users/delete_account.rs deleted file mode 100644 index 51b1e130..00000000 --- a/crates/application/src/users/delete_account.rs +++ /dev/null @@ -1,19 +0,0 @@ -use domain::{errors::DomainError, events::DomainEvent, value_objects::UserId}; - -use crate::users::deps::DeleteAccountDeps; - -pub async fn execute(deps: &DeleteAccountDeps, user_id: uuid::Uuid) -> Result<(), DomainError> { - let uid = UserId::from_uuid(user_id); - - deps.user - .find_by_id(&uid) - .await? - .ok_or_else(|| DomainError::NotFound("User not found".into()))?; - - // Notify federation peers before any data is removed so they can process the tombstone. - deps.event_publisher - .publish(&DomainEvent::UserDeleted { user_id: uid }) - .await?; - - Ok(()) -} diff --git a/crates/application/src/users/deps.rs b/crates/application/src/users/deps.rs index f60dd06f..7fe09032 100644 --- a/crates/application/src/users/deps.rs +++ b/crates/application/src/users/deps.rs @@ -2,7 +2,7 @@ use std::sync::Arc; use domain::ports::{ DiaryQuery, EventPublisher, FederatedProfileQuery, FederationAdminQuery, FollowGraphQuery, - ObjectStorage, StatsRepository, UserProfileFieldsRepository, UserRepository, + ObjectStorage, PasswordHasher, StatsRepository, UserProfileFieldsRepository, UserRepository, UserSettingsRepository, }; use domain::value_objects::InstanceIdentity; @@ -47,9 +47,13 @@ pub struct UpdateProfileDeps { pub event_publisher: Arc, } -pub struct DeleteAccountDeps { +pub struct RequestAccountDeletionDeps { + pub user: Arc, + pub password_hasher: Arc, +} + +pub struct EnsureActiveDeps { pub user: Arc, - pub event_publisher: Arc, } pub struct GetCurrentProfileDeps { diff --git a/crates/application/src/users/ensure_active.rs b/crates/application/src/users/ensure_active.rs new file mode 100644 index 00000000..6a448247 --- /dev/null +++ b/crates/application/src/users/ensure_active.rs @@ -0,0 +1,12 @@ +use domain::{errors::DomainError, value_objects::UserId}; + +use crate::users::deps::EnsureActiveDeps; + +/// A token outlives its account; this is the check that stops a deleted +/// (or vanished) user from acting on one. +pub async fn execute(deps: &EnsureActiveDeps, user: &UserId) -> Result<(), DomainError> { + match deps.user.find_by_id(user).await? { + Some(_) => Ok(()), + None => Err(DomainError::Unauthorized("account is not active".into())), + } +} diff --git a/crates/application/src/users/mod.rs b/crates/application/src/users/mod.rs index b60d9f7c..d83e3799 100644 --- a/crates/application/src/users/mod.rs +++ b/crates/application/src/users/mod.rs @@ -1,8 +1,8 @@ pub mod authorize_admin; pub mod commands; -pub mod delete_account; pub mod deps; mod diary_filter; +pub mod ensure_active; pub mod get_current_profile; pub mod get_federated_profile; pub mod get_federated_profile_stats; @@ -12,6 +12,7 @@ pub mod get_profile_settings; pub mod get_settings; pub mod get_users; pub mod queries; +pub mod request_account_deletion; pub mod resolve_username_to_id; pub mod update_profile; pub mod update_profile_fields; diff --git a/crates/application/src/users/request_account_deletion.rs b/crates/application/src/users/request_account_deletion.rs new file mode 100644 index 00000000..2da7e595 --- /dev/null +++ b/crates/application/src/users/request_account_deletion.rs @@ -0,0 +1,60 @@ +use domain::{errors::DomainError, models::UserRole, value_objects::UserId}; + +use crate::users::deps::RequestAccountDeletionDeps; + +/// Tombstones `target`. A user may delete themselves; an admin may delete +/// anyone. Either way the *actor's* password is the proof, and the last +/// active admin can never be removed. `UserDeleted` rides the repository +/// write, so the tombstone and its event commit together. +pub async fn execute( + deps: &RequestAccountDeletionDeps, + actor: UserId, + target: UserId, + password: &str, +) -> Result<(), DomainError> { + let actor_user = deps + .user + .find_by_id(&actor) + .await? + .ok_or_else(|| DomainError::Unauthorized("account is not active".into()))?; + + let is_self = actor == target; + if !is_self && !matches!(actor_user.role(), UserRole::Admin) { + return Err(DomainError::Forbidden( + "only an admin can delete another account".into(), + )); + } + + if !deps + .password_hasher + .verify(password, actor_user.password_hash()) + .await? + { + return Err(DomainError::Unauthorized("incorrect password".into())); + } + + let target_user = if is_self { + actor_user + } else { + deps.user + .find_by_id(&target) + .await? + .ok_or_else(|| DomainError::NotFound("User not found".into()))? + }; + + if matches!(target_user.role(), UserRole::Admin) && deps.user.count_active_admins().await? <= 1 + { + return Err(DomainError::Conflict( + "the last active admin cannot be deleted".into(), + )); + } + + deps.user + .mark_deleted( + &target, + &[domain::events::DomainEvent::UserDeleted { + user_id: target.clone(), + }], + ) + .await +} diff --git a/crates/application/tests/auth/register_test.rs b/crates/application/tests/auth/register_test.rs index e28f30be..cb93db58 100644 --- a/crates/application/tests/auth/register_test.rs +++ b/crates/application/tests/auth/register_test.rs @@ -71,3 +71,34 @@ async fn registering_with_a_short_password_fails() { "expected password length error, got: {err}" ); } + +#[tokio::test] +async fn registration_refuses_a_retired_username() { + let users = InMemoryStore::new(); + let b = TestContextBuilder::new().with_users(Arc::clone(&users) as _); + let deps = RegisterDeps { + user: b.user_repo.clone(), + password_hasher: b.password_hasher.clone(), + config: b.config.clone(), + }; + register::execute(&deps, register_command("old@example.com", "ghost")) + .await + .unwrap(); + let old = users + .find_by_email(&Email::new("old@example.com".into()).unwrap()) + .await + .unwrap() + .unwrap(); + users.mark_deleted(old.id(), &[]).await.unwrap(); + // The purge removes the row; only the retired name keeps the username taken. + users.delete_user(old.id()); + + let err = register::execute(&deps, register_command("new@example.com", "Ghost")) + .await + .unwrap_err(); + + assert!( + matches!(&err, domain::errors::DomainError::ValidationError(m) if m == "Username already taken"), + "got {err:?}" + ); +} diff --git a/crates/application/tests/users.rs b/crates/application/tests/users.rs index 59c516c3..4cd6ff02 100644 --- a/crates/application/tests/users.rs +++ b/crates/application/tests/users.rs @@ -2,8 +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/ensure_active_test.rs"] +mod ensure_active_test; #[path = "users/get_current_profile_test.rs"] mod get_current_profile_test; #[path = "users/get_federated_profile_stats_test.rs"] @@ -20,6 +20,8 @@ mod get_profile_settings_test; mod get_settings_test; #[path = "users/get_users_test.rs"] mod get_users_test; +#[path = "users/request_account_deletion_test.rs"] +mod request_account_deletion_test; #[path = "users/resolve_username_to_id_test.rs"] mod resolve_username_to_id_test; #[path = "users/update_profile_fields_test.rs"] diff --git a/crates/application/tests/users/delete_account_test.rs b/crates/application/tests/users/delete_account_test.rs deleted file mode 100644 index d65e7e44..00000000 --- a/crates/application/tests/users/delete_account_test.rs +++ /dev/null @@ -1,87 +0,0 @@ -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/application/tests/users/ensure_active_test.rs b/crates/application/tests/users/ensure_active_test.rs new file mode 100644 index 00000000..ab8d4a0f --- /dev/null +++ b/crates/application/tests/users/ensure_active_test.rs @@ -0,0 +1,84 @@ +use std::sync::Arc; + +use domain::{ + errors::DomainError, + ports::UserRepository, + testing::InMemoryStore, + value_objects::{Email, UserId}, +}; + +use application::{ + auth::{deps::RegisterDeps, register}, + users::{deps::EnsureActiveDeps, ensure_active}, +}; + +use crate::support::{TestContextBuilder, register_command}; + +#[tokio::test] +async fn a_live_user_is_active() { + let store = InMemoryStore::new(); + let b = TestContextBuilder::new().with_users(Arc::clone(&store) as _); + let reg = RegisterDeps { + user: b.user_repo.clone(), + password_hasher: b.password_hasher.clone(), + config: b.config.clone(), + }; + register::execute(®, register_command("alice@example.com", "alice")) + .await + .unwrap(); + let id = store + .find_by_email(&Email::new("alice@example.com".into()).unwrap()) + .await + .unwrap() + .unwrap() + .id() + .clone(); + let deps = EnsureActiveDeps { + user: b.user_repo.clone(), + }; + + ensure_active::execute(&deps, &id).await.unwrap(); +} + +#[tokio::test] +async fn a_deleted_user_is_rejected() { + let store = InMemoryStore::new(); + let b = TestContextBuilder::new().with_users(Arc::clone(&store) as _); + let reg = RegisterDeps { + user: b.user_repo.clone(), + password_hasher: b.password_hasher.clone(), + config: b.config.clone(), + }; + register::execute(®, register_command("alice@example.com", "alice")) + .await + .unwrap(); + let id = store + .find_by_email(&Email::new("alice@example.com".into()).unwrap()) + .await + .unwrap() + .unwrap() + .id() + .clone(); + store.mark_deleted(&id, &[]).await.unwrap(); + let deps = EnsureActiveDeps { + user: b.user_repo.clone(), + }; + + let err = ensure_active::execute(&deps, &id).await.unwrap_err(); + + assert!(matches!(err, DomainError::Unauthorized(_)), "got {err:?}"); +} + +#[tokio::test] +async fn an_unknown_user_is_rejected() { + let b = TestContextBuilder::new(); + let deps = EnsureActiveDeps { + user: b.user_repo.clone(), + }; + + let err = ensure_active::execute(&deps, &UserId::generate()) + .await + .unwrap_err(); + + assert!(matches!(err, DomainError::Unauthorized(_)), "got {err:?}"); +} diff --git a/crates/application/tests/users/request_account_deletion_test.rs b/crates/application/tests/users/request_account_deletion_test.rs new file mode 100644 index 00000000..3bb78f94 --- /dev/null +++ b/crates/application/tests/users/request_account_deletion_test.rs @@ -0,0 +1,185 @@ +use std::sync::Arc; + +use domain::{ + errors::DomainError, + events::DomainEvent, + models::UserRole, + ports::UserRepository, + testing::InMemoryStore, + value_objects::{Email, UserId}, +}; + +use application::{ + auth::{deps::RegisterDeps, register}, + users::{deps::RequestAccountDeletionDeps, request_account_deletion}, +}; + +use crate::support::{TestContextBuilder, register_command}; + +const PASSWORD: &str = "password123"; + +struct Fixture { + store: Arc, + deps: RequestAccountDeletionDeps, + register: RegisterDeps, +} + +fn fixture() -> Fixture { + let store = InMemoryStore::new(); + let b = TestContextBuilder::new().with_users(Arc::clone(&store) as _); + Fixture { + deps: RequestAccountDeletionDeps { + user: b.user_repo.clone(), + password_hasher: b.password_hasher.clone(), + }, + register: RegisterDeps { + user: b.user_repo.clone(), + password_hasher: b.password_hasher.clone(), + config: b.config.clone(), + }, + store, + } +} + +impl Fixture { + async fn user(&self, name: &str, role: UserRole) -> UserId { + let email = format!("{name}@example.com"); + let mut cmd = register_command(email.clone(), name); + cmd.role = role; + register::execute(&self.register, cmd).await.unwrap(); + self.store + .find_by_email(&Email::new(email).unwrap()) + .await + .unwrap() + .unwrap() + .id() + .clone() + } + + async fn is_live(&self, id: &UserId) -> bool { + self.store.find_by_id(id).await.unwrap().is_some() + } +} + +#[tokio::test] +async fn a_user_deletes_their_own_account_with_the_right_password() { + let f = fixture(); + let alice = f.user("alice", UserRole::Standard).await; + + request_account_deletion::execute(&f.deps, alice.clone(), alice.clone(), PASSWORD) + .await + .unwrap(); + + assert!(!f.is_live(&alice).await); + assert!( + matches!( + f.store.recorded_events().as_slice(), + [DomainEvent::UserDeleted { user_id }] if *user_id == alice + ), + "events: {:?}", + f.store.recorded_events() + ); +} + +#[tokio::test] +async fn a_wrong_password_is_unauthorized_and_changes_nothing() { + let f = fixture(); + let alice = f.user("alice", UserRole::Standard).await; + + let err = request_account_deletion::execute(&f.deps, alice.clone(), alice.clone(), "nope") + .await + .unwrap_err(); + + assert!(matches!(err, DomainError::Unauthorized(_)), "got {err:?}"); + assert!(f.is_live(&alice).await); + assert!(f.store.recorded_events().is_empty()); +} + +#[tokio::test] +async fn a_non_admin_cannot_delete_someone_else() { + let f = fixture(); + let alice = f.user("alice", UserRole::Standard).await; + let bob = f.user("bob", UserRole::Standard).await; + + let err = request_account_deletion::execute(&f.deps, alice, bob.clone(), PASSWORD) + .await + .unwrap_err(); + + assert!(matches!(err, DomainError::Forbidden(_)), "got {err:?}"); + assert!(f.is_live(&bob).await); +} + +#[tokio::test] +async fn an_admin_deletes_another_user_with_their_own_password() { + let f = fixture(); + let admin = f.user("root", UserRole::Admin).await; + let bob = f.user("bob", UserRole::Standard).await; + + let err = request_account_deletion::execute(&f.deps, admin.clone(), bob.clone(), "bobs-pw") + .await + .unwrap_err(); + assert!(matches!(err, DomainError::Unauthorized(_)), "got {err:?}"); + assert!(f.is_live(&bob).await); + + request_account_deletion::execute(&f.deps, admin.clone(), bob.clone(), PASSWORD) + .await + .unwrap(); + + assert!(!f.is_live(&bob).await); + assert!(f.is_live(&admin).await); +} + +#[tokio::test] +async fn the_last_active_admin_cannot_delete_themselves() { + let f = fixture(); + let admin = f.user("root", UserRole::Admin).await; + + let err = request_account_deletion::execute(&f.deps, admin.clone(), admin.clone(), PASSWORD) + .await + .unwrap_err(); + + assert!(matches!(err, DomainError::Conflict(_)), "got {err:?}"); + assert!(f.is_live(&admin).await); + assert!(f.store.recorded_events().is_empty()); +} + +#[tokio::test] +async fn an_admin_can_delete_themselves_when_another_admin_remains() { + let f = fixture(); + let first = f.user("root", UserRole::Admin).await; + let _second = f.user("root2", UserRole::Admin).await; + + request_account_deletion::execute(&f.deps, first.clone(), first.clone(), PASSWORD) + .await + .unwrap(); + + assert!(!f.is_live(&first).await); +} + +#[tokio::test] +async fn an_unknown_target_is_not_found() { + let f = fixture(); + let admin = f.user("root", UserRole::Admin).await; + + let err = request_account_deletion::execute(&f.deps, admin, UserId::generate(), PASSWORD) + .await + .unwrap_err(); + + assert!(matches!(err, DomainError::NotFound(_)), "got {err:?}"); +} + +#[tokio::test] +async fn an_already_deleted_target_is_not_found() { + let f = fixture(); + let admin = f.user("root", UserRole::Admin).await; + let bob = f.user("bob", UserRole::Standard).await; + request_account_deletion::execute(&f.deps, admin.clone(), bob.clone(), PASSWORD) + .await + .unwrap(); + + let err = request_account_deletion::execute(&f.deps, admin, bob, PASSWORD) + .await + .unwrap_err(); + + assert!(matches!(err, DomainError::NotFound(_)), "got {err:?}"); +} diff --git a/crates/bootstrap/src/build.rs b/crates/bootstrap/src/build.rs index 34a3e562..2a1a6f69 100644 --- a/crates/bootstrap/src/build.rs +++ b/crates/bootstrap/src/build.rs @@ -29,10 +29,10 @@ use application::person::deps::{EnrichPersonDeps, GetPersonDeps}; use application::search::deps::SearchDeps; use application::social::deps::{SocialCommandDeps, SocialQueryDeps}; use application::users::deps::{ - AuthorizeAdminDeps, DeleteAccountDeps, GetCurrentProfileDeps, GetFederatedProfileDeps, + AuthorizeAdminDeps, EnsureActiveDeps, GetCurrentProfileDeps, GetFederatedProfileDeps, GetFederatedProfileStatsDeps, GetLocalProfileDeps, GetPageViewerDeps, GetProfileSettingsDeps, - GetSettingsDeps, GetUsersListDeps, ResolveUsernameDeps, UpdateProfileDeps, - UpdateProfileFieldsDeps, UpdateSettingsDeps, + GetSettingsDeps, GetUsersListDeps, RequestAccountDeletionDeps, ResolveUsernameDeps, + UpdateProfileDeps, UpdateProfileFieldsDeps, UpdateSettingsDeps, }; use application::watchlist::deps::{ GetWatchlistDeps, GetWatchlistForOwnerDeps, IsOnWatchlistDeps, RemoveFromWatchlistDeps, @@ -280,9 +280,12 @@ pub fn build_deps( object_storage: services.object_storage.clone(), event_publisher: services.event_publisher.clone(), }, - delete_account: DeleteAccountDeps { + request_account_deletion: RequestAccountDeletionDeps { + user: repos.user.clone(), + password_hasher: services.password_hasher.clone(), + }, + ensure_active: EnsureActiveDeps { user: repos.user.clone(), - event_publisher: services.event_publisher.clone(), }, get_current_profile: GetCurrentProfileDeps { user: repos.user.clone(), diff --git a/crates/bootstrap/tests/bootstrap_test.rs b/crates/bootstrap/tests/bootstrap_test.rs index 318b8161..0c1fdb8d 100644 --- a/crates/bootstrap/tests/bootstrap_test.rs +++ b/crates/bootstrap/tests/bootstrap_test.rs @@ -215,3 +215,41 @@ async fn a_remote_block_publishes_its_event_after_the_k_ap_write() { assert_eq!(blocked, 1); assert_eq!(queued(&pool, "ActorBlocked").await, 1); } + +/// A token outlives the account that owns it, so every authenticated route +/// must re-check the account: a deleted user gets exactly what an anonymous +/// caller gets. +#[tokio::test] +async fn a_deleted_users_token_is_rejected() { + let (router, pool) = support::server().await; + let alice = support::register(&router, "alice", "alice@example.com").await; + let bearer = |token: &str| { + axum::http::Request::builder() + .uri("/api/v1/diary/export?format=json") + .header("authorization", format!("Bearer {token}")) + .body(axum::body::Body::empty()) + .unwrap() + }; + + let (status, _, body) = support::send(&router, bearer(&alice.token)).await; + assert_eq!(status, 200, "token works while the account is live: {body}"); + let (status, _, _) = + support::send(&router, support::get("/diary/export", Some(&alice.cookie))).await; + assert_eq!(status, 200, "cookie works while the account is live"); + + sqlx::query("UPDATE users SET deleted_at = '2026-01-01T00:00:00Z' WHERE id = ?") + .bind(&alice.user_id) + .execute(&pool) + .await + .unwrap(); + + let (status, _, body) = support::send(&router, bearer(&alice.token)).await; + assert_eq!(status, 401, "json route: {body}"); + let (anon_status, anon_headers, _) = + support::send(&router, support::get("/diary/export", None)).await; + let (status, headers, _) = + support::send(&router, support::get("/diary/export", Some(&alice.cookie))).await; + assert_eq!(status, anon_status, "classic owner page"); + assert_eq!(headers.get("location"), anon_headers.get("location")); + assert_eq!(headers.get("location").unwrap(), "/login"); +} diff --git a/crates/bootstrap/tests/http_handlers/api_handlers_test.rs b/crates/bootstrap/tests/http_handlers/api_handlers_test.rs index 29ae5b92..700725e2 100644 --- a/crates/bootstrap/tests/http_handlers/api_handlers_test.rs +++ b/crates/bootstrap/tests/http_handlers/api_handlers_test.rs @@ -1,4 +1,4 @@ -use crate::support::{AcceptingAuth, test_app_state, test_app_state_with_follow_graph}; +use crate::support::{AcceptingAuth, test_app_state, test_app_state_with_repos}; use axum::{ Router, body::Body, @@ -6,6 +6,8 @@ use axum::{ routing::get, }; use domain::errors::DomainError; +use domain::ports::UserRepository; +use domain::testing::InMemoryStore; use std::sync::Arc; use tower::ServiceExt; use uuid::Uuid; @@ -257,8 +259,19 @@ impl domain::ports::FollowGraphQuery for FixedPendingCount { #[tokio::test] async fn pending_count_endpoint_reports_the_use_case_result() { let uid = domain::value_objects::UserId::from_uuid(Uuid::new_v4()); - let state = test_app_state_with_follow_graph( - Arc::new(FixedPendingCount(3)), + let users = InMemoryStore::new(); + users + .save(&domain::testing::test_user_with_role( + uid.clone(), + domain::models::UserRole::Standard, + )) + .await + .unwrap(); + let state = test_app_state_with_repos( + |repos| { + repos.follow_graph = Arc::new(FixedPendingCount(3)); + repos.user = users as _; + }, Arc::new(AcceptingAuth(uid)), ); let app = Router::new() diff --git a/crates/bootstrap/tests/http_handlers/extractors_test.rs b/crates/bootstrap/tests/http_handlers/extractors_test.rs index d2d727e5..15d428a2 100644 --- a/crates/bootstrap/tests/http_handlers/extractors_test.rs +++ b/crates/bootstrap/tests/http_handlers/extractors_test.rs @@ -246,7 +246,8 @@ async fn admin_api_user_rejects_an_unknown_user() { ) .await .unwrap(); - assert_eq!(resp.status(), StatusCode::NOT_FOUND); + // An account the store does not hold is inactive, so it is unauthenticated. + assert_eq!(resp.status(), StatusCode::UNAUTHORIZED); } #[tokio::test] @@ -337,7 +338,9 @@ async fn admin_user_rejects_an_unknown_user() { ) .await .unwrap(); - assert_eq!(resp.status(), StatusCode::UNAUTHORIZED); + // An account the store does not hold is inactive: back to the login page. + assert_eq!(resp.status(), StatusCode::SEE_OTHER); + assert_eq!(resp.headers().get("location").unwrap(), "/login"); } #[tokio::test] diff --git a/crates/bootstrap/tests/http_handlers/support.rs b/crates/bootstrap/tests/http_handlers/support.rs index 0999592f..e53faeb1 100644 --- a/crates/bootstrap/tests/http_handlers/support.rs +++ b/crates/bootstrap/tests/http_handlers/support.rs @@ -7,7 +7,7 @@ use bootstrap::Repositories; use domain::{ errors::DomainError, models::GeneratedToken, - ports::{AuthService, FollowGraphQuery, PersonQuery, SearchPort, UserRepository}, + ports::{AuthService, PersonQuery, SearchPort, UserRepository}, testing::{ PanicAuthService, PanicDiaryExporter, PanicDiaryQuery, PanicDocumentParser, PanicEventPublisher, PanicGoalCommand, PanicGoalQuery, PanicImportProfileRepository, @@ -168,15 +168,15 @@ pub fn test_app_state_with_search_port( build_test_state_from(repos, Arc::new(PanicAuthService)) } -/// Test state with `follow_graph` overridden BEFORE `build_deps` runs, so the -/// stub is visible from both `app_ctx.repos.follow_graph` and the prebuilt -/// `deps.social.query` that the social handlers actually read from. -pub fn test_app_state_with_follow_graph( - follow_graph: Arc, +/// Test state whose repositories are adjusted by `adjust` BEFORE `build_deps` +/// runs, for tests that need more than one stub swapped in (e.g. a follow graph +/// plus the live user that authenticated routes now look up). +pub fn test_app_state_with_repos( + adjust: impl FnOnce(&mut Repositories), auth_service: Arc, ) -> http_axum::state::AppState { let mut repos = test_repositories(); - repos.follow_graph = follow_graph; + adjust(&mut repos); build_test_state_from(repos, auth_service) } diff --git a/crates/domain/src/errors/mod.rs b/crates/domain/src/errors/mod.rs index b09e30c6..1dbda26f 100644 --- a/crates/domain/src/errors/mod.rs +++ b/crates/domain/src/errors/mod.rs @@ -19,6 +19,9 @@ pub enum DomainError { #[error("Forbidden: {0}")] Forbidden(String), + + #[error("Conflict: {0}")] + Conflict(String), } impl DomainError { diff --git a/crates/domain/src/testing/fakes/store_user.rs b/crates/domain/src/testing/fakes/store_user.rs index c6092c1b..28c222d6 100644 --- a/crates/domain/src/testing/fakes/store_user.rs +++ b/crates/domain/src/testing/fakes/store_user.rs @@ -78,13 +78,18 @@ impl UserRepository for InMemoryStore { events: &[crate::events::DomainEvent], ) -> Result<(), DomainError> { let id = user_id.value(); - if let Some(user) = self.users.read().unwrap().get(&id) { - self.retired_usernames - .write() - .unwrap() - .insert(user.username().value().to_lowercase()); - self.tombstoned.write().unwrap().insert(id); - } + let users = self.users.read().unwrap(); + let mut gone = self.tombstoned.write().unwrap(); + let Some(user) = users.get(&id).filter(|_| !gone.contains(&id)) else { + return Err(DomainError::NotFound("User not found".into())); + }; + self.retired_usernames + .write() + .unwrap() + .insert(user.username().value().to_lowercase()); + gone.insert(id); + drop(gone); + drop(users); self.refresh_sessions .write() .unwrap() diff --git a/crates/domain/tests/testing/in_memory_store_test.rs b/crates/domain/tests/testing/in_memory_store_test.rs index 833c7e00..3e8ed07f 100644 --- a/crates/domain/tests/testing/in_memory_store_test.rs +++ b/crates/domain/tests/testing/in_memory_store_test.rs @@ -628,3 +628,33 @@ async fn fake_mark_deleted_mirrors_the_sqlite_tombstone() { assert_eq!(store.refresh_session_count(), 0); assert_eq!(store.recorded_events().len(), 1); } + +#[tokio::test] +async fn fake_mark_deleted_refuses_unknown_and_already_deleted_users() { + let store = InMemoryStore::new(); + let user = test_user("ghost"); + UserRepository::save(&*store, &user).await.unwrap(); + let event = domain::events::DomainEvent::UserDeleted { + user_id: user.id().clone(), + }; + + let unknown = store + .mark_deleted(&UserId::generate(), std::slice::from_ref(&event)) + .await; + assert!(matches!(unknown, Err(DomainError::NotFound(_)))); + assert!(store.recorded_events().is_empty()); + + store + .mark_deleted(user.id(), std::slice::from_ref(&event)) + .await + .unwrap(); + let again = store + .mark_deleted(user.id(), std::slice::from_ref(&event)) + .await; + assert!(matches!(again, Err(DomainError::NotFound(_)))); + assert_eq!( + store.recorded_events().len(), + 1, + "second call records nothing" + ); +} diff --git a/docs/adr/0005-single-composition-root.md b/docs/adr/0005-single-composition-root.md index d76f1948..daf79327 100644 --- a/docs/adr/0005-single-composition-root.md +++ b/docs/adr/0005-single-composition-root.md @@ -26,8 +26,7 @@ and each then builds only the deps it can honestly use from it. with no server-reachable consumer — `enrich_movie`, `reindex_search`, `merge_duplicates`, `enrich_person`, `handle_requested` — moved into a new `application::WorkerDeps`, built by `composition::build_worker_deps` and consumed only by `crates/worker`. Every field of `Deps` is - now reachable from `presentation` except `users.delete_account` — see the dead-code exception - recorded below — and every field of `WorkerDeps` is reachable from `worker`. No field needs an + now reachable from `presentation` and every field of `WorkerDeps` is reachable from `worker`. No field needs an `Option` or a Noop port to express "not for this binary" — which container it lives in already says that. - `application::WorkerServices` is the analogous split on the services side: the strict subset of @@ -42,11 +41,6 @@ and each then builds only the deps it can honestly use from it. sixth, `federated_profile`, is already `Option` in `Repositories` for unrelated reasons) have no worker-side adapter. `DatabaseOutput` only carries fields both binaries can genuinely fill, so passing it instead of `Repositories` needed no new `Option` and no fake adapter. -- Exactly one field is unreachable from its own binary, and it is called out by name rather than - folded into a group comment: `Deps.users.delete_account` (`DeleteAccountDeps`) has zero callers - anywhere in the workspace — a pre-existing dead use case, not a consequence of this split. Every - other field that used to be described as "worker-only" or "not reachable" is now reachable from - the binary that reaches it, because it lives in that binary's own container. - Both binaries were booted under the default feature set (`sqlite`, `sqlite-federation`) to prove this: the worker reached its steady polling state without panicking, and the server served both a Bearer-authenticated API call and a cookie-authenticated HTML page. A container wired to the 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 88928e42..2b45a18e 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`, `UNAUTHORIZED`, `FORBIDDEN`, `RATE_LIMITED`, `INTERNAL`. +`NOT_FOUND`, `INVALID_INPUT`, `UNAUTHORIZED`, `FORBIDDEN`, `CONFLICT`, `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