about summary refs log tree commit diff
diff options
context:
space:
mode:
authorFrederick Muriuki Muriithi2023-09-13 11:23:45 +0300
committerFrederick Muriuki Muriithi2023-09-26 03:44:30 +0300
commit37771b3be3142f705101beb4c5dc34c1000962f9 (patch)
treefce04cba9f99144d7893d30cd5a4d1ffb8823e8d
parentdd759423739dafebe1d2ce7adb9fc1230ae0ee9d (diff)
downloadgn-auth-37771b3be3142f705101beb4c5dc34c1000962f9.tar.gz
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.
-rw-r--r--gn_auth/auth/authorisation/checks.py9
-rw-r--r--gn_auth/auth/authorisation/groups/models.py39
-rw-r--r--gn_auth/auth/authorisation/resources/data.py7
-rw-r--r--gn_auth/auth/authorisation/resources/genotype_resource.py3
-rw-r--r--gn_auth/auth/authorisation/resources/models.py90
-rw-r--r--gn_auth/auth/authorisation/resources/mrna_resource.py3
-rw-r--r--gn_auth/auth/authorisation/resources/phenotype_resource.py4
-rw-r--r--gn_auth/auth/authorisation/roles/models.py14
-rw-r--r--scripts/migrate_existing_data.py38
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)