improving user.c error messages - Mailing list pgsql-hackers

moving this discussion to a new thread...

On Thu, Jan 19, 2023 at 10:20:33AM -0500, Robert Haas wrote:
> On Wed, Jan 18, 2023 at 6:17 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
>> However, as the attribute
>> system becomes more sophisticated, I think we ought to improve the error
>> messages in user.c.  IMHO messages like "permission denied" could be
>> greatly improved with some added context.
>>
>> This probably shouldn't block your patch, but I think it's worth doing in
>> v16 since there are other changes in this area.  I'm happy to help.
> 
> That would be great. I agree that it's good to try to improve the
> error messages. It hasn't been entirely clear to me how to do that.
> For instance, I don't think we want to say something like:
> 
> ERROR: must have CREATEROLE privilege and ADMIN OPTION on the target
> role, or in lieu of both of those to be superuser, to set the
> CONNECTION LIMIT for another role
> ERROR: must have CREATEROLE privilege and ADMIN OPTION on the target
> role, plus also CREATEDB, or in lieu of all that to be superuser, to
> remove the CREATEDB property from another role
> 
> Such messages are long and we'd end up with a lot of variants. It's
> possible that the messages could be multi-tier. For instance, if we
> determine that you're trying to manage users and you don't have
> permission to manage ANY user, we could say:
> 
> ERROR: permission to manage roles denied
> DETAIL: You must have the CREATEROLE privilege or be a superuser to
> manage roles.
> 
> If you could potentially manage some user, but not the one you're
> trying to manage, we could say:
> 
> ERROR: permission to manage role "%s" denied
> DETAIL: You need ADMIN OPTION on the target role to manage it.
> 
> If you have permission to manage the target role but not in the
> requested manner, we could then say something like:
> 
> ERROR: permission to manage CREATEDB for role "%s" denied
> DETAIL: You need CREATEDB to manage it.
> 
> This is just one idea, and maybe not the best one. I'm just trying to
> say that I think this is basically an organizational problem. We need
> a plan for how we're going to report errors that is not too
> complicated to implement with reasonable effort, and that will produce
> messages that users will understand. I'd be delighted if you wanted to
> provide either ideas or patches...

Here is an early draft of some modest improvements to the user.c error
messages.  I basically just tried to standardize the style of and add
context to the existing error messages.  I used errhint() for this extra
context, but errdetail() would work, too.  This isn't perfect.  You might
still have to go through a couple rounds of errors before your role has all
the privileges it needs for a command, but this seems to improve matters a
little.

I think there is still a lot of room for improvement, but I wanted to at
least get the discussion started before I went too far.

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