diff --git a/crates/adapters/activitypub/src/tombstone_gate.rs b/crates/adapters/activitypub/src/tombstone_gate.rs index 83701a93..ae6f00f5 100644 --- a/crates/adapters/activitypub/src/tombstone_gate.rs +++ b/crates/adapters/activitypub/src/tombstone_gate.rs @@ -27,10 +27,12 @@ pub async fn tombstone_gate( next.run(request).await } -/// The user id in `/users/{id}/outbox|featured|inbox`. +/// The user id in any `/users/{id}/...` path (outbox, inbox, collections, +/// content objects such as `/users/{id}/reviews/{rid}`). The bare actor path +/// has its own tombstone response. pub fn gated_user(path: &str) -> Option { let rest = path.strip_prefix("/users/")?; let (id, endpoint) = rest.split_once('/')?; - matches!(endpoint, "outbox" | "featured" | "inbox").then_some(())?; + (!endpoint.is_empty()).then_some(())?; uuid::Uuid::parse_str(id).ok() } diff --git a/crates/adapters/activitypub/tests/user_adapter.rs b/crates/adapters/activitypub/tests/user_adapter.rs index 03787492..16abb493 100644 --- a/crates/adapters/activitypub/tests/user_adapter.rs +++ b/crates/adapters/activitypub/tests/user_adapter.rs @@ -46,14 +46,22 @@ async fn an_unknown_user_is_none() { } #[test] -fn only_the_per_user_endpoints_are_gated() { +fn every_per_user_path_is_gated() { use activitypub::tombstone_gate::gated_user; let id = uuid::Uuid::new_v4(); - for endpoint in ["outbox", "featured", "inbox"] { + let rid = uuid::Uuid::new_v4(); + for endpoint in [ + "outbox", + "featured", + "inbox", + "followers", + "following", + &format!("reviews/{rid}"), + ] { assert_eq!(gated_user(&format!("/users/{id}/{endpoint}")), Some(id)); } assert_eq!(gated_user(&format!("/users/{id}")), None); - assert_eq!(gated_user(&format!("/users/{id}/followers")), None); + assert_eq!(gated_user(&format!("/users/{id}/")), None); assert_eq!(gated_user("/inbox"), None); assert_eq!(gated_user("/users/not-a-uuid/outbox"), None); } diff --git a/crates/adapters/sqlite/src/repositories/account_purge/mod.rs b/crates/adapters/sqlite/src/repositories/account_purge/mod.rs index ed84b580..eea71c14 100644 --- a/crates/adapters/sqlite/src/repositories/account_purge/mod.rs +++ b/crates/adapters/sqlite/src/repositories/account_purge/mod.rs @@ -223,6 +223,16 @@ impl AccountPurger for SqliteAccountPurger { .execute(&mut *tx) .await .map_err(map_sqlx_error)?; + // A Follow arriving on the shared inbox after the purge can leave + // a late row for the tombstone; delete it explicitly rather than + // rely on the foreign-key cascade of the final `users` delete. + for table in ["ap_followers", "ap_following"] { + sqlx::query(&format!("DELETE FROM {table} WHERE local_user_id = ?")) + .bind(id) + .execute(&mut *tx) + .await + .map_err(map_sqlx_error)?; + } for table in RETAINED_USER_TABLES { sqlx::query(&format!("DELETE FROM {table} WHERE user_id = ?")) .bind(id) diff --git a/crates/adapters/sqlite/tests/account_purge_test.rs b/crates/adapters/sqlite/tests/account_purge_test.rs index cbb1fcc5..91bc76ba 100644 --- a/crates/adapters/sqlite/tests/account_purge_test.rs +++ b/crates/adapters/sqlite/tests/account_purge_test.rs @@ -696,6 +696,14 @@ async fn tombstone_cleanup_removes_rows_older_than_retention_and_keeps_younger_o assert_eq!(count_for(&pool, "users", "id", &s.user).await, 1); assert_eq!(unfinished().await, before); + // A Follow that reached the shared inbox after the purge left a row. + exec( + &pool, + "INSERT INTO ap_followers (local_user_id, remote_actor_url, status, created_at) + VALUES (?, 'https://late.example/users/zed', 'pending', '2026-01-01T00:00:00Z')", + &[&alice], + ) + .await; // Once they finish, the cleanup takes the tombstone and the history. exec(&pool, "UPDATE event_queue SET status = 'done'", &[]).await; let removed = p @@ -712,6 +720,11 @@ async fn tombstone_cleanup_removes_rows_older_than_retention_and_keeps_younger_o left.iter().all(|p| !p.contains(&alice)), "no history of the user remains: {left:?}" ); + assert_eq!( + count_for(&pool, "ap_followers", "local_user_id", &s.user).await, + 0, + "late shared-inbox follower row" + ); assert_eq!( count_for(&pool, "ap_local_actors", "user_id", &s.user).await, 0 diff --git a/crates/application/src/users/purge_account.rs b/crates/application/src/users/purge_account.rs index 99d0ccae..3a4d6550 100644 --- a/crates/application/src/users/purge_account.rs +++ b/crates/application/src/users/purge_account.rs @@ -20,6 +20,10 @@ use domain::{ /// /// Idempotent: every step is a no-op on a re-run, and the notification is /// skipped once the account is purged. +/// +/// A purge that fails after the notification was queued is redelivered and +/// queues the `Delete` again. That is harmless: a repeated `Delete` of an +/// already deleted actor is idempotent on remote servers. pub struct AccountPurgeHandler { user: Arc, notifier: Arc, diff --git a/docs/adr/0016-account-deletion.md b/docs/adr/0016-account-deletion.md index 8936268b..11088ee9 100644 --- a/docs/adr/0016-account-deletion.md +++ b/docs/adr/0016-account-deletion.md @@ -29,11 +29,20 @@ exists), so two admins deleting each other concurrently cannot both succeed; the `count_active_admins` check is only the early, friendly refusal (409). **Immediate effects, before the purge runs.** Login is refused and tokens are rejected -(`ensure_active` in the extractors; the user lookups filter `deleted_at IS NULL`). The profile +(`ensure_active` in the extractors; the user lookups filter `deleted_at IS NULL`). The user's +webhook tokens stop ingesting at once (`find_by_token_hash` ignores tokens of tombstoned users, +so the endpoint answers 401). The profile and `?embed=true` pages 404. The ActivityPub actor document, inbox, outbox and featured answer -410 (`tombstone_gate` for the collection endpoints, `actor_is_gone` for the actor). WebFinger +410 (`tombstone_gate` for every `/users/{id}/...` path, content objects included, and +`actor_is_gone` for the actor). WebFinger answers 404 (owned by k-ap). +**The service actor is not deletable.** The instance's service actor (`UserId::service_actor()`, +seeded by migration 0022, used for signed fetches) is refused by `request_account_deletion` +(403 `Forbidden`), and both UIs hide the action (`is_service_account` on the profile response; +the classic template checks the same). Deleting it would retire its name and, after the +retention period, its keypair, breaking signed fetches for good. + **Purge.** Order matters and every step is idempotent: 1. Notify the fediverse through `FederationDeletionNotifier`, implemented over k-ap's @@ -47,6 +56,11 @@ answers 404 (owned by k-ap). `watch_events`, `wrap_up_records`, `import_*`, `blocked_actors`). `PURGED_USER_TABLES`, `URL_KEYED_USER_COLUMNS` and `PAYLOAD_KEYED_USER_COLUMNS` are checked against the schema by a guard test, so a new user-owned table cannot be forgotten. + Global wrap-up reports (`wrap_up_records` with no user) embed `most_active_user`; the purge + sets it to null in any report naming the user and keeps the rest (`wrap_up_records.report_json` + is in `PAYLOAD_KEYED_USER_COLUMNS`). + A failed purge is redelivered and queues the `Delete` again; a repeated `Delete` is + idempotent on remote servers. 3. Image objects (avatar, banner) are deleted after the commit, then the references are dropped (`forget_images`), so a crash in between is repaired by redelivery. @@ -72,7 +86,9 @@ registration path refuses a retired username, so a deleted name cannot be impers - Events already relayed to NATS JetStream are not erased; they live as long as the stream's retention allows. - A Follow arriving on the shared inbox can create a `Pending` `ap_followers` row for the - tombstoned user; it is removed by the cleanup. + tombstoned user; `purge_expired_tombstones` deletes the user's `ap_followers` and + `ap_following` rows explicitly before the `users` row (a test covers it), not by relying on + the foreign-key cascade. - Remote servers that stay down through the delivery retries, or that ignore `Delete`, may keep stale copies. - A handler that defers a tombstone-naming event forever delays the tombstone (and keypair) diff --git a/spa/src/routes/_app/settings/index.tsx b/spa/src/routes/_app/settings/index.tsx index e787537f..af03ea2e 100644 --- a/spa/src/routes/_app/settings/index.tsx +++ b/spa/src/routes/_app/settings/index.tsx @@ -133,7 +133,6 @@ function SettingsPage() { function DangerZone() { const { t } = useTranslation() const { logout } = useAuth() - const navigate = useNavigate() const [open, setOpen] = useState(false) const del = useDeleteOwnAccount() @@ -148,7 +147,8 @@ function DangerZone() { { onSuccess: () => { logout() - navigate({ to: "/login" }) + // Full page load: the classic public home, not the SPA login. + window.location.assign("/") }, }, )