Pass down current user ID to AddRoleMems and DelRoleMems.

This is just refactoring; there should be no functonal change. It
might have the effect of slightly reducing the number of calls to
GetUserId(), but the real point is to facilitate future work in
this area.

Patch by me, reviewed by Mark Dilger.

Discussion: http://postgr.es/m/CA+TgmobFzTLkLwOquFrAcdsWBsOWDr-_H-jw+qBvfx-wSzMwDA@mail.gmail.com
pull/111/merge
Robert Haas 3 years ago
parent 25bb03166b
commit 39cffe95f2
  1. 41
      src/backend/commands/user.c

@ -87,10 +87,10 @@ int Password_encryption = PASSWORD_TYPE_SCRAM_SHA_256;
/* Hook to check passwords in CreateRole() and AlterRole() */ /* Hook to check passwords in CreateRole() and AlterRole() */
check_password_hook_type check_password_hook = NULL; check_password_hook_type check_password_hook = NULL;
static void AddRoleMems(const char *rolename, Oid roleid, static void AddRoleMems(Oid currentUserId, const char *rolename, Oid roleid,
List *memberSpecs, List *memberIds, List *memberSpecs, List *memberIds,
Oid grantorId, GrantRoleOptions *popt); Oid grantorId, GrantRoleOptions *popt);
static void DelRoleMems(const char *rolename, Oid roleid, static void DelRoleMems(Oid currentUserId, const char *rolename, Oid roleid,
List *memberSpecs, List *memberIds, List *memberSpecs, List *memberIds,
Oid grantorId, GrantRoleOptions *popt, Oid grantorId, GrantRoleOptions *popt,
DropBehavior behavior); DropBehavior behavior);
@ -133,6 +133,7 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
HeapTuple tuple; HeapTuple tuple;
Datum new_record[Natts_pg_authid] = {0}; Datum new_record[Natts_pg_authid] = {0};
bool new_record_nulls[Natts_pg_authid] = {0}; bool new_record_nulls[Natts_pg_authid] = {0};
Oid currentUserId = GetUserId();
Oid roleid; Oid roleid;
ListCell *item; ListCell *item;
ListCell *option; ListCell *option;
@ -508,8 +509,8 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
char *oldrolename = NameStr(oldroleform->rolname); char *oldrolename = NameStr(oldroleform->rolname);
/* can only add this role to roles for which you have rights */ /* can only add this role to roles for which you have rights */
check_role_membership_authorization(GetUserId(), oldroleid, true); check_role_membership_authorization(currentUserId, oldroleid, true);
AddRoleMems(oldrolename, oldroleid, AddRoleMems(currentUserId, oldrolename, oldroleid,
thisrole_list, thisrole_list,
thisrole_oidlist, thisrole_oidlist,
InvalidOid, &popt); InvalidOid, &popt);
@ -525,12 +526,12 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
* NB: No permissions check is required here. If you have enough rights * NB: No permissions check is required here. If you have enough rights
* to create a role, you can add any members you like. * to create a role, you can add any members you like.
*/ */
AddRoleMems(stmt->role, roleid, AddRoleMems(currentUserId, stmt->role, roleid,
rolemembers, roleSpecsToIds(rolemembers), rolemembers, roleSpecsToIds(rolemembers),
InvalidOid, &popt); InvalidOid, &popt);
popt.specified |= GRANT_ROLE_SPECIFIED_ADMIN; popt.specified |= GRANT_ROLE_SPECIFIED_ADMIN;
popt.admin = true; popt.admin = true;
AddRoleMems(stmt->role, roleid, AddRoleMems(currentUserId, stmt->role, roleid,
adminmembers, roleSpecsToIds(adminmembers), adminmembers, roleSpecsToIds(adminmembers),
InvalidOid, &popt); InvalidOid, &popt);
@ -583,6 +584,7 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
DefElem *dvalidUntil = NULL; DefElem *dvalidUntil = NULL;
DefElem *dbypassRLS = NULL; DefElem *dbypassRLS = NULL;
Oid roleid; Oid roleid;
Oid currentUserId = GetUserId();
GrantRoleOptions popt; GrantRoleOptions popt;
check_rolespec_name(stmt->role, check_rolespec_name(stmt->role,
@ -727,13 +729,13 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
errmsg("permission denied"))); errmsg("permission denied")));
/* without CREATEROLE, can only change your own password */ /* without CREATEROLE, can only change your own password */
if (dpassword && roleid != GetUserId()) if (dpassword && roleid != currentUserId)
ereport(ERROR, ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("must have CREATEROLE privilege to change another user's password"))); errmsg("must have CREATEROLE privilege to change another user's password")));
/* without CREATEROLE, can only add members to roles you admin */ /* without CREATEROLE, can only add members to roles you admin */
if (drolemembers && !is_admin_of_role(GetUserId(), roleid)) if (drolemembers && !is_admin_of_role(currentUserId, roleid))
ereport(ERROR, ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("must have admin option on role \"%s\" to add members", errmsg("must have admin option on role \"%s\" to add members",
@ -888,11 +890,11 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
CommandCounterIncrement(); CommandCounterIncrement();
if (stmt->action == +1) /* add members to role */ if (stmt->action == +1) /* add members to role */
AddRoleMems(rolename, roleid, AddRoleMems(currentUserId, rolename, roleid,
rolemembers, roleSpecsToIds(rolemembers), rolemembers, roleSpecsToIds(rolemembers),
InvalidOid, &popt); InvalidOid, &popt);
else if (stmt->action == -1) /* drop members from role */ else if (stmt->action == -1) /* drop members from role */
DelRoleMems(rolename, roleid, DelRoleMems(currentUserId, rolename, roleid,
rolemembers, roleSpecsToIds(rolemembers), rolemembers, roleSpecsToIds(rolemembers),
InvalidOid, &popt, DROP_RESTRICT); InvalidOid, &popt, DROP_RESTRICT);
} }
@ -1378,6 +1380,7 @@ GrantRole(ParseState *pstate, GrantRoleStmt *stmt)
List *grantee_ids; List *grantee_ids;
ListCell *item; ListCell *item;
GrantRoleOptions popt; GrantRoleOptions popt;
Oid currentUserId = GetUserId();
/* Parse options list. */ /* Parse options list. */
InitGrantRoleOptions(&popt); InitGrantRoleOptions(&popt);
@ -1449,14 +1452,14 @@ GrantRole(ParseState *pstate, GrantRoleStmt *stmt)
errmsg("column names cannot be included in GRANT/REVOKE ROLE"))); errmsg("column names cannot be included in GRANT/REVOKE ROLE")));
roleid = get_role_oid(rolename, false); roleid = get_role_oid(rolename, false);
check_role_membership_authorization(GetUserId(), roleid, check_role_membership_authorization(currentUserId,
stmt->is_grant); roleid, stmt->is_grant);
if (stmt->is_grant) if (stmt->is_grant)
AddRoleMems(rolename, roleid, AddRoleMems(currentUserId, rolename, roleid,
stmt->grantee_roles, grantee_ids, stmt->grantee_roles, grantee_ids,
grantor, &popt); grantor, &popt);
else else
DelRoleMems(rolename, roleid, DelRoleMems(currentUserId, rolename, roleid,
stmt->grantee_roles, grantee_ids, stmt->grantee_roles, grantee_ids,
grantor, &popt, stmt->behavior); grantor, &popt, stmt->behavior);
} }
@ -1555,15 +1558,17 @@ roleSpecsToIds(List *memberNames)
/* /*
* AddRoleMems -- Add given members to the specified role * AddRoleMems -- Add given members to the specified role
* *
* currentUserId: OID of role performing the operation
* rolename: name of role to add to (used only for error messages) * rolename: name of role to add to (used only for error messages)
* roleid: OID of role to add to * roleid: OID of role to add to
* memberSpecs: list of RoleSpec of roles to add (used only for error messages) * memberSpecs: list of RoleSpec of roles to add (used only for error messages)
* memberIds: OIDs of roles to add * memberIds: OIDs of roles to add
* grantorId: who is granting the membership (InvalidOid if not set explicitly) * grantorId: OID that should be recorded as having granted the membership
* (InvalidOid if not set explicitly)
* popt: information about grant options * popt: information about grant options
*/ */
static void static void
AddRoleMems(const char *rolename, Oid roleid, AddRoleMems(Oid currentUserId, const char *rolename, Oid roleid,
List *memberSpecs, List *memberIds, List *memberSpecs, List *memberIds,
Oid grantorId, GrantRoleOptions *popt) Oid grantorId, GrantRoleOptions *popt)
{ {
@ -1571,7 +1576,6 @@ AddRoleMems(const char *rolename, Oid roleid,
TupleDesc pg_authmem_dsc; TupleDesc pg_authmem_dsc;
ListCell *specitem; ListCell *specitem;
ListCell *iditem; ListCell *iditem;
Oid currentUserId = GetUserId();
Assert(list_length(memberSpecs) == list_length(memberIds)); Assert(list_length(memberSpecs) == list_length(memberIds));
@ -1859,7 +1863,7 @@ AddRoleMems(const char *rolename, Oid roleid,
* behavior: RESTRICT or CASCADE behavior for recursive removal * behavior: RESTRICT or CASCADE behavior for recursive removal
*/ */
static void static void
DelRoleMems(const char *rolename, Oid roleid, DelRoleMems(Oid currentUserId, const char *rolename, Oid roleid,
List *memberSpecs, List *memberIds, List *memberSpecs, List *memberIds,
Oid grantorId, GrantRoleOptions *popt, DropBehavior behavior) Oid grantorId, GrantRoleOptions *popt, DropBehavior behavior)
{ {
@ -1867,7 +1871,6 @@ DelRoleMems(const char *rolename, Oid roleid,
TupleDesc pg_authmem_dsc; TupleDesc pg_authmem_dsc;
ListCell *specitem; ListCell *specitem;
ListCell *iditem; ListCell *iditem;
Oid currentUserId = GetUserId();
CatCList *memlist; CatCList *memlist;
RevokeRoleGrantAction *actions; RevokeRoleGrantAction *actions;
int i; int i;

Loading…
Cancel
Save