Access (auth) refactor & database transactions

src/app/api/crud/__init__.py

@@ -0,0 +1 @@
+from .base import *

src/app/api/crud/accesses.py

@@ -0,0 +1,37 @@
+from typing import Dict, Any
+from fastapi import HTTPException
+from app.api import crud, security
+from app.db import accesses
+from app.api.schemas import AccessRead, AccessCreation, Cred, AccessBase, CredHash
+
+
+async def post_access(login: str, password: str, scopes: str) -> AccessRead:
+    """Assert login does not exist, hash password, post entry into accesses table"""
+    if await crud.fetch_one(accesses, [('login', login)]) is not None:
+        raise HTTPException(
+            status_code=400,
+            detail=f"An entry with login='{login}' already exists.",
+        )
+
+    pwd = await security.hash_password(password)
+    access = AccessCreation(login=login, hashed_password=pwd, scopes=scopes)
+    entry = await crud.create_entry(accesses, access)
+    return AccessRead(**entry)
+
+
+async def update_access_login(payload: AccessBase, entry_id: int) -> Dict[str, Any]:
+    # TODO check that works
+    entry = await crud.update_entry(accesses, payload, entry_id)
+    # Return non-sensitive information
+    return {"login": entry["login"]}
+
+
+async def update_access_pwd(payload: Cred, entry_id: int) -> Dict[str, Any]:
+    """hash new password & update entry"""
+    pwd = await security.hash_password(payload.password)
+
+    updated_payload = CredHash(hashed_password=pwd)
+    entry = await crud.update_entry(accesses, updated_payload, entry_id)
+
+    # Return non-sensitive information
+    return {"login": entry["login"]}

src/app/api/crud.py → src/app/api/crud/base.py

@@ -1,9 +1,10 @@
 from typing import Optional, Tuple, Any, List, Dict
-from app.db import database
 from sqlalchemy import Table
 from pydantic import BaseModel
 from fastapi import HTTPException, Path
 
+from app.db import database
+
 
 async def post(payload: BaseModel, table: Table) -> int:
     query = table.insert().values(**payload.dict())

src/app/api/routes/accesses.py

@@ -1,44 +1,15 @@
-from typing import List, Dict, Any
-from fastapi import APIRouter, Path, HTTPException
-from app.api import crud, security
+"""Access route read-only for debugging purpose, aims to be removed"""
+
+from typing import List
+from fastapi import APIRouter, Path
+from app.api import crud
 from app.db import accesses
-from app.api.schemas import AccessBase, AccessRead, AccessAuth, AccessCreation, Cred, CredHash
+from app.api.schemas import AccessRead
 
 
 router = APIRouter()
 
 
-async def post_access(login: str, password: str, scopes: str) -> AccessRead:
-    # Check that the login does not already exist
-    if await crud.fetch_one(accesses, [('login', login)]) is not None:
-        raise HTTPException(
-            status_code=400,
-            detail=f"An entry with login='{login}' already exists.",
-        )
-    # Hash the password
-    pwd = await security.hash_password(password)
-    access = AccessCreation(login=login, hashed_password=pwd, scopes=scopes)
-    entry = await crud.create_entry(accesses, access)
-
-    return AccessRead(**entry)
-
-
-async def update_access_pwd(payload: Cred, entry_id: int = Path(..., gt=0)) -> Dict[str, Any]:
-    entry = await crud.get_entry(accesses, entry_id)
-    # Hash the password
-    pwd = await security.hash_password(payload.password)
-    # Update the password
-    payload = CredHash(hashed_password=pwd)
-    await crud.update_entry(accesses, payload, entry_id)
-    # Return non-sensitive information
-    return {"login": entry["login"]}
-
-
-@router.post("/", response_model=AccessRead, status_code=201)
-async def create_access(payload: AccessAuth):
-    return await post_access(payload.login, payload.password, payload.scopes)
-
-
 @router.get("/{access_id}/", response_model=AccessRead)
 async def get_access(access_id: int = Path(..., gt=0)):
     return await crud.get_entry(accesses, access_id)
@@ -47,13 +18,3 @@ async def get_access(access_id: int = Path(..., gt=0)):
 @router.get("/", response_model=List[AccessRead])
 async def fetch_accesses():
     return await crud.fetch_all(accesses)
-
-
-@router.put("/{access_id}/", response_model=AccessRead)
-async def update_access(payload: AccessBase, access_id: int = Path(..., gt=0)):
-    return await crud.update_entry(accesses, payload, access_id)
-
-
-@router.delete("/{access_id}/", response_model=AccessRead)
-async def delete_access(access_id: int = Path(..., gt=0)):
-    return await crud.delete_entry(accesses, access_id)

