Skip to content

Commit

Permalink
Error out for GRANT ROLE with iherit/set option used
Browse files Browse the repository at this point in the history
We currently don't support propagating these options in Citus

Relevant PG commits:
postgres/postgres@e3ce2de
postgres/postgres@3d14e17
  • Loading branch information
naisila committed Sep 11, 2023
1 parent c1dc378 commit 704ce97
Show file tree
Hide file tree
Showing 5 changed files with 116 additions and 8 deletions.
26 changes: 26 additions & 0 deletions src/backend/distributed/commands/role.c
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ 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 void ErrorIfGrantRoleWithInheritOrSetOption(GrantRoleStmt *stmt);

/* controlled via GUC */
bool EnableCreateRolePropagation = true;
Expand Down Expand Up @@ -1141,6 +1142,8 @@ PreprocessGrantRoleStmt(Node *node, const char *queryString,
return NIL;
}

ErrorIfGrantRoleWithInheritOrSetOption(stmt);

/*
* 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 @@ -1190,6 +1193,29 @@ PostprocessGrantRoleStmt(Node *node, const char *queryString)
}


/*
* ErrorIfGrantRoleWithInheritOrSetOption errors out if the given
* GrantRoleStmt has inherit or set option specified in its options
*/
static void
ErrorIfGrantRoleWithInheritOrSetOption(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)
{
ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),

Check warning on line 1209 in src/backend/distributed/commands/role.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/commands/role.c#L1209

Added line #L1209 was not covered by tests
errmsg(
"cannot propagate GRANT/REVOKE statements with"
" specified INHERIT/SET option")));
}
}
#endif
}


/*
* ConfigGenericNameCompare compares two config_generic structs based on their
* name fields. If the name fields contain the same strings two structs are
Expand Down
17 changes: 9 additions & 8 deletions src/backend/distributed/commands/utility_hook.c
Original file line number Diff line number Diff line change
Expand Up @@ -694,9 +694,9 @@ ProcessUtilityInternal(PlannedStmt *pstmt,
}

/* inform the user about potential caveats */
if (IsA(parsetree, CreatedbStmt))
if (EnableUnsupportedFeatureMessages)
{
if (EnableUnsupportedFeatureMessages)
if (IsA(parsetree, CreatedbStmt))
{
ereport(NOTICE, (errmsg("Citus partially supports CREATE DATABASE for "
"distributed databases"),
Expand All @@ -705,13 +705,14 @@ ProcessUtilityInternal(PlannedStmt *pstmt,
errhint("You can manually create a database and its "
"extensions on workers.")));
}
}
else if (IsA(parsetree, CreateRoleStmt) && !EnableCreateRolePropagation)
{
ereport(NOTICE, (errmsg("not propagating CREATE ROLE/USER commands to worker"
" nodes"),
errhint("Connect to worker nodes directly to manually create all"
else if (IsA(parsetree, CreateRoleStmt) && !EnableCreateRolePropagation)
{
ereport(NOTICE, (errmsg("not propagating CREATE ROLE/USER commands to worker"
" nodes"),
errhint(
"Connect to worker nodes directly to manually create all"
" necessary users and roles.")));
}
}

/*
Expand Down
44 changes: 44 additions & 0 deletions src/test/regress/expected/pg16.out
Original file line number Diff line number Diff line change
Expand Up @@ -1009,6 +1009,50 @@ 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
-- Error out for now in Citus
--
GRANT role1 TO role2 WITH INHERIT FALSE;
ERROR: cannot propagate GRANT/REVOKE statements with specified INHERIT/SET option
GRANT role1 TO role2 WITH INHERIT TRUE;
ERROR: cannot propagate GRANT/REVOKE statements with specified INHERIT/SET option
GRANT role1 TO role2 WITH INHERIT OPTION;
ERROR: cannot propagate GRANT/REVOKE statements with specified INHERIT/SET option
GRANT role1 TO role2 WITH SET FALSE;
ERROR: cannot propagate GRANT/REVOKE statements with specified INHERIT/SET option
GRANT role1 TO role2 WITH SET TRUE;
ERROR: cannot propagate GRANT/REVOKE statements with specified INHERIT/SET option
GRANT role1 TO role2 WITH SET OPTION;
ERROR: cannot propagate GRANT/REVOKE statements with specified INHERIT/SET option
GRANT role1 TO role2 WITH ADMIN OPTION, INHERIT FALSE, SET FALSE;
ERROR: cannot propagate GRANT/REVOKE statements with specified INHERIT/SET option
-- test REVOKES as well
GRANT role1 TO role2;
REVOKE SET OPTION FOR role1 FROM role2;
ERROR: cannot propagate GRANT/REVOKE statements with specified INHERIT/SET option
REVOKE INHERIT OPTION FOR role1 FROM role2;
ERROR: cannot propagate GRANT/REVOKE statements with specified INHERIT/SET option
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;
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;
\set VERBOSITY terse
SET client_min_messages TO ERROR;
DROP EXTENSION postgres_fdw CASCADE;
Expand Down
3 changes: 3 additions & 0 deletions src/test/regress/pg_regress_multi.pl
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,9 @@ sub generate_hba

# we disable some restrictions for local objects like local views to not break postgres vanilla test behaviour.
push(@pgOptions, "citus.enforce_object_restrictions_for_local_objects=false");

# we disable propagating create role statements to not have roles as distributed objects during vanilla tests.
push(@pgOptions, "citus.enable_create_role_propagation=false");
}

if ($useMitmproxy)
Expand Down
34 changes: 34 additions & 0 deletions src/test/regress/sql/pg16.sql
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,40 @@ 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
-- Error out for now in Citus
--
GRANT role1 TO role2 WITH INHERIT FALSE;
GRANT role1 TO role2 WITH INHERIT TRUE;
GRANT role1 TO role2 WITH INHERIT OPTION;
GRANT role1 TO role2 WITH SET FALSE;
GRANT role1 TO role2 WITH SET TRUE;
GRANT role1 TO role2 WITH SET OPTION;
GRANT role1 TO role2 WITH ADMIN OPTION, INHERIT FALSE, SET FALSE;

-- 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;
Expand Down

0 comments on commit 704ce97

Please sign in to comment.