diff --git a/backend/app/routes/auth.py b/backend/app/routes/auth.py index b7863313..10c6c123 100644 --- a/backend/app/routes/auth.py +++ b/backend/app/routes/auth.py @@ -636,7 +636,7 @@ def _register_account(email: str, password: str) -> None: # no-op as the pre-check hit — roll back and stop. db.rollback() return - inv = upsert_pending_invite(db, email) # admin-approval gate + upsert_pending_invite(db, email) # admin-approval gate # Persist the account ourselves. upsert_pending_invite commits only when it actually writes an # invite row — not when this address already has a pending or approved one — and _issue_token # below only runs with SMTP on. So for an address the admin had already approved (or one whose @@ -650,10 +650,10 @@ 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) - # Only when there is a fresh decision to make, matching the other two call sites. Otherwise - # someone who used Request access and then registered sent admins a second copy of the same - # request, and an already-approved address produced a notice pointing at an empty queue. - if inv is not None and admins: + # 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. + if admins: email_mod.send_admin_new_request(admins, email) diff --git a/backend/tests/test_auth_session.py b/backend/tests/test_auth_session.py index 0f41b613..2e77a0dd 100644 --- a/backend/tests/test_auth_session.py +++ b/backend/tests/test_auth_session.py @@ -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" - # …and no "new access request" notice, because there is no fresh decision to make. - assert notified == [] + # 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. + assert notified == ["roland@example.com"] def test_registration_with_an_already_pending_request_persists_the_account( @@ -204,7 +205,7 @@ def test_registration_with_an_already_pending_request_persists_the_account( assert user is not None, "registration was rolled back — the account is gone" assert user.is_active is False assert invite.status == "pending" - assert notified == [] # the admin already has this request; don't send a second copy + assert notified == ["pia@example.com"] def test_an_admin_can_activate_a_pre_approved_registrant(db, make_user, monkeypatch): diff --git a/frontend/src/lib/releaseNotes.ts b/frontend/src/lib/releaseNotes.ts index abae450c..aac775bf 100644 --- a/frontend/src/lib/releaseNotes.ts +++ b/frontend/src/lib/releaseNotes.ts @@ -20,13 +20,12 @@ export const RELEASE_NOTES: ReleaseEntry[] = [ summary: "On an instance that cannot send email, signing up with an address that had already been whitelisted or had already requested access created no account at all. The sign-up said it worked, nothing existed, and trying again did the same.", fixes: [ - "Such a sign-up now creates the account. Previously the person could neither sign in nor get back into the access queue — they had no account to approve, and every retry silently repeated the same nothing. Only a brand-new address was unaffected. As before, the account starts inactive and an admin's approval is what lets it sign in.", - "Signing up with an address that had already asked for access no longer sends admins a second copy of the same request, and one that was already whitelisted no longer produces a notice pointing at a queue with nothing in it.", + "Such a sign-up now creates the account. Before, there was nothing behind the request for an admin to approve, and every retry silently repeated the same nothing. Other sign-ups were unaffected — a brand-new address, one that had been denied and was asking again, and every sign-up on an instance with working email all created their account as expected. As before, a new account starts inactive and an admin's approval is what lets it sign in.", "Operator note: an incomplete SMTP setting makes Siftlode skip every message silently by design — that is what left the sign-ups above with nothing to fall back on. If your instance has never delivered mail, check Configuration → Email and use “Send test email”.", ], chores: [ - "Registration commits the new account explicitly. upsert_pending_invite() commits only when it actually writes an invite row — not when the address already has a pending or approved one — 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. The admin notice is now gated on that same fresh-decision signal, matching the two other callers.", - "Three DB-lane regression tests (pre-approved and already-pending sign-ups on a no-SMTP instance, plus the admin-activation recovery that depends on the account existing), 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, unit tests for the rate limiter, and an autouse fixture resetting the module-level limiters between tests so their windows stop pooling across the suite.", + "Registration commits the new account explicitly. upsert_pending_invite() commits only when it actually writes an invite row — not when the address already has a pending or approved one — 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. That one line is the whole production change; two attempts to also fix the pre-approved account's activation UX were reverted after review (roadmap UX-25 records why).", + "Tests: three that fail without the fix (pre-approved and already-pending sign-ups on a no-SMTP instance, and the admin-activation recovery that depends on the account existing), guards for the unknown-email, previously-denied and SMTP-on branches and for verification not granting access on its own, the first end-to-end tests of the /auth/register route and its allow_registration gate, unit tests for the rate limiter, and an autouse fixture resetting the module-level limiters between tests so their windows stop pooling across the suite.", ], }, {