phase: 101_tokens_page_overhaul

All gates verified green — no defects found, nothing to fix. Final report: ## Phase 101 — final verification pass (all tasks already complete; verified, not re-implemented) **Verified against the working tree** (task files 01–04 present in `complete/`): - Active/revoked table split (Status column gone, section hidden while 0 revoked, per-table search with query state surviving re-renders) in `frontend/index.html` + `tokens.js` - `generateToken()` refuses blank/whitespace names (`Give the token a name first.`, no request); `|| "token"` fallback deleted - Atomic rotation: `regenerate_token` service + `POST /api/tokens/{id}/regenerate` (201 new-plaintext-once / 404 / 409 / router-wide 403) + `TokenListItem.revoked_at` (D5) - Regenerate two-step confirm UI + CSS (`.token-regenerate`, neutral hover, no new hue); A4 pins intact **Test / lint / coverage results:** - `uv run pytest` → **2065 passed** - `uv run pytest --cov=app --cov-report=term-missing` → **TOTAL 99%** (>90% ✓) - `uv run ruff check . && uv run pyright` → clean (0 errors) - `uv run pytest tests/e2e/test_tokens_page.py -v --no-cov` → **4 passed** (isolation, DB up) - Regression, each in isolation: `test_api_tokens.py` **9 passed**, `test_admin_auth.py` **6 passed**, `test_shared_header.py` **6 passed**, `test_theme_semantic_completion.py` **8 passed** (its revoked-pill pin was correctly re-scoped to the revoked table in this phase) **Completion criteria:** 1 ✓ split+search (E2E 1–2) · 2 ✓ required name (E2E 3 + source pin) · 3 ✓ rotation end-to-end, old token refused at gate (E2E 4 + API 404/409 pinned) · 4 ✓ A4 holds (list carries no plaintext/hashes) · 5 ✓ suite/coverage/lint green · 6 ✓ E2E + regressions green in isolation · 7 commit left to the harness per executor rules (all changes uncommitted in the working tree) **Deviations:** none. Next pending phase: `98_sync_summary_visibility`.
2026-09-12 15:16:02 -04:00
parent 58e9d94cff
commit 4dbac1660a
32 changed files with 2321 additions and 150 deletions
@@ -4,9 +4,9 @@ The admin surface for issued access tokens, driven through the real app
 (TestClient keeps the cookie jar — the house ``test_auth_api`` admin-login
 pattern):

