From df769c435ed94cfdd7c4b0f3281f1bff21970344 Mon Sep 17 00:00:00 2001 From: Git'Fellow <12234510+solracsf@users.noreply.github.com> Date: Tue, 22 Sep 2026 12:45:03 +0200 Subject: [PATCH] fix(sharing): fix public share viewing and password-protected shares The public load/save flow read #isPublic, #sharingToken and #mimetype hidden inputs to detect a public share page. Those inputs were removed when the files_sharing public page moved to initial state, so a public share always fell through to the authenticated endpoints and failed. Detect the public share the same way the rest of the app already does, via isPublicShare()/getSharingToken() from @nextcloud/sharing/public. The iframe's error paths also called FilesMindMap.hide(), a method that no longer exists since the app moved to the Viewer API. Call the Viewer's own OCA.Viewer.close() instead. On the backend, a password-protected share was always rejected: the session stores an array of authenticated share IDs (public_link_authenticated, still filled by files_sharing's ShareController for this exact purpose), but the check compared it directly against a single share ID and could never match. Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com> --- .../PublicFileHandlingController.php | 22 ++++-- src/__tests__/mindmap.spec.js | 72 +++++++++++++++++-- src/mindmap.js | 21 ++---- src/viewer.js | 4 +- .../PublicFileHandlingControllerTest.php | 40 ++++++++++- 5 files changed, 126 insertions(+), 33 deletions(-) diff --git a/lib/Controller/PublicFileHandlingController.php b/lib/Controller/PublicFileHandlingController.php index ed0a420..6ae8045 100644 --- a/lib/Controller/PublicFileHandlingController.php +++ b/lib/Controller/PublicFileHandlingController.php @@ -63,6 +63,20 @@ protected function checkPermissions($share, $permissions) { return ($share->getPermissions() & $permissions) === $permissions; } + /** + * A password-protected share is authenticated once its ID is in the + * 'public_link_authenticated' session array, the same key + * files_sharing's ShareController fills on successful password entry. + */ + protected function isShareAuthenticated($share): bool { + if ($share->getPassword() === null) { + return true; + } + + $allowedShareIds = $this->session->get('public_link_authenticated'); + return is_array($allowedShareIds) && in_array($share->getId(), $allowedShareIds); + } + /** * load share mindmap file by path * @@ -83,9 +97,7 @@ public function load($token) { return new DataResponse(['message' => $this->l->t('Share not found')], Http::STATUS_NOT_FOUND); } - if ($share->getPassword() !== null && - (!$this->session->exists('public_link_authenticated') - || $this->session->get('public_link_authenticated') !== (string)$share->getId())) { + if (!$this->isShareAuthenticated($share)) { return new DataResponse(['message' => $this->l->t('You are not authorized to open this share')], Http::STATUS_BAD_REQUEST); } @@ -165,9 +177,7 @@ public function save($token, $filecontents, $path, $mtime) { return new DataResponse(['message' => $this->l->t('Share not found')], Http::STATUS_NOT_FOUND); } - if ($share->getPassword() !== null && - (!$this->session->exists('public_link_authenticated') - || $this->session->get('public_link_authenticated') !== (string)$share->getId())) { + if (!$this->isShareAuthenticated($share)) { return new DataResponse(['message' => $this->l->t('You are not authorized to open this share')], Http::STATUS_BAD_REQUEST); } diff --git a/src/__tests__/mindmap.spec.js b/src/__tests__/mindmap.spec.js index 1122173..63fd161 100644 --- a/src/__tests__/mindmap.spec.js +++ b/src/__tests__/mindmap.spec.js @@ -5,6 +5,8 @@ import axios from '@nextcloud/axios' import { showMessage as showToast } from '@nextcloud/dialogs' +import { generateUrl } from '@nextcloud/router' +import { getSharingToken, isPublicShare } from '@nextcloud/sharing/public' import { beforeEach, describe, expect, it, vi } from 'vitest' import FilesMindMap from '../mindmap.js' @@ -15,7 +17,7 @@ vi.mock('@nextcloud/l10n', () => ({ })) vi.mock('@nextcloud/router', () => ({ - generateUrl: (path) => `/nc${path}`, + generateUrl: vi.fn((path) => `/nc${path}`), })) vi.mock('@nextcloud/dialogs', () => ({ @@ -28,6 +30,7 @@ vi.mock('@nextcloud/auth', () => ({ vi.mock('@nextcloud/sharing/public', () => ({ isPublicShare: vi.fn(() => false), + getSharingToken: vi.fn(() => null), })) vi.mock('@nextcloud/event-bus', () => ({ @@ -74,6 +77,9 @@ describe('FilesMindMap', () => { FilesMindMap._file = {} FilesMindMap._currentContext = null vi.clearAllMocks() + // mockReturnValue survives clearAllMocks(), so restore the defaults explicitly + isPublicShare.mockReturnValue(false) + getSharingToken.mockReturnValue(null) }) // ─── Extension management ─────────────────────────────────────────────────── @@ -203,14 +209,12 @@ describe('FilesMindMap', () => { // ─── Public share detection ──────────────────────────────────────────────── describe('isMindmapPublic', () => { - it('returns false when not on a public share page', async () => { - const { isPublicShare } = await import('@nextcloud/sharing/public') + it('returns false when not on a public share page', () => { isPublicShare.mockReturnValue(false) expect(FilesMindMap.isMindmapPublic()).toBe(false) }) - it('returns true when on a public share page with a supported mime type', async () => { - const { isPublicShare } = await import('@nextcloud/sharing/public') + it('returns true when on a public share page with a supported mime type', () => { isPublicShare.mockReturnValue(true) FilesMindMap.registerExtension({ name: 'km', mimes: ['application/km'] }) @@ -225,8 +229,7 @@ describe('FilesMindMap', () => { } }) - it('returns false when on a public share page but mime type is unsupported', async () => { - const { isPublicShare } = await import('@nextcloud/sharing/public') + it('returns false when on a public share page but mime type is unsupported', () => { isPublicShare.mockReturnValue(true) const input = document.createElement('input') @@ -332,6 +335,30 @@ describe('FilesMindMap', () => { expect(fail).toHaveBeenCalledWith('Save failed') }) + + it('PUTs to the public share endpoint with the sharing token on a public share', async () => { + isPublicShare.mockReturnValue(true) + getSharingToken.mockReturnValue('the-token') + + const ext = { + name: 'km', + mimes: ['application/km'], + encode: vi.fn().mockResolvedValue('data'), + decode: null, + } + FilesMindMap._extensions = [ext] + FilesMindMap._file = { dir: '/docs', name: 'test.km', mime: 'application/km', mtime: 100 } + axios.mockResolvedValue({ data: { mtime: 200 } }) + + FilesMindMap.save('data', vi.fn(), vi.fn()) + await flushPromises() + + expect(axios).toHaveBeenCalledWith(expect.objectContaining({ + method: 'PUT', + url: '/nc/apps/files_mindmap/share/save', + data: expect.objectContaining({ token: 'the-token' }), + })) + }) }) // ─── load() ──────────────────────────────────────────────────────────────── @@ -448,5 +475,36 @@ describe('FilesMindMap', () => { expect(FilesMindMap._file.supportedWrite).toBe(false) }) + + it('GETs the public share endpoint with the sharing token on a public share', async () => { + isPublicShare.mockReturnValue(true) + getSharingToken.mockReturnValue('the-token') + + const ext = { + name: 'km', + mimes: ['application/km'], + encode: vi.fn(), + decode: vi.fn().mockResolvedValue({}), + } + FilesMindMap._extensions = [ext] + FilesMindMap._file = { dir: '/docs', name: 'test.km' } + + axios.get.mockResolvedValue({ + data: { + filecontents: btoa('content'), + mime: 'application/km', + writeable: true, + mtime: 1, + }, + }) + + FilesMindMap.load(vi.fn(), vi.fn()) + await flushPromises() + + expect(generateUrl).toHaveBeenCalledWith( + expect.stringContaining('/apps/files_mindmap/public/{token}'), + expect.objectContaining({ token: 'the-token' }), + ) + }) }) }) diff --git a/src/mindmap.js b/src/mindmap.js index dd7d524..46313c3 100644 --- a/src/mindmap.js +++ b/src/mindmap.js @@ -20,7 +20,7 @@ import { import { translate as t } from '@nextcloud/l10n' import { dirname } from '@nextcloud/paths' import { generateUrl } from '@nextcloud/router' -import { isPublicShare } from '@nextcloud/sharing/public' +import { getSharingToken, isPublicShare } from '@nextcloud/sharing/public' import logger from './logger.js' import freemind from './plugins/freemind.js' import km from './plugins/km.js' @@ -88,7 +88,6 @@ const FilesMindMap = { save(data, success, fail) { const self = this - let url = '' let path = this._file.dir + '/' + this._file.name if (this._file.dir === '/') { path = '/' + this._file.name @@ -108,12 +107,10 @@ const FilesMindMap = { mtime: self._file.mtime, // send modification time of currently loaded file } - if (document.getElementById('isPublic')?.value) { - putObject.token = document.getElementById('sharingToken')?.value + let url + if (isPublicShare()) { + putObject.token = getSharingToken() url = generateUrl('/apps/files_mindmap/share/save') - if (self.isSupportedMime(document.getElementById('mimetype')?.value)) { - putObject.path = '' - } } else { url = generateUrl('/apps/files_mindmap/ajax/savefile') } @@ -141,14 +138,8 @@ const FilesMindMap = { const filename = this._file.name const dir = this._file.dir let url - let sharingToken - const mimetype = document.getElementById('mimetype')?.value - if (document.getElementById('isPublic')?.value && this.isSupportedMime(mimetype)) { - sharingToken = document.getElementById('sharingToken')?.value - url = generateUrl('/apps/files_mindmap/public/{token}', { token: sharingToken }) - } else if (document.getElementById('isPublic')?.value) { - sharingToken = document.getElementById('sharingToken')?.value - url = generateUrl('/apps/files_mindmap/public/{token}?dir={dir}&filename={filename}', { token: sharingToken, filename, dir }) + if (isPublicShare()) { + url = generateUrl('/apps/files_mindmap/public/{token}?dir={dir}&filename={filename}', { token: getSharingToken(), filename, dir }) } else { url = generateUrl('/apps/files_mindmap/ajax/loadfile?filename={filename}&dir={dir}', { filename, dir }) } diff --git a/src/viewer.js b/src/viewer.js index 7ce7849..4f5d123 100644 --- a/src/viewer.js +++ b/src/viewer.js @@ -99,7 +99,7 @@ redirectIfNotDisplayedInFrame(); if (self._autoSaveTimer !== null) { clearInterval(self._autoSaveTimer) } - window.parent.OCA.FilesMindMap.hide() + window.parent.OCA.Viewer.close() } if (this._changed && window.parent.OCA.FilesMindMap._file.supportedWrite) { const result = window.confirm(t('The file has not been saved. Is it saved?')) @@ -233,7 +233,7 @@ redirectIfNotDisplayedInFrame(); }, function(msg) { self._loadStatus = false window.alert(t('Load file fail!') + msg) - window.parent.OCA.FilesMindMap.hide() + window.parent.OCA.Viewer.close() }) }, isDataSchema(url) { diff --git a/tests/Unit/Controller/PublicFileHandlingControllerTest.php b/tests/Unit/Controller/PublicFileHandlingControllerTest.php index 1fba76e..63f0137 100644 --- a/tests/Unit/Controller/PublicFileHandlingControllerTest.php +++ b/tests/Unit/Controller/PublicFileHandlingControllerTest.php @@ -25,6 +25,7 @@ class PublicFileHandlingControllerTest extends TestCase { private IManager&MockObject $shareManager; + private ISession&MockObject $session; private PublicFileHandlingController $controller; protected function setUp(): void { @@ -36,6 +37,7 @@ protected function setUp(): void { ['filename', null, 'secret.km'], ]); $this->shareManager = $this->createMock(IManager::class); + $this->session = $this->createMock(ISession::class); $l10n = $this->createMock(IL10N::class); $l10n->method('t')->willReturnArgument(0); @@ -45,7 +47,7 @@ protected function setUp(): void { $l10n, $this->createMock(LoggerInterface::class), $this->shareManager, - $this->createMock(ISession::class), + $this->session, ); } @@ -70,10 +72,42 @@ public function testLoadFromReadableShare(): void { $this->assertSame(base64_encode('{"root":{}}'), $response->getData()['filecontents']); } - private function mockShare(int $permissions, Folder $node): void { + public function testLoadFromPasswordProtectedShareWithoutSessionAuthIsRefused(): void { + $this->mockShare(password: 'secret', id: '42'); + $this->session->method('get')->with('public_link_authenticated')->willReturn(['1', '2']); + + $this->assertSame(Http::STATUS_BAD_REQUEST, $this->controller->load('token')->getStatus()); + } + + public function testLoadFromPasswordProtectedShareWithSessionAuthSucceeds(): void { + $file = $this->createMock(File::class); + $file->method('getContent')->willReturn('{"root":{}}'); + $folder = $this->createMock(Folder::class); + $folder->method('get')->with('/secret.km')->willReturn($file); + $this->mockShare(Constants::PERMISSION_READ, $folder, password: 'secret', id: '42'); + $this->session->method('get')->with('public_link_authenticated')->willReturn(['1', '42']); + + $this->assertSame(Http::STATUS_OK, $this->controller->load('token')->getStatus()); + } + + public function testSaveFromPasswordProtectedShareWithoutSessionAuthIsRefused(): void { + $this->mockShare(password: 'secret', id: '42')->expects($this->never())->method('getNode'); + $this->session->method('get')->with('public_link_authenticated')->willReturn(null); + + $response = $this->controller->save('token', 'data', '/secret.km', 123); + + $this->assertSame(Http::STATUS_BAD_REQUEST, $response->getStatus()); + } + + private function mockShare(int $permissions = 0, ?Folder $node = null, ?string $password = null, string $id = '1'): IShare&MockObject { $share = $this->createMock(IShare::class); + $share->method('getPassword')->willReturn($password); + $share->method('getId')->willReturn($id); $share->method('getPermissions')->willReturn($permissions); - $share->method('getNode')->willReturn($node); + if ($node !== null) { + $share->method('getNode')->willReturn($node); + } $this->shareManager->method('getShareByToken')->willReturn($share); + return $share; } }