feat(auth): enforce the role matrix on every data route (M2c, #94)

api/auth.py

@@ -73,3 +73,31 @@ async def tenant_scope(authorization: str = Header(default=""),
     session = await run_in_threadpool(_live_session, bearer_token(authorization))
     with scoped(schema_for(session["tenant"])):
         yield session
+
+
+# M2c (#94): which roles may perform each action. docs/ROLES.md shows the same
+# table, and tests/test_roles.py checks that the two agree.
+PERMISSIONS: dict[str, frozenset[str]] = {
+    "preview":     frozenset({"uploader", "tenant_admin"}),
+    "upload":      frozenset({"uploader", "tenant_admin"}),
+    "list_files":  frozenset({"uploader", "reviewer", "tenant_admin", "auditor"}),
+    "read_rows":   frozenset({"uploader", "reviewer", "tenant_admin", "auditor"}),
+    "list_te":     frozenset({"uploader", "reviewer", "tenant_admin", "auditor"}),
+    "delete_file": frozenset({"tenant_admin"}),
+    "read_audit":  frozenset({"tenant_admin", "auditor"}),
+}
+
+
+def allow(action: str):
+    """Route dependency: 403 unless the signed-in role may perform ``action``.
+
+    Requests without a session (API key or dev) are not role-checked; they keep
+    today's single-operator access until the shared key is retired.
+    """
+    roles = PERMISSIONS[action]
+
+    def check(session: dict | None = Depends(tenant_scope)) -> None:
+        if session is not None and session["role"] not in roles:
+            raise HTTPException(status_code=403,
+                                detail=f"Role '{session['role']}' may not {action.replace('_', ' ')}")
+    return check

api/routers/audit_routes.py

@@ -12,7 +12,7 @@
 from fastapi import APIRouter, Depends
 from psycopg2 import sql
 
-from api.auth import tenant_scope
+from api.auth import allow, tenant_scope
 from api.tenancy import uploads_schema
 from api.db import Conn
 
@@ -23,7 +23,7 @@
 )
 
 
-@router.get("/log")
+@router.get("/log", dependencies=[Depends(allow("read_audit"))])
 def audit_log(limit: int = 50, offset: int = 0) -> dict:
     limit = max(1, min(limit, 200))
     offset = max(0, offset)

api/routers/csv_routes.py

@@ -8,7 +8,7 @@
 from psycopg2 import sql
 from pydantic import BaseModel, Field
 
-from api.auth import tenant_scope
+from api.auth import allow, tenant_scope
 from api.config import settings
 from api.tenancy import current_schema, uploads_schema
 from api.db import Conn
@@ -42,7 +42,7 @@ class UploadRequest(BaseModel):
     targetTable: str | None = None  # required when mode == "te"
 
 
-@router.post("/preview")
+@router.post("/preview", dependencies=[Depends(allow("preview"))])
 def preview(req: PreviewRequest) -> dict:
     if len(req.content.encode("utf-8")) > settings.MAX_UPLOAD_BYTES:
         raise HTTPException(413, "File too large")
@@ -67,7 +67,7 @@ def preview(req: PreviewRequest) -> dict:
     return result
 
 
-@router.post("/upload")
+@router.post("/upload", dependencies=[Depends(allow("upload"))])
 def upload(req: UploadRequest) -> dict:
     if len(req.content.encode("utf-8")) > settings.MAX_UPLOAD_BYTES:
         raise HTTPException(413, "File too large")
@@ -98,7 +98,7 @@ def upload(req: UploadRequest) -> dict:
     return result
 
 
-@router.get("/files")
+@router.get("/files", dependencies=[Depends(allow("list_files"))])
 def list_files() -> list[dict]:
     with Conn() as conn:
         with conn.cursor() as cur:
@@ -126,7 +126,7 @@ def list_files() -> list[dict]:
             ]
 
 
-@router.get("/tables/{table_name}/rows")
+@router.get("/tables/{table_name}/rows", dependencies=[Depends(allow("read_rows"))])
 def table_rows(table_name: str, limit: int = 50) -> dict:
     if not table_name.startswith("csv_") or len(table_name) > 64:
         raise HTTPException(422, "Invalid table name")
@@ -158,7 +158,7 @@ def table_rows(table_name: str, limit: int = 50) -> dict:
     return {"rows": rows}
 
 
-@router.delete("/files/{file_id}")
+@router.delete("/files/{file_id}", dependencies=[Depends(allow("delete_file"))])
 def delete_file(file_id: int) -> dict:
     if not settings.allow_destructive:
         raise HTTPException(

api/routers/te_routes.py

@@ -2,7 +2,7 @@
 
 from fastapi import APIRouter, Depends
 
-from api.auth import tenant_scope
+from api.auth import allow, tenant_scope
 from api.config import TE_TABLES
 from api.tenancy import te_schema
 from api.db import Conn
@@ -15,7 +15,7 @@
 )
 
 
