diff --git a/src/app/Console/Commands/UserAddAlias.php b/src/app/Console/Commands/UserAddAlias.php new file mode 100644 --- /dev/null +++ b/src/app/Console/Commands/UserAddAlias.php @@ -0,0 +1,69 @@ +argument('user'))->first(); + + if (!$user) { + return 1; + } + + $alias = \strtolower($this->argument('alias')); + + // Check if the alias already exists + $existing = $user->aliases()->get()->pluck('alias')->all(); + + if (in_array($alias, $existing)) { + $this->error("Address is already assigned to the user."); + return 1; + } + + $controller = $user->wallet()->owner; + + // Validate the alias + $error = UsersController::validateEmail($alias, $controller, true); + + if ($error) { + $this->error($error); + return 1; + } + + $user->aliases()->create(['alias' => $alias]); + } +} diff --git a/src/app/Http/Controllers/API/V4/UsersController.php b/src/app/Http/Controllers/API/V4/UsersController.php --- a/src/app/Http/Controllers/API/V4/UsersController.php +++ b/src/app/Http/Controllers/API/V4/UsersController.php @@ -643,14 +643,7 @@ } // Check if it is one of domains available to the user - // TODO: We should have a helper that returns "flat" array with domain names - // I guess we could use pluck() somehow - $domains = array_map( - function ($domain) { - return $domain->namespace; - }, - $user->domains() - ); + $domains = \collect($user->domains())->pluck('namespace')->all(); if (!in_array($domain->namespace, $domains)) { return \trans('validation.entryexists', ['attribute' => 'domain']); @@ -659,7 +652,8 @@ // Check if a user/alias with specified address already exists // Allow assigning the same alias to a user in the same group account, // but only for non-public domains - if ($exists = User::emailExists($email, true, $alias_exists)) { + // Allow an alias in a custom domain to an address that was a user before + if ($exists = User::emailExists($email, true, $alias_exists, $is_alias && !$domain->isPublic())) { if ( !$is_alias || !$alias_exists diff --git a/src/app/User.php b/src/app/User.php --- a/src/app/User.php +++ b/src/app/User.php @@ -342,10 +342,11 @@ * @param string $email Email address * @param bool $return_user Return User instance instead of boolean * @param bool $is_alias Set to True if the existing email is an alias + * @param bool $existing Ignore deleted users * * @return \App\User|bool True or User model object if found, False otherwise */ - public static function emailExists(string $email, bool $return_user = false, &$is_alias = false) + public static function emailExists(string $email, bool $return_user = false, &$is_alias = false, $existing = false) { if (strpos($email, '@') === false) { return false; @@ -353,17 +354,28 @@ $email = \strtolower($email); - $user = self::withTrashed()->where('email', $email)->first(); + if ($existing) { + $user = self::where('email', $email)->first(); + } else { + $user = self::withTrashed()->where('email', $email)->first(); + } if ($user) { return $return_user ? $user : true; } - $alias = UserAlias::where('alias', $email)->first(); + $aliases = UserAlias::where('alias', $email); + + if ($existing) { + $aliases = $aliases->join('users', 'user_id', '=', 'users.id') + ->whereNull('users.deleted_at'); + } + + $alias = $aliases->first(); if ($alias) { $is_alias = true; - return $return_user ? User::withTrashed()->find($alias->user_id) : true; + return $return_user ? self::withTrashed()->find($alias->user_id) : true; } return false; diff --git a/src/tests/Feature/Controller/UsersTest.php b/src/tests/Feature/Controller/UsersTest.php --- a/src/tests/Feature/Controller/UsersTest.php +++ b/src/tests/Feature/Controller/UsersTest.php @@ -29,6 +29,8 @@ $this->deleteTestUser('UsersControllerTest3@userscontroller.com'); $this->deleteTestUser('UserEntitlement2A@UserEntitlement.com'); $this->deleteTestUser('john2.doe2@kolab.org'); + $this->deleteTestUser('deleted@kolab.org'); + $this->deleteTestUser('deleted@kolabnow.com'); $this->deleteTestDomain('userscontroller.com'); $user = $this->getTestUser('john@kolab.org'); @@ -51,6 +53,8 @@ $this->deleteTestUser('UsersControllerTest3@userscontroller.com'); $this->deleteTestUser('UserEntitlement2A@UserEntitlement.com'); $this->deleteTestUser('john2.doe2@kolab.org'); + $this->deleteTestUser('deleted@kolab.org'); + $this->deleteTestUser('deleted@kolabnow.com'); $this->deleteTestDomain('userscontroller.com'); $user = $this->getTestUser('john@kolab.org'); @@ -1008,4 +1012,38 @@ $this->assertSame($expected_result, $result); } + + /** + * User email/alias validation - more cases. + * + * Note: Technically these include unit tests, but let's keep it here for now. + * FIXME: Shall we do a http request for each case? + */ + public function testValidateEmail2(): void + { + Queue::fake(); + + $john = $this->getTestUser('john@kolab.org'); + $jack = $this->getTestUser('jack@kolab.org'); + $user = $this->getTestUser('UsersControllerTest1@userscontroller.com'); + $deleted_priv = $this->getTestUser('deleted@kolab.org'); + $deleted_priv->setAliases(['deleted-alias@kolab.org']); + $deleted_priv->delete(); + $deleted_pub = $this->getTestUser('deleted@kolabnow.com'); + $deleted_pub->setAliases(['deleted-alias@kolabnow.com']); + $deleted_pub->delete(); + + // An alias that was a user email before is allowed, but only for custom domains + $result = UsersController::validateEmail('deleted@kolab.org', $john, true); + $this->assertSame(null, $result); + + $result = UsersController::validateEmail('deleted-alias@kolab.org', $john, true); + $this->assertSame(null, $result); + + $result = UsersController::validateEmail('deleted@kolabnow.com', $john, true); + $this->assertSame('The specified alias is not available.', $result); + + $result = UsersController::validateEmail('deleted-alias@kolabnow.com', $john, true); + $this->assertSame('The specified alias is not available.', $result); + } }