fix(auth): reopen a pre-dated approval instead of auto-granting it

Replaces the activation added in ea931dc, which round 2 of the review showed was
a real weakening: on a no-SMTP instance it made a standing whitelist entry a
bearer grant — the first anonymous party to register an approved-but-unclaimed
address got an immediately usable account under that identity, with no
attestation, no admin action and no signal anywhere. The verify-time activation
had the same shape one step removed, and neither site ran the SEC-10 preset-
password guard or left an audit trail.

Keep a human in the loop instead. An approval granted before any account existed
was a decision about the ADDRESS, so registration reopens the invite
(upsert_pending_invite gains revive_approved, used only by the register worker).
The request lands in the Access-requests queue, approving it runs
_activate_user_by_email — and with it revoke_preset_password_if_unverified — and
the account activates. One mechanism, already audited, no new grant path, and
the "awaiting admin approval" copy is honest again.

The explicit commit from the previous commit still matters: with an invite
already pending, upsert returns without committing and _issue_token is gated on
SMTP, so a no-SMTP registration was still being rolled back.

Also from round 2: gate the admin notice on a fresh decision (matching the two
other upsert callers) so a retry no longer re-pings every admin, and reset the
module-level rate limiters between tests — the suite pooled every login into one
300s window and was two logins short of failing an unrelated test with a 429.

backend/app/ratelimit.py

@@ -18,6 +18,13 @@ class RateLimiter:
         self._hits: dict[str, list[float]] = {}
         self._lock = threading.Lock()
 
+    def reset(self) -> None:
+        """Forget every recorded attempt. Each limiter is a module-level singleton, so a test suite
+        sharing one process would otherwise pool all of its attempts into a single window and
+        eventually trip an unrelated test with a 429."""
+        with self._lock:
+            self._hits.clear()
+
     def allow(self, key: str) -> bool:
         """Record an attempt for ``key`` and return whether it is within the limit.
 

backend/app/routes/auth.py

@@ -147,16 +147,25 @@ def _client_ip(request: Request) -> str:
     return peer
 
 
-def upsert_pending_invite(db: Session, email: str) -> Invite | None:
+def upsert_pending_invite(
+    db: Session, email: str, *, revive_approved: bool = False
+) -> Invite | None:
     """Idempotently record an access request. Returns the Invite if a *new* pending row
-    was created (so the caller can notify admins), else None for already-known emails."""
+    was created (so the caller can notify admins), else None for already-known emails.
+
+    `revive_approved` is for the registration path only. An approval granted BEFORE any account
+    existed was a decision about the ADDRESS, not about whoever has now set a password on it, so the
+    request goes back in front of a human rather than becoming an automatic grant. It also un-strands
+    the registrant: an approved invite never appears in the Access-requests queue and `_decide` 409s
+    on an already-decided one, so nothing in the admin UI could otherwise activate the account."""
     email = (email or "").lower()
     if not valid_email(email):
         return None
     inv = db.execute(select(Invite).where(Invite.email == email)).scalar_one_or_none()
     if inv is not None:
-        # Let a previously-denied person ask again; don't disturb approved/pending rows.
-        if inv.status == "denied":
+        # Let a previously-denied person ask again; don't disturb a pending row (the admin already
+        # has that request in front of them).
+        if inv.status == "denied" or (revive_approved and inv.status == "approved"):
             inv.status = "pending"
             inv.requested_at = datetime.now(timezone.utc)
             inv.decided_at = None
@@ -636,29 +645,25 @@ def _register_account(email: str, password: str) -> None:
             # no-op as the pre-check hit — roll back and stop.
             db.rollback()
             return
-        upsert_pending_invite(db, email)  # admin-approval gate (commits only when it writes a row)
-        # Honour an approval the admin had ALREADY given for this address. Without this the account
-        # lands is_active=False against a non-pending invite: it never shows up in the Access-requests
-        # queue (which lists pending only) and approve_invite 409s on an already-decided invite, so
-        # the admin has no obvious way to let the person in. Attestation still gates it — with SMTP on
-        # the address is unverified at this point, so activation waits for the verify link (see
-        # verify_email_confirm). With no SMTP there is no attestation to wait for and the admin's
-        # whitelist is by design the only gate, the very one add_invite already activates on.
-        if not email_ok and is_allowed(db, email):
-            user.is_active = True
-        # Persist the account ourselves. upsert_pending_invite commits only when it actually inserts
-        # (or revives) an invite row, and _issue_token below only runs with SMTP on — so for an email
-        # the admin had ALREADY approved, on an instance with no SMTP, nothing along this path
-        # committed and the flushed user was silently rolled back when the session closed. The
-        # would-be user got a uniform "ok" and no account. Commit unconditionally; it is a no-op
-        # when upsert already committed the same transaction.
+        # admin-approval gate. revive_approved: an approval that predates the account is re-opened so
+        # a human binds it to THIS registration — that keeps the SEC-10 preset-password guard in the
+        # loop (approve_invite runs it) and puts the request somewhere the admin can actually see.
+        inv = upsert_pending_invite(db, email, revive_approved=True)
+        # Persist the account ourselves. upsert_pending_invite commits only when it actually writes an
+        # invite row, and _issue_token below only runs with SMTP on — so when a request for this
+        # address was already pending, on an instance with no SMTP, nothing along this path committed
+        # and the flushed user was silently rolled back when the session closed. The would-be user got
+        # a uniform "ok" and no account. 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}")
         admins = admin_notify_emails(db)
-        if admins:
+        # Only when there is a fresh decision to make, matching the other two call sites. Without the
+        # guard an address whose request was already pending re-pinged every admin on each retry.
+        if inv is not None and admins:
             email_mod.send_admin_new_request(admins, email)
 
 
@@ -672,12 +677,6 @@ def verify_email_confirm(body: TokenIn, db: Session = Depends(get_db)) -> dict:
             status_code=400, detail="This verification link is invalid or has expired."
         )
     user.email_verified = True
-    # Ownership is attested now (only the mailbox owner can follow this link), so an approval the
-    # admin granted BEFORE the person signed up can finally be honoured. Otherwise they sit at
-    # is_active=False behind a request no admin screen shows — the invite is already decided, so it
-    # is absent from the pending queue and approve_invite would 409 on it.
-    if not user.is_active and is_allowed(db, user.email):
-        user.is_active = True
     db.commit()
     return {"ok": True}
 

backend/tests/conftest.py

@@ -189,3 +189,20 @@ def as_user(db, override_dep, _configured):
         return TestClient(app, headers={"x-test-user": str(user.id)})
 
     return _as
+
+
+@pytest.fixture(autouse=True)
+def _reset_rate_limiters():
+    """Every RateLimiter is a module-level singleton keyed by client IP, and TestClient always
+    presents the same host — so without this the whole suite's login/register attempts pool into one
+    300s window and, past the cap, some unrelated test starts failing with a 429 depending on order.
+    Cleared before each test; a test that wants to exercise throttling can still exhaust one itself."""
+    import app.routes.auth as _auth
+    import app.routes.messages as _messages
+    import app.routes.public as _public
+    from app.ratelimit import RateLimiter
+
+    for mod in (_auth, _messages, _public):
+        for obj in vars(mod).values():
+            if isinstance(obj, RateLimiter):
+                obj.reset()

backend/tests/test_auth_session.py

@@ -144,56 +144,106 @@ def _register_and_fetch(db, email: str):
     return user, invite
 
 
-def test_registration_of_a_pre_approved_email_persists_the_account_without_smtp(db, monkeypatch):
-    # THE regression (the only one of these that fails without the explicit commit in
-    # _register_account): an admin pre-approves an address, the person then registers, and the
-    # account silently never existed. upsert_pending_invite returns early WITHOUT committing for an
-    # already-approved email, and the only other commit on this path (_issue_token) is gated on
-    # SMTP — so on a no-SMTP instance the flushed user was rolled back at session close.
-    from app.models import Invite
+def _no_smtp(monkeypatch):
+    """Put the worker on the no-SMTP path and capture the admin notice instead of sending it."""
     from app.routes import auth as auth_mod
 
+    notified: list[str] = []
     monkeypatch.setattr(auth_mod.email_mod, "email_enabled", lambda: False)
