Skip to content

Commit

Permalink
PG16 update GRANT... ADMIN | INHERIT | SET, and REVOKE
Browse files Browse the repository at this point in the history 
Allowing GRANT ADMIN to now also be INHERIT or SET in support of psql16

GRANT role_name [, ...] TO role_specification [, ...] [ WITH { ADMIN |
INHERIT | SET } { OPTION | TRUE | FALSE } ] [ GRANTED BY
role_specification ]

Fixes: #7148 
Related: #7138

See review changes from #7164
  • Loading branch information
@francisjodi
francisjodi authored Dec 13, 2023
1 parent dbdde11 commit 6801a1e
Show file tree
Hide file tree
Showing 5 changed files with 269 additions and 129 deletions.
60 changes: 23 additions & 37 deletions src/backend/distributed/commands/role.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ static const char * WrapQueryInAlterRoleIfExistsCall(const char *query, RoleSpec
static VariableSetStmt * MakeVariableSetStmt(const char *config);
static int ConfigGenericNameCompare(const void *lhs, const void *rhs);
static List * RoleSpecToObjectAddress(RoleSpec *role, bool missing_ok);
static bool IsGrantRoleWithInheritOrSetOption(GrantRoleStmt *stmt);

/* controlled via GUC */
bool EnableCreateRolePropagation = true;
Expand Down Expand Up @@ -893,10 +892,31 @@ GenerateGrantRoleStmtsOfRole(Oid roleid)
grantRoleStmt->grantor = NULL;

#if PG_VERSION_NUM >= PG_VERSION_16

/* inherit option is always included */
DefElem *inherit_opt;
if (membership->inherit_option)
{
inherit_opt = makeDefElem("inherit", (Node *) makeBoolean(true), -1);
}
else
{
inherit_opt = makeDefElem("inherit", (Node *) makeBoolean(false), -1);
}
grantRoleStmt->opt = list_make1(inherit_opt);

/* admin option is false by default, only include true case */
if (membership->admin_option)
{
DefElem *opt = makeDefElem("admin", (Node *) makeBoolean(true), -1);
grantRoleStmt->opt = list_make1(opt);
DefElem *admin_opt = makeDefElem("admin", (Node *) makeBoolean(true), -1);
grantRoleStmt->opt = lappend(grantRoleStmt->opt, admin_opt);
}

/* set option is true by default, only include false case */
if (!membership->set_option)
{
DefElem *set_opt = makeDefElem("set", (Node *) makeBoolean(false), -1);
grantRoleStmt->opt = lappend(grantRoleStmt->opt, set_opt);
}
#else
grantRoleStmt->admin_opt = membership->admin_option;
Expand Down Expand Up @@ -1209,19 +1229,6 @@ PreprocessGrantRoleStmt(Node *node, const char *queryString,
return NIL;
}

if (IsGrantRoleWithInheritOrSetOption(stmt))
{
if (EnableUnsupportedFeatureMessages)
{
ereport(NOTICE, (errmsg("not propagating GRANT/REVOKE commands with specified"
" INHERIT/SET options to worker nodes"),
errhint(
"Connect to worker nodes directly to manually run the same"
" GRANT/REVOKE command after disabling DDL propagation.")));
}
return NIL;
}

/*
* Postgres don't seem to use the grantor. Even dropping the grantor doesn't
* seem to affect the membership. If this changes, we might need to add grantors
Expand Down Expand Up @@ -1273,27 +1280,6 @@ PostprocessGrantRoleStmt(Node *node, const char *queryString)
}


/*
* IsGrantRoleWithInheritOrSetOption returns true if the given
* GrantRoleStmt has inherit or set option specified in its options
*/
static bool
IsGrantRoleWithInheritOrSetOption(GrantRoleStmt *stmt)
{
#if PG_VERSION_NUM >= PG_VERSION_16
DefElem *opt = NULL;
foreach_ptr(opt, stmt->opt)
{
if (strcmp(opt->defname, "inherit") == 0 || strcmp(opt->defname, "set") == 0)
{
return true;
}
}
#endif
return false;
}


/*
* ConfigGenericNameCompare compares two config_generic structs based on their
* name fields. If the name fields contain the same strings two structs are
Expand Down
33 changes: 28 additions & 5 deletions src/backend/distributed/deparser/deparse_role_stmts.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,16 @@ AppendRevokeAdminOptionFor(StringInfo buf, GrantRoleStmt *stmt)
appendStringInfo(buf, "ADMIN OPTION FOR ");
break;
}
else if (strcmp(opt->defname, "inherit") == 0)
{
appendStringInfo(buf, "INHERIT OPTION FOR ");
break;
}
else if (strcmp(opt->defname, "set") == 0)
{
appendStringInfo(buf, "SET OPTION FOR ");
break;
}
}
}
#else
Expand All @@ -428,16 +438,29 @@ AppendGrantWithAdminOption(StringInfo buf, GrantRoleStmt *stmt)
if (stmt->is_grant)
{
#if PG_VERSION_NUM >= PG_VERSION_16
int opt_count = 0;
DefElem *opt = NULL;
foreach_ptr(opt, stmt->opt)
{
bool admin_option = false;
char *optval = defGetString(opt);
if (strcmp(opt->defname, "admin") == 0 &&
parse_bool(optval, &admin_option) && admin_option)
bool option_value = false;
if (parse_bool(optval, &option_value))
{
appendStringInfo(buf, " WITH ADMIN OPTION");
break;
opt_count++;
char *prefix = opt_count > 1 ? "," : " WITH";
if (strcmp(opt->defname, "inherit") == 0)
{
appendStringInfo(buf, "%s INHERIT %s", prefix, option_value ? "TRUE" :
"FALSE");
}
else if (strcmp(opt->defname, "admin") == 0 && option_value)
{
appendStringInfo(buf, "%s ADMIN OPTION", prefix);
}
else if (strcmp(opt->defname, "set") == 0 && !option_value)
{
appendStringInfo(buf, "%s SET FALSE", prefix);
}
}
}
#else
Expand Down
187 changes: 132 additions & 55 deletions src/test/regress/expected/pg16.out
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1007,49 +1007,18 @@ DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
NOTICE: issuing GRANT role1 TO role2 WITH ADMIN OPTION;
DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
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
-- We now propagate these options in Citus
--
GRANT role1 TO role2 WITH INHERIT FALSE;
NOTICE: not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes
HINT: Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation.
REVOKE role1 FROM role2;
GRANT role1 TO role2 WITH INHERIT TRUE;
NOTICE: not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes
HINT: Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation.
REVOKE role1 FROM role2;
GRANT role1 TO role2 WITH INHERIT OPTION;
NOTICE: not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes
HINT: Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation.
REVOKE role1 FROM role2;
GRANT role1 TO role2 WITH SET FALSE;
NOTICE: not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes
HINT: Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation.
REVOKE role1 FROM role2;
GRANT role1 TO role2 WITH SET TRUE;
NOTICE: not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes
HINT: Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation.
REVOKE role1 FROM role2;
GRANT role1 TO role2 WITH SET OPTION;
NOTICE: not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes
HINT: Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation.
REVOKE role1 FROM role2;
-- connect to worker node
GRANT role1 TO role2 WITH ADMIN OPTION, INHERIT FALSE, SET FALSE;
NOTICE: not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes
HINT: Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling 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;
role | member | admin_option | inherit_option | set_option
role | member | admin_option | inherit_option | set_option
---------------------------------------------------------------------
role1 | role2 | t | f | f
(1 row)
(0 rows)

\c - - - :worker_1_port
SELECT roleid::regrole::text AS role, member::regrole::text,
Expand All @@ -1059,27 +1028,22 @@ WHERE roleid::regrole::text = 'role1' ORDER BY 1, 2;
---------------------------------------------------------------------
(0 rows)

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;
role | member | admin_option | inherit_option | set_option
---------------------------------------------------------------------
role1 | role2 | t | f | f
(1 row)

\c - - - :master_port
REVOKE role1 FROM role2;
-- test REVOKES as well
GRANT role1 TO role2;
REVOKE SET OPTION FOR role1 FROM role2;
NOTICE: not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes
HINT: Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation.
REVOKE INHERIT OPTION FOR role1 FROM role2;
NOTICE: not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes
HINT: Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation.
-- Set GUCs to log remote commands and filter on REVOKE commands
SET citus.log_remote_commands TO on;
SET citus.grep_remote_commands = '%REVOKE%';
-- test REVOKES as well
GRANT role1 TO role2;
REVOKE SET OPTION FOR role1 FROM role2;
NOTICE: issuing REVOKE SET OPTION FOR role1 FROM role2 RESTRICT;
DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
NOTICE: issuing REVOKE SET OPTION FOR role1 FROM role2 RESTRICT;
DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
REVOKE INHERIT OPTION FOR role1 FROM role2;
NOTICE: issuing REVOKE INHERIT OPTION FOR role1 FROM role2 RESTRICT;
DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
NOTICE: issuing REVOKE INHERIT OPTION FOR role1 FROM role2 RESTRICT;
DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
DROP ROLE role1, role2;
-- test that everything works fine for roles that are not propagated
SET citus.enable_ddl_propagation TO off;
Expand All @@ -1090,14 +1054,127 @@ 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;
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;
role | member | admin_option | inherit_option | set_option
---------------------------------------------------------------------
role3 | role4 | f | t | t
role3 | role5 | t | f | f
(2 rows)

DROP ROLE role3, role4, role5;
-- Test that everything works fine for roles that are propagated
CREATE ROLE role6;
CREATE ROLE role7;
CREATE ROLE role8;
CREATE ROLE role9;
CREATE ROLE role10;
CREATE ROLE role11;
CREATE ROLE role12;
CREATE ROLE role13;
CREATE ROLE role14;
CREATE ROLE role15;
CREATE ROLE role16;
CREATE ROLE role17;
CREATE ROLE role18 NOINHERIT;
CREATE ROLE role19;
CREATE ROLE role20;
-- Grant role with admin and inherit options set to true
GRANT role6 TO role7 WITH ADMIN OPTION, INHERIT TRUE;
-- GRANT with INHERIT and SET Options
-- note that set is true by default so we don't include it in the propagation
GRANT role7 TO role8 WITH INHERIT TRUE, SET TRUE;
-- Grant role with admin option set to true and inherit option set to false
GRANT role9 TO role10 WITH ADMIN OPTION, INHERIT FALSE;
-- Grant role with admin option set to true, and inherit/set options set to false
GRANT role11 TO role12 WITH INHERIT FALSE, ADMIN TRUE, SET FALSE;
-- Grant role with inherit set to false
GRANT role13 TO role14 WITH INHERIT FALSE;
-- Grant role with set option set to false
GRANT role15 TO role16 WITH SET FALSE;
-- Handles with default inherit false
-- we created role18 with noinherit option above
GRANT role17 TO role18;
-- Run GRANT/REVOKE commands on worker nodes
\c - - - :worker_1_port
-- Run GRANT command on worker node
GRANT role19 TO role20;
\c - - - :master_port
SELECT roleid::regrole::text AS role, member::regrole::text, admin_option, inherit_option, set_option
FROM pg_auth_members
WHERE roleid::regrole::text LIKE 'role%'
ORDER BY 1, 2;
role | member | admin_option | inherit_option | set_option
---------------------------------------------------------------------
role11 | role12 | t | f | f
role13 | role14 | f | f | t
role15 | role16 | f | t | f
role17 | role18 | f | f | t
role19 | role20 | f | t | t
role6 | role7 | t | t | t
role7 | role8 | f | t | t
role9 | role10 | t | f | t
(8 rows)

\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 LIKE 'role%'
ORDER BY 1, 2;
role | member | admin_option | inherit_option | set_option
---------------------------------------------------------------------
role11 | role12 | t | f | f
role13 | role14 | f | f | t
role15 | role16 | f | t | f
role17 | role18 | f | f | t
role19 | role20 | f | t | t
role6 | role7 | t | t | t
role7 | role8 | f | t | t
role9 | role10 | t | f | t
(8 rows)

\c - - - :master_port
DROP ROLE role6, role7, role8, role9, role10, role11, role12,
role13, role14, role15, role16, role17, role18, role19, role20;
-- here we test that we propagate admin, set and inherit options correctly
-- when adding a new node.
-- First, we need to remove the node:
SELECT 1 FROM citus_remove_node('localhost', :worker_2_port);
?column?
---------------------------------------------------------------------
1
(1 row)

CREATE ROLE create_role1;
CREATE ROLE create_role2;
CREATE ROLE create_role3;
-- test grant role
GRANT create_role1 TO create_role2 WITH ADMIN OPTION, INHERIT FALSE, SET FALSE;
GRANT create_role2 TO create_role3 WITH INHERIT TRUE, ADMIN FALSE, SET FALSE;
SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option, inherit_option, set_option FROM pg_auth_members WHERE roleid::regrole::text LIKE 'create\_%' ORDER BY 1, 2;
role | member | grantor | admin_option | inherit_option | set_option
---------------------------------------------------------------------
create_role1 | create_role2 | postgres | t | f | f
create_role2 | create_role3 | postgres | f | t | f
(2 rows)

-- Add second worker node
SELECT 1 FROM citus_add_node('localhost', :worker_2_port);
?column?
---------------------------------------------------------------------
1
(1 row)

\c - - - :worker_2_port
SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option, inherit_option, set_option FROM pg_auth_members WHERE roleid::regrole::text LIKE 'create\_%' ORDER BY 1, 2;
role | member | grantor | admin_option | inherit_option | set_option
---------------------------------------------------------------------
create_role1 | create_role2 | postgres | t | f | f
create_role2 | create_role3 | postgres | f | t | f
(2 rows)

\c - - - :master_port
DROP ROLE create_role1, create_role2, create_role3;
\set VERBOSITY terse
SET client_min_messages TO ERROR;
DROP EXTENSION postgres_fdw CASCADE;
Expand Down
2 changes: 1 addition & 1 deletion src/test/regress/multi_schedule
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
test: multi_test_helpers multi_test_helpers_superuser
test: multi_cluster_management
test: create_role_propagation
test: pg16
test: multi_create_fdw
test: multi_test_catalog_views
test: replicated_table_disable_node
Expand Down Expand Up @@ -65,7 +66,6 @@ test: pg13 pg12
test: pg14
test: pg15
test: pg15_jsonpath detect_conn_close
test: pg16
test: drop_column_partitioned_table
test: tableam

Expand Down
Loading

0 comments on commit 6801a1e

Please sign in to comment.