Thread: Raise a WARNING if a REVOKE affects nothing?
Hi all I'm seeing lots of confusion from people about why: REVOKE CONNECT ON DATABASE foo FROM someuser; doesn't stop them connecting. Users seem to struggle to understand that: - There's a default GRANT to public; and - REVOKE removes existing permissions, it doesn't add deny rules It'd really help if REVOKE consistently raised warnings when it didn't actually revoke anything. Even better, a special case for REVOKEs on objects that only have owner and public permissions could say: WARNING: REVOKE didn't remove any permissions for user <blah>. This <table/db/whatever> has default permissions, so there were no GRANTs for user <blah> to revoke. See the documentation for REVOKE for more information. Opinions? -- Craig Ringer
That sounds like a good change to me. -- Darren Duncan Craig Ringer wrote: > Hi all > > I'm seeing lots of confusion from people about why: > > REVOKE CONNECT ON DATABASE foo FROM someuser; > > doesn't stop them connecting. Users seem to struggle to understand that: > > - There's a default GRANT to public; and > - REVOKE removes existing permissions, it doesn't add deny rules > > It'd really help if REVOKE consistently raised warnings when it didn't > actually revoke anything. > > Even better, a special case for REVOKEs on objects that only have owner > and public permissions could say: > > WARNING: REVOKE didn't remove any permissions for user <blah>. This > <table/db/whatever> > has default permissions, so there were no GRANTs for user <blah> to > revoke. See the documentation > for REVOKE for more information. > > > Opinions? > > > -- > Craig Ringer > >
On Tue, Aug 21, 2012 at 02:31:29PM +0800, Craig Ringer wrote: > It'd really help if REVOKE consistently raised warnings when it didn't > actually revoke anything. +1 This will invite the same mixed feelings as the CREATE x IF NOT EXISTS notices, but I think it's worthwhile. > Even better, a special case for REVOKEs on objects that only have owner > and public permissions could say: > > WARNING: REVOKE didn't remove any permissions for user <blah>. This > <table/db/whatever> > has default permissions, so there were no GRANTs for user <blah> to > revoke. See the documentation > for REVOKE for more information. The extra aid from saying those particular things is not clear to me. It might be overkill, but we could report any other roles indirectly conveying access to the named role.
> -----Original Message----- > From: pgsql-hackers-owner@postgresql.org [mailto:pgsql-hackers- > owner@postgresql.org] On Behalf Of Noah Misch > Sent: Tuesday, October 02, 2012 3:02 PM > To: Craig Ringer > Cc: PostgreSQL Hackers > Subject: Re: [HACKERS] Raise a WARNING if a REVOKE affects nothing? > > On Tue, Aug 21, 2012 at 02:31:29PM +0800, Craig Ringer wrote: > > It'd really help if REVOKE consistently raised warnings when it didn't > > actually revoke anything. > > +1 > > This will invite the same mixed feelings as the CREATE x IF NOT EXISTS > notices, but I think it's worthwhile. > > > Even better, a special case for REVOKEs on objects that only have > > owner and public permissions could say: > > > > WARNING: REVOKE didn't remove any permissions for user <blah>. This > > <table/db/whatever> has default permissions, so there were no GRANTs > > for user <blah> to revoke. See the documentation for REVOKE for more > > information. > > The extra aid from saying those particular things is not clear to me. > > It might be overkill, but we could report any other roles indirectly conveying > access to the named role. > Having been bitten by this myself I do see the value in such a warning. It is not uncommon for someone using REVOKE to believe they are installing a block instead of removing an allowance; especially as it interacts with default permissions. That said, and this is an off-the-cuff thought, the entire UI for permissions, and its treatment in the documentation, seems to be fact oriented. The system is well documented but actually getting up to speed to learn and use it is still a matter of reading the documentation and figuring out how everything fits together. I haven't given it that much thought but I am curious if others are of the same opinion. IOW, this proposal is an attempt to fix a symptom without addressing the root cause. Food for thought. David J.
On Tue, Oct 2, 2012 at 3:01 PM, Noah Misch <noah@leadboat.com> wrote: > On Tue, Aug 21, 2012 at 02:31:29PM +0800, Craig Ringer wrote: >> It'd really help if REVOKE consistently raised warnings when it didn't >> actually revoke anything. > > +1 > > This will invite the same mixed feelings as the CREATE x IF NOT EXISTS > notices, but I think it's worthwhile. Just to ask a possibly stupid question: why is attempting to a REVOKE a non-existent privilege anything other than an ERROR? We would throw an ERROR if you tried to insert into a nonexistent table, or if you tried to drop a nonexistent table, or if you tried to call a nonexistent function, so why not also here? We could have REVOKE IF EXISTS for the current behavior (and users could boost client_min_messages to suppress the notice when deisred). -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes: > Just to ask a possibly stupid question: why is attempting to a REVOKE > a non-existent privilege anything other than an ERROR? Because the SQL standard says so? regards, tom lane