diff --git a/apps/federation/composer/composer/autoload_classmap.php b/apps/federation/composer/composer/autoload_classmap.php index d648b643c4642..4195911150cb2 100644 --- a/apps/federation/composer/composer/autoload_classmap.php +++ b/apps/federation/composer/composer/autoload_classmap.php @@ -16,6 +16,7 @@ 'OCA\\Federation\\DAV\\FedAuth' => $baseDir . '/../lib/DAV/FedAuth.php', 'OCA\\Federation\\DbHandler' => $baseDir . '/../lib/DbHandler.php', 'OCA\\Federation\\Listener\\SabrePluginAuthInitListener' => $baseDir . '/../lib/Listener/SabrePluginAuthInitListener.php', + 'OCA\\Federation\\Listener\\TrustedServerRemovedListener' => $baseDir . '/../lib/Listener/TrustedServerRemovedListener.php', 'OCA\\Federation\\Migration\\Version1010Date20200630191302' => $baseDir . '/../lib/Migration/Version1010Date20200630191302.php', 'OCA\\Federation\\Settings\\Admin' => $baseDir . '/../lib/Settings/Admin.php', 'OCA\\Federation\\SyncFederationAddressBooks' => $baseDir . '/../lib/SyncFederationAddressBooks.php', diff --git a/apps/federation/composer/composer/autoload_static.php b/apps/federation/composer/composer/autoload_static.php index 29932e1dffc09..b2945ddb80d39 100644 --- a/apps/federation/composer/composer/autoload_static.php +++ b/apps/federation/composer/composer/autoload_static.php @@ -31,6 +31,7 @@ class ComposerStaticInitFederation 'OCA\\Federation\\DAV\\FedAuth' => __DIR__ . '/..' . '/../lib/DAV/FedAuth.php', 'OCA\\Federation\\DbHandler' => __DIR__ . '/..' . '/../lib/DbHandler.php', 'OCA\\Federation\\Listener\\SabrePluginAuthInitListener' => __DIR__ . '/..' . '/../lib/Listener/SabrePluginAuthInitListener.php', + 'OCA\\Federation\\Listener\\TrustedServerRemovedListener' => __DIR__ . '/..' . '/../lib/Listener/TrustedServerRemovedListener.php', 'OCA\\Federation\\Migration\\Version1010Date20200630191302' => __DIR__ . '/..' . '/../lib/Migration/Version1010Date20200630191302.php', 'OCA\\Federation\\Settings\\Admin' => __DIR__ . '/..' . '/../lib/Settings/Admin.php', 'OCA\\Federation\\SyncFederationAddressBooks' => __DIR__ . '/..' . '/../lib/SyncFederationAddressBooks.php', diff --git a/apps/federation/lib/AppInfo/Application.php b/apps/federation/lib/AppInfo/Application.php index 358e3f68d508d..f6b3d8c26f512 100644 --- a/apps/federation/lib/AppInfo/Application.php +++ b/apps/federation/lib/AppInfo/Application.php @@ -9,10 +9,12 @@ use OCA\DAV\Events\SabrePluginAuthInitEvent; use OCA\Federation\Listener\SabrePluginAuthInitListener; +use OCA\Federation\Listener\TrustedServerRemovedListener; use OCP\AppFramework\App; use OCP\AppFramework\Bootstrap\IBootContext; use OCP\AppFramework\Bootstrap\IBootstrap; use OCP\AppFramework\Bootstrap\IRegistrationContext; +use OCP\Federation\Events\TrustedServerRemovedEvent; class Application extends App implements IBootstrap { @@ -25,6 +27,7 @@ public function __construct($urlParams = []) { public function register(IRegistrationContext $context): void { $context->registerEventListener(SabrePluginAuthInitEvent::class, SabrePluginAuthInitListener::class); + $context->registerEventListener(TrustedServerRemovedEvent::class, TrustedServerRemovedListener::class); } public function boot(IBootContext $context): void { diff --git a/apps/federation/lib/Listener/TrustedServerRemovedListener.php b/apps/federation/lib/Listener/TrustedServerRemovedListener.php new file mode 100644 index 0000000000000..ba5239d642402 --- /dev/null +++ b/apps/federation/lib/Listener/TrustedServerRemovedListener.php @@ -0,0 +1,57 @@ + */ +class TrustedServerRemovedListener implements IEventListener { + public function __construct( + private readonly IJobList $jobList, + ) { + } + + public function handle(Event $event): void { + if (!$event instanceof TrustedServerRemovedEvent) { + return; + } + + if ($event->getUrl() === null) { + return; // safe guard + } + + $this->removeJobsByUrl(RequestSharedSecret::class, $event->getUrl()); + $this->removeJobsByUrl(GetSharedSecret::class, $event->getUrl()); + } + + /** + * Remove RequestSharedSecret or GetSharedSecret jobs from the job list by their URL. + * The jobs are scheduled with url, token, and created as arguments. + * Thus, we have to loop over the jobs here and cannot use IJobList.remove. + */ + private function removeJobsByUrl(string $class, string $url): void { + foreach ($this->jobList->getJobsIterator($class, null, 0) as $job) { + $arguments = $job->getArgument(); + if (isset($arguments['url']) && $arguments['url'] === $url) { + try { + $this->jobList->removeById($job->getId()); + } catch (\Exception) { + // Removing the background jobs is optional because they will expire sometime. + // Therefore, we are using catch and ignore. + } + } + } + } +} diff --git a/apps/federation/lib/TrustedServers.php b/apps/federation/lib/TrustedServers.php index 3d15cfac448ee..0b4f004add233 100644 --- a/apps/federation/lib/TrustedServers.php +++ b/apps/federation/lib/TrustedServers.php @@ -88,7 +88,7 @@ public function addSharedSecret(string $url, string $sharedSecret): void { public function removeServer(int $id): void { $server = $this->dbHandler->getServerById($id); $this->dbHandler->removeServer($id); - $this->dispatcher->dispatchTyped(new TrustedServerRemovedEvent($server['url_hash'])); + $this->dispatcher->dispatchTyped(new TrustedServerRemovedEvent($server['url_hash'], $server['url'])); } diff --git a/apps/federation/tests/Listener/TrustedServerRemovedListenerTest.php b/apps/federation/tests/Listener/TrustedServerRemovedListenerTest.php new file mode 100644 index 0000000000000..a979d08bcdbe0 --- /dev/null +++ b/apps/federation/tests/Listener/TrustedServerRemovedListenerTest.php @@ -0,0 +1,71 @@ +jobList = new DummyJobList(); + $this->listener = new TrustedServerRemovedListener($this->jobList); + } + + public function testHandle(): void { + // Arrange + $url = 'https://example.com'; + $event = new TrustedServerRemovedEvent(md5($url), $url); // we are using a different hashing in the tests. + $job1 = $this->createGetSharedSecretMock(); + $job2 = $this->createGetSharedSecretMock(); + $job3 = $this->createGetSharedSecretMock(); + $job4 = $this->createGetSharedSecretMock(); + $this->jobList->add($job1, ['url' => 'https://example.org', 'token' => 'nei0dooX', 'created' => 0]); + $this->jobList->add($job2, ['url' => 'https://example.net', 'token' => 'ci6Shah7', 'created' => 0]); + $this->jobList->add($job3, ['url' => $url, 'token' => 'ieXie6Me', 'created' => 0]); + $this->jobList->add($job4, ['url' => $url, 'token' => 'thoQu8th', 'created' => 0]); + + // Act + $this->listener->handle($event); + $jobs = iterator_to_array($this->jobList->getJobsIterator(GetSharedSecret::class, null, 0), false); + + // Assert + $this->assertCount(2, $jobs); + } + + private function createGetSharedSecretMock(): GetSharedSecret { + return new GetSharedSecret( + $this->createMock(IClientService::class), + $this->createMock(IURLGenerator::class), + $this->jobList, + $this->createMock(TrustedServers::class), + new NullLogger(), + $this->createMock(IDiscoveryService::class), + new TimeFactory(), + $this->createMock(IConfig::class), + ); + } +} diff --git a/apps/federation/tests/TrustedServersTest.php b/apps/federation/tests/TrustedServersTest.php index 0c900f6edf7c7..0b8eed426729e 100644 --- a/apps/federation/tests/TrustedServersTest.php +++ b/apps/federation/tests/TrustedServersTest.php @@ -120,15 +120,15 @@ public function testGetSharedSecret(): void { public function testRemoveServer(): void { $id = 42; - $server = ['url_hash' => 'url_hash']; + $server = ['url' => 'url', 'url_hash' => 'url_hash']; $this->dbHandler->expects($this->once())->method('removeServer')->with($id); $this->dbHandler->expects($this->once())->method('getServerById')->with($id) ->willReturn($server); $this->dispatcher->expects($this->once())->method('dispatchTyped') ->willReturnCallback( function ($event): void { - $this->assertSame(get_class($event), TrustedServerRemovedEvent::class); - /** @var \OCP\Federated\Events\TrustedServerRemovedEvent $event */ + $this->assertInstanceOf(TrustedServerRemovedEvent::class, $event); + $this->assertSame('url', $event->getUrl()); $this->assertSame('url_hash', $event->getUrlHash()); } ); diff --git a/lib/public/Federation/Events/TrustedServerRemovedEvent.php b/lib/public/Federation/Events/TrustedServerRemovedEvent.php index 41899d67a881c..773377eb8533b 100644 --- a/lib/public/Federation/Events/TrustedServerRemovedEvent.php +++ b/lib/public/Federation/Events/TrustedServerRemovedEvent.php @@ -14,14 +14,16 @@ * @since 25.0.0 */ class TrustedServerRemovedEvent extends Event { - private string $urlHash; /** * @since 25.0.0 + * @since 32.0.0 Added $url argument */ - public function __construct(string $urlHash) { + public function __construct( + private readonly string $urlHash, + private readonly ?string $url = null, + ) { parent::__construct(); - $this->urlHash = $urlHash; } /** @@ -30,4 +32,11 @@ public function __construct(string $urlHash) { public function getUrlHash(): string { return $this->urlHash; } + + /** + * @since 32.0.0 + */ + public function getUrl(): ?string { + return $this->url; + } } diff --git a/tests/lib/BackgroundJob/DummyJobList.php b/tests/lib/BackgroundJob/DummyJobList.php index 717db52715ff5..8cb9523b0ddac 100644 --- a/tests/lib/BackgroundJob/DummyJobList.php +++ b/tests/lib/BackgroundJob/DummyJobList.php @@ -8,6 +8,7 @@ namespace Test\BackgroundJob; +use ArrayIterator; use OC\BackgroundJob\JobList; use OCP\BackgroundJob\IJob; use OCP\BackgroundJob\Job; @@ -98,20 +99,23 @@ public function getAll(): array { return $this->jobs; } - public function getJobsIterator($job, ?int $limit, int $offset): array { + public function getJobsIterator($job, ?int $limit, int $offset): iterable { if ($job instanceof IJob) { $jobClass = get_class($job); } else { $jobClass = $job; } - return array_slice( + + $jobs = array_slice( array_filter( $this->jobs, - fn ($job) => ($jobClass === null) || (get_class($job) == $jobClass) + fn ($job) => ($jobClass === null) || (get_class($job) === $jobClass) ), $offset, $limit ); + + return new ArrayIterator($jobs); } /**