CREATEROLE does not permit commenting on newly-created roles

Started by Owen Jacobsonabout 15 years ago10 messagesbugs
Jump to latest
#1Owen Jacobson
owen.jacobson@grimoire.ca

Salut,

I'm making use of database-side comments to document the oddities and
intended uses for various database symbols. While this is mostly going
well, it's falling down a bit for documenting role objects. For the
record, I'm running the EnterpriseDB Postgres 9.0.3 package on OS X
10.6 as well as Martin Pitt's Postgres 9.0 PPA on Ubuntu 10.10
(x86_64).

Steps to reproduce:

1. Create a role with CREATEROLE to act as an owner
("comment_createrole" in the repro transcript below).
2. Log in as this role and issue the following statements:

CREATE ROLE commented_role;
COMMENT ON ROLE commented_role IS 'A comment';

Expected results:

The new role has the comment 'A comment' applied to it.

Actual results:

psql:repro.sql:2: ERROR: must be member of role "commented_role" to
comment upon it

While I realize that roles have no owner, and therefore no obvious
person who should be able to fiddle the comment, being unable to
comment on newly-created roles without joining them really limits the
usefulness of role comments for database administrators. It
intuitively (to me, anyways) feels like CREATEROLE should permit
commenting on (non-superuser) roles.

-o

P.S. A complete reproduction:

$ cat repro.sql
CREATE ROLE commented_role;
COMMENT ON ROLE commented_role IS 'A comment';

$ createdb pg-comment-role
$ psql -U postgres pg-comment-role
psql (9.0.3)
Type "help" for help.

pg-comment-role=# CREATE ROLE comment_createrole WITH LOGIN CREATEROLE
PASSWORD 'comment_createrole';
CREATE ROLE

pg-comment-role=# \q
$ psql -h localhost -U comment_createrole pg-comment-role
psql (9.0.3)
Type "help" for help.

pg-comment-role=> \i repro.sql
CREATE ROLE
psql:repro.sql:2: ERROR: must be member of role "commented_role" to
comment upon it

