feat(admin): surface the sign-ups that are stuck outside, and let an admin finish letting them in (E9.5 S5)
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 sat empty, the tab badge read zero, and the only working move was re-typing the address into the whitelist box. Nobody would guess that, and on 2026-09-03 nobody did. This is roadmap UX-25, where two earlier attempts were reverted after review. Both moved AUTHORITY: 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. What was actually missing was visibility, so this changes nothing about what an unauthenticated caller can cause — the admin could already do exactly this by re-entering the address; they just had no way to know they had to. The section lists every account that cannot sign in yet and says WHICH gate is shut, because the two dead ends look the same from the admin's chair: waiting for activation, or waiting on a confirmation mail. Naming the unconfirmed ones is not decoration — on a no-SMTP instance that is the difference between the owner and whoever registered the address first (SEC-15), and an admin about to let someone in should see it. A person the queue above already covers is deliberately NOT listed here; two buttons for one person is worse than one. Activate is gated on `is_allowed` — the same function the sign-in paths consult, in core, so the button that appears and the badge that counts cannot disagree. Without that gate it would manufacture 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 stays the whitelist box's job, which says so out loud. The badge counts activations SEPARATELY from pending invites rather than widening an existing field's meaning, and only counts what genuinely waits on the admin: an unconfirmed account waits on its owner, so it is listed with a re-send but does not nag. The admin-side resend says plainly what happened, including refusing when the instance has no working SMTP — mail is fail-soft everywhere else, and that silence is exactly what left the 0.78.5 sign-ups with nothing to fall back on. `send_verification_link` is now one function with three callers (registration, the public resend, this one), so the link's shape and TTL cannot drift by who triggered it. UAT on localdev, both languages: the UX-25 shape reproduced end to end (approve the address, then register), the badge counting it, Activate letting them in and dropping the badge, the row losing its Activate but keeping the re-send, a fresh token issued by the re-send, and both actions in the audit log. Test data removed.
This commit is contained in:
@@ -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"
|
||||
|
||||
@@ -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.
|
||||
|
||||
+132
-1
@@ -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,
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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 {},
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user