mirror of
https://github.com/bulwarkmail/native.git
synced 2026-11-01 16:05:39 +00:00
fix: explain sharing when Stalwart 1.0 keeps its directory closed
Stalwart 1.0 refuses Principal/query with `forbidden` unless the
administrator allows directory queries, which is off by default. The
app took the missing Principal/get answer for an empty directory: the
calendar share sheet said "Sharing is not available on this server.",
the file share sheet said "No other users found", and existing shares
showed bare ids.
- `listPrincipals` says whether the listing is complete, `restricted`
or failed. On `forbidden` it falls back to Principal/get without ids,
which 1.0 still answers with the user's own account, their groups and
the accounts that shared something with them.
- Principal/get asks for `id`, `type`, `name`, `description` and `email`
only; 1.0's default list adds `capabilities` and `accounts`.
- Both share sheets explain a restricted directory under the search
box. Only 1.0's own refusal ("The administrator has disabled directory
queries.") gets the wording that points at Stalwart's sharing
settings; any other refusal gets a generic one, and a failed listing
says so.
- A sharee the app can't name shows as "Unknown user" with its id, and
can still be changed or removed.
On 0.16 with the default roles the listing is complete and the sheets
behave as before.
This commit is contained in:
@@ -36,11 +36,13 @@ import {
|
||||
getPrincipals,
|
||||
isCrossAccountId,
|
||||
isFolder,
|
||||
listPrincipals,
|
||||
moveFileNode,
|
||||
renameFileNode,
|
||||
setFileNodeShare,
|
||||
supportsSharing,
|
||||
} from '../files';
|
||||
import { directoryNotice } from '../../lib/principal-directory';
|
||||
|
||||
const mockRequest = jmapClient.request as ReturnType<typeof vi.fn>;
|
||||
const mockHasCapability = jmapClient.hasCapability as ReturnType<typeof vi.fn>;
|
||||
@@ -488,6 +490,88 @@ describe('getPrincipals', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('listPrincipals', () => {
|
||||
// userc's view on the test servers (live-checked).
|
||||
const DIRECTORY = [
|
||||
{ id: 'b', type: 'individual', name: 'admin@example.org', description: 'System administrator', email: 'admin@example.org' },
|
||||
{ id: 'c', type: 'group', name: 'team@example.org', description: 'Team (test group)', email: 'team@example.org' },
|
||||
{ id: 'd', type: 'individual', name: 'usera@example.org', description: 'User A', email: 'usera@example.org' },
|
||||
{ id: 'e', type: 'individual', name: 'userb@example.org', description: 'User B', email: 'userb@example.org' },
|
||||
{ id: 'f', type: 'individual', name: 'userc@example.org', description: 'User C', email: 'userc@example.org' },
|
||||
];
|
||||
const OWN = [DIRECTORY[4]];
|
||||
const PROPERTIES = ['id', 'type', 'name', 'description', 'email'];
|
||||
// Stalwart 1.0 with allowDirectoryQueries off (the default).
|
||||
const QUERIES_OFF = [
|
||||
['error', { type: 'forbidden', description: 'The administrator has disabled directory queries.' }, '0'],
|
||||
['error', { type: 'invalidResultReference', description: 'Result reference to 0#Principal/query not found.' }, '1'],
|
||||
];
|
||||
|
||||
it('lists the whole directory on Stalwart 0.16, asking only for the properties it shows', async () => {
|
||||
mockRequest.mockResolvedValue({
|
||||
methodResponses: [
|
||||
['Principal/query', { accountId: 'acc-1', queryState: 'n', canCalculateChanges: true, position: 0, ids: ['b', 'c', 'd', 'e', 'f'] }, '0'],
|
||||
['Principal/get', { accountId: 'acc-1', state: 'n', list: DIRECTORY, notFound: [] }, '1'],
|
||||
],
|
||||
});
|
||||
|
||||
expect(await listPrincipals()).toEqual({ principals: DIRECTORY, status: 'ok' });
|
||||
expect(mockRequest).toHaveBeenCalledTimes(1);
|
||||
expect(mockRequest.mock.calls[0][0][1][1].properties).toEqual(PROPERTIES);
|
||||
});
|
||||
|
||||
it('falls back to the principals the user knows when Stalwart 1.0 keeps its directory closed', async () => {
|
||||
mockRequest
|
||||
.mockResolvedValueOnce({ methodResponses: QUERIES_OFF })
|
||||
.mockResolvedValueOnce({
|
||||
methodResponses: [['Principal/get', { accountId: 'acc-1', state: 'n', list: OWN, notFound: [] }, '0']],
|
||||
});
|
||||
|
||||
expect(await listPrincipals()).toEqual({ principals: OWN, status: 'restricted', directoryQueriesOff: true });
|
||||
const [calls, using] = mockRequest.mock.calls[1];
|
||||
expect(calls).toEqual([['Principal/get', { accountId: 'acc-1', ids: null, properties: PROPERTIES }, '0']]);
|
||||
expect(using).toEqual([CAPABILITIES.CORE, CAPABILITIES.PRINCIPALS]);
|
||||
});
|
||||
|
||||
it('calls another forbidden answer restricted without claiming directory queries are off', async () => {
|
||||
mockRequest
|
||||
.mockResolvedValueOnce({
|
||||
methodResponses: [
|
||||
['error', { type: 'forbidden', description: 'You do not have enough permissions to access this resource.' }, '0'],
|
||||
['error', { type: 'invalidResultReference' }, '1'],
|
||||
],
|
||||
})
|
||||
.mockResolvedValueOnce({ methodResponses: [['error', { type: 'forbidden' }, '0']] });
|
||||
|
||||
expect(await listPrincipals()).toEqual({ principals: [], status: 'restricted', directoryQueriesOff: false });
|
||||
});
|
||||
|
||||
it('reports a failed listing with what arrived', async () => {
|
||||
mockMaxObjectsInGet.mockReturnValue(2);
|
||||
mockRequest
|
||||
.mockResolvedValueOnce({
|
||||
methodResponses: [
|
||||
['Principal/query', { ids: ['b', 'c'] }, '0'],
|
||||
['Principal/get', { list: DIRECTORY.slice(0, 2) }, '1'],
|
||||
],
|
||||
})
|
||||
.mockRejectedValueOnce(new Error('JMAP request failed: 503'));
|
||||
expect(await listPrincipals()).toEqual({ principals: DIRECTORY.slice(0, 2), status: 'error' });
|
||||
|
||||
mockRequest.mockResolvedValueOnce({ methodResponses: [['error', { type: 'serverFail' }, '0']] });
|
||||
expect(await listPrincipals()).toEqual({ principals: [], status: 'error' });
|
||||
});
|
||||
});
|
||||
|
||||
describe('directoryNotice', () => {
|
||||
it('uses the 1.0 wording only for Stalwart 1.0\'s own refusal', () => {
|
||||
expect(directoryNotice({ status: 'ok' })).toBeNull();
|
||||
expect(directoryNotice({ status: 'restricted', directoryQueriesOff: true })).toBe('restricted');
|
||||
expect(directoryNotice({ status: 'restricted', directoryQueriesOff: false })).toBe('not_browsable');
|
||||
expect(directoryNotice({ status: 'error' })).toBe('failed');
|
||||
});
|
||||
});
|
||||
|
||||
describe('decodeFileNodeName at the API boundary (#869)', () => {
|
||||
it('decodes percent-encoded names from getAllFileNodes and the cross-account list', async () => {
|
||||
mockRequest.mockResolvedValue({
|
||||
|
||||
+109
-25
@@ -422,35 +422,119 @@ 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. 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 [];
|
||||
// The Principal properties the share sheets read. 0.16's default list is the
|
||||
// same; 1.0's adds `capabilities` for every principal and `accounts`, a whole
|
||||
// account object with every account capability.
|
||||
const PRINCIPAL_PROPERTIES = ['id', 'type', 'name', 'description', 'email'];
|
||||
|
||||
// Stalwart 1.0's answer to Principal/query while the administrator keeps
|
||||
// directory queries off (the default).
|
||||
const DIRECTORY_QUERIES_OFF = 'The administrator has disabled directory queries.';
|
||||
|
||||
/**
|
||||
* How complete a principal listing is:
|
||||
* - `ok`: the whole directory;
|
||||
* - `restricted`: the server refused to list its directory (`forbidden`:
|
||||
* Stalwart 1.0 with directory queries off, or a role without the
|
||||
* permission). The principals are what Principal/get without ids still
|
||||
* returns, on 1.0 the user's own account, their groups and the accounts
|
||||
* that shared something with them;
|
||||
* - `error`: the listing failed part-way; the principals are what arrived.
|
||||
*/
|
||||
export type PrincipalDirectoryStatus = 'ok' | 'restricted' | 'error';
|
||||
|
||||
export interface PrincipalListing {
|
||||
principals: Principal[];
|
||||
status: PrincipalDirectoryStatus;
|
||||
/**
|
||||
* The refusal was Stalwart 1.0's own: an administrator can allow directory
|
||||
* queries in its sharing settings.
|
||||
*/
|
||||
directoryQueriesOff?: boolean;
|
||||
}
|
||||
|
||||
function principalsUsing(): string[] {
|
||||
return [CAPABILITIES.CORE, CAPABILITIES.PRINCIPALS];
|
||||
}
|
||||
|
||||
// List all principals visible to the user, and how complete the list is.
|
||||
// Stalwart returns the full 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 listPrincipals(): Promise<PrincipalListing> {
|
||||
if (!supportsSharing()) return { principals: [], status: 'ok' };
|
||||
const accountId = jmapClient.accountId;
|
||||
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;
|
||||
try {
|
||||
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' },
|
||||
properties: PRINCIPAL_PROPERTIES,
|
||||
}, '1'],
|
||||
],
|
||||
principalsUsing(),
|
||||
);
|
||||
const responses = res.methodResponses ?? [];
|
||||
const queryResp = responses.find((r) => r[0] === 'Principal/query');
|
||||
const getResp = responses.find((r) => r[0] === 'Principal/get');
|
||||
if (!queryResp || !getResp) {
|
||||
// A refused query makes the chained get fail with
|
||||
// invalidResultReference; only the query's own error says why.
|
||||
const error = (responses.find((r) => r[0] === 'error' && r[2] === '0')
|
||||
?? responses.find((r) => r[0] === 'error'))?.[1] as { type?: string; description?: string } | undefined;
|
||||
if (error?.type === 'forbidden') {
|
||||
return {
|
||||
principals: await getKnownPrincipals(accountId),
|
||||
status: 'restricted',
|
||||
directoryQueriesOff: error.description === DIRECTORY_QUERIES_OFF,
|
||||
};
|
||||
}
|
||||
console.warn('[files] listing principals failed', error);
|
||||
return { principals: all, status: 'error' };
|
||||
}
|
||||
const ids = (queryResp[1].ids ?? []) as string[];
|
||||
all.push(...((getResp[1].list ?? []) as Principal[]));
|
||||
if (ids.length < pageSize) break;
|
||||
position += ids.length;
|
||||
}
|
||||
return { principals: all, status: 'ok' };
|
||||
} catch (err) {
|
||||
console.warn('[files] listing principals failed', err);
|
||||
return { principals: all, status: 'error' };
|
||||
}
|
||||
return all;
|
||||
}
|
||||
|
||||
/**
|
||||
* Principal/get without ids. When the directory can't be listed, Stalwart 1.0
|
||||
* still returns the principals the user already knows: their own account,
|
||||
* their groups and the accounts that shared something with them. Empty when
|
||||
* that is refused too.
|
||||
*/
|
||||
async function getKnownPrincipals(accountId: string): Promise<Principal[]> {
|
||||
try {
|
||||
const res = await jmapClient.request(
|
||||
[['Principal/get', { accountId, ids: null, properties: PRINCIPAL_PROPERTIES }, '0']],
|
||||
principalsUsing(),
|
||||
);
|
||||
const [method, result] = res.methodResponses?.[0] ?? [];
|
||||
if (method !== 'Principal/get') return [];
|
||||
return (result.list ?? []) as Principal[];
|
||||
} catch (err) {
|
||||
console.warn('[files] reading known principals failed', err);
|
||||
return [];
|
||||
}
|
||||
}
|
||||
|
||||
/** The principals of {@link listPrincipals}, without the status. */
|
||||
export async function getPrincipals(): Promise<Principal[]> {
|
||||
return (await listPrincipals()).principals;
|
||||
}
|
||||
|
||||
// The principal id that represents the logged-in user (excluded from the
|
||||
|
||||
Reference in New Issue
Block a user