From 37771b3be3142f705101beb4c5dc34c1000962f9 Mon Sep 17 00:00:00 2001 From: Frederick Muriuki Muriithi Date: Wed, 13 Sep 2023 11:23:45 +0300 Subject: Remove group from resource objects With the new schema, not all Resource objects are "owned" by a group. Those that are, are linked together through a different db table (`resource_ownership`). This commit removes the `Group` object from `Resource` objects and updates the `resource_ownership` where relevant. --- gn_auth/auth/authorisation/checks.py | 9 +-- gn_auth/auth/authorisation/groups/models.py | 39 +++++++++- gn_auth/auth/authorisation/resources/data.py | 7 +- .../authorisation/resources/genotype_resource.py | 3 +- gn_auth/auth/authorisation/resources/models.py | 90 +++++++++------------- .../auth/authorisation/resources/mrna_resource.py | 3 +- .../authorisation/resources/phenotype_resource.py | 4 +- gn_auth/auth/authorisation/roles/models.py | 14 +++- scripts/migrate_existing_data.py | 38 +++++++-- 9 files changed, 129 insertions(+), 78 deletions(-) diff --git a/gn_auth/auth/authorisation/checks.py b/gn_auth/auth/authorisation/checks.py index 55af0b1..ad71110 100644 --- a/gn_auth/auth/authorisation/checks.py +++ b/gn_auth/auth/authorisation/checks.py @@ -10,7 +10,7 @@ from .errors import InvalidData, AuthorisationError from ..db import sqlite3 as db from ..authentication.oauth2.resource_server import require_oauth -def __system_privileges_in_roles__(conn, user): +def __system_privileges_in_roles__(conn, user): # TODO: Remove this hack. """ This really is a hack since groups are not treated as resources at the moment of writing this. @@ -19,12 +19,11 @@ def __system_privileges_in_roles__(conn, user): """ query = ( "SELECT DISTINCT p.* FROM users AS u " - "INNER JOIN group_user_roles_on_resources AS guror " - "ON u.user_id=guror.user_id " - "INNER JOIN roles AS r ON guror.role_id=r.role_id " + "INNER JOIN user_roles AS ur ON u.user_id=ur.user_id " + "INNER JOIN roles AS r ON ur.role_id=r.role_id " "INNER JOIN role_privileges AS rp ON r.role_id=rp.role_id " "INNER JOIN privileges AS p ON rp.privilege_id=p.privilege_id " - "WHERE u.user_id=? AND p.privilege_id LIKE 'system:%'") + "WHERE u.user_id=? AND p.privilege_id LIKE 'system:%';") with db.cursor(conn) as cursor: cursor.execute(query, (str(user.user_id),)) return (row["privilege_id"] for row in cursor.fetchall()) diff --git a/gn_auth/auth/authorisation/groups/models.py b/gn_auth/auth/authorisation/groups/models.py index 6a39681..c40adbd 100644 --- a/gn_auth/auth/authorisation/groups/models.py +++ b/gn_auth/auth/authorisation/groups/models.py @@ -89,6 +89,19 @@ def create_group( conn: db.DbConnection, group_name: str, group_leader: User, group_description: Optional[str] = None) -> Group: """Create a new group.""" + def resource_category_by_key( + cursor: db.DbCursor, category_key: str): + """Retrieve a resource category by its key.""" + cursor.execute( + "SELECT * FROM resource_categories WHERE " + "resource_category_key=?", + (category_key,)) + results = cursor.fetchone() + if results: + return dict(results) + raise NotFoundError( + f"Could not find a ResourceCategory with key '{category_key}'") + user_groups = user_membership(conn, group_leader) if len(user_groups) > 0: raise MembershipError(group_leader, user_groups) @@ -98,9 +111,29 @@ def create_group( cursor, group_name,( {"group_description": group_description} if group_description else {})) + group_resource = { + "group_id": str(new_group.group_id), + "resource_id": str(uuid4()), + "resource_name": group_name, + "resource_category_id": str( + resource_category_by_key(cursor, "group")["resource_category_id"]), + "public": 0 + } + cursor.execute( + "INSERT INTO resources VALUES " + "(:resource_id, :resource_name, :resource_category_id, :public)", + group_resource) + cursor.execute( + "INSERT INTO group_resources(resource_id, group_id) " + "VALUES(:resource_id, :group_id)", + group_resource) add_user_to_group(cursor, new_group, group_leader) revoke_user_role_by_name(cursor, group_leader, "group-creator") - assign_user_role_by_name(cursor, group_leader, "group-leader") + assign_user_role_by_name( + cursor, + group_leader, + UUID(str(group_resource["resource_id"])), + "group-leader") return new_group @authorised_p(("group:role:create-role",), @@ -208,8 +241,8 @@ def save_group( "VALUES(:group_id, :group_name, :group_metadata) " "ON CONFLICT (group_id) DO UPDATE SET " "group_name=:group_name, group_metadata=:group_metadata"), - {"group_id": str(the_group.group_id), "group_name": the_group.group_name, - "group_metadata": json.dumps(the_group.group_metadata)}) + {"group_id": str(the_group.group_id), "group_name": the_group.group_name, + "group_metadata": json.dumps(the_group.group_metadata)}) return the_group def add_user_to_group(cursor: db.DbCursor, the_group: Group, user: User): diff --git a/gn_auth/auth/authorisation/resources/data.py b/gn_auth/auth/authorisation/resources/data.py index 8f5e625..54c500a 100644 --- a/gn_auth/auth/authorisation/resources/data.py +++ b/gn_auth/auth/authorisation/resources/data.py @@ -22,8 +22,7 @@ def __attach_data__( } organised: dict[UUID, tuple[dict, ...]] = reduce(__organise__, data_rows, {}) return tuple( - Resource( - resource.group, resource.resource_id, resource.resource_name, - resource.resource_category, resource.public, - organised.get(resource.resource_id, tuple())) + Resource(resource.resource_id, resource.resource_name, + resource.resource_category, resource.public, + organised.get(resource.resource_id, tuple())) for resource in resources) diff --git a/gn_auth/auth/authorisation/resources/genotype_resource.py b/gn_auth/auth/authorisation/resources/genotype_resource.py index 03e2e68..206ab61 100644 --- a/gn_auth/auth/authorisation/resources/genotype_resource.py +++ b/gn_auth/auth/authorisation/resources/genotype_resource.py @@ -32,13 +32,12 @@ def link_data_to_resource( """Link Genotype data with a resource.""" with db.cursor(conn) as cursor: params = { - "group_id": str(resource.group.group_id), "resource_id": str(resource.resource_id), "data_link_id": str(data_link_id) } cursor.execute( "INSERT INTO genotype_resources VALUES" - "(:group_id, :resource_id, :data_link_id)", + "(:resource_id, :data_link_id)", params) return params diff --git a/gn_auth/auth/authorisation/resources/models.py b/gn_auth/auth/authorisation/resources/models.py index 783bf8a..f307fdc 100644 --- a/gn_auth/auth/authorisation/resources/models.py +++ b/gn_auth/auth/authorisation/resources/models.py @@ -4,8 +4,6 @@ from uuid import UUID, uuid4 from functools import reduce, partial from typing import Dict, Sequence, Optional -from pymonad.maybe import Just, Maybe, Nothing - from ...db import sqlite3 as db from ...dictify import dictify from ...authentication.users import User @@ -13,8 +11,7 @@ from ...db.sqlite3 import with_db_connection from ..checks import authorised_p from ..errors import NotFoundError, AuthorisationError -from ..groups.models import ( - Group, GroupRole, user_group, group_by_id, is_group_leader) +from ..groups.models import Group, GroupRole, user_group, is_group_leader from .checks import authorised_for from .base import Resource, ResourceCategory @@ -56,14 +53,13 @@ def __assign_resource_owner_role__(cursor, resource, user, group): "role_id": role["role_id"]}) cursor.execute( - "INSERT INTO group_user_roles_on_resources " - "VALUES (" - ":group_id, :user_id, :role_id, :resource_id" - ")", - {"group_id": str(resource.group_id), - "user_id": str(user.user_id), - "role_id": role["role_id"], - "resource_id": str(resource.resource_id)}) + "INSERT INTO user_roles " + "VALUES (:user_id, :role_id, :resource_id)", + { + "user_id": str(user.user_id), + "role_id": role["role_id"], + "resource_id": str(resource.resource_id) + }) @authorised_p(("group:resource:create-resource",), error_description="Insufficient privileges to create a resource", @@ -128,18 +124,8 @@ def public_resources(conn: db.DbConnection) -> Sequence[Resource]: with db.cursor(conn) as cursor: cursor.execute("SELECT * FROM resources WHERE public=1") results = cursor.fetchall() - group_uuids = tuple(row[0] for row in results) - query = ("SELECT * FROM groups WHERE group_id IN " - f"({', '.join(['?'] * len(group_uuids))})") - cursor.execute(query, group_uuids) - groups = { - row[0]: Group( - UUID(row[0]), row[1], json.loads(row[2] or "{}")) - for row in cursor.fetchall() - } return tuple( - Resource(groups[row[0]], UUID(row[1]), row[2], categories[row[3]], - bool(row[4])) + Resource(UUID(row[0]), row[1], categories[row[2]], bool(row[3])) for row in results) def group_leader_resources( @@ -148,11 +134,14 @@ def group_leader_resources( """Return all the resources available to the group leader""" if is_group_leader(conn, user, group): with db.cursor(conn) as cursor: - cursor.execute("SELECT * FROM resources WHERE group_id=?", - (str(group.group_id),)) + cursor.execute( + "SELECT r.* FROM resource_ownership AS ro " + "INNER JOIN resources AS r " + "WHERE ro.group_id=?", + (str(group.group_id),)) return tuple( - Resource(group, UUID(row[1]), row[2], - res_categories[UUID(row[3])], bool(row[4])) + Resource(UUID(row[0]), row[1], + res_categories[UUID(row[2])], bool(row[3])) for row in cursor.fetchall()) return tuple() @@ -166,16 +155,14 @@ def user_resources(conn: db.DbConnection, user: User) -> Sequence[Resource]: gl_resources = group_leader_resources(conn, user, group, categories) cursor.execute( - ("SELECT resources.* FROM group_user_roles_on_resources " - "LEFT JOIN resources " - "ON group_user_roles_on_resources.resource_id=resources.resource_id " - "WHERE group_user_roles_on_resources.group_id = ? " - "AND group_user_roles_on_resources.user_id = ?"), - (str(group.group_id), str(user.user_id))) + ("SELECT resources.* FROM user_roles LEFT JOIN resources " + "ON user_roles.resource_id=resources.resource_id " + "WHERE user_roles.user_id=?"), + (str(user.user_id),)) rows = cursor.fetchall() private_res = tuple( - Resource(group, UUID(row[1]), row[2], categories[UUID(row[3])], - bool(row[4])) + Resource(UUID(row[0]), row[1], categories[UUID(row[2])], + bool(row[3])) for row in rows) return tuple({ res.resource_id: res @@ -216,7 +203,7 @@ def attach_resource_data(cursor: db.DbCursor, resource: Resource) -> Resource: resource_data_function[category.resource_category_key]( cursor, resource.resource_id)) return Resource( - resource.group, resource.resource_id, resource.resource_name, + resource.resource_id, resource.resource_name, resource.resource_category, resource.public, data_rows) def resource_by_id( @@ -235,7 +222,6 @@ def resource_by_id( row = cursor.fetchone() if row: return Resource( - group_by_id(conn, UUID(row["group_id"])), UUID(row["resource_id"]), row["resource_name"], resource_category_by_id(conn, row["resource_category_id"]), bool(int(row["public"]))) @@ -255,11 +241,12 @@ def link_data_to_resource( resource = with_db_connection(partial( resource_by_id, user=user, resource_id=resource_id)) - return { + return {# type: ignore[operator] "mrna": mrna_link_data_to_resource, "genotype": genotype_link_data_to_resource, "phenotype": phenotype_link_data_to_resource, - }[dataset_type.lower()](conn, resource, data_link_id) + }[dataset_type.lower()]( + conn, resource, resource_group(conn, resource), data_link_id) def unlink_data_from_resource( conn: db.DbConnection, user: User, resource_id: UUID, data_link_id: UUID): @@ -319,14 +306,12 @@ def assign_resource_user( """Assign `role` to `user` for the specific `resource`.""" with db.cursor(conn) as cursor: cursor.execute( - "INSERT INTO " - "group_user_roles_on_resources(group_id, user_id, role_id, " - "resource_id) " - "VALUES (?, ?, ?, ?) " - "ON CONFLICT (group_id, user_id, role_id, resource_id) " + "INSERT INTO user_roles(user_id, role_id, resource_id)" + "VALUES (?, ?, ?) " + "ON CONFLICT (user_id, role_id, resource_id) " "DO NOTHING", - (str(resource.group.group_id), str(user.user_id), - str(role.role.role_id), str(resource.resource_id))) + (str(user.user_id), str(role.role.role_id), + str(resource.resource_id))) return { "resource": dictify(resource), "user": dictify(user), @@ -346,10 +331,11 @@ def unassign_resource_user( """Assign `role` to `user` for the specific `resource`.""" with db.cursor(conn) as cursor: cursor.execute( - "DELETE FROM group_user_roles_on_resources " - "WHERE group_id=? AND user_id=? AND role_id=? AND resource_id=?", - (str(resource.group.group_id), str(user.user_id), - str(role.role.role_id), str(resource.resource_id))) + "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))) return { "resource": dictify(resource), "user": dictify(user), @@ -371,12 +357,10 @@ def save_resource( "UPDATE resources SET " "resource_name=:resource_name, " "public=:public " - "WHERE group_id=:group_id " - "AND resource_id=:resource_id", + "WHERE resource_id=:resource_id", { "resource_name": resource.resource_name, "public": 1 if resource.public else 0, - "group_id": str(resource.group.group_id), "resource_id": str(resource.resource_id) }) return resource diff --git a/gn_auth/auth/authorisation/resources/mrna_resource.py b/gn_auth/auth/authorisation/resources/mrna_resource.py index 77295f3..7fce227 100644 --- a/gn_auth/auth/authorisation/resources/mrna_resource.py +++ b/gn_auth/auth/authorisation/resources/mrna_resource.py @@ -30,13 +30,12 @@ def link_data_to_resource( """Link mRNA Assay data with a resource.""" with db.cursor(conn) as cursor: params = { - "group_id": str(resource.group.group_id), "resource_id": str(resource.resource_id), "data_link_id": str(data_link_id) } cursor.execute( "INSERT INTO mrna_resources VALUES" - "(:group_id, :resource_id, :data_link_id)", + "(:resource_id, :data_link_id)", params) return params diff --git a/gn_auth/auth/authorisation/resources/phenotype_resource.py b/gn_auth/auth/authorisation/resources/phenotype_resource.py index cf33579..046357c 100644 --- a/gn_auth/auth/authorisation/resources/phenotype_resource.py +++ b/gn_auth/auth/authorisation/resources/phenotype_resource.py @@ -5,6 +5,7 @@ from typing import Optional, Sequence import sqlite3 import gn_auth.auth.db.sqlite3 as db +from gn_auth.auth.authorisation.groups import Group from .base import Resource from .data import __attach_data__ @@ -27,11 +28,12 @@ def resource_data( def link_data_to_resource( conn: db.DbConnection, resource: Resource, + group: Group, data_link_id: uuid.UUID) -> dict: """Link Phenotype data with a resource.""" with db.cursor(conn) as cursor: params = { - "group_id": str(resource.group.group_id), + "group_id": str(group.group_id), "resource_id": str(resource.resource_id), "data_link_id": str(data_link_id) } diff --git a/gn_auth/auth/authorisation/roles/models.py b/gn_auth/auth/authorisation/roles/models.py index e1b0d6b..579c9dc 100644 --- a/gn_auth/auth/authorisation/roles/models.py +++ b/gn_auth/auth/authorisation/roles/models.py @@ -136,6 +136,9 @@ def assign_default_roles(cursor: db.DbCursor, user: User): def revoke_user_role_by_name(cursor: db.DbCursor, user: User, role_name: str): """Revoke a role from `user` by the role's name""" + # TODO: Pass in the resource_id - this works somewhat correctly, but it's + # only because it is used in for revoking the "group-creator" role so + # far cursor.execute( "SELECT role_id FROM roles WHERE role_name=:role_name", {"role_name": role_name}) @@ -146,7 +149,8 @@ def revoke_user_role_by_name(cursor: db.DbCursor, user: User, role_name: str): "WHERE user_id=:user_id AND role_id=:role_id"), {"user_id": str(user.user_id), "role_id": role["role_id"]}) -def assign_user_role_by_name(cursor: db.DbCursor, user: User, role_name: str): +def assign_user_role_by_name( + cursor: db.DbCursor, user: User, resource_id: UUID, role_name: str): """Revoke a role from `user` by the role's name""" cursor.execute( "SELECT role_id FROM roles WHERE role_name=:role_name", @@ -155,6 +159,10 @@ def assign_user_role_by_name(cursor: db.DbCursor, user: User, role_name: str): if role: cursor.execute( - ("INSERT INTO user_roles VALUES(:user_id, :role_id) " + ("INSERT INTO user_roles VALUES(:user_id, :role_id, :resource_id) " "ON CONFLICT DO NOTHING"), - {"user_id": str(user.user_id), "role_id": role["role_id"]}) + { + "user_id": str(user.user_id), + "role_id": role["role_id"], + "resource_id": str(resource_id) + }) diff --git a/scripts/migrate_existing_data.py b/scripts/migrate_existing_data.py index 7cd6086..db3f426 100644 --- a/scripts/migrate_existing_data.py +++ b/scripts/migrate_existing_data.py @@ -86,10 +86,31 @@ def admin_group(conn: authdb.DbConnection, admin: User) -> Group: "Existing data was migrated into this group and assigned " "to publicly visible resources according to type.") }) + + cursor.execute( + "SELECT * FROM resource_categories WHERE " + "resource_category_key='group'") + res_cat_id = cursor.fetchone()["resource_category_id"] + grp_res = { + "group_id": str(new_group.group_id), + "resource_id": str(uuid4()), + "resource_name": new_group.group_name, + "resource_category_id": res_cat_id, + "public": 0 + } + cursor.execute( + "INSERT INTO resources VALUES " + "(:resource_id, :resource_name, :resource_category_id, :public)", + grp_res) + cursor.execute( + "INSERT INTO group_resources(resource_id, group_id) " + "VALUES(:resource_id, :group_id)", + grp_res) cursor.execute("INSERT INTO group_users VALUES (?, ?)", (str(new_group.group_id), str(admin.user_id))) revoke_user_role_by_name(cursor, admin, "group-creator") - assign_user_role_by_name(cursor, admin, "group-leader") + assign_user_role_by_name( + cursor, admin, UUID(grp_res["resource_id"]), "group-leader") return new_group def __resource_category_by_key__( @@ -110,7 +131,7 @@ def __create_resources__(cursor: authdb.DbCursor, group: Group) -> tuple[ Resource, ...]: """Create default resources.""" resources = tuple(Resource( - group, uuid4(), name, __resource_category_by_key__(cursor, catkey), + uuid4(), name, __resource_category_by_key__(cursor, catkey), True, tuple() ) for name, catkey in ( ("mRNA-euhrin", "mrna"), @@ -125,6 +146,12 @@ def __create_resources__(cursor: authdb.DbCursor, group: Group) -> tuple[ "rcid": str(res.resource_category.resource_category_id), "pub": 1 } for res in resources)) + cursor.executemany("INSERT INTO resource_ownership(group_id, resource_id) " + "VALUES (:group_id, :resource_id)", + tuple({ + "group_id": str(group.group_id), + "resource_id": str(resource.resource_id) + } for resource in resources)) return resources def default_resources(conn: authdb.DbConnection, group: Group) -> tuple[ @@ -143,7 +170,6 @@ def default_resources(conn: authdb.DbConnection, group: Group) -> tuple[ return __create_resources__(cursor, group) return tuple(Resource( - group, UUID(row["resource_id"]), row["resource_name"], ResourceCategory( @@ -360,12 +386,14 @@ def entry(authdbpath, mysqldburi): with (authdb.connection(authdbpath) as authconn, biodb.database_connection(mysqldburi) as bioconn): admin = select_sys_admin(sys_admins(authconn)) + the_admin_group = admin_group(authconn, admin) resources = default_resources( - authconn, admin_group(authconn, admin)) + authconn, the_admin_group) for resource in resources: assign_data_to_resource(authconn, bioconn, resource) with authdb.cursor(authconn) as cursor: - __assign_resource_owner_role__(cursor, resource, admin) + __assign_resource_owner_role__( + cursor, resource, admin, the_admin_group) except DataNotFound as dnf: print(dnf.args[0], file=sys.stderr) sys.exit(1) -- cgit v1.2.3