From 9f947cb38d6207341ad2a1ea23957fd642d80e34 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 24 Jun 2026 13:18:40 +0000 Subject: [PATCH 1/2] =?UTF-8?q?Sprint=204=20PR=204.5b=20=E2=80=94=20close?= =?UTF-8?q?=20/calendar/export=20auth=20gap?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: PR 4.5 added auth to /calendar/subscribe and /reset-token but missed /calendar/export, which was still fully unauthenticated. Any caller could download anyone's personal schedule as ICS by guessing or scraping person_id values — a PII leak. Apply the same self-or-admin-in-same-org rule already used by the sibling endpoints. Changed files: - api/routers/calendar.py: export_personal_schedule now takes current_user via Depends(get_current_user) and runs _ensure_self_or_same_org_admin after the person lookup. - tests/api/test_calendar_auth.py: New TestExportAuth class with 5 cases covering no-auth, cross-user, cross-org admin, self, and admin-in-same-org. Allowed cases land on the existing 404 "No assignments found" body — proving auth cleared without having to seed an Assignment row. - tests/contract/openapi.snapshot.json: Refreshed; only adds HTTPBearer security on /api/v1/calendar/export and the new sentence in its description. Validation: - poetry run pytest tests/api/test_calendar_auth.py -v -> 15 passed - poetry run pytest tests/unit/test_calendar.py -v -> 18 passed (mocked auth satisfies the new dep, no test changes needed) - make test-unit-fast -> 339 passed, 21 skipped - poetry run pytest tests/api -> 315 passed - poetry run pytest tests/contract -> 1 passed - black and ruff clean on touched files Follow-ups: - /calendar/org/export still uses the spoofable person_id-as-auth proxy pattern (passes person_id in query, looks up the admin record by id instead of by JWT). Worth a separate PR to switch it to Depends(get_current_admin_user) + verify_org_member. --- api/routers/calendar.py | 3 + tests/api/test_calendar_auth.py | 91 ++++++++++++++++++++++++++++ tests/contract/openapi.snapshot.json | 7 ++- 3 files changed, 100 insertions(+), 1 deletion(-) diff --git a/api/routers/calendar.py b/api/routers/calendar.py index 785cfeec..83b5edc0 100644 --- a/api/routers/calendar.py +++ b/api/routers/calendar.py @@ -66,6 +66,7 @@ def get_base_url(request: Request) -> str: @router.get("/export") def export_personal_schedule( person_id: str, + current_user: Person = Depends(get_current_user), db: Session = Depends(get_db), request: Request = None, ): @@ -73,6 +74,7 @@ def export_personal_schedule( Export personal schedule as ICS file. This endpoint downloads an ICS file with all assigned events for a person. + Caller must be the target person or an admin in the same organization. """ # Verify person exists person = db.query(Person).filter(Person.id == person_id).first() @@ -81,6 +83,7 @@ def export_personal_schedule( status_code=status.HTTP_404_NOT_FOUND, detail=f"Person '{person_id}' not found", ) + _ensure_self_or_same_org_admin(current_user, person) # Get all assignments for this person assignments = db.query(Assignment).filter(Assignment.person_id == person_id).all() diff --git a/tests/api/test_calendar_auth.py b/tests/api/test_calendar_auth.py index ab8b31b9..489fe97e 100644 --- a/tests/api/test_calendar_auth.py +++ b/tests/api/test_calendar_auth.py @@ -5,6 +5,10 @@ authenticated user who is either the owner or an admin in the same org. A new admin-only `/calendar/{person_id}/admin-reset` endpoint forces a token reset on a different user. Both reset paths are audited. + +Follow-up: `/calendar/export?person_id=...` was also fully unauthenticated +and leaked anyone's personal schedule as ICS. It now requires the same +"self or admin in same org" rule as `/subscribe` and `/reset-token`. """ import pytest @@ -209,3 +213,90 @@ def test_admin_cross_org_blocked(self, client, db): resp = client.post(f"/api/v1/calendar/{b_id}/admin-reset", headers=a_hdrs) assert resp.status_code == 403 + + +@pytest.mark.no_mock_auth +class TestExportAuth: + """Auth on `/calendar/export?person_id=...` — previously unauthenticated + PII leak: any caller could download anyone's schedule as ICS. + + Reaching the "no assignments" 404 in the allowed cases proves auth passed + and code entered the body — we don't have to seed an assignment here. + """ + + def test_no_auth_rejected(self, client, db): + org_id = "cal-export-noauth" + seed_org(client, org_id) + admin_hdrs = _admin_for(client, org_id, "exp-noauth") + my_id = _person_id_for_email(client, admin_hdrs, "admin-exp-noauth@o.org") + + resp = client.get(f"/api/v1/calendar/export?person_id={my_id}") + assert resp.status_code in (401, 403) + + def test_volunteer_cannot_export_other(self, client, db): + org_id = "cal-export-vol" + seed_org(client, org_id) + _admin_for(client, org_id, "exp-vol-admin") + seed_user( + client, + org_id, + email="exp-v1@o.org", + name="ExpV1", + password="ExpV1Pass1!", + roles=["volunteer"], + ) + seed_user( + client, + org_id, + email="exp-v2@o.org", + name="ExpV2", + password="ExpV2Pass1!", + roles=["volunteer"], + ) + v1_hdrs = auth_headers(client, email="exp-v1@o.org", password="ExpV1Pass1!") + v2_hdrs = auth_headers(client, email="exp-v2@o.org", password="ExpV2Pass1!") + v2_id = _person_id_for_email(client, v2_hdrs, "exp-v2@o.org") + + resp = client.get(f"/api/v1/calendar/export?person_id={v2_id}", headers=v1_hdrs) + assert resp.status_code == 403 + + def test_cross_org_admin_blocked(self, client, db): + seed_org(client, "cal-exp-a") + seed_org(client, "cal-exp-b") + a_hdrs = _admin_for(client, "cal-exp-a", "exp-x-a") + b_hdrs = _admin_for(client, "cal-exp-b", "exp-x-b") + b_id = _person_id_for_email(client, b_hdrs, "admin-exp-x-b@o.org") + + resp = client.get(f"/api/v1/calendar/export?person_id={b_id}", headers=a_hdrs) + assert resp.status_code == 403 + + def test_self_reaches_body(self, client, db): + """No assignments yet → 404, but the 404 message proves we cleared auth.""" + org_id = "cal-export-self" + seed_org(client, org_id) + hdrs = _admin_for(client, org_id, "exp-self") + my_id = _person_id_for_email(client, hdrs, "admin-exp-self@o.org") + + resp = client.get(f"/api/v1/calendar/export?person_id={my_id}", headers=hdrs) + assert resp.status_code == 404, resp.text + assert "No assignments found" in resp.json()["detail"] + + def test_admin_same_org_reaches_body(self, client, db): + """Admin pulling another user's schedule clears auth.""" + org_id = "cal-export-admin-other" + seed_org(client, org_id) + admin_hdrs = _admin_for(client, org_id, "exp-admin-other-a") + seed_user( + client, + org_id, + email="exp-other@o.org", + name="ExpOther", + password="ExpOtherPass1!", + roles=["volunteer"], + ) + other_hdrs = auth_headers(client, email="exp-other@o.org", password="ExpOtherPass1!") + other_id = _person_id_for_email(client, other_hdrs, "exp-other@o.org") + + resp = client.get(f"/api/v1/calendar/export?person_id={other_id}", headers=admin_hdrs) + assert resp.status_code == 404, resp.text + assert "No assignments found" in resp.json()["detail"] diff --git a/tests/contract/openapi.snapshot.json b/tests/contract/openapi.snapshot.json index 6efe6602..705e5785 100644 --- a/tests/contract/openapi.snapshot.json +++ b/tests/contract/openapi.snapshot.json @@ -7769,7 +7769,7 @@ }, "/api/v1/calendar/export": { "get": { - "description": "Export personal schedule as ICS file.\n\nThis endpoint downloads an ICS file with all assigned events for a person.", + "description": "Export personal schedule as ICS file.\n\nThis endpoint downloads an ICS file with all assigned events for a person.\nCaller must be the target person or an admin in the same organization.", "operationId": "exportPersonalSchedule", "parameters": [ { @@ -7802,6 +7802,11 @@ "description": "Validation Error" } }, + "security": [ + { + "HTTPBearer": [] + } + ], "summary": "Export Personal Schedule", "tags": [ "calendar" From 3b67a6d4e77c257073ce23ce250975012a6b88e7 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 25 Jun 2026 13:12:38 +0000 Subject: [PATCH 2/2] =?UTF-8?q?ci:=20rerun=20e2e=20=E2=80=94=20open-list?= =?UTF-8?q?=20timeouts=20looked=20like=20runner-flake,=20retrying?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit