From 2ec76d283b14823b2ae69932bdeb71b21caaeed9 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sun, 4 Feb 2018 15:38:38 -0600 Subject: [PATCH] Fix bad API behavior --- CHANGELOG.md | 4 +++ .../Api/Application/Users/UserController.php | 5 ++- .../Middleware/Api/ApiSubstituteBindings.php | 2 ++ .../Application/Nodes/UpdateNodeRequest.php | 15 +-------- .../Api/Application/Users/GetUserRequest.php | 20 ------------ .../Application/Users/StoreUserRequest.php | 29 ++++++++++++++--- .../Application/Users/UpdateUserRequest.php | 28 +++------------- .../Api/Application/UserTransformer.php | 17 ++++++++-- ...02_04_145617_AllowTextInUserExternalId.php | 32 +++++++++++++++++++ routes/api-application.php | 2 +- 10 files changed, 86 insertions(+), 68 deletions(-) delete mode 100644 app/Http/Requests/Api/Application/Users/GetUserRequest.php create mode 100644 database/migrations/2018_02_04_145617_AllowTextInUserExternalId.php diff --git a/CHANGELOG.md b/CHANGELOG.md index f7fd464bd..369b972aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,10 @@ This file is a running track of new features and fixes to each version of the pa This project follows [Semantic Versioning](http://semver.org) guidelines. +## v0.7.0-rc.3 (Derelict Dermodactylus) +### Fixed +* `[rc.2]` — Fixes bad API behavior on `/user` routes. + ## v0.7.0-rc.2 (Derelict Dermodactylus) ### Fixed * `[rc.1]` — Fixes exception thrown when revoking user sessions. diff --git a/app/Http/Controllers/Api/Application/Users/UserController.php b/app/Http/Controllers/Api/Application/Users/UserController.php index 96d8c986f..316fc1c68 100644 --- a/app/Http/Controllers/Api/Application/Users/UserController.php +++ b/app/Http/Controllers/Api/Application/Users/UserController.php @@ -10,7 +10,6 @@ use Pterodactyl\Services\Users\UserCreationService; use Pterodactyl\Services\Users\UserDeletionService; use Pterodactyl\Contracts\Repository\UserRepositoryInterface; use Pterodactyl\Transformers\Api\Application\UserTransformer; -use Pterodactyl\Http\Requests\Api\Application\Users\GetUserRequest; use Pterodactyl\Http\Requests\Api\Application\Users\GetUsersRequest; use Pterodactyl\Http\Requests\Api\Application\Users\StoreUserRequest; use Pterodactyl\Http\Requests\Api\Application\Users\DeleteUserRequest; @@ -82,10 +81,10 @@ class UserController extends ApplicationApiController * Handle a request to view a single user. Includes any relations that * were defined in the request. * - * @param \Pterodactyl\Http\Requests\Api\Application\Users\GetUserRequest $request + * @param \Pterodactyl\Http\Requests\Api\Application\Users\GetUsersRequest $request * @return array */ - public function view(GetUserRequest $request): array + public function view(GetUsersRequest $request): array { return $this->fractal->item($request->getModel(User::class)) ->transformWith($this->getTransformer(UserTransformer::class)) diff --git a/app/Http/Middleware/Api/ApiSubstituteBindings.php b/app/Http/Middleware/Api/ApiSubstituteBindings.php index 41ca8ee70..b270be4ca 100644 --- a/app/Http/Middleware/Api/ApiSubstituteBindings.php +++ b/app/Http/Middleware/Api/ApiSubstituteBindings.php @@ -6,6 +6,7 @@ use Closure; use Pterodactyl\Models\Egg; use Pterodactyl\Models\Nest; use Pterodactyl\Models\Node; +use Pterodactyl\Models\User; use Pterodactyl\Models\Server; use Pterodactyl\Models\Database; use Pterodactyl\Models\Location; @@ -28,6 +29,7 @@ class ApiSubstituteBindings extends SubstituteBindings 'nest' => Nest::class, 'node' => Node::class, 'server' => Server::class, + 'user' => User::class, ]; /** diff --git a/app/Http/Requests/Api/Application/Nodes/UpdateNodeRequest.php b/app/Http/Requests/Api/Application/Nodes/UpdateNodeRequest.php index f6e92f812..ffba39e6d 100644 --- a/app/Http/Requests/Api/Application/Nodes/UpdateNodeRequest.php +++ b/app/Http/Requests/Api/Application/Nodes/UpdateNodeRequest.php @@ -6,19 +6,6 @@ use Pterodactyl\Models\Node; class UpdateNodeRequest extends StoreNodeRequest { - /** - * Determine if the node being requested for editing exists - * on the Panel before validating the data. - * - * @return bool - */ - public function resourceExists(): bool - { - $node = $this->route()->parameter('node'); - - return $node instanceof Node && $node->exists; - } - /** * Apply validation rules to this request. Uses the parent class rules() * function but passes in the rules for updating rather than creating. @@ -28,7 +15,7 @@ class UpdateNodeRequest extends StoreNodeRequest */ public function rules(array $rules = null): array { - $nodeId = $this->route()->parameter('node')->id; + $nodeId = $this->getModel(Node::class)->id; return parent::rules(Node::getUpdateRulesForId($nodeId)); } diff --git a/app/Http/Requests/Api/Application/Users/GetUserRequest.php b/app/Http/Requests/Api/Application/Users/GetUserRequest.php deleted file mode 100644 index 0c96e3bb4..000000000 --- a/app/Http/Requests/Api/Application/Users/GetUserRequest.php +++ /dev/null @@ -1,20 +0,0 @@ -route()->parameter('user'); - - return $user instanceof User && $user->exists; - } -} diff --git a/app/Http/Requests/Api/Application/Users/StoreUserRequest.php b/app/Http/Requests/Api/Application/Users/StoreUserRequest.php index e6416826c..71153836d 100644 --- a/app/Http/Requests/Api/Application/Users/StoreUserRequest.php +++ b/app/Http/Requests/Api/Application/Users/StoreUserRequest.php @@ -21,20 +21,41 @@ class StoreUserRequest extends ApplicationApiRequest /** * Return the validation rules for this request. * + * @param array|null $rules * @return array */ - public function rules(): array + public function rules(array $rules = null): array { - return collect(User::getCreateRules())->only([ + $rules = $rules ?? User::getCreateRules(); + + $response = collect($rules)->only([ 'external_id', 'email', 'username', - 'name_first', - 'name_last', 'password', 'language', 'root_admin', ])->toArray(); + + $response['first_name'] = $rules['name_first']; + $response['last_name'] = $rules['name_last']; + + return $response; + } + + /** + * @return array + */ + public function validated() + { + $data = parent::validated(); + + $data['name_first'] = $data['first_name']; + $data['name_last'] = $data['last_name']; + + unset($data['first_name'], $data['last_name']); + + return $data; } /** diff --git a/app/Http/Requests/Api/Application/Users/UpdateUserRequest.php b/app/Http/Requests/Api/Application/Users/UpdateUserRequest.php index 10b6c54a5..929a77f32 100644 --- a/app/Http/Requests/Api/Application/Users/UpdateUserRequest.php +++ b/app/Http/Requests/Api/Application/Users/UpdateUserRequest.php @@ -6,36 +6,16 @@ use Pterodactyl\Models\User; class UpdateUserRequest extends StoreUserRequest { - /** - * Determine if the requested user exists on the Panel. - * - * @return bool - */ - public function resourceExists(): bool - { - $user = $this->route()->parameter('user'); - - return $user instanceof User && $user->exists; - } - /** * Return the validation rules for this request. * + * @param array|null $rules * @return array */ - public function rules(): array + public function rules(array $rules = null): array { - $userId = $this->route()->parameter('user')->id; + $userId = $this->getModel(User::class)->id; - return collect(User::getUpdateRulesForId($userId))->only([ - 'external_id', - 'email', - 'username', - 'name_first', - 'name_last', - 'password', - 'language', - 'root_admin', - ])->toArray(); + return parent::rules(User::getUpdateRulesForId($userId)); } } diff --git a/app/Transformers/Api/Application/UserTransformer.php b/app/Transformers/Api/Application/UserTransformer.php index d3d680ed7..56749deaa 100644 --- a/app/Transformers/Api/Application/UserTransformer.php +++ b/app/Transformers/Api/Application/UserTransformer.php @@ -25,14 +25,27 @@ class UserTransformer extends BaseTransformer } /** - * Return a generic transformed subuser array. + * Return a transformed User model that can be consumed by external services. * * @param \Pterodactyl\Models\User $user * @return array */ public function transform(User $user): array { - return $user->toArray(); + return [ + 'id' => $user->id, + 'external_id' => $user->external_id, + 'uuid' => $user->uuid, + 'username' => $user->username, + 'email' => $user->email, + 'first_name' => $user->name_first, + 'last_name' => $user->name_last, + 'language' => $user->language, + 'root_admin' => (bool) $user->root_admin, + '2fa' => (bool) $user->use_totp, + 'created_at' => $this->formatTimestamp($user->created_at), + 'updated_at' => $this->formatTimestamp($user->updated_at), + ]; } /** diff --git a/database/migrations/2018_02_04_145617_AllowTextInUserExternalId.php b/database/migrations/2018_02_04_145617_AllowTextInUserExternalId.php new file mode 100644 index 000000000..6a4a04e7d --- /dev/null +++ b/database/migrations/2018_02_04_145617_AllowTextInUserExternalId.php @@ -0,0 +1,32 @@ +string('external_id')->nullable()->change(); + }); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::table('users', function (Blueprint $table) { + $table->unsignedInteger('external_id')->change(); + }); + } +} diff --git a/routes/api-application.php b/routes/api-application.php index 049ba391b..212b75c2b 100644 --- a/routes/api-application.php +++ b/routes/api-application.php @@ -10,7 +10,7 @@ */ Route::group(['prefix' => '/users'], function () { Route::get('/', 'Users\UserController@index')->name('api.application.users'); - Route::get('/{user}', 'Users\UserController@view')->name('api.applications.users.view'); + Route::get('/{user}', 'Users\UserController@view')->name('api.application.users.view'); Route::post('/', 'Users\UserController@store'); Route::patch('/{user}', 'Users\UserController@update');