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 @@ -17,7 +17,7 @@ class UsersController extends Controller { - // List of user settings keys available for modification in UI + /** @const array List of user setting keys available for modification in UI */ public const USER_SETTINGS = [ 'billing_address', 'country', @@ -30,6 +30,15 @@ ]; /** + * On user create it is filled with a user object to force-delete + * before the creation of a new user record is possible. + * + * @var \App\User|null + */ + protected $deleteBeforeCreate; + + + /** * Delete a user. * * @param int $id User identifier @@ -270,6 +279,8 @@ return $this->errorResponse(403); } + $this->deleteBeforeCreate = null; + if ($error_response = $this->validateUserRequest($request, null, $settings)) { return $error_response; } @@ -286,6 +297,10 @@ DB::beginTransaction(); + if ($this->deleteBeforeCreate) { + $this->deleteBeforeCreate->forceDelete(); + } + // Create user record $user = User::create([ 'email' => $request->email, @@ -554,7 +569,7 @@ if (empty($email)) { $errors['email'] = \trans('validation.required', ['attribute' => 'email']); - } elseif ($error = self::validateEmail($email, $controller)) { + } elseif ($error = self::validateEmail($email, $controller, $this->deleteBeforeCreate)) { $errors['email'] = $error; } } @@ -657,13 +672,17 @@ /** * Email address validation for use as a user mailbox (login). * - * @param string $email Email address - * @param \App\User $user The account owner + * @param string $email Email address + * @param \App\User $user The account owner + * @param ?\App\User $deleted Filled with an instance of a deleted user with + * the specified email address, if exists * * @return ?string Error message on validation error */ - public static function validateEmail(string $email, \App\User $user): ?string + public static function validateEmail(string $email, \App\User $user, &$deleted = null): ?string { + $deleted = null; + if (strpos($email, '@') === false) { return \trans('validation.entryinvalid', ['attribute' => 'email']); } @@ -699,9 +718,14 @@ } // 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']); + if ($existing_user = User::emailExists($email, true)) { + // If this is a deleted user in the same custom domain + // we'll force delete him before + if (!$domain->isPublic() && $existing_user->trashed()) { + $deleted = $existing_user; + } else { + return \trans('validation.entryexists', ['attribute' => 'email']); + } } // Check if an alias with specified address already exists. 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 @@ -462,6 +462,8 @@ */ public function testStore(): void { + Queue::fake(); + $jack = $this->getTestUser('jack@kolab.org'); $john = $this->getTestUser('john@kolab.org'); $deleted_priv = $this->getTestUser('deleted@kolab.org'); @@ -579,18 +581,29 @@ $wallet = $user->wallet(); $this->assertSame($john->wallets()->first()->id, $wallet->id); - // Test acting as account controller (not owner) - /* - // FIXME: How do we know to which wallet the new user should be assigned to? + // Attempt to create a user previously deleted + $user->delete(); - $this->deleteTestUser('john2.doe2@kolab.org'); - $response = $this->actingAs($ned)->post("/api/v4/users", $post); + $post['package'] = $package_kolab->id; + $post['aliases'] = []; + $response = $this->actingAs($john)->post("/api/v4/users", $post); $json = $response->json(); $response->assertStatus(200); $this->assertSame('success', $json['status']); - */ + $this->assertSame("User created successfully.", $json['message']); + $this->assertCount(2, $json); + + $user = User::where('email', 'john2.doe2@kolab.org')->first(); + $this->assertInstanceOf(User::class, $user); + $this->assertSame('John2', $user->getSetting('first_name')); + $this->assertSame('Doe2', $user->getSetting('last_name')); + $this->assertSame('TestOrg', $user->getSetting('organization')); + $this->assertCount(0, $user->aliases()->get()); + $this->assertUserEntitlements($user, ['groupware', 'mailbox', 'storage', 'storage']); + + // Test acting as account controller (not owner) $this->markTestIncomplete(); } @@ -1084,6 +1097,35 @@ } /** + * User email validation - tests for $deleted argument + * + * 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 testValidateEmailDeleted(): void + { + Queue::fake(); + + $john = $this->getTestUser('john@kolab.org'); + $deleted_priv = $this->getTestUser('deleted@kolab.org'); + $deleted_priv->delete(); + $deleted_pub = $this->getTestUser('deleted@kolabnow.com'); + $deleted_pub->delete(); + + $result = UsersController::validateEmail('deleted@kolab.org', $john, $deleted); + $this->assertSame(null, $result); + $this->assertSame($deleted_priv->id, $deleted->id); + + $result = UsersController::validateEmail('deleted@kolabnow.com', $john, $deleted); + $this->assertSame('The specified email is not available.', $result); + $this->assertSame(null, $deleted); + + $result = UsersController::validateEmail('jack@kolab.org', $john, $deleted); + $this->assertSame('The specified email is not available.', $result); + $this->assertSame(null, $deleted); + } + + /** * List of alias validation cases for testValidateAlias() * * @return array Arguments for testValidateAlias()