Skip to content

Commit e22a9ea

Browse files
mejo-mejo-
authored
Merge pull request #8977 from nextcloud/backport/8951/stable31
[stable31] fix(DirectEditing): check if user is enabled
2 parents bc9190f + efe7df6 commit e22a9ea

5 files changed

Lines changed: 131 additions & 2 deletions

File tree

‎composer/composer/autoload_classmap.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
'OCA\\Text\\DirectEditing\\TextDocumentCreator' => $baseDir . '/../lib/DirectEditing/TextDocumentCreator.php',
3131
'OCA\\Text\\Event\\LoadEditor' => $baseDir . '/../lib/Event/LoadEditor.php',
3232
'OCA\\Text\\Event\\MentionEvent' => $baseDir . '/../lib/Event/MentionEvent.php',
33+
'OCA\\Text\\Exception\\AccountDisabledException' => $baseDir . '/../lib/Exception/AccountDisabledException.php',
3334
'OCA\\Text\\Exception\\DocumentHasUnsavedChangesException' => $baseDir . '/../lib/Exception/DocumentHasUnsavedChangesException.php',
3435
'OCA\\Text\\Exception\\DocumentSaveConflictException' => $baseDir . '/../lib/Exception/DocumentSaveConflictException.php',
3536
'OCA\\Text\\Exception\\InvalidDocumentBaseVersionEtagException' => $baseDir . '/../lib/Exception/InvalidDocumentBaseVersionEtagException.php',

‎composer/composer/autoload_static.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ class ComposerStaticInitText
4545
'OCA\\Text\\DirectEditing\\TextDocumentCreator' => __DIR__ . '/..' . '/../lib/DirectEditing/TextDocumentCreator.php',
4646
'OCA\\Text\\Event\\LoadEditor' => __DIR__ . '/..' . '/../lib/Event/LoadEditor.php',
4747
'OCA\\Text\\Event\\MentionEvent' => __DIR__ . '/..' . '/../lib/Event/MentionEvent.php',
48+
'OCA\\Text\\Exception\\AccountDisabledException' => __DIR__ . '/..' . '/../lib/Exception/AccountDisabledException.php',
4849
'OCA\\Text\\Exception\\DocumentHasUnsavedChangesException' => __DIR__ . '/..' . '/../lib/Exception/DocumentHasUnsavedChangesException.php',
4950
'OCA\\Text\\Exception\\DocumentSaveConflictException' => __DIR__ . '/..' . '/../lib/Exception/DocumentSaveConflictException.php',
5051
'OCA\\Text\\Exception\\InvalidDocumentBaseVersionEtagException' => __DIR__ . '/..' . '/../lib/Exception/InvalidDocumentBaseVersionEtagException.php',

‎lib/Exception/AccountDisabledException.php‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCA\Text\Exception;
11+
12+
class AccountDisabledException extends \Exception {
13+
14+
}

‎lib/Middleware/SessionMiddleware.php‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
use OC\User\NoUserException;
1111
use OCA\Text\Controller\ISessionAwareController;
12+
use OCA\Text\Exception\AccountDisabledException;
1213
use OCA\Text\Exception\InvalidDocumentBaseVersionEtagException;
1314
use OCA\Text\Exception\InvalidSessionException;
1415
use OCA\Text\Middleware\Attribute\RequireDocumentBaseVersionEtag;
@@ -27,6 +28,7 @@
2728
use OCP\IL10N;
2829
use OCP\IRequest;
2930
use OCP\ISession;
31+
use OCP\IUserManager;
3032
use OCP\IUserSession;
3133
use OCP\Share\Exceptions\ShareNotFound;
3234
use OCP\Share\IManager as ShareManager;
@@ -43,13 +45,15 @@ public function __construct(
4345
private IRootFolder $rootFolder,
4446
private ShareManager $shareManager,
4547
private IL10N $l10n,
48+
private IUserManager $userManager,
4649
) {
4750
}
4851

4952
/**
5053
* @throws ReflectionException
5154
* @throws InvalidDocumentBaseVersionEtagException
5255
* @throws InvalidSessionException
56+
* @throws AccountDisabledException
5357
*/
5458
public function beforeController(Controller $controller, string $methodName): void {
5559
if (!$controller instanceof ISessionAwareController) {
@@ -90,6 +94,7 @@ private function assertDocumentBaseVersionEtag(): void {
9094

9195
/**
9296
* @throws InvalidSessionException
97+
* @throws AccountDisabledException
9398
*/
9499
private function assertDocumentSession(ISessionAwareController $controller): void {
95100
$documentId = (int)$this->request->getParam('documentId');
@@ -102,6 +107,13 @@ private function assertDocumentSession(ISessionAwareController $controller): voi
102107
throw new InvalidSessionException();
103108
}
104109

110+
if (!$session->isGuest()) {
111+
$user = $this->userManager->get($session->getUserId());
112+
if ($user === null || !$user->isEnabled()) {
113+
throw new AccountDisabledException();
114+
}
115+
}
116+
105117
$document = $this->documentService->getDocument($documentId);
106118
if (!$document) {
107119
throw new InvalidSessionException();
@@ -174,6 +186,10 @@ public function afterException($controller, $methodName, \Exception $exception):
174186
return new JSONResponse(['error' => $this->l10n->t('Editing session has expired. Please reload the page.')], Http::STATUS_PRECONDITION_FAILED);
175187
}
176188

189+
if ($exception instanceof AccountDisabledException) {
190+
return new JSONResponse([], Http::STATUS_FORBIDDEN);
191+
}
192+
177193
if ($exception instanceof InvalidSessionException) {
178194
return new JSONResponse([], 403);
179195
}

‎tests/unit/Middleware/SessionMiddlewareTest.php‎

Lines changed: 99 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,15 @@
33
namespace OCA\Text\Tests;
44

55
use OCA\Text\Controller\ISessionAwareController;
6+
use OCA\Text\Db\Document;
7+
use OCA\Text\Db\Session;
8+
use OCA\Text\Exception\AccountDisabledException;
69
use OCA\Text\Exception\InvalidSessionException;
710
use OCA\Text\Middleware\SessionMiddleware;
811
use OCA\Text\Service\DocumentService;
912
use OCA\Text\Service\SessionService;
13+
use OCP\AppFramework\Http;
14+
use OCP\AppFramework\Http\JSONResponse;
1015
use OCP\Constants;
1116
use OCP\Files\File;
1217
use OCP\Files\Folder;
@@ -15,6 +20,7 @@
1520
use OCP\IRequest;
1621
use OCP\ISession;
1722
use OCP\IUser;
23+
use OCP\IUserManager;
1824
use OCP\IUserSession;
1925
use OCP\Share\Exceptions\ShareNotFound;
2026
use OCP\Share\IManager;
@@ -28,6 +34,9 @@ class SessionMiddlewareTest extends TestCase {
2834
private IUserSession $userSession;
2935
private IRootFolder $rootFolder;
3036
private IManager $shareManager;
37+
private SessionService $sessionService;
38+
private DocumentService $documentService;
39+
private IUserManager $userManager;
3140

3241
protected function setUp(): void {
3342
parent::setUp();
@@ -37,16 +46,20 @@ protected function setUp(): void {
3746
$this->userSession = $this->createMock(IUserSession::class);
3847
$this->rootFolder = $this->createMock(IRootFolder::class);
3948
$this->shareManager = $this->createMock(IManager::class);
49+
$this->sessionService = $this->createMock(SessionService::class);
50+
$this->documentService = $this->createMock(DocumentService::class);
51+
$this->userManager = $this->createMock(IUserManager::class);
4052

4153
$this->middleware = new SessionMiddleware(
4254
$this->request,
43-
$this->createMock(SessionService::class),
44-
$this->createMock(DocumentService::class),
55+
$this->sessionService,
56+
$this->documentService,
4557
$this->session,
4658
$this->userSession,
4759
$this->rootFolder,
4860
$this->shareManager,
4961
$this->createMock(IL10N::class),
62+
$this->userManager,
5063
);
5164
}
5265

@@ -138,6 +151,90 @@ public function testLoggedInUserWithValidTokenUnauthenticated(): void {
138151
$this->invokeMiddleware($share, $user);
139152
}
140153

154+
public function testDocumentSessionWithEnabledUserAllowed(): void {
155+
$session = new Session();
156+
$session->setUserId('alice');
157+
158+
$user = $this->createMock(IUser::class);
159+
$user->method('isEnabled')->willReturn(true);
160+
161+
$this->sessionService->method('getValidSession')->willReturn($session);
162+
$this->userManager->method('get')->with('alice')->willReturn($user);
163+
$this->documentService->method('getDocument')->willReturn($this->createMock(Document::class));
164+
165+
$controller = $this->createMock(ISessionAwareController::class);
166+
$controller->expects($this->once())->method('setUserId')->with('alice');
167+
168+
$this->invokeAssertDocumentSession($controller);
169+
$this->assertTrue(true);
170+
}
171+
172+
public function testDocumentSessionWithDisabledUserBlocked(): void {
173+
$this->expectException(AccountDisabledException::class);
174+
175+
$session = new Session();
176+
$session->setUserId('alice');
177+
178+
$user = $this->createMock(IUser::class);
179+
$user->method('isEnabled')->willReturn(false);
180+
181+
$this->sessionService->method('getValidSession')->willReturn($session);
182+
$this->userManager->method('get')->with('alice')->willReturn($user);
183+
184+
$controller = $this->createMock(ISessionAwareController::class);
185+
$controller->expects($this->never())->method('setUserId');
186+
187+
$this->invokeAssertDocumentSession($controller);
188+
}
189+
190+
public function testDocumentSessionWithNonexistentUserBlocked(): void {
191+
$this->expectException(AccountDisabledException::class);
192+
193+
$session = new Session();
194+
$session->setUserId('alice');
195+
196+
$this->sessionService->method('getValidSession')->willReturn($session);
197+
$this->userManager->method('get')->with('alice')->willReturn(null);
198+
199+
$controller = $this->createMock(ISessionAwareController::class);
200+
$controller->expects($this->never())->method('setUserId');
201+
202+
$this->invokeAssertDocumentSession($controller);
203+
}
204+
205+
public function testDocumentSessionGuestSessionSkipsUserCheck(): void {
206+
$session = new Session();
207+
208+
$this->sessionService->method('getValidSession')->willReturn($session);
209+
$this->userManager->expects($this->never())->method('get');
210+
$this->documentService->method('getDocument')->willReturn($this->createMock(Document::class));
211+
212+
$controller = $this->createMock(ISessionAwareController::class);
213+
214+
$this->invokeAssertDocumentSession($controller, 'shareToken123');
215+
$this->assertTrue(true);
216+
}
217+
218+
public function testAfterExceptionMapsAccountDisabledToForbidden(): void {
219+
$controller = $this->createMock(ISessionAwareController::class);
220+
221+
$response = $this->middleware->afterException($controller, 'push', new AccountDisabledException());
222+
223+
$this->assertInstanceOf(JSONResponse::class, $response);
224+
$this->assertSame(Http::STATUS_FORBIDDEN, $response->getStatus());
225+
}
226+
227+
private function invokeAssertDocumentSession(ISessionAwareController $controller, ?string $shareToken = null): void {
228+
$this->request->method('getParam')->willReturnMap([
229+
['documentId', null, 999],
230+
['sessionId', null, 1],
231+
['sessionToken', null, 'sessionToken'],
232+
['token', null, $shareToken],
233+
]);
234+
235+
self::invokePrivate($this->middleware, 'assertDocumentSession', [$controller]);
236+
}
237+
141238
private function createPasswordProtectedShare(string $id): IShare {
142239
$share = $this->createMock(IShare::class);
143240
$share->method('getId')->willReturn($id);

0 commit comments

Comments
 (0)