From 9da1a700720408136f6c33f5d918e5d905749e48 Mon Sep 17 00:00:00 2001 From: Naisila Puka <37271756+naisila@users.noreply.github.com> Date: Mon, 11 Sep 2023 15:58:24 +0300 Subject: [PATCH] Fix WITH ADMIN FALSE propagation (#7191) --- .../distributed/deparser/deparse_role_stmts.c | 32 ++---- src/test/regress/expected/pg16.out | 22 ---- src/test/regress/sql/pg16.sql | 100 ------------------ 3 files changed, 6 insertions(+), 148 deletions(-) diff --git a/src/backend/distributed/deparser/deparse_role_stmts.c b/src/backend/distributed/deparser/deparse_role_stmts.c index 419a106cc..c0f336fd1 100644 --- a/src/backend/distributed/deparser/deparse_role_stmts.c +++ b/src/backend/distributed/deparser/deparse_role_stmts.c @@ -453,35 +453,15 @@ AppendGrantWithAdminOption(StringInfo buf, GrantRoleStmt *stmt) int opt_count = 0; foreach_ptr(opt, stmt->opt) { - switch (opt->defname) - { - case "admin": - appendStringInfo(buf, " WITH ADMIN OPTION"); - opt_count++; - break; - - case "inherit": - if (opt_count > 0) - { - appendStringInfo(buf, ", "); - } - appendStringInfo(buf, "INHERIT OPTION "); - opt_count++; + bool admin_option = false; + char *optval = defGetString(opt); + if (strcmp(opt->defname, "admin") == 0 && + parse_bool(optval, &admin_option) && admin_option) + { + appendStringInfo(buf, " WITH ADMIN OPTION"); break; - - - case "set": - if (opt_count > 0) - { - appendStringInfo(buf, ", "); - } - appendStringInfo(buf, "SET OPTION "); - opt_count++; - break; - } } } - } #else if (stmt->admin_opt) { diff --git a/src/test/regress/expected/pg16.out b/src/test/regress/expected/pg16.out index 3d2d51896..39fd5a29b 100644 --- a/src/test/regress/expected/pg16.out +++ b/src/test/regress/expected/pg16.out @@ -916,28 +916,6 @@ SELECT result FROM run_command_on_workers REINDEX (2 rows) --- REINDEX DATABASE/SYSTEM name is optional --- We already don't propagate these commands automatically --- Testing here with run_command_on_workers --- Relevant PG commit: https://github.com/postgres/postgres/commit/2cbc3c1 -REINDEX DATABASE; -SELECT result FROM run_command_on_workers -($$REINDEX DATABASE$$); - result ---------------------------------------------------------------------- - REINDEX - REINDEX -(2 rows) - -REINDEX SYSTEM; -SELECT result FROM run_command_on_workers -($$REINDEX SYSTEM$$); - result ---------------------------------------------------------------------- - REINDEX - REINDEX -(2 rows) - -- -- random_normal() to provide normally-distributed random numbers -- adding here the same tests as the ones with random() in aggregate_support.sql diff --git a/src/test/regress/sql/pg16.sql b/src/test/regress/sql/pg16.sql index 5055d5a5d..a078e76b9 100644 --- a/src/test/regress/sql/pg16.sql +++ b/src/test/regress/sql/pg16.sql @@ -591,106 +591,6 @@ REVOKE role1 FROM role2; RESET citus.log_remote_commands; RESET citus.grep_remote_commands; --- --- PG16 added new options to GRANT ROLE --- inherit: https://github.com/postgres/postgres/commit/e3ce2de --- set: https://github.com/postgres/postgres/commit/3d14e17 --- We don't propagate for now in Citus --- -GRANT role1 TO role2 WITH INHERIT FALSE; -REVOKE role1 FROM role2; -GRANT role1 TO role2 WITH INHERIT TRUE; -REVOKE role1 FROM role2; -GRANT role1 TO role2 WITH INHERIT OPTION; -REVOKE role1 FROM role2; -GRANT role1 TO role2 WITH SET FALSE; -REVOKE role1 FROM role2; -GRANT role1 TO role2 WITH SET TRUE; -REVOKE role1 FROM role2; -GRANT role1 TO role2 WITH SET OPTION; -REVOKE role1 FROM role2; - --- connect to worker node -GRANT role1 TO role2 WITH ADMIN OPTION, INHERIT FALSE, SET FALSE; - -SELECT roleid::regrole::text AS role, member::regrole::text, -admin_option, inherit_option, set_option FROM pg_auth_members -WHERE roleid::regrole::text = 'role1' ORDER BY 1, 2; - -\c - - - :worker_1_port - -SELECT roleid::regrole::text AS role, member::regrole::text, -admin_option, inherit_option, set_option FROM pg_auth_members -WHERE roleid::regrole::text = 'role1' ORDER BY 1, 2; - -SET citus.enable_ddl_propagation TO off; -GRANT role1 TO role2 WITH ADMIN OPTION, INHERIT FALSE, SET FALSE; -RESET citus.enable_ddl_propagation; - -SELECT roleid::regrole::text AS role, member::regrole::text, -admin_option, inherit_option, set_option FROM pg_auth_members -WHERE roleid::regrole::text = 'role1' ORDER BY 1, 2; - -\c - - - :master_port -REVOKE role1 FROM role2; - --- test REVOKES as well -GRANT role1 TO role2; -REVOKE SET OPTION FOR role1 FROM role2; -REVOKE INHERIT OPTION FOR role1 FROM role2; - -DROP ROLE role1, role2; - --- test that everything works fine for roles that are not propagated -SET citus.enable_ddl_propagation TO off; -CREATE ROLE role3; -CREATE ROLE role4; -CREATE ROLE role5; -RESET citus.enable_ddl_propagation; --- by default, admin option is false, inherit is true, set is true -GRANT role3 TO role4; -GRANT role3 TO role5 WITH ADMIN TRUE, INHERIT FALSE, SET FALSE; -SELECT roleid::regrole::text AS role, member::regrole::text, admin_option, inherit_option, set_option FROM pg_auth_members WHERE roleid::regrole::text = 'role3' ORDER BY 1, 2; - -DROP ROLE role3, role4, role5; - -\set VERBOSITY terse -SET client_min_messages TO ERROR; -DROP EXTENSION postgres_fdw CASCADE; -DROP SCHEMA pg16 CASCADE; - -======= ->>>>>>> 1bfef9d5c (Review changes for pg16 update GRANT and REVOKE) --- --- PG16 allows GRANT WITH ADMIN | INHERIT | SET --- --- GRANT privileges to a role or roles -\c - - - :master_port -CREATE ROLE create_role; -CREATE ROLE create_role_2; -CREATE ROLE create_role_3; -CREATE ROLE create_role_4; -CREATE USER create_user; -CREATE USER create_user_2; -CREATE GROUP create_group; -CREATE GROUP create_group_2; - ---test grant role -GRANT create_group TO create_role; -GRANT create_group TO create_role_2 WITH ADMIN OPTION; -GRANT create_group TO create_role_3 WITH INHERIT; -GRANT create_group TO create_role_4 WITH SET; - --- ADMIN role can perfom administrative tasks --- role can now access the data and permissions of the table (owner of table) --- role can change current user to any other user/role that has access -GRANT ADMIN TO joe; -GRANT INHERIT ON ROLE joe TO james; - -GRANT SELECT ON companies TO joe WITH GRANT OPTION; -GRANT SET (SELECT) ON companies TO james; - - \set VERBOSITY terse SET client_min_messages TO ERROR; DROP SCHEMA pg16 CASCADE;