diff --git a/app/Http/Controllers/Api/Client/ClientController.php b/app/Http/Controllers/Api/Client/ClientController.php index 300770aa..5eec40b5 100644 --- a/app/Http/Controllers/Api/Client/ClientController.php +++ b/app/Http/Controllers/Api/Client/ClientController.php @@ -2,7 +2,6 @@ namespace Pterodactyl\Http\Controllers\Api\Client; -use Pterodactyl\Models\User; use Pterodactyl\Models\Server; use Pterodactyl\Models\Permission; use Spatie\QueryBuilder\QueryBuilder; @@ -39,31 +38,27 @@ class ClientController extends ClientApiController public function index(GetServersRequest $request): array { $user = $request->user(); - $level = $request->getFilterLevel(); $transformer = $this->getTransformer(ServerTransformer::class); // Start the query builder and ensure we eager load any requested relationships from the request. - $builder = Server::query()->with($this->getIncludesForTransformer($transformer, ['node'])); + $builder = QueryBuilder::for( + Server::query()->with($this->getIncludesForTransformer($transformer, ['node'])) + )->allowedFilters('uuid', 'name', 'external_id'); - if ($level === User::FILTER_LEVEL_OWNER) { - $builder = $builder->where('owner_id', $request->user()->id); - } - // If set to all, display all servers they can access, including those they access as an - // admin. If set to subuser, only return the servers they can access because they are owner, - // or marked as a subuser of the server. - elseif (($level === User::FILTER_LEVEL_ALL && ! $user->root_admin) || $level === User::FILTER_LEVEL_SUBUSER) { + // Either return all of the servers the user has access to because they are an admin `?type=admin` or + // just return all of the servers the user has access to because they are the owner or a subuser of the + // server. + if ($request->input('type') === 'admin') { + $builder = $user->root_admin + ? $builder->whereNotIn('id', $user->accessibleServers()->pluck('id')->all()) + // If they aren't an admin but want all the admin servers don't fail the request, just + // make it a query that will never return any results back. + : $builder->whereRaw('1 = 2'); + } elseif ($request->input('type') === 'owner') { + $builder = $builder->where('owner_id', $user->id); + } else { $builder = $builder->whereIn('id', $user->accessibleServers()->pluck('id')->all()); } - // If set to admin, only display the servers a user can access because they are an administrator. - // This means only servers the user would not have access to if they were not an admin (because they - // are not an owner or subuser) are returned. - elseif ($level === User::FILTER_LEVEL_ADMIN && $user->root_admin) { - $builder = $builder->whereNotIn('id', $user->accessibleServers()->pluck('id')->all()); - } - - $builder = QueryBuilder::for($builder)->allowedFilters( - 'uuid', 'name', 'external_id' - ); $servers = $builder->paginate(min($request->query('per_page', 50), 100))->appends($request->query()); diff --git a/app/Http/Requests/Api/Client/GetServersRequest.php b/app/Http/Requests/Api/Client/GetServersRequest.php index c28f0a94..9b4601f2 100644 --- a/app/Http/Requests/Api/Client/GetServersRequest.php +++ b/app/Http/Requests/Api/Client/GetServersRequest.php @@ -2,8 +2,6 @@ namespace Pterodactyl\Http\Requests\Api\Client; -use Pterodactyl\Models\User; - class GetServersRequest extends ClientApiRequest { /** @@ -13,28 +11,4 @@ class GetServersRequest extends ClientApiRequest { return true; } - - /** - * Return the filtering method for servers when the client base endpoint is requested. - * - * @return int - */ - public function getFilterLevel(): int - { - switch ($this->input('type')) { - case 'all': - return User::FILTER_LEVEL_ALL; - break; - case 'admin': - return User::FILTER_LEVEL_ADMIN; - break; - case 'owner': - return User::FILTER_LEVEL_OWNER; - break; - case 'subuser-of': - default: - return User::FILTER_LEVEL_SUBUSER; - break; - } - } } diff --git a/app/Models/User.php b/app/Models/User.php index baff65b6..39954fbf 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -57,11 +57,6 @@ class User extends Model implements const USER_LEVEL_USER = 0; const USER_LEVEL_ADMIN = 1; - const FILTER_LEVEL_ALL = 0; - const FILTER_LEVEL_OWNER = 1; - const FILTER_LEVEL_ADMIN = 2; - const FILTER_LEVEL_SUBUSER = 3; - /** * The resource name for this model when it is transformed into an * API representation using fractal. diff --git a/resources/scripts/api/getServers.ts b/resources/scripts/api/getServers.ts index 492898f7..63329bfa 100644 --- a/resources/scripts/api/getServers.ts +++ b/resources/scripts/api/getServers.ts @@ -4,14 +4,14 @@ import http, { getPaginationSet, PaginatedResult } from '@/api/http'; interface QueryParams { query?: string; page?: number; - includeAdmin?: boolean; + onlyAdmin?: boolean; } -export default ({ query, page = 1, includeAdmin = false }: QueryParams): Promise> => { +export default ({ query, page = 1, onlyAdmin = false }: QueryParams): Promise> => { return new Promise((resolve, reject) => { http.get('/api/client', { params: { - type: includeAdmin ? 'all' : undefined, + type: onlyAdmin ? 'admin' : undefined, 'filter[name]': query, page, }, diff --git a/resources/scripts/components/dashboard/DashboardContainer.tsx b/resources/scripts/components/dashboard/DashboardContainer.tsx index 60c18cb1..a2047260 100644 --- a/resources/scripts/components/dashboard/DashboardContainer.tsx +++ b/resources/scripts/components/dashboard/DashboardContainer.tsx @@ -17,11 +17,11 @@ export default () => { const { clearFlashes, clearAndAddHttpError } = useFlash(); const [ page, setPage ] = useState(1); const { rootAdmin } = useStoreState(state => state.user.data!); - const [ includeAdmin, setIncludeAdmin ] = usePersistedState('show_all_servers', false); + const [ showOnlyAdmin, setShowOnlyAdmin ] = usePersistedState('show_all_servers', false); const { data: servers, error } = useSWR>( - [ '/api/client/servers', includeAdmin, page ], - () => getServers({ includeAdmin, page }), + [ '/api/client/servers', showOnlyAdmin, page ], + () => getServers({ onlyAdmin: showOnlyAdmin, page }), ); useEffect(() => { @@ -34,12 +34,12 @@ export default () => { {rootAdmin &&

- {includeAdmin ? 'Showing all servers' : 'Showing your servers'} + {showOnlyAdmin ? 'Showing other\'s servers' : 'Showing your servers'}

setIncludeAdmin(s => !s)} + defaultChecked={showOnlyAdmin} + onChange={() => setShowOnlyAdmin(s => !s)} />
} @@ -58,7 +58,11 @@ export default () => { )) :

