diff --git a/app/Http/Middleware/Server/DatabaseBelongsToServer.php b/app/Http/Middleware/Server/DatabaseBelongsToServer.php index bc31c29c8..d7e34d211 100644 --- a/app/Http/Middleware/Server/DatabaseBelongsToServer.php +++ b/app/Http/Middleware/Server/DatabaseBelongsToServer.php @@ -12,7 +12,7 @@ class DatabaseBelongsToServer /** * @var \Pterodactyl\Contracts\Repository\DatabaseRepositoryInterface */ - protected $repository; + private $repository; /** * DatabaseAccess constructor. @@ -40,7 +40,7 @@ class DatabaseBelongsToServer $server = $request->attributes->get('server'); $database = $this->repository->find($request->input('database')); - if ($database->server_id !== $server->id) { + if (is_null($database) || $database->server_id !== $server->id) { throw new NotFoundHttpException; } diff --git a/app/Http/Middleware/Server/SubuserBelongsToServer.php b/app/Http/Middleware/Server/SubuserBelongsToServer.php index 18291245d..cdcd3f097 100644 --- a/app/Http/Middleware/Server/SubuserBelongsToServer.php +++ b/app/Http/Middleware/Server/SubuserBelongsToServer.php @@ -50,7 +50,7 @@ class SubuserBelongsToServer $hash = $request->route()->parameter('subuser', 0); $subuser = $this->repository->find($this->hashids->decodeFirst($hash, 0)); - if (! $subuser || $subuser->server_id !== $server->id) { + if (is_null($subuser) || $subuser->server_id !== $server->id) { throw new NotFoundHttpException; } diff --git a/tests/Assertions/MiddlewareAttributeAssertionsTrait.php b/tests/Assertions/MiddlewareAttributeAssertionsTrait.php new file mode 100644 index 000000000..fb8f70086 --- /dev/null +++ b/tests/Assertions/MiddlewareAttributeAssertionsTrait.php @@ -0,0 +1,39 @@ +request->attributes->has($attribute), 'Assert that request mock has ' . $attribute . ' attribute.'); + } + + /** + * Assert a request does not have an attribute assigned to it. + * + * @param string $attribute + */ + public function assertRequestMissingAttribute(string $attribute) + { + Assert::assertFalse($this->request->attributes->has($attribute), 'Assert that request mock does not have ' . $attribute . ' attribute.'); + } + + /** + * Assert a request attribute matches an expected value. + * + * @param mixed $expected + * @param string $attribute + */ + public function assertRequestAttributeEquals($expected, string $attribute) + { + Assert::assertEquals($expected, $this->request->attributes->get($attribute)); + } +} diff --git a/tests/Traits/Http/MocksMiddlewareClosure.php b/tests/Traits/Http/MocksMiddlewareClosure.php new file mode 100644 index 000000000..53b922585 --- /dev/null +++ b/tests/Traits/Http/MocksMiddlewareClosure.php @@ -0,0 +1,31 @@ +request)) { + throw new BadFunctionCallException('Calling getClosureAssertions without defining a request object is not supported.'); + } + + return function ($response) { + $this->assertInstanceOf(Request::class, $response); + $this->assertSame($this->request, $response); + }; + } +} diff --git a/tests/Unit/Http/Middleware/Daemon/DaemonAuthenticateTest.php b/tests/Unit/Http/Middleware/Daemon/DaemonAuthenticateTest.php index efe667743..140c07286 100644 --- a/tests/Unit/Http/Middleware/Daemon/DaemonAuthenticateTest.php +++ b/tests/Unit/Http/Middleware/Daemon/DaemonAuthenticateTest.php @@ -2,29 +2,21 @@ namespace Tests\Unit\Http\Middleware\Daemon; -use Closure; use Mockery as m; -use Tests\TestCase; -use Illuminate\Http\Request; use Pterodactyl\Models\Node; -use Symfony\Component\HttpFoundation\ParameterBag; +use Tests\Unit\Http\Middleware\MiddlewareTestCase; use Symfony\Component\HttpKernel\Exception\HttpException; use Pterodactyl\Http\Middleware\Daemon\DaemonAuthenticate; use Pterodactyl\Contracts\Repository\NodeRepositoryInterface; use Pterodactyl\Exceptions\Repository\RecordNotFoundException; -class DaemonAuthenticateTest extends TestCase +class DaemonAuthenticateTest extends MiddlewareTestCase { /** * @var \Pterodactyl\Contracts\Repository\NodeRepositoryInterface|\Mockery\Mock */ private $repository; - /** - * @var \Illuminate\Http\Request|\Mockery\Mock - */ - private $request; - /** * Setup tests. */ @@ -33,8 +25,6 @@ class DaemonAuthenticateTest extends TestCase parent::setUp(); $this->repository = m::mock(NodeRepositoryInterface::class); - $this->request = m::mock(Request::class); - $this->request->attributes = new ParameterBag(); } /** @@ -98,8 +88,8 @@ class DaemonAuthenticateTest extends TestCase $this->repository->shouldReceive('findFirstWhere')->with([['daemonSecret', '=', $model->daemonSecret]])->once()->andReturn($model); $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); - $this->assertTrue($this->request->attributes->has('node'), 'Assert request attributes contains node.'); - $this->assertSame($model, $this->request->attributes->get('node')); + $this->assertRequestHasAttribute('node'); + $this->assertRequestAttributeEquals($model, 'node'); } /** @@ -111,16 +101,4 @@ class DaemonAuthenticateTest extends TestCase { return new DaemonAuthenticate($this->repository); } - - /** - * Provide a closure to be used when validating that the response from the middleware - * is the same request object we passed into it. - */ - private function getClosureAssertions(): Closure - { - return function ($response) { - $this->assertInstanceOf(Request::class, $response); - $this->assertSame($this->request, $response); - }; - } } diff --git a/tests/Unit/Http/Middleware/MiddlewareTestCase.php b/tests/Unit/Http/Middleware/MiddlewareTestCase.php new file mode 100644 index 000000000..463570f0e --- /dev/null +++ b/tests/Unit/Http/Middleware/MiddlewareTestCase.php @@ -0,0 +1,58 @@ +request = m::mock(Request::class); + $this->request->attributes = new ParameterBag(); + } + + /** + * Set a request attribute on the mock object. + * + * @param string $attribute + * @param mixed $value + */ + protected function setRequestAttribute(string $attribute, $value) + { + $this->request->attributes->set($attribute, $value); + } + + /** + * Sets the mocked request user. If a user model is not provided, a factory model + * will be created and returned. + * + * @param \Pterodactyl\Models\User|null $user + * @return \Pterodactyl\Models\User + */ + protected function setRequestUser(User $user = null): User + { + $user = $user instanceof User ? $user : factory(User::class)->make(); + $this->request->shouldReceive('user')->withNoArgs()->andReturn($user); + + return $user; + } +} diff --git a/tests/Unit/Http/Middleware/Server/AccessingValidServerTest.php b/tests/Unit/Http/Middleware/Server/AccessingValidServerTest.php index 0cf27e8a6..deed73a69 100644 --- a/tests/Unit/Http/Middleware/Server/AccessingValidServerTest.php +++ b/tests/Unit/Http/Middleware/Server/AccessingValidServerTest.php @@ -2,20 +2,16 @@ namespace Tests\Unit\Http\Middleware\Server; -use Closure; use Mockery as m; -use Tests\TestCase; -use Illuminate\View\View; -use Illuminate\Http\Request; use Illuminate\Http\Response; use Pterodactyl\Models\Server; use Illuminate\Contracts\Session\Session; use Illuminate\Contracts\Config\Repository; -use Symfony\Component\HttpFoundation\ParameterBag; +use Tests\Unit\Http\Middleware\MiddlewareTestCase; use Pterodactyl\Http\Middleware\AccessingValidServer; use Pterodactyl\Contracts\Repository\ServerRepositoryInterface; -class AccessingValidServerTest extends TestCase +class AccessingValidServerTest extends MiddlewareTestCase { /** * @var \Illuminate\Contracts\Config\Repository|\Mockery\Mock @@ -27,11 +23,6 @@ class AccessingValidServerTest extends TestCase */ private $repository; - /** - * @var \Illuminate\Http\Request|\Mockery\Mock - */ - private $request; - /** * @var \Illuminate\Contracts\Session\Session|\Mockery\Mock */ @@ -46,8 +37,6 @@ class AccessingValidServerTest extends TestCase $this->config = m::mock(Repository::class); $this->repository = m::mock(ServerRepositoryInterface::class); - $this->request = m::mock(Request::class); - $this->request->attributes = new ParameterBag(); $this->session = m::mock(Session::class); } @@ -139,8 +128,8 @@ class AccessingValidServerTest extends TestCase $this->session->shouldReceive('now')->with('server_data.model', $model)->once()->andReturnNull(); $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); - $this->assertTrue($this->request->attributes->has('server'), 'Assert request attributes contains server.'); - $this->assertSame($model, $this->request->attributes->get('server')); + $this->assertRequestHasAttribute('server'); + $this->assertRequestAttributeEquals($model, 'server'); } /** @@ -170,16 +159,4 @@ class AccessingValidServerTest extends TestCase { return new AccessingValidServer($this->config, $this->repository, $this->session); } - - /** - * Provide a closure to be used when validating that the response from the middleware - * is the same request object we passed into it. - */ - private function getClosureAssertions(): Closure - { - return function ($response) { - $this->assertInstanceOf(Request::class, $response); - $this->assertSame($this->request, $response); - }; - } } diff --git a/tests/Unit/Http/Middleware/Server/AuthenticateAsSubuserTest.php b/tests/Unit/Http/Middleware/Server/AuthenticateAsSubuserTest.php new file mode 100644 index 000000000..9645ddb6b --- /dev/null +++ b/tests/Unit/Http/Middleware/Server/AuthenticateAsSubuserTest.php @@ -0,0 +1,79 @@ +keyProviderService = m::mock(DaemonKeyProviderService::class); + $this->session = m::mock(Session::class); + } + + /** + * Test a successful instance of the middleware. + */ + public function testSuccessfulMiddleware() + { + $model = factory(Server::class)->make(); + $user = $this->setRequestUser(); + $this->setRequestAttribute('server', $model); + + $this->keyProviderService->shouldReceive('handle')->with($model->id, $user->id)->once()->andReturn('abc123'); + $this->session->shouldReceive('now')->with('server_data.token', 'abc123')->once()->andReturnNull(); + + $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + $this->assertRequestHasAttribute('server_token'); + $this->assertRequestAttributeEquals('abc123', 'server_token'); + } + + /** + * Test middleware handles missing token exception. + * + * @expectedException \Illuminate\Auth\AuthenticationException + * @expectedExceptionMessage This account does not have permission to access this server. + */ + public function testExceptionIsThrownIfNoTokenIsFound() + { + $model = factory(Server::class)->make(); + $user = $this->setRequestUser(); + $this->setRequestAttribute('server', $model); + + $this->keyProviderService->shouldReceive('handle')->with($model->id, $user->id)->once()->andThrow(new RecordNotFoundException); + + $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + } + + /** + * Return an instance of the middleware using mocked dependencies. + * + * @return \Pterodactyl\Http\Middleware\Server\AuthenticateAsSubuser + */ + public function getMiddleware(): AuthenticateAsSubuser + { + return new AuthenticateAsSubuser($this->keyProviderService, $this->session); + } +} diff --git a/tests/Unit/Http/Middleware/Server/DatabaseBelongsToServerTest.php b/tests/Unit/Http/Middleware/Server/DatabaseBelongsToServerTest.php new file mode 100644 index 000000000..856ea1b98 --- /dev/null +++ b/tests/Unit/Http/Middleware/Server/DatabaseBelongsToServerTest.php @@ -0,0 +1,92 @@ +repository = m::mock(DatabaseRepositoryInterface::class); + } + + /** + * Test a successful middleware instance. + */ + public function testSuccessfulMiddleware() + { + $model = factory(Server::class)->make(); + $database = factory(Database::class)->make([ + 'server_id' => $model->id, + ]); + $this->setRequestAttribute('server', $model); + + $this->request->shouldReceive('input')->with('database')->once()->andReturn($database->id); + $this->repository->shouldReceive('find')->with($database->id)->once()->andReturn($database); + + $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + $this->assertRequestHasAttribute('database'); + $this->assertRequestAttributeEquals($database, 'database'); + } + + /** + * Test that an exception is thrown if no database record is found. + * + * @expectedException \Symfony\Component\HttpKernel\Exception\NotFoundHttpException + */ + public function testExceptionIsThrownIfNoDatabaseRecordFound() + { + $model = factory(Server::class)->make(); + $database = factory(Database::class)->make(); + $this->setRequestAttribute('server', $model); + + $this->request->shouldReceive('input')->with('database')->once()->andReturn($database->id); + $this->repository->shouldReceive('find')->with($database->id)->once()->andReturnNull(); + + $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + } + + /** + * Test that an exception is found if the database server does not match the + * request server. + * + * @expectedException \Symfony\Component\HttpKernel\Exception\NotFoundHttpException + */ + public function testExceptionIsThrownIfDatabaseServerDoesNotMatchCurrent() + { + $model = factory(Server::class)->make(); + $database = factory(Database::class)->make(); + $this->setRequestAttribute('server', $model); + + $this->request->shouldReceive('input')->with('database')->once()->andReturn($database->id); + $this->repository->shouldReceive('find')->with($database->id)->once()->andReturn($database); + + $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + } + + /** + * Return an instance of the middleware using mocked dependencies. + * + * @return \Pterodactyl\Http\Middleware\Server\DatabaseBelongsToServer + */ + private function getMiddleware(): DatabaseBelongsToServer + { + return new DatabaseBelongsToServer($this->repository); + } +} diff --git a/tests/Unit/Http/Middleware/Server/ScheduleBelongsToServerTest.php b/tests/Unit/Http/Middleware/Server/ScheduleBelongsToServerTest.php new file mode 100644 index 000000000..755808e06 --- /dev/null +++ b/tests/Unit/Http/Middleware/Server/ScheduleBelongsToServerTest.php @@ -0,0 +1,81 @@ +hashids = m::mock(HashidsInterface::class); + $this->repository = m::mock(ScheduleRepositoryInterface::class); + } + + /** + * Test a successful middleware instance. + */ + public function testSuccessfulMiddleware() + { + $model = factory(Server::class)->make(); + $schedule = factory(Schedule::class)->make([ + 'server_id' => $model->id, + ]); + $this->setRequestAttribute('server', $model); + + $this->request->shouldReceive('route->parameter')->with('schedule')->once()->andReturn('abc123'); + $this->hashids->shouldReceive('decodeFirst')->with('abc123', 0)->once()->andReturn($schedule->id); + $this->repository->shouldReceive('getScheduleWithTasks')->with($schedule->id)->once()->andReturn($schedule); + + $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + $this->assertRequestHasAttribute('schedule'); + $this->assertRequestAttributeEquals($schedule, 'schedule'); + } + + /** + * Test that an exception is thrown if the schedule does not belong to + * the request server. + * + * @expectedException \Symfony\Component\HttpKernel\Exception\NotFoundHttpException + */ + public function testExceptionIsThrownIfScheduleDoesNotBelongToServer() + { + $model = factory(Server::class)->make(); + $schedule = factory(Schedule::class)->make(); + $this->setRequestAttribute('server', $model); + + $this->request->shouldReceive('route->parameter')->with('schedule')->once()->andReturn('abc123'); + $this->hashids->shouldReceive('decodeFirst')->with('abc123', 0)->once()->andReturn($schedule->id); + $this->repository->shouldReceive('getScheduleWithTasks')->with($schedule->id)->once()->andReturn($schedule); + + $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + } + + /** + * Return an instance of the middleware using mocked dependencies. + * + * @return \Pterodactyl\Http\Middleware\Server\ScheduleBelongsToServer + */ + private function getMiddleware(): ScheduleBelongsToServer + { + return new ScheduleBelongsToServer($this->hashids, $this->repository); + } +} diff --git a/tests/Unit/Http/Middleware/Server/SubuserBelongsToServerTest.php b/tests/Unit/Http/Middleware/Server/SubuserBelongsToServerTest.php new file mode 100644 index 000000000..a4cd7e4f7 --- /dev/null +++ b/tests/Unit/Http/Middleware/Server/SubuserBelongsToServerTest.php @@ -0,0 +1,156 @@ +hashids = m::mock(HashidsInterface::class); + $this->repository = m::mock(SubuserRepositoryInterface::class); + } + + /** + * Test a successful middleware instance. + */ + public function testSuccessfulMiddleware() + { + $model = factory(Server::class)->make(); + $subuser = factory(Subuser::class)->make([ + 'server_id' => $model->id, + ]); + $this->setRequestAttribute('server', $model); + + $this->request->shouldReceive('route->parameter')->with('subuser', 0)->once()->andReturn('abc123'); + $this->hashids->shouldReceive('decodeFirst')->with('abc123', 0)->once()->andReturn($subuser->id); + $this->repository->shouldReceive('find')->with($subuser->id)->once()->andReturn($subuser); + + $this->request->shouldReceive('method')->withNoArgs()->once()->andReturn('GET'); + + $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + $this->assertRequestHasAttribute('subuser'); + $this->assertRequestAttributeEquals($subuser, 'subuser'); + } + + /** + * Test that a user can edit a user other than themselves. + */ + public function testSuccessfulMiddlewareWhenPatchRequest() + { + $this->setRequestUser(); + $model = factory(Server::class)->make(); + $subuser = factory(Subuser::class)->make([ + 'server_id' => $model->id, + ]); + $this->setRequestAttribute('server', $model); + + $this->request->shouldReceive('route->parameter')->with('subuser', 0)->once()->andReturn('abc123'); + $this->hashids->shouldReceive('decodeFirst')->with('abc123', 0)->once()->andReturn($subuser->id); + $this->repository->shouldReceive('find')->with($subuser->id)->once()->andReturn($subuser); + + $this->request->shouldReceive('method')->withNoArgs()->once()->andReturn('PATCH'); + + $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + $this->assertRequestHasAttribute('subuser'); + $this->assertRequestAttributeEquals($subuser, 'subuser'); + } + + /** + * Test that an exception is thrown if a user attempts to edit themself. + */ + public function testExceptionIsThrownIfUserTriesToEditSelf() + { + $user = $this->setRequestUser(); + $model = factory(Server::class)->make(); + $subuser = factory(Subuser::class)->make([ + 'server_id' => $model->id, + 'user_id' => $user->id, + ]); + $this->setRequestAttribute('server', $model); + + $this->request->shouldReceive('route->parameter')->with('subuser', 0)->once()->andReturn('abc123'); + $this->hashids->shouldReceive('decodeFirst')->with('abc123', 0)->once()->andReturn($subuser->id); + $this->repository->shouldReceive('find')->with($subuser->id)->once()->andReturn($subuser); + + $this->request->shouldReceive('method')->withNoArgs()->once()->andReturn('PATCH'); + + try { + $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + } catch (PterodactylException $exception) { + $this->assertInstanceOf(DisplayException::class, $exception); + $this->assertEquals(trans('exceptions.subusers.editing_self'), $exception->getMessage()); + } + } + + /** + * Test that an exception is thrown if a subuser server does not match the + * request server. + * + * @expectedException \Symfony\Component\HttpKernel\Exception\NotFoundHttpException + */ + public function testExceptionIsThrownIfSubuserServerDoesNotMatchRequestServer() + { + $model = factory(Server::class)->make(); + $subuser = factory(Subuser::class)->make(); + $this->setRequestAttribute('server', $model); + + $this->request->shouldReceive('route->parameter')->with('subuser', 0)->once()->andReturn('abc123'); + $this->hashids->shouldReceive('decodeFirst')->with('abc123', 0)->once()->andReturn($subuser->id); + $this->repository->shouldReceive('find')->with($subuser->id)->once()->andReturn($subuser); + + $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + } + + /** + * Test that an exception is thrown if no subuser is found. + * + * @expectedException \Symfony\Component\HttpKernel\Exception\NotFoundHttpException + */ + public function testExceptionIsThrownIfNoSubuserIsFound() + { + $model = factory(Server::class)->make(); + $subuser = factory(Subuser::class)->make(); + $this->setRequestAttribute('server', $model); + + $this->request->shouldReceive('route->parameter')->with('subuser', 0)->once()->andReturn('abc123'); + $this->hashids->shouldReceive('decodeFirst')->with('abc123', 0)->once()->andReturn($subuser->id); + $this->repository->shouldReceive('find')->with($subuser->id)->once()->andReturnNull(); + + $this->getMiddleware()->handle($this->request, $this->getClosureAssertions()); + } + + /** + * Return an instance of the middleware using mocked dependencies. + * + * @return \Pterodactyl\Http\Middleware\Server\SubuserBelongsToServer + */ + private function getMiddleware(): SubuserBelongsToServer + { + return new SubuserBelongsToServer($this->hashids, $this->repository); + } +}