diff --git a/scripts-available/CDB_FederatedServer.sql b/scripts-available/CDB_FederatedServer.sql index c496e59..bf5a841 100644 --- a/scripts-available/CDB_FederatedServer.sql +++ b/scripts-available/CDB_FederatedServer.sql @@ -5,9 +5,16 @@ -- -- This function is just a placement to store and use the pattern for -- foreign object names --- Servers: cdb_fs_$(server_name) --- Schemas: cdb_fs_schema_$(md5sum(server_name || remote_schema_name)) --- Owner role: cdb_fs_$(md5sum(current_database() || server_name) +-- Servers: cdb_fs_$(server_name) +-- View schema: cdb_fs_$(server_name) +-- > This is where all views created when importing tables are placed +-- > One server has only one view schema +-- Import Schemas: cdb_fs_schema_$(md5sum(server_name || remote_schema_name)) +-- > This is where the foreign tables are placed +-- > One server has one import schema per remote schema plus auxiliar ones used +-- to access the remote catalog (pg_catalog, information_schema...) +-- Owner role: cdb_fs_$(md5sum(current_database() || server_name) +-- > This is the role than owns all schemas and tables related to the server -- CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_Name_Pattern() RETURNS TEXT @@ -19,6 +26,7 @@ LANGUAGE SQL IMMUTABLE PARALLEL SAFE; -- -- Produce a valid DB name for servers generated for the Federated Server -- If check_existence is true, it'll throw if the server doesn't exists +-- This name is also used as the schema to store views -- CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_Generate_Server_Name(input_name TEXT, check_existence BOOL) RETURNS NAME @@ -230,16 +238,19 @@ BEGIN IF NOT EXISTS (SELECT * FROM pg_foreign_server WHERE srvname = server_internal) THEN BEGIN EXECUTE FORMAT('CREATE SERVER %I FOREIGN DATA WRAPPER postgres_fdw', server_internal); - -- TODO: Delete this IF before merging to make sure nobody creates a role - -- that is later used automatically by us granting them all permissions in the foreign server - -- TODO: This is here to help debugging during development (so failures to destroy objects are allowed) - -- TODO IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = role_name) THEN EXECUTE FORMAT('CREATE ROLE %I NOLOGIN', role_name); END IF; EXECUTE FORMAT('GRANT ALL PRIVILEGES ON DATABASE %I TO %I', current_database(), role_name); - EXECUTE FORMAT('GRANT ALL ON SCHEMA %I TO %I', '@extschema@', role_name); - EXECUTE FORMAT('GRANT EXECUTE ON ALL FUNCTIONS in SCHEMA %I TO %I', '@extschema@', role_name); + + -- These grants over `@extschema@` and `@postgisschema@` are necessary for the cases + -- where the schemas aren't accessible to PUBLIC, which is what happens in a CARTO database + EXECUTE FORMAT('GRANT USAGE ON SCHEMA %I TO %I', '@extschema@', role_name); + EXECUTE FORMAT('GRANT EXECUTE ON ALL FUNCTIONS IN SCHEMA %I TO %I', '@extschema@', role_name); + EXECUTE FORMAT('GRANT USAGE ON SCHEMA %I TO %I', '@postgisschema@', role_name); + EXECUTE FORMAT('GRANT EXECUTE ON ALL FUNCTIONS IN SCHEMA %I TO %I', '@postgisschema@', role_name); + EXECUTE FORMAT('GRANT SELECT ON ALL TABLES IN SCHEMA %I TO %I', '@postgisschema@', role_name); + EXECUTE FORMAT('GRANT USAGE ON FOREIGN DATA WRAPPER postgres_fdw TO %I', role_name); EXECUTE FORMAT('GRANT USAGE ON FOREIGN DATA WRAPPER postgres_fdw TO %I', role_name); EXECUTE FORMAT('GRANT USAGE ON FOREIGN SERVER %I TO %I', server_internal, role_name); @@ -348,6 +359,8 @@ LANGUAGE PLPGSQL IMMUTABLE PARALLEL SAFE; -- -- Grant access to a server +-- In the future we might consider adding the server's view schema to the role search_path +-- to make it easier to access the created views -- CREATE OR REPLACE FUNCTION @extschema@.CDB_Federated_Server_Grant_Access(server TEXT, db_role NAME) RETURNS void diff --git a/scripts-available/CDB_FederatedServerTables.sql b/scripts-available/CDB_FederatedServerTables.sql index 8356340..013de5e 100644 --- a/scripts-available/CDB_FederatedServerTables.sql +++ b/scripts-available/CDB_FederatedServerTables.sql @@ -173,6 +173,8 @@ LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; -- can be used through SQL and Maps API's. -- If the table was already exported, it will be dropped and re-imported -- +-- The view is placed under the server's view schema (cdb_fs_$(server_name)) +-- -- E.g: -- SELECT cartodb.CDB_SetUp_PG_Federated_Table( -- 'amazon', -- mandatory, name of the federated server @@ -198,7 +200,10 @@ AS $$ DECLARE server_internal name := @extschema@.__CDB_FS_Generate_Server_Name(input_name => server, check_existence => false); local_schema name := @extschema@.__CDB_FS_Create_Schema(server_internal, remote_schema); - src_table REGCLASS; + role_name name := @extschema@.__CDB_FS_Generate_Server_Role_Name(server_internal); + + src_table REGCLASS; -- import_schema.remote_table - import_schema is local_schema + local_view text; -- view_schema.local_name - view_schema is server_internal rest_of_cols TEXT[]; geom_expression TEXT; @@ -284,31 +289,28 @@ BEGIN webmercator_expression ]; - -- To create the view we switch to the caller role to make sure we have permissions - -- to write in the destination schema - RESET ROLE; + -- Create view schema if it doesn't exist + IF NOT EXISTS (SELECT oid FROM pg_namespace WHERE nspname = server_internal) THEN + EXECUTE 'CREATE SCHEMA ' || quote_ident(server_internal) || ' AUTHORIZATION ' || quote_ident(role_name); + END IF; -- Create a view with homogeneous CDB fields BEGIN EXECUTE format( - 'CREATE OR REPLACE VIEW %1$I AS - SELECT %2s - FROM %3$s t', - local_name, + 'CREATE OR REPLACE VIEW %1$I.%2$I AS + SELECT %3s + FROM %4$s t', + server_internal, local_name, array_to_string(carto_columns_expression || rest_of_cols, ','), src_table ); EXCEPTION WHEN OTHERS THEN - IF EXISTS (SELECT to_regclass(local_name)) THEN - RAISE EXCEPTION 'Could not import table "%" as "%" already exists: %', remote_table, local_name, SQLERRM; + IF EXISTS (SELECT to_regclass(format('%1$I.%2$I', server_internal, local_name))) THEN + RAISE EXCEPTION 'Could not import table "%" as "%.%" already exists', remote_table, server_internal, local_name; ELSE RAISE EXCEPTION 'Could not import table "%" as "%": %', remote_table, local_name, SQLERRM; END IF; END; - - EXECUTE format('ALTER VIEW %1$I OWNER TO %I', - local_name, - cartodb.__CDB_FS_Generate_Server_Role_Name(server_internal)); END $$ LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; diff --git a/test/CDB_FederatedServerTables.sql b/test/CDB_FederatedServerTables.sql index 328a05b..fdd1d7a 100644 --- a/test/CDB_FederatedServerTables.sql +++ b/test/CDB_FederatedServerTables.sql @@ -71,7 +71,8 @@ SELECT 'R1', cartodb.CDB_Federated_Table_Register( geom_column => 'geom' ); -SELECT 'S1', cartodb_id, ST_AsText(the_geom), another_field FROM remote_geom; +SELECT 'S1', cartodb_id, ST_AsText(the_geom), another_field FROM cdb_fs_loopback.remote_geom; + Select * FROM CDB_Federated_Server_List_Remote_Tables( server => 'loopback', remote_schema => 'remote_schema' @@ -88,7 +89,7 @@ SELECT 'R2', cartodb.CDB_Federated_Table_Register( local_name => 'myFullTable' ); -SELECT 'S2', cartodb_id, ST_AsText(the_geom), another_field FROM "myFullTable"; +SELECT 'S2', cartodb_id, ST_AsText(the_geom), another_field FROM cdb_fs_loopback."myFullTable"; Select * FROM CDB_Federated_Server_List_Remote_Tables( server => 'loopback', remote_schema => 'remote_schema' @@ -107,9 +108,9 @@ SELECT 'R3', cartodb.CDB_Federated_Table_Register( ); -- The old view should dissapear -SELECT 'S3_old', cartodb_id, another_field FROM "myFullTable"; +SELECT 'S3_old', cartodb_id, another_field FROM cdb_fs_loopback."myFullTable"; -- And the new appear -SELECT 'S3_new', cartodb_id, another_field FROM different_name; +SELECT 'S3_new', cartodb_id, another_field FROM cdb_fs_loopback.different_name; \echo '## Unregistering works' -- Deregistering the first table @@ -255,7 +256,7 @@ SELECT cartodb.CDB_Federated_Table_Unregister( -- =================================================================== \echo '## Target conflict is handled nicely: Table' -CREATE TABLE localtable (a integer); +CREATE TABLE cdb_fs_loopback.localtable (a integer); SELECT cartodb.CDB_Federated_Table_Register( server => 'loopback', remote_schema => 'remote_schema', @@ -265,7 +266,7 @@ SELECT cartodb.CDB_Federated_Table_Register( local_name => 'localtable'); \echo '## Target conflict is handled nicely: View' -CREATE VIEW localtable2 AS Select * from localtable; +CREATE VIEW cdb_fs_loopback.localtable2 AS Select * from cdb_fs_loopback.localtable; SELECT cartodb.CDB_Federated_Table_Register( server => 'loopback', remote_schema => 'remote_schema', @@ -274,8 +275,8 @@ SELECT cartodb.CDB_Federated_Table_Register( geom_column => 'geom', local_name => 'localtable2'); -DROP VIEW localtable2; -DROP TABLE localtable; +DROP VIEW cdb_fs_loopback.localtable2; +DROP TABLE cdb_fs_loopback.localtable; -- =================================================================== -- Test permissions @@ -292,6 +293,9 @@ SELECT cartodb.CDB_Federated_Table_Register( geom_column => 'geom', local_name => 'localtable'); +\echo '## Normal users can not write in the import schema' +CREATE TABLE cdb_fs_loopback.localtable (a integer); + \echo '## Listing remote tables does not work without permissions' Select * FROM cartodb.CDB_Federated_Server_List_Remote_Tables(server => 'loopback', remote_schema => 'remote_schema'); @@ -311,7 +315,7 @@ SELECT cartodb.CDB_Federated_Table_Register( Select * FROM cartodb.CDB_Federated_Server_List_Remote_Tables(server => 'loopback', remote_schema => 'remote_schema'); \echo '## Selecting from a registered table with granted permissions works' -Select cartodb_id, ST_AsText(the_geom) from localtable; +Select cartodb_id, ST_AsText(the_geom) from cdb_fs_loopback.localtable; \echo '## Selecting from a registered table without permissions does not work' \c contrib_regression cdb_fs_tester2 @@ -329,7 +333,7 @@ EXCEPTION RETURN FALSE; END $$ LANGUAGE 'plpgsql'; -Select catch_permission_error($$SELECT cartodb_id, ST_AsText(the_geom) from localtable$$); +Select catch_permission_error($$SELECT cartodb_id, ST_AsText(the_geom) from cdb_fs_loopback.localtable$$); DROP FUNCTION catch_permission_error(text); \echo '## Deleting a registered table without permissions does not work' @@ -347,7 +351,7 @@ SELECT cartodb.CDB_Federated_Server_Grant_Access(server := 'loopback', db_role : SELECT cartodb.CDB_Federated_Server_Grant_Access(server := 'loopback', db_role := 'cdb_fs_tester2'::name); \c contrib_regression cdb_fs_tester2 Select * FROM cartodb.CDB_Federated_Server_List_Remote_Tables(server => 'loopback', remote_schema => 'remote_schema'); -Select cartodb_id, ST_AsText(the_geom) from localtable; +Select cartodb_id, ST_AsText(the_geom) from cdb_fs_loopback.localtable; \echo '## A different user can unregister a table' SELECT cartodb.CDB_Federated_Table_Unregister( diff --git a/test/CDB_FederatedServerTables_expect b/test/CDB_FederatedServerTables_expect index b8f9abc..f8dd882 100644 --- a/test/CDB_FederatedServerTables_expect +++ b/test/CDB_FederatedServerTables_expect @@ -3,24 +3,24 @@ C1| R1| S1|1|POINT(1 1)|patata S1|2|POINT(2 2)|patata2 -t|remote_geom|public.remote_geom|id|geom|geom|[{"Name" : "another_field", "Type" : "text"}, {"Name" : "geom", "Type" : "GEOMETRY,4326"}, {"Name" : "id", "Type" : "integer"}] +t|remote_geom|cdb_fs_loopback.remote_geom|id|geom|geom|[{"Name" : "another_field", "Type" : "text"}, {"Name" : "geom", "Type" : "GEOMETRY,4326"}, {"Name" : "id", "Type" : "integer"}] f|remote_geom2|||||[{"Name" : "another_field", "Type" : "text"}, {"Name" : "geom", "Type" : "GEOMETRY,4326"}, {"Name" : "geom_mercator", "Type" : "GEOMETRY,3857"}, {"Name" : "id", "Type" : "bigint"}] f|remote_other|||||[{"Name" : "field", "Type" : "text"}, {"Name" : "field2", "Type" : "text"}, {"Name" : "id", "Type" : "bigint"}] ## Registering another existing table works R2| S2|3|POINT(3 3)|patata -t|remote_geom|public.remote_geom|id|geom|geom|[{"Name" : "another_field", "Type" : "text"}, {"Name" : "geom", "Type" : "GEOMETRY,4326"}, {"Name" : "id", "Type" : "integer"}] -t|remote_geom2|public."myFullTable"|id|geom|geom_mercator|[{"Name" : "another_field", "Type" : "text"}, {"Name" : "geom", "Type" : "GEOMETRY,4326"}, {"Name" : "geom_mercator", "Type" : "GEOMETRY,3857"}, {"Name" : "id", "Type" : "bigint"}] +t|remote_geom|cdb_fs_loopback.remote_geom|id|geom|geom|[{"Name" : "another_field", "Type" : "text"}, {"Name" : "geom", "Type" : "GEOMETRY,4326"}, {"Name" : "id", "Type" : "integer"}] +t|remote_geom2|cdb_fs_loopback."myFullTable"|id|geom|geom_mercator|[{"Name" : "another_field", "Type" : "text"}, {"Name" : "geom", "Type" : "GEOMETRY,4326"}, {"Name" : "geom_mercator", "Type" : "GEOMETRY,3857"}, {"Name" : "id", "Type" : "bigint"}] f|remote_other|||||[{"Name" : "field", "Type" : "text"}, {"Name" : "field2", "Type" : "text"}, {"Name" : "id", "Type" : "bigint"}] ## Re-registering a table works R3| -ERROR: relation "myFullTable" does not exist at character 49 +ERROR: relation "cdb_fs_loopback.myFullTable" does not exist at character 49 S3_new|3|patata ## Unregistering works U1| ERROR: relation "remote_geom" does not exist at character 71 f|remote_geom|||||[{"Name" : "another_field", "Type" : "text"}, {"Name" : "geom", "Type" : "GEOMETRY,4326"}, {"Name" : "id", "Type" : "integer"}] -t|remote_geom2|public.different_name|id|geom_mercator|geom|[{"Name" : "another_field", "Type" : "text"}, {"Name" : "geom", "Type" : "GEOMETRY,4326"}, {"Name" : "geom_mercator", "Type" : "GEOMETRY,3857"}, {"Name" : "id", "Type" : "bigint"}] +t|remote_geom2|cdb_fs_loopback.different_name|id|geom_mercator|geom|[{"Name" : "another_field", "Type" : "text"}, {"Name" : "geom", "Type" : "GEOMETRY,4326"}, {"Name" : "geom_mercator", "Type" : "GEOMETRY,3857"}, {"Name" : "id", "Type" : "bigint"}] f|remote_other|||||[{"Name" : "field", "Type" : "text"}, {"Name" : "field2", "Type" : "text"}, {"Name" : "id", "Type" : "bigint"}] ## Registering a table: Invalid server fails ERROR: Server "Does not exist" does not exist @@ -50,15 +50,17 @@ ERROR: non geometry column "Does not exists" ## Target conflict is handled nicely: Table CREATE TABLE -ERROR: Could not import table "remote_geom" as "localtable" already exists: "localtable" is not a view +ERROR: Could not import table "remote_geom" as "cdb_fs_loopback.localtable" already exists ## Target conflict is handled nicely: View CREATE VIEW -ERROR: Could not import table "remote_geom" as "localtable2" already exists: cannot change name of view column "a" to "cartodb_id" +ERROR: Could not import table "remote_geom" as "cdb_fs_loopback.localtable2" already exists DROP VIEW DROP TABLE ## Registering tables does not work without permissions You are now connected to database "contrib_regression" as user "cdb_fs_tester". ERROR: Not enough permissions to access the server "loopback" +## Normal users can not write in the import schema +ERROR: permission denied for schema cdb_fs_loopback at character 14 ## Listing remote tables does not work without permissions ERROR: Not enough permissions to access the server "loopback" ## Registering tables works with granted permissions @@ -67,8 +69,8 @@ You are now connected to database "contrib_regression" as user "postgres". You are now connected to database "contrib_regression" as user "cdb_fs_tester". ## Listing remote tables works with granted permissions -t|remote_geom|public.localtable|id|geom|geom|[{"Name" : "another_field", "Type" : "text"}, {"Name" : "geom", "Type" : "GEOMETRY,4326"}, {"Name" : "id", "Type" : "integer"}] -t|remote_geom2|public.different_name|id|geom_mercator|geom|[{"Name" : "another_field", "Type" : "text"}, {"Name" : "geom", "Type" : "GEOMETRY,4326"}, {"Name" : "geom_mercator", "Type" : "GEOMETRY,3857"}, {"Name" : "id", "Type" : "bigint"}] +t|remote_geom|cdb_fs_loopback.localtable|id|geom|geom|[{"Name" : "another_field", "Type" : "text"}, {"Name" : "geom", "Type" : "GEOMETRY,4326"}, {"Name" : "id", "Type" : "integer"}] +t|remote_geom2|cdb_fs_loopback.different_name|id|geom_mercator|geom|[{"Name" : "another_field", "Type" : "text"}, {"Name" : "geom", "Type" : "GEOMETRY,4326"}, {"Name" : "geom_mercator", "Type" : "GEOMETRY,3857"}, {"Name" : "id", "Type" : "bigint"}] f|remote_other|||||[{"Name" : "field", "Type" : "text"}, {"Name" : "field2", "Type" : "text"}, {"Name" : "id", "Type" : "bigint"}] ## Selecting from a registered table with granted permissions works 1|POINT(1 1) @@ -86,16 +88,16 @@ ERROR: You do not have rights to grant access on "loopback" You are now connected to database "contrib_regression" as user "postgres". You are now connected to database "contrib_regression" as user "cdb_fs_tester2". -t|remote_geom|public.localtable|id|geom|geom|[{"Name" : "another_field", "Type" : "text"}, {"Name" : "geom", "Type" : "GEOMETRY,4326"}, {"Name" : "id", "Type" : "integer"}] -t|remote_geom2|public.different_name|id|geom_mercator|geom|[{"Name" : "another_field", "Type" : "text"}, {"Name" : "geom", "Type" : "GEOMETRY,4326"}, {"Name" : "geom_mercator", "Type" : "GEOMETRY,3857"}, {"Name" : "id", "Type" : "bigint"}] +t|remote_geom|cdb_fs_loopback.localtable|id|geom|geom|[{"Name" : "another_field", "Type" : "text"}, {"Name" : "geom", "Type" : "GEOMETRY,4326"}, {"Name" : "id", "Type" : "integer"}] +t|remote_geom2|cdb_fs_loopback.different_name|id|geom_mercator|geom|[{"Name" : "another_field", "Type" : "text"}, {"Name" : "geom", "Type" : "GEOMETRY,4326"}, {"Name" : "geom_mercator", "Type" : "GEOMETRY,3857"}, {"Name" : "id", "Type" : "bigint"}] f|remote_other|||||[{"Name" : "field", "Type" : "text"}, {"Name" : "field2", "Type" : "text"}, {"Name" : "id", "Type" : "bigint"}] 1|POINT(1 1) 2|POINT(2 2) ## A different user can unregister a table -NOTICE: drop cascades to view localtable +NOTICE: drop cascades to view cdb_fs_loopback.localtable f|remote_geom|||||[{"Name" : "another_field", "Type" : "text"}, {"Name" : "geom", "Type" : "GEOMETRY,4326"}, {"Name" : "id", "Type" : "integer"}] -t|remote_geom2|public.different_name|id|geom_mercator|geom|[{"Name" : "another_field", "Type" : "text"}, {"Name" : "geom", "Type" : "GEOMETRY,4326"}, {"Name" : "geom_mercator", "Type" : "GEOMETRY,3857"}, {"Name" : "id", "Type" : "bigint"}] +t|remote_geom2|cdb_fs_loopback.different_name|id|geom_mercator|geom|[{"Name" : "another_field", "Type" : "text"}, {"Name" : "geom", "Type" : "GEOMETRY,4326"}, {"Name" : "geom_mercator", "Type" : "GEOMETRY,3857"}, {"Name" : "id", "Type" : "bigint"}] f|remote_other|||||[{"Name" : "field", "Type" : "text"}, {"Name" : "field2", "Type" : "text"}, {"Name" : "id", "Type" : "bigint"}] ## Only the owner can revoke permissions over the server ERROR: You do not have rights to revoke access on "loopback"