peter
8c51062e5b
Round 6 converted three of four `is_demo` 403s to a structured detail and wrote a docstring saying it
had converted all the sites there are. The account-delete route in `routes/me.py` still answered a
hand-written English sentence, which `notifyActionError` discards in favour of the caller's
translated fallback — so the demo user gets no hint that the demo account is the reason. It is
UI-gated today, so the string is unreachable; what was reachable was the false completeness claim,
which is precisely what stops the next reader from looking. The site is converted, and the list is
now DERIVED from the source: a new test walks `app/` for every `is_demo`-guarded 403, asserts each
raises a structured detail, and fails when the count moves — so this file cannot fall behind the code
the way it just did.
The same round's frontend contract guard was failing open. Its pattern required a bare identifier
before `.detail` and excluded `.detail` used as a last call argument, so `setErr(e.detail)`,
`(e as HttpError).detail`, `String(e.detail)` and four more idiomatic forms were invisible — and
because the file set is compared with `toEqual`, a file the pattern cannot see is simply never listed
and the gate stays green over a fresh copy of the contract. Widened to cover cast receivers, optional
chains and call-argument reads; the pinned set is unchanged, so the narrowing had removed no false
positive. A destructured `detail` stays out of reach (a pattern loose enough for it fires on
react-query's own `const { data: detail }`), and that hole is written into the test rather than
papered over with two innocent files.
Its anti-vacuity guard also did not guard anything: the comment says the glob must have matched
something, but the assertion under it read a sibling file by its own URL, which resolves whether or
not the glob enumerated anything. That only mattered while the pinned list was non-empty — the moment
the deferral sweep lands and it becomes [], the whole file would pass over a glob matching zero
files. The scan size is asserted now.
91 lines
4.8 KiB
Python
91 lines
4.8 KiB
Python
"""The shared demo account's refusal says WHY, in the reader's language (deep-review round 6).
|
|
|
|
`require_human` answered `403 "Not available in the demo account."` — a plain string. `req` leaves
|
|
403 to the caller, and round 5 taught `notifyActionError` to discard a plain-string detail in favour
|
|
of the caller's already-translated fallback, so from that commit on the demo user got a generic
|
|
"couldn't do that" and no hint that the demo account was the reason. Not every affordance behind
|
|
this gate is UI-gated on `is_demo` either — the comfort shelf is not, so the pin button is live.
|
|
|
|
A structured `{code: "demo"}` is what `localizeDetail` can turn into `errors.demo` in both locales.
|
|
Pinned at all FOUR sites: the shared dependency, the two auth routes that hand-roll the same check,
|
|
and the account-delete route in `routes/me.py` — which round 6 missed while its docstring claimed
|
|
all the sites were covered, so the next reader had no reason to look (round 7). An asymmetric fix is
|
|
the smell this repo bans, and a docstring asserting symmetry it does not have is worse than none.
|
|
|
|
The list is derived from the source, not typed out: `test_every_is_demo_refusal_is_on_the_list`
|
|
below fails when a fifth `is_demo` 403 appears anywhere in `app/`, so this file cannot fall behind
|
|
the code the way round 6's did. Needs the DB.
|
|
"""
|
|
|
|
import re
|
|
from pathlib import Path
|
|
|
|
from app.models import User
|
|
|
|
|
|
def test_every_demo_refusal_carries_the_same_structured_reason(db, make_user, as_user):
|
|
demo = make_user(email="demo@example.com", is_demo=True)
|
|
client = as_user(demo)
|
|
for method, url, body in [
|
|
# `require_human` itself, through an affordance the UI does NOT gate on is_demo.
|
|
("post", "/api/discover/pins", {"kind": "video", "ref": "v1"}),
|
|
# The three routes that repeat the check inline rather than depending on the gate.
|
|
("post", "/auth/logout-others", None),
|
|
("post", "/auth/set-password", {"password": "a long enough password"}),
|
|
("delete", "/api/me/account", None),
|
|
]:
|
|
r = getattr(client, method)(url, **({"json": body} if body is not None else {}))
|
|
assert r.status_code == 403, (url, r.text)
|
|
detail = r.json()["detail"]
|
|
# Every one of them is a dict whose `code` the frontend can localize; the delete route adds
|
|
# a `reason` so its refusal can be worded specifically.
|
|
assert isinstance(detail, dict), (url, detail)
|
|
assert detail["code"] == "demo", (url, detail)
|
|
|
|
|
|
def test_every_is_demo_refusal_is_on_the_list():
|
|
"""A fifth site must not be able to appear unnoticed.
|
|
|
|
Round 6 converted three of four `is_demo` 403s and wrote a docstring saying it had converted
|
|
them all; the fourth kept answering a bare English string for another round because nothing
|
|
counted them. So count them here: every `is_demo`-guarded 403 in `app/` has to raise a
|
|
structured detail, and the number of them has to match what this file exercises."""
|
|
app_dir = Path(__file__).resolve().parents[1] / "app"
|
|
sites = []
|
|
for path in sorted(app_dir.rglob("*.py")):
|
|
lines = path.read_text(encoding="utf-8").splitlines()
|
|
for i, line in enumerate(lines):
|
|
# `if user.is_demo:` / `if u.is_demo:` — the hand-rolled gate, not the ORM filters
|
|
# (`User.is_demo.is_(False)`) and not the column definition.
|
|
if not re.search(r"^\s*if\s+\w+\.is_demo\s*:", line):
|
|
continue
|
|
block = "\n".join(lines[i : i + 6])
|
|
if "403" not in block:
|
|
continue
|
|
sites.append(f"{path.relative_to(app_dir)}:{i + 1}")
|
|
assert 'detail={"code": "demo"' in block, (
|
|
f"{path.relative_to(app_dir)}:{i + 1} refuses the demo account with an "
|
|
f"unstructured detail — the frontend cannot localize it:\n{block}"
|
|
)
|
|
# auth.py's shared gate + routes/auth.py's two + routes/me.py's delete. A new one is welcome,
|
|
# but it has to be added to the loop above in the same commit.
|
|
assert len(sites) == 4, sites
|
|
|
|
|
|
def test_a_real_account_is_not_refused_by_the_gate(db, make_user, as_user):
|
|
"""The other half: the structured detail must not come from having broken the gate open the
|
|
other way. A non-demo account gets past `require_human` and fails (or succeeds) on its own
|
|
merits — anything but a 403 demo refusal."""
|
|
client = as_user(make_user(email="real@example.com"))
|
|
r = client.post("/auth/logout-others")
|
|
assert r.status_code == 200, r.text
|
|
assert not isinstance(r.json().get("detail"), dict)
|
|
|
|
|
|
def test_the_gate_reads_the_flag_and_not_the_email(db):
|
|
"""`is_demo` is a column, not a convention about the address — a real account that happens to
|
|
be called demo@ must not be refused."""
|
|
from app.auth import require_human
|
|
|
|
assert require_human(User(email="demo@example.com", is_demo=False)) is not None
|