mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix: stop copying default privileges to new data table roles
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DsU2Lf6wYQJ9o8ASKRgCmK
This commit is contained in:
Diego Imbert
co-authored by
Claude Opus 5
Claude Opus 5
parent
996d43080c
commit
4004a0fc14
@@ -1 +1 @@
|
||||
5bc3830f04ff2de110c862eb31689cb4e1724d66
|
||||
1a24b189f8c94a7a1a3867385f7002d0f607a673
|
||||
|
||||
@@ -2635,7 +2635,6 @@ async fn create_datatable_role(
|
||||
catalog.insert(id.clone(), entry);
|
||||
tx.commit().await?;
|
||||
converge_connect_grants_everywhere(&db, &catalog).await;
|
||||
copy_default_privileges_everywhere(&db, &req.name).await;
|
||||
windmill_common::feature_usage::log_feature_usage("datatable", "role_created", "");
|
||||
|
||||
audit_log(
|
||||
@@ -2750,28 +2749,6 @@ async fn delete_datatable_role(
|
||||
Ok(Json(()))
|
||||
}
|
||||
|
||||
/// Best-effort, like the `CONNECT` convergence below: a database this misses leaves the new role's
|
||||
/// objects there granting nobody anything, which re-applying the "created later" grant in the ACL
|
||||
/// editor repairs.
|
||||
async fn copy_default_privileges_everywhere(db: &DB, name: &str) {
|
||||
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 copy default privileges: {e}");
|
||||
return;
|
||||
}
|
||||
};
|
||||
for dbname in dbnames {
|
||||
if let Err(e) =
|
||||
windmill_common::datatable_roles::copy_default_privileges_to(db, &dbname, name).await
|
||||
{
|
||||
tracing::warn!(
|
||||
"Could not copy default privileges to data table role '{name}' on '{dbname}': {e}"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// 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.
|
||||
|
||||
@@ -453,95 +453,6 @@ pub async fn drop_instance_role(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// `ALTER DEFAULT PRIVILEGES` binds only what its own creating role makes, so a role added after a
|
||||
/// "created later" grant would create objects that grant nobody anything. The ACL editor writes
|
||||
/// every such grant for `custom_instance_user` as well, which makes its default privileges the
|
||||
/// record of that intent: a new role is given the same ones in `dbname`.
|
||||
///
|
||||
/// Authorization: rewrites default privileges with the server's own credentials and checks
|
||||
/// nothing. Callers MUST restrict this to superadmin paths.
|
||||
pub async fn copy_default_privileges_to(db: &DB, dbname: &str, name: &str) -> Result<()> {
|
||||
validate_role_name(name)?;
|
||||
crate::validate_dbname(dbname)?;
|
||||
let base = crate::PgDatabase::parse_uri(&crate::get_database_url().await?.as_str().await)?;
|
||||
let creds = crate::PgDatabase { dbname: dbname.to_string(), ..base };
|
||||
let (client, connection) = creds.connect(Some(db)).await?;
|
||||
let join_handle = tokio::spawn(async move { connection.await });
|
||||
let result = async {
|
||||
let rows = client
|
||||
.query(
|
||||
"SELECT n.nspname::text, d.defaclobjtype::text,
|
||||
CASE WHEN a.grantee = 0 THEN NULL ELSE pg_get_userbyid(a.grantee)::text END,
|
||||
a.privilege_type
|
||||
FROM pg_default_acl d JOIN pg_namespace n ON n.oid = d.defaclnamespace,
|
||||
aclexplode(d.defaclacl) a
|
||||
WHERE d.defaclrole = (SELECT oid FROM pg_roles WHERE rolname = $1)",
|
||||
&[&CUSTOM_INSTANCE_USER],
|
||||
)
|
||||
.await?;
|
||||
let entries: Vec<(String, String, Option<String>, String)> = rows
|
||||
.iter()
|
||||
.map(|row| (row.get(0), row.get(1), row.get(2), row.get(3)))
|
||||
.collect();
|
||||
for statement in default_privilege_copies(name, &entries) {
|
||||
client.batch_execute(&statement).await?;
|
||||
}
|
||||
Ok::<(), tokio_postgres::Error>(())
|
||||
}
|
||||
.await;
|
||||
drop(client);
|
||||
crate::shutdown_pg_connection(join_handle).await?;
|
||||
result.map_err(|e| {
|
||||
Error::internal_err(format!(
|
||||
"Copying default privileges to role '{name}' in '{dbname}': {}",
|
||||
crate::error::pg_error_message(&e)
|
||||
))
|
||||
})
|
||||
}
|
||||
|
||||
/// The statements giving `role` the default privileges listed — (schema, `defaclobjtype`, grantee,
|
||||
/// privilege), grantee `None` for PUBLIC — one per schema, kind of object and grantee. Privileges
|
||||
/// are Postgres's own keywords, read back from its catalog.
|
||||
fn default_privilege_copies(
|
||||
role: &str,
|
||||
entries: &[(String, String, Option<String>, String)],
|
||||
) -> Vec<String> {
|
||||
let mut grouped: BTreeMap<(&str, &str, Option<&str>), Vec<&str>> = BTreeMap::new();
|
||||
for (schema, objtype, grantee, privilege) in entries {
|
||||
let plural = match objtype.as_str() {
|
||||
"r" => "TABLES",
|
||||
"S" => "SEQUENCES",
|
||||
"f" => "FUNCTIONS",
|
||||
"T" => "TYPES",
|
||||
_ => continue,
|
||||
};
|
||||
if grantee.as_deref() == Some(role) {
|
||||
continue;
|
||||
}
|
||||
grouped
|
||||
.entry((schema.as_str(), plural, grantee.as_deref()))
|
||||
.or_default()
|
||||
.push(privilege.as_str());
|
||||
}
|
||||
grouped
|
||||
.into_iter()
|
||||
.map(|((schema, plural, grantee), mut privileges)| {
|
||||
privileges.sort();
|
||||
privileges.dedup();
|
||||
format!(
|
||||
"ALTER DEFAULT PRIVILEGES FOR ROLE {} IN SCHEMA {} GRANT {} ON {} TO {}",
|
||||
quote_ident(role),
|
||||
quote_ident(schema),
|
||||
privileges.join(", "),
|
||||
plural,
|
||||
grantee
|
||||
.map(quote_ident)
|
||||
.unwrap_or_else(|| "PUBLIC".to_string())
|
||||
)
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -578,34 +489,4 @@ mod tests {
|
||||
catalog.get_mut("id1").unwrap().enabled = true;
|
||||
assert_eq!(role_id_by_name(&catalog, "analytics").unwrap(), "id1");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_new_role_is_given_the_admin_connections_default_privileges() {
|
||||
let entry = |schema: &str, objtype: &str, grantee: Option<&str>, privilege: &str| {
|
||||
(
|
||||
schema.to_string(),
|
||||
objtype.to_string(),
|
||||
grantee.map(str::to_string),
|
||||
privilege.to_string(),
|
||||
)
|
||||
};
|
||||
let statements = default_privilege_copies(
|
||||
"late",
|
||||
&[
|
||||
entry("public", "r", Some("analytics"), "SELECT"),
|
||||
entry("public", "r", Some("analytics"), "INSERT"),
|
||||
entry("public", "f", None, "EXECUTE"),
|
||||
// A grant to itself says nothing, and `n` has no per-schema form.
|
||||
entry("public", "S", Some("late"), "USAGE"),
|
||||
entry("public", "n", Some("analytics"), "USAGE"),
|
||||
],
|
||||
);
|
||||
assert_eq!(
|
||||
statements,
|
||||
[
|
||||
r#"ALTER DEFAULT PRIVILEGES FOR ROLE "late" IN SCHEMA "public" GRANT EXECUTE ON FUNCTIONS TO PUBLIC"#,
|
||||
r#"ALTER DEFAULT PRIVILEGES FOR ROLE "late" IN SCHEMA "public" GRANT INSERT, SELECT ON TABLES TO "analytics""#,
|
||||
]
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -132,7 +132,7 @@
|
||||
<span class="text-xs font-semibold text-emphasis">Owner</span>
|
||||
<span class="text-xs text-secondary">
|
||||
{target.kind === 'schema'
|
||||
? 'The role that owns the schema and everything already in it, except what belongs to an extension, which stays with the extension. Changing it also keeps the new owner in reach of what the other roles create here later.'
|
||||
? 'The role that owns the schema and everything already in it, except what belongs to an extension, which stays with the extension. Changing it also keeps the new owner in reach of what the roles there are now create here later.'
|
||||
: 'The role that owns the table. Its owner may always read and write it, and is who ALTER and DROP answer to.'}
|
||||
</span>
|
||||
</div>
|
||||
|
||||
Reference in New Issue
Block a user