From 7882250bafd8840ceb95fec8171216b7f04d0247 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Fri, 3 Nov 2017 18:16:49 -0500 Subject: [PATCH] Add more middleware tests --- app/Http/Middleware/Authenticate.php | 29 +-- app/Http/Middleware/DaemonAuthenticate.php | 13 +- app/Http/Middleware/LanguageMiddleware.php | 13 +- .../Middleware/RedirectIfAuthenticated.php | 4 +- .../RequireTwoFactorAuthentication.php | 8 +- database/factories/ModelFactory.php | 1 + tests/Traits/Http/MocksMiddlewareClosure.php | 5 - .../Http/Middleware/AdminAuthenticateTest.php | 62 ++++++ .../Unit/Http/Middleware/AuthenticateTest.php | 57 ++++++ .../Middleware/DaemonAuthenticateTest.php | 77 ++++++++ .../Middleware/LanguageMiddlewareTest.php | 53 +++++ .../RedirectIfAuthenticatedTest.php | 60 ++++++ .../RequireTwoFactorAuthenticationTest.php | 181 ++++++++++++++++++ 13 files changed, 515 insertions(+), 48 deletions(-) create mode 100644 tests/Unit/Http/Middleware/AdminAuthenticateTest.php create mode 100644 tests/Unit/Http/Middleware/AuthenticateTest.php create mode 100644 tests/Unit/Http/Middleware/DaemonAuthenticateTest.php create mode 100644 tests/Unit/Http/Middleware/LanguageMiddlewareTest.php create mode 100644 tests/Unit/Http/Middleware/RedirectIfAuthenticatedTest.php create mode 100644 tests/Unit/Http/Middleware/RequireTwoFactorAuthenticationTest.php diff --git a/app/Http/Middleware/Authenticate.php b/app/Http/Middleware/Authenticate.php index 019f92a2f..473ad5064 100644 --- a/app/Http/Middleware/Authenticate.php +++ b/app/Http/Middleware/Authenticate.php @@ -4,41 +4,26 @@ namespace Pterodactyl\Http\Middleware; use Closure; use Illuminate\Http\Request; -use Illuminate\Contracts\Auth\Guard; +use Illuminate\Auth\AuthenticationException; class Authenticate { - /** - * The Guard implementation. - * - * @var \Illuminate\Contracts\Auth\Guard - */ - protected $auth; - - /** - * Create a new filter instance. - * - * @param \Illuminate\Contracts\Auth\Guard $auth - */ - public function __construct(Guard $auth) - { - $this->auth = $auth; - } - /** * Handle an incoming request. * * @param \Illuminate\Http\Request $request * @param \Closure $next * @return mixed + * + * @throws \Illuminate\Auth\AuthenticationException */ public function handle(Request $request, Closure $next) { - if ($this->auth->guest()) { - if ($request->ajax()) { - return response('Unauthorized.', 401); + if (! $request->user()) { + if ($request->ajax() || $request->expectsJson()) { + throw new AuthenticationException(); } else { - return redirect()->guest('auth/login'); + return redirect()->route('auth.login'); } } diff --git a/app/Http/Middleware/DaemonAuthenticate.php b/app/Http/Middleware/DaemonAuthenticate.php index ca77e70d2..775a7783c 100644 --- a/app/Http/Middleware/DaemonAuthenticate.php +++ b/app/Http/Middleware/DaemonAuthenticate.php @@ -11,10 +11,8 @@ namespace Pterodactyl\Http\Middleware; use Closure; use Illuminate\Http\Request; -use Pterodactyl\Models\Node; -use Symfony\Component\HttpKernel\Exception\HttpException; use Pterodactyl\Contracts\Repository\NodeRepositoryInterface; -use Pterodactyl\Exceptions\Repository\RecordNotFoundException; +use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; class DaemonAuthenticate { @@ -56,15 +54,10 @@ class DaemonAuthenticate } if (! $request->header('X-Access-Node')) { - throw new HttpException(403); - } - - try { - $node = $this->repository->findWhere(['daemonSecret' => $request->header('X-Access-Node')]); - } catch (RecordNotFoundException $exception) { - throw new HttpException(401); + throw new AccessDeniedHttpException; } + $node = $this->repository->findWhere(['daemonSecret' => $request->header('X-Access-Node')]); $request->attributes->set('node', $node); return $next($request); diff --git a/app/Http/Middleware/LanguageMiddleware.php b/app/Http/Middleware/LanguageMiddleware.php index d348f3b8d..2e581f58f 100644 --- a/app/Http/Middleware/LanguageMiddleware.php +++ b/app/Http/Middleware/LanguageMiddleware.php @@ -11,11 +11,16 @@ namespace Pterodactyl\Http\Middleware; use Closure; use Illuminate\Http\Request; -use Illuminate\Support\Facades\App; +use Illuminate\Foundation\Application; use Illuminate\Contracts\Config\Repository; class LanguageMiddleware { + /** + * @var \Illuminate\Foundation\Application + */ + private $app; + /** * @var \Illuminate\Contracts\Config\Repository */ @@ -24,10 +29,12 @@ class LanguageMiddleware /** * LanguageMiddleware constructor. * + * @param \Illuminate\Foundation\Application $app * @param \Illuminate\Contracts\Config\Repository $config */ - public function __construct(Repository $config) + public function __construct(Application $app, Repository $config) { + $this->app = $app; $this->config = $config; } @@ -40,7 +47,7 @@ class LanguageMiddleware */ public function handle(Request $request, Closure $next) { - App::setLocale($this->config->get('app.locale', 'en')); + $this->app->setLocale($this->config->get('app.locale', 'en')); return $next($request); } diff --git a/app/Http/Middleware/RedirectIfAuthenticated.php b/app/Http/Middleware/RedirectIfAuthenticated.php index ae55fef92..8a5220cb5 100644 --- a/app/Http/Middleware/RedirectIfAuthenticated.php +++ b/app/Http/Middleware/RedirectIfAuthenticated.php @@ -9,7 +9,7 @@ use Illuminate\Auth\AuthManager; class RedirectIfAuthenticated { /** - * @var \Illuminate\Contracts\Auth\Guard + * @var \Illuminate\Auth\AuthManager */ private $authManager; @@ -34,7 +34,7 @@ class RedirectIfAuthenticated public function handle(Request $request, Closure $next, string $guard = null) { if ($this->authManager->guard($guard)->check()) { - return redirect(route('index')); + return redirect()->route('index'); } return $next($request); diff --git a/app/Http/Middleware/RequireTwoFactorAuthentication.php b/app/Http/Middleware/RequireTwoFactorAuthentication.php index 75fb01664..bc5ff70ee 100644 --- a/app/Http/Middleware/RequireTwoFactorAuthentication.php +++ b/app/Http/Middleware/RequireTwoFactorAuthentication.php @@ -73,27 +73,23 @@ class RequireTwoFactorAuthentication */ public function handle(Request $request, Closure $next) { - // Ignore non-users if (! $request->user()) { return $next($request); } - // Skip the 2FA pages if (in_array($request->route()->getName(), $this->except)) { return $next($request); } - // Get the setting switch ((int) $this->settings->get('2fa', 0)) { case self::LEVEL_NONE: return $next($request); - + break; case self::LEVEL_ADMIN: - if (! $request->user()->root_admin) { + if (! $request->user()->root_admin || $request->user()->use_totp) { return $next($request); } break; - case self::LEVEL_ALL: if ($request->user()->use_totp) { return $next($request); diff --git a/database/factories/ModelFactory.php b/database/factories/ModelFactory.php index 54ba984be..1df550e0e 100644 --- a/database/factories/ModelFactory.php +++ b/database/factories/ModelFactory.php @@ -74,6 +74,7 @@ $factory->define(Pterodactyl\Models\Location::class, function (Faker\Generator $ $factory->define(Pterodactyl\Models\Node::class, function (Faker\Generator $faker) { return [ 'id' => $faker->unique()->randomNumber(), + 'uuid' => $faker->unique()->uuid, 'public' => true, 'name' => $faker->firstName, 'fqdn' => $faker->ipv4, diff --git a/tests/Traits/Http/MocksMiddlewareClosure.php b/tests/Traits/Http/MocksMiddlewareClosure.php index 53b922585..5c34c4fb6 100644 --- a/tests/Traits/Http/MocksMiddlewareClosure.php +++ b/tests/Traits/Http/MocksMiddlewareClosure.php @@ -8,11 +8,6 @@ use BadFunctionCallException; trait MocksMiddlewareClosure { - /** - * @var \Illuminate\Http\Request - */ - protected $request; - /** * Provide a closure to be used when validating that the response from the middleware * is the same request object we passed into it. diff --git a/tests/Unit/Http/Middleware/AdminAuthenticateTest.php b/tests/Unit/Http/Middleware/AdminAuthenticateTest.php new file mode 100644 index 000000000..2e1850505 --- /dev/null +++ b/tests/Unit/Http/Middleware/AdminAuthenticateTest.php @@ -0,0 +1,62 @@ +make(['root_admin' => 1]); + + $this->request->shouldReceive('user')->withNoArgs()->twice()->andReturn($user); + + $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + } + + /** + * Test that a missing user in the request triggers an error. + */ + public function testExceptionIsThrownIfUserDoesNotExist() + { + $this->request->shouldReceive('user')->withNoArgs()->once()->andReturnNull(); + + try { + $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + } catch (HttpException $exception) { + $this->assertEquals(403, $exception->getStatusCode()); + } + } + + /** + * Test that an exception is thrown if the user is not an admin. + */ + public function testExceptionIsThrownIfUserIsNotAnAdmin() + { + $user = factory(User::class)->make(['root_admin' => 0]); + + $this->request->shouldReceive('user')->withNoArgs()->twice()->andReturn($user); + + try { + $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + } catch (HttpException $exception) { + $this->assertEquals(403, $exception->getStatusCode()); + } + } + + /** + * Return an instance of the middleware using mocked dependencies. + * + * @return \Pterodactyl\Http\Middleware\AdminAuthenticate + */ + private function getMiddleware(): AdminAuthenticate + { + return new AdminAuthenticate(); + } +} diff --git a/tests/Unit/Http/Middleware/AuthenticateTest.php b/tests/Unit/Http/Middleware/AuthenticateTest.php new file mode 100644 index 000000000..88c5990d5 --- /dev/null +++ b/tests/Unit/Http/Middleware/AuthenticateTest.php @@ -0,0 +1,57 @@ +request->shouldReceive('user')->withNoArgs()->once()->andReturn(true); + + $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + } + + /** + * Test that a logged out user results in a redirect. + */ + public function testLoggedOutUser() + { + $this->request->shouldReceive('user')->withNoArgs()->once()->andReturnNull(); + $this->request->shouldReceive('ajax')->withNoArgs()->once()->andReturn(false); + $this->request->shouldReceive('expectsJson')->withNoArgs()->once()->andReturn(false); + + $response = $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + $this->assertInstanceOf(RedirectResponse::class, $response); + $this->assertEquals(302, $response->getStatusCode()); + $this->assertEquals(route('auth.login'), $response->getTargetUrl()); + } + + /** + * Test that a logged out user via an API/Ajax request returns a HTTP error. + * + * @expectedException \Illuminate\Auth\AuthenticationException + */ + public function testLoggedOUtUserApiRequest() + { + $this->request->shouldReceive('user')->withNoArgs()->once()->andReturnNull(); + $this->request->shouldReceive('ajax')->withNoArgs()->once()->andReturn(true); + + $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + } + + /** + * Return an instance of the middleware using mocked dependencies. + * + * @return \Pterodactyl\Http\Middleware\Authenticate + */ + private function getMiddleware(): Authenticate + { + return new Authenticate(); + } +} diff --git a/tests/Unit/Http/Middleware/DaemonAuthenticateTest.php b/tests/Unit/Http/Middleware/DaemonAuthenticateTest.php new file mode 100644 index 000000000..f6531bfbc --- /dev/null +++ b/tests/Unit/Http/Middleware/DaemonAuthenticateTest.php @@ -0,0 +1,77 @@ +repository = m::mock(NodeRepositoryInterface::class); + } + + /** + * Test a valid daemon connection. + */ + public function testValidDaemonConnection() + { + $node = factory(Node::class)->make(); + + $this->request->shouldReceive('route->getName')->withNoArgs()->once()->andReturn('random.name'); + $this->request->shouldReceive('header')->with('X-Access-Node')->twice()->andReturn($node->uuid); + + $this->repository->shouldReceive('findWhere')->with(['daemonSecret' => $node->uuid])->once()->andReturn($node); + + $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + $this->assertRequestHasAttribute('node'); + $this->assertRequestAttributeEquals($node, 'node'); + } + + /** + * Test that ignored routes do not continue through the middleware. + */ + public function testIgnoredRouteShouldContinue() + { + $this->request->shouldReceive('route->getName')->withNoArgs()->once()->andReturn('daemon.configuration'); + + $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + $this->assertRequestMissingAttribute('node'); + } + + /** + * Test that a request missing a X-Access-Node header causes an exception. + * + * @expectedException \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException + */ + public function testExceptionThrownIfMissingHeader() + { + $this->request->shouldReceive('route->getName')->withNoArgs()->once()->andReturn('random.name'); + $this->request->shouldReceive('header')->with('X-Access-Node')->once()->andReturn(false); + + $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + } + + /** + * Return an instance of the middleware using mocked dependencies. + * + * @return \Pterodactyl\Http\Middleware\DaemonAuthenticate + */ + private function getMiddleware(): DaemonAuthenticate + { + return new DaemonAuthenticate($this->repository); + } +} diff --git a/tests/Unit/Http/Middleware/LanguageMiddlewareTest.php b/tests/Unit/Http/Middleware/LanguageMiddlewareTest.php new file mode 100644 index 000000000..c156665aa --- /dev/null +++ b/tests/Unit/Http/Middleware/LanguageMiddlewareTest.php @@ -0,0 +1,53 @@ +appMock = m::mock(Application::class); + $this->config = m::mock(Repository::class); + } + + /** + * Test that a language is defined via the middleware. + */ + public function testLanguageIsSet() + { + $this->config->shouldReceive('get')->with('app.locale', 'en')->once()->andReturn('en'); + $this->appMock->shouldReceive('setLocale')->with('en')->once()->andReturnNull(); + + $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + } + + /** + * Return an instance of the middleware using mocked dependencies. + * + * @return \Pterodactyl\Http\Middleware\LanguageMiddleware + */ + private function getMiddleware(): LanguageMiddleware + { + return new LanguageMiddleware($this->appMock, $this->config); + } +} diff --git a/tests/Unit/Http/Middleware/RedirectIfAuthenticatedTest.php b/tests/Unit/Http/Middleware/RedirectIfAuthenticatedTest.php new file mode 100644 index 000000000..ee56156d9 --- /dev/null +++ b/tests/Unit/Http/Middleware/RedirectIfAuthenticatedTest.php @@ -0,0 +1,60 @@ +authManager = m::mock(AuthManager::class); + } + + /** + * Test that an authenticated user is redirected. + */ + public function testAuthenticatedUserIsRedirected() + { + $this->authManager->shouldReceive('guard')->with(null)->once()->andReturnSelf(); + $this->authManager->shouldReceive('check')->with(null)->once()->andReturn(true); + + $response = $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + $this->assertInstanceOf(RedirectResponse::class, $response); + $this->assertEquals(route('index'), $response->getTargetUrl()); + } + + /** + * Test that a non-authenticated user continues through the middleware. + */ + public function testNonAuthenticatedUserIsNotRedirected() + { + $this->authManager->shouldReceive('guard')->with(null)->once()->andReturnSelf(); + $this->authManager->shouldReceive('check')->with(null)->once()->andReturn(false); + + $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + } + + /** + * Return an instance of the middleware using mocked dependencies. + * + * @return \Pterodactyl\Http\Middleware\RedirectIfAuthenticated + */ + private function getMiddleware(): RedirectIfAuthenticated + { + return new RedirectIfAuthenticated($this->authManager); + } +} diff --git a/tests/Unit/Http/Middleware/RequireTwoFactorAuthenticationTest.php b/tests/Unit/Http/Middleware/RequireTwoFactorAuthenticationTest.php new file mode 100644 index 000000000..fac13aab9 --- /dev/null +++ b/tests/Unit/Http/Middleware/RequireTwoFactorAuthenticationTest.php @@ -0,0 +1,181 @@ +alert = m::mock(AlertsMessageBag::class); + $this->settings = m::mock(Settings::class); + } + + /** + * Test that a missing user does not trigger this middleware. + */ + public function testRequestMissingUser() + { + $this->request->shouldReceive('user')->withNoArgs()->once()->andReturnNull(); + + $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + } + + /** + * Test that the middleware is ignored on specific routes. + * + * @dataProvider ignoredRoutesDataProvider + */ + public function testRequestOnIgnoredRoute($route) + { + $this->request->shouldReceive('user')->withNoArgs()->once()->andReturn(true); + $this->request->shouldReceive('route->getName')->withNoArgs()->once()->andReturn($route); + + $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + } + + /** + * Test disabled 2FA requirement. + */ + public function testTwoFactorRequirementDisabled() + { + $this->request->shouldReceive('user')->withNoArgs()->once()->andReturn(true); + $this->request->shouldReceive('route->getName')->withNoArgs()->once()->andReturn('random.route'); + + $this->settings->shouldReceive('get')->with('2fa', 0)->once()->andReturn(RequireTwoFactorAuthentication::LEVEL_NONE); + + $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + } + + /** + * Test 2FA required for admins as an administrative user who has 2FA disabled. + */ + public function testTwoFactorEnabledForAdminsAsAdminUserWith2FADisabled() + { + $user = factory(User::class)->make(['root_admin' => 1, 'use_totp' => 0]); + + $this->request->shouldReceive('user')->withNoArgs()->times(3)->andReturn($user); + $this->request->shouldReceive('route->getName')->withNoArgs()->once()->andReturn('random.route'); + + $this->settings->shouldReceive('get')->with('2fa', 0)->once()->andReturn(RequireTwoFactorAuthentication::LEVEL_ADMIN); + $this->alert->shouldReceive('danger')->with(trans('auth.2fa_must_be_enabled'))->once()->andReturnSelf(); + $this->alert->shouldReceive('flash')->withNoArgs()->once()->andReturnSelf(); + + $response = $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + $this->assertInstanceOf(RedirectResponse::class, $response); + $this->assertEquals(route('account.security'), $response->getTargetUrl()); + } + + /** + * Test 2FA required for admins as an administrative user who has 2FA enabled. + */ + public function testTwoFactorEnabledForAdminsAsAdminUserWith2FAEnabled() + { + $user = factory(User::class)->make(['root_admin' => 1, 'use_totp' => 1]); + + $this->request->shouldReceive('user')->withNoArgs()->times(3)->andReturn($user); + $this->request->shouldReceive('route->getName')->withNoArgs()->once()->andReturn('random.route'); + + $this->settings->shouldReceive('get')->with('2fa', 0)->once()->andReturn(RequireTwoFactorAuthentication::LEVEL_ADMIN); + + $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + } + + /** + * Test 2FA required for admins as an administrative user. + */ + public function testTwoFactorEnabledForAdminsAsNonAdmin() + { + $user = factory(User::class)->make(['root_admin' => 0]); + + $this->request->shouldReceive('user')->withNoArgs()->twice()->andReturn($user); + $this->request->shouldReceive('route->getName')->withNoArgs()->once()->andReturn('random.route'); + + $this->settings->shouldReceive('get')->with('2fa', 0)->once()->andReturn(RequireTwoFactorAuthentication::LEVEL_ADMIN); + + $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + } + + /** + * Test 2FA required for all users without 2FA enabled. + */ + public function testTwoFactorEnabledForAllUsersAsUserWith2FADisabled() + { + $user = factory(User::class)->make(['use_totp' => 0]); + + $this->request->shouldReceive('user')->withNoArgs()->twice()->andReturn($user); + $this->request->shouldReceive('route->getName')->withNoArgs()->once()->andReturn('random.route'); + + $this->settings->shouldReceive('get')->with('2fa', 0)->once()->andReturn(RequireTwoFactorAuthentication::LEVEL_ALL); + $this->alert->shouldReceive('danger')->with(trans('auth.2fa_must_be_enabled'))->once()->andReturnSelf(); + $this->alert->shouldReceive('flash')->withNoArgs()->once()->andReturnSelf(); + + $response = $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + $this->assertInstanceOf(RedirectResponse::class, $response); + $this->assertEquals(route('account.security'), $response->getTargetUrl()); + } + + /** + * Test 2FA required for all users without 2FA enabled. + */ + public function testTwoFactorEnabledForAllUsersAsUserWith2FAEnabled() + { + $user = factory(User::class)->make(['use_totp' => 1]); + + $this->request->shouldReceive('user')->withNoArgs()->twice()->andReturn($user); + $this->request->shouldReceive('route->getName')->withNoArgs()->once()->andReturn('random.route'); + + $this->settings->shouldReceive('get')->with('2fa', 0)->once()->andReturn(RequireTwoFactorAuthentication::LEVEL_ALL); + + $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + } + + /** + * Routes that should be ignored. + * + * @return array + */ + public function ignoredRoutesDataProvider() + { + return [ + ['account.security'], + ['account.security.revoke'], + ['account.security.totp'], + ['account.security.totp.set'], + ['account.security.totp.disable'], + ['auth.totp'], + ['auth.logout'], + ]; + } + + /** + * Return an instance of the middleware using mocked dependencies. + * + * @return \Pterodactyl\Http\Middleware\RequireTwoFactorAuthentication + */ + private function getMiddleware(): RequireTwoFactorAuthentication + { + return new RequireTwoFactorAuthentication($this->alert, $this->settings); + } +}