Page MenuHomePhorge

D5451.1775324652.diff
No OneTemporary

Authored By
Unknown
Size
5 KB
Referenced Files
None
Subscribers
None

D5451.1775324652.diff

diff --git a/src/app/Backends/IMAP.php b/src/app/Backends/IMAP.php
--- a/src/app/Backends/IMAP.php
+++ b/src/app/Backends/IMAP.php
@@ -35,9 +35,10 @@
// Mailbox already exists
if (self::folderExists($imap, $mailbox)) {
- $imap->closeConnection();
- self::createDefaultFolders($user);
- return true;
+ if (!\env('IMAP_CREATE_EXCEPTION_DISABLE')) {
+ $imap->closeConnection();
+ throw new IMAP\Exceptions\MailboxExistsException("Mailbox already exists: {$mailbox}");
+ }
}
// Create the mailbox
@@ -132,8 +133,20 @@
$config = self::getConfig();
$imap = self::initIMAP($config);
+ $result = self::deleteMailboxEx($imap, $mailbox);
+
+ $imap->closeConnection();
+
+ return $result;
+ }
+
+ /**
+ * Execute user mailbox deletion (as cyrus-admin)
+ */
+ protected static function deleteMailboxEx($imap, $mailbox): bool
+ {
// To delete the mailbox cyrus-admin needs extra permissions
- $result = $imap->setACL($mailbox, $config['user'], 'c');
+ $result = $imap->setACL($mailbox, \config('services.imap.admin_login'), 'c');
// Ignore the error if the folder doesn't exist (maybe it was removed already).
if (
@@ -141,15 +154,11 @@
&& str_contains($imap->error, 'Mailbox does not exist')
) {
\Log::info("The mailbox to delete was already removed: {$mailbox}");
- $result = true;
- } else {
- // Delete the mailbox (no need to delete subfolders?)
- $result = $imap->deleteFolder($mailbox);
+ return true;
}
- $imap->closeConnection();
-
- return $result;
+ // Delete the mailbox (no need to delete subfolders?)
+ return $imap->deleteFolder($mailbox);
}
/**
diff --git a/src/app/Backends/IMAP/Exceptions/MailboxExistsException.php b/src/app/Backends/IMAP/Exceptions/MailboxExistsException.php
new file mode 100644
--- /dev/null
+++ b/src/app/Backends/IMAP/Exceptions/MailboxExistsException.php
@@ -0,0 +1,5 @@
+<?php
+
+namespace App\Backends\IMAP\Exceptions;
+
+class MailboxExistsException extends \Exception {}
diff --git a/src/app/Jobs/User/CreateJob.php b/src/app/Jobs/User/CreateJob.php
--- a/src/app/Jobs/User/CreateJob.php
+++ b/src/app/Jobs/User/CreateJob.php
@@ -2,6 +2,7 @@
namespace App\Jobs\User;
+use App\Backends\IMAP\Exceptions\MailboxExistsException;
use App\EventLog;
use App\Jobs\UserJob;
use App\Plan;
@@ -97,8 +98,13 @@
if (!$user->isImapReady()) {
if (\config('app.with_imap')) {
- if (!IMAP::createUser($user)) {
- throw new \Exception("Failed to create mailbox for user {$user->email}.");
+ try {
+ if (!IMAP::createUser($user)) {
+ throw new \Exception("Failed to create mailbox for user {$user->email}.");
+ }
+ } catch (MailboxExistsException $e) {
+ $this->fail($e);
+ return;
}
} else {
if (!IMAP::verifyAccount($user->email)) {
@@ -108,6 +114,7 @@
}
$user->status |= User::STATUS_IMAP_READY;
+ $user->saveQuietly();
}
// FIXME: Should we ignore exceptions on this operation or introduce DAV_READY status?
diff --git a/src/phpunit.xml b/src/phpunit.xml
--- a/src/phpunit.xml
+++ b/src/phpunit.xml
@@ -44,5 +44,6 @@
<server name="SWOOLE_HTTP_ACCESS_LOG" value="false"/>
<server name="PGP_LENGTH" value="1024"/>
<server name="DAV_WITH_DEFAULT_FOLDERS" value="false"/>
+ <server name="IMAP_CREATE_EXCEPTION_DISABLE" value="false"/>
</php>
</phpunit>
diff --git a/src/tests/Feature/Backends/IMAPTest.php b/src/tests/Feature/Backends/IMAPTest.php
--- a/src/tests/Feature/Backends/IMAPTest.php
+++ b/src/tests/Feature/Backends/IMAPTest.php
@@ -3,6 +3,7 @@
namespace Tests\Feature\Backends;
use App\Backends\IMAP;
+use App\Backends\IMAP\Exceptions\MailboxExistsException;
use App\Sku;
use Illuminate\Support\Facades\Queue;
use Tests\TestCase;
@@ -174,6 +175,10 @@
$quota = $imap->getQuota('user/' . $user->email);
$this->assertSame($expectedQuota, $quota['all']);
+ // Create the mailbox again throws exception
+ $this->expectException(MailboxExistsException::class);
+ IMAP::createUser($user);
+
// Update the mailbox (increase quota)
$user->assignSku($storage, 1, $user->wallets->first());
$expectedQuota['user/' . $user->email]['storage']['total'] = 1048576 * 2;
diff --git a/src/tests/Feature/Jobs/User/CreateTest.php b/src/tests/Feature/Jobs/User/CreateTest.php
--- a/src/tests/Feature/Jobs/User/CreateTest.php
+++ b/src/tests/Feature/Jobs/User/CreateTest.php
@@ -2,6 +2,7 @@
namespace Tests\Feature\Jobs\User;
+use App\Backends\IMAP\Exceptions\MailboxExistsException;
use App\Domain;
use App\Jobs\User\CreateJob;
use App\Sku;
@@ -64,6 +65,15 @@
$this->assertTrue($user->isImapReady());
$this->assertTrue($user->isActive());
+ // Test mailbox exists case
+ $user->status ^= User::STATUS_IMAP_READY;
+ $user->save();
+ IMAP::shouldReceive('createUser')->once()->with($user)->andThrow(new MailboxExistsException());
+
+ $job = (new CreateJob($user->id))->withFakeQueueInteractions();
+ $job->handle();
+ $job->assertFailedWith(MailboxExistsException::class);
+
// Test deleted user
$user->delete();

File Metadata

Mime Type
text/plain
Expires
Sat, Apr 4, 5:44 PM (18 h, 18 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
18823817
Default Alt Text
D5451.1775324652.diff (5 KB)

Event Timeline