Refactor AlterRole()
Get rid of the three-valued logic for the Boolean variables to track whether the value was been specified and what the new value should be. Instead, we can use the "dfoo" variables to determine whether the value was specified and should be applied. This was already done in some cases, so this makes this more uniform and removes one layer of indirection. Reviewed-by: Pavel Stehule <pavel.stehule@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/8c1a2e37-c68d-703c-5a83-7a6077f4f997@enterprisedb.com
This commit is contained in:
Peter Eisentraut
parent
bb42bfb5cc
commit
93415a3b5a
|
|
@ -501,20 +501,12 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
|
|||
new_tuple;
|
||||
Form_pg_authid authform;
|
||||
ListCell *option;
|
||||
char *rolename = NULL;
|
||||
char *rolename;
|
||||
char *password = NULL; /* user password */
|
||||
int issuper = -1; /* Make the user a superuser? */
|
||||
int inherit = -1; /* Auto inherit privileges? */
|
||||
int createrole = -1; /* Can this user create roles? */
|
||||
int createdb = -1; /* Can the user create databases? */
|
||||
int canlogin = -1; /* Can this user login? */
|
||||
int isreplication = -1; /* Is this a replication role? */
|
||||
int connlimit = -1; /* maximum connections allowed */
|
||||
List *rolemembers = NIL; /* roles to be added/removed */
|
||||
char *validUntil = NULL; /* time the login is valid until */
|
||||
Datum validUntil_datum; /* same, as timestamptz Datum */
|
||||
bool validUntil_null;
|
||||
int bypassrls = -1;
|
||||
DefElem *dpassword = NULL;
|
||||
DefElem *dissuper = NULL;
|
||||
DefElem *dinherit = NULL;
|
||||
|
|
@ -610,18 +602,6 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
|
|||
|
||||
if (dpassword && dpassword->arg)
|
||||
password = strVal(dpassword->arg);
|
||||
if (dissuper)
|
||||
issuper = intVal(dissuper->arg);
|
||||
if (dinherit)
|
||||
inherit = intVal(dinherit->arg);
|
||||
if (dcreaterole)
|
||||
createrole = intVal(dcreaterole->arg);
|
||||
if (dcreatedb)
|
||||
createdb = intVal(dcreatedb->arg);
|
||||
if (dcanlogin)
|
||||
canlogin = intVal(dcanlogin->arg);
|
||||
if (disreplication)
|
||||
isreplication = intVal(disreplication->arg);
|
||||
if (dconnlimit)
|
||||
{
|
||||
connlimit = intVal(dconnlimit->arg);
|
||||
|
|
@ -630,12 +610,8 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
|
|||
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
|
||||
errmsg("invalid connection limit: %d", connlimit)));
|
||||
}
|
||||
if (drolemembers)
|
||||
rolemembers = (List *) drolemembers->arg;
|
||||
if (dvalidUntil)
|
||||
validUntil = strVal(dvalidUntil->arg);
|
||||
if (dbypassRLS)
|
||||
bypassrls = intVal(dbypassRLS->arg);
|
||||
|
||||
/*
|
||||
* Scan the pg_authid relation to be certain the user exists.
|
||||
|
|
@ -654,21 +630,21 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
|
|||
* property. Otherwise, if you don't have createrole, you're only allowed
|
||||
* to change your own password.
|
||||
*/
|
||||
if (authform->rolsuper || issuper >= 0)
|
||||
if (authform->rolsuper || dissuper)
|
||||
{
|
||||
if (!superuser())
|
||||
ereport(ERROR,
|
||||
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
|
||||
errmsg("must be superuser to alter superuser roles or change superuser attribute")));
|
||||
}
|
||||
else if (authform->rolreplication || isreplication >= 0)
|
||||
else if (authform->rolreplication || disreplication)
|
||||
{
|
||||
if (!superuser())
|
||||
ereport(ERROR,
|
||||
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
|
||||
errmsg("must be superuser to alter replication roles or change replication attribute")));
|
||||
}
|
||||
else if (bypassrls >= 0)
|
||||
else if (dbypassRLS)
|
||||
{
|
||||
if (!superuser())
|
||||
ereport(ERROR,
|
||||
|
|
@ -677,23 +653,16 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
|
|||
}
|
||||
else if (!have_createrole_privilege())
|
||||
{
|
||||
/* We already checked issuper, isreplication, and bypassrls */
|
||||
if (!(inherit < 0 &&
|
||||
createrole < 0 &&
|
||||
createdb < 0 &&
|
||||
canlogin < 0 &&
|
||||
!dconnlimit &&
|
||||
!rolemembers &&
|
||||
!validUntil &&
|
||||
dpassword &&
|
||||
roleid == GetUserId()))
|
||||
/* check the rest */
|
||||
if (dinherit || dcreaterole || dcreatedb || dcanlogin || dconnlimit ||
|
||||
drolemembers || dvalidUntil || !dpassword || roleid != GetUserId())
|
||||
ereport(ERROR,
|
||||
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
|
||||
errmsg("permission denied")));
|
||||
}
|
||||
|
||||
/* Convert validuntil to internal form */
|
||||
if (validUntil)
|
||||
if (dvalidUntil)
|
||||
{
|
||||
validUntil_datum = DirectFunctionCall3(timestamptz_in,
|
||||
CStringGetDatum(validUntil),
|
||||
|
|
@ -729,39 +698,39 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
|
|||
/*
|
||||
* issuper/createrole/etc
|
||||
*/
|
||||
if (issuper >= 0)
|
||||
if (dissuper)
|
||||
{
|
||||
new_record[Anum_pg_authid_rolsuper - 1] = BoolGetDatum(issuper > 0);
|
||||
new_record[Anum_pg_authid_rolsuper - 1] = BoolGetDatum(intVal(dissuper->arg));
|
||||
new_record_repl[Anum_pg_authid_rolsuper - 1] = true;
|
||||
}
|
||||
|
||||
if (inherit >= 0)
|
||||
if (dinherit)
|
||||
{
|
||||
new_record[Anum_pg_authid_rolinherit - 1] = BoolGetDatum(inherit > 0);
|
||||
new_record[Anum_pg_authid_rolinherit - 1] = BoolGetDatum(intVal(dinherit->arg));
|
||||
new_record_repl[Anum_pg_authid_rolinherit - 1] = true;
|
||||
}
|
||||
|
||||
if (createrole >= 0)
|
||||
if (dcreaterole)
|
||||
{
|
||||
new_record[Anum_pg_authid_rolcreaterole - 1] = BoolGetDatum(createrole > 0);
|
||||
new_record[Anum_pg_authid_rolcreaterole - 1] = BoolGetDatum(intVal(dcreaterole->arg));
|
||||
new_record_repl[Anum_pg_authid_rolcreaterole - 1] = true;
|
||||
}
|
||||
|
||||
if (createdb >= 0)
|
||||
if (dcreatedb)
|
||||
{
|
||||
new_record[Anum_pg_authid_rolcreatedb - 1] = BoolGetDatum(createdb > 0);
|
||||
new_record[Anum_pg_authid_rolcreatedb - 1] = BoolGetDatum(intVal(dcreatedb->arg));
|
||||
new_record_repl[Anum_pg_authid_rolcreatedb - 1] = true;
|
||||
}
|
||||
|
||||
if (canlogin >= 0)
|
||||
if (dcanlogin)
|
||||
{
|
||||
new_record[Anum_pg_authid_rolcanlogin - 1] = BoolGetDatum(canlogin > 0);
|
||||
new_record[Anum_pg_authid_rolcanlogin - 1] = BoolGetDatum(intVal(dcanlogin->arg));
|
||||
new_record_repl[Anum_pg_authid_rolcanlogin - 1] = true;
|
||||
}
|
||||
|
||||
if (isreplication >= 0)
|
||||
if (disreplication)
|
||||
{
|
||||
new_record[Anum_pg_authid_rolreplication - 1] = BoolGetDatum(isreplication > 0);
|
||||
new_record[Anum_pg_authid_rolreplication - 1] = BoolGetDatum(intVal(disreplication->arg));
|
||||
new_record_repl[Anum_pg_authid_rolreplication - 1] = true;
|
||||
}
|
||||
|
||||
|
|
@ -808,9 +777,9 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
|
|||
new_record_nulls[Anum_pg_authid_rolvaliduntil - 1] = validUntil_null;
|
||||
new_record_repl[Anum_pg_authid_rolvaliduntil - 1] = true;
|
||||
|
||||
if (bypassrls >= 0)
|
||||
if (dbypassRLS)
|
||||
{
|
||||
new_record[Anum_pg_authid_rolbypassrls - 1] = BoolGetDatum(bypassrls > 0);
|
||||
new_record[Anum_pg_authid_rolbypassrls - 1] = BoolGetDatum(intVal(dbypassRLS->arg));
|
||||
new_record_repl[Anum_pg_authid_rolbypassrls - 1] = true;
|
||||
}
|
||||
|
||||
|
|
@ -827,17 +796,21 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
|
|||
* Advance command counter so we can see new record; else tests in
|
||||
* AddRoleMems may fail.
|
||||
*/
|
||||
if (rolemembers)
|
||||
if (drolemembers)
|
||||
{
|
||||
List *rolemembers = (List *) drolemembers->arg;
|
||||
|
||||
CommandCounterIncrement();
|
||||
|
||||
if (stmt->action == +1) /* add members to role */
|
||||
AddRoleMems(rolename, roleid,
|
||||
rolemembers, roleSpecsToIds(rolemembers),
|
||||
GetUserId(), false);
|
||||
else if (stmt->action == -1) /* drop members from role */
|
||||
DelRoleMems(rolename, roleid,
|
||||
rolemembers, roleSpecsToIds(rolemembers),
|
||||
false);
|
||||
if (stmt->action == +1) /* add members to role */
|
||||
AddRoleMems(rolename, roleid,
|
||||
rolemembers, roleSpecsToIds(rolemembers),
|
||||
GetUserId(), false);
|
||||
else if (stmt->action == -1) /* drop members from role */
|
||||
DelRoleMems(rolename, roleid,
|
||||
rolemembers, roleSpecsToIds(rolemembers),
|
||||
false);
|
||||
}
|
||||
|
||||
/*
|
||||
* Close pg_authid, but keep lock till commit.
|
||||
|
|
|
|||
Loading…
Reference in New Issue