diff --git a/src/api/__tests__/files.test.ts b/src/api/__tests__/files.test.ts index ba3f6577..d49bac33 100644 --- a/src/api/__tests__/files.test.ts +++ b/src/api/__tests__/files.test.ts @@ -45,6 +45,7 @@ import { listPrincipals, moveFileNode, renameFileNode, + resolveFileNodeId, setFileNodeShare, supportsSharing, uploadFileBatch, @@ -304,6 +305,8 @@ function fakeFilesServer(nodes: FakeNode[], opts: { // Stalwart before 0.16.6 returns leaf files only. queryOmitsFolders?: boolean; queryFails?: boolean; + // Stalwart 1.0 refuses a return-all /get past the limit (#1385). + refusesReturnAll?: boolean; }): MethodCall[][] { mockMaxObjectsInGet.mockReturnValue(opts.maxObjectsInGet); if (opts.maxCallsInRequest) mockMaxCallsInRequest.mockReturnValue(opts.maxCallsInRequest); @@ -313,6 +316,9 @@ function fakeFilesServer(nodes: FakeNode[], opts: { const methodResponses = methodCalls.map(([method, args, callId]) => { if (method === 'FileNode/get') { const ids = args.ids as string[] | null; + if (ids === null && opts.refusesReturnAll && nodes.length > opts.maxObjectsInGet) { + return ['error', { type: 'requestTooLarge' }, callId]; + } if (ids === null) return [method, { list: nodes.slice(0, opts.maxObjectsInGet) }, callId]; if (ids.length > opts.maxObjectsInGet) return ['error', { type: 'requestTooLarge' }, callId]; return [method, { @@ -420,6 +426,27 @@ describe('listing past maxObjectsInGet (#1069)', () => { expect(askedForHidden).toHaveLength(1); }); + // #1385: Stalwart 1.0 answers the return-all /get of a larger account with + // requestTooLarge instead of a first page. + it('lists every node when Stalwart 1.0 refuses the return-all /get', async () => { + const all = [ + fakeFolder('d1'), + ...Array.from({ length: 6 }, (_, i) => fakeFile(`f${i}`, 'd1')), + ]; + fakeFilesServer(all, { maxObjectsInGet: 3, refusesReturnAll: true }); + + const nodes = await getAllFileNodes(); + expect(sortedIds(nodes)).toEqual(sortedIds(all)); + }); + + it('reports a failed listing of the own drive instead of an empty one', async () => { + fakeFilesServer([fakeFile('f0'), fakeFile('f1'), fakeFile('f2')], { + maxObjectsInGet: 2, refusesReturnAll: true, queryFails: true, + }); + + await expect(getAllFileNodesAcrossAccounts()).rejects.toThrow(); + }); + it('keeps the first page when the query is refused', async () => { fakeFilesServer([fakeFile('f0'), fakeFile('f1'), fakeFile('f2')], { maxObjectsInGet: 2, queryFails: true }); @@ -674,6 +701,87 @@ describe('fileNodeKind', () => { }); }); +describe('Stalwart 1.0.0 (File Storage draft 18)', () => { + // The filenode account capability of 1.0.0 lists `supportedNodeTypes`; 0.16.27's doesn't. + const CAP_100 = { maxFileNodeDepth: 64, forbiddenNameChars: '/<>:"\\|?*', supportedNodeTypes: ['Directory', 'File', 'SymLink'] }; + const CAP_016 = { maxFileNodeDepth: null, forbiddenNameChars: '/<>:"\\|?*' }; + + function session(own: object, shared?: object) { + setSession({ + primaryAccounts: { [CAPABILITIES.FILES]: 'acc-1' }, + accounts: { + 'acc-1': { name: 'me@example.com', isPersonal: true, accountCapabilities: { [CAPABILITIES.FILES]: own } }, + ...(shared ? { 'acc-2': { name: 'Team', isPersonal: false, accountCapabilities: shared } } : {}), + }, + }); + } + + const creates = () => mockRequest.mock.calls.map(([calls]) => Object.values(calls[0][1].create ?? {})[0]); + + // Seen live: 1.0.0 answers a create without `@type` with "@type is required + // on creation." and one with `type` with "Invalid property.". + it('creates folders and files with @type and mediaType', async () => { + session(CAP_100); + mockUploadBlobOnce('blob-up', 'text/plain'); + mockRequest + .mockResolvedValueOnce({ methodResponses: [['FileNode/set', { created: { 'new-dir': { id: 'd1' } } }, '0']] }) + .mockResolvedValueOnce({ methodResponses: [['FileNode/set', { created: { 'new-file': { id: 'f1', size: 3 } } }, '0']] }) + .mockResolvedValueOnce({ methodResponses: [['FileNode/set', { created: { 'new-file': { id: 'f2', size: 3 } } }, '0']] }); + + const dir = await createFolder('New', null); + const { uploaded } = await uploadFileBatch([{ uri: 'file:///b', name: 'b.txt', mimeType: 'text/plain' }], 'd1', []); + const copy = await copyFileNode(uploaded[0], null, 'b (1).txt'); + + expect(creates()).toEqual([ + { '@type': 'Directory', name: 'New' }, + { '@type': 'File', name: 'b.txt', mediaType: 'text/plain', blobId: 'blob-up', parentId: 'd1' }, + { '@type': 'File', name: 'b (1).txt', mediaType: 'text/plain', blobId: 'blob-up' }, + ]); + expect(dir).toMatchObject({ id: 'd1', name: 'New', nodeType: 'directory' }); + expect(copy).toMatchObject({ id: 'f2', type: 'text/plain', nodeType: 'file' }); + }); + + it('keeps the 0.16 shape on Stalwart 0.16, which refuses @type', async () => { + session(CAP_016); + mockRequest.mockResolvedValue({ methodResponses: [['FileNode/set', { created: { 'new-dir': { id: 'd1' }, 'new-file': { id: 'f1' } } }, '0']] }); + + await createFolder('New', null); + await copyFileNode({ id: 'f0', name: 'a.txt', type: 'text/plain', blobId: 'b1' }, null); + + expect(creates()).toEqual([ + { name: 'New' }, + { name: 'a.txt', type: 'text/plain', blobId: 'b1' }, + ]); + }); + + it('reads the kind from @type and the media type from mediaType', async () => { + session(CAP_100); + mockRequest.mockResolvedValue({ + methodResponses: [['FileNode/get', { + list: [ + { id: 'd1', name: 'Docs', parentId: null, '@type': 'Directory' }, + { id: 'l1', name: 'latest', parentId: 'd1', '@type': 'SymLink' }, + { id: 'f1', name: 'a.pdf', parentId: 'd1', '@type': 'File', mediaType: 'application/pdf', blobId: 'b1', size: 3 }, + ], + }, '0']], + }); + + const nodes = await getAllFileNodes(); + + expect(mockRequest.mock.calls[0][0][0][1].properties).toEqual(expect.arrayContaining(['@type', 'mediaType'])); + const byId = Object.fromEntries(nodes.map((n) => [n.id, n])); + expect(fileNodeKind(byId.d1)).toBe('directory'); + // Without @type the blob rule made this an empty folder. + expect(isSymlink(byId.l1)).toBe(true); + expect(isFolder(byId.l1)).toBe(false); + expect(byId.f1).toMatchObject({ type: 'application/pdf', nodeType: 'file' }); + }); +}); + +function mockUploadBlobOnce(blobId: string, type: string) { + (uploadBlob as ReturnType).mockResolvedValueOnce({ blobId, type, size: 3 }); +} + describe('"Shared with me" placeholders', () => { const RIGHTS_NONE = { mayRead: false, mayAddChildren: false, mayRename: false, mayDelete: false, mayModifyContent: false, mayShare: false }; const RIGHTS_READ = { ...RIGHTS_NONE, mayRead: true }; @@ -907,6 +1015,86 @@ describe('moveFileNode', () => { }); }); +describe('writes in a shared account', () => { + const CAP_100 = { forbiddenNameChars: '/<>:"\\|?*', supportedNodeTypes: ['Directory', 'File', 'SymLink'] }; + + beforeEach(() => { + // A Stalwart 1.0.0 session where the shared drive publishes no filenode + // capability of its own (non-personal accounts don't always). + setSession({ + primaryAccounts: { [CAPABILITIES.FILES]: 'acc-1' }, + accounts: { + 'acc-1': { name: 'me@example.com', isPersonal: true, accountCapabilities: { [CAPABILITIES.FILES]: CAP_100 } }, + 'acc-2': { name: 'Team', isPersonal: false, accountCapabilities: {} }, + }, + }); + }); + + const sent = () => mockRequest.mock.calls.map(([calls]) => calls[0][1] as Record); + + it('creates folders and uploads in the account that holds the folder, with the id it knows', async () => { + (uploadBlob as ReturnType).mockResolvedValueOnce({ blobId: 'blob-up', type: 'text/plain', size: 3 }); + mockRequest + .mockResolvedValueOnce({ methodResponses: [['FileNode/set', { created: { 'new-dir': { id: 'n1' } } }, '0']] }) + .mockResolvedValueOnce({ methodResponses: [['FileNode/set', { created: { 'new-file': { id: 'n2' } } }, '0']] }); + + const dir = await createFolder('New', 'acc-2:s1'); + const { uploaded } = await uploadFileBatch([{ uri: 'file:///a', name: 'a.txt', mimeType: 'text/plain' }], 'acc-2:s1', []); + + expect(sent().map((a) => [a.accountId, Object.values(a.create)[0]])).toEqual([ + ['acc-2', { '@type': 'Directory', name: 'New', parentId: 's1' }], + ['acc-2', { '@type': 'File', name: 'a.txt', mediaType: 'text/plain', blobId: 'blob-up', parentId: 's1' }], + ]); + // Named like the listing names that account's nodes. + expect(dir).toMatchObject({ id: 'acc-2:n1', parentId: 'acc-2:s1', accountId: 'acc-2', isShared: true }); + expect(uploaded[0].id).toBe('acc-2:n2'); + }); + + it('renames, moves, duplicates and shares in the node\'s account', async () => { + mockRequest.mockImplementation(async (calls: MethodCall[]) => { + const args = calls[0][1] as Record; + if (args.create) return { methodResponses: [['FileNode/set', { created: { 'new-file': { id: 'c1' } } }, '0']] }; + const id = Object.keys(args.update)[0]; + return { methodResponses: [['FileNode/set', { updated: { [id]: null } }, '0']] }; + }); + + await renameFileNode('acc-2:f1', 'b.txt'); + await moveFileNode('acc-2:f1', 'acc-2:s1'); + await copyFileNode({ id: 'acc-2:f1', name: 'b.txt', type: 'text/plain', blobId: 'b1', accountId: 'acc-2' }, 'acc-2:s1', 'b (1).txt'); + await setFileNodeShare('acc-2:f1', 'p1', null); + + expect(sent()).toEqual([ + { accountId: 'acc-2', update: { f1: { name: 'b.txt' } } }, + { accountId: 'acc-2', update: { f1: { parentId: 's1' } } }, + { accountId: 'acc-2', create: { 'new-file': { '@type': 'File', name: 'b (1).txt', mediaType: 'text/plain', blobId: 'b1', parentId: 's1' } } }, + { accountId: 'acc-2', update: { f1: { 'shareWith/p1': null } } }, + ]); + }); + + it('refuses a move into another account before sending anything', async () => { + await expect(moveFileNode('acc-2:f1', 'own-dir')).rejects.toThrow('Files cannot be moved between accounts'); + await expect(moveFileNode('own-file', 'acc-2:s1')).rejects.toThrow('Files cannot be moved between accounts'); + expect(mockRequest).not.toHaveBeenCalled(); + }); + + it('deletes each account\'s nodes in that account', async () => { + mockRequest.mockResolvedValue({ methodResponses: [['FileNode/set', { destroyed: [] }, '0']] }); + + await deleteFileNodes(['own-1', 'acc-2:f1', 'acc-2:f2']); + + expect(sent().map((a) => [a.accountId, a.destroy])).toEqual([ + ['acc-1', ['own-1']], + ['acc-2', ['f1', 'f2']], + ]); + }); + + it('leaves an id alone whose prefix is no account of the session', () => { + expect(resolveFileNodeId('nope:f1')).toEqual({ accountId: 'acc-1', id: 'nope:f1' }); + expect(resolveFileNodeId('acc-1:f1')).toEqual({ accountId: 'acc-1', id: 'acc-1:f1' }); + expect(resolveFileNodeId(null)).toEqual({ accountId: 'acc-1', id: null }); + }); +}); + describe('FileNode/set refused by the server', () => { const methodError = { methodResponses: [['error', { type: 'forbidden', description: 'No write access' }, '0']], diff --git a/src/api/__tests__/live-stalwart-files.test.ts b/src/api/__tests__/live-stalwart-files.test.ts new file mode 100644 index 00000000..8a879c50 --- /dev/null +++ b/src/api/__tests__/live-stalwart-files.test.ts @@ -0,0 +1,210 @@ +/** + * Live check of the Files API against a real Stalwart server. + * + * Opt-in like live-stalwart.test.ts: skipped unless `BULWARK_LIVE_JMAP` + * and `BULWARK_LIVE_PASS` are set. + * + * BULWARK_LIVE_JMAP=http://10.10.10.62:8120 \ + * BULWARK_LIVE_USER=e2e-phone-dark@example.org \ + * BULWARK_LIVE_PASS=… npx vitest run src/api/__tests__/live-stalwart-files.test.ts + * + * Everything it makes lives in one scratch folder that is deleted afterwards. + * + * Verified against stalwartlabs/stalwart:v0.16.27 and v1.0.0. + */ +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; + +// The app uploads picked files from a device URI; here the "URI" names bytes +// held in memory, which go up through the same upload endpoint. +const pickedFiles = new Map(); +vi.mock('../blob', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + uploadBlob: (uri: string, type: string) => { + const bytes = pickedFiles.get(uri); + if (!bytes) throw new Error(`no bytes for ${uri}`); + return actual.uploadBytes(bytes, type); + }, + }; +}); + +import { JMAPClient } from '../jmap-client'; +import { CAPABILITIES } from '../types'; +import type { FileNode } from '../types'; +import { + copyFileNode, + createFolder, + deleteFileNodes, + fileNodeKind, + filesAccountId, + getAllFileNodesAcrossAccounts, + getFileNodeDownloadUrl, + listFolder, + moveFileNode, + renameFileNode, + uploadFileBatch, +} from '../files'; + +const SERVER = process.env.BULWARK_LIVE_JMAP; +const USER = process.env.BULWARK_LIVE_USER ?? 'usera@example.org'; +const PASS = process.env.BULWARK_LIVE_PASS ?? ''; +const live = SERVER && PASS ? describe : describe.skip; + +let client: JMAPClient; + +async function connectSingleton(): Promise { + const mod = await import('../jmap-client'); + client = new JMAPClient(); + await client.connect(SERVER!, USER, PASS); + (mod.jmapClient as unknown as JMAPClient).restoreSnapshot(client.snapshot()); +} + +function pick(name: string, text: string) { + const uri = `mem://${name}-${pickedFiles.size}`; + pickedFiles.set(uri, new TextEncoder().encode(text)); + return { uri, name, mimeType: 'text/plain' }; +} + +async function nodes(): Promise { + return getAllFileNodesAcrossAccounts(); +} + +/** Stalwart 1.0 (File Storage draft 18) names the kind `@type`; it lists `supportedNodeTypes`. */ +function draft18(): boolean { + const cap = client.getAccountCapability(CAPABILITIES.FILES, filesAccountId()) as Record | undefined; + return !!cap && 'supportedNodeTypes' in cap; +} + +live('live Stalwart files', () => { + let scratch = ''; + + beforeAll(async () => { + await connectSingleton(); + scratch = (await createFolder(`live-files-${Date.now().toString(36)}`, null)).id; + }, 60_000); + + afterAll(async () => { + if (!scratch) return; + await deleteFileNodes([scratch]).catch(() => undefined); + // Asked by id, so a failed listing can't pass for a clean account. + const res = await client.request([['FileNode/get', { + accountId: filesAccountId(), ids: [scratch], properties: ['id'], + }, '0']], [CAPABILITIES.CORE, CAPABILITIES.FILES]); + expect(res.methodResponses[0][1].notFound).toEqual([scratch]); + }, 120_000); + + it('creates a folder, lists it as a folder and lists what it holds', async () => { + const sub = await createFolder('Sub folder', scratch); + const all = await nodes(); + const listed = all.find((n) => n.id === sub.id); + expect(listed?.name).toBe('Sub folder'); + expect(listed?.parentId).toBe(scratch); + expect(fileNodeKind(listed!)).toBe('directory'); + expect(listFolder(all, scratch).map((n) => n.id)).toContain(sub.id); + }, 60_000); + + it('uploads files, lists them as files with their media type and downloads the bytes', async () => { + const { uploaded, failed } = await uploadFileBatch( + [pick('notes.txt', 'hello from the live test'), pick('con.txt', 'reserved stem')], + scratch, + [], + ); + expect(failed).toEqual([]); + expect(uploaded).toHaveLength(2); + + const all = await nodes(); + const notes = all.find((n) => n.id === uploaded[0].id)!; + expect(notes.name).toBe('notes.txt'); + expect(fileNodeKind(notes)).toBe('file'); + expect(notes.type).toBe('text/plain'); + expect(notes.size).toBe(24); + + const res = await fetch(getFileNodeDownloadUrl(notes), { headers: { Authorization: client.authHeader } }); + expect(res.status).toBe(200); + expect(await res.text()).toBe('hello from the live test'); + }, 60_000); + + it('renames, moves, copies and deletes', async () => { + const box = await createFolder('Box', scratch); + const { uploaded } = await uploadFileBatch([pick('a.txt', 'abc')], scratch, []); + const file = uploaded[0]; + + await renameFileNode(file.id, 'b.txt'); + await moveFileNode(file.id, box.id); + let all = await nodes(); + expect(all.find((n) => n.id === file.id)).toMatchObject({ name: 'b.txt', parentId: box.id }); + + // A folder moves too, and back. + const inner = await createFolder('Inner', scratch); + await moveFileNode(inner.id, box.id); + await renameFileNode(inner.id, 'Inner renamed'); + all = await nodes(); + expect(all.find((n) => n.id === inner.id)).toMatchObject({ name: 'Inner renamed', parentId: box.id }); + + const listed = all.find((n) => n.id === file.id)!; + const copy = await copyFileNode(listed, scratch, 'b (copy).txt'); + all = await nodes(); + const copied = all.find((n) => n.id === copy.id)!; + expect(copied).toMatchObject({ name: 'b (copy).txt', parentId: scratch, size: 3, type: 'text/plain' }); + expect(fileNodeKind(copied)).toBe('file'); + + await deleteFileNodes([copy.id, box.id]); + all = await nodes(); + // The folder's children went with it. + expect(all.filter((n) => [copy.id, box.id, file.id, inner.id].includes(n.id))).toEqual([]); + }, 90_000); + + it('moves a node to the drive root and back', async () => { + const { uploaded } = await uploadFileBatch([pick('root-trip.txt', 'x')], scratch, []); + const id = uploaded[0].id; + try { + await moveFileNode(id, null); + expect((await nodes()).find((n) => n.id === id)?.parentId ?? null).toBeNull(); + } finally { + await moveFileNode(id, scratch); + } + expect((await nodes()).find((n) => n.id === id)?.parentId).toBe(scratch); + }, 60_000); + + it('cannot create a symbolic link over JMAP on Stalwart 1.0', async () => { + if (!draft18()) return; + // Stalwart 1.0 lists SymLink among its node types but refuses every + // `target`, so the app offers no way to make one. + const res = await client.request([['FileNode/set', { + accountId: filesAccountId(), + create: { l: { '@type': 'SymLink', name: 'link', parentId: scratch, target: 'Sub folder' } }, + }, '0']], [CAPABILITIES.CORE, CAPABILITIES.FILES]); + const [, result] = res.methodResponses[0] as unknown as [string, { created?: Record; notCreated?: Record }]; + expect(result.created?.l).toBeUndefined(); + expect(result.notCreated?.l?.properties).toEqual(['target']); + }, 30_000); + + // #1385: past maxObjectsInGet, 0.16 cuts the return-all /get short and 1.0 + // refuses it with requestTooLarge. + it('lists an account holding more nodes than one FileNode/get returns', async () => { + const max = client.getMaxObjectsInGet(); + const perSet = client.getMaxObjectsInSet(); + const bulk = await createFolder('bulk', scratch); + const count = max + 5; + const made: string[] = []; + for (let start = 0; start < count; start += perSet) { + const create: Record> = {}; + for (let i = start; i < Math.min(count, start + perSet); i++) { + create[`n${i}`] = { name: `n${i}`, parentId: bulk.id, ...(draft18() ? { '@type': 'Directory' } : {}) }; + } + const res = await client.request([['FileNode/set', { accountId: filesAccountId(), create }, '0']], + [CAPABILITIES.CORE, CAPABILITIES.FILES]); + const [, result] = res.methodResponses[0] as unknown as [string, { created?: Record }]; + made.push(...Object.values(result.created ?? {}).map((c) => c.id)); + } + expect(made).toHaveLength(count); + + const all = await nodes(); + const ids = new Set(all.map((n) => n.id)); + expect(made.filter((id) => !ids.has(id))).toEqual([]); + expect(ids.has(scratch)).toBe(true); + + await deleteFileNodes([bulk.id]); + }, 180_000); +}); diff --git a/src/api/files.ts b/src/api/files.ts index a96b6759..86193963 100644 --- a/src/api/files.ts +++ b/src/api/files.ts @@ -40,6 +40,37 @@ export function isSymlink(node: Pick): boolean return fileNodeKind(node) === 'symlink'; } +/** + * Stalwart 1.0.0 implements File Storage draft 18, which names the kind + * `@type` ("File", "Directory", "SymLink") and the media type `mediaType`. + * 0.16 calls them `nodeType` and `type`. Each server leaves the other's names + * out of FileNode/get and refuses them in a create ("@type is required on + * creation." on 1.0.0, "Invalid property." for `@type` on 0.16.27). + */ +const DRAFT_NODE_TYPES: Record = { File: 'file', Directory: 'directory', SymLink: 'symlink' }; + +type DraftFileNode = { '@type'?: unknown; mediaType?: unknown }; + +/** A node of either draft, with the kind in `nodeType` and the media type in `type`. */ +export function fromDraftFileNode>(node: T & DraftFileNode): T { + const kind = typeof node['@type'] === 'string' ? DRAFT_NODE_TYPES[node['@type']] : undefined; + const mediaType = typeof node.mediaType === 'string' ? node.mediaType : undefined; + if (!kind && mediaType === undefined) return node; + return { + ...node, + ...(kind && !node.nodeType ? { nodeType: kind } : {}), + ...(mediaType !== undefined && node.type == null ? { type: mediaType } : {}), + }; +} + +/** The properties of a draft-18 FileNode/set create, from 0.16 ones (no blob: a folder). */ +function toDraftFileNodeCreate(props: Record): Record { + const { type, ...rest } = props; + return rest.blobId == null + ? { '@type': 'Directory', ...rest } + : { '@type': 'File', ...rest, mediaType: type }; +} + // Stalwart omits shareWith/myRights from FileNode/get unless they are // requested explicitly, so the share sheet and indicators must name them here. // The modification timestamp is `modified` (RFC draft-ietf-jmap-filenode); @@ -50,6 +81,8 @@ const FILE_NODE_PROPERTIES = [ // Tells 1.0's symlinks from folders (see fileNodeKind); servers before // 0.16.6 skip it. 'nodeType', + // `nodeType` and `type` as Stalwart 1.0.0 names them (see fromDraftFileNode). + '@type', 'mediaType', 'shareWith', 'myRights', ]; @@ -134,6 +167,50 @@ export function isCrossAccountId(id: string | null | undefined): boolean { return id != null && id.includes(':'); } +/** + * The account and bare id behind a FileNode id as the listings name it. A + * write must go to the account that holds the node, with the id that + * account knows: sent to the user's own account, "acc:node" is an unknown + * id at best and, stripped, could name an unrelated node of the user's own. + * `null` is the user's own drive root. + */ +export function resolveFileNodeId(id: string | null): { accountId: string; id: string | null } { + const primary = filesAccountId(); + if (id === null) return { accountId: primary, id: null }; + const sep = id.indexOf(':'); + if (sep > 0) { + const accountId = id.slice(0, sep); + if (accountId !== primary && filesCapableAccountIds().includes(accountId)) { + return { accountId, id: id.slice(sep + 1) }; + } + } + return { accountId: primary, id }; +} + +/** A node created or listed in `accountId`, with ids namespaced as the listings do. */ +function namespaceFileNode(node: FileNode, accountId: string): FileNode { + if (accountId === filesAccountId()) return node; + return { + ...node, + id: `${accountId}:${node.id}`, + parentId: node.parentId == null ? null : `${accountId}:${node.parentId}`, + accountId, + isShared: true, + }; +} + +/** + * The bare id of `parentId` in `accountId`. Folders hold nodes of their own + * account only, so a parent elsewhere is refused before anything is sent. + */ +function parentInAccount(parentId: string | null, accountId: string): string | null { + const parent = resolveFileNodeId(parentId); + if (parent.accountId !== accountId) { + throw new JMAPSetError('invalidProperties', 'Files cannot be moved between accounts', ['parentId']); + } + return parent.id; +} + /** Ids asked for per FileNode/query page; Stalwart clamps it to queryMaxResults (5000 by default). */ const FILE_NODE_QUERY_PAGE = 5000; /** Safety bound on how many FileNode ids one listing pages through. */ @@ -159,12 +236,15 @@ async function fetchAllFileNodes(accountId: string): Promise { using, ); const result = res.methodResponses[0]; - if (!result || result[0] === 'error') { + // Stalwart 1.0 refuses the return-all /get instead of cutting it short + // (#1385); then every node comes from the query below. + const tooMany = result?.[0] === 'error' && result[1]?.type === 'requestTooLarge'; + if (!result || (result[0] === 'error' && !tooMany)) { throw new Error(result?.[1]?.description || 'FileNode list failed'); } - const firstPage = (result[1].list ?? []) as FileNode[]; + const firstPage = tooMany ? [] : ((result[1].list ?? []) as FileNode[]).map(fromDraftFileNode); const maxObjects = jmapClient.getMaxObjectsInGet(); - if (firstPage.length < maxObjects) return firstPage; + if (!tooMany && firstPage.length < maxObjects) return firstPage; const known = new Map(firstPage.map((node) => [node.id, node])); // Several /get calls share one request, so a large account costs a few @@ -178,7 +258,7 @@ async function fetchAllFileNodes(accountId: string): Promise { if (r[0] !== 'FileNode/get') { throw new Error(r[1]?.description || 'FileNode/get failed'); } - for (const node of (r[1].list ?? []) as FileNode[]) known.set(node.id, node); + for (const node of (r[1].list ?? []) as FileNode[]) known.set(node.id, fromDraftFileNode(node)); } } }; @@ -223,6 +303,8 @@ async function fetchAllFileNodes(accountId: string): Promise { } } catch (err) { // A partial tree beats none: keep what was read, as before #1069. + // With nothing read there is no tree to show, so say why. + if (known.size === 0) throw err; console.warn(`[files] listing for account ${accountId} is incomplete past ${maxObjects} nodes`, err); } @@ -278,7 +360,10 @@ export async function getAllFileNodesAcrossAccounts(): Promise { ...(!isPrimary && isDiscoverableOnly(node) ? { discoverableOnly: true } : {}), }); } - } catch { + } catch (err) { + // A failed listing of the user's own drive is an error to show, not + // an empty drive (#1385 looked like "No files yet"). + if (isPrimary) throw err; // A single unreachable shared account shouldn't hide the user's own // files; skip it and keep aggregating. } @@ -325,38 +410,71 @@ export function listFolder(nodes: FileNode[], parentId: string | null): FileNode return nodes.filter((n) => !n.discoverableOnly && (n.parentId ?? null) === parentId); } +/** + * Stalwart 1.0.0 (File Storage draft 18) wants `@type` and `mediaType` in a + * create (see toDraftFileNodeCreate). Of the drafts Stalwart has shipped, + * only that one lists `supportedNodeTypes`. A shared account may come + * without the capability; the user's own files account stands in, as + * Stalwart publishes the same one for every account. + */ +function usesDraftFileNodeCreate(accountId: string): boolean { + // Read the accounts' own capabilities: the session-wide one is empty on + // both versions. + const accounts = jmapClient.currentSession?.accounts; + const cap = accounts?.[accountId]?.accountCapabilities?.[CAPABILITIES.FILES] + ?? accounts?.[filesAccountId()]?.accountCapabilities?.[CAPABILITIES.FILES]; + return !!cap && typeof cap === 'object' && 'supportedNodeTypes' in cap; +} + +/** One FileNode/set create in `accountId`, in the shape that server takes. */ +async function createFileNodeIn( + accountId: string, + props: Record, + key: string, + fallback: string, +): Promise { + const wire = usesDraftFileNodeCreate(accountId) ? toDraftFileNodeCreate(props) : props; + const res = await jmapClient.request( + [['FileNode/set', { accountId, create: { [key]: wire } }, '0']], + fileUsing(), + ); + const result = requireMethodResult(res, '0', 'FileNode/set'); + const created = result.created?.[key]; + if (!created) { + throw fileSetError(result.notCreated?.[key], fallback); + } + // The create response carries only server-set properties; keep the ones + // sent so callers can rely on name, parentId and type. + return fromDraftFileNode({ ...wire, ...created } as FileNode); +} + +/** A new folder in `parentId`, which may be a folder of a shared account. */ export async function createFolder( name: string, parentId: string | null, ): Promise { - const accountId = filesAccountId(); + const parent = resolveFileNodeId(parentId); const props: Record = { name: acceptedFileName(name, getFileNameRules()) }; - if (parentId !== null) props.parentId = parentId; - - const res = await jmapClient.request( - [['FileNode/set', { accountId, create: { 'new-dir': props } }, '0']], - fileUsing(), - ); - const result = requireMethodResult(res, '0', 'FileNode/set'); - const created = result.created?.['new-dir']; - if (!created) { - throw fileSetError(result.notCreated?.['new-dir'], 'Create folder failed'); - } - return { ...props, ...created } as FileNode; + if (parent.id !== null) props.parentId = parent.id; + const created = await createFileNodeIn(parent.accountId, props, 'new-dir', 'Create folder failed'); + return namespaceFileNode(created, parent.accountId); } export async function updateFileNode( id: string, updates: Partial>, ): Promise { - const accountId = filesAccountId(); + const node = resolveFileNodeId(id); + const nodeId = node.id as string; + const patch: Record = { ...updates }; + if ('parentId' in updates) patch.parentId = parentInAccount(updates.parentId ?? null, node.accountId); const res = await jmapClient.request( - [['FileNode/set', { accountId, update: { [id]: updates } }, '0']], + [['FileNode/set', { accountId: node.accountId, update: { [nodeId]: patch } }, '0']], fileUsing(), ); // A method-level error (e.g. forbidden) has no notUpdated entry, so without // this check a refused rename or move reported success. - const notUpdated = requireMethodResult(res, '0', 'FileNode/set').notUpdated?.[id]; + const notUpdated = requireMethodResult(res, '0', 'FileNode/set').notUpdated?.[nodeId]; if (notUpdated) throw fileSetError(notUpdated, 'Update failed'); } @@ -366,32 +484,42 @@ export async function renameFileNode(id: string, newName: string): Promise // Re-parent a node. `null` moves it to the drive root (the property must be // sent explicitly as null, omitting it would leave the node where it is). +// The new parent must be in the node's own account. export async function moveFileNode(id: string, parentId: string | null): Promise { await updateFileNode(id, { parentId }); } export async function deleteFileNodes(ids: string[]): Promise { if (ids.length === 0) return; - const accountId = filesAccountId(); + // Each account gets its own FileNode/set with the ids it knows. + const byAccount = new Map(); + for (const id of ids) { + const node = resolveFileNodeId(id); + const list = byAccount.get(node.accountId) ?? []; + list.push(node.id as string); + byAccount.set(node.accountId, list); + } // One FileNode/set per `maxObjectsInSet` ids: a request over the limit // fails whole (RFC 8620 §3.6.1), so a large selection deleted nothing. // Every batch runs; refusals are reported once all of them are done. const refused: SetError[] = []; - for (const slice of batched(ids, jmapClient.getMaxObjectsInSet())) { - const res = await jmapClient.request( - [['FileNode/set', { - accountId, - destroy: slice, - // The server removes descendant nodes of destroyed folders. - onDestroyRemoveChildren: true, - }, '0']], - fileUsing(), - ); - const notDestroyed = requireMethodResult(res, '0', 'FileNode/set').notDestroyed as - | Record - | undefined; - // A node inside a folder an earlier batch removed is already gone. - refused.push(...Object.values(notDestroyed ?? {}).filter((err) => err?.type !== 'notFound')); + for (const [accountId, accountIds] of byAccount) { + for (const slice of batched(accountIds, jmapClient.getMaxObjectsInSet())) { + const res = await jmapClient.request( + [['FileNode/set', { + accountId, + destroy: slice, + // The server removes descendant nodes of destroyed folders. + onDestroyRemoveChildren: true, + }, '0']], + fileUsing(), + ); + const notDestroyed = requireMethodResult(res, '0', 'FileNode/set').notDestroyed as + | Record + | undefined; + // A node inside a folder an earlier batch removed is already gone. + refused.push(...Object.values(notDestroyed ?? {}).filter((err) => err?.type !== 'notFound')); + } } if (refused.length > 0) { throw fileSetError(refused[0], `Failed to delete ${refused.length} item(s)`); @@ -414,24 +542,17 @@ async function createFileNodeFromBlob( type: string | undefined, parentId: string | null, ): Promise { - const accountId = filesAccountId(); + // The blob may come from the user's own account (an upload) or another + // one (a copy): Stalwart takes any blob the user may read. + const parent = resolveFileNodeId(parentId); const props: Record = { name: acceptedFileName(name, getFileNameRules()), type: mediaTypeEssence(type), blobId, }; - if (parentId !== null) props.parentId = parentId; - - const res = await jmapClient.request( - [['FileNode/set', { accountId, create: { 'new-file': props } }, '0']], - fileUsing(), - ); - const result = requireMethodResult(res, '0', 'FileNode/set'); - const created = result.created?.['new-file']; - if (!created) { - throw fileSetError(result.notCreated?.['new-file'], 'Upload failed'); - } - return { ...props, ...created } as FileNode; + if (parent.id !== null) props.parentId = parent.id; + const created = await createFileNodeIn(parent.accountId, props, 'new-file', 'Upload failed'); + return namespaceFileNode(created, parent.accountId); } export async function uploadFileNode( @@ -529,19 +650,20 @@ export async function setFileNodeShare( principalId: string, rights: FileNodeRights | null, ): Promise { - const accountId = filesAccountId(); + const node = resolveFileNodeId(fileNodeId); + const nodeId = node.id as string; const res = await jmapClient.request( [['FileNode/set', { - accountId, - update: { [fileNodeId]: { [`shareWith/${principalId}`]: rights } }, + accountId: node.accountId, + update: { [nodeId]: { [`shareWith/${principalId}`]: rights } }, }, '0']], fileUsing(), ); const result = requireMethodResult(res, '0', 'FileNode/set'); - if (result.notUpdated?.[fileNodeId]) { - throw fileSetError(result.notUpdated[fileNodeId], 'Failed to update file share'); + if (result.notUpdated?.[nodeId]) { + throw fileSetError(result.notUpdated[nodeId], 'Failed to update file share'); } - if (!result.updated || !(fileNodeId in result.updated)) { + if (!result.updated || !(nodeId in result.updated)) { throw new Error('Server did not confirm the share update'); } } diff --git a/src/lib/__tests__/files-access.test.ts b/src/lib/__tests__/files-access.test.ts new file mode 100644 index 00000000..024b8e16 --- /dev/null +++ b/src/lib/__tests__/files-access.test.ts @@ -0,0 +1,59 @@ +import { describe, it, expect } from 'vitest'; +import type { FileNode, FileNodeRights } from '../../api/types'; +import { fileNodeAccess, mayCreateIn } from '../files-access'; + +// What the Files screen offers inside folders other accounts shared with the +// user. It used to offer no write there at all, since every write went to +// the user's own account. + +const NONE: FileNodeRights = { + mayRead: true, mayAddChildren: false, mayRename: false, mayDelete: false, mayModifyContent: false, mayShare: false, +}; +const WRITE: FileNodeRights = { ...NONE, mayAddChildren: true, mayRename: true, mayDelete: true, mayModifyContent: true }; + +const node = (id: string, parentId: string | null, extra: Partial = {}): FileNode => + ({ id, parentId, name: id, type: '', ...extra }); + +const own = [node('d1', null, { blobId: null }), node('f1', 'd1', { blobId: 'b1' })]; +const shared = (rootRights: FileNodeRights, childRights: FileNodeRights) => [ + // Stalwart 1.0's placeholder above the shared folder. + node('acc-2:top', null, { blobId: null, isShared: true, discoverableOnly: true, myRights: { ...NONE, mayRead: false } }), + node('acc-2:s1', 'acc-2:top', { blobId: null, isShared: true, myRights: rootRights }), + node('acc-2:f2', 'acc-2:s1', { blobId: 'b2', isShared: true, myRights: childRights }), +]; + +describe('fileNodeAccess', () => { + it('lets the user do everything with their own nodes', () => { + expect(fileNodeAccess(own[1], own)).toEqual({ rename: true, delete: true, move: true, duplicate: true }); + }); + + it('follows the rights inside a folder shared with write access', () => { + const nodes = [...own, ...shared(WRITE, WRITE)]; + expect(fileNodeAccess(nodes.at(-1)!, nodes)).toEqual({ rename: true, delete: true, move: false, duplicate: true }); + }); + + it('offers nothing inside a folder shared read-only', () => { + const nodes = shared(NONE, NONE); + expect(fileNodeAccess(nodes.at(-1)!, nodes)).toEqual({ rename: false, delete: false, move: false, duplicate: false }); + }); + + it('never renames or deletes the shared root, even with the rights', () => { + const nodes = shared(WRITE, WRITE); + expect(fileNodeAccess(nodes[1], nodes)).toEqual({ rename: false, delete: false, move: false, duplicate: false }); + }); +}); + +describe('mayCreateIn', () => { + it('creates in the own drive and its folders', () => { + expect(mayCreateIn(null, own)).toBe(true); + expect(mayCreateIn('d1', own)).toBe(true); + }); + + it('creates in a shared folder only with mayAddChildren', () => { + expect(mayCreateIn('acc-2:s1', shared(WRITE, NONE))).toBe(true); + expect(mayCreateIn('acc-2:s1', shared(NONE, NONE))).toBe(false); + // A placeholder folder is never a place to write. + expect(mayCreateIn('acc-2:top', shared(WRITE, NONE))).toBe(false); + expect(mayCreateIn('gone', own)).toBe(false); + }); +}); diff --git a/src/lib/files-access.ts b/src/lib/files-access.ts new file mode 100644 index 00000000..641ae2f2 --- /dev/null +++ b/src/lib/files-access.ts @@ -0,0 +1,55 @@ +import type { FileNode } from '../api/types'; + +/** + * What the Files screen offers for a node another account shared with the + * user (or a node of a shared drive). The writes go to that account + * (api/files resolveFileNodeId); what the server allows there is the node's + * `myRights`. Own nodes may do everything, as before. + * + * A shared root (the node the user was given, at the top of "Shared with + * me") is never renamed, deleted or moved from here: it sits in the owner's + * tree, under folders the user can't see. Moving any shared node is left + * out, like in the webmail: a node can only move within its own account, + * and the picker lists the user's own folders. + */ +export interface FileNodeAccess { + rename: boolean; + delete: boolean; + move: boolean; + /** A copy next to the original: needs the right to add to its folder. */ + duplicate: boolean; +} + +const ALL: FileNodeAccess = { rename: true, delete: true, move: true, duplicate: true }; + +function sharedParent(node: FileNode, nodes: FileNode[]): FileNode | undefined { + if (node.parentId == null) return undefined; + return nodes.find((n) => n.id === node.parentId && !n.discoverableOnly); +} + +export function fileNodeAccess(node: FileNode, nodes: FileNode[]): FileNodeAccess { + if (!node.isShared) return ALL; + const parent = sharedParent(node, nodes); + if (!parent) return { rename: false, delete: false, move: false, duplicate: false }; + const rights = node.myRights; + return { + rename: !!rights?.mayRename, + delete: !!rights?.mayDelete, + move: false, + duplicate: !!parent.myRights?.mayAddChildren, + }; +} + +/** + * Whether new folders and uploads may go into the folder `folderId` (null: + * the user's own drive root). Inside a shared folder only with its + * `mayAddChildren`; Stalwart 0.16 reports a child of a shared folder with + * every right false, so there it is offered in the shared folder itself. + */ +export function mayCreateIn(folderId: string | null, nodes: FileNode[]): boolean { + if (folderId === null) return true; + const folder = nodes.find((n) => n.id === folderId); + if (!folder) return false; + if (!folder.isShared) return true; + return !folder.discoverableOnly && !!folder.myRights?.mayAddChildren; +} diff --git a/src/screens/FilesScreen.tsx b/src/screens/FilesScreen.tsx index ebd6b36b..fb31a433 100644 --- a/src/screens/FilesScreen.tsx +++ b/src/screens/FilesScreen.tsx @@ -46,7 +46,7 @@ function loadDocumentPicker(): DocumentPickerModule | null { import { copyFileNode, createFolder, deleteFileNodes, getAllFileNodesAcrossAccounts, - getFileNameRules, getFileNodeDownloadUrl, getMaxSizeUpload, isCrossAccountId, isFolder, + getFileNameRules, getFileNodeDownloadUrl, getMaxSizeUpload, isFolder, isSymlink, listFolder, moveFileNode, renameFileNode, supportsSharing, uploadFileBatch, } from '../api/files'; import { jmapClient } from '../api/jmap-client'; @@ -62,6 +62,7 @@ import { useLayout, useSheetInsets } from '../lib/use-layout'; import { useBackWhileFocused } from '../lib/use-back-while-focused'; import { filesBackStep } from '../lib/files-back'; import { driveChange, type DriveScope } from '../lib/files-drive'; +import { fileNodeAccess, mayCreateIn, type FileNodeAccess } from '../lib/files-access'; import type { FileNode } from '../api/types'; import { spacing, radius, typography, componentSizes, type ThemePalette } from '../theme/tokens'; import { useColors } from '../theme/colors'; @@ -248,10 +249,9 @@ export default function FilesScreen() { const uploading = uploadProgress != null; const selectionMode = selection.size > 0; const currentParentId = path.length === 0 ? null : path[path.length - 1].id; - // True while browsing inside a folder another principal shared with us. - // Creates/uploads would have to route to the owner's account, which this - // screen doesn't do (matches the webmail's Files app); writes are hidden. - const inSharedSubtree = isCrossAccountId(currentParentId); + // Inside a folder another account shared with us, new folders and uploads + // go to that account, and only where its rights allow them. + const canCreateHere = mayCreateIn(currentParentId, allNodes); const rootLabel = t('files.title', 'Files'); const headerName = path.length === 0 ? rootLabel : path[path.length - 1].name; const sharingEnabled = supportsSharing(); @@ -561,7 +561,7 @@ export default function FilesScreen() { }, [renameTarget, renameValue, loadFiles, checkName, t]); const duplicateFile = useCallback(async (row: FileRow) => { - if (isFolder(row) || isSymlink(row) || row.isShared) return; + if (isFolder(row) || isSymlink(row) || !fileNodeAccess(row, allNodes).duplicate) return; setBusyId(row.id); try { const siblings = new Set( @@ -765,7 +765,7 @@ export default function FilesScreen() { > - {!inSharedSubtree ? ( + {canCreateHere ? ( setNewFolderOpen(true)} hitSlop={8} @@ -775,7 +775,7 @@ export default function FilesScreen() { ) : null} - {!inSharedSubtree ? ( + {canCreateHere ? ( void startUpload()} hitSlop={8} @@ -1096,6 +1096,7 @@ export default function FilesScreen() { setActionsTarget(null)} @@ -1238,6 +1239,8 @@ function PromptModal(props: PromptModalProps) { interface ActionsSheetProps { target: FileRow | null; + /** The writes the target allows (lib/files-access). */ + access: FileNodeAccess | null; sharingEnabled: boolean; t: Translate; onClose: () => void; @@ -1252,22 +1255,22 @@ interface ActionsSheetProps { } function ActionsSheet({ - target, sharingEnabled, t, onClose, onPreview, onOpenWith, onDownload, onShare, onMove, onDuplicate, onRename, onDelete, + target, access, sharingEnabled, t, onClose, onPreview, onOpenWith, onDownload, onShare, onMove, onDuplicate, onRename, onDelete, }: ActionsSheetProps) { // Centred on a wide window, clear of side insets in landscape. const sheetInsets = useSheetInsets(); const formatModified = useFormatModified(); const c = useColors(); const styles = React.useMemo(() => makeSheetStyles(c), [c]); - if (!target) return null; + if (!target || !access) return null; const isDir = isFolder(target); // A symlink (Stalwart 1.0) has no content: it can be renamed, moved and // deleted, but not previewed, opened, downloaded, duplicated or shared. const link = isSymlink(target); const isFile = !isDir && !link; // Owned nodes report full rights (or no myRights at all); shared-with-me - // nodes are managed by their owner, and write ops would have to route to - // the owner's account — so they only get preview/download here. + // nodes are shared on by their owner, and get the writes their rights + // allow (`access`). const canShare = sharingEnabled && !link && !target.isShared && (target.myRights?.mayShare ?? true); const modified = modifiedOf(target); return ( @@ -1312,25 +1315,25 @@ function ActionsSheet({ {t('files.sharing_access', 'Sharing & access')} ) : null} - {!target.isShared ? ( + {access.move ? ( onMove(target)}> {t('files.move_to', 'Move to…')} ) : null} - {!target.isShared && isFile ? ( + {access.duplicate && isFile ? ( onDuplicate(target)}> {t('files.duplicate', 'Duplicate')} ) : null} - {!target.isShared ? ( + {access.rename ? ( onRename(target)}> {t('files.rename', 'Rename')} ) : null} - {!target.isShared ? ( + {access.delete ? ( onDelete(target)}> {t('files.delete', 'Delete')}