Page MenuHomePhorge
Files F117742810

D1930.1775167044.diff
No OneTemporary
Actions

Authored By
Unknown
Size
19 KB
Referenced Files
None
Subscribers
None

D1930.1775167044.diff
View Options

diff --git a/src/app/Console/Commands/UserAddAlias.php b/src/app/Console/Commands/UserAddAlias.php
--- a/src/app/Console/Commands/UserAddAlias.php
+++ b/src/app/Console/Commands/UserAddAlias.php
@@ -55,7 +55,7 @@
$controller = $user->wallet()->owner;
// Validate the alias
- $error = UsersController::validateEmail($alias, $controller, true);
+ $error = UsersController::validateAlias($alias, $controller);
if ($error) {
if (!$this->option('force')) {
diff --git a/src/app/Http/Controllers/API/SignupController.php b/src/app/Http/Controllers/API/SignupController.php
--- a/src/app/Http/Controllers/API/SignupController.php
+++ b/src/app/Http/Controllers/API/SignupController.php
@@ -371,7 +371,7 @@
// Check if user with specified login already exists
$email = $login . '@' . $domain;
- if (User::emailExists($email)) {
+ if (User::emailExists($email) || User::aliasExists($email)) {
return ['login' => \trans('validation.loginexists')];
}
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
@@ -554,7 +554,7 @@
if (empty($email)) {
$errors['email'] = \trans('validation.required', ['attribute' => 'email']);
- } elseif ($error = self::validateEmail($email, $controller, false)) {
+ } elseif ($error = self::validateEmail($email, $controller)) {
$errors['email'] = $error;
}
}
@@ -574,7 +574,7 @@
// validate new aliases
if (
!in_array($alias, $existing_aliases)
- && ($error = self::validateEmail($alias, $controller, true))
+ && ($error = self::validateAlias($alias, $controller))
) {
if (!isset($errors['aliases'])) {
$errors['aliases'] = [];
@@ -655,29 +655,81 @@
}
/**
- * Email address (login or alias) validation
+ * Email address validation for use as a user mailbox (login).
*
- * @param string $email Email address
- * @param \App\User $user The account owner
- * @param bool $is_alias The email is an alias
+ * @param string $email Email address
+ * @param \App\User $user The account owner
*
- * @return string Error message on validation error
+ * @return ?string Error message on validation error
*/
- public static function validateEmail(
- string $email,
- \App\User $user,
- bool $is_alias = false
- ): ?string {
- $attribute = $is_alias ? 'alias' : 'email';
+ public static function validateEmail(string $email, \App\User $user): ?string
+ {
+ if (strpos($email, '@') === false) {
+ return \trans('validation.entryinvalid', ['attribute' => 'email']);
+ }
+
+ list($login, $domain) = explode('@', Str::lower($email));
+
+ if (strlen($login) === 0 || strlen($domain) === 0) {
+ return \trans('validation.entryinvalid', ['attribute' => 'email']);
+ }
+
+ // Check if domain exists
+ $domain = Domain::where('namespace', $domain)->first();
+
+ if (empty($domain)) {
+ return \trans('validation.domaininvalid');
+ }
+
+ // Validate login part alone
+ $v = Validator::make(
+ ['email' => $login],
+ ['email' => ['required', new UserEmailLocal(!$domain->isPublic())]]
+ );
+
+ if ($v->fails()) {
+ return $v->errors()->toArray()['email'][0];
+ }
+ // Check if it is one of domains available to the user
+ $domains = \collect($user->domains())->pluck('namespace')->all();
+
+ if (!in_array($domain->namespace, $domains)) {
+ return \trans('validation.entryexists', ['attribute' => 'domain']);
+ }
+
+ // Check if a user with specified address already exists
+ if (User::emailExists($email)) {
+ // TODO: Allow force-delete if this is a deleted user in the same custom domain
+ return \trans('validation.entryexists', ['attribute' => 'email']);
+ }
+
+ // Check if an alias with specified address already exists.
+ if (User::aliasExists($email)) {
+ return \trans('validation.entryexists', ['attribute' => 'email']);
+ }
+
+ return null;
+ }
+
+ /**
+ * Email address validation for use as an alias.
+ *
+ * @param string $email Email address
+ * @param \App\User $user The account owner
+ *
+ * @return ?string Error message on validation error
+ */
+ public static function validateAlias(string $email, \App\User $user): ?string
+ {
if (strpos($email, '@') === false) {
- return \trans('validation.entryinvalid', ['attribute' => $attribute]);
+ return \trans('validation.entryinvalid', ['attribute' => 'alias']);
}
list($login, $domain) = explode('@', Str::lower($email));
if (strlen($login) === 0 || strlen($domain) === 0) {
- return \trans('validation.entryinvalid', ['attribute' => $attribute]);
+ return \trans('validation.entryinvalid', ['attribute' => 'alias']);
}
// Check if domain exists
@@ -689,12 +741,12 @@
// Validate login part alone
$v = Validator::make(
- [$attribute => $login],
- [$attribute => ['required', new UserEmailLocal(!$domain->isPublic())]]
+ ['alias' => $login],
+ ['alias' => ['required', new UserEmailLocal(!$domain->isPublic())]]
);
if ($v->fails()) {
- return $v->errors()->toArray()[$attribute][0];
+ return $v->errors()->toArray()['alias'][0];
}
// Check if it is one of domains available to the user
@@ -704,18 +756,20 @@
return \trans('validation.entryexists', ['attribute' => 'domain']);
}
- // 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
- // 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
- || $domain->isPublic()
- || $exists->wallet()->user_id != $user->id
- ) {
- return \trans('validation.entryexists', ['attribute' => $attribute]);
+ // Check if a user with specified address already exists
+ if ($existing_user = User::emailExists($email, true)) {
+ // Allow an alias in a custom domain to an address that was a user before
+ if ($domain->isPublic() || !$existing_user->trashed()) {
+ return \trans('validation.entryexists', ['attribute' => 'alias']);
+ }
+ }
+
+ // Check if an alias with specified address already exists
+ if (User::aliasExists($email)) {
+ // Allow assigning the same alias to a user in the same group account,
+ // but only for non-public domains
+ if ($domain->isPublic()) {
+ return \trans('validation.entryexists', ['attribute' => 'alias']);
}
}
diff --git a/src/app/Observers/UserAliasObserver.php b/src/app/Observers/UserAliasObserver.php
--- a/src/app/Observers/UserAliasObserver.php
+++ b/src/app/Observers/UserAliasObserver.php
@@ -29,24 +29,7 @@
\Log::error("Failed creating alias {$alias->alias}. Domain does not exist.");
return false;
}
-/*
- if ($exists = User::emailExists($alias->alias, true, $alias_exists, !$domain->isPublic())) {
- if (!$alias_exists) {
- \Log::error("Failed creating alias {$alias->alias}. Email address exists.");
- return false;
- }
- if ($domain->isPublic()) {
- \Log::error("Failed creating alias {$alias->alias}. Alias exists in public domain.");
- return false;
- }
-
- if ($exists->wallet()->user_id != $alias->user->wallet()->user_id) {
- \Log::error("Failed creating alias {$alias->alias}. Alias exists in another account.");
- return false;
- }
- }
-*/
return true;
}
diff --git a/src/app/Traits/UserAliasesTrait.php b/src/app/Traits/UserAliasesTrait.php
--- a/src/app/Traits/UserAliasesTrait.php
+++ b/src/app/Traits/UserAliasesTrait.php
@@ -5,6 +5,27 @@
trait UserAliasesTrait
{
/**
+ * Find whether an email address exists as an alias
+ * (including aliases of deleted users).
+ *
+ * @param string $email Email address
+ *
+ * @return bool True if found, False otherwise
+ */
+ public static function aliasExists(string $email): bool
+ {
+ if (strpos($email, '@') === false) {
+ return false;
+ }
+
+ $email = \strtolower($email);
+
+ $count = \App\UserAlias::where('alias', $email)->count();
+
+ return $count > 0;
+ }
+
+ /**
* A helper to update user aliases list.
*
* Example Usage:
diff --git a/src/app/User.php b/src/app/User.php
--- a/src/app/User.php
+++ b/src/app/User.php
@@ -350,17 +350,14 @@
}
/**
- * Find whether an email address exists (user or alias).
- * Note: This will also find deleted users.
+ * Find whether an email address exists as a user (including deleted users).
*
* @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, $existing = false)
+ public static function emailExists(string $email, bool $return_user = false)
{
if (strpos($email, '@') === false) {
return false;
@@ -368,30 +365,12 @@
$email = \strtolower($email);
- if ($existing) {
- $user = self::where('email', $email)->first();
- } else {
- $user = self::withTrashed()->where('email', $email)->first();
- }
+ $user = self::withTrashed()->where('email', $email)->first();
if ($user) {
return $return_user ? $user : true;
}
- $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 ? 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
@@ -1027,7 +1027,7 @@
}
/**
- * List of alias validation cases for testValidateEmail()
+ * List of email address validation cases for testValidateEmail()
*
* @return array Arguments for testValidateEmail()
*/
@@ -1043,61 +1043,113 @@
return [
// Invalid format
- ["$domain", $john, true, 'The specified alias is invalid.'],
- [".@$domain", $john, true, 'The specified alias is invalid.'],
- ["test123456@localhost", $john, true, 'The specified domain is invalid.'],
- ["test123456@unknown-domain.org", $john, true, 'The specified domain is invalid.'],
+ ["$domain", $john, 'The specified email is invalid.'],
+ [".@$domain", $john, 'The specified email is invalid.'],
+ ["test123456@localhost", $john, 'The specified domain is invalid.'],
+ ["test123456@unknown-domain.org", $john, 'The specified domain is invalid.'],
- ["$domain", $john, false, 'The specified email is invalid.'],
- [".@$domain", $john, false, 'The specified email is invalid.'],
+ ["$domain", $john, 'The specified email is invalid.'],
+ [".@$domain", $john, 'The specified email is invalid.'],
// forbidden local part on public domains
- ["admin@$domain", $john, true, 'The specified alias is not available.'],
- ["administrator@$domain", $john, true, 'The specified alias is not available.'],
+ ["admin@$domain", $john, 'The specified email is not available.'],
+ ["administrator@$domain", $john, 'The specified email is not available.'],
// forbidden (other user's domain)
- ["testtest@kolab.org", $user, true, 'The specified domain is not available.'],
+ ["testtest@kolab.org", $user, 'The specified domain is not available.'],
// existing alias of other user, to be a user email
- ["jack.daniels@kolab.org", $john, false, 'The specified email is not available.'],
+ ["jack.daniels@kolab.org", $john, 'The specified email is not available.'],
+
+ // valid (user domain)
+ ["admin@kolab.org", $john, null],
+
+ // valid (public domain)
+ ["test.test@$domain", $john, null],
+ ];
+ }
+
+ /**
+ * User email address validation.
+ *
+ * Note: Technically these include unit tests, but let's keep it here for now.
+ * FIXME: Shall we do a http request for each case?
+ *
+ * @dataProvider dataValidateEmail
+ */
+ public function testValidateEmail($email, $user, $expected_result): void
+ {
+ $result = UsersController::validateEmail($email, $user);
+ $this->assertSame($expected_result, $result);
+ }
+
+ /**
+ * List of alias validation cases for testValidateAlias()
+ *
+ * @return array Arguments for testValidateAlias()
+ */
+ public function dataValidateAlias(): array
+ {
+ $this->refreshApplication();
+ $public_domains = Domain::getPublicDomains();
+ $domain = reset($public_domains);
+
+ $john = $this->getTestUser('john@kolab.org');
+ $jack = $this->getTestUser('jack@kolab.org');
+ $user = $this->getTestUser('UsersControllerTest1@userscontroller.com');
+
+ return [
+ // Invalid format
+ ["$domain", $john, 'The specified alias is invalid.'],
+ [".@$domain", $john, 'The specified alias is invalid.'],
+ ["test123456@localhost", $john, 'The specified domain is invalid.'],
+ ["test123456@unknown-domain.org", $john, 'The specified domain is invalid.'],
+
+ ["$domain", $john, 'The specified alias is invalid.'],
+ [".@$domain", $john, 'The specified alias is invalid.'],
+
+ // forbidden local part on public domains
+ ["admin@$domain", $john, 'The specified alias is not available.'],
+ ["administrator@$domain", $john, 'The specified alias is not available.'],
+
+ // forbidden (other user's domain)
+ ["testtest@kolab.org", $user, 'The specified domain is not available.'],
// existing alias of other user, to be an alias, user in the same group account
- ["jack.daniels@kolab.org", $john, true, null],
+ ["jack.daniels@kolab.org", $john, null],
// existing user
- ["jack@kolab.org", $john, true, 'The specified alias is not available.'],
+ ["jack@kolab.org", $john, 'The specified alias is not available.'],
// valid (user domain)
- ["admin@kolab.org", $john, true, null],
+ ["admin@kolab.org", $john, null],
// valid (public domain)
- ["test.test@$domain", $john, true, null],
+ ["test.test@$domain", $john, null],
];
}
/**
- * User email/alias validation.
+ * User email alias validation.
*
* Note: Technically these include unit tests, but let's keep it here for now.
* FIXME: Shall we do a http request for each case?
*
- * @dataProvider dataValidateEmail
+ * @dataProvider dataValidateAlias
*/
- public function testValidateEmail($alias, $user, $is_alias, $expected_result): void
+ public function testValidateAlias($alias, $user, $expected_result): void
{
- $args = [$alias, $user, $is_alias];
- $result = $this->invokeMethod(new UsersController(), 'validateEmail', $args);
-
+ $result = UsersController::validateAlias($alias, $user);
$this->assertSame($expected_result, $result);
}
/**
- * User email/alias validation - more cases.
+ * User 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
+ public function testValidateAlias2(): void
{
Queue::fake();
@@ -1112,16 +1164,16 @@
$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);
+ $result = UsersController::validateAlias('deleted@kolab.org', $john);
$this->assertSame(null, $result);
- $result = UsersController::validateEmail('deleted-alias@kolab.org', $john, true);
+ $result = UsersController::validateAlias('deleted-alias@kolab.org', $john);
$this->assertSame(null, $result);
- $result = UsersController::validateEmail('deleted@kolabnow.com', $john, true);
+ $result = UsersController::validateAlias('deleted@kolabnow.com', $john);
$this->assertSame('The specified alias is not available.', $result);
- $result = UsersController::validateEmail('deleted-alias@kolabnow.com', $john, true);
+ $result = UsersController::validateAlias('deleted-alias@kolabnow.com', $john);
$this->assertSame('The specified alias is not available.', $result);
}
}
diff --git a/src/tests/Feature/UserTest.php b/src/tests/Feature/UserTest.php
--- a/src/tests/Feature/UserTest.php
+++ b/src/tests/Feature/UserTest.php
@@ -281,11 +281,27 @@
}
/**
+ * Tests for User::aliasExists()
+ */
+ public function testAliasExists(): void
+ {
+ $this->assertTrue(User::aliasExists('jack.daniels@kolab.org'));
+
+ $this->assertFalse(User::aliasExists('j.daniels@kolab.org'));
+ $this->assertFalse(User::aliasExists('john@kolab.org'));
+ }
+
+ /**
* Tests for User::emailExists()
*/
public function testEmailExists(): void
{
- $this->markTestIncomplete();
+ $this->assertFalse(User::emailExists('jack.daniels@kolab.org'));
+ $this->assertFalse(User::emailExists('j.daniels@kolab.org'));
+
+ $this->assertTrue(User::emailExists('john@kolab.org'));
+ $user = User::emailExists('john@kolab.org', true);
+ $this->assertSame('john@kolab.org', $user->email);
}
/**

File Metadata

Mime Type
text/plain
Expires
Thu, Apr 2, 9:57 PM (3 h, 54 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
18821090
Default Alt Text
D1930.1775167044.diff (19 KB)

Event Timeline