Skip to content

Commit 553918a

Browse files
zilderzilder
committed
fix permissions
1 parent 59b96dd commit 553918a

File tree

9 files changed

+168
-85
lines changed

9 files changed

+168
-85
lines changed

‎Makefile

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,13 @@ EXTVERSION = 1.0
1212
DATA_built = $(EXTENSION)--$(EXTVERSION).sql
1313
PGFILEDESC = "pg_pathman - partitioning tool"
1414

15-
REGRESS = pathman_basic \
16-
pathman_runtime_nodes \
17-
pathman_callbacks \
18-
pathman_domains \
19-
pathman_foreign_keys
15+
# REGRESS = pathman_basic \
16+
# pathman_runtime_nodes \
17+
# pathman_callbacks \
18+
# pathman_domains \
19+
# pathman_foreign_keys \
20+
# pathman_permissions
21+
REGRESS = pathman_permissions
2022
EXTRA_REGRESS_OPTS=--temp-config=$(top_srcdir)/$(subdir)/conf.add
2123
EXTRA_CLEAN = $(EXTENSION)--$(EXTVERSION).sql ./isolation_output
2224

‎expected/pathman_permissions.out

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
\set VERBOSITY terse
2+
CREATE EXTENSION pg_pathman;
3+
CREATE ROLE user1 LOGIN;
4+
CREATE ROLE user2 LOGIN;
5+
SET ROLE user1;
6+
CREATE TABLE user1_table(id serial, a int);
7+
INSERT INTO user1_table SELECT g, g FROM generate_series(1, 20) as g;
8+
ALTER DEFAULT PRIVILEGES IN SCHEMA public GRANT SELECT, INSERT, UPDATE, DELETE ON TABLES TO public;
9+
SET ROLE user2;
10+
/* Not owner and not superuser cannot create partitions */
11+
SELECT create_range_partitions('user1_table', 'id', 1, 10, 2);
12+
ERROR: permission denied for relation user1_table
13+
SET ROLE user1;
14+
/* Owner can */
15+
SELECT create_range_partitions('user1_table', 'id', 1, 10, 2);
16+
NOTICE: sequence "user1_table_seq" does not exist, skipping
17+
create_range_partitions
18+
-------------------------
19+
2
20+
(1 row)
21+
22+
SET ROLE user2;
23+
/* Try to change partitioning parameters for user1_table */
24+
SELECT set_enable_parent('user1_table', true);
25+
ERROR: Only table owner or superuser can change partitioning configuration
26+
SELECT set_auto('user1_table', false);
27+
ERROR: Only table owner or superuser can change partitioning configuration
28+
/*
29+
* Check that user is able to propagate partitions by inserting rows that
30+
* doesn't fit into range
31+
*/
32+
INSERT INTO user1_table (id, a) VALUES (25, 0);
33+
ERROR: permission denied for relation user1_table
34+
SET ROLE user1;
35+
SELECT drop_partitions('test1_table');
36+
ERROR: relation "test1_table" does not exist at character 24
37+
SET ROLE user2;
38+
/* Test ddl event trigger */
39+
CREATE TABLE user2_table(id serial);
40+
SELECT create_hash_partitions('user2_table', 'id', 3);
41+
create_hash_partitions
42+
------------------------
43+
3
44+
(1 row)
45+
46+
INSERT INTO user2_table SELECT generate_series(1, 30);
47+
SELECT drop_partitions('user2_table');
48+
NOTICE: function public.user2_table_upd_trig_func() does not exist, skipping
49+
NOTICE: 9 rows copied from user2_table_0
50+
NOTICE: 11 rows copied from user2_table_1
51+
NOTICE: 10 rows copied from user2_table_2
52+
drop_partitions
53+
-----------------
54+
3
55+
(1 row)
56+
57+
RESET ROLE;
58+
DROP OWNED BY user1;
59+
DROP OWNED BY user2;
60+
DROP USER user1;
61+
DROP USER user2;

‎hash.sql

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,6 @@ DECLARE
2727
v_init_callback REGPROCEDURE;
2828

2929
BEGIN
30-
PERFORM @extschema@.check_permissions(parent_relid);
31-
3230
IF partition_data = true THEN
3331
/* Acquire data modification lock */
3432
PERFORM @extschema@.prevent_relation_modification(parent_relid);

