Re: improving user.c error messages

Nathan Bossart <nathandbossart@gmail.com>

From: Nathan Bossart <nathandbossart@gmail.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Robert Haas <robertmhaas@gmail.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2023-01-27T23:15:07Z
Lists: pgsql-hackers

Attachments

On Fri, Jan 27, 2023 at 07:31:19PM +0100, Alvaro Herrera wrote:
> On 2023-Jan-26, Nathan Bossart wrote:
>>  			ereport(ERROR,
>>  					(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
>> -					 errmsg("permission denied: bootstrap user must be superuser")));
>> +					 errmsg("permission denied to alter role"),
>> +					 errdetail("The bootstrap user must be superuser.")));
> 
> I think this one isn't using the right errcode; this is not a case of
> insufficient privileges.  There's no priv you can acquire that lets you
> do it.  So I'd change it to unsupported operation.

І fixed this in v4.  I've also attached a second patch in which I've
adjusted the messages that Peter mentioned upthread.

One thing that feels a bit odd is how some of the DETAILs mention the
operation being attempted while others do not.  For example, we have

	ERROR:  permission denied to drop role
	DETAIL: You must have SUPERUSER privilege to drop roles with SUPERUSER.

In this case, the DETAIL explains the action that is prohibited.  In other
cases, we have something like

	ERROR:  permission denied to alter role
	DETAIL: You must have CREATEROLE privilege and ADMIN OPTION on role "myrole".

which does not.  I think this is okay because adding "to alter the role" to
the end of the DETAIL seems kind of awkward.  But in other cases, such as

	ERROR:  permission denied to use replication slots
	DETAIL:  You must have REPLICATION privilege.

adding the operation to the end seems less awkward (i.e., "You must have
REPLICATION privilege to use replication slots.").  I don't think there's
any information lost by omitting the action in the DETAIL, so perhaps this
is just a stylistic choice.  I think I'm inclined to add the action to the
DETAIL whenever it doesn't make the message lengthy and awkward, and leave
it out otherwise.  Thoughts?

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Improve several permission-related error messages.

  2. Integrate superuser check into has_rolreplication()

  3. Small code simplification

  4. Adjust interaction of CREATEROLE with role properties.

  5. Add new GUC reserved_connections.

  6. Rename ReservedBackends variable to SuperuserReservedConnections.

  7. Update docs and error message for superuser_reserved_connections.

  8. Add new GUC createrole_self_grant.

  9. Restrict the privileges of CREATEROLE users.

  10. Add a SET option to the GRANT command.