fix: mail delivery never fails requests after commit
This commit is contained in:
parent
62d01b5097
commit
6e12720ad6
3 changed files with 62 additions and 12 deletions
|
|
@ -15,9 +15,21 @@ def magic_link(token: str) -> str:
|
|||
return f"{settings.frontend_url.rstrip('/')}/login?token={token}"
|
||||
|
||||
|
||||
def _send(message: EmailMessage) -> None:
|
||||
with smtplib.SMTP(settings.smtp_host, settings.smtp_port, timeout=10) as smtp:
|
||||
smtp.starttls()
|
||||
if settings.smtp_username:
|
||||
smtp.login(settings.smtp_username, settings.smtp_password)
|
||||
smtp.send_message(message)
|
||||
|
||||
|
||||
def send_magic_link(to_email: str, link: str) -> bool:
|
||||
"""Mail the login link. Returns False (logs instead) when SMTP is unset.
|
||||
|
||||
Never raises: callers invoke it after committing (approve/bootstrap),
|
||||
so a delivery failure must not turn into a 500 — the token stays valid
|
||||
and the admin sees ``email_sent: false``.
|
||||
|
||||
The plaintext link only ever appears here: service/api logging must
|
||||
never include tokens or session ids (see test_auth no-leak test).
|
||||
"""
|
||||
|
|
@ -33,11 +45,11 @@ def send_magic_link(to_email: str, link: str) -> bool:
|
|||
message.set_content(
|
||||
f"Hello,\n\nUse this link to log in (valid 15 minutes, single use):\n{link}\n"
|
||||
)
|
||||
with smtplib.SMTP(settings.smtp_host, settings.smtp_port, timeout=10) as smtp:
|
||||
smtp.starttls()
|
||||
if settings.smtp_username:
|
||||
smtp.login(settings.smtp_username, settings.smtp_password)
|
||||
smtp.send_message(message)
|
||||
try:
|
||||
_send(message)
|
||||
except Exception: # noqa: BLE001 - delivery is best effort, see docstring
|
||||
logger.warning("magic-link delivery failed for %s", to_email)
|
||||
return False
|
||||
logger.info("magic link mailed to %s", to_email)
|
||||
return True
|
||||
|
||||
|
|
@ -55,12 +67,8 @@ def send_access_notification(request_id: str, name: str, email: str) -> bool:
|
|||
f"Name: {name}\nEmail: {email}\nRequest: {request_id}\n"
|
||||
)
|
||||
try:
|
||||
with smtplib.SMTP(settings.smtp_host, settings.smtp_port, timeout=10) as smtp:
|
||||
smtp.starttls()
|
||||
if settings.smtp_username:
|
||||
smtp.login(settings.smtp_username, settings.smtp_password)
|
||||
smtp.send_message(message)
|
||||
except OSError:
|
||||
_send(message)
|
||||
except Exception: # noqa: BLE001 - best effort, request is already stored
|
||||
logger.warning("access-request notification failed for %s", request_id)
|
||||
return False
|
||||
return True
|
||||
|
|
|
|||
|
|
@ -226,3 +226,45 @@ def test_resolve_tenant():
|
|||
user = User(email="a@b.c", tenant_id="parish-7")
|
||||
assert resolve_tenant(user, None) == "parish-7"
|
||||
assert resolve_tenant(user, "other") == "parish-7"
|
||||
|
||||
|
||||
def _broken_smtp(monkeypatch):
|
||||
import smtplib
|
||||
|
||||
from app.auth import mail
|
||||
|
||||
class ExplodingSMTP:
|
||||
def __init__(self, *args, **kwargs):
|
||||
raise smtplib.SMTPException("relay down")
|
||||
|
||||
monkeypatch.setattr(mail.smtplib, "SMTP", ExplodingSMTP)
|
||||
|
||||
|
||||
def test_magic_link_delivery_failure_returns_false(monkeypatch):
|
||||
from app.auth import mail
|
||||
|
||||
monkeypatch.setattr(settings, "smtp_host", "dummy")
|
||||
_broken_smtp(monkeypatch)
|
||||
assert mail.send_magic_link("a@example.com", "http://x/login?token=t") is False
|
||||
assert (
|
||||
mail.send_access_notification("req-1", "A", "a@example.com") is False
|
||||
)
|
||||
|
||||
|
||||
async def test_approve_survives_delivery_failure(client, session, monkeypatch):
|
||||
monkeypatch.setattr(settings, "smtp_host", "dummy")
|
||||
_broken_smtp(monkeypatch)
|
||||
await client.post("/api/auth/admin-bootstrap", json={"email": ADMIN})
|
||||
await _login(client, session, ADMIN, role="admin")
|
||||
created = await client.post(
|
||||
"/api/access/requests",
|
||||
json={"name": "M", "email": "m@example.com", "reason": ""},
|
||||
)
|
||||
assert created.status_code == 201, created.text
|
||||
approved = await client.post(
|
||||
f"/api/access/requests/{created.json()['id']}/approve"
|
||||
)
|
||||
assert approved.status_code == 200, approved.text
|
||||
body = approved.json()
|
||||
assert body["request"]["status"] == "approved"
|
||||
assert body["email_sent"] is False
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue