diff --git a/scripts-available/CDB_FederatedServer.sql b/scripts-available/CDB_FederatedServer.sql index 3c822b7..1d55198 100644 --- a/scripts-available/CDB_FederatedServer.sql +++ b/scripts-available/CDB_FederatedServer.sql @@ -98,12 +98,17 @@ DECLARE schema_name text := @extschema@.__CDB_FS_Generate_Schema_Name(internal_server_name, schema_name); role_name text := @extschema@.__CDB_FS_Generate_Server_Role_Name(internal_server_name); BEGIN + -- By changing the local role to the owner of the server we have an + -- easy way to check for permissions and keep all objects under the same owner + BEGIN + EXECUTE 'SET LOCAL ROLE ' || quote_ident(role_name); + EXCEPTION WHEN OTHERS THEN + RAISE EXCEPTION 'Not enough permissions to access the server "%"', + @extschema@.__CDB_FS_Extract_Server_Name(internal_server_name); + END; + IF NOT EXISTS (SELECT oid FROM pg_namespace WHERE nspname = schema_name) THEN - BEGIN - EXECUTE 'CREATE SCHEMA IF NOT EXISTS ' || quote_ident(schema_name) || ' AUTHORIZATION ' || quote_ident(role_name); - EXCEPTION WHEN OTHERS THEN - RAISE EXCEPTION 'TODO: This needs a better error handling after reviewing permissions: %', SQLERRM; - END; + EXECUTE 'CREATE SCHEMA ' || quote_ident(schema_name) || ' AUTHORIZATION ' || quote_ident(role_name); END IF; RETURN schema_name; END @@ -219,13 +224,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); - EXECUTE FORMAT('CREATE ROLE %I NOLOGIN', role_name); + -- 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 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); EXECUTE FORMAT('ALTER SERVER %I OWNER TO %I', server_internal, role_name); - -- NOTE: we use a PUBLIC user mapping but control access to the SERVER - -- so that we don't need to create a mapping for every user nor store credentials elsewhere - EXECUTE FORMAT ('CREATE USER MAPPING FOR public SERVER %I', server_internal); + EXECUTE FORMAT ('CREATE USER MAPPING FOR %I SERVER %I', role_name, server_internal); EXCEPTION WHEN OTHERS THEN RAISE EXCEPTION 'Could not create server %: %', server, SQLERRM USING HINT = 'Please clean the left over objects'; @@ -251,12 +262,12 @@ BEGIN LOOP IF NOT EXISTS ( WITH a AS ( - SELECT split_part(unnest(umoptions), '=', 1) as options from pg_user_mappings WHERE srvname = server_internal AND usename = 'public' + SELECT split_part(unnest(umoptions), '=', 1) as options from pg_user_mappings WHERE srvname = server_internal AND usename = role_name ) SELECT * from a where options = option.key) THEN - EXECUTE FORMAT('ALTER USER MAPPING FOR PUBLIC SERVER %I OPTIONS (ADD %I %L)', server_internal, option.key, option.value); + EXECUTE FORMAT('ALTER USER MAPPING FOR %I SERVER %I OPTIONS (ADD %I %L)', role_name, server_internal, option.key, option.value); ELSE - EXECUTE FORMAT('ALTER USER MAPPING FOR PUBLIC SERVER %I OPTIONS (SET %I %L)', server_internal, option.key, option.value); + EXECUTE FORMAT('ALTER USER MAPPING FOR %I SERVER %I OPTIONS (SET %I %L)', role_name, server_internal, option.key, option.value); END IF; END LOOP; END @@ -275,7 +286,7 @@ DECLARE BEGIN SET client_min_messages = ERROR; BEGIN - EXECUTE FORMAT ('DROP USER MAPPING FOR public SERVER %I', server_internal); + EXECUTE FORMAT ('DROP USER MAPPING FOR %I SERVER %I', role_name, server_internal); EXECUTE FORMAT ('DROP OWNED BY %I CASCADE', role_name); EXECUTE FORMAT ('DROP ROLE %I', role_name); EXCEPTION WHEN OTHERS THEN diff --git a/scripts-available/CDB_FederatedServerListRemote.sql b/scripts-available/CDB_FederatedServerListRemote.sql index 0b00f24..09fb678 100644 --- a/scripts-available/CDB_FederatedServerListRemote.sql +++ b/scripts-available/CDB_FederatedServerListRemote.sql @@ -34,11 +34,16 @@ BEGIN END IF; -- Return the result we're interested in. Exclude toast and temp schemas - RETURN QUERY EXECUTE format(' - SELECT schema_name::name AS remote_schema FROM %I.%I - WHERE schema_name NOT LIKE %s - ORDER BY remote_schema - ', local_schema, remote_table, '''pg_%'''); + BEGIN + RETURN QUERY EXECUTE format(' + SELECT schema_name::name AS remote_schema FROM %I.%I + WHERE schema_name NOT LIKE %s + ORDER BY remote_schema + ', local_schema, remote_table, '''pg_%'''); + EXCEPTION WHEN OTHERS THEN + RAISE EXCEPTION 'Not enough permissions to access the server "%": %', + @extschema@.__CDB_FS_Extract_Server_Name(server_internal), SQLERRM; + END; END $$ LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; diff --git a/test/CDB_FederatedServerListRemote.sql b/test/CDB_FederatedServerListRemote.sql index 189e032..f72fb42 100644 --- a/test/CDB_FederatedServerListRemote.sql +++ b/test/CDB_FederatedServerListRemote.sql @@ -6,9 +6,24 @@ SET client_min_messages TO error; \set VERBOSITY terse SET SESSION AUTHORIZATION postgres; CREATE EXTENSION postgres_fdw; -CREATE ROLE cdb_fs_tester SUPERUSER LOGIN PASSWORD 'cdb_fs_passwd'; + +-- We create a username following the same steps as organization members +CREATE ROLE cdb_fs_tester LOGIN PASSWORD 'cdb_fs_passwd'; +GRANT CONNECT ON DATABASE contrib_regression TO cdb_fs_tester; +CREATE SCHEMA cdb_fs_tester AUTHORIZATION cdb_fs_tester; +SELECT cartodb.CDB_Organization_Create_Member('cdb_fs_tester'); +ALTER ROLE cdb_fs_tester SET search_path TO cdb_fs_tester,cartodb,public; + +CREATE ROLE cdb_fs_tester2 LOGIN PASSWORD 'cdb_fs_passwd2'; +GRANT CONNECT ON DATABASE contrib_regression TO cdb_fs_tester2; +CREATE SCHEMA cdb_fs_tester2 AUTHORIZATION cdb_fs_tester2; +SELECT cartodb.CDB_Organization_Create_Member('cdb_fs_tester2'); +ALTER ROLE cdb_fs_tester2 SET search_path TO cdb_fs_tester2,cartodb,public; + +-- Create database to be used as remote CREATE DATABASE cdb_fs_tester OWNER cdb_fs_tester; + SELECT 'C1', cartodb.CDB_Federated_Server_Register_PG(server := 'loopback'::text, config := '{ "server": { "host": "localhost", @@ -80,33 +95,139 @@ SET client_min_messages TO notice; -- =================================================================== --- Test the listing functions +-- Test listing remote schemas -- =================================================================== -\echo 'Test listing of remote schemas (sunny day)' +\echo '## Test listing of remote schemas without permissions before the first instantiation (rainy day)' +\c contrib_regression cdb_fs_tester +SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Schemas(server => 'loopback'); +\c contrib_regression postgres + +\echo '## Test listing of remote schemas (sunny day)' SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Schemas(server => 'loopback'); -\echo 'Test listing of remote schemas (rainy day): Server does not exist' +\echo '## Test listing of remote schemas without permissions after the first instantiation (rainy day)' +\c contrib_regression cdb_fs_tester +SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Schemas(server => 'loopback'); +\c contrib_regression postgres + +\echo '## Test listing of remote schemas with permissions (sunny day)' +SELECT cartodb.CDB_Federated_Server_Grant_Access(server := 'loopback', usernames := ARRAY['cdb_fs_tester']); +\c contrib_regression cdb_fs_tester +SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Schemas(server => 'loopback'); +\c contrib_regression postgres + +\echo '## Test listing of remote schemas without permissions after revoking access (rainy day)' +SELECT cartodb.CDB_Federated_Server_Revoke_Access(server := 'loopback', usernames := ARRAY['cdb_fs_tester']); +\c contrib_regression cdb_fs_tester +SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Schemas(server => 'loopback'); +\c contrib_regression postgres + +\echo '## Test listing of remote schemas (rainy day): Server does not exist' SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Schemas(server => 'Does Not Exist'); -\echo 'Test listing of remote tables (sunny day)' + +-- =================================================================== +-- Test listing remote tables +-- =================================================================== + +\echo '## Test listing of remote tables without permissions before the first instantiation (rainy day)' +\c contrib_regression cdb_fs_tester +SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Tables(server => 'loopback', remote_schema => 'S 1'); +\c contrib_regression postgres + +\echo '## Test listing of remote tables (sunny day)' SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Tables(server => 'loopback', remote_schema => 'S 1'); -\echo 'Test listing of remote tables (rainy day): Server does not exist' +\echo '## Test listing of remote tables without permissions after the first instantiation (rainy day)' +\c contrib_regression cdb_fs_tester +SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Tables(server => 'loopback', remote_schema => 'S 1'); +\c contrib_regression postgres + +\echo '## Test listing of remote tables with permissions (sunny day)' +SELECT cartodb.CDB_Federated_Server_Grant_Access(server := 'loopback', usernames := ARRAY['cdb_fs_tester']); +\c contrib_regression cdb_fs_tester +SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Tables(server => 'loopback', remote_schema => 'S 1'); +\c contrib_regression postgres + +\echo '## Test listing of remote tables without permissions after revoking access (rainy day)' +SELECT cartodb.CDB_Federated_Server_Revoke_Access(server := 'loopback', usernames := ARRAY['cdb_fs_tester']); +\c contrib_regression cdb_fs_tester +SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Tables(server => 'loopback', remote_schema => 'S 1'); +\c contrib_regression postgres + +\echo '## Test listing of remote tables (rainy day): Server does not exist' SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Tables(server => 'Does Not Exist', remote_schema => 'S 1'); -\echo 'Test listing of remote tables (rainy day): Remote schema does not exist' + +\echo '## Test listing of remote tables (rainy day): Remote schema does not exist' SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Tables(server => 'loopback', remote_schema => 'Does Not Exist'); -\echo 'Test listing of remote columns (sunny day)' + +-- =================================================================== +-- Test listing remote columns +-- =================================================================== + +\echo '## Test listing of remote columns without permissions before the first instantiation (rainy day)' +\c contrib_regression cdb_fs_tester +SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Columns(server => 'loopback', remote_schema => 'S 1', remote_table => 'T 1'); +\c contrib_regression postgres + +\echo '## Test listing of remote columns (sunny day)' SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Columns(server => 'loopback', remote_schema => 'S 1', remote_table => 'T 1'); -\echo 'Test listing of remote columns (rainy day): Server does not exist' +\echo '## Test listing of remote columns without permissions after the first instantiation (rainy day)' +\c contrib_regression cdb_fs_tester +SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Columns(server => 'loopback', remote_schema => 'S 1', remote_table => 'T 1'); +\c contrib_regression postgres + +\echo '## Test listing of remote columns with permissions (sunny day)' +SELECT cartodb.CDB_Federated_Server_Grant_Access(server := 'loopback', usernames := ARRAY['cdb_fs_tester']); +\c contrib_regression cdb_fs_tester +SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Columns(server => 'loopback', remote_schema => 'S 1', remote_table => 'T 1'); +\c contrib_regression postgres + +\echo '## Test listing of remote columns without permissions after revoking access (rainy day)' +SELECT cartodb.CDB_Federated_Server_Revoke_Access(server := 'loopback', usernames := ARRAY['cdb_fs_tester']); +\c contrib_regression cdb_fs_tester +SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Columns(server => 'loopback', remote_schema => 'S 1', remote_table => 'T 1'); +\c contrib_regression postgres + +\echo '## Test listing of remote columns (rainy day): Server does not exist' SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Columns(server => 'Does Not Exist', remote_schema => 'S 1', remote_table => 'T 1'); -\echo 'Test listing of remote columns (rainy day): Remote schema does not exist' + +\echo '## Test listing of remote columns (rainy day): Remote schema does not exist' SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Columns(server => 'loopback', remote_schema => 'Does Not Exist', remote_table => 'T 1'); -\echo 'Test listing of remote columns (rainy day): Remote table does not exist' + +\echo '## Test listing of remote columns (rainy day): Remote table does not exist' SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Columns(server => 'loopback', remote_schema => 'S 1', remote_table => 'Does Not Exist'); +-- =================================================================== +-- Test that using a different user to list tables and dropping it +-- does not break the server: We use loopback2 as it's in a clean state +-- =================================================================== + + +\echo '## Test listing of remote objects with permissions (sunny day)' +SELECT cartodb.CDB_Federated_Server_Grant_Access(server := 'loopback2', usernames := ARRAY['cdb_fs_tester2']); +\c contrib_regression cdb_fs_tester2 +SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Schemas(server => 'loopback2'); +SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Tables(server => 'loopback2', remote_schema => 'S 1'); +SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Columns(server => 'loopback2', remote_schema => 'S 1', remote_table => 'T 1'); + +\c contrib_regression postgres +\echo '## Test that dropping the granted user works fine (sunny day)' +DROP SCHEMA cdb_fs_tester2 CASCADE; +REVOKE CONNECT ON DATABASE contrib_regression FROM cdb_fs_tester2; +DROP ROLE cdb_fs_tester2; + +\echo '## Test listing of remote objects with other user still works (sunny day)' +SELECT cartodb.CDB_Federated_Server_Grant_Access(server := 'loopback2', usernames := ARRAY['cdb_fs_tester']); +\c contrib_regression cdb_fs_tester +SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Schemas(server => 'loopback2'); +SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Tables(server => 'loopback2', remote_schema => 'S 1'); +SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Columns(server => 'loopback2', remote_schema => 'S 1', remote_table => 'T 1'); + + -- =================================================================== -- Cleanup 1 -- =================================================================== @@ -150,9 +271,9 @@ SET client_min_messages TO notice; -- Test the listing functions -- =================================================================== -\echo 'Test listing of remote geometry columns (sunny day)' +\echo '## Test listing of remote geometry columns (sunny day)' SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Columns(server => 'loopback', remote_schema => 'S 1', remote_table => 'T 5'); -\echo 'Test listing of remote geometry columns (sunny day) - Rerun' +\echo '## Test listing of remote geometry columns (sunny day) - Rerun' -- Rerun should be ok SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Columns(server => 'loopback', remote_schema => 'S 1', remote_table => 'T 5'); @@ -175,6 +296,12 @@ SELECT 'D1', cartodb.CDB_Federated_Server_Unregister(server := 'loopback'::text) SELECT 'D2', cartodb.CDB_Federated_Server_Unregister(server := 'loopback2'::text); DROP DATABASE cdb_fs_tester; + +-- Drop role +DROP SCHEMA cdb_fs_tester CASCADE; +REVOKE CONNECT ON DATABASE contrib_regression FROM cdb_fs_tester; DROP ROLE cdb_fs_tester; + DROP EXTENSION postgres_fdw; + \set QUIET off diff --git a/test/CDB_FederatedServerListRemote_expect b/test/CDB_FederatedServerListRemote_expect index 6a2dc46..3228992 100644 --- a/test/CDB_FederatedServerListRemote_expect +++ b/test/CDB_FederatedServerListRemote_expect @@ -1,20 +1,67 @@ + + C1| C2| -Test listing of remote schemas (sunny day) +## Test listing of remote schemas without permissions before the first instantiation (rainy day) +You are now connected to database "contrib_regression" as user "cdb_fs_tester". +ERROR: Not enough permissions to access the server "loopback" +You are now connected to database "contrib_regression" as user "postgres". +## Test listing of remote schemas (sunny day) S 1 information_schema public -Test listing of remote schemas (rainy day): Server does not exist +## Test listing of remote schemas without permissions after the first instantiation (rainy day) +You are now connected to database "contrib_regression" as user "cdb_fs_tester". +ERROR: Not enough permissions to access the server "loopback" +You are now connected to database "contrib_regression" as user "postgres". +## Test listing of remote schemas with permissions (sunny day) + +You are now connected to database "contrib_regression" as user "cdb_fs_tester". +S 1 +information_schema +public +You are now connected to database "contrib_regression" as user "postgres". +## Test listing of remote schemas without permissions after revoking access (rainy day) + +You are now connected to database "contrib_regression" as user "cdb_fs_tester". +ERROR: Not enough permissions to access the server "loopback" +You are now connected to database "contrib_regression" as user "postgres". +## Test listing of remote schemas (rainy day): Server does not exist ERROR: Server "Does Not Exist" does not exist -Test listing of remote tables (sunny day) +## Test listing of remote tables without permissions before the first instantiation (rainy day) +You are now connected to database "contrib_regression" as user "cdb_fs_tester". +ERROR: Not enough permissions to access the server "loopback" +You are now connected to database "contrib_regression" as user "postgres". +## Test listing of remote tables (sunny day) T 1 T 2 T 3 T 4 -Test listing of remote tables (rainy day): Server does not exist +## Test listing of remote tables without permissions after the first instantiation (rainy day) +You are now connected to database "contrib_regression" as user "cdb_fs_tester". +ERROR: Not enough permissions to access the server "loopback" +You are now connected to database "contrib_regression" as user "postgres". +## Test listing of remote tables with permissions (sunny day) + +You are now connected to database "contrib_regression" as user "cdb_fs_tester". +T 1 +T 2 +T 3 +T 4 +You are now connected to database "contrib_regression" as user "postgres". +## Test listing of remote tables without permissions after revoking access (rainy day) + +You are now connected to database "contrib_regression" as user "cdb_fs_tester". +ERROR: Not enough permissions to access the server "loopback" +You are now connected to database "contrib_regression" as user "postgres". +## Test listing of remote tables (rainy day): Server does not exist ERROR: Server "Does Not Exist" does not exist -Test listing of remote tables (rainy day): Remote schema does not exist -Test listing of remote columns (sunny day) +## Test listing of remote tables (rainy day): Remote schema does not exist +## Test listing of remote columns without permissions before the first instantiation (rainy day) +You are now connected to database "contrib_regression" as user "cdb_fs_tester". +ERROR: Not enough permissions to access the server "loopback" +You are now connected to database "contrib_regression" as user "postgres". +## Test listing of remote columns (sunny day) NOTICE: Could not find Postgis installation in the remote "public" schema C 1|integer c2|integer @@ -24,16 +71,81 @@ c5|timestamp without time zone c6|character varying c7|character c8|USER-DEFINED -Test listing of remote columns (rainy day): Server does not exist +## Test listing of remote columns without permissions after the first instantiation (rainy day) +You are now connected to database "contrib_regression" as user "cdb_fs_tester". +ERROR: Not enough permissions to access the server "loopback" +You are now connected to database "contrib_regression" as user "postgres". +## Test listing of remote columns with permissions (sunny day) + +You are now connected to database "contrib_regression" as user "cdb_fs_tester". +NOTICE: Could not find Postgis installation in the remote "public" schema +C 1|integer +c2|integer +c3|text +c4|timestamp with time zone +c5|timestamp without time zone +c6|character varying +c7|character +c8|USER-DEFINED +You are now connected to database "contrib_regression" as user "postgres". +## Test listing of remote columns without permissions after revoking access (rainy day) + +You are now connected to database "contrib_regression" as user "cdb_fs_tester". +ERROR: Not enough permissions to access the server "loopback" +You are now connected to database "contrib_regression" as user "postgres". +## Test listing of remote columns (rainy day): Server does not exist ERROR: Server "Does Not Exist" does not exist -Test listing of remote columns (rainy day): Remote schema does not exist -Test listing of remote columns (rainy day): Remote table does not exist -Test listing of remote geometry columns (sunny day) +## Test listing of remote columns (rainy day): Remote schema does not exist +## Test listing of remote columns (rainy day): Remote table does not exist +## Test listing of remote objects with permissions (sunny day) + +You are now connected to database "contrib_regression" as user "cdb_fs_tester2". +S 1 +information_schema +public +T 1 +T 2 +T 3 +T 4 +NOTICE: Could not find Postgis installation in the remote "public" schema +C 1|integer +c2|integer +c3|text +c4|timestamp with time zone +c5|timestamp without time zone +c6|character varying +c7|character +c8|USER-DEFINED +You are now connected to database "contrib_regression" as user "postgres". +## Test that dropping the granted user works fine (sunny day) +DROP SCHEMA +REVOKE +DROP ROLE +## Test listing of remote objects with other user still works (sunny day) + +You are now connected to database "contrib_regression" as user "cdb_fs_tester". +S 1 +information_schema +public +T 1 +T 2 +T 3 +T 4 +NOTICE: Could not find Postgis installation in the remote "public" schema +C 1|integer +c2|integer +c3|text +c4|timestamp with time zone +c5|timestamp without time zone +c6|character varying +c7|character +c8|USER-DEFINED +## Test listing of remote geometry columns (sunny day) geo_nosrid|GEOMETRY,0 geog|Geometry,0 geom|GEOMETRY,4326 geom_wm|GEOMETRY,3857 -Test listing of remote geometry columns (sunny day) - Rerun +## Test listing of remote geometry columns (sunny day) - Rerun geo_nosrid|GEOMETRY,0 geog|Geometry,0 geom|GEOMETRY,4326