From 7445ec7b040df3b4a78d525bc523961b2b87d9ce Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Tue, 20 Oct 2026 19:13:02 -0400 Subject: [PATCH] feat(sharing): dispatch an event when a share link access fails Add ShareLinkAccessFailedEvent, dispatched when a public share link is opened with a token that does not exist or with a wrong share password, on the share page and over public WebDAV (both public.php/dav and the legacy public.php/webdav). It is dispatched next to the existing brute-force attempt registration, so apps can recognise someone guessing share tokens or share passwords across addresses. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Frank Karlitschek --- apps/dav/lib/Connector/LegacyPublicAuth.php | 6 ++ apps/dav/lib/Connector/Sabre/PublicAuth.php | 7 ++ .../unit/Connector/LegacyPublicAuthTest.php | 18 +++++ .../unit/Connector/Sabre/PublicAuthTest.php | 19 +++++ .../lib/Controller/ShareController.php | 7 ++ .../tests/Controller/ShareControllerTest.php | 37 ++++++++++ lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + .../Events/ShareLinkAccessFailedEvent.php | 69 +++++++++++++++++++ 9 files changed, 165 insertions(+) create mode 100644 lib/public/Share/Events/ShareLinkAccessFailedEvent.php diff --git a/apps/dav/lib/Connector/LegacyPublicAuth.php b/apps/dav/lib/Connector/LegacyPublicAuth.php index 32b69cccab534..1d1dd16dcd4d4 100644 --- a/apps/dav/lib/Connector/LegacyPublicAuth.php +++ b/apps/dav/lib/Connector/LegacyPublicAuth.php @@ -10,10 +10,12 @@ use OCA\DAV\Connector\Sabre\PublicAuth; use OCP\Defaults; +use OCP\EventDispatcher\IEventDispatcher; use OCP\IRequest; use OCP\ISession; use OCP\IUserSession; use OCP\Security\Bruteforce\IThrottler; +use OCP\Share\Events\ShareLinkAccessFailedEvent; use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IManager; use OCP\Share\IShare; @@ -36,6 +38,7 @@ public function __construct( private ISession $session, private IThrottler $throttler, private IUserSession $userSession, + private IEventDispatcher $eventDispatcher, ) { // setup realm $defaults = new Defaults(); @@ -62,6 +65,7 @@ protected function validateUserPass($username, $password) { $share = $this->shareManager->getShareByToken($username); } catch (ShareNotFound $e) { $this->throttler->registerAttempt(self::BRUTEFORCE_ACTION, $this->request->getRemoteAddress()); + $this->eventDispatcher->dispatchTyped(new ShareLinkAccessFailedEvent($username, null, ShareLinkAccessFailedEvent::REASON_NOT_FOUND)); return false; } @@ -88,6 +92,8 @@ protected function validateUserPass($username, $password) { return true; } + $this->eventDispatcher->dispatchTyped(new ShareLinkAccessFailedEvent($username, $share, ShareLinkAccessFailedEvent::REASON_WRONG_PASSWORD)); + if (in_array('XMLHttpRequest', explode(',', $this->request->getHeader('X-Requested-With')), true)) { // do not re-authenticate over ajax, use dummy auth name to prevent browser popup http_response_code(401); diff --git a/apps/dav/lib/Connector/Sabre/PublicAuth.php b/apps/dav/lib/Connector/Sabre/PublicAuth.php index 4649a9a00f40c..c400a5c4faa65 100644 --- a/apps/dav/lib/Connector/Sabre/PublicAuth.php +++ b/apps/dav/lib/Connector/Sabre/PublicAuth.php @@ -11,12 +11,14 @@ namespace OCA\DAV\Connector\Sabre; use OCP\Defaults; +use OCP\EventDispatcher\IEventDispatcher; use OCP\IRequest; use OCP\ISession; use OCP\IURLGenerator; use OCP\IUserSession; use OCP\Security\Bruteforce\IThrottler; use OCP\Security\Bruteforce\MaxDelayReached; +use OCP\Share\Events\ShareLinkAccessFailedEvent; use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IManager; use OCP\Share\IShare; @@ -49,6 +51,7 @@ public function __construct( private LoggerInterface $logger, private IURLGenerator $urlGenerator, private IUserSession $userSession, + private IEventDispatcher $eventDispatcher, ) { // setup realm $defaults = new Defaults(); @@ -132,6 +135,7 @@ private function checkToken(): array { $share = $this->shareManager->getShareByToken($token); } catch (ShareNotFound $e) { $this->throttler->registerAttempt(self::BRUTEFORCE_ACTION, $this->request->getRemoteAddress()); + $this->eventDispatcher->dispatchTyped(new ShareLinkAccessFailedEvent($token, null, ShareLinkAccessFailedEvent::REASON_NOT_FOUND)); throw new NotFound(); } @@ -172,6 +176,7 @@ protected function validateUserPass($username, $password) { $share = $this->getShare(); } catch (ShareNotFound $e) { $this->throttler->registerAttempt(self::BRUTEFORCE_ACTION, $this->request->getRemoteAddress()); + $this->eventDispatcher->dispatchTyped(new ShareLinkAccessFailedEvent($this->getToken(), null, ShareLinkAccessFailedEvent::REASON_NOT_FOUND)); return false; } @@ -196,6 +201,8 @@ protected function validateUserPass($username, $password) { return true; } + $this->eventDispatcher->dispatchTyped(new ShareLinkAccessFailedEvent($this->getToken(), $share, ShareLinkAccessFailedEvent::REASON_WRONG_PASSWORD)); + if (in_array('XMLHttpRequest', explode(',', $this->request->getHeader('X-Requested-With')), true)) { // do not re-authenticate over ajax, use dummy auth name to prevent browser popup http_response_code(401); diff --git a/apps/dav/tests/unit/Connector/LegacyPublicAuthTest.php b/apps/dav/tests/unit/Connector/LegacyPublicAuthTest.php index abcd993ce3ec8..724d91afe5cd6 100644 --- a/apps/dav/tests/unit/Connector/LegacyPublicAuthTest.php +++ b/apps/dav/tests/unit/Connector/LegacyPublicAuthTest.php @@ -10,10 +10,12 @@ namespace OCA\DAV\Tests\unit\Connector; use OCA\DAV\Connector\LegacyPublicAuth; +use OCP\EventDispatcher\IEventDispatcher; use OCP\IRequest; use OCP\ISession; use OCP\IUserSession; use OCP\Security\Bruteforce\IThrottler; +use OCP\Share\Events\ShareLinkAccessFailedEvent; use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IManager; use OCP\Share\IShare; @@ -28,6 +30,7 @@ class LegacyPublicAuthTest extends TestCase { private IManager&MockObject $shareManager; private IThrottler&MockObject $throttler; private IUserSession&MockObject $userSession; + private IEventDispatcher&MockObject $eventDispatcher; private LegacyPublicAuth $auth; private string|false $oldUser; @@ -39,6 +42,7 @@ protected function setUp(): void { $this->shareManager = $this->createMock(IManager::class); $this->throttler = $this->createMock(IThrottler::class); $this->userSession = $this->createMock(IUserSession::class); + $this->eventDispatcher = $this->createMock(IEventDispatcher::class); $this->auth = new LegacyPublicAuth( $this->request, @@ -46,6 +50,7 @@ protected function setUp(): void { $this->session, $this->throttler, $this->userSession, + $this->eventDispatcher, ); // Store current user @@ -68,6 +73,7 @@ public function testNoShare(): void { $this->shareManager->expects($this->once()) ->method('getShareByToken') ->willThrowException(new ShareNotFound()); + $this->expectFailureEvent('username', null, ShareLinkAccessFailedEvent::REASON_NOT_FOUND); $result = $this->invokePrivate($this->auth, 'validateUserPass', ['username', 'password']); @@ -133,6 +139,7 @@ public function testSharePasswordLinkValidPassword(): void { $this->equalTo($share), $this->equalTo('password') )->willReturn(true); + $this->eventDispatcher->expects($this->never())->method('dispatchTyped'); $result = $this->invokePrivate($this->auth, 'validateUserPass', ['username', 'password']); @@ -154,6 +161,7 @@ public function testSharePasswordMailValidPassword(): void { $this->equalTo($share), $this->equalTo('password') )->willReturn(true); + $this->eventDispatcher->expects($this->never())->method('dispatchTyped'); $result = $this->invokePrivate($this->auth, 'validateUserPass', ['username', 'password']); @@ -204,6 +212,7 @@ public function testSharePasswordLinkInvalidSession(): void { $this->session->method('exists')->with('public_link_authenticated')->willReturn(true); $this->session->method('get')->with('public_link_authenticated')->willReturn(['43']); + $this->expectFailureEvent('username', $share, ShareLinkAccessFailedEvent::REASON_WRONG_PASSWORD); $result = $this->invokePrivate($this->auth, 'validateUserPass', ['username', 'password']); @@ -229,9 +238,18 @@ public function testSharePasswordMailInvalidSession(): void { $this->session->method('exists')->with('public_link_authenticated')->willReturn(true); $this->session->method('get')->with('public_link_authenticated')->willReturn(['43']); + $this->expectFailureEvent('username', $share, ShareLinkAccessFailedEvent::REASON_WRONG_PASSWORD); $result = $this->invokePrivate($this->auth, 'validateUserPass', ['username', 'password']); $this->assertFalse($result); } + + private function expectFailureEvent(string $token, ?IShare $share, string $reason): void { + $this->eventDispatcher->expects($this->once()) + ->method('dispatchTyped') + ->with($this->callback(static fn (ShareLinkAccessFailedEvent $event): bool => $event->getToken() === $token + && $event->getShare() === $share + && $event->getReason() === $reason)); + } } diff --git a/apps/dav/tests/unit/Connector/Sabre/PublicAuthTest.php b/apps/dav/tests/unit/Connector/Sabre/PublicAuthTest.php index e76fc347d9359..ad6a50589c2bb 100644 --- a/apps/dav/tests/unit/Connector/Sabre/PublicAuthTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/PublicAuthTest.php @@ -10,11 +10,13 @@ namespace OCA\DAV\Tests\unit\Connector; use OCA\DAV\Connector\Sabre\PublicAuth; +use OCP\EventDispatcher\IEventDispatcher; use OCP\IRequest; use OCP\ISession; use OCP\IURLGenerator; use OCP\IUserSession; use OCP\Security\Bruteforce\IThrottler; +use OCP\Share\Events\ShareLinkAccessFailedEvent; use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IManager; use OCP\Share\IShare; @@ -37,6 +39,7 @@ class PublicAuthTest extends \Test\TestCase { private LoggerInterface&MockObject $logger; private IURLGenerator&MockObject $urlGenerator; private IUserSession&MockObject $userSession; + private IEventDispatcher&MockObject $eventDispatcher; private PublicAuth $auth; private bool|string $oldUser; @@ -51,6 +54,7 @@ protected function setUp(): void { $this->logger = $this->createMock(LoggerInterface::class); $this->urlGenerator = $this->createMock(IURLGenerator::class); $this->userSession = $this->createMock(IUserSession::class); + $this->eventDispatcher = $this->createMock(IEventDispatcher::class); $this->auth = new PublicAuth( $this->request, @@ -60,6 +64,7 @@ protected function setUp(): void { $this->logger, $this->urlGenerator, $this->userSession, + $this->eventDispatcher, ); // Store current user @@ -121,6 +126,7 @@ public function testCheckTokenInvalidShare(): void { ->method('getShareByToken') ->with('GX9HSGQrGE') ->willThrowException(new ShareNotFound()); + $this->expectFailureEvent('GX9HSGQrGE', null, ShareLinkAccessFailedEvent::REASON_NOT_FOUND); $this->expectException(\Sabre\DAV\Exception\NotFound::class); self::invokePrivate($this->auth, 'checkToken'); @@ -194,6 +200,7 @@ public function testNoShare(): void { ->method('getShareByToken') ->with('GX9HSGQrGE') ->willThrowException(new ShareNotFound()); + $this->expectFailureEvent('GX9HSGQrGE', null, ShareLinkAccessFailedEvent::REASON_NOT_FOUND); $result = self::invokePrivate($this->auth, 'validateUserPass', ['username', 'password']); @@ -275,6 +282,7 @@ public function testSharePasswordLinkValidPassword(): void { $this->equalTo($share), $this->equalTo('password') )->willReturn(true); + $this->eventDispatcher->expects($this->never())->method('dispatchTyped'); $result = self::invokePrivate($this->auth, 'validateUserPass', ['username', 'password']); @@ -300,6 +308,7 @@ public function testSharePasswordMailValidPassword(): void { $this->equalTo($share), $this->equalTo('password') )->willReturn(true); + $this->eventDispatcher->expects($this->never())->method('dispatchTyped'); $result = self::invokePrivate($this->auth, 'validateUserPass', ['username', 'password']); @@ -360,6 +369,7 @@ public function testSharePasswordLinkInvalidSession(): void { $this->session->method('exists')->with('public_link_authenticated')->willReturn(true); $this->session->method('get')->with('public_link_authenticated')->willReturn('43'); + $this->expectFailureEvent('GX9HSGQrGE', $share, ShareLinkAccessFailedEvent::REASON_WRONG_PASSWORD); $result = self::invokePrivate($this->auth, 'validateUserPass', ['username', 'password']); @@ -390,9 +400,18 @@ public function testSharePasswordMailInvalidSession(): void { $this->session->method('exists')->with('public_link_authenticated')->willReturn(true); $this->session->method('get')->with('public_link_authenticated')->willReturn('43'); + $this->expectFailureEvent('GX9HSGQrGE', $share, ShareLinkAccessFailedEvent::REASON_WRONG_PASSWORD); $result = self::invokePrivate($this->auth, 'validateUserPass', ['username', 'password']); $this->assertFalse($result); } + + private function expectFailureEvent(string $token, ?IShare $share, string $reason): void { + $this->eventDispatcher->expects($this->once()) + ->method('dispatchTyped') + ->with($this->callback(static fn (ShareLinkAccessFailedEvent $event): bool => $event->getToken() === $token + && $event->getShare() === $share + && $event->getReason() === $reason)); + } } diff --git a/apps/files_sharing/lib/Controller/ShareController.php b/apps/files_sharing/lib/Controller/ShareController.php index 38a355bda5d39..6281d016503b9 100644 --- a/apps/files_sharing/lib/Controller/ShareController.php +++ b/apps/files_sharing/lib/Controller/ShareController.php @@ -45,6 +45,7 @@ use OCP\Security\ISecureRandom; use OCP\Security\PasswordContext; use OCP\Share; +use OCP\Share\Events\ShareLinkAccessFailedEvent; use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IManager as ShareManager; use OCP\Share\IPublicShareTemplateFactory; @@ -200,6 +201,12 @@ protected function authSucceeded() { protected function authFailed() { $this->emitAccessShareHook($this->share, 403, 'Wrong password'); $this->emitShareAccessEvent($this->share, self::SHARE_AUTH, 403, 'Wrong password'); + $this->eventDispatcher->dispatchTyped(new ShareLinkAccessFailedEvent($this->getToken(), $this->share, ShareLinkAccessFailedEvent::REASON_WRONG_PASSWORD)); + } + + #[\Override] + public function shareNotFound() { + $this->eventDispatcher->dispatchTyped(new ShareLinkAccessFailedEvent($this->getToken(), null, ShareLinkAccessFailedEvent::REASON_NOT_FOUND)); } /** diff --git a/apps/files_sharing/tests/Controller/ShareControllerTest.php b/apps/files_sharing/tests/Controller/ShareControllerTest.php index f3e247cda2141..8ee37331678d0 100644 --- a/apps/files_sharing/tests/Controller/ShareControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareControllerTest.php @@ -28,6 +28,7 @@ use OCP\AppFramework\Services\IInitialState; use OCP\Constants; use OCP\Defaults; +use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\File; use OCP\Files\IRootFolder; @@ -46,6 +47,7 @@ use OCP\IUserSession; use OCP\Security\ISecureRandom; use OCP\Server; +use OCP\Share\Events\ShareLinkAccessFailedEvent; use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IAttributes; use OCP\Share\IPublicShareTemplateFactory; @@ -193,6 +195,41 @@ public function testShowShareInvalidToken(): void { $this->shareController->showShare(); } + public function testShareNotFoundDispatchesFailureEvent(): void { + $this->shareController->setToken('invalidtoken'); + + $this->eventDispatcher->expects($this->once()) + ->method('dispatchTyped') + ->with($this->callback(static fn (ShareLinkAccessFailedEvent $event): bool => $event->getToken() === 'invalidtoken' + && $event->getShare() === null + && $event->getReason() === ShareLinkAccessFailedEvent::REASON_NOT_FOUND)); + + $this->shareController->shareNotFound(); + } + + public function testAuthFailedDispatchesFailureEvent(): void { + $this->shareController->setToken('token'); + $share = $this->createMock(IShare::class); + $this->shareManager->method('getShareByToken') + ->with('token') + ->willReturn($share); + $this->assertTrue($this->shareController->isValidToken()); + + $events = []; + $this->eventDispatcher->method('dispatchTyped') + ->willReturnCallback(static function (Event $event) use (&$events): void { + $events[] = $event; + }); + + self::invokePrivate($this->shareController, 'authFailed'); + + $failures = array_values(array_filter($events, static fn (Event $event): bool => $event instanceof ShareLinkAccessFailedEvent)); + $this->assertCount(1, $failures); + $this->assertSame('token', $failures[0]->getToken()); + $this->assertSame($share, $failures[0]->getShare()); + $this->assertSame(ShareLinkAccessFailedEvent::REASON_WRONG_PASSWORD, $failures[0]->getReason()); + } + public function testShowShareNotAuthenticated(): void { $this->shareController->setToken('validtoken'); diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 4467e4e6179c2..8e3cc575d8095 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -947,6 +947,7 @@ 'OCP\\Share\\Events\\ShareCreatedEvent' => $baseDir . '/lib/public/Share/Events/ShareCreatedEvent.php', 'OCP\\Share\\Events\\ShareDeletedEvent' => $baseDir . '/lib/public/Share/Events/ShareDeletedEvent.php', 'OCP\\Share\\Events\\ShareDeletedFromSelfEvent' => $baseDir . '/lib/public/Share/Events/ShareDeletedFromSelfEvent.php', + 'OCP\\Share\\Events\\ShareLinkAccessFailedEvent' => $baseDir . '/lib/public/Share/Events/ShareLinkAccessFailedEvent.php', 'OCP\\Share\\Events\\ShareMovedEvent' => $baseDir . '/lib/public/Share/Events/ShareMovedEvent.php', 'OCP\\Share\\Events\\ShareTransferredEvent' => $baseDir . '/lib/public/Share/Events/ShareTransferredEvent.php', 'OCP\\Share\\Events\\VerifyMountPointEvent' => $baseDir . '/lib/public/Share/Events/VerifyMountPointEvent.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index dac59af04087a..ebcb1ebd289f0 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -988,6 +988,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OCP\\Share\\Events\\ShareCreatedEvent' => __DIR__ . '/../../..' . '/lib/public/Share/Events/ShareCreatedEvent.php', 'OCP\\Share\\Events\\ShareDeletedEvent' => __DIR__ . '/../../..' . '/lib/public/Share/Events/ShareDeletedEvent.php', 'OCP\\Share\\Events\\ShareDeletedFromSelfEvent' => __DIR__ . '/../../..' . '/lib/public/Share/Events/ShareDeletedFromSelfEvent.php', + 'OCP\\Share\\Events\\ShareLinkAccessFailedEvent' => __DIR__ . '/../../..' . '/lib/public/Share/Events/ShareLinkAccessFailedEvent.php', 'OCP\\Share\\Events\\ShareMovedEvent' => __DIR__ . '/../../..' . '/lib/public/Share/Events/ShareMovedEvent.php', 'OCP\\Share\\Events\\ShareTransferredEvent' => __DIR__ . '/../../..' . '/lib/public/Share/Events/ShareTransferredEvent.php', 'OCP\\Share\\Events\\VerifyMountPointEvent' => __DIR__ . '/../../..' . '/lib/public/Share/Events/VerifyMountPointEvent.php', diff --git a/lib/public/Share/Events/ShareLinkAccessFailedEvent.php b/lib/public/Share/Events/ShareLinkAccessFailedEvent.php new file mode 100644 index 0000000000000..de2a5680a508f --- /dev/null +++ b/lib/public/Share/Events/ShareLinkAccessFailedEvent.php @@ -0,0 +1,69 @@ +token; + } + + /** + * The share, or null if no share exists for the token + * + * @since 36.0.0 + */ + public function getShare(): ?IShare { + return $this->share; + } + + /** + * @return self::REASON_* + * @since 36.0.0 + */ + public function getReason(): string { + return $this->reason; + } +}