diff --git a/app/Services/Servers/GetUserPermissionsService.php b/app/Services/Servers/GetUserPermissionsService.php index 93b659103..0c645e5c9 100644 --- a/app/Services/Servers/GetUserPermissionsService.php +++ b/app/Services/Servers/GetUserPermissionsService.php @@ -31,13 +31,22 @@ class GetUserPermissionsService 'admin.websocket.transfer', ]; - if ($isAdmin) { - return $isOwner || $user->can('update', $server) ? array_merge(['*'], $adminPermissions) : array_merge([SubuserPermission::WebsocketConnect->value], $adminPermissions); + if ($isAdmin && ($isOwner || $user->can('update', $server))) { + return array_merge(['*'], $adminPermissions); } /** @var Subuser|null $subuser */ $subuser = $server->subusers()->where('user_id', $user->id)->first(); + $subuserPermissions = $subuser !== null ? $subuser->permissions : []; - return $subuser->permissions ?? []; + if ($isAdmin) { + return array_unique(array_merge( + [SubuserPermission::WebsocketConnect->value], + $adminPermissions, + $subuserPermissions, + )); + } + + return $subuserPermissions; } } diff --git a/tests/Integration/Api/Remote/SftpAuthenticationControllerTest.php b/tests/Integration/Api/Remote/SftpAuthenticationControllerTest.php index 13e5ff8e6..f8e6e2d9a 100644 --- a/tests/Integration/Api/Remote/SftpAuthenticationControllerTest.php +++ b/tests/Integration/Api/Remote/SftpAuthenticationControllerTest.php @@ -11,6 +11,7 @@ use App\Models\User; use App\Models\UserSSHKey; use App\Tests\Integration\IntegrationTestCase; use PHPUnit\Framework\Attributes\DataProvider; +use Spatie\Permission\Models\Permission; class SftpAuthenticationControllerTest extends IntegrationTestCase { @@ -195,6 +196,39 @@ class SftpAuthenticationControllerTest extends IntegrationTestCase $this->post('/api/remote/sftp/auth', $data)->assertForbidden(); } + public function test_subuser_sftp_works_when_user_has_view_only_role(): void + { + [$user, $server] = $this->generateTestAccount([SubuserPermission::FileRead, SubuserPermission::FileSftp]); + + $user->update(['password' => password_hash('foobar', PASSWORD_DEFAULT)]); + + $this->setAuthorization($server->node); + + $data = [ + 'username' => $user->username . '.' . $server->uuid_short, + 'password' => 'foobar', + ]; + + // SFTP works as a plain subuser + $this->postJson('/api/remote/sftp/auth', $data) + ->assertOk() + ->assertJsonPath('permissions', [SubuserPermission::FileRead->value, SubuserPermission::FileSftp->value]); + + // Assign a role with only "view server" permission + $role = Role::findOrCreate('view-only-test', 'web'); + $permission = Permission::findOrCreate('view server', 'web'); + $role->givePermissionTo($permission); + $user->syncRoles($role); + + // SFTP should still work — subuser permissions must be merged with admin permissions + $response = $this->postJson('/api/remote/sftp/auth', $data) + ->assertOk(); + + $permissions = $response->json('permissions'); + $this->assertContains(SubuserPermission::FileSftp->value, $permissions); + $this->assertContains(SubuserPermission::FileRead->value, $permissions); + } + public static function authorizationTypeDataProvider(): array { return [