chore: Refactor session handling to middleware and move over Attachme…

…ntController and UserApiController

Signed-off-by: Julius Härtl <[email protected]>

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',

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',

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 {

lib/Controller/ASessionAwareController.php

@@ -0,0 +1,25 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OCA\Text\Controller;
+
+use OCA\Text\Db\Session;
+use OCA\Text\Exception\InvalidSessionException;
+use OCP\AppFramework\ApiController;
+
+class ASessionAwareController extends ApiController {
+	private ?Session $session = null;
+
+	public function setSession(?Session $session): void {
+		$this->session = $session;
+	}
+
+	public function getSession(): Session {
+		if ($this->session === null) {
+			throw new InvalidSessionException();
+		}
+
+		return $this->session;
+	}
+}

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