Merge pull request 'Fix leak: redact per-person on authed non-member reads' (#46) from fix-authed-nonmember-redaction into main

backend/app/services/event_service.py

@@ -97,6 +97,13 @@ async def list_events(
     """All events in the tree — lets the family view compute birth/death years."""
     if not await privacy.can_view_tree(session, user_id=viewer_id, tree=tree):
         raise Forbidden("not permitted to view this tree")
+    # Non-members get the redacted projection (no living-person dates).
+    if await privacy.get_membership_role(session, viewer_id, tree.id) is None:
+        from app.services import public_view_service
+
+        return await public_view_service.list_public_events(
+            session, viewer_id=viewer_id, tree=tree
+        )
     stmt = (
         select(Event)
         .where(Event.tree_id == tree.id, Event.deleted_at.is_(None))
@@ -110,6 +117,13 @@ async def list_events_for_person(
 ) -> list[Event]:
     if not await privacy.can_view_tree(session, user_id=viewer_id, tree=tree):
         raise Forbidden("not permitted to view this tree")
+    # Non-members only see a full-visibility person's events (redacted → none).
+    if await privacy.get_membership_role(session, viewer_id, tree.id) is None:
+        from app.services import public_view_service
+
+        return await public_view_service.list_public_person_events(
+            session, viewer_id=viewer_id, tree=tree, person_id=person_id
+        )
     stmt = (
         select(Event)
         .where(

backend/app/services/media_service.py

@@ -72,6 +72,13 @@ async def upload_media(
 async def list_media(session: AsyncSession, *, viewer_id: uuid.UUID, tree: Tree) -> list[Media]:
     if not await privacy.can_view_tree(session, user_id=viewer_id, tree=tree):
         raise Forbidden("not permitted to view this tree")
+    # Non-members only see media of a FULL-visibility person (no living-person photos).
+    if await privacy.get_membership_role(session, viewer_id, tree.id) is None:
+        from app.services import public_view_service
+
+        return await public_view_service.list_public_media(
+            session, viewer_id=viewer_id, tree=tree
+        )
     stmt = (
         select(Media)
         .where(Media.tree_id == tree.id, Media.deleted_at.is_(None))
@@ -94,6 +101,15 @@ async def get_media(
     ).scalar_one_or_none()
     if media is None:
         raise NotFound("media not found")
+    # Non-members may only see/download media of a FULL-visibility person. 404
+    # (not 403) so the item's existence isn't revealed. This gates media_content.
+    if await privacy.get_membership_role(session, viewer_id, tree.id) is None:
+        from app.services import public_view_service
+
+        if not await public_view_service.can_view_media(
+            session, viewer_id=viewer_id, tree=tree, media=media
+        ):
+            raise NotFound("media not found")
     return media
 
 

backend/app/services/name_service.py

@@ -51,6 +51,13 @@ async def list_names(
     if not await privacy.can_view_tree(session, user_id=viewer_id, tree=tree):
         raise Forbidden("not permitted to view this tree")
     await _get_person(session, tree=tree, person_id=person_id)
+    # Non-members: a redacted/hidden person's real names must not leak.
+    if await privacy.get_membership_role(session, viewer_id, tree.id) is None:
+        from app.services import public_view_service
+
+        return await public_view_service.list_public_person_names(
+            session, viewer_id=viewer_id, tree=tree, person_id=person_id
+        )
     stmt = (
         select(Name)
         .where(Name.person_id == person_id, Name.deleted_at.is_(None))

backend/app/services/public_view_service.py

@@ -19,11 +19,12 @@ surface can't be used to probe whether a private tree exists.
 
 import uuid
 
-from sqlalchemy import select
+from sqlalchemy import or_, select
 from sqlalchemy.ext.asyncio import AsyncSession
 
 from app.models.enums import TreeVisibility
 from app.models.event import Event
+from app.models.media import Media
 from app.models.person import Name, Person
 from app.models.relationship import Relationship
 from app.models.tree import Tree
@@ -234,6 +235,67 @@ async def list_public_person_events(
     )
 
 
+async def list_public_relationships_for_person(
+    session: AsyncSession, *, viewer_id: uuid.UUID | None, tree: Tree, person_id: uuid.UUID
+) -> list[Relationship]:
+    persons = await _persons(session, tree)
+    vis = await _visibility_map(session, viewer_id=viewer_id, tree=tree, persons=persons)
+    if vis.get(person_id) in (None, Visibility.hidden):
+        return []
+    nonhidden = {pid for pid, v in vis.items() if v != Visibility.hidden}
+    rels = list(
+        (
+            await session.execute(
+                select(Relationship).where(
+                    Relationship.tree_id == tree.id,
+                    Relationship.deleted_at.is_(None),
+                    or_(
+                        Relationship.person_from_id == person_id,
+                        Relationship.person_to_id == person_id,
+                    ),
+                )
+            )
+        )
+        .scalars()
+        .all()
+    )
+    return [r for r in rels if r.person_from_id in nonhidden and r.person_to_id in nonhidden]
+
+
+async def list_public_media(
+    session: AsyncSession, *, viewer_id: uuid.UUID | None, tree: Tree
+) -> list[Media]:
+    """Only media linked to a FULL-visibility person. Media without a person (or
+    linked only to an event/source) is not exposed to non-members — a photo of a
+    living person must never leak."""
+    persons = await _persons(session, tree)
+    vis = await _visibility_map(session, viewer_id=viewer_id, tree=tree, persons=persons)
+    full = {pid for pid, v in vis.items() if v == Visibility.full}
+    media = list(
+        (
+            await session.execute(
+                select(Media).where(Media.tree_id == tree.id, Media.deleted_at.is_(None))
+            )
+        )
+        .scalars()
+        .all()
+    )
+    return [m for m in media if m.person_id is not None and m.person_id in full]
+
+
+async def can_view_media(
+    session: AsyncSession, *, viewer_id: uuid.UUID | None, tree: Tree, media: Media
+) -> bool:
+    """Whether a non-member may see/download a single media item: only when it is
+    linked to a FULL-visibility person."""
+    if media.person_id is None:
+        return False
+    vis = await _person_visibility(
+        session, viewer_id=viewer_id, tree=tree, person_id=media.person_id
+    )
+    return vis == Visibility.full
+
+
 async def list_public_trees(
     session: AsyncSession,
     *,

backend/app/services/relationship_service.py

@@ -111,6 +111,13 @@ async def list_relationships(
     """All relationships in the tree — powers the family/pedigree view in one call."""
     if not await privacy.can_view_tree(session, user_id=viewer_id, tree=tree):
         raise Forbidden("not permitted to view this tree")
+    # Non-members: drop relationships touching a hidden person.
+    if await privacy.get_membership_role(session, viewer_id, tree.id) is None:
+        from app.services import public_view_service
+
+        return await public_view_service.list_public_relationships(
+            session, viewer_id=viewer_id, tree=tree
+        )
     stmt = (
         select(Relationship)
         .where(Relationship.tree_id == tree.id, Relationship.deleted_at.is_(None))
@@ -124,6 +131,12 @@ async def list_relationships_for_person(
 ) -> list[Relationship]:
     if not await privacy.can_view_tree(session, user_id=viewer_id, tree=tree):
         raise Forbidden("not permitted to view this tree")
+    if await privacy.get_membership_role(session, viewer_id, tree.id) is None:
+        from app.services import public_view_service
+
+        return await public_view_service.list_public_relationships_for_person(
+            session, viewer_id=viewer_id, tree=tree, person_id=person_id
+        )
     stmt = (
         select(Relationship)
         .where(

backend/tests/test_authed_nonmember_redaction.py

@@ -0,0 +1,121 @@
+"""Authed non-member reads must redact PER-PERSON, not just gate on the tree.
+
+A logged-in user who is NOT a member of a public tree previously saw living
+people's dates, real alternate names, and media through the family-view
+endpoints — only the person *list* was redacted. These tests assert that leak is
+closed while members still see everything.
+"""
+
+from tests.conftest import auth, register
+
+LSURNAME = "Authleaksurname"
+LALIAS = "Authleakalias"
+LYEAR = "2003"
+
+
+async def _setup(client):
+    owner = auth(await register(client, "anm-owner@ex.com"))
+    tid = (
+        await client.post(
+            "/api/v1/trees", json={"name": "Pub", "visibility": "public"}, headers=owner
+        )
+    ).json()["id"]
+    old = (
+        await client.post(
+            f"/api/v1/trees/{tid}/persons",
+            json={"given": "Olde", "surname": "Gone", "is_living": False},
+            headers=owner,
+        )
+    ).json()["id"]
+    young = (
+        await client.post(
+            f"/api/v1/trees/{tid}/persons",
+            json={"given": "Youngauth", "surname": LSURNAME, "is_living": True},
+            headers=owner,
+        )
+    ).json()["id"]
+    for pid, year in ((old, "1855"), (young, LYEAR)):
+        await client.post(
+            f"/api/v1/trees/{tid}/events",
+            json={"event_type": "birth", "person_id": pid, "date_value": year},
+            headers=owner,
+        )
+    await client.post(
+        f"/api/v1/trees/{tid}/persons/{young}/names",
+        json={"name_type": "alias", "given": LALIAS},
+        headers=owner,
+    )
+    om = (
+        await client.post(
+            f"/api/v1/trees/{tid}/media",
+            files={"file": ("o.txt", b"old-photo", "text/plain")},
+            data={"person_id": old},
+            headers=owner,
+        )
+    ).json()["id"]
+    ym = (
+        await client.post(
+            f"/api/v1/trees/{tid}/media",
+            files={"file": ("y.txt", b"young-photo", "text/plain")},
+            data={"person_id": young},
+            headers=owner,
+        )
+    ).json()["id"]
+    return owner, tid, old, young, om, ym
+
+
+async def test_authed_nonmember_does_not_see_living_pii(client):
+    owner, tid, old, young, om, ym = await _setup(client)
+    stranger = auth(await register(client, "anm-stranger@ex.com"))
+
+    # Living person's events dropped; deceased kept.
+    events = (await client.get(f"/api/v1/trees/{tid}/events", headers=stranger)).json()
+    assert any(e["person_id"] == old for e in events)
+    assert not any(e["person_id"] == young for e in events)
+
+    # Per-person living: names + events empty.
+    assert (
+        await client.get(f"/api/v1/trees/{tid}/persons/{young}/names", headers=stranger)
+    ).json() == []
+    assert (
+        await client.get(f"/api/v1/trees/{tid}/persons/{young}/events", headers=stranger)
+    ).json() == []
+
+    # The living surname/alias/birth-year must not appear in any of these.
+    for path in (
+        f"/api/v1/trees/{tid}/events",
+        f"/api/v1/trees/{tid}/relationships",
+        f"/api/v1/trees/{tid}/persons/{young}/names",
+        f"/api/v1/trees/{tid}/media",
+    ):
+        body = (await client.get(path, headers=stranger)).text
+        assert LSURNAME not in body, path
+        assert LALIAS not in body, path
+        assert LYEAR not in body, path
+
+    # Media: living person's media hidden from the list and undownloadable;
+    # deceased person's media is fine.
+    media_ids = {m["id"] for m in (await client.get(f"/api/v1/trees/{tid}/media", headers=stranger)).json()}
+    assert om in media_ids
+    assert ym not in media_ids
+    assert (
+        await client.get(f"/api/v1/trees/{tid}/media/{ym}/content", headers=stranger)
+    ).status_code == 404
+    assert (
+        await client.get(f"/api/v1/trees/{tid}/media/{om}/content", headers=stranger)
+    ).status_code == 200
+
+
+async def test_member_still_sees_everything(client):
+    owner, tid, old, young, om, ym = await _setup(client)
+
+    events = (await client.get(f"/api/v1/trees/{tid}/events", headers=owner)).json()
+    assert any(e["person_id"] == young for e in events)
+    assert (
+        await client.get(f"/api/v1/trees/{tid}/persons/{young}/names", headers=owner)
+    ).json() != []
+    member_media = {m["id"] for m in (await client.get(f"/api/v1/trees/{tid}/media", headers=owner)).json()}
+    assert ym in member_media
+    assert (
+        await client.get(f"/api/v1/trees/{tid}/media/{ym}/content", headers=owner)
+    ).status_code == 200