diff --git a/backend/alembic/versions/0081_invite_invited_at.py b/backend/alembic/versions/0081_invite_invited_at.py new file mode 100644 index 00000000..c9230499 --- /dev/null +++ b/backend/alembic/versions/0081_invite_invited_at.py @@ -0,0 +1,34 @@ +"""invites.invited_at — when the admin actually told this person the door was open + +Revision ID: 0081_invite_invited_at +Revises: 0080_google_email +Create Date: 2026-09-05 + +Whitelisting an address stays SILENT by design: the operator is opening a door as a possibility, +usually after agreeing it with the person, and a row can sit approved for weeks without anyone +registering. Telling them is therefore a separate, deliberate act — a send button on the row. + +Which raises the one question the admin cannot otherwise answer: "did I already write to them?" +Without a record the only failure mode is writing to a real person twice, which is exactly the +nudging the silent-by-default design exists to avoid. The audit log holds the event, but not where +the decision is made. + +NULL means "never invited from here", which is true of every existing row: no backfill is possible +or wanted, since the historic approval mails answered a REQUEST rather than inviting anyone. +""" + +import sqlalchemy as sa +from alembic import op + +revision: str = "0081_invite_invited_at" +down_revision: str | None = "0080_google_email" +branch_labels: str | None = None +depends_on: str | None = None + + +def upgrade() -> None: + op.add_column("invites", sa.Column("invited_at", sa.DateTime(timezone=True), nullable=True)) + + +def downgrade() -> None: + op.drop_column("invites", "invited_at") diff --git a/backend/app/audit.py b/backend/app/audit.py index 95d24515..dae44955 100644 --- a/backend/app/audit.py +++ b/backend/app/audit.py @@ -35,6 +35,9 @@ class AuditAction: INVITE_DENY = "invite_deny" INVITE_ADD = "invite_add" INVITE_DELETE = "invite_delete" + # E9.5 S6: the admin chose to tell someone their address is whitelisted. Adding the address is + # silent, so this is the act that actually reaches a person — and the one worth a record. + INVITE_SEND = "invite_send" DEMO_ADD = "demo_add" DEMO_REMOVE = "demo_remove" DEMO_RESET = "demo_reset" diff --git a/backend/app/auth.py b/backend/app/auth.py index 17a4e9e1..f2283b1f 100644 --- a/backend/app/auth.py +++ b/backend/app/auth.py @@ -121,14 +121,20 @@ def count_pending_activations(db: Session) -> int: 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), + # Narrowed in SQL, then decided by can_activate itself — so the number the badge shows and the + # button the admin sees cannot drift apart by one of them re-deriving the rule. + 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)) + .scalars() + .all() + ) + return sum(1 for u in rows if can_activate(db, u)) def find_user_by_email(db: Session, email: str) -> User | None: diff --git a/backend/app/email.py b/backend/app/email.py index b0e9d451..18062b3e 100644 --- a/backend/app/email.py +++ b/backend/app/email.py @@ -184,19 +184,74 @@ def _send(to: list[str], subject: str, body: str, reply_to: str | None = None) - return False -def send_access_approved(email: str) -> bool: +def _next_step(verify_link: str | None, has_account: bool, hours: int) -> str: + """What this person actually has to do next, in the one paragraph that carries the whole flow. + + Three states, because a mail that guesses wrong sends someone looking for a link that isn't + coming — or worse, tells them to sign in when their address is still unconfirmed and sign-in will + refuse them (E9.5 S4 named that state on screen; the mail must not contradict it). + + The link branch is the point of the C option: in the common ordering — approve first, the person + signs up later, or signs up and waits — they would otherwise have to find and open TWO separate + messages. One mail, one click. + """ + if verify_link: + return ( + "You already started an account, so there is just one step left. Confirm your email " + "address and you're in:\n\n" + f"{verify_link}\n\n" + f"The link works once and expires in {hours} hours. If it does, you can ask for a fresh " + "one from the sign-in page." + ) + if has_account: + return f"Your account is ready — just sign in:\n\n{settings.app_base}" + return ( + "Create your account whenever you feel like it:\n\n" + f"{settings.app_base}\n\n" + "We'll send you a confirmation link to finish, and then you're in." + ) + + +def send_access_approved( + email: str, verify_link: str | None = None, has_account: bool = False, hours: int = 24 +) -> bool: + """Answer someone who ASKED for access. Sent automatically on approval, because a person who + made a request is owed a reply — unlike the admin-initiated invitation below.""" admin = _admin_contact() body = ( "Hi,\n\n" "Your request to use Siftlode has been approved — welcome aboard.\n\n" - "Open Siftlode and sign in, and your YouTube subscriptions feed will be ready:\n\n" - f"{settings.app_base}\n\n" + f"{_next_step(verify_link, has_account, hours)}\n\n" "If you weren't expecting this, you can ignore the message.\n\n" "— Siftlode" + (f"\n\nQuestions? Just reply to this email ({admin})." if admin else "") ) return _send([email], "You're in — your Siftlode access is approved", body, reply_to=admin) +def send_access_invitation( + email: str, verify_link: str | None = None, has_account: bool = False, hours: int = 24 +) -> bool: + """Invite someone the admin let in WITHOUT them asking — sent only when the admin presses send. + + Whitelisting an address is opening a door, not knocking on one: the operator has usually already + agreed it with the person, and an address can sit approved for weeks without anyone registering. + So adding to the whitelist stays silent and this is a separate, deliberate act. That is also why + the closing line offers no deadline — nudging is the one thing this mail must not do. + """ + admin = _admin_contact() + body = ( + "Hi,\n\n" + "Someone has opened up Siftlode for you — a quiet, self-hosted reader for your YouTube " + "subscriptions. Every upload from the channels you follow lands in one feed you can " + "actually filter and sort, with no algorithm deciding what you see, and no Shorts or " + "livestreams unless you want them.\n\n" + f"{_next_step(verify_link, has_account, hours)}\n\n" + "No rush — the door stays open.\n\n" + "— Siftlode" + (f"\n\nQuestions? Just reply to this email ({admin})." if admin else "") + ) + return _send([email], "You're invited to Siftlode", body, reply_to=admin) + + def send_access_denied(email: str) -> bool: admin = _admin_contact() body = ( @@ -277,11 +332,15 @@ def send_admin_new_request(admins: list[str], requester: str) -> bool: return _send(admins, f"Siftlode access request from {requester}", body, reply_to=requester) -def send_verify_email(to: str, link: str) -> bool: +def send_verify_email(to: str, link: str, hours: int = 24) -> bool: body = ( "Hi,\n\n" "Confirm your email to finish creating your Siftlode account:\n\n" f"{link}\n\n" + # Say what the link's rules are. The reset mail always has; this one left the reader to + # discover both by meeting a dead link, which is precisely the silence this epic removes. + f"The link works once and expires in {hours} hours. If it does, you can ask for a fresh one " + "from the sign-in page.\n\n" "If you didn't sign up, you can ignore this message.\n\n" "— Siftlode" ) diff --git a/backend/app/models.py b/backend/app/models.py index c9601577..2b3e5fca 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -167,6 +167,14 @@ class Invite(Base, CanonicalEmailMixin): ) decided_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) decided_by: Mapped[str | None] = mapped_column(String(320)) # admin email + # When the admin last SENT this person an invitation from the whitelist row. Adding an + # address stays silent (it opens a door, it does not knock), so telling them is a separate + # act — and this is what stops it happening twice to a real person. + invited_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) + # When the admin last SENT this person an invitation from the whitelist row. Adding an address + # stays silent — it opens a door, it does not knock — so telling them is a separate, deliberate + # act, and this is what stops it happening twice to a real person. + invited_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) class DemoWhitelist(Base, TimestampMixin): diff --git a/backend/app/routes/admin.py b/backend/app/routes/admin.py index bf867d76..da807d67 100644 --- a/backend/app/routes/admin.py +++ b/backend/app/routes/admin.py @@ -16,6 +16,7 @@ from app.auth import ( admin_user, can_activate, count_admins, + find_user_by_email, get_or_create_demo_user, is_allowed, operator_contact, @@ -30,7 +31,7 @@ from app.models import ( User, ) from app.routes._common import get_or_404 -from app.routes.auth import send_verification_link +from app.routes.auth import APPROVAL_VERIFY_TTL, issue_verification_link, send_verification_link from app.utils import canonical_email, iso, valid_email router = APIRouter(prefix="/api/admin", tags=["admin"]) @@ -71,6 +72,10 @@ def _serialize(inv: Invite, *, has_account: bool = False) -> dict: "requested_at": iso(inv.requested_at), "decided_at": iso(inv.decided_at), "decided_by": inv.decided_by, + # When (if ever) the admin sent this person an invitation. Adding an address is silent, so + # without this the admin has no way to answer "did I already write to them?" — and the only + # failure mode is writing to a real person twice. + "invited_at": iso(inv.invited_at), # Drives the delete affordance: an entry backed by an account is not removable here. "has_account": has_account, # …and whether removing it would actually take access away, or leave the env allow-list @@ -96,6 +101,30 @@ def list_invites( return [_serialize(i, has_account=canonical_email(i.email) in accounts) for i in rows] +def _welcome_mail_parts(db: Session, email: str) -> tuple[str | None, bool, int]: + """What a welcome or invitation mail should tell this address to do next: a confirmation link (if + there is an unconfirmed account behind it), whether an account exists at all, and how long the + link lasts. + + This is the C option's whole mechanic. In the common ordering — the address is approved and the + person signs up either side of that — they would otherwise have to find and open TWO messages: + one saying they are in, another asking them to confirm. Carrying the link inside the mail that is + already going out makes it one mail and one click. + + Resolved through `find_user_by_email`, so an account registered under another spelling of the + same Gmail mailbox is still recognised — and, deliberately, so several candidates resolve to + "no single answer" rather than a guess: an invitation that carried the wrong account's link would + be worse than one that simply pointed at the sign-up page. + """ + hours = int(APPROVAL_VERIFY_TTL.total_seconds() // 3600) + user = find_user_by_email(db, email) + if user is None or user.is_demo: + return None, False, hours + if user.email_verified or not email_mod.email_enabled(db): + return None, True, hours + return issue_verification_link(db, user, APPROVAL_VERIFY_TTL), True, hours + + def _activate_user_by_email(db: Session, email: str) -> None: """Approving access activates a matching (e.g. pending password-registration) account so it can sign in. No-op if no such user exists yet (a Google user is created at first login). @@ -159,7 +188,11 @@ def approve_invite( ) -> dict: inv = _decide(db, invite_id, admin, approved=True) _activate_user_by_email(db, inv.email) - background.add_task(email_mod.send_access_approved, inv.email) + # Computed here, not in the background task: issuing the token needs this request's session, and + # the task must not outlive it. Approval answers a request the person MADE, so unlike the + # invitation below it is sent automatically — a reply is owed, not a nudge. + link, has_account, hours = _welcome_mail_parts(db, inv.email) + background.add_task(email_mod.send_access_approved, inv.email, link, has_account, hours) return _serialize(inv, has_account=bool(_accounts_for(db, [inv.email]))) @@ -211,6 +244,49 @@ def add_invite( return _serialize(inv, has_account=bool(_accounts_for(db, [email]))) +@router.post("/invites/{invite_id}/invite") +def send_invitation( + invite_id: int, + background: BackgroundTasks, + admin: User = Depends(admin_user), + db: Session = Depends(get_db), +) -> dict: + """Tell someone their address is whitelisted — only when the admin asks for it. + + Whitelisting stays SILENT on purpose. The operator opens a door as a possibility, usually having + already agreed it with the person, and an address can sit approved for weeks with nobody + registering; a mail fired automatically would be nudging someone who never asked to be nudged. + So the send is its own act, on its own button, and `invited_at` records it so the admin can see + they have already written rather than writing twice. + + Only an APPROVED row can be invited: there is nothing to invite someone to while their request is + undecided, and inviting a denied address would contradict the decision.""" + inv = get_or_404(db, Invite, invite_id, detail="No such invite") + if inv.status != "approved": + raise HTTPException( + status_code=409, + detail={"code": "invite_send", "reason": "not_approved", "email": inv.email}, + ) + if not email_mod.email_enabled(db): + # Same reasoning as the admin-side verification resend: mail is fail-soft everywhere else, + # but a button the admin pressed must not answer with a success toast for a mail that was + # never sent. + raise HTTPException(status_code=409, detail={"code": "invite_send", "reason": "email_off"}) + link, has_account, hours = _welcome_mail_parts(db, inv.email) + inv.invited_at = datetime.now(timezone.utc) + audit.record( + db, + admin.id, + AuditAction.INVITE_SEND, + target_type="invite", + target_id=inv.id, + summary=inv.email, + ) + db.commit() + background.add_task(email_mod.send_access_invitation, inv.email, link, has_account, hours) + return _serialize(inv, has_account=has_account) + + @router.delete("/invites/{invite_id}") def delete_invite( invite_id: int, @@ -459,13 +535,16 @@ def list_pending_access(_: User = Depends(admin_user), db: Session = Depends(get 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 - ] + out = [] + for u in _pending_access_rows(db): + # The whole rule in one line: this address MAY sign in, and this account still can't. An + # undecided request fails it (nothing is allowed yet) and so does a denied one — both belong + # to the queue above, which owns that decision. Without this, every fresh registration + # appeared in both lists at once, which UAT caught on 2026-09-05. + if not is_allowed(db, u.email): + continue + out.append(_serialize_user(u) | {"can_activate": can_activate(db, u)}) + return out @router.post("/users/{user_id}/activate") diff --git a/backend/app/routes/auth.py b/backend/app/routes/auth.py index 1f8a743d..85234d01 100644 --- a/backend/app/routes/auth.py +++ b/backend/app/routes/auth.py @@ -43,6 +43,12 @@ from app.security import encrypt, hash_password, hash_token, verify_password from app.utils import canonical_email, valid_email VERIFY_TTL = timedelta(hours=24) +# A confirmation link that rides an APPROVAL or an invitation lives longer than one the person is +# waiting for. Nobody sits refreshing their inbox for an approval — it can easily be read on Monday +# after a weekend, and if it has expired by then the one-mail-one-click promise is spent and they are +# back to two messages. The risk is small: this token only ever confirms an address. It signs nobody +# in and sets no password. +APPROVAL_VERIFY_TTL = timedelta(days=7) RESET_TTL = timedelta(hours=1) _DECOY_PASSWORD_HASH = hash_password(secrets.token_urlsafe(16)) @@ -254,6 +260,12 @@ class EmailIn(BaseModel): class EmailPasswordIn(BaseModel): email: str | None = None password: str | None = None + # The language the sign-up page was in. Captured at registration because it is the ONLY moment + # the choice exists anywhere the server can see: it lives in the browser until an account does, + # and a preference nobody recorded cannot be backfilled later (the lesson `google_email` taught + # in E9.5 S3 — those rows still read NULL until each user signs in again). Nothing reads it yet; + # the bilingual mail templates are E9.5 S7. + lang: str | None = None class TokenIn(BaseModel): @@ -718,11 +730,11 @@ def register( # session). The create path hashes the password + writes several rows + schedules emails; doing it # inline would make a NEW email respond measurably slower than an already-registered one (which # skips all that) — a timing oracle for enumeration. Off-path, any valid email responds the same. - background.add_task(_register_account, email, password) + background.add_task(_register_account, email, password, body.lang) return {"status": "ok"} -def _register_account(email: str, password: str) -> None: +def _register_account(email: str, password: str, lang: str | None = None) -> None: """Background worker for /register: create the pending account (+ verification email + admin notice) for a genuinely new email; no-op for an already-registered one. Runs after the response with its own DB session, so the request's timing never reveals whether the email already exists.""" @@ -734,7 +746,20 @@ def _register_account(email: str, password: str) -> None: select(User).where(User.email_canonical == canonical_email(email)) ).first() if existing is not None: - return # already registered — nothing to do (the uniform "ok" was already returned) + # Already registered. This used to return in silence, and that silence is the most + # likely explanation for the 2026-09-03 report that a confirmation mail "never arrived": + # nothing was ever SENT. The person re-registered, got the same cheerful "check your + # inbox", and waited for a message that did not exist. + # + # Re-sending the link instead leaks nothing new. The response and its timing are + # unchanged (uniform "ok", decided off the response path), the mail goes to the address + # ON THE ACCOUNT, and this is exactly what the public /auth/verify/resend already does + # for the same input — so it is not a new capability, just the end of a dead end. An + # account that is already confirmed still gets nothing: there is nothing to confirm. + user = existing[0] + if not user.email_verified and not user.is_demo and email_mod.email_enabled(): + send_verification_link(db, user) + return # Without working SMTP we can't deliver a verification link, so email ownership can't gate # sign-in. Admin approval stays the real gate (is_active=False); mark the account verified so # the flow still completes on a no-SMTP self-host. See email.email_enabled(). @@ -744,6 +769,8 @@ def _register_account(email: str, password: str) -> None: password_hash=hash_password(password), is_active=False, email_verified=not email_ok, + # Only a supported code is stored, so a forged body can't put junk in preferences. + preferences={"language": lang} if lang in ("en", "hu") else {}, ) db.add(user) try: @@ -775,6 +802,16 @@ def _register_account(email: str, password: str) -> None: email_mod.send_admin_new_request(admins, email) +def issue_verification_link(db: Session, user: User, ttl: timedelta | None = None) -> str: + """A fresh confirmation link for `user`, returned rather than sent — for a mail that carries the + link inside a larger message (the branching approval/invitation mails, E9.5 S6). + + Same token machinery and the same fragment (SB3) as the standalone confirmation mail, so the two + kinds of link cannot drift apart in shape or in how they are consumed.""" + raw = _issue_token(db, user, "verify", ttl or VERIFY_TTL) + return f"{_app_base()}/#verify={raw}" + + def send_verification_link(db: Session, user: User) -> None: """Issue a fresh verification token for `user` and mail them the link. @@ -786,8 +823,11 @@ def send_verification_link(db: Session, user: User) -> None: (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}") + email_mod.send_verify_email( + user.email, + issue_verification_link(db, user), + hours=int(VERIFY_TTL.total_seconds() // 3600), + ) @router.post("/verify") diff --git a/backend/tests/test_admin_pending_access.py b/backend/tests/test_admin_pending_access.py index 615b33be..3f286657 100644 --- a/backend/tests/test_admin_pending_access.py +++ b/backend/tests/test_admin_pending_access.py @@ -44,8 +44,9 @@ def test_a_pre_approved_registrant_is_listed_and_offers_activation(db, make_user def test_an_ordinary_registrant_awaiting_a_decision_is_not_listed_twice(db, make_user, as_user): - # They already have a pending invite with Approve on it in the queue above. Two buttons for one - # person is worse than one. + # They already have a pending invite with Approve on it in the queue above, and their address is + # not allowed YET — the decision is the queue's to own. Caught in UAT on 2026-09-05: an earlier + # filter keyed on "unconfirmed" put every fresh registration in both lists at once. admin = make_user(email="admin@example.com", role="admin") db.add(Invite(email="normal@example.com", status="pending")) db.commit() @@ -54,6 +55,17 @@ def test_an_ordinary_registrant_awaiting_a_decision_is_not_listed_twice(db, make assert "normal@example.com" not in _pending_access(as_user(admin)) +def test_a_fresh_registration_stays_in_the_queue_and_out_of_this_list(db, make_user, as_user): + # The realistic version of the case above: a brand-new sign-up is inactive AND unconfirmed. It + # still belongs only to the queue — the admin's next act is the decision, not a re-send. + admin = make_user(email="admin@example.com", role="admin") + db.add(Invite(email="fresh@example.com", status="pending")) + db.commit() + make_user(email="fresh@example.com", password=_PW, is_active=False, email_verified=False) + + assert "fresh@example.com" not in _pending_access(as_user(admin)) + + def test_activating_lets_them_in_and_is_recorded(db, make_user, as_user): admin = make_user(email="admin@example.com", role="admin") stuck = _stuck_registrant(db, make_user, admin) @@ -116,12 +128,15 @@ def test_an_unconfirmed_account_is_listed_and_can_be_sent_a_fresh_link( monkeypatch.setattr(admin_routes.email_mod, "email_enabled", lambda db=None: True) sent: list[tuple[str, str]] = [] monkeypatch.setattr( - auth_routes.email_mod, "send_verify_email", lambda to, link: sent.append((to, link)) + auth_routes.email_mod, "send_verify_email", lambda to, link, **kw: sent.append((to, link)) ) admin = make_user(email="admin@example.com", role="admin") # Active but unconfirmed — a real state since E9.5 S1 made the two gates order-independent. It is - # not the admin's to decide, but the mail may simply never have arrived (2026-09-03: the first - # message to a fresh Gmail landed in Spam), so it is theirs to help with. + # not the admin's to decide, but the mail may simply never have arrived, so it is theirs to help + # with. The approved invite is what puts them on this screen at all: the list is "the address may + # sign in, this account still can't", so an undecided request stays with the queue that owns it. + db.add(Invite(email="unconfirmed@example.com", status="approved", decided_by=admin.email)) + db.commit() u = make_user(email="unconfirmed@example.com", password=_PW, email_verified=False) assert "unconfirmed@example.com" in _pending_access(as_user(admin)) diff --git a/backend/tests/test_auth_session.py b/backend/tests/test_auth_session.py index 905709eb..68a42286 100644 --- a/backend/tests/test_auth_session.py +++ b/backend/tests/test_auth_session.py @@ -320,7 +320,9 @@ def test_registration_of_a_pre_approved_email_persists_the_account_with_smtp(db, 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_verify_email", lambda to, link, **kw: 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() diff --git a/backend/tests/test_welcome_mail.py b/backend/tests/test_welcome_mail.py new file mode 100644 index 00000000..ca5af938 --- /dev/null +++ b/backend/tests/test_welcome_mail.py @@ -0,0 +1,249 @@ +"""One mail, one click — the branching welcome/invitation mail (E9.5 S6), DB-lane. + +The funnel's common ordering makes a person open TWO messages: one saying their access is approved, +another asking them to confirm their address. The C option carries the confirmation link inside the +mail that is already going out, so the usual case is one mail and one click. + +Also here: the silent no-op that is the most likely explanation for the 2026-09-03 report that a +confirmation mail "never arrived". Re-registering an address that already had an unconfirmed account +sent nothing at all, while answering with the same cheerful "check your inbox". +""" + +from datetime import datetime, timedelta, timezone + +from sqlalchemy import select + +from app.models import AuditLog, AuthToken, Invite, User + +_PW = "correct horse battery" + + +def _capture(monkeypatch, name): + """Record the arguments a given email_mod.send_* was called with.""" + from app import email as email_mod + + calls: list[tuple] = [] + monkeypatch.setattr(email_mod, name, lambda *a, **kw: calls.append((a, kw)) or True) + return calls + + +def _smtp_on(monkeypatch): + from app import email as email_mod + + monkeypatch.setattr(email_mod, "email_enabled", lambda db=None: True) + + +def _approve(client, invite_id): + r = client.post(f"/api/admin/invites/{invite_id}/approve") + assert r.status_code == 200 + return r + + +# --- the approval mail branches ------------------------------------------------------------------ + + +def test_an_unconfirmed_account_gets_the_confirmation_link_in_the_approval_mail( + db, make_user, as_user, monkeypatch +): + _smtp_on(monkeypatch) + sent = _capture(monkeypatch, "send_access_approved") + admin = make_user(email="admin@example.com", role="admin") + inv = Invite(email="waiting@example.com", status="pending") + db.add(inv) + db.commit() + make_user(email="waiting@example.com", password=_PW, email_verified=False, is_active=False) + + _approve(as_user(admin), inv.id) + + (email, link, has_account, hours), _ = sent[0] + assert email == "waiting@example.com" and has_account is True + assert link and "#verify=" in link + # The whole point: this is the ONLY message they now have to open. + assert hours == 24 * 7 + + +def test_the_link_in_that_mail_actually_confirms_the_address(db, make_user, as_user, monkeypatch): + _smtp_on(monkeypatch) + sent = _capture(monkeypatch, "send_access_approved") + admin = make_user(email="admin@example.com", role="admin") + inv = Invite(email="waiting@example.com", status="pending") + db.add(inv) + db.commit() + user = make_user( + email="waiting@example.com", password=_PW, email_verified=False, is_active=False + ) + _approve(as_user(admin), inv.id) + token = sent[0][0][1].split("#verify=")[1] + + r = as_user(admin).post("/auth/verify", json={"token": token}) + + assert r.status_code == 200 + db.expire_all() + assert db.get(User, user.id).email_verified is True + + +def test_that_link_outlives_a_weekend(db, make_user, as_user, monkeypatch): + # An approval is not a mail anyone sits waiting for; the standard 24h would routinely expire + # before it is read, and the one-mail promise with it. + _smtp_on(monkeypatch) + _capture(monkeypatch, "send_access_approved") + admin = make_user(email="admin@example.com", role="admin") + inv = Invite(email="waiting@example.com", status="pending") + db.add(inv) + db.commit() + user = make_user( + email="waiting@example.com", password=_PW, email_verified=False, is_active=False + ) + + _approve(as_user(admin), inv.id) + + tok = db.execute(select(AuthToken).where(AuthToken.user_id == user.id)).scalars().all()[-1] + assert tok.expires_at > datetime.now(timezone.utc) + timedelta(days=6) + + +def test_an_already_confirmed_account_is_just_told_to_sign_in(db, make_user, as_user, monkeypatch): + _smtp_on(monkeypatch) + sent = _capture(monkeypatch, "send_access_approved") + admin = make_user(email="admin@example.com", role="admin") + inv = Invite(email="ready@example.com", status="pending") + db.add(inv) + db.commit() + make_user(email="ready@example.com", password=_PW, is_active=False) + + _approve(as_user(admin), inv.id) + + (_, link, has_account, _), _ = sent[0] + assert link is None and has_account is True # no link to offer, and none needed + + +def test_someone_with_no_account_yet_is_told_a_second_mail_is_coming( + db, make_user, as_user, monkeypatch +): + _smtp_on(monkeypatch) + sent = _capture(monkeypatch, "send_access_approved") + admin = make_user(email="admin@example.com", role="admin") + inv = Invite(email="stranger@example.com", status="pending") + db.add(inv) + db.commit() + + _approve(as_user(admin), inv.id) + + (_, link, has_account, _), _ = sent[0] + # Nothing to link to yet. Saying a confirmation mail follows is what makes its ABSENCE + # noticeable — the failure that went unnoticed on 2026-09-03. + assert link is None and has_account is False + + +# --- the deliberate invitation -------------------------------------------------------------------- + + +def test_adding_an_address_to_the_whitelist_mails_nobody(db, make_user, as_user, monkeypatch): + _smtp_on(monkeypatch) + approved = _capture(monkeypatch, "send_access_approved") + invited = _capture(monkeypatch, "send_access_invitation") + admin = make_user(email="admin@example.com", role="admin") + + r = as_user(admin).post("/api/admin/invites", json={"email": "maybe@example.com"}) + + # Whitelisting opens a door as a possibility — often already agreed in person — and an address + # can sit approved for weeks. A mail fired automatically here would be nudging someone who never + # asked to be nudged. + assert r.status_code == 200 and r.json()["invited_at"] is None + assert approved == [] and invited == [] + + +def test_the_admin_can_then_invite_them_on_purpose(db, make_user, as_user, monkeypatch): + _smtp_on(monkeypatch) + invited = _capture(monkeypatch, "send_access_invitation") + admin = make_user(email="admin@example.com", role="admin") + inv = Invite(email="maybe@example.com", status="approved", decided_by=admin.email) + db.add(inv) + db.commit() + + r = as_user(admin).post(f"/api/admin/invites/{inv.id}/invite") + + assert r.status_code == 200 and r.json()["invited_at"] is not None + assert invited[0][0][0] == "maybe@example.com" + # Recorded on the row, so "did I already write to them?" has an answer where the decision is + # made — the wrong answer means mailing a real person twice. + db.expire_all() + assert db.get(Invite, inv.id).invited_at is not None + assert "invite_send" in db.execute(select(AuditLog.action)).scalars().all() + + +def test_an_undecided_request_cannot_be_invited(db, make_user, as_user, monkeypatch): + _smtp_on(monkeypatch) + admin = make_user(email="admin@example.com", role="admin") + inv = Invite(email="asking@example.com", status="pending") + db.add(inv) + db.commit() + + r = as_user(admin).post(f"/api/admin/invites/{inv.id}/invite") + + assert r.status_code == 409 and r.json()["detail"]["reason"] == "not_approved" + + +def test_the_invitation_says_so_when_the_instance_cannot_send_mail( + db, make_user, as_user, monkeypatch +): + from app import email as email_mod + + monkeypatch.setattr(email_mod, "email_enabled", lambda db=None: False) + admin = make_user(email="admin@example.com", role="admin") + inv = Invite(email="maybe@example.com", status="approved", decided_by=admin.email) + db.add(inv) + db.commit() + + r = as_user(admin).post(f"/api/admin/invites/{inv.id}/invite") + + assert r.status_code == 409 and r.json()["detail"]["reason"] == "email_off" + db.expire_all() + assert db.get(Invite, inv.id).invited_at is None # nothing sent, nothing recorded + + +# --- the silent register no-op -------------------------------------------------------------------- + + +def test_registering_again_re_sends_the_link_instead_of_sending_nothing(db, client, monkeypatch): + _smtp_on(monkeypatch) + sent = _capture(monkeypatch, "send_verify_email") + assert ( + client.post( + "/auth/register", json={"email": "twice@example.com", "password": _PW} + ).status_code + == 200 + ) + assert len(sent) == 1 + + r = client.post("/auth/register", json={"email": "twice@example.com", "password": _PW}) + + # The old behaviour was to return in silence: same cheerful "check your inbox", no mail. That is + # the most likely explanation for the 2026-09-03 confirmation mail that "never arrived" — none + # was ever sent. The response stays uniform; only the dead end is gone. + assert r.status_code == 200 and r.json() == {"status": "ok"} + assert len(sent) == 2 + assert sent[1][0][0] == "twice@example.com" + + +def test_registering_again_does_not_create_a_second_account(db, client, monkeypatch): + _smtp_on(monkeypatch) + _capture(monkeypatch, "send_verify_email") + for _ in range(2): + client.post("/auth/register", json={"email": "twice@example.com", "password": _PW}) + + db.expire_all() + rows = db.execute(select(User).where(User.email == "twice@example.com")).scalars().all() + assert len(rows) == 1 + + +def test_a_confirmed_account_still_gets_nothing(db, client, make_user, monkeypatch): + _smtp_on(monkeypatch) + sent = _capture(monkeypatch, "send_verify_email") + make_user(email="done@example.com", password=_PW) # already verified + + r = client.post("/auth/register", json={"email": "done@example.com", "password": _PW}) + + # There is nothing to confirm, so there is nothing to send — and mailing "confirm your address" + # to someone who already did would be a lie. + assert r.status_code == 200 + assert sent == [] diff --git a/frontend/src/components/AdminUsers.tsx b/frontend/src/components/AdminUsers.tsx index 0b42f9ff..d02c0fc1 100644 --- a/frontend/src/components/AdminUsers.tsx +++ b/frontend/src/components/AdminUsers.tsx @@ -7,6 +7,7 @@ import { Check, Clock, MailWarning, + Mails, RotateCcw, Send, Shield, @@ -291,6 +292,12 @@ function UsersRoles({ me }: { me: Me }) { ); } +/** Did the invitation fail because this instance has no working SMTP? */ +function inviteEmailOff(err: HttpError): boolean { + const d = err.detailData as { code?: string; reason?: string } | null | undefined; + return d?.code === "invite_send" && d.reason === "email_off"; +} + // --- Sign-ups that are still outside (roadmap UX-25) -------------------------------------------- // // The gap this closes: approve an address BEFORE the person registers and their sign-up produces no @@ -528,6 +535,26 @@ function AdminInvites() { }); if (ok) remove.mutate(i.id); }; + // Sending is its OWN act, never a side effect of approving an address: the operator opens a door + // as a possibility (often already agreed in person), and a mail fired automatically would be + // nudging someone who never asked. `invited_at` comes back on the row so a second click is a + // decision rather than an accident. + const invite = useMutation({ + mutationFn: (i: Invite) => api.sendInvitation(i.id), + onSuccess: (_r, i) => { + refresh(); + notify({ level: "success", message: t("settings.invites.invited", { email: i.email }) }); + }, + onError: (err) => + notify({ + level: "error", + message: + err instanceof HttpError && inviteEmailOff(err) + ? t("settings.invites.inviteEmailOff") + : t("settings.invites.inviteFailed"), + }), + }); + const add = useMutation({ mutationFn: (email: string) => api.addInvite(email), onSuccess: (_d, email) => { @@ -604,6 +631,35 @@ function AdminInvites() { {i.status} + {/* Only an approved row with NOBODY behind it yet: this button means "the door is + open, come in", which is meaningless to someone already inside — and half the + whitelist is existing users. An account that exists but is still stuck has its + own surface above, with a re-send. The label carries the answer to "did I already + write to them?", the question whose wrong answer means mailing a person twice. */} + {i.status === "approved" && !i.has_account && ( + + + + )} {/* An entry with an account behind it stays read-only: deleting it would revoke that user's access, since is_allowed reads this row. Delete the user instead. */} {i.has_account ? ( diff --git a/frontend/src/components/Welcome.tsx b/frontend/src/components/Welcome.tsx index 203ab53c..dcbef1ca 100644 --- a/frontend/src/components/Welcome.tsx +++ b/frontend/src/components/Welcome.tsx @@ -657,6 +657,18 @@ function AuthCard({ verify }: { verify: VerifyEntry }) { + {/* A confirmation mail can be slow, filtered, or — before E9.5 S6 — never sent at all for a + repeat sign-up. Offered right under the form rather than below the mode switches: it is + the second thing a stuck person needs, after the password they just tried. */} + {mode === "signin" && ( + + )} + {/* Mode switches */}
{mode === "signin" ? ( @@ -695,14 +707,6 @@ function AuthCard({ verify }: { verify: VerifyEntry }) { {mode === "signin" && ( <> - {/* A verification email can be lost or never arrive, so offer a resend path from - sign-in too — not only from the expired-link banner (needs a broken link to show). */} -
{t("welcome.auth.or")} diff --git a/frontend/src/components/adminInvites.dom.test.tsx b/frontend/src/components/adminInvites.dom.test.tsx index ebc7c9e8..71e009ee 100644 --- a/frontend/src/components/adminInvites.dom.test.tsx +++ b/frontend/src/components/adminInvites.dom.test.tsx @@ -55,6 +55,7 @@ vi.mock("./Tabs", () => ({ const listMock = vi.fn(); const deleteMock = vi.fn(); +const inviteMock = vi.fn(); vi.mock("../lib/api", async (importOriginal) => { const actual = await importOriginal(); return { @@ -63,6 +64,7 @@ vi.mock("../lib/api", async (importOriginal) => { ...actual.api, adminInvites: () => listMock(), deleteInvite: (id: number) => deleteMock(id), + sendInvitation: (id: number) => inviteMock(id), // The access tab also renders the stuck-sign-ups section (E9.5 S5); nothing stuck here, so it // renders nothing and this file stays about the delete affordance. adminPendingAccess: () => Promise.resolve([]), @@ -83,6 +85,7 @@ const invite = (over: Partial> = {}) => ({ decided_by: null, has_account: false, still_allowed_by_env: false, + invited_at: null, ...over, }); @@ -100,6 +103,9 @@ beforeEach(() => { notifyMock.mockClear(); deleteMock.mockReset().mockResolvedValue({ deleted: 1, still_allowed_by_env: false }); listMock.mockReset().mockResolvedValue([invite()]); + inviteMock + .mockReset() + .mockResolvedValue(invite({ status: "approved", invited_at: "2026-09-05T09:00:00Z" })); meMock.email_configured = true; }); afterEach(cleanup); @@ -223,3 +229,60 @@ describe("Access requests — the delete affordance", () => { expect(await screen.findByText("settings.invites.emailOff")).toBeTruthy(); }); }); + +// Whitelisting an address is opening a door as a possibility, not knocking on one — the operator has +// usually already agreed it with the person, and a row can sit approved for weeks. So adding stays +// silent and telling them is its own act, on its own button (E9.5 S6). +describe("inviting someone the admin whitelisted", () => { + // The decided rows live behind a
; await the summary rather than the section title, or + // the click races the invite query that populates it. + const open = async () => { + renderPanel(); + fireEvent.click(await screen.findByText(/settings.invites.decided/)); + }; + + it("offers a send button on an approved row", async () => { + listMock.mockResolvedValue([invite({ status: "approved" })]); + await open(); + + fireEvent.click(screen.getByLabelText("settings.invites.inviteLabel:asker@example.com")); + + await waitFor(() => expect(inviteMock).toHaveBeenCalledWith(1)); + expect(notifyMock).toHaveBeenCalledWith( + expect.objectContaining({ message: "settings.invites.invited:asker@example.com" }), + ); + }); + + it("offers none on a denied row, which would contradict the decision", async () => { + listMock.mockResolvedValue([invite({ status: "denied" })]); + await open(); + + expect(screen.queryByLabelText("settings.invites.inviteLabel:asker@example.com")).toBeNull(); + }); + + it("offers none once somebody is already behind that address", async () => { + // "The door is open, come in" is meaningless to someone already inside, and most of a mature + // whitelist is existing users. An account that exists but is stuck has its own row above. + listMock.mockResolvedValue([invite({ status: "approved", has_account: true })]); + await open(); + + expect(screen.queryByLabelText("settings.invites.inviteLabel:asker@example.com")).toBeNull(); + }); + + it("says the mail was never sent when the instance has no SMTP", async () => { + listMock.mockResolvedValue([invite({ status: "approved" })]); + inviteMock.mockRejectedValue( + new HttpError(409, "ignored", { code: "invite_send", reason: "email_off" }), + ); + await open(); + + fireEvent.click(screen.getByLabelText("settings.invites.inviteLabel:asker@example.com")); + + // A button the admin pressed must not answer with a success toast for a mail nobody received. + await waitFor(() => + expect(notifyMock).toHaveBeenCalledWith( + expect.objectContaining({ message: "settings.invites.inviteEmailOff" }), + ), + ); + }); +}); diff --git a/frontend/src/i18n/locales/en/auditActions.json b/frontend/src/i18n/locales/en/auditActions.json index 942ca170..c569c299 100644 --- a/frontend/src/i18n/locales/en/auditActions.json +++ b/frontend/src/i18n/locales/en/auditActions.json @@ -22,5 +22,6 @@ "audit_clear": "Audit log cleared", "download_credential_set": "Download cookies set", "download_credential_delete": "Download cookies removed", - "invite_delete": "Whitelist entry deleted" + "invite_delete": "Whitelist entry deleted", + "invite_send": "Invitation sent" } diff --git a/frontend/src/i18n/locales/en/settings.json b/frontend/src/i18n/locales/en/settings.json index 8bfc564e..797332f2 100644 --- a/frontend/src/i18n/locales/en/settings.json +++ b/frontend/src/i18n/locales/en/settings.json @@ -157,6 +157,12 @@ "approveFailed": "Approve failed", "denyFailed": "Deny failed", "addedToWhitelist": "{{email}} added to the whitelist", + "inviteHint": "Send them an invitation. Adding an address never mails anyone — this does.", + "invitedAt": "Invited {{time}}. Sending again will mail them a second time.", + "inviteLabel": "Send an invitation to {{email}}", + "invited": "Invitation sent to {{email}}.", + "inviteFailed": "Couldn't send that invitation.", + "inviteEmailOff": "No mail was sent: this instance has no working SMTP setup. Check Configuration → Email.", "addFailed": "Couldn't add that email", "requested": "requested {{time}}", "approveHint": "Approve — whitelist this email and email them they're in.", diff --git a/frontend/src/i18n/locales/en/welcome.json b/frontend/src/i18n/locales/en/welcome.json index 15c3686f..0dfecd5b 100644 --- a/frontend/src/i18n/locales/en/welcome.json +++ b/frontend/src/i18n/locales/en/welcome.json @@ -82,8 +82,8 @@ "demoEmail": "Demo email", "demoEnter": "Enter", "demoDenied": "That email isn't enabled for the demo. Ask the admin to add it.", - "registerDone": "Almost there — check your email for a verification link. After you confirm it, an admin approves your account before you can sign in.", - "forgotDone": "If that email has an account, we've sent a reset link. Check your inbox.", + "registerDone": "Almost there — check your email for a confirmation link, and your spam folder if it isn't there in a minute. After you confirm it, an admin approves your account before you can sign in.", + "forgotDone": "If that email has an account, we've sent a reset link. Check your inbox, and your spam folder if it hasn't arrived in a minute.", "resetDone": "Your password has been set — you can sign in now.", "deadLink": { "title": "This link is no longer valid", @@ -96,7 +96,7 @@ "verifyOkResetCta": "Set a new password", "verifyInvalid": "That verification link is invalid or has expired.", "resendCta": "Send a new link", - "resendDone": "If that email has an unverified account, we've sent a new verification link. Check your inbox.", + "resendDone": "If that email has an unconfirmed account, we've sent a new confirmation link. Check your inbox, and your spam folder if it hasn't arrived in a minute.", "oauthCancelled": "Google sign-in was cancelled. You can try again or use email and password.", "accessRequested": "Thanks — we recorded your request. An admin will review it.", "accessRequestedFor": "We've recorded your access request for:", diff --git a/frontend/src/i18n/locales/hu/auditActions.json b/frontend/src/i18n/locales/hu/auditActions.json index f6c8eb62..636e8a53 100644 --- a/frontend/src/i18n/locales/hu/auditActions.json +++ b/frontend/src/i18n/locales/hu/auditActions.json @@ -22,5 +22,6 @@ "audit_clear": "Napló ürítve", "download_credential_set": "Letöltési cookie-k beállítva", "download_credential_delete": "Letöltési cookie-k eltávolítva", - "invite_delete": "Whitelista-bejegyzés törölve" + "invite_delete": "Whitelista-bejegyzés törölve", + "invite_send": "Meghívó elküldve" } diff --git a/frontend/src/i18n/locales/hu/settings.json b/frontend/src/i18n/locales/hu/settings.json index 72adf40e..eab22b25 100644 --- a/frontend/src/i18n/locales/hu/settings.json +++ b/frontend/src/i18n/locales/hu/settings.json @@ -157,6 +157,12 @@ "approveFailed": "A jóváhagyás sikertelen", "denyFailed": "Az elutasítás sikertelen", "addedToWhitelist": "{{email}} hozzáadva a fehérlistához", + "inviteHint": "Küldj neki meghívót. A cím hozzáadása soha nem küld levelet — ez igen.", + "invitedAt": "Meghívva: {{time}}. Az újraküldés másodszor is levelet küld neki.", + "inviteLabel": "Meghívó küldése ide: {{email}}", + "invited": "Meghívó elküldve ide: {{email}}.", + "inviteFailed": "Nem sikerült elküldeni a meghívót.", + "inviteEmailOff": "Nem ment ki levél: ezen a példányon nincs működő SMTP. Nézd meg a Beállítások → E-mail részt.", "addFailed": "Nem sikerült hozzáadni ezt az e-mailt", "requested": "kérve: {{time}}", "approveHint": "Jóváhagyás — fehérlistára teszi ezt az e-mailt és e-mailben értesíti őket, hogy bekerültek.", diff --git a/frontend/src/i18n/locales/hu/welcome.json b/frontend/src/i18n/locales/hu/welcome.json index fd3cd8e1..2bb1f998 100644 --- a/frontend/src/i18n/locales/hu/welcome.json +++ b/frontend/src/i18n/locales/hu/welcome.json @@ -82,8 +82,8 @@ "demoEmail": "Demó e-mail", "demoEnter": "Belépés", "demoDenied": "Ez az e-mail nincs engedélyezve a demóhoz. Kérd az admint, hogy adja hozzá.", - "registerDone": "Majdnem kész — nézd meg a postafiókod a megerősítő linkért. Miután megerősítetted, egy admin jóváhagyja a fiókodat, mielőtt beléphetnél.", - "forgotDone": "Ha ehhez az e-mailhez tartozik fiók, elküldtük a visszaállító linket. Nézd meg a postafiókod.", + "registerDone": "Majdnem kész — nézd meg a postafiókod a megerősítő linkért, és a Spam mappát is, ha egy percen belül nem jön meg. Miután megerősítetted, egy admin jóváhagyja a fiókodat, mielőtt beléphetnél.", + "forgotDone": "Ha ehhez az e-mailhez tartozik fiók, elküldtük a visszaállító linket. Nézd meg a postafiókod, és a Spam mappát is, ha egy percen belül nem érkezik meg.", "resetDone": "A jelszavad be van állítva — most már beléphetsz.", "deadLink": { "title": "Ez a link már nem érvényes", @@ -96,7 +96,7 @@ "verifyOkResetCta": "Új jelszó beállítása", "verifyInvalid": "Ez a megerősítő link érvénytelen vagy lejárt.", "resendCta": "Új link küldése", - "resendDone": "Ha ehhez az e-mailhez tartozik megerősítetlen fiók, elküldtünk egy új megerősítő linket. Nézd meg a postafiókod.", + "resendDone": "Ha ehhez az e-mailhez tartozik megerősítetlen fiók, elküldtünk egy új megerősítő linket. Nézd meg a postafiókod, és a Spam mappát is, ha egy percen belül nem érkezik meg.", "oauthCancelled": "A Google-belépés megszakadt. Próbáld újra, vagy lépj be e-maillel és jelszóval.", "accessRequested": "Köszönjük — rögzítettük a kérésed. Egy admin felülvizsgálja.", "accessRequestedFor": "Rögzítettük a hozzáférési kérelmedet erre a címre:", diff --git a/frontend/src/lib/api/admin.ts b/frontend/src/lib/api/admin.ts index c2282737..a4dfd5a2 100644 --- a/frontend/src/lib/api/admin.ts +++ b/frontend/src/lib/api/admin.ts @@ -25,6 +25,10 @@ export interface Invite { /** The address stays allow-listed by the server's env ALLOWED_EMAILS / ADMIN_EMAILS, so removing * this row would NOT take access away. The delete confirmation says so. */ still_allowed_by_env: boolean; + /** When the admin last invited this person, or null if never. Adding an address to the whitelist + * is deliberately silent — it opens a door rather than knocking — so telling them is a separate + * act, and this is how the admin sees they have already done it. */ + invited_at: string | null; } /** A sign-up that is still outside, as the admin screen shows it. Same shape as a user row plus the @@ -193,6 +197,10 @@ export const adminApi = { req(`/api/admin/invites${status ? `?status=${status}` : ""}`), approveInvite: (id: number) => req(`/api/admin/invites/${id}/approve`, { method: "POST" }), denyInvite: (id: number) => req(`/api/admin/invites/${id}/deny`, { method: "POST" }), + // Tell someone their address is whitelisted. Never automatic: adding the address stays silent, so + // this is the deliberate act that actually reaches a person. + sendInvitation: (id: number): Promise => + req(`/api/admin/invites/${id}/invite`, { method: "POST" }, { quiet: true }), addInvite: (email: string) => req("/api/admin/invites", { method: "POST", body: JSON.stringify({ email }) }), /** `still_allowed_by_env` is true when the address stays allow-listed by the server's env