diff --git a/crates/octos-cli/src/api/user_admin.rs b/crates/octos-cli/src/api/user_admin.rs index 01d323c3a6..a995c9e528 100644 --- a/crates/octos-cli/src/api/user_admin.rs +++ b/crates/octos-cli/src/api/user_admin.rs @@ -10,6 +10,7 @@ use serde::{Deserialize, Serialize}; use super::AppState; use crate::login_allowlist::AllowedLogin; +use crate::profiles::{ProfileStore, UserProfile}; use crate::user_store::User; #[derive(Serialize)] @@ -194,12 +195,17 @@ pub async fn delete_user( .as_ref() .ok_or(StatusCode::SERVICE_UNAVAILABLE)?; - if let Some(ref pm) = state.process_manager { - let _ = pm.stop(&id).await; + if us + .get(&id) + .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)? + .is_none() + { + tracing::warn!(user_id = %id, "delete_user: user not found"); + return Err(StatusCode::NOT_FOUND); } if let Some(ref ps) = state.profile_store { - let _ = ps.delete(&id); + delete_user_profiles(&state, ps, &id).await?; } match us.delete(&id) { @@ -221,9 +227,87 @@ pub async fn delete_user( } } +async fn delete_user_profiles( + state: &AppState, + profile_store: &ProfileStore, + user_id: &str, +) -> Result<(), StatusCode> { + let parent = profile_store + .get(user_id) + .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; + let sub_accounts = profile_store + .list_sub_accounts(user_id) + .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; + + for sub in sub_accounts { + delete_profile_record(state, profile_store, &sub).await?; + } + + if let Some(parent) = parent { + delete_profile_record(state, profile_store, &parent).await?; + } + + Ok(()) +} + +async fn delete_profile_record( + state: &AppState, + profile_store: &ProfileStore, + profile: &UserProfile, +) -> Result<(), StatusCode> { + if let Some(ref pm) = state.process_manager { + let _ = pm.stop(&profile.id).await; + } + + let data_dir = profile_store.resolve_data_dir(profile); + if data_dir.exists() { + if let Err(e) = std::fs::remove_dir_all(&data_dir) { + tracing::warn!( + profile = %profile.id, + dir = %data_dir.display(), + error = %e, + "failed to clean up profile data directory" + ); + } + } + + profile_store + .delete(&profile.id) + .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; + + Ok(()) +} + #[cfg(test)] mod tests { use super::*; + use crate::profiles::ProfileConfig; + use crate::user_store::{UserRole, UserStore}; + + fn test_user(id: &str) -> User { + User { + id: id.to_string(), + email: format!("{id}@example.com"), + name: id.to_string(), + role: UserRole::User, + created_at: Utc::now(), + last_login_at: None, + } + } + + fn test_profile(id: &str, parent_id: Option<&str>) -> UserProfile { + UserProfile { + id: id.to_string(), + name: id.to_string(), + public_subdomain: None, + enabled: true, + data_dir: None, + parent_id: parent_id.map(ToString::to_string), + config: ProfileConfig::default(), + created_at: Utc::now(), + updated_at: Utc::now(), + } + } #[test] fn users_list_response_serialize() { @@ -253,4 +337,61 @@ mod tests { assert_eq!(json["ok"], false); assert_eq!(json["message"], "not found"); } + + #[tokio::test] + async fn delete_user_cascades_sub_account_profiles_and_data_dirs() { + let dir = tempfile::tempdir().unwrap(); + let user_store = Arc::new(UserStore::open(dir.path()).unwrap()); + let profile_store = Arc::new(ProfileStore::open(dir.path()).unwrap()); + let parent = test_profile("alice", None); + let sub_account = test_profile("alice--bot", Some("alice")); + let parent_data_dir = profile_store.resolve_data_dir(&parent); + let sub_data_dir = profile_store.resolve_data_dir(&sub_account); + + user_store.save(&test_user("alice")).unwrap(); + profile_store.save(&parent).unwrap(); + profile_store.save(&sub_account).unwrap(); + assert!(parent_data_dir.exists()); + assert!(sub_data_dir.exists()); + + let state = Arc::new(AppState { + user_store: Some(user_store.clone()), + profile_store: Some(profile_store.clone()), + ..AppState::empty_for_tests() + }); + + let response = delete_user(State(state), Path("alice".to_string())) + .await + .unwrap() + .0; + + assert!(response.ok); + assert!(user_store.get("alice").unwrap().is_none()); + assert!(profile_store.get("alice").unwrap().is_none()); + assert!(profile_store.get("alice--bot").unwrap().is_none()); + assert!(!parent_data_dir.exists()); + assert!(!sub_data_dir.exists()); + } + + #[tokio::test] + async fn delete_user_missing_user_does_not_delete_matching_profile() { + let dir = tempfile::tempdir().unwrap(); + let user_store = Arc::new(UserStore::open(dir.path()).unwrap()); + let profile_store = Arc::new(ProfileStore::open(dir.path()).unwrap()); + let profile = test_profile("alice", None); + let data_dir = profile_store.resolve_data_dir(&profile); + profile_store.save(&profile).unwrap(); + + let state = Arc::new(AppState { + user_store: Some(user_store), + profile_store: Some(profile_store.clone()), + ..AppState::empty_for_tests() + }); + + let result = delete_user(State(state), Path("alice".to_string())).await; + + assert!(matches!(result, Err(StatusCode::NOT_FOUND))); + assert!(profile_store.get("alice").unwrap().is_some()); + assert!(data_dir.exists()); + } }