diff --git a/composer/composer/autoload_classmap.php b/composer/composer/autoload_classmap.php index 14ca8cc780d..885bd3bebee 100644 --- a/composer/composer/autoload_classmap.php +++ b/composer/composer/autoload_classmap.php @@ -9,6 +9,7 @@ 'Composer\\InstalledVersions' => $vendorDir . '/composer/InstalledVersions.php', 'OCA\\Text\\AppInfo\\Application' => $baseDir . '/../lib/AppInfo/Application.php', 'OCA\\Text\\Command\\ResetDocument' => $baseDir . '/../lib/Command/ResetDocument.php', + 'OCA\\Text\\Controller\\ASessionAwareController' => $baseDir . '/../lib/Controller/ASessionAwareController.php', 'OCA\\Text\\Controller\\AttachmentController' => $baseDir . '/../lib/Controller/AttachmentController.php', 'OCA\\Text\\Controller\\NavigationController' => $baseDir . '/../lib/Controller/NavigationController.php', 'OCA\\Text\\Controller\\PublicSessionController' => $baseDir . '/../lib/Controller/PublicSessionController.php', @@ -29,6 +30,7 @@ 'OCA\\Text\\Event\\LoadEditor' => $baseDir . '/../lib/Event/LoadEditor.php', 'OCA\\Text\\Exception\\DocumentHasUnsavedChangesException' => $baseDir . '/../lib/Exception/DocumentHasUnsavedChangesException.php', 'OCA\\Text\\Exception\\DocumentSaveConflictException' => $baseDir . '/../lib/Exception/DocumentSaveConflictException.php', + 'OCA\\Text\\Exception\\InvalidSessionException' => $baseDir . '/../lib/Exception/InvalidSessionException.php', 'OCA\\Text\\Exception\\UploadException' => $baseDir . '/../lib/Exception/UploadException.php', 'OCA\\Text\\Exception\\VersionMismatchException' => $baseDir . '/../lib/Exception/VersionMismatchException.php', 'OCA\\Text\\Listeners\\AddMissingIndicesListener' => $baseDir . '/../lib/Listeners/AddMissingIndicesListener.php', @@ -41,6 +43,8 @@ 'OCA\\Text\\Listeners\\LoadViewerListener' => $baseDir . '/../lib/Listeners/LoadViewerListener.php', 'OCA\\Text\\Listeners\\NodeCopiedListener' => $baseDir . '/../lib/Listeners/NodeCopiedListener.php', 'OCA\\Text\\Listeners\\RegisterDirectEditorEventListener' => $baseDir . '/../lib/Listeners/RegisterDirectEditorEventListener.php', + 'OCA\\Text\\Middleware\\Attribute\\RequireDocumentSession' => $baseDir . '/../lib/Middleware/Attribute/RequireDocumentSession.php', + 'OCA\\Text\\Middleware\\SessionMiddleware' => $baseDir . '/../lib/Middleware/SessionMiddleware.php', 'OCA\\Text\\Migration\\ResetSessionsBeforeYjs' => $baseDir . '/../lib/Migration/ResetSessionsBeforeYjs.php', 'OCA\\Text\\Migration\\Version010000Date20190617184535' => $baseDir . '/../lib/Migration/Version010000Date20190617184535.php', 'OCA\\Text\\Migration\\Version030001Date20200402075029' => $baseDir . '/../lib/Migration/Version030001Date20200402075029.php', diff --git a/composer/composer/autoload_static.php b/composer/composer/autoload_static.php index 546cc473a8e..2e2d2aa1d3a 100644 --- a/composer/composer/autoload_static.php +++ b/composer/composer/autoload_static.php @@ -24,6 +24,7 @@ class ComposerStaticInitText 'Composer\\InstalledVersions' => __DIR__ . '/..' . '/composer/InstalledVersions.php', 'OCA\\Text\\AppInfo\\Application' => __DIR__ . '/..' . '/../lib/AppInfo/Application.php', 'OCA\\Text\\Command\\ResetDocument' => __DIR__ . '/..' . '/../lib/Command/ResetDocument.php', + 'OCA\\Text\\Controller\\ASessionAwareController' => __DIR__ . '/..' . '/../lib/Controller/ASessionAwareController.php', 'OCA\\Text\\Controller\\AttachmentController' => __DIR__ . '/..' . '/../lib/Controller/AttachmentController.php', 'OCA\\Text\\Controller\\NavigationController' => __DIR__ . '/..' . '/../lib/Controller/NavigationController.php', 'OCA\\Text\\Controller\\PublicSessionController' => __DIR__ . '/..' . '/../lib/Controller/PublicSessionController.php', @@ -44,6 +45,7 @@ class ComposerStaticInitText 'OCA\\Text\\Event\\LoadEditor' => __DIR__ . '/..' . '/../lib/Event/LoadEditor.php', 'OCA\\Text\\Exception\\DocumentHasUnsavedChangesException' => __DIR__ . '/..' . '/../lib/Exception/DocumentHasUnsavedChangesException.php', 'OCA\\Text\\Exception\\DocumentSaveConflictException' => __DIR__ . '/..' . '/../lib/Exception/DocumentSaveConflictException.php', + 'OCA\\Text\\Exception\\InvalidSessionException' => __DIR__ . '/..' . '/../lib/Exception/InvalidSessionException.php', 'OCA\\Text\\Exception\\UploadException' => __DIR__ . '/..' . '/../lib/Exception/UploadException.php', 'OCA\\Text\\Exception\\VersionMismatchException' => __DIR__ . '/..' . '/../lib/Exception/VersionMismatchException.php', 'OCA\\Text\\Listeners\\AddMissingIndicesListener' => __DIR__ . '/..' . '/../lib/Listeners/AddMissingIndicesListener.php', @@ -56,6 +58,8 @@ class ComposerStaticInitText 'OCA\\Text\\Listeners\\LoadViewerListener' => __DIR__ . '/..' . '/../lib/Listeners/LoadViewerListener.php', 'OCA\\Text\\Listeners\\NodeCopiedListener' => __DIR__ . '/..' . '/../lib/Listeners/NodeCopiedListener.php', 'OCA\\Text\\Listeners\\RegisterDirectEditorEventListener' => __DIR__ . '/..' . '/../lib/Listeners/RegisterDirectEditorEventListener.php', + 'OCA\\Text\\Middleware\\Attribute\\RequireDocumentSession' => __DIR__ . '/..' . '/../lib/Middleware/Attribute/RequireDocumentSession.php', + 'OCA\\Text\\Middleware\\SessionMiddleware' => __DIR__ . '/..' . '/../lib/Middleware/SessionMiddleware.php', 'OCA\\Text\\Migration\\ResetSessionsBeforeYjs' => __DIR__ . '/..' . '/../lib/Migration/ResetSessionsBeforeYjs.php', 'OCA\\Text\\Migration\\Version010000Date20190617184535' => __DIR__ . '/..' . '/../lib/Migration/Version010000Date20190617184535.php', 'OCA\\Text\\Migration\\Version030001Date20200402075029' => __DIR__ . '/..' . '/../lib/Migration/Version030001Date20200402075029.php', diff --git a/cypress/e2e/SessionApi.spec.js b/cypress/e2e/api/SessionApi.spec.js similarity index 99% rename from cypress/e2e/SessionApi.spec.js rename to cypress/e2e/api/SessionApi.spec.js index 863efede0c7..8cd210de4b8 100644 --- a/cypress/e2e/SessionApi.spec.js +++ b/cypress/e2e/api/SessionApi.spec.js @@ -20,7 +20,7 @@ * */ -import { randUser } from '../utils/index.js' +import { randUser } from '../../utils/index.js' const user = randUser() const messages = { diff --git a/cypress/e2e/api/UsersApi.spec.js b/cypress/e2e/api/UsersApi.spec.js new file mode 100644 index 00000000000..141776f0e77 --- /dev/null +++ b/cypress/e2e/api/UsersApi.spec.js @@ -0,0 +1,112 @@ +/* + * @copyright Copyright (c) 2022 Max + * @copyright Copyright (c) 2023 Julius Härtl + * + * @author Max + * @author Julius Härtl + * + * @license AGPL-3.0-or-later + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +import { randUser } from '../../utils/index.js' + +const user = randUser() + +describe('The user mention API', function() { + + before(function() { + cy.createUser(user) + window.OC = { + config: { modRewriteWorking: false }, + webroot: '', + } + }) + + let fileId + let requesttoken + + beforeEach(function() { + cy.login(user) + cy.prepareSessionApi().then((token) => { + requesttoken = token + cy.uploadTestFile('test.md') + .then(id => { + fileId = id + }) + }) + }) + + it('fetches users with valid session', function() { + cy.createTextSession(fileId).then(connection => { + cy.wrap(connection) + .its('document.id') + .should('equal', fileId) + const requestData = { + method: 'POST', + url: '/apps/text/api/v1/users', + body: { + documentId: connection.document.id, + sessionId: connection.session.id, + sessionToken: connection.session.token, + requesttoken, + }, + failOnStatusCode: false, + } + + cy.request(requestData).then(({ status }) => { + expect(status).to.eq(200) + }) + + const invalidRequestData = { ...requestData } + cy.wrap(() => { + invalidRequestData.body = { + ...requestData.body, + sessionToken: 'invalid', + } + }) + cy.request(invalidRequestData).then(({ status }) => { + expect(status).to.eq(403) + }) + + cy.wrap(() => { + invalidRequestData.body = { + ...requestData.body, + sessionId: 0, + } + }) + cy.request(invalidRequestData).then(({ status }) => { + expect(status).to.eq(403) + }) + + cy.wrap(() => { + invalidRequestData.body = { + ...requestData.body, + documentId: 0, + } + }) + cy.request(invalidRequestData).then(({ status }) => { + expect(status).to.eq(403) + }) + + cy.wrap(connection.close()) + + cy.request(requestData).then(({ status, body }) => { + expect(status).to.eq(403) + }) + }) + }) +}) diff --git a/cypress/support/sessions.js b/cypress/support/sessions.js index 587edae7542..7cfbd171ab2 100644 --- a/cypress/support/sessions.js +++ b/cypress/support/sessions.js @@ -23,9 +23,12 @@ import SessionApi from '../../src/services/SessionApi.js' import { emit } from '@nextcloud/event-bus' -Cypress.Commands.add('prepareSessionApi', (fileId) => { +Cypress.Commands.add('prepareSessionApi', () => { return cy.request('/csrftoken') - .then(({ body }) => emit('csrf-token-update', body)) + .then(({ body }) => { + emit('csrf-token-update', body) + return body.token + }) }) Cypress.Commands.add('createTextSession', (fileId, options = {}) => { diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 8571cf6ac26..7fdf15251a7 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -38,6 +38,7 @@ use OCA\Text\Listeners\LoadViewerListener; use OCA\Text\Listeners\NodeCopiedListener; use OCA\Text\Listeners\RegisterDirectEditorEventListener; +use OCA\Text\Middleware\SessionMiddleware; use OCA\Text\Notification\Notifier; use OCA\Text\Service\ConfigService; use OCA\TPAssistant\Event\BeforeAssistantNotificationEvent; @@ -76,6 +77,7 @@ public function register(IRegistrationContext $context): void { $context->registerEventListener(BeforeAssistantNotificationEvent::class, BeforeAssistantNotificationListener::class); $context->registerNotifierService(Notifier::class); + $context->registerMiddleware(SessionMiddleware::class); } public function boot(IBootContext $context): void { diff --git a/lib/Controller/ASessionAwareController.php b/lib/Controller/ASessionAwareController.php new file mode 100644 index 00000000000..6ec6f389793 --- /dev/null +++ b/lib/Controller/ASessionAwareController.php @@ -0,0 +1,25 @@ +session = $session; + } + + public function getSession(): Session { + if ($this->session === null) { + throw new InvalidSessionException(); + } + + return $this->session; + } +} diff --git a/lib/Controller/AttachmentController.php b/lib/Controller/AttachmentController.php index 066b670df19..7276f792f88 100644 --- a/lib/Controller/AttachmentController.php +++ b/lib/Controller/AttachmentController.php @@ -27,10 +27,12 @@ use Exception; use OCA\Text\Exception\UploadException; +use OCA\Text\Middleware\Attribute\RequireDocumentSession; use OCA\Text\Service\AttachmentService; -use OCA\Text\Service\SessionService; -use OCP\AppFramework\Controller; use OCP\AppFramework\Http; +use OCP\AppFramework\Http\Attribute\NoAdminRequired; +use OCP\AppFramework\Http\Attribute\NoCSRFRequired; +use OCP\AppFramework\Http\Attribute\PublicPage; use OCP\AppFramework\Http\DataDownloadResponse; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\RedirectResponse; @@ -40,7 +42,7 @@ use OCP\Util; use Psr\Log\LoggerInterface; -class AttachmentController extends Controller { +class AttachmentController extends ASessionAwareController { public const IMAGE_MIME_TYPES = [ 'image/png', 'image/jpeg', @@ -66,46 +68,25 @@ class AttachmentController extends Controller { 'image/webp', ]; - private AttachmentService $attachmentService; - private LoggerInterface $logger; - private SessionService $sessionService; - private IL10N $l10n; - private IMimeTypeDetector $mimeTypeDetector; - - public function __construct(string $appName, + public function __construct( + string $appName, IRequest $request, - IL10N $l10n, - LoggerInterface $logger, - IMimeTypeDetector $mimeTypeDetector, - AttachmentService $attachmentService, - SessionService $sessionService) { + private IL10N $l10n, + private LoggerInterface $logger, + private IMimeTypeDetector $mimeTypeDetector, + private AttachmentService $attachmentService + ) { parent::__construct($appName, $request); - $this->attachmentService = $attachmentService; - $this->request = $request; - $this->logger = $logger; - $this->sessionService = $sessionService; - $this->l10n = $l10n; - $this->mimeTypeDetector = $mimeTypeDetector; } - /** - * @NoAdminRequired - * @PublicPage - * - * @param int $documentId - * @param int $sessionId - * @param string $sessionToken - * @param string $filePath - * @return DataResponse - */ - public function insertAttachmentFile(int $documentId, int $sessionId, string $sessionToken, string $filePath): DataResponse { - if (!$this->sessionService->isValidSession($documentId, $sessionId, $sessionToken)) { - return new DataResponse([], Http::STATUS_FORBIDDEN); - } - $userId = $this->getUserIdFromSession($documentId, $sessionId, $sessionToken); + #[NoAdminRequired] + #[PublicPage] + #[RequireDocumentSession] + public function insertAttachmentFile(string $filePath): DataResponse { + $userId = $this->getSession()->getUserId(); try { - $insertResult = $this->attachmentService->insertAttachmentFile($documentId, $filePath, $userId); + $insertResult = $this->attachmentService->insertAttachmentFile($this->getSession()->getDocumentId(), $filePath, $userId); if (isset($insertResult['error'])) { return new DataResponse($insertResult, Http::STATUS_BAD_REQUEST); } else { @@ -117,25 +98,11 @@ public function insertAttachmentFile(int $documentId, int $sessionId, string $se } } - /** - * @NoAdminRequired - * @PublicPage - * - * @param int $documentId - * @param int $sessionId - * @param string $sessionToken - * @param string|null $shareToken - * @return DataResponse - */ - public function uploadAttachment(int $documentId, int $sessionId, string $sessionToken, ?string $shareToken = null): DataResponse { - if (!$this->sessionService->isValidSession($documentId, $sessionId, $sessionToken)) { - $this->logger->debug('Invalid session found when uploading', [ - 'documentId' => $documentId, - 'sessionId' => $sessionId, - 'sessionToken' => $sessionToken - ]); - return new DataResponse(['error' => 'Upload error, unauthorized action'], Http::STATUS_FORBIDDEN); - } + #[NoAdminRequired] + #[PublicPage] + #[RequireDocumentSession] + public function uploadAttachment(?string $shareToken = null): DataResponse { + $documentId = $this->getSession()->getDocumentId(); try { $file = $this->getUploadedFile('file'); @@ -148,7 +115,7 @@ public function uploadAttachment(int $documentId, int $sessionId, string $sessio if ($shareToken) { $uploadResult = $this->attachmentService->uploadAttachmentPublic($documentId, $newFileName, $newFileResource, $shareToken); } else { - $userId = $this->getUserIdFromSession($documentId, $sessionId, $sessionToken); + $userId = $this->getSession()->getUserId(); $uploadResult = $this->attachmentService->uploadAttachment($documentId, $newFileName, $newFileResource, $userId); } if (isset($uploadResult['error'])) { @@ -191,30 +158,21 @@ private function getUploadedFile(string $key): array { } /** - * @NoAdminRequired - * @NoCSRFRequired - * @PublicPage - * * Serve the image files in the editor - * @param int $documentId - * @param int $sessionId - * @param string $sessionToken - * @param string $imageFileName - * @param string|null $shareToken - * @param int $preferRawImage - * @return DataDownloadResponse|DataResponse */ - public function getImageFile(int $documentId, int $sessionId, string $sessionToken, string $imageFileName, ?string $shareToken = null, + #[NoAdminRequired] + #[PublicPage] + #[NoCSRFRequired] + #[RequireDocumentSession] + public function getImageFile(string $imageFileName, ?string $shareToken = null, int $preferRawImage = 0) { - if (!$this->sessionService->isValidSession($documentId, $sessionId, $sessionToken)) { - return new DataResponse('', Http::STATUS_FORBIDDEN); - } + $documentId = $this->getSession()->getDocumentId(); try { if ($shareToken) { $imageFile = $this->attachmentService->getImageFilePublic($documentId, $imageFileName, $shareToken, $preferRawImage === 1); } else { - $userId = $this->getUserIdFromSession($documentId, $sessionId, $sessionToken); + $userId = $this->getSession()->getUserId(); $imageFile = $this->attachmentService->getImageFile($documentId, $imageFileName, $userId, $preferRawImage === 1); } return $imageFile !== null @@ -231,28 +189,20 @@ public function getImageFile(int $documentId, int $sessionId, string $sessionTok } /** - * @NoAdminRequired - * @NoCSRFRequired - * @PublicPage - * * Serve the media files in the editor - * @param int $documentId - * @param int $sessionId - * @param string $sessionToken - * @param string $mediaFileName - * @param string|null $shareToken - * @return DataDownloadResponse|DataResponse */ - public function getMediaFile(int $documentId, int $sessionId, string $sessionToken, string $mediaFileName, ?string $shareToken = null) { - if (!$this->sessionService->isValidSession($documentId, $sessionId, $sessionToken)) { - return new DataResponse('', Http::STATUS_FORBIDDEN); - } + #[NoAdminRequired] + #[PublicPage] + #[NoCSRFRequired] + #[RequireDocumentSession] + public function getMediaFile(string $mediaFileName, ?string $shareToken = null) { + $documentId = $this->getSession()->getDocumentId(); try { if ($shareToken) { $mediaFile = $this->attachmentService->getMediaFilePublic($documentId, $mediaFileName, $shareToken); } else { - $userId = $this->getUserIdFromSession($documentId, $sessionId, $sessionToken); + $userId = $this->getSession()->getUserId(); $mediaFile = $this->attachmentService->getMediaFile($documentId, $mediaFileName, $userId); } return $mediaFile !== null @@ -269,28 +219,21 @@ public function getMediaFile(int $documentId, int $sessionId, string $sessionTok } /** - * @NoAdminRequired - * @NoCSRFRequired - * @PublicPage - * * Serve the media files preview in the editor - * @param int $documentId - * @param int $sessionId - * @param string $sessionToken - * @param string $mediaFileName - * @param string|null $shareToken * @return DataDownloadResponse|DataResponse|RedirectResponse */ - public function getMediaFilePreview(int $documentId, int $sessionId, string $sessionToken, string $mediaFileName, ?string $shareToken = null) { - if (!$this->sessionService->isValidSession($documentId, $sessionId, $sessionToken)) { - return new DataResponse('', Http::STATUS_FORBIDDEN); - } + #[NoAdminRequired] + #[PublicPage] + #[NoCSRFRequired] + #[RequireDocumentSession] + public function getMediaFilePreview(string $mediaFileName, ?string $shareToken = null) { + $documentId = $this->getSession()->getDocumentId(); try { if ($shareToken) { $preview = $this->attachmentService->getMediaFilePreviewPublic($documentId, $mediaFileName, $shareToken); } else { - $userId = $this->getUserIdFromSession($documentId, $sessionId, $sessionToken); + $userId = $this->getSession()->getUserId(); $preview = $this->attachmentService->getMediaFilePreview($documentId, $mediaFileName, $userId); } if ($preview === null) { @@ -312,29 +255,19 @@ public function getMediaFilePreview(int $documentId, int $sessionId, string $ses } /** - * @NoAdminRequired - * @NoCSRFRequired - * @PublicPage - * * Serve the media files metadata in the editor - * @param int $documentId - * @param int $sessionId - * @param string $sessionToken - * @param string $mediaFileName - * @param string|null $shareToken - * @return DataResponse */ - public function getMediaFileMetadata(int $documentId, int $sessionId, string $sessionToken, - string $mediaFileName, ?string $shareToken = null): DataResponse { - if (!$this->sessionService->isValidSession($documentId, $sessionId, $sessionToken)) { - return new DataResponse('', Http::STATUS_FORBIDDEN); - } - + #[NoAdminRequired] + #[PublicPage] + #[NoCSRFRequired] + #[RequireDocumentSession] + public function getMediaFileMetadata(string $mediaFileName, ?string $shareToken = null): DataResponse { + $documentId = $this->getSession()->getDocumentId(); try { if ($shareToken) { $metadata = $this->attachmentService->getMediaFileMetadataPublic($documentId, $mediaFileName, $shareToken); } else { - $userId = $this->getUserIdFromSession($documentId, $sessionId, $sessionToken); + $userId = $this->getSession()->getUserId(); $metadata = $this->attachmentService->getMediaFileMetadataPrivate($documentId, $mediaFileName, $userId); } if ($metadata === null) { @@ -347,19 +280,6 @@ public function getMediaFileMetadata(int $documentId, int $sessionId, string $se } } - /** - * Extract the user ID from the edition session - * - * @param int $documentId - * @param int $sessionId - * @param string $sessionToken - * @return ?string - */ - private function getUserIdFromSession(int $documentId, int $sessionId, string $sessionToken): ?string { - $session = $this->sessionService->getSession($documentId, $sessionId, $sessionToken); - return $session->getUserId(); - } - /** * Allow all supported mimetypes * Use mimetype detector for the other ones diff --git a/lib/Controller/SessionController.php b/lib/Controller/SessionController.php index 7823afd622c..c38e8dc6948 100644 --- a/lib/Controller/SessionController.php +++ b/lib/Controller/SessionController.php @@ -116,7 +116,7 @@ public function mention(int $documentId, int $sessionId, string $sessionToken, s private function loginSessionUser(int $documentId, int $sessionId, string $sessionToken) { $currentSession = $this->sessionService->getSession($documentId, $sessionId, $sessionToken); - if ($currentSession !== false && !$this->userSession->isLoggedIn()) { + if ($currentSession !== null && !$this->userSession->isLoggedIn()) { $user = $this->userManager->get($currentSession->getUserId()); if ($user !== null) { $this->restoreUser = true; diff --git a/lib/Controller/UserApiController.php b/lib/Controller/UserApiController.php index 59ceb8824b9..ff48a843e15 100644 --- a/lib/Controller/UserApiController.php +++ b/lib/Controller/UserApiController.php @@ -1,38 +1,35 @@ sessionService = $sessionService; - $this->collaboratorSearch = $ISearch; - $this->userManager = $userManager; } - /** - * @NoAdminRequired - * @PublicPage - */ - public function index(int $documentId, int $sessionId, string $sessionToken, string $filter, int $limit = 5): DataResponse { - if (!$this->sessionService->isValidSession($documentId, $sessionId, $sessionToken)) { - return new DataResponse([], 403); - } - - $sessions = $this->sessionService->getAllSessions($documentId); + #[PublicPage] + #[NoAdminRequired] + #[RequireDocumentSession] + public function index(string $filter = '', int $limit = 5): DataResponse { + $sessions = $this->sessionService->getAllSessions($this->getSession()->getDocumentId()); $users = []; @@ -47,8 +44,7 @@ public function index(int $documentId, int $sessionId, string $sessionToken, str } } - $currentSession = $this->sessionService->getSession($documentId, $sessionId, $sessionToken); - if ($currentSession->getUserId() !== null) { + if ($this->getSession()->getUserId() !== null) { // Add other users to the autocomplete list [$result] = $this->collaboratorSearch->search($filter, [IShare::TYPE_USER], false, $limit, 0); $userSearch = array_merge($result['users'], $result['exact']['users']); diff --git a/lib/Exception/InvalidSessionException.php b/lib/Exception/InvalidSessionException.php new file mode 100644 index 00000000000..38ce10c43a6 --- /dev/null +++ b/lib/Exception/InvalidSessionException.php @@ -0,0 +1,9 @@ +getAttributes(RequireDocumentSession::class))) { + $this->assertDocumentSession($controller); + } + } + + private function assertDocumentSession(ASessionAwareController $controller): void { + $documentId = (int)$this->request->getParam('documentId'); + $sessionId = (int)$this->request->getParam('sessionId'); + $token = (string)$this->request->getParam('sessionToken'); + + $session = $this->sessionService->getValidSession($documentId, $sessionId, $token); + if (!$session) { + throw new InvalidSessionException(); + } + + $controller->setSession($session); + } + + public function afterException($controller, $methodName, \Exception $exception) { + if ($exception instanceof InvalidSessionException) { + return new DataResponse([], 403); + } + + return parent::afterException($controller, $methodName, $exception); + } +} diff --git a/lib/Service/AttachmentService.php b/lib/Service/AttachmentService.php index e07e7a1bdae..41aa522478b 100644 --- a/lib/Service/AttachmentService.php +++ b/lib/Service/AttachmentService.php @@ -35,7 +35,6 @@ use OCP\Files\IMimeTypeDetector; use OCP\Files\InvalidPathException; use OCP\Files\IRootFolder; -use OCP\Files\Node; use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; use OCP\Files\SimpleFS\ISimpleFile; @@ -76,49 +75,35 @@ public function __construct(IRootFolder $rootFolder, /** * Get image content or preview from file name - * @param int $documentId - * @param string $imageFileName - * @param string $userId - * @param bool $preferRawImage - * @return File|Node|ISimpleFile|null * @throws InvalidPathException * @throws NoUserException * @throws NotFoundException * @throws NotPermittedException */ - public function getImageFile(int $documentId, string $imageFileName, string $userId, bool $preferRawImage) { + public function getImageFile(int $documentId, string $imageFileName, string $userId, bool $preferRawImage): File|ISimpleFile|null { $textFile = $this->getTextFile($documentId, $userId); return $this->getImageFileContent($imageFileName, $textFile, $preferRawImage); } /** * Get image content or preview from file id in public context - * @param int $documentId - * @param string $imageFileName - * @param string $shareToken - * @param bool $preferRawImage - * @return File|Node|ISimpleFile|null * @throws NotFoundException * @throws NotPermittedException * @throws InvalidPathException * @throws NoUserException */ - public function getImageFilePublic(int $documentId, string $imageFileName, string $shareToken, bool $preferRawImage) { + public function getImageFilePublic(int $documentId, string $imageFileName, string $shareToken, bool $preferRawImage): File|ISimpleFile|null { $textFile = $this->getTextFilePublic($documentId, $shareToken); return $this->getImageFileContent($imageFileName, $textFile, $preferRawImage); } /** - * @param string $imageFileName - * @param File $textFile - * @param bool $preferRawImage - * @return File|Node|ISimpleFile|null * @throws InvalidPathException * @throws NoUserException * @throws NotFoundException * @throws NotPermittedException */ - private function getImageFileContent(string $imageFileName, File $textFile, bool $preferRawImage) { + private function getImageFileContent(string $imageFileName, File $textFile, bool $preferRawImage): File|ISimpleFile|null { $attachmentFolder = $this->getAttachmentDirectoryForFile($textFile, true); $imageFile = $attachmentFolder->get($imageFileName); if ($imageFile instanceof File && in_array($imageFile->getMimetype(), AttachmentController::IMAGE_MIME_TYPES)) { @@ -141,39 +126,28 @@ private function getImageFileContent(string $imageFileName, File $textFile, bool /** * Get media file from file name - * @param int $documentId - * @param string $mediaFileName - * @param string $userId - * @return File|\OCP\Files\Node|ISimpleFile|null * @throws NotFoundException * @throws \OCP\Files\InvalidPathException * @throws \OCP\Files\NotPermittedException */ - public function getMediaFile(int $documentId, string $mediaFileName, string $userId) { + public function getMediaFile(int $documentId, string $mediaFileName, string $userId): File|null { $textFile = $this->getTextFile($documentId, $userId); return $this->getMediaFullFile($mediaFileName, $textFile); } /** * Get image content or preview from file id in public context - * @param int $documentId - * @param string $mediaFileName - * @param string $shareToken - * @return File|\OCP\Files\Node|ISimpleFile|null * @throws NotFoundException * @throws NotPermittedException * @throws \OCP\Files\InvalidPathException * @throws \OC\User\NoUserException */ - public function getMediaFilePublic(int $documentId, string $mediaFileName, string $shareToken) { + public function getMediaFilePublic(int $documentId, string $mediaFileName, string $shareToken): File|null { $textFile = $this->getTextFilePublic($documentId, $shareToken); return $this->getMediaFullFile($mediaFileName, $textFile); } /** - * @param string $mediaFileName - * @param File $textFile - * @return File|null * @throws NotFoundException * @throws NotPermittedException * @throws \OCP\Files\InvalidPathException @@ -189,10 +163,6 @@ private function getMediaFullFile(string $mediaFileName, File $textFile): ?File } /** - * @param int $documentId - * @param string $mediaFileName - * @param string $userId - * @return array|null * @throws NotFoundException * @throws NotPermittedException * @throws \OCP\Files\InvalidPathException @@ -203,10 +173,6 @@ public function getMediaFilePreview(int $documentId, string $mediaFileName, stri return $this->getMediaFilePreviewFile($mediaFileName, $textFile); } /** - * @param int $documentId - * @param string $mediaFileName - * @param string $shareToken - * @return array|null * @throws NotFoundException * @throws NotPermittedException * @throws \OCP\Files\InvalidPathException @@ -219,9 +185,6 @@ public function getMediaFilePreviewPublic(int $documentId, string $mediaFileName /** * Get media preview or mimetype icon address - * @param string $mediaFileName - * @param File $textFile - * @return array|null * @throws NotFoundException * @throws NotPermittedException * @throws \OCP\Files\InvalidPathException diff --git a/lib/Service/SessionService.php b/lib/Service/SessionService.php index 4bf0a5e105c..1ee3b827ae9 100644 --- a/lib/Service/SessionService.php +++ b/lib/Service/SessionService.php @@ -1,4 +1,7 @@ * @@ -43,15 +46,11 @@ class SessionService { private ITimeFactory $timeFactory; private IUserManager $userManager; private IAvatarManager $avatarManager; + private ?string $userId; + private ICache $cache; - /** @var string|null */ - private $userId; - - /** @var Session cache current session in the request */ - private $session = null; - - /** @var ICache */ - private $cache; + /** @var ?Session cache current session in the request */ + private ?Session $session = null; public function __construct( SessionMapper $sessionMapper, @@ -112,7 +111,7 @@ public function closeSession(int $documentId, int $sessionId, string $token): vo } } - public function getAllSessions($documentId): array { + public function getAllSessions(int $documentId): array { $sessions = $this->sessionMapper->findAll($documentId); return array_map(function (Session $session) { $result = $session->jsonSerialize(); @@ -123,7 +122,7 @@ public function getAllSessions($documentId): array { }, $sessions); } - public function getActiveSessions($documentId): array { + public function getActiveSessions(int $documentId): array { $sessions = $this->sessionMapper->findAllActive($documentId); return array_map(function (Session $session) { $result = $session->jsonSerialize(); @@ -146,50 +145,42 @@ public function findAllInactive() { return $this->sessionMapper->findAllInactive(); } - public function removeInactiveSessionsWithoutSteps($documentId = -1) { + public function removeInactiveSessionsWithoutSteps(?int $documentId = null) { // No need to clear the cache here as we already set a TTL return $this->sessionMapper->deleteInactiveWithoutSteps($documentId); } - /** - * @return bool|Session - */ - public function getSession($documentId, $sessionId, $token) { + public function getSession(int $documentId, int $sessionId, string $token): ?Session { if ($this->session !== null) { return $this->session; } $data = $this->cache->get($token); if ($data !== null) { - $session = Session::fromRow(json_decode($data, true)); - if ($session->getId() !== $sessionId || $session->getDocumentId() !== $documentId) { + $this->session = Session::fromRow(json_decode($data, true)); + if ($this->session->getId() !== $sessionId || $this->session->getDocumentId() !== $documentId) { $this->cache->remove($token); - $this->session = false; - return false; + $this->session = null; } - return $session; + return $this->session; } try { - $data = $this->sessionMapper->find($documentId, $sessionId, $token); - $jsonData = json_encode($data); - - if ($jsonData) { - $this->cache->set($token, json_encode($data), self::SESSION_VALID_TIME - 30); - return $data; - } + $this->session = $this->sessionMapper->find($documentId, $sessionId, $token); + $this->cache->set($token, json_encode($this->session), self::SESSION_VALID_TIME - 30); } catch (DoesNotExistException $e) { + $this->session = null; + $this->cache->remove($token); } - $this->session = false; - return false; + return $this->session; } - public function isValidSession($documentId, $sessionId, $token): bool { + public function getValidSession(int $documentId, int $sessionId, string $token): ?Session { $session = $this->getSession($documentId, $sessionId, $token); - if ($session === false) { - return false; + if ($session === null) { + return null; } $currentTime = $this->timeFactory->getTime(); @@ -201,15 +192,21 @@ public function isValidSession($documentId, $sessionId, $token): bool { try { $session = $this->sessionMapper->find($documentId, $sessionId, $token); } catch (DoesNotExistException $e) { - $this->session = false; + $this->session = null; $this->cache->remove($token); - return false; + return null; } $session->setLastContact($this->timeFactory->getTime()); $this->sessionMapper->update($session); $this->cache->set($token, json_encode($session), self::SESSION_VALID_TIME - 30); + $this->session = $session; } - return true; + + return $session; + } + + public function isValidSession($documentId, $sessionId, $token): bool { + return $this->getValidSession($documentId, $sessionId, $token) !== null; } /** diff --git a/src/services/SessionApi.js b/src/services/SessionApi.js index a5676564ec8..8cb8993b565 100644 --- a/src/services/SessionApi.js +++ b/src/services/SessionApi.js @@ -78,6 +78,10 @@ export class Connection { this.closed = false } + get session() { + return this.#session + } + get document() { return this.#document } diff --git a/tests/unit/Controller/UserApiControllerTest.php b/tests/unit/Controller/UserApiControllerTest.php index 86066518bf8..14db310e38e 100644 --- a/tests/unit/Controller/UserApiControllerTest.php +++ b/tests/unit/Controller/UserApiControllerTest.php @@ -52,17 +52,16 @@ protected function setUp(): void { $this->userManager, $this->userSession ); + + $session = new Session(); + $session->setUserId('admin'); + $this->userApiController->setSession($session); } /** * @dataProvider dataTestUsersIndex */ public function testUsersIndex(int $documentId, int $sessionId, string $sessionToken, string $filter) { - $session = new Session(); - $session->setUserId('admin'); - $this->sessionService - ->expects($this->once()) - ->method('isValidSession')->willReturn(true); $this->sessionService ->expects($this->once()) ->method('getAllSessions')->willReturn([[ @@ -72,9 +71,6 @@ public function testUsersIndex(int $documentId, int $sessionId, string $sessionT $this->userManager->expects($this->once()) ->method('getDisplayName') ->willReturn('Administrator'); - $this->sessionService - ->expects($this->once()) - ->method('getSession')->willReturn($session); $this->collaboratorSearch ->expects($this->once()) ->method('search')->willReturn([ @@ -99,9 +95,6 @@ public function testUsersIndex(int $documentId, int $sessionId, string $sessionT ]); $response = $this->userApiController->index( - $documentId, - $sessionId, - $sessionToken, $filter ); $this->assertInstanceOf(DataResponse::class, $response);