From 41467dc47fa3a5403c7050cd2e8b1496ba856fb3 Mon Sep 17 00:00:00 2001 From: FreddleSpl0it <75116288+FreddleSpl0it@users.noreply.github.com> Date: Mon, 2 Nov 2026 09:50:24 +0100 Subject: [PATCH] [Web] imapsync: change the server of a sync source only with access to all its sync jobs --- .../Imapsync/Repository/SourceRepository.php | 6 +++++ .../Imapsync/Service/SourceService.php | 24 ++++++++++++++++++- data/web/lang/lang.de-de.json | 1 + data/web/lang/lang.en-gb.json | 1 + .../Imapsync/Service/SourceServiceTest.php | 14 +++++++++++ 5 files changed, 45 insertions(+), 1 deletion(-) diff --git a/data/web/inc/lib/mailcow/Imapsync/Repository/SourceRepository.php b/data/web/inc/lib/mailcow/Imapsync/Repository/SourceRepository.php index b35397013..6e6b37dfe 100644 --- a/data/web/inc/lib/mailcow/Imapsync/Repository/SourceRepository.php +++ b/data/web/inc/lib/mailcow/Imapsync/Repository/SourceRepository.php @@ -48,6 +48,12 @@ class SourceRepository extends Repository return (bool)$this->query("SELECT 1 FROM `imapsync` WHERE `source_id` = :id LIMIT 1", ['id' => $id])->fetchColumn(); } + /** @return string[] mailboxes whose sync jobs use the source */ + public function jobMailboxes(int $id): array + { + return $this->query("SELECT DISTINCT `user2` FROM `imapsync` WHERE `source_id` = :id", ['id' => $id])->fetchAll(PDO::FETCH_COLUMN); + } + public function isSuperAdmin(string $username): bool { return (bool)$this->query("SELECT 1 FROM `admin` WHERE `username` = :username AND `superadmin` = '1'", diff --git a/data/web/inc/lib/mailcow/Imapsync/Service/SourceService.php b/data/web/inc/lib/mailcow/Imapsync/Service/SourceService.php index c2a9b6d6d..2aa01b633 100644 --- a/data/web/inc/lib/mailcow/Imapsync/Service/SourceService.php +++ b/data/web/inc/lib/mailcow/Imapsync/Service/SourceService.php @@ -121,7 +121,11 @@ class SourceService if ($writeTargets) { $this->applyScope($source, $input['scope'] ?? $current->scope, $input['domains'] ?? [], $input['users'] ?? []); } - $this->repo->update($source, $writeTargets, self::connection($current) !== self::connection($source)); + $connectionChanged = self::connection($current) !== self::connection($source); + if ($connectionChanged) { + $this->requireAllJobsAccessible($source); + } + $this->repo->update($source, $writeTargets, $connectionChanged); $this->fetchToken($source); return $source; } @@ -200,6 +204,24 @@ class SourceService return $this->context->hasAcl('syncjobs') || $this->context->isAdmin(); } + /** + * The connection of a source only changes when the caller may access every mailbox whose sync jobs use it, + * their stored passwords would go to the new server. + * + * @throws UserError imapsync_source_used_by_others + */ + private function requireAllJobsAccessible(Source $source): void + { + if ($this->context->hasAdminRights()) { + return; + } + foreach ($this->repo->jobMailboxes((int)$source->id) as $mailbox) { + if (!$this->context->canAccessMailbox($mailbox)) { + throw new UserError('imapsync_source_used_by_others', [htmlspecialchars($source->name)]); + } + } + } + /** @throws AccessDenied when the caller cannot see or edit the source */ private function editable(int $id): Source { diff --git a/data/web/lang/lang.de-de.json b/data/web/lang/lang.de-de.json index ca9a093b8..6e4e5c357 100644 --- a/data/web/lang/lang.de-de.json +++ b/data/web/lang/lang.de-de.json @@ -666,6 +666,7 @@ "template_name_invalid": "Name der Vorlage ungültig", "required_data_missing": "Die benötigten Daten: %s fehlen", "imapsync_source_in_use": "Sync-Quelle %s wird noch von einem Sync-Job verwendet und kann nicht gelöscht werden", + "imapsync_source_used_by_others": "Der Server der Sync-Quelle %s kann nicht geändert werden, Sync-Jobs anderer Postfächer verwenden sie", "imapsync_source_token_refresh_failed": "OAuth-Token-Erneuerung für %s fehlgeschlagen", "imapsync_source_scope_empty": "Für die gewählte Sichtbarkeit mindestens eine Domain wählen", "imapsync_source_unavailable": "Die gewählte Sync-Quelle ist nicht verfügbar", diff --git a/data/web/lang/lang.en-gb.json b/data/web/lang/lang.en-gb.json index 7ba74740f..cd5848d03 100644 --- a/data/web/lang/lang.en-gb.json +++ b/data/web/lang/lang.en-gb.json @@ -666,6 +666,7 @@ "version_invalid": "Version %s is invalid", "yotp_verification_failed": "Yubico OTP verification failed: %s", "imapsync_source_in_use": "Sync source %s is still used by a sync job and cannot be deleted", + "imapsync_source_used_by_others": "The server of sync source %s cannot be changed, sync jobs of other mailboxes use it", "imapsync_source_token_refresh_failed": "OAuth token refresh for %s failed", "imapsync_source_scope_empty": "Select at least one domain for the chosen visibility", "imapsync_source_unavailable": "The selected sync source is unavailable", diff --git a/tests/php/Imapsync/Service/SourceServiceTest.php b/tests/php/Imapsync/Service/SourceServiceTest.php index d14d26fa7..2c13ad978 100644 --- a/tests/php/Imapsync/Service/SourceServiceTest.php +++ b/tests/php/Imapsync/Service/SourceServiceTest.php @@ -292,6 +292,20 @@ class SourceServiceTest extends TestCase $this->expectUserError('access_denied', fn() => $service->update(7, ['name' => 'b'])); } + public function testServerOnlyChangesWhenAllJobsAreAccessible(): void + { + $this->repo->method('findVisible')->willReturn(self::source(['created_by' => 'u@example.org'])); + $this->repo->expects($this->exactly(2))->method('jobMailboxes') + ->willReturnOnConsecutiveCalls(['u@example.org', 'other@example.org'], ['u@example.org']); + $this->repo->expects($this->exactly(2))->method('update'); + $service = $this->service(FakeContext::user('u@example.org')); + + $this->expectUserError(['imapsync_source_used_by_others', 'src'], fn() => $service->update(7, ['host' => 'other.example.org'])); + $service->update(7, ['host' => 'new.example.org']); + // Name or description changes keep the server and need no check + $service->update(7, ['description' => 'x']); + } + public function testUsersMayNotSwitchToXOAuth2(): void { $this->repo->method('findVisible')->willReturn(self::source(['created_by' => 'u@example.org']));