Update transformers and controllers to no longer pull an API key attribute
This commit is contained in:
parent
bd37978a98
commit
e9c633fd03
9 changed files with 91 additions and 173 deletions
|
@ -55,17 +55,20 @@ abstract class ApplicationApiController extends Controller
|
|||
/**
|
||||
* Return an instance of an application transformer.
|
||||
*
|
||||
* @return \Pterodactyl\Transformers\Api\Application\BaseTransformer
|
||||
* @template T of \Pterodactyl\Transformers\Api\Application\BaseTransformer
|
||||
*
|
||||
* @param class-string<T> $abstract
|
||||
*
|
||||
* @return T
|
||||
*
|
||||
* @noinspection PhpDocSignatureInspection
|
||||
* @noinspection PhpUndefinedClassInspection
|
||||
*/
|
||||
public function getTransformer(string $abstract)
|
||||
{
|
||||
/** @var \Pterodactyl\Transformers\Api\Application\BaseTransformer $transformer */
|
||||
$transformer = Container::getInstance()->make($abstract);
|
||||
$transformer->setKey($this->request->attributes->get('api_key'));
|
||||
Assert::subclassOf($abstract, BaseTransformer::class);
|
||||
|
||||
Assert::isInstanceOf($transformer, BaseTransformer::class);
|
||||
|
||||
return $transformer;
|
||||
return $abstract::fromRequest($this->request);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -3,7 +3,6 @@
|
|||
namespace Pterodactyl\Http\Controllers\Api\Client;
|
||||
|
||||
use Webmozart\Assert\Assert;
|
||||
use Illuminate\Container\Container;
|
||||
use Pterodactyl\Transformers\Daemon\BaseDaemonTransformer;
|
||||
use Pterodactyl\Transformers\Api\Client\BaseClientTransformer;
|
||||
use Pterodactyl\Http\Controllers\Api\Application\ApplicationApiController;
|
||||
|
@ -45,21 +44,23 @@ abstract class ClientApiController extends ApplicationApiController
|
|||
/**
|
||||
* Return an instance of an application transformer.
|
||||
*
|
||||
* @return \Pterodactyl\Transformers\Api\Client\BaseClientTransformer
|
||||
* @template T of \Pterodactyl\Transformers\Api\Client\BaseClientTransformer
|
||||
*
|
||||
* @param class-string<T> $abstract
|
||||
*
|
||||
* @return T
|
||||
*
|
||||
* @noinspection PhpUndefinedClassInspection
|
||||
* @noinspection PhpDocSignatureInspection
|
||||
*/
|
||||
public function getTransformer(string $abstract)
|
||||
{
|
||||
/** @var \Pterodactyl\Transformers\Api\Client\BaseClientTransformer $transformer */
|
||||
$transformer = Container::getInstance()->make($abstract);
|
||||
Assert::isInstanceOfAny($transformer, [
|
||||
BaseClientTransformer::class,
|
||||
BaseDaemonTransformer::class,
|
||||
]);
|
||||
Assert::methodExists($abstract, 'fromRequest');
|
||||
|
||||
if ($transformer instanceof BaseClientTransformer) {
|
||||
$transformer->setKey($this->request->attributes->get('api_key'));
|
||||
$transformer->setUser($this->request->user());
|
||||
}
|
||||
/** @var T $transformer */
|
||||
$transformer = $abstract::fromRequest($this->request);
|
||||
|
||||
Assert::isInstanceOfAny($transformer, [BaseClientTransformer::class, BaseDaemonTransformer::class]);
|
||||
|
||||
return $transformer;
|
||||
}
|
||||
|
|
|
@ -3,6 +3,7 @@
|
|||
namespace Pterodactyl\Http\Requests\Api\Application;
|
||||
|
||||
use Webmozart\Assert\Assert;
|
||||
use Laravel\Sanctum\TransientToken;
|
||||
use Illuminate\Validation\Validator;
|
||||
use Illuminate\Database\Eloquent\Model;
|
||||
use Pterodactyl\Services\Acl\Api\AdminAcl;
|
||||
|
@ -11,14 +12,6 @@ use Pterodactyl\Exceptions\PterodactylException;
|
|||
|
||||
abstract class ApplicationApiRequest extends FormRequest
|
||||
{
|
||||
/**
|
||||
* Tracks if the request has been validated internally or not to avoid
|
||||
* making duplicate validation calls.
|
||||
*
|
||||
* @var bool
|
||||
*/
|
||||
private $hasValidated = false;
|
||||
|
||||
/**
|
||||
* The resource that should be checked when performing the authorization
|
||||
* function for this request.
|
||||
|
@ -47,7 +40,12 @@ abstract class ApplicationApiRequest extends FormRequest
|
|||
throw new PterodactylException('An ACL resource must be defined on API requests.');
|
||||
}
|
||||
|
||||
return AdminAcl::check($this->attributes->get('api_key'), $this->resource, $this->permission);
|
||||
$token = $this->user()->currentAccessToken();
|
||||
if ($token instanceof TransientToken) {
|
||||
return true;
|
||||
}
|
||||
|
||||
return AdminAcl::check($token, $this->resource, $this->permission);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -3,12 +3,13 @@
|
|||
namespace Pterodactyl\Transformers\Api\Application;
|
||||
|
||||
use Carbon\CarbonImmutable;
|
||||
use Illuminate\Http\Request;
|
||||
use Webmozart\Assert\Assert;
|
||||
use Pterodactyl\Models\ApiKey;
|
||||
use Illuminate\Container\Container;
|
||||
use Illuminate\Database\Eloquent\Model;
|
||||
use League\Fractal\TransformerAbstract;
|
||||
use Pterodactyl\Services\Acl\Api\AdminAcl;
|
||||
use Pterodactyl\Exceptions\Transformer\InvalidTransformerLevelException;
|
||||
|
||||
/**
|
||||
* @method array transform(Model $model)
|
||||
|
@ -17,15 +18,7 @@ abstract class BaseTransformer extends TransformerAbstract
|
|||
{
|
||||
public const RESPONSE_TIMEZONE = 'UTC';
|
||||
|
||||
/**
|
||||
* @var \Pterodactyl\Models\ApiKey
|
||||
*/
|
||||
private $key;
|
||||
|
||||
/**
|
||||
* Return the resource name for the JSONAPI output.
|
||||
*/
|
||||
abstract public function getResourceName(): string;
|
||||
protected Request $request;
|
||||
|
||||
/**
|
||||
* BaseTransformer constructor.
|
||||
|
@ -39,54 +32,69 @@ abstract class BaseTransformer extends TransformerAbstract
|
|||
}
|
||||
|
||||
/**
|
||||
* Set the HTTP request class being used for this request.
|
||||
*
|
||||
* @return $this
|
||||
* Return the resource name for the JSONAPI output.
|
||||
*/
|
||||
public function setKey(ApiKey $key)
|
||||
abstract public function getResourceName(): string;
|
||||
|
||||
/**
|
||||
* Sets the request on the instance.
|
||||
*
|
||||
* @return static
|
||||
*/
|
||||
public function setRequest(Request $request): self
|
||||
{
|
||||
$this->key = $key;
|
||||
$this->request = $request;
|
||||
|
||||
return $this;
|
||||
}
|
||||
|
||||
/**
|
||||
* Return the request instance being used for this transformer.
|
||||
* Returns a new transformer instance with the request set on the instance.
|
||||
*
|
||||
* @return \Pterodactyl\Transformers\Api\Application\BaseTransformer
|
||||
*/
|
||||
public function getKey(): ApiKey
|
||||
public static function fromRequest(Request $request)
|
||||
{
|
||||
return $this->key;
|
||||
return app(static::class)->setRequest($request);
|
||||
}
|
||||
|
||||
/**
|
||||
* Determine if the API key loaded onto the transformer has permission
|
||||
* to access a different resource. This is used when including other
|
||||
* models on a transformation request.
|
||||
*
|
||||
* @deprecated — prefer $user->can/cannot methods
|
||||
*/
|
||||
protected function authorize(string $resource): bool
|
||||
{
|
||||
return AdminAcl::check($this->getKey(), $resource, AdminAcl::READ);
|
||||
$token = $this->request->user()->currentAccessToken();
|
||||
if (!$token instanceof ApiKey || $token->key_type !== ApiKey::TYPE_APPLICATION) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return AdminAcl::check($token, $resource, AdminAcl::READ);
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a new instance of the transformer and pass along the currently
|
||||
* set API key.
|
||||
*
|
||||
* @return \Pterodactyl\Transformers\Api\Application\BaseTransformer
|
||||
* @template T of \Pterodactyl\Transformers\Api\Application\BaseTransformer
|
||||
*
|
||||
* @param class-string<T> $abstract
|
||||
*
|
||||
* @return T
|
||||
*
|
||||
* @throws \Pterodactyl\Exceptions\Transformer\InvalidTransformerLevelException
|
||||
*
|
||||
* @noinspection PhpUndefinedClassInspection
|
||||
* @noinspection PhpDocSignatureInspection
|
||||
*/
|
||||
protected function makeTransformer(string $abstract, array $parameters = [])
|
||||
protected function makeTransformer(string $abstract)
|
||||
{
|
||||
/** @var \Pterodactyl\Transformers\Api\Application\BaseTransformer $transformer */
|
||||
$transformer = Container::getInstance()->makeWith($abstract, $parameters);
|
||||
$transformer->setKey($this->getKey());
|
||||
Assert::subclassOf($abstract, self::class);
|
||||
|
||||
if (!$transformer instanceof self) {
|
||||
throw new InvalidTransformerLevelException('Calls to ' . __METHOD__ . ' must return a transformer that is an instance of ' . __CLASS__);
|
||||
}
|
||||
|
||||
return $transformer;
|
||||
return $abstract::fromRequest($this->request);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -5,31 +5,16 @@ namespace Pterodactyl\Transformers\Api\Client;
|
|||
use Pterodactyl\Models\User;
|
||||
use Webmozart\Assert\Assert;
|
||||
use Pterodactyl\Models\Server;
|
||||
use Illuminate\Container\Container;
|
||||
use Pterodactyl\Exceptions\Transformer\InvalidTransformerLevelException;
|
||||
use Pterodactyl\Transformers\Api\Application\BaseTransformer as BaseApplicationTransformer;
|
||||
|
||||
abstract class BaseClientTransformer extends BaseApplicationTransformer
|
||||
{
|
||||
/**
|
||||
* @var \Pterodactyl\Models\User
|
||||
*/
|
||||
private $user;
|
||||
|
||||
/**
|
||||
* Return the user model of the user requesting this transformation.
|
||||
*/
|
||||
public function getUser(): User
|
||||
{
|
||||
return $this->user;
|
||||
}
|
||||
|
||||
/**
|
||||
* Set the user model of the user requesting this transformation.
|
||||
*/
|
||||
public function setUser(User $user)
|
||||
{
|
||||
$this->user = $user;
|
||||
return $this->request->user();
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -37,33 +22,22 @@ abstract class BaseClientTransformer extends BaseApplicationTransformer
|
|||
* to access a different resource. This is used when including other
|
||||
* models on a transformation request.
|
||||
*
|
||||
* @param \Pterodactyl\Models\Server $server
|
||||
* @noinspection PhpParameterNameChangedDuringInheritanceInspection
|
||||
*/
|
||||
protected function authorize(string $ability, Server $server = null): bool
|
||||
{
|
||||
Assert::isInstanceOf($server, Server::class);
|
||||
|
||||
return $this->getUser()->can($ability, [$server]);
|
||||
return $this->request->user()->can($ability, [$server]);
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a new instance of the transformer and pass along the currently
|
||||
* set API key.
|
||||
*
|
||||
* @return self
|
||||
*
|
||||
* @throws \Pterodactyl\Exceptions\Transformer\InvalidTransformerLevelException
|
||||
* {@inheritDoc}
|
||||
*/
|
||||
protected function makeTransformer(string $abstract, array $parameters = [])
|
||||
protected function makeTransformer(string $abstract)
|
||||
{
|
||||
/** @var \Pterodactyl\Transformers\Api\Application\BaseTransformer $transformer */
|
||||
$transformer = Container::getInstance()->makeWith($abstract, $parameters);
|
||||
$transformer->setKey($this->getKey());
|
||||
Assert::subclassOf($abstract, self::class);
|
||||
|
||||
if (!$transformer instanceof self) {
|
||||
throw new InvalidTransformerLevelException('Calls to ' . __METHOD__ . ' must return a transformer that is an instance of ' . __CLASS__);
|
||||
}
|
||||
|
||||
return $transformer;
|
||||
return parent::makeTransformer($abstract);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -59,7 +59,7 @@ class DatabaseTransformer extends BaseClientTransformer
|
|||
*/
|
||||
public function includePassword(Database $database)
|
||||
{
|
||||
if (!$this->getUser()->can(Permission::ACTION_DATABASE_VIEW_PASSWORD, $database->server)) {
|
||||
if (!$this->request->user()->can(Permission::ACTION_DATABASE_VIEW_PASSWORD, $database->server)) {
|
||||
return $this->null();
|
||||
}
|
||||
|
||||
|
|
|
@ -34,8 +34,10 @@ class ServerTransformer extends BaseClientTransformer
|
|||
/** @var \Pterodactyl\Services\Servers\StartupCommandService $service */
|
||||
$service = Container::getInstance()->make(StartupCommandService::class);
|
||||
|
||||
$user = $this->request->user();
|
||||
|
||||
return [
|
||||
'server_owner' => $this->getKey()->user_id === $server->owner_id,
|
||||
'server_owner' => $user->id === $server->owner_id,
|
||||
'identifier' => $server->uuidShort,
|
||||
'internal_id' => $server->id,
|
||||
'uuid' => $server->uuid,
|
||||
|
@ -55,7 +57,7 @@ class ServerTransformer extends BaseClientTransformer
|
|||
'threads' => $server->threads,
|
||||
'oom_disabled' => $server->oom_disabled,
|
||||
],
|
||||
'invocation' => $service->handle($server, !$this->getUser()->can(Permission::ACTION_STARTUP_READ, $server)),
|
||||
'invocation' => $service->handle($server, !$user->can(Permission::ACTION_STARTUP_READ, $server)),
|
||||
'docker_image' => $server->image,
|
||||
'egg_features' => $server->egg->inherit_features,
|
||||
'feature_limits' => [
|
||||
|
@ -75,7 +77,7 @@ class ServerTransformer extends BaseClientTransformer
|
|||
/**
|
||||
* Returns the allocations associated with this server.
|
||||
*
|
||||
* @return \League\Fractal\Resource\Collection|\League\Fractal\Resource\NullResource
|
||||
* @return \League\Fractal\Resource\Collection
|
||||
*
|
||||
* @throws \Pterodactyl\Exceptions\Transformer\InvalidTransformerLevelException
|
||||
*/
|
||||
|
@ -83,6 +85,7 @@ class ServerTransformer extends BaseClientTransformer
|
|||
{
|
||||
$transformer = $this->makeTransformer(AllocationTransformer::class);
|
||||
|
||||
$user = $this->request->user();
|
||||
// While we include this permission, we do need to actually handle it slightly different here
|
||||
// for the purpose of keeping things functionally working. If the user doesn't have read permissions
|
||||
// for the allocations we'll only return the primary server allocation, and any notes associated
|
||||
|
@ -90,7 +93,7 @@ class ServerTransformer extends BaseClientTransformer
|
|||
//
|
||||
// This allows us to avoid too much permission regression, without also hiding information that
|
||||
// is generally needed for the frontend to make sense when browsing or searching results.
|
||||
if (!$this->getUser()->can(Permission::ACTION_ALLOCATION_READ, $server)) {
|
||||
if (!$user->can(Permission::ACTION_ALLOCATION_READ, $server)) {
|
||||
$primary = clone $server->allocation;
|
||||
$primary->notes = null;
|
||||
|
||||
|
@ -107,7 +110,7 @@ class ServerTransformer extends BaseClientTransformer
|
|||
*/
|
||||
public function includeVariables(Server $server)
|
||||
{
|
||||
if (!$this->getUser()->can(Permission::ACTION_STARTUP_READ, $server)) {
|
||||
if (!$this->request->user()->can(Permission::ACTION_STARTUP_READ, $server)) {
|
||||
return $this->null();
|
||||
}
|
||||
|
||||
|
@ -139,7 +142,7 @@ class ServerTransformer extends BaseClientTransformer
|
|||
*/
|
||||
public function includeSubusers(Server $server)
|
||||
{
|
||||
if (!$this->getUser()->can(Permission::ACTION_USER_READ, $server)) {
|
||||
if (!$this->request->user()->can(Permission::ACTION_USER_READ, $server)) {
|
||||
return $this->null();
|
||||
}
|
||||
|
||||
|
|
|
@ -3,6 +3,7 @@
|
|||
namespace Pterodactyl\Tests\Integration\Api\Application;
|
||||
|
||||
use Pterodactyl\Models\User;
|
||||
use Illuminate\Http\Request;
|
||||
use PHPUnit\Framework\Assert;
|
||||
use Pterodactyl\Models\ApiKey;
|
||||
use Pterodactyl\Services\Acl\Api\AdminAcl;
|
||||
|
@ -110,9 +111,12 @@ abstract class ApplicationApiIntegrationTestCase extends IntegrationTestCase
|
|||
*/
|
||||
protected function getTransformer(string $abstract): BaseTransformer
|
||||
{
|
||||
/** @var \Pterodactyl\Transformers\Api\Application\BaseTransformer $transformer */
|
||||
$transformer = $this->app->make($abstract);
|
||||
$transformer->setKey($this->getApiKey());
|
||||
$request = Request::createFromGlobals();
|
||||
$request->setUserResolver(function () {
|
||||
return $this->getApiKey()->user;
|
||||
});
|
||||
|
||||
$transformer = $abstract::fromRequest($request);
|
||||
|
||||
Assert::assertInstanceOf(BaseTransformer::class, $transformer);
|
||||
Assert::assertNotInstanceOf(BaseClientTransformer::class, $transformer);
|
||||
|
|
|
@ -1,73 +0,0 @@
|
|||
<?php
|
||||
|
||||
namespace Pterodactyl\Tests\Unit\Http\Middleware\Api;
|
||||
|
||||
use Pterodactyl\Models\ApiKey;
|
||||
use Pterodactyl\Http\Middleware\Api\AuthenticateIPAccess;
|
||||
use Pterodactyl\Tests\Unit\Http\Middleware\MiddlewareTestCase;
|
||||
use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
|
||||
|
||||
class AuthenticateIPAccessTest extends MiddlewareTestCase
|
||||
{
|
||||
/**
|
||||
* Test middleware when there are no IP restrictions.
|
||||
*/
|
||||
public function testWithNoIPRestrictions()
|
||||
{
|
||||
$model = ApiKey::factory()->make(['allowed_ips' => []]);
|
||||
$this->setRequestAttribute('api_key', $model);
|
||||
|
||||
$this->getMiddleware()->handle($this->request, $this->getClosureAssertions());
|
||||
}
|
||||
|
||||
/**
|
||||
* Test middleware works correctly when a valid IP accesses
|
||||
* and there is an IP restriction.
|
||||
*/
|
||||
public function testWithValidIP()
|
||||
{
|
||||
$model = ApiKey::factory()->make(['allowed_ips' => ['127.0.0.1']]);
|
||||
$this->setRequestAttribute('api_key', $model);
|
||||
|
||||
$this->request->shouldReceive('ip')->withNoArgs()->once()->andReturn('127.0.0.1');
|
||||
|
||||
$this->getMiddleware()->handle($this->request, $this->getClosureAssertions());
|
||||
}
|
||||
|
||||
/**
|
||||
* Test that a CIDR range can be used.
|
||||
*/
|
||||
public function testValidIPAgainstCIDRRange()
|
||||
{
|
||||
$model = ApiKey::factory()->make(['allowed_ips' => ['192.168.1.1/28']]);
|
||||
$this->setRequestAttribute('api_key', $model);
|
||||
|
||||
$this->request->shouldReceive('ip')->withNoArgs()->once()->andReturn('192.168.1.15');
|
||||
|
||||
$this->getMiddleware()->handle($this->request, $this->getClosureAssertions());
|
||||
}
|
||||
|
||||
/**
|
||||
* Test that an exception is thrown when an invalid IP address
|
||||
* tries to connect and there is an IP restriction.
|
||||
*/
|
||||
public function testWithInvalidIP()
|
||||
{
|
||||
$this->expectException(AccessDeniedHttpException::class);
|
||||
|
||||
$model = ApiKey::factory()->make(['allowed_ips' => ['127.0.0.1']]);
|
||||
$this->setRequestAttribute('api_key', $model);
|
||||
|
||||
$this->request->shouldReceive('ip')->withNoArgs()->twice()->andReturn('127.0.0.2');
|
||||
|
||||
$this->getMiddleware()->handle($this->request, $this->getClosureAssertions());
|
||||
}
|
||||
|
||||
/**
|
||||
* Return an instance of the middleware to be used when testing.
|
||||
*/
|
||||
private function getMiddleware(): AuthenticateIPAccess
|
||||
{
|
||||
return new AuthenticateIPAccess();
|
||||
}
|
||||
}
|
Loading…
Reference in a new issue