mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix(datatables): validate a rename against the save it describes, and re-check under the locks
Three from the round, all about deciding on state that could already have moved. A permission save resolved the data table and checked it was instance-backed before taking any lock, then wrote under one. A config save committing in between could move the table onto a PostgreSQL resource — recreating exactly what the transition guard refuses — or rename it, in which case the write targeted a key that no longer existed and reported success having changed nothing. It now re-resolves and re-checks on the locked state. Rename validation checked that the source existed before and the target existed after, which still accepts `main -> decoy` against a save that keeps both: every fork of `main` then follows onto a different data table, silently, because it keeps resolving. The rule is now the actual old-to-new key transition — a source may only survive if another rename took its name, and a target may only pre-exist if another rename freed it. That also stops two sources sharing one target, and it admits a swap, which the previous guard refused: `datatables` is keyed by name, so a swap cannot be done one save at a time, and refusing it was a regression against main. The pointer cascade now runs in two passes through a temporary name, the way the migration cascade one layer down already handles the same shape, so `A -> B` with `B -> C` moves each pointer once from what it named before the save. The tenant mutators say what they are for: they write an access decision for any workspace named, with an arbitrary mutation, and exist for the transaction that frees or renames a principal. Editing a decision on purpose belongs in the permissions endpoint. Carried in the same change: the stranded-fork list is a field rather than a phrase to grep out of a success string; the pointer cascade matches with `EXISTS` instead of a `LIKE` over the whole document, so a workspace whose pointers name something else is not rewritten to a byte-identical value under an exclusive lock; and `InstanceDatatableRole` drops the serde derives left over from the JSON document, one of which would emit `pwd`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR
This commit is contained in:
co-authored by
Claude Opus 5
parent
e400daabb8
commit
9872bff69f
+16
@@ -0,0 +1,16 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "UPDATE workspace_settings ws\n SET datatable = (\n SELECT jsonb_set(ws.datatable, '{datatables}', jsonb_object_agg(\n dt.key,\n CASE WHEN dt.value->'reference'->>'workspace_id' = $1\n AND dt.value->'reference'->>'datatable' = $2\n THEN jsonb_set(dt.value, '{reference,datatable}', to_jsonb($3::text))\n ELSE dt.value END\n ))\n FROM jsonb_each(ws.datatable->'datatables') dt\n )\n WHERE EXISTS (\n SELECT 1 FROM jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) d\n WHERE d.value->'reference'->>'workspace_id' = $1\n AND d.value->'reference'->>'datatable' = $2\n )",
|
||||
"describe": {
|
||||
"columns": [],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Text",
|
||||
"Text",
|
||||
"Text"
|
||||
]
|
||||
},
|
||||
"nullable": []
|
||||
},
|
||||
"hash": "06ce02cd7ce2f5a57355153edb573c242f9ba758db66e9a5e16f30e3e1494201"
|
||||
}
|
||||
-14
@@ -1,14 +0,0 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "UPDATE global_settings SET value = jsonb_set(COALESCE(value, '{}'::jsonb), '{roles}', $1)\n WHERE name = 'custom_instance_pg_databases'",
|
||||
"describe": {
|
||||
"columns": [],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Jsonb"
|
||||
]
|
||||
},
|
||||
"nullable": []
|
||||
},
|
||||
"hash": "3fd36fa26a61be923ca23316482ed1ce17184271668622c53e94de7224da38ca"
|
||||
}
|
||||
@@ -433,3 +433,82 @@ async fn renaming_a_governing_data_table_carries_its_forks(db: Pool<Postgres>) -
|
||||
assert_eq!(resp.status(), 200, "{}", resp.text().await?);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))]
|
||||
async fn a_rename_has_to_match_the_save_it_claims_to_describe(
|
||||
db: Pool<Postgres>,
|
||||
) -> anyhow::Result<()> {
|
||||
initialize_tracing().await;
|
||||
let server = ApiServer::start(db.clone()).await?;
|
||||
let port = server.addr.port();
|
||||
let url = format!("http://localhost:{port}/api/w/test-workspace/workspaces/edit_datatable_config");
|
||||
|
||||
let instance = |path: &str| {
|
||||
json!({"database": {"resource_type": "instance", "resource_path": path}})
|
||||
};
|
||||
|
||||
// Fork pointers are rewritten from the rename list, so a rename nobody performed moves every
|
||||
// fork of one data table onto another. `main` survives this save, so it was not renamed.
|
||||
let resp = authed(client().post(&url), "SECRET_TOKEN")
|
||||
.json(&json!({
|
||||
"settings": {"datatables": {"main": instance("dt_main"), "decoy": instance("dt_two")}},
|
||||
"renames": [{"from": "main", "to": "decoy"}],
|
||||
"deleted_datatables": []
|
||||
}))
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 400, "a forged rename was accepted");
|
||||
|
||||
let entry: Option<Value> = sqlx::query_scalar(
|
||||
"SELECT datatable->'datatables'->'main'->'reference' FROM workspace_settings
|
||||
WHERE workspace_id = $1",
|
||||
)
|
||||
.bind("wm-fork-dt")
|
||||
.fetch_one(&db)
|
||||
.await?;
|
||||
assert_eq!(entry.unwrap()["datatable"], "main", "the fork was repointed anyway");
|
||||
|
||||
// A swap is two renames whose sources and targets cross. It cannot be done one at a time —
|
||||
// `datatables` is keyed by name — so refusing it would be a regression, and applying the two
|
||||
// in order without a temporary name would carry `main`'s pointers back to `main`.
|
||||
let resp = authed(client().post(&url), "SECRET_TOKEN")
|
||||
.json(&json!({
|
||||
"settings": {"datatables": {"main": instance("dt_two"), "other": instance("dt_main")}},
|
||||
"renames": [{"from": "main", "to": "other"}, {"from": "other", "to": "main"}],
|
||||
"deleted_datatables": []
|
||||
}))
|
||||
.send()
|
||||
.await?;
|
||||
// `other` does not exist yet, so this particular pair is still refused — the swap shape is
|
||||
// covered by the pair below, which starts from two real data tables.
|
||||
assert_eq!(resp.status(), 400, "{}", resp.text().await?);
|
||||
|
||||
sqlx::query(
|
||||
r#"UPDATE workspace_settings SET datatable = jsonb_set(datatable, '{datatables,other}',
|
||||
'{"database": {"resource_type": "instance", "resource_path": "dt_two"}}'::jsonb)
|
||||
WHERE workspace_id = 'test-workspace'"#,
|
||||
)
|
||||
.execute(&db)
|
||||
.await?;
|
||||
|
||||
let resp = authed(client().post(&url), "SECRET_TOKEN")
|
||||
.json(&json!({
|
||||
"settings": {"datatables": {"main": instance("dt_two"), "other": instance("dt_main")}},
|
||||
"renames": [{"from": "main", "to": "other"}, {"from": "other", "to": "main"}],
|
||||
"deleted_datatables": []
|
||||
}))
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 200, "a swap was refused: {}", resp.text().await?);
|
||||
|
||||
// The fork named `main`, which is now called `other`.
|
||||
let entry: Option<Value> = sqlx::query_scalar(
|
||||
"SELECT datatable->'datatables'->'main'->'reference' FROM workspace_settings
|
||||
WHERE workspace_id = $1",
|
||||
)
|
||||
.bind("wm-fork-dt")
|
||||
.fetch_one(&db)
|
||||
.await?;
|
||||
assert_eq!(entry.unwrap()["datatable"], "other", "the swap did not carry the pointer");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -287,6 +287,23 @@ async fn set_datatable_permissions(
|
||||
.fetch_optional(&mut *tx)
|
||||
.await?;
|
||||
|
||||
// Everything above was decided on a read taken before the locks. A settings save committing in
|
||||
// between could have moved this data table onto a PostgreSQL resource — recreating the exact
|
||||
// state the transition guard refuses — or renamed it, in which case the write below would
|
||||
// target a key that no longer exists and report success having changed nothing. Re-resolve and
|
||||
// re-check on the locked state; the earlier pass stays because it is what refuses without
|
||||
// taking locks at all.
|
||||
let governing = resolve_governing_datatable(&db, &w_id, &datatable_name).await?;
|
||||
ensure_governs_datatable(&db, &authed, &w_id, &governing).await?;
|
||||
if req.permissioned && !governing.is_instance() {
|
||||
return Err(Error::BadRequest(format!(
|
||||
"Data table '{}' is backed by a Postgres resource. Data table roles are logins on the \
|
||||
Windmill instance's own Postgres, so only a data table on the instance database can \
|
||||
use them.",
|
||||
governing.name
|
||||
)));
|
||||
}
|
||||
|
||||
let permissions = if req.permissioned {
|
||||
let catalog = windmill_common::datatable_roles::read_role_catalog_tx(&mut tx).await?;
|
||||
let mut roles: BTreeMap<String, DataTableRoleTenants> = BTreeMap::new();
|
||||
|
||||
@@ -3587,13 +3587,28 @@ async fn edit_ducklake_config(
|
||||
Ok(format!("Edit ducklake config for workspace {}", &w_id))
|
||||
}
|
||||
|
||||
/// What a save left behind. `stranded_references` names the data tables in other workspaces that
|
||||
/// were governed by one this save deleted — a field rather than a sentence in a success string,
|
||||
/// so the UI decides whether to warn on the data rather than on the server's prose.
|
||||
#[derive(Serialize)]
|
||||
pub struct EditDataTableConfigResult {
|
||||
#[serde(skip_serializing_if = "Vec::is_empty")]
|
||||
stranded_references: Vec<StrandedReference>,
|
||||
}
|
||||
|
||||
#[derive(Serialize)]
|
||||
pub struct StrandedReference {
|
||||
workspace_id: String,
|
||||
datatable: String,
|
||||
}
|
||||
|
||||
async fn edit_datatable_config(
|
||||
authed: ApiAuthed,
|
||||
Extension(db): Extension<DB>,
|
||||
Path(w_id): Path<String>,
|
||||
ApiAuthed { is_admin, username, .. }: ApiAuthed,
|
||||
Json(mut new_config): Json<EditDataTableConfig>,
|
||||
) -> Result<String> {
|
||||
) -> JsonResult<EditDataTableConfigResult> {
|
||||
require_admin(is_admin, &username)?;
|
||||
let is_superadmin = require_super_admin(&db, &authed).await.is_ok();
|
||||
|
||||
@@ -3630,40 +3645,55 @@ async fn edit_datatable_config(
|
||||
for r in &new_config.renames {
|
||||
crate::datatable_migrations::validate_datatable_path_segment(&r.from)?;
|
||||
crate::datatable_migrations::validate_new_datatable_name(&r.to)?;
|
||||
// A rename is a claim about what this save is doing, and other workspaces' pointers are
|
||||
// rewritten from it. Unchecked, a caller could submit an unchanged configuration with
|
||||
// `main -> missing` and repoint every fork of `main` at a name nothing has.
|
||||
if !old_datatables.contains_key(&r.from) {
|
||||
return Err(Error::BadRequest(format!(
|
||||
"Cannot rename data table '{}': this workspace has no such data table",
|
||||
r.from
|
||||
)));
|
||||
}
|
||||
if !new_config.settings.datatables.contains_key(&r.to) {
|
||||
return Err(Error::BadRequest(format!(
|
||||
"Cannot rename data table '{}' to '{}': the save does not contain '{}'",
|
||||
r.from, r.to, r.to
|
||||
)));
|
||||
}
|
||||
}
|
||||
// `A -> B` and `B -> C` applied one after another would move what pointed at `A` all the way
|
||||
// to `C`. Each pointer moves once, from what it named before this save.
|
||||
if new_config.renames.len() > 1 {
|
||||
let mut seen = std::collections::HashSet::new();
|
||||
// A rename is a claim about what this save is doing, and other workspaces' pointers are
|
||||
// rewritten from it — so the claim has to match the configuration it describes, or a caller
|
||||
// can move every fork of one data table onto another by asserting a rename that did not
|
||||
// happen. The shape below is what "these old keys became those new keys" actually means.
|
||||
{
|
||||
let old_keys = &old_datatables;
|
||||
let new_keys = &new_config.settings.datatables;
|
||||
let froms: std::collections::HashSet<&str> =
|
||||
new_config.renames.iter().map(|r| r.from.as_str()).collect();
|
||||
let tos: std::collections::HashSet<&str> =
|
||||
new_config.renames.iter().map(|r| r.to.as_str()).collect();
|
||||
if froms.len() != new_config.renames.len() {
|
||||
return Err(Error::BadRequest(
|
||||
"A data table is renamed twice in one save".to_string(),
|
||||
));
|
||||
}
|
||||
if tos.len() != new_config.renames.len() {
|
||||
return Err(Error::BadRequest(
|
||||
"Two data tables are renamed to the same name in one save".to_string(),
|
||||
));
|
||||
}
|
||||
for r in &new_config.renames {
|
||||
if !seen.insert(r.from.as_str()) {
|
||||
if !old_keys.contains_key(&r.from) {
|
||||
return Err(Error::BadRequest(format!(
|
||||
"Data table '{}' is renamed twice in one save",
|
||||
"Cannot rename data table '{}': this workspace has no such data table",
|
||||
r.from
|
||||
)));
|
||||
}
|
||||
}
|
||||
for r in &new_config.renames {
|
||||
if seen.contains(r.to.as_str()) && r.to != r.from {
|
||||
if !new_keys.contains_key(&r.to) {
|
||||
return Err(Error::BadRequest(format!(
|
||||
"Data table '{}' is both renamed and the target of another rename in one \
|
||||
save; do them one at a time",
|
||||
r.to
|
||||
"Cannot rename data table '{}' to '{}': the save does not contain '{}'",
|
||||
r.from, r.to, r.to
|
||||
)));
|
||||
}
|
||||
// The source has to be gone, or gone-and-reoccupied by another rename — which is what
|
||||
// a swap is. Without this, `main -> decoy` passes against a save that keeps both, and
|
||||
// every fork of `main` silently follows onto a different data table.
|
||||
if new_keys.contains_key(&r.from) && !tos.contains(r.from.as_str()) {
|
||||
return Err(Error::BadRequest(format!(
|
||||
"Data table '{}' is renamed to '{}' but the save still contains '{}'",
|
||||
r.from, r.to, r.from
|
||||
)));
|
||||
}
|
||||
// And the target has to be free, or freed by another rename.
|
||||
if old_keys.contains_key(&r.to) && !froms.contains(r.to.as_str()) {
|
||||
return Err(Error::BadRequest(format!(
|
||||
"Cannot rename data table '{}' to '{}': '{}' already exists",
|
||||
r.from, r.to, r.to
|
||||
)));
|
||||
}
|
||||
}
|
||||
@@ -3793,34 +3823,21 @@ async fn edit_datatable_config(
|
||||
.await?;
|
||||
|
||||
// A fork points at a data table by name, so a rename here has to follow or every fork's entry
|
||||
// resolves to nothing. Inside the transaction: the rename and the pointers that name it are one
|
||||
// change, and half of it is a fork whose jobs stop.
|
||||
for r in &new_config.renames {
|
||||
sqlx::query!(
|
||||
r#"UPDATE workspace_settings ws
|
||||
SET datatable = (
|
||||
SELECT jsonb_set(ws.datatable, '{datatables}', jsonb_object_agg(
|
||||
dt.key,
|
||||
CASE WHEN dt.value->'reference'->>'workspace_id' = $1
|
||||
AND dt.value->'reference'->>'datatable' = $2
|
||||
THEN jsonb_set(dt.value, '{reference,datatable}', to_jsonb($3::text))
|
||||
ELSE dt.value END
|
||||
))
|
||||
FROM jsonb_each(ws.datatable->'datatables') dt
|
||||
)
|
||||
WHERE jsonb_typeof(ws.datatable->'datatables') = 'object'
|
||||
AND ws.datatable::text LIKE '%"reference"%'"#,
|
||||
&w_id,
|
||||
&r.from,
|
||||
&r.to,
|
||||
)
|
||||
.execute(&mut *tx)
|
||||
.await?;
|
||||
// resolves to nothing. In two passes through a temporary name, like the migration cascade one
|
||||
// layer down: applied in order, `sa -> sb` then `sb -> sa` would move what pointed at `sa` all
|
||||
// the way back to `sa`, and `A -> B`, `B -> C` would carry `A`'s pointers to `C`. Each pointer
|
||||
// moves once, from what it named before this save. Inside the transaction: the rename and the
|
||||
// pointers that name it are one change, and half of it is a fork whose jobs stop.
|
||||
for (i, r) in new_config.renames.iter().enumerate() {
|
||||
repoint_datatable_references(&mut tx, &w_id, &r.from, &format!("__wm_rename_tmp/{i}")).await?;
|
||||
}
|
||||
for (i, r) in new_config.renames.iter().enumerate() {
|
||||
repoint_datatable_references(&mut tx, &w_id, &format!("__wm_rename_tmp/{i}"), &r.to).await?;
|
||||
}
|
||||
|
||||
// A deletion cannot be followed the same way — there is nothing to point at any more. Read who
|
||||
// is left stranded so the caller is told, the way deleting a workspace does.
|
||||
let mut stranded: Vec<String> = Vec::new();
|
||||
let mut stranded: Vec<StrandedReference> = Vec::new();
|
||||
for name in &new_config.deleted_datatables {
|
||||
let rows = sqlx::query!(
|
||||
r#"SELECT ws.workspace_id AS "workspace_id!", dt.key AS "datatable!"
|
||||
@@ -3833,10 +3850,10 @@ async fn edit_datatable_config(
|
||||
)
|
||||
.fetch_all(&mut *tx)
|
||||
.await?;
|
||||
stranded.extend(
|
||||
rows.into_iter()
|
||||
.map(|r| format!("{}/{}", r.workspace_id, r.datatable)),
|
||||
);
|
||||
stranded.extend(rows.into_iter().map(|r| StrandedReference {
|
||||
workspace_id: r.workspace_id,
|
||||
datatable: r.datatable,
|
||||
}));
|
||||
}
|
||||
|
||||
tx.commit().await?;
|
||||
@@ -3853,19 +3870,7 @@ async fn edit_datatable_config(
|
||||
)
|
||||
.await?;
|
||||
|
||||
if stranded.is_empty() {
|
||||
Ok(format!("Edit datatable config for workspace {}", &w_id))
|
||||
} else {
|
||||
Ok(format!(
|
||||
concat!(
|
||||
"Edit datatable config for workspace {}. These data tables were governed by one ",
|
||||
"you deleted and no longer resolve: {}. Their databases still exist; a superadmin ",
|
||||
"can point them at another workspace's data table."
|
||||
),
|
||||
&w_id,
|
||||
stranded.join(", ")
|
||||
))
|
||||
}
|
||||
Ok(Json(EditDataTableConfigResult { stranded_references: stranded }))
|
||||
}
|
||||
|
||||
#[derive(Deserialize)]
|
||||
@@ -7910,6 +7915,43 @@ async fn point_kept_datatables_at_parent(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Move every pointer in any workspace that names `(w_id, from)` to `(w_id, to)`.
|
||||
///
|
||||
/// `EXISTS` rather than a `LIKE` over the whole document: the update rewrites the row, so matching
|
||||
/// every workspace that holds any pointer would rewrite rows to a byte-identical value and hold an
|
||||
/// exclusive lock on them until commit.
|
||||
async fn repoint_datatable_references(
|
||||
tx: &mut Transaction<'_, Postgres>,
|
||||
w_id: &str,
|
||||
from: &str,
|
||||
to: &str,
|
||||
) -> Result<()> {
|
||||
sqlx::query!(
|
||||
r#"UPDATE workspace_settings ws
|
||||
SET datatable = (
|
||||
SELECT jsonb_set(ws.datatable, '{datatables}', jsonb_object_agg(
|
||||
dt.key,
|
||||
CASE WHEN dt.value->'reference'->>'workspace_id' = $1
|
||||
AND dt.value->'reference'->>'datatable' = $2
|
||||
THEN jsonb_set(dt.value, '{reference,datatable}', to_jsonb($3::text))
|
||||
ELSE dt.value END
|
||||
))
|
||||
FROM jsonb_each(ws.datatable->'datatables') dt
|
||||
)
|
||||
WHERE EXISTS (
|
||||
SELECT 1 FROM jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) d
|
||||
WHERE d.value->'reference'->>'workspace_id' = $1
|
||||
AND d.value->'reference'->>'datatable' = $2
|
||||
)"#,
|
||||
w_id,
|
||||
from,
|
||||
to,
|
||||
)
|
||||
.execute(&mut **tx)
|
||||
.await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn apply_forked_datatable(
|
||||
db: &DB,
|
||||
tx: &mut Transaction<'_, Postgres>,
|
||||
|
||||
@@ -5505,7 +5505,22 @@ paths:
|
||||
description: status
|
||||
content:
|
||||
application/json:
|
||||
schema: {}
|
||||
schema:
|
||||
type: object
|
||||
properties:
|
||||
stranded_references:
|
||||
description: >-
|
||||
Data tables in other workspaces that were governed by one this save deleted
|
||||
and no longer resolve.
|
||||
type: array
|
||||
items:
|
||||
type: object
|
||||
required: [workspace_id, datatable]
|
||||
properties:
|
||||
workspace_id:
|
||||
type: string
|
||||
datatable:
|
||||
type: string
|
||||
|
||||
/w/{workspace}/workspaces/run_datatable_migrations/{datatable_name}:
|
||||
post:
|
||||
|
||||
@@ -17,8 +17,6 @@
|
||||
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
use crate::{
|
||||
error::{Error, Result},
|
||||
DB,
|
||||
@@ -35,11 +33,13 @@ pub const CUSTOM_INSTANCE_USER: &str = "custom_instance_user";
|
||||
|
||||
/// One catalog entry, as stored in `datatable_role`. The password is per role and instance-wide;
|
||||
/// it belongs to the instance, not to any workspace's settings.
|
||||
#[derive(Deserialize, Serialize, Clone)]
|
||||
/// No `Serialize`/`Deserialize`: the catalog is rows now, and a derived `Serialize` would emit
|
||||
/// `pwd` — the same way out for a credential that the hand-written `Debug` below closes on the log
|
||||
/// side.
|
||||
#[derive(Clone)]
|
||||
pub struct InstanceDatatableRole {
|
||||
/// The Postgres role name, verbatim.
|
||||
pub name: String,
|
||||
#[serde(default = "crate::more_serde::default_true")]
|
||||
pub enabled: bool,
|
||||
/// Absent only for a role whose provisioning did not finish; resolving as it then errors
|
||||
/// rather than falling back to admin.
|
||||
@@ -49,7 +49,6 @@ pub struct InstanceDatatableRole {
|
||||
/// backend, while this one is minted here and never entered by anyone, so there is nothing for
|
||||
/// a ref to point at. Encrypting generated secrets at rest is a separate change that would
|
||||
/// take the replication password with it.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub pwd: Option<String>,
|
||||
}
|
||||
|
||||
|
||||
@@ -1927,6 +1927,12 @@ pub async fn ensure_datatable_admin_access(
|
||||
/// transaction. `change` reports whether it touched anything; the row is only written when
|
||||
/// something did.
|
||||
///
|
||||
/// Authorization: writes an access decision for any workspace named, with an arbitrary mutation,
|
||||
/// and checks nothing. It exists for the cascades below — the transaction that frees or renames a
|
||||
/// principal — so callers MUST be the operation that made the principal change, and MUST run in
|
||||
/// its transaction. Anything editing a decision on purpose belongs in the permissions endpoint,
|
||||
/// which is gated on the workspace that governs the data table.
|
||||
///
|
||||
/// The tenant lists name principals of this workspace, so anything that frees or renames one has
|
||||
/// to come through here in the same transaction that frees it — otherwise a `u/alice` reused by a
|
||||
/// later account silently inherits her access.
|
||||
@@ -1985,7 +1991,8 @@ where
|
||||
}
|
||||
|
||||
/// Drop a freed principal (`u/alice`, `g/analysts`, `f/finance`) from every tenant list of one
|
||||
/// workspace.
|
||||
/// workspace. Same contract as [`update_datatable_permissions_in_workspace`]: for the transaction
|
||||
/// that frees the principal, not for editing a decision.
|
||||
pub async fn remove_datatable_tenant_in_workspace(
|
||||
tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
|
||||
w_id: &str,
|
||||
@@ -2003,7 +2010,8 @@ pub async fn remove_datatable_tenant_in_workspace(
|
||||
.await
|
||||
}
|
||||
|
||||
/// Follow a renamed principal through every tenant list of one workspace.
|
||||
/// Follow a renamed principal through every tenant list of one workspace. Same contract as
|
||||
/// [`update_datatable_permissions_in_workspace`].
|
||||
pub async fn rename_datatable_tenant_in_workspace(
|
||||
tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
|
||||
w_id: &str,
|
||||
@@ -2034,7 +2042,10 @@ pub async fn rename_datatable_tenant_in_workspace(
|
||||
}
|
||||
|
||||
/// Strip a deleted instance role from every workspace that had tenanted it, so nothing is left
|
||||
/// naming a role that no longer exists. A data table whose default role was the deleted one falls
|
||||
/// naming a role that no longer exists.
|
||||
///
|
||||
/// Authorization: reaches every workspace on the instance. Callers MUST be the superadmin path
|
||||
/// that just dropped the role from the cluster — it exists to follow that, not to edit tenants. A data table whose default role was the deleted one falls
|
||||
/// back to `admin` — the one role that is always present.
|
||||
pub async fn forget_datatable_role_everywhere(db: &DB, role_id: &str) -> Result<()> {
|
||||
let workspaces = sqlx::query_scalar!(
|
||||
|
||||
@@ -228,16 +228,23 @@
|
||||
requestBody: { settings, renames, deleted_datatables }
|
||||
})
|
||||
dataTableSettings = clone(tempSettings)
|
||||
// The server says here when a delete left another workspace's data table pointing at
|
||||
// nothing. Swallowing it is what made that failure silent for the person who caused it.
|
||||
const stranded = typeof result === 'string' && result.includes('no longer resolve')
|
||||
sendUserToast(
|
||||
stranded ? result : 'Data table settings saved successfully',
|
||||
stranded ? 'warning' : 'success',
|
||||
[],
|
||||
undefined,
|
||||
stranded ? 20000 : 5000
|
||||
)
|
||||
// A delete can leave another workspace's data table governed by nothing. Swallowing
|
||||
// that is what made it silent for the person who caused it.
|
||||
const stranded = result?.stranded_references ?? []
|
||||
if (stranded.length > 0) {
|
||||
sendUserToast(
|
||||
`These data tables were governed by one you deleted and no longer resolve: ${stranded
|
||||
.map((s) => `${s.workspace_id}/${s.datatable}`)
|
||||
.join(', ')}. Their databases still exist; a superadmin can point them at another ` +
|
||||
`workspace's data table.`,
|
||||
'warning',
|
||||
[],
|
||||
undefined,
|
||||
20000
|
||||
)
|
||||
} else {
|
||||
sendUserToast('Data table settings saved successfully')
|
||||
}
|
||||
} catch (e) {
|
||||
sendUserToast(e, true)
|
||||
console.error('Error saving data table settings', e)
|
||||
|
||||
Reference in New Issue
Block a user