From 4457634127180bbfdfbaf99a0c9cb2fadfb5ce7b Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Mon, 1 Jan 2018 12:13:08 -0600 Subject: [PATCH] Fix user creation to use UUIDs correctly Also updates the notification send method to be cleaner and more maintainable --- app/Http/Requests/Admin/UserFormRequest.php | 31 +--- app/Models/User.php | 2 + app/Notifications/AccountCreated.php | 34 ++-- app/Services/Users/UserCreationService.php | 46 +----- .../Users/UserCreationServiceTest.php | 155 +++++++----------- 5 files changed, 97 insertions(+), 171 deletions(-) diff --git a/app/Http/Requests/Admin/UserFormRequest.php b/app/Http/Requests/Admin/UserFormRequest.php index ab760a7f1..c6a358398 100644 --- a/app/Http/Requests/Admin/UserFormRequest.php +++ b/app/Http/Requests/Admin/UserFormRequest.php @@ -7,34 +7,21 @@ use Pterodactyl\Models\User; class UserFormRequest extends AdminFormRequest { /** - * {@inheritdoc} + * Rules to apply to requests for updating or creating a user + * in the Admin CP. */ public function rules() { + $rules = collect(User::getCreateRules()); if ($this->method() === 'PATCH') { - $rules = User::getUpdateRulesForId($this->route()->parameter('user')->id); - - return array_merge($rules, [ - 'ignore_connection_error' => 'sometimes|nullable|boolean', + $rules = collect(User::getUpdateRulesForId($this->route()->parameter('user')->id))->merge([ + 'ignore_connection_error' => ['sometimes', 'nullable', 'boolean'], ]); } - return User::getCreateRules(); - } - - /** - * @param array|null $only - * @return array - */ - public function normalize(array $only = null) - { - if ($this->method === 'PATCH') { - return array_merge( - $this->all(['password']), - $this->only(['email', 'username', 'name_first', 'name_last', 'root_admin', 'language', 'ignore_connection_error']) - ); - } - - return parent::normalize(); + return $rules->only([ + 'email', 'username', 'name_first', 'name_last', 'password', + 'language', 'ignore_connection_error', 'root_admin', + ])->toArray(); } } diff --git a/app/Models/User.php b/app/Models/User.php index 02c7e7add..4452f0c33 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -115,6 +115,7 @@ class User extends Model implements * @var array */ protected static $applicationRules = [ + 'uuid' => 'required', 'email' => 'required', 'username' => 'required', 'name_first' => 'required', @@ -130,6 +131,7 @@ class User extends Model implements * @var array */ protected static $dataIntegrityRules = [ + 'uuid' => 'string|size:36|unique:users,uuid', 'email' => 'email|unique:users,email', 'username' => 'alpha_dash|between:1,255|unique:users,username', 'name_first' => 'string|between:1,255', diff --git a/app/Notifications/AccountCreated.php b/app/Notifications/AccountCreated.php index d83ee3fc7..8312c3bd1 100644 --- a/app/Notifications/AccountCreated.php +++ b/app/Notifications/AccountCreated.php @@ -1,14 +1,8 @@ . - * - * This software is licensed under the terms of the MIT license. - * https://opensource.org/licenses/MIT - */ namespace Pterodactyl\Notifications; +use Pterodactyl\Models\User; use Illuminate\Bus\Queueable; use Illuminate\Notifications\Notification; use Illuminate\Contracts\Queue\ShouldQueue; @@ -19,7 +13,15 @@ class AccountCreated extends Notification implements ShouldQueue use Queueable; /** - * The password reset token to send. + * The authentication token to be used for the user to set their + * password for the first time. + * + * @var string|null + */ + public $token; + + /** + * The user model for the created user. * * @var object */ @@ -28,11 +30,13 @@ class AccountCreated extends Notification implements ShouldQueue /** * Create a new notification instance. * - * @param aray $user + * @param \Pterodactyl\Models\User $user + * @param string|null $token */ - public function __construct(array $user) + public function __construct(User $user, string $token = null) { - $this->user = (object) $user; + $this->token = $token; + $this->user = $user; } /** @@ -56,12 +60,12 @@ class AccountCreated extends Notification implements ShouldQueue { $message = (new MailMessage) ->greeting('Hello ' . $this->user->name . '!') - ->line('You are recieving this email because an account has been created for you on Pterodactyl Panel.') + ->line('You are recieving this email because an account has been created for you on ' . config('app.name') . '.') ->line('Username: ' . $this->user->username) - ->line('Email: ' . $notifiable->email); + ->line('Email: ' . $this->user->email); - if (! is_null($this->user->token)) { - return $message->action('Setup Your Account', url('/auth/password/reset/' . $this->user->token . '?email=' . $notifiable->email)); + if (! is_null($this->token)) { + return $message->action('Setup Your Account', url('/auth/password/reset/' . $this->token . '?email=' . $this->user->email)); } return $message; diff --git a/app/Services/Users/UserCreationService.php b/app/Services/Users/UserCreationService.php index b267a18f5..f4824e48b 100644 --- a/app/Services/Users/UserCreationService.php +++ b/app/Services/Users/UserCreationService.php @@ -1,77 +1,52 @@ . - * - * This software is licensed under the terms of the MIT license. - * https://opensource.org/licenses/MIT - */ namespace Pterodactyl\Services\Users; use Ramsey\Uuid\Uuid; -use Illuminate\Foundation\Application; use Illuminate\Contracts\Hashing\Hasher; use Illuminate\Database\ConnectionInterface; -use Illuminate\Notifications\ChannelManager; use Pterodactyl\Notifications\AccountCreated; use Pterodactyl\Services\Helpers\TemporaryPasswordService; use Pterodactyl\Contracts\Repository\UserRepositoryInterface; class UserCreationService { - /** - * @var \Illuminate\Foundation\Application - */ - protected $app; - /** * @var \Illuminate\Database\ConnectionInterface */ - protected $connection; + private $connection; /** * @var \Illuminate\Contracts\Hashing\Hasher */ - protected $hasher; - - /** - * @var \Illuminate\Notifications\ChannelManager - */ - protected $notification; + private $hasher; /** * @var \Pterodactyl\Services\Helpers\TemporaryPasswordService */ - protected $passwordService; + private $passwordService; /** * @var \Pterodactyl\Contracts\Repository\UserRepositoryInterface */ - protected $repository; + private $repository; /** * CreationService constructor. * - * @param \Illuminate\Foundation\Application $application - * @param \Illuminate\Notifications\ChannelManager $notification * @param \Illuminate\Database\ConnectionInterface $connection * @param \Illuminate\Contracts\Hashing\Hasher $hasher * @param \Pterodactyl\Services\Helpers\TemporaryPasswordService $passwordService * @param \Pterodactyl\Contracts\Repository\UserRepositoryInterface $repository */ public function __construct( - Application $application, - ChannelManager $notification, ConnectionInterface $connection, Hasher $hasher, TemporaryPasswordService $passwordService, UserRepositoryInterface $repository ) { - $this->app = $application; $this->connection = $connection; $this->hasher = $hasher; - $this->notification = $notification; $this->passwordService = $passwordService; $this->repository = $repository; } @@ -97,20 +72,13 @@ class UserCreationService $token = $this->passwordService->handle($data['email']); } + /** @var \Pterodactyl\Models\User $user */ $user = $this->repository->create(array_merge($data, [ 'uuid' => Uuid::uuid4()->toString(), - ])); + ]), true, true); $this->connection->commit(); - - // @todo fire event, handle notification there - $this->notification->send($user, $this->app->makeWith(AccountCreated::class, [ - 'user' => [ - 'name' => $user->name_first, - 'username' => $user->username, - 'token' => $token ?? null, - ], - ])); + $user->notify(new AccountCreated($user, $token ?? null)); return $user; } diff --git a/tests/Unit/Services/Users/UserCreationServiceTest.php b/tests/Unit/Services/Users/UserCreationServiceTest.php index 5650b0322..4012dc655 100644 --- a/tests/Unit/Services/Users/UserCreationServiceTest.php +++ b/tests/Unit/Services/Users/UserCreationServiceTest.php @@ -4,11 +4,11 @@ namespace Tests\Unit\Services; use Mockery as m; use Tests\TestCase; +use Pterodactyl\Models\User; use Tests\Traits\MocksUuids; -use Illuminate\Foundation\Application; use Illuminate\Contracts\Hashing\Hasher; use Illuminate\Database\ConnectionInterface; -use Illuminate\Notifications\ChannelManager; +use Illuminate\Support\Facades\Notification; use Pterodactyl\Notifications\AccountCreated; use Pterodactyl\Services\Users\UserCreationService; use Pterodactyl\Services\Helpers\TemporaryPasswordService; @@ -19,39 +19,24 @@ class UserCreationServiceTest extends TestCase use MocksUuids; /** - * @var \Illuminate\Foundation\Application + * @var \Illuminate\Database\ConnectionInterface|\Mockery\Mock */ - protected $appMock; + private $connection; /** - * @var \Illuminate\Database\ConnectionInterface + * @var \Illuminate\Contracts\Hashing\Hasher|\Mockery\Mock */ - protected $database; + private $hasher; /** - * @var \Illuminate\Contracts\Hashing\Hasher + * @var \Pterodactyl\Services\Helpers\TemporaryPasswordService|\Mockery\Mock */ - protected $hasher; + private $passwordService; /** - * @var \Illuminate\Notifications\ChannelManager + * @var \Pterodactyl\Contracts\Repository\UserRepositoryInterface|\Mockery\Mock */ - protected $notification; - - /** - * @var \Pterodactyl\Services\Helpers\TemporaryPasswordService - */ - protected $passwordService; - - /** - * @var \Pterodactyl\Contracts\Repository\UserRepositoryInterface - */ - protected $repository; - - /** - * @var \Pterodactyl\Services\Users\UserCreationService - */ - protected $service; + private $repository; /** * Setup tests. @@ -60,21 +45,11 @@ class UserCreationServiceTest extends TestCase { parent::setUp(); - $this->appMock = m::mock(Application::class); - $this->database = m::mock(ConnectionInterface::class); + Notification::fake(); + $this->connection = m::mock(ConnectionInterface::class); $this->hasher = m::mock(Hasher::class); - $this->notification = m::mock(ChannelManager::class); $this->passwordService = m::mock(TemporaryPasswordService::class); $this->repository = m::mock(UserRepositoryInterface::class); - - $this->service = new UserCreationService( - $this->appMock, - $this->notification, - $this->database, - $this->hasher, - $this->passwordService, - $this->repository - ); } /** @@ -82,35 +57,27 @@ class UserCreationServiceTest extends TestCase */ public function testUserIsCreatedWhenPasswordIsProvided() { - $user = (object) [ - 'name_first' => 'FirstName', - 'username' => 'user_name', - ]; + $user = factory(User::class)->make(); $this->hasher->shouldReceive('make')->with('raw-password')->once()->andReturn('enc-password'); - $this->database->shouldReceive('beginTransaction')->withNoArgs()->once()->andReturnNull(); + $this->connection->shouldReceive('beginTransaction')->withNoArgs()->once()->andReturnNull(); $this->repository->shouldReceive('create')->with([ 'password' => 'enc-password', 'uuid' => $this->getKnownUuid(), - ])->once()->andReturn($user); - $this->database->shouldReceive('commit')->withNoArgs()->once()->andReturnNull(); - $this->appMock->shouldReceive('makeWith')->with(AccountCreated::class, [ - 'user' => [ - 'name' => 'FirstName', - 'username' => 'user_name', - 'token' => null, - ], - ])->once()->andReturnNull(); + ], true, true)->once()->andReturn($user); + $this->connection->shouldReceive('commit')->withNoArgs()->once()->andReturnNull(); - $this->notification->shouldReceive('send')->with($user, null)->once()->andReturnNull(); - - $response = $this->service->handle([ + $response = $this->getService()->handle([ 'password' => 'raw-password', ]); $this->assertNotNull($response); - $this->assertEquals($user->username, $response->username); - $this->assertEquals($user->name_first, 'FirstName'); + Notification::assertSentTo($user, AccountCreated::class, function ($notification) use ($user) { + $this->assertSame($user, $notification->user); + $this->assertNull($notification->token); + + return true; + }); } /** @@ -119,29 +86,29 @@ class UserCreationServiceTest extends TestCase */ public function testUuidPassedInDataIsIgnored() { - $user = (object) [ - 'name_first' => 'FirstName', - 'username' => 'user_name', - ]; + $user = factory(User::class)->make(); $this->hasher->shouldReceive('make')->andReturn('enc-password'); - $this->database->shouldReceive('beginTransaction')->andReturnNull(); + $this->connection->shouldReceive('beginTransaction')->andReturnNull(); $this->repository->shouldReceive('create')->with([ 'password' => 'enc-password', 'uuid' => $this->getKnownUuid(), - ])->once()->andReturn($user); - $this->database->shouldReceive('commit')->andReturnNull(); - $this->appMock->shouldReceive('makeWith')->andReturnNull(); - $this->notification->shouldReceive('send')->andReturnNull(); + ], true, true)->once()->andReturn($user); + $this->connection->shouldReceive('commit')->andReturnNull(); - $response = $this->service->handle([ + $response = $this->getService()->handle([ 'password' => 'raw-password', 'uuid' => 'test-uuid', ]); $this->assertNotNull($response); - $this->assertEquals($user->username, $response->username); - $this->assertEquals($user->name_first, 'FirstName'); + $this->assertInstanceOf(User::class, $response); + Notification::assertSentTo($user, AccountCreated::class, function ($notification) use ($user) { + $this->assertSame($user, $notification->user); + $this->assertNull($notification->token); + + return true; + }); } /** @@ -149,44 +116,42 @@ class UserCreationServiceTest extends TestCase */ public function testUserIsCreatedWhenNoPasswordIsProvided() { - $user = (object) [ - 'name_first' => 'FirstName', - 'username' => 'user_name', - 'email' => 'user@example.com', - ]; + $user = factory(User::class)->make(); $this->hasher->shouldNotReceive('make'); - $this->database->shouldReceive('beginTransaction')->withNoArgs()->once()->andReturnNull(); + $this->connection->shouldReceive('beginTransaction')->withNoArgs()->once()->andReturnNull(); $this->hasher->shouldReceive('make')->once()->andReturn('created-enc-password'); - $this->passwordService->shouldReceive('handle') - ->with('user@example.com') - ->once() - ->andReturn('random-token'); + $this->passwordService->shouldReceive('handle')->with($user->email)->once()->andReturn('random-token'); $this->repository->shouldReceive('create')->with([ 'password' => 'created-enc-password', - 'email' => 'user@example.com', + 'email' => $user->email, 'uuid' => $this->getKnownUuid(), - ])->once()->andReturn($user); + ], true, true)->once()->andReturn($user); - $this->database->shouldReceive('commit')->withNoArgs()->once()->andReturnNull(); - $this->appMock->shouldReceive('makeWith')->with(AccountCreated::class, [ - 'user' => [ - 'name' => 'FirstName', - 'username' => 'user_name', - 'token' => 'random-token', - ], - ])->once()->andReturnNull(); + $this->connection->shouldReceive('commit')->withNoArgs()->once()->andReturnNull(); - $this->notification->shouldReceive('send')->with($user, null)->once()->andReturnNull(); - - $response = $this->service->handle([ - 'email' => 'user@example.com', + $response = $this->getService()->handle([ + 'email' => $user->email, ]); $this->assertNotNull($response); - $this->assertEquals($user->username, $response->username); - $this->assertEquals($user->name_first, 'FirstName'); - $this->assertEquals($user->email, $response->email); + $this->assertInstanceOf(User::class, $response); + Notification::assertSentTo($user, AccountCreated::class, function ($notification) use ($user) { + $this->assertSame($user, $notification->user); + $this->assertSame('random-token', $notification->token); + + return true; + }); + } + + /** + * Return a new instance of the service using mocked dependencies. + * + * @return \Pterodactyl\Services\Users\UserCreationService + */ + private function getService(): UserCreationService + { + return new UserCreationService($this->connection, $this->hasher, $this->passwordService, $this->repository); } }