diff --git a/backend/.sqlx/query-f338741f0866dc717ccfcf6b7d46b4e55ca07e472610b29fad24b64f45f98e18.json b/backend/.sqlx/query-f338741f0866dc717ccfcf6b7d46b4e55ca07e472610b29fad24b64f45f98e18.json new file mode 100644 index 0000000000..0f57f5d951 --- /dev/null +++ b/backend/.sqlx/query-f338741f0866dc717ccfcf6b7d46b4e55ca07e472610b29fad24b64f45f98e18.json @@ -0,0 +1,59 @@ +{ + "db_name": "PostgreSQL", + "query": "DELETE FROM draft\n WHERE workspace_id = $1 AND path = $2 AND typ = $3\n AND ($4::text IS NULL OR email = $4)\n RETURNING 1 as \"one!\"", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "one!", + "type_info": "Int4" + } + ], + "parameters": { + "Left": [ + "Text", + "Text", + { + "Custom": { + "name": "draft_kind", + "kind": { + "Enum": [ + "script", + "flow", + "app", + "raw_app", + "resource", + "variable", + "trigger_schedule", + "trigger_webhook", + "trigger_default_email", + "trigger_email", + "trigger_http", + "trigger_websocket", + "trigger_postgres", + "trigger_kafka", + "trigger_nats", + "trigger_mqtt", + "trigger_sqs", + "trigger_gcp", + "trigger_azure", + "trigger_poll", + "trigger_cli", + "trigger_nextcloud", + "trigger_google", + "trigger_github", + "data_pipeline", + "trigger_amqp" + ] + } + } + }, + "Text" + ] + }, + "nullable": [ + null + ] + }, + "hash": "f338741f0866dc717ccfcf6b7d46b4e55ca07e472610b29fad24b64f45f98e18" +} diff --git a/backend/tests/drafts_delete_for_user.rs b/backend/tests/drafts_delete_for_user.rs new file mode 100644 index 0000000000..9c48f8828c --- /dev/null +++ b/backend/tests/drafts_delete_for_user.rs @@ -0,0 +1,85 @@ +//! Only a workspace admin can delete a draft that belongs to another user. +//! +//! A discard through `drafts/update` is scoped to the caller's own row, so it +//! never reaches someone else's draft. `drafts/delete_for_user` is the admin +//! route for that, and it must stay closed to everyone else. + +use serde_json::{json, Value}; +use sqlx::{Pool, Postgres}; + +use windmill_test_utils::*; + +#[sqlx::test(fixtures("base"))] +async fn test_only_admin_deletes_another_users_draft(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let base = format!("http://localhost:{port}/api/w/test-workspace/drafts"); + let client = reqwest::Client::new(); + let path = "script/u/test-user-2/theirs"; + + let save = |token: &'static str| { + client + .post(format!("{base}/update/{path}")) + .header("Authorization", format!("Bearer {token}")) + .json(&json!({ "value": { "path": "u/test-user-2/theirs", "content": "" } })) + .send() + }; + let resp = save("SECRET_TOKEN_2").await?; + assert_eq!(resp.status(), 200, "{}", resp.text().await?); + + let delete = |token: &'static str| { + client + .post(format!( + "{base}/delete_for_user/{path}?username=test-user-2" + )) + .header("Authorization", format!("Bearer {token}")) + .send() + }; + let own_draft = || async { + client + .get(format!("{base}/get_own/{path}")) + .header("Authorization", "Bearer SECRET_TOKEN_2") + .send() + .await? + .json::() + .await + }; + + let resp = delete("SECRET_TOKEN_3").await?; + assert_eq!(resp.status(), 403, "{}", resp.text().await?); + assert!( + !own_draft().await?.is_null(), + "a non-admin deleted the draft" + ); + + let resp = delete("SECRET_TOKEN").await?; + assert_eq!(resp.status(), 200, "{}", resp.text().await?); + assert!(own_draft().await?.is_null(), "the draft is still there"); + + let resp = delete("SECRET_TOKEN").await?; + assert_eq!(resp.status(), 404, "{}", resp.text().await?); + + // Without a username the delete takes every draft at the path. + for token in ["SECRET_TOKEN_2", "SECRET_TOKEN"] { + let resp = save(token).await?; + assert_eq!(resp.status(), 200, "{}", resp.text().await?); + } + let delete_all = |token: &'static str| { + client + .post(format!("{base}/delete_for_user/{path}")) + .header("Authorization", format!("Bearer {token}")) + .send() + }; + let resp = delete_all("SECRET_TOKEN_3").await?; + assert_eq!(resp.status(), 403, "{}", resp.text().await?); + let resp = delete_all("SECRET_TOKEN").await?; + assert_eq!(resp.status(), 200, "{}", resp.text().await?); + let left: i64 = sqlx::query_scalar( + "SELECT count(*) FROM draft WHERE workspace_id = 'test-workspace' AND path = 'u/test-user-2/theirs'", + ) + .fetch_one(&db) + .await?; + assert_eq!(left, 0, "drafts survived a path-wide delete"); + Ok(()) +} diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 6eaf9f06ea..5a0dcb4aa8 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -10925,6 +10925,37 @@ paths: description: "`saved` only, upsert or delete: where the write landed. Differs from the URL path when the item had moved away from it; the editor follows it there. Absent when a delete found nothing to remove and the caller cannot read the path it moved to." required: [status, current_timestamp] + /w/{workspace}/drafts/delete_for_user/{kind}/{path}: + post: + summary: delete another user's draft, or every draft at a path (admin only) + description: Delete the draft a given user holds at a path, or, without `username`, every draft at that path (the legacy ownerless one and the caller's own included). Workspace admins / superadmins only, and only for script, flow, app and raw_app drafts. Unlike a discard of one's own draft through `drafts/update`, this returns 404 when nothing was deleted. + operationId: deleteDraftForUser + tags: + - draft + parameters: + - $ref: "#/components/parameters/WorkspaceId" + - name: kind + in: path + required: true + schema: + $ref: "#/components/schemas/UserDraftItemKind" + - $ref: "#/components/parameters/ScriptPath" + - name: username + in: query + required: false + description: Workspace username of the draft owner. Omit to delete every user's draft at the path. + schema: + type: string + responses: + "200": + description: draft(s) deleted + content: + text/plain: + schema: + type: string + "404": + description: no matching draft at that path + /w/{workspace}/drafts/move/{kind}/{path}: post: summary: move the current user's draft-only item to another path diff --git a/backend/windmill-api/src/drafts.rs b/backend/windmill-api/src/drafts.rs index a3cd34f3f8..c26eb092a3 100644 --- a/backend/windmill-api/src/drafts.rs +++ b/backend/windmill-api/src/drafts.rs @@ -15,13 +15,14 @@ use axum::{ }; use serde::{Deserialize, Serialize}; use windmill_api_flows::flows::validate_operator_flow; +use windmill_audit::{audit_oss::audit_log, ActionKind}; use windmill_common::{ db::UserDB, error::{Error, Result}, flows::FlowValue, user_drafts::{DraftUserRef, UserDraftItemKind, ENCRYPTED_DRAFT_PREFIX}, users::resolve_username_to_email, - utils::{check_proper_path, strip_json_nul}, + utils::{check_proper_path, require_admin, strip_json_nul}, variables::{build_crypt, encrypt}, workspaces::operator_can_build_flows, }; @@ -32,6 +33,10 @@ pub fn workspaced_service() -> Router { .route("/get/{kind}/{*path}", get(get_draft_for_user)) .route("/get_own/{kind}/{*path}", get(get_own_draft)) .route("/update/{kind}/{*path}", post(update_draft)) + .route( + "/delete_for_user/{kind}/{*path}", + post(delete_draft_for_user), + ) .route("/move/{kind}/{*path}", post(move_draft)) .route("/migrate_legacy/{kind}/{*path}", post(migrate_legacy_draft)) } @@ -1000,6 +1005,85 @@ async fn migrate_legacy_draft( } } +#[derive(Deserialize, Debug)] +pub struct DeleteDraftForUserQuery { + /// Workspace username of the draft owner. Absent: every draft at the path, + /// the legacy ownerless one and the caller's own included. + pub username: Option, +} + +/// Delete ANOTHER user's draft at a path, or every draft there, for workspace +/// admins (and superadmins, which carry `is_admin` in a workspace). Covers the +/// kinds whose authors are visible to others (`shares_drafts_across_users`); the +/// owner is named by workspace username, as on `/drafts/get`. +async fn delete_draft_for_user( + authed: ApiAuthed, + Extension(db): Extension, + Path((w_id, kind, path)): Path<(String, UserDraftItemKind, windmill_common::utils::StripPath)>, + Query(query): Query, +) -> Result { + require_admin(authed.is_admin, &authed.username)?; + if !kind.shares_drafts_across_users() { + return Err(Error::NotFound( + "drafts for this item kind are private to their owner".to_string(), + )); + } + let path = path.to_path(); + let username = query.username.as_deref(); + let owner_email = match username { + Some(username) => Some( + resolve_username_to_email(&w_id, username, &db) + .await? + .ok_or_else(|| { + Error::NotFound(format!("no user with username {username} in workspace")) + })?, + ), + None => None, + }; + + let mut tx = db.begin().await?; + let deleted = sqlx::query_scalar!( + r#"DELETE FROM draft + WHERE workspace_id = $1 AND path = $2 AND typ = $3 + AND ($4::text IS NULL OR email = $4) + RETURNING 1 as "one!""#, + &w_id, + path, + kind as UserDraftItemKind, + owner_email.as_deref(), + ) + .fetch_all(&mut *tx) + .await? + .len(); + if deleted == 0 { + return Err(Error::NotFound(match username { + Some(username) => format!("no draft for {username} at {path}"), + None => format!("no draft at {path}"), + })); + } + audit_log( + &mut *tx, + &authed, + "drafts.delete_for_user", + ActionKind::Delete, + &w_id, + Some(path), + Some( + [ + ("kind", kind.as_str()), + ("username", username.unwrap_or("*")), + ] + .into(), + ), + ) + .await?; + tx.commit().await?; + Ok(match username { + Some(username) => format!("Deleted {username}'s draft at {path}"), + None => format!("Deleted {deleted} draft(s) at {path}"), + }) +} + /// For variable-kind drafts with `variable.is_secret == true`, encrypt /// `variable.value` with the workspace crypt key and mark it /// `$encrypted:` so the secret never persists in plaintext at rest. diff --git a/frontend/src/lib/components/common/confirmationModal/OtherUsersDraftsModal.svelte b/frontend/src/lib/components/common/confirmationModal/OtherUsersDraftsModal.svelte index 49356d888e..ce9b07768e 100644 --- a/frontend/src/lib/components/common/confirmationModal/OtherUsersDraftsModal.svelte +++ b/frontend/src/lib/components/common/confirmationModal/OtherUsersDraftsModal.svelte @@ -7,7 +7,8 @@ */ import { DraftService, type UserDraftItemKind } from '$lib/gen' import { sendUserToast } from '$lib/toast' - import { Users, Pencil, GitCompareArrows, Wrench } from 'lucide-svelte' + import { Users, Pencil, GitCompareArrows, Wrench, Trash2 } from 'lucide-svelte' + import ConfirmationModal from './ConfirmationModal.svelte' import Modal2 from '$lib/components/common/modal/Modal2.svelte' import Button from '$lib/components/common/button/Button.svelte' import Tooltip from '$lib/components/Tooltip.svelte' @@ -17,6 +18,11 @@ import { OtherUserDraftLoad } from '$lib/components/otherUserDraftLoad.svelte' import { displayDate } from '$lib/utils' import { userStore } from '$lib/stores' + import { armRestartOnFirstInteraction } from '$lib/userDraftToast' + import { UserDraft } from '$lib/userDraft.svelte' + import { UserDraftDbSyncer } from '$lib/userDraftDbSyncer.svelte' + import { goto } from '$app/navigation' + import { base } from '$app/paths' export type OtherDraftUser = { username?: string | null; draft_saved_at?: string } @@ -54,8 +60,70 @@ let diffDrawer: DiffDrawer | undefined = $state(undefined) let migrateOpen = $state(false) - // Legacy (no-owner) drafts can only be resolved by workspace admins / superadmins. - const canMigrateLegacy = $derived(!!$userStore?.is_admin || !!$userStore?.is_super_admin) + // One owner's draft, or every draft at the path. + let deleting = $state(undefined) + let deleteBusy = $state(false) + + // Legacy (no-owner) drafts can only be resolved by workspace admins / superadmins, + // and only they can delete a draft that belongs to someone else. + const isAdmin = $derived(!!$userStore?.is_admin || !!$userStore?.is_super_admin) + + async function deleteDraft(target: OtherDraftUser | 'all') { + const username = target === 'all' ? undefined : target.username + if (target !== 'all' && !username) return + deleteBusy = true + const query = { workspace, itemKind, path } + try { + // A path-wide delete takes our own draft too. Mute autosave and let any save + // already in flight land first: one committing after the delete would put the + // draft back for the reload below to read. + if (target === 'all') { + UserDraft.stopSync(itemKind, path, { workspace }) + await UserDraftDbSyncer.quiesce(query) + } + await DraftService.deleteDraftForUser({ + workspace, + kind: itemKind, + path, + username: username ?? undefined + }) + } catch (e) { + if (target === 'all') { + UserDraft.restartSync(itemKind, path, { workspace }) + // `quiesce` cancelled the timer of an edit still waiting to save. + void UserDraftDbSyncer.flush(query, { honorAutosaveToggle: true }) + } + sendUserToast(`Could not delete draft: ${e.body ?? e.message}`, true) + return + } finally { + deleteBusy = false + } + sendUserToast(username ? `Deleted ${username}'s draft` : `Deleted all drafts at ${path}`) + deleting = undefined + isOpen = false + + // Drop the editor's copy of our draft as well, or its next write would save + // it right back. Autosave re-arms on the next interaction. + if (target === 'all') { + UserDraft.remove(itemKind, path, { workspace }) + await UserDraftDbSyncer.flush(query) + armRestartOnFirstInteraction(workspace, itemKind, path) + } + // Nothing is left at a never-deployed path once its last draft is gone, so + // there is no item to reload the editor on. + const lastDraftGone = + target === 'all' || + (!hasOwnDraft && otherDraftsUsers.every((o) => ownerKey(o) === ownerKey(target))) + if (draftOnly && lastDraftGone) { + await goto(`${base}/`) + return + } + try { + await onReload?.() + } catch (e) { + sendUserToast(`Could not reload the editor: ${e.body ?? e.message}`, true) + } + } function ownerLabel(owner: OtherDraftUser): string { return owner.username ?? 'Legacy draft' @@ -184,7 +252,7 @@ > Load - {#if !owner.username && canMigrateLegacy} + {#if !owner.username && isAdmin} + {:else if owner.username && isAdmin} + {/if} {/each} -
+
+ {#if isAdmin} + + {/if}
@@ -206,6 +297,29 @@ + { + if (deleting) void deleteDraft(deleting) + }} + onCanceled={() => (deleting = undefined)} +> + + {#if deleting === 'all'} + This permanently deletes every draft at + {path}, yours included. All unsaved work there + is lost and can't be recovered. + {:else} + This permanently deletes the draft {deleting?.username} has at + {path}. Their unsaved work there is lost and + can't be recovered. + {/if} + + +