docs(auth): correct the last round of over-claiming comments and notes

Round 5's correctness/security pass came back clean; what was left was text that
said more than the code does.

- The register-path notify comment (and its test) claimed the admin "is still
  told". The call fires, but on a no-SMTP instance _send drops it — which is the
  open half of the problem, not something this branch fixed. Both now say so.
- The release notes tell admins what to actually do: an account whose address was
  whitelisted in advance has no pending request to approve, so the whitelist box
  is what activates it. That instruction only existed in the roadmap and in
  chores, which the dialog does not render.
- "That one line is the whole production change" now accounts for the test-only
  RateLimiter.reset() that also ships.
- The rate-limiter test claimed to pin that blocked attempts are not counted, on
  an assertion that held either way. Replaced with one that drives the clock
  across the window boundary and fails if hammering extends it.
This commit is contained in:
2026-09-01 22:02:43 +02:00
parent f6e1d1e69f
commit cb1f4f00b0
4 changed files with 31 additions and 8 deletions
+5 -3
View File
@@ -650,9 +650,11 @@ def _register_account(email: str, password: str) -> None:
# 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)
# Unconditional, unlike the other two callers which gate on a fresh invite row: for an
# address the admin already approved this notice is the ONLY signal that someone has now
# created an account on it, and that account is inactive until the admin acts.
# 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
# is the only place the account's existence is announced at all. Delivery is a separate
# matter — with no working SMTP `_send` drops it, which is why the pre-approved registrant
# still needs the admin to notice them (roadmap UX-25).
if admins:
email_mod.send_admin_new_request(admins, email)
+3 -2
View File
@@ -183,8 +183,9 @@ def test_registration_of_a_pre_approved_email_persists_the_account_without_smtp(
# so their Google sign-in keeps working and an anonymous sign-up cannot revoke access.
assert invite.status == "approved"
assert invite.decided_by == "admin@example.com"
# The admin is still told: for an already-approved address this notice is their only signal
# that an account now exists on it, and it is inactive until they act.
# The notify path still fires for an already-approved address — nothing lands in the queue, so
# it is the only announcement there is. (Whether it is DELIVERED is separate: on a real no-SMTP
# instance `_send` drops it, which is the open half of the problem, roadmap UX-25.)
assert notified == ["roland@example.com"]
+20 -1
View File
@@ -5,6 +5,7 @@ the module-level limiters between tests), so without a test here a refactor coul
no-op and nothing would notice until an unrelated test started failing with a 429.
"""
from app import ratelimit
from app.ratelimit import RateLimiter
@@ -14,10 +15,28 @@ def test_allow_trips_at_the_cap_and_stays_tripped():
assert lim.allow("ip") is True
assert lim.allow("ip") is True
assert lim.allow("ip") is False
# A blocked attempt is not counted, so a caller cannot push the window forward by hammering.
assert lim.allow("ip") is False
def test_hammering_while_blocked_does_not_push_the_window_forward(monkeypatch):
# A blocked attempt must not be recorded, otherwise someone who keeps retrying holds their own
# window open forever — an indefinite lockout for anyone sharing that IP. Only observable across
# the window boundary, so drive the clock: two attempts fill the cap, ten blocked retries land
# just before it expires, and the caller must still be let back in on the far side.
clock = {"t": 1000.0}
monkeypatch.setattr(ratelimit.time, "monotonic", lambda: clock["t"])
lim = RateLimiter(max_events=2, window_seconds=60)
assert lim.allow("ip") is True
assert lim.allow("ip") is True
for _ in range(10):
clock["t"] += 5 # still inside the window; every one of these is refused
assert lim.allow("ip") is False
clock["t"] = 1000.0 + 61 # past the window of the two attempts that actually counted
assert lim.allow("ip") is True
def test_keys_are_independent():
lim = RateLimiter(max_events=1, window_seconds=300)