Merge branch 'develop' into permissions

DaneEveritt · web-flow · commit 802f88fc7862 · 2020-11-08T11:47:45.000-08:00
diff --git a/app/Http/Controllers/Api/Client/Servers/FileController.php b/app/Http/Controllers/Api/Client/Servers/FileController.php
@@ -6,6 +6,7 @@
 use Illuminate\Http\Response;
 use Pterodactyl\Models\Server;
 use Illuminate\Http\JsonResponse;
+use Illuminate\Support\Collection;
 use Pterodactyl\Services\Nodes\NodeJWTService;
 use Illuminate\Contracts\Routing\ResponseFactory;
 use Pterodactyl\Repositories\Wings\DaemonFileRepository;
@@ -70,7 +71,7 @@ public function directory(ListFilesRequest $request, Server $server): array
     {
         $contents = $this->fileRepository
             ->setServer($server)
-            ->getDirectory(urlencode(urldecode($request->get('directory') ?? '/')));
+            ->getDirectory($this->encode($request->get('directory') ?? '/'));
 
         return $this->fractal->collection($contents)
             ->transformWith($this->getTransformer(FileObjectTransformer::class))
@@ -91,7 +92,7 @@ public function contents(GetFileContentsRequest $request, Server $server): Respo
     {
         return new Response(
             $this->fileRepository->setServer($server)->getContent(
-                urlencode(urldecode($request->get('file'))), config('pterodactyl.files.max_edit_size')
+                $this->encode($request->get('file')), config('pterodactyl.files.max_edit_size')
             ),
             Response::HTTP_OK,
             ['Content-Type' => 'text/plain']
@@ -113,7 +114,7 @@ public function download(GetFileContentsRequest $request, Server $server)
         $token = $this->jwtService
             ->setExpiresAt(CarbonImmutable::now()->addMinutes(15))
             ->setClaims([
-                'file_path' => $request->get('file'),
+                'file_path' => rawurldecode($request->get('file')),
                 'server_uuid' => $server->uuid,
             ])
             ->handle($server->node, $request->user()->id . $server->uuid);
@@ -142,7 +143,7 @@ public function download(GetFileContentsRequest $request, Server $server)
     public function write(WriteFileContentRequest $request, Server $server): JsonResponse
     {
         $this->fileRepository->setServer($server)->putContent(
-            $request->get('file'),
+            $this->encode($request->get('file')),
             $request->getContent()
         );
 
@@ -261,4 +262,18 @@ public function delete(DeleteFileRequest $request, Server $server): JsonResponse
 
         return new JsonResponse([], Response::HTTP_NO_CONTENT);
     }
+
+    /**
+     * Encodes a given file name & path in a format that should work for a good majority
+     * of file names without too much confusing logic.
+     *
+     * @param string $path
+     * @return string
+     */
+    private function encode(string $path): string
+    {
+        return Collection::make(explode('/', rawurldecode($path)))->map(function ($value) {
+            return rawurlencode($value);
+        })->join('/');
+    }
 }
diff --git a/app/Http/Controllers/Api/Client/Servers/SubuserController.php b/app/Http/Controllers/Api/Client/Servers/SubuserController.php
@@ -3,15 +3,16 @@
 namespace Pterodactyl\Http\Controllers\Api\Client\Servers;
 
 use Illuminate\Http\Request;
-use Pterodactyl\Models\User;
 use Pterodactyl\Models\Server;
-use Pterodactyl\Models\Subuser;
 use Illuminate\Http\JsonResponse;
 use Pterodactyl\Models\Permission;
+use Illuminate\Support\Facades\Log;
 use Pterodactyl\Repositories\Eloquent\SubuserRepository;
 use Pterodactyl\Services\Subusers\SubuserCreationService;
+use Pterodactyl\Repositories\Wings\DaemonServerRepository;
 use Pterodactyl\Transformers\Api\Client\SubuserTransformer;
 use Pterodactyl\Http\Controllers\Api\Client\ClientApiController;
+use Pterodactyl\Exceptions\Http\Connection\DaemonConnectionException;
 use Pterodactyl\Http\Requests\Api\Client\Servers\Subusers\GetSubuserRequest;
 use Pterodactyl\Http\Requests\Api\Client\Servers\Subusers\StoreSubuserRequest;
 use Pterodactyl\Http\Requests\Api\Client\Servers\Subusers\DeleteSubuserRequest;
@@ -29,20 +30,28 @@ class SubuserController extends ClientApiController
      */
     private $creationService;
 
+    /**
+     * @var \Pterodactyl\Repositories\Wings\DaemonServerRepository
+     */
+    private $serverRepository;
+
     /**
      * SubuserController constructor.
      *
      * @param \Pterodactyl\Repositories\Eloquent\SubuserRepository $repository
      * @param \Pterodactyl\Services\Subusers\SubuserCreationService $creationService
+     * @param \Pterodactyl\Repositories\Wings\DaemonServerRepository $serverRepository
      */
     public function __construct(
         SubuserRepository $repository,
-        SubuserCreationService $creationService
+        SubuserCreationService $creationService,
+        DaemonServerRepository $serverRepository
     ) {
         parent::__construct();
 
         $this->repository = $repository;
         $this->creationService = $creationService;
+        $this->serverRepository = $serverRepository;
     }
 
     /**
@@ -101,19 +110,38 @@ public function store(StoreSubuserRequest $request, Server $server)
      * Update a given subuser in the system for the server.
      *
      * @param \Pterodactyl\Http\Requests\Api\Client\Servers\Subusers\UpdateSubuserRequest $request
+     * @param \Pterodactyl\Models\Server $server
      * @return array
      *
      * @throws \Pterodactyl\Exceptions\Model\DataValidationException
      * @throws \Pterodactyl\Exceptions\Repository\RecordNotFoundException
      */
-    public function update(UpdateSubuserRequest $request): array
+    public function update(UpdateSubuserRequest $request, Server $server): array
     {
         /** @var \Pterodactyl\Models\Subuser $subuser */
         $subuser = $request->attributes->get('subuser');
 
-        $this->repository->update($subuser->id, [
-            'permissions' => $this->getDefaultPermissions($request),
-        ]);
+        $permissions = $this->getDefaultPermissions($request);
+        $current = $subuser->permissions;
+
+        sort($permissions);
+        sort($current);
+
+        // Only update the database and hit up the Wings instance to invalidate JTI's if the permissions
+        // have actually changed for the user.
+        if ($permissions !== $current) {
+            $this->repository->update($subuser->id, [
+                'permissions' => $this->getDefaultPermissions($request),
+            ]);
+
+            try {
+                $this->serverRepository->setServer($server)->revokeJTIs([md5($subuser->user_id . $server->uuid)]);
+            } catch (DaemonConnectionException $exception) {
+                // Don't block this request if we can't connect to the Wings instance. Chances are it is
+                // offline in this event and the token will be invalid anyways once Wings boots back.
+                Log::warning($exception, ['user_id' => $subuser->user_id, 'server_id' => $server->id]);
+            }
+        }
 
         return $this->fractal->item($subuser->refresh())
             ->transformWith($this->getTransformer(SubuserTransformer::class))
@@ -124,15 +152,23 @@ public function update(UpdateSubuserRequest $request): array
      * Removes a subusers from a server's assignment.
      *
      * @param \Pterodactyl\Http\Requests\Api\Client\Servers\Subusers\DeleteSubuserRequest $request
+     * @param \Pterodactyl\Models\Server $server
      * @return \Illuminate\Http\JsonResponse
      */
-    public function delete(DeleteSubuserRequest $request)
+    public function delete(DeleteSubuserRequest $request, Server $server)
     {
         /** @var \Pterodactyl\Models\Subuser $subuser */
         $subuser = $request->attributes->get('subuser');
 
         $this->repository->delete($subuser->id);
 
+        try {
+            $this->serverRepository->setServer($server)->revokeJTIs([md5($subuser->user_id . $server->uuid)]);
+        } catch (DaemonConnectionException $exception) {
+            // Don't block this request if we can't connect to the Wings instance.
+            Log::warning($exception, ['user_id' => $subuser->user_id, 'server_id' => $server->id]);
+        }
+
         return new JsonResponse([], JsonResponse::HTTP_NO_CONTENT);
     }
 
diff --git a/app/Http/Controllers/Api/Client/Servers/WebsocketController.php b/app/Http/Controllers/Api/Client/Servers/WebsocketController.php
@@ -59,7 +59,7 @@ public function __invoke(ClientApiRequest $request, Server $server)
         }
 
         $token = $this->jwtService
-            ->setExpiresAt(CarbonImmutable::now()->addMinutes(15))
+            ->setExpiresAt(CarbonImmutable::now()->addMinutes(10))
             ->setClaims([
                 'user_id' => $request->user()->id,
                 'server_uuid' => $server->uuid,
diff --git a/app/Models/Permission.php b/app/Models/Permission.php
@@ -163,7 +163,7 @@ class Permission extends Model
         'allocation' => [
             'description' => 'Permissions that control a user\'s ability to modify the port allocations for this server.',
             'keys' => [
-                'read' => 'Allows a user to view the allocations assigned to this server.',
+                'read' => 'Allows a user to view all allocations currently assigned to this server. Users with any level of access to this server can always view the primary allocation.',
                 'create' => 'Allows a user to assign additional allocations to the server.',
                 'update' => 'Allows a user to change the primary server allocation and attach notes to each allocation.',
                 'delete' => 'Allows a user to delete an allocation from the server.',
diff --git a/app/Repositories/Wings/DaemonServerRepository.php b/app/Repositories/Wings/DaemonServerRepository.php
@@ -126,11 +126,10 @@ public function suspend(bool $unsuspend = false): void
     }
 
     /**
-     * Requests the daemon to create a full archive of the server.
-     * Once the daemon is finished they will send a POST request to
-     * "/api/remote/servers/{uuid}/archive" with a boolean.
+     * Requests the daemon to create a full archive of the server. Once the daemon is finished
+     * they will send a POST request to "/api/remote/servers/{uuid}/archive" with a boolean.
      *
-     * @throws DaemonConnectionException
+     * @throws \Pterodactyl\Exceptions\Http\Connection\DaemonConnectionException
      */
     public function requestArchive(): void
     {
@@ -144,4 +143,25 @@ public function requestArchive(): void
             throw new DaemonConnectionException($exception);
         }
     }
+
+    /**
+     * Revokes an array of JWT JTI's by marking any token generated before the current time on
+     * the Wings instance as being invalid.
+     *
+     * @param array $jtis
+     * @throws \Pterodactyl\Exceptions\Http\Connection\DaemonConnectionException
+     */
+    public function revokeJTIs(array $jtis): void
+    {
+        Assert::isInstanceOf($this->server, Server::class);
+
+        try {
+            $this->getHttpClient()
+                ->post(sprintf('/api/servers/%s/ws/deny', $this->server->uuid), [
+                    'json' => ['jtis' => $jtis],
+                ]);
+        } catch (TransferException $exception) {
+            throw new DaemonConnectionException($exception);
+        }
+    }
 }
diff --git a/app/Services/Nodes/NodeJWTService.php b/app/Services/Nodes/NodeJWTService.php
@@ -55,7 +55,7 @@ public function handle(Node $node, string $identifiedBy)
 
         $builder = (new Builder)->issuedBy(config('app.url'))
             ->permittedFor($node->getConnectionAddress())
-            ->identifiedBy(hash('sha256', $identifiedBy), true)
+            ->identifiedBy(md5($identifiedBy), true)
             ->issuedAt(CarbonImmutable::now()->getTimestamp())
             ->canOnlyBeUsedAfter(CarbonImmutable::now()->subMinutes(5)->getTimestamp());
 
diff --git a/app/Transformers/Api/Client/ServerTransformer.php b/app/Transformers/Api/Client/ServerTransformer.php
@@ -83,15 +83,23 @@ public function transform(Server $server): array
      */
     public function includeAllocations(Server $server)
     {
+        $transformer = $this->makeTransformer(AllocationTransformer::class);
+
+        // 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
+        // with it will be hidden.
+        //
+        // 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)) {
-            return $this->null();
+            $primary = clone $server->allocation;
+            $primary->notes = null;
+
+            return $this->collection([$primary], $transformer, Allocation::RESOURCE_NAME);
         }
 
-        return $this->collection(
-            $server->allocations,
-            $this->makeTransformer(AllocationTransformer::class),
-            Allocation::RESOURCE_NAME
-        );
+        return $this->collection($server->allocations, $transformer, Allocation::RESOURCE_NAME);
     }
 
     /**
diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh
@@ -20,10 +20,15 @@ else
   touch /app/var/.env
 
   ## manually generate a key because key generate --force fails
-  echo -e "Generating key."
-  APP_KEY=$(cat /dev/urandom | tr -dc 'a-zA-Z0-9' | fold -w 32 | head -n 1)
-  echo -e "Generated app key: $APP_KEY"
-  echo -e "APP_KEY=$APP_KEY" > /app/var/.env
+  if [ -z $APP_KEY ]; then
+     echo -e "Generating key."
+     APP_KEY=$(cat /dev/urandom | tr -dc 'a-zA-Z0-9' | fold -w 32 | head -n 1)
+     echo -e "Generated app key: $APP_KEY"
+     echo -e "APP_KEY=$APP_KEY" > /app/var/.env
+  else
+    echo -e "APP_KEY exists in environment, using that."
+    echo -e "APP_KEY=$APP_KEY" > /app/var/.env
+  fi
 
   ln -s /app/var/.env /app/
 fi
@@ -77,4 +82,4 @@ yarn add cross-env
 yarn run build:production
 
 echo -e "Starting supervisord."
-exec "$@"
+exec "$@"
diff --git a/resources/scripts/api/server/files/getFileContents.ts b/resources/scripts/api/server/files/getFileContents.ts
@@ -3,7 +3,7 @@ import http from '@/api/http';
 export default (server: string, file: string): Promise<string> => {
     return new Promise((resolve, reject) => {
         http.get(`/api/client/servers/${server}/files/contents`, {
-            params: { file: file.split('/').map(item => encodeURIComponent(item)).join('/') },
+            params: { file: encodeURI(decodeURI(file)) },
             transformResponse: res => res,
             responseType: 'text',
         })
diff --git a/resources/scripts/api/server/files/loadDirectory.ts b/resources/scripts/api/server/files/loadDirectory.ts
@@ -17,7 +17,7 @@ export interface FileObject {
 
 export default async (uuid: string, directory?: string): Promise<FileObject[]> => {
     const { data } = await http.get(`/api/client/servers/${uuid}/files/list`, {
-        params: { directory: directory?.split('/').map(item => encodeURIComponent(item)).join('/') },
+        params: { directory: encodeURI(directory ?? '/') },
     });
 
     return (data.data || []).map(rawDataToFileObject);
diff --git a/resources/scripts/api/server/files/saveFileContents.ts b/resources/scripts/api/server/files/saveFileContents.ts
@@ -2,7 +2,7 @@ import http from '@/api/http';
 
 export default async (uuid: string, file: string, content: string): Promise<void> => {
     await http.post(`/api/client/servers/${uuid}/files/write`, content, {
-        params: { file },
+        params: { file: encodeURI(decodeURI(file)) },
         headers: {
             'Content-Type': 'text/plain',
         },
diff --git a/resources/scripts/components/server/WebsocketHandler.tsx b/resources/scripts/components/server/WebsocketHandler.tsx
@@ -7,6 +7,11 @@ import { CSSTransition } from 'react-transition-group';
 import Spinner from '@/components/elements/Spinner';
 import tw from 'twin.macro';
 
+const reconnectErrors = [
+    'jwt: exp claim is invalid',
+    'jwt: created too far in past (denylist)',
+];
+
 export default () => {
     let updatingToken = false;
     const [ error, setError ] = useState<'connecting' | string>('');
@@ -64,7 +69,7 @@ export default () => {
             setConnectionState(false);
             console.warn('JWT validation error from wings:', error);
 
-            if (error === 'jwt: exp claim is invalid') {
+            if (reconnectErrors.find(v => error.toLowerCase().indexOf(v) >= 0)) {
                 updateToken(uuid, socket);
             } else {
                 setError('There was an error validating the credentials provided for the websocket. Please refresh the page.');
@@ -95,7 +100,7 @@ export default () => {
                                 </p>
                             </>
                             :
-                            <p css={tw`ml-2 text-sm text-red-100`}>
+                            <p css={tw`ml-2 text-sm text-white`}>
                                 {error}
                             </p>
                         }
diff --git a/resources/scripts/components/server/files/FileEditContainer.tsx b/resources/scripts/components/server/files/FileEditContainer.tsx
@@ -61,7 +61,7 @@ export default () => {
         setLoading(true);
         clearFlashes('files:view');
         fetchFileContent()
-            .then(content => saveFileContents(uuid, encodeURIComponent(name || hash.replace(/^#/, '')), content))
+            .then(content => saveFileContents(uuid, name || hash.replace(/^#/, ''), content))
             .then(() => {
                 if (name) {
                     history.push(`/server/${id}/files/edit#/${name}`);
diff --git a/resources/scripts/components/server/files/FileManagerBreadcrumbs.tsx b/resources/scripts/components/server/files/FileManagerBreadcrumbs.tsx
@@ -33,10 +33,10 @@ export default ({ withinFileEditor, isNewFile }: Props) => {
         .filter(directory => !!directory)
         .map((directory, index, dirs) => {
             if (!withinFileEditor && index === dirs.length - 1) {
-                return { name: decodeURIComponent(encodeURIComponent(directory)) };
+                return { name: directory };
             }
 
-            return { name: decodeURIComponent(encodeURIComponent(directory)), path: `/${dirs.slice(0, index + 1).join('/')}` };
+            return { name: directory, path: `/${dirs.slice(0, index + 1).join('/')}` };
         });
 
     const onSelectAllClick = (e: React.ChangeEvent<HTMLInputElement>) => {
@@ -79,7 +79,7 @@ export default ({ withinFileEditor, isNewFile }: Props) => {
             }
             {file &&
             <React.Fragment>
-                <span css={tw`px-1 text-neutral-300`}>{decodeURIComponent(encodeURIComponent(file))}</span>
+                <span css={tw`px-1 text-neutral-300`}>{decodeURI(file)}</span>
             </React.Fragment>
             }
         </div>
diff --git a/resources/scripts/components/server/files/FileManagerContainer.tsx b/resources/scripts/components/server/files/FileManagerContainer.tsx
diff --git a/resources/scripts/components/server/files/FileObjectRow.tsx b/resources/scripts/components/server/files/FileObjectRow.tsx
diff --git a/resources/scripts/components/server/files/NewDirectoryButton.tsx b/resources/scripts/components/server/files/NewDirectoryButton.tsx
diff --git a/tests/Integration/Api/Client/ClientControllerTest.php b/tests/Integration/Api/Client/ClientControllerTest.php
diff --git a/tests/Integration/Api/Client/Server/Subuser/DeleteSubuserTest.php b/tests/Integration/Api/Client/Server/Subuser/DeleteSubuserTest.php
diff --git a/tests/Integration/Api/Client/Server/WebsocketControllerTest.php b/tests/Integration/Api/Client/Server/WebsocketControllerTest.php
diff --git a/tests/Integration/Services/Servers/BuildModificationServiceTest.php b/tests/Integration/Services/Servers/BuildModificationServiceTest.php

Original file line number	Diff line number	Diff line change
`@@ -59,7 +59,7 @@ public function __invoke(ClientApiRequest $request, Server $server)`
`59`	`59`	`}`
`60`	`60`
`61`	`61`	`$token = $this->jwtService`
`62`		`- ->setExpiresAt(CarbonImmutable::now()->addMinutes(15))`
	`62`	`+ ->setExpiresAt(CarbonImmutable::now()->addMinutes(10))`
`63`	`63`	`->setClaims([`
`64`	`64`	`'user_id' => $request->user()->id,`
`65`	`65`	`'server_uuid' => $server->uuid,`