From 3ef3936e67229c106f0d4f426cc874dc53f98bfc Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Tue, 1 Sep 2026 19:06:58 +0200 Subject: [PATCH 01/23] feat(files): list folders through graph behind a toggle --- .../src/graph/driveItems/driveItems.ts | 11 ++ .../web-client/src/graph/driveItems/types.ts | 10 ++ .../web-client/src/helpers/resource/graph.ts | 146 ++++++++++++++++++ .../web-client/src/helpers/resource/index.ts | 1 + .../tests/unit/helpers/resource/graph.spec.ts | 110 +++++++++++++ .../services/folder/loaders/loaderSpace.ts | 59 ++++++- 6 files changed, 336 insertions(+), 1 deletion(-) create mode 100644 packages/web-client/src/helpers/resource/graph.ts create mode 100644 packages/web-client/tests/unit/helpers/resource/graph.spec.ts diff --git a/packages/web-client/src/graph/driveItems/driveItems.ts b/packages/web-client/src/graph/driveItems/driveItems.ts index 45a4943d7a2..06483a0ec23 100644 --- a/packages/web-client/src/graph/driveItems/driveItems.ts +++ b/packages/web-client/src/graph/driveItems/driveItems.ts @@ -63,6 +63,17 @@ export const DriveItemsFactory = ({ async listSharedWithMe(options, requestOptions) { const { data } = await meDriveApiFactory.listSharedWithMe(options?.expand, requestOptions) return data?.value || [] + }, + + // listDriveItemChildren lists a folder's children. Hand-rolled because the + // generated client only covers the personal drive root. + async listDriveItemChildren(driveId, itemId, options, requestOptions) { + const select = options?.select?.length ? `?$select=${options.select.join(',')}` : '' + const { data } = await axiosClient.get( + `${config.basePath}/v1.0/drives/${driveId}/items/${itemId}/children${select}`, + requestOptions + ) + return data?.value || [] } } } diff --git a/packages/web-client/src/graph/driveItems/types.ts b/packages/web-client/src/graph/driveItems/types.ts index a4998c3d68c..55f056bc32b 100644 --- a/packages/web-client/src/graph/driveItems/types.ts +++ b/packages/web-client/src/graph/driveItems/types.ts @@ -1,7 +1,17 @@ import { DriveItem } from '../generated' import type { GraphRequestOptions } from '../types' +export interface ListDriveItemChildrenOptions { + select?: string[] +} + export interface GraphDriveItems { + listDriveItemChildren: ( + driveId: string, + itemId: string, + options?: ListDriveItemChildrenOptions, + requestOptions?: GraphRequestOptions + ) => Promise getDriveItem: ( driveId: string, itemId: string, diff --git a/packages/web-client/src/helpers/resource/graph.ts b/packages/web-client/src/helpers/resource/graph.ts new file mode 100644 index 00000000000..eaee8fde932 --- /dev/null +++ b/packages/web-client/src/helpers/resource/graph.ts @@ -0,0 +1,146 @@ +import { basename, extname } from 'path' +import { urlJoin } from '../../utils' +import { DavPermission } from '../../webdav/constants' +import type { DriveItem } from '../../graph/generated' +import type { SpaceResource } from '../space' +import type { Resource } from './types' + +// graphActionToDavPermission maps the actions a driveItem reports to the DAV +// permission letters the resource helpers built their can* checks on, so +// listings can move to graph without touching every consumer. +const graphActionToDavPermission: Record = { + 'libre.graph/driveItem/permissions/create': DavPermission.Shareable, + 'libre.graph/driveItem/standard/delete': DavPermission.Deletable, + 'libre.graph/driveItem/path/update': DavPermission.Renameable + DavPermission.Moveable, + 'libre.graph/driveItem/children/create': DavPermission.FolderCreateable, + 'libre.graph/driveItem/upload/create': DavPermission.FileUpdateable, + 'libre.graph/driveItem/permissions/deny': DavPermission.Deny +} + +export const davPermissionsFromActions = (actions: string[] = []): string => { + const letters = actions.reduce((acc, action) => { + const mapped = graphActionToDavPermission[action] + if (!mapped) { + return acc + } + for (const letter of mapped) { + if (!acc.includes(letter)) { + acc.push(letter) + } + } + return acc + }, [] as string[]) + + // no read on the content means the item can be viewed but not downloaded + if (!actions.includes('libre.graph/driveItem/content/read')) { + letters.push(DavPermission.SecureView) + } + + return letters.join('') +} + +// buildResourceFromDriveItem turns a graph driveItem into the Resource shape the +// UI works with. The counterpart of buildResource, which reads a PROPFIND entry. +export const buildResourceFromDriveItem = ( + driveItem: DriveItem, + space: SpaceResource, + parentPath = '' +): Resource => { + const isFolder = !!driveItem.folder + const name = driveItem.name || '' + const path = urlJoin(parentPath, name, { leadingSlash: true }) + const actions = (driveItem as any)['@libre.graph.permissions.actions.allowedValues'] as string[] + const shareTypes = ((driveItem as any)['@libre.graph.shareTypes'] || []) as string[] + const lock = (driveItem as any).lockInfo + const permissions = davPermissionsFromActions(actions) + + const r: any = { + id: driveItem.id, + fileId: driveItem.id, + storageId: space.id, + parentFolderId: driveItem.parentReference?.id, + mimeType: driveItem.file?.mimeType, + name, + extension: isFolder ? '' : extname(name).replace(/^\./, ''), + path, + webDavPath: urlJoin(space.webDavPath, path), + type: isFolder ? 'folder' : 'file', + isFolder, + locked: !!lock, + lockOwner: lock?.owners?.[0]?.displayName, + lockTime: lock?.createdDateTime, + processing: !!(driveItem as any).pendingOperations?.pendingContentUpdate, + mdate: driveItem.lastModifiedDateTime, + size: (driveItem.size ?? 0).toString(), + permissions, + isInVault: false, + starred: (driveItem as any)['@libre.graph.me.following'] === true, + etag: driveItem.eTag, + shareTypes, + privateLink: driveItem.webUrl, + remoteItemId: (driveItem as any).remoteItem?.id, + remoteItemPath: (driveItem as any).remoteItem?.path, + // the item owner is always the space owner, see node.Owner() in reva + owner: (space as any).owner?.user || (space as any).owner, + tags: ((driveItem as any)['@libre.graph.tags'] || []) as string[], + audio: driveItem.audio, + location: driveItem.location, + image: driveItem.image, + photo: driveItem.photo, + video: (driveItem as any).video, + motionPhoto: (driveItem as any)['@libre.graph.motionPhoto'], + livePhoto: (driveItem as any)['@libre.graph.livePhoto'], + extraProps: {}, + hasPreview: () => !!driveItem.thumbnails?.length || !isFolder, + canUpload: function () { + return this.permissions.indexOf(DavPermission.FolderCreateable) >= 0 + }, + canDownload: function () { + return this.permissions.indexOf(DavPermission.SecureView) === -1 + }, + canBeDeleted: function () { + return this.permissions.indexOf(DavPermission.Deletable) >= 0 + }, + canRename: function () { + return this.permissions.indexOf(DavPermission.Renameable) >= 0 + }, + canShare: function ({ ability }: { ability: any }) { + return ( + ability.can('create-all', 'Share') && this.permissions.indexOf(DavPermission.Shareable) >= 0 + ) + }, + canCreate: function () { + return this.permissions.indexOf(DavPermission.FolderCreateable) >= 0 + }, + canEditTags: function () { + return ( + this.permissions.indexOf(DavPermission.Updateable) >= 0 || + this.permissions.indexOf(DavPermission.FileUpdateable) >= 0 || + this.permissions.indexOf(DavPermission.FolderCreateable) >= 0 + ) + }, + canListVersions: function () { + return !this.isFolder + }, + isMounted: function () { + return this.permissions.indexOf(DavPermission.Mounted) >= 0 + }, + isReceivedShare: function () { + return this.permissions.indexOf(DavPermission.Shared) >= 0 + }, + isShareRoot(): boolean { + return !!(driveItem as any).remoteItem + }, + getDomSelector: () => (driveItem.id || '').replace(/[^A-Za-z0-9\-_]/g, '') + } + + return r as Resource +} + +export const buildResourcesFromDriveItems = ( + driveItems: DriveItem[], + space: SpaceResource, + parentPath = '' +): Resource[] => driveItems.map((item) => buildResourceFromDriveItem(item, space, parentPath)) + +export { basename } diff --git a/packages/web-client/src/helpers/resource/index.ts b/packages/web-client/src/helpers/resource/index.ts index ab6b35419db..32817eafe0a 100644 --- a/packages/web-client/src/helpers/resource/index.ts +++ b/packages/web-client/src/helpers/resource/index.ts @@ -1,2 +1,3 @@ export * from './functions' export * from './types' +export * from './graph' diff --git a/packages/web-client/tests/unit/helpers/resource/graph.spec.ts b/packages/web-client/tests/unit/helpers/resource/graph.spec.ts new file mode 100644 index 00000000000..cd95fd55578 --- /dev/null +++ b/packages/web-client/tests/unit/helpers/resource/graph.spec.ts @@ -0,0 +1,110 @@ +import { davPermissionsFromActions, buildResourceFromDriveItem } from '../../../../src/helpers/resource/graph' +import type { SpaceResource } from '../../../../src/helpers/space' + +// a manager's action list, taken verbatim from a running server +const managerActions = [ + 'libre.graph/driveItem/permissions/create', + 'libre.graph/driveItem/children/create', + 'libre.graph/driveItem/standard/delete', + 'libre.graph/driveItem/path/read', + 'libre.graph/driveItem/quota/read', + 'libre.graph/driveItem/content/read', + 'libre.graph/driveItem/upload/create', + 'libre.graph/driveItem/permissions/read', + 'libre.graph/driveItem/children/read', + 'libre.graph/driveItem/versions/read', + 'libre.graph/driveItem/deleted/read', + 'libre.graph/driveItem/path/update', + 'libre.graph/driveItem/permissions/delete' +] + +const space = { + id: 'storage$space', + webDavPath: '/dav/spaces/storage$space', + owner: { user: { id: 'alice', displayName: 'Alice' } } +} as unknown as SpaceResource + +describe('davPermissionsFromActions', () => { + it('maps a manager to the full dav permission set', () => { + const permissions = davPermissionsFromActions(managerActions) + expect(permissions).toContain('R') // shareable + expect(permissions).toContain('D') // deletable + expect(permissions).toContain('N') // renameable + expect(permissions).toContain('CK') // folder createable + expect(permissions).not.toContain('X') // content is readable, so no secure view + }) + + it('marks an item without content read as secure view', () => { + const permissions = davPermissionsFromActions(['libre.graph/driveItem/children/read']) + expect(permissions).toContain('X') + }) + + it('handles an empty action list', () => { + expect(davPermissionsFromActions([])).toBe('X') + expect(davPermissionsFromActions(undefined)).toBe('X') + }) +}) + +describe('buildResourceFromDriveItem', () => { + it('builds a folder resource with working capability checks', () => { + const r = buildResourceFromDriveItem( + { + id: 'storage$space!folder', + name: 'music', + size: 0, + eTag: '"abc"', + folder: {}, + lastModifiedDateTime: '2026-09-01T12:00:00Z', + parentReference: { id: 'storage$space!root' }, + '@libre.graph.permissions.actions.allowedValues': managerActions + } as any, + space + ) + + expect(r.isFolder).toBe(true) + expect(r.name).toBe('music') + expect(r.path).toBe('/music') + expect(r.canUpload()).toBe(true) + expect(r.canBeDeleted()).toBe(true) + expect(r.canRename()).toBe(true) + expect(r.canDownload()).toBe(true) + expect(r.owner).toEqual({ id: 'alice', displayName: 'Alice' }) + }) + + it('carries the facets and the lock through', () => { + const r = buildResourceFromDriveItem( + { + id: 'storage$space!song', + name: 'fight.mp3', + size: 42, + file: { mimeType: 'audio/mpeg' }, + audio: { artist: 'Motörhead', title: 'Fight' }, + lockInfo: { lockType: 'exclusive', owners: [{ displayName: 'Alice' }] }, + pendingOperations: { pendingContentUpdate: {} }, + '@libre.graph.shareTypes': ['user', 'link'], + '@libre.graph.permissions.actions.allowedValues': managerActions + } as any, + space, + '/music' + ) + + expect(r.isFolder).toBe(false) + expect(r.extension).toBe('mp3') + expect(r.path).toBe('/music/fight.mp3') + expect(r.mimeType).toBe('audio/mpeg') + expect((r as any).audio.artist).toBe('Motörhead') + expect(r.locked).toBe(true) + expect(r.lockOwner).toBe('Alice') + expect(r.processing).toBe(true) + expect(r.shareTypes).toEqual(['user', 'link']) + }) + + it('reports a shared item as a share root', () => { + const r = buildResourceFromDriveItem( + { id: 'x', name: 'shared.txt', remoteItem: { id: 'other$drive!item', path: '/Project X' } } as any, + space + ) + expect(r.isShareRoot()).toBe(true) + expect(r.remoteItemPath).toBe('/Project X') + }) +}) diff --git a/packages/web-pkg/src/services/folder/loaders/loaderSpace.ts b/packages/web-pkg/src/services/folder/loaders/loaderSpace.ts index 1c14bfc5b21..5e22b57c194 100644 --- a/packages/web-pkg/src/services/folder/loaders/loaderSpace.ts +++ b/packages/web-pkg/src/services/folder/loaders/loaderSpace.ts @@ -13,6 +13,7 @@ import { import { unref } from 'vue' import { FolderLoaderOptions } from './types' import { DriveItem } from '@opencloud-eu/web-client/graph/generated' +import { buildResourcesFromDriveItems } from '@opencloud-eu/web-client' import { isLocationSpacesActive, isLocationPublicActive } from '../../../router' import { getSharedDriveItem, setCurrentUserShareSpacePermissions } from '../../../helpers' import { useFileRouteReplace } from '../../../composables' @@ -64,9 +65,23 @@ export class FolderLoaderSpace implements FolderLoader { davProperties.push(DavProperty.DownloadURL) } + // Graph listing is opt-in while it is being validated against PROPFIND, + // toggle with localStorage.setItem('oc_graph_listing', '1'). + const useGraphListing = + !isPublicSpaceResource(space) && + (() => { + try { + return window.localStorage.getItem('oc_graph_listing') === '1' + } catch { + return false + } + })() + // eslint-disable-next-line prefer-const let { resource: currentFolder, children: resources } = yield* call( - webdav.listFiles(space, { path, fileId }, { signal: signal1, davProperties }) + useGraphListing + ? listFilesViaGraph({ graphClient, webdav, space, path, fileId, signal: signal1 }) + : webdav.listFiles(space, { path, fileId }, { signal: signal1, davProperties }) ) // if current folder has no id (= singe file public link) we must not correct the route @@ -137,3 +152,45 @@ export class FolderLoaderSpace implements FolderLoader { }).restartable() } } + +// listFilesViaGraph lists a folder through the graph children endpoint. The +// current folder still comes from webdav: graph has no "stat this item" in the +// listing call, and the loader needs it for the route correction. +const listFilesViaGraph = async ({ + graphClient, + webdav, + space, + path, + fileId, + signal +}: { + graphClient: any + webdav: any + space: SpaceResource + path: string + fileId: string + signal: AbortSignal +}) => { + const { resource: currentFolder } = await webdav.listFiles( + space, + { path, fileId }, + { depth: 0, signal } + ) + + const driveItems = await graphClient.driveItems.listDriveItemChildren( + space.id.toString(), + currentFolder.id.toString(), + { + select: [ + '@libre.graph.permissions.actions.allowedValues', + '@libre.graph.shareTypes' + ] + }, + { signal } + ) + + return { + resource: currentFolder, + children: buildResourcesFromDriveItems(driveItems, space, currentFolder.path) + } +} From 2a6bf5b39b78029cc342135bd4431058c16c42a7 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Tue, 1 Sep 2026 20:16:48 +0200 Subject: [PATCH 02/23] feat: list folders through graph in a single request --- .../src/graph/driveItems/driveItems.ts | 28 +++++++++-- .../web-client/src/graph/driveItems/types.ts | 11 +++- .../web-client/src/helpers/resource/graph.ts | 8 +-- .../tests/unit/helpers/resource/graph.spec.ts | 2 +- .../services/folder/loaders/loaderSpace.ts | 50 ++++++++++--------- 5 files changed, 67 insertions(+), 32 deletions(-) diff --git a/packages/web-client/src/graph/driveItems/driveItems.ts b/packages/web-client/src/graph/driveItems/driveItems.ts index 06483a0ec23..bb3278f00fc 100644 --- a/packages/web-client/src/graph/driveItems/driveItems.ts +++ b/packages/web-client/src/graph/driveItems/driveItems.ts @@ -1,6 +1,15 @@ import { DriveItemApiFactory, DrivesRootApiFactory, MeDriveApiFactory } from './../generated' +import { urlJoin } from '../../utils' import type { GraphFactoryOptions } from './../types' -import type { GraphDriveItems } from './types' +import type { DriveItemQueryOptions, GraphDriveItems } from './types' + +const odataQuery = ({ select, expand }: DriveItemQueryOptions = {}) => { + const params = [ + ...(select?.length ? [`$select=${select.join(',')}`] : []), + ...(expand?.length ? [`$expand=${expand.join(',')}`] : []) + ] + return params.length ? `?${params.join('&')}` : '' +} export const DriveItemsFactory = ({ axiosClient, @@ -65,12 +74,25 @@ export const DriveItemsFactory = ({ return data?.value || [] }, + // statDriveItem stats an item by id or by graph's colon path syntax. + // Hand-rolled for the same reason as listDriveItemChildren: the generated + // client has no $select, no $expand and no path lookup. + async statDriveItem(driveId, ref, options, requestOptions) { + const suffix = ref.itemId + ? `/items/${ref.itemId}` + : `/root:${urlJoin(ref.path, { leadingSlash: true })}` + const { data } = await axiosClient.get( + `${config.basePath}/v1.0/drives/${driveId}${suffix}${odataQuery(options)}`, + requestOptions + ) + return data + }, + // listDriveItemChildren lists a folder's children. Hand-rolled because the // generated client only covers the personal drive root. async listDriveItemChildren(driveId, itemId, options, requestOptions) { - const select = options?.select?.length ? `?$select=${options.select.join(',')}` : '' const { data } = await axiosClient.get( - `${config.basePath}/v1.0/drives/${driveId}/items/${itemId}/children${select}`, + `${config.basePath}/v1.0/drives/${driveId}/items/${itemId}/children${odataQuery(options)}`, requestOptions ) return data?.value || [] diff --git a/packages/web-client/src/graph/driveItems/types.ts b/packages/web-client/src/graph/driveItems/types.ts index 55f056bc32b..6c1dd55e006 100644 --- a/packages/web-client/src/graph/driveItems/types.ts +++ b/packages/web-client/src/graph/driveItems/types.ts @@ -1,15 +1,16 @@ import { DriveItem } from '../generated' import type { GraphRequestOptions } from '../types' -export interface ListDriveItemChildrenOptions { +export interface DriveItemQueryOptions { select?: string[] + expand?: string[] } export interface GraphDriveItems { listDriveItemChildren: ( driveId: string, itemId: string, - options?: ListDriveItemChildrenOptions, + options?: DriveItemQueryOptions, requestOptions?: GraphRequestOptions ) => Promise getDriveItem: ( @@ -17,6 +18,12 @@ export interface GraphDriveItems { itemId: string, requestOptions?: GraphRequestOptions ) => Promise + statDriveItem: ( + driveId: string, + ref: { itemId?: string; path?: string }, + options?: DriveItemQueryOptions, + requestOptions?: GraphRequestOptions + ) => Promise createDriveItem: ( driveId: string, data: DriveItem, diff --git a/packages/web-client/src/helpers/resource/graph.ts b/packages/web-client/src/helpers/resource/graph.ts index eaee8fde932..11c40902e3f 100644 --- a/packages/web-client/src/helpers/resource/graph.ts +++ b/packages/web-client/src/helpers/resource/graph.ts @@ -44,11 +44,13 @@ export const davPermissionsFromActions = (actions: string[] = []): string => { export const buildResourceFromDriveItem = ( driveItem: DriveItem, space: SpaceResource, - parentPath = '' + parentPath = '', + // the drive root reports its own name, so callers that know the path pin it + pathOverride?: string ): Resource => { const isFolder = !!driveItem.folder const name = driveItem.name || '' - const path = urlJoin(parentPath, name, { leadingSlash: true }) + const path = pathOverride ?? urlJoin(parentPath, name, { leadingSlash: true }) const actions = (driveItem as any)['@libre.graph.permissions.actions.allowedValues'] as string[] const shareTypes = ((driveItem as any)['@libre.graph.shareTypes'] || []) as string[] const lock = (driveItem as any).lockInfo @@ -92,7 +94,7 @@ export const buildResourceFromDriveItem = ( livePhoto: (driveItem as any)['@libre.graph.livePhoto'], extraProps: {}, hasPreview: () => !!driveItem.thumbnails?.length || !isFolder, - canUpload: function () { + canUpload: function (this: Resource) { return this.permissions.indexOf(DavPermission.FolderCreateable) >= 0 }, canDownload: function () { diff --git a/packages/web-client/tests/unit/helpers/resource/graph.spec.ts b/packages/web-client/tests/unit/helpers/resource/graph.spec.ts index cd95fd55578..18efa343e93 100644 --- a/packages/web-client/tests/unit/helpers/resource/graph.spec.ts +++ b/packages/web-client/tests/unit/helpers/resource/graph.spec.ts @@ -64,7 +64,7 @@ describe('buildResourceFromDriveItem', () => { expect(r.isFolder).toBe(true) expect(r.name).toBe('music') expect(r.path).toBe('/music') - expect(r.canUpload()).toBe(true) + expect(r.canUpload({})).toBe(true) expect(r.canBeDeleted()).toBe(true) expect(r.canRename()).toBe(true) expect(r.canDownload()).toBe(true) diff --git a/packages/web-pkg/src/services/folder/loaders/loaderSpace.ts b/packages/web-pkg/src/services/folder/loaders/loaderSpace.ts index 5e22b57c194..03778077839 100644 --- a/packages/web-pkg/src/services/folder/loaders/loaderSpace.ts +++ b/packages/web-pkg/src/services/folder/loaders/loaderSpace.ts @@ -11,9 +11,10 @@ import { SpaceResource } from '@opencloud-eu/web-client' import { unref } from 'vue' +import { urlJoin } from '@opencloud-eu/web-client' import { FolderLoaderOptions } from './types' import { DriveItem } from '@opencloud-eu/web-client/graph/generated' -import { buildResourcesFromDriveItems } from '@opencloud-eu/web-client' +import { buildResourceFromDriveItem, buildResourcesFromDriveItems } from '@opencloud-eu/web-client' import { isLocationSpacesActive, isLocationPublicActive } from '../../../router' import { getSharedDriveItem, setCurrentUserShareSpacePermissions } from '../../../helpers' import { useFileRouteReplace } from '../../../composables' @@ -80,7 +81,7 @@ export class FolderLoaderSpace implements FolderLoader { // eslint-disable-next-line prefer-const let { resource: currentFolder, children: resources } = yield* call( useGraphListing - ? listFilesViaGraph({ graphClient, webdav, space, path, fileId, signal: signal1 }) + ? listFilesViaGraph({ graphClient, space, path, fileId, signal: signal1 }) : webdav.listFiles(space, { path, fileId }, { signal: signal1, davProperties }) ) @@ -153,44 +154,47 @@ export class FolderLoaderSpace implements FolderLoader { } } -// listFilesViaGraph lists a folder through the graph children endpoint. The -// current folder still comes from webdav: graph has no "stat this item" in the -// listing call, and the loader needs it for the route correction. +const graphListingSelect = [ + '@libre.graph.permissions.actions.allowedValues', + '@libre.graph.shareTypes' +] + +// listFilesViaGraph lists a folder through graph, folder and children in one +// request via $expand=children, the same shape PROPFIND with Depth: 1 returns. const listFilesViaGraph = async ({ graphClient, - webdav, space, path, fileId, signal }: { graphClient: any - webdav: any space: SpaceResource path: string fileId: string signal: AbortSignal }) => { - const { resource: currentFolder } = await webdav.listFiles( - space, - { path, fileId }, - { depth: 0, signal } - ) - - const driveItems = await graphClient.driveItems.listDriveItemChildren( - space.id.toString(), - currentFolder.id.toString(), - { - select: [ - '@libre.graph.permissions.actions.allowedValues', - '@libre.graph.shareTypes' - ] - }, + const driveId = space.id.toString() + // graph has no path lookup for the drive root, it is addressed by its id + const isRoot = !path || path === '/' + const driveItem = await graphClient.driveItems.statDriveItem( + driveId, + fileId || isRoot ? { itemId: fileId || space.root?.id } : { path }, + { select: graphListingSelect, expand: ['children'] }, { signal } ) + // the item is authoritative, not the url: the route correction below exists + // to fix a stale path. the drive root reports itself as '.' + const parentPath = driveItem.parentReference?.path + const currentPath = + !parentPath || parentPath === '.' + ? '/' + : urlJoin(parentPath, driveItem.name, { leadingSlash: true }) + const currentFolder = buildResourceFromDriveItem(driveItem, space, '', currentPath) + return { resource: currentFolder, - children: buildResourcesFromDriveItems(driveItems, space, currentFolder.path) + children: buildResourcesFromDriveItems(driveItem.children || [], space, currentFolder.path) } } From dc1b0813c3d8b5a555b36dd90044682f59ae3ba8 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Tue, 1 Sep 2026 20:35:39 +0200 Subject: [PATCH 03/23] refactor: drop the graph listing toggle --- .../services/folder/loaders/loaderSpace.ts | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/packages/web-pkg/src/services/folder/loaders/loaderSpace.ts b/packages/web-pkg/src/services/folder/loaders/loaderSpace.ts index 03778077839..0a14f08e35f 100644 --- a/packages/web-pkg/src/services/folder/loaders/loaderSpace.ts +++ b/packages/web-pkg/src/services/folder/loaders/loaderSpace.ts @@ -66,23 +66,12 @@ export class FolderLoaderSpace implements FolderLoader { davProperties.push(DavProperty.DownloadURL) } - // Graph listing is opt-in while it is being validated against PROPFIND, - // toggle with localStorage.setItem('oc_graph_listing', '1'). - const useGraphListing = - !isPublicSpaceResource(space) && - (() => { - try { - return window.localStorage.getItem('oc_graph_listing') === '1' - } catch { - return false - } - })() - // eslint-disable-next-line prefer-const let { resource: currentFolder, children: resources } = yield* call( - useGraphListing - ? listFilesViaGraph({ graphClient, space, path, fileId, signal: signal1 }) - : webdav.listFiles(space, { path, fileId }, { signal: signal1, davProperties }) + // public links have no drive, they are only reachable over webdav + isPublicSpaceResource(space) + ? webdav.listFiles(space, { path, fileId }, { signal: signal1, davProperties }) + : listFilesViaGraph({ graphClient, space, path, fileId, signal: signal1 }) ) // if current folder has no id (= singe file public link) we must not correct the route From 8cc1e9d5e8780d1ff5e3de1fe131cd909da20a15 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 00:22:44 +0200 Subject: [PATCH 04/23] chore(web-client): regenerate libre-graph client Picks up the driveItem stat and children operations with $select and $expand, plus lockInfo, pendingOperations, tags, following, allowedValues and shareTypes on the driveItem type. Generated from opencloud-eu/libre-graph-api#70 and #72, neither merged yet. --- .../src/graph/driveItems/driveItems.ts | 1 + .../graph/generated/.openapi-generator/FILES | 1 + .../web-client/src/graph/generated/api.ts | 166 +++++++++++++++--- .../web-client/src/helpers/resource/graph.ts | 1 - 4 files changed, 140 insertions(+), 29 deletions(-) diff --git a/packages/web-client/src/graph/driveItems/driveItems.ts b/packages/web-client/src/graph/driveItems/driveItems.ts index bb3278f00fc..9a1fe54fedb 100644 --- a/packages/web-client/src/graph/driveItems/driveItems.ts +++ b/packages/web-client/src/graph/driveItems/driveItems.ts @@ -25,6 +25,7 @@ export const DriveItemsFactory = ({ driveId, itemId, undefined, + undefined, requestOptions ) return data diff --git a/packages/web-client/src/graph/generated/.openapi-generator/FILES b/packages/web-client/src/graph/generated/.openapi-generator/FILES index 3f3e8315d79..5fd7c6cedff 100644 --- a/packages/web-client/src/graph/generated/.openapi-generator/FILES +++ b/packages/web-client/src/graph/generated/.openapi-generator/FILES @@ -75,6 +75,7 @@ docs/InvitationsApi.md docs/InvitedUserMessageInfo.md docs/ItemReference.md docs/LivePhoto.md +docs/LockInfo.md docs/MeChangepasswordApi.md docs/MeDriveApi.md docs/MeDriveRootApi.md diff --git a/packages/web-client/src/graph/generated/api.ts b/packages/web-client/src/graph/generated/api.ts index 098db666956..96bb7aab827 100644 --- a/packages/web-client/src/graph/generated/api.ts +++ b/packages/web-client/src/graph/generated/api.ts @@ -447,6 +447,7 @@ export interface DriveItem { 'video'?: Video; '@libre.graph.motionPhoto'?: MotionPhoto; '@libre.graph.livePhoto'?: LivePhoto; + 'lockInfo'?: LockInfo; /** * Indicates if the item is synchronized with the underlying storage provider. Read-only. */ @@ -471,7 +472,21 @@ export interface DriveItem { * A list of actions the caller is allowed to perform on this item. Only returned when explicitly requested via `$select` on endpoints that support it. Mirrors the annotation of the same name on the `/permissions` endpoint, allowing clients to learn a caller\'s effective actions on an item without a separate round-trip. */ '@libre.graph.permissions.actions.allowedValues'?: Array; + /** + * The types of shares existing on this item, aggregated over all of its grants. Absent or empty if the item is not shared. This is a summary of the item\'s `permissions` collection. For the full grants use the permissions endpoints, for the caller\'s own capabilities use `@libre.graph.permissions.actions.allowedValues`. Only returned when explicitly requested via `$select`. + */ + '@libre.graph.shareTypes'?: Array; } + +export const DriveItemAtLibreGraphShareTypesEnum = { + User: 'user', + Group: 'group', + Link: 'link', + Remote: 'remote', +} as const; + +export type DriveItemAtLibreGraphShareTypesEnum = typeof DriveItemAtLibreGraphShareTypesEnum[keyof typeof DriveItemAtLibreGraphShareTypesEnum]; + export interface DriveItemCreateLink { 'type'?: SharingLinkType; /** @@ -515,6 +530,10 @@ export interface DriveItemInvite { * Represents a person, group, or other recipient to share a drive item with using the invite action. When using invite to add permissions, the `driveRecipient` object would specify the `email`, `alias`, or `objectId` of the recipient. Only one of these values is required; multiple values are not accepted. */ export interface DriveRecipient { + /** + * The email address for the recipient, if the recipient has an associated email address. + */ + 'email'?: string; /** * The unique identifier for the recipient in the directory. */ @@ -977,6 +996,39 @@ export interface LivePhoto { */ 'vitalityScoringVersion'?: number; } +/** + * Read-only lock metadata for a file, matching the MS Graph beta lockInfo resource. Indicates whether the file is locked, the kind of lock, when it was created, when it expires and who holds it. + */ +export interface LockInfo { + /** + * The type of lock currently held on the file. OpenCloud currently only issues exclusive locks, same as MS Graph, even if it defines more. Read-only. + */ + 'lockType'?: LockInfoLockTypeEnum; + /** + * The date and time when the lock was created, in UTC. Read-only. + */ + 'createdDateTime'?: string; + /** + * The date and time when the lock expires, in UTC. Read-only. + */ + 'expirationDateTime'?: string; + /** + * The collection of users that currently hold the lock on the file. Read-only. + */ + 'owners'?: Array; + /** + * Name of the application holding the lock, for example an office application. Not part of MS Graph. Read-only. + */ + '@libre.graph.appName'?: string; +} + +export const LockInfoLockTypeEnum = { + None: 'none', + Exclusive: 'exclusive', +} as const; + +export type LockInfoLockTypeEnum = typeof LockInfoLockTypeEnum[keyof typeof LockInfoLockTypeEnum]; + export interface MemberReference { '@odata.id'?: string; } @@ -1978,10 +2030,11 @@ export const DriveItemApiAxiosParamCreator = function (configuration?: Configura * @param {string} driveId key: id of drive * @param {string} itemId key: id of item * @param {Set} [$select] Select additional properties to be returned. + * @param {Set} [$expand] Expand related entities to be returned. * @param {*} [options] Override http request option. * @throws {RequiredError} */ - getDriveItem: async (driveId: string, itemId: string, $select?: Set, options: RawAxiosRequestConfig = {}): Promise => { + getDriveItem: async (driveId: string, itemId: string, $select?: Set, $expand?: Set, options: RawAxiosRequestConfig = {}): Promise => { // verify required parameter 'driveId' is not null or undefined assertParamExists('getDriveItem', 'driveId', driveId) // verify required parameter 'itemId' is not null or undefined @@ -2010,6 +2063,10 @@ export const DriveItemApiAxiosParamCreator = function (configuration?: Configura localVarQueryParameter['$select'] = Array.from($select).join(COLLECTION_FORMATS.csv); } + if ($expand) { + localVarQueryParameter['$expand'] = Array.from($expand).join(COLLECTION_FORMATS.csv); + } + localVarHeaderParameter['Accept'] = 'application/json'; setSearchParams(localVarUrlObj, localVarQueryParameter); @@ -2120,10 +2177,11 @@ export const DriveItemApiAxiosParamCreator = function (configuration?: Configura * @param {string} driveId key: id of drive * @param {string} itemId key: id of item * @param {Set} [$select] Select additional properties to be returned. + * @param {Set} [$expand] Expand related entities to be returned. * @param {*} [options] Override http request option. * @throws {RequiredError} */ - getDriveItemV1: async (driveId: string, itemId: string, $select?: Set, options: RawAxiosRequestConfig = {}): Promise => { + getDriveItemV1: async (driveId: string, itemId: string, $select?: Set, $expand?: Set, options: RawAxiosRequestConfig = {}): Promise => { // verify required parameter 'driveId' is not null or undefined assertParamExists('getDriveItemV1', 'driveId', driveId) // verify required parameter 'itemId' is not null or undefined @@ -2152,6 +2210,10 @@ export const DriveItemApiAxiosParamCreator = function (configuration?: Configura localVarQueryParameter['$select'] = Array.from($select).join(COLLECTION_FORMATS.csv); } + if ($expand) { + localVarQueryParameter['$expand'] = Array.from($expand).join(COLLECTION_FORMATS.csv); + } + localVarHeaderParameter['Accept'] = 'application/json'; setSearchParams(localVarUrlObj, localVarQueryParameter); @@ -2258,11 +2320,12 @@ export const DriveItemApiFp = function(configuration?: Configuration) { * @param {string} driveId key: id of drive * @param {string} itemId key: id of item * @param {Set} [$select] Select additional properties to be returned. + * @param {Set} [$expand] Expand related entities to be returned. * @param {*} [options] Override http request option. * @throws {RequiredError} */ - async getDriveItem(driveId: string, itemId: string, $select?: Set, options?: RawAxiosRequestConfig): Promise<(axios?: AxiosInstance, basePath?: string) => AxiosPromise> { - const localVarAxiosArgs = await localVarAxiosParamCreator.getDriveItem(driveId, itemId, $select, options); + async getDriveItem(driveId: string, itemId: string, $select?: Set, $expand?: Set, options?: RawAxiosRequestConfig): Promise<(axios?: AxiosInstance, basePath?: string) => AxiosPromise> { + const localVarAxiosArgs = await localVarAxiosParamCreator.getDriveItem(driveId, itemId, $select, $expand, options); const localVarOperationServerIndex = configuration?.serverIndex ?? 0; const localVarOperationServerBasePath = operationServerMap['DriveItemApi.getDriveItem']?.[localVarOperationServerIndex]?.url; return (axios, basePath) => createRequestFunction(localVarAxiosArgs, globalAxios, BASE_PATH, configuration)(axios, localVarOperationServerBasePath || basePath); @@ -2302,11 +2365,12 @@ export const DriveItemApiFp = function(configuration?: Configuration) { * @param {string} driveId key: id of drive * @param {string} itemId key: id of item * @param {Set} [$select] Select additional properties to be returned. + * @param {Set} [$expand] Expand related entities to be returned. * @param {*} [options] Override http request option. * @throws {RequiredError} */ - async getDriveItemV1(driveId: string, itemId: string, $select?: Set, options?: RawAxiosRequestConfig): Promise<(axios?: AxiosInstance, basePath?: string) => AxiosPromise> { - const localVarAxiosArgs = await localVarAxiosParamCreator.getDriveItemV1(driveId, itemId, $select, options); + async getDriveItemV1(driveId: string, itemId: string, $select?: Set, $expand?: Set, options?: RawAxiosRequestConfig): Promise<(axios?: AxiosInstance, basePath?: string) => AxiosPromise> { + const localVarAxiosArgs = await localVarAxiosParamCreator.getDriveItemV1(driveId, itemId, $select, $expand, options); const localVarOperationServerIndex = configuration?.serverIndex ?? 0; const localVarOperationServerBasePath = operationServerMap['DriveItemApi.getDriveItemV1']?.[localVarOperationServerIndex]?.url; return (axios, basePath) => createRequestFunction(localVarAxiosArgs, globalAxios, BASE_PATH, configuration)(axios, localVarOperationServerBasePath || basePath); @@ -2366,11 +2430,12 @@ export const DriveItemApiFactory = function (configuration?: Configuration, base * @param {string} driveId key: id of drive * @param {string} itemId key: id of item * @param {Set} [$select] Select additional properties to be returned. + * @param {Set} [$expand] Expand related entities to be returned. * @param {*} [options] Override http request option. * @throws {RequiredError} */ - getDriveItem(driveId: string, itemId: string, $select?: Set, options?: RawAxiosRequestConfig): AxiosPromise { - return localVarFp.getDriveItem(driveId, itemId, $select, options).then((request) => request(axios, basePath)); + getDriveItem(driveId: string, itemId: string, $select?: Set, $expand?: Set, options?: RawAxiosRequestConfig): AxiosPromise { + return localVarFp.getDriveItem(driveId, itemId, $select, $expand, options).then((request) => request(axios, basePath)); }, /** * List the children of the item identified by `item-id` in the drive identified by `drive-id`. The item must exist and be a folder. Modeled on the MS Graph list driveItem children endpoint (https://learn.microsoft.com/en-us/graph/api/driveitem-list-children). This endpoint also accepts the MS Graph colon-syntax URL forms: GET /v1.0/drives/{drive-id}/root:/{path}:/children GET /v1.0/drives/{drive-id}/items/{item-id}:/{path}:/children OpenAPI cannot express the colon-delimited path segment, so these URL forms are not represented as separate operations in this specification. The server still accepts them, resolves `:/{path}:` as the parent item, and lists its children. @@ -2401,11 +2466,12 @@ export const DriveItemApiFactory = function (configuration?: Configuration, base * @param {string} driveId key: id of drive * @param {string} itemId key: id of item * @param {Set} [$select] Select additional properties to be returned. + * @param {Set} [$expand] Expand related entities to be returned. * @param {*} [options] Override http request option. * @throws {RequiredError} */ - getDriveItemV1(driveId: string, itemId: string, $select?: Set, options?: RawAxiosRequestConfig): AxiosPromise { - return localVarFp.getDriveItemV1(driveId, itemId, $select, options).then((request) => request(axios, basePath)); + getDriveItemV1(driveId: string, itemId: string, $select?: Set, $expand?: Set, options?: RawAxiosRequestConfig): AxiosPromise { + return localVarFp.getDriveItemV1(driveId, itemId, $select, $expand, options).then((request) => request(axios, basePath)); }, /** * Update a DriveItem. The request body must include a JSON object with the properties to update. Only the properties that are provided will be updated. Currently it supports updating the following properties: * `@UI.Hidden` - Hides the item from the UI. @@ -2459,11 +2525,12 @@ export class DriveItemApi extends BaseAPI { * @param {string} driveId key: id of drive * @param {string} itemId key: id of item * @param {Set} [$select] Select additional properties to be returned. + * @param {Set} [$expand] Expand related entities to be returned. * @param {*} [options] Override http request option. * @throws {RequiredError} */ - public getDriveItem(driveId: string, itemId: string, $select?: Set, options?: RawAxiosRequestConfig) { - return DriveItemApiFp(this.configuration).getDriveItem(driveId, itemId, $select, options).then((request) => request(this.axios, this.basePath)); + public getDriveItem(driveId: string, itemId: string, $select?: Set, $expand?: Set, options?: RawAxiosRequestConfig) { + return DriveItemApiFp(this.configuration).getDriveItem(driveId, itemId, $select, $expand, options).then((request) => request(this.axios, this.basePath)); } /** @@ -2497,11 +2564,12 @@ export class DriveItemApi extends BaseAPI { * @param {string} driveId key: id of drive * @param {string} itemId key: id of item * @param {Set} [$select] Select additional properties to be returned. + * @param {Set} [$expand] Expand related entities to be returned. * @param {*} [options] Override http request option. * @throws {RequiredError} */ - public getDriveItemV1(driveId: string, itemId: string, $select?: Set, options?: RawAxiosRequestConfig) { - return DriveItemApiFp(this.configuration).getDriveItemV1(driveId, itemId, $select, options).then((request) => request(this.axios, this.basePath)); + public getDriveItemV1(driveId: string, itemId: string, $select?: Set, $expand?: Set, options?: RawAxiosRequestConfig) { + return DriveItemApiFp(this.configuration).getDriveItemV1(driveId, itemId, $select, $expand, options).then((request) => request(this.axios, this.basePath)); } /** @@ -2531,18 +2599,31 @@ export type CreateChildDriveItemAtLibreGraphMissingParentsBehaviorEnum = typeof export const GetDriveItemSelectEnum = { MicrosoftGraphDownloadUrl: '@microsoft.graph.downloadUrl', LibreGraphPermissionsActionsAllowedValues: '@libre.graph.permissions.actions.allowedValues', + LibreGraphShareTypes: '@libre.graph.shareTypes', } as const; export type GetDriveItemSelectEnum = typeof GetDriveItemSelectEnum[keyof typeof GetDriveItemSelectEnum]; +export const GetDriveItemExpandEnum = { + Children: 'children', + Thumbnails: 'thumbnails', +} as const; +export type GetDriveItemExpandEnum = typeof GetDriveItemExpandEnum[keyof typeof GetDriveItemExpandEnum]; export const GetDriveItemChildrenSelectEnum = { MicrosoftGraphDownloadUrl: '@microsoft.graph.downloadUrl', LibreGraphPermissionsActionsAllowedValues: '@libre.graph.permissions.actions.allowedValues', + LibreGraphShareTypes: '@libre.graph.shareTypes', } as const; export type GetDriveItemChildrenSelectEnum = typeof GetDriveItemChildrenSelectEnum[keyof typeof GetDriveItemChildrenSelectEnum]; export const GetDriveItemV1SelectEnum = { MicrosoftGraphDownloadUrl: '@microsoft.graph.downloadUrl', LibreGraphPermissionsActionsAllowedValues: '@libre.graph.permissions.actions.allowedValues', + LibreGraphShareTypes: '@libre.graph.shareTypes', } as const; export type GetDriveItemV1SelectEnum = typeof GetDriveItemV1SelectEnum[keyof typeof GetDriveItemV1SelectEnum]; +export const GetDriveItemV1ExpandEnum = { + Children: 'children', + Thumbnails: 'thumbnails', +} as const; +export type GetDriveItemV1ExpandEnum = typeof GetDriveItemV1ExpandEnum[keyof typeof GetDriveItemV1ExpandEnum]; /** @@ -4009,10 +4090,11 @@ export const DrivesRootApiAxiosParamCreator = function (configuration?: Configur * @summary Get root from arbitrary space * @param {string} driveId key: id of drive * @param {Set} [$select] Select additional properties to be returned. + * @param {Set} [$expand] Expand related entities to be returned. * @param {*} [options] Override http request option. * @throws {RequiredError} */ - getRoot: async (driveId: string, $select?: Set, options: RawAxiosRequestConfig = {}): Promise => { + getRoot: async (driveId: string, $select?: Set, $expand?: Set, options: RawAxiosRequestConfig = {}): Promise => { // verify required parameter 'driveId' is not null or undefined assertParamExists('getRoot', 'driveId', driveId) const localVarPath = `/v1.0/drives/{drive-id}/root` @@ -4038,6 +4120,10 @@ export const DrivesRootApiAxiosParamCreator = function (configuration?: Configur localVarQueryParameter['$select'] = Array.from($select).join(COLLECTION_FORMATS.csv); } + if ($expand) { + localVarQueryParameter['$expand'] = Array.from($expand).join(COLLECTION_FORMATS.csv); + } + localVarHeaderParameter['Accept'] = 'application/json'; setSearchParams(localVarUrlObj, localVarQueryParameter); @@ -4322,11 +4408,12 @@ export const DrivesRootApiFp = function(configuration?: Configuration) { * @summary Get root from arbitrary space * @param {string} driveId key: id of drive * @param {Set} [$select] Select additional properties to be returned. + * @param {Set} [$expand] Expand related entities to be returned. * @param {*} [options] Override http request option. * @throws {RequiredError} */ - async getRoot(driveId: string, $select?: Set, options?: RawAxiosRequestConfig): Promise<(axios?: AxiosInstance, basePath?: string) => AxiosPromise> { - const localVarAxiosArgs = await localVarAxiosParamCreator.getRoot(driveId, $select, options); + async getRoot(driveId: string, $select?: Set, $expand?: Set, options?: RawAxiosRequestConfig): Promise<(axios?: AxiosInstance, basePath?: string) => AxiosPromise> { + const localVarAxiosArgs = await localVarAxiosParamCreator.getRoot(driveId, $select, $expand, options); const localVarOperationServerIndex = configuration?.serverIndex ?? 0; const localVarOperationServerBasePath = operationServerMap['DrivesRootApi.getRoot']?.[localVarOperationServerIndex]?.url; return (axios, basePath) => createRequestFunction(localVarAxiosArgs, globalAxios, BASE_PATH, configuration)(axios, localVarOperationServerBasePath || basePath); @@ -4452,11 +4539,12 @@ export const DrivesRootApiFactory = function (configuration?: Configuration, bas * @summary Get root from arbitrary space * @param {string} driveId key: id of drive * @param {Set} [$select] Select additional properties to be returned. + * @param {Set} [$expand] Expand related entities to be returned. * @param {*} [options] Override http request option. * @throws {RequiredError} */ - getRoot(driveId: string, $select?: Set, options?: RawAxiosRequestConfig): AxiosPromise { - return localVarFp.getRoot(driveId, $select, options).then((request) => request(axios, basePath)); + getRoot(driveId: string, $select?: Set, $expand?: Set, options?: RawAxiosRequestConfig): AxiosPromise { + return localVarFp.getRoot(driveId, $select, $expand, options).then((request) => request(axios, basePath)); }, /** * Sends a sharing invitation for the root of a `drive`. A sharing invitation provides permissions to the recipients and optionally sends them an email with a sharing link. The response will be a permission object with the grantedToV2 property containing the created grant details. ## Roles property values For now, roles are only identified by a uuid. There are no hardcoded aliases like `read` or `write` because role actions can be completely customized. @@ -4569,11 +4657,12 @@ export class DrivesRootApi extends BaseAPI { * @summary Get root from arbitrary space * @param {string} driveId key: id of drive * @param {Set} [$select] Select additional properties to be returned. + * @param {Set} [$expand] Expand related entities to be returned. * @param {*} [options] Override http request option. * @throws {RequiredError} */ - public getRoot(driveId: string, $select?: Set, options?: RawAxiosRequestConfig) { - return DrivesRootApiFp(this.configuration).getRoot(driveId, $select, options).then((request) => request(this.axios, this.basePath)); + public getRoot(driveId: string, $select?: Set, $expand?: Set, options?: RawAxiosRequestConfig) { + return DrivesRootApiFp(this.configuration).getRoot(driveId, $select, $expand, options).then((request) => request(this.axios, this.basePath)); } /** @@ -4643,8 +4732,14 @@ export type CreateDriveItemAtLibreGraphMissingParentsBehaviorEnum = typeof Creat export const GetRootSelectEnum = { MicrosoftGraphDownloadUrl: '@microsoft.graph.downloadUrl', LibreGraphPermissionsActionsAllowedValues: '@libre.graph.permissions.actions.allowedValues', + LibreGraphShareTypes: '@libre.graph.shareTypes', } as const; export type GetRootSelectEnum = typeof GetRootSelectEnum[keyof typeof GetRootSelectEnum]; +export const GetRootExpandEnum = { + Children: 'children', + Thumbnails: 'thumbnails', +} as const; +export type GetRootExpandEnum = typeof GetRootExpandEnum[keyof typeof GetRootExpandEnum]; export const ListPermissionsSpaceRootSelectEnum = { LibreGraphPermissionsActionsAllowedValues: '@libre.graph.permissions.actions.allowedValues', LibreGraphPermissionsRolesAllowedValues: '@libre.graph.permissions.roles.allowedValues', @@ -8387,10 +8482,11 @@ export const MeDriveRootApiAxiosParamCreator = function (configuration?: Configu * * @summary Get root from personal space * @param {Set} [$select] Select additional properties to be returned. + * @param {Set} [$expand] Expand related entities to be returned. * @param {*} [options] Override http request option. * @throws {RequiredError} */ - homeGetRoot: async ($select?: Set, options: RawAxiosRequestConfig = {}): Promise => { + homeGetRoot: async ($select?: Set, $expand?: Set, options: RawAxiosRequestConfig = {}): Promise => { const localVarPath = `/v1.0/me/drive/root`; // use dummy base URL string because the URL constructor only accepts absolute URLs. const localVarUrlObj = new URL(localVarPath, DUMMY_BASE_URL); @@ -8413,6 +8509,10 @@ export const MeDriveRootApiAxiosParamCreator = function (configuration?: Configu localVarQueryParameter['$select'] = Array.from($select).join(COLLECTION_FORMATS.csv); } + if ($expand) { + localVarQueryParameter['$expand'] = Array.from($expand).join(COLLECTION_FORMATS.csv); + } + localVarHeaderParameter['Accept'] = 'application/json'; setSearchParams(localVarUrlObj, localVarQueryParameter); @@ -8437,11 +8537,12 @@ export const MeDriveRootApiFp = function(configuration?: Configuration) { * * @summary Get root from personal space * @param {Set} [$select] Select additional properties to be returned. + * @param {Set} [$expand] Expand related entities to be returned. * @param {*} [options] Override http request option. * @throws {RequiredError} */ - async homeGetRoot($select?: Set, options?: RawAxiosRequestConfig): Promise<(axios?: AxiosInstance, basePath?: string) => AxiosPromise> { - const localVarAxiosArgs = await localVarAxiosParamCreator.homeGetRoot($select, options); + async homeGetRoot($select?: Set, $expand?: Set, options?: RawAxiosRequestConfig): Promise<(axios?: AxiosInstance, basePath?: string) => AxiosPromise> { + const localVarAxiosArgs = await localVarAxiosParamCreator.homeGetRoot($select, $expand, options); const localVarOperationServerIndex = configuration?.serverIndex ?? 0; const localVarOperationServerBasePath = operationServerMap['MeDriveRootApi.homeGetRoot']?.[localVarOperationServerIndex]?.url; return (axios, basePath) => createRequestFunction(localVarAxiosArgs, globalAxios, BASE_PATH, configuration)(axios, localVarOperationServerBasePath || basePath); @@ -8459,11 +8560,12 @@ export const MeDriveRootApiFactory = function (configuration?: Configuration, ba * * @summary Get root from personal space * @param {Set} [$select] Select additional properties to be returned. + * @param {Set} [$expand] Expand related entities to be returned. * @param {*} [options] Override http request option. * @throws {RequiredError} */ - homeGetRoot($select?: Set, options?: RawAxiosRequestConfig): AxiosPromise { - return localVarFp.homeGetRoot($select, options).then((request) => request(axios, basePath)); + homeGetRoot($select?: Set, $expand?: Set, options?: RawAxiosRequestConfig): AxiosPromise { + return localVarFp.homeGetRoot($select, $expand, options).then((request) => request(axios, basePath)); }, }; }; @@ -8476,19 +8578,26 @@ export class MeDriveRootApi extends BaseAPI { * * @summary Get root from personal space * @param {Set} [$select] Select additional properties to be returned. + * @param {Set} [$expand] Expand related entities to be returned. * @param {*} [options] Override http request option. * @throws {RequiredError} */ - public homeGetRoot($select?: Set, options?: RawAxiosRequestConfig) { - return MeDriveRootApiFp(this.configuration).homeGetRoot($select, options).then((request) => request(this.axios, this.basePath)); + public homeGetRoot($select?: Set, $expand?: Set, options?: RawAxiosRequestConfig) { + return MeDriveRootApiFp(this.configuration).homeGetRoot($select, $expand, options).then((request) => request(this.axios, this.basePath)); } } export const HomeGetRootSelectEnum = { MicrosoftGraphDownloadUrl: '@microsoft.graph.downloadUrl', LibreGraphPermissionsActionsAllowedValues: '@libre.graph.permissions.actions.allowedValues', + LibreGraphShareTypes: '@libre.graph.shareTypes', } as const; export type HomeGetRootSelectEnum = typeof HomeGetRootSelectEnum[keyof typeof HomeGetRootSelectEnum]; +export const HomeGetRootExpandEnum = { + Children: 'children', + Thumbnails: 'thumbnails', +} as const; +export type HomeGetRootExpandEnum = typeof HomeGetRootExpandEnum[keyof typeof HomeGetRootExpandEnum]; /** @@ -8600,6 +8709,7 @@ export class MeDriveRootChildrenApi extends BaseAPI { export const HomeGetChildrenSelectEnum = { MicrosoftGraphDownloadUrl: '@microsoft.graph.downloadUrl', LibreGraphPermissionsActionsAllowedValues: '@libre.graph.permissions.actions.allowedValues', + LibreGraphShareTypes: '@libre.graph.shareTypes', } as const; export type HomeGetChildrenSelectEnum = typeof HomeGetChildrenSelectEnum[keyof typeof HomeGetChildrenSelectEnum]; diff --git a/packages/web-client/src/helpers/resource/graph.ts b/packages/web-client/src/helpers/resource/graph.ts index 11c40902e3f..6dcdf7847e6 100644 --- a/packages/web-client/src/helpers/resource/graph.ts +++ b/packages/web-client/src/helpers/resource/graph.ts @@ -90,7 +90,6 @@ export const buildResourceFromDriveItem = ( image: driveItem.image, photo: driveItem.photo, video: (driveItem as any).video, - motionPhoto: (driveItem as any)['@libre.graph.motionPhoto'], livePhoto: (driveItem as any)['@libre.graph.livePhoto'], extraProps: {}, hasPreview: () => !!driveItem.thumbnails?.length || !isFolder, From c646eb3c220cfe59793d80c09e3fb67539653445 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Sun, 6 Sep 2026 22:21:34 +0200 Subject: [PATCH 05/23] refactor(web-client): use the generated ops for the graph listing Stat and children now go through the generated v1.0 operations, so $select and $expand are typed and the driveItem facets no longer need casts. The path form still cannot use the generated operation: it percent-encodes the item id, which turns the ':/' the server matches on into '%3A%2F'. It now builds the request with the generated param creator and rewrites the item segment, encoding each path segment as the server expects. An unencoded ':' in a folder name was read as the colon-syntax delimiter before. Dropping the casts also surfaced that graph reports share types by key while every consumer compares against the numeric values. --- .../src/graph/driveItems/driveItems.ts | 78 ++++++++++----- .../web-client/src/graph/driveItems/types.ts | 26 +++-- .../web-client/src/helpers/resource/graph.ts | 42 +++++---- .../unit/graph/driveItems/driveItems.spec.ts | 94 +++++++++++++++++++ .../tests/unit/helpers/resource/graph.spec.ts | 18 +++- .../services/folder/loaders/loaderSpace.ts | 26 +++-- 6 files changed, 224 insertions(+), 60 deletions(-) create mode 100644 packages/web-client/tests/unit/graph/driveItems/driveItems.spec.ts diff --git a/packages/web-client/src/graph/driveItems/driveItems.ts b/packages/web-client/src/graph/driveItems/driveItems.ts index 9a1fe54fedb..4ceedc5f794 100644 --- a/packages/web-client/src/graph/driveItems/driveItems.ts +++ b/packages/web-client/src/graph/driveItems/driveItems.ts @@ -1,15 +1,24 @@ -import { DriveItemApiFactory, DrivesRootApiFactory, MeDriveApiFactory } from './../generated' -import { urlJoin } from '../../utils' +import { + DriveItem, + DriveItemApiAxiosParamCreator, + DriveItemApiFactory, + DrivesRootApiFactory, + MeDriveApiFactory +} from './../generated' import type { GraphFactoryOptions } from './../types' -import type { DriveItemQueryOptions, GraphDriveItems } from './types' +import type { GraphDriveItems } from './types' -const odataQuery = ({ select, expand }: DriveItemQueryOptions = {}) => { - const params = [ - ...(select?.length ? [`$select=${select.join(',')}`] : []), - ...(expand?.length ? [`$expand=${expand.join(',')}`] : []) - ] - return params.length ? `?${params.join('&')}` : '' -} +// placeholder for the item id in a colon-syntax url. it consists of unreserved +// characters only, so it survives encodeURIComponent and can be swapped for the +// path after the generated param creator has built the url. +const COLON_PATH_PLACEHOLDER = '__colon_path__' + +// the server recognizes a path lookup by a literal ':/' in the encoded url and +// expects every path segment to be percent-encoded, a ':' inside a name as +// '%3A'. encodeURIComponent does exactly that. See ResolveGraphPath in +// services/graph/pkg/middleware/path_lookup.go on the server side. +const colonPathRef = (path: string) => + `root:/${path.split('/').filter(Boolean).map(encodeURIComponent).join('/')}` export const DriveItemsFactory = ({ axiosClient, @@ -75,25 +84,50 @@ export const DriveItemsFactory = ({ return data?.value || [] }, - // statDriveItem stats an item by id or by graph's colon path syntax. - // Hand-rolled for the same reason as listDriveItemChildren: the generated - // client has no $select, no $expand and no path lookup. + // statDriveItem stats an item by id or by path. The path form cannot go + // through the generated operation: it percent-encodes the item id, which + // turns the ':/' the server matches on into '%3A%2F'. So the request is + // built by the generated param creator and only the item segment is + // rewritten afterwards, keeping the query and headers generated. async statDriveItem(driveId, ref, options, requestOptions) { - const suffix = ref.itemId - ? `/items/${ref.itemId}` - : `/root:${urlJoin(ref.path, { leadingSlash: true })}` - const { data } = await axiosClient.get( - `${config.basePath}/v1.0/drives/${driveId}${suffix}${odataQuery(options)}`, + if (ref.itemId) { + const { data } = await driveItemApiFactory.getDriveItemV1( + driveId, + ref.itemId, + options?.select, + options?.expand, + requestOptions + ) + return data + } + + const { url, options: axiosOptions } = await DriveItemApiAxiosParamCreator( + config + ).getDriveItemV1( + driveId, + COLON_PATH_PLACEHOLDER, + options?.select, + options?.expand, requestOptions ) + + const { data } = await axiosClient.request({ + ...axiosOptions, + // the path form is anchored at the drive root, so it replaces the + // whole '/items/{item-id}' segment rather than just the id + url: `${config.basePath}${url.replace( + `items/${COLON_PATH_PLACEHOLDER}`, + colonPathRef(ref.path) + )}` + }) return data }, - // listDriveItemChildren lists a folder's children. Hand-rolled because the - // generated client only covers the personal drive root. async listDriveItemChildren(driveId, itemId, options, requestOptions) { - const { data } = await axiosClient.get( - `${config.basePath}/v1.0/drives/${driveId}/items/${itemId}/children${odataQuery(options)}`, + const { data } = await driveItemApiFactory.getDriveItemChildren( + driveId, + itemId, + options?.select, requestOptions ) return data?.value || [] diff --git a/packages/web-client/src/graph/driveItems/types.ts b/packages/web-client/src/graph/driveItems/types.ts index 6c1dd55e006..002b96f38c4 100644 --- a/packages/web-client/src/graph/driveItems/types.ts +++ b/packages/web-client/src/graph/driveItems/types.ts @@ -1,16 +1,28 @@ -import { DriveItem } from '../generated' +import { + DriveItem, + GetDriveItemChildrenSelectEnum, + GetDriveItemV1ExpandEnum, + GetDriveItemV1SelectEnum +} from '../generated' import type { GraphRequestOptions } from '../types' -export interface DriveItemQueryOptions { - select?: string[] - expand?: string[] +export interface DriveItemStatOptions { + select?: Set + expand?: Set } +export interface DriveItemChildrenOptions { + select?: Set +} + +// a driveItem is addressed either by its id or by its path, never by both +export type DriveItemRef = { itemId: string; path?: never } | { itemId?: never; path: string } + export interface GraphDriveItems { listDriveItemChildren: ( driveId: string, itemId: string, - options?: DriveItemQueryOptions, + options?: DriveItemChildrenOptions, requestOptions?: GraphRequestOptions ) => Promise getDriveItem: ( @@ -20,8 +32,8 @@ export interface GraphDriveItems { ) => Promise statDriveItem: ( driveId: string, - ref: { itemId?: string; path?: string }, - options?: DriveItemQueryOptions, + ref: DriveItemRef, + options?: DriveItemStatOptions, requestOptions?: GraphRequestOptions ) => Promise createDriveItem: ( diff --git a/packages/web-client/src/helpers/resource/graph.ts b/packages/web-client/src/helpers/resource/graph.ts index 6dcdf7847e6..8aa8b49eb15 100644 --- a/packages/web-client/src/helpers/resource/graph.ts +++ b/packages/web-client/src/helpers/resource/graph.ts @@ -1,6 +1,8 @@ -import { basename, extname } from 'path' +import { extname } from 'path' import { urlJoin } from '../../utils' import { DavPermission } from '../../webdav/constants' +import { ShareTypes } from '../share' +import { extractStorageId } from './functions' import type { DriveItem } from '../../graph/generated' import type { SpaceResource } from '../space' import type { Resource } from './types' @@ -51,15 +53,18 @@ export const buildResourceFromDriveItem = ( const isFolder = !!driveItem.folder const name = driveItem.name || '' const path = pathOverride ?? urlJoin(parentPath, name, { leadingSlash: true }) - const actions = (driveItem as any)['@libre.graph.permissions.actions.allowedValues'] as string[] - const shareTypes = ((driveItem as any)['@libre.graph.shareTypes'] || []) as string[] - const lock = (driveItem as any).lockInfo + const actions = driveItem['@libre.graph.permissions.actions.allowedValues'] + const lock = driveItem.lockInfo const permissions = davPermissionsFromActions(actions) + // graph reports share types by key, the resource carries the numeric values + const shareTypes = ShareTypes.getValues( + ShareTypes.getByKeys(driveItem['@libre.graph.shareTypes'] || []).filter(Boolean) + ) - const r: any = { + const r: Resource = { id: driveItem.id, fileId: driveItem.id, - storageId: space.id, + storageId: extractStorageId(driveItem.id), parentFolderId: driveItem.parentReference?.id, mimeType: driveItem.file?.mimeType, name, @@ -71,28 +76,29 @@ export const buildResourceFromDriveItem = ( locked: !!lock, lockOwner: lock?.owners?.[0]?.displayName, lockTime: lock?.createdDateTime, - processing: !!(driveItem as any).pendingOperations?.pendingContentUpdate, + processing: !!driveItem.pendingOperations?.pendingContentUpdate, mdate: driveItem.lastModifiedDateTime, size: (driveItem.size ?? 0).toString(), permissions, isInVault: false, - starred: (driveItem as any)['@libre.graph.me.following'] === true, + starred: driveItem['@libre.graph.me.following'] === true, etag: driveItem.eTag, shareTypes, privateLink: driveItem.webUrl, - remoteItemId: (driveItem as any).remoteItem?.id, - remoteItemPath: (driveItem as any).remoteItem?.path, + remoteItemId: driveItem.remoteItem?.id, + remoteItemPath: driveItem.remoteItem?.path, // the item owner is always the space owner, see node.Owner() in reva - owner: (space as any).owner?.user || (space as any).owner, - tags: ((driveItem as any)['@libre.graph.tags'] || []) as string[], + owner: space.owner, + tags: driveItem['@libre.graph.tags'] || [], audio: driveItem.audio, location: driveItem.location, image: driveItem.image, photo: driveItem.photo, - video: (driveItem as any).video, - livePhoto: (driveItem as any)['@libre.graph.livePhoto'], extraProps: {}, - hasPreview: () => !!driveItem.thumbnails?.length || !isFolder, + // PROPFIND has a has-preview property, graph has none and cannot expand + // thumbnails on a stat. Approximated by "any file might have one", the + // preview service falls back to the file type icon when it doesn't. + hasPreview: () => !isFolder, canUpload: function (this: Resource) { return this.permissions.indexOf(DavPermission.FolderCreateable) >= 0 }, @@ -130,12 +136,12 @@ export const buildResourceFromDriveItem = ( return this.permissions.indexOf(DavPermission.Shared) >= 0 }, isShareRoot(): boolean { - return !!(driveItem as any).remoteItem + return !!driveItem.remoteItem }, getDomSelector: () => (driveItem.id || '').replace(/[^A-Za-z0-9\-_]/g, '') } - return r as Resource + return r } export const buildResourcesFromDriveItems = ( @@ -143,5 +149,3 @@ export const buildResourcesFromDriveItems = ( space: SpaceResource, parentPath = '' ): Resource[] => driveItems.map((item) => buildResourceFromDriveItem(item, space, parentPath)) - -export { basename } diff --git a/packages/web-client/tests/unit/graph/driveItems/driveItems.spec.ts b/packages/web-client/tests/unit/graph/driveItems/driveItems.spec.ts new file mode 100644 index 00000000000..4cf5e452cee --- /dev/null +++ b/packages/web-client/tests/unit/graph/driveItems/driveItems.spec.ts @@ -0,0 +1,94 @@ +import { AxiosInstance } from 'axios' +import { DriveItemsFactory } from '../../../../src/graph/driveItems/driveItems' +import { Configuration } from '../../../../src/graph/generated' + +const basePath = 'https://cloud.test/graph' + +const getClient = () => { + const request = vi.fn().mockResolvedValue({ data: { id: 'item' } }) + const axiosClient = { request, defaults: {} } as unknown as AxiosInstance + const driveItems = DriveItemsFactory({ + axiosClient, + config: new Configuration({ basePath }) + }) + return { driveItems, request } +} + +const requestedUrl = (request: ReturnType) => request.mock.calls[0][0].url + +describe('statDriveItem', () => { + it('stats by id through the generated operation', async () => { + const { driveItems, request } = getClient() + + await driveItems.statDriveItem('storage$space', { itemId: 'storage$space!item' }) + + expect(requestedUrl(request)).toBe( + `${basePath}/v1.0/drives/storage%24space/items/storage%24space!item` + ) + }) + + it('passes select and expand along', async () => { + const { driveItems, request } = getClient() + + await driveItems.statDriveItem( + 'storage$space', + { itemId: 'storage$space!item' }, + { + select: new Set(['@libre.graph.shareTypes' as const]), + expand: new Set(['children' as const]) + } + ) + + const url = requestedUrl(request) + expect(url).toContain('%24select=%40libre.graph.shareTypes') + expect(url).toContain('%24expand=children') + }) + + it('stats by path through the colon syntax', async () => { + const { driveItems, request } = getClient() + + await driveItems.statDriveItem('storage$space', { path: '/Documents/Notes' }) + + expect(requestedUrl(request)).toBe( + `${basePath}/v1.0/drives/storage%24space/root:/Documents/Notes` + ) + }) + + // the server splits the path on a literal ':/', so a colon in a name must + // arrive encoded or it would be read as the delimiter + it('encodes each path segment', async () => { + const { driveItems, request } = getClient() + + await driveItems.statDriveItem('storage$space', { path: '/Urlaub 2026/tag:1/foo&bar.txt' }) + + expect(requestedUrl(request)).toBe( + `${basePath}/v1.0/drives/storage%24space/root:/Urlaub%202026/tag%3A1/foo%26bar.txt` + ) + }) + + it('ignores a leading and trailing slash on the path', async () => { + const { driveItems, request } = getClient() + + await driveItems.statDriveItem('storage$space', { path: 'Documents/' }) + + expect(requestedUrl(request)).toBe(`${basePath}/v1.0/drives/storage%24space/root:/Documents`) + }) +}) + +describe('listDriveItemChildren', () => { + it('lists the children of an item', async () => { + const request = vi.fn().mockResolvedValue({ data: { value: [{ id: 'child' }] } }) + const axiosClient = { request, defaults: {} } as unknown as AxiosInstance + const driveItems = DriveItemsFactory({ + axiosClient, + config: new Configuration({ basePath }) + }) + + const children = await driveItems.listDriveItemChildren('storage$space', 'storage$space!item') + + expect(requestedUrl(request)).toBe( + `${basePath}/v1.0/drives/storage%24space/items/storage%24space!item/children` + ) + expect(children).toEqual([{ id: 'child' }]) + }) +}) diff --git a/packages/web-client/tests/unit/helpers/resource/graph.spec.ts b/packages/web-client/tests/unit/helpers/resource/graph.spec.ts index 18efa343e93..52b2eb707ca 100644 --- a/packages/web-client/tests/unit/helpers/resource/graph.spec.ts +++ b/packages/web-client/tests/unit/helpers/resource/graph.spec.ts @@ -1,4 +1,8 @@ -import { davPermissionsFromActions, buildResourceFromDriveItem } from '../../../../src/helpers/resource/graph' +import { + davPermissionsFromActions, + buildResourceFromDriveItem +} from '../../../../src/helpers/resource/graph' +import { ShareTypes } from '../../../../src/helpers/share' import type { SpaceResource } from '../../../../src/helpers/space' // a manager's action list, taken verbatim from a running server @@ -21,7 +25,7 @@ const managerActions = [ const space = { id: 'storage$space', webDavPath: '/dav/spaces/storage$space', - owner: { user: { id: 'alice', displayName: 'Alice' } } + owner: { id: 'alice', displayName: 'Alice' } } as unknown as SpaceResource describe('davPermissionsFromActions', () => { @@ -68,6 +72,7 @@ describe('buildResourceFromDriveItem', () => { expect(r.canBeDeleted()).toBe(true) expect(r.canRename()).toBe(true) expect(r.canDownload()).toBe(true) + expect(r.storageId).toBe('storage$space') expect(r.owner).toEqual({ id: 'alice', displayName: 'Alice' }) }) @@ -96,12 +101,17 @@ describe('buildResourceFromDriveItem', () => { expect(r.locked).toBe(true) expect(r.lockOwner).toBe('Alice') expect(r.processing).toBe(true) - expect(r.shareTypes).toEqual(['user', 'link']) + // graph reports keys, consumers compare against the numeric share types + expect(r.shareTypes).toEqual([ShareTypes.user.value, ShareTypes.link.value]) }) it('reports a shared item as a share root', () => { const r = buildResourceFromDriveItem( - { id: 'x', name: 'shared.txt', remoteItem: { id: 'other$drive!item', path: '/Project X' } } as any, + { + id: 'x', + name: 'shared.txt', + remoteItem: { id: 'other$drive!item', path: '/Project X' } + } as any, space ) expect(r.isShareRoot()).toBe(true) diff --git a/packages/web-pkg/src/services/folder/loaders/loaderSpace.ts b/packages/web-pkg/src/services/folder/loaders/loaderSpace.ts index 0a14f08e35f..fd064f2fcfd 100644 --- a/packages/web-pkg/src/services/folder/loaders/loaderSpace.ts +++ b/packages/web-pkg/src/services/folder/loaders/loaderSpace.ts @@ -11,10 +11,18 @@ import { SpaceResource } from '@opencloud-eu/web-client' import { unref } from 'vue' -import { urlJoin } from '@opencloud-eu/web-client' +import { + buildResourceFromDriveItem, + buildResourcesFromDriveItems, + urlJoin +} from '@opencloud-eu/web-client' import { FolderLoaderOptions } from './types' -import { DriveItem } from '@opencloud-eu/web-client/graph/generated' -import { buildResourceFromDriveItem, buildResourcesFromDriveItems } from '@opencloud-eu/web-client' +import { Graph } from '@opencloud-eu/web-client/graph' +import { + DriveItem, + GetDriveItemV1ExpandEnum, + GetDriveItemV1SelectEnum +} from '@opencloud-eu/web-client/graph/generated' import { isLocationSpacesActive, isLocationPublicActive } from '../../../router' import { getSharedDriveItem, setCurrentUserShareSpacePermissions } from '../../../helpers' import { useFileRouteReplace } from '../../../composables' @@ -143,10 +151,11 @@ export class FolderLoaderSpace implements FolderLoader { } } -const graphListingSelect = [ +const graphListingSelect = new Set([ '@libre.graph.permissions.actions.allowedValues', '@libre.graph.shareTypes' -] +]) +const graphListingExpand = new Set(['children']) // listFilesViaGraph lists a folder through graph, folder and children in one // request via $expand=children, the same shape PROPFIND with Depth: 1 returns. @@ -157,7 +166,7 @@ const listFilesViaGraph = async ({ fileId, signal }: { - graphClient: any + graphClient: Graph space: SpaceResource path: string fileId: string @@ -166,10 +175,11 @@ const listFilesViaGraph = async ({ const driveId = space.id.toString() // graph has no path lookup for the drive root, it is addressed by its id const isRoot = !path || path === '/' + const itemId = fileId || (isRoot ? space.root?.id : undefined) const driveItem = await graphClient.driveItems.statDriveItem( driveId, - fileId || isRoot ? { itemId: fileId || space.root?.id } : { path }, - { select: graphListingSelect, expand: ['children'] }, + itemId ? { itemId } : { path }, + { select: graphListingSelect, expand: graphListingExpand }, { signal } ) From b8cab7466d7e7b07460d0b7ad4b8b6f47146bc48 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 00:10:32 +0200 Subject: [PATCH 06/23] fix(web-pkg): translate folder vaults on the graph listing too The vault translation lived inside the webdav decorator, so the graph listing walked past it: a vault folder showed the encrypted names the server stores, and isInVault stayed false. The path translation and the decrypt/flag pass move to helpers/vaultTranslate, the webdav decorator and the graph listing both go through them. The graph listing moves next to the loader so it can be tested without importing the loader, which pulls the folderService singleton along. --- packages/web-pkg/src/helpers/index.ts | 1 + .../web-pkg/src/helpers/vaultTranslate.ts | 110 +++++++++++++ .../src/services/client/vaultWebDav.ts | 112 ++----------- .../services/folder/loaders/graphListing.ts | 68 ++++++++ .../services/folder/loaders/loaderSpace.ts | 60 +------ .../unit/services/client/vaultWebDav.spec.ts | 5 +- .../unit/services/folder/graphListing.spec.ts | 153 ++++++++++++++++++ 7 files changed, 351 insertions(+), 158 deletions(-) create mode 100644 packages/web-pkg/src/helpers/vaultTranslate.ts create mode 100644 packages/web-pkg/src/services/folder/loaders/graphListing.ts create mode 100644 packages/web-pkg/tests/unit/services/folder/graphListing.spec.ts diff --git a/packages/web-pkg/src/helpers/index.ts b/packages/web-pkg/src/helpers/index.ts index 82c5fe04193..8613bd9a2e6 100644 --- a/packages/web-pkg/src/helpers/index.ts +++ b/packages/web-pkg/src/helpers/index.ts @@ -13,6 +13,7 @@ export * from './fileExtension' export * from './extensionMarker' export * from './vault' export * from './vaultEngine' +export * from './vaultTranslate' export * from './filesize' export * from './fuse' export * from './locale' diff --git a/packages/web-pkg/src/helpers/vaultTranslate.ts b/packages/web-pkg/src/helpers/vaultTranslate.ts new file mode 100644 index 00000000000..83e083cb70e --- /dev/null +++ b/packages/web-pkg/src/helpers/vaultTranslate.ts @@ -0,0 +1,110 @@ +import { Resource, SpaceResource } from '@opencloud-eu/web-client' +import { decryptResourceInPlace, getVaultClaim, markVaultStatus, resolveVaultEngine } from './vault' +import { encryptVaultPath } from './vaultEngine' +import { ExtensionRegistry } from '../composables/piniaStores/extensionRegistry' + +/** + * Vault translation between what the user sees and what the server stores, + * independent of the client that carries the request. The webdav decorator + * (`createVaultWebDav`) and the graph folder listing both translate through + * these, so a vault behaves the same no matter which API served the listing. + */ + +/** + * Encrypt a clear-text path into its server-side form. No-op (sync fast path) + * when the path isn't claimed by any vault. For a *locked* vault we have no + * key, so we leave the path untouched - mutations on a locked vault aren't + * reachable through the UI (the unlock gate stops them first). + */ +export async function toVaultServerPath( + extensionRegistry: ExtensionRegistry, + space: SpaceResource, + path: string | undefined +): Promise { + if (!space || !path) { + return path + } + if (!getVaultClaim(extensionRegistry, space, path)) { + return path + } + const engine = await resolveVaultEngine(extensionRegistry, space, path) + return engine ? await encryptVaultPath(engine, path) : path +} + +/** + * Like `toVaultServerPath`, but for *writes*: refuse to operate when the path + * belongs to a vault that is locked. Reads can fall through and just show + * ciphertext, but a write (create / put / move / copy / delete) with the + * untranslated clear-text path would put a clear-text name on the server and + * corrupt the vault. The UI never reaches a locked vault, so this only ever + * fires as a fail-closed backstop - never silently send clear text. + */ +export async function toVaultServerPathForWrite( + extensionRegistry: ExtensionRegistry, + space: SpaceResource, + path: string | undefined +): Promise { + if (!space || !path) { + return path + } + const claim = getVaultClaim(extensionRegistry, space, path) + if (!claim) { + return path + } + // The vault *root* itself is a clear-text folder name - creating, renaming + // or deleting the vault needs no key, so let it through untouched even when + // no engine exists (e.g. while creating the vault, or for a locked one). + // Only *content* below the root carries an encryptable name. + if (claim.vaultRoot === path) { + return path + } + const engine = await resolveVaultEngine(extensionRegistry, space, path) + if (!engine) { + throw new Error( + `Refusing to write a clear-text path into the locked vault "${claim.vaultRoot}"` + ) + } + return encryptVaultPath(engine, path) +} + +/** + * Decrypt the names of resources coming back from the server and flag their + * vault status. Resources are grouped by vault root so a mixed listing (e.g. + * the trash bin, where each item's original location may sit in a different + * vault) resolves each engine exactly once. `markVaultStatus` is claim-based + * and runs even while a vault is locked; the actual name decrypt only happens + * when the vault is unlocked (the engine resolves). + */ +export async function applyVaultFromServer( + extensionRegistry: ExtensionRegistry, + space: SpaceResource, + resources: Array +): Promise { + const list = resources.filter((r): r is Resource => !!r?.path) + if (!space || !list.length) { + return + } + + const byRoot = new Map() + for (const r of list) { + const claim = getVaultClaim(extensionRegistry, space, r.path) + if (!claim) { + continue + } + const group = byRoot.get(claim.vaultRoot) ?? [] + group.push(r) + byRoot.set(claim.vaultRoot, group) + } + + for (const [vaultRoot, group] of byRoot) { + const engine = await resolveVaultEngine(extensionRegistry, space, vaultRoot) + if (engine) { + await Promise.all(group.map((r) => decryptResourceInPlace(engine, r))) + } + } + + // Always (re-)flag vault status. Idempotent after decryptResourceInPlace, and + // the only thing that fires for a locked vault or for a vault root surfaced + // in a parent listing (where no engine resolves against it). + markVaultStatus(extensionRegistry, space, list) +} diff --git a/packages/web-pkg/src/services/client/vaultWebDav.ts b/packages/web-pkg/src/services/client/vaultWebDav.ts index d8cf9186355..40d7311198e 100644 --- a/packages/web-pkg/src/services/client/vaultWebDav.ts +++ b/packages/web-pkg/src/services/client/vaultWebDav.ts @@ -6,12 +6,12 @@ import { WebDAV } from '@opencloud-eu/web-client/webdav' // before `services`) creates an evaluation cycle that leaves unrelated // composable exports temporarily undefined. import { useExtensionRegistry } from '../../composables/piniaStores/extensionRegistry' +import { getVaultClaim, resolveVaultEngine } from '../../helpers/vault' import { - decryptResourceInPlace, - getVaultClaim, - markVaultStatus, - resolveVaultEngine -} from '../../helpers/vault' + applyVaultFromServer, + toVaultServerPath, + toVaultServerPathForWrite +} from '../../helpers/vaultTranslate' import { encryptVaultPath } from '../../helpers/vaultEngine' import { streamToArrayBuffer } from '../../helpers/streams' @@ -46,104 +46,18 @@ import { streamToArrayBuffer } from '../../helpers/streams' * then this is a known limitation. */ export function createVaultWebDav(inner: WebDAV): WebDAV { - /** - * Encrypt a clear-text path into its server-side form. No-op (sync fast path) - * when the path isn't claimed by any vault. For a *locked* vault we have no - * key, so we leave the path untouched - mutations on a locked vault aren't - * reachable through the UI (the unlock gate stops them first). - */ - async function toServerPath( - space: SpaceResource, - path: string | undefined - ): Promise { - if (!space || !path) { - return path - } - const registry = useExtensionRegistry() - if (!getVaultClaim(registry, space, path)) { - return path - } - const engine = await resolveVaultEngine(registry, space, path) - return engine ? await encryptVaultPath(engine, path) : path + function registry() { + return useExtensionRegistry() } - /** - * Like `toServerPath`, but for *writes*: refuse to operate when the path - * belongs to a vault that is locked. Reads can fall through and just show - * ciphertext, but a write (create / put / move / copy / delete) with the - * untranslated clear-text path would put a clear-text name on the server and - * corrupt the vault. The UI never reaches a locked vault, so this only ever - * fires as a fail-closed backstop - never silently send clear text. - */ - async function toServerPathForWrite( - space: SpaceResource, - path: string | undefined - ): Promise { - if (!space || !path) { - return path - } - const registry = useExtensionRegistry() - const claim = getVaultClaim(registry, space, path) - if (!claim) { - return path - } - // The vault *root* itself is a clear-text folder name - creating, renaming - // or deleting the vault needs no key, so let it through untouched even when - // no engine exists (e.g. while creating the vault, or for a locked one). - // Only *content* below the root carries an encryptable name. - if (claim.vaultRoot === path) { - return path - } - const engine = await resolveVaultEngine(registry, space, path) - if (!engine) { - throw new Error( - `Refusing to write a clear-text path into the locked vault "${claim.vaultRoot}"` - ) - } - return encryptVaultPath(engine, path) - } + const toServerPath = (space: SpaceResource, path: string | undefined) => + toVaultServerPath(registry(), space, path) - /** - * Decrypt the names of resources coming back from the server and flag their - * vault status. Resources are grouped by vault root so a mixed listing (e.g. - * the trash bin, where each item's original location may sit in a different - * vault) resolves each engine exactly once. `markVaultStatus` is claim-based - * and runs even while a vault is locked; the actual name decrypt only happens - * when the vault is unlocked (the engine resolves). - */ - async function fromServer( - space: SpaceResource, - resources: Array - ): Promise { - const list = resources.filter((r): r is Resource => !!r?.path) - if (!space || !list.length) { - return - } - const registry = useExtensionRegistry() + const toServerPathForWrite = (space: SpaceResource, path: string | undefined) => + toVaultServerPathForWrite(registry(), space, path) - const byRoot = new Map() - for (const r of list) { - const claim = getVaultClaim(registry, space, r.path) - if (!claim) { - continue - } - const group = byRoot.get(claim.vaultRoot) ?? [] - group.push(r) - byRoot.set(claim.vaultRoot, group) - } - - for (const [vaultRoot, group] of byRoot) { - const engine = await resolveVaultEngine(registry, space, vaultRoot) - if (engine) { - await Promise.all(group.map((r) => decryptResourceInPlace(engine, r))) - } - } - - // Always (re-)flag vault status. Idempotent after decryptResourceInPlace, - // and the only thing that fires for a locked vault or for a vault root - // surfaced in a parent listing (where no engine resolves against it). - markVaultStatus(registry, space, list) - } + const fromServer = (space: SpaceResource, resources: Array) => + applyVaultFromServer(registry(), space, resources) return { ...inner, diff --git a/packages/web-pkg/src/services/folder/loaders/graphListing.ts b/packages/web-pkg/src/services/folder/loaders/graphListing.ts new file mode 100644 index 00000000000..9ac6073abf6 --- /dev/null +++ b/packages/web-pkg/src/services/folder/loaders/graphListing.ts @@ -0,0 +1,68 @@ +import { + buildResourceFromDriveItem, + buildResourcesFromDriveItems, + SpaceResource, + urlJoin +} from '@opencloud-eu/web-client' +import { Graph } from '@opencloud-eu/web-client/graph' +import { + GetDriveItemV1ExpandEnum, + GetDriveItemV1SelectEnum +} from '@opencloud-eu/web-client/graph/generated' +// the specific store / helper modules, not the barrels: this file sits in the +// services layer and re-entering those barrels creates an evaluation cycle +import { useExtensionRegistry } from '../../../composables/piniaStores/extensionRegistry' +import { applyVaultFromServer, toVaultServerPath } from '../../../helpers/vaultTranslate' + +const graphListingSelect = new Set([ + '@libre.graph.permissions.actions.allowedValues', + '@libre.graph.shareTypes' +]) +const graphListingExpand = new Set(['children']) + +// listFilesViaGraph lists a folder through graph, folder and children in one +// request via $expand=children, the same shape PROPFIND with Depth: 1 returns. +// Lives next to the loader rather than inside it so it can be tested without +// importing the loader, which pulls the folderService singleton along. +export const listFilesViaGraph = async ({ + graphClient, + space, + path, + fileId, + signal +}: { + graphClient: Graph + space: SpaceResource + path: string + fileId: string + signal: AbortSignal +}) => { + const driveId = space.id.toString() + // graph has no path lookup for the drive root, it is addressed by its id + const isRoot = !path || path === '/' + const itemId = fileId || (isRoot ? space.root?.id : undefined) + const registry = useExtensionRegistry() + // inside a vault the server knows the encrypted names only + const serverPath = await toVaultServerPath(registry, space, path) + const driveItem = await graphClient.driveItems.statDriveItem( + driveId, + itemId ? { itemId } : { path: serverPath }, + { select: graphListingSelect, expand: graphListingExpand }, + { signal } + ) + + // the item is authoritative, not the url: the route correction in the loader + // exists to fix a stale path. the drive root reports itself as '.' + const parentPath = driveItem.parentReference?.path + const currentPath = + !parentPath || parentPath === '.' + ? '/' + : urlJoin(parentPath, driveItem.name, { leadingSlash: true }) + const currentFolder = buildResourceFromDriveItem(driveItem, space, '', currentPath) + const children = buildResourcesFromDriveItems(driveItem.children || [], space, currentFolder.path) + + // the webdav client has its vault decorator, the graph path translates here + await applyVaultFromServer(registry, space, [currentFolder, ...children]) + + return { resource: currentFolder, children } +} diff --git a/packages/web-pkg/src/services/folder/loaders/loaderSpace.ts b/packages/web-pkg/src/services/folder/loaders/loaderSpace.ts index fd064f2fcfd..b03bed101a3 100644 --- a/packages/web-pkg/src/services/folder/loaders/loaderSpace.ts +++ b/packages/web-pkg/src/services/folder/loaders/loaderSpace.ts @@ -11,18 +11,9 @@ import { SpaceResource } from '@opencloud-eu/web-client' import { unref } from 'vue' -import { - buildResourceFromDriveItem, - buildResourcesFromDriveItems, - urlJoin -} from '@opencloud-eu/web-client' import { FolderLoaderOptions } from './types' -import { Graph } from '@opencloud-eu/web-client/graph' -import { - DriveItem, - GetDriveItemV1ExpandEnum, - GetDriveItemV1SelectEnum -} from '@opencloud-eu/web-client/graph/generated' +import { listFilesViaGraph } from './graphListing' +import { DriveItem } from '@opencloud-eu/web-client/graph/generated' import { isLocationSpacesActive, isLocationPublicActive } from '../../../router' import { getSharedDriveItem, setCurrentUserShareSpacePermissions } from '../../../helpers' import { useFileRouteReplace } from '../../../composables' @@ -150,50 +141,3 @@ export class FolderLoaderSpace implements FolderLoader { }).restartable() } } - -const graphListingSelect = new Set([ - '@libre.graph.permissions.actions.allowedValues', - '@libre.graph.shareTypes' -]) -const graphListingExpand = new Set(['children']) - -// listFilesViaGraph lists a folder through graph, folder and children in one -// request via $expand=children, the same shape PROPFIND with Depth: 1 returns. -const listFilesViaGraph = async ({ - graphClient, - space, - path, - fileId, - signal -}: { - graphClient: Graph - space: SpaceResource - path: string - fileId: string - signal: AbortSignal -}) => { - const driveId = space.id.toString() - // graph has no path lookup for the drive root, it is addressed by its id - const isRoot = !path || path === '/' - const itemId = fileId || (isRoot ? space.root?.id : undefined) - const driveItem = await graphClient.driveItems.statDriveItem( - driveId, - itemId ? { itemId } : { path }, - { select: graphListingSelect, expand: graphListingExpand }, - { signal } - ) - - // the item is authoritative, not the url: the route correction below exists - // to fix a stale path. the drive root reports itself as '.' - const parentPath = driveItem.parentReference?.path - const currentPath = - !parentPath || parentPath === '.' - ? '/' - : urlJoin(parentPath, driveItem.name, { leadingSlash: true }) - const currentFolder = buildResourceFromDriveItem(driveItem, space, '', currentPath) - - return { - resource: currentFolder, - children: buildResourcesFromDriveItems(driveItem.children || [], space, currentFolder.path) - } -} diff --git a/packages/web-pkg/tests/unit/services/client/vaultWebDav.spec.ts b/packages/web-pkg/tests/unit/services/client/vaultWebDav.spec.ts index 7a9c9ba029b..34ea6be139b 100644 --- a/packages/web-pkg/tests/unit/services/client/vaultWebDav.spec.ts +++ b/packages/web-pkg/tests/unit/services/client/vaultWebDav.spec.ts @@ -11,7 +11,10 @@ import { vi.mock('../../../../src/composables/piniaStores/extensionRegistry', () => ({ useExtensionRegistry: vi.fn(() => ({})) })) -vi.mock('../../../../src/helpers/vault', () => ({ +// only the primitives are mocked, the translation on top of them (which moved +// to vaultTranslate so the graph listing can use it too) runs for real +vi.mock('../../../../src/helpers/vault', async (importOriginal) => ({ + ...(await importOriginal()), getVaultClaim: vi.fn(), resolveVaultEngine: vi.fn(), decryptResourceInPlace: vi.fn((_engine, r) => Promise.resolve(r)), diff --git a/packages/web-pkg/tests/unit/services/folder/graphListing.spec.ts b/packages/web-pkg/tests/unit/services/folder/graphListing.spec.ts new file mode 100644 index 00000000000..58464a1af25 --- /dev/null +++ b/packages/web-pkg/tests/unit/services/folder/graphListing.spec.ts @@ -0,0 +1,153 @@ +import { SpaceResource } from '@opencloud-eu/web-client' +import { Graph } from '@opencloud-eu/web-client/graph' +import { DriveItem } from '@opencloud-eu/web-client/graph/generated' +import { listFilesViaGraph } from '../../../../src/services/folder/loaders/graphListing' +import { + decryptResourceInPlace, + getVaultClaim, + markVaultStatus, + resolveVaultEngine +} from '../../../../src/helpers/vault' + +vi.mock('../../../../src/composables/piniaStores/extensionRegistry', () => ({ + useExtensionRegistry: vi.fn(() => ({})) +})) +// only the vault primitives are mocked, the translation on top of them runs +vi.mock('../../../../src/helpers/vault', () => ({ + getVaultClaim: vi.fn(() => null), + resolveVaultEngine: vi.fn(), + decryptResourceInPlace: vi.fn((_engine, r) => Promise.resolve(r)), + markVaultStatus: vi.fn() +})) + +const space = { + id: 'storage$space', + webDavPath: '/dav/spaces/storage$space', + root: { id: 'storage$space!root' } +} as unknown as SpaceResource + +const folder = { + id: 'storage$space!folder', + name: 'Fotos', + folder: {}, + parentReference: { id: 'storage$space!root', path: '/' }, + children: [ + { id: 'storage$space!child', name: 'bild.jpg', file: { mimeType: 'image/jpeg' }, size: 12 } + ] +} as DriveItem + +function getGraphClient(driveItem: DriveItem = folder) { + const statDriveItem = vi.fn().mockResolvedValue(driveItem) + return { + graphClient: { driveItems: { statDriveItem } } as unknown as Graph, + statDriveItem + } +} + +beforeEach(() => { + vi.clearAllMocks() + vi.mocked(getVaultClaim).mockReturnValue(null) + vi.mocked(decryptResourceInPlace).mockImplementation((_engine, r) => Promise.resolve(r)) +}) + +describe('listFilesViaGraph', () => { + it('addresses the drive root by id, it has no path lookup', async () => { + const { graphClient, statDriveItem } = getGraphClient() + + await listFilesViaGraph({ graphClient, space, path: '/', fileId: null, signal: null }) + + expect(statDriveItem).toHaveBeenCalledWith( + 'storage$space', + { itemId: 'storage$space!root' }, + expect.objectContaining({ expand: new Set(['children']) }), + { signal: null } + ) + }) + + it('prefers the file id over the path', async () => { + const { graphClient, statDriveItem } = getGraphClient() + + await listFilesViaGraph({ + graphClient, + space, + path: '/Fotos', + fileId: 'storage$space!folder', + signal: null + }) + + expect(statDriveItem.mock.calls[0][1]).toEqual({ itemId: 'storage$space!folder' }) + }) + + it('looks a folder up by path when there is no file id', async () => { + const { graphClient, statDriveItem } = getGraphClient() + + await listFilesViaGraph({ graphClient, space, path: '/Fotos', fileId: null, signal: null }) + + expect(statDriveItem.mock.calls[0][1]).toEqual({ path: '/Fotos' }) + }) + + it('builds the folder and its children in one go', async () => { + const { graphClient } = getGraphClient() + + const { resource, children } = await listFilesViaGraph({ + graphClient, + space, + path: '/Fotos', + fileId: null, + signal: null + }) + + expect(resource.path).toBe('/Fotos') + expect(resource.isFolder).toBe(true) + expect(children).toHaveLength(1) + expect(children[0].path).toBe('/Fotos/bild.jpg') + expect(children[0].mimeType).toBe('image/jpeg') + }) + + describe('inside a vault', () => { + beforeEach(() => { + vi.mocked(getVaultClaim).mockImplementation((_registry, _space, path) => + path?.startsWith('/my.vault') ? ({ vaultRoot: '/my.vault' } as any) : null + ) + vi.mocked(resolveVaultEngine).mockResolvedValue({ + vaultRoot: '/my.vault', + encryptPath: vi.fn((p: string) => Promise.resolve(`ENC(${p})`)) + } as any) + }) + + it('encrypts the looked up path, the server knows the encrypted names only', async () => { + const { graphClient, statDriveItem } = getGraphClient() + + await listFilesViaGraph({ + graphClient, + space, + path: '/my.vault/Urlaub', + fileId: null, + signal: null + }) + + expect(statDriveItem.mock.calls[0][1]).toEqual({ path: '/my.vault/ENC(Urlaub)' }) + }) + + it('decrypts the folder and its children on the way back', async () => { + const { graphClient } = getGraphClient({ + id: 'storage$space!enc', + name: 'enc-folder', + folder: {}, + parentReference: { path: '/my.vault' }, + children: [{ id: 'storage$space!encChild', name: 'enc-child' }] + } as DriveItem) + + await listFilesViaGraph({ + graphClient, + space, + path: '/my.vault/Urlaub', + fileId: null, + signal: null + }) + + expect(decryptResourceInPlace).toHaveBeenCalledTimes(2) + expect(markVaultStatus).toHaveBeenCalledTimes(1) + }) + }) +}) From 1804f7164b8ad7a2a321ec531c1658c2c27feff7 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 00:14:51 +0200 Subject: [PATCH 07/23] feat: take has-preview from the expanded thumbnails The listing asked for $expand=children and guessed a preview for every file, since graph had no counterpart to PROPFIND's has-preview property. It now asks for thumbnails as well and reports a preview exactly for the items the server expanded them for, folder and children alike. Needs opencloud-eu/opencloud#3471 on the server side. --- .../web-client/src/helpers/resource/graph.ts | 7 +++---- .../tests/unit/helpers/resource/graph.spec.ts | 19 +++++++++++++++++++ .../services/folder/loaders/graphListing.ts | 4 +++- .../unit/services/folder/graphListing.spec.ts | 2 +- 4 files changed, 26 insertions(+), 6 deletions(-) diff --git a/packages/web-client/src/helpers/resource/graph.ts b/packages/web-client/src/helpers/resource/graph.ts index 8aa8b49eb15..c4bc83a0e6a 100644 --- a/packages/web-client/src/helpers/resource/graph.ts +++ b/packages/web-client/src/helpers/resource/graph.ts @@ -95,10 +95,9 @@ export const buildResourceFromDriveItem = ( image: driveItem.image, photo: driveItem.photo, extraProps: {}, - // PROPFIND has a has-preview property, graph has none and cannot expand - // thumbnails on a stat. Approximated by "any file might have one", the - // preview service falls back to the file type icon when it doesn't. - hasPreview: () => !isFolder, + // the server answers this through $expand=thumbnails, the counterpart of + // PROPFIND's has-preview property + hasPreview: () => !!driveItem.thumbnails?.length, canUpload: function (this: Resource) { return this.permissions.indexOf(DavPermission.FolderCreateable) >= 0 }, diff --git a/packages/web-client/tests/unit/helpers/resource/graph.spec.ts b/packages/web-client/tests/unit/helpers/resource/graph.spec.ts index 52b2eb707ca..3c07e33144d 100644 --- a/packages/web-client/tests/unit/helpers/resource/graph.spec.ts +++ b/packages/web-client/tests/unit/helpers/resource/graph.spec.ts @@ -105,6 +105,25 @@ describe('buildResourceFromDriveItem', () => { expect(r.shareTypes).toEqual([ShareTypes.user.value, ShareTypes.link.value]) }) + it('has a preview exactly when the server expanded thumbnails for it', () => { + const withThumbnail = buildResourceFromDriveItem( + { + id: 'x', + name: 'bild.jpg', + file: { mimeType: 'image/jpeg' }, + thumbnails: [{ small: { url: 'https://cloud.test/preview' } }] + } as any, + space + ) + const withoutThumbnail = buildResourceFromDriveItem( + { id: 'y', name: 'notes.json', file: { mimeType: 'application/json' } } as any, + space + ) + + expect(withThumbnail.hasPreview()).toBe(true) + expect(withoutThumbnail.hasPreview()).toBe(false) + }) + it('reports a shared item as a share root', () => { const r = buildResourceFromDriveItem( { diff --git a/packages/web-pkg/src/services/folder/loaders/graphListing.ts b/packages/web-pkg/src/services/folder/loaders/graphListing.ts index 9ac6073abf6..fac7485a266 100644 --- a/packages/web-pkg/src/services/folder/loaders/graphListing.ts +++ b/packages/web-pkg/src/services/folder/loaders/graphListing.ts @@ -18,7 +18,9 @@ const graphListingSelect = new Set([ '@libre.graph.permissions.actions.allowedValues', '@libre.graph.shareTypes' ]) -const graphListingExpand = new Set(['children']) +// thumbnails answer whether an item has a preview, for the folder and its +// children alike, which saves the client from guessing by mime type +const graphListingExpand = new Set(['children', 'thumbnails']) // listFilesViaGraph lists a folder through graph, folder and children in one // request via $expand=children, the same shape PROPFIND with Depth: 1 returns. diff --git a/packages/web-pkg/tests/unit/services/folder/graphListing.spec.ts b/packages/web-pkg/tests/unit/services/folder/graphListing.spec.ts index 58464a1af25..6bd47f48880 100644 --- a/packages/web-pkg/tests/unit/services/folder/graphListing.spec.ts +++ b/packages/web-pkg/tests/unit/services/folder/graphListing.spec.ts @@ -59,7 +59,7 @@ describe('listFilesViaGraph', () => { expect(statDriveItem).toHaveBeenCalledWith( 'storage$space', { itemId: 'storage$space!root' }, - expect.objectContaining({ expand: new Set(['children']) }), + expect.objectContaining({ expand: new Set(['children', 'thumbnails']) }), { signal: null } ) }) From 3c9f9313e5bc960fa27547658318b20891424ad5 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 09:06:07 +0200 Subject: [PATCH 08/23] fix(web-pkg): keep share paths relative to the share root A share space is rooted at the shared item, its webDavPath points straight at it. Graph answers in drive coordinates though: the stat of a received share reports the share root as "/folderToShare" rather than "/", so every child ended up one segment too deep and requests against it hit a 404. PROPFIND never showed this, its paths come back relative to the requested webdav root. Inside a share the requested path is the authoritative one. --- .../services/folder/loaders/graphListing.ts | 25 +++++++++++++------ .../unit/services/folder/graphListing.spec.ts | 24 ++++++++++++++++++ 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/packages/web-pkg/src/services/folder/loaders/graphListing.ts b/packages/web-pkg/src/services/folder/loaders/graphListing.ts index fac7485a266..2d64b1441cb 100644 --- a/packages/web-pkg/src/services/folder/loaders/graphListing.ts +++ b/packages/web-pkg/src/services/folder/loaders/graphListing.ts @@ -1,11 +1,13 @@ import { buildResourceFromDriveItem, buildResourcesFromDriveItems, + isShareSpaceResource, SpaceResource, urlJoin } from '@opencloud-eu/web-client' import { Graph } from '@opencloud-eu/web-client/graph' import { + DriveItem, GetDriveItemV1ExpandEnum, GetDriveItemV1SelectEnum } from '@opencloud-eu/web-client/graph/generated' @@ -22,6 +24,21 @@ const graphListingSelect = new Set([ // children alike, which saves the client from guessing by mime type const graphListingExpand = new Set(['children', 'thumbnails']) +// A share space is rooted at the shared item, but graph answers with paths in +// the owner's drive: the stat of a received share reports the share root as +// "/" rather than "/". Inside a share the requested path is therefore +// authoritative, everywhere else the item is (the route correction in the +// loader exists to fix a stale url, and the drive root reports itself as '.'). +const currentPathOf = (driveItem: DriveItem, space: SpaceResource, path: string) => { + if (isShareSpaceResource(space)) { + return path || '/' + } + const parentPath = driveItem.parentReference?.path + return !parentPath || parentPath === '.' + ? '/' + : urlJoin(parentPath, driveItem.name, { leadingSlash: true }) +} + // listFilesViaGraph lists a folder through graph, folder and children in one // request via $expand=children, the same shape PROPFIND with Depth: 1 returns. // Lives next to the loader rather than inside it so it can be tested without @@ -53,13 +70,7 @@ export const listFilesViaGraph = async ({ { signal } ) - // the item is authoritative, not the url: the route correction in the loader - // exists to fix a stale path. the drive root reports itself as '.' - const parentPath = driveItem.parentReference?.path - const currentPath = - !parentPath || parentPath === '.' - ? '/' - : urlJoin(parentPath, driveItem.name, { leadingSlash: true }) + const currentPath = currentPathOf(driveItem, space, path) const currentFolder = buildResourceFromDriveItem(driveItem, space, '', currentPath) const children = buildResourcesFromDriveItems(driveItem.children || [], space, currentFolder.path) diff --git a/packages/web-pkg/tests/unit/services/folder/graphListing.spec.ts b/packages/web-pkg/tests/unit/services/folder/graphListing.spec.ts index 6bd47f48880..fb91e812496 100644 --- a/packages/web-pkg/tests/unit/services/folder/graphListing.spec.ts +++ b/packages/web-pkg/tests/unit/services/folder/graphListing.spec.ts @@ -104,6 +104,30 @@ describe('listFilesViaGraph', () => { expect(children[0].mimeType).toBe('image/jpeg') }) + it('keeps the paths relative to the share root in a share space', async () => { + // graph answers in drive coordinates: the share root reports itself as + // "/folderToShare", while the space is rooted at exactly that item + const { graphClient } = getGraphClient({ + id: 'storage$space!shared', + name: 'folderToShare', + folder: {}, + parentReference: { path: '/' }, + children: [{ id: 'storage$space!child', name: 'lorem.txt' }] + } as DriveItem) + const shareSpace = { ...space, driveType: 'share' } as unknown as SpaceResource + + const { resource, children } = await listFilesViaGraph({ + graphClient, + space: shareSpace, + path: '/', + fileId: 'storage$space!shared', + signal: null + }) + + expect(resource.path).toBe('/') + expect(children[0].path).toBe('/lorem.txt') + }) + describe('inside a vault', () => { beforeEach(() => { vi.mocked(getVaultClaim).mockImplementation((_registry, _space, path) => From 2ca9c004b243ebb09243e8d3dda791027abde4fc Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 09:06:36 +0200 Subject: [PATCH 09/23] test(e2e): wait for the folder listing on either api Navigation waited for a PROPFIND, which spaces and shares no longer send now that they list through graph. The helpers accept either response, so public links and the trash bin keep working on webdav. --- .../objects/app-files/resource/actions.ts | 22 ++++++------------- .../objects/app-files/spaces/actions.ts | 3 ++- tests/e2e/support/utils/folderListing.ts | 19 ++++++++++++++++ tests/e2e/support/utils/index.ts | 1 + 4 files changed, 29 insertions(+), 16 deletions(-) create mode 100644 tests/e2e/support/utils/folderListing.ts diff --git a/tests/e2e/support/objects/app-files/resource/actions.ts b/tests/e2e/support/objects/app-files/resource/actions.ts index ab8d2bd0c72..e210a478c71 100644 --- a/tests/e2e/support/objects/app-files/resource/actions.ts +++ b/tests/e2e/support/objects/app-files/resource/actions.ts @@ -10,6 +10,7 @@ import { waitProcessingToFinish } from '../fileEvents' import { state } from '../../../../environment/shared' import { lstatSync, readFileSync } from 'fs' import { encodeWebDavPath } from '../../../utils' +import { isFolderListingResponse } from '../../../utils/folderListing' const appLoadingSpinner = '#app-loading-spinner' const topbarFilenameSelector = '#app-top-bar-resource .oc-resource-name' @@ -223,9 +224,7 @@ const clickResourceInEmbedMode = async ({ } await resource.waitFor() - const waitResponse = page.waitForResponse( - (resp) => resp.status() === 207 && resp.request().method() === 'PROPFIND' - ) + const waitResponse = page.waitForResponse(isFolderListingResponse) await resource.click() await waitResponse @@ -248,14 +247,12 @@ export const clickResource = async ({ const folder = name.replace(/'/g, "\\'").replace(/"/g, '\\"') const resource = page.locator(util.format(resourceNameSelector, folder)) - const propfindPromise = page.waitForResponse( - (resp) => resp.status() === 207 && resp.request().method() === 'PROPFIND' - ) + const listingPromise = page.waitForResponse(isFolderListingResponse) await resource.click() if (password && folder.includes('.vault')) { await unlockVault({ page, passphrase: password }) } - await propfindPromise + await listingPromise // wait for the loading spinner to disappear and page is loaded await expect(page.locator('#app-loading-spinner')).toBeHidden() } @@ -275,9 +272,7 @@ export const clickResourceFromBreadcrumb = async ({ await Promise.all([ page.waitForResponse( (resp) => - (resp.status() === 207 && - resp.request().method() === 'PROPFIND' && - resp.url().endsWith(encodeURIComponent(resource))) || + isFolderListingResponse(resp) || resp.url().endsWith(itemId) || resp.url().endsWith(encodeURIComponent(itemId)) ), @@ -615,10 +610,7 @@ const createDocumentFile = async ( "Editor should be either 'Collabora' or 'Euro-Office' but found " + editorToOpen ) } - await Promise.all([ - page.waitForResponse((res) => res.status() === 207 && res.request().method() === 'PROPFIND'), - editor.close(page) - ]) + await Promise.all([page.waitForResponse(isFolderListingResponse), editor.close(page)]) await page.locator(util.format(resourceNameSelector, name)).waitFor() // wait for lock to be removed @@ -744,7 +736,7 @@ export const editTextDocument = async ({ await page.locator(textEditorPlainTextInput).fill(content) const [putRequest] = await Promise.all([ page.waitForResponse((resp) => resp.status() === 204 && resp.request().method() === 'PUT'), - page.waitForResponse((resp) => resp.status() === 207 && resp.request().method() === 'PROPFIND'), + page.waitForResponse(isFolderListingResponse), page.locator(saveTextFileInEditorButton).click() ]) diff --git a/tests/e2e/support/objects/app-files/spaces/actions.ts b/tests/e2e/support/objects/app-files/spaces/actions.ts index 54b3b0ed037..d50c6b31c3b 100644 --- a/tests/e2e/support/objects/app-files/spaces/actions.ts +++ b/tests/e2e/support/objects/app-files/spaces/actions.ts @@ -6,6 +6,7 @@ import Collaborator, { ICollaborator } from '../share/collaborator' import { createLink } from '../link/actions' import { File } from '../../../types' import { closeNotifications } from '../../../utils/closeNotifications' +import { isFolderListingResponse } from '../../../utils/folderListing' const newSpaceMenuButton = '.oc-app-floating-action-button' const spaceContextMenuButton = '#space-context-btn' @@ -220,7 +221,7 @@ export const changeSpaceDescription = async (args: { await page.locator(spacesDescriptionInputArea).fill(value) await Promise.all([ page.waitForResponse((resp) => resp.status() === 204 && resp.request().method() === 'PUT'), - page.waitForResponse((resp) => resp.status() === 207 && resp.request().method() === 'PROPFIND'), + page.waitForResponse(isFolderListingResponse), page.locator(spacesDescriptionSaveTextFileInEditorButton).click() ]) await editor.close(page) diff --git a/tests/e2e/support/utils/folderListing.ts b/tests/e2e/support/utils/folderListing.ts new file mode 100644 index 00000000000..280e4fcf18a --- /dev/null +++ b/tests/e2e/support/utils/folderListing.ts @@ -0,0 +1,19 @@ +import { Response } from '@playwright/test' + +/** + * Matches the response that carries a folder listing, whichever API served it: + * a PROPFIND for the places still on webdav (public links, trash bin) and a + * driveItem stat with expanded children for spaces and shares. + */ +export const isFolderListingResponse = (resp: Response): boolean => { + if (resp.request().method() === 'PROPFIND') { + return resp.status() === 207 + } + + return ( + resp.request().method() === 'GET' && + resp.status() === 200 && + /\/graph\/v1\.0\/drives\/[^/]+\/(items|root)/.test(resp.url()) && + resp.url().includes('expand=children') + ) +} diff --git a/tests/e2e/support/utils/index.ts b/tests/e2e/support/utils/index.ts index b69bcac81e9..93afe149ff1 100644 --- a/tests/e2e/support/utils/index.ts +++ b/tests/e2e/support/utils/index.ts @@ -5,3 +5,4 @@ export * from './dragDrop' export * from './datePicker' export * from './tokenHelper' export * from './urlJoin' +export * from './folderListing' From 30b4970fde855531aa01587a497082f0bec2e843 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 09:06:44 +0200 Subject: [PATCH 10/23] test(e2e): don't leave the upload response promise pending An upload that is expected to fail never produces the 201/204 the helper waits for, so the promise stayed pending and rejected once the page closed, failing a scenario that had otherwise passed. --- .../support/objects/app-files/resource/actions.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/e2e/support/objects/app-files/resource/actions.ts b/tests/e2e/support/objects/app-files/resource/actions.ts index e210a478c71..f3d6fbfb021 100644 --- a/tests/e2e/support/objects/app-files/resource/actions.ts +++ b/tests/e2e/support/objects/app-files/resource/actions.ts @@ -772,11 +772,15 @@ const performUpload = async (args: uploadResourceArgs): Promise => { await clickResource({ page, path: to, password }) } - const respPromise = page.waitForResponse( - (resp) => - [201, 204].includes(resp.status()) && - ['POST', 'PUT', 'PATCH'].includes(resp.request().method()) - ) + // an upload that is expected to fail never produces this response, and a + // promise left pending rejects once the page closes + const respPromise = expectToFail + ? null + : page.waitForResponse( + (resp) => + [201, 204].includes(resp.status()) && + ['POST', 'PUT', 'PATCH'].includes(resp.request().method()) + ) const inputSelector = type === 'folder' ? folderUploadInput : fileUploadInput let uploadAction: Promise = page From 12ed3e2af8e327a2506a77d3217bee26b5eb81c1 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 09:06:44 +0200 Subject: [PATCH 11/23] test(e2e): authenticate basic auth requests with the username --- tests/e2e/support/api/http.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/e2e/support/api/http.ts b/tests/e2e/support/api/http.ts index 955b0ee7e4f..6755654ca33 100644 --- a/tests/e2e/support/api/http.ts +++ b/tests/e2e/support/api/http.ts @@ -7,7 +7,7 @@ import { TokenEnvironmentFactory } from '../environment' export const getAuthHeader = (user: User, isKeycloakRequest: boolean = false) => { const tokenEnvironment = TokenEnvironmentFactory(isKeycloakRequest ? 'keycloak' : null) const authHeader = { - Authorization: 'Basic ' + Buffer.from(user.id + ':' + user.password).toString('base64') + Authorization: 'Basic ' + Buffer.from(user.username + ':' + user.password).toString('base64') } if (!appConfig.basicAuth) { From 5f4377e0e6dce9543f83f39fb4e96c54c4aa7665 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 09:22:11 +0200 Subject: [PATCH 12/23] feat: list public links through graph as well A public link is a drive of its own, the client builds its id from the link token. With that the listing has no webdav branch left: the loader always goes through graph, and the dav properties that only the public link needed go with it. The link token and password now ride along on the graph client too, they were only set on the webdav one. --- .../src/helpers/space/graphDrive.ts | 16 ++++++ .../web-client/src/helpers/space/index.ts | 1 + .../web-pkg/src/services/client/client.ts | 51 +++++++++++-------- .../services/folder/loaders/graphListing.ts | 3 +- .../services/folder/loaders/loaderSpace.ts | 12 +---- 5 files changed, 51 insertions(+), 32 deletions(-) create mode 100644 packages/web-client/src/helpers/space/graphDrive.ts diff --git a/packages/web-client/src/helpers/space/graphDrive.ts b/packages/web-client/src/helpers/space/graphDrive.ts new file mode 100644 index 00000000000..7877df1a045 --- /dev/null +++ b/packages/web-client/src/helpers/space/graphDrive.ts @@ -0,0 +1,16 @@ +import { isPublicSpaceResource, SpaceResource } from './types' + +// reva's PublicStorageProviderID: every public link lives in this one mountpoint +// space, the link token is the item below it +const publicStorageProviderId = '7993447f-687f-490d-875c-ac95e89a62a4' + +/** + * The graph drive a space is addressed by. For a public link the client builds + * it from the link token, everywhere else the space id is the drive id. + */ +export const graphDriveIdOfSpace = (space: SpaceResource): string => { + if (isPublicSpaceResource(space)) { + return `${publicStorageProviderId}$${publicStorageProviderId}!${space.id}` + } + return space.id.toString() +} diff --git a/packages/web-client/src/helpers/space/index.ts b/packages/web-client/src/helpers/space/index.ts index ab6b35419db..6c1abea489c 100644 --- a/packages/web-client/src/helpers/space/index.ts +++ b/packages/web-client/src/helpers/space/index.ts @@ -1,2 +1,3 @@ export * from './functions' export * from './types' +export * from './graphDrive' diff --git a/packages/web-pkg/src/services/client/client.ts b/packages/web-pkg/src/services/client/client.ts index 519d2e1b4fa..636a72e9a05 100644 --- a/packages/web-pkg/src/services/client/client.ts +++ b/packages/web-pkg/src/services/client/client.ts @@ -11,6 +11,7 @@ import { Language } from 'vue3-gettext' import { FetchEventSourceInit } from '@microsoft/fetch-event-source' import { sse } from '@opencloud-eu/web-client/sse' import { AuthStore, ConfigStore } from '../../composables' +import { createGraphWebDav } from './graphWebDav' import { createVaultWebDav } from './vaultWebDav' const createFetchOptions = (authParams: AuthParameters, language: string): FetchEventSourceInit => { @@ -121,7 +122,7 @@ export class ClientService { private initGraphClient() { const axiosClient = axios.create({ headers: this.staticHeaders }) axiosClient.interceptors.request.use((config) => { - Object.assign(config.headers, this.getDynamicHeaders()) + Object.assign(config.headers, this.getDynamicHeaders(), this.getPublicLinkHeaders()) return config }) this.graphClient = graph(this.configStore.serverUrl, axiosClient) @@ -146,31 +147,41 @@ export class ClientService { } private initWebDavClient() { - const client = webdav(this.configStore.serverUrl, () => { - const headers = { ...this.staticHeaders, ...this.getDynamicHeaders() } - - if (this.authStore.publicLinkToken) { - headers['public-token'] = this.authStore.publicLinkToken - } - - if (this.authStore.publicLinkPassword) { - headers['Authorization'] = - 'Basic ' + - Buffer.from(['public', this.authStore.publicLinkPassword].join(':')).toString('base64') - } - - return headers - }) - // Wrap the raw client so vault path/name translation happens - // transparently for every caller (clear-text in, clear-text out). It's a - // strict pass-through for any path that isn't inside a vault. - this.webDavClient = createVaultWebDav(client) + const client = webdav(this.configStore.serverUrl, () => ({ + ...this.staticHeaders, + ...this.getDynamicHeaders(), + ...this.getPublicLinkHeaders() + })) + // Two wrappers, outside in: vault translation hands clear-text paths in and + // clear-text names out, below it the graph layer answers what graph can + // answer. The vault layer stays outermost so the graph requests carry the + // encrypted names the server stores. + this.webDavClient = createVaultWebDav(createGraphWebDav(client, () => this.graphClient)) } /** * Dynamic headers that should be provided via callback or interceptor because they may * change during the lifetime of the application (e.g. token renewal). */ + // A public link session has no access token: the link token identifies it and + // a link password rides along as basic auth. Graph needs them just like webdav + // does, public links are listed through graph as well. + private getPublicLinkHeaders(): Record { + const headers: Record = {} + + if (this.authStore.publicLinkToken) { + headers['public-token'] = this.authStore.publicLinkToken + } + + if (this.authStore.publicLinkPassword) { + headers['Authorization'] = + 'Basic ' + + Buffer.from(['public', this.authStore.publicLinkPassword].join(':')).toString('base64') + } + + return headers + } + private getDynamicHeaders({ useAuth = true }: { useAuth?: boolean } = {}): Record< string, string diff --git a/packages/web-pkg/src/services/folder/loaders/graphListing.ts b/packages/web-pkg/src/services/folder/loaders/graphListing.ts index 2d64b1441cb..40914eeb3bb 100644 --- a/packages/web-pkg/src/services/folder/loaders/graphListing.ts +++ b/packages/web-pkg/src/services/folder/loaders/graphListing.ts @@ -1,6 +1,7 @@ import { buildResourceFromDriveItem, buildResourcesFromDriveItems, + graphDriveIdOfSpace, isShareSpaceResource, SpaceResource, urlJoin @@ -56,7 +57,7 @@ export const listFilesViaGraph = async ({ fileId: string signal: AbortSignal }) => { - const driveId = space.id.toString() + const driveId = graphDriveIdOfSpace(space) // graph has no path lookup for the drive root, it is addressed by its id const isRoot = !path || path === '/' const itemId = fileId || (isRoot ? space.root?.id : undefined) diff --git a/packages/web-pkg/src/services/folder/loaders/loaderSpace.ts b/packages/web-pkg/src/services/folder/loaders/loaderSpace.ts index b03bed101a3..a855ee2c026 100644 --- a/packages/web-pkg/src/services/folder/loaders/loaderSpace.ts +++ b/packages/web-pkg/src/services/folder/loaders/loaderSpace.ts @@ -17,7 +17,6 @@ import { DriveItem } from '@opencloud-eu/web-client/graph/generated' import { isLocationSpacesActive, isLocationPublicActive } from '../../../router' import { getSharedDriveItem, setCurrentUserShareSpacePermissions } from '../../../helpers' import { useFileRouteReplace } from '../../../composables' -import { DavProperties, DavProperty } from '@opencloud-eu/web-client/webdav' export class FolderLoaderSpace implements FolderLoader { public isEnabled(): boolean { @@ -59,18 +58,9 @@ export class FolderLoaderSpace implements FolderLoader { try { resourcesStore.clearResourceList() - const davProperties = DavProperties.Default - if (isPublicSpaceResource(space)) { - // needed for public links for make previews work - davProperties.push(DavProperty.DownloadURL) - } - // eslint-disable-next-line prefer-const let { resource: currentFolder, children: resources } = yield* call( - // public links have no drive, they are only reachable over webdav - isPublicSpaceResource(space) - ? webdav.listFiles(space, { path, fileId }, { signal: signal1, davProperties }) - : listFilesViaGraph({ graphClient, space, path, fileId, signal: signal1 }) + listFilesViaGraph({ graphClient, space, path, fileId, signal: signal1 }) ) // if current folder has no id (= singe file public link) we must not correct the route From ad892b88a99f975d11cb7280583d8d61dd3a3f5c Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 09:22:20 +0200 Subject: [PATCH 13/23] feat(web-pkg): stat single items through graph getFileInfo was a PROPFIND with depth 0, which is what a driveItem stat is. A decorator under the vault one answers it through graph now, so all of its callers keep their signature and get permissions, share types, the download url and the preview information in the same request. The vault wrapper stays outermost: the graph request has to carry the encrypted names the server stores, and its answer is decrypted on the way back. --- .../src/services/client/graphWebDav.ts | 58 ++++++++++++++ .../unit/services/client/graphWebDav.spec.ts | 80 +++++++++++++++++++ 2 files changed, 138 insertions(+) create mode 100644 packages/web-pkg/src/services/client/graphWebDav.ts create mode 100644 packages/web-pkg/tests/unit/services/client/graphWebDav.spec.ts diff --git a/packages/web-pkg/src/services/client/graphWebDav.ts b/packages/web-pkg/src/services/client/graphWebDav.ts new file mode 100644 index 00000000000..fc348fd8f1d --- /dev/null +++ b/packages/web-pkg/src/services/client/graphWebDav.ts @@ -0,0 +1,58 @@ +import { buildResourceFromDriveItem, graphDriveIdOfSpace, urlJoin } from '@opencloud-eu/web-client' +import { Resource, SpaceResource } from '@opencloud-eu/web-client' +import { WebDAV } from '@opencloud-eu/web-client/webdav' +import { Graph } from '@opencloud-eu/web-client/graph' +import { + GetDriveItemV1ExpandEnum, + GetDriveItemV1SelectEnum +} from '@opencloud-eu/web-client/graph/generated' + +const statSelect = new Set([ + '@libre.graph.permissions.actions.allowedValues', + '@libre.graph.shareTypes', + '@microsoft.graph.downloadUrl' +]) +const statExpand = new Set(['thumbnails']) + +/** + * Wrap a WebDAV client so a single stat goes through graph instead of a + * PROPFIND with depth 0. Callers keep using `clientService.webdav.getFileInfo` + * and get the same Resource back, whichever API answered. + * + * Everything a stat can be addressed by works: an item id, a path (through + * graph's colon syntax) and a public link, which is a drive of its own built + * from the link token. + */ +export function createGraphWebDav(inner: WebDAV, graphClient: () => Graph): WebDAV { + return { + ...inner, + + async getFileInfo(space, resource = {}, options): Promise { + const driveItem = await graphClient().driveItems.statDriveItem( + graphDriveIdOfSpace(space), + resource.fileId ? { itemId: resource.fileId } : { path: resource.path || '/' }, + { select: statSelect, expand: statExpand }, + { signal: options?.signal } + ) + + return buildResourceFromDriveItem(driveItem, space, '', pathOf(driveItem, space, resource)) + } + } +} + +// The item carries its path in drive coordinates, which is what the caller +// asked for everywhere except a share space: that one is rooted at the shared +// item, so the requested path is the one relative to it. +const pathOf = ( + driveItem: { name?: string; parentReference?: { path?: string } }, + space: SpaceResource, + resource: { path?: string } +) => { + if (resource.path) { + return resource.path + } + const parentPath = driveItem.parentReference?.path + return !parentPath || parentPath === '.' + ? '/' + : urlJoin(parentPath, driveItem.name, { leadingSlash: true }) +} diff --git a/packages/web-pkg/tests/unit/services/client/graphWebDav.spec.ts b/packages/web-pkg/tests/unit/services/client/graphWebDav.spec.ts new file mode 100644 index 00000000000..3f944a15ed9 --- /dev/null +++ b/packages/web-pkg/tests/unit/services/client/graphWebDav.spec.ts @@ -0,0 +1,80 @@ +import { SpaceResource } from '@opencloud-eu/web-client' +import { WebDAV } from '@opencloud-eu/web-client/webdav' +import { Graph } from '@opencloud-eu/web-client/graph' +import { DriveItem } from '@opencloud-eu/web-client/graph/generated' +import { createGraphWebDav } from '../../../../src/services/client/graphWebDav' + +const space = { + id: 'storage$space', + webDavPath: '/dav/spaces/storage$space', + driveType: 'personal' +} as unknown as SpaceResource + +const file = { + id: 'storage$space!item', + name: 'lorem.txt', + file: { mimeType: 'text/plain' }, + parentReference: { path: '/Documents' } +} as DriveItem + +function getDav(driveItem: DriveItem = file) { + const statDriveItem = vi.fn().mockResolvedValue(driveItem) + const inner = { getFileInfo: vi.fn() } as unknown as WebDAV + const dav = createGraphWebDav( + inner, + () => ({ driveItems: { statDriveItem } }) as unknown as Graph + ) + return { dav, statDriveItem, inner } +} + +describe('createGraphWebDav', () => { + it('stats by id', async () => { + const { dav, statDriveItem, inner } = getDav() + + const resource = await dav.getFileInfo(space, { fileId: 'storage$space!item' }) + + expect(statDriveItem.mock.calls[0][0]).toBe('storage$space') + expect(statDriveItem.mock.calls[0][1]).toEqual({ itemId: 'storage$space!item' }) + expect(inner.getFileInfo).not.toHaveBeenCalled() + expect(resource.name).toBe('lorem.txt') + expect(resource.mimeType).toBe('text/plain') + }) + + it('stats by path and keeps the requested one', async () => { + const { dav, statDriveItem } = getDav() + + const resource = await dav.getFileInfo(space, { path: '/Documents/lorem.txt' }) + + expect(statDriveItem.mock.calls[0][1]).toEqual({ path: '/Documents/lorem.txt' }) + expect(resource.path).toBe('/Documents/lorem.txt') + }) + + it('derives the path from the item when only an id was given', async () => { + const { dav } = getDav() + + const resource = await dav.getFileInfo(space, { fileId: 'storage$space!item' }) + + expect(resource.path).toBe('/Documents/lorem.txt') + }) + + it('addresses a public link by the drive built from its token', async () => { + const { dav, statDriveItem } = getDav() + const publicSpace = { ...space, id: 'sometoken', driveType: 'public' } as unknown as SpaceResource + + await dav.getFileInfo(publicSpace, { path: '/' }) + + expect(statDriveItem.mock.calls[0][0]).toBe( + '7993447f-687f-490d-875c-ac95e89a62a4$7993447f-687f-490d-875c-ac95e89a62a4!sometoken' + ) + }) + + it('asks for the previews and the download url', async () => { + const { dav, statDriveItem } = getDav() + + await dav.getFileInfo(space, { fileId: 'storage$space!item' }) + + const options = statDriveItem.mock.calls[0][2] + expect(options.expand).toEqual(new Set(['thumbnails'])) + expect(options.select).toContain('@microsoft.graph.downloadUrl') + }) +}) From 5b162e8cf5033d3e729a8abdccb09486342131da Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 09:27:17 +0200 Subject: [PATCH 14/23] refactor(web-pkg): stat the breadcrumb ancestors through graph loadAncestorMetaData asked webdav for each ancestor with a depth 0 PROPFIND and a hand picked property list. getFileInfo answers the same thing through graph now, so the ancestors ride along and the dav properties go away. Still one request per level, graph has no ancestor endpoint. --- .../src/composables/piniaStores/resources.ts | 23 ++++---- .../composables/piniaStores/resources.spec.ts | 56 ++++++++++++++++++- 2 files changed, 65 insertions(+), 14 deletions(-) diff --git a/packages/web-pkg/src/composables/piniaStores/resources.ts b/packages/web-pkg/src/composables/piniaStores/resources.ts index c6f008a5eee..c16b97aaea1 100644 --- a/packages/web-pkg/src/composables/piniaStores/resources.ts +++ b/packages/web-pkg/src/composables/piniaStores/resources.ts @@ -3,7 +3,7 @@ import { Ref, computed, ref, unref } from 'vue' import { isProjectSpaceResource, SpaceResource, type Resource } from '@opencloud-eu/web-client' import { getParentPaths } from '../../helpers' import { AncestorMetaData, AncestorMetaDataValue } from '../../types' -import { DavProperty, WebDAV } from '@opencloud-eu/web-client/webdav' +import { WebDAV } from '@opencloud-eu/web-client/webdav' import { useSpacesStore } from './spaces' import { eventBus, releaseFilePreviews } from '../../services' @@ -225,7 +225,6 @@ export const useResourcesStore = defineStore('resources', () => { } } const promises = [] - const davProperties = [DavProperty.FileId, DavProperty.ShareTypes, DavProperty.FileParent] const parentPaths = getParentPaths(folder.path) for (const path of parentPaths) { @@ -236,17 +235,15 @@ export const useResourcesStore = defineStore('resources', () => { } promises.push( - client - .listFiles(space, { path }, { depth: 0, davProperties, signal }) - .then(({ resource }) => { - data[path] = { - id: resource.fileId, - shareTypes: resource.shareTypes, - parentFolderId: resource.parentFolderId, - spaceId: space.id, - path - } - }) + client.getFileInfo(space, { path }, { signal }).then((resource) => { + data[path] = { + id: resource.fileId, + shareTypes: resource.shareTypes, + parentFolderId: resource.parentFolderId, + spaceId: space.id, + path + } + }) ) } diff --git a/packages/web-pkg/tests/unit/composables/piniaStores/resources.spec.ts b/packages/web-pkg/tests/unit/composables/piniaStores/resources.spec.ts index 2f54d43b2ed..d6ec39286d6 100644 --- a/packages/web-pkg/tests/unit/composables/piniaStores/resources.spec.ts +++ b/packages/web-pkg/tests/unit/composables/piniaStores/resources.spec.ts @@ -1,6 +1,7 @@ import { createPinia, setActivePinia } from 'pinia' import { mock } from 'vitest-mock-extended' -import { Resource } from '@opencloud-eu/web-client' +import { Resource, SpaceResource } from '@opencloud-eu/web-client' +import { WebDAV } from '@opencloud-eu/web-client/webdav' import { useResourcesStore } from '../../../../src/composables/piniaStores/resources' import { buildFilePreviewCacheKey, cacheService } from '../../../../src/services' @@ -10,6 +11,59 @@ describe('useResourcesStore', () => { cacheService.filePreview.clear() }) + describe('loadAncestorMetaData', () => { + const space = mock({ id: 'storage$space' }) + + const getClient = () => + mock({ + getFileInfo: vi.fn().mockImplementation((_space, { path }) => + Promise.resolve( + mock({ + fileId: `id-of-${path}`, + parentFolderId: 'parent', + shareTypes: [] + }) + ) + ) + }) + + it('stats every ancestor of the folder', async () => { + const store = useResourcesStore() + const client = getClient() + + await store.loadAncestorMetaData({ + folder: mock({ path: '/a/b/c', fileId: 'id-of-/a/b/c' }), + space, + client + }) + + const statted = vi.mocked(client.getFileInfo).mock.calls.map(([, ref]) => ref.path) + // the root is filled in from the space, not statted + expect(statted).toEqual(['/a/b', '/a']) + expect(store.ancestorMetaData['/a/b'].id).toBe('id-of-/a/b') + expect(store.ancestorMetaData['/a/b/c'].id).toBe('id-of-/a/b/c') + }) + + it('reuses what it already knows about the same space', async () => { + const store = useResourcesStore() + await store.loadAncestorMetaData({ + folder: mock({ path: '/a/b', fileId: 'id-of-/a/b' }), + space, + client: getClient() + }) + + const client = getClient() + await store.loadAncestorMetaData({ + folder: mock({ path: '/a/b/c', fileId: 'id-of-/a/b/c' }), + space, + client + }) + + const statted = vi.mocked(client.getFileInfo).mock.calls.map(([, ref]) => ref.path) + expect(statted).toEqual([]) + }) + }) + describe('preview releasing', () => { const file = mock({ id: '1', name: 'file.png', type: 'file' }) From be2aab65342a42d77c620ea3a7e5f4979b79e371 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 10:08:10 +0200 Subject: [PATCH 15/23] fix(web-pkg): carry the graph error shape over to the callers A failing stat threw a raw axios error, so callers lost the status code they branch on, and a public link never showed its password prompt: that one hangs on telling "needs a password" from "wrong password" apart. Graph carries both as an error code, publicLinkPasswordRequired and publicLinkPasswordInvalid, which map onto the dav codes the callers already know. --- .../src/services/client/graphWebDav.ts | 50 +++++++++++++++---- .../unit/services/client/graphWebDav.spec.ts | 24 ++++++++- 2 files changed, 63 insertions(+), 11 deletions(-) diff --git a/packages/web-pkg/src/services/client/graphWebDav.ts b/packages/web-pkg/src/services/client/graphWebDav.ts index fc348fd8f1d..20ad431ded4 100644 --- a/packages/web-pkg/src/services/client/graphWebDav.ts +++ b/packages/web-pkg/src/services/client/graphWebDav.ts @@ -1,5 +1,11 @@ -import { buildResourceFromDriveItem, graphDriveIdOfSpace, urlJoin } from '@opencloud-eu/web-client' -import { Resource, SpaceResource } from '@opencloud-eu/web-client' +import { + buildResourceFromDriveItem, + DavHttpError, + graphDriveIdOfSpace, + Resource, + SpaceResource, + urlJoin +} from '@opencloud-eu/web-client' import { WebDAV } from '@opencloud-eu/web-client/webdav' import { Graph } from '@opencloud-eu/web-client/graph' import { @@ -28,14 +34,18 @@ export function createGraphWebDav(inner: WebDAV, graphClient: () => Graph): WebD ...inner, async getFileInfo(space, resource = {}, options): Promise { - const driveItem = await graphClient().driveItems.statDriveItem( - graphDriveIdOfSpace(space), - resource.fileId ? { itemId: resource.fileId } : { path: resource.path || '/' }, - { select: statSelect, expand: statExpand }, - { signal: options?.signal } - ) - - return buildResourceFromDriveItem(driveItem, space, '', pathOf(driveItem, space, resource)) + try { + const driveItem = await graphClient().driveItems.statDriveItem( + graphDriveIdOfSpace(space), + resource.fileId ? { itemId: resource.fileId } : { path: resource.path || '/' }, + { select: statSelect, expand: statExpand }, + { signal: options?.signal } + ) + + return buildResourceFromDriveItem(driveItem, space, '', pathOf(driveItem, space, resource)) + } catch (error) { + throw asDavError(error) + } } } } @@ -56,3 +66,23 @@ const pathOf = ( ? '/' : urlJoin(parentPath, driveItem.name, { leadingSlash: true }) } + +// Callers branch on the shape webdav throws: a status code and, for a public +// link, the code that tells "needs a password" from "wrong password" apart. +// Graph carries the same information in its error body. +const asDavError = (error: any) => { + const response = error?.response + if (!response) { + return error + } + + const code = response.data?.error?.code + const message = response.data?.error?.message || error.message + + return new DavHttpError(message, davErrorCodes[code] ?? code, response, response.status) +} + +const davErrorCodes: Record = { + publicLinkPasswordRequired: 'ERR_MISSING_BASIC_AUTH', + publicLinkPasswordInvalid: 'ERR_INVALID_CREDENTIALS' +} diff --git a/packages/web-pkg/tests/unit/services/client/graphWebDav.spec.ts b/packages/web-pkg/tests/unit/services/client/graphWebDav.spec.ts index 3f944a15ed9..1cb836d96a7 100644 --- a/packages/web-pkg/tests/unit/services/client/graphWebDav.spec.ts +++ b/packages/web-pkg/tests/unit/services/client/graphWebDav.spec.ts @@ -59,7 +59,11 @@ describe('createGraphWebDav', () => { it('addresses a public link by the drive built from its token', async () => { const { dav, statDriveItem } = getDav() - const publicSpace = { ...space, id: 'sometoken', driveType: 'public' } as unknown as SpaceResource + const publicSpace = { + ...space, + id: 'sometoken', + driveType: 'public' + } as unknown as SpaceResource await dav.getFileInfo(publicSpace, { path: '/' }) @@ -68,6 +72,24 @@ describe('createGraphWebDav', () => { ) }) + it('maps a needed link password onto the code the callers branch on', async () => { + const statDriveItem = vi.fn().mockRejectedValue({ + response: { + status: 401, + data: { error: { code: 'publicLinkPasswordRequired', message: 'password required' } } + } + }) + const dav = createGraphWebDav( + { getFileInfo: vi.fn() } as unknown as WebDAV, + () => ({ driveItems: { statDriveItem } }) as unknown as Graph + ) + + await expect(dav.getFileInfo(space, { path: '/' })).rejects.toMatchObject({ + statusCode: 401, + errorCode: 'ERR_MISSING_BASIC_AUTH' + }) + }) + it('asks for the previews and the download url', async () => { const { dav, statDriveItem } = getDav() From ef4ce81b1d2dd16972a7ef57cbc47c0234321d94 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 10:10:15 +0200 Subject: [PATCH 16/23] test(e2e): wait for the stat when a file opens in an app Opening a file stats it, and that stat goes through graph now. The helpers match a stat on either api, the listing predicate builds on the same check and additionally looks for the expanded children. --- .../objects/app-files/resource/actions.ts | 18 ++++-------------- tests/e2e/support/utils/folderListing.ts | 19 +++++++++++++------ 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/tests/e2e/support/objects/app-files/resource/actions.ts b/tests/e2e/support/objects/app-files/resource/actions.ts index f3d6fbfb021..ac9db73b5a8 100644 --- a/tests/e2e/support/objects/app-files/resource/actions.ts +++ b/tests/e2e/support/objects/app-files/resource/actions.ts @@ -10,7 +10,7 @@ import { waitProcessingToFinish } from '../fileEvents' import { state } from '../../../../environment/shared' import { lstatSync, readFileSync } from 'fs' import { encodeWebDavPath } from '../../../utils' -import { isFolderListingResponse } from '../../../utils/folderListing' +import { isFolderListingResponse, isResourceStatResponse } from '../../../utils/folderListing' const appLoadingSpinner = '#app-loading-spinner' const topbarFilenameSelector = '#app-top-bar-resource .oc-resource-name' @@ -2225,12 +2225,7 @@ export const openFileInViewer = async (args: openFileInViewerArgs): Promise - resp.status() === 207 && - resp.request().method() === 'PROPFIND' && - resp.url().includes(encodeWebDavPath(name)) - ), + page.waitForResponse(isResourceStatResponse), page.locator(util.format(resourceNameSelector, name)).click() ]) } else { @@ -2255,7 +2250,7 @@ export const openFileInViewer = async (args: openFileInViewerArgs): Promise resp.status() === 207 && resp.request().method() === 'PROPFIND' + isResourceStatResponse ), page.locator(util.format(resourceNameSelector, name)).click() ]) @@ -2264,12 +2259,7 @@ export const openFileInViewer = async (args: openFileInViewerArgs): Promise - resp.status() === 207 && - resp.request().method() === 'PROPFIND' && - (!verifyPropfindPath || resp.url().includes(encodeWebDavPath(name))) - ), + page.waitForResponse(isResourceStatResponse), page.locator(util.format(resourceNameSelector, name)).click() ]) await page.locator(textEditorContainer).waitFor() diff --git a/tests/e2e/support/utils/folderListing.ts b/tests/e2e/support/utils/folderListing.ts index 280e4fcf18a..47e18bead83 100644 --- a/tests/e2e/support/utils/folderListing.ts +++ b/tests/e2e/support/utils/folderListing.ts @@ -1,11 +1,11 @@ import { Response } from '@playwright/test' /** - * Matches the response that carries a folder listing, whichever API served it: - * a PROPFIND for the places still on webdav (public links, trash bin) and a - * driveItem stat with expanded children for spaces and shares. + * Matches the response that carries a resource, whichever API served it: a + * PROPFIND for what is still on webdav (the trash bin) and a driveItem stat + * for everything that moved to graph. */ -export const isFolderListingResponse = (resp: Response): boolean => { +export const isResourceStatResponse = (resp: Response): boolean => { if (resp.request().method() === 'PROPFIND') { return resp.status() === 207 } @@ -13,7 +13,14 @@ export const isFolderListingResponse = (resp: Response): boolean => { return ( resp.request().method() === 'GET' && resp.status() === 200 && - /\/graph\/v1\.0\/drives\/[^/]+\/(items|root)/.test(resp.url()) && - resp.url().includes('expand=children') + /\/graph\/v1\.0\/drives\/[^/]+\/(items|root)/.test(resp.url()) ) } + +/** + * A stat that carries the folder's children, so a listing rather than a single + * resource. + */ +export const isFolderListingResponse = (resp: Response): boolean => + isResourceStatResponse(resp) && + (resp.request().method() === 'PROPFIND' || resp.url().includes('expand=children')) From 7be3bef45d669353675ccf4399bb0e73482bbe5b Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 10:36:50 +0200 Subject: [PATCH 17/23] feat: resolve a public link through graph The link's root stat returns the space the app navigates in, so it carries the link's own properties: the role comes from the actions the server capped at it, the owner from the mountpoint drive. The expiration, the share date and the item type came from dav properties with no graph counterpart, and nothing reads them. The link token rides on the request itself rather than the auth store, the very first stat runs before the store knows about it. Addressing the root is one rule now, shared by the stat and the listing: a root has no path to look up, and for a public link it is the mountpoint drive itself. --- packages/web-client/src/graph/index.ts | 1 + .../src/helpers/space/graphDrive.ts | 19 ++++++ .../web-client/src/helpers/space/index.ts | 1 + .../src/helpers/space/publicLink.ts | 60 +++++++++++++++++++ .../src/services/client/graphWebDav.ts | 53 ++++++++++++++-- .../services/folder/loaders/graphListing.ts | 12 ++-- 6 files changed, 137 insertions(+), 9 deletions(-) create mode 100644 packages/web-client/src/helpers/space/publicLink.ts diff --git a/packages/web-client/src/graph/index.ts b/packages/web-client/src/graph/index.ts index 84a63c5a62e..2e6944dbe5c 100644 --- a/packages/web-client/src/graph/index.ts +++ b/packages/web-client/src/graph/index.ts @@ -5,6 +5,7 @@ import { type GraphGroups, GroupsFactory } from './groups' import { ApplicationsFactory, GraphApplications } from './applications' import { DrivesFactory, GraphDrives } from './drives' import { DriveItemsFactory, GraphDriveItems } from './driveItems' +export type { DriveItemRef, DriveItemStatOptions } from './driveItems' import { TagsFactory, GraphTags } from './tags' import { ActivitiesFactory, GraphActivities } from './activities' import { PermissionsFactory, GraphPermissions } from './permissions' diff --git a/packages/web-client/src/helpers/space/graphDrive.ts b/packages/web-client/src/helpers/space/graphDrive.ts index 7877df1a045..d4fbd4eaae2 100644 --- a/packages/web-client/src/helpers/space/graphDrive.ts +++ b/packages/web-client/src/helpers/space/graphDrive.ts @@ -1,4 +1,5 @@ import { isPublicSpaceResource, SpaceResource } from './types' +import type { DriveItemRef } from '../../graph/driveItems' // reva's PublicStorageProviderID: every public link lives in this one mountpoint // space, the link token is the item below it @@ -14,3 +15,21 @@ export const graphDriveIdOfSpace = (space: SpaceResource): string => { } return space.id.toString() } + +/** + * How graph addresses an item of the space: by id where there is one, by path + * otherwise. A root has no path to look up, it is addressed by its id, and for + * a public link that is the mountpoint drive itself. + */ +export const graphRefOfSpace = ( + space: SpaceResource, + { path, fileId }: { path?: string; fileId?: string } +): DriveItemRef => { + if (fileId) { + return { itemId: fileId } + } + if (!path || path === '/') { + return { itemId: isPublicSpaceResource(space) ? graphDriveIdOfSpace(space) : space.root?.id } + } + return { path } +} diff --git a/packages/web-client/src/helpers/space/index.ts b/packages/web-client/src/helpers/space/index.ts index 6c1abea489c..9d028908fe2 100644 --- a/packages/web-client/src/helpers/space/index.ts +++ b/packages/web-client/src/helpers/space/index.ts @@ -1,3 +1,4 @@ export * from './functions' export * from './types' export * from './graphDrive' +export * from './publicLink' diff --git a/packages/web-client/src/helpers/space/publicLink.ts b/packages/web-client/src/helpers/space/publicLink.ts new file mode 100644 index 00000000000..3b62036f02e --- /dev/null +++ b/packages/web-client/src/helpers/space/publicLink.ts @@ -0,0 +1,60 @@ +import { SharePermissionBit } from '../share/constants' +import { buildPublicSpaceResource } from './functions' +import { PublicSpaceResource, SpaceResource } from './types' +import type { DriveItem } from '../../graph/generated' +import type { Resource } from '../resource' + +// The actions a public link grants, capped at the link role by the server. +const actionToPermissionBit: Record = { + 'libre.graph/driveItem/content/read': SharePermissionBit.Read, + 'libre.graph/driveItem/path/update': SharePermissionBit.Update, + 'libre.graph/driveItem/upload/create': SharePermissionBit.Create, + 'libre.graph/driveItem/children/create': SharePermissionBit.Create, + 'libre.graph/driveItem/standard/delete': SharePermissionBit.Delete, + 'libre.graph/driveItem/permissions/create': SharePermissionBit.Share +} + +/** + * The link role as the permission bits the callers test against. Graph reports + * the role as the actions it allows, there is no permission number on a public + * link item. + */ +export const publicLinkPermissionFromActions = (actions: string[] = []): number => + actions.reduce((bits, action) => bits | (actionToPermissionBit[action] ?? 0), 0) + +/** + * Turn the stat of a public link's root into the space the app works with. The + * counterpart of the PROPFIND based buildPublicSpaceResource, for the graph + * listing. + * + * The link's expiration, its share date and its item type came from dav + * properties that graph has no counterpart for. Nothing reads them. The owner + * comes from the mountpoint drive, which does not carry it yet. + */ +export const buildPublicSpaceResourceFromDriveItem = ({ + driveItem, + resource, + space, + drive +}: { + driveItem: DriveItem + resource: Resource + space: PublicSpaceResource + drive?: SpaceResource +}): PublicSpaceResource => { + const actions = driveItem['@libre.graph.permissions.actions.allowedValues'] + + return Object.assign( + buildPublicSpaceResource({ + ...resource, + id: space.id, + driveAlias: space.driveAlias, + webDavPath: space.webDavPath, + publicLinkType: space.publicLinkType + }), + { + publicLinkPermission: publicLinkPermissionFromActions(actions), + ...(drive?.owner?.displayName && { publicLinkShareOwner: drive.owner.displayName }) + } + ) +} diff --git a/packages/web-pkg/src/services/client/graphWebDav.ts b/packages/web-pkg/src/services/client/graphWebDav.ts index 20ad431ded4..fcc6d9adc67 100644 --- a/packages/web-pkg/src/services/client/graphWebDav.ts +++ b/packages/web-pkg/src/services/client/graphWebDav.ts @@ -1,7 +1,11 @@ import { + buildPublicSpaceResourceFromDriveItem, buildResourceFromDriveItem, DavHttpError, graphDriveIdOfSpace, + graphRefOfSpace, + isPublicSpaceResource, + PublicSpaceResource, Resource, SpaceResource, urlJoin @@ -34,15 +38,41 @@ export function createGraphWebDav(inner: WebDAV, graphClient: () => Graph): WebD ...inner, async getFileInfo(space, resource = {}, options): Promise { + const driveId = graphDriveIdOfSpace(space) + // a public link is identified by its token, and the first stat runs + // before the auth store knows about it: it is what tells the client + // whether the link needs a password at all + const requestOptions = { + signal: options?.signal, + ...(isPublicSpaceResource(space) && { headers: { 'public-token': space.id.toString() } }) + } + try { const driveItem = await graphClient().driveItems.statDriveItem( - graphDriveIdOfSpace(space), - resource.fileId ? { itemId: resource.fileId } : { path: resource.path || '/' }, + driveId, + graphRefOfSpace(space, resource), { select: statSelect, expand: statExpand }, - { signal: options?.signal } + requestOptions ) + const built = buildResourceFromDriveItem( + driveItem, + space, + '', + pathOf(driveItem, space, resource) + ) + + // the root of a public link is the space the app navigates in, so it + // carries the link's own properties rather than being a plain resource + if (isPublicSpaceResource(space) && !resource.fileId && !resource.path) { + return buildPublicSpaceResourceFromDriveItem({ + driveItem, + resource: built, + space: space as PublicSpaceResource, + drive: await publicLinkDrive(graphClient, driveId, requestOptions) + }) + } - return buildResourceFromDriveItem(driveItem, space, '', pathOf(driveItem, space, resource)) + return built } catch (error) { throw asDavError(error) } @@ -50,6 +80,21 @@ export function createGraphWebDav(inner: WebDAV, graphClient: () => Graph): WebD } } +// The mountpoint drive of a public link carries its owner. Failing to read it +// costs the owner's name on the drop upload page, nothing else, so a link that +// still works stays usable. +const publicLinkDrive = async ( + graphClient: () => Graph, + driveId: string, + requestOptions: Record +) => { + try { + return await graphClient().drives.getDrive(driveId, undefined, requestOptions) + } catch { + return undefined + } +} + // The item carries its path in drive coordinates, which is what the caller // asked for everywhere except a share space: that one is rooted at the shared // item, so the requested path is the one relative to it. diff --git a/packages/web-pkg/src/services/folder/loaders/graphListing.ts b/packages/web-pkg/src/services/folder/loaders/graphListing.ts index 40914eeb3bb..928c59a3b06 100644 --- a/packages/web-pkg/src/services/folder/loaders/graphListing.ts +++ b/packages/web-pkg/src/services/folder/loaders/graphListing.ts @@ -2,6 +2,8 @@ import { buildResourceFromDriveItem, buildResourcesFromDriveItems, graphDriveIdOfSpace, + graphRefOfSpace, + isPublicSpaceResource, isShareSpaceResource, SpaceResource, urlJoin @@ -58,17 +60,17 @@ export const listFilesViaGraph = async ({ signal: AbortSignal }) => { const driveId = graphDriveIdOfSpace(space) - // graph has no path lookup for the drive root, it is addressed by its id - const isRoot = !path || path === '/' - const itemId = fileId || (isRoot ? space.root?.id : undefined) const registry = useExtensionRegistry() // inside a vault the server knows the encrypted names only const serverPath = await toVaultServerPath(registry, space, path) const driveItem = await graphClient.driveItems.statDriveItem( driveId, - itemId ? { itemId } : { path: serverPath }, + graphRefOfSpace(space, { path: serverPath, fileId }), { select: graphListingSelect, expand: graphListingExpand }, - { signal } + { + signal, + ...(isPublicSpaceResource(space) && { headers: { 'public-token': space.id.toString() } }) + } ) const currentPath = currentPathOf(driveItem, space, path) From 80a2c9ddfbf08e5f2fb1a4226d64bd1c345e88ba Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 10:50:45 +0200 Subject: [PATCH 18/23] fix: tell a single file link apart by the item behind it The link root carries its type: a folder for a link to a folder, a file for a link to a single file. Dav could not say, it reported "folder" for both, so the presence of a file id had to stand in for the distinction. That stand-in broke when the link started resolving through graph, where the id comes from the item rather than from a dav property: every folder link looked like a single file link and opened its first child instead of listing. --- packages/web-client/src/helpers/space/publicLink.ts | 10 +++++++--- packages/web-runtime/src/pages/resolvePublicLink.vue | 11 ++++------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/packages/web-client/src/helpers/space/publicLink.ts b/packages/web-client/src/helpers/space/publicLink.ts index 3b62036f02e..45195b171a6 100644 --- a/packages/web-client/src/helpers/space/publicLink.ts +++ b/packages/web-client/src/helpers/space/publicLink.ts @@ -27,9 +27,9 @@ export const publicLinkPermissionFromActions = (actions: string[] = []): number * counterpart of the PROPFIND based buildPublicSpaceResource, for the graph * listing. * - * The link's expiration, its share date and its item type came from dav - * properties that graph has no counterpart for. Nothing reads them. The owner - * comes from the mountpoint drive, which does not carry it yet. + * The link's expiration and its share date came from dav properties that graph + * has no counterpart for. Nothing reads them. The owner comes from the + * mountpoint drive. */ export const buildPublicSpaceResourceFromDriveItem = ({ driveItem, @@ -54,6 +54,10 @@ export const buildPublicSpaceResourceFromDriveItem = ({ }), { publicLinkPermission: publicLinkPermissionFromActions(actions), + // the item behind the link, which dav could not tell apart: it reported + // "folder" for a link to a single file as well + publicLinkItemType: driveItem.folder ? 'folder' : 'file', + fileId: driveItem.id, ...(drive?.owner?.displayName && { publicLinkShareOwner: drive.owner.displayName }) } ) diff --git a/packages/web-runtime/src/pages/resolvePublicLink.vue b/packages/web-runtime/src/pages/resolvePublicLink.vue index ec65192bb77..72c9880597b 100644 --- a/packages/web-runtime/src/pages/resolvePublicLink.vue +++ b/packages/web-runtime/src/pages/resolvePublicLink.vue @@ -217,14 +217,11 @@ const resolvePublicLinkTask = useTask(function* (signal, passwordRequired: boole }) /** - * A public link to a single file has no file id of its own, while a link to a folder does. - * The `public-link-item-type` dav property can't be used here, the server reports "folder" - * in both cases. + * The item type of the link root, which graph reports for what it is. Dav could + * not: it answered "folder" for a link to a single file as well, so the file id + * had to stand in for the distinction. */ -const isSingleFileLink = computed(() => { - const space = unref(loadedSpace) - return !space.fileId || space.fileId === space.id -}) +const isSingleFileLink = computed(() => unref(loadedSpace).publicLinkItemType === 'file') /** * For a public link pointing to a single file, the link root is the file itself. Since the root From 57c9c6cc29537ab2381c55f639f43bd4202aed58 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 11:00:48 +0200 Subject: [PATCH 19/23] feat(web-pkg): stat and list through graph for every caller listFiles joins getFileInfo in the graph decorator, so the remaining callers move over too: upload conflict checks, the rename parent listing, save-as, the space duplicate and the app folder handling. getPathForFileId follows, the item knows where it sits. The listing itself lost its vault translation, the decorator above does that for every method, and the loader calls listFiles again like any other caller. What is left on webdav is what graph cannot answer: the trash bin and the file versions. --- .../loaders => client}/graphListing.ts | 43 +++--- .../src/services/client/graphWebDav.ts | 133 ++++++++---------- .../services/folder/loaders/loaderSpace.ts | 3 +- .../{folder => client}/graphListing.spec.ts | 69 +-------- 4 files changed, 82 insertions(+), 166 deletions(-) rename packages/web-pkg/src/services/{folder/loaders => client}/graphListing.ts (64%) rename packages/web-pkg/tests/unit/services/{folder => client}/graphListing.spec.ts (60%) diff --git a/packages/web-pkg/src/services/folder/loaders/graphListing.ts b/packages/web-pkg/src/services/client/graphListing.ts similarity index 64% rename from packages/web-pkg/src/services/folder/loaders/graphListing.ts rename to packages/web-pkg/src/services/client/graphListing.ts index 928c59a3b06..c9b4d0c7c86 100644 --- a/packages/web-pkg/src/services/folder/loaders/graphListing.ts +++ b/packages/web-pkg/src/services/client/graphListing.ts @@ -14,18 +14,16 @@ import { GetDriveItemV1ExpandEnum, GetDriveItemV1SelectEnum } from '@opencloud-eu/web-client/graph/generated' -// the specific store / helper modules, not the barrels: this file sits in the -// services layer and re-entering those barrels creates an evaluation cycle -import { useExtensionRegistry } from '../../../composables/piniaStores/extensionRegistry' -import { applyVaultFromServer, toVaultServerPath } from '../../../helpers/vaultTranslate' - const graphListingSelect = new Set([ '@libre.graph.permissions.actions.allowedValues', - '@libre.graph.shareTypes' + '@libre.graph.shareTypes', + // the callers that used to ask for the DownloadURL dav property + '@microsoft.graph.downloadUrl' ]) // thumbnails answer whether an item has a preview, for the folder and its // children alike, which saves the client from guessing by mime type const graphListingExpand = new Set(['children', 'thumbnails']) +const graphThumbnailsExpand = new Set(['thumbnails']) // A share space is rooted at the shared item, but graph answers with paths in // the owner's drive: the stat of a received share reports the share root as @@ -44,29 +42,30 @@ const currentPathOf = (driveItem: DriveItem, space: SpaceResource, path: string) // listFilesViaGraph lists a folder through graph, folder and children in one // request via $expand=children, the same shape PROPFIND with Depth: 1 returns. -// Lives next to the loader rather than inside it so it can be tested without -// importing the loader, which pulls the folderService singleton along. +// Vault translation happens in the decorator above, this is the plain listing. export const listFilesViaGraph = async ({ graphClient, space, path, fileId, - signal + signal, + withChildren = true }: { graphClient: Graph space: SpaceResource - path: string - fileId: string - signal: AbortSignal + path?: string + fileId?: string + signal?: AbortSignal + withChildren?: boolean }) => { const driveId = graphDriveIdOfSpace(space) - const registry = useExtensionRegistry() - // inside a vault the server knows the encrypted names only - const serverPath = await toVaultServerPath(registry, space, path) const driveItem = await graphClient.driveItems.statDriveItem( driveId, - graphRefOfSpace(space, { path: serverPath, fileId }), - { select: graphListingSelect, expand: graphListingExpand }, + graphRefOfSpace(space, { path, fileId }), + { + select: graphListingSelect, + expand: withChildren ? graphListingExpand : graphThumbnailsExpand + }, { signal, ...(isPublicSpaceResource(space) && { headers: { 'public-token': space.id.toString() } }) @@ -75,10 +74,10 @@ export const listFilesViaGraph = async ({ const currentPath = currentPathOf(driveItem, space, path) const currentFolder = buildResourceFromDriveItem(driveItem, space, '', currentPath) - const children = buildResourcesFromDriveItems(driveItem.children || [], space, currentFolder.path) - // the webdav client has its vault decorator, the graph path translates here - await applyVaultFromServer(registry, space, [currentFolder, ...children]) - - return { resource: currentFolder, children } + return { + driveItem, + resource: currentFolder, + children: buildResourcesFromDriveItems(driveItem.children || [], space, currentFolder.path) + } } diff --git a/packages/web-pkg/src/services/client/graphWebDav.ts b/packages/web-pkg/src/services/client/graphWebDav.ts index fcc6d9adc67..2eb488e5bdf 100644 --- a/packages/web-pkg/src/services/client/graphWebDav.ts +++ b/packages/web-pkg/src/services/client/graphWebDav.ts @@ -1,78 +1,84 @@ import { buildPublicSpaceResourceFromDriveItem, - buildResourceFromDriveItem, DavHttpError, graphDriveIdOfSpace, - graphRefOfSpace, isPublicSpaceResource, PublicSpaceResource, Resource, - SpaceResource, urlJoin } from '@opencloud-eu/web-client' -import { WebDAV } from '@opencloud-eu/web-client/webdav' +import { ListFilesResult, WebDAV } from '@opencloud-eu/web-client/webdav' import { Graph } from '@opencloud-eu/web-client/graph' -import { - GetDriveItemV1ExpandEnum, - GetDriveItemV1SelectEnum -} from '@opencloud-eu/web-client/graph/generated' - -const statSelect = new Set([ - '@libre.graph.permissions.actions.allowedValues', - '@libre.graph.shareTypes', - '@microsoft.graph.downloadUrl' -]) -const statExpand = new Set(['thumbnails']) +import { listFilesViaGraph } from './graphListing' /** - * Wrap a WebDAV client so a single stat goes through graph instead of a - * PROPFIND with depth 0. Callers keep using `clientService.webdav.getFileInfo` - * and get the same Resource back, whichever API answered. + * Wrap a WebDAV client so everything that only reads metadata goes through + * graph instead of a PROPFIND. Callers keep using `clientService.webdav` and + * get the same shapes back, whichever API answered. * - * Everything a stat can be addressed by works: an item id, a path (through - * graph's colon syntax) and a public link, which is a drive of its own built - * from the link token. + * What stays on webdav is what graph has no answer for: the trash bin, which + * has no listing, and the file versions, which have no endpoint. */ export function createGraphWebDav(inner: WebDAV, graphClient: () => Graph): WebDAV { + const listFiles: WebDAV['listFiles'] = async (space, { path, fileId } = {}, options = {}) => { + if (options.isTrash) { + return inner.listFiles(space, { path, fileId }, options) + } + + try { + const { driveItem, resource, children } = await listFilesViaGraph({ + graphClient: graphClient(), + space, + path, + fileId, + signal: options.signal, + withChildren: options.depth !== 0 + }) + + // the root of a public link is the space the app navigates in, so it + // carries the link's own properties rather than being a plain resource + if (isPublicSpaceResource(space) && !fileId && (!path || path === '/')) { + return { + resource: buildPublicSpaceResourceFromDriveItem({ + driveItem, + resource, + space: space as PublicSpaceResource, + drive: await publicLinkDrive(graphClient, graphDriveIdOfSpace(space), options.signal) + }), + children + } as ListFilesResult + } + + return { resource, children } + } catch (error) { + throw asDavError(error) + } + } + return { ...inner, + listFiles, + async getFileInfo(space, resource = {}, options): Promise { - const driveId = graphDriveIdOfSpace(space) - // a public link is identified by its token, and the first stat runs - // before the auth store knows about it: it is what tells the client - // whether the link needs a password at all - const requestOptions = { - signal: options?.signal, - ...(isPublicSpaceResource(space) && { headers: { 'public-token': space.id.toString() } }) - } + return (await listFiles(space, resource, { ...options, depth: 0 })).resource + }, + async getPathForFileId(id, options) { try { + // the item knows where it sits, and the drive it sits in is the first + // part of its own id const driveItem = await graphClient().driveItems.statDriveItem( - driveId, - graphRefOfSpace(space, resource), - { select: statSelect, expand: statExpand }, - requestOptions + id.split('!')[0], + { itemId: id }, + {}, + options ) - const built = buildResourceFromDriveItem( - driveItem, - space, - '', - pathOf(driveItem, space, resource) - ) - - // the root of a public link is the space the app navigates in, so it - // carries the link's own properties rather than being a plain resource - if (isPublicSpaceResource(space) && !resource.fileId && !resource.path) { - return buildPublicSpaceResourceFromDriveItem({ - driveItem, - resource: built, - space: space as PublicSpaceResource, - drive: await publicLinkDrive(graphClient, driveId, requestOptions) - }) - } + const parentPath = driveItem.parentReference?.path - return built + return !parentPath || parentPath === '.' + ? urlJoin(driveItem.name, { leadingSlash: true }) + : urlJoin(parentPath, driveItem.name, { leadingSlash: true }) } catch (error) { throw asDavError(error) } @@ -83,35 +89,14 @@ export function createGraphWebDav(inner: WebDAV, graphClient: () => Graph): WebD // The mountpoint drive of a public link carries its owner. Failing to read it // costs the owner's name on the drop upload page, nothing else, so a link that // still works stays usable. -const publicLinkDrive = async ( - graphClient: () => Graph, - driveId: string, - requestOptions: Record -) => { +const publicLinkDrive = async (graphClient: () => Graph, driveId: string, signal?: AbortSignal) => { try { - return await graphClient().drives.getDrive(driveId, undefined, requestOptions) + return await graphClient().drives.getDrive(driveId, undefined, { signal }) } catch { return undefined } } -// The item carries its path in drive coordinates, which is what the caller -// asked for everywhere except a share space: that one is rooted at the shared -// item, so the requested path is the one relative to it. -const pathOf = ( - driveItem: { name?: string; parentReference?: { path?: string } }, - space: SpaceResource, - resource: { path?: string } -) => { - if (resource.path) { - return resource.path - } - const parentPath = driveItem.parentReference?.path - return !parentPath || parentPath === '.' - ? '/' - : urlJoin(parentPath, driveItem.name, { leadingSlash: true }) -} - // Callers branch on the shape webdav throws: a status code and, for a public // link, the code that tells "needs a password" from "wrong password" apart. // Graph carries the same information in its error body. diff --git a/packages/web-pkg/src/services/folder/loaders/loaderSpace.ts b/packages/web-pkg/src/services/folder/loaders/loaderSpace.ts index a855ee2c026..18a1dfbc275 100644 --- a/packages/web-pkg/src/services/folder/loaders/loaderSpace.ts +++ b/packages/web-pkg/src/services/folder/loaders/loaderSpace.ts @@ -12,7 +12,6 @@ import { } from '@opencloud-eu/web-client' import { unref } from 'vue' import { FolderLoaderOptions } from './types' -import { listFilesViaGraph } from './graphListing' import { DriveItem } from '@opencloud-eu/web-client/graph/generated' import { isLocationSpacesActive, isLocationPublicActive } from '../../../router' import { getSharedDriveItem, setCurrentUserShareSpacePermissions } from '../../../helpers' @@ -60,7 +59,7 @@ export class FolderLoaderSpace implements FolderLoader { // eslint-disable-next-line prefer-const let { resource: currentFolder, children: resources } = yield* call( - listFilesViaGraph({ graphClient, space, path, fileId, signal: signal1 }) + webdav.listFiles(space, { path, fileId }, { signal: signal1 }) ) // if current folder has no id (= singe file public link) we must not correct the route diff --git a/packages/web-pkg/tests/unit/services/folder/graphListing.spec.ts b/packages/web-pkg/tests/unit/services/client/graphListing.spec.ts similarity index 60% rename from packages/web-pkg/tests/unit/services/folder/graphListing.spec.ts rename to packages/web-pkg/tests/unit/services/client/graphListing.spec.ts index fb91e812496..70e49209426 100644 --- a/packages/web-pkg/tests/unit/services/folder/graphListing.spec.ts +++ b/packages/web-pkg/tests/unit/services/client/graphListing.spec.ts @@ -1,25 +1,7 @@ import { SpaceResource } from '@opencloud-eu/web-client' import { Graph } from '@opencloud-eu/web-client/graph' import { DriveItem } from '@opencloud-eu/web-client/graph/generated' -import { listFilesViaGraph } from '../../../../src/services/folder/loaders/graphListing' -import { - decryptResourceInPlace, - getVaultClaim, - markVaultStatus, - resolveVaultEngine -} from '../../../../src/helpers/vault' - -vi.mock('../../../../src/composables/piniaStores/extensionRegistry', () => ({ - useExtensionRegistry: vi.fn(() => ({})) -})) -// only the vault primitives are mocked, the translation on top of them runs -vi.mock('../../../../src/helpers/vault', () => ({ - getVaultClaim: vi.fn(() => null), - resolveVaultEngine: vi.fn(), - decryptResourceInPlace: vi.fn((_engine, r) => Promise.resolve(r)), - markVaultStatus: vi.fn() -})) - +import { listFilesViaGraph } from '../../../../src/services/client/graphListing' const space = { id: 'storage$space', webDavPath: '/dav/spaces/storage$space', @@ -46,8 +28,6 @@ function getGraphClient(driveItem: DriveItem = folder) { beforeEach(() => { vi.clearAllMocks() - vi.mocked(getVaultClaim).mockReturnValue(null) - vi.mocked(decryptResourceInPlace).mockImplementation((_engine, r) => Promise.resolve(r)) }) describe('listFilesViaGraph', () => { @@ -127,51 +107,4 @@ describe('listFilesViaGraph', () => { expect(resource.path).toBe('/') expect(children[0].path).toBe('/lorem.txt') }) - - describe('inside a vault', () => { - beforeEach(() => { - vi.mocked(getVaultClaim).mockImplementation((_registry, _space, path) => - path?.startsWith('/my.vault') ? ({ vaultRoot: '/my.vault' } as any) : null - ) - vi.mocked(resolveVaultEngine).mockResolvedValue({ - vaultRoot: '/my.vault', - encryptPath: vi.fn((p: string) => Promise.resolve(`ENC(${p})`)) - } as any) - }) - - it('encrypts the looked up path, the server knows the encrypted names only', async () => { - const { graphClient, statDriveItem } = getGraphClient() - - await listFilesViaGraph({ - graphClient, - space, - path: '/my.vault/Urlaub', - fileId: null, - signal: null - }) - - expect(statDriveItem.mock.calls[0][1]).toEqual({ path: '/my.vault/ENC(Urlaub)' }) - }) - - it('decrypts the folder and its children on the way back', async () => { - const { graphClient } = getGraphClient({ - id: 'storage$space!enc', - name: 'enc-folder', - folder: {}, - parentReference: { path: '/my.vault' }, - children: [{ id: 'storage$space!encChild', name: 'enc-child' }] - } as DriveItem) - - await listFilesViaGraph({ - graphClient, - space, - path: '/my.vault/Urlaub', - fileId: null, - signal: null - }) - - expect(decryptResourceInPlace).toHaveBeenCalledTimes(2) - expect(markVaultStatus).toHaveBeenCalledTimes(1) - }) - }) }) From 94a65afca101b33c95281b5a9dfcf172f086247b Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 12:25:47 +0200 Subject: [PATCH 20/23] fix(design-system): close a drop on escape when the focus never entered it A drop opened by pointer, a context menu on right click for instance, only listened for escape on the drop element itself, and the key never reached it while the focus sat outside. The listener now sits on the document, registered before the drop positions itself so an early key press is not lost either, and showDrop bails out when the drop was closed again while it was still waiting for a frame. --- .../src/components/OcDrop/OcDrop.spec.ts | 14 +++++++++++ .../src/components/OcDrop/OcDrop.vue | 24 +++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/packages/design-system/src/components/OcDrop/OcDrop.spec.ts b/packages/design-system/src/components/OcDrop/OcDrop.spec.ts index 0564e2614a9..36fb3b9ab79 100644 --- a/packages/design-system/src/components/OcDrop/OcDrop.spec.ts +++ b/packages/design-system/src/components/OcDrop/OcDrop.spec.ts @@ -188,4 +188,18 @@ describe('OcDrop', () => { expect(wrapper.find('oc-mobile-drop-stub').exists()).toBeTruthy() }) }) + + it('closes on escape when it was opened by pointer, so the focus never entered it', async () => { + const { wrapper } = dom() + document.querySelector('#trigger').click() + // no flushPromises: the drop is still positioning itself, escape has to + // close it even then + await nextTick() + expect(wrapper.find('.oc-drop').exists()).toBe(true) + + document.body.dispatchEvent(new KeyboardEvent('keydown', { code: 'Escape', bubbles: true })) + await nextTick() + + expect(wrapper.find('.oc-drop').exists()).toBe(false) + }) }) diff --git a/packages/design-system/src/components/OcDrop/OcDrop.vue b/packages/design-system/src/components/OcDrop/OcDrop.vue index 08c62a3aa1a..b5404e22ae7 100644 --- a/packages/design-system/src/components/OcDrop/OcDrop.vue +++ b/packages/design-system/src/components/OcDrop/OcDrop.vue @@ -273,6 +273,9 @@ const showDrop = async ({ const anchorEl: HTMLElement | VirtualElement | null = anchorElement || unref(anchor) activeAnchorElement = anchorEl isOpen.value = true + // registered before the drop is positioned: everything below waits for a + // frame, and a key pressed in between has to close the drop as well + registerEventListener(document, 'keydown', handleDocumentKeydown, 'document') await nextTick() if (!anchorEl) { console.warn('OcDrop cannot be opened: anchor element not found') @@ -282,6 +285,11 @@ const showDrop = async ({ // fixes a timing issue with the rendering of the drop await awaitAnimationFrame() + // escape can close the drop again while it is still positioning itself + if (!unref(isOpen) || !unref(drop)) { + return + } + if (isMenu) { // if drop is a menu, set role="menu" on all ul elements in the drop for better screen reader support const uls = unref(drop)?.getElementsByTagName('ul') @@ -317,6 +325,10 @@ const showDrop = async ({ ] }) + if (!unref(isOpen) || !unref(drop)) { + return + } + Object.assign(unref(drop).style, { left: `${x}px`, top: `${y}px` }) unref(anchor)?.setAttribute('aria-expanded', 'true') emit('showDrop') @@ -376,6 +388,18 @@ const handleDropClickOutside = async (event: Event) => { } } +// Escape has to close the drop even when the focus never entered it, which is +// the case whenever it was opened by pointer: a context menu on right click for +// instance. A key pressed inside the drop never reaches this handler, the drop's +// own one below stops it from travelling further. +const handleDocumentKeydown = (event: Event) => { + if (!isKeyboardEvent(event) || event.code !== 'Escape') { + return + } + hideDrop() + unref(anchor)?.focus() +} + const handleDropKeydown = (event: Event) => { if (!isKeyboardEvent(event)) { return From 1b360c10818896e7b110bac81b409ab727390090 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 12:25:47 +0200 Subject: [PATCH 21/23] fix(web-client): treat a drive root as a folder A drive root carries neither the folder nor the file facet, it reports itself as a root, so every stat of a space root came back as a file. The app defaults folder handling reads that type to decide whether the listing or the item itself belongs in the resource list, which left the media viewer without files to show. --- .../web-client/src/helpers/resource/graph.ts | 3 ++- .../tests/unit/helpers/resource/graph.spec.ts | 20 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/packages/web-client/src/helpers/resource/graph.ts b/packages/web-client/src/helpers/resource/graph.ts index c4bc83a0e6a..beac9924737 100644 --- a/packages/web-client/src/helpers/resource/graph.ts +++ b/packages/web-client/src/helpers/resource/graph.ts @@ -50,7 +50,8 @@ export const buildResourceFromDriveItem = ( // the drive root reports its own name, so callers that know the path pin it pathOverride?: string ): Resource => { - const isFolder = !!driveItem.folder + // a drive root carries neither facet, it reports itself as a root instead + const isFolder = !!driveItem.folder || !!driveItem.root const name = driveItem.name || '' const path = pathOverride ?? urlJoin(parentPath, name, { leadingSlash: true }) const actions = driveItem['@libre.graph.permissions.actions.allowedValues'] diff --git a/packages/web-client/tests/unit/helpers/resource/graph.spec.ts b/packages/web-client/tests/unit/helpers/resource/graph.spec.ts index 3c07e33144d..343035eae7a 100644 --- a/packages/web-client/tests/unit/helpers/resource/graph.spec.ts +++ b/packages/web-client/tests/unit/helpers/resource/graph.spec.ts @@ -76,6 +76,26 @@ describe('buildResourceFromDriveItem', () => { expect(r.owner).toEqual({ id: 'alice', displayName: 'Alice' }) }) + it('treats a drive root as a folder, it carries neither facet', () => { + const r = buildResourceFromDriveItem( + { + id: 'storage$space!space', + name: '.', + size: 4897, + root: {}, + parentReference: { id: 'storage$space', path: '.' }, + '@libre.graph.permissions.actions.allowedValues': managerActions + } as any, + space, + '', + '/' + ) + + expect(r.isFolder).toBe(true) + expect(r.type).toBe('folder') + expect(r.path).toBe('/') + }) + it('carries the facets and the lock through', () => { const r = buildResourceFromDriveItem( { From a1c4b5daa40d90c451aa974b9246e251abbe2377 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 12:25:47 +0200 Subject: [PATCH 22/23] fix(web-pkg): keep the vault working through the graph seam A caller asking for its own dav properties, the vault's integrity token for one, can only be answered by a PROPFIND, so those listings fall back to webdav. The preview follows the cleartext type as well: the server only ever sees the encrypted blob and reports no thumbnail for it. --- packages/web-pkg/src/helpers/vault.ts | 5 +++++ packages/web-pkg/src/services/client/graphWebDav.ts | 8 ++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/packages/web-pkg/src/helpers/vault.ts b/packages/web-pkg/src/helpers/vault.ts index db9c27a1591..7bbf6d6d141 100644 --- a/packages/web-pkg/src/helpers/vault.ts +++ b/packages/web-pkg/src/helpers/vault.ts @@ -190,6 +190,11 @@ export async function decryptResourceInPlace( const guessed = mimeTypeForExtension(r.extension) if (guessed) { r.mimeType = guessed + // Same reason for the preview: the server sees an opaque blob and never + // renders a thumbnail for it, so it reports no preview. The client can + // render it once decrypted, and the preview service goes through the + // vault-aware client to get the plaintext. + r.hasPreview = () => true } } // The engine resolved → resource is by definition inside (or *is*) a vault. diff --git a/packages/web-pkg/src/services/client/graphWebDav.ts b/packages/web-pkg/src/services/client/graphWebDav.ts index 2eb488e5bdf..72b36829edc 100644 --- a/packages/web-pkg/src/services/client/graphWebDav.ts +++ b/packages/web-pkg/src/services/client/graphWebDav.ts @@ -17,11 +17,15 @@ import { listFilesViaGraph } from './graphListing' * get the same shapes back, whichever API answered. * * What stays on webdav is what graph has no answer for: the trash bin, which - * has no listing, and the file versions, which have no endpoint. + * has no listing, the file versions, which have no endpoint, and a request for + * custom dav properties, which graph has no mechanism for. */ export function createGraphWebDav(inner: WebDAV, graphClient: () => Graph): WebDAV { const listFiles: WebDAV['listFiles'] = async (space, { path, fileId } = {}, options = {}) => { - if (options.isTrash) { + // the trash bin has no graph listing, and a caller asking for its own dav + // properties (the vault's integrity token, say) can only be answered by a + // PROPFIND: graph has no arbitrary property mechanism + if (options.isTrash || options.extraProps?.length) { return inner.listFiles(space, { path, fileId }, options) } From 7820f7ee045e42f7eb8e5b1315997c66dc413913 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 12:25:47 +0200 Subject: [PATCH 23/23] test: follow the public link item type and drop an unused import --- .../tests/unit/pages/resolvePublicLink.spec.ts | 8 ++++++-- tests/e2e/support/objects/app-files/resource/actions.ts | 1 - 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/web-runtime/tests/unit/pages/resolvePublicLink.spec.ts b/packages/web-runtime/tests/unit/pages/resolvePublicLink.spec.ts index 221fedc9161..daf53332e01 100644 --- a/packages/web-runtime/tests/unit/pages/resolvePublicLink.spec.ts +++ b/packages/web-runtime/tests/unit/pages/resolvePublicLink.spec.ts @@ -98,10 +98,11 @@ describe('resolvePublicLink', () => { canBeDeleted: () => false, canRestore: () => false } as Resource - // a link to a single file has no file id of its own + // the item behind the link tells a single file link apart const { mocks } = getWrapper({ redirectUrl: '', spaceFileId: 'token', + publicLinkItemType: 'file', children: [file] }) await flushPromises() @@ -146,12 +147,14 @@ function getWrapper({ getFileInfoErrorStatusCode = null, redirectUrl = 'redirectUrl', spaceFileId = 'folder-id', + publicLinkItemType = 'folder', children = [] }: { passwordRequired?: boolean getFileInfoErrorStatusCode?: number redirectUrl?: string spaceFileId?: string + publicLinkItemType?: 'file' | 'folder' children?: Resource[] } = {}) { const $clientService = mockDeep() @@ -160,7 +163,8 @@ function getWrapper({ fileId: spaceFileId, driveType: 'public', driveAlias: 'public/token', - isFolder: true, + publicLinkItemType, + isFolder: publicLinkItemType === 'folder', getDriveAliasAndItem: ({ path }: Resource) => urlJoin('public/token', path, { leadingSlash: false }) }) diff --git a/tests/e2e/support/objects/app-files/resource/actions.ts b/tests/e2e/support/objects/app-files/resource/actions.ts index ac9db73b5a8..006cf5cec73 100644 --- a/tests/e2e/support/objects/app-files/resource/actions.ts +++ b/tests/e2e/support/objects/app-files/resource/actions.ts @@ -9,7 +9,6 @@ import { File, Space } from '../../../types' import { waitProcessingToFinish } from '../fileEvents' import { state } from '../../../../environment/shared' import { lstatSync, readFileSync } from 'fs' -import { encodeWebDavPath } from '../../../utils' import { isFolderListingResponse, isResourceStatResponse } from '../../../utils/folderListing' const appLoadingSpinner = '#app-loading-spinner'