Re: [PATCH] Implement (and document, and test) has_sequence_privilege() - Mailing list pgsql-hackers
| From | Joe Conway |
|---|---|
| Subject | Re: [PATCH] Implement (and document, and test) has_sequence_privilege() |
| Date | |
| Msg-id | 4A74EC13.3010200@joeconway.com Whole thread Raw |
| In response to | [PATCH] Implement (and document, and test) has_sequence_privilege() (Abhijit Menon-Sen <ams@oryx.com>) |
| Responses |
Re: [PATCH] Implement (and document, and test) has_sequence_privilege()
|
| List | pgsql-hackers |
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 In response to: http://archives.postgresql.org/message-id/1238394513-21474-1-git-send-email-ams@oryx.com Review complete. Somewhat modified patch attached. I wasn't happy with creation of verify_sequence_oid() when it more-or-less duplicates the functionality of get_rel_relkind(). Also, the original patch was misleading in that the following comment: + * The result is a boolean value: true if user has the indicated + * privilege, false if not. The variants that take a relation OID + * return NULL if the OID doesn't exist (rather than failing, as + * they did before Postgres 8.4). implies that the relname variants should throw an ERROR for non-existing names (which they in fact do), but the code for these functions reads, e.g.: +Datum +has_sequence_privilege_name_name(PG_FUNCTION_ARGS) +{ + Name rolename = PG_GETARG_NAME(0); + text *sequencename = PG_GETARG_TEXT_P(1); <snip> + sequenceoid = convert_table_name(sequencename); + if (!verify_sequence_oid(sequenceoid)) + PG_RETURN_NULL(); Here, verify_sequence_oid() is never called for a non-existing relname, because the call to convert_table_name() will throw an ERROR first. And verify_sequence_oid() will throw an ERROR if the relation is not a sequence. Therefore verify_sequence_oid() will only return if TRUE, and PG_RETURN_NULL() is never reached. But if it were able to be reached, it would be in conflict with the comment. If there are no objections, I'll commit in a day or two. Joe -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org iQIcBAEBCAAGBQJKdOwTAAoJEDfy90M199hlFpwP/0merKdY+czyozrFuEdiGsPA Ai7twjRNyNNiJlWfr7hU+RxsXlNxST7lxCI792b/ZKJe8Z1gx4u7qYlddKVCMgFn Qeqvo0sC2N2FUMWMuBOPpDCl2sR6N2YtCxHep1NrAxDK58nupeSZ9vZ69ywwWA8e xFhbTabJDSIECbN+FISfaR60LIrTd6dlV18maPOkWmDgM3Ff11GagtB9ngZngmdY 3eN/D6wOMqtAQOcCZNyTIf68rLvKrMDu1U+bses3JUZE0NcMa76H1jm2BE3KNY1A tp+5fXEuazk+NEzBND6eSksUOnemqCt+MsyC4eim2bGQX61+tmm8tKHjCt0CYucO ERTeOhX7vJcZILzzrU287SC880spGXUz74ivApLG7SNquZ8qr1YkdYLHJdKt4SYh pJc1T+b1ngYK1/SWWzoUzEiZa+3dBReRXXmXv2IP59RSaCzYMgQ+Ohqt6D8cTHnS gSdV2csd0nae2vRuSSOms4yviLzPzh2t8GxVo7eDHre8/kHsOw0eMPPVYeA6nE/l v5r42raBr++WT/VSS0/3nOGOCAZoYcPGCv7R6Cbz7QkIOTNXbZ9PA2IUaQ2znNcQ 9nF0/aRmgPOuJUtubxMN8Qs5UE5g7HqDU7EaJ3NfL5jWCrYC2f1HWnwxS17xszCs ropug1p5344+3GZRLQtS =1T1A -----END PGP SIGNATURE----- Index: doc/src/sgml/func.sgml =================================================================== RCS file: /opt/src/cvs/pgsql/doc/src/sgml/func.sgml,v retrieving revision 1.483 diff -c -r1.483 func.sgml *** doc/src/sgml/func.sgml 22 Jul 2009 18:07:26 -0000 1.483 --- doc/src/sgml/func.sgml 1 Aug 2009 23:08:49 -0000 *************** *** 11789,11794 **** --- 11789,11809 ---- <entry>does current user have privilege for foreign server</entry> </row> <row> + <entry><literal><function>has_sequence_privilege</function>(<parameter>user</parameter>, + <parameter>sequence</parameter>, + <parameter>privilege</parameter>)</literal> + </entry> + <entry><type>boolean</type></entry> + <entry>does user have privilege for sequence</entry> + </row> + <row> + <entry><literal><function>has_sequence_privilege</function>(<parameter>sequence</parameter>, + <parameter>privilege</parameter>)</literal> + </entry> + <entry><type>boolean</type></entry> + <entry>does current user have privilege for sequence</entry> + </row> + <row> <entry><literal><function>has_table_privilege</function>(<parameter>user</parameter>, <parameter>table</parameter>, <parameter>privilege</parameter>)</literal> *************** *** 11862,11867 **** --- 11877,11885 ---- <primary>has_server_privilege</primary> </indexterm> <indexterm> + <primary>has_sequence_privilege</primary> + </indexterm> + <indexterm> <primary>has_table_privilege</primary> </indexterm> <indexterm> *************** *** 11901,11906 **** --- 11919,11934 ---- </para> <para> + <function>has_sequence_privilege</function> checks whether a user + can access a sequence in a particular way. The possibilities for its + arguments are analogous to <function>has_table_privilege</function>. + The desired access privilege type must evaluate to one of + <literal>USAGE</literal>, + <literal>SELECT</literal>, or + <literal>UPDATE</literal>. + </para> + + <para> <function>has_any_column_privilege</function> checks whether a user can access any column of a table in a particular way. Its argument possibilities Index: src/backend/utils/adt/acl.c =================================================================== RCS file: /opt/src/cvs/pgsql/src/backend/utils/adt/acl.c,v retrieving revision 1.148 diff -c -r1.148 acl.c *** src/backend/utils/adt/acl.c 11 Jun 2009 14:49:03 -0000 1.148 --- src/backend/utils/adt/acl.c 2 Aug 2009 00:59:12 -0000 *************** *** 20,25 **** --- 20,26 ---- #include "catalog/pg_authid.h" #include "catalog/pg_auth_members.h" #include "catalog/pg_type.h" + #include "catalog/pg_class.h" #include "commands/dbcommands.h" #include "commands/tablespace.h" #include "foreign/foreign.h" *************** *** 88,93 **** --- 89,95 ---- static Oid convert_table_name(text *tablename); static AclMode convert_table_priv_string(text *priv_type_text); + static AclMode convert_sequence_priv_string(text *priv_type_text); static AttrNumber convert_column_name(Oid tableoid, text *column); static AclMode convert_column_priv_string(text *priv_type_text); static Oid convert_database_name(text *databasename); *************** *** 1704,1709 **** --- 1706,1922 ---- return convert_any_priv_string(priv_type_text, table_priv_map); } + /* + * has_sequence_privilege variants + * These are all named "has_sequence_privilege" at the SQL level. + * They take various combinations of relation name, relation OID, + * user name, user OID, or implicit user = current_user. + * + * The result is a boolean value: true if user has the indicated + * privilege, false if not. The variants that take a relation OID + * return NULL if the OID doesn't exist (rather than failing, as + * they did before Postgres 8.4). + */ + + /* + * has_sequence_privilege_name_name + * Check user privileges on a sequence given + * name username, text sequencename, and text priv name. + */ + Datum + has_sequence_privilege_name_name(PG_FUNCTION_ARGS) + { + Name rolename = PG_GETARG_NAME(0); + text *sequencename = PG_GETARG_TEXT_P(1); + text *priv_type_text = PG_GETARG_TEXT_P(2); + Oid roleid; + Oid sequenceoid; + AclMode mode; + AclResult aclresult; + + roleid = get_roleid_checked(NameStr(*rolename)); + mode = convert_sequence_priv_string(priv_type_text); + sequenceoid = convert_table_name(sequencename); + if (get_rel_relkind(sequenceoid) != RELKIND_SEQUENCE) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("\"%s\" is not a sequence", + text_to_cstring(sequencename)))); + + aclresult = pg_class_aclcheck(sequenceoid, roleid, mode); + + PG_RETURN_BOOL(aclresult == ACLCHECK_OK); + } + + /* + * has_sequence_privilege_name + * Check user privileges on a sequence given + * text sequencename and text priv name. + * current_user is assumed + */ + Datum + has_sequence_privilege_name(PG_FUNCTION_ARGS) + { + text *sequencename = PG_GETARG_TEXT_P(0); + text *priv_type_text = PG_GETARG_TEXT_P(1); + Oid roleid; + Oid sequenceoid; + AclMode mode; + AclResult aclresult; + + roleid = GetUserId(); + mode = convert_sequence_priv_string(priv_type_text); + sequenceoid = convert_table_name(sequencename); + if (get_rel_relkind(sequenceoid) != RELKIND_SEQUENCE) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("\"%s\" is not a sequence", + text_to_cstring(sequencename)))); + + aclresult = pg_class_aclcheck(sequenceoid, roleid, mode); + + PG_RETURN_BOOL(aclresult == ACLCHECK_OK); + } + + /* + * has_sequence_privilege_name_id + * Check user privileges on a sequence given + * name usename, sequence oid, and text priv name. + */ + Datum + has_sequence_privilege_name_id(PG_FUNCTION_ARGS) + { + Name username = PG_GETARG_NAME(0); + Oid sequenceoid = PG_GETARG_OID(1); + text *priv_type_text = PG_GETARG_TEXT_P(2); + Oid roleid; + AclMode mode; + AclResult aclresult; + char relkind; + + roleid = get_roleid_checked(NameStr(*username)); + mode = convert_sequence_priv_string(priv_type_text); + relkind = get_rel_relkind(sequenceoid); + if (relkind == '\0') + PG_RETURN_NULL(); + else if (relkind != RELKIND_SEQUENCE) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("\"%s\" is not a sequence", + get_rel_name(sequenceoid)))); + + aclresult = pg_class_aclcheck(sequenceoid, roleid, mode); + + PG_RETURN_BOOL(aclresult == ACLCHECK_OK); + } + + /* + * has_sequence_privilege_id + * Check user privileges on a sequence given + * sequence oid, and text priv name. + * current_user is assumed + */ + Datum + has_sequence_privilege_id(PG_FUNCTION_ARGS) + { + Oid sequenceoid = PG_GETARG_OID(0); + text *priv_type_text = PG_GETARG_TEXT_P(1); + Oid roleid; + AclMode mode; + AclResult aclresult; + char relkind; + + roleid = GetUserId(); + mode = convert_sequence_priv_string(priv_type_text); + relkind = get_rel_relkind(sequenceoid); + if (relkind == '\0') + PG_RETURN_NULL(); + else if (relkind != RELKIND_SEQUENCE) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("\"%s\" is not a sequence", + get_rel_name(sequenceoid)))); + + aclresult = pg_class_aclcheck(sequenceoid, roleid, mode); + + PG_RETURN_BOOL(aclresult == ACLCHECK_OK); + } + + /* + * has_sequence_privilege_id_name + * Check user privileges on a sequence given + * roleid, text sequencename, and text priv name. + */ + Datum + has_sequence_privilege_id_name(PG_FUNCTION_ARGS) + { + Oid roleid = PG_GETARG_OID(0); + text *sequencename = PG_GETARG_TEXT_P(1); + text *priv_type_text = PG_GETARG_TEXT_P(2); + Oid sequenceoid; + AclMode mode; + AclResult aclresult; + + mode = convert_sequence_priv_string(priv_type_text); + sequenceoid = convert_table_name(sequencename); + if (get_rel_relkind(sequenceoid) != RELKIND_SEQUENCE) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("\"%s\" is not a sequence", + text_to_cstring(sequencename)))); + + aclresult = pg_class_aclcheck(sequenceoid, roleid, mode); + + PG_RETURN_BOOL(aclresult == ACLCHECK_OK); + } + + /* + * has_sequence_privilege_id_id + * Check user privileges on a sequence given + * roleid, sequence oid, and text priv name. + */ + Datum + has_sequence_privilege_id_id(PG_FUNCTION_ARGS) + { + Oid roleid = PG_GETARG_OID(0); + Oid sequenceoid = PG_GETARG_OID(1); + text *priv_type_text = PG_GETARG_TEXT_P(2); + AclMode mode; + AclResult aclresult; + char relkind; + + mode = convert_sequence_priv_string(priv_type_text); + relkind = get_rel_relkind(sequenceoid); + if (relkind == '\0') + PG_RETURN_NULL(); + else if (relkind != RELKIND_SEQUENCE) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("\"%s\" is not a sequence", + get_rel_name(sequenceoid)))); + + aclresult = pg_class_aclcheck(sequenceoid, roleid, mode); + + PG_RETURN_BOOL(aclresult == ACLCHECK_OK); + } + + /* + * convert_sequence_priv_string + * Convert text string to AclMode value. + */ + static AclMode + convert_sequence_priv_string(text *priv_type_text) + { + static const priv_map sequence_priv_map[] = { + { "USAGE", ACL_USAGE }, + { "SELECT", ACL_SELECT }, + { "UPDATE", ACL_UPDATE }, + { NULL, 0 } + }; + + return convert_any_priv_string(priv_type_text, sequence_priv_map); + } + /* * has_any_column_privilege variants Index: src/include/catalog/catversion.h =================================================================== RCS file: /opt/src/cvs/pgsql/src/include/catalog/catversion.h,v retrieving revision 1.534 diff -c -r1.534 catversion.h *** src/include/catalog/catversion.h 29 Jul 2009 20:56:19 -0000 1.534 --- src/include/catalog/catversion.h 1 Aug 2009 23:44:11 -0000 *************** *** 53,58 **** */ /* yyyymmddN */ ! #define CATALOG_VERSION_NO 200907291 #endif --- 53,58 ---- */ /* yyyymmddN */ ! #define CATALOG_VERSION_NO 200908011 #endif Index: src/include/catalog/pg_proc.h =================================================================== RCS file: /opt/src/cvs/pgsql/src/include/catalog/pg_proc.h,v retrieving revision 1.547 diff -c -r1.547 pg_proc.h *** src/include/catalog/pg_proc.h 29 Jul 2009 20:56:20 -0000 1.547 --- src/include/catalog/pg_proc.h 1 Aug 2009 23:08:49 -0000 *************** *** 2918,2923 **** --- 2918,2936 ---- DATA(insert OID = 1927 ( has_table_privilege PGNSP PGUID 12 1 0 0 f f f t f s 2 0 16 "26 25" _null_ _null_ _null__null_ has_table_privilege_id _null_ _null_ _null_ )); DESCR("current user privilege on relation by rel oid"); + DATA(insert OID = 2181 ( has_sequence_privilege PGNSP PGUID 12 1 0 0 f f f t f s 3 0 16 "19 25 25" _null_ _null__null_ _null_ has_sequence_privilege_name_name _null_ _null_ _null_ )); + DESCR("user privilege on sequence by username, seq name"); + DATA(insert OID = 2182 ( has_sequence_privilege PGNSP PGUID 12 1 0 0 f f f t f s 3 0 16 "19 26 25" _null_ _null__null_ _null_ has_sequence_privilege_name_id _null_ _null_ _null_ )); + DESCR("user privilege on sequence by username, seq oid"); + DATA(insert OID = 2183 ( has_sequence_privilege PGNSP PGUID 12 1 0 0 f f f t f s 3 0 16 "26 25 25" _null_ _null__null_ _null_ has_sequence_privilege_id_name _null_ _null_ _null_ )); + DESCR("user privilege on sequence by user oid, seq name"); + DATA(insert OID = 2184 ( has_sequence_privilege PGNSP PGUID 12 1 0 0 f f f t f s 3 0 16 "26 26 25" _null_ _null__null_ _null_ has_sequence_privilege_id_id _null_ _null_ _null_ )); + DESCR("user privilege on sequence by user oid, seq oid"); + DATA(insert OID = 2185 ( has_sequence_privilege PGNSP PGUID 12 1 0 0 f f f t f s 2 0 16 "25 25" _null_ _null__null_ _null_ has_sequence_privilege_name _null_ _null_ _null_ )); + DESCR("current user privilege on sequence by seq name"); + DATA(insert OID = 2186 ( has_sequence_privilege PGNSP PGUID 12 1 0 0 f f f t f s 2 0 16 "26 25" _null_ _null__null_ _null_ has_sequence_privilege_id _null_ _null_ _null_ )); + DESCR("current user privilege on sequence by seq oid"); + DATA(insert OID = 3012 ( has_column_privilege PGNSP PGUID 12 1 0 0 f f f t f s 4 0 16 "19 25 25 25" _null_ _null__null_ _null_ has_column_privilege_name_name_name _null_ _null_ _null_ )); DESCR("user privilege on column by username, rel name, col name"); DATA(insert OID = 3013 ( has_column_privilege PGNSP PGUID 12 1 0 0 f f f t f s 4 0 16 "19 25 21 25" _null_ _null__null_ _null_ has_column_privilege_name_name_attnum _null_ _null_ _null_ )); Index: src/include/utils/builtins.h =================================================================== RCS file: /opt/src/cvs/pgsql/src/include/utils/builtins.h,v retrieving revision 1.335 diff -c -r1.335 builtins.h *** src/include/utils/builtins.h 29 Jul 2009 20:56:21 -0000 1.335 --- src/include/utils/builtins.h 1 Aug 2009 23:08:49 -0000 *************** *** 46,51 **** --- 46,57 ---- extern Datum has_table_privilege_id_id(PG_FUNCTION_ARGS); extern Datum has_table_privilege_name(PG_FUNCTION_ARGS); extern Datum has_table_privilege_id(PG_FUNCTION_ARGS); + extern Datum has_sequence_privilege_name_name(PG_FUNCTION_ARGS); + extern Datum has_sequence_privilege_name_id(PG_FUNCTION_ARGS); + extern Datum has_sequence_privilege_id_name(PG_FUNCTION_ARGS); + extern Datum has_sequence_privilege_id_id(PG_FUNCTION_ARGS); + extern Datum has_sequence_privilege_name(PG_FUNCTION_ARGS); + extern Datum has_sequence_privilege_id(PG_FUNCTION_ARGS); extern Datum has_database_privilege_name_name(PG_FUNCTION_ARGS); extern Datum has_database_privilege_name_id(PG_FUNCTION_ARGS); extern Datum has_database_privilege_id_name(PG_FUNCTION_ARGS); Index: src/test/regress/expected/privileges.out =================================================================== RCS file: /opt/src/cvs/pgsql/src/test/regress/expected/privileges.out,v retrieving revision 1.46 diff -c -r1.46 privileges.out *** src/test/regress/expected/privileges.out 5 Mar 2009 17:30:29 -0000 1.46 --- src/test/regress/expected/privileges.out 1 Aug 2009 23:08:49 -0000 *************** *** 815,822 **** --- 815,844 ---- t (1 row) + -- has_sequence_privilege tests + \c - + CREATE SEQUENCE x_seq; + GRANT USAGE on x_seq to regressuser2; + SELECT has_sequence_privilege('regressuser1', 'atest1', 'SELECT'); + ERROR: "atest1" is not a sequence + SELECT has_sequence_privilege('regressuser1', 'x_seq', 'INSERT'); + ERROR: unrecognized privilege type: "INSERT" + SELECT has_sequence_privilege('regressuser1', 'x_seq', 'SELECT'); + has_sequence_privilege + ------------------------ + f + (1 row) + + SET SESSION AUTHORIZATION regressuser2; + SELECT has_sequence_privilege('x_seq', 'USAGE'); + has_sequence_privilege + ------------------------ + t + (1 row) + -- clean up \c + drop sequence x_seq; DROP FUNCTION testfunc2(int); DROP FUNCTION testfunc4(boolean); DROP VIEW atestv1; Index: src/test/regress/sql/privileges.sql =================================================================== RCS file: /opt/src/cvs/pgsql/src/test/regress/sql/privileges.sql,v retrieving revision 1.25 diff -c -r1.25 privileges.sql *** src/test/regress/sql/privileges.sql 5 Mar 2009 17:30:29 -0000 1.25 --- src/test/regress/sql/privileges.sql 1 Aug 2009 23:08:49 -0000 *************** *** 469,478 **** --- 469,495 ---- SELECT has_table_privilege('regressuser1', 'atest4', 'SELECT WITH GRANT OPTION'); -- true + -- has_sequence_privilege tests + \c - + + CREATE SEQUENCE x_seq; + + GRANT USAGE on x_seq to regressuser2; + + SELECT has_sequence_privilege('regressuser1', 'atest1', 'SELECT'); + SELECT has_sequence_privilege('regressuser1', 'x_seq', 'INSERT'); + SELECT has_sequence_privilege('regressuser1', 'x_seq', 'SELECT'); + + SET SESSION AUTHORIZATION regressuser2; + + SELECT has_sequence_privilege('x_seq', 'USAGE'); + -- clean up \c + drop sequence x_seq; + DROP FUNCTION testfunc2(int); DROP FUNCTION testfunc4(boolean);
pgsql-hackers by date: