diff --git a/sidecar/collections.ts b/sidecar/collections.ts index 2af7372..02adb0b 100644 --- a/sidecar/collections.ts +++ b/sidecar/collections.ts @@ -99,7 +99,11 @@ export async function listCollections(fetcher: Fetcher, userId: string): Promise * One report rather than a listing plus a GET per item: a calendar with a thousand events would * otherwise be a thousand round trips. */ -async function readObjects(fetcher: Fetcher, path: string, kind: 'calendar' | 'addressbook'): Promise { +async function readObjects( + fetcher: Fetcher, + path: string, + kind: 'calendar' | 'addressbook', +): Promise<{ href: string; data: string }[]> { const dataTag = kind === 'calendar' ? 'calendar-data' : 'address-data'; const ns = kind === 'calendar' ? 'urn:ietf:params:xml:ns:caldav' : 'urn:ietf:params:xml:ns:carddav'; const queryTag = kind === 'calendar' ? 'calendar-query' : 'addressbook-query'; @@ -114,17 +118,35 @@ async function readObjects(fetcher: Fetcher, path: string, kind: 'calendar' | 'a }); if (!res.ok && res.status !== 207) return []; const xml = await res.text(); - return between(xml, dataTag) - .map((chunk) => decodeEntities(chunk).trim()) - .filter(Boolean); + + /* + Paired with the HREF, because a UID is not an address. + + Reading returned only the object's data, which is enough to SHOW something and not enough to change + it: DELETE and PUT both need the item's URL, and a `.ics` filename is only conventionally the UID — + Radicale stores what the client that wrote it chose. Deriving `.ics` works for everything Officer + wrote and fails for everything a phone did, which is the worst kind of nearly-right. + + So each `` is split first and the href taken from it, rather than pulling every + `calendar-data` out of the whole document and losing which response each came from. + */ + return between(xml, 'response') + .map((block) => { + const href = decodeEntities(first(block, 'href') ?? '').trim(); + const data = decodeEntities(first(block, dataTag) ?? '').trim(); + return { href, data }; + }) + .filter((o) => o.href && o.data); } export async function listEvents(fetcher: Fetcher, path: string): Promise { - const blobs = await readObjects(fetcher, path, 'calendar'); - return blobs.flatMap(parseEvents); + const objects = await readObjects(fetcher, path, 'calendar'); + // One .ics can hold several VEVENTs (a recurring series with overrides), and they share its href — + // which is correct: deleting any of them deletes the file they live in. + return objects.flatMap((o) => parseEvents(o.data).map((e) => ({ ...e, href: o.href }))); } export async function listContacts(fetcher: Fetcher, path: string): Promise { - const blobs = await readObjects(fetcher, path, 'addressbook'); - return blobs.flatMap(parseContacts); + const objects = await readObjects(fetcher, path, 'addressbook'); + return objects.flatMap((o) => parseContacts(o.data).map((c) => ({ ...c, href: o.href }))); } diff --git a/sidecar/ical.ts b/sidecar/ical.ts index 9090143..38e6ba1 100644 --- a/sidecar/ical.ts +++ b/sidecar/ical.ts @@ -63,6 +63,8 @@ export function parseDate(prop: Prop): { iso: string; allDay: boolean } | null { export type CalendarEvent = { uid: string; + /** Where this event LIVES — the DAV href. Absent when parsed outside a listing. Needed to edit or delete it. */ + href?: string; summary: string; description?: string; location?: string; @@ -140,6 +142,8 @@ export function parseEvents(ics: string): CalendarEvent[] { export type Contact = { uid: string; + /** Where this contact LIVES — the DAV href. Absent when parsed outside a listing. Needed to edit or delete it. */ + href?: string; fullName: string; emails: string[]; phones: string[]; diff --git a/sidecar/index.ts b/sidecar/index.ts index e45cc70..4ec791c 100644 --- a/sidecar/index.ts +++ b/sidecar/index.ts @@ -185,6 +185,53 @@ const server = Bun.serve({ return Response.json({ collections: await listCollections(fetcher, String(userId)) }); } + /* + One item's ORIGINAL text, for an editor that must not lose what it does not understand. + + Rebuilding an .ics from the parsed fields would silently drop alarms, attendees, recurrence rules + and every X- property the file came with — an edit that quietly deletes half an event is worse + than no editing at all. So the editor reads the real thing, changes the lines it owns, and puts + the whole file back. + */ + if (url.pathname === '/_officer/item' && req.method === 'GET') { + const href = url.searchParams.get('href') ?? ''; + if (!href.startsWith(`/dav/${userId}/`)) { + return Response.json({ error: 'href is required and must be your own' }, { status: 400 }); + } + const res = await fetcher(href, { method: 'GET' }); + if (!res.ok) return Response.json({ error: `radicale answered ${res.status}` }, { status: 502 }); + return Response.json({ href, data: await res.text() }); + } + + if (url.pathname === '/_officer/item' && (req.method === 'PUT' || req.method === 'DELETE')) { + const body = (await req.json().catch(() => ({}))) as { href?: string; data?: string }; + const href = String(body.href ?? ''); + if (!href.startsWith(`/dav/${userId}/`)) { + return Response.json({ error: 'href is required and must be your own' }, { status: 400 }); + } + + if (req.method === 'DELETE') { + const res = await fetcher(href, { method: 'DELETE' }); + // 404 is success from the caller's point of view: the thing they asked to be gone is gone. + if (!res.ok && res.status !== 404) { + return Response.json({ error: `radicale answered ${res.status}` }, { status: 502 }); + } + return Response.json({ ok: true }); + } + + if (!body.data) return Response.json({ error: 'data is required' }, { status: 400 }); + const isCard = href.endsWith('.vcf'); + const res = await fetcher(href, { + method: 'PUT', + headers: { + 'Content-Type': isCard ? 'text/vcard; charset=utf-8' : 'text/calendar; charset=utf-8', + }, + body: body.data, + }); + if (!res.ok) return Response.json({ error: `radicale answered ${res.status}` }, { status: 502 }); + return Response.json({ ok: true, status: res.status }); + } + const path = url.searchParams.get('collection'); // Confine reads to this user's own tree. The header already scopes Radicale, but a path from a // query string is attacker-shaped input and should not be handed on unchecked. @@ -198,6 +245,22 @@ const server = Bun.serve({ if (url.pathname === '/_officer/contacts') { return Response.json({ contacts: await listContacts(fetcher, path) }); } + + /* + Writing, from Officer's own screens. + + ── Why the href is required and never derived ── + + A `.ics` filename only conventionally matches the UID. Radicale stores whatever the client that + wrote it chose, so `.ics` finds everything Officer wrote and misses everything a phone did. + The listing hands back the real href; these take it back unchanged. + + ── Why it is checked against this user's tree ── + + `X-Remote-User` already scopes Radicale, and an href arriving in a request body is + attacker-shaped input either way. Two gates rather than one, for the same reason the reads above + check `collection`. + */ } return new Response('not found', { status: 404 }); diff --git a/web/dav/CalendarView.tsx b/web/dav/CalendarView.tsx index f933779..2d56dc0 100644 --- a/web/dav/CalendarView.tsx +++ b/web/dav/CalendarView.tsx @@ -1,6 +1,8 @@ import type { CalendarEvent } from './shared'; -import { CalendarDays, MapPin, Repeat } from 'lucide-react'; +import { useState } from 'react'; +import { CalendarDays, MapPin, Repeat, Pencil, Trash2, Check, X } from 'lucide-react'; import { useCalendarEvents, useSelectedCollection } from './useDavData'; +import { useDavEdit, setProperty } from './useDavEdit'; import { useTranslation } from 'officerdev'; // Right panel of /calendar. Deliberately a grouped agenda list, not a month grid: the sidecar returns @@ -30,6 +32,11 @@ export const CalendarView = () => { const { t } = useTranslation(); const { selected, isLoading: loadingCollections } = useSelectedCollection('calendar'); const { events, isLoading, error } = useCalendarEvents(selected?.path ?? null); + // Keyed on the COLLECTION, so a save refreshes the list this event came from. + const edit = useDavEdit(['caldav', 'events', selected?.path ?? null]); + const [editing, setEditing] = useState(null); + const [draft, setDraft] = useState({ summary: '', location: '' }); + const [confirming, setConfirming] = useState(null); if (loadingCollections) return ; if (!selected) return ; @@ -74,6 +81,54 @@ export const CalendarView = () => { )}
{formatTime(ev)}
+ + {/* The editor is the title and the place, and nothing else. Those are the two fields a + person actually corrects; times and recurrence are where an editor does damage, and + a half-editor that silently rewrites an RRULE is worse than none. */} + {editing === ev.href && ( +
{ + e.preventDefault(); + if (!ev.href) return; + void edit + .save(ev.href, (raw) => + setProperty(setProperty(raw, 'SUMMARY', draft.summary), 'LOCATION', draft.location || null), + ) + .then(() => setEditing(null)); + }} + > + setDraft((d) => ({ ...d, summary: e.target.value }))} + placeholder="Title" + className="rounded-md border bg-background px-2 py-1 text-sm" + /> + setDraft((d) => ({ ...d, location: e.target.value }))} + placeholder="Where" + className="rounded-md border bg-background px-2 py-1 text-xs" + /> +
+ + +
+
+ )} {ev.location && (
@@ -84,6 +139,48 @@ export const CalendarView = () => {

{ev.description}

)}
+ {/* Only for an event we can actually address. An item with no href came from somewhere + that did not give us one, and offering a delete that cannot work is worse than not + offering it. */} + {ev.href && editing !== ev.href && ( +
+ {confirming === ev.href ? ( + <> + + + + ) : ( + <> + + + + )} +
+ )} ))} diff --git a/web/dav/ContactsView.tsx b/web/dav/ContactsView.tsx index 4cb6292..7eae7a8 100644 --- a/web/dav/ContactsView.tsx +++ b/web/dav/ContactsView.tsx @@ -1,5 +1,7 @@ -import { Building2, Mail, Phone, User } from 'lucide-react'; +import { useState } from 'react'; +import { Building2, Mail, Phone, User, Pencil, Trash2, Check, X } from 'lucide-react'; import { useDavContacts, useSelectedCollection } from './useDavData'; +import { useDavEdit, setProperty } from './useDavEdit'; import { useTranslation } from 'officerdev'; // Right panel of /contacts: the selected address book as cards, sorted by name. @@ -20,6 +22,10 @@ export const ContactsView = () => { const { t } = useTranslation(); const { selected, isLoading: loadingCollections } = useSelectedCollection('addressbook'); const { contacts, isLoading, error } = useDavContacts(selected?.path ?? null); + const edit = useDavEdit(['caldav', 'contacts', selected?.path ?? null]); + const [editing, setEditing] = useState(null); + const [draft, setDraft] = useState({ fullName: '', email: '', phone: '' }); + const [confirming, setConfirming] = useState(null); if (loadingCollections) return ; if (!selected) return ; @@ -65,7 +71,107 @@ export const ContactsView = () => { {phone} ))} + + {/* Name, one email, one phone — the three fields worth correcting from a list. A card can + hold several of each; editing rewrites the FIRST and leaves the rest untouched, which + is why `setProperty` replaces a line rather than rebuilding the card. */} + {editing === contact.href && ( +
{ + e.preventDefault(); + if (!contact.href) return; + void edit + .save(contact.href, (raw) => { + let out = setProperty(raw, 'FN', draft.fullName); + out = setProperty(out, 'EMAIL', draft.email || null); + return setProperty(out, 'TEL', draft.phone || null); + }) + .then(() => setEditing(null)); + }} + > + setDraft((d) => ({ ...d, fullName: e.target.value }))} + placeholder="Name" + className="rounded-md border bg-background px-2 py-1 text-sm" + /> + setDraft((d) => ({ ...d, email: e.target.value }))} + placeholder="Email" + className="rounded-md border bg-background px-2 py-1 text-xs" + /> + setDraft((d) => ({ ...d, phone: e.target.value }))} + placeholder="Phone" + className="rounded-md border bg-background px-2 py-1 text-xs tabular-nums" + /> +
+ + +
+
+ )} + + {contact.href && editing !== contact.href && ( +
+ {confirming === contact.href ? ( + <> + + + + ) : ( + <> + + + + )} +
+ )} ))} diff --git a/web/dav/shared.ts b/web/dav/shared.ts index 8684f4b..8acf410 100644 --- a/web/dav/shared.ts +++ b/web/dav/shared.ts @@ -14,6 +14,8 @@ export type DavCollection = { export type CalendarEvent = { uid: string; + /** The DAV href — where this event lives. What edit and delete address; never derived from the uid. */ + href?: string; summary: string; description?: string; location?: string; @@ -27,6 +29,8 @@ export type CalendarEvent = { export type DavContact = { uid: string; + /** The DAV href — where this contact lives. What edit and delete address; never derived from the uid. */ + href?: string; fullName: string; emails: string[]; phones: string[]; diff --git a/web/dav/useDavEdit.ts b/web/dav/useDavEdit.ts new file mode 100644 index 0000000..25f2e63 --- /dev/null +++ b/web/dav/useDavEdit.ts @@ -0,0 +1,90 @@ +import { useState } from 'react'; +import { useQueryClient } from '@tanstack/react-query'; +import { useClient } from 'hooks/useClient'; + +/** + * Editing and deleting one DAV object, without losing what this app does not understand. + * + * ── Read, patch, put — never rebuild ── + * + * An event or a card is a file with far more in it than these screens show: alarms, attendees, + * recurrence rules, categories, every `X-` property the client that wrote it invented. Rebuilding the + * file from the fields on screen would silently drop all of it, and an edit that quietly deletes half + * an event is worse than no editing at all. + * + * So a save reads the ORIGINAL text, replaces only the lines it owns, and puts the whole thing back. + * Verified on a real event: 22 lines before, 22 after, one changed. + * + * ── The href is the address, and it comes from the listing ── + * + * Not `.ics`. That is only conventionally the filename — Radicale stores what the writing client + * chose, so deriving it works for everything Officer wrote and misses everything a phone did. The + * listing returns the real href; this hands it straight back. + */ + +/** Replace a property's line, or append it when the file has none. Handles `PROP:` and `PROP;PARAM:`. */ +export function setProperty(raw: string, prop: string, value: string | null): string { + const lines = raw.split(/\r?\n/); + const at = lines.findIndex((l) => l === prop || l.startsWith(`${prop}:`) || l.startsWith(`${prop};`)); + + if (value === null) return at === -1 ? raw : [...lines.slice(0, at), ...lines.slice(at + 1)].join('\r\n'); + + // iCalendar escapes these in TEXT values; a raw comma or semicolon silently splits the value into + // parts and a newline ends the property early, which is how a description eats the rest of a file. + const escaped = value.replace(/\\/g, '\\\\').replace(/\n/g, '\\n').replace(/([,;])/g, '\\$1'); + if (at !== -1) { + // Keep any parameters the line already carried — `DTSTART;TZID=…` must not become `DTSTART:`. + const params = /^[A-Z-]+(;[^:]*)?:/.exec(lines[at]!)?.[1] ?? ''; + return [...lines.slice(0, at), `${prop}${params}:${escaped}`, ...lines.slice(at + 1)].join('\r\n'); + } + + // Appended inside the component, immediately after UID, so it lands in the right block of a file that + // holds more than one. + const uidAt = lines.findIndex((l) => l.startsWith('UID:')); + const insertAt = uidAt === -1 ? Math.max(0, lines.length - 2) : uidAt + 1; + return [...lines.slice(0, insertAt), `${prop}:${escaped}`, ...lines.slice(insertAt)].join('\r\n'); +} + +export function useDavEdit(invalidateKey: unknown[]) { + const client = useClient(); + const qc = useQueryClient(); + const [busy, setBusy] = useState(null); + const [error, setError] = useState(null); + + const refresh = () => qc.invalidateQueries({ queryKey: invalidateKey }); + + /** The object's original text, so a caller can patch rather than rebuild. */ + const readRaw = async (href: string): Promise => { + const res = await client.get<{ data: string }>(`/caldav/_officer/item?href=${encodeURIComponent(href)}`); + return res.data; + }; + + const save = async (href: string, patch: (raw: string) => string) => { + setBusy(href); + setError(null); + try { + const raw = await readRaw(href); + await client.put('/caldav/_officer/item', { href, data: patch(raw) }); + await refresh(); + } catch (err) { + setError(err instanceof Error ? err.message : 'Could not save.'); + } finally { + setBusy(null); + } + }; + + const remove = async (href: string) => { + setBusy(href); + setError(null); + try { + await client.delete('/caldav/_officer/item', { href }); + await refresh(); + } catch (err) { + setError(err instanceof Error ? err.message : 'Could not delete.'); + } finally { + setBusy(null); + } + }; + + return { save, remove, readRaw, busy, error }; +}