From d8228f2da8c4650cb51df1dd678c6451a3d3f327 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sat, 10 Oct 2020 16:45:24 -0700 Subject: [PATCH] Allow passing empty values through for variables, covers with test, closes #2433 --- .../Api/Client/Servers/StartupController.php | 2 +- .../Startup/UpdateStartupVariableRequest.php | 2 +- .../Startup/GetStartupAndVariablesTest.php | 69 ++++++++ .../Startup/UpdateStartupVariableTest.php | 161 ++++++++++++++++++ 4 files changed, 232 insertions(+), 2 deletions(-) create mode 100644 tests/Integration/Api/Client/Server/Startup/GetStartupAndVariablesTest.php create mode 100644 tests/Integration/Api/Client/Server/Startup/UpdateStartupVariableTest.php diff --git a/app/Http/Controllers/Api/Client/Servers/StartupController.php b/app/Http/Controllers/Api/Client/Servers/StartupController.php index 8ab62a02c..e0c580279 100644 --- a/app/Http/Controllers/Api/Client/Servers/StartupController.php +++ b/app/Http/Controllers/Api/Client/Servers/StartupController.php @@ -100,7 +100,7 @@ class StartupController extends ClientApiController 'server_id' => $server->id, 'variable_id' => $variable->id, ], [ - 'variable_value' => $request->input('value'), + 'variable_value' => $request->input('value') ?? '', ]); $variable = $variable->refresh(); diff --git a/app/Http/Requests/Api/Client/Servers/Startup/UpdateStartupVariableRequest.php b/app/Http/Requests/Api/Client/Servers/Startup/UpdateStartupVariableRequest.php index 63005c78b..b46e6ea9a 100644 --- a/app/Http/Requests/Api/Client/Servers/Startup/UpdateStartupVariableRequest.php +++ b/app/Http/Requests/Api/Client/Servers/Startup/UpdateStartupVariableRequest.php @@ -24,7 +24,7 @@ class UpdateStartupVariableRequest extends ClientApiRequest { return [ 'key' => 'required|string', - 'value' => 'present|string', + 'value' => 'present', ]; } } diff --git a/tests/Integration/Api/Client/Server/Startup/GetStartupAndVariablesTest.php b/tests/Integration/Api/Client/Server/Startup/GetStartupAndVariablesTest.php new file mode 100644 index 000000000..6daa05958 --- /dev/null +++ b/tests/Integration/Api/Client/Server/Startup/GetStartupAndVariablesTest.php @@ -0,0 +1,69 @@ +generateTestAccount($permissions); + + $egg = $this->cloneEggAndVariables($server->egg); + // BUNGEE_VERSION should never be returned back to the user in this API call, either in + // the array of variables, or revealed in the startup command. + $egg->variables()->first()->update([ + 'user_viewable' => false, + ]); + + $server->fill([ + 'egg_id' => $egg->id, + 'startup' => 'java {{SERVER_JARFILE}} --version {{BUNGEE_VERSION}}', + ])->save(); + $server = $server->refresh(); + + $response = $this->actingAs($user)->getJson($this->link($server) . "/startup"); + + $response->assertOk(); + $response->assertJsonPath('meta.startup_command', 'java bungeecord.jar --version [hidden]'); + $response->assertJsonPath('meta.raw_startup_command', $server->startup); + + $response->assertJsonPath('object', 'list'); + $response->assertJsonCount(1, 'data'); + $response->assertJsonPath('data.0.object', EggVariable::RESOURCE_NAME); + $this->assertJsonTransformedWith($response->json('data.0.attributes'), $egg->variables[1]); + } + + /** + * Test that a user without the required permission, or who does not have any permission to + * access the server cannot get the startup information for it. + */ + public function testStartupDataIsNotReturnedWithoutPermission() + { + [$user, $server] = $this->generateTestAccount([Permission::ACTION_WEBSOCKET_CONNECT]); + $this->actingAs($user)->getJson($this->link($server) . "/startup")->assertForbidden(); + + $user2 = factory(User::class)->create(); + $this->actingAs($user2)->getJson($this->link($server) . "/startup")->assertNotFound(); + } + + /** + * @return array[] + */ + public function permissionsDataProvider() + { + return [[[]], [[Permission::ACTION_STARTUP_READ]]]; + } +} diff --git a/tests/Integration/Api/Client/Server/Startup/UpdateStartupVariableTest.php b/tests/Integration/Api/Client/Server/Startup/UpdateStartupVariableTest.php new file mode 100644 index 000000000..3ed1a89cc --- /dev/null +++ b/tests/Integration/Api/Client/Server/Startup/UpdateStartupVariableTest.php @@ -0,0 +1,161 @@ +generateTestAccount($permissions); + $server->fill([ + 'startup' => 'java {{SERVER_JARFILE}} --version {{BUNGEE_VERSION}}', + ])->save(); + + $response = $this->actingAs($user)->putJson($this->link($server) . '/startup/variable', [ + 'key' => 'BUNGEE_VERSION', + 'value' => '1.2.3', + ]); + + $response->assertStatus(Response::HTTP_UNPROCESSABLE_ENTITY); + $response->assertJsonPath('errors.0.code', 'ValidationException'); + $response->assertJsonPath('errors.0.detail', 'The value may only contain letters and numbers.'); + + $response = $this->actingAs($user)->putJson($this->link($server) . '/startup/variable', [ + 'key' => 'BUNGEE_VERSION', + 'value' => '123', + ]); + + $response->assertOk(); + $response->assertJsonPath('object', EggVariable::RESOURCE_NAME); + $this->assertJsonTransformedWith($response->json('attributes'), $server->variables[0]); + $response->assertJsonPath('meta.startup_command', 'java bungeecord.jar --version 123'); + $response->assertJsonPath('meta.raw_startup_command', $server->startup); + } + + /** + * Test that variables that are either not user_viewable, or not user_editable, cannot be + * updated via this endpoint. + * + * @param array $permissions + * @dataProvider permissionsDataProvider + */ + public function testStartupVariableCannotBeUpdatedIfNotUserViewableOrEditable(array $permissions) + { + /** @var \Pterodactyl\Models\Server $server */ + [$user, $server] = $this->generateTestAccount($permissions); + + $egg = $this->cloneEggAndVariables($server->egg); + $egg->variables()->where('env_variable', 'BUNGEE_VERSION')->update(['user_viewable' => false]); + $egg->variables()->where('env_variable', 'SERVER_JARFILE')->update(['user_editable' => false]); + + $server->fill(['egg_id' => $egg->id])->save(); + $server->refresh(); + + $response = $this->actingAs($user)->putJson($this->link($server) . '/startup/variable', [ + 'key' => 'BUNGEE_VERSION', + 'value' => '123', + ]); + + $response->assertStatus(Response::HTTP_BAD_REQUEST); + $response->assertJsonPath('errors.0.code', 'BadRequestHttpException'); + $response->assertJsonPath('errors.0.detail', 'The environment variable you are trying to edit does not exist.'); + + $response = $this->actingAs($user)->putJson($this->link($server) . '/startup/variable', [ + 'key' => 'SERVER_JARFILE', + 'value' => 'server2.jar', + ]); + + $response->assertStatus(Response::HTTP_BAD_REQUEST); + $response->assertJsonPath('errors.0.code', 'BadRequestHttpException'); + $response->assertJsonPath('errors.0.detail', 'The environment variable you are trying to edit is read-only.'); + } + + /** + * Test that a hidden variable is not included in the startup_command output for the server if + * a different variable is updated. + */ + public function testHiddenVariablesAreNotReturnedInStartupCommandWhenUpdatingVariable() + { + /** @var \Pterodactyl\Models\Server $server */ + [$user, $server] = $this->generateTestAccount(); + + $egg = $this->cloneEggAndVariables($server->egg); + $egg->variables()->first()->update(['user_viewable' => false]); + + $server->fill([ + 'egg_id' => $egg->id, + 'startup' => 'java {{SERVER_JARFILE}} --version {{BUNGEE_VERSION}}', + ])->save(); + + $server->refresh(); + + $response = $this->actingAs($user)->putJson($this->link($server) . '/startup/variable', [ + 'key' => 'SERVER_JARFILE', + 'value' => 'server2.jar', + ]); + + $response->assertOk(); + $response->assertJsonPath('meta.startup_command', 'java server2.jar --version [hidden]'); + $response->assertJsonPath('meta.raw_startup_command', $server->startup); + } + + /** + * Test that an egg variable with a validation rule of 'nullable|string' works if no value + * is passed through in the request. + * + * @see https://github.com/pterodactyl/panel/issues/2433 + */ + public function testEggVariableWithNullableStringIsNotRequired() + { + /** @var \Pterodactyl\Models\Server $server */ + [$user, $server] = $this->generateTestAccount(); + + $egg = $this->cloneEggAndVariables($server->egg); + $egg->variables()->first()->update(['rules' => 'nullable|string']); + + $server->fill(['egg_id' => $egg->id])->save(); + $server->refresh(); + + $response = $this->actingAs($user)->putJson($this->link($server) . '/startup/variable', [ + 'key' => 'BUNGEE_VERSION', + 'value' => '', + ]); + + $response->assertOk(); + $response->assertJsonPath('attributes.server_value', null); + } + + /** + * Test that a variable cannot be updated if the user does not have permission to perform + * that action, or they aren't assigned at all to the server. + */ + public function testStartupVariableCannotBeUpdatedIfNotUserViewable() + { + [$user, $server] = $this->generateTestAccount([Permission::ACTION_WEBSOCKET_CONNECT]); + $this->actingAs($user)->putJson($this->link($server) . "/startup/variable")->assertForbidden(); + + $user2 = factory(User::class)->create(); + $this->actingAs($user2)->putJson($this->link($server) . "/startup/variable")->assertNotFound(); + } + + /** + * @return \array[][] + */ + public function permissionsDataProvider() + { + return [[[]], [[Permission::ACTION_STARTUP_UPDATE]]]; + } +}