diff --git a/src/admin/users.rs b/src/admin/users.rs index 6fb6504..cf82b98 100644 --- a/src/admin/users.rs +++ b/src/admin/users.rs @@ -413,47 +413,50 @@ pub(super) async fn transfer_super( let backend = state.db_backend; let now = now_rfc3339(); - // Remove super from current user - let update1_sql = adapt_sql( - "UPDATE happyview_users SET is_super = ? WHERE id = ?", - backend, - ); - sqlx::query(&update1_sql) - .bind(0_i32) - .bind(&auth.user_id) - .execute(&state.db) + // Run every mutation in a single transaction so a failure or crash can never + // leave the instance with zero super users (M6). On any early return the + // transaction is dropped and rolled back, preserving the current super. + let mut tx = state + .db + .begin() .await - .map_err(|e| AppError::Internal(format!("failed to remove super: {e}")))?; + .map_err(|e| AppError::Internal(format!("failed to begin transaction: {e}")))?; - // Set super on target user - let update2_sql = adapt_sql( + // Promote the target first (and require it to exist) so a missing target + // aborts before the current super is touched. + let set_super_sql = adapt_sql( "UPDATE happyview_users SET is_super = ? WHERE id = ?", backend, ); - let result = sqlx::query(&update2_sql) + let result = sqlx::query(&set_super_sql) .bind(1_i32) .bind(&body.target_user_id) - .execute(&state.db) + .execute(&mut *tx) .await .map_err(|e| AppError::Internal(format!("failed to set super: {e}")))?; if result.rows_affected() == 0 { - // Restore super on current user - let restore_sql = adapt_sql( - "UPDATE happyview_users SET is_super = ? WHERE id = ?", - backend, - ); - let _ = sqlx::query(&restore_sql) - .bind(1_i32) - .bind(&auth.user_id) - .execute(&state.db) - .await; return Err(AppError::NotFound(format!( "user '{}' not found", body.target_user_id ))); } + // Demote the current super — unless they are transferring to themselves, in + // which case demoting would undo the promotion above. + if body.target_user_id != auth.user_id { + let remove_super_sql = adapt_sql( + "UPDATE happyview_users SET is_super = ? WHERE id = ?", + backend, + ); + sqlx::query(&remove_super_sql) + .bind(0_i32) + .bind(&auth.user_id) + .execute(&mut *tx) + .await + .map_err(|e| AppError::Internal(format!("failed to remove super: {e}")))?; + } + // Ensure target has all permissions let perm_sql = adapt_sql( "INSERT INTO happyview_user_permissions (user_id, permission, granted_by, granted_at) VALUES (?, ?, ?, ?) ON CONFLICT DO NOTHING", @@ -466,11 +469,15 @@ pub(super) async fn transfer_super( .bind(perm.as_str()) .bind(&auth.user_id) .bind(&now) - .execute(&state.db) + .execute(&mut *tx) .await .map_err(|e| AppError::Internal(format!("failed to grant permission: {e}")))?; } + tx.commit() + .await + .map_err(|e| AppError::Internal(format!("failed to commit super transfer: {e}")))?; + log_event( &state.db, EventLog { diff --git a/tests/transfer_super.rs b/tests/transfer_super.rs new file mode 100644 index 0000000..a33caa7 --- /dev/null +++ b/tests/transfer_super.rs @@ -0,0 +1,124 @@ +mod common; + +use axum::body::Body; +use axum::http::{Request, StatusCode}; +use serde_json::json; +use serial_test::serial; +use tower::ServiceExt; +use uuid::Uuid; + +use common::app::TestApp; + +async fn user_id_for_did(app: &TestApp, did: &str) -> String { + let sql = happyview::db::adapt_sql( + "SELECT id FROM happyview_users WHERE did = ?", + app.state.db_backend, + ); + let row: (String,) = sqlx::query_as(&sql) + .bind(did) + .fetch_one(&app.state.db) + .await + .unwrap(); + row.0 +} + +async fn is_super(app: &TestApp, user_id: &str) -> bool { + let sql = happyview::db::adapt_sql( + "SELECT is_super FROM happyview_users WHERE id = ?", + app.state.db_backend, + ); + let row: (i32,) = sqlx::query_as(&sql) + .bind(user_id) + .fetch_one(&app.state.db) + .await + .unwrap(); + row.0 != 0 +} + +async fn insert_user(app: &TestApp, did: &str) -> String { + let id = Uuid::new_v4().to_string(); + let sql = happyview::db::adapt_sql( + "INSERT INTO happyview_users (id, did, is_super, created_at) VALUES (?, ?, ?, ?)", + app.state.db_backend, + ); + sqlx::query(&sql) + .bind(&id) + .bind(did) + .bind(0_i32) + .bind(happyview::db::now_rfc3339()) + .execute(&app.state.db) + .await + .unwrap(); + id +} + +fn transfer_req(app: &TestApp, target_user_id: &str) -> Request { + let (name, value) = app.admin_cookie(); + Request::builder() + .method("POST") + .uri("/admin/users/transfer-super") + .header(name, value) + .header("content-type", "application/json") + .body(Body::from( + json!({ "target_user_id": target_user_id }).to_string(), + )) + .unwrap() +} + +/// A successful transfer promotes the target (with all permissions) and demotes +/// the previous super. +#[tokio::test] +#[serial] +async fn transfer_super_moves_super_and_grants_permissions() { + common::require_db!(); + let app = TestApp::new().await; + let admin_id = user_id_for_did(&app, &app.admin_did).await; + let target_id = insert_user(&app, "did:plc:new-super").await; + + let resp = app + .router + .clone() + .oneshot(transfer_req(&app, &target_id)) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::NO_CONTENT); + + assert!(is_super(&app, &target_id).await, "target should be super"); + assert!( + !is_super(&app, &admin_id).await, + "previous super should be demoted" + ); + + let sql = happyview::db::adapt_sql( + "SELECT COUNT(*) FROM happyview_user_permissions WHERE user_id = ?", + app.state.db_backend, + ); + let count: (i64,) = sqlx::query_as(&sql) + .bind(&target_id) + .fetch_one(&app.state.db) + .await + .unwrap(); + assert!(count.0 > 0, "target should have permissions granted"); +} + +/// Transferring to a non-existent user fails without demoting the current super +/// — the transaction rolls back, so the instance is never left without a super. +#[tokio::test] +#[serial] +async fn transfer_super_to_missing_user_preserves_current_super() { + common::require_db!(); + let app = TestApp::new().await; + let admin_id = user_id_for_did(&app, &app.admin_did).await; + + let resp = app + .router + .clone() + .oneshot(transfer_req(&app, "does-not-exist")) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::NOT_FOUND); + assert!( + is_super(&app, &admin_id).await, + "current super must be preserved when the transfer fails (no lockout)" + ); +}