Skip to content

Commit

Permalink
fix(files_sharing): show proper share not found error message
Browse files Browse the repository at this point in the history
Signed-off-by: skjnldsv <[email protected]>
  • Loading branch information
skjnldsv committed Aug 2, 2024
1 parent 014fcb0 commit f039d1b
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 25 deletions.
8 changes: 4 additions & 4 deletions apps/files_sharing/lib/Controller/ShareController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -309,15 +309,15 @@ 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
try {
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');
Expand Down
19 changes: 19 additions & 0 deletions apps/files_sharing/templates/sharenotfound.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?php
/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-only
*/

use OCP\IURLGenerator;
use OCP\Server;

$urlGenerator = Server::get(IURLGenerator::class);
?>
<div class="body-login-container update">
<div class="icon-big icon-share"></div>
<h2><?php p($l->t('Share not found')); ?></h2>
<p class="infogroup"><?php p($_['message'] ?: $l->t('This share does not exist or is no longer available')); ?></p>
<p><a class="button primary" href="<?php p($urlGenerator->linkTo('', 'index.php')) ?>">
<?php p($l->t('Back to %s', [$theme->getName()])); ?>
</a></p>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -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)
)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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() {
Expand Down

0 comments on commit f039d1b

Please sign in to comment.