Fix a wrong errmsg in AlterRole()
Hi,
```
postgres=# create group g1;
CREATE ROLE
postgres=# create role r1 in group g1;
CREATE ROLE
postgres=# set session authorization r1;
SET
postgres=> alter group g1 drop user r1;
ERROR: permission denied to alter role
DETAIL: Only roles with the ADMIN option on role "g1" may add members.
```
The errmsg is "add members", but we are doing a drop.
Here is a small patch to fix it.
--
Regards,
ChangAo Chen
Attachments:
0001-Fix-a-wrong-errmsg-in-AlterRole.patchapplication/octet-stream; charset=ISO-8859-1; name=0001-Fix-a-wrong-errmsg-in-AlterRole.patchDownload
From 255fc96c0b636f0cd233425d5d02f513315e0ba6 Mon Sep 17 00:00:00 2001
From: ChangAo Chen <cca5507@qq.com>
Date: Wed, 8 Jan 2025 16:27:55 +0800
Subject: [PATCH] Fix a wrong errmsg in AlterRole().
---
src/backend/commands/user.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index 266635d5e2..42fabc51dc 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -817,13 +817,22 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
"BYPASSRLS", "BYPASSRLS")));
}
- /* To add members to a role, you need ADMIN OPTION. */
+ /* To add/drop members to/from a role, you need ADMIN OPTION. */
if (drolemembers && !is_admin_of_role(currentUserId, roleid))
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- errmsg("permission denied to alter role"),
- errdetail("Only roles with the %s option on role \"%s\" may add members.",
- "ADMIN", rolename)));
+ {
+ if (stmt->action == +1)
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("permission denied to alter role"),
+ errdetail("Only roles with the %s option on role \"%s\" may add members.",
+ "ADMIN", rolename)));
+ else if (stmt->action == -1)
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("permission denied to alter role"),
+ errdetail("Only roles with the %s option on role \"%s\" may drop members.",
+ "ADMIN", rolename)));
+ }
/* Convert validuntil to internal form */
if (dvalidUntil)
--
2.34.1
"=?ISO-8859-1?B?Y2NhNTUwNw==?=" <cca5507@qq.com> writes:
postgres=> alter group g1 drop user r1;
ERROR: permission denied to alter role
DETAIL: Only roles with the ADMIN option on role "g1" may add members.
The errmsg is "add members", but we are doing a drop.
Here is a small patch to fix it.
Hmm. Seems simpler to just make the errdetail say "... may add or
drop members."
regards, tom lane
On Wed, Jan 08, 2025 at 10:29:12AM -0500, Tom Lane wrote:
"=?ISO-8859-1?B?Y2NhNTUwNw==?=" <cca5507@qq.com> writes:
postgres=> alter group g1 drop user r1;
ERROR: permission denied to alter role
DETAIL: Only roles with the ADMIN option on role "g1" may add members.The errmsg is "add members", but we are doing a drop.
Here is a small patch to fix it.Hmm. Seems simpler to just make the errdetail say "... may add or
drop members."
+1. It looks like this goes back to commit ce6b672 (v16). The error
message was changed in commit de4d456, which is also a v16 change, so
fortunately this has a good chance of back-patching cleanly.
--
nathan
Hi,
I modified the patch according to Tom's suggestion.
--
Regards,
ChangAo Chen
Attachments:
v2-0001-Fix-a-wrong-errdetail-in-AlterRole.patchapplication/octet-stream; charset=ISO-8859-1; name=v2-0001-Fix-a-wrong-errdetail-in-AlterRole.patchDownload
From b6cc5cf86bcf17f65e058a93c435ed95dfa743cf Mon Sep 17 00:00:00 2001
From: ChangAo Chen <cca5507@qq.com>
Date: Thu, 9 Jan 2025 10:05:08 +0800
Subject: [PATCH v2] Fix a wrong errdetail in AlterRole().
---
src/backend/commands/user.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index 266635d5e2..c74a9768fc 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -817,12 +817,12 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
"BYPASSRLS", "BYPASSRLS")));
}
- /* To add members to a role, you need ADMIN OPTION. */
+ /* To add/drop members to/from a role, you need ADMIN OPTION. */
if (drolemembers && !is_admin_of_role(currentUserId, roleid))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied to alter role"),
- errdetail("Only roles with the %s option on role \"%s\" may add members.",
+ errdetail("Only roles with the %s option on role \"%s\" may add or drop members.",
"ADMIN", rolename)));
/* Convert validuntil to internal form */
--
2.34.1
On Thu, Jan 09, 2025 at 10:13:11AM +0800, cca5507 wrote:
I modified the patch according to Tom's suggestion.
LGTM. I'll plan on committing this shortly.
--
nathan