P1: gate members documents per unit, session auth on the admin API

documents.visible() now takes the viewer's unit set. A members document is
served and listed only to a signed-in member of the matching unit; 'both'
reaches any member; owner and admin reach everything including unit types
added later. Everyone else gets 404, never 403.

The gate moved INSIDE find(), so there is one path from a slug to a file and
no route can forget to check. listed() and visible() stay separate functions.

admin_api takes a session first and falls back to X-Admin-Token as break
glass. Still fails closed: no session and no ADMIN_TOKEN is 503. Capability,
not role, decides per route. announcements.created_by now comes from the
session and ignores any value in the request body.

tests/smoke_documents.py, 24 checks, including the invariant that the index
can never list something serving would refuse.

app/admin_api.py

@@ -19,17 +19,31 @@ shows, it comes down. Writing it is the entire feature. The caps that keep
 the banner from becoming a mess live in store.py, not here, so the panel and
 any future client inherit them rather than reimplementing them.
 
-Auth: every route requires the X-Admin-Token header to match ADMIN_TOKEN.
-If ADMIN_TOKEN is unset the whole router returns 503 - it FAILS CLOSED. These
-endpoints expose parent names, emails and phone numbers for minors' families,
-so an unconfigured deployment must not serve them.
+Auth, two ways, session first.
+
+A signed-in person holding the required capability is allowed. Otherwise the
+X-Admin-Token header must match ADMIN_TOKEN, which is retained as BREAK GLASS:
+if the identity layer is broken there has to be a way in that does not depend
+on the identity layer.
+
+Still FAILS CLOSED. With no session and no ADMIN_TOKEN set, every route returns
+503. These endpoints expose parent names, emails and phone numbers for minors'
+families, so an unconfigured deployment must not serve them. Adding sessions
+widened who may read; it did not soften what happens when nothing is
+configured.
+
+Capability, not role, decides. Lead routes need leads:read, announcement routes
+need announcements:write, and both are answered by identity.can() against the
+one CAPS dictionary - never by a role comparison here.
 """
 
 import hmac
 import os
 
-from fastapi import APIRouter, Body, Header, HTTPException, Query
+from fastapi import APIRouter, Body, Header, HTTPException, Query, Request
 
+import auth
+import identity
 import store
 
 ADMIN_TOKEN = os.environ.get("ADMIN_TOKEN", "").strip()
@@ -37,30 +51,42 @@ ADMIN_TOKEN = os.environ.get("ADMIN_TOKEN", "").strip()
 router = APIRouter(prefix="/api/admin", tags=["admin"])
 
 
-def _auth(token):
+def _auth(request, token, capability):
+    """Authorise, and return a label naming who acted, for created_by.
+
+    Order matters: session first, so a normal signed-in leader never depends on
+    the shared token, and the token stays a fallback rather than the everyday
+    path.
+    """
+    person = auth.current_person(request)
+    if person:
+        if not identity.can(person, capability):
+            raise HTTPException(403, "your account does not have %s" % capability)
+        return person["email"]
     if not ADMIN_TOKEN:
-        raise HTTPException(503, "admin API disabled: ADMIN_TOKEN is not set")
+        raise HTTPException(503, "admin API disabled: sign in, or set ADMIN_TOKEN")
     if not token or not hmac.compare_digest(token, ADMIN_TOKEN):
-        raise HTTPException(401, "bad or missing X-Admin-Token")
+        raise HTTPException(401, "sign in, or send a valid X-Admin-Token")
+    return "admin-token"
 
 
 @router.get("/summary")
-def get_summary(x_admin_token: str = Header(None)):
-    _auth(x_admin_token)
+def get_summary(request: Request, x_admin_token: str = Header(None)):
+    _auth(request, x_admin_token, "leads:read")
     return store.summary()
 
 
 @router.get("/leads")
-def get_leads(since: str = None, q: str = None,
+def get_leads(request: Request, since: str = None, q: str = None,
               limit: int = Query(100, ge=1, le=500), offset: int = 0,
               x_admin_token: str = Header(None)):
-    _auth(x_admin_token)
+    _auth(request, x_admin_token, "leads:read")
     return {"leads": store.list_leads(since=since, q=q, limit=limit, offset=offset)}
 
 
 @router.get("/leads/{record_id}")
-def get_one(record_id: str, x_admin_token: str = Header(None)):
-    _auth(x_admin_token)
+def get_one(request: Request, record_id: str, x_admin_token: str = Header(None)):
+    _auth(request, x_admin_token, "leads:read")
     rec = store.get_lead(record_id)
     if not rec:
         raise HTTPException(404, "no such lead")
@@ -68,16 +94,16 @@ def get_one(record_id: str, x_admin_token: str = Header(None)):
 
 
 @router.get("/mirrors/failed")
-def failed(target: str = "google_sheet", x_admin_token: str = Header(None)):
-    _auth(x_admin_token)
+def failed(request: Request, target: str = "google_sheet", x_admin_token: str = Header(None)):
+    _auth(request, x_admin_token, "leads:read")
     return {"target": target, "leads": store.failed_mirror_records(target)}
 
 
 @router.post("/mirrors/retry")
-def retry(target: str = "google_sheet", x_admin_token: str = Header(None)):
+def retry(request: Request, target: str = "google_sheet", x_admin_token: str = Header(None)):
     """Replay leads whose copy to an external target failed. Idempotent-ish:
     a lead already marked ok is never retried."""
-    _auth(x_admin_token)
+    _auth(request, x_admin_token, "leads:read")
     if target != "google_sheet":
         raise HTTPException(422, "only google_sheet retry is implemented")
     import app as main_app
@@ -106,25 +132,25 @@ def _reject(e):
 
 
 @router.get("/announcements")
-def list_announcements(include_expired: bool = False,
+def list_announcements(request: Request, include_expired: bool = False,
                        limit: int = Query(100, ge=1, le=500),
                        x_admin_token: str = Header(None)):
     """Every announcement with its computed state: live, scheduled, expired,
     revoked, or over_cap. State is returned rather than left to be inferred
     from what the homepage happens to render."""
-    _auth(x_admin_token)
+    _auth(request, x_admin_token, "announcements:write")
     return {"announcements": store.list_announcements(
         include_expired=include_expired, limit=limit)}
 
 
 @router.post("/announcements", status_code=201)
-def create_announcement(payload: dict = Body(...), x_admin_token: str = Header(None)):
+def create_announcement(request: Request, payload: dict = Body(...), x_admin_token: str = Header(None)):
     """Post a notice. ends_at is required.
 
     422 if the message is over the character cap or the window is invalid.
     409 if the live cap is already reached, listing what is up so you can
     decide what to revoke."""
-    _auth(x_admin_token)
+    _actor = _auth(request, x_admin_token, "announcements:write")
     try:
         return store.create_announcement(
             message=payload.get("message"),
@@ -133,16 +159,21 @@ def create_announcement(payload: dict = Body(...), x_admin_token: str = Header(N
             level=payload.get("level", "info"),
             link_url=payload.get("link_url"),
             link_text=payload.get("link_text"),
-            created_by=payload.get("created_by"),
+            # From the session, never from the body. A caller must not be able
+            # to attribute a public notice to somebody else. Stored as the
+            # email rather than the uuid: the column is display-facing, people
+            # are never hard deleted, and a uuid in a banner audit trail helps
+            # nobody read it.
+            created_by=_actor,
         )
     except store.AnnouncementRejected as e:
         raise _reject(e)
 
 
 @router.delete("/announcements/{announcement_id}")
-def revoke_announcement(announcement_id: str, x_admin_token: str = Header(None)):
+def revoke_announcement(request: Request, announcement_id: str, x_admin_token: str = Header(None)):
     """Take one down early. Sets revoked_at; never deletes the row."""
-    _auth(x_admin_token)
+    _auth(request, x_admin_token, "announcements:write")
     if not store.get_announcement(announcement_id):
         raise HTTPException(404, "no such announcement")
     if not store.revoke_announcement(announcement_id):

app/app.py

@@ -710,8 +710,10 @@ def doc_row(d):
             f'<span class="docmeta">{meta}</span></a>')
 
 @app.get("/documents", response_class=HTMLResponse)
-def documents_index():
-    groups = documents.listing()
+def documents_index(request: Request):
+    viewer = auth.current_person(request)
+    units = documents.units_for(viewer)
+    groups = documents.listing(units)
     if groups:
         cards = []
         for cat, rows in groups:
@@ -721,6 +723,14 @@ def documents_index():
         inner = "".join(cards)
     else:
         inner = '<div class="noev">Nothing posted here yet. Forms and handouts will land on this page as the program year gets going.</div>'
+    if units:
+        signin_note = ('<div class="wrap" style="padding-top:14px"><div class="note">'
+                       'Signed in, so anything posted for families is included above. '
+                       '<a href="/account">Your account</a></div></div>')
+    else:
+        signin_note = ('<div class="wrap" style="padding-top:14px"><div class="note">'
+                       'Some handouts are posted for registered families only. '
+                       '<a href="/login">Sign in</a> if you have an account.</div></div>')
     body = f"""
 <div class="calhead"><div class="wrap">
 <div class="eyebrow gold">Pack &amp; Troop 73</div>