-* anonymous → 403 ``admin only`` on all three endpoints (router-level
-  ``require_admin``; a token USER, once task 03 lands, is 403 here too —
-  pinned in task 03's matrix);
+* anonymous → 403 ``admin only`` on every route (router-level
+  ``require_admin``; a token USER is 403 on the admin surface too —
+  pinned in task 03's matrix and, for the regenerate route, here);
 * create → 201 with the plaintext ``token`` (the ONE wire moment it
  exists, A4) — and ``GET /tokens`` NEVER exposes it: no ``token`` key,
  no ``token_hash`` key, and the hash string itself absent from the
@@ -14,9 +14,16 @@ pattern):
 * labels are display-only and NOT unique (two tokens, one label);
 * blank/over-long labels → 422 (the house ``ValueError`` pattern);
 * revoke → 204, idempotent (re-revoke 204, original stamp kept), the
-  list shows ``revoked: true`` and the row keeps its ``last_used_at``;
-  unknown id → 404 ``token not found``;
-* the list is newest-first (``created_at desc``).
+  list shows ``revoked: true`` + the ``revoked_at`` wire timestamp and
+  the row keeps its ``last_used_at``; unknown id → 404 ``token not
+  found``;
+* regenerate (phase 101, D2) → 201 with the NEW plaintext (same label,
+  one wire moment, A4) — the old row is revoked in the SAME transaction
+  (``revoked: true`` + non-null ``revoked_at`` in the follow-up list);
+  already-revoked id → 409 ``token already revoked``; unknown id → 404
+  ``token not found``;
+* the list items carry ``revoked_at`` (null active / ISO-8601 revoked —
+  D5, wire-additive) and are newest-first (``created_at desc``).

 Real Postgres (``podman compose up -d db``); no LLM involved — tokens
 are plain rows, so the suite is deterministic without a fake.
@@ -36,7 +43,9 @@ from fastapi.testclient import TestClient
 from sqlalchemy import select, text, update
 from sqlalchemy.orm import Session

+from app.main import app as fastapi_app
 from app.models import ApiToken
+from tests.conftest import ADMIN_PASSWORD

 TOKEN_SHAPE = re.compile(r"^bor_[0-9a-f]{32}$")
 #: A fixed "already used" stamp — the row keeps it through revocation
@@ -62,9 +71,21 @@ def _create(admin_client: TestClient, label: str = "alice") -> dict:
    return r.json()


-def test_anonymous_403_on_all_three(client: TestClient) -> None:
+def _admin_client() -> TestClient:
+    """A SEPARATE client signed in as the admin — for tests that need an
+    admin and a token user at the same time (the shared ``client``
+    fixture is the token holder in those; the ``test_auth_api``
+    pattern)."""
+    admin = TestClient(fastapi_app)
+    r = admin.post("/api/login", json={"password": ADMIN_PASSWORD})
+    assert r.status_code == 204, f"admin login failed: {r.status_code} {r.text}"
+    return admin
+
+
+def test_anonymous_403_on_all_routes(client: TestClient) -> None:
    """Router-level ``require_admin``: every route is 403 for the
-    unsigned-in caller (one fixed detail — no enumeration)."""
+    unsigned-in caller (one fixed detail — no enumeration), including
+    the phase-101 regenerate route."""
    r = client.post("/api/tokens", json={"label": "anon"})
    assert r.status_code == 403
    assert r.json() == {"detail": "admin only"}
@@ -74,6 +95,9 @@ def test_anonymous_403_on_all_three(client: TestClient) -> None:
    r = client.post(f"/api/tokens/{uuid.uuid4()}/revoke")
    assert r.status_code == 403
    assert r.json() == {"detail": "admin only"}
+    r = client.post(f"/api/tokens/{uuid.uuid4()}/regenerate")
+    assert r.status_code == 403
+    assert r.json() == {"detail": "admin only"}


 def test_admin_create_returns_plaintext_exactly_once(
@@ -103,11 +127,20 @@ def test_admin_create_returns_plaintext_exactly_once(
    items = r.json()["tokens"]
    assert len(items) == 1
    item = items[0]
-    assert set(item) == {"id", "label", "created_at", "last_used_at", "revoked"}
+    # Phase 101, D5: the wire gains ``revoked_at`` — null while active.
+    assert set(item) == {
+        "id",
+        "label",
+        "created_at",
+        "last_used_at",
+        "revoked",
+        "revoked_at",
+    }
    assert item["id"] == body["id"]
    assert item["label"] == "alice"
    assert item["last_used_at"] is None  # not used yet
    assert item["revoked"] is False
+    assert item["revoked_at"] is None
    serialized = r.text
    assert body["token"] not in serialized
    assert row.token_hash not in serialized
@@ -163,6 +196,11 @@ def test_revoke_204_idempotent_and_preserves_last_used(
    ).scalar_one().revoked_at
    assert original_stamp is not None

+    # Phase 101, D5: the revoked row carries the revocation timestamp on
+    # the wire — ISO-8601, the SAME instant as the DB stamp (the
+    # revoked table renders it).
+    assert datetime.fromisoformat(item["revoked_at"]) == original_stamp
+
    # Re-revoke: still 204, the original stamp survives (no re-stamp).
    assert admin_client.post(f"/api/tokens/{token_id}/revoke").status_code == 204
    db.expire_all()
@@ -173,6 +211,120 @@ def test_revoke_204_idempotent_and_preserves_last_used(
    ) == USED_STAMP


+# ---------- phase 101, task 01: the atomic rotation ----------
+
+
+def test_regenerate_201_rotates_atomically(admin_client: TestClient, db: Session) -> None:
+    """201 carries the NEW plaintext (same label, one wire moment, A4);
+    the old row is revoked in the SAME transaction — the follow-up list
+    shows it ``revoked: true`` with a non-null ISO ``revoked_at``, while
+    the new row is active (``revoked_at`` null), same label, with a
+    newer ``created_at``."""
+    body = _create(admin_client, "dave")
+    token_id = uuid.UUID(body["id"])
+    original = body["token"]
+
+    # A deterministic created_at gap (transaction timestamps can be
+    # coarser than the create gap — the test_list_is_newest_first
+    # pattern), so "newer" is a strict comparison.
+    old_created = datetime.now(UTC) - timedelta(hours=1)
+    db.execute(
+        update(ApiToken)
+        .where(ApiToken.id == token_id)
+        .values(created_at=old_created)
+    )
+    db.commit()
+
+    r = admin_client.post(f"/api/tokens/{token_id}/regenerate")
+    assert r.status_code == 201, r.text
+    rotated = r.json()
+
+    # The 201 shape is TokenCreated — the NEW token's one plaintext
+    # moment (the successor row, same label, a fresh credential).
+    assert set(rotated) == {"id", "label", "token", "created_at"}
+    assert rotated["id"] != body["id"]
+    assert rotated["label"] == "dave"
+    assert TOKEN_SHAPE.fullmatch(rotated["token"]), rotated["token"]
+    assert rotated["token"] != original
+    assert datetime.fromisoformat(rotated["created_at"]) > old_created
+
+    # The successor row stores the sha256 of the NEW plaintext only.
+    row = db.execute(
+        select(ApiToken).where(ApiToken.id == uuid.UUID(rotated["id"]))
+    ).scalar_one()
+    assert row.token_hash == hashlib.sha256(rotated["token"].encode("utf-8")).hexdigest()
+    assert row.revoked_at is None
+
+    # The follow-up list: newest first, the old row revoked (D5 wire
+    # timestamp), the new row active.
+    items = admin_client.get("/api/tokens").json()["tokens"]
+    assert [i["id"] for i in items] == [rotated["id"], body["id"]]
+    by_id = {i["id"]: i for i in items}
+    old_item, new_item = by_id[body["id"]], by_id[rotated["id"]]
+    assert old_item["revoked"] is True
+    assert old_item["revoked_at"] is not None
+    datetime.fromisoformat(old_item["revoked_at"])  # ISO-8601
+    assert new_item["revoked"] is False
+    assert new_item["revoked_at"] is None
+    assert old_item["label"] == new_item["label"] == "dave"
+
+    # A4: the list never carries either plaintext or any hash string.
+    serialized = admin_client.get("/api/tokens").text
+    assert original not in serialized
+    assert rotated["token"] not in serialized
+
+
+def test_regenerate_unknown_id_404(admin_client: TestClient) -> None:
+    """One fixed message for every unknown id (the revoke endpoint's)."""
+    r = admin_client.post(f"/api/tokens/{uuid.uuid4()}/regenerate")
+    assert r.status_code == 404
+    assert r.json() == {"detail": "token not found"}
+
+
+def test_regenerate_already_revoked_409(admin_client: TestClient, db: Session) -> None:
+    """A dead token cannot be rotated: 409 ``token already revoked``,
+    and the row is untouched — still ONE row, the original stamp, no
+    successor created."""
+    body = _create(admin_client, "erin")
+    token_id = uuid.UUID(body["id"])
+    assert admin_client.post(f"/api/tokens/{token_id}/revoke").status_code == 204
+    db.expire_all()
+    original_stamp = db.execute(
+        select(ApiToken).where(ApiToken.id == token_id)
+    ).scalar_one().revoked_at
+    assert original_stamp is not None
+
+    r = admin_client.post(f"/api/tokens/{token_id}/regenerate")
+    assert r.status_code == 409
+    assert r.json() == {"detail": "token already revoked"}
+
+    db.expire_all()
+    assert db.execute(text("SELECT count(*) FROM api_tokens")).scalar_one() == 1
+    row = db.execute(select(ApiToken).where(ApiToken.id == token_id)).scalar_one()
+    assert row.revoked_at == original_stamp
+
+
+def test_regenerate_403_for_token_user(client: TestClient) -> None:
+    """Router-wide gate: a token USER (signed in via /api/token-auth)
+    cannot rotate a token — 403 ``admin only``, and the rotation did not
+    happen (the shared ``client`` is the token holder; a separate client
+    is the admin — the ``test_auth_api`` pattern)."""
+    admin = _admin_client()
+    body = _create(admin, "frank")
+    assert client.post("/api/token-auth", json={"token": body["token"]}).status_code == 204
+    assert client.get("/api/whoami").json() == {"authenticated": True, "role": "user"}
+
+    r = client.post(f"/api/tokens/{body['id']}/regenerate")
+    assert r.status_code == 403
+    assert r.json() == {"detail": "admin only"}
+
+    # The token is untouched: one active row, the admin's list agrees.
+    items = admin.get("/api/tokens").json()["tokens"]
+    assert len(items) == 1
+    assert items[0]["id"] == body["id"]
+    assert items[0]["revoked"] is False
+
+
 def test_revoke_unknown_id_404(admin_client: TestClient) -> None:
    """One fixed message for every unknown id (no enumeration)."""
    r = admin_client.post(f"/api/tokens/{uuid.uuid4()}/revoke")