src/app/api/routes/devices.py

@@ -3,17 +3,20 @@
 from fastapi import APIRouter, Path, Security, HTTPException
 
 from app.api import crud
-from app.db import devices
+from app.db import database, devices
 from app.api.schemas import DeviceOut, DeviceAuth, DeviceCreation, DeviceIn, UserRead, DefaultPosition, Cred
 from app.api.deps import get_current_device, get_current_user
 
-from app.api.routes.accesses import post_access, update_access_pwd
+from app.api.crud.accesses import post_access, update_access_pwd
+
 
 router = APIRouter()
 
 
 @router.post("/", response_model=DeviceOut, status_code=201)
+@database.transaction()
 async def create_device(payload: DeviceAuth, _=Security(get_current_user, scopes=["admin"])):
+    """Use transaction to insert access/device."""
     access_entry = await post_access(payload.login, payload.password, scopes=payload.scopes)
     return await crud.create_entry(devices, DeviceCreation(**payload.dict(), access_id=access_entry.id))
 

src/app/api/routes/users.py

@@ -3,12 +3,10 @@
 from fastapi import APIRouter, Path, Security
 
 from app.api import crud
-from app.db import users
+from app.db import database, users
 from app.api.schemas import UserInfo, UserCreation, Cred, UserRead, UserAuth
 from app.api.deps import get_current_user
 
-from app.api.routes.accesses import post_access, update_access_pwd
-
 
 router = APIRouter()
 
