mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-10-31 00:12:39 +00:00
fix: keep contact UIDs on update and export MEMBER URIs unchanged
`contactToWire` sends a key that is present but undefined as null, so a caller passing a card back without `uid`, `id` or `addressBookIds` (the plugin API does) asked the server to clear them. Stalwart 1.0 refuses a null UID and 0.16 silently deletes it; a null `id` or `addressBookIds` fails the update on both. Update mode now leaves those keys out. vCard export prefixed every group member with `urn:uuid:`, but keys are already URIs for groups made in Bulwark (members are keyed by uid) and for groups from CardDAV clients, which gave `MEMBER:urn:uuid:urn:uuid:…`. Keys with a URI scheme are written as they are.
This commit is contained in:
@@ -32,6 +32,21 @@ describe('contactToWire', () => {
|
||||
expect(contactToWire(cleared, 'create')).toEqual({});
|
||||
});
|
||||
|
||||
it('never clears id, uid or addressBookIds on update', () => {
|
||||
const wire = contactToWire({
|
||||
id: undefined, uid: undefined, addressBookIds: undefined, nicknames: undefined,
|
||||
} as Partial<ContactCard>, 'update');
|
||||
expect(wire).toEqual({ nicknames: null });
|
||||
|
||||
const nulls = contactToWire({ id: null, uid: null, addressBookIds: null } as unknown as Partial<ContactCard>, 'update');
|
||||
expect(nulls).toEqual({});
|
||||
});
|
||||
|
||||
it('still sends a uid or addressBookIds that has a value', () => {
|
||||
expect(contactToWire({ uid: 'urn:uuid:1', addressBookIds: { ab: true } }, 'update'))
|
||||
.toEqual({ uid: 'urn:uuid:1', addressBookIds: { ab: true } });
|
||||
});
|
||||
|
||||
it('leaves calendar links alone when the update does not mention them', () => {
|
||||
expect(contactToWire({ keywords: { vip: true } }, 'update')).toEqual({ keywords: { vip: true } });
|
||||
});
|
||||
|
||||
@@ -589,6 +589,34 @@ describe('JMAPClient contact methods', () => {
|
||||
.rejects.toThrow('Failed to update contact');
|
||||
});
|
||||
|
||||
it.each(['1.0', '0.16'] as const)('leaves a cleared id, uid and addressBookIds out of the update on Stalwart %s', async (version) => {
|
||||
const client = createClient();
|
||||
// What ContactCard/set answers to those keys set to null (1.0 contact/set.rs:
|
||||
// 229-249, 602-607, 669-671; 0.16.23 set.rs:187-199, 524-529, 591-593).
|
||||
const answer = (update: Record<string, unknown>) => {
|
||||
const refuse = (property: string, description: string) =>
|
||||
({ notUpdated: { 'contact-1': { type: 'invalidProperties', properties: [property], description } } });
|
||||
if ('id' in update) return refuse('id', 'The id property is immutable.');
|
||||
if (update.addressBookIds === null) return refuse('addressBookIds', 'Invalid patch operation for addressBookIds.');
|
||||
// 0.16 takes a null uid and drops the card's UID without a word.
|
||||
if (update.uid === null && version === '1.0') return refuse('uid', 'You cannot change the UID of a contact.');
|
||||
return { updated: { 'contact-1': null } };
|
||||
};
|
||||
const fetchSpy = vi.spyOn(globalThis, 'fetch').mockImplementation(async (_url, init) => {
|
||||
const update = JSON.parse(String(init?.body)).methodCalls[0][1].update['contact-1'];
|
||||
const body = { methodResponses: [['ContactCard/set', answer(update), '0']] };
|
||||
return { ok: true, status: 200, text: () => Promise.resolve(JSON.stringify(body)) } as Response;
|
||||
});
|
||||
|
||||
// A plugin passes the card back with the keys it did not set.
|
||||
await client.updateContact('contact-1', {
|
||||
id: undefined, uid: undefined, addressBookIds: undefined, name: mockContact.name,
|
||||
} as never);
|
||||
|
||||
const sent = JSON.parse(fetchSpy.mock.calls[0][1]?.body as string).methodCalls[0][1].update['contact-1'];
|
||||
expect(sent).toEqual({ name: mockContact.name });
|
||||
});
|
||||
|
||||
it('passes a Stalwart 1.0 blob photo through unchanged', async () => {
|
||||
const client = createClient();
|
||||
const fetchSpy = mockFetch({
|
||||
|
||||
@@ -540,6 +540,46 @@ describe("organization-only cards (issue #701)", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("MEMBER export", () => {
|
||||
const group = (members: Record<string, boolean>): ContactCard => ({
|
||||
id: "g1",
|
||||
addressBookIds: {},
|
||||
kind: "group",
|
||||
name: { full: "Team" },
|
||||
members,
|
||||
});
|
||||
|
||||
it("writes a member key that is already a URI as it is", () => {
|
||||
// Groups made in Bulwark key members by uid (urn:uuid:); CardDAV groups
|
||||
// keep whatever MEMBER value the client sent.
|
||||
const vcf = generateVCard([group({ "urn:uuid:0f3c9d2e": true, "mailto:ann@example.com": true })]);
|
||||
const lines = vcf.split("\r\n");
|
||||
expect(lines).toContain("MEMBER:urn:uuid:0f3c9d2e");
|
||||
expect(lines).toContain("MEMBER:mailto:ann@example.com");
|
||||
expect(vcf).not.toContain("urn:uuid:urn:uuid:");
|
||||
});
|
||||
|
||||
it("prefixes a bare key with urn:uuid:", () => {
|
||||
expect(generateVCard([group({ "0f3c9d2e": true, gone: false })]).split("\r\n").filter(l => l.startsWith("MEMBER:")))
|
||||
.toEqual(["MEMBER:urn:uuid:0f3c9d2e"]);
|
||||
});
|
||||
|
||||
it("round-trips members through import and export", () => {
|
||||
const [parsed] = parseVCard([
|
||||
"BEGIN:VCARD",
|
||||
"VERSION:3.0",
|
||||
"FN:Team",
|
||||
"KIND:group",
|
||||
"MEMBER:urn:uuid:member-1",
|
||||
"MEMBER:mailto:ann@example.com",
|
||||
"END:VCARD",
|
||||
].join("\r\n"));
|
||||
const [reparsed] = parseVCard(generateVCard([parsed]));
|
||||
expect(reparsed.members).toEqual(parsed.members);
|
||||
expect(generateVCard([parsed])).toContain("MEMBER:urn:uuid:member-1");
|
||||
});
|
||||
});
|
||||
|
||||
describe("round-trip: parse → generate → parse", () => {
|
||||
it("produces structurally equivalent data", () => {
|
||||
const original = [
|
||||
|
||||
@@ -17,6 +17,15 @@ const CLIENT_ONLY_KEYS = [
|
||||
/** vCard CALURI / FBURL / CALADRURI, kept flat in the UI. */
|
||||
const URI_KEYS = ['calendarUri', 'freeBusyUri', 'schedulingUri', 'source'] as const;
|
||||
|
||||
/**
|
||||
* Properties an update never clears. A null `id` fails the whole update ("The
|
||||
* id property is immutable.") and so does a null `addressBookIds` ("Invalid
|
||||
* patch operation for addressBookIds."). A null `uid` is refused by Stalwart
|
||||
* 1.0 ("You cannot change the UID of a contact.") and on 0.16 silently
|
||||
* deletes the card's UID.
|
||||
*/
|
||||
const NEVER_CLEARED_KEYS = ['id', 'uid', 'addressBookIds'] as const;
|
||||
|
||||
const LEGACY_ADDRESS_FIELDS: Array<[keyof ContactAddress, AddressComponent['kind']]> = [
|
||||
['street', 'name'],
|
||||
['locality', 'locality'],
|
||||
@@ -49,12 +58,14 @@ type Mode = 'create' | 'update';
|
||||
* The card as ContactCard/set expects it. On `update`, a key that is present
|
||||
* with the value `undefined` means "the user cleared it" and is sent as
|
||||
* `null`: JSON drops undefined, and an omitted property keeps its old value.
|
||||
* `id`, `uid` and `addressBookIds` without a value are left out instead.
|
||||
*/
|
||||
export function contactToWire(card: Partial<ContactCard>, mode: Mode): Record<string, unknown> {
|
||||
const out: Record<string, unknown> = {};
|
||||
for (const [key, value] of Object.entries(card)) {
|
||||
if ((CLIENT_ONLY_KEYS as readonly string[]).includes(key)) continue;
|
||||
if ((URI_KEYS as readonly string[]).includes(key)) continue;
|
||||
if (mode === 'update' && value == null && (NEVER_CLEARED_KEYS as readonly string[]).includes(key)) continue;
|
||||
if (value === undefined) {
|
||||
if (mode === 'update') out[key] = null;
|
||||
continue;
|
||||
|
||||
+5
-1
@@ -1405,7 +1405,11 @@ function generateSingleVCard(contact: ContactCard): string {
|
||||
if (contact.members) {
|
||||
for (const memberId of Object.keys(contact.members)) {
|
||||
if (contact.members[memberId]) {
|
||||
lines.push(`MEMBER:urn:uuid:${memberId}`);
|
||||
// Keys are URIs already for groups made here (members are keyed by
|
||||
// uid, a urn:uuid:) and for groups from CardDAV clients (Stalwart
|
||||
// keeps MEMBER values verbatim); only a bare key gets the prefix.
|
||||
const uri = /^[a-z][a-z0-9+.-]*:/i.test(memberId) ? memberId : `urn:uuid:${memberId}`;
|
||||
lines.push(`MEMBER:${uri}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user