Fix SMTP auth: settings tab saved wrong config keys
The Settings → E-mailrapport tab (scheduler.js) saved the SMTP username as `user` and TLS flag as `starttls`, but every backend reader expects `username`/`use_tls` (routes/email.py). Result: username was always empty, server.login() was skipped, and the SMTP server rejected the send — surfacing as a misleading "authentication failed" message even with a valid App Password. The bug was latent because Graph is preferred whenever M365 is connected, so the SMTP path was rarely exercised. - scheduler.js: send/load canonical keys (username, use_tls). The send-report modal (scan.js) already used these. - _load_smtp_config(): normalise legacy user→username / starttls→use_tls so configs saved before the fix work without re-entry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
b661a94f98
commit
526e2b0b78
@ -878,6 +878,13 @@ def _load_smtp_config() -> dict:
|
|||||||
cfg = json.loads(_SMTP_CONFIG_PATH.read_text(encoding="utf-8"))
|
cfg = json.loads(_SMTP_CONFIG_PATH.read_text(encoding="utf-8"))
|
||||||
if cfg.get("password"):
|
if cfg.get("password"):
|
||||||
cfg["password"] = _decrypt_password(cfg["password"])
|
cfg["password"] = _decrypt_password(cfg["password"])
|
||||||
|
# Normalise legacy key names written by an older settings-tab UI
|
||||||
|
# (`user`/`starttls`) to the canonical keys every reader expects
|
||||||
|
# (`username`/`use_tls`), so configs saved before the fix still work.
|
||||||
|
if "username" not in cfg and "user" in cfg:
|
||||||
|
cfg["username"] = cfg["user"]
|
||||||
|
if "use_tls" not in cfg and "starttls" in cfg:
|
||||||
|
cfg["use_tls"] = cfg["starttls"]
|
||||||
return cfg
|
return cfg
|
||||||
except Exception:
|
except Exception:
|
||||||
pass
|
pass
|
||||||
|
|||||||
@ -68,6 +68,8 @@ Exception hierarchy (all inherit `M365Error(Exception)`):
|
|||||||
- **Graph preferred over SMTP** — `smtp_test` and `send_report` try `_send_email_graph()` first; fall back to SMTP only if Graph raises. If Graph fails and no SMTP host saved, the Graph exception surfaces directly.
|
- **Graph preferred over SMTP** — `smtp_test` and `send_report` try `_send_email_graph()` first; fall back to SMTP only if Graph raises. If Graph fails and no SMTP host saved, the Graph exception surfaces directly.
|
||||||
- **Auto-email after manual scan** — `_maybe_send_auto_email()` in `routes/scan.py` called from the `_run()` thread after `run_scan()` returns. Reads `smtp_cfg.get("auto_email_manual")`; no-ops if false, no flagged items, or no recipients.
|
- **Auto-email after manual scan** — `_maybe_send_auto_email()` in `routes/scan.py` called from the `_run()` thread after `run_scan()` returns. Reads `smtp_cfg.get("auto_email_manual")`; no-ops if false, no flagged items, or no recipients.
|
||||||
- **Gmail vs Google Workspace** — auth error handlers check if SMTP username ends in `@gmail.com`/`@googlemail.com`; custom domains are treated as Google Workspace and error message points to the Workspace admin console.
|
- **Gmail vs Google Workspace** — auth error handlers check if SMTP username ends in `@gmail.com`/`@googlemail.com`; custom domains are treated as Google Workspace and error message points to the Workspace admin console.
|
||||||
|
- **Canonical SMTP config keys are `username` and `use_tls`** — all backend readers (`smtp_test`, `_send_report_email`, `_send_email_graph`) use these. The Settings → E-mailrapport tab (`scheduler.js`) historically saved `user`/`starttls`, which left `username` empty so `server.login()` was skipped and the server rejected the send. Frontend now sends the canonical keys, and `_load_smtp_config()` normalises legacy `user`→`username` / `starttls`→`use_tls` for already-saved configs. The send-report modal (`scan.js`) already used the canonical keys. Keep both UIs and the backend on `username`/`use_tls`.
|
||||||
|
- **Graph 202 ≠ delivered** — `_send_email_graph` returns on Graph's HTTP 202 (queued), and `smtp_test`/`send_report` treat that as success and never fall back to SMTP. A recipient on a domain Exchange Online considers an accepted/internal domain (e.g. a Google-hosted subdomain of the O365 domain) is silently dropped after the 202. There is no in-app fix for that routing; reaching such recipients requires SMTP (e.g. Google Workspace `smtp.gmail.com`/`smtp-relay.gmail.com`) or fixing Exchange Accepted Domains.
|
||||||
|
|
||||||
## Scheduler — scan_scheduler.py + routes/scheduler.py
|
## Scheduler — scan_scheduler.py + routes/scheduler.py
|
||||||
|
|
||||||
|
|||||||
@ -314,11 +314,11 @@ function stLoadSmtp() {
|
|||||||
const set = function(id, val) { const el=document.getElementById(id); if(el) el.value=val||''; };
|
const set = function(id, val) { const el=document.getElementById(id); if(el) el.value=val||''; };
|
||||||
set('st-smtpHost', d.host);
|
set('st-smtpHost', d.host);
|
||||||
set('st-smtpPort', d.port || 587);
|
set('st-smtpPort', d.port || 587);
|
||||||
set('st-smtpUser', d.user);
|
set('st-smtpUser', d.username);
|
||||||
set('st-smtpFrom', d.from_addr);
|
set('st-smtpFrom', d.from_addr);
|
||||||
set('st-smtpTo', Array.isArray(d.recipients) ? d.recipients.join(', ') : (d.recipients||''));
|
set('st-smtpTo', Array.isArray(d.recipients) ? d.recipients.join(', ') : (d.recipients||''));
|
||||||
const tls = document.getElementById('st-smtpTls');
|
const tls = document.getElementById('st-smtpTls');
|
||||||
if (tls) tls.checked = d.starttls !== false;
|
if (tls) tls.checked = d.use_tls !== false;
|
||||||
const pw = document.getElementById('st-smtpPw');
|
const pw = document.getElementById('st-smtpPw');
|
||||||
if (pw) pw.value = d.has_password ? '\u2022\u2022\u2022\u2022\u2022\u2022\u2022\u2022' : '';
|
if (pw) pw.value = d.has_password ? '\u2022\u2022\u2022\u2022\u2022\u2022\u2022\u2022' : '';
|
||||||
const ae = document.getElementById('st-smtpAutoEmail');
|
const ae = document.getElementById('st-smtpAutoEmail');
|
||||||
@ -333,10 +333,13 @@ async function stSmtpSave() {
|
|||||||
const body = {
|
const body = {
|
||||||
host: document.getElementById('st-smtpHost').value.trim(),
|
host: document.getElementById('st-smtpHost').value.trim(),
|
||||||
port: parseInt(document.getElementById('st-smtpPort').value) || 587,
|
port: parseInt(document.getElementById('st-smtpPort').value) || 587,
|
||||||
user: document.getElementById('st-smtpUser').value.trim(),
|
// Backend (routes/email.py) reads these exact keys — `username`/`use_tls`,
|
||||||
|
// not `user`/`starttls`. Sending the wrong keys leaves username empty so
|
||||||
|
// server.login() is skipped and the SMTP server rejects the send.
|
||||||
|
username: document.getElementById('st-smtpUser').value.trim(),
|
||||||
from_addr: document.getElementById('st-smtpFrom').value.trim(),
|
from_addr: document.getElementById('st-smtpFrom').value.trim(),
|
||||||
recipients: document.getElementById('st-smtpTo').value.split(/[,;]/).map(function(s){return s.trim();}).filter(Boolean),
|
recipients: document.getElementById('st-smtpTo').value.split(/[,;]/).map(function(s){return s.trim();}).filter(Boolean),
|
||||||
starttls: document.getElementById('st-smtpTls').checked,
|
use_tls: document.getElementById('st-smtpTls').checked,
|
||||||
auto_email_manual: !!(document.getElementById('st-smtpAutoEmail') || {}).checked,
|
auto_email_manual: !!(document.getElementById('st-smtpAutoEmail') || {}).checked,
|
||||||
};
|
};
|
||||||
if (pw !== null) body.password = pw;
|
if (pw !== null) body.password = pw;
|
||||||
|
|||||||
@ -252,3 +252,36 @@ class TestFernet:
|
|||||||
def test_decrypt_empty_returns_empty(self):
|
def test_decrypt_empty_returns_empty(self):
|
||||||
result = app_config._decrypt_password("")
|
result = app_config._decrypt_password("")
|
||||||
assert result == ""
|
assert result == ""
|
||||||
|
|
||||||
|
|
||||||
|
class TestSmtpConfigLegacyKeys:
|
||||||
|
"""SMTP config saved by the older settings tab used `user`/`starttls`;
|
||||||
|
readers expect `username`/`use_tls`. _load_smtp_config must normalise them."""
|
||||||
|
|
||||||
|
def test_legacy_keys_normalised_on_load(self, tmp_path, monkeypatch):
|
||||||
|
import json
|
||||||
|
p = tmp_path / "smtp.json"
|
||||||
|
p.write_text(json.dumps({
|
||||||
|
"host": "smtp.gmail.com", "port": 587,
|
||||||
|
"user": "netadmin@adm.example.dk", # legacy key
|
||||||
|
"starttls": True, # legacy key
|
||||||
|
"from_addr": "netadmin@adm.example.dk",
|
||||||
|
"recipients": ["a@example.dk"],
|
||||||
|
}), encoding="utf-8")
|
||||||
|
monkeypatch.setattr(app_config, "_SMTP_CONFIG_PATH", p)
|
||||||
|
|
||||||
|
cfg = app_config._load_smtp_config()
|
||||||
|
assert cfg["username"] == "netadmin@adm.example.dk"
|
||||||
|
assert cfg["use_tls"] is True
|
||||||
|
|
||||||
|
def test_canonical_keys_take_precedence(self, tmp_path, monkeypatch):
|
||||||
|
import json
|
||||||
|
p = tmp_path / "smtp.json"
|
||||||
|
p.write_text(json.dumps({
|
||||||
|
"username": "canonical@example.dk",
|
||||||
|
"user": "legacy@example.dk",
|
||||||
|
}), encoding="utf-8")
|
||||||
|
monkeypatch.setattr(app_config, "_SMTP_CONFIG_PATH", p)
|
||||||
|
|
||||||
|
cfg = app_config._load_smtp_config()
|
||||||
|
assert cfg["username"] == "canonical@example.dk"
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user