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

@@ -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);
+            $rules = collect(User::getUpdateRulesForId($this->route()->parameter('user')->id))->merge([
-
+                'ignore_connection_error' => ['sometimes', 'nullable', 'boolean'],
-            return array_merge($rules, [
-                'ignore_connection_error' => 'sometimes|nullable|boolean',
             ]);
         }
 
-        return User::getCreateRules();
+        return $rules->only([
-    }
+            'email', 'username', 'name_first', 'name_last', 'password',
-
+            'language', 'ignore_connection_error', 'root_admin',
-    /**
+        ])->toArray();
-     * @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();
     }
 }

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',

app/Notifications/AccountCreated.php

@@ -1,14 +1,8 @@
 <?php
-/**
- * Pterodactyl - Panel
- * Copyright (c) 2015 - 2017 Dane Everitt <dane@daneeveritt.com>.
- *
- * 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)) {
+        if (! is_null($this->token)) {
-            return $message->action('Setup Your Account', url('/auth/password/reset/' . $this->user->token . '?email=' . $notifiable->email));
+            return $message->action('Setup Your Account', url('/auth/password/reset/' . $this->token . '?email=' . $this->user->email));
         }
 
         return $message;

app/Services/Users/UserCreationService.php

@@ -1,77 +1,52 @@
 <?php
-/**
- * Pterodactyl - Panel
- * Copyright (c) 2015 - 2017 Dane Everitt <dane@daneeveritt.com>.
- *
- * 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;
+    private $hasher;
-
-    /**
-     * @var \Illuminate\Notifications\ChannelManager
-     */
-    protected $notification;
 
     /**
      * @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();
-
+        $user->notify(new AccountCreated($user, $token ?? null));
-        // @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,
-            ],
-        ]));
 
         return $user;
     }

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;
+    private $repository;
-
-    /**
-     * @var \Pterodactyl\Services\Helpers\TemporaryPasswordService
-     */
-    protected $passwordService;
-
-    /**
-     * @var \Pterodactyl\Contracts\Repository\UserRepositoryInterface
-     */
-    protected $repository;
-
-    /**
-     * @var \Pterodactyl\Services\Users\UserCreationService
-     */
-    protected $service;
 
     /**
      * Setup tests.
@@ -60,21 +45,11 @@ class UserCreationServiceTest extends TestCase
     {
         parent::setUp();
 
-        $this->appMock = m::mock(Application::class);
+        Notification::fake();
-        $this->database = m::mock(ConnectionInterface::class);
+        $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) [
+        $user = factory(User::class)->make();
-            'name_first' => 'FirstName',
-            'username' => 'user_name',
-        ];
 
         $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);
+        ], true, true)->once()->andReturn($user);
-        $this->database->shouldReceive('commit')->withNoArgs()->once()->andReturnNull();
+        $this->connection->shouldReceive('commit')->withNoArgs()->once()->andReturnNull();
-        $this->appMock->shouldReceive('makeWith')->with(AccountCreated::class, [
-            'user' => [
-                'name' => 'FirstName',
-                'username' => 'user_name',
-                'token' => null,
-            ],
-        ])->once()->andReturnNull();
 
-        $this->notification->shouldReceive('send')->with($user, null)->once()->andReturnNull();
+        $response = $this->getService()->handle([
-
-        $response = $this->service->handle([
             'password' => 'raw-password',
         ]);
 
         $this->assertNotNull($response);
-        $this->assertEquals($user->username, $response->username);
+        Notification::assertSentTo($user, AccountCreated::class, function ($notification) use ($user) {
-        $this->assertEquals($user->name_first, 'FirstName');
+            $this->assertSame($user, $notification->user);
+            $this->assertNull($notification->token);
+
+            return true;
+        });
     }
 
     /**
@@ -119,29 +86,29 @@ class UserCreationServiceTest extends TestCase
      */
     public function testUuidPassedInDataIsIgnored()
     {
-        $user = (object) [
+        $user = factory(User::class)->make();
-            'name_first' => 'FirstName',
-            'username' => 'user_name',
-        ];
 
         $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);
+        ], true, true)->once()->andReturn($user);
-        $this->database->shouldReceive('commit')->andReturnNull();
+        $this->connection->shouldReceive('commit')->andReturnNull();
-        $this->appMock->shouldReceive('makeWith')->andReturnNull();
-        $this->notification->shouldReceive('send')->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->assertInstanceOf(User::class, $response);
-        $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;
+        });
     }
 
     /**
@@ -149,44 +116,42 @@ class UserCreationServiceTest extends TestCase
      */
     public function testUserIsCreatedWhenNoPasswordIsProvided()
     {
-        $user = (object) [
+        $user = factory(User::class)->make();
-            'name_first' => 'FirstName',
-            'username' => 'user_name',
-            'email' => 'user@example.com',
-        ];
 
         $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')
+        $this->passwordService->shouldReceive('handle')->with($user->email)->once()->andReturn('random-token');
-            ->with('user@example.com')
-            ->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->connection->shouldReceive('commit')->withNoArgs()->once()->andReturnNull();
-        $this->appMock->shouldReceive('makeWith')->with(AccountCreated::class, [
-            'user' => [
-                'name' => 'FirstName',
-                'username' => 'user_name',
-                'token' => 'random-token',
-            ],
-        ])->once()->andReturnNull();
 
-        $this->notification->shouldReceive('send')->with($user, null)->once()->andReturnNull();
+        $response = $this->getService()->handle([
-
+            'email' => $user->email,
-        $response = $this->service->handle([
-            'email' => 'user@example.com',
         ]);
 
         $this->assertNotNull($response);
-        $this->assertEquals($user->username, $response->username);
+        $this->assertInstanceOf(User::class, $response);
-        $this->assertEquals($user->name_first, 'FirstName');
+        Notification::assertSentTo($user, AccountCreated::class, function ($notification) use ($user) {
-        $this->assertEquals($user->email, $response->email);
+            $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);
     }
 }