@@ -730,12 +740,15 @@ def documents_index():
 <section style="padding:52px 0 70px"><div class="wrap">{inner}</div></section>
 <section style="padding:0 0 80px"><div class="wrap"><div class="note">
 Can't find something, or need a form in another format? Message us on Facebook or catch a leader on a {MEETING_DAY} night.
-</div></div></section>"""
+</div></div>{signin_note}</section>"""
     return page("Forms & Documents · Pack & Troop 73", body, "docs")
 
 @app.get("/documents/{slug}")
-def document_file(slug: str):
-    d = documents.find(slug)
+def document_file(slug: str, request: Request):
+    # A members document the viewer cannot reach is indistinguishable from one
+    # that does not exist. That is deliberate: 404, never 403, because a 403
+    # confirms the document to someone holding nothing but a guessed slug.
+    d = documents.find(slug, documents.units_for(auth.current_person(request)))
     if d is None:
         body = """
 <div class="calhead"><div class="wrap">

app/documents.py

@@ -23,9 +23,19 @@ visibility:
                 that need a durable link before member login exists. This is
                 obscurity, not access control: treat an unlisted link as
                 forwardable, because it is.
-    "members"   reserved for the login that does not exist yet. Until it does,
-                these are hidden AND unservable, returning 404 rather than 403,
-                because a 403 advertises a document we cannot actually gate yet.
+    "members"   served only to a signed-in member of the right unit, and listed
+                only for them. Everyone else gets 404, never 403: a 403
+                advertises the existence of a document to someone who cannot
+                have it, and the slug is the only thing they would need.
+
+Unit scoping. A document's `unit` is "pack", "troop" or "both". A viewer is
+resolved to the set of unit types they belong to, and "both" is readable by any
+member of any unit. An owner or admin reads everything, including units created
+after their role was granted.
+
+This module deliberately does NOT import identity. It reads a person as a plain
+dict, so the coupling is a data shape rather than a module dependency and the
+gate stays testable on its own.
 """
 
 import datetime
