From 204e5bd4bf6c58aeee5ca68ffba6b9d89b4a9b4f Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat Date: Thu, 17 Oct 2024 14:59:26 +0530 Subject: [PATCH v12 05/14] Access permissions on property graph Access to the property graph referenced in GRAPH_TABLE clause and the base relations underlying the graph pattern specified in that clause are governed by the following rules. 1. A user needs SELECT privilege to access a property graph in the query. 2. The property graph itself acts similar to a security invoker view in the sense that the current_user should have privileges to access the tables and columns, underlying the graph pattern. For example, a property graph g1 contains graph element tables e1 and e2. Assume two users u1 and u2 with privileges to access e1 and e2 respectively. Let's also assume that they have privileges to access the property graph g1. u1, as the current user, would be able to run query involving g1 successfully as long as it does *not* require access to e2. Similarly u2 would be able to run query involving g1 successfully as long as it does *not* require access to e1. 3. If a property graph is referenced in a security definer view, access to the property graph is governed by the privileges of the owner of the view but the access to the base relations underlying the graph pattern are governed by the privileges of the user executing the query. This is similar to how access to the base relations underlying a security invoker view referenced from a security definer view is handled. access 4. We have not implemented security definer property graphs since SQL/PGQ standard does not mention those. Note: The privileges tests defined views which are owned by users other than the table owners. Right now, property graph can not contain relations which are not owned by its owner. Hence I have not added tests for testing view access through property graphs. Per SQL/PGQ standard, a property graph can contain any relation as long as it has SELECT privilege on that relation. Once we implement what standard says, we can add the tests with views and preferrably add the views as edge tables. Author: Ashutosh Bapat Discussion: https://www.postgresql.org/message-id/flat/a855795d-e697-4fa5-8698-d20122126567@eisentraut.org --- doc/src/sgml/ref/create_property_graph.sgml | 8 ++ src/backend/rewrite/rewriteGraphTable.c | 18 ++++ src/test/regress/expected/privileges.out | 91 +++++++++++++++++++++ src/test/regress/sql/privileges.sql | 58 +++++++++++++ 4 files changed, 175 insertions(+) diff --git a/doc/src/sgml/ref/create_property_graph.sgml b/doc/src/sgml/ref/create_property_graph.sgml index f88d1194cbc..ae301b52e0a 100644 --- a/doc/src/sgml/ref/create_property_graph.sgml +++ b/doc/src/sgml/ref/create_property_graph.sgml @@ -276,6 +276,14 @@ CREATE PROPERTY GRAPH g1 Property graphs are queried using the GRAPH_TABLE clause of . + + + Access to the base relations underlying the GRAPH_TABLE + clause is determined by the permissions of the user executing the query, + rather than the property graph owner. Thus, the user of a property graph must + have the relevant permissions on the property graph and base relations + underlying the GRAPH_TABLE clause. + diff --git a/src/backend/rewrite/rewriteGraphTable.c b/src/backend/rewrite/rewriteGraphTable.c index 1ea5a5caeb1..9c14fa3b1b7 100644 --- a/src/backend/rewrite/rewriteGraphTable.c +++ b/src/backend/rewrite/rewriteGraphTable.c @@ -21,6 +21,7 @@ #include "catalog/pg_propgraph_property.h" #include "nodes/makefuncs.h" #include "nodes/nodeFuncs.h" +#include "optimizer/optimizer.h" #include "parser/analyze.h" #include "parser/parse_node.h" #include "parser/parse_relation.h" @@ -380,6 +381,7 @@ generate_query_for_graph_path(RangeTblEntry *rte, List *graph_path) Query *path_query = makeNode(Query); List *fromlist = NIL; List *qual_exprs = NIL; + List *vars; path_query->commandType = CMD_SELECT; @@ -493,6 +495,22 @@ generate_query_for_graph_path(RangeTblEntry *rte, List *graph_path) replace_property_refs(rte->relid, (Node *) rte->graph_table_columns, graph_path)); + + /* + * Mark the columns being accessed in the path query as requiring SELECT + * privilege. Any lateral columns should have been handled when the + * corresponding ColumnRefs were transformed. Ignore those here. + */ + vars = pull_vars_of_level((Node *) list_make2(qual_exprs, path_query->targetList), 0); + foreach_node(Var, var, vars) + { + RTEPermissionInfo *perminfo = getRTEPermissionInfo(path_query->rteperminfos, + rt_fetch(var->varno, path_query->rtable)); + + /* Must offset the attnum to fit in a bitmapset */ + perminfo->selectedCols = bms_add_member(perminfo->selectedCols, + var->varattno - FirstLowInvalidHeapAttributeNumber); + } return path_query; } diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out index a76256405fe..847d4dc31a0 100644 --- a/src/test/regress/expected/privileges.out +++ b/src/test/regress/expected/privileges.out @@ -2985,9 +2985,100 @@ revoke select on dep_priv_test from regress_priv_user4 cascade; set session role regress_priv_user1; drop table dep_priv_test; +-- +-- Property graphs +-- +set session role regress_priv_user1; +create property graph ptg1 + vertex tables ( + atest5 key (four) + default label properties (four) + label lttc properties (three as lttck), + atest1 key (a) + default label + label lttc properties (a as lttck), + atest2 key (col1) + default label + label ltv properties (col1 as ltvk)); +-- select privileges on property graph as well as table +select * from graph_table (ptg1 match ( : atest5) COLUMNS (1 as value)) limit 0; -- ok + value +------- +(0 rows) + +grant select on ptg1 to regress_priv_user2; +set session role regress_priv_user2; +select * from graph_table (ptg1 match ( : atest1) COLUMNS (1 as value)) limit 0; -- ok + value +------- +(0 rows) + +-- select privileges on property graph but not table +select * from graph_table (ptg1 match ( : atest5) COLUMNS (1 as value)) limit 0; -- fails +ERROR: permission denied for table atest5 +select * from graph_table (ptg1 match ( : lttc) COLUMNS (1 as value)) limit 0; -- fails +ERROR: permission denied for table atest5 +set session role regress_priv_user3; +-- select privileges on table but not property graph +select * from graph_table (ptg1 match ( : atest1) COLUMNS (1 as value)) limit 0; -- fails +ERROR: permission denied for property graph ptg1 +-- select privileges on neither +select * from graph_table (ptg1 match ( : atest5) COLUMNS (1 as value)) limit 0; -- fails +ERROR: permission denied for property graph ptg1 +-- column privileges +set session role regress_priv_user1; +select * from graph_table (ptg1 match (v : lttc) COLUMNS (v.lttck)) limit 0; -- ok + lttck +------- +(0 rows) + +grant select on ptg1 to regress_priv_user4; +set session role regress_priv_user4; +select * from graph_table (ptg1 match (a : atest5) COLUMNS (a.four)) limit 0; -- ok + four +------ +(0 rows) + +select * from graph_table (ptg1 match (v : lttc) COLUMNS (v.lttck)) limit 0; -- fail +ERROR: permission denied for table atest5 +-- access property graph through security definer view +set session role regress_priv_user4; +create view atpgv1 as select * from graph_table (ptg1 match ( : atest1) COLUMNS (1 as value)) limit 0; +grant select on atpgv1 to regress_priv_user3; +select * from atpgv1; -- ok + value +------- +(0 rows) + +set session role regress_priv_user3; +select * from atpgv1; -- ok + value +------- +(0 rows) + +set session role regress_priv_user4; +create view atpgv2 as select * from graph_table (ptg1 match (v : ltv) COLUMNS (v.ltvk)) limit 0; +-- though the session user is the owner of the view and also has access to the +-- property graph, it does not have access to a table referenced in the graph +-- pattern +select * from atpgv2; -- fail +ERROR: permission denied for table atest2 +grant select on atpgv2 to regress_priv_user2; +-- The user who otherwise does not have access to the property graph, gets +-- access to it through a security definer view and uses it successfully since +-- it has access to the tables referenced in the graph pattern. +set session role regress_priv_user2; +select * from atpgv2; -- ok + ltvk +------ +(0 rows) + -- clean up \c drop sequence x_seq; +drop view atpgv1; +drop view atpgv2; +drop property graph ptg1; DROP AGGREGATE priv_testagg1(int); DROP FUNCTION priv_testfunc2(int); DROP FUNCTION priv_testfunc4(boolean); diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql index d195aaf1377..f45412d8739 100644 --- a/src/test/regress/sql/privileges.sql +++ b/src/test/regress/sql/privileges.sql @@ -1764,6 +1764,60 @@ revoke select on dep_priv_test from regress_priv_user4 cascade; set session role regress_priv_user1; drop table dep_priv_test; +-- +-- Property graphs +-- +set session role regress_priv_user1; +create property graph ptg1 + vertex tables ( + atest5 key (four) + default label properties (four) + label lttc properties (three as lttck), + atest1 key (a) + default label + label lttc properties (a as lttck), + atest2 key (col1) + default label + label ltv properties (col1 as ltvk)); +-- select privileges on property graph as well as table +select * from graph_table (ptg1 match ( : atest5) COLUMNS (1 as value)) limit 0; -- ok +grant select on ptg1 to regress_priv_user2; +set session role regress_priv_user2; +select * from graph_table (ptg1 match ( : atest1) COLUMNS (1 as value)) limit 0; -- ok +-- select privileges on property graph but not table +select * from graph_table (ptg1 match ( : atest5) COLUMNS (1 as value)) limit 0; -- fails +select * from graph_table (ptg1 match ( : lttc) COLUMNS (1 as value)) limit 0; -- fails +set session role regress_priv_user3; +-- select privileges on table but not property graph +select * from graph_table (ptg1 match ( : atest1) COLUMNS (1 as value)) limit 0; -- fails +-- select privileges on neither +select * from graph_table (ptg1 match ( : atest5) COLUMNS (1 as value)) limit 0; -- fails +-- column privileges +set session role regress_priv_user1; +select * from graph_table (ptg1 match (v : lttc) COLUMNS (v.lttck)) limit 0; -- ok +grant select on ptg1 to regress_priv_user4; +set session role regress_priv_user4; +select * from graph_table (ptg1 match (a : atest5) COLUMNS (a.four)) limit 0; -- ok +select * from graph_table (ptg1 match (v : lttc) COLUMNS (v.lttck)) limit 0; -- fail +-- access property graph through security definer view +set session role regress_priv_user4; +create view atpgv1 as select * from graph_table (ptg1 match ( : atest1) COLUMNS (1 as value)) limit 0; +grant select on atpgv1 to regress_priv_user3; +select * from atpgv1; -- ok +set session role regress_priv_user3; +select * from atpgv1; -- ok +set session role regress_priv_user4; +create view atpgv2 as select * from graph_table (ptg1 match (v : ltv) COLUMNS (v.ltvk)) limit 0; +-- though the session user is the owner of the view and also has access to the +-- property graph, it does not have access to a table referenced in the graph +-- pattern +select * from atpgv2; -- fail +grant select on atpgv2 to regress_priv_user2; +-- The user who otherwise does not have access to the property graph, gets +-- access to it through a security definer view and uses it successfully since +-- it has access to the tables referenced in the graph pattern. +set session role regress_priv_user2; +select * from atpgv2; -- ok -- clean up @@ -1771,6 +1825,10 @@ drop table dep_priv_test; drop sequence x_seq; +drop view atpgv1; +drop view atpgv2; +drop property graph ptg1; + DROP AGGREGATE priv_testagg1(int); DROP FUNCTION priv_testfunc2(int); DROP FUNCTION priv_testfunc4(boolean); -- 2.39.5