From: Nathan Bossart <nathandbossart@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Cc: tgl@sss.pgh.pa.us
Subject: Re: improving user.c error messages
Date: Thu, 26 Jan 2023 11:59:29 -0800
Message-ID: <20230126195929.GB1702315@nathanxps13> (raw)
In-Reply-To: <CA+TgmoYZcZ-PUp7muZEG20Jz4BtgUSngA3p7k=Nk-_8FayO5xQ@mail.gmail.com>
References: <20230126002251.GA1506128@nathanxps13>
<20230126090739.nspznasnb4x3v5l4@alvherre.pgsql>
<20230126191358.GA1660026@nathanxps13>
<CA+TgmoYZcZ-PUp7muZEG20Jz4BtgUSngA3p7k=Nk-_8FayO5xQ@mail.gmail.com>
On Thu, Jan 26, 2023 at 02:42:05PM -0500, Robert Haas wrote:
> @@ -758,16 +776,13 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
> {
> /* things an unprivileged user certainly can't do */
> if (dinherit || dcreaterole || dcreatedb || dcanlogin || dconnlimit ||
> - dvalidUntil || disreplication || dbypassRLS)
> + dvalidUntil || disreplication || dbypassRLS ||
> + (dpassword && roleid != currentUserId))
> ereport(ERROR,
> (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
> - errmsg("permission denied")));
> -
> - /* an unprivileged user can change their own password */
> - if (dpassword && roleid != currentUserId)
> - ereport(ERROR,
> - (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
> - errmsg("must have CREATEROLE privilege to change another user's password")));
> + errmsg("permission denied to alter role"),
> + errdetail("You must have %s privilege and %s on role \"%s\".",
> + "CREATEROLE", "ADMIN OPTION", rolename)));
> }
> else if (!superuser())
> {
>
> Basically my question is whether having one error message for all of
> those cases is good enough, or whether we should be trying harder. I
> don't mind if the conclusion is that it's OK as-is, and I'm not
> entirely sure what would be better. But when I was working on this
> code, all of those cases OR'd together feeding into a single error
> message seemed a little sketchy to me, so I am wondering what others
> think.
I wondered the same thing, but I hesitated because I didn't want to change
too much in a patch for error messaging. I can give it a try.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: nathandbossart@gmail.com, robertmhaas@gmail.com, alvherre@alvh.no-ip.org, tgl@sss.pgh.pa.us
Subject: Re: improving user.c error messages
In-Reply-To: <20230126195929.GB1702315@nathanxps13>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox