[PATCH] allow has_table_privilege(..., 'usage') on sequences

Started by Abhijit Menon-Senover 17 years ago9 messages
1 attachment(s)

I just noticed, to my dismay, that has_table_privilege() does not allow
me to check for usage privileges on sequences. I suspect this may have
been an oversight. If so, the attached patch fixes it for me.

-- ams

Attachments:

usage.difftext/x-diff; charset=us-asciiDownload
*** a/src/backend/utils/adt/acl.c
--- b/src/backend/utils/adt/acl.c
***************
*** 1558,1563 **** convert_table_priv_string(text *priv_type_text)
--- 1558,1568 ----
  	if (pg_strcasecmp(priv_type, "TRIGGER WITH GRANT OPTION") == 0)
  		return ACL_GRANT_OPTION_FOR(ACL_TRIGGER);
  
+ 	if (pg_strcasecmp(priv_type, "USAGE") == 0)
+ 		return ACL_USAGE;
+ 	if (pg_strcasecmp(priv_type, "USAGE WITH GRANT OPTION") == 0)
+ 		return ACL_GRANT_OPTION_FOR(ACL_USAGE);
+ 
  	if (pg_strcasecmp(priv_type, "RULE") == 0)
  		return 0;				/* ignore old RULE privileges */
  	if (pg_strcasecmp(priv_type, "RULE WITH GRANT OPTION") == 0)
#2Jaime Casanova
jcasanov@systemguards.com.ec
In reply to: Abhijit Menon-Sen (#1)
Re: [PATCH] allow has_table_privilege(..., 'usage') on sequences

On Thu, Aug 7, 2008 at 3:08 AM, Abhijit Menon-Sen <ams@oryx.com> wrote:

I just noticed, to my dismay, that has_table_privilege() does not allow
me to check for usage privileges on sequences.

Maybe we want a new function has_sequence_privilege() instead?

--
regards,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. (593) 87171157

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Jaime Casanova (#2)
Re: [PATCH] allow has_table_privilege(..., 'usage') on sequences

"Jaime Casanova" <jcasanov@systemguards.com.ec> writes:

On Thu, Aug 7, 2008 at 3:08 AM, Abhijit Menon-Sen <ams@oryx.com> wrote:

I just noticed, to my dismay, that has_table_privilege() does not allow
me to check for usage privileges on sequences.

Maybe we want a new function has_sequence_privilege() instead?

Yeah, that seems like the $64 question for this patch. The presented
patch is certainly simple (it lacks only documentation to be considered
committable). The question is do we want to fuzz things up to the
extent of pretending that USAGE is a table privilege. The GRANT code
certainly doesn't think so:

regression=# grant usage on table t1 to joe;
ERROR: invalid privilege type USAGE for table

and in fact aclchk.c devotes quite a few lines of code to making sure
that sequence and table privileges are kept appropriately distinct.

As of right now, the proposed patch looks like a nice easy solution to a
minor problem. But I'm concerned that we might be backing ourselves
into a corner by inserting this inconsistency --- some day it might
cause a real problem. It also seems that it would be throwing away
a lot of hard work that was already put into aclchk.c to maintain the
distinction.

So I'm thinking it would be better to invent a has_sequence_privilege
family of functions.

On the other hand, that would require a couple hundred lines of new code
and docs. Even though it'd be a pretty easy copy-and-paste task,
perhaps that's overkill for what I have to admit is a mostly
hypothetical worry about future inconsistency.

Thoughts?

regards, tom lane

#4Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#3)
Re: [PATCH] allow has_table_privilege(..., 'usage') on sequences

Maybe we want a new function has_sequence_privilege() instead?

+1

In reply to: Tom Lane (#3)
Re: [PATCH] allow has_table_privilege(..., 'usage') on sequences

At 2008-09-06 19:59:55 -0400, tgl@sss.pgh.pa.us wrote:

So I'm thinking it would be better to invent a has_sequence_privilege
family of functions.

Perhaps.

I certainly wouldn't object to that approach. If there had been such a
function, I would have used it; and, since has_table_privilege doesn't
help me in any released version, I have nothing invested in that way
of doing things.

(I can't help but think that the USAGE privilege is a bit unfortunate.
If granting SELECT rights allowed currval(), INSERT allowed nextval(),
and UPDATE allowed nextval() and setval(), then has_table_privilege()
would have been sufficient and there would be no need to invent a new
set of functions just to check USAGE.

At the moment, however, I have to grant UPDATE instead of USAGE, both
for compatibility with 8.1, and because there is no easy way to check
if USAGE has already been granted, even though I don't want to allow
setval() at all. Pity.)

-- ams

PS. I'm sorry I haven't been able to review any patches this time. I
meant to, but a sequence of unfortunate events conspired to keep me
busy elsewhere. I look forward to participating again next time.

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Abhijit Menon-Sen (#5)
Re: [PATCH] allow has_table_privilege(..., 'usage') on sequences

Abhijit Menon-Sen <ams@oryx.com> writes:

(I can't help but think that the USAGE privilege is a bit unfortunate.
If granting SELECT rights allowed currval(), INSERT allowed nextval(),
and UPDATE allowed nextval() and setval(), then has_table_privilege()
would have been sufficient and there would be no need to invent a new
set of functions just to check USAGE.

That train left the station already, and anyway you are failing to
consider "SELECT * FROM sequence", which definitely needs to have
different privileges from nextval()/currval().

regards, tom lane

#7Jaime Casanova
jcasanov@systemguards.com.ec
In reply to: Tom Lane (#6)
Re: [PATCH] allow has_table_privilege(..., 'usage') on sequences

On Sun, Sep 7, 2008 at 10:55 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Abhijit Menon-Sen <ams@oryx.com> writes:

(I can't help but think that the USAGE privilege is a bit unfortunate.
If granting SELECT rights allowed currval(), INSERT allowed nextval(),
and UPDATE allowed nextval() and setval(), then has_table_privilege()
would have been sufficient and there would be no need to invent a new
set of functions just to check USAGE.

That train left the station already, and anyway you are failing to
consider "SELECT * FROM sequence", which definitely needs to have
different privileges from nextval()/currval().

can we tell there is consensus in create a new has_sequence_privilege()?
Abhijit will you make it? if not i can make a try...

--
regards,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157

In reply to: Jaime Casanova (#7)
Re: [PATCH] allow has_table_privilege(..., 'usage') on sequences

At 2008-09-22 12:54:34 -0500, jcasanov@systemguards.com.ec wrote:

can we tell there is consensus in create a new has_sequence_privilege()?
Abhijit will you make it? if not i can make a try...

Yes, I'll do it.

-- ams

#9Bruce Momjian
bruce@momjian.us
In reply to: Abhijit Menon-Sen (#8)
Re: [PATCH] allow has_table_privilege(..., 'usage') on sequences

Abhijit Menon-Sen wrote:

At 2008-09-22 12:54:34 -0500, jcasanov@systemguards.com.ec wrote:

can we tell there is consensus in create a new has_sequence_privilege()?
Abhijit will you make it? if not i can make a try...

Yes, I'll do it.

This hasn't been done so I added it to the TODO list.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +