From 655416425246639f8c1ca4b7d601b2d90653d5d0 Mon Sep 17 00:00:00 2001 From: DaneEveritt Date: Sat, 14 May 2022 18:08:48 -0400 Subject: [PATCH] Add test coverage for the SSH key endpoints --- .../Api/Client/SSHKeyController.php | 6 +- .../Api/Client/Account/StoreSSHKeyRequest.php | 4 +- app/Models/UserSSHKey.php | 2 +- ...nsformer.php => UserSSHKeyTransformer.php} | 2 +- database/Factories/UserSSHKeyFactory.php | 52 +++++++ .../Client/ClientApiIntegrationTestCase.php | 4 + .../Api/Client/SSHKeyControllerTest.php | 138 ++++++++++++++++++ 7 files changed, 201 insertions(+), 7 deletions(-) rename app/Transformers/Api/Client/{SSHKeyTransformer.php => UserSSHKeyTransformer.php} (90%) create mode 100644 database/Factories/UserSSHKeyFactory.php create mode 100644 tests/Integration/Api/Client/SSHKeyControllerTest.php diff --git a/app/Http/Controllers/Api/Client/SSHKeyController.php b/app/Http/Controllers/Api/Client/SSHKeyController.php index 5b25a221c..1b3db52d3 100644 --- a/app/Http/Controllers/Api/Client/SSHKeyController.php +++ b/app/Http/Controllers/Api/Client/SSHKeyController.php @@ -4,7 +4,7 @@ namespace Pterodactyl\Http\Controllers\Api\Client; use Illuminate\Http\JsonResponse; use Pterodactyl\Http\Requests\Api\Client\ClientApiRequest; -use Pterodactyl\Transformers\Api\Client\SSHKeyTransformer; +use Pterodactyl\Transformers\Api\Client\UserSSHKeyTransformer; use Pterodactyl\Http\Requests\Api\Client\Account\StoreSSHKeyRequest; class SSHKeyController extends ClientApiController @@ -16,7 +16,7 @@ class SSHKeyController extends ClientApiController public function index(ClientApiRequest $request): array { return $this->fractal->collection($request->user()->sshKeys) - ->transformWith($this->getTransformer(SSHKeyTransformer::class)) + ->transformWith($this->getTransformer(UserSSHKeyTransformer::class)) ->toArray(); } @@ -32,7 +32,7 @@ class SSHKeyController extends ClientApiController ]); return $this->fractal->item($model) - ->transformWith($this->getTransformer(SSHKeyTransformer::class)) + ->transformWith($this->getTransformer(UserSSHKeyTransformer::class)) ->toArray(); } diff --git a/app/Http/Requests/Api/Client/Account/StoreSSHKeyRequest.php b/app/Http/Requests/Api/Client/Account/StoreSSHKeyRequest.php index 5705056c4..29bf2d1ba 100644 --- a/app/Http/Requests/Api/Client/Account/StoreSSHKeyRequest.php +++ b/app/Http/Requests/Api/Client/Account/StoreSSHKeyRequest.php @@ -43,11 +43,11 @@ class StoreSSHKeyRequest extends ClientApiRequest } if ($this->key instanceof DSA) { - $this->validator->errors()->add('public_key', 'DSA public keys are not supported.'); + $this->validator->errors()->add('public_key', 'DSA keys are not supported.'); } if ($this->key instanceof RSA && $this->key->getLength() < 2048) { - $this->validator->errors()->add('public_key', 'RSA keys must be at 2048 bytes.'); + $this->validator->errors()->add('public_key', 'RSA keys must be at least 2048 bytes in length.'); } $fingerprint = $this->key->getFingerprint('sha256'); diff --git a/app/Models/UserSSHKey.php b/app/Models/UserSSHKey.php index 848ba32c8..718c6d1cf 100644 --- a/app/Models/UserSSHKey.php +++ b/app/Models/UserSSHKey.php @@ -17,7 +17,6 @@ use Illuminate\Database\Eloquent\Relations\BelongsTo; * @property \Illuminate\Support\Carbon|null $updated_at * @property \Illuminate\Support\Carbon|null $deleted_at * @property \Pterodactyl\Models\User $user - * * @method static \Illuminate\Database\Eloquent\Builder|UserSSHKey newModelQuery() * @method static \Illuminate\Database\Eloquent\Builder|UserSSHKey newQuery() * @method static \Illuminate\Database\Query\Builder|UserSSHKey onlyTrashed() @@ -33,6 +32,7 @@ use Illuminate\Database\Eloquent\Relations\BelongsTo; * @method static \Illuminate\Database\Query\Builder|UserSSHKey withTrashed() * @method static \Illuminate\Database\Query\Builder|UserSSHKey withoutTrashed() * @mixin \Eloquent + * @method static \Database\Factories\UserSSHKeyFactory factory(...$parameters) */ class UserSSHKey extends Model { diff --git a/app/Transformers/Api/Client/SSHKeyTransformer.php b/app/Transformers/Api/Client/UserSSHKeyTransformer.php similarity index 90% rename from app/Transformers/Api/Client/SSHKeyTransformer.php rename to app/Transformers/Api/Client/UserSSHKeyTransformer.php index 1f9b96c29..1a9349e67 100644 --- a/app/Transformers/Api/Client/SSHKeyTransformer.php +++ b/app/Transformers/Api/Client/UserSSHKeyTransformer.php @@ -4,7 +4,7 @@ namespace Pterodactyl\Transformers\Api\Client; use Pterodactyl\Models\UserSSHKey; -class SSHKeyTransformer extends BaseClientTransformer +class UserSSHKeyTransformer extends BaseClientTransformer { public function getResourceName(): string { diff --git a/database/Factories/UserSSHKeyFactory.php b/database/Factories/UserSSHKeyFactory.php new file mode 100644 index 000000000..5a1a17dcf --- /dev/null +++ b/database/Factories/UserSSHKeyFactory.php @@ -0,0 +1,52 @@ + $this->faker->name(), + 'public_key' => 'ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIOaXIq09NH4a93EVdrvHYiZ67Wj+GBEBQ9ou4W0qSYm2', + 'fingerprint' => 'T2b3VnvHWUmfhDOFLqzZg2VfLgqJhWxzrMUy8WZ+V8M', + ]; + } + + /** + * Returns a DSA public key. + */ + public function dsa(): self + { + return $this->state([ + 'public_key' => 'ssh-dss AAAAB3NzaC1kc3MAAACBAPfiWwEFvBOafdUmHDPjXsUttt+65FHSZSCVVeEFOTaL7Y3d0CJyrtck8KS1vmXHSb8QFBY2B1yVSb/reaQvNreWZN3KDYfLbF57/zimBn+IrHrJR+ZglhOxDRHoGPWK7q9jYIrOLwoOjkNKXxz1eOHKUgufFfSNtIRLycEXczLrAAAAFQC6LnBErezotG52jN4JostfC/TfEwAAAIACuTxRzYFDXHAxFICeqqY9w+y+v2yQfdeQ1BgCq2GMagUYfOdqnjizTO9M614r/nXZK1SV10TqhUcQtkJzDQIUtBqzBF5cIC/1cIFKzXi5rNHs8Y4bz/PBD+EbQJdiy+1so1oi790r710bqnkzTravAOJ5rGyfuQRLt+f+kuS9NAAAAIEA7tjGtJuXGUtPIXfnrMYS1iOWryO4irqnvaWfel002/DaGaNjRghNe/cUBYlAsjPhGJ1F7BQlLAY1koliTY6l0svs7ZPBM5QOumrr8OaNXGGVIq/RkkxuZHmRoUL2qH3DGYaktPUn4vFPliiAmGWOHAEu1K6B4g4vG/SKgMRpIvc=', + 'fingerprint' => '9Fb/RODt9N6aldcB+lc6ih0ovr2G/JUjts2Wh21uxYI', + ]); + } + + /** + * Returns an RSA public key, if "weak" is specified a 1024-bit RSA key is returned + * which should fail validation when being stored. + */ + public function rsa(bool $weak = false): self + { + if (!$weak) { + return $this->state([ + 'public_key' => 'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQCo/YLm2SPSlOIG7AagBSmEe5c0b2PLPzUGFp3gARhD6n6ydBS40TlWzeg2qV95lh6fWBd8LsNgPOFmmuKuNZdBjAGeTY4gxKfHY1vK5/zOI4jPPqAMcCMNfd82aM97kx6dO8Hw1R79OyVpOZylpXLHayVPGHUK37Tpih4W7TeVSMrOqQF9F72lzhwgEtkdjm4gLBL6RpdNXrdnjIaNVnuade0Sb3w384vecZPe+S/997WirOMNy2JU4NdMHEnSjd1/i463RpN96AsXFAu1zl9nrXVhA7DVfSHoigXAqbs/xav8PRpLgAKjYpPohxQ9Nu6tP5jRUhfWdYwNFFp/aWloD/0JdP9LqcBBc9sO9TLkz3fBiUf11VM/QT1UhO84G+ahMxVn95jA472VPUe8uKff69lzbvSavEE6qcQX2TzVKOSi1E26Fzc6IZ/tHEuGEbGFxTsiQ1GysVZ0wr1p6ftd1SVqH5F/oaEK7UO8+xn/syEqaPf6A0eJWRNc0+lHA1sIRjmo9MOBvbkKExkx5JLHgGG81DYDFdZUuHY1BgSxJJcmNWV5BKRm350EbgRngoYI5tB3tCiZVW1PI8qyff9mBae11LY5GPlUeDnPrMvSdCKMIWrg7nC8SbndBCO3Fx4z7G2dTQy4ZmY7Ae9jR4pyg7tTOI3qgl8Z462GZi/jzw==', + 'fingerprint' => 'vjccQdGfqAvuEK3Bki1Ji6aOo3rIuGU0BGJ0ml4CjLQ', + ]); + } + + return $this->state([ + 'public_key' => 'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQC4VVsHFO5MxvCtAPyoKGANWyuwZ4fvllvFog5RJbfpDpw8etDFVGEXl+uRR8p79g9oV7MscoFo6HiWrJc4/4vlP665msjosILdIcbnuzMhvXnKisaGh9zflkpyR3KhUxoHxqYp2q8XtffjKKAHz1a8o7OUG6fwaKIqu+d0PoICZQ==', + 'fingerprint' => 'LQSzAAfAsbKpU94gojxXfjGjYcEv8UZIwyzwhcEr/aw', + ]); + } +} diff --git a/tests/Integration/Api/Client/ClientApiIntegrationTestCase.php b/tests/Integration/Api/Client/ClientApiIntegrationTestCase.php index 33f6069a5..35e8d2719 100644 --- a/tests/Integration/Api/Client/ClientApiIntegrationTestCase.php +++ b/tests/Integration/Api/Client/ClientApiIntegrationTestCase.php @@ -15,6 +15,7 @@ use Pterodactyl\Models\Location; use Pterodactyl\Models\Schedule; use Illuminate\Support\Collection; use Pterodactyl\Models\Allocation; +use Pterodactyl\Models\UserSSHKey; use Pterodactyl\Models\DatabaseHost; use Pterodactyl\Tests\Integration\TestResponse; use Pterodactyl\Tests\Integration\IntegrationTestCase; @@ -77,6 +78,9 @@ abstract class ClientApiIntegrationTestCase extends IntegrationTestCase case Backup::class: $link = "/api/client/servers/{$model->server->uuid}/backups/{$model->uuid}"; break; + case UserSSHKey::class: + $link = "/api/client/account/ssh-keys/$model->fingerprint"; + break; default: throw new InvalidArgumentException(sprintf('Cannot create link for Model of type %s', class_basename($model))); } diff --git a/tests/Integration/Api/Client/SSHKeyControllerTest.php b/tests/Integration/Api/Client/SSHKeyControllerTest.php new file mode 100644 index 000000000..6744a1295 --- /dev/null +++ b/tests/Integration/Api/Client/SSHKeyControllerTest.php @@ -0,0 +1,138 @@ +create(); + $user2 = User::factory()->create(); + + $key = UserSSHKey::factory()->for($user)->create(); + UserSSHKey::factory()->for($user2)->rsa()->create(); + + $this->actingAs($user); + $response = $this->getJson('/api/client/account/ssh-keys') + ->assertOk() + ->assertJsonPath('object', 'list') + ->assertJsonPath('data.0.object', UserSSHKey::RESOURCE_NAME); + + $this->assertJsonTransformedWith($response->json('data.0.attributes'), $key); + } + + /** + * Test that a user's SSH key can be deleted, and that passing the fingerprint + * of another user's SSH key won't delete that key. + */ + public function testSSHKeyCanBeDeleted() + { + $user = User::factory()->create(); + $user2 = User::factory()->create(); + + $key = UserSSHKey::factory()->for($user)->create(); + $key2 = UserSSHKey::factory()->for($user2)->create(); + + $this->actingAs($user); + $this->deleteJson($this->link($key))->assertNoContent(); + + $this->assertSoftDeleted($key); + $this->assertNotSoftDeleted($key2); + + $this->deleteJson($this->link($key))->assertNoContent(); + $this->deleteJson($this->link($key2))->assertNoContent(); + + $this->assertNotSoftDeleted($key2); + } + + public function testDSAKeyIsRejected() + { + $user = User::factory()->create(); + $key = UserSSHKey::factory()->dsa()->make(); + + $this->actingAs($user)->postJson('/api/client/account/ssh-keys', [ + 'name' => 'Name', + 'public_key' => $key->public_key, + ]) + ->assertUnprocessable() + ->assertJsonPath('errors.0.detail', 'DSA keys are not supported.'); + + $this->assertEquals(0, $user->sshKeys()->count()); + } + + public function testWeakRSAKeyIsRejected() + { + $user = User::factory()->create(); + $key = UserSSHKey::factory()->rsa(true)->make(); + + $this->actingAs($user)->postJson('/api/client/account/ssh-keys', [ + 'name' => 'Name', + 'public_key' => $key->public_key, + ]) + ->assertUnprocessable() + ->assertJsonPath('errors.0.detail', 'RSA keys must be at least 2048 bytes in length.'); + + $this->assertEquals(0, $user->sshKeys()->count()); + } + + public function testInvalidOrPrivateKeyIsRejected() + { + $user = User::factory()->create(); + + $this->actingAs($user)->postJson('/api/client/account/ssh-keys', [ + 'name' => 'Name', + 'public_key' => 'invalid', + ]) + ->assertUnprocessable() + ->assertJsonPath('errors.0.detail', 'The public key provided is not valid.'); + + $this->assertEquals(0, $user->sshKeys()->count()); + + $key = EC::createKey('Ed25519'); + $this->actingAs($user)->postJson('/api/client/account/ssh-keys', [ + 'name' => 'Name', + 'public_key' => $key->toString('PKCS8'), + ]) + ->assertUnprocessable() + ->assertJsonPath('errors.0.detail', 'The public key provided is not valid.'); + } + + public function testPublicKeyCanBeStored() + { + $user = User::factory()->create(); + $key = UserSSHKey::factory()->make(); + + $this->actingAs($user)->postJson('/api/client/account/ssh-keys', [ + 'name' => 'Name', + 'public_key' => $key->public_key, + ]) + ->assertOk() + ->assertJsonPath('object', UserSSHKey::RESOURCE_NAME) + ->assertJsonPath('attributes.public_key', $key->public_key); + + $this->assertCount(1, $user->sshKeys); + $this->assertEquals($key->public_key, $user->sshKeys[0]->public_key); + } + + public function testPublicKeyThatAlreadyExistsCannotBeAddedASecondTime() + { + $user = User::factory()->create(); + $key = UserSSHKey::factory()->for($user)->create(); + + $this->actingAs($user)->postJson('/api/client/account/ssh-keys', [ + 'name' => 'Name', + 'public_key' => $key->public_key, + ]) + ->assertUnprocessable() + ->assertJsonPath('errors.0.detail', 'The public key provided already exists on your account.'); + + $this->assertEquals(1, $user->sshKeys()->count()); + } +}