The code cited its plan instead of saying why: `E28 review J4`, `(R6 S3b)`, `S1 justified…`,
`deep-review round 5`. That plan lives in the private ops repo, so for every reader of this one the
citation was noise, and a sentence whose subject was the plan step ("round 2 showed…") hid its
lesson behind an id. Each one is gone and the "why" kept: a bare citation is deleted, a sentence
carrying a lesson is reworded around it ("a later attempt showed…").
Measured (`scripts/audit_metrics.py`, product code): 652 lines in 270 files carried an epic code by
the definition the next commit holds (507 / 241 by the old, two-digit-only one) -> 0. Across all
code, tests, scripts and configs: ~1100 epic-code lines plus ~320 sprint/round ids.
Behaviour unchanged, checked mechanically: every changed .py file has the same AST once docstrings
are blanked; css/sh/toml/yml/json files are identical once comments are stripped; the only
non-comment changes in TS/JS are test titles, one description string in a contract table, and five
ESLint messages, whose text is all the gates snapshot diff holds. Not touched: Markdown (docs and
the changelog keep their history), Alembic migrations, TV episode notation (`S2 · E5`), DESIGN.md
ids (`C1`, `B4`, `S3h`), tracker ids (`SEC-16`, `DX-17`).
One stale claim fixed on the way: test_hls_reaper's docstring said the player checkpoints "every
10 s whether or not it is playing", which plex/stream.py calls wrong; it now says what the code does.
273 lines
10 KiB
Python
273 lines
10 KiB
Python
"""The admin-issued set-password link.
|
|
|
|
One mechanism answering three things: SEC-15 (a no-SMTP instance auto-verifies every registration, so
|
|
a pre-empted address cannot be recovered by mail), SEC-16 (a pre-empted address stays takeable until
|
|
its owner resets, and reset needs working mail), and the everyday "no SMTP and someone forgot theirs".
|
|
|
|
The property that must not regress is the one that makes it honest: an admin-issued link proves
|
|
NOTHING about the mailbox — the admin hands it over by whatever means they like — so it must not
|
|
confirm the address the way a mailed reset does.
|
|
"""
|
|
|
|
from app.models import AuthToken
|
|
|
|
|
|
def _link(client_admin, user_id: int):
|
|
r = client_admin.post(f"/api/admin/users/{user_id}/set-password-link")
|
|
assert r.status_code == 200, r.text
|
|
return r.json()["url"].split("#reset=", 1)[1]
|
|
|
|
|
|
def test_the_link_sets_a_password_without_confirming_the_address(db, as_user, make_user, client):
|
|
"""The whole point of a separate token kind. A mailed reset arrives IN the mailbox, so completing
|
|
it proves ownership and confirms the address. An admin-issued link does not, and claiming
|
|
otherwise would let an admin confirm an address nobody ever proved."""
|
|
admin = make_user(role="admin")
|
|
target = make_user(email_verified=False)
|
|
db.commit()
|
|
|
|
token = _link(as_user(admin), target.id)
|
|
r = client.post(
|
|
"/auth/password-reset/confirm", json={"token": token, "password": "a brand new passphrase"}
|
|
)
|
|
assert r.status_code == 200, r.text
|
|
|
|
db.refresh(target)
|
|
assert target.password_hash is not None
|
|
assert target.email_verified is False # the address is still unproven — and says so
|
|
|
|
|
|
def test_a_mailed_reset_still_confirms_the_address(db, as_user, make_user, client):
|
|
"""The mutation check for the test above: if `email_verified` were simply never set, this would
|
|
fail — and the rescue path for "registered but never clicked the link" would be gone."""
|
|
from datetime import timedelta
|
|
|
|
from app.routes.auth import _issue_token
|
|
|
|
user = make_user(email_verified=False)
|
|
db.commit()
|
|
token = _issue_token(db, user, "reset", timedelta(hours=1))
|
|
|
|
r = client.post(
|
|
"/auth/password-reset/confirm", json={"token": token, "password": "a brand new passphrase"}
|
|
)
|
|
assert r.status_code == 200, r.text
|
|
|
|
db.refresh(user)
|
|
assert user.email_verified is True
|
|
|
|
|
|
def test_using_the_link_evicts_whoever_held_the_old_password(db, as_user, make_user, client):
|
|
"""SEC-16's actual remedy: the squatter's session dies the moment the real owner sets a password.
|
|
The epoch bump is what does it, and it is the reason this reuses the reset confirm path rather
|
|
than writing a second one that could forget to bump."""
|
|
target = make_user()
|
|
db.commit()
|
|
before = target.session_epoch
|
|
|
|
admin = make_user(role="admin")
|
|
db.commit()
|
|
token = _link(as_user(admin), target.id)
|
|
assert (
|
|
client.post(
|
|
"/auth/password-reset/confirm",
|
|
json={"token": token, "password": "the owner's own choice"},
|
|
).status_code
|
|
== 200
|
|
)
|
|
|
|
db.refresh(target)
|
|
assert target.session_epoch != before
|
|
|
|
|
|
def test_issuing_does_not_touch_the_current_password(db, as_user, make_user):
|
|
"""An admin misclick must not lock anyone out. Issuing is an offer; only USING it changes
|
|
anything — which is also why it is safe to offer on a perfectly healthy account."""
|
|
target = make_user()
|
|
db.commit()
|
|
original = target.password_hash
|
|
|
|
admin = make_user(role="admin")
|
|
db.commit()
|
|
_link(as_user(admin), target.id)
|
|
|
|
db.refresh(target)
|
|
assert target.password_hash == original
|
|
assert target.password_revoked_at is None
|
|
|
|
|
|
def test_the_link_is_single_use(db, as_user, make_user, client):
|
|
target = make_user()
|
|
admin = make_user(role="admin")
|
|
db.commit()
|
|
|
|
token = _link(as_user(admin), target.id)
|
|
first = client.post(
|
|
"/auth/password-reset/confirm", json={"token": token, "password": "first passphrase here"}
|
|
)
|
|
second = client.post(
|
|
"/auth/password-reset/confirm", json={"token": token, "password": "second passphrase here"}
|
|
)
|
|
assert first.status_code == 200
|
|
assert second.status_code == 400
|
|
|
|
|
|
def test_a_new_link_burns_the_outstanding_ones(db, as_user, make_user, client):
|
|
"""Spending any password token burns the siblings of BOTH kinds. Without that, a spent reset
|
|
would leave a live admin link behind — a second key to a door someone just changed the lock on."""
|
|
target = make_user()
|
|
admin = make_user(role="admin")
|
|
db.commit()
|
|
|
|
first = _link(as_user(admin), target.id)
|
|
second = _link(as_user(admin), target.id)
|
|
|
|
ok = client.post(
|
|
"/auth/password-reset/confirm", json={"token": second, "password": "the newer passphrase"}
|
|
)
|
|
assert ok.status_code == 200
|
|
stale = client.post(
|
|
"/auth/password-reset/confirm", json={"token": first, "password": "the older passphrase"}
|
|
)
|
|
assert stale.status_code == 400
|
|
|
|
|
|
def test_the_probe_recognises_an_admin_link(db, as_user, make_user, client):
|
|
"""`/auth/password-reset/check` is what stops the card drawing a form for a dead link. It peeked
|
|
only at `reset` tokens, so without teaching it the new kind every admin-issued link would open
|
|
onto "this link is spent"."""
|
|
target = make_user()
|
|
admin = make_user(role="admin")
|
|
db.commit()
|
|
|
|
token = _link(as_user(admin), target.id)
|
|
r = client.post("/auth/password-reset/check", json={"token": token})
|
|
assert r.json() == {"ok": True}
|
|
|
|
|
|
def test_only_an_admin_can_mint_one(db, as_user, make_user):
|
|
plain = make_user()
|
|
victim = make_user()
|
|
db.commit()
|
|
|
|
r = as_user(plain).post(f"/api/admin/users/{victim.id}/set-password-link")
|
|
assert r.status_code in (401, 403)
|
|
assert db.query(AuthToken).filter(AuthToken.kind == "setpw").count() == 0
|
|
|
|
|
|
def test_the_access_panel_reports_the_gates(db, as_user, make_user):
|
|
admin = make_user(role="admin")
|
|
target = make_user(email_verified=False, is_active=False)
|
|
db.commit()
|
|
|
|
body = as_user(admin).get(f"/api/admin/users/{target.id}/access").json()
|
|
assert body["email_verified"] is False
|
|
assert body["is_active"] is False
|
|
assert set(body) >= {
|
|
"is_allowed",
|
|
"can_activate",
|
|
"email_configured",
|
|
"password_revoked_at",
|
|
"google_email",
|
|
"has_password",
|
|
"has_google",
|
|
}
|
|
|
|
|
|
def test_the_panel_names_a_google_address_only_when_it_differs(db, as_user, make_user):
|
|
""" "Connected" and "connected as someone else" are different statements, and only the second one
|
|
explains a confusing sign-in. Repeating the account's own address would be noise on every row."""
|
|
admin = make_user(role="admin")
|
|
same = make_user(email="same@example.com")
|
|
same.google_email = "same@example.com"
|
|
other = make_user(email="typed@example.com")
|
|
other.google_email = "google-primary@example.com"
|
|
db.commit()
|
|
|
|
c = as_user(admin)
|
|
assert c.get(f"/api/admin/users/{same.id}/access").json()["google_email"] is None
|
|
assert (
|
|
c.get(f"/api/admin/users/{other.id}/access").json()["google_email"]
|
|
== "google-primary@example.com"
|
|
)
|
|
|
|
|
|
def test_the_link_is_audited_without_recording_the_link(db, as_user, make_user):
|
|
"""The audit log is read by humans and kept long; a live credential-setting link is the last
|
|
thing that belongs in it. That one was ISSUED, by whom, for whom, is the auditable fact."""
|
|
from app.models import AuditLog
|
|
|
|
admin = make_user(role="admin")
|
|
target = make_user()
|
|
db.commit()
|
|
|
|
url = as_user(admin).post(f"/api/admin/users/{target.id}/set-password-link").json()["url"]
|
|
secret = url.split("#reset=", 1)[1]
|
|
|
|
rows = db.query(AuditLog).filter(AuditLog.action == "user_setpw_link").all()
|
|
assert len(rows) == 1
|
|
assert rows[0].summary == target.email
|
|
assert secret not in str(rows[0].summary) + str(rows[0].before) + str(rows[0].after)
|
|
|
|
|
|
def test_a_google_only_account_can_be_given_a_password(db, as_user, make_user, client):
|
|
"""Deliberately not gated on the account's state: the admin's real case is whichever one it is,
|
|
and narrowing this would only make it refuse exactly then."""
|
|
admin = make_user(role="admin")
|
|
google_only = make_user(password=None)
|
|
db.commit()
|
|
assert google_only.password_hash is None
|
|
|
|
token = _link(as_user(admin), google_only.id)
|
|
r = client.post(
|
|
"/auth/password-reset/confirm", json={"token": token, "password": "now it has one too"}
|
|
)
|
|
assert r.status_code == 200
|
|
db.refresh(google_only)
|
|
assert google_only.password_hash is not None
|
|
|
|
|
|
def test_an_unknown_user_is_a_404(db, as_user, make_user):
|
|
admin = make_user(role="admin")
|
|
db.commit()
|
|
assert as_user(admin).post("/api/admin/users/999999/set-password-link").status_code == 404
|
|
assert as_user(admin).get("/api/admin/users/999999/access").status_code == 404
|
|
|
|
|
|
def test_the_issued_token_is_stored_hashed(db, as_user, make_user):
|
|
"""Same rule as every other token here: the DB holds a hash, so a database read cannot hand
|
|
someone a working link."""
|
|
admin = make_user(role="admin")
|
|
target = make_user()
|
|
db.commit()
|
|
|
|
raw = _link(as_user(admin), target.id)
|
|
rows = db.query(AuthToken).filter(AuthToken.kind == "setpw").all()
|
|
assert len(rows) == 1
|
|
assert rows[0].token_hash != raw
|
|
assert raw not in rows[0].token_hash
|
|
|
|
|
|
def test_the_link_outlives_the_one_hour_reset_window(db, as_user, make_user):
|
|
"""It travels out of band — the admin has to actually reach the person — so the mailed reset's
|
|
one hour is far too short."""
|
|
from datetime import datetime, timedelta, timezone
|
|
|
|
admin = make_user(role="admin")
|
|
target = make_user()
|
|
db.commit()
|
|
|
|
_link(as_user(admin), target.id)
|
|
row = db.query(AuthToken).filter(AuthToken.kind == "setpw").one()
|
|
assert row.expires_at - datetime.now(timezone.utc) > timedelta(hours=2)
|
|
|
|
|
|
def test_users_listing_still_serializes(db, as_user, make_user):
|
|
"""The list gained two fields; this pins that it did not gain the two per-row QUERIES the panel
|
|
endpoint exists to keep out of it."""
|
|
admin = make_user(role="admin")
|
|
db.commit()
|
|
rows = as_user(admin).get("/api/admin/users").json()
|
|
assert rows and "password_revoked_at" in rows[0] and "google_email" in rows[0]
|
|
assert "can_activate" not in rows[0]
|