From 25da0232ac52509d6761e36ad80ed53b8dbbb64e Mon Sep 17 00:00:00 2001
From: Frederick Muriuki Muriithi
Date: Wed, 16 Nov 2022 12:50:47 +0300
Subject: auth: fix bugs in the code

* gn3/auth/authorisation/privileges.py: Set id to UUID type
* gn3/auth/authorisation/roles.py: fix parameters to types that sqlite3
  supports
* gn3/auth/db.py: add logging for errors and re-raise the exception
* tests/unit/auth/test_roles.py: fix test
---
 gn3/auth/authorisation/privileges.py |  2 +-
 gn3/auth/authorisation/roles.py      |  7 ++++---
 gn3/auth/db.py                       | 12 ++++++++++--
 tests/unit/auth/test_roles.py        | 15 +++++++++------
 4 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/gn3/auth/authorisation/privileges.py b/gn3/auth/authorisation/privileges.py
index c60a58c..09439ad 100644
--- a/gn3/auth/authorisation/privileges.py
+++ b/gn3/auth/authorisation/privileges.py
@@ -21,4 +21,4 @@ def user_privileges(conn: db.DbConnection, user_id: UUID) -> Iterable[Privilege]
             (str(user_id),))
         results = cursor.fetchall()
 
-    return (Privilege(row[0], row[1]) for row in results)
+    return (Privilege(UUID(row[0]), row[1]) for row in results)
diff --git a/gn3/auth/authorisation/roles.py b/gn3/auth/authorisation/roles.py
index 7c33ab3..8435c40 100644
--- a/gn3/auth/authorisation/roles.py
+++ b/gn3/auth/authorisation/roles.py
@@ -33,9 +33,10 @@ def create_role(
 
     cursor.execute(
         "INSERT INTO roles(role_id, role_name) VALUES (?, ?)",
-        (role.role_id, role.role_name))
-    cursor.execute(
+        (str(role.role_id), role.role_name))
+    cursor.executemany(
         "INSERT INTO role_privileges(role_id, privilege_id) VALUES (?, ?)",
-        ((role.role_id, priv.privilege_id) for priv in privileges))
+        tuple((str(role.role_id), str(priv.privilege_id))
+              for priv in privileges))
 
     return role
diff --git a/gn3/auth/db.py b/gn3/auth/db.py
index 8760153..e0e009c 100644
--- a/gn3/auth/db.py
+++ b/gn3/auth/db.py
@@ -3,6 +3,10 @@ import sqlite3
 import contextlib
 from typing import Any, Iterator, Protocol
 
+import traceback
+
+from flask import current_app as app
+
 class DbConnection(Protocol):
     """Type annotation for a generic database connection object."""
     def cursor(self) -> Any:
@@ -48,8 +52,10 @@ def connection(db_path: str) -> Iterator[DbConnection]:
     conn = sqlite3.connect(db_path)
     try:
         yield conn
-    except: # pylint: disable=bare-except
+    except sqlite3.Error as exc:
         conn.rollback()
+        app.logger.debug(traceback.format_exc())
+        raise exc
     finally:
         conn.commit()
         conn.close()
@@ -60,8 +66,10 @@ def cursor(conn: DbConnection) -> Iterator[DbCursor]:
     cur = conn.cursor()
     try:
         yield cur
-    except: # pylint: disable=bare-except
+    except sqlite3.Error as exc:
         conn.rollback()
+        app.logger.debug(traceback.format_exc())
+        raise exc
     finally:
         conn.commit()
         cur.close()
diff --git a/tests/unit/auth/test_roles.py b/tests/unit/auth/test_roles.py
index 091a8b0..70663b3 100644
--- a/tests/unit/auth/test_roles.py
+++ b/tests/unit/auth/test_roles.py
@@ -15,8 +15,10 @@ create_role_failure = {
 uuid_fn = lambda : uuid.UUID("d32611e3-07fc-4564-b56c-786c6db6de2b")
 
 PRIVILEGES = (
-    Privilege(uuid.uuid4(), "view-resource"),
-    Privilege(uuid.uuid4(), "edit-resource"))
+    Privilege(uuid.UUID("7f261757-3211-4f28-a43f-a09b800b164d"),
+              "view-resource"),
+    Privilege(uuid.UUID("2f980855-959b-4339-b80e-25d1ec286e21"),
+              "edit-resource"))
 
 @pytest.mark.unit_test
 @pytest.mark.parametrize(
@@ -28,7 +30,7 @@ PRIVILEGES = (
     ("ae9c6245-0966-41a5-9a5e-20885a96bea7", create_role_failure),
     ("9a0c7ce5-2f40-4e78-979e-bf3527a59579", create_role_failure),
     ("e614247d-84d2-491d-a048-f80b578216cb", create_role_failure)))
-def test_create_group(# pylint: disable=[too-many-arguments]
+def test_create_role(# pylint: disable=[too-many-arguments]
         test_app, auth_testdb_path, mocker, test_users, user_id, expected):# pylint: disable=[unused-argument]
     """
     GIVEN: an authenticated user
@@ -36,8 +38,9 @@ def test_create_group(# pylint: disable=[too-many-arguments]
     THEN: verify they are only able to create the role if they have the
           appropriate privileges
     """
-    mocker.patch("gn3.auth.authorisation.groups.uuid4", uuid_fn)
+    mocker.patch("gn3.auth.authorisation.roles.uuid4", uuid_fn)
     with test_app.app_context() as flask_context:
         flask_context.g.user_id = uuid.UUID(user_id)
-        with db.connection(auth_testdb_path) as conn:
-            assert create_role(conn, "a_test_role") == expected
+        with db.connection(auth_testdb_path) as conn, db.cursor(conn) as cursor:
+            the_role = create_role(cursor, "a_test_role", PRIVILEGES)
+            assert the_role == expected
-- 
cgit v1.2.3