Page MenuHomePhorge
Files F117763945

D5412.1775221137.diff
No OneTemporary
Actions

Authored By
Unknown
Size
8 KB
Referenced Files
None
Subscribers
None

D5412.1775221137.diff
View Options

diff --git a/src/app/Http/Controllers/API/V4/GroupsController.php b/src/app/Http/Controllers/API/V4/GroupsController.php
--- a/src/app/Http/Controllers/API/V4/GroupsController.php
+++ b/src/app/Http/Controllers/API/V4/GroupsController.php
@@ -29,6 +29,14 @@
/** @var array Common object properties in the API response */
protected $objectProps = ['email', 'name'];
+ /**
+ * On group create it is filled with a user or group object to force-delete
+ * before the creation of a new group record is possible.
+ *
+ * @var User|Group|null
+ */
+ protected $deleteBeforeCreate;
+
/**
* Group status (extended) information
*
@@ -70,8 +78,10 @@
'name' => 'required|string|max:191',
];
+ $this->deleteBeforeCreate = null;
+
// Validate group address
- if ($error = self::validateGroupEmail($email, $owner)) {
+ if ($error = self::validateGroupEmail($email, $owner, $this->deleteBeforeCreate)) {
$errors['email'] = $error;
} else {
[, $domainName] = explode('@', $email);
@@ -108,6 +118,10 @@
DB::beginTransaction();
+ if ($this->deleteBeforeCreate) {
+ $this->deleteBeforeCreate->forceDelete();
+ }
+
// Create the group
$group = new Group();
$group->name = $request->input('name');
@@ -230,12 +244,14 @@
/**
* Validate an email address for use as a group email
*
- * @param string $email Email address
- * @param User $user The group owner
+ * @param string $email Email address
+ * @param User $user The group owner
+ * @param mixed $deleted Filled with an instance of a deleted model object
+ * with the specified email address, if exists
*
* @return ?string Error message on validation error
*/
- public static function validateGroupEmail($email, User $user): ?string
+ public static function validateGroupEmail($email, User $user, &$deleted = null): ?string
{
if (empty($email)) {
return self::trans('validation.required', ['attribute' => 'email']);
@@ -275,18 +291,15 @@
return $v->errors()->toArray()['email'][0];
}
- // Check if a user with specified address already exists
- if (User::emailExists($email)) {
- return self::trans('validation.entryexists', ['attribute' => 'email']);
- }
-
- // Check if an alias with specified address already exists.
- if (User::aliasExists($email)) {
- return self::trans('validation.entryexists', ['attribute' => 'email']);
- }
-
- if (Group::emailExists($email)) {
- return self::trans('validation.entryexists', ['attribute' => 'email']);
+ // Check if the address is already taken
+ if ($existing = self::findEmail($email)) {
+ // If this is a deleted user/group/resource/folder in the same custom domain
+ // we'll force delete it before creating the target group
+ if (is_object($existing) && !$domain->isPublic() && $existing->trashed()) {
+ $deleted = $existing;
+ } else {
+ return self::trans('validation.entryexists', ['attribute' => 'email']);
+ }
}
return null;
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
@@ -696,27 +696,17 @@
return self::trans('validation.entryexists', ['attribute' => 'domain']);
}
- // Check if a user/group/resource/shared folder with specified address already exists
- if (
- ($existing = User::emailExists($email, true))
- || ($existing = Group::emailExists($email, true))
- || ($existing = Resource::emailExists($email, true))
- || ($existing = SharedFolder::emailExists($email, true))
- ) {
+ // Check if the address is already taken
+ if ($existing = self::findEmail($email)) {
// If this is a deleted user/group/resource/folder in the same custom domain
// we'll force delete it before creating the target user
- if (!$domain->isPublic() && $existing->trashed()) {
+ if (is_object($existing) && !$domain->isPublic() && $existing->trashed()) {
$deleted = $existing;
} else {
return self::trans('validation.entryexists', ['attribute' => 'email']);
}
}
- // Check if an alias with specified address already exists.
- if (User::aliasExists($email) || SharedFolder::aliasExists($email)) {
- return self::trans('validation.entryexists', ['attribute' => 'email']);
- }
-
return null;
}
diff --git a/src/app/Http/Controllers/RelationController.php b/src/app/Http/Controllers/RelationController.php
--- a/src/app/Http/Controllers/RelationController.php
+++ b/src/app/Http/Controllers/RelationController.php
@@ -2,6 +2,10 @@
namespace App\Http\Controllers;
+use App\Group;
+use App\Resource;
+use App\SharedFolder;
+use App\User;
use Carbon\Carbon;
use Illuminate\Http\JsonResponse;
use Illuminate\Support\Str;
@@ -50,6 +54,28 @@
]);
}
+ /**
+ * Find object or alias by specified email address
+ *
+ * @param string $email Email address
+ *
+ * @return bool|object False if not found, True or object if found
+ */
+ protected static function findEmail($email)
+ {
+ if (
+ ($existing = User::emailExists($email, true))
+ || ($existing = Group::emailExists($email, true))
+ || ($existing = Resource::emailExists($email, true))
+ || ($existing = SharedFolder::emailExists($email, true))
+ ) {
+ return $existing;
+ }
+
+ // Check if an alias with specified address already exists.
+ return User::aliasExists($email) || SharedFolder::aliasExists($email);
+ }
+
/**
* Listing of resources belonging to the authenticated user.
*
diff --git a/src/tests/Feature/Controller/GroupsTest.php b/src/tests/Feature/Controller/GroupsTest.php
--- a/src/tests/Feature/Controller/GroupsTest.php
+++ b/src/tests/Feature/Controller/GroupsTest.php
@@ -5,6 +5,7 @@
use App\Domain;
use App\Group;
use App\Http\Controllers\API\V4\GroupsController;
+use App\User;
use Carbon\Carbon;
use Illuminate\Support\Facades\Queue;
use Tests\TestCase;
@@ -17,12 +18,16 @@
$this->deleteTestGroup('group-test@kolab.org');
$this->deleteTestGroup('group-test2@kolab.org');
+ $this->deleteTestGroup('deleted@kolab.org');
+ $this->deleteTestUser('deleted@kolab.org');
}
protected function tearDown(): void
{
$this->deleteTestGroup('group-test@kolab.org');
$this->deleteTestGroup('group-test2@kolab.org');
+ $this->deleteTestGroup('deleted@kolab.org');
+ $this->deleteTestUser('deleted@kolab.org');
parent::tearDown();
}
@@ -517,6 +522,30 @@
$this->assertCount(2, $json);
$this->assertCount(1, $json['errors']);
$this->assertSame("The specified name is not available.", $json['errors']['name'][0]);
+
+ // Test a group email that belongs to a deleted user
+ $user = $this->getTestUser('deleted@kolab.org');
+ $user->delete();
+
+ $post = [
+ 'name' => 'Test Group 2',
+ 'email' => $user->email,
+ 'members' => ['test3@domain.tld'],
+ ];
+
+ $response = $this->actingAs($john)->post("/api/v4/groups", $post);
+ $response->assertStatus(200);
+
+ $this->assertCount(0, User::withTrashed()->where('email', $post['email'])->get());
+
+ // Test a group email that belongs to a deleted group
+ $group = Group::where('email', $post['email'])->first();
+ $group->delete();
+
+ $response = $this->actingAs($john)->post("/api/v4/groups", $post);
+ $response->assertStatus(200);
+
+ $this->assertCount(1, Group::where('email', $post['email'])->get());
}
/**

File Metadata

Mime Type
text/plain
Expires
Fri, Apr 3, 12:58 PM (1 h, 45 m ago)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
18824147
Default Alt Text
D5412.1775221137.diff (8 KB)

Event Timeline