From 3cf62ecd2e9e23d4d3292c1f21e093c698080f33 Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Tue, 10 Feb 2015 15:27:42 +0100 Subject: [PATCH 1/5] Do not grant select permission to public user - Fake the behaviour in tests by switching between users --- expected/test_ddl_triggers.out | 12 ++++++++++++ scripts-available/CDB_TableMetadata.sql | 6 +++--- sql/test_ddl_triggers.sql | 16 +++++++++++++--- 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/expected/test_ddl_triggers.out b/expected/test_ddl_triggers.out index 3380717..737468a 100644 --- a/expected/test_ddl_triggers.out +++ b/expected/test_ddl_triggers.out @@ -32,6 +32,7 @@ SET SESSION AUTHORIZATION 'cartodb_postgresql_unpriv_user'; ---------------------- -- CREATE TABLE ---------------------- +SET SESSION AUTHORIZATION 'cartodb_postgresql_unpriv_user'; select 1 as i INTO c.t3; NOTICE: trigger "track_updates" for table "c.t3" does not exist, skipping NOTICE: trigger "update_the_geom_webmercator_trigger" for table "c.t3" does not exist, skipping @@ -55,6 +56,7 @@ from c.t3; 1 | t | t | | | 1 (1 row) +RESET SESSION AUTHORIZATION; select tabname::text, round(extract('secs' from now() - updated_at)) as age @@ -64,6 +66,7 @@ FROM CDB_TableMetadata WHERE tabname = 'c.t3'::regclass; c.t3 | 0 (1 row) +SET SESSION AUTHORIZATION 'cartodb_postgresql_unpriv_user'; -- Table with cartodb_id field, see -- http://github.com/CartoDB/cartodb-postgresql/issues/32 select 1 as cartodb_id INTO c.t4; @@ -91,6 +94,7 @@ from c.t4; 1 | t | t | | (1 row) +RESET SESSION AUTHORIZATION; select tabname::text, round(extract('secs' from now() - updated_at)) as age @@ -103,6 +107,7 @@ FROM CDB_TableMetadata WHERE tabname = 'c.t4'::regclass; ---------------------------- -- ALTER TABLE RENAME COLUMN ---------------------------- +SET SESSION AUTHORIZATION 'cartodb_postgresql_unpriv_user'; select pg_sleep(.1); pg_sleep ---------- @@ -131,6 +136,7 @@ from c.t3; 1 | t | t | | | 1 | (1 row) +RESET SESSION AUTHORIZATION; select tabname::text, round(extract('secs' from now() - updated_at)*10) as agecs @@ -140,6 +146,7 @@ FROM CDB_TableMetadata WHERE tabname = 'c.t3'::regclass; c.t3 | 0 (1 row) +SET SESSION AUTHORIZATION 'cartodb_postgresql_unpriv_user'; select pg_sleep(.1); pg_sleep ---------- @@ -168,6 +175,7 @@ from c.t3; 1 | t | t | | | 1 | | (1 row) +RESET SESSION AUTHORIZATION; select tabname::text, round(extract('secs' from now() - updated_at)*10) as agecs @@ -180,6 +188,7 @@ FROM CDB_TableMetadata WHERE tabname = 'c.t3'::regclass; ---------------------------- -- ALTER TABLE DROP COLUMN ---------------------------- +SET SESSION AUTHORIZATION 'cartodb_postgresql_unpriv_user'; select pg_sleep(.1); pg_sleep ---------- @@ -208,6 +217,7 @@ from c.t3; 1 | t | t | | | 1 | | (1 row) +RESET SESSION AUTHORIZATION; select tabname::text, round(extract('secs' from now() - updated_at)*10) as agecs @@ -220,6 +230,7 @@ FROM CDB_TableMetadata WHERE tabname = 'c.t3'::regclass; ---------------------------- -- ALTER TABLE ADD COLUMN ---------------------------- +SET SESSION AUTHORIZATION 'cartodb_postgresql_unpriv_user'; select pg_sleep(.1); pg_sleep ---------- @@ -239,6 +250,7 @@ from c.t3; 1 | t | t | | | 1 | | | (1 row) +RESET SESSION AUTHORIZATION; select tabname::text, round(extract('secs' from now() - updated_at)*10) as agecs diff --git a/scripts-available/CDB_TableMetadata.sql b/scripts-available/CDB_TableMetadata.sql index 9fcf3d1..745abd9 100644 --- a/scripts-available/CDB_TableMetadata.sql +++ b/scripts-available/CDB_TableMetadata.sql @@ -5,9 +5,9 @@ CREATE TABLE IF NOT EXISTS updated_at timestamp with time zone not null default now() ); --- Anyone can see this, but updates are only possible trough --- the security definer trigger -GRANT SELECT ON public.CDB_TableMetadata TO public; +-- No one can see this +-- Updates are only possible trough the security definer trigger +-- GRANT SELECT ON public.CDB_TableMetadata TO public; -- -- Trigger logging updated_at in the CDB_TableMetadata diff --git a/sql/test_ddl_triggers.sql b/sql/test_ddl_triggers.sql index 4259914..df5c3a4 100644 --- a/sql/test_ddl_triggers.sql +++ b/sql/test_ddl_triggers.sql @@ -18,7 +18,7 @@ SET SESSION AUTHORIZATION 'cartodb_postgresql_unpriv_user'; ---------------------- -- CREATE TABLE ---------------------- - +SET SESSION AUTHORIZATION 'cartodb_postgresql_unpriv_user'; select 1 as i INTO c.t3; select @@ -28,11 +28,13 @@ select i from c.t3; +RESET SESSION AUTHORIZATION; select tabname::text, round(extract('secs' from now() - updated_at)) as age FROM CDB_TableMetadata WHERE tabname = 'c.t3'::regclass; +SET SESSION AUTHORIZATION 'cartodb_postgresql_unpriv_user'; -- Table with cartodb_id field, see -- http://github.com/CartoDB/cartodb-postgresql/issues/32 select 1 as cartodb_id INTO c.t4; @@ -41,6 +43,8 @@ select NOW() - updated_at < '1 secs' as "u<1s", the_geom, the_geom_webmercator from c.t4; + +RESET SESSION AUTHORIZATION; select tabname::text, round(extract('secs' from now() - updated_at)) as age @@ -49,6 +53,7 @@ FROM CDB_TableMetadata WHERE tabname = 'c.t4'::regclass; ---------------------------- -- ALTER TABLE RENAME COLUMN ---------------------------- +SET SESSION AUTHORIZATION 'cartodb_postgresql_unpriv_user'; select pg_sleep(.1); alter table c.t3 rename column the_geom_webmercator to webmerc; @@ -60,11 +65,13 @@ select i, webmerc from c.t3; +RESET SESSION AUTHORIZATION; select tabname::text, round(extract('secs' from now() - updated_at)*10) as agecs FROM CDB_TableMetadata WHERE tabname = 'c.t3'::regclass; +SET SESSION AUTHORIZATION 'cartodb_postgresql_unpriv_user'; select pg_sleep(.1); alter table c.t3 rename column the_geom_webmercator to webmerc2; @@ -75,6 +82,7 @@ select i, webmerc, webmerc2 from c.t3; +RESET SESSION AUTHORIZATION; select tabname::text, round(extract('secs' from now() - updated_at)*10) as agecs @@ -83,7 +91,7 @@ FROM CDB_TableMetadata WHERE tabname = 'c.t3'::regclass; ---------------------------- -- ALTER TABLE DROP COLUMN ---------------------------- - +SET SESSION AUTHORIZATION 'cartodb_postgresql_unpriv_user'; select pg_sleep(.1); alter table c.t3 drop column the_geom_webmercator; @@ -94,6 +102,7 @@ select i, webmerc, webmerc2 from c.t3; +RESET SESSION AUTHORIZATION; select tabname::text, round(extract('secs' from now() - updated_at)*10) as agecs @@ -102,7 +111,7 @@ FROM CDB_TableMetadata WHERE tabname = 'c.t3'::regclass; ---------------------------- -- ALTER TABLE ADD COLUMN ---------------------------- - +SET SESSION AUTHORIZATION 'cartodb_postgresql_unpriv_user'; select pg_sleep(.1); alter table c.t3 add column id2 int; @@ -113,6 +122,7 @@ select i, webmerc, webmerc2, id2 from c.t3; +RESET SESSION AUTHORIZATION; select tabname::text, round(extract('secs' from now() - updated_at)*10) as agecs From da7b3b70801bab0f3547bbc97ab8e99c2518d7e6 Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Wed, 18 Feb 2015 17:08:46 +0100 Subject: [PATCH 2/5] Adds CDB_TableMetadataTouch function to be able to upsert updated_at value in cdb_tablemetadata --- scripts-available/CDB_TableMetadata.sql | 18 +++++++++++++++ test/extension/run_at_cartodb_schema.sql | 1 + test/extension/test.sh | 29 ++++++++++++++++++++++++ 3 files changed, 48 insertions(+) mode change 100644 => 100755 test/extension/test.sh diff --git a/scripts-available/CDB_TableMetadata.sql b/scripts-available/CDB_TableMetadata.sql index 745abd9..d91f798 100644 --- a/scripts-available/CDB_TableMetadata.sql +++ b/scripts-available/CDB_TableMetadata.sql @@ -118,3 +118,21 @@ CREATE TRIGGER table_modified AFTER INSERT OR UPDATE ON CDB_TableMetadata FOR EACH ROW EXECUTE PROCEDURE _CDB_TableMetadata_Updated(); + +-- similar to TOUCH(1) in unix filesystems but for table in cdb_tablemetadata +CREATE OR REPLACE FUNCTION public.CDB_TableMetadataTouch(tablename regclass) + RETURNS void AS + $$ + BEGIN + WITH upsert AS ( + UPDATE public.cdb_tablemetadata + SET updated_at = NOW() + WHERE tabname = tablename + RETURNING * + ) + INSERT INTO public.cdb_tablemetadata (tabname, updated_at) + SELECT tablename, NOW() + WHERE NOT EXISTS (SELECT * FROM upsert); + END; + $$ +LANGUAGE 'plpgsql' VOLATILE STRICT; diff --git a/test/extension/run_at_cartodb_schema.sql b/test/extension/run_at_cartodb_schema.sql index a443aab..c3792ff 100644 --- a/test/extension/run_at_cartodb_schema.sql +++ b/test/extension/run_at_cartodb_schema.sql @@ -1,3 +1,4 @@ SET SCHEMA 'cartodb'; \i scripts-available/CDB_Quota.sql +\i scripts-available/CDB_TableMetadata.sql SET SCHEMA 'public'; \ No newline at end of file diff --git a/test/extension/test.sh b/test/extension/test.sh old mode 100644 new mode 100755 index 1cfd515..6b46a58 --- a/test/extension/test.sh +++ b/test/extension/test.sh @@ -75,6 +75,15 @@ function sql() { set_failed fi fi + + if [[ "$3" == "should-not" ]] + then + if [[ "${RESULT}" == "$4" ]] + then + log_error "QUERY '${QUERY}' did not expect '${RESULT}'" + set_failed + fi + fi } @@ -277,6 +286,26 @@ function test_quota_for_each_user() { sql cdb_testmember_2 "SELECT cartodb.CDB_UserDataSize('cdb_testmember_2'::TEXT);" should 4096 } +function test_cdb_tablemetadatatouch() { + sql "CREATE TABLE touch_example (a int)" + sql postgres "SELECT updated_at FROM CDB_TableMetadata WHERE tabname = 'touch_example'::regclass;" should '' + sql "SELECT CDB_TableMetadataTouch('touch_example');" + sql postgres "SELECT updated_at FROM CDB_TableMetadata WHERE tabname = 'touch_example'::regclass;" should-not '' + + # Another call doesn't fail + sql "SELECT CDB_TableMetadataTouch('touch_example');" + sql postgres "SELECT updated_at FROM CDB_TableMetadata WHERE tabname = 'touch_example'::regclass;" should-not '' + + + + #### test tear down + sql 'DROP TABLE touch_example;' +} + +function test_cdb_tablemetadatatouch_fails_for_unexistent_table() { + sql postgres "SELECT CDB_TableMetadataTouch('unexistent_example');" fails +} + #################################################### TESTS END HERE #################################################### run_tests $@ From f18232037d84f6c9594d087d6bc6c7efa599e5ff Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Wed, 18 Feb 2015 17:31:04 +0100 Subject: [PATCH 3/5] Release notes and version bump --- Makefile | 3 ++- NEWS.md | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 35e0ddf..6982440 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ # cartodb/Makefile EXTENSION = cartodb -EXTVERSION = 0.5.3 +EXTVERSION = 0.6.0 SED = sed @@ -32,6 +32,7 @@ UPGRADABLE = \ 0.5.0 \ 0.5.1 \ 0.5.2 \ + 0.5.3 \ $(EXTVERSION)dev \ $(EXTVERSION)next \ $(END) diff --git a/NEWS.md b/NEWS.md index 6f4d412..c841d14 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,8 @@ +0.6.0 (2015-02-xx) +------------------ +* Select permission in CDB_TableMetadata no longer granted to public +* New function to upsert the updated_at in CDB_TableMetadata for a regclass + 0.5.3 (2015-02-xx) ------------------ * Fixed secuity problem related with system tables From 27aec0d4b47bd2459a1f3bc5947b8d02399d253f Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Wed, 18 Feb 2015 18:01:29 +0100 Subject: [PATCH 4/5] Adds tests for qualified table names --- test/extension/test.sh | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/extension/test.sh b/test/extension/test.sh index 6b46a58..cea712c 100755 --- a/test/extension/test.sh +++ b/test/extension/test.sh @@ -296,7 +296,11 @@ function test_cdb_tablemetadatatouch() { sql "SELECT CDB_TableMetadataTouch('touch_example');" sql postgres "SELECT updated_at FROM CDB_TableMetadata WHERE tabname = 'touch_example'::regclass;" should-not '' - + # Works with qualified tables + sql "SELECT CDB_TableMetadataTouch('public.touch_example');" + sql "SELECT CDB_TableMetadataTouch('public.\"touch_example\"');" + sql "SELECT CDB_TableMetadataTouch('\"public\".touch_example');" + sql "SELECT CDB_TableMetadataTouch('\"public\".\"touch_example\"');" #### test tear down sql 'DROP TABLE touch_example;' From f16f53ceabb7f8bb5ff4e488a612951f06fcd44a Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Thu, 19 Feb 2015 14:02:27 +0100 Subject: [PATCH 5/5] Adds test for non-authorized writes to cdb_tablemetadata through CDB_TableMetadataTouch --- test/extension/test.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/extension/test.sh b/test/extension/test.sh index cea712c..171d323 100755 --- a/test/extension/test.sh +++ b/test/extension/test.sh @@ -310,6 +310,18 @@ function test_cdb_tablemetadatatouch_fails_for_unexistent_table() { sql postgres "SELECT CDB_TableMetadataTouch('unexistent_example');" fails } +function test_cdb_tablemetadatatouch_fails_from_user_without_permission() { + sql "CREATE TABLE touch_example (a int);" + sql postgres "SELECT CDB_TableMetadataTouch('touch_example');" + + sql cdb_testmember_1 "SELECT CDB_TableMetadataTouch('touch_example');" fails + + sql postgres "GRANT ALL ON CDB_TableMetadata TO cdb_testmember_1;" + sql cdb_testmember_1 "SELECT CDB_TableMetadataTouch('touch_example');" + + sql postgres "REVOKE ALL ON CDB_TableMetadata FROM cdb_testmember_1;" +} + #################################################### TESTS END HERE #################################################### run_tests $@