From 21fb4153481ed0d1f69e6231dd0e063883ddf827 Mon Sep 17 00:00:00 2001 From: Anish Umale Date: Wed, 21 Aug 2024 03:01:57 +0530 Subject: [PATCH 01/11] implement sql functions for adding roles --- db/sql/00_msar.sql | 89 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 84 insertions(+), 5 deletions(-) diff --git a/db/sql/00_msar.sql b/db/sql/00_msar.sql index b0c400ee99..10279b1e5c 100644 --- a/db/sql/00_msar.sql +++ b/db/sql/00_msar.sql @@ -948,6 +948,54 @@ FROM ( $$ LANGUAGE SQL STABLE; +CREATE OR REPLACE FUNCTION msar.get_role(rolename text) RETURNS jsonb AS $$/* +Given a rolename, return a JSON object describing the role in a database server. + +The returned JSON object has the form: + { + "oid": + "name": + "super": + "inherits": + "create_role": + "create_db": + "login": + "description": + "members": <[ + { "oid": , "admin": } + ]|null> + } +*/ +WITH rolemembers as ( + SELECT + pgr.oid AS oid, + jsonb_agg( + jsonb_build_object( + 'oid', pgm.member::bigint, + 'admin', pgm.admin_option + ) + ) AS members + FROM pg_catalog.pg_roles pgr + INNER JOIN pg_catalog.pg_auth_members pgm ON pgr.oid=pgm.roleid + GROUP BY pgr.oid +) +SELECT jsonb_build_object( + 'oid', r.oid::bigint, + 'name', r.rolname, + 'super', r.rolsuper, + 'inherits', r.rolinherit, + 'create_role', r.rolcreaterole, + 'create_db', r.rolcreatedb, + 'login', r.rolcanlogin, + 'description', pg_catalog.shobj_description(r.oid, 'pg_authid'), + 'members', rolemembers.members +) +FROM pg_catalog.pg_roles r + LEFT OUTER JOIN rolemembers ON r.oid = rolemembers.oid +WHERE r.rolname = rolename; +$$ LANGUAGE SQL STABLE; + + CREATE OR REPLACE FUNCTION msar.list_db_priv(db_name text) RETURNS jsonb AS $$/* Given a database name, returns a json array of objects with database privileges for non-inherited roles. @@ -1014,9 +1062,6 @@ $$ LANGUAGE SQL STABLE RETURNS NULL ON NULL INPUT; CREATE OR REPLACE FUNCTION msar.create_basic_mathesar_user(username text, password_ text) RETURNS TEXT AS $$/* */ -DECLARE - sch_name text; - mathesar_schemas text[] := ARRAY['mathesar_types', '__msar', 'msar']; BEGIN PERFORM __msar.exec_ddl('CREATE USER %I WITH PASSWORD %L', username, password_); PERFORM __msar.exec_ddl( @@ -1024,19 +1069,53 @@ BEGIN current_database()::text, username ); + PERFORM msar.grant_usage_on_mathesar_schemas(username); + RETURN username; +END; +$$ LANGUAGE plpgsql RETURNS NULL ON NULL INPUT; + + +CREATE OR REPLACE FUNCTION +msar.grant_usage_on_mathesar_schemas(rolename text) RETURNS void AS $$ +DECLARE + sch_name text; + mathesar_schemas text[] := ARRAY['mathesar_types', '__msar', 'msar']; +BEGIN FOREACH sch_name IN ARRAY mathesar_schemas LOOP BEGIN - PERFORM __msar.exec_ddl('GRANT USAGE ON SCHEMA %I TO %I', sch_name, username); + PERFORM __msar.exec_ddl('GRANT USAGE ON SCHEMA %I TO %I', sch_name, rolename); EXCEPTION WHEN invalid_schema_name THEN RAISE NOTICE 'Schema % does not exist', sch_name; END; END LOOP; - RETURN username; END; $$ LANGUAGE plpgsql RETURNS NULL ON NULL INPUT; +CREATE OR REPLACE FUNCTION +msar.create_role(rolename text, password_ text, login_ boolean) RETURNS jsonb AS $$/* +*/ +DECLARE + sch_name text; + mathesar_schemas text[] := ARRAY['mathesar_types', '__msar', 'msar']; +BEGIN + CASE WHEN login_ THEN + PERFORM create_basic_mathesar_user(rolename, password_); + ELSE + PERFORM __msar.exec_ddl('CREATE ROLE %I', rolename); + PERFORM __msar.exec_ddl( + 'GRANT CREATE, TEMP ON DATABASE %I TO %I', + current_database()::text, + rolename + ); + PERFORM msar.grant_usage_on_mathesar_schemas(rolename); + END CASE; + RETURN msar.get_role(rolename); +END; +$$ LANGUAGE plpgsql; + + ---------------------------------------------------------------------------------------------------- ---------------------------------------------------------------------------------------------------- -- ALTER SCHEMA FUNCTIONS From c7a9c0597e5e18ae506f19147fb112df97ff7375 Mon Sep 17 00:00:00 2001 From: Anish Umale Date: Wed, 21 Aug 2024 03:02:23 +0530 Subject: [PATCH 02/11] wire up sql functions to python --- db/roles/operations/create.py | 5 +++++ mathesar/rpc/roles.py | 16 +++++++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 db/roles/operations/create.py diff --git a/db/roles/operations/create.py b/db/roles/operations/create.py new file mode 100644 index 0000000000..afb1460186 --- /dev/null +++ b/db/roles/operations/create.py @@ -0,0 +1,5 @@ +from db.connection import exec_msar_func + + +def create_role(rolename, password, login, conn): + return exec_msar_func(conn, 'create_role', rolename, password, login).fetchone()[0] diff --git a/mathesar/rpc/roles.py b/mathesar/rpc/roles.py index 0db0c10c3c..a7aad1890c 100644 --- a/mathesar/rpc/roles.py +++ b/mathesar/rpc/roles.py @@ -9,6 +9,7 @@ from mathesar.rpc.exceptions.handlers import handle_rpc_exceptions from mathesar.rpc.utils import connect from db.roles.operations.select import get_roles +from db.roles.operations.create import create_role class RoleMember(TypedDict): @@ -53,6 +54,10 @@ class RoleInfo(TypedDict): description: Optional[str] members: Optional[list[RoleMember]] + @classmethod + def from_dict(cls, d): + return cls + @rpc_method(name="roles.list") @http_basic_auth_login_required @@ -71,5 +76,14 @@ def list_(*, database_id: int, **kwargs) -> list[RoleInfo]: user = kwargs.get(REQUEST_KEY).user with connect(database_id, user) as conn: roles = get_roles(conn) + return [RoleInfo.from_dict(role) for role in roles] + - return roles +@rpc_method(name="roles.add") +@http_basic_auth_login_required +@handle_rpc_exceptions +def add(*, rolename: str, database_id: int, password: str, login: bool, **kwargs) -> RoleInfo: + user = kwargs.get(REQUEST_KEY).user + with connect(database_id, user) as conn: + role = create_role(rolename, password, login, conn) + return RoleInfo.from_dict(role) From 44c2ce2cd6fcc6d5b2446379b9b3359a56d8b51e Mon Sep 17 00:00:00 2001 From: Anish Umale Date: Wed, 21 Aug 2024 22:07:44 +0530 Subject: [PATCH 03/11] add endpoint test --- mathesar/tests/rpc/test_endpoints.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/mathesar/tests/rpc/test_endpoints.py b/mathesar/tests/rpc/test_endpoints.py index 067bff05d1..4109a9bd10 100644 --- a/mathesar/tests/rpc/test_endpoints.py +++ b/mathesar/tests/rpc/test_endpoints.py @@ -225,6 +225,11 @@ "roles.list", [user_is_authenticated] ), + ( + roles.add, + "roles.add", + [user_is_authenticated] + ), ( schemas.add, "schemas.add", From 0448f6b12ad7600227e1c8c8cbe130676001470d Mon Sep 17 00:00:00 2001 From: Anish Umale Date: Wed, 21 Aug 2024 23:45:35 +0530 Subject: [PATCH 04/11] add docstrings for sql and python functions --- db/sql/00_msar.sql | 28 ++++++++++++++++++++++++++++ mathesar/rpc/roles.py | 23 +++++++++++++++++++++-- 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/db/sql/00_msar.sql b/db/sql/00_msar.sql index 10279b1e5c..015811e03a 100644 --- a/db/sql/00_msar.sql +++ b/db/sql/00_msar.sql @@ -1061,6 +1061,12 @@ $$ LANGUAGE SQL STABLE RETURNS NULL ON NULL INPUT; CREATE OR REPLACE FUNCTION msar.create_basic_mathesar_user(username text, password_ text) RETURNS TEXT AS $$/* +Given the username and password_, creates a user on the database server. +Additionally, grants CREATE, CONNECT and TEMP on the current database to the created user. + +Args: + username: The name of the user to be created, unquoted. + password_: The password for the user to set, unquoted. */ BEGIN PERFORM __msar.exec_ddl('CREATE USER %I WITH PASSWORD %L', username, password_); @@ -1095,6 +1101,28 @@ $$ LANGUAGE plpgsql RETURNS NULL ON NULL INPUT; CREATE OR REPLACE FUNCTION msar.create_role(rolename text, password_ text, login_ boolean) RETURNS jsonb AS $$/* +Creates a login/non-login role, depending on whether the login_ flag is set. +Only the rolename field is required, the password field is required only if login_ is set to true. + +Returns a JSON object describing the created role in the form: + { + "oid": + "name": + "super": + "inherits": + "create_role": + "create_db": + "login": + "description": + "members": <[ + { "oid": , "admin": } + ]|null> + } + +Args: + rolename: The name of the role to be created, unquoted. + password_: The password for the rolename to set, unquoted. + login_: Specify whether the role to be created could login. */ DECLARE sch_name text; diff --git a/mathesar/rpc/roles.py b/mathesar/rpc/roles.py index a7aad1890c..1c940e3e7f 100644 --- a/mathesar/rpc/roles.py +++ b/mathesar/rpc/roles.py @@ -68,7 +68,7 @@ def list_(*, database_id: int, **kwargs) -> list[RoleInfo]: Requires a database id inorder to connect to the server. Args: - database_id: The Django id of the database containing the table. + database_id: The Django id of the database. Returns: A list of roles present on the database server. @@ -82,7 +82,26 @@ def list_(*, database_id: int, **kwargs) -> list[RoleInfo]: @rpc_method(name="roles.add") @http_basic_auth_login_required @handle_rpc_exceptions -def add(*, rolename: str, database_id: int, password: str, login: bool, **kwargs) -> RoleInfo: +def add( + *, + rolename: str, + database_id: int, + password: str = None, + login: bool = None, + **kwargs +) -> RoleInfo: + """ + Add a new login/non-login role on a database server. + + Args: + rolename: The name of the role to be created. + database_id: The Django id of the database. + password: The password for the rolename to set. + login: Whether the role to be created could login. + + Returns: + A dict describing the created role. + """ user = kwargs.get(REQUEST_KEY).user with connect(database_id, user) as conn: role = create_role(rolename, password, login, conn) From 83f86bd78f772697c7eede0575d58a194b14d40c Mon Sep 17 00:00:00 2001 From: Anish Umale Date: Wed, 21 Aug 2024 23:45:48 +0530 Subject: [PATCH 05/11] add docs --- docs/docs/api/rpc.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/docs/api/rpc.md b/docs/docs/api/rpc.md index 1dc1378d00..cc70568626 100644 --- a/docs/docs/api/rpc.md +++ b/docs/docs/api/rpc.md @@ -230,6 +230,7 @@ To use an RPC function: options: members: - list_ + - add - RoleInfo - RoleMember From a9707925705e564a82a1b50a4da303ef6ededa0c Mon Sep 17 00:00:00 2001 From: Anish Umale Date: Thu, 22 Aug 2024 19:09:18 +0530 Subject: [PATCH 06/11] change get_roles->list_roles and extract pg_roles logic to role_info_table --- db/roles/operations/select.py | 4 +- db/sql/00_msar.sql | 114 +++++++++++------------ db/sql/test_00_msar.sql | 20 ++-- db/tests/roles/operations/test_select.py | 6 +- mathesar/rpc/roles.py | 16 +++- 5 files changed, 81 insertions(+), 79 deletions(-) diff --git a/db/roles/operations/select.py b/db/roles/operations/select.py index 1d0d94434b..9ad7131f21 100644 --- a/db/roles/operations/select.py +++ b/db/roles/operations/select.py @@ -1,8 +1,8 @@ from db.connection import exec_msar_func -def get_roles(conn): - return exec_msar_func(conn, 'get_roles').fetchone()[0] +def list_roles(conn): + return exec_msar_func(conn, 'list_roles').fetchone()[0] def list_db_priv(db_name, conn): diff --git a/db/sql/00_msar.sql b/db/sql/00_msar.sql index 015811e03a..b33fe5d133 100644 --- a/db/sql/00_msar.sql +++ b/db/sql/00_msar.sql @@ -897,7 +897,49 @@ FROM ( $$ LANGUAGE SQL; -CREATE OR REPLACE FUNCTION msar.get_roles() RETURNS jsonb AS $$/* +CREATE OR REPLACE FUNCTION msar.role_info_table() RETURNS TABLE +( + oid oid, + name name, + super boolean, + inherits boolean, + create_role boolean, + create_db boolean, + login boolean, + description text, + members jsonb +) AS $$/* +Returns a table describing all the roles present on the database server. +*/ +WITH rolemembers as ( + SELECT + pgr.oid AS oid, + jsonb_agg( + jsonb_build_object( + 'oid', pgm.member::bigint, + 'admin', pgm.admin_option + ) + ) AS members + FROM pg_catalog.pg_roles pgr + INNER JOIN pg_catalog.pg_auth_members pgm ON pgr.oid=pgm.roleid + GROUP BY pgr.oid +) +SELECT + r.oid::bigint AS oid, + r.rolname AS name, + r.rolsuper AS super, + r.rolinherit AS inherits, + r.rolcreaterole AS create_role, + r.rolcreatedb AS create_db, + r.rolcanlogin AS login, + pg_catalog.shobj_description(r.oid, 'pg_authid') AS description, + rolemembers.members AS members +FROM pg_catalog.pg_roles r +LEFT OUTER JOIN rolemembers ON r.oid = rolemembers.oid; +$$ LANGUAGE SQL STABLE; + + +CREATE OR REPLACE FUNCTION msar.list_roles() RETURNS jsonb AS $$/* Return a json array of objects with the list of roles in a database server, excluding pg system roles. @@ -916,35 +958,9 @@ Each returned JSON object in the array has the form: ]|null> } */ -WITH rolemembers as ( - SELECT - pgr.oid AS oid, - jsonb_agg( - jsonb_build_object( - 'oid', pgm.member::bigint, - 'admin', pgm.admin_option - ) - ) AS members - FROM pg_catalog.pg_roles pgr - INNER JOIN pg_catalog.pg_auth_members pgm ON pgr.oid=pgm.roleid - GROUP BY pgr.oid -) SELECT jsonb_agg(role_data) -FROM ( - SELECT - r.oid::bigint AS oid, - r.rolname AS name, - r.rolsuper AS super, - r.rolinherit AS inherits, - r.rolcreaterole AS create_role, - r.rolcreatedb AS create_db, - r.rolcanlogin AS login, - pg_catalog.shobj_description(r.oid, 'pg_authid') AS description, - rolemembers.members AS members - FROM pg_catalog.pg_roles r - LEFT OUTER JOIN rolemembers ON r.oid = rolemembers.oid - WHERE r.rolname NOT LIKE 'pg_%' -) AS role_data; +FROM msar.role_info_table() AS role_data +WHERE role_data.name NOT LIKE 'pg_%'; $$ LANGUAGE SQL STABLE; @@ -966,33 +982,9 @@ The returned JSON object has the form: ]|null> } */ -WITH rolemembers as ( - SELECT - pgr.oid AS oid, - jsonb_agg( - jsonb_build_object( - 'oid', pgm.member::bigint, - 'admin', pgm.admin_option - ) - ) AS members - FROM pg_catalog.pg_roles pgr - INNER JOIN pg_catalog.pg_auth_members pgm ON pgr.oid=pgm.roleid - GROUP BY pgr.oid -) -SELECT jsonb_build_object( - 'oid', r.oid::bigint, - 'name', r.rolname, - 'super', r.rolsuper, - 'inherits', r.rolinherit, - 'create_role', r.rolcreaterole, - 'create_db', r.rolcreatedb, - 'login', r.rolcanlogin, - 'description', pg_catalog.shobj_description(r.oid, 'pg_authid'), - 'members', rolemembers.members -) -FROM pg_catalog.pg_roles r - LEFT OUTER JOIN rolemembers ON r.oid = rolemembers.oid -WHERE r.rolname = rolename; +SELECT to_jsonb(role_data) +FROM msar.role_info_table() AS role_data +WHERE role_data.name = rolename; $$ LANGUAGE SQL STABLE; @@ -1069,8 +1061,8 @@ Args: password_: The password for the user to set, unquoted. */ BEGIN - PERFORM __msar.exec_ddl('CREATE USER %I WITH PASSWORD %L', username, password_); - PERFORM __msar.exec_ddl( + EXECUTE format('CREATE USER %I WITH PASSWORD %L', username, password_); + EXECUTE format( 'GRANT CREATE, CONNECT, TEMP ON DATABASE %I TO %I', current_database()::text, username @@ -1129,10 +1121,10 @@ DECLARE mathesar_schemas text[] := ARRAY['mathesar_types', '__msar', 'msar']; BEGIN CASE WHEN login_ THEN - PERFORM create_basic_mathesar_user(rolename, password_); + PERFORM msar.create_basic_mathesar_user(rolename, password_); ELSE - PERFORM __msar.exec_ddl('CREATE ROLE %I', rolename); - PERFORM __msar.exec_ddl( + EXECUTE format('CREATE ROLE %I', rolename); + EXECUTE format( 'GRANT CREATE, TEMP ON DATABASE %I TO %I', current_database()::text, rolename diff --git a/db/sql/test_00_msar.sql b/db/sql/test_00_msar.sql index 9c0030deaa..4bb11424aa 100644 --- a/db/sql/test_00_msar.sql +++ b/db/sql/test_00_msar.sql @@ -2760,18 +2760,18 @@ END; $$ LANGUAGE plpgsql; -CREATE OR REPLACE FUNCTION test_get_roles() RETURNS SETOF TEXT AS $$ +CREATE OR REPLACE FUNCTION test_list_roles() RETURNS SETOF TEXT AS $$ DECLARE initial_role_count int; foo_role jsonb; bar_role jsonb; BEGIN - SELECT jsonb_array_length(msar.get_roles()) INTO initial_role_count; + SELECT jsonb_array_length(msar.list_roles()) INTO initial_role_count; -- Create role and check if role is present in response & count is increased CREATE ROLE foo; - RETURN NEXT is(jsonb_array_length(msar.get_roles()), initial_role_count + 1); - SELECT jsonb_path_query(msar.get_roles(), '$[*] ? (@.name == "foo")') INTO foo_role; + RETURN NEXT is(jsonb_array_length(msar.list_roles()), initial_role_count + 1); + SELECT jsonb_path_query(msar.list_roles(), '$[*] ? (@.name == "foo")') INTO foo_role; -- Check if role has expected properties RETURN NEXT is(jsonb_typeof(foo_role), 'object'); @@ -2785,7 +2785,7 @@ BEGIN -- Modify properties and check role again ALTER ROLE foo WITH CREATEDB CREATEROLE LOGIN NOINHERIT; - SELECT jsonb_path_query(msar.get_roles(), '$[*] ? (@.name == "foo")') INTO foo_role; + SELECT jsonb_path_query(msar.list_roles(), '$[*] ? (@.name == "foo")') INTO foo_role; RETURN NEXT is((foo_role->>'super')::boolean, false); RETURN NEXT is((foo_role->>'inherits')::boolean, false); RETURN NEXT is((foo_role->>'create_role')::boolean, true); @@ -2794,15 +2794,15 @@ BEGIN -- Add comment and check if comment is present COMMENT ON ROLE foo IS 'A test role'; - SELECT jsonb_path_query(msar.get_roles(), '$[*] ? (@.name == "foo")') INTO foo_role; + SELECT jsonb_path_query(msar.list_roles(), '$[*] ? (@.name == "foo")') INTO foo_role; RETURN NEXT is(foo_role->'description'#>>'{}', 'A test role'); -- Add members and check result CREATE ROLE bar; GRANT foo TO bar; - RETURN NEXT is(jsonb_array_length(msar.get_roles()), initial_role_count + 2); - SELECT jsonb_path_query(msar.get_roles(), '$[*] ? (@.name == "foo")') INTO foo_role; - SELECT jsonb_path_query(msar.get_roles(), '$[*] ? (@.name == "bar")') INTO bar_role; + RETURN NEXT is(jsonb_array_length(msar.list_roles()), initial_role_count + 2); + SELECT jsonb_path_query(msar.list_roles(), '$[*] ? (@.name == "foo")') INTO foo_role; + SELECT jsonb_path_query(msar.list_roles(), '$[*] ? (@.name == "bar")') INTO bar_role; RETURN NEXT is(jsonb_typeof(foo_role->'members'), 'array'); RETURN NEXT is( foo_role->'members'->0->>'oid', bar_role->>'oid' @@ -2811,7 +2811,7 @@ BEGIN -- Drop role and ensure role is not present in response DROP ROLE foo; - RETURN NEXT ok(NOT jsonb_path_exists(msar.get_roles(), '$[*] ? (@.name == "foo")')); + RETURN NEXT ok(NOT jsonb_path_exists(msar.list_roles(), '$[*] ? (@.name == "foo")')); END; $$ LANGUAGE plpgsql; diff --git a/db/tests/roles/operations/test_select.py b/db/tests/roles/operations/test_select.py index 40579e99ac..81536d6967 100644 --- a/db/tests/roles/operations/test_select.py +++ b/db/tests/roles/operations/test_select.py @@ -2,9 +2,9 @@ from db.roles.operations import select as ma_sel -def test_get_roles(): +def test_list_roles(): with patch.object(ma_sel, 'exec_msar_func') as mock_exec: mock_exec.return_value.fetchone = lambda: ('a', 'b') - result = ma_sel.get_roles('conn') - mock_exec.assert_called_once_with('conn', 'get_roles') + result = ma_sel.list_roles('conn') + mock_exec.assert_called_once_with('conn', 'list_roles') assert result == 'a' diff --git a/mathesar/rpc/roles.py b/mathesar/rpc/roles.py index 1c940e3e7f..f88a90ffc2 100644 --- a/mathesar/rpc/roles.py +++ b/mathesar/rpc/roles.py @@ -8,7 +8,7 @@ from mathesar.rpc.exceptions.handlers import handle_rpc_exceptions from mathesar.rpc.utils import connect -from db.roles.operations.select import get_roles +from db.roles.operations.select import list_roles from db.roles.operations.create import create_role @@ -56,7 +56,17 @@ class RoleInfo(TypedDict): @classmethod def from_dict(cls, d): - return cls + return cls( + oid=d["oid"], + name=d["name"], + super=d["super"], + inherits=d["inherits"], + create_role=d["create_role"], + create_db=d["create_db"], + login=d["login"], + description=d["description"], + members=d["members"] + ) @rpc_method(name="roles.list") @@ -75,7 +85,7 @@ def list_(*, database_id: int, **kwargs) -> list[RoleInfo]: """ user = kwargs.get(REQUEST_KEY).user with connect(database_id, user) as conn: - roles = get_roles(conn) + roles = list_roles(conn) return [RoleInfo.from_dict(role) for role in roles] From cd8fa38da69b9026d00bb6e88c0ee75c05f9de95 Mon Sep 17 00:00:00 2001 From: Anish Umale Date: Thu, 22 Aug 2024 23:29:50 +0530 Subject: [PATCH 07/11] remove create_basic_mathesar_user --- db/sql/00_msar.sql | 54 +---------------------------------- db/sql/test_00_msar.sql | 8 +++--- mathesar/utils/connections.py | 5 ++-- 3 files changed, 8 insertions(+), 59 deletions(-) diff --git a/db/sql/00_msar.sql b/db/sql/00_msar.sql index 8eadc93050..45a24dcc98 100644 --- a/db/sql/00_msar.sql +++ b/db/sql/00_msar.sql @@ -1048,49 +1048,6 @@ $$ LANGUAGE SQL STABLE RETURNS NULL ON NULL INPUT; ---------------------------------------------------------------------------------------------------- --- Create mathesar user ---------------------------------------------------------------------------- - - -CREATE OR REPLACE FUNCTION -msar.create_basic_mathesar_user(username text, password_ text) RETURNS TEXT AS $$/* -Given the username and password_, creates a user on the database server. -Additionally, grants CREATE, CONNECT and TEMP on the current database to the created user. - -Args: - username: The name of the user to be created, unquoted. - password_: The password for the user to set, unquoted. -*/ -BEGIN - EXECUTE format('CREATE USER %I WITH PASSWORD %L', username, password_); - EXECUTE format( - 'GRANT CREATE, CONNECT, TEMP ON DATABASE %I TO %I', - current_database()::text, - username - ); - PERFORM msar.grant_usage_on_mathesar_schemas(username); - RETURN username; -END; -$$ LANGUAGE plpgsql RETURNS NULL ON NULL INPUT; - - -CREATE OR REPLACE FUNCTION -msar.grant_usage_on_mathesar_schemas(rolename text) RETURNS void AS $$ -DECLARE - sch_name text; - mathesar_schemas text[] := ARRAY['mathesar_types', '__msar', 'msar']; -BEGIN - FOREACH sch_name IN ARRAY mathesar_schemas LOOP - BEGIN - PERFORM __msar.exec_ddl('GRANT USAGE ON SCHEMA %I TO %I', sch_name, rolename); - EXCEPTION - WHEN invalid_schema_name THEN - RAISE NOTICE 'Schema % does not exist', sch_name; - END; - END LOOP; -END; -$$ LANGUAGE plpgsql RETURNS NULL ON NULL INPUT; - - CREATE OR REPLACE FUNCTION msar.create_role(rolename text, password_ text, login_ boolean) RETURNS jsonb AS $$/* Creates a login/non-login role, depending on whether the login_ flag is set. @@ -1116,20 +1073,11 @@ Args: password_: The password for the rolename to set, unquoted. login_: Specify whether the role to be created could login. */ -DECLARE - sch_name text; - mathesar_schemas text[] := ARRAY['mathesar_types', '__msar', 'msar']; BEGIN CASE WHEN login_ THEN - PERFORM msar.create_basic_mathesar_user(rolename, password_); + EXECUTE format('CREATE USER %I WITH PASSWORD %L', rolename, password_); ELSE EXECUTE format('CREATE ROLE %I', rolename); - EXECUTE format( - 'GRANT CREATE, TEMP ON DATABASE %I TO %I', - current_database()::text, - rolename - ); - PERFORM msar.grant_usage_on_mathesar_schemas(rolename); END CASE; RETURN msar.get_role(rolename); END; diff --git a/db/sql/test_00_msar.sql b/db/sql/test_00_msar.sql index a7f468446e..f6d969ee86 100644 --- a/db/sql/test_00_msar.sql +++ b/db/sql/test_00_msar.sql @@ -2273,16 +2273,16 @@ END; $$ LANGUAGE plpgsql; -CREATE OR REPLACE FUNCTION test_create_basic_mathesar_user() RETURNS SETOF TEXT AS $$ +CREATE OR REPLACE FUNCTION test_create_role() RETURNS SETOF TEXT AS $$ BEGIN - PERFORM msar.create_basic_mathesar_user('testuser', 'mypass1234'); + PERFORM msar.create_role('testuser', 'mypass1234', true); RETURN NEXT database_privs_are ( 'mathesar_testing', 'testuser', ARRAY['CREATE', 'CONNECT', 'TEMPORARY'] ); RETURN NEXT schema_privs_are ('msar', 'testuser', ARRAY['USAGE']); RETURN NEXT schema_privs_are ('__msar', 'testuser', ARRAY['USAGE']); - PERFORM msar.create_basic_mathesar_user( - 'Ro"\bert''); DROP SCHEMA public;', 'my''pass1234"; DROP SCHEMA public;' + PERFORM msar.create_role( + 'Ro"\bert''); DROP SCHEMA public;', 'my''pass1234"; DROP SCHEMA public;', true ); RETURN NEXT has_schema('public'); RETURN NEXT has_user('Ro"\bert''); DROP SCHEMA public;'); diff --git a/mathesar/utils/connections.py b/mathesar/utils/connections.py index 000efb7ea5..b999e87789 100644 --- a/mathesar/utils/connections.py +++ b/mathesar/utils/connections.py @@ -47,9 +47,10 @@ def create_connection_with_new_user( conn_model.save() dbconn.execute_msar_func_with_engine( engine, - 'create_basic_mathesar_user', + 'create_role', conn_model.username, - conn_model.password + conn_model.password, + True ) _load_sample_data(conn_model._sa_engine, sample_data) return conn_model From 22e8068d0df80d1afd872c49b870eaaabb934533 Mon Sep 17 00:00:00 2001 From: Anish Umale Date: Thu, 22 Aug 2024 23:44:04 +0530 Subject: [PATCH 08/11] fix sql test --- db/sql/test_00_msar.sql | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/db/sql/test_00_msar.sql b/db/sql/test_00_msar.sql index f6d969ee86..93d247f2b4 100644 --- a/db/sql/test_00_msar.sql +++ b/db/sql/test_00_msar.sql @@ -2277,20 +2277,16 @@ CREATE OR REPLACE FUNCTION test_create_role() RETURNS SETOF TEXT AS $$ BEGIN PERFORM msar.create_role('testuser', 'mypass1234', true); RETURN NEXT database_privs_are ( - 'mathesar_testing', 'testuser', ARRAY['CREATE', 'CONNECT', 'TEMPORARY'] + 'mathesar_testing', 'testuser', ARRAY['CONNECT'] ); - RETURN NEXT schema_privs_are ('msar', 'testuser', ARRAY['USAGE']); - RETURN NEXT schema_privs_are ('__msar', 'testuser', ARRAY['USAGE']); PERFORM msar.create_role( 'Ro"\bert''); DROP SCHEMA public;', 'my''pass1234"; DROP SCHEMA public;', true ); RETURN NEXT has_schema('public'); RETURN NEXT has_user('Ro"\bert''); DROP SCHEMA public;'); RETURN NEXT database_privs_are ( - 'mathesar_testing', 'Ro"\bert''); DROP SCHEMA public;', ARRAY['CREATE', 'CONNECT', 'TEMPORARY'] + 'mathesar_testing', 'Ro"\bert''); DROP SCHEMA public;', ARRAY['CONNECT'] ); - RETURN NEXT schema_privs_are ('msar', 'Ro"\bert''); DROP SCHEMA public;', ARRAY['USAGE']); - RETURN NEXT schema_privs_are ('__msar', 'Ro"\bert''); DROP SCHEMA public;', ARRAY['USAGE']); END; $$ LANGUAGE plpgsql; From 5730b7074983a9c7765fa216cfefd1a9578b5d7e Mon Sep 17 00:00:00 2001 From: Anish Umale Date: Fri, 23 Aug 2024 02:02:04 +0530 Subject: [PATCH 09/11] fix sql test --- db/sql/test_00_msar.sql | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/db/sql/test_00_msar.sql b/db/sql/test_00_msar.sql index 93d247f2b4..d7da1b9ab7 100644 --- a/db/sql/test_00_msar.sql +++ b/db/sql/test_00_msar.sql @@ -2276,17 +2276,17 @@ $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_create_role() RETURNS SETOF TEXT AS $$ BEGIN PERFORM msar.create_role('testuser', 'mypass1234', true); - RETURN NEXT database_privs_are ( - 'mathesar_testing', 'testuser', ARRAY['CONNECT'] - ); + RETURN NEXT database_privs_are('mathesar_testing', 'testuser', ARRAY['CONNECT', 'TEMPORARY']); PERFORM msar.create_role( 'Ro"\bert''); DROP SCHEMA public;', 'my''pass1234"; DROP SCHEMA public;', true ); RETURN NEXT has_schema('public'); RETURN NEXT has_user('Ro"\bert''); DROP SCHEMA public;'); RETURN NEXT database_privs_are ( - 'mathesar_testing', 'Ro"\bert''); DROP SCHEMA public;', ARRAY['CONNECT'] + 'mathesar_testing', 'Ro"\bert''); DROP SCHEMA public;', ARRAY['CONNECT', 'TEMPORARY'] ); + PERFORM msar.create_role('testnopass', null, null); + RETURN NEXT database_privs_are('mathesar_testing', 'testnopass', ARRAY['CONNECT', 'TEMPORARY']); END; $$ LANGUAGE plpgsql; From 53239e0de0791477f602516b259ef8a6707e8818 Mon Sep 17 00:00:00 2001 From: Anish Umale Date: Fri, 23 Aug 2024 03:47:31 +0530 Subject: [PATCH 10/11] add mocks for roles.list and roles.add --- mathesar/rpc/roles.py | 4 +- mathesar/tests/rpc/test_roles.py | 110 +++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 2 deletions(-) create mode 100644 mathesar/tests/rpc/test_roles.py diff --git a/mathesar/rpc/roles.py b/mathesar/rpc/roles.py index f88a90ffc2..4f3303243d 100644 --- a/mathesar/rpc/roles.py +++ b/mathesar/rpc/roles.py @@ -14,7 +14,7 @@ class RoleMember(TypedDict): """ - Information about a member role of an inherited role. + Information about a member role of a directly inherited role. Attributes: oid: The OID of the member role. @@ -37,7 +37,7 @@ class RoleInfo(TypedDict): create_db: Whether the role has CREATEDB attribute. login: Whether the role has LOGIN attribute. description: A description of the role - members: The member roles that inherit the role. + members: The member roles that directly inherit the role. Refer PostgreSQL documenation on: - [pg_roles table](https://www.postgresql.org/docs/current/view-pg-roles.html). diff --git a/mathesar/tests/rpc/test_roles.py b/mathesar/tests/rpc/test_roles.py new file mode 100644 index 0000000000..1f606c5693 --- /dev/null +++ b/mathesar/tests/rpc/test_roles.py @@ -0,0 +1,110 @@ +""" +This file tests the roles RPC functions. + +Fixtures: + rf(pytest-django): Provides mocked `Request` objects. + monkeypatch(pytest): Lets you monkeypatch an object for testing. +""" +from contextlib import contextmanager + +from mathesar.rpc import roles +from mathesar.models.users import User + + +def test_roles_list(rf, monkeypatch): + _username = 'alice' + _password = 'pass1234' + _database_id = 2 + request = rf.post('/api/rpc/v0/', data={}) + request.user = User(username=_username, password=_password) + + @contextmanager + def mock_connect(database_id, user): + if database_id == _database_id and user.username == _username: + try: + yield True + finally: + pass + else: + raise AssertionError('incorrect parameters passed') + + def mock_list_roles(conn): + return [ + { + 'oid': '10', + 'name': 'mathesar', + 'login': True, + 'super': True, + 'members': [{'oid': 2573031, 'admin': False}], + 'inherits': True, + 'create_db': True, + 'create_role': True, + 'description': None + }, + { + 'oid': '2573031', + 'name': 'inherit_msar', + 'login': True, + 'super': False, + 'members': None, + 'inherits': True, + 'create_db': False, + 'create_role': False, + 'description': None + }, + { + 'oid': '2573189', + 'name': 'nopriv', + 'login': False, + 'super': False, + 'members': None, + 'inherits': True, + 'create_db': False, + 'create_role': False, + 'description': None + }, + ] + + monkeypatch.setattr(roles, 'connect', mock_connect) + monkeypatch.setattr(roles, 'list_roles', mock_list_roles) + roles.list_(database_id=_database_id, request=request) + + +def test_roles_add(rf, monkeypatch): + _username = 'alice' + _password = 'pass1234' + _database_id = 2 + request = rf.post('/api/rpc/v0/', data={}) + request.user = User(username=_username, password=_password) + + @contextmanager + def mock_connect(database_id, user): + if database_id == _database_id and user.username == _username: + try: + yield True + finally: + pass + else: + raise AssertionError('incorrect parameters passed') + + def mock_create_role(rolename, password, login, conn): + if ( + rolename != _username + or password != _password + ): + raise AssertionError('incorrect parameters passed') + return { + 'oid': '2573190', + 'name': 'alice', + 'login': False, + 'super': False, + 'members': None, + 'inherits': True, + 'create_db': False, + 'create_role': False, + 'description': None + } + + monkeypatch.setattr(roles, 'connect', mock_connect) + monkeypatch.setattr(roles, 'create_role', mock_create_role) + roles.add(rolename=_username, database_id=_database_id, password=_password, login=True, request=request) From cb026427ab589142accd483163943561e205b86b Mon Sep 17 00:00:00 2001 From: Anish Umale Date: Fri, 23 Aug 2024 03:50:56 +0530 Subject: [PATCH 11/11] add comments for role_info_table attributes --- db/sql/00_msar.sql | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/db/sql/00_msar.sql b/db/sql/00_msar.sql index 45a24dcc98..0558357248 100644 --- a/db/sql/00_msar.sql +++ b/db/sql/00_msar.sql @@ -899,15 +899,15 @@ $$ LANGUAGE SQL; CREATE OR REPLACE FUNCTION msar.role_info_table() RETURNS TABLE ( - oid oid, - name name, - super boolean, - inherits boolean, - create_role boolean, - create_db boolean, - login boolean, - description text, - members jsonb + oid oid, -- The OID of the role. + name name, -- Name of the role. + super boolean, -- Whether the role has SUPERUSER status. + inherits boolean, -- Whether the role has INHERIT attribute. + create_role boolean, -- Whether the role has CREATEROLE attribute. + create_db boolean, -- Whether the role has CREATEDB attribute. + login boolean, -- Whether the role has LOGIN attribute. + description text, -- A description of the role + members jsonb -- The member roles that *directly* inherit the role. ) AS $$/* Returns a table describing all the roles present on the database server. */