From e82241cf898e581f93cb46c5fd1c163dd2306135 Mon Sep 17 00:00:00 2001 From: skjnldsv Date: Thu, 6 Jun 2024 18:48:02 +0200 Subject: [PATCH] fix(files_sharing): fix parsing of remote shares Signed-off-by: skjnldsv --- .../src/actions/moveOrCopyActionUtils.ts | 5 ++-- apps/files/src/components/FileEntry.vue | 10 +++---- .../src/actions/sharingStatusAction.ts | 22 ++++++++------ .../src/services/SharingService.spec.ts | 17 ++++++++++- .../src/services/SharingService.ts | 30 +++++++++++++++---- 5 files changed, 61 insertions(+), 23 deletions(-) diff --git a/apps/files/src/actions/moveOrCopyActionUtils.ts b/apps/files/src/actions/moveOrCopyActionUtils.ts index 01614b14d8a..82a12761cb9 100644 --- a/apps/files/src/actions/moveOrCopyActionUtils.ts +++ b/apps/files/src/actions/moveOrCopyActionUtils.ts @@ -70,7 +70,8 @@ export const canDownload = (nodes: Node[]) => { } export const canCopy = (nodes: Node[]) => { - // For now the only restriction is that a shared file - // cannot be copied if the download is disabled + // a shared file cannot be copied if the download is disabled + // it can be copied if the user has at least read permissions return canDownload(nodes) + && !nodes.some(node => node.permissions === Permission.NONE) } diff --git a/apps/files/src/components/FileEntry.vue b/apps/files/src/components/FileEntry.vue index 5d9fcf0e253..a0c0cfe9699 100644 --- a/apps/files/src/components/FileEntry.vue +++ b/apps/files/src/components/FileEntry.vue @@ -82,7 +82,7 @@ class="files-list__row-mtime" data-cy-files-list-row-mtime @click="openDetailsIfAvailable"> - + @@ -194,8 +194,8 @@ export default defineComponent({ }, size() { - const size = parseInt(this.source.size, 10) || 0 - if (typeof size !== 'number' || size < 0) { + const size = parseInt(this.source.size, 10) + if (typeof size !== 'number' || isNaN(size) || size < 0) { return this.t('files', 'Pending') } return formatFileSize(size, true) @@ -203,8 +203,8 @@ export default defineComponent({ sizeOpacity() { const maxOpacitySize = 10 * 1024 * 1024 - const size = parseInt(this.source.size, 10) || 0 - if (!size || size < 0) { + const size = parseInt(this.source.size, 10) + if (!size || isNaN(size) || size < 0) { return {} } diff --git a/apps/files_sharing/src/actions/sharingStatusAction.ts b/apps/files_sharing/src/actions/sharingStatusAction.ts index 98a7d3d6112..828408f1846 100644 --- a/apps/files_sharing/src/actions/sharingStatusAction.ts +++ b/apps/files_sharing/src/actions/sharingStatusAction.ts @@ -34,14 +34,18 @@ import { getCurrentUser } from '@nextcloud/auth' import './sharingStatusAction.scss' -const generateAvatarSvg = (userId: string) => { - const avatarUrl = generateUrl('/avatar/{userId}/32', { userId }) +const generateAvatarSvg = (userId: string, isGuest = false) => { + const avatarUrl = generateUrl(isGuest ? '/avatar/guest/{userId}/32' : '/avatar/{userId}/32?guestFallback=true', { userId }) return `` } +const isExternal = (node: Node) => { + return node.attributes.remote_id !== undefined +} + export const action = new FileAction({ id: 'sharing-status', displayName(nodes: Node[]) { @@ -50,7 +54,7 @@ export const action = new FileAction({ const ownerId = node?.attributes?.['owner-id'] if (shareTypes.length > 0 - || (ownerId && ownerId !== getCurrentUser()?.uid)) { + || (ownerId !== getCurrentUser()?.uid || isExternal(node))) { return t('files_sharing', 'Shared') } @@ -63,11 +67,11 @@ export const action = new FileAction({ const ownerDisplayName = node?.attributes?.['owner-display-name'] // Mixed share types - if (Array.isArray(node.attributes?.['share-types'])) { + if (Array.isArray(node.attributes?.['share-types']) && node.attributes?.['share-types'].length > 1) { return t('files_sharing', 'Shared multiple times with different people') } - if (ownerId && ownerId !== getCurrentUser()?.uid) { + if (ownerId && (ownerId !== getCurrentUser()?.uid || isExternal(node))) { return t('files_sharing', 'Shared by {ownerDisplayName}', { ownerDisplayName }) } @@ -79,7 +83,7 @@ export const action = new FileAction({ const shareTypes = Object.values(node?.attributes?.['share-types'] || {}).flat() as number[] // Mixed share types - if (Array.isArray(node.attributes?.['share-types'])) { + if (Array.isArray(node.attributes?.['share-types']) && node.attributes?.['share-types'].length > 1) { return AccountPlusSvg } @@ -101,8 +105,8 @@ export const action = new FileAction({ } const ownerId = node?.attributes?.['owner-id'] - if (ownerId && ownerId !== getCurrentUser()?.uid) { - return generateAvatarSvg(ownerId) + if (ownerId && (ownerId !== getCurrentUser()?.uid || isExternal(node))) { + return generateAvatarSvg(ownerId, isExternal(node)) } return AccountPlusSvg @@ -124,7 +128,7 @@ export const action = new FileAction({ } // If the node is shared by someone else - if (ownerId && ownerId !== getCurrentUser()?.uid) { + if (ownerId && (ownerId !== getCurrentUser()?.uid || isExternal(node))) { return true } diff --git a/apps/files_sharing/src/services/SharingService.spec.ts b/apps/files_sharing/src/services/SharingService.spec.ts index 79b91c9826f..ce603e670a4 100644 --- a/apps/files_sharing/src/services/SharingService.spec.ts +++ b/apps/files_sharing/src/services/SharingService.spec.ts @@ -353,12 +353,27 @@ describe('SharingService share to Node mapping', () => { expect(folder.attributes.favorite).toBe(1) }) + test('Empty', async () => { + jest.spyOn(logger, 'error').mockImplementationOnce(() => {}) + jest.spyOn(axios, 'get').mockReturnValueOnce(Promise.resolve({ + data: { + ocs: { + data: [], + }, + }, + })) + + const shares = await getContents(false, true, false, false) + expect(shares.contents).toHaveLength(0) + expect(logger.error).toHaveBeenCalledTimes(0) + }) + test('Error', async () => { jest.spyOn(logger, 'error').mockImplementationOnce(() => {}) jest.spyOn(axios, 'get').mockReturnValueOnce(Promise.resolve({ data: { ocs: { - data: [{}], + data: [null], }, }, })) diff --git a/apps/files_sharing/src/services/SharingService.ts b/apps/files_sharing/src/services/SharingService.ts index 41861c5d3ea..7841b71a634 100644 --- a/apps/files_sharing/src/services/SharingService.ts +++ b/apps/files_sharing/src/services/SharingService.ts @@ -23,7 +23,7 @@ import type { AxiosPromise } from 'axios' import type { OCSResponse } from '@nextcloud/typings/ocs' -import { Folder, File, type ContentsWithRoot } from '@nextcloud/files' +import { Folder, File, type ContentsWithRoot, Permission } from '@nextcloud/files' import { generateOcsUrl, generateRemoteUrl } from '@nextcloud/router' import { getCurrentUser } from '@nextcloud/auth' import axios from '@nextcloud/axios' @@ -36,16 +36,34 @@ const headers = { 'Content-Type': 'application/json', } -const ocsEntryToNode = function(ocsEntry: any): Folder | File | null { +const ocsEntryToNode = async function(ocsEntry: any): Promise { try { + // Federated share handling + if (ocsEntry?.remote_id !== undefined) { + const mime = (await import('mime')).default + // This won't catch files without an extension, but this is the best we can do + ocsEntry.mimetype = mime.getType(ocsEntry.name) + ocsEntry.item_type = ocsEntry.mimetype ? 'file' : 'folder' + + // Need to set permissions to NONE for federated shares + ocsEntry.item_permissions = Permission.NONE + ocsEntry.permissions = Permission.NONE + + ocsEntry.uid_owner = ocsEntry.owner + // TODO: have the real display name stored somewhere + ocsEntry.displayname_owner = ocsEntry.owner + } + const isFolder = ocsEntry?.item_type === 'folder' const hasPreview = ocsEntry?.has_preview === true const Node = isFolder ? Folder : File - const fileid = ocsEntry.file_source + // If this is an external share that is not yet accepted, + // we don't have an id. We can fallback to the row id temporarily + const fileid = ocsEntry.file_source || ocsEntry.id // Generate path and strip double slashes - const path = ocsEntry?.path || ocsEntry.file_target + const path = ocsEntry?.path || ocsEntry.file_target || ocsEntry.name const source = generateRemoteUrl(`dav/${rootPath}/${path}`.replaceAll(/\/\//gm, '/')) // Prefer share time if more recent than item mtime @@ -58,7 +76,7 @@ const ocsEntryToNode = function(ocsEntry: any): Folder | File | null { id: fileid, source, owner: ocsEntry?.uid_owner, - mime: ocsEntry?.mimetype, + mime: ocsEntry?.mimetype || 'application/octet-stream', mtime, size: ocsEntry?.item_size, permissions: ocsEntry?.item_permissions || ocsEntry?.permissions, @@ -167,7 +185,7 @@ export const getContents = async (sharedWithYou = true, sharedWithOthers = true, const responses = await Promise.all(promises) const data = responses.map((response) => response.data.ocs.data).flat() - let contents = data.map(ocsEntryToNode) + let contents = (await Promise.all(data.map(ocsEntryToNode))) .filter((node) => node !== null) as (Folder | File)[] if (filterTypes.length > 0) { -- 2.39.5