From 7d24015fc45b324313622963873153ac982f16e9 Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Wed, 24 Jun 2026 21:17:24 +0200 Subject: [PATCH] feat(drive): add drive deletion - conditions: drive must be empty - deletion forbidden on main personal drive --- frontend/src/lib/api/endpoints/admin.ts | 29 +++++ frontend/src/lib/api/endpoints/drives.ts | 26 +++++ frontend/src/routes/admin/+page.svelte | 39 +++++++ .../routes/config/drive/[uuid]/+page.svelte | 100 +++++++++++++++++- .../services/drive_management_service.rs | 99 +++++++++++++++++ .../services/subject_group_service.rs | 63 +++++++++++ src/domain/errors.rs | 7 ++ src/domain/repositories/drive_repository.rs | 13 +++ .../repositories/pg/drive_pg_repository.rs | 79 ++++++++++++++ src/interfaces/api/handlers/admin_handler.rs | 38 +++++++ src/interfaces/api/handlers/drive_handler.rs | 39 +++++++ src/interfaces/api/routes.rs | 4 + src/interfaces/errors.rs | 1 + tests/api/drives_membership.hurl | 74 +++++++++++++ tests/api/subject_groups.hurl | 64 ++++++++++- 15 files changed, 673 insertions(+), 2 deletions(-) diff --git a/frontend/src/lib/api/endpoints/admin.ts b/frontend/src/lib/api/endpoints/admin.ts index f29e74e1..dda24e49 100644 --- a/frontend/src/lib/api/endpoints/admin.ts +++ b/frontend/src/lib/api/endpoints/admin.ts @@ -157,6 +157,35 @@ export async function removeDriveMemberAdmin( } } +/** + * `DELETE /api/admin/drives/{id}` — admin-only drive delete (D3b). + * + * Bypasses the per-drive `Manage` check (the admin guard at the route + * edge is the access control). The default-personal-drive guard and + * the "drive must be empty" check still fire server-side — admins + * can't accidentally wipe a populated drive or a user's home folder. + * Throws on non-2xx so the caller can branch on `405` (default + * personal) vs `409` (non-empty) when surfacing the failure. + */ +export async function deleteDriveAdmin(driveId: string): Promise { + const res = await apiFetch(`/api/admin/drives/${encodeURIComponent(driveId)}`, { + method: 'DELETE', + credentials: 'same-origin', + headers: getCsrfHeaders() + }); + if (!res.ok) { + let detail = ''; + try { + const parsed = (await res.json()) as { error?: string; message?: string }; + detail = parsed.error ?? parsed.message ?? ''; + } catch { + /* response body wasn't JSON */ + } + // 405 / 409 carry actionable messages from the backend; bubble them. + throw new Error(detail || `delete drive failed: ${res.status}`); + } +} + // ── Users ─────────────────────────────────────────────────────────────── export interface AdminUsersPage { diff --git a/frontend/src/lib/api/endpoints/drives.ts b/frontend/src/lib/api/endpoints/drives.ts index 23409fbb..1a8ee9f5 100644 --- a/frontend/src/lib/api/endpoints/drives.ts +++ b/frontend/src/lib/api/endpoints/drives.ts @@ -104,6 +104,32 @@ export async function updateDriveMember( return (await res.json()) as DriveMember; } +/** + * `DELETE /api/drives/{id}` — Owner-only drive delete (D3b). + * + * Refused with `405` for the default Personal drive and `409` for a + * non-empty drive (caller must move/trash content first). Throws on + * non-2xx with the server's detail message when present so the caller + * can decide whether to surface a confirmation prompt vs an error. + */ +export async function deleteDrive(driveId: string): Promise { + const res = await apiFetch(`/api/drives/${encodeURIComponent(driveId)}`, { + method: 'DELETE', + credentials: 'same-origin', + headers: getCsrfHeaders() + }); + if (!res.ok) { + let detail = ''; + try { + const parsed = (await res.json()) as { error?: string; message?: string }; + detail = parsed.error ?? parsed.message ?? ''; + } catch { + /* response body wasn't JSON */ + } + throw new Error(detail || `delete drive failed: ${res.status}`); + } +} + /** * `DELETE /api/drives/{id}/members/{kind}/{sid}` — remove a member. * Idempotent (removing a non-member returns 204). Refused with 400 if it diff --git a/frontend/src/routes/admin/+page.svelte b/frontend/src/routes/admin/+page.svelte index 528741e8..fb3bbe4d 100644 --- a/frontend/src/routes/admin/+page.svelte +++ b/frontend/src/routes/admin/+page.svelte @@ -43,6 +43,7 @@ type PluginRetention, type ReextractResult, addDriveMemberAdmin, + deleteDriveAdmin, listAllDrives, listDriveMembersAdmin, removeDriveMemberAdmin, @@ -1061,6 +1062,30 @@ : [] ); + // Admin-driven delete-drive flow (D3b). Guarded by the confirm modal + // because the action is destructive and irreversible. The backend + // refuses the default Personal drive (405) and any non-empty drive + // (409); we surface those as toasts rather than silently swallow. + async function requestDeleteDrive(d: Drive) { + const msg = t( + 'admin.drive_delete_confirm', + { name: d.name }, + 'Delete drive "{{name}}"? This cannot be undone.' + ); + if (!(await showConfirm(msg))) return; + try { + await deleteDriveAdmin(d.id); + // Refresh the listing + the sidebar picker. Both have a cached + // view of this drive; without the invalidate the row lingers + // until the next full reload. + await loadDrivesTab(); + drivesStore.invalidate(); + ui.notify(t('admin.drive_deleted', 'Drive deleted.'), 'success'); + } catch (e) { + reportError(e); + } + } + async function submitDriveCreate(e: SubmitEvent) { e.preventDefault(); const name = driveForm.name.trim(); @@ -2192,6 +2217,20 @@ {/if} + + {#if !d.default_for_user} + + {/if} diff --git a/frontend/src/routes/config/drive/[uuid]/+page.svelte b/frontend/src/routes/config/drive/[uuid]/+page.svelte index 1bac5dcd..bf233026 100644 --- a/frontend/src/routes/config/drive/[uuid]/+page.svelte +++ b/frontend/src/routes/config/drive/[uuid]/+page.svelte @@ -3,9 +3,12 @@ import { page } from '$app/state'; import { onMount } from 'svelte'; - import { listDriveMembers } from '$lib/api/endpoints/drives'; + import { goto } from '$app/navigation'; + + import { deleteDrive, listDriveMembers } from '$lib/api/endpoints/drives'; import { renameFolder } from '$lib/api/endpoints/folders'; import { errorToast } from '$lib/utils/errors'; + import { ui } from '$lib/stores/ui.svelte'; import type { Drive, DriveMember, DriveRole } from '$lib/api/types'; import ShareDialog from '$lib/components/ShareDialog.svelte'; import UserVignette from '$lib/components/UserVignette.svelte'; @@ -34,6 +37,42 @@ // are the user themselves (seeded by the lifecycle hook). const canRename = $derived(drive?.caller_role === 'owner'); + // Delete is allowed for Owners — backend additionally refuses the + // default Personal drive (405) and non-empty drives (409). We hide + // the button on the default-personal drive so the affordance only + // appears when it can actually succeed. + const canDelete = $derived(drive?.caller_role === 'owner' && !drive?.default_for_user); + + let deleting = $state(false); + + async function confirmAndDelete() { + if (!drive) return; + const confirmText = t( + 'drive.delete_confirm', + { name: drive.name }, + 'Delete drive "{{name}}"? This cannot be undone — the drive ' + + 'must be empty first or the server will refuse.' + ); + if (typeof window === 'undefined' || !window.confirm(confirmText)) return; + deleting = true; + try { + await deleteDrive(drive.id); + drivesStore.invalidate(); + await drivesStore.load(); + ui.notify(t('drive.deleted', 'Drive deleted.'), 'success'); + // Send the user back to /files. The picker's reload above + // already removed the now-deleted drive from the sidebar. + await goto(resolve('/files')); + } catch (e) { + // 409 (non-empty) and 405 (default personal) come back as + // thrown errors with the server's detail in the message — + // surface as a toast rather than a silent failure. + errorToast(e); + } finally { + deleting = false; + } + } + // Inline rename state. `renameDraft` shadows `drive.name` while the // input is open; we don't write back to the store until the server // accepts the change. `renameBusy` disables the save/cancel buttons @@ -385,6 +424,32 @@ {/if} + + {#if canDelete} + +
+

{t('drive.danger_zone', 'Danger zone')}

+

+ {t( + 'drive.delete_hint', + 'Deleting a drive removes it permanently. The drive must be empty (no live files or folders) before delete is allowed.' + )} +

+ +
+ {/if} {/if} @@ -529,6 +594,39 @@ color: var(--color-text); } + /* Danger zone card hosts the delete-drive button at the bottom of + the page. Border tint makes the destructive context unmissable + without hijacking the whole layout — same convention as + admin/users delete affordances. */ + .danger-zone { + border-color: var(--color-error-text); + } + + .danger-zone h2 { + color: var(--color-error-text); + } + + .btn-danger { + display: inline-flex; + align-items: center; + gap: 0.5rem; + padding: 0.5rem 0.875rem; + border: 1px solid var(--color-error-text); + border-radius: var(--radius-md); + background: var(--color-error-text); + color: var(--color-text-light); + cursor: pointer; + } + + .btn-danger:disabled { + opacity: 0.5; + cursor: not-allowed; + } + + .icon-btn--danger { + color: var(--color-error-text); + } + /* Compact icon button used in the title row + nowhere else here. The shared `.icon-btn` style isn't promoted to a global yet, so we duplicate the minimum that this page needs. */ diff --git a/src/application/services/drive_management_service.rs b/src/application/services/drive_management_service.rs index 7b26edf8..38c67351 100644 --- a/src/application/services/drive_management_service.rs +++ b/src/application/services/drive_management_service.rs @@ -274,6 +274,105 @@ impl DriveManagementService { Ok(()) } + /// `DELETE /api/drives/{id}` and `DELETE /api/admin/drives/{id}`. + /// + /// Policy (drive.md §6 + memos): + /// - Caller must hold `Permission::Manage` on the drive — typically + /// the Owner. `caller_is_admin = true` bypasses this check; the + /// route gate is the access control then. Audit emits + /// `drive.deleted_via_admin` when the bypass fires. + /// - The user's default Personal drive (`drives.default_for_user + /// IS NOT NULL`) is refused with `405` — deleting your home is a + /// category error. Secondary personal drives + shared drives + /// follow the same content-empty rule below. + /// - The drive must be empty (no live folders other than the root, + /// no live files). Trashed rows are excluded — owners can + /// delete a drive whose trash bin still holds rows; the trash GC + /// cleans them up after the retention window. Non-empty drives + /// return `409 Conflict` so the UI can prompt the owner to + /// move/trash content first. + /// + /// On success the drive row, its root folder, and every + /// `role_grants` row scoped to the drive are removed in one + /// transaction. + pub async fn delete_drive( + &self, + caller_id: Uuid, + caller_is_admin: bool, + drive_id: Uuid, + ) -> Result<(), DomainError> { + let resource = Resource::Drive(drive_id); + if !caller_is_admin { + self.authz + .require(Subject::User(caller_id), Permission::Manage, resource) + .await?; + } + + let drive = self.drive_repo.get_by_id(drive_id).await.map_err(|e| { + DomainError::internal_error("Drive", format!("Failed to fetch drive: {e:?}")) + })?; + + if drive.drive.default_for_user.is_some() { + tracing::info!( + target: "audit", + event = "drive_delete.rejected", + reason = "default_personal_drive", + drive_id = %drive_id, + by = %caller_id, + "👮🏻‍♂️ refused delete on default personal drive {drive_id}", + ); + return Err(DomainError::operation_not_supported( + "Drive", + "The default Personal drive cannot be deleted.", + )); + } + + let empty = self.drive_repo.is_empty(drive_id).await.map_err(|e| { + DomainError::internal_error("Drive", format!("Failed to check emptiness: {e:?}")) + })?; + if !empty { + tracing::info!( + target: "audit", + event = "drive_delete.rejected", + reason = "drive_not_empty", + drive_id = %drive_id, + by = %caller_id, + "👮🏻‍♂️ refused delete on non-empty drive {drive_id}", + ); + return Err(DomainError::new( + crate::common::errors::ErrorKind::Conflict, + "Drive", + "Drive is not empty — move or trash its contents before deleting.", + )); + } + + self.drive_repo + .delete_atomic(drive_id) + .await + .map_err(|e| DomainError::internal_error("Drive", format!("delete failed: {e:?}")))?; + + // Drop every cached drive-role entry for this drive so the next + // /api/drives listing for any subject doesn't show a row pointing + // at a deleted drive_id. Single-key cache invalidations are safe + // even when no entry matches. + self.authz + .invalidate_drive_role_cache_for_drive(drive_id) + .await; + + tracing::info!( + target: "audit", + event = if caller_is_admin { + "drive.deleted_via_admin" + } else { + "drive.deleted" + }, + drive_id = %drive_id, + by = %caller_id, + "🗑 drive deleted", + ); + Ok(()) + } + // ── Business rules ────────────────────────────────────────────────────── /// Personal drives are single-user single-owner; any member mutation is diff --git a/src/application/services/subject_group_service.rs b/src/application/services/subject_group_service.rs index db021e0f..d8d68fa3 100644 --- a/src/application/services/subject_group_service.rs +++ b/src/application/services/subject_group_service.rs @@ -210,6 +210,69 @@ impl SubjectGroupService { )); } + // Refuse if this group is the **sole Owner** of any drive — the + // cascade-delete below would otherwise wipe the only `owner` + // grant on that drive and leave it orphaned (no one can ever + // manage it again). The check is "for every drive where this + // group holds Owner, does another Owner exist?". A single drive + // failing the check is enough to refuse. + // + // Matching D3a's last-owner-protection rule on `set_member_role` + // / `remove_member` — they catch the case where the drive's + // last Owner is *directly* a user or group being demoted / + // removed via the membership API. This guard catches the same + // invariant from the group-lifecycle side. + let orphaning: Option<(Uuid,)> = sqlx::query_as( + r#" + WITH group_owned AS ( + SELECT resource_id + FROM storage.role_grants + WHERE subject_type = 'group' + AND subject_id = $1 + AND resource_type = 'drive' + AND role = 'owner' + AND (expires_at IS NULL OR expires_at > NOW()) + ) + SELECT resource_id + FROM storage.role_grants + WHERE resource_type = 'drive' + AND role = 'owner' + AND (expires_at IS NULL OR expires_at > NOW()) + AND resource_id IN (SELECT resource_id FROM group_owned) + GROUP BY resource_id + HAVING COUNT(*) = 1 + LIMIT 1 + "#, + ) + .bind(id) + .fetch_optional(self.pool.as_ref()) + .await + .map_err(|e| { + DomainError::new( + ErrorKind::InternalError, + "SubjectGroup", + format!("sole-owner check: {e}"), + ) + })?; + if let Some((drive_id,)) = orphaning { + tracing::info!( + target: "audit", + event = "group_delete.rejected", + reason = "sole_drive_owner", + group_id = %id, + drive_id = %drive_id, + by = %caller_id, + "👮🏻‍♂️ refused group delete — sole Owner of drive {drive_id}", + ); + return Err(DomainError::new( + ErrorKind::Conflict, + "SubjectGroup", + "Group is the sole Owner of at least one shared drive — \ + promote another Owner first or delete the drive." + .to_string(), + )); + } + // Atomically delete grants pointing at this group, then the group // itself. If either fails, both roll back. let mut tx = self.pool.begin().await.map_err(|e| { diff --git a/src/domain/errors.rs b/src/domain/errors.rs index 605d58a5..83e96dde 100644 --- a/src/domain/errors.rs +++ b/src/domain/errors.rs @@ -33,6 +33,12 @@ pub enum ErrorKind { DatabaseError, /// Storage quota exceeded QuotaExceeded, + /// State conflict — the request is well-formed and permitted, but + /// the resource is in a state that refuses it (e.g. "drive must + /// be empty before delete"). Maps to HTTP 409. Distinct from + /// `AlreadyExists` (which is a uniqueness violation) so audit + /// readers can tell them apart. + Conflict, } impl Display for ErrorKind { @@ -48,6 +54,7 @@ impl Display for ErrorKind { ErrorKind::UnsupportedOperation => write!(f, "Unsupported Operation"), ErrorKind::DatabaseError => write!(f, "Database Error"), ErrorKind::QuotaExceeded => write!(f, "Quota Exceeded"), + ErrorKind::Conflict => write!(f, "Conflict"), } } } diff --git a/src/domain/repositories/drive_repository.rs b/src/domain/repositories/drive_repository.rs index 7bb23f4e..da625766 100644 --- a/src/domain/repositories/drive_repository.rs +++ b/src/domain/repositories/drive_repository.rs @@ -177,6 +177,19 @@ pub trait DriveRepository: Send + Sync + 'static { subject_ids: &[Uuid], ) -> Result, DriveRepositoryError>; + /// `true` when the drive holds no live (non-trashed) folders other + /// than its own root and no live files at all. Used by + /// `DriveManagementService::delete_drive` to enforce the + /// "empty-before-delete" rule — owners must clear / trash the + /// content first so a single click can't wipe a populated drive. + async fn is_empty(&self, drive_id: Uuid) -> Result; + + /// Hard-delete a drive: its `role_grants` rows, its root folder, + /// and the drive row itself, in one transaction. Caller is + /// responsible for ensuring `is_empty` first; this method does + /// **not** re-check. Returns `NotFound` if the drive id is gone. + async fn delete_atomic(&self, drive_id: Uuid) -> Result<(), DriveRepositoryError>; + /// List every drive on the system, regardless of caller membership. /// /// Used by the admin panel's `GET /api/admin/drives`. Distinct from diff --git a/src/infrastructure/repositories/pg/drive_pg_repository.rs b/src/infrastructure/repositories/pg/drive_pg_repository.rs index 3de44d2d..e4fb398e 100644 --- a/src/infrastructure/repositories/pg/drive_pg_repository.rs +++ b/src/infrastructure/repositories/pg/drive_pg_repository.rs @@ -303,6 +303,85 @@ impl DriveRepository for DrivePgRepository { Self::row_to_drive_with_name(&row) } + async fn is_empty(&self, drive_id: Uuid) -> Result { + // A "live" non-root folder = any folder with `parent_id IS NOT + // NULL` (root is the only NULL-parent row per drive) and not in + // the trash. Trashed items don't count — owners can delete a + // drive even when its trash bin still holds rows; the trash GC + // will clean those up after the standard retention window. + let count: (i64,) = sqlx::query_as( + r#" + SELECT ( + (SELECT COUNT(*) FROM storage.folders + WHERE drive_id = $1 AND parent_id IS NOT NULL AND NOT is_trashed) + + (SELECT COUNT(*) FROM storage.files + WHERE drive_id = $1 AND NOT is_trashed) + ) + "#, + ) + .bind(drive_id) + .fetch_one(self.pool.as_ref()) + .await + .map_err(|e| Self::map_sqlx_err("is_empty", e))?; + Ok(count.0 == 0) + } + + async fn delete_atomic(&self, drive_id: Uuid) -> Result<(), DriveRepositoryError> { + // Three-statement transaction: + // 1. Drop every role_grants row scoped to the drive itself + // (folder/file grants under it are gone by step 3 cascade). + // 2. Look up the root folder id (we'll need it to delete the + // folder row AFTER the drive row releases its FK). + // 3. Delete the drive — release the drive→root FK first. + // 4. Delete the root folder (drive_id FK on folders cascades + // from this row going away; only the root remains because + // is_empty was true). + // + // `drive_id` is bound once per statement; failure at any step + // rolls back. Caller (`DriveManagementService::delete_drive`) + // is responsible for the `is_empty` precheck. + let mut tx = self + .pool + .begin() + .await + .map_err(|e| Self::map_sqlx_err("delete_atomic.begin", e))?; + + sqlx::query( + "DELETE FROM storage.role_grants \ + WHERE resource_type = 'drive' AND resource_id = $1", + ) + .bind(drive_id) + .execute(&mut *tx) + .await + .map_err(|e| Self::map_sqlx_err("delete_atomic.grants", e))?; + + let root: (Uuid,) = sqlx::query_as( + "SELECT root_folder_id FROM storage.drives WHERE id = $1", + ) + .bind(drive_id) + .fetch_optional(&mut *tx) + .await + .map_err(|e| Self::map_sqlx_err("delete_atomic.lookup_root", e))? + .ok_or_else(|| DriveRepositoryError::NotFound(drive_id.to_string()))?; + + sqlx::query("DELETE FROM storage.drives WHERE id = $1") + .bind(drive_id) + .execute(&mut *tx) + .await + .map_err(|e| Self::map_sqlx_err("delete_atomic.drive", e))?; + + sqlx::query("DELETE FROM storage.folders WHERE id = $1") + .bind(root.0) + .execute(&mut *tx) + .await + .map_err(|e| Self::map_sqlx_err("delete_atomic.root", e))?; + + tx.commit() + .await + .map_err(|e| Self::map_sqlx_err("delete_atomic.commit", e))?; + Ok(()) + } + async fn get_by_id(&self, id: Uuid) -> Result { let row = sqlx::query( r#" diff --git a/src/interfaces/api/handlers/admin_handler.rs b/src/interfaces/api/handlers/admin_handler.rs index 39611695..bbe1cc0e 100644 --- a/src/interfaces/api/handlers/admin_handler.rs +++ b/src/interfaces/api/handlers/admin_handler.rs @@ -98,6 +98,7 @@ pub fn admin_routes() -> Router> { // Drives — admin-wide view (distinct from `/api/drives` which // is filtered to the caller's role grants). .route("/drives", get(list_all_drives)) + .route("/drives/{id}", delete(delete_drive_admin)) .route( "/drives/{id}/members", get(list_drive_members_admin).post(add_drive_member_admin), @@ -1957,3 +1958,40 @@ pub async fn remove_drive_member_admin( .map_err(AppError::from)?; Ok(StatusCode::NO_CONTENT) } + +/// `DELETE /api/admin/drives/{id}` — admin-only drive delete (D3b). +/// +/// Same shape as the user-facing `DELETE /api/drives/{id}`, but +/// bypasses the per-drive `Manage` check (the admin guard at the +/// route edge is the access control). The remaining invariants — +/// default Personal drive is undeletable, drive must be empty — still +/// apply: an admin can't accidentally wipe a populated drive or the +/// default home folder of any user. Audit emits +/// `drive.deleted_via_admin` on success. +#[utoipa::path( + delete, + path = "/api/admin/drives/{id}", + params(("id" = Uuid, Path, description = "Drive UUID")), + responses( + (status = 204, description = "Drive deleted"), + (status = 401, description = "Unauthorized"), + (status = 403, description = "Admin required"), + (status = 405, description = "Default Personal drive — undeletable"), + (status = 409, description = "Drive is not empty"), + ), + security(("bearerAuth" = [])), + tag = "admin" +)] +pub async fn delete_drive_admin( + State(state): State>, + headers: HeaderMap, + axum::extract::Path(drive_id): axum::extract::Path, +) -> Result { + let (admin_id, _) = admin_guard(&state, &headers).await?; + state + .drive_management_service + .delete_drive(admin_id, true, drive_id) + .await + .map_err(AppError::from)?; + Ok(StatusCode::NO_CONTENT) +} diff --git a/src/interfaces/api/handlers/drive_handler.rs b/src/interfaces/api/handlers/drive_handler.rs index aa488986..de45a658 100644 --- a/src/interfaces/api/handlers/drive_handler.rs +++ b/src/interfaces/api/handlers/drive_handler.rs @@ -356,3 +356,42 @@ pub async fn remove_drive_member( Err(e) => AppError::from(e).into_response(), } } + +/// `DELETE /api/drives/{id}` — Owner-only deletion (D3b). +/// +/// Refuses (per `DriveManagementService::delete_drive`): +/// - `404` when the caller lacks Manage on the drive (anti-enum). +/// - `405` when the drive is the user's default Personal drive. +/// - `409` when the drive still holds live folders/files; the caller +/// must trash or move them first. +/// +/// On success the drive row, its root folder, and every role grant +/// scoped to the drive are removed in one transaction; cached drive +/// roles are invalidated. +#[utoipa::path( + delete, + path = "/api/drives/{id}", + params(("id" = Uuid, Path, description = "Drive UUID")), + responses( + (status = 204, description = "Drive deleted"), + (status = 404, description = "Drive not found or caller lacks Manage"), + (status = 405, description = "Default Personal drive — undeletable"), + (status = 409, description = "Drive is not empty — move/trash contents first"), + ), + security(("bearerAuth" = [])), + tag = "drives" +)] +pub async fn delete_drive( + State(state): State>, + auth_user: AuthUser, + Path(drive_id): Path, +) -> impl IntoResponse { + match state + .drive_management_service + .delete_drive(auth_user.id, false, drive_id) + .await + { + Ok(()) => StatusCode::NO_CONTENT.into_response(), + Err(e) => AppError::from(e).into_response(), + } +} diff --git a/src/interfaces/api/routes.rs b/src/interfaces/api/routes.rs index 6093df07..97bf46d2 100644 --- a/src/interfaces/api/routes.rs +++ b/src/interfaces/api/routes.rs @@ -425,6 +425,10 @@ pub fn create_api_routes(app_state: &Arc) -> Router> { "/", get(drive_handler::list_drives).post(drive_handler::create_drive), ) + .route( + "/{id}", + axum::routing::delete(drive_handler::delete_drive), + ) .route( "/{id}/members", get(drive_handler::list_drive_members).post(drive_handler::add_drive_member), diff --git a/src/interfaces/errors.rs b/src/interfaces/errors.rs index e630f7b7..a0b8ba12 100644 --- a/src/interfaces/errors.rs +++ b/src/interfaces/errors.rs @@ -125,6 +125,7 @@ impl From for AppError { ErrorKind::UnsupportedOperation => StatusCode::METHOD_NOT_ALLOWED, ErrorKind::DatabaseError => StatusCode::INTERNAL_SERVER_ERROR, ErrorKind::QuotaExceeded => StatusCode::INSUFFICIENT_STORAGE, + ErrorKind::Conflict => StatusCode::CONFLICT, }; Self { diff --git a/tests/api/drives_membership.hurl b/tests/api/drives_membership.hurl index 58d4f872..e34bc1e5 100644 --- a/tests/api/drives_membership.hurl +++ b/tests/api/drives_membership.hurl @@ -734,6 +734,8 @@ Content-Type: application/json } HTTP 201 +[Captures] +editor_created_folder_id: jsonpath "$.id" [Asserts] jsonpath "$.name" == "editor-created-folder" @@ -813,3 +815,75 @@ GET {{base_url}}/api/drives/{{team_drive_id}}/members Authorization: Bearer {{dave_token}} HTTP 404 + + +# ───────────────────────────────────────────────────────────── +# Step 30 — Drive delete (D3b). +# - Non-Owner → 404 (Bob is Viewer post-Step 28). +# - Owner on non-empty drive → 409 (the editor-created-folder +# from Step 27 is still live). +# - Owner after the folder is trashed → 204. +# Personal-drive refusal (default_for_user IS NOT NULL) is +# covered separately — `mbr_dave` keeps his default drive, +# we exercise its 405 below. +# ───────────────────────────────────────────────────────────── + +# 30a — Viewer (Bob) cannot delete the drive → 404, anti-enum same as +# the member-mutation refusals. +DELETE {{base_url}}/api/drives/{{team_drive_id}} +Authorization: Bearer {{bob_token}} + +HTTP 404 + + +# 30b — Owner (Alice) on a non-empty drive → 409 with the canonical +# "drive_not_empty" reason in the audit log. +DELETE {{base_url}}/api/drives/{{team_drive_id}} +Authorization: Bearer {{alice_token}} + +HTTP 409 + + +# 30c — Clear the lingering content (the Editor-created folder from +# Step 27). Delete via the regular folder endpoint so the row +# lands in trash, not the live tree; `is_empty` excludes +# trashed rows so a populated trash bin is allowed. +DELETE {{base_url}}/api/folders/{{editor_created_folder_id}} +Authorization: Bearer {{alice_token}} + +HTTP 204 + + +# 30d — Owner on an empty drive → 204. +DELETE {{base_url}}/api/drives/{{team_drive_id}} +Authorization: Bearer {{alice_token}} + +HTTP 204 + + +# 30e — Drive is gone; subsequent reads return 404. +GET {{base_url}}/api/drives/{{team_drive_id}}/members +Authorization: Bearer {{alice_token}} + +HTTP 404 + + +# 30f — Default Personal drive — Dave's home — cannot be deleted. +# Look up the drive id via the picker listing. Dave is a fresh +# user and only has his default personal drive, so `$[0].id` +# is unambiguous. (Avoiding the `[?(...)]` filter — Hurl +# collapses single-match results to a scalar, which breaks +# `nth` / list-style assertions; see memory.) +GET {{base_url}}/api/drives +Authorization: Bearer {{dave_token}} + +HTTP 200 +[Captures] +dave_default_drive_id: jsonpath "$[0].id" +[Asserts] +jsonpath "$[0].default_for_user" == "{{dave_user_id}}" + +DELETE {{base_url}}/api/drives/{{dave_default_drive_id}} +Authorization: Bearer {{dave_token}} + +HTTP 405 diff --git a/tests/api/subject_groups.hurl b/tests/api/subject_groups.hurl index fcb404e8..b0c2ca90 100644 --- a/tests/api/subject_groups.hurl +++ b/tests/api/subject_groups.hurl @@ -334,13 +334,75 @@ HTTP 403 # ───────────────────────────────────────────────────────────── -# Step 11 — Cleanup: delete engineering (cascades to qa membership + grants). +# Step 11 — Sole-Owner group-delete guard (D3b). +# +# A group that is the only `Role::Owner` of a shared drive must NOT be +# deletable — wiping it would orphan the drive (no live Owner grant +# left). Symmetric to the last-owner-protection rule on `set_role` / +# `remove_member` from the membership API side; this guard catches +# the same invariant from the group-lifecycle side. +# +# Setup: admin creates a shared drive owned by `grp-engineering-hurl`, +# then tries to delete the group. Refused with 409. Promote a second +# Owner (a user), then the group delete succeeds. # ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/drives +Authorization: Bearer {{alice_token}} +Content-Type: application/json +{ + "kind": "shared", + "name": "grp-guarded-drive-hurl", + "owner": { "type": "group", "id": "{{engineers_id}}" } +} + +HTTP 201 +[Captures] +guarded_drive_id: jsonpath "$.id" + + +# 11a — Group delete refused while it's the sole Owner of the drive. +DELETE {{base_url}}/api/groups/{{engineers_id}} +Authorization: Bearer {{alice_token}} + +HTTP 409 + + +# 11b — Add Grace as a co-Owner of the drive via the admin endpoint. +# Alice (the OxiCloud admin) created the drive but doesn't +# auto-grant herself a role on it, so she lacks `Manage` on the +# user-facing `/api/drives/{id}/members` — the admin route +# bypasses that check for exactly this case. +POST {{base_url}}/api/admin/drives/{{guarded_drive_id}}/members +Authorization: Bearer {{alice_token}} +Content-Type: application/json +{ + "subject": { "type": "user", "id": "{{grace_user_id}}" }, + "role": "owner" +} + +HTTP 201 + + +# 11c — Group delete now succeeds — the drive still has Grace as Owner. DELETE {{base_url}}/api/groups/{{engineers_id}} Authorization: Bearer {{alice_token}} HTTP 204 + +# 11d — Cleanup: trash the drive (no content) so subsequent test files +# don't see a dangling shared drive. After 11c, Grace is the +# only remaining Owner via her direct grant, so she's the one +# who can delete via the user-facing route. +DELETE {{base_url}}/api/drives/{{guarded_drive_id}} +Authorization: Bearer {{grace_token}} + +HTTP 204 + + +# ───────────────────────────────────────────────────────────── +# Step 12 — Cleanup: delete qa group. +# ───────────────────────────────────────────────────────────── DELETE {{base_url}}/api/groups/{{qa_id}} Authorization: Bearer {{alice_token}}