fix(auth): commit the account on registration so a pre-approved sign-up isn't lost
_register_account() flushed the new User but relied on someone else to commit: upsert_pending_invite() commits only when it actually writes an invite row, and _issue_token() runs only when SMTP is enabled. For an email an admin had ALREADY approved, on an instance where email is unavailable, neither fired — the session closed and the account was rolled back. The would-be user got the uniform "ok" and no account, could not sign in, and re-registering was a silent no-op. Commit explicitly after the invite upsert (a no-op when upsert already committed the same transaction). Three DB-lane regression tests cover the pre-approved no-SMTP path (fails without the fix), the unknown-email path that always worked, and the pre-approved path with SMTP on.
This commit is contained in:
@@ -636,7 +636,14 @@ 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)
|
||||
upsert_pending_invite(db, email) # admin-approval gate (commits only when it writes a row)
|
||||
# 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.
|
||||
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).
|
||||
|
||||
@@ -127,3 +127,72 @@ def test_activating_a_verified_account_keeps_its_password(db, make_user):
|
||||
_activate_user_by_email(db, "legit@example.com")
|
||||
assert user.is_active is True
|
||||
assert user.password_hash is not None
|
||||
|
||||
|
||||
def _register_and_fetch(db, email: str):
|
||||
"""Run the /register background worker, then read the users/invites rows back through a FRESH
|
||||
query. The worker owns its own session, so anything it failed to commit is simply not there."""
|
||||
from sqlalchemy import select as _select
|
||||
|
||||
from app.models import Invite, User
|
||||
from app.routes.auth import _register_account
|
||||
|
||||
_register_account(email, _PW)
|
||||
db.expire_all()
|
||||
user = db.execute(_select(User).where(User.email == email)).scalar_one_or_none()
|
||||
invite = db.execute(_select(Invite).where(Invite.email == email)).scalar_one_or_none()
|
||||
return user, invite
|
||||
|
||||
|
||||
def test_registration_of_a_pre_approved_email_persists_the_account_without_smtp(db, monkeypatch):
|
||||
# The regression: 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
|
||||
from app.routes import auth as auth_mod
|
||||
|
||||
monkeypatch.setattr(auth_mod.email_mod, "email_enabled", lambda: False)
|
||||
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.is_active is False # admin approval still gates sign-in
|
||||
assert user.password_hash is not None
|
||||
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
|
||||
|
||||
|
||||
def test_registration_of_an_unknown_email_still_persists_account_and_pending_invite(
|
||||
db, monkeypatch
|
||||
):
|
||||
# The branch that always worked (upsert INSERTs the invite and commits) must keep working.
|
||||
from app.routes import auth as auth_mod
|
||||
|
||||
monkeypatch.setattr(auth_mod.email_mod, "email_enabled", lambda: False)
|
||||
|
||||
user, invite = _register_and_fetch(db, "stranger@example.com")
|
||||
|
||||
assert user is not None
|
||||
assert invite is not None and invite.status == "pending"
|
||||
|
||||
|
||||
def test_registration_of_a_pre_approved_email_persists_the_account_with_smtp(db, monkeypatch):
|
||||
# Same pre-approved path with SMTP on: the account must persist AND get a verification link.
|
||||
from app.models import Invite
|
||||
from app.routes import auth as auth_mod
|
||||
|
||||
sent: list[str] = []
|
||||
monkeypatch.setattr(auth_mod.email_mod, "email_enabled", lambda: True)
|
||||
monkeypatch.setattr(auth_mod.email_mod, "send_verify_email", lambda to, link: sent.append(to))
|
||||
monkeypatch.setattr(auth_mod.email_mod, "send_admin_new_request", lambda admins, to: True)
|
||||
db.add(Invite(email="rita@example.com", status="approved", decided_by="admin@example.com"))
|
||||
db.commit()
|
||||
|
||||
user, _ = _register_and_fetch(db, "rita@example.com")
|
||||
|
||||
assert user is not None
|
||||
assert user.email_verified is False # must confirm the address
|
||||
assert sent == ["rita@example.com"]
|
||||
|
||||
@@ -14,6 +14,19 @@ export interface ReleaseEntry {
|
||||
}
|
||||
|
||||
export const RELEASE_NOTES: ReleaseEntry[] = [
|
||||
{
|
||||
version: "0.78.5",
|
||||
date: "2026-09-01",
|
||||
summary:
|
||||
"Account emails are being delivered again — and a sign-up by someone the admin had already approved no longer vanishes without a trace.",
|
||||
fixes: [
|
||||
"Email is going out again. Verification links, password-reset links, approval and denial notices, and the “new access request” alert to the admin were all being silently dropped because the mail settings were incomplete — the app skipped each message instead of sending it. Check Configuration → Email and use “Send test email” if yours has ever gone quiet.",
|
||||
"Signing up with an address an admin had already approved now actually creates the account. The sign-up reported success but 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.",
|
||||
],
|
||||
chores: [
|
||||
"Registration now 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. Covered by three DB-lane regression tests: pre-approved without SMTP, unknown email, and pre-approved with SMTP.",
|
||||
],
|
||||
},
|
||||
{
|
||||
version: "0.78.4",
|
||||
date: "2026-08-30",
|
||||
|
||||
Reference in New Issue
Block a user