‎init.sql

Lines changed: 20 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -49,24 +49,24 @@ TO public;
4949
/*
5050
* Check if current user can alter/drop specified relation
5151
*/
52-
CREATE OR REPLACE FUNCTION @extschema@.can_manage_relation(relation regclass)
53-
RETURNS BOOL AS 'pg_pathman', 'can_manage_relation' LANGUAGE C STRICT;
52+
CREATE OR REPLACE FUNCTION @extschema@.check_security_policy(relation regclass)
53+
RETURNS BOOL AS 'pg_pathman', 'check_security_policy' LANGUAGE C STRICT;
5454

5555
/*
5656
* Check user permissions. If permission denied then throw an error.
5757
*/
58-
CREATE OR REPLACE FUNCTION @extschema@.check_permissions(relation regclass)
59-
RETURNS BOOL AS 'pg_pathman', 'check_permissions' LANGUAGE C STRICT;
58+
-- CREATE OR REPLACE FUNCTION @extschema@.check_permissions(relation regclass)
59+
-- RETURNS BOOL AS 'pg_pathman', 'check_permissions' LANGUAGE C STRICT;
6060

6161
/*
6262
* Row security policy to restrict partitioning operations to owner and
6363
* superusers only
6464
*/
6565
CREATE POLICY deny_modification ON @extschema@.pathman_config
66-
FOR ALL USING (can_manage_relation(partrel));
66+
FOR ALL USING (check_security_policy(partrel));
6767

6868
CREATE POLICY deny_modification ON @extschema@.pathman_config_params
69-
FOR ALL USING (can_manage_relation(partrel));
69+
FOR ALL USING (check_security_policy(partrel));
7070

7171
CREATE POLICY allow_select ON @extschema@.pathman_config FOR SELECT USING (true);
7272

