diff --git a/backend/.sqlx/query-0d10e0fa5cf4033c7d93c9ed56be8209046007917f44da954eccf2188e5bff1f.json b/backend/.sqlx/query-0d10e0fa5cf4033c7d93c9ed56be8209046007917f44da954eccf2188e5bff1f.json new file mode 100644 index 0000000000..a99ffad770 --- /dev/null +++ b/backend/.sqlx/query-0d10e0fa5cf4033c7d93c9ed56be8209046007917f44da954eccf2188e5bff1f.json @@ -0,0 +1,29 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT permissioned_as, permissioned_as_email FROM v2_job\n WHERE id = $1 AND workspace_id = $2", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "permissioned_as", + "type_info": "Varchar" + }, + { + "ordinal": 1, + "name": "permissioned_as_email", + "type_info": "Varchar" + } + ], + "parameters": { + "Left": [ + "Uuid", + "Text" + ] + }, + "nullable": [ + false, + false + ] + }, + "hash": "0d10e0fa5cf4033c7d93c9ed56be8209046007917f44da954eccf2188e5bff1f" +} diff --git a/backend/.sqlx/query-297c7a40dfce729d44aa37bc7c65560517bd25e40c0752a00467829191e2eb98.json b/backend/.sqlx/query-297c7a40dfce729d44aa37bc7c65560517bd25e40c0752a00467829191e2eb98.json new file mode 100644 index 0000000000..cda61b894a --- /dev/null +++ b/backend/.sqlx/query-297c7a40dfce729d44aa37bc7c65560517bd25e40c0752a00467829191e2eb98.json @@ -0,0 +1,15 @@ +{ + "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' = $2\n THEN jsonb_set(dt.value, '{reference,workspace_id}', to_jsonb($1::text))\n ELSE dt.value END\n ))\n FROM jsonb_each(ws.datatable->'datatables') dt\n )\n WHERE jsonb_typeof(ws.datatable->'datatables') = 'object'\n AND ws.datatable::text LIKE '%\"reference\"%'", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Text" + ] + }, + "nullable": [] + }, + "hash": "297c7a40dfce729d44aa37bc7c65560517bd25e40c0752a00467829191e2eb98" +} diff --git a/backend/.sqlx/query-334dbcd48fb59c96c62c2705ab2d1ce716cd52417f487cc1a8dd376017b2db7d.json b/backend/.sqlx/query-334dbcd48fb59c96c62c2705ab2d1ce716cd52417f487cc1a8dd376017b2db7d.json new file mode 100644 index 0000000000..0c94309c58 --- /dev/null +++ b/backend/.sqlx/query-334dbcd48fb59c96c62c2705ab2d1ce716cd52417f487cc1a8dd376017b2db7d.json @@ -0,0 +1,22 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = $1)", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "exists", + "type_info": "Bool" + } + ], + "parameters": { + "Left": [ + "Name" + ] + }, + "nullable": [ + null + ] + }, + "hash": "334dbcd48fb59c96c62c2705ab2d1ce716cd52417f487cc1a8dd376017b2db7d" +} diff --git a/backend/.sqlx/query-3fd36fa26a61be923ca23316482ed1ce17184271668622c53e94de7224da38ca.json b/backend/.sqlx/query-3fd36fa26a61be923ca23316482ed1ce17184271668622c53e94de7224da38ca.json new file mode 100644 index 0000000000..764ddd9808 --- /dev/null +++ b/backend/.sqlx/query-3fd36fa26a61be923ca23316482ed1ce17184271668622c53e94de7224da38ca.json @@ -0,0 +1,14 @@ +{ + "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" +} diff --git a/backend/.sqlx/query-4692810d2be817bbb5de9b476d68d695941bd4fb5ccef393e4da522ed479d601.json b/backend/.sqlx/query-4692810d2be817bbb5de9b476d68d695941bd4fb5ccef393e4da522ed479d601.json new file mode 100644 index 0000000000..b0ad1eaf86 --- /dev/null +++ b/backend/.sqlx/query-4692810d2be817bbb5de9b476d68d695941bd4fb5ccef393e4da522ed479d601.json @@ -0,0 +1,16 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE postgres_trigger SET server_id = NULL, last_server_ping = NULL\n WHERE workspace_id = $1\n AND (postgres_resource_path = $2 OR postgres_resource_path LIKE $3)", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Text", + "Text" + ] + }, + "nullable": [] + }, + "hash": "4692810d2be817bbb5de9b476d68d695941bd4fb5ccef393e4da522ed479d601" +} 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-79799b5a2e499df6c28e286c42b9ad2db940c2455ab19cc95e5198baf96d5629.json b/backend/.sqlx/query-79799b5a2e499df6c28e286c42b9ad2db940c2455ab19cc95e5198baf96d5629.json new file mode 100644 index 0000000000..c36b6fb361 --- /dev/null +++ b/backend/.sqlx/query-79799b5a2e499df6c28e286c42b9ad2db940c2455ab19cc95e5198baf96d5629.json @@ -0,0 +1,29 @@ +{ + "db_name": "PostgreSQL", + "query": "\n SELECT ws.workspace_id AS \"workspace_id!\", dt.key AS \"datatable!\"\n FROM workspace_settings ws\n JOIN workspace w ON w.id = ws.workspace_id AND w.deleted = false\n CROSS JOIN LATERAL jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) dt\n WHERE ws.workspace_id <> $1\n AND dt.value->'database'->>'resource_type' = 'instance'\n AND dt.value->'database'->>'resource_path' = $2\n ORDER BY ws.workspace_id, dt.key\n ", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "workspace_id!", + "type_info": "Varchar" + }, + { + "ordinal": 1, + "name": "datatable!", + "type_info": "Text" + } + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + }, + "nullable": [ + false, + null + ] + }, + "hash": "79799b5a2e499df6c28e286c42b9ad2db940c2455ab19cc95e5198baf96d5629" +} diff --git a/backend/.sqlx/query-a57de2bb0442a5ee8a607cd63cfcf675de175796184f620cb4b09670c8b0b19f.json b/backend/.sqlx/query-a57de2bb0442a5ee8a607cd63cfcf675de175796184f620cb4b09670c8b0b19f.json new file mode 100644 index 0000000000..f3b2cb571e --- /dev/null +++ b/backend/.sqlx/query-a57de2bb0442a5ee8a607cd63cfcf675de175796184f620cb4b09670c8b0b19f.json @@ -0,0 +1,16 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE capture_config SET server_id = NULL, last_server_ping = NULL\n WHERE workspace_id = $1 AND trigger_kind = 'postgres'\n AND (trigger_config->>'postgres_resource_path' = $2\n OR trigger_config->>'postgres_resource_path' LIKE $3)", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Text", + "Text" + ] + }, + "nullable": [] + }, + "hash": "a57de2bb0442a5ee8a607cd63cfcf675de175796184f620cb4b09670c8b0b19f" +} diff --git a/backend/.sqlx/query-b9842d2d8abf382bd82d8fa1de012373638be391f884f81dc387ffc465badac6.json b/backend/.sqlx/query-b9842d2d8abf382bd82d8fa1de012373638be391f884f81dc387ffc465badac6.json new file mode 100644 index 0000000000..4b3a2f33e9 --- /dev/null +++ b/backend/.sqlx/query-b9842d2d8abf382bd82d8fa1de012373638be391f884f81dc387ffc465badac6.json @@ -0,0 +1,20 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT jsonb_object_keys(value->'databases') FROM global_settings\n WHERE name = 'custom_instance_pg_databases'", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "jsonb_object_keys", + "type_info": "Text" + } + ], + "parameters": { + "Left": [] + }, + "nullable": [ + null + ] + }, + "hash": "b9842d2d8abf382bd82d8fa1de012373638be391f884f81dc387ffc465badac6" +} diff --git a/backend/.sqlx/query-c1d026c886799dabc39ce73e1fe09ccb175c7271df75d67aa9c72ad6f825a992.json b/backend/.sqlx/query-c1d026c886799dabc39ce73e1fe09ccb175c7271df75d67aa9c72ad6f825a992.json new file mode 100644 index 0000000000..f5fde93bf7 --- /dev/null +++ b/backend/.sqlx/query-c1d026c886799dabc39ce73e1fe09ccb175c7271df75d67aa9c72ad6f825a992.json @@ -0,0 +1,28 @@ +{ + "db_name": "PostgreSQL", + "query": "DELETE FROM usr WHERE email = $1 RETURNING username, workspace_id", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "username", + "type_info": "Varchar" + }, + { + "ordinal": 1, + "name": "workspace_id", + "type_info": "Varchar" + } + ], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [ + false, + false + ] + }, + "hash": "c1d026c886799dabc39ce73e1fe09ccb175c7271df75d67aa9c72ad6f825a992" +} diff --git a/backend/.sqlx/query-d5fb5dde6300862f978739a3d9249fc2b3e7697c0da7d3195398933d3d81aadf.json b/backend/.sqlx/query-d5fb5dde6300862f978739a3d9249fc2b3e7697c0da7d3195398933d3d81aadf.json new file mode 100644 index 0000000000..6297c4d366 --- /dev/null +++ b/backend/.sqlx/query-d5fb5dde6300862f978739a3d9249fc2b3e7697c0da7d3195398933d3d81aadf.json @@ -0,0 +1,22 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT datatable FROM workspace_settings WHERE workspace_id = $1", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "datatable", + "type_info": "Jsonb" + } + ], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [ + true + ] + }, + "hash": "d5fb5dde6300862f978739a3d9249fc2b3e7697c0da7d3195398933d3d81aadf" +} diff --git a/backend/.sqlx/query-dc8dfc37559e9b6713bde48155f48b5a2c7b8199eace1508e102b60d1ff40c04.json b/backend/.sqlx/query-dc8dfc37559e9b6713bde48155f48b5a2c7b8199eace1508e102b60d1ff40c04.json new file mode 100644 index 0000000000..def9ee0edb --- /dev/null +++ b/backend/.sqlx/query-dc8dfc37559e9b6713bde48155f48b5a2c7b8199eace1508e102b60d1ff40c04.json @@ -0,0 +1,20 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT value->'roles' FROM global_settings WHERE name = 'custom_instance_pg_databases'", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "?column?", + "type_info": "Jsonb" + } + ], + "parameters": { + "Left": [] + }, + "nullable": [ + null + ] + }, + "hash": "dc8dfc37559e9b6713bde48155f48b5a2c7b8199eace1508e102b60d1ff40c04" +} diff --git a/backend/.sqlx/query-e2061df65ffd4a72146c4ca316829265289c8d6f625ac272655c88e1ad0b1745.json b/backend/.sqlx/query-e2061df65ffd4a72146c4ca316829265289c8d6f625ac272655c88e1ad0b1745.json new file mode 100644 index 0000000000..0e3c0182a9 --- /dev/null +++ b/backend/.sqlx/query-e2061df65ffd4a72146c4ca316829265289c8d6f625ac272655c88e1ad0b1745.json @@ -0,0 +1,22 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT datatable FROM workspace_settings WHERE workspace_id = $1 FOR UPDATE", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "datatable", + "type_info": "Jsonb" + } + ], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [ + true + ] + }, + "hash": "e2061df65ffd4a72146c4ca316829265289c8d6f625ac272655c88e1ad0b1745" +} diff --git a/backend/.sqlx/query-f3ee09fb17955ca8d886f446d397063c4094546a7807343b570b823796372cef.json b/backend/.sqlx/query-f3ee09fb17955ca8d886f446d397063c4094546a7807343b570b823796372cef.json new file mode 100644 index 0000000000..64183afaad --- /dev/null +++ b/backend/.sqlx/query-f3ee09fb17955ca8d886f446d397063c4094546a7807343b570b823796372cef.json @@ -0,0 +1,22 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT workspace_id FROM workspace_settings WHERE datatable::text LIKE $1", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "workspace_id", + "type_info": "Varchar" + } + ], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [ + false + ] + }, + "hash": "f3ee09fb17955ca8d886f446d397063c4094546a7807343b570b823796372cef" +} diff --git a/backend/.sqlx/query-f4adc9e26ebfebce18a29fb2c21bf06394cacb8a9699a608327b097e0ac1363e.json b/backend/.sqlx/query-f4adc9e26ebfebce18a29fb2c21bf06394cacb8a9699a608327b097e0ac1363e.json new file mode 100644 index 0000000000..b3dd09e2ba --- /dev/null +++ b/backend/.sqlx/query-f4adc9e26ebfebce18a29fb2c21bf06394cacb8a9699a608327b097e0ac1363e.json @@ -0,0 +1,16 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE workspace_settings\n SET datatable = CASE WHEN $3::jsonb = 'null'::jsonb\n THEN datatable #- ARRAY['datatables', $2, 'permissions']\n ELSE jsonb_set(datatable, ARRAY['datatables', $2, 'permissions'], $3::jsonb)\n END\n WHERE workspace_id = $1", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Text", + "Jsonb" + ] + }, + "nullable": [] + }, + "hash": "f4adc9e26ebfebce18a29fb2c21bf06394cacb8a9699a608327b097e0ac1363e" +} diff --git a/backend/Cargo.lock b/backend/Cargo.lock index fbab92f733..4a71095c05 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -15470,6 +15470,7 @@ dependencies = [ "windmill-ai", "windmill-alerting", "windmill-api-auth", + "windmill-audit", "windmill-common", "windmill-object-store", ] diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 317ca12c5e..8a75d8815e 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -d252afcc80e77fcc4f9a2a346b80908c8605a6c0 +475e60445f44506c754db3f17e307875d9075ce3 diff --git a/backend/parsers/windmill-parser/src/asset_parser.rs b/backend/parsers/windmill-parser/src/asset_parser.rs index 30c6047255..70fe5aa28a 100644 --- a/backend/parsers/windmill-parser/src/asset_parser.rs +++ b/backend/parsers/windmill-parser/src/asset_parser.rs @@ -730,7 +730,12 @@ pub fn parse_asset_syntax( s: &str, enable_default_syntax: bool, ) -> Option<(AssetKind, Cow<'_, str>)> { - if enable_default_syntax && s == "datatable" { + // `datatable` and `datatable?role=analyst` both name the default data table: the role picks + // which Postgres login the connection is made as, not which data table is read. + if enable_default_syntax + && s.strip_prefix("datatable") + .is_some_and(|rest| rest.is_empty() || rest.starts_with('?')) + { return Some((AssetKind::DataTable, Cow::Borrowed("main"))); } else if enable_default_syntax && s == "ducklake" { return Some((AssetKind::Ducklake, Cow::Borrowed("main"))); @@ -741,6 +746,14 @@ pub fn parse_asset_syntax( if *kind == AssetKind::Dbt { return Some((*kind, Cow::Owned(canonicalize_table_asset_path(suffix)))); } + // Same reasoning as above, for the explicit form. Specific to data tables: a + // `Resource`'s `?table=` is part of what it names, and stripping it would merge two + // different assets. + if *kind == AssetKind::DataTable { + if let Some((path, _role)) = suffix.split_once('?') { + return Some((*kind, Cow::Borrowed(path))); + } + } // The suffix is kept verbatim. For S3 the path encodes the storage: // `s3:///`, with an EMPTY storage segment for the // workspace default — so `s3:///key` yields `/key` (leading slash @@ -1692,6 +1705,25 @@ fn parse_trigger_spec(s: &str) -> Option { mod pipeline_annotation_tests { use super::*; + #[test] + fn a_datatable_role_is_not_part_of_the_asset_it_names() { + // The role picks which Postgres login the connection is made as, so two references that + // differ only by role are the same asset and must land on one graph node. + assert_eq!( + parse_asset_syntax("datatable://sales?role=analytics", false), + Some((AssetKind::DataTable, Cow::Borrowed("sales"))) + ); + assert_eq!( + parse_asset_syntax("datatable?role=analytics", true), + Some((AssetKind::DataTable, Cow::Borrowed("main"))) + ); + // A resource's `?table=` is part of what it names, so it is kept. + assert_eq!( + parse_asset_syntax("$res:f/db/pg?table=users", false), + Some((AssetKind::Resource, Cow::Borrowed("f/db/pg?table=users"))) + ); + } + #[test] fn s3_path_keeps_storage_distinction() { // An S3 asset path is `/` with an empty storage segment diff --git a/backend/windmill-api-groups/src/folders.rs b/backend/windmill-api-groups/src/folders.rs index 646f764ec6..c52b8a6316 100644 --- a/backend/windmill-api-groups/src/folders.rs +++ b/backend/windmill-api-groups/src/folders.rs @@ -800,6 +800,14 @@ async fn delete_folder( not_found_if_none(get_folderopt(&mut tx, &w_id, &name).await?, "Folder", &name)?; + // See the same call in `delete_group`: a freed name must not stay in a tenant list. + windmill_common::workspaces::remove_datatable_tenant_in_workspace( + &mut tx, + &w_id, + &format!("f/{name}"), + ) + .await?; + let del = sqlx::query_scalar!( "DELETE FROM folder WHERE name = $1 AND workspace_id = $2 RETURNING 1", name, diff --git a/backend/windmill-api-groups/src/groups.rs b/backend/windmill-api-groups/src/groups.rs index c931cf2255..0209af8585 100644 --- a/backend/windmill-api-groups/src/groups.rs +++ b/backend/windmill-api-groups/src/groups.rs @@ -797,6 +797,15 @@ async fn delete_group( } not_found_if_none(get_group_opt(&mut tx, &w_id, &name).await?, "Group", &name)?; + // A tenant list names a principal, so a freed name must not linger in one: a later group + // reusing it would silently inherit the data table access this one had. + windmill_common::workspaces::remove_datatable_tenant_in_workspace( + &mut tx, + &w_id, + &format!("g/{name}"), + ) + .await?; + sqlx::query!( "DELETE FROM usr_to_group WHERE group_ = $1 AND workspace_id = $2", name, diff --git a/backend/windmill-api-integration-tests/tests/datatable_roles.rs b/backend/windmill-api-integration-tests/tests/datatable_roles.rs new file mode 100644 index 0000000000..620de4065e --- /dev/null +++ b/backend/windmill-api-integration-tests/tests/datatable_roles.rs @@ -0,0 +1,182 @@ +//! Who may connect to a data table as which role, across the two shapes an entry can take: one +//! that owns its database, and a fork's pointer at it. + +use serde_json::{json, Value}; +use sqlx::{Pool, Postgres}; + +use windmill_test_utils::*; + +fn client() -> reqwest::Client { + reqwest::Client::new() +} + +fn authed(builder: reqwest::RequestBuilder, token: &str) -> reqwest::RequestBuilder { + builder.header("Authorization", format!("Bearer {token}")) +} + +/// The `analytics` role's tenant list as stored, so a cascade can be observed directly. +async fn tenants(db: &Pool, w_id: &str) -> Vec { + let value: Option = sqlx::query_scalar( + "SELECT datatable->'datatables'->'main'->'permissions'->'roles'->'role1'->'tenants' + FROM workspace_settings WHERE workspace_id = $1", + ) + .bind(w_id) + .fetch_one(db) + .await + .unwrap(); + serde_json::from_value(value.unwrap_or(json!([]))).unwrap() +} + +#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))] +async fn freeing_a_principal_takes_its_datatable_tenant(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"); + + assert_eq!( + tenants(&db, "test-workspace").await, + vec!["u/test-user-2", "g/analysts", "f/finance"] + ); + + let resp = authed( + client().delete(format!("{base}/groups/delete/analysts")), + "SECRET_TOKEN", + ) + .send() + .await?; + assert_eq!(resp.status(), 200, "delete group: {}", resp.text().await?); + + let resp = authed( + client().delete(format!("{base}/folders/delete/finance")), + "SECRET_TOKEN", + ) + .send() + .await?; + assert_eq!(resp.status(), 200, "delete folder: {}", resp.text().await?); + + let resp = authed( + client().delete(format!("{base}/users/delete/test-user-2")), + "SECRET_TOKEN", + ) + .send() + .await?; + assert_eq!(resp.status(), 200, "delete user: {}", resp.text().await?); + + // Nothing left naming a principal that no longer exists: a later group or account reusing one + // of those names must not inherit the access this one had. + assert!(tenants(&db, "test-workspace").await.is_empty()); + Ok(()) +} + +#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))] +async fn a_fork_uses_the_data_table_it_points_at_but_never_administers_it( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let fork = format!("http://localhost:{port}/api/w/wm-fork-dt/workspaces"); + + // `test-user-2` is an admin of the fork and a plain member of the parent. The roles they can + // use are the ones the parent's tenants give them there, not what their fork admin bit says. + let resp = authed( + client().get(format!("{fork}/datatable_usable_roles/main")), + "SECRET_TOKEN_2", + ) + .send() + .await?; + assert_eq!(resp.status(), 200); + let body: Value = resp.json().await?; + assert_eq!(body["roles"], json!(["analytics"]), "{body}"); + assert_eq!(body["default_role"], "analytics"); + + // The drawer names the workspace that decides, and refuses to let the fork edit it. + let resp = authed( + client().get(format!("{fork}/datatable_permissions/main")), + "SECRET_TOKEN_2", + ) + .send() + .await?; + let body: Value = resp.json().await?; + assert_eq!(body["governing_workspace_id"], "test-workspace"); + assert_eq!(body["editable"], false, "{body}"); + + let resp = authed( + client().post(format!("{fork}/datatable_permissions/main")), + "SECRET_TOKEN_2", + ) + .json(&json!({"permissioned": true, "default_role": "admin", + "roles": [{"id": "admin", "tenants": ["*"]}]})) + .send() + .await?; + assert_eq!( + resp.status(), + 401, + "a fork admin widened the parent's access" + ); + + // Nor by saving the settings form: the pointer is server-owned, so a payload naming the + // parent's database leaves the entry exactly as it was. + let resp = authed( + client().post(format!("{fork}/edit_datatable_config")), + "SECRET_TOKEN_2", + ) + .json(&json!({ + "settings": {"datatables": {"main": { + "database": {"resource_type": "instance", "resource_path": "dt_main"} + }}}, + "renames": [], "deleted_datatables": [] + })) + .send() + .await?; + assert_eq!(resp.status(), 200, "{}", resp.text().await?); + + let entry: Option = sqlx::query_scalar( + "SELECT datatable->'datatables'->'main' FROM workspace_settings WHERE workspace_id = $1", + ) + .bind("wm-fork-dt") + .fetch_one(&db) + .await?; + let entry = entry.unwrap(); + assert_eq!( + entry["reference"]["workspace_id"], "test-workspace", + "{entry}" + ); + assert!(entry["database"].is_null(), "{entry}"); + Ok(()) +} + +#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))] +async fn a_second_entry_on_the_same_database_is_reported_rather_than_governed( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + // A copy of the parent's entry, as a fork created before data table roles would hold. It keeps + // its own access, so the owner is told about it instead of being told it is covered. + sqlx::query( + r#"UPDATE workspace_settings SET datatable = '{"datatables": {"copy": { + "database": {"resource_type": "instance", "resource_path": "dt_main"}}}}'::jsonb + WHERE workspace_id = 'wm-fork-dt'"#, + ) + .execute(&db) + .await?; + + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let resp = authed( + client().get(format!( + "http://localhost:{port}/api/w/test-workspace/workspaces/datatable_permissions/main" + )), + "SECRET_TOKEN", + ) + .send() + .await?; + let body: Value = resp.json().await?; + assert_eq!( + body["ungoverned_reachers"], + json!([{"workspace_id": "wm-fork-dt", "datatable": "copy"}]), + "{body}" + ); + Ok(()) +} diff --git a/backend/windmill-api-integration-tests/tests/fixtures/datatable_roles.sql b/backend/windmill-api-integration-tests/tests/fixtures/datatable_roles.sql new file mode 100644 index 0000000000..0a3fb9372f --- /dev/null +++ b/backend/windmill-api-integration-tests/tests/fixtures/datatable_roles.sql @@ -0,0 +1,46 @@ +-- A data table under roles in `test-workspace`, and a fork whose entry points at it rather than +-- carrying a copy. `test-user-2` is a non-admin of the parent and an admin of the fork: the shape +-- the pointer exists for. + +UPDATE global_settings SET value = jsonb_set(value, '{roles}', + '{"role1": {"name": "analytics", "enabled": true, "pwd": "pw"}}'::jsonb) + WHERE name = 'custom_instance_pg_databases'; +INSERT INTO global_settings (name, value) + SELECT 'custom_instance_pg_databases', + '{"user_pwd": "pw", "databases": {"dt_main": {}}, + "roles": {"role1": {"name": "analytics", "enabled": true, "pwd": "pw"}}}'::jsonb + WHERE NOT EXISTS (SELECT 1 FROM global_settings WHERE name = 'custom_instance_pg_databases'); + +UPDATE workspace_settings SET datatable = '{ + "datatables": { + "main": { + "database": {"resource_type": "instance", "resource_path": "dt_main"}, + "permissions": { + "default_role": "role1", + "roles": { + "admin": {"tenants": []}, + "role1": {"tenants": ["u/test-user-2", "g/analysts", "f/finance"]} + } + } + } + } +}'::jsonb WHERE workspace_id = 'test-workspace'; + +INSERT INTO group_ (workspace_id, name, summary, extra_perms) VALUES + ('test-workspace', 'analysts', 'Analysts', '{}'); +INSERT INTO folder (workspace_id, name, display_name, owners, extra_perms) VALUES + ('test-workspace', 'finance', 'finance', '{}', '{}'); + +INSERT INTO workspace (id, name, owner, parent_workspace_id) VALUES + ('wm-fork-dt', 'fork of test-workspace', 'test2@windmill.dev', 'test-workspace'); +INSERT INTO workspace_key (workspace_id, kind, key) VALUES ('wm-fork-dt', 'cloud', 'test-key'); +INSERT INTO group_ (workspace_id, name, summary, extra_perms) VALUES + ('wm-fork-dt', 'all', 'All users', '{}'); +INSERT INTO usr (workspace_id, email, username, is_admin, role) VALUES + ('wm-fork-dt', 'test2@windmill.dev', 'test-user-2', true, 'Admin'); + +INSERT INTO workspace_settings (workspace_id, datatable) VALUES ('wm-fork-dt', '{ + "datatables": { + "main": {"reference": {"workspace_id": "test-workspace", "datatable": "main"}} + } +}'::jsonb); diff --git a/backend/windmill-api-settings/Cargo.toml b/backend/windmill-api-settings/Cargo.toml index bc5078bce4..0ec99ef6d0 100644 --- a/backend/windmill-api-settings/Cargo.toml +++ b/backend/windmill-api-settings/Cargo.toml @@ -11,7 +11,7 @@ path = "src/lib.rs" [features] default = [] enterprise = ["license"] -private = ["windmill-common/private"] +private = ["windmill-common/private", "windmill-audit/private"] parquet = ["windmill-common/parquet", "windmill-object-store/parquet"] license = ["dep:rsa"] @@ -19,6 +19,7 @@ license = ["dep:rsa"] windmill-ai = { workspace = true, default-features = false } windmill-alerting.workspace = true windmill-api-auth.workspace = true +windmill-audit.workspace = true windmill-common = { workspace = true, default-features = false } axum.workspace = true anyhow.workspace = true diff --git a/backend/windmill-api-settings/src/lib.rs b/backend/windmill-api-settings/src/lib.rs index 2e2b3290fa..a4a0ec11c3 100644 --- a/backend/windmill-api-settings/src/lib.rs +++ b/backend/windmill-api-settings/src/lib.rs @@ -151,6 +151,11 @@ pub fn global_service() -> Router { "/list_custom_instance_pg_databases", post(list_custom_instance_pg_databases), ) + .route("/datatable_roles", get(list_datatable_roles).post(create_datatable_role)) + .route( + "/datatable_roles/{id}", + post(update_datatable_role).delete(delete_datatable_role), + ) .route( "/refresh_custom_instance_user_pwd", post(refresh_custom_instance_user_pwd), @@ -2583,3 +2588,238 @@ mod object_storage_test_hardening { } } } + +// --------------------------------------------------------------------------- +// Data table roles +// --------------------------------------------------------------------------- + +use windmill_audit::audit_oss::audit_log; +use windmill_audit::ActionKind; + +/// One catalog entry as the settings UI sees it. The password never leaves the instance: it is a +/// Postgres credential Windmill mints and hands only to a resolved connection. +#[derive(Serialize)] +struct DatatableRoleInfo { + id: String, + name: String, + enabled: bool, +} + +#[derive(Deserialize)] +struct CreateDatatableRole { + name: String, +} + +#[derive(Deserialize)] +struct UpdateDatatableRole { + /// A rename. Absent leaves the name alone. + #[serde(default)] + name: Option, + /// `LOGIN` / `NOLOGIN`. Grants and ownership survive either way. + #[serde(default)] + enabled: Option, +} + +fn datatable_role_infos( + catalog: &windmill_common::datatable_roles::DatatableRoleCatalog, +) -> Vec { + catalog + .iter() + .map(|(id, role)| DatatableRoleInfo { + id: id.clone(), + name: role.name.clone(), + enabled: role.enabled, + }) + .collect() +} + +/// Persist the catalog next to the instance Postgres password, in the same `global_settings` row +/// the instance database registry lives in. +async fn write_role_catalog( + db: &DB, + catalog: &windmill_common::datatable_roles::DatatableRoleCatalog, +) -> error::Result<()> { + let value = serde_json::to_value(catalog).map_err(to_anyhow)?; + sqlx::query!( + "UPDATE global_settings SET value = jsonb_set(COALESCE(value, '{}'::jsonb), '{roles}', $1) + WHERE name = 'custom_instance_pg_databases'", + value + ) + .execute(db) + .await?; + Ok(()) +} + +async fn list_datatable_roles( + authed: ApiAuthed, + Extension(db): Extension, +) -> JsonResult> { + require_super_admin(&db, &authed).await?; + let catalog = windmill_common::datatable_roles::read_role_catalog(&db).await?; + Ok(Json(datatable_role_infos(&catalog))) +} + +/// Create the Postgres role first, then record it. The cluster is the source of truth: a catalog +/// entry naming a role that does not exist would resolve to a login nothing can authenticate as. +async fn create_datatable_role( + authed: ApiAuthed, + Extension(db): Extension, + Json(req): Json, +) -> JsonResult { + require_super_admin(&db, &authed).await?; + windmill_common::datatable_roles::validate_role_name(&req.name)?; + + let mut catalog = windmill_common::datatable_roles::read_role_catalog(&db).await?; + if catalog.values().any(|r| r.name == req.name) { + return Err(error::Error::BadRequest(format!( + "A data table role named '{}' already exists", + req.name + ))); + } + + let id = windmill_common::utils::rd_string(12); + let pwd = uuid::Uuid::new_v4().to_string(); + windmill_common::datatable_roles::create_instance_role(&db, &req.name, &pwd).await?; + + catalog.insert( + id.clone(), + windmill_common::datatable_roles::InstanceDatatableRole { + name: req.name.clone(), + enabled: true, + pwd: Some(pwd), + }, + ); + write_role_catalog(&db, &catalog).await?; + converge_connect_grants_everywhere(&db, &catalog).await; + windmill_common::feature_usage::log_feature_usage("datatable", "role_created", ""); + + audit_log( + &db, + &authed, + "settings.create_datatable_role", + ActionKind::Create, + "global", + Some(&authed.email), + Some([("name", req.name.as_str())].into()), + ) + .await?; + + Ok(Json(DatatableRoleInfo { id, name: req.name, enabled: true })) +} + +async fn update_datatable_role( + authed: ApiAuthed, + Extension(db): Extension, + Path(id): Path, + Json(req): Json, +) -> JsonResult { + require_super_admin(&db, &authed).await?; + let mut catalog = windmill_common::datatable_roles::read_role_catalog(&db).await?; + let role = catalog + .get(&id) + .cloned() + .ok_or_else(|| error::Error::NotFound(format!("No data table role with id '{id}'")))?; + + let mut updated = role.clone(); + if let Some(name) = req.name.filter(|n| n != &role.name) { + windmill_common::datatable_roles::validate_role_name(&name)?; + if catalog.values().any(|r| r.name == name) { + return Err(error::Error::BadRequest(format!( + "A data table role named '{name}' already exists" + ))); + } + // RENAME discards an md5-hashed password, so the role gets a fresh one in the same + // statement and the catalog records it. Tenants name the id, so nothing else moves. + let pwd = uuid::Uuid::new_v4().to_string(); + windmill_common::datatable_roles::rename_instance_role(&db, &role.name, &name, &pwd) + .await?; + updated.name = name; + updated.pwd = Some(pwd); + } + if let Some(enabled) = req.enabled.filter(|e| *e != role.enabled) { + windmill_common::datatable_roles::set_instance_role_login(&db, &updated.name, enabled) + .await?; + updated.enabled = enabled; + } + + catalog.insert(id.clone(), updated.clone()); + write_role_catalog(&db, &catalog).await?; + converge_connect_grants_everywhere(&db, &catalog).await; + + audit_log( + &db, + &authed, + "settings.update_datatable_role", + ActionKind::Update, + "global", + Some(&authed.email), + Some([("name", updated.name.as_str())].into()), + ) + .await?; + + Ok(Json(DatatableRoleInfo { + id, + name: updated.name, + enabled: updated.enabled, + })) +} + +/// Drop the Postgres role, then forget it, then strip it from every workspace that tenanted it. +/// +/// Dropping first is what makes the catalog trustworthy: the drop refuses while any instance +/// database is unreachable, so a failure leaves the entry in place to retry rather than a live +/// Postgres login nothing names. +async fn delete_datatable_role( + authed: ApiAuthed, + Extension(db): Extension, + Path(id): Path, +) -> JsonResult<()> { + require_super_admin(&db, &authed).await?; + let mut catalog = windmill_common::datatable_roles::read_role_catalog(&db).await?; + let role = catalog + .get(&id) + .cloned() + .ok_or_else(|| error::Error::NotFound(format!("No data table role with id '{id}'")))?; + + windmill_common::datatable_roles::drop_instance_role(&db, &role.name).await?; + catalog.remove(&id); + write_role_catalog(&db, &catalog).await?; + windmill_common::workspaces::forget_datatable_role_everywhere(&db, &id).await?; + + audit_log( + &db, + &authed, + "settings.delete_datatable_role", + ActionKind::Delete, + "global", + Some(&authed.email), + Some([("name", role.name.as_str())].into()), + ) + .await?; + + Ok(Json(())) +} + +/// Best-effort `CONNECT` convergence over the instance database registry. A database that is +/// unreachable right now is repaired the next time one of its data tables is administered, so a +/// role creation is not held hostage by an unrelated database being down. +async fn converge_connect_grants_everywhere( + db: &DB, + catalog: &windmill_common::datatable_roles::DatatableRoleCatalog, +) { + let dbnames = match windmill_common::datatable_roles::registered_instance_databases(db).await { + Ok(dbnames) => dbnames, + Err(e) => { + tracing::warn!("Could not list instance databases to grant CONNECT: {e}"); + return; + } + }; + for dbname in dbnames { + if let Err(e) = + windmill_common::datatable_roles::converge_connect_grants_with(db, &dbname, catalog) + .await + { + tracing::warn!("Could not converge CONNECT grants on instance database '{dbname}': {e}"); + } + } +} diff --git a/backend/windmill-api-users/src/users.rs b/backend/windmill-api-users/src/users.rs index b7d30eaa4b..2bfac244f5 100644 --- a/backend/windmill-api-users/src/users.rs +++ b/backend/windmill-api-users/src/users.rs @@ -1703,14 +1703,25 @@ async fn delete_user( .await?; windmill_common::user_drafts::delete_drafts_of_email(&mut *tx, &email_to_delete).await?; - let usernames = sqlx::query_scalar!( - "DELETE FROM usr WHERE email = $1 RETURNING username", + let memberships = sqlx::query!( + "DELETE FROM usr WHERE email = $1 RETURNING username, workspace_id", &email_to_delete ) .fetch_all(&mut *tx) .await?; - for username in usernames { + for row in memberships { + let username = row.username; + // A tenant list names a principal of its workspace, so the name has to be freed in every + // workspace this account belonged to: a later account taking the username would otherwise + // inherit the data table access it had. + windmill_common::workspaces::remove_datatable_tenant_in_workspace( + &mut tx, + &row.workspace_id, + &format!("u/{username}"), + ) + .await?; + sqlx::query!("DELETE FROM password WHERE email = $1", &email_to_delete) .execute(&mut *tx) .await?; @@ -2456,6 +2467,15 @@ pub async fn delete_workspace_user_internal( tx: &mut Transaction<'_, Postgres>, authed: Option<&ApiAuthed>, // None for system operations ) -> Result<()> { + // Same reasoning as the `extra_perms` sweep below: a freed username must not stay named + // anywhere that grants access, tenant lists included. + windmill_common::workspaces::remove_datatable_tenant_in_workspace( + tx, + w_id, + &format!("u/{username_to_delete}"), + ) + .await?; + // ---- Clean up extra_perms referencing this user ---- let extra_perms_tables = [ "script", @@ -3965,6 +3985,12 @@ async fn leave_workspace( ) -> Result { forbid_job_token_account_destruction(&authed)?; let mut tx = db.begin().await?; + windmill_common::workspaces::remove_datatable_tenant_in_workspace( + &mut tx, + &w_id, + &format!("u/{}", authed.username), + ) + .await?; sqlx::query!( "DELETE FROM usr WHERE workspace_id = $1 AND username = $2", &w_id, diff --git a/backend/windmill-api-workspaces/src/datatable_migrations.rs b/backend/windmill-api-workspaces/src/datatable_migrations.rs index ef5ea37e1d..24e6fa74ff 100644 --- a/backend/windmill-api-workspaces/src/datatable_migrations.rs +++ b/backend/windmill-api-workspaces/src/datatable_migrations.rs @@ -38,7 +38,12 @@ use windmill_common::runnable_settings::{ConcurrencySettingsWithCustom, Debounci use windmill_common::scripts::ScriptLang; use windmill_common::users::username_to_permissioned_as; use windmill_common::worker::to_raw_value; -use windmill_common::workspaces::get_datatable_resource_from_db_unchecked; +use windmill_common::datatable_roles::ADMIN_DATATABLE_ROLE; +use windmill_common::worker::SqlAnnotations; +use windmill_common::workspaces::{ + ensure_can_use_datatable_role, ensure_datatable_admin_access, + get_datatable_resource_from_db_unchecked, DatatableAccess, +}; use windmill_common::{PgDatabase, DB}; use windmill_git_sync::{ handle_deployment_metadata, handle_deployment_metadata_batch, DeployedObject, @@ -86,6 +91,42 @@ pub(crate) fn routes() -> Router { ) } +/// Refuse a migration whose role this caller may not use, before a job is pushed or a version +/// recorded. +/// +/// A migration that declares `-- role ` runs as that role, so the caller has to be one of its +/// tenants. One that declares none runs as `admin` and reaches every object in the database +/// whatever the roles grant, so it is for the admins of the workspace that governs the data table +/// — a fork can run a migration under a role it holds, never a migration under `admin`. +/// +/// The executor re-checks the role when it resolves the connection, so this is not the boundary. It +/// is what makes the refusal legible: which migration, and which role. +async fn ensure_migration_role_allowed( + db: &DB, + w_id: &str, + datatable_name: &str, + authed: &ApiAuthed, + sql: &str, + timestamp: i64, + name: &str, +) -> Result<()> { + let context = format!("Migration {timestamp} ({name})"); + let access = DatatableAccess::Authed(authed.to_authed_ref()); + match SqlAnnotations::datatable_role(sql) { + Some(role) => { + ensure_can_use_datatable_role(db, w_id, datatable_name, Some(&role), &access, &context) + .await + } + None => ensure_datatable_admin_access(db, w_id, datatable_name, &access) + .await + .map_err(|e| { + Error::NotAuthorized(format!( + "{context} declares no role, so it would run as admin. {e}" + )) + }), + } +} + #[derive(Serialize)] struct AppliedMigration { version: i64, @@ -128,7 +169,14 @@ async fn datatable_database_arg( .await? .ok_or_else(|| Error::internal_err(format!("datatable {datatable_name} not found")))?; - Ok(to_raw_value(&format!("datatable://{datatable_name}"))) + // `?role=admin` rather than a bare reference, so a migration that declares no `-- role` runs + // as the connection that owns the schema instead of falling through to the data table's + // default role — which is what `ensure_migration_role_allowed` gated it as, and which is the + // only role a DDL statement can be expected to succeed under. A migration that does declare a + // role overrides this: the annotation wins over the reference. + Ok(to_raw_value(&format!( + "datatable://{datatable_name}?role={ADMIN_DATATABLE_ROLE}" + ))) } /// Run a migration's SQL as a normal Windmill `postgresql` job, permissioned as @@ -440,6 +488,16 @@ async fn run_datatable_migrations( if applied_versions.contains(&m.timestamp) { continue; } + ensure_migration_role_allowed( + &db, + &w_id, + &datatable_name, + &authed, + &m.code_up, + m.timestamp, + &m.name, + ) + .await?; run_datatable_migration_job(&db, &user_db, &authed, &w_id, &database_arg, &m.code_up) .await .map_err(|e| { @@ -588,6 +646,17 @@ async fn rollback_datatable_migrations( )) })?; + ensure_migration_role_allowed( + &db, + &w_id, + &datatable_name, + &authed, + &code_down, + version, + &definition.name, + ) + .await?; + let database_arg = datatable_database_arg(&db, &w_id, &datatable_name).await?; run_datatable_migration_job(&db, &user_db, &authed, &w_id, &database_arg, &code_down) .await diff --git a/backend/windmill-api-workspaces/src/datatable_permissions.rs b/backend/windmill-api-workspaces/src/datatable_permissions.rs new file mode 100644 index 0000000000..519742ca36 --- /dev/null +++ b/backend/windmill-api-workspaces/src/datatable_permissions.rs @@ -0,0 +1,446 @@ +/* + * Author: Ruben Fiszel + * Copyright: Windmill Labs, Inc 2022 + * This file and its contents are licensed under the AGPLv3 License. + * Please see the included NOTICE for copyright information and + * LICENSE-AGPL for a copy of the license. + */ + +//! Who may connect to a data table as which role. +//! +//! The decision lives on the data table entry of the workspace that governs it, which is not +//! necessarily the workspace asking: a fork's entry points at its parent's, and everything here +//! resolves through that pointer first. Nothing in this module runs SQL against the data table — +//! a save is tenant lists and a default, and the Postgres roles themselves are the instance +//! catalog's business. + +use std::collections::BTreeMap; + +use axum::{ + extract::{Extension, Path}, + routing::get, + Json, Router, +}; +use serde::{Deserialize, Serialize}; + +use windmill_api_auth::{require_super_admin, ApiAuthed}; +use windmill_audit::audit_oss::audit_log; +use windmill_audit::ActionKind; +use windmill_common::datatable_roles::{read_role_catalog, ADMIN_DATATABLE_ROLE}; +use windmill_common::error::{Error, JsonResult, Result}; +use windmill_common::workspaces::{ + can_use_datatable_role_in_governing_workspace, resolve_governing_datatable, + DataTableCatalogResourceType, DataTablePermissions, DataTableRoleTenants, DatatableAccess, + GoverningDatatable, DATATABLE_TENANT_WILDCARD, +}; +use windmill_common::DB; + +pub(crate) fn routes() -> Router { + Router::new() + .route( + "/datatable_permissions/{datatable_name}", + get(get_datatable_permissions).post(set_datatable_permissions), + ) + .route( + "/datatable_usable_roles/{datatable_name}", + get(list_usable_datatable_roles), + ) +} + +/// One row of the permissions drawer: an instance role (or the reserved `admin`) and who may +/// connect as it here. +#[derive(Serialize, Deserialize, Debug)] +pub struct DatatableRoleTenantsInfo { + /// The instance catalog id, or `admin`. + pub id: String, + /// The role's current name, for display. Absent when the catalog no longer has the id — a + /// role deleted out from under this data table, which the drawer shows so it can be removed. + #[serde(skip_serializing_if = "Option::is_none")] + pub name: Option, + pub tenants: Vec, +} + +#[derive(Serialize)] +struct DatatablePermissionsInfo { + /// Whether the data table is under roles at all. + permissioned: bool, + default_role: String, + roles: Vec, + /// The workspace whose entry this is, when it is not the one asking. + #[serde(skip_serializing_if = "Option::is_none")] + governing_workspace_id: Option, + /// Whether this caller may save. False from a fork, and for a non-admin. + editable: bool, + /// Every instance role the instance defines, to pick from. + available_roles: Vec, + /// Other workspaces whose own entry reaches the same database without being governed by this + /// one — a legacy fork's copy, or a second entry a superadmin pointed here. They keep their own + /// access, so a save here does not reach them. + #[serde(skip_serializing_if = "Vec::is_empty")] + ungoverned_reachers: Vec, +} + +#[derive(Serialize)] +struct AvailableRole { + id: String, + name: String, + enabled: bool, +} + +#[derive(Serialize)] +struct UngovernedReacher { + workspace_id: String, + datatable: String, +} + +#[derive(Deserialize)] +pub struct SetDatatablePermissions { + /// False clears the block: the data table goes back to everyone connecting as `admin`. + pub permissioned: bool, + #[serde(default)] + pub default_role: Option, + #[serde(default)] + pub roles: Vec, +} + +#[derive(Serialize)] +struct UsableDatatableRoles { + permissioned: bool, + /// Names, not ids: this is what a caller writes in `-- role `. + roles: Vec, + default_role: String, +} + +/// Administering a data table — its permissions, its migrations that declare no role, its exports +/// — is for the admins of the workspace that governs it. A fork can use the data table; it never +/// administers it. +pub(crate) async fn ensure_governs_datatable( + db: &DB, + authed: &ApiAuthed, + w_id: &str, + governing: &GoverningDatatable, +) -> Result<()> { + if governing.workspace_id == w_id && authed.is_admin { + return Ok(()); + } + if require_super_admin(db, authed).await.is_ok() { + return Ok(()); + } + Err(Error::NotAuthorized(format!( + "Data table '{}' is governed by workspace '{}'; this is for its admins.", + governing.name, governing.workspace_id + ))) +} + +fn validate_tenant(tenant: &str) -> Result<()> { + if tenant == DATATABLE_TENANT_WILDCARD { + return Ok(()); + } + match tenant.split_once('/') { + Some(("u" | "g" | "f", rest)) if !rest.is_empty() => Ok(()), + _ => Err(Error::BadRequest(format!( + "Invalid tenant '{tenant}': expected 'u/', 'g/', 'f/' or '*'" + ))), + } +} + +/// Entries in other workspaces that reach the same instance database without pointing at this one. +/// +/// They exist by construction and are allowed: a fork created before data table roles holds a +/// literal copy of its parent's entry, and a superadmin can point a second workspace at any +/// instance database. Each governs its own access, so a save here leaves them untouched — which is +/// exactly why they are worth naming at the moment someone turns roles on. +async fn ungoverned_reachers( + db: &DB, + governing: &GoverningDatatable, +) -> Result> { + let Some(database) = governing.datatable.database.as_ref() else { + return Ok(vec![]); + }; + if database.resource_type != DataTableCatalogResourceType::Instance { + return Ok(vec![]); + } + let rows = sqlx::query!( + r#" + SELECT ws.workspace_id AS "workspace_id!", dt.key AS "datatable!" + FROM workspace_settings ws + JOIN workspace w ON w.id = ws.workspace_id AND w.deleted = false + CROSS JOIN LATERAL jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) dt + WHERE ws.workspace_id <> $1 + AND dt.value->'database'->>'resource_type' = 'instance' + AND dt.value->'database'->>'resource_path' = $2 + ORDER BY ws.workspace_id, dt.key + "#, + &governing.workspace_id, + &database.resource_path, + ) + .fetch_all(db) + .await?; + Ok(rows + .into_iter() + .map(|r| UngovernedReacher { workspace_id: r.workspace_id, datatable: r.datatable }) + .collect()) +} + +async fn get_datatable_permissions( + authed: ApiAuthed, + Extension(db): Extension, + Path((w_id, datatable_name)): Path<(String, String)>, +) -> JsonResult { + let governing = resolve_governing_datatable(&db, &w_id, &datatable_name).await?; + let catalog = read_role_catalog(&db).await?; + let editable = ensure_governs_datatable(&db, &authed, &w_id, &governing) + .await + .is_ok(); + + let permissions = governing.datatable.permissions.as_ref(); + let roles = permissions + .map(|p| { + p.roles + .iter() + .map(|(id, tenants)| DatatableRoleTenantsInfo { + id: id.clone(), + name: if id == ADMIN_DATATABLE_ROLE { + Some(ADMIN_DATATABLE_ROLE.to_string()) + } else { + catalog.get(id).map(|r| r.name.clone()) + }, + tenants: tenants.tenants.clone(), + }) + .collect() + }) + .unwrap_or_default(); + + Ok(Json(DatatablePermissionsInfo { + permissioned: permissions.is_some(), + default_role: permissions + .map(|p| p.default_role().to_string()) + .unwrap_or_else(|| ADMIN_DATATABLE_ROLE.to_string()), + roles, + governing_workspace_id: (governing.workspace_id != w_id) + .then(|| governing.workspace_id.clone()), + editable, + available_roles: catalog + .iter() + .map(|(id, role)| AvailableRole { + id: id.clone(), + name: role.name.clone(), + enabled: role.enabled, + }) + .collect(), + ungoverned_reachers: if editable { + ungoverned_reachers(&db, &governing).await? + } else { + vec![] + }, + })) +} + +async fn set_datatable_permissions( + authed: ApiAuthed, + Extension(db): Extension, + Path((w_id, datatable_name)): Path<(String, String)>, + Json(req): Json, +) -> Result { + let governing = resolve_governing_datatable(&db, &w_id, &datatable_name).await?; + ensure_governs_datatable(&db, &authed, &w_id, &governing).await?; + + let permissions = if req.permissioned { + let catalog = read_role_catalog(&db).await?; + let mut roles: BTreeMap = BTreeMap::new(); + for role in req.roles { + if role.id != ADMIN_DATATABLE_ROLE && !catalog.contains_key(&role.id) { + return Err(Error::BadRequest(format!( + "'{}' is not a data table role of this instance", + role.name.unwrap_or(role.id) + ))); + } + for tenant in &role.tenants { + validate_tenant(tenant)?; + } + roles.insert(role.id, DataTableRoleTenants { tenants: role.tenants }); + } + // `admin` is always a row: it is the connection every object in the database is owned by, + // and a save that dropped it would leave the data table with no way back in. + roles.entry(ADMIN_DATATABLE_ROLE.to_string()).or_default(); + + let default_role = req + .default_role + .unwrap_or_else(|| ADMIN_DATATABLE_ROLE.to_string()); + if !roles.contains_key(&default_role) { + return Err(Error::BadRequest(format!( + "The default role '{default_role}' is not among the data table's roles" + ))); + } + Some(DataTablePermissions { default_role: Some(default_role), roles }) + } else { + None + }; + + // An instance database provisioned before data table roles existed has neither the grant + // options the admin connection needs to delegate privileges, nor a CONNECT grant for any role + // — so a role would be refused at login however its tenants read. Repair it here, at the one + // moment someone is deciding this data table's roles. Best-effort: neither is worth failing a + // tenant edit over, and both converge again on the next save. + if permissions.is_some() { + if let Some(database) = governing.datatable.database.as_ref() { + if database.resource_type == DataTableCatalogResourceType::Instance { + let dbname = &database.resource_path; + if let Err(e) = + windmill_common::ensure_instance_db_grant_options_unchecked(&db, dbname).await + { + tracing::warn!("Could not refresh grant options on '{dbname}': {e}"); + } + if let Err(e) = + windmill_common::datatable_roles::converge_connect_grants(&db, dbname).await + { + tracing::warn!("Could not refresh CONNECT grants on '{dbname}': {e}"); + } + } + } + } + + let mut tx = db.begin().await?; + let value = match &permissions { + Some(p) => serde_json::to_value(p).map_err(|e| Error::internal_err(e.to_string()))?, + None => serde_json::Value::Null, + }; + // Written straight onto the governing workspace's entry rather than through the settings form, + // which deliberately carries this block across untouched. + sqlx::query!( + r#"UPDATE workspace_settings + SET datatable = CASE WHEN $3::jsonb = 'null'::jsonb + THEN datatable #- ARRAY['datatables', $2, 'permissions'] + ELSE jsonb_set(datatable, ARRAY['datatables', $2, 'permissions'], $3::jsonb) + END + WHERE workspace_id = $1"#, + &governing.workspace_id, + &governing.name, + value, + ) + .execute(&mut *tx) + .await?; + + audit_log( + &mut *tx, + &authed, + "workspaces.set_datatable_permissions", + ActionKind::Update, + &governing.workspace_id, + Some(&authed.email), + Some([("datatable", governing.name.as_str())].into()), + ) + .await?; + tx.commit().await?; + + // A live replication stream holds a connection it opened under the old decision. Bouncing the + // rows makes every listener reconnect and re-authorize. + restart_streams_reaching(&db, &governing).await?; + + windmill_common::feature_usage::log_feature_usage( + "datatable", + "roles_toggled", + if permissions.is_some() { "on" } else { "off" }, + ); + + Ok(if permissions.is_some() { + format!("Updated the roles of data table '{}'", governing.name) + } else { + format!("Data table '{}' is no longer under roles", governing.name) + }) +} + +/// Make every Postgres trigger and capture reading this data table reconnect, so a revoked tenant +/// stops streaming rather than living on inside an already-open replication connection. +pub(crate) async fn restart_streams_reaching( + db: &DB, + governing: &GoverningDatatable, +) -> Result<()> { + let reference = format!("datatable://{}", governing.name); + let prefix = format!("{reference}?%"); + + sqlx::query!( + "UPDATE postgres_trigger SET server_id = NULL, last_server_ping = NULL + WHERE workspace_id = $1 + AND (postgres_resource_path = $2 OR postgres_resource_path LIKE $3)", + &governing.workspace_id, + &reference, + &prefix, + ) + .execute(db) + .await?; + + // A capture keeps the reference inside its `trigger_config` blob rather than in a column of + // its own, and only a postgres capture has one there at all. + sqlx::query!( + "UPDATE capture_config SET server_id = NULL, last_server_ping = NULL + WHERE workspace_id = $1 AND trigger_kind = 'postgres' + AND (trigger_config->>'postgres_resource_path' = $2 + OR trigger_config->>'postgres_resource_path' LIKE $3)", + &governing.workspace_id, + &reference, + &prefix, + ) + .execute(db) + .await?; + + Ok(()) +} + +/// The roles this caller may connect as, by name, plus the one they get without asking. Drives the +/// role pickers; an empty list means the data table is not under roles. +async fn list_usable_datatable_roles( + authed: ApiAuthed, + Extension(db): Extension, + Path((w_id, datatable_name)): Path<(String, String)>, +) -> JsonResult { + let governing = resolve_governing_datatable(&db, &w_id, &datatable_name).await?; + let Some(permissions) = governing.datatable.permissions.as_ref() else { + return Ok(Json(UsableDatatableRoles { + permissioned: false, + roles: vec![], + default_role: ADMIN_DATATABLE_ROLE.to_string(), + })); + }; + let catalog = read_role_catalog(&db).await?; + let access = DatatableAccess::Authed(authed.to_authed_ref()); + + let mut roles = Vec::new(); + for (id, tenants) in &permissions.roles { + let name = if id == ADMIN_DATATABLE_ROLE { + ADMIN_DATATABLE_ROLE.to_string() + } else { + match catalog.get(id).filter(|r| r.enabled) { + Some(role) => role.name.clone(), + // Deleted or disabled instance-side: it cannot be connected as, so it is not + // offered, even to someone the tenants cover. + None => continue, + } + }; + if can_use_datatable_role_in_governing_workspace( + &db, + &governing.workspace_id, + &w_id, + tenants, + &access, + ) + .await? + { + roles.push(name); + } + } + + let default_role = permissions.default_role(); + Ok(Json(UsableDatatableRoles { + permissioned: true, + roles, + default_role: if default_role == ADMIN_DATATABLE_ROLE { + ADMIN_DATATABLE_ROLE.to_string() + } else { + catalog + .get(default_role) + .map(|r| r.name.clone()) + .unwrap_or_else(|| default_role.to_string()) + }, + })) +} diff --git a/backend/windmill-api-workspaces/src/lib.rs b/backend/windmill-api-workspaces/src/lib.rs index 22f2a2c5bb..ac3f492ff0 100644 --- a/backend/windmill-api-workspaces/src/lib.rs +++ b/backend/windmill-api-workspaces/src/lib.rs @@ -2,6 +2,7 @@ pub mod ai_session_backups; pub mod data_metrics; pub mod datatable_migrations; +pub mod datatable_permissions; pub mod deployment_requests; pub mod workspaces; pub mod workspaces_extra; diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index 2bef0d28ab..167a307e08 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -45,10 +45,12 @@ use windmill_common::workspaces::GitRepositorySettings; #[cfg(feature = "enterprise")] use windmill_common::workspaces::WorkspaceDeploymentUISettings; use windmill_common::workspaces::{ - check_deploy_rules, check_user_against_rule, get_datatable_resource_from_db_unchecked, + check_deploy_rules, check_user_against_rule, get_datatable_resource_from_db, + get_datatable_resource_from_db_unchecked, resolve_governing_datatable, validate_dev_workspace_id, validate_fork_workspace_id, validate_workspace_name, DataTable, - DataTableCatalogResourceType, DataTableForkBehavior, ProtectionRuleKind, ProtectionRules, - ProtectionRuleset, RuleCheckResult, WorkspaceGitSyncSettings, DEV_WORKSPACE_LOCK_RULE_NAME, + DataTableCatalogResourceType, DataTableForkBehavior, DatatableAccess, ProtectionRuleKind, + ProtectionRules, ProtectionRuleset, RuleCheckResult, WorkspaceGitSyncSettings, + DEV_WORKSPACE_LOCK_RULE_NAME, }; use windmill_common::workspaces::{Ducklake, DucklakeCatalogResourceType}; use windmill_common::PgDatabase; @@ -141,6 +143,7 @@ pub fn workspaced_service() -> Router { get(test_datatable_connection), ) .merge(crate::datatable_migrations::routes()) + .merge(crate::datatable_permissions::routes()) .route("/git_sync_enabled", get(get_git_sync_enabled)) .route("/git_sync_deploy_mode", get(get_git_sync_deploy_mode)) .route("/edit_git_sync_config", post(edit_git_sync_config)) @@ -1140,6 +1143,8 @@ async fn get_settings( if let Some(git_sync) = settings.git_sync.as_mut() { redact_git_sync_webhook_secrets(git_sync); } + settings.datatable = + windmill_common::workspaces::strip_datatable_permissions(settings.datatable.take()); Ok(Json(settings)) } @@ -1176,8 +1181,10 @@ async fn get_public_settings( .await .map_err(|e| Error::internal_err(format!("getting public settings: {e:#}")))?; - let settings = not_found_if_none(settings, "workspace settings", &w_id)?; + let mut settings = not_found_if_none(settings, "workspace settings", &w_id)?; tx.commit().await?; + settings.datatable = + windmill_common::workspaces::strip_datatable_permissions(settings.datatable.take()); Ok(Json(settings)) } @@ -2184,6 +2191,12 @@ struct DataTableListItem { name: String, resource_type: String, resource_path: String, + /// The workspace whose entry governs this one, when it is not this workspace — a fork pointing + /// at its parent. Its permissions apply here, and only its admins may edit them. + #[serde(skip_serializing_if = "Option::is_none")] + governing_workspace_id: Option, + /// Whether the governing entry is under roles. + permissioned: bool, } async fn list_datatables( @@ -2191,26 +2204,29 @@ async fn list_datatables( 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 names = list_datatable_names(&db, &w_id).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 { - name, - resource_type: dt.database.resource_type.as_ref().to_string(), - resource_path: dt.database.resource_path, - }) - .collect() - } - None => vec![], - }; + let mut items = Vec::with_capacity(names.len()); + for name in names { + // A pointer entry owns no database, so what it resolves to is the only truthful answer + // here; a chain that cannot be followed is reported with what the caller can still see. + let Ok(governing) = resolve_governing_datatable(&db, &w_id, &name).await else { + continue; + }; + let database = governing + .datatable + .database + .as_ref() + .expect("a governing entry owns a database"); + items.push(DataTableListItem { + name, + resource_type: database.resource_type.as_ref().to_string(), + resource_path: database.resource_path.clone(), + governing_workspace_id: (governing.workspace_id != w_id) + .then(|| governing.workspace_id.clone()), + permissioned: governing.datatable.permissions.is_some(), + }); + } Ok(Json(items)) } @@ -2385,7 +2401,7 @@ async fn test_datatable_connection( } async fn list_datatable_schemas( - _authed: ApiAuthed, + authed: ApiAuthed, Extension(db): Extension, Path(w_id): Path, ) -> JsonResult> { @@ -2393,7 +2409,7 @@ async fn list_datatable_schemas( let mut results = Vec::new(); for datatable_name in datatable_names { - let schema = match get_datatable_schema(&db, &w_id, &datatable_name).await { + let schema = match get_datatable_schema(&db, &authed, &w_id, &datatable_name).await { Ok(schemas) => DataTableSchema { datatable_name, schemas, error: None }, Err(e) => DataTableSchema { datatable_name, @@ -2408,7 +2424,7 @@ async fn list_datatable_schemas( } async fn list_datatable_tables( - _authed: ApiAuthed, + authed: ApiAuthed, Extension(db): Extension, Path(w_id): Path, ) -> JsonResult> { @@ -2416,7 +2432,7 @@ async fn list_datatable_tables( let mut results = Vec::new(); for datatable_name in datatable_names { - let tables = match get_datatable_tables(&db, &w_id, &datatable_name).await { + let tables = match get_datatable_tables(&db, &authed, &w_id, &datatable_name).await { Ok(schemas) => DataTableTables { datatable_name, schemas, error: None }, Err(e) => DataTableTables { datatable_name, @@ -2431,13 +2447,14 @@ async fn list_datatable_tables( } async fn get_datatable_table_schema( - _authed: ApiAuthed, + authed: ApiAuthed, Extension(db): Extension, Path(w_id): Path, Query(query): Query, ) -> JsonResult { let columns = get_datatable_table_columns( &db, + &authed, &w_id, &query.datatable_name, &query.schema_name, @@ -2469,13 +2486,38 @@ async fn list_datatable_names(db: &DB, w_id: &str) -> Result> { .collect()) } -async fn get_datatable_schema(db: &DB, w_id: &str, datatable_name: &str) -> Result { - // Get the datatable resource (connection credentials) - let db_resource = get_datatable_resource_from_db_unchecked(db, w_id, datatable_name).await?; +/// Connect to a data table as the caller, not as `admin`: the role they named, or the data table's +/// default. A data table not under roles resolves as `admin`, exactly as it did before roles. +/// +/// Every schema-browsing query below then reports what this Postgres role can actually reach, +/// which is why they filter on `has_schema_privilege` — `pg_catalog` is world-readable, so an +/// unfiltered listing would name schemas the connection cannot even enter. +async fn resolve_datatable_pg_as_caller( + db: &DB, + authed: &ApiAuthed, + w_id: &str, + datatable_name: &str, + role: Option<&str>, +) -> Result { + let db_resource = get_datatable_resource_from_db( + db, + w_id, + datatable_name, + role, + DatatableAccess::Authed(authed.to_authed_ref()), + ) + .await?; + serde_json::from_value(db_resource) + .map_err(|e| Error::internal_err(format!("Failed to parse database credentials: {}", e))) +} - // Parse the resource as PgDatabase - let pg_db: PgDatabase = serde_json::from_value(db_resource) - .map_err(|e| Error::internal_err(format!("Failed to parse database credentials: {}", e)))?; +async fn get_datatable_schema( + db: &DB, + authed: &ApiAuthed, + w_id: &str, + datatable_name: &str, +) -> Result { + let pg_db = resolve_datatable_pg_as_caller(db, authed, w_id, datatable_name, None).await?; // Connect to the datatable database let (client, connection) = pg_db.connect(Some(db)).await?; @@ -2495,6 +2537,7 @@ async fn get_datatable_schema(db: &DB, w_id: &str, datatable_name: &str) -> Resu FROM pg_namespace WHERE nspname NOT IN ('information_schema', 'pg_toast', 'pg_catalog') AND nspname NOT LIKE 'pg_%' + AND has_schema_privilege(oid, 'USAGE') ORDER BY nspname "#, &[], @@ -2562,10 +2605,13 @@ async fn get_datatable_schema(db: &DB, w_id: &str, datatable_name: &str) -> Resu Ok(schema_map) } -async fn get_datatable_tables(db: &DB, w_id: &str, datatable_name: &str) -> Result { - let db_resource = get_datatable_resource_from_db_unchecked(db, w_id, datatable_name).await?; - let pg_db: PgDatabase = serde_json::from_value(db_resource) - .map_err(|e| Error::internal_err(format!("Failed to parse database credentials: {}", e)))?; +async fn get_datatable_tables( + db: &DB, + authed: &ApiAuthed, + w_id: &str, + datatable_name: &str, +) -> Result { + let pg_db = resolve_datatable_pg_as_caller(db, authed, w_id, datatable_name, None).await?; let (client, connection) = pg_db.connect(Some(db)).await?; tokio::spawn(async move { @@ -2581,6 +2627,7 @@ async fn get_datatable_tables(db: &DB, w_id: &str, datatable_name: &str) -> Resu FROM pg_namespace WHERE nspname NOT IN ('information_schema', 'pg_toast', 'pg_catalog') AND nspname NOT LIKE 'pg_%' + AND has_schema_privilege(oid, 'USAGE') ORDER BY nspname "#, &[], @@ -2629,6 +2676,7 @@ async fn get_datatable_tables(db: &DB, w_id: &str, datatable_name: &str) -> Resu async fn get_datatable_table_columns( db: &DB, + authed: &ApiAuthed, w_id: &str, datatable_name: &str, schema_name: &str, @@ -2641,9 +2689,7 @@ async fn get_datatable_table_columns( ))); } - let db_resource = get_datatable_resource_from_db_unchecked(db, w_id, datatable_name).await?; - let pg_db: PgDatabase = serde_json::from_value(db_resource) - .map_err(|e| Error::internal_err(format!("Failed to parse database credentials: {}", e)))?; + let pg_db = resolve_datatable_pg_as_caller(db, authed, w_id, datatable_name, None).await?; let (client, connection) = pg_db.connect(Some(db)).await?; tokio::spawn(async move { @@ -2904,22 +2950,13 @@ pub(crate) async fn resolve_pg_source_checked( /// Whether the data table `name` is backed by the Windmill instance's own PostgreSQL /// rather than a user resource. pub(crate) async fn is_instance_datatable(db: &DB, w_id: &str, name: &str) -> Result { - let config = sqlx::query_scalar!( - "SELECT datatable->'datatables'->$2 FROM workspace_settings WHERE workspace_id = $1", - w_id, - name - ) - .fetch_optional(db) - .await? - .flatten(); - Ok(config - .and_then(|v| { - v.get("database") - .and_then(|d| d.get("resource_type")) - .and_then(|r| r.as_str()) - .map(|s| s == "instance") - }) - .unwrap_or(false)) + // Resolved rather than read: a pointer entry owns no database of its own, so only the entry it + // lands on can answer. A name that resolves to nothing keeps the historical `false`. + Ok(resolve_governing_datatable(db, w_id, name) + .await + .ok() + .and_then(|g| g.datatable.database) + .is_some_and(|d| d.resource_type == DataTableCatalogResourceType::Instance)) } /// Same, for the `datatable://` / `$res:` form the import endpoints take. @@ -3583,18 +3620,34 @@ async fn edit_datatable_config( .get(name.as_str()) .copied() .unwrap_or(name.as_str()); - dt.migrations_enabled = match old_datatables.get(lookup) { + let old = old_datatables.get(lookup); + dt.migrations_enabled = match old { Some(old) => old.migrations_enabled, None => { // Keyed by how the substrate is serialized into `workspace_settings`, // so these line up with the `datatable_configured` adoption counts. - created_substrates.push(match dt.database.resource_type { - DataTableCatalogResourceType::Instance => "instance", - DataTableCatalogResourceType::Postgresql => "postgresql", + created_substrates.push(match dt.database.as_ref().map(|d| d.resource_type) { + Some(DataTableCatalogResourceType::Instance) => "instance", + Some(DataTableCatalogResourceType::Postgresql) => "postgresql", + None => "reference", }); Some(true) } }; + // Three fields this form does not own, carried across from the stored entry rather than + // taken from the request. `permissions` is an access decision, edited through its own + // endpoint; `reference` is what makes a fork answer to the workspace that governs its data + // table, and letting a save clear it would hand the fork the database outright; and + // `forked_from` is the clone stamp the fork flow writes. Only fork creation writes any of + // them, so a settings save can neither widen nor lose them. + dt.permissions = old.and_then(|old| old.permissions.clone()); + dt.reference = old.and_then(|old| old.reference.clone()); + dt.forked_from = old.and_then(|old| old.forked_from.clone()); + // A pointer names no database of its own, so the form's empty `database` is correct there. + if dt.reference.is_some() { + dt.database = None; + } + windmill_common::workspaces::validate_datatable_shape(name, dt)?; } let args_for_audit = format!("{:?}", new_config.settings); @@ -3609,16 +3662,22 @@ async fn edit_datatable_config( ) .await?; - // Check that non-superadmins are not abusing Instance databases + // Check that non-superadmins are not abusing Instance databases, nor pointing an entry at + // another workspace's data table. Both reach a database this workspace does not own: an + // instance database directly, a reference through whoever governs it. if !is_superadmin { for (name, dt) in new_config.settings.datatables.iter() { - if dt.database.resource_type == DataTableCatalogResourceType::Instance { - let old_dt = old_datatables.get(name); - if old_dt.is_none() - || old_dt.unwrap().database.resource_type - != DataTableCatalogResourceType::Instance - || old_dt.unwrap().database.resource_path != dt.database.resource_path - { + let old_dt = old_datatables.get(name); + if dt + .database + .as_ref() + .is_some_and(|d| d.resource_type == DataTableCatalogResourceType::Instance) + { + let unchanged = old_dt.and_then(|o| o.database.as_ref()).is_some_and(|o| { + o.resource_type == DataTableCatalogResourceType::Instance + && Some(&o.resource_path) == dt.database.as_ref().map(|d| &d.resource_path) + }); + if !unchanged { return Err(Error::BadRequest( "Only superadmins can create or modify data tables with Instance databases" .to_string(), @@ -7545,6 +7604,89 @@ async fn snapshot_datatable_schema( .map_err(|e| Error::internal_err(format!("Failed to serialize schema: {}", e))) } +/// Turn every data table the fork chose to keep into a pointer at the parent's entry. +/// +/// `clone_workspace_data` copies `workspace_settings` wholesale, so a kept data table arrives as a +/// byte-identical copy naming the parent's database — including the parent's `permissions`, which +/// a fork admin could then edit to widen their own access to it. A pointer has nothing local to +/// edit: the parent's entry stays the only place the decision lives. +/// +/// The cloned data tables are skipped: they own a fresh database of their own, and they keep the +/// copied `permissions` as their starting point, which they then govern. +async fn point_kept_datatables_at_parent( + tx: &mut Transaction<'_, Postgres>, + parent_w_id: &str, + forked_w_id: &str, + cloned: &[ForkedDatatableInfo], +) -> Result<()> { + let settings: Option = sqlx::query_scalar!( + "SELECT datatable FROM workspace_settings WHERE workspace_id = $1", + forked_w_id + ) + .fetch_optional(&mut **tx) + .await? + .flatten(); + + let Some(mut settings) = settings else { + return Ok(()); + }; + let Some(datatables) = settings + .get_mut("datatables") + .and_then(|d| d.as_object_mut()) + else { + return Ok(()); + }; + + let mut changed = false; + for (name, entry) in datatables.iter_mut() { + if cloned.iter().any(|c| &c.name == name) { + continue; + } + let dt: DataTable = match serde_json::from_value(entry.clone()) { + Ok(dt) => dt, + Err(_) => continue, + }; + // Already a pointer: the parent was itself a fork, and its entry names the workspace that + // governs. Following it from here is the same answer, so leave it alone. + if dt.reference.is_some() { + continue; + } + // Only instance databases. A resource-backed data table names a resource, and the settings + // clone gave the fork its own copy of that resource in its own workspace — pointing at the + // parent's entry would silently move the fork onto the parent's resource instead. + if dt + .database + .as_ref() + .is_none_or(|d| d.resource_type != DataTableCatalogResourceType::Instance) + { + continue; + } + *entry = serde_json::to_value(DataTable { + database: None, + reference: Some(windmill_common::workspaces::DataTableReference { + workspace_id: parent_w_id.to_string(), + datatable: name.clone(), + }), + forked_from: None, + migrations_enabled: dt.migrations_enabled, + permissions: None, + }) + .map_err(|e| Error::internal_err(format!("serializing data table '{name}': {e}")))?; + changed = true; + } + + if changed { + sqlx::query!( + "UPDATE workspace_settings SET datatable = $1 WHERE workspace_id = $2", + settings, + forked_w_id + ) + .execute(&mut **tx) + .await?; + } + Ok(()) +} + async fn apply_forked_datatable( db: &DB, tx: &mut Transaction<'_, Postgres>, @@ -7583,7 +7725,18 @@ async fn apply_forked_datatable( let dt: DataTable = serde_json::from_value(config_val) .map_err(|e| Error::internal_err(format!("Failed to parse datatable config: {}", e)))?; - if dt.database.resource_type == DataTableCatalogResourceType::Instance { + // A cloned data table owns its copy, so the fork's entry must be terminal. It arrived that way + // from the settings clone; a pointer here would mean the parent's own entry was one, and the + // clone has to name its new database rather than follow anything. + let database = dt.database.as_ref().ok_or_else(|| { + Error::BadRequest(format!( + "Data table '{}' points at another workspace's data table and cannot be cloned; \ + fork it from the workspace that owns it.", + fdt.name + )) + })?; + + if database.resource_type == DataTableCatalogResourceType::Instance { // Instance: update resource_path to the new dbname sqlx::query!( r#"UPDATE workspace_settings @@ -7601,7 +7754,7 @@ async fn apply_forked_datatable( .await?; } else { // Resource: update the resource's dbname and mark as ws_specific - let resource_path = &dt.database.resource_path; + let resource_path = &database.resource_path; sqlx::query!( r#"UPDATE resource SET value = jsonb_set(value, '{dbname}', to_jsonb($3::text)) @@ -8078,6 +8231,14 @@ async fn create_workspace_fork( apply_forked_datatable(&db, &mut tx, &parent_workspace_id, &forked_id, fdt).await?; } + point_kept_datatables_at_parent( + &mut tx, + &parent_workspace_id, + &forked_id, + &nw.forked_datatables, + ) + .await?; + // The settings clone copies the source's ducklake config verbatim — including a parent // fork's own `fork_behavior` stamps. Sharing is a per-fork-creation choice, never // inherited: reset any cloned stamps first, then apply this fork's requested list. diff --git a/backend/windmill-api-workspaces/src/workspaces_extra.rs b/backend/windmill-api-workspaces/src/workspaces_extra.rs index e75f0d1ef2..4da6d24624 100644 --- a/backend/windmill-api-workspaces/src/workspaces_extra.rs +++ b/backend/windmill-api-workspaces/src/workspaces_extra.rs @@ -492,6 +492,30 @@ pub(crate) async fn change_workspace_id( .fetch_all(&mut *tx) .await?; + // A fork's data table entry names the workspace that governs it by id, so the rename has to + // follow there too — anywhere, not just in the reparented children: a detached workspace can + // point at this one without being its fork. Left behind, the pointer resolves to the archived + // shell and every job through it stops. + info!("Re-pointing data table references to the new workspace id"); + 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' = $2 + THEN jsonb_set(dt.value, '{reference,workspace_id}', to_jsonb($1::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"%'"#, + &rw.new_id, + &old_id, + ) + .execute(&mut *tx) + .await?; + info!("Updating workspace_protection_rule table"); sqlx::query!( "UPDATE workspace_protection_rule SET workspace_id = $1 WHERE workspace_id = $2", @@ -1343,15 +1367,20 @@ pub async fn drop_forked_datatable_databases( let mut errors: Vec = Vec::new(); for dt_name in &req.datatable_names { - let dt = match datatables.get(dt_name) { - Some(dt) if dt.forked_from.is_some() => dt, + // Only a clone is droppable, and a clone is terminal by construction: a kept data table is + // a pointer at the parent's database, which this fork does not own. + let database = match datatables.get(dt_name) { + Some(dt) if dt.forked_from.is_some() => match dt.database.as_ref() { + Some(database) => database, + None => continue, + }, _ => continue, }; - if dt.database.resource_type + if database.resource_type == windmill_common::workspaces::DataTableCatalogResourceType::Instance { - let db_to_drop = &dt.database.resource_path; + let db_to_drop = &database.resource_path; if !db_to_drop.starts_with("wm_fork_") { errors.push(format!( "Refusing to drop instance database '{}' for datatable://{}: name does not start with 'wm_fork_'", diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 2cb8a0e831..3c29dba2e1 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -1588,6 +1588,89 @@ paths: additionalProperties: $ref: "#/components/schemas/CustomInstanceDb" + /settings/datatable_roles: + get: + summary: list the instance's data table roles + operationId: listInstanceDatatableRoles + tags: + - setting + responses: + "200": + description: the instance role catalog + content: + application/json: + schema: + type: array + items: + $ref: "#/components/schemas/InstanceDatatableRole" + post: + summary: create a data table role on the instance's Postgres cluster + operationId: createInstanceDatatableRole + tags: + - setting + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [name] + properties: + name: + type: string + responses: + "200": + description: the created role + content: + application/json: + schema: + $ref: "#/components/schemas/InstanceDatatableRole" + + /settings/datatable_roles/{id}: + post: + summary: rename a data table role or turn its login on and off + operationId: updateInstanceDatatableRole + tags: + - setting + parameters: + - in: path + name: id + required: true + schema: + type: string + requestBody: + required: true + content: + application/json: + schema: + type: object + properties: + name: + type: string + enabled: + type: boolean + responses: + "200": + description: the updated role + content: + application/json: + schema: + $ref: "#/components/schemas/InstanceDatatableRole" + delete: + summary: drop a data table role from the cluster and from every workspace that named it + operationId: deleteInstanceDatatableRole + tags: + - setting + parameters: + - in: path + name: id + required: true + schema: + type: string + responses: + "200": + description: deleted + /settings/setup_custom_instance_pg_database/{name}: post: summary: Runs CREATE DATABASE on the Windmill Postgres and grants access to the custom_instance_user @@ -5141,7 +5224,7 @@ paths: type: array items: type: object - required: [name, resource_type, resource_path] + required: [name, resource_type, resource_path, permissioned] properties: name: type: string @@ -5150,6 +5233,97 @@ paths: enum: [postgres, instance] resource_path: type: string + governing_workspace_id: + type: string + permissioned: + type: boolean + + /w/{workspace}/workspaces/datatable_permissions/{datatable_name}: + get: + summary: get who may connect to a data table as which role + operationId: getDatatablePermissions + tags: + - workspace + parameters: + - $ref: "#/components/parameters/WorkspaceId" + - name: datatable_name + in: path + required: true + schema: + type: string + responses: + "200": + description: the data table's roles and their tenants + content: + application/json: + schema: + $ref: "#/components/schemas/DatatablePermissions" + post: + summary: set who may connect to a data table as which role + operationId: setDatatablePermissions + tags: + - workspace + parameters: + - $ref: "#/components/parameters/WorkspaceId" + - name: datatable_name + in: path + required: true + schema: + type: string + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [permissioned] + properties: + permissioned: + type: boolean + default_role: + type: string + roles: + type: array + items: + $ref: "#/components/schemas/DatatableRoleTenants" + responses: + "200": + description: status + content: + text/plain: + schema: + type: string + + /w/{workspace}/workspaces/datatable_usable_roles/{datatable_name}: + get: + summary: list the data table roles the caller may connect as + operationId: listUsableDatatableRoles + tags: + - workspace + parameters: + - $ref: "#/components/parameters/WorkspaceId" + - name: datatable_name + in: path + required: true + schema: + type: string + responses: + "200": + description: usable roles + content: + application/json: + schema: + type: object + required: [permissioned, roles, default_role] + properties: + permissioned: + type: boolean + roles: + type: array + items: + type: string + default_role: + type: string /w/{workspace}/workspaces/list_datatable_schemas: get: @@ -33321,6 +33495,61 @@ components: - ducklake - datatable + InstanceDatatableRole: + type: object + required: [id, name, enabled] + properties: + id: + type: string + name: + type: string + enabled: + type: boolean + + DatatableRoleTenants: + type: object + required: [id, tenants] + properties: + id: + type: string + name: + type: string + tenants: + type: array + items: + type: string + + DatatablePermissions: + type: object + required: [permissioned, default_role, roles, editable, available_roles] + properties: + permissioned: + type: boolean + default_role: + type: string + roles: + type: array + items: + $ref: "#/components/schemas/DatatableRoleTenants" + governing_workspace_id: + type: string + editable: + type: boolean + available_roles: + type: array + items: + $ref: "#/components/schemas/InstanceDatatableRole" + ungoverned_reachers: + type: array + items: + type: object + required: [workspace_id, datatable] + properties: + workspace_id: + type: string + datatable: + type: string + CustomInstanceDb: type: object required: @@ -35369,9 +35598,11 @@ components: type: object additionalProperties: type: object - required: [database] properties: database: + description: >- + Set on an entry that owns its database. Absent on a fork's entry, which points at + another workspace's data table instead. type: object properties: resource_type: @@ -35383,6 +35614,17 @@ components: type: string required: - resource_type + reference: + description: >- + The workspace and data table that govern this one. Server-owned: written by fork + creation, and carried across a settings save whatever the request says. + type: object + required: [workspace_id, datatable] + properties: + workspace_id: + type: string + datatable: + type: string migrations_enabled: type: boolean description: Whether the SQL migrations feature is opted in for this data table diff --git a/backend/windmill-api/src/users.rs b/backend/windmill-api/src/users.rs index 4dd073b3af..a7f9aeda89 100644 --- a/backend/windmill-api/src/users.rs +++ b/backend/windmill-api/src/users.rs @@ -352,6 +352,17 @@ async fn update_username_in_workpsace<'c>( new_username: &str, w_id: &str, ) -> error::Result<()> { + // ---- data table tenants ---- + // Tenants name the user, so the rename has to follow here too; a list left naming the old + // username silently drops the access instead of moving it. + windmill_common::workspaces::rename_datatable_tenant_in_workspace( + tx, + w_id, + &format!("u/{old_username}"), + &format!("u/{new_username}"), + ) + .await?; + // ---- instance and workspace users ---- sqlx::query!( "UPDATE usr SET username = $1 WHERE email = $2", diff --git a/backend/windmill-api/src/workspaces_export.rs b/backend/windmill-api/src/workspaces_export.rs index 69565ffcbd..4f6a6d9efb 100644 --- a/backend/windmill-api/src/workspaces_export.rs +++ b/backend/windmill-api/src/workspaces_export.rs @@ -1639,7 +1639,7 @@ pub(crate) async fn tarball_workspace( mute_critical_alerts: row.mute_critical_alerts, color: row.color.clone(), operator_settings: row.operator_settings.clone(), - datatable: row.datatable.clone(), + datatable: windmill_common::workspaces::strip_datatable_permissions(row.datatable.clone()), slack_team_id: row.slack_team_id.clone(), slack_name: row.slack_name.clone(), slack_command_script: row.slack_command_script.clone(), @@ -1703,7 +1703,7 @@ pub(crate) async fn tarball_workspace( mute_critical_alerts: row.mute_critical_alerts, color: row.color, operator_settings: row.operator_settings, - datatable: row.datatable, + datatable: windmill_common::workspaces::strip_datatable_permissions(row.datatable), slack_team_id: row.slack_team_id, slack_name: row.slack_name, slack_command_script: row.slack_command_script, diff --git a/backend/windmill-common/src/datatable_roles.rs b/backend/windmill-common/src/datatable_roles.rs new file mode 100644 index 0000000000..9efa05eca9 --- /dev/null +++ b/backend/windmill-common/src/datatable_roles.rs @@ -0,0 +1,324 @@ +/* + * Author: Ruben Fiszel + * Copyright: Windmill Labs, Inc 2022 + * This file and its contents are licensed under the AGPLv3 License. + * Please see the included NOTICE for copyright information and + * LICENSE-AGPL for a copy of the license. + */ + +//! The instance's data table role catalog. +//! +//! A data table role is a real Postgres login role on the Windmill cluster, named exactly as the +//! user named it, shared by every instance database. Windmill decides who may ask for a role (the +//! per-data-table tenant lists in [`crate::workspaces`]); Postgres decides what the role may then +//! touch. The catalog here is only the first half's vocabulary plus the cluster provisioning. +//! +//! Entries are keyed by a generated id so a rename moves nothing else: tenants name the id. + +use std::collections::BTreeMap; + +use serde::{Deserialize, Serialize}; + +use crate::{ + error::{Error, Result}, + DB, +}; + +/// The connection every data table resolved to before roles existed (`custom_instance_user`). It +/// owns every pre-existing object, so it is a reserved name rather than a catalog entry: never +/// created, renamed or dropped. +pub const ADMIN_DATATABLE_ROLE: &str = "admin"; + +/// The login the admin connection uses, and the role every created role is granted to — that +/// membership is what later lets it `ALTER ... OWNER TO` a role and drop it. +pub const CUSTOM_INSTANCE_USER: &str = "custom_instance_user"; + +/// One catalog entry. The password is per role and instance-wide; it lives here rather than in any +/// workspace's settings, next to the `custom_instance_user` password in the same +/// `custom_instance_pg_databases` row. +#[derive(Deserialize, Serialize, Clone, Debug)] +#[cfg_attr(feature = "instance_config_schema", derive(schemars::JsonSchema))] +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. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub pwd: Option, +} + +pub type DatatableRoleCatalog = BTreeMap; + +/// Names Postgres or Windmill already owns. `admin` is excluded because it never reaches the +/// cluster as a role name at all — it resolves to `custom_instance_user`. +fn is_reserved_role_name(name: &str) -> bool { + let lower = name.to_ascii_lowercase(); + lower == ADMIN_DATATABLE_ROLE + || lower == "postgres" + || lower == "public" + || lower.starts_with("pg_") + || lower.starts_with("windmill_") + || lower.starts_with("custom_instance_") +} + +/// The charset is what makes every downstream interpolation safe: the name reaches Postgres as a +/// quoted identifier, a `-- role ` annotation, and a `?role=` query parameter. +pub fn validate_role_name(name: &str) -> Result<()> { + if name.is_empty() || name.len() > 63 { + return Err(Error::BadRequest(format!( + "Invalid data table role name '{name}': it must be between 1 and 63 characters" + ))); + } + if !name + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-') + { + return Err(Error::BadRequest(format!( + "Invalid data table role name '{name}': only letters, digits, '_' and '-' are allowed" + ))); + } + if is_reserved_role_name(name) { + return Err(Error::BadRequest(format!( + "'{name}' is reserved and cannot be used as a data table role name" + ))); + } + Ok(()) +} + +/// SAFETY: every caller must have run [`validate_role_name`] first — the charset it enforces is +/// what makes this quoting sufficient. +fn quote_ident(name: &str) -> String { + format!("\"{}\"", name.replace('"', "\"\"")) +} + +fn quote_literal(value: &str) -> String { + format!("'{}'", value.replace('\'', "''")) +} + +pub async fn read_role_catalog(db: &DB) -> Result { + let value = sqlx::query_scalar!( + "SELECT value->'roles' FROM global_settings WHERE name = 'custom_instance_pg_databases'" + ) + .fetch_optional(db) + .await? + .flatten(); + match value { + Some(v) => Ok(serde_json::from_value(v).unwrap_or_default()), + None => Ok(Default::default()), + } +} + +/// Resolve the role a caller named to its catalog id. A disabled role is an error rather than a +/// silent fallback: the caller asked for something the instance deliberately turned off. +pub fn role_id_by_name<'a>(catalog: &'a DatatableRoleCatalog, name: &str) -> Result<&'a str> { + let entry = catalog + .iter() + .find(|(_, role)| role.name == name) + .ok_or_else(|| { + Error::NotFound(format!( + "'{name}' is not a data table role of this instance. Defined roles: {}.", + catalog + .values() + .map(|r| r.name.as_str()) + .collect::>() + .join(", ") + )) + })?; + if !entry.1.enabled { + return Err(Error::BadRequest(format!( + "Data table role '{name}' is disabled on this instance" + ))); + } + Ok(entry.0.as_str()) +} + +/// Every instance database the registry knows about. Role provisioning has to reach all of them: +/// a role that cannot `CONNECT` to a database is refused by Postgres before any grant matters. +pub async fn registered_instance_databases(db: &DB) -> Result> { + let names = sqlx::query_scalar!( + "SELECT jsonb_object_keys(value->'databases') FROM global_settings + WHERE name = 'custom_instance_pg_databases'" + ) + .fetch_all(db) + .await?; + Ok(names.into_iter().flatten().collect()) +} + +/// `CONNECT` on `dbname` for every enabled role, and none for `PUBLIC`. Run at role creation, at +/// database creation, and lazily whenever an instance data table is administered, so a database +/// provisioned before a role existed is repaired rather than left silently unreachable. +pub async fn converge_connect_grants(db: &DB, dbname: &str) -> Result<()> { + let catalog = read_role_catalog(db).await?; + converge_connect_grants_with(db, dbname, &catalog).await +} + +pub async fn converge_connect_grants_with( + db: &DB, + dbname: &str, + catalog: &DatatableRoleCatalog, +) -> Result<()> { + crate::validate_dbname(dbname)?; + let quoted_db = quote_ident(dbname); + let mut sql = format!("REVOKE CONNECT ON DATABASE {quoted_db} FROM PUBLIC;\n"); + for role in catalog.values().filter(|r| r.enabled) { + validate_role_name(&role.name)?; + sql.push_str(&format!( + "GRANT CONNECT ON DATABASE {quoted_db} TO {};\n", + quote_ident(&role.name) + )); + } + sqlx::raw_sql(&sql).execute(db).await?; + Ok(()) +} + +/// `CREATE ROLE LOGIN PASSWORD ...; GRANT TO custom_instance_user`, and `CONNECT` on +/// every registered database. No privileges beyond that — an admin grants them through SQL or the +/// ACL editor. +pub async fn create_instance_role(db: &DB, name: &str, password: &str) -> Result<()> { + validate_role_name(name)?; + let exists = sqlx::query_scalar!( + "SELECT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = $1)", + name + ) + .fetch_one(db) + .await? + .unwrap_or(false); + if exists { + return Err(Error::BadRequest(format!( + "A Postgres role named '{name}' already exists on this cluster" + ))); + } + let quoted = quote_ident(name); + sqlx::raw_sql(&format!( + "CREATE ROLE {quoted} LOGIN PASSWORD {};\nGRANT {quoted} TO {};", + quote_literal(password), + quote_ident(CUSTOM_INSTANCE_USER), + )) + .execute(db) + .await?; + Ok(()) +} + +pub async fn set_instance_role_login(db: &DB, name: &str, enabled: bool) -> Result<()> { + validate_role_name(name)?; + sqlx::query(&format!( + "ALTER ROLE {} {}", + quote_ident(name), + if enabled { "LOGIN" } else { "NOLOGIN" } + )) + .execute(db) + .await?; + Ok(()) +} + +/// A rename discards an md5-hashed password, so the caller has to hand over a fresh one. +pub async fn rename_instance_role(db: &DB, from: &str, to: &str, password: &str) -> Result<()> { + validate_role_name(from)?; + validate_role_name(to)?; + let taken = sqlx::query_scalar!( + "SELECT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = $1)", + to + ) + .fetch_one(db) + .await? + .unwrap_or(false); + if taken { + return Err(Error::BadRequest(format!( + "A Postgres role named '{to}' already exists on this cluster" + ))); + } + sqlx::raw_sql(&format!( + "ALTER ROLE {} RENAME TO {};\nALTER ROLE {} PASSWORD {};", + quote_ident(from), + quote_ident(to), + quote_ident(to), + quote_literal(password), + )) + .execute(db) + .await?; + Ok(()) +} + +/// A role owning anything in any database blocks its own `DROP ROLE`, and both its objects and the +/// privileges granted to it are only visible from inside each database — hence the pass over the +/// registry. An unreachable database aborts the whole delete: dropping the role while one database +/// still holds objects owned by it leaves those objects owned by a numeric OID nobody can name. +/// +/// Each pass runs as the instance's own Postgres user rather than `custom_instance_user`, which +/// owns the databases and can therefore revoke a grant whoever made it. `custom_instance_user` +/// could only undo what it granted itself, so a privilege planted by an operator in psql — the +/// ordinary way privileges reach a role — would survive and block the drop. +pub async fn drop_instance_role(db: &DB, name: &str) -> Result<()> { + validate_role_name(name)?; + let quoted = quote_ident(name); + let reassign = format!( + "REASSIGN OWNED BY {quoted} TO {};\nDROP OWNED BY {quoted};", + quote_ident(CUSTOM_INSTANCE_USER) + ); + + let base = crate::PgDatabase::parse_uri(&crate::get_database_url().await?.as_str().await)?; + for dbname in registered_instance_databases(db).await? { + let creds = crate::PgDatabase { dbname: dbname.clone(), ..base.clone() }; + let (client, connection) = creds.connect(Some(db)).await.map_err(|e| { + Error::BadRequest(format!( + "Cannot delete role '{name}': instance database '{dbname}' is unreachable ({e}). \ + Objects it owns there would be orphaned." + )) + })?; + let join_handle = tokio::spawn(async move { connection.await }); + let result = client.batch_execute(&reassign).await; + drop(client); + crate::shutdown_pg_connection(join_handle).await?; + result.map_err(|e| { + Error::internal_err(format!( + "Reassigning what role '{name}' owns in '{dbname}': {}", + crate::error::pg_error_message(&e) + )) + })?; + } + + sqlx::raw_sql(&format!("{reassign}\nDROP ROLE {quoted};")) + .execute(db) + .await?; + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn role_names_are_validated() { + assert!(validate_role_name("analytics").is_ok()); + assert!(validate_role_name("read-only_2").is_ok()); + assert!(validate_role_name("").is_err()); + assert!(validate_role_name(&"a".repeat(64)).is_err()); + assert!(validate_role_name("has space").is_err()); + assert!(validate_role_name("quote\"injection").is_err()); + // Reserved, case-insensitively. + assert!(validate_role_name("admin").is_err()); + assert!(validate_role_name("Postgres").is_err()); + assert!(validate_role_name("pg_read_all_data").is_err()); + assert!(validate_role_name("windmill_user").is_err()); + assert!(validate_role_name("custom_instance_user").is_err()); + } + + #[test] + fn a_disabled_role_is_an_error_not_a_fallback() { + let mut catalog = DatatableRoleCatalog::new(); + catalog.insert( + "id1".to_string(), + InstanceDatatableRole { + name: "analytics".to_string(), + enabled: false, + pwd: Some("x".to_string()), + }, + ); + assert!(role_id_by_name(&catalog, "analytics").is_err()); + assert!(role_id_by_name(&catalog, "nope").is_err()); + catalog.get_mut("id1").unwrap().enabled = true; + assert_eq!(role_id_by_name(&catalog, "analytics").unwrap(), "id1"); + } +} diff --git a/backend/windmill-common/src/instance_config.rs b/backend/windmill-common/src/instance_config.rs index de3fd7684a..1f897f115f 100644 --- a/backend/windmill-common/src/instance_config.rs +++ b/backend/windmill-common/src/instance_config.rs @@ -440,11 +440,13 @@ impl GlobalSettings { serde_json::Value::Object(map) => map.into_iter().collect(), _ => unreachable!(), }; - // Strip runtime-only `databases` sub-field from custom_instance_pg_databases. - // It contains setup status/logs managed by the setup endpoint, not configuration. + // Strip the runtime-only sub-fields of custom_instance_pg_databases: `databases` is setup + // status/logs managed by the setup endpoint, and `roles` is the data table role catalog, + // which carries one Postgres password per role. Neither is configuration. if let Some(pg) = map.get_mut("custom_instance_pg_databases") { if let Some(obj) = pg.as_object_mut() { obj.remove("databases"); + obj.remove("roles"); } } map @@ -795,6 +797,10 @@ pub struct CustomInstancePgDatabases { pub user_pwd: Option, #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] pub databases: BTreeMap, + /// The instance's data table role catalog, keyed by generated id. Runtime state carrying one + /// password per role, so it is stripped from config sync exactly like `databases`. + #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + pub roles: BTreeMap, } /// Status of a single custom instance database. @@ -1205,14 +1211,17 @@ pub fn diff_global_settings( } else { desired_value.clone() }; - // Preserve the runtime-only `databases` sub-field inside - // `custom_instance_pg_databases` so that config sync never wipes - // setup status/logs that are managed by the setup endpoint. + // Preserve the runtime-only sub-fields inside `custom_instance_pg_databases` so that + // config sync never wipes setup status/logs managed by the setup endpoint, nor the data + // table role catalog — the latter mirrors real Postgres roles, so losing it would leave + // the cluster holding logins Windmill can no longer name. if key == "custom_instance_pg_databases" { if let Some(existing) = current.get(key) { - if let Some(databases) = existing.get("databases") { - if let Some(obj) = value.as_object_mut() { - obj.entry("databases").or_insert_with(|| databases.clone()); + for runtime_field in ["databases", "roles"] { + if let Some(kept) = existing.get(runtime_field) { + if let Some(obj) = value.as_object_mut() { + obj.entry(runtime_field).or_insert_with(|| kept.clone()); + } } } } diff --git a/backend/windmill-common/src/lib.rs b/backend/windmill-common/src/lib.rs index 8e17ec676e..774da9ecdc 100644 --- a/backend/windmill-common/src/lib.rs +++ b/backend/windmill-common/src/lib.rs @@ -37,6 +37,7 @@ pub mod bench; pub mod cache; pub mod client; pub mod data_metrics; +pub mod datatable_roles; pub mod db; #[cfg(all(feature = "enterprise", feature = "private"))] mod db_entra_ee; @@ -1514,6 +1515,52 @@ pub async fn drop_custom_instance_database(db: &DB, dbname: &str) -> error::Resu Ok(()) } +/// What `custom_instance_user` holds on an instance database. +/// +/// `WITH GRANT OPTION` throughout: this is the connection every data table resolves to as `admin`, +/// and it is the one that hands privileges to data table roles. Postgres refuses to let a role pass +/// on a privilege it does not itself hold with grant option, so without these an admin could own +/// the database and still be unable to grant `SELECT` on it to `analytics`. +fn instance_db_grants(dbname: &str) -> String { + format!( + "GRANT CONNECT ON DATABASE \"{dbname}\" TO custom_instance_user WITH GRANT OPTION; + GRANT CREATE ON DATABASE \"{dbname}\" TO custom_instance_user WITH GRANT OPTION; + DO $$ BEGIN + IF EXISTS (SELECT 1 FROM pg_namespace WHERE nspname = 'public') THEN + GRANT USAGE ON SCHEMA public TO custom_instance_user WITH GRANT OPTION; + GRANT CREATE ON SCHEMA public TO custom_instance_user WITH GRANT OPTION; + ALTER DEFAULT PRIVILEGES IN SCHEMA public + GRANT SELECT, INSERT, UPDATE, DELETE ON TABLES TO custom_instance_user; + END IF; + END $$;" + ) +} + +/// Re-apply [`instance_db_grants`] to an instance database provisioned before data table roles +/// existed, whose grants carry no grant option. Connects as the instance's own Postgres user — +/// the database and `public` schema owner — since only it can hand out an option it holds. +/// +/// Authorization: reaches an instance database with the server's own credentials and checks +/// nothing. Callers MUST restrict this to superadmin or internal server paths. +pub async fn ensure_instance_db_grant_options_unchecked(db: &DB, dbname: &str) -> error::Result<()> { + let dbname = dbname.trim(); + validate_dbname(dbname)?; + let wmill_pg_creds = PgDatabase::parse_uri(&get_database_url().await?.as_str().await)?; + let creds = PgDatabase { dbname: dbname.to_string(), ..wmill_pg_creds }; + let (client, connection) = creds.connect(Some(db)).await?; + let join_handle = tokio::spawn(async move { connection.await }); + let result = client.batch_execute(&instance_db_grants(dbname)).await; + drop(client); + shutdown_pg_connection(join_handle).await?; + result.map_err(|e| { + error::Error::internal_err(format!( + "Failed to grant permissions on '{}': {}", + dbname, + crate::error::pg_error_message(&e) + )) + }) +} + /// Create a custom instance database: CREATE DATABASE, grant permissions, register in global_settings. /// The `tag` is stored in global_settings metadata (e.g. "datatable" or "ducklake"). pub async fn create_custom_instance_database( @@ -1553,17 +1600,7 @@ pub async fn create_custom_instance_database( let (client, connection) = new_pg_creds.connect(Some(db)).await?; let join_handle = tokio::spawn(async move { connection.await }); - if let Err(e) = client - .batch_execute(&format!( - "GRANT CONNECT ON DATABASE \"{dbname}\" TO custom_instance_user; - GRANT USAGE ON SCHEMA public TO custom_instance_user; - GRANT CREATE ON SCHEMA public TO custom_instance_user; - GRANT CREATE ON DATABASE \"{dbname}\" TO custom_instance_user; - ALTER DEFAULT PRIVILEGES IN SCHEMA public - GRANT SELECT, INSERT, UPDATE, DELETE ON TABLES TO custom_instance_user;" - )) - .await - { + if let Err(e) = client.batch_execute(&instance_db_grants(dbname)).await { tracing::warn!( "Failed to grant permissions on '{}': {}. Continuing.", dbname, @@ -1592,6 +1629,13 @@ pub async fn create_custom_instance_database( .execute(db) .await?; + // A data table role can only reach a database it may CONNECT to, and PUBLIC's default CONNECT + // would otherwise let every role in regardless of what this instance defines. Best-effort: a + // failure here leaves the database usable as `admin`, and the next role change repairs it. + if let Err(e) = crate::datatable_roles::converge_connect_grants(db, dbname).await { + tracing::warn!("Could not set CONNECT grants on instance database '{dbname}': {e}"); + } + tracing::info!("Created custom instance database '{}'", dbname); Ok(()) } diff --git a/backend/windmill-common/src/worker.rs b/backend/windmill-common/src/worker.rs index ee8a389bb7..51fc378f0b 100644 --- a/backend/windmill-common/src/worker.rs +++ b/backend/windmill-common/src/worker.rs @@ -1082,6 +1082,39 @@ pub struct SqlAnnotations { pub raw_output: bool, } +impl SqlAnnotations { + /// The data table role a query declares as `-- role `, if any. Only meaningful against a + /// `datatable://` database that is under roles; absent means the data table's default role. + /// + /// Hand-written rather than derived because the value matters, not just the presence, and + /// because the executor needs it before it knows the connection is a data table at all. Like + /// every annotation it lives in the leading comment block and must be the whole line, so prose + /// such as `-- role based access is handled below` never matches. + pub fn datatable_role(code: &str) -> Option { + for line in code.lines() { + let line = line.trim(); + if line.is_empty() { + continue; + } + if !line.starts_with("--") { + break; + } + let mut tokens = line[2..].split_whitespace(); + if tokens.next() == Some("role") { + if let Some(role) = tokens.next() { + let is_role_name = role + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-'); + if is_role_name && tokens.next().is_none() { + return Some(role.to_string()); + } + } + } + } + None + } +} + #[annotations("#")] pub struct BashAnnotations { pub docker: bool, @@ -2653,6 +2686,31 @@ mod tests { use super::*; use std::collections::HashMap; + #[test] + fn datatable_role_is_read_from_the_leading_comment_block() { + assert_eq!( + SqlAnnotations::datatable_role("-- role analytics\nSELECT 1"), + Some("analytics".to_string()) + ); + // Blank lines and other annotations before it are fine. + assert_eq!( + SqlAnnotations::datatable_role("\n-- prepare\n-- role read_only\nSELECT 1"), + Some("read_only".to_string()) + ); + // Prose that merely starts with the word, and anything past the first statement, is not an + // annotation — otherwise a comment could silently change which login a query runs as. + assert_eq!( + SqlAnnotations::datatable_role("-- role based access is handled below\nSELECT 1"), + None + ); + assert_eq!( + SqlAnnotations::datatable_role("SELECT 1;\n-- role analytics"), + None + ); + assert_eq!(SqlAnnotations::datatable_role("-- role an;alytics"), None); + assert_eq!(SqlAnnotations::datatable_role("SELECT 1"), None); + } + fn matcher(id: &str) -> WorkspaceMatcher { WorkspaceMatcher { id: id.to_string(), include_forks: false } } diff --git a/backend/windmill-common/src/workspaces.rs b/backend/windmill-common/src/workspaces.rs index 7cf910ef84..4b1dc96cb6 100644 --- a/backend/windmill-common/src/workspaces.rs +++ b/backend/windmill-common/src/workspaces.rs @@ -6,6 +6,7 @@ use serde::{Deserialize, Serialize}; use strum::AsRefStr; use crate::{ + datatable_roles::{ADMIN_DATATABLE_ROLE, CUSTOM_INSTANCE_USER}, error::{self, to_anyhow, Error, Result}, get_database_url, secret_backend::{get_secret_value, is_external_stored_value}, @@ -1292,9 +1293,17 @@ impl Default for DataTableForkBehavior { } } -#[derive(Deserialize, Serialize, Debug)] +#[derive(Deserialize, Serialize, Debug, Clone)] pub struct DataTable { - pub database: DataTableDatabase, + /// Set on a *terminal* entry — one that owns its database. Mutually exclusive with + /// [`DataTable::reference`]; [`validate_datatable_shape`] is the one place that enforces it. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub database: Option, + /// Set on a *pointer* entry — one that names another workspace's entry and owns nothing. + /// A keep-original fork gets one of these instead of a copy of the parent's entry, so there is + /// nothing local for a fork admin to widen. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub reference: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub forked_from: Option, /// Whether the SQL-migrations feature is opted in for this data table. @@ -1302,22 +1311,85 @@ pub struct DataTable { /// when migrations already exist (see `datatable_migrations_enabled`). #[serde(default, skip_serializing_if = "Option::is_none")] pub migrations_enabled: Option, + /// Who may connect as which role. Absent = unpermissioned: every caller connects as `admin`, + /// which is how data tables behaved before roles existed. Only meaningful on a terminal entry; + /// a pointer is governed by what it points at. + /// + /// Never leaves the instance: stripped from the workspace export and ignored on import, since + /// tenants are workspace-scoped names and syncing them would make repo write access a second + /// door onto the access decision. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub permissions: Option, } -#[derive(Deserialize, Serialize, Debug)] +/// A pointer at another workspace's data table entry. +#[derive(Deserialize, Serialize, Debug, Clone, PartialEq)] +pub struct DataTableReference { + pub workspace_id: String, + pub datatable: String, +} + +/// The access decision for one data table: which role a caller gets, and who may ask for each. +#[derive(Deserialize, Serialize, Debug, Clone, Default)] +pub struct DataTablePermissions { + /// A role id from the instance catalog, or `admin`. Absent = `admin`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub default_role: Option, + /// Keyed by instance role id, plus the reserved `admin` key. A role absent from this map + /// cannot be used on this data table at all, whatever the instance catalog says. + #[serde(default)] + pub roles: std::collections::BTreeMap, +} + +impl DataTablePermissions { + pub fn default_role(&self) -> &str { + self.default_role.as_deref().unwrap_or(ADMIN_DATATABLE_ROLE) + } +} + +#[derive(Deserialize, Serialize, Debug, Clone, Default)] +pub struct DataTableRoleTenants { + /// `u/`, `g/`, `f/`, or `*` for every member. + #[serde(default)] + pub tenants: Vec, +} + +/// Every member of the governing workspace. +pub const DATATABLE_TENANT_WILDCARD: &str = "*"; + +/// How deep a chain of pointer entries may go before it is called a loop. Data tables are not +/// expected to chain at all — a fork points at its parent — so this only has to be generous +/// enough to survive a fork of a fork. +const DATATABLE_REFERENCE_MAX_DEPTH: usize = 20; + +/// Exactly one of `database` and `reference` must be set. Called wherever an entry is persisted, +/// so nothing downstream has to handle an entry that is both or neither. +pub fn validate_datatable_shape(name: &str, dt: &DataTable) -> Result<()> { + match (&dt.database, &dt.reference) { + (Some(_), None) | (None, Some(_)) => Ok(()), + (Some(_), Some(_)) => Err(Error::BadRequest(format!( + "Data table '{name}' both owns a database and points at another one" + ))), + (None, None) => Err(Error::BadRequest(format!( + "Data table '{name}' names neither a database nor another data table" + ))), + } +} + +#[derive(Deserialize, Serialize, Debug, Clone)] pub struct DataTableForkedFrom { /// Schema snapshot at fork time #[serde(default, skip_serializing_if = "Option::is_none")] pub schema: Option, } -#[derive(Deserialize, Serialize, Debug)] +#[derive(Deserialize, Serialize, Debug, Clone)] pub struct DataTableDatabase { pub resource_type: DataTableCatalogResourceType, pub resource_path: String, } -#[derive(Deserialize, Serialize, Debug, PartialEq)] +#[derive(Deserialize, Serialize, Debug, PartialEq, Clone, Copy)] #[serde(rename_all = "lowercase")] #[derive(AsRefStr)] #[strum(serialize_all = "lowercase")] @@ -1353,37 +1425,8 @@ fn datatable_not_found_error(name: &str, datatables: Option<&serde_json::Value>) )) } -pub async fn get_datatable_resource_from_db_unchecked( - db: &DB, - w_id: &str, - name: &str, -) -> Result { - get_datatable_resource_inner(db, w_id, name, false).await -} - -/// Same as [`get_datatable_resource_from_db_unchecked`] but for postgres trigger -/// connections: custom-instance datatables resolve to -/// `custom_instance_replication_user` rather than `custom_instance_user`. BYO-postgres -/// datatables resolve to the user's own resource unchanged; configuring it for -/// replication there is the user's responsibility. -/// -/// Authorization: like its `_unchecked` sibling, returns resolved connection -/// credentials and performs no authorization — callers MUST have already authorized -/// access to the datatable (e.g. the trigger's own create-time check). -pub async fn get_datatable_replication_resource_from_db_unchecked( - db: &DB, - w_id: &str, - name: &str, -) -> Result { - get_datatable_resource_inner(db, w_id, name, true).await -} - -async fn get_datatable_resource_inner( - db: &DB, - w_id: &str, - name: &str, - replication: bool, -) -> Result { +/// Read one workspace's data table entry, without following a pointer. +pub async fn read_datatable_entry(db: &DB, w_id: &str, name: &str) -> Result { let datatables = sqlx::query_scalar!( r#" SELECT ws.datatable->'datatables' AS datatables @@ -1401,39 +1444,592 @@ async fn get_datatable_resource_inner( .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())?; + Ok(serde_json::from_value::(datatable.clone())?) +} - let db_resource = if datatable.database.resource_type == DataTableCatalogResourceType::Instance - { +/// The terminal entry a reference chain lands on: the workspace that governs the data table, the +/// entry name there, and the entry itself. A terminal entry resolves to itself. +/// +/// Every decision downstream — which database to connect to, whose `permissions` apply, whose +/// members tenants are evaluated against, who may administer it — is taken on this, never on the +/// entry the caller named. +pub struct GoverningDatatable { + pub workspace_id: String, + pub name: String, + pub datatable: DataTable, +} + +pub async fn resolve_governing_datatable( + db: &DB, + w_id: &str, + name: &str, +) -> Result { + let mut workspace_id = w_id.to_string(); + let mut name = name.to_string(); + for _ in 0..DATATABLE_REFERENCE_MAX_DEPTH { + let datatable = read_datatable_entry(db, &workspace_id, &name).await?; + validate_datatable_shape(&name, &datatable)?; + match &datatable.reference { + None => return Ok(GoverningDatatable { workspace_id, name, datatable }), + Some(reference) => { + workspace_id = reference.workspace_id.clone(); + name = reference.datatable.clone(); + } + } + } + Err(Error::BadRequest(format!( + "Data table '{name}' points at another data table through more than \ + {DATATABLE_REFERENCE_MAX_DEPTH} hops; the chain is likely a loop" + ))) +} + +/// Build the `admin` connection for a governing entry: `custom_instance_user` for an instance +/// database, the user's own resource for a BYO-postgres one. +async fn resolve_datatable_connection_unchecked( + db: &DB, + governing: &GoverningDatatable, + replication: bool, +) -> Result { + let database = governing + .datatable + .database + .as_ref() + .expect("a governing entry owns a database"); + if 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(); + pg_creds.dbname = database.resource_path.clone(); if replication { pg_creds.user = Some("custom_instance_replication_user".to_string()); pg_creds.password = Some(get_custom_pg_instance_replication_password(&db).await?); } else { - pg_creds.user = Some("custom_instance_user".to_string()); + pg_creds.user = Some(CUSTOM_INSTANCE_USER.to_string()); pg_creds.password = Some(get_custom_pg_instance_password(&db).await?); } serde_json::to_value(&pg_creds) - .map_err(|e| Error::internal_err(format!("Error serializing pg creds: {}", e)))? + .map_err(|e| Error::internal_err(format!("Error serializing pg creds: {}", e))) } else { // Name the data table too: the caller asked for one by name, and a bare // "resource f/x/y does not exist" leaves them to work out which one points at it. transform_json_unchecked( - &serde_json::Value::String(format!("$res:{}", datatable.database.resource_path)), - w_id, + &serde_json::Value::String(format!("$res:{}", database.resource_path)), + &governing.workspace_id, db, ) .await .map_err(|e| match e { - Error::NotFound(m) => Error::NotFound(format!("data table {name}: {m}")), + Error::NotFound(m) => Error::NotFound(format!("data table {}: {m}", governing.name)), e => e, - })? + }) + } +} + +/// Resolve a data table to connection credentials **without authorizing anything**: always the +/// `admin` connection. +/// +/// Authorization: callers MUST have authorized access already. Anything that acts for a user or a +/// job wants [`get_datatable_resource_from_db`] instead. +pub async fn get_datatable_resource_from_db_unchecked( + db: &DB, + w_id: &str, + name: &str, +) -> Result { + let governing = resolve_governing_datatable(db, w_id, name).await?; + resolve_datatable_connection_unchecked(db, &governing, false).await +} + +/// Same as [`get_datatable_resource_from_db_unchecked`] but for postgres trigger +/// connections: custom-instance datatables resolve to +/// `custom_instance_replication_user` rather than `custom_instance_user`. BYO-postgres +/// datatables resolve to the user's own resource unchanged; configuring it for +/// replication there is the user's responsibility. +/// +/// Authorization: a replication connection reads every row whatever the roles grant, so callers +/// must gate it with [`ensure_datatable_admin_access`] rather than a role check. +pub async fn get_datatable_replication_resource_from_db_unchecked( + db: &DB, + w_id: &str, + name: &str, +) -> Result { + let governing = resolve_governing_datatable(db, w_id, name).await?; + resolve_datatable_connection_unchecked(db, &governing, true).await +} + +/// The identity a resolution is made for. `Unchecked` is for callers that authorized already; +/// everything else is checked against the governing entry's tenants. +pub enum DatatableAccess<'a> { + /// Reaches every role. For callers that already authorized, or that have no user at all. + Unchecked, + Authed(crate::db::AuthedRef<'a>), + /// A job's owner, without reading the job row — only fetched if the data table turns out to + /// be permissioned. + PermissionedAs { + permissioned_as: &'a str, + email: &'a str, + }, + /// A job identified by id; its owner is read from `v2_job`. For agent workers and anything + /// else that authenticates as infrastructure rather than as the job's user. + Job(uuid::Uuid), + /// No identity established. Unpermissioned data tables resolve as before; permissioned ones + /// are refused, so a caller predating this feature fails closed. + NoIdentity, +} + +/// Does one tenant list cover this identity? Admins of the governing workspace pass everything — +/// they can edit the tenant lists anyway, so refusing them would only be theatre. +pub fn can_use_datatable_role( + tenants: &DataTableRoleTenants, + authed: &crate::db::AuthedRef<'_>, +) -> bool { + *authed.is_admin + || tenants.tenants.iter().any(|tenant| { + if tenant == DATATABLE_TENANT_WILDCARD { + return true; + } + match tenant.split_once('/') { + Some(("u", user)) => authed.username == user, + Some(("g", group)) => authed.groups.iter().any(|g| g == group), + Some(("f", folder)) => authed.folders.iter().any(|(f, _, _)| f == folder), + _ => false, + } + }) +} + +/// Evaluate a tenant list **as a member of the governing workspace**, whoever is calling. +/// +/// A caller reaching a data table through a pointer is a member of some other workspace, and being +/// its admin means nothing here — that is the whole point of the pointer. They are looked up in +/// the governing workspace by email and evaluated there, or refused when they are not a member. +/// A `g/` or `f/` permissioned-as from a foreign workspace is refused outright: those names are +/// defined per workspace and mean nothing outside the one that defined them. +pub async fn can_use_datatable_role_in_governing_workspace( + db: &DB, + governing_w_id: &str, + w_id: &str, + tenants: &DataTableRoleTenants, + access: &DatatableAccess<'_>, +) -> Result { + let (permissioned_as, email): (String, String) = match access { + DatatableAccess::Unchecked => return Ok(true), + DatatableAccess::NoIdentity => return Ok(false), + DatatableAccess::Authed(authed) => { + if w_id == governing_w_id { + return Ok(can_use_datatable_role(tenants, authed)); + } + (format!("u/{}", authed.username), authed.email.to_string()) + } + 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, + w_id, + ) + .fetch_optional(db) + .await? + .ok_or_else(|| Error::NotFound(format!("job {job_id} not found in {w_id}")))?; + (job.permissioned_as, job.permissioned_as_email) + } }; + if w_id == governing_w_id { + let authed = + crate::auth::fetch_authed_from_permissioned_as(&permissioned_as, &email, w_id, db) + .await?; + return Ok(can_use_datatable_role(tenants, &authed.to_authed_ref())); + } + if crate::auth::is_super_admin_email(db, &email).await? { + return Ok(true); + } + if !permissioned_as.starts_with("u/") { + return Ok(false); + } + let Some(username) = sqlx::query_scalar!( + "SELECT username FROM usr WHERE workspace_id = $1 AND email = $2 AND disabled = false", + governing_w_id, + &email + ) + .fetch_optional(db) + .await? + else { + return Ok(false); + }; + let authed = crate::auth::fetch_authed_from_permissioned_as( + &format!("u/{username}"), + &email, + governing_w_id, + db, + ) + .await?; + Ok(can_use_datatable_role(tenants, &authed.to_authed_ref())) +} + +/// Which tenant list a caller's role selection lands on. `Ok(None)` means the data table is +/// unpermissioned and resolves through its own `admin` connection, as it did before roles existed. +/// +/// `role` is the **name** a caller wrote (`-- role analytics`); it is mapped to the catalog id the +/// tenant lists are keyed by here, so a rename moves nothing. +fn datatable_role_entry<'a>( + permissions: Option<&'a DataTablePermissions>, + catalog: &crate::datatable_roles::DatatableRoleCatalog, + name: &str, + role: Option<&str>, +) -> Result> { + let Some(permissions) = permissions else { + return match role { + Some(role) if role != ADMIN_DATATABLE_ROLE => Err(Error::BadRequest(format!( + "Cannot use role '{role}': data table '{name}' is not under roles. \ + Put it under roles in its permissions drawer first." + ))), + _ => Ok(None), + }; + }; + let role_id = match role { + None => permissions.default_role().to_string(), + Some(ADMIN_DATATABLE_ROLE) => ADMIN_DATATABLE_ROLE.to_string(), + Some(role) => crate::datatable_roles::role_id_by_name(catalog, role)?.to_string(), + }; + let tenants = permissions.roles.get(&role_id).ok_or_else(|| { + let display = role.map(str::to_string).unwrap_or_else(|| { + catalog + .get(&role_id) + .map(|r| r.name.clone()) + .unwrap_or_else(|| role_id.clone()) + }); + Error::NotFound(format!( + "Role '{display}' is not among the roles of data table '{name}'" + )) + })?; + Ok(Some((role_id, tenants))) +} + +/// Resolve a data table to connection credentials for one identity. +/// +/// This is the chokepoint: everything that opens a connection to a data table on someone's behalf +/// goes through it. `role` is the role name the caller asked for — the `-- role` annotation, the +/// `?role=` on a `datatable://` reference, or `None` for the data table's default. +/// +/// The resolved role **logs in as itself**. Never `SET ROLE`: a script could `RESET ROLE` its way +/// back to admin. +pub async fn get_datatable_resource_from_db( + db: &DB, + w_id: &str, + name: &str, + role: Option<&str>, + access: DatatableAccess<'_>, +) -> Result { + let governing = resolve_governing_datatable(db, w_id, name).await?; + let mut db_resource = resolve_datatable_connection_unchecked(db, &governing, false).await?; + + let catalog = crate::datatable_roles::read_role_catalog(db).await?; + let Some((role_id, tenants)) = datatable_role_entry( + governing.datatable.permissions.as_ref(), + &catalog, + name, + role, + )? + else { + return Ok(db_resource); + }; + + // Only for a data table actually under roles: whether people name a role or ride the default + // is what says if the `-- role` annotation is carrying its weight. + crate::feature_usage::log_feature_usage( + "datatable", + "role_connection", + if role.is_some() { "named" } else { "default" }, + ); + + if !can_use_datatable_role_in_governing_workspace( + db, + &governing.workspace_id, + w_id, + tenants, + &access, + ) + .await? + { + let display = catalog + .get(&role_id) + .map(|r| r.name.as_str()) + .unwrap_or(role_id.as_str()); + return Err(Error::NotAuthorized(format!( + "Not allowed to use role '{display}' of data table '{name}'" + ))); + } + + if role_id == ADMIN_DATATABLE_ROLE { + return Ok(db_resource); + } + let entry = catalog.get(&role_id).ok_or_else(|| { + Error::NotFound(format!( + "Data table '{name}' names a role that no longer exists on this instance" + )) + })?; + if !entry.enabled { + return Err(Error::BadRequest(format!( + "Data table role '{}' is disabled on this instance", + entry.name + ))); + } + let pwd = entry.pwd.as_ref().ok_or_else(|| { + Error::internal_err(format!( + "Data table role '{}' has no stored credential; recreate it in instance settings", + entry.name + )) + })?; + db_resource["user"] = serde_json::Value::String(entry.name.clone()); + db_resource["password"] = serde_json::Value::String(pwd.clone()); Ok(db_resource) } +/// Would the chokepoint accept this identity connecting as this role? Answers without resolving +/// credentials, for callers that want to refuse early and say which thing was refused. +/// +/// Not the security boundary — [`get_datatable_resource_from_db`] re-checks when it actually opens +/// the connection. This is what turns "permission denied for table x" into a message naming the +/// migration and the role. +pub async fn ensure_can_use_datatable_role( + db: &DB, + w_id: &str, + name: &str, + role: Option<&str>, + access: &DatatableAccess<'_>, + context: &str, +) -> Result<()> { + let governing = resolve_governing_datatable(db, w_id, name).await?; + let catalog = crate::datatable_roles::read_role_catalog(db).await?; + let Some((role_id, tenants)) = + datatable_role_entry(governing.datatable.permissions.as_ref(), &catalog, name, role)? + else { + return Ok(()); + }; + if can_use_datatable_role_in_governing_workspace( + db, + &governing.workspace_id, + w_id, + tenants, + access, + ) + .await? + { + return Ok(()); + } + let display = catalog + .get(&role_id) + .map(|r| r.name.as_str()) + .unwrap_or(role_id.as_str()); + Err(Error::NotAuthorized(format!( + "{context} runs as role '{display}' of data table '{name}', which you are not allowed to use" + ))) +} + +/// Gate the operations that see the whole database whatever the roles grant: replication streams, +/// a migration that declares no role, exports, and editing the permissions themselves. Passing +/// means the caller could have connected as `admin` anyway. +pub async fn ensure_datatable_admin_access( + db: &DB, + w_id: &str, + name: &str, + access: &DatatableAccess<'_>, +) -> Result<()> { + let governing = resolve_governing_datatable(db, w_id, name).await?; + let Some(permissions) = governing.datatable.permissions.as_ref() else { + return Ok(()); + }; + let admin = permissions + .roles + .get(ADMIN_DATATABLE_ROLE) + .cloned() + .unwrap_or_default(); + if can_use_datatable_role_in_governing_workspace( + db, + &governing.workspace_id, + w_id, + &admin, + access, + ) + .await? + { + Ok(()) + } else { + Err(Error::NotAuthorized(format!( + "Data table '{name}' is under roles; this reaches the whole database, so it is for \ + the admins of workspace '{}', which governs it.", + governing.workspace_id + ))) + } +} + +/// Rewrite the `permissions` of every data table entry of one workspace, in the caller's +/// transaction. `change` reports whether it touched anything; the row is only written when +/// something did. +/// +/// 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. +pub async fn update_datatable_permissions_in_workspace( + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + w_id: &str, + change: F, +) -> Result<()> +where + F: Fn(&mut DataTablePermissions) -> bool, +{ + let Some(mut settings) = sqlx::query_scalar!( + "SELECT datatable FROM workspace_settings WHERE workspace_id = $1 FOR UPDATE", + w_id + ) + .fetch_optional(&mut **tx) + .await? + .flatten() else { + return Ok(()); + }; + + let Some(datatables) = settings + .get_mut("datatables") + .and_then(|d| d.as_object_mut()) + else { + return Ok(()); + }; + + let mut touched = false; + for entry in datatables.values_mut() { + let Some(permissions) = entry.get("permissions").filter(|p| !p.is_null()) else { + continue; + }; + let Ok(mut permissions) = + serde_json::from_value::(permissions.clone()) + else { + continue; + }; + if change(&mut permissions) { + entry["permissions"] = serde_json::to_value(&permissions) + .map_err(|e| Error::internal_err(format!("serializing permissions: {e}")))?; + touched = true; + } + } + + if touched { + sqlx::query!( + "UPDATE workspace_settings SET datatable = $1 WHERE workspace_id = $2", + settings, + w_id + ) + .execute(&mut **tx) + .await?; + } + Ok(()) +} + +/// Drop a freed principal (`u/alice`, `g/analysts`, `f/finance`) from every tenant list of one +/// workspace. +pub async fn remove_datatable_tenant_in_workspace( + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + w_id: &str, + tenant: &str, +) -> Result<()> { + update_datatable_permissions_in_workspace(tx, w_id, |permissions| { + let mut touched = false; + for role in permissions.roles.values_mut() { + let before = role.tenants.len(); + role.tenants.retain(|t| t != tenant); + touched |= role.tenants.len() != before; + } + touched + }) + .await +} + +/// Follow a renamed principal through every tenant list of one workspace. +pub async fn rename_datatable_tenant_in_workspace( + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + w_id: &str, + old: &str, + new: &str, +) -> Result<()> { + update_datatable_permissions_in_workspace(tx, w_id, |permissions| { + let mut touched = false; + for role in permissions.roles.values_mut() { + for tenant in role.tenants.iter_mut() { + if tenant == old { + *tenant = new.to_string(); + touched = true; + } + } + if touched { + role.tenants.dedup(); + } + } + touched + }) + .await +} + +/// 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 +/// 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!( + "SELECT workspace_id FROM workspace_settings WHERE datatable::text LIKE $1", + format!("%{}%", role_id) + ) + .fetch_all(db) + .await?; + + for w_id in workspaces { + let mut tx = db.begin().await?; + update_datatable_permissions_in_workspace(&mut tx, &w_id, |permissions| { + let mut touched = permissions.roles.remove(role_id).is_some(); + if permissions.default_role.as_deref() == Some(role_id) { + permissions.default_role = Some(ADMIN_DATATABLE_ROLE.to_string()); + touched = true; + } + touched + }) + .await?; + tx.commit().await?; + } + Ok(()) +} + +/// Drop the `permissions` block from a `workspace_settings.datatable` value before it leaves the +/// server. +/// +/// Who may connect as which role is an access decision, not configuration, and its tenants name +/// principals of one workspace — `g/analysts` in dev is a different group from `g/analysts` in +/// prod. Shipping it would both mean nothing at the far end and turn a settings push into a way to +/// widen access, so the decision stays where it was made. [`DataTable`] deserializes fine without +/// it, and the settings-editing endpoint carries the stored block across untouched. +pub fn strip_datatable_permissions( + datatable: Option, +) -> Option { + let mut datatable = datatable?; + if let Some(entries) = datatable + .get_mut("datatables") + .and_then(|d| d.as_object_mut()) + { + for entry in entries.values_mut() { + if let Some(entry) = entry.as_object_mut() { + entry.remove("permissions"); + } + } + } + Some(datatable) +} + +/// Split a `datatable://` reference into its name and the role its query string names. +pub fn parse_datatable_ref(reference: &str) -> (&str, Option<&str>) { + let (name, query) = reference.split_once('?').unwrap_or((reference, "")); + let role = query + .split('&') + .find_map(|param| param.strip_prefix("role=")) + .filter(|role| !role.is_empty()); + (name, role) +} + #[derive(Deserialize, Serialize, Debug)] pub struct Ducklake { pub catalog: DucklakeCatalog, @@ -2642,6 +3238,90 @@ async fn transform_json_unchecked( mod tests { use super::*; + fn tenants(list: &[&str]) -> DataTableRoleTenants { + DataTableRoleTenants { tenants: list.iter().map(|t| t.to_string()).collect() } + } + + #[test] + fn a_tenant_list_covers_users_groups_folders_and_the_wildcard() { + let groups = vec!["analysts".to_string()]; + let folders = vec![("finance".to_string(), true, false)]; + let scopes = None; + let token_prefix = None; + let is_admin = false; + let is_operator = false; + let authed = crate::db::AuthedRef { + email: "alice@windmill.dev", + username: "alice", + is_admin: &is_admin, + is_operator: &is_operator, + groups: &groups, + folders: &folders, + scopes: &scopes, + token_prefix: &token_prefix, + }; + + assert!(can_use_datatable_role(&tenants(&["u/alice"]), &authed)); + assert!(can_use_datatable_role(&tenants(&["g/analysts"]), &authed)); + assert!(can_use_datatable_role(&tenants(&["f/finance"]), &authed)); + assert!(can_use_datatable_role(&tenants(&["*"]), &authed)); + assert!(!can_use_datatable_role(&tenants(&[]), &authed)); + assert!(!can_use_datatable_role(&tenants(&["u/bob", "g/ops"]), &authed)); + // A bare name is not a principal: only the three prefixes and the wildcard match. + assert!(!can_use_datatable_role(&tenants(&["alice"]), &authed)); + + // An admin of the governing workspace reaches every role: they can edit the lists anyway. + let is_admin = true; + let admin = crate::db::AuthedRef { is_admin: &is_admin, ..authed }; + assert!(can_use_datatable_role(&tenants(&[]), &admin)); + } + + #[test] + fn a_datatable_ref_splits_off_its_role() { + assert_eq!(parse_datatable_ref("sales"), ("sales", None)); + assert_eq!( + parse_datatable_ref("sales?role=analytics"), + ("sales", Some("analytics")) + ); + assert_eq!( + parse_datatable_ref("sales?x=1&role=analytics"), + ("sales", Some("analytics")) + ); + // An empty role is no role rather than a role named "". + assert_eq!(parse_datatable_ref("sales?role="), ("sales", None)); + } + + #[test] + fn an_entry_owns_a_database_or_points_at_one_but_never_both() { + let terminal = DataTable { + database: Some(DataTableDatabase { + resource_type: DataTableCatalogResourceType::Instance, + resource_path: "dt_main".to_string(), + }), + reference: None, + forked_from: None, + migrations_enabled: None, + permissions: None, + }; + assert!(validate_datatable_shape("main", &terminal).is_ok()); + + let pointer = DataTable { + database: None, + reference: Some(DataTableReference { + workspace_id: "prod".to_string(), + datatable: "main".to_string(), + }), + ..terminal.clone() + }; + assert!(validate_datatable_shape("main", &pointer).is_ok()); + + let both = DataTable { database: terminal.database.clone(), ..pointer.clone() }; + assert!(validate_datatable_shape("main", &both).is_err()); + + let neither = DataTable { database: None, reference: None, ..terminal.clone() }; + assert!(validate_datatable_shape("main", &neither).is_err()); + } + #[test] fn test_parse_fork_branch() { // Generated fork (`wm-fork-abc`) and dev workspace (`staging`) forms. diff --git a/backend/windmill-trigger-postgres/src/lib.rs b/backend/windmill-trigger-postgres/src/lib.rs index 571aac0a66..c77f7ce0d3 100644 --- a/backend/windmill-trigger-postgres/src/lib.rs +++ b/backend/windmill-trigger-postgres/src/lib.rs @@ -11,7 +11,10 @@ use serde::{Deserialize, Deserializer, Serialize}; use serde_json::value::RawValue; use sqlx::FromRow; use windmill_api_auth::ApiAuthed; -use windmill_common::workspaces::get_datatable_replication_resource_from_db_unchecked; +use windmill_common::workspaces::{ + ensure_datatable_admin_access, get_datatable_replication_resource_from_db_unchecked, + DatatableAccess, +}; use windmill_common::{ db::UserDB, error::{to_anyhow, Error, Result}, @@ -382,6 +385,16 @@ pub async fn resolve_postgres_resource( w_id: &str, ) -> Result { if let Some(datatable_name) = postgres_resource_path.strip_prefix("datatable://") { + // A replication stream reads every row of every table whatever the data table's roles + // grant, so it is not something a role can be tenanted into: only someone who could have + // connected as `admin` may open one. + ensure_datatable_admin_access( + db, + w_id, + datatable_name, + &DatatableAccess::Authed(authed.to_authed_ref()), + ) + .await?; // Trigger connections (publication/slot management + logical replication) run // as the dedicated replication user on custom-instance databases. let resource_value = diff --git a/backend/windmill-worker/src/agent_workers.rs b/backend/windmill-worker/src/agent_workers.rs index 8dccf691fa..e0971c08ce 100644 --- a/backend/windmill-worker/src/agent_workers.rs +++ b/backend/windmill-worker/src/agent_workers.rs @@ -64,16 +64,25 @@ pub async fn get_ducklake_from_agent_http( .await } +/// An agent worker authenticates as infrastructure, not as the job's user, so the job id travels +/// with the request: the server reads the job's owner from it and evaluates the data table's +/// tenants against them. A worker predating this sends neither, and the server fails it closed on +/// a data table under roles. #[allow(dead_code)] pub async fn get_datatable_resource_from_agent_http( client: &HttpClient, name: &str, w_id: &str, + role: Option<&str>, + job_id: &uuid::Uuid, ) -> anyhow::Result { + let role_query = role + .map(|r| format!("&role={}", urlencoding::encode(r))) + .unwrap_or_default(); client .get(&format!( - "/api/w/{}/agent_workers/get_datatable_resource/{}", - w_id, &name + "/api/w/{}/agent_workers/get_datatable_resource/{}?job_id={}{}", + w_id, &name, job_id, role_query )) .await } diff --git a/backend/windmill-worker/src/duckdb_executor.rs b/backend/windmill-worker/src/duckdb_executor.rs index b2e69d26d6..88b367b191 100644 --- a/backend/windmill-worker/src/duckdb_executor.rs +++ b/backend/windmill-worker/src/duckdb_executor.rs @@ -13,8 +13,8 @@ use windmill_common::error::{to_anyhow, Error, Result}; use windmill_common::utils::sanitize_string_from_password; use windmill_common::worker::{get_memory, to_raw_value, Connection, SqlResultCollectionStrategy}; use windmill_common::workspaces::{ - get_datatable_resource_from_db_unchecked, get_ducklake_from_db_unchecked, - strip_fork_reserved_attach_args, DucklakeCatalogResourceType, + get_datatable_resource_from_db, get_ducklake_from_db_unchecked, + strip_fork_reserved_attach_args, DatatableAccess, DucklakeCatalogResourceType, }; use windmill_common::PgDatabase; use windmill_object_store::S3_PROXY_LAST_ERRORS_CACHE; @@ -1494,12 +1494,8 @@ pub async fn do_duckdb( .await? { probe_blocks.extend(q); - } else if let Some(q) = transform_attach_datatable( - &query_block, - conn, - &mut hidden_passwords, - &job.workspace_id, - ) + } else if let Some(q) = + transform_attach_datatable(&query_block, conn, &mut hidden_passwords, job) .await? { probe_blocks.extend(q); @@ -1575,12 +1571,8 @@ pub async fn do_duckdb( .await? { v.extend(ducklake_query); - } else if let Some(datatable_query) = transform_attach_datatable( - &query_block, - conn, - &mut hidden_passwords, - &job.workspace_id, - ) + } else if let Some(datatable_query) = + transform_attach_datatable(&query_block, conn, &mut hidden_passwords, job) .await? { v.extend(datatable_query); @@ -2609,33 +2601,73 @@ fn fork_defer_statements( Ok(stmts) } +struct AttachedDatatable<'a> { + name: &'a str, + role: Option<&'a str>, + alias: &'a str, +} + +/// `ATTACH 'datatable[://][?role=]' AS `. A bare `datatable` names the default +/// data table, so the role query string has to be accepted with and without an explicit name. +fn parse_attach_datatable(query: &str) -> Option> { + lazy_static::lazy_static! { + static ref RE: regex::Regex = regex::Regex::new( + r"(?i)ATTACH\s*'datatable(://[^'?:]+)?(\?[^':]*)?'\s*AS\s+([^ ;]+)" + ).unwrap(); + } + let cap = RE.captures(query)?; + let name = cap.get(1).map(|m| &m.as_str()[3..]).unwrap_or("main"); + let role = cap + .get(2) + .and_then(|m| windmill_common::workspaces::parse_datatable_ref(m.as_str()).1); + let alias = cap.get(3).map(|m| m.as_str()).unwrap_or(""); + Some(AttachedDatatable { name, role, alias }) +} + async fn transform_attach_datatable( query: &str, conn: &Connection, hidden_passwords: &mut Arc>>, - w_id: &str, + job: &MiniPulledJob, ) -> Result>> { - lazy_static::lazy_static! { - static ref RE: regex::Regex = regex::Regex::new(r"(?i)ATTACH\s*'datatable(://[^':]+)?'\s*AS\s+([^ ;]+)").unwrap(); - } - let Some(cap) = RE.captures(query) else { + let Some(attached) = parse_attach_datatable(query) else { return Ok(None); }; - let name = cap.get(1).map(|m| &m.as_str()[3..]).unwrap_or("main"); - let alias_name = cap.get(2).map(|m| m.as_str()).unwrap_or(""); let db_resource = match conn { Connection::Http(client) => { - get_datatable_resource_from_agent_http(client, name, w_id).await? + get_datatable_resource_from_agent_http( + client, + attached.name, + &job.workspace_id, + attached.role, + &job.id, + ) + .await? + } + Connection::Sql(db) => { + get_datatable_resource_from_db( + db, + &job.workspace_id, + attached.name, + attached.role, + DatatableAccess::PermissionedAs { + permissioned_as: &job.permissioned_as, + email: &job.permissioned_as_email, + }, + ) + .await? } - Connection::Sql(db) => get_datatable_resource_from_db_unchecked(db, w_id, name).await?, }; if let Some(pwd) = db_resource.get("password").and_then(|p| p.as_str()) { hidden_passwords.lock().unwrap().push(pwd.to_string()); } - Ok(Some(pg_secret_attach_statements(db_resource, alias_name)?)) + Ok(Some(pg_secret_attach_statements( + db_resource, + attached.alias, + )?)) } // Secret names must be plain identifiers; the hash keeps two aliases distinct even @@ -2753,6 +2785,20 @@ pub struct Arg { mod tests { use super::*; + #[test] + fn attach_datatable_parses_name_and_role() { + let named = parse_attach_datatable("ATTACH 'datatable://sales?role=analytics' AS dt").unwrap(); + assert_eq!((named.name, named.role, named.alias), ("sales", Some("analytics"), "dt")); + // A bare `datatable` is the default one, and still takes a role. + let default = parse_attach_datatable("ATTACH 'datatable?role=analytics' AS dt").unwrap(); + assert_eq!((default.name, default.role), ("main", Some("analytics"))); + let no_role = parse_attach_datatable("ATTACH 'datatable://sales' AS dt").unwrap(); + assert_eq!((no_role.name, no_role.role), ("sales", None)); + let bare = parse_attach_datatable("ATTACH 'datatable' AS dt").unwrap(); + assert_eq!((bare.name, bare.role), ("main", None)); + assert!(parse_attach_datatable("SELECT 1").is_none()); + } + #[test] fn decode_ffi_error_unescapes_multiline_and_strips_quotes() { // Mirror the FFI: JSON-encode the raw DuckDB message, prefix "ERROR ". diff --git a/backend/windmill-worker/src/pg_executor.rs b/backend/windmill-worker/src/pg_executor.rs index 1413924bf3..631794a529 100644 --- a/backend/windmill-worker/src/pg_executor.rs +++ b/backend/windmill-worker/src/pg_executor.rs @@ -26,9 +26,11 @@ use windmill_common::azure_workload_identity::WORKLOAD_IDENTITY_PASSWORD; use windmill_common::error::to_anyhow; use windmill_common::error::{self, Error}; use windmill_common::worker::{ - to_raw_value, Connection, SqlResultCollectionStrategy, CLOUD_HOSTED, + to_raw_value, Connection, SqlAnnotations, SqlResultCollectionStrategy, CLOUD_HOSTED, +}; +use windmill_common::workspaces::{ + get_datatable_resource_from_db, parse_datatable_ref, DatatableAccess, }; -use windmill_common::workspaces::get_datatable_resource_from_db_unchecked; use windmill_common::{PgDatabase, PrepareQueryColumnInfo, PrepareQueryResult, DB}; use windmill_parser::{Arg, Typ}; use windmill_parser_sql::{ @@ -680,15 +682,35 @@ pub async fn do_postgresql( } else { match pg_args.get("database").cloned() { Some(Value::String(db_str)) if db_str.starts_with("datatable://") => { - let db_str = db_str.trim_start_matches("datatable://"); + let reference = db_str.trim_start_matches("datatable://"); + let (db_str, uri_role) = parse_datatable_ref(reference); + // The annotation wins: a generated query can carry a `?role=` in the reference it + // was handed, but only the script's author writes the leading comment block. + let annotated = SqlAnnotations::datatable_role(&query); + let role = annotated.as_deref().or(uri_role); Some(match conn { Connection::Http(client) => { - get_datatable_resource_from_agent_http(client, &db_str, &job.workspace_id) - .await? + get_datatable_resource_from_agent_http( + client, + db_str, + &job.workspace_id, + role, + &job.id, + ) + .await? } Connection::Sql(db) => { - get_datatable_resource_from_db_unchecked(db, &job.workspace_id, &db_str) - .await? + get_datatable_resource_from_db( + db, + &job.workspace_id, + db_str, + role, + DatatableAccess::PermissionedAs { + permissioned_as: &job.permissioned_as, + email: &job.permissioned_as_email, + }, + ) + .await? } }) } diff --git a/cli/src/guidance/skills.gen.ts b/cli/src/guidance/skills.gen.ts index 993f546d3a..cccb105373 100644 --- a/cli/src/guidance/skills.gen.ts +++ b/cli/src/guidance/skills.gen.ts @@ -1110,6 +1110,8 @@ parseS3Object(s3Object: S3Object): S3ObjectRecord /** * Create a SQL template function for PostgreSQL/datatable queries * @param name - Database/datatable name (default: "main") + * @param opts.role - Connect as this data table role instead of the data table's default one. + * Only meaningful on a data table under roles, and only for a role you are a tenant of. * @returns SQL template function for building parameterized queries * @example * let sql = wmill.datatable() @@ -1119,8 +1121,11 @@ parseS3Object(s3Object: S3Object): S3ObjectRecord * SELECT * FROM friends * WHERE name = \${name} AND age = \${age}::int * \`.fetch() + * @example + * // Read through a restricted role + * let sql = wmill.datatable("main", { role: "analytics" }) */ -datatable(name: string = "main"): DatatableSqlTemplateFunction +datatable(name: string = "main", opts?: DatatableOptions): DatatableSqlTemplateFunction /** * Create a SQL template function for DuckDB/ducklake queries @@ -1901,6 +1906,8 @@ parseS3Object(s3Object: S3Object): S3ObjectRecord /** * Create a SQL template function for PostgreSQL/datatable queries * @param name - Database/datatable name (default: "main") + * @param opts.role - Connect as this data table role instead of the data table's default one. + * Only meaningful on a data table under roles, and only for a role you are a tenant of. * @returns SQL template function for building parameterized queries * @example * let sql = wmill.datatable() @@ -1910,8 +1917,11 @@ parseS3Object(s3Object: S3Object): S3ObjectRecord * SELECT * FROM friends * WHERE name = \${name} AND age = \${age}::int * \`.fetch() + * @example + * // Read through a restricted role + * let sql = wmill.datatable("main", { role: "analytics" }) */ -datatable(name: string = "main"): DatatableSqlTemplateFunction +datatable(name: string = "main", opts?: DatatableOptions): DatatableSqlTemplateFunction /** * Create a SQL template function for DuckDB/ducklake queries @@ -2786,6 +2796,8 @@ parseS3Object(s3Object: S3Object): S3ObjectRecord /** * Create a SQL template function for PostgreSQL/datatable queries * @param name - Database/datatable name (default: "main") + * @param opts.role - Connect as this data table role instead of the data table's default one. + * Only meaningful on a data table under roles, and only for a role you are a tenant of. * @returns SQL template function for building parameterized queries * @example * let sql = wmill.datatable() @@ -2795,8 +2807,11 @@ parseS3Object(s3Object: S3Object): S3ObjectRecord * SELECT * FROM friends * WHERE name = \${name} AND age = \${age}::int * \`.fetch() + * @example + * // Read through a restricted role + * let sql = wmill.datatable("main", { role: "analytics" }) */ -datatable(name: string = "main"): DatatableSqlTemplateFunction +datatable(name: string = "main", opts?: DatatableOptions): DatatableSqlTemplateFunction /** * Create a SQL template function for DuckDB/ducklake queries @@ -4405,10 +4420,13 @@ def send_teams_message(conversation_id: str, text: str, success: bool = True, ca # # Args: # name: Database name (default: "main") +# role: Connect as this data table role instead of the data table's default one. +# Only meaningful on a data table under roles, and only for a role you are a +# tenant of. # # Returns: # DataTableClient instance -def datatable(name: str = 'main') +def datatable(name: str = 'main', role: Optional[str] = None) # Get a DuckLake client for DuckDB queries. # diff --git a/frontend/src/lib/components/InstanceSettings.svelte b/frontend/src/lib/components/InstanceSettings.svelte index f38c0d3892..6ceab37872 100644 --- a/frontend/src/lib/components/InstanceSettings.svelte +++ b/frontend/src/lib/components/InstanceSettings.svelte @@ -1093,7 +1093,8 @@ migrations are set up and used, how often an empty workspace home is seen, how often the home page’s create menu and hub-project picker are opened and from which entry point, the name of any public hub project imported from the home page and how far that - import got, and whether a pre-approved trial offer was opened, last 30 days)
  • feature adoption (counts of which flow, script, trigger, worker and data table @@ -1159,7 +1160,8 @@ migrations are set up and used, how often an empty workspace home is seen, how often the home page’s create menu and hub-project picker are opened and from which entry point, the name of any public hub project imported from the home page and how far that - import got, and whether a pre-approved trial offer was opened, last 30 days)
  • feature adoption (counts of which flow, script, trigger, worker and data table diff --git a/frontend/src/lib/components/sidebar/DeleteForkedWorkspaceModal.svelte b/frontend/src/lib/components/sidebar/DeleteForkedWorkspaceModal.svelte index 3a73758146..060a679d75 100644 --- a/frontend/src/lib/components/sidebar/DeleteForkedWorkspaceModal.svelte +++ b/frontend/src/lib/components/sidebar/DeleteForkedWorkspaceModal.svelte @@ -45,11 +45,13 @@ const settings = await WorkspaceService.getPublicSettings({ workspace: $workspaceStore }) const datatables = settings.datatable?.datatables ?? {} forkedDatatables = Object.entries(datatables) - .filter(([_, dt]) => dt.forked_from != null) + // A clone owns its database and is droppable; an entry pointing at the parent's is + // not this workspace's to drop, and never carries a clone stamp anyway. + .filter(([_, dt]) => dt.forked_from != null && dt.database != null) .map(([name, dt]) => ({ name, - resourceType: dt.database.resource_type ?? 'instance', - resourcePath: dt.database.resource_path ?? '', + resourceType: dt.database?.resource_type ?? 'instance', + resourcePath: dt.database?.resource_path ?? '', dropOnDelete: true })) } catch { diff --git a/frontend/src/lib/components/workspaceSettings/DataTablePermissionsButton.svelte b/frontend/src/lib/components/workspaceSettings/DataTablePermissionsButton.svelte new file mode 100644 index 0000000000..9ff4b67d4c --- /dev/null +++ b/frontend/src/lib/components/workspaceSettings/DataTablePermissionsButton.svelte @@ -0,0 +1,271 @@ + + + + {/if} +
    + {#if editable && row.id !== ADMIN_ROLE} + removeRole(row.id)} /> + {/if} + + + + {/each} + + + {#if editable && unusedRoles.length > 0} +
    + +