- There are no servers associated with your account. + {showOnlyAdmin ? + 'There are no other servers to display.' + : + 'There are no servers associated with your account.' + }

)} diff --git a/tests/Integration/Api/Client/ClientControllerTest.php b/tests/Integration/Api/Client/ClientControllerTest.php index 82aeb564..b894b14b 100644 --- a/tests/Integration/Api/Client/ClientControllerTest.php +++ b/tests/Integration/Api/Client/ClientControllerTest.php @@ -38,33 +38,6 @@ class ClientControllerTest extends ClientApiIntegrationTestCase $response->assertJsonPath('meta.pagination.per_page', 50); } - /** - * Tests that all of the servers on the system are returned when making the request as an - * administrator and including the ?filter=all parameter in the URL. - */ - public function testFilterIncludeAllServersWhenAdministrator() - { - /** @var \Pterodactyl\Models\User[] $users */ - $users = factory(User::class)->times(3)->create(); - $users[0]->root_admin = true; - - $servers = [ - $this->createServerModel(['user_id' => $users[0]->id]), - $this->createServerModel(['user_id' => $users[1]->id]), - $this->createServerModel(['user_id' => $users[2]->id]), - ]; - - $response = $this->actingAs($users[0])->getJson('/api/client?type=all'); - - $response->assertOk(); - $response->assertJsonCount(3, 'data'); - - for ($i = 0; $i < 3; $i++) { - $response->assertJsonPath("data.{$i}.attributes.server_owner", $i === 0); - $response->assertJsonPath("data.{$i}.attributes.identifier", $servers[$i]->uuidShort); - } - } - /** * Test that servers where the user is a subuser are returned by default in the API call. */ @@ -143,4 +116,59 @@ class ClientControllerTest extends ClientApiIntegrationTestCase ], ]); } + + /** + * Test that only servers a user can access because they are an administrator are returned. This + * will always exclude any servers they can see because they're the owner or a subuser of the server. + */ + public function testOnlyAdminLevelServersAreReturned() + { + /** @var \Pterodactyl\Models\User[] $users */ + $users = factory(User::class)->times(4)->create(); + $users[0]->update(['root_admin' => true]); + + $servers = [ + $this->createServerModel(['user_id' => $users[0]->id]), + $this->createServerModel(['user_id' => $users[1]->id]), + $this->createServerModel(['user_id' => $users[2]->id]), + $this->createServerModel(['user_id' => $users[3]->id]), + ]; + + Subuser::query()->create([ + 'user_id' => $users[0]->id, + 'server_id' => $servers[1]->id, + 'permissions' => [Permission::ACTION_WEBSOCKET_CONNECT], + ]); + + // Only servers 2 & 3 (0 indexed) should be returned by the API at this point. The user making + // the request is the owner of server 0, and a subuser of server 1 so they should be exluded. + $response = $this->actingAs($users[0])->getJson('/api/client?type=admin'); + + $response->assertOk(); + $response->assertJsonCount(2, 'data'); + + $response->assertJsonPath('data.0.attributes.server_owner', false); + $response->assertJsonPath('data.0.attributes.identifier', $servers[2]->uuidShort); + $response->assertJsonPath('data.1.attributes.server_owner', false); + $response->assertJsonPath('data.1.attributes.identifier', $servers[3]->uuidShort); + } + + /** + * Test that no servers get returned if the user requests all admin level servers by using + * ?type=admin in the request. + */ + public function testNoServersAreReturnedIfAdminFilterIsPassedByRegularUser() + { + /** @var \Pterodactyl\Models\User[] $users */ + $users = factory(User::class)->times(3)->create(); + + $this->createServerModel(['user_id' => $users[0]->id]); + $this->createServerModel(['user_id' => $users[1]->id]); + $this->createServerModel(['user_id' => $users[2]->id]); + + $response = $this->actingAs($users[0])->getJson('/api/client?type=admin'); + + $response->assertOk(); + $response->assertJsonCount(0, 'data'); + } }