@@ -137,14 +147,62 @@ def manifest():
     return _cache["value"]
 
 
-def visible(doc):
-    """The single gate on SERVING. Member login plugs in here and nowhere else."""
-    return doc.get("visibility") in ("public", "unlisted") and doc["path"].is_file()
+# Every unit type that can appear on a document. A membership in a unit type
+# outside this set (a Venturing crew, say) grants no document access until the
+# manifest vocabulary is widened to match - failing closed is correct here.
+DOC_UNITS = ("pack", "troop")
 
 
-def listed(doc):
-    """The separate, weaker question of whether it appears on the index."""
-    return doc.get("visibility") == "public" and visible(doc)
+def units_for(person):
+    """The document unit tokens a viewer may read. Empty set for anonymous.
+
+    An owner or admin gets everything, including a unit type added later. That
+    is the whole reason global_role is a column rather than a membership row.
+    """
+    if not person or person.get("disabled_at"):
+        return frozenset()
+    if person.get("global_role") in ("owner", "admin"):
+        return frozenset(DOC_UNITS)
+    return frozenset(
+        m.get("unit_type") for m in person.get("memberships", [])
+        if m.get("unit_type") in DOC_UNITS)
+
+
+def _member_ok(doc, units):
+    """Does this viewer's unit set reach this members-only document?
+
+    "both" means the document concerns both units, so any member reaches it.
+    It does not mean "requires membership of both".
+    """
+    if not units:
+        return False
+    return doc.get("unit") == "both" or doc.get("unit") in units
+
+
+def visible(doc, units=None):
+    """The single gate on SERVING. Login attaches here and nowhere else."""
+    if not doc["path"].is_file():
+        return False
+    vis = doc.get("visibility")
+    if vis in ("public", "unlisted"):
+        return True
+    if vis == "members":
+        return _member_ok(doc, units or frozenset())
+    return False
+
+
+def listed(doc, units=None):
+    """The separate, weaker question of whether it appears on the index.
+
+    Kept separate from visible() on purpose. Collapsing the two is how
+    "unlisted" quietly becomes public, or "members" quietly becomes servable.
+    """
+    vis = doc.get("visibility")
+    if vis == "public":
+        return visible(doc, units)
+    if vis == "members":
+        return visible(doc, units)
+    return False
 
 
 def noindex(doc):
