diff --git a/src/app/Console/Commands/SharedFolder/AddAliasCommand.php b/src/app/Console/Commands/SharedFolder/AddAliasCommand.php --- a/src/app/Console/Commands/SharedFolder/AddAliasCommand.php +++ b/src/app/Console/Commands/SharedFolder/AddAliasCommand.php @@ -3,7 +3,7 @@ namespace App\Console\Commands\SharedFolder; use App\Console\Command; -use App\Http\Controllers\API\V4\UsersController; +use App\Http\Controllers\API\V4\SharedFoldersController; class AddAliasCommand extends Command { @@ -44,7 +44,9 @@ } // Validate the alias - $error = UsersController::validateAlias($alias, $folder->walletOwner()); + $domain = explode('@', $folder->email, 2)[1]; + + $error = SharedFoldersController::validateAlias($alias, $folder->walletOwner(), $folder->name, $domain); if ($error) { if (!$this->option('force')) { diff --git a/src/app/Entitlement.php b/src/app/Entitlement.php --- a/src/app/Entitlement.php +++ b/src/app/Entitlement.php @@ -11,18 +11,18 @@ * * Owned by a {@link \App\User}, billed to a {@link \App\Wallet}. * - * @property int $cost - * @property ?string $description - * @property \App\Domain|\App\User $entitleable The entitled object (receiver of the entitlement). - * @property int $entitleable_id - * @property string $entitleable_type - * @property int $fee - * @property string $id - * @property \App\User $owner The owner of this entitlement (subject). - * @property \App\Sku $sku The SKU to which this entitlement applies. - * @property string $sku_id - * @property \App\Wallet $wallet The wallet to which this entitlement is charged. - * @property string $wallet_id + * @property int $cost + * @property ?string $description + * @property ?object $entitleable The entitled object (receiver of the entitlement). + * @property int $entitleable_id + * @property string $entitleable_type + * @property int $fee + * @property string $id + * @property \App\User $owner The owner of this entitlement (subject). + * @property \App\Sku $sku The SKU to which this entitlement applies. + * @property string $sku_id + * @property \App\Wallet $wallet The wallet to which this entitlement is charged. + * @property string $wallet_id */ class Entitlement extends Model { diff --git a/src/app/Http/Controllers/API/V4/SharedFoldersController.php b/src/app/Http/Controllers/API/V4/SharedFoldersController.php --- a/src/app/Http/Controllers/API/V4/SharedFoldersController.php +++ b/src/app/Http/Controllers/API/V4/SharedFoldersController.php @@ -200,6 +200,7 @@ $errors = []; if (empty($folder)) { + $name = $request->input('name'); $domain = $request->input('domain'); $rules = [ 'name' => ['required', 'string', new SharedFolderName($owner, $domain)], @@ -208,8 +209,9 @@ } else { // On update validate the folder name (if changed) $name = $request->input('name'); + $domain = explode('@', $folder->email, 2)[1]; + if ($name !== null && $name != $folder->name) { - $domain = explode('@', $folder->email, 2)[1]; $rules = ['name' => ['required', 'string', new SharedFolderName($owner, $domain)]]; } } @@ -237,7 +239,7 @@ // validate new aliases if ( !in_array($alias, $existing_aliases) - && ($error = UsersController::validateAlias($alias, $owner)) + && ($error = self::validateAlias($alias, $owner, $name, $domain)) ) { if (!isset($errors['aliases'])) { $errors['aliases'] = []; @@ -259,4 +261,25 @@ return null; } + + /** + * Email address validation for use as a shared folder alias. + * + * @param string $alias Email address + * @param \App\User $owner The account owner + * @param string $folderName Folder name + * @param string $domain Folder domain + * + * @return ?string Error message on validation error + */ + public static function validateAlias(string $alias, \App\User $owner, string $folderName, string $domain): ?string + { + $lmtp_alias = "shared+shared/{$folderName}@{$domain}"; + + if ($alias === $lmtp_alias) { + return null; + } + + return UsersController::validateAlias($alias, $owner); + } } diff --git a/src/app/Observers/SharedFolderAliasObserver.php b/src/app/Observers/SharedFolderAliasObserver.php --- a/src/app/Observers/SharedFolderAliasObserver.php +++ b/src/app/Observers/SharedFolderAliasObserver.php @@ -17,7 +17,7 @@ */ public function creating(SharedFolderAlias $alias): bool { - $alias->alias = \strtolower($alias->alias); + $alias->alias = \App\Utils::emailToLower($alias->alias); $domainName = explode('@', $alias->alias)[1]; @@ -30,7 +30,7 @@ if ($alias->sharedFolder) { if ($alias->sharedFolder->tenant_id != $domain->tenant_id) { - \Log::error("Reseller for folder '{$alias->sharedFolder->email}' and domain '{$domainName}' differ."); + \Log::error("Tenant for folder '{$alias->sharedFolder->email}' and domain '{$domainName}' differ."); return false; } } diff --git a/src/app/Observers/WalletObserver.php b/src/app/Observers/WalletObserver.php --- a/src/app/Observers/WalletObserver.php +++ b/src/app/Observers/WalletObserver.php @@ -99,7 +99,10 @@ // Un-suspend domains/users foreach ($wallet->entitlements as $entitlement) { - if (method_exists($entitlement->entitleable_type, 'unsuspend')) { + if ( + method_exists($entitlement->entitleable_type, 'unsuspend') + && !empty($entitlement->entitleable) + ) { $entitlement->entitleable->unsuspend(); } } diff --git a/src/app/Rules/SharedFolderName.php b/src/app/Rules/SharedFolderName.php --- a/src/app/Rules/SharedFolderName.php +++ b/src/app/Rules/SharedFolderName.php @@ -12,7 +12,7 @@ private $owner; private $domain; - private const FORBIDDEN_CHARS = '+/^%*!`@(){}|\\?<;"'; + private const FORBIDDEN_CHARS = '+^%*!`@(){}|\\?<;"'; /** * Class constructor. @@ -36,14 +36,17 @@ */ public function passes($attribute, $name): bool { - if (empty($name) || !is_string($name) || $name == 'Resources') { + if (empty($name) || !is_string($name) || $name == 'Resources' || \str_starts_with($name, 'Resources/')) { $this->message = \trans('validation.nameinvalid'); return false; } - if (strcspn($name, self::FORBIDDEN_CHARS) < strlen($name)) { - $this->message = \trans('validation.nameinvalid'); - return false; + foreach (explode('/', $name) as $subfolder) { + $length = strlen($subfolder); + if (!$length || strcspn($subfolder, self::FORBIDDEN_CHARS) < $length) { + $this->message = \trans('validation.nameinvalid'); + return false; + } } // Check the max length, according to the database column length diff --git a/src/app/SharedFolderAlias.php b/src/app/SharedFolderAlias.php --- a/src/app/SharedFolderAlias.php +++ b/src/app/SharedFolderAlias.php @@ -24,7 +24,7 @@ */ public function setAliasAttribute(string $alias) { - $this->attributes['alias'] = \strtolower($alias); + $this->attributes['alias'] = \App\Utils::emailToLower($alias); } /** diff --git a/src/app/Traits/AliasesTrait.php b/src/app/Traits/AliasesTrait.php --- a/src/app/Traits/AliasesTrait.php +++ b/src/app/Traits/AliasesTrait.php @@ -2,8 +2,6 @@ namespace App\Traits; -use Illuminate\Support\Str; - trait AliasesTrait { /** @@ -29,7 +27,7 @@ return false; } - $email = \strtolower($email); + $email = \App\Utils::emailToLower($email); $class = static::class . 'Alias'; return $class::where('alias', $email)->count() > 0; @@ -51,7 +49,7 @@ */ public function setAliases(array $aliases): void { - $aliases = array_map('strtolower', $aliases); + $aliases = array_map('\App\Utils::emailToLower', $aliases); $aliases = array_unique($aliases); $existing_aliases = []; @@ -64,7 +62,6 @@ $existing_aliases[] = $alias->alias; } } - foreach (array_diff($aliases, $existing_aliases) as $alias) { $this->aliases()->create(['alias' => $alias]); } diff --git a/src/app/Utils.php b/src/app/Utils.php --- a/src/app/Utils.php +++ b/src/app/Utils.php @@ -122,6 +122,27 @@ fclose($fp); } + /** + * Converts an email address to lower case. Keeps the LMTP shared folder + * addresses character case intact. + * + * @param string $email Email address + * + * @return string Email address + */ + public static function emailToLower(string $email): string + { + // For LMTP shared folder address lower case the domain part only + if (str_starts_with($email, 'shared+shared/')) { + $pos = strrpos($email, '@'); + $domain = substr($email, $pos + 1); + $local = substr($email, 0, strlen($email) - strlen($domain) - 1); + + return $local . '@' . strtolower($domain); + } + + return strtolower($email); + } /** * Generate a passphrase. Not intended for use in production, so limited to environments that are not production. @@ -301,7 +322,7 @@ return $asArray ? ['', ''] : ''; } - $address = \strtolower($address); + $address = self::emailToLower($address); if (strpos($address, '@') === false) { return $asArray ? [$address, ''] : $address; diff --git a/src/tests/Feature/Controller/SharedFoldersTest.php b/src/tests/Feature/Controller/SharedFoldersTest.php --- a/src/tests/Feature/Controller/SharedFoldersTest.php +++ b/src/tests/Feature/Controller/SharedFoldersTest.php @@ -18,7 +18,7 @@ parent::setUp(); $this->deleteTestSharedFolder('folder-test@kolab.org'); - SharedFolder::where('name', 'Test Folder')->delete(); + SharedFolder::where('name', 'like', 'Test_Folder')->forceDelete(); } /** @@ -27,7 +27,7 @@ public function tearDown(): void { $this->deleteTestSharedFolder('folder-test@kolab.org'); - SharedFolder::where('name', 'Test Folder')->delete(); + SharedFolder::where('name', 'like', 'Test_Folder')->forceDelete(); parent::tearDown(); } @@ -441,7 +441,7 @@ // Test successful folder creation $post['name'] = 'Test Folder'; $post['type'] = 'event'; - $post['aliases'] = ['folder-alias@kolab.org']; // expected to be ignored + $post['aliases'] = []; $response = $this->actingAs($john)->post("/api/v4/shared-folders", $post); $json = $response->json(); @@ -482,6 +482,20 @@ $folder = SharedFolder::where('name', $post['name'])->first(); $this->assertSame(['folder-alias@kolab.org'], $folder->aliases()->pluck('alias')->all()); + + $folder->forceDelete(); + + // Test handling subfolders and lmtp alias email + $post['name'] = 'Test/Folder'; + $post['type'] = 'mail'; + $post['aliases'] = ['shared+shared/Test/Folder@kolab.org']; + $response = $this->actingAs($john)->post("/api/v4/shared-folders", $post); + $json = $response->json(); + + $response->assertStatus(200); + + $folder = SharedFolder::where('name', $post['name'])->first(); + $this->assertSame(['shared+shared/Test/Folder@kolab.org'], $folder->aliases()->pluck('alias')->all()); } /** diff --git a/src/tests/Feature/Controller/WalletsTest.php b/src/tests/Feature/Controller/WalletsTest.php --- a/src/tests/Feature/Controller/WalletsTest.php +++ b/src/tests/Feature/Controller/WalletsTest.php @@ -71,7 +71,7 @@ $wallet->balance = 990; $notice = $method->invoke($controller, $wallet); - $this->assertMatchesRegularExpression('/\((1 month)\)/', $notice); + $this->assertMatchesRegularExpression('/\((1 month|4 weeks)\)/', $notice); // test "2 months" $wallet->balance = 990 * 2.6; diff --git a/src/tests/Unit/Rules/SharedFolderNameTest.php b/src/tests/Unit/Rules/SharedFolderNameTest.php --- a/src/tests/Unit/Rules/SharedFolderNameTest.php +++ b/src/tests/Unit/Rules/SharedFolderNameTest.php @@ -23,6 +23,12 @@ $v = Validator::make(['name' => []], $rules); $this->assertSame(['name' => ["The specified name is invalid."]], $v->errors()->toArray()); + $v = Validator::make(['name' => ['Resources']], $rules); + $this->assertSame(['name' => ["The specified name is invalid."]], $v->errors()->toArray()); + + $v = Validator::make(['name' => ['Resources/Test']], $rules); + $this->assertSame(['name' => ["The specified name is invalid."]], $v->errors()->toArray()); + // Forbidden chars $v = Validator::make(['name' => 'Test@'], $rules); $this->assertSame(['name' => ["The specified name is invalid."]], $v->errors()->toArray()); @@ -43,5 +49,18 @@ $rules = ['name' => ['present', new SharedFolderName($user, 'kolabnow.com')]]; $v = Validator::make(['name' => 'TestRule'], $rules); $this->assertSame(['name' => ["The specified domain is invalid."]], $v->errors()->toArray()); + + // Invalid subfolders + $rules = ['name' => ['present', new SharedFolderName($user, 'kolab.org')]]; + $v = Validator::make(['name' => 'Test//Rule'], $rules); + $this->assertSame(['name' => ["The specified name is invalid."]], $v->errors()->toArray()); + $v = Validator::make(['name' => '/TestRule'], $rules); + $this->assertSame(['name' => ["The specified name is invalid."]], $v->errors()->toArray()); + $v = Validator::make(['name' => 'TestRule/'], $rules); + $this->assertSame(['name' => ["The specified name is invalid."]], $v->errors()->toArray()); + + // Valid subfolder + $v = Validator::make(['name' => 'Test/Rule/Folder'], $rules); + $this->assertSame([], $v->errors()->toArray()); } } diff --git a/src/tests/Unit/UtilsTest.php b/src/tests/Unit/UtilsTest.php --- a/src/tests/Unit/UtilsTest.php +++ b/src/tests/Unit/UtilsTest.php @@ -8,6 +8,16 @@ class UtilsTest extends TestCase { /** + * Test for Utils::emailToLower() + */ + public function testEmailToLower(): void + { + $this->assertSame('test@test.tld', \App\Utils::emailToLower('test@Test.Tld')); + $this->assertSame('test@test.tld', \App\Utils::emailToLower('Test@Test.Tld')); + $this->assertSame('shared+shared/Test@test.tld', \App\Utils::emailToLower('shared+shared/Test@Test.Tld')); + } + + /** * Test for Utils::normalizeAddress() */ public function testNormalizeAddress(): void