diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f1ce7cc..0fd5e631 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,31 @@ exact tag (`ghcr.io/calnode/calnode:0.1.0`) if you need stability between upgrad the link-only fallback that needs no Zoom app. Answers [#35](https://github.com/Calnode/calnode/issues/35). +- **A CalDAV account now offers every calendar in it, not only the one it connected with.** + Closes [#42](https://github.com/Calnode/calnode/issues/42). + + Connecting binds one collection, and the calendar picker could only ever show that one, so + an account with several calendars (the report was Synology Calendar) could be checked and + booked into through its default calendar alone. The picker now lists every calendar under + the account's calendar home that can hold events, marks read-only shares (from + `DAV:current-user-privilege-set`) so they can be checked but not booked into, and free/busy + reads every calendar ticked for conflicts. Accounts that never saved a selection keep + reading only the calendar they were bound to, and nothing needs reconnecting: the list is + rediscovered from the stored calendar URL. The picker also says what Check and Book mean, + and why Book moves rather than unticks. + + Two refusals come with it, both CalDAV-only. A calendar listed on a different origin from + its calendar home is skipped, because a CalDAV calendar id is the URL that later receives the + account's credentials; listing an existing account's calendars goes further and never sends a + request, or follows a principal, calendar home or redirect, off the origin of the calendar it + connected with. This changes connecting for one setup: a server behind a reverse proxy + that reports its internal scheme or host in every href used to connect and now does not, with + an error naming both addresses and pointing at the proxy or base URL settings. And saving a + selection that names a CalDAV calendar the server did not list is refused with a 400, where + the endpoint used to store whatever the client sent. Google and Microsoft selections save + exactly as before, with no call to the provider: their ids carry no credentials anywhere, so + there is nothing for the check to protect. + ## [0.9.0] - 2026-09-10 ### Added diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 58defaab..cb44a6ab 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -481,6 +481,31 @@ Calnode talks to calendars through a **provider abstraction**, not a single vend added via the `openid` scope. A blanket `404 MailboxNotEnabledForRESTAPI` on every `/me/*` call means the account has **no Exchange Online mailbox** (not an auth error). Graph errors log their response body to make this self-evident. +- *CalDAV* (`internal/caldav`): username + app password over Basic auth, connected through + `POST /v1/calendar/caldav/connect`, which walks `current-user-principal` → + `calendar-home-set` → a Depth 1 listing and binds the account's default collection + (`calendar_connections.calendar_id`). **A calendar id is the collection URL**, so the picker, + free/busy and write-back all address collections directly. `ListCalendars` repeats that walk + from the bound collection with the stored credentials (the server URL typed at connect is not + stored; a server that reports no principal there gets the collection's parent) and lists + every VEVENT-capable collection under the home, with `Writable` read from + `DAV:current-user-privilege-set` (`all`, `write`, `write-content` or `bind`; not reported + counts as writable). **A collection on a different origin (scheme, host, port) from the home + is skipped**, at connect and when listing, because a stored id is sent the account's + credentials on every read and write. When that leaves connect with no event calendar at all + (typically a reverse proxy whose upstream reports its internal scheme or host in hrefs), it + fails naming both addresses rather than with "no writable calendar found". CalDAV implements + `calendar.SelectionValidator` (below). Free/busy REPORTs each calendar ticked for conflicts + (`ConflictCalendarIDs`) and makes no discovery requests; an account that never saved a + selection reads only its bound collection. A 401/403 wraps `calendar.ErrReauthRequired`, so + a revoked app password shows as "needs reconnecting" in the picker. + **Listing never leaves the bound collection's origin.** It runs on every picker load and + every saved selection, with the account's credentials, against hrefs the server chooses, so + `homeForCalendar` does not follow a principal or calendar home on another origin (it lists the + bound collection's parent instead), `propfind` is pinned to that origin and refuses a redirect + off it, and `ListCalendars` drops any collection off it. Connect-time discovery is not pinned: + it starts from a URL the user typed, and can legitimately cross hosts (iCloud's calendar home + is on a per-account partition host). **Online-meeting links are provider-matched (`booking_handler.go`).** A `google_meet`/`teams` event type auto-mints a link **only when the primary host's @@ -542,6 +567,15 @@ Saving a sub-calendar destination also moves the account-level destination to th or the choice silently does nothing when the picked calendar lives in a different account from the current destination. +**Where a calendar id says where credentials go, a saved selection is checked first.** A +provider implementing `calendar.SelectionValidator` (CalDAV, whose id is a collection URL that +free/busy and bookings authenticate to) is asked to accept every id before +`SetAccountCalendars` opens its transaction. A refused id is `*UnknownCalendarError` (400 from +`PUT .../calendars`) and nothing is saved; a failure to check is `ErrCalendarList` (502) or a +reauth error (409), as on the GET. Google and Microsoft do not implement it: their ids are +names inside an API the account's token already scopes, so saving there stays a local write +with no provider call, and aliases such as Google's `"primary"` still save. + **`booking_hosts.external_calendar_id` records where each event actually went** (migration 00055). Reschedule and cancel use it rather than re-resolving the current destination - otherwise changing the destination orphans every existing booking: the provider 404s, the diff --git a/frontend/src/routes/calendar/+page.svelte b/frontend/src/routes/calendar/+page.svelte index 67fec4f5..0f50df1c 100644 --- a/frontend/src/routes/calendar/+page.svelte +++ b/frontend/src/routes/calendar/+page.svelte @@ -318,8 +318,11 @@

No calendars found for this account.

{:else}

- Tick the calendars to check for conflicts, and choose the one bookings are - written into. + Check counts busy time on a calendar, so those + times can't be booked. Book is the single calendar + new bookings are written into (one across all your accounts), so it can't be + unticked, only moved to another calendar. Read-only calendars can be checked but + not booked into.

diff --git a/internal/caldav/caldav.go b/internal/caldav/caldav.go index cf143e75..4600ca0d 100644 --- a/internal/caldav/caldav.go +++ b/internal/caldav/caldav.go @@ -171,7 +171,7 @@ func (c *Client) loadConn(ctx context.Context, userID string, checkConflicts, is // For CalDAV the "calendar id" IS the collection URL, so picking a different calendar // inside the account is just a different URL to PUT into. Only applies to the write - // target; conflict checking already reads every selected collection. + // target; conflict checking reads every selected collection (conflictConns). if isDestination == 1 { if subCal, ok, sErr := connstore.DestinationCalendarID(ctx, c.db, userID, "caldav", cn.username); sErr != nil { return conn{}, false, sErr @@ -182,9 +182,10 @@ func (c *Client) loadConn(ctx context.Context, userID string, checkConflicts, is return cn, true, nil } -// conflictConns returns every CalDAV connection for the user with check_conflicts = 1, so a -// user can connect several CalDAV accounts and have them all checked. Decrypt failures on one -// row are logged and skipped (fail-open). +// conflictConns returns one entry per calendar to read for conflicts, across every CalDAV +// connection the user has with check_conflicts = 1, so a user can connect several CalDAV +// accounts and tick several calendars in each. Entries of one account share its credentials and +// differ only in calURL. Decrypt failures on one row are logged and skipped (fail-open). func (c *Client) conflictConns(ctx context.Context, userID string) ([]conn, error) { rows, err := c.db.QueryContext(ctx, ` SELECT id, COALESCE(account_email,''), access_token_enc, calendar_id @@ -208,9 +209,11 @@ func (c *Client) conflictConns(ctx context.Context, userID string) ([]conn, erro return nil, err } // Resolve selection + decrypt after the cursor is closed (single-connection DB pool; the - // ConflictCalendarIDs query would deadlock against an open cursor). CalDAV binds one - // calendar per connection, so the sub-calendar picker is simply an on/off toggle: an empty - // result means the user deselected it. + // ConflictCalendarIDs query would deadlock against an open cursor). An account with a saved + // selection yields exactly the calendars ticked in it, and none when all were unticked. One + // that never saved a selection yields its bound collection, which is all any CalDAV + // connection read before its calendars could be listed. The ids are collection URLs, and + // SetAccountCalendars only saves ones this provider listed for the account. var conns []conn for _, d := range data { calIDs, err := calendar.ConflictCalendarIDs(ctx, c.db, "caldav", userID, d.username, d.calURL) @@ -225,7 +228,9 @@ func (c *Client) conflictConns(ctx context.Context, userID string) ([]conn, erro c.logger.Warn("caldav: skipping connection with bad credentials", "user_id", userID, "error", err) continue } - conns = append(conns, conn{id: d.id, username: d.username, password: string(pw), calURL: d.calURL}) + for _, calURL := range calIDs { + conns = append(conns, conn{id: d.id, username: d.username, password: string(pw), calURL: calURL}) + } } return conns, nil } diff --git a/internal/caldav/calendars.go b/internal/caldav/calendars.go index 3a55808f..1973e748 100644 --- a/internal/caldav/calendars.go +++ b/internal/caldav/calendars.go @@ -3,27 +3,155 @@ package caldav import ( "context" "database/sql" + "fmt" "github.com/calnode/calnode/internal/calendar" ) -// ListCalendars returns the connected CalDAV calendar for the account. Discovery currently -// binds one calendar per connection; enumerating multiple CalDAV collections is a future -// enhancement, so this reports the single bound calendar. +// ListCalendars lists every calendar in the account that can hold events: each VEVENT-capable +// collection under the account's calendar home, on the same origin as that home (see +// listCollections). The collection the connection is bound to is marked Primary; Writable comes +// from the server's DAV:current-user-privilege-set, and counts as true when the server does not +// report one. +// +// The server URL typed at connect time is not stored, so discovery restarts from the bound +// collection (see homeForCalendar) using the stored credentials. Nothing new is persisted, which +// is why this needs no migration and works for connections made before calendars were listed. +// +// Every request made here stays on the bound collection's origin, and so does every calendar +// offered: this runs on each picker load and each saved selection, with the account's +// credentials, against URLs the server chose. homeForCalendar will not follow a principal or +// home on another origin, propfind refuses a redirect off it, and a collection off it is dropped +// below even if a listing somehow reached one. func (c *Client) ListCalendars(ctx context.Context, userID, accountEmail string) ([]calendar.CalendarInfo, error) { - var calID string + cn, ok, err := c.accountConn(ctx, userID, accountEmail) + if err != nil || !ok { + return nil, err + } + home, err := c.homeForCalendar(ctx, cn) + if err != nil { + return nil, err + } + l, err := c.listCollections(ctx, cn.calURL, home, cn.username, cn.password) + if err != nil { + return nil, err + } + + out := make([]calendar.CalendarInfo, 0, len(l.collections)+1) + boundListed := false + for _, col := range l.collections { + if !sameOrigin(col.url, cn.calURL) { + c.logger.Warn("caldav: skipping a calendar on another origin from the connected calendar", "connected", cn.calURL, "calendar", col.url) + continue + } + primary := col.url == cn.calURL + boundListed = boundListed || primary + out = append(out, calendar.CalendarInfo{ID: col.url, Name: col.name(), Primary: primary, Writable: col.writable}) + } + if !boundListed { + // The bound collection is what free/busy reads until a selection is saved, and it is + // already the URL this connection sends its credentials to, so it stays selectable even + // when the home listing leaves it out (a home found by the parent fallback, say). + // Nothing is known about it beyond what connect found, which is what this method + // reported before calendars were listed. + bound := calendar.CalendarInfo{ID: cn.calURL, Name: lastSegment(cn.calURL), Primary: true, Writable: true} + out = append([]calendar.CalendarInfo{bound}, out...) + } + return out, nil +} + +var _ calendar.SelectionValidator = (*Client)(nil) + +// ValidateSelection accepts a selection only when every calendar id is one ListCalendars offers +// for the account right now. A CalDAV calendar id is the collection URL that free/busy and +// booking writes send the account's credentials to, so an id the client made up would bypass +// both the connect-time URL check and the same-origin rule applied when listing. +func (c *Client) ValidateSelection(ctx context.Context, userID, accountEmail string, calendarIDs []string) error { + cals, err := c.ListCalendars(ctx, userID, accountEmail) + if err != nil { + return err + } + listed := make(map[string]bool, len(cals)) + for _, cal := range cals { + listed[cal.ID] = true + } + for _, id := range calendarIDs { + if !listed[id] { + return &calendar.UnknownCalendarError{CalendarID: id} + } + } + return nil +} + +// homeForCalendar finds the calendar home holding the connection's bound collection. +// DAV:current-user-principal is defined on every resource (RFC 5397), and the principal names +// its calendar-home-set, so this is the same walk connect makes, started from the collection +// instead of the server URL. A server that answers neither gets the collection's parent, which is +// the calendar home on the usual layout (.../calendars///). +// +// A failed principal or home lookup falls through to the parent rather than failing, so a bound +// collection that has since been deleted does not stop the user choosing another one. An +// authentication failure still surfaces: the parent listing is made with the same credentials. +// +// A principal or home href on another origin from the bound collection is not followed, and +// neither is a redirect off it (propfind is pinned to the bound collection). Following either +// would send the account's credentials there on every picker load, and a home there would make +// that origin's collections look local to it. Both fall through to the parent, which is on the +// bound collection's origin by construction. +func (c *Client) homeForCalendar(ctx context.Context, cn conn) (string, error) { + parent := parentCollection(cn.calURL) + ms, reqURL, err := c.propfind(ctx, cn.calURL, cn.calURL, cn.username, cn.password, "0", propCurrentUserPrincipal) + if err != nil { + c.logger.Warn("caldav: principal lookup failed, listing the bound calendar's parent", "calendar", cn.calURL, "error", err) + return parent, nil + } + var principal string + for _, r := range ms.Responses { + if h := r.okProp().CurrentUserPrincipal.Href; h != "" { + principal = resolveRef(reqURL, h) + break + } + } + if principal == "" { + return parent, nil + } + if !sameOrigin(principal, cn.calURL) { + c.logger.Warn("caldav: not following a principal on another origin, listing the bound calendar's parent", "calendar", cn.calURL, "principal", principal) + return parent, nil + } + home, err := c.calendarHome(ctx, cn.calURL, principal, cn.username, cn.password) + switch { + case err != nil: + c.logger.Warn("caldav: calendar home lookup failed, listing the bound calendar's parent", "principal", principal, "error", err) + case home == "": + case !sameOrigin(home, cn.calURL): + c.logger.Warn("caldav: not following a calendar home on another origin, listing the bound calendar's parent", "calendar", cn.calURL, "home", home) + default: + return home, nil + } + return parent, nil +} + +// accountConn loads one account's connection (by account email) regardless of its +// check_conflicts / is_destination flags. ok=false when the user has no such account. +func (c *Client) accountConn(ctx context.Context, userID, accountEmail string) (conn, bool, error) { + var cn conn + var pwEnc string err := c.db.QueryRowContext(ctx, ` - SELECT calendar_id FROM calendar_connections + SELECT id, COALESCE(account_email,''), access_token_enc, calendar_id + FROM calendar_connections WHERE user_id = ? AND provider = 'caldav' AND COALESCE(account_email,'') = ? - LIMIT 1`, userID, accountEmail).Scan(&calID) + LIMIT 1`, userID, accountEmail).Scan(&cn.id, &cn.username, &pwEnc, &cn.calURL) if err == sql.ErrNoRows { - return nil, nil + return conn{}, false, nil } if err != nil { - return nil, err + return conn{}, false, fmt.Errorf("caldav: load account connection: %w", err) + } + pw, err := c.decrypt(pwEnc) + if err != nil { + return conn{}, false, fmt.Errorf("caldav: decrypt password: %w", err) } - // CalDAV exposes exactly the one collection the user configured, and they configured it - // precisely so Calnode could write there - so unlike Google/Microsoft there is no - // read-only case to detect here. - return []calendar.CalendarInfo{{ID: calID, Name: "Calendar", Primary: true, Writable: true}}, nil + cn.password = string(pw) + return cn, true, nil } diff --git a/internal/caldav/calendars_test.go b/internal/caldav/calendars_test.go new file mode 100644 index 00000000..338e6566 --- /dev/null +++ b/internal/caldav/calendars_test.go @@ -0,0 +1,620 @@ +package caldav + +import ( + "context" + "errors" + "fmt" + "io" + "net/http" + "net/http/httptest" + "reflect" + "slices" + "sort" + "strings" + "sync" + "testing" + "time" + + "github.com/calnode/calnode/internal/calendar" +) + +// multiCalServer is one CalDAV account whose calendar home holds every kind of collection the +// calendar listing has to tell apart: +// +// /calendars/user/elsewhere/ an href on ANOTHER origin (a second server), listed first +// /calendars/user/personal/ "Personal", VEVENT, write privilege: the bound calendar +// /calendars/user/team/ no display name, VEVENT+VTODO, privileges not reported +// /calendars/user/tasks/ "Tasks", VTODO only +// /calendars/user/holidays/ "Holidays", VEVENT, read + write-properties only +// +// Every request is recorded, and any request reaching the other origin is counted. The other +// origin is a complete CalDAV account of its own (principal, home, and one writable "Stolen" +// calendar), so a listing that wandered there would find something to offer. +type multiCalServer struct { + srv *httptest.Server + other *httptest.Server + + mu sync.Mutex // guards everything below, including the switches set by tests + + // Switches for requests made after connect (a request to "/" is connect's own discovery). + // noPrincipalOnCollections makes a PROPFIND for the principal fail, as on a server that + // does not implement RFC 5397 on calendar collections. principalElsewhere and homeElsewhere + // answer with an href on the other origin, and homeRedirectsElsewhere makes the Depth 1 + // listing of the home a 301 to the other origin. + noPrincipalOnCollections bool + principalElsewhere bool + homeElsewhere bool + homeRedirectsElsewhere bool + + reqs []string // "METHOD /path" + events map[string]string // path -> stored iCalendar body + otherHits int +} + +func newMultiCalServer(t *testing.T) *multiCalServer { + t.Helper() + fs := &multiCalServer{events: map[string]string{}} + fs.other = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + fs.mu.Lock() + fs.otherHits++ + fs.mu.Unlock() + w.WriteHeader(http.StatusMultiStatus) + switch { + case strings.Contains(string(body), "current-user-principal"): + io.WriteString(w, msOpen+okResponse(r.URL.Path, `/principals/user/`)+``) + case strings.Contains(string(body), "calendar-home-set"): + io.WriteString(w, msOpen+okResponse(r.URL.Path, `/calendars/user/`)+``) + default: + io.WriteString(w, msOpen+okResponse("/calendars/user/stolen/", calendarProps("Stolen", vevent, fmt.Sprintf(priv, "all")))+``) + } + })) + t.Cleanup(fs.other.Close) + fs.srv = httptest.NewServer(http.HandlerFunc(fs.serve)) + t.Cleanup(fs.srv.Close) + return fs +} + +func (fs *multiCalServer) url(p string) string { return fs.srv.URL + p } + +// set flips switches under the lock the handler reads them with. +func (fs *multiCalServer) set(f func(fs *multiCalServer)) { + fs.mu.Lock() + defer fs.mu.Unlock() + f(fs) +} + +func (fs *multiCalServer) reset() { + fs.mu.Lock() + defer fs.mu.Unlock() + fs.reqs = nil +} + +// paths returns the sorted paths of recorded requests with the given method. +func (fs *multiCalServer) paths(method string) []string { + fs.mu.Lock() + defer fs.mu.Unlock() + var out []string + for _, r := range fs.reqs { + if m, p, _ := strings.Cut(r, " "); m == method { + out = append(out, p) + } + } + sort.Strings(out) + return out +} + +func (fs *multiCalServer) hitsElsewhere() int { + fs.mu.Lock() + defer fs.mu.Unlock() + return fs.otherHits +} + +const msOpen = `` + +func okResponse(href, props string) string { + return `` + href + `HTTP/1.1 200 OK` + + props + `` +} + +func calendarProps(name, comps, privileges string) string { + p := `` + if name != "" { + p += `` + name + `` + } + p += `` + comps + `` + if privileges != "" { + p += `` + privileges + `` + } + return p +} + +const ( + vevent = `` + vtodo = `` + priv = `` +) + +// busyHour is the hour (UTC, 2026-06-25) of the one event each calendar returns, so a FreeBusy +// result says which calendars were read. +var busyHour = map[string]int{ + "/calendars/user/personal/": 9, + "/calendars/user/team/": 10, + "/calendars/user/holidays/": 11, +} + +func (fs *multiCalServer) serve(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + fs.mu.Lock() + fs.reqs = append(fs.reqs, r.Method+" "+r.URL.Path) + noPrincipal, principalElsewhere := fs.noPrincipalOnCollections, fs.principalElsewhere + homeElsewhere, homeRedirects := fs.homeElsewhere, fs.homeRedirectsElsewhere + fs.mu.Unlock() + afterConnect := r.URL.Path != "/" + + multistatus := func(inner string) { + w.WriteHeader(http.StatusMultiStatus) + io.WriteString(w, msOpen+inner+``) + } + switch { + case r.Method == "PROPFIND" && strings.Contains(string(body), "current-user-principal"): + if noPrincipal && afterConnect { + w.WriteHeader(http.StatusNotFound) + return + } + principal := "/principals/user/" + if principalElsewhere && afterConnect { + principal = fs.other.URL + principal + } + multistatus(okResponse(r.URL.Path, ``+principal+``)) + case r.Method == "PROPFIND" && strings.Contains(string(body), "calendar-home-set") && r.URL.Path == "/principals/user/": + home := "/calendars/user/" + if homeElsewhere { + home = fs.other.URL + home + } + multistatus(okResponse(r.URL.Path, ``+home+``)) + case r.Method == "PROPFIND" && r.URL.Path == "/calendars/user/" && r.Header.Get("Depth") == "1" && homeRedirects: + w.Header().Set("Location", fs.other.URL+"/calendars/user/") + w.WriteHeader(http.StatusMovedPermanently) + case r.Method == "PROPFIND" && r.URL.Path == "/calendars/user/" && r.Header.Get("Depth") == "1": + multistatus( + okResponse("/calendars/user/", ``) + + okResponse(fs.other.URL+"/calendars/user/elsewhere/", + calendarProps("Elsewhere", vevent, fmt.Sprintf(priv, "read")+fmt.Sprintf(priv, "write"))) + + okResponse("/calendars/user/personal/", + calendarProps("Personal", vevent, fmt.Sprintf(priv, "read")+fmt.Sprintf(priv, "write"))) + + // Privileges reported under a 404 propstat, listed before the 200 one: not reported. + `/calendars/user/team/` + + `HTTP/1.1 404 Not Found` + + `HTTP/1.1 200 OK` + calendarProps("", vevent+vtodo, "") + `` + + `` + + okResponse("/calendars/user/tasks/", + calendarProps("Tasks", vtodo, fmt.Sprintf(priv, "all"))) + + okResponse("/calendars/user/holidays/", + calendarProps("Holidays", vevent, fmt.Sprintf(priv, "read")+fmt.Sprintf(priv, "write-properties"))), + ) + case r.Method == "REPORT": + hour, ok := busyHour[r.URL.Path] + if !ok { + w.WriteHeader(http.StatusNotFound) + return + } + multistatus(okResponse(r.URL.Path+"ev.ics", fmt.Sprintf(`BEGIN:VCALENDAR +BEGIN:VEVENT +UID:%[1]d@x +DTSTART:20260625T%02[1]d0000Z +DTEND:20260625T%02[2]d0000Z +END:VEVENT +END:VCALENDAR`, hour, hour+1))) + case r.Method == http.MethodPut: + fs.mu.Lock() + fs.events[r.URL.Path] = string(body) + fs.mu.Unlock() + w.WriteHeader(http.StatusCreated) + case r.Method == http.MethodGet: + fs.mu.Lock() + ev, ok := fs.events[r.URL.Path] + fs.mu.Unlock() + if !ok { + w.WriteHeader(http.StatusNotFound) + return + } + w.Header().Set("ETag", `"1"`) + io.WriteString(w, ev) + case r.Method == http.MethodDelete: + w.WriteHeader(http.StatusNoContent) + default: + w.WriteHeader(http.StatusNotFound) + } +} + +const multiCalAccount = "user@example.com" + +// connectMultiCal connects user u1 to a fresh multiCalServer and clears the request log. +func connectMultiCal(t *testing.T) (*Client, *multiCalServer) { + t.Helper() + c := newTestClient(t) + seedUser(t, c.db, "u1") + fs := newMultiCalServer(t) + _, bound, err := c.Connect(context.Background(), "u1", fs.srv.URL, multiCalAccount, "app-pw") + if err != nil { + t.Fatalf("Connect: %v", err) + } + if want := fs.url("/calendars/user/personal/"); bound != want { + t.Fatalf("connect bound %q, want %q (the first same-origin VEVENT calendar)", bound, want) + } + fs.reset() + return c, fs +} + +func selectCalendars(t *testing.T, c *Client, sels ...calendar.CalendarSelection) { + t.Helper() + svc := calendar.NewService(c.db) + svc.Register(c) + if err := svc.SetAccountCalendars(context.Background(), "u1", "caldav", multiCalAccount, sels); err != nil { + t.Fatalf("SetAccountCalendars: %v", err) + } +} + +func sel(id string, check, dest bool) calendar.CalendarSelection { + return calendar.CalendarSelection{CalendarInfo: calendar.CalendarInfo{ID: id}, CheckConflicts: check, IsDestination: dest} +} + +func busyHours(t *testing.T, c *Client) []int { + t.Helper() + day := time.Date(2026, 6, 25, 0, 0, 0, 0, time.UTC) + busy, err := c.FreeBusy(context.Background(), "u1", day, day.Add(24*time.Hour)) + if err != nil { + t.Fatalf("FreeBusy: %v", err) + } + var hours []int + for _, iv := range busy { + hours = append(hours, iv.Start.Hour()) + } + sort.Ints(hours) + return hours +} + +// Issue #42: a CalDAV account with several calendars only ever showed the one it was bound to. +func TestListCalendars_listsEveryEventCalendarOnTheAccount(t *testing.T) { + c, fs := connectMultiCal(t) + + got, err := c.ListCalendars(context.Background(), "u1", multiCalAccount) + if err != nil { + t.Fatalf("ListCalendars: %v", err) + } + want := []calendar.CalendarInfo{ + {ID: fs.url("/calendars/user/personal/"), Name: "Personal", Primary: true, Writable: true}, + // No display name: named by its path. Privileges not reported: writable. + {ID: fs.url("/calendars/user/team/"), Name: "team", Primary: false, Writable: true}, + // read + write-properties is a read-only share. + {ID: fs.url("/calendars/user/holidays/"), Name: "Holidays", Primary: false, Writable: false}, + } + if !reflect.DeepEqual(got, want) { + t.Errorf("ListCalendars =\n %+v\nwant (no VTODO-only list, nothing from another origin)\n %+v", got, want) + } + // Discovery restarted from the stored collection, not a server URL that is never stored. + if p := fs.paths("PROPFIND"); !slices.Contains(p, "/calendars/user/personal/") { + t.Errorf("PROPFINDs = %v, want discovery to start at the bound collection", p) + } + if n := fs.hitsElsewhere(); n != 0 { + t.Errorf("%d request(s) reached the other origin; its collection must be skipped, not contacted", n) + } +} + +// A server that does not answer current-user-principal on a calendar collection still gets its +// calendars listed, from the bound collection's parent. +func TestListCalendars_fallsBackToTheBoundCalendarsParent(t *testing.T) { + c, fs := connectMultiCal(t) + fs.set(func(fs *multiCalServer) { fs.noPrincipalOnCollections = true }) + + got, err := c.ListCalendars(context.Background(), "u1", multiCalAccount) + if err != nil { + t.Fatalf("ListCalendars: %v", err) + } + if len(got) != 3 || !got[0].Primary { + t.Errorf("ListCalendars = %+v, want the same three calendars with the bound one primary", got) + } + if slices.Contains(fs.paths("PROPFIND"), "/principals/user/") { + t.Error("expected no principal walk when the collection reports no principal") + } +} + +// A stored app password the server now rejects is a reconnect, not an unreachable server. +func TestListCalendars_rejectedPasswordNeedsAReconnect(t *testing.T) { + c := newTestClient(t) + seedUser(t, c.db, "u1") + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + })) + defer srv.Close() + if err := c.saveConnection(context.Background(), "u1", multiCalAccount, "revoked", srv.URL+"/calendars/user/personal/"); err != nil { + t.Fatalf("saveConnection: %v", err) + } + _, err := c.ListCalendars(context.Background(), "u1", multiCalAccount) + if !calendar.IsReauthErr(err) { + t.Errorf("err = %v, want one IsReauthErr recognises", err) + } + if err == nil || !strings.Contains(err.Error(), "authentication failed") { + t.Errorf("err = %v, want the connect form's authentication message kept", err) + } +} + +func TestFreeBusy_readsExactlyTheCalendarsSelectedForConflicts(t *testing.T) { + c, fs := connectMultiCal(t) + selectCalendars(t, c, + sel(fs.url("/calendars/user/personal/"), false, true), // unticked: never read + sel(fs.url("/calendars/user/team/"), true, false), + sel(fs.url("/calendars/user/holidays/"), true, false), // read-only is fine for conflicts + ) + fs.reset() + + if got, want := busyHours(t, c), []int{10, 11}; !reflect.DeepEqual(got, want) { + t.Errorf("busy hours = %v, want %v (team + holidays)", got, want) + } + if got, want := fs.paths("REPORT"), []string{"/calendars/user/holidays/", "/calendars/user/team/"}; !reflect.DeepEqual(got, want) { + t.Errorf("REPORTs = %v, want exactly %v", got, want) + } + if p := fs.paths("PROPFIND"); len(p) != 0 { + t.Errorf("free/busy made discovery requests %v; it must read stored selections only", p) + } +} + +// Connections that never saved a selection keep reading only the calendar they were bound to, +// whether they have no selection rows at all or the one row migration 00049 seeded. +func TestFreeBusy_withoutASavedSelectionReadsOnlyTheBoundCalendar(t *testing.T) { + for _, tc := range []struct { + name string + seed bool + }{{"no selection rows", false}, {"row seeded by migration 00049", true}} { + t.Run(tc.name, func(t *testing.T) { + c, fs := connectMultiCal(t) + if tc.seed { + if _, err := c.db.Exec(` + INSERT INTO connection_calendars (id, user_id, provider, account_email, calendar_id, name, check_conflicts, is_destination) + SELECT 'seeded', user_id, provider, account_email, calendar_id, '', check_conflicts, is_destination + FROM calendar_connections WHERE user_id = 'u1'`); err != nil { + t.Fatalf("seed: %v", err) + } + } + + if got, want := busyHours(t, c), []int{9}; !reflect.DeepEqual(got, want) { + t.Errorf("busy hours = %v, want %v (the bound calendar only)", got, want) + } + if got, want := fs.paths("REPORT"), []string{"/calendars/user/personal/"}; !reflect.DeepEqual(got, want) { + t.Errorf("REPORTs = %v, want exactly %v", got, want) + } + }) + } +} + +func TestCreateEvent_writesIntoTheChosenCalendar(t *testing.T) { + c, fs := connectMultiCal(t) + team := fs.url("/calendars/user/team/") + selectCalendars(t, c, + sel(fs.url("/calendars/user/personal/"), true, false), + sel(team, true, true), + ) + fs.reset() + + start := time.Date(2026, 6, 26, 15, 0, 0, 0, time.UTC) + eventID, _, calID, err := c.CreateEvent(context.Background(), "u1", calendar.CreateEventParams{ + Summary: "Intro call", Start: start, End: start.Add(time.Hour), + }) + if err != nil { + t.Fatalf("CreateEvent: %v", err) + } + puts := fs.paths(http.MethodPut) + if len(puts) != 1 || !strings.HasPrefix(puts[0], "/calendars/user/team/") { + t.Fatalf("PUTs = %v, want one into /calendars/user/team/", puts) + } + if eventID != fs.url(puts[0]) { + t.Errorf("eventID = %q, want the resource URL written %q", eventID, fs.url(puts[0])) + } + if calID != team { + t.Errorf("calendarID = %q, want %q recorded against the booking", calID, team) + } + + // The destination moves back to Personal. The booking's event still lives in Team, and + // moving or cancelling it must act on the stored resource URL. + selectCalendars(t, c, + sel(fs.url("/calendars/user/personal/"), true, true), + sel(team, true, false), + ) + fs.reset() + if err := c.UpdateEvent(context.Background(), "u1", calID, eventID, start.Add(time.Hour), start.Add(2*time.Hour)); err != nil { + t.Fatalf("UpdateEvent: %v", err) + } + if err := c.CancelEvent(context.Background(), "u1", calID, eventID); err != nil { + t.Fatalf("CancelEvent: %v", err) + } + for _, m := range []string{http.MethodGet, http.MethodPut, http.MethodDelete} { + if got := fs.paths(m); !reflect.DeepEqual(got, puts) { + t.Errorf("%s paths = %v, want %v (the stored event URL)", m, got, puts) + } + } +} + +// A calendar id is a URL that later receives the account's credentials, so a selection may only +// name calendars the server listed. Nothing is saved when any id is refused, and validating never +// contacts the refused URL. +func TestSetAccountCalendars_refusesACalendarTheServerDidNotList(t *testing.T) { + for _, tc := range []struct { + name string + id func(fs *multiCalServer) string + }{ + {"VTODO-only list", func(fs *multiCalServer) string { return fs.url("/calendars/user/tasks/") }}, + {"collection on another origin", func(fs *multiCalServer) string { return fs.other.URL + "/calendars/user/elsewhere/" }}, + {"URL the server never listed", func(fs *multiCalServer) string { return fs.url("/calendars/someone-else/private/") }}, + } { + t.Run(tc.name, func(t *testing.T) { + c, fs := connectMultiCal(t) + svc := calendar.NewService(c.db) + svc.Register(c) + id := tc.id(fs) + + err := svc.SetAccountCalendars(context.Background(), "u1", "caldav", multiCalAccount, + []calendar.CalendarSelection{sel(fs.url("/calendars/user/personal/"), true, false), sel(id, true, true)}) + var unknown *calendar.UnknownCalendarError + if !errors.As(err, &unknown) || unknown.CalendarID != id { + t.Fatalf("err = %v, want *UnknownCalendarError for %q", err, id) + } + var n int + if err := c.db.QueryRow(`SELECT COUNT(*) FROM connection_calendars WHERE user_id = 'u1'`).Scan(&n); err != nil { + t.Fatal(err) + } + if n != 0 { + t.Errorf("%d selection row(s) saved despite the refusal", n) + } + if n := fs.hitsElsewhere(); n != 0 { + t.Errorf("%d request(s) reached the other origin while validating", n) + } + }) + } +} + +// Listing runs on every picker load and every saved CalDAV selection, with the account's +// credentials, against hrefs and redirects the server chooses. A principal, a calendar home or a +// redirect on another origin must not receive a single request, and nothing there may be offered +// by ListCalendars or accepted by ValidateSelection. +func TestListCalendars_staysOnTheConnectedCalendarsOrigin(t *testing.T) { + for _, tc := range []struct { + name string + set func(fs *multiCalServer) + wantErr string // "" when the listing should still succeed from the bound calendar's parent + }{ + {"principal on another origin", func(fs *multiCalServer) { fs.principalElsewhere = true }, ""}, + {"calendar home on another origin", func(fs *multiCalServer) { fs.homeElsewhere = true }, ""}, + {"home listing redirects to another origin", func(fs *multiCalServer) { fs.homeRedirectsElsewhere = true }, + "refusing to follow a redirect"}, + } { + t.Run(tc.name, func(t *testing.T) { + c, fs := connectMultiCal(t) + fs.set(tc.set) + ctx := context.Background() + stolen := fs.other.URL + "/calendars/user/stolen/" + + cals, err := c.ListCalendars(ctx, "u1", multiCalAccount) + if tc.wantErr != "" { + if err == nil || !strings.Contains(err.Error(), tc.wantErr) { + t.Errorf("ListCalendars err = %v, want one containing %q", err, tc.wantErr) + } + } else { + if err != nil { + t.Fatalf("ListCalendars: %v", err) + } + var ids []string + for _, cal := range cals { + ids = append(ids, cal.ID) + } + want := []string{fs.url("/calendars/user/personal/"), fs.url("/calendars/user/team/"), fs.url("/calendars/user/holidays/")} + if !slices.Equal(ids, want) { + t.Errorf("ListCalendars ids = %v, want the account's own calendars %v", ids, want) + } + } + + if err := c.ValidateSelection(ctx, "u1", multiCalAccount, []string{stolen}); err == nil { + t.Errorf("ValidateSelection accepted %s", stolen) + } + svc := calendar.NewService(c.db) + svc.Register(c) + if err := svc.SetAccountCalendars(ctx, "u1", "caldav", multiCalAccount, + []calendar.CalendarSelection{sel(stolen, true, true)}); err == nil { + t.Errorf("SetAccountCalendars saved %s", stolen) + } + var n int + if err := c.db.QueryRow(`SELECT COUNT(*) FROM connection_calendars WHERE user_id = 'u1'`).Scan(&n); err != nil { + t.Fatal(err) + } + if n != 0 { + t.Errorf("%d selection row(s) saved", n) + } + if n := fs.hitsElsewhere(); n != 0 { + t.Errorf("%d request(s) reached the other origin with the account's credentials", n) + } + }) + } +} + +// A self-hosted server behind a TLS-terminating reverse proxy can report its internal scheme or +// host in every href. Those calendars are skipped for origin, and connect must say that is why it +// found nothing, rather than "no calendar found", which points the operator at the wrong thing. +// A server that genuinely has no event calendar keeps the old message. +func TestConnect_explainsCalendarsListedOnlyAtAnotherAddress(t *testing.T) { + for _, tc := range []struct { + name string + hrefBase func(r *http.Request) string // prefix for each calendar href + comps string + want func(host string) string + }{ + { + name: "every event calendar on another origin", + hrefBase: func(r *http.Request) string { return "https://" + r.Host }, + comps: vevent, + want: func(host string) string { + return "the server lists its calendars at a different address (https://" + host + + ") than the one Calnode reached it at (http://" + host + "); check the server's reverse proxy or base URL settings" + }, + }, + { + name: "no event calendar at all", + hrefBase: func(*http.Request) string { return "" }, + comps: vtodo, + want: func(string) string { return "no writable calendar found on the server" }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + c := newTestClient(t) + seedUser(t, c.db, "u1") + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + w.WriteHeader(http.StatusMultiStatus) + switch { + case strings.Contains(string(body), "current-user-principal"): + io.WriteString(w, msOpen+okResponse(r.URL.Path, `/principals/user/`)+``) + case strings.Contains(string(body), "calendar-home-set"): + io.WriteString(w, msOpen+okResponse(r.URL.Path, `/calendars/user/`)+``) + default: + base := tc.hrefBase(r) + io.WriteString(w, msOpen+ + okResponse("/calendars/user/", ``)+ + okResponse(base+"/calendars/user/personal/", calendarProps("Personal", tc.comps, ""))+ + okResponse(base+"/calendars/user/work/", calendarProps("Work", tc.comps, ""))+ + // A same-origin tasks list must not stop the origin explanation. + okResponse("/calendars/user/tasks/", calendarProps("Tasks", vtodo, ""))+ + ``) + } + })) + defer srv.Close() + + _, _, err := c.Connect(context.Background(), "u1", srv.URL, multiCalAccount, "app-pw") + if err == nil { + t.Fatal("Connect succeeded, want an error") + } + if want := tc.want(strings.TrimPrefix(srv.URL, "http://")); !strings.Contains(err.Error(), want) { + t.Errorf("err = %q\nwant it to contain %q", err, want) + } + if n := countConns(t, c.db, "u1"); n != 0 { + t.Errorf("%d connection(s) saved by a failed connect", n) + } + }) + } +} + +func TestSameOrigin(t *testing.T) { + for _, tc := range []struct { + a, b string + want bool + }{ + {"https://dav.example.com/cal/", "https://dav.example.com:443/other/", true}, + {"http://dav.example.com/cal/", "http://DAV.example.com:80/", true}, + {"https://dav.example.com/cal/", "http://dav.example.com/cal/", false}, + {"https://dav.example.com/cal/", "https://dav.example.com:8443/cal/", false}, + {"https://dav.example.com/cal/", "https://evil.example.com/cal/", false}, + {"/relative/", "/relative/", false}, + } { + if got := sameOrigin(tc.a, tc.b); got != tc.want { + t.Errorf("sameOrigin(%q, %q) = %v, want %v", tc.a, tc.b, got, tc.want) + } + } +} diff --git a/internal/caldav/discovery.go b/internal/caldav/discovery.go index 874e46d5..a65385d9 100644 --- a/internal/caldav/discovery.go +++ b/internal/caldav/discovery.go @@ -24,7 +24,7 @@ const ( ` propCalendarCollections = ` -` +` ) // Connect validates the given CalDAV credentials by discovering the user's default calendar @@ -49,59 +49,124 @@ func (c *Client) Connect(ctx context.Context, userID, serverURL, username, passw } // discoverCalendar walks principal → calendar-home → calendar collections and returns the URL -// of the first VEVENT-capable calendar collection. +// of the collection the connection binds to: the account's default, which free/busy reads and +// bookings are written to until the user picks calendars of their own. func (c *Client) discoverCalendar(ctx context.Context, serverURL, username, password string) (string, error) { // 1. current-user-principal — try the base URL, then the RFC 5785 well-known path. - principal, base, err := c.findPrincipal(ctx, serverURL, username, password) + principal, _, err := c.findPrincipal(ctx, serverURL, username, password) if err != nil { return "", err } // 2. calendar-home-set on the principal. - ms, homeReqURL, err := c.propfind(ctx, principal, username, password, "0", propCalendarHomeSet) + home, err := c.calendarHome(ctx, "", principal, username, password) if err != nil { return "", err } - var home string - for _, r := range ms.Responses { - if h := r.okProp().CalendarHomeSet.Href; h != "" { - home = resolveRef(homeReqURL, h) - break - } - } if home == "" { // Some servers expose the calendar home at the principal itself. home = principal } - // 3. list calendar collections (Depth 1) and pick the first that supports VEVENT. - ms, homeReqURL, err = c.propfind(ctx, home, username, password, "1", propCalendarCollections) + // 3. list calendar collections (Depth 1) and pick the default among them. + l, err := c.listCollections(ctx, "", home, username, password) if err != nil { return "", err } - var firstCal string + cols := l.collections + if len(cols) == 0 && len(l.foreign) > 0 { + // Every event calendar was on another origin. That is the signature of a self-hosted + // server behind a reverse proxy that reports its internal scheme or host in hrefs, and + // "no calendar found" would send the operator looking in the wrong place. + return "", fmt.Errorf("caldav: the server lists its calendars at a different address (%s) than the one Calnode reached it at (%s); check the server's reverse proxy or base URL settings", + displayOrigin(l.foreign[0]), displayOrigin(l.homeURL)) + } + if len(cols) == 0 { + return "", fmt.Errorf("caldav: no writable calendar found on the server") + } + for _, col := range cols { + // Prefer a calendar literally named/pathed "calendar" as the default when present. + if strings.EqualFold(col.displayName, "Calendar") || strings.Contains(strings.ToLower(col.url), "/calendar") { + return col.url, nil + } + } + return cols[0].url, nil +} + +// calendarHome returns the calendar-home-set a principal reports (RFC 4791 §6.2.1), or "" when +// it reports none. pinOrigin is passed to propfind. +func (c *Client) calendarHome(ctx context.Context, pinOrigin, principal, username, password string) (string, error) { + ms, reqURL, err := c.propfind(ctx, pinOrigin, principal, username, password, "0", propCalendarHomeSet) + if err != nil { + return "", err + } + for _, r := range ms.Responses { + if h := r.okProp().CalendarHomeSet.Href; h != "" { + return resolveRef(reqURL, h), nil + } + } + return "", nil +} + +// collection is one calendar collection that can hold events, as listed under a calendar home. +type collection struct { + url string + displayName string // as reported; "" when the server gave none + writable bool +} + +// name is what the calendar picker shows: the display name, else the last path segment. +func (col collection) name() string { + if n := strings.TrimSpace(col.displayName); n != "" { + return n + } + return lastSegment(col.url) +} + +// listing is what one Depth 1 PROPFIND of a calendar home returned. +type listing struct { + homeURL string // the URL that answered, after any redirects + collections []collection // same-origin, VEVENT-capable, in server order + foreign []string // VEVENT-capable collections skipped for being on another origin +} + +// listCollections lists the VEVENT-capable calendar collections directly under a calendar home, +// in the order the server returns them. +// +// A collection whose URL is on a different origin (scheme, host, port) from the home URL that +// listed it is logged and skipped. Every URL returned here can be stored as a calendar id, and a +// stored id is then sent the account's Basic credentials on every free/busy read and booking +// write. The server already sees those credentials; an href must not be able to make them go to +// some other host for as long as the selection is saved. Skipped URLs are reported in foreign so +// connect can say why it found nothing. pinOrigin is passed to propfind. +func (c *Client) listCollections(ctx context.Context, pinOrigin, home, username, password string) (listing, error) { + ms, homeReqURL, err := c.propfind(ctx, pinOrigin, home, username, password, "1", propCalendarCollections) + if err != nil { + return listing{}, err + } + l := listing{homeURL: homeReqURL} + seen := map[string]bool{} for _, r := range ms.Responses { p := r.okProp() if p.ResourceType.Calendar == nil { continue // not a calendar collection (e.g. the home container itself) } if !supportsVEvent(p.SupportedComps) { - continue // e.g. a tasks/reminders or birthdays calendar + continue // e.g. a tasks/reminders list } u := resolveRef(homeReqURL, r.Href) - if firstCal == "" { - firstCal = u + if !sameOrigin(u, homeReqURL) { + c.logger.Warn("caldav: skipping a calendar listed on another origin", "home", homeReqURL, "calendar", u) + l.foreign = append(l.foreign, u) + continue } - // Prefer a calendar literally named/pathed "calendar" as the default when present. - if strings.EqualFold(p.DisplayName, "Calendar") || strings.Contains(strings.ToLower(u), "/calendar") { - return u, nil + if seen[u] { + continue // the picker keys rows by id, and the selection table is unique on it } + seen[u] = true + l.collections = append(l.collections, collection{url: u, displayName: p.DisplayName, writable: p.PrivilegeSet.canWrite()}) } - if firstCal == "" { - _ = base - return "", fmt.Errorf("caldav: no writable calendar found on the server") - } - return firstCal, nil + return l, nil } // findPrincipal resolves current-user-principal, trying the base URL first and then the @@ -113,7 +178,7 @@ func (c *Client) findPrincipal(ctx context.Context, serverURL, username, passwor } var lastErr error for _, cand := range candidates { - ms, reqURL, err := c.propfind(ctx, cand, username, password, "0", propCurrentUserPrincipal) + ms, reqURL, err := c.propfind(ctx, "", cand, username, password, "0", propCurrentUserPrincipal) if err != nil { lastErr = err continue diff --git a/internal/caldav/freebusy.go b/internal/caldav/freebusy.go index 186d1984..3677f6a4 100644 --- a/internal/caldav/freebusy.go +++ b/internal/caldav/freebusy.go @@ -30,10 +30,11 @@ const calendarQueryTmpl = ` ` -// FreeBusy returns the union of busy intervals across every CalDAV account the user has -// connected with check_conflicts = 1. Each account is queried with a CalDAV calendar-query -// REPORT; transparent and cancelled events don't block. Fail-open per account: a connection -// that errors is logged and skipped (a flaky calendar never blocks availability). +// FreeBusy returns the union of busy intervals across every calendar selected for conflicts in +// every CalDAV account the user has connected with check_conflicts = 1 (see conflictConns). Each +// calendar is queried with a CalDAV calendar-query REPORT; transparent and cancelled events +// don't block. Fail-open per calendar: one that errors is logged and skipped (a flaky calendar +// never blocks availability). func (c *Client) FreeBusy(ctx context.Context, userID string, from, to time.Time) ([]slots.Interval, error) { conns, err := c.conflictConns(ctx, userID) if err != nil { diff --git a/internal/caldav/webdav.go b/internal/caldav/webdav.go index de10cc9a..02447a9f 100644 --- a/internal/caldav/webdav.go +++ b/internal/caldav/webdav.go @@ -5,9 +5,13 @@ import ( "encoding/xml" "fmt" "io" + "net" "net/http" "net/url" + "path" "strings" + + "github.com/calnode/calnode/internal/calendar" ) // do issues one WebDAV request with HTTP Basic auth and returns the status, body, and any @@ -44,7 +48,14 @@ func (c *Client) do(ctx context.Context, method, rawURL, username, password, dep // propfind issues a PROPFIND, following up to 5 redirects with the method preserved, and // returns the parsed multistatus. A non-2xx terminal status is an error. -func (c *Client) propfind(ctx context.Context, rawURL, username, password, depth, body string) (*msMultistatus, string, error) { +// +// pinOrigin, when set, is a URL whose origin every redirect must stay on: a redirect elsewhere +// is refused before anything is sent to it, so the account's credentials cannot follow it. +// Listing an existing account's calendars pins to the connected calendar. Connect-time +// discovery passes "" and is unchanged: it starts from the URL the user typed, and discovery +// from there can legitimately cross hosts (iCloud's calendar home is on a per-account partition +// host, and a /.well-known/caldav redirect may point anywhere). +func (c *Client) propfind(ctx context.Context, pinOrigin, rawURL, username, password, depth, body string) (*msMultistatus, string, error) { cur := rawURL for hop := 0; hop < 6; hop++ { status, b, loc, err := c.do(ctx, "PROPFIND", cur, username, password, depth, body) @@ -56,11 +67,15 @@ func (c *Client) propfind(ctx context.Context, rawURL, username, password, depth if loc == "" { return nil, cur, fmt.Errorf("caldav: redirect without Location from %s", cur) } - cur = resolveRef(cur, loc) + next := resolveRef(cur, loc) + if pinOrigin != "" && !sameOrigin(next, pinOrigin) { + return nil, cur, fmt.Errorf("caldav: refusing to follow a redirect from %s to another origin (%s)", cur, displayOrigin(next)) + } + cur = next continue } if status == http.StatusUnauthorized || status == http.StatusForbidden { - return nil, cur, fmt.Errorf("caldav: authentication failed (status %d) — check the username and app password", status) + return nil, cur, &authError{status: status} } if status != http.StatusMultiStatus && status != http.StatusOK { return nil, cur, fmt.Errorf("caldav: PROPFIND %s returned status %d", cur, status) @@ -74,6 +89,18 @@ func (c *Client) propfind(ctx context.Context, rawURL, username, password, depth return nil, cur, fmt.Errorf("caldav: too many redirects resolving %s", rawURL) } +// authError is a 401/403 from the server. Its message is what the connect form shows. It also +// unwraps to calendar.ErrReauthRequired, because listing an existing account's calendars now +// talks to the server: an app password that has since been revoked must read as "reconnect +// this account", not as the server being unreachable. +type authError struct{ status int } + +func (e *authError) Error() string { + return fmt.Sprintf("caldav: authentication failed (status %d) — check the username and app password", e.status) +} + +func (e *authError) Unwrap() error { return calendar.ErrReauthRequired } + // resolveRef resolves a (possibly relative) href against a base request URL, returning an // absolute URL string. On parse failure it returns the ref unchanged. func resolveRef(base, ref string) string { @@ -88,6 +115,65 @@ func resolveRef(base, ref string) string { return b.ResolveReference(r).String() } +// sameOrigin reports whether two absolute URLs share a scheme, host and port. An omitted port +// equals the scheme's default, so https://h and https://h:443 are the same origin. A URL that +// does not parse, or has no scheme or host, matches nothing. +func sameOrigin(a, b string) bool { + oa, ob := origin(a), origin(b) + return oa != "" && oa == ob +} + +func origin(raw string) string { + u, err := url.Parse(raw) + if err != nil { + return "" + } + scheme, host := strings.ToLower(u.Scheme), strings.ToLower(u.Hostname()) + if scheme == "" || host == "" { + return "" + } + port := u.Port() + if port == "" { + switch scheme { + case "http": + port = "80" + case "https": + port = "443" + } + } + return scheme + "://" + net.JoinHostPort(host, port) +} + +// displayOrigin renders a URL's origin as an operator would type it (https://host, or +// http://host:5000), for error messages. +func displayOrigin(raw string) string { + u, err := url.Parse(raw) + if err != nil || u.Host == "" { + return raw + } + return u.Scheme + "://" + u.Host +} + +// lastSegment returns the final path segment of a collection URL ("work" for .../user/work/), +// used as a calendar's name when the server reports no display name. +func lastSegment(raw string) string { + u, err := url.Parse(raw) + if err != nil { + return raw + } + seg := path.Base(strings.TrimRight(u.Path, "/")) + if seg == "" || seg == "." || seg == "/" { + return raw + } + return seg +} + +// parentCollection returns the URL of the collection that contains raw (.../user/ for +// .../user/work/). +func parentCollection(raw string) string { + return resolveRef(strings.TrimRight(raw, "/"), "./") +} + // ----- WebDAV / CalDAV multistatus XML ----- type msMultistatus struct { @@ -112,6 +198,38 @@ type msProp struct { ResourceType resourceType `xml:"DAV: resourcetype"` SupportedComps supportedCompSet `xml:"urn:ietf:params:xml:ns:caldav supported-calendar-component-set"` CalendarData string `xml:"urn:ietf:params:xml:ns:caldav calendar-data"` + // nil when the server did not report the property (or reported it under a non-2xx status). + PrivilegeSet *privilegeSet `xml:"DAV: current-user-privilege-set"` +} + +// privilegeSet is DAV:current-user-privilege-set (RFC 3744 §5.4), the privileges the signed-in +// user holds on a resource. Only the ones that allow adding an event are decoded. +type privilegeSet struct { + Privileges []privilege `xml:"DAV: privilege"` +} + +type privilege struct { + All *struct{} `xml:"DAV: all"` + Write *struct{} `xml:"DAV: write"` + WriteContent *struct{} `xml:"DAV: write-content"` + Bind *struct{} `xml:"DAV: bind"` +} + +// canWrite reports whether events can be created in the collection. A set the server did not +// report counts as writable: that is what every CalDAV calendar was assumed to be before +// privileges were read, and a server that omits the property has given no reason to refuse. +// write-properties alone does not count, since servers grant it on read-only shares so the +// sharee can rename or recolour them. +func (s *privilegeSet) canWrite() bool { + if s == nil { + return true + } + for _, p := range s.Privileges { + if p.All != nil || p.Write != nil || p.WriteContent != nil || p.Bind != nil { + return true + } + } + return false } type hrefHolder struct { diff --git a/internal/calendar/autherr.go b/internal/calendar/autherr.go index 73b371f7..fa24e42f 100644 --- a/internal/calendar/autherr.go +++ b/internal/calendar/autherr.go @@ -16,11 +16,15 @@ var ErrReauthRequired = errors.New("calendar: reconnect required") // IsReauthErr reports whether err indicates the account's OAuth grant is no longer usable, as // opposed to a transient failure. It matches both a typed oauth2.RetrieveError and the string // form providers wrap it in (Google returns "invalid_grant"; Microsoft returns "invalid_grant" -// alongside an AADSTS code). +// alongside an AADSTS code), and anything wrapping ErrReauthRequired, which is how CalDAV +// reports a server that rejects the stored app password. func IsReauthErr(err error) bool { if err == nil { return false } + if errors.Is(err, ErrReauthRequired) { + return true + } var re *oauth2.RetrieveError if errors.As(err, &re) { switch re.ErrorCode { diff --git a/internal/calendar/autherr_test.go b/internal/calendar/autherr_test.go index 67ae7bf5..1195b27f 100644 --- a/internal/calendar/autherr_test.go +++ b/internal/calendar/autherr_test.go @@ -20,6 +20,7 @@ func TestIsReauthErr(t *testing.T) { errors.New(`oauth2: "invalid_grant" "AADSTS70000: Account security interrupt..."`)), true}, {"typed retrieve error", &oauth2.RetrieveError{ErrorCode: "invalid_grant"}, true}, {"typed interaction required", &oauth2.RetrieveError{ErrorCode: "interaction_required"}, true}, + {"wraps ErrReauthRequired", fmt.Errorf("caldav: list: %w", ErrReauthRequired), true}, {"transient network", errors.New("microsoft: list calendars call: dial tcp: i/o timeout"), false}, {"http 500", errors.New("gcal: list calendars status 500"), false}, } diff --git a/internal/calendar/calselect.go b/internal/calendar/calselect.go index d59cc330..96e2833d 100644 --- a/internal/calendar/calselect.go +++ b/internal/calendar/calselect.go @@ -3,6 +3,7 @@ package calendar import ( "context" "database/sql" + "errors" "fmt" "github.com/calnode/calnode/internal/uid" @@ -77,8 +78,37 @@ func (s *Service) AccountCalendars(ctx context.Context, userID, provider, accoun return out, nil } +// SelectionValidator is implemented by a provider whose calendar ids must be checked before a +// selection is saved, because the id itself says where the account's credentials are sent. +// CalDAV is one: its calendar id is a collection URL that free/busy and booking writes then +// authenticate to. A provider whose ids are opaque names inside an API it already authorises +// (Google, Microsoft) does not implement it, so saving there stays a local write. +// +// ValidateSelection returns *UnknownCalendarError for an id the provider does not offer for +// the account, and any other error when it could not check. +type SelectionValidator interface { + ValidateSelection(ctx context.Context, userID, accountEmail string, calendarIDs []string) error +} + +// ErrCalendarList wraps a provider's failure to check a selection against the account's +// calendars, so a caller can report "the provider could not be reached" rather than an +// internal error. +var ErrCalendarList = errors.New("calendar: could not list the account's calendars") + +// UnknownCalendarError is a saved selection naming a calendar the provider does not list for +// the account. +type UnknownCalendarError struct{ CalendarID string } + +func (e *UnknownCalendarError) Error() string { + return fmt.Sprintf("calendar %q is not one of this account's calendars", e.CalendarID) +} + // SetAccountCalendars replaces the saved selection for the connection's account. At most one // calendar across the whole user may be the destination (enforced by clearing others first). +// +// When the provider is a SelectionValidator, nothing is saved unless it accepts every calendar +// id: a refused id comes back as *UnknownCalendarError, and a failure to check is wrapped in +// ErrCalendarList. Other providers save the ids as given, as they always have. func (s *Service) SetAccountCalendars(ctx context.Context, userID, provider, accountEmail string, sels []CalendarSelection) error { ok, err := s.accountExists(ctx, userID, provider, accountEmail) if err != nil { @@ -88,6 +118,22 @@ func (s *Service) SetAccountCalendars(ctx context.Context, userID, provider, acc return sql.ErrNoRows } + // Checked before the transaction opens: the pool is a single connection, and a provider + // checking the ids reads the connection row itself. + if v, ok := s.providers[provider].(SelectionValidator); ok { + ids := make([]string, 0, len(sels)) + for _, c := range sels { + ids = append(ids, c.ID) + } + if err := v.ValidateSelection(ctx, userID, accountEmail, ids); err != nil { + var unknown *UnknownCalendarError + if errors.As(err, &unknown) { + return err + } + return fmt.Errorf("%w: %w", ErrCalendarList, err) + } + } + tx, err := s.db.BeginTx(ctx, nil) if err != nil { return err diff --git a/internal/calendar/calselect_test.go b/internal/calendar/calselect_test.go new file mode 100644 index 00000000..314278e4 --- /dev/null +++ b/internal/calendar/calselect_test.go @@ -0,0 +1,153 @@ +package calendar + +import ( + "context" + "database/sql" + "errors" + "fmt" + "slices" + "testing" +) + +// plainProvider is a provider that does not validate selections (Google and Microsoft today). +// Only Name is expected to be called; the embedded nil Provider panics loudly otherwise, and +// ListCalendars is counted so a test can prove saving never lists. +type plainProvider struct { + Provider + name string + listed int +} + +func (p *plainProvider) Name() string { return p.name } + +func (p *plainProvider) ListCalendars(context.Context, string, string) ([]CalendarInfo, error) { + p.listed++ + return nil, errors.New("plainProvider: ListCalendars should not be called when saving") +} + +// validatingProvider accepts only the ids it offers, or fails to check at all when err is set. +type validatingProvider struct { + plainProvider + offered []string + err error +} + +func (p *validatingProvider) ValidateSelection(_ context.Context, _, _ string, ids []string) error { + if p.err != nil { + return p.err + } + for _, id := range ids { + if !slices.Contains(p.offered, id) { + return &UnknownCalendarError{CalendarID: id} + } + } + return nil +} + +func selection(id string, check, dest bool) CalendarSelection { + return CalendarSelection{CalendarInfo: CalendarInfo{ID: id}, CheckConflicts: check, IsDestination: dest} +} + +func savedSelection(t *testing.T, db *sql.DB, account string) (checked []string, dest string) { + t.Helper() + rows, err := db.Query(`SELECT calendar_id, check_conflicts, is_destination FROM connection_calendars + WHERE user_id = 'u1' AND account_email = ? ORDER BY calendar_id`, account) + if err != nil { + t.Fatal(err) + } + defer rows.Close() + for rows.Next() { + var id string + var cc, d int + if err := rows.Scan(&id, &cc, &d); err != nil { + t.Fatal(err) + } + if cc != 0 { + checked = append(checked, id) + } + if d != 0 { + dest = id + } + } + return checked, dest +} + +// A provider that does not validate selections saves exactly what it is given, with no call +// to the provider: Google's "primary" alias, which its calendar list never returns, still saves. +func TestSetAccountCalendars_nonValidatingProviderSavesAsGiven(t *testing.T) { + db := newTestDB(t) + seedConn(t, db, "c1", "u1", "google", "work@x.test", 1, 1) + p := &plainProvider{name: "google"} + svc := NewService(db) + svc.Register(p) + + if err := svc.SetAccountCalendars(context.Background(), "u1", "google", "work@x.test", []CalendarSelection{ + selection("primary", true, true), + selection("not-in-any-list@x.test", true, false), + }); err != nil { + t.Fatalf("SetAccountCalendars: %v", err) + } + if p.listed != 0 { + t.Errorf("ListCalendars called %d time(s); saving must stay a local write for this provider", p.listed) + } + checked, dest := savedSelection(t, db, "work@x.test") + if want := []string{"not-in-any-list@x.test", "primary"}; !slices.Equal(checked, want) || dest != "primary" { + t.Errorf("saved checked=%v dest=%q, want checked=%v dest=primary", checked, dest, want) + } +} + +// A validating provider's refusal saves nothing, leaving the previous selection as it was. +func TestSetAccountCalendars_validatingProviderRefusalSavesNothing(t *testing.T) { + db := newTestDB(t) + seedConn(t, db, "c1", "u1", "caldav", "me@x.test", 1, 1) + p := &validatingProvider{plainProvider: plainProvider{name: "caldav"}, + offered: []string{"https://dav.x.test/cal/home/", "https://dav.x.test/cal/work/"}} + svc := NewService(db) + svc.Register(p) + ctx := context.Background() + + if err := svc.SetAccountCalendars(ctx, "u1", "caldav", "me@x.test", []CalendarSelection{ + selection("https://dav.x.test/cal/home/", true, false), + selection("https://dav.x.test/cal/work/", true, true), + }); err != nil { + t.Fatalf("offered selection: %v", err) + } + + err := svc.SetAccountCalendars(ctx, "u1", "caldav", "me@x.test", []CalendarSelection{ + selection("https://dav.x.test/cal/home/", false, true), + selection("https://elsewhere.test/steal/", true, false), + }) + var unknown *UnknownCalendarError + if !errors.As(err, &unknown) || unknown.CalendarID != "https://elsewhere.test/steal/" { + t.Fatalf("err = %v, want *UnknownCalendarError for https://elsewhere.test/steal/", err) + } + if errors.Is(err, ErrCalendarList) { + t.Errorf("err = %v; a refused id is not a listing failure", err) + } + checked, dest := savedSelection(t, db, "me@x.test") + if want := []string{"https://dav.x.test/cal/home/", "https://dav.x.test/cal/work/"}; !slices.Equal(checked, want) || dest != "https://dav.x.test/cal/work/" { + t.Errorf("after a refused save checked=%v dest=%q, want the earlier selection %v with dest work", checked, dest, want) + } +} + +// When a validating provider cannot check at all, nothing is saved and the error says so: a +// caller maps ErrCalendarList to "provider unreachable" and a rejected login to "reconnect". +func TestSetAccountCalendars_validationFailureIsErrCalendarList(t *testing.T) { + db := newTestDB(t) + seedConn(t, db, "c1", "u1", "caldav", "me@x.test", 1, 1) + svc := NewService(db) + svc.Register(&validatingProvider{plainProvider: plainProvider{name: "caldav"}, + err: fmt.Errorf("caldav: authentication failed: %w", ErrReauthRequired)}) + + err := svc.SetAccountCalendars(context.Background(), "u1", "caldav", "me@x.test", + []CalendarSelection{selection("https://dav.x.test/cal/home/", true, true)}) + if !errors.Is(err, ErrCalendarList) { + t.Errorf("err = %v, want it to wrap ErrCalendarList", err) + } + if !IsReauthErr(err) { + t.Errorf("err = %v, want the provider's reauth error still recognisable", err) + } + if checked, dest := savedSelection(t, db, "me@x.test"); len(checked) != 0 || dest != "" { + t.Errorf("saved checked=%v dest=%q although the selection could not be checked", checked, dest) + } +} diff --git a/internal/handler/calendar.go b/internal/handler/calendar.go index 2514781e..df5ac6e6 100644 --- a/internal/handler/calendar.go +++ b/internal/handler/calendar.go @@ -367,6 +367,23 @@ func (h *Handler) PutConnectionCalendars(w http.ResponseWriter, r *http.Request) h.writeError(w, http.StatusNotFound, "calendar connection not found") return } + // A provider that validates selections (CalDAV) checks every id against the account's + // current calendars (see SetAccountCalendars), so its listing failures reach this + // handler the same way they reach the GET. + var unknown *calendar.UnknownCalendarError + if errors.As(err, &unknown) { + h.writeError(w, http.StatusBadRequest, unknown.Error()+". Reload the list of calendars and save again.") + return + } + if calendar.IsReauthErr(err) { + h.writeError(w, http.StatusConflict, "This calendar needs reconnecting — disconnect it and connect again.") + return + } + if errors.Is(err, calendar.ErrCalendarList) { + h.logger.ErrorContext(r.Context(), "set connection calendars: list", "error", err) + h.writeError(w, http.StatusBadGateway, "could not reach the calendar provider") + return + } h.logger.ErrorContext(r.Context(), "set connection calendars", "error", err) h.writeError(w, http.StatusInternalServerError, "internal error") return diff --git a/internal/handler/calendar_caldav_test.go b/internal/handler/calendar_caldav_test.go new file mode 100644 index 00000000..4f2ada47 --- /dev/null +++ b/internal/handler/calendar_caldav_test.go @@ -0,0 +1,116 @@ +package handler_test + +import ( + "encoding/json" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/calnode/calnode/internal/caldav" + "github.com/calnode/calnode/internal/calendar" + "github.com/calnode/calnode/internal/db" + "github.com/calnode/calnode/internal/handler" +) + +// twoCalendarDAV is a CalDAV server whose account holds two event calendars. +func twoCalendarDAV(t *testing.T) *httptest.Server { + t.Helper() + const ns = `` + cal := func(href, name string) string { + return `` + href + `HTTP/1.1 200 OK` + + `` + name + `` + + `` + + `` + } + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + w.WriteHeader(http.StatusMultiStatus) + switch { + case strings.Contains(string(body), "current-user-principal"): + io.WriteString(w, ns+``+r.URL.Path+`HTTP/1.1 200 OK/p/`) + case strings.Contains(string(body), "calendar-home-set"): + io.WriteString(w, ns+`/p/HTTP/1.1 200 OK/cal/`) + default: + io.WriteString(w, ns+cal("/cal/home/", "Home")+cal("/cal/work/", "Work")+``) + } + })) + t.Cleanup(srv.Close) + return srv +} + +// PUT /v1/calendar/connections/{id}/calendars must refuse a calendar the provider did not list, +// with a 400 that says what to do, and still accept one it did. +func TestPutConnectionCalendars_caldavRefusesAnUnlistedCalendar(t *testing.T) { + database, err := db.Open("sqlite://:memory:") + if err != nil { + t.Fatalf("db.Open: %v", err) + } + t.Cleanup(func() { database.Close() }) + if err := db.Migrate(database); err != nil { + t.Fatalf("db.Migrate: %v", err) + } + h := handler.New(database, slog.Default()) + cc, err := caldav.New(database, testGCalKeyHex) + if err != nil { + t.Fatalf("caldav.New: %v", err) + } + svc := calendar.NewService(database) + svc.Register(cc) + h.SetCalendar(svc) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/v1/setup", strings.NewReader(`{"name":"Cal User","email":"cal@example.com","timezone":"UTC"}`)) + req.Header.Set("Content-Type", "application/json") + h.Setup(rec, req) + var setup struct { + APIKey string `json:"api_key"` + } + if rec.Code != http.StatusCreated || json.Unmarshal(rec.Body.Bytes(), &setup) != nil { + t.Fatalf("setup: %d %s", rec.Code, rec.Body.String()) + } + + // Connect through the real handler and client: the loopback test server passes the + // metadata-only guard, which is the guard every listing and read goes through. + dav := twoCalendarDAV(t) + rec = httptest.NewRecorder() + h.RequireAuth(h.ConnectCalDAV)(rec, authReq(http.MethodPost, "/v1/calendar/caldav/connect", + `{"server_url":"`+dav.URL+`","username":"me@example.com","app_password":"pw"}`, setup.APIKey)) + if rec.Code != http.StatusOK { + t.Fatalf("connect: %d %s", rec.Code, rec.Body.String()) + } + + rec = httptest.NewRecorder() + h.RequireAuth(h.GetConnectionCalendars)(rec, authReq(http.MethodGet, + "/v1/calendar/connections/x/calendars?provider=caldav&account=me%40example.com", "", setup.APIKey)) + var listed struct { + Calendars []calendar.CalendarSelection `json:"calendars"` + } + if rec.Code != http.StatusOK || json.Unmarshal(rec.Body.Bytes(), &listed) != nil || len(listed.Calendars) != 2 { + t.Fatalf("list: %d %s, want both calendars", rec.Code, rec.Body.String()) + } + + put := func(calendarID string) *httptest.ResponseRecorder { + body, _ := json.Marshal(map[string]any{ + "provider": "caldav", "account_email": "me@example.com", + "calendars": []map[string]any{{"id": calendarID, "check_conflicts": true, "is_destination": true}}, + }) + rec := httptest.NewRecorder() + h.RequireAuth(h.PutConnectionCalendars)(rec, authReq(http.MethodPut, "/v1/calendar/connections/x/calendars", string(body), setup.APIKey)) + return rec + } + + rec = put("http://169.254.169.254/latest/meta-data/") + if rec.Code != http.StatusBadRequest { + t.Fatalf("unlisted calendar: status %d %s, want 400", rec.Code, rec.Body.String()) + } + if !strings.Contains(rec.Body.String(), "is not one of this account's calendars") { + t.Errorf("unlisted calendar: body %s, want a message naming the problem", rec.Body.String()) + } + + if rec = put(listed.Calendars[1].ID); rec.Code != http.StatusNoContent { + t.Errorf("listed calendar: status %d %s, want 204", rec.Code, rec.Body.String()) + } +}