Skip to content

Commit

Permalink
Fixes granted by cascade/restrict statements for revoke (#7517)
Browse files Browse the repository at this point in the history
DESCRIPTION: Fixes incorrect propagating of `GRANTED BY` and
`CASCADE/RESTRICT` clauses for `REVOKE` statements

There are two issues fixed in this PR
1. granted by statement will appear for revoke statements as well
2. revoke/cascade statement will appear after granted by

Since granted by statements does not appear in statements, this bug
hasn't been visible until now. However, after activating the granted by
statement for revoke, order problem arised and this issue was fixed
order problem for cascade/revoke as well
In summary, this PR provides usage of granted by statements properly now
with the correct order of statements.
We can verify the both errors, fixed with just single statement
REVOKE dist_role_3 from non_dist_role_3 granted by test_admin_role
cascade;
  • Loading branch information
gurkanindibay committed Feb 19, 2024
1 parent 74b55d0 commit 9a0cdbf
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 6 deletions.
2 changes: 1 addition & 1 deletion src/backend/distributed/deparser/citus_grantutils.c
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ AppendGrantRestrictAndCascade(StringInfo buf, GrantStmt *stmt)
void
AppendGrantedByInGrantForRoleSpec(StringInfo buf, RoleSpec *grantor, bool isGrant)
{
if (isGrant && grantor)
if (grantor)
{
appendStringInfo(buf, " GRANTED BY %s", RoleSpecString(grantor, true));
}
Expand Down
2 changes: 1 addition & 1 deletion src/backend/distributed/deparser/deparse_role_stmts.c
Original file line number Diff line number Diff line change
Expand Up @@ -486,8 +486,8 @@ AppendGrantRoleStmt(StringInfo buf, GrantRoleStmt *stmt)
appendStringInfo(buf, "%s ", stmt->is_grant ? " TO " : " FROM ");
AppendRoleList(buf, stmt->grantee_roles);
AppendGrantWithAdminOption(buf, stmt);
AppendGrantRestrictAndCascadeForRoleSpec(buf, stmt->behavior, stmt->is_grant);
AppendGrantedByInGrantForRoleSpec(buf, stmt->grantor, stmt->is_grant);
AppendGrantRestrictAndCascadeForRoleSpec(buf, stmt->behavior, stmt->is_grant);
appendStringInfo(buf, ";");
}

Expand Down
37 changes: 35 additions & 2 deletions src/test/regress/expected/create_role_propagation.out
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,24 @@ SELECT result FROM run_command_on_all_nodes(
{"member":"test_admin_role","role":"dist_role_3","grantor":"postgres","admin_option":true}]
(3 rows)

REVOKE dist_role_3 from dist_role_4 granted by test_admin_role;
REVOKE dist_role_3 from dist_role_4 granted by test_admin_role cascade;
SELECT result FROM run_command_on_all_nodes(
$$
SELECT json_agg(q.* ORDER BY member) FROM (
SELECT member::regrole::text, roleid::regrole::text AS role, grantor::regrole::text, admin_option
FROM pg_auth_members WHERE roleid::regrole::text = 'dist_role_3'
order by member::regrole::text
) q;
$$
);
result
---------------------------------------------------------------------
[{"member":"non_dist_role_3","role":"dist_role_3","grantor":"test_admin_role","admin_option":false}, +
{"member":"test_admin_role","role":"dist_role_3","grantor":"postgres","admin_option":true}]
[{"member":"test_admin_role","role":"dist_role_3","grantor":"postgres","admin_option":true}]
[{"member":"test_admin_role","role":"dist_role_3","grantor":"postgres","admin_option":true}]
(3 rows)

SELECT roleid::regrole::text AS role, member::regrole::text, (grantor::regrole::text IN ('postgres', 'non_dist_role_1', 'dist_role_1','test_admin_role')) AS grantor, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE '%dist\_%' ORDER BY 1, 2;
role | member | grantor | admin_option
---------------------------------------------------------------------
Expand All @@ -282,7 +299,23 @@ SELECT objid::regrole FROM pg_catalog.pg_dist_object WHERE classid='pg_authid'::
non_dist_role_4
(5 rows)

REVOKE dist_role_3 from non_dist_role_3 granted by test_admin_role;
REVOKE dist_role_3 from non_dist_role_3 granted by test_admin_role cascade;
SELECT result FROM run_command_on_all_nodes(
$$
SELECT json_agg(q.* ORDER BY member) FROM (
SELECT member::regrole::text, roleid::regrole::text AS role, grantor::regrole::text, admin_option
FROM pg_auth_members WHERE roleid::regrole::text = 'dist_role_3'
order by member::regrole::text
) q;
$$
);
result
---------------------------------------------------------------------
[{"member":"test_admin_role","role":"dist_role_3","grantor":"postgres","admin_option":true}]
[{"member":"test_admin_role","role":"dist_role_3","grantor":"postgres","admin_option":true}]
[{"member":"test_admin_role","role":"dist_role_3","grantor":"postgres","admin_option":true}]
(3 rows)

revoke dist_role_3,dist_role_1 from test_admin_role cascade;
drop role test_admin_role;
\c - - - :worker_1_port
Expand Down
24 changes: 22 additions & 2 deletions src/test/regress/sql/create_role_propagation.sql
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,32 @@ SELECT result FROM run_command_on_all_nodes(
$$
);

REVOKE dist_role_3 from dist_role_4 granted by test_admin_role;
REVOKE dist_role_3 from dist_role_4 granted by test_admin_role cascade;

SELECT result FROM run_command_on_all_nodes(
$$
SELECT json_agg(q.* ORDER BY member) FROM (
SELECT member::regrole::text, roleid::regrole::text AS role, grantor::regrole::text, admin_option
FROM pg_auth_members WHERE roleid::regrole::text = 'dist_role_3'
order by member::regrole::text
) q;
$$
);

SELECT roleid::regrole::text AS role, member::regrole::text, (grantor::regrole::text IN ('postgres', 'non_dist_role_1', 'dist_role_1','test_admin_role')) AS grantor, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE '%dist\_%' ORDER BY 1, 2;
SELECT objid::regrole FROM pg_catalog.pg_dist_object WHERE classid='pg_authid'::regclass::oid AND objid::regrole::text LIKE '%dist\_%' ORDER BY 1;

REVOKE dist_role_3 from non_dist_role_3 granted by test_admin_role;
REVOKE dist_role_3 from non_dist_role_3 granted by test_admin_role cascade;

SELECT result FROM run_command_on_all_nodes(
$$
SELECT json_agg(q.* ORDER BY member) FROM (
SELECT member::regrole::text, roleid::regrole::text AS role, grantor::regrole::text, admin_option
FROM pg_auth_members WHERE roleid::regrole::text = 'dist_role_3'
order by member::regrole::text
) q;
$$
);

revoke dist_role_3,dist_role_1 from test_admin_role cascade;
drop role test_admin_role;
Expand Down

0 comments on commit 9a0cdbf

Please sign in to comment.