diff --git a/scripts-available/CDB_FederatedServer.sql b/scripts-available/CDB_FederatedServer.sql index 0e113a7..af96f9f 100644 --- a/scripts-available/CDB_FederatedServer.sql +++ b/scripts-available/CDB_FederatedServer.sql @@ -347,23 +347,17 @@ LANGUAGE PLPGSQL IMMUTABLE PARALLEL SAFE; -- -- Grant access to a server -- -CREATE OR REPLACE FUNCTION @extschema@.CDB_Federated_Server_Grant_Access(server TEXT, usernames text[]) +CREATE OR REPLACE FUNCTION @extschema@.CDB_Federated_Server_Grant_Access(server TEXT, db_role NAME) RETURNS void AS $$ DECLARE server_internal text := @extschema@.__CDB_FS_Generate_Server_Name(input_name := server, check_existence := true); server_role_name name := @extschema@.__CDB_FS_Generate_Server_Role_Name(server_internal); - user_role TEXT; - username TEXT; BEGIN - FOREACH username IN ARRAY usernames - LOOP - user_role := @extschema@._CDB_User_RoleFromUsername(username); - IF (user_role IS NULL) THEN - RAISE EXCEPTION 'User role "%" does not exists', username; - END IF; - EXECUTE format('GRANT %I TO %I', server_role_name, user_role); - END loop; + IF (db_role IS NULL) THEN + RAISE EXCEPTION 'User role "%" cannot be NULL', username; + END IF; + EXECUTE format('GRANT %I TO %I', server_role_name, db_role); END $$ LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; @@ -371,23 +365,17 @@ LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; -- -- Revoke access to a server -- -CREATE OR REPLACE FUNCTION @extschema@.CDB_Federated_Server_Revoke_Access(server TEXT, usernames text[]) +CREATE OR REPLACE FUNCTION @extschema@.CDB_Federated_Server_Revoke_Access(server TEXT, db_role NAME) RETURNS void AS $$ DECLARE server_internal text := @extschema@.__CDB_FS_Generate_Server_Name(input_name := server, check_existence := true); server_role_name name := @extschema@.__CDB_FS_Generate_Server_Role_Name(server_internal); - user_role TEXT; - username TEXT; BEGIN - FOREACH username IN ARRAY usernames - LOOP - user_role := @extschema@._CDB_User_RoleFromUsername(username); - IF (user_role IS NULL) THEN - RAISE EXCEPTION 'User role "%" does not exists', username; - END IF; - EXECUTE format('REVOKE %I FROM %I', server_role_name, user_role); - END loop; + IF (db_role IS NULL) THEN + RAISE EXCEPTION 'User role "%" cannot be NULL', username; + END IF; + EXECUTE format('REVOKE %I FROM %I', server_role_name, db_role); END $$ LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; diff --git a/test/CDB_FederatedServer.sql b/test/CDB_FederatedServer.sql index a818f8e..287fdb0 100644 --- a/test/CDB_FederatedServer.sql +++ b/test/CDB_FederatedServer.sql @@ -138,14 +138,8 @@ SELECT '8.3', cartodb.CDB_Federated_Server_Unregister(server := 'myRemote" or''n -- Test permissions \set QUIET on - --- 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; - \set QUIET off SELECT '9.1', cartodb.CDB_Federated_Server_Register_PG(server := 'myRemote3'::text, config := '{ @@ -180,16 +174,16 @@ SELECT '9.3', cartodb.CDB_Federated_Server_Register_PG(server := 'myRemote4'::te \c contrib_regression postgres \echo '## Granting access to a user works' -SELECT '9.5', cartodb.CDB_Federated_Server_Grant_Access(server := 'myRemote3', usernames := ARRAY['cdb_fs_tester']); -SELECT '9.6', cartodb.CDB_Federated_Server_Grant_Access(server := 'does not exist', usernames := ARRAY['cdb_fs_tester']); -SELECT '9.7', cartodb.CDB_Federated_Server_Grant_Access(server := 'myRemote3', usernames := ARRAY['does not exist']); +SELECT '9.5', cartodb.CDB_Federated_Server_Grant_Access(server := 'myRemote3', db_role := 'cdb_fs_tester'::name); +SELECT '9.6', cartodb.CDB_Federated_Server_Grant_Access(server := 'does not exist', db_role := 'cdb_fs_tester'::name); +SELECT '9.7', cartodb.CDB_Federated_Server_Grant_Access(server := 'myRemote3', db_role := 'does not exist'::name); \echo '## Granting access again raises a notice' -SELECT '9.8', cartodb.CDB_Federated_Server_Grant_Access(server := 'myRemote3', usernames := ARRAY['cdb_fs_tester']); +SELECT '9.8', cartodb.CDB_Federated_Server_Grant_Access(server := 'myRemote3', db_role := 'cdb_fs_tester'::name); \echo '## Revoking access to a user works' -SELECT '9.9', cartodb.CDB_Federated_Server_Revoke_Access(server := 'myRemote3', usernames := ARRAY['cdb_fs_tester']); -SELECT '9.10', cartodb.CDB_Federated_Server_Grant_Access(server := 'myRemote3', usernames := ARRAY['cdb_fs_tester']); +SELECT '9.9', cartodb.CDB_Federated_Server_Revoke_Access(server := 'myRemote3', db_role := 'cdb_fs_tester'::name); +SELECT '9.10', cartodb.CDB_Federated_Server_Grant_Access(server := 'myRemote3', db_role := 'cdb_fs_tester'::name); \echo '## Unregistering a server with active grants works' SELECT '9.11', cartodb.CDB_Federated_Server_Unregister(server := 'myRemote3'::text); @@ -206,7 +200,7 @@ SELECT '10.1', cartodb.CDB_Federated_Server_Register_PG(server := 'myRemote4'::t "password": "foobarino" } }'::jsonb); -SELECT '10.2', cartodb.CDB_Federated_Server_Grant_Access(server := 'myRemote4', usernames := ARRAY['cdb_fs_tester']); +SELECT '10.2', cartodb.CDB_Federated_Server_Grant_Access(server := 'myRemote4', db_role := 'cdb_fs_tester'::name); \c contrib_regression cdb_fs_tester SELECT '10.3', cartodb.CDB_Federated_Server_Unregister(server := 'myRemote4'::text); @@ -217,7 +211,6 @@ SELECT '10.4', cartodb.CDB_Federated_Server_Unregister(server := 'myRemote4'::te -- Cleanup \set QUIET on -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; diff --git a/test/CDB_FederatedServerListRemote.sql b/test/CDB_FederatedServerListRemote.sql index e230ab0..9db8a80 100644 --- a/test/CDB_FederatedServerListRemote.sql +++ b/test/CDB_FederatedServerListRemote.sql @@ -7,23 +7,14 @@ SET client_min_messages TO error; SET SESSION AUTHORIZATION postgres; CREATE EXTENSION postgres_fdw; --- 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", @@ -111,13 +102,13 @@ SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Schemas(server => 'loopba \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']); +SELECT cartodb.CDB_Federated_Server_Grant_Access(server := 'loopback', db_role := 'cdb_fs_tester'::name); \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']); +SELECT cartodb.CDB_Federated_Server_Revoke_Access(server := 'loopback', db_role := 'cdb_fs_tester'::name); \c contrib_regression cdb_fs_tester SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Schemas(server => 'loopback'); \c contrib_regression postgres @@ -144,13 +135,13 @@ SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Tables(server => 'loopbac \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']); +SELECT cartodb.CDB_Federated_Server_Grant_Access(server := 'loopback', db_role := 'cdb_fs_tester'::name); \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']); +SELECT cartodb.CDB_Federated_Server_Revoke_Access(server := 'loopback', db_role := 'cdb_fs_tester'::name); \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 @@ -180,13 +171,13 @@ SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Columns(server => 'loopba \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']); +SELECT cartodb.CDB_Federated_Server_Grant_Access(server := 'loopback', db_role := 'cdb_fs_tester'::name); \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']); +SELECT cartodb.CDB_Federated_Server_Revoke_Access(server := 'loopback', db_role := 'cdb_fs_tester'::name); \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 @@ -211,7 +202,7 @@ SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Columns(server => 'loopba \echo '## Test listing of remote objects with permissions (sunny day)' -SELECT cartodb.CDB_Federated_Server_Grant_Access(server := 'loopback2', usernames := ARRAY['cdb_fs_tester2']); +SELECT cartodb.CDB_Federated_Server_Grant_Access(server := 'loopback2', db_role := 'cdb_fs_tester2'::name); \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'); @@ -219,12 +210,11 @@ SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Columns(server => 'loopba \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']); +SELECT cartodb.CDB_Federated_Server_Grant_Access(server := 'loopback2', db_role := 'cdb_fs_tester'::name); \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'); @@ -301,7 +291,6 @@ 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; diff --git a/test/CDB_FederatedServerListRemote_expect b/test/CDB_FederatedServerListRemote_expect index e05af3a..e4929a7 100644 --- a/test/CDB_FederatedServerListRemote_expect +++ b/test/CDB_FederatedServerListRemote_expect @@ -1,5 +1,3 @@ - - C1| C2| ## Test listing of remote schemas without permissions before the first instantiation (rainy day) @@ -120,7 +118,6 @@ 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) diff --git a/test/CDB_FederatedServerTables.sql b/test/CDB_FederatedServerTables.sql index 62d2bc2..50663d5 100644 --- a/test/CDB_FederatedServerTables.sql +++ b/test/CDB_FederatedServerTables.sql @@ -7,7 +7,15 @@ SET client_min_messages TO error; 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 ROLE cdb_fs_tester2 LOGIN PASSWORD 'cdb_fs_passwd2'; +GRANT CONNECT ON DATABASE contrib_regression TO cdb_fs_tester2; + +-- 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 := '{ @@ -122,6 +130,10 @@ Select 'list_remotes3', CDB_Federated_Server_List_Registered_Tables( remote_schema => 'remote_schema' ); +-- =================================================================== +-- Test input +-- =================================================================== + \echo '## Registering a table: Invalid server fails' SELECT cartodb.CDB_Federated_Table_Register( server => 'Does not exist', @@ -242,6 +254,9 @@ SELECT cartodb.CDB_Federated_Table_Unregister( remote_table => 'remote_geom' ); +-- =================================================================== +-- Test conflicts +-- =================================================================== \echo '## Target conflict is handled nicely: Table' CREATE TABLE localtable (a integer); @@ -266,6 +281,10 @@ SELECT cartodb.CDB_Federated_Table_Register( DROP VIEW localtable2; DROP TABLE localtable; +-- =================================================================== +-- Test permissions +-- =================================================================== + -- Try permissions tricks -- Try registering and accessing a table as normal user @@ -284,8 +303,12 @@ DROP TABLE localtable; SET client_min_messages TO error; \set VERBOSITY terse +REVOKE CONNECT ON DATABASE contrib_regression FROM cdb_fs_tester2; +DROP ROLE cdb_fs_tester2; + SELECT 'D1', cartodb.CDB_Federated_Server_Unregister(server := 'loopback'::text); DROP DATABASE cdb_fs_tester; +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_FederatedServer_expect b/test/CDB_FederatedServer_expect index aecffa1..c411d34 100644 --- a/test/CDB_FederatedServer_expect +++ b/test/CDB_FederatedServer_expect @@ -38,7 +38,6 @@ ERROR: Server information is mandatory 8.1| 8.2|("myRemote"" or'not",postgres_fdw,localhost,5432,"fdw target",read-only,"fdw user") 8.3| - 9.1| You are now connected to database "contrib_regression" as user "cdb_fs_tester". ## All users are able to list servers @@ -49,7 +48,7 @@ You are now connected to database "contrib_regression" as user "postgres". ## Granting access to a user works 9.5| ERROR: Server "does not exist" does not exist -ERROR: User role "does not exist" does not exists +ERROR: role "does not exist" does not exist ## Granting access again raises a notice NOTICE: role "cdb_fs_tester" is already a member of role "cdb_fs_role_95b63382aabca4433e7bd9cba6c30368" 9.8|