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"