From 8471ed1187a8abc5e28207776c5f49a59ba24b92 Mon Sep 17 00:00:00 2001 From: Frederick Muriuki Muriithi Date: Tue, 25 Apr 2023 09:42:36 +0300 Subject: auth: Roles: Check for editability Some roles should not be user-editable, and as such, we need to check before allowing any edits on such roles. This commit makes that possible. --- gn3/auth/authorisation/groups/models.py | 21 ++++++++++++++++----- gn3/auth/authorisation/groups/views.py | 1 + gn3/auth/authorisation/roles/models.py | 16 +++++++++++++--- tests/unit/auth/fixtures/role_fixtures.py | 5 +++-- tests/unit/auth/test_groups.py | 2 +- tests/unit/auth/test_roles.py | 6 +++--- 6 files changed, 37 insertions(+), 14 deletions(-) diff --git a/gn3/auth/authorisation/groups/models.py b/gn3/auth/authorisation/groups/models.py index bbe4ad6..accf2f2 100644 --- a/gn3/auth/authorisation/groups/models.py +++ b/gn3/auth/authorisation/groups/models.py @@ -15,7 +15,8 @@ from ..checks import authorised_p from ..privileges import Privilege from ..errors import NotFoundError, AuthorisationError, InconsistencyError from ..roles.models import ( - Role, create_role, revoke_user_role_by_name, assign_user_role_by_name) + Role, create_role, check_user_editable, revoke_user_role_by_name, + assign_user_role_by_name) class Group(NamedTuple): """Class representing a group.""" @@ -312,13 +313,19 @@ def __organise_privileges__(acc, row): if role: return { **acc, - role_id: Role(role.role_id, role.role_name, role.privileges + ( - Privilege(row["privilege_id"], row["privilege_description"]),)) + role_id: Role( + role.role_id, role.role_name, + bool(int(row["user_editable"])), + role.privileges + ( + Privilege(row["privilege_id"], + row["privilege_description"]),)) } return { **acc, - role_id: Role(UUID(row["role_id"]), row["role_name"], ( - Privilege(row["privilege_id"], row["privilege_description"]),)) + role_id: Role( + UUID(row["role_id"]), row["role_name"], + bool(int(row["user_editable"])), + (Privilege(row["privilege_id"], row["privilege_description"]),)) } # @authorised_p(("group:role:view",), @@ -351,6 +358,7 @@ def add_privilege_to_group_role(conn: db.DbConnection, group_role: GroupRole, privilege: Privilege) -> GroupRole: """Add `privilege` to `group_role`.""" ## TODO: do privileges check. + check_user_editable(group_role.role) with db.cursor(conn) as cursor: cursor.execute( "INSERT INTO role_privileges(role_id,privilege_id) " @@ -362,12 +370,14 @@ def add_privilege_to_group_role(conn: db.DbConnection, group_role: GroupRole, group_role.group, Role(group_role.role.role_id, group_role.role.role_name, + group_role.role.user_editable, group_role.role.privileges + (privilege,))) def delete_privilege_to_group_role(conn: db.DbConnection, group_role: GroupRole, privilege: Privilege) -> GroupRole: """Delete `privilege` to `group_role`.""" ## TODO: do privileges check. + check_user_editable(group_role.role) with db.cursor(conn) as cursor: cursor.execute( "DELETE FROM role_privileges WHERE " @@ -378,5 +388,6 @@ def delete_privilege_to_group_role(conn: db.DbConnection, group_role: GroupRole, group_role.group, Role(group_role.role.role_id, group_role.role.role_name, + group_role.role.user_editable, tuple(priv for priv in group_role.role.privileges if priv != privilege))) diff --git a/gn3/auth/authorisation/groups/views.py b/gn3/auth/authorisation/groups/views.py index 9e717a9..3f4ced0 100644 --- a/gn3/auth/authorisation/groups/views.py +++ b/gn3/auth/authorisation/groups/views.py @@ -298,6 +298,7 @@ def group_roles(): group, Role(uuid.UUID(row["role_id"]), row["role_name"], + bool(int(row["user_editable"])), tuple())) for row in cursor.fetchall()) return jsonify(tuple( diff --git a/gn3/auth/authorisation/roles/models.py b/gn3/auth/authorisation/roles/models.py index be04985..3c57b9d 100644 --- a/gn3/auth/authorisation/roles/models.py +++ b/gn3/auth/authorisation/roles/models.py @@ -8,6 +8,7 @@ from pymonad.either import Left, Right, Either from gn3.auth import db from gn3.auth.dictify import dictify from gn3.auth.authentication.users import User +from gn3.auth.authorisation.errors import AuthorisationError from ..checks import authorised_p from ..privileges import Privilege @@ -17,6 +18,7 @@ class Role(NamedTuple): """Class representing a role: creates immutable objects.""" role_id: UUID role_name: str + user_editable: bool privileges: tuple[Privilege, ...] def dictify(self) -> dict[str, Any]: @@ -26,6 +28,12 @@ class Role(NamedTuple): "privileges": tuple(dictify(priv) for priv in self.privileges) } +def check_user_editable(role: Role): + """Raise an exception if `role` is not user editable.""" + if not role.user_editable: + raise AuthorisationError( + f"The role `{role.role_name}` is not user editable.") + @authorised_p( privileges = ("group:role:create-role",), error_description="Could not create role") @@ -44,11 +52,11 @@ def create_role( RETURNS: An immutable `gn3.auth.authorisation.roles.Role` object """ - role = Role(uuid4(), role_name, tuple(privileges)) + role = Role(uuid4(), role_name, True, tuple(privileges)) cursor.execute( - "INSERT INTO roles(role_id, role_name) VALUES (?, ?)", - (str(role.role_id), role.role_name)) + "INSERT INTO roles(role_id, role_name, user_editable) VALUES (?, ?, ?)", + (str(role.role_id), role.role_name, (1 if role.user_editable else 0))) cursor.executemany( "INSERT INTO role_privileges(role_id, privilege_id) VALUES (?, ?)", tuple((str(role.role_id), str(priv.privilege_id)) @@ -65,6 +73,7 @@ def __organise_privileges__(roles_dict, privilege_row): role_id_str: Role( UUID(role_id_str), privilege_row["role_name"], + bool(int(privilege_row["user_editable"])), roles_dict[role_id_str].privileges + ( Privilege(privilege_row["privilege_id"], privilege_row["privilege_description"]),)) @@ -75,6 +84,7 @@ def __organise_privileges__(roles_dict, privilege_row): role_id_str: Role( UUID(role_id_str), privilege_row["role_name"], + bool(int(privilege_row["user_editable"])), (Privilege(privilege_row["privilege_id"], privilege_row["privilege_description"]),)) } diff --git a/tests/unit/auth/fixtures/role_fixtures.py b/tests/unit/auth/fixtures/role_fixtures.py index cde3d96..ee86aa2 100644 --- a/tests/unit/auth/fixtures/role_fixtures.py +++ b/tests/unit/auth/fixtures/role_fixtures.py @@ -8,12 +8,13 @@ from gn3.auth.authorisation.roles import Role from gn3.auth.authorisation.privileges import Privilege RESOURCE_READER_ROLE = Role( - uuid.UUID("c3ca2507-ee24-4835-9b31-8c21e1c072d3"), "resource_reader", + uuid.UUID("c3ca2507-ee24-4835-9b31-8c21e1c072d3"), "resource_reader", True, (Privilege("group:resource:view-resource", "view a resource and use it in computations"),)) RESOURCE_EDITOR_ROLE = Role( - uuid.UUID("89819f84-6346-488b-8955-86062e9eedb7"), "resource_editor", ( + uuid.UUID("89819f84-6346-488b-8955-86062e9eedb7"), "resource_editor", True, + ( Privilege("group:resource:view-resource", "view a resource and use it in computations"), Privilege("group:resource:edit-resource", "edit/update a resource"))) diff --git a/tests/unit/auth/test_groups.py b/tests/unit/auth/test_groups.py index 60b67a7..4824e14 100644 --- a/tests/unit/auth/test_groups.py +++ b/tests/unit/auth/test_groups.py @@ -80,7 +80,7 @@ create_role_failure = { UUID("d32611e3-07fc-4564-b56c-786c6db6de2b"), GROUP, Role(UUID("d32611e3-07fc-4564-b56c-786c6db6de2b"), - "ResourceEditor", PRIVILEGES)),)))) + "ResourceEditor", True, PRIVILEGES)),)))) def test_create_group_role(mocker, fxtr_users_in_group, user, expected): """ GIVEN: an authenticated user diff --git a/tests/unit/auth/test_roles.py b/tests/unit/auth/test_roles.py index 0a3ba19..02fd9f7 100644 --- a/tests/unit/auth/test_roles.py +++ b/tests/unit/auth/test_roles.py @@ -27,7 +27,7 @@ PRIVILEGES = ( @pytest.mark.parametrize( "user,expected", tuple(zip(conftest.TEST_USERS[0:1], ( Role(uuid.UUID("d32611e3-07fc-4564-b56c-786c6db6de2b"), "a_test_role", - PRIVILEGES),)))) + True, PRIVILEGES),)))) def test_create_role(# pylint: disable=[too-many-arguments] fxtr_app, auth_testdb_path, mocker, fxtr_users, user, expected):# pylint: disable=[unused-argument] """ @@ -68,7 +68,7 @@ def test_create_role_raises_exception_for_unauthorised_users(# pylint: disable=[ (zip(TEST_USERS, ((Role( role_id=uuid.UUID('a0e67630-d502-4b9f-b23f-6805d0f30e30'), - role_name='group-leader', + role_name='group-leader', user_editable=False, privileges=( Privilege(privilege_id='group:resource:create-resource', privilege_description='Create a resource object'), @@ -108,7 +108,7 @@ def test_create_role_raises_exception_for_unauthorised_users(# pylint: disable=[ privilege_description='List users in the system'))), Role( role_id=uuid.UUID("ade7e6b0-ba9c-4b51-87d0-2af7fe39a347"), - role_name="group-creator", + role_name="group-creator", user_editable=False, privileges=( Privilege(privilege_id='system:group:create-group', privilege_description = "Create a group"),))), -- cgit v1.2.3