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

diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index 280031e6eb2fb461a0934851856bea4bf1f42ca9..bc2b95ac8148e6f34c3edf37fac3fb91848d659c 100644 (file)
--- a/src/backend/commands/user.c
+++ b/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() */
 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,
                        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,
                        Oid grantorId, GrantRoleOptions *popt,
                        DropBehavior behavior);
@@ -133,6 +133,7 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
    HeapTuple   tuple;
    Datum       new_record[Natts_pg_authid] = {0};
    bool        new_record_nulls[Natts_pg_authid] = {0};
+   Oid         currentUserId = GetUserId();
    Oid         roleid;
    ListCell   *item;
    ListCell   *option;
@@ -508,8 +509,8 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
            char       *oldrolename = NameStr(oldroleform->rolname);
 
            /* can only add this role to roles for which you have rights */
-           check_role_membership_authorization(GetUserId(), oldroleid, true);
-           AddRoleMems(oldrolename, oldroleid,
+           check_role_membership_authorization(currentUserId, oldroleid, true);
+           AddRoleMems(currentUserId, oldrolename, oldroleid,
                        thisrole_list,
                        thisrole_oidlist,
                        InvalidOid, &popt);
@@ -525,12 +526,12 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
     * NB: No permissions check is required here. If you have enough rights
     * to create a role, you can add any members you like.
     */
-   AddRoleMems(stmt->role, roleid,
+   AddRoleMems(currentUserId, stmt->role, roleid,
                rolemembers, roleSpecsToIds(rolemembers),
                InvalidOid, &popt);
    popt.specified |= GRANT_ROLE_SPECIFIED_ADMIN;
    popt.admin = true;
-   AddRoleMems(stmt->role, roleid,
+   AddRoleMems(currentUserId, stmt->role, roleid,
                adminmembers, roleSpecsToIds(adminmembers),
                InvalidOid, &popt);
 
@@ -583,6 +584,7 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
    DefElem    *dvalidUntil = NULL;
    DefElem    *dbypassRLS = NULL;
    Oid         roleid;
+   Oid         currentUserId = GetUserId();
    GrantRoleOptions    popt;
 
    check_rolespec_name(stmt->role,
@@ -727,13 +729,13 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
                     errmsg("permission denied")));
 
        /* without CREATEROLE, can only change your own password */
-       if (dpassword && roleid != GetUserId())
+       if (dpassword && roleid != currentUserId)
            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))
+       if (drolemembers && !is_admin_of_role(currentUserId, roleid))
            ereport(ERROR,
                    (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
                     errmsg("must have admin option on role \"%s\" to add members",
@@ -888,11 +890,11 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
        CommandCounterIncrement();
 
        if (stmt->action == +1) /* add members to role */
-           AddRoleMems(rolename, roleid,
+           AddRoleMems(currentUserId, rolename, roleid,
                        rolemembers, roleSpecsToIds(rolemembers),
                        InvalidOid, &popt);
        else if (stmt->action == -1)    /* drop members from role */
-           DelRoleMems(rolename, roleid,
+           DelRoleMems(currentUserId, rolename, roleid,
                        rolemembers, roleSpecsToIds(rolemembers),
                        InvalidOid, &popt, DROP_RESTRICT);
    }
@@ -1378,6 +1380,7 @@ GrantRole(ParseState *pstate, GrantRoleStmt *stmt)
    List       *grantee_ids;
    ListCell   *item;
    GrantRoleOptions    popt;
+   Oid         currentUserId = GetUserId();
 
    /* Parse options list. */
    InitGrantRoleOptions(&popt);
@@ -1449,14 +1452,14 @@ GrantRole(ParseState *pstate, GrantRoleStmt *stmt)
                     errmsg("column names cannot be included in GRANT/REVOKE ROLE")));
 
        roleid = get_role_oid(rolename, false);
-       check_role_membership_authorization(GetUserId(), roleid,
-                                           stmt->is_grant);
+       check_role_membership_authorization(currentUserId,
+                                           roleid, stmt->is_grant);
        if (stmt->is_grant)
-           AddRoleMems(rolename, roleid,
+           AddRoleMems(currentUserId, rolename, roleid,
                        stmt->grantee_roles, grantee_ids,
                        grantor, &popt);
        else
-           DelRoleMems(rolename, roleid,
+           DelRoleMems(currentUserId, rolename, roleid,
                        stmt->grantee_roles, grantee_ids,
                        grantor, &popt, stmt->behavior);
    }
@@ -1555,15 +1558,17 @@ roleSpecsToIds(List *memberNames)
 /*
  * 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)
  * 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 (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
  */
 static void
-AddRoleMems(const char *rolename, Oid roleid,
+AddRoleMems(Oid currentUserId, const char *rolename, Oid roleid,
            List *memberSpecs, List *memberIds,
            Oid grantorId, GrantRoleOptions *popt)
 {
@@ -1571,7 +1576,6 @@ AddRoleMems(const char *rolename, Oid roleid,
    TupleDesc   pg_authmem_dsc;
    ListCell   *specitem;
    ListCell   *iditem;
-   Oid         currentUserId = GetUserId();
 
    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
  */
 static void
-DelRoleMems(const char *rolename, Oid roleid,
+DelRoleMems(Oid currentUserId, const char *rolename, Oid roleid,
            List *memberSpecs, List *memberIds,
            Oid grantorId, GrantRoleOptions *popt, DropBehavior behavior)
 {
@@ -1867,7 +1871,6 @@ DelRoleMems(const char *rolename, Oid roleid,
    TupleDesc   pg_authmem_dsc;
    ListCell   *specitem;
    ListCell   *iditem;
-   Oid         currentUserId = GetUserId();
    CatCList   *memlist;
    RevokeRoleGrantAction *actions;
    int         i;