Merge pull request #7154 from nextcloud/fix/7092-no-attachments-for-federated-shares

Disable attachment upload on federated shares

Author: max-nextcloud

lib/Service/ApiService.php

@@ -141,6 +141,8 @@ public function create(?int $fileId = null, ?string $filePath = null, ?string $b
 			$lockInfo = null;
 		}
 
+		$hasOwner = $file->getOwner() !== null;
+
 		if (!$readOnly) {
 			$isLocked = $this->documentService->lock($file->getId());
 			if (!$isLocked) {
@@ -155,6 +157,7 @@ public function create(?int $fileId = null, ?string $filePath = null, ?string $b
 			'content' => $content,
 			'documentState' => $documentState,
 			'lock' => $lockInfo,
+			'hasOwner' => $hasOwner,
 		]);
 	}
 

src/components/Menu/ActionAttachmentUpload.vue

@@ -6,7 +6,8 @@
 	<NcActions class="entry-action entry-action__image-upload"
 		:data-text-action-entry="actionEntry.key"
 		:name="actionEntry.label"
-		:title="actionEntry.label"
+		:disabled="isUploadDisabled"
+		:title="menuTitle"
 		:aria-label="actionEntry.label"
 		:container="menuIDSelector">
 		<template #icon>
@@ -56,7 +57,11 @@
 import { NcActions, NcActionSeparator, NcActionButton, NcIconSvgWrapper } from '@nextcloud/vue'
 import { loadState } from '@nextcloud/initial-state'
 import { Loading, Folder, Upload, Plus } from '../icons.js'
