fix: Fix unit tests, change function parameter from sftp username to …

…panel username
pterodactyl · Feb 4, 2025 · 9ef9ea3 · 9ef9ea3
1 parent f5523e6
commit 9ef9ea3
Show file tree

Hide file tree

Showing 4 changed files with 9 additions and 7 deletions.
diff --git a/app/Http/Controllers/Api/Client/Servers/SubuserController.php b/app/Http/Controllers/Api/Client/Servers/SubuserController.php
@@ -117,8 +117,7 @@ public function update(UpdateSubuserRequest $request, Server $server): array
                 try {
                     $this->serverRepository->setServer($server)->revokeUserJTI($subuser->user_id);
                     if (in_array(Permission::ACTION_FILE_SFTP, $current) && !in_array(Permission::ACTION_FILE_SFTP, $permissions)) {
-                        $sftpUsername = $subuser->user->username . '.' . $server->uuidShort;
-                        $this->serverRepository->setServer($server)->disconnectSFTP($sftpUsername);
+                        $this->serverRepository->setServer($server)->disconnectSFTP($subuser->user->username);
                     }
                 } catch (DaemonConnectionException $exception) {
                     // Don't block this request if we can't connect to the Wings instance. Chances are it is
@@ -155,8 +154,7 @@ public function delete(DeleteSubuserRequest $request, Server $server): JsonRespo
 
             try {
                 $this->serverRepository->setServer($server)->revokeUserJTI($subuser->user_id);
-                $sftpUsername = $subuser->user->username . '.' . $server->uuidShort;
-                $this->serverRepository->setServer($server)->disconnectSFTP($sftpUsername);
+                $this->serverRepository->setServer($server)->disconnectSFTP($subuser->user->username);
             } 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]);

diff --git a/app/Repositories/Wings/DaemonServerRepository.php b/app/Repositories/Wings/DaemonServerRepository.php
@@ -164,11 +164,12 @@ protected function revokeJTIs(array $jtis): void
     public function disconnectSFTP(string $username): void
     {
         Assert::isInstanceOf($this->server, Server::class);
+        $sftpUsername = $username . '.' . $this->server->uuidShort;
 
         try {
             $this->getHttpClient()
                 ->delete(sprintf('/api/servers/%s/sftp/disconnect', $this->server->uuid), [
-                    'json' => ['username' => $username],
+                    'json' => ['username' => $sftpUsername],
                 ]);
         } catch (TransferException $exception) {
             throw new DaemonConnectionException($exception);

diff --git a/tests/Integration/Api/Client/Server/Subuser/DeleteSubuserTest.php b/tests/Integration/Api/Client/Server/Subuser/DeleteSubuserTest.php
@@ -36,7 +36,7 @@ public function testCorrectSubuserIsDeletedFromServer()
         $uuid = $differentUser->id . substr($real, strlen((string) $differentUser->id));
 
         /** @var User $subuser */
-        $subuser = User::factory()->create(['uuid' => $uuid]);
+        $subuser = User::factory()->create(['uuid' => $uuid, 'username' => 'test']);
 
         Subuser::query()->forceCreate([
             'user_id' => $subuser->id,
@@ -45,14 +45,15 @@ public function testCorrectSubuserIsDeletedFromServer()
         ]);
 
         $mock->expects('setServer->revokeUserJTI')->with($subuser->id)->andReturnUndefined();
+        $mock->expects('setServer->disconnectSFTP')->with($subuser->username)->andReturnUndefined();
 
         $this->actingAs($user)->deleteJson($this->link($server) . "/users/$subuser->uuid")->assertNoContent();
 
         // Try the same test, but this time with a UUID that if cast to an int (shouldn't) line up with
         // anything in the database.
         $uuid = '18180000' . substr(Uuid::uuid4()->toString(), 8);
         /** @var User $subuser */
-        $subuser = User::factory()->create(['uuid' => $uuid]);
+        $subuser = User::factory()->create(['uuid' => $uuid, 'username' => 'test1']);
 
         Subuser::query()->forceCreate([
             'user_id' => $subuser->id,
@@ -61,6 +62,7 @@ public function testCorrectSubuserIsDeletedFromServer()
         ]);
 
         $mock->expects('setServer->revokeUserJTI')->with($subuser->id)->andReturnUndefined();
+        $mock->expects('setServer->disconnectSFTP')->with($subuser->username)->andReturnUndefined();
 
         $this->actingAs($user)->deleteJson($this->link($server) . "/users/$subuser->uuid")->assertNoContent();
     }

diff --git a/tests/Integration/Api/Client/Server/Subuser/SubuserAuthorizationTest.php b/tests/Integration/Api/Client/Server/Subuser/SubuserAuthorizationTest.php
@@ -37,6 +37,7 @@ public function testUserCannotAccessResourceBelongingToOtherServers(string $meth
         $this->instance(DaemonServerRepository::class, $mock = \Mockery::mock(DaemonServerRepository::class));
         if ($method === 'DELETE') {
             $mock->expects('setServer->revokeUserJTI')->with($internal->id)->andReturnUndefined();
+            $mock->expects('setServer->disconnectSFTP')->with($internal->username)->andReturnUndefined();
         }
 
         // This route is acceptable since they're accessing a subuser on their own server.