From 5b6d66edd1d0ac6c3de40b0674d08a8d63d0166f Mon Sep 17 00:00:00 2001 From: silver Date: Mon, 3 Aug 2026 15:48:32 +0200 Subject: [PATCH 1/2] fix(DirectEditing): check if user is enabled and add tests Signed-off-by: silver Assisted-by: ClaudeCode:claude-sonnet-5 --- lib/Exception/AccountDisabledException.php | 14 +++ lib/Middleware/SessionMiddleware.php | 16 +++ .../unit/Middleware/SessionMiddlewareTest.php | 101 +++++++++++++++++- 3 files changed, 129 insertions(+), 2 deletions(-) create mode 100644 lib/Exception/AccountDisabledException.php diff --git a/lib/Exception/AccountDisabledException.php b/lib/Exception/AccountDisabledException.php new file mode 100644 index 00000000000..30947dd27af --- /dev/null +++ b/lib/Exception/AccountDisabledException.php @@ -0,0 +1,14 @@ +request->getParam('documentId'); @@ -104,6 +109,13 @@ private function assertDocumentSession(ISessionAwareController $controller): voi throw new InvalidSessionException(); } + if (!$session->isGuest()) { + $user = $this->userManager->get($session->getUserId()); + if ($user === null || !$user->isEnabled()) { + throw new AccountDisabledException(); + } + } + $document = $this->documentService->getDocument($documentId); if (!$document) { throw new InvalidSessionException(); @@ -187,6 +199,10 @@ public function afterException($controller, $methodName, \Exception $exception): return new JSONResponse(['error' => $this->l10n->t('Editing session has expired. Please reload the page.')], Http::STATUS_PRECONDITION_FAILED); } + if ($exception instanceof AccountDisabledException) { + return new JSONResponse([], Http::STATUS_FORBIDDEN); + } + if ($exception instanceof InvalidSessionException) { return new JSONResponse([], 403); } diff --git a/tests/unit/Middleware/SessionMiddlewareTest.php b/tests/unit/Middleware/SessionMiddlewareTest.php index b5dd0414bfd..e00a7bdb394 100644 --- a/tests/unit/Middleware/SessionMiddlewareTest.php +++ b/tests/unit/Middleware/SessionMiddlewareTest.php @@ -3,10 +3,15 @@ namespace OCA\Text\Tests; use OCA\Text\Controller\ISessionAwareController; +use OCA\Text\Db\Document; +use OCA\Text\Db\Session; +use OCA\Text\Exception\AccountDisabledException; use OCA\Text\Exception\InvalidSessionException; use OCA\Text\Middleware\SessionMiddleware; use OCA\Text\Service\DocumentService; use OCA\Text\Service\SessionService; +use OCP\AppFramework\Http; +use OCP\AppFramework\Http\JSONResponse; use OCP\Constants; use OCP\Files\File; use OCP\Files\Folder; @@ -15,6 +20,7 @@ use OCP\IRequest; use OCP\ISession; use OCP\IUser; +use OCP\IUserManager; use OCP\IUserSession; use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IManager; @@ -28,6 +34,9 @@ class SessionMiddlewareTest extends TestCase { private IUserSession $userSession; private IRootFolder $rootFolder; private IManager $shareManager; + private SessionService $sessionService; + private DocumentService $documentService; + private IUserManager $userManager; protected function setUp(): void { parent::setUp(); @@ -37,16 +46,20 @@ protected function setUp(): void { $this->userSession = $this->createMock(IUserSession::class); $this->rootFolder = $this->createMock(IRootFolder::class); $this->shareManager = $this->createMock(IManager::class); + $this->sessionService = $this->createMock(SessionService::class); + $this->documentService = $this->createMock(DocumentService::class); + $this->userManager = $this->createMock(IUserManager::class); $this->middleware = new SessionMiddleware( $this->request, - $this->createMock(SessionService::class), - $this->createMock(DocumentService::class), + $this->sessionService, + $this->documentService, $this->session, $this->userSession, $this->rootFolder, $this->shareManager, $this->createMock(IL10N::class), + $this->userManager, ); } @@ -138,6 +151,90 @@ public function testLoggedInUserWithValidTokenUnauthenticated(): void { $this->invokeMiddleware($share, $user); } + public function testDocumentSessionWithEnabledUserAllowed(): void { + $session = new Session(); + $session->setUserId('alice'); + + $user = $this->createMock(IUser::class); + $user->method('isEnabled')->willReturn(true); + + $this->sessionService->method('getValidSession')->willReturn($session); + $this->userManager->method('get')->with('alice')->willReturn($user); + $this->documentService->method('getDocument')->willReturn($this->createMock(Document::class)); + + $controller = $this->createMock(ISessionAwareController::class); + $controller->expects($this->once())->method('setUserId')->with('alice'); + + $this->invokeAssertDocumentSession($controller); + $this->assertTrue(true); + } + + public function testDocumentSessionWithDisabledUserBlocked(): void { + $this->expectException(AccountDisabledException::class); + + $session = new Session(); + $session->setUserId('alice'); + + $user = $this->createMock(IUser::class); + $user->method('isEnabled')->willReturn(false); + + $this->sessionService->method('getValidSession')->willReturn($session); + $this->userManager->method('get')->with('alice')->willReturn($user); + + $controller = $this->createMock(ISessionAwareController::class); + $controller->expects($this->never())->method('setUserId'); + + $this->invokeAssertDocumentSession($controller); + } + + public function testDocumentSessionWithNonexistentUserBlocked(): void { + $this->expectException(AccountDisabledException::class); + + $session = new Session(); + $session->setUserId('alice'); + + $this->sessionService->method('getValidSession')->willReturn($session); + $this->userManager->method('get')->with('alice')->willReturn(null); + + $controller = $this->createMock(ISessionAwareController::class); + $controller->expects($this->never())->method('setUserId'); + + $this->invokeAssertDocumentSession($controller); + } + + public function testDocumentSessionGuestSessionSkipsUserCheck(): void { + $session = new Session(); + + $this->sessionService->method('getValidSession')->willReturn($session); + $this->userManager->expects($this->never())->method('get'); + $this->documentService->method('getDocument')->willReturn($this->createMock(Document::class)); + + $controller = $this->createMock(ISessionAwareController::class); + + $this->invokeAssertDocumentSession($controller, 'shareToken123'); + $this->assertTrue(true); + } + + public function testAfterExceptionMapsAccountDisabledToForbidden(): void { + $controller = $this->createMock(ISessionAwareController::class); + + $response = $this->middleware->afterException($controller, 'push', new AccountDisabledException()); + + $this->assertInstanceOf(JSONResponse::class, $response); + $this->assertSame(Http::STATUS_FORBIDDEN, $response->getStatus()); + } + + private function invokeAssertDocumentSession(ISessionAwareController $controller, ?string $shareToken = null): void { + $this->request->method('getParam')->willReturnMap([ + ['documentId', null, 999], + ['sessionId', null, 1], + ['sessionToken', null, 'sessionToken'], + ['token', null, $shareToken], + ]); + + self::invokePrivate($this->middleware, 'assertDocumentSession', [$controller]); + } + private function createPasswordProtectedShare(string $id): IShare { $share = $this->createMock(IShare::class); $share->method('getId')->willReturn($id); From ee7268cfede6a72271877c6f32a831bcb00dd90f Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 4 Aug 2026 08:26:33 +0200 Subject: [PATCH 2/2] chore(autoload): update to include AccountDisabledException Signed-off-by: Max --- composer/composer/autoload_classmap.php | 1 + composer/composer/autoload_static.php | 1 + 2 files changed, 2 insertions(+) diff --git a/composer/composer/autoload_classmap.php b/composer/composer/autoload_classmap.php index b3f1beea632..242f59b6782 100644 --- a/composer/composer/autoload_classmap.php +++ b/composer/composer/autoload_classmap.php @@ -32,6 +32,7 @@ 'OCA\\Text\\DirectEditing\\TextDocumentCreator' => $baseDir . '/../lib/DirectEditing/TextDocumentCreator.php', 'OCA\\Text\\Event\\LoadEditor' => $baseDir . '/../lib/Event/LoadEditor.php', 'OCA\\Text\\Event\\MentionEvent' => $baseDir . '/../lib/Event/MentionEvent.php', + 'OCA\\Text\\Exception\\AccountDisabledException' => $baseDir . '/../lib/Exception/AccountDisabledException.php', 'OCA\\Text\\Exception\\DocumentHasUnsavedChangesException' => $baseDir . '/../lib/Exception/DocumentHasUnsavedChangesException.php', 'OCA\\Text\\Exception\\DocumentSaveConflictException' => $baseDir . '/../lib/Exception/DocumentSaveConflictException.php', 'OCA\\Text\\Exception\\InvalidDocumentBaseVersionEtagException' => $baseDir . '/../lib/Exception/InvalidDocumentBaseVersionEtagException.php', diff --git a/composer/composer/autoload_static.php b/composer/composer/autoload_static.php index 432f8b5d562..d180d2bacbd 100644 --- a/composer/composer/autoload_static.php +++ b/composer/composer/autoload_static.php @@ -47,6 +47,7 @@ class ComposerStaticInitText 'OCA\\Text\\DirectEditing\\TextDocumentCreator' => __DIR__ . '/..' . '/../lib/DirectEditing/TextDocumentCreator.php', 'OCA\\Text\\Event\\LoadEditor' => __DIR__ . '/..' . '/../lib/Event/LoadEditor.php', 'OCA\\Text\\Event\\MentionEvent' => __DIR__ . '/..' . '/../lib/Event/MentionEvent.php', + 'OCA\\Text\\Exception\\AccountDisabledException' => __DIR__ . '/..' . '/../lib/Exception/AccountDisabledException.php', 'OCA\\Text\\Exception\\DocumentHasUnsavedChangesException' => __DIR__ . '/..' . '/../lib/Exception/DocumentHasUnsavedChangesException.php', 'OCA\\Text\\Exception\\DocumentSaveConflictException' => __DIR__ . '/..' . '/../lib/Exception/DocumentSaveConflictException.php', 'OCA\\Text\\Exception\\InvalidDocumentBaseVersionEtagException' => __DIR__ . '/..' . '/../lib/Exception/InvalidDocumentBaseVersionEtagException.php',