From e94327703cb0b4c9ce08da123588a66bfb876146 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Fri, 10 Jun 2022 16:29:10 -0400 Subject: [PATCH v2] Add a grant-level INHERIT option to override the role-level property. --- doc/src/sgml/catalogs.sgml | 11 ++ doc/src/sgml/ref/create_role.sgml | 20 +-- doc/src/sgml/ref/grant.sgml | 28 +++- doc/src/sgml/ref/revoke.sgml | 5 +- src/backend/commands/user.c | 204 ++++++++++++++++++----- src/backend/nodes/copyfuncs.c | 2 +- src/backend/nodes/equalfuncs.c | 2 +- src/backend/parser/gram.y | 50 +++++- src/backend/tcop/utility.c | 2 +- src/backend/utils/adt/acl.c | 49 ++++-- src/bin/pg_dump/pg_dumpall.c | 25 ++- src/include/catalog/pg_auth_members.h | 13 ++ src/include/commands/user.h | 2 +- src/include/nodes/parsenodes.h | 2 +- src/test/regress/expected/privileges.out | 22 ++- src/test/regress/sql/privileges.sql | 22 ++- 16 files changed, 367 insertions(+), 92 deletions(-) diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 25b02c4e37..0eee9fd692 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -1708,6 +1708,17 @@ SCRAM-SHA-256$<iteration count>:&l roleid to others + + + + inherit_option bool + + + t = privileges are automatically inherited, + f = privileges are not automatically inherited, + d = privileges are automatically inherited if the member has rolinherit = true + + diff --git a/doc/src/sgml/ref/create_role.sgml b/doc/src/sgml/ref/create_role.sgml index b6a4ea1f72..6ce6c9481b 100644 --- a/doc/src/sgml/ref/create_role.sgml +++ b/doc/src/sgml/ref/create_role.sgml @@ -133,16 +133,16 @@ in sync when changing the above synopsis! NOINHERIT - These clauses determine whether a role inherits the - privileges of roles it is a member of. - A role with the INHERIT attribute can automatically - use whatever database privileges have been granted to all roles - it is directly or indirectly a member of. - Without INHERIT, membership in another role - only grants the ability to SET ROLE to that other role; - the privileges of the other role are only available after having - done so. - If not specified, + These clauses determine whether a role will by default + inherit the privileges of roles it is a member of. + Regardless of the value of this setting, privileges for a + GRANT statement that specifies + WITH INHERIT TRUE will be inherited, and privileges + for a GRANT statement that specifies + WITH INHERIT FALSE will not be inherited. + For a GRANT that uses + WITH INHERIT DEFAULT, privileges will be inherited + according to how this property is set for the member role. INHERIT is the default. diff --git a/doc/src/sgml/ref/grant.sgml b/doc/src/sgml/ref/grant.sgml index f744b05b55..c3193c8227 100644 --- a/doc/src/sgml/ref/grant.sgml +++ b/doc/src/sgml/ref/grant.sgml @@ -98,7 +98,7 @@ GRANT { USAGE | ALL [ PRIVILEGES ] } [ GRANTED BY role_specification ] GRANT role_name [, ...] TO role_specification [, ...] - [ WITH ADMIN OPTION ] + [ WITH { ADMIN | INHERIT } { OPTION | TRUE | FALSE | DEFUALT } ] [ GRANTED BY role_specification ] where role_specification can be: @@ -255,7 +255,18 @@ GRANT role_name [, ...] TO - If WITH ADMIN OPTION is specified, the member can + The effect of membership in a role can be modified by specifying the + ADMIN or INHERIT option. + Either of these options can be set to TRUE or + FALSE; the INHERIT option can + also be set to DEFAULT. The keyword + OPTION is accepted as a synonym for + TRUE, so that WITH ADMIN OPTION + is a synonym for WITH ADMIN TRUE. + + + + The ADMIN option allows the member to in turn grant membership in the role to others, and revoke membership in the role as well. Without the admin option, ordinary users cannot do that. A role is not considered to hold WITH ADMIN @@ -265,6 +276,19 @@ GRANT role_name [, ...] TO + + The INHERIT option, if it is set to + TRUE, causes the member to inherit the privileges of + the granted role. That is, it can automatically use whatever database + privileges have been granted to that role. If set to + FALSE, the member does not inherit the privileges + of the granted role. If set to DEFAULT, the behavior + depends on the attributes of the member role: privileges are inherited + if the member role is set to INHERIT but not if the + member role is set to NOINHERIT. + See CREATE ROLE. + + If GRANTED BY is specified, the grant is recorded as having been done by the specified role. Only database superusers may diff --git a/doc/src/sgml/ref/revoke.sgml b/doc/src/sgml/ref/revoke.sgml index 62f1971036..7a83a087c3 100644 --- a/doc/src/sgml/ref/revoke.sgml +++ b/doc/src/sgml/ref/revoke.sgml @@ -125,7 +125,7 @@ REVOKE [ GRANT OPTION FOR ] [ GRANTED BY role_specification ] [ CASCADE | RESTRICT ] -REVOKE [ ADMIN OPTION FOR ] +REVOKE [ { ADMIN | INHERIT } OPTION FOR ] role_name [, ...] FROM role_specification [, ...] [ GRANTED BY role_specification ] [ CASCADE | RESTRICT ] @@ -198,6 +198,9 @@ REVOKE [ ADMIN OPTION FOR ] When revoking membership in a role, GRANT OPTION is instead called ADMIN OPTION, but the behavior is similar. + It is also possible to revoke the INHERIT OPTION, + which is equivalent to setting the value of that option to + FALSE. This form of the command also allows a GRANTED BY option, but that option is currently ignored (except for checking the existence of the named role). diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c index 984305ba31..b92e9a8481 100644 --- a/src/backend/commands/user.c +++ b/src/backend/commands/user.c @@ -42,6 +42,15 @@ /* Potentially set by pg_upgrade_support functions */ Oid binary_upgrade_next_pg_authid_oid = InvalidOid; +typedef struct +{ + unsigned specified; + bool admin; + char inherit; +} GrantRoleOptions; + +#define GRANT_ROLE_SPECIFIED_ADMIN 0x0001 +#define GRANT_ROLE_SPECIFIED_INHERIT 0x0002 /* GUC parameter */ int Password_encryption = PASSWORD_TYPE_SCRAM_SHA_256; @@ -51,10 +60,11 @@ check_password_hook_type check_password_hook = NULL; static void AddRoleMems(const char *rolename, Oid roleid, List *memberSpecs, List *memberIds, - Oid grantorId, bool admin_opt); + Oid grantorId, GrantRoleOptions *popt); static void DelRoleMems(const char *rolename, Oid roleid, List *memberSpecs, List *memberIds, - bool admin_opt); + GrantRoleOptions *popt); +static void InitGrantRoleOptions(GrantRoleOptions *popt); /* Check if current user has createrole privileges */ @@ -107,6 +117,7 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) DefElem *dadminmembers = NULL; DefElem *dvalidUntil = NULL; DefElem *dbypassRLS = NULL; + GrantRoleOptions popt; /* The defaults can vary depending on the original statement type */ switch (stmt->stmt_type) @@ -429,6 +440,9 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) if (addroleto || adminmembers || rolemembers) CommandCounterIncrement(); + /* Default grant. */ + InitGrantRoleOptions(&popt); + /* * Add the new role to the specified existing roles. */ @@ -453,7 +467,7 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) AddRoleMems(oldrolename, oldroleid, thisrole_list, thisrole_oidlist, - GetUserId(), false); + GetUserId(), &popt); ReleaseSysCache(oldroletup); } @@ -463,12 +477,14 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) * Add the specified members to this new role. adminmembers get the admin * option, rolemembers don't. */ - AddRoleMems(stmt->role, roleid, - adminmembers, roleSpecsToIds(adminmembers), - GetUserId(), true); AddRoleMems(stmt->role, roleid, rolemembers, roleSpecsToIds(rolemembers), - GetUserId(), false); + GetUserId(), &popt); + popt.specified |= GRANT_ROLE_SPECIFIED_ADMIN; + popt.admin = true; + AddRoleMems(stmt->role, roleid, + adminmembers, roleSpecsToIds(adminmembers), + GetUserId(), &popt); /* Post creation hook for new role */ InvokeObjectPostCreateHook(AuthIdRelationId, roleid, 0); @@ -519,6 +535,7 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt) DefElem *dvalidUntil = NULL; DefElem *dbypassRLS = NULL; Oid roleid; + GrantRoleOptions popt; check_rolespec_name(stmt->role, "Cannot alter reserved roles."); @@ -792,6 +809,8 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt) ReleaseSysCache(tuple); heap_freetuple(new_tuple); + InitGrantRoleOptions(&popt); + /* * Advance command counter so we can see new record; else tests in * AddRoleMems may fail. @@ -805,11 +824,11 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt) if (stmt->action == +1) /* add members to role */ AddRoleMems(rolename, roleid, rolemembers, roleSpecsToIds(rolemembers), - GetUserId(), false); + GetUserId(), &popt); else if (stmt->action == -1) /* drop members from role */ DelRoleMems(rolename, roleid, rolemembers, roleSpecsToIds(rolemembers), - false); + &popt); } /* @@ -1224,13 +1243,59 @@ RenameRole(const char *oldname, const char *newname) * Grant/Revoke roles to/from roles */ void -GrantRole(GrantRoleStmt *stmt) +GrantRole(ParseState *pstate, GrantRoleStmt *stmt) { Relation pg_authid_rel; Oid grantor; List *grantee_ids; ListCell *item; + GrantRoleOptions popt; + + /* Parse options list. */ + InitGrantRoleOptions(&popt); + foreach(item, stmt->opt) + { + DefElem *opt = (DefElem *) lfirst(item); + char *optval = defGetString(opt); + + if (strcmp(opt->defname, "admin") == 0) + { + popt.specified |= GRANT_ROLE_SPECIFIED_ADMIN; + + if (parse_bool(optval, &popt.admin)) + continue; + } + else if (strcmp(opt->defname, "inherit") == 0) + { + bool boolval; + + popt.specified |= GRANT_ROLE_SPECIFIED_INHERIT; + if (parse_bool(optval, &boolval)) + { + popt.inherit = boolval ? ROLE_GRANT_INHERIT_TRUE + : ROLE_GRANT_INHERIT_FALSE; + continue; + } + else if (strcmp(optval, "default") == 0) + { + popt.inherit = ROLE_GRANT_INHERIT_DEFAULT; + continue; + } + } + else + ereport(ERROR, + errcode(ERRCODE_SYNTAX_ERROR), + errmsg("unrecognized role option \"%s\"", opt->defname), + parser_errposition(pstate, opt->location)); + + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("unrecognized value for role option \"%s\": \"%s\"", + opt->defname, optval), + parser_errposition(pstate, opt->location))); + } + /* Determine grantor. */ if (stmt->grantor) grantor = get_rolespec_oid(stmt->grantor, false); else @@ -1243,8 +1308,7 @@ GrantRole(GrantRoleStmt *stmt) /* * Step through all of the granted roles and add/remove entries for the - * grantees, or, if admin_opt is set, then just add/remove the admin - * option. + * grantees, or, if opt != NIL, then just add/remove the named option(s). * * Note: Permissions checking is done by AddRoleMems/DelRoleMems */ @@ -1264,11 +1328,11 @@ GrantRole(GrantRoleStmt *stmt) if (stmt->is_grant) AddRoleMems(rolename, roleid, stmt->grantee_roles, grantee_ids, - grantor, stmt->admin_opt); + grantor, &popt); else DelRoleMems(rolename, roleid, stmt->grantee_roles, grantee_ids, - stmt->admin_opt); + &popt); } /* @@ -1375,7 +1439,7 @@ roleSpecsToIds(List *memberNames) static void AddRoleMems(const char *rolename, Oid roleid, List *memberSpecs, List *memberIds, - Oid grantorId, bool admin_opt) + Oid grantorId, GrantRoleOptions *popt) { Relation pg_authmem_rel; TupleDesc pg_authmem_dsc; @@ -1481,38 +1545,63 @@ AddRoleMems(const char *rolename, Oid roleid, errmsg("role \"%s\" is a member of role \"%s\"", rolename, get_rolespec_name(memberRole)))); - /* - * Check if entry for this role/member already exists; if so, give - * warning unless we are adding admin option. - */ - authmem_tuple = SearchSysCache2(AUTHMEMROLEMEM, - ObjectIdGetDatum(roleid), - ObjectIdGetDatum(memberid)); - if (HeapTupleIsValid(authmem_tuple) && - (!admin_opt || - ((Form_pg_auth_members) GETSTRUCT(authmem_tuple))->admin_option)) - { - ereport(NOTICE, - (errmsg("role \"%s\" is already a member of role \"%s\"", - get_rolespec_name(memberRole), rolename))); - ReleaseSysCache(authmem_tuple); - continue; - } - - /* Build a tuple to insert or update */ + /* Initialize bookkeeping for possible insert or update */ MemSet(new_record, 0, sizeof(new_record)); MemSet(new_record_nulls, false, sizeof(new_record_nulls)); MemSet(new_record_repl, false, sizeof(new_record_repl)); + new_record[Anum_pg_auth_members_roleid - 1] = + ObjectIdGetDatum(roleid); + new_record[Anum_pg_auth_members_member - 1] = + ObjectIdGetDatum(memberid); + new_record[Anum_pg_auth_members_grantor - 1] = + ObjectIdGetDatum(grantorId); + new_record[Anum_pg_auth_members_admin_option - 1] = + BoolGetDatum(popt->admin); + new_record[Anum_pg_auth_members_inherit_option - 1] = + CharGetDatum(popt->inherit); + + /* Find any existing tuple */ + authmem_tuple = SearchSysCache2(AUTHMEMROLEMEM, + ObjectIdGetDatum(roleid), + ObjectIdGetDatum(memberid)); - new_record[Anum_pg_auth_members_roleid - 1] = ObjectIdGetDatum(roleid); - new_record[Anum_pg_auth_members_member - 1] = ObjectIdGetDatum(memberid); - new_record[Anum_pg_auth_members_grantor - 1] = ObjectIdGetDatum(grantorId); - new_record[Anum_pg_auth_members_admin_option - 1] = BoolGetDatum(admin_opt); + /* + * If we found a tuple, update it with new option values, unless + * there are no changes, in which case issue a WARNING. + * + * If we didn't find a tuple, just insert one. + */ if (HeapTupleIsValid(authmem_tuple)) { - new_record_repl[Anum_pg_auth_members_grantor - 1] = true; - new_record_repl[Anum_pg_auth_members_admin_option - 1] = true; + Form_pg_auth_members form; + bool at_least_one_change = false; + + form = (Form_pg_auth_members) GETSTRUCT(authmem_tuple); + + if (form->admin_option != popt->admin) + { + new_record_repl[Anum_pg_auth_members_admin_option - 1] = + (popt->specified & GRANT_ROLE_SPECIFIED_ADMIN) != 0; + at_least_one_change = true; + } + + if (form->inherit_option != popt->inherit) + { + new_record_repl[Anum_pg_auth_members_inherit_option - 1] = + (popt->specified & GRANT_ROLE_SPECIFIED_INHERIT) != 0; + at_least_one_change = true; + } + + if (!at_least_one_change) + { + ereport(NOTICE, + (errmsg("role \"%s\" is already a member of role \"%s\"", + get_rolespec_name(memberRole), rolename))); + ReleaseSysCache(authmem_tuple); + continue; + } + tuple = heap_modify_tuple(authmem_tuple, pg_authmem_dsc, new_record, new_record_nulls, new_record_repl); @@ -1548,7 +1637,7 @@ AddRoleMems(const char *rolename, Oid roleid, static void DelRoleMems(const char *rolename, Oid roleid, List *memberSpecs, List *memberIds, - bool admin_opt) + GrantRoleOptions *popt) { Relation pg_authmem_rel; TupleDesc pg_authmem_dsc; @@ -1605,14 +1694,14 @@ DelRoleMems(const char *rolename, Oid roleid, continue; } - if (!admin_opt) + if (popt->specified == 0) { /* Remove the entry altogether */ CatalogTupleDelete(pg_authmem_rel, &authmem_tuple->t_self); } else { - /* Just turn off the admin option */ + /* Just turn off the specified option */ HeapTuple tuple; Datum new_record[Natts_pg_auth_members]; bool new_record_nulls[Natts_pg_auth_members]; @@ -1623,8 +1712,22 @@ DelRoleMems(const char *rolename, Oid roleid, MemSet(new_record_nulls, false, sizeof(new_record_nulls)); MemSet(new_record_repl, false, sizeof(new_record_repl)); - new_record[Anum_pg_auth_members_admin_option - 1] = BoolGetDatum(false); - new_record_repl[Anum_pg_auth_members_admin_option - 1] = true; + if ((popt->specified & GRANT_ROLE_SPECIFIED_ADMIN) != 0) + { + new_record[Anum_pg_auth_members_admin_option - 1] = + BoolGetDatum(false); + new_record_repl[Anum_pg_auth_members_admin_option - 1] = + true; + } + else if ((popt->specified & GRANT_ROLE_SPECIFIED_INHERIT) != 0) + { + new_record[Anum_pg_auth_members_inherit_option - 1] = + CharGetDatum(ROLE_GRANT_INHERIT_FALSE); + new_record_repl[Anum_pg_auth_members_inherit_option - 1] = + true; + } + else + elog(ERROR, "no role option to revoke?"); tuple = heap_modify_tuple(authmem_tuple, pg_authmem_dsc, new_record, @@ -1643,3 +1746,14 @@ DelRoleMems(const char *rolename, Oid roleid, */ table_close(pg_authmem_rel, NoLock); } + +/* + * Initialize a GrantRoleOptions object with default values. + */ +static void +InitGrantRoleOptions(GrantRoleOptions *popt) +{ + popt->specified = 0; + popt->admin = false; + popt->inherit = ROLE_GRANT_INHERIT_DEFAULT; +} diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 51d630fa89..135da1a8a8 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -3971,7 +3971,7 @@ _copyGrantRoleStmt(const GrantRoleStmt *from) COPY_NODE_FIELD(granted_roles); COPY_NODE_FIELD(grantee_roles); COPY_SCALAR_FIELD(is_grant); - COPY_SCALAR_FIELD(admin_opt); + COPY_NODE_FIELD(opt); COPY_NODE_FIELD(grantor); COPY_SCALAR_FIELD(behavior); diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index e747e1667d..e7f72a3c11 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -1572,7 +1572,7 @@ _equalGrantRoleStmt(const GrantRoleStmt *a, const GrantRoleStmt *b) COMPARE_NODE_FIELD(granted_roles); COMPARE_NODE_FIELD(grantee_roles); COMPARE_SCALAR_FIELD(is_grant); - COMPARE_SCALAR_FIELD(admin_opt); + COMPARE_NODE_FIELD(opt); COMPARE_NODE_FIELD(grantor); COMPARE_SCALAR_FIELD(behavior); diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 969c9c158f..18e02f5bf1 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -359,9 +359,12 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); %type utility_option_arg %type drop_option %type opt_or_replace opt_no - opt_grant_grant_option opt_grant_admin_option + opt_grant_grant_option opt_nowait opt_if_exists opt_with_data opt_transaction_chain +%type grant_role_opt_list +%type grant_role_opt +%type grant_role_opt_value %type opt_nowait_or_skip %type OptRoleList AlterOptRoleList @@ -7818,15 +7821,26 @@ opt_grant_grant_option: *****************************************************************************/ GrantRoleStmt: - GRANT privilege_list TO role_list opt_grant_admin_option opt_granted_by + GRANT privilege_list TO role_list opt_granted_by { GrantRoleStmt *n = makeNode(GrantRoleStmt); n->is_grant = true; n->granted_roles = $2; n->grantee_roles = $4; - n->admin_opt = $5; - n->grantor = $6; + n->opt = NIL; + n->grantor = $5; + $$ = (Node *) n; + } + | GRANT privilege_list TO role_list WITH grant_role_opt_list opt_granted_by + { + GrantRoleStmt *n = makeNode(GrantRoleStmt); + + n->is_grant = true; + n->granted_roles = $2; + n->grantee_roles = $4; + n->opt = $6; + n->grantor = $7; $$ = (Node *) n; } ; @@ -7837,18 +7851,21 @@ RevokeRoleStmt: GrantRoleStmt *n = makeNode(GrantRoleStmt); n->is_grant = false; - n->admin_opt = false; + n->opt = NIL; n->granted_roles = $2; n->grantee_roles = $4; n->behavior = $6; $$ = (Node *) n; } - | REVOKE ADMIN OPTION FOR privilege_list FROM role_list opt_granted_by opt_drop_behavior + | REVOKE ColId OPTION FOR privilege_list FROM role_list opt_granted_by opt_drop_behavior { GrantRoleStmt *n = makeNode(GrantRoleStmt); + DefElem *opt; + opt = makeDefElem(pstrdup($2), + (Node *) makeBoolean(false), @2); n->is_grant = false; - n->admin_opt = true; + n->opt = list_make1(opt); n->granted_roles = $5; n->grantee_roles = $7; n->behavior = $9; @@ -7856,8 +7873,23 @@ RevokeRoleStmt: } ; -opt_grant_admin_option: WITH ADMIN OPTION { $$ = true; } - | /*EMPTY*/ { $$ = false; } +grant_role_opt_list: + grant_role_opt_list ',' grant_role_opt { $$ = lappend($1, $3); } + | grant_role_opt { $$ = list_make1($1); } + ; + +grant_role_opt: + ColLabel grant_role_opt_value + { + $$ = makeDefElem(pstrdup($1), $2, @1); + } + ; + +grant_role_opt_value: + OPTION { $$ = (Node *) makeBoolean(true); } + | TRUE_P { $$ = (Node *) makeBoolean(true); } + | FALSE_P { $$ = (Node *) makeBoolean(false); } + | DEFAULT { $$ = (Node *) makeString(pstrdup($1)); } ; opt_granted_by: GRANTED BY RoleSpec { $$ = $3; } diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 6a5bcded55..d61645ddbb 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -767,7 +767,7 @@ standard_ProcessUtility(PlannedStmt *pstmt, case T_GrantRoleStmt: /* no event triggers for global objects */ - GrantRole((GrantRoleStmt *) parsetree); + GrantRole(pstate, (GrantRoleStmt *) parsetree); break; case T_CreatedbStmt: diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c index 772c04155c..f3c844443c 100644 --- a/src/backend/utils/adt/acl.c +++ b/src/backend/utils/adt/acl.c @@ -66,7 +66,7 @@ typedef struct */ enum RoleRecurseType { - ROLERECURSE_PRIVS = 0, /* recurse if rolinherit */ + ROLERECURSE_PRIVS = 0, /* recurse through inheritable grants */ ROLERECURSE_MEMBERS = 1 /* recurse unconditionally */ }; static Oid cached_role[] = {InvalidOid, InvalidOid}; @@ -4818,8 +4818,8 @@ has_rolinherit(Oid roleid) /* * Get a list of roles that the specified roleid is a member of * - * Type ROLERECURSE_PRIVS recurses only through roles that have rolinherit - * set, while ROLERECURSE_MEMBERS recurses through all roles. This sets + * Type ROLERECURSE_PRIVS recurses only through grants that have 'inherit' + * set, while ROLERECURSE_MEMBERS recurses through all grants. This sets * *is_admin==true if and only if role "roleid" has an ADMIN OPTION membership * in role "admin_of". * @@ -4883,9 +4883,8 @@ roles_is_member_of(Oid roleid, enum RoleRecurseType type, Oid memberid = lfirst_oid(l); CatCList *memlist; int i; - - if (type == ROLERECURSE_PRIVS && !has_rolinherit(memberid)) - continue; /* ignore non-inheriting roles */ + bool rolinherit = false; /* placate compiler */ + bool rolinherit_is_set = false; /* Find roles that memberid is directly a member of */ memlist = SearchSysCacheList1(AUTHMEMMEMROLE, @@ -4893,14 +4892,40 @@ roles_is_member_of(Oid roleid, enum RoleRecurseType type, for (i = 0; i < memlist->n_members; i++) { HeapTuple tup = &memlist->members[i]->tuple; - Oid otherid = ((Form_pg_auth_members) GETSTRUCT(tup))->roleid; + Form_pg_auth_members form = (Form_pg_auth_members) GETSTRUCT(tup); + Oid otherid = form->roleid; + + /* + * If we're supposed to ignore non-heritable grants, do so. + * + * Any given GRANT might be WITH INHERIT FALSE, in which case it + * is not inherited regardless of anything else, or it might be + * WITH INHERIT TRUE, in which case it definitely is inherited, + * or it might be WITH INHERIT DEFAULT, in which case it depends + * on whether the role is INHERIT or NOINHERIT. + */ + if (type == ROLERECURSE_PRIVS) + { + if (form->inherit_option == ROLE_GRANT_INHERIT_FALSE) + continue; + if (form->inherit_option == ROLE_GRANT_INHERIT_DEFAULT) + { + /* Avoid repeated catalog lookups. */ + if (!rolinherit_is_set) + { + rolinherit = has_rolinherit(memberid); + rolinherit_is_set = true; + } + if (!rolinherit) + continue; + } + } /* * While otherid==InvalidOid shouldn't appear in the catalog, the * OidIsValid() avoids crashing if that arises. */ - if (otherid == admin_of && - ((Form_pg_auth_members) GETSTRUCT(tup))->admin_option && + if (otherid == admin_of && form->admin_option && OidIsValid(admin_of)) *is_admin = true; @@ -4943,8 +4968,8 @@ roles_is_member_of(Oid roleid, enum RoleRecurseType type, /* * Does member have the privileges of role (directly or indirectly)? * - * This is defined not to recurse through roles that don't have rolinherit - * set; for such roles, membership implies the ability to do SET ROLE, but + * This is defined not to recurse through grants that are not inherited; + * in such cases, membership implies the ability to do SET ROLE, but * the privileges are not available until you've done so. */ bool @@ -4971,7 +4996,7 @@ has_privs_of_role(Oid member, Oid role) /* * Is member a member of role (directly or indirectly)? * - * This is defined to recurse through roles regardless of rolinherit. + * This is defined to recurse through grants whether they are inherited or not. * * Do not use this for privilege checking, instead use has_privs_of_role() */ diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c index ae41a652d7..5914165038 100644 --- a/src/bin/pg_dump/pg_dumpall.c +++ b/src/bin/pg_dump/pg_dumpall.c @@ -926,11 +926,16 @@ dumpRoleMembership(PGconn *conn) PQExpBuffer buf = createPQExpBuffer(); PGresult *res; int i; + bool server_has_inherit_option; + + server_has_inherit_option = (server_version >= 160000); printfPQExpBuffer(buf, "SELECT ur.rolname AS roleid, " "um.rolname AS member, " - "a.admin_option, " - "ug.rolname AS grantor " + "a.admin_option"); + if (server_has_inherit_option) + appendPQExpBuffer(buf, ", a.inherit_option"); + appendPQExpBuffer(buf, ", ug.rolname AS grantor " "FROM pg_auth_members a " "LEFT JOIN %s ur on ur.oid = a.roleid " "LEFT JOIN %s um on um.oid = a.member " @@ -946,20 +951,28 @@ dumpRoleMembership(PGconn *conn) { char *roleid = PQgetvalue(res, i, 0); char *member = PQgetvalue(res, i, 1); - char *option = PQgetvalue(res, i, 2); + char *admin_option = PQgetvalue(res, i, 2); fprintf(OPF, "GRANT %s", fmtId(roleid)); fprintf(OPF, " TO %s", fmtId(member)); - if (*option == 't') + if (*admin_option == 't') fprintf(OPF, " WITH ADMIN OPTION"); + if (server_has_inherit_option) + { + char *inherit_option = PQgetvalue(res, i, 3); + if (*inherit_option == 't') + fprintf(OPF, " WITH INHERIT TRUE"); + else if (*inherit_option == 'f') + fprintf(OPF, " WITH INHERIT FALSE"); + } /* * We don't track the grantor very carefully in the backend, so cope * with the possibility that it has been dropped. */ - if (!PQgetisnull(res, i, 3)) + if (!PQgetisnull(res, i, 4)) { - char *grantor = PQgetvalue(res, i, 3); + char *grantor = PQgetvalue(res, i, 4); fprintf(OPF, " GRANTED BY %s", fmtId(grantor)); } diff --git a/src/include/catalog/pg_auth_members.h b/src/include/catalog/pg_auth_members.h index 1bc027f133..27a99aa4b6 100644 --- a/src/include/catalog/pg_auth_members.h +++ b/src/include/catalog/pg_auth_members.h @@ -33,8 +33,21 @@ CATALOG(pg_auth_members,1261,AuthMemRelationId) BKI_SHARED_RELATION BKI_ROWTYPE_ Oid member BKI_LOOKUP(pg_authid); /* ID of a member of that role */ Oid grantor BKI_LOOKUP(pg_authid); /* who granted the membership */ bool admin_option; /* granted with admin option? */ + char inherit_option; /* exercise privileges without SET ROLE? */ } FormData_pg_auth_members; +/* + * Does this grant allow the granted role's privileges to be exercised without + * executing SET ROLE? + * + * If ROLE_GRANT_INHERIT_DEFAULT is used, then the answer will be yes if the + * role is marked INHERIT and no if the role is marked NOINHERIT. If either + * of the other values is used, it takes precedence over the role-level flag. + */ +#define ROLE_GRANT_INHERIT_TRUE 't' +#define ROLE_GRANT_INHERIT_FALSE 'f' +#define ROLE_GRANT_INHERIT_DEFAULT 'd' + /* ---------------- * Form_pg_auth_members corresponds to a pointer to a tuple with * the format of pg_auth_members relation. diff --git a/src/include/commands/user.h b/src/include/commands/user.h index d3dd8303d2..54c720d880 100644 --- a/src/include/commands/user.h +++ b/src/include/commands/user.h @@ -28,7 +28,7 @@ extern Oid CreateRole(ParseState *pstate, CreateRoleStmt *stmt); extern Oid AlterRole(ParseState *pstate, AlterRoleStmt *stmt); extern Oid AlterRoleSet(AlterRoleSetStmt *stmt); extern void DropRole(DropRoleStmt *stmt); -extern void GrantRole(GrantRoleStmt *stmt); +extern void GrantRole(ParseState *pstate, GrantRoleStmt *stmt); extern ObjectAddress RenameRole(const char *oldname, const char *newname); extern void DropOwnedObjects(DropOwnedStmt *stmt); extern void ReassignOwnedObjects(ReassignOwnedStmt *stmt); diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 73f635b455..3a1d21d9ff 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2439,7 +2439,7 @@ typedef struct GrantRoleStmt List *granted_roles; /* list of roles to be granted/revoked */ List *grantee_roles; /* list of member roles to add/delete */ bool is_grant; /* true = GRANT, false = REVOKE */ - bool admin_opt; /* with admin option */ + List *opt; /* options e.g. WITH GRANT OPTION */ RoleSpec *grantor; /* set grantor to other than current role */ DropBehavior behavior; /* drop behavior (for REVOKE) */ } GrantRoleStmt; diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out index 03df567d50..a24ce12f1a 100644 --- a/src/test/regress/expected/privileges.out +++ b/src/test/regress/expected/privileges.out @@ -422,7 +422,27 @@ BEGIN; RESET SESSION AUTHORIZATION; ALTER ROLE regress_priv_user1 NOINHERIT; SET SESSION AUTHORIZATION regress_priv_user1; -DELETE FROM atest3; +SAVEPOINT s1; +DELETE FROM atest3; -- fail because NOINHERIT +ERROR: permission denied for table atest3 +ROLLBACK TO s1; +RESET SESSION AUTHORIZATION; +GRANT regress_priv_group2 TO regress_priv_user1 WITH INHERIT TRUE; +SET SESSION AUTHORIZATION regress_priv_user1; +DELETE FROM atest3; -- ok because grant option overrides noinherit +ROLLBACK; +BEGIN; +RESET SESSION AUTHORIZATION; +REVOKE INHERIT OPTION FOR regress_priv_group2 FROM regress_priv_user1; +SET SESSION AUTHORIZATION regress_priv_user1; +DELETE FROM atest3; -- fail because grant option overrides inherit +ERROR: permission denied for table atest3 +ROLLBACK; +BEGIN; +RESET SESSION AUTHORIZATION; +GRANT regress_priv_group2 TO regress_priv_user1 WITH INHERIT FALSE; +SET SESSION AUTHORIZATION regress_priv_user1; +DELETE FROM atest3; -- as above, but change grant option using other syntax ERROR: permission denied for table atest3 ROLLBACK; -- views diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql index 2a6ba38e52..83353953a0 100644 --- a/src/test/regress/sql/privileges.sql +++ b/src/test/regress/sql/privileges.sql @@ -288,7 +288,27 @@ BEGIN; RESET SESSION AUTHORIZATION; ALTER ROLE regress_priv_user1 NOINHERIT; SET SESSION AUTHORIZATION regress_priv_user1; -DELETE FROM atest3; +SAVEPOINT s1; +DELETE FROM atest3; -- fail because NOINHERIT +ROLLBACK TO s1; +RESET SESSION AUTHORIZATION; +GRANT regress_priv_group2 TO regress_priv_user1 WITH INHERIT TRUE; +SET SESSION AUTHORIZATION regress_priv_user1; +DELETE FROM atest3; -- ok because grant option overrides noinherit +ROLLBACK; + +BEGIN; +RESET SESSION AUTHORIZATION; +REVOKE INHERIT OPTION FOR regress_priv_group2 FROM regress_priv_user1; +SET SESSION AUTHORIZATION regress_priv_user1; +DELETE FROM atest3; -- fail because grant option overrides inherit +ROLLBACK; + +BEGIN; +RESET SESSION AUTHORIZATION; +GRANT regress_priv_group2 TO regress_priv_user1 WITH INHERIT FALSE; +SET SESSION AUTHORIZATION regress_priv_user1; +DELETE FROM atest3; -- as above, but change grant option using other syntax ROLLBACK; -- views -- 2.24.3 (Apple Git-128)