diff --git a/backend/app/audit.py b/backend/app/audit.py index dae44955..8a87f95c 100644 --- a/backend/app/audit.py +++ b/backend/app/audit.py @@ -31,6 +31,7 @@ class AuditAction: # recorded because both are the admin acting ON someone's access, not merely reading a list. USER_ACTIVATE = "user_activate" USER_VERIFY_RESEND = "user_verify_resend" + USER_SETPW_LINK = "user_setpw_link" INVITE_APPROVE = "invite_approve" INVITE_DENY = "invite_deny" INVITE_ADD = "invite_add" diff --git a/backend/app/routes/admin.py b/backend/app/routes/admin.py index cc651976..b9b3a04f 100644 --- a/backend/app/routes/admin.py +++ b/backend/app/routes/admin.py @@ -32,7 +32,12 @@ from app.models import ( User, ) from app.routes._common import get_or_404 -from app.routes.auth import APPROVAL_VERIFY_TTL, issue_verification_link, send_verification_link +from app.routes.auth import ( + APPROVAL_VERIFY_TTL, + issue_set_password_link, + issue_verification_link, + send_verification_link, +) from app.utils import canonical_email, iso, valid_email router = APIRouter(prefix="/api/admin", tags=["admin"]) @@ -393,7 +398,15 @@ def _serialize_user(u: User) -> dict: "email_verified": u.email_verified, "is_demo": u.is_demo, "has_password": u.password_hash is not None, + # Distinguishes "this account never had a password" (Google-only) from "SEC-10 cleared it" — + # the same distinction _send_reset_if_eligible needs, and the one an admin has to see before + # deciding what kind of help someone needs. + "password_revoked_at": iso(u.password_revoked_at), "has_google": u.google_sub is not None, + # Only when it DIFFERS from the account address (E9.5 S3): "connected" is not the same + # statement as "connected as someone else", and the second one is the one that explains a + # confusing sign-in. + "google_email": u.google_email if u.google_email != u.email else None, "created_at": iso(u.created_at), } @@ -608,6 +621,63 @@ def list_pending_access(_: User = Depends(admin_user), db: Session = Depends(get return out +@router.get("/users/{user_id}/access") +def user_access( + user_id: int, + _: User = Depends(admin_user), + db: Session = Depends(get_db), +) -> dict: + """Everything the account panel shows about why this person can or cannot get in (E12 S5). + + Its own endpoint rather than extra columns on /users: `is_allowed` and `can_activate` each run a + query, and the panel opens for ONE user at a time — folding them into the list would pay that cost + per row on every load to answer a question nobody asked yet.""" + target = get_or_404(db, User, user_id, detail="No such user") + return _serialize_user(target) | { + "is_allowed": is_allowed(db, target.email), + "can_activate": can_activate(db, target), + "email_configured": email_mod.email_enabled(db), + } + + +@router.post("/users/{user_id}/set-password-link") +def create_set_password_link( + user_id: int, + admin: User = Depends(admin_user), + db: Session = Depends(get_db), +) -> dict: + """Mint a one-time link that lets this account set a password, with no email involved (E12 S5). + + The answer to SEC-15, SEC-16 and "no SMTP and someone forgot their password" at once. The admin + hands the link over by whatever channel they trust; opening it sets a password and signs out + whoever held the old one. + + Deliberately NOT gated on the account's state. It is useful for a pre-empted address, for a + revoked password, for a Google-only account that wants one, and for a plain forgotten password on + an instance with no mail — narrowing it to one of those would only make the admin's real case the + one it refuses. The demo account is the single exception: it is shared, so handing anyone a way to + change its password is handing them everyone else's session.""" + target = get_or_404(db, User, user_id, detail="No such user") + if target.is_demo: + raise HTTPException( + status_code=400, detail={"code": "setpw", "reason": "demo", "email": target.email} + ) + url, expires_at = issue_set_password_link(db, target) + # The URL itself is NOT recorded: the audit log is read by humans and kept long, and a live + # credential-setting link is the last thing that belongs in it. That the link was issued, by whom, + # for whom, is the auditable fact. + audit.record( + db, + admin.id, + AuditAction.USER_SETPW_LINK, + target_type="user", + target_id=target.id, + summary=target.email, + ) + db.commit() + return {"ok": True, "url": url, "expires_at": iso(expires_at), "email": target.email} + + @router.post("/users/{user_id}/activate") def activate_user( user_id: int, diff --git a/backend/app/routes/auth.py b/backend/app/routes/auth.py index ec3a25ea..c76bd49f 100644 --- a/backend/app/routes/auth.py +++ b/backend/app/routes/auth.py @@ -52,6 +52,16 @@ VERIFY_TTL = timedelta(hours=24) # in and sets no password. APPROVAL_VERIFY_TTL = timedelta(days=7) RESET_TTL = timedelta(hours=1) +# An admin-issued set-password link (E12 S5) travels out of band — the admin has to actually reach the +# person and hand it over — so the one-hour reset window is far too short. A day matches VERIFY_TTL and +# the affordance the admin was shown. +ADMIN_SETPW_TTL = timedelta(hours=24) +# Both kinds set a password from a link, and the SIGNED-OUT card consumes them through the same +# `#reset=` entry, so the user-facing flow is one flow. They stay separate kinds for one reason: a +# `reset` token proves control of the MAILBOX (it was mailed there) and therefore confirms the address; +# an admin-issued one proves nothing about the mailbox — the admin could hand it to anyone — so it must +# not silently satisfy the email gate. See password_reset_confirm. +_PASSWORD_TOKEN_KINDS = ("reset", "setpw") _DECOY_PASSWORD_HASH = hash_password(secrets.token_urlsafe(16)) @@ -773,6 +783,35 @@ def _consume_token(db: Session, raw: str | None, kind: str) -> User | None: return db.get(User, row.user_id) +def _consume_password_token(db: Session, raw: str | None) -> tuple[User, str] | None: + """Spend a password-setting token of either kind; returns (user, kind). The kind is returned, not + swallowed, because the caller has to treat them differently — only a mailed reset attests the + address.""" + for kind in _PASSWORD_TOKEN_KINDS: + user = _consume_token(db, raw, kind) + if user is not None: + return user, kind + return None + + +def issue_set_password_link(db: Session, user: User) -> tuple[str, datetime]: + """An admin-issued, single-use link that lets `user` set a password without any email (E12 S5). + + One mechanism answers three things: SEC-15 (on a no-SMTP instance every registration is + auto-verified, so a pre-empted address cannot be recovered by mail), SEC-16 (a pre-empted address + stays takeable until its owner resets, and password-reset needs working mail), and the everyday + "no SMTP configured and someone forgot their password". + + Issuing does NOT revoke the current password: an admin misclick must not lock anyone out, and the + account may be perfectly healthy. USING the link replaces the hash and bumps the session epoch, + which is what evicts a squatter — see password_reset_confirm.""" + expires_at = datetime.now(timezone.utc) + ADMIN_SETPW_TTL + raw = _issue_token(db, user, "setpw", ADMIN_SETPW_TTL) + # The same `#reset=` fragment the mailed link uses: the signed-out card already knows how to + # consume it, and a second entry shape would be a second thing to keep correct. + return f"{_app_base()}/#reset={raw}", expires_at + + @router.post("/register") def register( body: EmailPasswordIn, @@ -1105,7 +1144,7 @@ def password_reset_check(body: TokenIn, request: Request, db: Session = Depends( """ if not _reset_check_limiter.allow(_client_ip(request)): return {"ok": True} - return {"ok": _peek_token(db, body.token, "reset") is not None} + return {"ok": any(_peek_token(db, body.token, k) is not None for k in _PASSWORD_TOKEN_KINDS)} @router.post("/password-reset/confirm") @@ -1118,16 +1157,23 @@ def password_reset_confirm(body: TokenPasswordIn, db: Session = Depends(get_db)) status_code=400, detail={"code": "reset", "reason": "weak_password", "min": PASSWORD_MIN_LEN}, ) - user = _consume_token(db, token, "reset") - if user is None: + spent = _consume_password_token(db, token) + if spent is None: raise HTTPException(status_code=400, detail={"code": "reset", "reason": "invalid"}) + user, kind = spent user.password_hash = hash_password(new_password) user.password_revoked_at = None # the account has a password again — SEC-10's trace is spent # A valid reset link proves control of the mailbox, so completing a reset also verifies the # email — this rescues an account that registered but never clicked the verification link (it # can then sign in). The separate admin-approval gate (is_active) is deliberately NOT touched # here: a reset proves email ownership, not that an admin approved the account. - user.email_verified = True + # + # An admin-issued `setpw` link proves NOTHING about the mailbox — the admin hands it over by + # whatever means they choose — so it must not satisfy the email gate. Claiming otherwise would let + # an admin confirm an address nobody ever proved, which is the same false attestation SEC-15 is + # about. The person still confirms their address the normal way. + if kind == "reset": + user.email_verified = True bump_session_epoch( user ) # SA4: a reset (often a compromise response) kills all existing sessions. @@ -1135,7 +1181,7 @@ def password_reset_confirm(body: TokenPasswordIn, db: Session = Depends(get_db)) for row in db.execute( select(AuthToken).where( AuthToken.user_id == user.id, - AuthToken.kind == "reset", + AuthToken.kind.in_(_PASSWORD_TOKEN_KINDS), AuthToken.used_at.is_(None), ) ).scalars(): diff --git a/backend/tests/test_set_password_link.py b/backend/tests/test_set_password_link.py new file mode 100644 index 00000000..2e7f196f --- /dev/null +++ b/backend/tests/test_set_password_link.py @@ -0,0 +1,280 @@ +"""E12 · S5 — 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) + client.post( + "/auth/password-reset/confirm", json={"token": token, "password": "the owner's own choice"} + ) + + 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_shared_demo_account_is_refused(db, as_user, make_user): + """The demo account is shared, so handing anyone a way to change its password hands them + everyone else's session on it.""" + admin = make_user(role="admin") + demo = make_user(is_demo=True) + db.commit() + + r = as_user(admin).post(f"/api/admin/users/{demo.id}/set-password-link") + assert r.status_code == 400 + assert r.json()["detail"]["reason"] == "demo" + + +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] diff --git a/frontend/src/components/AccountPanel.tsx b/frontend/src/components/AccountPanel.tsx new file mode 100644 index 00000000..4060681d --- /dev/null +++ b/frontend/src/components/AccountPanel.tsx @@ -0,0 +1,238 @@ +import { useState } from "react"; +import { useTranslation } from "react-i18next"; +import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; +import { Check, Clock, Copy, Globe, KeyRound, Mail, Send, ShieldAlert } from "lucide-react"; +import Modal from "./Modal"; +import { LoadingState, StateMessage } from "./QueryState"; +import { api } from "../lib/api"; +import { qk } from "../lib/queryKeys"; +import { notify } from "../lib/notifications"; +import type { AdminUserAccess, AdminUserRow } from "../lib/api/admin"; + +// The admin's answer to "why can this person not get in?", and the place every recovery action for +// one account lives (E12 · S5, design option C). +// +// It exists because nothing in the admin UI showed an account's GATE STATE: an admin could see a +// "Pending approval" chip but not whether the address was confirmed, whether the password had been +// cleared by SEC-10, or which Google account was attached. Every support conversation started by +// guessing. The recovery actions sit under that readout rather than in the row, so the admin picks +// one having read what is actually wrong. + +/** One line of the Access readout: an icon, a label, and the state in the tone that state deserves. */ +function AccessRow({ + icon: Icon, + label, + value, + tone = "muted", +}: { + icon: typeof Mail; + label: string; + value: string; + tone?: "muted" | "warn" | "ok"; +}) { + const toneCls = tone === "warn" ? "text-warn" : tone === "ok" ? "text-success" : "text-muted"; + return ( +
{t("users.account.linkNote")}
+{t("users.roles.intro")}
{q.isError && !q.data ? (