@@ -26,14 +24,15 @@ async def update_my_info(payload: UserInfo, me: UserRead = Security(get_current_
 @router.put("/update-pwd", response_model=UserInfo)
 async def update_my_password(payload: Cred, me: UserRead = Security(get_current_user, scopes=["me"])):
     entry = await crud.get_entry(users, me.id)
-    await update_access_pwd(payload, entry["access_id"])
+    await crud.update_access_pwd(payload, entry["access_id"])
     return entry
 
 
 @router.post("/", response_model=UserRead, status_code=201)
+@database.transaction()
 async def create_user(payload: UserAuth, _=Security(get_current_user, scopes=["admin"])):
-    # Create a new access
-    access_entry = await post_access(payload.login, payload.password, payload.scopes)
+    """Use transaction to insert access/user."""
+    access_entry = await crud.accesses.post_access(payload.login, payload.password, payload.scopes)
     return await crud.create_entry(users, UserCreation(login=payload.login, access_id=access_entry.id))
 
 

src/app/db/tables.py

@@ -28,7 +28,7 @@
     "accesses",
     metadata,
     Column("id", Integer, primary_key=True),
-    Column("login", String(50)),
+    Column("login", String(50), index=True, unique=True),  # index for fast lookup
     Column("hashed_password", String(70), nullable=False),
     Column("scopes", String(30), default="me", nullable=False),
 )
@@ -75,7 +75,7 @@ class EventType(str, enum.Enum):
     Column("id", Integer, primary_key=True),
     Column("login", String(50)),
     Column("owner_id", Integer, ForeignKey("users.id")),
-    Column("access_id", Integer, ForeignKey("accesses.id"), unique=True),
+    Column("access_id", Integer, ForeignKey("accesses.id", ondelete="CASCADE"), unique=True),
     Column("specs", String(50)),
     Column("elevation", Float(1, asdecimal=True), default=None, nullable=True),
     Column("lat", Float(4, asdecimal=True), default=None, nullable=True),

src/tests/test_accesses.py

@@ -1,8 +1,7 @@
-import json
 import pytest
 from copy import deepcopy
 
-from app.api import crud
+from app.api.crud import base
 from app.api.routes import accesses
 
 
@@ -16,12 +15,9 @@ def _patch_session(monkeypatch, mock_table):
     # DB patching
     monkeypatch.setattr(accesses, "accesses", mock_table)
     # Sterilize all DB interactions through CRUD override
-    monkeypatch.setattr(crud, "get", pytest.mock_get)
-    monkeypatch.setattr(crud, "fetch_all", pytest.mock_fetch_all)
-    monkeypatch.setattr(crud, "fetch_one", pytest.mock_fetch_one)
-    monkeypatch.setattr(crud, "post", pytest.mock_post)
-    monkeypatch.setattr(crud, "put", pytest.mock_put)
-    monkeypatch.setattr(crud, "delete", pytest.mock_delete)
+    monkeypatch.setattr(base, "get", pytest.mock_get)
+    monkeypatch.setattr(base, "fetch_all", pytest.mock_fetch_all)
+    monkeypatch.setattr(base, "fetch_one", pytest.mock_fetch_one)
 
 
 def test_get_access(test_app, monkeypatch):
@@ -62,100 +58,3 @@ def test_fetch_accesses(test_app, monkeypatch):
     assert response.status_code == 200
     assert response.json() == [{k: v for k, v in entry.items() if k != "hashed_password"}
                                for entry in mock_access_table]
-
-
-def test_create_access(test_app, monkeypatch):
-
-    # Sterilize DB interactions
-    mock_access_table = deepcopy(ACCESS_TABLE)
-    _patch_session(monkeypatch, mock_access_table)
-
-    test_payload = {"login": "third_login", "scopes": "me", "password": "PickARobustOne"}
-    test_response = {"id": len(mock_access_table) + 1, **test_payload}
-
-    response = test_app.post("/accesses/", data=json.dumps(test_payload))
-
-    assert response.status_code == 201
-    assert response.json() == {k: v for k, v in test_response.items() if k != "password"}
-
-
-@pytest.mark.parametrize(
-    "payload, status_code, status_details",
-    [
-        [{"login": "first_login", "password": "PickARobustOne", "scopes": "me"}, 400,
-         "An entry with login='first_login' already exists."],
-        [{"login": "third_login", "scopes": "me", "hashed_password": "PickARobustOne"}, 422, None],
-        [{"login": 1, "scopes": "me", "password": "PickARobustOne"}, 422, None],
-        [{"login": "third_login", "scopes": "me"}, 422, None],
-    ],
-)
-def test_create_access_invalid(test_app, monkeypatch, payload, status_code, status_details):
-    # Sterilize DB interactions
-    mock_access_table = deepcopy(ACCESS_TABLE)
-    _patch_session(monkeypatch, mock_access_table)
-
-    response = test_app.post("/accesses/", data=json.dumps(payload))
-    assert response.status_code == status_code, print(payload)
-    if isinstance(status_details, str):
-        assert response.json()["detail"] == status_details, print(payload)
-
-
-def test_update_access(test_app, monkeypatch):
-    # Sterilize DB interactions
-    mock_access_table = deepcopy(ACCESS_TABLE)
-    _patch_session(monkeypatch, mock_access_table)
-
-    test_payload = {"login": "first_login", "scopes": "me", "password": "PickAnotherRobustOne"}
-    response = test_app.put("/accesses/1/", data=json.dumps(test_payload))
-    assert response.status_code == 200
-    for k, v in mock_access_table[0].items():
-        assert v == test_payload.get(k, ACCESS_TABLE[0][k])
-
-
-@pytest.mark.parametrize(
-    "access_id, payload, status_code",
-    [
-        [1, {}, 422],
-        [1, {"login": "first_login"}, 422],
-        [999, {"login": "first_login", "scopes": "me", "password": "PickAnotherRobustOne"}, 404],
-        [1, {"login": 1, "scopes": "me", "password": "PickAnotherRobustOne"}, 422],
-        [0, {"login": "first_login", "scopes": "me", "password": "PickAnotherRobustOne"}, 422],
-    ],
-)
-def test_update_access_invalid(test_app, monkeypatch, access_id, payload, status_code):
-    # Sterilize DB interactions
-    mock_access_table = deepcopy(ACCESS_TABLE)
-    _patch_session(monkeypatch, mock_access_table)
-
-    response = test_app.put(f"/accesses/{access_id}/", data=json.dumps(payload))
-    assert response.status_code == status_code, print(payload)
-
-
-def test_delete_access(test_app, monkeypatch):
-    # Sterilize DB interactions
-    mock_access_table = deepcopy(ACCESS_TABLE)
-    _patch_session(monkeypatch, mock_access_table)
-
-    response = test_app.delete("/accesses/1/")
-    assert response.status_code == 200
-    assert response.json() == {k: v for k, v in ACCESS_TABLE[0].items() if k != "hashed_password"}
-    for entry in mock_access_table:
-        assert entry['id'] != 1
-
-
-@pytest.mark.parametrize(
-    "access_id, status_code, status_details",
-    [
-        [999, 404, "Entry not found"],
-        [0, 422, None],
-    ],
-)
-def test_delete_access_invalid(test_app, monkeypatch, access_id, status_code, status_details):
-    # Sterilize DB interactions
-    mock_access_table = deepcopy(ACCESS_TABLE)
-    _patch_session(monkeypatch, mock_access_table)
-
-    response = test_app.delete(f"/accesses/{access_id}/")
-    assert response.status_code == status_code, print(access_id)
-    if isinstance(status_details, str):
-        assert response.json()["detail"] == status_details, print(access_id)

src/tests/test_alerts.py

@@ -3,7 +3,7 @@
 from copy import deepcopy
 from datetime import datetime
 
-from app.api import crud
+from app.api.crud import base
 from app.api.routes import alerts
 
 
@@ -21,11 +21,11 @@ def _patch_session(monkeypatch, mock_table):
     # DB patching
     monkeypatch.setattr(alerts, "alerts", mock_table)
     # Sterilize all DB interactions through CRUD override
-    monkeypatch.setattr(crud, "get", pytest.mock_get)
-    monkeypatch.setattr(crud, "fetch_all", pytest.mock_fetch_all)
-    monkeypatch.setattr(crud, "post", pytest.mock_post)
-    monkeypatch.setattr(crud, "put", pytest.mock_put)
-    monkeypatch.setattr(crud, "delete", pytest.mock_delete)
+    monkeypatch.setattr(base, "get", pytest.mock_get)
+    monkeypatch.setattr(base, "fetch_all", pytest.mock_fetch_all)
+    monkeypatch.setattr(base, "post", pytest.mock_post)
+    monkeypatch.setattr(base, "put", pytest.mock_put)
+    monkeypatch.setattr(base, "delete", pytest.mock_delete)
 
 
 def test_get_alert(test_app, monkeypatch):

src/tests/test_devices.py

@@ -4,7 +4,8 @@
 from datetime import datetime
 from fastapi import HTTPException
 
-from app.api import crud, security
+from app.api import security
+from app.api.crud import base
 from app.api.routes import devices
 from app.api.schemas import AccessRead, AccessCreation
 
@@ -30,12 +31,12 @@ def _patch_session(monkeypatch, mock_device_table, mock_access_table=None):
     # DB patching
     monkeypatch.setattr(devices, "devices", mock_device_table)
     # Sterilize all DB interactions through CRUD override
-    monkeypatch.setattr(crud, "get", pytest.mock_get)
-    monkeypatch.setattr(crud, "fetch_all", pytest.mock_fetch_all)
-    monkeypatch.setattr(crud, "fetch_one", pytest.mock_fetch_one)
-    monkeypatch.setattr(crud, "post", pytest.mock_post)
-    monkeypatch.setattr(crud, "put", pytest.mock_put)
-    monkeypatch.setattr(crud, "delete", pytest.mock_delete)
+    monkeypatch.setattr(base, "get", pytest.mock_get)
+    monkeypatch.setattr(base, "fetch_all", pytest.mock_fetch_all)
+    monkeypatch.setattr(base, "fetch_one", pytest.mock_fetch_one)
+    monkeypatch.setattr(base, "post", pytest.mock_post)
+    monkeypatch.setattr(base, "put", pytest.mock_put)
+    monkeypatch.setattr(base, "delete", pytest.mock_delete)
     # Password
     monkeypatch.setattr(security, "hash_password", pytest.mock_hash_password)
     # Access table specific

src/tests/test_events.py

@@ -3,7 +3,7 @@
 from copy import deepcopy
 from datetime import datetime
 
-from app.api import crud
+from app.api.crud import base
 from app.api.routes import events
 
 
@@ -19,11 +19,11 @@ def _patch_session(monkeypatch, mock_table):
     # DB patching
     monkeypatch.setattr(events, "events", mock_table)
     # Sterilize all DB interactions through CRUD override
-    monkeypatch.setattr(crud, "get", pytest.mock_get)
-    monkeypatch.setattr(crud, "fetch_all", pytest.mock_fetch_all)
-    monkeypatch.setattr(crud, "post", pytest.mock_post)
-    monkeypatch.setattr(crud, "put", pytest.mock_put)
-    monkeypatch.setattr(crud, "delete", pytest.mock_delete)
+    monkeypatch.setattr(base, "get", pytest.mock_get)
+    monkeypatch.setattr(base, "fetch_all", pytest.mock_fetch_all)
+    monkeypatch.setattr(base, "post", pytest.mock_post)
+    monkeypatch.setattr(base, "put", pytest.mock_put)
+    monkeypatch.setattr(base, "delete", pytest.mock_delete)
 
 
 def test_get_event(test_app, monkeypatch):