@@ -152,10 +210,10 @@ def noindex(doc):
     return doc.get("visibility") == "unlisted"
 
 
-def listing():
+def listing(units=None):
     """Listed documents grouped into their categories, in manifest order."""
     m = manifest()
-    docs = [d for d in m["documents"] if listed(d)]
+    docs = [d for d in m["documents"] if listed(d, units)]
     known = {c["id"] for c in m["categories"]}
     groups = []
     for cat in m["categories"]:
@@ -168,13 +226,18 @@ def listing():
     return groups
 
 
-def find(slug):
+def find(slug, units=None):
+    """The document at this slug, or None if the viewer cannot have it.
+
+    The gate is applied HERE rather than by the caller, so there is exactly one
+    path from a slug to a file and no route can forget to check.
+    """
     slug = (slug or "").strip().lower()
     if not SLUG_RE.match(slug):
         return None
     for d in manifest()["documents"]:
         if d["slug"] == slug:
-            return d if visible(d) else None
+            return d if visible(d, units) else None
     return None
 
 

app/identity.py

@@ -339,7 +339,7 @@ def _person_row(con, r):
     p = dict(r)
     p.pop("password_hash", None)
     p["memberships"] = [dict(m) for m in con.execute(
-        "SELECT m.unit_id, m.role, m.title, u.slug, u.short_name, u.display_name"
+        "SELECT m.unit_id, m.role, m.title, u.slug, u.unit_type, u.short_name, u.display_name"
         " FROM memberships m JOIN units u ON u.id = m.unit_id"
         " WHERE m.person_id = ? ORDER BY u.sort_order", (p["id"],))]
     p["capabilities"] = sorted(effective_caps(p))

tests/smoke_documents.py

