Skip to content

Commit 33695c6

Browse files
MrSoulPenguinMrSoulPenguin
authored
Fix transfer status permission checks (#5573)
1 parent a5e278e commit 33695c6

4 files changed

Lines changed: 159 additions & 4 deletions

File tree

‎app/Http/Controllers/Api/Remote/Servers/ServerTransferController.php‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22

33
namespace Pterodactyl\Http\Controllers\Api\Remote\Servers;
44

5+
use Illuminate\Http\Request;
6+
use Pterodactyl\Models\Node;
7+
use Webmozart\Assert\Assert;
58
use Illuminate\Http\Response;
69
use Illuminate\Http\JsonResponse;
710
use Pterodactyl\Models\Allocation;
@@ -32,17 +35,20 @@ public function __construct(
3235
*
3336
* @throws \Throwable
3437
*/
35-
public function failure(string $uuid): JsonResponse
38+
public function failure(Request $request, string $uuid): JsonResponse
3639
{
3740
$server = $this->repository->getByUuid($uuid);
3841
$transfer = $server->transfer;
3942
if (is_null($transfer)) {
4043
throw new ConflictHttpException('Server is not being transferred.');
4144
}
4245

46+
/* @var Node $node */
47+
Assert::isInstanceOf($node = $request->attributes->get('node'), Node::class);
48+
4349
// Either node can tell the panel that the transfer has failed. Only the new node
4450
// can tell the panel that it was successful.
45-
if (! $server->node->is($transfer->newNode) && ! $server->node->is($transfer->oldNode)) {
51+
if (! $node->is($transfer->newNode) && ! $node->is($transfer->oldNode)) {
4652
throw new HttpForbiddenException('Requesting node does not have permission to access this server.');
4753
}
4854

@@ -54,17 +60,20 @@ public function failure(string $uuid): JsonResponse
5460
*
5561
* @throws \Throwable
5662
*/
57-
public function success(string $uuid): JsonResponse
63+
public function success(Request $request, string $uuid): JsonResponse
5864
{
5965
$server = $this->repository->getByUuid($uuid);
6066
$transfer = $server->transfer;
6167
if (is_null($transfer)) {
6268
throw new ConflictHttpException('Server is not being transferred.');
6369
}
6470

71+
/* @var Node $node */
72+
Assert::isInstanceOf($node = $request->attributes->get('node'), Node::class);
73+
6574
// Only the new node communicates a successful state to the panel, so we should
6675
// not allow the old node to hit this endpoint.
67-
if (! $server->node->is($transfer->newNode)) {
76+
if (! $node->is($transfer->newNode)) {
6877
throw new HttpForbiddenException('Requesting node does not have permission to access this server.');
6978
}
7079

‎app/Models/ServerTransfer.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use Illuminate\Database\Eloquent\Relations\HasOne;
66
use Illuminate\Database\Eloquent\Relations\BelongsTo;
7+
use Illuminate\Database\Eloquent\Factories\HasFactory;
78

89
/**
910
* @property int $id
@@ -24,6 +25,9 @@
2425
*/
2526
class ServerTransfer extends Model
2627
{
28+
/** @use HasFactory<\Database\Factories\ServerTransferFactory> */
29+
use HasFactory;
30+
2731
/**
2832
* The resource name for this model when it is transformed into an
2933
* API representation using fractal.

‎database/Factories/ServerTransferFactory.php‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php
2+
3+
namespace Database\Factories;
4+
5+
use Pterodactyl\Models\ServerTransfer;
6+
use Illuminate\Database\Eloquent\Factories\Factory;
7+
8+
class ServerTransferFactory extends Factory
9+
{
10+
/**
11+
* The name of the factory's corresponding model.
12+
*
13+
* @var string
14+
*/
15+
protected $model = ServerTransfer::class;
16+
17+
/**
18+
* Define the model's default state.
19+
*
20+
* @return array
21+
*/
22+
public function definition()
23+
{
24+
return [
25+
'old_additional_allocations' => [],
26+
'new_additional_allocations' => [],
27+
'successful' => null,
28+
'archived' => false,
29+
];
30+
}
31+
}

‎tests/Integration/Api/Remote/ServerTransferControllerTest.php‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
<?php
2+
3+
namespace Pterodactyl\Tests\Integration\Api\Remote;
4+
5+
use Pterodactyl\Models\Node;
6+
use Pterodactyl\Models\Location;
7+
use Pterodactyl\Models\Allocation;
8+
use Pterodactyl\Models\ServerTransfer;
9+
use Pterodactyl\Tests\Integration\IntegrationTestCase;
10+
11+
class ServerTransferControllerTest extends IntegrationTestCase
12+
{
13+
protected ServerTransfer $transfer;
14+
15+
public function setup(): void
16+
{
17+
parent::setUp();
18+
19+
$server = $this->createServerModel();
20+
21+
$new = Node::factory()
22+
->for(Location::factory())
23+
->has(Allocation::factory())
24+
->create();
25+
26+
$this->transfer = ServerTransfer::factory()->for($server)->create([
27+
'old_allocation' => $server->allocation_id,
28+
'new_allocation' => $new->allocations->first()->id,
29+
'new_node' => $new->id,
30+
'old_node' => $server->node_id,
31+
]);
32+
}
33+
34+
public function testSuccessStatusUpdateCanBeSentFromNewNode(): void
35+
{
36+
$server = $this->transfer->server;
37+
$newNode = $this->transfer->newNode;
38+
39+
$this
40+
->withHeader('Authorization', "Bearer $newNode->daemon_token_id." . $newNode->getDecryptedKey())
41+
->postJson("/api/remote/servers/{$server->uuid}/transfer/success")
42+
->assertNoContent();
43+
44+
$this->assertTrue($this->transfer->refresh()->successful);
45+
}
46+
47+
public function testFailureStatusUpdateCanBeSentFromOldNode(): void
48+
{
49+
$server = $this->transfer->server;
50+
$oldNode = $this->transfer->oldNode;
51+
52+
$this->withHeader('Authorization', "Bearer $oldNode->daemon_token_id." . $oldNode->getDecryptedKey())
53+
->postJson("/api/remote/servers/{$server->uuid}/transfer/failure")
54+
->assertNoContent();
55+
56+
$this->assertFalse($this->transfer->refresh()->successful);
57+
}
58+
59+
public function testFailureStatusUpdateCanBeSentFromNewNode(): void
60+
{
61+
$server = $this->transfer->server;
62+
$newNode = $this->transfer->newNode;
63+
64+
$this->withHeader('Authorization', "Bearer $newNode->daemon_token_id." . $newNode->getDecryptedKey())
65+
->postJson("/api/remote/servers/{$server->uuid}/transfer/failure")
66+
->assertNoContent();
67+
68+
$this->assertFalse($this->transfer->refresh()->successful);
69+
}
70+
71+
public function testSuccessStatusUpdateCannotBeSentFromOldNode(): void
72+
{
73+
$server = $this->transfer->server;
74+
$oldNode = $this->transfer->oldNode;
75+
76+
$this->withHeader('Authorization', "Bearer $oldNode->daemon_token_id." . $oldNode->getDecryptedKey())
77+
->postJson("/api/remote/servers/{$server->uuid}/transfer/success")
78+
->assertForbidden()
79+
->assertJsonPath('errors.0.code', 'HttpForbiddenException')
80+
->assertJsonPath('errors.0.detail', 'Requesting node does not have permission to access this server.');
81+
82+
$this->assertNull($this->transfer->refresh()->successful);
83+
}
84+
85+
public function testSuccessStatusUpdateCannotBeSentFromUnauthorizedNode(): void
86+
{
87+
$server = $this->transfer->server;
88+
$node = Node::factory()->for(Location::factory())->create();
89+
90+
$this->withHeader('Authorization', "Bearer $node->daemon_token_id." . $node->getDecryptedKey())
91+
->postJson("/api/remote/servers/$server->uuid/transfer/success")
92+
->assertForbidden()
93+
->assertJsonPath('errors.0.code', 'HttpForbiddenException')
94+
->assertJsonPath('errors.0.detail', 'Requesting node does not have permission to access this server.');
95+
96+
$this->assertNull($this->transfer->refresh()->successful);
97+
}
98+
99+
public function testFailureStatusUpdateCannotBeSentFromUnauthorizedNode(): void
100+
{
101+
$server = $this->transfer->server;
102+
$node = Node::factory()->for(Location::factory())->create();
103+
104+
$this->withHeader('Authorization', "Bearer $node->daemon_token_id." . $node->getDecryptedKey())
105+
->postJson("/api/remote/servers/$server->uuid/transfer/failure")->assertForbidden()
106+
->assertJsonPath('errors.0.code', 'HttpForbiddenException')
107+
->assertJsonPath('errors.0.detail', 'Requesting node does not have permission to access this server.');
108+
109+
$this->assertNull($this->transfer->refresh()->successful);
110+
}
111+
}

0 commit comments

Comments
 (0)