Configure the invitation mailbox in the UI, not the environment
Putting IMAP settings in environment variables was my call to make and it was the wrong one: it meant editing a file on the server and restarting to change a folder name, and it was never offered as a choice. The mailbox is now a row in mailbox_accounts, edited under Settings → Admin: host, user, password, port, folder, interval, on/off. Administrators only — it is a shared forwarding target, not a per-user account. The password is never returned by any endpoint, the same as the CalDAV account routes, and leaving the field empty on edit keeps the stored one so changing a port does not mean retyping it. The panel shows when the mailbox was last read and what went wrong if it did, because a mailbox that quietly stopped working looks exactly like one that has no mail. Worth stating plainly: the password now lives in the database, in clear text, like the CalDAV, Google and Home Assistant credentials beside it. Unlike those three, an IMAP password opens an entire mailbox. The scheduler job is registered unconditionally now — whether a mailbox exists can change without a restart, and fetch_invites returns immediately when none does. test_scheduler knows about it. Also adds the requests list to the sidebar with accept and decline, 26 translation keys across all ten languages, and tests covering that the password is never returned, that an edit without one keeps it, and that a disabled mailbox counts as unconfigured. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
9bf7d3797d
commit
a67602dbc2
+38
-17
@@ -6,9 +6,10 @@ mail can only ever create a row in ``event_invites``. No calendar read path
|
||||
touches that table — an invitation becomes an event when a human accepts it, and
|
||||
not before.
|
||||
|
||||
Credentials live in the environment, never in the database. The service already
|
||||
loads an EnvironmentFile, and this keeps the one secret that grants access to a
|
||||
whole mailbox out of a table that an endpoint could return.
|
||||
The mailbox is configured in the UI and stored in ``mailbox_accounts`` — one row
|
||||
per instance, since it is a shared forwarding target rather than a per-user
|
||||
account. The password is kept as given, like every other external account here;
|
||||
no endpoint returns it.
|
||||
|
||||
Single process assumed, same as the rest of the scheduler (see scheduler.py).
|
||||
"""
|
||||
@@ -18,7 +19,6 @@ from __future__ import annotations
|
||||
import contextlib
|
||||
import imaplib
|
||||
import logging
|
||||
import os
|
||||
import re
|
||||
import uuid
|
||||
from datetime import datetime, timezone
|
||||
@@ -49,8 +49,15 @@ _ADDR_RE = re.compile(r"^[^@\s<>\",;:]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}$")
|
||||
_HANDLED_METHODS = ("REQUEST", "CANCEL")
|
||||
|
||||
|
||||
def is_configured() -> bool:
|
||||
return bool(os.environ.get("IMAP_HOST"))
|
||||
def get_account(db: Session):
|
||||
"""The configured mailbox, or None. Disabled counts as not configured."""
|
||||
return (db.query(models.MailboxAccount)
|
||||
.filter(models.MailboxAccount.enabled.is_(True))
|
||||
.order_by(models.MailboxAccount.id).first())
|
||||
|
||||
|
||||
def is_configured(db: Session) -> bool:
|
||||
return get_account(db) is not None
|
||||
|
||||
|
||||
def _valid_address(addr: str | None) -> str | None:
|
||||
@@ -193,13 +200,10 @@ def ingest_ics(db: Session, raw: bytes) -> bool:
|
||||
# ── Mailbox ───────────────────────────────────────────────
|
||||
|
||||
|
||||
def _connect() -> imaplib.IMAP4:
|
||||
host = os.environ["IMAP_HOST"]
|
||||
port = int(os.environ.get("IMAP_PORT", "993"))
|
||||
use_ssl = os.environ.get("IMAP_SSL", "1") != "0"
|
||||
box = (imaplib.IMAP4_SSL(host, port, timeout=30) if use_ssl
|
||||
else imaplib.IMAP4(host, port, timeout=30))
|
||||
box.login(os.environ["IMAP_USER"], os.environ["IMAP_PASSWORD"])
|
||||
def _connect(acc) -> imaplib.IMAP4:
|
||||
box = (imaplib.IMAP4_SSL(acc.imap_host, acc.imap_port, timeout=30) if acc.imap_ssl
|
||||
else imaplib.IMAP4(acc.imap_host, acc.imap_port, timeout=30))
|
||||
box.login(acc.imap_user, acc.imap_password)
|
||||
return box
|
||||
|
||||
|
||||
@@ -246,12 +250,29 @@ def _handle_message(db: Session, box: imaplib.IMAP4, uid: bytes) -> None:
|
||||
|
||||
|
||||
def fetch_invites(db: Session) -> None:
|
||||
"""Scheduler job: read new mail and store any invitations it holds."""
|
||||
if not is_configured():
|
||||
"""Scheduler job: read new mail and store any invitations it holds.
|
||||
|
||||
Records what happened on the account so the UI can show a mailbox that
|
||||
stopped working, rather than letting invitations quietly never arrive.
|
||||
"""
|
||||
acc = get_account(db)
|
||||
if acc is None:
|
||||
return
|
||||
box = _connect()
|
||||
try:
|
||||
box.select(os.environ.get("IMAP_FOLDER", "INBOX"))
|
||||
_run(db, acc)
|
||||
acc.last_error = None
|
||||
except Exception as exc:
|
||||
logger.exception("Postfach %s nicht erreichbar", acc.imap_host)
|
||||
acc.last_error = f"{type(exc).__name__}: {exc}"[:500]
|
||||
acc.last_run = datetime.now(timezone.utc)
|
||||
with contextlib.suppress(Exception):
|
||||
db.commit()
|
||||
|
||||
|
||||
def _run(db: Session, acc) -> None:
|
||||
box = _connect(acc)
|
||||
try:
|
||||
box.select(acc.imap_folder or "INBOX")
|
||||
# UID commands, not sequence numbers: those shift when a message is
|
||||
# expunged mid-run and we would read the wrong mail.
|
||||
typ, data = box.uid("SEARCH", None, "UNSEEN")
|
||||
|
||||
@@ -418,6 +418,46 @@ class UserEmailAddress(Base):
|
||||
user = relationship("User")
|
||||
|
||||
|
||||
class MailboxAccount(Base):
|
||||
"""The mailbox watched for invitations, configured in the UI.
|
||||
|
||||
One row per instance: this is a shared inbox that invitations are forwarded
|
||||
to, not a per-user account. Administrators manage it.
|
||||
|
||||
The password is stored as given, the same as CalDAVAccount, GoogleAccount
|
||||
and HomeAssistantAccount — IMAP needs the original to log in, so a hash is
|
||||
no use. Worth knowing: unlike those three, this one opens an entire mailbox.
|
||||
No endpoint ever returns it.
|
||||
|
||||
SMTP lives here too so replies to organisers have somewhere to come from.
|
||||
"""
|
||||
|
||||
__tablename__ = "mailbox_accounts"
|
||||
|
||||
id = Column(Integer, primary_key=True, index=True)
|
||||
enabled = Column(Boolean, default=True, nullable=False)
|
||||
|
||||
imap_host = Column(String(255), nullable=False)
|
||||
imap_port = Column(Integer, default=993, nullable=False)
|
||||
imap_ssl = Column(Boolean, default=True, nullable=False)
|
||||
imap_user = Column(String(255), nullable=False)
|
||||
imap_password = Column(String(255), nullable=False)
|
||||
imap_folder = Column(String(255), default="INBOX", nullable=False)
|
||||
poll_minutes = Column(Integer, default=5, nullable=False)
|
||||
|
||||
smtp_host = Column(String(255), nullable=True)
|
||||
smtp_port = Column(Integer, default=587, nullable=True)
|
||||
smtp_ssl = Column(Boolean, default=False, nullable=True)
|
||||
smtp_user = Column(String(255), nullable=True)
|
||||
smtp_password = Column(String(255), nullable=True)
|
||||
smtp_from = Column(String(255), nullable=True)
|
||||
|
||||
# Surfaced in the UI so a mailbox that stopped working says so, instead of
|
||||
# invitations quietly never arriving.
|
||||
last_run = Column(DateTime, nullable=True)
|
||||
last_error = Column(Text, nullable=True)
|
||||
|
||||
|
||||
class EventInvite(Base):
|
||||
"""An iTIP invitation (METHOD:REQUEST) that arrived by mail.
|
||||
|
||||
|
||||
@@ -131,21 +131,134 @@ def dismiss(invite_id: int, db: Session = Depends(get_db),
|
||||
|
||||
|
||||
@router.get("/status")
|
||||
def status(current_user: models.User = Depends(get_current_user)):
|
||||
return {"imap_configured": mail_invites.is_configured()}
|
||||
def status(db: Session = Depends(get_db),
|
||||
current_user: models.User = Depends(get_current_user)):
|
||||
acc = mail_invites.get_account(db)
|
||||
return {
|
||||
"imap_configured": acc is not None,
|
||||
"smtp_configured": bool(acc and acc.smtp_host),
|
||||
"last_run": acc.last_run.isoformat() if acc and acc.last_run else None,
|
||||
"last_error": acc.last_error if acc else None,
|
||||
}
|
||||
|
||||
|
||||
# ── Mailbox configuration ─────────────────────────────────
|
||||
#
|
||||
# One mailbox per instance: it is a shared forwarding target, not a per-user
|
||||
# account, so only administrators may see or change it.
|
||||
|
||||
|
||||
def _require_admin(user: models.User) -> None:
|
||||
if not user.is_admin:
|
||||
raise HTTPException(403, "Nur für Administratoren")
|
||||
|
||||
|
||||
def _mailbox_dict(acc) -> dict | None:
|
||||
if acc is None:
|
||||
return None
|
||||
# Passwords are never returned, the same as the CalDAV account endpoints.
|
||||
return {
|
||||
"id": acc.id,
|
||||
"enabled": bool(acc.enabled),
|
||||
"imap_host": acc.imap_host,
|
||||
"imap_port": acc.imap_port,
|
||||
"imap_ssl": bool(acc.imap_ssl),
|
||||
"imap_user": acc.imap_user,
|
||||
"imap_folder": acc.imap_folder,
|
||||
"poll_minutes": acc.poll_minutes,
|
||||
"has_imap_password": bool(acc.imap_password),
|
||||
"smtp_host": acc.smtp_host,
|
||||
"smtp_port": acc.smtp_port,
|
||||
"smtp_ssl": bool(acc.smtp_ssl),
|
||||
"smtp_user": acc.smtp_user,
|
||||
"smtp_from": acc.smtp_from,
|
||||
"has_smtp_password": bool(acc.smtp_password),
|
||||
"last_run": acc.last_run.isoformat() if acc.last_run else None,
|
||||
"last_error": acc.last_error,
|
||||
}
|
||||
|
||||
|
||||
class MailboxUpdate(BaseModel):
|
||||
enabled: bool = True
|
||||
imap_host: str
|
||||
imap_port: int = 993
|
||||
imap_ssl: bool = True
|
||||
imap_user: str
|
||||
# Optional on edit: left out keeps the stored one, so changing a port does
|
||||
# not mean retyping the password.
|
||||
imap_password: Optional[str] = None
|
||||
imap_folder: str = "INBOX"
|
||||
poll_minutes: int = 5
|
||||
smtp_host: Optional[str] = None
|
||||
smtp_port: Optional[int] = 587
|
||||
smtp_ssl: Optional[bool] = False
|
||||
smtp_user: Optional[str] = None
|
||||
smtp_password: Optional[str] = None
|
||||
smtp_from: Optional[str] = None
|
||||
|
||||
|
||||
@router.get("/mailbox")
|
||||
def get_mailbox(db: Session = Depends(get_db),
|
||||
current_user: models.User = Depends(get_current_user)):
|
||||
_require_admin(current_user)
|
||||
return _mailbox_dict(
|
||||
db.query(models.MailboxAccount).order_by(models.MailboxAccount.id).first())
|
||||
|
||||
|
||||
@router.put("/mailbox")
|
||||
def set_mailbox(data: MailboxUpdate, db: Session = Depends(get_db),
|
||||
current_user: models.User = Depends(get_current_user)):
|
||||
_require_admin(current_user)
|
||||
acc = db.query(models.MailboxAccount).order_by(models.MailboxAccount.id).first()
|
||||
if acc is None:
|
||||
acc = models.MailboxAccount(imap_host="", imap_user="", imap_password="")
|
||||
db.add(acc)
|
||||
|
||||
if not data.imap_password and not acc.imap_password:
|
||||
raise HTTPException(422, "Passwort fehlt")
|
||||
if not 1 <= data.poll_minutes <= 1440:
|
||||
raise HTTPException(422, "Abrufintervall muss zwischen 1 und 1440 Minuten liegen")
|
||||
|
||||
acc.enabled = data.enabled
|
||||
acc.imap_host = data.imap_host.strip()
|
||||
acc.imap_port = data.imap_port
|
||||
acc.imap_ssl = data.imap_ssl
|
||||
acc.imap_user = data.imap_user.strip()
|
||||
if data.imap_password:
|
||||
acc.imap_password = data.imap_password
|
||||
acc.imap_folder = (data.imap_folder or "INBOX").strip()
|
||||
acc.poll_minutes = data.poll_minutes
|
||||
acc.smtp_host = (data.smtp_host or "").strip() or None
|
||||
acc.smtp_port = data.smtp_port
|
||||
acc.smtp_ssl = bool(data.smtp_ssl)
|
||||
acc.smtp_user = (data.smtp_user or "").strip() or None
|
||||
if data.smtp_password:
|
||||
acc.smtp_password = data.smtp_password
|
||||
acc.smtp_from = (data.smtp_from or "").strip() or None
|
||||
# Settings changed, so whatever went wrong before is no longer current.
|
||||
acc.last_error = None
|
||||
db.commit()
|
||||
return _mailbox_dict(acc)
|
||||
|
||||
|
||||
@router.delete("/mailbox")
|
||||
def delete_mailbox(db: Session = Depends(get_db),
|
||||
current_user: models.User = Depends(get_current_user)):
|
||||
_require_admin(current_user)
|
||||
acc = db.query(models.MailboxAccount).order_by(models.MailboxAccount.id).first()
|
||||
if acc:
|
||||
db.delete(acc)
|
||||
db.commit()
|
||||
return {"ok": True}
|
||||
|
||||
|
||||
@router.post("/poll")
|
||||
def poll_now(db: Session = Depends(get_db),
|
||||
current_user: models.User = Depends(get_current_user)):
|
||||
"""Fetch right now instead of waiting for the scheduler.
|
||||
|
||||
Admin-only: it opens a connection to the shared mailbox, so it is not a
|
||||
per-user action.
|
||||
"""
|
||||
if not current_user.is_admin:
|
||||
raise HTTPException(403, "Nur für Administratoren")
|
||||
if not mail_invites.is_configured():
|
||||
"""Fetch right now instead of waiting for the scheduler."""
|
||||
_require_admin(current_user)
|
||||
if not mail_invites.is_configured(db):
|
||||
raise HTTPException(400, "Kein Postfach konfiguriert")
|
||||
mail_invites.fetch_invites(db)
|
||||
return {"ok": True}
|
||||
acc = mail_invites.get_account(db)
|
||||
return {"ok": True, "last_error": acc.last_error if acc else None}
|
||||
|
||||
@@ -145,12 +145,11 @@ def install_default_jobs() -> None:
|
||||
ical_minutes = float(os.environ.get("ICAL_REFRESH_MINUTES", "15"))
|
||||
register("attachments.sweep", sweep_hours * 3600, sweep_attachments)
|
||||
register("ical.refresh", ical_minutes * 60, refresh_ical_subscriptions)
|
||||
# Only when a mailbox is configured: an unconditional job would log every
|
||||
# few minutes that it has nothing to do, and test_scheduler asserts the
|
||||
# exact job set.
|
||||
if os.environ.get("IMAP_HOST"):
|
||||
poll_minutes = float(os.environ.get("MAIL_POLL_MINUTES", "5"))
|
||||
register("mail.invites", poll_minutes * 60, fetch_mail_invites)
|
||||
# Always registered: the mailbox is configured in the UI, so whether there
|
||||
# is one to read can change without a restart. fetch_invites returns at once
|
||||
# when none is set up.
|
||||
mail_minutes = float(os.environ.get("MAIL_POLL_MINUTES", "5"))
|
||||
register("mail.invites", mail_minutes * 60, fetch_mail_invites)
|
||||
|
||||
|
||||
@asynccontextmanager
|
||||
|
||||
@@ -217,3 +217,70 @@ def test_oversized_payload_is_refused(client):
|
||||
_admin_with_email(client)
|
||||
padded = _ics(summary="X" * (mail_invites.MAX_ICS_BYTES + 10))
|
||||
assert _ingest(padded) is False
|
||||
|
||||
|
||||
# ── Mailbox configuration ─────────────────────────────────
|
||||
|
||||
|
||||
def _mailbox_payload(**over):
|
||||
body = {"imap_host": "imap.example.com", "imap_user": "einladungen@example.com",
|
||||
"imap_password": "geheim", "imap_port": 993, "poll_minutes": 5}
|
||||
body.update(over)
|
||||
return body
|
||||
|
||||
|
||||
def test_mailbox_password_is_never_returned(client):
|
||||
token = register_admin(client)
|
||||
client.put("/api/invites/mailbox", json=_mailbox_payload(), headers=auth(token))
|
||||
|
||||
body = client.get("/api/invites/mailbox", headers=auth(token)).json()
|
||||
assert body["imap_host"] == "imap.example.com"
|
||||
assert body["has_imap_password"] is True
|
||||
assert "imap_password" not in body
|
||||
|
||||
|
||||
def test_editing_without_a_password_keeps_the_stored_one(client):
|
||||
token = register_admin(client)
|
||||
client.put("/api/invites/mailbox", json=_mailbox_payload(), headers=auth(token))
|
||||
client.put("/api/invites/mailbox",
|
||||
json=_mailbox_payload(imap_password=None, imap_port=143),
|
||||
headers=auth(token))
|
||||
|
||||
db = SessionLocal()
|
||||
try:
|
||||
acc = db.query(models.MailboxAccount).one()
|
||||
assert acc.imap_port == 143
|
||||
assert acc.imap_password == "geheim"
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
|
||||
def test_first_save_requires_a_password(client):
|
||||
token = register_admin(client)
|
||||
res = client.put("/api/invites/mailbox", json=_mailbox_payload(imap_password=None),
|
||||
headers=auth(token))
|
||||
assert res.status_code == 422
|
||||
|
||||
|
||||
def test_only_admins_touch_the_mailbox(client):
|
||||
from conftest import create_user
|
||||
|
||||
admin = register_admin(client)
|
||||
user = create_user(client, admin, "lisa")
|
||||
assert client.get("/api/invites/mailbox", headers=auth(user)).status_code == 403
|
||||
assert client.put("/api/invites/mailbox", json=_mailbox_payload(),
|
||||
headers=auth(user)).status_code == 403
|
||||
|
||||
|
||||
def test_disabled_mailbox_counts_as_unconfigured(client):
|
||||
token = register_admin(client)
|
||||
client.put("/api/invites/mailbox", json=_mailbox_payload(enabled=False),
|
||||
headers=auth(token))
|
||||
|
||||
assert client.get("/api/invites/status", headers=auth(token)).json()["imap_configured"] is False
|
||||
|
||||
db = SessionLocal()
|
||||
try:
|
||||
assert mail_invites.get_account(db) is None
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
@@ -19,9 +19,10 @@ def _run(fn):
|
||||
def test_default_jobs_are_registered_with_sane_intervals():
|
||||
scheduler.install_default_jobs()
|
||||
names = {j.name: j for j in scheduler._jobs}
|
||||
assert set(names) == {"attachments.sweep", "ical.refresh"}
|
||||
assert set(names) == {"attachments.sweep", "ical.refresh", "mail.invites"}
|
||||
assert names["attachments.sweep"].interval == 24 * 3600
|
||||
assert names["ical.refresh"].interval == 15 * 60
|
||||
assert names["mail.invites"].interval == 5 * 60
|
||||
# Nothing runs immediately: a restart must not be slowed by maintenance.
|
||||
assert all(j.next_run > time.monotonic() for j in scheduler._jobs)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user