Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit25bb031

Browse files
committed
Refactor permissions-checking for role grants.
Instead of having checks in AddRoleMems() and DelRoleMems(), havethe callers perform checks where it's required. In some cases itisn't, either because the caller has already performed a check forthe same condition, or because the check couldn't possibly fail.The "Skip permission check if nothing to do" check in each ofAddRoleMems() and DelRoleMems() is pointless. Some call sitescan't pass an empty list. Others can, but in those cases, the rolebeing modified is one that the current user has just created.Therefore, they must have permission to modify it, and so nopermission check is required at all.This patch is intended to have no user-visible consequences. It isintended to simplify 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
1 parent3f7836f commit25bb031

File tree

1 file changed

+54
-62
lines changed

1 file changed

+54
-62
lines changed

‎src/backend/commands/user.c‎

Lines changed: 54 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ static void DelRoleMems(const char *rolename, Oid roleid,
9494
List*memberSpecs,List*memberIds,
9595
OidgrantorId,GrantRoleOptions*popt,
9696
DropBehaviorbehavior);
97+
staticvoidcheck_role_membership_authorization(OidcurrentUserId,Oidroleid,
98+
boolis_grant);
9799
staticOidcheck_role_grantor(OidcurrentUserId,Oidroleid,OidgrantorId,
98100
boolis_grant);
99101
staticRevokeRoleGrantAction*initialize_revoke_actions(CatCList*memlist);
@@ -505,6 +507,8 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
505507
Oidoldroleid=oldroleform->oid;
506508
char*oldrolename=NameStr(oldroleform->rolname);
507509

510+
/* can only add this role to roles for which you have rights */
511+
check_role_membership_authorization(GetUserId(),oldroleid, true);
508512
AddRoleMems(oldrolename,oldroleid,
509513
thisrole_list,
510514
thisrole_oidlist,
@@ -517,6 +521,9 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
517521
/*
518522
* Add the specified members to this new role. adminmembers get the admin
519523
* option, rolemembers don't.
524+
*
525+
* NB: No permissions check is required here. If you have enough rights
526+
* to create a role, you can add any members you like.
520527
*/
521528
AddRoleMems(stmt->role,roleid,
522529
rolemembers,roleSpecsToIds(rolemembers),
@@ -1442,6 +1449,8 @@ GrantRole(ParseState *pstate, GrantRoleStmt *stmt)
14421449
errmsg("column names cannot be included in GRANT/REVOKE ROLE")));
14431450

14441451
roleid=get_role_oid(rolename, false);
1452+
check_role_membership_authorization(GetUserId(),roleid,
1453+
stmt->is_grant);
14451454
if (stmt->is_grant)
14461455
AddRoleMems(rolename,roleid,
14471456
stmt->grantee_roles,grantee_ids,
@@ -1566,43 +1575,6 @@ AddRoleMems(const char *rolename, Oid roleid,
15661575

15671576
Assert(list_length(memberSpecs)==list_length(memberIds));
15681577

1569-
/* Skip permission check if nothing to do */
1570-
if (!memberIds)
1571-
return;
1572-
1573-
/*
1574-
* Check permissions: must have createrole or admin option on the role to
1575-
* be changed. To mess with a superuser role, you gotta be superuser.
1576-
*/
1577-
if (superuser_arg(roleid))
1578-
{
1579-
if (!superuser())
1580-
ereport(ERROR,
1581-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
1582-
errmsg("must be superuser to alter superusers")));
1583-
}
1584-
else
1585-
{
1586-
if (!have_createrole_privilege()&&
1587-
!is_admin_of_role(currentUserId,roleid))
1588-
ereport(ERROR,
1589-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
1590-
errmsg("must have admin option on role \"%s\"",
1591-
rolename)));
1592-
}
1593-
1594-
/*
1595-
* The charter of pg_database_owner is to have exactly one, implicit,
1596-
* situation-dependent member. There's no technical need for this
1597-
* restriction. (One could lift it and take the further step of making
1598-
* object_ownercheck(DatabaseRelationId, ...) equivalent to has_privs_of_role(roleid,
1599-
* ROLE_PG_DATABASE_OWNER), in which case explicit, situation-independent
1600-
* members could act as the owner of any database.)
1601-
*/
1602-
if (roleid==ROLE_PG_DATABASE_OWNER)
1603-
ereport(ERROR,
1604-
errmsg("role \"%s\" cannot have explicit members",rolename));
1605-
16061578
/* Validate grantor (and resolve implicit grantor if not specified). */
16071579
grantorId=check_role_grantor(currentUserId,roleid,grantorId, true);
16081580

