Fix up creation of keys to fail when registering the same key again

app/Http/Controllers/Api/Client/SecurityKeyController.php

@@ -2,7 +2,6 @@
 
 namespace Pterodactyl\Http\Controllers\Api\Client;
 
-use Exception;
 use Carbon\CarbonImmutable;
 use Illuminate\Support\Str;
 use Illuminate\Http\Request;
@@ -53,7 +52,7 @@ class SecurityKeyController extends ClientApiController
     public function create(Request $request): JsonResponse
     {
         $tokenId = Str::random(64);
-        $credentials = $this->createPublicKeyCredentials->handle($request->user(), $request->get('display_name'));
+        $credentials = $this->createPublicKeyCredentials->handle($request->user());
 
         $this->cache->put(
             "register-security-key:$tokenId",
@@ -72,30 +71,30 @@ class SecurityKeyController extends ClientApiController
     /**
      * Stores a new key for a user account.
      *
-     * @throws \Exception
      * @throws \Pterodactyl\Exceptions\DisplayException
+     * @throws \Throwable
      */
     public function store(RegisterWebauthnTokenRequest $request): JsonResponse
     {
-        $stored = $this->cache->pull("register-security-key:{$request->input('token_id')}");
+        $credentials = unserialize(
-
+            $this->cache->pull("register-security-key:{$request->input('token_id')}", serialize(null))
-        if (!$stored) {
-            throw new DisplayException('Could not register security key: no data present in session, please try your request again.');
-        }
-
-        $credentials = unserialize($stored);
-        if (!$credentials instanceof PublicKeyCredentialCreationOptions) {
-            throw new Exception(sprintf('Unexpected security key data pulled from cache: expected "%s" but got "%s".', PublicKeyCredentialCreationOptions::class, get_class($credentials)));
-        }
-
-        $server = $this->webauthnServerRepository->getServer($request->user());
-
-        $source = $server->loadAndCheckAttestationResponse(
-            json_encode($request->input('registration')),
-            $credentials,
-            $this->getServerRequest($request),
         );
 
+        if (
+            !is_object($credentials) ||
+            !$credentials instanceof PublicKeyCredentialCreationOptions ||
+            $credentials->getUser()->getId() !== $request->user()->uuid
+        ) {
+            throw new DisplayException('Could not register security key: invalid data present in session, please try again.');
+        }
+
+        $source = $this->webauthnServerRepository->getServer($request->user())
+            ->loadAndCheckAttestationResponse(
+                json_encode($request->input('registration')),
+                $credentials,
+                $this->getServerRequest($request),
+            );
+
         // Unfortunately this repository interface doesn't define a response — it is explicitly
         // void — so we need to just query the database immediately after this to pull the information
         // we just stored to return to the caller.
@@ -108,7 +107,7 @@ class SecurityKeyController extends ClientApiController
         $created->update(['name' => $request->input('name')]);
 
         return new JsonResponse([
-            'data' => $created->toArray(),
+            'data' => [],
         ]);
     }
 

app/Models/SecurityKey.php

@@ -2,8 +2,14 @@
 
 namespace Pterodactyl\Models;
 
+use Stringable;
+use Ramsey\Uuid\Uuid;
+use Ramsey\Uuid\UuidInterface;
+use Webauthn\TrustPath\TrustPath;
 use Webauthn\PublicKeyCredentialSource;
+use Webauthn\TrustPath\TrustPathLoader;
 use Webauthn\PublicKeyCredentialDescriptor;
+use Illuminate\Database\Eloquent\Relations\BelongsTo;
 use Illuminate\Database\Eloquent\Factories\HasFactory;
 
 class SecurityKey extends Model
@@ -15,7 +21,6 @@ class SecurityKey extends Model
     protected $casts = [
         'user_id' => 'int',
         'transports' => 'array',
-        'trust_path' => 'array',
         'other_ui' => 'array',
     ];
 
@@ -24,12 +29,65 @@ class SecurityKey extends Model
         'user_id',
     ];
 
-    public function user()
+    public function getPublicKeyAttribute(string $value): string
+    {
+        return base64_decode($value);
+    }
+
+    public function setPublicKeyAttribute(string $value): void
+    {
+        $this->attributes['public_key'] = base64_encode($value);
+    }
+
+    public function getPublicKeyIdAttribute(string $value): string
+    {
+        return base64_decode($value);
+    }
+
+    public function setPublicKeyIdAttribute(string $value): void
+    {
+        $this->attributes['public_key_id'] = base64_encode($value);
+    }
+
+    public function getTrustPathAttribute(?string $value): ?TrustPath
+    {
+        if (is_null($value)) {
+            return null;
+        }
+
+        return TrustPathLoader::loadTrustPath(json_decode($value, true));
+    }
+
+    public function setTrustPathAttribute(?TrustPath $value): void
+    {
+        $this->attributes['trust_path'] = json_encode($value);
+    }
+
+    /**
+     * @param \Ramsey\Uuid\UuidInterface|string|null $value
+     */
+    public function setAaguidAttribute($value): void
+    {
+        $value = $value instanceof UuidInterface ? $value->__toString() : $value;
+
+        $this->attributes['aaguid'] = (is_null($value) || $value === Uuid::NIL) ? null : $value;
+    }
+
+    public function getAaguidAttribute(?string $value): ?UuidInterface
+    {
+        if (!is_null($value) && Uuid::isValid($value)) {
+            return Uuid::fromString($value);
+        }
+
+        return null;
+    }
+
+    public function user(): BelongsTo
     {
         return $this->belongsTo(User::class);
     }
 
-    public function toCredentialsDescriptor()
+    public function getPublicKeyCredentialsDescriptorAttribute(): PublicKeyCredentialDescriptor
     {
         return new PublicKeyCredentialDescriptor(
             $this->type,
@@ -38,19 +96,18 @@ class SecurityKey extends Model
         );
     }
 
-    public function toCredentialSource(): PublicKeyCredentialSource
+    public function getPublicKeyCredentialSourceAttribute(): PublicKeyCredentialSource
     {
-        return PublicKeyCredentialSource::createFromArray([
+        return new PublicKeyCredentialSource(
-            'publicKeyCredentialId' => $this->public_key_id,
+            $this->public_key_id,
-            'type' => $this->type,
+            $this->type,
-            'transports' => $this->transports,
+            $this->transports,
-            'attestationType' => $this->attestation_type,
+            $this->attestation_type,
-            // 'trustPath' => $key->trustPath->jsonSerialize(),
+            $this->trust_path,
-            'aaguid' => $this->aaguid,
+            $this->aaguid ?? Uuid::fromString(Uuid::NIL),
-            'credentialPublicKey' => $this->public_key,
+            $this->public_key,
-            'userHandle' => $this->user_handle,
+            (string) $this->user_id,
-            'counter' => $this->counter,
+            $this->counter
-            'otherUI' => $this->other_ui,
+        );
-        ]);
     }
 }

app/Repositories/SecurityKeys/PublicKeyCredentialSourceRepository.php

@@ -29,7 +29,7 @@ class PublicKeyCredentialSourceRepository implements PublicKeyRepositoryInterfac
             ->where('public_key_id', $id)
             ->first();
 
-        return $key ? $key->toCredentialSource() : null;
+        return $key ? $key->public_key_credential_source : null;
     }
 
     /**
@@ -43,7 +43,7 @@ class PublicKeyCredentialSourceRepository implements PublicKeyRepositoryInterfac
             ->get();
 
         return $results->map(function (SecurityKey $key) {
-            return $key->toCredentialSource();
+            return $key->public_key_credential_source;
         })->values()->toArray();
     }
 
@@ -54,20 +54,24 @@ class PublicKeyCredentialSourceRepository implements PublicKeyRepositoryInterfac
      */
     public function saveCredentialSource(PublicKeyCredentialSource $source): void
     {
-        $this->user->securityKeys()->forceCreate([
+        $key = $this->user->securityKeys()->make();
-            'uuid' => Uuid::uuid4()->toString(),
+
+        $key->forceFill([
+            'uuid' => Uuid::uuid4(),
             'user_id' => $this->user->id,
-            'public_key_id' => base64_encode($source->getPublicKeyCredentialId()),
+            'public_key_id' => $source->getPublicKeyCredentialId(),
-            'public_key' => base64_encode($source->getCredentialPublicKey()),
+            'public_key' => $source->getCredentialPublicKey(),
-            'aaguid' => $source->getAaguid()->toString(),
+            'aaguid' => $source->getAaguid(),
             'type' => $source->getType(),
             'transports' => $source->getTransports(),
             'attestation_type' => $source->getAttestationType(),
-            'trust_path' => $source->getTrustPath()->jsonSerialize(),
+            'trust_path' => $source->getTrustPath(),
             'user_handle' => $source->getUserHandle(),
             'counter' => $source->getCounter(),
             'other_ui' => $source->getOtherUI(),
         ]);
+
+        $key->saveOrFail();
     }
 
     /**

app/Services/Users/SecurityKeys/CreatePublicKeyCredentialsService.php

@@ -2,7 +2,6 @@
 
 namespace Pterodactyl\Services\Users\SecurityKeys;
 
-use Ramsey\Uuid\Uuid;
 use Pterodactyl\Models\User;
 use Pterodactyl\Models\SecurityKey;
 use Webauthn\PublicKeyCredentialUserEntity;
@@ -18,14 +17,12 @@ class CreatePublicKeyCredentialsService
         $this->webauthnServerRepository = $webauthnServerRepository;
     }
 
-    public function handle(User $user, ?string $displayName): PublicKeyCredentialCreationOptions
+    public function handle(User $user): PublicKeyCredentialCreationOptions
     {
-        $id = Uuid::uuid4()->toString();
+        $entity = new PublicKeyCredentialUserEntity($user->username, $user->uuid, $user->email, null);
-
-        $entity = new PublicKeyCredentialUserEntity($user->uuid, $id, $name ?? $user->email);
 
         $excluded = $user->securityKeys->map(function (SecurityKey $key) {
-            return $key->toCredentialsDescriptor();
+            return $key->public_key_credentials_descriptor;
         })->values()->toArray();
 
         $server = $this->webauthnServerRepository->getServer($user);

database/migrations/2021_08_07_170141_create_security_keys_table.php

@@ -20,7 +20,7 @@ class CreateSecurityKeysTable extends Migration
             $table->string('name');
             $table->text('public_key_id');
             $table->text('public_key');
-            $table->char('aaguid', 36);
+            $table->char('aaguid', 36)->nullable();
             $table->string('type');
             $table->json('transports');
             $table->string('attestation_type');

resources/scripts/api/account/webauthn/registerWebauthnKey.ts

@@ -46,11 +46,7 @@ const registerCredentialForAccount = async (name: string, tokenId: string, crede
 };
 
 export const register = async (name: string): Promise<void> => {
-    const { data } = await http.get('/api/client/account/security-keys/register', {
+    const { data } = await http.get('/api/client/account/security-keys/register');
-        params: {
-            display_name: name,
-        },
-    });
 
     const publicKey = data.data.credentials;
     publicKey.challenge = bufferDecode(base64Decode(publicKey.challenge));