From 69f27ed807a86013473d396dec726d84fb3b7f83 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sat, 10 Oct 2020 16:46:56 -0700 Subject: [PATCH] Update and test variable validator logic --- .../Servers/VariableValidatorService.php | 66 ++----- .../Servers/VariableValidatorServiceTest.php | 137 ++++++++++++++ .../Servers/VariableValidatorServiceTest.php | 175 ------------------ 3 files changed, 152 insertions(+), 226 deletions(-) create mode 100644 tests/Integration/Services/Servers/VariableValidatorServiceTest.php delete mode 100644 tests/Unit/Services/Servers/VariableValidatorServiceTest.php diff --git a/app/Services/Servers/VariableValidatorService.php b/app/Services/Servers/VariableValidatorService.php index 5fa0607f6..7cb1aa427 100644 --- a/app/Services/Servers/VariableValidatorService.php +++ b/app/Services/Servers/VariableValidatorService.php @@ -11,32 +11,15 @@ namespace Pterodactyl\Services\Servers; use Pterodactyl\Models\User; use Illuminate\Support\Collection; +use Pterodactyl\Models\EggVariable; use Illuminate\Validation\ValidationException; use Pterodactyl\Traits\Services\HasUserLevels; -use Pterodactyl\Contracts\Repository\ServerRepositoryInterface; use Illuminate\Contracts\Validation\Factory as ValidationFactory; -use Pterodactyl\Contracts\Repository\EggVariableRepositoryInterface; -use Pterodactyl\Contracts\Repository\ServerVariableRepositoryInterface; class VariableValidatorService { use HasUserLevels; - /** - * @var \Pterodactyl\Contracts\Repository\EggVariableRepositoryInterface - */ - private $optionVariableRepository; - - /** - * @var \Pterodactyl\Contracts\Repository\ServerRepositoryInterface - */ - private $serverRepository; - - /** - * @var \Pterodactyl\Contracts\Repository\ServerVariableRepositoryInterface - */ - private $serverVariableRepository; - /** * @var \Illuminate\Contracts\Validation\Factory */ @@ -45,20 +28,10 @@ class VariableValidatorService /** * VariableValidatorService constructor. * - * @param \Pterodactyl\Contracts\Repository\EggVariableRepositoryInterface $optionVariableRepository - * @param \Pterodactyl\Contracts\Repository\ServerRepositoryInterface $serverRepository - * @param \Pterodactyl\Contracts\Repository\ServerVariableRepositoryInterface $serverVariableRepository * @param \Illuminate\Contracts\Validation\Factory $validator */ - public function __construct( - EggVariableRepositoryInterface $optionVariableRepository, - ServerRepositoryInterface $serverRepository, - ServerVariableRepositoryInterface $serverVariableRepository, - ValidationFactory $validator - ) { - $this->optionVariableRepository = $optionVariableRepository; - $this->serverRepository = $serverRepository; - $this->serverVariableRepository = $serverVariableRepository; + public function __construct(ValidationFactory $validator) + { $this->validator = $validator; } @@ -72,16 +45,18 @@ class VariableValidatorService */ public function handle(int $egg, array $fields = []): Collection { - $variables = $this->optionVariableRepository->findWhere([['egg_id', '=', $egg]]); + $query = EggVariable::query()->where('egg_id', $egg); + if (! $this->isUserLevel(User::USER_LEVEL_ADMIN)) { + // Don't attempt to validate variables if they aren't user editable + // and we're not running this at an admin level. + $query = $query->where('user_editable', true)->where('user_viewable', true); + } + + /** @var \Pterodactyl\Models\EggVariable[] $variables */ + $variables = $query->get(); $data = $rules = $customAttributes = []; foreach ($variables as $variable) { - // Don't attempt to validate variables if they aren't user editable - // and we're not running this at an admin level. - if (! $variable->user_editable && ! $this->isUserLevel(User::USER_LEVEL_ADMIN)) { - continue; - } - $data['environment'][$variable->env_variable] = array_get($fields, $variable->env_variable); $rules['environment.' . $variable->env_variable] = $variable->rules; $customAttributes['environment.' . $variable->env_variable] = trans('validation.internal.variable_value', ['env' => $variable->name]); @@ -92,23 +67,12 @@ class VariableValidatorService throw new ValidationException($validator); } - $response = $variables->filter(function ($item) { - // Skip doing anything if user is not an admin and variable is not user viewable or editable. - if (! $this->isUserLevel(User::USER_LEVEL_ADMIN) && (! $item->user_editable || ! $item->user_viewable)) { - return false; - } - - return true; - })->map(function ($item) use ($fields) { - return (object) [ + return Collection::make($variables)->map(function ($item) use ($fields) { + return (object)[ 'id' => $item->id, 'key' => $item->env_variable, - 'value' => array_get($fields, $item->env_variable), + 'value' => $fields[$item->env_variable] ?? null, ]; - })->filter(function ($item) { - return is_object($item); }); - - return $response; } } diff --git a/tests/Integration/Services/Servers/VariableValidatorServiceTest.php b/tests/Integration/Services/Servers/VariableValidatorServiceTest.php new file mode 100644 index 000000000..79a97ff73 --- /dev/null +++ b/tests/Integration/Services/Servers/VariableValidatorServiceTest.php @@ -0,0 +1,137 @@ +cloneEggAndVariables(Egg::query()->findOrFail(1)); + + try { + $this->getService()->handle($egg->id, [ + 'BUNGEE_VERSION' => '1.2.3', + ]); + + $this->assertTrue(false, 'This statement should not be reached.'); + } catch (ValidationException $exception) { + $errors = $exception->errors(); + + $this->assertCount(2, $errors); + $this->assertArrayHasKey('environment.BUNGEE_VERSION', $errors); + $this->assertArrayHasKey('environment.SERVER_JARFILE', $errors); + $this->assertSame('The Bungeecord Version variable may only contain letters and numbers.', $errors['environment.BUNGEE_VERSION'][0]); + $this->assertSame('The Bungeecord Jar File variable field is required.', $errors['environment.SERVER_JARFILE'][0]); + } + + $response = $this->getService()->handle($egg->id, [ + 'BUNGEE_VERSION' => '1234', + 'SERVER_JARFILE' => 'server.jar', + ]); + + $this->assertInstanceOf(Collection::class, $response); + $this->assertCount(2, $response); + $this->assertSame('BUNGEE_VERSION', $response->get(0)->key); + $this->assertSame('1234', $response->get(0)->value); + $this->assertSame('SERVER_JARFILE', $response->get(1)->key); + $this->assertSame('server.jar', $response->get(1)->value); + } + + /** + * Test that variables that are user_editable=false do not get validated (or returned) by + * the handler. + */ + public function testNormalUserCannotValidateNonUserEditableVariables() + { + /** @noinspection PhpParamsInspection */ + $egg = $this->cloneEggAndVariables(Egg::query()->findOrFail(1)); + $egg->variables()->first()->update([ + 'user_editable' => false, + ]); + + $response = $this->getService()->handle($egg->id, [ + // This is an invalid value, but it shouldn't cause any issues since it should be skipped. + 'BUNGEE_VERSION' => '1.2.3', + 'SERVER_JARFILE' => 'server.jar', + ]); + + $this->assertInstanceOf(Collection::class, $response); + $this->assertCount(1, $response); + $this->assertSame('SERVER_JARFILE', $response->get(0)->key); + $this->assertSame('server.jar', $response->get(0)->value); + } + + public function testEnvironmentVariablesCanBeUpdatedAsAdmin() + { + /** @noinspection PhpParamsInspection */ + $egg = $this->cloneEggAndVariables(Egg::query()->findOrFail(1)); + $egg->variables()->first()->update([ + 'user_editable' => false, + ]); + + try { + $this->getService()->setUserLevel(User::USER_LEVEL_ADMIN)->handle($egg->id, [ + 'BUNGEE_VERSION' => '1.2.3', + 'SERVER_JARFILE' => 'server.jar', + ]); + + $this->assertTrue(false, 'This statement should not be reached.'); + } catch (ValidationException $exception) { + $this->assertCount(1, $exception->errors()); + $this->assertArrayHasKey('environment.BUNGEE_VERSION', $exception->errors()); + } + + + $response = $this->getService()->setUserLevel(User::USER_LEVEL_ADMIN)->handle($egg->id, [ + 'BUNGEE_VERSION' => '123', + 'SERVER_JARFILE' => 'server.jar', + ]); + + $this->assertInstanceOf(Collection::class, $response); + $this->assertCount(2, $response); + $this->assertSame('BUNGEE_VERSION', $response->get(0)->key); + $this->assertSame('123', $response->get(0)->value); + $this->assertSame('SERVER_JARFILE', $response->get(1)->key); + $this->assertSame('server.jar', $response->get(1)->value); + } + + public function testNullableEnvironmentVariablesCanBeUsedCorrectly() + { + /** @noinspection PhpParamsInspection */ + $egg = $this->cloneEggAndVariables(Egg::query()->findOrFail(1)); + $egg->variables()->where('env_variable', '!=', 'BUNGEE_VERSION')->delete(); + + $egg->variables()->update(['rules' => 'nullable|string']); + + $response = $this->getService()->handle($egg->id, []); + $this->assertCount(1, $response); + $this->assertNull($response->get(0)->value); + + $response = $this->getService()->handle($egg->id, ['BUNGEE_VERSION' => null]); + $this->assertCount(1, $response); + $this->assertNull($response->get(0)->value); + + $response = $this->getService()->handle($egg->id, ['BUNGEE_VERSION' => '']); + $this->assertCount(1, $response); + $this->assertSame('', $response->get(0)->value); + } + + /** + * @return \Pterodactyl\Services\Servers\VariableValidatorService + */ + private function getService() + { + return $this->app->make(VariableValidatorService::class); + } +} diff --git a/tests/Unit/Services/Servers/VariableValidatorServiceTest.php b/tests/Unit/Services/Servers/VariableValidatorServiceTest.php deleted file mode 100644 index 29d9c0d6c..000000000 --- a/tests/Unit/Services/Servers/VariableValidatorServiceTest.php +++ /dev/null @@ -1,175 +0,0 @@ -optionVariableRepository = m::mock(EggVariableRepositoryInterface::class); - $this->serverRepository = m::mock(ServerRepositoryInterface::class); - $this->serverVariableRepository = m::mock(ServerVariableRepositoryInterface::class); - } - - /** - * Test that when no variables are found for an option no data is returned. - */ - public function testEmptyResultSetShouldBeReturnedIfNoVariablesAreFound() - { - $this->optionVariableRepository->shouldReceive('findWhere')->with([['egg_id', '=', 1]])->andReturn(collect([])); - - $response = $this->getService()->handle(1, []); - $this->assertEmpty($response); - $this->assertInstanceOf(Collection::class, $response); - } - - /** - * Test that variables set as user_editable=0 and/or user_viewable=0 are skipped when admin flag is not set. - */ - public function testValidatorShouldNotProcessVariablesSetAsNotUserEditableWhenAdminFlagIsNotPassed() - { - $variables = $this->getVariableCollection(); - $this->optionVariableRepository->shouldReceive('findWhere')->with([['egg_id', '=', 1]])->andReturn($variables); - - $response = $this->getService()->handle(1, [ - $variables[0]->env_variable => 'Test_SomeValue_0', - $variables[1]->env_variable => 'Test_SomeValue_1', - $variables[2]->env_variable => 'Test_SomeValue_2', - $variables[3]->env_variable => 'Test_SomeValue_3', - ]); - - $this->assertNotEmpty($response); - $this->assertInstanceOf(Collection::class, $response); - $this->assertEquals(1, $response->count(), 'Assert response has a single item in collection.'); - - $variable = $response->first(); - $this->assertObjectHasAttribute('id', $variable); - $this->assertObjectHasAttribute('key', $variable); - $this->assertObjectHasAttribute('value', $variable); - $this->assertSame($variables[0]->id, $variable->id); - $this->assertSame($variables[0]->env_variable, $variable->key); - $this->assertSame('Test_SomeValue_0', $variable->value); - } - - /** - * Test that all variables are processed correctly if admin flag is set. - */ - public function testValidatorShouldProcessAllVariablesWhenAdminFlagIsSet() - { - $variables = $this->getVariableCollection(); - $this->optionVariableRepository->shouldReceive('findWhere')->with([['egg_id', '=', 1]])->andReturn($variables); - - $service = $this->getService(); - $service->setUserLevel(User::USER_LEVEL_ADMIN); - $response = $service->handle(1, [ - $variables[0]->env_variable => 'Test_SomeValue_0', - $variables[1]->env_variable => 'Test_SomeValue_1', - $variables[2]->env_variable => 'Test_SomeValue_2', - $variables[3]->env_variable => 'Test_SomeValue_3', - ]); - - $this->assertNotEmpty($response); - $this->assertInstanceOf(Collection::class, $response); - $this->assertEquals(4, $response->count(), 'Assert response has all four items in collection.'); - - $response->each(function ($variable, $key) use ($variables) { - $this->assertObjectHasAttribute('id', $variable); - $this->assertObjectHasAttribute('key', $variable); - $this->assertObjectHasAttribute('value', $variable); - $this->assertSame($variables[$key]->id, $variable->id); - $this->assertSame($variables[$key]->env_variable, $variable->key); - $this->assertSame('Test_SomeValue_' . $key, $variable->value); - }); - } - - /** - * Test that a DisplayValidationError is thrown when a variable is not validated. - */ - public function testValidatorShouldThrowExceptionWhenAValidationErrorIsEncountered() - { - $variables = $this->getVariableCollection(); - $this->optionVariableRepository->shouldReceive('findWhere')->with([['egg_id', '=', 1]])->andReturn($variables); - - try { - $this->getService()->handle(1, [$variables[0]->env_variable => null]); - } catch (ValidationException $exception) { - $messages = $exception->validator->getMessageBag()->all(); - - $this->assertNotEmpty($messages); - $this->assertSame(2, count($messages)); - - // We only expect to get the first two variables form the getVariableCollection - // function here since those are the only two that are editable, and the others - // should be discarded and not validated. - for ($i = 0; $i < 2; $i++) { - $this->assertSame(trans('validation.required', [ - 'attribute' => trans('validation.internal.variable_value', ['env' => $variables[$i]->name]), - ]), $messages[$i]); - } - } - } - - /** - * Return a collection of fake variables to use for testing. - * - * @return \Illuminate\Support\Collection - */ - private function getVariableCollection(): Collection - { - return collect( - [ - factory(EggVariable::class)->states('editable', 'viewable')->make(), - factory(EggVariable::class)->states('editable')->make(), - factory(EggVariable::class)->states('viewable')->make(), - factory(EggVariable::class)->make(), - ] - ); - } - - /** - * Return an instance of the service with mocked dependencies. - * - * @return \Pterodactyl\Services\Servers\VariableValidatorService - */ - private function getService(): VariableValidatorService - { - return new VariableValidatorService( - $this->optionVariableRepository, - $this->serverRepository, - $this->serverVariableRepository, - $this->app->make(Factory::class) - ); - } -}