Re: CREATEROLE users vs. role properties

On Wed, Jan 18, 2023 at 6:17 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
> > Here is a patch implementing the above proposal. Since this is fairly
> > closely related to already-committed work, I would like to get this
> > into v16. That way, all the changes to how CREATEROLE works will go
> > into a single release, which seems less confusing for users. It is
> > also fairly clear to me that this is an improvement over the status
> > quo. Sometimes things that seem clear to me turn out to be false, so
> > if this change seems like a problem to you, please let me know.
>
> This seems like a clear improvement to me.

Cool.

> 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...

-- 
Robert Haas
EDB: http://www.enterprisedb.com