Sprint 4 PR 4.5d — close /analytics auth gap

api/routers/analytics.py

@@ -7,6 +7,7 @@
 from sqlalchemy.orm import Session
 
 from api.database import get_db
+from api.dependencies import get_current_admin_user, verify_org_member
 from api.models import Assignment, Event, Person, Solution
 
 router = APIRouter(prefix="/analytics", tags=["analytics"])
@@ -15,10 +16,16 @@
 @router.get("/{org_id}/volunteer-stats")
 def get_volunteer_stats(
     org_id: str,
-    db: Session = Depends(get_db),
     days: int = Query(30, description="Number of days to analyze"),
+    current_admin: Person = Depends(get_current_admin_user),
+    db: Session = Depends(get_db),
 ):
-    """Get volunteer participation statistics."""
+    """Get volunteer participation statistics.
+
+    Admin-only within `org_id`. The caller must be an authenticated admin
+    whose own org matches the requested one.
+    """
+    verify_org_member(current_admin, org_id)
 
     since_date = datetime.now() - timedelta(days=days)
 
@@ -70,9 +77,14 @@ def get_volunteer_stats(
 @router.get("/{org_id}/schedule-health")
 def get_schedule_health(
     org_id: str,
+    current_admin: Person = Depends(get_current_admin_user),
     db: Session = Depends(get_db),
 ):
-    """Get schedule health metrics."""
+    """Get schedule health metrics.
+
+    Admin-only within `org_id`.
+    """
+    verify_org_member(current_admin, org_id)
 
     # Upcoming events
     upcoming_events = (
@@ -118,10 +130,16 @@ def get_schedule_health(
 @router.get("/{org_id}/burnout-risk")
 def get_burnout_risk(
     org_id: str,
-    db: Session = Depends(get_db),
     threshold: int = Query(4, description="Assignments per month threshold"),
+    current_admin: Person = Depends(get_current_admin_user),
+    db: Session = Depends(get_db),
 ):
-    """Identify volunteers at risk of burnout (serving too frequently)."""
+    """Identify volunteers at risk of burnout (serving too frequently).
+
+    Admin-only within `org_id`. This endpoint returns other volunteers'
+    names and emails, so peer volunteers can never read it.
+    """
+    verify_org_member(current_admin, org_id)
 
     one_month_ago = datetime.now() - timedelta(days=30)
 

tests/api/test_analytics.py

@@ -1,8 +1,14 @@
-"""Tests for /api/v1/analytics — covers the previously zero-test analytics router."""
+"""Tests for /api/v1/analytics — covers the previously zero-test analytics router.
+
+Sprint 4 PR 4.5d gated all three endpoints behind
+`Depends(get_current_admin_user)` + `verify_org_member`. Baseline
+smoke tests now authenticate as an admin so they exercise the same
+happy paths they always did.
+"""
 
 import pytest
 
-from tests.api.conftest import seed_org, seed_user
+from tests.api.conftest import auth_headers, seed_org, seed_user
 
 
 @pytest.fixture
@@ -13,45 +19,68 @@ def org_id(client):
     return org
 
 
+@pytest.fixture
+def admin_hdrs(client, org_id):
+    return auth_headers(client, email="admin@a.org", password="AdminPass1!")
+
+
 @pytest.mark.no_mock_auth
 class TestVolunteerStats:
-    def test_returns_baseline_for_empty_org(self, client, db, org_id):
-        resp = client.get(f"/api/v1/analytics/{org_id}/volunteer-stats")
+    def test_returns_baseline_for_empty_org(self, client, db, org_id, admin_hdrs):
+        resp = client.get(
+            f"/api/v1/analytics/{org_id}/volunteer-stats",
+            headers=admin_hdrs,
+        )
         assert resp.status_code == 200
         body = resp.json()
         # Whatever the schema, endpoint must succeed and return JSON
         assert isinstance(body, dict)
 
-    def test_404_or_empty_for_unknown_org(self, client):
-        # Unknown orgs should not crash; either 404 or empty stats are acceptable.
-        resp = client.get("/api/v1/analytics/no-such-org/volunteer-stats")
-        assert resp.status_code in (200, 404)
+    def test_403_for_unknown_org(self, client, db, org_id, admin_hdrs):
+        # Admin-of-org-A hitting unknown org must be rejected before any DB read.
+        resp = client.get(
+            "/api/v1/analytics/no-such-org/volunteer-stats",
+            headers=admin_hdrs,
+        )
+        assert resp.status_code == 403
 
-    def test_accepts_days_query_param(self, client, db, org_id):
-        resp = client.get(f"/api/v1/analytics/{org_id}/volunteer-stats?days=7")
+    def test_accepts_days_query_param(self, client, db, org_id, admin_hdrs):
+        resp = client.get(
+            f"/api/v1/analytics/{org_id}/volunteer-stats?days=7",
+            headers=admin_hdrs,
+        )
         assert resp.status_code == 200
 
 
 @pytest.mark.no_mock_auth
 class TestScheduleHealth:
-    def test_returns_for_org(self, client, db, org_id):
-        resp = client.get(f"/api/v1/analytics/{org_id}/schedule-health")
+    def test_returns_for_org(self, client, db, org_id, admin_hdrs):
+        resp = client.get(
+            f"/api/v1/analytics/{org_id}/schedule-health",
+            headers=admin_hdrs,
+        )
         assert resp.status_code == 200
         body = resp.json()
         assert isinstance(body, dict)
 
 
 @pytest.mark.no_mock_auth
 class TestBurnoutRisk:
-    def test_returns_for_org(self, client, db, org_id):
-        resp = client.get(f"/api/v1/analytics/{org_id}/burnout-risk")
+    def test_returns_for_org(self, client, db, org_id, admin_hdrs):
+        resp = client.get(
+            f"/api/v1/analytics/{org_id}/burnout-risk",
+            headers=admin_hdrs,
+        )
         assert resp.status_code == 200
         body = resp.json()
         assert isinstance(body, dict | list)
 
-    def test_no_assignments_means_no_at_risk(self, client, db, org_id):
+    def test_no_assignments_means_no_at_risk(self, client, db, org_id, admin_hdrs):
         """An org with zero events/assignments cannot have burnout risk by definition."""
-        resp = client.get(f"/api/v1/analytics/{org_id}/burnout-risk")
+        resp = client.get(
+            f"/api/v1/analytics/{org_id}/burnout-risk",
+            headers=admin_hdrs,
+        )
         assert resp.status_code == 200
         body = resp.json()
         # Whether `at_risk_volunteers` is absent or `[]`, the value must be falsy.

tests/api/test_analytics_auth.py

@@ -0,0 +1,173 @@
+"""Analytics auth-gap tests (Sprint 4 PR 4.5d).
+
+Previously `/analytics/{org_id}/volunteer-stats`, `/schedule-health`, and
+`/burnout-risk` accepted `org_id` as a path parameter and returned
+volunteer names, emails, and event counts with **no auth check at all**.
+Anyone who could reach the API could enumerate any org's roster.
+
+After this PR each endpoint requires an authenticated admin whose own
+`org_id` matches the requested one, matching the pattern already used by
+`/calendar/org/export` (PR 4.5c).
+"""
+
+import pytest
+
+from tests.api.conftest import auth_headers, seed_org, seed_user
+
+
+def _admin_for(client, org_id: str, suffix: str):
+    seed_user(
+        client,
+        org_id,
+        email=f"admin-{suffix}@a.org",
+        name="Admin",
+        password="AdminPass1!",
+    )
+    return auth_headers(client, email=f"admin-{suffix}@a.org", password="AdminPass1!")
+
+
+def _volunteer_for(client, org_id: str, suffix: str):
+    seed_user(
+        client,
+        org_id,
+        email=f"vol-{suffix}@a.org",
+        name="Vol",
+        password="VolPass1!",
+        roles=["volunteer"],
+    )
+    return auth_headers(client, email=f"vol-{suffix}@a.org", password="VolPass1!")
+
+
+# ---------------------------------------------------------------------------
+# volunteer-stats
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.no_mock_auth
+class TestVolunteerStatsAuth:
+    def test_no_auth_rejected(self, client, db):
+        org_id = "an-vs-noauth"
+        seed_org(client, org_id)
+
+        resp = client.get(f"/api/v1/analytics/{org_id}/volunteer-stats")
+        assert resp.status_code in (401, 403)
+
+    def test_volunteer_in_same_org_rejected(self, client, db):
+        org_id = "an-vs-vol"
+        seed_org(client, org_id)
+        _admin_for(client, org_id, "vs-vol-a")
+        vol_hdrs = _volunteer_for(client, org_id, "vs-vol")
+
+        resp = client.get(f"/api/v1/analytics/{org_id}/volunteer-stats", headers=vol_hdrs)
+        assert resp.status_code == 403
+
+    def test_admin_cross_org_rejected(self, client, db):
+        seed_org(client, "an-vs-cr-a")
+        seed_org(client, "an-vs-cr-b")
+        a_hdrs = _admin_for(client, "an-vs-cr-a", "vs-cr-a")
+        _admin_for(client, "an-vs-cr-b", "vs-cr-b")
+
+        # Admin of org A asks for org B's volunteer stats.
+        resp = client.get(
+            "/api/v1/analytics/an-vs-cr-b/volunteer-stats",
+            headers=a_hdrs,
+        )
+        assert resp.status_code == 403
+
+    def test_admin_same_org_ok(self, client, db):
+        org_id = "an-vs-ok"
+        seed_org(client, org_id)
+        a_hdrs = _admin_for(client, org_id, "vs-ok")
+
+        resp = client.get(f"/api/v1/analytics/{org_id}/volunteer-stats", headers=a_hdrs)
+        assert resp.status_code == 200
+        body = resp.json()
+        assert body["org_id"] == org_id
+
+
+# ---------------------------------------------------------------------------
+# schedule-health
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.no_mock_auth
+class TestScheduleHealthAuth:
+    def test_no_auth_rejected(self, client, db):
+        org_id = "an-sh-noauth"
+        seed_org(client, org_id)
+
+        resp = client.get(f"/api/v1/analytics/{org_id}/schedule-health")
+        assert resp.status_code in (401, 403)
+
+    def test_volunteer_in_same_org_rejected(self, client, db):
+        org_id = "an-sh-vol"
+        seed_org(client, org_id)
+        _admin_for(client, org_id, "sh-vol-a")
+        vol_hdrs = _volunteer_for(client, org_id, "sh-vol")
+
+        resp = client.get(f"/api/v1/analytics/{org_id}/schedule-health", headers=vol_hdrs)
+        assert resp.status_code == 403
+
+    def test_admin_cross_org_rejected(self, client, db):
+        seed_org(client, "an-sh-cr-a")
+        seed_org(client, "an-sh-cr-b")
+        a_hdrs = _admin_for(client, "an-sh-cr-a", "sh-cr-a")
+        _admin_for(client, "an-sh-cr-b", "sh-cr-b")
+
+        resp = client.get("/api/v1/analytics/an-sh-cr-b/schedule-health", headers=a_hdrs)
+        assert resp.status_code == 403
+
+    def test_admin_same_org_ok(self, client, db):
+        org_id = "an-sh-ok"
+        seed_org(client, org_id)
+        a_hdrs = _admin_for(client, org_id, "sh-ok")
+
+        resp = client.get(f"/api/v1/analytics/{org_id}/schedule-health", headers=a_hdrs)
+        assert resp.status_code == 200
+        body = resp.json()
+        assert body["org_id"] == org_id
+
+
+# ---------------------------------------------------------------------------
+# burnout-risk
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.no_mock_auth
+class TestBurnoutRiskAuth:
+    def test_no_auth_rejected(self, client, db):
+        org_id = "an-br-noauth"
+        seed_org(client, org_id)
+
+        resp = client.get(f"/api/v1/analytics/{org_id}/burnout-risk")
+        assert resp.status_code in (401, 403)
+
+    def test_volunteer_in_same_org_rejected(self, client, db):
+        """Burnout-risk exposes other volunteers' names + emails — never
+        readable by a peer volunteer, even one in the same org."""
+        org_id = "an-br-vol"
+        seed_org(client, org_id)
+        _admin_for(client, org_id, "br-vol-a")
+        vol_hdrs = _volunteer_for(client, org_id, "br-vol")
+
+        resp = client.get(f"/api/v1/analytics/{org_id}/burnout-risk", headers=vol_hdrs)
+        assert resp.status_code == 403
+
+    def test_admin_cross_org_rejected(self, client, db):
+        seed_org(client, "an-br-cr-a")
+        seed_org(client, "an-br-cr-b")
+        a_hdrs = _admin_for(client, "an-br-cr-a", "br-cr-a")
+        _admin_for(client, "an-br-cr-b", "br-cr-b")
+
+        resp = client.get("/api/v1/analytics/an-br-cr-b/burnout-risk", headers=a_hdrs)
+        assert resp.status_code == 403
+
+    def test_admin_same_org_ok(self, client, db):
+        org_id = "an-br-ok"
+        seed_org(client, org_id)
+        a_hdrs = _admin_for(client, org_id, "br-ok")
+
+        resp = client.get(f"/api/v1/analytics/{org_id}/burnout-risk", headers=a_hdrs)
+        assert resp.status_code == 200
+        body = resp.json()
+        assert body["org_id"] == org_id

tests/contract/openapi.snapshot.json

@@ -5513,7 +5513,7 @@
     },
     "/api/v1/analytics/{org_id}/burnout-risk": {
       "get": {
-        "description": "Identify volunteers at risk of burnout (serving too frequently).",
+        "description": "Identify volunteers at risk of burnout (serving too frequently).\n\nAdmin-only within `org_id`. This endpoint returns other volunteers'\nnames and emails, so peer volunteers can never read it.",
         "operationId": "getBurnoutRisk",
         "parameters": [
           {
@@ -5558,6 +5558,11 @@
             "description": "Validation Error"
           }
         },
+        "security": [
+          {
+            "HTTPBearer": []
+          }
+        ],
         "summary": "Get Burnout Risk",
         "tags": [
           "analytics"
@@ -5566,7 +5571,7 @@
     },
     "/api/v1/analytics/{org_id}/schedule-health": {
       "get": {
-        "description": "Get schedule health metrics.",
+        "description": "Get schedule health metrics.\n\nAdmin-only within `org_id`.",
         "operationId": "getScheduleHealth",
         "parameters": [
           {
@@ -5599,6 +5604,11 @@
             "description": "Validation Error"
           }
         },
+        "security": [
+          {
+            "HTTPBearer": []
+          }
+        ],
         "summary": "Get Schedule Health",
         "tags": [
           "analytics"
@@ -5607,7 +5617,7 @@
     },
     "/api/v1/analytics/{org_id}/volunteer-stats": {
       "get": {
-        "description": "Get volunteer participation statistics.",
+        "description": "Get volunteer participation statistics.\n\nAdmin-only within `org_id`. The caller must be an authenticated admin\nwhose own org matches the requested one.",
         "operationId": "getVolunteerStats",
         "parameters": [
           {
@@ -5652,6 +5662,11 @@
             "description": "Validation Error"
           }
         },
+        "security": [
+          {
+            "HTTPBearer": []
+          }
+        ],
         "summary": "Get Volunteer Stats",
         "tags": [
           "analytics"