mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-06-16 01:35:36 -04:00
feat(calendar): support multiple CalDAV accounts (#2942)
* 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.
This commit is contained in:
Logan Davis
+87
-30
@@ -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
|
||||
|
||||
+49
-21
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user