-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 07/03/2015 10:03 AM, Noah Misch wrote:
> (2) CreatePolicy() and AlterPolicy() omit to create a pg_shdepend
> entry for each role in the TO clause. Test case:
Please see the attached patch. Note that I used SHARED_DEPENDENCY_ACL
for this. It seems appropriate, but possibly we should invent a new
shared dependency type for this use case? Comments?
Thanks,
Joe
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)
iQIcBAEBAgAGBQJVtSlAAAoJEDfy90M199hlrkIP/0BqMj30JpJVj3H5pIopU0RZ
pesBzFzzsWmt2QmasX9hajRfo6eXQAWPmKQmw/NDTJvHHNACxLdo0MHE7A9y7No7
aZIFTm0KhnG4jpzxfzpGqQ4ron5Vsc2TlNQgCBYCtbEEtuD0mV2qmcuTkqGte7iB
7iqneRBhmTYBy63X7S0ir4AhDi+JJg8P4F7YuU/XMcha5v5CpNJAToPpW7mCoZ8O
9w/RbXCHso7p1DIxfx4tfxVwLQ7j7G2j0rXbuA2e6OKfwZWWrk5E8Wnvc3xy3yCY
J37fcjOxFdhU/j1j+Tr90LYOSuRu5fQ4z6PmmsPkBM3T0Vllz/nNP64aVKbmHzga
szrIBERRs2icKa4OI08qZF42ObIEv0/t/NuyrXpegY6awRNNNsjyZvwM+51zdjw1
fuWh+2rObzh3RDH8lPB0N0rVfDMM+wU85Vaekckv/7UQ/pbWcwsYD/tykA1engQ8
Ww8kBAEct4dWdqppTqvLLxLgo4d+vuiS1mJ7SRY2aZFRX8QQ8TfB3eObETUsU4/X
9UWyMn+E0Au9ICX+wfD4whaBKst8EuO36qOZx0oZt+++EMOlzypgkCCxDtZO0KWd
KZHbtmJXHFVH+ihbEGXAKMIx+gLqSDG3IKy9MIJxpB3JY3XSdBNqobL2hiQy36/w
svK7i+mEYmUCQ6pB1j8c
=P1AS
-----END PGP SIGNATURE-----
diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c
index 17b48d4..20ac54e 100644
*** a/src/backend/commands/policy.c
--- b/src/backend/commands/policy.c
***************
*** 22,27 ****
--- 22,28 ----
#include "catalog/indexing.h"
#include "catalog/namespace.h"
#include "catalog/objectaccess.h"
+ #include "catalog/pg_authid.h"
#include "catalog/pg_policy.h"
#include "catalog/pg_type.h"
#include "commands/policy.h"
***************
*** 48,54 ****
static void RangeVarCallbackForPolicy(const RangeVar *rv,
Oid relid, Oid oldrelid, void *arg);
static char parse_policy_command(const char *cmd_name);
! static ArrayType *policy_role_list_to_array(List *roles);
/*
* Callback to RangeVarGetRelidExtended().
--- 49,55 ----
static void RangeVarCallbackForPolicy(const RangeVar *rv,
Oid relid, Oid oldrelid, void *arg);
static char parse_policy_command(const char *cmd_name);
! static Datum *policy_role_list_to_array(List *roles, int *num_roles);
/*
* Callback to RangeVarGetRelidExtended().
*************** parse_policy_command(const char *cmd_nam
*** 130,159 ****
/*
* policy_role_list_to_array
! * helper function to convert a list of RoleSpecs to an array of role ids.
*/
! static ArrayType *
! policy_role_list_to_array(List *roles)
{
! ArrayType *role_ids;
! Datum *temp_array;
ListCell *cell;
- int num_roles;
int i = 0;
/* Handle no roles being passed in as being for public */
if (roles == NIL)
{
! temp_array = (Datum *) palloc(sizeof(Datum));
! temp_array[0] = ObjectIdGetDatum(ACL_ID_PUBLIC);
! role_ids = construct_array(temp_array, 1, OIDOID, sizeof(Oid), true,
! 'i');
! return role_ids;
}
! num_roles = list_length(roles);
! temp_array = (Datum *) palloc(num_roles * sizeof(Datum));
foreach(cell, roles)
{
--- 131,158 ----
/*
* policy_role_list_to_array
! * helper function to convert a list of RoleSpecs to an array of
! * role id Datums.
*/
! static Datum *
! policy_role_list_to_array(List *roles, int *num_roles)
{
! Datum *role_oids;
ListCell *cell;
int i = 0;
/* Handle no roles being passed in as being for public */
if (roles == NIL)
{
! *num_roles = 1;
! role_oids = (Datum *) palloc(*num_roles * sizeof(Datum));
! role_oids[0] = ObjectIdGetDatum(ACL_ID_PUBLIC);
! return role_oids;
}
! *num_roles = list_length(roles);
! role_oids = (Datum *) palloc(*num_roles * sizeof(Datum));
foreach(cell, roles)
{
*************** policy_role_list_to_array(List *roles)
*** 164,187 ****
*/
if (spec->roletype == ROLESPEC_PUBLIC)
{
! if (num_roles != 1)
ereport(WARNING,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("ignoring roles specified other than public"),
errhint("All roles are members of the public role.")));
! temp_array[0] = ObjectIdGetDatum(ACL_ID_PUBLIC);
! num_roles = 1;
! break;
}
else
! temp_array[i++] =
ObjectIdGetDatum(get_rolespec_oid((Node *) spec, false));
}
! role_ids = construct_array(temp_array, num_roles, OIDOID, sizeof(Oid), true,
! 'i');
!
! return role_ids;
}
/*
--- 163,187 ----
*/
if (spec->roletype == ROLESPEC_PUBLIC)
{
! Datum *tmp_role_oids;
!
! if (*num_roles != 1)
ereport(WARNING,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("ignoring roles specified other than public"),
errhint("All roles are members of the public role.")));
! *num_roles = 1;
! tmp_role_oids = (Datum *) palloc(*num_roles * sizeof(Datum));
! tmp_role_oids[0] = ObjectIdGetDatum(ACL_ID_PUBLIC);
!
! return tmp_role_oids;
}
else
! role_oids[i++] =
ObjectIdGetDatum(get_rolespec_oid((Node *) spec, false));
}
! return role_oids;
}
/*
*************** CreatePolicy(CreatePolicyStmt *stmt)
*** 463,468 ****
--- 463,470 ----
Relation target_table;
Oid table_id;
char polcmd;
+ Datum *role_oids;
+ int nitems = 0;
ArrayType *role_ids;
ParseState *qual_pstate;
ParseState *with_check_pstate;
*************** CreatePolicy(CreatePolicyStmt *stmt)
*** 476,481 ****
--- 478,484 ----
bool isnull[Natts_pg_policy];
ObjectAddress target;
ObjectAddress myself;
+ int i;
/* Parse command */
polcmd = parse_policy_command(stmt->cmd);
*************** CreatePolicy(CreatePolicyStmt *stmt)
*** 498,506 ****
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("only WITH CHECK expression allowed for INSERT")));
-
/* Collect role ids */
! role_ids = policy_role_list_to_array(stmt->roles);
/* Parse the supplied clause */
qual_pstate = make_parsestate(NULL);
--- 501,510 ----
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("only WITH CHECK expression allowed for INSERT")));
/* Collect role ids */
! role_oids = policy_role_list_to_array(stmt->roles, &nitems);
! role_ids = construct_array(role_oids, nitems, OIDOID,
! sizeof(Oid), true, 'i');
/* Parse the supplied clause */
qual_pstate = make_parsestate(NULL);
*************** CreatePolicy(CreatePolicyStmt *stmt)
*** 614,619 ****
--- 618,635 ----
recordDependencyOnExpr(&myself, with_check_qual,
with_check_pstate->p_rtable, DEPENDENCY_NORMAL);
+ /* Register role dependencies */
+ target.classId = AuthIdRelationId;
+ target.objectSubId = 0;
+ for (i = 0; i < nitems; i++)
+ {
+ target.objectId = DatumGetObjectId(role_oids[i]);
+ /* no dependency if public */
+ if (target.objectId != ACL_ID_PUBLIC)
+ recordSharedDependencyOn(&myself, &target,
+ SHARED_DEPENDENCY_ACL);
+ }
+
/* Invalidate Relation Cache */
CacheInvalidateRelcache(target_table);
*************** AlterPolicy(AlterPolicyStmt *stmt)
*** 641,646 ****
--- 657,664 ----
Oid policy_id;
Relation target_table;
Oid table_id;
+ Datum *role_oids;
+ int nitems = 0;
ArrayType *role_ids = NULL;
List *qual_parse_rtable = NIL;
List *with_check_parse_rtable = NIL;
*************** AlterPolicy(AlterPolicyStmt *stmt)
*** 658,667 ****
Datum cmd_datum;
char polcmd;
bool polcmd_isnull;
/* Parse role_ids */
if (stmt->roles != NULL)
! role_ids = policy_role_list_to_array(stmt->roles);
/* Get id of table. Also handles permissions checks. */
table_id = RangeVarGetRelidExtended(stmt->table, AccessExclusiveLock,
--- 676,690 ----
Datum cmd_datum;
char polcmd;
bool polcmd_isnull;
+ int i;
/* Parse role_ids */
if (stmt->roles != NULL)
! {
! role_oids = policy_role_list_to_array(stmt->roles, &nitems);
! role_ids = construct_array(role_oids, nitems, OIDOID,
! sizeof(Oid), true, 'i');
! }
/* Get id of table. Also handles permissions checks. */
table_id = RangeVarGetRelidExtended(stmt->table, AccessExclusiveLock,
*************** AlterPolicy(AlterPolicyStmt *stmt)
*** 825,830 ****
--- 848,866 ----
recordDependencyOnExpr(&myself, with_check_qual, with_check_parse_rtable,
DEPENDENCY_NORMAL);
+ /* Register role dependencies */
+ deleteSharedDependencyRecordsFor(PolicyRelationId, policy_id, 0);
+ target.classId = AuthIdRelationId;
+ target.objectSubId = 0;
+ for (i = 0; i < nitems; i++)
+ {
+ target.objectId = DatumGetObjectId(role_oids[i]);
+ /* no dependency if public */
+ if (target.objectId != ACL_ID_PUBLIC)
+ recordSharedDependencyOn(&myself, &target,
+ SHARED_DEPENDENCY_ACL);
+ }
+
heap_freetuple(new_tuple);
/* Invalidate Relation Cache */
diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
index e7c242c..48c85ca 100644
*** a/src/test/regress/expected/rowsecurity.out
--- b/src/test/regress/expected/rowsecurity.out
*************** SELECT * FROM coll_t;
*** 2860,2865 ****
--- 2860,2936 ----
ROLLBACK;
--
+ -- Shared Object Dependencies
+ --
+ RESET SESSION AUTHORIZATION;
+ BEGIN;
+ CREATE ROLE alice;
+ CREATE ROLE bob;
+ CREATE TABLE tbl1 (c) AS VALUES ('bar'::text);
+ GRANT SELECT ON TABLE tbl1 TO alice;
+ CREATE POLICY P ON tbl1 TO alice, bob USING (true);
+ SELECT refclassid::regclass, deptype
+ FROM pg_depend
+ WHERE classid = 'pg_policy'::regclass
+ AND refobjid = 'tbl1'::regclass;
+ refclassid | deptype
+ ------------+---------
+ pg_class | a
+ (1 row)
+
+ SELECT refclassid::regclass, deptype
+ FROM pg_shdepend
+ WHERE classid = 'pg_policy'::regclass
+ AND refobjid IN ('alice'::regrole, 'bob'::regrole);
+ refclassid | deptype
+ ------------+---------
+ pg_authid | a
+ pg_authid | a
+ (2 rows)
+
+ SAVEPOINT q;
+ DROP ROLE alice; --fails due to dependency on POLICY p
+ ERROR: role "alice" cannot be dropped because some objects depend on it
+ DETAIL: privileges for policy p on table tbl1
+ privileges for table tbl1
+ ROLLBACK TO q;
+ ALTER POLICY p ON tbl1 TO bob USING (true);
+ SAVEPOINT q;
+ DROP ROLE alice; --fails due to dependency on GRANT SELECT
+ ERROR: role "alice" cannot be dropped because some objects depend on it
+ DETAIL: privileges for table tbl1
+ ROLLBACK TO q;
+ REVOKE ALL ON TABLE tbl1 FROM alice;
+ SAVEPOINT q;
+ DROP ROLE alice; --succeeds
+ ROLLBACK TO q;
+ SAVEPOINT q;
+ DROP ROLE bob; --fails due to dependency on POLICY p
+ ERROR: role "bob" cannot be dropped because some objects depend on it
+ DETAIL: privileges for policy p on table tbl1
+ ROLLBACK TO q;
+ DROP POLICY p ON tbl1;
+ SAVEPOINT q;
+ DROP ROLE bob; -- succeeds
+ ROLLBACK TO q;
+ SELECT refclassid::regclass, deptype
+ FROM pg_depend
+ WHERE classid = 'pg_policy'::regclass
+ AND refobjid = 'tbl1'::regclass;
+ refclassid | deptype
+ ------------+---------
+ (0 rows)
+
+ SELECT refclassid::regclass, deptype
+ FROM pg_shdepend
+ WHERE classid = 'pg_policy'::regclass
+ AND refobjid IN ('alice'::regrole, 'bob'::regrole);
+ refclassid | deptype
+ ------------+---------
+ (0 rows)
+
+ ROLLBACK; -- cleanup
+ --
-- Clean up objects
--
RESET SESSION AUTHORIZATION;
diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql
index e86f814..f1f1b6b 100644
*** a/src/test/regress/sql/rowsecurity.sql
--- b/src/test/regress/sql/rowsecurity.sql
*************** SELECT * FROM coll_t;
*** 1153,1158 ****
--- 1153,1211 ----
ROLLBACK;
--
+ -- Shared Object Dependencies
+ --
+ RESET SESSION AUTHORIZATION;
+ BEGIN;
+ CREATE ROLE alice;
+ CREATE ROLE bob;
+ CREATE TABLE tbl1 (c) AS VALUES ('bar'::text);
+ GRANT SELECT ON TABLE tbl1 TO alice;
+ CREATE POLICY P ON tbl1 TO alice, bob USING (true);
+ SELECT refclassid::regclass, deptype
+ FROM pg_depend
+ WHERE classid = 'pg_policy'::regclass
+ AND refobjid = 'tbl1'::regclass;
+ SELECT refclassid::regclass, deptype
+ FROM pg_shdepend
+ WHERE classid = 'pg_policy'::regclass
+ AND refobjid IN ('alice'::regrole, 'bob'::regrole);
+
+ SAVEPOINT q;
+ DROP ROLE alice; --fails due to dependency on POLICY p
+ ROLLBACK TO q;
+
+ ALTER POLICY p ON tbl1 TO bob USING (true);
+ SAVEPOINT q;
+ DROP ROLE alice; --fails due to dependency on GRANT SELECT
+ ROLLBACK TO q;
+
+ REVOKE ALL ON TABLE tbl1 FROM alice;
+ SAVEPOINT q;
+ DROP ROLE alice; --succeeds
+ ROLLBACK TO q;
+
+ SAVEPOINT q;
+ DROP ROLE bob; --fails due to dependency on POLICY p
+ ROLLBACK TO q;
+
+ DROP POLICY p ON tbl1;
+ SAVEPOINT q;
+ DROP ROLE bob; -- succeeds
+ ROLLBACK TO q;
+
+ SELECT refclassid::regclass, deptype
+ FROM pg_depend
+ WHERE classid = 'pg_policy'::regclass
+ AND refobjid = 'tbl1'::regclass;
+ SELECT refclassid::regclass, deptype
+ FROM pg_shdepend
+ WHERE classid = 'pg_policy'::regclass
+ AND refobjid IN ('alice'::regrole, 'bob'::regrole);
+
+ ROLLBACK; -- cleanup
+
+ --
-- Clean up objects
--
RESET SESSION AUTHORIZATION;
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
