diff --git a/doc/src/sgml/ref/alter_group.sgml b/doc/src/sgml/ref/alter_group.sgml
index fa4a8df912..b9e641818c 100644
--- a/doc/src/sgml/ref/alter_group.sgml
+++ b/doc/src/sgml/ref/alter_group.sgml
@@ -52,7 +52,11 @@ ALTER GROUP group_name RENAME TO group; so the preferred way to do this is to use
GRANT or
- REVOKE.
+ REVOKE. Note that
+ GRANT and REVOKE have additional
+ options which are not available with this command, such as the ability
+ to grant and revoke ADMIN OPTION, and the ability to
+ specify the grantor.
diff --git a/doc/src/sgml/ref/grant.sgml b/doc/src/sgml/ref/grant.sgml
index f744b05b55..2fd0f34d55 100644
--- a/doc/src/sgml/ref/grant.sgml
+++ b/doc/src/sgml/ref/grant.sgml
@@ -267,8 +267,14 @@ GRANT role_name [, ...] TO
If GRANTED BY is specified, the grant is recorded as
- having been done by the specified role. Only database superusers may
- use this option, except when it names the same role executing the command.
+ having been done by the specified role. A user can only attribute a grant
+ to another role if they possess the privileges of that role. The role
+ recorded as the grantor must have ADMIN OPTION on the
+ target role, unless it is the bootstrap superuser. When a grant is recorded
+ as having a grantor other than the bootstrap superuser, it depends on the
+ grantor continuing to posess ADMIN OPTION on the role;
+ so, if ADMIN OPTION is revoked, dependent grants must
+ be revoked as well.
@@ -333,7 +339,7 @@ GRANT role_name [, ...] TO
diff --git a/doc/src/sgml/ref/revoke.sgml b/doc/src/sgml/ref/revoke.sgml
index 62f1971036..16e840458c 100644
--- a/doc/src/sgml/ref/revoke.sgml
+++ b/doc/src/sgml/ref/revoke.sgml
@@ -198,9 +198,10 @@ REVOKE [ ADMIN OPTION FOR ]
When revoking membership in a role, GRANT OPTION is instead
called ADMIN OPTION, but the behavior is similar.
- 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).
+ Note that, in releases prior to PostgreSQL 16,
+ dependent privileges were not tracked for grants of role membership,
+ and thus CASCADE had no effect for role membership.
+ This is no longer the case.
Note also that this form of the command does not
allow the noise word GROUP
in role_specification.
@@ -239,7 +240,10 @@ REVOKE [ ADMIN OPTION FOR ]
If a superuser chooses to issue a GRANT or REVOKE
command, the command is performed as though it were issued by the
- owner of the affected object. Since all privileges ultimately come
+ owner of the affected object. (Since roles do not have owners, in the
+ case of a GRANT of role membership, the command is
+ performed as though it were issued by the bootstrap superuser.)
+ Since all privileges ultimately come
from the object owner (possibly indirectly via chains of grant options),
it is possible for a superuser to revoke all privileges, but this might
require use of CASCADE as stated above.
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index fc42b1cfd7..511ca8d8fd 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -35,10 +35,32 @@
#include "storage/lmgr.h"
#include "utils/acl.h"
#include "utils/builtins.h"
+#include "utils/catcache.h"
#include "utils/fmgroids.h"
#include "utils/syscache.h"
#include "utils/timestamp.h"
+/*
+ * Removing a role grant - or the admin option on it - might recurse to
+ * dependent grants. We use these values to reason about what would need to
+ * be done in such cases.
+ *
+ * RRG_NOOP indicates a grant that would not need to be altered by the
+ * operation.
+ *
+ * RRG_REMOVE_ADMIN_OPTION indicates a grant that would need to have
+ * admin_option set to false by the operation.
+ *
+ * RRG_DELETE_GRANT indicates a grant that would need to be removed entirely
+ * by the operation.
+ */
+typedef enum
+{
+ RRG_NOOP,
+ RRG_REMOVE_ADMIN_OPTION,
+ RRG_DELETE_GRANT
+} RevokeRoleGrantAction;
+
/* Potentially set by pg_upgrade_support functions */
Oid binary_upgrade_next_pg_authid_oid = InvalidOid;
@@ -54,7 +76,22 @@ static void AddRoleMems(const char *rolename, Oid roleid,
Oid grantorId, bool admin_opt);
static void DelRoleMems(const char *rolename, Oid roleid,
List *memberSpecs, List *memberIds,
- bool admin_opt);
+ Oid grantorId, bool admin_opt, DropBehavior behavior);
+static Oid check_role_grantor(Oid currentUserId, Oid roleid, Oid grantorId,
+ bool is_grant);
+static RevokeRoleGrantAction *initialize_revoke_actions(CatCList *memlist);
+static bool plan_single_revoke(CatCList *memlist,
+ RevokeRoleGrantAction *actions,
+ Oid member, Oid grantor,
+ bool revoke_admin_option_only,
+ DropBehavior behavior);
+static void plan_member_revoke(CatCList *memlist,
+ RevokeRoleGrantAction *actions, Oid member);
+static void plan_recursive_revoke(CatCList *memlist,
+ RevokeRoleGrantAction *actions,
+ int index,
+ bool revoke_admin_option_only,
+ DropBehavior behavior);
/* Check if current user has createrole privileges */
@@ -449,7 +486,7 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
AddRoleMems(oldrolename, oldroleid,
thisrole_list,
thisrole_oidlist,
- GetUserId(), false);
+ InvalidOid, false);
ReleaseSysCache(oldroletup);
}
@@ -461,10 +498,10 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
*/
AddRoleMems(stmt->role, roleid,
adminmembers, roleSpecsToIds(adminmembers),
- GetUserId(), true);
+ InvalidOid, true);
AddRoleMems(stmt->role, roleid,
rolemembers, roleSpecsToIds(rolemembers),
- GetUserId(), false);
+ InvalidOid, false);
/* Post creation hook for new role */
InvokeObjectPostCreateHook(AuthIdRelationId, roleid, 0);
@@ -624,7 +661,8 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
* To mess with a superuser or replication role in any way you gotta be
* superuser. We also insist on superuser to change the BYPASSRLS
* property. Otherwise, if you don't have createrole, you're only allowed
- * to change your own password.
+ * to (1) change your own password or (2) add members to a role for which
+ * you have ADMIN OPTION.
*/
if (authform->rolsuper || dissuper)
{
@@ -649,12 +687,25 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
}
else if (!have_createrole_privilege())
{
- /* check the rest */
+ /* things you certainly can't do without CREATEROLE */
if (dinherit || dcreaterole || dcreatedb || dcanlogin || dconnlimit ||
- drolemembers || dvalidUntil || !dpassword || roleid != GetUserId())
+ dvalidUntil)
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied")));
+
+ /* without CREATEROLE, can only change your own password */
+ if (dpassword && roleid != GetUserId())
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("must have CREATEROLE privilege to change another user's password")));
+
+ /* without CREATEROLE, can only add members to roles you admin */
+ if (drolemembers && !is_admin_of_role(GetUserId(), roleid))
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("must have admin option on role \"%s\" to add members",
+ rolename)));
}
/* Convert validuntil to internal form */
@@ -805,11 +856,11 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
if (stmt->action == +1) /* add members to role */
AddRoleMems(rolename, roleid,
rolemembers, roleSpecsToIds(rolemembers),
- GetUserId(), false);
+ InvalidOid, false);
else if (stmt->action == -1) /* drop members from role */
DelRoleMems(rolename, roleid,
rolemembers, roleSpecsToIds(rolemembers),
- false);
+ InvalidOid, false, DROP_RESTRICT);
}
/*
@@ -1296,7 +1347,7 @@ GrantRole(GrantRoleStmt *stmt)
if (stmt->grantor)
grantor = get_rolespec_oid(stmt->grantor, false);
else
- grantor = GetUserId();
+ grantor = InvalidOid;
grantee_ids = roleSpecsToIds(stmt->grantee_roles);
@@ -1330,7 +1381,7 @@ GrantRole(GrantRoleStmt *stmt)
else
DelRoleMems(rolename, roleid,
stmt->grantee_roles, grantee_ids,
- stmt->admin_opt);
+ grantor, stmt->admin_opt, stmt->behavior);
}
/*
@@ -1431,7 +1482,7 @@ roleSpecsToIds(List *memberNames)
* roleid: OID of role to add to
* memberSpecs: list of RoleSpec of roles to add (used only for error messages)
* memberIds: OIDs of roles to add
- * grantorId: who is granting the membership
+ * grantorId: who is granting the membership (InvalidOid if not set explicitly)
* admin_opt: granting admin option?
*/
static void
@@ -1443,6 +1494,7 @@ AddRoleMems(const char *rolename, Oid roleid,
TupleDesc pg_authmem_dsc;
ListCell *specitem;
ListCell *iditem;
+ Oid currentUserId = GetUserId();
Assert(list_length(memberSpecs) == list_length(memberIds));
@@ -1464,7 +1516,7 @@ AddRoleMems(const char *rolename, Oid roleid,
else
{
if (!have_createrole_privilege() &&
- !is_admin_of_role(grantorId, roleid))
+ !is_admin_of_role(currentUserId, roleid))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("must have admin option on role \"%s\"",
@@ -1483,29 +1535,25 @@ AddRoleMems(const char *rolename, Oid roleid,
ereport(ERROR,
errmsg("role \"%s\" cannot have explicit members", rolename));
- /*
- * The role membership grantor of record has little significance at
- * present. Nonetheless, inasmuch as users might look to it for a crude
- * audit trail, let only superusers impute the grant to a third party.
- */
- if (grantorId != GetUserId() && !superuser())
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- errmsg("must be superuser to set grantor")));
+ /* Validate grantor (and resolve implicit grantor if not specified). */
+ grantorId = check_role_grantor(currentUserId, roleid, grantorId, true);
pg_authmem_rel = table_open(AuthMemRelationId, RowExclusiveLock);
pg_authmem_dsc = RelationGetDescr(pg_authmem_rel);
+ /*
+ * Only allow changes to this role by one backend at a time, so that we
+ * can check integrity constraints like the lack of circular ADMIN OPTION
+ * grants without fear of race conditions.
+ */
+ LockSharedObject(AuthIdRelationId, roleid, 0,
+ ShareUpdateExclusiveLock);
+
+ /* Preliminary sanity checks. */
forboth(specitem, memberSpecs, iditem, memberIds)
{
RoleSpec *memberRole = lfirst_node(RoleSpec, specitem);
Oid memberid = lfirst_oid(iditem);
- HeapTuple authmem_tuple;
- HeapTuple tuple;
- Datum new_record[Natts_pg_auth_members] = {0};
- bool new_record_nulls[Natts_pg_auth_members] = {0};
- bool new_record_repl[Natts_pg_auth_members] = {0};
- Form_pg_auth_members authmem_form;
/*
* pg_database_owner is never a role member. Lifting this restriction
@@ -1543,14 +1591,94 @@ AddRoleMems(const char *rolename, Oid roleid,
(errcode(ERRCODE_INVALID_GRANT_OPERATION),
errmsg("role \"%s\" is a member of role \"%s\"",
rolename, get_rolespec_name(memberRole))));
+ }
+
+ /*
+ * Disallow attempts to grant ADMIN OPTION back to a user who granted it
+ * to you, similar to what check_circularity does for ACLs. We want the
+ * chains of grants to remain acyclic, so that it's always possible to use
+ * REVOKE .. CASCADE to clean up all grants that depend on the one being
+ * revoked.
+ *
+ * NB: This check might look redundant with the check for membership loops
+ * above, but it isn't. That's checking for role-member loop (e.g. A is a
+ * member of B and B is a member of A) while this is checking for a
+ * member-grantor loop (e.g. A gave ADMIN OPTION on X to B and now B, who
+ * has no other source of ADMIN OPTION on X, tries to give ADMIN OPTION on
+ * X back to A).
+ */
+ if (admin_opt && grantorId != BOOTSTRAP_SUPERUSERID)
+ {
+ CatCList *memlist;
+ RevokeRoleGrantAction *actions;
+ int i;
+
+ /* Get the list of members for this role. */
+ memlist = SearchSysCacheList1(AUTHMEMROLEMEM,
+ ObjectIdGetDatum(roleid));
+
+ /*
+ * Figure out what would happen if we removed all existing grants to
+ * every role to which we've been asked to make a new grant.
+ */
+ actions = initialize_revoke_actions(memlist);
+ foreach(iditem, memberIds)
+ {
+ Oid memberid = lfirst_oid(iditem);
+
+ if (memberid == BOOTSTRAP_SUPERUSERID)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_GRANT_OPERATION),
+ errmsg("admin option cannot be granted back to your own grantor")));
+ plan_member_revoke(memlist, actions, memberid);
+ }
+
+ /*
+ * If the result would be that the grantor role would no longer have
+ * the ability to perform the grant, then the proposed grant would
+ * create a circularity.
+ */
+ for (i = 0; i < memlist->n_members; ++i)
+ {
+ HeapTuple authmem_tuple;
+ Form_pg_auth_members authmem_form;
+
+ authmem_tuple = &memlist->members[i]->tuple;
+ authmem_form = (Form_pg_auth_members) GETSTRUCT(authmem_tuple);
+
+ if (actions[i] == RRG_NOOP &&
+ authmem_form->member == grantorId &&
+ authmem_form->admin_option)
+ break;
+ }
+ if (i >= memlist->n_members)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_GRANT_OPERATION),
+ errmsg("admin option cannot be granted back to your own grantor")));
+
+ ReleaseSysCacheList(memlist);
+ }
+
+ /* Now perform the catalog updates. */
+ forboth(specitem, memberSpecs, iditem, memberIds)
+ {
+ RoleSpec *memberRole = lfirst_node(RoleSpec, specitem);
+ Oid memberid = lfirst_oid(iditem);
+ HeapTuple authmem_tuple;
+ HeapTuple tuple;
+ Datum new_record[Natts_pg_auth_members] = {0};
+ bool new_record_nulls[Natts_pg_auth_members] = {0};
+ bool new_record_repl[Natts_pg_auth_members] = {0};
+ Form_pg_auth_members authmem_form;
/*
* Check if entry for this role/member already exists; if so, give
* warning unless we are adding admin option.
*/
- authmem_tuple = SearchSysCache2(AUTHMEMROLEMEM,
+ authmem_tuple = SearchSysCache3(AUTHMEMROLEMEM,
ObjectIdGetDatum(roleid),
- ObjectIdGetDatum(memberid));
+ ObjectIdGetDatum(memberid),
+ ObjectIdGetDatum(grantorId));
if (!HeapTupleIsValid(authmem_tuple))
{
authmem_form = NULL;
@@ -1562,8 +1690,9 @@ AddRoleMems(const char *rolename, Oid roleid,
if (!admin_opt || authmem_form->admin_option)
{
ereport(NOTICE,
- (errmsg("role \"%s\" is already a member of role \"%s\"",
- get_rolespec_name(memberRole), rolename)));
+ (errmsg("role \"%s\" has already been granted membership in role \"%s\" by role \"%s\"",
+ get_rolespec_name(memberRole), rolename,
+ GetUserNameFromId(grantorId, false))));
ReleaseSysCache(authmem_tuple);
continue;
}
@@ -1577,28 +1706,12 @@ AddRoleMems(const char *rolename, Oid roleid,
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;
tuple = heap_modify_tuple(authmem_tuple, pg_authmem_dsc,
new_record,
new_record_nulls, new_record_repl);
CatalogTupleUpdate(pg_authmem_rel, &tuple->t_self, tuple);
- if (authmem_form->grantor != grantorId)
- {
- Oid *oldmembers = palloc(sizeof(Oid));
- Oid *newmembers = palloc(sizeof(Oid));
-
- /* updateAclDependencies wants to pfree array inputs */
- oldmembers[0] = authmem_form->grantor;
- newmembers[0] = grantorId;
-
- updateAclDependencies(AuthMemRelationId, authmem_form->oid,
- 0, InvalidOid,
- 1, oldmembers,
- 1, newmembers);
- }
-
ReleaseSysCache(authmem_tuple);
}
else
@@ -1637,17 +1750,23 @@ AddRoleMems(const char *rolename, Oid roleid,
* roleid: OID of role to del from
* memberSpecs: list of RoleSpec of roles to del (used only for error messages)
* memberIds: OIDs of roles to del
+ * grantorId: who is revoking the membership
* admin_opt: remove admin option only?
+ * behavior: RESTRICT or CASCADE behavior for recursive removal
*/
static void
DelRoleMems(const char *rolename, Oid roleid,
List *memberSpecs, List *memberIds,
- bool admin_opt)
+ Oid grantorId, bool admin_opt, DropBehavior behavior)
{
Relation pg_authmem_rel;
TupleDesc pg_authmem_dsc;
ListCell *specitem;
ListCell *iditem;
+ Oid currentUserId = GetUserId();
+ CatCList *memlist;
+ RevokeRoleGrantAction *actions;
+ int i;
Assert(list_length(memberSpecs) == list_length(memberIds));
@@ -1669,40 +1788,69 @@ DelRoleMems(const char *rolename, Oid roleid,
else
{
if (!have_createrole_privilege() &&
- !is_admin_of_role(GetUserId(), roleid))
+ !is_admin_of_role(currentUserId, roleid))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("must have admin option on role \"%s\"",
rolename)));
}
+ /* Validate grantor (and resolve implicit grantor if not specified). */
+ grantorId = check_role_grantor(currentUserId, roleid, grantorId, false);
+
pg_authmem_rel = table_open(AuthMemRelationId, RowExclusiveLock);
pg_authmem_dsc = RelationGetDescr(pg_authmem_rel);
+ /*
+ * Only allow changes to this role by one backend at a time, so that we
+ * can check for things like dependent privileges without fear of race
+ * conditions.
+ */
+ LockSharedObject(AuthIdRelationId, roleid, 0,
+ ShareUpdateExclusiveLock);
+
+ memlist = SearchSysCacheList1(AUTHMEMROLEMEM, ObjectIdGetDatum(roleid));
+ actions = initialize_revoke_actions(memlist);
+
+ /*
+ * We may need to recurse to dependent privileges if DROP_CASCADE was
+ * specified, or refuse to perform the operation if dependent privileges
+ * exist and DROP_RESTRICT was specified. plan_single_revoke() will figure
+ * out what to do with each catalog tuple.
+ */
forboth(specitem, memberSpecs, iditem, memberIds)
{
RoleSpec *memberRole = lfirst(specitem);
Oid memberid = lfirst_oid(iditem);
+
+ if (!plan_single_revoke(memlist, actions, memberid, grantorId,
+ admin_opt, behavior))
+ {
+ ereport(WARNING,
+ (errmsg("role \"%s\" has not been granted membership in role \"%s\" by role \"%s\"",
+ get_rolespec_name(memberRole), rolename,
+ GetUserNameFromId(grantorId, false))));
+ continue;
+ }
+ }
+
+ /*
+ * We now know what to do with each catalog tuple: it should either be
+ * left alone, deleted, or just have the admin_option flag cleared.
+ * Perform the appropriate action in each case.
+ */
+ for (i = 0; i < memlist->n_members; ++i)
+ {
HeapTuple authmem_tuple;
Form_pg_auth_members authmem_form;
- /*
- * Find entry for this role/member
- */
- authmem_tuple = SearchSysCache2(AUTHMEMROLEMEM,
- ObjectIdGetDatum(roleid),
- ObjectIdGetDatum(memberid));
- if (!HeapTupleIsValid(authmem_tuple))
- {
- ereport(WARNING,
- (errmsg("role \"%s\" is not a member of role \"%s\"",
- get_rolespec_name(memberRole), rolename)));
+ if (actions[i] == RRG_NOOP)
continue;
- }
+ authmem_tuple = &memlist->members[i]->tuple;
authmem_form = (Form_pg_auth_members) GETSTRUCT(authmem_tuple);
- if (!admin_opt)
+ if (actions[i] == RRG_DELETE_GRANT)
{
/*
* Remove the entry altogether, after first removing its
@@ -1729,15 +1877,298 @@ DelRoleMems(const char *rolename, Oid roleid,
new_record_nulls, new_record_repl);
CatalogTupleUpdate(pg_authmem_rel, &tuple->t_self, tuple);
}
-
- ReleaseSysCache(authmem_tuple);
-
- /* CCI after each change, in case there are duplicates in list */
- CommandCounterIncrement();
}
+ ReleaseSysCacheList(memlist);
+
/*
* Close pg_authmem, but keep lock till commit.
*/
table_close(pg_authmem_rel, NoLock);
}
+
+/*
+ * Sanity-check, or infer, the grantor for a GRANT or REVOKE statement
+ * targeting a role.
+ *
+ * The grantor must always be either a role with ADMIN OPTION on the role in
+ * which membership is being granted, or the bootstrap superuser. This is
+ * similar to the restriction enforced by select_best_grantor, except that
+ * roles don't have owners, so we regard the bootstrap superuser as the
+ * implicit owner.
+ *
+ * If the grantor was not explicitly specified by the user, grantorId should
+ * be passed as InvalidOid, and this function will infer the user to be
+ * recorded as the grantor. In many cases, this will be the current user, but
+ * things get more complicated when the current user doesn't possess ADMIN
+ * OPTION on the role but rather relies on having CREATEROLE privileges, or
+ * on inheriting the privileges of a role which does have ADMIN OPTION. See
+ * below for details.
+ *
+ * If the grantor was specified by the user, then it must be a user that
+ * can legally be recorded as the grantor, as per the rule stated above.
+ * This is an integrity constraint, not a permissions check, and thus even
+ * superusers are subject to this restriction. However, there is also a
+ * permissions check: to specify a role as the grantor, the current user
+ * must possess the privileges of that role. Superusers will always pass
+ * this check, but for non-superusers it may lead to an error.
+ *
+ * The return value is the OID to be regarded as the grantor when executing
+ * the operation.
+ */
+static Oid
+check_role_grantor(Oid currentUserId, Oid roleid, Oid grantorId, bool is_grant)
+{
+ /* If the grantor ID was not specified, pick one to use. */
+ if (!OidIsValid(grantorId))
+ {
+ /*
+ * Grants where the grantor is recorded as the bootstrap superuser do
+ * not depend on any other existing grants, so always default to this
+ * interpretation when possible.
+ */
+ if (has_createrole_privilege(currentUserId))
+ return BOOTSTRAP_SUPERUSERID;
+
+ /*
+ * Otherwise, the grantor must either have ADMIN OPTION on the role or
+ * inherit the privileges of a role which does. In the former case,
+ * record the grantor as the current user; in the latter, pick one of
+ * the roles that is "most directly" inherited by the current role
+ * (i.e. fewest "hops").
+ *
+ * (We shouldn't fail to find a best grantor, because we've already
+ * established that the current user has permission to perform the
+ * operation.)
+ */
+ grantorId = select_best_admin(currentUserId, roleid);
+ if (!OidIsValid(grantorId))
+ elog(ERROR, "no possible grantors");
+ return grantorId;
+ }
+
+ /*
+ * If an explicit grantor is specified, it must be a role whose privileges
+ * the current user possesses.
+ *
+ * It should also be a role that has ADMIN OPTION on the target role, but
+ * we check this condition only in case of GRANT. For REVOKE, no matching
+ * grant should exist anyway, but if it somehow does, let the user get rid
+ * of it.
+ */
+ if (is_grant)
+ {
+ if (!has_privs_of_role(currentUserId, grantorId))
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("permission denied to grant privileges as role \"%s\"",
+ GetUserNameFromId(grantorId, false))));
+
+ if (grantorId != BOOTSTRAP_SUPERUSERID &&
+ select_best_admin(grantorId, roleid) != grantorId)
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("grantor must have ADMIN OPTION on \"%s\"",
+ GetUserNameFromId(roleid, false))));
+ }
+ else
+ {
+ if (!has_privs_of_role(currentUserId, grantorId))
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("permission denied to revoke privileges granted by role \"%s\"",
+ GetUserNameFromId(grantorId, false))));
+ }
+
+ /*
+ * If a grantor was specified explicitly, always attribute the grant to
+ * that role (unless we error out above).
+ */
+ return grantorId;
+}
+
+/*
+ * Initialize an array of RevokeRoleGrantAction objects.
+ *
+ * 'memlist' should be a list of all grants for the target role.
+ *
+ * This constructs an array indicating that no actions are to be performed;
+ * that is, every element is initially RRG_NOOP.
+ */
+static RevokeRoleGrantAction *
+initialize_revoke_actions(CatCList *memlist)
+{
+ RevokeRoleGrantAction *result;
+ int i;
+
+ if (memlist->n_members == 0)
+ return NULL;
+
+ result = palloc(sizeof(RevokeRoleGrantAction) * memlist->n_members);
+ for (i = 0; i < memlist->n_members; i++)
+ result[i] = RRG_NOOP;
+ return result;
+}
+
+/*
+ * Figure out what we would need to do in order to revoke a grant, or just the
+ * admin option on a grant, given that there might be dependent privileges.
+ *
+ * 'memlist' should be a list of all grants for the target role.
+ *
+ * Whatever actions prove to be necessary will be signalled by updating
+ * 'actions'.
+ *
+ * If behavior is DROP_RESTRICT, an error will occur if there are dependent
+ * role membership grants; if DROP_CASCADE, those grants will be scheduled
+ * for deletion.
+ *
+ * The return value is true if the matching grant was found in the list,
+ * and false if not.
+ */
+static bool
+plan_single_revoke(CatCList *memlist, RevokeRoleGrantAction *actions,
+ Oid member, Oid grantor, bool revoke_admin_option_only,
+ DropBehavior behavior)
+{
+ int i;
+
+ for (i = 0; i < memlist->n_members; ++i)
+ {
+ HeapTuple authmem_tuple;
+ Form_pg_auth_members authmem_form;
+
+ authmem_tuple = &memlist->members[i]->tuple;
+ authmem_form = (Form_pg_auth_members) GETSTRUCT(authmem_tuple);
+
+ if (authmem_form->member == member &&
+ authmem_form->grantor == grantor)
+ {
+ plan_recursive_revoke(memlist, actions, i,
+ revoke_admin_option_only, behavior);
+ return true;
+ }
+ }
+
+ return false;
+}
+
+/*
+ * Figure out what we would need to do in order to revoke all grants to
+ * a given member, given that there might be dependent privileges.
+ *
+ * 'memlist' should be a list of all grants for the target role.
+ *
+ * Whatever actions prove to be necessary will be signalled by updating
+ * 'actions'.
+ */
+static void
+plan_member_revoke(CatCList *memlist, RevokeRoleGrantAction *actions,
+ Oid member)
+{
+ int i;
+
+ for (i = 0; i < memlist->n_members; ++i)
+ {
+ HeapTuple authmem_tuple;
+ Form_pg_auth_members authmem_form;
+
+ authmem_tuple = &memlist->members[i]->tuple;
+ authmem_form = (Form_pg_auth_members) GETSTRUCT(authmem_tuple);
+
+ if (authmem_form->member == member)
+ plan_recursive_revoke(memlist, actions, i, false, DROP_CASCADE);
+ }
+}
+
+/*
+ * Workhorse for figuring out recursive revocation of role grants.
+ *
+ * This is similar to what recursive_revoke() does for ACLs.
+ */
+static void
+plan_recursive_revoke(CatCList *memlist, RevokeRoleGrantAction *actions,
+ int index,
+ bool revoke_admin_option_only, DropBehavior behavior)
+{
+ bool would_still_have_admin_option = false;
+ HeapTuple authmem_tuple;
+ Form_pg_auth_members authmem_form;
+ int i;
+
+ /* If it's already been done, we can just return. */
+ if (actions[index] == RRG_DELETE_GRANT)
+ return;
+ if (actions[index] == RRG_REMOVE_ADMIN_OPTION &&
+ revoke_admin_option_only)
+ return;
+
+ /* Locate tuple data. */
+ authmem_tuple = &memlist->members[index]->tuple;
+ authmem_form = (Form_pg_auth_members) GETSTRUCT(authmem_tuple);
+
+ /*
+ * If the existing tuple does not have admin_option set, then we do not
+ * need to recurse. If we're just supposed to clear that bit we don't need
+ * to do anything at all; if we're supposed to remove the grant, we need
+ * to do something, but only to the tuple, and not any others.
+ */
+ if (!revoke_admin_option_only)
+ {
+ actions[index] = RRG_DELETE_GRANT;
+ if (!authmem_form->admin_option)
+ return;
+ }
+ else
+ {
+ if (!authmem_form->admin_option)
+ return;
+ actions[index] = RRG_REMOVE_ADMIN_OPTION;
+ }
+
+ /* Determine whether the member would still have ADMIN OPTION. */
+ for (i = 0; i < memlist->n_members; ++i)
+ {
+ HeapTuple am_cascade_tuple;
+ Form_pg_auth_members am_cascade_form;
+
+ am_cascade_tuple = &memlist->members[i]->tuple;
+ am_cascade_form = (Form_pg_auth_members) GETSTRUCT(am_cascade_tuple);
+
+ if (am_cascade_form->member == authmem_form->member &&
+ am_cascade_form->admin_option && actions[i] == RRG_NOOP)
+ {
+ would_still_have_admin_option = true;
+ break;
+ }
+ }
+
+ /* If the member would still have ADMIN OPTION, we need not recurse. */
+ if (would_still_have_admin_option)
+ return;
+
+ /*
+ * Recurse to grants that are not yet slated for deletion which have this
+ * member as the grantor.
+ */
+ for (i = 0; i < memlist->n_members; ++i)
+ {
+ HeapTuple am_cascade_tuple;
+ Form_pg_auth_members am_cascade_form;
+
+ am_cascade_tuple = &memlist->members[i]->tuple;
+ am_cascade_form = (Form_pg_auth_members) GETSTRUCT(am_cascade_tuple);
+
+ if (am_cascade_form->grantor == authmem_form->member &&
+ actions[i] != RRG_DELETE_GRANT)
+ {
+ if (behavior == DROP_RESTRICT)
+ ereport(ERROR,
+ (errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST),
+ errmsg("dependent privileges exist"),
+ errhint("Use CASCADE to revoke them too.")));
+
+ plan_recursive_revoke(memlist, actions, i, false, behavior);
+ }
+ }
+}
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index f9037761f9..c8bd66dd54 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -7870,6 +7870,7 @@ RevokeRoleStmt:
n->admin_opt = false;
n->granted_roles = $2;
n->grantee_roles = $4;
+ n->grantor = $5;
n->behavior = $6;
$$ = (Node *) n;
}
@@ -7881,6 +7882,7 @@ RevokeRoleStmt:
n->admin_opt = true;
n->granted_roles = $5;
n->grantee_roles = $7;
+ n->grantor = $8;
n->behavior = $9;
$$ = (Node *) n;
}
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index 6fa58dd8eb..3e045da31f 100644
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -4791,9 +4791,7 @@ 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
- * *is_admin==true if and only if role "roleid" has an ADMIN OPTION membership
- * in role "admin_of".
+ * set, while ROLERECURSE_MEMBERS recurses through all roles.
*
* Since indirect membership testing is relatively expensive, we cache
* a list of memberships. Hence, the result is only guaranteed good until
@@ -4801,10 +4799,15 @@ has_rolinherit(Oid roleid)
*
* For the benefit of select_best_grantor, the result is defined to be
* in breadth-first order, ie, closer relationships earlier.
+ *
+ * If admin_of is not InvalidOid, this function sets *admin_role, either
+ * to the OID of the first role in the result list that directly possesses
+ * ADMIN OPTION on the role corresponding to admin_of, or to InvalidOid if
+ * there is no such role.
*/
static List *
roles_is_member_of(Oid roleid, enum RoleRecurseType type,
- Oid admin_of, bool *is_admin)
+ Oid admin_of, Oid *admin_role)
{
Oid dba;
List *roles_list;
@@ -4812,7 +4815,9 @@ roles_is_member_of(Oid roleid, enum RoleRecurseType type,
List *new_cached_roles;
MemoryContext oldctx;
- Assert(OidIsValid(admin_of) == PointerIsValid(is_admin));
+ Assert(OidIsValid(admin_of) == PointerIsValid(admin_role));
+ if (admin_role != NULL)
+ *admin_role = InvalidOid;
/* If cache is valid and ADMIN OPTION not sought, just return the list */
if (cached_role[type] == roleid && !OidIsValid(admin_of) &&
@@ -4873,8 +4878,8 @@ roles_is_member_of(Oid roleid, enum RoleRecurseType type,
*/
if (otherid == admin_of &&
((Form_pg_auth_members) GETSTRUCT(tup))->admin_option &&
- OidIsValid(admin_of))
- *is_admin = true;
+ OidIsValid(admin_of) && !OidIsValid(*admin_role))
+ *admin_role = memberid;
/*
* Even though there shouldn't be any loops in the membership
@@ -5014,7 +5019,7 @@ is_member_of_role_nosuper(Oid member, Oid role)
bool
is_admin_of_role(Oid member, Oid role)
{
- bool result = false;
+ Oid admin_role;
if (superuser_arg(member))
return true;
@@ -5023,8 +5028,30 @@ is_admin_of_role(Oid member, Oid role)
if (member == role)
return false;
- (void) roles_is_member_of(member, ROLERECURSE_MEMBERS, role, &result);
- return result;
+ (void) roles_is_member_of(member, ROLERECURSE_MEMBERS, role, &admin_role);
+ return OidIsValid(admin_role);
+}
+
+/*
+ * Find a role whose privileges "member" inherits which has ADMIN OPTION
+ * on "role", ignoring super-userness.
+ *
+ * There might be more than one such role; prefer one which involves fewer
+ * hops. That is, if member has ADMIN OPTION, prefer that over all other
+ * options; if not, prefer a role from which member inherits more directly
+ * over more indirect inheritance.
+ */
+Oid
+select_best_admin(Oid member, Oid role)
+{
+ Oid admin_role;
+
+ /* By policy, a role cannot have WITH ADMIN OPTION on itself. */
+ if (member == role)
+ return InvalidOid;
+
+ (void) roles_is_member_of(member, ROLERECURSE_PRIVS, role, &admin_role);
+ return admin_role;
}
diff --git a/src/backend/utils/cache/syscache.c b/src/backend/utils/cache/syscache.c
index 1912b12146..eec644ec84 100644
--- a/src/backend/utils/cache/syscache.c
+++ b/src/backend/utils/cache/syscache.c
@@ -213,22 +213,22 @@ static const struct cachedesc cacheinfo[] = {
},
{AuthMemRelationId, /* AUTHMEMMEMROLE */
AuthMemMemRoleIndexId,
- 2,
+ 3,
{
Anum_pg_auth_members_member,
Anum_pg_auth_members_roleid,
- 0,
+ Anum_pg_auth_members_grantor,
0
},
8
},
{AuthMemRelationId, /* AUTHMEMROLEMEM */
AuthMemRoleMemIndexId,
- 2,
+ 3,
{
Anum_pg_auth_members_roleid,
Anum_pg_auth_members_member,
- 0,
+ Anum_pg_auth_members_grantor,
0
},
8
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 26d3d53809..e8a2bfa6bd 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -21,6 +21,7 @@
#include "catalog/pg_authid_d.h"
#include "common/connect.h"
#include "common/file_utils.h"
+#include "common/hashfn.h"
#include "common/logging.h"
#include "common/string.h"
#include "dumputils.h"
@@ -31,6 +32,28 @@
/* version string we expect back from pg_dump */
#define PGDUMP_VERSIONSTR "pg_dump (PostgreSQL) " PG_VERSION "\n"
+static uint32 hash_string_pointer(char *s);
+
+typedef struct
+{
+ uint32 status;
+ uint32 hashval;
+ char *rolename;
+} RoleNameEntry;
+
+#define SH_PREFIX rolename
+#define SH_ELEMENT_TYPE RoleNameEntry
+#define SH_KEY_TYPE char *
+#define SH_KEY rolename
+#define SH_HASH_KEY(tb, key) hash_string_pointer(key)
+#define SH_EQUAL(tb, a, b) (strcmp(a, b) == 0)
+#define SH_STORE_HASH
+#define SH_GET_HASH(tb, a) (a)->hashval
+#define SH_SCOPE static inline
+#define SH_RAW_ALLOCATOR pg_malloc0
+#define SH_DECLARE
+#define SH_DEFINE
+#include "lib/simplehash.h"
static void help(void);
@@ -925,45 +948,150 @@ dumpRoleMembership(PGconn *conn)
{
PQExpBuffer buf = createPQExpBuffer();
PGresult *res;
- int i;
+ int start = 0,
+ end,
+ total;
+ bool dump_grantors;
- printfPQExpBuffer(buf, "SELECT ur.rolname AS roleid, "
+ /*
+ * Previous versions of PostgreSQL didn't used to track the grantor very
+ * carefully in the backend, and the grantor could be any user even if
+ * they didn't have ADMIN OPTION on the role, or a user that no longer
+ * existed. To avoid dump and restore failures, don't dump the grantor
+ * when talking to an old server version.
+ */
+ dump_grantors = (PQserverVersion(conn) >= 160000);
+
+ /* Generate and execute query. */
+ printfPQExpBuffer(buf, "SELECT ur.rolname AS role, "
"um.rolname AS member, "
- "a.admin_option, "
- "ug.rolname AS grantor "
+ "ug.oid AS grantorid, "
+ "ug.rolname AS grantor, "
+ "a.admin_option "
"FROM pg_auth_members a "
"LEFT JOIN %s ur on ur.oid = a.roleid "
"LEFT JOIN %s um on um.oid = a.member "
"LEFT JOIN %s ug on ug.oid = a.grantor "
"WHERE NOT (ur.rolname ~ '^pg_' AND um.rolname ~ '^pg_')"
- "ORDER BY 1,2,3", role_catalog, role_catalog, role_catalog);
+ "ORDER BY 1,2,4", role_catalog, role_catalog, role_catalog);
res = executeQuery(conn, buf->data);
if (PQntuples(res) > 0)
fprintf(OPF, "--\n-- Role memberships\n--\n\n");
- for (i = 0; i < PQntuples(res); i++)
+ /*
+ * We can't dump these GRANT commands in arbitary order, because a role
+ * that is named as a grantor must already have ADMIN OPTION on the
+ * role for which it is granting permissions, except for the boostrap
+ * superuser, who can always be named as the grantor.
+ *
+ * We handle this by considering these grants role by role. For each role,
+ * we initially consider the only allowable grantor to be the boostrap
+ * superuser. Every time we grant ADMIN OPTION on the role to some user,
+ * that user also becomes an allowable grantor. We make repeated passes
+ * over the grants for the role, each time dumping those whose grantors
+ * are allowable and which we haven't done yet. Eventually this should
+ * let us dump all the grants.
+ */
+ total = PQntuples(res);
+ while (start < total)
{
- char *roleid = PQgetvalue(res, i, 0);
- char *member = PQgetvalue(res, i, 1);
- char *option = PQgetvalue(res, i, 2);
+ char *role = PQgetvalue(res, start, 0);
+ int i;
+ bool *done;
+ int remaining;
+ int prev_remaining = 0;
+ rolename_hash *ht;
- fprintf(OPF, "GRANT %s", fmtId(roleid));
- fprintf(OPF, " TO %s", fmtId(member));
- if (*option == 't')
- fprintf(OPF, " WITH ADMIN OPTION");
+ /* All memberships for a single role should be adjacent. */
+ for (end = start; end < total; ++end)
+ {
+ char *otherrole;
+
+ otherrole = PQgetvalue(res, end, 0);
+ if (strcmp(role, otherrole) != 0)
+ break;
+ }
+
+ role = PQgetvalue(res, start, 0);
+ remaining = end - start;
+ done = pg_malloc0(remaining * sizeof(bool));
+ ht = rolename_create(remaining, NULL);
/*
- * We don't track the grantor very carefully in the backend, so cope
- * with the possibility that it has been dropped.
+ * Make repeated passses over the grants for this role until all have
+ * been dumped.
*/
- if (!PQgetisnull(res, i, 3))
+ while (remaining > 0)
{
- char *grantor = PQgetvalue(res, i, 3);
+ /*
+ * We should make progress on every iteration, because a notional
+ * graph whose vertices are grants and whose edges point from
+ * grantors to members should be connected and acyclic. If we fail
+ * to make progress, either we or the server have messed up.
+ */
+ if (remaining == prev_remaining)
+ {
+ pg_log_error("could not find a legal dump ordering for memberships in role \"%s\"",
+ role);
+ PQfinish(conn);
+ exit_nicely(1);
+ }
- fprintf(OPF, " GRANTED BY %s", fmtId(grantor));
+ /* Make one pass over the grants for this role. */
+ for (i = start; i < end; ++i)
+ {
+ char *member;
+ char *admin_option;
+ char *grantorid;
+ char *grantor;
+ bool found;
+
+ /* If we already did this grant, don't do it again. */
+ if (done[i - start])
+ continue;
+
+ member = PQgetvalue(res, i, 1);
+ grantorid = PQgetvalue(res, i, 2);
+ grantor = PQgetvalue(res, i, 3);
+ admin_option = PQgetvalue(res, i, 4);
+
+ /*
+ * If we're not dumping grantors or if the grantor is the
+ * bootstrap superuser, it's fine to dump this now. Otherwise,
+ * it's got to be someone who has already been granted ADMIN
+ * OPTION.
+ */
+ if (dump_grantors &&
+ atooid(grantorid) != BOOTSTRAP_SUPERUSERID &&
+ rolename_lookup(ht, grantor) == NULL)
+ continue;
+
+ /* Remember that we did this so that we don't do it again. */
+ done[i - start] = true;
+ --remaining;
+
+ /*
+ * If ADMIN OPTION is being granted, remember that grants
+ * listing this member as the grantor can now be dumped.
+ */
+ if (*admin_option == 't')
+ rolename_insert(ht, member, &found);
+
+ /* Generate the actual GRANT statement. */
+ fprintf(OPF, "GRANT %s", fmtId(role));
+ fprintf(OPF, " TO %s", fmtId(member));
+ if (*admin_option == 't')
+ fprintf(OPF, " WITH ADMIN OPTION");
+ if (dump_grantors)
+ fprintf(OPF, " GRANTED BY %s", fmtId(grantor));
+ fprintf(OPF, ";\n");
+ }
}
- fprintf(OPF, ";\n");
+
+ rolename_destroy(ht);
+ pg_free(done);
+ start = end;
}
PQclear(res);
@@ -1748,3 +1876,14 @@ dumpTimestamp(const char *msg)
if (strftime(buf, sizeof(buf), PGDUMP_STRFTIME_FMT, localtime(&now)) != 0)
fprintf(OPF, "-- %s %s\n\n", msg, buf);
}
+
+/*
+ * Helper function for rolenamehash hash table.
+ */
+static uint32
+hash_string_pointer(char *s)
+{
+ unsigned char *ss = (unsigned char *) s;
+
+ return hash_bytes(ss, strlen(s));
+}
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index 0aebda6a47..b227c7a337 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -57,6 +57,6 @@
*/
/* yyyymmddN */
-#define CATALOG_VERSION_NO 202208181
+#define CATALOG_VERSION_NO 202208221
#endif
diff --git a/src/include/catalog/pg_auth_members.h b/src/include/catalog/pg_auth_members.h
index c9d7697730..e57ec4f810 100644
--- a/src/include/catalog/pg_auth_members.h
+++ b/src/include/catalog/pg_auth_members.h
@@ -44,8 +44,8 @@ CATALOG(pg_auth_members,1261,AuthMemRelationId) BKI_SHARED_RELATION BKI_ROWTYPE_
typedef FormData_pg_auth_members *Form_pg_auth_members;
DECLARE_UNIQUE_INDEX_PKEY(pg_auth_members_oid_index, 9385, AuthMemOidIndexId, on pg_auth_members using btree(oid oid_ops));
-DECLARE_UNIQUE_INDEX(pg_auth_members_role_member_index, 2694, AuthMemRoleMemIndexId, on pg_auth_members using btree(roleid oid_ops, member oid_ops));
-DECLARE_UNIQUE_INDEX(pg_auth_members_member_role_index, 2695, AuthMemMemRoleIndexId, on pg_auth_members using btree(member oid_ops, roleid oid_ops));
+DECLARE_UNIQUE_INDEX(pg_auth_members_role_member_index, 2694, AuthMemRoleMemIndexId, on pg_auth_members using btree(roleid oid_ops, member oid_ops, grantor oid_ops));
+DECLARE_UNIQUE_INDEX(pg_auth_members_member_role_index, 2695, AuthMemMemRoleIndexId, on pg_auth_members using btree(member oid_ops, roleid oid_ops, grantor oid_ops));
DECLARE_INDEX(pg_auth_members_grantor_index, 9384, AuthMemGrantorIndexId, on pg_auth_members using btree(grantor oid_ops));
#endif /* PG_AUTH_MEMBERS_H */
diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h
index 48f7d72add..3d6411197c 100644
--- a/src/include/utils/acl.h
+++ b/src/include/utils/acl.h
@@ -212,6 +212,7 @@ extern bool has_privs_of_role(Oid member, Oid role);
extern bool is_member_of_role(Oid member, Oid role);
extern bool is_member_of_role_nosuper(Oid member, Oid role);
extern bool is_admin_of_role(Oid member, Oid role);
+extern Oid select_best_admin(Oid member, Oid role);
extern void check_is_member_of_role(Oid member, Oid role);
extern Oid get_role_oid(const char *rolename, bool missing_ok);
extern Oid get_role_oid_or_public(const char *rolename);
diff --git a/src/test/regress/expected/create_role.out b/src/test/regress/expected/create_role.out
index c2465d0f49..4e67d72760 100644
--- a/src/test/regress/expected/create_role.out
+++ b/src/test/regress/expected/create_role.out
@@ -103,21 +103,9 @@ ERROR: role "regress_nosuch_recursive" does not exist
DROP ROLE regress_nosuch_admin_recursive;
ERROR: role "regress_nosuch_admin_recursive" does not exist
DROP ROLE regress_plainrole;
--- fail, can't drop regress_createrole yet, due to outstanding grants
-DROP ROLE regress_createrole;
-ERROR: role "regress_createrole" cannot be dropped because some objects depend on it
-DETAIL: privileges for membership of role regress_read_all_data in role pg_read_all_data
-privileges for membership of role regress_write_all_data in role pg_write_all_data
-privileges for membership of role regress_monitor in role pg_monitor
-privileges for membership of role regress_read_all_settings in role pg_read_all_settings
-privileges for membership of role regress_read_all_stats in role pg_read_all_stats
-privileges for membership of role regress_stat_scan_tables in role pg_stat_scan_tables
-privileges for membership of role regress_read_server_files in role pg_read_server_files
-privileges for membership of role regress_write_server_files in role pg_write_server_files
-privileges for membership of role regress_execute_server_program in role pg_execute_server_program
-privileges for membership of role regress_signal_backend in role pg_signal_backend
-- ok, should be able to drop non-superuser roles we created
DROP ROLE regress_createdb;
+DROP ROLE regress_createrole;
DROP ROLE regress_login;
DROP ROLE regress_inherit;
DROP ROLE regress_connection_limit;
@@ -137,8 +125,6 @@ DROP ROLE regress_read_server_files;
DROP ROLE regress_write_server_files;
DROP ROLE regress_execute_server_program;
DROP ROLE regress_signal_backend;
--- ok, dropped the other roles first so this is ok now
-DROP ROLE regress_createrole;
-- fail, role still owns database objects
DROP ROLE regress_tenant;
ERROR: role "regress_tenant" cannot be dropped because some objects depend on it
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index 65b4a22ebc..0154a09262 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -33,6 +33,54 @@ CREATE USER regress_priv_user8;
CREATE USER regress_priv_user9;
CREATE USER regress_priv_user10;
CREATE ROLE regress_priv_role;
+-- circular ADMIN OPTION grants should be disallowed
+GRANT regress_priv_user1 TO regress_priv_user2 WITH ADMIN OPTION;
+GRANT regress_priv_user1 TO regress_priv_user3 WITH ADMIN OPTION GRANTED BY regress_priv_user2;
+GRANT regress_priv_user1 TO regress_priv_user2 WITH ADMIN OPTION GRANTED BY regress_priv_user3;
+ERROR: admin option cannot be granted back to your own grantor
+-- need CASCADE to revoke grant or admin option if dependent grants exist
+REVOKE ADMIN OPTION FOR regress_priv_user1 FROM regress_priv_user2; -- fail
+ERROR: dependent privileges exist
+HINT: Use CASCADE to revoke them too.
+REVOKE regress_priv_user1 FROM regress_priv_user2; -- fail
+ERROR: dependent privileges exist
+HINT: Use CASCADE to revoke them too.
+SELECT member::regrole, admin_option FROM pg_auth_members WHERE roleid = 'regress_priv_user1'::regrole;
+ member | admin_option
+--------------------+--------------
+ regress_priv_user2 | t
+ regress_priv_user3 | t
+(2 rows)
+
+BEGIN;
+REVOKE ADMIN OPTION FOR regress_priv_user1 FROM regress_priv_user2 CASCADE;
+SELECT member::regrole, admin_option FROM pg_auth_members WHERE roleid = 'regress_priv_user1'::regrole;
+ member | admin_option
+--------------------+--------------
+ regress_priv_user2 | f
+(1 row)
+
+ROLLBACK;
+REVOKE regress_priv_user1 FROM regress_priv_user2 CASCADE;
+SELECT member::regrole, admin_option FROM pg_auth_members WHERE roleid = 'regress_priv_user1'::regrole;
+ member | admin_option
+--------+--------------
+(0 rows)
+
+-- inferred grantor must be a role with ADMIN OPTION
+GRANT regress_priv_user1 TO regress_priv_user2 WITH ADMIN OPTION;
+GRANT regress_priv_user2 TO regress_priv_user3;
+SET ROLE regress_priv_user3;
+GRANT regress_priv_user1 TO regress_priv_user4;
+SELECT grantor::regrole FROM pg_auth_members WHERE roleid = 'regress_priv_user1'::regrole and member = 'regress_priv_user4'::regrole;
+ grantor
+--------------------
+ regress_priv_user2
+(1 row)
+
+RESET ROLE;
+REVOKE regress_priv_user2 FROM regress_priv_user3;
+REVOKE regress_priv_user1 FROM regress_priv_user2 CASCADE;
-- test GRANTED BY with DROP OWNED and REASSIGN OWNED
GRANT regress_priv_user1 TO regress_priv_user2 WITH ADMIN OPTION;
GRANT regress_priv_user1 TO regress_priv_user3 GRANTED BY regress_priv_user2;
@@ -68,15 +116,17 @@ CREATE USER regress_priv_user5;
GRANT pg_read_all_data TO regress_priv_user6;
GRANT pg_write_all_data TO regress_priv_user7;
GRANT pg_read_all_settings TO regress_priv_user8 WITH ADMIN OPTION;
+GRANT regress_priv_user9 TO regress_priv_user8;
SET SESSION AUTHORIZATION regress_priv_user8;
GRANT pg_read_all_settings TO regress_priv_user9 WITH ADMIN OPTION;
SET SESSION AUTHORIZATION regress_priv_user9;
GRANT pg_read_all_settings TO regress_priv_user10;
SET SESSION AUTHORIZATION regress_priv_user8;
-REVOKE pg_read_all_settings FROM regress_priv_user10;
+REVOKE pg_read_all_settings FROM regress_priv_user10 GRANTED BY regress_priv_user9;
REVOKE ADMIN OPTION FOR pg_read_all_settings FROM regress_priv_user9;
REVOKE pg_read_all_settings FROM regress_priv_user9;
RESET SESSION AUTHORIZATION;
+REVOKE regress_priv_user9 FROM regress_priv_user8;
REVOKE ADMIN OPTION FOR pg_read_all_settings FROM regress_priv_user8;
SET SESSION AUTHORIZATION regress_priv_user8;
SET ROLE pg_read_all_settings;
@@ -87,11 +137,20 @@ DROP USER regress_priv_user10;
DROP USER regress_priv_user9;
DROP USER regress_priv_user8;
CREATE GROUP regress_priv_group1;
-CREATE GROUP regress_priv_group2 WITH USER regress_priv_user1, regress_priv_user2;
+CREATE GROUP regress_priv_group2 WITH ADMIN regress_priv_user1 USER regress_priv_user2;
ALTER GROUP regress_priv_group1 ADD USER regress_priv_user4;
+GRANT regress_priv_group2 TO regress_priv_user2 GRANTED BY regress_priv_user1;
+SET SESSION AUTHORIZATION regress_priv_user1;
+ALTER GROUP regress_priv_group2 ADD USER regress_priv_user2;
+NOTICE: role "regress_priv_user2" has already been granted membership in role "regress_priv_group2" by role "regress_priv_user1"
ALTER GROUP regress_priv_group2 ADD USER regress_priv_user2; -- duplicate
-NOTICE: role "regress_priv_user2" is already a member of role "regress_priv_group2"
+NOTICE: role "regress_priv_user2" has already been granted membership in role "regress_priv_group2" by role "regress_priv_user1"
ALTER GROUP regress_priv_group2 DROP USER regress_priv_user2;
+ALTER USER regress_priv_user2 PASSWORD 'verysecret'; -- not permitted
+ERROR: must have CREATEROLE privilege to change another user's password
+RESET SESSION AUTHORIZATION;
+ALTER GROUP regress_priv_group2 DROP USER regress_priv_user2;
+REVOKE ADMIN OPTION FOR regress_priv_group2 FROM regress_priv_user1;
GRANT regress_priv_group2 TO regress_priv_user4 WITH ADMIN OPTION;
-- prepare non-leakproof function for later
CREATE FUNCTION leak(integer,integer) RETURNS boolean
@@ -99,9 +158,13 @@ CREATE FUNCTION leak(integer,integer) RETURNS boolean
LANGUAGE internal IMMUTABLE STRICT; -- but deliberately not LEAKPROOF
ALTER FUNCTION leak(integer,integer) OWNER TO regress_priv_user1;
-- test owner privileges
+GRANT regress_priv_role TO regress_priv_user1 WITH ADMIN OPTION GRANTED BY regress_priv_role; -- error, doesn't have ADMIN OPTION
+ERROR: grantor must have ADMIN OPTION on "regress_priv_role"
GRANT regress_priv_role TO regress_priv_user1 WITH ADMIN OPTION GRANTED BY CURRENT_ROLE;
REVOKE ADMIN OPTION FOR regress_priv_role FROM regress_priv_user1 GRANTED BY foo; -- error
-REVOKE ADMIN OPTION FOR regress_priv_role FROM regress_priv_user1 GRANTED BY regress_priv_user2; -- error
+ERROR: role "foo" does not exist
+REVOKE ADMIN OPTION FOR regress_priv_role FROM regress_priv_user1 GRANTED BY regress_priv_user2; -- warning, noop
+WARNING: role "regress_priv_user1" has not been granted membership in role "regress_priv_role" by role "regress_priv_user2"
REVOKE ADMIN OPTION FOR regress_priv_role FROM regress_priv_user1 GRANTED BY CURRENT_USER;
REVOKE regress_priv_role FROM regress_priv_user1 GRANTED BY CURRENT_ROLE;
DROP ROLE regress_priv_role;
@@ -1746,7 +1809,7 @@ SET SESSION AUTHORIZATION regress_priv_user1;
GRANT regress_priv_group2 TO regress_priv_user5; -- fails: no ADMIN OPTION
ERROR: must have admin option on role "regress_priv_group2"
SELECT dogrant_ok(); -- ok: SECURITY DEFINER conveys ADMIN
-NOTICE: role "regress_priv_user5" is already a member of role "regress_priv_group2"
+NOTICE: role "regress_priv_user5" has already been granted membership in role "regress_priv_group2" by role "regress_priv_user4"
dogrant_ok
------------
diff --git a/src/test/regress/sql/create_role.sql b/src/test/regress/sql/create_role.sql
index b696628238..292dc08797 100644
--- a/src/test/regress/sql/create_role.sql
+++ b/src/test/regress/sql/create_role.sql
@@ -98,11 +98,9 @@ DROP ROLE regress_nosuch_recursive;
DROP ROLE regress_nosuch_admin_recursive;
DROP ROLE regress_plainrole;
--- fail, can't drop regress_createrole yet, due to outstanding grants
-DROP ROLE regress_createrole;
-
-- ok, should be able to drop non-superuser roles we created
DROP ROLE regress_createdb;
+DROP ROLE regress_createrole;
DROP ROLE regress_login;
DROP ROLE regress_inherit;
DROP ROLE regress_connection_limit;
@@ -123,9 +121,6 @@ DROP ROLE regress_write_server_files;
DROP ROLE regress_execute_server_program;
DROP ROLE regress_signal_backend;
--- ok, dropped the other roles first so this is ok now
-DROP ROLE regress_createrole;
-
-- fail, role still owns database objects
DROP ROLE regress_tenant;
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index 66834e32a7..b4ef20f738 100644
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -37,6 +37,32 @@ CREATE USER regress_priv_user9;
CREATE USER regress_priv_user10;
CREATE ROLE regress_priv_role;
+-- circular ADMIN OPTION grants should be disallowed
+GRANT regress_priv_user1 TO regress_priv_user2 WITH ADMIN OPTION;
+GRANT regress_priv_user1 TO regress_priv_user3 WITH ADMIN OPTION GRANTED BY regress_priv_user2;
+GRANT regress_priv_user1 TO regress_priv_user2 WITH ADMIN OPTION GRANTED BY regress_priv_user3;
+
+-- need CASCADE to revoke grant or admin option if dependent grants exist
+REVOKE ADMIN OPTION FOR regress_priv_user1 FROM regress_priv_user2; -- fail
+REVOKE regress_priv_user1 FROM regress_priv_user2; -- fail
+SELECT member::regrole, admin_option FROM pg_auth_members WHERE roleid = 'regress_priv_user1'::regrole;
+BEGIN;
+REVOKE ADMIN OPTION FOR regress_priv_user1 FROM regress_priv_user2 CASCADE;
+SELECT member::regrole, admin_option FROM pg_auth_members WHERE roleid = 'regress_priv_user1'::regrole;
+ROLLBACK;
+REVOKE regress_priv_user1 FROM regress_priv_user2 CASCADE;
+SELECT member::regrole, admin_option FROM pg_auth_members WHERE roleid = 'regress_priv_user1'::regrole;
+
+-- inferred grantor must be a role with ADMIN OPTION
+GRANT regress_priv_user1 TO regress_priv_user2 WITH ADMIN OPTION;
+GRANT regress_priv_user2 TO regress_priv_user3;
+SET ROLE regress_priv_user3;
+GRANT regress_priv_user1 TO regress_priv_user4;
+SELECT grantor::regrole FROM pg_auth_members WHERE roleid = 'regress_priv_user1'::regrole and member = 'regress_priv_user4'::regrole;
+RESET ROLE;
+REVOKE regress_priv_user2 FROM regress_priv_user3;
+REVOKE regress_priv_user1 FROM regress_priv_user2 CASCADE;
+
-- test GRANTED BY with DROP OWNED and REASSIGN OWNED
GRANT regress_priv_user1 TO regress_priv_user2 WITH ADMIN OPTION;
GRANT regress_priv_user1 TO regress_priv_user3 GRANTED BY regress_priv_user2;
@@ -67,6 +93,7 @@ CREATE USER regress_priv_user5;
GRANT pg_read_all_data TO regress_priv_user6;
GRANT pg_write_all_data TO regress_priv_user7;
GRANT pg_read_all_settings TO regress_priv_user8 WITH ADMIN OPTION;
+GRANT regress_priv_user9 TO regress_priv_user8;
SET SESSION AUTHORIZATION regress_priv_user8;
GRANT pg_read_all_settings TO regress_priv_user9 WITH ADMIN OPTION;
@@ -75,11 +102,12 @@ SET SESSION AUTHORIZATION regress_priv_user9;
GRANT pg_read_all_settings TO regress_priv_user10;
SET SESSION AUTHORIZATION regress_priv_user8;
-REVOKE pg_read_all_settings FROM regress_priv_user10;
+REVOKE pg_read_all_settings FROM regress_priv_user10 GRANTED BY regress_priv_user9;
REVOKE ADMIN OPTION FOR pg_read_all_settings FROM regress_priv_user9;
REVOKE pg_read_all_settings FROM regress_priv_user9;
RESET SESSION AUTHORIZATION;
+REVOKE regress_priv_user9 FROM regress_priv_user8;
REVOKE ADMIN OPTION FOR pg_read_all_settings FROM regress_priv_user8;
SET SESSION AUTHORIZATION regress_priv_user8;
@@ -94,12 +122,19 @@ DROP USER regress_priv_user9;
DROP USER regress_priv_user8;
CREATE GROUP regress_priv_group1;
-CREATE GROUP regress_priv_group2 WITH USER regress_priv_user1, regress_priv_user2;
+CREATE GROUP regress_priv_group2 WITH ADMIN regress_priv_user1 USER regress_priv_user2;
ALTER GROUP regress_priv_group1 ADD USER regress_priv_user4;
+GRANT regress_priv_group2 TO regress_priv_user2 GRANTED BY regress_priv_user1;
+SET SESSION AUTHORIZATION regress_priv_user1;
+ALTER GROUP regress_priv_group2 ADD USER regress_priv_user2;
ALTER GROUP regress_priv_group2 ADD USER regress_priv_user2; -- duplicate
ALTER GROUP regress_priv_group2 DROP USER regress_priv_user2;
+ALTER USER regress_priv_user2 PASSWORD 'verysecret'; -- not permitted
+RESET SESSION AUTHORIZATION;
+ALTER GROUP regress_priv_group2 DROP USER regress_priv_user2;
+REVOKE ADMIN OPTION FOR regress_priv_group2 FROM regress_priv_user1;
GRANT regress_priv_group2 TO regress_priv_user4 WITH ADMIN OPTION;
-- prepare non-leakproof function for later
@@ -110,9 +145,10 @@ ALTER FUNCTION leak(integer,integer) OWNER TO regress_priv_user1;
-- test owner privileges
+GRANT regress_priv_role TO regress_priv_user1 WITH ADMIN OPTION GRANTED BY regress_priv_role; -- error, doesn't have ADMIN OPTION
GRANT regress_priv_role TO regress_priv_user1 WITH ADMIN OPTION GRANTED BY CURRENT_ROLE;
REVOKE ADMIN OPTION FOR regress_priv_role FROM regress_priv_user1 GRANTED BY foo; -- error
-REVOKE ADMIN OPTION FOR regress_priv_role FROM regress_priv_user1 GRANTED BY regress_priv_user2; -- error
+REVOKE ADMIN OPTION FOR regress_priv_role FROM regress_priv_user1 GRANTED BY regress_priv_user2; -- warning, noop
REVOKE ADMIN OPTION FOR regress_priv_role FROM regress_priv_user1 GRANTED BY CURRENT_USER;
REVOKE regress_priv_role FROM regress_priv_user1 GRANTED BY CURRENT_ROLE;
DROP ROLE regress_priv_role;