[Patch Review] TRUNCATE Permission
Hello all,
Robert Haas submitted the TRUNCATE permissions patch for the September
commit fest:
http://archives.postgresql.org/message-id/603c8f070807261444s133fb281sf34d069ab5b4c0b@mail.gmail.com
I had some extra time tonight, so I downloaded, installed and reviewed this
patch.
Here are my findings:
1. I found the patch style to be consistent with the surrounding code.
2. The patch provides documentation updates and regression test updates.
3. The patch applies (with some fuzz) to the latest GIT tree.
Three issues I found with the patch via code reading and verified via
testing:
1. File: src/backend/catalog/aclchk.c:
Function: pg_class_aclmask():
I believe the ACL_TRUNCATE trigger should be added to this check and mask.
/*
* Deny anyone permission to update a system catalog unless
* pg_authid.rolcatupdate is set. (This is to let superusers
protect
* themselves from themselves.) Also allow it if
allowSystemTableMods.
*
* As of 7.4 we have some updatable system views; those shouldn't be
* protected in this way. Assume the view rules can take care of
* themselves. ACL_USAGE is if we ever have system sequences.
*/
if ((mask & (ACL_INSERT | ACL_UPDATE | ACL_DELETE | ACL_USAGE)) &&
IsSystemClass(classForm) &&
classForm->relkind != RELKIND_VIEW &&
!has_rolcatupdate(roleid) &&
!allowSystemTableMods)
{
#ifdef ACLDEBUG
elog(DEBUG2, "permission denied for system catalog update");
#endif
mask &= ~(ACL_INSERT | ACL_UPDATE | ACL_DELETE | ACL_USAGE);
}
Here is where it is visible via the psql interface:
template1=# select rolname, rolcatupdate from pg_authid;
rolname | rolcatupdate
---------+--------------
rbrad | t
(1 row)
template1=# select has_table_privilege('pg_class', 'delete');
has_table_privilege
---------------------
t
(1 row)
template1=# select has_table_privilege('pg_class', 'truncate');
has_table_privilege
---------------------
t
(1 row)
template1=# update pg_authid set rolcatupdate = false;
UPDATE 1
template1=# select has_table_privilege('pg_class', 'delete');
has_table_privilege
---------------------
f
(1 row)
template1=# select has_table_privilege('pg_class', 'truncate');
has_table_privilege
---------------------
t
(1 row)
I do not believe this is a huge issue since truncate is prohibited on the
system catalogs
by the truncate_check_rel().
template1=# truncate pg_authid;
ERROR: permission denied: "pg_authid" is a system catalog
2. The src/test/regress/sql/privileges.sql regression test has tests for
the has_table_privilege() function. It looks like all the other
permissions
are tested in this function, but there is not a test for the TRUNCATE
privilege.
3. I believe you missed a documentation update in the ddl.sgml file:
There are several different privileges: <literal>SELECT</>,
<literal>INSERT</>, <literal>UPDATE</>, <literal>DELETE</>,
<literal>REFERENCES</>, <literal>TRIGGER</>,
<literal>CREATE</>, <literal>CONNECT</>, <literal>TEMPORARY</>,
<literal>EXECUTE</>, and <literal>USAGE</>.
I played around with the patch for an hour or so tonight and I did not
observer any other unusual behaviors.
Hopefully this review is useful. It is my first patch review for a
commit-fest.
I will update the wiki with a link to this email message for my review.
Thanks,
- Ryan
Hello all,
On Sun, Aug 31, 2008 at 11:55 PM, Ryan Bradetich <rbradetich@gmail.com>wrote:
I do not believe this is a huge issue since truncate is prohibited on the
system catalogs
by the truncate_check_rel().template1=# truncate pg_authid;
ERROR: permission denied: "pg_authid" is a system catalog
I thought about this some more. I believe my suggestion was incorrect.
Since truncate_check_rel() prevents the use of the truncate command on
system catalogs, the TRUNCATE permission should always be stripped
from the system catalogs.
Here is the inconsistency I observed:
template1=# \z pg_catalog.pg_authid
Access privileges
Schema | Name | Type | Access privileges
------------+-----------+-------+---------------------
pg_catalog | pg_authid | table | rbrad=arwdDxt/rbrad
(1 row)
template1=# select rolname, rolcatupdate from pg_authid;
rolname | rolcatupdate
---------+--------------
rbrad | t
(1 row)
template1=# select has_table_privilege('pg_authid', 'truncate');
has_table_privilege
---------------------
t
(1 row)
template1=# truncate pg_authid;
ERROR: permission denied: "pg_authid" is a system catalog
The TRUNCATE fails even though \z and has_table_privilege() said I had
permissions.
Compare with the DELETE privilege:
template1=# select has_table_privilege('pg_authid', 'delete');
has_table_privilege
---------------------
t
(1 row)
template1=# delete from pg_authid;
DELETE 1
Thanks,
- Ryan
"Ryan Bradetich" <rbradetich@gmail.com> writes:
I do not believe this is a huge issue since truncate is prohibited on the
system catalogs
by the truncate_check_rel().
Only when allowSystemTableMods is false. I think it would be a serious
mistake for your patch to treat the system catalogs differently from
other tables.
Here is the inconsistency I observed:
It seems to me that you are assuming that lack of a TRUNCATE permission
bit is the only valid reason for a "permission denied" failure. This is
fairly obviously not so, since multiple permissions typically enter into
any command (consider schema-level permissions for instance).
regards, tom lane
I had intended to send this message to the pgsql-hackers mailing list as
well.
Thanks,
- Ryan
---------- Forwarded message ----------
From: Ryan Bradetich <rbradetich@gmail.com>
Date: Mon, Sep 1, 2008 at 2:20 PM
Subject: Re: [HACKERS] [Patch Review] TRUNCATE Permission
To: Tom Lane <tgl@sss.pgh.pa.us>
Hello Tom,
On Mon, Sep 1, 2008 at 1:00 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
"Ryan Bradetich" <rbradetich@gmail.com> writes:
I do not believe this is a huge issue since truncate is prohibited on
the
system catalogs
by the truncate_check_rel().Only when allowSystemTableMods is false. I think it would be a serious
mistake for your patch to treat the system catalogs differently from
other tables.
Good Point. Looks like I still have more code reading to do :)
This is Robert Haas's patch for the September 2008 commit-fest.
I am just offering my review. Gives me a good excuse to dig into
the PostgreSQL code base. Hopefully this review is useful to the
person committing the patch.
Here is the inconsistency I observed:
It seems to me that you are assuming that lack of a TRUNCATE permission
bit is the only valid reason for a "permission denied" failure. This is
fairly obviously not so, since multiple permissions typically enter into
any command (consider schema-level permissions for instance).
I was just comparing the behavior between DELETE and TRUNCATE.
My last suggestion for the TRUNCATE permission always being removed
on the system tables is obviously bogus because of the allowSystemTableMods
issue you raised.
Does my first suggestion still make sense for removing the TRUNCATE in
pg_class_aclmask() when pg_Authid.rolcatupdate is not set?
Thanks,
- Ryan
Import Notes
Reply to msg id not found: e739902b0809011420v54137353h973df3844d96a24e@mail.gmail.com
"Ryan Bradetich" <rbradetich@gmail.com> writes:
On Mon, Sep 1, 2008 at 1:00 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
[ something about "your patch" ]
This is Robert Haas's patch for the September 2008 commit-fest.
I am just offering my review.
Sorry about that, I got confused by the reply-to-a-reply.
Does my first suggestion still make sense for removing the TRUNCATE in
pg_class_aclmask() when pg_Authid.rolcatupdate is not set?
Probably. AFAICS it should be treated exactly like ACL_DELETE, so
anyplace that acl-whacking code is doing something for ACL_DELETE and
the patch doesn't add in ACL_TRUNCATE, I'd be suspicious ...
regards, tom lane
Updated patch attached, based on comments from Ryan Bradetich and Tom
Lane, and sync'd to latest CVS version.
...Robert
Show quoted text
On Mon, Sep 1, 2008 at 9:33 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
"Ryan Bradetich" <rbradetich@gmail.com> writes:
On Mon, Sep 1, 2008 at 1:00 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
[ something about "your patch" ]
This is Robert Haas's patch for the September 2008 commit-fest.
I am just offering my review.Sorry about that, I got confused by the reply-to-a-reply.
Does my first suggestion still make sense for removing the TRUNCATE in
pg_class_aclmask() when pg_Authid.rolcatupdate is not set?Probably. AFAICS it should be treated exactly like ACL_DELETE, so
anyplace that acl-whacking code is doing something for ACL_DELETE and
the patch doesn't add in ACL_TRUNCATE, I'd be suspicious ...regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Attachments:
truncate-v2.patchtext/x-diff; name=truncate-v2.patchDownload
Index: doc/src/sgml/ddl.sgml
===================================================================
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/ddl.sgml,v
retrieving revision 1.82
diff -c -r1.82 ddl.sgml
*** doc/src/sgml/ddl.sgml 9 May 2008 23:32:03 -0000 1.82
--- doc/src/sgml/ddl.sgml 6 Sep 2008 03:07:12 -0000
***************
*** 1356,1362 ****
<para>
There are several different privileges: <literal>SELECT</>,
<literal>INSERT</>, <literal>UPDATE</>, <literal>DELETE</>,
! <literal>REFERENCES</>, <literal>TRIGGER</>,
<literal>CREATE</>, <literal>CONNECT</>, <literal>TEMPORARY</>,
<literal>EXECUTE</>, and <literal>USAGE</>.
The privileges applicable to a particular
--- 1356,1362 ----
<para>
There are several different privileges: <literal>SELECT</>,
<literal>INSERT</>, <literal>UPDATE</>, <literal>DELETE</>,
! <literal>TRUNCATE</>, <literal>REFERENCES</>, <literal>TRIGGER</>,
<literal>CREATE</>, <literal>CONNECT</>, <literal>TEMPORARY</>,
<literal>EXECUTE</>, and <literal>USAGE</>.
The privileges applicable to a particular
Index: doc/src/sgml/user-manag.sgml
===================================================================
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/user-manag.sgml,v
retrieving revision 1.39
diff -c -r1.39 user-manag.sgml
*** doc/src/sgml/user-manag.sgml 1 Feb 2007 00:28:18 -0000 1.39
--- doc/src/sgml/user-manag.sgml 6 Sep 2008 03:07:13 -0000
***************
*** 293,299 ****
granted.
There are several different kinds of privilege: <literal>SELECT</>,
<literal>INSERT</>, <literal>UPDATE</>, <literal>DELETE</>,
! <literal>REFERENCES</>, <literal>TRIGGER</>,
<literal>CREATE</>, <literal>CONNECT</>, <literal>TEMPORARY</>,
<literal>EXECUTE</>, and <literal>USAGE</>.
For more information on the different types of privileges supported by
--- 293,299 ----
granted.
There are several different kinds of privilege: <literal>SELECT</>,
<literal>INSERT</>, <literal>UPDATE</>, <literal>DELETE</>,
! <literal>TRUNCATE</>, <literal>REFERENCES</>, <literal>TRIGGER</>,
<literal>CREATE</>, <literal>CONNECT</>, <literal>TEMPORARY</>,
<literal>EXECUTE</>, and <literal>USAGE</>.
For more information on the different types of privileges supported by
Index: doc/src/sgml/ref/grant.sgml
===================================================================
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/ref/grant.sgml,v
retrieving revision 1.70
diff -c -r1.70 grant.sgml
*** doc/src/sgml/ref/grant.sgml 3 Jul 2008 15:59:55 -0000 1.70
--- doc/src/sgml/ref/grant.sgml 6 Sep 2008 03:07:14 -0000
***************
*** 20,26 ****
<refsynopsisdiv>
<synopsis>
! GRANT { { SELECT | INSERT | UPDATE | DELETE | REFERENCES | TRIGGER }
[,...] | ALL [ PRIVILEGES ] }
ON [ TABLE ] <replaceable class="PARAMETER">tablename</replaceable> [, ...]
TO { [ GROUP ] <replaceable class="PARAMETER">rolename</replaceable> | PUBLIC } [, ...] [ WITH GRANT OPTION ]
--- 20,26 ----
<refsynopsisdiv>
<synopsis>
! GRANT { { SELECT | INSERT | UPDATE | DELETE | TRUNCATE | REFERENCES | TRIGGER }
[,...] | ALL [ PRIVILEGES ] }
ON [ TABLE ] <replaceable class="PARAMETER">tablename</replaceable> [, ...]
TO { [ GROUP ] <replaceable class="PARAMETER">rolename</replaceable> | PUBLIC } [, ...] [ WITH GRANT OPTION ]
***************
*** 193,198 ****
--- 193,208 ----
</varlistentry>
<varlistentry>
+ <term>TRUNCATE</term>
+ <listitem>
+ <para>
+ Allows <xref linkend="sql-truncate" endterm="sql-truncate-title"> on
+ the specified table.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term>REFERENCES</term>
<listitem>
<para>
***************
*** 421,428 ****
=> \z mytable
Access privileges
Schema | Name | Type | Access privileges
! --------+---------+-------+----------------------
! public | mytable | table | miriam=arwdxt/miriam
: =r/miriam
: admin=arw/miriam
(1 row)
--- 431,438 ----
=> \z mytable
Access privileges
Schema | Name | Type | Access privileges
! --------+---------+-------+-----------------------
! public | mytable | table | miriam=arwdDxt/miriam
: =r/miriam
: admin=arw/miriam
(1 row)
***************
*** 436,441 ****
--- 446,452 ----
w -- UPDATE ("write")
a -- INSERT ("append")
d -- DELETE
+ D -- TRUNCATE
x -- REFERENCES
t -- TRIGGER
X -- EXECUTE
***************
*** 443,449 ****
C -- CREATE
c -- CONNECT
T -- TEMPORARY
! arwdxt -- ALL PRIVILEGES (for tables)
* -- grant option for preceding privilege
/yyyy -- role that granted this privilege
--- 454,460 ----
C -- CREATE
c -- CONNECT
T -- TEMPORARY
! arwdDxt -- ALL PRIVILEGES (for tables)
* -- grant option for preceding privilege
/yyyy -- role that granted this privilege
***************
*** 466,472 ****
object type, as explained above. The first <command>GRANT</> or
<command>REVOKE</> on an object
will instantiate the default privileges (producing, for example,
! <literal>{miriam=arwdxt/miriam}</>) and then modify them per the
specified request.
</para>
--- 477,483 ----
object type, as explained above. The first <command>GRANT</> or
<command>REVOKE</> on an object
will instantiate the default privileges (producing, for example,
! <literal>{miriam=arwdDxt/miriam}</>) and then modify them per the
specified request.
</para>
Index: doc/src/sgml/ref/revoke.sgml
===================================================================
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/ref/revoke.sgml,v
retrieving revision 1.47
diff -c -r1.47 revoke.sgml
*** doc/src/sgml/ref/revoke.sgml 3 Mar 2008 19:17:27 -0000 1.47
--- doc/src/sgml/ref/revoke.sgml 6 Sep 2008 03:07:14 -0000
***************
*** 21,27 ****
<refsynopsisdiv>
<synopsis>
REVOKE [ GRANT OPTION FOR ]
! { { SELECT | INSERT | UPDATE | DELETE | REFERENCES | TRIGGER }
[,...] | ALL [ PRIVILEGES ] }
ON [ TABLE ] <replaceable class="PARAMETER">tablename</replaceable> [, ...]
FROM { [ GROUP ] <replaceable class="PARAMETER">rolename</replaceable> | PUBLIC } [, ...]
--- 21,27 ----
<refsynopsisdiv>
<synopsis>
REVOKE [ GRANT OPTION FOR ]
! { { SELECT | INSERT | UPDATE | DELETE | TRUNCATE | REFERENCES | TRIGGER }
[,...] | ALL [ PRIVILEGES ] }
ON [ TABLE ] <replaceable class="PARAMETER">tablename</replaceable> [, ...]
FROM { [ GROUP ] <replaceable class="PARAMETER">rolename</replaceable> | PUBLIC } [, ...]
Index: doc/src/sgml/ref/truncate.sgml
===================================================================
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/ref/truncate.sgml,v
retrieving revision 1.27
diff -c -r1.27 truncate.sgml
*** doc/src/sgml/ref/truncate.sgml 17 May 2008 23:36:27 -0000 1.27
--- doc/src/sgml/ref/truncate.sgml 6 Sep 2008 03:07:14 -0000
***************
*** 97,103 ****
<title>Notes</title>
<para>
! Only the owner of a table can <command>TRUNCATE</> it.
</para>
<para>
--- 97,104 ----
<title>Notes</title>
<para>
! You must have the <literal>TRUNCATE</literal> privilege on the table
! to truncate it.
</para>
<para>
Index: src/backend/catalog/aclchk.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/catalog/aclchk.c,v
retrieving revision 1.147
diff -c -r1.147 aclchk.c
*** src/backend/catalog/aclchk.c 19 Jun 2008 00:46:03 -0000 1.147
--- src/backend/catalog/aclchk.c 6 Sep 2008 03:07:16 -0000
***************
*** 1331,1336 ****
--- 1331,1338 ----
return ACL_UPDATE;
if (strcmp(privname, "delete") == 0)
return ACL_DELETE;
+ if (strcmp(privname, "truncate") == 0)
+ return ACL_TRUNCATE;
if (strcmp(privname, "references") == 0)
return ACL_REFERENCES;
if (strcmp(privname, "trigger") == 0)
***************
*** 1368,1373 ****
--- 1370,1377 ----
return "UPDATE";
case ACL_DELETE:
return "DELETE";
+ case ACL_TRUNCATE:
+ return "TRUNCATE";
case ACL_REFERENCES:
return "REFERENCES";
case ACL_TRIGGER:
***************
*** 1582,1588 ****
* protected in this way. Assume the view rules can take care of
* themselves. ACL_USAGE is if we ever have system sequences.
*/
! if ((mask & (ACL_INSERT | ACL_UPDATE | ACL_DELETE | ACL_USAGE)) &&
IsSystemClass(classForm) &&
classForm->relkind != RELKIND_VIEW &&
!has_rolcatupdate(roleid) &&
--- 1586,1593 ----
* protected in this way. Assume the view rules can take care of
* themselves. ACL_USAGE is if we ever have system sequences.
*/
! if ((mask & (ACL_INSERT | ACL_UPDATE | ACL_DELETE | ACL_USAGE
! | ACL_TRUNCATE)) &&
IsSystemClass(classForm) &&
classForm->relkind != RELKIND_VIEW &&
!has_rolcatupdate(roleid) &&
***************
*** 1591,1597 ****
#ifdef ACLDEBUG
elog(DEBUG2, "permission denied for system catalog update");
#endif
! mask &= ~(ACL_INSERT | ACL_UPDATE | ACL_DELETE | ACL_USAGE);
}
/*
--- 1596,1603 ----
#ifdef ACLDEBUG
elog(DEBUG2, "permission denied for system catalog update");
#endif
! mask &= ~(ACL_INSERT | ACL_UPDATE | ACL_DELETE | ACL_USAGE |
! ACL_TRUNCATE);
}
/*
Index: src/backend/commands/tablecmds.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/commands/tablecmds.c,v
retrieving revision 1.265
diff -c -r1.265 tablecmds.c
*** src/backend/commands/tablecmds.c 1 Sep 2008 20:42:44 -0000 1.265
--- src/backend/commands/tablecmds.c 6 Sep 2008 03:07:21 -0000
***************
*** 989,994 ****
--- 989,996 ----
static void
truncate_check_rel(Relation rel)
{
+ AclResult aclresult;
+
/* Only allow truncate on regular tables */
if (rel->rd_rel->relkind != RELKIND_RELATION)
ereport(ERROR,
***************
*** 997,1004 ****
RelationGetRelationName(rel))));
/* Permissions checks */
! if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
! aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
RelationGetRelationName(rel));
if (!allowSystemTableMods && IsSystemRelation(rel))
--- 999,1008 ----
RelationGetRelationName(rel))));
/* Permissions checks */
! aclresult = pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),
! ACL_TRUNCATE);
! if (aclresult != ACLCHECK_OK)
! aclcheck_error(aclresult, ACL_KIND_CLASS,
RelationGetRelationName(rel));
if (!allowSystemTableMods && IsSystemRelation(rel))
Index: src/backend/utils/adt/acl.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/adt/acl.c,v
retrieving revision 1.140
diff -c -r1.140 acl.c
*** src/backend/utils/adt/acl.c 25 Mar 2008 22:42:43 -0000 1.140
--- src/backend/utils/adt/acl.c 6 Sep 2008 03:07:23 -0000
***************
*** 265,270 ****
--- 265,273 ----
case ACL_DELETE_CHR:
read = ACL_DELETE;
break;
+ case ACL_TRUNCATE_CHR:
+ read = ACL_TRUNCATE;
+ break;
case ACL_REFERENCES_CHR:
read = ACL_REFERENCES;
break;
***************
*** 1323,1328 ****
--- 1326,1333 ----
return ACL_UPDATE;
if (pg_strcasecmp(priv_type, "DELETE") == 0)
return ACL_DELETE;
+ if (pg_strcasecmp(priv_type, "TRUNCATE") == 0)
+ return ACL_TRUNCATE;
if (pg_strcasecmp(priv_type, "REFERENCES") == 0)
return ACL_REFERENCES;
if (pg_strcasecmp(priv_type, "TRIGGER") == 0)
***************
*** 1548,1553 ****
--- 1553,1563 ----
if (pg_strcasecmp(priv_type, "DELETE WITH GRANT OPTION") == 0)
return ACL_GRANT_OPTION_FOR(ACL_DELETE);
+ if (pg_strcasecmp(priv_type, "TRUNCATE") == 0)
+ return ACL_TRUNCATE;
+ if (pg_strcasecmp(priv_type, "TRUNCATE WITH GRANT OPTION") == 0)
+ return ACL_GRANT_OPTION_FOR(ACL_TRUNCATE);
+
if (pg_strcasecmp(priv_type, "REFERENCES") == 0)
return ACL_REFERENCES;
if (pg_strcasecmp(priv_type, "REFERENCES WITH GRANT OPTION") == 0)
Index: src/bin/pg_dump/dumputils.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/bin/pg_dump/dumputils.c,v
retrieving revision 1.40
diff -c -r1.40 dumputils.c
*** src/bin/pg_dump/dumputils.c 1 Jan 2008 19:45:55 -0000 1.40
--- src/bin/pg_dump/dumputils.c 6 Sep 2008 03:07:24 -0000
***************
*** 656,661 ****
--- 656,663 ----
if (remoteVersion >= 70200)
{
CONVERT_PRIV('d', "DELETE");
+ if (remoteVersion >= 80400)
+ CONVERT_PRIV('D', "TRUNCATE");
CONVERT_PRIV('x', "REFERENCES");
CONVERT_PRIV('t', "TRIGGER");
}
Index: src/bin/psql/tab-complete.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/bin/psql/tab-complete.c,v
retrieving revision 1.171
diff -c -r1.171 tab-complete.c
*** src/bin/psql/tab-complete.c 16 Aug 2008 01:36:35 -0000 1.171
--- src/bin/psql/tab-complete.c 6 Sep 2008 03:07:26 -0000
***************
*** 1610,1617 ****
pg_strcasecmp(prev_wd, "REVOKE") == 0)
{
static const char *const list_privileg[] =
! {"SELECT", "INSERT", "UPDATE", "DELETE", "RULE", "REFERENCES",
! "TRIGGER", "CREATE", "CONNECT", "TEMPORARY", "EXECUTE", "USAGE",
"ALL", NULL};
COMPLETE_WITH_LIST(list_privileg);
--- 1610,1618 ----
pg_strcasecmp(prev_wd, "REVOKE") == 0)
{
static const char *const list_privileg[] =
! {"SELECT", "INSERT", "UPDATE", "DELETE", "TRUNCATE", "RULE",
! "REFERENCES", "TRIGGER", "CREATE", "CONNECT", "TEMPORARY",
! "EXECUTE", "USAGE",
"ALL", NULL};
COMPLETE_WITH_LIST(list_privileg);
Index: src/include/nodes/parsenodes.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/nodes/parsenodes.h,v
retrieving revision 1.374
diff -c -r1.374 parsenodes.h
*** src/include/nodes/parsenodes.h 1 Sep 2008 20:42:45 -0000 1.374
--- src/include/nodes/parsenodes.h 6 Sep 2008 03:07:28 -0000
***************
*** 63,69 ****
#define ACL_SELECT (1<<1)
#define ACL_UPDATE (1<<2)
#define ACL_DELETE (1<<3)
! /* #define ACL_RULE (1<<4) unused, available */
#define ACL_REFERENCES (1<<5)
#define ACL_TRIGGER (1<<6)
#define ACL_EXECUTE (1<<7) /* for functions */
--- 63,69 ----
#define ACL_SELECT (1<<1)
#define ACL_UPDATE (1<<2)
#define ACL_DELETE (1<<3)
! #define ACL_TRUNCATE (1<<4)
#define ACL_REFERENCES (1<<5)
#define ACL_TRIGGER (1<<6)
#define ACL_EXECUTE (1<<7) /* for functions */
Index: src/include/utils/acl.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/utils/acl.h,v
retrieving revision 1.103
diff -c -r1.103 acl.h
*** src/include/utils/acl.h 1 Jan 2008 19:45:59 -0000 1.103
--- src/include/utils/acl.h 6 Sep 2008 03:07:28 -0000
***************
*** 128,133 ****
--- 128,134 ----
#define ACL_SELECT_CHR 'r' /* formerly known as "read" */
#define ACL_UPDATE_CHR 'w' /* formerly known as "write" */
#define ACL_DELETE_CHR 'd'
+ #define ACL_TRUNCATE_CHR 'D' /* super-delete, as it were */
#define ACL_REFERENCES_CHR 'x'
#define ACL_TRIGGER_CHR 't'
#define ACL_EXECUTE_CHR 'X'
***************
*** 137,148 ****
#define ACL_CONNECT_CHR 'c'
/* string holding all privilege code chars, in order by bitmask position */
! #define ACL_ALL_RIGHTS_STR "arwdRxtXUCTc"
/*
* Bitmasks defining "all rights" for each supported object type
*/
! #define ACL_ALL_RIGHTS_RELATION (ACL_INSERT|ACL_SELECT|ACL_UPDATE|ACL_DELETE|ACL_REFERENCES|ACL_TRIGGER)
#define ACL_ALL_RIGHTS_SEQUENCE (ACL_USAGE|ACL_SELECT|ACL_UPDATE)
#define ACL_ALL_RIGHTS_DATABASE (ACL_CREATE|ACL_CREATE_TEMP|ACL_CONNECT)
#define ACL_ALL_RIGHTS_FUNCTION (ACL_EXECUTE)
--- 138,149 ----
#define ACL_CONNECT_CHR 'c'
/* string holding all privilege code chars, in order by bitmask position */
! #define ACL_ALL_RIGHTS_STR "arwdDxtXUCTc"
/*
* Bitmasks defining "all rights" for each supported object type
*/
! #define ACL_ALL_RIGHTS_RELATION (ACL_INSERT|ACL_SELECT|ACL_UPDATE|ACL_DELETE|ACL_TRUNCATE|ACL_REFERENCES|ACL_TRIGGER)
#define ACL_ALL_RIGHTS_SEQUENCE (ACL_USAGE|ACL_SELECT|ACL_UPDATE)
#define ACL_ALL_RIGHTS_DATABASE (ACL_CREATE|ACL_CREATE_TEMP|ACL_CONNECT)
#define ACL_ALL_RIGHTS_FUNCTION (ACL_EXECUTE)
Index: src/test/regress/expected/dependency.out
===================================================================
RCS file: /projects/cvsroot/pgsql/src/test/regress/expected/dependency.out,v
retrieving revision 1.7
diff -c -r1.7 dependency.out
*** src/test/regress/expected/dependency.out 3 Jul 2008 15:59:55 -0000 1.7
--- src/test/regress/expected/dependency.out 6 Sep 2008 03:07:30 -0000
***************
*** 21,27 ****
REVOKE SELECT ON deptest FROM GROUP regression_group;
DROP GROUP regression_group;
-- can't drop the user if we revoke the privileges partially
! REVOKE SELECT, INSERT, UPDATE, DELETE, RULE, REFERENCES ON deptest FROM regression_user;
DROP USER regression_user;
ERROR: role "regression_user" cannot be dropped because some objects depend on it
DETAIL: access to table deptest
--- 21,27 ----
REVOKE SELECT ON deptest FROM GROUP regression_group;
DROP GROUP regression_group;
-- can't drop the user if we revoke the privileges partially
! REVOKE SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON deptest FROM regression_user;
DROP USER regression_user;
ERROR: role "regression_user" cannot be dropped because some objects depend on it
DETAIL: access to table deptest
***************
*** 70,79 ****
\z deptest1
Access privileges
Schema | Name | Type | Access privileges
! --------+----------+-------+------------------------------------------------
! public | deptest1 | table | regression_user0=arwdxt/regression_user0
! : regression_user1=a*r*w*d*x*t*/regression_user0
! : regression_user2=arwdxt/regression_user1
(1 row)
DROP OWNED BY regression_user1;
--- 70,79 ----
\z deptest1
Access privileges
Schema | Name | Type | Access privileges
! --------+----------+-------+--------------------------------------------------
! public | deptest1 | table | regression_user0=arwdDxt/regression_user0
! : regression_user1=a*r*w*d*D*x*t*/regression_user0
! : regression_user2=arwdDxt/regression_user1
(1 row)
DROP OWNED BY regression_user1;
***************
*** 81,88 ****
\z deptest1
Access privileges
Schema | Name | Type | Access privileges
! --------+----------+-------+------------------------------------------
! public | deptest1 | table | regression_user0=arwdxt/regression_user0
(1 row)
-- table was dropped
--- 81,88 ----
\z deptest1
Access privileges
Schema | Name | Type | Access privileges
! --------+----------+-------+-------------------------------------------
! public | deptest1 | table | regression_user0=arwdDxt/regression_user0
(1 row)
-- table was dropped
Index: src/test/regress/expected/privileges.out
===================================================================
RCS file: /projects/cvsroot/pgsql/src/test/regress/expected/privileges.out,v
retrieving revision 1.38
diff -c -r1.38 privileges.out
*** src/test/regress/expected/privileges.out 3 Jul 2008 16:01:10 -0000 1.38
--- src/test/regress/expected/privileges.out 6 Sep 2008 03:07:30 -0000
***************
*** 16,23 ****
CREATE USER regressuser2;
CREATE USER regressuser3;
CREATE USER regressuser4;
! CREATE USER regressuser4; -- duplicate
! ERROR: role "regressuser4" already exists
CREATE GROUP regressgroup1;
CREATE GROUP regressgroup2 WITH USER regressuser1, regressuser2;
ALTER GROUP regressgroup1 ADD USER regressuser4;
--- 16,24 ----
CREATE USER regressuser2;
CREATE USER regressuser3;
CREATE USER regressuser4;
! CREATE USER regressuser5;
! CREATE USER regressuser5; -- duplicate
! ERROR: role "regressuser5" already exists
CREATE GROUP regressgroup1;
CREATE GROUP regressgroup2 WITH USER regressuser1, regressuser2;
ALTER GROUP regressgroup1 ADD USER regressuser4;
***************
*** 42,47 ****
--- 43,49 ----
INSERT INTO atest1 VALUES (1, 'one');
DELETE FROM atest1;
UPDATE atest1 SET a = 1 WHERE b = 'blech';
+ TRUNCATE atest1;
LOCK atest1 IN ACCESS EXCLUSIVE MODE;
REVOKE ALL ON atest1 FROM PUBLIC;
SELECT * FROM atest1;
***************
*** 60,65 ****
--- 62,68 ----
GRANT SELECT ON atest2 TO regressuser2;
GRANT UPDATE ON atest2 TO regressuser3;
GRANT INSERT ON atest2 TO regressuser4;
+ GRANT TRUNCATE ON atest2 TO regressuser5;
SET SESSION AUTHORIZATION regressuser2;
SELECT session_user, current_user;
session_user | current_user
***************
*** 96,101 ****
--- 99,106 ----
ERROR: permission denied for relation atest2
DELETE FROM atest2; -- fail
ERROR: permission denied for relation atest2
+ TRUNCATE atest2; -- fail
+ ERROR: permission denied for relation atest2
LOCK atest2 IN ACCESS EXCLUSIVE MODE; -- fail
ERROR: permission denied for relation atest2
COPY atest2 FROM stdin; -- fail
***************
*** 147,152 ****
--- 152,159 ----
ERROR: permission denied for relation atest2
DELETE FROM atest2; -- fail
ERROR: permission denied for relation atest2
+ TRUNCATE atest2; -- fail
+ ERROR: permission denied for relation atest2
LOCK atest2 IN ACCESS EXCLUSIVE MODE; -- ok
COPY atest2 FROM stdin; -- fail
ERROR: permission denied for relation atest2
***************
*** 285,290 ****
--- 292,302 ----
DROP FUNCTION testfunc1(int); -- ok
-- restore to sanity
GRANT ALL PRIVILEGES ON LANGUAGE sql TO PUBLIC;
+ -- truncate
+ SET SESSION AUTHORIZATION regressuser5;
+ TRUNCATE atest2; -- ok
+ TRUNCATE atest3; -- fail
+ ERROR: permission denied for relation atest3
-- has_table_privilege function
-- bad-input checks
select has_table_privilege(NULL,'pg_authid','select');
***************
*** 375,380 ****
--- 387,398 ----
t
(1 row)
+ select has_table_privilege('pg_authid','truncate');
+ has_table_privilege
+ ---------------------
+ t
+ (1 row)
+
select has_table_privilege(t1.oid,'select')
from (select oid from pg_class where relname = 'pg_authid') as t1;
has_table_privilege
***************
*** 452,457 ****
--- 470,481 ----
f
(1 row)
+ select has_table_privilege('pg_class','truncate');
+ has_table_privilege
+ ---------------------
+ f
+ (1 row)
+
select has_table_privilege(t1.oid,'select')
from (select oid from pg_class where relname = 'pg_class') as t1;
has_table_privilege
***************
*** 527,532 ****
--- 551,562 ----
f
(1 row)
+ select has_table_privilege('atest1','truncate');
+ has_table_privilege
+ ---------------------
+ f
+ (1 row)
+
select has_table_privilege(t1.oid,'select')
from (select oid from pg_class where relname = 'atest1') as t1;
has_table_privilege
Index: src/test/regress/sql/dependency.sql
===================================================================
RCS file: /projects/cvsroot/pgsql/src/test/regress/sql/dependency.sql,v
retrieving revision 1.4
diff -c -r1.4 dependency.sql
*** src/test/regress/sql/dependency.sql 21 Aug 2006 00:57:26 -0000 1.4
--- src/test/regress/sql/dependency.sql 6 Sep 2008 03:07:30 -0000
***************
*** 21,27 ****
DROP GROUP regression_group;
-- can't drop the user if we revoke the privileges partially
! REVOKE SELECT, INSERT, UPDATE, DELETE, RULE, REFERENCES ON deptest FROM regression_user;
DROP USER regression_user;
-- now we are OK to drop him
--- 21,27 ----
DROP GROUP regression_group;
-- can't drop the user if we revoke the privileges partially
! REVOKE SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON deptest FROM regression_user;
DROP USER regression_user;
-- now we are OK to drop him
Index: src/test/regress/sql/privileges.sql
===================================================================
RCS file: /projects/cvsroot/pgsql/src/test/regress/sql/privileges.sql,v
retrieving revision 1.20
diff -c -r1.20 privileges.sql
*** src/test/regress/sql/privileges.sql 3 Jul 2008 16:01:10 -0000 1.20
--- src/test/regress/sql/privileges.sql 6 Sep 2008 03:07:30 -0000
***************
*** 23,29 ****
CREATE USER regressuser2;
CREATE USER regressuser3;
CREATE USER regressuser4;
! CREATE USER regressuser4; -- duplicate
CREATE GROUP regressgroup1;
CREATE GROUP regressgroup2 WITH USER regressuser1, regressuser2;
--- 23,30 ----
CREATE USER regressuser2;
CREATE USER regressuser3;
CREATE USER regressuser4;
! CREATE USER regressuser5;
! CREATE USER regressuser5; -- duplicate
CREATE GROUP regressgroup1;
CREATE GROUP regressgroup2 WITH USER regressuser1, regressuser2;
***************
*** 45,50 ****
--- 46,52 ----
INSERT INTO atest1 VALUES (1, 'one');
DELETE FROM atest1;
UPDATE atest1 SET a = 1 WHERE b = 'blech';
+ TRUNCATE atest1;
LOCK atest1 IN ACCESS EXCLUSIVE MODE;
REVOKE ALL ON atest1 FROM PUBLIC;
***************
*** 58,63 ****
--- 60,66 ----
GRANT SELECT ON atest2 TO regressuser2;
GRANT UPDATE ON atest2 TO regressuser3;
GRANT INSERT ON atest2 TO regressuser4;
+ GRANT TRUNCATE ON atest2 TO regressuser5;
SET SESSION AUTHORIZATION regressuser2;
***************
*** 75,80 ****
--- 78,84 ----
SELECT * FROM atest1 FOR UPDATE; -- ok
SELECT * FROM atest2 FOR UPDATE; -- fail
DELETE FROM atest2; -- fail
+ TRUNCATE atest2; -- fail
LOCK atest2 IN ACCESS EXCLUSIVE MODE; -- fail
COPY atest2 FROM stdin; -- fail
GRANT ALL ON atest1 TO PUBLIC; -- fail
***************
*** 99,104 ****
--- 103,109 ----
SELECT * FROM atest1 FOR UPDATE; -- fail
SELECT * FROM atest2 FOR UPDATE; -- fail
DELETE FROM atest2; -- fail
+ TRUNCATE atest2; -- fail
LOCK atest2 IN ACCESS EXCLUSIVE MODE; -- ok
COPY atest2 FROM stdin; -- fail
***************
*** 205,210 ****
--- 210,219 ----
-- restore to sanity
GRANT ALL PRIVILEGES ON LANGUAGE sql TO PUBLIC;
+ -- truncate
+ SET SESSION AUTHORIZATION regressuser5;
+ TRUNCATE atest2; -- ok
+ TRUNCATE atest3; -- fail
-- has_table_privilege function
***************
*** 243,248 ****
--- 252,258 ----
select has_table_privilege('pg_authid','update');
select has_table_privilege('pg_authid','delete');
+ select has_table_privilege('pg_authid','truncate');
select has_table_privilege(t1.oid,'select')
from (select oid from pg_class where relname = 'pg_authid') as t1;
***************
*** 272,277 ****
--- 282,288 ----
select has_table_privilege('pg_class','update');
select has_table_privilege('pg_class','delete');
+ select has_table_privilege('pg_class','truncate');
select has_table_privilege(t1.oid,'select')
from (select oid from pg_class where relname = 'pg_class') as t1;
***************
*** 298,303 ****
--- 309,315 ----
select has_table_privilege('atest1','update');
select has_table_privilege('atest1','delete');
+ select has_table_privilege('atest1','truncate');
select has_table_privilege(t1.oid,'select')
from (select oid from pg_class where relname = 'atest1') as t1;
Hello Robert,
On Fri, Sep 5, 2008 at 8:13 PM, Robert Haas <robertmhaas@gmail.com> wrote:
Updated patch attached, based on comments from Ryan Bradetich and Tom
Lane, and sync'd to latest CVS version.
Thanks for the update. I am out of town until tomorrow evening.
I will re-review this patch when I get back if it has not been
committed by then.
- Ryan
"Robert Haas" <robertmhaas@gmail.com> writes:
Updated patch attached, based on comments from Ryan Bradetich and Tom
Lane, and sync'd to latest CVS version.
Applied with really pretty minor revisions --- this was a nice clean
patch. Changes I can recall making:
* You missed one or two documentation references to DELETE privilege.
* You modified the privileges test to create another userid, but forgot
to clean up afterwards.
* LOCK TABLE requires UPDATE or DELETE privilege for locks stronger
than AccessShareLock. I thought it would be inconsistent to not allow
TRUNCATE to satisfy this requirement too.
* Many of the information_schema views require some privilege on a table
to show details about the table. Again, it seemed inconsistent to not
allow TRUNCATE privilege to satisfy this requirement.
* A couple of the information_schema views show available privileges on
tables by name. It's a bit dubious whether we should show TRUNCATE in
them, since there is no such privilege bit in the SQL standard, but
after some reflection I concluded that functionality trumps a narrow
reading of the spec here. We can revisit that if anyone wants to argue
for the other way, though.
regards, tom lane
Applied with really pretty minor revisions --- this was a nice clean
patch. Changes I can recall making:
Woo-hoo, my first patch. Thanks for the cleanup.
...Robert
Show quoted text
* You missed one or two documentation references to DELETE privilege.
* You modified the privileges test to create another userid, but forgot
to clean up afterwards.* LOCK TABLE requires UPDATE or DELETE privilege for locks stronger
than AccessShareLock. I thought it would be inconsistent to not allow
TRUNCATE to satisfy this requirement too.* Many of the information_schema views require some privilege on a table
to show details about the table. Again, it seemed inconsistent to not
allow TRUNCATE privilege to satisfy this requirement.* A couple of the information_schema views show available privileges on
tables by name. It's a bit dubious whether we should show TRUNCATE in
them, since there is no such privilege bit in the SQL standard, but
after some reflection I concluded that functionality trumps a narrow
reading of the spec here. We can revisit that if anyone wants to argue
for the other way, though.regards, tom lane