diff --git a/backend/app/audit.py b/backend/app/audit.py index 52690925..95d24515 100644 --- a/backend/app/audit.py +++ b/backend/app/audit.py @@ -27,6 +27,10 @@ class AuditAction: USER_SUSPEND = "user_suspend" USER_UNSUSPEND = "user_unsuspend" USER_DELETE = "user_delete" + # E9.5 S5: an admin let a stuck registrant in, or re-sent their confirmation link. Both are + # 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" INVITE_APPROVE = "invite_approve" INVITE_DENY = "invite_deny" INVITE_ADD = "invite_add" diff --git a/backend/app/auth.py b/backend/app/auth.py index 29f664af..17a4e9e1 100644 --- a/backend/app/auth.py +++ b/backend/app/auth.py @@ -92,6 +92,45 @@ def is_allowed(db: Session, email: str) -> bool: return email in settings.allowed_email_set or email in settings.admin_email_set +def can_activate(db: Session, user: User) -> bool: + """Is this account merely waiting for an admin to finish letting it in? + + True when it is inactive but its address is ALREADY allowed — the shape roadmap UX-25 describes: + the admin approved the address, the person signed up afterwards, and nothing connects the two + (the Access-requests queue only offers Approve on a still-pending invite). + + Gated on `is_allowed`, the same function the sign-in paths consult, because activating an address + that is NOT allowed manufactures an inconsistent account: password sign-in only checks + `is_active` and would let them in, while their Google sign-in would be turned away and silently + reopened as a fresh access request. Letting in someone with no approved invite is the whitelist + box's job — that action says so out loud. + + Lives in core so the admin surface and the /api/me badge count answer from ONE definition; a + button that appears and a number that counts must never disagree about who is stuck. + """ + if user.is_demo or user.is_suspended or user.is_active: + return False + return is_allowed(db, user.email) + + +def count_pending_activations(db: Session) -> int: + """How many sign-ups are waiting on an admin click (see can_activate). Drives the admin badge. + + Deliberately NOT "everyone who cannot sign in": an account that has simply not confirmed its + address yet is waiting on its owner, not on the admin, and badging that would nag about something + the admin did not cause. It is still listed on the admin screen — with a re-send — because the + mail may never have arrived. + """ + rows = db.execute( + select(User).where( + User.is_demo.is_(False), + User.is_suspended.is_(False), + User.is_active.is_(False), + ) + ).scalars() + return sum(1 for u in rows if is_allowed(db, u.email)) + + def find_user_by_email(db: Session, email: str) -> User | None: """The account someone means when they type an address at sign-in, password-reset or verify-resend. None when there isn't exactly one answer. diff --git a/backend/app/routes/admin.py b/backend/app/routes/admin.py index 220327ca..bf867d76 100644 --- a/backend/app/routes/admin.py +++ b/backend/app/routes/admin.py @@ -6,7 +6,7 @@ from datetime import datetime, timezone from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException from pydantic import BaseModel -from sqlalchemy import select +from sqlalchemy import or_, select from sqlalchemy.orm import Session from app import audit @@ -14,8 +14,10 @@ from app import email as email_mod from app.audit import AuditAction from app.auth import ( admin_user, + can_activate, count_admins, get_or_create_demo_user, + is_allowed, operator_contact, purge_user, ) @@ -27,6 +29,8 @@ from app.models import ( Invite, User, ) +from app.routes._common import get_or_404 +from app.routes.auth import send_verification_link from app.utils import canonical_email, iso, valid_email router = APIRouter(prefix="/api/admin", tags=["admin"]) @@ -414,6 +418,133 @@ def admin_delete_user( # --- Demo account: email whitelist + state reset ------------------------------------- +# --- Stuck sign-ups: the admin lifeline (E9.5 S5, roadmap UX-25) --------------------------------- + + +def _pending_access_rows(db: Session) -> list[User]: + """Accounts that have signed up but still cannot sign in, newest first. + + Two different dead ends share this list, because from the admin's chair they look the same — a + person who registered and is still outside: + · `is_active=False` — approved BEFORE they signed up, so nothing ever landed in the + Access-requests queue and its Approve button doesn't apply (`_decide` only takes a pending + row). This is roadmap UX-25, and it is why the list exists. + · `email_verified=False` — the confirmation mail hasn't been acted on. Not the admin's to + decide, but theirs to help with: on 2026-09-03 the first mail to a fresh Gmail landed in + Spam, so "send it again" is a real move. + A suspended account is deliberately excluded: it cannot sign in either, but that is a decision + already made, not a person stuck waiting on one. + """ + return list( + db.execute( + select(User) + .where( + User.is_demo.is_(False), + User.is_suspended.is_(False), + or_(User.is_active.is_(False), User.email_verified.is_(False)), + ) + .order_by(User.created_at.desc()) + ) + .scalars() + .all() + ) + + +@router.get("/pending-access") +def list_pending_access(_: User = Depends(admin_user), db: Session = Depends(get_db)) -> list[dict]: + """The sign-ups that are still outside, with which gate is shut for each. `can_activate` cannot + be derived on the client — it needs the invite/allow-list lookup — so it is answered here. + + One `is_allowed` query per row rather than a batched re-implementation of it: the row count is + the admin's own backlog of stuck sign-ups (single digits on any real instance), and a second, + subtly different copy of the "may this address sign in" rule is exactly the drift this codebase + keeps getting bitten by.""" + rows = [(u, can_activate(db, u)) for u in _pending_access_rows(db)] + # An account that is inactive, confirmed and merely waiting on a still-pending invite is ALREADY + # an actionable row in the Access-requests queue above, with Approve on it. Listing it here too + # would make the admin choose between two buttons for one person. + return [ + _serialize_user(u) | {"can_activate": ca} for u, ca in rows if ca or not u.email_verified + ] + + +@router.post("/users/{user_id}/activate") +def activate_user( + user_id: int, + admin: User = Depends(admin_user), + db: Session = Depends(get_db), +) -> dict: + """Let in a registrant whose address was approved before they signed up (roadmap UX-25). + + Admin-side ONLY, and that is the whole design. Two earlier attempts at this were reverted after + review, and both moved authority rather than visibility: auto-activating on registration turns a + standing whitelist entry into a bearer grant for whoever registers the address first, and + reopening the approved invite to `pending` lets an anonymous sign-up REVOKE a standing approval. + This endpoint changes nothing about what an unauthenticated caller can cause — the admin could + already do exactly this by re-entering the address in the whitelist box (`add_invite` → + `_activate_user_by_email`). What was missing was any way to know they had to.""" + target = get_or_404(db, User, user_id, detail="No such user") + if target.is_demo: + raise HTTPException(status_code=400, detail="The demo account can't be activated.") + if target.is_active: + return _serialize_user(target) # idempotent: a double click is not an error + if not is_allowed(db, target.email): + raise HTTPException( + status_code=409, + detail={"code": "activate", "reason": "not_allowed", "email": target.email}, + ) + audit.record( + db, + admin.id, + AuditAction.USER_ACTIVATE, + target_type="user", + target_id=target.id, + summary=target.email, + before={"is_active": False}, + after={"is_active": True}, + ) + target.is_active = True + db.commit() + return _serialize_user(target) + + +@router.post("/users/{user_id}/resend-verification") +def resend_user_verification( + user_id: int, + admin: User = Depends(admin_user), + db: Session = Depends(get_db), +) -> dict: + """Re-send an account's confirmation link from the admin side. + + The public /auth/verify/resend answers uniformly and reveals nothing, which is right for an + anonymous caller but leaves the admin unable to tell a delivered mail from a dropped one. Here + the caller is already authenticated as an admin, so this one says plainly what happened. + + Not rate-limited: an admin is not a threat model to enumerate against, and the deliberate silence + of the public endpoint is exactly what this exists to escape.""" + target = get_or_404(db, User, user_id, detail="No such user") + if target.is_demo or target.email_verified: + raise HTTPException( + status_code=409, + detail={"code": "resend", "reason": "not_applicable", "email": target.email}, + ) + if not email_mod.email_enabled(db): + # Mail is fail-soft everywhere else, which is what left the 0.78.5 sign-ups with nothing to + # fall back on. An admin pressing a button deserves the failure, not a success toast. + raise HTTPException(status_code=409, detail={"code": "resend", "reason": "email_off"}) + send_verification_link(db, target) + audit.record( + db, + admin.id, + AuditAction.USER_VERIFY_RESEND, + target_type="user", + target_id=target.id, + summary=target.email, + ) + db.commit() + return {"ok": True, "email": target.email} + + def _serialize_demo(row: DemoWhitelist) -> dict: return { "id": row.id, diff --git a/backend/app/routes/auth.py b/backend/app/routes/auth.py index a78dfb15..1f8a743d 100644 --- a/backend/app/routes/auth.py +++ b/backend/app/routes/auth.py @@ -764,9 +764,7 @@ def _register_account(email: str, password: str) -> None: # Commit unconditionally; it is a no-op when upsert already committed the same transaction. db.commit() if email_ok: - raw = _issue_token(db, user, "verify", VERIFY_TTL) - # Fragment (#), not a query token: keeps the token out of proxy/access logs + Referer (SB3). - email_mod.send_verify_email(email, f"{_app_base()}/#verify={raw}") + send_verification_link(db, user) admins = admin_notify_emails(db) # Fired unconditionally, unlike the other two callers which gate on a fresh invite row: for # an address the admin already approved, nothing lands in the Access-requests queue, so this @@ -777,6 +775,21 @@ def _register_account(email: str, password: str) -> None: email_mod.send_admin_new_request(admins, email) +def send_verification_link(db: Session, user: User) -> None: + """Issue a fresh verification token for `user` and mail them the link. + + Always to the address ON THE ACCOUNT, never a spelling typed into some form: the account's + address is the one its owner chose to be reachable at, and a `+tag` there is usually deliberate. + The token rides the URL FRAGMENT (#) so it can never reach a proxy/access log or a Referer (SB3). + + One definition, three callers — registration, the self-service resend, and the admin-side resend + (E9.5 S5). The link's shape and TTL must not drift between them; a verification mail that differs + by who triggered it is a bug waiting to be discovered by exactly one user. + """ + raw = _issue_token(db, user, "verify", VERIFY_TTL) + email_mod.send_verify_email(user.email, f"{_app_base()}/#verify={raw}") + + @router.post("/verify") def verify_email_confirm(body: TokenIn, db: Session = Depends(get_db)) -> dict: """Confirm an email-verification token the SPA read from the URL fragment (SB3). Uniform 400 @@ -832,11 +845,7 @@ def _resend_verify_if_eligible(email: str) -> None: with SessionLocal() as db: user = find_user_by_email(db, email) if user is not None and not user.email_verified and not user.is_demo: - raw = _issue_token(db, user, "verify", VERIFY_TTL) - # To the address ON THE ACCOUNT, not the one just typed: the account's address is the one - # its owner chose to be reachable at, and a `+tag` there is usually deliberate (it tells - # them which site leaked, if one ever does). Same mailbox either way for a Gmail alias. - email_mod.send_verify_email(user.email, f"{_app_base()}/#verify={raw}") + send_verification_link(db, user) def _login_refused(status: int, reason: str, user: User | None = None, **extra) -> HTTPException: diff --git a/backend/app/routes/me.py b/backend/app/routes/me.py index ea7fd425..6de8d72d 100644 --- a/backend/app/routes/me.py +++ b/backend/app/routes/me.py @@ -5,6 +5,7 @@ from sqlalchemy.orm import Session from app.auth import ( count_admins, + count_pending_activations, current_user, google_enabled, has_read_scope, @@ -99,6 +100,7 @@ def get_me( if user is None: return {"authenticated": False} pending_invites = 0 + pending_activations = 0 # Admin-only, because it drives an operator warning: email is fail-soft, so a half-filled SMTP # config costs verification links, password resets and access-request notices with nothing # visible anywhere. Surfacing it here puts the warning on the screens where onboarding happens. @@ -108,6 +110,10 @@ def get_me( db.scalar(select(func.count()).select_from(Invite).where(Invite.status == "pending")) or 0 ) + # Counted separately from pending_invites rather than folded into it: the two are different + # facts (a request awaiting a decision vs a decided address whose sign-up never got let in), + # and quietly widening one number would make an existing field mean something new. + pending_activations = count_pending_activations(db) email_configured = email_mod.email_enabled(db) # this request's session; no second connect return { "authenticated": True, @@ -133,6 +139,9 @@ def get_me( "can_read": has_read_scope(user), "can_write": has_write_scope(user), "pending_invites": pending_invites, + # Sign-ups an admin still has to let in (roadmap UX-25): approved BEFORE they registered, so + # nothing ever reached the Access-requests queue and the badge used to read zero. + "pending_activations": pending_activations, "email_configured": email_configured, "preferences": user.preferences or {}, } diff --git a/backend/tests/test_admin_pending_access.py b/backend/tests/test_admin_pending_access.py new file mode 100644 index 00000000..615b33be --- /dev/null +++ b/backend/tests/test_admin_pending_access.py @@ -0,0 +1,186 @@ +"""The admin lifeline for a sign-up that is stuck outside (E9.5 S5, roadmap UX-25), DB-lane. + +Approve an address BEFORE the person registers and their sign-up produces no queue row at all — +`_decide` only takes a still-pending invite — so the Access-requests screen stayed empty while they +sat locked out, and the badge read zero. The only working move was re-typing the address into the +whitelist box, which nobody would guess. + +Two earlier attempts at this were reverted after review, and both moved AUTHORITY rather than +visibility: auto-activating on registration, and reopening the approved invite to pending. So the +tests here pin the boundary as much as the feature — an admin click, gated on an address that is +already allowed, and nothing an unauthenticated caller can reach. +""" + +from sqlalchemy import select + +from app.models import AuditLog, AuthToken, Invite, User + +_PW = "correct horse battery" + + +def _pending_access(client): + r = client.get("/api/admin/pending-access") + assert r.status_code == 200 + return {u["email"]: u for u in r.json()} + + +def _stuck_registrant(db, make_user, admin): + """The UX-25 shape: the admin approved the address first, the account arrived afterwards.""" + db.add(Invite(email="later@example.com", status="approved", decided_by=admin.email)) + db.commit() + return make_user(email="later@example.com", password=_PW, is_active=False) + + +def test_a_pre_approved_registrant_is_listed_and_offers_activation(db, make_user, as_user): + admin = make_user(email="admin@example.com", role="admin") + _stuck_registrant(db, make_user, admin) + + row = _pending_access(as_user(admin))["later@example.com"] + + assert row["is_active"] is False + assert ( + row["can_activate"] is True + ) # the address is already allowed — the click just finishes it + + +def test_an_ordinary_registrant_awaiting_a_decision_is_not_listed_twice(db, make_user, as_user): + # They already have a pending invite with Approve on it in the queue above. Two buttons for one + # person is worse than one. + admin = make_user(email="admin@example.com", role="admin") + db.add(Invite(email="normal@example.com", status="pending")) + db.commit() + make_user(email="normal@example.com", password=_PW, is_active=False) + + assert "normal@example.com" not in _pending_access(as_user(admin)) + + +def test_activating_lets_them_in_and_is_recorded(db, make_user, as_user): + admin = make_user(email="admin@example.com", role="admin") + stuck = _stuck_registrant(db, make_user, admin) + + r = as_user(admin).post(f"/api/admin/users/{stuck.id}/activate") + + assert r.status_code == 200 and r.json()["is_active"] is True + db.expire_all() + assert db.get(User, stuck.id).is_active is True + actions = db.execute(select(AuditLog.action)).scalars().all() + assert ( + "user_activate" in actions + ) # the admin acted on someone's access; it is not a silent read + + +def test_activating_an_address_that_is_not_allowed_is_refused(db, make_user, as_user): + # The guard that keeps this button from manufacturing an inconsistent account: password sign-in + # only checks is_active and would let them in, while their Google sign-in would be turned away + # and silently reopened as a fresh access request. Whitelisting is the move that says "let them + # in"; this one only finishes a decision already made. + admin = make_user(email="admin@example.com", role="admin") + nobody = make_user(email="nobody@example.com", password=_PW, is_active=False) + + r = as_user(admin).post(f"/api/admin/users/{nobody.id}/activate") + + assert r.status_code == 409 + assert r.json()["detail"]["reason"] == "not_allowed" + db.expire_all() + assert db.get(User, nobody.id).is_active is False + + +def test_activating_twice_is_not_an_error(db, make_user, as_user): + admin = make_user(email="admin@example.com", role="admin") + stuck = _stuck_registrant(db, make_user, admin) + client = as_user(admin) + + assert client.post(f"/api/admin/users/{stuck.id}/activate").status_code == 200 + assert client.post(f"/api/admin/users/{stuck.id}/activate").status_code == 200 + + +def test_a_suspended_account_is_not_offered_as_stuck(db, make_user, as_user): + # It cannot sign in either, but that is a decision already made — not a person waiting on one. + admin = make_user(email="admin@example.com", role="admin") + db.add(Invite(email="banned@example.com", status="approved", decided_by=admin.email)) + db.commit() + make_user(email="banned@example.com", password=_PW, is_active=False, is_suspended=True) + + assert "banned@example.com" not in _pending_access(as_user(admin)) + + +# --- the confirmation resend --------------------------------------------------------------------- + + +def test_an_unconfirmed_account_is_listed_and_can_be_sent_a_fresh_link( + db, make_user, as_user, monkeypatch +): + from app.routes import admin as admin_routes + from app.routes import auth as auth_routes + + monkeypatch.setattr(admin_routes.email_mod, "email_enabled", lambda db=None: True) + sent: list[tuple[str, str]] = [] + monkeypatch.setattr( + auth_routes.email_mod, "send_verify_email", lambda to, link: sent.append((to, link)) + ) + admin = make_user(email="admin@example.com", role="admin") + # Active but unconfirmed — a real state since E9.5 S1 made the two gates order-independent. It is + # not the admin's to decide, but the mail may simply never have arrived (2026-09-03: the first + # message to a fresh Gmail landed in Spam), so it is theirs to help with. + u = make_user(email="unconfirmed@example.com", password=_PW, email_verified=False) + + assert "unconfirmed@example.com" in _pending_access(as_user(admin)) + r = as_user(admin).post(f"/api/admin/users/{u.id}/resend-verification") + + assert r.status_code == 200 and r.json()["email"] == "unconfirmed@example.com" + assert [to for to, _ in sent] == ["unconfirmed@example.com"] + tokens = db.execute(select(AuthToken).where(AuthToken.user_id == u.id)).scalars().all() + assert [t.kind for t in tokens] == ["verify"] + + +def test_the_resend_says_so_when_the_instance_cannot_send_mail(db, make_user, as_user, monkeypatch): + from app.routes import admin as admin_routes + + monkeypatch.setattr(admin_routes.email_mod, "email_enabled", lambda db=None: False) + admin = make_user(email="admin@example.com", role="admin") + u = make_user(email="unconfirmed@example.com", password=_PW, email_verified=False) + + r = as_user(admin).post(f"/api/admin/users/{u.id}/resend-verification") + + # Mail is fail-soft everywhere else, which is exactly what left the 0.78.5 sign-ups with nothing + # to fall back on. An admin pressing a button gets the failure, not a success toast. + assert r.status_code == 409 and r.json()["detail"]["reason"] == "email_off" + + +def test_resending_to_an_already_confirmed_account_is_refused(db, make_user, as_user): + admin = make_user(email="admin@example.com", role="admin") + u = make_user(email="fine@example.com", password=_PW) + + r = as_user(admin).post(f"/api/admin/users/{u.id}/resend-verification") + + assert r.status_code == 409 and r.json()["detail"]["reason"] == "not_applicable" + + +# --- the badge ----------------------------------------------------------------------------------- + + +def test_the_badge_counts_stuck_sign_ups_but_not_ones_the_queue_already_shows( + db, make_user, client +): + # Signed in for real, not via the as_user override: /api/me resolves its user through + # optional_current_user, which that override does not cover (see test_me_email_flag). + admin = make_user(email="admin@example.com", password=_PW, role="admin") + _stuck_registrant(db, make_user, admin) # approved first → counts + db.add(Invite(email="normal@example.com", status="pending")) # already in the queue → doesn't + db.commit() + make_user(email="normal@example.com", password=_PW, is_active=False) + make_user(email="unconfirmed@example.com", password=_PW, email_verified=False) # not ours + + assert ( + client.post( + "/auth/password-login", json={"email": admin.email, "password": _PW} + ).status_code + == 200 + ) + me = client.get("/api/me").json() + assert me["authenticated"] is True + + # An unconfirmed account waits on its OWNER, not on the admin — listing it is help, badging it + # would be nagging about something the admin did not cause. + assert me["pending_activations"] == 1 + assert me["pending_invites"] == 1 # unchanged meaning: still just the undecided requests diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 5d4975d2..4534ef55 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -247,7 +247,10 @@ export default function App() { // Keep a single, current nudge: drop any prior one (persisted copies reload as dismissed // history, so they'd otherwise pile up one-per-reload) before re-issuing the live notice. removeByMetaKind("access-requests"); - const pending = meQuery.data.pending_invites ?? 0; + // Both kinds of "someone is waiting on you": a request needing a decision, and a sign-up whose + // address was approved before they registered. The second used to produce NO signal at all — + // nothing lands in the queue for it — which is how the 2026-09-03 registrant went unnoticed. + const pending = (meQuery.data.pending_invites ?? 0) + (meQuery.data.pending_activations ?? 0); if (pending > 0) { notify({ level: "warning", @@ -266,7 +269,7 @@ export default function App() { }); } // eslint-disable-next-line react-hooks/exhaustive-deps - }, [meQuery.data?.role, meQuery.data?.pending_invites]); + }, [meQuery.data?.role, meQuery.data?.pending_invites, meQuery.data?.pending_activations]); // Language is edited outside the Settings page (sidebar/login), so it stays instant. function changeLanguage(code: LangCode) { diff --git a/frontend/src/components/AdminUsers.tsx b/frontend/src/components/AdminUsers.tsx index 1cbb78c7..0b42f9ff 100644 --- a/frontend/src/components/AdminUsers.tsx +++ b/frontend/src/components/AdminUsers.tsx @@ -2,8 +2,26 @@ import { useState } from "react"; import { qk } from "../lib/queryKeys"; import { useTranslation } from "react-i18next"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; -import { Ban, Check, RotateCcw, Shield, Trash2, UserPlus, X } from "lucide-react"; -import { api, HttpError, type AdminUserRow, type Invite, type Me } from "../lib/api"; +import { + Ban, + Check, + Clock, + MailWarning, + RotateCcw, + Send, + Shield, + Trash2, + UserPlus, + X, +} from "lucide-react"; +import { + api, + HttpError, + type AdminUserRow, + type Invite, + type Me, + type PendingAccessRow, +} from "../lib/api"; import { notify } from "../lib/notifications"; import { ADMIN_USERS_TAB_KEY } from "../lib/adminUsersTab"; import Tooltip from "./Tooltip"; @@ -27,7 +45,14 @@ export default function AdminUsers() { const [tab, setTab] = usePersistedTab(ADMIN_USERS_TAB_KEY, "roles"); const tabs = [ { id: "roles", label: t("users.tabs.roles") }, - { id: "access", label: t("users.tabs.access"), badge: me.pending_invites }, + // Both kinds of "waiting on the admin": a request needing a decision, and a sign-up whose + // address was approved BEFORE they registered — the second never reaches the invite queue, + // so the badge used to read zero while somebody sat locked out (roadmap UX-25). + { + id: "access", + label: t("users.tabs.access"), + badge: me.pending_invites + me.pending_activations, + }, { id: "demo", label: t("users.tabs.demo") }, ]; const active = tabs.some((x) => x.id === tab) ? tab : "roles"; @@ -41,7 +66,12 @@ export default function AdminUsers() {
{active === "roles" && } - {active === "access" && } + {active === "access" && ( + <> + + + + )} {active === "demo" && }
@@ -261,6 +291,166 @@ function UsersRoles({ me }: { me: Me }) { ); } +// --- Sign-ups that are still outside (roadmap UX-25) -------------------------------------------- +// +// The gap this closes: approve an address BEFORE the person registers and their sign-up produces no +// queue row at all — `_decide` only takes a pending invite — so the Access-requests screen above was +// empty while they sat locked out. The only working move was re-typing the address into the +// whitelist box, which nobody would guess. This section makes the state visible and the fix one +// click, WITHOUT changing who may cause what: the button is admin-authenticated and explicit. Two +// earlier attempts moved authority instead (auto-activating on registration, reopening the approved +// invite) and were reverted after review. +function PendingAccess() { + const { t } = useTranslation(); + const qc = useQueryClient(); + const rows = useQuery({ + queryKey: qk.adminPendingAccess(), + queryFn: () => api.adminPendingAccess(), + }); + // Activating changes /api/me's badge count too, so refresh both or the number lies until reload. + const refresh = () => { + qc.invalidateQueries({ queryKey: qk.adminPendingAccess() }); + qc.invalidateQueries({ queryKey: qk.me() }); + qc.invalidateQueries({ queryKey: qk.adminUsers() }); + }; + + const activate = useMutation({ + mutationFn: (u: PendingAccessRow) => api.activateUser(u.id), + onSuccess: (_r, u) => { + refresh(); + notify({ level: "success", message: t("users.pendingAccess.activated", { email: u.email }) }); + }, + onError: () => notify({ level: "error", message: t("users.pendingAccess.activateFailed") }), + }); + + const resend = useMutation({ + mutationFn: (u: PendingAccessRow) => api.resendUserVerification(u.id), + onSuccess: (_r, u) => { + refresh(); + notify({ level: "success", message: t("users.pendingAccess.resent", { email: u.email }) }); + }, + // The public resend is deliberately silent; this one is not, so say WHY it failed — an instance + // with no working SMTP would otherwise show a success toast for a mail that was never sent. + onError: (err) => + notify({ + level: "error", + message: + err instanceof HttpError && isEmailOff(err) + ? t("users.pendingAccess.resendEmailOff") + : t("users.pendingAccess.resendFailed"), + }), + }); + + const list = rows.data ?? []; + const busy = activate.isPending || resend.isPending; + if (rows.isSuccess && list.length === 0) return null; // nothing stuck: no empty box to explain + + return ( +
+

