diff --git a/compute_tools/src/compute.rs b/compute_tools/src/compute.rs index 9969b2166c..b39a800f14 100644 --- a/compute_tools/src/compute.rs +++ b/compute_tools/src/compute.rs @@ -252,7 +252,7 @@ fn create_neon_superuser(spec: &ComputeSpec, client: &mut Client) -> Result<()> IF NOT EXISTS ( SELECT FROM pg_catalog.pg_roles WHERE rolname = 'neon_superuser') THEN - CREATE ROLE neon_superuser CREATEDB CREATEROLE NOLOGIN REPLICATION IN ROLE pg_read_all_data, pg_write_all_data; + CREATE ROLE neon_superuser CREATEDB CREATEROLE NOLOGIN REPLICATION BYPASSRLS IN ROLE pg_read_all_data, pg_write_all_data; IF array_length(roles, 1) IS NOT NULL THEN EXECUTE format('GRANT neon_superuser TO %s', array_to_string(ARRAY(SELECT quote_ident(x) FROM unnest(roles) as x), ', ')); diff --git a/compute_tools/src/pg_helpers.rs b/compute_tools/src/pg_helpers.rs index 8722822f5e..b79e516650 100644 --- a/compute_tools/src/pg_helpers.rs +++ b/compute_tools/src/pg_helpers.rs @@ -193,16 +193,11 @@ impl Escaping for PgIdent { /// Build a list of existing Postgres roles pub fn get_existing_roles(xact: &mut Transaction<'_>) -> Result> { let postgres_roles = xact - .query( - "SELECT rolname, rolpassword, rolreplication, rolbypassrls FROM pg_catalog.pg_authid", - &[], - )? + .query("SELECT rolname, rolpassword FROM pg_catalog.pg_authid", &[])? .iter() .map(|row| Role { name: row.get("rolname"), encrypted_password: row.get("rolpassword"), - replication: Some(row.get("rolreplication")), - bypassrls: Some(row.get("rolbypassrls")), options: None, }) .collect(); diff --git a/compute_tools/src/spec.rs b/compute_tools/src/spec.rs index f98333d8bf..ba1ee6d1b2 100644 --- a/compute_tools/src/spec.rs +++ b/compute_tools/src/spec.rs @@ -252,8 +252,6 @@ pub fn handle_roles(spec: &ComputeSpec, client: &mut Client) -> Result<()> { let action = if let Some(r) = pg_role { if (r.encrypted_password.is_none() && role.encrypted_password.is_some()) || (r.encrypted_password.is_some() && role.encrypted_password.is_none()) - || !r.bypassrls.unwrap_or(false) - || !r.replication.unwrap_or(false) { RoleAction::Update } else if let Some(pg_pwd) = &r.encrypted_password { @@ -285,14 +283,22 @@ pub fn handle_roles(spec: &ComputeSpec, client: &mut Client) -> Result<()> { match action { RoleAction::None => {} RoleAction::Update => { - let mut query: String = - format!("ALTER ROLE {} BYPASSRLS REPLICATION", name.pg_quote()); + // This can be run on /every/ role! Not just ones created through the console. + // This means that if you add some funny ALTER here that adds a permission, + // this will get run even on user-created roles! This will result in different + // behavior before and after a spec gets reapplied. The below ALTER as it stands + // now only grants LOGIN and changes the password. Please do not allow this branch + // to do anything silly. + let mut query: String = format!("ALTER ROLE {} ", name.pg_quote()); query.push_str(&role.to_pg_options()); xact.execute(query.as_str(), &[])?; } RoleAction::Create => { + // This branch only runs when roles are created through the console, so it is + // safe to add more permissions here. BYPASSRLS and REPLICATION are inherited + // from neon_superuser. let mut query: String = format!( - "CREATE ROLE {} CREATEROLE CREATEDB BYPASSRLS REPLICATION IN ROLE neon_superuser", + "CREATE ROLE {} INHERIT CREATEROLE CREATEDB IN ROLE neon_superuser", name.pg_quote() ); info!("role create query: '{}'", &query); diff --git a/libs/compute_api/src/spec.rs b/libs/compute_api/src/spec.rs index d9c384a5d3..2a483188e4 100644 --- a/libs/compute_api/src/spec.rs +++ b/libs/compute_api/src/spec.rs @@ -207,8 +207,6 @@ pub struct DeltaOp { pub struct Role { pub name: PgIdent, pub encrypted_password: Option, - pub replication: Option, - pub bypassrls: Option, pub options: GenericOptions, }