-import { useIsPublicMixin, useEditorUpload } from '../Editor.provider.js'
+import {
+	useIsPublicMixin,
+	useEditorUpload,
+	useSyncServiceMixin,
+} from '../Editor.provider.js'
 import { BaseActionEntry } from './BaseActionEntry.js'
 import { useMenuIDMixin } from './MenuBar.provider.js'
 import {
@@ -82,6 +87,7 @@ export default {
 	mixins: [
 		useIsPublicMixin,
 		useEditorUpload,
+		useSyncServiceMixin,
 		useActionAttachmentPromptMixin,
 		useUploadingStateMixin,
 		useActionChooseLocalAttachmentMixin,
@@ -100,6 +106,17 @@ export default {
 		templates() {
 			return loadState('files', 'templates', [])
 		},
+		isUploadDisabled() {
+			return !this.$syncService.hasOwner
+		},
+		menuTitle() {
+			return this.isUploadDisabled
+				? t(
+					'text',
+					"Attachments cannot be created or uploaded because this file is shared from another cloud."
+				)
+				: this.actionEntry.label
+		},
 	},
 	methods: {
 		createAttachment(template) {

src/components/SuggestionsBar.vue

@@ -22,6 +22,8 @@
 				type="secondary"
 				size="normal"
 				class="suggestions--button"
+				:disabled="isUploadDisabled"
+				:title="uploadTitle"
 				@click="$callChooseLocalAttachment">
 				<template #icon>
 					<Upload :size="20" />
@@ -59,7 +61,11 @@ import { NcButton } from '@nextcloud/vue'
 import { Document, Shape, Upload, Table as TableIcon } from '../components/icons.js'
 import { useActionChooseLocalAttachmentMixin } from './Editor/MediaHandler.provider.js'
 import { getLinkWithPicker } from '@nextcloud/vue/dist/Components/NcRichText.js'
-import { useEditorMixin, useFileMixin } from './Editor.provider.js'
+import {
+	useEditorMixin,
+	useFileMixin,
+	useSyncServiceMixin,
+} from './Editor.provider.js'
 import { generateUrl } from '@nextcloud/router'
 import { buildFilePicker } from '../helpers/filePicker.js'
 import { isMobileDevice } from '../helpers/isMobileDevice.js'
@@ -73,7 +79,13 @@ export default {
 		Shape,
 		Upload,
 	},
-	mixins: [useActionChooseLocalAttachmentMixin, useEditorMixin, useFileMixin],
+
+	mixins: [
+		useActionChooseLocalAttachmentMixin,
+		useEditorMixin,
+		useFileMixin,
+		useSyncServiceMixin,
+	],
 
 	setup() {
 		return {
@@ -92,6 +104,18 @@ export default {
 		relativePath() {
 			return this.$file?.relativePath ?? '/'
 		},
+		isUploadDisabled() {
+			return !this.$syncService.hasOwner
+		},
+		uploadTitle() {
+			return (
+				this.isUploadDisabled
+				&& t(
+					'text',
+					'Uploading attachments is disabled because the file is shared from another cloud.',
+				)
+			)
+		},
 	},
 
 	mounted() {

src/services/SessionApi.js

@@ -48,18 +48,27 @@ export class Connection {
 	#session
 	#lock
 	#readOnly
+	#hasOwner
 	#options
 
 	constructor(response, options) {
-		const { document, session, lock, readOnly, content, documentState } =
-			response.data
+		const {
+			document,
+			session,
+			lock,
+			readOnly,
+			content,
+			documentState,
+			hasOwner,
+		} = response.data
 		this.#document = document
 		this.#session = session
 		this.#lock = lock
 		this.#readOnly = readOnly
 		this.#content = content
 		this.#documentState = documentState
 		this.#options = options
+		this.#hasOwner = hasOwner
 		this.isPublic = !!options.shareToken
 		this.closed = false
 	}
@@ -89,6 +98,10 @@ export class Connection {
 		return this.closed
 	}
 
+	get hasOwner() {
+		return this.#hasOwner
+	}
+
 	get #defaultParams() {
 		return {
 			documentId: this.#document.id,

src/services/SyncService.js

@@ -87,6 +87,10 @@ class SyncService {
 		return this.#connection.state.document.readOnly
 	}
 
+	get hasOwner() {
+		return this.#connection?.hasOwner
+	}
+
 	get guestName() {
 		return this.#connection.session.guestName
 	}

src/tests/services/SessionApi.spec.js

@@ -0,0 +1,25 @@
+/**
+ * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+import { describe, it, vi, expect } from 'vitest'
+import axios from '@nextcloud/axios'
+import SessionApi, { Connection } from '../../services/SessionApi.js'
+
+vi.mock('@nextcloud/axios', () => {
+	const put = vi.fn()
+	return { default: { put } }
+})
+
+describe('Session api', () => {
+	it('opens a connection', async () => {
+		const api = new SessionApi()
+		axios.put.mockResolvedValue({ data: { hasOwner: true } })
+		const connection = await api.open({ fileId: 123, baseBersionEtag: 'abc' })
+		expect(connection).toBeInstanceOf(Connection)
+		expect(connection.isClosed).toBe(false)
+		expect(connection.hasOwner).toBe(true)
+	})
+})
+

src/tests/services/SyncService.spec.js

@@ -0,0 +1,36 @@
+/**
+ * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+import { describe, it, vi, expect } from 'vitest'
+import { SyncService } from '../../services/SyncService.js'
+
+describe('Sync service', () => {
+
+	it('opens a connection', async () => {
+		const api = mockApi({ hasOwner: true })
+		const service = new SyncService({ api, baseVersionEtag: 'abc' })
+		await service.open({ fileId: 123 })
+		expect(service.hasOwner).toBe(true)
+	})
+
+	it('opens a connection to a file without owner', async () => {
+		const api = mockApi({ hasOwner: false })
+		const service = new SyncService({ api, baseVersionEtag: 'abc' })
+		await service.open({ fileId: 123 })
+		expect(service.hasOwner).toBe(false)
+	})
+
+	it('hasOwner is undefined without connection', async () => {
+		const service = new SyncService({})
+		expect(service.hasOwner).toBe(undefined)
+	})
+
+})
+
+const mockApi = (connectionOptions = {}) => {
+	const defaults = { document: { baseVersionEtag: 'abc' } }
+	const open = vi.fn().mockResolvedValue({ ...defaults, ...connectionOptions })
+	return { open }
+}

tests/unit/Service/ApiServiceTest.php

@@ -0,0 +1,78 @@
+<?php
+
+namespace OCA\Text\Tests;
+
+use OCA\Text\Db\Document;
+use OCA\Text\Service\ApiService;
+use OCA\Text\Service\ConfigService;
+use OCA\Text\Service\DocumentService;
+use OCA\Text\Service\EncodingService;
+use OCA\Text\Service\SessionService;
+use OCP\IL10N;
+use OCP\IRequest;
+use Psr\Log\LoggerInterface;
+
+class ApiServiceTest extends \PHPUnit\Framework\TestCase {
+	private ApiService $apiService;
+
+	private IRequest $request;
+	private ConfigService $configService;
+	private SessionService $sessionService;
+	private DocumentService $documentService;
+	private EncodingService $encodingService;
+	private LoggerInterface $loggerInterface;
+	private IL10N $l10n;
+	private string $userId;
+
+	public function setUp(): void {
+		$this->request = $this->createMock(IRequest::class);
+		$this->configService = $this->createMock(ConfigService::class);
+		$this->sessionService = $this->createMock(SessionService::class);
+		$this->documentService = $this->createMock(DocumentService::class);
+		$this->encodingService = $this->createMock(EncodingService::class);
+		$this->loggerInterface = $this->createMock(LoggerInterface::class);
+		$this->l10n = $this->createMock(IL10N::class);
+		$this->userId = 'admin';
+
+		$document = new Document();
+		$document->setId(123);
+		$this->documentService->method('getDocument')->willReturn($document);
+		$this->documentService->method('isReadOnly')->willReturn(false);
+
+		$this->apiService = new ApiService(
+			$this->request,
+			$this->configService,
+			$this->sessionService,
+			$this->documentService,
+			$this->encodingService,
+			$this->loggerInterface,
+			$this->l10n,
+			$this->userId,
+			null,
+		);
+	}
+
+	public function testCreateNewSession() {
+		$file = $this->mockFile(1234, 'admin');
+		$this->documentService->method('getFileById')->willReturn($file);
+		$actual = $this->apiService->create(1234);
+		self::assertTrue($actual->getData()['hasOwner']);
+	}
+
+	public function testCreateNewSessionWithoutOwner() {
+		$file = $this->mockFile(1234, null);
+		$this->documentService->method('getFileById')->willReturn($file);
+		$actual = $this->apiService->create(1234);
+		self::assertFalse($actual->getData()['hasOwner']);
+	}
+
+	private function mockFile(int $id, ?string $owner) {
+		$file = $this->createMock(\OCP\Files\File::class);
+		$storage = $this->createMock(\OCP\Files\Storage\IStorage::class);
+		$file->method('getStorage')->willReturn($storage);
+		$file->method('getId')->willReturn($id);
+		$file->method('getOwner')->willReturn($owner);
+		return $file;
+	}
+
+}