fix: offer sharing files and calendars with other users on Stalwart

`supportsSharing()` required `urn:ietf:params:jmap:principals:owner`,
which Stalwart advertises nowhere; its session only has `principals` and
`principals:availability`. The file menu therefore never showed
"Sharing & access", and `getPrincipals()` returned nothing, so the
calendar share sheet had no one to offer either.

- Gate sharing on `urn:ietf:params:jmap:principals`, like the webmail's
  `supportsPrincipals()`.
- Keep `principals:owner` out of FileNode requests unless the server
  advertises it, since an unknown capability in `using` fails the whole
  request (why the gate had been tied to it).
- Page the principal lookup by `maxObjectsInGet`, as the webmail does:
  one unbounded `Principal/query` fed every id to `Principal/get`, which
  the server refuses once the directory has more than 500 entries.
This commit is contained in:
Linus Rath
2026-09-24 18:41:23 +02:00
parent 00ad2f49fb
commit caea3d0c60
2 changed files with 87 additions and 33 deletions
+45 -10
View File
@@ -426,6 +426,26 @@ describe('getPrincipals', () => {
expect(await getPrincipals()).toEqual([]);
expect(mockRequest).not.toHaveBeenCalled();
});
it('pages through a directory larger than maxObjectsInGet', async () => {
const directory = Array.from({ length: 1203 }, (_, i) => ({ id: `p${i}`, name: `user${i}`, type: 'individual' }));
mockRequest.mockImplementation(async (calls: [string, Record<string, unknown>, string][]) => {
const { position, limit } = calls[0][1] as { position: number; limit: number };
const page = directory.slice(position, position + limit);
return {
methodResponses: [
['Principal/query', { ids: page.map((p) => p.id), position }, '0'],
['Principal/get', { list: page }, '1'],
],
};
});
const principals = await getPrincipals();
expect(principals).toHaveLength(1203);
expect(mockRequest.mock.calls.map(([calls]) => calls[0][1].position)).toEqual([0, 500, 1000]);
for (const [calls] of mockRequest.mock.calls) expect(calls[0][1].limit).toBe(500);
});
});
describe('decodeFileNodeName at the API boundary (#869)', () => {
@@ -540,19 +560,34 @@ describe('accountSupportsFiles (#563)', () => {
});
describe('supportsSharing', () => {
it('accepts principals:owner from the session or the account capabilities', () => {
const mockHasAccountCapability = jmapClient.hasAccountCapability as ReturnType<typeof vi.fn>;
// What Stalwart advertises: principals and principals:availability, never
// principals:owner.
const stalwartCapabilities = (urn: string) =>
urn === CAPABILITIES.FILES ||
urn === CAPABILITIES.PRINCIPALS ||
urn === 'urn:ietf:params:jmap:principals:availability';
it('offers sharing when the server advertises principals', () => {
mockHasCapability.mockImplementation(stalwartCapabilities);
expect(supportsSharing()).toBe(true);
mockHasCapability.mockImplementation((urn: string) => urn === CAPABILITIES.FILES);
mockHasAccountCapability.mockReturnValue(false);
expect(supportsSharing()).toBe(false);
});
mockHasAccountCapability.mockImplementation((urn: string) => urn === CAPABILITIES.PRINCIPALS_OWNER);
expect(supportsSharing()).toBe(true);
it('still lists principals and leaves principals:owner out of FileNode requests on Stalwart', async () => {
mockHasCapability.mockImplementation(stalwartCapabilities);
mockRequest.mockResolvedValue({
methodResponses: [
['Principal/query', { ids: ['p1'] }, '0'],
['Principal/get', { list: [{ id: 'p1', name: 'other', type: 'individual' }] }, '1'],
],
});
expect(await getPrincipals()).toHaveLength(1);
mockHasAccountCapability.mockReturnValue(false);
mockHasCapability.mockImplementation(
(urn: string) => urn === CAPABILITIES.FILES || urn === CAPABILITIES.PRINCIPALS_OWNER,
);
expect(supportsSharing()).toBe(true);
mockRequest.mockResolvedValue({ methodResponses: [['FileNode/set', { updated: { f1: null } }, '0']] });
await setFileNodeShare('f1', 'p1', null);
const [, using] = mockRequest.mock.calls[1];
expect(using).toEqual([CAPABILITIES.CORE, CAPABILITIES.FILES]);
});
});
+42 -23
View File
@@ -25,14 +25,14 @@ const FILE_NODE_PROPERTIES = [
'shareWith', 'myRights',
];
// RFC 9670 sharing is only usable when the server advertises the exact
// `principals:owner` capability, either in the session or on the account
// (Stalwart places it in accountCapabilities).
// Sharing with other users (RFC 9670) needs the principal directory to pick
// people from, so it is offered whenever the server advertises
// `urn:ietf:params:jmap:principals`, like the webmail's supportsPrincipals().
// It used to require `principals:owner`, which Stalwart advertises nowhere,
// so "Sharing & access" and the calendar share picker never had anyone to
// offer.
export function supportsSharing(): boolean {
return (
jmapClient.hasCapability(CAPABILITIES.PRINCIPALS_OWNER) ||
jmapClient.hasAccountCapability(CAPABILITIES.PRINCIPALS_OWNER, filesAccountId())
);
return jmapClient.hasCapability(CAPABILITIES.PRINCIPALS);
}
export function filesAccountId(): string {
@@ -74,9 +74,13 @@ function fileUsing(): string[] {
if (jmapClient.hasCapability(CAPABILITIES.FILES)) {
using.push(CAPABILITIES.FILES);
}
// Required for shareWith/myRights on FileNode and for cross-account
// (shared-with-me) FileNode/get.
if (supportsSharing()) {
// Only name principals:owner where the server advertises it: an unknown
// capability in `using` fails the whole request. Stalwart returns
// shareWith/myRights without it.
if (
jmapClient.hasCapability(CAPABILITIES.PRINCIPALS_OWNER) ||
jmapClient.hasAccountCapability(CAPABILITIES.PRINCIPALS_OWNER, filesAccountId())
) {
using.push(CAPABILITIES.PRINCIPALS_OWNER);
}
return using;
@@ -410,23 +414,38 @@ export async function setFileNodeShare(
}
}
/** Safety bound on how many principals the share picker pages through. */
const MAX_PRINCIPALS = 20_000;
// List all principals visible to the user. Stalwart returns the full
// directory regardless of `filter`, so callers filter client-side.
// directory regardless of `filter`, so callers filter client-side. Paged by
// maxObjectsInGet: an unbounded query hands every id to the chained
// Principal/get, which the server refuses with `requestTooLarge` once the
// directory holds more than that (500), leaving the picker empty.
export async function getPrincipals(): Promise<Principal[]> {
if (!supportsSharing()) return [];
const accountId = jmapClient.accountId;
const res = await jmapClient.request(
[
['Principal/query', { accountId }, '0'],
['Principal/get', {
accountId,
'#ids': { resultOf: '0', name: 'Principal/query', path: '/ids' },
}, '1'],
],
[CAPABILITIES.CORE, CAPABILITIES.PRINCIPALS],
);
const getResp = res.methodResponses.find((r) => r[0] === 'Principal/get');
return (getResp?.[1].list ?? []) as Principal[];
const pageSize = jmapClient.getMaxObjectsInGet();
const all: Principal[] = [];
for (let position = 0; position < MAX_PRINCIPALS;) {
const res = await jmapClient.request(
[
['Principal/query', { accountId, position, limit: pageSize }, '0'],
['Principal/get', {
accountId,
'#ids': { resultOf: '0', name: 'Principal/query', path: '/ids' },
}, '1'],
],
[CAPABILITIES.CORE, CAPABILITIES.PRINCIPALS],
);
const ids = (res.methodResponses.find((r) => r[0] === 'Principal/query')?.[1].ids ?? []) as string[];
const getResp = res.methodResponses.find((r) => r[0] === 'Principal/get');
if (!getResp) break;
all.push(...((getResp[1].list ?? []) as Principal[]));
if (ids.length < pageSize) break;
position += ids.length;
}
return all;
}
// The principal id that represents the logged-in user (excluded from the