@@ -0,0 +1,111 @@
+"""
+smoke_documents.py - the members document gate, per unit.
+
+Builds a throwaway /docs tree and manifest, then checks who can SEE and who can
+LIST each document. Runs in-process, stdlib only.
+
+    python3 tests/smoke_documents.py
+
+The rules under test are the ones that fail silently: a members document must
+be unservable AND unlisted to the wrong viewer, unlisted must stay servable but
+off the index, and listing must never be able to show something serving would
+refuse.
+"""
+
+import json, os, sys, tempfile
+
+TMP = tempfile.mkdtemp()
+DOCS = os.path.join(TMP, "docs"); os.makedirs(DOCS)
+os.environ["DOCS_DIR"] = DOCS
+os.environ["STORE_DB"] = os.path.join(TMP, "smoke.db")
+sys.path.insert(0, os.path.join(os.path.dirname(os.path.abspath(__file__)), "..", "app"))
+
+FILES = ["open.pdf", "hidden.pdf", "packonly.pdf", "trooponly.pdf", "bothunits.pdf"]
+for f in FILES:
+    open(os.path.join(DOCS, f), "w").write("x")
+json.dump({
+    "categories": [{"id": "forms", "title": "Forms", "blurb": ""}],
+    "documents": [
+        {"slug": "open", "file": "open.pdf", "title": "Open", "category": "forms",
+         "visibility": "public", "unit": "both"},
+        {"slug": "hidden", "file": "hidden.pdf", "title": "Hidden", "category": "forms",
+         "visibility": "unlisted", "unit": "both"},
+        {"slug": "packonly", "file": "packonly.pdf", "title": "Pack only", "category": "forms",
+         "visibility": "members", "unit": "pack"},
+        {"slug": "trooponly", "file": "trooponly.pdf", "title": "Troop only", "category": "forms",
+         "visibility": "members", "unit": "troop"},
+        {"slug": "bothunits", "file": "bothunits.pdf", "title": "Both", "category": "forms",
+         "visibility": "members", "unit": "both"},
+    ]}, open(os.path.join(DOCS, "manifest.json"), "w"))
+
+import documents as D
+
+PASS = FAIL = 0
+
+
+def check(label, cond):
+    global PASS, FAIL
+    if cond:
+        PASS += 1; print("  ok   %s" % label)
+    else:
+        FAIL += 1; print("  FAIL %s" % label)
+
+
+def served(units):
+    return {s for s in ("open", "hidden", "packonly", "trooponly", "bothunits")
+            if D.find(s, units) is not None}
+
+
+def listed(units):
+    return {d["slug"] for _, rows in D.listing(units) for d in rows}
+
+
+anon = frozenset()
+pack = D.units_for({"global_role": None,
+                    "memberships": [{"unit_type": "pack", "role": "member"}]})
+troop = D.units_for({"global_role": None,
+                     "memberships": [{"unit_type": "troop", "role": "leader"}]})
+both = D.units_for({"global_role": None,
+                    "memberships": [{"unit_type": "pack", "role": "member"},
+                                    {"unit_type": "troop", "role": "leader"}]})
+admin = D.units_for({"global_role": "admin", "memberships": []})
+crew = D.units_for({"global_role": None,
+                    "memberships": [{"unit_type": "crew", "role": "leader"}]})
+off = D.units_for({"global_role": "owner", "memberships": [], "disabled_at": "2026-01-01T00:00:00+00:00"})
+
+print("units_for")
+check("anonymous gets nothing", D.units_for(None) == frozenset())
+check("pack member -> {pack}", pack == frozenset({"pack"}))
+check("admin -> every unit type", admin == frozenset(D.DOC_UNITS))
+check("crew maps to nothing (manifest has no crew)", crew == frozenset())
+check("disabled owner gets nothing", off == frozenset())
+
+print("\nserving")
+check("anon: public and unlisted only", served(anon) == {"open", "hidden"})
+check("pack member: + pack and both", served(pack) == {"open", "hidden", "packonly", "bothunits"})
+check("troop leader: + troop and both", served(troop) == {"open", "hidden", "trooponly", "bothunits"})
+check("in both units: everything", served(both) == set(FILES and
+      {"open", "hidden", "packonly", "trooponly", "bothunits"}))
+check("admin: everything", served(admin) == {"open", "hidden", "packonly", "trooponly", "bothunits"})
+check("pack member cannot reach troop doc", D.find("trooponly", pack) is None)
+check("crew leader reaches no members doc", served(crew) == {"open", "hidden"})
+
+print("\nlisting")
+check("anon index is public only", listed(anon) == {"open"})
+check("unlisted never listed, for anyone", "hidden" not in listed(admin))
+check("pack member index", listed(pack) == {"open", "packonly", "bothunits"})
+check("troop leader index", listed(troop) == {"open", "trooponly", "bothunits"})
+check("admin index", listed(admin) == {"open", "packonly", "trooponly", "bothunits"})
+
+print("\nlisting can never exceed serving")
+for name, u in (("anon", anon), ("pack", pack), ("troop", troop), ("admin", admin), ("crew", crew)):
+    check("%s: listed is a subset of served" % name, listed(u) <= served(u))
+
+print("\nmissing file on disk")
+os.remove(os.path.join(DOCS, "packonly.pdf"))
+D._cache["key"] = None
+check("a members doc with no file is not served", D.find("packonly", pack) is None)
+check("and not listed", "packonly" not in listed(pack))
+
+print("\n%d passed, %d failed" % (PASS, FAIL))
+sys.exit(1 if FAIL else 0)