From ad82ee1c8317588a6e9edb0d3f7d898c2c5ae6ac Mon Sep 17 00:00:00 2001 From: Logan Davis Date: Fri, 5 Jun 2026 14:32:50 -0400 Subject: [PATCH] feat(calendar): support multiple CalDAV accounts (#2942) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(calendar): support multiple CalDAV accounts Replaces the single CalDAV credential slot with a named account list so users can sync both a personal and work calendar simultaneously. - Add `account_id` column to `CalendarCal` + startup migration - `_load_caldav_accounts()` in caldav_sync.py reads `caldav_accounts` list from prefs, auto-migrating the legacy single `caldav` key on first use (no user action required) - `sync_caldav()` iterates all accounts and aggregates counts/errors - `writeback_event()` resolves credentials via `CalendarCal.account_id`, falling back to the first account for legacy rows - New REST endpoints: GET/POST/PUT/DELETE `/api/calendar/config/accounts` - Legacy GET/POST `/api/calendar/config` preserved for backward compat - Settings UI: one card per account with Label, URL, Username, Password fields; Test button works for both unsaved (inline creds) and saved (by account_id) accounts; delete removes only that account - Update test_caldav_url_hardening.py mock to include `_save_for_user` and updated `_sync_blocking` signature * fix(calendar): restore #2765 PK scoping and #2819 writeback URL validation Two regressions introduced by the multi-account refactor: 1. PK collision (#2765): _stable_cal_id was back to hashing only the URL, so two users — or one user with two accounts on the same server — would collide on the primary key. Restore owner+account_id in the hash key (format: "{owner}\n{account_id}\n{url}") and thread both values through _sync_blocking → _writeback_blocking → push_event → find_remote_calendar so the hash round-trips correctly on write-back. 2. URL validation dropped (#2819): _load_caldav_accounts imported _save_for_user at function scope, causing an ImportError on test mocks that only provide _load_for_user, which prevented writeback_event from reaching the validate_caldav_url call. Move the import inside the migration branch and wrap in try/except (best-effort save; next call re-migrates from the still-present legacy key). Update fake_writeback_blocking in test_caldav_writeback.py to accept the new owner/account_id optional params. --- core/database.py | 28 +++- routes/calendar_routes.py | 211 ++++++++++++++++++++++------- src/caldav_sync.py | 117 ++++++++++++---- src/caldav_writeback.py | 70 +++++++--- static/js/settings.js | 103 +++++++------- tests/test_caldav_url_hardening.py | 6 +- tests/test_caldav_writeback.py | 6 +- 7 files changed, 389 insertions(+), 152 deletions(-) diff --git a/core/database.py b/core/database.py index 8a88b2854..989430ee9 100644 --- a/core/database.py +++ b/core/database.py @@ -1458,7 +1458,11 @@ class CalendarCal(TimestampMixin, Base): owner = Column(String, nullable=True, index=True) name = Column(String, nullable=False) color = Column(String, default="#5b8abf") - source = Column(String, default="local") # "local" or "timetree" + source = Column(String, default="local") # "local" or "caldav" + # UUID of the CalDAV account in user prefs that owns this calendar. + # NULL for local calendars and for CalDAV calendars created before + # multi-account support was added (treated as "use any configured account"). + account_id = Column(String, nullable=True, index=True) events = relationship("CalendarEvent", back_populates="calendar", cascade="all, delete-orphan") @@ -1622,6 +1626,7 @@ def init_db(): _migrate_add_calendar_metadata() _migrate_add_calendar_is_utc() _migrate_add_calendar_origin() + _migrate_add_calendar_account_id() _migrate_encrypt_email_passwords() _migrate_encrypt_signatures() _migrate_encrypt_endpoint_keys() @@ -1786,6 +1791,27 @@ def _migrate_add_calendar_origin(): logging.getLogger(__name__).warning(f"calendar_events.origin migration failed: {e}") +def _migrate_add_calendar_account_id(): + """Add `account_id` to calendars so each CalDAV-backed calendar knows which + credential set (from caldav_accounts in user prefs) owns it. Idempotent.""" + import sqlite3 + db_path = DATABASE_URL.replace("sqlite:///", "") + if not os.path.exists(db_path): + return + try: + conn = sqlite3.connect(db_path) + cursor = conn.execute("PRAGMA table_info(calendars)") + columns = [row[1] for row in cursor.fetchall()] + if columns and "account_id" not in columns: + conn.execute("ALTER TABLE calendars ADD COLUMN account_id TEXT") + conn.execute("CREATE INDEX IF NOT EXISTS ix_calendars_account_id ON calendars(account_id)") + conn.commit() + logging.getLogger(__name__).info("Migrated: added 'account_id' column to calendars") + conn.close() + except Exception as e: + logging.getLogger(__name__).warning(f"calendars.account_id migration failed: {e}") + + def _migrate_add_calendar_metadata(): """Add importance/event_type/last_pinged columns to calendar_events table.""" import sqlite3 diff --git a/routes/calendar_routes.py b/routes/calendar_routes.py index b8bb1e9f6..75b6a5715 100644 --- a/routes/calendar_routes.py +++ b/routes/calendar_routes.py @@ -579,72 +579,178 @@ def _expand_rrule( def setup_calendar_routes() -> APIRouter: router = APIRouter(prefix="/api/calendar", tags=["calendar"]) - # CalDAV connect form (Integrations → Calendar). Storage is local - # SQLite; sync (src/caldav_sync.py) pulls remote events into it on - # calendar open and periodically via the scheduler. + # ── CalDAV multi-account helpers ───────────────────────────────────────── + + def _get_caldav_accounts(owner: str) -> list: + from src.caldav_sync import _load_caldav_accounts + return _load_caldav_accounts(owner) + + def _save_caldav_accounts(owner: str, accounts: list) -> None: + from routes.prefs_routes import _load_for_user, _save_for_user + prefs = _load_for_user(owner) or {} + prefs["caldav_accounts"] = accounts + prefs.pop("caldav", None) + _save_for_user(owner, prefs) + + # ── CalDAV config routes (backward-compat single-account API) ──────────── + @router.get("/config") async def get_config(request: Request): + """Legacy single-account endpoint — returns the first configured account.""" owner = _require_user(request) - from routes.prefs_routes import _load_for_user - cfg = (_load_for_user(owner) or {}).get("caldav", {}) or {} - caldav_password = cfg.get("password") or "" - if caldav_password: + accounts = _get_caldav_accounts(owner) + if not accounts: + return {"url": "", "username": "", "password": "", "has_password": False, "local": True} + first = accounts[0] + pw = first.get("password") or "" + has_pw = False + if pw: try: from src.secret_storage import decrypt - caldav_password = decrypt(caldav_password) + has_pw = bool(decrypt(pw)) except Exception: - pass - # Surface url+username but never hand the password back to the - # client — saved-state UI shouldn't leak the credential. + has_pw = bool(pw) return { - "url": cfg.get("url", "") or "", - "username": cfg.get("username", "") or "", + "url": first.get("url", "") or "", + "username": first.get("username", "") or "", "password": "", - "has_password": bool(caldav_password), - "local": not bool(cfg.get("url")), + "has_password": has_pw, + "local": not bool(first.get("url")), } @router.post("/config") async def save_config(request: Request): + """Legacy single-account endpoint — upserts the first account.""" owner = _require_user(request) - from routes.prefs_routes import _load_for_user, _save_for_user try: body = await request.json() except Exception: body = {} - prefs = _load_for_user(owner) or {} - cfg = dict(prefs.get("caldav") or {}) - # Empty url => clear the whole entry (treat as "remove integration"). + accounts = _get_caldav_accounts(owner) if not (body.get("url") or "").strip(): - prefs.pop("caldav", None) - _save_for_user(owner, prefs) + _save_caldav_accounts(owner, []) return {"ok": True, "cleared": True} from src.caldav_sync import validate_caldav_url try: - cfg["url"] = validate_caldav_url(body.get("url", "")) + validated_url = validate_caldav_url(body.get("url", "")) except ValueError as e: raise HTTPException(400, str(e)) - cfg["username"] = (body.get("username") or "").strip() - # Preserve the stored password when the client sends an empty - # one (edit form re-submitted without re-typing the password). - # cfg already holds the existing (already-encrypted) password from - # prefs, so we only touch it when a new password is supplied — - # re-encrypting the stored value would double-encrypt it. + if accounts: + acc = dict(accounts[0]) + else: + import uuid as _uuid + acc = {"id": str(_uuid.uuid4()), "label": "CalDAV"} + acc["url"] = validated_url + acc["username"] = (body.get("username") or "").strip() if body.get("password"): from src.secret_storage import encrypt - cfg["password"] = encrypt(body["password"]) - prefs["caldav"] = cfg - _save_for_user(owner, prefs) + acc["password"] = encrypt(body["password"]) + new_accounts = [acc] + (accounts[1:] if len(accounts) > 1 else []) + _save_caldav_accounts(owner, new_accounts) + return {"ok": True} + + # ── CalDAV multi-account CRUD ───────────────────────────────────────────── + + @router.get("/config/accounts") + async def list_caldav_accounts(request: Request): + """Return all configured CalDAV accounts (passwords never returned).""" + owner = _require_user(request) + accounts = _get_caldav_accounts(owner) + safe = [] + for acc in accounts: + pw = acc.get("password") or "" + has_pw = False + if pw: + try: + from src.secret_storage import decrypt + has_pw = bool(decrypt(pw)) + except Exception: + has_pw = bool(pw) + safe.append({ + "id": acc.get("id", ""), + "label": acc.get("label", "") or acc.get("url", ""), + "url": acc.get("url", "") or "", + "username": acc.get("username", "") or "", + "has_password": has_pw, + }) + return {"accounts": safe} + + @router.post("/config/accounts") + async def add_caldav_account(request: Request): + """Add a new CalDAV account.""" + import uuid as _uuid + owner = _require_user(request) + try: + body = await request.json() + except Exception: + body = {} + from src.caldav_sync import validate_caldav_url + try: + url = validate_caldav_url(body.get("url", "")) + except ValueError as e: + raise HTTPException(400, str(e)) + if not body.get("password"): + raise HTTPException(400, "Password is required") + from src.secret_storage import encrypt + new_acc = { + "id": str(_uuid.uuid4()), + "label": (body.get("label") or "").strip() or "CalDAV", + "url": url, + "username": (body.get("username") or "").strip(), + "password": encrypt(body["password"]), + } + accounts = _get_caldav_accounts(owner) + accounts.append(new_acc) + _save_caldav_accounts(owner, accounts) + return {"ok": True, "id": new_acc["id"]} + + @router.put("/config/accounts/{account_id}") + async def update_caldav_account(account_id: str, request: Request): + """Update an existing CalDAV account by id.""" + owner = _require_user(request) + try: + body = await request.json() + except Exception: + body = {} + accounts = _get_caldav_accounts(owner) + idx = next((i for i, a in enumerate(accounts) if a.get("id") == account_id), None) + if idx is None: + raise HTTPException(404, "Account not found") + acc = dict(accounts[idx]) + if body.get("url"): + from src.caldav_sync import validate_caldav_url + try: + acc["url"] = validate_caldav_url(body["url"]) + except ValueError as e: + raise HTTPException(400, str(e)) + if body.get("label") is not None: + acc["label"] = (body.get("label") or "").strip() or "CalDAV" + if body.get("username") is not None: + acc["username"] = (body.get("username") or "").strip() + if body.get("password"): + from src.secret_storage import encrypt + acc["password"] = encrypt(body["password"]) + accounts[idx] = acc + _save_caldav_accounts(owner, accounts) + return {"ok": True} + + @router.delete("/config/accounts/{account_id}") + async def delete_caldav_account(account_id: str, request: Request): + """Remove a CalDAV account by id.""" + owner = _require_user(request) + accounts = _get_caldav_accounts(owner) + new_accounts = [a for a in accounts if a.get("id") != account_id] + if len(new_accounts) == len(accounts): + raise HTTPException(404, "Account not found") + _save_caldav_accounts(owner, new_accounts) return {"ok": True} @router.post("/test") async def test_connection(request: Request): - """Actually probe the configured CalDAV server with a PROPFIND - request (the same handshake every CalDAV client uses). Accepts - an optional {url, username, password} body so the user can test - a configuration BEFORE saving it; falls back to the stored - creds otherwise. Returns {ok, error?} with a useful message on - failure (status code, auth issue, network error).""" + """Probe a CalDAV server with a PROPFIND. Accepts an optional body: + {url, username, password} to test before saving, or {account_id} to + test an already-saved account. Falls back to the first saved account + when nothing is provided.""" owner = _require_user(request) try: body = await request.json() @@ -654,19 +760,24 @@ def setup_calendar_routes() -> APIRouter: user = (body.get("username") or "").strip() pw = body.get("password") or "" if not (url and user and pw): - # Fall back to saved settings for this user. - from routes.prefs_routes import _load_for_user - cfg = (_load_for_user(owner) or {}).get("caldav", {}) or {} - url = url or (cfg.get("url") or "") - user = user or (cfg.get("username") or "") - if not pw: - pw = cfg.get("password") or "" - if pw: - try: - from src.secret_storage import decrypt - pw = decrypt(pw) - except Exception: - pass + # Look up a saved account: by id if supplied, else first account. + accounts = _get_caldav_accounts(owner) + acc = None + if body.get("account_id"): + acc = next((a for a in accounts if a.get("id") == body["account_id"]), None) + if acc is None and accounts: + acc = accounts[0] + if acc: + url = url or (acc.get("url") or "") + user = user or (acc.get("username") or "") + if not pw: + pw = acc.get("password") or "" + if pw: + try: + from src.secret_storage import decrypt + pw = decrypt(pw) + except Exception: + pass if not (url and user and pw): return {"ok": False, "error": "Missing URL, username, or password"} from src.caldav_sync import validate_caldav_url diff --git a/src/caldav_sync.py b/src/caldav_sync.py index 578e37041..a2ce22acf 100644 --- a/src/caldav_sync.py +++ b/src/caldav_sync.py @@ -128,10 +128,14 @@ def validate_caldav_url(raw_url: str) -> str: return urlunparse(parsed._replace(fragment="")).rstrip("/") -def _stable_cal_id(remote_url: str) -> str: - """Deterministic local id for a remote CalDAV calendar — same URL - always maps to the same local row across restarts and re-syncs.""" - h = hashlib.sha256(remote_url.encode("utf-8")).hexdigest()[:24] +def _stable_cal_id(remote_url: str, owner: str = "", account_id: str = "") -> str: + """Deterministic local id for a remote CalDAV calendar, scoped to owner + and account so two users — or one user with two accounts — pointing at + the same server URL get distinct local rows (avoids PK collision, #2765). + The owner and account_id default to "" for the legacy/URL-only path so + existing callers without those arguments keep working.""" + key = f"{owner}\n{account_id}\n{remote_url}" + h = hashlib.sha256(key.encode("utf-8")).hexdigest()[:24] return f"caldav-{h}" @@ -212,7 +216,7 @@ def _open_url_as_calendar(client, url: str): return client.calendar(url=target) -def _sync_blocking(owner: str, url: str, username: str, password: str) -> dict: +def _sync_blocking(owner: str, url: str, username: str, password: str, account_id: str = "") -> dict: """The actual sync — synchronous, intended to run in a threadpool. Returns counts: {calendars, events, deleted, errors}.""" # Lazy imports so a missing `caldav` dep doesn't break app startup — @@ -258,7 +262,7 @@ def _sync_blocking(owner: str, url: str, username: str, password: str) -> dict: for remote_cal in calendars: try: remote_url = str(remote_cal.url) - cal_id = _stable_cal_id(remote_url) + cal_id = _stable_cal_id(remote_url, owner=owner, account_id=account_id) display_name = (remote_cal.name or "").strip() or "CalDAV" local_cal = db.query(CalendarCal).filter( @@ -272,14 +276,20 @@ def _sync_blocking(owner: str, url: str, username: str, password: str) -> dict: name=display_name, color="#5b8abf", source="caldav", + account_id=account_id or None, ) db.add(local_cal) db.commit() else: - # Refresh the display name if the user renamed it - # remotely; preserve any local color override. + # Refresh display name and stamp account_id if missing. + changed = False if local_cal.name != display_name: local_cal.name = display_name + changed = True + if account_id and not local_cal.account_id: + local_cal.account_id = account_id + changed = True + if changed: db.commit() result["calendars"] += 1 @@ -401,31 +411,78 @@ def _sync_blocking(owner: str, url: str, username: str, password: str) -> dict: return result -async def sync_caldav(owner: str) -> dict: - """Pull CalDAV state into local DB for `owner`. Returns counts + - errors. Loads credentials from the user's prefs; no-ops with a - clear error if CalDAV isn't configured.""" +def _load_caldav_accounts(owner: str) -> list: + """Return the list of CalDAV accounts for *owner*, auto-migrating the legacy + single-account ``caldav`` key to the new ``caldav_accounts`` list on first call. + + The save step is best-effort: if ``_save_for_user`` is unavailable (e.g. in a + test with a minimal prefs mock) the migrated accounts are still returned; the + next real call will just re-run the cheap migration again. + """ + import uuid as _uuid from routes.prefs_routes import _load_for_user - cfg = (_load_for_user(owner) or {}).get("caldav", {}) or {} - url = (cfg.get("url") or "").strip() - user = (cfg.get("username") or "").strip() - pw = cfg.get("password") or "" - try: - from src.secret_storage import decrypt - pw = decrypt(pw) - except Exception: - pass - if not (url and user and pw): + prefs = _load_for_user(owner) or {} + if "caldav_accounts" in prefs: + return list(prefs["caldav_accounts"] or []) + # Migrate legacy single-account config to the list format. + legacy = prefs.get("caldav", {}) or {} + if legacy.get("url"): + accounts = [{ + "id": str(_uuid.uuid4()), + "label": "CalDAV", + "url": legacy["url"], + "username": legacy.get("username", ""), + "password": legacy.get("password", ""), + }] + prefs["caldav_accounts"] = accounts + prefs.pop("caldav", None) + try: + from routes.prefs_routes import _save_for_user + _save_for_user(owner, prefs) + except (ImportError, AttributeError): + pass # best-effort; next call re-migrates from the still-present legacy key + return accounts + return [] + + +async def sync_caldav(owner: str) -> dict: + """Pull CalDAV state into local DB for `owner` across all configured accounts. + Returns aggregated counts + per-account errors.""" + from src.secret_storage import decrypt + + accounts = _load_caldav_accounts(owner) + if not accounts: return { "calendars": 0, "events": 0, "deleted": 0, "errors": ["CalDAV is not configured"], } - try: - url = validate_caldav_url(url) - return await asyncio.to_thread(_sync_blocking, owner, url, user, pw) - except ValueError as e: - return {"calendars": 0, "events": 0, "deleted": 0, "errors": [str(e)]} - except Exception as e: - logger.exception("CalDAV sync raised") - return {"calendars": 0, "events": 0, "deleted": 0, "errors": [str(e)[:200]]} + + totals: dict = {"calendars": 0, "events": 0, "deleted": 0, "errors": []} + for acc in accounts: + url = (acc.get("url") or "").strip() + user = (acc.get("username") or "").strip() + pw = acc.get("password") or "" + account_id = acc.get("id") or "" + label = acc.get("label") or url or account_id + try: + pw = decrypt(pw) + except Exception: + pass + if not (url and user and pw): + totals["errors"].append(f"{label}: missing URL, username, or password") + continue + try: + url = validate_caldav_url(url) + result = await asyncio.to_thread(_sync_blocking, owner, url, user, pw, account_id) + except ValueError as e: + result = {"calendars": 0, "events": 0, "deleted": 0, "errors": [str(e)]} + except Exception as e: + logger.exception("CalDAV sync raised for account %s", label) + result = {"calendars": 0, "events": 0, "deleted": 0, "errors": [str(e)[:200]]} + totals["calendars"] += result.get("calendars", 0) + totals["events"] += result.get("events", 0) + totals["deleted"] += result.get("deleted", 0) + for err in result.get("errors", []): + totals["errors"].append(f"{label}: {err}") + return totals diff --git a/src/caldav_writeback.py b/src/caldav_writeback.py index e5bc46d0e..b1b92c05f 100644 --- a/src/caldav_writeback.py +++ b/src/caldav_writeback.py @@ -23,11 +23,10 @@ from datetime import timezone logger = logging.getLogger(__name__) -def _stable_cal_id(remote_url: str) -> str: - # Reuse the sync module's hashing so a local CalDAV calendar id maps back to - # the same remote URL it was pulled from. +def _stable_cal_id(remote_url: str, owner: str = "", account_id: str = "") -> str: + # Reuse the sync module's hashing so owner+account_id scoping stays consistent. from src.caldav_sync import _stable_cal_id as _sync_id - return _sync_id(remote_url) + return _sync_id(remote_url, owner=owner, account_id=account_id) def build_event_ical(ev: dict) -> str: @@ -76,28 +75,34 @@ def build_event_ical(ev: dict) -> str: return cal.to_ical().decode("utf-8") -def find_remote_calendar(calendars, local_cal_id: str): - """Find the remote calendar whose URL hashes to ``local_cal_id``, or None.""" +def find_remote_calendar(calendars, local_cal_id: str, owner: str = "", account_id: str = ""): + """Find the remote calendar whose URL hashes to ``local_cal_id``, or None. + + ``owner`` and ``account_id`` must match what was used when the local calendar + id was originally computed in ``_sync_blocking`` so the hash round-trips.""" for cal in calendars: try: - if _stable_cal_id(str(cal.url)) == local_cal_id: + if _stable_cal_id(str(cal.url), owner=owner, account_id=account_id) == local_cal_id: return cal except Exception: continue return None -def push_event(calendars, local_cal_id: str, ev: dict, *, delete: bool = False) -> dict: +def push_event(calendars, local_cal_id: str, ev: dict, *, delete: bool = False, + owner: str = "", account_id: str = "") -> dict: """Create/update (or delete) ``ev`` on the matching remote calendar. Returns ``{"ok": bool, ...}``. ``calendars`` is the discovered caldav calendar list (injected so this is unit-testable with fakes). + ``owner`` and ``account_id`` are forwarded to ``find_remote_calendar`` + so the URL hash round-trips correctly (#2765). """ uid = (ev or {}).get("uid") if isinstance(ev, dict) else None if not uid: return {"ok": False, "error": "event uid is required"} - remote = find_remote_calendar(calendars, local_cal_id) + remote = find_remote_calendar(calendars, local_cal_id, owner=owner, account_id=account_id) if remote is None: return {"ok": False, "error": "remote calendar not found"} @@ -136,13 +141,15 @@ def _discover_calendars(client): return [] -def _writeback_blocking(local_cal_id, ev, delete, url, username, password) -> dict: +def _writeback_blocking(local_cal_id, ev, delete, url, username, password, + owner="", account_id="") -> dict: import caldav client = caldav.DAVClient(url=url, username=username, password=password) calendars = _discover_calendars(client) if not calendars: return {"ok": False, "error": "no remote calendars discovered"} - return push_event(calendars, local_cal_id, ev, delete=delete) + return push_event(calendars, local_cal_id, ev, delete=delete, + owner=owner, account_id=account_id) async def writeback_event(owner: str, calendar_source: str, calendar_id: str, @@ -156,24 +163,45 @@ async def writeback_event(owner: str, calendar_source: str, calendar_id: str, if calendar_source != "caldav": return {"skipped": "not a caldav calendar"} try: - from routes.prefs_routes import _load_for_user + from src.caldav_sync import _load_caldav_accounts from src.secret_storage import decrypt - cfg = (_load_for_user(owner) or {}).get("caldav", {}) or {} - url = (cfg.get("url") or "").strip() - user = (cfg.get("username") or "").strip() - # Stored encrypted by routes/calendar_routes; decrypt before use so - # the remote sees the real password (decrypt is a no-op on legacy - # plaintext). The pull path src/caldav_sync.py already does this. - pw = decrypt(cfg.get("password") or "") - if not (url and user and pw): + from core.database import CalendarCal, SessionLocal + + accounts = _load_caldav_accounts(owner) + if not accounts: return {"skipped": "caldav not configured"} + + # Find which account owns this calendar. + acc = None + if len(accounts) > 1: + db = SessionLocal() + try: + cal_row = db.query(CalendarCal).filter(CalendarCal.id == calendar_id).first() + cal_account_id = cal_row.account_id if cal_row else None + finally: + db.close() + if cal_account_id: + acc = next((a for a in accounts if a.get("id") == cal_account_id), None) + # Fall back to first account (covers single-account and legacy rows with + # no account_id stamped). + if acc is None: + acc = accounts[0] + + url = (acc.get("url") or "").strip() + user = (acc.get("username") or "").strip() + pw = decrypt(acc.get("password") or "") + if not (url and user and pw): + return {"skipped": "caldav account credentials incomplete"} from src.caldav_sync import validate_caldav_url try: url = validate_caldav_url(url) except ValueError as e: logger.warning("CalDAV write-back URL rejected: %s", e) return {"ok": False, "error": str(e)[:200]} - result = await asyncio.to_thread(_writeback_blocking, calendar_id, ev, delete, url, user, pw) + acc_id = acc.get("id") or "" + result = await asyncio.to_thread( + _writeback_blocking, calendar_id, ev, delete, url, user, pw, owner, acc_id + ) if not result.get("ok"): logger.warning("CalDAV write-back did not apply: %s", result.get("error") or result) return result diff --git a/static/js/settings.js b/static/js/settings.js index f9e76558c..dcc3498ba 100644 --- a/static/js/settings.js +++ b/static/js/settings.js @@ -3199,7 +3199,7 @@ async function initUnifiedIntegrations() { async function fetchAll() { const [apiRes, calRes, cardRes, contactsRes, emailAccountsRes, mcpRes, vaultRes, tokenRes, calendarsRes] = await Promise.all([ fetch('/api/auth/integrations', { credentials: 'same-origin' }).then(r => r.ok ? r.json() : { integrations: [] }).catch(() => ({ integrations: [] })), - fetch('/api/calendar/config', { credentials: 'same-origin' }).then(r => r.ok ? r.json() : {}).catch(() => ({})), + fetch('/api/calendar/config/accounts', { credentials: 'same-origin' }).then(r => r.ok ? r.json() : { accounts: [] }).catch(() => ({ accounts: [] })), fetch('/api/contacts/config', { credentials: 'same-origin' }).then(r => r.ok ? r.json() : {}).catch(() => ({})), fetch('/api/contacts/list', { credentials: 'same-origin' }).then(r => r.ok ? r.json() : { contacts: [], count: 0 }).catch(() => ({ contacts: [], count: 0 })), fetch('/api/email/accounts', { credentials: 'same-origin' }).then(r => r.ok ? r.json() : { accounts: [] }).catch(() => ({ accounts: [] })), @@ -3213,15 +3213,9 @@ async function initUnifiedIntegrations() { for (const intg of (apiRes.integrations || [])) { items.push({ type: 'api', id: intg.id, name: intg.name || 'Unnamed', detail: intg.base_url || '', enabled: intg.enabled !== false, data: intg }); } - // CalDAV — one card per synced calendar collection; fall back to the - // server-level entry if calendars haven't been synced yet. - const caldavCals = (calendarsRes.calendars || []).filter(c => c.source === 'caldav'); - if (caldavCals.length > 0) { - for (const cal of caldavCals) { - items.push({ type: 'caldav', id: cal.href, name: cal.name, detail: calRes.url || 'CalDAV', enabled: true, data: { ...cal, serverData: calRes } }); - } - } else if (calRes.url) { - items.push({ type: 'caldav', id: '__caldav__', name: 'Calendar (CalDAV)', detail: calRes.url, enabled: true, data: calRes }); + // CalDAV — one card per account + for (const acc of (calRes.accounts || [])) { + items.push({ type: 'caldav', id: acc.id, name: acc.label || 'Calendar (CalDAV)', detail: acc.url, enabled: true, data: acc }); } // Contacts import first, then the optional CardDAV sync account. const contactCount = Number(contactsRes.count || (contactsRes.contacts || []).length || 0); @@ -3334,14 +3328,7 @@ async function initUnifiedIntegrations() { const id = btn.dataset.intgId; try { if (type === 'api') await fetch(`/api/auth/integrations/${id}`, { method: 'DELETE', credentials: 'same-origin' }); - else if (type === 'caldav') { - if (id === '__caldav__') { - // Fallback card: server configured but never synced — clear credentials - await fetch('/api/calendar/config', { method: 'POST', credentials: 'same-origin', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ url: '', username: '', password: '' }) }); - } else { - await fetch(`/api/calendar/calendars/${id}`, { method: 'DELETE', credentials: 'same-origin' }); - } - } + else if (type === 'caldav') await fetch(`/api/calendar/config/accounts/${id}`, { method: 'DELETE', credentials: 'same-origin' }); else if (type === 'contacts') { await fetch('/api/contacts/clear', { method: 'DELETE', credentials: 'same-origin' }); } @@ -3363,7 +3350,7 @@ async function initUnifiedIntegrations() { function showForm(type, editId) { formEl.style.display = ''; if (type === 'api') showApiForm(editId); - else if (type === 'caldav') showCalDavForm(); + else if (type === 'caldav') showCalDavForm(editId); else if (type === 'contacts' || type === 'carddav') showCardDavForm(); else if (type === 'email') showEmailForm(editId); else if (type === 'mcp') showMcpForm(editId); @@ -3555,33 +3542,43 @@ async function initUnifiedIntegrations() { }); } - // ── CalDAV form ── - async function showCalDavForm() { + // ── CalDAV form (supports add + edit per account) ── + async function showCalDavForm(editId) { + const isNew = !editId || editId === 'new'; formEl.innerHTML = `
-

Calendar (CalDAV)

+

${isNew ? 'Add CalDAV Calendar' : 'Edit CalDAV Calendar'}

-
-
-
-
+
+
+
+
+
`; - try { - const r = await fetch('/api/calendar/config', { credentials: 'same-origin' }); const d = await r.json(); - el('uf-caldav-url').value = d.url || ''; el('uf-caldav-user').value = d.username || ''; - } catch (_) {} + + if (!isNew) { + try { + const r = await fetch('/api/calendar/config/accounts', { credentials: 'same-origin' }); + const d = await r.json(); + const acc = (d.accounts || []).find(a => a.id === editId); + if (acc) { + el('uf-caldav-label').value = acc.label || ''; + el('uf-caldav-url').value = acc.url || ''; + el('uf-caldav-user').value = acc.username || ''; + } + } catch (_) {} + } + el('uf-caldav-cancel').addEventListener('click', () => { formEl.style.display = 'none'; }); - // Run a PROPFIND with the form's current url+user+pass. Used by - // both the Test button (visible result only) and by Save (refuse - // to persist a broken config). Returns the parsed {ok, error?}. const _runCalDavTest = async () => { const body = { url: el('uf-caldav-url').value.trim(), username: el('uf-caldav-user').value.trim(), password: el('uf-caldav-pass').value, }; + if (!isNew && !body.password) body.account_id = editId; try { const r = await fetch('/api/calendar/test', { method: 'POST', credentials: 'same-origin', @@ -3593,6 +3590,7 @@ async function initUnifiedIntegrations() { return { ok: false, error: 'Network error: ' + e.message }; } }; + const _setCalDavMsg = (text, ok) => { const msg = el('uf-caldav-msg'); msg.textContent = text; @@ -3600,10 +3598,6 @@ async function initUnifiedIntegrations() { }; el('uf-caldav-save').addEventListener('click', async () => { - // Pre-validate by hitting the server with the same PROPFIND the - // Test button uses. If the CalDAV server rejects the creds/URL - // we won't persist garbage — the user gets the actual error - // (HTTP 401, "Not found", "Connection refused", etc.) in red. _setCalDavMsg('Testing…', true); el('uf-caldav-msg').style.color = ''; const d = await _runCalDavTest(); @@ -3612,15 +3606,31 @@ async function initUnifiedIntegrations() { return; } try { - await fetch('/api/calendar/config', { - method: 'POST', credentials: 'same-origin', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ - url: el('uf-caldav-url').value, - username: el('uf-caldav-user').value, - password: el('uf-caldav-pass').value, - }), - }); + const payload = { + label: el('uf-caldav-label').value.trim(), + url: el('uf-caldav-url').value.trim(), + username: el('uf-caldav-user').value.trim(), + password: el('uf-caldav-pass').value, + }; + let resp; + if (isNew) { + resp = await fetch('/api/calendar/config/accounts', { + method: 'POST', credentials: 'same-origin', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(payload), + }); + } else { + resp = await fetch(`/api/calendar/config/accounts/${editId}`, { + method: 'PUT', credentials: 'same-origin', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(payload), + }); + } + if (!resp.ok) { + const err = await resp.json().catch(() => ({})); + _setCalDavMsg(err.detail || 'Save failed', false); + return; + } _setCalDavMsg('Saved', true); formEl.style.display = 'none'; await renderList(); @@ -3629,6 +3639,7 @@ async function initUnifiedIntegrations() { _setCalDavMsg('Save failed', false); } }); + el('uf-caldav-test').addEventListener('click', async () => { _setCalDavMsg('Testing…', true); el('uf-caldav-msg').style.color = ''; diff --git a/tests/test_caldav_url_hardening.py b/tests/test_caldav_url_hardening.py index 39de9a9eb..0ea8b2bf9 100644 --- a/tests/test_caldav_url_hardening.py +++ b/tests/test_caldav_url_hardening.py @@ -88,6 +88,7 @@ def test_sync_caldav_decrypts_stored_password_and_validates_url(monkeypatch): "_resolve_caldav_host_ips", lambda host: [ipaddress.ip_address("93.184.216.34")], ) + saved = {} prefs_mod = types.ModuleType("routes.prefs_routes") prefs_mod._load_for_user = lambda owner: { "caldav": { @@ -96,6 +97,7 @@ def test_sync_caldav_decrypts_stored_password_and_validates_url(monkeypatch): "password": "enc:stored", } } + prefs_mod._save_for_user = lambda owner, prefs: saved.update({"owner": owner, "prefs": prefs}) monkeypatch.setitem(sys.modules, "routes.prefs_routes", prefs_mod) secret_mod = types.ModuleType("src.secret_storage") @@ -104,7 +106,7 @@ def test_sync_caldav_decrypts_stored_password_and_validates_url(monkeypatch): captured = {} - def fake_sync_blocking(owner, url, username, password): + def fake_sync_blocking(owner, url, username, password, account_id=""): captured.update( { "owner": owner, @@ -136,7 +138,7 @@ def test_calendar_routes_use_hardened_caldav_client_and_secret_storage(): text = Path("routes/calendar_routes.py").read_text(encoding="utf-8") assert "validate_caldav_url(body.get(\"url\", \"\"))" in text - assert "cfg[\"password\"] = encrypt(body[\"password\"])" in text + assert "encrypt(body[\"password\"])" in text assert "pw = decrypt(pw)" in text assert "follow_redirects=False, trust_env=False" in text assert "Redirects are not followed for CalDAV safety" in text diff --git a/tests/test_caldav_writeback.py b/tests/test_caldav_writeback.py index f63671236..7776e7541 100644 --- a/tests/test_caldav_writeback.py +++ b/tests/test_caldav_writeback.py @@ -151,7 +151,8 @@ def test_writeback_validates_saved_url_before_remote_call(monkeypatch): captured["validated_url"] = url return "https://dav.example.com/calendars/home" - def fake_writeback_blocking(local_cal_id, ev, delete, url, username, password): + def fake_writeback_blocking(local_cal_id, ev, delete, url, username, password, + owner="", account_id=""): captured.update( { "local_cal_id": local_cal_id, @@ -207,7 +208,8 @@ def test_writeback_rejects_unsafe_saved_url_before_remote_call(monkeypatch): def fake_validate(_url): raise ValueError("CalDAV URL host is not allowed") - def fake_writeback_blocking(*_args, **_kwargs): + def fake_writeback_blocking(local_cal_id, ev, delete, url, username, password, + owner="", account_id=""): nonlocal called called = True return {"ok": True}