From c8658a764cf30605c6e5a1361dcd09ddd422e371 Mon Sep 17 00:00:00 2001 From: Frederick Muriuki Muriithi Date: Tue, 25 Jun 2024 13:15:04 -0500 Subject: Roles: Get rid of use of GroupRole; use Role directly for resources The GroupRole idea was flawed, and led to a critical bug that would have allowed privilege escalation. This uses the Role directly acting on a specific resource when assigning said role to a user. --- gn_auth/auth/authorisation/resources/models.py | 37 ++++++++++++-------------- gn_auth/auth/authorisation/resources/views.py | 34 +++++++++++++---------- 2 files changed, 37 insertions(+), 34 deletions(-) (limited to 'gn_auth/auth/authorisation') diff --git a/gn_auth/auth/authorisation/resources/models.py b/gn_auth/auth/authorisation/resources/models.py index 94e817d..c7c8352 100644 --- a/gn_auth/auth/authorisation/resources/models.py +++ b/gn_auth/auth/authorisation/resources/models.py @@ -17,7 +17,7 @@ from gn_auth.auth.errors import NotFoundError, AuthorisationError from .checks import authorised_for from .base import Resource, ResourceCategory -from .groups.models import Group, GroupRole, user_group, is_group_leader +from .groups.models import Group, user_group, is_group_leader from .mrna import ( resource_data as mrna_resource_data, attach_resources_data as mrna_attach_resources_data, @@ -293,13 +293,13 @@ def attach_resources_data( for category, rscs in organised.items()) for resource in categories) -@authorised_p( - ("group:user:assign-role",), - "You cannot assign roles to users for this group.", - oauth2_scope="profile group role resource") + def assign_resource_user( - conn: db.DbConnection, resource: Resource, user: User, - role: GroupRole) -> dict: + conn: db.DbConnection, + resource: Resource, + user: User, + role: Role +) -> dict: """Assign `role` to `user` for the specific `resource`.""" with db.cursor(conn) as cursor: cursor.execute( @@ -307,39 +307,36 @@ def assign_resource_user( "VALUES (?, ?, ?) " "ON CONFLICT (user_id, role_id, resource_id) " "DO NOTHING", - (str(user.user_id), str(role.role.role_id), - str(resource.resource_id))) + (str(user.user_id), str(role.role_id), str(resource.resource_id))) return { "resource": asdict(resource), "user": asdict(user), "role": asdict(role), "description": ( f"The user '{user.name}'({user.email}) was assigned the " - f"'{role.role.role_name}' role on resource with ID " + f"'{role.role_name}' role on resource with ID " f"'{resource.resource_id}'.")} -@authorised_p( - ("group:user:assign-role",), - "You cannot assign roles to users for this group.", - oauth2_scope="profile group role resource") + def unassign_resource_user( - conn: db.DbConnection, resource: Resource, user: User, - role: GroupRole) -> dict: + conn: db.DbConnection, + resource: Resource, + user: User, + role: Role +) -> dict: """Assign `role` to `user` for the specific `resource`.""" with db.cursor(conn) as cursor: cursor.execute( "DELETE FROM user_roles " "WHERE user_id=? AND role_id=? AND resource_id=?", - (str(user.user_id), - str(role.role.role_id), - str(resource.resource_id))) + (str(user.user_id), str(role.role_id), str(resource.resource_id))) return { "resource": asdict(resource), "user": asdict(user), "role": asdict(role), "description": ( f"The user '{user.name}'({user.email}) had the " - f"'{role.role.role_name}' role on resource with ID " + f"'{role.role_name}' role on resource with ID " f"'{resource.resource_id}' taken away.")} def save_resource( diff --git a/gn_auth/auth/authorisation/resources/views.py b/gn_auth/auth/authorisation/resources/views.py index cf9ebc4..2eda72b 100644 --- a/gn_auth/auth/authorisation/resources/views.py +++ b/gn_auth/auth/authorisation/resources/views.py @@ -247,22 +247,25 @@ def resource_users(resource_id: UUID): @require_oauth("profile group resource role") def assign_role_to_user(resource_id: UUID) -> Response: """Assign a role on the specified resource to a user.""" - with require_oauth.acquire("profile group resource role") as the_token: + with require_oauth.acquire("profile group resource role") as _token: try: form = request_json() - group_role_id = form.get("group_role_id", "") + role_id = form.get("role_id", "") user_email = form.get("user_email", "") - assert bool(group_role_id), "The role must be provided." + assert bool(role_id), "The role must be provided." assert bool(user_email), "The user email must be provided." def __assign__(conn: db.DbConnection) -> dict: - resource = resource_by_id(conn, the_token.user, resource_id) + authorised_for( + conn, + _token.user, + ("resource:role:assign-role",), + (resource_id,)) + resource = resource_by_id(conn, _token.user, resource_id) user = user_by_email(conn, user_email) return assign_resource_user( conn, resource, user, - group_role_by_id(conn, - resource_owner(conn, resource), - UUID(group_role_id))) + role_by_id(conn, UUID(role_id))) except AssertionError as aserr: raise AuthorisationError(aserr.args[0]) from aserr @@ -272,21 +275,24 @@ def assign_role_to_user(resource_id: UUID) -> Response: @require_oauth("profile group resource role") def unassign_role_to_user(resource_id: UUID) -> Response: """Unassign a role on the specified resource from a user.""" - with require_oauth.acquire("profile group resource role") as the_token: + with require_oauth.acquire("profile group resource role") as _token: try: form = request_json() - group_role_id = form.get("group_role_id", "") + role_id = form.get("role_id", "") user_id = form.get("user_id", "") - assert bool(group_role_id), "The role must be provided." + assert bool(role_id), "The role must be provided." assert bool(user_id), "The user id must be provided." def __assign__(conn: db.DbConnection) -> dict: - resource = resource_by_id(conn, the_token.user, resource_id) + authorised_for( + conn, + _token.user, + ("resource:role:assign-role",), + (resource_id,)) + resource = resource_by_id(conn, _token.user, resource_id) return unassign_resource_user( conn, resource, user_by_id(conn, UUID(user_id)), - group_role_by_id(conn, - resource_owner(conn, resource), - UUID(group_role_id))) + role_by_id(conn, UUID(role_id))) except AssertionError as aserr: raise AuthorisationError(aserr.args[0]) from aserr -- cgit v1.2.3