diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 3a820786e3..4a22116566 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -56f5e6a82056fa63323d6d4e1ec44832e64512fc +e8e323558523176e544927b44acf084c27a1b2aa diff --git a/backend/windmill-api-workspaces/src/datatable_acl.rs b/backend/windmill-api-workspaces/src/datatable_acl.rs index f41bb39dd4..88931737d9 100644 --- a/backend/windmill-api-workspaces/src/datatable_acl.rs +++ b/backend/windmill-api-workspaces/src/datatable_acl.rs @@ -35,7 +35,7 @@ use windmill_common::workspaces::{ }; use windmill_common::{PgDatabase, DB}; -use crate::datatable_permissions::{connect_as_admin, quote_ident, read_datatable}; +use crate::datatable_permissions::{connect_as_admin, read_datatable}; pub(crate) fn routes() -> Router { Router::new() @@ -68,29 +68,8 @@ pub enum AclTarget { } impl AclTarget { - /// The schema the target is in, absent for the database itself. - fn schema(&self) -> Option<&str> { - match self { - AclTarget::Database => None, - AclTarget::Schema { schema } => Some(schema), - AclTarget::Table { schema, .. } => Some(schema), - } - } - - /// How the target reads in the statements that name it, `dbname` being the - /// database the connection is on — the target never carries it. - fn object(&self, dbname: &str) -> String { - match self { - AclTarget::Database => format!("DATABASE {}", quote_ident(dbname)), - AclTarget::Schema { schema } => format!("SCHEMA {}", quote_ident(schema)), - AclTarget::Table { schema, table } => { - format!("TABLE {}.{}", quote_ident(schema), quote_ident(table)) - } - } - } - /// What it is called in a message. - fn label(&self, dbname: &str) -> String { + pub(crate) fn label(&self, dbname: &str) -> String { match self { AclTarget::Database => dbname.to_string(), AclTarget::Schema { schema } => schema.clone(), @@ -143,65 +122,6 @@ pub enum GrantScope { FutureFunctions, } -impl GrantScope { - /// The privileges Postgres accepts for what this scope names. - fn allowed_privileges(&self, target: &AclTarget) -> Result<&'static [&'static str]> { - Ok(match self { - GrantScope::Target => match target { - AclTarget::Database => DATABASE_PRIVILEGES, - AclTarget::Schema { .. } => SCHEMA_PRIVILEGES, - AclTarget::Table { .. } => TABLE_PRIVILEGES, - }, - // Everything below reads `IN SCHEMA`, which a database target has - // none of: schemas are granted on one at a time. - _ if matches!(target, AclTarget::Database) => { - return Err(Error::BadRequest( - "A database can only be granted on itself".to_string(), - )) - } - GrantScope::AllTables | GrantScope::FutureTables => TABLE_PRIVILEGES, - GrantScope::AllSequences | GrantScope::FutureSequences => SEQUENCE_PRIVILEGES, - GrantScope::AllFunctions | GrantScope::FutureFunctions => FUNCTION_PRIVILEGES, - }) - } - - fn is_future(&self) -> bool { - matches!( - self, - GrantScope::FutureTables | GrantScope::FutureSequences | GrantScope::FutureFunctions - ) - } - - /// The plural Postgres uses in `ON ALL IN SCHEMA` and in - /// `ALTER DEFAULT PRIVILEGES ... ON `. - fn object_plural(&self) -> Option<&'static str> { - match self { - GrantScope::Target => None, - GrantScope::AllTables | GrantScope::FutureTables => Some("TABLES"), - GrantScope::AllSequences | GrantScope::FutureSequences => Some("SEQUENCES"), - GrantScope::AllFunctions | GrantScope::FutureFunctions => Some("FUNCTIONS"), - } - } -} - -/// `CREATE` on a database is the privilege to create schemas in it. -const DATABASE_PRIVILEGES: &[&str] = &["CONNECT", "CREATE", "TEMPORARY"]; -const SCHEMA_PRIVILEGES: &[&str] = &["USAGE", "CREATE"]; -const TABLE_PRIVILEGES: &[&str] = &[ - "SELECT", - "INSERT", - "UPDATE", - "DELETE", - "TRUNCATE", - "REFERENCES", - "TRIGGER", - // Postgres 17. Accepted whatever the server's version, so that a grant read - // back from a 17 catalog can be revoked; an older server refuses it itself. - "MAINTAIN", -]; -const SEQUENCE_PRIVILEGES: &[&str] = &["USAGE", "SELECT", "UPDATE"]; -const FUNCTION_PRIVILEGES: &[&str] = &["EXECUTE"]; - /// A change to plan. One at a time: each is confirmed against its own SQL. #[derive(Deserialize, Serialize, Debug, Clone)] #[serde(tag = "type", rename_all = "snake_case")] @@ -425,212 +345,15 @@ fn windmill_role_of(roles: &BTreeMap, pg_role: &str) -> String { .unwrap_or_else(|| pg_role.to_string()) } -fn validate_privileges(privileges: &[String], allowed: &[&str]) -> Result> { - if privileges.is_empty() { - return Err(Error::BadRequest("No privilege selected".to_string())); - } - privileges - .iter() - .map(|p| { - let upper = p.to_uppercase(); - allowed - .iter() - .find(|a| **a == upper) - .map(|a| a.to_string()) - .ok_or_else(|| { - Error::BadRequest(format!( - "Privilege '{p}' does not apply here; expected one of {}", - allowed.join(", ") - )) - }) - }) - .collect() -} - -/// The statements one change plans out, against Postgres role names. -/// -/// Pure so the preview the user confirms is the same string that runs. -fn plan_statements( - target: &AclTarget, - change: &AclChange, - dbname: &str, - pg_role: &str, - other_pg_roles: &[String], - existing_objects: &[OwnedObject], -) -> Result { - let role = quote_ident(pg_role); - // Only the scopes that name a schema use this, and those are refused on a - // database target. - let schema = target.schema().map(quote_ident).unwrap_or_default(); - let mut statements = Vec::new(); - let mut warnings = Vec::new(); - - match change { - AclChange::SetOwner { .. } => { - statements.push(format!("ALTER {} OWNER TO {}", target.object(dbname), role)); - for object in existing_objects { - statements.push(format!( - "ALTER {} {} OWNER TO {}", - object.keyword, - object_ref(&schema, &object.name, object.args.as_deref()), - role - )); - } - // Ownership cannot be set ahead of time: an object belongs to - // whoever creates it. Default privileges are what keeps the owner - // in reach of what the other roles create from here on — which only - // means something for a schema, the thing objects are created in. - for other in other_pg_roles - .iter() - .filter(|_| matches!(target, AclTarget::Schema { .. })) - { - for plural in ["TABLES", "SEQUENCES", "FUNCTIONS"] { - statements.push(format!( - "ALTER DEFAULT PRIVILEGES FOR ROLE {} IN SCHEMA {} GRANT ALL PRIVILEGES ON {} TO {}", - quote_ident(other), - schema, - plural, - role - )); - } - } - if existing_objects.is_empty() && matches!(target, AclTarget::Schema { .. }) { - warnings.push(format!( - "{} holds no objects yet; only the schema itself changes hands.", - target.label(dbname) - )); - } - } - AclChange::Grant { privileges, scope, .. } - | AclChange::Revoke { privileges, scope, .. } => { - let revoking = matches!(change, AclChange::Revoke { .. }); - let objects: &[AclObject] = match change { - AclChange::Revoke { objects, .. } => objects, - _ => &[], - }; - // Every object of one revoke is the same kind of thing, so the first - // decides which privileges are legal for all of them. - let object = objects.first(); - // A named object decides which privileges are legal, not the scope: - // `ON ALL TABLES` grants read back per object and revoke per object. - let allowed = match object { - Some(o) => match object_keyword(&o.kind)? { - "SEQUENCE" => SEQUENCE_PRIVILEGES, - "FUNCTION" => FUNCTION_PRIVILEGES, - _ => TABLE_PRIVILEGES, - }, - None => scope.allowed_privileges(target)?, - }; - let privileges = validate_privileges(privileges, allowed)?; - let privileges = privileges.join(", "); - let statement = match (scope.is_future(), scope.object_plural()) { - (true, Some(plural)) => { - // Default privileges are recorded per creating role, so a - // rule has to be written for each of them. - let mut creators = other_pg_roles.to_vec(); - creators.push(pg_role.to_string()); - creators.sort(); - creators.dedup(); - for creator in creators { - statements.push(if revoking { - format!( - "ALTER DEFAULT PRIVILEGES FOR ROLE {} IN SCHEMA {} REVOKE {} ON {} FROM {}", - quote_ident(&creator), schema, privileges, plural, role - ) - } else { - format!( - "ALTER DEFAULT PRIVILEGES FOR ROLE {} IN SCHEMA {} GRANT {} ON {} TO {}", - quote_ident(&creator), schema, privileges, plural, role - ) - }); - } - None - } - (false, Some(plural)) => Some(if revoking { - format!( - "REVOKE {} ON ALL {} IN SCHEMA {} FROM {}", - privileges, plural, schema, role - ) - } else { - format!( - "GRANT {} ON ALL {} IN SCHEMA {} TO {}", - privileges, plural, schema, role - ) - }), - (_, None) if !objects.is_empty() => { - for object in objects { - statements.push(format!( - "REVOKE {} ON {} {} FROM {}", - privileges, - object_keyword(&object.kind)?, - object_ref(&schema, &object.name, object.args.as_deref()), - role - )); - } - None - } - (_, None) => Some(if revoking { - format!( - "REVOKE {} ON {} FROM {}", - privileges, - target.object(dbname), - role - ) - } else { - format!( - "GRANT {} ON {} TO {}", - privileges, - target.object(dbname), - role - ) - }), - }; - if let Some(statement) = statement { - statements.push(statement); - } - if !revoking && matches!(scope, GrantScope::AllTables | GrantScope::FutureTables) { - warnings.push( - "Reaching a table also needs USAGE on the schema it lives in.".to_string(), - ); - } - } - } - - Ok(AclPlan { statements, warnings }) -} - -/// The keyword a `REVOKE ... ON` takes for one object, checked rather than -/// interpolated: it lands in SQL unquoted. -fn object_keyword(kind: &str) -> Result<&'static str> { - match kind.to_uppercase().as_str() { - "TABLE" | "VIEW" | "MATERIALIZED VIEW" | "FOREIGN TABLE" => Ok("TABLE"), - "SEQUENCE" => Ok("SEQUENCE"), - "FUNCTION" => Ok("FUNCTION"), - other => Err(Error::BadRequest(format!("Unknown object kind '{other}'"))), - } -} - /// An object whose ownership follows the schema's. #[derive(Debug, PartialEq)] -struct OwnedObject { - name: String, +pub(crate) struct OwnedObject { + pub(crate) name: String, /// The keyword `ALTER ... OWNER TO` takes for this kind of object. - keyword: &'static str, + pub(crate) keyword: &'static str, /// Identity arguments of a routine, which is what tells two of the same /// name apart. `None` for a relation. - args: Option, -} - -/// `"schema"."name"` — with `(args)` for a routine, which is not identified -/// without them. `quoted_schema` comes quoted already, being the same for a -/// whole plan. -fn object_ref(quoted_schema: &str, name: &str, args: Option<&str>) -> String { - format!( - "{}.{}{}", - quoted_schema, - quote_ident(name), - args.map(|a| format!("({a})")).unwrap_or_default() - ) + pub(crate) args: Option, } fn keyword_of_relkind(relkind: i8) -> Option<&'static str> { @@ -981,6 +704,7 @@ async fn build_acl_plan( datatable_name: &str, req: &AclChangeRequest, ) -> Result<(tokio_postgres::Client, AclPlan, String)> { + crate::datatable_permissions::require_datatable_permissions_license().await?; let (client, conn) = connect_as_caller(db, authed, w_id, datatable_name, req.role.as_deref()).await?; // What the caller's own role may change. Postgres cannot enforce the rule we @@ -1016,7 +740,7 @@ async fn build_acl_plan( } _ => vec![], }; - let plan = plan_statements( + let plan = crate::datatable_acl_oss::plan_statements( &req.target, &req.change, &conn.dbname, @@ -1090,337 +814,3 @@ async fn apply_datatable_acl( Ok(format!("Updated access on {}", req.target.label(&dbname))) } - -#[cfg(test)] -mod tests { - use super::*; - - fn schema() -> AclTarget { - AclTarget::Schema { schema: "analytics".to_string() } - } - - #[test] - fn set_owner_covers_the_schema_and_what_is_in_it() { - let objects = vec![ - OwnedObject { name: "orders".to_string(), keyword: "TABLE", args: None }, - OwnedObject { name: "orders_id_seq".to_string(), keyword: "SEQUENCE", args: None }, - OwnedObject { - name: "total".to_string(), - keyword: "ROUTINE", - args: Some("integer, text".to_string()), - }, - ]; - let plan = plan_statements( - &schema(), - &AclChange::SetOwner { role: "analyst".to_string() }, - "dt_probe", - "wm_analyst_1", - &["wm_admin".to_string()], - &objects, - ) - .unwrap(); - assert_eq!( - plan.statements[..4], - [ - r#"ALTER SCHEMA "analytics" OWNER TO "wm_analyst_1""#.to_string(), - r#"ALTER TABLE "analytics"."orders" OWNER TO "wm_analyst_1""#.to_string(), - r#"ALTER SEQUENCE "analytics"."orders_id_seq" OWNER TO "wm_analyst_1""#.to_string(), - // A routine is only named by its arguments. - r#"ALTER ROUTINE "analytics"."total"(integer, text) OWNER TO "wm_analyst_1""# - .to_string(), - ] - ); - // What the other roles create later stays within the owner's reach. - assert!(plan.statements.iter().any(|s| s - == r#"ALTER DEFAULT PRIVILEGES FOR ROLE "wm_admin" IN SCHEMA "analytics" GRANT ALL PRIVILEGES ON TABLES TO "wm_analyst_1""#)); - assert!(plan.warnings.is_empty()); - } - - #[test] - fn an_empty_schema_says_so() { - let plan = plan_statements( - &schema(), - &AclChange::SetOwner { role: "analyst".to_string() }, - "dt_probe", - "wm_analyst_1", - &[], - &[], - ) - .unwrap(); - assert_eq!(plan.statements.len(), 1); - assert_eq!(plan.warnings.len(), 1); - } - - #[test] - fn grants_render_the_scope_they_name() { - let cases = [ - ( - GrantScope::Target, - vec!["USAGE".to_string()], - r#"GRANT USAGE ON SCHEMA "analytics" TO "wm_analyst_1""#, - ), - ( - GrantScope::AllTables, - vec!["SELECT".to_string(), "INSERT".to_string()], - r#"GRANT SELECT, INSERT ON ALL TABLES IN SCHEMA "analytics" TO "wm_analyst_1""#, - ), - ( - GrantScope::AllSequences, - vec!["USAGE".to_string()], - r#"GRANT USAGE ON ALL SEQUENCES IN SCHEMA "analytics" TO "wm_analyst_1""#, - ), - ]; - for (scope, privileges, expected) in cases { - let plan = plan_statements( - &schema(), - &AclChange::Grant { role: "analyst".to_string(), privileges, scope }, - "dt_probe", - "wm_analyst_1", - &[], - &[], - ) - .unwrap(); - assert_eq!(plan.statements[0], expected); - } - } - - #[test] - fn future_grants_are_written_for_every_creating_role() { - let plan = plan_statements( - &schema(), - &AclChange::Grant { - role: "analyst".to_string(), - privileges: vec!["SELECT".to_string()], - scope: GrantScope::FutureTables, - }, - "dt_probe", - "wm_analyst_1", - &["wm_admin".to_string()], - &[], - ) - .unwrap(); - assert_eq!( - plan.statements, - [ - r#"ALTER DEFAULT PRIVILEGES FOR ROLE "wm_admin" IN SCHEMA "analytics" GRANT SELECT ON TABLES TO "wm_analyst_1""#, - r#"ALTER DEFAULT PRIVILEGES FOR ROLE "wm_analyst_1" IN SCHEMA "analytics" GRANT SELECT ON TABLES TO "wm_analyst_1""#, - ] - ); - } - - #[test] - fn revoke_mirrors_grant() { - let plan = plan_statements( - &schema(), - &AclChange::Revoke { - role: "analyst".to_string(), - privileges: vec!["select".to_string()], - scope: GrantScope::AllTables, - objects: vec![], - }, - "dt_probe", - "wm_analyst_1", - &[], - &[], - ) - .unwrap(); - assert_eq!( - plan.statements, - [r#"REVOKE SELECT ON ALL TABLES IN SCHEMA "analytics" FROM "wm_analyst_1""#] - ); - } - - #[test] - fn revoking_one_object_names_it() { - let plan = plan_statements( - &schema(), - &AclChange::Revoke { - role: "analyst".to_string(), - privileges: vec!["SELECT".to_string()], - scope: GrantScope::Target, - objects: vec![AclObject { - name: "orders".to_string(), - kind: "TABLE".to_string(), - args: None, - }], - }, - "dt_probe", - "wm_analyst_1", - &[], - &[], - ); - // SELECT is no schema privilege, but the object named is a table: what - // it is decides which privileges are legal. - assert_eq!( - plan.unwrap().statements, - [r#"REVOKE SELECT ON TABLE "analytics"."orders" FROM "wm_analyst_1""#] - ); - } - - #[test] - fn a_database_grants_the_right_to_create_schemas() { - let plan = plan_statements( - &AclTarget::Database, - &AclChange::Grant { - role: "analyst".to_string(), - privileges: vec!["CREATE".to_string()], - scope: GrantScope::Target, - }, - "dt_probe", - "wm_analyst_1", - &[], - &[], - ) - .unwrap(); - assert_eq!( - plan.statements, - [r#"GRANT CREATE ON DATABASE "dt_probe" TO "wm_analyst_1""#] - ); - - // A database has no schemas to scope onto, and none of its privileges - // are a table's. - for change in [ - AclChange::Grant { - role: "analyst".to_string(), - privileges: vec!["CREATE".to_string()], - scope: GrantScope::AllTables, - }, - AclChange::Grant { - role: "analyst".to_string(), - privileges: vec!["SELECT".to_string()], - scope: GrantScope::Target, - }, - ] { - assert!(matches!( - plan_statements( - &AclTarget::Database, - &change, - "dt_probe", - "wm_analyst_1", - &[], - &[] - ) - .unwrap_err(), - Error::BadRequest(_) - )); - } - } - - #[test] - fn a_tables_owner_change_is_only_that_table() { - let plan = plan_statements( - &AclTarget::Table { schema: "analytics".to_string(), table: "orders".to_string() }, - &AclChange::SetOwner { role: "analyst".to_string() }, - "dt_probe", - "wm_analyst_1", - &["wm_admin".to_string()], - &[], - ) - .unwrap(); - // Default privileges are about what gets created in a schema, which - // changing one table's owner says nothing about. - assert_eq!( - plan.statements, - [r#"ALTER TABLE "analytics"."orders" OWNER TO "wm_analyst_1""#] - ); - assert!(plan.warnings.is_empty()); - } - - #[test] - fn several_objects_are_revoked_together() { - let plan = plan_statements( - &schema(), - &AclChange::Revoke { - role: "analyst".to_string(), - privileges: vec!["SELECT".to_string()], - scope: GrantScope::Target, - objects: vec![ - AclObject { name: "a".to_string(), kind: "TABLE".to_string(), args: None }, - AclObject { name: "b".to_string(), kind: "TABLE".to_string(), args: None }, - ], - }, - "dt_probe", - "wm_analyst_1", - &[], - &[], - ) - .unwrap(); - assert_eq!( - plan.statements, - [ - r#"REVOKE SELECT ON TABLE "analytics"."a" FROM "wm_analyst_1""#, - r#"REVOKE SELECT ON TABLE "analytics"."b" FROM "wm_analyst_1""#, - ] - ); - } - - #[test] - fn a_privilege_the_object_does_not_have_is_refused() { - for (scope, privilege) in [ - (GrantScope::Target, "SELECT"), - (GrantScope::AllTables, "CREATE"), - (GrantScope::AllFunctions, "SELECT"), - (GrantScope::AllTables, "SELECT; DROP TABLE x"), - ] { - let err = plan_statements( - &schema(), - &AclChange::Grant { - role: "analyst".to_string(), - privileges: vec![privilege.to_string()], - scope, - }, - "dt_probe", - "wm_analyst_1", - &[], - &[], - ) - .unwrap_err(); - assert!( - matches!(err, Error::BadRequest(_)), - "{privilege} on {scope:?}" - ); - } - } - - #[test] - fn identifiers_are_quoted() { - let plan = plan_statements( - &AclTarget::Schema { schema: "we\"ird".to_string() }, - &AclChange::Grant { - role: "analyst".to_string(), - privileges: vec!["USAGE".to_string()], - scope: GrantScope::Target, - }, - "dt_probe", - "ro\"le", - &[], - &[], - ) - .unwrap(); - assert_eq!( - plan.statements, - [r#"GRANT USAGE ON SCHEMA "we""ird" TO "ro""le""#] - ); - } - - #[test] - fn a_table_target_names_the_table() { - let plan = plan_statements( - &AclTarget::Table { schema: "analytics".to_string(), table: "orders".to_string() }, - &AclChange::Grant { - role: "analyst".to_string(), - privileges: vec!["SELECT".to_string()], - scope: GrantScope::Target, - }, - "dt_probe", - "wm_analyst_1", - &[], - &[], - ) - .unwrap(); - assert_eq!( - plan.statements, - [r#"GRANT SELECT ON TABLE "analytics"."orders" TO "wm_analyst_1""#] - ); - } -} diff --git a/backend/windmill-api-workspaces/src/datatable_acl_oss.rs b/backend/windmill-api-workspaces/src/datatable_acl_oss.rs new file mode 100644 index 0000000000..c790507e3d --- /dev/null +++ b/backend/windmill-api-workspaces/src/datatable_acl_oss.rs @@ -0,0 +1,37 @@ +/* + * 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. + */ + +//! Where the ACL planner comes from: the enterprise one, or a refusal. +//! +//! Reading who owns what stays open; every owner change and every grant is the +//! plan this returns, so an edition without the enterprise module cannot make +//! one. + +#[cfg(feature = "private")] +#[allow(unused)] +pub(crate) use crate::datatable_acl_ee::*; + +#[cfg(not(feature = "private"))] +use { + crate::datatable_acl::{AclChange, AclPlan, AclTarget, OwnedObject}, + windmill_common::error::{Error, Result}, +}; + +#[cfg(not(feature = "private"))] +pub(crate) fn plan_statements( + _target: &AclTarget, + _change: &AclChange, + _dbname: &str, + _pg_role: &str, + _other_pg_roles: &[String], + _existing_objects: &[OwnedObject], +) -> Result { + Err(Error::BadRequest( + "Data table permissions are a Windmill Enterprise Edition feature".to_string(), + )) +} diff --git a/backend/windmill-api-workspaces/src/datatable_permissions.rs b/backend/windmill-api-workspaces/src/datatable_permissions.rs index a82bdac48a..021c9dcc60 100644 --- a/backend/windmill-api-workspaces/src/datatable_permissions.rs +++ b/backend/windmill-api-workspaces/src/datatable_permissions.rs @@ -30,11 +30,10 @@ use windmill_audit::ActionKind; use windmill_common::ensure_instance_db_grant_options; use windmill_common::error::{pg_error_message, Error, JsonResult, Result}; use windmill_common::query_builders::{render_db_quoted_identifier, DbType}; -use windmill_common::utils::{rd_string, require_admin}; +use windmill_common::utils::require_admin; use windmill_common::workspaces::{ - can_use_datatable_role, datatable_pg_role_name, get_datatable_resource_from_db_unchecked, - DataTable, DataTableCatalogResourceType, DataTablePermissions, DataTableRole, - ADMIN_DATATABLE_ROLE, DATATABLE_TENANT_WILDCARD, + can_use_datatable_role, get_datatable_resource_from_db_unchecked, DataTable, + DataTableCatalogResourceType, DataTablePermissions, ADMIN_DATATABLE_ROLE, }; use windmill_common::{PgDatabase, DB}; @@ -114,491 +113,47 @@ pub struct DatatablePermissionsPreview { /// One planned statement. `display` is what the preview shows: identical to /// `sql` except where a generated password would otherwise be printed. #[derive(Debug)] -struct PlannedStatement { - sql: String, - display: String, +pub(crate) struct PlannedStatement { + pub(crate) sql: String, + pub(crate) display: String, } impl PlannedStatement { - fn plain(sql: String) -> Self { + /// Only the planner builds these, and that is the enterprise module. + #[cfg_attr(not(feature = "private"), allow(dead_code))] + pub(crate) fn plain(sql: String) -> Self { Self { display: sql.clone(), sql } } } #[derive(Debug)] -struct RolePlan { - statements: Vec, +pub(crate) struct RolePlan { + pub(crate) statements: Vec, /// The permissions block to persist once the statements have run. - permissions: DataTablePermissions, - warnings: Vec, + pub(crate) permissions: DataTablePermissions, + pub(crate) warnings: Vec, } -pub(crate) fn quote_ident(ident: &str) -> String { - render_db_quoted_identifier(ident, DbType::Postgresql) -} - -fn quote_literal(value: &str) -> String { - format!("'{}'", value.replace('\'', "''")) -} - -fn validate_role_name(name: &str) -> Result<()> { - if name.is_empty() - || name.len() > 63 - || !name - .chars() - .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-') - { - return Err(Error::BadRequest(format!( - "Invalid role name '{name}': must be 1-63 characters of letters, digits, '_' or '-'" - ))); +/// Refuse to plan a change on a binary that carries the enterprise modules but +/// no active plan. Without them there is no planner at all, so this is the whole +/// gate on an enterprise build. +pub(crate) async fn require_datatable_permissions_license() -> Result<()> { + #[cfg(feature = "enterprise")] + if !matches!( + windmill_common::ee_oss::get_license_plan().await, + windmill_common::ee_oss::LicensePlan::Enterprise + ) { + return Err(Error::BadRequest( + "Data table permissions require an Enterprise license".to_string(), + )); } Ok(()) } -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/ or f/" - ))), - } -} - -/// Plan the SQL that takes the data table's Postgres roles from `old` to `req`. -/// -/// `existing_pg_roles` is the set of role names that actually exist in the -/// cluster, so the plan reconciles against reality rather than against what the -/// config claims: a role the config lost track of is still dropped, and one that -/// somehow already exists has its password reset instead of failing the CREATE. -fn plan_role_changes( - w_id: &str, - datatable: &str, - dbname: &str, - admin_pg_role: &str, - old: Option<&DataTablePermissions>, - req: &SetDatatablePermissions, - existing_pg_roles: &HashSet, - public_schema_is_open: bool, - default_acl_rules: &[DefaultAclRule], -) -> Result { - // A disabled data table has no Postgres roles, whatever its config says, so - // re-enabling always plans every role as a creation. - let old_roles: BTreeMap = match old { - Some(p) if p.enabled => p.roles.clone(), - _ => BTreeMap::new(), - }; - - let mut statements = Vec::new(); - let mut warnings = Vec::new(); - - let mut dropped_pg_roles: HashSet = HashSet::new(); - let drop_role = - |statements: &mut Vec, dropped: &mut HashSet, pg_role: &str| { - if !existing_pg_roles.contains(pg_role) { - return; - } - dropped.insert(pg_role.to_string()); - let q = quote_ident(pg_role); - // Repeated here rather than assumed: a role created before this grant - // existed would otherwise be impossible to drop without a superuser. - statements.push(grant_role_to_admin_statement(pg_role, admin_pg_role)); - // Give the objects back to admin before dropping, else the DROP fails on - // anything the role still owns. DROP OWNED then clears what is left: - // privileges granted to it and its default-privilege entries. - statements.push(PlannedStatement::plain(format!( - "REASSIGN OWNED BY {q} TO {};", - quote_ident(admin_pg_role) - ))); - statements.push(PlannedStatement::plain(format!("DROP OWNED BY {q};"))); - statements.push(PlannedStatement::plain(format!("DROP ROLE {q};"))); - }; - - if !req.enabled { - for (name, role) in old_roles.iter() { - if name == ADMIN_DATATABLE_ROLE { - continue; - } - if let Some(pg_role) = role.pg_rolename.as_deref() { - drop_role(&mut statements, &mut dropped_pg_roles, pg_role); - } - } - // Opting out drops the roles, so keeping their definitions would leave - // the config describing roles that no longer exist. - return Ok(RolePlan { - statements, - permissions: DataTablePermissions { - enabled: false, - roles: BTreeMap::new(), - default_role: None, - }, - warnings, - }); - } - - let mut requested: BTreeMap> = BTreeMap::new(); - for role in req.roles.iter() { - validate_role_name(&role.name)?; - for tenant in role.tenants.iter() { - validate_tenant(tenant)?; - } - if requested - .insert(role.name.clone(), role.tenants.clone()) - .is_some() - { - return Err(Error::BadRequest(format!( - "Duplicate role name '{}'", - role.name - ))); - } - } - if !requested.contains_key(ADMIN_DATATABLE_ROLE) { - return Err(Error::BadRequest(format!( - "The '{ADMIN_DATATABLE_ROLE}' role cannot be removed" - ))); - } - - // A default naming a role that is not being saved would leave every script - // that names no role failing to resolve. - let default_role = req.default_role.as_deref().unwrap_or(ADMIN_DATATABLE_ROLE); - if !requested.contains_key(default_role) { - return Err(Error::BadRequest(format!( - "Default role '{default_role}' is not one of the submitted roles" - ))); - } - - // new name -> old name, so a renamed role keeps its Postgres role (and the - // grants on it) instead of being planned as a drop plus a create. - let mut rename_src: BTreeMap<&str, &str> = BTreeMap::new(); - for r in req.renames.iter() { - validate_role_name(&r.from)?; - validate_role_name(&r.to)?; - if r.from == ADMIN_DATATABLE_ROLE || r.to == ADMIN_DATATABLE_ROLE { - return Err(Error::BadRequest(format!( - "The '{ADMIN_DATATABLE_ROLE}' role cannot be renamed" - ))); - } - if r.from == r.to { - continue; - } - if !old_roles.contains_key(&r.from) { - return Err(Error::BadRequest(format!( - "Cannot rename unknown role '{}'", - r.from - ))); - } - if !requested.contains_key(&r.to) { - return Err(Error::BadRequest(format!( - "Renamed role '{}' is missing from the submitted roles", - r.to - ))); - } - if rename_src.insert(&r.to, &r.from).is_some() { - return Err(Error::BadRequest(format!( - "Two roles were renamed to '{}'", - r.to - ))); - } - } - - let renamed_away: HashSet<&str> = rename_src.values().copied().collect(); - - // Which old role each requested role continues: its rename source if it was - // renamed, else the old role of the same name. Matching on the requested name - // alone would spare an old role that a *different* role is being renamed onto, - // leaving the name occupied when the rename runs. - let claimed: HashSet<&str> = requested - .keys() - .map(|name| { - rename_src - .get(name.as_str()) - .copied() - .unwrap_or(name.as_str()) - }) - .collect(); - - // Drops come first so a name freed in this save can be reused by a rename or - // a create in the same save: the other order aborts on "role already exists". - for (name, role) in old_roles.iter() { - if name == ADMIN_DATATABLE_ROLE || claimed.contains(name.as_str()) { - continue; - } - if let Some(pg_role) = role.pg_rolename.as_deref() { - drop_role(&mut statements, &mut dropped_pg_roles, pg_role); - } - } - - let mut roles: BTreeMap = BTreeMap::new(); - // Renames run before creates so a name freed by a rename can be taken by a new - // role in the same save; both run after the drops, for the same reason. - // Postgres names this save frees, by dropping the role or renaming it away. - // A name in here is occupied now but free by the time the creates run, so a new - // role may take it — treating it as taken would silently reuse the old role - // (and reset its password) instead of creating the new one. - let vacated_pg_roles: HashSet = dropped_pg_roles - .iter() - .cloned() - .chain(rename_src.iter().filter_map(|(to, from)| { - let old_pg = old_roles.get(*from)?.pg_rolename.clone()?; - let new_pg = datatable_pg_role_name(w_id, datatable, to); - (old_pg != new_pg).then_some(old_pg) - })) - .collect(); - - let mut pending_renames: Vec<(String, String, String)> = Vec::new(); - let mut creates_sql: Vec = Vec::new(); - - for (name, tenants) in requested.iter() { - if name == ADMIN_DATATABLE_ROLE { - roles.insert( - name.clone(), - DataTableRole { pg_rolename: None, pg_password: None, tenants: tenants.clone() }, - ); - continue; - } - - let pg_rolename = datatable_pg_role_name(w_id, datatable, name); - let previous = rename_src - .get(name.as_str()) - .and_then(|from| old_roles.get(*from).map(|r| (*from, r))) - .or_else(|| { - // An old role of the same name that is being renamed away is not - // this role's predecessor: the name is being freed and taken by a - // brand new role, which has to be created rather than inherited. - (!renamed_away.contains(name.as_str())) - .then(|| old_roles.get(name).map(|r| (name.as_str(), r))) - .flatten() - }); - - match previous { - Some((from, old_role)) if old_role.pg_rolename.is_some() => { - let old_pg = old_role.pg_rolename.clone().unwrap(); - let password = old_role - .pg_password - .clone() - .unwrap_or_else(|| rd_string(32)); - if old_pg != pg_rolename { - if existing_pg_roles.contains(&old_pg) { - pending_renames.push(( - old_pg.clone(), - pg_rolename.clone(), - password.clone(), - )); - } else { - warnings.push(format!( - "Role '{from}' was expected to exist in the database as '{old_pg}' but does not; it will be created as '{pg_rolename}'." - )); - creates_sql.push(create_role_statement(&pg_rolename, &password)); - creates_sql - .push(grant_role_to_admin_statement(&pg_rolename, admin_pg_role)); - creates_sql.push(revoke_public_create_statement(&pg_rolename)); - creates_sql.push(grant_connect_statement(&pg_rolename, dbname)); - creates_sql.extend(replay_default_acl_statements( - &pg_rolename, - default_acl_rules, - )); - } - } - roles.insert( - name.clone(), - DataTableRole { - pg_rolename: Some(pg_rolename), - pg_password: Some(password), - tenants: tenants.clone(), - }, - ); - } - _ => { - let password = rd_string(32); - if existing_pg_roles.contains(&pg_rolename) - && !vacated_pg_roles.contains(&pg_rolename) - { - warnings.push(format!( - "A Postgres role named '{pg_rolename}' already exists; it will be reused and its password reset." - )); - creates_sql.push(PlannedStatement { - sql: format!( - "ALTER ROLE {} WITH LOGIN PASSWORD {};", - quote_ident(&pg_rolename), - quote_literal(&password) - ), - display: format!( - "ALTER ROLE {} WITH LOGIN PASSWORD '';", - quote_ident(&pg_rolename) - ), - }); - } else { - creates_sql.push(create_role_statement(&pg_rolename, &password)); - } - creates_sql.push(grant_role_to_admin_statement(&pg_rolename, admin_pg_role)); - creates_sql.push(revoke_public_create_statement(&pg_rolename)); - creates_sql.push(grant_connect_statement(&pg_rolename, dbname)); - creates_sql.extend(replay_default_acl_statements( - &pg_rolename, - default_acl_rules, - )); - roles.insert( - name.clone(), - DataTableRole { - pg_rolename: Some(pg_rolename), - pg_password: Some(password), - tenants: tenants.clone(), - }, - ); - } - } - } - - // A role that can still create objects in `public` defeats "created bare". - // Only the schema's owner or a superuser can close that off, and the data - // table's own role is usually neither — Postgres silently ignores the REVOKE - // otherwise — so say it rather than emit a statement that would do nothing. - if public_schema_is_open && !creates_sql.is_empty() { - warnings.push( - "CREATE on schema `public` is granted to PUBLIC in this database, so new roles can \ - still create objects there — Postgres cannot deny it per role. To close it, a \ - superuser or the schema's owner must run: REVOKE CREATE ON SCHEMA public FROM PUBLIC;" - .to_string(), - ); - } - - statements.extend(order_renames( - pending_renames, - existing_pg_roles, - &dropped_pg_roles, - )?); - statements.extend(creates_sql); - - Ok(RolePlan { - statements, - permissions: DataTablePermissions { - enabled: true, - roles, - default_role: (default_role != ADMIN_DATATABLE_ROLE).then(|| default_role.to_string()), - }, - warnings, - }) -} - -/// Order the planned renames so each runs only once its target name is free, and -/// emit them. -/// -/// A rename whose target is still occupied aborts the whole transaction with a -/// bare "role already exists", so the ordering is part of the plan rather than -/// something the admin has to discover: `a -> b` where the old `b` is being -/// dropped or itself renamed away is a legitimate save, it just has to run in the -/// right order. A true cycle (swapping two names) cannot be ordered at all and is -/// reported as such instead of failing in Postgres. -fn order_renames( - renames: Vec<(String, String, String)>, - existing_pg_roles: &HashSet, - dropped_pg_roles: &HashSet, -) -> Result> { - let mut pending = renames; - let mut vacated: HashSet = dropped_pg_roles.clone(); - let mut statements = Vec::new(); - - while !pending.is_empty() { - let ready = pending - .iter() - .position(|(_, to, _)| !existing_pg_roles.contains(to) || vacated.contains(to)); - let Some(i) = ready else { - let blocked: Vec<&str> = pending.iter().map(|(from, _, _)| from.as_str()).collect(); - return Err(Error::BadRequest(format!( - "Renaming {} would swap Postgres role names, which cannot be done in one \ - transaction. Apply the renames in two separate saves.", - blocked.join(", ") - ))); - }; - let (from, to, password) = pending.remove(i); - statements.push(PlannedStatement::plain(format!( - "ALTER ROLE {} RENAME TO {};", - quote_ident(&from), - quote_ident(&to) - ))); - // RENAME discards an md5-hashed password, so the stored one would stop - // working; re-setting it is a no-op under scram-sha-256 and a repair under - // md5. - statements.push(PlannedStatement { - sql: format!( - "ALTER ROLE {} PASSWORD {};", - quote_ident(&to), - quote_literal(&password) - ), - display: format!("ALTER ROLE {} PASSWORD '';", quote_ident(&to)), - }); - vacated.insert(from); - } - Ok(statements) -} - -/// Give `admin` the privileges of a role it created. -/// -/// Postgres does not do this implicitly: a CREATEROLE non-superuser that creates a -/// role gets ADMIN OPTION on it but neither INHERIT nor SET, so -/// `has_privs_of_role` stays false. Without this grant, `REASSIGN OWNED BY -/// TO ` is refused ("Only roles with privileges of role X may reassign -/// objects owned by it") and the role can then never be dropped — wedging opt-out -/// on exactly the data tables whose admin is `custom_instance_user` rather than a -/// superuser. A plain GRANT is used rather than `WITH INHERIT TRUE` so the -/// statement also parses on Postgres before 16. -/// Keep a created role out of the `public` schema. -/// -/// Roles are created bare and privileges are added additively, which has to -/// include not being able to make objects. This only strips a *direct* grant — -/// when CREATE is held by `PUBLIC` the role still has it, and Postgres has no -/// per-role deny, so the plan warns instead (see `public_schema_warning`). -fn revoke_public_create_statement(pg_rolename: &str) -> PlannedStatement { - PlannedStatement::plain(format!( - "REVOKE CREATE ON SCHEMA public FROM {};", - quote_ident(pg_rolename) - )) -} - -fn grant_role_to_admin_statement(pg_rolename: &str, admin_pg_role: &str) -> PlannedStatement { - // `WITH SET TRUE` explicitly: reassigning an object to a role requires being - // able to SET ROLE to it, and a membership without that option — which is - // what Postgres 16 records for a CREATEROLE grantor's implicit self-grant — - // leaves admin unable to hand anything to the roles it created. - PlannedStatement::plain(format!( - "GRANT {} TO {} WITH SET TRUE;", - quote_ident(pg_rolename), - quote_ident(admin_pg_role) - )) -} - -fn create_role_statement(pg_rolename: &str, password: &str) -> PlannedStatement { - PlannedStatement { - sql: format!( - "CREATE ROLE {} LOGIN PASSWORD {};", - quote_ident(pg_rolename), - quote_literal(password) - ), - display: format!( - "CREATE ROLE {} LOGIN PASSWORD '';", - quote_ident(pg_rolename) - ), - } -} - -/// New roles are created bare — privileges are granted additively afterwards — -/// but they do need to reach the database. CONNECT is usually already theirs -/// through PUBLIC, and the data table's own role often cannot grant it (it is -/// rarely the database owner), so the grant is conditional rather than -/// unconditional: it stays a no-op in the common case instead of failing the -/// whole transaction. -fn grant_connect_statement(pg_rolename: &str, dbname: &str) -> PlannedStatement { - let role_lit = quote_literal(pg_rolename); - let db_lit = quote_literal(dbname); - // The names are passed to `format(%I)` rather than interpolated as quoted - // identifiers: this identifier sits inside a single-quoted EXECUTE string, so - // a `'` in the database name — which comes from a user-editable postgres - // resource — would otherwise close the literal and run as plpgsql of its own. - PlannedStatement::plain(format!( - "DO $$ BEGIN\n IF NOT has_database_privilege({role_lit}, {db_lit}, 'CONNECT') THEN\n EXECUTE format('GRANT CONNECT ON DATABASE %I TO %I', {db_lit}, {role_lit});\n END IF;\nEND $$;" - )) +/// Only the planners quote identifiers, and those are the enterprise modules. +#[cfg_attr(not(feature = "private"), allow(dead_code))] +pub(crate) fn quote_ident(ident: &str) -> String { + render_db_quoted_identifier(ident, DbType::Postgresql) } pub(crate) async fn read_datatable(db: &DB, w_id: &str, datatable_name: &str) -> Result { @@ -719,36 +274,6 @@ async fn read_default_acl_rules(client: &tokio_postgres::Client) -> Result Vec { - rules - .iter() - .filter(|rule| rule.grantee != pg_rolename && !rule.privileges.is_empty()) - .map(|rule| { - let grantee = if rule.grantee == "PUBLIC" { - "PUBLIC".to_string() - } else { - quote_ident(&rule.grantee) - }; - PlannedStatement::plain(format!( - "ALTER DEFAULT PRIVILEGES FOR ROLE {}{} GRANT {} ON {} TO {};", - quote_ident(pg_rolename), - rule.schema - .as_ref() - .map(|s| format!(" IN SCHEMA {}", quote_ident(s))) - .unwrap_or_default(), - rule.privileges.join(", "), - rule.objects, - grantee - )) - }) - .collect() -} - pub(crate) async fn connect_as_admin( db: &DB, w_id: &str, @@ -833,9 +358,10 @@ async fn build_plan( datatable_name: &str, req: &SetDatatablePermissions, ) -> Result<(tokio_postgres::Client, RolePlan)> { + require_datatable_permissions_license().await?; let datatable = read_datatable(db, w_id, datatable_name).await?; let (client, conn) = connect_as_admin(db, w_id, datatable_name).await?; - let plan = plan_role_changes( + let plan = crate::datatable_permissions_oss::plan_role_changes( w_id, datatable_name, &conn.dbname, @@ -1079,465 +605,3 @@ async fn set_datatable_permissions( "Updated permissions of data table {datatable_name}" )) } - -#[cfg(test)] -mod tests { - use super::*; - - const W_ID: &str = "acme"; - const DT: &str = "main"; - const DB_NAME: &str = "wm_acme_main"; - const ADMIN_PG: &str = "custom_instance_user"; - - fn role(name: &str, tenants: &[&str]) -> DatatableRoleInfo { - DatatableRoleInfo { - name: name.to_string(), - tenants: tenants.iter().map(|t| t.to_string()).collect(), - pg_rolename: None, - } - } - - fn enabled_with(roles: &[&str]) -> DataTablePermissions { - let mut map = BTreeMap::new(); - map.insert(ADMIN_DATATABLE_ROLE.to_string(), DataTableRole::default()); - for name in roles { - map.insert( - name.to_string(), - DataTableRole { - pg_rolename: Some(datatable_pg_role_name(W_ID, DT, name)), - pg_password: Some("kept-password".to_string()), - tenants: vec![], - }, - ); - } - DataTablePermissions { enabled: true, roles: map, default_role: None } - } - - fn plan( - old: Option<&DataTablePermissions>, - req: &SetDatatablePermissions, - existing: &[&str], - ) -> Result { - plan_with_public(old, req, existing, false) - } - - fn plan_with_public( - old: Option<&DataTablePermissions>, - req: &SetDatatablePermissions, - existing: &[&str], - public_schema_is_open: bool, - ) -> Result { - plan_with_default_acls(old, req, existing, public_schema_is_open, &[]) - } - - fn plan_with_default_acls( - old: Option<&DataTablePermissions>, - req: &SetDatatablePermissions, - existing: &[&str], - public_schema_is_open: bool, - default_acl_rules: &[DefaultAclRule], - ) -> Result { - plan_role_changes( - W_ID, - DT, - DB_NAME, - ADMIN_PG, - old, - req, - &existing.iter().map(|r| r.to_string()).collect(), - public_schema_is_open, - default_acl_rules, - ) - } - - fn sql(plan: &RolePlan) -> Vec<&str> { - plan.statements.iter().map(|s| s.sql.as_str()).collect() - } - - #[test] - fn adding_a_role_creates_it_and_stores_its_credentials() { - let req = SetDatatablePermissions { - enabled: true, - roles: vec![role("admin", &[]), role("analyst", &["u/alice", "g/devs"])], - default_role: None, - renames: vec![], - }; - let plan = plan(None, &req, &[]).unwrap(); - - let pg_role = datatable_pg_role_name(W_ID, DT, "analyst"); - assert!(sql(&plan)[0].starts_with(&format!("CREATE ROLE \"{pg_role}\" LOGIN PASSWORD "))); - // admin must end up with the new role's privileges, or it can never - // reassign its objects and drop it later. - assert_eq!( - sql(&plan)[1], - format!("GRANT \"{pg_role}\" TO \"{ADMIN_PG}\" WITH SET TRUE;") - ); - // Created bare: it must not be able to make objects in `public`. - assert_eq!( - sql(&plan)[2], - format!("REVOKE CREATE ON SCHEMA public FROM \"{pg_role}\";") - ); - assert!(sql(&plan)[3].contains("has_database_privilege")); - - let stored = &plan.permissions.roles["analyst"]; - assert_eq!(stored.pg_rolename.as_deref(), Some(pg_role.as_str())); - assert!(stored.pg_password.as_ref().is_some_and(|p| p.len() == 32)); - assert_eq!(stored.tenants, vec!["u/alice", "g/devs"]); - // admin reuses the data table's own connection, so it never gets one. - assert!(plan.permissions.roles[ADMIN_DATATABLE_ROLE] - .pg_rolename - .is_none()); - } - - /// A default-privilege rule binds only the roles it was written for, so a - /// role added later would create tables no one else can read. - #[test] - fn a_new_role_inherits_the_default_privileges_already_in_force() { - let req = SetDatatablePermissions { - enabled: true, - roles: vec![role("admin", &[]), role("writer", &[])], - default_role: None, - renames: vec![], - }; - let rules = vec![DefaultAclRule { - schema: Some("analytics".to_string()), - objects: "TABLES".to_string(), - grantee: "wm_analyst".to_string(), - privileges: vec!["SELECT".to_string()], - }]; - let plan = plan_with_default_acls(None, &req, &[], false, &rules).unwrap(); - let pg_role = datatable_pg_role_name(W_ID, DT, "writer"); - assert!(sql(&plan).contains(&format!( - "ALTER DEFAULT PRIVILEGES FOR ROLE \"{pg_role}\" IN SCHEMA \"analytics\" GRANT SELECT ON TABLES TO \"wm_analyst\";" - ).as_str())); - } - - /// A role-scoped REVOKE cannot take back what PUBLIC holds, so the plan has - /// to say so instead of pretending the statement closed it. - #[test] - fn an_open_public_schema_is_warned_about_not_silently_revoked() { - let req = SetDatatablePermissions { - enabled: true, - roles: vec![role("admin", &[]), role("analyst", &[])], - default_role: None, - renames: vec![], - }; - - let closed = plan_with_public(None, &req, &[], false).unwrap(); - assert!(closed.warnings.is_empty(), "{:?}", closed.warnings); - - let open = plan_with_public(None, &req, &[], true).unwrap(); - assert!( - open.warnings - .iter() - .any(|w| w.contains("REVOKE CREATE ON SCHEMA public FROM PUBLIC")), - "{:?}", - open.warnings - ); - // The plan still runs the role-scoped revoke, which strips a direct grant. - assert!(sql(&open) - .iter() - .any(|s| s.starts_with("REVOKE CREATE ON SCHEMA public FROM \"wm_"))); - } - - #[test] - fn renaming_a_role_keeps_its_postgres_role_and_password() { - let old = enabled_with(&["analyst"]); - let old_pg = datatable_pg_role_name(W_ID, DT, "analyst"); - let req = SetDatatablePermissions { - enabled: true, - roles: vec![role("admin", &[]), role("reader", &[])], - default_role: None, - renames: vec![DatatableRoleRename { - from: "analyst".to_string(), - to: "reader".to_string(), - }], - }; - let plan = plan(Some(&old), &req, &[old_pg.as_str()]).unwrap(); - - let new_pg = datatable_pg_role_name(W_ID, DT, "reader"); - assert_eq!( - sql(&plan)[0], - format!("ALTER ROLE \"{old_pg}\" RENAME TO \"{new_pg}\";") - ); - // The rename must not be planned as a drop plus a create: that would - // silently discard every grant the role had accumulated. - assert!(!sql(&plan).iter().any(|s| s.contains("DROP ROLE"))); - assert!(!sql(&plan).iter().any(|s| s.contains("CREATE ROLE"))); - assert_eq!( - plan.permissions.roles["reader"].pg_password.as_deref(), - Some("kept-password") - ); - } - - #[test] - fn removing_a_role_gives_its_objects_back_to_admin_before_dropping_it() { - let old = enabled_with(&["analyst"]); - let pg_role = datatable_pg_role_name(W_ID, DT, "analyst"); - let req = SetDatatablePermissions { - enabled: true, - roles: vec![role("admin", &[])], - default_role: None, - renames: vec![], - }; - let plan = plan(Some(&old), &req, &[pg_role.as_str()]).unwrap(); - - assert_eq!( - sql(&plan), - vec![ - // Postgres only lets a role reassign objects it has the privileges - // of, and creating a role does not confer those — so the grant has - // to be (re-)established before the reassign. - format!("GRANT \"{pg_role}\" TO \"{ADMIN_PG}\" WITH SET TRUE;"), - format!("REASSIGN OWNED BY \"{pg_role}\" TO \"{ADMIN_PG}\";"), - format!("DROP OWNED BY \"{pg_role}\";"), - format!("DROP ROLE \"{pg_role}\";"), - ] - ); - assert!(!plan.permissions.roles.contains_key("analyst")); - } - - #[test] - fn opting_out_drops_every_role_and_clears_the_definitions() { - let old = enabled_with(&["analyst", "writer"]); - let existing: Vec = ["analyst", "writer"] - .iter() - .map(|r| datatable_pg_role_name(W_ID, DT, r)) - .collect(); - let req = SetDatatablePermissions { - enabled: false, - roles: vec![], - default_role: None, - renames: vec![], - }; - let plan = plan( - Some(&old), - &req, - &existing.iter().map(|s| s.as_str()).collect::>(), - ) - .unwrap(); - - assert_eq!( - sql(&plan) - .iter() - .filter(|s| s.starts_with("DROP ROLE")) - .count(), - 2 - ); - assert!(!plan.permissions.enabled); - assert!(plan.permissions.roles.is_empty()); - } - - /// A name freed by a delete must be reusable by a rename in the same save, - /// which only holds if the drops are planned first. - #[test] - fn a_name_freed_in_the_same_save_can_be_reused() { - let old = enabled_with(&["analyst", "reader"]); - let existing: Vec = ["analyst", "reader"] - .iter() - .map(|r| datatable_pg_role_name(W_ID, DT, r)) - .collect(); - // Delete `reader`, rename `analyst` into its place. - let req = SetDatatablePermissions { - enabled: true, - roles: vec![role("admin", &[]), role("reader", &[])], - default_role: None, - renames: vec![DatatableRoleRename { - from: "analyst".to_string(), - to: "reader".to_string(), - }], - }; - let plan = plan( - Some(&old), - &req, - &existing.iter().map(|s| s.as_str()).collect::>(), - ) - .unwrap(); - - let statements = sql(&plan); - let drop_at = statements - .iter() - .position(|s| s.starts_with("DROP ROLE")) - .expect("the freed role is dropped"); - let rename_at = statements - .iter() - .position(|s| s.starts_with("ALTER ROLE") && s.contains("RENAME TO")) - .expect("the surviving role is renamed"); - assert!( - drop_at < rename_at, - "drop must precede the rename that reuses the name: {statements:?}" - ); - } - - /// Freeing a name by renaming its holder away and giving it to a brand new - /// role in the same save: the new role must actually be created, and only - /// after the rename has vacated the name. - #[test] - fn a_name_freed_by_a_rename_can_be_taken_by_a_new_role() { - let old = enabled_with(&["analyst"]); - let old_pg = datatable_pg_role_name(W_ID, DT, "analyst"); - let req = SetDatatablePermissions { - enabled: true, - roles: vec![ - role("admin", &[]), - role("reader", &[]), - role("analyst", &[]), - ], - default_role: None, - renames: vec![DatatableRoleRename { - from: "analyst".to_string(), - to: "reader".to_string(), - }], - }; - let plan = plan(Some(&old), &req, &[old_pg.as_str()]).unwrap(); - - let statements = sql(&plan); - let rename_at = statements - .iter() - .position(|s| s.starts_with("ALTER ROLE") && s.contains("RENAME TO")) - .expect("the old role is renamed away"); - let create_at = statements - .iter() - .position(|s| s.starts_with("CREATE ROLE")) - .expect("the reused name is created fresh, not silently inherited"); - assert!( - rename_at < create_at, - "the rename must vacate the name before the create takes it: {statements:?}" - ); - // The new role is a different Postgres role from the one that was renamed. - assert_eq!( - plan.permissions.roles["analyst"].pg_rolename.as_deref(), - Some(old_pg.as_str()) - ); - assert_ne!( - plan.permissions.roles["reader"].pg_rolename.as_deref(), - Some(old_pg.as_str()) - ); - } - - /// Swapping two role names cannot be ordered into a working sequence, so it - /// is refused with an actionable message rather than aborting in Postgres. - #[test] - fn swapping_two_role_names_is_refused_with_an_explanation() { - let old = enabled_with(&["a", "b"]); - let existing: Vec = ["a", "b"] - .iter() - .map(|r| datatable_pg_role_name(W_ID, DT, r)) - .collect(); - let req = SetDatatablePermissions { - enabled: true, - roles: vec![role("admin", &[]), role("a", &[]), role("b", &[])], - default_role: None, - renames: vec![ - DatatableRoleRename { from: "a".to_string(), to: "b".to_string() }, - DatatableRoleRename { from: "b".to_string(), to: "a".to_string() }, - ], - }; - let err = plan( - Some(&old), - &req, - &existing.iter().map(|s| s.as_str()).collect::>(), - ) - .unwrap_err() - .to_string(); - assert!(err.contains("separate saves"), "{err}"); - } - - #[test] - fn a_role_the_config_lost_track_of_is_not_dropped() { - let old = enabled_with(&["analyst"]); - let req = SetDatatablePermissions { - enabled: false, - roles: vec![], - default_role: None, - renames: vec![], - }; - // The Postgres role is already gone, so planning its drop would fail the - // whole transaction and wedge the opt-out. - let plan = plan(Some(&old), &req, &[]).unwrap(); - assert!(sql(&plan).is_empty()); - } - - #[test] - fn the_default_role_must_be_one_of_the_saved_roles() { - let mut req = SetDatatablePermissions { - enabled: true, - roles: vec![role("admin", &[]), role("analyst", &[])], - default_role: Some("analyst".to_string()), - renames: vec![], - }; - let planned = plan(None, &req, &[]).unwrap(); - assert_eq!(planned.permissions.default_role(), "analyst"); - - // A default naming a role that is not being saved would leave every - // script that names no role failing to resolve. - req.default_role = Some("ghost".to_string()); - assert!(plan(None, &req, &[]).is_err()); - - // Absent means admin, and is stored as absent rather than spelled out. - req.default_role = None; - let planned = plan(None, &req, &[]).unwrap(); - assert_eq!(planned.permissions.default_role(), ADMIN_DATATABLE_ROLE); - assert!(planned.permissions.default_role.is_none()); - } - - #[test] - fn admin_cannot_be_dropped_or_renamed() { - let req = SetDatatablePermissions { - enabled: true, - roles: vec![role("analyst", &[])], - default_role: None, - renames: vec![], - }; - assert!(plan(None, &req, &[]).is_err()); - - let old = enabled_with(&[]); - let req = SetDatatablePermissions { - enabled: true, - roles: vec![role("owner", &[])], - default_role: None, - renames: vec![DatatableRoleRename { - from: ADMIN_DATATABLE_ROLE.to_string(), - to: "owner".to_string(), - }], - }; - assert!(plan(Some(&old), &req, &[]).is_err()); - } - - #[test] - fn invalid_role_names_and_tenants_are_rejected() { - for bad_role in ["", "bad name", "a;b", "drop\"role"] { - let req = SetDatatablePermissions { - enabled: true, - roles: vec![role("admin", &[]), role(bad_role, &[])], - default_role: None, - renames: vec![], - }; - assert!( - plan(None, &req, &[]).is_err(), - "{bad_role} should be rejected" - ); - } - // The wildcard is the one tenant with no prefix. - let req = SetDatatablePermissions { - enabled: true, - roles: vec![role("admin", &["*"])], - default_role: None, - renames: vec![], - }; - assert!(plan(None, &req, &[]).is_ok()); - - for bad_tenant in ["alice", "x/alice", "u/", "", "*/alice", "**"] { - let req = SetDatatablePermissions { - enabled: true, - roles: vec![role("admin", &[bad_tenant])], - default_role: None, - renames: vec![], - }; - assert!( - plan(None, &req, &[]).is_err(), - "{bad_tenant} should be rejected" - ); - } - } -} diff --git a/backend/windmill-api-workspaces/src/datatable_permissions_oss.rs b/backend/windmill-api-workspaces/src/datatable_permissions_oss.rs new file mode 100644 index 0000000000..d62783c955 --- /dev/null +++ b/backend/windmill-api-workspaces/src/datatable_permissions_oss.rs @@ -0,0 +1,42 @@ +/* + * 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. + */ + +//! Where the role planner comes from: the enterprise one, or a refusal. +//! +//! Every change to a data table's Postgres roles — creating, renaming, dropping +//! them — is the plan this returns, so an edition without the enterprise module +//! cannot make one. + +#[cfg(feature = "private")] +#[allow(unused)] +pub(crate) use crate::datatable_permissions_ee::*; + +#[cfg(not(feature = "private"))] +use { + crate::datatable_permissions::{DefaultAclRule, RolePlan, SetDatatablePermissions}, + std::collections::HashSet, + windmill_common::error::{Error, Result}, + windmill_common::workspaces::DataTablePermissions, +}; + +#[cfg(not(feature = "private"))] +pub(crate) fn plan_role_changes( + _w_id: &str, + _datatable: &str, + _dbname: &str, + _admin_pg_role: &str, + _old: Option<&DataTablePermissions>, + _req: &SetDatatablePermissions, + _existing_pg_roles: &HashSet, + _public_schema_is_open: bool, + _default_acl_rules: &[DefaultAclRule], +) -> Result { + Err(Error::BadRequest( + "Data table permissions are a Windmill Enterprise Edition feature".to_string(), + )) +} diff --git a/backend/windmill-api-workspaces/src/lib.rs b/backend/windmill-api-workspaces/src/lib.rs index b94f87ab34..3fb81da3d2 100644 --- a/backend/windmill-api-workspaces/src/lib.rs +++ b/backend/windmill-api-workspaces/src/lib.rs @@ -1,11 +1,17 @@ pub mod data_metrics; pub mod datatable_acl; +pub mod datatable_acl_oss; pub mod datatable_migrations; pub mod datatable_permissions; +pub mod datatable_permissions_oss; pub mod deployment_requests; pub mod workspaces; pub mod workspaces_extra; pub mod workspaces_oss; +#[cfg(feature = "private")] +pub mod datatable_acl_ee; +#[cfg(feature = "private")] +pub mod datatable_permissions_ee; #[cfg(feature = "private")] pub mod workspaces_ee; diff --git a/frontend/src/lib/components/DBManager.svelte b/frontend/src/lib/components/DBManager.svelte index 43bfb05255..9e08bcff73 100644 --- a/frontend/src/lib/components/DBManager.svelte +++ b/frontend/src/lib/components/DBManager.svelte @@ -1,5 +1,5 @@