From f039d1b33b6f60116329339680273f9ca54e579c Mon Sep 17 00:00:00 2001 From: skjnldsv Date: Thu, 1 Aug 2024 22:28:08 +0200 Subject: [PATCH] fix(files_sharing): show proper share not found error message Signed-off-by: skjnldsv --- .../lib/Controller/ShareController.php | 8 ++--- .../files_sharing/templates/sharenotfound.php | 19 ++++++++++++ .../DependencyInjection/DIContainer.php | 2 +- .../PublicShare/PublicShareMiddleware.php | 30 ++++++++----------- .../PublicShare/PublicShareMiddlewareTest.php | 6 ++-- 5 files changed, 40 insertions(+), 25 deletions(-) create mode 100644 apps/files_sharing/templates/sharenotfound.php diff --git a/apps/files_sharing/lib/Controller/ShareController.php b/apps/files_sharing/lib/Controller/ShareController.php index eef2f5f32bc75..642065c99832b 100644 --- a/apps/files_sharing/lib/Controller/ShareController.php +++ b/apps/files_sharing/lib/Controller/ShareController.php @@ -294,11 +294,11 @@ public function showShare($path = ''): TemplateResponse { } catch (ShareNotFound $e) { // The share does not exists, we do not emit an ShareLinkAccessedEvent $this->emitAccessShareHook($this->getToken(), 404, 'Share not found'); - throw new NotFoundException(); + throw new NotFoundException($this->l10n->t('This share does not exist or is no longer available')); } if (!$this->validateShare($share)) { - throw new NotFoundException(); + throw new NotFoundException($this->l10n->t('This share does not exist or is no longer available')); } $shareNode = $share->getNode(); @@ -309,7 +309,7 @@ public function showShare($path = ''): TemplateResponse { } catch (NotFoundException $e) { $this->emitAccessShareHook($share, 404, 'Share not found'); $this->emitShareAccessEvent($share, ShareController::SHARE_ACCESS, 404, 'Share not found'); - throw new NotFoundException(); + throw new NotFoundException($this->l10n->t('This share does not exist or is no longer available')); } // We can't get the path of a file share @@ -317,7 +317,7 @@ public function showShare($path = ''): TemplateResponse { if ($shareNode instanceof \OCP\Files\File && $path !== '') { $this->emitAccessShareHook($share, 404, 'Share not found'); $this->emitShareAccessEvent($share, self::SHARE_ACCESS, 404, 'Share not found'); - throw new NotFoundException(); + throw new NotFoundException($this->l10n->t('This share does not exist or is no longer available')); } } catch (\Exception $e) { $this->emitAccessShareHook($share, 404, 'Share not found'); diff --git a/apps/files_sharing/templates/sharenotfound.php b/apps/files_sharing/templates/sharenotfound.php new file mode 100644 index 0000000000000..e3163e63c2ed9 --- /dev/null +++ b/apps/files_sharing/templates/sharenotfound.php @@ -0,0 +1,19 @@ + + diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php index 81365900d9b13..208031b942fea 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -288,7 +288,7 @@ public function __construct(string $appName, array $urlParams = [], ?ServerConta new OC\AppFramework\Middleware\PublicShare\PublicShareMiddleware( $c->get(IRequest::class), $c->get(ISession::class), - $c->get(\OCP\IConfig::class), + $c->get(IConfig::class), $c->get(IThrottler::class) ) ); diff --git a/lib/private/AppFramework/Middleware/PublicShare/PublicShareMiddleware.php b/lib/private/AppFramework/Middleware/PublicShare/PublicShareMiddleware.php index 41cba1aacd37e..34291dfef100f 100644 --- a/lib/private/AppFramework/Middleware/PublicShare/PublicShareMiddleware.php +++ b/lib/private/AppFramework/Middleware/PublicShare/PublicShareMiddleware.php @@ -6,8 +6,10 @@ namespace OC\AppFramework\Middleware\PublicShare; use OC\AppFramework\Middleware\PublicShare\Exceptions\NeedAuthenticationException; +use OCA\Files_Sharing\AppInfo\Application; use OCP\AppFramework\AuthPublicShareController; -use OCP\AppFramework\Http\NotFoundResponse; +use OCP\AppFramework\Http; +use OCP\AppFramework\Http\TemplateResponse; use OCP\AppFramework\Middleware; use OCP\AppFramework\PublicShareController; use OCP\Files\NotFoundException; @@ -17,23 +19,13 @@ use OCP\Security\Bruteforce\IThrottler; class PublicShareMiddleware extends Middleware { - /** @var IRequest */ - private $request; - /** @var ISession */ - private $session; - - /** @var IConfig */ - private $config; - - /** @var IThrottler */ - private $throttler; - - public function __construct(IRequest $request, ISession $session, IConfig $config, IThrottler $throttler) { - $this->request = $request; - $this->session = $session; - $this->config = $config; - $this->throttler = $throttler; + public function __construct( + private IRequest $request, + private ISession $session, + private IConfig $config, + private IThrottler $throttler + ) { } public function beforeController($controller, $methodName) { @@ -92,7 +84,9 @@ public function afterException($controller, $methodName, \Exception $exception) } if ($exception instanceof NotFoundException) { - return new NotFoundResponse(); + return new TemplateResponse(Application::APP_ID, 'sharenotfound', [ + 'message' => $exception->getMessage(), + ], 'guest', Http::STATUS_NOT_FOUND); } if ($controller instanceof AuthPublicShareController && $exception instanceof NeedAuthenticationException) { diff --git a/tests/lib/AppFramework/Middleware/PublicShare/PublicShareMiddlewareTest.php b/tests/lib/AppFramework/Middleware/PublicShare/PublicShareMiddlewareTest.php index 03cd253044de6..64a9efbfe60b4 100644 --- a/tests/lib/AppFramework/Middleware/PublicShare/PublicShareMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/PublicShare/PublicShareMiddlewareTest.php @@ -10,8 +10,9 @@ use OC\AppFramework\Middleware\PublicShare\PublicShareMiddleware; use OCP\AppFramework\AuthPublicShareController; use OCP\AppFramework\Controller; -use OCP\AppFramework\Http\NotFoundResponse; +use OCP\AppFramework\Http; use OCP\AppFramework\Http\RedirectResponse; +use OCP\AppFramework\Http\TemplateResponse; use OCP\AppFramework\PublicShareController; use OCP\Files\NotFoundException; use OCP\IConfig; @@ -236,7 +237,8 @@ public function testAfterExceptionPublicShareControllerNotFoundException() { $exception = new NotFoundException(); $result = $this->middleware->afterException($controller, 'method', $exception); - $this->assertInstanceOf(NotFoundResponse::class, $result); + $this->assertInstanceOf(TemplateResponse::class, $result); + $this->assertEquals($result->getStatus(), Http::STATUS_NOT_FOUND); } public function testAfterExceptionPublicShareController() {