-@router.get("/tables")
+@router.get("/tables", dependencies=[Depends(allow("list_te"))])
 def te_tables() -> list[dict]:
     # BUG-042: this used to run 1-2 queries per table (24 for the 12-table
     # whitelist) — a COUNT(*) existence check plus a full COUNT(*) scan.

docs/ROLES.md

@@ -0,0 +1,20 @@
+# Roles and permissions (BR-36, #94)
+
+Every member of a tenant has one role (ADR-0003). This table is enforced by `PERMISSIONS` in `api/auth.py`. `tests/test_roles.py` fails if the two disagree, and it tries every role against every route.
+
+| Action | Route | uploader | reviewer | tenant_admin | auditor |
+|---|---|---|---|---|---|
+| preview | `POST /api/csv/preview` | ✓ | | ✓ | |
+| upload | `POST /api/csv/upload` | ✓ | | ✓ | |
+| list_files | `GET /api/csv/files` | ✓ | ✓ | ✓ | ✓ |
+| read_rows | `GET /api/csv/tables/{table}/rows` | ✓ | ✓ | ✓ | ✓ |
+| list_te | `GET /api/te/tables` | ✓ | ✓ | ✓ | ✓ |
+| delete_file | `DELETE /api/csv/files/{id}` | | | ✓ | |
+| read_audit | `GET /api/audit/log` | | | ✓ | ✓ |
+
+A role that isn't allowed gets **403**. Two kinds of request are outside this table:
+
+- **Sign-in routes** (`/api/auth/*`) are open to anyone, so people can sign in.
+- **Requests with no session** use the shared API key, or nothing in `dev`. They keep full access until the shared key is retired.
+
+Approving and editing data arrive with M4 and M6. The reviewer gets those permissions then.

scripts/lint.sh

@@ -38,6 +38,7 @@ PY_TESTS=(
     tests/test_import_counts.py
     tests/test_accounts.py
     tests/test_tenant_isolation.py
+    tests/test_roles.py
 )
 
 # Collect scripts/*.py dynamically

tests/test_roles.py