{t("users.pendingAccess.body")}

+ {rows.isError && !rows.data ? ( + rows.refetch()}> + {t("common.loadError")} + + ) : rows.isLoading && !rows.data ? ( + + ) : ( +
+ {list.map((u) => ( + activate.mutate(u)} + onResend={() => resend.mutate(u)} + /> + ))} +
+ )} +
+ ); +} + +/** Did the resend fail because this instance has no working SMTP? */ +function isEmailOff(err: HttpError): boolean { + const d = err.detailData as { code?: string; reason?: string } | null | undefined; + return d?.code === "resend" && d.reason === "email_off"; +} + +// One stuck sign-up. The row names WHICH gate is shut rather than just offering buttons — the same +// reasoning as the sign-in card's blocked panel, from the admin's side: an admin about to let someone +// in should see that the address has not been confirmed yet, because on a no-SMTP instance that is +// the difference between the owner and whoever registered the address first (roadmap SEC-15). +function PendingAccessRowView({ + row, + busy, + onActivate, + onResend, +}: { + row: PendingAccessRow; + busy: boolean; + onActivate: () => void; + onResend: () => void; +}) { + const { t } = useTranslation(); + return ( +
+
+
{row.email}
+
+ {row.created_at && ( + + {t("users.pendingAccess.signedUp", { + time: new Date(row.created_at).toLocaleString(), + })} + + )} + {row.email_verified ? ( + + + {t("users.pendingAccess.emailConfirmed")} + + ) : ( + + + {t("users.pendingAccess.emailUnconfirmed")} + + )} + {!row.is_active && ( + + + {t("users.pendingAccess.inactive")} + + )} +
+
+ {!row.email_verified && ( + + + + )} + {row.can_activate && ( + + + + )} +
+ ); +} + // --- Access requests (the Invite whitelist) — moved verbatim from Settings → Account ------- function AdminInvites() { diff --git a/frontend/src/components/adminInvites.dom.test.tsx b/frontend/src/components/adminInvites.dom.test.tsx index 1e3cda1a..ebc7c9e8 100644 --- a/frontend/src/components/adminInvites.dom.test.tsx +++ b/frontend/src/components/adminInvites.dom.test.tsx @@ -37,7 +37,13 @@ vi.mock("../lib/notifications", async (importOriginal) => { return { ...actual, notify: (...a: unknown[]) => notifyMock(...a) }; }); -const meMock = { pending_invites: 0, email_configured: true, id: 1, role: "admin" }; +const meMock = { + pending_invites: 0, + pending_activations: 0, + email_configured: true, + id: 1, + role: "admin", +}; vi.mock("../lib/useMe", () => ({ useMe: () => meMock })); // The tab strip is rendered into an app-level toolbar slot that does not exist here, so pin the @@ -57,6 +63,9 @@ vi.mock("../lib/api", async (importOriginal) => { ...actual.api, adminInvites: () => listMock(), deleteInvite: (id: number) => deleteMock(id), + // The access tab also renders the stuck-sign-ups section (E9.5 S5); nothing stuck here, so it + // renders nothing and this file stays about the delete affordance. + adminPendingAccess: () => Promise.resolve([]), }, }; }); diff --git a/frontend/src/components/adminPendingAccess.dom.test.tsx b/frontend/src/components/adminPendingAccess.dom.test.tsx new file mode 100644 index 00000000..bfedaa10 --- /dev/null +++ b/frontend/src/components/adminPendingAccess.dom.test.tsx @@ -0,0 +1,186 @@ +// @vitest-environment jsdom +// +// The admin lifeline for a sign-up that is stuck outside (E9.5 S5, roadmap UX-25). Approve an address +// BEFORE the person registers and their sign-up produces no queue row at all, so the Access-requests +// screen was empty while they sat locked out and the only working move — re-typing the address into +// the whitelist box — was undiscoverable. These pin what the section shows and, just as importantly, +// what it does NOT show: the section is a discovery surface, and a person the queue above already +// covers must not appear twice. +// +// i18n is a key-passthrough so assertions read stable keys; the API and the notify singleton are +// stubbed so the panel can be driven without a server. +import { describe, it, expect, afterEach, beforeEach, vi } from "vitest"; +import { render, screen, cleanup, waitFor, fireEvent } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; + +vi.mock("react-i18next", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + useTranslation: () => ({ + t: (key: string, vars?: Record) => + vars?.email ? `${key}:${String(vars.email)}` : key, + i18n: { language: "en" }, + }), + }; +}); + +vi.mock("./ConfirmProvider", () => ({ + useConfirm: () => vi.fn(async () => true), + ConfirmProvider: ({ children }: { children: React.ReactNode }) => children, +})); + +const notifyMock = vi.fn(); +vi.mock("../lib/notifications", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, notify: (...a: unknown[]) => notifyMock(...a) }; +}); + +const meMock = { + pending_invites: 0, + pending_activations: 0, + email_configured: true, + id: 1, + role: "admin", +}; +vi.mock("../lib/useMe", () => ({ useMe: () => meMock })); + +// The tab strip renders into an app-level toolbar slot that does not exist here; pin the tab. +vi.mock("./Tabs", () => ({ + default: () => null, + usePersistedTab: () => ["access", () => {}], +})); + +const pendingMock = vi.fn(); +const activateMock = vi.fn(); +const resendMock = vi.fn(); +vi.mock("../lib/api", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + api: { + ...actual.api, + adminInvites: () => Promise.resolve([]), + adminPendingAccess: () => pendingMock(), + activateUser: (id: number) => activateMock(id), + resendUserVerification: (id: number) => resendMock(id), + }, + }; +}); + +import AdminUsers from "./AdminUsers"; + +/** A row as /api/admin/pending-access serves it. */ +const row = (over: Record = {}) => ({ + id: 7, + email: "later@example.com", + display_name: null, + role: "user", + is_active: false, + is_suspended: false, + email_verified: true, + is_demo: false, + has_password: true, + has_google: false, + created_at: "2026-09-01T10:00:00Z", + can_activate: true, + ...over, +}); + +function mount() { + const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + render( + + + , + ); +} + +beforeEach(() => { + pendingMock.mockReset(); + activateMock.mockReset().mockResolvedValue({}); + resendMock.mockReset().mockResolvedValue({ ok: true, email: "later@example.com" }); + notifyMock.mockReset(); +}); +afterEach(cleanup); + +describe("the stuck sign-ups section", () => { + it("offers Activate for a registrant whose address was already approved", async () => { + pendingMock.mockResolvedValue([row()]); + mount(); + + const button = await screen.findByText("users.pendingAccess.activate"); + fireEvent.click(button); + + await waitFor(() => expect(activateMock).toHaveBeenCalledWith(7)); + expect(notifyMock).toHaveBeenCalledWith( + expect.objectContaining({ message: "users.pendingAccess.activated:later@example.com" }), + ); + }); + + it("shows no Activate when the address is not allowed, since that would be a different decision", async () => { + // Activating an address with no approved invite makes an account that signs in with a password + // but is turned away by Google — the whitelist box is the honest move for that. + pendingMock.mockResolvedValue([row({ can_activate: false, email_verified: false })]); + mount(); + + await screen.findByText("users.pendingAccess.emailUnconfirmed"); + expect(screen.queryByText("users.pendingAccess.activate")).toBeNull(); + }); + + it("names which gate is shut, so an admin sees an unconfirmed address before letting it in", async () => { + pendingMock.mockResolvedValue([row({ email_verified: false })]); + mount(); + + // On a no-SMTP instance this is the difference between the owner and whoever registered the + // address first (roadmap SEC-15), so it belongs on the row, not in a tooltip. + expect(await screen.findByText("users.pendingAccess.emailUnconfirmed")).toBeTruthy(); + expect(screen.getByText("users.pendingAccess.inactive")).toBeTruthy(); + }); + + it("re-sends the confirmation link for an unconfirmed account", async () => { + pendingMock.mockResolvedValue([row({ email_verified: false })]); + mount(); + + fireEvent.click(await screen.findByLabelText("users.pendingAccess.resend")); + + await waitFor(() => expect(resendMock).toHaveBeenCalledWith(7)); + }); + + it("offers no resend once the address is confirmed", async () => { + pendingMock.mockResolvedValue([row()]); + mount(); + + await screen.findByText("users.pendingAccess.activate"); + expect(screen.queryByLabelText("users.pendingAccess.resend")).toBeNull(); + }); + + it("says the mail was never sent when the instance has no SMTP", async () => { + const { HttpError } = await import("../lib/api"); + pendingMock.mockResolvedValue([row({ email_verified: false })]); + resendMock.mockRejectedValue( + new HttpError(409, "ignored", { code: "resend", reason: "email_off" }), + ); + mount(); + + fireEvent.click(await screen.findByLabelText("users.pendingAccess.resend")); + + // Mail is fail-soft everywhere else, which is what left the 0.78.5 sign-ups with nothing to fall + // back on. A button the admin pressed must not answer with a success toast. + await waitFor(() => + expect(notifyMock).toHaveBeenCalledWith( + expect.objectContaining({ message: "users.pendingAccess.resendEmailOff" }), + ), + ); + }); + + it("renders nothing at all when nobody is stuck", async () => { + pendingMock.mockResolvedValue([]); + mount(); + + // The section withdraws itself rather than leaving an empty box to explain — so wait for it to + // GO, not merely for the request to have been made. + await waitFor(() => expect(screen.queryByText("users.pendingAccess.title")).toBeNull()); + expect(pendingMock).toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/i18n/locales/en/auditActions.json b/frontend/src/i18n/locales/en/auditActions.json index 09eaa33b..942ca170 100644 --- a/frontend/src/i18n/locales/en/auditActions.json +++ b/frontend/src/i18n/locales/en/auditActions.json @@ -3,6 +3,8 @@ "user_suspend": "User suspended", "user_unsuspend": "User reinstated", "user_delete": "User deleted", + "user_activate": "Account activated", + "user_verify_resend": "Confirmation link re-sent", "invite_approve": "Access request approved", "invite_deny": "Access request denied", "invite_add": "Email whitelisted", diff --git a/frontend/src/i18n/locales/en/users.json b/frontend/src/i18n/locales/en/users.json index a5f955d6..103f8c37 100644 --- a/frontend/src/i18n/locales/en/users.json +++ b/frontend/src/i18n/locales/en/users.json @@ -4,6 +4,23 @@ "access": "Access requests", "demo": "Demo" }, + "pendingAccess": { + "title": "Signed up, but still outside", + "body": "These people created an account that can't sign in yet. If you approved their address before they registered, no request reached the queue above — let them in here.", + "signedUp": "Signed up {{time}}", + "emailConfirmed": "email confirmed", + "emailUnconfirmed": "email not confirmed", + "inactive": "waiting for activation", + "activate": "Activate", + "activateHint": "Let this account in. Their address is already approved — this only finishes it.", + "activated": "{{email}} can sign in now.", + "activateFailed": "Couldn't activate that account.", + "resend": "Send the confirmation link again", + "resendHint": "Send the confirmation link again, to the address on the account.", + "resent": "Confirmation link sent to {{email}}.", + "resendFailed": "Couldn't send the confirmation link.", + "resendEmailOff": "No mail was sent: this instance has no working SMTP setup. Check Configuration → Email." + }, "roles": { "title": "Users & roles", "intro": "Everyone who can sign in. Promote a user to admin, or back to a regular user.", diff --git a/frontend/src/i18n/locales/hu/auditActions.json b/frontend/src/i18n/locales/hu/auditActions.json index d57ab117..f6c8eb62 100644 --- a/frontend/src/i18n/locales/hu/auditActions.json +++ b/frontend/src/i18n/locales/hu/auditActions.json @@ -3,6 +3,8 @@ "user_suspend": "Felhasználó felfüggesztve", "user_unsuspend": "Felhasználó visszaállítva", "user_delete": "Felhasználó törölve", + "user_activate": "Fiók aktiválva", + "user_verify_resend": "Megerősítő link újraküldve", "invite_approve": "Hozzáférési kérelem jóváhagyva", "invite_deny": "Hozzáférési kérelem elutasítva", "invite_add": "Email fehérlistázva", diff --git a/frontend/src/i18n/locales/hu/users.json b/frontend/src/i18n/locales/hu/users.json index d65554d6..207fbf87 100644 --- a/frontend/src/i18n/locales/hu/users.json +++ b/frontend/src/i18n/locales/hu/users.json @@ -4,6 +4,23 @@ "access": "Hozzáférési kérések", "demo": "Demo" }, + "pendingAccess": { + "title": "Regisztrált, de még kívül van", + "body": "Ezek a fiókok léteznek, de még nem tudnak belépni. Ha a címüket a regisztrációjuk előtt hagytad jóvá, a fenti sorba semmi nem került — itt engedheted be őket.", + "signedUp": "Regisztrált: {{time}}", + "emailConfirmed": "cím megerősítve", + "emailUnconfirmed": "cím nincs megerősítve", + "inactive": "aktiválásra vár", + "activate": "Aktiválás", + "activateHint": "Beengedi ezt a fiókot. A címe már jóvá van hagyva — ez csak befejezi.", + "activated": "{{email}} mostantól be tud lépni.", + "activateFailed": "Nem sikerült aktiválni a fiókot.", + "resend": "Megerősítő link újraküldése", + "resendHint": "Újraküldi a megerősítő linket, a fiókon szereplő címre.", + "resent": "Megerősítő link elküldve ide: {{email}}.", + "resendFailed": "Nem sikerült elküldeni a megerősítő linket.", + "resendEmailOff": "Nem ment ki levél: ezen a példányon nincs működő SMTP. Nézd meg a Beállítások → E-mail részt." + }, "roles": { "title": "Felhasználók és szerepek", "intro": "Mindenki, aki be tud lépni. Léptess egy felhasználót adminná, vagy vissza sima felhasználóvá.", diff --git a/frontend/src/lib/api/account.ts b/frontend/src/lib/api/account.ts index 865d62eb..89473148 100644 --- a/frontend/src/lib/api/account.ts +++ b/frontend/src/lib/api/account.ts @@ -20,6 +20,11 @@ export interface Me { can_read: boolean; can_write: boolean; pending_invites: number; + /** Admin-only: sign-ups an admin still has to let in — approved BEFORE they registered, so nothing + * ever reached the Access-requests queue and its badge read zero (roadmap UX-25). Counted apart + * from pending_invites because they are different facts, and widening one number silently would + * change what an existing field means. */ + pending_activations: number; /** Admin-only: false when SMTP is incomplete, so every outbound message is silently skipped. */ email_configured: boolean; preferences: Record; diff --git a/frontend/src/lib/api/admin.ts b/frontend/src/lib/api/admin.ts index 2c10790a..c2282737 100644 --- a/frontend/src/lib/api/admin.ts +++ b/frontend/src/lib/api/admin.ts @@ -27,6 +27,16 @@ export interface Invite { still_allowed_by_env: boolean; } +/** A sign-up that is still outside, as the admin screen shows it. Same shape as a user row plus the + * one thing the client cannot work out for itself. */ +export interface PendingAccessRow extends AdminUserRow { + /** The address is already allowed and the account is merely inactive, so Activate finishes a + * decision the admin has already made. False when there is no approved invite behind it — + * activating then would make an account that signs in with a password but is turned away by + * Google, so the whitelist box is the honest move instead. */ + can_activate: boolean; +} + // Admin Configuration page: one DB-overridable setting (registry entry on the backend). export interface ConfigItem { key: string; @@ -169,6 +179,16 @@ export const adminApi = { resetDemo: (): Promise<{ reset: boolean; playlists_seeded: number }> => req("/api/admin/demo/reset", { method: "POST" }), // --- access invites --- + // Sign-ups that registered but still cannot get in — the admin lifeline for roadmap UX-25. + adminPendingAccess: (): Promise => req("/api/admin/pending-access"), + // Let one in. Admin-only and explicit: it changes discoverability, not authority — the same thing + // was always possible by re-entering the address in the whitelist box. + activateUser: (id: number): Promise => + req(`/api/admin/users/${id}/activate`, { method: "POST" }, { quiet: true }), + // Re-send someone's confirmation link. Unlike the public resend this one says what happened, + // because the caller is already an authenticated admin. + resendUserVerification: (id: number): Promise<{ ok: boolean; email: string }> => + req(`/api/admin/users/${id}/resend-verification`, { method: "POST" }, { quiet: true }), adminInvites: (status?: string): Promise => req(`/api/admin/invites${status ? `?status=${status}` : ""}`), approveInvite: (id: number) => req(`/api/admin/invites/${id}/approve`, { method: "POST" }), diff --git a/frontend/src/lib/queryKeys.ts b/frontend/src/lib/queryKeys.ts index 58adb1d7..fa635c03 100644 --- a/frontend/src/lib/queryKeys.ts +++ b/frontend/src/lib/queryKeys.ts @@ -86,6 +86,7 @@ export const qk = { // --- admin --- adminUsers: () => ["admin-users"] as const, adminInvites: () => ["admin-invites"] as const, + adminPendingAccess: () => ["admin-pending-access"] as const, adminConfig: () => ["admin-config"] as const, adminAudit: () => ["admin-audit"] as const, adminStorage: () => ["admin-storage"] as const,