@@ -129,7 +129,7 @@ CREATE OR REPLACE FUNCTION @extschema@.pathman_set_param(
129129
RETURNS VOID AS
130130
$$
131131
BEGIN
132-
PERFORM @extschema@.check_permissions(relation);
132+
-- PERFORM @extschema@.check_permissions(relation);
133133

134134
EXECUTE format('INSERT INTO @extschema@.pathman_config_params
135135
(partrel, %1$s) VALUES ($1, $2)
@@ -336,7 +336,7 @@ CREATE OR REPLACE FUNCTION @extschema@.disable_pathman_for(
336336
RETURNS VOID AS
337337
$$
338338
BEGIN
339-
PERFORM @extschema@.check_permissions(parent_relid);
339+
-- PERFORM @extschema@.check_permissions(parent_relid);
340340

341341
DELETE FROM @extschema@.pathman_config WHERE partrel = parent_relid;
342342
PERFORM @extschema@.drop_triggers(parent_relid);
@@ -468,34 +468,29 @@ $$
468468
LANGUAGE plpgsql;
469469

470470
/*
471-
* DDL trigger that deletes entry from pathman_config table.
471+
* DDL trigger that removes entry from pathman_config table.
472472
*/
473473
CREATE OR REPLACE FUNCTION @extschema@.pathman_ddl_trigger_func()
474474
RETURNS event_trigger AS
475475
$$
476476
DECLARE
477477
obj record;
478478
pg_class_oid oid;
479+
relids regclass[];
479480
BEGIN
480481
pg_class_oid = 'pg_catalog.pg_class'::regclass;
481482

482-
/* Handle 'DROP TABLE' events */
483-
WITH to_be_deleted AS (
484-
SELECT cfg.partrel AS rel FROM pg_event_trigger_dropped_objects() AS events
485-
JOIN @extschema@.pathman_config AS cfg ON cfg.partrel::oid = events.objid
486-
WHERE events.classid = pg_class_oid
487-
)
488-
DELETE FROM @extschema@.pathman_config
489-
WHERE partrel IN (SELECT rel FROM to_be_deleted);
483+
/* Find relids to remove from config */
484+
SELECT array_agg(cfg.partrel) INTO relids
485+
FROM pg_event_trigger_dropped_objects() AS events
486+
JOIN @extschema@.pathman_config AS cfg ON cfg.partrel::oid = events.objid
487+
WHERE events.classid = pg_class_oid;
488+
489+
/* Cleanup pathman_config */
490+
DELETE FROM @extschema@.pathman_config WHERE partrel = ANY(relids);
490491

491492
/* Cleanup params table too */
492-
WITH to_be_deleted AS (
493-
SELECT cfg.partrel AS rel FROM pg_event_trigger_dropped_objects() AS events
494-
JOIN @extschema@.pathman_config_params AS cfg ON cfg.partrel::oid = events.objid
495-
WHERE events.classid = pg_class_oid
496-
)
497-
DELETE FROM @extschema@.pathman_config_params
498-
WHERE partrel IN (SELECT rel FROM to_be_deleted);
493+
DELETE FROM @extschema@.pathman_config_params WHERE partrel = ANY(relids);
499494
END
500495
$$
501496
LANGUAGE plpgsql;
@@ -530,7 +525,7 @@ DECLARE
530525
v_relkind CHAR;
531526

532527
BEGIN
533-
PERFORM @extschema@.check_permissions(parent_relid);
528+
-- PERFORM @extschema@.check_permissions(parent_relid);
534529

535530
/* Drop trigger first */
536531
PERFORM @extschema@.drop_triggers(parent_relid);

‎range.sql

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,6 @@ DECLARE
9595
i INTEGER;
9696

9797
BEGIN
98-
PERFORM @extschema@.check_permissions(parent_relid);
99-
10098
IF partition_data = true THEN
10199
/* Acquire data modification lock */
102100
PERFORM @extschema@.prevent_relation_modification(parent_relid);
@@ -208,8 +206,6 @@ DECLARE
208206
i INTEGER;
209207

210208
BEGIN
211-
PERFORM @extschema@.check_permissions(parent_relid);
212-
213209
IF partition_data = true THEN
214210
/* Acquire data modification lock */
215211
PERFORM @extschema@.prevent_relation_modification(parent_relid);
@@ -316,8 +312,6 @@ DECLARE
316312
part_count INTEGER := 0;
317313

318314
BEGIN
319-
PERFORM @extschema@.check_permissions(parent_relid);
320-
321315
IF partition_data = true THEN
322316
/* Acquire data modification lock */
323317
PERFORM @extschema@.prevent_relation_modification(parent_relid);
@@ -390,8 +384,6 @@ DECLARE
390384
part_count INTEGER := 0;
391385

392386
BEGIN
393-
PERFORM @extschema@.check_permissions(parent_relid);
394-
395387
IF partition_data = true THEN
396388
/* Acquire data modification lock */
397389
PERFORM @extschema@.prevent_relation_modification(parent_relid);

‎sql/pathman_permissions.sql

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,53 @@
1-
\SET VERBOSITY terse
1+
\set VERBOSITY terse
22

33
CREATE EXTENSION pg_pathman;
44
CREATE ROLE user1 LOGIN;
55
CREATE ROLE user2 LOGIN;
66

7-
\SET ROLE user1
7+
SET ROLE user1;
88

99
CREATE TABLE user1_table(id serial, a int);
10-
SELECT create_hash_partitioning('user1_table', 'id', 3);
10+
INSERT INTO user1_table SELECT g, g FROM generate_series(1, 20) as g;
1111

12-
ALTER DEFAULT PRIVILEGES IN SCHEMA public GRANT INSERT ON TABLES TO public;
12+
ALTER DEFAULT PRIVILEGES IN SCHEMA public GRANT SELECT, INSERT, UPDATE, DELETE ON TABLES TO public;
1313

14-
\SET ROLE user2
14+
SET ROLE user2;
1515

16-
/* Trying to change partitioning parameters for user1_table */
16+
/* Not owner and not superuser cannot create partitions */
17+
SELECT create_range_partitions('user1_table', 'id', 1, 10, 2);
18+
19+
SET ROLE user1;
20+
21+
/* Owner can */
22+
SELECT create_range_partitions('user1_table', 'id', 1, 10, 2);
23+
24+
SET ROLE user2;
25+
26+
/* Try to change partitioning parameters for user1_table */
1727
SELECT set_enable_parent('user1_table', true);
28+
SELECT set_auto('user1_table', false);
29+
30+
/*
31+
* Check that user is able to propagate partitions by inserting rows that
32+
* doesn't fit into range
33+
*/
34+
INSERT INTO user1_table (id, a) VALUES (25, 0);
35+
36+
SET ROLE user1;
37+
38+
SELECT drop_partitions('test1_table');
39+
40+
SET ROLE user2;
41+
42+
/* Test ddl event trigger */
43+
CREATE TABLE user2_table(id serial);
44+
SELECT create_hash_partitions('user2_table', 'id', 3);
45+
INSERT INTO user2_table SELECT generate_series(1, 30);
46+
SELECT drop_partitions('user2_table');
47+
48+
RESET ROLE;
49+
50+
DROP OWNED BY user1;
51+
DROP OWNED BY user2;
52+
DROP USER user1;
53+
DROP USER user2;

‎src/pl_funcs.c

Lines changed: 3 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
#include "utils/lsyscache.h"
2929
#include "utils/syscache.h"
3030

31-
static bool can_manage_relation_internal(Oid relid);
3231

3332
/* Function declarations */
3433

@@ -60,8 +59,7 @@ PG_FUNCTION_INFO_V1( prevent_relation_modification );
6059
PG_FUNCTION_INFO_V1( validate_on_part_init_callback_pl );
6160
PG_FUNCTION_INFO_V1( invoke_on_partition_created_callback );
6261

63-
PG_FUNCTION_INFO_V1( can_manage_relation );
64-
PG_FUNCTION_INFO_V1( check_permissions );
62+
PG_FUNCTION_INFO_V1( check_security_policy );
6563

6664
PG_FUNCTION_INFO_V1( debug_capture );
6765

@@ -858,45 +856,11 @@ invoke_on_partition_created_callback(PG_FUNCTION_ARGS)
858856
* check user permissions in order to let other users.
859857
*/
860858
Datum
861-
can_manage_relation(PG_FUNCTION_ARGS)
859+
check_security_policy(PG_FUNCTION_ARGS)
862860
{
863861
Oid relid = PG_GETARG_OID(0);
864862

865-
PG_RETURN_BOOL(can_manage_relation_internal(relid));
866-
}
867-
868-
static bool
869-
can_manage_relation_internal(Oid relid)
870-
{
871-
Oid owner;
872-
873-
/*
874-
* If user has superuser privileges then he or she can do whatever wants
875-
*/
876-
if (superuser())
877-
return true;
878-
879-
/* Otherwise check if user is owner of the relation */
880-
owner = get_rel_owner(relid);
881-
if (owner == GetUserId())
882-
return true;
883-
884-
return false;
885-
}
886-
887-
/*
888-
* Check user permissions. If permission denied then throw an error.
889-
*/
890-
Datum
891-
check_permissions(PG_FUNCTION_ARGS)
892-
{
893-
Oid relid = PG_GETARG_OID(0);
894-
895-
if (!can_manage_relation_internal(relid))
896-
elog(ERROR,
897-
"Only table owner or superuser can change partitioning configuration");
898-
899-
PG_RETURN_VOID();
863+
PG_RETURN_BOOL(check_security_policy_internal(relid));
900864
}
901865

902866

‎src/utils.c

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -743,3 +743,37 @@ validate_on_part_init_cb(Oid procid, bool emit_error)
743743

744744
return is_ok;
745745
}
746+
747+
/*
748+
* Check if user can alter/drop specified relation. This function is used to
749+
* make sure that current user can change pg_pathman's config. Returns true
750+
* if user can manage relation, false otherwise.
751+
*
752+
* XXX currently we just check if user is a table owner. Probably it's better to
753+
* check user permissions in order to let other users.
754+
*/
755+
bool
756+
check_security_policy_internal(Oid relid)
757+
{
758+
Oid owner;
759+
760+
/*
761+
* If user has superuser privileges then he or she can do whatever wants
762+
*/
763+
if (superuser())
764+
return true;
765+
766+
/*
767+
* Sometimes the relation doesn't exist anymore but there is still a record
768+
* in config. It for example happens in event trigger function. So we
769+
* should be able to remove this record
770+
*/
771+
if ((owner = get_rel_owner(relid)) == InvalidOid)
772+
return true;
773+
774+
/* Check if current user is an owner of the relation */
775+
if (owner != GetUserId())
776+
elog(ERROR, "Only table owner or superuser can change partitioning configuration");
777+
778+
return true;
779+
}

0 commit comments

Comments
 (0)