Skip to content

Sprint 4 PR 4.5d — close /analytics auth gap - #229

Merged
tomqwu merged 1 commit into
mainfrom
sprint-4-pr-4-5d-analytics-auth
Jul 9, 2026
Merged

tomqwu merged 1 commit into
mainfrom
sprint-4-pr-4-5d-analytics-auth

Conversation

@tomqwu

@tomqwu tomqwu commented Jul 8, 2026

Copy link
Copy Markdown
Owner

Summary

PR 4.5b/c closed the auth gaps on the calendar router; the analytics router was the last unauthenticated read surface with the same cross-tenant shape. GET /api/v1/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 reaching the API could enumerate any org's roster.

This PR gates the three endpoints on Depends(get_current_admin_user) + verify_org_member, matching the pattern used by /calendar/org/export (PR 4.5c).

Threat closed

Before this PR, an unauthenticated caller could:

  • GET /api/v1/analytics/{any_org_id}/volunteer-stats → top volunteers by name + assignment counts
  • GET /api/v1/analytics/{any_org_id}/burnout-risk → volunteer id + name + email of anyone serving ≥ threshold in the last 30 days
  • GET /api/v1/analytics/{any_org_id}/schedule-health → org KPIs + latest solution id + health score

After this PR: 401/403 without JWT, 403 for volunteers (even in-org), 403 for cross-org admins, 200 only for admin-in-same-org.

Changes

File Change
api/routers/analytics.py Add current_admin: Person = Depends(get_current_admin_user) + verify_org_member(current_admin, org_id) to all three endpoints. days/threshold Query params moved before the dep so keyword call sites in web/ still work.
tests/api/test_analytics_auth.py New — 12 no_mock_auth cases: per endpoint, unauth → 401/403, volunteer in same org → 403, admin cross-org → 403, admin same-org → 200.
tests/api/test_analytics.py Pre-existing baseline tests now send admin auth headers via an admin_hdrs fixture. The "unknown-org accepts 200 or 404" case is now "admin-of-known-org gets 403 on unknown org" (verify_org_member rejects before the DB read).
web/routers/pages.py _dashboard_kpis and _analytics do in-process direct calls to the API functions and had to grow the same current_admin kwarg. Both helpers now take person instead of org_id and forward it as current_admin=person so verify_org_member still runs on the admin dashboard + /a/analytics pages. Call sites in admin_dashboard / admin_analytics updated.
tests/contract/openapi.snapshot.json Refreshed via make update-openapi-snapshot — HTTPBearer security added to the three analytics operations, descriptions bumped.

Test plan

  • TDD: 12 new auth-gap tests fail before the router change, pass after
  • poetry run pytest tests/api/test_analytics.py tests/api/test_analytics_auth.py tests/web/test_analytics_page.py → 21 passed
  • make test-unit-fast → 338 passed, 21 skipped
  • poetry run pytest tests/api tests/contract tests/web tests/integration tests/cli → 609 passed
  • poetry run black --check api tests → clean
  • poetry run ruff check api tests → clean
  • OpenAPI snapshot refreshed and committed

Follow-ups

None. This closes the last analytics auth gap. The E2E lane should run against updated main post-merge to pick up any downstream web-flow assumptions.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SvZvSuB2SYqxrJBXoX1vBY


Generated by Claude Code

Summary:
  PR 4.5b/c closed the auth gaps on the calendar router; the analytics
  router was the last unauthenticated read surface with the same
  cross-tenant shape. `GET /api/v1/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 reaching the API could
  enumerate any org's roster. Gate the three endpoints on
  `Depends(get_current_admin_user)` + `verify_org_member`, matching
  the pattern used by `/calendar/org/export`.

Changed files:
  - api/routers/analytics.py — add current_admin dependency and
    verify_org_member call to get_volunteer_stats, get_schedule_health,
    get_burnout_risk. `days`/`threshold` Query params moved before the
    dep so keyword call sites in web/ still work.
  - tests/api/test_analytics_auth.py — 12 new no_mock_auth cases: for
    each endpoint, unauth → 401/403, volunteer in same org → 403,
    admin cross-org → 403, admin same-org → 200.
  - tests/api/test_analytics.py — pre-existing baseline tests now send
    admin auth headers via an `admin_hdrs` fixture; the "unknown org
    accepts 200|404" case is now "admin-of-known-org gets 403 on
    unknown org" (verify_org_member rejects before the DB read).
  - web/routers/pages.py — `_dashboard_kpis` and `_analytics` do
    in-process direct calls to the API functions and had to grow the
    same `current_admin` kwarg. Both helpers now take `person` instead
    of `org_id` and forward it as `current_admin=person` so the
    verify_org_member check still runs on the dashboard/analytics
    pages. Call sites in admin_dashboard/admin_analytics updated.
  - tests/contract/openapi.snapshot.json — refreshed via
    make update-openapi-snapshot: HTTPBearer security added to the
    three analytics operations; descriptions bumped.

Validation:
  - poetry run pytest tests/api/test_analytics.py \
      tests/api/test_analytics_auth.py \
      tests/web/test_analytics_page.py --tb=short → 21 passed
  - make test-unit-fast → 338 passed, 21 skipped
  - poetry run pytest tests/api tests/contract tests/web \
      tests/integration tests/cli --tb=short → 609 passed
  - poetry run black --check api tests → clean
  - poetry run ruff check api tests → clean

Follow-ups:
  - None. This closes the last analytics auth gap. Post-merge the E2E
    lane should run against updated main to pick up any downstream
    web-flow assumptions.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SvZvSuB2SYqxrJBXoX1vBY
@tomqwu
tomqwu marked this pull request as ready for review July 9, 2026 13:04
@tomqwu
tomqwu merged commit 942aa53 into main Jul 9, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants