From 79a42d30ec22b7a794216e9e80c2fd2487940496 Mon Sep 17 00:00:00 2001 From: Hoang Pham Date: Tue, 7 May 2024 20:02:44 +0700 Subject: [PATCH 1/7] refactor(session):remove dup queries,refactorcode Signed-off-by: Hoang Pham --- lib/Middleware/SessionMiddleware.php | 212 ++++++++++++++++++--------- 1 file changed, 146 insertions(+), 66 deletions(-) diff --git a/lib/Middleware/SessionMiddleware.php b/lib/Middleware/SessionMiddleware.php index e72646e404d..c4875db3d90 100644 --- a/lib/Middleware/SessionMiddleware.php +++ b/lib/Middleware/SessionMiddleware.php @@ -2,8 +2,11 @@ namespace OCA\Text\Middleware; +use Exception; use OC\User\NoUserException; use OCA\Text\Controller\ISessionAwareController; +use OCA\Text\Db\Document; +use OCA\Text\Db\Session; use OCA\Text\Exception\InvalidDocumentBaseVersionEtagException; use OCA\Text\Exception\InvalidSessionException; use OCA\Text\Middleware\Attribute\RequireDocumentBaseVersionEtag; @@ -16,6 +19,7 @@ use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\Response; use OCP\AppFramework\Middleware; +use OCP\Files\Folder; use OCP\Files\IRootFolder; use OCP\Files\NotPermittedException; use OCP\IL10N; @@ -24,17 +28,20 @@ use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IManager as ShareManager; use ReflectionException; +use ReflectionMethod; class SessionMiddleware extends Middleware { - public function __construct( - private IRequest $request, - private SessionService $sessionService, + private IRequest $request, + private SessionService $sessionService, private DocumentService $documentService, - private IUserSession $userSession, - private IRootFolder $rootFolder, - private ShareManager $shareManager, - private IL10N $l10n, + private IUserSession $userSession, + private IRootFolder $rootFolder, + private ShareManager $shareManager, + private IL10N $l10n, + private ?Document $document = null, + private ?Session $session = null, + private ?string $userId = null, ) { } @@ -48,34 +55,63 @@ public function beforeController(Controller $controller, string $methodName): vo return; } - $reflectionMethod = new \ReflectionMethod($controller, $methodName); + //ASSERTION + $documentId = $this->getDocumentId(); + $this->document = $this->documentService->getDocument($documentId); + + $reflectionMethod = new ReflectionMethod($controller, $methodName); + $requiresDocumentBaseVersionEtag = !empty($reflectionMethod->getAttributes(RequireDocumentBaseVersionEtag::class)); + + if ($requiresDocumentBaseVersionEtag) { + $this->assertDocumentBaseVersionEtag(); + } + + $requiresDocumentSession = !empty($reflectionMethod->getAttributes(RequireDocumentSession::class)); + $requiresDocumentSessionOrUserOrShareToken = !empty($reflectionMethod->getAttributes(RequireDocumentSessionOrUserOrShareToken::class)); + + if (!$requiresDocumentSession && !$requiresDocumentSessionOrUserOrShareToken) { + return; + } + + $this->session = $this->sessionService->getValidSession($documentId, $this->getSessionId(), $this->getSessionToken()); + + try { + $this->assertDocumentSession(); - if (!empty($reflectionMethod->getAttributes(RequireDocumentSessionOrUserOrShareToken::class))) { - try { - $this->assertDocumentSession($controller); - } catch (InvalidSessionException) { - $this->assertUserOrShareToken($controller); + if (!$this->getShareToken()) { + $this->userId = $this->session->getUserId(); } + } catch (InvalidSessionException) { + if (!$requiresDocumentSessionOrUserOrShareToken) { + throw new InvalidSessionException(); + } + + $this->assertUserOrShareToken(); } - if (!empty($reflectionMethod->getAttributes(RequireDocumentBaseVersionEtag::class))) { - $this->assertDocumentBaseVersionEtag(); + //OTHERS + $this->setControllerData($controller); + } + + public function afterException($controller, $methodName, Exception $exception): JSONResponse|Response { + if ($exception instanceof InvalidDocumentBaseVersionEtagException) { + return new JSONResponse(['error' => $this->l10n->t('Editing session has expired. Please reload the page.')], Http::STATUS_PRECONDITION_FAILED); } - if (!empty($reflectionMethod->getAttributes(RequireDocumentSession::class))) { - $this->assertDocumentSession($controller); + if ($exception instanceof InvalidSessionException) { + return new JSONResponse([], 403); } + + return parent::afterException($controller, $methodName, $exception); } /** * @throws InvalidDocumentBaseVersionEtagException */ private function assertDocumentBaseVersionEtag(): void { - $documentId = (int)$this->request->getParam('documentId'); - $baseVersionEtag = $this->request->getParam('baseVersionEtag'); + $baseVersionEtag = $this->getBaseVersionEtag(); - $document = $this->documentService->getDocument($documentId); - if ($baseVersionEtag && $document?->getBaseVersionEtag() !== $baseVersionEtag) { + if ($baseVersionEtag && $this->document?->getBaseVersionEtag() !== $baseVersionEtag) { throw new InvalidDocumentBaseVersionEtagException(); } } @@ -83,73 +119,117 @@ private function assertDocumentBaseVersionEtag(): void { /** * @throws InvalidSessionException */ - private function assertDocumentSession(ISessionAwareController $controller): void { - $documentId = (int)$this->request->getParam('documentId'); - $sessionId = (int)$this->request->getParam('sessionId'); - $token = (string)$this->request->getParam('sessionToken'); - $shareToken = (string)$this->request->getParam('token'); - - $session = $this->sessionService->getValidSession($documentId, $sessionId, $token); - if (!$session) { + private function assertDocumentSession(): void { + if (!$this->document || !$this->session) { throw new InvalidSessionException(); } + } - $document = $this->documentService->getDocument($documentId); - if (!$document) { + + /** + * @throws InvalidSessionException + */ + private function assertUserOrShareToken(): void { + if (!$this->document) { throw new InvalidSessionException(); } - $controller->setSession($session); - $controller->setDocument($document); - if (!$shareToken) { - $controller->setUserId($session->getUserId()); + $documentId = $this->getDocumentId(); + + if ($userId = $this->getSessionUserId()) { + $this->assertUserHasAccessToDocument($userId, $documentId); + + $this->userId = $userId; + + return; + } + + if ($shareToken = $this->getShareToken()) { + $this->assertShareTokenHasAccessToDocument($shareToken, $documentId); + + return; } + + throw new InvalidSessionException(); } /** - * @throws NotPermittedException - * @throws NoUserException * @throws InvalidSessionException */ - private function assertUserOrShareToken(ISessionAwareController $controller): void { - $documentId = (int)$this->request->getParam('documentId'); - if (null !== $userId = $this->userSession->getUser()?->getUID()) { - // Check if user has access to document - if (count($this->rootFolder->getUserFolder($userId)->getById($documentId)) === 0) { - throw new InvalidSessionException(); - } - $controller->setUserId($userId); - } elseif ('' !== $shareToken = (string)$this->request->getParam('shareToken')) { - try { - $share = $this->shareManager->getShareByToken($shareToken); - } catch (ShareNotFound) { - throw new InvalidSessionException(); - } - // Check if shareToken has access to document - if (count($this->rootFolder->getUserFolder($share->getShareOwner())->getById($documentId)) === 0) { - throw new InvalidSessionException(); - } - } else { + private function assertUserHasAccessToDocument(string $userId, int $documentId): void { + try { + $userFolder = $this->getUserFolder($userId); + } catch (NoUserException|NotPermittedException) { throw new InvalidSessionException(); } - $document = $this->documentService->getDocument($documentId); - if (!$document) { + if (count($userFolder->getById($documentId)) === 0) { throw new InvalidSessionException(); } - - $controller->setDocument($document); } - public function afterException($controller, $methodName, \Exception $exception): JSONResponse|Response { - if ($exception instanceof InvalidDocumentBaseVersionEtagException) { - return new JSONResponse(['error' => $this->l10n->t('Editing session has expired. Please reload the page.')], Http::STATUS_PRECONDITION_FAILED); + /** + * @throws InvalidSessionException + */ + private function assertShareTokenHasAccessToDocument(string $shareToken, int $documentId): void { + try { + $share = $this->shareManager->getShareByToken($shareToken); + } catch (ShareNotFound) { + throw new InvalidSessionException(); } - if ($exception instanceof InvalidSessionException) { - return new JSONResponse([], 403); + try { + $userFolder = $this->getUserFolder($share->getShareOwner()); + } catch (NoUserException|NotPermittedException) { + throw new InvalidSessionException(); } - return parent::afterException($controller, $methodName, $exception); + if (count($userFolder->getById($documentId)) === 0) { + throw new InvalidSessionException(); + } + } + + private function getDocumentId(): int { + return (int)$this->request->getParam('documentId'); + } + + private function getSessionId(): int { + return (int)$this->request->getParam('sessionId'); + } + + private function getSessionToken(): string { + return (string)$this->request->getParam('sessionToken'); + } + + private function getShareToken(): string { + return (string)$this->request->getParam('token'); + } + + private function getBaseVersionEtag(): string { + return (string)$this->request->getParam('baseVersionEtag'); + } + + private function getSessionUserId(): ?string { + return $this->userSession->getUser()?->getUID(); + } + + private function setControllerData(ISessionAwareController $controller): void { + if ($this->document) { + $controller->setDocument($this->document); + } + if ($this->session) { + $controller->setSession($this->session); + } + if ($this->userId) { + $controller->setUserId($this->userId); + } + } + + /** + * @throws NotPermittedException + * @throws NoUserException + */ + private function getUserFolder(string $userId): Folder { + return $this->rootFolder->getUserFolder($userId); } } From d307b7c9c4ab146760ecc82d9ad299ef01c9bf01 Mon Sep 17 00:00:00 2001 From: Hoang Pham Date: Tue, 7 May 2024 20:22:34 +0700 Subject: [PATCH 2/7] refactor(session):remove dup queries,refactorcode Signed-off-by: Hoang Pham --- lib/Middleware/SessionMiddleware.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Middleware/SessionMiddleware.php b/lib/Middleware/SessionMiddleware.php index c4875db3d90..75ee63dca63 100644 --- a/lib/Middleware/SessionMiddleware.php +++ b/lib/Middleware/SessionMiddleware.php @@ -136,7 +136,7 @@ private function assertUserOrShareToken(): void { $documentId = $this->getDocumentId(); - if ($userId = $this->getSessionUserId()) { + if (null !== ($userId = $this->getSessionUserId())) { $this->assertUserHasAccessToDocument($userId, $documentId); $this->userId = $userId; @@ -220,7 +220,7 @@ private function setControllerData(ISessionAwareController $controller): void { if ($this->session) { $controller->setSession($this->session); } - if ($this->userId) { + if (null !== $this->userId) { $controller->setUserId($this->userId); } } From 8b177957d918bc3aace2d6af2040386c61fcaaf9 Mon Sep 17 00:00:00 2001 From: Hoang Pham Date: Tue, 7 May 2024 20:29:38 +0700 Subject: [PATCH 3/7] refactor(session):remove dup queries,refactorcode Signed-off-by: Hoang Pham --- lib/Middleware/SessionMiddleware.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Middleware/SessionMiddleware.php b/lib/Middleware/SessionMiddleware.php index 75ee63dca63..0dcad8b6918 100644 --- a/lib/Middleware/SessionMiddleware.php +++ b/lib/Middleware/SessionMiddleware.php @@ -220,7 +220,7 @@ private function setControllerData(ISessionAwareController $controller): void { if ($this->session) { $controller->setSession($this->session); } - if (null !== $this->userId) { + if ($this->userId !== null) { $controller->setUserId($this->userId); } } From 10c1d96fb2412294f6ccc42b5632e5e6135252fb Mon Sep 17 00:00:00 2001 From: Hoang Pham Date: Wed, 8 May 2024 19:40:36 +0700 Subject: [PATCH 4/7] refactor(session):remove dup queries,refactorcode Signed-off-by: Hoang Pham --- lib/Middleware/SessionMiddleware.php | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/lib/Middleware/SessionMiddleware.php b/lib/Middleware/SessionMiddleware.php index 0dcad8b6918..8fdabefc06d 100644 --- a/lib/Middleware/SessionMiddleware.php +++ b/lib/Middleware/SessionMiddleware.php @@ -78,7 +78,7 @@ public function beforeController(Controller $controller, string $methodName): vo try { $this->assertDocumentSession(); - if (!$this->getShareToken()) { + if (!$this->getToken()) { $this->userId = $this->session->getUserId(); } } catch (InvalidSessionException) { @@ -90,7 +90,9 @@ public function beforeController(Controller $controller, string $methodName): vo } //OTHERS - $this->setControllerData($controller); + $controller->setDocument($this->document); + $controller->setSession($this->session); + $controller->setUserId($this->userId); } public function afterException($controller, $methodName, Exception $exception): JSONResponse|Response { @@ -201,10 +203,14 @@ private function getSessionToken(): string { return (string)$this->request->getParam('sessionToken'); } - private function getShareToken(): string { + private function getToken(): string { return (string)$this->request->getParam('token'); } + private function getShareToken(): ?string { + return $this->request->getParam('shareToken'); + } + private function getBaseVersionEtag(): string { return (string)$this->request->getParam('baseVersionEtag'); } @@ -213,18 +219,6 @@ private function getSessionUserId(): ?string { return $this->userSession->getUser()?->getUID(); } - private function setControllerData(ISessionAwareController $controller): void { - if ($this->document) { - $controller->setDocument($this->document); - } - if ($this->session) { - $controller->setSession($this->session); - } - if ($this->userId !== null) { - $controller->setUserId($this->userId); - } - } - /** * @throws NotPermittedException * @throws NoUserException From 26963519febc7b139eaacd421abd16217049e739 Mon Sep 17 00:00:00 2001 From: Hoang Pham Date: Wed, 8 May 2024 19:46:40 +0700 Subject: [PATCH 5/7] refactor(session):remove dup queries,refactorcode Signed-off-by: Hoang Pham --- lib/Middleware/SessionMiddleware.php | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/lib/Middleware/SessionMiddleware.php b/lib/Middleware/SessionMiddleware.php index 8fdabefc06d..e63734c390d 100644 --- a/lib/Middleware/SessionMiddleware.php +++ b/lib/Middleware/SessionMiddleware.php @@ -90,9 +90,7 @@ public function beforeController(Controller $controller, string $methodName): vo } //OTHERS - $controller->setDocument($this->document); - $controller->setSession($this->session); - $controller->setUserId($this->userId); + $this->setControllerData($controller); } public function afterException($controller, $methodName, Exception $exception): JSONResponse|Response { @@ -226,4 +224,16 @@ private function getSessionUserId(): ?string { private function getUserFolder(string $userId): Folder { return $this->rootFolder->getUserFolder($userId); } + + private function setControllerData(ISessionAwareController $controller): void { + if ($this->document) { + $controller->setDocument($this->document); + } + if ($this->session) { + $controller->setSession($this->session); + } + if ($this->userId !== null) { + $controller->setUserId($this->userId); + } + } } From e29b50fbdc2776cb3f86c0cade41461a9512173e Mon Sep 17 00:00:00 2001 From: Hoang Pham Date: Wed, 8 May 2024 19:50:08 +0700 Subject: [PATCH 6/7] refactor(session):remove dup queries,refactorcode Signed-off-by: Hoang Pham --- lib/Middleware/SessionMiddleware.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Middleware/SessionMiddleware.php b/lib/Middleware/SessionMiddleware.php index e63734c390d..6a673b29239 100644 --- a/lib/Middleware/SessionMiddleware.php +++ b/lib/Middleware/SessionMiddleware.php @@ -144,7 +144,7 @@ private function assertUserOrShareToken(): void { return; } - if ($shareToken = $this->getShareToken()) { + if (null !== ($shareToken = $this->getShareToken())) { $this->assertShareTokenHasAccessToDocument($shareToken, $documentId); return; @@ -232,7 +232,7 @@ private function setControllerData(ISessionAwareController $controller): void { if ($this->session) { $controller->setSession($this->session); } - if ($this->userId !== null) { + if (null !== $this->userId) { $controller->setUserId($this->userId); } } From f0c0aadf1add4135d641726f5257e8396c717033 Mon Sep 17 00:00:00 2001 From: Hoang Pham Date: Wed, 8 May 2024 20:36:02 +0700 Subject: [PATCH 7/7] refactor(session):remove dup queries,refactorcode Signed-off-by: Hoang Pham --- lib/Middleware/SessionMiddleware.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Middleware/SessionMiddleware.php b/lib/Middleware/SessionMiddleware.php index 6a673b29239..0f5db0831ac 100644 --- a/lib/Middleware/SessionMiddleware.php +++ b/lib/Middleware/SessionMiddleware.php @@ -232,7 +232,7 @@ private function setControllerData(ISessionAwareController $controller): void { if ($this->session) { $controller->setSession($this->session); } - if (null !== $this->userId) { + if ($this->userId !== null) { $controller->setUserId($this->userId); } }