@@ -0,0 +1,128 @@
+"""Role-based authorisation matrix (M2c, #94, BR-36).
+
+The unit test keeps docs/ROLES.md and api/auth.PERMISSIONS in step. The
+integration test tries every role against every protected route.
+"""
+
+from __future__ import annotations
+
+import os
+import unittest
+import uuid
+from pathlib import Path
+
+import pytest
+from fastapi.testclient import TestClient
+from psycopg2 import sql
+
+from api.auth import PERMISSIONS
+from api.config import settings
+from api.db import Conn
+from api.main import app
+from api.services import accounts
+from api.services.accounts import ROLES
+from api.tenancy import migrate_tenant, schema_for
+
+_PW = "correct horse battery staple"
+_DOC = Path(__file__).resolve().parent.parent / "docs" / "ROLES.md"
+# action -> (method, path); {table} and {id} are filled per run.
+_ROUTES = {
+    "preview": ("POST", "/api/csv/preview"),
+    "upload": ("POST", "/api/csv/upload"),
+    "list_files": ("GET", "/api/csv/files"),
+    "read_rows": ("GET", "/api/csv/tables/{table}/rows"),
+    "list_te": ("GET", "/api/te/tables"),
+    "delete_file": ("DELETE", "/api/csv/files/{id}"),
+    "read_audit": ("GET", "/api/audit/log"),
+}
+
+
+def _doc_matrix() -> dict[str, frozenset[str]]:
+    rows = [ln for ln in _DOC.read_text(encoding="utf-8").splitlines() if ln.startswith("| ")]
+    header = [c.strip() for c in rows[0].strip("|").split("|")]
+    out = {}
+    for ln in rows[1:]:   # the |---| separator does not start with "| "
+        cells = [c.strip() for c in ln.strip("|").split("|")]
+        out[cells[0]] = frozenset(r for r, c in zip(header[2:], cells[2:]) if c == "✓")
+    return out
+
+
+@pytest.mark.unit
+class MatrixIsDocumented(unittest.TestCase):
+    def test_docs_table_matches_the_enforced_permissions(self):
+        self.assertEqual(_doc_matrix(), PERMISSIONS)
+
+    def test_every_action_has_a_route_and_only_known_roles(self):
+        self.assertEqual(set(PERMISSIONS), set(_ROUTES))
+        for action, roles in PERMISSIONS.items():
+            self.assertTrue(roles <= set(ROLES), action)
+
+
+@pytest.mark.integration
+class RoleMatrixOverTheApi(unittest.TestCase):
+    @classmethod
+    def setUpClass(cls):
+        cls._orig_allow_destructive = settings.allow_destructive
+        settings.allow_destructive = True
+        cls.ctx = TestClient(app)
+        cls.client = cls.ctx.__enter__()
+        cls.slug = f"roles_{os.getpid()}_{uuid.uuid4().hex[:6]}"
+        cls.headers = {}
+        with Conn() as conn:
+            with conn.cursor() as cur:
+                tid = accounts.create_tenant(cur, cls.slug, "Roles test")
+                for role in ROLES:
+                    uid = accounts.create_user(cur, f"{cls.slug}_{role}", _PW)
+                    accounts.add_membership(cur, uid, tid, role)
+            conn.commit()
+        migrate_tenant(cls.slug)
+        for role in ROLES:
+            res = cls.client.post("/api/auth/login", json={
+                "tenant": cls.slug, "username": f"{cls.slug}_{role}", "password": _PW})
+            assert res.status_code == 200, res.text
+            cls.headers[role] = {"Authorization": f"Bearer {res.json()['token']}"}
+
+    @classmethod
+    def tearDownClass(cls):
+        settings.allow_destructive = cls._orig_allow_destructive
+        with Conn() as conn:
+            with conn.cursor() as cur:
+                cur.execute(sql.SQL("DROP SCHEMA IF EXISTS {} CASCADE").format(sql.Identifier(schema_for(cls.slug))))
+                cur.execute(sql.SQL("DELETE FROM {}.tenants WHERE slug = %s").format(
+                    sql.Identifier(settings.CONTROL_SCHEMA)), (cls.slug,))
+                cur.execute(sql.SQL("DELETE FROM {}.users WHERE username LIKE %s").format(
+                    sql.Identifier(settings.CONTROL_SCHEMA)), (f"{cls.slug}_%",))
+            conn.commit()
+        cls.ctx.__exit__(None, None, None)
+
+    def _file(self) -> tuple[str, str]:
+        tag = uuid.uuid4().hex[:10]
+        res = self.client.post("/api/csv/upload", headers=self.headers["tenant_admin"],
+                               json={"fileName": f"r_{tag}.csv", "content": f"k\n{tag}\n"}).json()
+        assert res["status"] == "ok", res
+        return res["tableName"], res["fileId"]
+
+    def _call(self, role: str, action: str) -> int:
+        method, path = _ROUTES[action]
+        table, file_id = self._file()
+        body = None
+        if action in ("preview", "upload"):
+            tag = uuid.uuid4().hex[:10]
+            body = {"fileName": f"p_{tag}.csv", "content": f"k\n{tag}\n"}
+        res = self.client.request(method, path.format(table=table, id=file_id),
+                                  headers=self.headers[role], json=body)
+        return res.status_code
+
+    def test_every_role_against_every_route(self):
+        for action, allowed in PERMISSIONS.items():
+            for role in ROLES:
+                with self.subTest(action=action, role=role):
+                    code = self._call(role, action)
+                    if role in allowed:
+                        self.assertNotIn(code, (401, 403), f"{role} should be allowed to {action}")
+                        self.assertLess(code, 500)
+                    else:
+                        self.assertEqual(code, 403, f"{role} must be refused {action}")
+
+    def test_no_session_requests_are_not_role_checked(self):
+        self.assertEqual(self.client.get("/api/audit/log").status_code, 200)

tests/test_tenant_isolation.py

@@ -1,6 +1,6 @@
 """Standing cross-tenant negative tests (M2b, #93, BR-36; ADR-0003).
 
-Two tenants, each with an uploader. Nothing one tenant does may be visible to
+Two tenants, each with a tenant admin (the most privileged role). Nothing one tenant does may be visible to
 or changeable by the other, or by a request that has no session.
 Integration: needs a live PostgreSQL. Fails (never skips) without one.
 """
@@ -46,7 +46,7 @@ def setUpClass(cls):
                 for key, slug in cls.slugs.items():
                     tid = accounts.create_tenant(cur, slug, f"Tenant {key}")
                     uid = accounts.create_user(cur, f"{slug}_user", _PW)
-                    accounts.add_membership(cur, uid, tid, "uploader")
+                    accounts.add_membership(cur, uid, tid, "tenant_admin")  # full rights: isolation must still hold
             conn.commit()
         for slug in cls.slugs.values():
             migrate_tenant(slug)