+    monkeypatch.setattr(
+        auth_mod.email_mod, "send_admin_new_request", lambda admins, to: notified.append(to) or True
+    )
+    return notified
+
+
+def test_registration_with_an_already_pending_request_persists_the_account(
+    db, make_user, monkeypatch
+):
+    # THE regression: someone asks for access, then registers before the admin has decided.
+    # upsert_pending_invite leaves the pending row alone and returns WITHOUT committing, and the only
+    # other commit on this path (_issue_token) is gated on SMTP — so on a no-SMTP instance the flushed
+    # user was rolled back at session close. The sign-up answered "ok" and created nothing.
+    from app.models import Invite
+
+    notified = _no_smtp(monkeypatch)
+    make_user(email="admin@example.com", role="admin")
+    db.add(Invite(email="pia@example.com", status="pending"))
+    db.commit()
+
+    user, invite = _register_and_fetch(db, "pia@example.com")
+
+    assert user is not None, "registration was rolled back — the account is gone"
+    assert user.password_hash is not None
+    assert user.is_active is False  # the admin has not decided yet
+    assert invite.status == "pending"
+    assert notified == []  # the admin already has this request; don't re-ping on every retry
+
+
+def test_registration_of_a_pre_approved_email_reopens_the_request_for_the_admin(
+    db, make_user, monkeypatch
+):
+    # An approval granted before any account existed was a decision about the ADDRESS. Binding it to
+    # whoever has now set a password on that address is a human's call, so the invite goes back to
+    # pending — which is also the only way the admin can reach it: an approved invite is absent from
+    # the Access-requests queue and _decide 409s on an already-decided one.
+    from app.models import Invite
+
+    notified = _no_smtp(monkeypatch)
+    make_user(email="admin@example.com", role="admin")
     db.add(Invite(email="roland@example.com", status="approved", decided_by="admin@example.com"))
     db.commit()
 
     user, invite = _register_and_fetch(db, "roland@example.com")
 
-    assert user is not None, "pre-approved registration was rolled back — the account is gone"
-    assert user.password_hash is not None
+    assert user is not None
+    assert user.is_active is False  # no automatic grant — approving it is what activates
     assert user.email_verified is True  # no SMTP => ownership can't be proven by link
-    assert invite.status == "approved"  # an existing approval is never disturbed
-    # With no SMTP there is no attestation to wait for, so the admin's standing approval IS the gate
-    # and the account is usable at once. Otherwise it would sit inactive behind an invite that is no
-    # longer pending — invisible in the Access-requests queue, and approve_invite 409s on it.
+    assert invite.status == "pending"
+    assert invite.decided_at is None and invite.decided_by is None
+    assert notified == ["roland@example.com"]
+
+
+def test_approving_the_reopened_request_activates_the_account(db, make_user, monkeypatch):
+    # The other half of the loop: the admin clicks Approve and the person can sign in. This is what
+    # keeps the SEC-10 preset-password guard in the path — _activate_user_by_email runs it.
+    from app.models import Invite
+    from app.routes.admin import _activate_user_by_email
+
+    _no_smtp(monkeypatch)
+    db.add(Invite(email="roland@example.com", status="approved", decided_by="admin@example.com"))
+    db.commit()
+    user, _ = _register_and_fetch(db, "roland@example.com")
+
+    _activate_user_by_email(db, "roland@example.com")
+
+    db.expire_all()
     assert user.is_active is True
+    assert user.password_hash is not None  # verified account keeps its own password
 
 
 def test_registration_of_an_unknown_email_still_persists_account_and_pending_invite(
     db, monkeypatch
 ):
     # Guard, not a regression test: this branch always worked (upsert INSERTs the invite and commits
-    # the same transaction). It must keep working — and the account must stay inactive, since nobody
-    # has approved this address.
-    from app.routes import auth as auth_mod
-
-    monkeypatch.setattr(auth_mod.email_mod, "email_enabled", lambda: False)
+    # the same transaction). It must keep working — and the account stays inactive, nobody has
+    # approved this address.
+    notified = _no_smtp(monkeypatch)
 
     user, invite = _register_and_fetch(db, "stranger@example.com")
 
     assert user is not None
     assert user.is_active is False
     assert invite is not None and invite.status == "pending"
