Disregard superuserness when checking to see if a role GRANT would

tglsfdc · tglsfdc · commit c3d8de0907b5 · 2005-11-04T17:25:15.000Z
create circularity of role memberships.  This is a minimum-impact fix
for the problem reported by Florian Pflug.  I thought about removing
the superuser_arg test from is_member_of_role() altogether, as it seems
redundant for many of the callers --- but not all, and it's way too late
in the 8.1 cycle to be making large changes.  Perhaps reconsider this
later.
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
@@ -6,7 +6,7 @@
  * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/backend/commands/user.c,v 1.163 2005/10/29 00:31:51 petere Exp $
+ * $PostgreSQL: pgsql/src/backend/commands/user.c,v 1.164 2005/11/04 17:25:15 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1214,9 +1214,10 @@ AddRoleMems(const char *rolename, Oid roleid,
 		 * Refuse creation of membership loops, including the trivial case
 		 * where a role is made a member of itself.  We do this by checking to
 		 * see if the target role is already a member of the proposed member
-		 * role.
+		 * role.  We have to ignore possible superuserness, however, else we
+		 * could never grant membership in a superuser-privileged role.
 		 */
-		if (is_member_of_role(roleid, memberid))
+		if (is_member_of_role_nosuper(roleid, memberid))
 			ereport(ERROR,
 					(errcode(ERRCODE_INVALID_GRANT_OPERATION),
 					 (errmsg("role \"%s\" is a member of role \"%s\"",
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/adt/acl.c,v 1.126 2005/10/15 02:49:27 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/adt/acl.c,v 1.127 2005/11/04 17:25:15 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -3067,6 +3067,26 @@ check_is_member_of_role(Oid member, Oid role)
 						GetUserNameFromId(role))));
 }
 
+/*
+ * Is member a member of role, not considering superuserness?
+ *
+ * This is identical to is_member_of_role except we ignore superuser
+ * status.
+ */
+bool
+is_member_of_role_nosuper(Oid member, Oid role)
+{
+	/* Fast path for simple case */
+	if (member == role)
+		return true;
+
+	/*
+	 * Find all the roles that member is a member of, including multi-level
+	 * recursion, then see if target role is any one of them.
+	 */
+	return list_member_oid(roles_is_member_of(member), role);
+}
+
 
 /*
  * Is member an admin of role (directly or indirectly)?  That is, is it
diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/utils/acl.h,v 1.85 2005/10/15 02:49:46 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/utils/acl.h,v 1.86 2005/11/04 17:25:15 tgl Exp $
  *
  * NOTES
  *	  An ACL array is simply an array of AclItems, representing the union
@@ -212,6 +212,7 @@ extern int	aclmembers(const Acl *acl, Oid **roleids);
 
 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 void check_is_member_of_role(Oid member, Oid role);
 

Original file line number	Diff line number	Diff line change
`@@ -7,7 +7,7 @@`
`7`	`7`	`* Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group`
`8`	`8`	`* Portions Copyright (c) 1994, Regents of the University of California`
`9`	`9`	`*`
`10`		`- * $PostgreSQL: pgsql/src/include/utils/acl.h,v 1.85 2005/10/15 02:49:46 momjian Exp $`
	`10`	`+ * $PostgreSQL: pgsql/src/include/utils/acl.h,v 1.86 2005/11/04 17:25:15 tgl Exp $`
`11`	`11`	`*`
`12`	`12`	`* NOTES`
`13`	`13`	`* An ACL array is simply an array of AclItems, representing the union`
`@@ -212,6 +212,7 @@ extern int aclmembers(const Acl acl, Oid *roleids);`
`212`	`212`
`213`	`213`	`extern bool has_privs_of_role(Oid member, Oid role);`
`214`	`214`	`extern bool is_member_of_role(Oid member, Oid role);`
	`215`	`+extern bool is_member_of_role_nosuper(Oid member, Oid role);`
`215`	`216`	`extern bool is_admin_of_role(Oid member, Oid role);`
`216`	`217`	`extern void check_is_member_of_role(Oid member, Oid role);`
`217`	`218`