From 6e4cfe8f98fb4a0a57904f111ad264b2544fcc3a Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sun, 2 Dec 2018 13:38:59 -0800 Subject: [PATCH] Ensure that a Node's daemon secret can be properly reset --- .../Controllers/Admin/NodesController.php | 4 +-- .../Api/Application/Nodes/NodeController.php | 2 +- app/Services/Nodes/NodeUpdateService.php | 27 +++++++++++++++---- .../Services/Nodes/NodeUpdateServiceTest.php | 25 ++++++++++++----- 4 files changed, 44 insertions(+), 14 deletions(-) diff --git a/app/Http/Controllers/Admin/NodesController.php b/app/Http/Controllers/Admin/NodesController.php index 70c9ff071..ab1ce5eb7 100644 --- a/app/Http/Controllers/Admin/NodesController.php +++ b/app/Http/Controllers/Admin/NodesController.php @@ -263,7 +263,7 @@ class NodesController extends Controller */ public function updateSettings(NodeFormRequest $request, Node $node) { - $this->updateService->handle($node, $request->normalize()); + $this->updateService->handle($node, $request->normalize(), $request->input('reset_secret') === 'on'); $this->alert->success(trans('admin/node.notices.node_updated'))->flash(); return redirect()->route('admin.nodes.view.settings', $node->id)->withInput(); @@ -289,7 +289,7 @@ class NodesController extends Controller * Removes multiple individual allocations from a node. * * @param \Illuminate\Http\Request $request - * @param int $node + * @param int $node * @return \Illuminate\Http\Response * * @throws \Pterodactyl\Exceptions\Service\Allocation\ServerUsingAllocationException diff --git a/app/Http/Controllers/Api/Application/Nodes/NodeController.php b/app/Http/Controllers/Api/Application/Nodes/NodeController.php index 99be100ee..2ae2b9603 100644 --- a/app/Http/Controllers/Api/Application/Nodes/NodeController.php +++ b/app/Http/Controllers/Api/Application/Nodes/NodeController.php @@ -125,7 +125,7 @@ class NodeController extends ApplicationApiController public function update(UpdateNodeRequest $request): array { $node = $this->updateService->handle( - $request->getModel(Node::class), $request->validated() + $request->getModel(Node::class), $request->validated(), $request->input('reset_secret') === true ); return $this->fractal->item($node) diff --git a/app/Services/Nodes/NodeUpdateService.php b/app/Services/Nodes/NodeUpdateService.php index 16cc982de..219e2a34f 100644 --- a/app/Services/Nodes/NodeUpdateService.php +++ b/app/Services/Nodes/NodeUpdateService.php @@ -50,24 +50,41 @@ class NodeUpdateService * * @param \Pterodactyl\Models\Node $node * @param array $data + * @param bool $resetToken + * * @return \Pterodactyl\Models\Node * - * @throws \Pterodactyl\Exceptions\DisplayException * @throws \Pterodactyl\Exceptions\Model\DataValidationException * @throws \Pterodactyl\Exceptions\Repository\RecordNotFoundException + * @throws \Pterodactyl\Exceptions\Http\Connection\DaemonConnectionException + * @throws \Pterodactyl\Exceptions\Service\Node\ConfigurationNotPersistedException */ - public function handle(Node $node, array $data) + public function handle(Node $node, array $data, bool $resetToken = false) { - if (! is_null(array_get($data, 'reset_secret'))) { + if ($resetToken) { $data['daemonSecret'] = str_random(Node::DAEMON_SECRET_LENGTH); - unset($data['reset_secret']); } $this->connection->beginTransaction(); + + /** @var \Pterodactyl\Models\Node $updatedModel */ $updatedModel = $this->repository->update($node->id, $data); try { - $this->configRepository->setNode($updatedModel)->update(); + if ($resetToken) { + // We need to clone the new model and set it's authentication token to be the + // old one so we can connect. Then we will pass the new token through as an + // override on the call. + $cloned = $updatedModel->replicate(['daemonSecret']); + $cloned->setAttribute('daemonSecret', $node->getAttribute('daemonSecret')); + + $this->configRepository->setNode($cloned)->update([ + 'keys' => [$data['daemonSecret']], + ]); + } else { + $this->configRepository->setNode($updatedModel)->update(); + } + $this->connection->commit(); } catch (RequestException $exception) { // Failed to connect to the Daemon. Let's go ahead and save the configuration diff --git a/tests/Unit/Services/Nodes/NodeUpdateServiceTest.php b/tests/Unit/Services/Nodes/NodeUpdateServiceTest.php index 97f551330..9b757c2cd 100644 --- a/tests/Unit/Services/Nodes/NodeUpdateServiceTest.php +++ b/tests/Unit/Services/Nodes/NodeUpdateServiceTest.php @@ -51,21 +51,34 @@ class NodeUpdateServiceTest extends TestCase public function testNodeIsUpdatedAndDaemonSecretIsReset() { $model = factory(Node::class)->make(); + $updatedModel = factory(Node::class)->make([ + 'name' => 'New Name', + 'daemonSecret' => 'abcd1234', + ]); $this->getFunctionMock('\\Pterodactyl\\Services\\Nodes', 'str_random') - ->expects($this->once())->willReturn('random_string'); + ->expects($this->once())->willReturn($updatedModel->daemonSecret); $this->connection->shouldReceive('beginTransaction')->withNoArgs()->once()->andReturnNull(); $this->repository->shouldReceive('update')->with($model->id, [ - 'name' => 'NewName', - 'daemonSecret' => 'random_string', + 'name' => $updatedModel->name, + 'daemonSecret' => $updatedModel->daemonSecret, ])->andReturn($model); - $this->configRepository->shouldReceive('setNode')->with($model)->once()->andReturnSelf() - ->shouldReceive('update')->withNoArgs()->once()->andReturn(new Response); + $cloned = $updatedModel->replicate(['daemonSecret']); + $cloned->daemonSecret = $model->daemonSecret; + + $this->configRepository->shouldReceive('setNode')->with(m::on(function ($model) use ($updatedModel) { + return $model->daemonSecret !== $updatedModel->daemonSecret; + }))->once()->andReturnSelf(); + + $this->configRepository->shouldReceive('update')->with([ + 'keys' => ['abcd1234'], + ])->once()->andReturn(new Response); + $this->connection->shouldReceive('commit')->withNoArgs()->once()->andReturnNull(); - $response = $this->getService()->handle($model, ['name' => 'NewName', 'reset_secret' => true]); + $response = $this->getService()->handle($model, ['name' => $updatedModel->name], true); $this->assertInstanceOf(Node::class, $response); }