@@ -1902,31 +1874,6 @@ DelRoleMems(const char *rolename, Oid roleid,
19021874

19031875
Assert(list_length(memberSpecs)==list_length(memberIds));
19041876

1905-
/* Skip permission check if nothing to do */
1906-
if (!memberIds)
1907-
return;
1908-
1909-
/*
1910-
* Check permissions: must have createrole or admin option on the role to
1911-
* be changed. To mess with a superuser role, you gotta be superuser.
1912-
*/
1913-
if (superuser_arg(roleid))
1914-
{
1915-
if (!superuser())
1916-
ereport(ERROR,
1917-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
1918-
errmsg("must be superuser to alter superusers")));
1919-
}
1920-
else
1921-
{
1922-
if (!have_createrole_privilege()&&
1923-
!is_admin_of_role(currentUserId,roleid))
1924-
ereport(ERROR,
1925-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
1926-
errmsg("must have admin option on role \"%s\"",
1927-
rolename)));
1928-
}
1929-
19301877
/* Validate grantor (and resolve implicit grantor if not specified). */
19311878
grantorId=check_role_grantor(currentUserId,roleid,grantorId, false);
19321879

@@ -2040,6 +1987,51 @@ DelRoleMems(const char *rolename, Oid roleid,
20401987
table_close(pg_authmem_rel,NoLock);
20411988
}
20421989

1990+
/*
1991+
* Check that currentUserId has permission to modify the membership list for
1992+
* roleid. Throw an error if not.
1993+
*/
1994+
staticvoid
1995+
check_role_membership_authorization(OidcurrentUserId,Oidroleid,
1996+
boolis_grant)
1997+
{
1998+
/*
1999+
* The charter of pg_database_owner is to have exactly one, implicit,
2000+
* situation-dependent member. There's no technical need for this
2001+
* restriction. (One could lift it and take the further step of making
2002+
* object_ownercheck(DatabaseRelationId, ...) equivalent to
2003+
* has_privs_of_role(roleid, ROLE_PG_DATABASE_OWNER), in which case
2004+
* explicit, situation-independent members could act as the owner of any
2005+
* database.)
2006+
*/
2007+
if (is_grant&&roleid==ROLE_PG_DATABASE_OWNER)
2008+
ereport(ERROR,
2009+
errmsg("role \"%s\" cannot have explicit members",
2010+
GetUserNameFromId(roleid, false)));
2011+
2012+
/* To mess with a superuser role, you gotta be superuser. */
2013+
if (superuser_arg(roleid))
2014+
{
2015+
if (!superuser_arg(currentUserId))
2016+
ereport(ERROR,
2017+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
2018+
errmsg("must be superuser to alter superusers")));
2019+
}
2020+
else
2021+
{
2022+
/*
2023+
* Otherwise, must have createrole or admin option on the role to be
2024+
* changed.
2025+
*/
2026+
if (!has_createrole_privilege(currentUserId)&&
2027+
!is_admin_of_role(currentUserId,roleid))
2028+
ereport(ERROR,
2029+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
2030+
errmsg("must have admin option on role \"%s\"",
2031+
GetUserNameFromId(roleid, false))));
2032+
}
2033+
}
2034+
20432035
/*
20442036
* Sanity-check, or infer, the grantor for a GRANT or REVOKE statement
20452037
* targeting a role.

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp