mirror of
https://github.com/bulwarkmail/native.git
synced 2026-11-01 00:07:01 +00:00
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:
@@ -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
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user