diff --git a/scripts-available/CDB_Groups.sql b/scripts-available/CDB_Groups.sql index fffa9a5..4efe102 100644 --- a/scripts-available/CDB_Groups.sql +++ b/scripts-available/CDB_Groups.sql @@ -5,7 +5,6 @@ FUNCTION cartodb.CDB_Group_CreateGroup(group_name text) DECLARE cdb_group_role TEXT; BEGIN - -- TODO: escape group_name cdb_group_role := cartodb._CDB_Group_GroupRole(group_name); IF NOT EXISTS ( SELECT 1 FROM pg_roles WHERE rolname = cdb_group_role ) THEN @@ -15,12 +14,19 @@ END $$ LANGUAGE PLPGSQL; -- Drops group and everything that role owns +-- TODO: LIMITATION: in order to drop a role all its owned objects must be dropped before. +-- Right now this is done with DROP OWNED, which can only be done by a superadmin. +-- Not even the role creator can drop the role and the objects it owns. +-- All group owned objects by the group are permissions. CREATE OR REPLACE FUNCTION cartodb.CDB_Group_DropGroup(group_name text) RETURNS VOID AS $$ +DECLARE + cdb_group_role TEXT; BEGIN - EXECUTE 'DROP OWNED BY "' || cartodb._CDB_Group_GroupRole(group_name) || '"'; - EXECUTE 'DROP ROLE IF EXISTS "' || cartodb._CDB_Group_GroupRole(group_name) || '"'; + cdb_group_role := cartodb._CDB_Group_GroupRole(group_name); + EXECUTE 'DROP OWNED BY "' || cdb_group_role || '"'; + EXECUTE 'DROP ROLE IF EXISTS "' || cdb_group_role || '"'; END $$ LANGUAGE PLPGSQL; @@ -124,7 +130,9 @@ FUNCTION cartodb._CDB_User_RoleFromUsername(username text) DECLARE user_role TEXT; BEGIN - EXECUTE 'SELECT SCHEMA_OWNER FROM INFORMATION_SCHEMA.SCHEMATA WHERE SCHEMA_NAME = $1 LIMIT 1' INTO user_role USING username; + -- This was preferred, but non-superadmins won't get results + --EXECUTE 'SELECT SCHEMA_OWNER FROM INFORMATION_SCHEMA.SCHEMATA WHERE SCHEMA_NAME = $1 LIMIT 1' INTO user_role USING username; + EXECUTE 'SELECT pg_get_userbyid(nspowner) FROM pg_namespace WHERE nspname = $1;' INTO user_role USING username; RETURN user_role; END $$ LANGUAGE PLPGSQL; diff --git a/scripts-available/CDB_Organizations.sql b/scripts-available/CDB_Organizations.sql index 4328fe4..6ca48c2 100644 --- a/scripts-available/CDB_Organizations.sql +++ b/scripts-available/CDB_Organizations.sql @@ -25,6 +25,57 @@ BEGIN END $$ LANGUAGE PLPGSQL VOLATILE; +------------------------------------------------------------------------------- +-- Administrator +------------------------------------------------------------------------------- +CREATE OR REPLACE +FUNCTION cartodb._CDB_Organization_Admin_Role_Name() + RETURNS TEXT +AS 'SELECT ''cdb_org_admin''::text || ''_'' || md5(current_database());' +LANGUAGE SQL IMMUTABLE; + +DO LANGUAGE 'plpgsql' $$ +DECLARE + cdb_org_admin_role_name TEXT; +BEGIN + cdb_org_admin_role_name := cartodb._CDB_Organization_Admin_Role_Name(); + IF NOT EXISTS ( SELECT * FROM pg_roles WHERE rolname= cdb_org_admin_role_name ) + THEN + EXECUTE 'CREATE ROLE "' || cdb_org_admin_role_name || '" CREATEROLE NOLOGIN;'; + END IF; +END +$$; + +CREATE OR REPLACE +FUNCTION cartodb.CDB_Organization_AddAdmin(username text) + RETURNS void +AS $$ +DECLARE + cdb_user_role TEXT; + cdb_admin_role TEXT; +BEGIN + cdb_admin_role := cartodb._CDB_Organization_Admin_Role_Name(); + cdb_user_role := cartodb._CDB_User_RoleFromUsername(username); + EXECUTE 'GRANT "' || cdb_admin_role || '" TO "' || cdb_user_role || '" WITH ADMIN OPTION'; + -- CREATEROLE is not inherited, and is needed for user creation + EXECUTE 'ALTER ROLE "' || cdb_user_role || '" CREATEROLE'; +END +$$ LANGUAGE PLPGSQL; + +CREATE OR REPLACE +FUNCTION cartodb.CDB_Organization_RemoveAdmin(username text) + RETURNS void +AS $$ +DECLARE + cdb_user_role TEXT; + cdb_admin_role TEXT; +BEGIN + cdb_admin_role := cartodb._CDB_Organization_Admin_Role_Name(); + cdb_user_role := cartodb._CDB_User_RoleFromUsername(username); + EXECUTE 'ALTER ROLE "' || cdb_user_role || '" NOCREATEROLE'; + EXECUTE 'REVOKE "' || cdb_admin_role || '" FROM "' || cdb_user_role || '"'; +END +$$ LANGUAGE PLPGSQL; ------------------------------------------------------------------------------- -- Sharing tables diff --git a/test/organization/test.sh b/test/organization/test.sh index 38cf199..595124f 100644 --- a/test/organization/test.sh +++ b/test/organization/test.sh @@ -166,6 +166,8 @@ function setup() { ${CMD} -d ${DATABASE} -f scripts-available/CDB_Groups.sql log_info "############################# SETUP #############################" + create_role_and_schema cdb_org_admin + sql "SELECT cartodb.CDB_Organization_AddAdmin('cdb_org_admin');" create_role_and_schema cdb_testmember_1 create_role_and_schema cdb_testmember_2 #publicuser# sql "CREATE ROLE publicuser LOGIN;" @@ -201,20 +203,24 @@ function tear_down() { sql "SELECT cartodb.CDB_Group_RemoveMember('group_a', 'cdb_testmember_1')" sql "select cartodb.CDB_Group_DropGroup('group_a')" - - sql "DROP SCHEMA cartodb CASCADE" + sql "SELECT cartodb.CDB_Organization_RemoveAdmin('cdb_org_admin');" log_info "########################### TEAR DOWN ###########################" sql 'DROP SCHEMA cdb_testmember_1;' sql 'DROP SCHEMA cdb_testmember_2;' + sql 'DROP SCHEMA cdb_org_admin;' sql "REVOKE CONNECT ON DATABASE \"${DATABASE}\" FROM cdb_testmember_1;" sql "REVOKE CONNECT ON DATABASE \"${DATABASE}\" FROM cdb_testmember_2;" #publicuser# sql "REVOKE CONNECT ON DATABASE \"${DATABASE}\" FROM publicuser;" + sql "REVOKE CONNECT ON DATABASE \"${DATABASE}\" FROM cdb_org_admin;" sql 'DROP ROLE cdb_testmember_1;' sql 'DROP ROLE cdb_testmember_2;' #publicuser# sql 'DROP ROLE publicuser;' + sql 'DROP ROLE cdb_org_admin;' + + sql "DROP SCHEMA cartodb CASCADE" ${CMD} -c "DROP DATABASE ${DATABASE}" } @@ -470,7 +476,9 @@ function test_group_management_functions_cant_be_used_by_normal_members() { sql cdb_testmember_1 "SELECT cartodb.CDB_Group_DropGroup('group_a');" fails sql cdb_testmember_1 "SELECT cartodb.CDB_Group_AddMember('group_a', 'cdb_testmember_2');" fails sql cdb_testmember_1 "SELECT cartodb.CDB_Group_RemoveMember('group_a', 'cdb_testmember_1');" fails +} +function test_group_permission_functions_cant_be_used_by_normal_members() { create_table cdb_testmember_2 shared_with_group sql cdb_testmember_1 "select cartoDB.CDB_Group_Table_GrantRead('group_a', 'cdb_testmember_2', 'shared_with_group');" fails @@ -485,6 +493,30 @@ function test_group_management_functions_cant_be_used_by_normal_members() { sql cdb_testmember_2 'DROP TABLE cdb_testmember_2.shared_with_group;' } +function test_group_management_functions_can_be_used_by_org_admin() { + sql cdb_org_admin "SELECT cartodb.CDB_Group_CreateGroup('group_x_tmp');" + sql cdb_org_admin "SELECT cartodb.CDB_Group_RenameGroup('group_x_tmp', 'group_x');" + sql cdb_org_admin "SELECT cartodb.CDB_Group_AddMember('group_x', 'cdb_testmember_1');" + sql cdb_org_admin "SELECT cartodb.CDB_Group_RemoveMember('group_x', 'cdb_testmember_1');" + # TODO: workaround superadmin limitation + sql "SELECT cartodb.CDB_Group_DropGroup('group_x');" +} + +function test_org_admin_cant_grant_permissions_on_tables_he_does_not_own() { + create_table cdb_testmember_2 shared_with_group + + sql cdb_org_admin "select cartoDB.CDB_Group_Table_GrantRead('group_a', 'cdb_testmember_2', 'shared_with_group');" fails + sql cdb_org_admin "select cartoDB.CDB_Group_Table_GrantReadWrite('group_a', 'cdb_testmember_2', 'shared_with_group');" fails + + # Checks that you can't grant even if your group has RW permissions + sql cdb_testmember_2 "select cartoDB.CDB_Group_Table_GrantReadWrite('group_a', 'cdb_testmember_2', 'shared_with_group')" + sql cdb_org_admin "select cartoDB.CDB_Group_Table_GrantRead('group_a', 'cdb_testmember_2', 'shared_with_group');" fails + sql cdb_org_admin "select cartoDB.CDB_Group_Table_GrantReadWrite('group_b', 'cdb_testmember_2', 'shared_with_group');" fails + sql cdb_org_admin "select cartoDB.CDB_Group_Table_RevokeAll('group_b', 'cdb_testmember_2', 'shared_with_group');" fails + + sql cdb_testmember_2 'DROP TABLE cdb_testmember_2.shared_with_group;' +} + function test_valid_group_names() { sql "select cartodb._CDB_Group_GroupRole('group_1$_a');" sql "select cartodb._CDB_Group_GroupRole('GROUP_1$_A');"