From 497f24bca7a112d58a7695111e02cba7d4e32731 Mon Sep 17 00:00:00 2001 From: Diego Imbert Date: Fri, 4 Sep 2026 22:42:29 +0200 Subject: [PATCH] feat(datatables): a fork shares its parent's data table, restrictions and all MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A permissioned data table used to be dropped from a fork's config outright: copying it would either hand every fork member the connection that owns the parent's database, or freeze the parent's tenant lists at the moment of the fork. So a fork could not use one at all. It now gets a pointer instead — `shared_datatables: {"": {"from": ""}}`, deliberately outside `datatables`, so nothing that plans role drops or reads which logins a workspace claims can see it. Resolution follows the pointer and asks every question of the workspace that owns it: its live config, its live roles, and the caller as *that* workspace knows them. Checking the fork's own identity would have made forking the way to reach a data table as admin, since `can_use_datatable_role` starts at `is_admin` and a fork's owner is an admin of their own fork. A member of the fork who is not a member of the owner is refused, and does not see the data table at all. A job permissioned as a folder or a group is refused too: those exist per workspace, so the fork's admin creates both sides of the name. Administering it — permissions, ACLs, migrations, role drops — is refused from the fork and belongs to the workspace that owns it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01S5arH3G2Sa1Qqm32veJQ1n --- ...fa179d323f60b27e27b7094830350e839f61c.json | 22 + ...29a58406dd999eb973a4212ccc3ed5ec325f0.json | 29 ++ ...61b6e7b9d35a41594681e2a92a87359e6a018.json | 23 ++ ...a9740dcd95424df17059da709b7df2d033db2.json | 22 - ...b3656e27f1c962c2f0438e0cc06530661fbf8.json | 22 - ...6cfcc34b0a89f88964273d1272188a9765f7c.json | 15 + ...ad8f3c7829d2f6c4320ec716f3585ee28b0cb.json | 16 + .../tests/datatable_tenants.rs | 87 ++++ .../src/datatable_permissions.rs | 36 +- .../windmill-api-workspaces/src/workspaces.rs | 143 ++++--- backend/windmill-api/openapi.yaml | 8 +- backend/windmill-common/src/workspaces.rs | 384 ++++++++++++++++-- .../ForkDatatableSection.svelte | 24 +- 13 files changed, 663 insertions(+), 168 deletions(-) create mode 100644 backend/.sqlx/query-1160df7cf1a3ba621949c9a17eafa179d323f60b27e27b7094830350e839f61c.json create mode 100644 backend/.sqlx/query-27067c8d9a35db71592e154277629a58406dd999eb973a4212ccc3ed5ec325f0.json create mode 100644 backend/.sqlx/query-58e5cfe9eb87bda9f7de87c403861b6e7b9d35a41594681e2a92a87359e6a018.json delete mode 100644 backend/.sqlx/query-800e04197578631bd75b2f8e511a9740dcd95424df17059da709b7df2d033db2.json delete mode 100644 backend/.sqlx/query-dc60d43814e2b5220db648677dbb3656e27f1c962c2f0438e0cc06530661fbf8.json create mode 100644 backend/.sqlx/query-f4c9adda04d44527bff3c8da3696cfcc34b0a89f88964273d1272188a9765f7c.json create mode 100644 backend/.sqlx/query-f8e1ebb6e468835a90ef9dc32bead8f3c7829d2f6c4320ec716f3585ee28b0cb.json diff --git a/backend/.sqlx/query-1160df7cf1a3ba621949c9a17eafa179d323f60b27e27b7094830350e839f61c.json b/backend/.sqlx/query-1160df7cf1a3ba621949c9a17eafa179d323f60b27e27b7094830350e839f61c.json new file mode 100644 index 0000000000..5fea643175 --- /dev/null +++ b/backend/.sqlx/query-1160df7cf1a3ba621949c9a17eafa179d323f60b27e27b7094830350e839f61c.json @@ -0,0 +1,22 @@ +{ + "db_name": "PostgreSQL", + "query": "\n SELECT ws.datatable AS settings\n FROM workspace_settings ws\n WHERE ws.workspace_id = $1\n ", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "settings", + "type_info": "Jsonb" + } + ], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [ + true + ] + }, + "hash": "1160df7cf1a3ba621949c9a17eafa179d323f60b27e27b7094830350e839f61c" +} diff --git a/backend/.sqlx/query-27067c8d9a35db71592e154277629a58406dd999eb973a4212ccc3ed5ec325f0.json b/backend/.sqlx/query-27067c8d9a35db71592e154277629a58406dd999eb973a4212ccc3ed5ec325f0.json new file mode 100644 index 0000000000..1041fb782f --- /dev/null +++ b/backend/.sqlx/query-27067c8d9a35db71592e154277629a58406dd999eb973a4212ccc3ed5ec325f0.json @@ -0,0 +1,29 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT ws.datatable->'datatables'->$2 AS own,\n ws.datatable->'shared_datatables'->$2->>'from' AS shared_from\n FROM workspace_settings ws WHERE ws.workspace_id = $1", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "own", + "type_info": "Jsonb" + }, + { + "ordinal": 1, + "name": "shared_from", + "type_info": "Text" + } + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + }, + "nullable": [ + null, + null + ] + }, + "hash": "27067c8d9a35db71592e154277629a58406dd999eb973a4212ccc3ed5ec325f0" +} diff --git a/backend/.sqlx/query-58e5cfe9eb87bda9f7de87c403861b6e7b9d35a41594681e2a92a87359e6a018.json b/backend/.sqlx/query-58e5cfe9eb87bda9f7de87c403861b6e7b9d35a41594681e2a92a87359e6a018.json new file mode 100644 index 0000000000..f4f7acd0b7 --- /dev/null +++ b/backend/.sqlx/query-58e5cfe9eb87bda9f7de87c403861b6e7b9d35a41594681e2a92a87359e6a018.json @@ -0,0 +1,23 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT username FROM usr WHERE workspace_id = $1 AND email = $2 AND disabled = false", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "username", + "type_info": "Varchar" + } + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + }, + "nullable": [ + false + ] + }, + "hash": "58e5cfe9eb87bda9f7de87c403861b6e7b9d35a41594681e2a92a87359e6a018" +} diff --git a/backend/.sqlx/query-800e04197578631bd75b2f8e511a9740dcd95424df17059da709b7df2d033db2.json b/backend/.sqlx/query-800e04197578631bd75b2f8e511a9740dcd95424df17059da709b7df2d033db2.json deleted file mode 100644 index bfc791daf5..0000000000 --- a/backend/.sqlx/query-800e04197578631bd75b2f8e511a9740dcd95424df17059da709b7df2d033db2.json +++ /dev/null @@ -1,22 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "\n SELECT jsonb_object_keys(ws.datatable->'datatables') AS datatable_name\n FROM workspace_settings ws\n WHERE ws.workspace_id = $1\n ", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "datatable_name", - "type_info": "Text" - } - ], - "parameters": { - "Left": [ - "Text" - ] - }, - "nullable": [ - null - ] - }, - "hash": "800e04197578631bd75b2f8e511a9740dcd95424df17059da709b7df2d033db2" -} diff --git a/backend/.sqlx/query-dc60d43814e2b5220db648677dbb3656e27f1c962c2f0438e0cc06530661fbf8.json b/backend/.sqlx/query-dc60d43814e2b5220db648677dbb3656e27f1c962c2f0438e0cc06530661fbf8.json deleted file mode 100644 index 0d36677337..0000000000 --- a/backend/.sqlx/query-dc60d43814e2b5220db648677dbb3656e27f1c962c2f0438e0cc06530661fbf8.json +++ /dev/null @@ -1,22 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "\n SELECT ws.datatable->'datatables' AS datatables\n FROM workspace_settings ws\n WHERE ws.workspace_id = $1\n ", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "datatables", - "type_info": "Jsonb" - } - ], - "parameters": { - "Left": [ - "Text" - ] - }, - "nullable": [ - null - ] - }, - "hash": "dc60d43814e2b5220db648677dbb3656e27f1c962c2f0438e0cc06530661fbf8" -} diff --git a/backend/.sqlx/query-f4c9adda04d44527bff3c8da3696cfcc34b0a89f88964273d1272188a9765f7c.json b/backend/.sqlx/query-f4c9adda04d44527bff3c8da3696cfcc34b0a89f88964273d1272188a9765f7c.json new file mode 100644 index 0000000000..8dc466f9be --- /dev/null +++ b/backend/.sqlx/query-f4c9adda04d44527bff3c8da3696cfcc34b0a89f88964273d1272188a9765f7c.json @@ -0,0 +1,15 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE workspace_settings\n SET datatable = $1::jsonb || jsonb_build_object('shared_datatables', COALESCE((\n SELECT jsonb_object_agg(key, value)\n FROM jsonb_each(CASE WHEN jsonb_typeof(datatable->'shared_datatables') = 'object'\n THEN datatable->'shared_datatables' ELSE '{}'::jsonb END)\n WHERE NOT ($1::jsonb->'datatables' ? key)\n ), '{}'::jsonb))\n WHERE workspace_id = $2", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Jsonb", + "Text" + ] + }, + "nullable": [] + }, + "hash": "f4c9adda04d44527bff3c8da3696cfcc34b0a89f88964273d1272188a9765f7c" +} diff --git a/backend/.sqlx/query-f8e1ebb6e468835a90ef9dc32bead8f3c7829d2f6c4320ec716f3585ee28b0cb.json b/backend/.sqlx/query-f8e1ebb6e468835a90ef9dc32bead8f3c7829d2f6c4320ec716f3585ee28b0cb.json new file mode 100644 index 0000000000..8105690a40 --- /dev/null +++ b/backend/.sqlx/query-f8e1ebb6e468835a90ef9dc32bead8f3c7829d2f6c4320ec716f3585ee28b0cb.json @@ -0,0 +1,16 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE workspace_settings\n SET datatable = jsonb_set(\n jsonb_set(datatable, '{datatables}', (\n SELECT COALESCE(jsonb_object_agg(\n key,\n CASE WHEN key = ANY($2) THEN value - 'permissions' ELSE value END\n ), '{}'::jsonb)\n FROM jsonb_each(datatable->'datatables')\n WHERE key = ANY($2)\n OR COALESCE((value->'permissions'->>'enabled')::boolean, false) = false\n )),\n '{shared_datatables}', (\n SELECT COALESCE(jsonb_object_agg(key, entry), '{}'::jsonb) FROM (\n SELECT key, jsonb_build_object('from', $3::text) AS entry\n FROM jsonb_each(datatable->'datatables')\n WHERE NOT (key = ANY($2))\n AND COALESCE((value->'permissions'->>'enabled')::boolean, false) = true\n UNION ALL\n SELECT key, value AS entry\n FROM jsonb_each(CASE\n WHEN jsonb_typeof(datatable->'shared_datatables') = 'object'\n THEN datatable->'shared_datatables' ELSE '{}'::jsonb END)\n WHERE NOT (key = ANY($2))\n ) shared\n ))\n WHERE workspace_id = $1 AND jsonb_typeof(datatable->'datatables') = 'object'", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "TextArray", + "Text" + ] + }, + "nullable": [] + }, + "hash": "f8e1ebb6e468835a90ef9dc32bead8f3c7829d2f6c4320ec716f3585ee28b0cb" +} diff --git a/backend/windmill-api-integration-tests/tests/datatable_tenants.rs b/backend/windmill-api-integration-tests/tests/datatable_tenants.rs index 2dd476afd0..3eec2820a1 100644 --- a/backend/windmill-api-integration-tests/tests/datatable_tenants.rs +++ b/backend/windmill-api-integration-tests/tests/datatable_tenants.rs @@ -106,3 +106,90 @@ async fn freeing_a_principal_takes_its_datatable_tenant(db: Pool) -> a Ok(()) } + +/// A fork does not copy a permissioned data table, it points at it — and every +/// question about who may run as what is asked of the workspace it points at. +/// Asking it of the fork instead would make forking a workspace the way to reach +/// its data table as admin: `can_use_datatable_role` starts at `is_admin`, and a +/// fork's owner is an admin of their own fork. +#[sqlx::test(migrations = "../migrations", fixtures("base"))] +async fn a_shared_data_table_is_governed_by_the_workspace_that_owns_it( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + + sqlx::query( + r#"UPDATE workspace_settings SET datatable = $1 WHERE workspace_id = 'test-workspace'"#, + ) + .bind(json!({ + "datatables": { + "main": { + "database": { "resource_type": "instance", "resource_path": "dt_main" }, + "permissions": { "enabled": true, "roles": { + "admin": { "tenants": [] }, + "analyst": { "tenants": ["u/test-user-2"] } + }} + } + } + })) + .execute(&db) + .await?; + + // A fork of it: no config of its own, one pointer, and both users are admins + // of the fork. test-user-3 is only that — the parent does not know them. + sqlx::query( + "INSERT INTO workspace (id, name, owner, parent_workspace_id) + VALUES ('wm-fork-t', 'wm-fork-t', 'test-user', 'test-workspace')", + ) + .execute(&db) + .await?; + sqlx::query("INSERT INTO workspace_settings (workspace_id, datatable) VALUES ('wm-fork-t', $1)") + .bind(json!({ "datatables": {}, "shared_datatables": { "main": { "from": "test-workspace" } } })) + .execute(&db) + .await?; + sqlx::query( + "INSERT INTO usr (workspace_id, email, username, is_admin, role) VALUES + ('wm-fork-t', 'test2@windmill.dev', 'test-user-2', true, 'Admin'), + ('wm-fork-t', 'test3@windmill.dev', 'test-user-3', true, 'Admin')", + ) + .execute(&db) + .await?; + sqlx::query("DELETE FROM usr WHERE workspace_id = 'test-workspace' AND email = 'test3@windmill.dev'") + .execute(&db) + .await?; + + let roles = |token: &'static str| async move { + authed( + client().get(format!( + "http://localhost:{port}/api/w/wm-fork-t/workspaces/datatable_usable_roles/main" + )), + token, + ) + .send() + .await + }; + + // An admin of the fork, a plain member of the parent: the role the parent + // gave them, and only that one. + let resp = roles("SECRET_TOKEN_2").await?; + assert_eq!(resp.status(), 200); + let body: serde_json::Value = resp.json().await?; + assert_eq!(body["roles"], json!(["analyst"])); + + // A member of the fork alone: the data table is not theirs to see at all. + let resp = roles("SECRET_TOKEN_3").await?; + assert_eq!(resp.status(), 404, "{}", resp.text().await?); + let resp = authed( + client().get(format!( + "http://localhost:{port}/api/w/wm-fork-t/workspaces/list_datatables" + )), + "SECRET_TOKEN_3", + ) + .send() + .await?; + assert_eq!(resp.json::().await?, json!([])); + + Ok(()) +} diff --git a/backend/windmill-api-workspaces/src/datatable_permissions.rs b/backend/windmill-api-workspaces/src/datatable_permissions.rs index 3d56bcb14d..6587cf90e0 100644 --- a/backend/windmill-api-workspaces/src/datatable_permissions.rs +++ b/backend/windmill-api-workspaces/src/datatable_permissions.rs @@ -182,15 +182,27 @@ pub(crate) async fn read_datatable_unchecked( w_id: &str, datatable_name: &str, ) -> Result { - let value = sqlx::query_scalar!( - "SELECT ws.datatable->'datatables'->$2 FROM workspace_settings ws WHERE ws.workspace_id = $1", + let row = sqlx::query!( + "SELECT ws.datatable->'datatables'->$2 AS own, + ws.datatable->'shared_datatables'->$2->>'from' AS shared_from + FROM workspace_settings ws WHERE ws.workspace_id = $1", w_id, datatable_name, ) .fetch_one(db) - .await? - .filter(|v| !v.is_null()) - .ok_or_else(|| Error::NotFound(format!("Data table '{datatable_name}' not found")))?; + .await?; + let value = row + .own + .filter(|v| !v.is_null()) + .ok_or_else(|| match row.shared_from { + // Everything reading a data table's own config administers it, and a + // data table is administered where it lives. + Some(owner) => Error::NotAuthorized(format!( + "Data table '{datatable_name}' is shared from workspace '{owner}': \ + it can only be administered from there" + )), + None => Error::NotFound(format!("Data table '{datatable_name}' not found")), + })?; serde_json::from_value(value) .map_err(|e| Error::internal_err(format!("Invalid data table config: {e}"))) } @@ -730,22 +742,28 @@ async fn list_usable_datatable_roles( Extension(db): Extension, Path((w_id, datatable_name)): Path<(String, String)>, ) -> JsonResult { - let datatable = read_datatable_unchecked(&db, &w_id, &datatable_name).await?; - let Some(permissions) = datatable.permissions.filter(|p| p.enabled) else { + // Through the shared-aware lookup: a fork sees its parent's permissioned data + // tables, and the roles it may use on one are the ones it may use *there*. + let authed_ref = authed.to_authed_ref(); + let usable = windmill_common::workspaces::usable_datatables(&db, &authed_ref, &w_id) + .await? + .remove(&datatable_name) + .ok_or_else(|| Error::NotFound(format!("Data table '{datatable_name}' not found")))?; + let who = usable.who(&authed_ref); + let Some(permissions) = usable.datatable.permissions.as_ref().filter(|p| p.enabled) else { return Ok(Json(UsableDatatableRoles { enabled: false, roles: vec![], default_role: ADMIN_DATATABLE_ROLE.to_string(), })); }; - let authed_ref = authed.to_authed_ref(); Ok(Json(UsableDatatableRoles { enabled: true, default_role: permissions.default_role().to_string(), roles: permissions .roles .iter() - .filter(|(_, role)| can_use_datatable_role(role, &authed_ref)) + .filter(|(_, role)| can_use_datatable_role(role, &who)) .map(|(name, _)| name.clone()) .collect(), })) diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index c597f5ba63..087e70b015 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -2111,33 +2111,40 @@ struct DataTableListItem { name: String, resource_type: String, resource_path: String, + /// The workspace this data table belongs to, when it is not this one. Its + /// database is not this workspace's to name, so the two fields above are + /// blank for it. + #[serde(skip_serializing_if = "Option::is_none")] + shared_from: Option, } async fn list_datatables( - _authed: ApiAuthed, + authed: ApiAuthed, Extension(db): Extension, Path(w_id): Path, ) -> JsonResult> { - let config = sqlx::query_scalar!( - "SELECT datatable->'datatables' FROM workspace_settings WHERE workspace_id = $1", - &w_id - ) - .fetch_one(&db) - .await?; - - let items: Vec = match config { - Some(val) => { - let map: HashMap = serde_json::from_value(val).unwrap_or_default(); - map.into_iter() - .map(|(name, dt)| DataTableListItem { + let authed_ref = authed.to_authed_ref(); + let items = windmill_common::workspaces::usable_datatables(&db, &authed_ref, &w_id) + .await? + .into_iter() + .map(|(name, usable)| { + if usable.owner_w_id == w_id { + DataTableListItem { name, - resource_type: dt.database.resource_type.as_ref().to_string(), - resource_path: dt.database.resource_path, - }) - .collect() - } - None => vec![], - }; + resource_type: usable.datatable.database.resource_type.as_ref().to_string(), + resource_path: usable.datatable.database.resource_path, + shared_from: None, + } + } else { + DataTableListItem { + name, + resource_type: String::new(), + resource_path: String::new(), + shared_from: Some(usable.owner_w_id), + } + } + }) + .collect(); Ok(Json(items)) } @@ -2338,7 +2345,7 @@ async fn list_datatable_schemas( Extension(db): Extension, Path(w_id): Path, ) -> JsonResult> { - let datatable_names = list_datatable_names(&db, &w_id).await?; + let datatable_names = list_datatable_names(&db, &authed, &w_id).await?; let mut results = Vec::new(); for datatable_name in datatable_names { @@ -2371,8 +2378,8 @@ async fn list_datatable_tables( Path(w_id): Path, Query(query): Query, ) -> JsonResult> { - let datatable_names = list_datatable_names(&db, &w_id).await?; let mut roles = list_datatable_roles(&db, &authed, &w_id).await?; + let datatable_names: Vec = roles.keys().cloned().collect(); let mut results = Vec::new(); for datatable_name in datatable_names { @@ -2424,26 +2431,20 @@ async fn list_datatable_roles( authed: &ApiAuthed, w_id: &str, ) -> Result, String)>> { - let Some(datatables) = sqlx::query_scalar!( - "SELECT ws.datatable->'datatables' FROM workspace_settings ws WHERE ws.workspace_id = $1", - w_id - ) - .fetch_optional(db) - .await? - .flatten() - .and_then(|v| serde_json::from_value::>(v).ok()) else { - return Ok(HashMap::new()); - }; - let authed_ref = authed.to_authed_ref(); - Ok(datatables + let usable = windmill_common::workspaces::usable_datatables(db, &authed_ref, w_id).await?; + Ok(usable .into_iter() - .map(|(name, dt)| { - let info = match dt.permissions.filter(|p| p.enabled) { + .map(|(name, usable)| { + // A shared data table's tenancy is its owner's question, so it is + // asked of who the caller is there. + let who = usable.who(&authed_ref); + let permissions = usable.datatable.permissions.as_ref().filter(|p| p.enabled); + let info = match permissions { Some(p) => ( p.roles .iter() - .filter(|(_, role)| can_use_datatable_role(role, &authed_ref)) + .filter(|(_, role)| can_use_datatable_role(role, &who)) .map(|(role_name, _)| role_name.clone()) .collect(), p.default_role().to_string(), @@ -2499,20 +2500,14 @@ async fn get_datatable_resource_as_default_role( .await } -async fn list_datatable_names(db: &DB, w_id: &str) -> Result> { - Ok(sqlx::query_scalar!( - r#" - SELECT jsonb_object_keys(ws.datatable->'datatables') AS datatable_name - FROM workspace_settings ws - WHERE ws.workspace_id = $1 - "#, - w_id +/// The data tables the caller can reach here, shared ones included. +async fn list_datatable_names(db: &DB, authed: &ApiAuthed, w_id: &str) -> Result> { + Ok( + windmill_common::workspaces::usable_datatables(db, &authed.to_authed_ref(), w_id) + .await? + .into_keys() + .collect(), ) - .fetch_all(db) - .await? - .into_iter() - .filter_map(|s| s) - .collect()) } async fn get_datatable_schema( @@ -3890,8 +3885,20 @@ async fn edit_datatable_config( let config: serde_json::Value = serde_json::to_value(new_config.settings) .map_err(|err| Error::internal_err(err.to_string()))?; + // This form owns `datatables` and nothing else. `shared_datatables` names data + // tables of other workspaces, which this workspace does not administer, so it + // is carried across untouched — except where a name has just been defined + // here: the workspace has taken the name back, and a pointer nothing resolves + // is worse than no pointer. sqlx::query!( - "UPDATE workspace_settings SET datatable = $1 WHERE workspace_id = $2", + "UPDATE workspace_settings + SET datatable = $1::jsonb || jsonb_build_object('shared_datatables', COALESCE(( + SELECT jsonb_object_agg(key, value) + FROM jsonb_each(CASE WHEN jsonb_typeof(datatable->'shared_datatables') = 'object' + THEN datatable->'shared_datatables' ELSE '{}'::jsonb END) + WHERE NOT ($1::jsonb->'datatables' ? key) + ), '{}'::jsonb)) + WHERE workspace_id = $2", config, &w_id ) @@ -8188,12 +8195,14 @@ async fn create_workspace_fork( // opts in on its own. // // A data table that was NOT forked still points at the parent's database, where those - // roles do hold grants, and neither answer is safe: dropping the block would let a - // fork (which any member may create) reach the parent's data as root, while keeping it - // freezes who may run as what at the moment of the fork — the parent revoking a tenant - // would never reach the copy, and the fork would keep running as the role it named. So - // a permissioned data table is not shared into a fork at all; the fork can fork it, or - // go without it. + // roles do hold grants, so the fork must not carry a copy of the config: dropping the + // `permissions` block would let a fork (which any member may create) reach the parent's + // data as root, and keeping it would freeze who may run as what at the moment of the + // fork. It becomes a `shared_datatables` entry instead — a pointer, resolved in the + // parent, against the caller's identity there. + // + // What the parent itself only points at is carried through naming the same owner, not + // the parent: a chain that grows with every fork is a chain that outlives its middle. let forked_datatable_names: Vec = nw .forked_datatables .iter() @@ -8201,7 +8210,8 @@ async fn create_workspace_fork( .collect(); sqlx::query!( r#"UPDATE workspace_settings - SET datatable = jsonb_set(datatable, '{datatables}', ( + SET datatable = jsonb_set( + jsonb_set(datatable, '{datatables}', ( SELECT COALESCE(jsonb_object_agg( key, CASE WHEN key = ANY($2) THEN value - 'permissions' ELSE value END @@ -8209,10 +8219,25 @@ async fn create_workspace_fork( FROM jsonb_each(datatable->'datatables') WHERE key = ANY($2) OR COALESCE((value->'permissions'->>'enabled')::boolean, false) = false - )) + )), + '{shared_datatables}', ( + SELECT COALESCE(jsonb_object_agg(key, entry), '{}'::jsonb) FROM ( + SELECT key, jsonb_build_object('from', $3::text) AS entry + FROM jsonb_each(datatable->'datatables') + WHERE NOT (key = ANY($2)) + AND COALESCE((value->'permissions'->>'enabled')::boolean, false) = true + UNION ALL + SELECT key, value AS entry + FROM jsonb_each(CASE + WHEN jsonb_typeof(datatable->'shared_datatables') = 'object' + THEN datatable->'shared_datatables' ELSE '{}'::jsonb END) + WHERE NOT (key = ANY($2)) + ) shared + )) WHERE workspace_id = $1 AND jsonb_typeof(datatable->'datatables') = 'object'"#, &forked_id, &forked_datatable_names[..], + &parent_workspace_id, ) .execute(&mut *tx) .await?; diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index a63c6ba63d..92efcfab57 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -4821,9 +4821,15 @@ paths: type: string resource_type: type: string - enum: [postgres, instance] + enum: [postgres, instance, ""] resource_path: type: string + shared_from: + type: string + description: >- + The workspace this data table belongs to, when it is not + this one. Its database is not this workspace's to name, + so resource_type and resource_path are empty for it. /w/{workspace}/workspaces/list_datatable_schemas: get: diff --git a/backend/windmill-common/src/workspaces.rs b/backend/windmill-common/src/workspaces.rs index f45319ad26..6380771a6c 100644 --- a/backend/windmill-common/src/workspaces.rs +++ b/backend/windmill-common/src/workspaces.rs @@ -1034,6 +1034,26 @@ pub struct DataTable { pub permissions: Option, } +/// A data table this workspace uses but does not own. +/// +/// The entry names the workspace whose data table it is and holds nothing else — +/// no database, no roles, no tenants. A fork gets one of these for each +/// permissioned data table of its parent, so it reads the parent's live config +/// and the parent's live tenancy instead of a copy that would freeze at the +/// moment of the fork. Being outside `datatables` is load-bearing: everything +/// that plans role drops, or reads which logins a workspace claims, walks that +/// map, and none of what is here belongs to this workspace. +#[derive(Deserialize, Serialize, Debug, Clone)] +pub struct SharedDataTable { + /// The workspace whose data table this points at. + pub from: String, +} + +/// The most workspaces a chain of shared data tables may cross. A fork of a fork +/// is ordinary; two workspaces naming each other is not, and would recurse until +/// the stack ran out. +const MAX_SHARED_DATATABLE_HOPS: usize = 8; + /// The role every permissioned data table has: it is the connection the data /// table resolves to without permissions (`custom_instance_user`, or the /// postgres resource's own user), so it owns every object created so far and @@ -1617,6 +1637,9 @@ pub enum DatatableAccess<'a> { /// every role. Unchecked, Authed(crate::db::AuthedRef<'a>), + /// The caller, resolved again in the workspace that owns a shared data table. + /// Owned because it is built during the hop, not borrowed from the request. + AuthedOwned(crate::db::Authed), /// A job's owner. The identity is only fetched if the data table turns out /// to be permissioned, so unpermissioned resolutions cost no extra query. PermissionedAs { @@ -1756,6 +1779,9 @@ async fn resolve_datatable_role( DatatableAccess::Unchecked => true, DatatableAccess::NoIdentity => false, DatatableAccess::Authed(ref authed) => can_use_datatable_role(role_entry, authed), + DatatableAccess::AuthedOwned(ref authed) => { + can_use_datatable_role(role_entry, &authed.to_authed_ref()) + } DatatableAccess::PermissionedAs { permissioned_as, email } => { let authed = crate::auth::fetch_authed_from_permissioned_as(permissioned_as, email, w_id, db) @@ -1803,6 +1829,234 @@ async fn resolve_datatable_role( } } +/// Every data table this workspace can use, with the workspace that owns each. +/// +/// Its own come back paired with `w_id`; the ones it only points at come back +/// paired with the workspace they live in, following a chain of forks to its end. +/// Authorizes nothing — who may use which role is still asked of +/// [`can_use_datatable_role`], with the identity [`authed_for_datatable`] builds. +pub async fn effective_datatables( + db: &DB, + w_id: &str, +) -> Result> { + let mut out = std::collections::BTreeMap::new(); + let mut pending: Vec<(String, String)> = vec![]; + let mut w_id = w_id.to_string(); + let mut hops = 0; + loop { + let settings = sqlx::query_scalar!( + "SELECT datatable FROM workspace_settings WHERE workspace_id = $1", + &w_id, + ) + .fetch_optional(db) + .await? + .flatten(); + if let Some(own) = settings + .as_ref() + .and_then(|s| s.get("datatables")) + .and_then(|d| d.as_object()) + { + for (name, value) in own { + // A name already answered belongs to the workspace nearest the + // caller, which is the one whose pointer led here. + if !out.contains_key(name) { + if let Ok(dt) = serde_json::from_value::(value.clone()) { + out.insert(name.clone(), (w_id.clone(), dt)); + } + } + } + } + if hops < MAX_SHARED_DATATABLE_HOPS { + if let Some(shared) = settings + .as_ref() + .and_then(|s| s.get("shared_datatables")) + .and_then(|d| d.as_object()) + { + for (name, value) in shared { + if let Ok(entry) = serde_json::from_value::(value.clone()) { + pending.push((name.clone(), entry.from)); + } + } + } + } + // Names still unanswered decide where to look next; a workspace already + // walked cannot answer twice, so a cycle runs out of pending entries. + pending.retain(|(name, _)| !out.contains_key(name)); + let Some((_, next)) = pending.first().cloned() else { + return Ok(out); + }; + pending.retain(|(_, from)| *from != next); + w_id = next; + hops += 1; + } +} + +/// A data table this workspace can use, and who the caller is where it lives. +pub struct UsableDataTable { + /// The workspace that owns it — `w_id` itself unless it is shared. + pub owner_w_id: String, + pub datatable: DataTable, + /// The caller in `owner_w_id`, when that is another workspace. Every tenancy + /// question about this data table is asked of this, never of the identity the + /// caller has in the workspace they are browsing. + pub owner_authed: Option, +} + +impl UsableDataTable { + /// The identity to ask [`can_use_datatable_role`] about. + pub fn who<'a>(&'a self, here: &'a crate::db::AuthedRef<'a>) -> crate::db::AuthedRef<'a> { + match &self.owner_authed { + Some(authed) => authed.to_authed_ref(), + None => here.clone(), + } + } +} + +/// The data tables this caller can use in `w_id`: every one the workspace owns, +/// plus the ones it only points at and the caller is also a member of. A shared +/// data table the caller cannot reach is left out rather than listed and refused +/// — they are not a member of the workspace it belongs to, so it is not theirs to +/// know about. +pub async fn usable_datatables( + db: &DB, + authed: &crate::db::AuthedRef<'_>, + w_id: &str, +) -> Result> { + let mut out = std::collections::BTreeMap::new(); + for (name, (owner_w_id, datatable)) in effective_datatables(db, w_id).await? { + let owner_authed = if owner_w_id == w_id { + None + } else { + match authed_for_datatable(db, authed, &owner_w_id, &name).await { + Ok(authed) => Some(authed), + Err(_) => continue, + } + }; + out.insert( + name, + UsableDataTable { owner_w_id, datatable, owner_authed }, + ); + } + Ok(out) +} + +/// The caller, resolved in the workspace that owns a data table. +/// +/// Every tenancy question about a shared data table has to be asked there — see +/// [`access_in_owner_workspace`], which this shares its rules with. +pub async fn authed_for_datatable( + db: &DB, + authed: &crate::db::AuthedRef<'_>, + owner_w_id: &str, + name: &str, +) -> Result { + match access_in_owner_workspace( + db, + DatatableAccess::Authed(authed.clone()), + owner_w_id, + owner_w_id, + name, + ) + .await? + { + DatatableAccess::AuthedOwned(authed) => Ok(authed), + _ => Err(Error::internal_err( + "resolving a shared data table's caller".to_string(), + )), + } +} + +/// Who the caller is in the workspace that owns a shared data table. +/// +/// Sharing means the owner's restrictions apply unchanged, and +/// [`can_use_datatable_role`] starts at `is_admin` — so checking the *using* +/// workspace's identity would make forking a workspace the way to reach its data +/// table as admin. The caller is looked up again in the owner workspace by email: +/// a username is per-workspace and can drift, the identity behind it does not. +async fn access_in_owner_workspace<'b>( + db: &DB, + access: DatatableAccess<'_>, + using_w_id: &str, + owner_w_id: &str, + name: &str, +) -> Result> { + let refused = |detail: &str| { + Error::NotAuthorized(format!( + "Data table '{name}' is shared from workspace '{owner_w_id}': {detail}" + )) + }; + let (permissioned_as, email) = match access { + // An unchecked caller was authorized in its own workspace, which says + // nothing about the owner's. Everything that legitimately runs against + // the owner's database — migrations, ACL edits, permission saves, role + // drops — belongs to the workspace that owns it. + DatatableAccess::Unchecked => { + return Err(refused("it can only be administered from there")) + } + DatatableAccess::NoIdentity => { + return Err(refused("the caller has no identity to resolve there")) + } + DatatableAccess::Authed(ref authed) => { + (format!("u/{}", authed.username), authed.email.to_string()) + } + DatatableAccess::AuthedOwned(ref authed) => { + (format!("u/{}", authed.username), authed.email.clone()) + } + DatatableAccess::PermissionedAs { permissioned_as, email } => { + (permissioned_as.to_string(), email.to_string()) + } + DatatableAccess::Job(job_id) => { + let job = sqlx::query!( + "SELECT permissioned_as, permissioned_as_email FROM v2_job WHERE id = $1 AND workspace_id = $2", + job_id, + using_w_id, + ) + .fetch_optional(db) + .await? + .ok_or_else(|| Error::NotFound(format!("job {job_id} not found in {using_w_id}")))?; + (job.permissioned_as, job.permissioned_as_email) + } + }; + + // A folder or group exists per workspace, so `f/reports` here and `f/reports` + // there are two different things — and the fork's admin creates both. Only an + // identity that names a person carries across. + if !permissioned_as.starts_with("u/") { + return Err(refused( + "only a workspace member can use it, and this runs as a folder or a group", + )); + } + + let username = sqlx::query_scalar!( + "SELECT username FROM usr WHERE workspace_id = $1 AND email = $2 AND disabled = false", + owner_w_id, + email, + ) + .fetch_optional(db) + .await?; + let permissioned_as = match username { + Some(username) => format!("u/{username}"), + None => { + // A superadmin belongs to no workspace and reaches every one; anyone + // else is simply not there, and a `*` tenant would otherwise have let + // them in. + let is_super_admin = + sqlx::query_scalar!("SELECT super_admin FROM password WHERE email = $1", email) + .fetch_optional(db) + .await? + .unwrap_or(false); + if !is_super_admin { + return Err(refused("you are not a member of that workspace")); + } + email.clone() + } + }; + Ok(DatatableAccess::AuthedOwned( + crate::auth::fetch_authed_from_permissioned_as(&permissioned_as, &email, owner_w_id, db) + .await?, + )) +} + async fn get_datatable_resource_inner( db: &DB, w_id: &str, @@ -1811,30 +2065,78 @@ async fn get_datatable_resource_inner( role: Option<&str>, access: DatatableAccess<'_>, ) -> Result { - let datatables = sqlx::query_scalar!( - r#" - SELECT ws.datatable->'datatables' AS datatables + get_datatable_resource_hops(db, w_id.to_string(), name, replication, role, access, 0).await +} + +fn get_datatable_resource_hops<'a>( + db: &'a DB, + w_id: String, + name: &'a str, + replication: bool, + role: Option<&'a str>, + access: DatatableAccess<'a>, + hops: usize, +) -> std::pin::Pin> + Send + 'a>> { + Box::pin(async move { + let settings = sqlx::query_scalar!( + r#" + SELECT ws.datatable AS settings FROM workspace_settings ws WHERE ws.workspace_id = $1 "#, - &w_id, - ) - .fetch_one(db) - .await - .map_err(|err| Error::internal_err(format!("getting datatable {name}: {err}")))?; + &w_id, + ) + .fetch_one(db) + .await + .map_err(|err| Error::internal_err(format!("getting datatable {name}: {err}")))?; - let datatable = datatables - .as_ref() - .and_then(|d| d.get(name)) - .filter(|v| !v.is_null()) - .ok_or_else(|| datatable_not_found_error(name, datatables.as_ref()))?; - let datatable = serde_json::from_value::(datatable.clone())?; + let datatables = settings.as_ref().and_then(|s| s.get("datatables")).cloned(); + let own = datatables + .as_ref() + .and_then(|d| d.get(name)) + .filter(|v| !v.is_null()); - let internal = matches!(access, DatatableAccess::Unchecked); - let role_override = resolve_datatable_role(db, w_id, name, &datatable, role, access).await?; + // A data table the workspace does not own, only uses: resolve it where it + // lives, as whoever the caller is there. + if own.is_none() { + if let Some(shared) = settings + .as_ref() + .and_then(|s| s.get("shared_datatables")) + .and_then(|d| d.get(name)) + .filter(|v| !v.is_null()) + { + let shared = serde_json::from_value::(shared.clone())?; + if hops >= MAX_SHARED_DATATABLE_HOPS { + return Err(Error::internal_err(format!( + "Data table '{name}' is shared through more than {MAX_SHARED_DATATABLE_HOPS} workspaces; the chain does not end" + ))); + } + let access = + access_in_owner_workspace(db, access, &w_id, &shared.from, name).await?; + return get_datatable_resource_hops( + db, + shared.from, + name, + replication, + role, + access, + hops + 1, + ) + .await; + } + } - let mut db_resource = - if datatable.database.resource_type == DataTableCatalogResourceType::Instance { + let datatable = own.ok_or_else(|| datatable_not_found_error(name, datatables.as_ref()))?; + let datatable = serde_json::from_value::(datatable.clone())?; + let w_id = w_id.as_str(); + + let internal = matches!(access, DatatableAccess::Unchecked); + let role_override = + resolve_datatable_role(db, w_id, name, &datatable, role, access).await?; + + let mut db_resource = if datatable.database.resource_type + == DataTableCatalogResourceType::Instance + { let mut pg_creds = PgDatabase::parse_uri(&get_database_url().await?.as_str().await)?; pg_creds.dbname = datatable.database.resource_path.clone(); if replication { @@ -1861,29 +2163,30 @@ async fn get_datatable_resource_inner( })? }; - // Before the swap below, not merely outside it: the recorded identity is of the - // connection as the resource resolves it, and swapping a role's login into - // `user` would make every comparison fail. - if resolution_proves_database_identity(internal, &datatable) { - ensure_datatable_database_unchanged(name, &datatable, &db_resource)?; - } + // Before the swap below, not merely outside it: the recorded identity is of the + // connection as the resource resolves it, and swapping a role's login into + // `user` would make every comparison fail. + if resolution_proves_database_identity(internal, &datatable) { + ensure_datatable_database_unchanged(name, &datatable, &db_resource)?; + } - // The role logs in as itself rather than through `SET ROLE`, which a script - // could `RESET ROLE` its way back out of and regain admin's privileges. - if let Some((pg_rolename, pg_password)) = role_override { - let creds = db_resource.as_object_mut().ok_or_else(|| { - Error::internal_err(format!( - "Data table '{name}' does not resolve to a postgres resource" - )) - })?; - creds.insert("user".to_string(), serde_json::Value::String(pg_rolename)); - creds.insert( - "password".to_string(), - serde_json::Value::String(pg_password), - ); - } + // The role logs in as itself rather than through `SET ROLE`, which a script + // could `RESET ROLE` its way back out of and regain admin's privileges. + if let Some((pg_rolename, pg_password)) = role_override { + let creds = db_resource.as_object_mut().ok_or_else(|| { + Error::internal_err(format!( + "Data table '{name}' does not resolve to a postgres resource" + )) + })?; + creds.insert("user".to_string(), serde_json::Value::String(pg_rolename)); + creds.insert( + "password".to_string(), + serde_json::Value::String(pg_password), + ); + } - Ok(db_resource) + Ok(db_resource) + }) } #[derive(Deserialize, Serialize, Debug)] @@ -3289,7 +3592,8 @@ mod tests { // Repointed — however the resource got there, including through a `$var:` // no guard on the resource itself would see. assert!( - ensure_datatable_database_unchanged("main", &dt, &resolved("elsewhere", "one")).is_err() + ensure_datatable_database_unchanged("main", &dt, &resolved("elsewhere", "one")) + .is_err() ); // A config that never recorded one cannot claim to match: the roles it // names were created against a database nobody wrote down. diff --git a/frontend/src/lib/components/workspaceSettings/ForkDatatableSection.svelte b/frontend/src/lib/components/workspaceSettings/ForkDatatableSection.svelte index 9abe4855f4..7aab62184a 100644 --- a/frontend/src/lib/components/workspaceSettings/ForkDatatableSection.svelte +++ b/frontend/src/lib/components/workspaceSettings/ForkDatatableSection.svelte @@ -41,11 +41,9 @@ let effectiveSource = $derived(sourceWorkspace ?? $workspaceStore ?? undefined) - // Listed with whether each is permissioned, in one unit: a data table whose - // roles were created in the source's database is not shared with the fork — - // the fork would either run as the data table's own connection, which owns - // everything there, or as a tenant list frozen at fork time. It has to be - // cloned, or the fork goes without it. + // Listed with whether each is permissioned, in one unit: a permissioned data + // table cannot be cloned — the copy would carry none of its roles — so the + // fork points at the original and every restriction on it stays the source's. let allDatatables = resource( () => effectiveSource, async (ws) => { @@ -216,22 +214,18 @@ items={[ { value: 'keep_original', - // What the backend does is decided by the config, not by this - // label, so where the check did not answer the label says both - // outcomes rather than promising the one it cannot know. - label: - dt.permissioned === undefined - ? 'Keep original unless permissioned (check failed)' - : dt.permissioned - ? 'Not shared (permissions enabled)' - : 'Keep original' + label: dt.shared_from + ? `Keep sharing (from ${dt.shared_from})` + : dt.permissioned + ? 'Keep original (shared, same restrictions)' + : 'Keep original' }, // A clone cannot carry the data table's roles, and the fork's copy // is stripped of its permissions — so the copy would be readable in // full by every member of the fork. The backend refuses it; not // offering it is what keeps the two in step. Where the check could // not answer, the safe reading is "permissioned". - ...(dt.permissioned === false + ...(dt.permissioned === false && !dt.shared_from ? [ { value: 'schema_only', label: 'Clone schema only' }, ...(!isCloudHosted() && $userStore?.is_admin