diff --git a/backend/.sqlx/query-161cc8750a9c6f22b28bc0cc565c621cd3e6ee378587620deb0ea2f69f1fcbdb.json b/backend/.sqlx/query-161cc8750a9c6f22b28bc0cc565c621cd3e6ee378587620deb0ea2f69f1fcbdb.json new file mode 100644 index 0000000000..408a9a948f --- /dev/null +++ b/backend/.sqlx/query-161cc8750a9c6f22b28bc0cc565c621cd3e6ee378587620deb0ea2f69f1fcbdb.json @@ -0,0 +1,16 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE workspace_settings\n SET datatable = jsonb_set(datatable, ARRAY['datatables', $2, 'permissions'], $3)\n WHERE workspace_id = $1\n AND datatable->'datatables' ? $2", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Text", + "Jsonb" + ] + }, + "nullable": [] + }, + "hash": "161cc8750a9c6f22b28bc0cc565c621cd3e6ee378587620deb0ea2f69f1fcbdb" +} diff --git a/backend/.sqlx/query-2daf3786f0b7195a3acfd80e01f2dba171ae5850f22503e846b7355a4f250611.json b/backend/.sqlx/query-2daf3786f0b7195a3acfd80e01f2dba171ae5850f22503e846b7355a4f250611.json new file mode 100644 index 0000000000..00af10bafa --- /dev/null +++ b/backend/.sqlx/query-2daf3786f0b7195a3acfd80e01f2dba171ae5850f22503e846b7355a4f250611.json @@ -0,0 +1,15 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE workspace_settings\n SET datatable = datatable #- ARRAY['datatables', $2, 'permissions']\n WHERE workspace_id = $1", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Text" + ] + }, + "nullable": [] + }, + "hash": "2daf3786f0b7195a3acfd80e01f2dba171ae5850f22503e846b7355a4f250611" +} diff --git a/backend/.sqlx/query-7845539644254c5f6394241b2207209d52a54104d1fb6632468f63ba691a3ed0.json b/backend/.sqlx/query-7845539644254c5f6394241b2207209d52a54104d1fb6632468f63ba691a3ed0.json deleted file mode 100644 index c69cf61433..0000000000 --- a/backend/.sqlx/query-7845539644254c5f6394241b2207209d52a54104d1fb6632468f63ba691a3ed0.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "UPDATE workspace_settings\n SET datatable = datatable #- ARRAY['datatables', $2, 'permissions']\n WHERE workspace_id = $1", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Text", - "Text" - ] - }, - "nullable": [] - }, - "hash": "7845539644254c5f6394241b2207209d52a54104d1fb6632468f63ba691a3ed0" -} diff --git a/backend/windmill-api-workspaces/src/workspaces_extra.rs b/backend/windmill-api-workspaces/src/workspaces_extra.rs index 87ddef96a6..c7fba6f101 100644 --- a/backend/windmill-api-workspaces/src/workspaces_extra.rs +++ b/backend/windmill-api-workspaces/src/workspaces_extra.rs @@ -975,14 +975,13 @@ pub(crate) async fn delete_workspace( // database whose only record is the settings row below, so what to drop is // resolved while that row is here and the drop itself runs after the commit. // Nothing is dropped here. + // The first row this transaction locks — see `lock_workspace_settings_unchecked`. + // Without it a permissions save that commits between this read and the row's + // deletion adds a login nothing then drops, and the config that named it is + // gone. let mut planned_role_drops = Vec::new(); - let datatable_config: Option = sqlx::query_scalar!( - "SELECT datatable FROM workspace_settings WHERE workspace_id = $1", - &w_id - ) - .fetch_optional(&mut *tx) - .await? - .flatten(); + let datatable_config = + windmill_common::workspaces::lock_workspace_settings_unchecked(&mut tx, &w_id).await?; for name in datatable_config .as_ref() .and_then(|c| c.get("datatables")) @@ -1316,6 +1315,96 @@ pub struct DropForkedDatatableDatabasesRequest { datatable_names: Vec, } +/// Take a data table's generated logins away before its database goes. +/// +/// A login is cluster-wide, so it outlives the database it was created in and +/// stays adoptable by whatever takes this workspace's id and this data table's +/// name next. The config stops naming them first: a data table whose database is +/// being dropped has no business claiming roles in it, and that is also what the +/// drop reads to know they are nobody's. +/// +/// Returns the block it removed, which the caller must put back if the drop it +/// was clearing the way for does not happen: a data table that keeps its +/// database and loses its permissions leaves every member of the workspace +/// resolving to the data table's own connection, which owns everything in it. +async fn drop_datatable_roles_before_its_database( + db: &DB, + w_id: &str, + dt_name: &str, + errors: &mut Vec, +) -> Option { + let planned = + crate::datatable_permissions::plan_drop_of_deleted_datatable(db, w_id, dt_name).await?; + // Read and clear under the settings lock, so what comes back is what was + // taken away and is what putting it back would restore. + let cleared = async { + let mut tx = db.begin().await?; + let removed = windmill_common::workspaces::lock_workspace_settings_unchecked(&mut tx, w_id) + .await? + .and_then(|c| { + c.get("datatables") + .and_then(|d| d.get(dt_name)) + .and_then(|d| d.get("permissions")) + .cloned() + }); + sqlx::query!( + "UPDATE workspace_settings + SET datatable = datatable #- ARRAY['datatables', $2, 'permissions'] + WHERE workspace_id = $1", + w_id, + dt_name, + ) + .execute(&mut *tx) + .await?; + tx.commit().await?; + Ok::<_, windmill_common::error::Error>(removed) + } + .await; + match cleared { + Ok(removed) => { + crate::datatable_permissions::run_planned_drop(db, w_id, dt_name, planned).await; + removed + } + Err(e) => { + errors.push(format!( + "Could not clear the permissions of datatable://{}: {}", + dt_name, e + )); + None + } + } +} + +/// Put back the block a drop that did not happen took away. The logins it names +/// are already gone, so every role now resolves to nothing and is refused — the +/// safe answer, and the one a re-save of the data table's permissions repairs by +/// recreating them. +async fn restore_datatable_permissions( + db: &DB, + w_id: &str, + dt_name: &str, + permissions: serde_json::Value, + errors: &mut Vec, +) { + if let Err(e) = sqlx::query!( + "UPDATE workspace_settings + SET datatable = jsonb_set(datatable, ARRAY['datatables', $2, 'permissions'], $3) + WHERE workspace_id = $1 + AND datatable->'datatables' ? $2", + w_id, + dt_name, + permissions, + ) + .execute(db) + .await + { + errors.push(format!( + "Could not restore the permissions of datatable://{}, which is now open to every member of the workspace: {}", + dt_name, e + )); + } +} + /// Drop forked datatable databases. Returns errors per datatable that failed. /// Same permission as delete_workspace: fork owner or super admin. pub async fn drop_forked_datatable_databases( @@ -1367,38 +1456,6 @@ pub async fn drop_forked_datatable_databases( _ => continue, }; - // Before the database goes: the logins are in it, and the plan is built by - // connecting to it. Dropping the database first leaves them behind — a - // login is cluster-wide, so it survives its database and stays adoptable by - // whatever takes this workspace's id and this data table's name next. - // - // The config stops naming them first, both because a data table whose - // database is being dropped has no business claiming roles in it, and - // because that is what the drop reads to know they are nobody's. - if let Some(planned) = - crate::datatable_permissions::plan_drop_of_deleted_datatable(&db, &w_id, dt_name).await - { - let cleared = sqlx::query!( - "UPDATE workspace_settings - SET datatable = datatable #- ARRAY['datatables', $2, 'permissions'] - WHERE workspace_id = $1", - &w_id, - dt_name, - ) - .execute(&db) - .await; - match cleared { - Ok(_) => { - crate::datatable_permissions::run_planned_drop(&db, &w_id, dt_name, planned) - .await - } - Err(e) => errors.push(format!( - "Could not clear the permissions of datatable://{}: {}", - dt_name, e - )), - } - } - if dt.database.resource_type == windmill_common::workspaces::DataTableCatalogResourceType::Instance { @@ -1410,11 +1467,17 @@ pub async fn drop_forked_datatable_databases( )); continue; } + let removed = + drop_datatable_roles_before_its_database(&db, &w_id, dt_name, &mut errors).await; if let Err(e) = windmill_common::drop_custom_instance_database(&db, db_to_drop).await { errors.push(format!( "Could not drop instance database '{}' for datatable://{}: {}", db_to_drop, dt_name, e )); + if let Some(permissions) = removed { + restore_datatable_permissions(&db, &w_id, dt_name, permissions, &mut errors) + .await; + } } } else { let fork_pg = match crate::workspaces::resolve_pg_source_checked( @@ -1475,6 +1538,9 @@ pub async fn drop_forked_datatable_databases( match parent_pg.connect(Some(&db)).await { Ok((client, connection)) => { let join_handle = tokio::spawn(async move { connection.await }); + let removed = + drop_datatable_roles_before_its_database(&db, &w_id, dt_name, &mut errors) + .await; if let Err(e) = client .execute(&format!("DROP DATABASE \"{}\"", db_to_drop), &[]) .await @@ -1483,6 +1549,16 @@ pub async fn drop_forked_datatable_databases( "Could not drop database '{}' for datatable://{}: {}", db_to_drop, dt_name, e )); + if let Some(permissions) = removed { + restore_datatable_permissions( + &db, + &w_id, + dt_name, + permissions, + &mut errors, + ) + .await; + } } drop(client); let _ = windmill_common::shutdown_pg_connection(join_handle).await; diff --git a/frontend/src/lib/components/raw_apps/RawAppDataTableDrawer.svelte b/frontend/src/lib/components/raw_apps/RawAppDataTableDrawer.svelte index 6e1d1b9e1e..4bce2d30d1 100644 --- a/frontend/src/lib/components/raw_apps/RawAppDataTableDrawer.svelte +++ b/frontend/src/lib/components/raw_apps/RawAppDataTableDrawer.svelte @@ -6,6 +6,7 @@ import Button from '../common/button/Button.svelte' import { sendUserToast } from '$lib/toast' import type { DataTableRef } from './dataTableRefUtils' + import { untrack } from 'svelte' import { resource } from 'runed' import { ArrowLeft, Expand, Minimize, Plus, RefreshCcw } from 'lucide-svelte' import DBManagerContent from '../DBManagerContent.svelte' @@ -60,6 +61,56 @@ } }) + // Roles the *caller* may use, so the picker never offers one that would be + // refused. Absent/disabled permissions yield no roles and hide the picker. + const usableRoles = resource( + () => [open, opWs, selectedDatatable] as const, + async ([isOpen, workspace, datatable]) => { + if (!isOpen || !workspace || !datatable) return undefined + try { + return { + datatable, + ...(await WorkspaceService.listUsableDatatableRoles({ + workspace, + datatableName: datatable + })) + } + } catch (e) { + // Never leave the drawer waiting on this: fall back to the + // unpermissioned shape so it opens and the server picks the role. + console.error('Failed to load datatable roles:', e) + return { datatable, enabled: false, roles: [], default_role: 'admin' } + } + } + ) + + // Roles are per data table, and a resource keeps its previous value while it + // refetches. + const rolesOfCurrent = $derived( + usableRoles.current?.datatable === selectedDatatable ? usableRoles.current : undefined + ) + + // The content must not mount until the role is settled: mounting is what fires + // the schema and metadata queries, and a first round sent without a role would + // run — and cache — as whatever the server defaults to. + const roleSettled = $derived( + selectedDatatable === undefined || + (rolesOfCurrent !== undefined && + (!rolesOfCurrent.enabled || + rolesOfCurrent.roles.length === 0 || + selectedRole !== undefined)) + ) + + // Settle the role before anything queries the data table: leaving it unset + // until the user touches the picker would send the first — and cached — round + // of queries as a role they may not be allowed to use. + $effect(() => { + const roles = rolesOfCurrent + if (!roles?.enabled || selectedRole !== undefined) return + const effective = roles.roles.includes(roles.default_role) ? roles.default_role : roles.roles[0] + if (effective) untrack(() => (selectedRole = effective)) + }) + // Every data table with its schemas and tables: the tree is the picker, so it // has to cover the data tables the query editor is not pointed at. Asked for // when the drawer opens — it reaches every data table's database in turn, and @@ -186,25 +237,27 @@ noPadding > {#if dbInput && opWs} - {#key `${selectedDatatable}~${selectedRole ?? ''}`} - workerTag.tag, (v) => (workerTag.tag = v)} - bind:hasReplResult - bind:selectedSchemaKey - bind:selectedTableKey - multiSelectMode={true} - bind:selectedTables - {disabledTables} - datatableTree={datatableTree.current} - datatableTreeLoading={datatableTree.loading} - onSelectDatatable={(dt) => ((selectedDatatable = dt), (selectedRole = undefined))} - onSelectRole={(dt, role) => ((selectedDatatable = dt), (selectedRole = role))} - bind:pendingAction - /> - {/key} + {#if roleSettled} + {#key `${selectedDatatable}~${selectedRole ?? ''}`} + workerTag.tag, (v) => (workerTag.tag = v)} + bind:hasReplResult + bind:selectedSchemaKey + bind:selectedTableKey + multiSelectMode={true} + bind:selectedTables + {disabledTables} + datatableTree={datatableTree.current} + datatableTreeLoading={datatableTree.loading} + onSelectDatatable={(dt) => ((selectedDatatable = dt), (selectedRole = undefined))} + onSelectRole={(dt, role) => ((selectedDatatable = dt), (selectedRole = role))} + bind:pendingAction + /> + {/key} + {/if} {:else}
Select a data table to explore