From ce7c913e18e488df37e81d130314346112e2032d Mon Sep 17 00:00:00 2001 From: Matthew Penner Date: Mon, 31 Oct 2022 13:20:06 -0600 Subject: [PATCH] tests(unit): fix `RequireTwoFactorAuthenticationTest` --- .../RequireTwoFactorAuthentication.php | 4 +- composer.json | 2 +- composer.lock | 4 +- database/Factories/SecurityKeyFactory.php | 12 +-- .../RequireTwoFactorAuthenticationTest.php | 84 +++++++++++-------- 5 files changed, 55 insertions(+), 51 deletions(-) diff --git a/app/Http/Middleware/RequireTwoFactorAuthentication.php b/app/Http/Middleware/RequireTwoFactorAuthentication.php index 2f966c4cc..4e50976a0 100644 --- a/app/Http/Middleware/RequireTwoFactorAuthentication.php +++ b/app/Http/Middleware/RequireTwoFactorAuthentication.php @@ -50,9 +50,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->has2FAEnabled()) { - return $next($request); - } elseif ($level === self::LEVEL_ADMIN && !$user->root_admin) { + if ($level === self::LEVEL_NONE || $user->has2FAEnabled() || ($level === self::LEVEL_ADMIN && !$user->root_admin)) { return $next($request); } diff --git a/composer.json b/composer.json index cadbabb62..d9ed89dad 100644 --- a/composer.json +++ b/composer.json @@ -17,7 +17,7 @@ } ], "require": { - "php": "^8.0.2 || ^8.1 || ^8.2", + "php": "^8.1 || ^8.2", "ext-json": "*", "ext-mbstring": "*", "ext-pdo": "*", diff --git a/composer.lock b/composer.lock index aa7a2ee8d..d6b3daee2 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "1e13394cf95fb955fbf12cb9f71e215d", + "content-hash": "04a0ce5d940be3c570a4705b0d009641", "packages": [ { "name": "aws/aws-crt-php", @@ -12074,7 +12074,7 @@ "prefer-stable": true, "prefer-lowest": false, "platform": { - "php": "^8.0.2 || ^8.1 || ^8.2", + "php": "^8.1 || ^8.2", "ext-json": "*", "ext-mbstring": "*", "ext-pdo": "*", diff --git a/database/Factories/SecurityKeyFactory.php b/database/Factories/SecurityKeyFactory.php index d98f54252..58a69bef6 100644 --- a/database/Factories/SecurityKeyFactory.php +++ b/database/Factories/SecurityKeyFactory.php @@ -24,20 +24,16 @@ class SecurityKeyFactory extends Factory { return [ 'uuid' => Uuid::uuid4()->toString(), + 'user_id' => User::factory(), 'name' => $this->faker->word, 'type' => 'public-key', 'transports' => [], 'attestation_type' => 'none', 'trust_path' => new EmptyTrustPath(), + 'user_handle' => function (array $attributes) { + return User::find($attributes['user_id'])->uuid; + }, 'counter' => 0, ]; } - - public function withUser(User $user): self - { - return $this->state([ - 'user_id' => $user->id, - 'user_handle' => $user->uuid, - ]); - } } diff --git a/tests/Unit/Http/Middleware/RequireTwoFactorAuthenticationTest.php b/tests/Unit/Http/Middleware/RequireTwoFactorAuthenticationTest.php index dcd7db5e9..3b600e6e3 100644 --- a/tests/Unit/Http/Middleware/RequireTwoFactorAuthenticationTest.php +++ b/tests/Unit/Http/Middleware/RequireTwoFactorAuthenticationTest.php @@ -19,6 +19,7 @@ class RequireTwoFactorAuthenticationTest extends MiddlewareTestCase $this->assertFalse($user->use_totp); $this->assertEmpty($user->totp_secret); $this->assertEmpty($user->totp_authenticated_at); + $this->assertTrue($user->securityKeys->isEmpty()); $this->request->shouldReceive('getRequestUri')->withNoArgs()->andReturn('/'); $this->request->shouldReceive('route->getName')->withNoArgs()->andReturn(null); @@ -39,6 +40,7 @@ class RequireTwoFactorAuthenticationTest extends MiddlewareTestCase $this->assertTrue($user->use_totp); $this->assertEmpty($user->totp_secret); $this->assertEmpty($user->totp_authenticated_at); + $this->assertTrue($user->securityKeys->isEmpty()); $this->request->shouldReceive('getRequestUri')->withNoArgs()->andReturn('/'); $this->request->shouldReceive('route->getName')->withNoArgs()->andReturn(null); @@ -49,21 +51,21 @@ class RequireTwoFactorAuthenticationTest extends MiddlewareTestCase $middleware->handle($this->request, $this->getClosureAssertions()); } - public function testNoRequirementUserWithWebauthn2fa() + public function testNoRequirementUserWithSecurityKey2fa() { // Disable the 2FA requirement config()->set('pterodactyl.auth.2fa_required', RequireTwoFactorAuthentication::LEVEL_NONE); /** @var \Pterodactyl\Models\User $user */ $user = User::factory() - ->has(SecurityKey::factory()->count(1)) - ->create(['use_totp' => false]); + ->make(['use_totp' => false]) + ->setRelation('securityKeys', SecurityKey::factory()->count(1)->make()); $this->setRequestUserModel($user); $this->assertFalse($user->use_totp); $this->assertEmpty($user->totp_secret); $this->assertEmpty($user->totp_authenticated_at); - $this->assertNotEmpty($user->securityKeys); + $this->assertTrue($user->securityKeys->isNotEmpty()); $this->request->shouldReceive('getRequestUri')->withNoArgs()->andReturn('/'); $this->request->shouldReceive('route->getName')->withNoArgs()->andReturn(null); @@ -102,7 +104,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->assertTrue($user->securityKeys->isEmpty()); $this->request->shouldReceive('getRequestUri')->withNoArgs()->andReturn('/'); $this->request->shouldReceive('route->getName')->withNoArgs()->andReturn(null); @@ -123,6 +125,7 @@ class RequireTwoFactorAuthenticationTest extends MiddlewareTestCase $this->assertTrue($user->use_totp); $this->assertEmpty($user->totp_secret); $this->assertEmpty($user->totp_authenticated_at); + $this->assertTrue($user->securityKeys->isEmpty()); $this->request->shouldReceive('getRequestUri')->withNoArgs()->andReturn('/'); $this->request->shouldReceive('route->getName')->withNoArgs()->andReturn(null); @@ -133,21 +136,21 @@ class RequireTwoFactorAuthenticationTest extends MiddlewareTestCase $middleware->handle($this->request, $this->getClosureAssertions()); } - public function testAllRequirementRuserWithWebauthn2fa() + public function testAllRequirementUserWithSecurityKey2fa() { // Disable the 2FA requirement config()->set('pterodactyl.auth.2fa_required', RequireTwoFactorAuthentication::LEVEL_ALL); /** @var \Pterodactyl\Models\User $user */ $user = User::factory() - ->has(SecurityKey::factory()->count(1)) - ->create(['use_totp' => false]); + ->make(['use_totp' => false]) + ->setRelation('securityKeys', SecurityKey::factory()->count(1)->make()); $this->setRequestUserModel($user); $this->assertFalse($user->use_totp); $this->assertEmpty($user->totp_secret); $this->assertEmpty($user->totp_authenticated_at); - $this->assertNotEmpty($user->securityKeys); + $this->assertTrue($user->securityKeys->isNotEmpty()); $this->request->shouldReceive('getRequestUri')->withNoArgs()->andReturn('/'); $this->request->shouldReceive('route->getName')->withNoArgs()->andReturn(null); @@ -184,6 +187,7 @@ class RequireTwoFactorAuthenticationTest extends MiddlewareTestCase $this->assertFalse($user->use_totp); $this->assertEmpty($user->totp_secret); $this->assertEmpty($user->totp_authenticated_at); + $this->assertTrue($user->securityKeys->isEmpty()); $this->assertFalse($user->root_admin); $this->request->shouldReceive('getRequestUri')->withNoArgs()->andReturn('/'); @@ -207,6 +211,7 @@ class RequireTwoFactorAuthenticationTest extends MiddlewareTestCase $this->assertFalse($user->use_totp); $this->assertEmpty($user->totp_secret); $this->assertEmpty($user->totp_authenticated_at); + $this->assertTrue($user->securityKeys->isEmpty()); $this->assertTrue($user->root_admin); $this->request->shouldReceive('getRequestUri')->withNoArgs()->andReturn('/'); @@ -228,6 +233,7 @@ class RequireTwoFactorAuthenticationTest extends MiddlewareTestCase $this->assertTrue($user->use_totp); $this->assertEmpty($user->totp_secret); $this->assertEmpty($user->totp_authenticated_at); + $this->assertTrue($user->securityKeys->isEmpty()); $this->assertFalse($user->root_admin); $this->request->shouldReceive('getRequestUri')->withNoArgs()->andReturn('/'); @@ -249,6 +255,7 @@ class RequireTwoFactorAuthenticationTest extends MiddlewareTestCase $this->assertTrue($user->use_totp); $this->assertEmpty($user->totp_secret); $this->assertEmpty($user->totp_authenticated_at); + $this->assertTrue($user->securityKeys->isEmpty()); $this->assertTrue($user->root_admin); $this->request->shouldReceive('getRequestUri')->withNoArgs()->andReturn('/'); @@ -260,45 +267,22 @@ class RequireTwoFactorAuthenticationTest extends MiddlewareTestCase $middleware->handle($this->request, $this->getClosureAssertions()); } - public function testAdminRequirementUserWithWebauthn2fa() - { - // Disable the 2FA requirement - config()->set('pterodactyl.auth.2fa_required', RequireTwoFactorAuthentication::LEVEL_ADMIN); - - /** @var \Pterodactyl\Models\User $user */ - $user = User::factory()->has(SecurityKey::factory()->count(1))->create(['use_totp' => false]); - $this->setRequestUserModel($user); - - $this->assertFalse($user->use_totp); - $this->assertEmpty($user->totp_secret); - $this->assertEmpty($user->totp_authenticated_at); - $this->assertFalse($user->root_admin); - $this->assertNotEmpty($user->securityKeys); - - $this->request->shouldReceive('getRequestUri')->withNoArgs()->andReturn('/'); - $this->request->shouldReceive('route->getName')->withNoArgs()->andReturn(null); - $this->request->shouldReceive('isJson')->withNoArgs()->andReturn(true); - - /** @var \Pterodactyl\Http\Middleware\RequireTwoFactorAuthentication $controller */ - $middleware = $this->app->make(RequireTwoFactorAuthentication::class); - $middleware->handle($this->request, $this->getClosureAssertions()); - } - - public function testAdminRequirementAdminUserWithWebauthn2fa() + public function testAdminRequirementUserWithSecurityKey2fa() { // Disable the 2FA requirement config()->set('pterodactyl.auth.2fa_required', RequireTwoFactorAuthentication::LEVEL_ADMIN); /** @var \Pterodactyl\Models\User $user */ $user = User::factory() - ->has(SecurityKey::factory()->count(1)) - ->create(['use_totp' => false, 'root_admin' => true]); + ->make(['use_totp' => false]) + ->setRelation('securityKeys', SecurityKey::factory()->count(1)->make()); $this->setRequestUserModel($user); $this->assertFalse($user->use_totp); $this->assertEmpty($user->totp_secret); $this->assertEmpty($user->totp_authenticated_at); - $this->assertTrue($user->root_admin); + $this->assertFalse($user->root_admin); + $this->assertTrue($user->securityKeys->isNotEmpty()); $this->assertNotEmpty($user->securityKeys); $this->request->shouldReceive('getRequestUri')->withNoArgs()->andReturn('/'); @@ -310,6 +294,32 @@ class RequireTwoFactorAuthenticationTest extends MiddlewareTestCase $middleware->handle($this->request, $this->getClosureAssertions()); } + public function testAdminRequirementAdminUserWithSecurityKey2fa() + { + // Disable the 2FA requirement + config()->set('pterodactyl.auth.2fa_required', RequireTwoFactorAuthentication::LEVEL_ADMIN); + + /** @var \Pterodactyl\Models\User $user */ + $user = User::factory() + ->make(['use_totp' => false, 'root_admin' => true]) + ->setRelation('securityKeys', SecurityKey::factory()->count(1)->make()); + $this->setRequestUserModel($user); + + $this->assertFalse($user->use_totp); + $this->assertEmpty($user->totp_secret); + $this->assertEmpty($user->totp_authenticated_at); + $this->assertTrue($user->securityKeys->isNotEmpty()); + $this->assertTrue($user->root_admin); + + $this->request->shouldReceive('getRequestUri')->withNoArgs()->andReturn('/'); + $this->request->shouldReceive('route->getName')->withNoArgs()->andReturn(null); + $this->request->shouldReceive('isJson')->withNoArgs()->andReturn(true); + + /** @var \Pterodactyl\Http\Middleware\RequireTwoFactorAuthentication $controller */ + $middleware = $this->app->make(RequireTwoFactorAuthentication::class); + $middleware->handle($this->request, $this->getClosureAssertions()); + } + public function testAdminRequirementGuestUser() { // Disable the 2FA requirement