+    assert notified == []  # no admin in this DB, so nothing to notify
 
 
-def test_registration_of_a_previously_denied_email_revives_the_request(db, monkeypatch):
-    # Guard: a denied person may ask again — upsert_pending_invite flips the invite back to pending
-    # (and commits) so the admin sees a fresh request. The stale decision must not leak into the new
-    # account's state, and re-registering must not be a dead end.
+def test_registration_of_a_previously_denied_email_revives_the_request(db, make_user, monkeypatch):
+    # Guard: a denied person may ask again — the invite flips back to pending so the admin sees a
+    # fresh request. The stale decision must not leak into the new account's state.
     from app.models import Invite
-    from app.routes import auth as auth_mod
 
-    monkeypatch.setattr(auth_mod.email_mod, "email_enabled", lambda: False)
+    notified = _no_smtp(monkeypatch)
+    make_user(email="admin@example.com", role="admin")
     db.add(Invite(email="denis@example.com", status="denied", decided_by="admin@example.com"))
     db.commit()
 
@@ -203,12 +253,13 @@ def test_registration_of_a_previously_denied_email_revives_the_request(db, monke
     assert user.is_active is False
     assert invite.status == "pending"
     assert invite.decided_at is None and invite.decided_by is None
+    assert notified == ["denis@example.com"]
 
 
 def test_registration_of_a_pre_approved_email_persists_the_account_with_smtp(db, monkeypatch):
     # Guard, not a regression test: with SMTP on, _issue_token's own commit already persisted the
-    # user, so this branch was never broken and must stay unbroken. Here the account waits for the
-    # verify link (see the activation test below), so it is inactive and unverified.
+    # user, so this branch was never broken and must stay unbroken. The reopened request and the
+    # verification link are independent gates — the account clears both before it can sign in.
     from app.models import Invite
     from app.routes import auth as auth_mod
 
@@ -219,51 +270,32 @@ def test_registration_of_a_pre_approved_email_persists_the_account_with_smtp(db,
     db.add(Invite(email="rita@example.com", status="approved", decided_by="admin@example.com"))
     db.commit()
 
-    user, _ = _register_and_fetch(db, "rita@example.com")
+    user, invite = _register_and_fetch(db, "rita@example.com")
 
     assert user is not None
     assert user.email_verified is False  # must confirm the address
     assert user.is_active is False
+    assert invite.status == "pending"
     assert sent == ["rita@example.com"]
 
 
-def test_verifying_an_email_activates_a_pre_approved_account(client, db, make_user):
-    # Confirming the emailed link attests ownership, so an approval the admin granted before the
-    # person signed up is honoured right there. Without this the SMTP-on pre-approved registrant is
-    # stuck: their invite is already decided, so it never reaches the Access-requests queue and
-    # approve_invite 409s on it.
+def test_verifying_an_email_never_grants_access_on_its_own(client, db, make_user):
+    # Verification attests the address; it does not decide access. An account whose request is still
+    # pending stays inactive and cannot sign in — the two gates are deliberately separate.
     from app.models import Invite
     from app.routes.auth import _issue_token
 
     user = make_user(email="rita@example.com", password=_PW, email_verified=False, is_active=False)
-    db.add(Invite(email="rita@example.com", status="approved", decided_by="admin@example.com"))
+    db.add(Invite(email="rita@example.com", status="pending"))
     db.commit()
     raw = _issue_token(db, user, "verify", timedelta(hours=1))
 
     assert client.post("/auth/verify", json={"token": raw}).status_code == 200
 
-    db.expire_all()
-    assert user.email_verified is True
-    assert user.is_active is True
-    assert _login(client, "rita@example.com", _PW).status_code == 200
-
-
-def test_verifying_an_email_does_not_activate_an_unapproved_account(client, db, make_user):
-    # The mirror guard: verification proves the address, it does not grant access. With no approved
-    # invite the account stays inactive and sign-in is still refused.
-    from app.routes.auth import _issue_token
-
-    user = make_user(
-        email="nobody@example.com", password=_PW, email_verified=False, is_active=False
-    )
-    raw = _issue_token(db, user, "verify", timedelta(hours=1))
-
-    assert client.post("/auth/verify", json={"token": raw}).status_code == 200
-
     db.expire_all()
     assert user.email_verified is True
     assert user.is_active is False
-    assert _login(client, "nobody@example.com", _PW).status_code == 403
+    assert _login(client, "rita@example.com", _PW).status_code == 403
 
 
 def test_register_route_creates_the_account_end_to_end(client, db, monkeypatch):
@@ -274,9 +306,8 @@ def test_register_route_creates_the_account_end_to_end(client, db, monkeypatch):
     from sqlalchemy import select as _select
 
     from app.models import User
-    from app.routes import auth as auth_mod
 
-    monkeypatch.setattr(auth_mod.email_mod, "email_enabled", lambda: False)
+    _no_smtp(monkeypatch)
 
     r = client.post("/auth/register", json={"email": "newbie@example.com", "password": _PW})
 
@@ -293,9 +324,8 @@ def test_register_route_is_refused_when_registration_is_closed(client, db, monke
 
     from app import sysconfig
     from app.models import User
-    from app.routes import auth as auth_mod
 
-    monkeypatch.setattr(auth_mod.email_mod, "email_enabled", lambda: False)
+    _no_smtp(monkeypatch)
     sysconfig.set_value(db, "allow_registration", False)
 
     r = client.post("/auth/register", json={"email": "closed@example.com", "password": _PW})

frontend/src/lib/releaseNotes.ts

@@ -18,13 +18,15 @@ export const RELEASE_NOTES: ReleaseEntry[] = [
     version: "0.78.5",
     date: "2026-09-01",
     summary:
-      "Signing up with an address the admin had already approved now works end to end — the account is created, and confirming your email lets you straight in instead of parking you behind an approval that already happened.",
+      "Two ways a sign-up could quietly go nowhere are fixed: on an instance that sends no email the account was discarded outright, and an address the admin had whitelisted in advance landed in a request no admin screen ever showed.",
     fixes: [
-      "Signing up with an address an admin had already approved now actually creates the account. Before, the sign-up reported success and the account was discarded on the spot, so the person could neither sign in nor ask for access again — they simply had no account at all.",
-      "Such an account no longer waits for an approval it already has. Confirming the emailed link now activates it straight away, and on an instance that sends no email the admin's whitelist alone is enough — previously the sign-up landed in a state the Access-requests screen never showed, because the request had already been decided.",
+      "On an instance that sends no email, signing up with an address that already had a request on file created nothing at all. The sign-up reported success, no account existed, and trying again did the same — so the person could neither sign in nor get back into the queue.",
+      "Whitelisting someone before they sign up now works the way it looks like it should. Their sign-up reopens the request so it appears under Users → Access requests, where approving it lets them in; previously it landed against a decision already made, which the queue does not list and the Approve action refuses, leaving nobody able to complete it.",
+      "Retrying a sign-up while a request is already pending no longer emails every admin again.",
     ],
     chores: [
-      "Registration commits the new account explicitly: for an already-approved email, upsert_pending_invite() returns without committing and the only other commit on that path is gated on working SMTP, so with mail unavailable the flushed user row was rolled back when the background task's session closed. Activation is now derived from is_allowed() at the moment ownership is attested — at email verification, or at registration when the instance has no email to verify with. Covered by one regression test plus guards for every other branch of the worker, and the first end-to-end tests of the /auth/register route itself.",
+      "Registration commits the new account explicitly — upsert_pending_invite() commits only when it actually writes an invite row, and the only other commit on that path is gated on working SMTP, so with mail unavailable the flushed user row was rolled back when the background task's session closed. An approval that predates the account now reopens the invite instead of being treated as a standing grant, which keeps a human (and with them the SEC-10 preset-password guard in _activate_user_by_email) between a whitelisted address and whoever registers it. The admin notice is gated on a fresh decision, matching the other two callers.",
+      "Two regression tests (no-SMTP with a request already pending, and the reopen-then-approve loop) plus guards for the unknown-email, previously-denied and SMTP-on branches, the first end-to-end tests of the /auth/register route and its allow_registration gate, and an autouse fixture that resets the module-level rate limiters between tests so their windows stop pooling across the suite.",
       "Operator note: an incomplete SMTP setting makes Siftlode skip every message silently by design. If your instance has never delivered mail, check Configuration → Email and use “Send test email”.",
     ],
   },