In reply to: Owen Jacobson (#1)
Re: CREATEROLE does not permit commenting on newly-created roles

Em 07-03-2011 16:53, Owen Jacobson escreveu:

psql:repro.sql:2: ERROR: must be member of role "commented_role" to
comment upon it

This isn't a bug; let say it is a limitation (and a documented one [1]http://www.postgresql.org/docs/9.0/static/sql-comment.html).
Unfortunately only the role, superuser or its members can add/drop comments.

[1]: http://www.postgresql.org/docs/9.0/static/sql-comment.html

--
Euler Taveira de Oliveira
http://www.timbira.com/

#3Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Euler Taveira de Oliveira (#2)
Re: CREATEROLE does not permit commenting on newly-created roles

Excerpts from Euler Taveira de Oliveira's message of mar mar 08 02:06:13 -0300 2011:

Em 07-03-2011 16:53, Owen Jacobson escreveu:

psql:repro.sql:2: ERROR: must be member of role "commented_role" to
comment upon it

This isn't a bug; let say it is a limitation (and a documented one [1]).
Unfortunately only the role, superuser or its members can add/drop comments.

Maybe it would be good to have a COMMENT clause on the CREATE ROLE
command. It would be inconsistent with the rest of the comment system,
but this privilege problem is inconsistent too.

--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#3)
Re: CREATEROLE does not permit commenting on newly-created roles

Alvaro Herrera <alvherre@commandprompt.com> writes:

Excerpts from Euler Taveira de Oliveira's message of mar mar 08 02:06:13 -0300 2011:

Em 07-03-2011 16:53, Owen Jacobson escreveu:

psql:repro.sql:2: ERROR: must be member of role "commented_role" to
comment upon it

This isn't a bug; let say it is a limitation (and a documented one [1]).
Unfortunately only the role, superuser or its members can add/drop comments.

Maybe it would be good to have a COMMENT clause on the CREATE ROLE
command. It would be inconsistent with the rest of the comment system,
but this privilege problem is inconsistent too.

I thought there was nothing particularly unreasonable about Owen's
suggestion: let users with the CREATEROLE attribute comment on any role.
I don't think COMMENT added to CREATE ROLE would be a very nice fix
(aside from being ugly, what if you want to change the comment later?).

It strikes me actually that letting members of the role comment on it
is not an amazingly good idea. They are not owners of the role in any
meaningful sense --- for instance, they can't drop it. It'd be more
reasonable and consistent to say that only superusers and holders of
CREATEROLE can do COMMENT ON ROLE.

regards, tom lane

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#4)
Re: CREATEROLE does not permit commenting on newly-created roles

I wrote:

I thought there was nothing particularly unreasonable about Owen's
suggestion: let users with the CREATEROLE attribute comment on any role.
I don't think COMMENT added to CREATE ROLE would be a very nice fix
(aside from being ugly, what if you want to change the comment later?).

It strikes me actually that letting members of the role comment on it
is not an amazingly good idea. They are not owners of the role in any
meaningful sense --- for instance, they can't drop it. It'd be more
reasonable and consistent to say that only superusers and holders of
CREATEROLE can do COMMENT ON ROLE.

In particular, I suggest the attached patch (code-complete, but sans
documentation changes). The changes here bring COMMENT ON ROLE into
line with the permission requirements for other operations on roles
that require ownership-like permissions. This patch modifies
check_object_ownership, which means it affects three call sites at
present:

COMMENT ON ROLE

ALTER EXTENSION ADD/DROP (but the target object cannot be a role)

SECURITY LABEL IS (also couldn't be a role, at the moment)

The SECURITY LABEL case, even though it's presently unimplemented,
seems to me to be a darn good argument for redefining the notion
of "role ownership" like this. Who would want a mere member of some
group role to be able to set that role's security label?

Comments, objections?

regards, tom lane

#6Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#5)
Re: CREATEROLE does not permit commenting on newly-created roles

On Tue, Mar 8, 2011 at 11:48 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I wrote:

I thought there was nothing particularly unreasonable about Owen's
suggestion: let users with the CREATEROLE attribute comment on any role.
I don't think COMMENT added to CREATE ROLE would be a very nice fix
(aside from being ugly, what if you want to change the comment later?).

It strikes me actually that letting members of the role comment on it
is not an amazingly good idea.  They are not owners of the role in any
meaningful sense --- for instance, they can't drop it.  It'd be more
reasonable and consistent to say that only superusers and holders of
CREATEROLE can do COMMENT ON ROLE.

In particular, I suggest the attached patch (code-complete, but sans
documentation changes).  The changes here bring COMMENT ON ROLE into
line with the permission requirements for other operations on roles
that require ownership-like permissions.  This patch modifies
check_object_ownership, which means it affects three call sites at
present:

       COMMENT ON ROLE

       ALTER EXTENSION ADD/DROP (but the target object cannot be a role)

       SECURITY LABEL IS (also couldn't be a role, at the moment)

The SECURITY LABEL case, even though it's presently unimplemented,
seems to me to be a darn good argument for redefining the notion
of "role ownership" like this.  Who would want a mere member of some
group role to be able to set that role's security label?

Comments, objections?

I think it's a good change, but we should make sure to release-note it
properly, along with the change you made for PLs.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#6)
Re: CREATEROLE does not permit commenting on newly-created roles

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Mar 8, 2011 at 11:48 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

In particular, I suggest the attached patch (code-complete, but sans
documentation changes).

I think it's a good change, but we should make sure to release-note it
properly,

I had already drafted a commit message:

Adjust the permissions required for COMMENT ON ROLE.

Formerly, any member of a role could change the role's comment, as of
course could superusers; but holders of CREATEROLE privilege could not,
unless they were also members. This led to the odd situation that a
CREATEROLE holder could create a role but then could not comment on it.
It also seems a bit dubious to let an unprivileged user change his own
comment, let alone those of group roles he belongs to. So, change the
rule to be "you must be superuser to comment on a superuser role, or
hold CREATEROLE to comment on non-superuser roles". This is the same
as the privilege check for creating/dropping roles, and thus fits much
better with the rule for other object types, namely that only the owner
of an object can comment on it.

Per complaint from Owen Jacobson and subsequent discussion.

How that gets boiled down into a release note will depend on whoever
writes the release notes.

along with the change you made for PLs.

Hrm?

regards, tom lane

#8Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#7)
Re: CREATEROLE does not permit commenting on newly-created roles

On Wed, Mar 9, 2011 at 12:45 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

How that gets boiled down into a release note will depend on whoever
writes the release notes.

along with the change you made for PLs.

In 9.0, to comment on a procedural language, you must be superuser.
But your recent commit changed it to check for language ownership.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#9Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#8)
Re: CREATEROLE does not permit commenting on newly-created roles

Robert Haas <robertmhaas@gmail.com> writes:

On Wed, Mar 9, 2011 at 12:45 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

along with the change you made for PLs.

Huh?

In 9.0, to comment on a procedural language, you must be superuser.
But your recent commit changed it to check for language ownership.

Oh. That's just a bug fix: it should have been changed when
we invented pg_language.lanowner.

regards, tom lane

#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#5)
Re: CREATEROLE does not permit commenting on newly-created roles

I wrote:

In particular, I suggest the attached patch (code-complete, but sans
documentation changes).

Applied with doc fixes. I waited till after alpha4 was cut, though.

regards, tom lane