From e683c0a518a34de231162991e53d705976df91b4 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sun, 13 Feb 2022 18:32:02 -0500 Subject: [PATCH] Fix failing tests related to these changes --- app/Http/Controllers/Auth/LoginController.php | 6 +-- .../RequireTwoFactorAuthentication.php | 2 +- database/Factories/SecurityKeyFactory.php | 39 ++++++++++++++++ ..._02_13_181550_drop_webauthn_keys_table.php | 46 +++++++++++++++++++ .../RequireTwoFactorAuthenticationTest.php | 1 + 5 files changed, 90 insertions(+), 4 deletions(-) create mode 100644 database/Factories/SecurityKeyFactory.php create mode 100644 database/migrations/2022_02_13_181550_drop_webauthn_keys_table.php diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index 54c2a44b6..446623916 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -74,7 +74,7 @@ class LoginController extends AbstractLoginController return; } - if (!$user->use_totp && empty($user->securityKeys)) { + if (!$user->use_totp && $user->securityKeys->isEmpty()) { return $this->sendLoginResponse($user, $request); } @@ -89,12 +89,12 @@ class LoginController extends AbstractLoginController 'complete' => false, 'methods' => array_values(array_filter([ $user->use_totp ? self::METHOD_TOTP : null, - !empty($user->securityKeys) ? self::METHOD_WEBAUTHN : null, + $user->securityKeys->isNotEmpty() ? self::METHOD_WEBAUTHN : null, ])), 'confirmation_token' => $token, ]; - if (!empty($user->securityKeys)) { + if ($user->securityKeys->isNotEmpty()) { $key = $this->service->handle($user); $request->session()->put(SecurityKey::PK_SESSION_NAME, $key); diff --git a/app/Http/Middleware/RequireTwoFactorAuthentication.php b/app/Http/Middleware/RequireTwoFactorAuthentication.php index 83fd0a9ce..cdf452322 100644 --- a/app/Http/Middleware/RequireTwoFactorAuthentication.php +++ b/app/Http/Middleware/RequireTwoFactorAuthentication.php @@ -55,7 +55,7 @@ class RequireTwoFactorAuthentication // send them right through, nothing else needs to be checked. // // If the level is set as admin and the user is not an admin, pass them through as well. - if ($level === self::LEVEL_NONE || ($user->use_totp || !empty($user->securityKeys))) { + if ($level === self::LEVEL_NONE || ($user->use_totp || $user->securityKeys->isNotEmpty())) { return $next($request); } elseif ($level === self::LEVEL_ADMIN && !$user->root_admin) { return $next($request); diff --git a/database/Factories/SecurityKeyFactory.php b/database/Factories/SecurityKeyFactory.php new file mode 100644 index 000000000..730466c9c --- /dev/null +++ b/database/Factories/SecurityKeyFactory.php @@ -0,0 +1,39 @@ + Uuid::uuid4()->toString(), + 'name' => $this->faker->word, + 'type' => 'public-key', + 'transports' => [], + 'attestation_type' => 'none', + 'trust_path' => new EmptyTrustPath(), + 'counter' => 0, + ]; + } + + /** + * @param \Pterodactyl\Models\User $user + * @return $this + */ + public function withUser(User $user): self + { + return $this->state([ + 'user_id' => $user->id, + 'user_handle' => $user->uuid, + ]); + } +} diff --git a/database/migrations/2022_02_13_181550_drop_webauthn_keys_table.php b/database/migrations/2022_02_13_181550_drop_webauthn_keys_table.php new file mode 100644 index 000000000..0b28e4299 --- /dev/null +++ b/database/migrations/2022_02_13_181550_drop_webauthn_keys_table.php @@ -0,0 +1,46 @@ +increments('id'); + $table->unsignedInteger('user_id'); + + $table->string('name')->default('key'); + $table->string('credentialId', 255); + $table->string('type', 255); + $table->text('transports'); + $table->string('attestationType', 255); + $table->text('trustPath'); + $table->text('aaguid'); + $table->text('credentialPublicKey'); + $table->integer('counter'); + $table->timestamps(); + + $table->foreign('user_id')->references('id')->on('users')->onDelete('cascade'); + $table->index('credentialId'); + }); + } +} diff --git a/tests/Unit/Http/Middleware/RequireTwoFactorAuthenticationTest.php b/tests/Unit/Http/Middleware/RequireTwoFactorAuthenticationTest.php index f77969831..4c242da7e 100644 --- a/tests/Unit/Http/Middleware/RequireTwoFactorAuthenticationTest.php +++ b/tests/Unit/Http/Middleware/RequireTwoFactorAuthenticationTest.php @@ -108,6 +108,7 @@ class RequireTwoFactorAuthenticationTest extends MiddlewareTestCase $this->assertFalse($user->use_totp); $this->assertEmpty($user->totp_secret); $this->assertEmpty($user->totp_authenticated_at); + $this->assertEmpty($user->securityKeys); $this->request->shouldReceive('getRequestUri')->withNoArgs()->andReturn('/'); $this->request->shouldReceive('route->getName')->withNoArgs()->andReturn(null);